You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(server): a timed-out preview call no longer evicts the shared server browser - #36241
In PreviewAutomationBroker.invoke, a foreground request that got no answer within timeoutMs + HOST_RESPONSE_GRACE_MS called disconnect(..., true). That fails every pending request on the connection. For the server browser (SERVER_BROWSER_AUTOMATION_CLIENT_ID), one host is shared by all threads in the environment, so one stalled call (for example a second call queued behind a long waitFor on the same tab) failed other threads' unrelated in-flight calls with PreviewAutomationClientDisconnectedError, and new calls got NoAvailableHost until the host re-registered. The fix skips the timeout eviction when the connection is the server browser. The stalled call still fails with its own PreviewAutomationTimeoutError, and a late response is dropped by respond() because the pending entry is already removed. Eviction for desktop and web hosts is unchanged; it exists to recover a hung remote stream, which cannot happen to the in-process server browser, and the ServerBrowser reconnect loop still handles real disconnects. The change is 5 lines in one file, with no new regex or parser. I reproduced the bug with the repro branch's unit test, which fails on the base, rather than by rerunning the reporter's live two-thread dev-server repro.
Commit 9b6dde9 (origin/main). Ran T3CODE_SERVER_BROWSER_SANDBOX=0 T3CODE_DEV_SEED=0 vp run dev (fresh DB), added a project via node src/bin.ts project add, started two threads A and B with thread start --provider devReplay --model question (turns stay active waiting for an answer), and drove the real /mcp endpoint with each thread's own MCP bearer token from a scratch Node script. Served page A at 127.0.0.1 (static) and page B at localhost (text 'Ready now' appears after 28 s). Steps: A preview_open page A; B preview_open page B; B preview_wait_for {tabId:B, text:'Ready now', timeoutMs:40000}; A preview_wait_for {tabId:A, text:'never appears', timeoutMs:25000}; 1 s later, in parallel, A preview_evaluate {tabId:A, expression:'document.title'} (queued behind A's own wait_for on the same tab). Observed at +18.7 s: A evaluate -> 'Preview automation evaluate timed out after 15000ms.'; at the same instant B's wait_for -> 'Preview automation client server-browser disconnected during waitFor.' (it would have succeeded at 28 s); A's wait_for got the same disconnect; B's preview_status immediately after -> 'No preview automation host is available for status ...'. Control run: when A only froze its page and then snapshotted it, the browser answered with a typed timeout at about 10 s and nothing was evicted, so a single slow call does not trigger it (this is why the earlier #34970 attempt passed on both commits). Unit test on branch factory/repro-16921 (PreviewAutomationBroker.test.ts, 'keeps another thread's request alive when one request times out on the shared server browser') fails on main with PreviewAutomationClientDisconnectedError for the other thread's waitFor.
Evidence
Run two active replay threads through the real MCP endpoint: B waits up to 40s for text appearing at 28s; A waits 25s for absent text, then queues evaluate after 1s.
Scenario source. Environment: Linux, server Chromium, fresh synthetic state, 1280×800 transcript capture. scenarios:prove reported failsOnBase=true and passesOnHead=true.
Not shown:
Native Windows execution was not exercised; the supplied shared-browser reproduction was demonstrated on Linux.
Tests
vp test run src/mcp/PreviewAutomationBroker.test.ts -t 'keeps another thread' without the fix: fails with PreviewAutomationClientDisconnectedError. vp test run src/mcp/PreviewAutomationBroker.test.ts src/preview/ServerBrowser.test.ts with the fix: 110 passed. vp run check:changed: format, lint, knip, typecheck (3 packages), tests (2 files) all passed.
Not in this change
SessionControl.agent in ServerBrowser checks a queued agent operation's deadline only when it starts, so a call waiting behind a busy tab still gets no typed timeout of its own from the browser; it now fails alone through the broker's timeout instead of evicting the host.
The comment in ServerBrowser.ts above the host reconnect loop says the broker disconnects timed-out hosts including slow first installs; that no longer applies to the server browser.
Who did the work
Written by Claude Opus 5.5, in Claude Code
Reviewed by GPT-6.1 Sol, in Codex, and by Claude Opus 5.5, each on its own
Proof captured by GPT-6.1 Sol and checked by Claude Opus 5.5
Macroscope's review found this PR approvable — Timed-out preview requests no longer evict the shared server browser, avoiding disruption to other threads. Timeout errors and pending-request cleanup remain unchanged, and other browser hosts retain their existing eviction behavior.
You can add or adjust custom eligibility rules. Learn more.
No actionable comments were generated in the recent review.
Walkthrough
The broker now retains the in-process server-browser connection when an eligible request times out. A concurrent-request test verifies that another request can succeed after an evaluate request times out.
Changes: Server-browser timeout retention
Layer / File(s)
Summary
Preserve the server-browser connection on timeout apps/server/src/mcp/PreviewAutomationBroker.ts, apps/server/src/mcp/PreviewAutomationBroker.test.ts
The timeout path skips disconnecting and completing the host stream when the connection is the in-process server browser. A test verifies that a concurrent waitFor request succeeds after evaluate times out.
A timed-out request on the shared server-browser connection no longer disconnects other threads' requests. Timeout behavior for other hosts is unchanged. No actionable merge-blocking risk is identified.
Pre-merge checks: 4
Check name
Status
Explanation
Title check
The title states the main change: a timed-out preview call no longer disconnects (evicts) the shared server browser host. It follows the conventional commit form and matches the raw summary's change t…
Linked Issues check
Issue #16921's core requirement is that a slow operation on a healthy shared host must not disrupt unrelated requests. The reviewed diff addresses the mechanism in PreviewAutomationBroker.invoke. On a…
Out of Scope Changes check
The whole-PR diff against the merge base 9b6dde9 changes only the timeout branch in apps/server/src/mcp/PreviewAutomationBroker.ts (+5/-1 in the reviewed diff). It add…
Description check
The description covers the problem, the change, scope, and verification. It explains that a timeout eviction on the shared server browser failed unrelated in-flight calls, and that the fix skips evict…
Full details: Title check
Explanation
The title states the main change: a timed-out preview call no longer disconnects (evicts) the shared server browser host. It follows the conventional commit form and matches the raw summary's change to invoke's timeout path, which skips disconnect for the server browser.
Full details: Linked Issues check
Explanation
Issue #16921's core requirement is that a slow operation on a healthy shared host must not disrupt unrelated requests. The reviewed diff addresses the mechanism in PreviewAutomationBroker.invoke. On a request timeout, the broker called disconnect(..., true) for every connection. The head now skips that eviction when connection.clientId equals SERVER_BROWSER_AUTOMATION_CLIENT_ID. The timed-out request still returns PreviewAutomationTimeoutError. Eviction for other hosts is unchanged. The unit test 'keeps another thread's request alive when one request times out on the shared server browser' is described as failing on the base. The live two-thread reproduction was not rerun, and the PR evidence section is still pending. The issue also asks that mutations not be replayed automatically. The broker code keeps the updateCurrentTab !== false guard, so that behavior is unchanged. The recovery-state reporting clause is not separately addressed by this diff. The issue states that the snapshot stall cause is unknown and that the Cloudflare verification link is unproven. Those items are not treated as requirements here, per the linked issue objectives.
Full details: Out of Scope Changes check
Explanation
The whole-PR diff against the merge base 9b6dde9 changes only the timeout branch in apps/server/src/mcp/PreviewAutomationBroker.ts (+5/-1 in the reviewed diff). It adds a server-browser exclusion to the existing eviction condition and a comment. The raw summary also lists a supporting test in PreviewAutomationBroker.test.ts, which covers the same objective. No unrelated changes were identified. The PR's 'Not in this change' items are documented exclusions and are not code changes.
Full details: Description check
Explanation
The description covers the problem, the change, scope, and verification. It explains that a timeout eviction on the shared server browser failed unrelated in-flight calls, and that the fix skips eviction only for the server browser. It links the issue (#16921) and gives a reproduction on main, a unit test that fails on base, and test and check results. It also names the tools and models used, as the template asks. The template's Verification section asks for focused tests and observed results; the author supplies both. Remaining gaps are minor: the live two-thread repro was not rerun on the fixed commit in the PR text, the before/after evidence links are external and may not persist, and the Windows case is not exercised. These do not make the description largely incomplete. The Scope section links an issue that describes a bug, so it meets the bug-fix requirement.
Finishing Touches
Generate unit tests (beta)
Commit to this branch
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Upstream pingdotgg#36215 decodes recorder frames with Uint8Array.fromBase64. The
fork holds playwright-core at 1.60, whose Chromium lacks it, so every
server browser recording failed. The page now falls back to atob.
Upstream pingdotgg#36241 stops the broker evicting the shared server browser on a
timeout. The fork test that expected a re-register now expects the host
to stay on its first registration.
The reader/writer statement test returns to upstream's form: the fork's
writer worker posts its statements through worker_threads, where the
recorder sees them. The $0 cap expectation gains the fork's fullReadAt.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
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
size:XS0-9 changed lines (additions + deletions).vouch:trustedAuthor is trusted by repo permissions or the VOUCHED list.
1 participant
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.
github-actions
Bot
added
vouch:trusted
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
In PreviewAutomationBroker.invoke, a foreground request that got no answer within timeoutMs + HOST_RESPONSE_GRACE_MS called disconnect(..., true). That fails every pending request on the connection. For the server browser (SERVER_BROWSER_AUTOMATION_CLIENT_ID), one host is shared by all threads in the environment, so one stalled call (for example a second call queued behind a long waitFor on the same tab) failed other threads' unrelated in-flight calls with PreviewAutomationClientDisconnectedError, and new calls got NoAvailableHost until the host re-registered. The fix skips the timeout eviction when the connection is the server browser. The stalled call still fails with its own PreviewAutomationTimeoutError, and a late response is dropped by respond() because the pending entry is already removed. Eviction for desktop and web hosts is unchanged; it exists to recover a hung remote stream, which cannot happen to the in-process server browser, and the ServerBrowser reconnect loop still handles real disconnects. The change is 5 lines in one file, with no new regex or parser. I reproduced the bug with the repro branch's unit test, which fails on the base, rather than by rerunning the reporter's live two-thread dev-server repro.
Fixes #16921
Reproduction on main
Commit 9b6dde9 (origin/main). Ran
T3CODE_SERVER_BROWSER_SANDBOX=0 T3CODE_DEV_SEED=0 vp run dev(fresh DB), added a project vianode src/bin.ts project add, started two threads A and B withthread start --provider devReplay --model question(turns stay active waiting for an answer), and drove the real /mcp endpoint with each thread's own MCP bearer token from a scratch Node script. Served page A at 127.0.0.1 (static) and page B at localhost (text 'Ready now' appears after 28 s). Steps: A preview_open page A; B preview_open page B; B preview_wait_for {tabId:B, text:'Ready now', timeoutMs:40000}; A preview_wait_for {tabId:A, text:'never appears', timeoutMs:25000}; 1 s later, in parallel, A preview_evaluate {tabId:A, expression:'document.title'} (queued behind A's own wait_for on the same tab). Observed at +18.7 s: A evaluate -> 'Preview automation evaluate timed out after 15000ms.'; at the same instant B's wait_for -> 'Preview automation client server-browser disconnected during waitFor.' (it would have succeeded at 28 s); A's wait_for got the same disconnect; B's preview_status immediately after -> 'No preview automation host is available for status ...'. Control run: when A only froze its page and then snapshotted it, the browser answered with a typed timeout at about 10 s and nothing was evicted, so a single slow call does not trigger it (this is why the earlier #34970 attempt passed on both commits). Unit test on branch factory/repro-16921 (PreviewAutomationBroker.test.ts, 'keeps another thread's request alive when one request times out on the shared server browser') fails on main with PreviewAutomationClientDisconnectedError for the other thread's waitFor.Evidence
Run two active replay threads through the real MCP endpoint: B waits up to 40s for text appearing at 28s; A waits 25s for absent text, then queues evaluate after 1s.
Scenario source. Environment: Linux, server Chromium, fresh synthetic state, 1280×800 transcript capture. scenarios:prove reported failsOnBase=true and passesOnHead=true.
Not shown:
Tests
vp test run src/mcp/PreviewAutomationBroker.test.ts -t 'keeps another thread' without the fix: fails with PreviewAutomationClientDisconnectedError. vp test run src/mcp/PreviewAutomationBroker.test.ts src/preview/ServerBrowser.test.ts with the fix: 110 passed. vp run check:changed: format, lint, knip, typecheck (3 packages), tests (2 files) all passed.
Not in this change
Who did the work
Run by the t3code factory in T3 Code.