Skip to content

Commit 8bcee44

Browse files
ethanndicksonethanndickson
committed
🤖 refactor: gate background tools and extract LocalBackgroundHandle
- 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
1 parent 8a7d8f7 commit 8bcee44

File tree

3 files changed

+133
-126
lines changed

3 files changed

+133
-126
lines changed

‎src/common/utils/tools/tools.ts‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -107,12 +107,17 @@ export async function getToolsForModel(
107107
// and line number miscalculations. Use file_edit_replace_string instead.
108108
// file_edit_replace_lines: wrap(createFileEditReplaceLinesTool(config)),
109109
bash: wrap(createBashTool(config)),
110-
bash_background_read: wrap(createBashBackgroundReadTool(config)),
111-
bash_background_list: wrap(createBashBackgroundListTool(config)),
112-
bash_background_terminate: wrap(createBashBackgroundTerminateTool(config)),
113110
web_fetch: wrap(createWebFetchTool(config)),
114111
};
115112

113+
// Only include background tools when manager is available
114+
// (not available in CLI/debug paths)
115+
if (config.backgroundProcessManager) {
116+
runtimeTools.bash_background_read = wrap(createBashBackgroundReadTool(config));
117+
runtimeTools.bash_background_list = wrap(createBashBackgroundListTool(config));
118+
runtimeTools.bash_background_terminate = wrap(createBashBackgroundTerminateTool(config));
119+
}
120+
116121
// Non-runtime tools execute immediately (no init wait needed)
117122
const nonRuntimeTools: Record<string, Tool> = {
118123
propose_plan: createProposePlanTool(config),

‎src/node/runtime/LocalBackgroundHandle.ts‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
import type { BackgroundHandle } from "./Runtime";
2+
import type { DisposableProcess } from "@/node/utils/disposableExec";
3+
import { log } from "@/node/services/log";
4+
5+
/**
6+
* Handle to a local background process.
7+
*
8+
* Buffers early events until callbacks are registered, since the manager
9+
* registers callbacks after spawn() returns (but output may arrive before).
10+
*/
11+
export class LocalBackgroundHandle implements BackgroundHandle {
12+
private stdoutCallback?: (line: string) => void;
13+
private stderrCallback?: (line: string) => void;
14+
private exitCallback?: (exitCode: number) => void;
15+
private terminated = false;
16+
17+
// Buffers for events that arrive before callbacks are registered
18+
private pendingStdout: string[] = [];
19+
private pendingStderr: string[] = [];
20+
private pendingExitCode?: number;
21+
22+
constructor(private readonly disposable: DisposableProcess) {}
23+
24+
onStdout(callback: (line: string) => void): void {
25+
this.stdoutCallback = callback;
26+
// Flush buffered events
27+
for (const line of this.pendingStdout) {
28+
callback(line);
29+
}
30+
this.pendingStdout = [];
31+
}
32+
33+
onStderr(callback: (line: string) => void): void {
34+
this.stderrCallback = callback;
35+
// Flush buffered events
36+
for (const line of this.pendingStderr) {
37+
callback(line);
38+
}
39+
this.pendingStderr = [];
40+
}
41+
42+
onExit(callback: (exitCode: number) => void): void {
43+
this.exitCallback = callback;
44+
// Flush buffered event
45+
if (this.pendingExitCode !== undefined) {
46+
callback(this.pendingExitCode);
47+
this.pendingExitCode = undefined;
48+
}
49+
}
50+
51+
/** Internal: called when stdout line arrives */
52+
_emitStdout(line: string): void {
53+
if (this.stdoutCallback) {
54+
this.stdoutCallback(line);
55+
} else {
56+
this.pendingStdout.push(line);
57+
}
58+
}
59+
60+
/** Internal: called when stderr line arrives */
61+
_emitStderr(line: string): void {
62+
if (this.stderrCallback) {
63+
this.stderrCallback(line);
64+
} else {
65+
this.pendingStderr.push(line);
66+
}
67+
}
68+
69+
/** Internal: called when process exits */
70+
_emitExit(exitCode: number): void {
71+
if (this.exitCallback) {
72+
this.exitCallback(exitCode);
73+
} else {
74+
this.pendingExitCode = exitCode;
75+
}
76+
}
77+
78+
isRunning(): Promise<boolean> {
79+
return Promise.resolve(this.disposable.underlying.exitCode === null);
80+
}
81+
82+
async terminate(): Promise<void> {
83+
if (this.terminated) return;
84+
85+
const pid = this.disposable.underlying.pid;
86+
if (pid === undefined) {
87+
this.terminated = true;
88+
return;
89+
}
90+
91+
try {
92+
// Send SIGTERM to the process group for graceful shutdown
93+
// Use negative PID to kill the entire process group (detached processes are group leaders)
94+
const pgid = -pid;
95+
log.debug(`LocalBackgroundHandle: Sending SIGTERM to process group (PGID: ${pgid})`);
96+
process.kill(pgid, "SIGTERM");
97+
98+
// Wait 2 seconds for graceful shutdown
99+
await new Promise((resolve) => setTimeout(resolve, 2000));
100+
101+
// Check if process is still running
102+
if (await this.isRunning()) {
103+
log.debug(`LocalBackgroundHandle: Process still running, sending SIGKILL`);
104+
process.kill(pgid, "SIGKILL");
105+
}
106+
} catch (error) {
107+
// Process may already be dead - that's fine
108+
log.debug(
109+
`LocalBackgroundHandle: Error during terminate: ${error instanceof Error ? error.message : String(error)}`
110+
);
111+
}
112+
113+
this.terminated = true;
114+
}
115+
116+
dispose(): Promise<void> {
117+
return Promise.resolve(this.disposable[Symbol.dispose]());
118+
}
119+
120+
/** Get the underlying child process (for spawn event waiting) */
121+
get child() {
122+
return this.disposable.underlying;
123+
}
124+
}

‎src/node/runtime/LocalRuntime.ts‎

Lines changed: 1 addition & 123 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ import type {
1717
InitLogger,
1818
BackgroundSpawnOptions,
1919
BackgroundSpawnResult,
20-
BackgroundHandle,
2120
} from "./Runtime";
21+
import { LocalBackgroundHandle } from "./LocalBackgroundHandle";
2222
import { RuntimeError as RuntimeErrorClass } from "./Runtime";
2323
import { NON_INTERACTIVE_ENV_VARS } from "@/common/constants/env";
2424
import { getBashPath } from "@/node/utils/main/bashPath";
@@ -35,128 +35,6 @@ import { getProjectName } from "@/node/utils/runtime/helpers";
3535
import { getErrorMessage } from "@/common/utils/errors";
3636
import { once } from "node:events";
3737
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-
16038
import { expandTilde } from "./tildeExpansion";
16139

16240
/**

0 commit comments

Comments
 (0)