From 39c142440c29453d633f8fb1b7a611aacf6065dc Mon Sep 17 00:00:00 2001 From: t Date: Sat, 15 Aug 2026 11:46:38 +0800 Subject: [PATCH] feat(cli): let a shell outlive the turn that opened it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The registry landed in #273 owned by a single runAgent call, so a shell opened in one turn was closed before the next began — a slower Bash with extra steps. The whole point of a shell that keeps its working directory is that the next message still has it. The REPL now owns one registry for the session and threads it through every turn, the same way it already owns the background-task manager. `cd`, `export`, and an activated virtualenv now survive from one message to the next. `/shells` lists what is open and where each started; `/shells close ` closes one and whatever is still running in it. That command is part of this change rather than a follow-up: making these processes outlive a turn is exactly what creates the need to see and stop them. Cleanup is a wrapper, not a habit. `startRepl` closes every shell in a finally, so it also runs when the session ends by throwing — these are real processes in their own process group and do not die with the CLI. Background tasks and sub-agents deliberately do not share the session's shells. Two agents interleaving commands in one shell would each be wrong about its state, so they get their own, closed when their run ends. A test pins that, including an assertion that the delegation actually happened — otherwise it would pass for the wrong reason if Task were ever missing from the registry. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 18 +++++++ apps/cli/src/commands.ts | 40 ++++++++++++++ apps/cli/src/repl.ts | 29 ++++++++++ apps/cli/src/shells-command.test.ts | 83 +++++++++++++++++++++++++++++ docs/BEHAVIOR_PARITY.md | 1 + packages/core/src/agent.test.ts | 73 +++++++++++++++++++++++++ 6 files changed, 244 insertions(+) create mode 100644 apps/cli/src/shells-command.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 69dd74a..74fcf8f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,24 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### ✨ Added +- **Persistent shells now last a whole CLI session, and `/shells` shows them.** + The registry landed in #273 owned by a single `runAgent` call, which meant a + shell opened in one turn was gone by the next — a slower `Bash` with extra + steps. The REPL now owns one registry for the session and threads it through + every turn, so `cd`, `export`, and an activated virtualenv survive from one + message to the next. + + `/shells` lists what is open and where each started; `/shells close ` + closes one and whatever is still running in it. Worth having because the + change is what makes these processes outlive a turn: without a view of them, + the user has no way to see or stop something the agent left running. + + Everything closes when the session ends, including when it ends by throwing — + these are real processes in their own process group, so they do not die with + the CLI. Background tasks and sub-agents deliberately do **not** share the + session's shells: two agents interleaving commands in one shell would each be + wrong about its state. + - **A shell that survives between tool calls** (#273) — `ShellOpen` / `ShellRun` / `ShellClose` / `ShellList`. Every `Bash` call is a fresh process, so `cd`, `export`, and `source venv/bin/activate` were forgotten the moment they diff --git a/apps/cli/src/commands.ts b/apps/cli/src/commands.ts index f5e7e54..63abb84 100644 --- a/apps/cli/src/commands.ts +++ b/apps/cli/src/commands.ts @@ -10,6 +10,7 @@ import type { SessionManager, SessionMeta, StoredMessage, + ShellRegistry, TaskManager, VoiceStatus, } from '@deepcode/core'; @@ -183,6 +184,10 @@ export interface SessionContext { * /background. Same instance the agent loop uses, so tasks the agent starts * are visible here and vice-versa. */ tasks?: TaskManager; + /** Session-scoped persistent shells (REPL-injected) — backs /shells. Same + * instance the agent loop uses, so a shell the agent opened is listed here + * and closing one here really closes it. */ + shells?: ShellRegistry; } export interface SlashCommand { @@ -1220,6 +1225,40 @@ export const TasksCommand: SlashCommand = { }, }; +export const ShellsCommand: SlashCommand = { + name: '/shells', + description: 'List persistent shells this session, or `/shells close ` to close one.', + async run(args, ctx) { + if (!ctx.shells) return ['(Persistent shells are unavailable here.)']; + + if (args[0] === 'close') { + const id = args[1]?.trim(); + if (!id) return ['Usage: /shells close ']; + return [ + (await ctx.shells.close(id)) + ? `Closed ${id} and anything still running in it.` + : `No open shell "${id}". Run /shells to list them.`, + ]; + } + if (args[0]) return [`Unknown argument "${args[0]}". Usage: /shells [close ]`]; + + const shells = ctx.shells.list(); + if (shells.length === 0) { + return [ + 'No persistent shells open.', + 'The agent opens one with its ShellOpen tool when commands need to build on each other.', + ]; + } + const lines = [`Persistent shells (${shells.length}):`]; + for (const s of shells) { + lines.push(` ${s.id} ${s.cwd} last used ${s.lastUsedAt}${s.busy ? ' [running]' : ''}`); + } + lines.push(''); + lines.push('Close one with `/shells close `. All of them close when this session ends.'); + return lines; + }, +}; + /** "Ready" status lines for /voice (non-interactive / headless fallback). */ export function voiceReadyLines(status: VoiceStatus): string[] { return [ @@ -1433,6 +1472,7 @@ export const BUILTIN_COMMANDS: SlashCommand[] = [ BtwCommand, TasksCommand, BackgroundCommand, + ShellsCommand, VoiceCommand, ]; diff --git a/apps/cli/src/repl.ts b/apps/cli/src/repl.ts index 706b9e6..7976a94 100644 --- a/apps/cli/src/repl.ts +++ b/apps/cli/src/repl.ts @@ -11,6 +11,7 @@ import { ReadTool, RuntimeHost, SessionManager, + ShellRegistry, TaskManager, ToolRegistry, WebFetchTool, @@ -222,7 +223,29 @@ async function pickSessionId( return list[n - 1]!.id; } +/** + * Run the interactive REPL. + * + * The persistent-shell registry is owned here, one per REPL session, so a shell + * the agent opens in one turn is still there in the next — which is the whole + * point of a shell that keeps its working directory. The wrapper closes every + * one of them on the way out, including when the session throws: these are real + * OS processes in their own process group, so they do not die with the CLI and + * "the loop will remember" is not a guarantee. + * + * @param opts REPL configuration. + * @returns Process exit code. + */ export async function startRepl(opts: ReplOpts): Promise { + const shells = new ShellRegistry(); + try { + return await runReplSession(opts, shells); + } finally { + await shells.closeAll(); + } +} + +async function runReplSession(opts: ReplOpts, shells: ShellRegistry): Promise { const { output, cwd } = opts; // Load config + creds. Trust-gate first: in an untrusted directory, project @@ -547,6 +570,7 @@ export async function startRepl(opts: ReplOpts): Promise { return { done, abort: () => ac.abort() }; }); ctx.tasks = tasks; + ctx.shells = shells; // Colour resolves once: a --no-color flag, NO_COLOR/FORCE_COLOR, or whether // stdout is actually a terminal. Piped output stays plain. @@ -730,6 +754,11 @@ export async function startRepl(opts: ReplOpts): Promise { // Session-scoped manager: the agent's TaskCreate calls land here too, so // background tasks persist across turns and show up in /tasks. taskManager: tasks, + // Session-scoped too, for the same reason: a shell opened this turn has to + // still be there next turn or it is just a slower Bash. Deliberately NOT + // given to the background-task runner below — two agents interleaving + // commands in one shell would each be wrong about its state. + shells, approval: async (toolName, input, verdict) => { output.write( `\n ${palette.yellow('⏸')} Approve ${palette.bold(toolName)}? ${palette.dim(verdict.reason)}\n`, diff --git a/apps/cli/src/shells-command.test.ts b/apps/cli/src/shells-command.test.ts new file mode 100644 index 0000000..4c1e9c4 --- /dev/null +++ b/apps/cli/src/shells-command.test.ts @@ -0,0 +1,83 @@ +// Tests for /shells, which drives the session-scoped ShellRegistry the REPL +// owns (ctx.shells). Uses a real registry — a persistent shell is a real +// process and the interesting assertions are about real ones being listed and +// really closed. + +import { afterEach, describe, expect, it } from 'vitest'; +import { tmpdir } from 'node:os'; +import { SessionManager, ShellRegistry } from '@deepcode/core'; +import { CommandRegistry, type SessionContext } from './commands.js'; + +const reg = new CommandRegistry(); +const opened: ShellRegistry[] = []; + +function registry(): ShellRegistry { + const r = new ShellRegistry(); + opened.push(r); + return r; +} + +afterEach(async () => { + await Promise.all(opened.splice(0).map((r) => r.closeAll())); +}); + +function ctx(overrides: Partial = {}): SessionContext { + return { + cwd: tmpdir(), + model: 'deepseek-chat', + mode: 'default', + effort: 'medium', + settings: {}, + creds: { apiKey: 'sk-test' }, + sessionId: 's1', + sessions: new SessionManager({ root: tmpdir() }), + usage: { inputTokens: 0, outputTokens: 0, reasoningTokens: 0, cacheReadTokens: 0 }, + ...overrides, + }; +} + +const run = async (args: string[], c: SessionContext): Promise => + (await reg.match('/shells')!.cmd.run(args, c)).join('\n'); + +describe('/shells', () => { + it('says what opens a shell when none are open', async () => { + const out = await run([], ctx({ shells: registry() })); + expect(out).toContain('No persistent shells open'); + expect(out).toContain('ShellOpen'); + }); + + it('lists an open shell with where it started', async () => { + const shells = registry(); + const id = await shells.open({ cwd: tmpdir() }); + const out = await run([], ctx({ shells })); + expect(out).toContain(id); + expect(out).toContain(tmpdir()); + expect(out).toContain('close when this session ends'); + }); + + it('closes a shell for real, not just from the listing', async () => { + const shells = registry(); + const id = await shells.open({ cwd: tmpdir() }); + expect(await run(['close', id], ctx({ shells }))).toContain(`Closed ${id}`); + expect(shells.get(id)).toBeUndefined(); + expect(shells.list()).toEqual([]); + }); + + it('reports an unknown id instead of claiming it closed something', async () => { + const out = await run(['close', 'shell-999'], ctx({ shells: registry() })); + expect(out).toContain('No open shell'); + }); + + it('asks for an id when close is given none', async () => { + expect(await run(['close'], ctx({ shells: registry() }))).toContain('Usage:'); + }); + + it('rejects an unrecognised argument rather than silently listing', async () => { + const out = await run(['kill', 'shell-1'], ctx({ shells: registry() })); + expect(out).toContain('Unknown argument'); + }); + + it('says so when the host owns no registry', async () => { + expect(await run([], ctx())).toContain('unavailable'); + }); +}); diff --git a/docs/BEHAVIOR_PARITY.md b/docs/BEHAVIOR_PARITY.md index 67b16f1..f96f176 100644 --- a/docs/BEHAVIOR_PARITY.md +++ b/docs/BEHAVIOR_PARITY.md @@ -52,6 +52,7 @@ Legend: `✅` matches · `🟡` matches with caveats · `🔄` deferred · `⚠ | `/background` | ✓ | ✓ | ✅ — runs a prompt as a background sub-agent via the session TaskManager (alias `/bg`); agent-started TaskCreate tasks appear too | | `/batch` | ✓ | ✗ | 🔄 — batch-of-prompts not yet wired (use `/background` per prompt) | | `/tasks` | ✓ | ✓ | ✅ — lists this session's background tasks; `/tasks ` shows one's status + output | +| `/shells` | ✗ | ✓ | 🆕 DeepCode-only — lists this session's persistent shells; `/shells close ` closes one | | `/plan` | ✓ | ✗ | 🔄 — set via `/mode plan` in DeepCode | | `/login` / `/logout` | ✓ | ✓ | ✅ — /logout clears creds + exits; /login stores a new key (next launch) | | `/export` | ✓ | ✓ | ✅ — writes the conversation to a markdown file | diff --git a/packages/core/src/agent.test.ts b/packages/core/src/agent.test.ts index 001e1e0..8648416 100644 --- a/packages/core/src/agent.test.ts +++ b/packages/core/src/agent.test.ts @@ -1501,6 +1501,26 @@ describe('runAgent', () => { }); describe('persistent shells', () => { + /** The built-in registry plus one extra tool, so `Task` is still there. */ + function withBuiltins(extra: ToolHandler): ToolRegistry { + const tools = new ToolRegistry(); + tools.register(extra); + return tools; + } + + /** Tool results from a run, keyed by the call id that produced them. */ + function toolResults(history: StoredMessage[]): Map { + const out = new Map(); + for (const msg of history) { + for (const block of msg.content) { + if (typeof block !== 'string' && block.type === 'tool_result') { + out.set(block.tool_use_id, block.content); + } + } + } + return out; + } + 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 @@ -1555,6 +1575,59 @@ describe('runAgent', () => { expect(seen?.list()).toEqual([]); }); + it("does not hand a sub-agent the parent session's shells", async () => { + // The REPL owns one registry for the whole session. A delegated agent + // sharing it could `cd` or close a shell the parent is mid-way through + // using, and each would then be wrong about its state. Sub-agents get + // their own, closed when their run ends. + const { ShellRegistry } = await import('./shell/registry.js'); + const parentShells = new ShellRegistry(); + let subShells: unknown = 'never ran'; + + const peek: ToolHandler = { + name: 'Peek', + definition: { + name: 'Peek', + description: 'reports the registry it was given', + inputSchema: { type: 'object', properties: {} }, + }, + execute: (_input, toolCtx) => { + subShells = toolCtx.shells; + return Promise.resolve({ content: 'peeked' }); + }, + }; + + const result = await runAgent({ + provider: new MockProvider([ + toolUse('delegating', { + type: 'tool_use', + id: 'task1', + name: 'Task', + input: { prompt: 'peek at the shells' }, + }), + toolUse('peeking', { type: 'tool_use', id: 'p1', name: 'Peek', input: {} }), + endTurn('peeked'), + endTurn('done'), + ]), + // Built-ins plus Peek: `new ToolRegistry([peek])` would replace them and + // there would be no Task tool to delegate through. + tools: withBuiltins(peek), + systemPrompt: '', + userMessage: 'go', + model: 'deepseek-chat', + cwd, + shells: parentShells, + }); + + // The delegation has to have actually happened, or `Peek` ran in the + // parent and the assertion below would pass for the wrong reason. + expect(toolResults(result.history).get('task1')).not.toMatch(/tool not found/); + + expect(subShells).toBeDefined(); + expect(subShells).not.toBe(parentShells); + await parentShells.closeAll(); + }); + 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.