Commit Graph

6 Commits

  • [codex] Remove async_trait from first-party code (#27475)
    ## Why
    
    First-party async traits should expose their `Send` contracts explicitly
    without requiring `async_trait`. This completes the migration pattern
    established in #27303 and #27304.
    
    ## What changed
    
    - Replaced the remaining first-party `async_trait` traits with native
    return-position `impl Future + Send` where statically dispatched and
    explicit boxed `Send` futures where object safety is required.
    - Kept implementations behavior-preserving, outlining existing async
    bodies into inherent methods where that keeps the diff reviewable.
    - Removed all direct first-party `async-trait` dependencies and the
    workspace dependency declaration.
    - Added a cargo-deny policy that permits `async-trait` only through the
    remaining transitive wrapper crates.
    - Updated `rand` from 0.8.5 to 0.8.6 to resolve RUSTSEC-2026-0097 and
    keep the full cargo-deny check passing.
    
    ## Validation
    
    - `just test -p codex-exec-server`: 216 passed, 2 skipped.
    - `just test -p codex-model-provider`: 39 passed.
    - `just test -p codex-core` and `just test`: changed tests passed;
    remaining failures are environment-sensitive suites unrelated to this
    migration.
    - `cargo deny check`
    - `just fix`
    - `just fmt`
    - `cargo shear`
    - `just bazel-lock-check`
  • refactor: narrow async lock guard lifetimes (#18211)
    Follow-up to https://github.com/openai/codex/pull/18178, where we called
    out enabling the await-holding lint as a follow-up.
    
    The long-term goal is to enable Clippy coverage for async guards held
    across awaits. This PR is intentionally only the first, low-risk cleanup
    pass: it narrows obvious lock guard lifetimes and leaves
    `codex-rs/Cargo.toml` unchanged so the lint is not enabled until the
    remaining cases are fixed or explicitly justified. It intentionally
    leaves the active-turn/turn-state locking pattern alone because those
    checks and mutations need to stay atomic.
    
    ## Common fixes used here
    
    These are the main patterns reviewers should expect in this PR, and they
    are also the patterns to reach for when fixing future `await_holding_*`
    findings:
    
    - **Scope the guard to the synchronous work.** If the code only needs
    data from a locked value, move the lock into a small block, clone or
    compute the needed values, and do the later `.await` after the block.
    - **Use direct one-line mutations when there is no later await.** Cases
    like `map.lock().await.remove(&id)` are acceptable when the guard is
    only needed for that single mutation and the statement ends before any
    async work.
    - **Drain or clone work out of the lock before notifying or awaiting.**
    For example, the JS REPL drains pending exec senders into a local vector
    and the websocket writer clones buffered envelopes before it serializes
    or sends them.
    - **Use a `Semaphore` only when serialization is intentional across
    async work.** The test serialization guards intentionally span awaited
    setup or execution, so using a semaphore communicates "one at a time"
    without holding a mutex guard.
    - **Remove the mutex when there is only one owner.** The PTY stdin
    writer task owns `stdin` directly; the old `Arc<Mutex<_>>` did not
    protect shared access because nothing else had access to the writer.
    - **Do not split locks that protect an atomic invariant.** This PR
    deliberately leaves active-turn/turn-state paths alone because those
    checks and mutations need to stay atomic. Those cases should be fixed
    separately with a design change or documented with `#[expect]`.
    
    ## What changed
    
    - Narrow scoped async mutex guards in app-server, JS REPL, network
    approval, remote-control websocket, and the RMCP test server.
    - Replace test-only async mutex serialization guards with semaphores
    where the guard intentionally lives across async work.
    - Let the PTY pipe writer task own stdin directly instead of wrapping it
    in an async mutex.
    
    ## Verification
    
    - `just fix -p codex-core -p codex-app-server -p codex-rmcp-client -p
    codex-shell-escalation -p codex-utils-pty -p codex-utils-readiness`
    - `just clippy -p codex-core`
    - `cargo test -p codex-core -p codex-app-server -p codex-rmcp-client -p
    codex-shell-escalation -p codex-utils-pty -p codex-utils-readiness` was
    run; the app-server suite passed, and `codex-core` failed in the local
    sandbox on six otel approval tests plus
    `suite::user_shell_cmd::user_shell_command_does_not_set_network_sandbox_env_var`,
    which appear to depend on local command approval/default rules and
    `CODEX_SANDBOX_NETWORK_DISABLED=1` in this environment.
  • fix: fix readiness subscribe token wrap-around (#8770)
    Fixes ReadinessFlag::subscribe to avoid handing out token 0 or duplicate
    tokens on i32 wrap-around, adds regression tests, and prevents readiness
    gates from getting stuck waiting on an unmarkable or mis-authorized
    token.
  • Use assert_matches (#4756)
    assert_matches is soon to be in std but is experimental for now.
  • chore: extract readiness in a dedicated utils crate (#4140)
    Create an `utils` directory for the small utils crates