Skip to content

Fix remote app-server shutdown race#18936

Merged
bolinfest merged 1 commit intomainfrom
pr18936
Apr 22, 2026
Merged

Fix remote app-server shutdown race#18936
bolinfest merged 1 commit intomainfrom
pr18936

Conversation

@bolinfest
Copy link
Copy Markdown
Collaborator

@bolinfest bolinfest commented Apr 22, 2026

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

@bolinfest bolinfest changed the title app-server-client: tolerate remote shutdown race Fix remote app-server shutdown race Apr 22, 2026
@bolinfest bolinfest enabled auto-merge (squash) April 22, 2026 02:36
@bolinfest bolinfest merged commit 8fea372 into main Apr 22, 2026
25 checks passed
@bolinfest bolinfest deleted the pr18936 branch April 22, 2026 02:41
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 22, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants