Commit Graph

16 Commits

  • (tui): Decode percent-escaped bare local file links (#16810)
    Addresses #16622
    
    Problem: bare local file links in TUI markdown render percent-encoded
    path bytes literally, unlike file:// links.
    
    Solution: decode bare path targets before local-path expansion and add
    regression coverage for spaces and Unicode.
  • Rename tui_app_server to tui (#16104)
    This is a follow-up to https://github.com/openai/codex/pull/15922. That
    previous PR deleted the old `tui` directory and left the new
    `tui_app_server` directory in place. This PR renames `tui_app_server` to
    `tui` and fixes up all references.
  • Remove the legacy TUI split (#15922)
    This is the part 1 of 2 PRs that will delete the `tui` /
    `tui_app_server` split. This part simply deletes the existing `tui`
    directory and marks the `tui_app_server` feature flag as removed. I left
    the `tui_app_server` feature flag in place for now so its presence
    doesn't result in an error. It is simply ignored.
    
    Part 2 will rename the `tui_app_server` directory `tui`. I did this as
    two parts to reduce visible code churn.
  • Apply argument comment lint across codex-rs (#14652)
    ## Why
    
    Once the repo-local lint exists, `codex-rs` needs to follow the
    checked-in convention and CI needs to keep it from drifting. This commit
    applies the fallback `/*param*/` style consistently across existing
    positional literal call sites without changing those APIs.
    
    The longer-term preference is still to avoid APIs that require comments
    by choosing clearer parameter types and call shapes. This PR is
    intentionally the mechanical follow-through for the places where the
    existing signatures stay in place.
    
    After rebasing onto newer `main`, the rollout also had to cover newly
    introduced `tui_app_server` call sites. That made it clear the first cut
    of the CI job was too expensive for the common path: it was spending
    almost as much time installing `cargo-dylint` and re-testing the lint
    crate as a representative test job spends running product tests. The CI
    update keeps the full workspace enforcement but trims that extra
    overhead from ordinary `codex-rs` PRs.
    
    ## What changed
    
    - keep a dedicated `argument_comment_lint` job in `rust-ci`
    - mechanically annotate remaining opaque positional literals across
    `codex-rs` with exact `/*param*/` comments, including the rebased
    `tui_app_server` call sites that now fall under the lint
    - keep the checked-in style aligned with the lint policy by using
    `/*param*/` and leaving string and char literals uncommented
    - cache `cargo-dylint`, `dylint-link`, and the relevant Cargo
    registry/git metadata in the lint job
    - split changed-path detection so the lint crate's own `cargo test` step
    runs only when `tools/argument-comment-lint/*` or `rust-ci.yml` changes
    - continue to run the repo wrapper over the `codex-rs` workspace, so
    product-code enforcement is unchanged
    
    Most of the code changes in this commit are intentionally mechanical
    comment rewrites or insertions driven by the lint itself.
    
    ## Verification
    
    - `./tools/argument-comment-lint/run.sh --workspace`
    - `cargo test -p codex-tui-app-server -p codex-tui`
    - parsed `.github/workflows/rust-ci.yml` locally with PyYAML
    
    ---
    
    * -> #14652
    * #14651
  • tui: restore visible line numbers for hidden file links (#12870)
    we recently changed file linking so the model uses markdown links when
    it wants something to be clickable.
    
    This works well across the GUI surfaces because they can render markdown
    cleanly and use the full absolute path in the anchor target.
    
    A previous pass hid the absolute path in the TUI (and only showed the
    label), but that also meant we could lose useful location info when the
    model put the line number or range in the anchor target instead of the
    label.
    
    This follow-up keeps the TUI behavior simple while making local file
    links feel closer to the old TUI file reference style.
    
    key changes:
    - Local markdown file links in the TUI keep the old file-ref feel: code
    styling, no underline, no visible absolute path.
    - If the hidden local anchor target includes a location suffix and the
    label does not already include one, we append that suffix to the visible
    label.
    - This works for single lines, line/column references, and ranges.
    - If the label already includes the location, we leave it alone.
    - normal web links keep the old TUI markdown-link behavior
    
    some examples:
    - `[foo.rs](/abs/path/foo.rs)` renders as `foo.rs`
    - `[foo.rs](/abs/path/foo.rs:45)` renders as `foo.rs:45`
    - `[foo.rs](/abs/path/foo.rs:45:3-48:9)` renders as `foo.rs:45:3-48:9`
    - `[foo.rs:45](/abs/path/foo.rs:45)` stays `foo.rs:45`
    - `[docs](https://example.com/docs)` still renders like a normal web
    link
    
    how it looks:
    <img width="732" height="813" alt="Screenshot 2026-02-26 at 9 27 55 AM"
    src="https://github.com/user-attachments/assets/d51bf236-653a-4e83-96e4-9427f0804471"
    />
  • Hide local file link destinations in TUI markdown (#12705)
    ## Summary
    - hide appended destinations for local path-style markdown links in the
    TUI renderer
    - keep web links rendering with their visible destination and style link
    labels consistently
    - add markdown renderer tests and a snapshot for the new file-link
    output
    
    ## Testing
    - just fmt
    - cargo test -p codex-tui
    <img width="1120" height="968" alt="image"
    src="https://github.com/user-attachments/assets/490e8eda-ae47-4231-89fa-b254a1f83eed"
    />
  • feat(tui): syntax highlighting via syntect with theme picker (#11447)
    ## Summary
    
    Adds syntax highlighting to the TUI for fenced code blocks in markdown
    responses and file diffs, plus a `/theme` command with live preview and
    persistent theme selection. Uses syntect (~250 grammars, 32 bundled
    themes, ~1 MB binary cost) — the same engine behind `bat`, `delta`, and
    `xi-editor`. Includes guardrails for large inputs, graceful fallback to
    plain text, and SSH-aware clipboard integration for the `/copy` command.
    
    <img width="1554" height="1014" alt="image"
    src="https://github.com/user-attachments/assets/38737a79-8717-4715-b857-94cf1ba59b85"
    />
    
    <img width="2354" height="1374" alt="image"
    src="https://github.com/user-attachments/assets/25d30a00-c487-4af8-9cb6-63b0695a4be7"
    />
    
    ## Problem
    
    Code blocks in the TUI (markdown responses and file diffs) render
    without syntax highlighting, making it hard to scan code at a glance.
    Users also have no way to pick a color theme that matches their terminal
    aesthetic.
    
    ## Mental model
    
    The highlighting system has three layers:
    
    1. **Syntax engine** (`render::highlight`) -- a thin wrapper around
    syntect + two-face. It owns a process-global `SyntaxSet` (~250 grammars)
    and a `RwLock<Theme>` that can be swapped at runtime. All public entry
    points accept `(code, lang)` and return ratatui `Span`/`Line` vectors or
    `None` when the language is unrecognized or the input exceeds safety
    guardrails.
    
    2. **Rendering consumers** -- `markdown_render` feeds fenced code blocks
    through the engine; `diff_render` highlights Add/Delete content as a
    whole file and Update hunks per-hunk (preserving parser state across
    hunk lines). Both callers fall back to plain unstyled text when the
    engine returns `None`.
    
    3. **Theme lifecycle** -- at startup the config's `tui.theme` is
    resolved to a syntect `Theme` via `set_theme_override`. At runtime the
    `/theme` picker calls `set_syntax_theme` to swap themes live; on cancel
    it restores the snapshot taken at open. On confirm it persists `[tui]
    theme = "..."` to config.toml.
    
    ## Non-goals
    
    - Inline diff highlighting (word-level change detection within a line).
    - Semantic / LSP-backed highlighting.
    - Theme authoring tooling; users supply standard `.tmTheme` files.
    
    ## Tradeoffs
    
    | Decision | Upside | Downside |
    | ------------------------------------------------ |
    ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
    |
    -----------------------------------------------------------------------------------------------------------------------
    |
    | syntect over tree-sitter / arborium | ~1 MB binary increase for ~250
    grammars + 32 themes; battle-tested crate powering widely-used tools
    (`bat`, `delta`, `xi-editor`). tree-sitter would add ~12 MB for 20-30
    languages or ~35 MB for full coverage. | Regex-based; less structurally
    accurate than tree-sitter for some languages (e.g. language injections
    like JS-in-HTML). |
    | Global `RwLock<Theme>` | Enables live `/theme` preview without
    threading Theme through every call site | Lock contention risk
    (mitigated: reads vastly outnumber writes, single UI thread) |
    | Skip background / italic / underline from themes | Terminal BG
    preserved, avoids ugly rendering on some themes | Themes that rely on
    these properties lose fidelity |
    | Guardrails: 512 KB / 10k lines | Prevents pathological stalls on huge
    diffs or pastes | Very large files render without color |
    
    ## Architecture
    
    ```
    config.toml  ─[tui.theme]─>  set_theme_override()  ─>  THEME (RwLock)
                                                                  │
                      ┌───────────────────────────────────────────┘
                      │
      markdown_render ─── highlight_code_to_lines(code, lang) ─> Vec<Line>
      diff_render     ─── highlight_code_to_styled_spans(code, lang) ─> Option<Vec<Vec<Span>>>
                      │
                      │   (None ⇒ plain text fallback)
                      │
      /theme picker   ─── set_syntax_theme(theme)    // live preview swap
                      ─── current_syntax_theme()      // snapshot for cancel
                      ─── resolve_theme_by_name(name) // lookup by kebab-case
    ```
    
    Key files:
    
    - `tui/src/render/highlight.rs` -- engine, theme management, guardrails
    - `tui/src/diff_render.rs` -- syntax-aware diff line wrapping
    - `tui/src/theme_picker.rs` -- `/theme` command builder
    - `tui/src/bottom_pane/list_selection_view.rs` -- side content panel,
    callbacks
    - `core/src/config/types.rs` -- `Tui::theme` field
    - `core/src/config/edit.rs` -- `syntax_theme_edit()` helper
    
    ## Observability
    
    - `tracing::warn` when a configured theme name cannot be resolved.
    - `Config::startup_warnings` surfaces the same message as a TUI banner.
    - `tracing::error` when persisting theme selection fails.
    
    ## Tests
    
    - Unit tests in `highlight.rs`: language coverage, fallback behavior,
    CRLF stripping, style conversion, guardrail enforcement, theme name
    mapping exhaustiveness.
    - Unit tests in `diff_render.rs`: snapshot gallery at multiple terminal
    sizes (80x24, 94x35, 120x40), syntax-highlighted wrapping, large-diff
    guardrail, rename-to-different-extension highlighting, parser state
    preservation across hunk lines.
    - Unit tests in `theme_picker.rs`: preview rendering (wide + narrow),
    dim overlay on deletions, subtitle truncation, cancel-restore, fallback
    for unavailable configured theme.
    - Unit tests in `list_selection_view.rs`: side layout geometry, stacked
    fallback, buffer clearing, cancel/selection-changed callbacks.
    - Integration test in `lib.rs`: theme warning uses the final
    (post-resume) config.
    
    ## Cargo Deny: Unmaintained Dependency Exceptions
    
    This PR adds two `cargo deny` advisory exceptions for transitive
    dependencies pulled in by `syntect v5.3.0`:
    
    | Advisory | Crate | Status |
    |----------|-------|--------|
    | RUSTSEC-2024-0320 | `yaml-rust` | Unmaintained (maintainer
    unreachable) |
    | RUSTSEC-2025-0141 | `bincode` | Unmaintained (development ceased;
    v1.3.3 considered complete) |
    
    **Why this is safe in our usage:**
    
    - Neither advisory describes a known security vulnerability. Both are
    "unmaintained" notices only.
    - `bincode` is used by syntect to deserialize pre-compiled syntax sets.
    Again, these are **static vendored artifacts** baked into the binary at
    build time. No user-supplied bincode data is ever deserialized. - Attack
    surface is zero for both crates; exploitation would require a
    supply-chain compromise of our own build artifacts.
    - These exceptions can be removed when syntect migrates to `yaml-rust2`
    and drops `bincode`, or when alternative crates are available upstream.
  • fix(tui): preserve URL clickability across all TUI views (#12067)
    ## Problem
    
    Long URLs containing `/` and `-` characters are split across multiple
    terminal lines by `textwrap`'s default hyphenation rules. This breaks
    terminal link detection: emulators can no longer identify the URL as
    clickable, and copy-paste yields a truncated fragment. The issue affects
    every view that renders user or agent text — exec output, history cells,
    markdown, the app-link setup screen, and the VT100 scrollback path.
    
    A secondary bug compounds the first: `desired_height()` calculations
    count logical lines rather than viewport rows. When a URL overflows its
    line and wraps visually, the height budget is too small, causing content
    to clip or leave gaps.
    
    Here is how the complete URL is interpreted by the terminal before
    (first line only) and after (complete URL):
    
    | Before | After |
    |---|---|
    | <img width="777" height="1002" alt="Screenshot 2026-02-17 at 7 59 11
    PM"
    src="https://github.com/user-attachments/assets/193a89a0-7e56-49c5-8b76-53499a76e7e3"
    /> | <img width="777" height="1002" alt="Screenshot 2026-02-17 at 7 58
    40 PM"
    src="https://github.com/user-attachments/assets/0b9b4c14-aafb-439f-9ffe-f6bba556f95e"
    /> |
    
    ## Mental model
    
    The TUI now treats URL-like tokens as atomic units that must never be
    split by the wrapping engine. Every call site that previously used
    `word_wrap_*` has been migrated to `adaptive_wrap_*`, which inspects
    each line for URL-like tokens and switches wrapping strategy
    accordingly:
    
    - **Non-URL lines** follow the existing `textwrap` path unchanged (word
    boundaries, optional indentation, hyphenation).
    - **URL-only lines** (with at most decorative markers like `│`, `-`,
    `1.`) are emitted unwrapped so terminal link detection works; ratatui's
    `Wrap { trim: false }` handles the final character wrap at render time.
    - **Mixed lines** (URL + substantive non-URL prose) flow through
    `adaptive_wrap_line` so prose wraps naturally at word boundaries while
    URL tokens remain unsplit.
    
    Height measurement everywhere now delegates to
    `Paragraph::line_count(width)`, which accounts for the visual row cost
    of overflowed lines. This single source of truth replaces ad-hoc line
    counting in individual cells.
    
    For terminal scrollback (the VT100 path that prints history when the TUI
    exits), URL-only lines are emitted unwrapped so the terminal's own link
    detector can find them. Mixed URL+prose lines use adaptive wrapping so
    surrounding text wraps naturally. Continuation rows are pre-cleared to
    avoid stale content artifacts.
    
    ## Non-goals
    
    - Full RFC 3986 URL parsing. The detector is a conservative heuristic
    that covers `scheme://host`, bare domains (`example.com/path`),
    `localhost:port`, and IPv4 hosts. IPv6 (`[::1]:8080`) and exotic schemes
    are intentionally excluded from v1.
    - Changing wrapping behavior for non-URL content.
    - Reflowing or reformatting existing terminal scrollback on resize.
    
    ## Tradeoffs
    
    | Decision | Upside | Downside |
    |----------|--------|----------|
    | Heuristic URL detection vs. full parser | Fast, zero-alloc on the hot
    path; conservative enough to reject file paths like `src/main.rs` |
    False negatives on obscure URL formats (they get split as before) |
    | Adaptive (three-path) wrapping | Non-URL lines are untouched — no
    behavior change, no perf cost; mixed lines wrap prose naturally while
    preserving URLs | Three wrapping strategies to reason about when
    debugging layout |
    | Row-based truncation with line-unit ellipsis | Accurate viewport
    budget; stable "N lines omitted" count across terminal widths |
    `truncate_lines_middle` is more complex (must compute per-line row cost)
    |
    | Unwrapped URL-only lines in scrollback | Terminal emulators detect
    clickable links; copy-paste gets the full URL | TUI and scrollback
    formatting diverge for URL-only lines |
    | Default `desired_height` via `Paragraph::line_count` | DRY — most
    cells inherit correct measurement | Cells with custom layout must
    remember to override |
    
    ## Architecture
    
    ```mermaid
    flowchart TD
        A["adaptive_wrap_*()"] --> B{"line_contains_url_like?"}
        B -- No URL tokens --> C["word_wrap_line<br/>(textwrap default)"]
        B -- Has URL tokens --> D{"mixed URL + prose?"}
        D -- "URL-only<br/>(+ decorative markers)" --> E["emit unwrapped<br/>(terminal char-wraps)"]
        D -- "Mixed<br/>(URL + substantive text)" --> F["adaptive_wrap_line<br/>(AsciiSpace + custom WordSplitter)"]
        C --> G["Paragraph::line_count(w)<br/>(single height truth)"]
        E --> G
        F --> G
    ```
    
    **Changed files:**
    
    | File | Role |
    |------|------|
    | `wrapping.rs` | URL detection heuristics, mixed-line detection,
    `adaptive_wrap_*` functions, custom `WordSplitter` |
    | `exec_cell/render.rs` | Row-aware `truncate_lines_middle`, adaptive
    wrapping for command/output display |
    | `history_cell.rs` | Migrate all cell types to `adaptive_wrap_*`;
    default `desired_height` via `Paragraph::line_count` |
    | `insert_history.rs` | Three-path scrollback wrapping (unwrapped
    URL-only, adaptive mixed, word-wrapped text); continuation row clearing
    |
    | `app_link_view.rs` | Adaptive wrapping for setup URL; `desired_height`
    via `Paragraph::line_count` |
    | `markdown_render.rs` | Adaptive wrapping in `finish_paragraph` |
    | `model_migration.rs` | Viewport-aware wrapping for narrow-pane
    markdown |
    | `pager_overlay.rs` | `Wrap { trim: false }` for transcript and
    streaming chunks |
    | `queued_user_messages.rs` | Migrate to `adaptive_wrap_lines` |
    | `status/card.rs` | Migrate to `adaptive_wrap_lines` |
    
    ## Observability
    
    - **Ellipsis message** in truncated exec output reports omitted count in
    logical lines (stable across resize) rather than viewport rows
    (fluctuates).
    - URL detection is deterministic and stateless — no hidden caching or
    memoization to go stale.
    - Height mismatch bugs surface immediately as visual clipping or gaps;
    the `Paragraph::line_count` path is the same code ratatui uses at render
    time, so measurement and rendering cannot diverge.
    
    ## Tests
    
    26 new unit tests across 7 files, covering:
    
    - **URL integrity**: assert a URL-like token appears on exactly one
    rendered line (not split across two).
    - **Height accuracy**: compare `desired_height()` against
    `Paragraph::line_count()` for URL-containing content.
    - **Row-aware truncation**: verify ellipsis counts logical lines and
    output fits within the row budget.
    - **Scrollback rendering**: VT100 backend tests confirm prefix and URL
    land on the same row; continuation rows are cleared; mixed URL+prose
    lines wrap prose while preserving URL tokens.
    - **Mixed URL+prose detection**: `line_has_mixed_url_and_non_url_tokens`
    correctly distinguishes lines with substantive non-URL text from lines
    with only decorative markers alongside a URL.
    - **Heuristic correctness**: positive matches (`https://...`,
    `example.com/path`, `localhost:3000/api`, `192.168.1.1:8080/health`) and
    negative matches (`src/main.rs`, `foo/bar`, `hello-world`).
    
    ## Risks and open items
    
    1. **URL-like tokens in code output** (e.g. `example.com/api` inside a
    JSON blob) will trigger URL-preserving wrap on that line. This is
    acceptable — the worst case is a slightly wider line, not broken output.
    2. **Very long non-URL tokens on a URL line** can only break at
    character boundaries (the custom splitter emits all char indices for
    non-URL words). On extremely narrow terminals this could overflow, but
    narrow terminals already degrade gracefully.
    3. **No IPv6 support** — `[::1]:8080/path` will be treated as a non-URL
    and may get split. Can be added later without API changes.
    
    Fixes #5457
  • Changed default wrap algorithm from OptimalFit to FirstFit (#7960)
    Codex identified this as the cause of a reported hang:
    https://github.com/openai/codex/issues/7822. Apparently, the wrapping
    algorithm we're using has known issues and bad worst-case behaviors when
    OptimalFit is used on certain strings. It recommended switching to
    FirstFit instead.
  • tui: drop citation rendering (#4855)
    We don't instruct the model to use citations, so it never emits them.
    Further, ratatui [doesn't currently support rendering links into the
    terminal with OSC 8](https://github.com/ratatui/ratatui/issues/1028), so
    even if we did parse citations, we can't correctly render them.
    
    So, remove all the code related to rendering them.
  • wrap markdown at render time (#4506)
    This results in correctly indenting list items with long lines.
    
    <img width="1006" height="251" alt="Screenshot 2025-09-30 at 10 00
    48 AM"
    src="https://github.com/user-attachments/assets/0a076cf6-ca3c-4efb-b3af-dc07617cdb6f"
    />
  • Add cloud tasks (#3197)
    Adds a TUI for managing, applying, and creating cloud tasks
  • chore: clippy on redundant closure (#4058)
    Add redundant closure clippy rules and let Codex fix it by minimising
    FQP
  • replace tui_markdown with a custom markdown renderer (#3396)
    Also, simplify the streaming behavior.
    
    This fixes a number of display issues with streaming markdown, and paves
    the way for better markdown features (e.g. customizable styles, syntax
    highlighting, markdown-aware wrapping).
    
    Not currently supported:
    - footnotes
    - tables
    - reference-style links