Repository navigation
SSH remote runtime: work on a destination over SSH as an ordinary Subshell session (replaces the destination-execution model) - #330
theogravity wants to merge 142 commits into
Conversation
SSH UX review: corrections required before mergeReviewed head The intended feature is: let an agent already running in Subshell work on another machine over SSH, without installing Subshell on that destination. Connections are personal, and a human chooses which agents may use them. Keep those ownership and security rules while correcting the UX. 1. Move SSH out of admin navigation (P1; direct plan deviation)
Sources: navigation, route. 2. Explain the user's task before configuration details (P2; UX completion)The page currently says “Saved destinations agents can reach through an enrolled node,” and its empty state explains account scope. Neither gives a newcomer a concrete task or the next steps. The plan specified these mechanisms more precisely than onboarding copy, so this is clarification of the intended experience, not a claim that exact missing wording was mandated.
Source: page. 3. Offer only usable connecting nodes (P2; frontend/backend mismatch)The editor filters with
Sources: editor, server eligibility. 4. Fix editor state across New/Edit/open cycles (P1; functional defect)
5. Make setup and agent access action-oriented (P2)
Sources: editor, connection card, access dialog. 6. Align docs and verify the journey, not just components (P2)
Add browser coverage that starts from the sidebar as a non-admin node owner, rather than navigating directly to Update this PR's review-state/verification section after these corrections. “Gate C … zero open findings” may describe the prior security review, but it must not imply these UX and functional findings are resolved. Provide new test evidence and a short recorded/manual walkthrough or screenshots of the member navigation, empty state, setup, and agent-access flow. These corrections belong in this PR, not a deferred release-time navigation pass. No change requested to v1's keys-only authentication, private ownership, explicit per-pane grants, no automatic replay, or no remote Subshell-install requirement. |
Architecture correction: match the remote-runtime-over-SSH experienceThis supersedes the old The current PR keeps an agent on an existing node and offers tools for executing commands on an SSH destination without Subshell installed. The desired experience instead runs the Subshell runtime and the selected agent on the destination, with SSH carrying the ongoing runtime connection. Moving the settings page cannot close that gap. Do not merge the existing architecture as fulfilling the corrected request. Required user journey
Support both origins, explicitly confirmed by the user:
There is no separate enrollment of the SSH destination, no setup key or daemon service install, and no requirement for the destination to connect back to the server. The existing connecting node in the web flow is intentional; that is not the destination. Internal node/launcher reuse must not become a user-facing enrollment prerequisite. Reference and boundariesThe official Codex documentation describes local SSH-config discovery, a remotely installed/authenticated Codex CLI, starting/managing its app server through SSH, and choosing a remote project. Match that product experience. It does not establish automatic installation, internal wire details, or our implementation's reliability. Require the remote Subshell binary initially and give useful missing/version-incompatible instructions without sending the user through Rework, do not polish the old product
Design gate before parallel implementationFreeze and prove:
Then parallelize runtime, connection brokers, and control-plane adapter under frozen contracts; follow with UX/MCP and independent verification. One coordinator owns shared wire contracts, registries, migrations, and integration files. Acceptance: independently demonstrate desktop-origin SSH and non-admin web-origin SSH through an owned existing node. On a destination with Subshell installed but no enrollment config and no route to the control plane, select a directory, launch the agent there, attach, and complete an authorized MCP round trip through the session. Also verify missing runtime/harness/authentication, host-key refusal, jump hosts, cross-account isolation, remote-webview boundaries, disconnect/reconnect, stale input, bounded resources, and safe cleanup. Agent model-provider access is a separate requirement from access to the Subshell server.
|
theogravity
changed the title
|
Disposition recorded per the revised SSH-SUPPORT.md (commit d192fa6): this PR implements the superseded destination model and must not be merged as satisfying the clarified remote-runtime-over-SSH experience. Branch kept as the implementation + review record for selective salvage (§5). The revised plan authorizes plan/PR-instruction updates only; implementation rework is not yet requested. |
|
Rework started per the architecture-correction comment: Gate A design frozen at 53de2f4 (docs/superpowers/specs/2026-10-05-ssh-remote-runtime-design.md); the vertical-slice build (runtime-serve over a brokered SSH session, protocol 17, e2e/tests/22-ssh-runtime.spec.ts) is in progress on this branch. Old doors retire with the replacement, not before. |
theogravity
changed the title
Acceptance record (plan §7), supersedes the pre-pivot verification sectionsGate A: design frozen (docs/superpowers/specs/2026-10-05-ssh-remote-runtime-design.md); vertical slice proven on the fixture sshd with no destination enrollment and no destination->plane route. Review loop: slice 3 rounds, detect/UX waves to zero findings (UX 4 rounds, backend 2), retirement clean at round 2, acceptance fixes clean at round 2 - final verdicts all Approved, 0 open Critical/Important/Minor at HEAD 0c6cdb1. Independent verification: cold-stack runs of e2e specs 22+23 green (16/16 at the fix wave, twice), destination-isolation measured green (pane env carries no token; foreign callback forbidden; sentinel base URL dies at DNS; zero TCP listeners/established from runtime pids via ss), and a 20-screenshot walkthrough of the actual journey as a non-admin node owner (sidebar connect -> machine -> alias -> folder -> launch -> typed markers -> close -> kill/reopen re-adopt -> missing-runtime guidance). The walkthrough caught three real gaps, all fixed and pinned: live /ws attach for runtime panes (plus the execProbe socket-derivation blocker), named dir_missing refusal before any lying row, and reopen re-adoption of surviving panes with token rotation on the same row - race-pinned by genuine-await interleaving tests. Known scope edges: destinations paired with old agents refuse runtime attaches until the paired agent ships (17/17 same-release posture, plan's protocol policy); agent-CLI authentication absence on the destination is detected by the CLI's own failure output, not by us (v1); desktop-origin is the same web flow over the client's own enrolled node (design §1), no native code needed. |
|
CI is green (16 pass, 0 fail). The one red that surfaced was a real product race the container exposed and local timing hid: pipe-pane streams only post-attach output, so a pane program that prints in its first microseconds loses its banner to tmux history. Fixed with a shared seedLogFromHistory (pane-runtime) wired into both attach twins - backfills the log from pane history exactly once, only while the log is still empty (a non-empty file IS the stream flowing, so reconstruction is never prepended), never throws. Reviewed, corrected after review caught that the capture mode-preamble defeated the silent-pane guard, and re-verified green. |
|
Final review loop closed: the seed-log fix's re-review came back Clean (0 MAJOR / 0 MINOR / 0 NIT), with all three prior findings independently re-confirmed against real tmux. Every workstream review on this branch is now clean and CI is green (16 pass / 1 skip / 0 fail, including End-to-end). The branch is review-clean and merge-ready. Merging is the operator's call; nothing further until then. |
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Protocol (Apache, Metro-safe barrel): nine ssh_* node wire commands with per-arm parsers delegated from parseNodeCommandBody, approved normalized connection snapshot (eight absent-forbidden members typed literal null), run facts with accepted/running/completed/unknown plus honest 255 and local-vs-remote exit grammar, frozen limit constants and error code set, six result validators, and an additive inputGeneration fence on input and prompt_deliver. NODE_PROTOCOL_VERSION stays 15; the bump is owned by integration. Server (AGPL): five Kysely tables (ssh_connections, ssh_grants, ssh_runs, ssh_panes, ssh_terminal_execs) with migrations 0047/0048 registered at both sites, partial indexes with revocation/quota rationale, the deny-by- default SshPolicy interface (no implementation), and the frozen REST/MCP response shapes. Backup engine updated with the migration registration: LATEST_BACKUP_MIGRATION and the shipped-partial-index allowlist. Shared fixtures for the four parallel workers.
…s split) Restore the commented MCP reference block (it was missing from the first commit); make the unknown lifecycle's ONLY admissible remote observation the ambiguous 255, enforced in readSshRunFacts; drop generation_stale from the error-code set in favor of the single NODE_RESULT_SSH_GENERATION_STALE wire spelling; extend the human policy arm with run/terminal acts and fix the gateGrantedUse bypass implication; split ssh-frames.ts into ssh-frames + ssh-run-facts + ssh-results inside the size ceiling with the barrel re-pointed (names unchanged); bound isAbsPath by SSH_PATH_MAX_CHARS; rewrite the grant history rationale (pane cascade takes revoked rows too, api_key_id is the survivor); True=/1= doc fixes; pin SSH_PROBE_DEADLINE_MS.
…lost The commented MCP tool-shape block (list_ssh_connections, execute_ssh_command, read_ssh_command, cancel_ssh_command, open_ssh_terminal, get_terminal_execution) was present at edit time but missing from the committed file tail a second time; re-appended and verified in the committed blob by grep before and after commit.
…railing export-attached docs)
The block was dropped from the last two commits by biome's assist actions, which the pre-commit lint:staged runs with --write --unsafe and which delete a trailing JSDoc attached to an empty export {}. Reproduced directly; re-landed as detached line comments after the last interface, verified to survive a --write --unsafe pass and present in the committed blob. Six tools with snake_case args and camelCase outputs named after the frozen views: list_ssh_connections, execute_ssh_command, read_ssh_command, cancel_ssh_command, open_ssh_terminal, get_terminal_execution.
… a bypass IMPORTANT-4 second half: the old last sentence said human callers reach uses 'through the owning-human path, not this one', which read as a way around SshPolicy. The five run/terminal actions already live in SshHumanConfigAction; the doc now says human use is gated through gateHumanConfig, the same interface's other arm.
…and refusal plumbing
…/api/ssh route family
… seam pre-proven)
…im drop, control-arm ownership, runs service split, audit doc per family
…cy, durable runs, terminal log Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…inal, sweep) wired into dispatch Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…solve tests Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…classification under stop Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… Minor) The node-frames arm has always promised raw stdout 'in <= 192 KiB pieces'; the broker sealed whatever one stdout read returned, and a bigger read was a bigger event. The seam now splits at the bound (pumpChunks: exact offsets, byte-identical concat - the session codec reassembles frames across pushes, so a cut is free) and throttles per piece. Unit test pins the at-bound pass-through, the split, and the empty read.
…(Minors) The loopback-only self-fetch means a NON-loopback HOST bind answers runtime callbacks with 504 - the note says so and why a bind-host fallback is the wrong repair (it reopens the outbound-hop hole the hardcode prevents). The input-hold doc gains the re-adoption case: holds keyed to a settled runtime node never re-fire, die with the socket, bounded by the client's own queue.
The re-review's one open Minor. launch refuses a relative cwd with the named code dir_relative at HTTP 409 (the F3 dir-shape family), but the route's flat refusal mapper listed dir_missing in the EXISTS_ERROR branch and not dir_relative, so the browser saw a 403 for a typo while a missing dir sees 409. Add dir_relative to the same 409 branch; the sshCode still rides in metadataSafe. Route test pins 409 + sshCode dir_relative. Co-Authored-By: Claude <noreply@anthropic.com>
…exists, race stated (review m1) The comment credited a plane-side refuse-a-second-live-session gate that was deliberately never built (openSession checks only the per-node quota, and a pre-open refusal would break design 6's reopen-and-adopt journey). The bind's live-listener probe is the one enforcement point; the non-atomic probe/unlink/bind window is now stated as the accepted residual.
…issing (review m2) The second serve on a busy destination dies at its callback-door bind BEFORE hello, and the pre-hello classifier's default arm dressed that up as "install the binary" on a machine whose binary is running the first session. The serve now exits with its own code (SSH_RUNTIME_SERVE_IN_USE_EXIT, mapped only by the LiveCallbackDoorError bind refusal - fresh dirs and stale inodes keep their old readings), the broker maps it to the new equality code session_in_use ahead of the 127/255/default arms, mapOpenError names it, the route answers 409 EXISTS like its quota sibling, and the SPA copy says the truth: close the live session, then connect again. The frozen 127 -> runtime_missing path (e2e 22/23's cells) is untouched, pinned by the new classifier test's control arm.
…osed (review m3) After I3 made isNodeOffline read a live runtime session as online, the route's remote branch sailed past the offline gate and composed the non-session write_file RPC - the session frame set does not speak it - answering NODE_UNREACHABLE with restart advice the runtime seal makes impossible. The route now reads the pane's node kind first: a `runtime` row is a session's hidden destination, reached only through its framed channel, so uploads are refused with the new RUNTIME_UPLOAD_UNSUPPORTED 409 and the honest remedy (put the file on the destination yourself). Kind-first, so a lost session's runtime pane gets the same truth instead of "start the node"; local, ordinary-agent and the offline-409 paths are untouched (pinned by the existing relay suite, 10 cells green alongside the two new repro cells).
Plane-side a node-link close is terminal for the sessions riding it (markSessionsLostForNode; design 6: a lost session is never resumable), but nothing agent-side drained the SSH-session supervisor: commandWs.send swallows events with no socket, the emitBytes self-heal cannot fire on an idle session, and the daemon's close path stopped only the tails. Every brokered child therefore outlived the flap - holding a supervisor quota slot and the destination-deterministic callback door that EVERY future open to that host:port:user needs (the child's serve would die at the bind probe and mis-map, the m2 case). The daemon's finish now drains the supervisor through its own close: closed recorded first (the stopping latch; no loss spam into a socket that could not deliver it), group-kill second. The destination's tmux server and panes stay up, which is what a future session's census re-adopts. Daemon test drives the real fake-plane through one flap: the child is gone, the live map drains, no ssh_session_lost rides the reconnect, and a later open to the same destination lands (red: the child sat alive; the pid assertion is real - an earlier draft's quoted-$$ fake recorded a literal and passed vacuously).
…review round-3 MAJOR) Wave 2's drain made the agent kill its brokered children on EVERY link close; the plane's terminality did not follow through the superseded arm. A black-hole flap (the plane never gets FIN for the old socket; the agent's 15 s heartbeat notices, drains, and redials) lands the old close in the held/superseded branch, which deliberately touches no session, so the plane kept active rows and online hidden runtime nodes over runtimes the agent had already killed - ssh_session_send answered SSH_SESSION_UNKNOWN until a reboot or a manual close of each zombie. Fix (design A, identity-tagged): the session registry keeps the node connection each session was brokered on (captured in the same tick as the open RPC, so the tag names the link that carried it), and a `ready` on a NEWER connection marks every session of that node not tagged to it lost - the ready IS the moment the prior link is proven gone, for held outcomes included. Branch 1 keeps marking all sessions directly as the faster FIN path. The tag is compared by IDENTITY, so an open that rode the new socket in the attach-to-ready window (or any later open, or a misbehaving agent's second ready) is never clobbered. Pre-existing registry behavior, close arms, and the credential invariant are untouched. Red proof: the two new node-ws-handler orderings failed against the old code (session stayed active after the SUPERSEDED close and the replacement ready); the branch-1 pin stayed green across both. Tests: branch-1 close, black-hole flap convergence, and the newer-connection no-clobber case in both orders.
…child (review m-A + m-B) m-A: the drain ran bare on the reconnect path - one writeFileSync/chmod throw (ENOSPC/EROFS/EACCES are ordinary) inside `sup.close` escaped `finish`, skipped `resolve(close)`, and the loop never dialed again (alongside the remaining refs), against the file's own stated rule that a broken sweep must not cost the node its connection. Every ref now carries its own guard in `drainBrokeredSessions`, and the call in `finish` wears the outer belt; the close always settles. m-B: a ref enters the supervisor's live map only at hello - up to ~60 s after the drain's snapshot - so an open in flight across the flap reappeared AFTER the drain as a fresh orphan (quota slot + the destination's door, and post-m2 every later open there answers session_in_use naming a session the plane unrolled). `dispatchCommand` now hands the ssh_session_open executor a per-command liveness gate built from the daemon's `currentWs` vs the delivering socket (identity, not an attached-bool that a fast redial would fool); when the open answers on a dead link the child goes through the supervisor's own close and the command answers refused. Tests (daemon.test.ts, shared fake-serve fakes): m-A red = the throw escaped `finish` at daemon.ts:930 and no second `ready` landed; both throws (per-ref and first-line) now redial and the surviving refs are attempted. m-B red = the late-born child sat alive and the live map held it; green = group-killed, map empty, no loss spam onto the new link, and a later open to the same destination lands.
… (review NIT) The m1 rewrite said a pre-open refusal "would misread every REOPEN", which overstates it: an active-row-keyed gate would pass reopens after lost or closed rows just fine. The precise argument is that any gate able to catch a live-serve collision needs destination truth (who is listening behind the door right now) which the plane cannot have across sessions, so it would miss the collision rather than misread a reopen - and the bind at the destination stays the enforcement point. Comment only.
…iew round-4 MINOR1) disconnectNode detaches the live entry synchronously, so the socket's close lands in the superseded arm (no sessions) and the eviction's own act refuses or replaces the redial (no ready sweep). The agent drained its brokered children the moment the socket died; the plane now says so at the detach, where only it can. The held branch and a refused (no-detach) eviction mark nothing, unchanged. Comments name the R3 hold's pre-sweep return (NIT).
…NOR2) The record write is bare sync fs, and the old order let a disk fault (ENOSPC/EROFS/EACCES) throw before killGroup at EVERY close() call site - m-B reclaim, emitBytes self-heal, the link-close drain (m-A's per-ref catch keeps the loop alive, not the child) - orphaning a child that holds a quota slot and the destination's door. The kill leads now; the stopping latch, not the record, keeps the death quiet, and the boot reconcile re-derives closed for a dead pid. Return contract unchanged. Test drives the real supervisor with a store that throws on the closed write: RED showed the child survive 3 s, green shows it group-killed with a lost-on-disk settle and no loss report.
The m-A per-ref catch comment justified itself with a disk fault in the record write that the round-4 kill-first reorder made unreachable (close() now has no reachable throw). The guard stays - the drain runs on the reconnect path, so an unexpected throw must not strand the remaining children or the link - but the comment now says belt-against-a-future failure, not a fault that can no longer occur. Round-5 review NIT1. Co-Authored-By: Claude <noreply@anthropic.com>
5749abf to
4dfdd3a
Compare
|
Implemented the SSH UX corrections in 3ffbd5e. SSH is now a location choice in the shared New subshell form: choose SSH host, connect or reuse a connection, choose a remote folder, then launch an agent or terminal. The same flow works inside workspace creation and Add pane, preserving the workspace and its placement controls. Remote panes show the SSH host name in the sidebar and home grouping. Removed the primary Connect over SSH sidebar section. A secondary SSH connections page remains reachable from the launch form and disconnected-pane recovery, for connection history, reconnecting and disconnecting. Recent hosts and active connections are available directly in the launch form. This makes starting remote work the entry point, rather than asking users to manage SSH sessions first. The connecting-machine step explains whose SSH configuration and credentials are used. Members can use their own enrolled nodes; admins can also use the server account when server launching is enabled. The destination still runs the Subshell runtime without requiring enrollment. Server-origin connections enforce authorization and maintenance restrictions and drain owned SSH children on shutdown. Remote launch also honors the normal prompt stack, with no automatic replay after an uncertain delivery. Updated SSH-SUPPORT.md, internal launch/sidebar documentation, and a release changeset. Validation:
This change covers the shared web launch experience (including the served desktop dashboard); it does not add a new privileged native desktop SSH bridge. |
|
Implemented the SSH UX revision in 14acfc3. The primary workflow is New subshell → SSH host → connecting computer → host → folder → agent/terminal, shared by standalone panes and workspaces. Personal Settings → Connections handles pairing, connection history, and revocation; it is secondary to launching work.
Validation: server 4,057 tests, web 2,491 tests plus the focused terminal-generation regression, node 953 tests, desktop UI 444 tests plus focused reset coverage, docs 169 tests, all five SSH browser tests against an isolated server and real SSH fixture, repository type/lint checks, and Rust formatting/Clippy/tests across all three crates. Native window interaction was not manually smoke-tested; the bundled UI build, native tests, and actual CLI broker transport were tested. To exercise the desktop path, rebuild Subshell Client; refreshing the web UI alone does not add its native commands. Remaining documented limits: destination runtime installation is manual, SSH authentication must be noninteractive, and cross-host conversation/Git handoff is out of scope. This implements the agreed remote-work flow, not a claim of complete Codex feature parity. |
|
Addressed the UX findings from the server-to-Docker SSH walkthrough:
Verified using an Ubuntu 24.04 Docker destination with OpenSSH, tmux, and a non-root account. Created an admin through the isolated server's setup wizard, exercised missing config, unknown host key, and missing runtime, then browsed/saved a remote project and typed commands into its terminal. Footer visibility was verified at 1440×1000 and 390×844. Test container/server were cleaned up. Validation: 2,497 web tests, all five SSH browser tests, and repository lint/type checks. Added permanent browser assertions for contextual setup and desktop/mobile footer visibility, plus unit coverage for trust guidance and shell-safe copied commands. |
…are copy
The SSH connect step leads with the machine-and-destination journey and demotes
shortcuts to a Quick connect group. The "Connect from" machine list becomes a
SearchableSelect like the other launch pickers: the detected local machine folds
in as a "This computer" option, and selectable machines sort above greyed ones
so the scan is gone. The host-step copy names the connecting machine and its
direction ("X SSHes into one of these hosts, listed in its SSH config") instead
of pointing at a config the machine may not own.
Tests and the connect e2e drive the picker instead of the old card buttons.
Co-Authored-By: Claude <noreply@anthropic.com>
Rewrites the UI section around the real job (pick a destination; origin/key source only as a progressive follow-up; the agent upgrade is a secondary act in the pane). Records that reuse from #330 is code/ideas only and its connect UX is a cautionary example, not a base. Co-Authored-By: Claude <noreply@anthropic.com>
…esign) Co-Authored-By: Claude <noreply@anthropic.com>
|
Superseded by the ssh-anywhere design (#333): same destination-over-ssh goal, rebuilt around plain SSH to any sshd host with a default-off per-node gate, and a destination-first UX replacing this line's flow. The discovery/resolve grammar, snapshot policy, and sshd fixture here are the port source for Plan 1 (feat/ssh-anywhere-1), so the branch stays as a reference rather than a merge candidate. |
1 participant
What this is
The corrected experience from SSH-SUPPORT.md (revised 2026-10-05): pick a machine to connect from, pick an SSH host from that machine's config, start the Subshell runtime on the destination THROUGH SSH, pick a remote folder, and launch ordinary panes there (terminal or an installed harness). SSH is the ongoing transport: no destination enrollment, no setup key, no service, no network route from the destination back to the server. The earlier destination-execution product this PR first implemented is retired here, per the plan's §5 disposition, in the same change that lands the replacement.
Architecture (Gate A frozen contract: docs/superpowers/specs/2026-10-05-ssh-remote-runtime-design.md)
subshell runtime-serve: a session-scoped mode of the node binary; it drives tmux on the destination with its own namespaced dataDir/socket and answers a framed protocol over the SSH child's stdio (4-byte length-prefixed JSON, bounded, fail-closed; hello negotiatesruntimeProtocol1; node link protocol bumps to 17).RuntimeSessionLauncher; panes are ordinarysubshellsrows, so list/detail/log/attach/sharing stay normal; disconnect marks unavailable-not-completed, never replays.subshell setup) -> folder browse -> terminal/harness launch; personal sessions history with honest lost/closed states; trusted server-facts identity line on the pane.Retirement (same PR, deliberate)
Old admin SSH page, per-pane grants, coarse ssh token scope, run/terminal REST + six ssh MCP tools, snapshot ceremony, old ssh protocol arms (no extra bump: 16/17 never shipped), e2e spec 21, and migrations 0047/0048 tables via 0050 including a sweep that deletes the old managed panes' zombie rows before the drop. Salvage kept: discovery/diagnostics/render/fixture, bounded subprocess primitives, the input-generation fence seam, presetless terminals, and the fenced launch posture.
Review record
Gate A design + vertical slice reviewed clean over three rounds; the detect/preset + UX waves reviewed clean (UX four rounds, backend two); the retirement reviewed clean at round 2 (Important: zombie pane rows, fixed with a tested sweep). Full repo suite exit 0; e2e specs 22 (runtime, 11 tests) and 23 (sidebar journey as a non-admin owner) green; dep-audit unblocked via in-range bumps + one justified ignore. Remaining acceptance before merge: the §7 walkthrough record on a real browser (§6 verification task).
Co-ordinate: this PR replaces #330's original product shape; do not treat any review of the pre-pivot head as certifying the current code.