From e74530304aeb2c3d64f04858f059f3eba6be898d Mon Sep 17 00:00:00 2001 From: Theo Gravity Date: Wed, 7 Oct 2026 23:12:27 -0700 Subject: [PATCH 01/16] docs(plans): Plan 3 - the destination-first UI (access downgrade + Connect page + view-only proof) --- ...-10-08-ssh-anywhere-3-destination-first.md | 125 ++++++++++++++++++ 1 file changed, 125 insertions(+) create mode 100644 docs/superpowers/plans/2026-10-08-ssh-anywhere-3-destination-first.md diff --git a/docs/superpowers/plans/2026-10-08-ssh-anywhere-3-destination-first.md b/docs/superpowers/plans/2026-10-08-ssh-anywhere-3-destination-first.md new file mode 100644 index 000000000..2334eb995 --- /dev/null +++ b/docs/superpowers/plans/2026-10-08-ssh-anywhere-3-destination-first.md @@ -0,0 +1,125 @@ +# SSH Anywhere - Plan 3: The Destination-First UI Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Ship Milestone 1's human half: a `Connect` page that leads with the destination question ("where do you want to work?"), feeds itself from the chosen machine's aliases plus saved and recent hosts, launches the SSH pane with one button, names the gate remedy in copy, and shows an SSH-terminal pane as view-only to everyone but its owner. + +**Architecture:** The server already owns every rule (tier 2's `/api/ssh` surface, the owner-only input predicate, the WS `canInput` stamp). This tier adds ONE server behavior - the per-viewer `access` in subshell views downgrades to `view` for non-owners of an ssh pane, so every client's existing `access === "view"` posture (input disabled, read-only terminal) becomes truthful for free - and then thin UI: a `use-ssh` hook set following `use-prompts`, a `/connect` route following the launch-form patterns, a SearchableSelect destination field, and the disclosure copy spec §11 demands. + +**Tech Stack:** Bun + TypeScript, Elysia (`t` schemas with descriptions), TanStack Router (file-based) + React Query, Tailwind with the design-system role tokens, `@internal/node-admin`'s `apiFetch`/`apiPost`, Playwright e2e. + +## Global Constraints + +- Work ONLY in `/home/theo/projects/wt-ssh-anywhere-3` (branch `feat/ssh-anywhere-3`). Never touch other checkouts. +- Tests: `env -u SHELLOPTS -u BASHOPTS bun test `; check the reported file count. +- No U+2014/U+2013 anywhere (`bun run lint:prose` scans shipped strings and docs; UI copy too). +- Design system (`docs/design-system.md` enforced by `bun run lint:design`): role tokens only (`text-label font-strong`, `text-detail` via its spellings in that doc), **UI explanations at most two sentences, no em dashes in copy**, required input gold `*` + gold caption when empty, hard errors `text-destructive` red (`fieldErrorToned` from `src/lib/form.ts:88` reads the split), 4px grid, no 12px. +- Terminology is exactly three nouns (spec §11): **destination**, **connecting machine**, **key source**. Key source does not appear in this tier's UI (M2); do not invent it. +- The disclosure obligation (tier 1's engine note, satisfied server-side by the route descriptions in fe125346): the UI copy must say, before the button, in at most two sentences, that resolving runs `ssh -G` on the connecting machine and a hidden `Match exec` can run a local command. +- Route-level: `/api/ssh` response shapes are already typed by `apps/server/api/src/api/ssh/ssh-views.ts` - the SPA's local view types must mirror them field-for-field (the SPA's house style: local `lib/*.ts` types + `apiFetch`, mirroring `lib/prompts.ts`). +- Focused verification while iterating; the full `bun run verify-types && bun run lint:check && bun run lint:prose && bun run test` at Task 6 only. `bunx turbo build` whenever `apps/server/api` schemas changed (treaty/dist consumers) and before e2e (it serves the web dist - memory). +- Commit as theo@suteki.nu; push only at Task 6. + +## File Structure (map) + +``` +apps/server/api/src/api/models.ts # MODIFY: SubshellSchema gains ssh: boolean +apps/server/api/src/services/subshells.service.ts # MODIFY: viewer-relative access downgrade for ssh panes (ONE helper, both view paths) +apps/server/api/src/api/subshells/__tests__/... # MODIFY: view-access tests extended +apps/server/web/src/lib/ssh.ts # CREATE: view types mirroring ssh-views.ts + copy constants +apps/server/web/src/hooks/use-ssh.ts # CREATE: queries/mutations (aliases/saved-hosts/preferences) + useLaunchSsh +apps/server/web/src/components/connect/connect-panel.tsx # CREATE: the destination-first flow +apps/server/web/src/components/connect/saved-hosts-section.tsx # CREATE: saved/recent ledger (delete + save toggle) +apps/server/web/src/routes/connect.tsx # CREATE: createFileRoute("/connect") +apps/server/web/src/components/sidebar/sidebar-nav.ts # MODIFY: top-level Connect entry (unique icon, icons test) +apps/server/web/src/components/subshell-terminal.tsx # MODIFY: SSH badge + view-only sentence for non-owners +apps/server/web/docs/connect-page.md # CREATE: the 3-sentence surface doc +packages/node-admin/src/types/node.ts # MODIFY: sshEnabled? on the shared Node type IF the panel needs it (it does: gate display) +e2e/tests/23-connect-flow.spec.ts # CREATE: the Playwright proof +.changeset/ssh-destination-first-ui.md # CREATE at Task 6 +``` + +## Decisions this plan has already made (do not relitigate) + +1. **Access downgrade, not a new flag:** `SubshellView.access` is the client's single truth (terminal: `readOnly = access === "view"`, subshell-terminal.tsx:567). For an ssh row, a non-owner whose resolved access is `edit` receives `view` in the VIEW layer only; every enforcement seam (REST/exec/WS/nudge) is already server-side from tier 2 and unchanged. `SubshellSchema` gains `ssh: boolean` (derived `row.ssh !== null`) so the terminal can label the pane; the snapshot itself stays never-serialized (its db-types doc). +2. **One page, three steps, §11's order:** destination field first (SearchableSelect fed by saved + recent + the machine's aliases + free text), machine follow-up as a disclosure line ("Connect from …") defaulted to the preference; NO key-source affordance (M2). With no default machine set and more than one SSH-enabled machine, the machine disclosure is required-with-gold-asterisk, never silently chosen (spec §7). With exactly one usable machine, it is pre-selected honestly and still shown. +3. **Gate-off is a named remedy, never a hidden option:** machines whose `sshEnabled` is false stay VISIBLE in the machine disclosure but disabled with a `reason` (the SearchableSelect `ComboboxOption.disabled/reason` fields), and the panel copy names the remedy: "SSH is off on . Ask to enable it." (owner display name from the node row's fields the SPA already reads; for `local`, "You can enable it in Server Settings" is wrong - admins only: use "An admin can enable SSH on this machine in its settings." - pick by `isAdmin` if the client already knows it; otherwise the generic sentence). +4. **Launch is one POST:** `{node, destination}`; the 422 `outcome` refusal renders the blocked settings verbatim in red under the field; SSH_GATE_OFF 403 renders the remedy; 409 held/offline name their own cause. Success navigates to `/subshells/$id` exactly like `launch-subshell-dialog.tsx:49-55`. +5. **Remembering a host is automatic recency + explicit save:** every successful launch already touches the row (tier 2); the panel offers "Remember this destination" (PUT) and the saved section lists saved + recent with delete (DELETE) and the default-machine PATCH ("Use by default" radio-ish control on the machine disclosure). +6. **The disclosure copy (exact, shipped verbatim, under the destination field, `text-detail`):** "Resolving asks the connecting machine to read its SSH config. A hidden Match exec in that config can run a local command while it resolves." - two sentences, no em dashes, matching the route descriptions' substance. + +--- + +### Task 1: Server - `ssh` on the view + the non-owner access downgrade + +**Files:** Modify `apps/server/api/src/api/models.ts` (SubshellSchema beside `access` at :64), `apps/server/api/src/services/subshells.service.ts` (the list view ~:655-675 and the single-row view used by GET detail + launch), new tests in the existing view test home (`grep -l "access" apps/server/api/src/api/subshells/__tests__/` - extend the file that asserts the access stamping; if none, add `subshell-view-ssh.test.ts`). + +**Interfaces:** +- Produces: `SubshellView.ssh: boolean`; and a helper `effectiveViewerAccess(row, access): Access` (module-private is fine, exported for test) that maps `access === "edit" && row.ssh !== null && row.userId !== viewerId` to `"view"` - owner and non-edit stay as resolved. Both view paths (list + get) pass through it. MCP `list_subshells`/detail ride the same views: one change, both doors. +- Consumes: the row's `ssh` column (migration 0048). + +- [ ] Step 1: failing tests - a row with `ssh` JSON + an edit grantee's GET detail + GET list see `access: "view"` and `ssh: true`; the owner sees `access: "owner"`, `ssh: true`; an ordinary pane's edit grantee is UNCHANGED (`access: "edit"`); a view grantee was already `view`. +- [ ] Step 2: run, watch fail (unknown key ssh / access unchanged). +- [ ] Step 3: implement `ssh: t.Boolean({description: "True when this pane is an SSH-terminal pane: only its owner can type into it."})` in SubshellSchema + mapper `ssh: row.ssh !== null` (locate `toSubshellView` at subshell-manager.service.ts:2158 - the schema lives in models.ts; keep schema and mapper edits paired); the access helper applied at the two sites. +- [ ] Step 4: focused tests green; `bunx turbo build` (treaty consumers); `bunx turbo verify-types --filter=@internal/server`; biome; commit `"feat(api): ssh panes read as view-only in subshell views for everyone but their owner"`. + +--- + +### Task 2: SPA plumbing - types, hooks, nav entry + +**Files:** Create `apps/server/web/src/lib/ssh.ts`, `apps/server/web/src/hooks/use-ssh.ts`; modify `apps/server/web/src/components/sidebar/sidebar-nav.ts` (top-level entry after "Subshells": `{ to: "/connect", label: "Connect", icon: Cable }` - pick a lucide icon NOT already used in the nav (the icons test pins global uniqueness; read `components/__tests__/sidebar-nav.test.ts` and the current icon set first)); extend the Node type the SPA reads (grep `sshEnabled` in packages/node-admin/src/types/node.ts - add the boolean the node view already serializes, with the same comment class as its siblings). + +**Interfaces:** +- Produces (`lib/ssh.ts`): `SshSavedHost { id, destination, alias, nodeId, savedAt, lastConnectAt }`, `SshSavedHostsView { saved: SshSavedHost[]; recent: SshSavedHost[]; defaultNodeId: string | null }`, `SshAliasesView { aliases: string[]; includeCycle: boolean; truncated: boolean }`, `SshResolveOutcome` mirroring ssh-views' union, `SshLaunchResponse { subshell: { id: string } }` (only what the panel consumes); the refusal copy strings are NOT here (they are built in the panel from codes). +- Produces (`hooks/use-ssh.ts`, all following `use-prompts.ts` exactly): `useSshSavedHosts()` (key `["ssh-saved-hosts"]`), `useSshAliases(nodeId: string | null)` (key `["ssh-aliases", nodeId]`, `enabled: nodeId !== null`), `useSaveSshHost()`, `useDeleteSshHost()`, `useSshPreferences()` mutation, `useLaunchSsh()` - mutations via `apiPost`/`apiFetch`-POST helper the file uses; launch invalidates `["ssh-saved-hosts"]` (recency moved) only. +- Consumes: `/api/ssh/*` routes (task inventory: GET aliases?node=, GET saved-hosts, PUT saved-hosts, DELETE saved-hosts/:id, PATCH preferences, POST launch). + +- [ ] Step 1: extend the nav test (failing: no Connect entry) + a small type-shape compile test if the lib has a sibling pattern (check `lib/__tests__` style; a plain `tsc`-covered type file needs no runtime test - do not invent vacuous ones). +- [ ] Step 2: implement; nav icon uniqueness green; `env -u SHELLOPTS -u BASHOPTS bun test apps/server/web/src/components/__tests__/sidebar-nav.test.ts`; biome; verify-types @internal/server-web; commit `"feat(web): ssh surface plumbing - types, hooks, and the Connect entry"`. + +--- + +### Task 3: The /connect page + +**Files:** Create `apps/server/web/src/routes/connect.tsx`, `components/connect/connect-panel.tsx`, `components/connect/saved-hosts-section.tsx`; co-located `__tests__` (jsdom component tests - follow the existing `components/__tests__/` style; check what render helper they use). + +**Behavior contract (each a test):** +1. Machine disclosure: options from `useNodes()` filtered to `launchable` (reuse `launchableNodes` from `lib/subshell-compat` exactly as new-subshell-form.tsx does), each option `disabled: !n.sshEnabled` with `reason: "SSH is off on this machine"`; the default from `defaultNodeId`; unset preference + several enabled machines => required (gold `*`, `fieldErrorToned` gold caption when untouched-submit); exactly one => pre-selected. +2. Destination field: `SearchableSelect` (ui/combobox.tsx; props as the launch form uses them) grouped: Saved (`label = alias ?? destination`), Recent, `From 's config` (aliases, fetched only while a machine with the gate ON is picked); free text allowed = the field accepts a typed host not in any list (how: the combobox's input value as candidate - mirror how the working-directory field takes free text, read directory-picker-input.tsx's posture); selecting an item keeps destination semantics client-side ONLY for display, the SERVER resolves (no pre-resolve step; §11's "the common case just opens"). +3. Connect button: `useLaunchSsh` POST; loading state is the button's own disabled+label swap (design-system rule: no spinners elsewhere? check the launch dialog's precedent and match it); on 201 `navigate({ to: "/subshells/$id", params: { id } })`. +4. Refusals: 422 -> red line: `Config needs settings Subshell does not run: ${settings.join(", ")}. Edit them on the connecting machine and retry.`; 403 SSH_GATE_OFF -> the named-remedy sentence (decision 3); 409 -> the held/offline sentence; each maps from the ApiErrorResponse `code` (read the launch route's real response map in apps/server/api/src/api/ssh/launch.route.ts and name every branch). +5. Disclosure (decision 6's exact two sentences) under the destination field, `text-detail`. +6. "Remember this destination" checkbox, unchecked by default (every launch is already recent server-side; remembering is the explicit act). Label: "Remember this destination"; on a successful launch with it checked, PUT saved-hosts `{node, destination, alias?}` where alias is sent only when the pick came from the machine's config list (an alias token), per §7's alias-is-display rule. +7. Saved section: two lists (Remembered, Recent) rendered as label-over-detail lines, each remembered row a delete button; recent rows a save (star?) button; empty states are one short sentence each ("No destinations yet. Pick a machine to see its aliases."). +8. Route + a11y: `createFileRoute("/connect")`, page heading role `heading`, no dialog titles involved; all copy passes the two-sentence + no-em-dash scan (`bun run lint:prose` + the UI strings). + +- [ ] Steps: tests-first per brief (render the panel with mocked hooks - follow existing component-test mocking style), implement, focused green, biome, `bun run lint:design` clean (it's a pre-push check but run it now), verify-types, commit `"feat(web): the destination-first Connect page"`. + +--- + +### Task 4: The terminal's ssh affordances + +**Files:** Modify `apps/server/web/src/components/subshell-terminal.tsx` (+ its test file if one asserts the header). + +**Contract:** when `subshell.ssh === true`: (a) a small "SSH" badge/label in the pane header row (role `detail`, matching the node/badges posture the header already renders - read the header JSX first); (b) for `access === "view"` AND `ssh`: the header detail line reads "SSH sessions take input from their owner only." (one sentence, `text-detail`); non-ssh view grantees keep today's silence (do NOT restate the general view rule here; out of scope creep); (c) owner: nothing extra (the pane works as today). + +- [ ] Steps: failing component assertions, implement, focused green, biome, commit `"feat(web): ssh panes label themselves and tell viewers why typing is off"`. + +--- + +### Task 5: e2e - the Playwright proof + +**File:** Create `e2e/tests/23-connect-flow.spec.ts`. Read first: the SPA-driving specs (grep routes `/new` or nodes pages in e2e/tests for the login/navigate helpers) and reuse spec 22's fixture recipe (`e2e/fixtures/sshd.ts`) with the SAME isolated backend boot so the alias exists. + +**Scenario:** admin cookie session; `PUT /api/nodes/local/ssh-enabled on`; UI: navigate to `/connect`, machine shows `local`, destination picker shows the fixture alias among "From local's config"; pick it, Connect; assert redirect to `/subshells/` and the terminal shows the pane (reuse spec 22's sshd "Starting session" settle before typing `echo CONNECT-OK`); assert the log. Then a second user with an EDIT share (REST) opens the SAME pane URL: the terminal renders read-only, the SSH detail line names the rule, typing does nothing (log unchanged; the REST 403 is tier 2's, this proves the client posture). Then toggle the gate back off via REST, reload `/connect`: `local` is disabled with the reason, the remedy sentence appears when it is selected anyway (design the exact interaction so this is testable). + +- [ ] Steps: write, run ONLY this spec (`bun run test:e2e 23-connect`), iterate; remember `bunx turbo build` first (e2e serves web dist - memory); commit `"test(e2e): connect flow end to end - destination-first launch and the view-only pane"`. + +--- + +### Task 6: Boundary - docs, verification, PR, review loop + +- [ ] `apps/server/web/docs/connect-page.md`: the 3-sentence class doc (what leads, what the machine disclosure is, what refusals say), matching `launch-form-picker.md`'s voice. +- [ ] Full trio + prose + tests; `bunx turbo build` fresh; changeset `@internal/server` minor + `@internal/server-web` minor; push; `gh pr create --base feat/ssh-anywhere-2`; no auto-merge. +- [ ] Whole-branch review (opus) rounds until zero C/I/M; fix waves folded; final head pushed. +- [ ] Copy the review-disposition list into `$(git rev-parse --git-path sdd)/progress.md` as you go. From d92f23b52bd5149ad2ea705ed9f31ed808955525 Mon Sep 17 00:00:00 2001 From: Theo Gravity Date: Wed, 7 Oct 2026 23:16:36 -0700 Subject: [PATCH 02/16] feat(api): ssh panes read as view-only in subshell views for everyone but their owner --- apps/server/api/src/api/models.ts | 3 + .../__tests__/subshell-view-ssh.test.ts | 169 ++++++++++++++++++ .../__tests__/subshell-manager-view.test.ts | 11 ++ .../src/services/subshell-manager.service.ts | 5 + .../api/src/services/subshells.service.ts | 32 +++- e2e/tests/22-ssh-terminal.spec.ts | 11 +- 6 files changed, 225 insertions(+), 6 deletions(-) create mode 100644 apps/server/api/src/api/subshells/__tests__/subshell-view-ssh.test.ts diff --git a/apps/server/api/src/api/models.ts b/apps/server/api/src/api/models.ts index 863f0e6ce..a2b33a908 100644 --- a/apps/server/api/src/api/models.ts +++ b/apps/server/api/src/api/models.ts @@ -66,6 +66,9 @@ export const SubshellSchema = t.Object({ access: t.Union([t.Literal("owner"), t.Literal("edit"), t.Literal("view")], { description: "Caller's effective access to this subshell (viewer-relative; never 'none' on a returned row)", }), + ssh: t.Boolean({ + description: "True when this pane is an SSH-terminal pane: only its owner can type into it.", + }), nodeOffline: t.Boolean({ description: "True when the subshell's node is disconnected. The subshell may still be running there. Always false for subshells on the server host.", diff --git a/apps/server/api/src/api/subshells/__tests__/subshell-view-ssh.test.ts b/apps/server/api/src/api/subshells/__tests__/subshell-view-ssh.test.ts new file mode 100644 index 000000000..64bede2ab --- /dev/null +++ b/apps/server/api/src/api/subshells/__tests__/subshell-view-ssh.test.ts @@ -0,0 +1,169 @@ +import { afterAll, beforeAll, describe, expect, it } from "bun:test"; +import { hashPassword } from "better-auth/crypto"; +import { subshellRoutes } from "@/api/subshells/index.js"; +import { db } from "@/db/index.js"; +import { SubshellSharesRepository } from "@/db/repositories/subshell-shares.repository.js"; +import { SubshellsRepository } from "@/db/repositories/subshells.repository.js"; +import { UsersRepository } from "@/db/repositories/users.repository.js"; +import { effectiveViewerAccess } from "@/services/subshells.service.js"; +import { deleteUserByEmailOrId, setupAuthTables, signIn } from "../../__tests__/helpers/auth-tables.js"; + +/** + * The ssh view truth (spec 2026-10-07 §5.4, plan 3 decision 1): an ssh pane + * reads as `view` + `ssh: true` to EVERY non-owner in both view paths (GET + * detail and GET list, the same two doors the MCP `get_subshell`/ + * `list_subshells` read through), while enforcement itself stays where tier 2 + * put it (`paneInputAllowed` at the input doors). Ordinary panes are + * untouched: an `edit` grantee still reads `edit`. Owner and `view` pass + * through unchanged. The downgrade must never widen anything: it keys on the + * snapshot column's PRESENCE, not its content. + */ +describe("ssh panes in subshell views: the non-owner access downgrade", () => { + const pw = "ssh-view-1"; + const aliceEmail = `sv-alice-${crypto.randomUUID()}@subshell.local`; + const bobEmail = `sv-bob-${crypto.randomUUID()}@subshell.local`; + const carolEmail = `sv-carol-${crypto.randomUUID()}@subshell.local`; + const adminEmail = `sv-admin-${crypto.randomUUID()}@subshell.local`; + let aliceId: string; + let bobId: string; + let carolId: string; + let aliceCookie: string; + let bobCookie: string; + let carolCookie: string; + const created: string[] = []; + + async function mkUser(email: string, role: "user" | "admin"): Promise { + return await new UsersRepository(db).createUser({ + email, + name: email, + passwordHash: await hashPassword(pw), + role, + }); + } + /** Alice owns every fixture row; `ssh` rides the row exactly as launch writes it. */ + async function ownSubshell(id: string, ssh: string | null = null): Promise { + created.push(id); + await new SubshellsRepository(db).create({ + id, + userId: aliceId, + presetId: "p", + harnessId: "ssh", + name: id, + workingDir: "/tmp", + tmuxSocket: null, + ssh, + }); + } + + beforeAll(async () => { + await setupAuthTables(); + aliceId = await mkUser(aliceEmail, "user"); + bobId = await mkUser(bobEmail, "user"); + carolId = await mkUser(carolEmail, "user"); + await mkUser(adminEmail, "admin"); // signed in per test (mirrors the sharing matrix file) + aliceCookie = await signIn(aliceEmail, pw); + bobCookie = await signIn(bobEmail, pw); + carolCookie = await signIn(carolEmail, pw); + + // The snapshot content is irrelevant to the rule (presence is the kind + // fact); this one is valid JSON so the fixture also reads plausibly. + await ownSubshell("s_ssh", JSON.stringify({ destination: "dev.example.com", node: "local" })); + await ownSubshell("s_plain"); // ordinary pane: ssh NULL + const shares = new SubshellSharesRepository(db); + await shares.replaceForSubshell( + "s_ssh", + [ + { granteeUserId: bobId, permission: "edit" }, + { granteeUserId: carolId, permission: "view" }, + ], + aliceId, + ); + await shares.replaceForSubshell("s_plain", [{ granteeUserId: bobId, permission: "edit" }], aliceId); + }); + + afterAll(async () => { + for (const id of created) await db.deleteFrom("subshells").where("id", "=", id).execute(); + for (const email of [aliceEmail, bobEmail, carolEmail, adminEmail]) await deleteUserByEmailOrId(email); + }); + + function req(method: string, path: string, cookie: string) { + return subshellRoutes.fetch( + new Request(`http://localhost:3080/api/subshells${path}`, { + method, + headers: { cookie: `better-auth.session_token=${cookie}` }, + }), + ); + } + async function get(path: string, cookie: string) { + return (await (await req("GET", path, cookie)).json()) as { access: string; ssh: boolean }; + } + + it("an edit grantee GETs an ssh pane as view-only: access 'view', ssh true (detail path)", async () => { + const got = await get("/s_ssh", bobCookie); + expect(got.access).toBe("view"); + expect(got.ssh).toBe(true); + }); + + it("the owner keeps access 'owner' and sees ssh true", async () => { + const got = await get("/s_ssh", aliceCookie); + expect(got.access).toBe("owner"); + expect(got.ssh).toBe(true); + }); + + it("a view grantee is unchanged (already view), and still learns ssh", async () => { + const got = await get("/s_ssh", carolCookie); + expect(got.access).toBe("view"); + expect(got.ssh).toBe(true); + }); + + it("an ordinary pane is untouched: the edit grantee still reads access 'edit', ssh false", async () => { + const got = await get("/s_plain", bobCookie); + expect(got.access).toBe("edit"); + expect(got.ssh).toBe(false); + }); + + it("the admin's effective edit on a foreign ssh pane reads as view too (the boost is not ownership)", async () => { + const cookie = await signIn(adminEmail, pw); + try { + const sshRow = await get("/s_ssh", cookie); + expect(sshRow.access).toBe("view"); + expect(sshRow.ssh).toBe(true); + // Same admin, ordinary pane: still the resolved `edit` (pinning that the + // downgrade is keyed to the ssh column, not to the viewer's role). + const plain = await get("/s_plain", cookie); + expect(plain.access).toBe("edit"); + expect(plain.ssh).toBe(false); + } finally { + await deleteUserByEmailOrId(adminEmail); + } + }); + + it("the LIST path downgrades the same way (one helper, both doors; the MCP list rides this route)", async () => { + const rows = (await (await req("GET", "", bobCookie)).json()) as { + id: string; + access: string; + ssh: boolean; + }[]; + const byId = new Map(rows.map((r) => [r.id, r])); + expect(byId.get("s_ssh")).toMatchObject({ access: "view", ssh: true }); + expect(byId.get("s_plain")).toMatchObject({ access: "edit", ssh: false }); + }); +}); + +describe("effectiveViewerAccess (the downgrade helper, exported for test)", () => { + const sshRow = { ssh: "x", userId: "owner-1" }; // presence keys the rule: content never parsed + const plainRow = { ssh: null, userId: "owner-1" }; + + it("downgrades edit to view on an ssh row", () => { + expect(effectiveViewerAccess(sshRow, "edit")).toBe("view"); + }); + it("leaves owner and view as resolved on an ssh row", () => { + expect(effectiveViewerAccess(sshRow, "owner")).toBe("owner"); + expect(effectiveViewerAccess(sshRow, "view")).toBe("view"); + }); + it("leaves every level as resolved on an ordinary row", () => { + expect(effectiveViewerAccess(plainRow, "edit")).toBe("edit"); + expect(effectiveViewerAccess(plainRow, "owner")).toBe("owner"); + expect(effectiveViewerAccess(plainRow, "view")).toBe("view"); + }); +}); diff --git a/apps/server/api/src/services/__tests__/subshell-manager-view.test.ts b/apps/server/api/src/services/__tests__/subshell-manager-view.test.ts index 13c7f0b01..f173845b0 100644 --- a/apps/server/api/src/services/__tests__/subshell-manager-view.test.ts +++ b/apps/server/api/src/services/__tests__/subshell-manager-view.test.ts @@ -31,6 +31,7 @@ function row(overrides: Partial[0]> = {}) { waitingSince: null, lastPushUrgency: null, crossAgent: 0, + ssh: null, ...overrides, } satisfies Parameters[0]; } @@ -51,6 +52,16 @@ describe("toSubshellView notification fields", () => { expect(toSubshellView(row({ crossAgent: 0 }), "running").crossAgent).toBe(false); }); + it("maps the ssh snapshot column to a presence boolean and never echoes the snapshot (spec 2026-10-07)", () => { + // The rule keys on PRESENCE, not parseability: an unparseable string is + // still an ssh pane (validation belongs to the launch boundary). + expect(toSubshellView(row({ ssh: "x" }), "running").ssh).toBe(true); + const mapped = toSubshellView(row({ ssh: '{"destination":"dev.example.com"}' }), "running"); + expect(mapped.ssh).toBe(true); + expect(JSON.stringify(mapped)).not.toContain("dev.example.com"); + expect(toSubshellView(row(), "running").ssh).toBe(false); + }); + it("defaults access to 'owner' and honours an explicit override (sharing, spec 2026-08-31)", () => { // A returned view is always visible to someone, so it never carries "none". expect(toSubshellView(row(), "running").access).toBe("owner"); diff --git a/apps/server/api/src/services/subshell-manager.service.ts b/apps/server/api/src/services/subshell-manager.service.ts index daea235e1..1c6b7fa51 100644 --- a/apps/server/api/src/services/subshell-manager.service.ts +++ b/apps/server/api/src/services/subshell-manager.service.ts @@ -2204,6 +2204,8 @@ export function toSubshellView( waitingSince: string | null; lastPushUrgency: number | null; crossAgent: number; + /** The ssh snapshot column (migration 0048); read for presence only, never parsed here. */ + ssh: string | null; }, status: string, /** The subshell's current screen, bottom-first-trimmed; empty when not running. */ @@ -2262,6 +2264,9 @@ export function toSubshellView( // (spec 2026-09-23) — what turns the rail's dot into a bell. unseenPush: row.lastPushUrgency !== null, access, + // The pane's KIND fact: the ssh snapshot column's PRESENCE (migration + // 0048). The snapshot itself stays never-serialized (its db-types doc). + ssh: row.ssh !== null, // Agent node unreachable right now (see the param doc) — the UI's // "node offline" chip; false for every local subshell. nodeOffline, diff --git a/apps/server/api/src/services/subshells.service.ts b/apps/server/api/src/services/subshells.service.ts index a3af12539..b345ccae8 100644 --- a/apps/server/api/src/services/subshells.service.ts +++ b/apps/server/api/src/services/subshells.service.ts @@ -56,6 +56,30 @@ type SubshellView = NonNullable, + access: Exclude, +): Exclude { + if (access === "edit" && row.ssh !== null) return "view"; + return access; +} + /** A sharing grant as returned to the client, with the grantee label resolved. */ interface SubshellShareView { /** Share row id */ @@ -671,7 +695,7 @@ export class SubshellsService extends BaseService { const accessBy = new Map>(); for (const row of rows) { const access = resolveSubshellAccess(viewerId, row.userId, sharesBy.get(row.id) ?? []); - accessBy.set(row.id, access === "none" ? "view" : access); + accessBy.set(row.id, effectiveViewerAccess(row, access === "none" ? "view" : access)); } const views = await this.#manager.toViews(rows, opts); return views.map((view) => ({ @@ -854,11 +878,13 @@ export class SubshellsService extends BaseService { if (!subshell) throw new SubshellError("not_found", "Subshell not found"); // One extra read on a single-row path: the gate resolves access without // handing back the grants it looked at, and the disclosure warning needs - // the audience, not just the caller's own level. + // the audience, not just the caller's own level. The gate's resolver hands + // back the RAW level; the ssh view downgrade (plan 3 decision 1) rides the + // same helper the list applies, so both doors answer alike. const shares = (await this.repos.subshellShares.listForSubshells([id])).get(id) ?? []; return { ...subshell, - access: access === "none" ? "view" : access, + access: effectiveViewerAccess(row, access === "none" ? "view" : access), sharedWithViewer: access !== "owner" && shares.some((share) => share.granteeUserId === null || share.granteeUserId === viewerId), ...shareExposure(shares), diff --git a/e2e/tests/22-ssh-terminal.spec.ts b/e2e/tests/22-ssh-terminal.spec.ts index f4c5a7349..065503f06 100644 --- a/e2e/tests/22-ssh-terminal.spec.ts +++ b/e2e/tests/22-ssh-terminal.spec.ts @@ -378,11 +378,16 @@ test("ssh pane: gated launch to the fixture sshd, owner input echoes from the re data: { email: memberEmail, password: MEMBER_PASSWORD }, }); expect(memberSignIn.ok(), await memberSignIn.text()).toBe(true); - // The grant is real: the member sees the pane at `edit` (the refusal below - // must be the SSH rule, not an invisibility or a missing grant). + // The grant is real: the member can read the pane (a stranger gets a 404, + // never the 403 below). And the VIEW says what the doors say: on an ssh + // pane every non-owner reads `view` + `ssh: true` (spec 2026-10-07 §5.4, + // plan 3 decision 1); enforcement itself is the refusal below, keyed to + // the SSH rule, not to invisibility or a missing grant. const memberView = await member.get(`/api/subshells/${subshellId}`); expect(memberView.ok(), await memberView.text()).toBe(true); - expect(((await memberView.json()) as { access: string }).access).toBe("edit"); + const memberSubshell = (await memberView.json()) as { access: string; ssh: boolean }; + expect(memberSubshell.access).toBe("view"); + expect(memberSubshell.ssh).toBe(true); const refused = await member.post(`/api/subshells/${subshellId}/input`, { data: { text: `echo NOT-YOUR-PANE-${nonce}` }, From 80ef59fc679d28481f4b27c6034ea879a7e932d1 Mon Sep 17 00:00:00 2001 From: Theo Gravity Date: Wed, 7 Oct 2026 23:24:07 -0700 Subject: [PATCH 03/16] feat(web): ssh surface plumbing - types, hooks, and the Connect entry --- .../components/__tests__/app-sidebar.test.ts | 8 +- .../__tests__/new-subshell-form.test.tsx | 1 + .../components/__tests__/node-row.test.tsx | 1 + .../components/__tests__/sidebar-nav.test.ts | 3 +- .../__tests__/subshell-card.test.tsx | 1 + .../__tests__/node-harness-card.test.tsx | 1 + .../nodes/__tests__/node-list-row.test.tsx | 1 + .../nodes/__tests__/node-table.test.tsx | 1 + .../nodes/__tests__/node-update-card.test.tsx | 1 + .../web/src/components/sidebar/sidebar-nav.ts | 5 + .../__tests__/new-subshell-form.test.tsx | 1 + .../__tests__/no-launch-targets.test.tsx | 1 + .../__tests__/use-node-harnesses.test.tsx | 1 + .../src/hooks/__tests__/use-nodes.test.tsx | 1 + .../web/src/hooks/__tests__/use-ssh.test.tsx | 128 ++++++++++++++ apps/server/web/src/hooks/use-ssh.ts | 108 ++++++++++++ apps/server/web/src/lib/__tests__/api.test.ts | 7 + .../src/lib/__tests__/subshell-compat.test.ts | 1 + apps/server/web/src/lib/ssh.ts | 157 ++++++++++++++++++ .../routes/__tests__/nodes-detail.test.tsx | 1 + .../routes/__tests__/nodes-page-gate.test.tsx | 1 + .../src/routes/__tests__/nodes-tabs.test.tsx | 1 + .../src/__tests__/node-allowed-dirs.test.tsx | 1 + .../__tests__/node-maintenance-card.test.tsx | 1 + .../src/__tests__/node-runtime-card.test.tsx | 1 + .../src/__tests__/node-service-card.test.tsx | 1 + packages/node-admin/src/lib/api.ts | 4 + packages/node-admin/src/types/node.ts | 10 ++ 28 files changed, 447 insertions(+), 2 deletions(-) create mode 100644 apps/server/web/src/hooks/__tests__/use-ssh.test.tsx create mode 100644 apps/server/web/src/hooks/use-ssh.ts create mode 100644 apps/server/web/src/lib/ssh.ts diff --git a/apps/server/web/src/components/__tests__/app-sidebar.test.ts b/apps/server/web/src/components/__tests__/app-sidebar.test.ts index 8fd469941..b1fbef7fe 100644 --- a/apps/server/web/src/components/__tests__/app-sidebar.test.ts +++ b/apps/server/web/src/components/__tests__/app-sidebar.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "bun:test"; -import { ServerCog, Settings, SlidersHorizontal } from "lucide-react"; +import { Cable, ServerCog, Settings, SlidersHorizontal } from "lucide-react"; import { isNavGroup, visibleNavEntries, visibleNavItems } from "@/components/sidebar/sidebar-nav"; describe("sidebar nav icons (spec 2026-09-02 §3, 2026-09-11 §3.1)", () => { @@ -20,6 +20,12 @@ describe("sidebar nav icons (spec 2026-09-02 §3, 2026-09-11 §3.1)", () => { expect(entries.filter(isNavGroup).find((g) => g.id === "server-settings")?.icon).toBe(ServerCog); }); + it("Connect takes Cable, beside Subshells as a primary action", () => { + // The Connect page is a top-level action (spec 2026-10-07 §7), not a + // settings page; Cable was free of the rail when it landed. + expect(items.find((i) => i.to === "/connect")?.icon).toBe(Cable); + }); + it("no two visible icons are the same — group headers included", () => { // Icons are how the collapsed rail names a page, so a duplicate there is // two rows that read as one. The group's own icon counts: it sits in the diff --git a/apps/server/web/src/components/__tests__/new-subshell-form.test.tsx b/apps/server/web/src/components/__tests__/new-subshell-form.test.tsx index 9e1fc8c93..5bb7f16a7 100644 --- a/apps/server/web/src/components/__tests__/new-subshell-form.test.tsx +++ b/apps/server/web/src/components/__tests__/new-subshell-form.test.tsx @@ -55,6 +55,7 @@ function node(overrides: Partial): Node { maintenance: false, maintenanceAt: null, maintenanceSource: null, + sshEnabled: false, held: null, ...overrides, }; diff --git a/apps/server/web/src/components/__tests__/node-row.test.tsx b/apps/server/web/src/components/__tests__/node-row.test.tsx index 7b2d51b51..f30afc40d 100644 --- a/apps/server/web/src/components/__tests__/node-row.test.tsx +++ b/apps/server/web/src/components/__tests__/node-row.test.tsx @@ -29,6 +29,7 @@ const BASE: Node = { maintenance: false, maintenanceAt: null, maintenanceSource: null, + sshEnabled: false, held: null, }; diff --git a/apps/server/web/src/components/__tests__/sidebar-nav.test.ts b/apps/server/web/src/components/__tests__/sidebar-nav.test.ts index 2138424ed..37a79cec2 100644 --- a/apps/server/web/src/components/__tests__/sidebar-nav.test.ts +++ b/apps/server/web/src/components/__tests__/sidebar-nav.test.ts @@ -36,13 +36,14 @@ describe("visibleNavItems", () => { for (const flag of [false, undefined]) { const paths = visibleNavItems(flag).map((i) => i.to); for (const page of GROUP_PAGES) expect(paths).not.toContain(page); - expect(paths).toEqual(["/", "/workspaces", "/nodes", "/presets", "/prompts"]); + expect(paths).toEqual(["/", "/connect", "/workspaces", "/nodes", "/presets", "/prompts"]); } }); it("gives an admin every group page, flattened in rail order", () => { expect(visibleNavItems(true).map((i) => i.to)).toEqual([ "/", + "/connect", "/workspaces", "/nodes", "/presets", diff --git a/apps/server/web/src/components/__tests__/subshell-card.test.tsx b/apps/server/web/src/components/__tests__/subshell-card.test.tsx index fb8abf740..65e223466 100644 --- a/apps/server/web/src/components/__tests__/subshell-card.test.tsx +++ b/apps/server/web/src/components/__tests__/subshell-card.test.tsx @@ -46,6 +46,7 @@ function agent(overrides: Partial = {}): Node { maintenance: false, maintenanceAt: null, maintenanceSource: null, + sshEnabled: false, held: null, ...overrides, }; diff --git a/apps/server/web/src/components/nodes/__tests__/node-harness-card.test.tsx b/apps/server/web/src/components/nodes/__tests__/node-harness-card.test.tsx index 33ca7c658..fd28b3c1b 100644 --- a/apps/server/web/src/components/nodes/__tests__/node-harness-card.test.tsx +++ b/apps/server/web/src/components/nodes/__tests__/node-harness-card.test.tsx @@ -104,6 +104,7 @@ function view(over: Partial): NodeDetail { maintenance: false, maintenanceAt: null, maintenanceSource: null, + sshEnabled: false, held: null, ...over, }; diff --git a/apps/server/web/src/components/nodes/__tests__/node-list-row.test.tsx b/apps/server/web/src/components/nodes/__tests__/node-list-row.test.tsx index 7799cc453..465af89b7 100644 --- a/apps/server/web/src/components/nodes/__tests__/node-list-row.test.tsx +++ b/apps/server/web/src/components/nodes/__tests__/node-list-row.test.tsx @@ -35,6 +35,7 @@ function node(overrides: Partial = {}): Node { maintenance: false, maintenanceAt: null, maintenanceSource: null, + sshEnabled: false, held: null, ...overrides, }; diff --git a/apps/server/web/src/components/nodes/__tests__/node-table.test.tsx b/apps/server/web/src/components/nodes/__tests__/node-table.test.tsx index f7796c21f..0b80031b8 100644 --- a/apps/server/web/src/components/nodes/__tests__/node-table.test.tsx +++ b/apps/server/web/src/components/nodes/__tests__/node-table.test.tsx @@ -28,6 +28,7 @@ function node(overrides: Partial = {}): Node { maintenance: false, maintenanceAt: null, maintenanceSource: null, + sshEnabled: false, held: null, ...overrides, }; diff --git a/apps/server/web/src/components/nodes/__tests__/node-update-card.test.tsx b/apps/server/web/src/components/nodes/__tests__/node-update-card.test.tsx index f44e3e7ba..57aa2193c 100644 --- a/apps/server/web/src/components/nodes/__tests__/node-update-card.test.tsx +++ b/apps/server/web/src/components/nodes/__tests__/node-update-card.test.tsx @@ -35,6 +35,7 @@ function node(over: Partial = {}): NodeDetail { maintenance: false, maintenanceAt: null, maintenanceSource: null, + sshEnabled: false, held: null, ...over, }; diff --git a/apps/server/web/src/components/sidebar/sidebar-nav.ts b/apps/server/web/src/components/sidebar/sidebar-nav.ts index 5d95cbcf0..946af9fb8 100644 --- a/apps/server/web/src/components/sidebar/sidebar-nav.ts +++ b/apps/server/web/src/components/sidebar/sidebar-nav.ts @@ -1,6 +1,7 @@ import { Activity, ArrowUpCircle, + Cable, DatabaseBackup, KeyRound, LayoutDashboard, @@ -57,6 +58,10 @@ const NAV_ENTRIES: NavEntry[] = [ // Terminal, like the empty subshells box — subshells are terminal harnesses, // not a grid (the grid icon belongs to the tiles/list view toggle). { to: "/", label: "Subshells", icon: TerminalSquare }, + // A primary action, so it sits above the objects it creates, not under + // Settings (spec 2026-10-07 §7). Cable names the link being opened; the + // icons test pins global uniqueness. + { to: "/connect", label: "Connect", icon: Cable }, { to: "/workspaces", label: "Workspaces", icon: LayoutDashboard, short: "Wksp" }, // Personal settings (spec 2026-10-07 §A): the pages this viewer always owns, // grouped the way the admin's Server Settings group is. No gate: every diff --git a/apps/server/web/src/components/subshell-picker/__tests__/new-subshell-form.test.tsx b/apps/server/web/src/components/subshell-picker/__tests__/new-subshell-form.test.tsx index 44ae5f17b..e2bef6da1 100644 --- a/apps/server/web/src/components/subshell-picker/__tests__/new-subshell-form.test.tsx +++ b/apps/server/web/src/components/subshell-picker/__tests__/new-subshell-form.test.tsx @@ -193,6 +193,7 @@ const BOX = { maintenance: false, maintenanceAt: null, maintenanceSource: null, + sshEnabled: false, held: null, }; diff --git a/apps/server/web/src/components/subshell-picker/__tests__/no-launch-targets.test.tsx b/apps/server/web/src/components/subshell-picker/__tests__/no-launch-targets.test.tsx index 351f7fb79..60f367fac 100644 --- a/apps/server/web/src/components/subshell-picker/__tests__/no-launch-targets.test.tsx +++ b/apps/server/web/src/components/subshell-picker/__tests__/no-launch-targets.test.tsx @@ -56,6 +56,7 @@ function node(overrides: Partial): Node { maintenance: false, maintenanceAt: null, maintenanceSource: null, + sshEnabled: false, held: null, ...overrides, }; diff --git a/apps/server/web/src/hooks/__tests__/use-node-harnesses.test.tsx b/apps/server/web/src/hooks/__tests__/use-node-harnesses.test.tsx index 3839a5387..181047db7 100644 --- a/apps/server/web/src/hooks/__tests__/use-node-harnesses.test.tsx +++ b/apps/server/web/src/hooks/__tests__/use-node-harnesses.test.tsx @@ -38,6 +38,7 @@ const NODE: NodeDetail = { maintenance: false, maintenanceAt: null, maintenanceSource: null, + sshEnabled: false, held: null, }; diff --git a/apps/server/web/src/hooks/__tests__/use-nodes.test.tsx b/apps/server/web/src/hooks/__tests__/use-nodes.test.tsx index 0665a93c4..eac2e457e 100644 --- a/apps/server/web/src/hooks/__tests__/use-nodes.test.tsx +++ b/apps/server/web/src/hooks/__tests__/use-nodes.test.tsx @@ -40,6 +40,7 @@ const NODE: Node = { maintenance: false, maintenanceAt: null, maintenanceSource: null, + sshEnabled: false, held: null, }; diff --git a/apps/server/web/src/hooks/__tests__/use-ssh.test.tsx b/apps/server/web/src/hooks/__tests__/use-ssh.test.tsx new file mode 100644 index 000000000..e48096316 --- /dev/null +++ b/apps/server/web/src/hooks/__tests__/use-ssh.test.tsx @@ -0,0 +1,128 @@ +import { afterEach, describe, expect, it } from "bun:test"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { cleanup, renderHook, waitFor } from "@testing-library/react"; +import type { ReactNode } from "react"; +import { useLaunchSsh, useSshAliases, useSshSavedHosts } from "@/hooks/use-ssh"; + +/** + * The keys and URLs this file states, asserted on the WIRE the way + * `use-instance-plugins.test.tsx` asserts its invalidations — a hook that + * stops refreshing cannot pass by invalidating some other key. + * + * Three facts: the ledger fetches once under `["ssh-saved-hosts"]`; aliases + * are per-machine and gated (a null picker asks the node nothing); and a + * successful launch refetches the ledger ONLY — the recency row moved, the + * machine's config did not. + */ +interface Call { + method: string; + url: string; +} + +const json = (body: unknown, status = 200) => new Response(JSON.stringify(body), { status }); + +function mockFetch(overrides: Record Response> = {}) { + const calls: Call[] = []; + const original = globalThis.fetch; + globalThis.fetch = ((input: unknown, init?: RequestInit) => { + const url = new URL(String(input), "http://localhost"); + const method = init?.method ?? "GET"; + calls.push({ method, url: url.pathname + url.search }); + const custom = overrides[`${method} ${url.pathname}`]; + return Promise.resolve(custom ? custom() : json({})); + }) as typeof fetch; + return { calls, restore: () => (globalThis.fetch = original) }; +} + +function makeWrapper() { + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + const wrapper = ({ children }: { children: ReactNode }) => ( + {children} + ); + return { client, wrapper }; +} + +const savedHosts = { saved: [], recent: [], defaultNodeId: null }; + +afterEach(cleanup); + +describe("useSshSavedHosts", () => { + it("fetches the ledger once at /api/ssh/saved-hosts", async () => { + const { calls, restore } = mockFetch({ "GET /api/ssh/saved-hosts": () => json(savedHosts) }); + try { + const { wrapper } = makeWrapper(); + const { result } = renderHook(() => useSshSavedHosts(), { wrapper }); + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + expect(calls.filter((c) => c.url === "/api/ssh/saved-hosts")).toEqual([ + { method: "GET", url: "/api/ssh/saved-hosts" }, + ]); + } finally { + restore(); + } + }); +}); + +describe("useSshAliases", () => { + it("asks nothing while no machine is chosen", async () => { + const { calls, restore } = mockFetch(); + try { + const { wrapper } = makeWrapper(); + const { result } = renderHook(() => useSshAliases(null), { wrapper }); + // `enabled: false` parks the query in its idle state — never fetching. + await new Promise((r) => setTimeout(r, 20)); + expect(result.current.fetchStatus).toBe("idle"); + expect(calls.filter((c) => c.url.startsWith("/api/ssh/aliases"))).toEqual([]); + } finally { + restore(); + } + }); + + it("names the machine on the wire when one is", async () => { + const { calls, restore } = mockFetch({ + "GET /api/ssh/aliases": () => json({ aliases: ["box"], includeCycle: false, truncated: false }), + }); + try { + const { wrapper } = makeWrapper(); + const { result } = renderHook(() => useSshAliases("n1"), { wrapper }); + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + expect(calls).toEqual([{ method: "GET", url: "/api/ssh/aliases?node=n1" }]); + } finally { + restore(); + } + }); +}); + +describe("useLaunchSsh", () => { + it("refetches the ledger and nothing else on success", async () => { + const { calls, restore } = mockFetch({ + "GET /api/ssh/saved-hosts": () => json(savedHosts), + "GET /api/ssh/aliases": () => json({ aliases: [], includeCycle: false, truncated: false }), + "POST /api/ssh/launch": () => json({ subshell: { id: "s1" } }, 201), + }); + try { + const { wrapper } = makeWrapper(); + // Both lists mounted as observers: only the ledger may move. + const { result } = renderHook( + () => ({ launch: useLaunchSsh(), hosts: useSshSavedHosts(), aliases: useSshAliases("n1") }), + { + wrapper, + }, + ); + await waitFor(() => expect(result.current.hosts.isSuccess && result.current.aliases.isSuccess).toBe(true)); + const before = { ...counts(calls) }; + result.current.launch.mutate({ node: "n1", destination: "box" }); + await waitFor(() => expect(result.current.launch.isSuccess).toBe(true)); + await waitFor(() => expect(counts(calls).saved - before.saved).toBe(1)); + expect(counts(calls).aliases).toBe(before.aliases); + } finally { + restore(); + } + }); +}); + +function counts(calls: Call[]): { saved: number; aliases: number } { + return { + saved: calls.filter((c) => c.url === "/api/ssh/saved-hosts").length, + aliases: calls.filter((c) => c.url.startsWith("/api/ssh/aliases")).length, + }; +} diff --git a/apps/server/web/src/hooks/use-ssh.ts b/apps/server/web/src/hooks/use-ssh.ts new file mode 100644 index 000000000..c688cae8b --- /dev/null +++ b/apps/server/web/src/hooks/use-ssh.ts @@ -0,0 +1,108 @@ +import { apiFetch, apiPost } from "@internal/node-admin"; +import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; +import type { + SshAliasesView, + SshLaunchRequest, + SshLaunchResponse, + SshSavedHost, + SshSavedHostsView, + SshSaveHostRequest, +} from "@/lib/ssh"; + +/** + * The SSH launcher's data (spec 2026-10-07 §7), following `use-prompts.ts`: + * one key per list, one URL per key, mutations invalidate rather than patch. + * + * The saved-hosts view carries the ledger AND the default-machine preference, + * so every mutation that can move either one invalidates `["ssh-saved-hosts"]` + * and nothing else: a launch touched the recency row server-side, a save or + * delete moves the ledger, the preference PATCH moves `defaultNodeId`. The + * aliases are the MACHINE's config, which none of these acts edits, so no + * mutation invalidates them — the machine disclosure refetches them only when + * the chosen machine changes. + */ + +export const SSH_SAVED_HOSTS_QUERY_KEY = ["ssh-saved-hosts"] as const; +export const sshAliasesQueryKey = (nodeId: string) => ["ssh-aliases", nodeId] as const; + +/** The owner's saved + recent destinations and their default connecting machine. */ +export function useSshSavedHosts() { + return useQuery({ + queryKey: SSH_SAVED_HOSTS_QUERY_KEY, + queryFn: () => apiFetch("/api/ssh/saved-hosts"), + }); +} + +/** + * The machine's alias names for the destination picker. `enabled` gates on + * the caller actually having chosen a machine: an unresolved picker asks + * nothing, so a half-open panel never pings a node with `node=` empty. + */ +export function useSshAliases(nodeId: string | null) { + return useQuery({ + queryKey: sshAliasesQueryKey(nodeId ?? ""), + queryFn: () => apiFetch(`/api/ssh/aliases?node=${encodeURIComponent(nodeId ?? "")}`), + enabled: nodeId !== null, + }); +} + +/** Invalidate the ledger after an act that moved it (the one rule the mutations share). */ +function useInvalidateSavedHosts(): () => Promise { + const queryClient = useQueryClient(); + return () => queryClient.invalidateQueries({ queryKey: SSH_SAVED_HOSTS_QUERY_KEY }); +} + +/** + * `PUT /api/ssh/saved-hosts` — resolves the destination first, so a + * refusal-shaped outcome answers 422 `{outcome}` and stores nothing; the + * panel renders that, an `ApiError` with status 422. + */ +export function useSaveSshHost() { + const invalidate = useInvalidateSavedHosts(); + return useMutation({ + mutationFn: (draft: SshSaveHostRequest) => + apiFetch("/api/ssh/saved-hosts", { method: "PUT", body: JSON.stringify(draft) }), + onSuccess: () => void invalidate(), + }); +} + +/** `DELETE /api/ssh/saved-hosts/:id` — 204 no body; a foreign row is the same 404 as an absent one. */ +export function useDeleteSshHost() { + const invalidate = useInvalidateSavedHosts(); + return useMutation({ + mutationFn: (id: string) => apiFetch(`/api/ssh/saved-hosts/${encodeURIComponent(id)}`, { method: "DELETE" }), + onSuccess: () => void invalidate(), + }); +} + +/** + * `PATCH /api/ssh/preferences` — sets (or clears, with null) the default + * connecting machine. The answer echoes the stored id; the cache still + * refetches, because the SAME view carries the ledger and a stale preference + * there is the picker defaulting to the wrong machine. + */ +export function useSetDefaultNode() { + const invalidate = useInvalidateSavedHosts(); + return useMutation({ + mutationFn: (defaultNodeId: string | null) => + apiFetch<{ defaultNodeId: string | null }>("/api/ssh/preferences", { + method: "PATCH", + body: JSON.stringify({ defaultNodeId }), + }), + onSuccess: () => void invalidate(), + }); +} + +/** + * `POST /api/ssh/launch` `{node, destination, name?}` — one POST, one pane. + * Success invalidates the ledger only: the launch touched (or created) the + * destination's recency row server-side, while the machine's aliases are + * untouched by the act. + */ +export function useLaunchSsh() { + const invalidate = useInvalidateSavedHosts(); + return useMutation({ + mutationFn: (draft: SshLaunchRequest) => apiPost("/api/ssh/launch", draft), + onSuccess: () => void invalidate(), + }); +} diff --git a/apps/server/web/src/lib/__tests__/api.test.ts b/apps/server/web/src/lib/__tests__/api.test.ts index c8a984c08..a0401a3fd 100644 --- a/apps/server/web/src/lib/__tests__/api.test.ts +++ b/apps/server/web/src/lib/__tests__/api.test.ts @@ -27,6 +27,13 @@ describe("apiFetch error classes", () => { expect((err as ApiError).status).toBe(404); }); + it("204 No Content resolves undefined — a body-less success is not a parse failure", async () => { + // The ssh saved-host DELETE answers 204 with NO body (a 204 may not carry + // one); parsing one is a throw, so the helper must return before it tries. + globalThis.fetch = (async () => new Response(null, { status: 204 })) as unknown as typeof fetch; + await expect(apiFetch("/api/ssh/saved-hosts/x")).resolves.toBeUndefined(); + }); + it("isNetworkError matches only NetworkError (plain errors stay fail-fast)", () => { expect(isNetworkError(new NetworkError(new TypeError("x")))).toBe(true); expect(isNetworkError(new TypeError("x"))).toBe(false); diff --git a/apps/server/web/src/lib/__tests__/subshell-compat.test.ts b/apps/server/web/src/lib/__tests__/subshell-compat.test.ts index ebd06f1cb..6f8c75f5f 100644 --- a/apps/server/web/src/lib/__tests__/subshell-compat.test.ts +++ b/apps/server/web/src/lib/__tests__/subshell-compat.test.ts @@ -32,6 +32,7 @@ function node(overrides: Partial): Node { maintenance: false, maintenanceAt: null, maintenanceSource: null, + sshEnabled: false, held: null, ...overrides, }; diff --git a/apps/server/web/src/lib/ssh.ts b/apps/server/web/src/lib/ssh.ts new file mode 100644 index 000000000..c36040491 --- /dev/null +++ b/apps/server/web/src/lib/ssh.ts @@ -0,0 +1,157 @@ +/** + * The client shapes of the `/api/ssh` surface (spec 2026-10-07 §5–§7), the + * same mirror `lib/prompts.ts` keeps for the prompt library: local types that + * state the wire field for field, read through `apiFetch`. The definitions + * live beside the routes that answer them + * (`apps/server/api/src/api/ssh/ssh-views.ts`); the copy here restates that + * contract for the SPA, and the two are kept honest by review like every + * other mirror type in this app. + */ + +/** One ProxyJump hop as the frozen snapshot spells it. */ +export interface SshHop { + /** Hop hostname (bracketed IPv6 literal kept as given) */ + host: string; + /** Hop user; null means the connecting account's own default */ + user: string | null; + /** Hop port, resolved */ + port: number; +} + +/** + * The approved connection snapshot, exactly as the resolve/422 outcome + * serializes it. The always-null fields are load-bearing: a resolved value + * that the approved normalization cannot run makes the destination REFUSABLE, + * never renderable — so a field arriving non-null here is a server bug, and + * the `null` type is the wire's way of saying the panel never has to render + * one. + */ +export interface SshSnapshot { + /** The config token the human chose (display and review context only) */ + alias: string; + /** Resolved destination hostname */ + host: string; + /** Destination user; null means the connecting account's own default */ + user: string | null; + /** Destination port, resolved (1..65535) */ + port: number; + /** Identity file references (absolute paths on the connecting machine, never key contents) */ + identityFiles: string[]; + /** Certificate file references (paths only) */ + certificateFiles: string[]; + /** The authentication-agent socket to use, or null (runs with no agent) */ + authAgentSocket: string | null; + /** Known-hosts files to consult (paths only) */ + knownHostsFiles: string[]; + /** The name strict host checking looks up, or null */ + hostKeyAlias: string | null; + /** Bounded ProxyJump chain, outermost first */ + proxyJumps: SshHop[]; + /** Always null: a resolved ProxyCommand refuses the snapshot, never renders it */ + proxyCommand: null; + /** Always null: any resolved forwarding refuses the snapshot */ + forwards: null; + /** Always null: any resolved tunnel refuses the snapshot */ + tunnels: null; + /** Always null: any resolved local command refuses the snapshot */ + localCommands: null; + /** Always null: a resolved RemoteCommand refuses the snapshot */ + remoteCommand: null; + /** Always null: a resolved SendEnv refuses the snapshot */ + sendEnv: null; + /** Always null: a resolved SetEnv refuses the snapshot */ + setEnv: null; + /** Always null: live escape characters refuse the snapshot */ + escapes: null; +} + +/** + * `POST /api/ssh/resolve`'s answer, and the body of every 422 the launch and + * save endpoints answer: the refusal is IN THE DATA, so the panel renders it + * rather than catching it. `code` is an `SshErrorCode` name + * (unsupported_setting, config_missing, config_ambiguous, + * proxy_chain_too_long, host_key_unknown, …); `settings` names what blocked, + * empty when the code names the whole cause. + */ +export type SshResolveOutcome = + | { + /** The destination normalized cleanly into an approved snapshot */ + accepted: true; + snapshot: SshSnapshot; + /** The connecting account's OS user name, when the machine could report it (display only) */ + connectingAccount?: string; + } + | { + /** Resolution refused: the config needs more than the approved normalization can run */ + accepted: false; + code: string; + settings: string[]; + }; + +/** One saved/recency row as its owner sees it. */ +export interface SshSavedHost { + /** Row id (uuid) */ + id: string; + /** Canonical host:port (or user@host:port), the resolved destination as key */ + destination: string; + /** Display token (the alias typed or discovered); never a key */ + alias: string | null; + /** The connecting machine of the most recent launch to this destination */ + nodeId: string; + /** ISO 8601 the human saved it; null = recency-only row */ + savedAt: string | null; + /** ISO 8601 of the most recent launch to this destination */ + lastConnectAt: string; +} + +/** The `GET /api/ssh/saved-hosts` body: the destination field's feed and the machine preference. */ +export interface SshSavedHostsView { + /** Rows a human saved, newest save first (max 20) */ + saved: SshSavedHost[]; + /** Most-recently-connected destinations, saved or not, newest first (max 20) */ + recent: SshSavedHost[]; + /** + * The caller's default connecting machine, or null. A node deleted since + * reads back as the stored id; the client decides. + */ + defaultNodeId: string | null; +} + +/** The `GET /api/ssh/aliases?node=` body: names only, never config contents. */ +export interface SshAliasesView { + /** Usable alias NAMES (sorted, deduplicated, concrete names only), at most 500 */ + aliases: string[]; + /** An include cycle was detected; the list is what parsed before it */ + includeCycle: boolean; + /** The alias cap was hit; more exist */ + truncated: boolean; +} + +/** `PUT /api/ssh/saved-hosts` body: the machine gates the resolve, the alias is a display override. */ +export interface SshSaveHostRequest { + /** Machine to resolve the destination on (the gate owner) */ + node: string; + /** Destination token (1..253): an alias from the machine's config or a concrete host */ + destination: string; + /** Display label override; absent keeps the resolved snapshot's own alias */ + alias?: string; +} + +/** `POST /api/ssh/launch` body: name is the pane's display name, optional. */ +export interface SshLaunchRequest { + /** Connecting machine ('local' = the control-plane host) */ + node: string; + /** Destination token (1..253), resolved on the machine before anything launches */ + destination: string; + /** Pane display name */ + name?: string; +} + +/** + * The `POST /api/ssh/launch` 201 body. The wire carries the full subshell + * view; only the id is declared because only the id is consumed — success + * navigates to `/subshells/$id`, exactly like the launch dialog's create. + */ +export interface SshLaunchResponse { + subshell: { id: string }; +} diff --git a/apps/server/web/src/routes/__tests__/nodes-detail.test.tsx b/apps/server/web/src/routes/__tests__/nodes-detail.test.tsx index 89f0563cb..5d3819259 100644 --- a/apps/server/web/src/routes/__tests__/nodes-detail.test.tsx +++ b/apps/server/web/src/routes/__tests__/nodes-detail.test.tsx @@ -39,6 +39,7 @@ function enrolledNode(overrides: Partial = {}): NodeDetail { maintenance: false, maintenanceAt: null, maintenanceSource: null, + sshEnabled: false, held: null, ...overrides, }; diff --git a/apps/server/web/src/routes/__tests__/nodes-page-gate.test.tsx b/apps/server/web/src/routes/__tests__/nodes-page-gate.test.tsx index 92c867167..a676e46e1 100644 --- a/apps/server/web/src/routes/__tests__/nodes-page-gate.test.tsx +++ b/apps/server/web/src/routes/__tests__/nodes-page-gate.test.tsx @@ -151,6 +151,7 @@ describe("/nodes add-node gating", () => { maintenance: false, maintenanceAt: null, maintenanceSource: null, + sshEnabled: false, held: null, }, ], diff --git a/apps/server/web/src/routes/__tests__/nodes-tabs.test.tsx b/apps/server/web/src/routes/__tests__/nodes-tabs.test.tsx index 307dce4a0..ddb6938c4 100644 --- a/apps/server/web/src/routes/__tests__/nodes-tabs.test.tsx +++ b/apps/server/web/src/routes/__tests__/nodes-tabs.test.tsx @@ -28,6 +28,7 @@ const NODE = { maintenance: false, maintenanceAt: null, maintenanceSource: null, + sshEnabled: false, held: null, }; diff --git a/packages/node-admin/src/__tests__/node-allowed-dirs.test.tsx b/packages/node-admin/src/__tests__/node-allowed-dirs.test.tsx index 3c380d3d8..eaea6ebfd 100644 --- a/packages/node-admin/src/__tests__/node-allowed-dirs.test.tsx +++ b/packages/node-admin/src/__tests__/node-allowed-dirs.test.tsx @@ -33,6 +33,7 @@ function node(over: Partial = {}): Node { maintenance: false, maintenanceAt: null, maintenanceSource: null, + sshEnabled: false, held: null, ...over, }; diff --git a/packages/node-admin/src/__tests__/node-maintenance-card.test.tsx b/packages/node-admin/src/__tests__/node-maintenance-card.test.tsx index 8ce12dc2d..9c91ec548 100644 --- a/packages/node-admin/src/__tests__/node-maintenance-card.test.tsx +++ b/packages/node-admin/src/__tests__/node-maintenance-card.test.tsx @@ -40,6 +40,7 @@ function node(overrides: Partial = {}): NodeDetail { maintenance: false, maintenanceAt: null, maintenanceSource: null, + sshEnabled: false, held: null, ...overrides, }; diff --git a/packages/node-admin/src/__tests__/node-runtime-card.test.tsx b/packages/node-admin/src/__tests__/node-runtime-card.test.tsx index 7420c17f3..03310153a 100644 --- a/packages/node-admin/src/__tests__/node-runtime-card.test.tsx +++ b/packages/node-admin/src/__tests__/node-runtime-card.test.tsx @@ -33,6 +33,7 @@ const base: NodeDetail = { maintenance: false, maintenanceAt: null, maintenanceSource: null, + sshEnabled: false, held: null, }; diff --git a/packages/node-admin/src/__tests__/node-service-card.test.tsx b/packages/node-admin/src/__tests__/node-service-card.test.tsx index b377a3dfd..0f791e570 100644 --- a/packages/node-admin/src/__tests__/node-service-card.test.tsx +++ b/packages/node-admin/src/__tests__/node-service-card.test.tsx @@ -48,6 +48,7 @@ const base: NodeDetail = { maintenance: false, maintenanceAt: null, maintenanceSource: null, + sshEnabled: false, held: null, }; diff --git a/packages/node-admin/src/lib/api.ts b/packages/node-admin/src/lib/api.ts index 4a559b7b9..32a9cb444 100644 --- a/packages/node-admin/src/lib/api.ts +++ b/packages/node-admin/src/lib/api.ts @@ -107,6 +107,10 @@ export async function apiFetch(path: string, init?: RequestInit): Promise const { message, code, errId } = parseErrorBody(body); throw new ApiError(res.status, message, { code, errId }); } + // 204 may not carry a body (a `Response` constructed with one throws), and + // `res.json()` on an empty one throws too. The ssh saved-host DELETE is the + // first route to answer it; a body-less success resolves undefined. + if (res.status === 204) return undefined as unknown as T; return (await res.json()) as T; } diff --git a/packages/node-admin/src/types/node.ts b/packages/node-admin/src/types/node.ts index 2ac496763..941035cdc 100644 --- a/packages/node-admin/src/types/node.ts +++ b/packages/node-admin/src/types/node.ts @@ -167,6 +167,16 @@ export interface Node { maintenanceAt: string | null; /** Which end declared it; null when it has never been declared */ maintenanceSource: MaintenanceSource | null; + /** + * Whether this machine may be used for Subshell SSH (spec 2026-10-07). + * + * Opt-in per node, off by default on every row including the control-plane + * host; the SSH gate refuses launches, discovery and saves here while false, + * whoever asks. The server's schema requires the boolean, so it rides every + * node view — the launcher's machine disclosure reads it to keep a gated-off + * machine visible but disabled, never hidden. + */ + sshEnabled: boolean; /** * This node is connected but REFUSED — held open for exactly one * command and offline for every other purpose (spec 2026-09-15 §5.3). From c43cb77014ddc2019102ae7a1dc144846bc45165 Mon Sep 17 00:00:00 2001 From: Theo Gravity Date: Wed, 7 Oct 2026 23:33:44 -0700 Subject: [PATCH 04/16] fix(node-admin): ApiError keeps the parsed failure body beside the sliced message The SSH launch route answers a refused resolve with a 422 {outcome} whose settings list can outgrow the 200-char display slice every ApiError message carries. parseErrorBody now hands the parsed JSON object to its callers, and apiFetch passes it through as err.body; the API : string is byte-identical, and legacy text bodies keep no body. --- .../node-admin/src/lib/__tests__/api.test.ts | 66 +++++++++++++++++++ packages/node-admin/src/lib/api.ts | 41 +++++++++--- 2 files changed, 98 insertions(+), 9 deletions(-) create mode 100644 packages/node-admin/src/lib/__tests__/api.test.ts diff --git a/packages/node-admin/src/lib/__tests__/api.test.ts b/packages/node-admin/src/lib/__tests__/api.test.ts new file mode 100644 index 000000000..2caa14629 --- /dev/null +++ b/packages/node-admin/src/lib/__tests__/api.test.ts @@ -0,0 +1,66 @@ +import { afterEach, describe, expect, it } from "bun:test"; +import { ApiError, apiFetch, parseErrorBody } from "../api"; + +/** + * `ApiError.body` (Task 3's un-truncation): the display message is a + * `API : ` string sliced to 200 chars, and a structured + * refusal body can be longer than the slice. The parsed JSON object must + * survive on `err.body` so a renderer (the SSH launcher's 422 `outcome`) + * reads the WHOLE answer; the message stays byte-identical, and a legacy + * text body carries no `body` at all. + */ +const originalFetch = globalThis.fetch; +afterEach(() => { + globalThis.fetch = originalFetch; +}); + +describe("ApiError.body", () => { + it("keeps a 422-shaped outcome on err.body even when the message is sliced to 200 chars", async () => { + // The launch route answers the refused resolve with `{outcome}` — NOT the + // standard envelope — so `message` there is the raw JSON text and the + // slice lands mid-body. Many blocking settings push it past 200 chars. + const settings = Array.from({ length: 40 }, (_, i) => `BlockingSetting${i}`); + const body = { outcome: { accepted: false, code: "unsupported_setting", settings } }; + const raw = JSON.stringify(body); + expect(raw.length).toBeGreaterThan(200); // the slice is actually exercised + globalThis.fetch = (async () => new Response(raw, { status: 422 })) as unknown as typeof fetch; + const err = (await apiFetch("/api/ssh/launch", { method: "POST", body: "{}" }).catch( + (e: unknown) => e, + )) as ApiError; + expect(err).toBeInstanceOf(ApiError); + expect(err.status).toBe(422); + // The display string is byte-identical to the old sliced form. + expect(err.message).toBe(`API 422: ${raw.slice(0, 200)}`); + // The whole refusal survived the slice: every setting is readable. + const outcome = (err.body as { outcome: { accepted: boolean; code: string; settings: string[] } }).outcome; + expect(outcome.accepted).toBe(false); + expect(outcome.settings).toEqual(settings); + }); + + it("carries the parsed envelope too (code/errId unchanged, body added)", async () => { + const body = { errId: "e1", code: "SSH_GATE_OFF", message: "Subshell SSH is off on this machine", statusCode: 403 }; + globalThis.fetch = (async () => new Response(JSON.stringify(body), { status: 403 })) as unknown as typeof fetch; + const err = (await apiFetch("/api/ssh/launch").catch((e: unknown) => e)) as ApiError; + expect(err).toBeInstanceOf(ApiError); + expect(err.status).toBe(403); + expect(err.code).toBe("SSH_GATE_OFF"); + expect(err.errId).toBe("e1"); + expect(err.message).toBe("API 403: Subshell SSH is off on this machine"); + expect(err.body).toEqual(body); + }); + + it("leaves body undefined for a legacy plain-text failure (nothing to keep)", async () => { + globalThis.fetch = (async () => new Response("Bad Gateway", { status: 502 })) as unknown as typeof fetch; + const err = (await apiFetch("/api/anything").catch((e: unknown) => e)) as ApiError; + expect(err).toBeInstanceOf(ApiError); + expect(err.message).toBe("API 502: Bad Gateway"); + expect(err.body).toBeUndefined(); + }); + + it("parseErrorBody hands the object to every caller unchanged", () => { + const parsed = parseErrorBody(JSON.stringify({ outcome: { accepted: false } })); + expect(parsed.message).toContain("outcome"); // no `message` key: raw text stands + expect(parsed.body).toEqual({ outcome: { accepted: false } }); + expect(parseErrorBody("plain").body).toBeUndefined(); + }); +}); diff --git a/packages/node-admin/src/lib/api.ts b/packages/node-admin/src/lib/api.ts index 32a9cb444..dabf9d729 100644 --- a/packages/node-admin/src/lib/api.ts +++ b/packages/node-admin/src/lib/api.ts @@ -21,12 +21,22 @@ export class ApiError extends Error { * thing to quote when reporting a bug; undefined for legacy text failures. */ readonly errId?: string; - constructor(status: number, body: string, meta: { code?: string; errId?: string } = {}) { + /** + * The parsed JSON body itself when the failure carried one — the machine- + * readable payload the display message cannot hold: the message is the + * raw text sliced to 200 chars, while a structured refusal (the SSH 422 + * `{outcome}` with its `settings` list) can run longer than that. Readers + * that need the WHOLE answer (`err.body`) must not parse the message. + * Undefined for legacy plain-text bodies (nothing to keep). + */ + readonly body?: unknown; + constructor(status: number, body: string, meta: { code?: string; errId?: string; body?: unknown } = {}) { super(`API ${status}: ${body.slice(0, 200)}`); this.name = "ApiError"; this.status = status; this.code = meta.code; this.errId = meta.errId; + this.body = meta.body; } } @@ -59,10 +69,14 @@ export function isAbortError(err: unknown): boolean { } /** - * Splits a failed response body into the display message and the structured - * fields. Every backend failure now carries `{errId, code, message, statusCode}`; - * anything that is not that shape (a proxy error page, an older body) falls - * back to the raw text so the `API : ` copy never changes form. + * Splits a failed response body into the display message, the structured + * fields, and the parsed body itself. Every backend failure now carries + * `{errId, code, message, statusCode}`; anything that is not that shape (a + * proxy error page, an older body) falls back to the raw text so the + * `API : ` copy never changes form. Any JSON OBJECT — the + * envelope or not — returns its parsed form as `body`, so a caller who needs + * the whole answer (the SSH 422 `{outcome}`) reads `body`, never the + * length-limited message. */ /** * Exported for the one caller that cannot use {@link apiFetch}: the agent @@ -70,7 +84,12 @@ export function isAbortError(err: unknown): boolean { * arrives as an ordinary status code with this body, and it should surface as * the same `ApiError` every other call produces. */ -export function parseErrorBody(raw: string): { message: string; code?: string; errId?: string } { +export function parseErrorBody(raw: string): { + message: string; + code?: string; + errId?: string; + body?: unknown; +} { try { const parsed: unknown = JSON.parse(raw); if (parsed && typeof parsed === "object") { @@ -79,6 +98,10 @@ export function parseErrorBody(raw: string): { message: string; code?: string; e message: typeof message === "string" ? message : raw, code: typeof code === "string" ? code : undefined, errId: typeof errId === "string" ? errId : undefined, + // The whole parsed object rides along: `message` is a DISPLAY string + // sliced to 200 chars by ApiError, and a structured refusal body + // (the SSH 422 `{outcome}`) can be longer than the display allows. + body: parsed, }; } } catch { @@ -103,9 +126,9 @@ export async function apiFetch(path: string, init?: RequestInit): Promise throw new NetworkError(err); } if (!res.ok) { - const body = await res.text().catch(() => ""); - const { message, code, errId } = parseErrorBody(body); - throw new ApiError(res.status, message, { code, errId }); + const raw = await res.text().catch(() => ""); + const { message, code, errId, body } = parseErrorBody(raw); + throw new ApiError(res.status, message, { code, errId, body }); } // 204 may not carry a body (a `Response` constructed with one throws), and // `res.json()` on an empty one throws too. The ssh saved-host DELETE is the From 0d695f1dc6910e783d353ad64afd99b1b1fd3c2c Mon Sep 17 00:00:00 2001 From: Theo Gravity Date: Wed, 7 Oct 2026 23:48:32 -0700 Subject: [PATCH 05/16] feat(web): the destination-first Connect page /connect opens with the destination leading: saved rows, recent rows, the picked machine's config aliases, and a typed host the list mirrors as its own row (input-plus-panel posture - Base UI's held combobox cannot keep typed text across per-keystroke item swaps, the working-directory field learned this first). The connecting machine follows as a disclosure line; a gated-off machine stays visible and greyed with its reason, the default pre-selects honestly, and no preference plus several enabled machines is a required question with the gold star, never a silent pick. One POST opens the pane; refusals render on the field they belong to, the 422's blocked settings read verbatim from the parsed body (the message slice could cut them). The two-sentence Match exec disclosure sits under the destination before the button; Remember is the explicit PUT after success, the alias token riding only config-list picks. The ledger below lists remembered and recent rows with forget/remember acts, the recent copy reading once when a destination is already saved. --- apps/server/web/docs/connect-page.md | 17 + .../connect/__tests__/connect-panel.test.tsx | 478 ++++++++++++++++++ .../__tests__/destination-options.test.ts | 90 ++++ .../__tests__/saved-hosts-section.test.tsx | 137 +++++ .../connect/__tests__/ssh-refusal.test.ts | 136 +++++ .../src/components/connect/connect-panel.tsx | 252 +++++++++ .../components/connect/destination-field.tsx | 139 +++++ .../components/connect/destination-options.ts | 98 ++++ .../connect/saved-hosts-section.tsx | 111 ++++ .../web/src/components/connect/ssh-refusal.ts | 122 +++++ apps/server/web/src/routeTree.gen.ts | 21 + .../web/src/routes/__tests__/connect.test.tsx | 58 +++ apps/server/web/src/routes/connect.tsx | 24 + 13 files changed, 1683 insertions(+) create mode 100644 apps/server/web/docs/connect-page.md create mode 100644 apps/server/web/src/components/connect/__tests__/connect-panel.test.tsx create mode 100644 apps/server/web/src/components/connect/__tests__/destination-options.test.ts create mode 100644 apps/server/web/src/components/connect/__tests__/saved-hosts-section.test.tsx create mode 100644 apps/server/web/src/components/connect/__tests__/ssh-refusal.test.ts create mode 100644 apps/server/web/src/components/connect/connect-panel.tsx create mode 100644 apps/server/web/src/components/connect/destination-field.tsx create mode 100644 apps/server/web/src/components/connect/destination-options.ts create mode 100644 apps/server/web/src/components/connect/saved-hosts-section.tsx create mode 100644 apps/server/web/src/components/connect/ssh-refusal.ts create mode 100644 apps/server/web/src/routes/__tests__/connect.test.tsx create mode 100644 apps/server/web/src/routes/connect.tsx diff --git a/apps/server/web/docs/connect-page.md b/apps/server/web/docs/connect-page.md new file mode 100644 index 000000000..da752b110 --- /dev/null +++ b/apps/server/web/docs/connect-page.md @@ -0,0 +1,17 @@ +# The Connect page + +Surface doc for `apps/server/web`, spec 2026-10-07 §7 (`/connect`, plan 3). + +The page asks one question first: a destination, chosen from the caller's +saved rows, their recent rows, and the picked machine's SSH config aliases - +or typed free, which the list mirrors as its own row until it is committed. +One POST launches: the connecting machine resolves the token on the human's +own SSH config, and the panel renders the server's refusal (the blocked +`settings` list included, read from the parsed body) rather than guessing. + +Two UI facts worth keeping deliberate: a machine whose SSH is off stays in +the disclosure greyed with its reason, never hidden (spec 2026-10-07 §4.3); +and the destination field is an input-plus-panel, not the shared combobox, +because Base UI's held input loses typed text when its items swap on every +keystroke (the posture is the working-directory field's, for the same +reason). diff --git a/apps/server/web/src/components/connect/__tests__/connect-panel.test.tsx b/apps/server/web/src/components/connect/__tests__/connect-panel.test.tsx new file mode 100644 index 000000000..a984dbb7f --- /dev/null +++ b/apps/server/web/src/components/connect/__tests__/connect-panel.test.tsx @@ -0,0 +1,478 @@ +import { afterEach, describe, expect, it } from "bun:test"; +import type { Node } from "@internal/node-admin"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { + createMemoryHistory, + createRootRoute, + createRoute, + createRouter, + RouterProvider, +} from "@tanstack/react-router"; +import { act, cleanup, fireEvent, render, screen, waitFor, within } from "@testing-library/react"; +import { ConnectPanel, SSH_DISCLOSURE_COPY } from "@/components/connect/connect-panel"; + +/** + * The /connect behavior contract (plan 3 Task 3), pinned on the composed + * panel with the wire mocked the way `new-subshell-form.test.tsx` mocks it + * (fetch, not the hooks - the hooks are pinned by `use-ssh.test.tsx`). The + * refusal CODE matrix is pinned purely in `ssh-refusal.test.ts`; two arms run + * through the panel here to prove the wiring (the 422 out of the parsed body, + * the 409 under the machine field). + */ + +interface Call { + method: string; + url: string; + body?: unknown; +} + +function node(overrides: Partial): Node { + return { + id: "n", + name: "node", + kind: "agent", + os: null, + arch: null, + hostname: null, + status: "online", + lastSeenAt: null, + agentVersion: null, + protocolVersion: null, + access: "owner", + canManage: true, + canLaunch: true, + allowedDirs: [], + capabilities: [], + harnesses: [], + inventoryStale: false, + maintenance: false, + maintenanceAt: null, + maintenanceSource: null, + sshEnabled: false, + held: null, + ...overrides, + }; +} + +const HOST_A = node({ id: "a1", name: "mac mini", sshEnabled: true }); +const HOST_B = node({ id: "b1", name: "studio", sshEnabled: false }); +const HOST_C = node({ id: "c1", name: "server room", sshEnabled: true }); + +const json = (body: unknown, status = 200) => new Response(JSON.stringify(body), { status }); + +interface WireOpts { + nodes?: Node[]; + ledger?: unknown; + aliases?: string[]; + launch?: () => Response; +} + +/** Mock the whole /connect surface and record every call with its body. */ +function mockFetch(opts: WireOpts = {}) { + const calls: Call[] = []; + const original = globalThis.fetch; + globalThis.fetch = ((input: unknown, init?: RequestInit) => { + const url = new URL(String(input), "http://localhost"); + const method = init?.method ?? "GET"; + const path = url.pathname; + calls.push({ + method, + url: path + url.search, + ...(typeof init?.body === "string" ? { body: JSON.parse(init.body) as unknown } : {}), + }); + if (path === "/api/nodes") return Promise.resolve(json({ nodes: opts.nodes ?? [] })); + if (path === "/api/ssh/saved-hosts" && method === "GET") { + return Promise.resolve(json(opts.ledger ?? { saved: [], recent: [], defaultNodeId: null })); + } + if (path === "/api/ssh/saved-hosts" && method === "PUT") { + return Promise.resolve( + json({ id: "r1", destination: "x", alias: null, nodeId: "a1", savedAt: "t", lastConnectAt: "t" }), + ); + } + if (path === "/api/ssh/preferences") return Promise.resolve(json({ defaultNodeId: "a1" })); + if (path === "/api/ssh/aliases") { + return Promise.resolve(json({ aliases: opts.aliases ?? [], includeCycle: false, truncated: false })); + } + if (path === "/api/ssh/launch") { + return Promise.resolve(opts.launch ? opts.launch() : json({ subshell: { id: "s-1" } }, 201)); + } + return Promise.resolve(json({})); + }) as typeof fetch; + return { calls, restore: () => (globalThis.fetch = original) }; +} + +interface Rendered { + calls: Call[]; + /** Current path: the navigation assertion reads the router state live. */ + pathname: () => string; + restore: () => void; +} + +async function renderPanel(opts: WireOpts): Promise { + const { calls, restore } = mockFetch(opts); + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + const rootRoute = createRootRoute(); + const indexRoute = createRoute({ getParentRoute: () => rootRoute, path: "/", component: ConnectPanel }); + const paneRoute = createRoute({ getParentRoute: () => rootRoute, path: "/subshells/$id", component: () => null }); + const router = createRouter({ + routeTree: rootRoute.addChildren([indexRoute, paneRoute]), + history: createMemoryHistory({ initialEntries: ["/"] }), + defaultPreload: false, + }); + await router.load(); + render( + + + , + ); + await settle(); + return { calls, pathname: () => router.state.location.pathname, restore }; +} + +/** Flush query/effect updates inside act() (the launch-form test idiom). */ +async function settle(): Promise { + await act(async () => { + await new Promise((r) => setTimeout(r, 50)); + }); +} + +/** Base UI opens its machine popup on the pointer path, not a bare click. */ +function openMachinePopup(): void { + const input = document.getElementById("connect-machine"); + if (!input) throw new Error("no machine field"); + act(() => { + for (const type of ["pointerdown", "pointerup"]) { + input.dispatchEvent(new PointerEvent(type, { bubbles: true, cancelable: true, pointerType: "mouse" })); + } + for (const type of ["mousedown", "mouseup", "click"]) { + input.dispatchEvent(new MouseEvent(type, { bubbles: true, cancelable: true })); + } + }); +} + +function machineInput(): HTMLInputElement { + return screen.getByPlaceholderText("Choose a machine") as HTMLInputElement; +} + +function destinationInput(): HTMLInputElement { + return screen.getByPlaceholderText("Choose or type a destination") as HTMLInputElement; +} + +/** The destination field opens on focus (the working-directory posture). */ +function focusDestination(): void { + fireEvent.focus(destinationInput()); +} + +/** Commit a typed host through its mirror row (the free-text path, §7). */ +async function commitTyped(text: string): Promise { + focusDestination(); + await settle(); + fireEvent.change(destinationInput(), { target: { value: text } }); + await settle(); + fireEvent.click(await screen.findByRole("button", { name: text })); + await settle(); +} + +async function clickConnect(): Promise { + fireEvent.click(screen.getByRole("button", { name: /^Connect$/ })); + await settle(); +} + +afterEach(cleanup); + +describe("machine disclosure (contract 1)", () => { + it("keeps a gated-off machine visible, greyed, with the reason naming what is in the way", async () => { + const { pathname, restore } = await renderPanel({ nodes: [HOST_A, HOST_B] }); + try { + // HOST_A is the sole SSH-enabled machine, so it pre-selects honestly - + // and HOST_B still stands in the list, disabled. + openMachinePopup(); + await settle(); + const studio = await screen.findByRole("option", { name: /studio/ }); + expect(studio.getAttribute("data-disabled")).toBe(""); + expect(studio.textContent).toContain("SSH is off on this machine"); + expect(pathname()).toBe("/"); + } finally { + restore(); + } + }); + + it("pre-selects the stored default machine", async () => { + const { restore } = await renderPanel({ + nodes: [HOST_A, HOST_C], + ledger: { saved: [], recent: [], defaultNodeId: "c1" }, + }); + try { + await waitFor(() => expect(machineInput().value).toBe("server room")); + } finally { + restore(); + } + }); + + it("pre-selects the single SSH-enabled machine with the preference unset", async () => { + const { restore } = await renderPanel({ nodes: [HOST_A, HOST_B] }); + try { + await waitFor(() => expect(machineInput().value).toBe("mac mini")); + } finally { + restore(); + } + }); + + it("several enabled machines and no preference is a required question: gold *, gold caption on the submit, no POST", async () => { + const { calls, restore } = await renderPanel({ nodes: [HOST_A, HOST_C] }); + try { + expect(machineInput().value).toBe(""); // never silently chosen + const label = document.querySelector('label[for="connect-machine"]'); + expect(label?.querySelector('[class*="after:content"]')).not.toBeNull(); // the gold star + await commitTyped("box.example"); + await clickConnect(); + const caption = screen.getByText("Choose a connecting machine first."); + expect(caption.className).toContain("text-amber-600"); // gold: nothing typed yet, not red + expect(calls.filter((c) => c.url === "/api/ssh/launch")).toEqual([]); + } finally { + restore(); + } + }); +}); + +describe("destination field (contract 2)", () => { + it("feeds the grouped rows: Saved, Recent, and the picked machine's config", async () => { + const ledger = { + saved: [ + { + id: "s1", + destination: "web01.example.com:22", + alias: "web", + nodeId: "a1", + savedAt: "2026-10-01T00:00:00.000Z", + lastConnectAt: "2026-10-05T00:00:00.000Z", + }, + ], + recent: [ + { + id: "r1", + destination: "db.example.com:22", + alias: null, + nodeId: "a1", + savedAt: null, + lastConnectAt: "2026-10-06T00:00:00.000Z", + }, + ], + defaultNodeId: "a1", + }; + const { restore } = await renderPanel({ nodes: [HOST_A], ledger, aliases: ["workbox"] }); + try { + await waitFor(() => expect(machineInput().value).toBe("mac mini")); + focusDestination(); + await settle(); + const popup = await waitFor(() => { + const el = document.getElementById("connect-destination-panel"); + if (!el) throw new Error("popup not open"); + return el; + }); + // Scoped to the popup: "Recent" is also the ledger's own heading below. + expect(within(popup).getByText("Saved")).toBeDefined(); + expect(within(popup).getByText("Recent")).toBeDefined(); + expect(within(popup).getByText("From mac mini's config")).toBeDefined(); + expect(within(popup).getByRole("button", { name: "web" })).toBeDefined(); + expect(within(popup).getByRole("button", { name: "workbox" })).toBeDefined(); + } finally { + restore(); + } + }); + + it("asks the machine for its aliases only once a gate-ON machine is picked", async () => { + const empty = await renderPanel({ nodes: [] }); + try { + // No machine at all: the picker asks nothing (a half-open panel must not + // ping a node with node= empty). + await settle(); + expect(empty.calls.filter((c) => c.url.startsWith("/api/ssh/aliases"))).toEqual([]); + } finally { + empty.restore(); + } + cleanup(); // one tree at a time: the second render must not stack queries + const picked = await renderPanel({ nodes: [HOST_A] }); + try { + await waitFor(() => expect(machineInput().value).toBe("mac mini")); + expect(picked.calls.some((c) => c.url === "/api/ssh/aliases?node=a1")).toBe(true); + } finally { + picked.restore(); + } + }); + + it("accepts a host typed that no list carries: the mirror row commits it", async () => { + const { calls, restore } = await renderPanel({ nodes: [HOST_A] }); + try { + await commitTyped("box.example"); + expect(destinationInput().value).toBe("box.example"); // the held echo is what will launch + await clickConnect(); + const post = calls.find((c) => c.url === "/api/ssh/launch"); + expect(post?.body).toEqual({ node: "a1", destination: "box.example" }); + } finally { + restore(); + } + }); + + it("a config-alias pick launches the token, and the save sends it as the alias", async () => { + const { calls, restore } = await renderPanel({ nodes: [HOST_A], aliases: ["workbox"] }); + try { + await waitFor(() => expect(machineInput().value).toBe("mac mini")); + focusDestination(); + await settle(); + fireEvent.click(await screen.findByRole("button", { name: "workbox" })); + await settle(); + fireEvent.click(screen.getByLabelText("Remember this destination")); + await clickConnect(); + expect(calls.find((c) => c.url === "/api/ssh/launch")?.body).toEqual({ node: "a1", destination: "workbox" }); + await waitFor(() => expect(calls.some((c) => c.method === "PUT" && c.url === "/api/ssh/saved-hosts")).toBe(true)); + const put = calls.find((c) => c.method === "PUT" && c.url === "/api/ssh/saved-hosts"); + expect(put?.body).toEqual({ node: "a1", destination: "workbox", alias: "workbox" }); + } finally { + restore(); + } + }); +}); + +describe("connect and the ledger (contracts 3, 6)", () => { + it("the button shows the process's own line while it runs and lands on the pane at 201", async () => { + let release!: () => void; + const gate = new Promise((r) => { + release = r; + }); + const realFetch = globalThis.fetch; + globalThis.fetch = ((input: unknown, _init?: RequestInit) => { + const url = new URL(String(input), "http://localhost"); + if (url.pathname === "/api/ssh/launch") return gate.then(() => json({ subshell: { id: "s-9" } }, 201)); + if (url.pathname === "/api/nodes") return Promise.resolve(json({ nodes: [HOST_A] })); + if (url.pathname === "/api/ssh/saved-hosts") + return Promise.resolve(json({ saved: [], recent: [], defaultNodeId: null })); + return Promise.resolve(json({})); + }) as typeof fetch; + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + const rootRoute = createRootRoute(); + const router = createRouter({ + routeTree: rootRoute.addChildren([ + createRoute({ getParentRoute: () => rootRoute, path: "/", component: ConnectPanel }), + createRoute({ getParentRoute: () => rootRoute, path: "/subshells/$id", component: () => null }), + ]), + history: createMemoryHistory({ initialEntries: ["/"] }), + defaultPreload: false, + }); + await router.load(); + render( + + + , + ); + await settle(); + try { + await commitTyped("box.example"); + fireEvent.click(screen.getByRole("button", { name: /^Connect$/ })); + await settle(); + const button = screen.getByRole("button", { name: /Connecting/ }); // no spinner: the button owns the wait + expect((button as HTMLButtonElement).disabled).toBe(true); + release(); + await waitFor(() => expect(router.state.location.pathname).toBe("/subshells/s-9")); + } finally { + globalThis.fetch = realFetch; + } + }); + + it("Remember starts unchecked; a checked launch PUTs the saved row with no alias, unchecked PUTs nothing", async () => { + const off = await renderPanel({ nodes: [HOST_A] }); + try { + expect((screen.getByLabelText("Remember this destination") as HTMLInputElement).checked).toBe(false); + await commitTyped("box.example"); + await clickConnect(); + await settle(); + expect(off.calls.filter((c) => c.method === "PUT" && c.url === "/api/ssh/saved-hosts")).toEqual([]); + } finally { + off.restore(); + } + cleanup(); // one tree at a time + const on = await renderPanel({ nodes: [HOST_A] }); + try { + await commitTyped("box.example"); + fireEvent.click(screen.getByLabelText("Remember this destination")); + await clickConnect(); + await waitFor(() => + expect(on.calls.some((c) => c.method === "PUT" && c.url === "/api/ssh/saved-hosts")).toBe(true), + ); + const put = on.calls.find((c) => c.method === "PUT" && c.url === "/api/ssh/saved-hosts"); + expect(put?.body).toEqual({ node: "a1", destination: "box.example" }); + } finally { + on.restore(); + } + }); + + it("Use by default PATCHes the connecting-machine preference", async () => { + const { calls, restore } = await renderPanel({ nodes: [HOST_A] }); + try { + await waitFor(() => expect(machineInput().value).toBe("mac mini")); + fireEvent.click(screen.getByRole("button", { name: "Use by default" })); + await waitFor(() => + expect(calls.some((c) => c.method === "PATCH" && c.url === "/api/ssh/preferences")).toBe(true), + ); + expect(calls.find((c) => c.method === "PATCH")?.body).toEqual({ defaultNodeId: "a1" }); + } finally { + restore(); + } + }); +}); + +describe("refusals and disclosure (contracts 4, 5)", () => { + it("renders a 422 outcome's blocked settings verbatim in red, past the message slice", async () => { + const settings = ["ProxyCommand", "LocalForward", ...Array.from({ length: 30 }, (_, i) => `Setting${i}`)]; + const raw = JSON.stringify({ outcome: { accepted: false, code: "unsupported_setting", settings } }); + expect(raw.length).toBeGreaterThan(200); // the 200-char message slice is exercised + const { restore } = await renderPanel({ + nodes: [HOST_A], + launch: () => json({ outcome: { accepted: false, code: "unsupported_setting", settings } }, 422), + }); + try { + await commitTyped("web01"); + await clickConnect(); + const line = await screen.findByText( + (_text, el) => + el?.tagName === "P" && (el.textContent ?? "").startsWith("Config needs settings Subshell does not run:"), + ); + expect(line.className).toContain("text-destructive"); // a hard server refusal: red + expect(line.textContent).toContain("Setting29"); // the WHOLE list, not the 200-char slice + expect(line.textContent).toContain("Edit them on the connecting machine and retry."); + } finally { + restore(); + } + }); + + it("names the machine in its own 409 sentence, under the machine field", async () => { + const { restore } = await renderPanel({ + nodes: [HOST_A], + launch: () => json({ errId: "e1", code: "NODE_OFFLINE", message: "offline", statusCode: 409 }, 409), + }); + try { + await commitTyped("web01"); + await clickConnect(); + expect( + await screen.findByText("mac mini has no live connection right now. Bring its Subshell app online and retry."), + ).toBeDefined(); + } finally { + restore(); + } + }); + + it("ships the two-sentence disclosure under the destination field, wired as its description", async () => { + const { restore } = await renderPanel({ nodes: [HOST_A] }); + try { + const line = await screen.findByText(SSH_DISCLOSURE_COPY); + expect(line.className).toContain("text-detail"); + expect(destinationInput().getAttribute("aria-describedby")).toBe("connect-destination-disclosure"); + // The exact sentences (decision 6), dash-free. + expect(SSH_DISCLOSURE_COPY).toBe( + "Resolving asks the connecting machine to read its SSH config. A hidden Match exec in that config can run a local command while it resolves.", + ); + } finally { + restore(); + } + }); +}); diff --git a/apps/server/web/src/components/connect/__tests__/destination-options.test.ts b/apps/server/web/src/components/connect/__tests__/destination-options.test.ts new file mode 100644 index 000000000..99a3ff626 --- /dev/null +++ b/apps/server/web/src/components/connect/__tests__/destination-options.test.ts @@ -0,0 +1,90 @@ +import { describe, expect, it } from "bun:test"; +import type { SshSavedHost } from "@/lib/ssh"; +import { buildDestinationOptions, configGroupLabel, RECENT_GROUP, SAVED_GROUP } from "../destination-options"; + +/** + * The destination field's list as pure data: the fed groups and their + * headers, the typed mirror row that makes the field accept a host no list + * holds, and the candidate map that keeps DISPLAY (an alias label) out of the + * WIRE (the destination token). The alias token rides only config rows - + * §7's alias-is-display rule. + */ +const row = (over: Partial): SshSavedHost => ({ + id: "r1", + destination: "web01.example.com:22", + alias: null, + nodeId: "n1", + savedAt: null, + lastConnectAt: "2026-10-07T00:00:00.000Z", + ...over, +}); + +describe("buildDestinationOptions", () => { + it("groups saved, recent and config rows under their headers, in order", () => { + const { options, candidates } = buildDestinationOptions({ + saved: [row({ id: "s1", alias: "web" })], + recent: [row({ id: "c1", destination: "db.example.com:22", alias: null })], + aliases: ["workbox"], + machineName: "mac mini", + typed: "", + }); + expect(options.map((o) => o.group)).toEqual([SAVED_GROUP, RECENT_GROUP, configGroupLabel("mac mini")]); + // A saved row LABELS with its alias and searches with its destination. + expect(options[0].label).toBe("web"); + expect(options[0].searchText).toBe("web01.example.com:22"); + expect(options[2].label).toBe("workbox"); + // The wire meanings: the alias label never leaks into the destination. + expect(candidates.get("saved:s1")).toEqual({ destination: "web01.example.com:22" }); + // Only the config row carries the alias token (the save sends it). + expect(candidates.get("config:workbox")).toEqual({ destination: "workbox", aliasToken: "workbox" }); + }); + + it("a recent row already saved reads once, under Saved", () => { + const shared = row({ id: "d1", savedAt: "2026-10-01T00:00:00.000Z" }); + const { options } = buildDestinationOptions({ + saved: [shared], + recent: [shared], + aliases: [], + machineName: null, + typed: "", + }); + expect(options).toHaveLength(1); + expect(options[0].group).toBe(SAVED_GROUP); + }); + + it("typed text with no equal row becomes its own candidate, leading the list", () => { + const { options, candidates } = buildDestinationOptions({ + saved: [row({})], + recent: [], + aliases: [], + machineName: null, + typed: "box.example", + }); + expect(options[0]).toEqual({ value: "typed:box.example", label: "box.example" }); + expect(candidates.get("typed:box.example")).toEqual({ destination: "box.example" }); + }); + + it("typed text a real row already carries gets no mirror row", () => { + const { options } = buildDestinationOptions({ + saved: [row({ id: "s1", destination: "box.example", alias: null })], + recent: [], + aliases: ["box.example"], + machineName: "mac mini", + typed: "box.example", + }); + expect(options.some((o) => o.value.startsWith("typed:"))).toBe(false); + // The real rows still carry it, so the choice exists once, not twice. + expect(options.some((o) => o.label === "box.example")).toBe(true); + }); + + it("no machine picked means no config group, and no aliases leak into it", () => { + const { options } = buildDestinationOptions({ + saved: [], + recent: [], + aliases: ["workbox"], + machineName: null, + typed: "", + }); + expect(options).toEqual([]); + }); +}); diff --git a/apps/server/web/src/components/connect/__tests__/saved-hosts-section.test.tsx b/apps/server/web/src/components/connect/__tests__/saved-hosts-section.test.tsx new file mode 100644 index 000000000..9cbc84dd5 --- /dev/null +++ b/apps/server/web/src/components/connect/__tests__/saved-hosts-section.test.tsx @@ -0,0 +1,137 @@ +import { afterEach, describe, expect, it } from "bun:test"; +import type { Node } from "@internal/node-admin"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { act, cleanup, fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { SavedHostsSection } from "@/components/connect/saved-hosts-section"; +import type { SshSavedHost } from "@/lib/ssh"; + +/** + * The ledger under the panel (contract 7): label-over-detail rows, delete on + * Remembered, remember on Recent, saved rows never shown twice, and the two + * one-sentence empties. Wire-mocked the way the panel test mocks it. + */ +const json = (body: unknown, status = 200) => new Response(JSON.stringify(body), { status }); + +const savedRow: SshSavedHost = { + id: "s1", + destination: "web01.example.com:22", + alias: "web", + nodeId: "a1", + savedAt: "2026-10-01T00:00:00.000Z", + lastConnectAt: "2026-10-05T00:00:00.000Z", +}; +const recentRow: SshSavedHost = { + id: "r1", + destination: "db.example.com:22", + alias: null, + nodeId: "a1", + savedAt: null, + lastConnectAt: "2026-10-06T00:00:00.000Z", +}; + +function node(overrides: Partial): Node { + return { + id: "n", + name: "node", + kind: "agent", + os: null, + arch: null, + hostname: null, + status: "online", + lastSeenAt: null, + agentVersion: null, + protocolVersion: null, + access: "owner", + canManage: true, + canLaunch: true, + allowedDirs: [], + capabilities: [], + harnesses: [], + inventoryStale: false, + maintenance: false, + maintenanceAt: null, + maintenanceSource: null, + sshEnabled: false, + held: null, + ...overrides, + }; +} + +interface Call { + method: string; + url: string; + body?: unknown; +} + +async function renderSection(ledger: unknown) { + const calls: Call[] = []; + const original = globalThis.fetch; + globalThis.fetch = ((input: unknown, init?: RequestInit) => { + const url = new URL(String(input), "http://localhost"); + const method = init?.method ?? "GET"; + calls.push({ + method, + url: url.pathname + url.search, + ...(typeof init?.body === "string" ? { body: JSON.parse(init.body) as unknown } : {}), + }); + if (url.pathname === "/api/ssh/saved-hosts" && method === "GET") return Promise.resolve(json(ledger)); + if (url.pathname === "/api/nodes") { + return Promise.resolve(json({ nodes: [node({ id: "a1", name: "mac mini", sshEnabled: true })] })); + } + return Promise.resolve(json({})); + }) as typeof fetch; + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + render( + + + , + ); + await act(async () => { + await new Promise((r) => setTimeout(r, 50)); + }); + return { calls, restore: () => (globalThis.fetch = original) }; +} + +afterEach(cleanup); + +describe("SavedHostsSection", () => { + it("renders remembered and recent rows label-over-detail, the machine named in the detail", async () => { + const { restore } = await renderSection({ saved: [savedRow], recent: [savedRow, recentRow], defaultNodeId: null }); + try { + expect(screen.getByText("web")).toBeDefined(); // the ALIAS is the display + expect(screen.getByText("mac mini · 2026-10-01")).toBeDefined(); // saved date rides remembered rows + expect(screen.getByText("db.example.com:22")).toBeDefined(); + expect(screen.getByText("mac mini · 2026-10-06")).toBeDefined(); // last connect rides recents + // The row already Saved appears once, under Remembered (reads once). + expect(screen.getAllByText("web")).toHaveLength(1); + } finally { + restore(); + } + }); + + it("a remembered row forgets by id; a recent row remembers on its own machine", async () => { + const { calls, restore } = await renderSection({ saved: [savedRow], recent: [recentRow], defaultNodeId: null }); + try { + fireEvent.click(screen.getByRole("button", { name: "Forget web" })); + await waitFor(() => expect(calls.some((c) => c.method === "DELETE")).toBe(true)); + expect(calls.find((c) => c.method === "DELETE")?.url).toBe("/api/ssh/saved-hosts/s1"); + fireEvent.click(screen.getByRole("button", { name: "Remember db.example.com:22" })); + await waitFor(() => expect(calls.some((c) => c.method === "PUT")).toBe(true)); + const put = calls.find((c) => c.method === "PUT"); + expect(put?.url).toBe("/api/ssh/saved-hosts"); + expect(put?.body).toEqual({ node: "a1", destination: "db.example.com:22" }); // the row's own machine, no alias + } finally { + restore(); + } + }); + + it("the two empty states are one short sentence each", async () => { + const { restore } = await renderSection({ saved: [], recent: [], defaultNodeId: null }); + try { + expect(screen.getByText("No remembered destinations yet.")).toBeDefined(); + expect(screen.getByText("No recent destinations yet.")).toBeDefined(); + } finally { + restore(); + } + }); +}); diff --git a/apps/server/web/src/components/connect/__tests__/ssh-refusal.test.ts b/apps/server/web/src/components/connect/__tests__/ssh-refusal.test.ts new file mode 100644 index 000000000..45e8b4997 --- /dev/null +++ b/apps/server/web/src/components/connect/__tests__/ssh-refusal.test.ts @@ -0,0 +1,136 @@ +import { describe, expect, it } from "bun:test"; +import { ApiError, NetworkError } from "@internal/node-admin"; +import { isSshResolveRefusal, type SshMachineFacts, sshLaunchRefusal } from "@/components/connect/ssh-refusal"; + +/** + * The refusal matrix of `POST /api/ssh/launch` (launch.route.ts's response + * map), each branch named (Task 3 addition 1). The 422 is the branch with + * two roads: a parsed `{outcome}` body renders the blocked settings verbatim + * even when the display message was sliced to 200 chars, and an old or + * body-less 422 falls back to the message text rather than inventing one. + */ +const machine = (over: Partial): SshMachineFacts => ({ + name: "mac mini", + kind: "agent", + sshEnabled: true, + ...over, +}); + +describe("sshLaunchRefusal", () => { + it("422 with a body renders every blocked setting verbatim, sliced message or not", () => { + const settings = ["ProxyCommand", "LocalForward", "RemoteForward"]; + const raw = JSON.stringify({ outcome: { accepted: false, code: "unsupported_setting", settings } }); + const err = new ApiError(422, raw.slice(0, 200), { body: JSON.parse(raw) }); + const copy = sshLaunchRefusal(err, machine({})); + expect(copy).toEqual({ + text: "Config needs settings Subshell does not run: ProxyCommand, LocalForward, RemoteForward. Edit them on the connecting machine and retry.", + field: "destination", + }); + }); + + it("422 whose code names the whole cause (no settings) says so without an empty list", () => { + const err = new ApiError(422, "", { body: { outcome: { accepted: false, code: "config_missing", settings: [] } } }); + const copy = sshLaunchRefusal(err, machine({})); + expect(copy?.text).toBe( + "Subshell cannot run this destination as its config stands. Edit that config on the connecting machine and retry.", + ); + expect(copy?.field).toBe("destination"); + }); + + it("422 with no parseable body falls back to the message text", () => { + const err = new ApiError(422, "refused"); // no body: an old server or a proxy page + expect(sshLaunchRefusal(err, machine({}))).toEqual({ text: "API 422: refused", field: "destination" }); + }); + + it("400 names the destination as invalid, not the machine", () => { + expect(sshLaunchRefusal(new ApiError(400, "x", { code: "ALIAS_UNSAFE" }), machine({}))).toEqual({ + text: "That destination is not a valid host name.", + field: "destination", + }); + }); + + it("403 SSH_GATE_OFF on a gated-off agent names the owner remedy", () => { + expect(sshLaunchRefusal(new ApiError(403, "x", { code: "SSH_GATE_OFF" }), machine({ sshEnabled: false }))).toEqual({ + text: "SSH is off on this machine. Ask its owner to enable it.", + field: "machine", + }); + }); + + it("403 SSH_GATE_OFF on the control-plane host names the admin remedy", () => { + const local = machine({ kind: "local", name: "this host", sshEnabled: false }); + expect(sshLaunchRefusal(new ApiError(403, "x", { code: "SSH_GATE_OFF" }), local)).toEqual({ + text: "An admin can enable SSH on this machine in its settings.", + field: "machine", + }); + }); + + it("403 with the gate ON is the owner-door: no share reaches SSH", () => { + expect(sshLaunchRefusal(new ApiError(403, "x", { code: "SSH_GATE_OFF" }), machine({}))).toEqual({ + text: "You can't connect from this machine.", + field: "machine", + }); + }); + + it("404 says the machine isn't available", () => { + expect(sshLaunchRefusal(new ApiError(404, "x", { code: "NOT_FOUND_ERROR" }), machine({}))).toEqual({ + text: "That machine isn't available.", + field: "machine", + }); + }); + + it("409 arms name the machine and their own cause", () => { + expect(sshLaunchRefusal(new ApiError(409, "x", { code: "NODE_OFFLINE" }), machine({}))?.text).toBe( + "mac mini has no live connection right now. Bring its Subshell app online and retry.", + ); + expect(sshLaunchRefusal(new ApiError(409, "x", { code: "NODE_PROTOCOL_HELD" }), machine({}))?.text).toBe( + "mac mini runs a Subshell version this server does not speak. Update it from its machine page and retry.", + ); + expect(sshLaunchRefusal(new ApiError(409, "x", { code: "NODE_OUTDATED" }), machine({}))?.text).toBe( + "The Subshell app on mac mini is too old to speak SSH. Update it from its machine page.", + ); + expect(sshLaunchRefusal(new ApiError(409, "x", { code: "NODE_UNREACHABLE" }), machine({}))?.text).toBe( + "mac mini did not answer the SSH request in time. Check its connection and retry.", + ); + // No machine facts (the row vanished) keeps the sentence grammatical. + expect(sshLaunchRefusal(new ApiError(409, "x", { code: "NODE_OFFLINE" }), null)?.text).toBe( + "That machine has no live connection right now. Bring its Subshell app online and retry.", + ); + }); + + it("502 is the machine's own refusal, in one sentence", () => { + expect(sshLaunchRefusal(new ApiError(502, "x", { code: "SSH_NODE_REFUSED" }), machine({}))).toEqual({ + text: "The connecting machine did not answer.", + field: "machine", + }); + }); + + it("a NetworkError renders nothing: the global offline banner already says it", () => { + expect(sshLaunchRefusal(new NetworkError(new TypeError("down")), machine({}))).toBeNull(); + }); + + it("an unexpected failure gets the generic red line, never an API string", () => { + expect(sshLaunchRefusal(new Error("boom"), machine({}))?.text).toBe("The connection could not be started."); + expect(sshLaunchRefusal(new ApiError(500, "Internal"), machine({}))?.text).toBe( + "The connection could not be started.", + ); + }); + + it("every shipped sentence is at most two sentences and dash-free", () => { + const texts = [422, 400, 403, 404, 409, 502].flatMap((status) => { + const copy = sshLaunchRefusal(new ApiError(status, "x", { code: "ANY" }), machine({ sshEnabled: false })); + return copy === null ? [] : [copy.text]; + }); + for (const text of texts) { + expect(text).not.toMatch(/[─—–]/); // U+2500, U+2014, U+2013 + expect(text.split(/[.!?] /).length).toBeLessThanOrEqual(2); + } + }); + + it("isSshResolveRefusal narrows the refused arm only", () => { + expect(isSshResolveRefusal({ accepted: false, code: "config_missing", settings: [] })).toBe(true); + expect(isSshResolveRefusal({ accepted: true, snapshot: {} })).toBe(false); + expect(isSshResolveRefusal({ accepted: false, code: "x" })).toBe(false); // no settings array + expect(isSshResolveRefusal(null)).toBe(false); + expect(isSshResolveRefusal("outcome")).toBe(false); + }); +}); diff --git a/apps/server/web/src/components/connect/connect-panel.tsx b/apps/server/web/src/components/connect/connect-panel.tsx new file mode 100644 index 000000000..dc6c03d57 --- /dev/null +++ b/apps/server/web/src/components/connect/connect-panel.tsx @@ -0,0 +1,252 @@ +import type { Node } from "@internal/node-admin"; +import { Button, Label } from "@internal/node-admin"; +import { useNavigate } from "@tanstack/react-router"; +import { type JSX, useEffect, useState } from "react"; +import { DestinationField } from "@/components/connect/destination-field"; +import { buildDestinationOptions, type DestinationCandidate } from "@/components/connect/destination-options"; +import { SavedHostsSection } from "@/components/connect/saved-hosts-section"; +import { type SshRefusalCopy, sshLaunchRefusal } from "@/components/connect/ssh-refusal"; +import { type ComboboxOption, SearchableSelect } from "@/components/ui/combobox"; +import { RequiredMark } from "@/components/ui/required-mark"; +import { useNodes } from "@/hooks/use-nodes"; +import { useLaunchSsh, useSaveSshHost, useSetDefaultNode, useSshAliases, useSshSavedHosts } from "@/hooks/use-ssh"; +import { fieldErrorToned } from "@/lib/form"; +import { nodeOptionLabel } from "@/lib/node-label"; +import { REQUIREMENT_CAPTION_CLASS } from "@/lib/requirement-tone"; +import { launchableNodes } from "@/lib/subshell-compat"; + +/** + * The destination-first Connect flow (spec 2026-10-07 §7, decision 2): the + * destination leads, the connecting machine follows as a disclosure line, + * and one POST opens the pane - the SERVER resolves, so selecting a row is + * display, never a pre-resolve step. Three nouns only (the §11 vocabulary): + * destination, connecting machine. No key-source affordance (that is M2). + * + * A machine whose SSH is off stays VISIBLE and disabled with its reason - + * greying explains, hiding does not (decision 3). With no default machine and + * several SSH-enabled ones the disclosure is required with the gold `*` and + * never silently chosen; with a preference or exactly one usable machine it + * pre-selects honestly and stays on screen. The disclosure sentence (the + * `Match exec` note, decision 6) ships verbatim under the destination field, + * before the button, because approving a destination approves running its + * resolution. + * + * The destination is one COMMITTED candidate: a picked row's meaning, or + * nothing. Typed text becomes a candidate through the mirror row the list + * offers (free text accepted, §7), and the commitment releases under any + * later edit, so the field's text and the launch's truth cannot drift. + * The field is the working-directory field's input-plus-panel posture + * (`destination-field.tsx`), not the shared combobox: the list must swap on + * every keystroke, which is exactly what Base UI's held input cannot keep. + * Refusals render on the field they belong to, in red, from + * `ssh-refusal.ts`; the 422's settings list comes from the parsed body, + * because the display message is sliced. Remembering is explicit: every + * launch is already recent server-side; the checkbox PUTs the saved row only + * when checked, sending the alias only for a pick from the machine's own + * config list (§7's alias-is-display rule). + */ +const DESTINATION_IDS = { field: "connect-destination", disclosure: "connect-destination-disclosure" }; +const MACHINE_IDS = { field: "connect-machine", requirement: "connect-machine-requirement" }; + +/** The disclosure every ssh surface owes before the button (decision 6, verbatim). */ +export const SSH_DISCLOSURE_COPY = + "Resolving asks the connecting machine to read its SSH config. A hidden Match exec in that config can run a local command while it resolves."; + +/** The destination field's committed state, held as ONE value so a pick and + * the text echo that follows it cannot interleave into a stale pair. */ +interface DestinationState { + /** The picked row's wire meaning; null = nothing committed yet */ + pick: DestinationCandidate | null; + /** The picked row's option id (the picker's held value) */ + pickId: string | null; + /** The input's live text (feeds the typed mirror row; not a commitment) */ + typed: string; +} + +export function ConnectPanel(): JSX.Element { + const navigate = useNavigate(); + const { data: nodeData } = useNodes(); + const nodes = Array.isArray(nodeData?.nodes) ? nodeData.nodes : null; + const { data: ledger } = useSshSavedHosts(); + const launch = useLaunchSsh(); + const save = useSaveSshHost(); + const setDefault = useSetDefaultNode(); + + const [nodeId, setNodeId] = useState(""); + const [dest, setDest] = useState({ pick: null, pickId: null, typed: "" }); + const [remember, setRemember] = useState(false); + // The gold caption waits for a submit that found the machine untouched: + // "nothing typed yet", never before the first attempt (ruling 2026-09-30). + const [submitAttempted, setSubmitAttempted] = useState(false); + const [refusal, setRefusal] = useState(null); + + const targets = launchableNodes(nodes ?? []); + const selectedNode: Node | null = (nodes ?? []).find((n) => n.id === nodeId) ?? null; + const defaultNodeId = ledger?.defaultNodeId ?? null; + const enabledCount = targets.filter((n) => n.sshEnabled).length; + // No preference, several doors: the choice is the human's (spec §7). + const machineRequired = nodeId === "" && defaultNodeId === null && enabledCount > 1; + + // The machine disclosure keeps gated-off rows visible (decision 3): grey, + // with the reason naming what is in the way. + const machineOptions: ComboboxOption[] = targets.map((n) => ({ + value: n.id, + label: nodeOptionLabel(n), + disabled: !n.sshEnabled, + reason: n.sshEnabled ? undefined : "SSH is off on this machine", + })); + + // Aliases are the MACHINE's config, asked only while a gate-ON machine is + // picked (spec §7: discovery runs where it can answer). + const aliasesQ = useSshAliases(selectedNode?.sshEnabled ? selectedNode.id : null); + const aliases = aliasesQ.data?.aliases ?? []; + + const { options: destinationOptions, candidates } = buildDestinationOptions({ + saved: ledger?.saved ?? [], + recent: ledger?.recent ?? [], + aliases, + machineName: selectedNode?.name ?? null, + typed: dest.typed, + }); + + // The default pre-fill, once and only over an untouched pick: the stored + // preference wins when its machine is still a usable row; an only child + // pre-selects honestly; several with no preference stay an open question + // (decision 2). Keyed on the loaded node list, never on `targets` (a new + // array every render). + useEffect(() => { + if (nodeId !== "" || nodes === null) return; + const usable = launchableNodes(nodes).filter((n) => n.sshEnabled); + if (defaultNodeId !== null && usable.some((n) => n.id === defaultNodeId)) { + setNodeId(defaultNodeId); + } else if (usable.length === 1) { + setNodeId(usable[0].id); + } + }, [nodeId, nodes, defaultNodeId]); + + const destination = dest.pick?.destination ?? ""; + // A committed row's label is the field's text; while that echo stands the + // list is UNFILTERED (the whole ledger again), because the echo is display, + // not a search. Any keystroke releases the pick, and text becomes query. + const pickLabel = dest.pickId !== null ? destinationOptions.find((o) => o.value === dest.pickId)?.label : undefined; + const destinationQuery = dest.pick !== null && dest.typed === pickLabel ? "" : dest.typed; + + // Typing is only typing here (the field reports keystrokes, never echoes), + // so an edit always releases the commitment: what the field shows is what + // the launch would carry, never a stale pick under new text. + function onDestinationText(text: string): void { + setDest({ pick: null, pickId: null, typed: text }); + } + + /** Clicking (or Enter on) a row commits its candidate; the label becomes + * the field's text, which is what a re-opened list then echoes. */ + function pickDestinationOption(option: ComboboxOption): void { + const candidate = candidates.get(option.value); + if (candidate === undefined) return; + setDest({ pick: candidate, pickId: option.value, typed: option.label }); + } + + async function submit(): Promise { + setSubmitAttempted(true); + if (nodeId === "" || destination === "") return; + setRefusal(null); + try { + const created = await launch.mutateAsync({ node: nodeId, destination }); + if (remember) { + // The alias rides ONLY for a config-list pick (an alias token); saved + // and recent rows already carry their own. + save.mutate({ + node: nodeId, + destination, + ...(dest.pick?.aliasToken !== undefined ? { alias: dest.pick.aliasToken } : {}), + }); + } + void navigate({ to: "/subshells/$id", params: { id: created.subshell.id } }); + } catch (err) { + setRefusal(sshLaunchRefusal(err, selectedNode)); + } + } + + const machineGap = + submitAttempted && machineRequired + ? fieldErrorToned([{ message: "Choose a connecting machine first.", gap: true }]) + : null; + + return ( +
+
+
+ + + {refusal?.field === "destination" && ( +

+ {refusal.text} +

+ )} +

+ {SSH_DISCLOSURE_COPY} +

+
+ +
+ + id !== "" && setNodeId(id)} + /> + {machineGap && ( +

+ {machineGap.text} +

+ )} + {refusal?.field === "machine" && ( +

+ {refusal.text} +

+ )} + {selectedNode !== null && defaultNodeId !== selectedNode.id && ( + + )} +
+ +
+ setRemember(e.target.checked)} + className="h-4 w-4 rounded border border-input bg-background accent-primary" + /> + +
+ + +
+ + +
+ ); +} diff --git a/apps/server/web/src/components/connect/destination-field.tsx b/apps/server/web/src/components/connect/destination-field.tsx new file mode 100644 index 000000000..c25e24be2 --- /dev/null +++ b/apps/server/web/src/components/connect/destination-field.tsx @@ -0,0 +1,139 @@ +import { Input } from "@internal/node-admin"; +import type { JSX } from "react"; +import { useEffect, useRef, useState } from "react"; +import type { ComboboxOption } from "@/components/ui/combobox"; + +/** + * The destination field: a real input, because the answer may be a host no + * list carries, with an in-flow row panel that opens on focus. The posture + * is the working-directory field's (`directory-picker-input.tsx`), copied + * deliberately: a controlled input, a panel that follows its text, and an + * outside press or Escape as the dismissal. `SearchableSelect` is NOT used + * here on purpose - Base UI's collection reset wipes a held input's typed + * text when its items swap mid-typing (measured and documented in + * `ui/combobox.tsx`), and this field's items must swap on EVERY keystroke: + * the typed mirror row is the list growing to meet what was typed. + * + * The list is the three fed groups (`destination-options.ts`): Saved, + * Recent, and the picked machine's config aliases, each under its header, + * plus the mirror row. Clicking (or pressing Enter on the top row) COMMITS + * a row - the commitment is the candidate the launch carries, never the raw + * echo - and typing over a committed row releases it, so the field's text + * and the panel's truth cannot drift apart. + */ +export function DestinationField({ + id, + describedBy, + placeholder, + typed, + query, + options, + onTextChange, + onPick, +}: { + /** Input id - the Label's htmlFor and the test/e2e anchor */ + id: string; + /** Id of the element explaining this field (the disclosure sentence) */ + describedBy?: string; + placeholder: string; + /** The input's text: typed free text, or the committed row's label */ + typed: string; + /** What filters the list; "" while a committed row's echo stands */ + query: string; + /** The full fed list (groups + mirror row); the field only filters it */ + options: readonly ComboboxOption[]; + onTextChange: (text: string) => void; + /** Commits one row (a click, or Enter on the top visible row) */ + onPick: (option: ComboboxOption) => void; +}): JSX.Element { + const [open, setOpen] = useState(false); + const rootRef = useRef(null); + + // Dismissal by anything outside the field, or Escape - the directory + // picker's rule, and the listeners live with `open` so an idle field costs + // nothing. + useEffect(() => { + if (!open) return; + function onPointerDown(e: PointerEvent) { + if (!rootRef.current?.contains(e.target as Node)) setOpen(false); + } + function onKeyDown(e: KeyboardEvent) { + if (e.key === "Escape") setOpen(false); + } + document.addEventListener("pointerdown", onPointerDown); + document.addEventListener("keydown", onKeyDown); + return () => { + document.removeEventListener("pointerdown", onPointerDown); + document.removeEventListener("keydown", onKeyDown); + }; + }, [open]); + + // The same filter the shared combobox applies, so the picker and every + // other list in the product answer a query identically: label or searchText, + // case-insensitive contains, trimmed. + const needle = query.trim().toLowerCase(); + const visible = + needle === "" + ? [...options] + : options.filter( + (o) => o.label.toLowerCase().includes(needle) || (o.searchText ?? "").toLowerCase().includes(needle), + ); + + function pick(option: ComboboxOption): void { + onPick(option); + setOpen(false); + } + + let lastGroup: string | undefined; + return ( +
+ onTextChange(e.target.value)} + onFocus={() => setOpen(true)} + placeholder={placeholder} + aria-describedby={describedBy} + onKeyDown={(e) => { + if (e.key === "Enter" && visible.length > 0) { + e.preventDefault(); // committing the top row is the act; it must not submit the page + pick(visible[0]); + } + }} + /> + {open && visible.length > 0 && ( + // The id lets callers (and tests) scope to the popup: a group header + // like "Recent" names the same word the ledger heading below uses, + // and only the popup's copy is the picker's. +
+ {visible.map((option) => { + const header = option.group !== undefined && option.group !== lastGroup; + lastGroup = option.group; + return ( +
+ {header &&
{option.group}
} + +
+ ); + })} +
+ )} + {open && visible.length === 0 &&

No matches

} +
+ ); +} diff --git a/apps/server/web/src/components/connect/destination-options.ts b/apps/server/web/src/components/connect/destination-options.ts new file mode 100644 index 000000000..4e0b63a31 --- /dev/null +++ b/apps/server/web/src/components/connect/destination-options.ts @@ -0,0 +1,98 @@ +import type { ComboboxOption } from "@/components/ui/combobox"; +import type { SshSavedHost } from "@/lib/ssh"; + +/** + * The destination field's list, as pure data (the picker itself is the + * panel's). Three fed groups per spec 2026-10-07 §7: the caller's saved rows, + * their recent rows, and the picked machine's own config aliases - plus the + * one row that makes the field accept free text: a row mirroring whatever is + * typed. Picking is the commitment; the candidate map says what each row + * MEANS on the wire, so display aliases never leak into the launch body. + */ + +/** What choosing one row commits the launch to. */ +export interface DestinationCandidate { + /** The token sent as `destination` (an alias from the config, or a concrete host) */ + destination: string; + /** + * The alias display token, set ONLY for rows that came from the machine's + * config list - the save sends it as the row's display override (§7's + * alias-is-display rule). Saved and recent rows already own an alias. + */ + aliasToken?: string; +} + +/** The typed mirror row's group: none. It leads the list unlabelled. */ +const TYPED_PREFIX = "typed:"; +const SAVED_PREFIX = "saved:"; +const RECENT_PREFIX = "recent:"; +const CONFIG_PREFIX = "config:"; + +/** The group headers, one definition so the panel and its tests agree. */ +export const SAVED_GROUP = "Saved"; +export const RECENT_GROUP = "Recent"; +/** `From mac mini's config` - the aliases belong to the machine, and the row says so. */ +export const configGroupLabel = (machineName: string): string => `From ${machineName}'s config`; + +/** The destination field's full list plus the per-row candidate map. */ +export function buildDestinationOptions(args: { + /** The caller's saved rows (newest first, as the server sends them) */ + saved: readonly SshSavedHost[]; + /** The caller's recent rows; rows already Saved are dropped from Recent */ + recent: readonly SshSavedHost[]; + /** The picked machine's alias names, fetched only while its gate is on */ + aliases: readonly string[]; + /** The picked machine's display name, for the config group header */ + machineName: string | null; + /** The field's current typed text - the mirror row's source */ + typed: string; +}): { options: ComboboxOption[]; candidates: Map } { + const options: ComboboxOption[] = []; + const candidates = new Map(); + + const push = (id: string, option: ComboboxOption, candidate: DestinationCandidate): void => { + options.push(option); + candidates.set(id, candidate); + }; + + // The free-text mirror first: whatever is typed is offered as a row the + // click commits. Suppressed when a real row already carries the same + // token - the row would be a duplicate of a choice already on screen. + const text = args.typed.trim(); + const covered = (token: string): boolean => + args.saved.some((h) => h.destination === token || h.alias === token) || + args.recent.some((h) => h.destination === token || h.alias === token) || + args.aliases.some((a) => a === token); + if (text !== "" && !covered(text)) { + push(`${TYPED_PREFIX}${text}`, { value: `${TYPED_PREFIX}${text}`, label: text }, { destination: text }); + } + + const savedIds = new Set(args.saved.map((h) => h.id)); + for (const row of args.saved) { + const label = row.alias ?? row.destination; + push( + `${SAVED_PREFIX}${row.id}`, + { value: `${SAVED_PREFIX}${row.id}`, label, group: SAVED_GROUP, searchText: row.destination }, + { destination: row.destination }, + ); + } + for (const row of args.recent) { + if (savedIds.has(row.id)) continue; // the destination reads once (the favorites precedent) + const label = row.alias ?? row.destination; + push( + `${RECENT_PREFIX}${row.id}`, + { value: `${RECENT_PREFIX}${row.id}`, label, group: RECENT_GROUP, searchText: row.destination }, + { destination: row.destination }, + ); + } + if (args.machineName !== null) { + for (const token of args.aliases) { + push( + `${CONFIG_PREFIX}${token}`, + { value: `${CONFIG_PREFIX}${token}`, label: token, group: configGroupLabel(args.machineName) }, + { destination: token, aliasToken: token }, + ); + } + } + return { options, candidates }; +} diff --git a/apps/server/web/src/components/connect/saved-hosts-section.tsx b/apps/server/web/src/components/connect/saved-hosts-section.tsx new file mode 100644 index 000000000..fd8bc7584 --- /dev/null +++ b/apps/server/web/src/components/connect/saved-hosts-section.tsx @@ -0,0 +1,111 @@ +import { Star, Trash2 } from "lucide-react"; +import type { JSX } from "react"; +import { useNodes } from "@/hooks/use-nodes"; +import { useDeleteSshHost, useSaveSshHost, useSshSavedHosts } from "@/hooks/use-ssh"; +import type { SshSavedHost } from "@/lib/ssh"; + +/** + * The destination ledger under the Connect panel (spec 2026-10-07 §7): what + * the caller remembered, and what they last connected to. Two lists, one + * shape: `label` over `detail`, differing by weight and colour - the label is + * the display alias or the destination, the detail names the connecting + * machine and the day. Remembered rows carry the way OUT (delete); recent + * rows carry the way IN (remember). Recent rows already Saved read once: the + * remembered copy wins, nothing duplicates. + * + * The save is a plain PUT keyed to the row's own machine - the server + * resolves first and stores the canonical destination, so a rename of the + * alias later cannot re-point the key. The delete is by row id. No confirms: + * a ledger row is cheap to remake, and the ACT that matters is the launch. + */ +export function SavedHostsSection(): JSX.Element { + const { data: ledger } = useSshSavedHosts(); + const { data: nodeData } = useNodes(); + const save = useSaveSshHost(); + const remove = useDeleteSshHost(); + + const saved = ledger?.saved ?? []; + const savedIds = new Set(saved.map((h) => h.id)); + const recent = (ledger?.recent ?? []).filter((h) => !savedIds.has(h.id)); + const nameById = new Map( + (Array.isArray(nodeData?.nodes) ? nodeData.nodes : []).map((n) => [n.id, n.name]), + ); + const machineName = (row: SshSavedHost): string => nameById.get(row.nodeId) ?? row.nodeId; + + return ( +
+
+

Remembered

+ {saved.length === 0 ? ( +

No remembered destinations yet.

+ ) : ( +
    + {saved.map((row) => ( + remove.mutate(row.id), + }} + /> + ))} +
+ )} +
+
+

Recent

+ {recent.length === 0 ? ( +

No recent destinations yet.

+ ) : ( +
    + {recent.map((row) => ( + save.mutate({ node: row.nodeId, destination: row.destination }), + }} + /> + ))} +
+ )} +
+
+ ); +} + +/** One ledger line: label over detail, with the row's single act on the right. */ +function LedgerRow({ + row, + detail, + action, +}: { + row: SshSavedHost; + detail: string; + action: { label: string; icon: typeof Star; onSelect: () => void }; +}): JSX.Element { + const Icon = action.icon; + return ( +
  • +
    +
    {row.alias ?? row.destination}
    +
    {detail}
    +
    + +
    • + ); +} diff --git a/apps/server/web/src/components/connect/ssh-refusal.ts b/apps/server/web/src/components/connect/ssh-refusal.ts new file mode 100644 index 000000000..8785bae63 --- /dev/null +++ b/apps/server/web/src/components/connect/ssh-refusal.ts @@ -0,0 +1,122 @@ +import type { Node } from "@internal/node-admin"; +import { ApiError, isNetworkError } from "@internal/node-admin"; +import type { SshResolveOutcome } from "@/lib/ssh"; + +/** + * The launch refusal, in the client's words. `POST /api/ssh/launch` answers + * every no with a status and a code the panel can name (the response map in + * `apps/server/api/src/api/ssh/launch.route.ts`); this file is the ONE place + * that maps them to copy, one sentence pair per branch, so the panel renders + * a cause and a remedy rather than an `API 409:` string. + * + * The 422 is the special case: its body is not the error envelope but the + * resolve outcome itself (`{outcome}`), and `ApiError.message` slices the + * display text to 200 chars, so the settings list is read from `err.body` + * (the node-admin un-truncation). An older or truncated body falls back to + * the message text; nothing renders a half sentence from a missing field. + */ + +/** The refused arm of the resolve outcome, narrowed from `unknown`. */ +export function isSshResolveRefusal(value: unknown): value is Extract { + if (typeof value !== "object" || value === null) return false; + const v = value as Record; + return v.accepted === false && typeof v.code === "string" && Array.isArray(v.settings); +} + +/** Which field the refusal belongs under: the token typed, or the machine chosen. */ +export type RefusalField = "destination" | "machine"; + +/** The red line the panel renders, and where. */ +export interface SshRefusalCopy { + text: string; + field: RefusalField; +} + +/** What the panel knows about the connecting machine at submit time. */ +export type SshMachineFacts = Pick; + +/** The settings-bearing sentence names each blocked keyword verbatim; the + * code-only arms (empty settings) name the fact and the place to fix it. */ +function refusalFromOutcome(outcome: Extract): string { + return outcome.settings.length > 0 + ? `Config needs settings Subshell does not run: ${outcome.settings.join(", ")}. Edit them on the connecting machine and retry.` + : "Subshell cannot run this destination as its config stands. Edit that config on the connecting machine and retry."; +} + +/** + * Maps a caught launch failure to copy. A `NetworkError` returns null: the + * app-wide offline banner already says it, and restating "server unreachable" + * under the form would be the same sentence twice. + * @param err - the value from the launch mutation's catch + * @param machine - the picked node's facts (null when the row is unknown) + */ +export function sshLaunchRefusal(err: unknown, machine: SshMachineFacts | null): SshRefusalCopy | null { + if (isNetworkError(err)) return null; + if (err instanceof ApiError) { + switch (err.status) { + case 400: + // ALIAS_UNSAFE: the token was refused before the machine was asked. + return { text: "That destination is not a valid host name.", field: "destination" }; + case 403: { + if (err.code === "SSH_GATE_OFF" && machine !== null && !machine.sshEnabled) { + // The gate's off-door. Naming WHO flips it follows the row's kind: + // `local` is an admin's setting, every other machine its owner's + // (spec 2026-10-07 decision 3). + return { + text: + machine.kind === "local" + ? "An admin can enable SSH on this machine in its settings." + : "SSH is off on this machine. Ask its owner to enable it.", + field: "machine", + }; + } + // The owner-door (the code is shared) or any other 403: the machine + // is open to someone else, and a share is not a remedy here. + return { text: "You can't connect from this machine.", field: "machine" }; + } + case 404: + return { text: "That machine isn't available.", field: "machine" }; + case 409: { + const name = machine?.name ?? "That machine"; + switch (err.code) { + case "NODE_PROTOCOL_HELD": + return { + text: `${name} runs a Subshell version this server does not speak. Update it from its machine page and retry.`, + field: "machine", + }; + case "NODE_OUTDATED": + return { + text: `The Subshell app on ${name} is too old to speak SSH. Update it from its machine page.`, + field: "machine", + }; + case "NODE_UNREACHABLE": + return { + text: `${name} did not answer the SSH request in time. Check its connection and retry.`, + field: "machine", + }; + default: + // NODE_OFFLINE and any other 409: no live link to carry the ask. + return { + text: `${name} has no live connection right now. Bring its Subshell app online and retry.`, + field: "machine", + }; + } + } + case 422: { + // The resolve refused: read the outcome from the parsed body, because + // the message is a 200-char slice of it. + const outcome = + typeof err.body === "object" && err.body !== null ? (err.body as { outcome?: unknown }).outcome : undefined; + if (isSshResolveRefusal(outcome)) return { text: refusalFromOutcome(outcome), field: "destination" }; + return { text: err.message, field: "destination" }; + } + case 502: + // The machine answered its own refusal (no ssh binary, the mirror + // says no); its verbatim text never rides the wire to here. + return { text: "The connecting machine did not answer.", field: "machine" }; + default: + break; + } + } + return { text: "The connection could not be started.", field: "machine" }; +} diff --git a/apps/server/web/src/routeTree.gen.ts b/apps/server/web/src/routeTree.gen.ts index d7ae6c85e..288ff1d1f 100644 --- a/apps/server/web/src/routeTree.gen.ts +++ b/apps/server/web/src/routeTree.gen.ts @@ -11,6 +11,7 @@ import { Route as rootRouteImport } from './routes/__root' import { Route as IndexRouteImport } from './routes/index' import { Route as AccountRouteImport } from './routes/account' +import { Route as ConnectRouteImport } from './routes/connect' import { Route as LoginRouteImport } from './routes/login' import { Route as NewRouteImport } from './routes/new' import { Route as NodesRouteImport } from './routes/nodes' @@ -48,6 +49,11 @@ const AccountRoute = AccountRouteImport.update({ path: '/account', getParentRoute: () => rootRouteImport, } as any) +const ConnectRoute = ConnectRouteImport.update({ + id: '/connect', + path: '/connect', + getParentRoute: () => rootRouteImport, +} as any) const LoginRoute = LoginRouteImport.update({ id: '/login', path: '/login', @@ -182,6 +188,7 @@ const NodesIdServiceRoute = NodesIdServiceRouteImport.update({ export interface FileRoutesByFullPath { '/': typeof IndexRoute '/account': typeof AccountRoute + '/connect': typeof ConnectRoute '/login': typeof LoginRoute '/new': typeof NewRoute '/nodes': typeof NodesRoute @@ -212,6 +219,7 @@ export interface FileRoutesByFullPath { export interface FileRoutesByTo { '/': typeof IndexRoute '/account': typeof AccountRoute + '/connect': typeof ConnectRoute '/login': typeof LoginRoute '/new': typeof NewRoute '/nodes': typeof NodesRoute @@ -243,6 +251,7 @@ export interface FileRoutesById { __root__: typeof rootRouteImport '/': typeof IndexRoute '/account': typeof AccountRoute + '/connect': typeof ConnectRoute '/login': typeof LoginRoute '/new': typeof NewRoute '/nodes': typeof NodesRoute @@ -275,6 +284,7 @@ export interface FileRouteTypes { fullPaths: | '/' | '/account' + | '/connect' | '/login' | '/new' | '/nodes' @@ -305,6 +315,7 @@ export interface FileRouteTypes { to: | '/' | '/account' + | '/connect' | '/login' | '/new' | '/nodes' @@ -335,6 +346,7 @@ export interface FileRouteTypes { | '__root__' | '/' | '/account' + | '/connect' | '/login' | '/new' | '/nodes' @@ -366,6 +378,7 @@ export interface FileRouteTypes { export interface RootRouteChildren { IndexRoute: typeof IndexRoute AccountRoute: typeof AccountRoute + ConnectRoute: typeof ConnectRoute LoginRoute: typeof LoginRoute NewRoute: typeof NewRoute NodesRoute: typeof NodesRoute @@ -410,6 +423,13 @@ declare module '@tanstack/react-router' { preLoaderRoute: typeof AccountRouteImport parentRoute: typeof rootRouteImport } + '/connect': { + id: '/connect' + path: '/connect' + fullPath: '/connect' + preLoaderRoute: typeof ConnectRouteImport + parentRoute: typeof rootRouteImport + } '/login': { id: '/login' path: '/login' @@ -598,6 +618,7 @@ declare module '@tanstack/react-router' { const rootRouteChildren: RootRouteChildren = { IndexRoute: IndexRoute, AccountRoute: AccountRoute, + ConnectRoute: ConnectRoute, LoginRoute: LoginRoute, NewRoute: NewRoute, NodesRoute: NodesRoute, diff --git a/apps/server/web/src/routes/__tests__/connect.test.tsx b/apps/server/web/src/routes/__tests__/connect.test.tsx new file mode 100644 index 000000000..164a85bd9 --- /dev/null +++ b/apps/server/web/src/routes/__tests__/connect.test.tsx @@ -0,0 +1,58 @@ +import { afterEach, expect, it } from "bun:test"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { + createMemoryHistory, + createRootRoute, + createRoute, + createRouter, + RouterProvider, +} from "@tanstack/react-router"; +import { act, cleanup, render, screen } from "@testing-library/react"; +import { ConnectPage, Route } from "../connect"; + +/** + * Contract 8: the route exists at /connect and opens with a real page + * heading. The panel itself is pinned by `components/connect/`; the nav + * entry by the sidebar tests; the e2e spec lands here end-to-end. + */ +afterEach(cleanup); + +it("declares the route with a component (the path binds in the generated tree)", () => { + // createFileRoute attaches `path`/`id` when the router builds the tree from + // routeTree.gen.ts; the loadable unit here is the options object, and the + // e2e spec lands on the URL. + expect(typeof Route.options.component).toBe("function"); +}); + +it("opens on the Connect heading over the panel", async () => { + const original = globalThis.fetch; + globalThis.fetch = (async (input: unknown) => { + const path = new URL(String(input), "http://localhost").pathname; + const body = path === "/api/nodes" ? { nodes: [] } : { saved: [], recent: [], defaultNodeId: null }; + return new Response(JSON.stringify(body), { status: 200 }); + }) as typeof fetch; + try { + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + const rootRoute = createRootRoute(); + const router = createRouter({ + routeTree: rootRoute.addChildren([ + createRoute({ getParentRoute: () => rootRoute, path: "/connect", component: ConnectPage }), + ]), + history: createMemoryHistory({ initialEntries: ["/connect"] }), + defaultPreload: false, + }); + await router.load(); + render( + + + , + ); + await act(async () => { + await new Promise((r) => setTimeout(r, 50)); + }); + expect(await screen.findByRole("heading", { name: "Connect" })).toBeDefined(); + expect(screen.getByPlaceholderText("Choose or type a destination")).toBeDefined(); + } finally { + globalThis.fetch = original; + } +}); diff --git a/apps/server/web/src/routes/connect.tsx b/apps/server/web/src/routes/connect.tsx new file mode 100644 index 000000000..5f520131a --- /dev/null +++ b/apps/server/web/src/routes/connect.tsx @@ -0,0 +1,24 @@ +import { createFileRoute } from "@tanstack/react-router"; +import { ConnectPanel } from "@/components/connect/connect-panel"; +import { PageHeader } from "@/components/page-header"; + +/** + * The SSH launcher (spec 2026-10-07 §7): the destination-first page the top- + * level Connect nav entry lands on. The route carries no search params - the + * panel is one act, not a view over data; the ledger lives in the panel's + * own queries. All the flow lives in `components/connect/`. + */ +export const Route = createFileRoute("/connect")({ + component: ConnectPage, +}); + +// Exported for the route test (the composition pin); the router reads it +// through `Route`, no other caller should. +export function ConnectPage() { + return ( +
    + + +
    + ); +} From d45df991ee92f7125231524566471abf651905ec Mon Sep 17 00:00:00 2001 From: Theo Gravity Date: Wed, 7 Oct 2026 23:57:52 -0700 Subject: [PATCH 06/16] =?UTF-8?q?fix(web):=20reviewer=20round=201=20-=20th?= =?UTF-8?q?e=20machine=20field's=20explanation=20lines=20are=20now=20assoc?= =?UTF-8?q?iated,=20the=20save=20hook's=20render=20claim=20matches=20reali?= =?UTF-8?q?ty,=20spec=20=C2=A711=20records=20the=20combobox=20free-text=20?= =?UTF-8?q?contradiction?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/components/connect/connect-panel.tsx | 20 +++++++++++++++---- apps/server/web/src/hooks/use-ssh.ts | 7 +++++-- .../specs/2026-10-07-ssh-anywhere-design.md | 8 ++++++++ 3 files changed, 29 insertions(+), 6 deletions(-) diff --git a/apps/server/web/src/components/connect/connect-panel.tsx b/apps/server/web/src/components/connect/connect-panel.tsx index dc6c03d57..3248d0df8 100644 --- a/apps/server/web/src/components/connect/connect-panel.tsx +++ b/apps/server/web/src/components/connect/connect-panel.tsx @@ -46,7 +46,11 @@ import { launchableNodes } from "@/lib/subshell-compat"; * config list (§7's alias-is-display rule). */ const DESTINATION_IDS = { field: "connect-destination", disclosure: "connect-destination-disclosure" }; -const MACHINE_IDS = { field: "connect-machine", requirement: "connect-machine-requirement" }; +const MACHINE_IDS = { + field: "connect-machine", + requirement: "connect-machine-requirement", + refusal: "connect-machine-refusal", +}; /** The disclosure every ssh surface owes before the button (decision 6, verbatim). */ export const SSH_DISCLOSURE_COPY = @@ -171,6 +175,13 @@ export function ConnectPanel(): JSX.Element { submitAttempted && machineRequired ? fieldErrorToned([{ message: "Choose a connecting machine first.", gap: true }]) : null; + // aria-describedby may name nothing that is not rendered, so the + // association is exactly the explanation lines currently on screen. + const machineRefusal = refusal?.field === "machine" ? refusal.text : null; + const machineDescribedBy = + [machineGap ? MACHINE_IDS.requirement : null, machineRefusal !== null ? MACHINE_IDS.refusal : null] + .filter((id): id is string => id !== null) + .join(" ") || undefined; return (
    @@ -208,15 +219,16 @@ export function ConnectPanel(): JSX.Element { placeholder="Choose a machine" options={machineOptions} onValueChange={(id) => id !== "" && setNodeId(id)} + describedBy={machineDescribedBy} /> {machineGap && (

    {machineGap.text}

    )} - {refusal?.field === "machine" && ( -

    - {refusal.text} + {machineRefusal !== null && ( +

    )} {selectedNode !== null && defaultNodeId !== selectedNode.id && ( diff --git a/apps/server/web/src/hooks/use-ssh.ts b/apps/server/web/src/hooks/use-ssh.ts index c688cae8b..3bd151114 100644 --- a/apps/server/web/src/hooks/use-ssh.ts +++ b/apps/server/web/src/hooks/use-ssh.ts @@ -54,8 +54,11 @@ function useInvalidateSavedHosts(): () => Promise { /** * `PUT /api/ssh/saved-hosts` — resolves the destination first, so a - * refusal-shaped outcome answers 422 `{outcome}` and stores nothing; the - * panel renders that, an `ApiError` with status 422. + * refusal-shaped outcome answers 422 `{outcome}` (an `ApiError` with status + * 422) and stores nothing. Both callers treat it best-effort: the connect + * panel's Remember act and the Recent-star act do not render that refusal, + * because a row already exists from the launch's server-side recency touch, + * so a failed save costs only the `savedAt` pin, never the destination. */ export function useSaveSshHost() { const invalidate = useInvalidateSavedHosts(); diff --git a/docs/superpowers/specs/2026-10-07-ssh-anywhere-design.md b/docs/superpowers/specs/2026-10-07-ssh-anywhere-design.md index 2ac355107..aff870a4e 100644 --- a/docs/superpowers/specs/2026-10-07-ssh-anywhere-design.md +++ b/docs/superpowers/specs/2026-10-07-ssh-anywhere-design.md @@ -377,6 +377,14 @@ nouns: destination, connecting machine, key source. The destination field is a `SearchableSelect` like the other launch pickers. Shipped copy follows the design system: role tokens only, at most two sentences, no em dashes. +(Amendment, built. "SearchableSelect like the other launch pickers" holds for the +connecting-machine picker, which is one. For the destination it contradicts "Pick or +type a host" above: the pinned Base UI combobox (measured 2026-09-29, re-verified at +1.7.0) wipes mid-typing input when its option set swaps and cannot commit a value +outside its registered options, so a typed host would vanish - the silent-omission +defect in the control itself. The destination ships as the working-directory field's +controlled input-plus-panel posture: same role, same search, free text commitable.) + ## 12. Failure modes and UX - A offline in relay mode: refuse before the handshake, name the remedy (bring the From db782422d3cc6212d475e87a77257f88a7a0c68d Mon Sep 17 00:00:00 2001 From: Theo Gravity Date: Thu, 8 Oct 2026 00:01:21 -0700 Subject: [PATCH 07/16] feat(web): ssh panes label themselves and tell viewers why typing is off --- .../__tests__/clone-subshell-dialog.test.tsx | 1 + .../__tests__/existing-subshell-list.test.tsx | 1 + .../__tests__/open-in-browser.test.tsx | 1 + .../__tests__/pane-diagnostics-hud.test.tsx | 1 + .../__tests__/ssh-pane-labels.test.tsx | 55 +++++++++++++++++++ .../__tests__/subshell-actions-menu.test.tsx | 1 + .../__tests__/subshell-card.test.tsx | 1 + .../__tests__/subshell-manager-table.test.tsx | 1 + .../__tests__/subshell-status.test.tsx | 1 + .../__tests__/switch-preset-dialog.test.tsx | 1 + .../__tests__/terminal-overlay-stack.test.tsx | 1 + .../__tests__/workspace-pane-actions.test.tsx | 1 + .../__tests__/subshell-recent-row.test.tsx | 1 + .../web/src/components/ssh-pane-labels.tsx | 27 +++++++++ .../__tests__/add-subshell-dialog.test.tsx | 1 + apps/server/web/src/routes/subshells_.$id.tsx | 6 ++ apps/server/web/src/types/subshell.ts | 2 + 17 files changed, 103 insertions(+) create mode 100644 apps/server/web/src/components/__tests__/ssh-pane-labels.test.tsx create mode 100644 apps/server/web/src/components/ssh-pane-labels.tsx diff --git a/apps/server/web/src/components/__tests__/clone-subshell-dialog.test.tsx b/apps/server/web/src/components/__tests__/clone-subshell-dialog.test.tsx index d5cf02ed6..198af03b6 100644 --- a/apps/server/web/src/components/__tests__/clone-subshell-dialog.test.tsx +++ b/apps/server/web/src/components/__tests__/clone-subshell-dialog.test.tsx @@ -37,6 +37,7 @@ function makeSource(overrides: Partial = {}): SubshellView { waitingSince: null, unseenPush: false, access: "owner", + ssh: false, ...overrides, }; } diff --git a/apps/server/web/src/components/__tests__/existing-subshell-list.test.tsx b/apps/server/web/src/components/__tests__/existing-subshell-list.test.tsx index 65a2b3c78..58731f77b 100644 --- a/apps/server/web/src/components/__tests__/existing-subshell-list.test.tsx +++ b/apps/server/web/src/components/__tests__/existing-subshell-list.test.tsx @@ -27,6 +27,7 @@ function makeSubshell(overrides: Partial = {}): SubshellView { waitingSince: null, unseenPush: false, access: "owner", + ssh: false, nodeOffline: false, ...overrides, }; diff --git a/apps/server/web/src/components/__tests__/open-in-browser.test.tsx b/apps/server/web/src/components/__tests__/open-in-browser.test.tsx index 70fa74908..e09567bb0 100644 --- a/apps/server/web/src/components/__tests__/open-in-browser.test.tsx +++ b/apps/server/web/src/components/__tests__/open-in-browser.test.tsx @@ -155,6 +155,7 @@ function makeSubshell(overrides: Partial = {}): SubshellView { waitingSince: null, unseenPush: false, access: "owner", + ssh: false, ...overrides, }; } diff --git a/apps/server/web/src/components/__tests__/pane-diagnostics-hud.test.tsx b/apps/server/web/src/components/__tests__/pane-diagnostics-hud.test.tsx index 52eba59a8..f4b59b40c 100644 --- a/apps/server/web/src/components/__tests__/pane-diagnostics-hud.test.tsx +++ b/apps/server/web/src/components/__tests__/pane-diagnostics-hud.test.tsx @@ -32,6 +32,7 @@ function makeSubshell(overrides: Partial = {}): SubshellView { waitingSince: null, unseenPush: false, access: "owner", + ssh: false, ...overrides, }; } diff --git a/apps/server/web/src/components/__tests__/ssh-pane-labels.test.tsx b/apps/server/web/src/components/__tests__/ssh-pane-labels.test.tsx new file mode 100644 index 000000000..cf721faaf --- /dev/null +++ b/apps/server/web/src/components/__tests__/ssh-pane-labels.test.tsx @@ -0,0 +1,55 @@ +import { afterEach, describe, expect, it } from "bun:test"; +import { cleanup, render, screen } from "@testing-library/react"; +import { SSH_VIEW_ONLY_COPY, SshPaneLabels } from "@/components/ssh-pane-labels"; +import type { SubshellView } from "@/types/subshell"; + +afterEach(() => cleanup()); + +/** The fields this header row reads; the rest of the view is irrelevant here. */ +function row(overrides: Partial = {}): SubshellView { + return { id: "s-1", ssh: false, access: "owner", ...overrides } as SubshellView; +} + +describe("SshPaneLabels", () => { + it("labels an owner's ssh pane with the SSH badge and says nothing more", () => { + render(); + expect(screen.queryByText("SSH")).not.toBeNull(); + expect(screen.queryByText(SSH_VIEW_ONLY_COPY)).toBeNull(); + }); + + it("labels an edit grantee's ssh pane with the badge only (the sentence is the viewer's)", () => { + render(); + expect(screen.queryByText("SSH")).not.toBeNull(); + expect(screen.queryByText(SSH_VIEW_ONLY_COPY)).toBeNull(); + }); + + it("tells a view-only ssh viewer why typing is off", () => { + render(); + expect(screen.queryByText("SSH")).not.toBeNull(); + expect(screen.queryByText("SSH sessions take input from their owner only.")).not.toBeNull(); + // One sentence pinned exactly (copy rule), and no em/en dash in it. + expect(SSH_VIEW_ONLY_COPY).toBe("SSH sessions take input from their owner only."); + expect(SSH_VIEW_ONLY_COPY).not.toMatch(/[–—]/); + }); + + it("stays silent for an ordinary pane at any access (no regression for view grantees)", () => { + const view = render(); + expect(view.container.textContent).toBe(""); + cleanup(); + const owner = render(); + expect(owner.container.textContent).toBe(""); + }); + + it("renders nothing while the record has not arrived", () => { + const { container } = render(); + expect(container.textContent).toBe(""); + }); + + it("treats an absent ssh field as not-ssh (an older cached payload)", () => { + // A payload cached before the field existed has no `ssh` key at all; + // the component's test is `=== true`, so it must read as not-ssh. + const cached = { id: "s-1", access: "view" } as SubshellView; + const { container } = render(); + expect(container.textContent).toBe(""); + }); +}); diff --git a/apps/server/web/src/components/__tests__/subshell-actions-menu.test.tsx b/apps/server/web/src/components/__tests__/subshell-actions-menu.test.tsx index c4ed25f43..8e7702805 100644 --- a/apps/server/web/src/components/__tests__/subshell-actions-menu.test.tsx +++ b/apps/server/web/src/components/__tests__/subshell-actions-menu.test.tsx @@ -38,6 +38,7 @@ function makeSubshell(overrides: Partial = {}): SubshellView { waitingSince: null, unseenPush: false, access: "owner", + ssh: false, ...overrides, }; } diff --git a/apps/server/web/src/components/__tests__/subshell-card.test.tsx b/apps/server/web/src/components/__tests__/subshell-card.test.tsx index 65e223466..15e7da1fc 100644 --- a/apps/server/web/src/components/__tests__/subshell-card.test.tsx +++ b/apps/server/web/src/components/__tests__/subshell-card.test.tsx @@ -77,6 +77,7 @@ function makeSubshell(overrides: Partial = {}): SubshellView { waitingSince: null, unseenPush: false, access: "owner", + ssh: false, ...overrides, }; } diff --git a/apps/server/web/src/components/__tests__/subshell-manager-table.test.tsx b/apps/server/web/src/components/__tests__/subshell-manager-table.test.tsx index 1a1b02817..7539d6946 100644 --- a/apps/server/web/src/components/__tests__/subshell-manager-table.test.tsx +++ b/apps/server/web/src/components/__tests__/subshell-manager-table.test.tsx @@ -40,6 +40,7 @@ function makeSubshell(overrides: Partial = {}): SubshellView { waitingSince: null, unseenPush: false, access: "owner", + ssh: false, ...overrides, }; } diff --git a/apps/server/web/src/components/__tests__/subshell-status.test.tsx b/apps/server/web/src/components/__tests__/subshell-status.test.tsx index e313ffe4a..853ac6503 100644 --- a/apps/server/web/src/components/__tests__/subshell-status.test.tsx +++ b/apps/server/web/src/components/__tests__/subshell-status.test.tsx @@ -27,6 +27,7 @@ function makeSubshell(overrides: Partial = {}): SubshellView { waitingSince: null, unseenPush: false, access: "owner", + ssh: false, nodeOffline: false, ...overrides, }; diff --git a/apps/server/web/src/components/__tests__/switch-preset-dialog.test.tsx b/apps/server/web/src/components/__tests__/switch-preset-dialog.test.tsx index cb9da72ab..b959a2347 100644 --- a/apps/server/web/src/components/__tests__/switch-preset-dialog.test.tsx +++ b/apps/server/web/src/components/__tests__/switch-preset-dialog.test.tsx @@ -30,6 +30,7 @@ function makeSubshell(overrides: Partial = {}): SubshellView { waitingSince: null, unseenPush: false, access: "owner", + ssh: false, ...overrides, }; } diff --git a/apps/server/web/src/components/__tests__/terminal-overlay-stack.test.tsx b/apps/server/web/src/components/__tests__/terminal-overlay-stack.test.tsx index 72bddcbae..d1442898f 100644 --- a/apps/server/web/src/components/__tests__/terminal-overlay-stack.test.tsx +++ b/apps/server/web/src/components/__tests__/terminal-overlay-stack.test.tsx @@ -32,6 +32,7 @@ function makeSubshell(overrides: Partial = {}): SubshellView { waitingSince: null, unseenPush: false, access: "owner", + ssh: false, ...overrides, }; } diff --git a/apps/server/web/src/components/__tests__/workspace-pane-actions.test.tsx b/apps/server/web/src/components/__tests__/workspace-pane-actions.test.tsx index 35c60191a..afad745b9 100644 --- a/apps/server/web/src/components/__tests__/workspace-pane-actions.test.tsx +++ b/apps/server/web/src/components/__tests__/workspace-pane-actions.test.tsx @@ -32,6 +32,7 @@ function row(id: string, access: SubshellView["access"] = "owner"): SubshellView status: "running", alive: true, access, + ssh: false, nodeOffline: false, notify: false, activity: "idle", diff --git a/apps/server/web/src/components/sidebar/__tests__/subshell-recent-row.test.tsx b/apps/server/web/src/components/sidebar/__tests__/subshell-recent-row.test.tsx index b25d3da24..f446b554c 100644 --- a/apps/server/web/src/components/sidebar/__tests__/subshell-recent-row.test.tsx +++ b/apps/server/web/src/components/sidebar/__tests__/subshell-recent-row.test.tsx @@ -51,6 +51,7 @@ function makeSubshell(overrides: Partial = {}): SubshellView { waitingSince: null, unseenPush: false, access: "owner", + ssh: false, ...overrides, }; } diff --git a/apps/server/web/src/components/ssh-pane-labels.tsx b/apps/server/web/src/components/ssh-pane-labels.tsx new file mode 100644 index 000000000..fc0f795ef --- /dev/null +++ b/apps/server/web/src/components/ssh-pane-labels.tsx @@ -0,0 +1,27 @@ +import type { SubshellView } from "@/types/subshell"; + +/** The header sentence a non-owner of an ssh pane sees (spec 2026-10-07 §5.4). */ +export const SSH_VIEW_ONLY_COPY = "SSH sessions take input from their owner only."; + +/** + * The ssh facts a pane header states beside its other labels: an SSH badge + * naming what kind of pane this is, and — only for a viewer who is not the + * owner — the one sentence explaining why the terminal takes no input from + * them. Enforcement is NOT here: the view layer already answers a non-owner + * with `access: "view"` for an ssh row (spec §5.4), and the terminal reads + * exactly that (`readOnly`, subshell-terminal.tsx). An ordinary pane renders + * nothing, including for a `view` grantee: the general view rule is not + * restated per-surface. + * + * `=== true` on the flag is deliberate: a payload cached before the field + * existed has no `ssh` key and must read as an ordinary pane. + */ +export function SshPaneLabels({ subshell }: { subshell?: SubshellView }) { + if (subshell?.ssh !== true) return null; + return ( + <> + SSH + {subshell.access === "view" && {SSH_VIEW_ONLY_COPY}} + + ); +} diff --git a/apps/server/web/src/components/subshell-picker/__tests__/add-subshell-dialog.test.tsx b/apps/server/web/src/components/subshell-picker/__tests__/add-subshell-dialog.test.tsx index 8430eecf3..233eaf045 100644 --- a/apps/server/web/src/components/subshell-picker/__tests__/add-subshell-dialog.test.tsx +++ b/apps/server/web/src/components/subshell-picker/__tests__/add-subshell-dialog.test.tsx @@ -28,6 +28,7 @@ function makeSubshell(overrides: Partial & { id: string }): Subshe waitingSince: null, unseenPush: false, access: "owner", + ssh: false, nodeOffline: false, ...overrides, }; diff --git a/apps/server/web/src/routes/subshells_.$id.tsx b/apps/server/web/src/routes/subshells_.$id.tsx index 8d814ead7..e71f6c9f8 100644 --- a/apps/server/web/src/routes/subshells_.$id.tsx +++ b/apps/server/web/src/routes/subshells_.$id.tsx @@ -11,6 +11,7 @@ import { EditableText } from "@/components/editable-text"; import { SubshellNotFoundCard } from "@/components/not-found-page"; import { InjectPromptDialog } from "@/components/prompts/inject-prompt-dialog"; import { SplitSubshellButton } from "@/components/split-subshell-button"; +import { SshPaneLabels } from "@/components/ssh-pane-labels"; import { StatusPill } from "@/components/status-pill"; import { SubshellActionsMenu } from "@/components/subshell-actions-menu"; import { SubshellDevices } from "@/components/subshell-devices"; @@ -310,6 +311,11 @@ function SubshellPage() { about the pane stated where its own name is, worded the way the rail's section labels it. */} {subshell?.crossAgent && Cross-agent comms} + {/* What kind of pane this is, and for a non-owner why typing is + off (spec 2026-10-07 §5.4). The badge is the label; the + enforcement is the server's access downgrade the terminal + already honours. */} + {/* Permanent disclosure: whose machine this runs on, and who else can read it. Never suppressible — see components/trust-indicators.tsx. */} diff --git a/apps/server/web/src/types/subshell.ts b/apps/server/web/src/types/subshell.ts index 24cc48c50..c391d1394 100644 --- a/apps/server/web/src/types/subshell.ts +++ b/apps/server/web/src/types/subshell.ts @@ -85,6 +85,8 @@ export interface SubshellView { unseenPush: boolean; /** The caller's effective access to this subshell (viewer-relative; drives which controls render) */ access: SubshellAccess; + /** True when this pane is an SSH-terminal pane: only its owner can type into it. */ + ssh: boolean; /** * How many sharing grants this subshell carries; 0 = private to its owner. * Optional for the same reason `nodeId` is — a payload cached by a client From 8da407e2118bcdaf73225345fd937f1815fd12d5 Mon Sep 17 00:00:00 2001 From: Theo Gravity Date: Thu, 8 Oct 2026 00:09:07 -0700 Subject: [PATCH 08/16] test(e2e): connect flow end to end - destination-first launch and the view-only pane --- e2e/AGENTS.md | 8 +- e2e/ports.ts | 6 + e2e/tests/23-connect-flow.spec.ts | 480 ++++++++++++++++++++++++++++++ 3 files changed, 492 insertions(+), 2 deletions(-) create mode 100644 e2e/tests/23-connect-flow.spec.ts diff --git a/e2e/AGENTS.md b/e2e/AGENTS.md index 5889afbf0..d4b8e7faf 100644 --- a/e2e/AGENTS.md +++ b/e2e/AGENTS.md @@ -51,13 +51,17 @@ runs discovery against this fake), and no spec ever dials a real IdP. Every fixed port in `ports.ts` takes an env override (`E2E_PORT_BACKEND`, `E2E_PORT_FAKE_REGISTRY`, `E2E_PORT_FAKE_IDP`, `E2E_PORT_ONBOARDING`, -`E2E_PORT_SSH`) with the committed defaults unchanged, the escape hatch for -two suites running side by side on one machine, which the defaults collide on. +`E2E_PORT_SSH`, `E2E_PORT_CONNECT`) with the committed defaults unchanged, the +escape hatch for two suites running side by side on one machine, which the +defaults collide on. Spec `22` boots its own backend too (port 3201) for a reason the shared stack cannot serve: the ssh tier reads the connecting account's HOME and bun caches `os.homedir()` per process, so that child is SPAWNED with `HOME=` plus the spec's `ssh-agent` socket, on its own temp DB/data dir/tmux base. +Spec `23` repeats that boot on port 3202 (its own fresh slot, since spec 22's +instance lives across its retries) and drives the same fixture through the +browser: the `/connect` page, the gate flip, and a view-only ssh pane. Spec `12` extends the stack itself: it spawns the **real `subshell` from source** (`bun apps/node/agent/src/main.ts enroll|run` via `stub/client.ts`, with diff --git a/e2e/ports.ts b/e2e/ports.ts index 07c7451f3..fc44d77da 100644 --- a/e2e/ports.ts +++ b/e2e/ports.ts @@ -35,6 +35,11 @@ * carry for the other specs. Its own temp DB, its own data dir (whose * `ssh/` dir the spec asserts on through the filesystem), and its own * tmux base. + * + * `connect` belongs to spec 23, the browser twin of `ssh`: the same HOME- + * override reason, and a FRESH slot rather than a reuse because spec 22 keeps + * its instance alive across its own retries — two ssh backends boot in one + * run and must never collide on the same port. */ const envPort = (name: string, fallback: number): number => { const raw = process.env[name]; @@ -52,6 +57,7 @@ export const PORTS = { fakeIdp: envPort("E2E_PORT_FAKE_IDP", 3197), onboarding: envPort("E2E_PORT_ONBOARDING", 3200), ssh: envPort("E2E_PORT_SSH", 3201), + connect: envPort("E2E_PORT_CONNECT", 3202), } as const; export const BASE_URL = `http://127.0.0.1:${PORTS.backend}`; diff --git a/e2e/tests/23-connect-flow.spec.ts b/e2e/tests/23-connect-flow.spec.ts new file mode 100644 index 000000000..23cd9fb8a --- /dev/null +++ b/e2e/tests/23-connect-flow.spec.ts @@ -0,0 +1,480 @@ +import { spawn, spawnSync } from "node:child_process"; +import { mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { type APIRequestContext, expect, type Page, type PlaywrightWorkerArgs, test } from "@playwright/test"; +import { missingSshBins, SSH_FIXTURE_ALIAS, type SshFixture, startSshFixture } from "../fixtures/sshd"; +import { PORTS } from "../ports"; +import { shortTmuxBase } from "../stack"; +import { expectSubshellRunning } from "./helpers"; + +/** + * Plan 3's browser proof (spec 2026-10-07 §7, plan Task 5): the destination- + * first /connect page driven in a REAL browser against a REAL sshd. Four + * scenarios, in order, each its own test with its own page context: + * + * 1. gate OFF (the fresh instance's default): the machine picker keeps the + * control-plane host VISIBLE and DISABLED with the reason "SSH is off on + * this machine" - greying explains, hiding does not (decision 3). The + * disabled row is never clicked; the field staying empty IS the inertness + * assertion. + * 2. the two-sentence Match exec disclosure, verbatim (decision 6), above the + * Connect button BEFORE any Connect is attempted - the box-order check + * makes "before the button" a literal assertion. + * 3. gate ON (a REST flip on the admin's own cookie session, then a reload): + * the sole SSH-enabled machine pre-selects honestly, the destination + * picker's config group carries the fixture alias, Connect POSTs once and + * the router lands on /subshells/; the typed line comes back EVALUATED + * by the remote shell (only the shell's arithmetic spells CONNECT-7-OK), + * read through the pane-log route - spec 22's proof, browser-driven. + * 4. an EVERYONE-edit sharee opening the SAME pane sees the SSH badge and the + * view-only sentence, and their typing leaves nothing in the log (the + * client posture; the 403 itself is tier 2's, pinned in spec 22). + * + * Why its own backend: exactly spec 22's reason - the ssh tier resolves as the + * server process's HOME and bun caches `os.homedir()` per process, so this + * child is SPAWNED with `HOME=` + the spec's ssh-agent socket, + * on its own temp DB, data dir and tmux base (port: PORTS.connect, a fresh + * slot so the two ssh specs' instances can never collide). + * + * The control-plane host is the seeded `local` node, whose picker NAME is + * "Server" (seed-local.ts) - the assertions read that name because the page + * reads node rows, not ids. + * + * Binaries: ssh / ssh-keygen / sshd, decided by the fixture's own gate; a host + * without them SKIPS LOUDLY and never fakes a pass. + */ + +const ROOT = path.join(import.meta.dirname, "..", ".."); +const BACKEND_DIR = path.join(ROOT, "apps", "server", "api"); +const ORIGIN = `http://127.0.0.1:${PORTS.connect}`; + +const READY_TIMEOUT = 60_000; +const SPAWN_TIMEOUT = 90_000; + +const missing = missingSshBins(); +test.skip(missing.length > 0, `SSH binaries absent on this host (${missing.join(", ")}) - spec 23 needs a real sshd`); + +/** The exact strings the page ships (components/connect/* + components/ssh-pane-labels.tsx). */ +const SSH_DISCLOSURE_COPY = + "Resolving asks the connecting machine to read its SSH config. A hidden Match exec in that config can run a local command while it resolves."; +const SSH_VIEW_ONLY_COPY = "SSH sessions take input from their owner only."; +const MACHINE_OFF_REASON = "SSH is off on this machine"; +/** The seeded control-plane host's display name (seed-local.ts): the picker reads names, not ids. */ +const LOCAL_NODE_NAME = "Server"; +/** `configGroupLabel(selectedNode.name)` for that row. */ +const CONFIG_GROUP = `From ${LOCAL_NODE_NAME}'s config`; + +const sleep = (ms: number): Promise => new Promise((r) => setTimeout(r, ms)); + +/** Polls `check` until true or the deadline; on timeout throws the label's current text. */ +async function pollUntil( + label: string | (() => string), + check: () => Promise | boolean, + timeoutMs = SPAWN_TIMEOUT, +): Promise { + const deadline = Date.now() + timeoutMs; + for (;;) { + if (await check()) return; + if (Date.now() > deadline) throw new Error(typeof label === "string" ? label : label()); + await sleep(500); + } +} + +interface ConnectInstance { + child: ReturnType; + dir: string; + tmuxBase: string; +} + +/** Boots the spec's OWN backend on `PORTS.connect` with HOME + SSH_AUTH_SOCK in ITS env (spec 22's recipe). */ +async function startConnectBackend(fixtureHome: string, sshAuthSock: string): Promise { + // Same orphan guard as specs 15/22: a dead retry's detached backend would + // hold :3202 for the next boot. + stopConnectBackend(); + const dir = mkdtempSync(path.join(tmpdir(), "subshell-e2e-connect-")); + const dataDir = path.join(dir, "data"); + const tmuxBase = shortTmuxBase(); + mkdirSync(tmuxBase, { recursive: true }); + + const env: Record = { + ...process.env, + NODE_ENV: "development", + SUBSHELL_TEST_MODE: "false", + SUBSHELL_SERVER_CONFIG_DIR: dir, + SERVER_PORT: String(PORTS.connect), + HOST: "127.0.0.1", + DATABASE_PATH: path.join(dir, "subshell.db"), + SUBSHELL_SERVER_DATA_DIR: dataDir, + APP_BASE_URL: ORIGIN, + BETTER_AUTH_SECRET: "e2e-secret-not-used-outside-tests-0000000000", + SUBSHELL_PLUGIN_REGISTRY_URL: `http://127.0.0.1:${PORTS.fakeRegistry}`, + SUBSHELL_RELEASE_URL: "", + TMUX_TMPDIR: tmuxBase, + // The two the ssh tier reads (spec 22's header): HOME is the alias/config + // source for discovery AND for `ssh -G`; SSH_AUTH_SOCK is the resolve's + // agent-socket fact. + HOME: fixtureHome, + SSH_AUTH_SOCK: sshAuthSock, + }; + // A developer inside tmux must not hand the backend their session's pane. + delete env.TMUX; + delete env.TMUX_PANE; + + const child = spawn("bun", ["run", "src/index.ts"], { + cwd: BACKEND_DIR, + detached: true, + stdio: process.env.E2E_VERBOSE ? "inherit" : "ignore", + env, + }); + child.unref(); + const inst: ConnectInstance = { child, dir, tmuxBase }; + instance = inst; + + const deadline = Date.now() + READY_TIMEOUT; + for (;;) { + try { + const res = await fetch(`${ORIGIN}/api/setup/status`); + if (res.ok) { + const body = (await res.json()) as { needsSetup: boolean }; + if (!body.needsSetup) throw new Error("[e2e] connect instance: port already set up (leaked run)"); + return inst; + } + } catch (err) { + if ((err as Error).message.includes("port already set up")) throw err as Error; + // Not listening yet. + } + if (Date.now() > deadline) throw new Error(`[e2e] connect instance did not become ready at ${ORIGIN}`); + await sleep(300); + } +} + +/** The instance this file's beforeAll started (module scope: the cleanup paths reach it across retries). */ +let instance: ConnectInstance | undefined; + +function stopConnectBackend(): void { + const inst = instance; + if (!inst) return; + instance = undefined; + try { + if (inst.child.pid) process.kill(-inst.child.pid, "SIGTERM"); + } catch { + // Already gone. + } + // The ssh pane's tmux server detached away from the backend's process group + // (the stack.ts lesson), so kill by socket, then wipe the scratch dirs. + for (const uidDir of readdirSafe(inst.tmuxBase)) { + for (const socket of readdirSafe(path.join(inst.tmuxBase, uidDir))) { + try { + spawnSync("tmux", ["-S", path.join(inst.tmuxBase, uidDir, socket), "kill-server"]); + } catch { + // Best-effort per socket. + } + } + } + rmSync(inst.dir, { recursive: true, force: true }); + rmSync(path.dirname(inst.tmuxBase), { recursive: true, force: true }); +} + +function readdirSafe(dir: string): string[] { + try { + return readdirSync(dir); + } catch { + return []; + } +} + +const ADMIN = { + name: "Connect Admin", + email: "connect-admin@subshell.test", + password: "e2e-connect-admin-pass-1", +} as const; +const MEMBER_PASSWORD = "e2e-connect-member-pass-1"; + +/** + * A fresh cookieless API context (spec 22's rule: the sign-up itself passes + * the Origin/CSRF guard and seeds the jar, so each principal acts as itself). + */ +function freshCtx(playwright: PlaywrightWorkerArgs["playwright"]): Promise { + return playwright.request.newContext({ baseURL: ORIGIN, storageState: { cookies: [], origins: [] } }); +} + +let fixture: SshFixture | undefined; +let fixtureRoot: string | undefined; +let agentPid: number | undefined; +/** The admin's REST session: the gate flips, the pane row, and every log read ride it while the browser does the UI. */ +let admin: APIRequestContext | undefined; +/** The pane test 3 launched; test 4 drives a viewer into it, and the cleanup reaches it by id. */ +let paneId: string | undefined; + +test.beforeAll(async ({ playwright }) => { + // The module-level skip already holds the file off this path; the guard is + // so a skipped file never boots daemons either. + if (missing.length > 0) return; + + fixtureRoot = mkdtempSync(path.join(tmpdir(), "subshell-e2e-connfix-")); + fixture = await startSshFixture(fixtureRoot); + + // An ssh-agent holding the fixture's trusted key (spec 22's setup): the + // backend's resolve reads SSH_AUTH_SOCK from the SERVER process env. The + // fixture alias pins `IdentityAgent none`, so the pane dials with the + // IdentityFile key; the agent is the env fact, not the authenticator. + const agentOut = spawnSync("ssh-agent", ["-s"], { encoding: "utf8" }); + if (agentOut.status !== 0) throw new Error(`ssh-agent failed: ${agentOut.stderr}`); + const agentEnv = Object.fromEntries( + ["SSH_AUTH_SOCK", "SSH_AGENT_PID"].map((k) => [k, agentOut.stdout.match(new RegExp(`${k}=([^;]+)`))?.[1] ?? ""]), + ); + agentPid = Number(agentEnv.SSH_AGENT_PID); + if (!agentEnv.SSH_AUTH_SOCK || !Number.isInteger(agentPid) || agentPid <= 0) { + throw new Error(`ssh-agent output unparseable: ${agentOut.stdout.slice(0, 200)}`); + } + const add = spawnSync("ssh-add", [fixture.trustedKeyPath], { + env: { ...process.env, SSH_AUTH_SOCK: agentEnv.SSH_AUTH_SOCK }, + encoding: "utf8", + }); + if (add.status !== 0) throw new Error(`ssh-add failed: ${add.stderr}`); + + await startConnectBackend(fixture.sshConfigHome, agentEnv.SSH_AUTH_SOCK); + + // The first registered account becomes admin (spec 01's rule on a fresh + // instance), through the wizard's own endpoint. needsSetup is `!hasUsers` + // server-side, so from here the SPA answers the app, not /setup. + admin = await freshCtx(playwright); + const signUp = await admin.post("/api/auth/sign-up/email", { data: ADMIN }); + expect(signUp.ok(), await signUp.text()).toBe(true); + + // The first promoted account is born with the wizard bookmark "network" + // (promoteFirstUserAtomically), which keeps the signed-in root gate on + // /setup. This account was minted by the API, not the wizard, so the + // bookmark is cleared the way the wizard's own completion clears it - the + // PATCH is the same route the wizard's writes go through (cookie session + // only, which the sign-up above seeded). + const retired = await admin.patch("/api/setup/progress", { data: { step: null } }); + expect(retired.ok(), await retired.text()).toBe(true); + expect(((await retired.json()) as { step: string | null }).step).toBeNull(); +}); + +test.afterAll(async () => { + const leaks: string[] = []; + if (paneId !== undefined && admin) { + try { + const res = await admin.post(`/api/subshells/${paneId}/terminate`); + if (!res.ok() && res.status() !== 404) leaks.push(`cleanup terminate: HTTP ${res.status()}`); + await admin.delete(`/api/subshells/${paneId}`); + } catch (err) { + leaks.push(`cleanup row: ${String(err)}`); + } + } + try { + await admin?.dispose(); + } catch (err) { + leaks.push(`admin ctx dispose: ${String(err)}`); + } + admin = undefined; + paneId = undefined; + if (agentPid !== undefined) { + try { + process.kill(agentPid, "SIGTERM"); + } catch { + // Already gone. + } + } + try { + await fixture?.stop(); + } catch (err) { + leaks.push(`sshd fixture stop: ${String(err)}`); + } + try { + stopConnectBackend(); + } catch (err) { + leaks.push(`backend stop: ${String(err)}`); + } + if (fixtureRoot !== undefined) rmSync(fixtureRoot, { recursive: true, force: true }); + fixture = undefined; + fixtureRoot = undefined; + if (leaks.length > 0) console.error(`[23-connect-flow] cleanup problems: ${leaks.join("; ")}`); +}); + +/** Sign in through the REAL login form on this instance (spec 11's pattern, absolute origin: the page fixture's baseURL is the shared stack). */ +async function login(page: Page, email: string, password: string): Promise { + await page.goto(`${ORIGIN}/login`); + await page.fill("#email", email); + await page.fill("#password", password); + await page.getByRole("button", { name: "Sign in", exact: true }).click(); + await page.waitForURL((url) => !url.pathname.startsWith("/login"), { timeout: READY_TIMEOUT }); +} + +/** The /connect page, fields mounted (the panel renders them on mount; the queries fill them in). */ +async function openConnectPage(page: Page): Promise { + await page.goto(`${ORIGIN}/connect`); + await expect(page.getByRole("heading", { name: "Connect", exact: true })).toBeVisible(); + await expect(page.locator("#connect-destination")).toBeVisible(); +} + +function requireAdmin(): APIRequestContext { + if (!admin) throw new Error("[23] beforeAll did not produce the admin session"); + return admin; +} + +function requirePaneId(): string { + if (paneId === undefined) throw new Error("[23] test 3 never produced a pane"); + return paneId; +} + +test("gate off: the control-plane host sits disabled in the machine picker with its reason", async ({ page }) => { + await login(page, ADMIN.email, ADMIN.password); + await openConnectPage(page); + + // Base UI opens its popup on the pointer path; a real Playwright click is + // exactly that. The picker keeps the gated-off row LISTED (decision 3). + await page.locator("#connect-machine").click(); + const popup = page.locator('[data-slot="combobox-content"]'); + const serverRow = popup.getByRole("option", { name: /Server/ }); + await expect(serverRow).toHaveAttribute("data-disabled", ""); + await expect(serverRow).toContainText(MACHINE_OFF_REASON); + + // Inertness, asserted rather than attempted (the T3 handoff): no click is + // fired at the disabled row; the field still carries no selection. + await expect(page.locator("#connect-machine")).toHaveValue(""); +}); + +test("the disclosure names the Match exec risk above the Connect button, before any Connect", async ({ page }) => { + await login(page, ADMIN.email, ADMIN.password); + await openConnectPage(page); + + const disclosure = page.getByText(SSH_DISCLOSURE_COPY); + await expect(disclosure).toBeVisible(); + const connectButton = page.getByRole("button", { name: "Connect", exact: true }); + await expect(connectButton).toBeVisible(); + + // "Before the button" literally: the paragraph's box lies above the + // button's box in the reading column. + const disclosureBox = await disclosure.boundingBox(); + const buttonBox = await connectButton.boundingBox(); + expect(disclosureBox && buttonBox ? disclosureBox.y + disclosureBox.height <= buttonBox.y : false).toBe(true); +}); + +test("gate on: destination-first launch reaches a live ssh pane whose remote shell answers", async ({ page }) => { + test.setTimeout(300_000); + const adminCtx = requireAdmin(); + + // The flip happens OUTSIDE the browser (REST, admin cookie) and the reload + // picks the fact up: the page reads sshEnabled off the node view it fetches + // fresh on mount. + const enable = await adminCtx.put("/api/nodes/local/ssh-enabled", { data: { on: true } }); + expect(enable.ok(), await enable.text()).toBe(true); + expect(((await enable.json()) as { sshEnabled: boolean }).sshEnabled).toBe(true); + + await login(page, ADMIN.email, ADMIN.password); + await openConnectPage(page); + + // Exactly one SSH-enabled machine and no preference: pre-selected honestly + // and still on screen (decision 2), labelled with the node's own name. + await expect(page.locator("#connect-machine")).toHaveValue(/Server/); + + // The destination picker's config group (the handoff): the BROWSER session + // fetches the machine's aliases; the fixture's alias rides under it. + await page.locator("#connect-destination").click(); + const panel = page.locator("#connect-destination-panel"); + await expect(panel.getByText(CONFIG_GROUP, { exact: true })).toBeVisible({ timeout: READY_TIMEOUT }); + await panel.getByRole("button", { name: SSH_FIXTURE_ALIAS, exact: true }).click(); + // The committed pick: the field's text is the row's label, and the wire + // value behind it is the alias token (buildDestinationOptions). + await expect(page.locator("#connect-destination")).toHaveValue(SSH_FIXTURE_ALIAS); + + // Connect: one POST, and the router lands on the pane's page. Arm the + // attach socket BEFORE the click (spec 15's rule): typing needs the live + // /ws, not just a running row. + const socket = page.waitForEvent("websocket", { + predicate: (w) => w.url().includes("/ws"), + timeout: READY_TIMEOUT, + }); + await page.getByRole("button", { name: "Connect", exact: true }).click(); + await expect(page).toHaveURL(/\/subshells\/[a-zA-Z0-9_-]{1,64}$/, { timeout: READY_TIMEOUT }); + paneId = new URL(page.url()).pathname.split("/").pop(); + + // The pane's own header labels the kind it is (T4's badge, owner included). + await expect(page.getByText("SSH", { exact: true }).first()).toBeVisible({ timeout: READY_TIMEOUT }); + await expectSubshellRunning(page, READY_TIMEOUT); + await expect(page.locator(".xterm-screen")).toBeVisible(); + await socket; + + // Settle on sshd's own truth, not the row (spec 22): `alive` is stamped at + // create, and the pty is in the pre-raw line discipline during the ssh + // handshake - it swallows the Enter. The beat after covers the remote + // shell's init before its line editor exists. + const daemonLog = fixture?.logPath; + if (daemonLog === undefined) throw new Error("[23] fixture is gone"); + await pollUntil( + () => `sshd never reported a shell for ${paneId}; its log ends: ${readFileSync(daemonLog, "utf8").slice(-800)}`, + () => /Starting session: shell/.test(readFileSync(daemonLog, "utf8")), + READY_TIMEOUT, + ); + await sleep(1_500); + + await page.locator(".xterm-helper-textarea").click(); + // The typed line NEVER spells the result (spec 22's argument): only the + // REMOTE shell's arithmetic prints CONNECT-7-OK, so the log match proves + // bytes travelled both ways through real ssh, typed here through the page. + await page.keyboard.type("echo CONNECT-$((6+1))-OK\r"); + + let lastLogView = ""; + await pollUntil( + () => + `the pane log never carried the remote shell's evaluated marker; last read: ${JSON.stringify(lastLogView.slice(-1200))}`, + async () => { + const res = await adminCtx.get(`/api/subshells/${paneId}/log`); + if (!res.ok()) { + lastLogView = `HTTP ${res.status()} ${(await res.text()).slice(0, 300)}`; + return false; + } + lastLogView = ((await res.json()) as { lines: string[] }).lines.join("\n"); + return lastLogView.includes("CONNECT-7-OK"); + }, + SPAWN_TIMEOUT, + ); +}); + +test("view only: an edit sharee sees the SSH rule in the header and their typing changes nothing", async ({ page }) => { + test.setTimeout(180_000); + const adminCtx = requireAdmin(); + const id = requirePaneId(); + + // A second account with the ordinary EVERYONE edit grant (spec 22's shape). + const memberEmail = `connect-member-${test.info().retry}@subshell.test`; + const created = await adminCtx.post("/api/users", { + data: { email: memberEmail, name: memberEmail, password: MEMBER_PASSWORD, role: "user" }, + }); + expect(created.ok(), await created.text()).toBe(true); + const shared = await adminCtx.put(`/api/subshells/${id}/shares`, { + data: { shares: [{ granteeUserId: null, permission: "edit" }] }, + }); + expect(shared.ok(), await shared.text()).toBe(true); + + await login(page, memberEmail, MEMBER_PASSWORD); + + // The VIEW layer's downgrade read through the member's own browser session + // (decision 1): the grant says edit, the ssh rule answers view. + const memberView = await page.request.get(`${ORIGIN}/api/subshells/${id}`); + expect(memberView.ok(), await memberView.text()).toBe(true); + const view = (await memberView.json()) as { access: string; ssh: boolean }; + expect(view.access).toBe("view"); + expect(view.ssh).toBe(true); + + await page.goto(`${ORIGIN}/subshells/${id}`); + await expect(page.locator(".xterm-screen")).toBeVisible({ timeout: READY_TIMEOUT }); + // The badge names the kind of pane; the sentence names the rule (T4's copy). + await expect(page.getByText("SSH", { exact: true }).first()).toBeVisible(); + await expect(page.getByText(SSH_VIEW_ONLY_COPY)).toBeVisible(); + + // The client posture: the terminal is mounted but takes nothing. Whether the + // keystroke dies at disableStdin or at the WS gate, the PROOF is the same - + // the nonce never reaches the log (spec 22 owns the 403 itself). + await page.locator(".xterm-helper-textarea").click(); + const nonce = `VIEW-TYPE-${test.info().retry}-${Date.now()}`; + await page.keyboard.type(`echo ${nonce}\r`); + await sleep(1_500); // give any raced write a beat, then require absence from the whole log + const logRes = await adminCtx.get(`/api/subshells/${id}/log`); + expect(logRes.ok(), await logRes.text()).toBe(true); + expect(((await logRes.json()) as { lines: string[] }).lines.join("\n")).not.toContain(nonce); +}); From 20edf35ddc04b25bf04869aa7ba467624b97454c Mon Sep 17 00:00:00 2001 From: Theo Gravity Date: Thu, 8 Oct 2026 00:11:16 -0700 Subject: [PATCH 09/16] chore(changeset): the destination-first UI (Milestone 1 complete) --- .changeset/ssh-destination-first-ui.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 .changeset/ssh-destination-first-ui.md diff --git a/.changeset/ssh-destination-first-ui.md b/.changeset/ssh-destination-first-ui.md new file mode 100644 index 000000000..0cce7f795 --- /dev/null +++ b/.changeset/ssh-destination-first-ui.md @@ -0,0 +1,18 @@ +--- +"@internal/server": minor +"@internal/server-web": minor +--- + +The Connect page: one question first - where do you want to work? Pick a +destination from your remembered hosts, your recent ones, or the connecting +machine's SSH config aliases, or type any host; one button opens the pane. +The connecting machine defaults to your chosen one, machines whose SSH is off +stay named with the remedy rather than silently skipped, and a config that +needs settings Subshell does not run shows which ones blocked. SSH-terminal +panes now read as view-only to everyone but their owner everywhere the pane +is shown (the server's view says so, so the terminal is honestly read-only), +and the pane's header labels itself SSH and tells a viewer why typing is off. + +Milestone 1 of the SSH-anywhere design is complete with this: an operator on +an enabled machine opens an interactive session at any sshd-only host, and no +private key or plaintext challenge ever crosses the plane. From 50ef7526cc87186d947c57bc8b4724c9028d4c6b Mon Sep 17 00:00:00 2001 From: Theo Gravity Date: Thu, 8 Oct 2026 00:23:05 -0700 Subject: [PATCH 10/16] fix(web): reviewer round 1 - maintenance names itself, Connect cannot no-op on an empty machine, the gate remedy rides the row --- .../connect/__tests__/connect-panel.test.tsx | 83 ++++++++++++++++++- .../connect/__tests__/ssh-refusal.test.ts | 40 +++++++-- .../src/components/connect/connect-panel.tsx | 60 +++++++++----- .../web/src/components/connect/ssh-refusal.ts | 20 ++++- 4 files changed, 172 insertions(+), 31 deletions(-) diff --git a/apps/server/web/src/components/connect/__tests__/connect-panel.test.tsx b/apps/server/web/src/components/connect/__tests__/connect-panel.test.tsx index a984dbb7f..d3b590542 100644 --- a/apps/server/web/src/components/connect/__tests__/connect-panel.test.tsx +++ b/apps/server/web/src/components/connect/__tests__/connect-panel.test.tsx @@ -181,7 +181,7 @@ async function clickConnect(): Promise { afterEach(cleanup); describe("machine disclosure (contract 1)", () => { - it("keeps a gated-off machine visible, greyed, with the reason naming what is in the way", async () => { + it("keeps a gated-off machine visible, greyed, with the reason naming the remedy", async () => { const { pathname, restore } = await renderPanel({ nodes: [HOST_A, HOST_B] }); try { // HOST_A is the sole SSH-enabled machine, so it pre-selects honestly - @@ -190,13 +190,31 @@ describe("machine disclosure (contract 1)", () => { await settle(); const studio = await screen.findByRole("option", { name: /studio/ }); expect(studio.getAttribute("data-disabled")).toBe(""); - expect(studio.textContent).toContain("SSH is off on this machine"); + // The row carries the remedy, not just the cause (the changeset's + // promise; the server's gateCause sentences are the pattern). + expect(studio.textContent).toContain( + "SSH is off on this machine. Its owner can switch it on from this machine's settings.", + ); expect(pathname()).toBe("/"); } finally { restore(); } }); + it("names the admin remedy on a gated-off local row, kind-aware like the server's gate", async () => { + const localOff = node({ id: "loc", name: "this host", kind: "local", sshEnabled: false }); + const { restore } = await renderPanel({ nodes: [HOST_A, localOff] }); + try { + openMachinePopup(); + await settle(); + const row = await screen.findByRole("option", { name: /this host/ }); + expect(row.getAttribute("data-disabled")).toBe(""); + expect(row.textContent).toContain("SSH is off here. An admin can switch it on in Server Settings."); + } finally { + restore(); + } + }); + it("pre-selects the stored default machine", async () => { const { restore } = await renderPanel({ nodes: [HOST_A, HOST_C], @@ -209,16 +227,60 @@ describe("machine disclosure (contract 1)", () => { } }); - it("pre-selects the single SSH-enabled machine with the preference unset", async () => { + it("pre-selects the single SSH-enabled machine with the preference unset, and asks nothing extra", async () => { const { restore } = await renderPanel({ nodes: [HOST_A, HOST_B] }); try { await waitFor(() => expect(machineInput().value).toBe("mac mini")); + // The one usable machine is pre-selected honestly: no gold question + // mark, no requirement caption (Connect is live once a destination stands). + const label = document.querySelector('label[for="connect-machine"]'); + expect(label?.querySelector('[class*="after:content"]')).toBeNull(); + expect(screen.queryByText("Choose a connecting machine first.")).toBeNull(); + } finally { + restore(); + } + }); + + it("a stale default with one enabled machine still pre-selects it, unmarked", async () => { + // The ledger points at a vanished node; the only usable machine wins. + const { restore } = await renderPanel({ + nodes: [HOST_A, HOST_B], + ledger: { saved: [], recent: [], defaultNodeId: "vanished-node" }, + }); + try { + await waitFor(() => expect(machineInput().value).toBe("mac mini")); + const label = document.querySelector('label[for="connect-machine"]'); + expect(label?.querySelector('[class*="after:content"]')).toBeNull(); + } finally { + restore(); + } + }); + + it("a stale default with NO enabled machine: the caption names the missing machine and Connect cannot fire", async () => { + // The honest dead end: the preference points at a row that is gone and + // nothing is SSH-enabled. The sentence names the ENVIRONMENT (gold, one + // line), and the disabled button means Connect can never be a silent + // no-op: a typed destination alone must not POST. + const { calls, restore } = await renderPanel({ + nodes: [], + ledger: { saved: [], recent: [], defaultNodeId: "vanished-node" }, + }); + try { + expect(machineInput().value).toBe(""); + const caption = screen.getByText("No SSH-enabled machine is available. Enable SSH on a machine first."); + expect(caption.className).toContain("text-amber-600"); // gold: nothing chosen, not a refused value + await commitTyped("box.example"); + const button = screen.getByRole("button", { name: /^Connect$/ }) as HTMLButtonElement; + expect(button.disabled).toBe(true); // the machine is in the disabled condition + await clickConnect(); + expect(calls.filter((c) => c.url === "/api/ssh/launch")).toEqual([]); + expect(screen.getByText("No SSH-enabled machine is available. Enable SSH on a machine first.")).toBeDefined(); } finally { restore(); } }); - it("several enabled machines and no preference is a required question: gold *, gold caption on the submit, no POST", async () => { + it("several enabled machines and no preference is a required question: gold *, gold caption, no POST", async () => { const { calls, restore } = await renderPanel({ nodes: [HOST_A, HOST_C] }); try { expect(machineInput().value).toBe(""); // never silently chosen @@ -475,4 +537,17 @@ describe("refusals and disclosure (contracts 4, 5)", () => { restore(); } }); + + it("wires the machine requirement caption as the picker's description", async () => { + // The same aria posture the destination field pins above: whatever the + // machine picker is explaining about itself is its accessible + // description, named by id only while it is on screen. + const { restore } = await renderPanel({ nodes: [HOST_A, HOST_C] }); + try { + await waitFor(() => expect(document.getElementById("connect-machine-requirement")).not.toBeNull()); + expect(machineInput().getAttribute("aria-describedby")).toContain("connect-machine-requirement"); + } finally { + restore(); + } + }); }); diff --git a/apps/server/web/src/components/connect/__tests__/ssh-refusal.test.ts b/apps/server/web/src/components/connect/__tests__/ssh-refusal.test.ts index 45e8b4997..b622c0f09 100644 --- a/apps/server/web/src/components/connect/__tests__/ssh-refusal.test.ts +++ b/apps/server/web/src/components/connect/__tests__/ssh-refusal.test.ts @@ -44,7 +44,7 @@ describe("sshLaunchRefusal", () => { it("400 names the destination as invalid, not the machine", () => { expect(sshLaunchRefusal(new ApiError(400, "x", { code: "ALIAS_UNSAFE" }), machine({}))).toEqual({ - text: "That destination is not a valid host name.", + text: "That destination is not a valid name to connect to.", field: "destination", }); }); @@ -97,9 +97,24 @@ describe("sshLaunchRefusal", () => { ); }); - it("502 is the machine's own refusal, in one sentence", () => { + it("409 NODE_IN_MAINTENANCE names maintenance, not the missing connection", () => { + // The create under the ssh launch throws this code when the machine is in + // a window (`rethrowLaunchRefusal`, subshells.service.ts) - a live link + // exists, it just takes no new panes, so the offline sentence would lie. + const copy = sshLaunchRefusal(new ApiError(409, "x", { code: "NODE_IN_MAINTENANCE" }), machine({})); + expect(copy?.text).toBe( + "mac mini is in maintenance and takes no new panes. Wait for it to come out of maintenance.", + ); + expect(copy?.text).not.toContain("no live connection"); + // The vanish-the-row fallback stays grammatical too. + expect(sshLaunchRefusal(new ApiError(409, "x", { code: "NODE_IN_MAINTENANCE" }), null)?.text).toBe( + "That machine is in maintenance and takes no new panes. Wait for it to come out of maintenance.", + ); + }); + + it("502 is the machine's own refusal, with the checks that answer it", () => { expect(sshLaunchRefusal(new ApiError(502, "x", { code: "SSH_NODE_REFUSED" }), machine({}))).toEqual({ - text: "The connecting machine did not answer.", + text: "The connecting machine refused the SSH request. Check that SSH is switched on there and that ssh is installed.", field: "machine", }); }); @@ -116,8 +131,23 @@ describe("sshLaunchRefusal", () => { }); it("every shipped sentence is at most two sentences and dash-free", () => { - const texts = [422, 400, 403, 404, 409, 502].flatMap((status) => { - const copy = sshLaunchRefusal(new ApiError(status, "x", { code: "ANY" }), machine({ sshEnabled: false })); + // Named codes too, so the per-branch arms are all in the sweep, not just + // the default ones. + const cases: [number, string][] = [ + [422, "ANY"], + [400, "ALIAS_UNSAFE"], + [403, "SSH_GATE_OFF"], + [404, "NOT_FOUND_ERROR"], + [409, "NODE_IN_MAINTENANCE"], + [409, "NODE_PROTOCOL_HELD"], + [409, "NODE_OUTDATED"], + [409, "NODE_UNREACHABLE"], + [409, "NODE_OFFLINE"], + [409, "ANY"], + [502, "SSH_NODE_REFUSED"], + ]; + const texts = cases.flatMap(([status, code]) => { + const copy = sshLaunchRefusal(new ApiError(status, "x", { code }), machine({ sshEnabled: false })); return copy === null ? [] : [copy.text]; }); for (const text of texts) { diff --git a/apps/server/web/src/components/connect/connect-panel.tsx b/apps/server/web/src/components/connect/connect-panel.tsx index 3248d0df8..dfff77df8 100644 --- a/apps/server/web/src/components/connect/connect-panel.tsx +++ b/apps/server/web/src/components/connect/connect-panel.tsx @@ -22,11 +22,13 @@ import { launchableNodes } from "@/lib/subshell-compat"; * display, never a pre-resolve step. Three nouns only (the §11 vocabulary): * destination, connecting machine. No key-source affordance (that is M2). * - * A machine whose SSH is off stays VISIBLE and disabled with its reason - - * greying explains, hiding does not (decision 3). With no default machine and - * several SSH-enabled ones the disclosure is required with the gold `*` and - * never silently chosen; with a preference or exactly one usable machine it - * pre-selects honestly and stays on screen. The disclosure sentence (the + * A machine whose SSH is off stays VISIBLE and disabled with its reason and + * remedy - greying explains, hiding does not (decision 3). Once both feeds + * have answered, an empty picker IS the open question: the gold `*` and the + * gold sentence stand under it and Connect stays disabled, never a silent + * no-op (a stored default or the only usable machine pre-selects honestly and + * clears them; when no machine is enabled the sentence names that). The + * disclosure sentence (the * `Match exec` note, decision 6) ships verbatim under the destination field, * before the button, because approving a destination approves running its * resolution. @@ -71,7 +73,7 @@ export function ConnectPanel(): JSX.Element { const navigate = useNavigate(); const { data: nodeData } = useNodes(); const nodes = Array.isArray(nodeData?.nodes) ? nodeData.nodes : null; - const { data: ledger } = useSshSavedHosts(); + const { data: ledger, isPending: ledgerPending } = useSshSavedHosts(); const launch = useLaunchSsh(); const save = useSaveSshHost(); const setDefault = useSetDefaultNode(); @@ -79,25 +81,32 @@ export function ConnectPanel(): JSX.Element { const [nodeId, setNodeId] = useState(""); const [dest, setDest] = useState({ pick: null, pickId: null, typed: "" }); const [remember, setRemember] = useState(false); - // The gold caption waits for a submit that found the machine untouched: - // "nothing typed yet", never before the first attempt (ruling 2026-09-30). - const [submitAttempted, setSubmitAttempted] = useState(false); const [refusal, setRefusal] = useState(null); const targets = launchableNodes(nodes ?? []); const selectedNode: Node | null = (nodes ?? []).find((n) => n.id === nodeId) ?? null; const defaultNodeId = ledger?.defaultNodeId ?? null; const enabledCount = targets.filter((n) => n.sshEnabled).length; - // No preference, several doors: the choice is the human's (spec §7). - const machineRequired = nodeId === "" && defaultNodeId === null && enabledCount > 1; + // "Settled" means both feeds have answered, so the pre-fill effect has had + // its say: a stored default or the only usable machine has landed, or there + // was nothing to land. From then on an empty machine IS the open question - + // several doors with no preference, or no door at all (spec §7, decision 2). + const machineRequired = nodes !== null && !ledgerPending && nodeId === ""; // The machine disclosure keeps gated-off rows visible (decision 3): grey, - // with the reason naming what is in the way. + // with the reason carrying the remedy, not just the cause - the sentence + // pair mirrors the server's own gate (ssh-launch.service.ts's `gateCause`): + // the control-plane host's switch is an admin's, every other machine's is + // its owner's. const machineOptions: ComboboxOption[] = targets.map((n) => ({ value: n.id, label: nodeOptionLabel(n), disabled: !n.sshEnabled, - reason: n.sshEnabled ? undefined : "SSH is off on this machine", + reason: n.sshEnabled + ? undefined + : n.kind === "local" + ? "SSH is off here. An admin can switch it on in Server Settings." + : "SSH is off on this machine. Its owner can switch it on from this machine's settings.", })); // Aliases are the MACHINE's config, asked only while a gate-ON machine is @@ -151,7 +160,8 @@ export function ConnectPanel(): JSX.Element { } async function submit(): Promise { - setSubmitAttempted(true); + // Belt over gate: the button's own disabled condition already holds both + // fields, so this can only fire on a render race, never as a silent no-op. if (nodeId === "" || destination === "") return; setRefusal(null); try { @@ -171,10 +181,22 @@ export function ConnectPanel(): JSX.Element { } } - const machineGap = - submitAttempted && machineRequired - ? fieldErrorToned([{ message: "Choose a connecting machine first.", gap: true }]) - : null; + // An open machine question speaks the moment it stands: Connect is disabled + // while the picker is empty, and a dim button with no sentence is the + // silent no-op this round refused. Gold, never red - "nothing chosen yet" + // (ruling 2026-09-30) - and when no enabled machine exists the sentence + // names the environment, not the person. + const machineGap = machineRequired + ? fieldErrorToned([ + { + message: + enabledCount > 0 + ? "Choose a connecting machine first." + : "No SSH-enabled machine is available. Enable SSH on a machine first.", + gap: true, + }, + ]) + : null; // aria-describedby may name nothing that is not rendered, so the // association is exactly the explanation lines currently on screen. const machineRefusal = refusal?.field === "machine" ? refusal.text : null; @@ -253,7 +275,7 @@ export function ConnectPanel(): JSX.Element {
    - diff --git a/apps/server/web/src/components/connect/ssh-refusal.ts b/apps/server/web/src/components/connect/ssh-refusal.ts index 8785bae63..5816c50c4 100644 --- a/apps/server/web/src/components/connect/ssh-refusal.ts +++ b/apps/server/web/src/components/connect/ssh-refusal.ts @@ -56,7 +56,7 @@ export function sshLaunchRefusal(err: unknown, machine: SshMachineFacts | null): switch (err.status) { case 400: // ALIAS_UNSAFE: the token was refused before the machine was asked. - return { text: "That destination is not a valid host name.", field: "destination" }; + return { text: "That destination is not a valid name to connect to.", field: "destination" }; case 403: { if (err.code === "SSH_GATE_OFF" && machine !== null && !machine.sshEnabled) { // The gate's off-door. Naming WHO flips it follows the row's kind: @@ -94,6 +94,15 @@ export function sshLaunchRefusal(err: unknown, machine: SshMachineFacts | null): text: `${name} did not answer the SSH request in time. Check its connection and retry.`, field: "machine", }; + case "NODE_IN_MAINTENANCE": + // The create under the launch refuses in a window (`rethrow- + // LaunchRefusal`, subshells.service.ts): the link is LIVE, it + // just takes no new panes - saying "no live connection" here + // would name the wrong fact and the wrong wait. + return { + text: `${name} is in maintenance and takes no new panes. Wait for it to come out of maintenance.`, + field: "machine", + }; default: // NODE_OFFLINE and any other 409: no live link to carry the ask. return { @@ -112,8 +121,13 @@ export function sshLaunchRefusal(err: unknown, machine: SshMachineFacts | null): } case 502: // The machine answered its own refusal (no ssh binary, the mirror - // says no); its verbatim text never rides the wire to here. - return { text: "The connecting machine did not answer.", field: "machine" }; + // says no); its verbatim text never rides the wire to here. This is + // the server's own sentence (`rpcRefusal`): the two checks that + // answer the machine's no. + return { + text: "The connecting machine refused the SSH request. Check that SSH is switched on there and that ssh is installed.", + field: "machine", + }; default: break; } From 02c644a65cbcb5b899e510a2b054220415c5e83a Mon Sep 17 00:00:00 2001 From: Theo Gravity Date: Thu, 8 Oct 2026 00:24:27 -0700 Subject: [PATCH 11/16] test(e2e): the gate-off pin follows the kind-aware remedy copy --- e2e/tests/23-connect-flow.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e/tests/23-connect-flow.spec.ts b/e2e/tests/23-connect-flow.spec.ts index 23cd9fb8a..c128ffc34 100644 --- a/e2e/tests/23-connect-flow.spec.ts +++ b/e2e/tests/23-connect-flow.spec.ts @@ -59,7 +59,7 @@ test.skip(missing.length > 0, `SSH binaries absent on this host (${missing.join( const SSH_DISCLOSURE_COPY = "Resolving asks the connecting machine to read its SSH config. A hidden Match exec in that config can run a local command while it resolves."; const SSH_VIEW_ONLY_COPY = "SSH sessions take input from their owner only."; -const MACHINE_OFF_REASON = "SSH is off on this machine"; +const MACHINE_OFF_REASON = "SSH is off here. An admin can switch it on in Server Settings."; /** The seeded control-plane host's display name (seed-local.ts): the picker reads names, not ids. */ const LOCAL_NODE_NAME = "Server"; /** `configGroupLabel(selectedNode.name)` for that row. */ From db86b7f986494e9372bceae54f714eebe24fc9f5 Mon Sep 17 00:00:00 2001 From: Theo Gravity Date: Thu, 8 Oct 2026 00:31:21 -0700 Subject: [PATCH 12/16] fix(web): reviewer round 2 - the local gate remedy points at the machine's settings (matching the server's gateCause); the at-rest-gold picker carve-out is documented --- .claude/rules/design-system.md | 4 +++- .../components/connect/__tests__/connect-panel.test.tsx | 2 +- apps/server/web/src/components/connect/connect-panel.tsx | 2 +- docs/design-system.md | 7 ++++++- e2e/tests/23-connect-flow.spec.ts | 2 +- 5 files changed, 12 insertions(+), 5 deletions(-) diff --git a/.claude/rules/design-system.md b/.claude/rules/design-system.md index eb7271a3e..5e0b5bb4d 100644 --- a/.claude/rules/design-system.md +++ b/.claude/rules/design-system.md @@ -46,7 +46,9 @@ fields. Name-length limits and historical specs aren't exceptions. Follow the [content guide](../../apps/docs/STYLE.md#product-copy-and-confirmations) for action labels, consequences, and examples. **A required input takes a gold `*` at rest and a gold caption once the caret -leaves it empty; a hard error (a refused value, a server refusal) stays +leaves it empty; a required control with no caret-leave moment (a picker whose +nothing-picked state disables the submit) wears the gold caption at rest once +its options settle; a hard error (a refused value, a server refusal) stays `text-destructive` red** (ruling 2026-09-30: gold says "nothing typed yet", red says "what you typed is wrong"; `fieldErrorToned` reads the split). **There is no 12px**: `detail` (13) is the floor, and `text-xs` / diff --git a/apps/server/web/src/components/connect/__tests__/connect-panel.test.tsx b/apps/server/web/src/components/connect/__tests__/connect-panel.test.tsx index d3b590542..2533c07b9 100644 --- a/apps/server/web/src/components/connect/__tests__/connect-panel.test.tsx +++ b/apps/server/web/src/components/connect/__tests__/connect-panel.test.tsx @@ -209,7 +209,7 @@ describe("machine disclosure (contract 1)", () => { await settle(); const row = await screen.findByRole("option", { name: /this host/ }); expect(row.getAttribute("data-disabled")).toBe(""); - expect(row.textContent).toContain("SSH is off here. An admin can switch it on in Server Settings."); + expect(row.textContent).toContain("SSH is off here. An admin can switch it on from this machine's settings."); } finally { restore(); } diff --git a/apps/server/web/src/components/connect/connect-panel.tsx b/apps/server/web/src/components/connect/connect-panel.tsx index dfff77df8..b8be30ef5 100644 --- a/apps/server/web/src/components/connect/connect-panel.tsx +++ b/apps/server/web/src/components/connect/connect-panel.tsx @@ -105,7 +105,7 @@ export function ConnectPanel(): JSX.Element { reason: n.sshEnabled ? undefined : n.kind === "local" - ? "SSH is off here. An admin can switch it on in Server Settings." + ? "SSH is off here. An admin can switch it on from this machine's settings." : "SSH is off on this machine. Its owner can switch it on from this machine's settings.", })); diff --git a/docs/design-system.md b/docs/design-system.md index e6b006858..1a4f7df5f 100644 --- a/docs/design-system.md +++ b/docs/design-system.md @@ -116,7 +116,12 @@ admitted here. - **Required input**: a gold `*` (`RequiredMark`) stands beside every REQUIRED label at rest, and the submit is disabled until the one schema is satisfied. When the caret LEAVES a field still empty, the sentence appears - under it in the same gold. Gold means "nothing typed yet". + under it in the same gold. Gold means "nothing typed yet". A REQUIRED + control with no caret-leave moment - a picker whose nothing-picked-yet state + itself disables the submit - wears the gold sentence at rest instead, once + its options have settled, since its emptiness is already the "nothing chosen + yet" fact and waiting for a blur that never comes would be the same silent + disabled button the ruling kills. *(The disabled button was honest but silent: an untouched dialog said nothing about what it demanded, and a live-tester dismissed a half-filled preset deciding the form "didn't want the name". Operator ruling, diff --git a/e2e/tests/23-connect-flow.spec.ts b/e2e/tests/23-connect-flow.spec.ts index c128ffc34..cd7ade38e 100644 --- a/e2e/tests/23-connect-flow.spec.ts +++ b/e2e/tests/23-connect-flow.spec.ts @@ -59,7 +59,7 @@ test.skip(missing.length > 0, `SSH binaries absent on this host (${missing.join( const SSH_DISCLOSURE_COPY = "Resolving asks the connecting machine to read its SSH config. A hidden Match exec in that config can run a local command while it resolves."; const SSH_VIEW_ONLY_COPY = "SSH sessions take input from their owner only."; -const MACHINE_OFF_REASON = "SSH is off here. An admin can switch it on in Server Settings."; +const MACHINE_OFF_REASON = "SSH is off here. An admin can switch it on from this machine's settings."; /** The seeded control-plane host's display name (seed-local.ts): the picker reads names, not ids. */ const LOCAL_NODE_NAME = "Server"; /** `configGroupLabel(selectedNode.name)` for that row. */ From 74a7c4a92b5c5cee426a7bc65ef6cfed81ff09cf Mon Sep 17 00:00:00 2001 From: Theo Gravity Date: Thu, 8 Oct 2026 00:58:28 -0700 Subject: [PATCH 13/16] test(web): raise the per-test budget knob 30s to 45s CI's Test: web job timed out NetworkPluginCard's fail-then-succeed chain at 30.098s on this larger suite. The file is pre-existing (#318, last touched before this stack); its own comments carry the whole measurement trail, and the CI regime is the documented starved slice (serial, 0.4 CPU, width 1 once stretched a 2.5s react-act chain past 25s). The chain runs 3.7s idle, so this is the package's one sanctioned knob (par-test.sh, server testing-notes rule), not an assertion change: every per-wait budget stays honest, and the knob moves 20 -> 30 -> 45 exactly where it has before. Focused: network-plugin-card 86 pass serially; full web suite green. --- apps/server/web/package.json | 2 +- .../__tests__/network-plugin-card.test.tsx | 15 +++++++++------ 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/apps/server/web/package.json b/apps/server/web/package.json index 72118dc8c..0355d4e9c 100644 --- a/apps/server/web/package.json +++ b/apps/server/web/package.json @@ -12,7 +12,7 @@ "lint": "biome check --no-errors-on-unmatched --write --unsafe src", "lint:check": "biome check --no-errors-on-unmatched src", "lint:staged": "biome check --no-errors-on-unmatched --write --unsafe --staged src", - "test": "bash \"$(git rev-parse --show-toplevel)/scripts/par-test.sh\" 12 bun test --timeout 30000 --pass-with-no-tests", + "test": "bash \"$(git rev-parse --show-toplevel)/scripts/par-test.sh\" 12 bun test --timeout 45000 --pass-with-no-tests", "verify-types": "tsc --noEmit" }, "dependencies": { diff --git a/apps/server/web/src/components/__tests__/network-plugin-card.test.tsx b/apps/server/web/src/components/__tests__/network-plugin-card.test.tsx index 10eca96c9..bec05a4a5 100644 --- a/apps/server/web/src/components/__tests__/network-plugin-card.test.tsx +++ b/apps/server/web/src/components/__tests__/network-plugin-card.test.tsx @@ -1355,10 +1355,12 @@ describe("NetworkPluginCard: the rules that are not about one state", () => { // 7.6s wall for this act/NDJSON chain at 0.4 CPU (it passes in 25ms // idle). bun's default 5s per-test timeout was firing mid-chain. The // per-wait budgets stay honest; the load allowance is NOT here anymore - - // the package's script carries `--timeout 30000` (issue #261 round 2, - // after CI's serial run measured this file's sibling chain at 26.2 s - // against a 20 s per-test ceiling), and one knob per package is the rule - // the server package's testing-notes record for exactly this whack-a-mole. + // the package's script carries `--timeout 45000` (issue #261 round 2 set it + // to 30000 after CI's serial run measured this file's sibling chain at + // 26.2 s against a 20 s ceiling; the 2026-10-08 serial run timed out THIS + // test at 30.098 s on a larger suite, so the knob moved one notch), and one + // knob per package is the rule the server package's testing-notes record + // for exactly this whack-a-mole. it("a failed act says so", async () => { failedPublishThenLeave(); await renderCard(JOINED); @@ -1399,10 +1401,11 @@ describe("NetworkPluginCard: the rules that are not about one state", () => { const dialog = await screen.findByRole("dialog", {}, { timeout: 1200 }); fireEvent.click(within(dialog).getByRole("button", { name: "Disconnect" })); await waitFor(() => expect(screen.queryByText(/the daemon went away/)).toBeNull(), { timeout: 1200 }); - // No per-test ceiling: the package's 30 s script budget governs, per the + // No per-test ceiling: the package's 45 s script budget governs, per the // rationale on the test above. This one runs the full fail-then-succeed // chain - measured 12-16 s on CI's slice when the 20 s ceiling was set, - // 26.2 s on 2026-09-30's, which is what moved it onto the package knob. + // 26.2 s on 2026-09-30's, and it crossed 30 s (timed out at 30.098 s) on + // 2026-10-08's larger suite, the two events that moved the knob 30 to 45. }); it("uses the vendor's own words for the credential and for publishing", async () => { From feb17daaec429cd7bb38e9fdec3908440747d67d Mon Sep 17 00:00:00 2001 From: Theo Gravity Date: Thu, 8 Oct 2026 01:08:34 -0700 Subject: [PATCH 14/16] test(web): reset the global fetch to the delegator after every test CI's Test: web job failed on backup-settings with unmocked fetches to happy-dom's 127.0.0.1:80 - a victim file, not the cause. The per-file pattern swaps globalThis.fetch and restores it inline with no afterEach; a mid-test throw skips the restore, the next file that imports a fetch-time binder (better-auth binds at module eval) captures the leftover mock, and the poisoned global walks forward through the serial worker until something reaches the real network. The file already re-parks window.location after every test for the identical cross-file reason; this completes that invariant for fetch so every file's import-time binders see the delegator regardless of ordering. Root cause, not an ordering chase: the full serial suite (2521) is green. Focused: web suite serial 2521 pass; biome + verify-types + prose clean. --- apps/server/web/src/test-setup.ts | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/apps/server/web/src/test-setup.ts b/apps/server/web/src/test-setup.ts index 327c53bd4..83e64cbfa 100644 --- a/apps/server/web/src/test-setup.ts +++ b/apps/server/web/src/test-setup.ts @@ -56,14 +56,40 @@ const underlyingFetch = globalThis.fetch.bind(globalThis) as ( input: RequestInfo | URL, init?: RequestInit, ) => Promise; -globalThis.fetch = ((input: RequestInfo | URL, init?: RequestInit) => +const fetchDelegator = ((input: RequestInfo | URL, init?: RequestInit) => (fetchRouter ?? underlyingFetch)(input, init)) as typeof globalThis.fetch; +globalThis.fetch = fetchDelegator; /** Routes (or, with null, un-routes) the global fetch used by import-time binders. */ export function setFetchRouter(router: FetchRouter): void { fetchRouter = router; } +/** + * Park the global fetch back on the delegator after EVERY test, exactly as the + * `window.location` reset above does for the URL. + * + * The established per-file pattern swaps `globalThis.fetch` at a test's top and + * restores it inline at the bottom, with no `afterEach` (several suites do this, + * including the SSH Connect panel's). A mid-test throw skips that inline + * restore, so a mock is left on the global; the next file to import a fetch-time + * binder (better-auth captures whatever `globalThis.fetch` is at module + * evaluation, the reason the delegator lives here in the first place) then binds + * the leftover mock instead of the delegator, and it stays bound for that + * worker. The poisoned global walks forward through the CI serial run until some + * file's binder reaches the real network and dies as `ECONNREFUSED 127.0.0.1:80` + * (happy-dom's default origin) — a victim file and one or more workers away from + * whichever test leaked. The URL has carried this exact reset since 2026-09-17 + * for the same cross-file reason; the delegator is installed for that reason + * too, so the reset belongs here, at the boundary, rather than chasing the one + * file whose ordering happened to trip it. A `beforeEach` that sets the global + * still wins (it runs after this, before the next test), and a file that swaps + * inside its own body is untouched until the boundary — which is the point. + */ +afterEach(() => { + globalThis.fetch = fetchDelegator; +}); + const globals = globalThis as Record; // `__BUILD_ID__` is a Vite `define`: the real build inlines it at compile // time, so under bun test the global simply is not there. It is typed as a From fe0197be9beb79ad1b9f43d9cb9e4a2fa5156170 Mon Sep 17 00:00:00 2001 From: Theo Gravity Date: Thu, 8 Oct 2026 01:24:58 -0700 Subject: [PATCH 15/16] test(web): reviewer - attribute the budget history and the fetch leak correctly Two comment-accuracy fixes from review. The 30.098s timeout belongs to the fail-then-succeed test below the block, not the one the block annotates, and only the 2026-10-08 event moved 30 to 45 (26.2s moved 20 to 30). In test-setup the poisoned-global narrative said 'happy-dom's default origin' and 'one or more workers'; the suite registers its window at WINDOW_URL (localhost -> :80, where a relative fetch lands) and CI's serial regime is one process, so the victim is several files down the same worker. Comments only; no behavior change. Focused: the affected files pass serially. --- .../__tests__/network-plugin-card.test.tsx | 12 ++++++------ apps/server/web/src/test-setup.ts | 17 +++++++++-------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/apps/server/web/src/components/__tests__/network-plugin-card.test.tsx b/apps/server/web/src/components/__tests__/network-plugin-card.test.tsx index bec05a4a5..7b0d68bfc 100644 --- a/apps/server/web/src/components/__tests__/network-plugin-card.test.tsx +++ b/apps/server/web/src/components/__tests__/network-plugin-card.test.tsx @@ -1357,10 +1357,10 @@ describe("NetworkPluginCard: the rules that are not about one state", () => { // per-wait budgets stay honest; the load allowance is NOT here anymore - // the package's script carries `--timeout 45000` (issue #261 round 2 set it // to 30000 after CI's serial run measured this file's sibling chain at - // 26.2 s against a 20 s ceiling; the 2026-10-08 serial run timed out THIS - // test at 30.098 s on a larger suite, so the knob moved one notch), and one - // knob per package is the rule the server package's testing-notes record - // for exactly this whack-a-mole. + // 26.2 s against a 20 s ceiling; the 2026-10-08 serial run timed out the + // fail-then-succeed test BELOW at 30.098 s on a larger suite, moving the + // knob one more notch), and one knob per package is the rule the server + // package's testing-notes record for exactly this whack-a-mole. it("a failed act says so", async () => { failedPublishThenLeave(); await renderCard(JOINED); @@ -1404,8 +1404,8 @@ describe("NetworkPluginCard: the rules that are not about one state", () => { // No per-test ceiling: the package's 45 s script budget governs, per the // rationale on the test above. This one runs the full fail-then-succeed // chain - measured 12-16 s on CI's slice when the 20 s ceiling was set, - // 26.2 s on 2026-09-30's, and it crossed 30 s (timed out at 30.098 s) on - // 2026-10-08's larger suite, the two events that moved the knob 30 to 45. + // 26.2 s on 2026-09-30's (which moved the knob 20 to 30), and it crossed + // 30 s (timed out at 30.098 s) on 2026-10-08's larger suite (30 to 45). }); it("uses the vendor's own words for the credential and for publishing", async () => { diff --git a/apps/server/web/src/test-setup.ts b/apps/server/web/src/test-setup.ts index 83e64cbfa..6898e8a54 100644 --- a/apps/server/web/src/test-setup.ts +++ b/apps/server/web/src/test-setup.ts @@ -72,14 +72,15 @@ export function setFetchRouter(router: FetchRouter): void { * The established per-file pattern swaps `globalThis.fetch` at a test's top and * restores it inline at the bottom, with no `afterEach` (several suites do this, * including the SSH Connect panel's). A mid-test throw skips that inline - * restore, so a mock is left on the global; the next file to import a fetch-time - * binder (better-auth captures whatever `globalThis.fetch` is at module - * evaluation, the reason the delegator lives here in the first place) then binds - * the leftover mock instead of the delegator, and it stays bound for that - * worker. The poisoned global walks forward through the CI serial run until some - * file's binder reaches the real network and dies as `ECONNREFUSED 127.0.0.1:80` - * (happy-dom's default origin) — a victim file and one or more workers away from - * whichever test leaked. The URL has carried this exact reset since 2026-09-17 + * restore, so a mock is left on the global; a file evaluated after it that + * imports a fetch-time binder (better-auth captures whatever `globalThis.fetch` + * is at module evaluation, the reason the delegator lives here in the first + * place) then binds the leftover mock instead of the delegator. The poisoned + * global walks forward through the CI serial run until some later file's binder + * reaches the real network and dies as `ECONNREFUSED 127.0.0.1:80` — the + * `WINDOW_URL` this file registers the happy-dom window at, which is where a + * relative fetch lands — a victim file several positions down the same worker + * from whichever test leaked. The URL has carried this exact reset since 2026-09-17 * for the same cross-file reason; the delegator is installed for that reason * too, so the reset belongs here, at the boundary, rather than chasing the one * file whose ordering happened to trip it. A `beforeEach` that sets the global From 389dff323864b50b6fa7cd6960ab9cf3a0bec815 Mon Sep 17 00:00:00 2001 From: Theo Gravity Date: Sun, 11 Oct 2026 01:37:38 -0700 Subject: [PATCH 16/16] test(ssh): align fixtures with current main --- .../__tests__/subshell-view-ssh.test.ts | 18 +++++------------- .../connect/__tests__/connect-panel.test.tsx | 1 + .../__tests__/saved-hosts-section.test.tsx | 1 + .../nodes/__tests__/node-section-nav.test.tsx | 1 + 4 files changed, 8 insertions(+), 13 deletions(-) diff --git a/apps/server/api/src/api/subshells/__tests__/subshell-view-ssh.test.ts b/apps/server/api/src/api/subshells/__tests__/subshell-view-ssh.test.ts index 64bede2ab..edbd37d11 100644 --- a/apps/server/api/src/api/subshells/__tests__/subshell-view-ssh.test.ts +++ b/apps/server/api/src/api/subshells/__tests__/subshell-view-ssh.test.ts @@ -122,20 +122,12 @@ describe("ssh panes in subshell views: the non-owner access downgrade", () => { expect(got.ssh).toBe(false); }); - it("the admin's effective edit on a foreign ssh pane reads as view too (the boost is not ownership)", async () => { + it("admins need a grant to see foreign SSH and ordinary panes", async () => { const cookie = await signIn(adminEmail, pw); - try { - const sshRow = await get("/s_ssh", cookie); - expect(sshRow.access).toBe("view"); - expect(sshRow.ssh).toBe(true); - // Same admin, ordinary pane: still the resolved `edit` (pinning that the - // downgrade is keyed to the ssh column, not to the viewer's role). - const plain = await get("/s_plain", cookie); - expect(plain.access).toBe("edit"); - expect(plain.ssh).toBe(false); - } finally { - await deleteUserByEmailOrId(adminEmail); - } + expect((await req("GET", "/s_ssh", cookie)).status).toBe(404); + expect((await req("GET", "/s_plain", cookie)).status).toBe(404); + const rows = (await (await req("GET", "", cookie)).json()) as { id: string }[]; + expect(rows.some((row) => row.id === "s_ssh" || row.id === "s_plain")).toBe(false); }); it("the LIST path downgrades the same way (one helper, both doors; the MCP list rides this route)", async () => { diff --git a/apps/server/web/src/components/connect/__tests__/connect-panel.test.tsx b/apps/server/web/src/components/connect/__tests__/connect-panel.test.tsx index 2533c07b9..659c0ffc7 100644 --- a/apps/server/web/src/components/connect/__tests__/connect-panel.test.tsx +++ b/apps/server/web/src/components/connect/__tests__/connect-panel.test.tsx @@ -40,6 +40,7 @@ function node(overrides: Partial): Node { protocolVersion: null, access: "owner", canManage: true, + canRetire: true, canLaunch: true, allowedDirs: [], capabilities: [], diff --git a/apps/server/web/src/components/connect/__tests__/saved-hosts-section.test.tsx b/apps/server/web/src/components/connect/__tests__/saved-hosts-section.test.tsx index 9cbc84dd5..1def92046 100644 --- a/apps/server/web/src/components/connect/__tests__/saved-hosts-section.test.tsx +++ b/apps/server/web/src/components/connect/__tests__/saved-hosts-section.test.tsx @@ -43,6 +43,7 @@ function node(overrides: Partial): Node { protocolVersion: null, access: "owner", canManage: true, + canRetire: true, canLaunch: true, allowedDirs: [], capabilities: [], diff --git a/apps/server/web/src/components/nodes/__tests__/node-section-nav.test.tsx b/apps/server/web/src/components/nodes/__tests__/node-section-nav.test.tsx index 881b24fe9..b6101c03d 100644 --- a/apps/server/web/src/components/nodes/__tests__/node-section-nav.test.tsx +++ b/apps/server/web/src/components/nodes/__tests__/node-section-nav.test.tsx @@ -34,6 +34,7 @@ function navNode(overrides: Partial = {}): NodeDetail { access: "owner", canManage: true, canRetire: true, + sshEnabled: false, canLaunch: true, allowedDirs: [], capabilities: [],