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.