Skip to content

Commit 1df0b68

Browse files
committed
Agent mode phase 2: persistent, interactive terminals
Agent mode's only process-management tool (start_background_command) spawns with stdio: ["ignore", ...] — there's no way to send input to a running process, so answering an interactive prompt or driving a REPL was impossible; output was poll-only, never streamed. - New app/src/terminal-manager.ts, backed by node-pty (real pseudo-terminal, not a plain pipe): create/write/resize/close/list, workspace-scoped cleanup wired into the same lifecycle hooks phase 0 built for background tasks (agent:closeWorkspace, window-all-closed, before-quit). Reuses phase 1's command-sandbox.ts for filesystem confinement where the platform supports it, defaulting to network allowed (unlike run_command's per-call default) since this is an interactive session a human can see and type into directly, not a one-shot command the model composed unsupervised. - Two separate paths reach it, deliberately: a human opening the terminal panel goes through a dedicated terminal:create IPC channel with live push-streamed output (mirrors the existing chat:chunk streaming pattern) and never touches tool approval, since the human is driving their own keystrokes. The model gets four new agent tools — create_terminal, write_to_terminal, read_terminal_output, close_terminal — that poll instead of stream, going through the normal approval-card pipeline like every other tool. Kept start_background_command's existing four tools unchanged rather than folding them into this, so no schema break for prompts/evals that already know them. - New frontend/src/components/terminal-panel.tsx (xterm.js + fit addon): one mounted terminal instance, tab-switching replays each session's buffered output rather than keeping N instances alive at once. - Extracted resolveSafePath out of agent-tools.ts into workspace-path.ts so terminal-manager.ts could reuse the same workspace-containment logic without agent-tools.ts <-> terminal-manager.ts becoming a circular import (agent-tools.ts's own new create_terminal/write_to_terminal/etc tool cases need to import terminal-manager.ts, which already needed resolveSafePath). New dependencies: node-pty (main, native — added to asarUnpack alongside the other native deps; electron-builder rebuilds it for Electron's ABI automatically, same as the existing node-llama-cpp dependency already relies on), @xterm/xterm + @xterm/addon-fit (renderer). 17 new tests across terminal-manager.test.ts and 3 new agent-tools.test.ts cases for the tool-call dispatch path, all exercising a real spawned pty rather than mocking node-pty.
1 parent 948bbe0 commit 1df0b68

16 files changed

Lines changed: 754 additions & 40 deletions

app/package-lock.json

Lines changed: 17 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

app/package.json

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,8 @@
3333
"**/node_modules/tesseract.js/**",
3434
"**/node_modules/tesseract.js-core/**",
3535
"**/node_modules/node-llama-cpp/**",
36-
"**/node_modules/@node-llama-cpp/**"
36+
"**/node_modules/@node-llama-cpp/**",
37+
"**/node_modules/node-pty/**"
3738
],
3839
"extraResources": [
3940
{
@@ -91,6 +92,7 @@
9192
"electron-updater": "^6.6.2",
9293
"ffmpeg-static": "^5.3.0",
9394
"node-llama-cpp": "^3.19.1",
95+
"node-pty": "^1.1.0",
9496
"pdf-parse": "^2.4.5",
9597
"pidusage": "^4.0.1",
9698
"tesseract.js": "^5.1.1"

app/src/agent-tools.test.ts

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import * as fs from "node:fs";
33
import * as os from "node:os";
44
import * as path from "node:path";
55
import { execSync } from "node:child_process";
6+
import { closeAll as closeAllTerminals } from "./terminal-manager";
67
import {
78
readFile,
89
writeFile,
@@ -393,6 +394,48 @@ describe("agent-tools", () => {
393394
});
394395
});
395396

397+
describe("terminal tools", () => {
398+
afterEach(() => {
399+
closeAllTerminals();
400+
});
401+
402+
function waitFor(predicate: () => boolean | Promise<boolean>, timeoutMs = 5000): Promise<void> {
403+
const start = Date.now();
404+
return new Promise((resolve, reject) => {
405+
const check = async () => {
406+
if (await predicate()) return resolve();
407+
if (Date.now() - start > timeoutMs) return reject(new Error("timed out waiting"));
408+
setTimeout(check, 20);
409+
};
410+
check();
411+
});
412+
}
413+
414+
it("creates a terminal, writes a command to it, and reads the output back", async () => {
415+
const created = (await executeTool(workspace, "create_terminal", { name: "test" })) as { id: string; name: string };
416+
expect(created.id).toBeTruthy();
417+
418+
await executeTool(workspace, "write_to_terminal", { terminal_id: created.id, input: "echo from-agent-terminal\r" });
419+
420+
await waitFor(async () => {
421+
const output = (await executeTool(workspace, "read_terminal_output", { terminal_id: created.id })) as string;
422+
return output.includes("from-agent-terminal");
423+
});
424+
const output = (await executeTool(workspace, "read_terminal_output", { terminal_id: created.id })) as string;
425+
expect(output).toContain("from-agent-terminal");
426+
});
427+
428+
it("closes a terminal so further reads/writes fail", async () => {
429+
const created = (await executeTool(workspace, "create_terminal", {})) as { id: string; name: string };
430+
await executeTool(workspace, "close_terminal", { terminal_id: created.id });
431+
await expect(executeTool(workspace, "read_terminal_output", { terminal_id: created.id })).rejects.toThrow(/No terminal/);
432+
});
433+
434+
it("throws for an unknown terminal id", async () => {
435+
await expect(executeTool(workspace, "write_to_terminal", { terminal_id: "nope", input: "x" })).rejects.toThrow(/No terminal/);
436+
});
437+
});
438+
396439
describe("run_command", () => {
397440
it("captures stdout and a zero exit code from a successful command", async () => {
398441
const output = await executeTool(workspace, "run_command", { command: "echo hello" });

app/src/agent-tools.ts

Lines changed: 70 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ import { killProcessTree } from "./process-tree";
1111
import { applySandbox } from "./command-sandbox";
1212
import { monitorProcess } from "./resource-monitor";
1313
import * as settingsStore from "./settings-store";
14+
import { resolveSafePath } from "./workspace-path";
15+
import * as terminalManager from "./terminal-manager";
1416

1517
const execAsync = promisify(exec);
1618

@@ -202,6 +204,54 @@ export const AGENT_TOOLS: ToolDefinition[] = [
202204
description: "List all background commands from this session with their status.",
203205
parameters: { type: "object", properties: {}, required: [] },
204206
},
207+
{
208+
name: "create_terminal",
209+
description:
210+
"Open a real interactive shell (a pseudo-terminal) in the workspace and return its id. Unlike run_command/start_background_command, this can be driven interactively over multiple turns — write_to_terminal to send input (including to a program already waiting for it, e.g. a REPL or a confirmation prompt), read_terminal_output to see what happened. Use this instead of start_background_command when you need to send input after the process has already started, not just read its output.",
211+
parameters: {
212+
type: "object",
213+
properties: {
214+
name: { type: "string", description: "Short human-readable label (e.g. \"debug session\")." },
215+
cwd: { type: "string", description: 'Working directory, relative to the workspace root. Defaults to "."' },
216+
},
217+
required: [],
218+
},
219+
},
220+
{
221+
name: "write_to_terminal",
222+
description: "Send input to a terminal opened with create_terminal, as if typed at the keyboard. Include \\n (or \\r) to press Enter and actually run a typed command — without it, the text is typed but not submitted.",
223+
parameters: {
224+
type: "object",
225+
properties: {
226+
terminal_id: { type: "string", description: "The id returned by create_terminal." },
227+
input: { type: "string", description: "The text to send." },
228+
},
229+
required: ["terminal_id", "input"],
230+
},
231+
},
232+
{
233+
name: "read_terminal_output",
234+
description: "Read the recent output of a terminal opened with create_terminal.",
235+
parameters: {
236+
type: "object",
237+
properties: {
238+
terminal_id: { type: "string", description: "The id returned by create_terminal." },
239+
tail_chars: { type: "number", description: "How many characters of recent output to return, from the end. Defaults to 4000." },
240+
},
241+
required: ["terminal_id"],
242+
},
243+
},
244+
{
245+
name: "close_terminal",
246+
description: "Close a terminal opened with create_terminal, ending its shell process.",
247+
parameters: {
248+
type: "object",
249+
properties: {
250+
terminal_id: { type: "string", description: "The id returned by create_terminal." },
251+
},
252+
required: ["terminal_id"],
253+
},
254+
},
205255
{
206256
name: "git_status",
207257
description: "Show the working tree status (git status) for the workspace.",
@@ -415,38 +465,6 @@ const MAX_COMMAND_OUTPUT_CHARS = 50_000;
415465
const COMMAND_TIMEOUT_MS = 60_000;
416466
const IGNORED_DIRS = new Set(["node_modules", ".git", "dist", "build", "out", "release", "__pycache__"]);
417467

418-
// Every tool call is confined to the chosen workspace directory — this
419-
// resolves the (possibly relative, possibly attacker-crafted via a prompt
420-
// injection in file content the model read) path and throws if it would
421-
// escape that directory via ../ or an absolute path elsewhere on disk.
422-
function resolveSafePath(workspaceRoot: string, relativePath: string): string {
423-
const root = path.resolve(workspaceRoot);
424-
const resolved = path.resolve(root, relativePath || ".");
425-
const isWithin = (parent: string, child: string): boolean => {
426-
const relative = path.relative(parent, child);
427-
return relative === "" || (!relative.startsWith(`..${path.sep}`) && relative !== ".." && !path.isAbsolute(relative));
428-
};
429-
if (!isWithin(root, resolved)) {
430-
throw new Error(`Path "${relativePath}" is outside the workspace directory.`);
431-
}
432-
433-
// Lexical checks alone are bypassable through a symlink inside the
434-
// workspace that points elsewhere. Resolve the target, or its nearest
435-
// existing parent for new files, and verify the real path too.
436-
const realRoot = fs.realpathSync(root);
437-
let existing = resolved;
438-
while (!fs.existsSync(existing)) {
439-
const parent = path.dirname(existing);
440-
if (parent === existing) break;
441-
existing = parent;
442-
}
443-
const realExisting = fs.realpathSync(existing);
444-
if (!isWithin(realRoot, realExisting)) {
445-
throw new Error(`Path "${relativePath}" resolves outside the workspace directory through a symbolic link.`);
446-
}
447-
return resolved;
448-
}
449-
450468
export function readFile(workspaceRoot: string, relativePath: string, startLine?: number, endLine?: number): string {
451469
const target = resolveSafePath(workspaceRoot, relativePath);
452470
const stat = fs.statSync(target);
@@ -1467,6 +1485,26 @@ export async function executeTool(workspaceRoot: string, name: string, args: Rec
14671485
return stopBackgroundCommand(String(args.task_id ?? ""));
14681486
case "list_background_commands":
14691487
return listBackgroundCommands();
1488+
case "create_terminal":
1489+
// No-op streaming callbacks: the model drives this by polling
1490+
// read_terminal_output rather than receiving push events — live
1491+
// streaming is reserved for the human-facing terminal panel,
1492+
// which goes through the dedicated terminal:create IPC channel
1493+
// instead of this tool-call path.
1494+
return terminalManager.createTerminal(
1495+
workspaceRoot,
1496+
{ name: args.name ? String(args.name) : undefined, cwd: args.cwd ? String(args.cwd) : undefined },
1497+
() => {},
1498+
() => {}
1499+
);
1500+
case "write_to_terminal":
1501+
terminalManager.writeToTerminal(String(args.terminal_id ?? ""), String(args.input ?? ""));
1502+
return { ok: true };
1503+
case "read_terminal_output":
1504+
return terminalManager.readTerminalOutput(String(args.terminal_id ?? ""), typeof args.tail_chars === "number" ? args.tail_chars : undefined);
1505+
case "close_terminal":
1506+
terminalManager.closeTerminal(String(args.terminal_id ?? ""));
1507+
return { ok: true };
14701508
case "git_status":
14711509
return gitStatus(workspaceRoot);
14721510
case "git_diff":

app/src/command-sandbox.ts

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,34 @@ function commandExists(cmd: string): boolean {
2424
}
2525
}
2626

27-
// `hasCommand` is injectable so tests can simulate "bwrap is/isn't on PATH"
27+
// bwrap being on PATH isn't sufficient on its own — modern Ubuntu (23.10+)
28+
// restricts unprivileged user-namespace creation by default (AppArmor
29+
// policy), which makes bwrap fail at runtime with a permission error even
30+
// when it's installed. A plain `which bwrap` check wouldn't catch that and
31+
// would report false confidence, so this actually runs a trivial sandboxed
32+
// no-op and checks it really works.
33+
function canUseBubblewrap(): boolean {
34+
if (!commandExists("bwrap")) return false;
35+
try {
36+
execFileSync("bwrap", ["--unshare-all", "--dev", "/dev", "--proc", "/proc", "true"], {
37+
stdio: "ignore",
38+
timeout: 5000,
39+
});
40+
return true;
41+
} catch {
42+
return false;
43+
}
44+
}
45+
46+
function defaultAvailabilityCheck(cmd: string): boolean {
47+
return cmd === "bwrap" ? canUseBubblewrap() : commandExists(cmd);
48+
}
49+
50+
// `hasCommand` is injectable so tests can simulate "bwrap is/isn't usable"
2851
// without actually shelling out.
2952
export function detectSandboxCapabilities(
3053
platform: NodeJS.Platform = process.platform,
31-
hasCommand: (cmd: string) => boolean = commandExists
54+
hasCommand: (cmd: string) => boolean = defaultAvailabilityCheck
3255
): SandboxCapabilities {
3356
if (platform === "linux") {
3457
if (hasCommand("bwrap")) return { filesystemConfinement: true, networkDenial: true, mechanism: "bubblewrap" };
@@ -65,7 +88,7 @@ export function wrapCommand(
6588
command: string,
6689
opts: WrapCommandOptions,
6790
platform: NodeJS.Platform = process.platform,
68-
hasCommand: (cmd: string) => boolean = commandExists
91+
hasCommand: (cmd: string) => boolean = defaultAvailabilityCheck
6992
): WrappedCommand | null {
7093
const caps = detectSandboxCapabilities(platform, hasCommand);
7194
const root = path.resolve(opts.workspaceRoot);
@@ -144,7 +167,7 @@ export function applySandbox(
144167
command: string,
145168
opts: WrapCommandOptions,
146169
platform: NodeJS.Platform = process.platform,
147-
hasCommand: (cmd: string) => boolean = commandExists
170+
hasCommand: (cmd: string) => boolean = defaultAvailabilityCheck
148171
): string {
149172
const wrapped = wrapCommand(command, opts, platform, hasCommand);
150173
if (!wrapped) return command;

app/src/main.ts

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import * as dataTransfer from "./data-transfer";
1515
import * as rag from "./rag";
1616
import * as agentTools from "./agent-tools";
1717
import { detectSandboxCapabilities } from "./command-sandbox";
18+
import * as terminalManager from "./terminal-manager";
1819
import * as mcpClient from "./mcp-client";
1920
import * as figma from "./figma";
2021
import * as ocr from "./ocr";
@@ -763,11 +764,50 @@ function registerIpcHandlers(): void {
763764
ipcMain.handle("agent:closeWorkspace", (_event: IpcMainInvokeEvent, workspaceRoot: string) => {
764765
requireString(workspaceRoot, "workspace root");
765766
const killedBackgroundTasks = agentTools.killBackgroundCommandsForWorkspace(workspaceRoot);
766-
return { killedBackgroundTasks };
767+
const killedTerminals = terminalManager.closeAllForWorkspace(workspaceRoot);
768+
return { killedBackgroundTasks, killedTerminals };
767769
});
768770

769771
ipcMain.handle("agent:getSandboxCapabilities", () => detectSandboxCapabilities());
770772

773+
ipcMain.handle(
774+
"terminal:create",
775+
(
776+
event: IpcMainInvokeEvent,
777+
{ workspaceRoot, opts }: { workspaceRoot: string; opts?: { cwd?: string; name?: string } }
778+
) => {
779+
requireString(workspaceRoot, "workspace root");
780+
// `id` is referenced inside these callbacks before it's assigned
781+
// below, but that's safe: node-pty never calls onData/onExit
782+
// synchronously during createTerminal() itself, only later once
783+
// its own async event loop runs — by which point `id` is set.
784+
const { id, name } = terminalManager.createTerminal(
785+
workspaceRoot,
786+
opts ?? {},
787+
(chunk) => event.sender.send(`terminal:data:${id}`, chunk),
788+
(exitCode) => event.sender.send(`terminal:exit:${id}`, exitCode)
789+
);
790+
return { id, name };
791+
}
792+
);
793+
794+
ipcMain.handle("terminal:write", (_event: IpcMainInvokeEvent, { id, data }: { id: string; data: string }) => {
795+
requireString(id, "terminal id");
796+
terminalManager.writeToTerminal(id, typeof data === "string" ? data : "");
797+
});
798+
799+
ipcMain.handle("terminal:resize", (_event: IpcMainInvokeEvent, { id, cols, rows }: { id: string; cols: number; rows: number }) => {
800+
requireString(id, "terminal id");
801+
terminalManager.resizeTerminal(id, Number(cols) || 80, Number(rows) || 24);
802+
});
803+
804+
ipcMain.handle("terminal:close", (_event: IpcMainInvokeEvent, id: string) => {
805+
requireString(id, "terminal id");
806+
terminalManager.closeTerminal(id);
807+
});
808+
809+
ipcMain.handle("terminal:list", (_event: IpcMainInvokeEvent, workspaceRoot?: string) => terminalManager.listTerminals(workspaceRoot));
810+
771811
ipcMain.handle("mcp:connect", async (_event: IpcMainInvokeEvent, config: McpServerConfig) => {
772812
try {
773813
const { tools } = await mcpClient.connectServer(config);
@@ -824,6 +864,7 @@ app.on("window-all-closed", () => {
824864
ollama.stop();
825865
localServers.stopAll();
826866
agentTools.killAllBackgroundCommands();
867+
terminalManager.closeAll();
827868
mcpClient.disconnectAll();
828869
if (process.platform !== "darwin") app.quit();
829870
});
@@ -832,5 +873,6 @@ app.on("before-quit", () => {
832873
ollama.stop();
833874
localServers.stopAll();
834875
agentTools.killAllBackgroundCommands();
876+
terminalManager.closeAll();
835877
void llamacpp.dispose();
836878
});

0 commit comments

Comments
 (0)