Repository navigation
feat: preview subshell apps and screens in movable workspace tabs - #426
Open
theogravity wants to merge 94 commits into
Open
theogravity wants to merge 94 commits into
theogravity wants to merge 94 commits into
Conversation
theogravity
force-pushed
the
feat/pane-preview
branch
2 times, most recently
from
October 23, 2026 23:13
548ff56 to
97fe585
Compare
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
force-pushed
the
feat/pane-preview
branch
from
October 24, 2026 01:28
97fe585 to
6efc19d
Compare
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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 preview their dev servers, browser frames, and Linux virtual displays in independent workspace tabs. Open the picker from the terminal or subshell header; each selected resource initially splits to the right of its terminal and appears beneath its subshell in the sidebar. Reopening it focuses the existing tab. Tabs can be moved, split, maximized, and restored without closing the terminal or sibling previews.
The compact viewer toolbar keeps status and refresh beside an action menu. Restore pane and Exit full screen appear directly while active. The picker uses compact rows, sorted ports, search for large lists, and a bounded app list. Virtual display launch checks the selected machine's platform, Preview setting, and Xvfb/x11vnc/xauth prerequisites before enabling the checkbox; missing programs show gold guidance. Preview supports enrolled nodes and the server's built-in node. Phone tabs preserve the desktop layout.
The website now introduces this workflow directly below the mobile video: a real Codex agent fixes and tests a website's category filters, then the developer checks the result beside the terminal. The 31.5-second silent recording, poster, and three guide screenshots come from a disposable sample stack. Public docs cover setup, workspace controls, browser compatibility, security, architecture, and protocol 23. Internal architecture and security docs record implementation boundaries and the audit evidence.
Security review
The targeted source and browser review found and fixed these issues:
Preview URLs remain bearer capabilities. Closing a tab does not immediately revoke its copied URL, and switching Preview off blocks new operations without draining existing connections. The server can read preview traffic. Loopback RFB has no separate password, so virtual displays are unsuitable for hosts shared with untrusted local OS users. Opaque browser isolation limits storage, service workers, and some login flows; the node browser through Virtual display supplies a normal browser context. Custom edge authentication headers outside recognized families require operator filtering. These limits and review scope are documented in
docs/security.md§11.15 anddocs/security-actionable.md.Validation
A fresh review after the fixes found no additional high- or medium-severity issues within the preview threat model. Two existing high dependency advisories remain in Expo tooling (node-forge 1.4.0 and Zod 3.25.76), outside the shipped preview runtime; the audit record includes their upstream links and follow-up constraints. The repository-wide dependency audit is not clean.
This is a targeted audit and regression pass, not an independent penetration test. Native macOS behavior, all browser engines, and every framework-specific HMR/login flow remain outside the tested scope.