Repository navigation
fix(server): a slow typed address no longer freezes input in a server browser tab - #16691
josephv123 wants to merge 1 commit into
Conversation
… browser tab Viewer navigate, history, and reload no longer wait for the commit inside SessionControl's input queue. A new SessionControl.trackUntilHandoff keeps release waiting for them, so an agent still acts only after the viewer's navigation settles. Fixes pingdotgg#16641
ApprovabilityVerdict: Approved at Macroscope's review found this PR approvable — This is a focused server-browser bug fix that unblocks subsequent viewer input during slow navigations while preserving navigation settlement before control returns to the agent. The affected paths are covered by targeted tests, with no product-default or static-analysis override changes. You can add or adjust custom eligibility rules. Learn more. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (3)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughViewer navigation, history, and soft reload now track navigation promises without awaiting commit. SessionControl keeps this work separate from pending work and waits for handoff work before calling afterDrain. Tests cover follow-up navigation and text input while the initial navigation remains pending. ChangesViewer Navigation
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~15 minutes Change: Bug fix · Severity of issue fixed: Low Suggested reviewers: Merge Risk: ⚪ Minimal · up to Later viewer input can proceed while navigation is pending, and session release still waits for that navigation. No actionable merge-blocking risk was identified. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to Ownership checks and navigation handoff ordering remain intact, and no new authorization bypass was identified. Risk is limited, but interruption and shutdown behavior has not been validated against a real browser in this review. Retained concerns Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Problem
In an environment-hosted (server runtime) browser tab, an address typed in the address bar whose host never answers holds back everything else the viewer does in that tab for up to 15 seconds. Clicks, scrolling, keys, Back, Reload, and a corrected address all wait behind it, then run at once. Chrome would abandon the slow load as soon as you navigate somewhere else.
The viewer's
navigate,history, and softreloadawaited the commit (waitUntil: "commit", 15 s timeout) insideSessionControl's serialized input queue, so a navigation that never commits held the queue for the full timeout.Change
ServerBrowser.dispatchViewerInputstarts viewer navigations (goto,goBack/goForward, softreload) without awaiting them inside the queue. The viewer's next input runs immediately, and a newer navigation replaces the slow one in Chromium, as it does in Chrome.SessionControl.trackUntilHandoffrecords those navigations. Unliketrack, which makes every later action wait (used by the agent'sreadiness: "none"path), it only makesrelease/disconnectwait. That keeps the existing guarantee that an agent acts only after the viewer's navigation settles (covered by the existing "release waits for viewer $method to commit before agent actions" test).ignoreCache) and every other input are unchanged.This is the human-navigation change from the maintainer's suggested fix direction on the issue. It does not add a per-action deadline to
SessionControl, which is the separate problem in #16567.Scope and approval
Fixes #16641, a bug triaged and confirmed on
mainby @juliusmarminge, with the suggested fix direction: #16641 (comment)Verification
ServerBrowser.test.ts, run forgoto,goBack,goForward, andreload: a viewer navigation that never commits, followed by a corrected address and typed text. Onmainall 4 cases time out (the next input is stuck behind the hung navigation). With this change all 4 pass, and the correctedgotoandInput.insertTextboth reach the page.vp test run src/preview/ServerBrowser.test.ts src/preview/SessionControl.test.ts: 36 passed, including the existing release-waits-for-commit tests.chrome-headless-shell154 on Ubuntu 26.04, using the issue's unreachablehttps://10.255.255.1/. Starting a secondgotowhile the first was pending aborted the first withnet::ERR_ABORTED, and the replacement committed in about 10 ms. Back during a pending load landed on the previous page. Playwright rejects thatgoBackwithERR_ABORTED, which the tracked promise absorbs.tsc --noEmitforapps/serverpasses. Lint is clean on the changed files.Not checked: I did not drive the Browser panel end to end in a running app.
Claude Opus 5.5 via Claude Code in T3 Code