From e0f20088d85b40491891dd7a632312a16884f040 Mon Sep 17 00:00:00 2001 From: Dudu-0223 Date: Thu, 9 Jul 2026 20:44:32 +0800 Subject: [PATCH 1/7] feat: bash-backed glob/grep discovery tools (dsh-tool-fs-search) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements docs/rfc/implemented/feature/2026-07-09-bash-backed-grep-glob- discovery.md: model-facing glob/grep in a new @deepseek-ai/dsh-tool-fs-search package, executing fixed ripgrep templates through ctx.bash.resolve/run — not ctx.fs provider methods — so filesystem backends stay free of a search contract and sandboxed/remote executors substitute cleanly. The tools never call ctx.bash.start(); the tool layer owns quoting (one singleQuote safety boundary), rg --json parsing, ItemRetainer/TextRetainer retention, and the first tool-owned ctx.spillFiles.saveText() handoff (item-level retention the generic post-execute spill policy cannot recover). RFC amendments on the way to implemented/: a shared src/search-core.ts (the SEARCH_* vocabulary + bash-run/raw-spill/spill plumbing was byte-identical across both tools — the missed-extraction smell), and a snapshot-gap note: wiring the acp-agent tree changes the assembled prompt, so goldens need a keyed re-record; the spill notice text is pinned by unit tests instead and only the coding-agent example ships the tools for now. --- docs/config-catalog.md | 22 + docs/module-graph.md | 9 + docs/rfc/INDEX.md | 1 + ...6-07-09-bash-backed-grep-glob-discovery.md | 168 +++++ docs/tool-catalog.md | 59 ++ examples/coding-agent/composition.md | 3 + examples/coding-agent/cordis.yml | 6 + knip.json | 5 + packages/README.md | 2 +- .../core/tools/tests/gen-tool-catalog.spec.ts | 2 +- packages/fs/README.md | 7 +- packages/fs/tool-fs-search/README.md | 46 ++ packages/fs/tool-fs-search/package.json | 49 ++ packages/fs/tool-fs-search/src/glob.ts | 168 +++++ packages/fs/tool-fs-search/src/grep.ts | 314 +++++++++ packages/fs/tool-fs-search/src/index.ts | 110 ++++ packages/fs/tool-fs-search/src/search-core.ts | 257 ++++++++ packages/fs/tool-fs-search/src/shell-quote.ts | 27 + .../tool-fs-search/tests/integration.spec.ts | 161 +++++ .../fs/tool-fs-search/tests/load-path.spec.ts | 50 ++ .../tool-fs-search/tests/shell-quote.spec.ts | 59 ++ .../fs/tool-fs-search/tests/tools.spec.ts | 623 ++++++++++++++++++ packages/fs/tool-fs-search/tsconfig.json | 20 + pnpm-lock.yaml | 37 ++ scripts/gen-tool-catalog.ts | 18 + tsconfig.build.json | 1 + tsconfig.json | 1 + 27 files changed, 2220 insertions(+), 5 deletions(-) create mode 100644 docs/rfc/implemented/feature/2026-07-09-bash-backed-grep-glob-discovery.md create mode 100644 packages/fs/tool-fs-search/README.md create mode 100644 packages/fs/tool-fs-search/package.json create mode 100644 packages/fs/tool-fs-search/src/glob.ts create mode 100644 packages/fs/tool-fs-search/src/grep.ts create mode 100644 packages/fs/tool-fs-search/src/index.ts create mode 100644 packages/fs/tool-fs-search/src/search-core.ts create mode 100644 packages/fs/tool-fs-search/src/shell-quote.ts create mode 100644 packages/fs/tool-fs-search/tests/integration.spec.ts create mode 100644 packages/fs/tool-fs-search/tests/load-path.spec.ts create mode 100644 packages/fs/tool-fs-search/tests/shell-quote.spec.ts create mode 100644 packages/fs/tool-fs-search/tests/tools.spec.ts create mode 100644 packages/fs/tool-fs-search/tsconfig.json diff --git a/docs/config-catalog.md b/docs/config-catalog.md index c4a982e12a..01c8ead2f0 100644 --- a/docs/config-catalog.md +++ b/docs/config-catalog.md @@ -724,6 +724,28 @@ export interface Config { Source: [`packages/fs/tool-fs/src/index.ts:48`](../packages/fs/tool-fs/src/index.ts) +## `@deepseek-ai/dsh-tool-fs-search` + +Requires: `tools` · `systemPrompt` · `bash` + +```ts config-catalog +/** Plugin config (all optional — `Config` supplies the defaults). */ +export interface Config { + /** Max paths one `glob` call retains inline; later paths go to the formatted spill file. */ + globMaxResults?: number + /** Max flat matches one `grep` call retains inline; later matches go to the formatted spill file. */ + grepMaxMatches?: number + /** Max bytes retained for one matched-line preview (the cut preserves UTF-8 boundaries). */ + grepMaxLineBytes?: number + /** Max complete raw `rg` stdout bytes a search will parse; larger raw output fails with `SEARCH_RAW_OUTPUT_OVERFLOW`. */ + rawOutputMaxBytes?: number + /** Cooperative tool-call timeout budget (ms) on both tools, enforced by `@deepseek-ai/dsh-timeout-policy` through `exec.signal`. */ + timeoutMs?: number +} +``` + +Source: [`packages/fs/tool-fs-search/src/index.ts:59`](../packages/fs/tool-fs-search/src/index.ts) + ## `@deepseek-ai/dsh-tool-subagent` Requires: `tools` · `subagents` diff --git a/docs/module-graph.md b/docs/module-graph.md index c283bfe2a6..641566b03c 100644 --- a/docs/module-graph.md +++ b/docs/module-graph.md @@ -35,6 +35,7 @@ flowchart TD pkg_fs_local["fs-local"] pkg_fs_policy["fs-policy"] pkg_tool_fs["tool-fs"] + pkg_tool_fs_search["tool-fs-search"] end subgraph group_compact["packages/compact"] pkg_compact["compact"] @@ -161,6 +162,13 @@ flowchart TD pkg_tool_fs --> pkg_session pkg_tool_fs --> pkg_system_prompt pkg_tool_fs --> pkg_tools + pkg_tool_fs_search --> pkg_bash + pkg_tool_fs_search --> pkg_llm + pkg_tool_fs_search --> pkg_retention + pkg_tool_fs_search --> pkg_session + pkg_tool_fs_search --> pkg_spill + pkg_tool_fs_search --> pkg_system_prompt + pkg_tool_fs_search --> pkg_tools pkg_subagent --> pkg_agent pkg_subagent --> pkg_llm pkg_subagent --> pkg_tools @@ -278,6 +286,7 @@ flowchart TD | [`agent-loop`](../packages/core/agent-loop) | `core` | [`agent`](../packages/core/agent), [`llm`](../packages/llm/llm), [`session`](../packages/core/session), [`session-persistence`](../packages/session-persistence/session-persistence), [`system-prompt`](../packages/core/system-prompt), [`tools`](../packages/core/tools) | | [`tool-bash`](../packages/bash/tool-bash) | `bash` | [`agent`](../packages/core/agent), [`bash`](../packages/bash/bash), [`llm`](../packages/llm/llm), [`system-prompt`](../packages/core/system-prompt), [`tools`](../packages/core/tools) | | [`tool-fs`](../packages/fs/tool-fs) | `fs` | [`fs`](../packages/fs/fs), [`llm`](../packages/llm/llm), [`session`](../packages/core/session), [`system-prompt`](../packages/core/system-prompt), [`tools`](../packages/core/tools) | +| [`tool-fs-search`](../packages/fs/tool-fs-search) | `fs` | [`bash`](../packages/bash/bash), [`llm`](../packages/llm/llm), [`retention`](../packages/util/retention), [`session`](../packages/core/session), [`spill`](../packages/spill/spill), [`system-prompt`](../packages/core/system-prompt), [`tools`](../packages/core/tools) | | [`subagent`](../packages/subagent/subagent) | `subagent` | [`agent`](../packages/core/agent), [`llm`](../packages/llm/llm), [`tools`](../packages/core/tools) | | [`tool-web`](../packages/web/tool-web) | `web` | [`llm`](../packages/llm/llm), [`system-prompt`](../packages/core/system-prompt), [`tools`](../packages/core/tools), [`web`](../packages/web/web) | | [`spill-policy`](../packages/spill/spill-policy) | `spill` | [`llm`](../packages/llm/llm), [`retention`](../packages/util/retention), [`session`](../packages/core/session), [`spill`](../packages/spill/spill), [`tools`](../packages/core/tools) | diff --git a/docs/rfc/INDEX.md b/docs/rfc/INDEX.md index 475f3f01ae..c5febfe749 100644 --- a/docs/rfc/INDEX.md +++ b/docs/rfc/INDEX.md @@ -63,6 +63,7 @@ Generated by `pnpm run gen-rfc-index` from the RFC tree — never edit by hand; | [Subagent lifecycle enrichment — lastAssistantMessage (observe-only)](implemented/feature/2026-06-30-subagent-observe-enrich.md) | 2026-06-30 | | [Explicit model-facing tool order](implemented/feature/2026-07-06-explicit-tool-order.md) | 2026-07-06 | | [Repeat-tool-call guard plugin](implemented/feature/2026-07-08-repeat-tool-guard.md) | 2026-07-08 | +| [Bash-backed grep and glob discovery tools](implemented/feature/2026-07-09-bash-backed-grep-glob-discovery.md) | 2026-07-09 | ### Simplification diff --git a/docs/rfc/implemented/feature/2026-07-09-bash-backed-grep-glob-discovery.md b/docs/rfc/implemented/feature/2026-07-09-bash-backed-grep-glob-discovery.md new file mode 100644 index 0000000000..185444872f --- /dev/null +++ b/docs/rfc/implemented/feature/2026-07-09-bash-backed-grep-glob-discovery.md @@ -0,0 +1,168 @@ +# RFC: Bash-backed grep and glob discovery tools + +Status: implemented + +## Problem + +The harness needs model-facing `glob` and `grep` tools, but making them `ctx.fs` provider methods turns a local product convenience into a universal filesystem backend contract. Local workspace discovery is naturally a process-backed `rg` workflow; remote or virtual filesystem backends may expose their own search API, may not share a local `ripgrep` view, or may not support discovery at all. The v1 should not require every filesystem backend to implement search before the file read/write/edit seam has proven that need. + +Search output also has two distinct budgets. The tool needs enough raw `rg` output to compute a stable logical result, but the model should receive only a bounded preview plus a recovery path when the formatted result is larger than the inline budget. The generic spill policy only sees the final tool result, so it cannot recover matches that a search tool already omitted. Search therefore needs tool-owned retention and best-effort formatted-result spill. + +## Decision + +`glob` and `grep` are model-facing tools in `@deepseek-ai/dsh-tool-fs-search`, backed by the bash seam, not by new `ctx.fs` provider methods. The package registers model-facing filesystem discovery tools, but execution uses `ctx.bash.resolve(request)` followed by `ctx.bash.run(spec)` with fixed `rg` command templates assembled by the tool. The tool layer owns schemas, argument validation, shell quoting, result parsing, result formatting, retention, formatted-result spill handoff, and timeout declaration. The bash executor owns request defaulting/capping, subprocess execution, process-group termination, environment scrubbing, raw output capture, and backend substitution across local, sandboxed, or remote bash implementations. + +The tools do not use `ctx.bash.start()` and do not create model-visible background tasks. They run as ordinary foreground tools from the agent loop's perspective: the tool call returns only after the `rg` command exits, times out, is aborted, or fails. `defineTool({ timeoutMs })` declares the cooperative tool-call budget, `@deepseek-ai/dsh-timeout-policy` enforces it through `exec.signal`, and the tool forwards that signal into the bash request before `resolve()` / `run()`. The bash backend's own timeout remains a second safety cap; whichever aborts first wins. + +The tools align `path` with Claude Code's search tools while binding resolution to the bash workdir, not to `ctx.fs`. The tool derives the bash request workdir from `exec.agent?.session.header.cwd`, mirroring `dsh-tool-bash` and `dsh-tool-fs`; when no session cwd exists, it omits `request.workdir` so the bash implementation applies its configured cwd or process cwd through `resolve()`. For `grep`, `path` is an optional ripgrep target and may be a file or directory; omitted means the resolved bash workdir. For `glob`, `path` is an optional directory search root; omitted means the resolved bash workdir. Relative `path` values resolve against that workdir. Returned paths are displayed relative to the resolved bash workdir when possible and are intended to be follow-up-readable only in co-located deployments where the bash workdir and filesystem `read` root are the same workspace. v1 documents that deployment requirement but does not perform runtime cross-service validation. Remote or virtual filesystem search is deferred until there is a shared workspace/root contract or a provider-specific search backend. + +The package does not inject `fs`. It injects `tools`, `systemPrompt`, and `bash`; it deliberately reads `spillFiles` with `ctx.get('spillFiles')` instead of static inject because formatted-result spill is optional. Existing `@deepseek-ai/dsh-tool-fs` deployments that only want `read` / `write` / `edit` do not need to load bash. + +### Package shape + +The v1 package stays small. Inside `@deepseek-ai/dsh-tool-fs-search`, the source layout is: + +```text +src/index.ts +src/glob.ts +src/grep.ts +src/search-core.ts +src/shell-quote.ts +``` + +`glob.ts` and `grep.ts` own their parameter validation, command construction, result parsing, formatting, and registration. `shell-quote.ts` is one shared helper because shell quoting is the safety boundary both tools must use; `search-core.ts` is the other (an implementation-time amendment to the original four-file plan): the `SEARCH_*` error vocabulary, the bash-run + raw-output acquisition, the formatted-spill handoff, and workdir-relative display are byte-identical between the two tools, and duplicating that delicate plumbing per tool is exactly the missed extraction the symmetry convention flags. Command builders must not hand-roll quoting or concatenate unquoted model-controlled values into the shell command. + +### Schemas and config + +`glob` exposes the small discovery shape: + +```ts +interface GlobArgs { + pattern: string + path?: string +} +``` + +`grep` exposes the OpenCode-style minimal shape: + +```ts +interface GrepArgs { + pattern: string + path?: string + include?: string +} +``` + +Routine budgets stay out of the model-facing schema. `@deepseek-ai/dsh-tool-fs-search` owns these defaulted, validated config fields: + +| Field | Default | Role | +|---|---:|---| +| `globMaxResults` | `100` | Max paths retained inline; matches Claude Code's default `GlobTool` result limit. | +| `grepMaxMatches` | `250` | Max flat matches retained inline; matches Claude Code's default `GrepTool` `head_limit`. | +| `grepMaxLineBytes` | `2000` | Max bytes retained for one matched-line preview, applied with `TextRetainer({ kind: 'head', maxBytes: grepMaxLineBytes })`. | +| `rawOutputMaxBytes` | `20000000` | Max complete raw `rg` stdout the tool will parse; matches Claude Code's ripgrep raw buffer. | +| `timeoutMs` | `30000` | Tool-call timeout attached to both tool definitions and enforced by `@deepseek-ai/dsh-timeout-policy`. | + +`globMaxResults` and `grepMaxMatches` use `ItemRetainer({ kind: 'head' })`. `grepMaxLineBytes` uses `TextRetainer({ kind: 'head', maxBytes: grepMaxLineBytes })` for each matched line so preview cuts preserve UTF-8 boundaries. This follows the [tool result retention library](../../implemented/architecture/2026-07-06-tool-result-retention-library.md) mapping for discovery items: collect the complete result, retain head items inline, and keep path mapping, grouping, and per-line preview outside the retainer. `grep` does not expose `case_insensitive`, `head_limit`, `offset`, `count`, multiline, context lines, output modes, or file type filters in v1. A model that needs surrounding context reads the matched file with `read`; a model that needs later results reads the formatted spill file with `read offset/limit`. + +The Claude Code values are reference points for the two-layer budget, not model-facing schema precedent. Its dedicated search tools buffer raw ripgrep output up to 20 MB for internal processing, use a 20-second ripgrep timeout on non-WSL platforms (60 seconds on WSL), then apply search-specific caps before the model sees a result: `GrepTool` defaults to `head_limit = 250` and persists formatted results above 20,000 characters, while `GlobTool` defaults to 100 paths and persists formatted results above 100,000 characters. This RFC mirrors the raw-buffer and inline-count defaults, chooses a 30-second default search timeout, and uses this harness's `ctx.spillFiles.saveText()` path for formatted-result recovery. + +The `path` field follows the same split as Claude Code: `grep.path` is a file-or-directory ripgrep target, while `glob.path` is a directory search root. v1 does not expose a separate cwd/workdir argument on these tools. + +`include` is one positive glob filter, not a list and not an exclude syntax. Reject comma-separated or negated include patterns up front with a structured argument error. Every model-controlled value used in a shell command, including `pattern`, `path`, and `include`, must pass through the package-private shell quoting helper. + +### Execution + +`glob` builds a fixed `rg --files` command rooted at the resolved directory search root (`path` when supplied, else the bash workdir): `rg --files --glob --sort=modified --no-ignore --hidden`, plus VCS metadata excludes for `.git`, `.svn`, `.hg`, `.bzr`, `.jj`, and `.sl`. This aligns with Claude Code on hidden/ignored-file discovery and modified-time ordering while keeping VCS internals out of broad searches. The tool parses one path per line, maps results back to paths relative to the bash workdir when possible, pushes each path into `ItemRetainer({ kind: 'head', maxItems: globMaxResults })`, and formats the full sorted path list for a spill file when the retained result is capped. + +`grep` builds a fixed line-oriented `rg --json` command against the supplied file/directory target (`path` when supplied, else the bash workdir) so file path, line number, and line text are parsed without colon-splitting ambiguity. It consumes `match` records, treats malformed JSON or malformed match records as `SEARCH_FAILED`, maps result paths relative to the bash workdir when possible, applies per-line preview retention with `grepMaxLineBytes`, pushes each match into `ItemRetainer({ kind: 'head', maxItems: grepMaxMatches })`, then groups only the retained preview matches by file for inline output. The spill file stores the full formatted match list, not only the omitted tail, so `read offset/limit` works against the same logical result the model saw. + +Raw `rg` stdout is an internal transport detail. If `ctx.bash.run()` returns untruncated stdout, the tool parses `stdout.text`. If stdout is truncated and `stdout.spillPath` is present, the tool reads that local raw spill file up to `rawOutputMaxBytes + 1` bytes and parses it only when the complete file fits within `rawOutputMaxBytes`. If the raw spill file is larger than `rawOutputMaxBytes`, or stdout is truncated without a spill path, the tool fails with a clear search error telling the model to narrow `pattern`, `path`, or `include`. The tool never exposes raw `rg` output or bash raw spill paths to the model. + +Only stdout is a parse source. Stderr is diagnostic text for invalid patterns, missing `rg`, and search failures; if bash truncates stderr, the tool uses the retained stderr tail with a truncation note and does not read `stderr.spillPath`. + +If `ctx.bash.run()` reports `aborted` because the tool timeout or caller cancellation fired, the tool returns a structured failure rather than pretending there were no matches. If bash reports its own timeout first, the tool likewise fails with a clear timeout message. Nonzero ripgrep exit semantics are tool-owned: exit 0 is success with matches, exit 1 is success with no matches, invalid pattern / missing `rg` / inaccessible search workdir are failures. + +Search failures use a package-owned `HarnessError` subclass with `SEARCH_*` codes, not `FsErrorCode`, because these tools are not `ctx.fs` provider operations. The v1 vocabulary is `SEARCH_INVALID_PATTERN`, `SEARCH_FAILED`, `SEARCH_RAW_OUTPUT_OVERFLOW`, and `SEARCH_ABORTED`. Model argument validation failures such as missing required fields, blank strings, or unsupported negated/list `include` values remain ordinary tool argument errors. + +### Formatted result spill + +`ctx.spillFiles` is optional and used only for model-facing formatted results. This is the first tool-owned spill call pattern in the codebase, and it is intentional because search retention is item-level policy: `globMaxResults` caps paths and `grepMaxMatches` caps matches while the tool still holds the complete logical result. The generic `dsh-spill-policy` caps final text bytes on `tools/post-execute`; by then a search tool would already have omitted later paths or matches, so the policy cannot recover them. + +When a search produces more logical results than the inline cap and `ctx.spillFiles` is present, the tool saves the complete formatted result with `saveText()`. The spill owner is the calling agent's session header id (`exec.agent?.session.header.id`); without that owner, the search keeps the inline result and reports that the complete result could not be saved. The spill source is the tool execution identity: `{ toolName: exec.name, callId: exec.callId, label: 'result' }`. The suggested filenames are `grep-results.txt` and `glob-results.txt`; the spill backend still sanitizes them as hints, never paths. + +When spill storage is absent, the call has no session owner, or saving fails, the tool still returns the inline page and a footer explaining that the complete result could not be saved. Search success must not turn into an `isError` result solely because formatted-result spill storage is unavailable. + +The bash raw spill file and the formatted search spill file are different artifacts. The raw bash spill file is a local executor implementation detail used only so the search tool can parse complete `rg` stdout. The formatted spill file is the stable model-facing recovery path produced by `ctx.spillFiles.saveText()`. + +### Result shape + +A capped `glob` result with successful formatted spill returns the inline page and a spill notice: + +```text + + +(Showing N of M paths. Full sorted result saved to: /.../session-abc123/9f8e7d-glob-results.txt. Use read with offset/limit to inspect it.) +``` + +A capped `grep` result with successful formatted spill returns grouped preview matches and a spill notice: + +```text +Found N of M matches + + +Line 12: ... + +(Full grep result saved to: /.../session-abc123/9f8e7d-grep-results.txt. Use read with offset/limit to inspect it.) +``` + +If the complete logical result fits under the inline cap, no formatted spill file is created. If the complete logical result is too large but formatted spill is unavailable, the footer says that the result was capped and the complete result could not be saved. The `truncated` / omitted count is a budget fact, not an incomplete-search fact; timeout, invalid regex, missing `rg`, inaccessible workdirs, raw-output overflow, binary skips, and parse failures stay in tool-domain error or incomplete fields. + +## Alternatives considered + +**Put `glob` / `grep` on `ctx.fs`.** Rejected for v1: it forces every filesystem backend to grow a search API and makes local ripgrep behavior part of the provider seam. Search is useful product behavior, but it is not a universal text-storage primitive like `readText` or `writeText`. + +**Directly spawn ripgrep from `dsh-fs-local`.** Rejected for this RFC's v1: direct spawn gives the cleanest argv boundary, stdout/stderr control, and early-stop control, but it duplicates process execution concerns that the bash seam already owns: environment scrubbing, process-group kill, timeout propagation, sandbox/remote executor substitution, and raw output spill. It remains a reasonable optimization if bash-backed search proves too shell-string-sensitive or if raw bash spill recovery is not portable enough. + +**Use `ctx.bash.start()` for streaming early stop.** Rejected: `start()` creates model-visible background task semantics: task ids, owner tokens, `bash_output`, `bash_kill`, completion notifications, and no built-in timeout. `grep` needs a foreground tool result, not a background bash workflow. If streaming search becomes necessary, the right abstraction is a foreground streaming process handle on the bash/process seam, not borrowing the public background-task API. + +**Expose bash raw spill paths to the model.** Rejected: a bash raw spill path contains raw `rg` stdout (`rg --json` records for grep), not the stable formatted search result. The search tool may read raw spill internally, but model recovery uses a formatted result saved through `ctx.spillFiles.saveText()`. + +**Add `spillFiles.saveFile()` for bash output normalization first.** Rejected for this RFC's v1: `saveFile()` would help a future bash normalization pass move existing executor spill files into session-scoped spill storage, but search still has to parse raw `rg` output before producing the model-facing artifact. `saveText()` is sufficient for the formatted search result, and raw bash spill remains an executor-local recovery detail. + +**Rely on the generic `dsh-spill-policy`.** Rejected: generic post-execute spill sees only the final tool result. If `grep` / `glob` return the first page inline, the generic policy cannot recover omitted results. The search tools must save the complete formatted result themselves before returning the bounded model-facing text. + +**Expose Claude Code's full `GrepTool` schema.** Rejected for v1: `output_mode`, context flags, multiline, `head_limit`, `offset`, `case_insensitive`, and type filters make the model-facing surface into a ripgrep wrapper. This harness keeps routine budgets and continuation mechanics in deployment policy and spill files. + +**Keep early-stop search and skip formatted spill files.** Rejected for this proposal: early stop is more efficient but gives the model no path to inspect later results. The chosen v1 optimizes result recoverability and implementation simplicity, with `timeoutMs`, `rawOutputMaxBytes`, bash backend caps, and formatted spill files as safety backstops. + +**Expand the bash seam with a raw-output reader first.** Deferred: a remote bash backend may eventually need a portable `readRawOutput(ref, maxBytes)` style API instead of local `spillPath` reads. v1 uses the existing local-readable `stdout.spillPath` to avoid widening the bash seam for one consumer. + +## Testing + +- Tests prove an aborted `exec.signal` reaches the bash backend (same-reference spec assertion plus the `SEARCH_ABORTED` result), and cover command construction/quoting (malicious patterns, paths with spaces, leading-dash values, quotes, newlines, glob metacharacters — unit assertions plus a real `bash -c` round-trip for every hostile value), `grep.path` as file and directory targets, `glob.path` as a directory search root, invalid pattern handling, no matches, malformed `rg --json` output, matched-line preview truncation, raw-output overflow, timeout/abort, formatted spill success/failure, the package-owned `SEARCH_*` error codes, and the no-background-task invariant. +- The first-party tool-owned spill precedent is covered directly: spill backend present, spill backend absent, `saveText()` failure, and missing spill owner. +- The package has real Loader-path coverage for the namespace plugin export shape (`name`, `inject`, `Config`, and `apply`, with no default export). +- A real-executor integration suite (`dsh-bash-local` + a real `rg`) verifies the world: hostile patterns stay inert, per-session cwd resolution, VCS-metadata exclusion, modification-time ordering, and real ripgrep stderr classification. It self-skips where `rg` is not on PATH (a CI accommodation mirroring the keyless e2e skip); the fake-executor suite alone carries the per-file 100% coverage gate. +- Snapshot gap note for the transcript-visible spill notice: this landed with the gap note, not a snapshot. The snapshot tier replays the acp-agent tree, and adding the search plugin there changes the assembled system prompt — every golden would need re-recording with a real key, which the implementing environment did not hold. The spill notice's exact transcript text is pinned by unit tests (`formatGlobOutput`/`formatGrepOutput` and the through-the-registry spill tests); wiring the plugin into the acp-agent tree plus a `test:snapshot:record` pass is the follow-up for the next key-holding session. + +## Consequences + +- `glob` and `grep` are model-facing tools in `@deepseek-ai/dsh-tool-fs-search`, not `ctx.fs` provider methods and not part of the existing `@deepseek-ai/dsh-tool-fs` root plugin. The package injects `tools`, `systemPrompt`, and `bash`; it does not inject `fs`, and `ctx.spillFiles` stays optional via `ctx.get('spillFiles')`. +- The schemas are exactly `glob(pattern, path?)` and `grep(pattern, path?, include?)`; search caps and timeout are defaulted, validated Config fields (`globMaxResults`, `grepMaxMatches`, `grepMaxLineBytes`, `rawOutputMaxBytes`, `timeoutMs`). +- The tools execute through `ctx.bash.resolve(request)` → `ctx.bash.run(spec)`, forward `exec.signal`, never call `ctx.bash.start()`, and never expose a bash task id. The bash request workdir comes from `exec.agent?.session.header.cwd` when available; the resolved `spec.workdir` drives execution and relative-path display. +- When bash stdout is truncated, the tools parse the full raw stdout only through a local `stdout.spillPath` that fits within `rawOutputMaxBytes`; missing spill paths or over-cap raw output are clear search failures, and raw `rg` output is never exposed to the model. +- Oversized complete formatted results are saved through `ctx.spillFiles.saveText()` when available while inline results stay bounded; spill failure, a missing backend, or a missing owner preserves the inline result and reports the unsaved remainder — never an `isError`. +- The package README, the generated config catalog, and exported JSDoc document the Config fields and `SEARCH_*` codes; the coding-agent example ships the tools (the acp-agent tree waits on the snapshot re-record above); the fs group README records the co-located bash/filesystem deployment requirement. + +## Risks + +Full-run `grep` can be slower than an early-stop search on broad patterns. The v1 accepts that cost for simpler implementation and complete-result recovery, bounded by tool timeout, bash timeout, `rawOutputMaxBytes`, and output caps. If this proves too slow, the direct-ripgrep or foreground-streaming alternatives remain available. + +Shell command construction is the sharpest safety edge. Because `ctx.bash` accepts a command string rather than an argv vector, the implementation must centralize shell quoting and test malicious patterns, paths with spaces, leading-dash patterns, quotes, newlines, and glob metacharacters. + +The v1 assumes a co-located bash/filesystem deployment. If bash searches one workspace and the `read` tool resolves paths against another, returned paths may not be follow-up-readable. The package documents this requirement but does not verify it at runtime. + +Raw bash spill recovery is local-path-shaped in v1. A remote or sandboxed bash backend may return no readable `spillPath` or may require a future raw-output read API. In that case broad searches fail clearly instead of pretending a truncated raw result is complete. + +Spill paths are local filesystem paths in v1. The formatted-result design works for local deployments where `read` can open spill files; remote or workspace-confined deployments need either an allowlist for spill paths or a future virtual spill URI bridge. diff --git a/docs/tool-catalog.md b/docs/tool-catalog.md index 3645ff40e9..824d86bfdd 100644 --- a/docs/tool-catalog.md +++ b/docs/tool-catalog.md @@ -17,6 +17,7 @@ This table connects model-visible tool names to the plugin package and service s | --- | --- | --- | --- | --- | --- | | `@deepseek-ai/dsh-tool-bash` | `bash`, `bash_kill`, `bash_output` | `ctx.tools`, `ctx.bash` | `tool/call`, `tool/result`, `context/message via agent.inject() for background completion notices` | - | The bash/bash_output/bash_kill tools are model-facing consumers of the bash executor seam. | | `@deepseek-ai/dsh-tool-fs` | `edit`, `read`, `write` | `ctx.tools`, `ctx.fs`, `ctx.systemPrompt` | `tool/call`, `fs/write-intent or fs/edit-intent for mutations`, `fs/observed after successful file operations`, `tool/result` | - | The read-before-write/edit policy is added by `@deepseek-ai/dsh-fs-policy` (an `fs/*` event-gate plugin, no schema change); a deployment that loads these tools is expected to also load it. The tool schemas above are identical with or without the policy plugin. | +| `@deepseek-ai/dsh-tool-fs-search` | `glob`, `grep` | `ctx.tools`, `ctx.bash`, `ctx.systemPrompt` | `tool/call`, `tool/result` | - | glob and grep are bash-backed discovery tools: they run fixed ripgrep commands through ctx.bash as ordinary foreground calls (never background tasks). Capped results save the complete formatted list through the optional ctx.spillFiles backend; returned paths are follow-up-readable in co-located bash/filesystem deployments. | | `@deepseek-ai/dsh-tool-subagent` | `subagent` | `ctx.tools`, `ctx.subagents` | `tool/call`, `tool/result`, `child session events through the chosen provider` | `subagent`, `subagent_fork` | The registered tool name is the load-time `toolName` config (default `subagent`); the schema above is that default. The shipped example agents load this package once per subagent backend, so the model additionally sees `subagent_fork` (bound to the fork backend) with an identical schema — see `examples/coding-agent/cordis.yml` and `examples/acp-agent/cordis.yml`. | | `@deepseek-ai/dsh-tool-todo` | `todo_write` | `ctx.tools`, `owning Agent session` | `tool/call`, `todo/write`, `tool/result` | - | todo_write is session-owned state; UIs render the latest todo/write event as a checklist or ACP plan. | | `@deepseek-ai/dsh-tool-web` | `web_fetch`, `web_search` | `ctx.tools`, `ctx.web`, `ctx.systemPrompt` | `tool/call`, `tool/result` | - | web_search and web_fetch keep provider selection behind ctx.web so model-visible schemas stay stable across backend swaps. | @@ -199,6 +200,64 @@ Source: [`packages/fs/tool-fs/src/index.ts`](../packages/fs/tool-fs/src/index.ts The read-before-write/edit policy is added by `@deepseek-ai/dsh-fs-policy` (an `fs/*` event-gate plugin, no schema change); a deployment that loads these tools is expected to also load it. The tool schemas above are identical with or without the policy plugin. +## `@deepseek-ai/dsh-tool-fs-search` + +### `glob` + +Find files whose paths match a glob pattern. Returns matching paths sorted by modification time, including hidden and ignored files (VCS metadata directories are excluded). Returns the first 100 paths inline; a capped result reports where the complete list was saved. + +```json +{ + "type": "object", + "properties": { + "pattern": { + "type": "string", + "description": "Glob pattern to match file paths against (e.g. \"**/*.ts\", \"src/**/*.test.js\")." + }, + "path": { + "type": "string", + "description": "Directory to search in. Defaults to the session workspace; a relative path resolves against it." + } + }, + "required": [ + "pattern" + ] +} +``` + +Source: [`packages/fs/tool-fs-search/src/index.ts`](../packages/fs/tool-fs-search/src/index.ts) + +### `grep` + +Search file contents with a ripgrep regular expression. Returns matching lines with line numbers, grouped by file. Returns the first 250 matches inline; a capped result reports where the complete match list was saved. Use read on a matched file for surrounding context. + +```json +{ + "type": "object", + "properties": { + "pattern": { + "type": "string", + "description": "Regular expression to search for (ripgrep syntax)." + }, + "path": { + "type": "string", + "description": "File or directory to search. Defaults to the session workspace; a relative path resolves against it." + }, + "include": { + "type": "string", + "description": "One glob filter for which files to search (e.g. \"*.ts\", \"*.{js,jsx}\"). Not a list; negation is not supported." + } + }, + "required": [ + "pattern" + ] +} +``` + +Source: [`packages/fs/tool-fs-search/src/index.ts`](../packages/fs/tool-fs-search/src/index.ts) + +glob and grep are bash-backed discovery tools: they run fixed ripgrep commands through ctx.bash as ordinary foreground calls (never background tasks). Capped results save the complete formatted list through the optional ctx.spillFiles backend; returned paths are follow-up-readable in co-located bash/filesystem deployments. + ## `@deepseek-ai/dsh-tool-subagent` ### `subagent` diff --git a/examples/coding-agent/composition.md b/examples/coding-agent/composition.md index 979737d05f..c6c624f0fb 100644 --- a/examples/coding-agent/composition.md +++ b/examples/coding-agent/composition.md @@ -43,6 +43,8 @@ flowchart LR cfg --> plugin_coding_fs_policy plugin_coding_tool_fs["tool-fs
@deepseek-ai/dsh-tool-fs"] cfg --> plugin_coding_tool_fs + plugin_coding_tool_fs_search["tool-fs-search
@deepseek-ai/dsh-tool-fs-search"] + cfg --> plugin_coding_tool_fs_search plugin_coding_spill_local["spill-local
@deepseek-ai/dsh-spill-local"] cfg --> plugin_coding_spill_local plugin_coding_spill_policy["spill-policy
@deepseek-ai/dsh-spill-policy"] @@ -65,6 +67,7 @@ flowchart LR | `fs-local` | `@deepseek-ai/dsh-fs-local` | | `fs-policy` | `@deepseek-ai/dsh-fs-policy` | | `tool-fs` | `@deepseek-ai/dsh-tool-fs` | +| `tool-fs-search` | `@deepseek-ai/dsh-tool-fs-search` | | `spill-local` | `@deepseek-ai/dsh-spill-local` | | `spill-policy` | `@deepseek-ai/dsh-spill-policy` | diff --git a/examples/coding-agent/cordis.yml b/examples/coding-agent/cordis.yml index c740aa2907..5861e6a602 100644 --- a/examples/coding-agent/cordis.yml +++ b/examples/coding-agent/cordis.yml @@ -121,6 +121,12 @@ - id: tool-fs name: '@deepseek-ai/dsh-tool-fs' +# Bash-backed discovery tools (glob/grep): fixed ripgrep commands through the +# local bash executor above — not ctx.fs. Capped results save the complete +# formatted list through the spill backend below (ctx.spillFiles, optional). +- id: tool-fs-search + name: '@deepseek-ai/dsh-tool-fs-search' + # Tool-output spill stack: a local backend that saves oversized tool text under # a private session-scoped dir, and the tools/post-execute policy that replaces # an over-budget plain-text result with a preview + the spill path (the model diff --git a/knip.json b/knip.json index 4b63fa01c4..18870b3f0b 100644 --- a/knip.json +++ b/knip.json @@ -83,6 +83,11 @@ "packages/fs/tool-fs": { "entry": ["tests/**/*.spec.ts", "tests/**/*.e2e.ts"], "project": ["src/**/*.ts", "tests/**/*.ts"] + }, + "packages/fs/tool-fs-search": { + "entry": ["tests/**/*.spec.ts"], + "project": ["src/**/*.ts", "tests/**/*.ts"], + "ignoreBinaries": ["rg"] } } } diff --git a/packages/README.md b/packages/README.md index 7060760311..fcb7b08cb4 100644 --- a/packages/README.md +++ b/packages/README.md @@ -12,7 +12,7 @@ Packages are grouped by role at `packages///`. The group directory i | [`llm/`](llm/README.md) | LLM capability family: the abstract service + provider adapters | Product — stable surface | | [`bash/`](bash/README.md) | Bash capability family: the executor seam, a local impl, and the model-facing tool | Product — stable surface | | [`code-runtime/`](code-runtime/README.md) | Code-execution capability family: the abstract runtime seam for model-written programs + a worker-thread backend | Product — stable surface | -| [`fs/`](fs/README.md) | Filesystem capability family: the abstract seam, a local impl, and the model-facing file tools | Product — stable surface | +| [`fs/`](fs/README.md) | Filesystem capability family: the abstract seam, a local impl, the model-facing file tools, and the bash-backed discovery tools | Product — stable surface | | [`compact/`](compact/README.md) | Compaction capability family: the abstract seam + a basic backend (tool deferred) | Product — stable surface | | [`subagent/`](subagent/README.md) | Subagent capability family: the provider-registry seam and the model-facing delegation tool | Product — stable surface | | [`web/`](web/README.md) | Web capability family: the abstract seam, search/fetch provider impls, and the model-facing web tools | Product — stable surface | diff --git a/packages/core/tools/tests/gen-tool-catalog.spec.ts b/packages/core/tools/tests/gen-tool-catalog.spec.ts index cacc2eef66..57d3e5dbf1 100644 --- a/packages/core/tools/tests/gen-tool-catalog.spec.ts +++ b/packages/core/tools/tests/gen-tool-catalog.spec.ts @@ -35,7 +35,7 @@ describe('gen-tool-catalog collectToolCatalog', () => { it('boots every shipped tool package and harvests its model-facing schemas', async () => { const catalog = await collectToolCatalog() const names = catalog.flatMap(entry => entry.schemas.map(s => s.name)).sort() - expect(names).toEqual(['bash', 'bash_kill', 'bash_output', 'edit', 'read', 'subagent', 'todo_write', 'web_fetch', 'web_search', 'write']) + expect(names).toEqual(['bash', 'bash_kill', 'bash_output', 'edit', 'glob', 'grep', 'read', 'subagent', 'todo_write', 'web_fetch', 'web_search', 'write']) // Every tool carries a JSON-Schema `parameters` object (what the model sees). for (const entry of catalog) { for (const schema of entry.schemas) { diff --git a/packages/fs/README.md b/packages/fs/README.md index ec3bb62afb..039cb39ae9 100644 --- a/packages/fs/README.md +++ b/packages/fs/README.md @@ -1,6 +1,6 @@ # fs/ - filesystem capability family -The filesystem stack: a provider seam (text IO + atomic mutation with an optional version guard), a local implementation, a policy gate plugin (observed-state + read-before-edit + version-guarded write/edit), and the model-facing file tools + executor. All **product** packages. +The filesystem stack: a provider seam (text IO + atomic mutation with an optional version guard), a local implementation, a policy gate plugin (observed-state + read-before-edit + version-guarded write/edit), the model-facing file tools + executor, and the bash-backed discovery tools. All **product** packages. | Package | Role | ctx key | |---|---|---| @@ -8,9 +8,10 @@ The filesystem stack: a provider seam (text IO + atomic mutation with an optiona | `fs-local/` | Local-filesystem `FileSystem` implementation | (registers `ctx.fs`) | | `fs-policy/` | Policy gate plugin: observed-state + read-before-edit + version-guarded write/edit, via the `fs/*` event gate | (no service — `fs/*` listeners) | | `tool-fs/` | Model-facing `read`/`write`/`edit` tools AND the executor (reads via `ctx.fs`, owns read windowing, dispatches `fs/*`) | (registers on `ctx.tools`) | +| `tool-fs-search/` | Model-facing `glob`/`grep` discovery tools, backed by fixed ripgrep commands through the bash seam (`ctx.bash`), NOT by `ctx.fs` provider methods | (registers on `ctx.tools`) | -The interface lives at `fs/fs/`. A sandboxed, remote, or project-scoped filesystem backend can replace `fs-local` without touching the seam, the policy gate, or the model-facing tool schemas. The policy (`fs-policy/`) is a plugin that participates only through the `fs/*` event gate, not a service the tool injects — so dropping it gracefully loses the policy and leaves the unconstrained bare provider rather than breaking the tool. A deployment that loads `tool-fs/` is expected to also load it. +The interface lives at `fs/fs/`. A sandboxed, remote, or project-scoped filesystem backend can replace `fs-local` without touching the seam, the policy gate, or the model-facing tool schemas. The policy (`fs-policy/`) is a plugin that participates only through the `fs/*` event gate, not a service the tool injects — so dropping it gracefully loses the policy and leaves the unconstrained bare provider rather than breaking the tool. A deployment that loads `tool-fs/` is expected to also load it. Discovery (`tool-fs-search/`) deliberately does NOT extend the provider seam: search is a process-backed `rg` workflow on the bash executor, so filesystem backends stay free of a universal search contract; its results are follow-up-readable when the bash workdir and the `read` root are the same workspace (the co-located deployment its README documents). ## No timeouts on file IO -`read`/`write`/`edit` take **no** `timeoutMs`, and the provider seam arms no deadline — unlike bash and web, which consume [`@deepseek-ai/dsh-timeout`](../util/timeout/README.md). A local syscall is best-effort-abortable at most: a timeout could not force an in-progress `fsync`/`rename` to stop, so a deadline here would be a knob that cannot deliver on its promise. Adding one would also be an implicit default in the exact place explicit-over-implicit forbids. Both reference agents (Claude Code, Codex) leave file IO untimed for the same reason; cancellation still propagates through the tool-execution signal for best-effort abort at syscall boundaries. +`read`/`write`/`edit` take **no** `timeoutMs`, and the provider seam arms no deadline — unlike bash and web (which consume [`@deepseek-ai/dsh-timeout`](../util/timeout/README.md)) and the bash-backed `glob`/`grep` (whose declared `timeoutMs` is enforced by `@deepseek-ai/dsh-timeout-policy`): those are process-backed, where a deadline can really kill the work. A local syscall is best-effort-abortable at most: a timeout could not force an in-progress `fsync`/`rename` to stop, so a deadline here would be a knob that cannot deliver on its promise. Adding one would also be an implicit default in the exact place explicit-over-implicit forbids. Both reference agents (Claude Code, Codex) leave file IO untimed for the same reason; cancellation still propagates through the tool-execution signal for best-effort abort at syscall boundaries. diff --git a/packages/fs/tool-fs-search/README.md b/packages/fs/tool-fs-search/README.md new file mode 100644 index 0000000000..e1ed680060 --- /dev/null +++ b/packages/fs/tool-fs-search/README.md @@ -0,0 +1,46 @@ +# @deepseek-ai/dsh-tool-fs-search + +The **model-facing filesystem discovery tools** — `glob`, `grep` — backed by the **bash executor seam**, not by `ctx.fs` provider methods. Each call assembles a fixed ripgrep command (every model-controlled value through one package-private shell-quoting helper), runs it via `ctx.bash.resolve(request)` → `ctx.bash.run(spec)` as an ordinary foreground tool call, parses the raw `rg` output, and returns a bounded, workdir-relative result. The package injects `tools`, `systemPrompt`, and `bash` — deliberately **not** `fs`; `ctx.spillFiles` is read opportunistically with `ctx.get()` because formatted-result spill is optional. + +```ts ignore-check +// Default deployment: a bash executor, then the discovery tools. +await ctx.plugin(LocalBashExecutor, { cwd: process.cwd() }) // @deepseek-ai/dsh-bash-local +await ctx.plugin(ToolFsSearch) // this package — registers glob/grep +// Optional: a spill backend makes capped results fully recoverable. +await ctx.plugin(LocalSpillFiles) // @deepseek-ai/dsh-spill-local +``` + +Why bash-backed: local workspace discovery is naturally a process-backed `rg` workflow, and putting search on `ctx.fs` would force every filesystem backend to grow a search API. The bash executor owns request defaulting/capping, subprocess execution, process-group termination, environment scrubbing, raw output capture, and backend substitution (local, sandboxed, remote); this package owns schemas, argument validation, shell quoting, parsing, retention, formatted-result spill, and timeout declaration. The tools never call `ctx.bash.start()` and never expose a bash task id — the call returns only after `rg` exits, times out, is aborted, or fails. + +## Deployment requirement: co-located bash + filesystem + +Returned paths are displayed relative to the resolved bash workdir (the calling agent's session cwd when present, else the executor's configured default) and are follow-up-readable with `read` only when the bash workdir and the filesystem root are the same workspace. v1 documents that requirement and performs no runtime cross-service validation; remote or virtual filesystem search waits for a shared workspace contract or a provider-specific search backend. + +## Config + +All keys are optional; the defaults are the shipped search caps. + +| Key | Default | Meaning | +|---|---|---| +| `globMaxResults` | `100` | Max paths one `glob` call retains inline (matches Claude Code's `GlobTool` limit); later paths go to the formatted spill file. | +| `grepMaxMatches` | `250` | Max flat matches one `grep` call retains inline (matches Claude Code's `GrepTool` `head_limit`); later matches go to the formatted spill file. | +| `grepMaxLineBytes` | `2000` | Byte cap per matched-line preview; the cut preserves UTF-8 boundaries and is marked `(line truncated)`. | +| `rawOutputMaxBytes` | `20000000` | Max complete raw `rg` stdout a search will parse (matches Claude Code's ripgrep raw buffer); larger raw output fails with `SEARCH_RAW_OUTPUT_OVERFLOW`. | +| `timeoutMs` | `30000` | Cooperative tool-call budget attached to both tool definitions, enforced by `@deepseek-ai/dsh-timeout-policy` through `exec.signal`; the bash backend's own timeout stays a second safety cap. | + +## Tools + +| Tool | Arguments | Behavior | +|---|---|---| +| `glob` | `pattern`, `path?` | `rg --files --glob --sort=modified --no-ignore --hidden` plus VCS metadata excludes (`.git`, `.svn`, `.hg`, `.bzr`, `.jj`, `.sl`). `path` is an optional **directory** search root; omitted means the resolved bash workdir. Returns one path per line, modification-time ordered. | +| `grep` | `pattern`, `path?`, `include?` | Line-oriented `rg --json` parse (no colon-splitting ambiguity). `pattern` is a ripgrep regex; `path` is an optional **file or directory** target; `include` is ONE positive glob filter — a comma-separated list or a negated (`!…`) value is rejected up front (brace alternation like `*.{ts,tsx}` is fine). Returns matches grouped by file as `Line N: `. | + +Routine budgets stay out of the model-facing schema (no `head_limit`/`offset`/`case_insensitive`/output modes): a model that needs surrounding context reads the matched file with `read`; one that needs later results reads the formatted spill file with `read offset/limit`. + +## Two budgets, two artifacts + +Raw `rg` stdout is an internal transport detail. When the executor truncates it, the tool recovers the complete stream from the executor's **raw bash spill file** — read locally, capped at `rawOutputMaxBytes`, never shown to the model. The model-facing recovery artifact is different: when a search yields more logical results than the inline cap, the tool saves the COMPLETE formatted result through `ctx.spillFiles.saveText()` (suggested names `glob-results.txt` / `grep-results.txt`, owner = the calling session, source = the tool execution identity) and appends a footer naming the saved path. This is the first tool-owned spill call in the codebase — deliberate, because retention here is item-level: the generic `@deepseek-ai/dsh-spill-policy` only sees the final text on `tools/post-execute`, by which point a capped search has already omitted later paths/matches. A missing spill backend, a call with no session owner, or a `saveText()` failure keeps the inline page and reports that the complete result could not be saved — never an `isError`. + +## Errors + +Search failures carry the package-owned `SearchError` (a `HarnessError` subclass), surfaced as `{ name, code }` on `isError` results: `SEARCH_INVALID_PATTERN` (ripgrep rejected the regex/glob), `SEARCH_FAILED` (missing `rg`, inaccessible target, signal kill, malformed `--json` output), `SEARCH_RAW_OUTPUT_OVERFLOW` (raw output over `rawOutputMaxBytes`, or truncated with no recovery file), and `SEARCH_ABORTED` (tool timeout, caller cancellation, or the bash executor's own timeout). ripgrep exit semantics are tool-owned: exit 0 is success with results, exit 1 is a successful empty search (`No files found` / `No matches found`), and only other exits are failures. Model argument mistakes (blank pattern, a list-valued `include`) stay ordinary tool argument errors. diff --git a/packages/fs/tool-fs-search/package.json b/packages/fs/tool-fs-search/package.json new file mode 100644 index 0000000000..002d54569a --- /dev/null +++ b/packages/fs/tool-fs-search/package.json @@ -0,0 +1,49 @@ +{ + "name": "@deepseek-ai/dsh-tool-fs-search", + "description": "Model-facing filesystem discovery tools (glob, grep) backed by the DeepSeek Harness bash seam (ctx.bash)", + "version": "0.0.1", + "private": true, + "type": "module", + "main": "lib/index.js", + "types": "lib/types/index.d.ts", + "exports": { + ".": { + "types": "./lib/types/index.d.ts", + "default": "./lib/index.js" + }, + "./src/*": "./src/*", + "./package.json": "./package.json" + }, + "files": [ + "lib/index.js", + "lib/types/**/*.d.ts", + "lib/types/**/*.d.ts.map", + "src" + ], + "license": "BSD-3-Clause", + "dependencies": { + "schemastery": "^3.18.0" + }, + "peerDependencies": { + "@deepseek-ai/dsh-bash": "^0.0.1", + "@deepseek-ai/dsh-llm": "^0.0.1", + "@deepseek-ai/dsh-retention": "^0.0.1", + "@deepseek-ai/dsh-session": "^0.0.1", + "@deepseek-ai/dsh-spill": "^0.0.1", + "@deepseek-ai/dsh-system-prompt": "^0.0.1", + "@deepseek-ai/dsh-tools": "^0.0.1", + "cordis": "^4.0.0-rc.6" + }, + "devDependencies": { + "@deepseek-ai/dsh-agent": "workspace:^", + "@deepseek-ai/dsh-bash": "workspace:^", + "@deepseek-ai/dsh-bash-local": "workspace:^", + "@deepseek-ai/dsh-llm": "workspace:^", + "@deepseek-ai/dsh-retention": "workspace:^", + "@deepseek-ai/dsh-session": "workspace:^", + "@deepseek-ai/dsh-spill": "workspace:^", + "@deepseek-ai/dsh-system-prompt": "workspace:^", + "@deepseek-ai/dsh-tools": "workspace:^", + "cordis": "^4.0.0-rc.6" + } +} diff --git a/packages/fs/tool-fs-search/src/glob.ts b/packages/fs/tool-fs-search/src/glob.ts new file mode 100644 index 0000000000..e565699e2c --- /dev/null +++ b/packages/fs/tool-fs-search/src/glob.ts @@ -0,0 +1,168 @@ +/** + * The model-facing `glob` tool: discover files whose paths match a glob + * pattern, sorted by modification time. Execution goes through the bash seam + * (`ctx.bash`) with a fixed `rg --files` command — this module owns the + * model-facing schema, argument validation, shell-safe command construction, + * result parsing, retention, and formatting; process concerns (defaulting, + * scrubbing, kill, backend substitution) stay behind `ctx.bash`. + * + * @module @deepseek-ai/dsh-tool-fs-search/glob + */ + +import type { Context } from 'cordis' +import { defineTool } from '@deepseek-ai/dsh-tools' +import type { GenericCallView } from '@deepseek-ai/dsh-tools' +import type { ContentBlock } from '@deepseek-ai/dsh-llm' +import { ItemRetainer } from '@deepseek-ai/dsh-retention' +import type { RetainedItems } from '@deepseek-ai/dsh-retention' +import type {} from '@deepseek-ai/dsh-bash' +import type {} from '@deepseek-ai/dsh-system-prompt' +import { runRipgrep, toWorkdirRelative, trySaveFormattedResult } from './search-core.ts' +import { singleQuote } from './shell-quote.ts' + +/** + * Default cap on paths retained inline by one `glob` call (the `globMaxResults` + * config), matching Claude Code's default `GlobTool` result limit. + */ +export const GLOB_MAX_RESULTS = 100 + +/** + * Directory names ripgrep must never descend into for a discovery listing: VCS + * metadata stores. `--no-ignore --hidden` would otherwise surface them in every + * broad search. Each is excluded with a negated any-depth `--glob` (see + * {@link buildGlobCommand}), which matches — and prunes — the directory + * wherever it appears. + */ +export const GLOB_VCS_EXCLUDES: readonly string[] = ['.git', '.svn', '.hg', '.bzr', '.jj', '.sl'] + +/** Resolved glob-tool caps — plugin config after defaulting (see `Config` in index.ts). */ +export interface GlobToolCaps { + /** Max paths retained inline; later paths go to the formatted spill file. */ + maxResults: number + /** Cap on the complete raw `rg` stdout the tool will parse. */ + rawOutputMaxBytes: number + /** Cooperative tool-call budget (ms) attached as `ToolDefinition.timeoutMs`. */ + timeoutMs: number +} + +/** Validated `glob` arguments. */ +export interface GlobInput { + pattern: string + path?: string +} + +/** + * Validate value constraints the schema DSL can't express: a non-blank + * `pattern`, and a non-blank `path` when given. Throws a plain `Error` (an + * ordinary tool argument error) otherwise. + * + * @param args - the schema-validated `glob` arguments. + * @returns the accepted input, unchanged. + */ +export function parseGlobArgs(args: { pattern: string; path?: string }): GlobInput { + if (args.pattern.trim().length === 0) throw new Error('pattern must be a non-empty string') + if (args.path !== undefined && args.path.trim().length === 0) throw new Error('path must be a non-empty string when given') + return { pattern: args.pattern, ...args.path !== undefined ? { path: args.path } : {} } +} + +/** + * Build the fixed `rg --files` command for one `glob` call. Every + * model-controlled value ({@link GlobInput.pattern}, {@link GlobInput.path}) + * passes through {@link singleQuote}; the search root rides behind `--` so a + * leading-dash path can never be parsed as a flag. `--sort=modified` orders by + * modification time, `--no-ignore --hidden` searches ignored and hidden files, + * and {@link GLOB_VCS_EXCLUDES} keeps VCS metadata out. + * + * @param input - the validated arguments. + * @returns the complete, shell-safe command string. + */ +export function buildGlobCommand(input: GlobInput): string { + const parts = [ + 'rg --files', + `--glob=${singleQuote(input.pattern)}`, + '--sort=modified --no-ignore --hidden', + ...GLOB_VCS_EXCLUDES.map(name => `--glob=${singleQuote(`!**/${name}`)}`), + ] + if (input.path !== undefined) parts.push('--', singleQuote(input.path)) + return parts.join(' ') +} + +/** + * Format the model-facing `glob` result: the retained paths, then — when the + * result was capped — a footer carrying either the formatted-spill recovery + * path or the could-not-save explanation. The omitted count is a budget fact: + * the search itself completed. + * + * @param retained - the retention outcome over every discovered path. + * @param spillPath - the saved complete-result path, or `undefined` when unsaved. + * @returns the model-facing text. + */ +export function formatGlobOutput(retained: RetainedItems, spillPath: string | undefined): string { + const body = retained.items.join('\n') + if (!retained.truncated) return body + const recovery = spillPath !== undefined + ? `Full sorted result saved to: ${spillPath}. Use read with offset/limit to inspect it.` + : 'The complete result could not be saved; narrow pattern or path to see more.' + return `${body}\n\n(Showing ${retained.kept} of ${retained.seen} paths. ${recovery})` +} + +/** + * Pending-call presentation: a search card titled by the pattern (and root). + * + * @param args - the raw tool arguments; `pattern` and `path` feed the title. + * @returns the generic card view (`kind: 'search'`) shown while the call runs. + */ +export function presentGlobCall(args: { pattern: string; path?: string }): GenericCallView { + const where = args.path !== undefined ? ` in ${args.path}` : '' + return { card: 'generic', title: `Glob ${args.pattern}${where}`, kind: 'search', rawInput: args.pattern } +} + +/** + * Register the `glob` tool and its system-prompt guidance. + * + * @param ctx - the plugin context; registrations are effects scoped to it, and + * execution uses its `bash` service. + * @param caps - the deployment's resolved glob caps (plugin config after defaulting). + */ +export function applyGlobTool(ctx: Context, caps: GlobToolCaps): void { + ctx.systemPrompt.section({ + name: 'tool:glob', + order: 103, + text: 'Use the glob tool — not shell find or ls — to discover files by path pattern. Results are sorted by modification time and include hidden and ignored files.', + }) + + ctx.tools.register(defineTool({ + name: 'glob', + description: 'Find files whose paths match a glob pattern. Returns matching paths sorted by modification time, ' + + 'including hidden and ignored files (VCS metadata directories are excluded). ' + + `Returns the first ${caps.maxResults} paths inline; a capped result reports where the complete list was saved.`, + parameters: { + pattern: { type: 'string', required: true, description: 'Glob pattern to match file paths against (e.g. "**/*.ts", "src/**/*.test.js").' }, + path: { type: 'string', description: 'Directory to search in. Defaults to the session workspace; a relative path resolves against it.' }, + }, + timeoutMs: caps.timeoutMs, + async execute(args, exec): Promise { + const input = parseGlobArgs(args) + const run = await runRipgrep(ctx, exec, 'glob', buildGlobCommand(input), caps.rawOutputMaxBytes) + if (run.noMatches) return [{ type: 'text', text: 'No files found' }] + + const retainer = new ItemRetainer({ kind: 'head', maxItems: caps.maxResults }) + const all: string[] = [] + for (const line of run.stdout.split('\n')) { + if (line.length === 0) continue + const displayPath = toWorkdirRelative(line, run.workdir) + all.push(displayPath) + retainer.push(displayPath) + } + const retained = retainer.finish() + + // The complete sorted list is the recovery artifact; save it only when + // the inline page omitted paths (an uncapped result needs no spill file). + const spillPath = retained.truncated + ? await trySaveFormattedResult(ctx, exec, 'glob-results.txt', all.join('\n')) + : undefined + return [{ type: 'text', text: formatGlobOutput(retained, spillPath) }] + }, + presentCall: presentGlobCall, + })) +} diff --git a/packages/fs/tool-fs-search/src/grep.ts b/packages/fs/tool-fs-search/src/grep.ts new file mode 100644 index 0000000000..e5e64e5222 --- /dev/null +++ b/packages/fs/tool-fs-search/src/grep.ts @@ -0,0 +1,314 @@ +/** + * The model-facing `grep` tool: search file contents with a ripgrep regular + * expression. Execution goes through the bash seam (`ctx.bash`) with a fixed + * line-oriented `rg --json` command so file path, line number, and line text + * parse without colon-splitting ambiguity — this module owns the model-facing + * schema, argument validation, shell-safe command construction, `--json` + * record parsing, per-line preview retention, match retention, grouping, and + * formatting; process concerns stay behind `ctx.bash`. + * + * @module @deepseek-ai/dsh-tool-fs-search/grep + */ + +import type { Context } from 'cordis' +import { defineTool } from '@deepseek-ai/dsh-tools' +import type { GenericCallView } from '@deepseek-ai/dsh-tools' +import type { ContentBlock } from '@deepseek-ai/dsh-llm' +import { ItemRetainer, TextRetainer } from '@deepseek-ai/dsh-retention' +import type { RetainedItems } from '@deepseek-ai/dsh-retention' +import type {} from '@deepseek-ai/dsh-bash' +import type {} from '@deepseek-ai/dsh-system-prompt' +import { SearchError, runRipgrep, toWorkdirRelative, trySaveFormattedResult } from './search-core.ts' +import { singleQuote } from './shell-quote.ts' + +/** + * Default cap on flat matches retained inline by one `grep` call (the + * `grepMaxMatches` config), matching Claude Code's default `GrepTool` + * `head_limit`. + */ +export const GREP_MAX_MATCHES = 250 + +/** + * Default cap in bytes on one matched-line preview (the `grepMaxLineBytes` + * config); the cut preserves UTF-8 boundaries. + */ +export const GREP_MAX_LINE_BYTES = 2000 + +/** Resolved grep-tool caps — plugin config after defaulting (see `Config` in index.ts). */ +export interface GrepToolCaps { + /** Max flat matches retained inline; later matches go to the formatted spill file. */ + maxMatches: number + /** Max bytes retained per matched-line preview. */ + maxLineBytes: number + /** Cap on the complete raw `rg` stdout the tool will parse. */ + rawOutputMaxBytes: number + /** Cooperative tool-call budget (ms) attached as `ToolDefinition.timeoutMs`. */ + timeoutMs: number +} + +/** Validated `grep` arguments. */ +export interface GrepInput { + pattern: string + path?: string + include?: string +} + +/** One parsed match: the file, the 1-based line number, and the (possibly previewed) line text. */ +export interface GrepMatch { + path: string + lineNumber: number + line: string +} + +/** + * Reject an `include` that is not ONE positive glob filter: blank strings, + * negated patterns (`!…`), and comma-separated lists. A comma inside a brace + * group is fine — `*.{ts,tsx}` is one glob with alternation, not a list. + */ +function validateInclude(include: string): void { + if (include.trim().length === 0) throw new Error('include must be a non-empty glob when given') + if (include.startsWith('!')) throw new Error('include must be a positive glob filter; negated patterns ("!…") are not supported') + let braceDepth = 0 + for (const char of include) { + if (char === '{') braceDepth++ + else if (char === '}') braceDepth = Math.max(0, braceDepth - 1) + else if (char === ',' && braceDepth === 0) { + throw new Error('include must be one glob, not a comma-separated list (use {a,b} alternation instead)') + } + } +} + +/** + * Validate value constraints the schema DSL can't express: a non-EMPTY + * `pattern` (whitespace is a legitimate regex), a non-blank `path` when given, + * and a single positive `include` glob ({@link GrepInput}). Throws a plain + * `Error` (an ordinary tool argument error) otherwise. + * + * @param args - the schema-validated `grep` arguments. + * @returns the accepted input, unchanged. + */ +export function parseGrepArgs(args: { pattern: string; path?: string; include?: string }): GrepInput { + if (args.pattern.length === 0) throw new Error('pattern must be a non-empty string') + if (args.path !== undefined && args.path.trim().length === 0) throw new Error('path must be a non-empty string when given') + if (args.include !== undefined) validateInclude(args.include) + return { + pattern: args.pattern, + ...args.path !== undefined ? { path: args.path } : {}, + ...args.include !== undefined ? { include: args.include } : {}, + } +} + +/** + * Build the fixed line-oriented `rg --json` command for one `grep` call. Every + * model-controlled value ({@link GrepInput.pattern}, {@link GrepInput.path}, + * {@link GrepInput.include}) passes through {@link singleQuote}; the pattern + * and include ride in `--flag=value` form and the target behind `--`, so a + * leading-dash value can never be parsed as a flag. + * + * @param input - the validated arguments. + * @returns the complete, shell-safe command string. + */ +export function buildGrepCommand(input: GrepInput): string { + const parts = ['rg --json', `--regexp=${singleQuote(input.pattern)}`] + if (input.include !== undefined) parts.push(`--glob=${singleQuote(input.include)}`) + if (input.path !== undefined) parts.push('--', singleQuote(input.path)) + return parts.join(' ') +} + +/** + * The uniform malformed-output failure: raw `rg --json` is an internal + * transport, so a shape surprise is a search failure, not a partial result. + */ +function malformedRecord(detail: string, cause?: unknown): SearchError { + return new SearchError(`grep received malformed ripgrep --json output (${detail})`, 'SEARCH_FAILED', cause !== undefined ? { cause } : undefined) +} + +/** + * Parse one `rg --json` NDJSON line into a match, `undefined` for the + * non-match record types (`begin`/`end`/`context`/`summary`). A line that is + * not JSON, or a `match` record missing its path / line number / line content, + * throws {@link SearchError} `SEARCH_FAILED`. A match whose line is not valid + * UTF-8 (ripgrep sends base64 `bytes` instead of `text`) yields a placeholder + * preview rather than failing the whole search. + */ +function parseRecord(line: string): GrepMatch | undefined { + let parsed: unknown + try { + parsed = JSON.parse(line) + } catch (error: unknown) { + throw malformedRecord('a line is not JSON', error) + } + if (typeof parsed !== 'object' || parsed === null) throw malformedRecord('a record is not an object') + const record = parsed as { type?: unknown; data?: unknown } + // Non-match record types (begin/end/context/summary — and any future type) + // are transport framing, not results: skipped, not malformed. + if (record.type !== 'match') return undefined + if (typeof record.data !== 'object' || record.data === null) throw malformedRecord('a match record has no data') + const data = record.data as { path?: unknown; line_number?: unknown; lines?: unknown } + const pathText = typeof data.path === 'object' && data.path !== null ? (data.path as { text?: unknown }).text : undefined + if (typeof pathText !== 'string') throw malformedRecord('a match record has no path text') + if (typeof data.line_number !== 'number') throw malformedRecord('a match record has no line number') + if (typeof data.lines !== 'object' || data.lines === null) throw malformedRecord('a match record has no line content') + const lines = data.lines as { text?: unknown; bytes?: unknown } + if (typeof lines.text === 'string') { + return { path: pathText, lineNumber: data.line_number, line: lines.text.replace(/\r?\n$/, '') } + } + if (typeof lines.bytes === 'string') { + return { path: pathText, lineNumber: data.line_number, line: '(line is not valid UTF-8)' } + } + throw malformedRecord('a match record has neither line text nor bytes') +} + +/** + * Parse complete `rg --json` stdout into flat matches, in output order (ripgrep + * emits one file's matches contiguously). Only `match` records are consumed. + * + * @param stdout - the complete raw `rg --json` stdout. + * @returns the flat matches; empty for output with no match records. + */ +export function parseGrepMatches(stdout: string): GrepMatch[] { + const matches: GrepMatch[] = [] + for (const line of stdout.split('\n')) { + if (line.length === 0) continue + const match = parseRecord(line) + if (match !== undefined) matches.push(match) + } + return matches +} + +/** + * Bound one matched-line preview to `maxBytes` (UTF-8 boundary preserved) and + * mark the cut. The cap is a per-line budget fact; the complete line stays in + * the searched file for `read`. + * + * @param line - the matched line text (trailing newline already stripped). + * @param maxBytes - the preview budget in bytes. + * @returns the preview, suffixed with ` (line truncated)` when bytes were cut. + */ +export function previewLine(line: string, maxBytes: number): string { + const retainer = new TextRetainer({ kind: 'head', maxBytes }) + retainer.push(line) + const kept = retainer.finish() + return kept.truncated ? `${kept.text} (line truncated)` : kept.text +} + +/** `match` / `matches` for a count. */ +function matchNoun(count: number): string { + return count === 1 ? 'match' : 'matches' +} + +/** + * Group flat matches by file (first-seen order) into the model-facing body: + * each file's display path, then one `Line N: ` row per match. + * + * @param matches - the flat matches to render. + * @returns the grouped body text. + */ +export function formatGrepMatches(matches: GrepMatch[]): string { + const byFile = new Map() + for (const match of matches) { + const group = byFile.get(match.path) + if (group !== undefined) group.push(match) + else byFile.set(match.path, [match]) + } + const sections: string[] = [] + for (const [path, group] of byFile) { + sections.push(`${path}\n${group.map(m => `Line ${m.lineNumber}: ${m.line}`).join('\n')}`) + } + return sections.join('\n\n') +} + +/** + * Format the model-facing `grep` result: a found-count header, the retained + * matches grouped by file, then — when the result was capped — a footer + * carrying either the formatted-spill recovery path or the could-not-save + * explanation. The omitted count is a budget fact: the search itself completed. + * + * @param retained - the retention outcome over every parsed match. + * @param spillPath - the saved complete-result path, or `undefined` when unsaved. + * @returns the model-facing text. + */ +export function formatGrepOutput(retained: RetainedItems, spillPath: string | undefined): string { + const header = retained.truncated + ? `Found ${retained.kept} of ${retained.seen} matches` + : `Found ${retained.seen} ${matchNoun(retained.seen)}` + const body = formatGrepMatches(retained.items) + if (!retained.truncated) return `${header}\n\n${body}` + const recovery = spillPath !== undefined + ? `Full grep result saved to: ${spillPath}. Use read with offset/limit to inspect it.` + : 'The complete result could not be saved; narrow pattern, path, or include to see more.' + return `${header}\n\n${body}\n\n(${recovery})` +} + +/** + * Pending-call presentation: a search card titled by the pattern (and target / + * include filter). + * + * @param args - the raw tool arguments; `pattern`, `path`, and `include` feed the title. + * @returns the generic card view (`kind: 'search'`) shown while the call runs. + */ +export function presentGrepCall(args: { pattern: string; path?: string; include?: string }): GenericCallView { + const where = args.path !== undefined ? ` in ${args.path}` : '' + const filter = args.include !== undefined ? ` (${args.include})` : '' + return { card: 'generic', title: `Grep ${args.pattern}${where}${filter}`, kind: 'search', rawInput: args.pattern } +} + +/** + * Register the `grep` tool and its system-prompt guidance. + * + * @param ctx - the plugin context; registrations are effects scoped to it, and + * execution uses its `bash` service. + * @param caps - the deployment's resolved grep caps (plugin config after defaulting). + */ +export function applyGrepTool(ctx: Context, caps: GrepToolCaps): void { + ctx.systemPrompt.section({ + name: 'tool:grep', + order: 104, + text: 'Use the grep tool — not shell grep or rg — to search file contents. Use read on a matched file when you need surrounding context.', + }) + + ctx.tools.register(defineTool({ + name: 'grep', + description: 'Search file contents with a ripgrep regular expression. Returns matching lines with line numbers, grouped by file. ' + + `Returns the first ${caps.maxMatches} matches inline; a capped result reports where the complete match list was saved. ` + + 'Use read on a matched file for surrounding context.', + parameters: { + pattern: { type: 'string', required: true, description: 'Regular expression to search for (ripgrep syntax).' }, + path: { type: 'string', description: 'File or directory to search. Defaults to the session workspace; a relative path resolves against it.' }, + include: { type: 'string', description: 'One glob filter for which files to search (e.g. "*.ts", "*.{js,jsx}"). Not a list; negation is not supported.' }, + }, + timeoutMs: caps.timeoutMs, + async execute(args, exec): Promise { + const input = parseGrepArgs(args) + const run = await runRipgrep(ctx, exec, 'grep', buildGrepCommand(input), caps.rawOutputMaxBytes) + if (run.noMatches) return [{ type: 'text', text: 'No matches found' }] + + const retainer = new ItemRetainer({ kind: 'head', maxItems: caps.maxMatches }) + const all: GrepMatch[] = [] + for (const raw of parseGrepMatches(run.stdout)) { + const match: GrepMatch = { + path: toWorkdirRelative(raw.path, run.workdir), + lineNumber: raw.lineNumber, + line: previewLine(raw.line, caps.maxLineBytes), + } + all.push(match) + retainer.push(match) + } + const retained = retainer.finish() + + // The spill file stores the FULL formatted match list (same grouped, + // per-line-previewed shape the model saw), so read offset/limit pages the + // same logical result; save only when the inline page omitted matches. + const spillPath = retained.truncated + ? await trySaveFormattedResult( + ctx, + exec, + 'grep-results.txt', + `Found ${all.length} ${matchNoun(all.length)}\n\n${formatGrepMatches(all)}`, + ) + : undefined + return [{ type: 'text', text: formatGrepOutput(retained, spillPath) }] + }, + presentCall: presentGrepCall, + })) +} diff --git a/packages/fs/tool-fs-search/src/index.ts b/packages/fs/tool-fs-search/src/index.ts new file mode 100644 index 0000000000..1fec6a999e --- /dev/null +++ b/packages/fs/tool-fs-search/src/index.ts @@ -0,0 +1,110 @@ +/** + * The model-facing filesystem discovery tool suite (`glob`, `grep`) over the + * bash executor seam (`ctx.bash`). This single plugin registers both tools. + * + * ## Bash-backed, not a `ctx.fs` provider method + * + * Local workspace discovery is a process-backed `rg` workflow, so these tools + * execute through `ctx.bash.resolve(request)` → `ctx.bash.run(spec)` with fixed + * ripgrep command templates — never `ctx.bash.start()`, never a model-visible + * background task. The tool layer owns schemas, argument validation, shell + * quoting ({@link module:@deepseek-ai/dsh-tool-fs-search/shell-quote}), result + * parsing, retention, formatted-result spill, and timeout declaration; the + * bash executor owns request defaulting/capping, subprocess execution, + * process-group termination, environment scrubbing, raw output capture, and + * backend substitution. The package injects `tools`, `systemPrompt`, and + * `bash` — deliberately NOT `fs`, and `ctx.spillFiles` is read opportunistically + * with `ctx.get()` because formatted-result spill is optional. + * + * Returned paths are displayed relative to the resolved bash workdir and are + * follow-up-readable only in co-located deployments where the bash workdir and + * the filesystem `read` root are the same workspace — a documented v1 + * deployment requirement, not runtime-validated. + * + * @module @deepseek-ai/dsh-tool-fs-search + */ + +import type { Context } from 'cordis' +import z from 'schemastery' +import { GLOB_MAX_RESULTS, applyGlobTool } from './glob.ts' +import { GREP_MAX_LINE_BYTES, GREP_MAX_MATCHES, applyGrepTool } from './grep.ts' +import { RAW_OUTPUT_MAX_BYTES, SEARCH_TIMEOUT_MS } from './search-core.ts' + +export { GLOB_MAX_RESULTS, GLOB_VCS_EXCLUDES, applyGlobTool, buildGlobCommand, formatGlobOutput, parseGlobArgs, presentGlobCall } from './glob.ts' +export type { GlobInput, GlobToolCaps } from './glob.ts' +export { + GREP_MAX_LINE_BYTES, + GREP_MAX_MATCHES, + applyGrepTool, + buildGrepCommand, + formatGrepMatches, + formatGrepOutput, + parseGrepArgs, + parseGrepMatches, + presentGrepCall, + previewLine, +} from './grep.ts' +export type { GrepInput, GrepMatch, GrepToolCaps } from './grep.ts' +export { RAW_OUTPUT_MAX_BYTES, SEARCH_TIMEOUT_MS, SearchError, runRipgrep, toWorkdirRelative, trySaveFormattedResult } from './search-core.ts' +export type { RipgrepRun, SearchErrorCode } from './search-core.ts' +export { singleQuote } from './shell-quote.ts' + +/** Cordis plugin name used by loader diagnostics. */ +export const name = 'tool-fs-search' + +/** Services required by the search tool suite (`spillFiles` is optional, read via `ctx.get()`). */ +export const inject = ['tools', 'systemPrompt', 'bash'] + +/** Plugin config (all optional — `Config` supplies the defaults). */ +export interface Config { + /** Max paths one `glob` call retains inline; later paths go to the formatted spill file. */ + globMaxResults?: number + /** Max flat matches one `grep` call retains inline; later matches go to the formatted spill file. */ + grepMaxMatches?: number + /** Max bytes retained for one matched-line preview (the cut preserves UTF-8 boundaries). */ + grepMaxLineBytes?: number + /** Max complete raw `rg` stdout bytes a search will parse; larger raw output fails with `SEARCH_RAW_OUTPUT_OVERFLOW`. */ + rawOutputMaxBytes?: number + /** Cooperative tool-call timeout budget (ms) on both tools, enforced by `@deepseek-ai/dsh-timeout-policy` through `exec.signal`. */ + timeoutMs?: number +} + +export const Config: z = z.object({ + globMaxResults: z.number().default(GLOB_MAX_RESULTS), + grepMaxMatches: z.number().default(GREP_MAX_MATCHES), + grepMaxLineBytes: z.number().default(GREP_MAX_LINE_BYTES), + rawOutputMaxBytes: z.number().default(RAW_OUTPUT_MAX_BYTES), + timeoutMs: z.number().default(SEARCH_TIMEOUT_MS), +}) + +/** The shape after schemastery applied the defaults. */ +type ResolvedConfig = Required + +/** Every search cap counts items/bytes/milliseconds — a positive integer, or retention and timeout arithmetic misbehaves silently. */ +function assertPositiveInteger(name: string, value: number): void { + if (!Number.isInteger(value) || value < 1) { + throw new Error(`tool-fs-search: ${name} must be a positive integer`) + } +} + +/** Register the `glob`/`grep` filesystem discovery tool suite. */ +export function apply(ctx: Context, config: Config): void { + // schemastery (Config) has already filled every defaulted field. + const resolved = config as ResolvedConfig + assertPositiveInteger('globMaxResults', resolved.globMaxResults) + assertPositiveInteger('grepMaxMatches', resolved.grepMaxMatches) + assertPositiveInteger('grepMaxLineBytes', resolved.grepMaxLineBytes) + assertPositiveInteger('rawOutputMaxBytes', resolved.rawOutputMaxBytes) + assertPositiveInteger('timeoutMs', resolved.timeoutMs) + applyGlobTool(ctx, { + maxResults: resolved.globMaxResults, + rawOutputMaxBytes: resolved.rawOutputMaxBytes, + timeoutMs: resolved.timeoutMs, + }) + applyGrepTool(ctx, { + maxMatches: resolved.grepMaxMatches, + maxLineBytes: resolved.grepMaxLineBytes, + rawOutputMaxBytes: resolved.rawOutputMaxBytes, + timeoutMs: resolved.timeoutMs, + }) +} diff --git a/packages/fs/tool-fs-search/src/search-core.ts b/packages/fs/tool-fs-search/src/search-core.ts new file mode 100644 index 0000000000..2d9d0b63dd --- /dev/null +++ b/packages/fs/tool-fs-search/src/search-core.ts @@ -0,0 +1,257 @@ +/** + * Shared execution plumbing for the `glob` / `grep` search tools: the + * package-owned `SEARCH_*` error vocabulary, one bash-seam run helper that + * turns a fixed `rg` command into complete raw stdout, the best-effort + * formatted-result spill handoff, and workdir-relative path display. + * + * Both tools execute through `ctx.bash.resolve(request)` → `ctx.bash.run(spec)` + * as ordinary foreground tool calls — never `ctx.bash.start()`, never a + * model-visible background task. Raw `rg` stdout is an internal transport + * detail: when the executor truncates it, the ONLY recovery source is the + * executor's local raw spill file, read here up to `rawOutputMaxBytes` and + * never exposed to the model. The model-facing recovery artifact is the + * formatted result saved through `ctx.spillFiles.saveText()` + * ({@link trySaveFormattedResult}) — a different artifact from the bash raw + * spill file. + * + * @module @deepseek-ai/dsh-tool-fs-search/search-core + */ + +import { readFile, stat } from 'node:fs/promises' +import { isAbsolute, relative, sep } from 'node:path' +import type { Context } from 'cordis' +import { HarnessError } from '@deepseek-ai/dsh-llm' +import type { BashRunResult, CollectedOutput } from '@deepseek-ai/dsh-bash' +import type { SaveTextSpill } from '@deepseek-ai/dsh-spill' +import type { ToolExecution } from '@deepseek-ai/dsh-tools' + +/** + * Default cap on the complete raw `rg` stdout the tools will parse (the + * `rawOutputMaxBytes` config), matching Claude Code's ripgrep raw buffer. + */ +export const RAW_OUTPUT_MAX_BYTES = 20_000_000 + +/** + * Default cooperative tool-call timeout budget in milliseconds (the `timeoutMs` + * config), attached to both tool definitions for + * `@deepseek-ai/dsh-timeout-policy` to enforce through `exec.signal`. + */ +export const SEARCH_TIMEOUT_MS = 30_000 + +/** + * Stable, machine-routable codes for search failures. Package-owned (not + * `FsErrorCode`) because these tools are bash-backed discovery, not `ctx.fs` + * provider operations: `SEARCH_INVALID_PATTERN` — ripgrep rejected the regex or + * glob; `SEARCH_FAILED` — the search could not run or its output could not be + * parsed (missing `rg`, inaccessible target, signal kill, malformed `--json`); + * `SEARCH_RAW_OUTPUT_OVERFLOW` — raw `rg` output exceeded `rawOutputMaxBytes` + * (or was truncated with no recovery file); `SEARCH_ABORTED` — the tool + * timeout, caller cancellation, or the bash executor's own timeout cut the + * search short. + */ +export type SearchErrorCode = + | 'SEARCH_INVALID_PATTERN' + | 'SEARCH_FAILED' + | 'SEARCH_RAW_OUTPUT_OVERFLOW' + | 'SEARCH_ABORTED' + +/** + * Typed search failure. Extends {@link HarnessError} so it carries a stable + * {@link SearchErrorCode} and chains `cause`; the tool registry surfaces + * `{ name, code }` on `isError` results so retry/permission/UI layers can + * branch without parsing messages. + */ +export class SearchError extends HarnessError { + override readonly code: SearchErrorCode + + constructor(message: string, code: SearchErrorCode, options?: ErrorOptions) { + super(message, code, options) + this.code = code + } +} + +/** The completed acquisition of one `rg` run: complete stdout plus the resolved workdir. */ +export interface RipgrepRun { + /** Complete raw stdout — inline executor text, or the raw spill file's content. */ + stdout: string + /** True when ripgrep exited 1: a successful search with zero results. */ + noMatches: boolean + /** The resolved working directory the command ran in (the display-relativization base). */ + workdir: string +} + +/** + * The retained stderr tail as a diagnostic excerpt, with a truncation note when + * the executor dropped bytes (the tool never reads `stderr.spillPath`). + */ +function stderrExcerpt(stderr: CollectedOutput): string { + const text = stderr.text.trim() + if (text.length === 0) return '' + return stderr.truncated ? `${text} [stderr truncated]` : text +} + +/** Classify a nonzero-exit `rg` run into the search error vocabulary (invalid pattern vs missing `rg` vs everything else). */ +function classifyRunFailure(toolName: string, result: BashRunResult): SearchError { + const stderr = stderrExcerpt(result.stderr) + if (/regex parse error|error parsing glob/i.test(stderr)) { + return new SearchError(`${toolName} pattern rejected by ripgrep: ${stderr}`, 'SEARCH_INVALID_PATTERN') + } + if (result.exitCode === 127 || /command not found/i.test(stderr)) { + return new SearchError(`${toolName} requires ripgrep (rg) on the bash executor's PATH${stderr.length > 0 ? `: ${stderr}` : ''}`, 'SEARCH_FAILED') + } + return new SearchError(`${toolName} search failed (exit ${result.exitCode})${stderr.length > 0 ? `: ${stderr}` : ''}`, 'SEARCH_FAILED') +} + +/** + * Acquire the COMPLETE raw stdout of a finished run. Untruncated stdout is used + * as-is; truncated stdout is recovered from the executor's local raw spill file + * only when the complete file fits within `rawOutputMaxBytes`. A missing spill + * path or an over-cap file is a clear failure telling the model to narrow the + * search — never a silently-partial parse. + */ +async function completeStdout(toolName: string, result: BashRunResult, rawOutputMaxBytes: number): Promise { + if (!result.stdout.truncated) return result.stdout.text + const narrow = 'narrow pattern, path, or include and retry' + const spillPath = result.stdout.spillPath + if (spillPath === undefined) { + throw new SearchError( + `${toolName} produced more raw output than the bash executor retained and no raw spill file is available; ${narrow}`, + 'SEARCH_RAW_OUTPUT_OVERFLOW', + ) + } + try { + const { size } = await stat(spillPath) + if (size > rawOutputMaxBytes) { + throw new SearchError( + `${toolName} produced ${size} bytes of raw output, over the ${rawOutputMaxBytes}-byte cap; ${narrow}`, + 'SEARCH_RAW_OUTPUT_OVERFLOW', + ) + } + return await readFile(spillPath, 'utf8') + } catch (error: unknown) { + if (error instanceof SearchError) throw error + throw new SearchError(`${toolName} could not read the executor's raw output spill file`, 'SEARCH_FAILED', { cause: error }) + } +} + +/** + * Run one fixed `rg` command through the bash seam and return its complete raw + * stdout. The bash request workdir is the calling agent's session cwd + * (`exec.agent.session.header.cwd`) when available — mirroring `dsh-tool-bash` / + * `dsh-tool-fs` — else omitted so the implementation's `resolve()` applies its + * configured default. `exec.signal` is forwarded so the cooperative tool + * timeout (`@deepseek-ai/dsh-timeout-policy`) and caller cancellation kill the + * command; the bash backend's own timeout stays a second safety cap. + * + * Exit semantics are tool-owned: exit 0 is success with results, exit 1 is + * success with zero results (`noMatches`), anything else throws a + * {@link SearchError} (abort/timeout → `SEARCH_ABORTED`, invalid pattern → + * `SEARCH_INVALID_PATTERN`, the rest → `SEARCH_FAILED` / + * `SEARCH_RAW_OUTPUT_OVERFLOW`). + * + * @param ctx - the plugin context; execution uses its `bash` service. + * @param exec - the tool-execution context; supplies the session cwd and the abort signal. + * @param toolName - `glob` or `grep`, used in error messages. + * @param command - the fully-quoted `rg` command string (every model value already through `singleQuote`). + * @param rawOutputMaxBytes - cap on the complete raw stdout the tool will parse. + * @returns the complete stdout, the zero-result flag, and the resolved workdir. + */ +export async function runRipgrep( + ctx: Context, + exec: ToolExecution, + toolName: string, + command: string, + rawOutputMaxBytes: number, +): Promise { + const cwd = exec.agent?.session.header.cwd + const spec = ctx.bash.resolve({ + command, + ...cwd !== undefined ? { workdir: cwd } : {}, + ...exec.signal ? { signal: exec.signal } : {}, + }) + const result = await ctx.bash.run(spec) + if (result.aborted) { + throw new SearchError(`${toolName} was aborted before completion (tool timeout or caller cancellation)`, 'SEARCH_ABORTED') + } + if (result.timedOut) { + throw new SearchError(`${toolName} timed out after ${result.timeoutMs}ms in the bash executor; narrow pattern, path, or include and retry`, 'SEARCH_ABORTED') + } + if (result.signal !== null || result.exitCode === null) { + throw new SearchError(`${toolName} search command was killed by signal ${result.signal ?? '(unknown)'}`, 'SEARCH_FAILED') + } + if (result.exitCode !== 0 && result.exitCode !== 1) { + throw classifyRunFailure(toolName, result) + } + const stdout = await completeStdout(toolName, result, rawOutputMaxBytes) + return { stdout, noMatches: result.exitCode === 1, workdir: spec.workdir } +} + +/** + * Map an `rg` output path to its display form: absolute paths inside the + * resolved bash workdir become workdir-relative; everything else (relative + * output, paths outside the workdir) passes through unchanged. Display-only — + * returned paths are follow-up-readable in co-located bash/filesystem + * deployments where both resolve the same workspace (the documented v1 + * deployment requirement). + * + * @param path - one path as ripgrep printed it. + * @param workdir - the resolved bash workdir the command ran in. + * @returns the workdir-relative display path when possible, else `path` unchanged. + */ +export function toWorkdirRelative(path: string, workdir: string): string { + if (!isAbsolute(path)) return path + const rel = relative(workdir, path) + if (rel.length === 0) return '.' + if (rel === '..' || rel.startsWith(`..${sep}`)) return path + return rel +} + +/** + * Best-effort save of one COMPLETE formatted search result through + * `ctx.spillFiles.saveText()` — the model-facing recovery path for a capped + * result. `spillFiles` is read with `ctx.get()` (not static inject) because + * formatted-result spill is optional; the spill owner is the calling agent's + * session header id and the source is the tool execution identity. A missing + * backend, a call with no session owner, or a `saveText()` rejection logs a + * warning and returns `undefined` — the caller keeps the inline result and + * reports that the complete result could not be saved; search success never + * turns into `isError` because spill storage is unavailable. + * + * @param ctx - the plugin context; `spillFiles` is looked up opportunistically. + * @param exec - the tool-execution context; supplies the owning session, tool name, and call id. + * @param suggestedName - the backend-sanitized filename hint (e.g. `grep-results.txt`). + * @param content - the complete formatted result to persist. + * @returns the saved spill path, or `undefined` when the result could not be saved. + */ +export async function trySaveFormattedResult( + ctx: Context, + exec: ToolExecution, + suggestedName: string, + content: string, +): Promise { + const sessionId = exec.agent?.session.header.id + if (sessionId === undefined) { + ctx.logger.warn(`tool-fs-search: no session owner for ${exec.name} result; complete result not saved`) + return undefined + } + const spillFiles = ctx.get('spillFiles') + if (!spillFiles) { + ctx.logger.warn(`tool-fs-search: no ctx.spillFiles backend loaded; complete ${exec.name} result not saved`) + return undefined + } + const save: SaveTextSpill = { + owner: { sessionId }, + source: { toolName: exec.name, callId: exec.callId, label: 'result' }, + suggestedName, + content, + } + try { + const { path } = await spillFiles.saveText(save) + return path + } catch (error: unknown) { + // Best-effort: a storage failure must never fail the search or hide the + // inline result — the footer reports the unsaved remainder instead. + ctx.logger.warn(`tool-fs-search: saveText failed for ${exec.name}: ${String(error)}; complete result not saved`) + return undefined + } +} diff --git a/packages/fs/tool-fs-search/src/shell-quote.ts b/packages/fs/tool-fs-search/src/shell-quote.ts new file mode 100644 index 0000000000..9453b8e255 --- /dev/null +++ b/packages/fs/tool-fs-search/src/shell-quote.ts @@ -0,0 +1,27 @@ +/** + * The one shell-quoting helper both search tools MUST route every + * model-controlled value through before it enters an `rg` command string. The + * bash seam (`ctx.bash`) accepts a command STRING, not an argv vector, so this + * is the safety boundary that stops a `pattern`, `path`, or `include` from + * breaking out of its argument and injecting shell syntax. + * + * Command builders in `glob.ts` / `grep.ts` must never hand-roll quoting or + * concatenate an unquoted model value — they call {@link singleQuote}. + * + * @module @deepseek-ai/dsh-tool-fs-search/shell-quote + */ + +/** + * POSIX single-quote a string for safe use as ONE shell word. Wraps the value + * in single quotes and rewrites every embedded single quote as `'\''` (close + * quote, an escaped literal quote, reopen quote). Inside single quotes the shell + * treats every other byte literally — spaces, newlines, `$`, backticks, `;`, + * `|`, `&`, glob metacharacters, and a leading `-` are all inert — so the result + * is a single, injection-safe argument regardless of the input. + * + * @param value - the raw, possibly model-controlled string to quote. + * @returns the value wrapped as one safe single-quoted shell word. + */ +export function singleQuote(value: string): string { + return `'${value.replaceAll("'", "'\\''")}'` +} diff --git a/packages/fs/tool-fs-search/tests/integration.spec.ts b/packages/fs/tool-fs-search/tests/integration.spec.ts new file mode 100644 index 0000000000..74a418238b --- /dev/null +++ b/packages/fs/tool-fs-search/tests/integration.spec.ts @@ -0,0 +1,161 @@ +/** + * Integration tests: the REAL local bash executor (`dsh-bash-local`) plus a + * REAL ripgrep binary, exercised through `ctx.tools.execute()`. These verify + * the WORLD — actual files on disk are discovered and grepped, hostile + * patterns stay inert in a real shell, and real `rg` stderr classifies into + * the `SEARCH_*` vocabulary. The whole suite self-skips when `rg` is not on + * PATH (a CI accommodation mirroring the keyless e2e skip); the fake-executor + * suite (tools.spec.ts) carries the coverage gate. + */ + +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { spawnSync } from 'node:child_process' +import { mkdir, mkdtemp, rm, utimes, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { Context } from 'cordis' +import { CallId } from '@deepseek-ai/dsh-llm' +import SystemPrompt from '@deepseek-ai/dsh-system-prompt' +import ToolRegistry from '@deepseek-ai/dsh-tools' +import { LocalBashExecutor } from '@deepseek-ai/dsh-bash-local' +import * as ToolFsSearch from '@deepseek-ai/dsh-tool-fs-search' + +const hasRg = spawnSync('rg', ['--version'], { encoding: 'utf8' }).status === 0 + +let dir: string +let ctx: Context + +let callCounter = 0 +function call(name: string, args: unknown, agentObj?: object) { + return ctx.tools.execute({ + callId: CallId(`it-${++callCounter}`), + name, + arguments: args, + ...agentObj ? { agent: agentObj as never } : {}, + }) +} + +function text(result: { content: { type: string; text?: string }[] }): string { + return result.content.filter(b => b.type === 'text').map(b => b.text).join('') +} + +describe.skipIf(!hasRg)('search tools over the real bash executor + real rg', () => { + beforeEach(async () => { + dir = await mkdtemp(join(tmpdir(), 'dsh-search-int-')) + await mkdir(join(dir, 'src'), { recursive: true }) + await mkdir(join(dir, '.git'), { recursive: true }) + await mkdir(join(dir, 'spaced dir'), { recursive: true }) + await writeFile(join(dir, 'src', 'alpha.ts'), 'export const alpha = 1\n// TODO: refit alpha\n') + await writeFile(join(dir, 'src', 'beta.ts'), 'export const beta = 2\n') + await writeFile(join(dir, 'notes.md'), 'alpha appears here too\n') + await writeFile(join(dir, '.hidden.ts'), 'export const hidden = 3\n') + await writeFile(join(dir, '.git', 'config.ts'), 'never listed\n') + await writeFile(join(dir, 'spaced dir', "wei'rd \"name\".ts"), 'const inside = true\n') + // Deterministic --sort=modified order: alpha oldest, beta newest. + await utimes(join(dir, 'src', 'alpha.ts'), new Date(2000, 0, 1), new Date(2000, 0, 1)) + await utimes(join(dir, 'src', 'beta.ts'), new Date(2020, 0, 1), new Date(2020, 0, 1)) + + ctx = new Context() + await ctx.plugin(SystemPrompt) + await ctx.plugin(ToolRegistry) + await ctx.plugin(LocalBashExecutor, { cwd: dir, timeoutMs: 20_000 }) + await ctx.plugin(ToolFsSearch) + }) + + afterEach(async () => { + await rm(dir, { recursive: true, force: true }) + }) + + describe('glob', () => { + it('discovers files by pattern, sorted by modification time, hidden included, .git excluded', async () => { + const result = await call('glob', { pattern: '**/*.ts' }) + expect(result.isError).toBe(false) + const paths = text(result).split('\n') + expect(paths.indexOf('src/alpha.ts')).toBeLessThan(paths.indexOf('src/beta.ts')) + expect(paths).toContain('.hidden.ts') + expect(paths).toContain("spaced dir/wei'rd \"name\".ts") + expect(paths).not.toContain('.git/config.ts') + expect(paths).not.toContain('notes.md') + }) + + it('scopes to a directory search root (path arg)', async () => { + const result = await call('glob', { pattern: '*.ts', path: 'src' }) + expect(text(result).split('\n').sort()).toEqual(['src/alpha.ts', 'src/beta.ts']) + }) + + it('reports zero discoveries as No files found', async () => { + expect(text(await call('glob', { pattern: '*.nomatch' }))).toBe('No files found') + }) + + it('classifies an invalid glob as SEARCH_INVALID_PATTERN', async () => { + const result = await call('glob', { pattern: '[' }) + expect(result.isError).toBe(true) + expect(result.error).toMatchObject({ name: 'SearchError', code: 'SEARCH_INVALID_PATTERN' }) + }) + }) + + describe('grep', () => { + it('greps a directory tree with grouped, line-numbered output', async () => { + const result = await call('grep', { pattern: 'alpha' }) + expect(result.isError).toBe(false) + const output = text(result) + expect(output).toContain('Found 3 matches') + expect(output).toContain('src/alpha.ts\nLine 1: export const alpha = 1\nLine 2: // TODO: refit alpha') + expect(output).toContain('notes.md\nLine 1: alpha appears here too') + }) + + it('greps a single FILE target', async () => { + const result = await call('grep', { pattern: 'alpha', path: 'notes.md' }) + expect(text(result)).toBe('Found 1 match\n\nnotes.md\nLine 1: alpha appears here too') + }) + + it('greps a directory target with an include filter', async () => { + const result = await call('grep', { pattern: 'alpha', path: '.', include: '*.ts' }) + const output = text(result) + expect(output).toContain('alpha.ts') + expect(output).not.toContain('notes.md') + }) + + it('a hostile pattern stays inert (no command substitution, the world untouched)', async () => { + const canary = join(dir, 'pwned') + const result = await call('grep', { pattern: `$(touch ${canary})` }) + expect(result.isError).toBe(false) // exit 1: found nothing, executed nothing + expect(text(result)).toBe('No matches found') + expect(spawnSync('test', ['-e', canary]).status).not.toBe(0) + }) + + it('a leading-dash pattern is a pattern, not a flag', async () => { + await writeFile(join(dir, 'dashes.txt'), 'value --flag value\n') + const result = await call('grep', { pattern: '--flag', path: 'dashes.txt' }) + expect(text(result)).toBe('Found 1 match\n\ndashes.txt\nLine 1: value --flag value') + }) + + it('classifies a real rg regex error as SEARCH_INVALID_PATTERN', async () => { + const result = await call('grep', { pattern: '(unclosed' }) + expect(result.isError).toBe(true) + expect(result.error).toMatchObject({ code: 'SEARCH_INVALID_PATTERN' }) + }) + + it('classifies a missing target as SEARCH_FAILED', async () => { + const result = await call('grep', { pattern: 'x', path: 'no-such-dir' }) + expect(result.isError).toBe(true) + expect(result.error).toMatchObject({ code: 'SEARCH_FAILED' }) + }) + }) + + describe('per-session cwd', () => { + it('resolves the search in the SESSION workspace, not the executor config cwd', async () => { + const sessionDir = await mkdtemp(join(tmpdir(), 'dsh-search-session-')) + try { + await writeFile(join(sessionDir, 'only-here.ts'), 'const sessionFile = true\n') + const agentObj = { session: { header: { id: 'session-int', cwd: sessionDir } } } + const globbed = await call('glob', { pattern: '*.ts' }, agentObj) + expect(text(globbed)).toBe('only-here.ts') + const grepped = await call('grep', { pattern: 'sessionFile' }, agentObj) + expect(text(grepped)).toContain('only-here.ts\nLine 1: const sessionFile = true') + } finally { + await rm(sessionDir, { recursive: true, force: true }) + } + }) + }) +}) diff --git a/packages/fs/tool-fs-search/tests/load-path.spec.ts b/packages/fs/tool-fs-search/tests/load-path.spec.ts new file mode 100644 index 0000000000..d3c28619a3 --- /dev/null +++ b/packages/fs/tool-fs-search/tests/load-path.spec.ts @@ -0,0 +1,50 @@ +/** + * Real-load-path guard for @deepseek-ai/dsh-tool-fs-search. `tool-fs-search` is + * a NAMESPACE plugin with `inject` — so a stray `export default apply` would + * make the cordis Loader's `unwrapExports` (`exports.default ?? exports`) + * collapse the module to the bare `apply` function, DROPPING `inject`. The + * plugin would then read `ctx.bash` without having injected it and throw + * `cannot get property … without inject` the moment it loads (postmortem 0001). + * + * A hand-built `ctx.plugin({ apply, inject })` mount CANNOT catch that — it + * bypasses `unwrapExports`. So this test unwraps the module through the REAL + * `Loader.prototype.unwrapExports` and mounts the result over a bash executor, + * exercising the exact path the Loader uses. Prove the guard bites: add + * `export default apply` to `src/index.ts`, watch this go red, revert. + */ + +import { describe, expect, it } from 'vitest' +import { Context } from 'cordis' +import Loader from '@cordisjs/plugin-loader' +import SystemPrompt from '@deepseek-ai/dsh-system-prompt' +import ToolRegistry from '@deepseek-ai/dsh-tools' +import { LocalBashExecutor } from '@deepseek-ai/dsh-bash-local' +import * as toolFsSearch from '@deepseek-ai/dsh-tool-fs-search' + +describe('dsh-tool-fs-search real-load-path guard', () => { + it('has no default export and keeps name/inject/Config through unwrapExports', () => { + expect('default' in toolFsSearch).toBe(false) + + const loader = Object.create(Loader.prototype) as Loader + const unwrapped = loader.unwrapExports(toolFsSearch) as Record + expect(unwrapped).toBe(toolFsSearch) + expect(unwrapped.name).toBe('tool-fs-search') + expect(unwrapped.inject).toEqual(['tools', 'systemPrompt', 'bash']) + expect(typeof unwrapped.Config).toBe('function') + expect(typeof unwrapped.apply).toBe('function') + }) + + it('boots over ctx.bash through the unwrapped module without an inject error', async () => { + const ctx = new Context() + await ctx.plugin(SystemPrompt) + await ctx.plugin(ToolRegistry) + await ctx.plugin(LocalBashExecutor, {}) + + const loader = Object.create(Loader.prototype) as Loader + const unwrapped = loader.unwrapExports(toolFsSearch) as Parameters[0] + // A collapsed export shape (dropped inject) would throw "without inject" here. + const fiber = await ctx.plugin(unwrapped) + expect(ctx.tools.schemas().map(s => s.name)).toEqual(expect.arrayContaining(['glob', 'grep'])) + await fiber.dispose() + }) +}) diff --git a/packages/fs/tool-fs-search/tests/shell-quote.spec.ts b/packages/fs/tool-fs-search/tests/shell-quote.spec.ts new file mode 100644 index 0000000000..84c8506be1 --- /dev/null +++ b/packages/fs/tool-fs-search/tests/shell-quote.spec.ts @@ -0,0 +1,59 @@ +/** + * Unit tests for the shell-quoting safety boundary, plus a REAL round-trip: + * every adversarial value, quoted, must survive `bash -c "printf '%s' "` + * byte-for-byte — proving the quoting is inert in an actual shell, not just + * against a mental model of one. + */ + +import { describe, expect, it } from 'vitest' +import { spawnSync } from 'node:child_process' +import { singleQuote } from '@deepseek-ai/dsh-tool-fs-search' + +/** Adversarial values a model could pass as pattern / path / include. */ +const HOSTILE: readonly string[] = [ + 'plain', + 'with spaces', + "it's got 'quotes'", + '"double quoted"', + '$(rm -rf /tmp/nope)', + '`touch /tmp/nope`', + '$HOME and ${PATH}', + 'semi;colon && chain || pipe | bg &', + 'newline\nin the middle', + '-leading-dash', + '--leading-double-dash', + '*?[a-z]{x,y}', + '!bang', + '\\backslash\\', + '~tilde', + '# not a comment', + '>redirect &1', +] + +describe('singleQuote', () => { + it('wraps a plain value in single quotes', () => { + expect(singleQuote('abc')).toBe("'abc'") + }) + + it("rewrites embedded single quotes as '\\''", () => { + expect(singleQuote("a'b")).toBe("'a'\\''b'") + expect(singleQuote("''")).toBe("''\\'''\\'''") + }) + + it.each(HOSTILE.map(value => [JSON.stringify(value), value] as const))( + 'round-trips %s through a real bash -c unchanged', + (_label, value) => { + const result = spawnSync('bash', ['-c', `printf '%s' ${singleQuote(value)}`], { encoding: 'utf8' }) + expect(result.status).toBe(0) + expect(result.stdout).toBe(value) + }, + ) + + it('a quoted command substitution does not execute (the world stays untouched)', () => { + const canary = `/tmp/dsh-quote-canary-${process.pid}` + const result = spawnSync('bash', ['-c', `printf '%s' ${singleQuote(`$(touch ${canary})`)}`], { encoding: 'utf8' }) + expect(result.stdout).toBe(`$(touch ${canary})`) + // The canary file must NOT exist — the substitution stayed literal. + expect(spawnSync('test', ['-e', canary]).status).not.toBe(0) + }) +}) diff --git a/packages/fs/tool-fs-search/tests/tools.spec.ts b/packages/fs/tool-fs-search/tests/tools.spec.ts new file mode 100644 index 0000000000..6a539956a4 --- /dev/null +++ b/packages/fs/tool-fs-search/tests/tools.spec.ts @@ -0,0 +1,623 @@ +/** + * Consumer-surface tests for the search tools over a FAKE bash executor and a + * FAKE spill backend, exercised through `ctx.tools.execute()` so nothing + * bypasses the tool registry. The fake executor makes every seam outcome + * scriptable — truncated stdout with/without a raw spill file, abort/timeout, + * signal kills, ripgrep exit codes — so these tests verify schemas, argument + * validation, shell-safe command construction, workdir derivation, signal + * forwarding, `SEARCH_*` error classification, retention, formatted-result + * spill handoff, and the no-background-task invariant. Real-`rg` behavior is + * pinned separately in integration.spec.ts. + */ + +import { afterEach, describe, expect, it } from 'vitest' +import { mkdtemp, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { Context } from 'cordis' +import { CallId } from '@deepseek-ai/dsh-llm' +import SystemPrompt, { renderPrompt } from '@deepseek-ai/dsh-system-prompt' +import ToolRegistry from '@deepseek-ai/dsh-tools' +import { BashExecutor } from '@deepseek-ai/dsh-bash' +import type { BashExecRequest, BashExecSpec, BashRunResult, BashTask, BashTaskId, BashTaskRead, OwnerToken } from '@deepseek-ai/dsh-bash' +import { SpillFiles, SpillPath } from '@deepseek-ai/dsh-spill' +import type { SaveTextSpill, SpillRef } from '@deepseek-ai/dsh-spill' +import * as ToolFsSearch from '@deepseek-ai/dsh-tool-fs-search' +import { + buildGlobCommand, + buildGrepCommand, + formatGrepMatches, + parseGrepMatches, + presentGlobCall, + presentGrepCall, + previewLine, + toWorkdirRelative, +} from '@deepseek-ai/dsh-tool-fs-search' + +/** A successful run result over the given stdout; overrides script the failure shapes. */ +function runResult(stdout: string, overrides?: Partial): BashRunResult { + return { + exitCode: 0, + signal: null, + timedOut: false, + aborted: false, + timeoutMs: 60_000, + stdout: { text: stdout, truncated: false }, + stderr: { text: '', truncated: false }, + ...overrides, + } +} + +/** + * A scriptable fake executor: `resolve()` mirrors the real request→spec + * defaulting (workdir falls back to `/work`), `run()` returns whatever the + * test armed via `handler`, and `start()` throws — the search tools must NEVER + * create a background task. + */ +class FakeBash extends BashExecutor { + requests: BashExecRequest[] = [] + specs: BashExecSpec[] = [] + startCalls = 0 + handler: (spec: BashExecSpec) => BashRunResult = () => runResult('') + + override resolve(request: BashExecRequest): BashExecSpec { + this.requests.push(request) + return { + command: request.command, + workdir: request.workdir ?? '/work', + timeoutMs: request.timeoutMs ?? 60_000, + signal: request.signal, + owner: request.owner, + } + } + override run(spec: BashExecSpec): Promise { + this.specs.push(spec) + return Promise.resolve(this.handler(spec)) + } + override start(): BashTask { + this.startCalls++ + throw new Error('search tools must never start a background task') + } + override get(): BashTask | undefined { + return undefined + } + override ownerOf(): OwnerToken | undefined { + return undefined + } + override list(): BashTask[] { + return [] + } + override readOutput(id: BashTaskId): BashTaskRead { + throw new Error(`unknown bash task ${id}`) + } + override kill(id: BashTaskId): boolean { + throw new Error(`unknown bash task ${id}`) + } +} + +/** A recording spill backend; arm `failWith` to script a storage failure. */ +class FakeSpill extends SpillFiles { + saves: SaveTextSpill[] = [] + failWith?: Error + + override saveText(input: SaveTextSpill): Promise { + if (this.failWith) return Promise.reject(this.failWith) + this.saves.push(input) + return Promise.resolve({ path: SpillPath(`/spill/${input.suggestedName}`), bytes: Buffer.byteLength(input.content, 'utf8') }) + } +} + +interface SetupOptions { + config?: ToolFsSearch.Config + spill?: boolean +} + +async function setup(options: SetupOptions = {}) { + const ctx = new Context() + await ctx.plugin(SystemPrompt) + await ctx.plugin(ToolRegistry) + await ctx.plugin(FakeBash) + if (options.spill === true) await ctx.plugin(FakeSpill) + const fiber = await ctx.plugin(ToolFsSearch, options.config) + const bash = ctx.bash as FakeBash + const spill = options.spill === true ? ctx.get('spillFiles') as FakeSpill : undefined + return { ctx, bash, spill, fiber } +} + +/** A stand-in agent whose session header carries the given cwd (and a stable id). */ +const agent = (cwd?: string) => ({ session: { header: { id: 'session-1', ...cwd !== undefined ? { cwd } : {} } } }) + +let callCounter = 0 +function call(ctx: Context, name: string, args: unknown, options: { agent?: object; signal?: AbortSignal } = {}) { + return ctx.tools.execute({ + callId: CallId(`call-${++callCounter}`), + name, + arguments: args, + ...options.agent ? { agent: options.agent as never } : {}, + ...options.signal ? { signal: options.signal } : {}, + }) +} + +function text(result: { content: { type: string; text?: string }[] }): string { + return result.content.filter(b => b.type === 'text').map(b => b.text).join('') +} + +/** One rg --json match record line. */ +function matchLine(path: string, lineNumber: number, lineText: string): string { + return JSON.stringify({ type: 'match', data: { path: { text: path }, lines: { text: lineText }, line_number: lineNumber, absolute_offset: 0, submatches: [] } }) +} + +describe('registration', () => { + it('registers glob and grep with their prompt sections', async () => { + const { ctx } = await setup() + expect(ctx.tools.schemas().map(s => s.name).sort()).toEqual(['glob', 'grep']) + const prompt = renderPrompt(await ctx.systemPrompt.assemble()) + expect(prompt).toContain('Use the glob tool') + expect(prompt).toContain('Use the grep tool') + }) + + it('stays pending until ctx.bash exists (inject)', async () => { + const ctx = new Context() + await ctx.plugin(SystemPrompt) + await ctx.plugin(ToolRegistry) + await ctx.plugin(ToolFsSearch) // no bash executor + expect(ctx.tools.schemas()).toHaveLength(0) + }) + + it('unregisters everything on fiber disposal (HMR safety)', async () => { + const { ctx, fiber } = await setup() + expect(ctx.tools.schemas()).toHaveLength(2) + await fiber.dispose() + expect(ctx.tools.schemas()).toHaveLength(0) + const sections = (await ctx.systemPrompt.assemble()).sections.map(s => s.name) + expect(sections).not.toContain('tool:glob') + expect(sections).not.toContain('tool:grep') + }) + + it('attaches the configured timeoutMs to both tool definitions', async () => { + const { ctx } = await setup({ config: { timeoutMs: 5000 } }) + expect(ctx.tools.get('glob')?.timeoutMs).toBe(5000) + expect(ctx.tools.get('grep')?.timeoutMs).toBe(5000) + }) + + it('defaults the timeout budget to 30 seconds', async () => { + const { ctx } = await setup() + expect(ctx.tools.get('glob')?.timeoutMs).toBe(30_000) + expect(ctx.tools.get('grep')?.timeoutMs).toBe(30_000) + }) +}) + +describe('config validation', () => { + it.each([ + ['globMaxResults', { globMaxResults: 0 }], + ['grepMaxMatches', { grepMaxMatches: -1 }], + ['grepMaxLineBytes', { grepMaxLineBytes: 1.5 }], + ['rawOutputMaxBytes', { rawOutputMaxBytes: 0 }], + ['timeoutMs', { timeoutMs: -100 }], + ] as const)('rejects a non-positive or fractional %s at load', async (name, config) => { + const ctx = new Context() + await ctx.plugin(SystemPrompt) + await ctx.plugin(ToolRegistry) + await ctx.plugin(FakeBash) + await expect(ctx.plugin(ToolFsSearch, config)).rejects.toThrow(new RegExp(`tool-fs-search: ${name} must be a positive integer`)) + }) +}) + +describe('command construction (shell-safe)', () => { + it('glob: fixed rg --files template with quoted pattern and VCS excludes', () => { + const command = buildGlobCommand({ pattern: '**/*.ts' }) + expect(command).toBe( + "rg --files --glob='**/*.ts' --sort=modified --no-ignore --hidden " + + "--glob='!**/.git' --glob='!**/.svn' --glob='!**/.hg' --glob='!**/.bzr' --glob='!**/.jj' --glob='!**/.sl'", + ) + }) + + it('glob: the search root rides behind -- and is quoted', () => { + const command = buildGlobCommand({ pattern: '*.md', path: 'docs dir' }) + expect(command).toContain("-- 'docs dir'") + }) + + it('grep: fixed rg --json template with the pattern in --regexp= form', () => { + expect(buildGrepCommand({ pattern: 'foo.*bar' })).toBe("rg --json --regexp='foo.*bar'") + }) + + it('grep: include and path are quoted, include in --glob= form, path behind --', () => { + const command = buildGrepCommand({ pattern: 'x', path: '-leading-dash', include: '*.{ts,tsx}' }) + expect(command).toBe("rg --json --regexp='x' --glob='*.{ts,tsx}' -- '-leading-dash'") + }) + + it.each([ + ['a command-substitution pattern', '$(rm -rf /)', "'$(rm -rf /)'"], + ['a backtick pattern', '`touch pwned`', "'`touch pwned`'"], + ['a pattern with double quotes and spaces', 'say "hi there"', '\'say "hi there"\''], + ['a pattern with single quotes', "it's", '\'it\'\\\'\'s\''], + ['a pattern with newlines', 'a\nb', "'a\nb'"], + ['a leading-dash pattern', '--flag', "'--flag'"], + ['glob metacharacters', '*?[a-z]{x,y}', "'*?[a-z]{x,y}'"], + ])('quotes %s into one inert shell word', (_label, raw, quoted) => { + expect(buildGrepCommand({ pattern: raw })).toBe(`rg --json --regexp=${quoted}`) + }) +}) + +describe('workdir derivation and signal forwarding', () => { + it('forwards the session cwd as the request workdir', async () => { + const { ctx, bash } = await setup() + bash.handler = () => runResult('a.ts\n') + await call(ctx, 'glob', { pattern: '*' }, { agent: agent('/sessions/s1') }) + expect(bash.requests[0]?.workdir).toBe('/sessions/s1') + expect(bash.specs[0]?.workdir).toBe('/sessions/s1') + }) + + it('omits the request workdir without a session cwd so resolve() defaults apply', async () => { + const { ctx, bash } = await setup() + bash.handler = () => runResult('a.ts\n') + await call(ctx, 'glob', { pattern: '*' }, { agent: agent() }) + expect(bash.requests[0]).not.toHaveProperty('workdir') + expect(bash.specs[0]?.workdir).toBe('/work') + // A non-agent caller takes the same default path. + await call(ctx, 'grep', { pattern: 'x' }) + expect(bash.requests[1]).not.toHaveProperty('workdir') + }) + + it('forwards exec.signal into the bash spec (the abort reaches the backend)', async () => { + const { ctx, bash } = await setup() + const controller = new AbortController() + controller.abort() + bash.handler = spec => runResult('', { aborted: spec.signal?.aborted === true }) + const result = await call(ctx, 'grep', { pattern: 'x' }, { signal: controller.signal }) + expect(bash.specs[0]?.signal).toBe(controller.signal) + expect(result.isError).toBe(true) + expect(result.error).toMatchObject({ name: 'SearchError', code: 'SEARCH_ABORTED' }) + expect(text(result)).toContain('aborted') + }) + + it('reports the bash executor timeout as SEARCH_ABORTED with the budget', async () => { + const { ctx, bash } = await setup() + bash.handler = () => runResult('', { timedOut: true, timeoutMs: 1234, exitCode: null, signal: 'SIGTERM' }) + const result = await call(ctx, 'glob', { pattern: '*' }) + expect(result.isError).toBe(true) + expect(result.error).toMatchObject({ code: 'SEARCH_ABORTED' }) + expect(text(result)).toContain('timed out after 1234ms') + }) +}) + +describe('exit semantics and failure classification', () => { + it('exit 1 is a successful empty search', async () => { + const { ctx, bash } = await setup() + bash.handler = () => runResult('', { exitCode: 1 }) + const glob = await call(ctx, 'glob', { pattern: '*.nope' }) + expect(glob.isError).toBe(false) + expect(text(glob)).toBe('No files found') + const grep = await call(ctx, 'grep', { pattern: 'nope' }) + expect(grep.isError).toBe(false) + expect(text(grep)).toBe('No matches found') + }) + + it('a regex parse error classifies as SEARCH_INVALID_PATTERN', async () => { + const { ctx, bash } = await setup() + bash.handler = () => runResult('', { exitCode: 2, stderr: { text: 'rg: regex parse error:\n (\nerror: unclosed group', truncated: false } }) + const result = await call(ctx, 'grep', { pattern: '(' }) + expect(result.error).toMatchObject({ code: 'SEARCH_INVALID_PATTERN' }) + expect(text(result)).toContain('regex parse error') + }) + + it('a glob parse error classifies as SEARCH_INVALID_PATTERN', async () => { + const { ctx, bash } = await setup() + bash.handler = () => runResult('', { exitCode: 2, stderr: { text: 'rg: error parsing glob \'[\': unclosed character class', truncated: false } }) + const result = await call(ctx, 'glob', { pattern: '[' }) + expect(result.error).toMatchObject({ code: 'SEARCH_INVALID_PATTERN' }) + }) + + it('a missing rg binary classifies as SEARCH_FAILED naming ripgrep', async () => { + const { ctx, bash } = await setup() + bash.handler = () => runResult('', { exitCode: 127, stderr: { text: 'bash: line 1: rg: command not found', truncated: false } }) + const result = await call(ctx, 'glob', { pattern: '*' }) + expect(result.error).toMatchObject({ code: 'SEARCH_FAILED' }) + expect(text(result)).toContain('requires ripgrep (rg)') + // The same classification holds from either evidence alone: the 127 exit + // with silent stderr, or a shell's command-not-found text on another exit. + bash.handler = () => runResult('', { exitCode: 127 }) + expect(text(await call(ctx, 'glob', { pattern: '*' }))).toContain('requires ripgrep (rg)') + bash.handler = () => runResult('', { exitCode: 2, stderr: { text: 'sh: rg: command not found', truncated: false } }) + expect(text(await call(ctx, 'grep', { pattern: 'x' }))).toContain('requires ripgrep (rg)') + }) + + it('other nonzero exits are SEARCH_FAILED carrying the stderr excerpt', async () => { + const { ctx, bash } = await setup() + bash.handler = () => runResult('', { exitCode: 2, stderr: { text: 'rg: missing.dir: IO error: no such file or directory', truncated: false } }) + const result = await call(ctx, 'grep', { pattern: 'x', path: 'missing.dir' }) + expect(result.error).toMatchObject({ code: 'SEARCH_FAILED' }) + expect(text(result)).toContain('IO error') + }) + + it('a nonzero exit with EMPTY stderr still reports the exit code', async () => { + const { ctx, bash } = await setup() + bash.handler = () => runResult('', { exitCode: 3 }) + const result = await call(ctx, 'glob', { pattern: '*' }) + expect(result.error).toMatchObject({ code: 'SEARCH_FAILED' }) + expect(text(result)).toContain('exit 3') + }) + + it('truncated stderr gains a truncation note and stderr.spillPath is never read', async () => { + const { ctx, bash } = await setup() + bash.handler = () => runResult('', { + exitCode: 2, + stderr: { text: 'tail of diagnostics', truncated: true, spillPath: '/does/not/exist-and-never-read' }, + }) + const result = await call(ctx, 'grep', { pattern: 'x' }) + expect(text(result)).toContain('tail of diagnostics [stderr truncated]') + }) + + it('a signal kill (not timeout, not abort) is SEARCH_FAILED', async () => { + const { ctx, bash } = await setup() + bash.handler = () => runResult('', { exitCode: null, signal: 'SIGKILL' }) + const result = await call(ctx, 'grep', { pattern: 'x' }) + expect(result.error).toMatchObject({ code: 'SEARCH_FAILED' }) + expect(text(result)).toContain('SIGKILL') + }) + + it('a null exit with no signal (defensive) is SEARCH_FAILED', async () => { + const { ctx, bash } = await setup() + bash.handler = () => runResult('', { exitCode: null, signal: null }) + const result = await call(ctx, 'glob', { pattern: '*' }) + expect(result.error).toMatchObject({ code: 'SEARCH_FAILED' }) + }) +}) + +describe('raw output acquisition', () => { + let dir: string + afterEach(async () => { + await rm(dir, { recursive: true, force: true }) + }) + + it('parses the complete raw spill file when stdout is truncated', async () => { + dir = await mkdtemp(join(tmpdir(), 'dsh-search-raw-')) + const spillPath = join(dir, 'raw.txt') + await writeFile(spillPath, 'one.ts\ntwo.ts\nthree.ts\n') + const { ctx, bash } = await setup() + bash.handler = () => runResult('', { stdout: { text: 'one.ts\n', truncated: true, spillPath } }) + const result = await call(ctx, 'glob', { pattern: '*.ts' }) + expect(result.isError).toBe(false) + expect(text(result)).toBe('one.ts\ntwo.ts\nthree.ts') + }) + + it('fails with SEARCH_RAW_OUTPUT_OVERFLOW when the raw spill file exceeds the cap', async () => { + dir = await mkdtemp(join(tmpdir(), 'dsh-search-raw-')) + const spillPath = join(dir, 'raw.txt') + await writeFile(spillPath, 'x'.repeat(64)) + const { ctx, bash } = await setup({ config: { rawOutputMaxBytes: 16 } }) + bash.handler = () => runResult('', { stdout: { text: 'x', truncated: true, spillPath } }) + const result = await call(ctx, 'glob', { pattern: '*' }) + expect(result.error).toMatchObject({ code: 'SEARCH_RAW_OUTPUT_OVERFLOW' }) + expect(text(result)).toContain('narrow pattern, path, or include') + }) + + it('fails with SEARCH_RAW_OUTPUT_OVERFLOW when truncated stdout has no spill path', async () => { + dir = await mkdtemp(join(tmpdir(), 'dsh-search-raw-')) + const { ctx, bash } = await setup() + bash.handler = () => runResult('', { stdout: { text: 'partial', truncated: true } }) + const result = await call(ctx, 'grep', { pattern: 'x' }) + expect(result.error).toMatchObject({ code: 'SEARCH_RAW_OUTPUT_OVERFLOW' }) + }) + + it('fails with SEARCH_FAILED when the raw spill file cannot be read', async () => { + dir = await mkdtemp(join(tmpdir(), 'dsh-search-raw-')) + const { ctx, bash } = await setup() + bash.handler = () => runResult('', { stdout: { text: 'partial', truncated: true, spillPath: join(dir, 'gone.txt') } }) + const result = await call(ctx, 'glob', { pattern: '*' }) + expect(result.error).toMatchObject({ code: 'SEARCH_FAILED' }) + expect(text(result)).toContain('raw output spill file') + }) +}) + +describe('glob results', () => { + it('lists workdir-relative paths (absolute output under the workdir is relativized)', async () => { + const { ctx, bash } = await setup() + bash.handler = () => runResult('/sessions/s1/src/a.ts\n/elsewhere/b.ts\nrel/c.ts\n') + const result = await call(ctx, 'glob', { pattern: '*' }, { agent: agent('/sessions/s1') }) + expect(text(result)).toBe('src/a.ts\n/elsewhere/b.ts\nrel/c.ts') + }) + + it('validates arguments (blank pattern, blank path)', async () => { + const { ctx } = await setup() + expect(text(await call(ctx, 'glob', { pattern: ' ' }))).toContain('pattern must be a non-empty string') + expect(text(await call(ctx, 'glob', { pattern: '*', path: ' ' }))).toContain('path must be a non-empty string') + }) + + it('caps at globMaxResults and saves the FULL sorted list through spillFiles', async () => { + const { ctx, bash, spill } = await setup({ config: { globMaxResults: 2 }, spill: true }) + bash.handler = () => runResult('a.ts\nb.ts\nc.ts\nd.ts\n') + const result = await call(ctx, 'glob', { pattern: '*.ts' }, { agent: agent('/w') }) + expect(result.isError).toBe(false) + expect(text(result)).toBe('a.ts\nb.ts\n\n(Showing 2 of 4 paths. Full sorted result saved to: /spill/glob-results.txt. Use read with offset/limit to inspect it.)') + expect(spill?.saves).toHaveLength(1) + expect(spill?.saves[0]).toMatchObject({ + owner: { sessionId: 'session-1' }, + source: { toolName: 'glob', label: 'result' }, + suggestedName: 'glob-results.txt', + content: 'a.ts\nb.ts\nc.ts\nd.ts', + }) + expect(spill?.saves[0]?.source.callId).toBeDefined() + }) + + it('does not create a spill file when the result fits inline', async () => { + const { ctx, bash, spill } = await setup({ spill: true }) + bash.handler = () => runResult('a.ts\nb.ts\n') + const result = await call(ctx, 'glob', { pattern: '*' }, { agent: agent('/w') }) + expect(text(result)).toBe('a.ts\nb.ts') + expect(spill?.saves).toHaveLength(0) + }) + + it.each([ + ['no spill backend loaded', { fail: false, spill: false, ownerless: false }], + ['saveText fails', { fail: true, spill: true, ownerless: false }], + ['no session owner', { fail: false, spill: true, ownerless: true }], + ])('keeps the inline page and reports the unsaved remainder when %s', async (_label, mode) => { + const { ctx, bash, spill } = await setup({ config: { globMaxResults: 1 }, spill: mode.spill }) + if (mode.fail && spill) spill.failWith = new Error('disk full') + bash.handler = () => runResult('a.ts\nb.ts\n') + const result = await call(ctx, 'glob', { pattern: '*' }, mode.ownerless ? {} : { agent: agent('/w') }) + expect(result.isError).toBe(false) // spill unavailability never fails the search + expect(text(result)).toBe('a.ts\n\n(Showing 1 of 2 paths. The complete result could not be saved; narrow pattern or path to see more.)') + }) +}) + +describe('grep results', () => { + it('groups matches by file with line numbers', async () => { + const { ctx, bash } = await setup() + bash.handler = () => runResult([ + JSON.stringify({ type: 'begin', data: { path: { text: 'a.ts' } } }), + matchLine('a.ts', 3, 'const x = 1\n'), + matchLine('a.ts', 9, 'const y = 2\n'), + JSON.stringify({ type: 'end', data: { path: { text: 'a.ts' } } }), + matchLine('b.ts', 1, 'const z = 3'), + JSON.stringify({ type: 'summary', data: {} }), + '', + ].join('\n')) + const result = await call(ctx, 'grep', { pattern: 'const' }) + expect(result.isError).toBe(false) + expect(text(result)).toBe('Found 3 matches\n\na.ts\nLine 3: const x = 1\nLine 9: const y = 2\n\nb.ts\nLine 1: const z = 3') + }) + + it('reports a single match in the singular', async () => { + const { ctx, bash } = await setup() + bash.handler = () => runResult(`${matchLine('a.ts', 1, 'hit')}\n`) + expect(text(await call(ctx, 'grep', { pattern: 'hit' }))).toBe('Found 1 match\n\na.ts\nLine 1: hit') + }) + + it('relativizes absolute match paths against the resolved workdir', async () => { + const { ctx, bash } = await setup() + bash.handler = () => runResult(`${matchLine('/sessions/s1/deep/a.ts', 2, 'hit')}\n`) + const result = await call(ctx, 'grep', { pattern: 'hit', path: '/sessions/s1' }, { agent: agent('/sessions/s1') }) + expect(text(result)).toContain('deep/a.ts\nLine 2: hit') + }) + + it('previews a long matched line at grepMaxLineBytes preserving UTF-8', async () => { + const { ctx, bash } = await setup({ config: { grepMaxLineBytes: 7 } }) + // 'héllo wörld' cut at 7 bytes lands mid-'é'? h(1)é(2)l(1)l(1)o(1)=6, space=7 → clean cut at 7. + // Use a multibyte straddle instead: 'aé' repeated — cut at 7 bytes: a(1)é(2)a(1)é(2)=6 +a(1)=7 → next é straddles: trimmed. + bash.handler = () => runResult(`${matchLine('a.txt', 1, 'aéaéaéaé')}\n`) + const result = await call(ctx, 'grep', { pattern: 'a' }) + expect(text(result)).toContain('Line 1: aéaéa (line truncated)') + }) + + it('renders a non-UTF-8 line (rg bytes form) as a placeholder instead of failing', async () => { + const { ctx, bash } = await setup() + const record = JSON.stringify({ type: 'match', data: { path: { text: 'bin.dat' }, lines: { bytes: 'AAECww==' }, line_number: 4 } }) + bash.handler = () => runResult(`${record}\n`) + expect(text(await call(ctx, 'grep', { pattern: 'x' }))).toContain('Line 4: (line is not valid UTF-8)') + }) + + it('strips a CRLF terminator from the matched line text', () => { + const matches = parseGrepMatches(`${matchLine('a.txt', 1, 'windows line\r\n')}\n`) + expect(matches[0]?.line).toBe('windows line') + }) + + it('caps at grepMaxMatches and spills the full formatted match list', async () => { + const { ctx, bash, spill } = await setup({ config: { grepMaxMatches: 2 }, spill: true }) + bash.handler = () => runResult([ + matchLine('a.ts', 1, 'one'), + matchLine('a.ts', 2, 'two'), + matchLine('b.ts', 3, 'three'), + '', + ].join('\n')) + const result = await call(ctx, 'grep', { pattern: 'e' }, { agent: agent('/w') }) + expect(text(result)).toBe('Found 2 of 3 matches\n\na.ts\nLine 1: one\nLine 2: two\n\n(Full grep result saved to: /spill/grep-results.txt. Use read with offset/limit to inspect it.)') + expect(spill?.saves[0]).toMatchObject({ + source: { toolName: 'grep', label: 'result' }, + suggestedName: 'grep-results.txt', + content: 'Found 3 matches\n\na.ts\nLine 1: one\nLine 2: two\n\nb.ts\nLine 3: three', + }) + }) + + it('reports the unsaved remainder when capped with no spill backend', async () => { + const { ctx, bash } = await setup({ config: { grepMaxMatches: 1 } }) + bash.handler = () => runResult(`${matchLine('a.ts', 1, 'one')}\n${matchLine('a.ts', 2, 'two')}\n`) + const result = await call(ctx, 'grep', { pattern: 'o' }, { agent: agent('/w') }) + expect(result.isError).toBe(false) + expect(text(result)).toBe('Found 1 of 2 matches\n\na.ts\nLine 1: one\n\n(The complete result could not be saved; narrow pattern, path, or include to see more.)') + }) + + it('validates arguments (empty pattern, blank path, bad include)', async () => { + const { ctx } = await setup() + expect(text(await call(ctx, 'grep', { pattern: '' }))).toContain('pattern must be a non-empty string') + expect(text(await call(ctx, 'grep', { pattern: 'x', path: ' ' }))).toContain('path must be a non-empty string') + expect(text(await call(ctx, 'grep', { pattern: 'x', include: ' ' }))).toContain('include must be a non-empty glob') + expect(text(await call(ctx, 'grep', { pattern: 'x', include: '!*.ts' }))).toContain('negated patterns') + expect(text(await call(ctx, 'grep', { pattern: 'x', include: '*.ts,*.js' }))).toContain('comma-separated list') + }) + + it('accepts a whitespace-only pattern (a legitimate regex) and brace alternation in include', async () => { + const { ctx, bash } = await setup() + bash.handler = () => runResult('', { exitCode: 1 }) + const result = await call(ctx, 'grep', { pattern: ' ', include: '*.{ts,tsx}' }) + expect(result.isError).toBe(false) + }) +}) + +describe('rg --json transport failures (SEARCH_FAILED)', () => { + it.each([ + ['a non-JSON line', 'not json at all'], + ['a non-object record', '42'], + ['a match record with no data', JSON.stringify({ type: 'match' })], + ['a match record with no path text', JSON.stringify({ type: 'match', data: { path: {}, lines: { text: 'x' }, line_number: 1 } })], + ['a match record with a non-object path', JSON.stringify({ type: 'match', data: { path: 'a.ts', lines: { text: 'x' }, line_number: 1 } })], + ['a match record with no line number', JSON.stringify({ type: 'match', data: { path: { text: 'a.ts' }, lines: { text: 'x' } } })], + ['a match record with no line content', JSON.stringify({ type: 'match', data: { path: { text: 'a.ts' }, line_number: 1 } })], + ['a match record with neither text nor bytes', JSON.stringify({ type: 'match', data: { path: { text: 'a.ts' }, lines: {}, line_number: 1 } })], + ])('%s fails the search', async (_label, line) => { + const { ctx, bash } = await setup() + bash.handler = () => runResult(`${line}\n`) + const result = await call(ctx, 'grep', { pattern: 'x' }) + expect(result.isError).toBe(true) + expect(result.error).toMatchObject({ name: 'SearchError', code: 'SEARCH_FAILED' }) + }) +}) + +describe('the no-background-task invariant', () => { + it('never calls ctx.bash.start() across successful and failed searches', async () => { + const { ctx, bash } = await setup() + bash.handler = () => runResult('a.ts\n') + await call(ctx, 'glob', { pattern: '*' }) + bash.handler = () => runResult('', { exitCode: 2, stderr: { text: 'boom', truncated: false } }) + await call(ctx, 'grep', { pattern: 'x' }) + expect(bash.startCalls).toBe(0) + }) +}) + +describe('presentation', () => { + it('glob titles carry the pattern and optional root', () => { + expect(presentGlobCall({ pattern: '**/*.ts' })).toMatchObject({ card: 'generic', title: 'Glob **/*.ts', kind: 'search' }) + expect(presentGlobCall({ pattern: '*.md', path: 'docs' }).title).toBe('Glob *.md in docs') + }) + + it('grep titles carry the pattern, target, and include filter', () => { + expect(presentGrepCall({ pattern: 'todo' })).toMatchObject({ card: 'generic', title: 'Grep todo', kind: 'search' }) + expect(presentGrepCall({ pattern: 'todo', path: 'src', include: '*.ts' }).title).toBe('Grep todo in src (*.ts)') + }) +}) + +describe('helpers', () => { + it('toWorkdirRelative maps inside-workdir absolutes and passes everything else through', () => { + expect(toWorkdirRelative('/w/a/b.ts', '/w')).toBe('a/b.ts') + expect(toWorkdirRelative('/w', '/w')).toBe('.') + expect(toWorkdirRelative('/other/b.ts', '/w')).toBe('/other/b.ts') + expect(toWorkdirRelative('/w-sibling/b.ts', '/w')).toBe('/w-sibling/b.ts') + expect(toWorkdirRelative('rel/b.ts', '/w')).toBe('rel/b.ts') + // Normalization makes this land OUTSIDE the workdir → original path kept. + expect(toWorkdirRelative('/w/../up.ts', '/w')).toBe('/w/../up.ts') + }) + + it('previewLine keeps a within-budget line untouched', () => { + expect(previewLine('short', 100)).toBe('short') + }) + + it('formatGrepMatches groups by first-seen file order', () => { + const grouped = formatGrepMatches([ + { path: 'b.ts', lineNumber: 2, line: 'x' }, + { path: 'a.ts', lineNumber: 1, line: 'y' }, + { path: 'b.ts', lineNumber: 5, line: 'z' }, + ]) + expect(grouped).toBe('b.ts\nLine 2: x\nLine 5: z\n\na.ts\nLine 1: y') + }) +}) diff --git a/packages/fs/tool-fs-search/tsconfig.json b/packages/fs/tool-fs-search/tsconfig.json new file mode 100644 index 0000000000..9241aca15b --- /dev/null +++ b/packages/fs/tool-fs-search/tsconfig.json @@ -0,0 +1,20 @@ +{ + "extends": "../../../tsconfig.base.json", + "compilerOptions": { + "rootDir": "src", + "outDir": "lib/types" + }, + "include": ["src"], + "references": [ + { "path": "../../../vendor/cosmokit" }, + { "path": "../../../vendor/cordis" }, + { "path": "../../../vendor/schemastery" }, + { "path": "../../util/retention" }, + { "path": "../../llm/llm" }, + { "path": "../../core/session" }, + { "path": "../../core/tools" }, + { "path": "../../core/system-prompt" }, + { "path": "../../bash/bash" }, + { "path": "../../spill/spill" } + ] +} diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 18129c26cc..9d0c641453 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -403,6 +403,43 @@ importers: specifier: ^4.0.0-rc.6 version: 4.0.0-rc.6(@cordisjs/plugin-include@1.0.4)(@cordisjs/plugin-loader@1.0.0-rc.4) + packages/fs/tool-fs-search: + dependencies: + schemastery: + specifier: ^3.18.0 + version: 3.18.0 + devDependencies: + '@deepseek-ai/dsh-agent': + specifier: workspace:^ + version: link:../../core/agent + '@deepseek-ai/dsh-bash': + specifier: workspace:^ + version: link:../../bash/bash + '@deepseek-ai/dsh-bash-local': + specifier: workspace:^ + version: link:../../bash/bash-local + '@deepseek-ai/dsh-llm': + specifier: workspace:^ + version: link:../../llm/llm + '@deepseek-ai/dsh-retention': + specifier: workspace:^ + version: link:../../util/retention + '@deepseek-ai/dsh-session': + specifier: workspace:^ + version: link:../../core/session + '@deepseek-ai/dsh-spill': + specifier: workspace:^ + version: link:../../spill/spill + '@deepseek-ai/dsh-system-prompt': + specifier: workspace:^ + version: link:../../core/system-prompt + '@deepseek-ai/dsh-tools': + specifier: workspace:^ + version: link:../../core/tools + cordis: + specifier: ^4.0.0-rc.6 + version: 4.0.0-rc.6(@cordisjs/plugin-include@1.0.4)(@cordisjs/plugin-loader@1.0.0-rc.4) + packages/guard/repeat-tool-guard: dependencies: schemastery: diff --git a/scripts/gen-tool-catalog.ts b/scripts/gen-tool-catalog.ts index 24739e1388..e808dfdd1b 100644 --- a/scripts/gen-tool-catalog.ts +++ b/scripts/gen-tool-catalog.ts @@ -48,6 +48,7 @@ import SubagentService from '@deepseek-ai/dsh-subagent' import * as SubagentMock from '@deepseek-ai/dsh-subagent-mock' import * as ToolBash from '@deepseek-ai/dsh-tool-bash' import * as ToolFs from '@deepseek-ai/dsh-tool-fs' +import * as ToolFsSearch from '@deepseek-ai/dsh-tool-fs-search' import * as ToolTodo from '@deepseek-ai/dsh-tool-todo' import * as ToolSubagent from '@deepseek-ai/dsh-tool-subagent' import * as ToolWeb from '@deepseek-ai/dsh-tool-web' @@ -129,6 +130,23 @@ const TOOL_PACKAGES: ToolPackage[] = [ note: 'The read-before-write/edit policy is added by `@deepseek-ai/dsh-fs-policy` (an `fs/*` event-gate plugin, no schema change); a deployment that loads these tools is expected to also load it. The tool schemas above are identical with or without the policy plugin.', }, + { + pkg: '@deepseek-ai/dsh-tool-fs-search', + dir: 'tool-fs-search', + source: 'packages/fs/tool-fs-search/src/index.ts', + requires: ['ctx.tools', 'ctx.bash', 'ctx.systemPrompt'], + writes: ['tool/call', 'tool/result'], + async mount(ctx) { + // The tools inject `bash` (search executes fixed `rg` commands through + // the executor seam, not ctx.fs); boot the local executor to satisfy it. + // `ctx.spillFiles` is optional (read via ctx.get) and does not affect the + // schemas, so no spill backend is mounted. + await ctx.plugin(LocalBashExecutor) + await ctx.plugin(ToolFsSearch) + }, + note: + 'glob and grep are bash-backed discovery tools: they run fixed ripgrep commands through ctx.bash as ordinary foreground calls (never background tasks). Capped results save the complete formatted list through the optional ctx.spillFiles backend; returned paths are follow-up-readable in co-located bash/filesystem deployments.', + }, { pkg: '@deepseek-ai/dsh-tool-subagent', dir: 'tool-subagent', diff --git a/tsconfig.build.json b/tsconfig.build.json index 625efd3a44..4bb8da6759 100644 --- a/tsconfig.build.json +++ b/tsconfig.build.json @@ -36,6 +36,7 @@ { "path": "./packages/fs/fs-local" }, { "path": "./packages/fs/fs-policy" }, { "path": "./packages/fs/tool-fs" }, + { "path": "./packages/fs/tool-fs-search" }, { "path": "./packages/web/web" }, { "path": "./packages/web/web-search-exa" }, { "path": "./packages/web/web-search-perplexity" }, diff --git a/tsconfig.json b/tsconfig.json index 91fc52ebf3..e65121cf19 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -45,6 +45,7 @@ { "path": "./packages/fs/fs-local" }, { "path": "./packages/fs/fs-policy" }, { "path": "./packages/fs/tool-fs" }, + { "path": "./packages/fs/tool-fs-search" }, { "path": "./packages/compact/compact" }, { "path": "./packages/compact/compact-basic" }, { "path": "./packages/web/web" }, From e94305d99ebcee7c93208b3e347b4ee791ada0b2 Mon Sep 17 00:00:00 2001 From: Dudu-0223 Date: Thu, 9 Jul 2026 21:12:41 +0800 Subject: [PATCH 2/7] fix: address codex review round 1 Two functional gaps in the search tools change: - Enforce rawOutputMaxBytes on UNTRUNCATED inline stdout too. The cap was only checked on the truncated->raw-spill path, so an executor retaining more inline than the search cap (or a deployment lowering the cap below the bash retention) could smuggle an over-cap parse through, contradicting the documented SEARCH_RAW_OUTPUT_OVERFLOW contract. Covered by a new over-cap-inline test. - Load @deepseek-ai/dsh-timeout-policy in the coding-agent tree. The search tools declare timeoutMs but nothing in the demo enforced it, so the advertised 30s budget silently degraded to the bash executor's 60s backstop. The keyless smoke boots the amended tree. --- examples/coding-agent/composition.md | 3 +++ examples/coding-agent/cordis.yml | 7 ++++++ packages/fs/tool-fs-search/src/search-core.ts | 23 ++++++++++++++----- .../fs/tool-fs-search/tests/tools.spec.ts | 12 ++++++++++ 4 files changed, 39 insertions(+), 6 deletions(-) diff --git a/examples/coding-agent/composition.md b/examples/coding-agent/composition.md index c6c624f0fb..9896c44420 100644 --- a/examples/coding-agent/composition.md +++ b/examples/coding-agent/composition.md @@ -45,6 +45,8 @@ flowchart LR cfg --> plugin_coding_tool_fs plugin_coding_tool_fs_search["tool-fs-search
@deepseek-ai/dsh-tool-fs-search"] cfg --> plugin_coding_tool_fs_search + plugin_coding_timeout_policy["timeout-policy
@deepseek-ai/dsh-timeout-policy"] + cfg --> plugin_coding_timeout_policy plugin_coding_spill_local["spill-local
@deepseek-ai/dsh-spill-local"] cfg --> plugin_coding_spill_local plugin_coding_spill_policy["spill-policy
@deepseek-ai/dsh-spill-policy"] @@ -68,6 +70,7 @@ flowchart LR | `fs-policy` | `@deepseek-ai/dsh-fs-policy` | | `tool-fs` | `@deepseek-ai/dsh-tool-fs` | | `tool-fs-search` | `@deepseek-ai/dsh-tool-fs-search` | +| `timeout-policy` | `@deepseek-ai/dsh-timeout-policy` | | `spill-local` | `@deepseek-ai/dsh-spill-local` | | `spill-policy` | `@deepseek-ai/dsh-spill-policy` | diff --git a/examples/coding-agent/cordis.yml b/examples/coding-agent/cordis.yml index 5861e6a602..c175bbaf97 100644 --- a/examples/coding-agent/cordis.yml +++ b/examples/coding-agent/cordis.yml @@ -127,6 +127,13 @@ - id: tool-fs-search name: '@deepseek-ai/dsh-tool-fs-search' +# The tool-call timeout enforcer: arms each declared ToolDefinition.timeoutMs +# (the search tools above declare 30s) as a deadline on exec.signal. Without +# it a declared budget is advisory and only the bash executor's own timeout +# backstop applies. +- id: timeout-policy + name: '@deepseek-ai/dsh-timeout-policy' + # Tool-output spill stack: a local backend that saves oversized tool text under # a private session-scoped dir, and the tools/post-execute policy that replaces # an over-budget plain-text result with a preview + the spill path (the model diff --git a/packages/fs/tool-fs-search/src/search-core.ts b/packages/fs/tool-fs-search/src/search-core.ts index 2d9d0b63dd..b73c31bd1f 100644 --- a/packages/fs/tool-fs-search/src/search-core.ts +++ b/packages/fs/tool-fs-search/src/search-core.ts @@ -103,15 +103,26 @@ function classifyRunFailure(toolName: string, result: BashRunResult): SearchErro } /** - * Acquire the COMPLETE raw stdout of a finished run. Untruncated stdout is used - * as-is; truncated stdout is recovered from the executor's local raw spill file - * only when the complete file fits within `rawOutputMaxBytes`. A missing spill - * path or an over-cap file is a clear failure telling the model to narrow the - * search — never a silently-partial parse. + * Acquire the COMPLETE raw stdout of a finished run, enforcing + * `rawOutputMaxBytes` on BOTH transports: inline executor text (an executor + * retaining more than this package's cap must not smuggle an over-cap parse + * through the untruncated path) and the executor's local raw spill file, read + * only when the complete file fits the cap. A missing spill path or over-cap + * output is a clear failure telling the model to narrow the search — never a + * silently-partial parse. */ async function completeStdout(toolName: string, result: BashRunResult, rawOutputMaxBytes: number): Promise { - if (!result.stdout.truncated) return result.stdout.text const narrow = 'narrow pattern, path, or include and retry' + if (!result.stdout.truncated) { + const inlineBytes = Buffer.byteLength(result.stdout.text, 'utf8') + if (inlineBytes > rawOutputMaxBytes) { + throw new SearchError( + `${toolName} produced ${inlineBytes} bytes of raw output, over the ${rawOutputMaxBytes}-byte cap; ${narrow}`, + 'SEARCH_RAW_OUTPUT_OVERFLOW', + ) + } + return result.stdout.text + } const spillPath = result.stdout.spillPath if (spillPath === undefined) { throw new SearchError( diff --git a/packages/fs/tool-fs-search/tests/tools.spec.ts b/packages/fs/tool-fs-search/tests/tools.spec.ts index 6a539956a4..55b54555ae 100644 --- a/packages/fs/tool-fs-search/tests/tools.spec.ts +++ b/packages/fs/tool-fs-search/tests/tools.spec.ts @@ -392,6 +392,18 @@ describe('raw output acquisition', () => { expect(text(result)).toContain('narrow pattern, path, or include') }) + it('fails with SEARCH_RAW_OUTPUT_OVERFLOW when UNTRUNCATED inline stdout exceeds the cap', async () => { + // An executor retaining more inline than this package's cap (or a + // deployment lowering rawOutputMaxBytes below the bash retention) must not + // smuggle an over-cap parse through the untruncated path. + dir = await mkdtemp(join(tmpdir(), 'dsh-search-raw-')) + const { ctx, bash } = await setup({ config: { rawOutputMaxBytes: 16 } }) + bash.handler = () => runResult(`${'x'.repeat(64)}\n`) + const result = await call(ctx, 'grep', { pattern: 'x' }) + expect(result.error).toMatchObject({ name: 'SearchError', code: 'SEARCH_RAW_OUTPUT_OVERFLOW' }) + expect(text(result)).toContain('narrow pattern, path, or include') + }) + it('fails with SEARCH_RAW_OUTPUT_OVERFLOW when truncated stdout has no spill path', async () => { dir = await mkdtemp(join(tmpdir(), 'dsh-search-raw-')) const { ctx, bash } = await setup() From 590f520949dcd57bb52aebd713a5fa593bf209de Mon Sep 17 00:00:00 2001 From: Dudu-0223 Date: Thu, 9 Jul 2026 21:28:42 +0800 Subject: [PATCH 3/7] fix: address codex review round 2 Translate ctx.bash.run() REJECTIONS into the SEARCH_* taxonomy. The seam contract has run() reject for infrastructure failures (a pre-aborted signal, an unusable/deleted session workdir, a missing shell); the bare await let those escape as plain Errors, so the tool registry produced isError results without the structured SearchError { name, code } the package documents. A pre-aborted spec.signal now maps to SEARCH_ABORTED and any other start failure to SEARCH_FAILED, original error chained as cause. Covered by fake-executor tests for both branches plus real-executor integration tests pinning the exact pre-aborted-signal and deleted-cwd paths. --- packages/fs/tool-fs-search/src/search-core.ts | 19 +++++++++++++-- .../tool-fs-search/tests/integration.spec.ts | 23 +++++++++++++++++++ .../fs/tool-fs-search/tests/tools.spec.ts | 21 +++++++++++++++++ 3 files changed, 61 insertions(+), 2 deletions(-) diff --git a/packages/fs/tool-fs-search/src/search-core.ts b/packages/fs/tool-fs-search/src/search-core.ts index b73c31bd1f..a47adfa7c3 100644 --- a/packages/fs/tool-fs-search/src/search-core.ts +++ b/packages/fs/tool-fs-search/src/search-core.ts @@ -158,7 +158,11 @@ async function completeStdout(toolName: string, result: BashRunResult, rawOutput * success with zero results (`noMatches`), anything else throws a * {@link SearchError} (abort/timeout → `SEARCH_ABORTED`, invalid pattern → * `SEARCH_INVALID_PATTERN`, the rest → `SEARCH_FAILED` / - * `SEARCH_RAW_OUTPUT_OVERFLOW`). + * `SEARCH_RAW_OUTPUT_OVERFLOW`). A `run()` REJECTION — the seam's + * infrastructure failures (pre-aborted signal, unusable workdir, missing + * shell) — is translated into the same taxonomy: a pre-aborted signal becomes + * `SEARCH_ABORTED`, everything else `SEARCH_FAILED`, with the original as + * `cause`. * * @param ctx - the plugin context; execution uses its `bash` service. * @param exec - the tool-execution context; supplies the session cwd and the abort signal. @@ -180,7 +184,18 @@ export async function runRipgrep( ...cwd !== undefined ? { workdir: cwd } : {}, ...exec.signal ? { signal: exec.signal } : {}, }) - const result = await ctx.bash.run(spec) + let result: BashRunResult + try { + result = await ctx.bash.run(spec) + } catch (error: unknown) { + // The seam contract: run() REJECTS only for infrastructure failures — a + // pre-aborted signal, an unusable workdir, a missing shell. Translate them + // so these failures stay machine-routable under the SEARCH_* taxonomy. + if (spec.signal?.aborted === true) { + throw new SearchError(`${toolName} was aborted before completion (tool timeout or caller cancellation)`, 'SEARCH_ABORTED', { cause: error }) + } + throw new SearchError(`${toolName} could not start its search command (unusable working directory or missing shell)`, 'SEARCH_FAILED', { cause: error }) + } if (result.aborted) { throw new SearchError(`${toolName} was aborted before completion (tool timeout or caller cancellation)`, 'SEARCH_ABORTED') } diff --git a/packages/fs/tool-fs-search/tests/integration.spec.ts b/packages/fs/tool-fs-search/tests/integration.spec.ts index 74a418238b..481d3af620 100644 --- a/packages/fs/tool-fs-search/tests/integration.spec.ts +++ b/packages/fs/tool-fs-search/tests/integration.spec.ts @@ -158,4 +158,27 @@ describe.skipIf(!hasRg)('search tools over the real bash executor + real rg', () } }) }) + + describe('bash-start infrastructure failures stay in the SEARCH_* taxonomy', () => { + it('a pre-aborted exec.signal (real executor rejects before spawn) is SEARCH_ABORTED', async () => { + const controller = new AbortController() + controller.abort() + const result = await ctx.tools.execute({ + callId: CallId(`it-${++callCounter}`), + name: 'grep', + arguments: { pattern: 'x' }, + signal: controller.signal, + }) + expect(result.isError).toBe(true) + expect(result.error).toMatchObject({ name: 'SearchError', code: 'SEARCH_ABORTED' }) + }) + + it('an unusable session cwd (spawn failure) is SEARCH_FAILED', async () => { + const gone = join(dir, 'deleted-session-dir') + const result = await call('glob', { pattern: '*' }, { session: { header: { id: 'session-int', cwd: gone } } }) + expect(result.isError).toBe(true) + expect(result.error).toMatchObject({ name: 'SearchError', code: 'SEARCH_FAILED' }) + expect(text(result)).toContain('could not start') + }) + }) }) diff --git a/packages/fs/tool-fs-search/tests/tools.spec.ts b/packages/fs/tool-fs-search/tests/tools.spec.ts index 55b54555ae..6afca54efd 100644 --- a/packages/fs/tool-fs-search/tests/tools.spec.ts +++ b/packages/fs/tool-fs-search/tests/tools.spec.ts @@ -279,6 +279,27 @@ describe('workdir derivation and signal forwarding', () => { expect(result.error).toMatchObject({ code: 'SEARCH_ABORTED' }) expect(text(result)).toContain('timed out after 1234ms') }) + + it('translates a run() rejection under a pre-aborted signal into SEARCH_ABORTED', async () => { + // The seam contract: run() REJECTS for a pre-aborted signal (it never + // spawns). The plain rejection must not escape the SEARCH_* taxonomy. + const { ctx, bash } = await setup() + const controller = new AbortController() + controller.abort() + bash.handler = () => { throw new Error('aborted before spawn') } + const result = await call(ctx, 'grep', { pattern: 'x' }, { signal: controller.signal }) + expect(result.isError).toBe(true) + expect(result.error).toMatchObject({ name: 'SearchError', code: 'SEARCH_ABORTED' }) + }) + + it('translates a run() rejection without an abort (unusable workdir) into SEARCH_FAILED', async () => { + const { ctx, bash } = await setup() + bash.handler = () => { throw new Error('spawn bash ENOENT') } + const result = await call(ctx, 'glob', { pattern: '*' }) + expect(result.isError).toBe(true) + expect(result.error).toMatchObject({ name: 'SearchError', code: 'SEARCH_FAILED' }) + expect(text(result)).toContain('could not start') + }) }) describe('exit semantics and failure classification', () => { From 460a58639ae5aba84f97c24f8ba957d25b64db94 Mon Sep 17 00:00:00 2001 From: Dudu-0223 Date: Thu, 9 Jul 2026 21:47:39 +0800 Subject: [PATCH 4/7] fix: address codex review round 3 glob leaked VCS internals when the model rooted the search AT a VCS directory (path: '.git' or 'sub/.git'): the prune glob !**/.git is matched against root-prefixed candidate paths, which never end in the directory name when the walk starts inside it. Pair each VCS exclude with a contents glob (!**//**), verified empirically to exclude relative, nested, and absolute VCS roots while leaving broad searches untouched. Pinned by the command-construction test and a real-rg integration case rooting at .git. --- packages/fs/tool-fs-search/src/glob.ts | 18 ++++++++++++++---- .../tool-fs-search/tests/integration.spec.ts | 6 ++++++ packages/fs/tool-fs-search/tests/tools.spec.ts | 6 ++++-- 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/packages/fs/tool-fs-search/src/glob.ts b/packages/fs/tool-fs-search/src/glob.ts index e565699e2c..09a3e1d9ce 100644 --- a/packages/fs/tool-fs-search/src/glob.ts +++ b/packages/fs/tool-fs-search/src/glob.ts @@ -29,9 +29,12 @@ export const GLOB_MAX_RESULTS = 100 /** * Directory names ripgrep must never descend into for a discovery listing: VCS * metadata stores. `--no-ignore --hidden` would otherwise surface them in every - * broad search. Each is excluded with a negated any-depth `--glob` (see - * {@link buildGlobCommand}), which matches — and prunes — the directory - * wherever it appears. + * broad search. Each name is excluded with TWO negated `--glob`s (see + * {@link buildGlobCommand}): an any-depth directory glob that matches — and + * prunes — the directory during traversal, and a contents glob that still + * excludes the internals when the search root itself is at or inside the + * directory (an explicit `path` of `.git` or `sub/.git`), where the prune glob + * alone never matches. */ export const GLOB_VCS_EXCLUDES: readonly string[] = ['.git', '.svn', '.hg', '.bzr', '.jj', '.sl'] @@ -81,7 +84,14 @@ export function buildGlobCommand(input: GlobInput): string { 'rg --files', `--glob=${singleQuote(input.pattern)}`, '--sort=modified --no-ignore --hidden', - ...GLOB_VCS_EXCLUDES.map(name => `--glob=${singleQuote(`!**/${name}`)}`), + // Two negated globs per VCS name: the bare form prunes the directory + // during traversal; the /** form still excludes the contents when the + // search root is AT or INSIDE the directory (where the bare form, + // matched against root-prefixed paths, never fires). + ...GLOB_VCS_EXCLUDES.flatMap(name => [ + `--glob=${singleQuote(`!**/${name}`)}`, + `--glob=${singleQuote(`!**/${name}/**`)}`, + ]), ] if (input.path !== undefined) parts.push('--', singleQuote(input.path)) return parts.join(' ') diff --git a/packages/fs/tool-fs-search/tests/integration.spec.ts b/packages/fs/tool-fs-search/tests/integration.spec.ts index 481d3af620..36fb2c28e6 100644 --- a/packages/fs/tool-fs-search/tests/integration.spec.ts +++ b/packages/fs/tool-fs-search/tests/integration.spec.ts @@ -87,6 +87,12 @@ describe.skipIf(!hasRg)('search tools over the real bash executor + real rg', () expect(text(await call('glob', { pattern: '*.nomatch' }))).toBe('No files found') }) + it('excludes VCS internals even when the search root IS the VCS directory', async () => { + // The prune glob alone never matches root-prefixed paths when rg is + // rooted at .git; the paired contents glob keeps the exclusion airtight. + expect(text(await call('glob', { pattern: '*', path: '.git' }))).toBe('No files found') + }) + it('classifies an invalid glob as SEARCH_INVALID_PATTERN', async () => { const result = await call('glob', { pattern: '[' }) expect(result.isError).toBe(true) diff --git a/packages/fs/tool-fs-search/tests/tools.spec.ts b/packages/fs/tool-fs-search/tests/tools.spec.ts index 6afca54efd..a3a3e89cce 100644 --- a/packages/fs/tool-fs-search/tests/tools.spec.ts +++ b/packages/fs/tool-fs-search/tests/tools.spec.ts @@ -204,11 +204,13 @@ describe('config validation', () => { }) describe('command construction (shell-safe)', () => { - it('glob: fixed rg --files template with quoted pattern and VCS excludes', () => { + it('glob: fixed rg --files template with quoted pattern and paired VCS excludes', () => { const command = buildGlobCommand({ pattern: '**/*.ts' }) expect(command).toBe( "rg --files --glob='**/*.ts' --sort=modified --no-ignore --hidden " - + "--glob='!**/.git' --glob='!**/.svn' --glob='!**/.hg' --glob='!**/.bzr' --glob='!**/.jj' --glob='!**/.sl'", + + "--glob='!**/.git' --glob='!**/.git/**' --glob='!**/.svn' --glob='!**/.svn/**' " + + "--glob='!**/.hg' --glob='!**/.hg/**' --glob='!**/.bzr' --glob='!**/.bzr/**' " + + "--glob='!**/.jj' --glob='!**/.jj/**' --glob='!**/.sl' --glob='!**/.sl/**'", ) }) From 1df9f3a84ade29f3393afbe4737f014b5ee4dde1 Mon Sep 17 00:00:00 2001 From: Dudu-0223 Date: Thu, 9 Jul 2026 22:00:43 +0800 Subject: [PATCH 5/7] test: cover the glob path arg in the fake-executor tier CI has no rg, so the integration suite self-skips there and the fake-executor suite must carry the per-file 100% coverage gate alone. parseGlobArgs's valid-path branch was only exercised by integration (node 24 / coverage failed at 95.45% branches on glob.ts); a fake-tier test now threads a valid path through to the quoted `-- 'sub'` root. --- packages/fs/tool-fs-search/tests/tools.spec.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/fs/tool-fs-search/tests/tools.spec.ts b/packages/fs/tool-fs-search/tests/tools.spec.ts index a3a3e89cce..7abbe20638 100644 --- a/packages/fs/tool-fs-search/tests/tools.spec.ts +++ b/packages/fs/tool-fs-search/tests/tools.spec.ts @@ -459,6 +459,14 @@ describe('glob results', () => { expect(text(await call(ctx, 'glob', { pattern: '*', path: ' ' }))).toContain('path must be a non-empty string') }) + it('threads a valid path through to the command as the quoted search root', async () => { + const { ctx, bash } = await setup() + bash.handler = () => runResult('sub/a.ts\n') + const result = await call(ctx, 'glob', { pattern: '*.ts', path: 'sub' }) + expect(result.isError).toBe(false) + expect(bash.specs[0]?.command).toContain("-- 'sub'") + }) + it('caps at globMaxResults and saves the FULL sorted list through spillFiles', async () => { const { ctx, bash, spill } = await setup({ config: { globMaxResults: 2 }, spill: true }) bash.handler = () => runResult('a.ts\nb.ts\nc.ts\nd.ts\n') From 3bb90bd4b6d3aa2bdae23509136f585e56912bb8 Mon Sep 17 00:00:00 2001 From: Dudu-0223 Date: Fri, 10 Jul 2026 11:53:02 +0800 Subject: [PATCH 6/7] fix: make search raw output recovery backend-neutral --- docs/core-data-structures/bash.md | 18 ++++++- ...6-07-09-bash-backed-grep-glob-discovery.md | 16 +++--- packages/bash/bash-local/README.md | 2 +- packages/bash/bash-local/src/index.ts | 9 +++- packages/bash/bash-local/src/run.ts | 10 ++-- .../bash/bash-local/tests/executor.spec.ts | 17 ++++++ packages/bash/bash-local/tests/run.spec.ts | 27 +++++++--- packages/bash/bash/README.md | 2 +- packages/bash/bash/src/types.ts | 12 +++++ packages/bash/bash/tests/service.spec.ts | 1 + packages/bash/tool-bash/README.md | 2 +- packages/bash/tool-bash/tests/tools.spec.ts | 23 +++++--- packages/fs/tool-fs-search/README.md | 4 +- packages/fs/tool-fs-search/src/search-core.ts | 52 ++++++------------- .../fs/tool-fs-search/tests/tools.spec.ts | 47 +++++------------ .../hooks/hook-protocol/tests/runner.spec.ts | 1 + 16 files changed, 136 insertions(+), 107 deletions(-) diff --git a/docs/core-data-structures/bash.md b/docs/core-data-structures/bash.md index 273ba5ebe8..81ddcd06b7 100644 --- a/docs/core-data-structures/bash.md +++ b/docs/core-data-structures/bash.md @@ -6,7 +6,7 @@ Source: [`packages/bash/bash/src/types.ts`](../../packages/bash/bash/src/types.t ## Request vs. spec: the `resolve()` split -The seam separates the **model-/plugin-facing request** (optional `workdir`/`timeoutMs`, filled from config) from the **fully-resolved spec** the executor acts on (those fields required). The tool layer calls `ctx.bash.resolve(request)` between them — this is the repo's "explicit > implicit at package seams" rule made concrete: the reader of a `BashExecSpec` never wonders where the working directory came from. +The seam separates the **model-/plugin-facing request** (optional `workdir`/`timeoutMs`/`stdoutMaxBytes`, filled from config or request policy) from the **fully-resolved spec** the executor acts on (those fields required). The tool layer calls `ctx.bash.resolve(request)` between them — this is the repo's "explicit > implicit at package seams" rule made concrete: the reader of a `BashExecSpec` never wonders where the working directory or output budget came from. ```ts type-equiv interface BashExecRequest { @@ -15,6 +15,13 @@ interface BashExecRequest { workdir?: string | undefined /** Timeout override in milliseconds (implementations cap it). */ timeoutMs?: number | undefined + /** + * Foreground stdout capture budget in bytes. Absent uses the executor's + * default output cap. Trusted in-process consumers use this when they must + * parse complete stdout up to their own bounded limit; the model-facing bash + * tool does not expose it as a parameter. + */ + stdoutMaxBytes?: number | undefined /** Abort signal — implementations kill the command when it fires. */ signal?: AbortSignal | undefined /** @@ -52,6 +59,11 @@ interface BashExecSpec { command: string workdir: string timeoutMs: number + /** + * Resolved foreground stdout capture budget in bytes. `run()` uses it for + * stdout; background tasks and stderr keep the executor's own output cap. + */ + stdoutMaxBytes: number /** Abort signal — implementations kill the command when it fires. */ signal?: AbortSignal | undefined /** @@ -84,7 +96,9 @@ interface BashExecSpec { The `owner` token is the isolation key: the executor stores it but never interprets it (access policy is the consumer's job), so a background task started by one agent isn't readable cross-session. A required-but-nullable field makes a forgotten owner a visible `undefined` rather than a silently-unowned task. -`stdin` and `env` are set by in-process plugins (the hooks bridges, native plugins) to feed a hook command its JSON payload on stdin and its `CLAUDE_PROJECT_DIR`/`CLAUDE_PLUGIN_ROOT` env. The model-facing `dsh-tool-bash` tool does not expose them as parameters — its request is built from `command`/`workdir`/`timeoutMs`/`signal`/`owner` only — because a model already has equivalent power through shell syntax (`FOO=bar cmd`, a heredoc), so duplicating them as tool params would be redundant. This is NOT a security boundary: the credential scrub in `dsh-bash-local` is what stops the harness's ambient secrets reaching a spawned command, and it works regardless of these fields (a model cannot read a value the scrub removed, and tool-call args are static JSON, never shell-evaluated). A guard test asserts the tool doesn't forward model `env`/`stdin` — to catch a future `...args` spread, not to defend a trust wall. `env` is merged AFTER the scrub so an explicit caller entry (a value it already holds) wins even on a credential-shaped name. See [the bash-stdin-env RFC](../rfc/implemented/architecture/2026-06-30-bash-stdin-env-trusted-plugin-surface.md). +`stdin` and `env` are set by in-process plugins (the hooks bridges, native plugins) to feed a hook command its JSON payload on stdin and its `CLAUDE_PROJECT_DIR`/`CLAUDE_PLUGIN_ROOT` env. The model-facing `dsh-tool-bash` tool does not expose them as parameters — its request is built from `command`/`workdir`/`timeoutMs`/`signal`/`owner` only — because a model already has equivalent power through shell syntax (`FOO=bar cmd`, a heredoc), so duplicating them as tool params would be redundant. This is NOT a security boundary: the credential scrub in `dsh-bash-local` is what stops the harness's ambient secrets reaching a spawned command, and it works regardless of these fields (a model cannot read a value the scrub removed, and tool-call args are static JSON, never shell-evaluated). A guard test asserts the tool doesn't forward model `env`/`stdin`/`stdoutMaxBytes` — to catch a future `...args` spread, not to defend a trust wall. `env` is merged AFTER the scrub so an explicit caller entry (a value it already holds) wins even on a credential-shaped name. See [the bash-stdin-env RFC](../rfc/implemented/architecture/2026-06-30-bash-stdin-env-trusted-plugin-surface.md). + +`stdoutMaxBytes` is also trusted-plugin-only. It lets a foreground consumer ask the executor to retain complete stdout up to a bounded parser budget without changing stderr, background tasks, or the model-facing bash tool's ordinary `maxOutputBytes` behavior. Both ids the seam handles are [branded](core.md) (zero-cost `string` brands, the same machinery as `SessionId`/`AgentId`): `BashTaskId` (a tracked background task, generated `bash-N` by the local executor) and `OwnerToken` (the opaque isolation key). `OwnerToken` is deliberately a DISTINCT brand from `SessionId`, not an alias: the bash seam is a capability seam that must not know what an owner token *means*, so it never imports `dsh-session`'s vocabulary — the `dsh-tool-bash` consumer is the single boundary that casts the owning agent's `SessionId` into an `OwnerToken`. Branding both stops a raw `string` (or a `BashTaskId` where an `OwnerToken` is expected, or vice versa) from slipping through the type checker on the model-facing `task_id` path. diff --git a/docs/rfc/implemented/feature/2026-07-09-bash-backed-grep-glob-discovery.md b/docs/rfc/implemented/feature/2026-07-09-bash-backed-grep-glob-discovery.md index 185444872f..74648ab40d 100644 --- a/docs/rfc/implemented/feature/2026-07-09-bash-backed-grep-glob-discovery.md +++ b/docs/rfc/implemented/feature/2026-07-09-bash-backed-grep-glob-discovery.md @@ -77,7 +77,7 @@ The `path` field follows the same split as Claude Code: `grep.path` is a file-or `grep` builds a fixed line-oriented `rg --json` command against the supplied file/directory target (`path` when supplied, else the bash workdir) so file path, line number, and line text are parsed without colon-splitting ambiguity. It consumes `match` records, treats malformed JSON or malformed match records as `SEARCH_FAILED`, maps result paths relative to the bash workdir when possible, applies per-line preview retention with `grepMaxLineBytes`, pushes each match into `ItemRetainer({ kind: 'head', maxItems: grepMaxMatches })`, then groups only the retained preview matches by file for inline output. The spill file stores the full formatted match list, not only the omitted tail, so `read offset/limit` works against the same logical result the model saw. -Raw `rg` stdout is an internal transport detail. If `ctx.bash.run()` returns untruncated stdout, the tool parses `stdout.text`. If stdout is truncated and `stdout.spillPath` is present, the tool reads that local raw spill file up to `rawOutputMaxBytes + 1` bytes and parses it only when the complete file fits within `rawOutputMaxBytes`. If the raw spill file is larger than `rawOutputMaxBytes`, or stdout is truncated without a spill path, the tool fails with a clear search error telling the model to narrow `pattern`, `path`, or `include`. The tool never exposes raw `rg` output or bash raw spill paths to the model. +Raw `rg` stdout is an internal transport detail. The tool requests `stdoutMaxBytes: rawOutputMaxBytes` through `ctx.bash.resolve()` and parses `stdout.text` only when the executor returns untruncated stdout within that cap. If stdout is larger than `rawOutputMaxBytes`, or the executor still returns `stdout.truncated`, the tool fails with a clear search error telling the model to narrow `pattern`, `path`, or `include`. The tool never exposes raw `rg` output or bash raw spill paths to the model. Only stdout is a parse source. Stderr is diagnostic text for invalid patterns, missing `rg`, and search failures; if bash truncates stderr, the tool uses the retained stderr tail with a truncation note and does not read `stderr.spillPath`. @@ -93,7 +93,7 @@ When a search produces more logical results than the inline cap and `ctx.spillFi When spill storage is absent, the call has no session owner, or saving fails, the tool still returns the inline page and a footer explaining that the complete result could not be saved. Search success must not turn into an `isError` result solely because formatted-result spill storage is unavailable. -The bash raw spill file and the formatted search spill file are different artifacts. The raw bash spill file is a local executor implementation detail used only so the search tool can parse complete `rg` stdout. The formatted spill file is the stable model-facing recovery path produced by `ctx.spillFiles.saveText()`. +The bash raw output stream and the formatted search spill file are different artifacts. Raw `rg` stdout is parsed only in memory within the requested bash stdout cap; the formatted spill file is the stable model-facing recovery path produced by `ctx.spillFiles.saveText()`. ### Result shape @@ -122,13 +122,13 @@ If the complete logical result fits under the inline cap, no formatted spill fil **Put `glob` / `grep` on `ctx.fs`.** Rejected for v1: it forces every filesystem backend to grow a search API and makes local ripgrep behavior part of the provider seam. Search is useful product behavior, but it is not a universal text-storage primitive like `readText` or `writeText`. -**Directly spawn ripgrep from `dsh-fs-local`.** Rejected for this RFC's v1: direct spawn gives the cleanest argv boundary, stdout/stderr control, and early-stop control, but it duplicates process execution concerns that the bash seam already owns: environment scrubbing, process-group kill, timeout propagation, sandbox/remote executor substitution, and raw output spill. It remains a reasonable optimization if bash-backed search proves too shell-string-sensitive or if raw bash spill recovery is not portable enough. +**Directly spawn ripgrep from `dsh-fs-local`.** Rejected for this RFC's v1: direct spawn gives the cleanest argv boundary, stdout/stderr control, and early-stop control, but it duplicates process execution concerns that the bash seam already owns: environment scrubbing, process-group kill, timeout propagation, sandbox/remote executor substitution, and bounded output capture. It remains a reasonable optimization if bash-backed search proves too shell-string-sensitive or if foreground streaming becomes necessary. **Use `ctx.bash.start()` for streaming early stop.** Rejected: `start()` creates model-visible background task semantics: task ids, owner tokens, `bash_output`, `bash_kill`, completion notifications, and no built-in timeout. `grep` needs a foreground tool result, not a background bash workflow. If streaming search becomes necessary, the right abstraction is a foreground streaming process handle on the bash/process seam, not borrowing the public background-task API. -**Expose bash raw spill paths to the model.** Rejected: a bash raw spill path contains raw `rg` stdout (`rg --json` records for grep), not the stable formatted search result. The search tool may read raw spill internally, but model recovery uses a formatted result saved through `ctx.spillFiles.saveText()`. +**Expose bash raw spill paths to the model.** Rejected: a bash raw spill path contains raw `rg` stdout (`rg --json` records for grep), not the stable formatted search result. Search parses raw stdout only as an internal transport; model recovery uses a formatted result saved through `ctx.spillFiles.saveText()`. -**Add `spillFiles.saveFile()` for bash output normalization first.** Rejected for this RFC's v1: `saveFile()` would help a future bash normalization pass move existing executor spill files into session-scoped spill storage, but search still has to parse raw `rg` output before producing the model-facing artifact. `saveText()` is sufficient for the formatted search result, and raw bash spill remains an executor-local recovery detail. +**Add `spillFiles.saveFile()` for bash output normalization first.** Rejected for this RFC's v1: `saveFile()` would help a future bash normalization pass move existing executor spill files into session-scoped spill storage, but search only needs bounded in-memory raw `rg` stdout before producing the model-facing artifact. `saveText()` is sufficient for the formatted search result. **Rely on the generic `dsh-spill-policy`.** Rejected: generic post-execute spill sees only the final tool result. If `grep` / `glob` return the first page inline, the generic policy cannot recover omitted results. The search tools must save the complete formatted result themselves before returning the bounded model-facing text. @@ -136,7 +136,7 @@ If the complete logical result fits under the inline cap, no formatted spill fil **Keep early-stop search and skip formatted spill files.** Rejected for this proposal: early stop is more efficient but gives the model no path to inspect later results. The chosen v1 optimizes result recoverability and implementation simplicity, with `timeoutMs`, `rawOutputMaxBytes`, bash backend caps, and formatted spill files as safety backstops. -**Expand the bash seam with a raw-output reader first.** Deferred: a remote bash backend may eventually need a portable `readRawOutput(ref, maxBytes)` style API instead of local `spillPath` reads. v1 uses the existing local-readable `stdout.spillPath` to avoid widening the bash seam for one consumer. +**Expand the bash seam with a raw-output reader first.** Rejected: a portable `readRawOutput(ref, maxBytes)` API would add reference lifetime, permission, and backend storage semantics. A per-run `stdoutMaxBytes` request is the narrower seam: search either receives complete stdout within `rawOutputMaxBytes` or fails clearly. ## Testing @@ -151,7 +151,7 @@ If the complete logical result fits under the inline cap, no formatted spill fil - `glob` and `grep` are model-facing tools in `@deepseek-ai/dsh-tool-fs-search`, not `ctx.fs` provider methods and not part of the existing `@deepseek-ai/dsh-tool-fs` root plugin. The package injects `tools`, `systemPrompt`, and `bash`; it does not inject `fs`, and `ctx.spillFiles` stays optional via `ctx.get('spillFiles')`. - The schemas are exactly `glob(pattern, path?)` and `grep(pattern, path?, include?)`; search caps and timeout are defaulted, validated Config fields (`globMaxResults`, `grepMaxMatches`, `grepMaxLineBytes`, `rawOutputMaxBytes`, `timeoutMs`). - The tools execute through `ctx.bash.resolve(request)` → `ctx.bash.run(spec)`, forward `exec.signal`, never call `ctx.bash.start()`, and never expose a bash task id. The bash request workdir comes from `exec.agent?.session.header.cwd` when available; the resolved `spec.workdir` drives execution and relative-path display. -- When bash stdout is truncated, the tools parse the full raw stdout only through a local `stdout.spillPath` that fits within `rawOutputMaxBytes`; missing spill paths or over-cap raw output are clear search failures, and raw `rg` output is never exposed to the model. +- The tools request `stdoutMaxBytes: rawOutputMaxBytes` from the bash seam, parse only untruncated stdout within that cap, and treat over-cap or still-truncated raw output as a clear search failure; raw `rg` output is never exposed to the model. - Oversized complete formatted results are saved through `ctx.spillFiles.saveText()` when available while inline results stay bounded; spill failure, a missing backend, or a missing owner preserves the inline result and reports the unsaved remainder — never an `isError`. - The package README, the generated config catalog, and exported JSDoc document the Config fields and `SEARCH_*` codes; the coding-agent example ships the tools (the acp-agent tree waits on the snapshot re-record above); the fs group README records the co-located bash/filesystem deployment requirement. @@ -163,6 +163,4 @@ Shell command construction is the sharpest safety edge. Because `ctx.bash` accep The v1 assumes a co-located bash/filesystem deployment. If bash searches one workspace and the `read` tool resolves paths against another, returned paths may not be follow-up-readable. The package documents this requirement but does not verify it at runtime. -Raw bash spill recovery is local-path-shaped in v1. A remote or sandboxed bash backend may return no readable `spillPath` or may require a future raw-output read API. In that case broad searches fail clearly instead of pretending a truncated raw result is complete. - Spill paths are local filesystem paths in v1. The formatted-result design works for local deployments where `read` can open spill files; remote or workspace-confined deployments need either an allowlist for spill paths or a future virtual spill URI bridge. diff --git a/packages/bash/bash-local/README.md b/packages/bash/bash-local/README.md index dec29ce93b..bdf4ec5f7f 100644 --- a/packages/bash/bash-local/README.md +++ b/packages/bash/bash-local/README.md @@ -21,7 +21,7 @@ Design surveyed against the bash tools of Claude Code, OpenCode, Codex, and pi; - **Spawn per call, no shell state** — every call is a fresh non-login `bash -c` (deterministic; no rc files). All four surveyed tools spawn per call. `XXX(stateful-shell)` in `src/run.ts` records the two proven stateful designs (Claude Code's cwd-only persistence; Codex's PTY exec sessions) for when real workflows demand them. - **Process-group kills with escalation** — children are spawned `detached` (own process group); kills send SIGTERM to the group, then SIGKILL after the `graceMs` grace (default 3s — OpenCode's escalation; pipelines and subshells die with the parent). ESRCH is tolerated; daemons that re-parent away from the group can still survive — same caveat as the surveyed tools. -- **Tail-keep truncation + spill files** — output beyond `maxOutputBytes` keeps the in-memory TAIL (errors/results cluster at the end — pi/OpenCode rationale) while the FULL stream is appended to a temp file whose path is reported when available. If the final spill close reports a delayed writeback failure, the executor still returns the tail but withholds the path rather than advertising a possibly incomplete file. +- **Tail-keep truncation + spill files** — output beyond `maxOutputBytes` keeps the in-memory TAIL (errors/results cluster at the end — pi/OpenCode rationale) while the FULL stream is appended to a temp file whose path is reported when available. A foreground `BashExecRequest.stdoutMaxBytes` can raise stdout's capture budget for one trusted caller; stderr and background tasks still use `maxOutputBytes`. If the final spill close reports a delayed writeback failure, the executor still returns the tail but withholds the path rather than advertising a possibly incomplete file. - **Model-friendly env + credential scrub** — `process.env` minus credential-shaped vars (`*KEY*`/`*SECRET*`/`*TOKEN*`), then `NO_COLOR=1 TERM=dumb PAGER=cat GIT_PAGER=cat` (Codex's hardcoded set) so pagers and ANSI color don't garble results. This scrub is the security control that keeps the harness's *ambient* credentials out of a spawned command. A spec's `env` is merged LAST (after the scrub), so a caller's explicit entry — a value it already holds — wins even on a credential-shaped name. The spec's `stdin`, when supplied, is written to the child and closed; with none supplied, fd 0 is `/dev/null` — the exact pre-seam default, so a command that probes stdin's file type is unaffected. Both `env`/`stdin` are set by in-process plugins (the hooks bridges); the model-facing tool doesn't expose them. See [the bash-stdin-env RFC](../../../docs/rfc/implemented/architecture/2026-06-30-bash-stdin-env-trusted-plugin-surface.md). - **Background tasks** — `start()` returns immediately, no timeout applies (Claude Code detaches timeouts when backgrounding), `readOutput()` is incremental with whole-stream byte offsets, and disposal kills everything. The spec's opaque `owner` token is stored on the tracked task and returned by `ownerOf(id)` — the executor never interprets it (the consumer's access policy does), and because it lives with the task here it survives a `tool-bash` HMR reload. diff --git a/packages/bash/bash-local/src/index.ts b/packages/bash/bash-local/src/index.ts index 3e09d7e35b..e8efbabc71 100644 --- a/packages/bash/bash-local/src/index.ts +++ b/packages/bash/bash-local/src/index.ts @@ -121,10 +121,13 @@ export class LocalBashExecutor extends BashExecutor { this.config.maxTimeoutMs, 'bash-local: request.timeoutMs', ) + const stdoutMaxBytes = request.stdoutMaxBytes ?? this.config.maxOutputBytes + assertPositiveFinite('request.stdoutMaxBytes', stdoutMaxBytes) return { command: request.command, workdir: request.workdir ?? this.config.cwd ?? process.cwd(), timeoutMs, + stdoutMaxBytes, ...request.signal ? { signal: request.signal } : {}, // Carry stdin/env through verbatim — optional, no config default (absent // means none). env merges AFTER the scrub in run.ts. @@ -144,7 +147,8 @@ export class LocalBashExecutor extends BashExecutor { const outcome = await runBash({ command: spec.command, cwd: spec.workdir, - maxOutputBytes: this.config.maxOutputBytes, + stdoutMaxBytes: spec.stdoutMaxBytes, + stderrMaxBytes: this.config.maxOutputBytes, graceMs: this.config.graceMs, signal: d.signal, stdin: spec.stdin, @@ -170,7 +174,8 @@ export class LocalBashExecutor extends BashExecutor { const running = runBash({ command: spec.command, cwd: spec.workdir, - maxOutputBytes: this.config.maxOutputBytes, + stdoutMaxBytes: this.config.maxOutputBytes, + stderrMaxBytes: this.config.maxOutputBytes, graceMs: this.config.graceMs, signal: spec.signal, stdin: spec.stdin, diff --git a/packages/bash/bash-local/src/run.ts b/packages/bash/bash-local/src/run.ts index bc4a017dea..bc29da60f5 100644 --- a/packages/bash/bash-local/src/run.ts +++ b/packages/bash/bash-local/src/run.ts @@ -77,8 +77,10 @@ export function childEnv(extra?: Record): NodeJS.ProcessEnv { export interface SpawnSpec { command: string cwd: string - /** Per-stream in-memory cap; overflow spills to disk (tail kept in memory). */ - maxOutputBytes: number + /** Stdout in-memory cap; overflow spills to disk (tail kept in memory). */ + stdoutMaxBytes: number + /** Stderr in-memory cap; overflow spills to disk (tail kept in memory). */ + stderrMaxBytes: number /** Grace period between the SIGTERM and the SIGKILL escalation on a kill. */ graceMs: number /** @@ -351,8 +353,8 @@ export function runBash(spec: SpawnSpec, internals: RunInternals = {}): RunningB ? spawn('bash', ['-c', spec.command], { cwd: spec.cwd, env, stdio: ['pipe', 'pipe', 'pipe'], detached: true }) : spawn('bash', ['-c', spec.command], { cwd: spec.cwd, env, stdio: ['ignore', 'pipe', 'pipe'], detached: true }) - const stdout = new OutputCollector(spec.maxOutputBytes, 'stdout', spillDir) - const stderr = new OutputCollector(spec.maxOutputBytes, 'stderr', spillDir) + const stdout = new OutputCollector(spec.stdoutMaxBytes, 'stdout', spillDir) + const stderr = new OutputCollector(spec.stderrMaxBytes, 'stderr', spillDir) child.stdout.on('data', (chunk: Buffer) => { stdout.push(chunk) }) child.stderr.on('data', (chunk: Buffer) => { stderr.push(chunk) }) diff --git a/packages/bash/bash-local/tests/executor.spec.ts b/packages/bash/bash-local/tests/executor.spec.ts index 98095b8d47..3d845a88c3 100644 --- a/packages/bash/bash-local/tests/executor.spec.ts +++ b/packages/bash/bash-local/tests/executor.spec.ts @@ -86,6 +86,23 @@ describe('LocalBashExecutor.run', () => { const { bash } = await setup() expect(() => bash.resolve({ command: 'true', timeoutMs: Number.NaN })).toThrow(/request\.timeoutMs/) expect(() => bash.resolve({ command: 'true', timeoutMs: -1 })).toThrow(/request\.timeoutMs/) + expect(() => bash.resolve({ command: 'true', stdoutMaxBytes: Number.NaN })).toThrow(/request\.stdoutMaxBytes/) + expect(() => bash.resolve({ command: 'true', stdoutMaxBytes: -1 })).toThrow(/request\.stdoutMaxBytes/) + }) + + it('defaults stdoutMaxBytes to maxOutputBytes and lets foreground callers raise stdout only', async () => { + const { bash } = await setup({ maxOutputBytes: 100 }) + expect(bash.resolve({ command: 'true' }).stdoutMaxBytes).toBe(100) + + const result = await bash.run(bash.resolve({ + command: 'printf "%.0sx" $(seq 1 500); printf "%.0se" $(seq 1 500) >&2', + stdoutMaxBytes: 500, + })) + + expect(result.stdout.truncated).toBe(false) + expect(result.stdout.text).toBe('x'.repeat(500)) + expect(result.stderr.truncated).toBe(true) + expect(result.stderr.text.length).toBeLessThanOrEqual(100) }) it('kill escalation uses the configured graceMs (a TERM-trapping task dies by SIGKILL)', async () => { diff --git a/packages/bash/bash-local/tests/run.spec.ts b/packages/bash/bash-local/tests/run.spec.ts index 1d6e93afe3..dc4146f9de 100644 --- a/packages/bash/bash-local/tests/run.spec.ts +++ b/packages/bash/bash-local/tests/run.spec.ts @@ -26,7 +26,8 @@ function spec(command: string, overrides: Partial[0]> return { command, cwd: process.cwd(), - maxOutputBytes: 64_000, + stdoutMaxBytes: 64_000, + stderrMaxBytes: 64_000, graceMs: 3_000, ...overrides, } @@ -229,10 +230,24 @@ describe('stdin and extra env (set by in-process plugins)', () => { }) describe('output truncation and spill', () => { + it('applies stdout and stderr caps independently', async () => { + const result = await runBash( + spec('printf "%.0sx" $(seq 1 500); printf "%.0se" $(seq 1 500) >&2', { + stdoutMaxBytes: 500, + stderrMaxBytes: 100, + }), + { spillDir }, + ).done + expect(result.stdout.truncated).toBe(false) + expect(result.stdout.text).toBe('x'.repeat(500)) + expect(result.stderr.truncated).toBe(true) + expect(result.stderr.text.length).toBeLessThanOrEqual(100) + }) + it('keeps the tail and spills the full stream to disk', async () => { // 200 numbered lines of ~10 bytes; cap at 500 bytes keeps a late tail. const result = await runBash( - spec('for i in $(seq 1 200); do printf "line-%04d\\n" $i; done', { maxOutputBytes: 500 }), + spec('for i in $(seq 1 200); do printf "line-%04d\\n" $i; done', { stdoutMaxBytes: 500, stderrMaxBytes: 500 }), { spillDir }, ).done expect(result.stdout.truncated).toBe(true) @@ -247,7 +262,7 @@ describe('output truncation and spill', () => { it('does not truncate output exactly at the cap', async () => { const result = await runBash( - spec('printf "%.0sx" $(seq 1 500)', { maxOutputBytes: 500 }), + spec('printf "%.0sx" $(seq 1 500)', { stdoutMaxBytes: 500, stderrMaxBytes: 500 }), { spillDir }, ).done expect(result.stdout.truncated).toBe(false) @@ -258,7 +273,7 @@ describe('output truncation and spill', () => { it('settles with the tail and no spill path when final spill close fails', async () => { failNextClose.value = true const result = await runBash( - spec('for i in $(seq 1 200); do printf "line-%04d\\n" $i; done', { maxOutputBytes: 500 }), + spec('for i in $(seq 1 200); do printf "line-%04d\\n" $i; done', { stdoutMaxBytes: 500, stderrMaxBytes: 500 }), { spillDir }, ).done expect(failNextClose.value).toBe(false) @@ -377,7 +392,7 @@ describe('review fixes: env scrubbing and spill hardening', () => { it('creates spill files with owner-only permissions and random names', async () => { const result = await runBash( - spec('for i in $(seq 1 200); do printf "line-%04d\\n" $i; done', { maxOutputBytes: 500 }), + spec('for i in $(seq 1 200); do printf "line-%04d\\n" $i; done', { stdoutMaxBytes: 500, stderrMaxBytes: 500 }), { spillDir }, ).done const path = result.stdout.spillPath! @@ -388,7 +403,7 @@ describe('review fixes: env scrubbing and spill hardening', () => { it('defaults spills into a private per-process directory', async () => { const result = await runBash( - spec('for i in $(seq 1 200); do printf "line-%04d\\n" $i; done', { maxOutputBytes: 500 }), + spec('for i in $(seq 1 200); do printf "line-%04d\\n" $i; done', { stdoutMaxBytes: 500, stderrMaxBytes: 500 }), ).done const dir = dirname(result.stdout.spillPath!) expect(dir).toMatch(/dsh-bash-/) diff --git a/packages/bash/bash/README.md b/packages/bash/bash/README.md index 39318ae371..dc46ad78f1 100644 --- a/packages/bash/bash/README.md +++ b/packages/bash/bash/README.md @@ -28,6 +28,6 @@ Implementations subclass `BashExecutor`, implement the abstract methods, and cal ## Vocabulary -`BashExecRequest` (command, workdir?, timeoutMs?, signal?, stdin?, env?, owner?) resolves to `BashExecSpec` (command, workdir, timeoutMs, signal?, stdin?, env?, owner) before execution; `owner` is optional on the request and **required-but-nullable** (`OwnerToken | undefined`) on the resolved spec, so a forgotten owner is a visible `undefined` rather than a silently-absent property. The task id (`BashTaskId`) and the `owner` token (`OwnerToken`) are [branded](../../util/brand) — `OwnerToken` is a DISTINCT brand from `SessionId` (the seam never imports `dsh-session`; the `dsh-tool-bash` consumer is the single boundary that casts its `SessionId` into one). `run()` returns `BashRunResult` (exitCode, signal, timedOut, aborted, timeoutMs, stdout/stderr as `CollectedOutput`) and `start()`/`readOutput()` use `BashTask`/`BashTaskRead` for the background side. See `src/types.ts` for the full contracts. +`BashExecRequest` (command, workdir?, timeoutMs?, stdoutMaxBytes?, signal?, stdin?, env?, owner?) resolves to `BashExecSpec` (command, workdir, timeoutMs, stdoutMaxBytes, signal?, stdin?, env?, owner) before execution; `owner` is optional on the request and **required-but-nullable** (`OwnerToken | undefined`) on the resolved spec, so a forgotten owner is a visible `undefined` rather than a silently-absent property. The task id (`BashTaskId`) and the `owner` token (`OwnerToken`) are [branded](../../util/brand) — `OwnerToken` is a DISTINCT brand from `SessionId` (the seam never imports `dsh-session`; the `dsh-tool-bash` consumer is the single boundary that casts its `SessionId` into one). `stdoutMaxBytes` is a trusted foreground-run capture budget for consumers that must parse complete stdout up to their own limit; the model-facing bash tool does not expose it. `run()` returns `BashRunResult` (exitCode, signal, timedOut, aborted, timeoutMs, stdout/stderr as `CollectedOutput`) and `start()`/`readOutput()` use `BashTask`/`BashTaskRead` for the background side. See `src/types.ts` for the full contracts. `stdin` and `env` are set by in-process plugins (the hooks bridges, native plugins) to feed a hook command its JSON payload on stdin and its `CLAUDE_PROJECT_DIR`/`CLAUDE_PLUGIN_ROOT` env. The model-facing `dsh-tool-bash` tool does not expose them as parameters — a model already has equivalent power through shell syntax (`FOO=bar cmd`, a heredoc), so they would be redundant tool params. This is not a security boundary: the implementation's credential scrub (not these fields) is what keeps the harness's ambient secrets out of a spawned command. They are plain optionals on the resolved spec (unlike `owner`'s required-but-nullable): a missing one means "none", the safe default. See [the bash-stdin-env RFC](../../../docs/rfc/implemented/architecture/2026-06-30-bash-stdin-env-trusted-plugin-surface.md). diff --git a/packages/bash/bash/src/types.ts b/packages/bash/bash/src/types.ts index 7b27231312..11ff7a0124 100644 --- a/packages/bash/bash/src/types.ts +++ b/packages/bash/bash/src/types.ts @@ -52,6 +52,13 @@ export interface BashExecRequest { workdir?: string | undefined /** Timeout override in milliseconds (implementations cap it). */ timeoutMs?: number | undefined + /** + * Foreground stdout capture budget in bytes. Absent uses the executor's + * default output cap. Trusted in-process consumers use this when they must + * parse complete stdout up to their own bounded limit; the model-facing bash + * tool does not expose it as a parameter. + */ + stdoutMaxBytes?: number | undefined /** Abort signal — implementations kill the command when it fires. */ signal?: AbortSignal | undefined /** @@ -95,6 +102,11 @@ export interface BashExecSpec { command: string workdir: string timeoutMs: number + /** + * Resolved foreground stdout capture budget in bytes. `run()` uses it for + * stdout; background tasks and stderr keep the executor's own output cap. + */ + stdoutMaxBytes: number /** Abort signal — implementations kill the command when it fires. */ signal?: AbortSignal | undefined /** diff --git a/packages/bash/bash/tests/service.spec.ts b/packages/bash/bash/tests/service.spec.ts index 81530843ed..38b79031d2 100644 --- a/packages/bash/bash/tests/service.spec.ts +++ b/packages/bash/bash/tests/service.spec.ts @@ -13,6 +13,7 @@ class StubExecutor extends BashExecutor { command: request.command, workdir: request.workdir ?? '/stub', timeoutMs: request.timeoutMs ?? 1000, + stdoutMaxBytes: request.stdoutMaxBytes ?? 64_000, ...request.signal ? { signal: request.signal } : {}, owner: request.owner, } diff --git a/packages/bash/tool-bash/README.md b/packages/bash/tool-bash/README.md index eabb9298aa..d827e7c383 100644 --- a/packages/bash/tool-bash/README.md +++ b/packages/bash/tool-bash/README.md @@ -44,7 +44,7 @@ When a background task finishes, a short notice is injected into the owning agen ## The tool builds its request from named args only -The `BashExecRequest` seam carries optional `stdin` and `env`, used by the hooks bridges to feed a hook command its JSON payload and `CLAUDE_*` env. This tool does **not** expose them as parameters: its request is built from `command`/`workdir`/`timeoutMs`/`signal`/`owner` only, so a model that includes `env` or `stdin` keys in its tool arguments has them ignored. This is not a trust boundary — a model already has equivalent power through shell syntax (`FOO=bar cmd`, a heredoc), and the real defense against leaking the harness's ambient secrets is `dsh-bash-local`'s credential scrub, which works regardless. A regression guard drives the real tool with those extra args and asserts the resulting request carries neither field — its job is to catch a future refactor that blindly spreads `...args` into the request (which would silently forward model input into the post-scrub `env` merge), not to defend a wall. See [the bash-stdin-env RFC](../../../docs/rfc/implemented/architecture/2026-06-30-bash-stdin-env-trusted-plugin-surface.md). +The `BashExecRequest` seam carries optional trusted-plugin fields (`stdoutMaxBytes`, `stdin`, and `env`); hooks use `stdin`/`env` to feed a hook command its JSON payload and `CLAUDE_*` env. This tool does **not** expose them as parameters: its request is built from `command`/`workdir`/`timeoutMs`/`signal`/`owner` only, so a model that includes `env`, `stdin`, or `stdoutMaxBytes` keys in its tool arguments has them ignored. This is not a trust boundary — a model already has equivalent power through shell syntax (`FOO=bar cmd`, a heredoc), and the real defense against leaking the harness's ambient secrets is `dsh-bash-local`'s credential scrub, which works regardless. A regression guard drives the real tool with those extra args and asserts the resulting request carries none of those fields — its job is to catch a future refactor that blindly spreads `...args` into the request (which would silently forward model input into the post-scrub `env` merge), not to defend a wall. See [the bash-stdin-env RFC](../../../docs/rfc/implemented/architecture/2026-06-30-bash-stdin-env-trusted-plugin-surface.md). ## Permissions diff --git a/packages/bash/tool-bash/tests/tools.spec.ts b/packages/bash/tool-bash/tests/tools.spec.ts index 7d4b34f74f..a6f1bac5b1 100644 --- a/packages/bash/tool-bash/tests/tools.spec.ts +++ b/packages/bash/tool-bash/tests/tools.spec.ts @@ -98,6 +98,7 @@ class LossyReadBashExecutor extends BashExecutor { command: request.command, workdir: request.workdir ?? process.cwd(), timeoutMs: request.timeoutMs ?? 0, + stdoutMaxBytes: request.stdoutMaxBytes ?? 64_000, ...request.signal ? { signal: request.signal } : {}, owner: request.owner, } @@ -873,11 +874,12 @@ describe('the model-facing bash tool builds its request from named args only (no /** * Records every {@link BashExecRequest} the consumer hands to `resolve()`, so a * test can assert what the model-facing tool DID and DID NOT forward. The `bash` - * tool does not expose `stdin`/`env` as parameters (bash syntax already gives a - * model that power), so it must build its request from named args only and + * tool does not expose trusted-plugin fields (`stdoutMaxBytes`, `stdin`, or + * `env`) as parameters, so it must build its request from named args only and * never spread unknown tool-call keys into it. This guard's job is to catch a * future refactor that blindly forwards `...args` — which would silently thread - * model input into the post-scrub `env` merge — NOT to defend a trust boundary + * model input into the post-scrub `env` merge or per-run capture budget — NOT + * to defend a trust boundary * (the credential scrub in dsh-bash-local is the security control; see the * bash-stdin-env RFC). Foreground `run()` returns a canned result; `start()` is * unused here. @@ -890,6 +892,7 @@ describe('the model-facing bash tool builds its request from named args only (no command: request.command, workdir: request.workdir ?? process.cwd(), timeoutMs: request.timeoutMs ?? 0, + stdoutMaxBytes: request.stdoutMaxBytes ?? 64_000, ...request.signal ? { signal: request.signal } : {}, ...request.stdin !== undefined ? { stdin: request.stdin } : {}, ...request.env !== undefined ? { env: request.env } : {}, @@ -920,12 +923,12 @@ describe('the model-facing bash tool builds its request from named args only (no return { ctx, bash: ctx.bash as RecordingBashExecutor } } - it('does not forward env/stdin even when the model includes them as extra arguments', async () => { + it('does not forward trusted-only fields even when the model includes them as extra arguments', async () => { const { ctx, bash } = await setupRecording() - // Extra args: the model includes `env` and `stdin` keys hoping they reach the + // Extra args: the model includes trusted-plugin keys hoping they reach the // executor. The bash tool's schema ignores unknown keys, and execute() builds // the request from only command/workdir/timeoutMs/signal — so the recorded - // request carries NEITHER. (Not a security wall — the model could set an env + // request carries NONE. (Not a security wall — the model could set an env // var or feed stdin via shell syntax anyway; this just keeps the request // shape honest so a future `...args` spread can't silently forward input.) await ctx.tools.execute({ @@ -936,6 +939,7 @@ describe('the model-facing bash tool builds its request from named args only (no description: 'echo', env: { SNEAKY_API_KEY: 'leak' }, stdin: 'malicious payload', + stdoutMaxBytes: 999_999, }, }) expect(bash.requests).toHaveLength(1) @@ -943,9 +947,10 @@ describe('the model-facing bash tool builds its request from named args only (no expect(request.command).toBe('echo hi') expect('env' in request).toBe(false) expect('stdin' in request).toBe(false) + expect('stdoutMaxBytes' in request).toBe(false) }) - it('a background bash call likewise carries no env/stdin', async () => { + it('a background bash call likewise carries no trusted-only fields', async () => { const { ctx, bash } = await setupRecording() // start() throws in this recorder, but resolve() runs first and records the // request — which is all this no-forward assertion needs. @@ -958,15 +963,17 @@ describe('the model-facing bash tool builds its request from named args only (no run_in_background: true, env: { TOKEN: 'leak' }, stdin: 'x', + stdoutMaxBytes: 999_999, }, }) expect(bash.requests).toHaveLength(1) const request = bash.requests[0]! expect('env' in request).toBe(false) expect('stdin' in request).toBe(false) + expect('stdoutMaxBytes' in request).toBe(false) // The owner token IS set on a background call (the isolation fence) — proving // the recorder sees the real request the consumer built, so the absent - // env/stdin above is a real negative, not a recorder that drops everything. + // trusted-only fields above are a real negative, not a recorder that drops everything. expect('owner' in request).toBe(true) }) }) diff --git a/packages/fs/tool-fs-search/README.md b/packages/fs/tool-fs-search/README.md index e1ed680060..469830683d 100644 --- a/packages/fs/tool-fs-search/README.md +++ b/packages/fs/tool-fs-search/README.md @@ -39,8 +39,8 @@ Routine budgets stay out of the model-facing schema (no `head_limit`/`offset`/`c ## Two budgets, two artifacts -Raw `rg` stdout is an internal transport detail. When the executor truncates it, the tool recovers the complete stream from the executor's **raw bash spill file** — read locally, capped at `rawOutputMaxBytes`, never shown to the model. The model-facing recovery artifact is different: when a search yields more logical results than the inline cap, the tool saves the COMPLETE formatted result through `ctx.spillFiles.saveText()` (suggested names `glob-results.txt` / `grep-results.txt`, owner = the calling session, source = the tool execution identity) and appends a footer naming the saved path. This is the first tool-owned spill call in the codebase — deliberate, because retention here is item-level: the generic `@deepseek-ai/dsh-spill-policy` only sees the final text on `tools/post-execute`, by which point a capped search has already omitted later paths/matches. A missing spill backend, a call with no session owner, or a `saveText()` failure keeps the inline page and reports that the complete result could not be saved — never an `isError`. +Raw `rg` stdout is an internal transport detail. Each search requests `stdoutMaxBytes: rawOutputMaxBytes` from the bash seam and parses only complete retained stdout; if the executor still returns `stdout.truncated`, the search fails with `SEARCH_RAW_OUTPUT_OVERFLOW` and tells the model to narrow the query. The model-facing recovery artifact is different: when a search yields more logical results than the inline cap, the tool saves the COMPLETE formatted result through `ctx.spillFiles.saveText()` (suggested names `glob-results.txt` / `grep-results.txt`, owner = the calling session, source = the tool execution identity) and appends a footer naming the saved path. This is the first tool-owned spill call in the codebase — deliberate, because retention here is item-level: the generic `@deepseek-ai/dsh-spill-policy` only sees the final text on `tools/post-execute`, by which point a capped search has already omitted later paths/matches. A missing spill backend, a call with no session owner, or a `saveText()` failure keeps the inline page and reports that the complete result could not be saved — never an `isError`. ## Errors -Search failures carry the package-owned `SearchError` (a `HarnessError` subclass), surfaced as `{ name, code }` on `isError` results: `SEARCH_INVALID_PATTERN` (ripgrep rejected the regex/glob), `SEARCH_FAILED` (missing `rg`, inaccessible target, signal kill, malformed `--json` output), `SEARCH_RAW_OUTPUT_OVERFLOW` (raw output over `rawOutputMaxBytes`, or truncated with no recovery file), and `SEARCH_ABORTED` (tool timeout, caller cancellation, or the bash executor's own timeout). ripgrep exit semantics are tool-owned: exit 0 is success with results, exit 1 is a successful empty search (`No files found` / `No matches found`), and only other exits are failures. Model argument mistakes (blank pattern, a list-valued `include`) stay ordinary tool argument errors. +Search failures carry the package-owned `SearchError` (a `HarnessError` subclass), surfaced as `{ name, code }` on `isError` results: `SEARCH_INVALID_PATTERN` (ripgrep rejected the regex/glob), `SEARCH_FAILED` (missing `rg`, inaccessible target, signal kill, malformed `--json` output), `SEARCH_RAW_OUTPUT_OVERFLOW` (raw output over `rawOutputMaxBytes`, or still truncated after the requested stdout capture budget), and `SEARCH_ABORTED` (tool timeout, caller cancellation, or the bash executor's own timeout). ripgrep exit semantics are tool-owned: exit 0 is success with results, exit 1 is a successful empty search (`No files found` / `No matches found`), and only other exits are failures. Model argument mistakes (blank pattern, a list-valued `include`) stay ordinary tool argument errors. diff --git a/packages/fs/tool-fs-search/src/search-core.ts b/packages/fs/tool-fs-search/src/search-core.ts index a47adfa7c3..233c1e78d4 100644 --- a/packages/fs/tool-fs-search/src/search-core.ts +++ b/packages/fs/tool-fs-search/src/search-core.ts @@ -7,17 +7,15 @@ * Both tools execute through `ctx.bash.resolve(request)` → `ctx.bash.run(spec)` * as ordinary foreground tool calls — never `ctx.bash.start()`, never a * model-visible background task. Raw `rg` stdout is an internal transport - * detail: when the executor truncates it, the ONLY recovery source is the - * executor's local raw spill file, read here up to `rawOutputMaxBytes` and - * never exposed to the model. The model-facing recovery artifact is the + * detail: the tools request a per-run stdout capture budget from the bash seam, + * parse only complete in-memory stdout within `rawOutputMaxBytes`, and never + * read executor spill files. The model-facing recovery artifact is the * formatted result saved through `ctx.spillFiles.saveText()` - * ({@link trySaveFormattedResult}) — a different artifact from the bash raw - * spill file. + * ({@link trySaveFormattedResult}). * * @module @deepseek-ai/dsh-tool-fs-search/search-core */ -import { readFile, stat } from 'node:fs/promises' import { isAbsolute, relative, sep } from 'node:path' import type { Context } from 'cordis' import { HarnessError } from '@deepseek-ai/dsh-llm' @@ -45,7 +43,7 @@ export const SEARCH_TIMEOUT_MS = 30_000 * glob; `SEARCH_FAILED` — the search could not run or its output could not be * parsed (missing `rg`, inaccessible target, signal kill, malformed `--json`); * `SEARCH_RAW_OUTPUT_OVERFLOW` — raw `rg` output exceeded `rawOutputMaxBytes` - * (or was truncated with no recovery file); `SEARCH_ABORTED` — the tool + * or stayed truncated after that requested stdout budget; `SEARCH_ABORTED` — the tool * timeout, caller cancellation, or the bash executor's own timeout cut the * search short. */ @@ -72,7 +70,7 @@ export class SearchError extends HarnessError { /** The completed acquisition of one `rg` run: complete stdout plus the resolved workdir. */ export interface RipgrepRun { - /** Complete raw stdout — inline executor text, or the raw spill file's content. */ + /** Complete raw stdout retained by the bash executor within the requested cap. */ stdout: string /** True when ripgrep exited 1: a successful search with zero results. */ noMatches: boolean @@ -104,14 +102,11 @@ function classifyRunFailure(toolName: string, result: BashRunResult): SearchErro /** * Acquire the COMPLETE raw stdout of a finished run, enforcing - * `rawOutputMaxBytes` on BOTH transports: inline executor text (an executor - * retaining more than this package's cap must not smuggle an over-cap parse - * through the untruncated path) and the executor's local raw spill file, read - * only when the complete file fits the cap. A missing spill path or over-cap - * output is a clear failure telling the model to narrow the search — never a - * silently-partial parse. + * `rawOutputMaxBytes` on the in-memory transport. A truncated result means the + * bash backend could not retain complete stdout within the requested budget, so + * the tool fails clearly instead of parsing a silently-partial stream. */ -async function completeStdout(toolName: string, result: BashRunResult, rawOutputMaxBytes: number): Promise { +function completeStdout(toolName: string, result: BashRunResult, rawOutputMaxBytes: number): string { const narrow = 'narrow pattern, path, or include and retry' if (!result.stdout.truncated) { const inlineBytes = Buffer.byteLength(result.stdout.text, 'utf8') @@ -123,26 +118,10 @@ async function completeStdout(toolName: string, result: BashRunResult, rawOutput } return result.stdout.text } - const spillPath = result.stdout.spillPath - if (spillPath === undefined) { - throw new SearchError( - `${toolName} produced more raw output than the bash executor retained and no raw spill file is available; ${narrow}`, - 'SEARCH_RAW_OUTPUT_OVERFLOW', - ) - } - try { - const { size } = await stat(spillPath) - if (size > rawOutputMaxBytes) { - throw new SearchError( - `${toolName} produced ${size} bytes of raw output, over the ${rawOutputMaxBytes}-byte cap; ${narrow}`, - 'SEARCH_RAW_OUTPUT_OVERFLOW', - ) - } - return await readFile(spillPath, 'utf8') - } catch (error: unknown) { - if (error instanceof SearchError) throw error - throw new SearchError(`${toolName} could not read the executor's raw output spill file`, 'SEARCH_FAILED', { cause: error }) - } + throw new SearchError( + `${toolName} produced more raw output than the bash executor retained within the ${rawOutputMaxBytes}-byte cap; ${narrow}`, + 'SEARCH_RAW_OUTPUT_OVERFLOW', + ) } /** @@ -181,6 +160,7 @@ export async function runRipgrep( const cwd = exec.agent?.session.header.cwd const spec = ctx.bash.resolve({ command, + stdoutMaxBytes: rawOutputMaxBytes, ...cwd !== undefined ? { workdir: cwd } : {}, ...exec.signal ? { signal: exec.signal } : {}, }) @@ -208,7 +188,7 @@ export async function runRipgrep( if (result.exitCode !== 0 && result.exitCode !== 1) { throw classifyRunFailure(toolName, result) } - const stdout = await completeStdout(toolName, result, rawOutputMaxBytes) + const stdout = completeStdout(toolName, result, rawOutputMaxBytes) return { stdout, noMatches: result.exitCode === 1, workdir: spec.workdir } } diff --git a/packages/fs/tool-fs-search/tests/tools.spec.ts b/packages/fs/tool-fs-search/tests/tools.spec.ts index 7abbe20638..b3afa8ce3e 100644 --- a/packages/fs/tool-fs-search/tests/tools.spec.ts +++ b/packages/fs/tool-fs-search/tests/tools.spec.ts @@ -2,7 +2,7 @@ * Consumer-surface tests for the search tools over a FAKE bash executor and a * FAKE spill backend, exercised through `ctx.tools.execute()` so nothing * bypasses the tool registry. The fake executor makes every seam outcome - * scriptable — truncated stdout with/without a raw spill file, abort/timeout, + * scriptable — truncated stdout with/without a raw spill path, abort/timeout, * signal kills, ripgrep exit codes — so these tests verify schemas, argument * validation, shell-safe command construction, workdir derivation, signal * forwarding, `SEARCH_*` error classification, retention, formatted-result @@ -10,10 +10,7 @@ * pinned separately in integration.spec.ts. */ -import { afterEach, describe, expect, it } from 'vitest' -import { mkdtemp, rm, writeFile } from 'node:fs/promises' -import { tmpdir } from 'node:os' -import { join } from 'node:path' +import { describe, expect, it } from 'vitest' import { Context } from 'cordis' import { CallId } from '@deepseek-ai/dsh-llm' import SystemPrompt, { renderPrompt } from '@deepseek-ai/dsh-system-prompt' @@ -66,6 +63,7 @@ class FakeBash extends BashExecutor { command: request.command, workdir: request.workdir ?? '/work', timeoutMs: request.timeoutMs ?? 60_000, + stdoutMaxBytes: request.stdoutMaxBytes ?? 64_000, signal: request.signal, owner: request.owner, } @@ -388,28 +386,18 @@ describe('exit semantics and failure classification', () => { }) describe('raw output acquisition', () => { - let dir: string - afterEach(async () => { - await rm(dir, { recursive: true, force: true }) + it('passes rawOutputMaxBytes to bash as the stdout capture budget', async () => { + const { ctx, bash } = await setup({ config: { rawOutputMaxBytes: 1234 } }) + bash.handler = () => runResult('', { exitCode: 1 }) + await call(ctx, 'glob', { pattern: '*.ts' }) + await call(ctx, 'grep', { pattern: 'needle' }) + expect(bash.requests.map(request => request.stdoutMaxBytes)).toEqual([1234, 1234]) + expect(bash.specs.map(spec => spec.stdoutMaxBytes)).toEqual([1234, 1234]) }) - it('parses the complete raw spill file when stdout is truncated', async () => { - dir = await mkdtemp(join(tmpdir(), 'dsh-search-raw-')) - const spillPath = join(dir, 'raw.txt') - await writeFile(spillPath, 'one.ts\ntwo.ts\nthree.ts\n') - const { ctx, bash } = await setup() - bash.handler = () => runResult('', { stdout: { text: 'one.ts\n', truncated: true, spillPath } }) - const result = await call(ctx, 'glob', { pattern: '*.ts' }) - expect(result.isError).toBe(false) - expect(text(result)).toBe('one.ts\ntwo.ts\nthree.ts') - }) - - it('fails with SEARCH_RAW_OUTPUT_OVERFLOW when the raw spill file exceeds the cap', async () => { - dir = await mkdtemp(join(tmpdir(), 'dsh-search-raw-')) - const spillPath = join(dir, 'raw.txt') - await writeFile(spillPath, 'x'.repeat(64)) + it('fails with SEARCH_RAW_OUTPUT_OVERFLOW when truncated stdout has a raw spill path', async () => { const { ctx, bash } = await setup({ config: { rawOutputMaxBytes: 16 } }) - bash.handler = () => runResult('', { stdout: { text: 'x', truncated: true, spillPath } }) + bash.handler = () => runResult('', { stdout: { text: 'x', truncated: true, spillPath: '/does/not/get-read' } }) const result = await call(ctx, 'glob', { pattern: '*' }) expect(result.error).toMatchObject({ code: 'SEARCH_RAW_OUTPUT_OVERFLOW' }) expect(text(result)).toContain('narrow pattern, path, or include') @@ -419,7 +407,6 @@ describe('raw output acquisition', () => { // An executor retaining more inline than this package's cap (or a // deployment lowering rawOutputMaxBytes below the bash retention) must not // smuggle an over-cap parse through the untruncated path. - dir = await mkdtemp(join(tmpdir(), 'dsh-search-raw-')) const { ctx, bash } = await setup({ config: { rawOutputMaxBytes: 16 } }) bash.handler = () => runResult(`${'x'.repeat(64)}\n`) const result = await call(ctx, 'grep', { pattern: 'x' }) @@ -428,21 +415,11 @@ describe('raw output acquisition', () => { }) it('fails with SEARCH_RAW_OUTPUT_OVERFLOW when truncated stdout has no spill path', async () => { - dir = await mkdtemp(join(tmpdir(), 'dsh-search-raw-')) const { ctx, bash } = await setup() bash.handler = () => runResult('', { stdout: { text: 'partial', truncated: true } }) const result = await call(ctx, 'grep', { pattern: 'x' }) expect(result.error).toMatchObject({ code: 'SEARCH_RAW_OUTPUT_OVERFLOW' }) }) - - it('fails with SEARCH_FAILED when the raw spill file cannot be read', async () => { - dir = await mkdtemp(join(tmpdir(), 'dsh-search-raw-')) - const { ctx, bash } = await setup() - bash.handler = () => runResult('', { stdout: { text: 'partial', truncated: true, spillPath: join(dir, 'gone.txt') } }) - const result = await call(ctx, 'glob', { pattern: '*' }) - expect(result.error).toMatchObject({ code: 'SEARCH_FAILED' }) - expect(text(result)).toContain('raw output spill file') - }) }) describe('glob results', () => { diff --git a/packages/hooks/hook-protocol/tests/runner.spec.ts b/packages/hooks/hook-protocol/tests/runner.spec.ts index 1972a39c99..45e6598eeb 100644 --- a/packages/hooks/hook-protocol/tests/runner.spec.ts +++ b/packages/hooks/hook-protocol/tests/runner.spec.ts @@ -22,6 +22,7 @@ function recordingBash(run: (spec: BashExecSpec) => Promise): { command: request.command, workdir: request.workdir ?? '/stub', timeoutMs: request.timeoutMs ?? 0, + stdoutMaxBytes: request.stdoutMaxBytes ?? 64_000, ...request.signal ? { signal: request.signal } : {}, ...request.stdin !== undefined ? { stdin: request.stdin } : {}, ...request.env !== undefined ? { env: request.env } : {}, From 8b86c3febc03d4416b1e2868f04511ff9d1720cc Mon Sep 17 00:00:00 2001 From: Dudu-0223 Date: Fri, 10 Jul 2026 13:15:15 +0800 Subject: [PATCH 7/7] fix: stabilize spill snapshot path budget --- examples/acp-agent/cordis.yml | 6 ++++-- .../tests/snapshots/bash-spill/session.jsonl | 2 +- .../snapshots/bash-spill/stdout.golden.jsonl | 2 +- packages/support/acp-snapshot/src/harness.ts | 5 +++++ packages/support/acp-snapshot/src/normalize.ts | 6 ++++++ .../support/acp-snapshot/tests/normalize.spec.ts | 15 +++++++++++++++ 6 files changed, 32 insertions(+), 4 deletions(-) diff --git a/examples/acp-agent/cordis.yml b/examples/acp-agent/cordis.yml index 48760828e2..a6889114fd 100644 --- a/examples/acp-agent/cordis.yml +++ b/examples/acp-agent/cordis.yml @@ -113,11 +113,13 @@ # results under the session cwd, and the post-execute policy that replaces the # model-facing result with a bounded preview + read path. Snapshots lower the # cap so a deterministic bash result exercises this transcript surface without a -# real model call; normal demo runs keep the coding-agent cap. +# real model call. The snapshot harness supplies a fixed spill root so the +# spill-policy preview budget is stable across macOS/Linux path lengths; normal +# demo runs keep the session-local `.spill` root and the coding-agent cap. - id: spill-local name: '@deepseek-ai/dsh-spill-local' config: - root: ./.spill + root: !!js process.env.DSH_SNAPSHOT_SPILL_ROOT ?? './.spill' - id: spill-policy name: '@deepseek-ai/dsh-spill-policy' diff --git a/examples/acp-agent/tests/snapshots/bash-spill/session.jsonl b/examples/acp-agent/tests/snapshots/bash-spill/session.jsonl index 7c7fe12630..aa60d5143e 100644 --- a/examples/acp-agent/tests/snapshots/bash-spill/session.jsonl +++ b/examples/acp-agent/tests/snapshots/bash-spill/session.jsonl @@ -10,7 +10,7 @@ {"type":"assistant/chunk","seq":8,"time":0,"data":{"turn":1,"step":1,"chunk":{"type":"finish","reason":{"kind":"tool-calls"}}}} {"type":"assistant/message","seq":9,"time":0,"data":{"turn":1,"step":1,"content":[{"type":"tool-call","id":"call_spill","name":"bash","arguments":"{\"command\":\"node -e \\\"process.stdout.write('SPILL_START-' + 'x'.repeat(2000) + '-SPILL_END')\\\"\",\"description\":\"Print large deterministic output\"}"}],"usage":{"inputTokens":10,"outputTokens":5}},"sourceEventSeqs":[4,5,6,7,8],"surfaceOp":"append"} {"type":"tool/call","seq":10,"time":0,"data":{"turn":1,"step":1,"callId":"call_spill","name":"bash","arguments":"{\"command\":\"node -e \\\"process.stdout.write('SPILL_START-' + 'x'.repeat(2000) + '-SPILL_END')\\\"\",\"description\":\"Print large deterministic output\"}"}} -{"type":"tool/result","seq":11,"time":0,"data":{"turn":1,"step":1,"callId":"call_spill","content":[{"type":"text","text":"SPILL_START-xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx-SPILL_END\n\n(Omitted 1447 bytes. Full formatted result saved to: {{spillPath:bash.txt}}. Use read with offset/limit to inspect it.)"}],"isError":false},"sourceEventSeqs":[10],"surfaceOp":"append"} +{"type":"tool/result","seq":11,"time":0,"data":{"turn":1,"step":1,"callId":"call_spill","content":[{"type":"text","text":"SPILL_START-xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx-SPILL_END\n\n(Omitted 1391 bytes. Full formatted result saved to: {{spillPath:bash.txt}}. Use read with offset/limit to inspect it.)"}],"isError":false},"sourceEventSeqs":[10],"surfaceOp":"append"} {"type":"step/end","seq":12,"time":0,"data":{"turn":1,"step":1}} {"type":"step/start","seq":13,"time":0,"data":{"turn":1,"step":2}} {"type":"assistant/chunk","seq":14,"time":0,"data":{"turn":1,"step":2,"chunk":{"type":"block-start","index":0,"blockType":"text"}}} diff --git a/examples/acp-agent/tests/snapshots/bash-spill/stdout.golden.jsonl b/examples/acp-agent/tests/snapshots/bash-spill/stdout.golden.jsonl index dfd9c4fb2b..df47558232 100644 --- a/examples/acp-agent/tests/snapshots/bash-spill/stdout.golden.jsonl +++ b/examples/acp-agent/tests/snapshots/bash-spill/stdout.golden.jsonl @@ -1,6 +1,6 @@ {"jsonrpc":"2.0","id":1,"result":{"protocolVersion":1,"agentInfo":{"name":"deepseek-harness-acp","version":"0.0.1"},"agentCapabilities":{"loadSession":true,"promptCapabilities":{"image":false,"audio":false,"embeddedContext":false}},"authMethods":[]}} {"jsonrpc":"2.0","id":2,"result":{"sessionId":"{{sessionId}}"}} {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"tool_call","toolCallId":"call_spill","title":"node -e \"process.stdout.write('SPILL_START-' + 'x'.repeat(2000) + '-SPILL_END')\"","kind":"execute","status":"in_progress","rawInput":"node -e \"process.stdout.write('SPILL_START-' + 'x'.repeat(2000) + '-SPILL_END')\"","content":[{"type":"content","content":{"type":"text","text":"Print large deterministic output"}}]}}} -{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"tool_call_update","toolCallId":"call_spill","status":"completed","content":[{"type":"content","content":{"type":"text","text":"```console\nSPILL_START-xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx-SPILL_END\n\n(Omitted 1447 bytes. Full formatted result saved to: {{spillPath:bash.txt}}. Use read with offset/limit to inspect it.)\n```"}}]}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"tool_call_update","toolCallId":"call_spill","status":"completed","content":[{"type":"content","content":{"type":"text","text":"```console\nSPILL_START-xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx-SPILL_END\n\n(Omitted 1391 bytes. Full formatted result saved to: {{spillPath:bash.txt}}. Use read with offset/limit to inspect it.)\n```"}}]}}} {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_message_chunk","content":{"type":"text","text":"DONE"}}}} {"jsonrpc":"2.0","id":3,"result":{"stopReason":"end_turn"}} diff --git a/packages/support/acp-snapshot/src/harness.ts b/packages/support/acp-snapshot/src/harness.ts index 538b81d57e..befcf32dca 100644 --- a/packages/support/acp-snapshot/src/harness.ts +++ b/packages/support/acp-snapshot/src/harness.ts @@ -180,6 +180,9 @@ export interface RunOptions { export async function runScenario(input: InputScript, opts: RunOptions): Promise { const cwd = await mkdtemp(join(tmpdir(), 'acp-snap-cwd-')) const sessionsRoot = await mkdtemp(join(tmpdir(), 'acp-snap-sessions-')) + // Fixed path length: spill-policy budgets the preview against the REAL path + // before stdout normalization, so tmpdir() length differences churn goldens. + const spillRoot = '/tmp/dsh-acp-snapshot-spill' // Everything past the temp-dir creation runs under a try/finally that always // removes both dirs — so a failure in workspace seeding, spawn, or any step // never leaks them (the "e2e tests own their resources" rule). @@ -201,6 +204,7 @@ export async function runScenario(input: InputScript, opts: RunOptions): Promise DSH_SNAPSHOT: opts.mode, DSH_SNAPSHOT_FILE: opts.fixtureFile, DSH_SNAPSHOT_SESSIONS_ROOT: sessionsRoot, + DSH_SNAPSHOT_SPILL_ROOT: spillRoot, ...opts.overrideFile !== undefined ? { DSH_SNAPSHOT_OVERRIDE: opts.overrideFile } : {}, ...opts.childFiles !== undefined && opts.childFiles.length > 0 ? { DSH_SNAPSHOT_CHILD_FILES: opts.childFiles.join(delimiter) } @@ -310,6 +314,7 @@ export async function runScenario(input: InputScript, opts: RunOptions): Promise } await rm(cwd, { recursive: true, force: true }) await rm(sessionsRoot, { recursive: true, force: true }) + await rm(spillRoot, { recursive: true, force: true }) } return { diff --git a/packages/support/acp-snapshot/src/normalize.ts b/packages/support/acp-snapshot/src/normalize.ts index a4ce886f8b..e5b15a4abb 100644 --- a/packages/support/acp-snapshot/src/normalize.ts +++ b/packages/support/acp-snapshot/src/normalize.ts @@ -38,6 +38,11 @@ const LOCAL_SPILL_PATH_RE = new RegExp( + String.raw`(?=\. Use read with offset/limit|[\s)]|$)`, 'g', ) +const SNAPSHOT_SPILL_PATH_RE = new RegExp( + String.raw`/tmp/dsh-acp-snapshot-spill/session-[0-9a-f]{12}/[0-9a-f]{12}-([A-Za-z0-9._~-]+?)` + + String.raw`(?=\. Use read with offset/limit|[\s)]|$)`, + 'g', +) /** Inputs the normalizers need to recognize a run's volatile values. */ export interface NormalizeContext { @@ -55,6 +60,7 @@ function scrubString(value: string, ctx: NormalizeContext): string { out = out.split(ctx.cwd).join(CWD) out = out.split(`/private${CWD}`).join(CWD) out = out.replace(LOCAL_SPILL_PATH_RE, (_match, name: string) => `{{spillPath:${name}}}`) + out = out.replace(SNAPSHOT_SPILL_PATH_RE, (_match, name: string) => `{{spillPath:${name}}}`) for (const id of ctx.sessionIds) out = out.split(id).join(SESSION_ID) out = out.replace(UUID_RE, SESSION_ID) return out diff --git a/packages/support/acp-snapshot/tests/normalize.spec.ts b/packages/support/acp-snapshot/tests/normalize.spec.ts index a33436a164..ec388294bf 100644 --- a/packages/support/acp-snapshot/tests/normalize.spec.ts +++ b/packages/support/acp-snapshot/tests/normalize.spec.ts @@ -117,6 +117,21 @@ describe('normalizeSessionLog', () => { expect(out).not.toContain('/private{{spillPath') }) + it('scrubs fixed snapshot spill paths', () => { + const ev = JSON.stringify({ + type: 'tool/result', seq: 2, time: 5, + data: { + content: [{ + type: 'text', + text: 'Full formatted result saved to: /tmp/dsh-acp-snapshot-spill/session-c22bc3f1d2af/8a7b6c5d4e3f-bash.txt. Use read with offset/limit to inspect it.', + }], + }, + }) + const out = normalizeSessionLog(`${header({ cwd: ctx.cwd })}\n${ev}\n`, ctx) + expect(out).toContain('{{spillPath:bash.txt}}') + expect(out).not.toContain('/tmp/dsh-acp-snapshot-spill') + }) + it('scrubs the session id in the header', () => { const out = normalizeSessionLog(`${header({ id: ctx.sessionIds[0] })}\n`, ctx) expect(out).toContain('{{sessionId}}')