Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions apps/loopover-ui/src/lib/ams-env-reference.ts
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,11 @@ export const AMS_ENV_REFERENCE_ROWS: MinerEnvReferenceRow[] = [
firstReference: "packages/loopover-engine/src/miner/driver-factory.ts",
defaultValue: null,
},
{
name: "MINER_SKIP_TARGET_REPO_VERIFICATION",
firstReference: "lib/attempt-cli.ts",
defaultValue: "",
},
];

export const AMS_ENV_REFERENCE_MARKDOWN = [
Expand Down Expand Up @@ -297,5 +302,6 @@ export const AMS_ENV_REFERENCE_MARKDOWN = [
'| `MINER_CODING_AGENT_PAUSED` | `packages/loopover-engine/src/miner/coding-agent-mode.ts` | `""` |',
'| `MINER_CODING_AGENT_PROVIDER` | `lib/laptop-init.ts` | `""` |',
"| `MINER_CODING_AGENT_TIMEOUT_MS` | `packages/loopover-engine/src/miner/driver-factory.ts` | (none) |",
'| `MINER_SKIP_TARGET_REPO_VERIFICATION` | `lib/attempt-cli.ts` | `""` |',
"",
].join("\n");
1 change: 1 addition & 0 deletions packages/loopover-miner/docs/env-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,3 +51,4 @@ Generated by `npm run miner:env-reference`. Do not edit manually.
| `MINER_CODING_AGENT_PAUSED` | `packages/loopover-engine/src/miner/coding-agent-mode.ts` | `""` |
| `MINER_CODING_AGENT_PROVIDER` | `lib/laptop-init.ts` | `""` |
| `MINER_CODING_AGENT_TIMEOUT_MS` | `packages/loopover-engine/src/miner/driver-factory.ts` | (none) |
| `MINER_SKIP_TARGET_REPO_VERIFICATION` | `lib/attempt-cli.ts` | `""` |
19 changes: 19 additions & 0 deletions packages/loopover-miner/lib/attempt-cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ import { isValidRepoSegment } from "./repo-clone.js";
import { REJECTION_REASON_AI_USAGE_POLICY_BAN, REJECTION_REASON_OWN_SUBMISSION_REJECTED, resolveOwnOpenPrForIssue, resolveRejectionSignaled } from "./rejection-signal.js";
import { initDenyHookSynthesisStore } from "./deny-hook-synthesis.js";
import type { DenyRule } from "@loopover/engine";
import { runTargetRepoVerification } from "./target-repo-verification.js";
import { detectRepoStack } from "./stack-detection.js";
import type { resolveRejectionSignaled as ResolveRejectionSignaledFn } from "./rejection-signal.js";
import { cleanupAttemptWorktree, prepareAttemptWorktree } from "./attempt-worktree.js";
import type {
Expand Down Expand Up @@ -145,6 +147,8 @@ export type RunAttemptOptions = {
resolveRejectionSignaled?: typeof ResolveRejectionSignaledFn;
// #8808: injection seam for the own-open-PR idempotency guard, mirroring resolveRejectionSignaled above.
resolveOwnOpenPrForIssue?: typeof resolveOwnOpenPrForIssue;
// #8807: injection seam for the target-repo verification gate, mirroring the resolver seams above.
runTargetRepoVerification?: typeof runTargetRepoVerification;
fetchImpl?: SelfReviewContextFetch;
prepareAttemptWorktree?: typeof PrepareAttemptWorktreeFn;
cleanupAttemptWorktree?: typeof CleanupAttemptWorktreeFn;
Expand Down Expand Up @@ -739,6 +743,9 @@ export async function runAttempt(args: string[], options: RunAttemptOptions = {}
};
};

// #8807: captured as a const here (where the !ok early-return has already narrowed the union) because
// the verification thunk below closes over it — TS drops narrowing on a mutable binding inside closures.
const attemptWorktreePath = worktreeResult.worktreePath;
const loopInput = buildAttemptLoopInput({
codingTaskSpec,
reviewContext,
Expand Down Expand Up @@ -872,6 +879,18 @@ export async function runAttempt(args: string[], options: RunAttemptOptions = {}
...deps,
shouldAbort,
resolveKillSwitchScope: () => resolveLiveKillSwitch().scope,
// #8807: pre-bound target-repo verification against THIS attempt's worktree, using the same stack
// detection the agent's own validation guidance rendered. Opt-out escape hatch for repos whose
// suites exceed the per-command bound; the gate itself skips (never fails) on an undetected stack.
...(/^(1|true|yes|on)$/i.test((env.MINER_SKIP_TARGET_REPO_VERIFICATION ?? "").trim())
? {}
: {
verifyTargetRepo: () =>
(options.runTargetRepoVerification ?? runTargetRepoVerification)({
worktreeDir: attemptWorktreePath,
stack: detectRepoStack(attemptWorktreePath),
}),
}),
},
);
} catch (error) {
Expand Down
19 changes: 19 additions & 0 deletions packages/loopover-miner/lib/attempt-runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,12 @@ export type AttemptInput = {
};

export type AttemptDeps = {
/** #8807: pre-bound target-repo verification (worktree + detected stack captured by the caller). Runs the
* TARGET repo's own test/lint/build commands after handoff and BEFORE any submission read/write — a
* failed verification blocks the PR instead of trusting the coding agent's self-attestation. Optional:
* absent (older callers, tests) preserves the pre-#8807 flow byte-identically. Loosely typed at this
* public boundary like runSlopAssessment above; the real shape is TargetRepoVerificationResult. */
verifyTargetRepo?: () => Promise<{ status: string } & Record<string, unknown>>;
driver: CodingAgentDriver;
runSlopAssessment: (input: unknown) => unknown;
appendAttemptLogEvent: (event: unknown) => void;
Expand All @@ -83,6 +89,7 @@ export type AttemptDeps = {

export type AttemptResult =
| { outcome: "abandon"; loopResult: IterateLoopResult }
| { outcome: "verification_failed"; verification: unknown; loopResult: IterateLoopResult }
| { outcome: "stale"; reason: FreshnessAbortReason; loopResult: IterateLoopResult }
| { outcome: "blocked"; decision: HarnessSubmissionDecision; loopResult: IterateLoopResult }
| { outcome: "governed"; decision: GovernorDecision; loopResult: IterateLoopResult }
Expand Down Expand Up @@ -213,6 +220,18 @@ export async function runMinerAttempt(input: AttemptInput, deps: AttemptDeps): P
}
}

// #8807: the independent quality gate — the target repo's own commands against the worktree. Placed
// BEFORE the freshness read so a failing build never spends GitHub API budget. A "skipped" or "passed"
// result proceeds; only a real command failure blocks. Deliberately NOT re-entering the iterate loop in
// this change: the loop's internal self-review iterations already ran, and never-submit-known-bad is the
// trust win — feeding the failure back as loop input is the tracked follow-up on the issue.
if (typeof deps.verifyTargetRepo === "function") {
const verification = await deps.verifyTargetRepo();
if (verification.status === "failed") {
return { outcome: "verification_failed", verification, loopResult };
}
}

const freshness = await checkSubmissionFreshness(
{ repoFullName: input.loopInput.repoFullName, issueNumber: input.issueNumber, minerLogin: input.minerLogin },
{ claimLedger: deps.claimLedger, fetchLiveIssueSnapshot: deps.fetchLiveIssueSnapshot, eventLedger: deps.eventLedger },
Expand Down
146 changes: 146 additions & 0 deletions packages/loopover-miner/lib/target-repo-verification.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
// Target-repo verification gate (#8807): run the TARGET repository's own detected test/lint/build commands
// against the attempt's worktree BEFORE a PR opens — the independent check the audit found missing: the
// only verification module that existed (engine lint-guard.ts) is hardcoded to loopover's own monorepo
// commands and never passed in production, and coding-task-spec's validation guidance only TELLS the agent
// which commands to run, trusting its self-attestation. A coding agent that skips or fakes its own test run
// previously produced a PR that passed every AMS-side gate and still broke the target repo's build.
//
// Commands come from stack-detection.js's already-inferred RepoStackResult (the same source the agent's own
// guidance renders), run in test → lint → build order (highest signal first), stop at the first failure,
// with a per-command timeout and a bounded output tail (the postmortem detail, never an unbounded dump).
// An UNDETECTED stack or a stack with no inferred commands SKIPS (recorded, never a failure): this gate can
// only ever be as smart as detection, and refusing to submit because detection came up empty would block
// legitimate work on repos with unconventional tooling.
import { spawn as nodeSpawn } from "node:child_process";
import type { RepoStackResult } from "./stack-detection.js";

/** The slice of ChildProcess the tree-kill needs — narrow so tests can drive both arms with plain fakes. */
export type KillableChild = { pid?: number | undefined; kill: (signal: NodeJS.Signals) => boolean };

export type TargetRepoVerificationSpawn = (
command: string,
options: { cwd: string; timeoutMs: number },
) => Promise<{ code: number | null; output: string }>;

export type TargetRepoVerificationCheck = {
kind: "test" | "lint" | "build";
command: string;
ok: boolean;
exitCode: number | null;
outputTail: string;
};

export type TargetRepoVerificationResult =
| { status: "passed"; checks: TargetRepoVerificationCheck[] }
| { status: "failed"; checks: TargetRepoVerificationCheck[]; firstFailure: TargetRepoVerificationCheck }
| { status: "skipped"; reason: "stack_undetected" | "no_commands_detected" | "disabled" };

/** Per-command wall-clock bound. A target repo's test suite legitimately runs minutes; 10 is the ceiling
* before the gate itself becomes the attempt's bottleneck — a suite slower than this is skipped territory
* for a future per-repo override, not something to silently wait out. */
export const DEFAULT_VERIFICATION_TIMEOUT_MS = 10 * 60 * 1000;
/** Postmortem detail bound — enough tail to show the failing assertion, never an unbounded log dump. */
export const VERIFICATION_OUTPUT_TAIL_CHARS = 4000;

/** Post-timeout grace before giving up on the `close` event: a killed process group's pipes close nearly
* instantly, so this only fires when something double-forked out of the group and kept the pipes open —
* the gate resolves as failed rather than hanging on that orphan. */
export const VERIFICATION_KILL_SETTLE_MS = 5000;

/** Kill the command's whole detached process group via the NEGATIVE pid: a test command is routinely a tree
* (`npm test` → node → workers), and killing only the shell leaves grandchildren holding the stdio pipes —
* the `close` event then waits on THEM, stalling the gate far past its own timeout (observed as the 30s
* hang on Linux CI). Falls back to the plain single-process kill when the group kill isn't possible
* (no pid, or the group is already gone and the signal throws). */
export function killVerificationProcessTree(child: KillableChild, killGroup: (pid: number, signal: NodeJS.Signals) => void = (pid, signal) => process.kill(-pid, signal)): void {
try {
if (typeof child.pid !== "number") throw new Error("child has no pid");
killGroup(child.pid, "SIGKILL");
} catch {
child.kill("SIGKILL");
}
}

/** Default spawn: shell-executed (detected commands are shell strings like "npm test" / "ruff check ."),
* merged stdout+stderr, killed at the timeout (a killed/timed-out command reports code null → treated as
* failure upstream). `internals` exists ONLY for tests to reach the timeout/settle arms deterministically;
* production callers always take the defaults. */
export function runShellCommandWithTreeKill(
command: string,
options: { cwd: string; timeoutMs: number },
internals: { killTree?: (child: KillableChild) => void; settleMs?: number } = {},
): Promise<{ code: number | null; output: string }> {
const killTree = internals.killTree ?? killVerificationProcessTree;
const settleMs = internals.settleMs ?? VERIFICATION_KILL_SETTLE_MS;
return new Promise((resolve) => {
// detached: its own process group, so the timeout can kill the entire tree, not just the shell.
const child = nodeSpawn(command, { cwd: options.cwd, shell: true, stdio: ["ignore", "pipe", "pipe"], detached: true });
let output = "";
let settled = false;
let settleTimer: ReturnType<typeof setTimeout> | undefined;
const finish = (result: { code: number | null; output: string }) => {
if (settled) return;
settled = true;
clearTimeout(timer);
if (settleTimer !== undefined) clearTimeout(settleTimer);
resolve(result);
};
const capture = (chunk: Buffer) => {
output = (output + chunk.toString()).slice(-VERIFICATION_OUTPUT_TAIL_CHARS * 4);
};
child.stdout?.on("data", capture);
child.stderr?.on("data", capture);
const timer = setTimeout(() => {
killTree(child);
// Bounded settle: if some orphan still holds the pipes open after the group kill, resolve as a
// timeout failure anyway — the verification gate must never outlive its own per-command bound.
settleTimer = setTimeout(() => {
finish({ code: null, output: `${output}\n[verification timeout after ${options.timeoutMs}ms — process tree killed]` });
}, settleMs);
}, options.timeoutMs);
child.on("error", (error) => {
finish({ code: null, output: `${output}\n${String(error)}` });
});
child.on("close", (code) => {
finish({ code, output });
});
});
}

export const defaultVerificationSpawn: TargetRepoVerificationSpawn = (command, options) => runShellCommandWithTreeKill(command, options);

export async function runTargetRepoVerification(options: {
worktreeDir: string;
stack: RepoStackResult;
spawn?: TargetRepoVerificationSpawn;
timeoutMsPerCommand?: number;
}): Promise<TargetRepoVerificationResult> {
const stack = options.stack;
if (stack.detected !== true) return { status: "skipped", reason: "stack_undetected" };
const commands: Array<{ kind: TargetRepoVerificationCheck["kind"]; command: string | null }> = [
{ kind: "test", command: stack.testCommand },
{ kind: "lint", command: stack.lintCommand },
{ kind: "build", command: stack.buildCommand },
];
const runnable = commands.filter((entry): entry is { kind: TargetRepoVerificationCheck["kind"]; command: string } => entry.command !== null);
if (runnable.length === 0) return { status: "skipped", reason: "no_commands_detected" };

const spawn = options.spawn ?? defaultVerificationSpawn;
const timeoutMs = options.timeoutMsPerCommand ?? DEFAULT_VERIFICATION_TIMEOUT_MS;
const checks: TargetRepoVerificationCheck[] = [];
for (const { kind, command } of runnable) {
const { code, output } = await spawn(command, { cwd: options.worktreeDir, timeoutMs });
const check: TargetRepoVerificationCheck = {
kind,
command,
ok: code === 0,
exitCode: code,
outputTail: output.slice(-VERIFICATION_OUTPUT_TAIL_CHARS),
};
checks.push(check);
// Stop at the first failure: the remaining commands' results would only pile noise onto an attempt that
// is already not submitting, and a broken build often cascades into misleading downstream failures.
if (!check.ok) return { status: "failed", checks, firstFailure: check };
}
return { status: "passed", checks };
}
74 changes: 74 additions & 0 deletions test/unit/miner-attempt-cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2670,3 +2670,77 @@ describe("resolveAttemptHouseRulesConfig (#8806)", () => {
expect(close).toHaveBeenCalled();
});
});

describe("target-repo verification wiring (#8807)", () => {
it("binds verifyTargetRepo into the runner deps (worktree-scoped thunk over the injected verifier)", async () => {
const { allocator, claimLedger, eventLedger, attemptLog, governorLedger } = tempLedgers();
vi.spyOn(console, "log").mockImplementation(() => undefined);
const runMinerAttemptSpy = vi.fn(async (_input: unknown, deps: { verifyTargetRepo?: () => Promise<unknown> }) => {
// The thunk exists and resolves through the injected verifier when invoked.
expect(typeof deps.verifyTargetRepo).toBe("function");
const verification = await deps.verifyTargetRepo!();
expect(verification).toEqual({ status: "skipped", reason: "stack_undetected" });
return { outcome: "abandon", loopResult: { outcome: "abandon", iterations: [], finalMeterTotals: { tokens: 0 } } };
});
const runVerifier = vi.fn(async (opts: { worktreeDir: string }) => {
expect(opts.worktreeDir).toBeTruthy(); // bound to THIS attempt's worktree
return { status: "skipped" as const, reason: "stack_undetected" as const };
});

await runAttempt(["acme/widgets", "7", "--miner-login", "alice", "--json"], {
env: { MINER_CODING_AGENT_PROVIDER: "noop" },
openWorktreeAllocator: () => allocator,
openClaimLedger: () => claimLedger,
initEventLedger: () => eventLedger,
initAttemptLog: () => attemptLog,
initGovernorLedger: () => governorLedger,
...readyPipelineOptions({ runMinerAttempt: runMinerAttemptSpy, runTargetRepoVerification: runVerifier }),
});

expect(runMinerAttemptSpy).toHaveBeenCalled();
expect(runVerifier).toHaveBeenCalled();
});

it("without an injected verifier the thunk runs the REAL verification (default arm) — an unmarked temp worktree skips", async () => {
const { allocator, claimLedger, eventLedger, attemptLog, governorLedger } = tempLedgers();
vi.spyOn(console, "log").mockImplementation(() => undefined);
const runMinerAttemptSpy = vi.fn(async (_input: unknown, deps: { verifyTargetRepo?: () => Promise<{ status: string }> }) => {
const verification = await deps.verifyTargetRepo!();
expect(verification.status).toBe("skipped"); // no stack markers in the fixture worktree
return { outcome: "abandon", loopResult: { outcome: "abandon", iterations: [], finalMeterTotals: { tokens: 0 } } };
});

await runAttempt(["acme/widgets", "7", "--miner-login", "alice", "--json"], {
env: { MINER_CODING_AGENT_PROVIDER: "noop" },
openWorktreeAllocator: () => allocator,
openClaimLedger: () => claimLedger,
initEventLedger: () => eventLedger,
initAttemptLog: () => attemptLog,
initGovernorLedger: () => governorLedger,
...readyPipelineOptions({ runMinerAttempt: runMinerAttemptSpy }),
});

expect(runMinerAttemptSpy).toHaveBeenCalled();
});

it("MINER_SKIP_TARGET_REPO_VERIFICATION omits the thunk entirely — the documented escape hatch", async () => {
const { allocator, claimLedger, eventLedger, attemptLog, governorLedger } = tempLedgers();
vi.spyOn(console, "log").mockImplementation(() => undefined);
const runMinerAttemptSpy = vi.fn(async (_input: unknown, deps: { verifyTargetRepo?: unknown }) => {
expect(deps.verifyTargetRepo).toBeUndefined();
return { outcome: "abandon", loopResult: { outcome: "abandon", iterations: [], finalMeterTotals: { tokens: 0 } } };
});

await runAttempt(["acme/widgets", "7", "--miner-login", "alice", "--json"], {
env: { MINER_CODING_AGENT_PROVIDER: "noop", MINER_SKIP_TARGET_REPO_VERIFICATION: "1" },
openWorktreeAllocator: () => allocator,
openClaimLedger: () => claimLedger,
initEventLedger: () => eventLedger,
initAttemptLog: () => attemptLog,
initGovernorLedger: () => governorLedger,
...readyPipelineOptions({ runMinerAttempt: runMinerAttemptSpy }),
});

expect(runMinerAttemptSpy).toHaveBeenCalled();
});
});
Loading
Loading