diff --git a/docs/config-catalog.md b/docs/config-catalog.md index 86a20c52eb..089a644b90 100644 --- a/docs/config-catalog.md +++ b/docs/config-catalog.md @@ -145,7 +145,7 @@ export interface Config { } ``` -Source: [`packages/bash/bash-local/src/index.ts:21`](../packages/bash/bash-local/src/index.ts) +Source: [`packages/bash/bash-local/src/index.ts:18`](../packages/bash/bash-local/src/index.ts) ## `@deepseek-ai/dsh-bash-sandbox` @@ -831,7 +831,7 @@ export interface Config { } ``` -Source: [`packages/bash/tool-bash/src/index.ts:39`](../packages/bash/tool-bash/src/index.ts) +Source: [`packages/bash/tool-bash/src/index.ts:40`](../packages/bash/tool-bash/src/index.ts) ## `@deepseek-ai/dsh-tool-cordis` diff --git a/docs/cordis-catalog/services.md b/docs/cordis-catalog/services.md index 590ab4566d..c6468f11f3 100644 --- a/docs/cordis-catalog/services.md +++ b/docs/cordis-catalog/services.md @@ -84,7 +84,7 @@ list(): BashEnvVariableInfo[] Types: [ToolExecution](../core-data-structures/tools.md) -Source: [`packages/bash/tool-bash/src/index.ts:99`](../../packages/bash/tool-bash/src/index.ts) +Source: [`packages/bash/tool-bash/src/index.ts:100`](../../packages/bash/tool-bash/src/index.ts) ## `ctx.codeRuntime` — `CodeRuntime` (abstract seam) diff --git a/docs/core-data-structures/bash.md b/docs/core-data-structures/bash.md index 80697356f9..640e078b77 100644 --- a/docs/core-data-structures/bash.md +++ b/docs/core-data-structures/bash.md @@ -222,7 +222,6 @@ A long-running command started with `start()` is tracked as a `BashTask`. `BashT ```ts type-equiv interface BashTask { readonly id: BashTaskId - readonly command: string status: BashTaskStatus /** Exit code once finished (null = killed by signal / still running). */ exitCode: number | null diff --git a/packages/bash/bash-local/README.md b/packages/bash/bash-local/README.md index 317bc2e4c7..8b96cd7c2e 100644 --- a/packages/bash/bash-local/README.md +++ b/packages/bash/bash-local/README.md @@ -2,6 +2,8 @@ Local-subprocess implementation of the `@deepseek-ai/dsh-bash` executor seam: `LocalBashExecutor` spawns `bash -c ` per call in its own process group, collects bounded output with full-stream spill files, and escalates kills SIGTERM→SIGKILL across the whole group. +The package root exports the default and named `LocalBashExecutor` plugin plus its `Config`; subprocess plumbing stays internal to the implementation package. + ## Config ```yaml diff --git a/packages/bash/bash-local/src/index.ts b/packages/bash/bash-local/src/index.ts index 7fdc75a7b0..40c0977c71 100644 --- a/packages/bash/bash-local/src/index.ts +++ b/packages/bash/bash-local/src/index.ts @@ -14,9 +14,6 @@ import { clampTimeout, deadline, timeoutOf } from '@deepseek-ai/dsh-timeout' import { DEFAULT_GRACE_MS, runBash } from './run.ts' import type { RunInternals, RunningBash } from './run.ts' -export { DEFAULT_GRACE_MS, ENV_OVERRIDES, killGroup, OutputCollector, runBash } from './run.ts' -export type { RunInternals, RunningBash, SpawnOutcome, SpawnSpec } from './run.ts' - /** Plugin config (all optional — `static Config` supplies the defaults). */ export interface Config { /** Default working directory for commands (default: process.cwd()). */ @@ -173,7 +170,6 @@ export class LocalBashExecutor extends BashExecutor { const id = BashTaskId(`bash-${this.nextTaskId++}`) const task: TrackedTask = { id, - command: spec.command, status: 'running', exitCode: null, signal: null, diff --git a/packages/bash/bash-local/src/run.ts b/packages/bash/bash-local/src/run.ts index b5fbd7ce2a..f6f0103604 100644 --- a/packages/bash/bash-local/src/run.ts +++ b/packages/bash/bash-local/src/run.ts @@ -200,27 +200,6 @@ export class OutputCollector { writeSync(this.spillFd, chunk) } - // TODO(snapshot-scope): `snapshot()` has one internal caller (`finalize()` at - // the bottom of this file) and `totalBytes` is read only by a test. The live - // background-poll path goes through `readFrom()`, so inline snapshot() into - // finalize() and drop or privatize the totalBytes getter. - /** - * Read the collected tail without finalizing (the final-result snapshot). - * @returns the retained tail text, the truncation flag, and the spill path when one was created. - */ - snapshot(): CollectedOutput { - return { - text: Buffer.concat(this.chunks).toString('utf8'), - truncated: this.dropped, - ...this.spillFile !== undefined ? { spillPath: this.spillFile } : {}, - } - } - - /** Total bytes ever pushed (including bytes dropped from memory). */ - get totalBytes(): number { - return this.total - } - /** * Incremental read in whole-stream byte coordinates: returns everything * pushed since `fromByte`. When `fromByte` has already slid out of the @@ -259,7 +238,11 @@ export class OutputCollector { } this.spillFd = undefined } - return this.snapshot() + return { + text: Buffer.concat(this.chunks).toString('utf8'), + truncated: this.dropped, + ...this.spillFile !== undefined ? { spillPath: this.spillFile } : {}, + } } } diff --git a/packages/bash/bash-local/tests/run.spec.ts b/packages/bash/bash-local/tests/run.spec.ts index c7f86848ff..45f082fb77 100644 --- a/packages/bash/bash-local/tests/run.spec.ts +++ b/packages/bash/bash-local/tests/run.spec.ts @@ -2,9 +2,9 @@ import { mkdtempSync, readFileSync, statSync } from 'node:fs' import { tmpdir } from 'node:os' import { dirname, join } from 'node:path' import { describe, expect, it, vi } from 'vitest' -import { killGroup, OutputCollector, runBash } from '@deepseek-ai/dsh-bash-local' -import type { RunningBash } from '@deepseek-ai/dsh-bash-local' import type { DshEnvironment } from '@deepseek-ai/dsh-bash' +import { killGroup, OutputCollector, runBash } from '../src/run.ts' +import type { RunningBash } from '../src/run.ts' const { failNextClose } = vi.hoisted(() => ({ failNextClose: { value: false } })) vi.mock('node:fs', async (importOriginal) => { @@ -50,7 +50,7 @@ async function waitGone(pid: number, timeoutMs = 5_000): Promise { async function waitForStdout(running: RunningBash, expected: string, timeoutMs = 5_000): Promise { const deadline = Date.now() + timeoutMs while (Date.now() < deadline) { - if (running.stdout.snapshot().text.includes(expected)) return + if (running.stdout.readFrom(0).text.includes(expected)) return await new Promise(resolve => setTimeout(resolve, 20)) } throw new Error(`stdout did not include ${JSON.stringify(expected)} after ${timeoutMs}ms`) @@ -296,19 +296,11 @@ describe('OutputCollector', () => { expect(third.spillPath).toBeDefined() }) - it('tracks totalBytes across drops', () => { - const collector = new OutputCollector(4, 'test', spillDir) - collector.push(Buffer.from('aaaa')) - collector.push(Buffer.from('bbbb')) - expect(collector.totalBytes).toBe(8) - expect(collector.finalize().text).toBe('bbbb') - }) - it('contains close failures and drops the spill path', () => { const collector = new OutputCollector(4, 'closefail', spillDir) collector.push(Buffer.from('aaaa')) collector.push(Buffer.from('bbbb')) - expect(collector.snapshot().spillPath).toBeDefined() + expect(collector.readFrom(0).spillPath).toBeDefined() failNextClose.value = true let out: ReturnType diff --git a/packages/bash/bash/src/types.ts b/packages/bash/bash/src/types.ts index d43d9d494e..d0bd02e723 100644 --- a/packages/bash/bash/src/types.ts +++ b/packages/bash/bash/src/types.ts @@ -230,7 +230,6 @@ export type BashTaskStatus = 'running' | 'completed' | 'killed' /** A tracked background task handle. */ export interface BashTask { readonly id: BashTaskId - readonly command: string status: BashTaskStatus /** Exit code once finished (null = killed by signal / still running). */ exitCode: number | null diff --git a/packages/bash/bash/tests/service.spec.ts b/packages/bash/bash/tests/service.spec.ts index 94d299f175..bdee15aedd 100644 --- a/packages/bash/bash/tests/service.spec.ts +++ b/packages/bash/bash/tests/service.spec.ts @@ -34,7 +34,6 @@ class StubExecutor extends BashExecutor { start(spec: BashExecSpec): BashTask { const task: BashTask = { id: BashTaskId(`stub-${this.tasks.size + 1}`), - command: spec.command, status: 'running', exitCode: null, signal: null, diff --git a/packages/bash/tool-bash/README.md b/packages/bash/tool-bash/README.md index fa6a5285ed..2ecfaf88f5 100644 --- a/packages/bash/tool-bash/README.md +++ b/packages/bash/tool-bash/README.md @@ -4,6 +4,8 @@ The model-facing bash tools — `bash`, `bash_output`, `bash_kill` — registere Requires a loaded executor implementation (e.g. `@deepseek-ai/dsh-bash-local`); the plugin stays pending until `ctx.bash` exists (`inject: ['tools', 'bash', 'systemPrompt']`). +The package root exposes only the Cordis plugin contract (`name`, `inject`, `apply`); result rendering remains an implementation detail covered by same-package tests. + The plugin also contributes the `tool:bash` prompt section (order 105) — the cross-call habit the per-tool descriptions cannot carry: check the `[exit code: N]` marker on every result and investigate failures before moving on. A sandboxing executor changes the `bash` schema and result markers but adds no mode statement or switch notice; see [Per-session mode](#per-session-mode-switching). ## Tools diff --git a/packages/bash/tool-bash/src/index.ts b/packages/bash/tool-bash/src/index.ts index 33712c1d1b..b2c93e57b7 100644 --- a/packages/bash/tool-bash/src/index.ts +++ b/packages/bash/tool-bash/src/index.ts @@ -23,8 +23,9 @@ import type {} from '@deepseek-ai/dsh-system-prompt' import type {} from '@deepseek-ai/dsh-user-approval' import type { SandboxMode } from '@deepseek-ai/dsh-sandbox' import { BashTaskId, DSH_ENV_PREFIX, OwnerToken, effectiveSandboxMode } from '@deepseek-ai/dsh-bash' -import type { BashRunResult, BashTask, CollectedOutput, DshEnvironment, DshEnvironmentKey } from '@deepseek-ai/dsh-bash' +import type { BashTask, DshEnvironment, DshEnvironmentKey } from '@deepseek-ai/dsh-bash' import { DSH_HOME_ENV, resolveDshHome } from '@deepseek-ai/dsh-home' +import { parseExitStatus, renderResult } from './render.ts' declare module 'cordis' { interface Context { @@ -295,65 +296,6 @@ function bashDescription(escalationModes: readonly SandboxMode[]): string { + 'it — but it does not forbid attempting or escalating other commands later.' } -/** Append the truncation notice (with the full-output spill path) to a stream's text. */ -function streamText(output: CollectedOutput): string { - if (!output.truncated) return output.text - return `${output.text}\n[output truncated; full output: ${output.spillPath ?? '(unavailable)'}]` -} - -/** - * Shape one finished run into model-visible stdout, marked stderr, and status - * facts. Non-zero exits and sandbox denials remain ordinary results; only - * infrastructure failure or abort makes the tool call itself fail. - * - * @param result - the completed foreground run from the executor. - * @param escalationModes - the escalation targets this composition advertises; non-empty - * adds the same-turn escalation hint after a denial marker (default `[]`: no hint). - * @returns the model-facing text: output body (or `(no output)`), then any - * timeout/signal/exit markers, each on its own line. - */ -export function renderResult( - result: BashRunResult, - escalationModes: readonly SandboxMode[] = [], -): string { - const out = streamText(result.stdout) - const err = streamText(result.stderr) - - let body = out - if (err.length > 0) { - // Single newline between sections (stdout usually ends with one already). - if (body.length > 0 && !body.endsWith('\n')) body += '\n' - body += `[stderr]\n${err}` - } - if (body.length === 0) body = '(no output)' - - const markers: string[] = [] - // Keep `[exit code: N]` last so parseExitStatus() can recover it. A denial, - // like a timeout, remains a reported fact for the model to handle. - if (result.sandbox?.denied) { - markers.push(`[sandbox: file access denied under ${result.sandbox.mode} mode]`) - // Add the retry hint only when the schema advertises escalation, before - // the final exit marker. - if (escalationModes.length > 0) { - markers.push('[sandbox: escalation available — retry this exact command once with sandbox_permissions (the narrowest wider mode that suffices) + justification; the approval prompt asks the user]') - } - } - // Timeout is reported independently of how the process actually ended: a - // command can trap SIGTERM and exit 0 after our timer fired (e.g. - // `trap "exit 0" TERM; sleep 60`), giving timedOut:true / exitCode:0 / - // signal:null — the model must still see that the command was cut short. - if (result.timedOut) markers.push(`[timed out after ${result.timeoutMs}ms]`) - if (result.signal !== null) { - markers.push(`[killed by signal: ${result.signal}]`) - } else if (result.exitCode !== 0) { - markers.push(`[exit code: ${result.exitCode}]`) - } - if (markers.length === 0) return body - - if (!body.endsWith('\n')) body += '\n' - return body + markers.join('\n') -} - // Pure tool-owned presentation used for both live events and replay. /** @@ -401,18 +343,6 @@ function presentBashResult(args: unknown, result: ToolResult): ToolResultView | return { card: 'terminal', output: raw, ...parseExitStatus(raw) } } -/** - * Recover exit status from the final marked line emitted by {@link renderResult}. - * A program whose own final line exactly mimics a marker remains ambiguous for UI display. - */ -function parseExitStatus(text: string): { exitCode: number } | { signal: string } { - const signal = /\n\[killed by signal: ([^\]\n]+)\]$/.exec(text) - if (signal?.[1] !== undefined) return { signal: signal[1] } - const exit = /\n\[exit code: (\d+)\]$/.exec(text) - if (exit?.[1] !== undefined) return { exitCode: Number(exit[1]) } - return { exitCode: 0 } -} - /** Pending-state presentation for `bash_output`/`bash_kill` (background-task tools). */ function presentTaskCall(verb: string, args: { task_id: string }): GenericCallView { return { card: 'generic', title: `${verb} background task ${args.task_id}`, kind: 'execute', rawInput: args.task_id } diff --git a/packages/bash/tool-bash/src/render.ts b/packages/bash/tool-bash/src/render.ts new file mode 100644 index 0000000000..924861bb1e --- /dev/null +++ b/packages/bash/tool-bash/src/render.ts @@ -0,0 +1,92 @@ +/** + * Model-facing result rendering for the bash tool. + * + * @module @deepseek-ai/dsh-tool-bash/render + */ + +import type { BashRunResult, CollectedOutput } from '@deepseek-ai/dsh-bash' +import type { SandboxMode } from '@deepseek-ai/dsh-sandbox' + +/** Append the truncation notice (with the full-output spill path) to a stream's text. */ +function streamText(output: CollectedOutput): string { + if (!output.truncated) return output.text + return `${output.text}\n[output truncated; full output: ${output.spillPath ?? '(unavailable)'}]` +} + +/** + * Shape one finished run into the text the model sees: stdout, then a marked + * stderr section, then exit-status markers. Non-zero exits are REPORTED, not + * errored — the model decides how to react; only infrastructure failures + * (spawn errors, aborts) surface as isError results. + * @param result - the completed foreground run from the executor. + * @param escalationModes - the escalation targets this composition advertises; + * non-empty adds the same-turn escalation hint after a denial marker + * (default `[]`: no hint). + * @returns the model-facing text: output body (or `(no output)`), then any timeout/signal/exit markers, each on its own line. + */ +export function renderResult( + result: BashRunResult, + escalationModes: readonly SandboxMode[] = [], +): string { + const out = streamText(result.stdout) + const err = streamText(result.stderr) + + let body = out + if (err.length > 0) { + // Single newline between sections (stdout usually ends with one already). + if (body.length > 0 && !body.endsWith('\n')) body += '\n' + body += `[stderr]\n${err}` + } + if (body.length === 0) body = '(no output)' + + const markers: string[] = [] + // The sandbox marker precedes the exit-status markers so `[exit code: N]` + // stays the LAST line (exitStatus() anchors its parse there). Denial is a + // reported fact like timeout: the model decides how to react. + if (result.sandbox?.denied) { + markers.push(`[sandbox: file access denied under ${result.sandbox.mode} mode]`) + // The same-turn nudge lives at the decision point: only when this + // composition advertises the fields (a lever is never hinted that the + // schema does not offer), and inside the sandbox marker family so the + // exit-code marker stays the last line. + if (escalationModes.length > 0) { + markers.push('[sandbox: escalation available — retry this exact command once with sandbox_permissions (the narrowest wider mode that suffices) + justification; the approval prompt asks the user]') + } + } + // Timeout is reported independently of how the process actually ended: a + // command can trap SIGTERM and exit 0 after our timer fired (e.g. + // `trap "exit 0" TERM; sleep 60`), giving timedOut:true / exitCode:0 / + // signal:null — the model must still see that the command was cut short. + if (result.timedOut) markers.push(`[timed out after ${result.timeoutMs}ms]`) + if (result.signal !== null) { + markers.push(`[killed by signal: ${result.signal}]`) + } else if (result.exitCode !== 0) { + markers.push(`[exit code: ${result.exitCode}]`) + } + if (markers.length === 0) return body + + if (!body.endsWith('\n')) body += '\n' + return body + markers.join('\n') +} + +/** + * Recover the structured exit status from a rendered {@link renderResult} + * string — the inverse of the status markers it appends. A killed marker + * yields `signal`; otherwise a non-zero marker yields `exitCode`; absent both + * means a clean exit 0. + * + * Replay only retains the rendered content text, not the original + * `BashRunResult`, so terminal presentation must recover the exit pill here. + * Requiring a leading newline and the end of the string keeps ordinary output + * that merely ends with marker-like text from matching unless the final line + * is indistinguishable from a real marker. + * @param text - rendered model-facing bash result. + * @returns the recovered terminal exit code or signal. + */ +export function parseExitStatus(text: string): { exitCode: number } | { signal: string } { + const signal = /\n\[killed by signal: ([^\]\n]+)\]$/.exec(text) + if (signal?.[1] !== undefined) return { signal: signal[1] } + const exit = /\n\[exit code: (\d+)\]$/.exec(text) + if (exit?.[1] !== undefined) return { exitCode: Number(exit[1]) } + return { exitCode: 0 } +} diff --git a/packages/bash/tool-bash/tests/tools.spec.ts b/packages/bash/tool-bash/tests/tools.spec.ts index 69ca7ed92c..42ac9df704 100644 --- a/packages/bash/tool-bash/tests/tools.spec.ts +++ b/packages/bash/tool-bash/tests/tools.spec.ts @@ -21,7 +21,7 @@ import { LocalSandboxProvider } from '@deepseek-ai/dsh-sandbox-local' import ApprovalService from '@deepseek-ai/dsh-user-approval' import type { ApprovalOutcome } from '@deepseek-ai/dsh-user-approval' import * as ToolBash from '@deepseek-ai/dsh-tool-bash' -import { renderResult } from '@deepseek-ai/dsh-tool-bash' +import { renderResult } from '../src/render.ts' const spillDir = mkdtempSync(join(tmpdir(), 'dsh-tool-bash-spec-')) @@ -116,7 +116,6 @@ abstract class TestBashExecutor extends BashExecutor { class LossyReadBashExecutor extends TestBashExecutor { private readonly task: BashTask = { id: BashTaskId('bash-lossy'), - command: 'fake', status: 'running', exitCode: null, signal: null, @@ -1146,7 +1145,6 @@ describe('sandbox rendering', () => { class FactsOnlyExecutor extends TestBashExecutor { private readonly task: BashTask = { id: BashTaskId('bash-facts'), - command: 'fake', status: 'completed', exitCode: 1, signal: null, diff --git a/packages/cordis/tool-cordis/src/api-catalog.ts b/packages/cordis/tool-cordis/src/api-catalog.ts index c5c38910bf..0a33f24caf 100644 --- a/packages/cordis/tool-cordis/src/api-catalog.ts +++ b/packages/cordis/tool-cordis/src/api-catalog.ts @@ -597,7 +597,7 @@ export const TYPE_API: readonly TypeApiEntry[] = [ }, { name: 'BashTask', - declaration: 'export interface BashTask {\n readonly id: BashTaskId;\n readonly command: string;\n status: BashTaskStatus;\n exitCode: number | null;\n signal: NodeJS.Signals | null;\n readonly done: Promise;\n sandbox?: BashSandboxInfo;\n}', + declaration: 'export interface BashTask {\n readonly id: BashTaskId;\n status: BashTaskStatus;\n exitCode: number | null;\n signal: NodeJS.Signals | null;\n readonly done: Promise;\n sandbox?: BashSandboxInfo;\n}', }, { name: 'BashTaskId',