Repository navigation
feat: add web previews and virtual displays to workspace tabs - #426
Merged
Merged
Conversation
theogravity
changed the title
theogravity
force-pushed
the
feat/pane-preview
branch
3 times, most recently
from
October 24, 2026 01:28
97fe585 to
6efc19d
Compare
theogravity
changed the title
14 TDD tasks in dependency order: protocol 23 grammar, node switch, port probe, forward lanes, CDP bridge, virtual displays, plane broker, REST authz matrix, /pv proxy, live watch fan-out through the §11.14 control, dashboard surfaces, noVNC screen viewer, e2e and docs. Recorded decisions against the spec: @novnc/novnc npm dep replaces git vendoring (same pinned/MPL ruling, cleaner); forward lanes are per browser connection, not per session (dev servers mix keep-alive HTTP with a permanent HMR socket); close-reason set gains target-gone and a preview-closed node event because the relay precedent has no target-death signal. Route and command names follow the spec exactly (preview_open kinds forward/screencast, /preview/open). Co-Authored-By: Claude <noreply@anthropic.com>
Reviewer findings folded in: pending-ack off-chain dial for forward lanes (ssh-relay-exec A-branch precedent), the real launch command and pane-runtime tmux-runner paths, launch.screen switch gate before spawn, fps clamp [1,5] with optional wire field, snake_case NodeEvent members plus a preview_closed event and target-gone/not-cdp reasons, node 1 MiB slow-link constant, Xvfb -auth pinned env, census tests are hand-pinned, node-results validators, five spec §7 dashboard behaviors, version-floor wording, and the static noVNC import (no dynamic import rule has one named exception). Spec fixes: the nonexistent polling-routes request-log exclusion, the dead docs/node-gates.md pointer. Branch rebuilt on current main: the docs commits had landed on the dropdown branch after #422 merged. Co-Authored-By: Claude <noreply@anthropic.com>
The WS tunnel split is the substantive one: one blind byte pump cannot serve both its consumers (raw RFB needs no handshake, HMR targets need theirs), so sessions carry an http/raw mode the broker derives from the pane's registered screen port - no wire or REST change. Plus the close-reason runtime list and pinned census, a closeLaneForConnection broker member, deps seams on the forward lane, bridge, and broker, the queue constant, raw/ws test cases that cannot silently pass, the docs meta.json registration, and the child-process e2e reality. Co-Authored-By: Claude <noreply@anthropic.com>
Watch is now wired end to end: REST owns bridge lifecycle, the live watch/unwatch frames attach the VIEWING SOCKET, and the SPA sends both halves (the feed hook's existing send() becomes the seam). Lane subscriptions key on laneRef so an iframe and an HMR socket under one session stop trading bytes. preview_close reaches the CDP bridge and link-drop reaches bridgeCloseAll. Plane->node lane bytes get a real sender cloning sendRelayFrameOverNodeSocket - they never rode sendCommand. The bridge drops the invented traffic-idle rule (a static page repaints nothing) and keeps spec §5's zero-subscriber semantics. Plus stopWatch by pane id, closeForPane replacing the uncalled closeSession, deps seams the tests name, per-socket §11.14 precision, the e2e/docs/commit-discipline minors, and the NIT list. Co-Authored-By: Claude <noreply@anthropic.com>
Eight minors and six nits, every one re-verified on disk first: the 401 node-key row, the paneEnvFor env seam (tmux-runner opts.env is the tmux client env, not the pane's), launch leaving the dataless list and set_preview_enabled's unread ack, the revoke cut inside setShares beside the shares-changed announce, no ref in preview audit rows (the relay says out loud its ref is not a credential; ours is), a per-subscription watch counter not a Set, mode discrimination from the live ports record so a plane restart cannot downgrade a display to http, REQUEST_LOG_IGNORE entries for the info poll and the /pv path - which also means my earlier spec erratum was wrong: the exclusion mechanism exists, restored in spec and plan. Task 3 now spells its storage clone out as the 0062 column-pair migration, and the /pv upgrade probe is described as the reachability/gate probe it actually is. Co-Authored-By: Claude <noreply@anthropic.com>
One major: the node event switch in node-ws-handler.ts is where every node event is dispatched, has no default branch, and no task touched it - all three preview event types would have landed dead with every unit test green. Preview arms, named, plus a dispatch test in the file's own handleNodeMessage style. Five minors: Task 1's commit stops staging whole app directories in a tree carrying other workstreams' WIP; the lane-bypass test now counts the fake node's bytes (an in-proc target answers plane loopback, so a bypassed dial used to pass its own guard); sendToSockets' production wiring has an owner (Task 11), the per-viewer byte-debt seam is designed (bufferedAmount as the optional LiveWsSocket member the readyState habit already allows, checked at broker fan-out, PREVIEW_STREAM_DEBT_MAX pinned at 4 MiB), and Task 3's node test states the gate it can actually express. Nits: the routing-ref-not-a-credential quote is at :583, not :608; screen-launch-hint joins the file structure; /pv names server.ts and the before-the-SPA-plugin ordering; PREVIEW_COMMAND_TYPES is package-exported like its SSH twin. Co-Authored-By: Claude <noreply@anthropic.com>
Five minors, all quote-verified: SSH_COMMAND_TYPES lives in ssh-frames.ts, not node-frames.ts, so the census instruction now copies the shape and puts PREVIEW_COMMAND_TYPES with its own grammar; the 0039 curated-list precedent is in apps/server/api/AGENTS.md:824, not the testing rules; the e2e spec lands where Playwright's testDir will find it (e2e/tests/47-preview.spec.ts); the fps comment no longer promises a default the arm must not apply; and the permission surface got planned - the switch route gets its own nodes:preview catalog member (reusing nodes:ssh would let every ssh-scoped key flip preview) and the pane routes carry the read/execute annotations their siblings draw, with the scoped-key refusal pinned as its own matrix row. Nits: the push assertion moves to the service test whose scripted wire can actually see it (the route test's node fixture is offline on purpose); the disable/role routes' missing relay cuts are named as a different axis; bufferedAmount is described as the Task 11 addition it is, not something LiveWsSocket already has. One nit was rejected on the disk: "POLLED by a page somebody leaves open" is verbatim at context.plugin.ts:19. Co-Authored-By: Claude <noreply@anthropic.com>
- scoped-key refusal row: #424's named-permission 403 branch, block re-anchored :201-213 - /pv upgrade gated with browserOriginAllowed (registry origins pass; plane-origin equality rejected published-network dashboards and contradicted spec §6's 'cross-site' wording) - Task 11 updates ws.plugin.ts's stale watch-previews forward-reference - Task 1 staging note: sibling WIP merged; refresh the dirty-tree discipline text
- preview_open range fixture gains the required port (lifetime-bound cases were vacuous: the missing field refused the body before bounds ran) - watch bridge is pane-keyed, matching the spec's portless DELETE, pane-keyed socket map and ref-less stream frame; a second port on a live bridge is a named 409, and the SPA sends its live frame only after a REST join - decision citations fixed: spec §6 (not §5) owns the raw-tunnel quote; the POLLED-by-a-page reason belongs to context.plugin.ts:19, not the test
- MAJOR: Task 7 now wires preview-ports-exec.ts to the display registry (Task 4 hardwired screen:null per its own note and nothing un-hardwired it: decision 6's discriminator and Task 14's real leg read a supply line no task built); + ports-screen and bindProbe-seam tests - PREVIEW_STREAM_DEBT_MAX actually lands in the constants fence (was named, fenced nowhere) - byte-cap seam keys named where the cap tests need them (Task 5 laneBufferMaxBytes, Task 10 preAckMaxBytes) - live sender decided: feed context value gains bound send(frame): boolean (helper at :23 needs a socket the provider closure never shared), plus watch re-fire on reconnect - decision 7 recorded: per-BRIDGE subscriber sum vs PREVIEW_WATCH_SUBSCRIBERS_MAX, and the socket fan-out capped at the same 8 with over-cap audit - NITs: phantom previewFlags() -> getPreviewEnabled(); ::1 added to the CDP host check per the Global Constraint; orphan dropViewer removed; collision-retry seam
- Task 1 runs lint:lockfile:fix and ships the re-recorded bun.lock version line (bun.lock:128 pins 2.3.0; bun install never resyncs; lint.yml:84 gates it, so the bump commit without the fix reddens CI forward) - closeForGrantee is detach-only, matching Task 11's pinned socket-alive outcome; Task 8's fake asserts close is never called - Files-block gaps: Task 7 gains commands/context.ts, Task 8 gains node-ws-handler.test.ts (the dispatch rows) - Task 11 revocation pointer corrected: the matrix has no revoke case today, the precedent is live-ws.ts:284-288 - info on an online non-Linux node pins nodeEnabled true + preview-ports-unsupported reason (Task 14's mac leg reads exactly that)
preview_enabled event wired end to end (Task 1 grammar, Task 3 reconcile/ dispatch/heartbeat, spec bump entry), preview-node-off named in Task 4, wire-level tests for the disable/role/shares cuts, PreviewStreamSocket + PreviewStreamFrame typed once, closeAll lane test, Task 1 barrel in Files. Co-Authored-By: Claude <noreply@anthropic.com>
- Cookie jar gets an owner: rides PreviewSession (dies with it, no second teardown path), pv-http reads/writes it, pv-ws http re-dial carries it - Sender cap named and placed: sendLaneBytes closes over-cap plane-side, forward pump case (i) node-side; over-cap's only close use is spec section 6's sentence that reserved it - Node-reported bridge close now tears the plane half down (session, viewer counts, detach-not-close) so startWatch re-opens instead of joining a phantom bridge - fps grammar reworded to optional-under-screencast (matches the test) - Task 14 manual live leg grounded in spec section 10's own non-gating sentence (reviewer's CI-job premise not in the spec)
- Files entries now have owning steps: Task 9 info-poll REQUEST_LOG_IGNORE (+ pinned-test row), Task 12 settings toggle + create-dialog screen field (tests in Step 1, green in Step 4), Task 13 Screen-button wiring + render test, Task 14 changeset step, Task 10 bun add ws + server.ts registration - Task 2's barrel line: PreviewStreamFrame cannot reach Tasks 8/11 until index.ts names it (named-list block, no subpath export) - over-cap naming unified: refusals carry preview-*-over-cap codes, the bare word names closes (the sender rule, both halves), and node.preview_watch.viewer_drop gives the two unnamed audits (attach cap, byte debt) an action with reason in metadata; Task 14 inventory extended - Commit discipline restated in Global Constraints (every task commits, neighbors see only their own task); Task 1 Step 7 stages file paths, not the protocol directory - Probe-lane close named (viewer-left, wire-only; audit pairs belong to sessions), getPorts gains its declared shape and openForward's catch-and-fall-back for the mac leg - NITs: barrel Files parenthetical no longer a stale review note
Union fold of round 9's decomposition pair (0/2/2) and correctness pair (0/1/3), both pinned to b4ad882, plus the six held round-8 re-review findings (H1-H6). The wiring clause moves onto live-ws.ts and preview-broker.ts is named NOT-an-edit-site; Task 2's barrel export gets its step; the screencast branch pins its pane-gone rung; display.json is written incrementally and boot rehydrate is conditional on bindProbe; deps gains kill; the switch-off screen-launch refusal moves to a failing step; Task 13 feeds viewer-left the way it actually arrives.
Union fold of round 10's decomposition pair (0/4/3) and correctness pair (0/1/0); the badge gap was found independently by both. The spec §9 mid-pane display loss gains a real mechanism: the display module watches its spawned children, reaps on exit, and marks display.lostFor, so the ports answer grows one optional screenLost binary-name field threaded Tasks 1-4-7-9-12 (deliberately NOT folded into the info route's reason word). Also owned at last: Task 7's pane-runtime extraEnv test, the fps/ age badge, the allowedHosts doc sentence, Task 13's novnc install step, Task 3's 0062 migration test, and Task 11's ws.plugin.ts comment fix.
theogravity
changed the title
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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Subshells can open web previews, watch browsers, connect to saved web services, and use virtual displays in independent workspace tabs. Views open beside their terminal and can be moved, split, maximized, or closed independently. The picker uses compact rows and plain port numbers.
Node settings independently control web previews and virtual displays. Enabling web previews makes them available to every subshell on the node. Virtual displays remain a launch option, with provider requirements checked in node settings, the launch dialog, and MCP. The server and node CLIs receive major version bumps; desktop apps receive their normal dependent bumps. No new publishable npm package is introduced.
Virtual displays now use a shared provider contract for prerequisites, allocation, local launch settings, recovery, and cleanup. Nodes can enable multiple providers and choose a default; both settings are persisted and sent to enrolled nodes through signed configuration. New subshells can choose any enabled provider, and presets and MCP carry the same choice. Each subshell pins its actual provider and display size across restarts. Disabling a provider prevents new allocation and restarts before stopping the current subshell. Xvfb + VNC remains the only bundled provider. The VNC viewer is a separate adapter, and provider launch credentials stay on the machine. This supplies the extension points for other backends; it does not add macOS support yet.
Changing the launch node clears an incompatible harness and its preset, resets the directory to that node’s own recent path or home, and turns off a display that the new node cannot provide. Compatible choices and prompts stay. The form explains corrections, waits for confirmed requirements, and ignores late results from a prior node.
The public guide covers setup, controls, limitations, and node permissions. Internal documentation explains provider integration, protocol behavior, and security boundaries. The website includes a recorded workflow and the guide includes screenshots.
Security and dependencies
Preview content is isolated from dashboard browser authority. Forwarding strips dashboard credentials, keeps target cookies server-side, rechecks access around asynchronous opens, and revokes views when permissions or subshell lifecycle change. Transport queues, image dimensions, and pending operations are bounded. Display recovery and cleanup verify ownership before signalling processes. The provider security pass fixed concurrent VNC port selection, permission changes during asynchronous readiness, and overlapping allocation/cleanup. VNC allocation and recovery now verify socket ownership; per-subshell operations are ordered and permissions are rechecked before allocation.
Preview URLs remain bearer capabilities, and the server can read preview traffic. Disabling a node feature prevents new connections while existing connections continue. Loopback VNC has no separate password, so virtual displays are unsuitable for hosts shared with untrusted local OS users. Browser isolation restricts some storage and login flows. These limits are documented in the guide and security model.
Zod dependencies are updated to 4.6.7, including the Expo CLI consumer. A tracked node-forge patch rejects malformed nested DigestAlgorithm fields; regression tests cover invalid signatures and valid Expo consumers. The audit records the remaining upstream version advisory and local mitigation. The server’s direct
wsdependency is retained because its preview transport uses the package’s frame codecs.Validation
wsdependency; frozen installation and workspace build verified after restoring it. The remaining SSH test race now waits for command delivery instead of assuming it completes in 25 ms.The source/security review found no remaining major or minor issues within the reviewed scope. Native macOS providers and every browser/framework combination remain outside this implementation.