From b528b9f9e8d61ebe31ad2989d4f7f12843ce0f75 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 1 Sep 2026 13:37:33 -0700 Subject: [PATCH 1/5] feat(tool): take over the calling pane when the invocation is naked MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `dor tool` typed alone at a prompt now runs in that pane instead of splitting — same Surface, same id, same scrollback. Typing a command at a prompt is how a terminal works; the split placement stays for every other caller (an agent, a script, `--surface`, `--minimize`, a `--cwd` elsewhere). The handshake is the part that needed building: `dor` is the caller's foreground process when the host answers it, so the host answers `takeover` first, waits for the shell to report itself back at a prompt, and only then types the command. Waiting first deadlocks. The spawn lock is held past the response until the command is live, so a second invocation of the same key dedupes against a running tool rather than racing this one. The leaf changes kind through one meta write (`setMeta`), so the component pair and params commit together and the leaf id — the SessionId — never moves. dor-tool.md's word budget goes up 425 words: the take-over design moves out of `## Future` into a `## Take-over` section, plus its rationale. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0166PG9g7V3kZ6Uo9EHrpoTD --- docs/specs/dor-tool.md | 62 ++++++- docs/specs/dor-tool.rationale.md | 6 + docs/specs/tiling-engine.md | 2 +- dor/src/commands/tool.ts | 5 +- dor/src/commands/types.ts | 4 +- dor/test/snapshots/help/tool.md | 5 +- lib/src/components/Wall.test.tsx | 155 ++++++++++++++++++ lib/src/components/wall/lath-wall-store.ts | 12 ++ lib/src/components/wall/tool-takeover.test.ts | 69 ++++++++ lib/src/components/wall/tool-takeover.ts | 73 +++++++++ lib/src/components/wall/use-dor-control.ts | 91 ++++++++-- lib/src/lib/terminal-state.ts | 21 ++- scripts/spec-word-budgets.json | 4 +- 13 files changed, 477 insertions(+), 32 deletions(-) create mode 100644 lib/src/components/wall/tool-takeover.test.ts create mode 100644 lib/src/components/wall/tool-takeover.ts diff --git a/docs/specs/dor-tool.md b/docs/specs/dor-tool.md index b01a358fd..087c5c373 100644 --- a/docs/specs/dor-tool.md +++ b/docs/specs/dor-tool.md @@ -259,9 +259,8 @@ header), `isToolParams` / `toolFace` in `browser-surface.ts`, [serving](#serving) trigger. - **`dor tool `** — run a `dormouse.yml` entry with whatever `prespawn_dedupe` it declares. -- **Always splits focus-neutrally** and returns a handle. Taking over the - calling pane when a human types the invocation alone at a prompt is designed - but not built — see [Future](#future). +- **Splits focus-neutrally** and returns a handle, except when it + [takes over the calling pane](#take-over). - **A keyed invocation that matches reveals and reports**, in both placements, so the calling pane never appears to do nothing. - `dor list`: rows report `kind: tool` with `render_mode`; JSON carries command @@ -270,6 +269,57 @@ header), `isToolParams` / `toolFace` in `browser-surface.ts`, Source of truth: `dor/src/commands/tool.ts` and its help snapshot `dor/test/snapshots/help/tool.md`; `surface.tool` in `dor/src/protocol.ts`. +## Take-over + +**`dor tool` typed alone at a prompt runs in that pane** rather than splitting — +same Surface, same id, same scrollback. Typing a command at a prompt is how a +terminal works. Nothing else about the invocation changes: same trust gate, same +dedupe, same serving trigger. + +Every condition holds or it splits, and a split is never wrong — only more panes +than were asked for (rationale): + +- **The line is naked**: the caller's OSC 633 command line is one command and + that command is `dor tool`. An agent's invocation runs under whatever it + launched, so the pane reports *that* line instead. Human intent, never a + security boundary — see [Trust](#trust) rule 2. +- **The caller is a visible pane whose leaf is a plain terminal.** A Door is not + a pane a human is typing in; a tool or browser leaf is not one to transform. +- **The tool's directory is that pane's own.** The command is typed into the + caller's shell and runs where that shell already is, so a `--cwd` naming + anywhere else has to spawn its own. +- **The placement was not asked for.** `--surface` names a split reference and + `--minimize` asks for a background Surface. +- **A pending approval never takes over**, and neither does a key match: the + first needs a pane that has spawned nothing ([Trust](#trust) rule 3), the + second reveals its survivor ([CLI](#cli)). + +**Respond, then wait for the prompt.** `dor` is the caller's foreground process +when the host answers it, so the host answers `takeover` first, waits for the +shell to report itself back at a prompt, and types the command only then. +Waiting first deadlocks: the prompt cannot return until `dor` exits, and `dor` +cannot exit until it is answered. + +- **The response promises the placement, not the run** — it is sent before the + command is typed, because the caller is gone by the time it runs. Failure + after it shows in the pane. +- **A shell that never comes back to its prompt is left alone**: nothing typed, + leaf still a terminal. The transformation happens on the way *in* to typing, + so a timeout costs nothing. +- **The spawn lock is held past the response** until the command is live, so a + second invocation of the same key dedupes against a running tool instead of + racing this one. +- **The transformation is one meta write.** Component pair and params commit + together — a body swapped ahead of its params would render a tool whose params + are still a terminal's — and the leaf id, which is the SessionId, never + changes. That is what keeps the terminal, its buffer, and its PTY untouched. +- Accepted: **keystrokes in the window between `dor` exiting and the command + landing** interleave with it. The window is one control round trip. + +Source of truth: `lib/src/components/wall/tool-takeover.ts` (the gate), the +take-over arm of `surface.tool` in `lib/src/components/wall/use-dor-control.ts` +(the handshake), `setMeta` in `lib/src/components/wall/lath-wall-store.ts`. + ## OSC 367 `DOR` on a phone keypad. Verb-multiplexed (the OSC 633 pattern): one registry @@ -376,12 +426,6 @@ Source of truth: `PersistedSurfaceType` in `lib/src/lib/session-types.ts`; `DORMOUSE_DEHYDRATE`; the `dehydrate` flag is reserved in the serve payload from the shipped `serve` payload. The Windows graceful-stop is needed here only. -- **Pane take-over.** `dor tool` typed alone at a prompt should run in that - pane rather than splitting — typing a command at a prompt is how a terminal - works. The gate is three conditions the host can already read (sole command on - the OSC 633 line, pane at a prompt, pane not already a tool); what it needs is - the handshake, since `dor` is itself the foreground process when it answers, - so the command can only be typed once its own shell returns to a prompt. - **The announced `name`.** Wire the reserved [OSC 367](#osc-367) `name` into the title-candidates channel and `dor list`'s location column. - **Later** — `prespawn_*` beyond the dedupe literal: a computed key, and diff --git a/docs/specs/dor-tool.rationale.md b/docs/specs/dor-tool.rationale.md index 7a10440c4..be3328d0c 100644 --- a/docs/specs/dor-tool.rationale.md +++ b/docs/specs/dor-tool.rationale.md @@ -42,6 +42,12 @@ **What the announcement is still needed for.** Multi-port tools. `pnpm dev:standalone:ab` binds vite, the dev bridge, and the sidecar's control socket, and no scan can guess which one to frame. ssh is the other case: the control socket does not exist across it, and neither does the host's view of the remote process tree. +## Take-over + +**Why the gate is conservative in the split direction.** Every condition can be read wrong in two directions, and the two costs are nowhere near equal. Declining a take-over that should have happened costs a pane the user closes — the tool still runs, in the placement `dor tool` has always used. Taking over a pane that should have split types a command into a shell that belongs to something else: an agent's session, a line with work queued behind `dor`, a directory the tool was not asked to run in. So each condition is written to fail closed, and quoting is not unpicked — a line carrying `&&` inside quotes splits rather than being parsed for whether that `&&` is real. + +**Why the naked test is worth having at all, given `dor send`.** It answers "did a human ask for this *here*", not "is this trustworthy". The discrimination it actually makes is placement: an agent's `dor tool` runs under the agent's own command line, so the pane reports `claude` (or `bash script.sh`) and never matches — which is the whole point, since an agent's tool must not commandeer the pane the human is watching the agent in. Trust is a separate gate with a separate ceremony, and it is the one that carries the security weight. + ## Security **Why the content-driven announce risk is accepted.** The blast radius is the containment rule applied to ports: an announce reveals and frames, never transferring input authority, grants, or state. The iframe proxy dials upstream as a fresh client with no browser cookie authority, and the link-local/cloud-metadata SSRF guard stands regardless. Two properties of this design narrow it further than an announce-triggered one: the scan supplies the port, so an announced port that nothing bound frames nothing at all, and a runtime re-key cannot dedupe, so it cannot reach another pane. diff --git a/docs/specs/tiling-engine.md b/docs/specs/tiling-engine.md index 3ab569973..c455d34d0 100644 --- a/docs/specs/tiling-engine.md +++ b/docs/specs/tiling-engine.md @@ -117,7 +117,7 @@ A **parked** leaf is mounted by the adapter but absent from the split tree: its - **Detaching and parking are separate things.** `doorLeaf` takes a leaf out of the tree and *keeps its meta* — what every minimize does, terminal or browser, because the store stays the authority for a Doored Surface's live title/params. `{ park: true }` additionally keeps the leaf **mounted** (the browser-only part). `removeLeaf` destroys a leaf and its meta (a kill); `forgetLeaf` destroys a Door, unmounting it if parked. A Surface **born minimized** — `dor split` / `dor ensure` targeting another Door, with no pane to detach — registers its meta through `addDoor`, so the "one map holds every leaf" invariant has no exception for creation path. - **Parking must be one commit.** An id absent from both the tree and `parked` for even one render would make React unmount the leaf and lose the DOM state, so every op that re-admits a leaf (`addLeaf`, `restoreLeaf`, `insertLeaf`, `replaceLeaf`, `seed`) unparks it in that same commit through the one shared `admit` helper — which also seeds the enter hint, so an op added later cannot honor half the contract. **`seed` admits by tree membership**, not by the metadata it is handed: hydration passes Door rows alongside the tree's leaves, and a parked id appearing only as a Door row is still a Door — unparking it would unmount the very DOM the park preserves. Dormant while `seed` runs once at startup; live in the workspaces-rollout switch. -- **`leafMeta` covers Doors.** One map holds every leaf the Wall owns, laid out or Doored; `parked` is pure render state (`Map`) naming the subset that keeps its DOM. Detachment is a fact about the *tree*, so **no Door record carries a metadata copy that can go stale**: `setTitle` / `updateParams` reach a Doored leaf by the same single path as a visible one, and every reader — reattach, `dor` param lookup, kill/session teardown, `buildDorSurfaces`, `dor list`, the dev-server port scan, the session save — goes through `lath.getMeta(id)`. `serializeLayout` filters `leafMeta` down to the tree's own leaves, because the persisted *layout* is the tree — a Door persists as its own row. +- **`leafMeta` covers Doors.** One map holds every leaf the Wall owns, laid out or Doored; `parked` is pure render state (`Map`) naming the subset that keeps its DOM. Detachment is a fact about the *tree*, so **no Door record carries a metadata copy that can go stale**: `setTitle` / `updateParams` / `setMeta` — the last one replacing a leaf's whole meta in one commit, so a leaf can change kind in place without changing id (`docs/specs/dor-tool.md` → Take-over) — reach a Doored leaf by the same single path as a visible one, and every reader — reattach, `dor` param lookup, kill/session teardown, `buildDorSurfaces`, `dor list`, the dev-server port scan, the session save — goes through `lath.getMeta(id)`. `serializeLayout` filters `leafMeta` down to the tree's own leaves, because the persisted *layout* is the tree — a Door persists as its own row. - **The store holds the last rect.** `doorLeaf({ park: true })` captures the leaf's current layout rect into `parked` in the same commit that removes it from the tree; LathHost unions parked ids into the same sorted leaf list and renders them at that rect with `visibility: hidden; pointer-events: none` and `data-lath-parked`, so the guest document never sees a zero-extent viewport and reattach is pixel-identical (rationale). A leaf parked before the Wall reports geometry falls back to the whole wall rect. **Keep the rect in the store, never in the adapter**: `registerEl(null)` is a ref detach, not an unmount, and React detaches whenever a callback identity changes and on every StrictMode commit — adapter-local pruning on detach silently lost every parked rect. On re-admission `admit` replays the held rect into the animator, for every admitting op and every drop target (Animation → Enter). - **Visibility is a real signal.** A parked leaf is on screen only in the DOM sense, so `PaneProps.parked` carries it to the body and `useSurfaceVisibility(parked)` folds it together with document visibility: a minimized `ab-screencast` stays mounted and connected but stops pulling frames. - **Who parks**: `shouldParkOnMinimize` — browser Surfaces, not terminals (a terminal's state is in the PTY and the registry replays it, so parking one would only cost memory). Both still door through `doorLeaf`. diff --git a/dor/src/commands/tool.ts b/dor/src/commands/tool.ts index ff250f731..0bdd633ee 100644 --- a/dor/src/commands/tool.ts +++ b/dor/src/commands/tool.ts @@ -117,13 +117,16 @@ A dormouse.yml is repo-controlled and its entries execute, so it is inert until Approving an upstream covers every worktree and clone of that repo. Approving a folder covers that checkout only, which is what you want for a branch you have not read. -Where the tool lands: it always splits without taking focus and prints the new surface's handle, whether a human typed it or a script did. Taking over the calling pane when the invocation is typed alone at a prompt is designed but not built. +Where the tool lands: typed alone at a prompt, it takes over the pane you typed it in — no split, same surface, same scrollback — and reports "takeover". Anything else splits without taking focus and prints the new surface's handle. The take-over needs an integrated shell running \`dor tool\` as the whole command line, a plain terminal pane you can see, and the tool's directory to be that pane's own, so an agent's invocation, a compound line, --minimize, --surface, and --cwd elsewhere all split instead. + +Take-over answers before the tool starts: \`dor\` is the pane's foreground process, so the command can only be typed once dor itself has exited and the shell is back at a prompt. --cwd sets the working directory used to find dormouse.yml and to run the command; it defaults to the directory dor was invoked from. Text output: created surface:3 "pnpm storybook" existing surface:3 "pnpm storybook" + takeover surface:1 "pnpm storybook" JSON output: { diff --git a/dor/src/commands/types.ts b/dor/src/commands/types.ts index 571a95e43..0649606d9 100644 --- a/dor/src/commands/types.ts +++ b/dor/src/commands/types.ts @@ -172,8 +172,10 @@ export interface ToolSurfaceResponse { * `existing` is a key match on a live tool: the redundant spawn never * started. `adopted` is a key match whose command had exited — the Surface is * reused and the command re-run in place, keeping its position and scrollback. + * `takeover` is the calling pane itself becoming the tool, answered before the + * command is typed — `dor` has to exit before its own shell is free to run it. */ - status: 'created' | 'existing' | 'adopted' | 'pending'; + status: 'created' | 'existing' | 'adopted' | 'pending' | 'takeover'; surfaceId: string; surfaceRef: string; /** The rendered command, as typed into the shell. */ diff --git a/dor/test/snapshots/help/tool.md b/dor/test/snapshots/help/tool.md index 0ceb96d8c..7e2263908 100644 --- a/dor/test/snapshots/help/tool.md +++ b/dor/test/snapshots/help/tool.md @@ -20,13 +20,16 @@ A dormouse.yml is repo-controlled and its entries execute, so it is inert until Approving an upstream covers every worktree and clone of that repo. Approving a folder covers that checkout only, which is what you want for a branch you have not read. -Where the tool lands: it always splits without taking focus and prints the new surface's handle, whether a human typed it or a script did. Taking over the calling pane when the invocation is typed alone at a prompt is designed but not built. +Where the tool lands: typed alone at a prompt, it takes over the pane you typed it in — no split, same surface, same scrollback — and reports "takeover". Anything else splits without taking focus and prints the new surface's handle. The take-over needs an integrated shell running `dor tool` as the whole command line, a plain terminal pane you can see, and the tool's directory to be that pane's own, so an agent's invocation, a compound line, --minimize, --surface, and --cwd elsewhere all split instead. + +Take-over answers before the tool starts: `dor` is the pane's foreground process, so the command can only be typed once dor itself has exited and the shell is back at a prompt. --cwd sets the working directory used to find dormouse.yml and to run the command; it defaults to the directory dor was invoked from. Text output: created surface:3 "pnpm storybook" existing surface:3 "pnpm storybook" + takeover surface:1 "pnpm storybook" JSON output: { diff --git a/lib/src/components/Wall.test.tsx b/lib/src/components/Wall.test.tsx index 43b95fd01..2e48d1540 100644 --- a/lib/src/components/Wall.test.tsx +++ b/lib/src/components/Wall.test.tsx @@ -1353,6 +1353,161 @@ describe('Wall on the Lath engine', () => { } }); + // Pane take-over: `dor tool` typed alone at a prompt runs in that pane rather + // than splitting (docs/specs/dor-tool.md -> Take-over). The handshake is the + // point — `dor` is the pane's foreground process when the host answers, so the + // command may only be typed once its own shell is back at a prompt. + it('takes over the calling pane when `dor tool` is typed alone at a prompt', async () => { + setToolsEnabled(true); + const typed: string[] = []; + (fake as FakePtyAdapter & Pick).toolControl = vi.fn(async () => ({ + status: 'ok' as const, + projectRoot: '/repo', + path: '/repo/dormouse.yml', + name: 'storybook', + run: 'pnpm storybook', + render: 'iframe' as const, + port: 'announced' as const, + key: ['/repo'], + warnings: [], + })); + + try { + await act(async () => { + root.render(); + }); + await flush(); + act(() => fake.spawnPty('pane-a')); + fake.setInputHandler('pane-a', (data) => typed.push(data)); + terminalRegistry.seedTerminalManualCwd('pane-a', '/repo'); + terminalRegistry.applyTerminalSemanticEvents('pane-a', [ + { type: 'commandLine', commandLine: 'dor tool storybook' }, + { type: 'commandStart', source: 'osc633_boundaries' }, + ]); + + let response: { ok: boolean; result?: { status: string; surfaceId: string; minimized: boolean } } | undefined; + await act(async () => { + window.dispatchEvent(new CustomEvent('dormouse:control-request', { + detail: { + method: SURFACE_CONTROL_METHODS.tool, + surfaceId: 'pane-a', + params: { name: 'storybook', cwd: '/repo', minimized: false, fresh: false }, + respond: (result: typeof response) => { response = result; }, + }, + })); + }); + await flush(); + + // Answered before the tool starts, and nothing typed while `dor` still owns + // the shell: waiting for the prompt first would deadlock. + expect(response).toMatchObject({ + ok: true, + result: { status: 'takeover', surfaceId: 'pane-a', minimized: false }, + }); + expect(leafCount()).toBe(1); + expect(typed).toEqual([]); + + // `dor` exits; the shell reports its prompt back and the command lands. + act(() => { + terminalRegistry.applyTerminalSemanticEvents('pane-a', [{ type: 'promptStart' }]); + }); + await act(async () => { await new Promise((r) => setTimeout(r, 250)); }); + expect(typed).toEqual(['pnpm storybook\r']); + expect(leafCount()).toBe(1); + + // Same Surface, now a tool: the leaf changed kind without changing id, so + // the session persists as one. + act(() => { + terminalRegistry.applyTerminalSemanticEvents('pane-a', [ + { type: 'commandLine', commandLine: 'pnpm storybook' }, + { type: 'commandStart', source: 'osc633_boundaries' }, + ]); + }); + // The spawn lock is held until the command is live; let the handler see it. + await act(async () => { await new Promise((r) => setTimeout(r, 250)); }); + await act(async () => { window.dispatchEvent(new Event('pagehide')); }); + await flush(); + await flush(); + const saved = fake.getState() as { + panes?: Array<{ id: string; surfaceType?: string; command?: string }>; + } | null; + expect(saved?.panes?.find((pane) => pane.id === 'pane-a')).toMatchObject({ + surfaceType: 'tool', + command: 'pnpm storybook', + }); + } finally { + fake.clearInputHandler('pane-a'); + act(() => terminalRegistry.removeTerminalPaneState('pane-a')); + setToolsEnabled(false); + } + }); + + it('splits instead of taking over when the caller is running something else', async () => { + setToolsEnabled(true); + const typed: string[] = []; + (fake as FakePtyAdapter & Pick).toolControl = vi.fn(async () => ({ + status: 'ok' as const, + projectRoot: '/repo', + path: '/repo/dormouse.yml', + name: 'storybook', + run: 'pnpm storybook', + render: 'iframe' as const, + port: 'announced' as const, + key: null, + warnings: [], + })); + let splitId: string | undefined; + + try { + await act(async () => { + root.render(); + }); + await flush(); + act(() => fake.spawnPty('pane-a')); + fake.setInputHandler('pane-a', (data) => typed.push(data)); + terminalRegistry.seedTerminalManualCwd('pane-a', '/repo'); + // An agent's `dor tool` runs under the agent, so the pane reports that line. + terminalRegistry.applyTerminalSemanticEvents('pane-a', [ + { type: 'commandLine', commandLine: 'claude' }, + { type: 'commandStart', source: 'osc633_boundaries' }, + ]); + + let response: { ok: boolean; result?: { status: string; surfaceId: string } } | undefined; + await act(async () => { + window.dispatchEvent(new CustomEvent('dormouse:control-request', { + detail: { + method: SURFACE_CONTROL_METHODS.tool, + surfaceId: 'pane-a', + params: { name: 'storybook', cwd: '/repo', minimized: false, fresh: false }, + respond: (result: typeof response) => { response = result; }, + }, + })); + }); + await flush(); + // The split exists before its handle is reported: a created tool answers + // only once the new shell reports OSC 633. + expect(leafCount()).toBe(2); + splitId = Array.from(container.querySelectorAll('[data-lath-leaf]')) + .map((leaf) => leaf.getAttribute('data-lath-leaf')!) + .find((id) => id !== 'pane-a'); + act(() => { + terminalRegistry.applyTerminalSemanticEvents(splitId!, [{ type: 'promptStart' }]); + }); + await act(async () => { await new Promise((r) => setTimeout(r, 250)); }); + + expect(response?.result).toMatchObject({ status: 'created', surfaceId: splitId }); + expect(typed).toEqual([]); + } finally { + if (splitId) { + pendingShellOpts.delete(splitId); + act(() => terminalRegistry.removeTerminalPaneState(splitId!)); + } + fake.clearInputHandler('pane-a'); + act(() => terminalRegistry.removeTerminalPaneState('pane-a')); + setToolsEnabled(false); + } + }); + it('rejects a non-integrated shell before offering tool approval', async () => { setToolsEnabled(true); terminalRegistry.setDefaultShellOpts({ shell: 'C:\\Windows\\System32\\cmd.exe' }); diff --git a/lib/src/components/wall/lath-wall-store.ts b/lib/src/components/wall/lath-wall-store.ts index 7826bf093..fd41f8983 100644 --- a/lib/src/components/wall/lath-wall-store.ts +++ b/lib/src/components/wall/lath-wall-store.ts @@ -137,6 +137,13 @@ export type LathWallStore = { /** Meta write: merge `patch` into a leaf's params. No-op if the leaf is absent. * Reaches parked and cap-evicted leaves too. */ updateParams(id: LeafId, patch: Record): void; + /** Meta write: replace a leaf's whole meta — component pair, title and params + * in ONE commit. The in-place kind change behind the `dor tool` take-over + * (`docs/specs/dor-tool.md` -> Take-over): the id never moves, so the Session + * and its scrollback are untouched. One commit because a body swapped ahead + * of its params would render a tool whose params are still a terminal's. + * No-op if the leaf is absent. */ + setMeta(id: LeafId, meta: LeafMeta): void; /** Presentation-only zoom target (the tree is untouched). No-op if unchanged. */ setZoomed(id: LeafId | null): void; @@ -462,6 +469,11 @@ export function createLathWallStore(): LathWallStore { commit({ leafMeta: new Map(snapshot.leafMeta).set(id, { ...cur, params }) }); }, + setMeta(id, meta) { + if (!snapshot.leafMeta.has(id)) return; + commit({ leafMeta: new Map(snapshot.leafMeta).set(id, meta) }); + }, + setZoomed(id) { if (snapshot.zoomedId === id) return; commit({ zoomedId: id }); diff --git a/lib/src/components/wall/tool-takeover.test.ts b/lib/src/components/wall/tool-takeover.test.ts new file mode 100644 index 000000000..10ee56162 --- /dev/null +++ b/lib/src/components/wall/tool-takeover.test.ts @@ -0,0 +1,69 @@ +import { describe, expect, it } from 'vitest'; +import { isNakedToolInvocation, toolTakesOverCaller, type ToolTakeoverGate } from './tool-takeover'; + +describe('isNakedToolInvocation', () => { + it('accepts a `dor tool` line typed on its own', () => { + expect(isNakedToolInvocation('dor tool storybook')).toBe(true); + expect(isNakedToolInvocation(' dor tool storybook ')).toBe(true); + expect(isNakedToolInvocation('dor tool -- pnpm storybook')).toBe(true); + expect(isNakedToolInvocation('dor tool --fresh storybook')).toBe(true); + expect(isNakedToolInvocation('/usr/local/bin/dor tool storybook')).toBe(true); + expect(isNakedToolInvocation('C:\\tools\\dor.cmd tool storybook')).toBe(true); + }); + + it('rejects a line that is not a bare `dor tool`', () => { + expect(isNakedToolInvocation(null)).toBe(false); + expect(isNakedToolInvocation('')).toBe(false); + expect(isNakedToolInvocation('dor')).toBe(false); + expect(isNakedToolInvocation('dor split')).toBe(false); + expect(isNakedToolInvocation('dortool storybook')).toBe(false); + // The agent case: `dor tool` runs under whatever the pane is running. + expect(isNakedToolInvocation('claude')).toBe(false); + expect(isNakedToolInvocation('bash deploy.sh')).toBe(false); + }); + + it('rejects anything that could be more than one command', () => { + expect(isNakedToolInvocation('dor tool storybook && pnpm build')).toBe(false); + expect(isNakedToolInvocation('dor tool storybook; echo done')).toBe(false); + expect(isNakedToolInvocation('dor tool storybook | tee log')).toBe(false); + expect(isNakedToolInvocation('dor tool storybook &')).toBe(false); + expect(isNakedToolInvocation('dor tool storybook > log')).toBe(false); + expect(isNakedToolInvocation('echo $(dor tool storybook)')).toBe(false); + // Quoting is not unpicked: a conservative split beats parsing for intent. + expect(isNakedToolInvocation('dor tool -- sh -c "a && b"')).toBe(false); + }); +}); + +describe('toolTakesOverCaller', () => { + const passing: ToolTakeoverGate = { + callerId: 'pane-a', + explicitSurface: false, + minimized: false, + visible: true, + component: 'terminal', + oscDriven: true, + rawCommandLine: 'dor tool storybook', + cwdMatches: true, + }; + + it('takes over the pane the invocation was typed in', () => { + expect(toolTakesOverCaller(passing)).toBe(true); + }); + + it('splits when any condition fails', () => { + const splits: Array<[string, Partial]> = [ + ['no caller (dor ran outside Dormouse)', { callerId: undefined }], + ['--surface named a reference', { explicitSurface: true }], + ['--minimize asked for a background surface', { minimized: true }], + ['the caller is minimized', { visible: false }], + ['the caller is already a tool', { component: 'tool' }], + ['the caller is a browser', { component: 'browser' }], + ['the shell reports no OSC 633', { oscDriven: false }], + ['the line is not naked', { rawCommandLine: 'claude' }], + ['--cwd named another directory', { cwdMatches: false }], + ]; + for (const [why, override] of splits) { + expect(toolTakesOverCaller({ ...passing, ...override }), why).toBe(false); + } + }); +}); diff --git a/lib/src/components/wall/tool-takeover.ts b/lib/src/components/wall/tool-takeover.ts new file mode 100644 index 000000000..ca722031f --- /dev/null +++ b/lib/src/components/wall/tool-takeover.ts @@ -0,0 +1,73 @@ +/** + * The take-over gate: `dor tool` typed alone at a prompt runs the tool in that + * pane instead of splitting (`docs/specs/dor-tool.md` -> Take-over). + * + * Pure predicates over facts the host has already read, so the placement rule + * is testable without a Wall: the handler in `use-dor-control.ts` gathers the + * facts, this decides, and the handshake that follows is the handler's. + */ + +/** Basenames the staged `dor` shim answers to, plus the Windows spellings. */ +const DOR_ARGV0 = new Set(['dor', 'dor.cmd', 'dor.exe', 'dor.bat']); + +/** + * Shell syntax that can make one line more than one command: separators, + * pipelines, backgrounding, redirection, and substitution. Anything here and + * the line is not naked, even quoted — a `dor tool -- sh -c "a && b"` that + * splits is a wrong *placement*, where typing into a shell that still has work + * queued behind `dor` is a wrong *command*. + */ +const COMPOUND_SYNTAX = /[;&|<>()`\n\r]/; + +/** + * Whether the shell reported running exactly one command and that command is + * `dor tool`. This is the human-intent signal: an agent's `dor tool` runs under + * whatever it launched (`claude`, `bash script.sh`), which reports that line + * instead. It is **not** a security boundary — `dor send` can type bytes + * identical to a human's (`docs/specs/dor-tool.md` -> Trust rule 2). + */ +export function isNakedToolInvocation(rawCommandLine: string | null | undefined): boolean { + const line = rawCommandLine?.trim(); + if (!line || COMPOUND_SYNTAX.test(line)) return false; + const tokens = line.split(/\s+/); + const argv0 = tokens[0].replace(/^["']|["']$/g, '').split(/[\\/]/).pop()?.toLowerCase(); + return !!argv0 && DOR_ARGV0.has(argv0) && tokens[1] === 'tool'; +} + +/** What the placement rule reads. Every field is already known to the handler. */ +export interface ToolTakeoverGate { + /** The pane `dor` ran in (`DORMOUSE_SURFACE_ID`); undefined off a Dormouse shell. */ + callerId: string | undefined; + /** `--surface`: an explicit placement, which take-over must not override. */ + explicitSurface: boolean; + /** `--minimize`: a request for a background Surface, which the caller is not. */ + minimized: boolean; + /** Whether the caller is a visible pane of the active Workspace. */ + visible: boolean; + /** The caller leaf's body component — only a plain `terminal` may transform. */ + component: string | undefined; + /** Whether the caller's shell reports OSC 633. */ + oscDriven: boolean; + /** The command line the caller's shell reports running, or null. */ + rawCommandLine: string | null; + /** Whether the tool's resolved cwd is the caller pane's own directory. */ + cwdMatches: boolean; +} + +/** + * Whether this `dor tool` runs in its calling pane. Every condition below is + * conservative: failing one is a split, which is always a correct outcome. + */ +export function toolTakesOverCaller(gate: ToolTakeoverGate): boolean { + if (!gate.callerId || gate.explicitSurface || gate.minimized) return false; + // A minimized caller cannot be the pane a human is typing in, and taking one + // over would run the tool where nobody can see it. + if (!gate.visible) return false; + if (gate.component !== 'terminal') return false; + // The command is typed into the caller's own shell, so it runs in that + // shell's directory: a `--cwd` naming anywhere else has to spawn one. + if (!gate.cwdMatches) return false; + // Both the naked test and the prompt-return handshake read integration-driven + // state, so a shell that reports none can never take over. + return gate.oscDriven && isNakedToolInvocation(gate.rawCommandLine); +} diff --git a/lib/src/components/wall/use-dor-control.ts b/lib/src/components/wall/use-dor-control.ts index fd03549e3..42b2d1f81 100644 --- a/lib/src/components/wall/use-dor-control.ts +++ b/lib/src/components/wall/use-dor-control.ts @@ -21,7 +21,12 @@ import { isPaneOscDriven, resolveTerminalSessionId, } from '../../lib/terminal-registry'; -import { surfaceRunsCommand, type TerminalPaneState } from '../../lib/terminal-state'; +import { + cwdPathsEqual, + surfaceRunsCommand, + UNNAMED_PANEL_TITLE, + type TerminalPaneState, +} from '../../lib/terminal-state'; import { hostPathDisplay } from './browser-url'; import { agentBrowserSessionFromParams, @@ -34,6 +39,7 @@ import { // One-way import: connect-port no longer depends on this module (its eager-surface // and refresh seams are injected as plain functions). import { connectPortToDefaultBrowser } from './connect-port'; +import { toolTakesOverCaller } from './tool-takeover'; import { listenerUrlsByPort } from './port-url'; import { dorDirectionForEdge, toolLeafMeta, type LathWallEngine } from './lath-wall-engine'; import type { WallNav } from './keyboard/types'; @@ -251,6 +257,12 @@ const RESTART_POLL_INTERVAL_MS = 100; const RESTART_INTERRUPT_TIMEOUT_MS = 15_000; const RESTART_START_TIMEOUT_MS = 15_000; +// The take-over handshake's half of the same idea: `dor` exits as soon as it +// has printed its handle, so the prompt is back within a round trip. Generous +// anyway — the cost of waiting is nothing, and a shell still busy after this is +// one whose pane we must leave alone. +const TAKEOVER_PROMPT_TIMEOUT_MS = 15_000; + /** * Serializes `surface.tool` requests. A plain promise chain rather than a real * mutex: the critical section is "check for a key match, then create", and the @@ -961,6 +973,73 @@ export function useDorControl({ } } + const toolParams = { + surfaceType: 'tool', + command, + cwd, + toolRender: render, + toolPort: port, + ...(key ? { toolKey: key } : {}), + ...(toolName ? { toolName } : {}), + }; + + // Take-over: typed alone at a prompt, the tool runs in the calling + // pane rather than splitting (docs/specs/dor-tool.md -> Take-over). + const callerId = detail.surfaceId; + const callerMeta = callerId ? lath.getMeta(callerId) : undefined; + if (callerId && toolTakesOverCaller({ + callerId, + explicitSurface: stringParam(params.surface) !== undefined, + minimized: booleanParam(params.minimized), + visible: nav.hasPane(callerId), + component: callerMeta?.component, + oscDriven: isPaneOscDriven(callerId), + rawCommandLine: getTerminalPaneState(callerId).currentCommand?.rawCommandLine ?? null, + cwdMatches: cwdPathsEqual(getTerminalPaneState(callerId).cwd?.path, cwd), + })) { + detail.respond({ + ok: true, + result: { + status: 'takeover', + surfaceId: callerId, + surfaceRef: surfaceRefForId(callerId), + command, + cwd, + minimized: false, + key, + ...(warnings.length > 0 ? { warnings } : {}), + }, + }); + // The handshake. `dor` is this pane's foreground process until the + // response above lets it exit, so the command can only be typed once + // its own shell is back at a prompt — respond first, then wait. A + // shell that never returns (or a pane killed while we wait) is left + // exactly as it was: nothing typed, still a plain terminal. + const backAtPrompt = await waitForTerminalState( + callerId, + (state) => state.currentCommand === null, + TAKEOVER_PROMPT_TIMEOUT_MS, + ); + const meta = lath.getMeta(callerId); + if (!backAtPrompt || !meta) return; + // A rename the user made outlives the transformation; an untouched + // fallback title becomes the tool's, as a spawned one would be. + const title = meta.title === UNNAMED_PANEL_TITLE ? (toolName ?? command) : meta.title; + // One meta write, not a params patch plus a component swap: the + // Session and its scrollback stay put while the leaf changes kind. + lath.store.setMeta(callerId, toolLeafMeta(title, toolParams)); + getPlatform().writePty(callerId, `${command}\r`); + // Still inside the spawn lock, held until the command is live so a + // second invocation of the same key dedupes against a running tool + // rather than racing this one into a duplicate. + await waitForTerminalState( + callerId, + (state) => surfaceRunsCommand(state, command, cwd), + RESTART_START_TIMEOUT_MS, + ); + return; + } + // A tool is a shell-hosted PTY with the command typed into it, exactly // as `dor ensure` spawns one — but with no command+cwd matching, and a // leaf that renders both capabilities. @@ -980,15 +1059,7 @@ export function useDorControl({ // Focus-neutral like `dor ensure`: a tool spawned by a script or an // agent must not steal the caller's selection. focusNeutral: true, - leafMeta: toolLeafMeta(toolName ?? command, { - surfaceType: 'tool', - command, - cwd, - toolRender: render, - toolPort: port, - ...(key ? { toolKey: key } : {}), - ...(toolName ? { toolName } : {}), - }), + leafMeta: toolLeafMeta(toolName ?? command, toolParams), }); if (!created.ok) { detail.respond({ ok: false, error: created.message }); diff --git a/lib/src/lib/terminal-state.ts b/lib/src/lib/terminal-state.ts index 862c7bc1b..ce41abdd4 100644 --- a/lib/src/lib/terminal-state.ts +++ b/lib/src/lib/terminal-state.ts @@ -441,6 +441,19 @@ function canonicalizeCwdForMatch(path: string): string { return unified.charAt(0).toUpperCase() + unified.slice(1); } +/** + * Whether two reported paths name the same directory. The CLI sends a + * path.resolve'd cwd (trailing slashes, `..`, `.` collapsed), so the only + * remaining divergence to bridge is the Windows/MSYS dialect split (see + * canonicalizeCwdForMatch). Symlinks and true case differences are still + * treated as distinct, matching the exact-key intent. A missing path on either + * side never matches. + */ +export function cwdPathsEqual(a: string | null | undefined, b: string | null | undefined): boolean { + if (!a || !b) return false; + return canonicalizeCwdForMatch(a) === canonicalizeCwdForMatch(b); +} + /** * The idempotency predicate for `dor ensure`: true when the pane is *currently * running* `command` in `cwdPath`. It matches only while the command is live @@ -457,13 +470,7 @@ export function surfaceRunsCommand( const run = state.currentCommand; if (!run || run.rawCommandLine === null) return false; if (run.rawCommandLine !== command) return false; - // The CLI sends a path.resolve'd cwd (trailing slashes, `..`, `.` collapsed), - // so the only remaining divergence to bridge is the Windows/MSYS dialect split - // (see canonicalizeCwdForMatch). Symlinks and true case differences are still - // treated as distinct, matching the exact-key intent. - const runCwd = run.cwdAtStart?.path ?? state.cwd?.path; - if (runCwd === undefined) return false; - return canonicalizeCwdForMatch(runCwd) === canonicalizeCwdForMatch(cwdPath); + return cwdPathsEqual(run.cwdAtStart?.path ?? state.cwd?.path, cwdPath); } export function deriveFallbackCommandTitle( diff --git a/scripts/spec-word-budgets.json b/scripts/spec-word-budgets.json index f7b0102a4..d277bf7d2 100644 --- a/scripts/spec-word-budgets.json +++ b/scripts/spec-word-budgets.json @@ -13,8 +13,8 @@ "docs/specs/dor-browser.rationale.md": 650, "docs/specs/dor-cli.md": 6050, "docs/specs/dor-cli.rationale.md": 600, - "docs/specs/dor-tool.md": 3350, - "docs/specs/dor-tool.rationale.md": 1560, + "docs/specs/dor-tool.md": 3775, + "docs/specs/dor-tool.rationale.md": 1775, "docs/specs/glossary.md": 3325, "docs/specs/layout.md": 9150, "docs/specs/layout.rationale.md": 700, From fe9907b667b134faa410ce54ab91e19fba4ba702 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 1 Sep 2026 13:54:29 -0700 Subject: [PATCH 2/5] simplify(tool): apply /simplify findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reuse the shared command parser (`commandArgv0` + a new `primaryCommandTokens` export) instead of a second whitespace tokenizer that did not know about quoting, and classify the caller with `surfaceKindFromParams` rather than reading `leafMeta.component` — the codebase's one params-level kind switch. Extract the handshake as `takeOverPaneWithTool` next to its sibling `restartSurfaceInPlace`, read the caller's terminal state once, and drop the gate's `callerId` field (the call site proves it). The spawn lock now releases once the pane is the tool rather than once the command is live: the key reaches the leaf's params in that write, so the extra 15s wait only pinned a module-global lock — every other `dor tool` in the app queued behind it. `RESTART_INTERRUPT_TIMEOUT_MS` and the take-over's own prompt timeout were the same 15s wait on the same predicate, now one `PROMPT_RETURN_TIMEOUT_MS`. Trim the restatements: the handshake and one-meta-write rationale live in dor-tool.md, with pointers at the code; the help text keeps only what a user can act on. Tests poll instead of sleeping a fixed 250ms. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0166PG9g7V3kZ6Uo9EHrpoTD --- docs/specs/dor-tool.md | 13 ++- dor/src/commands/tool.ts | 4 +- dor/test/snapshots/help/tool.md | 4 +- lib/src/components/Wall.test.tsx | 21 ++--- lib/src/components/wall/lath-wall-store.ts | 10 +-- lib/src/components/wall/tool-takeover.test.ts | 12 +-- lib/src/components/wall/tool-takeover.ts | 56 ++++++------ lib/src/components/wall/use-dor-control.ts | 90 ++++++++++--------- lib/src/lib/terminal-state.ts | 12 ++- 9 files changed, 114 insertions(+), 108 deletions(-) diff --git a/docs/specs/dor-tool.md b/docs/specs/dor-tool.md index 087c5c373..4b0d03d4a 100644 --- a/docs/specs/dor-tool.md +++ b/docs/specs/dor-tool.md @@ -306,13 +306,12 @@ cannot exit until it is answered. - **A shell that never comes back to its prompt is left alone**: nothing typed, leaf still a terminal. The transformation happens on the way *in* to typing, so a timeout costs nothing. -- **The spawn lock is held past the response** until the command is live, so a - second invocation of the same key dedupes against a running tool instead of - racing this one. -- **The transformation is one meta write.** Component pair and params commit - together — a body swapped ahead of its params would render a tool whose params - are still a terminal's — and the leaf id, which is the SessionId, never - changes. That is what keeps the terminal, its buffer, and its PTY untouched. +- **The spawn lock is held past the response** until the pane is the tool: the + key reaches the leaf's params in the same write, and until it does a second + invocation of that key would not dedupe. +- **The transformation is one meta write**, so the component pair and the params + commit together, and the leaf id — the SessionId — never changes. That is what + keeps the terminal, its buffer, and its PTY untouched. - Accepted: **keystrokes in the window between `dor` exiting and the command landing** interleave with it. The window is one control round trip. diff --git a/dor/src/commands/tool.ts b/dor/src/commands/tool.ts index 0bdd633ee..c8c8d17e0 100644 --- a/dor/src/commands/tool.ts +++ b/dor/src/commands/tool.ts @@ -117,9 +117,7 @@ A dormouse.yml is repo-controlled and its entries execute, so it is inert until Approving an upstream covers every worktree and clone of that repo. Approving a folder covers that checkout only, which is what you want for a branch you have not read. -Where the tool lands: typed alone at a prompt, it takes over the pane you typed it in — no split, same surface, same scrollback — and reports "takeover". Anything else splits without taking focus and prints the new surface's handle. The take-over needs an integrated shell running \`dor tool\` as the whole command line, a plain terminal pane you can see, and the tool's directory to be that pane's own, so an agent's invocation, a compound line, --minimize, --surface, and --cwd elsewhere all split instead. - -Take-over answers before the tool starts: \`dor\` is the pane's foreground process, so the command can only be typed once dor itself has exited and the shell is back at a prompt. +Where the tool lands: typed alone at a prompt, it takes over the pane you typed it in — no split, same surface, same scrollback — and reports "takeover". Anything else splits without taking focus and prints the new surface's handle. The take-over needs an integrated shell running \`dor tool\` as the whole command line, a plain terminal pane you can see, and the tool's directory to be that pane's own, so an agent's invocation, a compound line, --minimize, --surface, and --cwd elsewhere all split instead. The handle prints before the command starts, since dor has to exit before its own shell is free to run it. --cwd sets the working directory used to find dormouse.yml and to run the command; it defaults to the directory dor was invoked from. diff --git a/dor/test/snapshots/help/tool.md b/dor/test/snapshots/help/tool.md index 7e2263908..b0bd8d738 100644 --- a/dor/test/snapshots/help/tool.md +++ b/dor/test/snapshots/help/tool.md @@ -20,9 +20,7 @@ A dormouse.yml is repo-controlled and its entries execute, so it is inert until Approving an upstream covers every worktree and clone of that repo. Approving a folder covers that checkout only, which is what you want for a branch you have not read. -Where the tool lands: typed alone at a prompt, it takes over the pane you typed it in — no split, same surface, same scrollback — and reports "takeover". Anything else splits without taking focus and prints the new surface's handle. The take-over needs an integrated shell running `dor tool` as the whole command line, a plain terminal pane you can see, and the tool's directory to be that pane's own, so an agent's invocation, a compound line, --minimize, --surface, and --cwd elsewhere all split instead. - -Take-over answers before the tool starts: `dor` is the pane's foreground process, so the command can only be typed once dor itself has exited and the shell is back at a prompt. +Where the tool lands: typed alone at a prompt, it takes over the pane you typed it in — no split, same surface, same scrollback — and reports "takeover". Anything else splits without taking focus and prints the new surface's handle. The take-over needs an integrated shell running `dor tool` as the whole command line, a plain terminal pane you can see, and the tool's directory to be that pane's own, so an agent's invocation, a compound line, --minimize, --surface, and --cwd elsewhere all split instead. The handle prints before the command starts, since dor has to exit before its own shell is free to run it. --cwd sets the working directory used to find dormouse.yml and to run the command; it defaults to the directory dor was invoked from. diff --git a/lib/src/components/Wall.test.tsx b/lib/src/components/Wall.test.tsx index 2e48d1540..9c4f5014c 100644 --- a/lib/src/components/Wall.test.tsx +++ b/lib/src/components/Wall.test.tsx @@ -83,6 +83,15 @@ async function flush(): Promise { await act(async () => { await new Promise((r) => setTimeout(r, 0)); }); } +/** Poll until `ready()` — for the host's own 100ms state waits (a tool taking + * over a pane, a split waiting on OSC 633), which no event can flush. */ +async function settle(ready: () => boolean, timeoutMs = 2_000): Promise { + const deadline = Date.now() + timeoutMs; + while (!ready() && Date.now() < deadline) { + await act(async () => { await new Promise((r) => setTimeout(r, 25)); }); + } +} + async function flushFrame(): Promise { await act(async () => { await new Promise((r) => requestAnimationFrame(() => r(undefined))); }); } @@ -1411,20 +1420,12 @@ describe('Wall on the Lath engine', () => { act(() => { terminalRegistry.applyTerminalSemanticEvents('pane-a', [{ type: 'promptStart' }]); }); - await act(async () => { await new Promise((r) => setTimeout(r, 250)); }); + await settle(() => typed.length > 0); expect(typed).toEqual(['pnpm storybook\r']); expect(leafCount()).toBe(1); // Same Surface, now a tool: the leaf changed kind without changing id, so // the session persists as one. - act(() => { - terminalRegistry.applyTerminalSemanticEvents('pane-a', [ - { type: 'commandLine', commandLine: 'pnpm storybook' }, - { type: 'commandStart', source: 'osc633_boundaries' }, - ]); - }); - // The spawn lock is held until the command is live; let the handler see it. - await act(async () => { await new Promise((r) => setTimeout(r, 250)); }); await act(async () => { window.dispatchEvent(new Event('pagehide')); }); await flush(); await flush(); @@ -1493,7 +1494,7 @@ describe('Wall on the Lath engine', () => { act(() => { terminalRegistry.applyTerminalSemanticEvents(splitId!, [{ type: 'promptStart' }]); }); - await act(async () => { await new Promise((r) => setTimeout(r, 250)); }); + await settle(() => response !== undefined); expect(response?.result).toMatchObject({ status: 'created', surfaceId: splitId }); expect(typed).toEqual([]); diff --git a/lib/src/components/wall/lath-wall-store.ts b/lib/src/components/wall/lath-wall-store.ts index fd41f8983..78e73bf11 100644 --- a/lib/src/components/wall/lath-wall-store.ts +++ b/lib/src/components/wall/lath-wall-store.ts @@ -137,12 +137,10 @@ export type LathWallStore = { /** Meta write: merge `patch` into a leaf's params. No-op if the leaf is absent. * Reaches parked and cap-evicted leaves too. */ updateParams(id: LeafId, patch: Record): void; - /** Meta write: replace a leaf's whole meta — component pair, title and params - * in ONE commit. The in-place kind change behind the `dor tool` take-over - * (`docs/specs/dor-tool.md` -> Take-over): the id never moves, so the Session - * and its scrollback are untouched. One commit because a body swapped ahead - * of its params would render a tool whose params are still a terminal's. - * No-op if the leaf is absent. */ + /** Meta write: **replace** a leaf's whole meta in one commit, dropping + * anything the caller does not hand back — component pair included, which is + * what lets a leaf change kind without changing id (`docs/specs/dor-tool.md` + * -> Take-over). No-op if the leaf is absent. */ setMeta(id: LeafId, meta: LeafMeta): void; /** Presentation-only zoom target (the tree is untouched). No-op if unchanged. */ diff --git a/lib/src/components/wall/tool-takeover.test.ts b/lib/src/components/wall/tool-takeover.test.ts index 10ee56162..9293b945f 100644 --- a/lib/src/components/wall/tool-takeover.test.ts +++ b/lib/src/components/wall/tool-takeover.test.ts @@ -8,7 +8,9 @@ describe('isNakedToolInvocation', () => { expect(isNakedToolInvocation('dor tool -- pnpm storybook')).toBe(true); expect(isNakedToolInvocation('dor tool --fresh storybook')).toBe(true); expect(isNakedToolInvocation('/usr/local/bin/dor tool storybook')).toBe(true); - expect(isNakedToolInvocation('C:\\tools\\dor.cmd tool storybook')).toBe(true); + expect(isNakedToolInvocation('dor.cmd tool storybook')).toBe(true); + // The shared tokenizer skips a leading assignment, as `commandArgv0` does. + expect(isNakedToolInvocation('DEBUG=1 dor tool storybook')).toBe(true); }); it('rejects a line that is not a bare `dor tool`', () => { @@ -36,11 +38,10 @@ describe('isNakedToolInvocation', () => { describe('toolTakesOverCaller', () => { const passing: ToolTakeoverGate = { - callerId: 'pane-a', explicitSurface: false, minimized: false, visible: true, - component: 'terminal', + kind: 'terminal', oscDriven: true, rawCommandLine: 'dor tool storybook', cwdMatches: true, @@ -52,12 +53,11 @@ describe('toolTakesOverCaller', () => { it('splits when any condition fails', () => { const splits: Array<[string, Partial]> = [ - ['no caller (dor ran outside Dormouse)', { callerId: undefined }], ['--surface named a reference', { explicitSurface: true }], ['--minimize asked for a background surface', { minimized: true }], ['the caller is minimized', { visible: false }], - ['the caller is already a tool', { component: 'tool' }], - ['the caller is a browser', { component: 'browser' }], + ['the caller is already a tool', { kind: 'tool' }], + ['the caller is a browser', { kind: 'browser' }], ['the shell reports no OSC 633', { oscDriven: false }], ['the line is not naked', { rawCommandLine: 'claude' }], ['--cwd named another directory', { cwdMatches: false }], diff --git a/lib/src/components/wall/tool-takeover.ts b/lib/src/components/wall/tool-takeover.ts index ca722031f..2666b7662 100644 --- a/lib/src/components/wall/tool-takeover.ts +++ b/lib/src/components/wall/tool-takeover.ts @@ -2,18 +2,20 @@ * The take-over gate: `dor tool` typed alone at a prompt runs the tool in that * pane instead of splitting (`docs/specs/dor-tool.md` -> Take-over). * - * Pure predicates over facts the host has already read, so the placement rule - * is testable without a Wall: the handler in `use-dor-control.ts` gathers the + * Pure predicates over facts the host has already read, so the placement rule is + * testable without a Wall: the handler in `use-dor-control.ts` gathers the * facts, this decides, and the handshake that follows is the handler's. */ +import type { SurfaceKind } from 'dor/commands/types'; +import { commandArgv0, primaryCommandTokens } from '../../lib/terminal-state'; -/** Basenames the staged `dor` shim answers to, plus the Windows spellings. */ -const DOR_ARGV0 = new Set(['dor', 'dor.cmd', 'dor.exe', 'dor.bat']); +/** The launcher names `dor/bin/` ships, lowercased. */ +const DOR_ARGV0 = new Set(['dor', 'dor.cmd']); /** * Shell syntax that can make one line more than one command: separators, - * pipelines, backgrounding, redirection, and substitution. Anything here and - * the line is not naked, even quoted — a `dor tool -- sh -c "a && b"` that + * pipelines, backgrounding, redirection, and substitution. Tested against the + * raw line, so quoting is not unpicked — a `dor tool -- sh -c "a && b"` that * splits is a wrong *placement*, where typing into a shell that still has work * queued behind `dor` is a wrong *command*. */ @@ -29,45 +31,41 @@ const COMPOUND_SYNTAX = /[;&|<>()`\n\r]/; export function isNakedToolInvocation(rawCommandLine: string | null | undefined): boolean { const line = rawCommandLine?.trim(); if (!line || COMPOUND_SYNTAX.test(line)) return false; - const tokens = line.split(/\s+/); - const argv0 = tokens[0].replace(/^["']|["']$/g, '').split(/[\\/]/).pop()?.toLowerCase(); - return !!argv0 && DOR_ARGV0.has(argv0) && tokens[1] === 'tool'; + const argv0 = commandArgv0(line)?.toLowerCase(); + return !!argv0 && DOR_ARGV0.has(argv0) && primaryCommandTokens(line)[1] === 'tool'; } /** What the placement rule reads. Every field is already known to the handler. */ export interface ToolTakeoverGate { - /** The pane `dor` ran in (`DORMOUSE_SURFACE_ID`); undefined off a Dormouse shell. */ - callerId: string | undefined; /** `--surface`: an explicit placement, which take-over must not override. */ explicitSurface: boolean; /** `--minimize`: a request for a background Surface, which the caller is not. */ minimized: boolean; - /** Whether the caller is a visible pane of the active Workspace. */ + /** Whether the caller is a visible pane of the active Workspace — a Door is + * not a pane a human is typing in. */ visible: boolean; - /** The caller leaf's body component — only a plain `terminal` may transform. */ - component: string | undefined; - /** Whether the caller's shell reports OSC 633. */ + /** The caller's Surface kind; only a plain terminal may transform. */ + kind: SurfaceKind; + /** Whether the caller's shell reports OSC 633 — both the naked test and the + * prompt-return handshake read integration-driven state. */ oscDriven: boolean; /** The command line the caller's shell reports running, or null. */ rawCommandLine: string | null; - /** Whether the tool's resolved cwd is the caller pane's own directory. */ + /** Whether the tool's resolved cwd is the caller pane's own directory: the + * command is typed into that shell, so it runs where the shell already is. */ cwdMatches: boolean; } /** - * Whether this `dor tool` runs in its calling pane. Every condition below is - * conservative: failing one is a split, which is always a correct outcome. + * Whether this `dor tool` runs in its calling pane. Every condition is + * conservative: failing one is a split, which is never wrong (rationale). */ export function toolTakesOverCaller(gate: ToolTakeoverGate): boolean { - if (!gate.callerId || gate.explicitSurface || gate.minimized) return false; - // A minimized caller cannot be the pane a human is typing in, and taking one - // over would run the tool where nobody can see it. - if (!gate.visible) return false; - if (gate.component !== 'terminal') return false; - // The command is typed into the caller's own shell, so it runs in that - // shell's directory: a `--cwd` naming anywhere else has to spawn one. - if (!gate.cwdMatches) return false; - // Both the naked test and the prompt-return handshake read integration-driven - // state, so a shell that reports none can never take over. - return gate.oscDriven && isNakedToolInvocation(gate.rawCommandLine); + return !gate.explicitSurface + && !gate.minimized + && gate.visible + && gate.kind === 'terminal' + && gate.cwdMatches + && gate.oscDriven + && isNakedToolInvocation(gate.rawCommandLine); } diff --git a/lib/src/components/wall/use-dor-control.ts b/lib/src/components/wall/use-dor-control.ts index 42b2d1f81..fe399c972 100644 --- a/lib/src/components/wall/use-dor-control.ts +++ b/lib/src/components/wall/use-dor-control.ts @@ -32,6 +32,7 @@ import { agentBrowserSessionFromParams, isAgentBrowserParams, namespacedToolKey, + surfaceKindFromParams, toolKeysEqual, toolPendingFromParams, type ToolPending, @@ -254,15 +255,11 @@ function readSurfaceText(surfaceId: string, lines: number | undefined, scrollbac // terminal state: a command is gone once `currentCommand` clears (commandFinish // → prompt) and back once the surface reports the same command live again. const RESTART_POLL_INTERVAL_MS = 100; -const RESTART_INTERRUPT_TIMEOUT_MS = 15_000; +// How long a shell gets to come back to its prompt — after `dor ensure +// --restart` interrupts a command, or after a taken-over pane's `dor` exits. +const PROMPT_RETURN_TIMEOUT_MS = 15_000; const RESTART_START_TIMEOUT_MS = 15_000; -// The take-over handshake's half of the same idea: `dor` exits as soon as it -// has printed its handle, so the prompt is back within a round trip. Generous -// anyway — the cost of waiting is nothing, and a shell still busy after this is -// one whose pane we must leave alone. -const TAKEOVER_PROMPT_TIMEOUT_MS = 15_000; - /** * Serializes `surface.tool` requests. A plain promise chain rather than a real * mutex: the critical section is "check for a key match, then create", and the @@ -323,7 +320,7 @@ async function restartSurfaceInPlace(id: string, command: string, cwd: string): const interrupted = await waitForTerminalState( id, (state) => state.currentCommand === null, - RESTART_INTERRUPT_TIMEOUT_MS, + PROMPT_RETURN_TIMEOUT_MS, ); if (!interrupted) return { ok: false, message: 'did not return to a prompt after interrupt' }; platform.writePty(id, `${command}\r`); @@ -336,6 +333,30 @@ async function restartSurfaceInPlace(id: string, command: string, cwd: string): return { ok: true, value: undefined }; } +/** + * The take-over handshake (docs/specs/dor-tool.md -> Take-over): `dor` is the + * pane's foreground process until the host answers it, so the command can only + * be typed once its own shell is back at a prompt. A shell that never comes back + * — or a pane killed while we wait — is left exactly as it was. + */ +async function takeOverPaneWithTool( + lath: LathWallEngine, + id: string, + tool: { params: Record; title: string; command: string }, +): Promise { + const backAtPrompt = await waitForTerminalState( + id, + (state) => state.currentCommand === null, + PROMPT_RETURN_TIMEOUT_MS, + ); + const meta = lath.getMeta(id); + if (!backAtPrompt || !meta) return; + // A rename the user made outlives the transformation; an untouched fallback + // title becomes the tool's, as a spawned one would be. + lath.store.setMeta(id, toolLeafMeta(meta.title === UNNAMED_PANEL_TITLE ? tool.title : meta.title, tool.params)); + getPlatform().writePty(id, `${tool.command}\r`); +} + // A `dor ensure -- ` command is typed into the shell programmatically, // which bypasses the keystroke heuristic — so only a shell whose integration // emits OSC 633 boundaries ever reports the command back, which is what makes the @@ -983,20 +1004,23 @@ export function useDorControl({ ...(toolName ? { toolName } : {}), }; - // Take-over: typed alone at a prompt, the tool runs in the calling - // pane rather than splitting (docs/specs/dor-tool.md -> Take-over). + // Take-over: typed alone at a prompt, the tool runs in the calling pane + // rather than splitting (docs/specs/dor-tool.md -> Take-over). Must stay + // below the pending-approval and key-match returns above: both of those + // placements win over this one. const callerId = detail.surfaceId; - const callerMeta = callerId ? lath.getMeta(callerId) : undefined; - if (callerId && toolTakesOverCaller({ - callerId, + const callerState = callerId ? getTerminalPaneState(callerId) : null; + if (callerId && callerState && toolTakesOverCaller({ explicitSurface: stringParam(params.surface) !== undefined, minimized: booleanParam(params.minimized), visible: nav.hasPane(callerId), - component: callerMeta?.component, + kind: surfaceKindFromParams(lath.getMeta(callerId)?.params), oscDriven: isPaneOscDriven(callerId), - rawCommandLine: getTerminalPaneState(callerId).currentCommand?.rawCommandLine ?? null, - cwdMatches: cwdPathsEqual(getTerminalPaneState(callerId).cwd?.path, cwd), + rawCommandLine: callerState.currentCommand?.rawCommandLine ?? null, + cwdMatches: cwdPathsEqual(callerState.cwd?.path, cwd), })) { + // Answered before the tool starts, because answering is what frees + // the shell to run it. detail.respond({ ok: true, result: { @@ -1010,33 +1034,13 @@ export function useDorControl({ ...(warnings.length > 0 ? { warnings } : {}), }, }); - // The handshake. `dor` is this pane's foreground process until the - // response above lets it exit, so the command can only be typed once - // its own shell is back at a prompt — respond first, then wait. A - // shell that never returns (or a pane killed while we wait) is left - // exactly as it was: nothing typed, still a plain terminal. - const backAtPrompt = await waitForTerminalState( - callerId, - (state) => state.currentCommand === null, - TAKEOVER_PROMPT_TIMEOUT_MS, - ); - const meta = lath.getMeta(callerId); - if (!backAtPrompt || !meta) return; - // A rename the user made outlives the transformation; an untouched - // fallback title becomes the tool's, as a spawned one would be. - const title = meta.title === UNNAMED_PANEL_TITLE ? (toolName ?? command) : meta.title; - // One meta write, not a params patch plus a component swap: the - // Session and its scrollback stay put while the leaf changes kind. - lath.store.setMeta(callerId, toolLeafMeta(title, toolParams)); - getPlatform().writePty(callerId, `${command}\r`); - // Still inside the spawn lock, held until the command is live so a - // second invocation of the same key dedupes against a running tool - // rather than racing this one into a duplicate. - await waitForTerminalState( - callerId, - (state) => surfaceRunsCommand(state, command, cwd), - RESTART_START_TIMEOUT_MS, - ); + // Awaited inside the spawn lock: the key reaches the leaf's params in + // there, and until it does a second invocation of it would not dedupe. + await takeOverPaneWithTool(lath, callerId, { + params: toolParams, + title: toolName ?? command, + command, + }); return; } diff --git a/lib/src/lib/terminal-state.ts b/lib/src/lib/terminal-state.ts index ce41abdd4..8a12206a9 100644 --- a/lib/src/lib/terminal-state.ts +++ b/lib/src/lib/terminal-state.ts @@ -384,12 +384,22 @@ export function summarizeCommandLine(raw: string): string { * This is the key WATCHING rules are stored under — see `docs/specs/alert.md`. */ export function commandArgv0(raw: string): string | null { - const commandTokens = takePrimaryCommandTokens(tokenizeCommand(raw.trim())); + const commandTokens = primaryCommandTokens(raw); const command = commandTokens[0]; if (!command) return null; return command.split(/[\\/]/).pop() || null; } +/** + * The tokens of the first command on a line: quote- and escape-aware, truncated + * at the first pipeline/compound boundary, with leading `VAR=value` assignments + * and a leading `env` skipped. `commandArgv0` is this reduced to a program name; + * `dor tool`'s take-over gate reads the verb after it. + */ +export function primaryCommandTokens(raw: string): string[] { + return takePrimaryCommandTokens(tokenizeCommand(raw.trim())); +} + export interface ResolvedCommandStart { rawCommandLine: string | null; displayCommand: string; From 156b3ab96e76cf2944b17dd2df779826eec7023f Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 1 Sep 2026 15:08:05 -0700 Subject: [PATCH 3/5] fix(tool): close the take-over races code review found MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four real ones, all created or sharpened by take-over: - A Session that announced an OSC 367 port under an earlier command handed that port — and that key — to the tool taking its pane over. The announcement is cleared as the leaf changes kind. - A keyed re-invocation from the tool's own pane, which take-over makes the normal place to retype, matched itself and reported `existing` forever: its `dor` is what the shell is running, so the tool read as live. It is idle by construction there, and now re-runs through the same handshake, reported `adopted` — never through `restartSurfaceInPlace`, whose Ctrl+C would kill the `dor` still waiting for the answer. - The gate's facts were read before a wait of up to 15s and never re-checked, so a pane minimized in that window became a Doored tool with a command typed into it. - Releasing the spawn lock at the meta write (the previous /simplify pass) was too early: a pane typed into but not yet reporting reads as an idle tool, which a queued same-key invocation interrupts and retypes. Held until the command is live again, with the reason recorded so it does not get re-simplified. Also documented as an accepted limit: a listener the taken-over shell already owned is in the port scan's process tree, so `port: auto` can frame it or refuse the pair. A split-spawned tool cannot hit it. Skipped: `cwdPathsEqual` rejecting an empty-string cwd where the old inline compare accepted it (no CwdState can hold one — `cwdFrom*` returns null), and folding case on the `tool` verb to match the launcher (stricli parses verbs case-sensitively; the asymmetry is correct, now stated). Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0166PG9g7V3kZ6Uo9EHrpoTD --- docs/specs/dor-tool.md | 25 +++-- lib/src/components/Wall.test.tsx | 55 +++++++++- .../components/wall/lath-wall-store.test.ts | 25 +++++ lib/src/components/wall/tool-takeover.test.ts | 16 ++- lib/src/components/wall/tool-takeover.ts | 39 ++++--- lib/src/components/wall/use-dor-control.ts | 100 +++++++++++++----- scripts/spec-word-budgets.json | 2 +- 7 files changed, 212 insertions(+), 50 deletions(-) diff --git a/docs/specs/dor-tool.md b/docs/specs/dor-tool.md index 4b0d03d4a..30fb31dea 100644 --- a/docs/specs/dor-tool.md +++ b/docs/specs/dor-tool.md @@ -290,9 +290,14 @@ than were asked for (rationale): anywhere else has to spawn its own. - **The placement was not asked for.** `--surface` names a split reference and `--minimize` asks for a background Surface. -- **A pending approval never takes over**, and neither does a key match: the - first needs a pane that has spawned nothing ([Trust](#trust) rule 3), the - second reveals its survivor ([CLI](#cli)). +- **A pending approval never takes over** — it needs a pane that has spawned + nothing ([Trust](#trust) rule 3). +- **A key match reveals its survivor** ([CLI](#cli)) — unless the survivor *is* + the calling pane, the place take-over makes normal to retype in. Its command + cannot be live (its shell is running `dor`), so it is idle by construction and + **re-runs there through the same handshake**, reported `adopted`. Never + through the interrupt-and-retype restart: Ctrl+C would kill the `dor` still + waiting for the answer. **Respond, then wait for the prompt.** `dor` is the caller's foreground process when the host answers it, so the host answers `takeover` first, waits for the @@ -306,14 +311,22 @@ cannot exit until it is answered. - **A shell that never comes back to its prompt is left alone**: nothing typed, leaf still a terminal. The transformation happens on the way *in* to typing, so a timeout costs nothing. -- **The spawn lock is held past the response** until the pane is the tool: the - key reaches the leaf's params in the same write, and until it does a second - invocation of that key would not dedupe. +- **The spawn lock is held past the response** until the command is live. The + key reaches the leaf's params at the meta write, but a pane that has been + typed into and has not yet reported reads as an idle tool, which a queued + invocation of the same key would interrupt and retype. - **The transformation is one meta write**, so the component pair and the params commit together, and the leaf id — the SessionId — never changes. That is what keeps the terminal, its buffer, and its PTY untouched. +- **The Session's [OSC 367](#osc-367) hint is cleared as it transforms.** What + the pane announced under an earlier command is not this tool's, and would + otherwise name its port or re-key it. - Accepted: **keystrokes in the window between `dor` exiting and the command landing** interleave with it. The window is one control round trip. +- Accepted: **a listener the taken-over shell already owned** (a backgrounded + server, an `ssh -L`) is in the [scan](#serving)'s process tree, so `port: auto` + can frame it or refuse the pair as a conflict. A shell that never ran a server + before — every split-spawned tool — cannot hit this. Source of truth: `lib/src/components/wall/tool-takeover.ts` (the gate), the take-over arm of `surface.tool` in `lib/src/components/wall/use-dor-control.ts` diff --git a/lib/src/components/Wall.test.tsx b/lib/src/components/Wall.test.tsx index 9c4f5014c..a49c21584 100644 --- a/lib/src/components/Wall.test.tsx +++ b/lib/src/components/Wall.test.tsx @@ -1424,8 +1424,59 @@ describe('Wall on the Lath engine', () => { expect(typed).toEqual(['pnpm storybook\r']); expect(leafCount()).toBe(1); - // Same Surface, now a tool: the leaf changed kind without changing id, so - // the session persists as one. + // The tool goes live, which releases the spawn lock, and then exits. The + // host learns that from its own 100ms state poll, so the live state has to + // outlast one tick. + act(() => { + terminalRegistry.applyTerminalSemanticEvents('pane-a', [ + { type: 'commandLine', commandLine: 'pnpm storybook' }, + { type: 'commandStart', source: 'osc633_boundaries' }, + ]); + }); + await act(async () => { await new Promise((r) => setTimeout(r, 150)); }); + act(() => { + terminalRegistry.applyTerminalSemanticEvents('pane-a', [{ type: 'promptStart' }]); + }); + + // Retyped in the tool's own pane: a key match on the caller re-runs there + // through the same handshake, never an interrupt — Ctrl+C would kill the + // `dor` still waiting for the answer. + act(() => { + terminalRegistry.applyTerminalSemanticEvents('pane-a', [ + { type: 'commandLine', commandLine: 'dor tool storybook' }, + { type: 'commandStart', source: 'osc633_boundaries' }, + ]); + }); + let rerun: { ok: boolean; result?: { status: string; surfaceId: string } } | undefined; + await act(async () => { + window.dispatchEvent(new CustomEvent('dormouse:control-request', { + detail: { + method: SURFACE_CONTROL_METHODS.tool, + surfaceId: 'pane-a', + params: { name: 'storybook', cwd: '/repo', minimized: false, fresh: false }, + respond: (result: typeof rerun) => { rerun = result; }, + }, + })); + }); + await settle(() => rerun !== undefined); + expect(rerun).toMatchObject({ ok: true, result: { status: 'adopted', surfaceId: 'pane-a' } }); + act(() => { + terminalRegistry.applyTerminalSemanticEvents('pane-a', [{ type: 'promptStart' }]); + }); + await settle(() => typed.length > 1); + expect(typed).toEqual(['pnpm storybook\r', 'pnpm storybook\r']); + expect(leafCount()).toBe(1); + + // Same Surface throughout: the leaf changed kind without changing id, so + // the session persists as one. The live state again outlasts a poll tick, + // so the re-run releases its lock before the next test takes one. + act(() => { + terminalRegistry.applyTerminalSemanticEvents('pane-a', [ + { type: 'commandLine', commandLine: 'pnpm storybook' }, + { type: 'commandStart', source: 'osc633_boundaries' }, + ]); + }); + await act(async () => { await new Promise((r) => setTimeout(r, 150)); }); await act(async () => { window.dispatchEvent(new Event('pagehide')); }); await flush(); await flush(); diff --git a/lib/src/components/wall/lath-wall-store.test.ts b/lib/src/components/wall/lath-wall-store.test.ts index 1c8513b28..a0ab0e897 100644 --- a/lib/src/components/wall/lath-wall-store.test.ts +++ b/lib/src/components/wall/lath-wall-store.test.ts @@ -364,6 +364,31 @@ describe('meta writes', () => { expect(listener).not.toHaveBeenCalled(); }); + // The in-place kind change behind the `dor tool` take-over: one commit, so a + // body never renders against the previous kind's params. + it('setMeta replaces a leaf\'s whole meta, dropping what it is not handed', () => { + const store = seeded(); + store.updateParams('a', { stale: true }); + const before = store.getSnapshot(); + store.setMeta('a', { component: 'tool', tabComponent: 'tool', title: 'storybook', params: { surfaceType: 'tool' } }); + const after = store.getSnapshot(); + expect(after.leafMeta.get('a')).toEqual({ + component: 'tool', + tabComponent: 'tool', + title: 'storybook', + params: { surfaceType: 'tool' }, + }); + expect(before.leafMeta.get('a')?.component).toBe('terminal'); + expect(after.leafMeta).not.toBe(before.leafMeta); + }); + + it('setMeta is a no-op on an absent id', () => { + const store = seeded(); + const before = store.getSnapshot(); + store.setMeta('missing', { component: 'tool', tabComponent: 'tool', title: 'x' }); + expect(store.getSnapshot()).toBe(before); + }); + it('updateParams merges a patch into params', () => { const store = seeded(); store.updateParams('b', { url: 'https://example.com' }); diff --git a/lib/src/components/wall/tool-takeover.test.ts b/lib/src/components/wall/tool-takeover.test.ts index 9293b945f..b3baa2f97 100644 --- a/lib/src/components/wall/tool-takeover.test.ts +++ b/lib/src/components/wall/tool-takeover.test.ts @@ -1,5 +1,10 @@ import { describe, expect, it } from 'vitest'; -import { isNakedToolInvocation, toolTakesOverCaller, type ToolTakeoverGate } from './tool-takeover'; +import { + isNakedToolInvocation, + toolRerunsInCaller, + toolTakesOverCaller, + type ToolTakeoverGate, +} from './tool-takeover'; describe('isNakedToolInvocation', () => { it('accepts a `dor tool` line typed on its own', () => { @@ -66,4 +71,13 @@ describe('toolTakesOverCaller', () => { expect(toolTakesOverCaller({ ...passing, ...override }), why).toBe(false); } }); + + // The two placements are the same conditions over different caller kinds: a + // plain terminal becomes the tool, the tool's own pane re-runs it. + it('re-runs in the caller only when the caller is that tool', () => { + expect(toolRerunsInCaller({ ...passing, kind: 'tool' })).toBe(true); + expect(toolRerunsInCaller(passing)).toBe(false); + expect(toolRerunsInCaller({ ...passing, kind: 'tool', rawCommandLine: 'claude' })).toBe(false); + expect(toolRerunsInCaller({ ...passing, kind: 'tool', visible: false })).toBe(false); + }); }); diff --git a/lib/src/components/wall/tool-takeover.ts b/lib/src/components/wall/tool-takeover.ts index 2666b7662..44c13da38 100644 --- a/lib/src/components/wall/tool-takeover.ts +++ b/lib/src/components/wall/tool-takeover.ts @@ -12,21 +12,16 @@ import { commandArgv0, primaryCommandTokens } from '../../lib/terminal-state'; /** The launcher names `dor/bin/` ships, lowercased. */ const DOR_ARGV0 = new Set(['dor', 'dor.cmd']); -/** - * Shell syntax that can make one line more than one command: separators, - * pipelines, backgrounding, redirection, and substitution. Tested against the - * raw line, so quoting is not unpicked — a `dor tool -- sh -c "a && b"` that - * splits is a wrong *placement*, where typing into a shell that still has work - * queued behind `dor` is a wrong *command*. - */ +/** Shell syntax that can make one line more than one command: separators, + * pipelines, backgrounding, redirection, substitution. Tested against the raw + * line, so quoting is not unpicked (rationale). */ const COMPOUND_SYNTAX = /[;&|<>()`\n\r]/; /** * Whether the shell reported running exactly one command and that command is - * `dor tool`. This is the human-intent signal: an agent's `dor tool` runs under - * whatever it launched (`claude`, `bash script.sh`), which reports that line - * instead. It is **not** a security boundary — `dor send` can type bytes - * identical to a human's (`docs/specs/dor-tool.md` -> Trust rule 2). + * `dor tool` — the human-intent signal, not a security boundary + * (`docs/specs/dor-tool.md` -> Take-over). Case folds on the launcher, which is + * a filename, and not on the verb, which stricli parses case-sensitively. */ export function isNakedToolInvocation(rawCommandLine: string | null | undefined): boolean { const line = rawCommandLine?.trim(); @@ -57,15 +52,29 @@ export interface ToolTakeoverGate { } /** - * Whether this `dor tool` runs in its calling pane. Every condition is - * conservative: failing one is a split, which is never wrong (rationale). + * What both placements below share: a naked invocation in a visible, integrated + * pane whose directory is the tool's. Every condition is conservative — failing + * one is a split, which is never wrong (rationale). */ -export function toolTakesOverCaller(gate: ToolTakeoverGate): boolean { +function callerMayRunTool(gate: ToolTakeoverGate): boolean { return !gate.explicitSurface && !gate.minimized && gate.visible - && gate.kind === 'terminal' && gate.cwdMatches && gate.oscDriven && isNakedToolInvocation(gate.rawCommandLine); } + +/** Whether this `dor tool` transforms its calling pane into the tool. */ +export function toolTakesOverCaller(gate: ToolTakeoverGate): boolean { + return gate.kind === 'terminal' && callerMayRunTool(gate); +} + +/** + * Whether a keyed match on the calling pane re-runs there. The caller is then + * the tool's own Surface — the place take-over makes normal to retype in — and + * its command cannot be live, since its shell is running `dor`. + */ +export function toolRerunsInCaller(gate: ToolTakeoverGate): boolean { + return gate.kind === 'tool' && callerMayRunTool(gate); +} diff --git a/lib/src/components/wall/use-dor-control.ts b/lib/src/components/wall/use-dor-control.ts index fe399c972..2bedaa66b 100644 --- a/lib/src/components/wall/use-dor-control.ts +++ b/lib/src/components/wall/use-dor-control.ts @@ -40,8 +40,9 @@ import { // One-way import: connect-port no longer depends on this module (its eager-surface // and refresh seams are injected as plain functions). import { connectPortToDefaultBrowser } from './connect-port'; -import { toolTakesOverCaller } from './tool-takeover'; +import { toolRerunsInCaller, toolTakesOverCaller, type ToolTakeoverGate } from './tool-takeover'; import { listenerUrlsByPort } from './port-url'; +import { clearToolAnnounce } from '../../lib/tool-announce-store'; import { dorDirectionForEdge, toolLeafMeta, type LathWallEngine } from './lath-wall-engine'; import type { WallNav } from './keyboard/types'; import type { LeafMeta } from '../../lib/lath/persistence'; @@ -342,7 +343,7 @@ async function restartSurfaceInPlace(id: string, command: string, cwd: string): async function takeOverPaneWithTool( lath: LathWallEngine, id: string, - tool: { params: Record; title: string; command: string }, + tool: { params: Record; title: string; command: string; cwd: string }, ): Promise { const backAtPrompt = await waitForTerminalState( id, @@ -350,11 +351,25 @@ async function takeOverPaneWithTool( PROMPT_RETURN_TIMEOUT_MS, ); const meta = lath.getMeta(id); - if (!backAtPrompt || !meta) return; + // Re-checked after the wait, not only before it: the pane can be killed or + // minimized while `dor` exits, and a Door keeps its meta — `store.has` is + // membership of the tree, so it answers both. + if (!backAtPrompt || !meta || !lath.store.has(id)) return; + // Whatever this Session announced under its previous command is not this + // tool's: a stale OSC 367 would hand the new tool that port, or re-key it. + clearToolAnnounce(id); // A rename the user made outlives the transformation; an untouched fallback // title becomes the tool's, as a spawned one would be. lath.store.setMeta(id, toolLeafMeta(meta.title === UNNAMED_PANEL_TITLE ? tool.title : meta.title, tool.params)); getPlatform().writePty(id, `${tool.command}\r`); + // The caller holds the spawn lock until this resolves: until the shell reports + // the command, a queued invocation of the same key reads this pane as idle and + // interrupts what was just typed. + await waitForTerminalState( + id, + (state) => surfaceRunsCommand(state, tool.command, tool.cwd), + RESTART_START_TIMEOUT_MS, + ); } // A `dor ensure -- ` command is typed into the shell programmatically, @@ -945,6 +960,32 @@ export function useDorControl({ } } + const toolParams = { + surfaceType: 'tool', + command, + cwd, + toolRender: render, + toolPort: port, + ...(key ? { toolKey: key } : {}), + ...(toolName ? { toolName } : {}), + }; + + // What both placements below read of the pane `dor` ran in + // (docs/specs/dor-tool.md -> Take-over). + const callerId = detail.surfaceId; + const callerGate = callerId === undefined ? null : ((): ToolTakeoverGate => { + const state = getTerminalPaneState(callerId); + return { + explicitSurface: stringParam(params.surface) !== undefined, + minimized: booleanParam(params.minimized), + visible: nav.hasPane(callerId), + kind: surfaceKindFromParams(lath.getMeta(callerId)?.params), + oscDriven: isPaneOscDriven(callerId), + rawCommandLine: state.currentCommand?.rawCommandLine ?? null, + cwdMatches: cwdPathsEqual(state.cwd?.path, cwd), + }; + })(); + // Spawn-time dedupe, and only for a tool that was given an identity // (docs/specs/dor-tool.md -> Identity and dedupe). if (key && !booleanParam(params.fresh)) { @@ -953,6 +994,34 @@ export function useDorControl({ const match = findSurfaceByParams(matchesToolKey); if (match) { const matchedCommand = toolCommandFromParams(lath.getMeta(match.id)?.params) || command; + // A match that is the calling pane is the tool's own Surface, which + // take-over makes the normal place to retype in. Its command cannot + // be live — `dor` is what its shell is running — so it is idle by + // construction, and it re-runs through the take-over handshake: + // `restartSurfaceInPlace` would fire Ctrl+C into the `dor` still + // waiting for this answer. + if (match.id === callerId && callerGate && toolRerunsInCaller(callerGate)) { + detail.respond({ + ok: true, + result: { + status: 'adopted', + surfaceId: match.id, + surfaceRef: surfaceRefForId(match.id), + command, + cwd, + minimized: false, + key, + ...(warnings.length > 0 ? { warnings } : {}), + }, + }); + await takeOverPaneWithTool(lath, match.id, { + params: toolParams, + title: toolName ?? command, + command, + cwd, + }); + return; + } // A dedicated Surface whose command exited is unambiguously free, // so re-run in place rather than splitting — where `dor ensure`, // aimed at arbitrary shells, would stop matching. @@ -994,31 +1063,11 @@ export function useDorControl({ } } - const toolParams = { - surfaceType: 'tool', - command, - cwd, - toolRender: render, - toolPort: port, - ...(key ? { toolKey: key } : {}), - ...(toolName ? { toolName } : {}), - }; - // Take-over: typed alone at a prompt, the tool runs in the calling pane // rather than splitting (docs/specs/dor-tool.md -> Take-over). Must stay // below the pending-approval and key-match returns above: both of those // placements win over this one. - const callerId = detail.surfaceId; - const callerState = callerId ? getTerminalPaneState(callerId) : null; - if (callerId && callerState && toolTakesOverCaller({ - explicitSurface: stringParam(params.surface) !== undefined, - minimized: booleanParam(params.minimized), - visible: nav.hasPane(callerId), - kind: surfaceKindFromParams(lath.getMeta(callerId)?.params), - oscDriven: isPaneOscDriven(callerId), - rawCommandLine: callerState.currentCommand?.rawCommandLine ?? null, - cwdMatches: cwdPathsEqual(callerState.cwd?.path, cwd), - })) { + if (callerId && callerGate && toolTakesOverCaller(callerGate)) { // Answered before the tool starts, because answering is what frees // the shell to run it. detail.respond({ @@ -1035,11 +1084,12 @@ export function useDorControl({ }, }); // Awaited inside the spawn lock: the key reaches the leaf's params in - // there, and until it does a second invocation of it would not dedupe. + // there, and a queued invocation of it must find a running tool. await takeOverPaneWithTool(lath, callerId, { params: toolParams, title: toolName ?? command, command, + cwd, }); return; } diff --git a/scripts/spec-word-budgets.json b/scripts/spec-word-budgets.json index d277bf7d2..2d4561241 100644 --- a/scripts/spec-word-budgets.json +++ b/scripts/spec-word-budgets.json @@ -13,7 +13,7 @@ "docs/specs/dor-browser.rationale.md": 650, "docs/specs/dor-cli.md": 6050, "docs/specs/dor-cli.rationale.md": 600, - "docs/specs/dor-tool.md": 3775, + "docs/specs/dor-tool.md": 3900, "docs/specs/dor-tool.rationale.md": 1775, "docs/specs/glossary.md": 3325, "docs/specs/layout.md": 9150, From 47af0906cd51908d44b7a1420f10e970f004ef62 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 1 Sep 2026 15:45:31 -0700 Subject: [PATCH 4/5] fix(tool): answer a self-match honestly, and stop the lock outliving a dead tool MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both from review of the last commit. A keyed match on the calling pane that failed the gate fell through to a branch that could only report `existing` and do nothing: with the match being the caller, `idle` reads the live `dor tool` line. Reachable by `cd`-ing inside the tool's pane before retyping, or a compound line. The re-run gate now drops the conditions that only govern placement — the pane already is the tool, so it re-runs in its own directory like an `adopted` match from anywhere else — and a caller that genuinely cannot be typed behind gets an error naming why, not a survivor it is sitting in. A self-match whose command really is live (the tool spawned this `dor`) still reports `existing`. The lock's tail waited for the command to be *live*, which a tool that dies on boot never is between two 100ms samples — a typo'd `dor tool --` pinned the module-global lock for the full 15s with every other `dor tool` queued behind it. It now ends on either outcome, watching for a newly finished run by id rather than by command line, so a previous run of the same command cannot satisfy it early. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0166PG9g7V3kZ6Uo9EHrpoTD --- docs/specs/dor-tool.md | 20 +++- lib/src/components/Wall.test.tsx | 39 ++++++++ lib/src/components/wall/tool-takeover.test.ts | 11 ++- lib/src/components/wall/tool-takeover.ts | 35 ++++--- lib/src/components/wall/use-dor-control.ts | 94 ++++++++++++------- scripts/spec-word-budgets.json | 2 +- 6 files changed, 144 insertions(+), 57 deletions(-) diff --git a/docs/specs/dor-tool.md b/docs/specs/dor-tool.md index 30fb31dea..9edb85b39 100644 --- a/docs/specs/dor-tool.md +++ b/docs/specs/dor-tool.md @@ -294,10 +294,18 @@ than were asked for (rationale): nothing ([Trust](#trust) rule 3). - **A key match reveals its survivor** ([CLI](#cli)) — unless the survivor *is* the calling pane, the place take-over makes normal to retype in. Its command - cannot be live (its shell is running `dor`), so it is idle by construction and + is live only when the tool spawned this `dor` itself; otherwise `dor` is what + its shell is running, so the tool is idle however the pane reads, and it **re-runs there through the same handshake**, reported `adopted`. Never through the interrupt-and-retype restart: Ctrl+C would kill the `dor` still waiting for the answer. +- **A re-run is a placement of nothing**, so only the two conditions that govern + typing apply — the naked line and integration. It runs in the tool's own + directory, like an `adopted` match from any other pane, and `--surface` / + `--minimize` / a `--cwd` elsewhere do not change that. +- **A caller that is the match but cannot be typed behind fails loudly.** There + is no survivor to reveal — the user is sitting in it — so reporting `existing` + would be a silent no-op. It says so instead. **Respond, then wait for the prompt.** `dor` is the caller's foreground process when the host answers it, so the host answers `takeover` first, waits for the @@ -311,10 +319,12 @@ cannot exit until it is answered. - **A shell that never comes back to its prompt is left alone**: nothing typed, leaf still a terminal. The transformation happens on the way *in* to typing, so a timeout costs nothing. -- **The spawn lock is held past the response** until the command is live. The - key reaches the leaf's params at the meta write, but a pane that has been - typed into and has not yet reported reads as an idle tool, which a queued - invocation of the same key would interrupt and retype. +- **The spawn lock is held past the response** until the shell has processed the + line — the command live, or already finished. The key reaches the leaf's params + at the meta write, but a pane typed into and not yet reporting reads as an idle + tool, which a queued invocation of the same key would interrupt and retype. + Waiting for *live* alone would pin the lock for the full timeout on every tool + that dies on boot: it can start and finish between two samples. - **The transformation is one meta write**, so the component pair and the params commit together, and the leaf id — the SessionId — never changes. That is what keeps the terminal, its buffer, and its PTY untouched. diff --git a/lib/src/components/Wall.test.tsx b/lib/src/components/Wall.test.tsx index a49c21584..329541d03 100644 --- a/lib/src/components/Wall.test.tsx +++ b/lib/src/components/Wall.test.tsx @@ -1467,6 +1467,45 @@ describe('Wall on the Lath engine', () => { expect(typed).toEqual(['pnpm storybook\r', 'pnpm storybook\r']); expect(leafCount()).toBe(1); + // The re-run goes live (releasing the lock) and exits. + act(() => { + terminalRegistry.applyTerminalSemanticEvents('pane-a', [ + { type: 'commandLine', commandLine: 'pnpm storybook' }, + { type: 'commandStart', source: 'osc633_boundaries' }, + ]); + }); + await act(async () => { await new Promise((r) => setTimeout(r, 150)); }); + act(() => { + terminalRegistry.applyTerminalSemanticEvents('pane-a', [{ type: 'promptStart' }]); + }); + + // A line the host cannot type behind says so, rather than reporting a tool + // that is not running as `existing` back into the pane it is sitting in. + act(() => { + terminalRegistry.applyTerminalSemanticEvents('pane-a', [ + { type: 'commandLine', commandLine: 'dor tool storybook && open http://localhost:6006' }, + { type: 'commandStart', source: 'osc633_boundaries' }, + ]); + }); + let compound: { ok: boolean; error?: string } | undefined; + await act(async () => { + window.dispatchEvent(new CustomEvent('dormouse:control-request', { + detail: { + method: SURFACE_CONTROL_METHODS.tool, + surfaceId: 'pane-a', + params: { name: 'storybook', cwd: '/repo', minimized: false, fresh: false }, + respond: (result: typeof compound) => { compound = result; }, + }, + })); + }); + await settle(() => compound !== undefined); + expect(compound?.ok).toBe(false); + expect(compound?.error).toContain("is this tool's own pane"); + expect(typed).toHaveLength(2); + act(() => { + terminalRegistry.applyTerminalSemanticEvents('pane-a', [{ type: 'promptStart' }]); + }); + // Same Surface throughout: the leaf changed kind without changing id, so // the session persists as one. The live state again outlasts a poll tick, // so the re-run releases its lock before the next test takes one. diff --git a/lib/src/components/wall/tool-takeover.test.ts b/lib/src/components/wall/tool-takeover.test.ts index b3baa2f97..02df1aaea 100644 --- a/lib/src/components/wall/tool-takeover.test.ts +++ b/lib/src/components/wall/tool-takeover.test.ts @@ -78,6 +78,15 @@ describe('toolTakesOverCaller', () => { expect(toolRerunsInCaller({ ...passing, kind: 'tool' })).toBe(true); expect(toolRerunsInCaller(passing)).toBe(false); expect(toolRerunsInCaller({ ...passing, kind: 'tool', rawCommandLine: 'claude' })).toBe(false); - expect(toolRerunsInCaller({ ...passing, kind: 'tool', visible: false })).toBe(false); + expect(toolRerunsInCaller({ ...passing, kind: 'tool', oscDriven: false })).toBe(false); + }); + + // The pane already is the tool, so there is nothing to place and the tool + // re-runs in its own directory — as an `adopted` match from any pane does. + it('re-runs regardless of the conditions that only govern placement', () => { + for (const override of [{ cwdMatches: false }, { explicitSurface: true }, { minimized: true }, { visible: false }]) { + expect(toolRerunsInCaller({ ...passing, kind: 'tool', ...override })).toBe(true); + expect(toolTakesOverCaller({ ...passing, ...override })).toBe(false); + } }); }); diff --git a/lib/src/components/wall/tool-takeover.ts b/lib/src/components/wall/tool-takeover.ts index 44c13da38..25c373df4 100644 --- a/lib/src/components/wall/tool-takeover.ts +++ b/lib/src/components/wall/tool-takeover.ts @@ -52,29 +52,34 @@ export interface ToolTakeoverGate { } /** - * What both placements below share: a naked invocation in a visible, integrated - * pane whose directory is the tool's. Every condition is conservative — failing - * one is a split, which is never wrong (rationale). + * Whether the caller's own shell is one the host may type into: an integrated + * pane whose reported line is this invocation and nothing else. Both placements + * need it, and neither can proceed without it. */ -function callerMayRunTool(gate: ToolTakeoverGate): boolean { - return !gate.explicitSurface - && !gate.minimized - && gate.visible - && gate.cwdMatches - && gate.oscDriven - && isNakedToolInvocation(gate.rawCommandLine); +function callerTypedTool(gate: ToolTakeoverGate): boolean { + return gate.oscDriven && isNakedToolInvocation(gate.rawCommandLine); } -/** Whether this `dor tool` transforms its calling pane into the tool. */ +/** + * Whether this `dor tool` transforms its calling pane into the tool. Every + * condition is conservative — failing one is a split, which is never wrong + * (rationale). + */ export function toolTakesOverCaller(gate: ToolTakeoverGate): boolean { - return gate.kind === 'terminal' && callerMayRunTool(gate); + return gate.kind === 'terminal' + && !gate.explicitSurface + && !gate.minimized + && gate.visible + && gate.cwdMatches + && callerTypedTool(gate); } /** * Whether a keyed match on the calling pane re-runs there. The caller is then - * the tool's own Surface — the place take-over makes normal to retype in — and - * its command cannot be live, since its shell is running `dor`. + * the tool's own Surface, so the placement conditions above are moot — there is + * nothing to place, and the tool re-runs in its own directory, exactly as an + * `adopted` match from any other pane does. */ export function toolRerunsInCaller(gate: ToolTakeoverGate): boolean { - return gate.kind === 'tool' && callerMayRunTool(gate); + return gate.kind === 'tool' && callerTypedTool(gate); } diff --git a/lib/src/components/wall/use-dor-control.ts b/lib/src/components/wall/use-dor-control.ts index 2bedaa66b..d6aa35a4b 100644 --- a/lib/src/components/wall/use-dor-control.ts +++ b/lib/src/components/wall/use-dor-control.ts @@ -338,12 +338,20 @@ async function restartSurfaceInPlace(id: string, command: string, cwd: string): * The take-over handshake (docs/specs/dor-tool.md -> Take-over): `dor` is the * pane's foreground process until the host answers it, so the command can only * be typed once its own shell is back at a prompt. A shell that never comes back - * — or a pane killed while we wait — is left exactly as it was. + * — or a pane killed while we wait — is left exactly as it was. Shared by the + * take-over, which transforms the pane on the way in, and a keyed re-run in the + * tool's own pane, which does not. */ -async function takeOverPaneWithTool( +async function runToolInCallerPane( lath: LathWallEngine, id: string, - tool: { params: Record; title: string; command: string; cwd: string }, + tool: { + command: string; + cwd: string; + /** The tool leaf to become — omitted when the pane already is this tool and + * is only re-running it. */ + become?: { title: string; params: Record }; + }, ): Promise { const backAtPrompt = await waitForTerminalState( id, @@ -355,19 +363,26 @@ async function takeOverPaneWithTool( // minimized while `dor` exits, and a Door keeps its meta — `store.has` is // membership of the tree, so it answers both. if (!backAtPrompt || !meta || !lath.store.has(id)) return; - // Whatever this Session announced under its previous command is not this - // tool's: a stale OSC 367 would hand the new tool that port, or re-key it. + // Whatever this Session announced under its previous command is not this run's: + // a stale OSC 367 would hand the tool that port, or re-key it. clearToolAnnounce(id); - // A rename the user made outlives the transformation; an untouched fallback - // title becomes the tool's, as a spawned one would be. - lath.store.setMeta(id, toolLeafMeta(meta.title === UNNAMED_PANEL_TITLE ? tool.title : meta.title, tool.params)); + if (tool.become) { + // A rename the user made outlives the transformation; an untouched fallback + // title becomes the tool's, as a spawned one would be. + const title = meta.title === UNNAMED_PANEL_TITLE ? tool.become.title : meta.title; + lath.store.setMeta(id, toolLeafMeta(title, tool.become.params)); + } + const previousRun = getTerminalPaneState(id).lastCommand?.id ?? null; getPlatform().writePty(id, `${tool.command}\r`); - // The caller holds the spawn lock until this resolves: until the shell reports - // the command, a queued invocation of the same key reads this pane as idle and - // interrupts what was just typed. + // The caller holds the spawn lock until this resolves: a pane typed into but + // not yet reporting reads as an idle tool, which a queued invocation of the + // same key would interrupt and retype. It ends on either outcome — a command + // that dies on boot (a typo, a missing `pnpm`) can start and finish between two + // samples, and waiting out the timeout for it would pin the lock for 15s. await waitForTerminalState( id, - (state) => surfaceRunsCommand(state, tool.command, tool.cwd), + (state) => surfaceRunsCommand(state, tool.command, tool.cwd) + || (state.lastCommand !== null && state.lastCommand.id !== previousRun), RESTART_START_TIMEOUT_MS, ); } @@ -994,32 +1009,42 @@ export function useDorControl({ const match = findSurfaceByParams(matchesToolKey); if (match) { const matchedCommand = toolCommandFromParams(lath.getMeta(match.id)?.params) || command; - // A match that is the calling pane is the tool's own Surface, which - // take-over makes the normal place to retype in. Its command cannot - // be live — `dor` is what its shell is running — so it is idle by - // construction, and it re-runs through the take-over handshake: - // `restartSurfaceInPlace` would fire Ctrl+C into the `dor` still - // waiting for this answer. - if (match.id === callerId && callerGate && toolRerunsInCaller(callerGate)) { + // A match that is the calling pane is the tool's own Surface — the + // place take-over makes normal to retype in. Its command is live + // only when the tool itself spawned this `dor`; otherwise `dor` is + // what its shell is running, so the tool is idle however its pane + // reads, and it re-runs in its own directory like any `adopted` + // match. Through the handshake, never `restartSurfaceInPlace`, + // whose Ctrl+C would kill the `dor` awaiting this answer. + const matchedCwd = getTerminalPaneState(match.id).cwd?.path ?? cwd; + if (match.id === callerId + && !surfaceRunsCommand(getTerminalPaneState(match.id), matchedCommand, matchedCwd)) { + if (!callerGate || !toolRerunsInCaller(callerGate)) { + // Nothing can be typed behind a line that is not this + // invocation alone, and there is no survivor to reveal — the + // user is sitting in it. Say so instead of reporting a tool + // that is not running as `existing`. + detail.respond({ + ok: false, + error: `surface '${surfaceRefForId(match.id)}' is this tool's own pane and its command is not running; re-run it by typing the invocation alone at its prompt`, + }); + return; + } + revealSurface(match.id); detail.respond({ ok: true, result: { status: 'adopted', surfaceId: match.id, surfaceRef: surfaceRefForId(match.id), - command, - cwd, + command: matchedCommand, + cwd: matchedCwd, minimized: false, key, ...(warnings.length > 0 ? { warnings } : {}), }, }); - await takeOverPaneWithTool(lath, match.id, { - params: toolParams, - title: toolName ?? command, - command, - cwd, - }); + await runToolInCallerPane(lath, match.id, { command: matchedCommand, cwd: matchedCwd }); return; } // A dedicated Surface whose command exited is unambiguously free, @@ -1027,11 +1052,11 @@ export function useDorControl({ // aimed at arbitrary shells, would stop matching. const idle = getTerminalPaneState(match.id).currentCommand === null; if (idle) { - // The tool's own cwd, not the caller's: `surfaceRunsCommand` - // compares against the matched Surface's `cwdAtStart`, so waiting - // on the caller's would never resolve when `dor tool` is run from - // a subdirectory — the command restarts and we report failure. - const matchedCwd = getTerminalPaneState(match.id).cwd?.path ?? cwd; + // `matchedCwd` above is the tool's own, not the caller's: + // `surfaceRunsCommand` compares against the matched Surface's + // `cwdAtStart`, so waiting on the caller's would never resolve + // when `dor tool` is run from a subdirectory — the command + // restarts and we report failure. const restarted = await restartSurfaceInPlace(match.id, matchedCommand, matchedCwd); if (!restarted.ok) { detail.respond({ @@ -1085,11 +1110,10 @@ export function useDorControl({ }); // Awaited inside the spawn lock: the key reaches the leaf's params in // there, and a queued invocation of it must find a running tool. - await takeOverPaneWithTool(lath, callerId, { - params: toolParams, - title: toolName ?? command, + await runToolInCallerPane(lath, callerId, { command, cwd, + become: { title: toolName ?? command, params: toolParams }, }); return; } diff --git a/scripts/spec-word-budgets.json b/scripts/spec-word-budgets.json index 2d4561241..6a7962cf9 100644 --- a/scripts/spec-word-budgets.json +++ b/scripts/spec-word-budgets.json @@ -13,7 +13,7 @@ "docs/specs/dor-browser.rationale.md": 650, "docs/specs/dor-cli.md": 6050, "docs/specs/dor-cli.rationale.md": 600, - "docs/specs/dor-tool.md": 3900, + "docs/specs/dor-tool.md": 4025, "docs/specs/dor-tool.rationale.md": 1775, "docs/specs/glossary.md": 3325, "docs/specs/layout.md": 9150, From bdbee6d99401d1b95072bbd5970602077f1c22b0 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 1 Sep 2026 15:57:19 -0700 Subject: [PATCH 5/5] test(tool): pin the lock release on a tool that dies on boot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The disjunct added last commit had no coverage: nothing in `Wall.test.tsx` emits `commandFinish`, so `lastCommand` stayed null and only `surfaceRunsCommand` ever satisfied the wait — the fixed sleeps were what released the lock. The re-run now starts and dies inside one sample, the shape the disjunct exists for, and the request after it fails on `settle`'s deadline if the lock is not released on the finished run (verified by removing the disjunct). Two 150ms sleeps go with it. `adopted` also means two things about timing now, so the CLI contract says which: off the caller's pane the restart is awaited before the status goes out, on its own pane the answer precedes the re-run, for the same reason `takeover` does. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0166PG9g7V3kZ6Uo9EHrpoTD --- dor/src/commands/types.ts | 6 ++++-- lib/src/components/Wall.test.tsx | 20 ++++++-------------- 2 files changed, 10 insertions(+), 16 deletions(-) diff --git a/dor/src/commands/types.ts b/dor/src/commands/types.ts index 0649606d9..2658186d2 100644 --- a/dor/src/commands/types.ts +++ b/dor/src/commands/types.ts @@ -172,8 +172,10 @@ export interface ToolSurfaceResponse { * `existing` is a key match on a live tool: the redundant spawn never * started. `adopted` is a key match whose command had exited — the Surface is * reused and the command re-run in place, keeping its position and scrollback. - * `takeover` is the calling pane itself becoming the tool, answered before the - * command is typed — `dor` has to exit before its own shell is free to run it. + * On the calling pane's own match it is answered before the re-run is typed, + * for the same reason `takeover` is. `takeover` is the calling pane itself + * becoming the tool, answered before the command is typed — `dor` has to exit + * before its own shell is free to run it. */ status: 'created' | 'existing' | 'adopted' | 'pending' | 'takeover'; surfaceId: string; diff --git a/lib/src/components/Wall.test.tsx b/lib/src/components/Wall.test.tsx index 329541d03..dba32a9e6 100644 --- a/lib/src/components/Wall.test.tsx +++ b/lib/src/components/Wall.test.tsx @@ -1467,17 +1467,17 @@ describe('Wall on the Lath engine', () => { expect(typed).toEqual(['pnpm storybook\r', 'pnpm storybook\r']); expect(leafCount()).toBe(1); - // The re-run goes live (releasing the lock) and exits. + // The re-run starts and dies inside one 100ms sample, so no poll ever sees + // it live: the lock has to release on the finished run instead. Without + // that, the request below waits out the 15s timeout and `settle` gives up. act(() => { terminalRegistry.applyTerminalSemanticEvents('pane-a', [ { type: 'commandLine', commandLine: 'pnpm storybook' }, { type: 'commandStart', source: 'osc633_boundaries' }, + { type: 'commandFinish', exitCode: 1 }, + { type: 'promptStart' }, ]); }); - await act(async () => { await new Promise((r) => setTimeout(r, 150)); }); - act(() => { - terminalRegistry.applyTerminalSemanticEvents('pane-a', [{ type: 'promptStart' }]); - }); // A line the host cannot type behind says so, rather than reporting a tool // that is not running as `existing` back into the pane it is sitting in. @@ -1507,15 +1507,7 @@ describe('Wall on the Lath engine', () => { }); // Same Surface throughout: the leaf changed kind without changing id, so - // the session persists as one. The live state again outlasts a poll tick, - // so the re-run releases its lock before the next test takes one. - act(() => { - terminalRegistry.applyTerminalSemanticEvents('pane-a', [ - { type: 'commandLine', commandLine: 'pnpm storybook' }, - { type: 'commandStart', source: 'osc633_boundaries' }, - ]); - }); - await act(async () => { await new Promise((r) => setTimeout(r, 150)); }); + // the session persists as one. await act(async () => { window.dispatchEvent(new Event('pagehide')); }); await flush(); await flush();