mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
01f89c8c59fc2d92f7c60bfb834a8f01635e57a1
8
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  Eric TrautandGitHub
|
227bee0445 |
Enforce animations = false for screen readers (#20564)
## Why Issue #20489 calls out that animated TUI affordances can be noisy for screen-reader users. Codex already has `tui.animations = false` as a reduced-motion setting, but some live activity rows render spinner-style prefixes in that mode. These were relatively recent regressions. We have also regressed this pattern more than once by adding new spinner/shimmer callsites that do not think through the reduced-motion path, so this PR adds a small guardrail while fixing the current surfaces. ## What changed - Omit the live status-row spinner when animations are disabled, so the row starts with stable text like `Working (...)`. - Render running hook headers without the spinner prefix when animations are disabled, while preserving shimmer/spinner behavior when animations are enabled. - Centralize TUI activity indicators in `tui/src/motion.rs`, with explicit reduced-motion choices for hidden prefixes, static bullets, and plain shimmer-text fallbacks. - Route existing spinner/shimmer callsites through the central motion helper, including exec rows, MCP/web-search/loading rows, hook rows, plugin loading, and onboarding loading text. - Add a source-scan regression test that rejects direct `spinner(...)` or `shimmer_spans(...)` usage outside the central module and primitive definition. - Add focused coverage that reduced-motion active exec rows are stable, status rows start without a spinner, running hooks omit the spinner, and MCP inventory loading stays stable. - Update the one affected status-indicator snapshot; the existing detail tree prefix remains unchanged. ## Verification - `cargo test -p codex-tui` |
||
 Felipe CouryandGitHub
|
5e737372ee |
feat(tui): add configurable keymap support (#18593)
## Why The TUI currently handles keyboard shortcuts as hard-coded event matches spread across app, composer, pager, list, approval, and navigation code. That makes shortcuts hard to customize, makes displayed hints easy to drift from actual behavior, and makes future keymap work riskier because there is no central action inventory. This PR adds the foundation for configurable, action-based keymaps without adding the interactive remapping UI yet. Onboarding intentionally stays on fixed startup shortcuts because users cannot reasonably configure keymaps before completing onboarding. This is PR1 in the keymap stack: - PR1: #18593: configurable keymap foundation - PR2: #18594: `/keymap` picker and guided remapping UI - PR3: #18595: Vim composer mode and the remap option ## Design Notes The new model resolves named actions into concrete runtime bindings once from config, then passes those bindings to the UI surfaces that handle input or render shortcut hints. The main concepts are: - **Context**: a scope where an action is active, such as `global`, `chat`, `composer`, `editor`, `pager`, `list`, or `approval`. - **Action**: a named operation inside a context, such as `global.open_transcript`, `composer.submit`, or `pager.close`. - **Binding**: one or more single-key shortcuts assigned to an action, written as config strings such as `ctrl-t`, `alt-backspace`, or `page-down`. Multi-step sequences such as `ctrl-x ctrl-s`, `g g`, or leader-key flows are not part of this PR. - **Resolution order**: context-specific config wins first, supported global fallbacks come next, and built-in defaults fill in anything unset. - **Explicit unbinding**: an empty array removes an action binding in that scope and does not fall through to a fallback binding. - **Conflict validation**: a resolved keymap rejects duplicate active bindings inside the same scope so one keypress cannot dispatch two actions. ## What Changed - Added `TuiKeymap` config support under `[tui.keymap]`, including typed contexts/actions, key alias normalization, generated schema coverage, and user-facing config errors. - Added `RuntimeKeymap` resolution in `codex-rs/tui/src/keymap.rs`, including fallback precedence, built-in defaults, explicit unbinding, and per-context conflict validation. - Rewired existing TUI handlers to consume resolved keymap actions instead of directly matching hard-coded keys in each component. - Updated key hint rendering and footer/pager/list surfaces so displayed shortcuts follow the resolved keymap. - Kept onboarding shortcuts fixed in `codex-rs/tui/src/onboarding/keys.rs` instead of exposing them through `[tui.keymap]`. ## Validation The branch includes focused coverage for config parsing, key normalization, runtime fallback resolution, explicit unbinding, duplicate-key conflict validation, default keymap consistency, onboarding startup key behavior, and UI hint snapshots affected by resolved key bindings. |
||
|
Eric TrautandGitHub
|
e88c2cf4d7 |
tui: route device-code auth through app server (#16827)
Addresses #7646 Also enables device code auth for remote TUI sessions Problem: TUI onboarding handled device-code login directly rather than using the recently-added app server support for device auth. Also, auth screens kept animating while users needed to copy login details. Solution: Route device-code onboarding through app-server login APIs and make the auth screens static while those copy-oriented flows are visible. |
||
 Michael BolinandGitHub
|
aa2403e2eb |
core: remove cross-crate re-exports from lib.rs (#16512)
## Why `codex-core` was re-exporting APIs owned by sibling `codex-*` crates, which made downstream crates depend on `codex-core` as a proxy module instead of the actual owner crate. Removing those forwards makes crate boundaries explicit and lets leaf crates drop unnecessary `codex-core` dependencies. In this PR, this reduces the dependency on `codex-core` to `codex-login` in the following files: ``` codex-rs/backend-client/Cargo.toml codex-rs/mcp-server/tests/common/Cargo.toml ``` ## What - Remove `codex-rs/core/src/lib.rs` re-exports for symbols owned by `codex-login`, `codex-mcp`, `codex-rollout`, `codex-analytics`, `codex-protocol`, `codex-shell-command`, `codex-sandboxing`, `codex-tools`, and `codex-utils-path`. - Delete the `default_client` forwarding shim in `codex-rs/core`. - Update in-crate and downstream callsites to import directly from the owning `codex-*` crate. - Add direct Cargo dependencies where callsites now target the owner crate, and remove `codex-core` from `codex-rs/backend-client`. |
||
|
Eric TrautandGitHub
|
61429a6c10 |
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. |
||
|
Eric TrautandGitHub
|
d65deec617 |
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. |
||
 Felipe CouryandGitHub
|
2ba2c57af4 |
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 |
||
 Matthew ZengandGitHub
|
24d6e0114f |
[device-auth] When headless environment is detected, show device login flow instead. (#8756)
When headless environment is detected, show device login flow instead. |