Commit Graph

3 Commits

  • fix(tui): handle zellij redraw and composer rendering (#16578)
    ## TL;DR
    
    Fixes the issues when using Codex CLI with Zellij multiplexer. Before
    this PR there would be no scrollback when using it inside a zellij
    terminal.
    
    ## Problem
    
    Addresses #2558
    
    Zellij does not support ANSI scroll-region manipulation (`DECSTBM` /
    Reverse Index) or the alternate screen buffer in the way traditional
    terminals do. When codex's TUI runs inside Zellij, two things break: (1)
    inline history insertion corrupts the display because the scroll-region
    escape sequences are silently dropped or mishandled, and (2) the
    composer textarea renders with inherited background/foreground styles
    that produce unreadable text against Zellij's pane chrome.
    
    ## Mental model
    
    The fix introduces a **Zellij mode** — a runtime boolean detected once
    at startup via `codex_terminal_detection::terminal_info().is_zellij()` —
    that gates two subsystems onto Zellij-safe terminal strategies:
    
    - **History insertion** (`insert_history.rs`): Instead of using
    `DECSTBM` scroll regions and Reverse Index (`ESC M`) to slide content
    above the viewport, Zellij mode scrolls the screen by emitting `\n` at
    the bottom row and then writes history lines at absolute positions. This
    avoids every escape sequence Zellij mishandles.
    - **Viewport expansion** (`tui.rs`): When the viewport grows taller than
    available space, the standard path uses `scroll_region_up` on the
    backend. Zellij mode instead emits newlines at the screen bottom to push
    content up, then invalidates the ratatui diff buffer so the next draw is
    a full repaint.
    - **Composer rendering** (`chat_composer.rs`, `textarea.rs`): All text
    rendering in the input area uses an explicit `base_style` with
    `Color::Reset` foreground, preventing Zellij's pane styling from
    bleeding into the textarea. The prompt chevron (`›`) and placeholder
    text use explicit color constants instead of relying on `.bold()` /
    `.dim()` modifiers that render inconsistently under Zellij.
    
    ## Non-goals
    
    - This change does not fix or improve Zellij's terminal emulation
    itself.
    - It does not rearchitect the inline viewport model; it adds a parallel
    code path gated on detection.
    - It does not touch the alternate-screen disable logic (that already
    existed and continues to use `is_zellij` via the same detection).
    
    ## Tradeoffs
    
    - **Code duplication in `insert_history.rs`**: The Zellij and Standard
    branches share the line-rendering loop (color setup, span merging,
    `write_spans`) but differ in the scrolling preamble. The duplication is
    intentional — merging them would force a complex conditional state
    machine that's harder to reason about than two flat sequences.
    - **`invalidate_viewport` after every Zellij history flush or viewport
    expansion**: This forces a full repaint on every draw cycle in Zellij,
    which is more expensive than ratatui's normal diff-based rendering. This
    is necessary because Zellij's lack of scroll-region support means the
    diff buffer's assumptions about what's on screen are invalid after we
    manually move content.
    - **Explicit colors vs semantic modifiers**: Replacing `.bold()` /
    `.dim()` with `Color::Cyan` / `Color::DarkGray` / `Color::White` in the
    Zellij branch sacrifices theme-awareness for correctness. If the project
    ever adopts a theming system, Zellij styling will need to participate.
    
    ## Architecture
    
    The Zellij detection flag flows through three layers:
    
    1. **`codex_terminal_detection`** — `TerminalInfo::is_zellij()` (new
    convenience method) reads the already-detected `Multiplexer` variant.
    2. **`Tui` struct** — caches `is_zellij` at construction; passes it into
    `update_inline_viewport`, `flush_pending_history_lines`, and
    `insert_history_lines_with_mode`.
    3. **`ChatComposer` struct** — independently caches `is_zellij` at
    construction; uses it in `render_textarea` for style decisions.
    
    The two caches (`Tui.is_zellij` and `ChatComposer.is_zellij`) are read
    from the same global `OnceLock<TerminalInfo>`, so they always agree.
    
    ## Observability
    
    No new logging, metrics, or tracing is introduced. Diagnosis depends on:
    - Whether `ZELLIJ` or `ZELLIJ_SESSION_NAME` env vars are set (the
    detection heuristic).
    - Visual inspection of the rendered TUI inside Zellij vs a standard
    terminal.
    - The insta snapshot `zellij_empty_composer` captures the Zellij-mode
    render path.
    
    ## Tests
    
    - `terminal_info_reports_is_zellij` — unit test in `terminal-detection`
    confirming the convenience method.
    - `zellij_empty_composer_snapshot` — insta snapshot in `chat_composer`
    validating the Zellij render path for an empty composer.
    - `vt100_zellij_mode_inserts_history_and_updates_viewport` — integration
    test in `insert_history` verifying that Zellij-mode history insertion
    writes content and shifts the viewport.
    
    ---------
    
    Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
  • chore: clean up argument-comment lint and roll out all-target CI on macOS (#16054)
    ## Why
    
    `argument-comment-lint` was green in CI even though the repo still had
    many uncommented literal arguments. The main gap was target coverage:
    the repo wrapper did not force Cargo to inspect test-only call sites, so
    examples like the `latest_session_lookup_params(true, ...)` tests in
    `codex-rs/tui_app_server/src/lib.rs` never entered the blocking CI path.
    
    This change cleans up the existing backlog, makes the default repo lint
    path cover all Cargo targets, and starts rolling that stricter CI
    enforcement out on the platform where it is currently validated.
    
    ## What changed
    
    - mechanically fixed existing `argument-comment-lint` violations across
    the `codex-rs` workspace, including tests, examples, and benches
    - updated `tools/argument-comment-lint/run-prebuilt-linter.sh` and
    `tools/argument-comment-lint/run.sh` so non-`--fix` runs default to
    `--all-targets` unless the caller explicitly narrows the target set
    - fixed both wrappers so forwarded cargo arguments after `--` are
    preserved with a single separator
    - documented the new default behavior in
    `tools/argument-comment-lint/README.md`
    - updated `rust-ci` so the macOS lint lane keeps the plain wrapper
    invocation and therefore enforces `--all-targets`, while Linux and
    Windows temporarily pass `-- --lib --bins`
    
    That temporary CI split keeps the stricter all-targets check where it is
    already cleaned up, while leaving room to finish the remaining Linux-
    and Windows-specific target-gated cleanup before enabling
    `--all-targets` on those runners. The Linux and Windows failures on the
    intermediate revision were caused by the wrapper forwarding bug, not by
    additional lint findings in those lanes.
    
    ## Validation
    
    - `bash -n tools/argument-comment-lint/run.sh`
    - `bash -n tools/argument-comment-lint/run-prebuilt-linter.sh`
    - shell-level wrapper forwarding check for `-- --lib --bins`
    - shell-level wrapper forwarding check for `-- --tests`
    - `just argument-comment-lint`
    - `cargo test` in `tools/argument-comment-lint`
    - `cargo test -p codex-terminal-detection`
    
    ## Follow-up
    
    - Clean up remaining Linux-only target-gated callsites, then switch the
    Linux lint lane back to the plain wrapper invocation.
    - Clean up remaining Windows-only target-gated callsites, then switch
    the Windows lint lane back to the plain wrapper invocation.
  • Move terminal module to terminal-detection crate (#15216)
    - Move core/src/terminal.rs and its tests into a standalone
    terminal-detection workspace crate.
    - Update direct consumers to depend on codex-terminal-detection and
    import terminal APIs directly.
    
    ---------
    
    Co-authored-by: Codex <noreply@openai.com>