Skip to content

Dor Tools: accept inputs and user-global definitions - #668

Merged
nedtwigg merged 22 commits into
tool-takeoverfrom
dor-tool-inputs
Sep 17, 2026
Merged

nedtwigg merged 22 commits into
tool-takeoverfrom
dor-tool-inputs

Conversation

@nedtwigg

@nedtwigg nedtwigg commented Sep 15, 2026

Copy link
Copy Markdown
Member

Named Tools accept arguments and user-global definitions. List-valued run expands $ARGS and canonical local-file $TARGET inputs as argument values, quoted for the destination shell. Shell-string commands stay literal and reject extra inputs. Argument values containing terminal control characters are rejected before injection.

Resolved argv is saved independently of the live command string. Cold restore re-quotes it for the selected shell and updates terminal options plus pane/Door metadata, including when the original Tool took over a different shell. Invalid persisted argv is rejected before any PTY starts; literal shell-string commands keep their saved text.

Project lookup falls back to $XDG_CONFIG_HOME/dormouse/dormouse.yml or ~/.config/dormouse/dormouse.yml; --global skips project lookup. Project and user Tools have separate reuse scopes. User configuration supports dotfiles symlinks. Unknown names report the user file when no project file exists; target-based commands warn when their reuse key omits the target.

Approval validates inputs before presenting the command and distinguishes pending requests by arguments and invocation directory, preserving --fresh through approval. If lookup fails after permission is saved, Retry repeats lookup and Close leaves permission intact; a failed lookup leaves the pane without a PTY.

Validation coverage: host resolution and input validation, destination-shell quoting, changed-shell cold restore, persisted control rejection, approval failures/retries, pending-request identity, and CLI output. No dogfooding or installation.

Local validation: 149 Tool lookup, registry, and Wall integration tests, library TypeScript, and spec checks pass after parent integration. CI and Chromatic run on the pushed head.

Tool stack 3/7, based on #514. Next: #669. Merge bottom-up after its predecessors.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Sep 15, 2026

Copy link
Copy Markdown

Deploying mouseterm with  Cloudflare Pages  Cloudflare Pages

Latest commit: 7991e2d
Status: ✅  Deploy successful!
Preview URL: https://816c2d53.mouseterm.pages.dev
Branch Preview URL: https://dor-tool-inputs.mouseterm.pages.dev

View logs

@dormouse-bot dormouse-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feedback on work in progress, not a merge verdict — mark ready when you want the full pass.

Four findings from tracing the input-resolution path end to end; details and two suggestions are inline.

The one worth fixing first is the approval round-trip. createToolHost's untrusted arm resolves inputs against dedupeTemplate: null, so the pending prompt never enforces a $TARGET that appears only in prespawn_dedupe. With run: [viewer] and prespawn_dedupe: [$PROJECT_ROOT, $TARGET] in an unapproved checkout, dor tool viewer with no arguments shows the approval prompt; the re-lookup after "Always allow for folder" goes through the trusted arm with the full template, throws $TARGET requires exactly one local file argument, and Wall.tsx maps every non-ok status to closeSurface — so the pane the user just approved vanishes with no message, having spent a trust grant. Inputs make that silent close routine rather than exotic: a target moved or deleted between the prompt and the click lands on the same path.

Two cleanup notes, while the shape is still moving:

  • Wall.tsx re-derives dorCommandString's body — buildShellCommandForKind(shellCommandKind(getDefaultShellOpts()?.shell, PLATFORM_STRING), …) — rather than calling it, so the quoting policy for a tool command now lives in two places that have to agree. Exporting it from use-dor-control.ts, or moving it beside getDefaultShellOpts, keeps one.
  • runToolCommand re-walks raw argv against FLAGS_WITH_VALUES / BOOLEAN_FLAGS to recover the positional split validateToolArgs computed during pre-parse. That is a third place the flag lists have to stay in sync, against the "Keep the flag lists above in sync" note the file already carries; a shared helper returning the positionals would collapse it.

Comment thread lib/src/host/tool-input.ts Outdated
Comment thread lib/src/host/tool-user-config.ts
Comment thread lib/src/host/tool-user-config.ts Outdated
Comment thread lib/src/components/Wall.tsx Outdated
@nedtwigg
nedtwigg added this pull request to stack #578 September 15, 2026 23:51

@dormouse-bot dormouse-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feedback on work in progress, not a merge verdict.

The four findings from the previous pass are all addressed, and the $TARGET-in-dedupe rejection moved to parseToolFile as suggested. Two points on the new failure state, inline.

The one worth deciding now: the failure state reuses the approval prompt verbatim, so after a failed launch the pane's only actions are to grant trust that has already been recorded, or to decline — and decline now discards a pane whose grant was written, contradicting the panel's own "Declining records nothing" and resolveToolApproval's decline-branch comment ("A refusal writes nothing ... leaves no record"). docs/specs/dor-tool.md says the failed pane allows "retry or closure", which is a different pair of buttons than the ones it currently shows.

Comment thread lib/src/components/Wall.tsx Outdated
Comment thread lib/src/components/wall/ToolApproval.tsx Outdated
nedtwigg and others added 9 commits September 15, 2026 17:58
# Conflicts:
#	lib/src/components/Wall.test.tsx
#	lib/src/components/wall/use-dor-control.ts
One substitution engine in tool-registry.ts serves both the dedupe key and
the run list, so an unknown token fails the same way in either. The host
assembles its ok result once for project and user Tools, and lookupTool
takes an options object instead of a sixth positional. The renderer quotes
a resolved run through one exported toolRunCommand, dedupes user Tools on
the persisted scope rather than a key prefix, and compares pending inputs
with toolKeysEqual. The dor CLI walks the argv head once for both
validation and the named/anonymous decision, dropping the derived
hasArgumentEscape context field. A missing $TARGET file reports a plain
message instead of an errno, and the approval pane's error uses the error
token.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
# Conflicts:
#	docs/specs/dor-tool.md
#	dor/src/commands/tool.ts
#	lib/src/components/Wall.tsx
#	lib/src/components/wall/use-dor-control.ts
#	lib/src/components/wall/wall-context.tsx
#	lib/src/host/tool-trust.ts
#	scripts/spec-word-budgets.json
# Conflicts:
#	docs/specs/dor-tool.md
#	lib/src/components/Wall.test.tsx
#	lib/src/components/Wall.tsx
#	lib/src/components/wall/use-dor-control.ts
#	scripts/spec-word-budgets.json

@dormouse-bot dormouse-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both fixes confirmed on de6c2d4, and CI is green.

The resurrection guard holds because isCurrent() compares the pending marker by identity against the object captured at entry, and updateParams shallow-merges params, so the stored toolPending survives by reference — once launch writes toolPending: undefined, no later showFailure can match. The failAfterSpawn case pins it. The Retry/Close state now matches what dor-tool.md claims, and all three trust failure modes keep the approval buttons without reaching lookup, so a failed write can't strand the pane claiming a permission it doesn't have.

I also read the two commits that landed after my last pass (destination-shell quoting, persisted-argv requote). What I checked holds: restore re-quotes for the same getDefaultShellOpts() shell every restored pane actually spawns with, toolArgv is written on all three launch paths (approval, key match, take-over), and $TARGET alongside a string run is rejected at parse time now. One consistency point inline, not a blocker.

Comment thread lib/src/components/wall/browser-surface.ts
# Conflicts:
#	docs/specs/dor-tool.md
#	lib/src/components/Wall.test.tsx
#	lib/src/components/Wall.tsx
# Conflicts:
#	docs/specs/dor-tool.md
# Conflicts:
#	docs/specs/dor-tool.md
#	lib/src/components/Wall.tsx
# Conflicts:
#	docs/specs/dor-tool.md
#	lib/src/components/Wall.tsx
#	lib/src/components/wall/ToolApproval.tsx
#	scripts/spec-word-budgets.json
# Conflicts:
#	docs/specs/dor-tool.md
#	lib/src/components/wall/ToolApproval.tsx
@nedtwigg
nedtwigg marked this pull request as ready for review September 16, 2026 23:24

@dormouse-bot dormouse-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Full pass now that this is ready for review. Two findings, both inline.

createToolHost falls back to the user file on a project miss, but if (project) return project; hands back the project's no-file whenever a project lookup ran at all. So with a populated ~/.config/dormouse/dormouse.yml and no repo in scope, dor tool viewr foo.md reports no dormouse.yml found in '<cwd>' or any parent directory — naming neither the user file that was read nor the tools in it. The unknown-tool arm immediately below already carries both, and is unreachable on that path.

Separately, a $TARGET run whose prespawn_dedupe omits $TARGET keys every target to one Surface: dor tool viewer b.md matches the pane still showing a.md, reports existing, and reveals it without re-running. parseToolFile already warns for the analogous $PROJECT_ROOT omission, so the same warning fits beside it — with a companion line under Declaring tools, which dor-tool.md is freshly at budget for.

Comment thread lib/src/host/tool-host.ts Outdated
Comment thread lib/src/host/tool-registry.ts

@dormouse-bot dormouse-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both findings from the last pass are fixed — the user-file fallback now routes a project no-file into the unknown-tool that names the real path, and the $TARGET-key warning sits inside the prespawn_dedupe block so a Tool with no key is untouched.

Build & Test is red at this head, and not for anything in this commit. pnpm test is an && chain with node scripts/spec-lint.mjs first, so the failure stops the run before any of the new tool-host / tool-registry cases execute — they have not run in CI yet.

The failure is a ratchet collision, invisible from the branch alone. CI lints the merge tree, and scripts/spec-md.mjs's countWords puts docs/specs/security-local.md at 2899 words on cd5162d5 (under the 2900 this PR ratcheted to) but 2925 on the merge with main — base commit 9c11aa87 ("test: pin Vite Host protection and document its security boundary") added a FAIL IF line and a Source of truth: continuation to the same spec. Both sides are individually under their own budget; only the merge is over. Suggestion inline bumps it to 2950, which is what --ratchet would write for 2925 and stays valid on the branch too, since the lint only fails on words > budget.

Happy to push that one-line bump if you'd rather not round-trip it.

Comment thread scripts/spec-word-budgets.json Outdated
@nedtwigg
nedtwigg merged commit 4955b4a into main Sep 17, 2026
12 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants