fix(sandboxes): make live-process RPCs retry-safe via sandboxd idempotency keys - #867
Open
akirillo wants to merge 6 commits into
Open
fix(sandboxes): make live-process RPCs retry-safe via sandboxd idempotency keys#867akirillo wants to merge 6 commits into
akirillo wants to merge 6 commits into
Conversation
…tency keys Stamp a session_uuid on Start (create-or-attach), select Connect/SendInput/ SendSignal by session UUID, mint a per-write input_uuid, and restore transient retries on the control RPCs. Delete the temporary tag/List reconnect recovery. Requires sandboxd images with idempotent command sessions (platform #4672 / #4710); ship only after image rollout, as a release > 0.2.39. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
akirillo
requested review from
DamianB-BitFlipper,
JannikSt,
JohannesHa,
burnpiro,
kcoopermiller,
kennethnym and
willccbb
as code owners
August 27, 2026 00:32
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
prime/packages/prime-sandboxes/src/prime_sandboxes/sandbox.py
Lines 2828 to 2830 in b9ae099
Avoid retrying signals without a signal idempotency key
When SendSignal applies a SIGTERM but its response is lost with one of these transient errors, this generic retry sends the signal again because the request has only a session UUID, unlike stdin's per-operation input_uuid. A process with a custom SIGTERM handler can remain alive after the first delivery and therefore execute the handler twice or take a different shutdown path; either give signals their own server-deduplicated operation key or exclude ambiguous signal failures from this retry loop.
ℹ️ 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".
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…plumbing Consolidated-review follow-up to the idempotency work: - Both fault predicates live in rpc_command_session.py beside the wire contract: is_recoverable_stream_fault (deny-list; stream re-attach is idempotent) and is_transient_control_fault (allow-list; an unknown unary fault may be a definitive answer), with the polarity asymmetry stated once and the codes cited from command_session.proto's promises. - pyqwest body-read marker strings move to _connectrpc.py as a named set, widened to the sibling strings verified in the installed pyqwest 0.8.0 binary (error reading content / error reading full content / response body read cancelled / read cancelled). - _canonical_uuid_key() mints all three idempotency keys; builders are keyword-only; the one-key-per-op rule is stated once in _execute_process_control_rpc's docstring (the retry loop stays hand-rolled: the one-shot 401 reauth must not consume a transient attempt). Worst-case retry horizon (~91.5s) documented at the constants. - reconnect() picks its arm (Start vs Connect) once per invocation; the Connect arm's replay contract is now truthful server-side (platform #4735 Connect retained-exit fix) and the docstrings state it precisely. - Shared test doubles (fake auth cache, event builders) move to tests/conftest.py for the two live-process test files. Verified: ruff check/format clean; hermetic pytest suites green (remaining failures are pre-existing PRIME_API_KEY integration tests). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…empotency Gated behind PRIME_LIVE_VM_SMOKE=1 (collected as skipped by default); PRIME_VM_IMAGE picks the sandbox image. One module-scoped VM covers four tests proving sandboxd's dedup semantics the hermetic suite cannot reach: retried Start attaches to the same pid, duplicate SendInput/SendSignal applies land once, and a Start replay after exit returns the retained EndEvent without re-running the command. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… API Maintainer review on platform #4735 keeps the CommandSession List RPC as permanent live-process introspection, so restore the plumbing this branch deleted in the session_uuid shape of the current proto: the MethodInfo (NO_SIDE_EFFECTS), the request builder, and typed response Protocols. The tag matcher (find_command_session_pid) stays deleted, and no public list_processes() client method is added. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
DamianB-BitFlipper
approved these changes
Aug 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Chunk 3 of 3 of the sandboxd live-process idempotency work (server side: platform #4672 + #4710 + #4735). The SDK now retries every live-process RPC safely instead of surfacing ambiguous transient link faults, and the temporary tag-based reconnect recovery from #837 is deleted — this fulfills its in-code "remove once sandboxd provides idempotent Start" TODO. Per maintainer review, the
ListSDK plumbing stays (permanent live-process introspection API): the MethodInfo, request builder, and typed response Protocols are restored in the current proto'ssession_uuidshape; only the tag matcher stays deleted, and no publiclist_processes()wrapper is added.open_processstamps a clientsession_uuidon Start; a drop before the first StartEvent re-issues Start (create-or-attach, so a retry attaches instead of respawning), a later drop re-attaches viaConnectselected by session UUID — no pid needed, and an exit missed while detached is replayed from sandboxd's retention window.write_stdinmints oneinput_uuidper logical write, reused across that write's retries; sandboxd acks a duplicate apply without writing twice.terminate/killselect by session UUID (immune to pid reuse; no-op ack after exit), mint onesignal_uuidper logical signal reused across that call's retries (platform #4735 dedups the delivery, so a retried TERM cannot re-run a tenant handler), and retry transient faults.rpc_command_session.py: stream re-attach retries everything exceptNOT_FOUND/FAILED_PRECONDITION(deny-list — re-attach is idempotent); unary control RPCs retry onlyDEADLINE_EXCEEDED,UNAVAILABLE, and pyqwest'sINTERNALbody-read markers (allow-list — marker set widened to the sibling strings verified in the installed pyqwest 0.8.0, named in_connectrpc.py).command_session_pb2.pyregenerated from the platform proto (protoc 31.1 via grpcio-tools 1.74.0; runtime gate unchanged at 6.31.1, satisfied by theprotobuf>=6.31.1pin).test_live_process_idempotency_live.py, gated behindPRIME_LIVE_VM_SMOKE=1, collected as skipped by default) prove the server-side dedup the hermetic suite can only assert the client requests: a retried Start attaches to the same pid, duplicate SendInput/SendSignal applies land once, and a Start replay after exit returns the retained EndEvent.Release constraints a reviewer must know:
Tests: hermetic suite green (225 passed locally; remaining suite failures are pre-existing
PRIME_API_KEYintegration tests).🤖 Generated with Claude Code