|
| 1 | +// Target-repo verification gate (#8807): run the TARGET repository's own detected test/lint/build commands |
| 2 | +// against the attempt's worktree BEFORE a PR opens — the independent check the audit found missing: the |
| 3 | +// only verification module that existed (engine lint-guard.ts) is hardcoded to loopover's own monorepo |
| 4 | +// commands and never passed in production, and coding-task-spec's validation guidance only TELLS the agent |
| 5 | +// which commands to run, trusting its self-attestation. A coding agent that skips or fakes its own test run |
| 6 | +// previously produced a PR that passed every AMS-side gate and still broke the target repo's build. |
| 7 | +// |
| 8 | +// Commands come from stack-detection.js's already-inferred RepoStackResult (the same source the agent's own |
| 9 | +// guidance renders), run in test → lint → build order (highest signal first), stop at the first failure, |
| 10 | +// with a per-command timeout and a bounded output tail (the postmortem detail, never an unbounded dump). |
| 11 | +// An UNDETECTED stack or a stack with no inferred commands SKIPS (recorded, never a failure): this gate can |
| 12 | +// only ever be as smart as detection, and refusing to submit because detection came up empty would block |
| 13 | +// legitimate work on repos with unconventional tooling. |
| 14 | +import { spawn as nodeSpawn } from "node:child_process"; |
| 15 | +import type { RepoStackResult } from "./stack-detection.js"; |
| 16 | + |
| 17 | +/** The slice of ChildProcess the tree-kill needs — narrow so tests can drive both arms with plain fakes. */ |
| 18 | +export type KillableChild = { pid?: number | undefined; kill: (signal: NodeJS.Signals) => boolean }; |
| 19 | + |
| 20 | +export type TargetRepoVerificationSpawn = ( |
| 21 | + command: string, |
| 22 | + options: { cwd: string; timeoutMs: number }, |
| 23 | +) => Promise<{ code: number | null; output: string }>; |
| 24 | + |
| 25 | +export type TargetRepoVerificationCheck = { |
| 26 | + kind: "test" | "lint" | "build"; |
| 27 | + command: string; |
| 28 | + ok: boolean; |
| 29 | + exitCode: number | null; |
| 30 | + outputTail: string; |
| 31 | +}; |
| 32 | + |
| 33 | +export type TargetRepoVerificationResult = |
| 34 | + | { status: "passed"; checks: TargetRepoVerificationCheck[] } |
| 35 | + | { status: "failed"; checks: TargetRepoVerificationCheck[]; firstFailure: TargetRepoVerificationCheck } |
| 36 | + | { status: "skipped"; reason: "stack_undetected" | "no_commands_detected" | "disabled" }; |
| 37 | + |
| 38 | +/** Per-command wall-clock bound. A target repo's test suite legitimately runs minutes; 10 is the ceiling |
| 39 | + * before the gate itself becomes the attempt's bottleneck — a suite slower than this is skipped territory |
| 40 | + * for a future per-repo override, not something to silently wait out. */ |
| 41 | +export const DEFAULT_VERIFICATION_TIMEOUT_MS = 10 * 60 * 1000; |
| 42 | +/** Postmortem detail bound — enough tail to show the failing assertion, never an unbounded log dump. */ |
| 43 | +export const VERIFICATION_OUTPUT_TAIL_CHARS = 4000; |
| 44 | + |
| 45 | +/** Post-timeout grace before giving up on the `close` event: a killed process group's pipes close nearly |
| 46 | + * instantly, so this only fires when something double-forked out of the group and kept the pipes open — |
| 47 | + * the gate resolves as failed rather than hanging on that orphan. */ |
| 48 | +export const VERIFICATION_KILL_SETTLE_MS = 5000; |
| 49 | + |
| 50 | +/** Kill the command's whole detached process group via the NEGATIVE pid: a test command is routinely a tree |
| 51 | + * (`npm test` → node → workers), and killing only the shell leaves grandchildren holding the stdio pipes — |
| 52 | + * the `close` event then waits on THEM, stalling the gate far past its own timeout (observed as the 30s |
| 53 | + * hang on Linux CI). Falls back to the plain single-process kill when the group kill isn't possible |
| 54 | + * (no pid, or the group is already gone and the signal throws). */ |
| 55 | +export function killVerificationProcessTree(child: KillableChild, killGroup: (pid: number, signal: NodeJS.Signals) => void = (pid, signal) => process.kill(-pid, signal)): void { |
| 56 | + try { |
| 57 | + if (typeof child.pid !== "number") throw new Error("child has no pid"); |
| 58 | + killGroup(child.pid, "SIGKILL"); |
| 59 | + } catch { |
| 60 | + child.kill("SIGKILL"); |
| 61 | + } |
| 62 | +} |
| 63 | + |
| 64 | +/** Default spawn: shell-executed (detected commands are shell strings like "npm test" / "ruff check ."), |
| 65 | + * merged stdout+stderr, killed at the timeout (a killed/timed-out command reports code null → treated as |
| 66 | + * failure upstream). `internals` exists ONLY for tests to reach the timeout/settle arms deterministically; |
| 67 | + * production callers always take the defaults. */ |
| 68 | +export function runShellCommandWithTreeKill( |
| 69 | + command: string, |
| 70 | + options: { cwd: string; timeoutMs: number }, |
| 71 | + internals: { killTree?: (child: KillableChild) => void; settleMs?: number } = {}, |
| 72 | +): Promise<{ code: number | null; output: string }> { |
| 73 | + const killTree = internals.killTree ?? killVerificationProcessTree; |
| 74 | + const settleMs = internals.settleMs ?? VERIFICATION_KILL_SETTLE_MS; |
| 75 | + return new Promise((resolve) => { |
| 76 | + // detached: its own process group, so the timeout can kill the entire tree, not just the shell. |
| 77 | + const child = nodeSpawn(command, { cwd: options.cwd, shell: true, stdio: ["ignore", "pipe", "pipe"], detached: true }); |
| 78 | + let output = ""; |
| 79 | + let settled = false; |
| 80 | + let settleTimer: ReturnType<typeof setTimeout> | undefined; |
| 81 | + const finish = (result: { code: number | null; output: string }) => { |
| 82 | + if (settled) return; |
| 83 | + settled = true; |
| 84 | + clearTimeout(timer); |
| 85 | + if (settleTimer !== undefined) clearTimeout(settleTimer); |
| 86 | + resolve(result); |
| 87 | + }; |
| 88 | + const capture = (chunk: Buffer) => { |
| 89 | + output = (output + chunk.toString()).slice(-VERIFICATION_OUTPUT_TAIL_CHARS * 4); |
| 90 | + }; |
| 91 | + child.stdout?.on("data", capture); |
| 92 | + child.stderr?.on("data", capture); |
| 93 | + const timer = setTimeout(() => { |
| 94 | + killTree(child); |
| 95 | + // Bounded settle: if some orphan still holds the pipes open after the group kill, resolve as a |
| 96 | + // timeout failure anyway — the verification gate must never outlive its own per-command bound. |
| 97 | + settleTimer = setTimeout(() => { |
| 98 | + finish({ code: null, output: `${output}\n[verification timeout after ${options.timeoutMs}ms — process tree killed]` }); |
| 99 | + }, settleMs); |
| 100 | + }, options.timeoutMs); |
| 101 | + child.on("error", (error) => { |
| 102 | + finish({ code: null, output: `${output}\n${String(error)}` }); |
| 103 | + }); |
| 104 | + child.on("close", (code) => { |
| 105 | + finish({ code, output }); |
| 106 | + }); |
| 107 | + }); |
| 108 | +} |
| 109 | + |
| 110 | +export const defaultVerificationSpawn: TargetRepoVerificationSpawn = (command, options) => runShellCommandWithTreeKill(command, options); |
| 111 | + |
| 112 | +export async function runTargetRepoVerification(options: { |
| 113 | + worktreeDir: string; |
| 114 | + stack: RepoStackResult; |
| 115 | + spawn?: TargetRepoVerificationSpawn; |
| 116 | + timeoutMsPerCommand?: number; |
| 117 | +}): Promise<TargetRepoVerificationResult> { |
| 118 | + const stack = options.stack; |
| 119 | + if (stack.detected !== true) return { status: "skipped", reason: "stack_undetected" }; |
| 120 | + const commands: Array<{ kind: TargetRepoVerificationCheck["kind"]; command: string | null }> = [ |
| 121 | + { kind: "test", command: stack.testCommand }, |
| 122 | + { kind: "lint", command: stack.lintCommand }, |
| 123 | + { kind: "build", command: stack.buildCommand }, |
| 124 | + ]; |
| 125 | + const runnable = commands.filter((entry): entry is { kind: TargetRepoVerificationCheck["kind"]; command: string } => entry.command !== null); |
| 126 | + if (runnable.length === 0) return { status: "skipped", reason: "no_commands_detected" }; |
| 127 | + |
| 128 | + const spawn = options.spawn ?? defaultVerificationSpawn; |
| 129 | + const timeoutMs = options.timeoutMsPerCommand ?? DEFAULT_VERIFICATION_TIMEOUT_MS; |
| 130 | + const checks: TargetRepoVerificationCheck[] = []; |
| 131 | + for (const { kind, command } of runnable) { |
| 132 | + const { code, output } = await spawn(command, { cwd: options.worktreeDir, timeoutMs }); |
| 133 | + const check: TargetRepoVerificationCheck = { |
| 134 | + kind, |
| 135 | + command, |
| 136 | + ok: code === 0, |
| 137 | + exitCode: code, |
| 138 | + outputTail: output.slice(-VERIFICATION_OUTPUT_TAIL_CHARS), |
| 139 | + }; |
| 140 | + checks.push(check); |
| 141 | + // Stop at the first failure: the remaining commands' results would only pile noise onto an attempt that |
| 142 | + // is already not submitting, and a broken build often cascades into misleading downstream failures. |
| 143 | + if (!check.ok) return { status: "failed", checks, firstFailure: check }; |
| 144 | + } |
| 145 | + return { status: "passed", checks }; |
| 146 | +} |
0 commit comments