Skip to content

Commit 77db22a

Browse files
phodalQoder-AI
andcommitted
fix(acp): reap the agent process before a run resolves
The ACP executor spawned the Agent with the run workspace as its cwd and called kill() without awaiting exit, so execute() resolved while the child was still alive. On Windows a live process' cwd is an open directory handle, so the harness-studio server test failed with EBUSY: rmdir on its temp workspace. POSIX permits that removal, which is why only windows-latest broke. reapAgent now awaits the child's exit with a bounded grace period and escalates to SIGKILL so a wedged Agent cannot hang a run instead. A new executor test asserts the process has exited and its cwd is removable once execute() resolves; it fails against the previous kill-and-forget path. Validated with packages/harness Vitest (168 tests), tsc --noEmit, and harness-studio test/server.test.ts (43 tests) on macOS; the windows-latest job is the authoritative receipt for the platform behavior. Co-authored-by: QoderAI <qoder_ai@qoder.com>
1 parent bc98827 commit 77db22a

2 files changed

Lines changed: 65 additions & 4 deletions

File tree

packages/harness/src/exec/acp-sdk.ts

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import { redactTraceValue } from "./qoder-sdk.js";
2525
const ACP_SDK_MODULE = "@agentclientprotocol/sdk";
2626
const MAX_PROTOCOL_EVENTS = 2_000;
2727
const MAX_PROTOCOL_PAYLOAD_BYTES = 65_536;
28+
const AGENT_EXIT_GRACE_MS = 2_000;
2829

2930
type AcpSdk = typeof import("@agentclientprotocol/sdk");
3031

@@ -253,13 +254,35 @@ export class AcpSdkExecutor implements HarnessExecutor {
253254
},
254255
};
255256
} finally {
256-
if (child !== undefined && child.exitCode === null && child.signalCode === null) {
257-
child.kill();
258-
}
257+
await reapAgent(child);
259258
}
260259
}
261260
}
262261

262+
/**
263+
* Terminate the Agent and wait for its exit before the run resolves. A live
264+
* child holds an open handle on its cwd, so a caller that removes the run
265+
* workspace right after `execute` fails with EBUSY on Windows.
266+
*/
267+
async function reapAgent(child: ChildProcessWithoutNullStreams | undefined): Promise<void> {
268+
if (child === undefined || child.exitCode !== null || child.signalCode !== null) return;
269+
const exited = new Promise<void>((resolvePromise) => {
270+
child.once("exit", () => resolvePromise());
271+
child.once("error", () => resolvePromise());
272+
});
273+
child.kill();
274+
await Promise.race([exited, delay(AGENT_EXIT_GRACE_MS)]);
275+
if (child.exitCode !== null || child.signalCode !== null) return;
276+
child.kill("SIGKILL");
277+
await Promise.race([exited, delay(AGENT_EXIT_GRACE_MS)]);
278+
}
279+
280+
function delay(ms: number): Promise<void> {
281+
return new Promise((resolvePromise) => {
282+
setTimeout(resolvePromise, ms).unref();
283+
});
284+
}
285+
263286
async function requestWithAbort<T>(
264287
request: () => Promise<T>,
265288
signal: AbortSignal | undefined,

packages/harness/test/acp-sdk.test.ts

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
1-
import { dirname, resolve } from "node:path";
1+
import { spawn, type ChildProcess } from "node:child_process";
2+
import { mkdtemp, rm } from "node:fs/promises";
3+
import { tmpdir } from "node:os";
4+
import { dirname, join, resolve } from "node:path";
25
import { fileURLToPath } from "node:url";
36
import { describe, expect, it } from "vitest";
47
import { compileHarness } from "../src/compiler/compile.js";
@@ -135,4 +138,39 @@ describe("AcpSdkExecutor", () => {
135138
expect(methods).toContain("$/cancel_request");
136139
expect(Date.now() - startedAt).toBeLessThan(5_000);
137140
});
141+
142+
it("reaps the Agent before resolving so the run workspace can be removed", async () => {
143+
const { bundle } = await compileHarness(SOURCE);
144+
const { revision } = resolveHarness(bundle!, "live-acp", "acp", {
145+
adapter: () => ACP_ADAPTER_DESCRIPTOR,
146+
});
147+
const workspace = await mkdtemp(join(tmpdir(), "acp-agent-cwd-"));
148+
const agents: ChildProcess[] = [];
149+
const executor = new AcpSdkExecutor({
150+
command: process.execPath,
151+
args: [FIXTURE],
152+
requestPermission: async (_requestId, request) => ({
153+
outcome: { outcome: "selected", optionId: request.options[0]!.optionId },
154+
}),
155+
spawnAgent: ((command, args, options) => {
156+
const child = spawn(command as string, args as string[], options as object);
157+
agents.push(child);
158+
return child;
159+
}) as typeof spawn,
160+
});
161+
162+
// The fixture Agent stays alive until its stdio closes, so a run that
163+
// resolves without reaping it leaves the process holding this cwd.
164+
const result = await executor.execute(revision!, bundle!, {
165+
prompt: "Prove ACP works",
166+
cwd: workspace,
167+
});
168+
169+
expect(result.exitCode).toBe(0);
170+
expect(agents).toHaveLength(1);
171+
const agentProcess = agents[0]!;
172+
expect(agentProcess.exitCode === null && agentProcess.signalCode === null).toBe(false);
173+
// Windows refuses to remove a directory that is a live process' cwd.
174+
await rm(workspace, { recursive: true });
175+
});
138176
});

0 commit comments

Comments
 (0)