[codex] fix: retry transient shell subscription failures#3528
[codex] fix: retry transient shell subscription failures#3528StiensWout wants to merge 1 commit into
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ApprovabilityVerdict: Approved Single-line configuration change that enables retry behavior for transient shell subscription failures. The fix is minimal, well-tested, and follows existing patterns in the codebase. You can customize Macroscope's approvability policy. Learn more. |
Co-authored-by: Codex <codex@openai.com>
4de3d90 to
60a25cd
Compare
Summary
Root cause
Shell state handled expected subscription failures by surfacing a synchronization error, but it did not request a retry from the shared durable subscription helper. After a transient domain failure, shell sync could stay dormant until a session replacement.
Impact
Transient expected shell subscription failures now resubscribe and can recover to a live snapshot, clearing the shell synchronization error once fresh shell data arrives.
Validation
PATH="$HOME/.vite-plus/bin:$PATH" vp test packages/client-runtime/src/state/shell-sync.test.tsPATH="$HOME/.vite-plus/bin:$PATH" vp checkPATH="$HOME/.vite-plus/bin:$PATH" vp run typecheckNote
Low Risk
Small behavioral change in client shell sync using an existing RPC subscribe retry option; covered by a new unit test.
Overview
Environment shell sync now passes
retryExpectedFailureAfter: "250 millis"onsubscribeShell, aligning with thread detail sync so expected subscription failures still surface the sync error viaonExpectedFailurebut automatically resubscribe instead of staying dormant until the session is replaced.shell-sync.test.tsextracts shared supervisor/cache setup intomakeTestShellStateand adds a regression test: first subscribe attempt fails, state shows the synchronization error, after 250 ms a retry runs, and the next snapshot clears the error and reacheslivestatus.Reviewed by Cursor Bugbot for commit 60a25cd. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Retry transient shell subscription failures after 250ms in
makeEnvironmentShellStateAdds
retryExpectedFailureAfter: "250 millis"to thesubscribeShellcall in shell.ts so that expected subscription failures trigger an automatic retry after a 250ms delay, while still recording the error viaonExpectedFailure. A new test in shell-sync.test.ts verifies the retry behavior and successful transition to live state after the next snapshot.Macroscope summarized 60a25cd.