From 8fea372c77cb76887c036f5c43cc8a504e75477d Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Tue, 21 Apr 2026 19:41:19 -0700 Subject: [PATCH] Fix remote app-server shutdown race (#18936) ## Why A Mac Bazel CI run saw `remote_notifications_arrive_over_websocket` fail during shutdown with `remote app-server shutdown channel is closed` (https://app.buildbuddy.io/invocation/9dac05d6-ae20-40f9-b627-fca6e91cf127). The remote websocket worker can legitimately finish while `shutdown()` is waiting for the shutdown acknowledgement: after the test server sends a notification and exits, the worker may deliver the required disconnect event, observe that the caller has dropped the event receiver, and exit before it sends the shutdown one-shot. That state is already terminal cleanup, not a failed shutdown, so callers should not see a `BrokenPipe` from the acknowledgement channel. ## What Changed - Treat a closed remote shutdown acknowledgement as an already-exited worker while still propagating websocket close errors when the worker returns them. - Added a deterministic regression test for the interleaving where the shutdown command is received and the worker exits before replying. ## Verification - `cargo test -p codex-app-server-client` - New test: `remote::tests::shutdown_tolerates_worker_exit_after_command_is_queued` --- codex-rs/app-server-client/src/remote.rs | 29 ++++++++++++++++++------ 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/codex-rs/app-server-client/src/remote.rs b/codex-rs/app-server-client/src/remote.rs index 51015f8e7..c8e9a93d2 100644 --- a/codex-rs/app-server-client/src/remote.rs +++ b/codex-rs/app-server-client/src/remote.rs @@ -612,14 +612,9 @@ impl RemoteAppServerClient { .send(RemoteClientCommand::Shutdown { response_tx }) .await .is_ok() - && let Ok(command_result) = timeout(SHUTDOWN_TIMEOUT, response_rx).await + && let Ok(Ok(close_result)) = timeout(SHUTDOWN_TIMEOUT, response_rx).await { - command_result.map_err(|_| { - IoError::new( - ErrorKind::BrokenPipe, - "remote app-server shutdown channel is closed", - ) - })??; + close_result?; } if let Err(_elapsed) = timeout(SHUTDOWN_TIMEOUT, &mut worker_handle).await { @@ -981,4 +976,24 @@ mod tests { skipped: 1 })); } + + #[tokio::test] + async fn shutdown_tolerates_worker_exit_after_command_is_queued() { + let (command_tx, mut command_rx) = mpsc::channel(1); + let (_event_tx, event_rx) = mpsc::channel(1); + let worker_handle = tokio::spawn(async move { + let _ = command_rx.recv().await; + }); + let client = RemoteAppServerClient { + command_tx, + event_rx, + pending_events: VecDeque::new(), + worker_handle, + }; + + client + .shutdown() + .await + .expect("shutdown should complete when worker exits first"); + } }