mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
dev
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.
Anton Panasenko ·
2026-06-26 17:34:52 -07:00 -
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.
Adam Perry @ OpenAI ·
2026-06-24 03:10:20 +00:00 -
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.
jif ·
2026-06-21 14:53:16 +02:00 -
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.
Eric Traut ·
2026-05-12 18:40:53 -07:00 -
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.
Ruslan Nigmatullin ·
2026-05-11 10:17:26 -07:00 -
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>
Charlie Marsh ·
2026-05-07 15:44:17 -07:00 -
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`.
Ruslan Nigmatullin ·
2026-05-01 09:23:47 -07:00