fix(review): generalize spill storage locators
This commit is contained in:
@@ -1,13 +1,13 @@
|
||||
# @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.
|
||||
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.spillStore` 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
|
||||
await ctx.plugin(LocalSpillStore) // @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.
|
||||
@@ -22,8 +22,8 @@ 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. |
|
||||
| `globMaxResults` | `100` | Max paths one `glob` call retains inline (matches Claude Code's `GlobTool` limit); later paths go to the formatted spill artifact. |
|
||||
| `grepMaxMatches` | `250` | Max flat matches one `grep` call retains inline (matches Claude Code's `GrepTool` `head_limit`); later matches go to the formatted spill artifact. |
|
||||
| `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. |
|
||||
@@ -35,11 +35,11 @@ All keys are optional; the defaults are the shipped search caps.
|
||||
| `glob` | `pattern`, `path?` | `rg --files --glob <pattern> --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: <preview>`. |
|
||||
|
||||
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`.
|
||||
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 follows the returned spill locator's retrieval hint.
|
||||
|
||||
## Two budgets, two artifacts
|
||||
|
||||
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`.
|
||||
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.spillStore.saveText()` (suggested names `glob-results.txt` / `grep-results.txt`, owner = the calling session, source = the tool execution identity) and appends a footer naming the returned locator and retrieval hint. 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
|
||||
|
||||
|
||||
@@ -15,6 +15,7 @@ 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 { SpillRef } from '@deepseek-ai/dsh-spill'
|
||||
import type {} from '@deepseek-ai/dsh-bash'
|
||||
import type {} from '@deepseek-ai/dsh-system-prompt'
|
||||
import { runRipgrep, toWorkdirRelative, trySaveFormattedResult } from './search-core.ts'
|
||||
@@ -100,18 +101,18 @@ export function buildGlobCommand(input: GlobInput): string {
|
||||
/**
|
||||
* 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:
|
||||
* locator 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.
|
||||
* @param spillRef - the saved complete-result reference, or `undefined` when unsaved.
|
||||
* @returns the model-facing text.
|
||||
*/
|
||||
export function formatGlobOutput(retained: RetainedItems<string>, spillPath: string | undefined): string {
|
||||
export function formatGlobOutput(retained: RetainedItems<string>, spillRef: SpillRef | 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.`
|
||||
const recovery = spillRef !== undefined
|
||||
? `Full sorted result stored at: ${spillRef.locator}. ${spillRef.retrievalHint}`
|
||||
: '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})`
|
||||
}
|
||||
@@ -168,10 +169,10 @@ export function applyGlobTool(ctx: Context, caps: GlobToolCaps): void {
|
||||
|
||||
// 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
|
||||
const spillRef = retained.truncated
|
||||
? await trySaveFormattedResult(ctx, exec, 'glob-results.txt', all.join('\n'))
|
||||
: undefined
|
||||
return [{ type: 'text', text: formatGlobOutput(retained, spillPath) }]
|
||||
return [{ type: 'text', text: formatGlobOutput(retained, spillRef) }]
|
||||
},
|
||||
presentCall: presentGlobCall,
|
||||
}))
|
||||
|
||||
@@ -16,6 +16,7 @@ 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 { SpillRef } from '@deepseek-ai/dsh-spill'
|
||||
import type {} from '@deepseek-ai/dsh-bash'
|
||||
import type {} from '@deepseek-ai/dsh-system-prompt'
|
||||
import { SearchError, runRipgrep, toWorkdirRelative, trySaveFormattedResult } from './search-core.ts'
|
||||
@@ -221,21 +222,21 @@ export function formatGrepMatches(matches: GrepMatch[]): string {
|
||||
/**
|
||||
* 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
|
||||
* carrying either the formatted-spill recovery locator 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.
|
||||
* @param spillRef - the saved complete-result reference, or `undefined` when unsaved.
|
||||
* @returns the model-facing text.
|
||||
*/
|
||||
export function formatGrepOutput(retained: RetainedItems<GrepMatch>, spillPath: string | undefined): string {
|
||||
export function formatGrepOutput(retained: RetainedItems<GrepMatch>, spillRef: SpillRef | 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.`
|
||||
const recovery = spillRef !== undefined
|
||||
? `Full grep result stored at: ${spillRef.locator}. ${spillRef.retrievalHint}`
|
||||
: 'The complete result could not be saved; narrow pattern, path, or include to see more.'
|
||||
return `${header}\n\n${body}\n\n(${recovery})`
|
||||
}
|
||||
@@ -299,7 +300,7 @@ export function applyGrepTool(ctx: Context, caps: GrepToolCaps): void {
|
||||
// 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
|
||||
const spillRef = retained.truncated
|
||||
? await trySaveFormattedResult(
|
||||
ctx,
|
||||
exec,
|
||||
@@ -307,7 +308,7 @@ export function applyGrepTool(ctx: Context, caps: GrepToolCaps): void {
|
||||
`Found ${all.length} ${matchNoun(all.length)}\n\n${formatGrepMatches(all)}`,
|
||||
)
|
||||
: undefined
|
||||
return [{ type: 'text', text: formatGrepOutput(retained, spillPath) }]
|
||||
return [{ type: 'text', text: formatGrepOutput(retained, spillRef) }]
|
||||
},
|
||||
presentCall: presentGrepCall,
|
||||
}))
|
||||
|
||||
@@ -13,7 +13,7 @@
|
||||
* 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
|
||||
* `bash` — deliberately NOT `fs`, and `ctx.spillStore` is read opportunistically
|
||||
* with `ctx.get()` because formatted-result spill is optional.
|
||||
*
|
||||
* Returned paths are displayed relative to the resolved bash workdir and are
|
||||
@@ -52,7 +52,7 @@ 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()`). */
|
||||
/** Services required by the search tool suite (`spillStore` is optional, read via `ctx.get()`). */
|
||||
export const inject = ['tools', 'systemPrompt', 'bash']
|
||||
|
||||
/** Plugin config (all optional — `Config` supplies the defaults). */
|
||||
|
||||
@@ -10,7 +10,7 @@
|
||||
* 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()`
|
||||
* formatted result saved through `ctx.spillStore.saveText()`
|
||||
* ({@link trySaveFormattedResult}).
|
||||
*
|
||||
* @module @deepseek-ai/dsh-tool-fs-search/search-core
|
||||
@@ -20,7 +20,7 @@ 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 { SaveTextSpill, SpillRef } from '@deepseek-ai/dsh-spill'
|
||||
import type { ToolExecution } from '@deepseek-ai/dsh-tools'
|
||||
|
||||
/**
|
||||
@@ -214,8 +214,8 @@ export function toWorkdirRelative(path: string, workdir: string): string {
|
||||
|
||||
/**
|
||||
* 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
|
||||
* `ctx.spillStore.saveText()` — the model-facing recovery path for a capped
|
||||
* result. `spillStore` 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
|
||||
@@ -223,26 +223,26 @@ export function toWorkdirRelative(path: string, workdir: string): string {
|
||||
* 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 ctx - the plugin context; `spillStore` 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.
|
||||
* @returns the saved spill reference, or `undefined` when the result could not be saved.
|
||||
*/
|
||||
export async function trySaveFormattedResult(
|
||||
ctx: Context,
|
||||
exec: ToolExecution,
|
||||
suggestedName: string,
|
||||
content: string,
|
||||
): Promise<string | undefined> {
|
||||
): Promise<SpillRef | undefined> {
|
||||
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`)
|
||||
const spillStore = ctx.get('spillStore')
|
||||
if (!spillStore) {
|
||||
ctx.logger.warn(`tool-fs-search: no ctx.spillStore backend loaded; complete ${exec.name} result not saved`)
|
||||
return undefined
|
||||
}
|
||||
const save: SaveTextSpill = {
|
||||
@@ -252,8 +252,7 @@ export async function trySaveFormattedResult(
|
||||
content,
|
||||
}
|
||||
try {
|
||||
const { path } = await spillFiles.saveText(save)
|
||||
return path
|
||||
return await spillStore.saveText(save)
|
||||
} 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.
|
||||
|
||||
@@ -17,7 +17,7 @@ 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 { SpillLocator, SpillStore } 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 {
|
||||
@@ -95,14 +95,18 @@ class FakeBash extends BashExecutor {
|
||||
}
|
||||
|
||||
/** A recording spill backend; arm `failWith` to script a storage failure. */
|
||||
class FakeSpill extends SpillFiles {
|
||||
class FakeSpill extends SpillStore {
|
||||
saves: SaveTextSpill[] = []
|
||||
failWith?: Error
|
||||
|
||||
override saveText(input: SaveTextSpill): Promise<SpillRef> {
|
||||
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') })
|
||||
return Promise.resolve({
|
||||
locator: SpillLocator(`/spill/${input.suggestedName}`),
|
||||
bytes: Buffer.byteLength(input.content, 'utf8'),
|
||||
retrievalHint: 'Use the fake retrieval hint.',
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -119,7 +123,7 @@ async function setup(options: SetupOptions = {}) {
|
||||
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
|
||||
const spill = options.spill === true ? ctx.get('spillStore') as FakeSpill : undefined
|
||||
return { ctx, bash, spill, fiber }
|
||||
}
|
||||
|
||||
@@ -445,12 +449,12 @@ describe('glob results', () => {
|
||||
expect(bash.specs[0]?.command).toContain("-- 'sub'")
|
||||
})
|
||||
|
||||
it('caps at globMaxResults and saves the FULL sorted list through spillFiles', async () => {
|
||||
it('caps at globMaxResults and saves the FULL sorted list through spillStore', 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(text(result)).toBe('a.ts\nb.ts\n\n(Showing 2 of 4 paths. Full sorted result stored at: /spill/glob-results.txt. Use the fake retrieval hint.)')
|
||||
expect(spill?.saves).toHaveLength(1)
|
||||
expect(spill?.saves[0]).toMatchObject({
|
||||
owner: { sessionId: 'session-1' },
|
||||
@@ -543,7 +547,7 @@ describe('grep results', () => {
|
||||
'',
|
||||
].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(text(result)).toBe('Found 2 of 3 matches\n\na.ts\nLine 1: one\nLine 2: two\n\n(Full grep result stored at: /spill/grep-results.txt. Use the fake retrieval hint.)')
|
||||
expect(spill?.saves[0]).toMatchObject({
|
||||
source: { toolName: 'grep', label: 'result' },
|
||||
suggestedName: 'grep-results.txt',
|
||||
|
||||
Reference in New Issue
Block a user