Commit Graph

11 Commits

  • fix(remote-control): cap reconnect backoff (#24164)
    ## Why
    
    Remote-control websocket reconnects currently use the shared exponential
    backoff helper without a local ceiling, so a long failure streak can
    stretch retries out indefinitely and leave the runtime behavior hard to
    inspect from logs.
    
    ## What Changed
    
    Cap the remote-control reconnect delay at 30 seconds, then reset the
    reconnect attempt counter once that capped delay is emitted so the next
    failure starts from the initial jittered delay again.
    
    The reconnect failure log now records the attempt number, chosen delay,
    and whether the cap triggered a reset, with a separate info log when the
    backoff counter is reset after the cap.
    
    ## Verification
    
    `just test -p codex-app-server-transport`
    
    Related issue: N/A
  • fix(remote-control): retry after auth recovery (#23775)
    ## Why
    
    When remote control hits an auth failure such as a revoked or reused
    refresh token, the websocket loop falls into reconnect backoff. If the
    user fixes auth while that loop is sleeping, remote control can stay
    offline until the old retry timer expires because nothing wakes the loop
    or resets its exhausted auth recovery state.
    
    ## What Changed
    
    Added an auth-change watch on `AuthManager` for refresh-relevant cached
    auth updates.
    
    The remote-control websocket loop now subscribes to that signal, resets
    `UnauthorizedRecovery` and reconnect backoff when auth changes, and
    retries immediately instead of waiting for the previous delay.
    
    Updated the remote-control transport test to verify that reloading auth
    with the now-available account id wakes enrollment before the prior
    retry delay.
    
    ## Verification
    
    `cargo test -p codex-app-server-transport
    remote_control_waits_for_account_id_before_enrolling`
  • fix: serialize unix app-server startup (#23516)
    # Summary
    
    Unix-socket app-server startup can currently race when multiple launch
    attempts target the same `CODEX_HOME`. Those processes can overlap
    before the control socket exists, which lets them enter SQLite state
    initialization concurrently and reproduce the startup corruption pattern
    seen in SSH mode.
    
    This change makes the app-server own that singleton startup guarantee.
    Unix-socket startup now takes a `CODEX_HOME`-scoped advisory lock before
    SQLite initialization, runs the existing control-socket preparation
    check while holding that lock, returns the established `AddrInUse` error
    when another live listener already owns the socket, and releases the
    lock once the new listener has bound its socket.
    
    # Design decisions
    
    - The singleton rule lives in `app-server --listen unix://`, not in a
    desktop-only caller path, so every Unix-socket launch gets the same race
    protection.
    - A duplicate raw app-server launch returns an error instead of silently
    succeeding. The attach operation remains `app-server proxy`, which
    continues to connect to an already-running listener.
    - The lock is held only across the dangerous startup window: socket
    preparation, SQLite initialization, and socket bind. It is not held for
    the app-server lifetime.
    - Listener detection stays in `prepare_control_socket_path(...)`, so the
    preexisting live-listener and stale-socket behavior remains the single
    source of truth.
    
    # Testing
    
    Tests: targeted Unix-socket transport tests on the branch checkout, full
    `codex-cli` build on `efrazer-db10`, and an SSH-style smoke on
    `efrazer-db10` covering concurrent app-server starts, explicit
    duplicate-start errors, and absence of SQLite startup-error matches in
    launch logs.
  • feat(app-server): update remote control APIs for better UX (#22877)
    ## Why
    To help improve `codex remote-control` CLI UX which I plan to do in a
    followup, this PR adds `server-name` to the various remote control APIs:
    - `remoteControl/enable`
    - `remoteControl/disable`
    - `remoteControl/status/changed`
    
    Also, add a `remoteControl/status/read` API. This will be helpful in the
    Codex App.
  • enable/disable remote control at runtime, not via features (#22578)
    ## Why
    reapplies https://github.com/openai/codex/pull/22386 which was
    previously reverted
    
    Also, introduce `remoteControl/enable` and `remoteControl/disable`
    app-server APIs to toggle on/off remote control at runtime for a given
    running app-server instance.
    
    ## What Changed
    
    - Adds experimental v2 RPCs:
      - `remoteControl/enable`
      - `remoteControl/disable`
    - Adds `RemoteControlRequestProcessor` and routes the new RPCs through
    it instead of `ConfigRequestProcessor`.
    - Adds named `RemoteControlHandle::enable`, `disable`, and `status`
    methods.
    - Makes `remoteControl/enable` return an error when sqlite state DB is
    unavailable, while keeping enrollment/websocket failures as async status
    updates.
    - Adds `AppServerRuntimeOptions.remote_control_enabled` and hidden
    `--remote-control` flags for `codex app-server` and `codex-app-server`.
    - Updates managed daemon startup to use `codex app-server
    --remote-control --listen unix://`.
    - Marks `Feature::RemoteControl` as removed and ignores
    `[features].remote_control`.
    - Updates app-server README entries for the new remote-control methods.
  • 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.
  • feat: Use installation ID in remote enrollments (#21662)
    * Pass installation ID for storage on enrollments server for
    deduping/grouping multiple appservers per installation
    * Pass installation ID in remoteControl/status/changed events
  • 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`.