fix(preview): reconnect automation hosts after stream eviction - #12343
waffleflopper wants to merge 3 commits into
Conversation
ApprovabilityVerdict: Approved at Macroscope's review found this PR approvable — This is a contained preview automation recovery fix: only the explicitly configured automation subscription retries after remote stream eviction, while existing subscriptions and cancellation behavior remain unchanged. The added behavior is covered by focused tests and introduces no schema, deployment, security, billing, or static-analysis changes. You can add or adjust custom eligibility rules. Learn more. |
|
Important Review skippedWe couldn't safely recover the incremental review. No full review was started, and the last reviewed checkpoint was preserved. Retry later, or explicitly request a full review by commenting You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe preview automation subscription now uses a callback with delayed resubscription after stream completion. RPC client tests cover reopening, session preservation, and cancellation during interruption or session replacement. ChangesPreview automation resubscription
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~15 minutes Change: Bug fix · Severity of issue fixed: Medium Suggested reviewers: Merge Risk: ⚪ Minimal · up to The recovery path reconnects preview automation after stream termination while avoiding stale reconnections when the consumer or session changes. No actionable merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Carried from open upstream PR pingdotgg#12343 (head e4ef141), taking only its own diff to the three files it touches so the PR's merges of newer upstream main do not come along. Re-registers the preview automation host 250 ms after the broker ends its stream, so one timed-out request no longer leaves every session without a browser host until the app restarts. Fixes upstream pingdotgg#12146. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Superseded by #12535 (merged), which lands the same host-registration recovery after request/stream timeouts. Closing this open PR as wash. |
What Changed
Re-register preview automation 250 ms after its stream completes or is remotely interrupted, even while the WebSocket stays connected. Cancel pending recovery on unmount or session replacement, and leave transport failure handling with the existing supervisor. Timed-out commands are not replayed.
Why
An unanswered preview request can evict the only desktop automation host, leaving later browser tools unavailable until the app restarts.
Fixes #12146. Complements #12279, which addresses the optional 500 ms metadata timeout in #12273. This PR contains only host recovery.
Validation
The recording shows actual MCP requests/responses in T3’s terminal alongside the preview. It was captured with #12279 applied on the local validation branch; that change is excluded here. Cross-machine relay behavior was not tested.
Recording
recovery-terminal.mp4
Model: GPT-6 (Astra) · Harness: Codex in T3 Code
Summary by CodeRabbit