fix(engine): run user commands via async exec to avoid blocking event loop
The merger's test/build verification, executor's worktreeInitCommand,
setupScript, and script-mode workflow steps all used execSync, which
blocks Node's event loop until the child process exits. A hanging
pnpm test could freeze the entire engine (no logs, heartbeats, or
other task progress) for the full 5-minute timeout.
Switch these call sites to promisify(exec) with awaited calls so the
engine keeps running while user-configured commands execute. Short
internal git plumbing (rev-parse, branch -d, worktree remove) still
uses execSync since those commands are bounded and measured in ms.
Document the rule in AGENTS.md under a new "Engine process rules"
section so future agents don't reintroduce blocking behavior.
Tests: update child_process mocks in merger.test, executor.test, and
restart.integration.test to route the new async exec through the
existing execSync mock and expose promisify.custom so destructuring
{ stdout, stderr } matches real child_process.exec semantics.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -13,9 +13,40 @@ vi.mock("./pi.js", () => ({
|
||||
compactSessionContext: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("node:child_process", () => ({
|
||||
execSync: vi.fn(),
|
||||
}));
|
||||
// Route async `exec` through the `execSync` mock so existing tests that set up
|
||||
// mockedExecSync.mockImplementation for verification commands (vitest run,
|
||||
// pnpm build, etc.) keep working unchanged. `promisify(exec)` in merger.ts
|
||||
// resolves/rejects based on the callback wired here.
|
||||
vi.mock("node:child_process", async () => {
|
||||
const { promisify } = await import("node:util");
|
||||
const execSyncFn = vi.fn();
|
||||
const execFn: any = vi.fn((cmd: any, opts: any, cb: any) => {
|
||||
const callback = typeof opts === "function" ? opts : cb;
|
||||
try {
|
||||
const out = execSyncFn(cmd, { stdio: ["pipe", "pipe", "pipe"] });
|
||||
const stdout = out === undefined ? "" : out.toString();
|
||||
if (typeof callback === "function") callback(null, stdout, "");
|
||||
} catch (err: any) {
|
||||
if (typeof callback === "function") {
|
||||
callback(err, err?.stdout?.toString?.() ?? "", err?.stderr?.toString?.() ?? "");
|
||||
}
|
||||
}
|
||||
});
|
||||
// Mirror real child_process.exec: promisify resolves to { stdout, stderr }.
|
||||
execFn[promisify.custom] = (cmd: any, opts?: any) =>
|
||||
new Promise((resolve, reject) => {
|
||||
execFn(cmd, opts, (err: any, stdout: any, stderr: any) => {
|
||||
if (err) {
|
||||
err.stdout = stdout;
|
||||
err.stderr = stderr;
|
||||
reject(err);
|
||||
} else {
|
||||
resolve({ stdout, stderr });
|
||||
}
|
||||
});
|
||||
});
|
||||
return { execSync: execSyncFn, exec: execFn };
|
||||
});
|
||||
|
||||
vi.mock("node:fs", () => ({
|
||||
existsSync: vi.fn().mockReturnValue(true),
|
||||
|
||||
Reference in New Issue
Block a user