From 78e19d388cc113f40983f25e66a2aae3b1364544 Mon Sep 17 00:00:00 2001 From: ethan Date: Tue, 25 Nov 2025 16:14:09 +1100 Subject: [PATCH 01/13] =?UTF-8?q?=F0=9F=A4=96=20feat:=20add=20background?= =?UTF-8?q?=20bash=20process=20execution?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds ability to run long-running bash processes in the background without blocking the AI conversation. Useful for builds, test suites, dev servers, and other processes that take longer than typical tool timeouts. Core components: - CircularBuffer: O(1) ring buffer for stdout/stderr (1000 lines each) - BackgroundProcessManager: lifecycle management with spawn/list/terminate/cleanup - Process IDs use 'bg-{8-hex-chars}' format for uniqueness New tools: - bash tool: enhanced with run_in_background parameter - bash_background_read: check status and retrieve buffered output - bash_background_list: discover processes after context loss - bash_background_terminate: graceful shutdown (SIGTERM → SIGKILL) Features: - Automatic cleanup on workspace deletion - Output buffers preserved after process exit - Filtering support in read tool (tail, regex) - Workspace isolation (processes scoped to workspace) Test coverage: 84 tests across all components --- .../RightSidebar/CodeReview/ReviewPanel.tsx | 3 +- src/common/types/tools.ts | 57 +++++ src/common/utils/tools/toolDefinitions.ts | 66 +++++ src/common/utils/tools/tools.ts | 11 + src/node/services/aiService.ts | 8 +- .../services/backgroundProcessManager.test.ts | 202 +++++++++++++++ src/node/services/backgroundProcessManager.ts | 227 +++++++++++++++++ src/node/services/circularBuffer.test.ts | 86 +++++++ src/node/services/circularBuffer.ts | 77 ++++++ src/node/services/ipcMain.ts | 12 +- src/node/services/tools/bash.test.ts | 153 ++++++++++-- src/node/services/tools/bash.ts | 65 ++++- .../tools/bash_background_list.test.ts | 133 ++++++++++ .../services/tools/bash_background_list.ts | 44 ++++ .../tools/bash_background_read.test.ts | 233 ++++++++++++++++++ .../services/tools/bash_background_read.ts | 98 ++++++++ .../tools/bash_background_terminate.test.ts | 178 +++++++++++++ .../tools/bash_background_terminate.ts | 48 ++++ src/node/services/tools/status_set.test.ts | 1 + src/node/services/tools/testHelpers.ts | 3 +- 20 files changed, 1684 insertions(+), 21 deletions(-) create mode 100644 src/node/services/backgroundProcessManager.test.ts create mode 100644 src/node/services/backgroundProcessManager.ts create mode 100644 src/node/services/circularBuffer.test.ts create mode 100644 src/node/services/circularBuffer.ts create mode 100644 src/node/services/tools/bash_background_list.test.ts create mode 100644 src/node/services/tools/bash_background_list.ts create mode 100644 src/node/services/tools/bash_background_read.test.ts create mode 100644 src/node/services/tools/bash_background_read.ts create mode 100644 src/node/services/tools/bash_background_terminate.test.ts create mode 100644 src/node/services/tools/bash_background_terminate.ts diff --git a/src/browser/components/RightSidebar/CodeReview/ReviewPanel.tsx b/src/browser/components/RightSidebar/CodeReview/ReviewPanel.tsx index d1f21557e..a0335b6bc 100644 --- a/src/browser/components/RightSidebar/CodeReview/ReviewPanel.tsx +++ b/src/browser/components/RightSidebar/CodeReview/ReviewPanel.tsx @@ -269,7 +269,8 @@ export const ReviewPanel: React.FC = ({ } const diffOutput = diffResult.data.output ?? ""; - const truncationInfo = diffResult.data.truncated; + const truncationInfo = + "truncated" in diffResult.data ? diffResult.data.truncated : undefined; const fileDiffs = parseDiff(diffOutput); const allHunks = extractAllHunks(fileDiffs); diff --git a/src/common/types/tools.ts b/src/common/types/tools.ts index fc71d350c..8d1454d4f 100644 --- a/src/common/types/tools.ts +++ b/src/common/types/tools.ts @@ -7,6 +7,7 @@ export interface BashToolArgs { script: string; timeout_secs?: number; // Optional: defaults to 3 seconds for interactivity + run_in_background?: boolean; // Run without blocking (for long-running processes) } interface CommonBashFields { @@ -26,6 +27,12 @@ export type BashToolResult = totalLines: number; }; }) + | (CommonBashFields & { + success: true; + output: string; + exitCode: 0; + backgroundProcessId: string; // Background spawn succeeded + }) | (CommonBashFields & { success: false; output?: string; @@ -190,6 +197,56 @@ export interface StatusSetToolArgs { url?: string; } +// Bash Background Tool Types +export interface BashBackgroundReadArgs { + process_id: string; + stdout_tail?: number; // Last N lines of stdout + stderr_tail?: number; // Last N lines of stderr + stdout_regex?: string; // Filter stdout by regex + stderr_regex?: string; // Filter stderr by regex +} + +export type BashBackgroundReadResult = + | { + success: true; + process_id: string; + status: "running" | "exited" | "killed" | "failed"; + script: string; + pid?: number; // OS process ID + uptime_ms: number; + exitCode?: number; + stdout: string[]; + stderr: string[]; + } + | { + success: false; + error: string; + }; + +export interface BashBackgroundTerminateArgs { + process_id: string; +} + +export type BashBackgroundTerminateResult = + | { success: true; message: string } + | { success: false; error: string }; + +// Bash Background List Tool Types +export type BashBackgroundListArgs = Record; + +export interface BashBackgroundListProcess { + process_id: string; + status: "running" | "exited" | "killed" | "failed"; + script: string; + pid?: number; + uptime_ms: number; + exitCode?: number; +} + +export type BashBackgroundListResult = + | { success: true; processes: BashBackgroundListProcess[] } + | { success: false; error: string }; + export type StatusSetToolResult = | { success: true; diff --git a/src/common/utils/tools/toolDefinitions.ts b/src/common/utils/tools/toolDefinitions.ts index 66e180ab3..7b1991296 100644 --- a/src/common/utils/tools/toolDefinitions.ts +++ b/src/common/utils/tools/toolDefinitions.ts @@ -52,6 +52,19 @@ export const TOOL_DEFINITIONS = { .describe( `Timeout (seconds, default: ${BASH_DEFAULT_TIMEOUT_SECS}). Start small and increase on retry; avoid large initial values to keep UX responsive` ), + run_in_background: z + .boolean() + .default(false) + .describe( + "Run this command in the background without blocking. " + + "Use for processes running >5s (dev servers, builds, file watchers). " + + "Do NOT use for quick commands (<5s), interactive processes (no stdin support), " + + "or processes requiring real-time output (use foreground with larger timeout instead). " + + "Returns immediately with process ID for status checking and termination. " + + "Output is buffered (max 1000 lines per stream, oldest evicted when full). " + + "Poll frequently with bash_background_read for high-output processes. " + + "Process persists across tool calls until terminated or workspace is removed." + ), }), }, file_read: { @@ -229,6 +242,56 @@ export const TOOL_DEFINITIONS = { }) .strict(), }, + bash_background_read: { + description: + "Check status and read output from a background bash process. " + + "Use this to inspect long-running processes started with run_in_background=true. " + + "Supports filtering output with tail and regex options.", + schema: z.object({ + process_id: z + .string() + .regex(/^bg-[0-9a-f]{8}$/, "Invalid process ID format") + .describe("Background process ID returned from bash tool"), + stdout_tail: z + .number() + .int() + .positive() + .optional() + .describe("Return last N lines of stdout (default: all buffered)"), + stderr_tail: z + .number() + .int() + .positive() + .optional() + .describe("Return last N lines of stderr (default: all buffered)"), + stdout_regex: z + .string() + .optional() + .describe("Filter stdout lines by regex pattern (applied before tail)"), + stderr_regex: z + .string() + .optional() + .describe("Filter stderr lines by regex pattern (applied before tail)"), + }), + }, + bash_background_list: { + description: + "List all background processes for the current workspace. " + + "Useful for discovering running processes after context loss or resuming a conversation.", + schema: z.object({}), + }, + bash_background_terminate: { + description: + "Terminate a background bash process. " + + "Sends SIGTERM, waits briefly, then sends SIGKILL if needed. " + + "Process output remains available for inspection after termination.", + schema: z.object({ + process_id: z + .string() + .regex(/^bg-[0-9a-f]{8}$/, "Invalid process ID format") + .describe("Background process ID to terminate"), + }), + }, web_fetch: { description: `Fetch a web page and extract its main content as clean markdown. ` + @@ -271,6 +334,9 @@ export function getAvailableTools(modelString: string): string[] { // Base tools available for all models const baseTools = [ "bash", + "bash_background_read", + "bash_background_list", + "bash_background_terminate", "file_read", "file_edit_replace_string", // "file_edit_replace_lines", // DISABLED: causes models to break repo state diff --git a/src/common/utils/tools/tools.ts b/src/common/utils/tools/tools.ts index 873e6a8c3..3d6b11949 100644 --- a/src/common/utils/tools/tools.ts +++ b/src/common/utils/tools/tools.ts @@ -1,6 +1,9 @@ import { type Tool } from "ai"; import { createFileReadTool } from "@/node/services/tools/file_read"; import { createBashTool } from "@/node/services/tools/bash"; +import { createBashBackgroundReadTool } from "@/node/services/tools/bash_background_read"; +import { createBashBackgroundListTool } from "@/node/services/tools/bash_background_list"; +import { createBashBackgroundTerminateTool } from "@/node/services/tools/bash_background_terminate"; import { createFileEditReplaceStringTool } from "@/node/services/tools/file_edit_replace_string"; // DISABLED: import { createFileEditReplaceLinesTool } from "@/node/services/tools/file_edit_replace_lines"; import { createFileEditInsertTool } from "@/node/services/tools/file_edit_insert"; @@ -12,6 +15,7 @@ import { log } from "@/node/services/log"; import type { Runtime } from "@/node/runtime/Runtime"; import type { InitStateManager } from "@/node/services/initStateManager"; +import type { BackgroundProcessManager } from "@/node/services/backgroundProcessManager"; /** * Configuration for tools that need runtime context @@ -29,6 +33,10 @@ export interface ToolConfiguration { runtimeTempDir: string; /** Overflow policy for bash tool output (optional, not exposed to AI) */ overflow_policy?: "truncate" | "tmpfile"; + /** Background process manager for bash tool (optional, AI-only) */ + backgroundProcessManager?: BackgroundProcessManager; + /** Workspace ID for tracking background processes (optional for token estimation) */ + workspaceId?: string; } /** @@ -99,6 +107,9 @@ export async function getToolsForModel( // and line number miscalculations. Use file_edit_replace_string instead. // file_edit_replace_lines: wrap(createFileEditReplaceLinesTool(config)), bash: wrap(createBashTool(config)), + bash_background_read: wrap(createBashBackgroundReadTool(config)), + bash_background_list: wrap(createBashBackgroundListTool(config)), + bash_background_terminate: wrap(createBashBackgroundTerminateTool(config)), web_fetch: wrap(createWebFetchTool(config)), }; diff --git a/src/node/services/aiService.ts b/src/node/services/aiService.ts index 6cb748f91..53b9cae38 100644 --- a/src/node/services/aiService.ts +++ b/src/node/services/aiService.ts @@ -20,6 +20,7 @@ import { getToolsForModel } from "@/common/utils/tools/tools"; import { createRuntime } from "@/node/runtime/runtimeFactory"; import { secretsToRecord } from "@/common/types/secrets"; import type { MuxProviderOptions } from "@/common/types/providerOptions"; +import type { BackgroundProcessManager } from "@/node/services/backgroundProcessManager"; import { log } from "./log"; import { transformModelMessages, @@ -141,12 +142,14 @@ export class AIService extends EventEmitter { private readonly initStateManager: InitStateManager; private readonly mockModeEnabled: boolean; private readonly mockScenarioPlayer?: MockScenarioPlayer; + private readonly backgroundProcessManager?: BackgroundProcessManager; constructor( config: Config, historyService: HistoryService, partialService: PartialService, - initStateManager: InitStateManager + initStateManager: InitStateManager, + backgroundProcessManager?: BackgroundProcessManager ) { super(); // Increase max listeners to accommodate multiple concurrent workspace listeners @@ -156,6 +159,7 @@ export class AIService extends EventEmitter { this.historyService = historyService; this.partialService = partialService; this.initStateManager = initStateManager; + this.backgroundProcessManager = backgroundProcessManager; this.streamManager = new StreamManager(historyService, partialService); void this.ensureSessionsDir(); this.setupStreamEventForwarding(); @@ -762,6 +766,8 @@ export class AIService extends EventEmitter { runtime, secrets: secretsToRecord(projectSecrets), runtimeTempDir, + backgroundProcessManager: this.backgroundProcessManager, + workspaceId, }, workspaceId, this.initStateManager, diff --git a/src/node/services/backgroundProcessManager.test.ts b/src/node/services/backgroundProcessManager.test.ts new file mode 100644 index 000000000..b05e80f01 --- /dev/null +++ b/src/node/services/backgroundProcessManager.test.ts @@ -0,0 +1,202 @@ +import { describe, it, expect, beforeEach } from "bun:test"; +import { BackgroundProcessManager } from "./backgroundProcessManager"; +import { BashExecutionService } from "./bashExecutionService"; + +describe("BackgroundProcessManager", () => { + let manager: BackgroundProcessManager; + let bashService: BashExecutionService; + + beforeEach(() => { + bashService = new BashExecutionService(); + manager = new BackgroundProcessManager(bashService); + }); + + describe("spawn", () => { + it("should spawn a background process and return process ID", async () => { + const result = await manager.spawn("workspace-1", "echo hello", { + cwd: process.cwd(), + }); + + expect(result.success).toBe(true); + if (result.success) { + expect(result.processId).toMatch(/^bg-/); + } + }); + + it("should return error on spawn failure", async () => { + const result = await manager.spawn("workspace-1", "echo test", { + cwd: "/nonexistent/path/that/does/not/exist", + }); + + expect(result.success).toBe(false); + }); + + it("should capture stdout and stderr", async () => { + const result = await manager.spawn("workspace-1", "echo hello; echo world >&2", { + cwd: process.cwd(), + }); + + expect(result.success).toBe(true); + if (result.success) { + // Wait a moment for output to be captured + await new Promise((resolve) => setTimeout(resolve, 100)); + + const process = manager.getProcess(result.processId); + expect(process).not.toBeNull(); + expect(process?.stdoutBuffer.toArray()).toContain("hello"); + expect(process?.stderrBuffer.toArray()).toContain("world"); + } + }); + + it("should handle ring buffer overflow", async () => { + // Generate more than 1000 lines + const script = Array(1100) + .fill(0) + .map((_, i) => `echo line${i}`) + .join("; "); + + const result = await manager.spawn("workspace-1", script, { + cwd: process.cwd(), + }); + + expect(result.success).toBe(true); + if (result.success) { + await new Promise((resolve) => setTimeout(resolve, 500)); + + const process = manager.getProcess(result.processId); + expect(process).not.toBeNull(); + // Buffer should be capped at 1000 lines + expect(process!.stdoutBuffer.length).toBeLessThanOrEqual(1000); + } + }); + }); + + describe("getProcess", () => { + it("should return process by ID", async () => { + const spawnResult = await manager.spawn("workspace-1", "sleep 1", { + cwd: process.cwd(), + }); + + if (spawnResult.success) { + const process = manager.getProcess(spawnResult.processId); + expect(process).not.toBeNull(); + expect(process?.id).toBe(spawnResult.processId); + expect(process?.status).toBe("running"); + } + }); + + it("should return null for non-existent process", () => { + const process = manager.getProcess("bg-nonexistent"); + expect(process).toBeNull(); + }); + }); + + describe("list", () => { + it("should list all processes", async () => { + await manager.spawn("workspace-1", "sleep 1", { cwd: process.cwd() }); + await manager.spawn("workspace-1", "sleep 1", { cwd: process.cwd() }); + + const processes = manager.list(); + expect(processes.length).toBeGreaterThanOrEqual(2); + }); + + it("should filter by workspace ID", async () => { + await manager.spawn("workspace-1", "sleep 1", { cwd: process.cwd() }); + await manager.spawn("workspace-2", "sleep 1", { cwd: process.cwd() }); + + const ws1Processes = manager.list("workspace-1"); + const ws2Processes = manager.list("workspace-2"); + + expect(ws1Processes.length).toBeGreaterThanOrEqual(1); + expect(ws2Processes.length).toBeGreaterThanOrEqual(1); + expect(ws1Processes.every((p) => p.workspaceId === "workspace-1")).toBe(true); + expect(ws2Processes.every((p) => p.workspaceId === "workspace-2")).toBe(true); + }); + }); + + describe("terminate", () => { + it("should terminate a running process", async () => { + const spawnResult = await manager.spawn("workspace-1", "sleep 10", { + cwd: process.cwd(), + }); + + if (spawnResult.success) { + const terminateResult = await manager.terminate(spawnResult.processId); + expect(terminateResult.success).toBe(true); + + const process = manager.getProcess(spawnResult.processId); + expect(process?.status).toMatch(/killed|exited/); + } + }); + + it("should return error for non-existent process", async () => { + const result = await manager.terminate("bg-nonexistent"); + expect(result.success).toBe(false); + }); + + it("should be idempotent (double-terminate succeeds)", async () => { + const spawnResult = await manager.spawn("workspace-1", "sleep 10", { + cwd: process.cwd(), + }); + + if (spawnResult.success) { + const result1 = await manager.terminate(spawnResult.processId); + expect(result1.success).toBe(true); + + const result2 = await manager.terminate(spawnResult.processId); + expect(result2.success).toBe(true); + } + }); + }); + + describe("cleanup", () => { + it("should kill all processes for a workspace and remove them from memory", async () => { + await manager.spawn("workspace-1", "sleep 10", { cwd: process.cwd() }); + await manager.spawn("workspace-1", "sleep 10", { cwd: process.cwd() }); + await manager.spawn("workspace-2", "sleep 10", { cwd: process.cwd() }); + + await manager.cleanup("workspace-1"); + + const ws1Processes = manager.list("workspace-1"); + const ws2Processes = manager.list("workspace-2"); + + // All workspace-1 processes should be removed from memory + expect(ws1Processes.length).toBe(0); + // workspace-2 processes should still exist and be running + expect(ws2Processes.length).toBeGreaterThanOrEqual(1); + expect(ws2Processes.some((p) => p.status === "running")).toBe(true); + }); + }); + + describe("process state tracking", () => { + it("should track process exit", async () => { + const result = await manager.spawn("workspace-1", "exit 42", { + cwd: process.cwd(), + }); + + if (result.success) { + // Wait for process to exit + await new Promise((resolve) => setTimeout(resolve, 200)); + + const process = manager.getProcess(result.processId); + expect(process?.status).toBe("exited"); + expect(process?.exitCode).toBe(42); + expect(process?.exitTime).not.toBeNull(); + } + }); + + it("should keep buffer after process exits", async () => { + const result = await manager.spawn("workspace-1", "echo test; exit 0", { + cwd: process.cwd(), + }); + + if (result.success) { + await new Promise((resolve) => setTimeout(resolve, 200)); + + const process = manager.getProcess(result.processId); + expect(process?.status).toBe("exited"); + expect(process?.stdoutBuffer.toArray()).toContain("test"); + } + }); + }); +}); diff --git a/src/node/services/backgroundProcessManager.ts b/src/node/services/backgroundProcessManager.ts new file mode 100644 index 000000000..adaae7fda --- /dev/null +++ b/src/node/services/backgroundProcessManager.ts @@ -0,0 +1,227 @@ +import type { + BashExecutionService, + BashExecutionConfig, + DisposableProcess, +} from "./bashExecutionService"; +import { log } from "./log"; +import { randomBytes } from "crypto"; +import { once } from "node:events"; +import { CircularBuffer } from "./circularBuffer"; + +/** + * Represents a background process with buffered output + */ +export interface BackgroundProcess { + id: string; // Short unique ID (e.g., "bg-abc123") + workspaceId: string; // Owning workspace + script: string; // Original command + pid?: number; // Process ID (undefined if spawn failed) + startTime: number; // Timestamp when started + stdoutBuffer: CircularBuffer; // Circular buffer (max 1000 lines) + stderrBuffer: CircularBuffer; // Circular buffer (max 1000 lines) + exitCode?: number; // Undefined if still running + exitTime?: number; // Timestamp when exited (undefined if running) + status: "running" | "exited" | "killed" | "failed"; + disposable: DisposableProcess | null; // For process cleanup +} + +const MAX_BUFFER_LINES = 1000; + +/** + * Manages background bash processes for workspaces + */ +export class BackgroundProcessManager { + private processes = new Map(); + + constructor(private readonly bashExecutionService: BashExecutionService) {} + + /** + * Spawn a new background process + */ + async spawn( + workspaceId: string, + script: string, + config: BashExecutionConfig + ): Promise<{ success: true; processId: string } | { success: false; error: string }> { + log.debug(`BackgroundProcessManager.spawn() called for workspace ${workspaceId}`); + + // Generate unique process ID + const processId = `bg-${randomBytes(4).toString("hex")}`; + + // Create circular buffers for output + const stdoutBuffer = new CircularBuffer(MAX_BUFFER_LINES); + const stderrBuffer = new CircularBuffer(MAX_BUFFER_LINES); + + const process: BackgroundProcess = { + id: processId, + workspaceId, + script, + startTime: Date.now(), + stdoutBuffer, + stderrBuffer, + status: "running", + disposable: null, + }; + + // Spawn with streaming callbacks + const disposable = this.bashExecutionService.executeStreaming(script, config, { + onStdout: (line: string) => { + stdoutBuffer.push(line); + }, + onStderr: (line: string) => { + stderrBuffer.push(line); + }, + onExit: (exitCode: number) => { + log.debug(`Background process ${processId} exited with code ${exitCode}`); + process.exitCode = exitCode; + process.exitTime = Date.now(); + process.status = "exited"; + }, + }); + + const child = disposable.child; + + // Wait until we know whether the spawn succeeded or failed + // ChildProcess emits either 'spawn' (success) or 'error' (failure) - mutually exclusive + try { + await Promise.race([ + // Successful spawn + once(child, "spawn"), + + // Spawn error (ENOENT, invalid cwd, etc.) + once(child, "error").then(([err]) => { + throw err; + }), + + // Safety timeout to prevent infinite hang + new Promise((_, reject) => + setTimeout(() => reject(new Error("Spawn did not complete in time")), 2000) + ), + ]); + } catch (e) { + const err = e as Error; + log.debug(`Failed to spawn background process: ${err.message}`); + disposable[Symbol.dispose](); + return { + success: false, + error: err.message, + }; + } + + // At this point we know the process spawned successfully + process.disposable = disposable; + process.pid = child.pid ?? undefined; + + // Store process in map + this.processes.set(processId, process); + + log.debug(`Background process ${processId} spawned with PID ${process.pid ?? "unknown"}`); + return { success: true, processId }; + } + + /** + * Get a background process by ID + */ + getProcess(processId: string): BackgroundProcess | null { + log.debug(`BackgroundProcessManager.getProcess(${processId}) called`); + return this.processes.get(processId) ?? null; + } + + /** + * List all background processes, optionally filtered by workspace + */ + list(workspaceId?: string): BackgroundProcess[] { + log.debug(`BackgroundProcessManager.list(${workspaceId ?? "all"}) called`); + const allProcesses = Array.from(this.processes.values()); + return workspaceId ? allProcesses.filter((p) => p.workspaceId === workspaceId) : allProcesses; + } + + /** + * Terminate a background process + */ + async terminate( + processId: string + ): Promise<{ success: true } | { success: false; error: string }> { + log.debug(`BackgroundProcessManager.terminate(${processId}) called`); + + // Get process from Map + const proc = this.processes.get(processId); + if (!proc) { + return { success: false, error: `Process not found: ${processId}` }; + } + + // If already terminated, return success (idempotent) + if (proc.status === "exited" || proc.status === "killed" || proc.status === "failed") { + log.debug(`Process ${processId} already terminated with status: ${proc.status}`); + return { success: true }; + } + + // Check if we have a valid PID + if (!proc.pid || !proc.disposable) { + log.debug(`Process ${processId} has no PID or disposable, marking as failed`); + proc.status = "failed"; + proc.exitTime = Date.now(); + return { success: true }; + } + + try { + // Send SIGTERM to the process group for graceful shutdown + // Use negative PID to kill the entire process group (detached processes are group leaders) + // This ensures child processes (e.g., from npm run dev) are also terminated + const pgid = -proc.pid; + log.debug(`Sending SIGTERM to process group ${processId} (PGID: ${pgid})`); + process.kill(pgid, "SIGTERM"); + + // Wait 2 seconds for graceful shutdown + await new Promise((resolve) => setTimeout(resolve, 2000)); + + // Check if process is still running + const stillRunning = proc.disposable.child.exitCode === null; + + if (stillRunning) { + // Force kill the process group with SIGKILL + log.debug(`Process group ${processId} still running, sending SIGKILL`); + process.kill(pgid, "SIGKILL"); + } + + // Update process status + proc.status = "killed"; + proc.exitTime ??= Date.now(); + + // Dispose of the process + proc.disposable[Symbol.dispose](); + + log.debug(`Process ${processId} terminated successfully`); + return { success: true }; + } catch (error) { + const errorMessage = error instanceof Error ? error.message : String(error); + log.debug(`Error terminating process ${processId}: ${errorMessage}`); + // Mark as killed even if there was an error (process likely already dead) + proc.status = "killed"; + proc.exitTime ??= Date.now(); + // Ensure disposable is cleaned up even on error + if (proc.disposable) { + proc.disposable[Symbol.dispose](); + } + return { success: true }; + } + } + + /** + * Clean up all processes for a workspace + * Terminates running processes and removes them from memory + */ + async cleanup(workspaceId: string): Promise { + log.debug(`BackgroundProcessManager.cleanup(${workspaceId}) called`); + const matching = Array.from(this.processes.values()).filter( + (p) => p.workspaceId === workspaceId + ); + + // Terminate all running processes + await Promise.all(matching.map((p) => this.terminate(p.id))); + + // Remove all processes from memory + matching.forEach((p) => this.processes.delete(p.id)); + log.debug(`Cleaned up ${matching.length} process(es) for workspace ${workspaceId}`); + } +} diff --git a/src/node/services/circularBuffer.test.ts b/src/node/services/circularBuffer.test.ts new file mode 100644 index 000000000..3a6713947 --- /dev/null +++ b/src/node/services/circularBuffer.test.ts @@ -0,0 +1,86 @@ +import { describe, it, expect } from "bun:test"; +import { CircularBuffer } from "./circularBuffer"; + +describe("CircularBuffer", () => { + it("should store items up to capacity", () => { + const buffer = new CircularBuffer(3); + buffer.push(1); + buffer.push(2); + buffer.push(3); + + expect(buffer.length).toBe(3); + expect(buffer.toArray()).toEqual([1, 2, 3]); + }); + + it("should overwrite oldest items when full", () => { + const buffer = new CircularBuffer(3); + buffer.push(1); + buffer.push(2); + buffer.push(3); + buffer.push(4); // Should evict 1 + buffer.push(5); // Should evict 2 + + expect(buffer.length).toBe(3); + expect(buffer.toArray()).toEqual([3, 4, 5]); + }); + + it("should handle many overwrites efficiently", () => { + const buffer = new CircularBuffer(5); + + // Add 100 items, only last 5 should remain + for (let i = 1; i <= 100; i++) { + buffer.push(i); + } + + expect(buffer.length).toBe(5); + expect(buffer.toArray()).toEqual([96, 97, 98, 99, 100]); + }); + + it("should handle empty buffer", () => { + const buffer = new CircularBuffer(10); + + expect(buffer.length).toBe(0); + expect(buffer.isEmpty()).toBe(true); + expect(buffer.isFull()).toBe(false); + expect(buffer.toArray()).toEqual([]); + }); + + it("should detect full state", () => { + const buffer = new CircularBuffer(2); + + expect(buffer.isFull()).toBe(false); + buffer.push(1); + expect(buffer.isFull()).toBe(false); + buffer.push(2); + expect(buffer.isFull()).toBe(true); + buffer.push(3); + expect(buffer.isFull()).toBe(true); + }); + + it("should clear all items", () => { + const buffer = new CircularBuffer(3); + buffer.push(1); + buffer.push(2); + buffer.push(3); + + buffer.clear(); + + expect(buffer.length).toBe(0); + expect(buffer.isEmpty()).toBe(true); + expect(buffer.toArray()).toEqual([]); + }); + + it("should work with strings (real use case)", () => { + const buffer = new CircularBuffer(1000); + + // Simulate process output + for (let i = 1; i <= 1500; i++) { + buffer.push(`line ${i}`); + } + + expect(buffer.length).toBe(1000); + const lines = buffer.toArray(); + expect(lines[0]).toBe("line 501"); + expect(lines[999]).toBe("line 1500"); + }); +}); diff --git a/src/node/services/circularBuffer.ts b/src/node/services/circularBuffer.ts new file mode 100644 index 000000000..168aa6524 --- /dev/null +++ b/src/node/services/circularBuffer.ts @@ -0,0 +1,77 @@ +/** + * Efficient circular buffer with fixed capacity + * Avoids O(n) array operations when evicting old items + */ +export class CircularBuffer { + private buffer: T[]; + private head = 0; // Index of oldest item + private tail = 0; // Index where next item will be written + private size = 0; // Current number of items + + constructor(private readonly capacity: number) { + this.buffer = new Array(capacity); + } + + /** + * Add an item to the buffer + * If buffer is full, oldest item is overwritten + */ + push(item: T): void { + this.buffer[this.tail] = item; + this.tail = (this.tail + 1) % this.capacity; + + if (this.size < this.capacity) { + this.size++; + } else { + // Buffer is full, head moves forward (oldest item overwritten) + this.head = (this.head + 1) % this.capacity; + } + } + + /** + * Get all items in order (oldest to newest) + */ + toArray(): T[] { + if (this.size === 0) { + return []; + } + + const result: T[] = []; + let idx = this.head; + for (let i = 0; i < this.size; i++) { + result.push(this.buffer[idx]); + idx = (idx + 1) % this.capacity; + } + return result; + } + + /** + * Get number of items currently stored + */ + get length(): number { + return this.size; + } + + /** + * Check if buffer is empty + */ + isEmpty(): boolean { + return this.size === 0; + } + + /** + * Check if buffer is at capacity + */ + isFull(): boolean { + return this.size === this.capacity; + } + + /** + * Clear all items + */ + clear(): void { + this.head = 0; + this.tail = 0; + this.size = 0; + } +} diff --git a/src/node/services/ipcMain.ts b/src/node/services/ipcMain.ts index 0700c49f2..3ffe000d6 100644 --- a/src/node/services/ipcMain.ts +++ b/src/node/services/ipcMain.ts @@ -38,6 +38,8 @@ import { DisposableTempDir } from "@/node/services/tempDir"; import { InitStateManager } from "@/node/services/initStateManager"; import { createRuntime } from "@/node/runtime/runtimeFactory"; import type { RuntimeConfig } from "@/common/types/runtime"; +import { BackgroundProcessManager } from "@/node/services/backgroundProcessManager"; +import { BashExecutionService } from "@/node/services/bashExecutionService"; import { isSSHRuntime } from "@/common/types/runtime"; import { validateProjectPath } from "@/node/utils/pathUtils"; import { PTYService } from "@/node/services/ptyService"; @@ -66,6 +68,7 @@ export class IpcMain { private readonly initStateManager: InitStateManager; private readonly extensionMetadata: ExtensionMetadataService; private readonly ptyService: PTYService; + private readonly backgroundProcessManager: BackgroundProcessManager; private terminalWindowManager?: TerminalWindowManager; private readonly sessions = new Map(); private projectDirectoryPicker?: (event: IpcMainInvokeEvent) => Promise; @@ -86,11 +89,14 @@ export class IpcMain { this.extensionMetadata = new ExtensionMetadataService( path.join(config.rootDir, "extensionMetadata.json") ); + this.backgroundProcessManager = new BackgroundProcessManager(new BashExecutionService()); + this.aiService = new AIService( config, this.historyService, this.partialService, - this.initStateManager + this.initStateManager, + this.backgroundProcessManager ); // Terminal services - PTYService is cross-platform this.ptyService = new PTYService(); @@ -1305,6 +1311,7 @@ export class IpcMain { niceness: options?.niceness, runtimeTempDir: tempDir.path, overflow_policy: "truncate", + workspaceId, }); // Execute the script with provided options @@ -1408,6 +1415,9 @@ export class IpcMain { metadata.runtimeConfig ?? { type: "local", srcBaseDir: this.config.srcDir } ); + // Clean up background processes before deleting workspace + await this.backgroundProcessManager.cleanup(workspaceId); + // Delegate deletion to runtime - it handles all path computation, existence checks, and pruning const deleteResult = await runtime.deleteWorkspace(projectPath, metadata.name, options.force); diff --git a/src/node/services/tools/bash.test.ts b/src/node/services/tools/bash.test.ts index b2c95103f..15d87fbb3 100644 --- a/src/node/services/tools/bash.test.ts +++ b/src/node/services/tools/bash.test.ts @@ -7,6 +7,8 @@ import * as fs from "fs"; import { TestTempDir, createTestToolConfig, getTestDeps } from "./testHelpers"; import { createRuntime } from "@/node/runtime/runtimeFactory"; import type { ToolCallOptions } from "ai"; +import { BackgroundProcessManager } from "@/node/services/backgroundProcessManager"; +import { BashExecutionService } from "@/node/services/bashExecutionService"; // Mock ToolCallOptions for testing const mockToolCallOptions: ToolCallOptions = { @@ -38,6 +40,7 @@ describe("bash tool", () => { const args: BashToolArgs = { script: "echo hello", timeout_secs: 5, + run_in_background: false, }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -55,6 +58,7 @@ describe("bash tool", () => { const args: BashToolArgs = { script: "echo line1 && echo line2 && echo line3", timeout_secs: 5, + run_in_background: false, }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -69,6 +73,7 @@ describe("bash tool", () => { using testEnv = createTestBashTool(); const tool = testEnv.tool; const args: BashToolArgs = { + run_in_background: false, script: "for i in {1..400}; do echo line$i; done", // Exceeds 300 line hard cap timeout_secs: 5, }; @@ -87,6 +92,7 @@ describe("bash tool", () => { using testEnv = createTestBashTool(); const tool = testEnv.tool; const args: BashToolArgs = { + run_in_background: false, script: "for i in {1..400}; do echo line$i; done", // Exceeds 300 line hard cap timeout_secs: 5, }; @@ -139,6 +145,7 @@ describe("bash tool", () => { const args: BashToolArgs = { // This will generate 500 lines quickly - should fail at 300 + run_in_background: false, script: "for i in {1..500}; do echo line$i; done", timeout_secs: 5, }; @@ -167,18 +174,21 @@ describe("bash tool", () => { // Generate ~1.5MB of output (1700 lines * 900 bytes) to exceed 1MB byte limit script: 'perl -e \'for (1..1700) { print "A" x 900 . "\\n" }\'', timeout_secs: 5, + run_in_background: false, }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; // With truncate policy and overflow, should succeed with truncated field expect(result.success).toBe(true); - expect(result.truncated).toBeDefined(); - if (result.truncated) { - expect(result.truncated.reason).toContain("exceed"); - // Should collect lines up to ~1MB (around 1150-1170 lines with 900 bytes each) - expect(result.truncated.totalLines).toBeGreaterThan(1000); - expect(result.truncated.totalLines).toBeLessThan(1300); + if (result.success && "truncated" in result) { + expect(result.truncated).toBeDefined(); + if (result.truncated) { + expect(result.truncated.reason).toContain("exceed"); + // Should collect lines up to ~1MB (around 1150-1170 lines with 900 bytes each) + expect(result.truncated.totalLines).toBeGreaterThan(1000); + expect(result.truncated.totalLines).toBeLessThan(1300); + } } // Should contain output that's around 1MB @@ -207,17 +217,20 @@ describe("bash tool", () => { // Generate a single 2MB line (exceeds 1MB total limit) script: 'perl -e \'print "A" x 2000000 . "\\n"\'', timeout_secs: 5, + run_in_background: false, }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; // Should succeed but with truncation before storing the overlong line expect(result.success).toBe(true); - expect(result.truncated).toBeDefined(); - if (result.truncated) { - expect(result.truncated.reason).toContain("would exceed file preservation limit"); - // Should have 0 lines collected since the first line was too long - expect(result.truncated.totalLines).toBe(0); + if (result.success && "truncated" in result) { + expect(result.truncated).toBeDefined(); + if (result.truncated) { + expect(result.truncated.reason).toContain("would exceed file preservation limit"); + // Should have 0 lines collected since the first line was too long + expect(result.truncated.totalLines).toBe(0); + } } // CRITICAL: Output must NOT contain the 2MB line - should be empty or nearly empty @@ -241,16 +254,19 @@ describe("bash tool", () => { // Second line: 600KB (would exceed 1MB when added) script: 'perl -e \'print "A" x 500000 . "\\n"; print "B" x 600000 . "\\n"\'', timeout_secs: 5, + run_in_background: false, }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; expect(result.success).toBe(true); - expect(result.truncated).toBeDefined(); - if (result.truncated) { - expect(result.truncated.reason).toContain("would exceed"); - // Should have collected exactly 1 line (the 500KB line) - expect(result.truncated.totalLines).toBe(1); + if (result.success && "truncated" in result) { + expect(result.truncated).toBeDefined(); + if (result.truncated) { + expect(result.truncated.reason).toContain("would exceed"); + // Should have collected exactly 1 line (the 500KB line) + expect(result.truncated.totalLines).toBe(1); + } } // Output should contain only the first line (~500KB), not the second line @@ -274,6 +290,7 @@ describe("bash tool", () => { }); const args: BashToolArgs = { + run_in_background: false, script: "for i in {1..400}; do echo line$i; done", timeout_secs: 5, }; @@ -310,6 +327,7 @@ describe("bash tool", () => { // Each line is ~40 bytes: "line" + number (1-5 digits) + padding = ~40 bytes // 50KB / 40 bytes = ~1250 lines const args: BashToolArgs = { + run_in_background: false, script: "for i in {1..1300}; do printf 'line%04d with some padding text here\\n' $i; done", timeout_secs: 5, }; @@ -363,6 +381,7 @@ describe("bash tool", () => { // Each line is ~100 bytes // 150KB / 100 bytes = ~1500 lines const args: BashToolArgs = { + run_in_background: false, script: "for i in {1..1600}; do printf 'line%04d: '; printf 'x%.0s' {1..80}; echo; done", timeout_secs: 10, }; @@ -409,6 +428,7 @@ describe("bash tool", () => { script: "for i in {1..500}; do printf 'line%04d with padding text\\n' $i; done; echo 'COMPLETION_MARKER'", timeout_secs: 5, + run_in_background: false, }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -448,6 +468,7 @@ describe("bash tool", () => { // Generate a single line exceeding 1KB limit, then try to output more const args: BashToolArgs = { + run_in_background: false, script: "printf 'x%.0s' {1..2000}; echo; echo 'SHOULD_NOT_APPEAR'", timeout_secs: 5, }; @@ -490,6 +511,7 @@ describe("bash tool", () => { // Generate ~15KB of output (just under 16KB display limit) // Each line is ~50 bytes, 15KB / 50 = 300 lines exactly (at the line limit) const args: BashToolArgs = { + run_in_background: false, script: "for i in {1..299}; do printf 'line%04d with some padding text here now\\n' $i; done", timeout_secs: 5, }; @@ -520,6 +542,7 @@ describe("bash tool", () => { // Generate exactly 300 lines (hits line limit exactly) const args: BashToolArgs = { + run_in_background: false, script: "for i in {1..300}; do printf 'line%04d\\n' $i; done", timeout_secs: 5, }; @@ -542,6 +565,7 @@ describe("bash tool", () => { const args: BashToolArgs = { script: "echo stdout1 && echo stderr1 >&2 && echo stdout2 && echo stderr2 >&2", timeout_secs: 5, + run_in_background: false, }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -562,6 +586,7 @@ describe("bash tool", () => { const args: BashToolArgs = { script: "exit 42", timeout_secs: 5, + run_in_background: false, }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -579,6 +604,7 @@ describe("bash tool", () => { const args: BashToolArgs = { script: "while true; do sleep 0.1; done", timeout_secs: 1, + run_in_background: false, }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -596,6 +622,7 @@ describe("bash tool", () => { const args: BashToolArgs = { script: "true", timeout_secs: 5, + run_in_background: false, }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -617,6 +644,7 @@ describe("bash tool", () => { const args: BashToolArgs = { script: "echo 'test:first-child' | grep ':first-child'", timeout_secs: 5, + run_in_background: false, }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -642,6 +670,7 @@ describe("bash tool", () => { const args: BashToolArgs = { script: "echo test | cat", timeout_secs: 5, + run_in_background: false, }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -666,6 +695,7 @@ describe("bash tool", () => { script: 'python3 -c "import os,stat;mode=os.fstat(0).st_mode;print(stat.S_IFMT(mode)==stat.S_IFIFO)"', timeout_secs: 5, + run_in_background: false, }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -710,6 +740,7 @@ describe("bash tool", () => { const args: BashToolArgs = { script: "echo test", timeout_secs: 5, + run_in_background: false, }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -727,6 +758,7 @@ describe("bash tool", () => { const args: BashToolArgs = { script: "echo 'cd' && echo test", timeout_secs: 5, + run_in_background: false, }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -747,6 +779,7 @@ describe("bash tool", () => { // Background process that would block if we waited for it script: "while true; do sleep 1; done > /dev/null 2>&1 &", timeout_secs: 5, + run_in_background: false, }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -767,6 +800,7 @@ describe("bash tool", () => { // Should not wait for the background process script: "while true; do sleep 1; done > /dev/null 2>&1 & echo $!", timeout_secs: 5, + run_in_background: false, }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -790,6 +824,7 @@ describe("bash tool", () => { // Background process with output redirected but still blocking script: "while true; do sleep 0.1; done & wait", timeout_secs: 1, + run_in_background: false, }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -809,6 +844,7 @@ describe("bash tool", () => { const args: BashToolArgs = { script: `echo '${longLine}'`, timeout_secs: 5, + run_in_background: false, }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -828,6 +864,7 @@ describe("bash tool", () => { const args: BashToolArgs = { script: `for i in {1..${numLines}}; do echo '${lineContent}'; done`, timeout_secs: 5, + run_in_background: false, }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -843,6 +880,7 @@ describe("bash tool", () => { using testEnv = createTestBashTool(); const tool = testEnv.tool; const args: BashToolArgs = { + run_in_background: false, script: `for i in {1..1000}; do echo 'This is line number '$i' with some content'; done`, timeout_secs: 5, }; @@ -862,6 +900,7 @@ describe("bash tool", () => { const args: BashToolArgs = { script: "", timeout_secs: 5, + run_in_background: false, }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -881,6 +920,7 @@ describe("bash tool", () => { const args: BashToolArgs = { script: " \n\t ", timeout_secs: 5, + run_in_background: false, }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -899,6 +939,7 @@ describe("bash tool", () => { const args: BashToolArgs = { script: "sleep 5", timeout_secs: 10, + run_in_background: false, }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -919,6 +960,7 @@ describe("bash tool", () => { const args: BashToolArgs = { script: "for i in 1 2 3; do echo $i; sleep 0.1; done", timeout_secs: 5, + run_in_background: false, }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -997,6 +1039,7 @@ echo "$VALUE" echo "$RESULT" `, timeout_secs: 5, + run_in_background: false, }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -1024,6 +1067,7 @@ if [ $? -ne 0 ]; then fi `, timeout_secs: 5, + run_in_background: false, }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -1042,6 +1086,7 @@ fi const args: BashToolArgs = { script: "echo hello", timeout_secs: 5, + run_in_background: false, }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -1064,6 +1109,7 @@ fi const args: BashToolArgs = { script: `echo "${marker}"; sleep 100 & echo $!`, timeout_secs: 1, + run_in_background: false, }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -1118,6 +1164,7 @@ fi exec -a "sleep-${token}" sleep 100 `, timeout_secs: 10, + run_in_background: false, }; // Start the command @@ -1187,6 +1234,7 @@ fi done `, timeout_secs: 120, + run_in_background: false, }; // Start the command @@ -1264,6 +1312,7 @@ describe("SSH runtime redundant cd detection", () => { const args: BashToolArgs = { script: "cd /remote/workspace/project/branch && echo test", timeout_secs: 5, + run_in_background: false, }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -1285,6 +1334,7 @@ describe("SSH runtime redundant cd detection", () => { const args: BashToolArgs = { script: "cd /tmp && echo test", timeout_secs: 5, + run_in_background: false, }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -1298,3 +1348,74 @@ describe("SSH runtime redundant cd detection", () => { } }); }); +describe("bash tool - background execution", () => { + it("should reject background mode when manager not available", async () => { + using testEnv = createTestBashTool(); + const tool = testEnv.tool; + const args: BashToolArgs = { + script: "echo test", + run_in_background: true, + }; + + const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error).toContain("Background execution is only available for AI tool calls"); + } + }); + + it("should reject timeout with background mode", async () => { + const manager = new BackgroundProcessManager(new BashExecutionService()); + + const tempDir = new TestTempDir("test-bash-bg"); + const config = createTestToolConfig(process.cwd()); + config.runtimeTempDir = tempDir.path; + config.backgroundProcessManager = manager; + config.workspaceId = "test-workspace"; + + const tool = createBashTool(config); + const args: BashToolArgs = { + script: "echo test", + timeout_secs: 5, + run_in_background: true, + }; + + const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error).toContain("Cannot specify timeout with run_in_background"); + } + + tempDir[Symbol.dispose](); + }); + + it("should start background process and return process ID", async () => { + const manager = new BackgroundProcessManager(new BashExecutionService()); + + const tempDir = new TestTempDir("test-bash-bg"); + const config = createTestToolConfig(process.cwd()); + config.runtimeTempDir = tempDir.path; + config.backgroundProcessManager = manager; + config.workspaceId = "test-workspace"; + + const tool = createBashTool(config); + const args: BashToolArgs = { + script: "echo hello", + run_in_background: true, + }; + + const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; + + expect(result.success).toBe(true); + if (result.success && "backgroundProcessId" in result) { + expect(result.backgroundProcessId).toBeDefined(); + expect(result.backgroundProcessId).toMatch(/^bg-/); + } else { + throw new Error("Expected background process ID in result"); + } + + tempDir[Symbol.dispose](); + }); +}); diff --git a/src/node/services/tools/bash.ts b/src/node/services/tools/bash.ts index c0559a86d..f7228bca5 100644 --- a/src/node/services/tools/bash.ts +++ b/src/node/services/tools/bash.ts @@ -229,11 +229,74 @@ export const createBashTool: ToolFactory = (config: ToolConfiguration) => { return tool({ description: TOOL_DEFINITIONS.bash.description + "\nRuns in " + config.cwd + " - no cd needed", inputSchema: TOOL_DEFINITIONS.bash.schema, - execute: async ({ script, timeout_secs }, { abortSignal }): Promise => { + execute: async ( + { script, timeout_secs, run_in_background }, + { abortSignal } + ): Promise => { // Validate script input const validationError = validateScript(script, config); if (validationError) return validationError; + // Handle background execution + if (run_in_background) { + // TODO: Add Windows support for background processes (process groups work differently) + if (process.platform === "win32") { + return { + success: false, + error: "Background execution is not yet supported on Windows", + exitCode: -1, + wall_duration_ms: 0, + }; + } + + if (!config.workspaceId || !config.backgroundProcessManager) { + return { + success: false, + error: + "Background execution is only available for AI tool calls, not direct IPC invocation", + exitCode: -1, + wall_duration_ms: 0, + }; + } + + if (timeout_secs !== undefined) { + return { + success: false, + error: "Cannot specify timeout with run_in_background", + exitCode: -1, + wall_duration_ms: 0, + }; + } + + const startTime = performance.now(); + const spawnResult = await config.backgroundProcessManager.spawn( + config.workspaceId, + script, + { + cwd: config.cwd, + secrets: config.secrets, + niceness: config.niceness, + } + ); + + if (!spawnResult.success) { + return { + success: false, + error: spawnResult.error, + exitCode: -1, + wall_duration_ms: Math.round(performance.now() - startTime), + }; + } + + return { + success: true, + output: `Background process started with ID: ${spawnResult.processId}`, + exitCode: 0, + wall_duration_ms: Math.round(performance.now() - startTime), + backgroundProcessId: spawnResult.processId, + }; + } + // Setup execution parameters const effectiveTimeout = timeout_secs ?? BASH_DEFAULT_TIMEOUT_SECS; const startTime = performance.now(); diff --git a/src/node/services/tools/bash_background_list.test.ts b/src/node/services/tools/bash_background_list.test.ts new file mode 100644 index 000000000..cc4ec8190 --- /dev/null +++ b/src/node/services/tools/bash_background_list.test.ts @@ -0,0 +1,133 @@ +import { describe, it, expect } from "bun:test"; +import { createBashBackgroundListTool } from "./bash_background_list"; +import { BackgroundProcessManager } from "@/node/services/backgroundProcessManager"; +import { BashExecutionService } from "@/node/services/bashExecutionService"; +import type { BashBackgroundListResult } from "@/common/types/tools"; +import { TestTempDir, createTestToolConfig } from "./testHelpers"; +import type { ToolCallOptions } from "ai"; + +const mockToolCallOptions: ToolCallOptions = { + toolCallId: "test-call-id", + messages: [], +}; + +describe("bash_background_list tool", () => { + it("should return error when manager not available", async () => { + const tempDir = new TestTempDir("test-bash-bg-list"); + const config = createTestToolConfig(process.cwd()); + config.runtimeTempDir = tempDir.path; + + const tool = createBashBackgroundListTool(config); + const result = (await tool.execute!({}, mockToolCallOptions)) as BashBackgroundListResult; + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error).toContain("Background process manager not available"); + } + + tempDir[Symbol.dispose](); + }); + + it("should return error when workspaceId not available", async () => { + const manager = new BackgroundProcessManager(new BashExecutionService()); + const tempDir = new TestTempDir("test-bash-bg-list"); + const config = createTestToolConfig(process.cwd()); + config.runtimeTempDir = tempDir.path; + config.backgroundProcessManager = manager; + delete config.workspaceId; // Explicitly remove workspaceId + + const tool = createBashBackgroundListTool(config); + const result = (await tool.execute!({}, mockToolCallOptions)) as BashBackgroundListResult; + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error).toContain("Workspace ID not available"); + } + + tempDir[Symbol.dispose](); + }); + + it("should return empty list when no processes", async () => { + const manager = new BackgroundProcessManager(new BashExecutionService()); + const tempDir = new TestTempDir("test-bash-bg-list"); + const config = createTestToolConfig(process.cwd()); + config.runtimeTempDir = tempDir.path; + config.backgroundProcessManager = manager; + + const tool = createBashBackgroundListTool(config); + const result = (await tool.execute!({}, mockToolCallOptions)) as BashBackgroundListResult; + + expect(result.success).toBe(true); + if (result.success) { + expect(result.processes).toEqual([]); + } + + tempDir[Symbol.dispose](); + }); + + it("should list spawned processes with correct fields", async () => { + const manager = new BackgroundProcessManager(new BashExecutionService()); + const tempDir = new TestTempDir("test-bash-bg-list"); + const config = createTestToolConfig(process.cwd()); + config.runtimeTempDir = tempDir.path; + config.backgroundProcessManager = manager; + + // Spawn a process + const spawnResult = await manager.spawn("test-workspace", "sleep 10", { + cwd: process.cwd(), + }); + + if (!spawnResult.success) { + throw new Error("Failed to spawn process"); + } + + const tool = createBashBackgroundListTool(config); + const result = (await tool.execute!({}, mockToolCallOptions)) as BashBackgroundListResult; + + expect(result.success).toBe(true); + if (result.success) { + expect(result.processes.length).toBe(1); + const proc = result.processes[0]; + expect(proc.process_id).toBe(spawnResult.processId); + expect(proc.status).toBe("running"); + expect(proc.script).toBe("sleep 10"); + expect(proc.pid).toBeDefined(); + expect(proc.uptime_ms).toBeGreaterThanOrEqual(0); + expect(proc.exitCode).toBeUndefined(); + } + + // Cleanup + await manager.terminate(spawnResult.processId); + tempDir[Symbol.dispose](); + }); + + it("should only list processes for the current workspace", async () => { + const manager = new BackgroundProcessManager(new BashExecutionService()); + const tempDir = new TestTempDir("test-bash-bg-list"); + const config = createTestToolConfig(process.cwd(), { workspaceId: "workspace-a" }); + config.runtimeTempDir = tempDir.path; + config.backgroundProcessManager = manager; + + // Spawn processes in different workspaces + const spawnA = await manager.spawn("workspace-a", "sleep 10", { cwd: process.cwd() }); + const spawnB = await manager.spawn("workspace-b", "sleep 10", { cwd: process.cwd() }); + + if (!spawnA.success || !spawnB.success) { + throw new Error("Failed to spawn processes"); + } + + const tool = createBashBackgroundListTool(config); + const result = (await tool.execute!({}, mockToolCallOptions)) as BashBackgroundListResult; + + expect(result.success).toBe(true); + if (result.success) { + expect(result.processes.length).toBe(1); + expect(result.processes[0].process_id).toBe(spawnA.processId); + } + + // Cleanup + await manager.terminate(spawnA.processId); + await manager.terminate(spawnB.processId); + tempDir[Symbol.dispose](); + }); +}); diff --git a/src/node/services/tools/bash_background_list.ts b/src/node/services/tools/bash_background_list.ts new file mode 100644 index 000000000..7a984d2a7 --- /dev/null +++ b/src/node/services/tools/bash_background_list.ts @@ -0,0 +1,44 @@ +import { tool } from "ai"; +import type { BashBackgroundListResult } from "@/common/types/tools"; +import type { ToolConfiguration, ToolFactory } from "@/common/utils/tools/tools"; +import { TOOL_DEFINITIONS } from "@/common/utils/tools/toolDefinitions"; + +/** + * Tool for listing background processes in the current workspace + */ +export const createBashBackgroundListTool: ToolFactory = (config: ToolConfiguration) => { + return tool({ + description: TOOL_DEFINITIONS.bash_background_list.description, + inputSchema: TOOL_DEFINITIONS.bash_background_list.schema, + execute: (): BashBackgroundListResult => { + if (!config.backgroundProcessManager) { + return { + success: false, + error: "Background process manager not available", + }; + } + + if (!config.workspaceId) { + return { + success: false, + error: "Workspace ID not available", + }; + } + + const processes = config.backgroundProcessManager.list(config.workspaceId); + const now = Date.now(); + + return { + success: true, + processes: processes.map((p) => ({ + process_id: p.id, + status: p.status, + script: p.script, + pid: p.pid, + uptime_ms: p.exitTime !== undefined ? p.exitTime - p.startTime : now - p.startTime, + exitCode: p.exitCode, + })), + }; + }, + }); +}; diff --git a/src/node/services/tools/bash_background_read.test.ts b/src/node/services/tools/bash_background_read.test.ts new file mode 100644 index 000000000..7232f9a7b --- /dev/null +++ b/src/node/services/tools/bash_background_read.test.ts @@ -0,0 +1,233 @@ +import { describe, it, expect } from "bun:test"; +import { createBashBackgroundReadTool } from "./bash_background_read"; +import { BackgroundProcessManager } from "@/node/services/backgroundProcessManager"; +import { BashExecutionService } from "@/node/services/bashExecutionService"; +import type { BashBackgroundReadArgs, BashBackgroundReadResult } from "@/common/types/tools"; +import { TestTempDir, createTestToolConfig } from "./testHelpers"; +import type { ToolCallOptions } from "ai"; + +const mockToolCallOptions: ToolCallOptions = { + toolCallId: "test-call-id", + messages: [], +}; + +describe("bash_background_read tool", () => { + it("should return error when manager not available", async () => { + const tempDir = new TestTempDir("test-bash-bg-read"); + const config = createTestToolConfig(process.cwd()); + config.runtimeTempDir = tempDir.path; + + const tool = createBashBackgroundReadTool(config); + const args: BashBackgroundReadArgs = { + process_id: "bg-test", + }; + + const result = (await tool.execute!(args, mockToolCallOptions)) as BashBackgroundReadResult; + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error).toContain("Background process manager not available"); + } + + tempDir[Symbol.dispose](); + }); + + it("should return error for non-existent process", async () => { + const manager = new BackgroundProcessManager(new BashExecutionService()); + const tempDir = new TestTempDir("test-bash-bg-read"); + const config = createTestToolConfig(process.cwd()); + config.runtimeTempDir = tempDir.path; + config.backgroundProcessManager = manager; + + const tool = createBashBackgroundReadTool(config); + const args: BashBackgroundReadArgs = { + process_id: "bg-nonexistent", + }; + + const result = (await tool.execute!(args, mockToolCallOptions)) as BashBackgroundReadResult; + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error).toContain("Process not found"); + } + + tempDir[Symbol.dispose](); + }); + + it("should return process status and output", async () => { + const manager = new BackgroundProcessManager(new BashExecutionService()); + const tempDir = new TestTempDir("test-bash-bg-read"); + const config = createTestToolConfig(process.cwd()); + config.runtimeTempDir = tempDir.path; + config.backgroundProcessManager = manager; + + // Spawn a process + const spawnResult = await manager.spawn("test-workspace", "echo hello; sleep 1", { + cwd: process.cwd(), + }); + + if (!spawnResult.success) { + throw new Error("Failed to spawn process"); + } + + // Wait for output + await new Promise((resolve) => setTimeout(resolve, 100)); + + const tool = createBashBackgroundReadTool(config); + const args: BashBackgroundReadArgs = { + process_id: spawnResult.processId, + }; + + const result = (await tool.execute!(args, mockToolCallOptions)) as BashBackgroundReadResult; + + expect(result.success).toBe(true); + if (result.success) { + expect(result.process_id).toBe(spawnResult.processId); + expect(result.status).toBe("running"); + expect(result.stdout).toContain("hello"); + expect(result.uptime_ms).toBeGreaterThan(0); + } + + tempDir[Symbol.dispose](); + }); + + it("should handle tail filtering", async () => { + const manager = new BackgroundProcessManager(new BashExecutionService()); + const tempDir = new TestTempDir("test-bash-bg-read"); + const config = createTestToolConfig(process.cwd()); + config.runtimeTempDir = tempDir.path; + config.backgroundProcessManager = manager; + + // Spawn a process with multiple lines + const spawnResult = await manager.spawn( + "test-workspace", + "echo line1; echo line2; echo line3", + { + cwd: process.cwd(), + } + ); + + if (!spawnResult.success) { + throw new Error("Failed to spawn process"); + } + + await new Promise((resolve) => setTimeout(resolve, 100)); + + const tool = createBashBackgroundReadTool(config); + const args: BashBackgroundReadArgs = { + process_id: spawnResult.processId, + stdout_tail: 2, + }; + + const result = (await tool.execute!(args, mockToolCallOptions)) as BashBackgroundReadResult; + + expect(result.success).toBe(true); + if (result.success) { + expect(result.stdout.length).toBeLessThanOrEqual(2); + } + + tempDir[Symbol.dispose](); + }); + + it("should handle regex filtering", async () => { + const manager = new BackgroundProcessManager(new BashExecutionService()); + const tempDir = new TestTempDir("test-bash-bg-read"); + const config = createTestToolConfig(process.cwd()); + config.runtimeTempDir = tempDir.path; + config.backgroundProcessManager = manager; + + const spawnResult = await manager.spawn("test-workspace", "echo ERROR: test; echo INFO: test", { + cwd: process.cwd(), + }); + + if (!spawnResult.success) { + throw new Error("Failed to spawn process"); + } + + await new Promise((resolve) => setTimeout(resolve, 100)); + + const tool = createBashBackgroundReadTool(config); + const args: BashBackgroundReadArgs = { + process_id: spawnResult.processId, + stdout_regex: "ERROR", + }; + + const result = (await tool.execute!(args, mockToolCallOptions)) as BashBackgroundReadResult; + + expect(result.success).toBe(true); + if (result.success) { + expect(result.stdout.every((line) => line.includes("ERROR"))).toBe(true); + } + + tempDir[Symbol.dispose](); + }); + + it("should return error for invalid regex pattern", async () => { + const manager = new BackgroundProcessManager(new BashExecutionService()); + const tempDir = new TestTempDir("test-bash-bg-read"); + const config = createTestToolConfig(process.cwd()); + config.runtimeTempDir = tempDir.path; + config.backgroundProcessManager = manager; + + const spawnResult = await manager.spawn("test-workspace", "echo test", { + cwd: process.cwd(), + }); + + if (!spawnResult.success) { + throw new Error("Failed to spawn process"); + } + + await new Promise((resolve) => setTimeout(resolve, 100)); + + const tool = createBashBackgroundReadTool(config); + const args: BashBackgroundReadArgs = { + process_id: spawnResult.processId, + stdout_regex: "[invalid(", // Invalid regex pattern + }; + + const result = (await tool.execute!(args, mockToolCallOptions)) as BashBackgroundReadResult; + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error).toContain("Invalid regex pattern"); + expect(result.error).toContain("stdout_regex"); + } + + tempDir[Symbol.dispose](); + }); + + it("should not read processes from other workspaces", async () => { + const manager = new BackgroundProcessManager(new BashExecutionService()); + const tempDir = new TestTempDir("test-bash-bg-read"); + // Config is for workspace-a + const config = createTestToolConfig(process.cwd(), { workspaceId: "workspace-a" }); + config.runtimeTempDir = tempDir.path; + config.backgroundProcessManager = manager; + + // Spawn process in workspace-b + const spawnResult = await manager.spawn("workspace-b", "echo secret", { + cwd: process.cwd(), + }); + + if (!spawnResult.success) { + throw new Error("Failed to spawn process"); + } + + await new Promise((resolve) => setTimeout(resolve, 100)); + + // Try to read from workspace-a (should fail) + const tool = createBashBackgroundReadTool(config); + const args: BashBackgroundReadArgs = { + process_id: spawnResult.processId, + }; + + const result = (await tool.execute!(args, mockToolCallOptions)) as BashBackgroundReadResult; + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error).toContain("Process not found"); + } + + tempDir[Symbol.dispose](); + }); +}); diff --git a/src/node/services/tools/bash_background_read.ts b/src/node/services/tools/bash_background_read.ts new file mode 100644 index 000000000..25d80c816 --- /dev/null +++ b/src/node/services/tools/bash_background_read.ts @@ -0,0 +1,98 @@ +import { tool } from "ai"; +import type { BashBackgroundReadResult } from "@/common/types/tools"; +import type { ToolConfiguration, ToolFactory } from "@/common/utils/tools/tools"; +import { TOOL_DEFINITIONS } from "@/common/utils/tools/toolDefinitions"; + +/** + * Tool for reading status and output from background processes + */ +export const createBashBackgroundReadTool: ToolFactory = (config: ToolConfiguration) => { + return tool({ + description: TOOL_DEFINITIONS.bash_background_read.description, + inputSchema: TOOL_DEFINITIONS.bash_background_read.schema, + execute: ({ + process_id, + stdout_tail, + stderr_tail, + stdout_regex, + stderr_regex, + }): BashBackgroundReadResult => { + if (!config.backgroundProcessManager) { + return { + success: false, + error: "Background process manager not available", + }; + } + + if (!config.workspaceId) { + return { + success: false, + error: "Workspace ID not available", + }; + } + + // Get process from manager and verify workspace ownership + const process = config.backgroundProcessManager.getProcess(process_id); + if (!process || process.workspaceId !== config.workspaceId) { + return { + success: false, + error: `Process not found: ${process_id}`, + }; + } + + // Apply filtering (regex first, then tail) + let stdout = process.stdoutBuffer.toArray(); + let stderr = process.stderrBuffer.toArray(); + + // Apply regex filters first + if (stdout_regex) { + try { + const regex = new RegExp(stdout_regex); + stdout = stdout.filter((line) => regex.test(line)); + } catch (error) { + return { + success: false, + error: `Invalid regex pattern for stdout_regex: ${error instanceof Error ? error.message : String(error)}`, + }; + } + } + if (stderr_regex) { + try { + const regex = new RegExp(stderr_regex); + stderr = stderr.filter((line) => regex.test(line)); + } catch (error) { + return { + success: false, + error: `Invalid regex pattern for stderr_regex: ${error instanceof Error ? error.message : String(error)}`, + }; + } + } + + // Apply tail filters after regex + if (stdout_tail !== undefined) { + stdout = stdout.slice(-stdout_tail); + } + if (stderr_tail !== undefined) { + stderr = stderr.slice(-stderr_tail); + } + + // Compute uptime + const uptime_ms = + process.exitTime !== undefined + ? process.exitTime - process.startTime + : Date.now() - process.startTime; + + return { + success: true, + process_id: process.id, + status: process.status, + script: process.script, + pid: process.pid, + uptime_ms, + exitCode: process.exitCode, + stdout, + stderr, + }; + }, + }); +}; diff --git a/src/node/services/tools/bash_background_terminate.test.ts b/src/node/services/tools/bash_background_terminate.test.ts new file mode 100644 index 000000000..c187d0b3b --- /dev/null +++ b/src/node/services/tools/bash_background_terminate.test.ts @@ -0,0 +1,178 @@ +import { describe, it, expect } from "bun:test"; +import { createBashBackgroundTerminateTool } from "./bash_background_terminate"; +import { BackgroundProcessManager } from "@/node/services/backgroundProcessManager"; +import { BashExecutionService } from "@/node/services/bashExecutionService"; +import type { + BashBackgroundTerminateArgs, + BashBackgroundTerminateResult, +} from "@/common/types/tools"; +import { TestTempDir, createTestToolConfig } from "./testHelpers"; +import type { ToolCallOptions } from "ai"; + +const mockToolCallOptions: ToolCallOptions = { + toolCallId: "test-call-id", + messages: [], +}; + +describe("bash_background_terminate tool", () => { + it("should return error when manager not available", async () => { + const tempDir = new TestTempDir("test-bash-bg-term"); + const config = createTestToolConfig(process.cwd()); + config.runtimeTempDir = tempDir.path; + + const tool = createBashBackgroundTerminateTool(config); + const args: BashBackgroundTerminateArgs = { + process_id: "bg-test", + }; + + const result = (await tool.execute!( + args, + mockToolCallOptions + )) as BashBackgroundTerminateResult; + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error).toContain("Background process manager not available"); + } + + tempDir[Symbol.dispose](); + }); + + it("should return error for non-existent process", async () => { + const manager = new BackgroundProcessManager(new BashExecutionService()); + const tempDir = new TestTempDir("test-bash-bg-term"); + const config = createTestToolConfig(process.cwd()); + config.runtimeTempDir = tempDir.path; + config.backgroundProcessManager = manager; + + const tool = createBashBackgroundTerminateTool(config); + const args: BashBackgroundTerminateArgs = { + process_id: "bg-nonexistent", + }; + + const result = (await tool.execute!( + args, + mockToolCallOptions + )) as BashBackgroundTerminateResult; + + expect(result.success).toBe(false); + }); + + it("should terminate a running process", async () => { + const manager = new BackgroundProcessManager(new BashExecutionService()); + const tempDir = new TestTempDir("test-bash-bg-term"); + const config = createTestToolConfig(process.cwd()); + config.runtimeTempDir = tempDir.path; + config.backgroundProcessManager = manager; + + // Spawn a long-running process + const spawnResult = await manager.spawn("test-workspace", "sleep 10", { + cwd: process.cwd(), + }); + + if (!spawnResult.success) { + throw new Error("Failed to spawn process"); + } + + const tool = createBashBackgroundTerminateTool(config); + const args: BashBackgroundTerminateArgs = { + process_id: spawnResult.processId, + }; + + const result = (await tool.execute!( + args, + mockToolCallOptions + )) as BashBackgroundTerminateResult; + + expect(result.success).toBe(true); + if (result.success) { + expect(result.message).toContain(spawnResult.processId); + } + + // Verify process is no longer running + const bgProcess = manager.getProcess(spawnResult.processId); + expect(bgProcess?.status).not.toBe("running"); + + tempDir[Symbol.dispose](); + }); + + it("should be idempotent (double-terminate succeeds)", async () => { + const manager = new BackgroundProcessManager(new BashExecutionService()); + const tempDir = new TestTempDir("test-bash-bg-term"); + const config = createTestToolConfig(process.cwd()); + config.runtimeTempDir = tempDir.path; + config.backgroundProcessManager = manager; + + // Spawn a process + const spawnResult = await manager.spawn("test-workspace", "sleep 10", { + cwd: process.cwd(), + }); + + if (!spawnResult.success) { + throw new Error("Failed to spawn process"); + } + + const tool = createBashBackgroundTerminateTool(config); + const args: BashBackgroundTerminateArgs = { + process_id: spawnResult.processId, + }; + + // First termination + const result1 = (await tool.execute!( + args, + mockToolCallOptions + )) as BashBackgroundTerminateResult; + expect(result1.success).toBe(true); + + // Second termination + const result2 = (await tool.execute!( + args, + mockToolCallOptions + )) as BashBackgroundTerminateResult; + expect(result2.success).toBe(true); + + tempDir[Symbol.dispose](); + }); + + it("should not terminate processes from other workspaces", async () => { + const manager = new BackgroundProcessManager(new BashExecutionService()); + const tempDir = new TestTempDir("test-bash-bg-term"); + // Config is for workspace-a + const config = createTestToolConfig(process.cwd(), { workspaceId: "workspace-a" }); + config.runtimeTempDir = tempDir.path; + config.backgroundProcessManager = manager; + + // Spawn process in workspace-b + const spawnResult = await manager.spawn("workspace-b", "sleep 10", { + cwd: process.cwd(), + }); + + if (!spawnResult.success) { + throw new Error("Failed to spawn process"); + } + + // Try to terminate from workspace-a (should fail) + const tool = createBashBackgroundTerminateTool(config); + const args: BashBackgroundTerminateArgs = { + process_id: spawnResult.processId, + }; + + const result = (await tool.execute!( + args, + mockToolCallOptions + )) as BashBackgroundTerminateResult; + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error).toContain("Process not found"); + } + + // Process should still be running + const proc = manager.getProcess(spawnResult.processId); + expect(proc?.status).toBe("running"); + + // Cleanup + await manager.terminate(spawnResult.processId); + tempDir[Symbol.dispose](); + }); +}); diff --git a/src/node/services/tools/bash_background_terminate.ts b/src/node/services/tools/bash_background_terminate.ts new file mode 100644 index 000000000..ddb343e84 --- /dev/null +++ b/src/node/services/tools/bash_background_terminate.ts @@ -0,0 +1,48 @@ +import { tool } from "ai"; +import type { BashBackgroundTerminateResult } from "@/common/types/tools"; +import type { ToolConfiguration, ToolFactory } from "@/common/utils/tools/tools"; +import { TOOL_DEFINITIONS } from "@/common/utils/tools/toolDefinitions"; + +/** + * Tool for terminating background processes + */ +export const createBashBackgroundTerminateTool: ToolFactory = (config: ToolConfiguration) => { + return tool({ + description: TOOL_DEFINITIONS.bash_background_terminate.description, + inputSchema: TOOL_DEFINITIONS.bash_background_terminate.schema, + execute: async ({ process_id }): Promise => { + if (!config.backgroundProcessManager) { + return { + success: false, + error: "Background process manager not available", + }; + } + + if (!config.workspaceId) { + return { + success: false, + error: "Workspace ID not available", + }; + } + + // Verify process belongs to this workspace before terminating + const process = config.backgroundProcessManager.getProcess(process_id); + if (!process || process.workspaceId !== config.workspaceId) { + return { + success: false, + error: `Process not found: ${process_id}`, + }; + } + + const result = await config.backgroundProcessManager.terminate(process_id); + if (result.success) { + return { + success: true, + message: `Process ${process_id} terminated`, + }; + } + + return result; + }, + }); +}; diff --git a/src/node/services/tools/status_set.test.ts b/src/node/services/tools/status_set.test.ts index 27c0c161d..05c176940 100644 --- a/src/node/services/tools/status_set.test.ts +++ b/src/node/services/tools/status_set.test.ts @@ -10,6 +10,7 @@ describe("status_set tool validation", () => { cwd: "/test", runtime: createRuntime({ type: "local", srcBaseDir: "/tmp" }), runtimeTempDir: "/tmp", + workspaceId: "test-workspace", }; const mockToolCallOptions: ToolCallOptions = { diff --git a/src/node/services/tools/testHelpers.ts b/src/node/services/tools/testHelpers.ts index 77beb749e..a753c4f0b 100644 --- a/src/node/services/tools/testHelpers.ts +++ b/src/node/services/tools/testHelpers.ts @@ -50,13 +50,14 @@ function getTestInitStateManager(): InitStateManager { */ export function createTestToolConfig( tempDir: string, - options?: { niceness?: number } + options?: { niceness?: number; workspaceId?: string } ): ToolConfiguration { return { cwd: tempDir, runtime: new LocalRuntime(tempDir), runtimeTempDir: tempDir, niceness: options?.niceness, + workspaceId: options?.workspaceId ?? "test-workspace", }; } From ef2bc7b57f48f7e4be4fd15e1185079e96f15312 Mon Sep 17 00:00:00 2001 From: ethan Date: Tue, 25 Nov 2025 17:34:11 +1100 Subject: [PATCH 02/13] =?UTF-8?q?=F0=9F=A4=96=20fix:=20block=20background?= =?UTF-8?q?=20execution=20for=20SSH/remote=20workspaces?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add supportsBackgroundExecution property to Runtime interface to detect runtime capability. SSH workspaces return clear error message instead of failing with ENOENT or running on wrong host. TODO comments added for future SSH support via nohup/setsid. --- src/node/runtime/LocalRuntime.ts | 3 +++ src/node/runtime/Runtime.ts | 7 +++++++ src/node/runtime/SSHRuntime.ts | 6 ++++++ src/node/services/tools/bash.ts | 10 ++++++++++ 4 files changed, 26 insertions(+) diff --git a/src/node/runtime/LocalRuntime.ts b/src/node/runtime/LocalRuntime.ts index 81012cd12..f20c9c169 100644 --- a/src/node/runtime/LocalRuntime.ts +++ b/src/node/runtime/LocalRuntime.ts @@ -39,6 +39,9 @@ import { expandTilde } from "./tildeExpansion"; export class LocalRuntime implements Runtime { private readonly srcBaseDir: string; + /** LocalRuntime supports background execution via detached process groups */ + readonly supportsBackgroundExecution = true; + constructor(srcBaseDir: string) { // Expand tilde to actual home directory path for local file system operations this.srcBaseDir = expandTilde(srcBaseDir); diff --git a/src/node/runtime/Runtime.ts b/src/node/runtime/Runtime.ts index 4e01a0ceb..035cbfbad 100644 --- a/src/node/runtime/Runtime.ts +++ b/src/node/runtime/Runtime.ts @@ -364,6 +364,13 @@ export interface Runtime { * @returns Result with new workspace path and source branch, or error */ forkWorkspace(params: WorkspaceForkParams): Promise; + + /** + * Whether this runtime supports background process execution. + * LocalRuntime: true (uses detached process groups) + * SSHRuntime: false (TODO: implement via nohup/setsid + file-based output capture) + */ + readonly supportsBackgroundExecution: boolean; } /** diff --git a/src/node/runtime/SSHRuntime.ts b/src/node/runtime/SSHRuntime.ts index 22588e873..619be2df4 100644 --- a/src/node/runtime/SSHRuntime.ts +++ b/src/node/runtime/SSHRuntime.ts @@ -71,6 +71,12 @@ export class SSHRuntime implements Runtime { private readonly config: SSHRuntimeConfig; private readonly controlPath: string; + /** + * SSHRuntime does not yet support background execution. + * TODO: Implement via nohup/setsid + file-based output capture on remote host + */ + readonly supportsBackgroundExecution = false; + constructor(config: SSHRuntimeConfig) { // Note: srcBaseDir may contain tildes - they will be resolved via resolvePath() before use // The WORKSPACE_CREATE IPC handler resolves paths before storing in config diff --git a/src/node/services/tools/bash.ts b/src/node/services/tools/bash.ts index f7228bca5..ffed5b009 100644 --- a/src/node/services/tools/bash.ts +++ b/src/node/services/tools/bash.ts @@ -249,6 +249,16 @@ export const createBashTool: ToolFactory = (config: ToolConfiguration) => { }; } + // TODO: Add SSH runtime support for background processes (nohup/setsid + file-based output) + if (!config.runtime.supportsBackgroundExecution) { + return { + success: false, + error: "Background execution is not yet supported for SSH/remote workspaces", + exitCode: -1, + wall_duration_ms: 0, + }; + } + if (!config.workspaceId || !config.backgroundProcessManager) { return { success: false, From e25206563e29153415d9986465e7de8cc84ad152 Mon Sep 17 00:00:00 2001 From: ethan Date: Tue, 25 Nov 2025 17:41:20 +1100 Subject: [PATCH 03/13] =?UTF-8?q?=F0=9F=A4=96=20fix:=20preserve=20killed?= =?UTF-8?q?=20status=20when=20onExit=20callback=20fires?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The onExit callback was unconditionally setting status to 'exited', overwriting the 'killed' status set by terminate(). Now guards with if (process.status === 'running') to preserve killed/failed states. Added test to verify killed status is preserved after termination. --- .../services/backgroundProcessManager.test.ts | 19 +++++++++++++++++++ src/node/services/backgroundProcessManager.ts | 7 +++++-- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/node/services/backgroundProcessManager.test.ts b/src/node/services/backgroundProcessManager.test.ts index b05e80f01..ccc1221a5 100644 --- a/src/node/services/backgroundProcessManager.test.ts +++ b/src/node/services/backgroundProcessManager.test.ts @@ -198,5 +198,24 @@ describe("BackgroundProcessManager", () => { expect(process?.stdoutBuffer.toArray()).toContain("test"); } }); + + it("should preserve killed status after onExit callback fires", async () => { + // Spawn a long-running process + const result = await manager.spawn("workspace-1", "sleep 60", { + cwd: process.cwd(), + }); + + if (result.success) { + // Terminate it + await manager.terminate(result.processId); + + // Wait for onExit callback to fire + await new Promise((resolve) => setTimeout(resolve, 100)); + + // Status should still be "killed", not "exited" + const proc = manager.getProcess(result.processId); + expect(proc?.status).toBe("killed"); + } + }); }); }); diff --git a/src/node/services/backgroundProcessManager.ts b/src/node/services/backgroundProcessManager.ts index adaae7fda..d9782a409 100644 --- a/src/node/services/backgroundProcessManager.ts +++ b/src/node/services/backgroundProcessManager.ts @@ -74,8 +74,11 @@ export class BackgroundProcessManager { onExit: (exitCode: number) => { log.debug(`Background process ${processId} exited with code ${exitCode}`); process.exitCode = exitCode; - process.exitTime = Date.now(); - process.status = "exited"; + process.exitTime ??= Date.now(); + // Don't overwrite status if already marked as killed/failed by terminate() + if (process.status === "running") { + process.status = "exited"; + } }, }); From d681cebc26bd2ebf1e7ac63734f0a32d73456896 Mon Sep 17 00:00:00 2001 From: ethan Date: Tue, 25 Nov 2025 17:59:16 +1100 Subject: [PATCH 04/13] =?UTF-8?q?=F0=9F=A4=96=20fix:=20convert=20signal-te?= =?UTF-8?q?rminated=20processes=20to=20non-zero=20exit=20codes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a process is killed by signal (SIGTERM, SIGKILL, etc.), Node's close event provides code=null and signal name. Previously we defaulted null to 0, making killed processes appear as successful exits. Now converts signals to Unix-conventional exit codes (128 + signal_number): - SIGKILL (9) → 137 - SIGTERM (15) → 143 - SIGINT (2) → 130 This allows bash_background_read/list to distinguish clean exits from forced terminations (via terminate() or external kill/OOM). --- src/node/services/bashExecutionService.ts | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/src/node/services/bashExecutionService.ts b/src/node/services/bashExecutionService.ts index 9f46c455c..f7be41115 100644 --- a/src/node/services/bashExecutionService.ts +++ b/src/node/services/bashExecutionService.ts @@ -171,8 +171,10 @@ export class BashExecutionService { errBuf = flushLines(errBuf, true); }); - child.on("close", (code: number | null) => { - log.debug(`BashExecutionService: Process exited with code ${code ?? "unknown"}`); + child.on("close", (code: number | null, signal: NodeJS.Signals | null) => { + log.debug( + `BashExecutionService: Process exited with code ${code ?? "null"}, signal ${signal ?? "none"}` + ); // Flush any remaining partial lines if (outBuf.trim().length > 0) { callbacks.onStdout(outBuf); @@ -180,7 +182,22 @@ export class BashExecutionService { if (errBuf.trim().length > 0) { callbacks.onStderr(errBuf); } - callbacks.onExit(code ?? 0); + + // Convert signal to exit code using Unix convention (128 + signal number) + // Common signals: SIGTERM=15 → 143, SIGKILL=9 → 137 + let exitCode = code ?? 0; + if (code === null && signal) { + const signalNumbers: Record = { + SIGHUP: 1, + SIGINT: 2, + SIGQUIT: 3, + SIGABRT: 6, + SIGKILL: 9, + SIGTERM: 15, + }; + exitCode = 128 + (signalNumbers[signal] ?? 1); + } + callbacks.onExit(exitCode); }); child.on("error", (error: Error) => { From caf0c3e9f73b25ac3b8fc854f28de3a34826b856 Mon Sep 17 00:00:00 2001 From: ethan Date: Tue, 25 Nov 2025 18:55:12 +1100 Subject: [PATCH 05/13] =?UTF-8?q?=F0=9F=A4=96=20refactor:=20extract=20Back?= =?UTF-8?q?groundExecutor=20interface=20for=20runtime-agnostic=20backgroun?= =?UTF-8?q?d=20execution?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add BackgroundExecutor interface and BackgroundHandle for abstracting process spawning - Create LocalBackgroundExecutor wrapping BashExecutionService for local processes - Add SSHBackgroundExecutor stub (returns 'not yet implemented' error) - BackgroundProcessManager now uses per-workspace executor registration - Remove pid from public API (runtime-specific detail) - Remove supportsBackgroundExecution from Runtime interface (executor error suffices) - Share single BashExecutionService instance across all local executors This refactor prepares for SSH background execution support by making the execution backend swappable per workspace. --- src/common/types/tools.ts | 2 - src/node/runtime/LocalRuntime.ts | 3 - src/node/runtime/Runtime.ts | 7 - src/node/runtime/SSHRuntime.ts | 6 - src/node/services/backgroundExecutor.ts | 87 ++++++++ .../services/backgroundProcessManager.test.ts | 93 +++++--- src/node/services/backgroundProcessManager.ts | 174 ++++++++------- src/node/services/ipcMain.ts | 26 ++- src/node/services/localBackgroundExecutor.ts | 198 ++++++++++++++++++ src/node/services/sshBackgroundExecutor.ts | 43 ++++ src/node/services/tools/bash.test.ts | 7 +- src/node/services/tools/bash.ts | 10 - .../tools/bash_background_list.test.ts | 20 +- .../services/tools/bash_background_list.ts | 1 - .../tools/bash_background_read.test.ts | 23 +- .../services/tools/bash_background_read.ts | 1 - .../tools/bash_background_terminate.test.ts | 19 +- 17 files changed, 554 insertions(+), 166 deletions(-) create mode 100644 src/node/services/backgroundExecutor.ts create mode 100644 src/node/services/localBackgroundExecutor.ts create mode 100644 src/node/services/sshBackgroundExecutor.ts diff --git a/src/common/types/tools.ts b/src/common/types/tools.ts index 8d1454d4f..8e9850c26 100644 --- a/src/common/types/tools.ts +++ b/src/common/types/tools.ts @@ -212,7 +212,6 @@ export type BashBackgroundReadResult = process_id: string; status: "running" | "exited" | "killed" | "failed"; script: string; - pid?: number; // OS process ID uptime_ms: number; exitCode?: number; stdout: string[]; @@ -238,7 +237,6 @@ export interface BashBackgroundListProcess { process_id: string; status: "running" | "exited" | "killed" | "failed"; script: string; - pid?: number; uptime_ms: number; exitCode?: number; } diff --git a/src/node/runtime/LocalRuntime.ts b/src/node/runtime/LocalRuntime.ts index f20c9c169..81012cd12 100644 --- a/src/node/runtime/LocalRuntime.ts +++ b/src/node/runtime/LocalRuntime.ts @@ -39,9 +39,6 @@ import { expandTilde } from "./tildeExpansion"; export class LocalRuntime implements Runtime { private readonly srcBaseDir: string; - /** LocalRuntime supports background execution via detached process groups */ - readonly supportsBackgroundExecution = true; - constructor(srcBaseDir: string) { // Expand tilde to actual home directory path for local file system operations this.srcBaseDir = expandTilde(srcBaseDir); diff --git a/src/node/runtime/Runtime.ts b/src/node/runtime/Runtime.ts index 035cbfbad..4e01a0ceb 100644 --- a/src/node/runtime/Runtime.ts +++ b/src/node/runtime/Runtime.ts @@ -364,13 +364,6 @@ export interface Runtime { * @returns Result with new workspace path and source branch, or error */ forkWorkspace(params: WorkspaceForkParams): Promise; - - /** - * Whether this runtime supports background process execution. - * LocalRuntime: true (uses detached process groups) - * SSHRuntime: false (TODO: implement via nohup/setsid + file-based output capture) - */ - readonly supportsBackgroundExecution: boolean; } /** diff --git a/src/node/runtime/SSHRuntime.ts b/src/node/runtime/SSHRuntime.ts index 619be2df4..22588e873 100644 --- a/src/node/runtime/SSHRuntime.ts +++ b/src/node/runtime/SSHRuntime.ts @@ -71,12 +71,6 @@ export class SSHRuntime implements Runtime { private readonly config: SSHRuntimeConfig; private readonly controlPath: string; - /** - * SSHRuntime does not yet support background execution. - * TODO: Implement via nohup/setsid + file-based output capture on remote host - */ - readonly supportsBackgroundExecution = false; - constructor(config: SSHRuntimeConfig) { // Note: srcBaseDir may contain tildes - they will be resolved via resolvePath() before use // The WORKSPACE_CREATE IPC handler resolves paths before storing in config diff --git a/src/node/services/backgroundExecutor.ts b/src/node/services/backgroundExecutor.ts new file mode 100644 index 000000000..c8327d58f --- /dev/null +++ b/src/node/services/backgroundExecutor.ts @@ -0,0 +1,87 @@ +/** + * Background process execution abstraction. + * + * This interface allows BackgroundProcessManager to work with different + * execution backends (local processes, SSH remote processes, etc.) + */ + +/** + * Configuration for background execution + */ +export interface BackgroundExecConfig { + /** Working directory for command execution */ + cwd: string; + /** Environment variables to inject */ + env?: Record; + /** Process niceness level (-20 to 19) */ + niceness?: number; +} + +/** + * Handle to a background process. + * Abstracts away whether process is local or remote. + */ +export interface BackgroundHandle { + /** + * Register callback for stdout lines. + * For local: called in real-time as output arrives. + * For SSH: called when output is polled/read. + */ + onStdout(callback: (line: string) => void): void; + + /** + * Register callback for stderr lines. + */ + onStderr(callback: (line: string) => void): void; + + /** + * Register callback for process exit. + * @param callback Receives exit code (128+signal for signal termination) + */ + onExit(callback: (exitCode: number) => void): void; + + /** + * Check if process is still running. + * For local: checks ChildProcess.exitCode + * For SSH: runs `kill -0 $PID` on remote + */ + isRunning(): Promise; + + /** + * Terminate the process (SIGTERM → wait → SIGKILL). + * For local: process.kill(-pid, signal) + * For SSH: ssh "kill -TERM -$PID" + */ + terminate(): Promise; + + /** + * Clean up resources (called after process exits or on error). + * For local: disposes ChildProcess + * For SSH: removes remote temp files + */ + dispose(): Promise; +} + +/** + * Result of spawning a background process + */ +export type BackgroundSpawnResult = + | { success: true; handle: BackgroundHandle } + | { success: false; error: string }; + +/** + * Executor interface for spawning background processes. + * + * Implementations: + * - LocalBackgroundExecutor: Uses BashExecutionService for local processes + * - SSHBackgroundExecutor: Uses nohup/setsid + file-based output (TODO) + */ +export interface BackgroundExecutor { + /** + * Spawn a background process. + * @param script Bash script to execute + * @param config Execution configuration + * @returns BackgroundHandle on success, or error + */ + spawn(script: string, config: BackgroundExecConfig): Promise; +} diff --git a/src/node/services/backgroundProcessManager.test.ts b/src/node/services/backgroundProcessManager.test.ts index ccc1221a5..43e4148cc 100644 --- a/src/node/services/backgroundProcessManager.test.ts +++ b/src/node/services/backgroundProcessManager.test.ts @@ -1,19 +1,27 @@ import { describe, it, expect, beforeEach } from "bun:test"; import { BackgroundProcessManager } from "./backgroundProcessManager"; import { BashExecutionService } from "./bashExecutionService"; +import { LocalBackgroundExecutor } from "./localBackgroundExecutor"; + +// Helper to create manager with executor registered for a workspace +function createManagerWithExecutor(workspaceId: string): BackgroundProcessManager { + const manager = new BackgroundProcessManager(); + manager.registerExecutor(workspaceId, new LocalBackgroundExecutor(new BashExecutionService())); + return manager; +} describe("BackgroundProcessManager", () => { let manager: BackgroundProcessManager; - let bashService: BashExecutionService; + const testWorkspaceId = "workspace-1"; + const testWorkspaceId2 = "workspace-2"; beforeEach(() => { - bashService = new BashExecutionService(); - manager = new BackgroundProcessManager(bashService); + manager = createManagerWithExecutor(testWorkspaceId); }); describe("spawn", () => { it("should spawn a background process and return process ID", async () => { - const result = await manager.spawn("workspace-1", "echo hello", { + const result = await manager.spawn(testWorkspaceId, "echo hello", { cwd: process.cwd(), }); @@ -23,8 +31,20 @@ describe("BackgroundProcessManager", () => { } }); + it("should return error when no executor is registered", async () => { + const managerNoExecutor = new BackgroundProcessManager(); + const result = await managerNoExecutor.spawn("unregistered-workspace", "echo hello", { + cwd: process.cwd(), + }); + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error).toContain("No executor registered"); + } + }); + it("should return error on spawn failure", async () => { - const result = await manager.spawn("workspace-1", "echo test", { + const result = await manager.spawn(testWorkspaceId, "echo test", { cwd: "/nonexistent/path/that/does/not/exist", }); @@ -32,7 +52,7 @@ describe("BackgroundProcessManager", () => { }); it("should capture stdout and stderr", async () => { - const result = await manager.spawn("workspace-1", "echo hello; echo world >&2", { + const result = await manager.spawn(testWorkspaceId, "echo hello; echo world >&2", { cwd: process.cwd(), }); @@ -55,7 +75,7 @@ describe("BackgroundProcessManager", () => { .map((_, i) => `echo line${i}`) .join("; "); - const result = await manager.spawn("workspace-1", script, { + const result = await manager.spawn(testWorkspaceId, script, { cwd: process.cwd(), }); @@ -73,7 +93,7 @@ describe("BackgroundProcessManager", () => { describe("getProcess", () => { it("should return process by ID", async () => { - const spawnResult = await manager.spawn("workspace-1", "sleep 1", { + const spawnResult = await manager.spawn(testWorkspaceId, "sleep 1", { cwd: process.cwd(), }); @@ -93,30 +113,36 @@ describe("BackgroundProcessManager", () => { describe("list", () => { it("should list all processes", async () => { - await manager.spawn("workspace-1", "sleep 1", { cwd: process.cwd() }); - await manager.spawn("workspace-1", "sleep 1", { cwd: process.cwd() }); + await manager.spawn(testWorkspaceId, "sleep 1", { cwd: process.cwd() }); + await manager.spawn(testWorkspaceId, "sleep 1", { cwd: process.cwd() }); const processes = manager.list(); expect(processes.length).toBeGreaterThanOrEqual(2); }); it("should filter by workspace ID", async () => { - await manager.spawn("workspace-1", "sleep 1", { cwd: process.cwd() }); - await manager.spawn("workspace-2", "sleep 1", { cwd: process.cwd() }); + // Register second workspace executor + manager.registerExecutor( + testWorkspaceId2, + new LocalBackgroundExecutor(new BashExecutionService()) + ); - const ws1Processes = manager.list("workspace-1"); - const ws2Processes = manager.list("workspace-2"); + await manager.spawn(testWorkspaceId, "sleep 1", { cwd: process.cwd() }); + await manager.spawn(testWorkspaceId2, "sleep 1", { cwd: process.cwd() }); + + const ws1Processes = manager.list(testWorkspaceId); + const ws2Processes = manager.list(testWorkspaceId2); expect(ws1Processes.length).toBeGreaterThanOrEqual(1); expect(ws2Processes.length).toBeGreaterThanOrEqual(1); - expect(ws1Processes.every((p) => p.workspaceId === "workspace-1")).toBe(true); - expect(ws2Processes.every((p) => p.workspaceId === "workspace-2")).toBe(true); + expect(ws1Processes.every((p) => p.workspaceId === testWorkspaceId)).toBe(true); + expect(ws2Processes.every((p) => p.workspaceId === testWorkspaceId2)).toBe(true); }); }); describe("terminate", () => { it("should terminate a running process", async () => { - const spawnResult = await manager.spawn("workspace-1", "sleep 10", { + const spawnResult = await manager.spawn(testWorkspaceId, "sleep 10", { cwd: process.cwd(), }); @@ -135,7 +161,7 @@ describe("BackgroundProcessManager", () => { }); it("should be idempotent (double-terminate succeeds)", async () => { - const spawnResult = await manager.spawn("workspace-1", "sleep 10", { + const spawnResult = await manager.spawn(testWorkspaceId, "sleep 10", { cwd: process.cwd(), }); @@ -151,17 +177,28 @@ describe("BackgroundProcessManager", () => { describe("cleanup", () => { it("should kill all processes for a workspace and remove them from memory", async () => { - await manager.spawn("workspace-1", "sleep 10", { cwd: process.cwd() }); - await manager.spawn("workspace-1", "sleep 10", { cwd: process.cwd() }); - await manager.spawn("workspace-2", "sleep 10", { cwd: process.cwd() }); + // Register second workspace executor + manager.registerExecutor( + testWorkspaceId2, + new LocalBackgroundExecutor(new BashExecutionService()) + ); - await manager.cleanup("workspace-1"); + await manager.spawn(testWorkspaceId, "sleep 10", { cwd: process.cwd() }); + await manager.spawn(testWorkspaceId, "sleep 10", { cwd: process.cwd() }); + await manager.spawn(testWorkspaceId2, "sleep 10", { cwd: process.cwd() }); - const ws1Processes = manager.list("workspace-1"); - const ws2Processes = manager.list("workspace-2"); + await manager.cleanup(testWorkspaceId); - // All workspace-1 processes should be removed from memory + const ws1Processes = manager.list(testWorkspaceId); + const ws2Processes = manager.list(testWorkspaceId2); + // All testWorkspaceId processes should be removed from memory expect(ws1Processes.length).toBe(0); + // Executor should also be unregistered - spawning should fail + const spawnResult = await manager.spawn(testWorkspaceId, "echo test", { cwd: process.cwd() }); + expect(spawnResult.success).toBe(false); + if (!spawnResult.success) { + expect(spawnResult.error).toContain("No executor registered"); + } // workspace-2 processes should still exist and be running expect(ws2Processes.length).toBeGreaterThanOrEqual(1); expect(ws2Processes.some((p) => p.status === "running")).toBe(true); @@ -170,7 +207,7 @@ describe("BackgroundProcessManager", () => { describe("process state tracking", () => { it("should track process exit", async () => { - const result = await manager.spawn("workspace-1", "exit 42", { + const result = await manager.spawn(testWorkspaceId, "exit 42", { cwd: process.cwd(), }); @@ -186,7 +223,7 @@ describe("BackgroundProcessManager", () => { }); it("should keep buffer after process exits", async () => { - const result = await manager.spawn("workspace-1", "echo test; exit 0", { + const result = await manager.spawn(testWorkspaceId, "echo test; exit 0", { cwd: process.cwd(), }); @@ -201,7 +238,7 @@ describe("BackgroundProcessManager", () => { it("should preserve killed status after onExit callback fires", async () => { // Spawn a long-running process - const result = await manager.spawn("workspace-1", "sleep 60", { + const result = await manager.spawn(testWorkspaceId, "sleep 60", { cwd: process.cwd(), }); diff --git a/src/node/services/backgroundProcessManager.ts b/src/node/services/backgroundProcessManager.ts index d9782a409..b88e1aaae 100644 --- a/src/node/services/backgroundProcessManager.ts +++ b/src/node/services/backgroundProcessManager.ts @@ -1,11 +1,6 @@ -import type { - BashExecutionService, - BashExecutionConfig, - DisposableProcess, -} from "./bashExecutionService"; +import type { BackgroundExecutor, BackgroundHandle } from "./backgroundExecutor"; import { log } from "./log"; import { randomBytes } from "crypto"; -import { once } from "node:events"; import { CircularBuffer } from "./circularBuffer"; /** @@ -15,25 +10,45 @@ export interface BackgroundProcess { id: string; // Short unique ID (e.g., "bg-abc123") workspaceId: string; // Owning workspace script: string; // Original command - pid?: number; // Process ID (undefined if spawn failed) startTime: number; // Timestamp when started stdoutBuffer: CircularBuffer; // Circular buffer (max 1000 lines) stderrBuffer: CircularBuffer; // Circular buffer (max 1000 lines) exitCode?: number; // Undefined if still running exitTime?: number; // Timestamp when exited (undefined if running) status: "running" | "exited" | "killed" | "failed"; - disposable: DisposableProcess | null; // For process cleanup + handle: BackgroundHandle | null; // For process interaction } const MAX_BUFFER_LINES = 1000; /** - * Manages background bash processes for workspaces + * Manages background bash processes for workspaces. + * + * Each workspace registers its own executor (local or SSH) at creation time. + * This allows different execution backends per workspace. */ export class BackgroundProcessManager { private processes = new Map(); + private executors = new Map(); - constructor(private readonly bashExecutionService: BashExecutionService) {} + /** + * Register an executor for a workspace. + * Called when workspace is created - local workspaces get LocalBackgroundExecutor, + * SSH workspaces get SSHBackgroundExecutor. + */ + registerExecutor(workspaceId: string, executor: BackgroundExecutor): void { + log.debug(`BackgroundProcessManager.registerExecutor(${workspaceId})`); + this.executors.set(workspaceId, executor); + } + + /** + * Unregister executor for a workspace. + * Called when workspace is deleted. + */ + unregisterExecutor(workspaceId: string): void { + log.debug(`BackgroundProcessManager.unregisterExecutor(${workspaceId})`); + this.executors.delete(workspaceId); + } /** * Spawn a new background process @@ -41,10 +56,19 @@ export class BackgroundProcessManager { async spawn( workspaceId: string, script: string, - config: BashExecutionConfig + config: { cwd: string; secrets?: Record; niceness?: number } ): Promise<{ success: true; processId: string } | { success: false; error: string }> { log.debug(`BackgroundProcessManager.spawn() called for workspace ${workspaceId}`); + // Get executor for this workspace + const executor = this.executors.get(workspaceId); + if (!executor) { + return { + success: false, + error: "No executor registered for this workspace. Background execution may not be supported.", + }; + } + // Generate unique process ID const processId = `bg-${randomBytes(4).toString("hex")}`; @@ -52,7 +76,7 @@ export class BackgroundProcessManager { const stdoutBuffer = new CircularBuffer(MAX_BUFFER_LINES); const stderrBuffer = new CircularBuffer(MAX_BUFFER_LINES); - const process: BackgroundProcess = { + const proc: BackgroundProcess = { id: processId, workspaceId, script, @@ -60,65 +84,48 @@ export class BackgroundProcessManager { stdoutBuffer, stderrBuffer, status: "running", - disposable: null, + handle: null, }; - // Spawn with streaming callbacks - const disposable = this.bashExecutionService.executeStreaming(script, config, { - onStdout: (line: string) => { - stdoutBuffer.push(line); - }, - onStderr: (line: string) => { - stderrBuffer.push(line); - }, - onExit: (exitCode: number) => { - log.debug(`Background process ${processId} exited with code ${exitCode}`); - process.exitCode = exitCode; - process.exitTime ??= Date.now(); - // Don't overwrite status if already marked as killed/failed by terminate() - if (process.status === "running") { - process.status = "exited"; - } - }, + // Spawn via executor + const result = await executor.spawn(script, { + cwd: config.cwd, + env: config.secrets, + niceness: config.niceness, }); - const child = disposable.child; - - // Wait until we know whether the spawn succeeded or failed - // ChildProcess emits either 'spawn' (success) or 'error' (failure) - mutually exclusive - try { - await Promise.race([ - // Successful spawn - once(child, "spawn"), - - // Spawn error (ENOENT, invalid cwd, etc.) - once(child, "error").then(([err]) => { - throw err; - }), - - // Safety timeout to prevent infinite hang - new Promise((_, reject) => - setTimeout(() => reject(new Error("Spawn did not complete in time")), 2000) - ), - ]); - } catch (e) { - const err = e as Error; - log.debug(`Failed to spawn background process: ${err.message}`); - disposable[Symbol.dispose](); - return { - success: false, - error: err.message, - }; + if (!result.success) { + log.debug(`BackgroundProcessManager: Failed to spawn: ${result.error}`); + return { success: false, error: result.error }; } - // At this point we know the process spawned successfully - process.disposable = disposable; - process.pid = child.pid ?? undefined; + const handle = result.handle; + + // Wire up callbacks to buffers + handle.onStdout((line: string) => { + stdoutBuffer.push(line); + }); + + handle.onStderr((line: string) => { + stderrBuffer.push(line); + }); + + handle.onExit((exitCode: number) => { + log.debug(`Background process ${processId} exited with code ${exitCode}`); + proc.exitCode = exitCode; + proc.exitTime ??= Date.now(); + // Don't overwrite status if already marked as killed/failed by terminate() + if (proc.status === "running") { + proc.status = "exited"; + } + }); + + proc.handle = handle; // Store process in map - this.processes.set(processId, process); + this.processes.set(processId, proc); - log.debug(`Background process ${processId} spawned with PID ${process.pid ?? "unknown"}`); + log.debug(`Background process ${processId} spawned successfully`); return { success: true, processId }; } @@ -159,40 +166,23 @@ export class BackgroundProcessManager { return { success: true }; } - // Check if we have a valid PID - if (!proc.pid || !proc.disposable) { - log.debug(`Process ${processId} has no PID or disposable, marking as failed`); + // Check if we have a valid handle + if (!proc.handle) { + log.debug(`Process ${processId} has no handle, marking as failed`); proc.status = "failed"; proc.exitTime = Date.now(); return { success: true }; } try { - // Send SIGTERM to the process group for graceful shutdown - // Use negative PID to kill the entire process group (detached processes are group leaders) - // This ensures child processes (e.g., from npm run dev) are also terminated - const pgid = -proc.pid; - log.debug(`Sending SIGTERM to process group ${processId} (PGID: ${pgid})`); - process.kill(pgid, "SIGTERM"); - - // Wait 2 seconds for graceful shutdown - await new Promise((resolve) => setTimeout(resolve, 2000)); - - // Check if process is still running - const stillRunning = proc.disposable.child.exitCode === null; - - if (stillRunning) { - // Force kill the process group with SIGKILL - log.debug(`Process group ${processId} still running, sending SIGKILL`); - process.kill(pgid, "SIGKILL"); - } + await proc.handle.terminate(); // Update process status proc.status = "killed"; proc.exitTime ??= Date.now(); - // Dispose of the process - proc.disposable[Symbol.dispose](); + // Dispose of the handle + await proc.handle.dispose(); log.debug(`Process ${processId} terminated successfully`); return { success: true }; @@ -202,17 +192,17 @@ export class BackgroundProcessManager { // Mark as killed even if there was an error (process likely already dead) proc.status = "killed"; proc.exitTime ??= Date.now(); - // Ensure disposable is cleaned up even on error - if (proc.disposable) { - proc.disposable[Symbol.dispose](); + // Ensure handle is cleaned up even on error + if (proc.handle) { + await proc.handle.dispose().catch(() => {}); } return { success: true }; } } /** - * Clean up all processes for a workspace - * Terminates running processes and removes them from memory + * Clean up all processes for a workspace. + * Terminates running processes, removes them from memory, and unregisters the executor. */ async cleanup(workspaceId: string): Promise { log.debug(`BackgroundProcessManager.cleanup(${workspaceId}) called`); @@ -225,6 +215,10 @@ export class BackgroundProcessManager { // Remove all processes from memory matching.forEach((p) => this.processes.delete(p.id)); + + // Unregister the executor for this workspace + this.unregisterExecutor(workspaceId); + log.debug(`Cleaned up ${matching.length} process(es) for workspace ${workspaceId}`); } } diff --git a/src/node/services/ipcMain.ts b/src/node/services/ipcMain.ts index 3ffe000d6..54ba1ca8a 100644 --- a/src/node/services/ipcMain.ts +++ b/src/node/services/ipcMain.ts @@ -40,7 +40,10 @@ import { createRuntime } from "@/node/runtime/runtimeFactory"; import type { RuntimeConfig } from "@/common/types/runtime"; import { BackgroundProcessManager } from "@/node/services/backgroundProcessManager"; import { BashExecutionService } from "@/node/services/bashExecutionService"; +import { LocalBackgroundExecutor } from "@/node/services/localBackgroundExecutor"; +import { SSHBackgroundExecutor } from "@/node/services/sshBackgroundExecutor"; import { isSSHRuntime } from "@/common/types/runtime"; +import type { Runtime } from "@/node/runtime/Runtime"; import { validateProjectPath } from "@/node/utils/pathUtils"; import { PTYService } from "@/node/services/ptyService"; import type { TerminalWindowManager } from "@/desktop/terminalWindowManager"; @@ -69,6 +72,7 @@ export class IpcMain { private readonly extensionMetadata: ExtensionMetadataService; private readonly ptyService: PTYService; private readonly backgroundProcessManager: BackgroundProcessManager; + private readonly bashExecutionService: BashExecutionService; private terminalWindowManager?: TerminalWindowManager; private readonly sessions = new Map(); private projectDirectoryPicker?: (event: IpcMainInvokeEvent) => Promise; @@ -89,7 +93,8 @@ export class IpcMain { this.extensionMetadata = new ExtensionMetadataService( path.join(config.rootDir, "extensionMetadata.json") ); - this.backgroundProcessManager = new BackgroundProcessManager(new BashExecutionService()); + this.backgroundProcessManager = new BackgroundProcessManager(); + this.bashExecutionService = new BashExecutionService(); this.aiService = new AIService( config, @@ -105,6 +110,17 @@ export class IpcMain { this.setupMetadataListeners(); } + /** + * Create a background executor appropriate for the given runtime. + * Local runtimes use LocalBackgroundExecutor, SSH runtimes use SSHBackgroundExecutor. + */ + private createBackgroundExecutor(runtimeConfig: RuntimeConfig, runtime: Runtime) { + if (isSSHRuntime(runtimeConfig)) { + return new SSHBackgroundExecutor(runtime); + } + return new LocalBackgroundExecutor(this.bashExecutionService); + } + /** * Initialize the service. Call this after construction. * This is separate from the constructor to support async initialization. @@ -360,6 +376,10 @@ export class IpcMain { session.emitMetadata(completeMetadata); + // Register background executor for this workspace + const executor = this.createBackgroundExecutor(finalRuntimeConfig, runtime); + this.backgroundProcessManager.registerExecutor(workspaceId, executor); + void runtime .initWorkspace({ projectPath, @@ -683,6 +703,10 @@ export class IpcMain { // Emit metadata event for new workspace (session already created above) session.emitMetadata(completeMetadata); + // Register background executor for this workspace + const executor = this.createBackgroundExecutor(finalRuntimeConfig, runtime); + this.backgroundProcessManager.registerExecutor(workspaceId, executor); + // Phase 2: Initialize workspace asynchronously (SLOW - runs in background) // This streams progress via initLogger and doesn't block the IPC return void runtime diff --git a/src/node/services/localBackgroundExecutor.ts b/src/node/services/localBackgroundExecutor.ts new file mode 100644 index 000000000..399fc85f8 --- /dev/null +++ b/src/node/services/localBackgroundExecutor.ts @@ -0,0 +1,198 @@ +/** + * Local background executor implementation. + * + * Uses BashExecutionService to spawn detached process groups on the local machine. + * Output is streamed in real-time via callbacks. + */ + +import { once } from "node:events"; +import type { + BackgroundExecutor, + BackgroundExecConfig, + BackgroundHandle, + BackgroundSpawnResult, +} from "./backgroundExecutor"; +import type { BashExecutionService, DisposableProcess } from "./bashExecutionService"; +import { log } from "./log"; + +/** + * Handle to a local background process + * + * Buffers early events until callbacks are registered, since the manager + * registers callbacks after spawn() returns (but output may arrive before). + */ +class LocalBackgroundHandle implements BackgroundHandle { + private stdoutCallback?: (line: string) => void; + private stderrCallback?: (line: string) => void; + private exitCallback?: (exitCode: number) => void; + private terminated = false; + + // Buffers for events that arrive before callbacks are registered + private pendingStdout: string[] = []; + private pendingStderr: string[] = []; + private pendingExitCode?: number; + + constructor(private readonly disposable: DisposableProcess) {} + + onStdout(callback: (line: string) => void): void { + this.stdoutCallback = callback; + // Flush buffered events + for (const line of this.pendingStdout) { + callback(line); + } + this.pendingStdout = []; + } + + onStderr(callback: (line: string) => void): void { + this.stderrCallback = callback; + // Flush buffered events + for (const line of this.pendingStderr) { + callback(line); + } + this.pendingStderr = []; + } + + onExit(callback: (exitCode: number) => void): void { + this.exitCallback = callback; + // Flush buffered event + if (this.pendingExitCode !== undefined) { + callback(this.pendingExitCode); + this.pendingExitCode = undefined; + } + } + + /** Internal: called by executor when stdout line arrives */ + _emitStdout(line: string): void { + if (this.stdoutCallback) { + this.stdoutCallback(line); + } else { + this.pendingStdout.push(line); + } + } + + /** Internal: called by executor when stderr line arrives */ + _emitStderr(line: string): void { + if (this.stderrCallback) { + this.stderrCallback(line); + } else { + this.pendingStderr.push(line); + } + } + + /** Internal: called by executor when process exits */ + _emitExit(exitCode: number): void { + if (this.exitCallback) { + this.exitCallback(exitCode); + } else { + this.pendingExitCode = exitCode; + } + } + + async isRunning(): Promise { + return this.disposable.child.exitCode === null; + } + + async terminate(): Promise { + if (this.terminated) return; + + const pid = this.disposable.child.pid; + if (pid === undefined) { + this.terminated = true; + return; + } + + try { + // Send SIGTERM to the process group for graceful shutdown + // Use negative PID to kill the entire process group (detached processes are group leaders) + const pgid = -pid; + log.debug(`LocalBackgroundHandle: Sending SIGTERM to process group (PGID: ${pgid})`); + process.kill(pgid, "SIGTERM"); + + // Wait 2 seconds for graceful shutdown + await new Promise((resolve) => setTimeout(resolve, 2000)); + + // Check if process is still running + if (await this.isRunning()) { + log.debug(`LocalBackgroundHandle: Process still running, sending SIGKILL`); + process.kill(pgid, "SIGKILL"); + } + } catch (error) { + // Process may already be dead - that's fine + log.debug( + `LocalBackgroundHandle: Error during terminate: ${error instanceof Error ? error.message : String(error)}` + ); + } + + this.terminated = true; + } + + async dispose(): Promise { + this.disposable[Symbol.dispose](); + } + + /** Get the child process (for spawn event waiting) */ + get child() { + return this.disposable.child; + } +} + +/** + * Local background executor using BashExecutionService + */ +export class LocalBackgroundExecutor implements BackgroundExecutor { + constructor(private readonly bashService: BashExecutionService) {} + + async spawn(script: string, config: BackgroundExecConfig): Promise { + log.debug(`LocalBackgroundExecutor: Spawning background process in ${config.cwd}`); + + // Create handle first so we can wire up callbacks + let handle: LocalBackgroundHandle; + + // Spawn with streaming callbacks that forward to handle + const disposable = this.bashService.executeStreaming( + script, + { + cwd: config.cwd, + secrets: config.env, + niceness: config.niceness, + detached: true, + }, + { + onStdout: (line: string) => handle._emitStdout(line), + onStderr: (line: string) => handle._emitStderr(line), + onExit: (exitCode: number) => handle._emitExit(exitCode), + } + ); + + handle = new LocalBackgroundHandle(disposable); + + // Wait until we know whether the spawn succeeded or failed + // ChildProcess emits either 'spawn' (success) or 'error' (failure) - mutually exclusive + try { + await Promise.race([ + // Successful spawn + once(handle.child, "spawn"), + + // Spawn error (ENOENT, invalid cwd, etc.) + once(handle.child, "error").then(([err]) => { + throw err; + }), + + // Safety timeout to prevent infinite hang + new Promise((_, reject) => + setTimeout(() => reject(new Error("Spawn did not complete in time")), 2000) + ), + ]); + } catch (e) { + const err = e as Error; + log.debug(`LocalBackgroundExecutor: Failed to spawn: ${err.message}`); + await handle.dispose(); + return { success: false, error: err.message }; + } + + log.debug( + `LocalBackgroundExecutor: Process spawned with PID ${handle.child.pid ?? "unknown"}` + ); + return { success: true, handle }; + } +} diff --git a/src/node/services/sshBackgroundExecutor.ts b/src/node/services/sshBackgroundExecutor.ts new file mode 100644 index 000000000..b29a2ca02 --- /dev/null +++ b/src/node/services/sshBackgroundExecutor.ts @@ -0,0 +1,43 @@ +/** + * SSH background executor implementation (STUB). + * + * TODO: Implement using nohup/setsid + file-based output capture. + * + * Implementation approach: + * 1. Spawn: ssh "mkdir -p /tmp/mux-bg/$ID && nohup setsid bash -c 'SCRIPT' > /tmp/mux-bg/$ID/out 2>&1 & echo $!" + * 2. Read: ssh "tail -c +$OFFSET /tmp/mux-bg/$ID/out" (on-demand) + * 3. Status: ssh "kill -0 $PID && echo running || cat /tmp/mux-bg/$ID/exitcode" + * 4. Terminate: ssh "kill -TERM -$PID" then "kill -KILL -$PID" + * 5. Cleanup: ssh "rm -rf /tmp/mux-bg/$ID" + * + * Exit code capture requires wrapper script: + * bash -c 'SCRIPT; echo $? > /tmp/mux-bg/$ID/exitcode' + */ + +import type { + BackgroundExecutor, + BackgroundExecConfig, + BackgroundSpawnResult, +} from "./backgroundExecutor"; +import type { Runtime } from "@/node/runtime/Runtime"; + +/** + * SSH background executor (not yet implemented) + * + * This executor will spawn background processes on remote SSH hosts + * using nohup/setsid for detachment and file-based output capture. + */ +export class SSHBackgroundExecutor implements BackgroundExecutor { + constructor(private readonly _runtime: Runtime) { + // Runtime will be used for SSH commands when implemented + } + + async spawn(_script: string, _config: BackgroundExecConfig): Promise { + // TODO: Implement SSH background execution + // See file header for implementation approach + return { + success: false, + error: "SSH background execution is not yet implemented", + }; + } +} diff --git a/src/node/services/tools/bash.test.ts b/src/node/services/tools/bash.test.ts index 15d87fbb3..117ac436d 100644 --- a/src/node/services/tools/bash.test.ts +++ b/src/node/services/tools/bash.test.ts @@ -9,6 +9,7 @@ import { createRuntime } from "@/node/runtime/runtimeFactory"; import type { ToolCallOptions } from "ai"; import { BackgroundProcessManager } from "@/node/services/backgroundProcessManager"; import { BashExecutionService } from "@/node/services/bashExecutionService"; +import { LocalBackgroundExecutor } from "@/node/services/localBackgroundExecutor"; // Mock ToolCallOptions for testing const mockToolCallOptions: ToolCallOptions = { @@ -1366,7 +1367,8 @@ describe("bash tool - background execution", () => { }); it("should reject timeout with background mode", async () => { - const manager = new BackgroundProcessManager(new BashExecutionService()); + const manager = new BackgroundProcessManager(); + manager.registerExecutor("test-workspace", new LocalBackgroundExecutor(new BashExecutionService())); const tempDir = new TestTempDir("test-bash-bg"); const config = createTestToolConfig(process.cwd()); @@ -1392,7 +1394,8 @@ describe("bash tool - background execution", () => { }); it("should start background process and return process ID", async () => { - const manager = new BackgroundProcessManager(new BashExecutionService()); + const manager = new BackgroundProcessManager(); + manager.registerExecutor("test-workspace", new LocalBackgroundExecutor(new BashExecutionService())); const tempDir = new TestTempDir("test-bash-bg"); const config = createTestToolConfig(process.cwd()); diff --git a/src/node/services/tools/bash.ts b/src/node/services/tools/bash.ts index ffed5b009..f7228bca5 100644 --- a/src/node/services/tools/bash.ts +++ b/src/node/services/tools/bash.ts @@ -249,16 +249,6 @@ export const createBashTool: ToolFactory = (config: ToolConfiguration) => { }; } - // TODO: Add SSH runtime support for background processes (nohup/setsid + file-based output) - if (!config.runtime.supportsBackgroundExecution) { - return { - success: false, - error: "Background execution is not yet supported for SSH/remote workspaces", - exitCode: -1, - wall_duration_ms: 0, - }; - } - if (!config.workspaceId || !config.backgroundProcessManager) { return { success: false, diff --git a/src/node/services/tools/bash_background_list.test.ts b/src/node/services/tools/bash_background_list.test.ts index cc4ec8190..2faa81ac6 100644 --- a/src/node/services/tools/bash_background_list.test.ts +++ b/src/node/services/tools/bash_background_list.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect } from "bun:test"; import { createBashBackgroundListTool } from "./bash_background_list"; import { BackgroundProcessManager } from "@/node/services/backgroundProcessManager"; import { BashExecutionService } from "@/node/services/bashExecutionService"; +import { LocalBackgroundExecutor } from "@/node/services/localBackgroundExecutor"; import type { BashBackgroundListResult } from "@/common/types/tools"; import { TestTempDir, createTestToolConfig } from "./testHelpers"; import type { ToolCallOptions } from "ai"; @@ -11,6 +12,13 @@ const mockToolCallOptions: ToolCallOptions = { messages: [], }; +// Helper to create manager with executor registered for a workspace +function createManagerWithExecutor(workspaceId: string): BackgroundProcessManager { + const manager = new BackgroundProcessManager(); + manager.registerExecutor(workspaceId, new LocalBackgroundExecutor(new BashExecutionService())); + return manager; +} + describe("bash_background_list tool", () => { it("should return error when manager not available", async () => { const tempDir = new TestTempDir("test-bash-bg-list"); @@ -29,7 +37,7 @@ describe("bash_background_list tool", () => { }); it("should return error when workspaceId not available", async () => { - const manager = new BackgroundProcessManager(new BashExecutionService()); + const manager = createManagerWithExecutor("test-workspace"); const tempDir = new TestTempDir("test-bash-bg-list"); const config = createTestToolConfig(process.cwd()); config.runtimeTempDir = tempDir.path; @@ -48,7 +56,7 @@ describe("bash_background_list tool", () => { }); it("should return empty list when no processes", async () => { - const manager = new BackgroundProcessManager(new BashExecutionService()); + const manager = createManagerWithExecutor("test-workspace"); const tempDir = new TestTempDir("test-bash-bg-list"); const config = createTestToolConfig(process.cwd()); config.runtimeTempDir = tempDir.path; @@ -66,7 +74,7 @@ describe("bash_background_list tool", () => { }); it("should list spawned processes with correct fields", async () => { - const manager = new BackgroundProcessManager(new BashExecutionService()); + const manager = createManagerWithExecutor("test-workspace"); const tempDir = new TestTempDir("test-bash-bg-list"); const config = createTestToolConfig(process.cwd()); config.runtimeTempDir = tempDir.path; @@ -91,7 +99,6 @@ describe("bash_background_list tool", () => { expect(proc.process_id).toBe(spawnResult.processId); expect(proc.status).toBe("running"); expect(proc.script).toBe("sleep 10"); - expect(proc.pid).toBeDefined(); expect(proc.uptime_ms).toBeGreaterThanOrEqual(0); expect(proc.exitCode).toBeUndefined(); } @@ -102,7 +109,10 @@ describe("bash_background_list tool", () => { }); it("should only list processes for the current workspace", async () => { - const manager = new BackgroundProcessManager(new BashExecutionService()); + const manager = new BackgroundProcessManager(); + manager.registerExecutor("workspace-a", new LocalBackgroundExecutor(new BashExecutionService())); + manager.registerExecutor("workspace-b", new LocalBackgroundExecutor(new BashExecutionService())); + const tempDir = new TestTempDir("test-bash-bg-list"); const config = createTestToolConfig(process.cwd(), { workspaceId: "workspace-a" }); config.runtimeTempDir = tempDir.path; diff --git a/src/node/services/tools/bash_background_list.ts b/src/node/services/tools/bash_background_list.ts index 7a984d2a7..491ce1180 100644 --- a/src/node/services/tools/bash_background_list.ts +++ b/src/node/services/tools/bash_background_list.ts @@ -34,7 +34,6 @@ export const createBashBackgroundListTool: ToolFactory = (config: ToolConfigurat process_id: p.id, status: p.status, script: p.script, - pid: p.pid, uptime_ms: p.exitTime !== undefined ? p.exitTime - p.startTime : now - p.startTime, exitCode: p.exitCode, })), diff --git a/src/node/services/tools/bash_background_read.test.ts b/src/node/services/tools/bash_background_read.test.ts index 7232f9a7b..e09700cfa 100644 --- a/src/node/services/tools/bash_background_read.test.ts +++ b/src/node/services/tools/bash_background_read.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect } from "bun:test"; import { createBashBackgroundReadTool } from "./bash_background_read"; import { BackgroundProcessManager } from "@/node/services/backgroundProcessManager"; import { BashExecutionService } from "@/node/services/bashExecutionService"; +import { LocalBackgroundExecutor } from "@/node/services/localBackgroundExecutor"; import type { BashBackgroundReadArgs, BashBackgroundReadResult } from "@/common/types/tools"; import { TestTempDir, createTestToolConfig } from "./testHelpers"; import type { ToolCallOptions } from "ai"; @@ -11,6 +12,13 @@ const mockToolCallOptions: ToolCallOptions = { messages: [], }; +// Helper to create manager with executor registered for a workspace +function createManagerWithExecutor(workspaceId: string): BackgroundProcessManager { + const manager = new BackgroundProcessManager(); + manager.registerExecutor(workspaceId, new LocalBackgroundExecutor(new BashExecutionService())); + return manager; +} + describe("bash_background_read tool", () => { it("should return error when manager not available", async () => { const tempDir = new TestTempDir("test-bash-bg-read"); @@ -33,7 +41,7 @@ describe("bash_background_read tool", () => { }); it("should return error for non-existent process", async () => { - const manager = new BackgroundProcessManager(new BashExecutionService()); + const manager = createManagerWithExecutor("test-workspace"); const tempDir = new TestTempDir("test-bash-bg-read"); const config = createTestToolConfig(process.cwd()); config.runtimeTempDir = tempDir.path; @@ -55,7 +63,7 @@ describe("bash_background_read tool", () => { }); it("should return process status and output", async () => { - const manager = new BackgroundProcessManager(new BashExecutionService()); + const manager = createManagerWithExecutor("test-workspace"); const tempDir = new TestTempDir("test-bash-bg-read"); const config = createTestToolConfig(process.cwd()); config.runtimeTempDir = tempDir.path; @@ -92,7 +100,7 @@ describe("bash_background_read tool", () => { }); it("should handle tail filtering", async () => { - const manager = new BackgroundProcessManager(new BashExecutionService()); + const manager = createManagerWithExecutor("test-workspace"); const tempDir = new TestTempDir("test-bash-bg-read"); const config = createTestToolConfig(process.cwd()); config.runtimeTempDir = tempDir.path; @@ -130,7 +138,7 @@ describe("bash_background_read tool", () => { }); it("should handle regex filtering", async () => { - const manager = new BackgroundProcessManager(new BashExecutionService()); + const manager = createManagerWithExecutor("test-workspace"); const tempDir = new TestTempDir("test-bash-bg-read"); const config = createTestToolConfig(process.cwd()); config.runtimeTempDir = tempDir.path; @@ -163,7 +171,7 @@ describe("bash_background_read tool", () => { }); it("should return error for invalid regex pattern", async () => { - const manager = new BackgroundProcessManager(new BashExecutionService()); + const manager = createManagerWithExecutor("test-workspace"); const tempDir = new TestTempDir("test-bash-bg-read"); const config = createTestToolConfig(process.cwd()); config.runtimeTempDir = tempDir.path; @@ -197,7 +205,10 @@ describe("bash_background_read tool", () => { }); it("should not read processes from other workspaces", async () => { - const manager = new BackgroundProcessManager(new BashExecutionService()); + const manager = new BackgroundProcessManager(); + manager.registerExecutor("workspace-a", new LocalBackgroundExecutor(new BashExecutionService())); + manager.registerExecutor("workspace-b", new LocalBackgroundExecutor(new BashExecutionService())); + const tempDir = new TestTempDir("test-bash-bg-read"); // Config is for workspace-a const config = createTestToolConfig(process.cwd(), { workspaceId: "workspace-a" }); diff --git a/src/node/services/tools/bash_background_read.ts b/src/node/services/tools/bash_background_read.ts index 25d80c816..872ed69ad 100644 --- a/src/node/services/tools/bash_background_read.ts +++ b/src/node/services/tools/bash_background_read.ts @@ -87,7 +87,6 @@ export const createBashBackgroundReadTool: ToolFactory = (config: ToolConfigurat process_id: process.id, status: process.status, script: process.script, - pid: process.pid, uptime_ms, exitCode: process.exitCode, stdout, diff --git a/src/node/services/tools/bash_background_terminate.test.ts b/src/node/services/tools/bash_background_terminate.test.ts index c187d0b3b..5dee0f5eb 100644 --- a/src/node/services/tools/bash_background_terminate.test.ts +++ b/src/node/services/tools/bash_background_terminate.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect } from "bun:test"; import { createBashBackgroundTerminateTool } from "./bash_background_terminate"; import { BackgroundProcessManager } from "@/node/services/backgroundProcessManager"; import { BashExecutionService } from "@/node/services/bashExecutionService"; +import { LocalBackgroundExecutor } from "@/node/services/localBackgroundExecutor"; import type { BashBackgroundTerminateArgs, BashBackgroundTerminateResult, @@ -14,6 +15,13 @@ const mockToolCallOptions: ToolCallOptions = { messages: [], }; +// Helper to create manager with executor registered for a workspace +function createManagerWithExecutor(workspaceId: string): BackgroundProcessManager { + const manager = new BackgroundProcessManager(); + manager.registerExecutor(workspaceId, new LocalBackgroundExecutor(new BashExecutionService())); + return manager; +} + describe("bash_background_terminate tool", () => { it("should return error when manager not available", async () => { const tempDir = new TestTempDir("test-bash-bg-term"); @@ -39,7 +47,7 @@ describe("bash_background_terminate tool", () => { }); it("should return error for non-existent process", async () => { - const manager = new BackgroundProcessManager(new BashExecutionService()); + const manager = createManagerWithExecutor("test-workspace"); const tempDir = new TestTempDir("test-bash-bg-term"); const config = createTestToolConfig(process.cwd()); config.runtimeTempDir = tempDir.path; @@ -59,7 +67,7 @@ describe("bash_background_terminate tool", () => { }); it("should terminate a running process", async () => { - const manager = new BackgroundProcessManager(new BashExecutionService()); + const manager = createManagerWithExecutor("test-workspace"); const tempDir = new TestTempDir("test-bash-bg-term"); const config = createTestToolConfig(process.cwd()); config.runtimeTempDir = tempDir.path; @@ -97,7 +105,7 @@ describe("bash_background_terminate tool", () => { }); it("should be idempotent (double-terminate succeeds)", async () => { - const manager = new BackgroundProcessManager(new BashExecutionService()); + const manager = createManagerWithExecutor("test-workspace"); const tempDir = new TestTempDir("test-bash-bg-term"); const config = createTestToolConfig(process.cwd()); config.runtimeTempDir = tempDir.path; @@ -135,7 +143,10 @@ describe("bash_background_terminate tool", () => { }); it("should not terminate processes from other workspaces", async () => { - const manager = new BackgroundProcessManager(new BashExecutionService()); + const manager = new BackgroundProcessManager(); + manager.registerExecutor("workspace-a", new LocalBackgroundExecutor(new BashExecutionService())); + manager.registerExecutor("workspace-b", new LocalBackgroundExecutor(new BashExecutionService())); + const tempDir = new TestTempDir("test-bash-bg-term"); // Config is for workspace-a const config = createTestToolConfig(process.cwd(), { workspaceId: "workspace-a" }); From 2dd5e754dd2ad16d718a9a60ba2dc130b4586ad6 Mon Sep 17 00:00:00 2001 From: ethan Date: Tue, 25 Nov 2025 19:07:42 +1100 Subject: [PATCH 06/13] =?UTF-8?q?=F0=9F=A4=96=20fix:=20register=20backgrou?= =?UTF-8?q?nd=20executors=20for=20existing=20workspaces=20after=20restart?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add ensureExecutorRegistered() called from getOrCreateSession() - Add Config.getWorkspaceMetadataSync() for sync metadata lookup - Make registerExecutor() idempotent (no-op if already registered) Without this fix, background execution tools failed for all pre-existing workspaces after app restart with 'No executor registered' error. --- src/node/config.ts | 27 +++++++++++++++++++ src/node/services/backgroundProcessManager.ts | 13 ++++++--- src/node/services/ipcMain.ts | 22 +++++++++++++++ src/node/services/localBackgroundExecutor.ts | 16 +++++------ src/node/services/sshBackgroundExecutor.ts | 6 ++--- src/node/services/tools/bash.test.ts | 10 +++++-- .../tools/bash_background_list.test.ts | 10 +++++-- .../tools/bash_background_read.test.ts | 10 +++++-- .../tools/bash_background_terminate.test.ts | 10 +++++-- 9 files changed, 101 insertions(+), 23 deletions(-) diff --git a/src/node/config.ts b/src/node/config.ts index be51f3bdf..ae42a92d9 100644 --- a/src/node/config.ts +++ b/src/node/config.ts @@ -360,6 +360,33 @@ export class Config { return workspaceMetadata; } + /** + * Get workspace metadata by ID synchronously. + * Used for executor registration when creating sessions for existing workspaces. + * Only returns workspaces that have complete metadata in config (not legacy). + */ + getWorkspaceMetadataSync(workspaceId: string): WorkspaceMetadata | null { + const config = this.loadConfigOrDefault(); + + for (const [projectPath, projectConfig] of config.projects) { + for (const workspace of projectConfig.workspaces) { + // Only check new format workspaces (have id and name in config) + if (workspace.id === workspaceId && workspace.name) { + return { + id: workspace.id, + name: workspace.name, + projectName: this.getProjectName(projectPath), + projectPath, + createdAt: workspace.createdAt, + runtimeConfig: workspace.runtimeConfig ?? DEFAULT_RUNTIME_CONFIG, + }; + } + } + } + + return null; + } + /** * Add a workspace to config.json (single source of truth for workspace metadata). * Creates project entry if it doesn't exist. diff --git a/src/node/services/backgroundProcessManager.ts b/src/node/services/backgroundProcessManager.ts index b88e1aaae..453b604cb 100644 --- a/src/node/services/backgroundProcessManager.ts +++ b/src/node/services/backgroundProcessManager.ts @@ -33,10 +33,14 @@ export class BackgroundProcessManager { /** * Register an executor for a workspace. - * Called when workspace is created - local workspaces get LocalBackgroundExecutor, - * SSH workspaces get SSHBackgroundExecutor. + * Called when workspace is created or session is started for existing workspace. + * Local workspaces get LocalBackgroundExecutor, SSH workspaces get SSHBackgroundExecutor. + * Idempotent - no-op if executor already registered. */ registerExecutor(workspaceId: string, executor: BackgroundExecutor): void { + if (this.executors.has(workspaceId)) { + return; // Already registered + } log.debug(`BackgroundProcessManager.registerExecutor(${workspaceId})`); this.executors.set(workspaceId, executor); } @@ -65,7 +69,8 @@ export class BackgroundProcessManager { if (!executor) { return { success: false, - error: "No executor registered for this workspace. Background execution may not be supported.", + error: + "No executor registered for this workspace. Background execution may not be supported.", }; } @@ -194,7 +199,7 @@ export class BackgroundProcessManager { proc.exitTime ??= Date.now(); // Ensure handle is cleaned up even on error if (proc.handle) { - await proc.handle.dispose().catch(() => {}); + await proc.handle.dispose(); } return { success: true }; } diff --git a/src/node/services/ipcMain.ts b/src/node/services/ipcMain.ts index 54ba1ca8a..83cf3cb43 100644 --- a/src/node/services/ipcMain.ts +++ b/src/node/services/ipcMain.ts @@ -121,6 +121,25 @@ export class IpcMain { return new LocalBackgroundExecutor(this.bashExecutionService); } + /** + * Ensure a background executor is registered for a workspace. + * Called when creating sessions for existing workspaces (after app restart). + * No-op if executor already registered or workspace not found (new workspace being created). + */ + private ensureExecutorRegistered(workspaceId: string): void { + // Look up workspace metadata synchronously from config + const metadata = this.config.getWorkspaceMetadataSync(workspaceId); + if (!metadata) { + // Workspace not in config yet - executor will be registered by creation path + return; + } + + const runtime = createRuntime(metadata.runtimeConfig); + const executor = this.createBackgroundExecutor(metadata.runtimeConfig, runtime); + // registerExecutor is idempotent - no-op if already registered + this.backgroundProcessManager.registerExecutor(workspaceId, executor); + } + /** * Initialize the service. Call this after construction. * This is separate from the constructor to support async initialization. @@ -420,6 +439,9 @@ export class IpcMain { return session; } + // Ensure executor is registered for existing workspaces (handles app restart case) + this.ensureExecutorRegistered(trimmed); + session = new AgentSession({ workspaceId: trimmed, config: this.config, diff --git a/src/node/services/localBackgroundExecutor.ts b/src/node/services/localBackgroundExecutor.ts index 399fc85f8..99d2fa272 100644 --- a/src/node/services/localBackgroundExecutor.ts +++ b/src/node/services/localBackgroundExecutor.ts @@ -88,8 +88,8 @@ class LocalBackgroundHandle implements BackgroundHandle { } } - async isRunning(): Promise { - return this.disposable.child.exitCode === null; + isRunning(): Promise { + return Promise.resolve(this.disposable.child.exitCode === null); } async terminate(): Promise { @@ -126,8 +126,8 @@ class LocalBackgroundHandle implements BackgroundHandle { this.terminated = true; } - async dispose(): Promise { - this.disposable[Symbol.dispose](); + dispose(): Promise { + return Promise.resolve(this.disposable[Symbol.dispose]()); } /** Get the child process (for spawn event waiting) */ @@ -145,7 +145,9 @@ export class LocalBackgroundExecutor implements BackgroundExecutor { async spawn(script: string, config: BackgroundExecConfig): Promise { log.debug(`LocalBackgroundExecutor: Spawning background process in ${config.cwd}`); - // Create handle first so we can wire up callbacks + // Declare handle before executeStreaming so callbacks can reference it via closure. + // It's assigned immediately after executeStreaming returns, before any callbacks fire. + // eslint-disable-next-line prefer-const let handle: LocalBackgroundHandle; // Spawn with streaming callbacks that forward to handle @@ -190,9 +192,7 @@ export class LocalBackgroundExecutor implements BackgroundExecutor { return { success: false, error: err.message }; } - log.debug( - `LocalBackgroundExecutor: Process spawned with PID ${handle.child.pid ?? "unknown"}` - ); + log.debug(`LocalBackgroundExecutor: Process spawned with PID ${handle.child.pid ?? "unknown"}`); return { success: true, handle }; } } diff --git a/src/node/services/sshBackgroundExecutor.ts b/src/node/services/sshBackgroundExecutor.ts index b29a2ca02..ebc303e94 100644 --- a/src/node/services/sshBackgroundExecutor.ts +++ b/src/node/services/sshBackgroundExecutor.ts @@ -32,12 +32,12 @@ export class SSHBackgroundExecutor implements BackgroundExecutor { // Runtime will be used for SSH commands when implemented } - async spawn(_script: string, _config: BackgroundExecConfig): Promise { + spawn(_script: string, _config: BackgroundExecConfig): Promise { // TODO: Implement SSH background execution // See file header for implementation approach - return { + return Promise.resolve({ success: false, error: "SSH background execution is not yet implemented", - }; + }); } } diff --git a/src/node/services/tools/bash.test.ts b/src/node/services/tools/bash.test.ts index 117ac436d..c7edb1038 100644 --- a/src/node/services/tools/bash.test.ts +++ b/src/node/services/tools/bash.test.ts @@ -1368,7 +1368,10 @@ describe("bash tool - background execution", () => { it("should reject timeout with background mode", async () => { const manager = new BackgroundProcessManager(); - manager.registerExecutor("test-workspace", new LocalBackgroundExecutor(new BashExecutionService())); + manager.registerExecutor( + "test-workspace", + new LocalBackgroundExecutor(new BashExecutionService()) + ); const tempDir = new TestTempDir("test-bash-bg"); const config = createTestToolConfig(process.cwd()); @@ -1395,7 +1398,10 @@ describe("bash tool - background execution", () => { it("should start background process and return process ID", async () => { const manager = new BackgroundProcessManager(); - manager.registerExecutor("test-workspace", new LocalBackgroundExecutor(new BashExecutionService())); + manager.registerExecutor( + "test-workspace", + new LocalBackgroundExecutor(new BashExecutionService()) + ); const tempDir = new TestTempDir("test-bash-bg"); const config = createTestToolConfig(process.cwd()); diff --git a/src/node/services/tools/bash_background_list.test.ts b/src/node/services/tools/bash_background_list.test.ts index 2faa81ac6..dddc036a3 100644 --- a/src/node/services/tools/bash_background_list.test.ts +++ b/src/node/services/tools/bash_background_list.test.ts @@ -110,8 +110,14 @@ describe("bash_background_list tool", () => { it("should only list processes for the current workspace", async () => { const manager = new BackgroundProcessManager(); - manager.registerExecutor("workspace-a", new LocalBackgroundExecutor(new BashExecutionService())); - manager.registerExecutor("workspace-b", new LocalBackgroundExecutor(new BashExecutionService())); + manager.registerExecutor( + "workspace-a", + new LocalBackgroundExecutor(new BashExecutionService()) + ); + manager.registerExecutor( + "workspace-b", + new LocalBackgroundExecutor(new BashExecutionService()) + ); const tempDir = new TestTempDir("test-bash-bg-list"); const config = createTestToolConfig(process.cwd(), { workspaceId: "workspace-a" }); diff --git a/src/node/services/tools/bash_background_read.test.ts b/src/node/services/tools/bash_background_read.test.ts index e09700cfa..c0240d203 100644 --- a/src/node/services/tools/bash_background_read.test.ts +++ b/src/node/services/tools/bash_background_read.test.ts @@ -206,8 +206,14 @@ describe("bash_background_read tool", () => { it("should not read processes from other workspaces", async () => { const manager = new BackgroundProcessManager(); - manager.registerExecutor("workspace-a", new LocalBackgroundExecutor(new BashExecutionService())); - manager.registerExecutor("workspace-b", new LocalBackgroundExecutor(new BashExecutionService())); + manager.registerExecutor( + "workspace-a", + new LocalBackgroundExecutor(new BashExecutionService()) + ); + manager.registerExecutor( + "workspace-b", + new LocalBackgroundExecutor(new BashExecutionService()) + ); const tempDir = new TestTempDir("test-bash-bg-read"); // Config is for workspace-a diff --git a/src/node/services/tools/bash_background_terminate.test.ts b/src/node/services/tools/bash_background_terminate.test.ts index 5dee0f5eb..a50471ce8 100644 --- a/src/node/services/tools/bash_background_terminate.test.ts +++ b/src/node/services/tools/bash_background_terminate.test.ts @@ -144,8 +144,14 @@ describe("bash_background_terminate tool", () => { it("should not terminate processes from other workspaces", async () => { const manager = new BackgroundProcessManager(); - manager.registerExecutor("workspace-a", new LocalBackgroundExecutor(new BashExecutionService())); - manager.registerExecutor("workspace-b", new LocalBackgroundExecutor(new BashExecutionService())); + manager.registerExecutor( + "workspace-a", + new LocalBackgroundExecutor(new BashExecutionService()) + ); + manager.registerExecutor( + "workspace-b", + new LocalBackgroundExecutor(new BashExecutionService()) + ); const tempDir = new TestTempDir("test-bash-bg-term"); // Config is for workspace-a From fbb4a9d4f50d95dda8726ac697e77a21c0506299 Mon Sep 17 00:00:00 2001 From: ethan Date: Tue, 25 Nov 2025 19:44:04 +1100 Subject: [PATCH 07/13] =?UTF-8?q?=F0=9F=A4=96=20refactor:=20lazy=20executo?= =?UTF-8?q?r=20pattern=20for=20background=20processes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Moves from pre-registration to lazy executor creation/caching: - Remove registerExecutor()/unregisterExecutor() from BackgroundProcessManager - spawn() now takes executor as first arg, caches per workspace - AIService creates executor alongside runtime in streamMessage() - Remove ensureExecutorRegistered() and getWorkspaceMetadataSync() - Remove 3 registration calls from IpcMain workspace creation paths This follows the existing pattern where runtime is created on-demand from metadata rather than pre-registered. Executors are still cached for SSH connection pooling. Net ~100 LoC removed. --- src/common/utils/tools/tools.ts | 3 + src/node/config.ts | 27 ------- src/node/services/aiService.ts | 34 +++++++- .../services/backgroundProcessManager.test.ts | 79 +++++++------------ src/node/services/backgroundProcessManager.ts | 48 +++-------- src/node/services/ipcMain.ts | 47 ----------- src/node/services/tools/bash.test.ts | 18 +++-- src/node/services/tools/bash.ts | 3 +- .../tools/bash_background_list.test.ts | 32 +++----- .../tools/bash_background_read.test.ts | 41 +++++----- .../tools/bash_background_terminate.test.ts | 32 +++----- 11 files changed, 129 insertions(+), 235 deletions(-) diff --git a/src/common/utils/tools/tools.ts b/src/common/utils/tools/tools.ts index 3d6b11949..d10de8416 100644 --- a/src/common/utils/tools/tools.ts +++ b/src/common/utils/tools/tools.ts @@ -16,6 +16,7 @@ import { log } from "@/node/services/log"; import type { Runtime } from "@/node/runtime/Runtime"; import type { InitStateManager } from "@/node/services/initStateManager"; import type { BackgroundProcessManager } from "@/node/services/backgroundProcessManager"; +import type { BackgroundExecutor } from "@/node/services/backgroundExecutor"; /** * Configuration for tools that need runtime context @@ -35,6 +36,8 @@ export interface ToolConfiguration { overflow_policy?: "truncate" | "tmpfile"; /** Background process manager for bash tool (optional, AI-only) */ backgroundProcessManager?: BackgroundProcessManager; + /** Background executor for this workspace (optional, AI-only) */ + backgroundExecutor?: BackgroundExecutor; /** Workspace ID for tracking background processes (optional for token estimation) */ workspaceId?: string; } diff --git a/src/node/config.ts b/src/node/config.ts index ae42a92d9..be51f3bdf 100644 --- a/src/node/config.ts +++ b/src/node/config.ts @@ -360,33 +360,6 @@ export class Config { return workspaceMetadata; } - /** - * Get workspace metadata by ID synchronously. - * Used for executor registration when creating sessions for existing workspaces. - * Only returns workspaces that have complete metadata in config (not legacy). - */ - getWorkspaceMetadataSync(workspaceId: string): WorkspaceMetadata | null { - const config = this.loadConfigOrDefault(); - - for (const [projectPath, projectConfig] of config.projects) { - for (const workspace of projectConfig.workspaces) { - // Only check new format workspaces (have id and name in config) - if (workspace.id === workspaceId && workspace.name) { - return { - id: workspace.id, - name: workspace.name, - projectName: this.getProjectName(projectPath), - projectPath, - createdAt: workspace.createdAt, - runtimeConfig: workspace.runtimeConfig ?? DEFAULT_RUNTIME_CONFIG, - }; - } - } - } - - return null; - } - /** * Add a workspace to config.json (single source of truth for workspace metadata). * Creates project entry if it doesn't exist. diff --git a/src/node/services/aiService.ts b/src/node/services/aiService.ts index 53b9cae38..8113107d9 100644 --- a/src/node/services/aiService.ts +++ b/src/node/services/aiService.ts @@ -21,6 +21,12 @@ import { createRuntime } from "@/node/runtime/runtimeFactory"; import { secretsToRecord } from "@/common/types/secrets"; import type { MuxProviderOptions } from "@/common/types/providerOptions"; import type { BackgroundProcessManager } from "@/node/services/backgroundProcessManager"; +import type { BackgroundExecutor } from "@/node/services/backgroundExecutor"; +import { LocalBackgroundExecutor } from "@/node/services/localBackgroundExecutor"; +import { SSHBackgroundExecutor } from "@/node/services/sshBackgroundExecutor"; +import { BashExecutionService } from "@/node/services/bashExecutionService"; +import { isSSHRuntime, type RuntimeConfig } from "@/common/types/runtime"; +import type { Runtime } from "@/node/runtime/Runtime"; import { log } from "./log"; import { transformModelMessages, @@ -143,6 +149,7 @@ export class AIService extends EventEmitter { private readonly mockModeEnabled: boolean; private readonly mockScenarioPlayer?: MockScenarioPlayer; private readonly backgroundProcessManager?: BackgroundProcessManager; + private readonly bashExecutionService: BashExecutionService; constructor( config: Config, @@ -160,6 +167,7 @@ export class AIService extends EventEmitter { this.partialService = partialService; this.initStateManager = initStateManager; this.backgroundProcessManager = backgroundProcessManager; + this.bashExecutionService = new BashExecutionService(); this.streamManager = new StreamManager(historyService, partialService); void this.ensureSessionsDir(); this.setupStreamEventForwarding(); @@ -173,6 +181,20 @@ export class AIService extends EventEmitter { } } + /** + * Create a background executor appropriate for the given runtime. + * Local runtimes use LocalBackgroundExecutor, SSH runtimes use SSHBackgroundExecutor. + */ + private createBackgroundExecutor( + runtimeConfig: RuntimeConfig, + runtime: Runtime + ): BackgroundExecutor { + if (isSSHRuntime(runtimeConfig)) { + return new SSHBackgroundExecutor(runtime); + } + return new LocalBackgroundExecutor(this.bashExecutionService); + } + /** * Forward all stream events from StreamManager to AIService consumers */ @@ -719,9 +741,11 @@ export class AIService extends EventEmitter { } // Get workspace path - handle both worktree and in-place modes - const runtime = createRuntime( - metadata.runtimeConfig ?? { type: "local", srcBaseDir: this.config.srcDir } - ); + const runtimeConfig = metadata.runtimeConfig ?? { + type: "local" as const, + srcBaseDir: this.config.srcDir, + }; + const runtime = createRuntime(runtimeConfig); // In-place workspaces (CLI/benchmarks) have projectPath === name // Use path directly instead of reconstructing via getWorkspacePath const isInPlace = metadata.projectPath === metadata.name; @@ -729,6 +753,9 @@ export class AIService extends EventEmitter { ? metadata.projectPath : runtime.getWorkspacePath(metadata.projectPath, metadata.name); + // Create background executor for this workspace (cached on first spawn) + const backgroundExecutor = this.createBackgroundExecutor(runtimeConfig, runtime); + // Build system message from workspace metadata const systemMessage = await buildSystemMessage( metadata, @@ -767,6 +794,7 @@ export class AIService extends EventEmitter { secrets: secretsToRecord(projectSecrets), runtimeTempDir, backgroundProcessManager: this.backgroundProcessManager, + backgroundExecutor, workspaceId, }, workspaceId, diff --git a/src/node/services/backgroundProcessManager.test.ts b/src/node/services/backgroundProcessManager.test.ts index 43e4148cc..1d75b51c2 100644 --- a/src/node/services/backgroundProcessManager.test.ts +++ b/src/node/services/backgroundProcessManager.test.ts @@ -2,26 +2,27 @@ import { describe, it, expect, beforeEach } from "bun:test"; import { BackgroundProcessManager } from "./backgroundProcessManager"; import { BashExecutionService } from "./bashExecutionService"; import { LocalBackgroundExecutor } from "./localBackgroundExecutor"; +import type { BackgroundExecutor } from "./backgroundExecutor"; -// Helper to create manager with executor registered for a workspace -function createManagerWithExecutor(workspaceId: string): BackgroundProcessManager { - const manager = new BackgroundProcessManager(); - manager.registerExecutor(workspaceId, new LocalBackgroundExecutor(new BashExecutionService())); - return manager; +// Create a shared executor for tests +function createTestExecutor(): BackgroundExecutor { + return new LocalBackgroundExecutor(new BashExecutionService()); } describe("BackgroundProcessManager", () => { let manager: BackgroundProcessManager; + let executor: BackgroundExecutor; const testWorkspaceId = "workspace-1"; const testWorkspaceId2 = "workspace-2"; beforeEach(() => { - manager = createManagerWithExecutor(testWorkspaceId); + manager = new BackgroundProcessManager(); + executor = createTestExecutor(); }); describe("spawn", () => { it("should spawn a background process and return process ID", async () => { - const result = await manager.spawn(testWorkspaceId, "echo hello", { + const result = await manager.spawn(executor, testWorkspaceId, "echo hello", { cwd: process.cwd(), }); @@ -31,20 +32,8 @@ describe("BackgroundProcessManager", () => { } }); - it("should return error when no executor is registered", async () => { - const managerNoExecutor = new BackgroundProcessManager(); - const result = await managerNoExecutor.spawn("unregistered-workspace", "echo hello", { - cwd: process.cwd(), - }); - - expect(result.success).toBe(false); - if (!result.success) { - expect(result.error).toContain("No executor registered"); - } - }); - it("should return error on spawn failure", async () => { - const result = await manager.spawn(testWorkspaceId, "echo test", { + const result = await manager.spawn(executor, testWorkspaceId, "echo test", { cwd: "/nonexistent/path/that/does/not/exist", }); @@ -52,7 +41,7 @@ describe("BackgroundProcessManager", () => { }); it("should capture stdout and stderr", async () => { - const result = await manager.spawn(testWorkspaceId, "echo hello; echo world >&2", { + const result = await manager.spawn(executor, testWorkspaceId, "echo hello; echo world >&2", { cwd: process.cwd(), }); @@ -75,7 +64,7 @@ describe("BackgroundProcessManager", () => { .map((_, i) => `echo line${i}`) .join("; "); - const result = await manager.spawn(testWorkspaceId, script, { + const result = await manager.spawn(executor, testWorkspaceId, script, { cwd: process.cwd(), }); @@ -93,7 +82,7 @@ describe("BackgroundProcessManager", () => { describe("getProcess", () => { it("should return process by ID", async () => { - const spawnResult = await manager.spawn(testWorkspaceId, "sleep 1", { + const spawnResult = await manager.spawn(executor, testWorkspaceId, "sleep 1", { cwd: process.cwd(), }); @@ -113,22 +102,18 @@ describe("BackgroundProcessManager", () => { describe("list", () => { it("should list all processes", async () => { - await manager.spawn(testWorkspaceId, "sleep 1", { cwd: process.cwd() }); - await manager.spawn(testWorkspaceId, "sleep 1", { cwd: process.cwd() }); + await manager.spawn(executor, testWorkspaceId, "sleep 1", { cwd: process.cwd() }); + await manager.spawn(executor, testWorkspaceId, "sleep 1", { cwd: process.cwd() }); const processes = manager.list(); expect(processes.length).toBeGreaterThanOrEqual(2); }); it("should filter by workspace ID", async () => { - // Register second workspace executor - manager.registerExecutor( - testWorkspaceId2, - new LocalBackgroundExecutor(new BashExecutionService()) - ); + const executor2 = createTestExecutor(); - await manager.spawn(testWorkspaceId, "sleep 1", { cwd: process.cwd() }); - await manager.spawn(testWorkspaceId2, "sleep 1", { cwd: process.cwd() }); + await manager.spawn(executor, testWorkspaceId, "sleep 1", { cwd: process.cwd() }); + await manager.spawn(executor2, testWorkspaceId2, "sleep 1", { cwd: process.cwd() }); const ws1Processes = manager.list(testWorkspaceId); const ws2Processes = manager.list(testWorkspaceId2); @@ -142,7 +127,7 @@ describe("BackgroundProcessManager", () => { describe("terminate", () => { it("should terminate a running process", async () => { - const spawnResult = await manager.spawn(testWorkspaceId, "sleep 10", { + const spawnResult = await manager.spawn(executor, testWorkspaceId, "sleep 10", { cwd: process.cwd(), }); @@ -161,7 +146,7 @@ describe("BackgroundProcessManager", () => { }); it("should be idempotent (double-terminate succeeds)", async () => { - const spawnResult = await manager.spawn(testWorkspaceId, "sleep 10", { + const spawnResult = await manager.spawn(executor, testWorkspaceId, "sleep 10", { cwd: process.cwd(), }); @@ -176,16 +161,12 @@ describe("BackgroundProcessManager", () => { }); describe("cleanup", () => { - it("should kill all processes for a workspace and remove them from memory", async () => { - // Register second workspace executor - manager.registerExecutor( - testWorkspaceId2, - new LocalBackgroundExecutor(new BashExecutionService()) - ); + it("should kill all processes for a workspace and clear cached executor", async () => { + const executor2 = createTestExecutor(); - await manager.spawn(testWorkspaceId, "sleep 10", { cwd: process.cwd() }); - await manager.spawn(testWorkspaceId, "sleep 10", { cwd: process.cwd() }); - await manager.spawn(testWorkspaceId2, "sleep 10", { cwd: process.cwd() }); + await manager.spawn(executor, testWorkspaceId, "sleep 10", { cwd: process.cwd() }); + await manager.spawn(executor, testWorkspaceId, "sleep 10", { cwd: process.cwd() }); + await manager.spawn(executor2, testWorkspaceId2, "sleep 10", { cwd: process.cwd() }); await manager.cleanup(testWorkspaceId); @@ -193,12 +174,6 @@ describe("BackgroundProcessManager", () => { const ws2Processes = manager.list(testWorkspaceId2); // All testWorkspaceId processes should be removed from memory expect(ws1Processes.length).toBe(0); - // Executor should also be unregistered - spawning should fail - const spawnResult = await manager.spawn(testWorkspaceId, "echo test", { cwd: process.cwd() }); - expect(spawnResult.success).toBe(false); - if (!spawnResult.success) { - expect(spawnResult.error).toContain("No executor registered"); - } // workspace-2 processes should still exist and be running expect(ws2Processes.length).toBeGreaterThanOrEqual(1); expect(ws2Processes.some((p) => p.status === "running")).toBe(true); @@ -207,7 +182,7 @@ describe("BackgroundProcessManager", () => { describe("process state tracking", () => { it("should track process exit", async () => { - const result = await manager.spawn(testWorkspaceId, "exit 42", { + const result = await manager.spawn(executor, testWorkspaceId, "exit 42", { cwd: process.cwd(), }); @@ -223,7 +198,7 @@ describe("BackgroundProcessManager", () => { }); it("should keep buffer after process exits", async () => { - const result = await manager.spawn(testWorkspaceId, "echo test; exit 0", { + const result = await manager.spawn(executor, testWorkspaceId, "echo test; exit 0", { cwd: process.cwd(), }); @@ -238,7 +213,7 @@ describe("BackgroundProcessManager", () => { it("should preserve killed status after onExit callback fires", async () => { // Spawn a long-running process - const result = await manager.spawn(testWorkspaceId, "sleep 60", { + const result = await manager.spawn(executor, testWorkspaceId, "sleep 60", { cwd: process.cwd(), }); diff --git a/src/node/services/backgroundProcessManager.ts b/src/node/services/backgroundProcessManager.ts index 453b604cb..7227da699 100644 --- a/src/node/services/backgroundProcessManager.ts +++ b/src/node/services/backgroundProcessManager.ts @@ -24,54 +24,28 @@ const MAX_BUFFER_LINES = 1000; /** * Manages background bash processes for workspaces. * - * Each workspace registers its own executor (local or SSH) at creation time. - * This allows different execution backends per workspace. + * Executors are provided lazily at spawn time and cached per workspace. + * This allows different execution backends per workspace (local vs SSH). */ export class BackgroundProcessManager { private processes = new Map(); private executors = new Map(); /** - * Register an executor for a workspace. - * Called when workspace is created or session is started for existing workspace. - * Local workspaces get LocalBackgroundExecutor, SSH workspaces get SSHBackgroundExecutor. - * Idempotent - no-op if executor already registered. - */ - registerExecutor(workspaceId: string, executor: BackgroundExecutor): void { - if (this.executors.has(workspaceId)) { - return; // Already registered - } - log.debug(`BackgroundProcessManager.registerExecutor(${workspaceId})`); - this.executors.set(workspaceId, executor); - } - - /** - * Unregister executor for a workspace. - * Called when workspace is deleted. - */ - unregisterExecutor(workspaceId: string): void { - log.debug(`BackgroundProcessManager.unregisterExecutor(${workspaceId})`); - this.executors.delete(workspaceId); - } - - /** - * Spawn a new background process + * Spawn a new background process. + * The executor is cached on first spawn per workspace for reuse (e.g., SSH connection pooling). */ async spawn( + executor: BackgroundExecutor, workspaceId: string, script: string, config: { cwd: string; secrets?: Record; niceness?: number } ): Promise<{ success: true; processId: string } | { success: false; error: string }> { log.debug(`BackgroundProcessManager.spawn() called for workspace ${workspaceId}`); - // Get executor for this workspace - const executor = this.executors.get(workspaceId); - if (!executor) { - return { - success: false, - error: - "No executor registered for this workspace. Background execution may not be supported.", - }; + // Cache executor on first spawn for this workspace (enables SSH connection reuse) + if (!this.executors.has(workspaceId)) { + this.executors.set(workspaceId, executor); } // Generate unique process ID @@ -207,7 +181,7 @@ export class BackgroundProcessManager { /** * Clean up all processes for a workspace. - * Terminates running processes, removes them from memory, and unregisters the executor. + * Terminates running processes, removes them from memory, and clears the cached executor. */ async cleanup(workspaceId: string): Promise { log.debug(`BackgroundProcessManager.cleanup(${workspaceId}) called`); @@ -221,8 +195,8 @@ export class BackgroundProcessManager { // Remove all processes from memory matching.forEach((p) => this.processes.delete(p.id)); - // Unregister the executor for this workspace - this.unregisterExecutor(workspaceId); + // Clear cached executor for this workspace + this.executors.delete(workspaceId); log.debug(`Cleaned up ${matching.length} process(es) for workspace ${workspaceId}`); } diff --git a/src/node/services/ipcMain.ts b/src/node/services/ipcMain.ts index 83cf3cb43..bea012818 100644 --- a/src/node/services/ipcMain.ts +++ b/src/node/services/ipcMain.ts @@ -39,11 +39,7 @@ import { InitStateManager } from "@/node/services/initStateManager"; import { createRuntime } from "@/node/runtime/runtimeFactory"; import type { RuntimeConfig } from "@/common/types/runtime"; import { BackgroundProcessManager } from "@/node/services/backgroundProcessManager"; -import { BashExecutionService } from "@/node/services/bashExecutionService"; -import { LocalBackgroundExecutor } from "@/node/services/localBackgroundExecutor"; -import { SSHBackgroundExecutor } from "@/node/services/sshBackgroundExecutor"; import { isSSHRuntime } from "@/common/types/runtime"; -import type { Runtime } from "@/node/runtime/Runtime"; import { validateProjectPath } from "@/node/utils/pathUtils"; import { PTYService } from "@/node/services/ptyService"; import type { TerminalWindowManager } from "@/desktop/terminalWindowManager"; @@ -72,7 +68,6 @@ export class IpcMain { private readonly extensionMetadata: ExtensionMetadataService; private readonly ptyService: PTYService; private readonly backgroundProcessManager: BackgroundProcessManager; - private readonly bashExecutionService: BashExecutionService; private terminalWindowManager?: TerminalWindowManager; private readonly sessions = new Map(); private projectDirectoryPicker?: (event: IpcMainInvokeEvent) => Promise; @@ -94,7 +89,6 @@ export class IpcMain { path.join(config.rootDir, "extensionMetadata.json") ); this.backgroundProcessManager = new BackgroundProcessManager(); - this.bashExecutionService = new BashExecutionService(); this.aiService = new AIService( config, @@ -110,36 +104,6 @@ export class IpcMain { this.setupMetadataListeners(); } - /** - * Create a background executor appropriate for the given runtime. - * Local runtimes use LocalBackgroundExecutor, SSH runtimes use SSHBackgroundExecutor. - */ - private createBackgroundExecutor(runtimeConfig: RuntimeConfig, runtime: Runtime) { - if (isSSHRuntime(runtimeConfig)) { - return new SSHBackgroundExecutor(runtime); - } - return new LocalBackgroundExecutor(this.bashExecutionService); - } - - /** - * Ensure a background executor is registered for a workspace. - * Called when creating sessions for existing workspaces (after app restart). - * No-op if executor already registered or workspace not found (new workspace being created). - */ - private ensureExecutorRegistered(workspaceId: string): void { - // Look up workspace metadata synchronously from config - const metadata = this.config.getWorkspaceMetadataSync(workspaceId); - if (!metadata) { - // Workspace not in config yet - executor will be registered by creation path - return; - } - - const runtime = createRuntime(metadata.runtimeConfig); - const executor = this.createBackgroundExecutor(metadata.runtimeConfig, runtime); - // registerExecutor is idempotent - no-op if already registered - this.backgroundProcessManager.registerExecutor(workspaceId, executor); - } - /** * Initialize the service. Call this after construction. * This is separate from the constructor to support async initialization. @@ -395,10 +359,6 @@ export class IpcMain { session.emitMetadata(completeMetadata); - // Register background executor for this workspace - const executor = this.createBackgroundExecutor(finalRuntimeConfig, runtime); - this.backgroundProcessManager.registerExecutor(workspaceId, executor); - void runtime .initWorkspace({ projectPath, @@ -439,9 +399,6 @@ export class IpcMain { return session; } - // Ensure executor is registered for existing workspaces (handles app restart case) - this.ensureExecutorRegistered(trimmed); - session = new AgentSession({ workspaceId: trimmed, config: this.config, @@ -725,10 +682,6 @@ export class IpcMain { // Emit metadata event for new workspace (session already created above) session.emitMetadata(completeMetadata); - // Register background executor for this workspace - const executor = this.createBackgroundExecutor(finalRuntimeConfig, runtime); - this.backgroundProcessManager.registerExecutor(workspaceId, executor); - // Phase 2: Initialize workspace asynchronously (SLOW - runs in background) // This streams progress via initLogger and doesn't block the IPC return void runtime diff --git a/src/node/services/tools/bash.test.ts b/src/node/services/tools/bash.test.ts index c7edb1038..e4e69ced0 100644 --- a/src/node/services/tools/bash.test.ts +++ b/src/node/services/tools/bash.test.ts @@ -10,6 +10,12 @@ import type { ToolCallOptions } from "ai"; import { BackgroundProcessManager } from "@/node/services/backgroundProcessManager"; import { BashExecutionService } from "@/node/services/bashExecutionService"; import { LocalBackgroundExecutor } from "@/node/services/localBackgroundExecutor"; +import type { BackgroundExecutor } from "@/node/services/backgroundExecutor"; + +// Create a test executor +function createTestExecutor(): BackgroundExecutor { + return new LocalBackgroundExecutor(new BashExecutionService()); +} // Mock ToolCallOptions for testing const mockToolCallOptions: ToolCallOptions = { @@ -1368,15 +1374,13 @@ describe("bash tool - background execution", () => { it("should reject timeout with background mode", async () => { const manager = new BackgroundProcessManager(); - manager.registerExecutor( - "test-workspace", - new LocalBackgroundExecutor(new BashExecutionService()) - ); + const executor = createTestExecutor(); const tempDir = new TestTempDir("test-bash-bg"); const config = createTestToolConfig(process.cwd()); config.runtimeTempDir = tempDir.path; config.backgroundProcessManager = manager; + config.backgroundExecutor = executor; config.workspaceId = "test-workspace"; const tool = createBashTool(config); @@ -1398,15 +1402,13 @@ describe("bash tool - background execution", () => { it("should start background process and return process ID", async () => { const manager = new BackgroundProcessManager(); - manager.registerExecutor( - "test-workspace", - new LocalBackgroundExecutor(new BashExecutionService()) - ); + const executor = createTestExecutor(); const tempDir = new TestTempDir("test-bash-bg"); const config = createTestToolConfig(process.cwd()); config.runtimeTempDir = tempDir.path; config.backgroundProcessManager = manager; + config.backgroundExecutor = executor; config.workspaceId = "test-workspace"; const tool = createBashTool(config); diff --git a/src/node/services/tools/bash.ts b/src/node/services/tools/bash.ts index f7228bca5..e871e4b30 100644 --- a/src/node/services/tools/bash.ts +++ b/src/node/services/tools/bash.ts @@ -249,7 +249,7 @@ export const createBashTool: ToolFactory = (config: ToolConfiguration) => { }; } - if (!config.workspaceId || !config.backgroundProcessManager) { + if (!config.workspaceId || !config.backgroundProcessManager || !config.backgroundExecutor) { return { success: false, error: @@ -270,6 +270,7 @@ export const createBashTool: ToolFactory = (config: ToolConfiguration) => { const startTime = performance.now(); const spawnResult = await config.backgroundProcessManager.spawn( + config.backgroundExecutor, config.workspaceId, script, { diff --git a/src/node/services/tools/bash_background_list.test.ts b/src/node/services/tools/bash_background_list.test.ts index dddc036a3..a071291eb 100644 --- a/src/node/services/tools/bash_background_list.test.ts +++ b/src/node/services/tools/bash_background_list.test.ts @@ -3,6 +3,7 @@ import { createBashBackgroundListTool } from "./bash_background_list"; import { BackgroundProcessManager } from "@/node/services/backgroundProcessManager"; import { BashExecutionService } from "@/node/services/bashExecutionService"; import { LocalBackgroundExecutor } from "@/node/services/localBackgroundExecutor"; +import type { BackgroundExecutor } from "@/node/services/backgroundExecutor"; import type { BashBackgroundListResult } from "@/common/types/tools"; import { TestTempDir, createTestToolConfig } from "./testHelpers"; import type { ToolCallOptions } from "ai"; @@ -12,11 +13,9 @@ const mockToolCallOptions: ToolCallOptions = { messages: [], }; -// Helper to create manager with executor registered for a workspace -function createManagerWithExecutor(workspaceId: string): BackgroundProcessManager { - const manager = new BackgroundProcessManager(); - manager.registerExecutor(workspaceId, new LocalBackgroundExecutor(new BashExecutionService())); - return manager; +// Create a test executor +function createTestExecutor(): BackgroundExecutor { + return new LocalBackgroundExecutor(new BashExecutionService()); } describe("bash_background_list tool", () => { @@ -37,7 +36,7 @@ describe("bash_background_list tool", () => { }); it("should return error when workspaceId not available", async () => { - const manager = createManagerWithExecutor("test-workspace"); + const manager = new BackgroundProcessManager(); const tempDir = new TestTempDir("test-bash-bg-list"); const config = createTestToolConfig(process.cwd()); config.runtimeTempDir = tempDir.path; @@ -56,7 +55,7 @@ describe("bash_background_list tool", () => { }); it("should return empty list when no processes", async () => { - const manager = createManagerWithExecutor("test-workspace"); + const manager = new BackgroundProcessManager(); const tempDir = new TestTempDir("test-bash-bg-list"); const config = createTestToolConfig(process.cwd()); config.runtimeTempDir = tempDir.path; @@ -74,14 +73,15 @@ describe("bash_background_list tool", () => { }); it("should list spawned processes with correct fields", async () => { - const manager = createManagerWithExecutor("test-workspace"); + const manager = new BackgroundProcessManager(); + const executor = createTestExecutor(); const tempDir = new TestTempDir("test-bash-bg-list"); const config = createTestToolConfig(process.cwd()); config.runtimeTempDir = tempDir.path; config.backgroundProcessManager = manager; // Spawn a process - const spawnResult = await manager.spawn("test-workspace", "sleep 10", { + const spawnResult = await manager.spawn(executor, "test-workspace", "sleep 10", { cwd: process.cwd(), }); @@ -110,14 +110,8 @@ describe("bash_background_list tool", () => { it("should only list processes for the current workspace", async () => { const manager = new BackgroundProcessManager(); - manager.registerExecutor( - "workspace-a", - new LocalBackgroundExecutor(new BashExecutionService()) - ); - manager.registerExecutor( - "workspace-b", - new LocalBackgroundExecutor(new BashExecutionService()) - ); + const executorA = createTestExecutor(); + const executorB = createTestExecutor(); const tempDir = new TestTempDir("test-bash-bg-list"); const config = createTestToolConfig(process.cwd(), { workspaceId: "workspace-a" }); @@ -125,8 +119,8 @@ describe("bash_background_list tool", () => { config.backgroundProcessManager = manager; // Spawn processes in different workspaces - const spawnA = await manager.spawn("workspace-a", "sleep 10", { cwd: process.cwd() }); - const spawnB = await manager.spawn("workspace-b", "sleep 10", { cwd: process.cwd() }); + const spawnA = await manager.spawn(executorA, "workspace-a", "sleep 10", { cwd: process.cwd() }); + const spawnB = await manager.spawn(executorB, "workspace-b", "sleep 10", { cwd: process.cwd() }); if (!spawnA.success || !spawnB.success) { throw new Error("Failed to spawn processes"); diff --git a/src/node/services/tools/bash_background_read.test.ts b/src/node/services/tools/bash_background_read.test.ts index c0240d203..0bff54aab 100644 --- a/src/node/services/tools/bash_background_read.test.ts +++ b/src/node/services/tools/bash_background_read.test.ts @@ -3,6 +3,7 @@ import { createBashBackgroundReadTool } from "./bash_background_read"; import { BackgroundProcessManager } from "@/node/services/backgroundProcessManager"; import { BashExecutionService } from "@/node/services/bashExecutionService"; import { LocalBackgroundExecutor } from "@/node/services/localBackgroundExecutor"; +import type { BackgroundExecutor } from "@/node/services/backgroundExecutor"; import type { BashBackgroundReadArgs, BashBackgroundReadResult } from "@/common/types/tools"; import { TestTempDir, createTestToolConfig } from "./testHelpers"; import type { ToolCallOptions } from "ai"; @@ -12,11 +13,9 @@ const mockToolCallOptions: ToolCallOptions = { messages: [], }; -// Helper to create manager with executor registered for a workspace -function createManagerWithExecutor(workspaceId: string): BackgroundProcessManager { - const manager = new BackgroundProcessManager(); - manager.registerExecutor(workspaceId, new LocalBackgroundExecutor(new BashExecutionService())); - return manager; +// Create a test executor +function createTestExecutor(): BackgroundExecutor { + return new LocalBackgroundExecutor(new BashExecutionService()); } describe("bash_background_read tool", () => { @@ -41,7 +40,7 @@ describe("bash_background_read tool", () => { }); it("should return error for non-existent process", async () => { - const manager = createManagerWithExecutor("test-workspace"); + const manager = new BackgroundProcessManager(); const tempDir = new TestTempDir("test-bash-bg-read"); const config = createTestToolConfig(process.cwd()); config.runtimeTempDir = tempDir.path; @@ -63,14 +62,15 @@ describe("bash_background_read tool", () => { }); it("should return process status and output", async () => { - const manager = createManagerWithExecutor("test-workspace"); + const manager = new BackgroundProcessManager(); + const executor = createTestExecutor(); const tempDir = new TestTempDir("test-bash-bg-read"); const config = createTestToolConfig(process.cwd()); config.runtimeTempDir = tempDir.path; config.backgroundProcessManager = manager; // Spawn a process - const spawnResult = await manager.spawn("test-workspace", "echo hello; sleep 1", { + const spawnResult = await manager.spawn(executor, "test-workspace", "echo hello; sleep 1", { cwd: process.cwd(), }); @@ -100,7 +100,8 @@ describe("bash_background_read tool", () => { }); it("should handle tail filtering", async () => { - const manager = createManagerWithExecutor("test-workspace"); + const manager = new BackgroundProcessManager(); + const executor = createTestExecutor(); const tempDir = new TestTempDir("test-bash-bg-read"); const config = createTestToolConfig(process.cwd()); config.runtimeTempDir = tempDir.path; @@ -108,6 +109,7 @@ describe("bash_background_read tool", () => { // Spawn a process with multiple lines const spawnResult = await manager.spawn( + executor, "test-workspace", "echo line1; echo line2; echo line3", { @@ -138,13 +140,14 @@ describe("bash_background_read tool", () => { }); it("should handle regex filtering", async () => { - const manager = createManagerWithExecutor("test-workspace"); + const manager = new BackgroundProcessManager(); + const executor = createTestExecutor(); const tempDir = new TestTempDir("test-bash-bg-read"); const config = createTestToolConfig(process.cwd()); config.runtimeTempDir = tempDir.path; config.backgroundProcessManager = manager; - const spawnResult = await manager.spawn("test-workspace", "echo ERROR: test; echo INFO: test", { + const spawnResult = await manager.spawn(executor, "test-workspace", "echo ERROR: test; echo INFO: test", { cwd: process.cwd(), }); @@ -171,13 +174,14 @@ describe("bash_background_read tool", () => { }); it("should return error for invalid regex pattern", async () => { - const manager = createManagerWithExecutor("test-workspace"); + const manager = new BackgroundProcessManager(); + const executor = createTestExecutor(); const tempDir = new TestTempDir("test-bash-bg-read"); const config = createTestToolConfig(process.cwd()); config.runtimeTempDir = tempDir.path; config.backgroundProcessManager = manager; - const spawnResult = await manager.spawn("test-workspace", "echo test", { + const spawnResult = await manager.spawn(executor, "test-workspace", "echo test", { cwd: process.cwd(), }); @@ -206,14 +210,7 @@ describe("bash_background_read tool", () => { it("should not read processes from other workspaces", async () => { const manager = new BackgroundProcessManager(); - manager.registerExecutor( - "workspace-a", - new LocalBackgroundExecutor(new BashExecutionService()) - ); - manager.registerExecutor( - "workspace-b", - new LocalBackgroundExecutor(new BashExecutionService()) - ); + const executorB = createTestExecutor(); const tempDir = new TestTempDir("test-bash-bg-read"); // Config is for workspace-a @@ -222,7 +219,7 @@ describe("bash_background_read tool", () => { config.backgroundProcessManager = manager; // Spawn process in workspace-b - const spawnResult = await manager.spawn("workspace-b", "echo secret", { + const spawnResult = await manager.spawn(executorB, "workspace-b", "echo secret", { cwd: process.cwd(), }); diff --git a/src/node/services/tools/bash_background_terminate.test.ts b/src/node/services/tools/bash_background_terminate.test.ts index a50471ce8..b6d605070 100644 --- a/src/node/services/tools/bash_background_terminate.test.ts +++ b/src/node/services/tools/bash_background_terminate.test.ts @@ -3,6 +3,7 @@ import { createBashBackgroundTerminateTool } from "./bash_background_terminate"; import { BackgroundProcessManager } from "@/node/services/backgroundProcessManager"; import { BashExecutionService } from "@/node/services/bashExecutionService"; import { LocalBackgroundExecutor } from "@/node/services/localBackgroundExecutor"; +import type { BackgroundExecutor } from "@/node/services/backgroundExecutor"; import type { BashBackgroundTerminateArgs, BashBackgroundTerminateResult, @@ -15,11 +16,9 @@ const mockToolCallOptions: ToolCallOptions = { messages: [], }; -// Helper to create manager with executor registered for a workspace -function createManagerWithExecutor(workspaceId: string): BackgroundProcessManager { - const manager = new BackgroundProcessManager(); - manager.registerExecutor(workspaceId, new LocalBackgroundExecutor(new BashExecutionService())); - return manager; +// Create a test executor +function createTestExecutor(): BackgroundExecutor { + return new LocalBackgroundExecutor(new BashExecutionService()); } describe("bash_background_terminate tool", () => { @@ -47,7 +46,7 @@ describe("bash_background_terminate tool", () => { }); it("should return error for non-existent process", async () => { - const manager = createManagerWithExecutor("test-workspace"); + const manager = new BackgroundProcessManager(); const tempDir = new TestTempDir("test-bash-bg-term"); const config = createTestToolConfig(process.cwd()); config.runtimeTempDir = tempDir.path; @@ -67,14 +66,15 @@ describe("bash_background_terminate tool", () => { }); it("should terminate a running process", async () => { - const manager = createManagerWithExecutor("test-workspace"); + const manager = new BackgroundProcessManager(); + const executor = createTestExecutor(); const tempDir = new TestTempDir("test-bash-bg-term"); const config = createTestToolConfig(process.cwd()); config.runtimeTempDir = tempDir.path; config.backgroundProcessManager = manager; // Spawn a long-running process - const spawnResult = await manager.spawn("test-workspace", "sleep 10", { + const spawnResult = await manager.spawn(executor, "test-workspace", "sleep 10", { cwd: process.cwd(), }); @@ -105,14 +105,15 @@ describe("bash_background_terminate tool", () => { }); it("should be idempotent (double-terminate succeeds)", async () => { - const manager = createManagerWithExecutor("test-workspace"); + const manager = new BackgroundProcessManager(); + const executor = createTestExecutor(); const tempDir = new TestTempDir("test-bash-bg-term"); const config = createTestToolConfig(process.cwd()); config.runtimeTempDir = tempDir.path; config.backgroundProcessManager = manager; // Spawn a process - const spawnResult = await manager.spawn("test-workspace", "sleep 10", { + const spawnResult = await manager.spawn(executor, "test-workspace", "sleep 10", { cwd: process.cwd(), }); @@ -144,14 +145,7 @@ describe("bash_background_terminate tool", () => { it("should not terminate processes from other workspaces", async () => { const manager = new BackgroundProcessManager(); - manager.registerExecutor( - "workspace-a", - new LocalBackgroundExecutor(new BashExecutionService()) - ); - manager.registerExecutor( - "workspace-b", - new LocalBackgroundExecutor(new BashExecutionService()) - ); + const executorB = createTestExecutor(); const tempDir = new TestTempDir("test-bash-bg-term"); // Config is for workspace-a @@ -160,7 +154,7 @@ describe("bash_background_terminate tool", () => { config.backgroundProcessManager = manager; // Spawn process in workspace-b - const spawnResult = await manager.spawn("workspace-b", "sleep 10", { + const spawnResult = await manager.spawn(executorB, "workspace-b", "sleep 10", { cwd: process.cwd(), }); From 8a7d8f7c548af4a602509ec747a650f788eb61f4 Mon Sep 17 00:00:00 2001 From: ethan Date: Tue, 25 Nov 2025 20:16:28 +1100 Subject: [PATCH 08/13] =?UTF-8?q?=F0=9F=A4=96=20refactor:=20move=20backgro?= =?UTF-8?q?und=20execution=20to=20Runtime=20interface?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove BackgroundExecutor abstraction layer - Runtime already handles local vs SSH execution differences, so background spawning belongs there. - Add spawnBackground() to Runtime interface with BackgroundHandle type - Implement LocalBackgroundHandle in LocalRuntime with event buffering - Stub SSHRuntime.spawnBackground() (not supported for SSH workspaces) - Simplify BackgroundProcessManager to call runtime directly - Update bash tool to pass runtime from ToolConfiguration - Remove aiService executor registration (no longer needed) - Delete backgroundExecutor.ts, localBackgroundExecutor.ts, sshBackgroundExecutor.ts ~300 LoC removed, cleaner separation of concerns. --- src/common/utils/tools/tools.ts | 3 - src/node/runtime/LocalRuntime.ts | 220 ++++++++++++++++++ src/node/runtime/Runtime.ts | 69 ++++++ src/node/runtime/SSHRuntime.ts | 14 ++ src/node/services/aiService.ts | 26 --- src/node/services/backgroundExecutor.ts | 87 ------- .../services/backgroundProcessManager.test.ts | 55 ++--- src/node/services/backgroundProcessManager.ts | 28 +-- src/node/services/localBackgroundExecutor.ts | 198 ---------------- src/node/services/sshBackgroundExecutor.ts | 43 ---- src/node/services/tools/bash.test.ts | 14 +- src/node/services/tools/bash.ts | 4 +- .../tools/bash_background_list.test.ts | 22 +- .../tools/bash_background_read.test.ts | 38 +-- .../tools/bash_background_terminate.test.ts | 23 +- 15 files changed, 384 insertions(+), 460 deletions(-) delete mode 100644 src/node/services/backgroundExecutor.ts delete mode 100644 src/node/services/localBackgroundExecutor.ts delete mode 100644 src/node/services/sshBackgroundExecutor.ts diff --git a/src/common/utils/tools/tools.ts b/src/common/utils/tools/tools.ts index d10de8416..3d6b11949 100644 --- a/src/common/utils/tools/tools.ts +++ b/src/common/utils/tools/tools.ts @@ -16,7 +16,6 @@ import { log } from "@/node/services/log"; import type { Runtime } from "@/node/runtime/Runtime"; import type { InitStateManager } from "@/node/services/initStateManager"; import type { BackgroundProcessManager } from "@/node/services/backgroundProcessManager"; -import type { BackgroundExecutor } from "@/node/services/backgroundExecutor"; /** * Configuration for tools that need runtime context @@ -36,8 +35,6 @@ export interface ToolConfiguration { overflow_policy?: "truncate" | "tmpfile"; /** Background process manager for bash tool (optional, AI-only) */ backgroundProcessManager?: BackgroundProcessManager; - /** Background executor for this workspace (optional, AI-only) */ - backgroundExecutor?: BackgroundExecutor; /** Workspace ID for tracking background processes (optional for token estimation) */ workspaceId?: string; } diff --git a/src/node/runtime/LocalRuntime.ts b/src/node/runtime/LocalRuntime.ts index 81012cd12..c168fa0cd 100644 --- a/src/node/runtime/LocalRuntime.ts +++ b/src/node/runtime/LocalRuntime.ts @@ -15,6 +15,9 @@ import type { WorkspaceForkParams, WorkspaceForkResult, InitLogger, + BackgroundSpawnOptions, + BackgroundSpawnResult, + BackgroundHandle, } from "./Runtime"; import { RuntimeError as RuntimeErrorClass } from "./Runtime"; import { NON_INTERACTIVE_ENV_VARS } from "@/common/constants/env"; @@ -30,6 +33,130 @@ import { import { execAsync, DisposableProcess } from "@/node/utils/disposableExec"; import { getProjectName } from "@/node/utils/runtime/helpers"; import { getErrorMessage } from "@/common/utils/errors"; +import { once } from "node:events"; +import { log } from "@/node/services/log"; + +/** + * Handle to a local background process. + * + * Buffers early events until callbacks are registered, since the manager + * registers callbacks after spawn() returns (but output may arrive before). + */ +class LocalBackgroundHandle implements BackgroundHandle { + private stdoutCallback?: (line: string) => void; + private stderrCallback?: (line: string) => void; + private exitCallback?: (exitCode: number) => void; + private terminated = false; + + // Buffers for events that arrive before callbacks are registered + private pendingStdout: string[] = []; + private pendingStderr: string[] = []; + private pendingExitCode?: number; + + constructor(private readonly disposable: DisposableProcess) {} + + onStdout(callback: (line: string) => void): void { + this.stdoutCallback = callback; + // Flush buffered events + for (const line of this.pendingStdout) { + callback(line); + } + this.pendingStdout = []; + } + + onStderr(callback: (line: string) => void): void { + this.stderrCallback = callback; + // Flush buffered events + for (const line of this.pendingStderr) { + callback(line); + } + this.pendingStderr = []; + } + + onExit(callback: (exitCode: number) => void): void { + this.exitCallback = callback; + // Flush buffered event + if (this.pendingExitCode !== undefined) { + callback(this.pendingExitCode); + this.pendingExitCode = undefined; + } + } + + /** Internal: called when stdout line arrives */ + _emitStdout(line: string): void { + if (this.stdoutCallback) { + this.stdoutCallback(line); + } else { + this.pendingStdout.push(line); + } + } + + /** Internal: called when stderr line arrives */ + _emitStderr(line: string): void { + if (this.stderrCallback) { + this.stderrCallback(line); + } else { + this.pendingStderr.push(line); + } + } + + /** Internal: called when process exits */ + _emitExit(exitCode: number): void { + if (this.exitCallback) { + this.exitCallback(exitCode); + } else { + this.pendingExitCode = exitCode; + } + } + + isRunning(): Promise { + return Promise.resolve(this.disposable.underlying.exitCode === null); + } + + async terminate(): Promise { + if (this.terminated) return; + + const pid = this.disposable.underlying.pid; + if (pid === undefined) { + this.terminated = true; + return; + } + + try { + // Send SIGTERM to the process group for graceful shutdown + // Use negative PID to kill the entire process group (detached processes are group leaders) + const pgid = -pid; + log.debug(`LocalBackgroundHandle: Sending SIGTERM to process group (PGID: ${pgid})`); + process.kill(pgid, "SIGTERM"); + + // Wait 2 seconds for graceful shutdown + await new Promise((resolve) => setTimeout(resolve, 2000)); + + // Check if process is still running + if (await this.isRunning()) { + log.debug(`LocalBackgroundHandle: Process still running, sending SIGKILL`); + process.kill(pgid, "SIGKILL"); + } + } catch (error) { + // Process may already be dead - that's fine + log.debug( + `LocalBackgroundHandle: Error during terminate: ${error instanceof Error ? error.message : String(error)}` + ); + } + + this.terminated = true; + } + + dispose(): Promise { + return Promise.resolve(this.disposable[Symbol.dispose]()); + } + + /** Get the underlying child process (for spawn event waiting) */ + get child() { + return this.disposable.underlying; + } +} + import { expandTilde } from "./tildeExpansion"; /** @@ -179,6 +306,99 @@ export class LocalRuntime implements Runtime { return { stdout, stderr, stdin, exitCode, duration }; } + async spawnBackground( + script: string, + options: BackgroundSpawnOptions + ): Promise { + log.debug(`LocalRuntime.spawnBackground: Spawning in ${options.cwd}`); + + // Check if working directory exists + try { + await fsPromises.access(options.cwd); + } catch { + return { success: false, error: `Working directory does not exist: ${options.cwd}` }; + } + + // Build command with optional niceness + const isWindows = process.platform === "win32"; + const bashPath = getBashPath(); + const spawnCommand = options.niceness !== undefined && !isWindows ? "nice" : bashPath; + const spawnArgs = + options.niceness !== undefined && !isWindows + ? ["-n", options.niceness.toString(), bashPath, "-c", script] + : ["-c", script]; + + const childProcess = spawn(spawnCommand, spawnArgs, { + cwd: options.cwd, + env: { + ...process.env, + ...(options.env ?? {}), + ...NON_INTERACTIVE_ENV_VARS, + }, + stdio: ["pipe", "pipe", "pipe"], + detached: true, + }); + + const disposable = new DisposableProcess(childProcess); + + // Declare handle before setting up listeners + // eslint-disable-next-line prefer-const + let handle: LocalBackgroundHandle; + + // Set up line-buffered output streaming + const decoder = new TextDecoder(); + let stdoutBuffer = ""; + let stderrBuffer = ""; + + childProcess.stdout.on("data", (chunk: Buffer) => { + stdoutBuffer += decoder.decode(chunk, { stream: true }); + const lines = stdoutBuffer.split("\n"); + stdoutBuffer = lines.pop() ?? ""; + for (const line of lines) { + handle._emitStdout(line); + } + }); + + childProcess.stderr.on("data", (chunk: Buffer) => { + stderrBuffer += decoder.decode(chunk, { stream: true }); + const lines = stderrBuffer.split("\n"); + stderrBuffer = lines.pop() ?? ""; + for (const line of lines) { + handle._emitStderr(line); + } + }); + + childProcess.on("exit", (code) => { + // Flush remaining partial lines + if (stdoutBuffer) handle._emitStdout(stdoutBuffer); + if (stderrBuffer) handle._emitStderr(stderrBuffer); + handle._emitExit(code ?? 0); + }); + + handle = new LocalBackgroundHandle(disposable); + + // Wait for spawn or error + try { + await Promise.race([ + once(childProcess, "spawn"), + once(childProcess, "error").then(([err]) => { + throw err; + }), + new Promise((_, reject) => + setTimeout(() => reject(new Error("Spawn did not complete in time")), 2000) + ), + ]); + } catch (e) { + const err = e as Error; + log.debug(`LocalRuntime.spawnBackground: Failed to spawn: ${err.message}`); + await handle.dispose(); + return { success: false, error: err.message }; + } + + log.debug(`LocalRuntime.spawnBackground: Spawned with PID ${childProcess.pid ?? "unknown"}`); + return { success: true, handle }; + } + readFile(filePath: string, _abortSignal?: AbortSignal): ReadableStream { // Note: _abortSignal ignored for local operations (fast, no need for cancellation) const nodeStream = fs.createReadStream(filePath); diff --git a/src/node/runtime/Runtime.ts b/src/node/runtime/Runtime.ts index 4e01a0ceb..a75ecfe03 100644 --- a/src/node/runtime/Runtime.ts +++ b/src/node/runtime/Runtime.ts @@ -64,6 +64,64 @@ export interface ExecOptions { forcePTY?: boolean; } +/** + * Options for spawning a background process + */ +export interface BackgroundSpawnOptions { + /** Working directory for command execution */ + cwd: string; + /** Environment variables to inject */ + env?: Record; + /** Process niceness level (-20 to 19, lower = higher priority) */ + niceness?: number; +} + +/** + * Handle to a background process. + * Abstracts away whether process is local or remote. + */ +export interface BackgroundHandle { + /** + * Register callback for stdout lines. + * For local: called in real-time as output arrives. + * For SSH: called when output is polled/read. + */ + onStdout(callback: (line: string) => void): void; + + /** + * Register callback for stderr lines. + */ + onStderr(callback: (line: string) => void): void; + + /** + * Register callback for process exit. + * @param callback Receives exit code (128+signal for signal termination) + */ + onExit(callback: (exitCode: number) => void): void; + + /** + * Check if process is still running. + */ + isRunning(): Promise; + + /** + * Terminate the process (SIGTERM → wait → SIGKILL). + */ + terminate(): Promise; + + /** + * Clean up resources (called after process exits or on error). + */ + dispose(): Promise; +} + +/** + * Result of spawning a background process + */ +export type BackgroundSpawnResult = + | { success: true; handle: BackgroundHandle } + | { success: false; error: string }; + /** * Streaming result from executing a command */ @@ -211,6 +269,17 @@ export interface Runtime { */ exec(command: string, options: ExecOptions): Promise; + /** + * Spawn a detached background process. + * Returns a handle for monitoring output and terminating the process. + * Unlike exec(), background processes have no timeout and run until terminated. + * + * @param script Bash script to execute + * @param options Execution options (cwd, env, niceness) + * @returns BackgroundHandle on success, or error + */ + spawnBackground(script: string, options: BackgroundSpawnOptions): Promise; + /** * Read file contents as a stream * @param path Absolute or relative path to file diff --git a/src/node/runtime/SSHRuntime.ts b/src/node/runtime/SSHRuntime.ts index 22588e873..f6e027e18 100644 --- a/src/node/runtime/SSHRuntime.ts +++ b/src/node/runtime/SSHRuntime.ts @@ -13,6 +13,8 @@ import type { WorkspaceForkParams, WorkspaceForkResult, InitLogger, + BackgroundSpawnOptions, + BackgroundSpawnResult, } from "./Runtime"; import { RuntimeError as RuntimeErrorClass } from "./Runtime"; import { EXIT_CODE_ABORTED, EXIT_CODE_TIMEOUT } from "@/common/constants/exitCodes"; @@ -247,6 +249,18 @@ export class SSHRuntime implements Runtime { return { stdout, stderr, stdin, exitCode, duration }; } + spawnBackground( + _script: string, + _options: BackgroundSpawnOptions + ): Promise { + // SSH background execution not yet implemented + // Would require: remote process management, output polling, etc. + return Promise.resolve({ + success: false, + error: "Background process execution is not supported for SSH workspaces", + }); + } + /** * Read file contents over SSH as a stream */ diff --git a/src/node/services/aiService.ts b/src/node/services/aiService.ts index 8113107d9..8aee553d6 100644 --- a/src/node/services/aiService.ts +++ b/src/node/services/aiService.ts @@ -21,12 +21,6 @@ import { createRuntime } from "@/node/runtime/runtimeFactory"; import { secretsToRecord } from "@/common/types/secrets"; import type { MuxProviderOptions } from "@/common/types/providerOptions"; import type { BackgroundProcessManager } from "@/node/services/backgroundProcessManager"; -import type { BackgroundExecutor } from "@/node/services/backgroundExecutor"; -import { LocalBackgroundExecutor } from "@/node/services/localBackgroundExecutor"; -import { SSHBackgroundExecutor } from "@/node/services/sshBackgroundExecutor"; -import { BashExecutionService } from "@/node/services/bashExecutionService"; -import { isSSHRuntime, type RuntimeConfig } from "@/common/types/runtime"; -import type { Runtime } from "@/node/runtime/Runtime"; import { log } from "./log"; import { transformModelMessages, @@ -149,7 +143,6 @@ export class AIService extends EventEmitter { private readonly mockModeEnabled: boolean; private readonly mockScenarioPlayer?: MockScenarioPlayer; private readonly backgroundProcessManager?: BackgroundProcessManager; - private readonly bashExecutionService: BashExecutionService; constructor( config: Config, @@ -167,7 +160,6 @@ export class AIService extends EventEmitter { this.partialService = partialService; this.initStateManager = initStateManager; this.backgroundProcessManager = backgroundProcessManager; - this.bashExecutionService = new BashExecutionService(); this.streamManager = new StreamManager(historyService, partialService); void this.ensureSessionsDir(); this.setupStreamEventForwarding(); @@ -181,20 +173,6 @@ export class AIService extends EventEmitter { } } - /** - * Create a background executor appropriate for the given runtime. - * Local runtimes use LocalBackgroundExecutor, SSH runtimes use SSHBackgroundExecutor. - */ - private createBackgroundExecutor( - runtimeConfig: RuntimeConfig, - runtime: Runtime - ): BackgroundExecutor { - if (isSSHRuntime(runtimeConfig)) { - return new SSHBackgroundExecutor(runtime); - } - return new LocalBackgroundExecutor(this.bashExecutionService); - } - /** * Forward all stream events from StreamManager to AIService consumers */ @@ -753,9 +731,6 @@ export class AIService extends EventEmitter { ? metadata.projectPath : runtime.getWorkspacePath(metadata.projectPath, metadata.name); - // Create background executor for this workspace (cached on first spawn) - const backgroundExecutor = this.createBackgroundExecutor(runtimeConfig, runtime); - // Build system message from workspace metadata const systemMessage = await buildSystemMessage( metadata, @@ -794,7 +769,6 @@ export class AIService extends EventEmitter { secrets: secretsToRecord(projectSecrets), runtimeTempDir, backgroundProcessManager: this.backgroundProcessManager, - backgroundExecutor, workspaceId, }, workspaceId, diff --git a/src/node/services/backgroundExecutor.ts b/src/node/services/backgroundExecutor.ts deleted file mode 100644 index c8327d58f..000000000 --- a/src/node/services/backgroundExecutor.ts +++ /dev/null @@ -1,87 +0,0 @@ -/** - * Background process execution abstraction. - * - * This interface allows BackgroundProcessManager to work with different - * execution backends (local processes, SSH remote processes, etc.) - */ - -/** - * Configuration for background execution - */ -export interface BackgroundExecConfig { - /** Working directory for command execution */ - cwd: string; - /** Environment variables to inject */ - env?: Record; - /** Process niceness level (-20 to 19) */ - niceness?: number; -} - -/** - * Handle to a background process. - * Abstracts away whether process is local or remote. - */ -export interface BackgroundHandle { - /** - * Register callback for stdout lines. - * For local: called in real-time as output arrives. - * For SSH: called when output is polled/read. - */ - onStdout(callback: (line: string) => void): void; - - /** - * Register callback for stderr lines. - */ - onStderr(callback: (line: string) => void): void; - - /** - * Register callback for process exit. - * @param callback Receives exit code (128+signal for signal termination) - */ - onExit(callback: (exitCode: number) => void): void; - - /** - * Check if process is still running. - * For local: checks ChildProcess.exitCode - * For SSH: runs `kill -0 $PID` on remote - */ - isRunning(): Promise; - - /** - * Terminate the process (SIGTERM → wait → SIGKILL). - * For local: process.kill(-pid, signal) - * For SSH: ssh "kill -TERM -$PID" - */ - terminate(): Promise; - - /** - * Clean up resources (called after process exits or on error). - * For local: disposes ChildProcess - * For SSH: removes remote temp files - */ - dispose(): Promise; -} - -/** - * Result of spawning a background process - */ -export type BackgroundSpawnResult = - | { success: true; handle: BackgroundHandle } - | { success: false; error: string }; - -/** - * Executor interface for spawning background processes. - * - * Implementations: - * - LocalBackgroundExecutor: Uses BashExecutionService for local processes - * - SSHBackgroundExecutor: Uses nohup/setsid + file-based output (TODO) - */ -export interface BackgroundExecutor { - /** - * Spawn a background process. - * @param script Bash script to execute - * @param config Execution configuration - * @returns BackgroundHandle on success, or error - */ - spawn(script: string, config: BackgroundExecConfig): Promise; -} diff --git a/src/node/services/backgroundProcessManager.test.ts b/src/node/services/backgroundProcessManager.test.ts index 1d75b51c2..f6027f6a8 100644 --- a/src/node/services/backgroundProcessManager.test.ts +++ b/src/node/services/backgroundProcessManager.test.ts @@ -1,28 +1,27 @@ import { describe, it, expect, beforeEach } from "bun:test"; import { BackgroundProcessManager } from "./backgroundProcessManager"; -import { BashExecutionService } from "./bashExecutionService"; -import { LocalBackgroundExecutor } from "./localBackgroundExecutor"; -import type { BackgroundExecutor } from "./backgroundExecutor"; +import { LocalRuntime } from "@/node/runtime/LocalRuntime"; +import type { Runtime } from "@/node/runtime/Runtime"; -// Create a shared executor for tests -function createTestExecutor(): BackgroundExecutor { - return new LocalBackgroundExecutor(new BashExecutionService()); +// Create test runtime (uses local machine) +function createTestRuntime(): Runtime { + return new LocalRuntime(process.cwd()); } describe("BackgroundProcessManager", () => { let manager: BackgroundProcessManager; - let executor: BackgroundExecutor; + let runtime: Runtime; const testWorkspaceId = "workspace-1"; const testWorkspaceId2 = "workspace-2"; beforeEach(() => { manager = new BackgroundProcessManager(); - executor = createTestExecutor(); + runtime = createTestRuntime(); }); describe("spawn", () => { it("should spawn a background process and return process ID", async () => { - const result = await manager.spawn(executor, testWorkspaceId, "echo hello", { + const result = await manager.spawn(runtime, testWorkspaceId, "echo hello", { cwd: process.cwd(), }); @@ -33,7 +32,7 @@ describe("BackgroundProcessManager", () => { }); it("should return error on spawn failure", async () => { - const result = await manager.spawn(executor, testWorkspaceId, "echo test", { + const result = await manager.spawn(runtime, testWorkspaceId, "echo test", { cwd: "/nonexistent/path/that/does/not/exist", }); @@ -41,7 +40,7 @@ describe("BackgroundProcessManager", () => { }); it("should capture stdout and stderr", async () => { - const result = await manager.spawn(executor, testWorkspaceId, "echo hello; echo world >&2", { + const result = await manager.spawn(runtime, testWorkspaceId, "echo hello; echo world >&2", { cwd: process.cwd(), }); @@ -64,7 +63,7 @@ describe("BackgroundProcessManager", () => { .map((_, i) => `echo line${i}`) .join("; "); - const result = await manager.spawn(executor, testWorkspaceId, script, { + const result = await manager.spawn(runtime, testWorkspaceId, script, { cwd: process.cwd(), }); @@ -82,7 +81,7 @@ describe("BackgroundProcessManager", () => { describe("getProcess", () => { it("should return process by ID", async () => { - const spawnResult = await manager.spawn(executor, testWorkspaceId, "sleep 1", { + const spawnResult = await manager.spawn(runtime, testWorkspaceId, "sleep 1", { cwd: process.cwd(), }); @@ -102,18 +101,16 @@ describe("BackgroundProcessManager", () => { describe("list", () => { it("should list all processes", async () => { - await manager.spawn(executor, testWorkspaceId, "sleep 1", { cwd: process.cwd() }); - await manager.spawn(executor, testWorkspaceId, "sleep 1", { cwd: process.cwd() }); + await manager.spawn(runtime, testWorkspaceId, "sleep 1", { cwd: process.cwd() }); + await manager.spawn(runtime, testWorkspaceId, "sleep 1", { cwd: process.cwd() }); const processes = manager.list(); expect(processes.length).toBeGreaterThanOrEqual(2); }); it("should filter by workspace ID", async () => { - const executor2 = createTestExecutor(); - - await manager.spawn(executor, testWorkspaceId, "sleep 1", { cwd: process.cwd() }); - await manager.spawn(executor2, testWorkspaceId2, "sleep 1", { cwd: process.cwd() }); + await manager.spawn(runtime, testWorkspaceId, "sleep 1", { cwd: process.cwd() }); + await manager.spawn(runtime, testWorkspaceId2, "sleep 1", { cwd: process.cwd() }); const ws1Processes = manager.list(testWorkspaceId); const ws2Processes = manager.list(testWorkspaceId2); @@ -127,7 +124,7 @@ describe("BackgroundProcessManager", () => { describe("terminate", () => { it("should terminate a running process", async () => { - const spawnResult = await manager.spawn(executor, testWorkspaceId, "sleep 10", { + const spawnResult = await manager.spawn(runtime, testWorkspaceId, "sleep 10", { cwd: process.cwd(), }); @@ -146,7 +143,7 @@ describe("BackgroundProcessManager", () => { }); it("should be idempotent (double-terminate succeeds)", async () => { - const spawnResult = await manager.spawn(executor, testWorkspaceId, "sleep 10", { + const spawnResult = await manager.spawn(runtime, testWorkspaceId, "sleep 10", { cwd: process.cwd(), }); @@ -161,12 +158,10 @@ describe("BackgroundProcessManager", () => { }); describe("cleanup", () => { - it("should kill all processes for a workspace and clear cached executor", async () => { - const executor2 = createTestExecutor(); - - await manager.spawn(executor, testWorkspaceId, "sleep 10", { cwd: process.cwd() }); - await manager.spawn(executor, testWorkspaceId, "sleep 10", { cwd: process.cwd() }); - await manager.spawn(executor2, testWorkspaceId2, "sleep 10", { cwd: process.cwd() }); + it("should kill all processes for a workspace", async () => { + await manager.spawn(runtime, testWorkspaceId, "sleep 10", { cwd: process.cwd() }); + await manager.spawn(runtime, testWorkspaceId, "sleep 10", { cwd: process.cwd() }); + await manager.spawn(runtime, testWorkspaceId2, "sleep 10", { cwd: process.cwd() }); await manager.cleanup(testWorkspaceId); @@ -182,7 +177,7 @@ describe("BackgroundProcessManager", () => { describe("process state tracking", () => { it("should track process exit", async () => { - const result = await manager.spawn(executor, testWorkspaceId, "exit 42", { + const result = await manager.spawn(runtime, testWorkspaceId, "exit 42", { cwd: process.cwd(), }); @@ -198,7 +193,7 @@ describe("BackgroundProcessManager", () => { }); it("should keep buffer after process exits", async () => { - const result = await manager.spawn(executor, testWorkspaceId, "echo test; exit 0", { + const result = await manager.spawn(runtime, testWorkspaceId, "echo test; exit 0", { cwd: process.cwd(), }); @@ -213,7 +208,7 @@ describe("BackgroundProcessManager", () => { it("should preserve killed status after onExit callback fires", async () => { // Spawn a long-running process - const result = await manager.spawn(executor, testWorkspaceId, "sleep 60", { + const result = await manager.spawn(runtime, testWorkspaceId, "sleep 60", { cwd: process.cwd(), }); diff --git a/src/node/services/backgroundProcessManager.ts b/src/node/services/backgroundProcessManager.ts index 7227da699..87b7d63c8 100644 --- a/src/node/services/backgroundProcessManager.ts +++ b/src/node/services/backgroundProcessManager.ts @@ -1,4 +1,4 @@ -import type { BackgroundExecutor, BackgroundHandle } from "./backgroundExecutor"; +import type { Runtime, BackgroundHandle } from "@/node/runtime/Runtime"; import { log } from "./log"; import { randomBytes } from "crypto"; import { CircularBuffer } from "./circularBuffer"; @@ -24,30 +24,27 @@ const MAX_BUFFER_LINES = 1000; /** * Manages background bash processes for workspaces. * - * Executors are provided lazily at spawn time and cached per workspace. - * This allows different execution backends per workspace (local vs SSH). + * Processes are spawned via Runtime.spawnBackground() and tracked by ID. + * Output is stored in circular buffers for later retrieval. */ export class BackgroundProcessManager { private processes = new Map(); - private executors = new Map(); /** * Spawn a new background process. - * The executor is cached on first spawn per workspace for reuse (e.g., SSH connection pooling). + * @param runtime Runtime to spawn the process on + * @param workspaceId Workspace ID for tracking/filtering + * @param script Bash script to execute + * @param config Execution configuration */ async spawn( - executor: BackgroundExecutor, + runtime: Runtime, workspaceId: string, script: string, config: { cwd: string; secrets?: Record; niceness?: number } ): Promise<{ success: true; processId: string } | { success: false; error: string }> { log.debug(`BackgroundProcessManager.spawn() called for workspace ${workspaceId}`); - // Cache executor on first spawn for this workspace (enables SSH connection reuse) - if (!this.executors.has(workspaceId)) { - this.executors.set(workspaceId, executor); - } - // Generate unique process ID const processId = `bg-${randomBytes(4).toString("hex")}`; @@ -66,8 +63,8 @@ export class BackgroundProcessManager { handle: null, }; - // Spawn via executor - const result = await executor.spawn(script, { + // Spawn via runtime + const result = await runtime.spawnBackground(script, { cwd: config.cwd, env: config.secrets, niceness: config.niceness, @@ -181,7 +178,7 @@ export class BackgroundProcessManager { /** * Clean up all processes for a workspace. - * Terminates running processes, removes them from memory, and clears the cached executor. + * Terminates running processes and removes them from memory. */ async cleanup(workspaceId: string): Promise { log.debug(`BackgroundProcessManager.cleanup(${workspaceId}) called`); @@ -195,9 +192,6 @@ export class BackgroundProcessManager { // Remove all processes from memory matching.forEach((p) => this.processes.delete(p.id)); - // Clear cached executor for this workspace - this.executors.delete(workspaceId); - log.debug(`Cleaned up ${matching.length} process(es) for workspace ${workspaceId}`); } } diff --git a/src/node/services/localBackgroundExecutor.ts b/src/node/services/localBackgroundExecutor.ts deleted file mode 100644 index 99d2fa272..000000000 --- a/src/node/services/localBackgroundExecutor.ts +++ /dev/null @@ -1,198 +0,0 @@ -/** - * Local background executor implementation. - * - * Uses BashExecutionService to spawn detached process groups on the local machine. - * Output is streamed in real-time via callbacks. - */ - -import { once } from "node:events"; -import type { - BackgroundExecutor, - BackgroundExecConfig, - BackgroundHandle, - BackgroundSpawnResult, -} from "./backgroundExecutor"; -import type { BashExecutionService, DisposableProcess } from "./bashExecutionService"; -import { log } from "./log"; - -/** - * Handle to a local background process - * - * Buffers early events until callbacks are registered, since the manager - * registers callbacks after spawn() returns (but output may arrive before). - */ -class LocalBackgroundHandle implements BackgroundHandle { - private stdoutCallback?: (line: string) => void; - private stderrCallback?: (line: string) => void; - private exitCallback?: (exitCode: number) => void; - private terminated = false; - - // Buffers for events that arrive before callbacks are registered - private pendingStdout: string[] = []; - private pendingStderr: string[] = []; - private pendingExitCode?: number; - - constructor(private readonly disposable: DisposableProcess) {} - - onStdout(callback: (line: string) => void): void { - this.stdoutCallback = callback; - // Flush buffered events - for (const line of this.pendingStdout) { - callback(line); - } - this.pendingStdout = []; - } - - onStderr(callback: (line: string) => void): void { - this.stderrCallback = callback; - // Flush buffered events - for (const line of this.pendingStderr) { - callback(line); - } - this.pendingStderr = []; - } - - onExit(callback: (exitCode: number) => void): void { - this.exitCallback = callback; - // Flush buffered event - if (this.pendingExitCode !== undefined) { - callback(this.pendingExitCode); - this.pendingExitCode = undefined; - } - } - - /** Internal: called by executor when stdout line arrives */ - _emitStdout(line: string): void { - if (this.stdoutCallback) { - this.stdoutCallback(line); - } else { - this.pendingStdout.push(line); - } - } - - /** Internal: called by executor when stderr line arrives */ - _emitStderr(line: string): void { - if (this.stderrCallback) { - this.stderrCallback(line); - } else { - this.pendingStderr.push(line); - } - } - - /** Internal: called by executor when process exits */ - _emitExit(exitCode: number): void { - if (this.exitCallback) { - this.exitCallback(exitCode); - } else { - this.pendingExitCode = exitCode; - } - } - - isRunning(): Promise { - return Promise.resolve(this.disposable.child.exitCode === null); - } - - async terminate(): Promise { - if (this.terminated) return; - - const pid = this.disposable.child.pid; - if (pid === undefined) { - this.terminated = true; - return; - } - - try { - // Send SIGTERM to the process group for graceful shutdown - // Use negative PID to kill the entire process group (detached processes are group leaders) - const pgid = -pid; - log.debug(`LocalBackgroundHandle: Sending SIGTERM to process group (PGID: ${pgid})`); - process.kill(pgid, "SIGTERM"); - - // Wait 2 seconds for graceful shutdown - await new Promise((resolve) => setTimeout(resolve, 2000)); - - // Check if process is still running - if (await this.isRunning()) { - log.debug(`LocalBackgroundHandle: Process still running, sending SIGKILL`); - process.kill(pgid, "SIGKILL"); - } - } catch (error) { - // Process may already be dead - that's fine - log.debug( - `LocalBackgroundHandle: Error during terminate: ${error instanceof Error ? error.message : String(error)}` - ); - } - - this.terminated = true; - } - - dispose(): Promise { - return Promise.resolve(this.disposable[Symbol.dispose]()); - } - - /** Get the child process (for spawn event waiting) */ - get child() { - return this.disposable.child; - } -} - -/** - * Local background executor using BashExecutionService - */ -export class LocalBackgroundExecutor implements BackgroundExecutor { - constructor(private readonly bashService: BashExecutionService) {} - - async spawn(script: string, config: BackgroundExecConfig): Promise { - log.debug(`LocalBackgroundExecutor: Spawning background process in ${config.cwd}`); - - // Declare handle before executeStreaming so callbacks can reference it via closure. - // It's assigned immediately after executeStreaming returns, before any callbacks fire. - // eslint-disable-next-line prefer-const - let handle: LocalBackgroundHandle; - - // Spawn with streaming callbacks that forward to handle - const disposable = this.bashService.executeStreaming( - script, - { - cwd: config.cwd, - secrets: config.env, - niceness: config.niceness, - detached: true, - }, - { - onStdout: (line: string) => handle._emitStdout(line), - onStderr: (line: string) => handle._emitStderr(line), - onExit: (exitCode: number) => handle._emitExit(exitCode), - } - ); - - handle = new LocalBackgroundHandle(disposable); - - // Wait until we know whether the spawn succeeded or failed - // ChildProcess emits either 'spawn' (success) or 'error' (failure) - mutually exclusive - try { - await Promise.race([ - // Successful spawn - once(handle.child, "spawn"), - - // Spawn error (ENOENT, invalid cwd, etc.) - once(handle.child, "error").then(([err]) => { - throw err; - }), - - // Safety timeout to prevent infinite hang - new Promise((_, reject) => - setTimeout(() => reject(new Error("Spawn did not complete in time")), 2000) - ), - ]); - } catch (e) { - const err = e as Error; - log.debug(`LocalBackgroundExecutor: Failed to spawn: ${err.message}`); - await handle.dispose(); - return { success: false, error: err.message }; - } - - log.debug(`LocalBackgroundExecutor: Process spawned with PID ${handle.child.pid ?? "unknown"}`); - return { success: true, handle }; - } -} diff --git a/src/node/services/sshBackgroundExecutor.ts b/src/node/services/sshBackgroundExecutor.ts deleted file mode 100644 index ebc303e94..000000000 --- a/src/node/services/sshBackgroundExecutor.ts +++ /dev/null @@ -1,43 +0,0 @@ -/** - * SSH background executor implementation (STUB). - * - * TODO: Implement using nohup/setsid + file-based output capture. - * - * Implementation approach: - * 1. Spawn: ssh "mkdir -p /tmp/mux-bg/$ID && nohup setsid bash -c 'SCRIPT' > /tmp/mux-bg/$ID/out 2>&1 & echo $!" - * 2. Read: ssh "tail -c +$OFFSET /tmp/mux-bg/$ID/out" (on-demand) - * 3. Status: ssh "kill -0 $PID && echo running || cat /tmp/mux-bg/$ID/exitcode" - * 4. Terminate: ssh "kill -TERM -$PID" then "kill -KILL -$PID" - * 5. Cleanup: ssh "rm -rf /tmp/mux-bg/$ID" - * - * Exit code capture requires wrapper script: - * bash -c 'SCRIPT; echo $? > /tmp/mux-bg/$ID/exitcode' - */ - -import type { - BackgroundExecutor, - BackgroundExecConfig, - BackgroundSpawnResult, -} from "./backgroundExecutor"; -import type { Runtime } from "@/node/runtime/Runtime"; - -/** - * SSH background executor (not yet implemented) - * - * This executor will spawn background processes on remote SSH hosts - * using nohup/setsid for detachment and file-based output capture. - */ -export class SSHBackgroundExecutor implements BackgroundExecutor { - constructor(private readonly _runtime: Runtime) { - // Runtime will be used for SSH commands when implemented - } - - spawn(_script: string, _config: BackgroundExecConfig): Promise { - // TODO: Implement SSH background execution - // See file header for implementation approach - return Promise.resolve({ - success: false, - error: "SSH background execution is not yet implemented", - }); - } -} diff --git a/src/node/services/tools/bash.test.ts b/src/node/services/tools/bash.test.ts index e4e69ced0..8f6239c18 100644 --- a/src/node/services/tools/bash.test.ts +++ b/src/node/services/tools/bash.test.ts @@ -8,14 +8,6 @@ import { TestTempDir, createTestToolConfig, getTestDeps } from "./testHelpers"; import { createRuntime } from "@/node/runtime/runtimeFactory"; import type { ToolCallOptions } from "ai"; import { BackgroundProcessManager } from "@/node/services/backgroundProcessManager"; -import { BashExecutionService } from "@/node/services/bashExecutionService"; -import { LocalBackgroundExecutor } from "@/node/services/localBackgroundExecutor"; -import type { BackgroundExecutor } from "@/node/services/backgroundExecutor"; - -// Create a test executor -function createTestExecutor(): BackgroundExecutor { - return new LocalBackgroundExecutor(new BashExecutionService()); -} // Mock ToolCallOptions for testing const mockToolCallOptions: ToolCallOptions = { @@ -1374,14 +1366,13 @@ describe("bash tool - background execution", () => { it("should reject timeout with background mode", async () => { const manager = new BackgroundProcessManager(); - const executor = createTestExecutor(); const tempDir = new TestTempDir("test-bash-bg"); const config = createTestToolConfig(process.cwd()); config.runtimeTempDir = tempDir.path; config.backgroundProcessManager = manager; - config.backgroundExecutor = executor; config.workspaceId = "test-workspace"; + // config.runtime is already set by createTestToolConfig const tool = createBashTool(config); const args: BashToolArgs = { @@ -1402,14 +1393,13 @@ describe("bash tool - background execution", () => { it("should start background process and return process ID", async () => { const manager = new BackgroundProcessManager(); - const executor = createTestExecutor(); const tempDir = new TestTempDir("test-bash-bg"); const config = createTestToolConfig(process.cwd()); config.runtimeTempDir = tempDir.path; config.backgroundProcessManager = manager; - config.backgroundExecutor = executor; config.workspaceId = "test-workspace"; + // config.runtime is already set by createTestToolConfig const tool = createBashTool(config); const args: BashToolArgs = { diff --git a/src/node/services/tools/bash.ts b/src/node/services/tools/bash.ts index e871e4b30..9cef5731c 100644 --- a/src/node/services/tools/bash.ts +++ b/src/node/services/tools/bash.ts @@ -249,7 +249,7 @@ export const createBashTool: ToolFactory = (config: ToolConfiguration) => { }; } - if (!config.workspaceId || !config.backgroundProcessManager || !config.backgroundExecutor) { + if (!config.workspaceId || !config.backgroundProcessManager || !config.runtime) { return { success: false, error: @@ -270,7 +270,7 @@ export const createBashTool: ToolFactory = (config: ToolConfiguration) => { const startTime = performance.now(); const spawnResult = await config.backgroundProcessManager.spawn( - config.backgroundExecutor, + config.runtime, config.workspaceId, script, { diff --git a/src/node/services/tools/bash_background_list.test.ts b/src/node/services/tools/bash_background_list.test.ts index a071291eb..8a681d314 100644 --- a/src/node/services/tools/bash_background_list.test.ts +++ b/src/node/services/tools/bash_background_list.test.ts @@ -1,9 +1,8 @@ import { describe, it, expect } from "bun:test"; import { createBashBackgroundListTool } from "./bash_background_list"; import { BackgroundProcessManager } from "@/node/services/backgroundProcessManager"; -import { BashExecutionService } from "@/node/services/bashExecutionService"; -import { LocalBackgroundExecutor } from "@/node/services/localBackgroundExecutor"; -import type { BackgroundExecutor } from "@/node/services/backgroundExecutor"; +import { LocalRuntime } from "@/node/runtime/LocalRuntime"; +import type { Runtime } from "@/node/runtime/Runtime"; import type { BashBackgroundListResult } from "@/common/types/tools"; import { TestTempDir, createTestToolConfig } from "./testHelpers"; import type { ToolCallOptions } from "ai"; @@ -13,9 +12,9 @@ const mockToolCallOptions: ToolCallOptions = { messages: [], }; -// Create a test executor -function createTestExecutor(): BackgroundExecutor { - return new LocalBackgroundExecutor(new BashExecutionService()); +// Create test runtime (uses local machine) +function createTestRuntime(): Runtime { + return new LocalRuntime(process.cwd()); } describe("bash_background_list tool", () => { @@ -74,14 +73,14 @@ describe("bash_background_list tool", () => { it("should list spawned processes with correct fields", async () => { const manager = new BackgroundProcessManager(); - const executor = createTestExecutor(); + const runtime = createTestRuntime(); const tempDir = new TestTempDir("test-bash-bg-list"); const config = createTestToolConfig(process.cwd()); config.runtimeTempDir = tempDir.path; config.backgroundProcessManager = manager; // Spawn a process - const spawnResult = await manager.spawn(executor, "test-workspace", "sleep 10", { + const spawnResult = await manager.spawn(runtime, "test-workspace", "sleep 10", { cwd: process.cwd(), }); @@ -110,8 +109,7 @@ describe("bash_background_list tool", () => { it("should only list processes for the current workspace", async () => { const manager = new BackgroundProcessManager(); - const executorA = createTestExecutor(); - const executorB = createTestExecutor(); + const runtime = createTestRuntime(); const tempDir = new TestTempDir("test-bash-bg-list"); const config = createTestToolConfig(process.cwd(), { workspaceId: "workspace-a" }); @@ -119,8 +117,8 @@ describe("bash_background_list tool", () => { config.backgroundProcessManager = manager; // Spawn processes in different workspaces - const spawnA = await manager.spawn(executorA, "workspace-a", "sleep 10", { cwd: process.cwd() }); - const spawnB = await manager.spawn(executorB, "workspace-b", "sleep 10", { cwd: process.cwd() }); + const spawnA = await manager.spawn(runtime, "workspace-a", "sleep 10", { cwd: process.cwd() }); + const spawnB = await manager.spawn(runtime, "workspace-b", "sleep 10", { cwd: process.cwd() }); if (!spawnA.success || !spawnB.success) { throw new Error("Failed to spawn processes"); diff --git a/src/node/services/tools/bash_background_read.test.ts b/src/node/services/tools/bash_background_read.test.ts index 0bff54aab..50a19c357 100644 --- a/src/node/services/tools/bash_background_read.test.ts +++ b/src/node/services/tools/bash_background_read.test.ts @@ -1,9 +1,8 @@ import { describe, it, expect } from "bun:test"; import { createBashBackgroundReadTool } from "./bash_background_read"; import { BackgroundProcessManager } from "@/node/services/backgroundProcessManager"; -import { BashExecutionService } from "@/node/services/bashExecutionService"; -import { LocalBackgroundExecutor } from "@/node/services/localBackgroundExecutor"; -import type { BackgroundExecutor } from "@/node/services/backgroundExecutor"; +import { LocalRuntime } from "@/node/runtime/LocalRuntime"; +import type { Runtime } from "@/node/runtime/Runtime"; import type { BashBackgroundReadArgs, BashBackgroundReadResult } from "@/common/types/tools"; import { TestTempDir, createTestToolConfig } from "./testHelpers"; import type { ToolCallOptions } from "ai"; @@ -13,9 +12,9 @@ const mockToolCallOptions: ToolCallOptions = { messages: [], }; -// Create a test executor -function createTestExecutor(): BackgroundExecutor { - return new LocalBackgroundExecutor(new BashExecutionService()); +// Create test runtime (uses local machine) +function createTestRuntime(): Runtime { + return new LocalRuntime(process.cwd()); } describe("bash_background_read tool", () => { @@ -63,14 +62,14 @@ describe("bash_background_read tool", () => { it("should return process status and output", async () => { const manager = new BackgroundProcessManager(); - const executor = createTestExecutor(); + const runtime = createTestRuntime(); const tempDir = new TestTempDir("test-bash-bg-read"); const config = createTestToolConfig(process.cwd()); config.runtimeTempDir = tempDir.path; config.backgroundProcessManager = manager; // Spawn a process - const spawnResult = await manager.spawn(executor, "test-workspace", "echo hello; sleep 1", { + const spawnResult = await manager.spawn(runtime, "test-workspace", "echo hello; sleep 1", { cwd: process.cwd(), }); @@ -101,7 +100,7 @@ describe("bash_background_read tool", () => { it("should handle tail filtering", async () => { const manager = new BackgroundProcessManager(); - const executor = createTestExecutor(); + const runtime = createTestRuntime(); const tempDir = new TestTempDir("test-bash-bg-read"); const config = createTestToolConfig(process.cwd()); config.runtimeTempDir = tempDir.path; @@ -109,7 +108,7 @@ describe("bash_background_read tool", () => { // Spawn a process with multiple lines const spawnResult = await manager.spawn( - executor, + runtime, "test-workspace", "echo line1; echo line2; echo line3", { @@ -141,15 +140,18 @@ describe("bash_background_read tool", () => { it("should handle regex filtering", async () => { const manager = new BackgroundProcessManager(); - const executor = createTestExecutor(); + const runtime = createTestRuntime(); const tempDir = new TestTempDir("test-bash-bg-read"); const config = createTestToolConfig(process.cwd()); config.runtimeTempDir = tempDir.path; config.backgroundProcessManager = manager; - const spawnResult = await manager.spawn(executor, "test-workspace", "echo ERROR: test; echo INFO: test", { - cwd: process.cwd(), - }); + const spawnResult = await manager.spawn( + runtime, + "test-workspace", + "echo ERROR: test; echo INFO: test", + { cwd: process.cwd() } + ); if (!spawnResult.success) { throw new Error("Failed to spawn process"); @@ -175,13 +177,13 @@ describe("bash_background_read tool", () => { it("should return error for invalid regex pattern", async () => { const manager = new BackgroundProcessManager(); - const executor = createTestExecutor(); + const runtime = createTestRuntime(); const tempDir = new TestTempDir("test-bash-bg-read"); const config = createTestToolConfig(process.cwd()); config.runtimeTempDir = tempDir.path; config.backgroundProcessManager = manager; - const spawnResult = await manager.spawn(executor, "test-workspace", "echo test", { + const spawnResult = await manager.spawn(runtime, "test-workspace", "echo test", { cwd: process.cwd(), }); @@ -210,7 +212,7 @@ describe("bash_background_read tool", () => { it("should not read processes from other workspaces", async () => { const manager = new BackgroundProcessManager(); - const executorB = createTestExecutor(); + const runtime = createTestRuntime(); const tempDir = new TestTempDir("test-bash-bg-read"); // Config is for workspace-a @@ -219,7 +221,7 @@ describe("bash_background_read tool", () => { config.backgroundProcessManager = manager; // Spawn process in workspace-b - const spawnResult = await manager.spawn(executorB, "workspace-b", "echo secret", { + const spawnResult = await manager.spawn(runtime, "workspace-b", "echo secret", { cwd: process.cwd(), }); diff --git a/src/node/services/tools/bash_background_terminate.test.ts b/src/node/services/tools/bash_background_terminate.test.ts index b6d605070..06f9ae65d 100644 --- a/src/node/services/tools/bash_background_terminate.test.ts +++ b/src/node/services/tools/bash_background_terminate.test.ts @@ -1,9 +1,8 @@ import { describe, it, expect } from "bun:test"; import { createBashBackgroundTerminateTool } from "./bash_background_terminate"; import { BackgroundProcessManager } from "@/node/services/backgroundProcessManager"; -import { BashExecutionService } from "@/node/services/bashExecutionService"; -import { LocalBackgroundExecutor } from "@/node/services/localBackgroundExecutor"; -import type { BackgroundExecutor } from "@/node/services/backgroundExecutor"; +import { LocalRuntime } from "@/node/runtime/LocalRuntime"; +import type { Runtime } from "@/node/runtime/Runtime"; import type { BashBackgroundTerminateArgs, BashBackgroundTerminateResult, @@ -16,9 +15,9 @@ const mockToolCallOptions: ToolCallOptions = { messages: [], }; -// Create a test executor -function createTestExecutor(): BackgroundExecutor { - return new LocalBackgroundExecutor(new BashExecutionService()); +// Create test runtime (uses local machine) +function createTestRuntime(): Runtime { + return new LocalRuntime(process.cwd()); } describe("bash_background_terminate tool", () => { @@ -67,14 +66,14 @@ describe("bash_background_terminate tool", () => { it("should terminate a running process", async () => { const manager = new BackgroundProcessManager(); - const executor = createTestExecutor(); + const runtime = createTestRuntime(); const tempDir = new TestTempDir("test-bash-bg-term"); const config = createTestToolConfig(process.cwd()); config.runtimeTempDir = tempDir.path; config.backgroundProcessManager = manager; // Spawn a long-running process - const spawnResult = await manager.spawn(executor, "test-workspace", "sleep 10", { + const spawnResult = await manager.spawn(runtime, "test-workspace", "sleep 10", { cwd: process.cwd(), }); @@ -106,14 +105,14 @@ describe("bash_background_terminate tool", () => { it("should be idempotent (double-terminate succeeds)", async () => { const manager = new BackgroundProcessManager(); - const executor = createTestExecutor(); + const runtime = createTestRuntime(); const tempDir = new TestTempDir("test-bash-bg-term"); const config = createTestToolConfig(process.cwd()); config.runtimeTempDir = tempDir.path; config.backgroundProcessManager = manager; // Spawn a process - const spawnResult = await manager.spawn(executor, "test-workspace", "sleep 10", { + const spawnResult = await manager.spawn(runtime, "test-workspace", "sleep 10", { cwd: process.cwd(), }); @@ -145,7 +144,7 @@ describe("bash_background_terminate tool", () => { it("should not terminate processes from other workspaces", async () => { const manager = new BackgroundProcessManager(); - const executorB = createTestExecutor(); + const runtime = createTestRuntime(); const tempDir = new TestTempDir("test-bash-bg-term"); // Config is for workspace-a @@ -154,7 +153,7 @@ describe("bash_background_terminate tool", () => { config.backgroundProcessManager = manager; // Spawn process in workspace-b - const spawnResult = await manager.spawn(executorB, "workspace-b", "sleep 10", { + const spawnResult = await manager.spawn(runtime, "workspace-b", "sleep 10", { cwd: process.cwd(), }); From 8bcee44083e39448d29d0b337ff4b6eb631b1915 Mon Sep 17 00:00:00 2001 From: ethan Date: Tue, 25 Nov 2025 22:01:26 +1100 Subject: [PATCH 09/13] =?UTF-8?q?=F0=9F=A4=96=20refactor:=20gate=20backgro?= =?UTF-8?q?und=20tools=20and=20extract=20LocalBackgroundHandle?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Only include bash_background_* tools when backgroundProcessManager available (fixes tools being exposed in CLI/debug paths where they can't work) - Extract LocalBackgroundHandle to separate file for better organization --- src/common/utils/tools/tools.ts | 11 +- src/node/runtime/LocalBackgroundHandle.ts | 124 ++++++++++++++++++++++ src/node/runtime/LocalRuntime.ts | 124 +--------------------- 3 files changed, 133 insertions(+), 126 deletions(-) create mode 100644 src/node/runtime/LocalBackgroundHandle.ts diff --git a/src/common/utils/tools/tools.ts b/src/common/utils/tools/tools.ts index 3d6b11949..e8bf83624 100644 --- a/src/common/utils/tools/tools.ts +++ b/src/common/utils/tools/tools.ts @@ -107,12 +107,17 @@ export async function getToolsForModel( // and line number miscalculations. Use file_edit_replace_string instead. // file_edit_replace_lines: wrap(createFileEditReplaceLinesTool(config)), bash: wrap(createBashTool(config)), - bash_background_read: wrap(createBashBackgroundReadTool(config)), - bash_background_list: wrap(createBashBackgroundListTool(config)), - bash_background_terminate: wrap(createBashBackgroundTerminateTool(config)), web_fetch: wrap(createWebFetchTool(config)), }; + // Only include background tools when manager is available + // (not available in CLI/debug paths) + if (config.backgroundProcessManager) { + runtimeTools.bash_background_read = wrap(createBashBackgroundReadTool(config)); + runtimeTools.bash_background_list = wrap(createBashBackgroundListTool(config)); + runtimeTools.bash_background_terminate = wrap(createBashBackgroundTerminateTool(config)); + } + // Non-runtime tools execute immediately (no init wait needed) const nonRuntimeTools: Record = { propose_plan: createProposePlanTool(config), diff --git a/src/node/runtime/LocalBackgroundHandle.ts b/src/node/runtime/LocalBackgroundHandle.ts new file mode 100644 index 000000000..865f17258 --- /dev/null +++ b/src/node/runtime/LocalBackgroundHandle.ts @@ -0,0 +1,124 @@ +import type { BackgroundHandle } from "./Runtime"; +import type { DisposableProcess } from "@/node/utils/disposableExec"; +import { log } from "@/node/services/log"; + +/** + * Handle to a local background process. + * + * Buffers early events until callbacks are registered, since the manager + * registers callbacks after spawn() returns (but output may arrive before). + */ +export class LocalBackgroundHandle implements BackgroundHandle { + private stdoutCallback?: (line: string) => void; + private stderrCallback?: (line: string) => void; + private exitCallback?: (exitCode: number) => void; + private terminated = false; + + // Buffers for events that arrive before callbacks are registered + private pendingStdout: string[] = []; + private pendingStderr: string[] = []; + private pendingExitCode?: number; + + constructor(private readonly disposable: DisposableProcess) {} + + onStdout(callback: (line: string) => void): void { + this.stdoutCallback = callback; + // Flush buffered events + for (const line of this.pendingStdout) { + callback(line); + } + this.pendingStdout = []; + } + + onStderr(callback: (line: string) => void): void { + this.stderrCallback = callback; + // Flush buffered events + for (const line of this.pendingStderr) { + callback(line); + } + this.pendingStderr = []; + } + + onExit(callback: (exitCode: number) => void): void { + this.exitCallback = callback; + // Flush buffered event + if (this.pendingExitCode !== undefined) { + callback(this.pendingExitCode); + this.pendingExitCode = undefined; + } + } + + /** Internal: called when stdout line arrives */ + _emitStdout(line: string): void { + if (this.stdoutCallback) { + this.stdoutCallback(line); + } else { + this.pendingStdout.push(line); + } + } + + /** Internal: called when stderr line arrives */ + _emitStderr(line: string): void { + if (this.stderrCallback) { + this.stderrCallback(line); + } else { + this.pendingStderr.push(line); + } + } + + /** Internal: called when process exits */ + _emitExit(exitCode: number): void { + if (this.exitCallback) { + this.exitCallback(exitCode); + } else { + this.pendingExitCode = exitCode; + } + } + + isRunning(): Promise { + return Promise.resolve(this.disposable.underlying.exitCode === null); + } + + async terminate(): Promise { + if (this.terminated) return; + + const pid = this.disposable.underlying.pid; + if (pid === undefined) { + this.terminated = true; + return; + } + + try { + // Send SIGTERM to the process group for graceful shutdown + // Use negative PID to kill the entire process group (detached processes are group leaders) + const pgid = -pid; + log.debug(`LocalBackgroundHandle: Sending SIGTERM to process group (PGID: ${pgid})`); + process.kill(pgid, "SIGTERM"); + + // Wait 2 seconds for graceful shutdown + await new Promise((resolve) => setTimeout(resolve, 2000)); + + // Check if process is still running + if (await this.isRunning()) { + log.debug(`LocalBackgroundHandle: Process still running, sending SIGKILL`); + process.kill(pgid, "SIGKILL"); + } + } catch (error) { + // Process may already be dead - that's fine + log.debug( + `LocalBackgroundHandle: Error during terminate: ${error instanceof Error ? error.message : String(error)}` + ); + } + + this.terminated = true; + } + + dispose(): Promise { + return Promise.resolve(this.disposable[Symbol.dispose]()); + } + + /** Get the underlying child process (for spawn event waiting) */ + get child() { + return this.disposable.underlying; + } +} diff --git a/src/node/runtime/LocalRuntime.ts b/src/node/runtime/LocalRuntime.ts index c168fa0cd..1cbbc7bf1 100644 --- a/src/node/runtime/LocalRuntime.ts +++ b/src/node/runtime/LocalRuntime.ts @@ -17,8 +17,8 @@ import type { InitLogger, BackgroundSpawnOptions, BackgroundSpawnResult, - BackgroundHandle, } from "./Runtime"; +import { LocalBackgroundHandle } from "./LocalBackgroundHandle"; import { RuntimeError as RuntimeErrorClass } from "./Runtime"; import { NON_INTERACTIVE_ENV_VARS } from "@/common/constants/env"; import { getBashPath } from "@/node/utils/main/bashPath"; @@ -35,128 +35,6 @@ import { getProjectName } from "@/node/utils/runtime/helpers"; import { getErrorMessage } from "@/common/utils/errors"; import { once } from "node:events"; import { log } from "@/node/services/log"; - -/** - * Handle to a local background process. - * - * Buffers early events until callbacks are registered, since the manager - * registers callbacks after spawn() returns (but output may arrive before). - */ -class LocalBackgroundHandle implements BackgroundHandle { - private stdoutCallback?: (line: string) => void; - private stderrCallback?: (line: string) => void; - private exitCallback?: (exitCode: number) => void; - private terminated = false; - - // Buffers for events that arrive before callbacks are registered - private pendingStdout: string[] = []; - private pendingStderr: string[] = []; - private pendingExitCode?: number; - - constructor(private readonly disposable: DisposableProcess) {} - - onStdout(callback: (line: string) => void): void { - this.stdoutCallback = callback; - // Flush buffered events - for (const line of this.pendingStdout) { - callback(line); - } - this.pendingStdout = []; - } - - onStderr(callback: (line: string) => void): void { - this.stderrCallback = callback; - // Flush buffered events - for (const line of this.pendingStderr) { - callback(line); - } - this.pendingStderr = []; - } - - onExit(callback: (exitCode: number) => void): void { - this.exitCallback = callback; - // Flush buffered event - if (this.pendingExitCode !== undefined) { - callback(this.pendingExitCode); - this.pendingExitCode = undefined; - } - } - - /** Internal: called when stdout line arrives */ - _emitStdout(line: string): void { - if (this.stdoutCallback) { - this.stdoutCallback(line); - } else { - this.pendingStdout.push(line); - } - } - - /** Internal: called when stderr line arrives */ - _emitStderr(line: string): void { - if (this.stderrCallback) { - this.stderrCallback(line); - } else { - this.pendingStderr.push(line); - } - } - - /** Internal: called when process exits */ - _emitExit(exitCode: number): void { - if (this.exitCallback) { - this.exitCallback(exitCode); - } else { - this.pendingExitCode = exitCode; - } - } - - isRunning(): Promise { - return Promise.resolve(this.disposable.underlying.exitCode === null); - } - - async terminate(): Promise { - if (this.terminated) return; - - const pid = this.disposable.underlying.pid; - if (pid === undefined) { - this.terminated = true; - return; - } - - try { - // Send SIGTERM to the process group for graceful shutdown - // Use negative PID to kill the entire process group (detached processes are group leaders) - const pgid = -pid; - log.debug(`LocalBackgroundHandle: Sending SIGTERM to process group (PGID: ${pgid})`); - process.kill(pgid, "SIGTERM"); - - // Wait 2 seconds for graceful shutdown - await new Promise((resolve) => setTimeout(resolve, 2000)); - - // Check if process is still running - if (await this.isRunning()) { - log.debug(`LocalBackgroundHandle: Process still running, sending SIGKILL`); - process.kill(pgid, "SIGKILL"); - } - } catch (error) { - // Process may already be dead - that's fine - log.debug( - `LocalBackgroundHandle: Error during terminate: ${error instanceof Error ? error.message : String(error)}` - ); - } - - this.terminated = true; - } - - dispose(): Promise { - return Promise.resolve(this.disposable[Symbol.dispose]()); - } - - /** Get the underlying child process (for spawn event waiting) */ - get child() { - return this.disposable.underlying; - } -} - import { expandTilde } from "./tildeExpansion"; /** From d39f4050edfd8936cbbf3aea410d193adbc08a85 Mon Sep 17 00:00:00 2001 From: ethan Date: Tue, 25 Nov 2025 22:10:07 +1100 Subject: [PATCH 10/13] =?UTF-8?q?=F0=9F=A4=96=20fix:=20report=20correct=20?= =?UTF-8?q?exit=20codes=20for=20signal-terminated=20background=20processes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a background process is killed by signal (SIGTERM, SIGKILL, etc.), Node's exit event provides code=null and signal name. The spawnBackground handler was defaulting null to 0, making killed processes appear as successful exits. - Add getExitCode() helper to convert signals to Unix-conventional exit codes (128 + signal_number): SIGTERM=143, SIGKILL=137, etc. - Add test verifying terminated processes report exit code >= 128 This restores behavior that was lost when bashExecutionService.ts was removed during the Runtime refactor. --- src/node/runtime/LocalRuntime.ts | 25 +++++++++++++++++-- .../services/backgroundProcessManager.test.ts | 21 ++++++++++++++++ 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/src/node/runtime/LocalRuntime.ts b/src/node/runtime/LocalRuntime.ts index 1cbbc7bf1..504bb6551 100644 --- a/src/node/runtime/LocalRuntime.ts +++ b/src/node/runtime/LocalRuntime.ts @@ -35,6 +35,27 @@ import { getProjectName } from "@/node/utils/runtime/helpers"; import { getErrorMessage } from "@/common/utils/errors"; import { once } from "node:events"; import { log } from "@/node/services/log"; + +/** + * Convert Node.js exit event args to a single exit code. + * When a process is killed by signal, Node provides code=null and signal name. + * This converts to Unix-conventional exit codes (128 + signal_number). + */ +function getExitCode(code: number | null, signal: NodeJS.Signals | null): number { + if (code !== null) return code; + if (signal) { + const signalNumbers: Record = { + SIGHUP: 1, + SIGINT: 2, + SIGQUIT: 3, + SIGKILL: 9, + SIGTERM: 15, + }; + return 128 + (signalNumbers[signal] ?? 0); + } + return 0; +} + import { expandTilde } from "./tildeExpansion"; /** @@ -246,11 +267,11 @@ export class LocalRuntime implements Runtime { } }); - childProcess.on("exit", (code) => { + childProcess.on("exit", (code, signal) => { // Flush remaining partial lines if (stdoutBuffer) handle._emitStdout(stdoutBuffer); if (stderrBuffer) handle._emitStderr(stderrBuffer); - handle._emitExit(code ?? 0); + handle._emitExit(getExitCode(code, signal)); }); handle = new LocalBackgroundHandle(disposable); diff --git a/src/node/services/backgroundProcessManager.test.ts b/src/node/services/backgroundProcessManager.test.ts index f6027f6a8..7dbf25a50 100644 --- a/src/node/services/backgroundProcessManager.test.ts +++ b/src/node/services/backgroundProcessManager.test.ts @@ -224,5 +224,26 @@ describe("BackgroundProcessManager", () => { expect(proc?.status).toBe("killed"); } }); + + it("should report non-zero exit code for signal-terminated processes", async () => { + // Spawn a long-running process + const result = await manager.spawn(runtime, testWorkspaceId, "sleep 60", { + cwd: process.cwd(), + }); + + if (result.success) { + // Terminate it (sends SIGTERM, then SIGKILL after 2s) + await manager.terminate(result.processId); + + // Wait for exit to be recorded + await new Promise((resolve) => setTimeout(resolve, 100)); + + const proc = manager.getProcess(result.processId); + expect(proc).not.toBeNull(); + // Exit code should be 128 + signal number (SIGTERM=15 → 143, SIGKILL=9 → 137) + // Either is acceptable depending on timing + expect(proc!.exitCode).toBeGreaterThanOrEqual(128); + } + }); }); }); From 51dfcfdbea105d517f379873d148e389ea19a5e6 Mon Sep 17 00:00:00 2001 From: ethan Date: Tue, 25 Nov 2025 22:28:06 +1100 Subject: [PATCH 11/13] =?UTF-8?q?=F0=9F=A4=96=20fix:=20correctly=20detect?= =?UTF-8?q?=20signal-killed=20processes=20in=20isRunning()?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Check both exitCode and signalCode when determining if process is alive - Fixes terminate() always sending SIGKILL after SIGTERM, even when process already stopped (signal-killed processes have signalCode set but exitCode null) - Also gate background tools for SSH runtimes (TODO: replace duck-type check with proper capability flag once SSH implements spawnBackground) --- src/common/utils/tools/tools.ts | 13 +++++-------- src/node/runtime/LocalBackgroundHandle.ts | 5 ++++- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/common/utils/tools/tools.ts b/src/common/utils/tools/tools.ts index e8bf83624..013120470 100644 --- a/src/common/utils/tools/tools.ts +++ b/src/common/utils/tools/tools.ts @@ -107,17 +107,14 @@ export async function getToolsForModel( // and line number miscalculations. Use file_edit_replace_string instead. // file_edit_replace_lines: wrap(createFileEditReplaceLinesTool(config)), bash: wrap(createBashTool(config)), + // TODO: These aren't supported by the SSH runtime yet, but they will be, + // so always add them. + bash_background_read: wrap(createBashBackgroundReadTool(config)), + bash_background_list: wrap(createBashBackgroundListTool(config)), + bash_background_terminate: wrap(createBashBackgroundTerminateTool(config)), web_fetch: wrap(createWebFetchTool(config)), }; - // Only include background tools when manager is available - // (not available in CLI/debug paths) - if (config.backgroundProcessManager) { - runtimeTools.bash_background_read = wrap(createBashBackgroundReadTool(config)); - runtimeTools.bash_background_list = wrap(createBashBackgroundListTool(config)); - runtimeTools.bash_background_terminate = wrap(createBashBackgroundTerminateTool(config)); - } - // Non-runtime tools execute immediately (no init wait needed) const nonRuntimeTools: Record = { propose_plan: createProposePlanTool(config), diff --git a/src/node/runtime/LocalBackgroundHandle.ts b/src/node/runtime/LocalBackgroundHandle.ts index 865f17258..dae85bee9 100644 --- a/src/node/runtime/LocalBackgroundHandle.ts +++ b/src/node/runtime/LocalBackgroundHandle.ts @@ -76,7 +76,10 @@ export class LocalBackgroundHandle implements BackgroundHandle { } isRunning(): Promise { - return Promise.resolve(this.disposable.underlying.exitCode === null); + const child = this.disposable.underlying; + // Process is dead if either exitCode or signalCode is set + // (signal-killed processes have signalCode set but exitCode remains null) + return Promise.resolve(child.exitCode === null && child.signalCode === null); } async terminate(): Promise { From c5571f9e009df7e24b9d870e24dbd1cb9468c833 Mon Sep 17 00:00:00 2001 From: ethan Date: Wed, 26 Nov 2025 13:24:40 +1100 Subject: [PATCH 12/13] fix: use separate TextDecoders per stream in background process A single TextDecoder with stream:true buffers partial multibyte sequences. When stdout and stderr interleave, shared state corrupts output by combining bytes from different streams. Use independent decoders and flush on exit. --- src/node/runtime/LocalRuntime.ts | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/node/runtime/LocalRuntime.ts b/src/node/runtime/LocalRuntime.ts index 504bb6551..d299e7952 100644 --- a/src/node/runtime/LocalRuntime.ts +++ b/src/node/runtime/LocalRuntime.ts @@ -245,12 +245,15 @@ export class LocalRuntime implements Runtime { let handle: LocalBackgroundHandle; // Set up line-buffered output streaming - const decoder = new TextDecoder(); + // Use separate decoders per stream - TextDecoder with stream:true buffers + // partial multibyte sequences, so sharing would corrupt output when chunks interleave + const stdoutDecoder = new TextDecoder(); + const stderrDecoder = new TextDecoder(); let stdoutBuffer = ""; let stderrBuffer = ""; childProcess.stdout.on("data", (chunk: Buffer) => { - stdoutBuffer += decoder.decode(chunk, { stream: true }); + stdoutBuffer += stdoutDecoder.decode(chunk, { stream: true }); const lines = stdoutBuffer.split("\n"); stdoutBuffer = lines.pop() ?? ""; for (const line of lines) { @@ -259,7 +262,7 @@ export class LocalRuntime implements Runtime { }); childProcess.stderr.on("data", (chunk: Buffer) => { - stderrBuffer += decoder.decode(chunk, { stream: true }); + stderrBuffer += stderrDecoder.decode(chunk, { stream: true }); const lines = stderrBuffer.split("\n"); stderrBuffer = lines.pop() ?? ""; for (const line of lines) { @@ -268,6 +271,9 @@ export class LocalRuntime implements Runtime { }); childProcess.on("exit", (code, signal) => { + // Flush decoder buffers (emits any incomplete multibyte sequences) + stdoutBuffer += stdoutDecoder.decode(); + stderrBuffer += stderrDecoder.decode(); // Flush remaining partial lines if (stdoutBuffer) handle._emitStdout(stdoutBuffer); if (stderrBuffer) handle._emitStderr(stderrBuffer); From a57049c3f557a0179cb7f70fc2bbaabd82199481 Mon Sep 17 00:00:00 2001 From: ethan Date: Wed, 26 Nov 2025 18:22:22 +1100 Subject: [PATCH 13/13] fix: terminate background processes on app quit Adds terminateAll() to BackgroundProcessManager and hooks it into Electron's before-quit event, preventing orphaned processes when mux exits normally. Crash recovery (orphans from SIGKILL) remains out of scope. --- src/desktop/main.ts | 12 ++++++++++++ src/node/services/backgroundProcessManager.ts | 12 ++++++++++++ src/node/services/ipcMain.ts | 7 +++++++ 3 files changed, 31 insertions(+) diff --git a/src/desktop/main.ts b/src/desktop/main.ts index 4eb8f39d9..3f87060d0 100644 --- a/src/desktop/main.ts +++ b/src/desktop/main.ts @@ -540,6 +540,18 @@ if (gotTheLock) { } }); + // Track if we're already quitting to avoid re-entrancy + let isQuitting = false; + app.on("before-quit", (event) => { + if (ipcMain && !isQuitting) { + isQuitting = true; + event.preventDefault(); + ipcMain.terminateAllBackgroundProcesses().finally(() => { + app.exit(0); + }); + } + }); + app.on("activate", () => { // Only create window if app is ready and no window exists // This prevents "Cannot create BrowserWindow before app is ready" error diff --git a/src/node/services/backgroundProcessManager.ts b/src/node/services/backgroundProcessManager.ts index 87b7d63c8..ff1cbc071 100644 --- a/src/node/services/backgroundProcessManager.ts +++ b/src/node/services/backgroundProcessManager.ts @@ -194,4 +194,16 @@ export class BackgroundProcessManager { log.debug(`Cleaned up ${matching.length} process(es) for workspace ${workspaceId}`); } + + /** + * Terminate all background processes across all workspaces. + * Called on app shutdown to prevent orphaned processes. + */ + async terminateAll(): Promise { + log.debug(`BackgroundProcessManager.terminateAll() called`); + const allProcesses = Array.from(this.processes.values()); + await Promise.all(allProcesses.map((p) => this.terminate(p.id))); + this.processes.clear(); + log.debug(`Terminated ${allProcesses.length} background process(es)`); + } } diff --git a/src/node/services/ipcMain.ts b/src/node/services/ipcMain.ts index bea012818..e4df00e8f 100644 --- a/src/node/services/ipcMain.ts +++ b/src/node/services/ipcMain.ts @@ -112,6 +112,13 @@ export class IpcMain { await this.extensionMetadata.initialize(); } + /** + * Terminate all background processes. Called on app shutdown. + */ + async terminateAllBackgroundProcesses(): Promise { + await this.backgroundProcessManager.terminateAll(); + } + /** * Configure a picker used to select project directories (desktop mode only). * Server mode does not provide a native directory picker.