Commit Graph

11 Commits

  • tui_app_server: cancel active login before Ctrl+C exit (#15673)
    ## Summary
    
    Fixes slow `Ctrl+C` exit from the ChatGPT browser-login screen in
    `tui_app_server`.
    
    ## Root cause
    
    Onboarding-level `Ctrl+C` quit bypassed the auth widget's cancel path.
    That let the active ChatGPT login keep running, and in-process
    app-server shutdown then waited on the stale login attempt before
    finishing.
    
    ## Changes
    
    - Extract a shared `cancel_active_attempt()` path in the auth widget
    - Use that path from onboarding-level `Ctrl+C` before exiting the TUI
    - Add focused tests for canceling browser-login and device-code attempts
    - Add app-server shutdown cleanup that explicitly drops any active login
    before draining background work
  • app-server: Add back pressure and batching to command/exec (#15547)
    * Add
    `OutgoingMessageSender::send_server_notification_to_connection_and_wait`
    which returns only once message is written to websocket (or failed to do
    so)
    * Use this mechanism to apply back pressure to stdout/stderr streams of
    processes spawned by `command/exec`, to limit them to at most one
    message in-memory at a time
    * Use back pressure signal to also batch smaller chunks into ≈64KiB ones
    
    This should make commands execution more robust over
    high-latency/low-throughput networks
  • Finish moving codex exec to app-server (#15424)
    This PR completes the conversion of non-interactive `codex exec` to use
    app server rather than directly using core events and methods.
    
    ### Summary
    - move `codex-exec` off exec-owned `AuthManager` and `ThreadManager`
    state
    - route exec bootstrap, resume, and auth refresh through existing
    app-server paths
    - replace legacy `codex/event/*` decoding in exec with typed app-server
    notification handling
    - update human and JSONL exec output adapters to translate existing
    app-server notifications only
    - clean up "app server client" layer by eliminating support for legacy
    notifications; this is no longer needed
    - remove exposure of `authManager` and `threadManager` from "app server
    client" layer
    
    ### Testing
    - `exec` has pretty extensive unit and integration tests already, and
    these all pass
    - In addition, I asked Codex to put together a comprehensive manual set
    of tests to cover all of the `codex exec` functionality (including
    command-line options), and it successfully generated and ran these tests
  • Revert "fix: harden plugin feature gating" (#15102)
    Reverts openai/codex#15020
    
    I messed up the commit in my PR and accidentally merged changes that
    were still under review.
  • fix: harden plugin feature gating (#15020)
    1. Use requirement-resolved config.features as the plugin gate.
    2. Guard plugin/list, plugin/read, and related flows behind that gate.
    3. Skip bad marketplace.json files instead of failing the whole list.
    4. Simplify plugin state and caching.
  • 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
  • fix: tui freeze when sub-agents are present (#14816)
    The issue was due to a circular `Drop` schema where the embedded
    app-server wait for some listeners that wait for this app-server
    them-selves.
    
    The fix is an explicit cleaning
    
    **Repro:**
    * Start codex
    * Ask it to spawn a sub-agent
    * Close Codex
    * It takes 5s to exit
  • Start TUI on embedded app server (#14512)
    This PR is part of the effort to move the TUI on top of the app server.
    In a previous PR, we introduced an in-process app server and moved
    `exec` on top of it.
    
    For the TUI, we want to do the migration in stages. The app server
    doesn't currently expose all of the functionality required by the TUI,
    so we're going to need to support a hybrid approach as we make the
    transition.
    
    This PR changes the TUI initialization to instantiate an in-process app
    server and access its `AuthManager` and `ThreadManager` rather than
    constructing its own copies. It also adds a placeholder TUI event
    handler that will eventually translate app server events into TUI
    events. App server notifications are accepted but ignored for now. It
    also adds proper shutdown of the app server when the TUI terminates.
  • feat(app-server): propagate traces across tasks and core ops (#14387)
    ## Summary
    
    This PR keeps app-server RPC request trace context alive for the full
    lifetime of the work that request kicks off (e.g. for `thread/start`,
    this is `app-server rpc handler -> tokio background task -> core op
    submissions`). Previously we lose trace lineage once the request handler
    returns or hands work off to background tasks.
    
    This approach is especially relevant for `thread/start` and other RPC
    handlers that run in a non-blocking way. In the near future we'll most
    likely want to make all app-server handlers run in a non-blocking way by
    default, and only queue operations that must operate in order (e.g.
    thread RPCs per thread?), so we want to make sure tracing in app-server
    just generally works.
    
    Depends on https://github.com/openai/codex/pull/14300
    
    **Before**
    <img width="155" height="207" alt="image"
    src="https://github.com/user-attachments/assets/c9487459-36f1-436c-beb7-fafeb40737af"
    />
    
    
    **After**
    <img width="299" height="337" alt="image"
    src="https://github.com/user-attachments/assets/727392b2-d072-4427-9dc4-0502d8652dea"
    />
    
    ## What changed
    
    - Keep request-scoped trace context around until we send the final
    response or error, or the connection closes.
    - Thread that trace context through detached `thread/start` work so
    background startup stays attached to the originating request.
    - Pass request trace context through to downstream core operations,
    including:
      - thread creation
      - resume/fork flows
      - turn submission
      - review
      - interrupt
      - realtime conversation operations
    - Add tracing tests that verify:
      - remote W3C trace context is preserved for `thread/start`
      - remote W3C trace context is preserved for `turn/start`
      - downstream core spans stay under the originating request span
      - request-scoped tracing state is cleaned up correctly
    - Clean up shutdown behavior so detached background tasks and spawned
    threads are drained before process exit.
  • chore(app-server): stop emitting codex/event/ notifications (#14392)
    ## Description
    
    This PR stops emitting legacy `codex/event/*` notifications from the
    public app-server transports.
    
    It's been a long time coming! app-server was still producing a raw
    notification stream from core, alongside the typed app-server
    notifications and server requests, for compatibility reasons. Now,
    external clients should no longer be depending on those legacy
    notifications, so this change removes them from the stdio and websocket
    contract and updates the surrounding docs, examples, and tests to match.
    
    ### Caveat
    I left the "in-process" version of app-server alone for now, since
    `codex exec` was recently based on top of app-server via this in-process
    form here: https://github.com/openai/codex/pull/14005
    
    Seems like `codex exec` still consumes some legacy notifications
    internally, so this branch only removes `codex/event/*` from app-server
    over stdio and websockets.
    
    ## Follow-up
    
    Once `codex exec` is fully migrated off `codex/event/*` notifications,
    we'll be able to stop emitting them entirely entirely instead of just
    filtering it at the external transport boundary.
  • Add in-process app server and wire up exec to use it (#14005)
    This is a subset of PR #13636. See that PR for a full overview of the
    architectural change.
    
    This PR implements the in-process app server and modifies the
    non-interactive "exec" entry point to use the app server.
    
    ---------
    
    Co-authored-by: Felipe Coury <felipe.coury@gmail.com>