From b79c1a09bf0bd9f1933d8f807dded7dd5a9b831b Mon Sep 17 00:00:00 2001 From: dbarr5 Date: Wed, 19 Aug 2026 10:08:58 -0400 Subject: [PATCH] fix(runtime): async tool execution with process-tree teardown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit run_shell and run_tests used spawnSync with a timeout. Two defects the caller could not see: * a timeout signals the DIRECT child, which is the shell. Whatever the user actually started — npm test, pytest, a compiler — was orphaned and kept running, holding ports, files and CPU, while the call returned looking like a clean timeout. On Windows the cmd.exe shell made it near-certain. * the event loop was blocked for the duration (up to 15 minutes), freezing heartbeats, the renderer and any AbortController. That is why Ctrl+C could not interrupt a long test run: nothing was listening. `run` is now asynchronous and tree-aware. POSIX spawns into a new process group and kills the group, escalating SIGTERM to SIGKILL after 2s so a runner that traps SIGTERM cannot outlive its timeout. Windows has no equivalent grouping, so `taskkill /T /F` walks the tree instead. Timeout and cancellation resolve distinctly — 124 and 130 — because one is the clock and one is the operator, and the caller needs to tell them apart. Output is drained past the retention cap rather than truncated at the pipe, so a large-output command can never block on a full buffer, and results settle on 'close' rather than 'exit' so a test summary arriving with the exit is not lost. run_shell and run_tests move from the synchronous execute() to executeAsync, alongside the web tools that were already there. The consequential part is verify_gate. finalVerify is the gate that stops a brain claiming success on a red tree, and it called the synchronous path. Left alone it would have received "[tool run_tests is async]" in production while its tests kept passing against a fake — a silent break with green CI. So VerifyRunner now names executeAsync rather than execute. That is deliberate: a fake still implementing the sync method fails to compile instead of quietly diverging from what production calls. All 14 verify_gate tests were updated by adding await and renaming the fake's method — no assertion changed. Tests: 4 new, asserting survival by PID rather than a prompt return. A parent that exits while its grandchild keeps running looks identical to success from the caller's side, which is how this went unnoticed. a timed-out command kills its whole tree, not just the shell an aborted command kills its tree and reports aborted, not timed out a normal command still returns its real exit code and output the event loop keeps running while a command is in flight The last one distinguishes async execution from a merely faster synchronous one: it fails if a timer cannot tick during the call. Mutation-checked on the invariant that mattered: letting done.ok upgrade a red run fails "brain done.ok=true while host tests are RED → never ok" and the 24-bug corpus test (12 pass / 2 fail). Restored, 14 / 14. The ground-truth gate survived the conversion. Gates at this commit: npm run typecheck exit 0 npm test 1094 pass / 0 fail --- src/commands/code.ts | 7 +- src/core/tool_executor.ts | 179 +++++++++++++++++++++++++++++-------- src/core/verify_gate.ts | 12 ++- test/bridge.test.ts | 4 +- test/process_tree.test.ts | 133 +++++++++++++++++++++++++++ test/tool_executor.test.ts | 18 ++-- test/verify_gate.test.ts | 54 +++++------ 7 files changed, 326 insertions(+), 81 deletions(-) create mode 100644 test/process_tree.test.ts diff --git a/src/commands/code.ts b/src/commands/code.ts index 4852c10..f937796 100644 --- a/src/commands/code.ts +++ b/src/commands/code.ts @@ -430,7 +430,12 @@ export async function cmdCode(ctx: AppContext, task: string, opts: CodeOpts): Pr // The host re-runs the test command ITSELF and derives finalStatus from the real // exit code (verify_gate.ts). The brain's `done` is advisory — it only enriches a // red result with its breaker reason and can never upgrade a red run to "ok". - const { status: finalStatus, remaining, exitCode: verifyExit } = finalVerify(exec, opts.testCmd, lastDone, sawError); + const { status: finalStatus, remaining, exitCode: verifyExit } = await finalVerify( + exec, + opts.testCmd, + lastDone, + sawError, + ); log?.close(finalStatus, nowIso(), remaining); // The verdict line — printed even with --no-log (which used to end with // NOTHING); suppressed under --json (frames already carry the data). Surfaces diff --git a/src/core/tool_executor.ts b/src/core/tool_executor.ts index 88ae03e..6c4a670 100644 --- a/src/core/tool_executor.ts +++ b/src/core/tool_executor.ts @@ -6,7 +6,7 @@ // brain's grounding gate (tests_pass / parse_fail_count) reads the same shape // regardless of which side originally ran it. -import { spawnSync, type SpawnSyncReturns } from "node:child_process"; +import { spawn, spawnSync } from "node:child_process"; import { closeSync, constants as fsConstants, existsSync, mkdirSync, openSync, readFileSync, readdirSync, realpathSync, statSync, writeFileSync } from "node:fs"; import { dirname, relative, resolve, sep } from "node:path"; import type { ToolName } from "./brain_protocol.js"; @@ -26,6 +26,12 @@ const SNAPSHOT_MAX_BYTES = 1024 * 1024; const SEARCH_MAX_HITS = 40; const SEARCH_SKIP_DIRS = new Set([".git", "node_modules", "dist"]); +/** Per-call execution controls for the shell-backed tools. */ +export interface RunOptions { + timeoutMs?: number; + signal?: AbortSignal; +} + export interface ToolResult { output: string; exitCode: number; @@ -81,18 +87,124 @@ export class ToolExecutor { return abs; } - /** Run a shell command in the workspace; capture combined output, capped. */ - private run(command: string, timeoutMs = 900_000): ToolResult { - const shell = - process.platform === "win32" ? (process.env["ComSpec"] ?? "C:\\Windows\\System32\\cmd.exe") : true; - const r = spawnSync(command, { - shell, - cwd: this.root, - encoding: "utf8", - timeout: timeoutMs, - maxBuffer: 64 * 1024 * 1024, + /** + * Run a shell command in the workspace; capture combined output, capped. + * + * Asynchronous and tree-aware. The previous implementation used spawnSync + * with a timeout, which has two defects the caller cannot see: + * + * - a timeout signals the DIRECT child only, which is the shell. Whatever + * the user actually started (npm test, pytest, a compiler) is orphaned and + * keeps running, holding ports, files and CPU, while the call returns + * looking like a clean timeout. + * - the whole event loop is blocked for the duration, freezing heartbeats, + * the renderer and any AbortController. That is why Ctrl+C could not + * interrupt a long test run. + * + * Cancellation and timeout resolve distinctly (130 vs 124): one is the + * operator, one is the clock, and the caller needs to tell them apart. + */ + private run(command: string, options: RunOptions = {}): Promise { + const timeoutMs = options.timeoutMs ?? 900_000; + const signal = options.signal; + const onWindows = process.platform === "win32"; + const shell = onWindows ? (process.env["ComSpec"] ?? "C:\\Windows\\System32\\cmd.exe") : "/bin/sh"; + + return new Promise((resolve) => { + if (signal?.aborted) { + resolve({ output: "[aborted before start]", exitCode: 130 }); + return; + } + + const child = spawn(command, { + shell, + cwd: this.root, + // POSIX: a new process group, so one kill reaches every descendant. + // Windows has no equivalent here; taskkill /T walks the tree instead, + // so detaching there buys nothing and complicates exit reporting. + detached: !onWindows, + stdio: ["ignore", "pipe", "pipe"], + }); + + let out = ""; + let bytes = 0; + const CAP = 64 * 1024 * 1024; + const absorb = (chunk: Buffer): void => { + bytes += chunk.length; + // Keep draining past the cap so the pipe never blocks the child, but + // stop retaining; capHeadTail trims the ends at the boundary anyway. + if (bytes <= CAP) out += chunk.toString("utf8"); + }; + child.stdout?.on("data", absorb); + child.stderr?.on("data", absorb); + + let settled = false; + let verdict: "timeout" | "aborted" | null = null; + + const killTree = (): void => { + const pid = child.pid; + if (pid === undefined) return; + if (onWindows) { + // /T the tree, /F because a hung runner will not exit politely. + spawnSync("taskkill", ["/pid", String(pid), "/T", "/F"], { encoding: "utf8" }); + return; + } + try { + process.kill(-pid, "SIGTERM"); + } catch { + /* group already gone */ + } + // Escalate: a runner that traps SIGTERM must not outlive the timeout. + setTimeout(() => { + try { + process.kill(-pid, "SIGKILL"); + } catch { + /* already reaped */ + } + }, 2000).unref(); + }; + + const timer = setTimeout(() => { + verdict = "timeout"; + killTree(); + }, timeoutMs); + timer.unref(); + + const onAbort = (): void => { + verdict = "aborted"; + killTree(); + }; + signal?.addEventListener("abort", onAbort, { once: true }); + + const finish = (result: ToolResult): void => { + if (settled) return; + settled = true; + clearTimeout(timer); + signal?.removeEventListener("abort", onAbort); + resolve(result); + }; + + child.on("error", (err: NodeJS.ErrnoException) => { + const code = err.code === "ENOENT" ? 127 : 1; + finish({ output: `[spawn error ${err.code ?? "UNKNOWN"}: ${err.message}]`, exitCode: code }); + }); + + // 'close' rather than 'exit': it fires once the pipes are drained, so a + // test summary arriving with the exit is not lost. + child.on("close", (code, sig) => { + const body = capHeadTail(out, MAX_OUTPUT); + if (verdict === "timeout") { + finish({ output: `[timeout after ${Math.round(timeoutMs / 1000)}s]\n${body}`, exitCode: 124 }); + return; + } + if (verdict === "aborted") { + finish({ output: `[aborted]\n${body}`, exitCode: 130 }); + return; + } + const exit = code ?? (sig ? 1 : 1); + finish({ output: `[exit ${exit}]\n${body}`, exitCode: exit }); + }); }); - return toResult(r, timeoutMs); } /** @@ -114,16 +226,10 @@ export class ToolExecutor { case "write_file": return this.writeFile(String(args["path"] ?? ""), String(args["content"] ?? "")); case "run_shell": - return this.run(String(args["command"] ?? "")); - case "run_tests": { - const cmd = String(args["command"] ?? "") || this.testCmd; - // No explicit command and no configured testCmd: CONTRACTS.md invariant 5 - // — "" means "no ground truth to assert". Report it plainly instead of - // spawning an empty/undefined command (which reads as a confusing shell - // or ENOENT error) or silently substituting an unrelated test runner. - if (!cmd) return { output: "[no test_cmd configured — unverifiable]", exitCode: 1 }; - return this.run(cmd); - } + case "run_tests": + // Shell-backed tools became async so a timeout or Ctrl+C can reap the + // whole process tree. Routed through executeAsync like the web tools. + return { output: `[tool ${name} is async — call executeAsync]`, exitCode: 1 }; case "repo_search": return this.repoSearch(String(args["query"] ?? "")); case "git_commit": @@ -147,7 +253,7 @@ export class ToolExecutor { * Web tools never throw — they return a bracketed string, exit 0 (advisory * output the brain reads as ordinary tool output). */ - async executeAsync(name: string, rawArgs: unknown): Promise { + async executeAsync(name: string, rawArgs: unknown, options: RunOptions = {}): Promise { const validation = validateToolCall(name, rawArgs); if (!validation.ok) { return { output: `[tool ${name} rejected: ${validation.error}]`, exitCode: 1 }; @@ -161,6 +267,18 @@ export class ToolExecutor { ); return { output: capHeadTail(text, MAX_OUTPUT), exitCode: 0 }; } + if (name === "run_shell") { + return this.run(String(args["command"] ?? ""), options); + } + if (name === "run_tests") { + const cmd = String(args["command"] ?? "") || this.testCmd; + // No explicit command and no configured testCmd: CONTRACTS.md invariant 5 + // — "" means "no ground truth to assert". Report it plainly instead of + // spawning an empty/undefined command (which reads as a confusing shell + // or ENOENT error) or silently substituting an unrelated test runner. + if (!cmd) return { output: "[no test_cmd configured — unverifiable]", exitCode: 1 }; + return this.run(cmd, options); + } if (name === "web_fetch") { const text = await webFetch(String(args["url"] ?? ""), MAX_OUTPUT); return { output: capHeadTail(text, MAX_OUTPUT), exitCode: 0 }; @@ -253,21 +371,6 @@ export class ToolExecutor { } } -/** Shared spawnSync -> ToolResult mapping for run() (shell): - * timeout/ENOENT/exit-code handling. */ -function toResult(r: SpawnSyncReturns, timeoutMs: number): ToolResult { - if (r.error) { - const err = r.error as NodeJS.ErrnoException; - if (err.code === "ETIMEDOUT") { - return { output: `[timeout after ${Math.round(timeoutMs / 1000)}s]`, exitCode: 124 }; - } - const code = err.code === "ENOENT" ? 127 : 1; - return { output: `[spawn error ${err.code ?? "UNKNOWN"}: ${err.message}]`, exitCode: code }; - } - const code = r.status ?? 1; - const body = capHeadTail((r.stdout ?? "") + (r.stderr ?? ""), MAX_OUTPUT); - return { output: `[exit ${code}]\n${body}`, exitCode: code }; -} /** Cap text to `max` chars keeping BOTH ends. Test runners print detail first and * the summary (`N failed`, final assertion) LAST — a head-only slice loses the diff --git a/src/core/verify_gate.ts b/src/core/verify_gate.ts index 94c9fba..d85a2be 100644 --- a/src/core/verify_gate.ts +++ b/src/core/verify_gate.ts @@ -21,7 +21,11 @@ export interface BrainDone { /** What the host runs to establish ground truth — a ToolExecutor, or a fake in tests. */ export interface VerifyRunner { - execute(name: string, args: Record): ToolResult; + // executeAsync, not execute: the shell-backed tools became asynchronous so a + // timeout or Ctrl+C can reap the whole process tree. Naming the async method + // here is deliberate — a fake that still implements the sync one will fail to + // compile rather than quietly diverge from what production calls. + executeAsync(name: string, args: Record): Promise; } export interface VerifyOutcome { @@ -57,18 +61,18 @@ export function parseFailCount(output: string): number | null { * distinct from `done.ok=false`: a completed-but-failing brain is overruled by a * green host, but a CRASHED brain is not — its run never reached a clean end. */ -export function finalVerify( +export async function finalVerify( exec: VerifyRunner, testCmd: string | undefined, done: BrainDone | null, errored = false, -): VerifyOutcome { +): Promise { if (!testCmd) { return errored ? { status: "error", remaining: done?.remaining ?? 0, exitCode: 1 } : { status: "unverified", remaining: done?.remaining ?? 0, exitCode: -1 }; } - const verify = exec.execute("run_tests", { command: testCmd }); + const verify = await exec.executeAsync("run_tests", { command: testCmd }); const remaining = parseFailCount(verify.output) ?? done?.remaining ?? 0; // A crashed brain is never "ok", even on a green tree; we still run the gate so // the failing count (if any) is logged. diff --git a/test/bridge.test.ts b/test/bridge.test.ts index c6b54f8..869cb33 100644 --- a/test/bridge.test.ts +++ b/test/bridge.test.ts @@ -374,12 +374,12 @@ test("path-guard rejects traversal, absolute, and symlink escapes", () => { }); // --- probe 3: shell hardening (non-zero exit + stderr reach the brain) ------ -test("run_shell surfaces a non-zero exit code and captures stderr", (t) => { +test("run_shell surfaces a non-zero exit code and captures stderr", async (t) => { // `node -e` (not a platform-specific shell one-liner) so this passes // identically on Windows and POSIX without a win32/posix branch, and runs // in an isolated tmpdir so it can't leave stray output under process.cwd(). const ex = new ToolExecutor(mkdtempSync(join(tmpdir(), "aether-sh-"))); - const r = ex.execute("run_shell", { + const r = await ex.executeAsync("run_shell", { command: `node -e "process.stderr.write('boom'); process.exit(3)"`, }); if (/spawn error EPERM/.test(r.output)) { diff --git a/test/process_tree.test.ts b/test/process_tree.test.ts new file mode 100644 index 0000000..772b12a --- /dev/null +++ b/test/process_tree.test.ts @@ -0,0 +1,133 @@ +// Process-tree teardown for run_shell / run_tests. +// +// The old path used spawnSync with a `timeout`. That signals the DIRECT child +// only — the shell — so the thing the user actually started (npm test, pytest, +// a compiler) is orphaned and keeps running, holding ports, files and CPU. On +// Windows the cmd.exe shell makes it near-certain. +// +// These assert survival by PID, not that the call returned promptly. A parent +// that exits while its grandchild keeps running looks identical to success from +// the caller's side, which is exactly how this went unnoticed. + +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { mkdtempSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { ToolExecutor } from "../src/core/tool_executor.js"; + +/** True while a pid exists. Signal 0 tests for existence without delivering. */ +function alive(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch { + return false; + } +} + +async function settle(ms = 500): Promise { + await new Promise((resolve) => setTimeout(resolve, ms)); +} + +/** + * A child that prints its own pid, spawns a long-lived grandchild that prints + * ITS pid, then sleeps. Both pids reach stdout before anything is killed, so a + * test can look them up afterwards. + */ +function treeScript(dir: string): string { + const grandchild = join(dir, "grandchild.js"); + writeFileSync(grandchild, "console.log('GRANDCHILD:' + process.pid); setInterval(() => {}, 1000);\n"); + const child = join(dir, "child.js"); + writeFileSync( + child, + "const { spawn } = require('node:child_process');\n" + + "console.log('CHILD:' + process.pid);\n" + + `const g = spawn(process.execPath, [${JSON.stringify(grandchild)}], { stdio: 'inherit' });\n` + + "void g;\n" + + "setInterval(() => {}, 1000);\n", + ); + return child; +} + +function pidsFrom(output: string): { child: number | null; grandchild: number | null } { + const grab = (label: string): number | null => { + const m = output.match(new RegExp(`${label}:(\\d+)`)); + return m ? Number(m[1]) : null; + }; + return { child: grab("CHILD"), grandchild: grab("GRANDCHILD") }; +} + +test("a timed-out command kills its whole tree, not just the shell", async () => { + const dir = mkdtempSync(join(tmpdir(), "aether-tree-")); + const script = treeScript(dir); + const exec = new ToolExecutor(dir); + + const result = await exec.executeAsync("run_shell", { command: `"${process.execPath}" "${script}"` }, { timeoutMs: 1500 }); + + assert.equal(result.exitCode, 124, "a timeout must be distinguishable from an ordinary failure"); + assert.match(result.output, /timeout/i); + + const { child, grandchild } = pidsFrom(result.output); + assert.ok(child, `no child pid in output: ${result.output.slice(0, 300)}`); + assert.ok(grandchild, `no grandchild pid in output: ${result.output.slice(0, 300)}`); + + await settle(); + assert.equal(alive(child!), false, "the child survived the timeout"); + assert.equal(alive(grandchild!), false, "the GRANDCHILD survived — the tree was not reaped"); +}); + +test("an aborted command kills its whole tree and reports aborted, not timed out", async () => { + const dir = mkdtempSync(join(tmpdir(), "aether-abort-")); + const script = treeScript(dir); + const exec = new ToolExecutor(dir); + const controller = new AbortController(); + + const running = exec.executeAsync( + "run_shell", + { command: `"${process.execPath}" "${script}"` }, + { timeoutMs: 60_000, signal: controller.signal }, + ); + + await settle(900); // let both pids print + controller.abort(); + const result = await running; + + assert.equal(result.exitCode, 130, "an operator cancellation is not a timeout"); + assert.match(result.output, /abort/i); + + const { child, grandchild } = pidsFrom(result.output); + assert.ok(child && grandchild, `missing pids: ${result.output.slice(0, 300)}`); + await settle(); + assert.equal(alive(child!), false, "the child survived the abort"); + assert.equal(alive(grandchild!), false, "the GRANDCHILD survived the abort"); +}); + +test("a normal command still returns its real exit code and output", async () => { + const dir = mkdtempSync(join(tmpdir(), "aether-ok-")); + const exec = new ToolExecutor(dir); + + const ok = await exec.executeAsync("run_shell", { command: `"${process.execPath}" -e "console.log('hi')"` }); + assert.equal(ok.exitCode, 0); + assert.match(ok.output, /hi/); + + const bad = await exec.executeAsync("run_shell", { command: `"${process.execPath}" -e "process.exit(3)"` }); + assert.equal(bad.exitCode, 3, "a non-zero exit must survive unchanged"); +}); + +test("the event loop keeps running while a command is in flight", async () => { + // spawnSync blocked the loop outright, freezing heartbeats, renderers and any + // AbortController for the duration. Proving a timer fires during the call is + // what distinguishes async execution from a merely faster synchronous one. + const dir = mkdtempSync(join(tmpdir(), "aether-loop-")); + const exec = new ToolExecutor(dir); + let ticks = 0; + const timer = setInterval(() => { + ticks += 1; + }, 50); + + await exec.executeAsync("run_shell", { command: `"${process.execPath}" -e "setTimeout(()=>{},700)"` }); + clearInterval(timer); + + assert.ok(ticks > 2, `the event loop was blocked during execution (ticks=${ticks})`); +}); diff --git a/test/tool_executor.test.ts b/test/tool_executor.test.ts index 5965547..adabae8 100644 --- a/test/tool_executor.test.ts +++ b/test/tool_executor.test.ts @@ -19,7 +19,7 @@ function initRepo(): string { return dir; } -test("git_commit passes a message with shell metacharacters through unexecuted", (t) => { +test("git_commit passes a message with shell metacharacters through unexecuted", async (t) => { if (!canSpawnGit) { t.skip("sandbox blocks child process spawning"); return; } const dir = initRepo(); try { @@ -34,7 +34,7 @@ test("git_commit passes a message with shell metacharacters through unexecuted", } }); -test("git_commit with nothing staged reports it without a fabricated failure", (t) => { +test("git_commit with nothing staged reports it without a fabricated failure", async (t) => { if (!canSpawnGit) { t.skip("sandbox blocks child process spawning"); return; } const dir = initRepo(); try { @@ -47,7 +47,7 @@ test("git_commit with nothing staged reports it without a fabricated failure", ( } }); -test("run_tests with no explicit command and no configured testCmd does not default to pytest", () => { +test("run_tests with no explicit command and no configured testCmd does not default to pytest", async () => { // Regression for cac0399: an unset test_cmd must mean "unverifiable", never a // silent fallback to a real test runner. brain_protocol.ts's wire-encoding // default was fixed there; this covers the sibling default in ToolExecutor @@ -55,7 +55,7 @@ test("run_tests with no explicit command and no configured testCmd does not defa const dir = mkdtempSync(join(tmpdir(), "aether-runtests-")); try { const exec = new ToolExecutor(dir); // no testCmd passed — must NOT become "pytest -q" - const r = exec.execute("run_tests", {}); + const r = await exec.executeAsync("run_tests", {}); assert.doesNotMatch(r.output, /pytest/i, "must never silently run pytest when no test_cmd is configured"); assert.notEqual(r.exitCode, 0, "an unverifiable run_tests call must not report success"); } finally { @@ -63,11 +63,11 @@ test("run_tests with no explicit command and no configured testCmd does not defa } }); -test("run_tests still honors an explicit command even with no configured testCmd", (t) => { +test("run_tests still honors an explicit command even with no configured testCmd", async (t) => { const dir = mkdtempSync(join(tmpdir(), "aether-runtests-explicit-")); try { const exec = new ToolExecutor(dir); - const r = exec.execute("run_tests", { command: process.platform === "win32" ? "exit 0" : "true" }); + const r = await exec.executeAsync("run_tests", { command: process.platform === "win32" ? "exit 0" : "true" }); if (/spawn error EPERM/.test(r.output)) { t.skip("sandbox blocks child process spawning"); return; } assert.equal(r.exitCode, 0); } finally { @@ -75,11 +75,11 @@ test("run_tests still honors an explicit command even with no configured testCmd } }); -test("run_tests honors a configured testCmd when the call omits an explicit command", (t) => { +test("run_tests honors a configured testCmd when the call omits an explicit command", async (t) => { const dir = mkdtempSync(join(tmpdir(), "aether-runtests-cfg-")); try { const exec = new ToolExecutor(dir, process.platform === "win32" ? "exit 0" : "true"); - const r = exec.execute("run_tests", {}); + const r = await exec.executeAsync("run_tests", {}); if (/spawn error EPERM/.test(r.output)) { t.skip("sandbox blocks child process spawning"); return; } assert.equal(r.exitCode, 0); } finally { @@ -87,7 +87,7 @@ test("run_tests honors a configured testCmd when the call omits an explicit comm } }); -test("git_commit surfaces a real failure instead of reporting the old HEAD as success", (t) => { +test("git_commit surfaces a real failure instead of reporting the old HEAD as success", async (t) => { if (!canSpawnGit) { t.skip("sandbox blocks child process spawning"); return; } const dir = initRepo(); try { diff --git a/test/verify_gate.test.ts b/test/verify_gate.test.ts index 7a9a231..5186623 100644 --- a/test/verify_gate.test.ts +++ b/test/verify_gate.test.ts @@ -12,7 +12,7 @@ function fakeExec(result: ToolResult) { const calls: Array<{ name: string; args: Record }> = []; return { calls, - execute(name: string, args: Record): ToolResult { + async executeAsync(name: string, args: Record): Promise { calls.push({ name, args }); return result; }, @@ -22,7 +22,7 @@ function fakeExec(result: ToolResult) { const RED = (n: number): ToolResult => ({ output: `[exit 1]\n=== ${n} failed in 3.2s ===`, exitCode: 1 }); const GREEN: ToolResult = { output: "[exit 0]\n=== 24 passed in 3.2s ===", exitCode: 0 }; -test("parseFailCount matches the brain regex (\\d+\\s+failed)", () => { +test("parseFailCount matches the brain regex (\\d+\\s+failed)", async () => { assert.equal(parseFailCount("[exit 1]\n=== 24 failed, 0 passed ==="), 24); assert.equal(parseFailCount("[exit 1]\n24 failed"), 24); // two spaces — the single-space regex missed this assert.equal(parseFailCount("[exit 0]\n=== 12 passed ==="), null); @@ -30,58 +30,58 @@ test("parseFailCount matches the brain regex (\\d+\\s+failed)", () => { }); // ── THE regression: the brain lies, the host catches it ───────────────────── -test("brain done.ok=true while host tests are RED → never ok", () => { +test("brain done.ok=true while host tests are RED → never ok", async () => { const exec = fakeExec(RED(1)); - const out = finalVerify(exec, "pytest -q", { ok: true, remaining: 0, reason: "" }); + const out = await finalVerify(exec, "pytest -q", { ok: true, remaining: 0, reason: "" }); assert.notEqual(out.status, "ok"); // the old bug returned "ok" here assert.equal(out.status, "incomplete"); assert.equal(out.remaining, 1); assert.equal(out.exitCode, 1); }); -test("24-bug corpus: brain self-reports ok but 24 still failing → incomplete, remaining 24", () => { +test("24-bug corpus: brain self-reports ok but 24 still failing → incomplete, remaining 24", async () => { const exec = fakeExec(RED(24)); - const out = finalVerify(exec, "pytest -q", { ok: true, remaining: 0, reason: "" }); + const out = await finalVerify(exec, "pytest -q", { ok: true, remaining: 0, reason: "" }); assert.equal(out.status, "incomplete"); assert.equal(out.remaining, 24); }); -test("host GREEN → ok even if the brain gave up (host is authoritative)", () => { +test("host GREEN → ok even if the brain gave up (host is authoritative)", async () => { const exec = fakeExec(GREEN); - const out = finalVerify(exec, "pytest -q", { ok: false, remaining: 24, reason: "stalled" }); + const out = await finalVerify(exec, "pytest -q", { ok: false, remaining: 24, reason: "stalled" }); assert.equal(out.status, "ok"); assert.equal(out.remaining, 0); assert.equal(out.exitCode, 0); }); -test("no test command → unverified, never ok (no ground truth to assert)", () => { +test("no test command → unverified, never ok (no ground truth to assert)", async () => { const exec = fakeExec(GREEN); - const out = finalVerify(exec, undefined, { ok: true, remaining: 0, reason: "" }); + const out = await finalVerify(exec, undefined, { ok: true, remaining: 0, reason: "" }); assert.equal(out.status, "unverified"); assert.equal(exec.calls.length, 0); // must NOT run anything when there is no gate }); -test("breaker reason is surfaced through a RED host (stalled/max-turns, not flat incomplete)", () => { - const stalled = finalVerify(fakeExec(RED(5)), "pytest -q", { ok: false, remaining: 5, reason: "stalled" }); +test("breaker reason is surfaced through a RED host (stalled/max-turns, not flat incomplete)", async () => { + const stalled = await await finalVerify(fakeExec(RED(5)), "pytest -q", { ok: false, remaining: 5, reason: "stalled" }); assert.equal(stalled.status, "stalled"); - const maxTurns = finalVerify(fakeExec(RED(2)), "pytest -q", { ok: false, remaining: 2, reason: "max-turns" }); + const maxTurns = await await finalVerify(fakeExec(RED(2)), "pytest -q", { ok: false, remaining: 2, reason: "max-turns" }); assert.equal(maxTurns.status, "max-turns"); }); -test("RED with unparseable output falls back to the brain's remaining, not a -1 sentinel", () => { +test("RED with unparseable output falls back to the brain's remaining, not a -1 sentinel", async () => { const exec = fakeExec({ output: "[exit 1]\nsegfault, no summary line", exitCode: 1 }); - const out = finalVerify(exec, "pytest -q", { ok: false, remaining: 7, reason: "" }); + const out = await finalVerify(exec, "pytest -q", { ok: false, remaining: 7, reason: "" }); assert.equal(out.status, "incomplete"); assert.equal(out.remaining, 7); }); -test("null done (brain never reported) + RED → incomplete with the parsed count", () => { - const out = finalVerify(fakeExec(RED(3)), "pytest -q", null); +test("null done (brain never reported) + RED → incomplete with the parsed count", async () => { + const out = await await finalVerify(fakeExec(RED(3)), "pytest -q", null); assert.equal(out.status, "incomplete"); assert.equal(out.remaining, 3); }); -test("the gate runs the host's own test command via run_tests", () => { +test("the gate runs the host's own test command via run_tests", async () => { const exec = fakeExec(GREEN); finalVerify(exec, "pytest -q tests/unit", { ok: true, remaining: 0, reason: "" }); assert.equal(exec.calls.length, 1); @@ -94,32 +94,32 @@ test("the gate runs the host's own test command via run_tests", () => { // is overruled — that is a genuine success). An `error` event means the brain // CRASHED mid-run; a coincidentally-green tree must NOT be reported as a clean // success. (Regression guard: the gate used to mask this and exit 0.) -test("brain ERROR + host GREEN → error, never ok (a crashed run is not a success)", () => { +test("brain ERROR + host GREEN → error, never ok (a crashed run is not a success)", async () => { const exec = fakeExec(GREEN); - const out = finalVerify(exec, "pytest -q", { ok: false, remaining: 0, reason: "" }, true); + const out = await finalVerify(exec, "pytest -q", { ok: false, remaining: 0, reason: "" }, true); assert.equal(out.status, "error"); assert.notEqual(out.exitCode, 0); }); -test("brain ERROR + no test command → error (not unverified)", () => { - const out = finalVerify(fakeExec(GREEN), undefined, null, true); +test("brain ERROR + no test command → error (not unverified)", async () => { + const out = await await finalVerify(fakeExec(GREEN), undefined, null, true); assert.equal(out.status, "error"); }); -test("brain ERROR + host RED → error, with the parsed failing count", () => { - const out = finalVerify(fakeExec(RED(4)), "pytest -q", null, true); +test("brain ERROR + host RED → error, with the parsed failing count", async () => { + const out = await await finalVerify(fakeExec(RED(4)), "pytest -q", null, true); assert.equal(out.status, "error"); assert.equal(out.remaining, 4); }); -test("brain DONE self-reporting failure on a green tree stays ok (errored defaults false)", () => { +test("brain DONE self-reporting failure on a green tree stays ok (errored defaults false)", async () => { // The intentional 'host is authoritative' rule — a COMPLETED brain, not a crash. - const out = finalVerify(fakeExec(GREEN), "pytest -q", { ok: false, remaining: 9, reason: "stalled" }); + const out = await await finalVerify(fakeExec(GREEN), "pytest -q", { ok: false, remaining: 9, reason: "stalled" }); assert.equal(out.status, "ok"); }); // Type-level: BrainDone is the subset of the done event the gate consumes. -test("BrainDone shape is { ok, remaining, reason }", () => { +test("BrainDone shape is { ok, remaining, reason }", async () => { const d: BrainDone = { ok: false, remaining: 1, reason: "stalled" }; assert.equal(d.reason, "stalled"); });