Commit Graph

11 Commits

  • fix(tui): reduce startup and new-session latency (#17039)
    ## TL;DR
    
    - Fetches account/rateLimits/read asynchronously so the TUI can continue
    starting without waiting for the rate-limit response.
    - Fixes the /status card so it no longer leaves a stale “refreshing
    cached limits...” notice in terminal history.
    
    ## Problem
    
    The TUI bootstrap path fetched account rate limits synchronously
    (`account/rateLimits/read`) before the event loop started for
    ChatGPT/OpenAI-authenticated startups. This added ~670 ms of blocking
    latency in the measured hot-start case, even though rate-limit data is
    not needed to render the initial UI or accept user input. The delay was
    especially noticeable on hot starts where every other RPC
    (`account/read`, `model/list`, `thread/start`) completed in under 70 ms
    total.
    
    Moving that fetch to the background also exposed a `/status` UI bug: the
    status card is flattened into terminal scrollback when it is inserted. A
    transient "refreshing limits in background..." line could not be cleared
    later, because the async completion updated the retained `HistoryCell`,
    not the already-written terminal history.
    
    ## Mental model
    
    Before this change, `AppServerSession::bootstrap()` performed three
    sequential RPCs: `account/read` → `model/list` →
    `account/rateLimits/read`. The result of the third call was baked into
    `AppServerBootstrap` and applied to the chat widget before the event
    loop began.
    
    After this change, `bootstrap()` only performs two RPCs (`account/read`
    + `model/list`), and rate-limit fetching is kicked off as an async
    background task immediately after the first frame is scheduled. A new
    enum, `RateLimitRefreshOrigin`, tags each fetch so the event handler
    knows whether the result came from the startup prefetch or from a
    user-initiated `/status` command; they have different completion
    side-effects.
    
    The `get_login_status()` helper (used outside the main app flow) was
    also decoupled: it previously called the full `bootstrap()` just to
    check auth mode, wasting model-list and rate-limit work. It now calls
    the narrower `read_account()` directly.
    
    For `/status`, this PR keeps the background refresh request but stops
    printing transient refresh notices into status history when cached
    limits are already available. If a refresh updates the cache, the next
    `/status` command will render the new values.
    
    ## Non-goals
    
    - This change does not alter the rate-limit data itself.
    - This change does not introduce caching, retries, or staleness
    management for rate limits.
    - This change does not affect the `model/list` or `thread/start` RPCs;
    they remain on the critical startup path.
    
    ## Tradeoffs
    
    - **Stale-on-first-render**: The status bar will briefly show no
    rate-limit info until the background fetch completes; observed
    background fetches landed roughly in the 400-900 ms range after the UI
    appeared. This is acceptable because the user cannot meaningfully act on
    rate-limit data in the first fraction of a second.
    - **Error silence on startup prefetch**: If the startup prefetch fails,
    the error is logged but the UI is not notified (unlike `/status` refresh
    failures, which go through the status-command completion path). This
    avoids surfacing transient network errors as a startup blocker.
    - **Static `/status` history**: `/status` output is terminal history,
    not a live widget. The card now avoids progress-style language that
    would appear stuck in scrollback; users can run `/status` again to see
    newly cached values.
    - **`account_auth_mode` field removed from `AppServerBootstrap`**: The
    only consumer was `get_login_status()`, which no longer goes through
    `bootstrap()`. The field was dead weight.
    
    ## Architecture
    
    ### New types
    
    - `RateLimitRefreshOrigin` (in `app_event.rs`): A `Copy` enum
    distinguishing `StartupPrefetch` from `StatusCommand { request_id }`.
    Carried through `RefreshRateLimits` and `RateLimitsLoaded` events so the
    handler applies the right completion behavior.
    
    ### Modified types
    
    - `AppServerBootstrap`: Lost `account_auth_mode` and
    `rate_limit_snapshots`; gained `requires_openai_auth: bool` (passed
    through from the account response so the caller can decide whether to
    fire the prefetch).
    
    ### Control flow
    
    1. `bootstrap()` returns with `requires_openai_auth` and
    `has_chatgpt_account`.
    2. After scheduling the first frame, `App::run_inner` fires
    `refresh_rate_limits(StartupPrefetch)` if both flags are true.
    3. When `RateLimitsLoaded { StartupPrefetch, Ok(..) }` arrives,
    snapshots are applied and a frame is scheduled to repaint the status
    bar.
    4. When `RateLimitsLoaded { StartupPrefetch, Err(..) }` arrives, the
    error is logged and no UI update occurs.
    5. `/status`-initiated refreshes continue to use `StatusCommand {
    request_id }` and call `finish_status_rate_limit_refresh` on completion
    (success or failure).
    6. `/status` history cells with cached rate-limit rows no longer render
    an additional "refreshing limits" notice; the async refresh updates the
    cache for future status output.
    
    ### Extracted method
    
    - `AppServerSession::read_account()`: Factored out of `bootstrap()` so
    that `get_login_status()` can call it independently without triggering
    model-list or rate-limit work.
    
    ## Observability
    
    - The existing `tracing::warn!` for rate-limit fetch failures is
    preserved for the startup path.
    - No new metrics or spans are introduced. The startup-time improvement
    is observable via the existing `ready` timestamp in TUI startup logs.
    
    ## Tests
    
    - Existing tests in `status_command_tests.rs` are updated to match on
    `RateLimitRefreshOrigin::StatusCommand { request_id }` instead of a bare
    `request_id`.
    - Focused `/status` tests now assert that status history avoids
    transient refresh text, continues to request an async refresh, and uses
    refreshed cached limits in future status output.
    - No new tests are added for the startup prefetch path because it is a
    fire-and-forget spawn with no observable side-effect other than the
    widget state update, which is already covered by the
    snapshot-application tests.
    
    ---------
    
    Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
  • Use model metadata for Fast Mode status (#16949)
    Fast Mode status was still tied to one model name in the TUI and
    model-list plumbing. This changes the model metadata shape so a model
    can advertise additional speed tiers, carries that field through the
    app-server model list, and uses it to decide when to show Fast Mode
    status.
    
    For people using Codex, the behavior is intended to stay the same for
    existing models. Fast Mode still requires the existing signed-in /
    feature-gated path; the difference is that the UI can now recognize any
    model the model list marks as Fast-capable, instead of requiring a new
    client-side slug check.
  • [codex] Make AbsolutePathBuf joins infallible (#16981)
    Having to check for errors every time join is called is painful and
    unnecessary.
  • [codex] reduce module visibility (#16978)
    ## Summary
    - reduce public module visibility across Rust crates, preferring private
    or crate-private modules with explicit crate-root public exports
    - update external call sites and tests to use the intended public crate
    APIs instead of reaching through module trees
    - add the module visibility guideline to AGENTS.md
    
    ## Validation
    - `cargo check --workspace --all-targets --message-format=short` passed
    before the final fix/format pass
    - `just fix` completed successfully
    - `just fmt` completed successfully
    - `git diff --check` passed
  • [codex-analytics] add protocol-native turn timestamps (#16638)
    ---
    [//]: # (BEGIN SAPLING FOOTER)
    Stack created with [Sapling](https://sapling-scm.com). Best reviewed
    with [ReviewStack](https://reviewstack.dev/openai/codex/pull/16638).
    * #16870
    * #16706
    * #16659
    * #16641
    * #16640
    * __->__ #16638
  • Fix TUI fast mode toggle regression (#16833)
    Addresses #16832
    
    Problem: After `/fast on`, the TUI omitted an explicit service-tier
    clear on later turns, so `/fast off` left app-server sessions stuck on
    `priority` until restart.
    
    Solution: Always submit the current service tier with user turns,
    including an explicit clear when Fast mode is off, and add a regression
    test for the `/fast on` -> `/fast off` flow.
  • Remove OPENAI_BASE_URL config fallback (#16720)
    The `OPENAI_BASE_URL` environment variable has been a significant
    support issue, so we decided to deprecate it in favor of an
    `openai_base_url` config key. We've had the deprecation warning in place
    for about a month, so users have had time to migrate to the new
    mechanism. This PR removes support for `OPENAI_BASE_URL` entirely.
  • Fix TUI app-server permission profile conversions (#16284)
    Addresses #16283
    
    Problem: TUI app-server permission approvals could drop filesystem
    grants because request and response payloads were round-tripped through
    mismatched camelCase and snake_case JSON shapes.
    Solution: Replace the lossy JSON round-trips with typed app-server/core
    permission conversions so requested and granted permission profiles,
    including filesystem paths and scope, are preserved end to end.
  • fix(guardian): make GuardianAssessmentEvent.action strongly typed (#16448)
    ## Description
    
    Previously the `action` field on `EventMsg::GuardianAssessment`, which
    describes what Guardian is reviewing, was typed as an arbitrary JSON
    blob. This PR cleans it up and defines a sum type representing all the
    various actions that Guardian can review.
    
    This is a breaking change (on purpose), which is fine because:
    - the Codex app / VSCE does not actually use `action` at the moment
    - the TUI code that consumes `action` is updated in this PR as well
    - rollout files that serialized old `EventMsg::GuardianAssessment` will
    just silently drop these guardian events
    - the contract is defined as unstable, so other clients have a fair
    warning :)
    
    This will make things much easier for followup Guardian work.
    
    ## Why
    
    The old guardian review payloads worked, but they pushed too much shape
    knowledge into downstream consumers. The TUI had custom JSON parsing
    logic for commands, patches, network requests, and MCP calls, and the
    app-server protocol was effectively just passing through an opaque blob.
    
    Typing this at the protocol boundary makes the contract clearer.
  • Fix stale /status rate limits in active TUI sessions (#16201)
    Fix stale weekly limit in `/status` (#16194): /status reused the
    session’s cached rate-limit snapshot, so the weekly remaining limit
    could stay frozen within an active session.
    
    With this change, we now dynamically update the rate limits after status
    is displayed.
    
    I needed to delete a few low-value test cases from the chatWidget tests
    because the test.rs file is really large, and the new tests in this PR
    pushed us over the 512K mandated limit. I'm working on a separate PR to
    refactor that test file.
  • Refactor chatwidget tests into topical modules (#16361)
    Problem: `chatwidget/tests.rs` had grown into a single oversized test
    blob that was hard to maintain and exceeded the repo's blob size limit.
    
    Solution: split the chatwidget tests into topical modules with a thin
    root `tests.rs`, shared helper utilities, preserved snapshot naming, and
    hermetic test config so the refactor stays stable and passes the
    `codex-tui` test suite.