diff --git a/packages/core/src/agent.test.ts b/packages/core/src/agent.test.ts index 59a4195..001e1e0 100644 --- a/packages/core/src/agent.test.ts +++ b/packages/core/src/agent.test.ts @@ -1499,4 +1499,81 @@ describe('runAgent', () => { expect(resultText(result.history)).toBe('fast enough'); }); }); + + describe('persistent shells', () => { + it('closes every shell it opened, even when the loop throws', async () => { + // A crashed run leaving live shell processes on the machine is the + // objection this capability has to answer, so the guarantee cannot rest + // on the loop reaching its normal exit. + let seen: import('./shell/registry.js').ShellRegistry | undefined; + const grab: ToolHandler = { + name: 'Grab', + definition: { + name: 'Grab', + description: 'captures the registry then explodes the run', + inputSchema: { type: 'object', properties: {} }, + }, + async execute(_input, toolCtx) { + seen = toolCtx.shells; + await toolCtx.shells?.open({ cwd }); + return { content: 'grabbed' }; + }, + }; + + const exploding: Provider = { + name: 'exploding', + runTurn: (() => { + let first = true; + return () => { + if (first) { + first = false; + return Promise.resolve( + toolUse('grabbing', { + type: 'tool_use', + id: 'c1', + name: 'Grab', + input: {}, + }), + ); + } + // Not an AbortError, so the loop does not treat it as cancellation. + throw Object.assign(new Error('provider exploded'), { fatal: true }); + }; + })(), + }; + + await runAgent({ + provider: exploding, + tools: new ToolRegistry([grab]), + systemPrompt: '', + userMessage: 'go', + model: 'deepseek-chat', + cwd, + }); + + expect(seen).toBeDefined(); + expect(seen?.list()).toEqual([]); + }); + + it('leaves a host-owned registry alone', async () => { + // The host closes what the host owns; shells must survive between runs + // for a REPL session to be worth anything. + const { ShellRegistry } = await import('./shell/registry.js'); + const shells = new ShellRegistry(); + await shells.open({ cwd }); + + await runAgent({ + provider: new MockProvider([endTurn('done')]), + tools: new ToolRegistry(), + systemPrompt: '', + userMessage: 'go', + model: 'deepseek-chat', + cwd, + shells, + }); + + expect(shells.list()).toHaveLength(1); + await shells.closeAll(); + }); + }); }); diff --git a/packages/core/src/agent.ts b/packages/core/src/agent.ts index 3efbf12..a2249c3 100644 --- a/packages/core/src/agent.ts +++ b/packages/core/src/agent.ts @@ -21,6 +21,7 @@ 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'; +import { ShellRegistry } from './shell/registry.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`. @@ -141,6 +142,13 @@ export interface RunAgentOptions { /** Installed-plugin directories — so the Task tool can resolve plugin-bundled * sub-agents (`/agents/*.md`) in addition to user/project ones. */ pluginDirs?: string[]; + /** + * Host-owned registry for shells that outlive one tool call. When set, shells + * survive across runs and the host is responsible for closing them. When + * absent, this run owns one and closes every shell before it returns, so no + * shell can outlive the run that opened it. + */ + shells?: ShellRegistry; /** Optional host-owned background-task manager (e.g. the REPL's session-scoped * one). When set, this run attaches its sub-agent runner to it and exposes it * on the tool context, so background tasks persist across runAgent calls and @@ -241,7 +249,28 @@ const READ_ONLY_TOOLS = new Set([ * Runs the agent loop until the model produces an end_turn (no tool calls), * or `maxTurns` is reached, or the abort signal fires. */ +/** + * Run the agent loop. + * + * When the caller supplies no {@link RunAgentOptions.shells}, this run owns a + * registry and closes every shell it opened before returning — including when + * the loop throws. Leaving live shell processes behind after a crashed run is + * the failure this wrapper exists to make impossible. + * + * @param opts Everything the loop needs. + * @returns The final history, usage, and stop reason. + */ export async function runAgent(opts: RunAgentOptions): Promise { + if (opts.shells) return runAgentInner(opts); + const shells = new ShellRegistry(); + try { + return await runAgentInner({ ...opts, shells }); + } finally { + await shells.closeAll(); + } +} + +async function runAgentInner(opts: RunAgentOptions): Promise { const maxTurns = opts.maxTurns ?? DEFAULT_MAX_TURNS; const runtimePolicy = resolveRuntimePolicy(opts); const allowedToolNames = opts.allowedTools ? new Set(opts.allowedTools) : undefined; @@ -544,6 +573,8 @@ export async function runAgent(opts: RunAgentOptions): Promise { } } + toolCtx.shells = opts.shells; + const totalUsage = { inputTokens: 0, outputTokens: 0, reasoningTokens: 0, cacheReadTokens: 0 }; let turnsUsed = 0; diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index b07810f..954b587 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -40,6 +40,7 @@ export { TodoWriteTool, SessionSearchTool, SessionReadTool, + SHELL_TOOLS, WebFetchTool, WebSearchTool, AskUserQuestionTool, @@ -252,6 +253,15 @@ export { type RepeatReminderKind, type ToolDeadlineConfig, } from './guard/index.js'; +// Persistent shells +export { + PersistentShell, + ShellRegistry, + type ShellInfo, + type ShellRegistryOptions, + type ShellRunResult, + type ShellSessionOptions, +} from './shell/index.js'; // Agent loop's approval callback type (M3b) export type { ApprovalCallback, ApprovalDecision } from './agent.js'; diff --git a/packages/core/src/shell/index.ts b/packages/core/src/shell/index.ts new file mode 100644 index 0000000..9e985ab --- /dev/null +++ b/packages/core/src/shell/index.ts @@ -0,0 +1,5 @@ +// Shells that survive between tool calls. +// Spec: docs/DSH_ADOPTION_PLAN.md §1.4 + +export { PersistentShell, type ShellRunResult, type ShellSessionOptions } from './session.js'; +export { ShellRegistry, type ShellInfo, type ShellRegistryOptions } from './registry.js'; diff --git a/packages/core/src/shell/registry.ts b/packages/core/src/shell/registry.ts new file mode 100644 index 0000000..835cd5d --- /dev/null +++ b/packages/core/src/shell/registry.ts @@ -0,0 +1,155 @@ +// Owner of every open shell, and the thing that guarantees none outlive their run. +// Spec: docs/DSH_ADOPTION_PLAN.md §1.4 +// +// A long-lived process is a leak waiting to happen. Two rules keep it bounded: +// the registry closes everything when its owner ends, and an idle shell closes +// itself. Neither is optional — a crashed session leaving orphaned shells on the +// machine is exactly the objection this capability has to answer. + +import { PersistentShell, type ShellSessionOptions } from './session.js'; + +/** An open shell and what a caller is allowed to know about it. */ +export interface ShellInfo { + id: string; + /** Directory it started in. */ + cwd: string; + /** ISO timestamp of the last command run in it. */ + lastUsedAt: string; + busy: boolean; +} + +export interface ShellRegistryOptions { + /** Close a shell after this long without a command. Defaults to 30 minutes. */ + idleTimeoutMs?: number; + /** Maximum shells open at once. Defaults to 8. */ + maxShells?: number; +} + +const DEFAULT_IDLE_TIMEOUT_MS = 30 * 60_000; +const DEFAULT_MAX_SHELLS = 8; + +interface Entry { + shell: PersistentShell; + cwd: string; + lastUsedAt: number; + timer: ReturnType; +} + +/** Every shell open for one agent session. */ +export class ShellRegistry { + readonly #entries = new Map(); + readonly #idleTimeoutMs: number; + readonly #maxShells: number; + #seq = 0; + + /** + * @param opts Idle timeout and concurrency cap. + */ + constructor(opts: ShellRegistryOptions = {}) { + this.#idleTimeoutMs = opts.idleTimeoutMs ?? DEFAULT_IDLE_TIMEOUT_MS; + this.#maxShells = opts.maxShells ?? DEFAULT_MAX_SHELLS; + } + + /** + * Open a shell. + * + * @param opts Where to start it and under what confinement. + * @returns The new shell's id. + * @throws When the concurrency cap is already reached. + */ + async open(opts: ShellSessionOptions): Promise { + this.#reap(); + if (this.#entries.size >= this.#maxShells) { + throw new Error( + `Too many shells open (${this.#entries.size}/${this.#maxShells}). Close one with ShellClose first.`, + ); + } + const id = `shell-${++this.#seq}`; + const shell = await PersistentShell.open(opts); + this.#entries.set(id, { + shell, + cwd: opts.cwd, + lastUsedAt: Date.now(), + timer: this.#armIdle(id), + }); + return id; + } + + /** + * Look up a live shell. + * + * @param id Shell id. + * @returns The shell, or undefined when it is unknown or already gone. + */ + get(id: string): PersistentShell | undefined { + const entry = this.#entries.get(id); + if (!entry) return undefined; + if (entry.shell.closed) { + this.#forget(id); + return undefined; + } + return entry.shell; + } + + /** Record activity on a shell and restart its idle countdown. */ + touch(id: string): void { + const entry = this.#entries.get(id); + if (!entry) return; + entry.lastUsedAt = Date.now(); + clearTimeout(entry.timer); + entry.timer = this.#armIdle(id); + } + + /** Every open shell, oldest first. */ + list(): ShellInfo[] { + this.#reap(); + return [...this.#entries.entries()].map(([id, e]) => ({ + id, + cwd: e.cwd, + lastUsedAt: new Date(e.lastUsedAt).toISOString(), + busy: e.shell.busy, + })); + } + + /** + * Close one shell. + * + * @param id Shell id. + * @returns True when a shell was closed, false when the id was unknown. + */ + async close(id: string): Promise { + const entry = this.#entries.get(id); + if (!entry) return false; + this.#forget(id); + await entry.shell.close(); + return true; + } + + /** Close everything. Called when the owning run or session ends. */ + async closeAll(): Promise { + const entries = [...this.#entries.keys()]; + await Promise.all(entries.map((id) => this.close(id))); + } + + #armIdle(id: string): ReturnType { + const timer = setTimeout(() => { + void this.close(id); + }, this.#idleTimeoutMs); + // An idle shell must not be the reason the process stays alive. + timer.unref?.(); + return timer; + } + + #forget(id: string): void { + const entry = this.#entries.get(id); + if (entry) clearTimeout(entry.timer); + this.#entries.delete(id); + } + + /** Drop entries whose shell exited on its own. */ + #reap(): void { + for (const [id, entry] of this.#entries) { + if (entry.shell.closed) this.#forget(id); + } + } +} diff --git a/packages/core/src/shell/session.test.ts b/packages/core/src/shell/session.test.ts new file mode 100644 index 0000000..5706612 --- /dev/null +++ b/packages/core/src/shell/session.test.ts @@ -0,0 +1,181 @@ +import { describe, it, expect, afterEach } from 'vitest'; +import { mkdtemp } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { PersistentShell } from './session.js'; +import { ShellRegistry } from './registry.js'; + +const open = async (): Promise => + PersistentShell.open({ cwd: await mkdtemp(join(tmpdir(), 'dc-shell-')) }); + +describe('PersistentShell', () => { + const opened: PersistentShell[] = []; + const track = async (): Promise => { + const shell = await open(); + opened.push(shell); + return shell; + }; + afterEach(async () => { + await Promise.all(opened.splice(0).map((s) => s.close())); + }); + + it('runs a command and reports its exit code', async () => { + const shell = await track(); + const result = await shell.run('echo hello', 10_000); + expect(result.output).toBe('hello'); + expect(result.exitCode).toBe(0); + expect(result.timedOut).toBe(false); + }); + + it('reports a non-zero exit code', async () => { + const shell = await track(); + expect((await shell.run('bash -c "exit 3"', 10_000)).exitCode).toBe(3); + }); + + it('treats `exit` as closing the shell, because that is what it does', async () => { + // Not a bug to paper over: `exit` in a real shell ends the session too. The + // caller is told the shell is gone rather than being handed a dead id. + const shell = await track(); + const out = await shell.run('exit 3', 10_000); + expect(out.discarded).toBe(true); + expect(shell.closed).toBe(true); + }); + + it('keeps the working directory between commands', async () => { + // The entire point: `cd` in one call is still in effect in the next. + const shell = await track(); + await shell.run('mkdir -p sub && cd sub', 10_000); + expect((await shell.run('basename "$PWD"', 10_000)).output).toBe('sub'); + }); + + it('keeps environment variables between commands', async () => { + const shell = await track(); + await shell.run('export GREETING=hi', 10_000); + expect((await shell.run('echo "$GREETING"', 10_000)).output).toBe('hi'); + }); + + it('keeps shell functions between commands', async () => { + const shell = await track(); + await shell.run('greet() { echo "hey $1"; }', 10_000); + expect((await shell.run('greet there', 10_000)).output).toBe('hey there'); + }); + + it('interleaves stderr with stdout', async () => { + const shell = await track(); + const out = await shell.run('echo one; echo two >&2; echo three', 10_000); + expect(out.output.split('\n')).toEqual(['one', 'two', 'three']); + }); + + it('runs multi-line commands', async () => { + const shell = await track(); + const out = await shell.run('for i in 1 2 3; do\n echo "n$i"\ndone', 10_000); + expect(out.output).toBe('n1\nn2\nn3'); + }); + + it('is not fooled by a command that prints something sentinel-shaped', async () => { + // The sentinel is random per session, so this cannot collide by accident. + const shell = await track(); + const out = await shell.run('echo "__DEEPCODE_deadbeef__ 0"; echo real', 10_000); + expect(out.output).toContain('real'); + expect(out.exitCode).toBe(0); + }); + + it('does not let a command steal the sentinel through stdin', async () => { + // `cat` with an inherited stdin would swallow the sentinel line and hang + // until the deadline. Commands run with stdin closed for exactly this. + const shell = await track(); + const out = await shell.run('cat', 10_000); + expect(out.timedOut).toBe(false); + expect(out.exitCode).toBe(0); + }); + + it('interrupts a command that overruns, and stays usable', async () => { + const shell = await track(); + const out = await shell.run('sleep 30', 700); + expect(out.timedOut).toBe(true); + expect(out.discarded).toBe(false); + // Still alive, and still holding the state it had. + expect((await shell.run('echo alive', 10_000)).output).toBe('alive'); + }); + + it('refuses to run two commands at once', async () => { + const shell = await track(); + const first = shell.run('sleep 0.4', 10_000); + await expect(shell.run('echo second', 10_000)).rejects.toThrow(/already running/); + await first; + }); + + it('reports as discarded once closed', async () => { + const shell = await track(); + await shell.close(); + const out = await shell.run('echo anything', 10_000); + expect(out.discarded).toBe(true); + }); + + it('kills what it started when it closes', async () => { + const shell = await track(); + const started = await shell.run('sleep 60 & echo "$!"', 10_000); + const pid = Number(started.output.trim()); + expect(Number.isFinite(pid)).toBe(true); + + await shell.close(); + await new Promise((r) => setTimeout(r, 300)); + // A shell that leaves its children running is the leak this must not have. + expect(() => process.kill(pid, 0)).toThrow(); + }); +}); + +describe('ShellRegistry', () => { + it('hands back the same shell for an id', async () => { + const registry = new ShellRegistry(); + const id = await registry.open({ cwd: tmpdir() }); + await registry.get(id)?.run('export MARK=kept', 10_000); + expect((await registry.get(id)?.run('echo "$MARK"', 10_000))?.output).toBe('kept'); + await registry.closeAll(); + }); + + it('lists what is open and forgets what is closed', async () => { + const registry = new ShellRegistry(); + const id = await registry.open({ cwd: tmpdir() }); + expect(registry.list().map((s) => s.id)).toEqual([id]); + + expect(await registry.close(id)).toBe(true); + expect(registry.list()).toEqual([]); + expect(registry.get(id)).toBeUndefined(); + expect(await registry.close(id)).toBe(false); + }); + + it('refuses to open past the cap rather than accumulating shells', async () => { + const registry = new ShellRegistry({ maxShells: 2 }); + await registry.open({ cwd: tmpdir() }); + await registry.open({ cwd: tmpdir() }); + await expect(registry.open({ cwd: tmpdir() })).rejects.toThrow(/Too many shells/); + await registry.closeAll(); + }); + + it('closes an idle shell on its own', async () => { + const registry = new ShellRegistry({ idleTimeoutMs: 150 }); + const id = await registry.open({ cwd: tmpdir() }); + await new Promise((r) => setTimeout(r, 400)); + expect(registry.get(id)).toBeUndefined(); + await registry.closeAll(); + }); + + it('restarts the idle countdown when a shell is used', async () => { + const registry = new ShellRegistry({ idleTimeoutMs: 300 }); + const id = await registry.open({ cwd: tmpdir() }); + await new Promise((r) => setTimeout(r, 200)); + registry.touch(id); + await new Promise((r) => setTimeout(r, 200)); + expect(registry.get(id)).toBeDefined(); + await registry.closeAll(); + }); + + it('closeAll leaves nothing open', async () => { + const registry = new ShellRegistry(); + await registry.open({ cwd: tmpdir() }); + await registry.open({ cwd: tmpdir() }); + await registry.closeAll(); + expect(registry.list()).toEqual([]); + }); +}); diff --git a/packages/core/src/shell/session.ts b/packages/core/src/shell/session.ts new file mode 100644 index 0000000..fad2098 --- /dev/null +++ b/packages/core/src/shell/session.ts @@ -0,0 +1,275 @@ +// A shell that survives between tool calls. +// +// Spec: docs/DSH_ADOPTION_PLAN.md §1.4 +// +// Every `Bash` call is a fresh process, so `cd`, `export`, and +// `source venv/bin/activate` are all forgotten the moment they return. The +// model's workaround is to re-paste the whole prefix into every command, which +// is long, easy to get wrong, and still cannot hold a background server. +// +// This is NOT a PTY. dsh uses one; a PTY means `node-pty`, a native dependency +// needing a build for every platform the desktop ships to — a real release risk +// for a benefit (full-screen programs like vim and top) that is a small part of +// what makes a persistent shell useful. Instead the shell runs over ordinary +// pipes and command completion is detected with a sentinel line. Interactive +// full-screen programs do not work here, and the tool description says so. +// +// Two consequences of the pipe design, both deliberate: +// +// * Each command runs with stdin from /dev/null. Otherwise a command that +// reads stdin (`cat`, an interactive prompt) would swallow the sentinel +// that follows it and the session would hang until its deadline. +// * stderr is merged into stdout at the shell, so the two interleave in the +// order they were actually written, the way a terminal shows them. + +import { execFile, spawn, type ChildProcess } from 'node:child_process'; +import { randomBytes } from 'node:crypto'; +import type { SandboxConfig, SandboxMode } from '../config/types.js'; +import { wrapBashCommand } from '../sandbox/index.js'; + +/** Result of one command run in a persistent shell. */ +export interface ShellRunResult { + /** Everything the command wrote, stdout and stderr interleaved. */ + output: string; + /** Exit status, or null when the command did not finish. */ + exitCode: number | null; + /** True when the deadline elapsed before the command finished. */ + timedOut: boolean; + /** + * True when the shell could not be recovered and has been discarded. The + * caller must open a new one; this session's id is dead. + */ + discarded: boolean; +} + +export interface ShellSessionOptions { + /** Directory the shell starts in. */ + cwd: string; + /** Sandbox configuration, applied once when the shell starts. */ + sandboxConfig?: SandboxConfig; + /** Sandbox mode when the config names none. */ + sandboxDefaultMode?: SandboxMode; + /** Environment for the shell process. Defaults to the parent's. */ + env?: NodeJS.ProcessEnv; +} + +/** How long to wait for the shell to recover after interrupting a command. */ +const RECOVERY_GRACE_MS = 2_000; + +/** Bytes of output one command may accumulate before older output is dropped. */ +const MAX_OUTPUT_CHARS = 2_000_000; + +/** + * A long-lived shell process, one command at a time. + * + * The shell keeps its working directory, environment, functions, and any + * background jobs across calls, which is the entire point. + */ +export class PersistentShell { + /** Random per session, so a command cannot forge it even by accident. */ + readonly #sentinel = `__DEEPCODE_${randomBytes(9).toString('hex')}__`; + readonly #cwd: string; + #child: ChildProcess | undefined; + #buffer = ''; + #waiter: ((line: RegExpMatchArray) => void) | undefined; + #busy = false; + #closed = false; + + private constructor(cwd: string) { + this.#cwd = cwd; + } + + /** + * Start a shell. + * + * The sandbox wrapper is resolved once, here. A later change to sandbox + * settings does not re-arm a shell that is already running — the tool + * description states this, and it is why the policy is captured at open time + * rather than read per command. + * + * @param opts Where to start and under what confinement. + * @returns A ready session. + */ + static async open(opts: ShellSessionOptions): Promise { + const session = new PersistentShell(opts.cwd); + // `exec` replaces the wrapper shell so signals reach bash itself; `2>&1` + // merges the streams at the shell, preserving their true order. + const wrapped = await wrapBashCommand({ + userCommand: 'exec /bin/bash 2>&1', + cwd: opts.cwd, + config: opts.sandboxConfig, + ...(opts.sandboxDefaultMode !== undefined ? { defaultMode: opts.sandboxDefaultMode } : {}), + }); + + const child = spawn(wrapped.command, wrapped.args, { + cwd: opts.cwd, + env: opts.env ?? process.env, + stdio: ['pipe', 'pipe', 'pipe'], + // Its own process group, so interrupting a command reaches the whole + // pipeline rather than only the shell. + detached: process.platform !== 'win32', + }); + session.#child = child; + child.stdout?.on('data', (c: Buffer) => session.#ingest(c.toString('utf8'))); + child.stderr?.on('data', (c: Buffer) => session.#ingest(c.toString('utf8'))); + child.on('exit', () => { + session.#closed = true; + // Unblock anyone waiting: the sentinel is never arriving. + session.#waiter?.(['', ''] as unknown as RegExpMatchArray); + }); + return session; + } + + /** Directory the shell was started in. Its current one may differ after `cd`. */ + get cwd(): string { + return this.#cwd; + } + + /** True once the shell has exited or been closed. */ + get closed(): boolean { + return this.#closed; + } + + /** True while a command is running. */ + get busy(): boolean { + return this.#busy; + } + + #ingest(chunk: string): void { + this.#buffer += chunk; + if (this.#buffer.length > MAX_OUTPUT_CHARS) { + this.#buffer = this.#buffer.slice(this.#buffer.length - MAX_OUTPUT_CHARS); + } + const match = this.#buffer.match(new RegExp(`^${this.#sentinel} (-?\\d+)$`, 'm')); + if (match) this.#waiter?.(match); + } + + /** + * Run one command and wait for it to finish. + * + * @param command Shell source to run. Multi-line is fine. + * @param timeoutMs How long to wait before interrupting it. + * @returns Its output and exit status, or what is known if it timed out. + */ + async run(command: string, timeoutMs: number): Promise { + if (this.#closed) { + return { output: '', exitCode: null, timedOut: false, discarded: true }; + } + if (this.#busy) { + throw new Error('This shell is already running a command.'); + } + this.#busy = true; + this.#buffer = ''; + try { + // Braces rather than a subshell: `cd` and `export` must affect the shell + // itself, which is the reason this class exists. + this.#child?.stdin?.write( + `{\n${command}\n} { + return new Promise((resolve) => { + const timer = setTimeout(() => { + this.#waiter = undefined; + resolve(undefined); + }, ms); + this.#waiter = (match) => { + clearTimeout(timer); + this.#waiter = undefined; + resolve(match); + }; + }); + } + + /** Split the sentinel line off the buffer and report the result. */ + #harvest(match: RegExpMatchArray, timedOut: boolean): ShellRunResult { + if (this.#closed) { + return { output: this.#buffer, exitCode: null, timedOut, discarded: true }; + } + const at = this.#buffer.indexOf(match[0]); + // Trailing newlines are dropped, not just the one printed ahead of the + // sentinel. The protocol cannot tell that one apart from a newline the + // command itself ended with, and trailing blank lines in a tool result are + // noise every caller would strip anyway. + const output = (at === -1 ? this.#buffer : this.#buffer.slice(0, at)).replace(/\n+$/, ''); + const code = Number(match[1]); + return { + output, + exitCode: Number.isFinite(code) ? code : null, + timedOut, + discarded: false, + }; + } + + /** + * Interrupt the running command without killing the shell. + * + * Signalling the process group would take the shell with it: a + * non-interactive bash terminates on SIGINT, so the session would be lost + * every time a command overran. Signalling only the shell's children stops + * the command — bash reports exit 130 and carries on with its state intact. + */ + #interrupt(): void { + const pid = this.#child?.pid; + if (pid === undefined || process.platform === 'win32') { + this.#child?.kill('SIGINT'); + return; + } + execFile('ps', ['-o', 'pid=,ppid=', '-A'], (err, stdout) => { + if (err) return; + for (const line of stdout.split('\n')) { + const [child, parent] = line.trim().split(/\s+/).map(Number); + if (parent !== pid || !Number.isFinite(child)) continue; + try { + process.kill(child, 'SIGINT'); + } catch { + // Already exited between listing and signalling. + } + } + }); + } + + /** Stop the shell and everything it started. */ + async close(): Promise { + if (this.#closed) return; + this.#closed = true; + const child = this.#child; + if (!child || child.exitCode !== null) return; + const exited = new Promise((resolve) => child.once('exit', () => resolve())); + try { + child.stdin?.end(); + if (process.platform !== 'win32' && child.pid !== undefined) + process.kill(-child.pid, 'SIGTERM'); + else child.kill('SIGTERM'); + } catch { + // Already gone. + } + await Promise.race([exited, new Promise((r) => setTimeout(r, 1_000))]); + try { + if (child.exitCode === null && process.platform !== 'win32' && child.pid !== undefined) { + process.kill(-child.pid, 'SIGKILL'); + } + } catch { + // Already gone. + } + } +} diff --git a/packages/core/src/tools/index.ts b/packages/core/src/tools/index.ts index 10a4a19..44832b8 100644 --- a/packages/core/src/tools/index.ts +++ b/packages/core/src/tools/index.ts @@ -27,5 +27,12 @@ export { type ToolSearchRegistry, } from './tool-search.js'; export { SessionSearchTool, SessionReadTool } from './session-search.js'; +export { + ShellOpenTool, + ShellRunTool, + ShellCloseTool, + ShellListTool, + SHELL_TOOLS, +} from './shell.js'; export { ToolRegistry, BUILTIN_TOOLS } from './registry.js'; export type { ToolDefinition, ToolContext, ToolResult, ToolHandler } from './types.js'; diff --git a/packages/core/src/tools/registry.ts b/packages/core/src/tools/registry.ts index 62dce9f..ee7de73 100644 --- a/packages/core/src/tools/registry.ts +++ b/packages/core/src/tools/registry.ts @@ -14,6 +14,7 @@ import { GrepTool } from './grep.js'; import { NotebookEditTool } from './notebook.js'; import { ReadTool } from './read.js'; import { SessionReadTool, SessionSearchTool } from './session-search.js'; +import { SHELL_TOOLS } from './shell.js'; import { SubmitReviewFindingTool } from './review-finding.js'; import { TaskTool } from './task.js'; import { TodoWriteTool } from './todo.js'; @@ -41,6 +42,7 @@ export const BUILTIN_TOOLS: ToolHandler[] = [ TodoWriteTool, SessionSearchTool, SessionReadTool, + ...SHELL_TOOLS, WebFetchTool, WebSearchTool, AskUserQuestionTool, diff --git a/packages/core/src/tools/shell.test.ts b/packages/core/src/tools/shell.test.ts new file mode 100644 index 0000000..931474d --- /dev/null +++ b/packages/core/src/tools/shell.test.ts @@ -0,0 +1,98 @@ +import { describe, it, expect, afterEach, beforeEach } from 'vitest'; +import { tmpdir } from 'node:os'; +import { ShellRegistry } from '../shell/registry.js'; +import { ShellCloseTool, ShellListTool, ShellOpenTool, ShellRunTool } from './shell.js'; +import type { ToolContext } from '../types.js'; + +describe('shell tools', () => { + let shells: ShellRegistry; + let ctx: ToolContext; + + beforeEach(() => { + shells = new ShellRegistry(); + ctx = { cwd: tmpdir(), shells }; + }); + afterEach(async () => { + await shells.closeAll(); + }); + + async function openShell(): Promise { + const out = await ShellOpenTool.execute({}, ctx); + return (out.data as { shellId: string }).shellId; + } + + it('opens a shell and runs commands that build on each other', async () => { + const id = await openShell(); + await ShellRunTool.execute({ shell_id: id, command: 'export BUILT=yes' }, ctx); + const out = await ShellRunTool.execute({ shell_id: id, command: 'echo "$BUILT"' }, ctx); + + expect(out.isError).toBeFalsy(); + expect(out.content).toContain('yes'); + expect(out.content).toContain('exit: 0'); + }); + + it('reports a failing command as an error', async () => { + const id = await openShell(); + const out = await ShellRunTool.execute({ shell_id: id, command: 'bash -c "exit 7"' }, ctx); + expect(out.isError).toBe(true); + expect(out.content).toContain('exit: 7'); + }); + + it('says the shell is unknown instead of silently opening a new one', async () => { + const out = await ShellRunTool.execute({ shell_id: 'shell-999', command: 'echo hi' }, ctx); + expect(out.isError).toBe(true); + expect(out.content).toContain('no open shell'); + }); + + it('lists and closes shells', async () => { + const id = await openShell(); + expect((await ShellListTool.execute({}, ctx)).content).toContain(id); + + expect((await ShellCloseTool.execute({ shell_id: id }, ctx)).content).toContain('Closed'); + expect((await ShellListTool.execute({}, ctx)).content).toBe('No shells open.'); + }); + + it('closing an unknown shell is stated, not an error', async () => { + const out = await ShellCloseTool.execute({ shell_id: 'shell-999' }, ctx); + expect(out.isError).toBeFalsy(); + expect(out.content).toContain('nothing to close'); + }); + + it('points at Bash when the host has no registry', async () => { + // Silently doing nothing would leave the model waiting on state that never + // persists; naming the alternative lets it carry on. + const out = await ShellOpenTool.execute({}, { cwd: tmpdir() }); + expect(out.isError).toBe(true); + expect(out.content).toContain('Bash tool'); + }); + + it('says plainly when a timeout cost the shell its state', async () => { + const id = await openShell(); + const out = await ShellRunTool.execute( + { shell_id: id, command: 'sleep 30', timeout: 400 }, + ctx, + ); + // Interrupted but recovered: the shell survives, so state is intact. + expect(out.content).toContain('interrupted after 400ms'); + expect(out.content).toContain('still usable'); + expect(await ShellRunTool.execute({ shell_id: id, command: 'echo ok' }, ctx)).toMatchObject({ + isError: false, + }); + }); + + it('resolves a relative cwd against the workspace', async () => { + const out = await ShellOpenTool.execute({ cwd: '.' }, ctx); + expect((out.data as { cwd: string }).cwd).toBe(tmpdir()); + }); + + it('warns that the sandbox policy is fixed at open time', async () => { + // The shell is wrapped once, when it starts. A model that assumes otherwise + // would misread a later settings change as applying to an open shell. + const out = await ShellOpenTool.execute({}, ctx); + expect(out.content).toContain('sandbox policy is fixed'); + }); + + it('warns in its description that full-screen programs do not work', () => { + expect(ShellOpenTool.definition.description).toContain('vim'); + }); +}); diff --git a/packages/core/src/tools/shell.ts b/packages/core/src/tools/shell.ts new file mode 100644 index 0000000..f2fd35a --- /dev/null +++ b/packages/core/src/tools/shell.ts @@ -0,0 +1,182 @@ +// ShellOpen / ShellRun / ShellClose / ShellList — a shell that remembers. +// Spec: docs/DSH_ADOPTION_PLAN.md §1.4 + +import { isAbsolute, resolve } from 'node:path'; +import type { ToolContext, ToolHandler, ToolResult } from '../types.js'; + +const DEFAULT_TIMEOUT_MS = 120_000; +const MAX_TIMEOUT_MS = 600_000; + +function noRegistry(): ToolResult { + return { + content: + 'Error: persistent shells are not available in this run. Use the Bash tool, and repeat any cd/export prefix each call.', + isError: true, + }; +} + +export const ShellOpenTool: ToolHandler = { + name: 'ShellOpen', + definition: { + name: 'ShellOpen', + description: + 'Opens a shell that stays alive across calls, keeping its working directory, environment, shell functions, and background jobs. Use it when several commands depend on each other (activate a venv then install; cd then build; start a server then watch its log). One-off commands should still use Bash. Full-screen interactive programs (vim, top, less) do NOT work — commands run with stdin closed. Close it with ShellClose when done.', + inputSchema: { + type: 'object', + properties: { + cwd: { type: 'string', description: 'Directory to start in (default: the workspace).' }, + }, + }, + }, + async execute(rawInput: Record, ctx: ToolContext): Promise { + if (!ctx.shells) return noRegistry(); + const requested = rawInput['cwd']; + const cwd = + typeof requested === 'string' && requested.length > 0 + ? isAbsolute(requested) + ? requested + : resolve(ctx.cwd, requested) + : ctx.cwd; + + try { + const id = await ctx.shells.open({ + cwd, + ...(ctx.sandboxConfig !== undefined ? { sandboxConfig: ctx.sandboxConfig } : {}), + ...(ctx.sandboxDefaultMode !== undefined + ? { sandboxDefaultMode: ctx.sandboxDefaultMode } + : {}), + }); + return { + content: `Opened ${id} in ${cwd}. Run commands with ShellRun({ shell_id: "${id}", command: "..." }).\nIts sandbox policy is fixed as of now; changing sandbox settings later will not re-arm it.`, + data: { shellId: id, cwd }, + }; + } catch (err) { + return { content: `Error opening shell: ${(err as Error).message}`, isError: true }; + } + }, +}; + +export const ShellRunTool: ToolHandler = { + name: 'ShellRun', + definition: { + name: 'ShellRun', + description: + 'Runs a command in a shell opened by ShellOpen. State set by earlier commands (cd, export, activated environments) still applies. Returns output and exit code.', + inputSchema: { + type: 'object', + properties: { + shell_id: { type: 'string', description: 'Id returned by ShellOpen.' }, + command: { type: 'string', description: 'Shell source to run. Multi-line is fine.' }, + timeout: { type: 'number', description: `Milliseconds (default ${DEFAULT_TIMEOUT_MS}).` }, + }, + required: ['shell_id', 'command'], + }, + }, + async execute(rawInput: Record, ctx: ToolContext): Promise { + if (!ctx.shells) return noRegistry(); + const shellId = rawInput['shell_id']; + const command = rawInput['command']; + if (typeof shellId !== 'string' || typeof command !== 'string' || command.length === 0) { + return { content: 'Error: shell_id and command are required (strings).', isError: true }; + } + const shell = ctx.shells.get(shellId); + if (!shell) { + return { + content: `Error: no open shell ${shellId}. It may have been closed or timed out; open a new one with ShellOpen.`, + isError: true, + }; + } + + const requested = rawInput['timeout']; + const timeoutMs = + typeof requested === 'number' && requested > 0 + ? Math.min(requested, MAX_TIMEOUT_MS) + : DEFAULT_TIMEOUT_MS; + + let result; + try { + result = await shell.run(command, timeoutMs); + } catch (err) { + return { content: `Error: ${(err as Error).message}`, isError: true }; + } + ctx.shells.touch(shellId); + + if (result.discarded) { + // Say what is actually true: the command was interrupted, the shell could + // not be brought back, and any state it held is gone. + return { + content: `${result.output}\n\n[${shellId} did not respond after ${timeoutMs}ms and was discarded. Whatever the command changed is unknown, and the shell's state — working directory, environment, background jobs — is gone. Open a new shell.]`, + isError: true, + data: { shellId, discarded: true, timedOut: result.timedOut }, + }; + } + + const parts = [result.output]; + if (result.timedOut) { + parts.push(`[interrupted after ${timeoutMs}ms; the shell is still usable]`); + } + parts.push(`exit: ${result.exitCode ?? 'unknown'}`); + return { + content: parts.filter((p) => p.length > 0).join('\n'), + isError: result.timedOut || (result.exitCode !== null && result.exitCode !== 0), + data: { shellId, exitCode: result.exitCode, timedOut: result.timedOut }, + }; + }, +}; + +export const ShellCloseTool: ToolHandler = { + name: 'ShellClose', + definition: { + name: 'ShellClose', + description: + 'Closes a shell opened by ShellOpen, stopping anything still running in it. Close shells you no longer need.', + inputSchema: { + type: 'object', + properties: { shell_id: { type: 'string', description: 'Id returned by ShellOpen.' } }, + required: ['shell_id'], + }, + }, + async execute(rawInput: Record, ctx: ToolContext): Promise { + if (!ctx.shells) return noRegistry(); + const shellId = rawInput['shell_id']; + if (typeof shellId !== 'string') { + return { content: 'Error: shell_id is required (string).', isError: true }; + } + const closed = await ctx.shells.close(shellId); + return closed + ? { content: `Closed ${shellId}.`, data: { shellId } } + : { content: `No open shell ${shellId}; nothing to close.`, data: { shellId } }; + }, +}; + +export const ShellListTool: ToolHandler = { + name: 'ShellList', + definition: { + name: 'ShellList', + description: + 'Lists the shells currently open, with where each started and when it was last used.', + inputSchema: { type: 'object', properties: {} }, + }, + execute(_rawInput: Record, ctx: ToolContext): Promise { + if (!ctx.shells) return Promise.resolve(noRegistry()); + const shells = ctx.shells.list(); + if (shells.length === 0) { + return Promise.resolve({ content: 'No shells open.', data: { count: 0 } }); + } + const lines = shells.map( + (s) => `${s.id} ${s.cwd} last used ${s.lastUsedAt}${s.busy ? ' [running]' : ''}`, + ); + return Promise.resolve({ + content: `${shells.length} shell(s) open:\n${lines.join('\n')}`, + data: { count: shells.length }, + }); + }, +}; + +/** Every persistent-shell tool, for registration. */ +export const SHELL_TOOLS: ToolHandler[] = [ + ShellOpenTool, + ShellRunTool, + ShellCloseTool, + ShellListTool, +]; diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index 7e6d07a..27be380 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -123,6 +123,11 @@ export interface ToolContext { * user's behalf. */ sessionSearchScope?: import('./sessions/search.js').SessionSearchScope; + /** + * Shells that outlive one tool call. Absent when the host owns no registry, + * in which case the shell tools say so instead of silently doing nothing. + */ + shells?: import('./shell/registry.js').ShellRegistry; /** Canonical app-server turn associated with session-scoped mutations. */ turnId?: string; /** Abort signal propagated from the agent loop. */