From 3fceefb4a6fc565ba8a479f26ac7f2ddaed550d9 Mon Sep 17 00:00:00 2001 From: t Date: Fri, 14 Aug 2026 12:30:35 +0800 Subject: [PATCH 1/2] feat(core): stop tools from flooding the model, and stop losing what they said MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two verified defects, one cause: nothing central bounds what a tool result puts into the model's context, so each tool improvises. WebFetch improvised by not bounding at all — `content: body`, capped only at 5 MiB of bytes upstream. A 5 MiB page is roughly 1.5M tokens; one call ends the session. Bash improvised by slicing at 30 KB and appending a marker, writing the remainder nowhere. The tail of a failing test run is exactly the part worth reading, and it was the part being discarded. The spill policy now runs on every tool result on its way to the model. Results at or under the threshold pass through untouched — identical object, no copy — so this is invisible for almost every call. Oversized results become a head+tail preview naming the file where the full text was saved, under the session directory beside its snapshots. Head AND tail, weighted toward the tail: stack traces, assertion diffs, and exit lines live at the end. Head-only truncation throws away the answer and keeps the preamble. Bash's capture buffer had to grow for there to be anything to save, so it now uses the same bounded head+tail primitive at a much larger limit. Memory stays bounded no matter how much a runaway command prints — a gigabyte costs the same as a kilobyte. Hosts without a filesystem (the Tauri renderer) still get the bound; they just cannot offer retrieval, and the preview says so rather than implying a file exists. Saving failures degrade the same way instead of failing the call. Co-Authored-By: Claude Opus 5 --- packages/core/src/agent.test.ts | 81 +++++++++++++++++ packages/core/src/agent.ts | 40 +++++++++ packages/core/src/index.ts | 15 ++++ packages/core/src/spill/bound.test.ts | 75 ++++++++++++++++ packages/core/src/spill/bound.ts | 117 +++++++++++++++++++++++++ packages/core/src/spill/index.ts | 15 ++++ packages/core/src/spill/local.ts | 50 +++++++++++ packages/core/src/spill/policy.test.ts | 113 ++++++++++++++++++++++++ packages/core/src/spill/policy.ts | 103 ++++++++++++++++++++++ packages/core/src/spill/types.ts | 50 +++++++++++ packages/core/src/tools/bash.ts | 40 +++++---- 11 files changed, 682 insertions(+), 17 deletions(-) create mode 100644 packages/core/src/spill/bound.test.ts create mode 100644 packages/core/src/spill/bound.ts create mode 100644 packages/core/src/spill/index.ts create mode 100644 packages/core/src/spill/local.ts create mode 100644 packages/core/src/spill/policy.test.ts create mode 100644 packages/core/src/spill/policy.ts create mode 100644 packages/core/src/spill/types.ts diff --git a/packages/core/src/agent.test.ts b/packages/core/src/agent.test.ts index 1d5a58c..c50705f 100644 --- a/packages/core/src/agent.test.ts +++ b/packages/core/src/agent.test.ts @@ -1205,4 +1205,85 @@ describe('runAgent', () => { expect(result.stopReason).toBe('end_turn'); }); }); + + describe('tool-output spill', () => { + const floodTool: ToolHandler = { + name: 'Flood', + definition: { + name: 'Flood', + description: 'returns a lot of text', + inputSchema: { type: 'object', properties: {} }, + }, + execute: () => Promise.resolve({ content: `HEAD${'.'.repeat(200_000)}TAIL` }), + }; + const floodCall = (): ToolUseBlock => ({ + type: 'tool_use', + id: 'call_flood', + name: 'Flood', + input: {}, + }); + + /** The tool result the loop actually handed back to the provider. */ + function resultText(history: StoredMessage[]): string { + for (const msg of history) { + for (const block of msg.content) { + if (typeof block !== 'string' && block.type === 'tool_result') return block.content; + } + } + expect.fail('no tool result in history'); + } + + it('bounds what a tool can put into the model context', async () => { + const result = await runAgent({ + provider: new MockProvider([toolUse('flooding', floodCall()), endTurn('done')]), + tools: new ToolRegistry([floodTool]), + systemPrompt: '', + userMessage: 'go', + model: 'deepseek-chat', + cwd, + spillThresholdChars: 1_000, + }); + + const text = resultText(result.history); + expect(text.length).toBeLessThan(2_000); + expect(text.startsWith('HEAD')).toBe(true); + expect(text.trimEnd().endsWith('TAIL')).toBe(true); + }); + + it('saves the omitted output where the model can read it back', async () => { + const manager = new SessionManager(sessionsRoot); + const session = await manager.create({ cwd }); + const result = await runAgent({ + provider: new MockProvider([toolUse('flooding', floodCall()), endTurn('done')]), + tools: new ToolRegistry([floodTool]), + systemPrompt: '', + userMessage: 'go', + model: 'deepseek-chat', + cwd, + session: { manager, id: session.id }, + spillThresholdChars: 1_000, + }); + + const text = resultText(result.history); + const match = /Full output saved to:\n(.+)\n/.exec(text); + expect(match).not.toBeNull(); + const saved = await fs.readFile((match as RegExpExecArray)[1], 'utf8'); + expect(saved.length).toBe(200_008); + expect(saved.startsWith('HEAD')).toBe(true); + expect(saved.endsWith('TAIL')).toBe(true); + }); + + it('says so plainly when there is nowhere to save it', async () => { + const result = await runAgent({ + provider: new MockProvider([toolUse('flooding', floodCall()), endTurn('done')]), + tools: new ToolRegistry([floodTool]), + systemPrompt: '', + userMessage: 'go', + model: 'deepseek-chat', + cwd, + spillThresholdChars: 1_000, + }); + expect(resultText(result.history)).toContain('was not saved'); + }); + }); }); diff --git a/packages/core/src/agent.ts b/packages/core/src/agent.ts index 8fd4605..e1712a6 100644 --- a/packages/core/src/agent.ts +++ b/packages/core/src/agent.ts @@ -13,6 +13,7 @@ import type { HookDispatcher } from './hooks/index.js'; import type { Mode } from './types.js'; import type { Provider } from './providers/types.js'; import { resolveRuntimePolicy } from './runtime/policy.js'; +import { applySpillPolicy, type SpillStore } from './spill/index.js'; // NOTE: reminders + sessions are lazy-loaded inside the loop so a browser // build (Tauri renderer) that doesn't use them avoids pulling node:fs at // module-load time. See `loadRemindersIfEnabled` and `appendSessionIfSet`. @@ -109,6 +110,14 @@ export interface RunAgentOptions { /** Inject system reminders before the user message (date, todos, etc). * Pass `false` to disable; pass a partial list to limit which builders run. */ systemReminders?: false | { enabled?: ReminderType[] }; + /** Model-visible ceiling per tool result, in code units. See `spill/policy.ts`. */ + spillThresholdChars?: number; + /** + * Where oversized tool output is persisted. Hosts with a session directory + * get the local file store by default; supplying one here overrides that, + * and it is the seam tests use. + */ + spillStore?: SpillStore; /** Host callback for AskUserQuestion tool. Optional — when absent the tool * errors. */ askUser?: NonNullable; @@ -302,6 +311,29 @@ export async function runAgent(opts: RunAgentOptions): Promise { modeSignal, }; + // Spill storage is resolved once, lazily: the local backend imports node:fs, + // which a renderer build has no answer for. A host without one still gets the + // bounded preview — it just cannot offer retrieval. + const resolveSpillStore = async (): Promise => { + if (opts.spillStore) return opts.spillStore; + const dir = toolCtx.sessionDir; + if (dir === undefined) return undefined; + try { + const mod = /* @vite-ignore */ './spill/local.js'; + const { createLocalSpillStore } = (await import(mod)) as typeof import('./spill/local.js'); + return createLocalSpillStore(dir); + } catch { + // No filesystem in this build — preview-only is the documented fallback. + return undefined; + } + }; + // Memoized on the promise, not its result: tools run concurrently, and a + // second caller arriving mid-import must wait for the same answer rather than + // observe "not resolved yet" as "no store". + let spillStorePending: Promise | undefined; + const spillStore = (): Promise => + (spillStorePending ??= resolveSpillStore()); + // Wire the Task tool's sub-agent runner — but only below the recursion cap, // so a sub-agent can't spawn further sub-agents (it also never gets the Task // tool, see the denylist below; this is belt-and-suspenders). @@ -773,6 +805,14 @@ export async function runAgent(opts: RunAgentOptions): Promise { tr = { content: `Error: ${(err as Error).message}`, isError: true }; } + // Every result passes the spill policy on its way to the model, so no + // single tool can flood the context regardless of what it returns. + tr = await applySpillPolicy(tr, { + source: { toolName: toolUse.name, callId: toolUse.id, label: 'result' }, + store: await spillStore(), + thresholdChars: opts.spillThresholdChars, + }); + // PostToolUse hook (M3) — observation only; can inject additionalContext if (opts.hooks) { await opts.hooks.dispatch({ diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index bc70ceb..6916e34 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -216,6 +216,21 @@ export { type CompactionResult, } from './compaction/index.js'; +// Tool-output spill — the central bound on model-visible tool output. +export { + applySpillPolicy, + boundText, + BoundedCapture, + DEFAULT_SPILL_THRESHOLD_CHARS, + type BoundedText, + type SaveTextRequest, + type SpillOutcome, + type SpillPolicyOptions, + type SpillRef, + type SpillSource, + type SpillStore, +} from './spill/index.js'; + // Agent loop's approval callback type (M3b) export type { ApprovalCallback, ApprovalDecision } from './agent.js'; diff --git a/packages/core/src/spill/bound.test.ts b/packages/core/src/spill/bound.test.ts new file mode 100644 index 0000000..86b15e6 --- /dev/null +++ b/packages/core/src/spill/bound.test.ts @@ -0,0 +1,75 @@ +import { describe, it, expect } from 'vitest'; +import { boundText, BoundedCapture } from './bound.js'; + +describe('boundText', () => { + it('returns the whole string when it fits', () => { + expect(boundText('hello', 10, 10)).toEqual({ head: 'hello', tail: '', omitted: 0 }); + }); + + it('keeps both ends and reports the gap', () => { + const r = boundText('abcdefghij', 3, 2); + expect(r.head).toBe('abc'); + expect(r.tail).toBe('ij'); + expect(r.omitted).toBe(5); + }); + + it('accounts for every character', () => { + const text = 'x'.repeat(1000); + const r = boundText(text, 100, 250); + expect(r.head.length + r.tail.length + r.omitted).toBe(text.length); + }); + + it('never splits a surrogate pair', () => { + // Each emoji is two UTF-16 code units; cutting at an odd index would leave + // a lone surrogate that renders as a replacement character. + const text = '😀'.repeat(20); + const r = boundText(text, 3, 3); + expect(r.head).toBe('😀'); + expect(r.tail).toBe('😀'); + expect([...r.head].every((c) => c === '😀')).toBe(true); + expect([...r.tail].every((c) => c === '😀')).toBe(true); + }); + + it('supports a zero-length tail', () => { + const r = boundText('abcdef', 2, 0); + expect(r).toEqual({ head: 'ab', tail: '', omitted: 4 }); + }); +}); + +describe('BoundedCapture', () => { + it('reproduces the input exactly when under the limits', () => { + const c = new BoundedCapture(10, 10); + c.push('abc'); + c.push('def'); + expect(c.text()).toBe('abcdef'); + expect(c.omitted).toBe(0); + expect(c.total).toBe(6); + }); + + it('keeps the head and the tail once it overflows', () => { + const c = new BoundedCapture(3, 3); + for (const ch of 'abcdefghijklmnop') c.push(ch); + expect(c.total).toBe(16); + expect(c.omitted).toBe(10); + const text = c.text(); + expect(text.startsWith('abc')).toBe(true); + expect(text.endsWith('nop')).toBe(true); + expect(text).toContain('10 characters not captured'); + }); + + it('bounds memory regardless of how much is pushed', () => { + const c = new BoundedCapture(100, 100); + for (let i = 0; i < 500; i++) c.push('y'.repeat(1000)); + expect(c.total).toBe(500_000); + // Retained text is the two ends plus one marker line, not the 500 KB pushed. + expect(c.text().length).toBeLessThan(400); + }); + + it('splits a chunk that straddles the head boundary', () => { + const c = new BoundedCapture(4, 4); + c.push('abcdefghij'); + expect(c.text().startsWith('abcd')).toBe(true); + expect(c.text().endsWith('ghij')).toBe(true); + expect(c.omitted).toBe(2); + }); +}); diff --git a/packages/core/src/spill/bound.ts b/packages/core/src/spill/bound.ts new file mode 100644 index 0000000..bfd884e --- /dev/null +++ b/packages/core/src/spill/bound.ts @@ -0,0 +1,117 @@ +// Head+tail bounding — the one primitive both the capture buffer and the +// model-visible preview are built from. +// +// Spec: docs/DSH_ADOPTION_PLAN.md §1.1 +// +// Why head AND tail: a stack trace, an assertion diff, and a non-zero exit line +// all live at the END of a command's output, which is exactly what head-only +// truncation throws away. Keeping both ends costs nothing and is the difference +// between a usable excerpt and a useless one. +// +// Units are UTF-16 code units (JS string length), not bytes. Byte counts are +// reported only for content actually written to disk. + +/** A string reduced to its two ends, plus how much was dropped between them. */ +export interface BoundedText { + /** The retained head. Empty when `headChars` is 0. */ + head: string; + /** The retained tail. Empty when nothing was dropped (all of it is in `head`). */ + tail: string; + /** Code units dropped between head and tail. 0 means `head + tail` is the whole input. */ + omitted: number; +} + +/** + * Trim a lone surrogate off the end of a slice, so cutting mid-pair can never + * emit an unpaired code unit. + */ +function trimEnd(s: string): string { + const last = s.charCodeAt(s.length - 1); + return last >= 0xd800 && last <= 0xdbff ? s.slice(0, -1) : s; +} + +/** Trim a lone low surrogate off the start of a slice, for the same reason. */ +function trimStart(s: string): string { + const first = s.charCodeAt(0); + return first >= 0xdc00 && first <= 0xdfff ? s.slice(1) : s; +} + +/** + * Reduce `text` to at most `headChars` from the front and `tailChars` from the + * back. Returns the whole string as `head` when it already fits. + * + * @param text Input string. + * @param headChars Maximum code units to keep from the front (>= 0). + * @param tailChars Maximum code units to keep from the back (>= 0). + * @returns The two retained ends and the count dropped between them. + */ +export function boundText(text: string, headChars: number, tailChars: number): BoundedText { + if (text.length <= headChars + tailChars) return { head: text, tail: '', omitted: 0 }; + const head = trimEnd(text.slice(0, headChars)); + const tail = tailChars > 0 ? trimStart(text.slice(text.length - tailChars)) : ''; + return { head, tail, omitted: text.length - head.length - tail.length }; +} + +/** + * Streaming head+tail buffer, for output that arrives in chunks and whose total + * size is not known in advance. + * + * Memory stays bounded at roughly `headChars + tailChars` code units no matter + * how much is pushed through it — a command that prints gigabytes costs the same + * as one that prints kilobytes. + */ +export class BoundedCapture { + #head = ''; + #tail = ''; + #total = 0; + + /** + * @param headChars Maximum code units retained from the front. + * @param tailChars Maximum code units retained from the back. + */ + constructor( + private readonly headChars: number, + private readonly tailChars: number, + ) {} + + /** + * Append a chunk, discarding from the middle as needed. + * + * @param chunk Text to append. + */ + push(chunk: string): void { + this.#total += chunk.length; + let rest = chunk; + if (this.#head.length < this.headChars) { + const room = this.headChars - this.#head.length; + this.#head += rest.slice(0, room); + rest = rest.slice(room); + } + if (rest.length === 0) return; + const merged = this.#tail + rest; + this.#tail = + merged.length > this.tailChars ? merged.slice(merged.length - this.tailChars) : merged; + } + + /** Code units pushed in total, including those since discarded. */ + get total(): number { + return this.#total; + } + + /** Code units discarded from the middle. */ + get omitted(): number { + return this.#total - this.#head.length - this.#tail.length; + } + + /** + * The retained text. When nothing was discarded this is the exact input; + * otherwise the two ends are joined by a marker naming the gap. + * + * @returns Retained text, with an inline marker when a gap exists. + */ + text(): string { + const gap = this.omitted; + if (gap <= 0) return this.#head + this.#tail; + return `${trimEnd(this.#head)}\n... [${gap.toLocaleString('en-US')} characters not captured — output exceeded the capture limit] ...\n${trimStart(this.#tail)}`; + } +} diff --git a/packages/core/src/spill/index.ts b/packages/core/src/spill/index.ts new file mode 100644 index 0000000..eb97d9a --- /dev/null +++ b/packages/core/src/spill/index.ts @@ -0,0 +1,15 @@ +// Tool-output spill — entry point. +// Spec: docs/DSH_ADOPTION_PLAN.md §1.1 +// +// `local.js` is deliberately NOT re-exported here: it imports node:fs, and this +// module is reachable from the renderer bundle. Hosts with a filesystem import +// it directly. + +export { boundText, BoundedCapture, type BoundedText } from './bound.js'; +export { + applySpillPolicy, + DEFAULT_SPILL_THRESHOLD_CHARS, + type SpillPolicyOptions, + type SpillOutcome, +} from './policy.js'; +export type { SaveTextRequest, SpillRef, SpillSource, SpillStore } from './types.js'; diff --git a/packages/core/src/spill/local.ts b/packages/core/src/spill/local.ts new file mode 100644 index 0000000..7cd7371 --- /dev/null +++ b/packages/core/src/spill/local.ts @@ -0,0 +1,50 @@ +// Local spill backend — session-scoped files under `/spill/`. +// Spec: docs/DSH_ADOPTION_PLAN.md §1.1 +// +// Spilled output lives beside the session's snapshots so both are covered by +// whatever retention policy the session directory eventually gets, rather than +// accumulating somewhere nobody thinks to clean. + +import { promises as fs } from 'node:fs'; +import { join } from 'node:path'; +import type { SaveTextRequest, SpillRef, SpillStore } from './types.js'; + +/** Reduce an arbitrary label to one safe path segment. */ +function segment(s: string): string { + const cleaned = s.replace(/[^A-Za-z0-9._-]+/g, '_').replace(/^[._]+/, ''); + return cleaned.slice(0, 64) || 'spill'; +} + +/** + * Create a spill store writing under `/spill/`. + * + * @param dir Session directory. Created on first save. + * @returns A store that persists text as UTF-8 files. + */ +export function createLocalSpillStore(dir: string): SpillStore { + const root = join(dir, 'spill'); + return { + async saveText(req: SaveTextRequest): Promise { + await fs.mkdir(root, { recursive: true }); + const base = `${segment(req.source.toolName)}-${segment(req.source.callId)}-${segment(req.source.label)}`; + // `wx` rather than a plain write: sanitizing can collapse two distinct + // call ids onto one name, and silently overwriting the earlier artifact + // would hand the model a locator pointing at someone else's output. + let path = join(root, `${base}.txt`); + for (let n = 2; ; n++) { + try { + await fs.writeFile(path, req.content, { encoding: 'utf8', flag: 'wx' }); + break; + } catch (err) { + if ((err as NodeJS.ErrnoException).code !== 'EEXIST') throw err; + path = join(root, `${base}-${n}.txt`); + } + } + return { + locator: path, + bytes: Buffer.byteLength(req.content, 'utf8'), + retrievalHint: 'Read that file to see it in full; use offset/limit to page through it.', + }; + }, + }; +} diff --git a/packages/core/src/spill/policy.test.ts b/packages/core/src/spill/policy.test.ts new file mode 100644 index 0000000..c9b98e7 --- /dev/null +++ b/packages/core/src/spill/policy.test.ts @@ -0,0 +1,113 @@ +import { describe, it, expect } from 'vitest'; +import { mkdtemp, readFile, readdir } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { applySpillPolicy } from './policy.js'; +import { createLocalSpillStore } from './local.js'; +import type { SpillOutcome } from './policy.js'; +import type { SpillSource, SpillStore } from './types.js'; + +const source: SpillSource = { toolName: 'Bash', callId: 'toolu_01', label: 'result' }; + +function outcomeOf(data: Record | undefined): SpillOutcome { + return data?.['spill'] as SpillOutcome; +} + +describe('applySpillPolicy', () => { + it('leaves a result that fits completely untouched', async () => { + const result = { content: 'small', data: { exitCode: 0 } }; + const out = await applySpillPolicy(result, { source, thresholdChars: 100 }); + expect(out).toBe(result); + }); + + it('leaves a result exactly at the threshold untouched', async () => { + const result = { content: 'x'.repeat(100) }; + expect(await applySpillPolicy(result, { source, thresholdChars: 100 })).toBe(result); + }); + + it('bounds the model-visible content even with no store', async () => { + const result = { content: 'x'.repeat(10_000) }; + const out = await applySpillPolicy(result, { source, thresholdChars: 200 }); + // Threshold plus the fixed marker — the point is that 10 KB does not reach + // the model, not that the marker is free. + expect(out.content.length).toBeLessThan(600); + expect(out.content).toContain('was not saved'); + expect(outcomeOf(out.data).locator).toBeUndefined(); + expect(outcomeOf(out.data).unsavedReason).toContain('no session directory'); + }); + + it('keeps both the head and the tail of the original', async () => { + const content = `START${'-'.repeat(5000)}END`; + const out = await applySpillPolicy({ content }, { source, thresholdChars: 200 }); + expect(out.content.startsWith('START')).toBe(true); + expect(out.content.trimEnd().endsWith('END')).toBe(true); + }); + + it('gives the tail more room than the head', async () => { + // A failing command's useful output is at the end, so the split is not even. + const out = await applySpillPolicy( + { content: 'x'.repeat(10_000) }, + { source, thresholdChars: 1000 }, + ); + const [head, tail] = out.content.split(/\n\n\.\.\. \[.*?\] \.\.\.\n\n/s); + expect(head.length).toBeLessThan(tail.length); + }); + + it('preserves the tool result fields it does not own', async () => { + const out = await applySpillPolicy( + { content: 'x'.repeat(500), isError: true, data: { exitCode: 3 } }, + { source, thresholdChars: 100 }, + ); + expect(out.isError).toBe(true); + expect(out.data?.['exitCode']).toBe(3); + }); + + it('saves the full text and points the model at it', async () => { + const dir = await mkdtemp(join(tmpdir(), 'spill-')); + const content = `START${'-'.repeat(5000)}END`; + const out = await applySpillPolicy( + { content }, + { source, thresholdChars: 200, store: createLocalSpillStore(dir) }, + ); + + const locator = outcomeOf(out.data).locator; + expect(locator).toBeDefined(); + expect(out.content).toContain(locator as string); + expect(out.content).toContain('Read that file'); + expect(await readFile(locator as string, 'utf8')).toBe(content); + expect(outcomeOf(out.data).originalChars).toBe(content.length); + }); + + it('degrades to a preview when saving fails', async () => { + const store: SpillStore = { + saveText: () => Promise.reject(new Error('disk full')), + }; + const out = await applySpillPolicy( + { content: 'x'.repeat(500) }, + { source, thresholdChars: 100, store }, + ); + expect(out.content).toContain('disk full'); + expect(outcomeOf(out.data).locator).toBeUndefined(); + }); +}); + +describe('createLocalSpillStore', () => { + it('reports the byte length, not the character count', async () => { + const dir = await mkdtemp(join(tmpdir(), 'spill-')); + const ref = await createLocalSpillStore(dir).saveText({ source, content: '😀' }); + expect(ref.bytes).toBe(4); + }); + + it('does not overwrite an artifact whose name collides', async () => { + const dir = await mkdtemp(join(tmpdir(), 'spill-')); + const store = createLocalSpillStore(dir); + // Sanitizing maps both of these call ids onto the same base name. + const a = await store.saveText({ source: { ...source, callId: 'a/b' }, content: 'first' }); + const b = await store.saveText({ source: { ...source, callId: 'a:b' }, content: 'second' }); + + expect(a.locator).not.toBe(b.locator); + expect(await readFile(a.locator, 'utf8')).toBe('first'); + expect(await readFile(b.locator, 'utf8')).toBe('second'); + expect((await readdir(join(dir, 'spill'))).length).toBe(2); + }); +}); diff --git a/packages/core/src/spill/policy.ts b/packages/core/src/spill/policy.ts new file mode 100644 index 0000000..8f21f7d --- /dev/null +++ b/packages/core/src/spill/policy.ts @@ -0,0 +1,103 @@ +// Tool-output spill policy — the single place that bounds model-visible tool output. +// Spec: docs/DSH_ADOPTION_PLAN.md §1.1 +// +// Every tool result passes through here on its way to the model. Results at or +// under the threshold are returned untouched, so this is invisible for the +// overwhelming majority of calls. Oversized results are replaced by a head+tail +// preview naming where the full text went. +// +// The invariant worth stating: after this runs, no tool can put more than +// `thresholdChars` (plus a fixed-size marker) into the model's context. That is +// a stronger guarantee than each tool policing itself, which is what let +// WebFetch return a 5 MiB response body verbatim. + +import type { ToolResult } from '../types.js'; +import { boundText } from './bound.js'; +import type { SpillSource, SpillStore } from './types.js'; + +/** Default model-visible ceiling, matching the cap Bash has always applied per stream. */ +export const DEFAULT_SPILL_THRESHOLD_CHARS = 30_000; + +/** Share of the preview budget given to the head; the rest goes to the tail. */ +const HEAD_SHARE = 0.4; + +export interface SpillPolicyOptions { + /** Which call produced this result. */ + source: SpillSource; + /** Where to persist oversized text. Omitted when the host cannot persist. */ + store?: SpillStore; + /** Model-visible ceiling in code units. Defaults to {@link DEFAULT_SPILL_THRESHOLD_CHARS}. */ + thresholdChars?: number; +} + +/** What the policy did, recorded on `ToolResult.data.spill` for the UI. */ +export interface SpillOutcome { + /** Code units in the original result content. */ + originalChars: number; + /** Code units replaced by the marker. */ + omittedChars: number; + /** Absent when the content could not be persisted. */ + locator?: string; + /** Why persistence did not happen, when it did not. */ + unsavedReason?: string; +} + +function marker(omitted: number, locator: string | undefined, hint: string | undefined): string { + const omittedText = `${omitted.toLocaleString('en-US')} characters omitted`; + if (locator === undefined) { + return `\n\n... [${omittedText}. The full output was not saved — ${hint}. Re-run with a narrower scope (a filter, a smaller range, or head/tail) to see the middle.] ...\n\n`; + } + return `\n\n... [${omittedText}. Full output saved to:\n${locator}\n${hint}] ...\n\n`; +} + +/** + * Bound a tool result to the model-visible ceiling, persisting the full text + * when a store is available. + * + * Persistence failures are not propagated: a result the model can read in part + * beats an error, so a failed save degrades to the same preview with the reason + * stated inline. + * + * @param result The tool's own result. + * @param opts Source attribution, optional store, and threshold. + * @returns The original result when it fits, otherwise a preview-bearing copy. + */ +export async function applySpillPolicy( + result: ToolResult, + opts: SpillPolicyOptions, +): Promise { + const threshold = opts.thresholdChars ?? DEFAULT_SPILL_THRESHOLD_CHARS; + const content = result.content; + if (content.length <= threshold) return result; + + const headChars = Math.floor(threshold * HEAD_SHARE); + const { head, tail, omitted } = boundText(content, headChars, threshold - headChars); + + let locator: string | undefined; + let hint = 'no session directory is available for this run'; + let unsavedReason: string | undefined = hint; + if (opts.store) { + try { + const ref = await opts.store.saveText({ source: opts.source, content }); + locator = ref.locator; + hint = ref.retrievalHint; + unsavedReason = undefined; + } catch (err) { + hint = `saving it failed: ${(err as Error).message}`; + unsavedReason = hint; + } + } + + const outcome: SpillOutcome = { + originalChars: content.length, + omittedChars: omitted, + ...(locator !== undefined ? { locator } : {}), + ...(unsavedReason !== undefined ? { unsavedReason } : {}), + }; + + return { + ...result, + content: head + marker(omitted, locator, hint) + tail, + data: { ...result.data, spill: outcome }, + }; +} diff --git a/packages/core/src/spill/types.ts b/packages/core/src/spill/types.ts new file mode 100644 index 0000000..4bb8af8 --- /dev/null +++ b/packages/core/src/spill/types.ts @@ -0,0 +1,50 @@ +// Tool-output spill — storage contract. +// Spec: docs/DSH_ADOPTION_PLAN.md §1.1 + +/** Tool and call that produced a spilled artifact. Descriptive only — never consulted for access control. */ +export interface SpillSource { + /** The tool whose result was spilled (e.g. `Bash`). */ + toolName: string; + /** The model-issued call id the result belongs to. */ + callId: string; + /** Short human label for the artifact (e.g. `result`). */ + label: string; +} + +/** One request to persist a tool result that was too large to show in full. */ +export interface SaveTextRequest { + source: SpillSource; + /** The full text to persist. */ + content: string; +} + +/** A saved artifact: where it went, how big it is, and how to get it back. */ +export interface SpillRef { + /** + * Opaque model-facing handle. The local backend renders it as an absolute + * file path; another backend could render a URI or key, so consumers show it + * alongside `retrievalHint` rather than assuming `Read` always applies. + */ + locator: string; + /** Size of the persisted content in bytes. */ + bytes: number; + /** Backend-specific instruction for retrieving the content. */ + retrievalHint: string; +} + +/** + * Storage for oversized tool output. + * + * A host that cannot persist (no session directory, or a renderer with no + * filesystem) simply supplies no store — the policy still bounds what the model + * sees, it just cannot offer retrieval. + */ +export interface SpillStore { + /** + * Persist text verbatim and return its locator. + * + * @param req The content to persist and the call that produced it. + * @returns The saved artifact's locator, size, and retrieval hint. + */ + saveText(req: SaveTextRequest): Promise; +} diff --git a/packages/core/src/tools/bash.ts b/packages/core/src/tools/bash.ts index 9b58ec7..a9c467a 100644 --- a/packages/core/src/tools/bash.ts +++ b/packages/core/src/tools/bash.ts @@ -19,6 +19,7 @@ import { } from '../sandbox/index.js'; import type { NetworkSandboxHandle, SpawnNetworkSandboxOpts } from '../sandbox/index.js'; import type { SandboxConfig, SandboxMode } from '../config/types.js'; +import { BoundedCapture } from '../spill/bound.js'; import type { ToolContext, ToolHandler, ToolResult } from '../types.js'; interface BashInput { @@ -40,29 +41,34 @@ type SandboxCtx = ToolContext & { }; const DEFAULT_TIMEOUT_MS = 120_000; // 2 minutes -const MAX_OUTPUT_BYTES = 30_000; +// What one stream keeps in memory. This is NOT the model-visible limit — the +// spill policy bounds that centrally and saves the rest to a file the model can +// read. Capture has to exceed that limit for there to be anything worth saving, +// while staying small enough that a runaway command cannot exhaust memory. +const CAPTURE_HEAD_CHARS = 1_000_000; +const CAPTURE_TAIL_CHARS = 3_000_000; type TerminationReason = 'timeout' | 'aborted'; // Monotonic suffix so two background spawns in the same millisecond from the // same pid don't collide on a log filename. let bgSeq = 0; -function capStream(s: string, label: string): string { - return s.length > MAX_OUTPUT_BYTES - ? s.slice(0, MAX_OUTPUT_BYTES) + `\n... [${label} truncated]` - : s; +function newCapture(): BoundedCapture { + return new BoundedCapture(CAPTURE_HEAD_CHARS, CAPTURE_TAIL_CHARS); } /** Build the standard Bash ToolResult from captured output + exit info. */ function summarize( - stdout: string, - stderr: string, + out: BoundedCapture, + err: BoundedCapture, terminationReason: TerminationReason | undefined, code: number | null, timeoutMs: number, note?: string, ): ToolResult { const parts: string[] = []; + const stdout = out.text(); + const stderr = err.text(); if (note) parts.push(note); if (stdout) parts.push(`\n${stdout}\n`); if (stderr) parts.push(`\n${stderr}\n`); @@ -75,8 +81,8 @@ function summarize( exitCode: code, killed: terminationReason !== undefined, terminationReason, - stdoutBytes: stdout.length, - stderrBytes: stderr.length, + stdoutBytes: out.total, + stderrBytes: err.total, }, isError: terminationReason !== undefined || (code !== null && code !== 0), }; @@ -112,8 +118,8 @@ async function runForegroundNet( ): Promise { const handle = await spawnFn({ userCommand: command, cwd: ctx.cwd, config }); return new Promise((resolve) => { - let stdout = ''; - let stderr = ''; + const stdout = newCapture(); + const stderr = newCapture(); let terminationReason: TerminationReason | undefined; let settled = false; const finish = (r: ToolResult): void => { @@ -132,10 +138,10 @@ async function runForegroundNet( }; ctx.signal?.addEventListener('abort', onAbort, { once: true }); handle.child.stdout?.on('data', (c: Buffer) => { - stdout = capStream(stdout + c.toString('utf8'), 'stdout'); + stdout.push(c.toString('utf8')); }); handle.child.stderr?.on('data', (c: Buffer) => { - stderr = capStream(stderr + c.toString('utf8'), 'stderr'); + stderr.push(c.toString('utf8')); }); handle.exited .then((code) => { @@ -277,8 +283,8 @@ export const BashTool: ToolHandler = { cwd: ctx.cwd, detached: process.platform !== 'win32', }); - let stdout = ''; - let stderr = ''; + const stdout = newCapture(); + const stderr = newCapture(); let terminationReason: TerminationReason | undefined; let settled = false; const finish = (result: ToolResult): void => { @@ -304,10 +310,10 @@ export const BashTool: ToolHandler = { ctx.signal?.addEventListener('abort', onAbort, { once: true }); child.stdout.on('data', (chunk: Buffer) => { - stdout = capStream(stdout + chunk.toString('utf8'), 'stdout'); + stdout.push(chunk.toString('utf8')); }); child.stderr.on('data', (chunk: Buffer) => { - stderr = capStream(stderr + chunk.toString('utf8'), 'stderr'); + stderr.push(chunk.toString('utf8')); }); child.on('error', (err) => { From 69a16f442b997f4db46aef6f9134f7df46790077 Mon Sep 17 00:00:00 2001 From: t Date: Fri, 14 Aug 2026 12:50:21 +0800 Subject: [PATCH 2/2] test(core): keep the spill test inside its temp directory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `new SessionManager(sessionsRoot)` passes a string where the constructor wants `{ root }`, so `opts.root` was undefined and the manager silently fell back to the real `~/.deepcode/sessions`. The test still passed — it reads the file back through the locator it was handed — while writing a session and a spill artifact into the user's actual home directory on every run. Co-Authored-By: Claude Opus 5 --- packages/core/src/agent.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/src/agent.test.ts b/packages/core/src/agent.test.ts index c50705f..a53b8ee 100644 --- a/packages/core/src/agent.test.ts +++ b/packages/core/src/agent.test.ts @@ -1251,8 +1251,8 @@ describe('runAgent', () => { }); it('saves the omitted output where the model can read it back', async () => { - const manager = new SessionManager(sessionsRoot); - const session = await manager.create({ cwd }); + const manager = new SessionManager({ root: sessionsRoot }); + const session = await manager.create(cwd); const result = await runAgent({ provider: new MockProvider([toolUse('flooding', floodCall()), endTurn('done')]), tools: new ToolRegistry([floodTool]),