refactor: narrow async lock guard lifetimes

[bolinfest]

Follow-up to #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.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9b65a0cd60

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[starr-openai]

most changes were pretty trivial. for the ws outbound buffer it seems safe but perhaps the main thing to double check

[bolinfest]

most changes were pretty trivial. for the ws outbound buffer it seems safe but perhaps the main thing to double check

Pretty sure it should be equivalent in that, in the previous version, I don't believe anyone could call BoundedOutboundBuffer.insert() while state.lock().await.outbound_buffer.server_envelopes() is held because insert() takes &mut self, in which case the iterator couldn't get any "live updates" or anything like that.