7 Commits

  • fix(remote-control): avoid server token refresh retry storms (#30201)
    ## Why
    
    Remote-control websocket reconnects and pairing requests proactively
    refresh their server token. When `/server/refresh` returns a transient
    error such as `502`, the still-valid token was discarded as a usable
    connection path, causing reconnect failures and repeated refresh
    attempts that could amplify an upstream incident.
    
    ## What Changed
    
    - Start proactive refresh five minutes before token expiry and
    distinguish it from a required refresh for missing or expired tokens.
    - Continue websocket and pairing operations with the existing valid
    token after `429`, `5xx`, or timeout failures.
    - Share an in-memory `next_refresh_at` throttle across websocket and
    pairing callers, honoring both `Retry-After` formats and otherwise using
    a jittered 24–36 second delay.
    - Keep required refreshes strict, preserve `404` enrollment replacement,
    and clear token/throttle state for `401` and `403` auth recovery.
    - Preserve refresh response metadata internally and add focused
    wire-level and integration coverage.
    
    ## Verification
    
    Added behavioral coverage proving that:
    
    - a valid near-expiry token still completes websocket and pairing
    requests after transient refresh failures;
    - `Retry-After` suppresses a subsequent refresh across websocket and
    pairing callers;
    - request and response-body timeouts are classified as transient;
    - an expired token, including one that expires during refresh, cannot
    proceed to websocket connection;
    - auth failures clear the attempted token without overwriting a
    concurrently rotated token.
  • auth: move domain mode below app wire types (#29721)
    ## Why
    
    Authentication mode is a domain concept used by login, model selection,
    telemetry, and transports. Keeping the canonical type in app-server
    protocol forces those lower-level crates to depend on an unrelated wire
    API.
    
    ## What changed
    
    - Added canonical `codex_protocol::auth::AuthMode` domain values.
    - Kept the app-server wire DTO unchanged and added an explicit app-side
    conversion.
    - Removed production app-server-protocol dependencies from login,
    model-provider-info, models-manager, and otel call paths.
    
    ## Stack
    
    This is PR 2 of 6, stacked on [PR
    #29714](https://github.com/openai/codex/pull/29714). Review only the
    delta from `codex/split-json-rpc-protocols`. Next: [PR
    #29722](https://github.com/openai/codex/pull/29722).
    
    ## Validation
    
    - Auth and login coverage passed in the focused protocol/domain test
    run.
    - App-server account and auth conversion coverage passed.
  • Use controlled time for remote initialization timeout test (#29329)
    ## Summary
    
    The remote-control initialization timeout test used a 50 ms wall-clock
    deadline around a 10 ms transport timeout. A busy CI runner could miss
    that outer deadline even when the rollback behavior was correct.
    
    Pause Tokio time and advance it explicitly through the transport timeout
    instead. The test still verifies that initialization fails and emits the
    matching connection-closed event, without depending on scheduler speed.
  • Restore app-server websocket listener with auth guard (#22404)
    ## Why
    PR #21843 removed the TCP websocket app-server listener, but that also
    removed functionality that still needs to exist. Restoring it as-is
    would reopen the old remote exposure problem, so this keeps the restored
    listener while making remote and non-loopback usage require explicit
    auth.
    
    ## What Changed
    - Mostly reverts #21843 and reapplies the small merge-conflict
    resolutions needed on top of current main.
    - Restores ws://IP:PORT parsing, the app-server TCP websocket acceptor,
    websocket auth CLI flags, and the associated tests.
    - The only intentional behavior change from the restored code is that
    non-loopback websocket listeners now fail startup unless --ws-auth
    capability-token or --ws-auth signed-bearer-token is configured.
    Loopback listeners remain available for local and SSH-forwarding
    workflows.
    
    ## Reviewer Focus
    Please focus review on the small auth-enforcement delta layered on top
    of the revert:
    
    - codex-rs/app-server-transport/src/transport/websocket.rs:
    start_websocket_acceptor now rejects unauthenticated non-loopback
    websocket binds before accepting connections.
    - codex-rs/app-server-transport/src/transport/auth.rs: helper logic
    classifies unauthenticated non-loopback listeners.
    - codex-rs/app-server/tests/suite/v2/connection_handling_websocket.rs:
    tests cover unauthenticated ws://0.0.0.0 startup rejection and
    authenticated non-loopback capability-token startup.
    
    Everything else is intended to be revert/merge-conflict restoration
    rather than new product behavior.
    
    ## Verification
    
    - Manually verified that TUI remoting is restored and that auth is
    enforced for non-localhost urls.
  • app-server: remove TCP websocket listener (#21843)
    ## Why
    
    The app-server no longer needs to expose a TCP websocket listener.
    Keeping that transport also kept around a separate listener/auth surface
    that is unnecessary now that local clients can use stdio or the
    Unix-domain control socket, while remote connectivity is handled by
    `remote_control`.
    
    ## What Changed
    
    - Removed `ws://IP:PORT` parsing and the `AppServerTransport::WebSocket`
    startup path.
    - Deleted the app-server websocket listener auth module and removed
    related CLI flags/dependencies.
    - Kept websocket framing only where it is still needed: over the
    Unix-domain control socket and in the outbound `remote_control`
    connection.
    - Updated app-server CLI/help text and `app-server/README.md` to
    document only `stdio://`, `unix://`, `unix://PATH`, and `off` for local
    transports.
    - Converted affected app-server integration coverage from TCP websocket
    listeners to UDS-backed websocket connections, and added a parse test
    that rejects `ws://` listen URLs.
    - Removed the now-unused workspace `constant_time_eq` dependency and
    refreshed `Cargo.lock` after `cargo shear` caught the drift.
    - Moved test app-server UDS socket paths to short Unix temp paths so
    macOS Bazel test sandboxes do not exceed Unix socket path limits.
    
    ## Verification
    
    - Added/updated tests around UDS websocket transport behavior and
    `ws://` listen URL rejection.
    - `cargo shear`
    - `cargo metadata --no-deps --format-version 1`
    - `cargo test -p codex-app-server unix_socket_transport`
    - `cargo test -p codex-app-server unix_socket_disconnect`
    - `just fix -p codex-app-server`
    - `git diff --check`
    
    Local full Rust test execution was blocked before compilation by an
    external fetch failure for the pinned `nornagon/crossterm` git
    dependency. `just bazel-lock-update` and `just bazel-lock-check` were
    retried after the manifest cleanup but remain blocked by external
    BuildBuddy/V8 fetch timeouts.
  • Disable empty Cargo test targets (#21584)
    ## Summary
    
    `cargo test` has entails both running standard Rust tests and doctests.
    It turns out that the doctest discovery is fairly slow, and it's a cost
    you pay even for crates that don't include any doctests.
    
    This PR disables doctests with `doctest = false` for crates that lack
    any doctests.
    
    For the collection of crates below, this speeds up test execution by
    >4x.
    
    E.g., before this PR:
    
    ```
    Benchmark 1: cargo test     -p codex-utils-absolute-path     -p codex-utils-cache     -p codex-utils-cli     -p codex-utils-home-dir     -p codex-utils-output-truncation     -p codex-utils-path     -p codex-utils-string     -p codex-utils-template     -p codex-utils-elapsed     -p codex-utils-json-to-toml
      Time (mean ± σ):      1.849 s ±  4.455 s    [User: 0.752 s, System: 1.367 s]
      Range (min … max):    0.418 s … 14.529 s    10 runs
    ```
    
    And after:
    
    ```
    Benchmark 1: cargo test     -p codex-utils-absolute-path     -p codex-utils-cache     -p codex-utils-cli     -p codex-utils-home-dir     -p codex-utils-output-truncation     -p codex-utils-path     -p codex-utils-string     -p codex-utils-template     -p codex-utils-elapsed     -p codex-utils-json-to-toml
      Time (mean ± σ):     428.6 ms ±   6.9 ms    [User: 187.7 ms, System: 219.7 ms]
      Range (min … max):   418.0 ms … 436.8 ms    10 runs
    ```
    
    For a single crate, with >2x speedup, before:
    
    ```
    Benchmark 1: cargo test -p codex-utils-string
      Time (mean ± σ):     491.1 ms ±   9.0 ms    [User: 229.8 ms, System: 234.9 ms]
      Range (min … max):   480.9 ms … 512.0 ms    10 runs
    ```
    
    And after:
    
    ```
    Benchmark 1: cargo test -p codex-utils-string
      Time (mean ± σ):     213.9 ms ±   4.3 ms    [User: 112.8 ms, System: 84.0 ms]
      Range (min … max):   206.8 ms … 221.0 ms    13 runs
    ```
    
    Co-authored-by: Codex <noreply@openai.com>
  • app-server: move transport into dedicated crate (#20545)
    ## Why
    
    `codex-app-server` currently owns both request-processing code and
    transport implementation details. Splitting the transport layer into its
    own crate makes that boundary explicit, reduces the amount of
    transport-specific dependency surface carried by `codex-app-server`, and
    gives future transport work a narrower place to evolve.
    
    ## What changed
    
    - Added `codex-app-server-transport` and moved the existing transport
    tree into it, including stdio, unix socket, websocket, remote-control
    transport, and websocket auth.
    - Moved shared transport-facing message types into the new crate so both
    the transport implementation and `codex-app-server` use the same
    definitions.
    - Kept processor-facing connection state and outbound routing in
    `codex-app-server`, with the routing tests moved next to that local
    wrapper.
    - Updated workspace metadata, Bazel crate metadata, and
    `codex-app-server` dependencies for the new crate boundary.
    
    ## Validation
    
    - `cargo metadata --locked --no-deps`
    - `git diff --check`
    - Attempted `cargo test -p codex-app-server-transport`, `cargo test -p
    codex-app-server`, `just fix -p codex-app-server-transport`, and `just
    fix -p codex-app-server`; all were blocked before compilation by the
    existing `packageproxy` resolution failure for locked `rustls-webpki =
    0.103.13`.
    - Attempted Bazel build / lockfile validation; those were blocked by
    external fetch failures against BuildBuddy / GitHub while resolving
    `v8`.