Skip to content

Commit 8a7d8f7

Browse files
committed
🤖 refactor: move background execution to Runtime interface
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.
1 parent fbb4a9d commit 8a7d8f7

15 files changed

+384
-460
lines changed

src/common/utils/tools/tools.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import { log } from "@/node/services/log";
1616
import type { Runtime } from "@/node/runtime/Runtime";
1717
import type { InitStateManager } from "@/node/services/initStateManager";
1818
import type { BackgroundProcessManager } from "@/node/services/backgroundProcessManager";
19-
import type { BackgroundExecutor } from "@/node/services/backgroundExecutor";
2019

2120
/**
2221
* Configuration for tools that need runtime context
@@ -36,8 +35,6 @@ export interface ToolConfiguration {
3635
overflow_policy?: "truncate" | "tmpfile";
3736
/** Background process manager for bash tool (optional, AI-only) */
3837
backgroundProcessManager?: BackgroundProcessManager;
39-
/** Background executor for this workspace (optional, AI-only) */
40-
backgroundExecutor?: BackgroundExecutor;
4138
/** Workspace ID for tracking background processes (optional for token estimation) */
4239
workspaceId?: string;
4340
}

src/node/runtime/LocalRuntime.ts

Lines changed: 220 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@ import type {
1515
WorkspaceForkParams,
1616
WorkspaceForkResult,
1717
InitLogger,
18+
BackgroundSpawnOptions,
19+
BackgroundSpawnResult,
20+
BackgroundHandle,
1821
} from "./Runtime";
1922
import { RuntimeError as RuntimeErrorClass } from "./Runtime";
2023
import { NON_INTERACTIVE_ENV_VARS } from "@/common/constants/env";
@@ -30,6 +33,130 @@ import {
3033
import { execAsync, DisposableProcess } from "@/node/utils/disposableExec";
3134
import { getProjectName } from "@/node/utils/runtime/helpers";
3235
import { getErrorMessage } from "@/common/utils/errors";
36+
import { once } from "node:events";
37+
import { log } from "@/node/services/log";
38+
39+
/**
40+
* Handle to a local background process.
41+
*
42+
* Buffers early events until callbacks are registered, since the manager
43+
* registers callbacks after spawn() returns (but output may arrive before).
44+
*/
45+
class LocalBackgroundHandle implements BackgroundHandle {
46+
private stdoutCallback?: (line: string) => void;
47+
private stderrCallback?: (line: string) => void;
48+
private exitCallback?: (exitCode: number) => void;
49+
private terminated = false;
50+
51+
// Buffers for events that arrive before callbacks are registered
52+
private pendingStdout: string[] = [];
53+
private pendingStderr: string[] = [];
54+
private pendingExitCode?: number;
55+
56+
constructor(private readonly disposable: DisposableProcess) {}
57+
58+
onStdout(callback: (line: string) => void): void {
59+
this.stdoutCallback = callback;
60+
// Flush buffered events
61+
for (const line of this.pendingStdout) {
62+
callback(line);
63+
}
64+
this.pendingStdout = [];
65+
}
66+
67+
onStderr(callback: (line: string) => void): void {
68+
this.stderrCallback = callback;
69+
// Flush buffered events
70+
for (const line of this.pendingStderr) {
71+
callback(line);
72+
}
73+
this.pendingStderr = [];
74+
}
75+
76+
onExit(callback: (exitCode: number) => void): void {
77+
this.exitCallback = callback;
78+
// Flush buffered event
79+
if (this.pendingExitCode !== undefined) {
80+
callback(this.pendingExitCode);
81+
this.pendingExitCode = undefined;
82+
}
83+
}
84+
85+
/** Internal: called when stdout line arrives */
86+
_emitStdout(line: string): void {
87+
if (this.stdoutCallback) {
88+
this.stdoutCallback(line);
89+
} else {
90+
this.pendingStdout.push(line);
91+
}
92+
}
93+
94+
/** Internal: called when stderr line arrives */
95+
_emitStderr(line: string): void {
96+
if (this.stderrCallback) {
97+
this.stderrCallback(line);
98+
} else {
99+
this.pendingStderr.push(line);
100+
}
101+
}
102+
103+
/** Internal: called when process exits */
104+
_emitExit(exitCode: number): void {
105+
if (this.exitCallback) {
106+
this.exitCallback(exitCode);
107+
} else {
108+
this.pendingExitCode = exitCode;
109+
}
110+
}
111+
112+
isRunning(): Promise<boolean> {
113+
return Promise.resolve(this.disposable.underlying.exitCode === null);
114+
}
115+
116+
async terminate(): Promise<void> {
117+
if (this.terminated) return;
118+
119+
const pid = this.disposable.underlying.pid;
120+
if (pid === undefined) {
121+
this.terminated = true;
122+
return;
123+
}
124+
125+
try {
126+
// Send SIGTERM to the process group for graceful shutdown
127+
// Use negative PID to kill the entire process group (detached processes are group leaders)
128+
const pgid = -pid;
129+
log.debug(`LocalBackgroundHandle: Sending SIGTERM to process group (PGID: ${pgid})`);
130+
process.kill(pgid, "SIGTERM");
131+
132+
// Wait 2 seconds for graceful shutdown
133+
await new Promise((resolve) => setTimeout(resolve, 2000));
134+
135+
// Check if process is still running
136+
if (await this.isRunning()) {
137+
log.debug(`LocalBackgroundHandle: Process still running, sending SIGKILL`);
138+
process.kill(pgid, "SIGKILL");
139+
}
140+
} catch (error) {
141+
// Process may already be dead - that's fine
142+
log.debug(
143+
`LocalBackgroundHandle: Error during terminate: ${error instanceof Error ? error.message : String(error)}`
144+
);
145+
}
146+
147+
this.terminated = true;
148+
}
149+
150+
dispose(): Promise<void> {
151+
return Promise.resolve(this.disposable[Symbol.dispose]());
152+
}
153+
154+
/** Get the underlying child process (for spawn event waiting) */
155+
get child() {
156+
return this.disposable.underlying;
157+
}
158+
}
159+
33160
import { expandTilde } from "./tildeExpansion";
34161

35162
/**
@@ -179,6 +306,99 @@ export class LocalRuntime implements Runtime {
179306
return { stdout, stderr, stdin, exitCode, duration };
180307
}
181308

309+
async spawnBackground(
310+
script: string,
311+
options: BackgroundSpawnOptions
312+
): Promise<BackgroundSpawnResult> {
313+
log.debug(`LocalRuntime.spawnBackground: Spawning in ${options.cwd}`);
314+
315+
// Check if working directory exists
316+
try {
317+
await fsPromises.access(options.cwd);
318+
} catch {
319+
return { success: false, error: `Working directory does not exist: ${options.cwd}` };
320+
}
321+
322+
// Build command with optional niceness
323+
const isWindows = process.platform === "win32";
324+
const bashPath = getBashPath();
325+
const spawnCommand = options.niceness !== undefined && !isWindows ? "nice" : bashPath;
326+
const spawnArgs =
327+
options.niceness !== undefined && !isWindows
328+
? ["-n", options.niceness.toString(), bashPath, "-c", script]
329+
: ["-c", script];
330+
331+
const childProcess = spawn(spawnCommand, spawnArgs, {
332+
cwd: options.cwd,
333+
env: {
334+
...process.env,
335+
...(options.env ?? {}),
336+
...NON_INTERACTIVE_ENV_VARS,
337+
},
338+
stdio: ["pipe", "pipe", "pipe"],
339+
detached: true,
340+
});
341+
342+
const disposable = new DisposableProcess(childProcess);
343+
344+
// Declare handle before setting up listeners
345+
// eslint-disable-next-line prefer-const
346+
let handle: LocalBackgroundHandle;
347+
348+
// Set up line-buffered output streaming
349+
const decoder = new TextDecoder();
350+
let stdoutBuffer = "";
351+
let stderrBuffer = "";
352+
353+
childProcess.stdout.on("data", (chunk: Buffer) => {
354+
stdoutBuffer += decoder.decode(chunk, { stream: true });
355+
const lines = stdoutBuffer.split("\n");
356+
stdoutBuffer = lines.pop() ?? "";
357+
for (const line of lines) {
358+
handle._emitStdout(line);
359+
}
360+
});
361+
362+
childProcess.stderr.on("data", (chunk: Buffer) => {
363+
stderrBuffer += decoder.decode(chunk, { stream: true });
364+
const lines = stderrBuffer.split("\n");
365+
stderrBuffer = lines.pop() ?? "";
366+
for (const line of lines) {
367+
handle._emitStderr(line);
368+
}
369+
});
370+
371+
childProcess.on("exit", (code) => {
372+
// Flush remaining partial lines
373+
if (stdoutBuffer) handle._emitStdout(stdoutBuffer);
374+
if (stderrBuffer) handle._emitStderr(stderrBuffer);
375+
handle._emitExit(code ?? 0);
376+
});
377+
378+
handle = new LocalBackgroundHandle(disposable);
379+
380+
// Wait for spawn or error
381+
try {
382+
await Promise.race([
383+
once(childProcess, "spawn"),
384+
once(childProcess, "error").then(([err]) => {
385+
throw err;
386+
}),
387+
new Promise((_, reject) =>
388+
setTimeout(() => reject(new Error("Spawn did not complete in time")), 2000)
389+
),
390+
]);
391+
} catch (e) {
392+
const err = e as Error;
393+
log.debug(`LocalRuntime.spawnBackground: Failed to spawn: ${err.message}`);
394+
await handle.dispose();
395+
return { success: false, error: err.message };
396+
}
397+
398+
log.debug(`LocalRuntime.spawnBackground: Spawned with PID ${childProcess.pid ?? "unknown"}`);
399+
return { success: true, handle };
400+
}
401+
182402
readFile(filePath: string, _abortSignal?: AbortSignal): ReadableStream<Uint8Array> {
183403
// Note: _abortSignal ignored for local operations (fast, no need for cancellation)
184404
const nodeStream = fs.createReadStream(filePath);

src/node/runtime/Runtime.ts

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,64 @@ export interface ExecOptions {
6464
forcePTY?: boolean;
6565
}
6666

67+
/**
68+
* Options for spawning a background process
69+
*/
70+
export interface BackgroundSpawnOptions {
71+
/** Working directory for command execution */
72+
cwd: string;
73+
/** Environment variables to inject */
74+
env?: Record<string, string>;
75+
/** Process niceness level (-20 to 19, lower = higher priority) */
76+
niceness?: number;
77+
}
78+
79+
/**
80+
* Handle to a background process.
81+
* Abstracts away whether process is local or remote.
82+
*/
83+
export interface BackgroundHandle {
84+
/**
85+
* Register callback for stdout lines.
86+
* For local: called in real-time as output arrives.
87+
* For SSH: called when output is polled/read.
88+
*/
89+
onStdout(callback: (line: string) => void): void;
90+
91+
/**
92+
* Register callback for stderr lines.
93+
*/
94+
onStderr(callback: (line: string) => void): void;
95+
96+
/**
97+
* Register callback for process exit.
98+
* @param callback Receives exit code (128+signal for signal termination)
99+
*/
100+
onExit(callback: (exitCode: number) => void): void;
101+
102+
/**
103+
* Check if process is still running.
104+
*/
105+
isRunning(): Promise<boolean>;
106+
107+
/**
108+
* Terminate the process (SIGTERM → wait → SIGKILL).
109+
*/
110+
terminate(): Promise<void>;
111+
112+
/**
113+
* Clean up resources (called after process exits or on error).
114+
*/
115+
dispose(): Promise<void>;
116+
}
117+
118+
/**
119+
* Result of spawning a background process
120+
*/
121+
export type BackgroundSpawnResult =
122+
| { success: true; handle: BackgroundHandle }
123+
| { success: false; error: string };
124+
67125
/**
68126
* Streaming result from executing a command
69127
*/
@@ -211,6 +269,17 @@ export interface Runtime {
211269
*/
212270
exec(command: string, options: ExecOptions): Promise<ExecStream>;
213271

272+
/**
273+
* Spawn a detached background process.
274+
* Returns a handle for monitoring output and terminating the process.
275+
* Unlike exec(), background processes have no timeout and run until terminated.
276+
*
277+
* @param script Bash script to execute
278+
* @param options Execution options (cwd, env, niceness)
279+
* @returns BackgroundHandle on success, or error
280+
*/
281+
spawnBackground(script: string, options: BackgroundSpawnOptions): Promise<BackgroundSpawnResult>;
282+
214283
/**
215284
* Read file contents as a stream
216285
* @param path Absolute or relative path to file

src/node/runtime/SSHRuntime.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ import type {
1313
WorkspaceForkParams,
1414
WorkspaceForkResult,
1515
InitLogger,
16+
BackgroundSpawnOptions,
17+
BackgroundSpawnResult,
1618
} from "./Runtime";
1719
import { RuntimeError as RuntimeErrorClass } from "./Runtime";
1820
import { EXIT_CODE_ABORTED, EXIT_CODE_TIMEOUT } from "@/common/constants/exitCodes";
@@ -247,6 +249,18 @@ export class SSHRuntime implements Runtime {
247249
return { stdout, stderr, stdin, exitCode, duration };
248250
}
249251

252+
spawnBackground(
253+
_script: string,
254+
_options: BackgroundSpawnOptions
255+
): Promise<BackgroundSpawnResult> {
256+
// SSH background execution not yet implemented
257+
// Would require: remote process management, output polling, etc.
258+
return Promise.resolve({
259+
success: false,
260+
error: "Background process execution is not supported for SSH workspaces",
261+
});
262+
}
263+
250264
/**
251265
* Read file contents over SSH as a stream
252266
*/

0 commit comments

Comments
 (0)