Repository navigation
fix(web): the desktop browser host answers every request that arrives together, not only the last - #15965
fix(web): the desktop browser host answers every request that arrives together, not only the last#15965santiago-ramos-02 wants to merge 1 commit into
Conversation
… together, not only the last
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
ApprovabilityVerdict: Approved at Macroscope's review found this PR approvable — This is a focused preview automation bug fix that preserves batched stream events and routes each request through the existing handler, with targeted tests covering multiple requests arriving together. It does not introduce schema, deployment, security, billing, product-default, or static-analysis changes. You can add or adjust custom eligibility rules. Learn more. |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (4)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe automation request stream now retains batches of events. The request consumer processes every event in each batch and derives initial connection state from the batch’s last event. ChangesPreview automation event batches
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~12 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant Stream as Automation request stream
participant Chunks as Stream.chunks
participant Atom as requestsAtom
participant Consumer as Preview automation request consumer
Stream->>Chunks: Emit events
Chunks->>Atom: Preserve event batch
Atom->>Consumer: Provide array of events
Consumer->>Consumer: Process each event
Merge Risk: ⚪ Minimal · up to The change makes the preview automation host answer every request in a batch instead of only the last, which addresses the reported timeouts. No merge-blocking risk was identified. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The fix delivers requests that were previously dropped while preserving existing browser targeting and connection checks. No new privilege or cross-environment access path was identified. Concurrent desktop behavior was assessed from source, not a live multi-thread run. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Closing: #15328 moved the browser host to the environment server, which removed the web request consumer and stream atom this fixed. The server host now handles each broker event with |
1 participant
Problem
When several threads use the browser at once, some of their
preview_*calls are never answered. Each one waits out the broker's 15 s deadline and fails withPreview automation <op> timed out after 15000ms. The broker then disconnects the host, so the other threads' requests in flight fail with… disconnected during waitForand the next calls getNo preview automation host is available.The desktop host reads its request stream through
previewEnvironment.automationRequests, a stream atom. A stream atom keeps only the last element of each batch it pulls (makeStreamsetsArr.lastNonEmpty(arr)). The broker queues requests for a host, so requests from several threads that are queued together reach the client in one batch, and the consumer only ever sees the last of them. The others are dropped without a response.Change
automationRequestsdelivers each batch whole (transform: Stream.chunks), so the atom's value is every event that arrived together, in order.createPreviewAutomationRequestConsumerAtomhandles each event of a batch the way it handled a single event before: the connection checks, stale-generation drops and request handling are unchanged.Scope and approval
A small, focused fix of an obvious bug: the desktop host silently drops automation requests, and each dropped one costs a 15 s timeout and a host disconnect. The fix only changes how the request stream reaches the existing consumer. It does not change the broker's timeout or eviction rules, which open PRs such as #12279, #12899 and #14552 address for specific operations.
Verification
main, a stream atom over[connected, r1, r2, r3]feedingcreatePreviewAutomationRequestConsumerAtomhandles only["r3"].Stream.chunks, asautomationRequestsnow does, and the host answersrequest-1,request-2andrequest-3in order. The existing consumer tests were updated to deliver batches.vp test run apps/web/src/components/preview/previewAutomationRequestConsumer.test.ts apps/web/src/components/preview/PreviewAutomationHosts.test.tsx packages/client-runtime/src/state/preview.test.ts: 19 passed. Typecheck is clean forapps/webandpackages/client-runtime, and lint and format are clean for the changed files.timed out after 15000ms,disconnected during waitForandNo preview automation host is availablewithin the same few minutes.Claude Opus 5.5, Claude Code harness in T3 Code.