From e94305d99ebcee7c93208b3e347b4ee791ada0b2 Mon Sep 17 00:00:00 2001 From: Dudu-0223 Date: Thu, 9 Jul 2026 21:12:41 +0800 Subject: [PATCH] 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()