Workspaces 4/10: dor workspace verbs - #617
Conversation
Every fresh Window installed the lib's `DEFAULT_WORKSPACE_ID`, so a second window opened after the first one closed wrote a blob naming a Workspace id that is already live in another window's blob; a relaunch then met the same id twice and the whole restore was refused. A fresh standalone Window now mints its own id; a bare Wall — one Window's whole application — keeps the default. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QBf4rQbwuxV6n6v7E3twYe
Adds `WORKSPACE_CONTROL_METHODS` beside the Surface methods, the `surface.list` scope param, and the window-level handler behind them: one row per Workspace with its union status, background create, rename/switch/close resolving `workspace:<n|name>` (a name only when exactly one Workspace carries it), and a close that refuses running or touched work unless forced rather than raising a prompt no caller can see. Routing grows two steps: `--workspace` now resolves names, and a target named by its stable Surface id is answered by whichever Workspace holds it, since that handle — unlike `surface:N` — is unique across the Window. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QBf4rQbwuxV6n6v7E3twYe
`dor workspace new|rename|close|switch` mutates; `dor list` keeps every read: `--workspace <ref>` narrows to another Workspace, `--all` groups every Workspace's Surfaces under a header, and `--workspaces` prints the overview. Every action command gains `--workspace <ref>`, including the `dor ab` passthrough, which intercepts it beside the identity flags. One command with a leading action rather than a route map: the published CLI reference renders one help page per top-level command, and a nested one would have no page of its own. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QBf4rQbwuxV6n6v7E3twYe
Each Workspace is its own webview there, so a `--all` listing would report one webview's Workspace as the whole Window and a container verb would move a strip that does not exist. The extension host — the only side that knows how many webviews it holds — answers them with what VS Code can and cannot do, and leaves every other request untouched. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QBf4rQbwuxV6n6v7E3twYe
dor-cli.md gains a `dor workspace` section and the promoted Handle Model rules: the name handle and its ambiguity error, the routing order including the Window's own verbs and stable-id targeting, `surface.list`'s scope, and `--workspace` on every action command. Its two staged bullets are gone, and cross-Window targeting — still reserved — is now the Future item the Reserved line points at. The workspaces-rollout scope is empty, so its ledger and every reference to it are retired; layout.md keeps the Workspace model, and vscode.md and standalone.md record the refusal and the minted first Workspace id. Budgets ratcheted: dor-cli 5450, standalone 8350, vscode 7450. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QBf4rQbwuxV6n6v7E3twYe
The flag names a Workspace rather than a browser, so it must reach the two control calls and never the agent-browser binary, which knows nothing about it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QBf4rQbwuxV6n6v7E3twYe
Moves each rule to one home. `dor/src/protocol.ts` gains the wire-only predicates the whole control plane shares — `isWorkspaceControlMethod`, `spansWorkspaces`, `parseWorkspaceRef` — so the router, the `workspace.*` handlers, and the VS Code guard read one enumeration and one ref grammar; the guard becomes "this host serves positional 1, nothing that spans". Caller identity is rewritten at the seam that knows it: `dispatchDorControl` drops a `surfaceId` the answering Wall does not hold, so every `surface:self` / implicit-target consumer is correct by construction rather than re-deriving it. One `classifySurfaceTarget` beside the matcher replaces the router's private copy of the target grammar, and the Wall's `DorControlParams` sheds the Window-level params it never read. Elsewhere: one `WorkspaceScopedRequest` / `WorkspaceScopedFlags` behind the per-command copies, `workspaceFlag` / `workspaceParam` at every call site, one `ACTIONS` table for `dor workspace` in place of four parallel enumerations, one tag trailer shared by both listings, a `switch` with an exhaustiveness check over the container verbs, a parallel `surface.list --all` fan-out, and `resolveWorkspaceRef` / `installWindowPersistence` answering with what their callers were reading back out of the store. Behavior is unchanged; the one help edit is `dor list --workspace` taking the shared flag's wording. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QBf4rQbwuxV6n6v7E3twYe
Ten confirmed findings on the `dor` Workspace CLI, each pinned by a test: - `dor ab --key` is Workspace-scoped for real. `sessionForKey(key, workspaceId)` namespaces a managed key under the Workspace that holds the browser, and the CLI asks the host for the name (`surface.resolveAgentBrowser` with `key`) rather than building `dormouse.1.<key>` itself. A bare Wall — VS Code, the website — keeps the unscoped names, and so does a `dor ab` with no control endpoint at all. - `dor list --all` marks only the active Workspace's selection focused; every Wall marks its own, and the Window has one focus. - A refused Workspace close reveals the Workspace only in `prompt` mode: a `dor workspace close` gets the message and leaves the user where they were. - `--all` fails the listing on a Workspace whose Wall never registers, after the routing retry, instead of dropping it; the text renderer keeps every Workspace's header so it agrees with the JSON `workspaces` array. - `workspace.close` refuses a Workspace with no registered Wall rather than dropping it with its Sessions still running. - An explicit `--workspace` whose Wall has not registered waits out the same retry and then answers "still mounting", not "unknown workspace target". - `--all --ports` scans once for the whole Window: a batched `getOpenPortsMany` through the sidecar (one process table, one socket scan), Rust, both standalone adapters, the browser-dev bridge, and an optional adapter method with a per-id fallback. - `dor workspace`'s CLI test drives one client across all four verbs and asserts the whole ordered conversation. - The VS Code guard accepts its own Workspace by name as well as by position. - notepad.md's Closure section carries the Workspace close path, prompt vs silent. Plus the cleanups: `workspaceRows()` goes through `workspaceRefFor`, `dor list --workspaces` refuses by allowlist so a new flag is refused by default, and standalone.md cites `window-restore.test.ts` by its full path. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QBf4rQbwuxV6n6v7E3twYe
Carries origin/main (#611, #612), the hidden-Workspace minimize, the repeat-quit vote fix, and the arrival deadline up the stack. Only the spec word budgets conflicted. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PkPyEFCxiPo5UFeju5Ya9u
Deploying mouseterm with
|
| Latest commit: |
99161e1
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://621c55a2.mouseterm.pages.dev |
| Branch Preview URL: | https://workspaces-dor.mouseterm.pages.dev |
dormouse-bot
left a comment
There was a problem hiding this comment.
Feedback on work in progress — not a merge verdict. The routing rewrite, the --all fan-out and the VS Code guard all land with tests I'd have asked for, and pnpm lint:specs / lint:public-docs pass on the head once AGENTS.md is read from the PR rather than the base-restored worktree copy. Five points are inline; two spec-drift items sit outside the diff hunks and are below.
docs/specs/layout.md corner case #8 now contradicts the code and notepad.md. It still reads "A refused close reveals its Workspace: a closeAll that returns a refusal activates that Workspace" — unconditional — while revealForPrompt in lib/src/components/wall/workspace-lifecycle.ts reveals only in prompt mode. docs/specs/notepad.md got the qualified version in this PR; layout.md did not, and the corner-case list is the one whose "numbers are stable" for cross-spec citation. The same section's close-refusal enumeration ("the last remaining Workspace cannot be closed", "One close runs at a time") also predates the new NO_WALL_REFUSAL, which the strip and the command-mode & can now hit — it is documented only in dor-cli.md's close row, i.e. in the spec that does not own the strip path.
docs/specs/dor-tool.md links a scope this PR deleted. Its "Dehydrate and rehydrate" section points at "docs/specs/layout.md → Future, workspaces-rollout", and **Scope: workspaces-rollout** is gone from layout.md's ## Future as of this PR. It is the only remaining hit in docs/, and spec-lint's scope check does not catch it: SCOPE_REF_RE matches only the bolded forms, so this prose reference reads as fine forever. docs/specs/layout.md → Workspaces is the live target.
…ough the registration gap Review findings on the workspaces-dor branch: - macListeningPorts threw away the stdout lsof printed before a non-zero exit, so one descendant exiting between `ps` and `lsof` emptied a whole Window's `dor list --all --ports`. It now parses `err.stdout`, the shape getCwdsForPids already documents. The batched `pty_get_open_ports_many` reused the per-terminal 3 s for the whole Window while the sidecar spends it twice serially; it now waits both scans plus `OPEN_PORT_TIMEOUT_PER_ID_MS` per id, with the sidecar's socket scan capped to match. The new constant is mirrored TS/sidecar/Rust and pinned by mirrored-constants.test.ts. - The router left a request with no Wall mounted to the client's 5 s deadline, which every managed `dor ab` now pays. After the bounded retry it answers the mounting refusal for the active Workspace; `dor ab` reports it and exits non-zero without running the binary, and there is deliberately no fallback to a CLI-namespaced key (it would name the wrong Workspace's browser). - The strip's `×` / command-mode `&` swallowed the no-Wall refusal and did nothing. requestWorkspaceClose now waits out the registration gap the way `dor workspace close` does, and the refusal is one wording everywhere (`mountingRefusal`; NO_WALL_REFUSAL is gone). - `dor list --all --json` adds caller_workspace_ref / focused_workspace_ref, since the `_surface_ref` pair names a `surface:N` every Workspace has. - sessionForKey scrubs the key like the scope: it arrives over the control socket from clients other than `dor`. - Specs: layout.md corner case #8 reveals only in `prompt` mode and the close refusals list the no-Wall case; dor-tool.md links the live Workspaces section instead of the deleted workspaces-rollout scope; dor-browser.md states that every managed `dor ab` depends on the host answering. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RChsJ5rMUMyfu22UZDfUus
Carries the review fixes from the earlier stages up the stack; the two mirrored-constants blocks that met here are kept side by side. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RChsJ5rMUMyfu22UZDfUus
dormouse-bot
left a comment
There was a problem hiding this comment.
Feedback on work in progress — not a merge verdict; mark the PR ready when you want the full one. Every finding from the previous round is genuinely closed: the lsof stdout salvage, the batched budget, the router answering the no-Wall case, the --all workspace pointers, and the key scrub all land with tests, and the layout.md / dor-tool.md spec drift is fixed (pnpm lint:specs and lint:public-docs pass on the head once AGENTS.md is read from the PR rather than the base-restored worktree copy). Two new points, both inline.
The first is a hard failure: the new sessionForKey scrub assertion does not match what the function returns, so pnpm -r run test (which builds dor-lib-common first) goes red. Build & Test is still pending at 70de2a3, so this hasn't surfaced yet.
Two smaller things, neither worth a change on its own:
resolveDorControlRoute's pending and none variants are now handled identically at their only consumer — dor-control-router.ts does if (route.kind === 'none' || route.kind === 'pending') and then reads route.message from either. Nothing outside the tests distinguishes them any more. Worth collapsing them, or keeping both deliberately because the provenance (a named Workspace vs the active fallback) is what a future consumer would branch on.
docs/specs/standalone.md → "Open ports" states the batch budget unconditionally — "the socket scan runs under OPEN_PORT_TIMEOUT_MS + OPEN_PORT_TIMEOUT_PER_ID_MS × ids" — but only macListeningPorts reads scanTimeoutMs. The Windows branch still passes a flat timeout: OPEN_PORT_TIMEOUT_MS to its PowerShell call and Linux reads /proc with no subprocess at all. No bug follows (the Windows scan's cost doesn't grow with the batch — Get-NetTCPConnection lists every connection regardless), but the rule as written is macOS-only, and the sidecar's own comment on OPEN_PORT_TIMEOUT_PER_ID_MS reads the same way.
# Conflicts: # docs/specs/layout.md # lib/src/components/wall/workspace-lifecycle.test.ts # lib/src/components/wall/workspace-lifecycle.ts
dormouse-bot
left a comment
There was a problem hiding this comment.
Feedback on work in progress — not a merge verdict; mark the PR ready when you want the full one.
One new point inline: the deadline this commit matched to both scans is matched only on the Tauri side. The VS Code host runs the same pty-core — pty-host.js requires a shim re-exporting standalone/sidecar/pty-core.js — behind a flat 3 s at two boundaries, so the pane header there can still abandon a scan the host is about to answer. A smaller note on the 1000 literal is inline too.
There was a problem hiding this comment.
Feedback on work in progress — not a merge verdict; mark the PR ready when you want the full one.
Both prior points are closed: pty_get_open_ports now takes open_ports_many_timeout(1), the VS Code child and webview boundaries carry the derived deadline, and the 1000 is a named constant with a mirror pin. The two new tests discriminate the change they pin — 6500 ms fails at hops = 0, 7500 ms fails at hops = 1. pnpm lint:specs and lint:public-docs pass on the head (reading AGENTS.md from the PR rather than the base-restored worktree copy).
Four points inline, the first about the rule itself rather than the arithmetic.
| degrades to an empty list rather than failing the call. | ||
| `--port`) the host scans each terminal Surface's process tree | ||
| (`docs/specs/dor-browser.md` → Dev-Server Chip), shelling out (`lsof` / | ||
| `Get-NetTCPConnection`) under `OPEN_PORT_TIMEOUT_MS`. **One listing costs one |
There was a problem hiding this comment.
This cap is stale for the case the sentence is about. getOpenPortsForPids passes scanTimeoutMs: openPortScanTimeoutMs(roots.length) to the socket scan, and macListeningPorts reads it — so the batched lsof this paragraph describes runs under OPEN_PORT_TIMEOUT_MS + OPEN_PORT_TIMEOUT_PER_ID_MS × terminals, not OPEN_PORT_TIMEOUT_MS. It stays flat only on the Windows branch, which is the half the named cap now describes least well.
Simplest resolution is to drop the claim rather than qualify it per-platform here — the deadline rule now lives in docs/specs/transport.md → "Port scan deadlines" and the sidecar's scan budget in docs/specs/standalone.md, and AGENTS.md wants it stated once in the spec that owns it.
| `Get-NetTCPConnection`) under `OPEN_PORT_TIMEOUT_MS`. **One listing costs one | |
| `Get-NetTCPConnection`). **One listing costs one |
| }); | ||
|
|
||
|
|
||
| describe('port request IPC margin', () => { |
There was a problem hiding this comment.
The constant is pinned but the derivation that uses it is not, and there are now two hand-written copies of it: openPortRequestTimeoutMs here in TS and open_ports_many_timeout in lib.rs. docs/specs/transport.md names both as Source of truth: for one formula, so a change to either — dropping a 2 × once the two scans stop being serial, scaling the margin — leaves the other silently on the old budget and the spec's single formula false for one host. Nothing goes red.
This file already has the shape for it: counts the round-trip margin Rust adds in %s reads Duration::from_millis(timeout + (\d+)) out of each Rust command body and compares it to the quit.ts derivation. The same read over open_ports_many_timeout's body against openPortRequestTimeoutMs would close it, and it belongs in this describe rather than beside the constant pin.
| // Mirrors platform/types.ts; pinned by mirrored-constants.test.ts. | ||
| const OPEN_PORT_ROUND_TRIP_MARGIN_MS: u64 = 1000; | ||
|
|
||
| /// Budget for either port command over `count` ids, including 1 s for IPC. The sidecar runs two |
There was a problem hiding this comment.
Same hand-written margin the last round moved into a constant, one layer up as prose — change OPEN_PORT_ROUND_TRIP_MARGIN_MS and this sentence is wrong with nothing to catch it.
| /// Budget for either port command over `count` ids, including 1 s for IPC. The sidecar runs two | |
| /// Budget for either port command over `count` ids, plus `OPEN_PORT_ROUND_TRIP_MARGIN_MS` for IPC. The sidecar runs two |
dormouse-bot
left a comment
There was a problem hiding this comment.
Feedback on work in progress — not a merge verdict; mark the PR ready when you want the full one.
The deadline gap from the last round is closed on the arithmetic: all three Windows subprocesses now draw on one openPortScanTimeoutMs(count), so the worst case is 2 × OPEN_PORT_TIMEOUT_MS + count × OPEN_PORT_TIMEOUT_PER_ID_MS on every platform, matching what docs/specs/transport.md → "Port scan deadlines" now states. Both new tests discriminate the change — the budget test pins the exact remaining-time sequence [3000, budget, budget − 500, budget − 1500], and the exhaustion test pins that netstat is never spawned. node --test standalone/sidecar/pty-core.test.js is green (101/101), and pnpm lint:specs passes on the head once AGENTS.md is read from the PR rather than the base-restored worktree copy.
One point inline is about what sharing the budget does to the fallback order; the second is a naming collision.
One smaller thing, outside the hunks: the netstat call in windowsListeningPorts still passes timeout: OPEN_PORT_TIMEOUT_MS in its options literal, which execFileSyncFn now always overwrites with remaining. It is the one place a reader would look to check the new rule, and it reads as the old flat 3 s — worth deleting so the options object doesn't contradict the comment above it.
dormouse-bot
left a comment
There was a problem hiding this comment.
Feedback on work in progress — not a merge verdict; mark the PR ready when you want the full one.
The reorder closes the ordering point from the last round, and both tests discriminate it: the budget test's [3000, budget, budget − 1000, budget − 1500] sequence only holds if Get-NetTCPConnection draws on the scan allowance first, and the new test pins that a name lookup consuming the entire remainder still returns the port that was already enumerated. node --test standalone/sidecar/pty-core.test.js is green (102/102), and pnpm lint:specs passes on the head once AGENTS.md is read from the PR rather than the base-restored worktree copy.
One point inline, about what the reorder left behind at the two parse calls.
| ports = parseNetTcpConnections(json, pidSet, nameByPid); | ||
| } catch { | ||
| return []; | ||
| try { | ||
| const out = execFileSyncFn('netstat', ['-ano', '-p', 'TCP'], { | ||
| encoding: 'utf-8', | ||
| stdio: ['ignore', 'pipe', 'ignore'], | ||
| windowsHide: true, | ||
| }); | ||
| ports = parseNetstatListening(out, pidSet, nameByPid); |
There was a problem hiding this comment.
nameByPid can no longer hold anything at either parse call — it is built empty on the line above and populated only after both, so processName comes entirely from the .map at the end. Threading it in tells a reader checking whether the netstat path gets names the opposite of what happens, on the one function whose subject is now the order these steps run in. Both parameters default to new Map(), so dropping them is a no-op, and the const nameByPid = new Map() above can move down beside the lookup that fills it.
The same hunk dropped windowsHide's reason on the netstat call. It is the one option here whose deletion reintroduces a deadlock rather than a slowdown, and it went as collateral in a commit that only meant to remove the timeout beside it.
| ports = parseNetTcpConnections(json, pidSet, nameByPid); | |
| } catch { | |
| return []; | |
| try { | |
| const out = execFileSyncFn('netstat', ['-ano', '-p', 'TCP'], { | |
| encoding: 'utf-8', | |
| stdio: ['ignore', 'pipe', 'ignore'], | |
| windowsHide: true, | |
| }); | |
| ports = parseNetstatListening(out, pidSet, nameByPid); | |
| ports = parseNetTcpConnections(json, pidSet); | |
| } catch { | |
| try { | |
| const out = execFileSyncFn('netstat', ['-ano', '-p', 'TCP'], { | |
| encoding: 'utf-8', | |
| stdio: ['ignore', 'pipe', 'ignore'], | |
| windowsHide: true, // see runPowerShell: avoid the console-allocation deadlock | |
| }); | |
| ports = parseNetstatListening(out, pidSet); |
dor workspace new|rename|close|switch,dor list --workspaces|--all,--workspaceon every action command, the VS Code refusal of Workspace-spanning requests.Based on
workspaces-multiwindow.🤖 Generated with Claude Code
https://claude.ai/code/session_01PkPyEFCxiPo5UFeju5Ya9u