Repository navigation
Conversation
|
What: Independent review found no blocking findings at Reviewed head: Read Issue #627 and both investigation/handoff comments in full, repository AGENTS.md, the complete six-file diff, and surrounding consumers under the operations verification and code-review procedures. Independently traced the following:
Scope is appropriate: two existing controller implementation files, focused regressions, the browser guide and the existing feature-map reference. No new component, mode, timeout, wire contract, provider-specific path, version change or unrelated implementation was added. The change supports shared browsing for the current Objective. Proof limits: This was a read-only source review; I ran no tests, launched no browser, made no repository changes and did not touch the live install. I inspected the regression assertions and the author's exact-head proof record: baseline owner/snapshot failures, 81/81 focused tests, server typecheck, scoped lint with one pre-existing regex warning, formatting/diff and fork checks. Those results are author-reported, not independent reruns, and are not a complete repository gate. The broker/viewer regressions use the existing fake Chromium/CDP boundary. The original Messenger renderer/transport disconnect trigger was not observed live, and actual browser/client behavior, browser-host recreation and restart persistence are not live-verified by this review. Elon record:
Reviewer: GPT-6.1 Sol, Codex through T3 Code; independent of the authoring turn. |
Thread transfer impact✅ Thread transfer remains within every enforced ceiling.
Baseline: Scenario and decoded snapshot size10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.
Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed. |
What: Human control of an agent-opened preview tab survives viewer disconnects and passive agent reads.
Why: Viewer teardown called
SessionControl.release, which made the opening agent the controller again without a human release.So what: Review this fix for #627; merge and installation require Lukasz's approval and are not authorized here.
Closes #627.
Keep a disconnected human reservation in
SessionControl, invalidating queued input and draining held keys/buttons without returning authority to agents. A reconnected human can choose Take control, then Release control to resume the agent. Failed replacement authorization retains the human reservation. Allow the opener's snapshots through the existing observation path while a human drives. Existing watch ownership fences remain unchanged.This supports the current Objective by keeping shared browsing safe for Promachos and Drafters. The same server owner governs web, desktop streamed tabs, mobile, and remote viewers; no provider-specific or wire-contract changes are needed. The Browser guide explains reconnect recovery and the map includes #627.
Reproduction and root cause
On base
3ec7a5dd84753d18902a39ca3390f95879b25528, the two new broker/viewer regressions failed before the implementation change:human, receivedagent.PreviewAutomationControlInterruptedError, reasonhumanControl.The real broker, viewer handler and scope finalizer run against the existing fake Chromium/CDP boundary. Viewer disconnect is the reproduced handback mechanism. The precise renderer dependency or transport event that disconnected the original Messenger viewer is unknown; no live login or browser automation was performed. Status, navigation/focus, watch bindings and reconnect source paths were inspected; retained watch reads and watch stop/start attempts now have control-invariance coverage. Full investigation and upstream search: #627 (comment).
Proof
Candidate:
7c6abcdf82a80c0468c4329707cb893dfbcb4b3a.node_modules/.bin/vp test run apps/server/src/preview/SessionControl.test.ts apps/server/src/preview/ServerBrowser.test.ts apps/server/src/preview/ServerBrowserStream.test.ts node_modules/.bin/vp run --filter t3 typecheck node_modules/.bin/vp lint apps/server/src/preview/SessionControl.ts apps/server/src/preview/SessionControl.test.ts apps/server/src/preview/ServerBrowser.ts apps/server/src/preview/ServerBrowser.test.ts node_modules/.bin/vp fmt --check apps/server/src/preview/SessionControl.ts apps/server/src/preview/SessionControl.test.ts apps/server/src/preview/ServerBrowser.ts apps/server/src/preview/ServerBrowser.test.ts docs/map.md docs/user/browser.md git diff --check bash scripts/fork-check.sh --upstream upstream81 tests passed; server typecheck passed; scoped lint passed with one existing
no-control-regexwarning; formatting, diff check and fork-stack checks passed. No repo-wide checks. Environment: Linux, available Node 26.8.2 (repository requests Node 24), pnpm 11.10.0, Vite+ 1.0.0. No version bump per repository policy. No website runtime/SEO impact; this changes the shared server browser controller. Independent exact-head review passed with no blocking findings: #628 (comment). Verifiedreview/independent=success, posted bylukemaj, on the exact candidate SHA.Elon record
Requirements and who asked: Lukasz requested sticky human control, passive read safety, failing-before proof and independent review for #627.
Deleted: Automatic release on viewer disconnect; no new mode, timer, preference, retry loop or browser setup.
Bottleneck: The control owner conflated a disconnected human with agent permission to resume.
Checked myself: Reproduced both baseline failures, traced passive reads and watch paths, then ran 81 focused tests, server typecheck, scoped lint/formatting and fork-stack proof.
Model: GPT-6.1 Sol. Harness: Codex through T3 Code.