diff --git a/packages/engine/src/__tests__/mission-execution-loop.test.ts b/packages/engine/src/__tests__/mission-execution-loop.test.ts index 640c41ccb..28ec984d5 100644 --- a/packages/engine/src/__tests__/mission-execution-loop.test.ts +++ b/packages/engine/src/__tests__/mission-execution-loop.test.ts @@ -6,6 +6,10 @@ */ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { execSync, spawnSync } from "node:child_process"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import { TEST_MODE_RESOLVED } from "@fusion/core"; import type { Mission, @@ -472,6 +476,25 @@ function expectNoValidationBoardTaskMutation(taskStore: ReturnType { let loop: MissionExecutionLoop; let missionStore: ReturnType; @@ -1881,6 +1904,16 @@ describe("MissionExecutionLoop", () => { // ── premerge guard (fail verdict while the linked task is unmerged) ────── describe("premerge guard", () => { + const gitRepos: string[] = []; + afterEach(() => { + for (const dir of gitRepos.splice(0)) rmSync(dir, { recursive: true, force: true }); + }); + function makeGitRepo(): string { + const repo = initGitRepo(); + gitRepos.push(repo); + return repo; + } + function primeFailVerdict() { const failResponse = JSON.stringify({ status: "fail", @@ -2012,6 +2045,144 @@ describe("MissionExecutionLoop", () => { expect.objectContaining({ featureId: "F-001" }), ); }); + + // ── stale-workspace guard (task done, but rootDir predates the merge) ──── + + (hasGit ? it : it.skip)( + "should defer a fail when the judged checkout predates the merged commit", + async () => { + primeFeature(); + primeFailVerdict(); + + // rootDir HEAD is `main` @ commit1. The merged commit lives on a side + // branch and is NOT reachable from HEAD — exactly the stale-checkout + // case (merge landed on remote / another worktree, rootDir never reset). + const repo = makeGitRepo(); + git(repo, "git checkout -q -b feature"); + writeFileSync(join(repo, "foo.ts"), "line1\nmerged\n"); + git(repo, "git add foo.ts && git commit -m merged"); + const mergedSha = git(repo, "git rev-parse HEAD"); + git(repo, "git checkout -q main"); + + // Column is `done`, so the premerge column guard passes; only the + // ancestry check can catch the stale workspace. + taskStore._setTask({ id: "FN-001", title: "Test", description: "d", log: [], column: "done", integrationSha: mergedSha } as any); + + loop = new MissionExecutionLoop({ + taskStore: taskStore as any, + missionStore: missionStore as any, + rootDir: repo, + }); + const emitSpy = vi.spyOn(loop, "emit"); + loop.start(); + + await loop.processTaskOutcome("FN-001"); + + expect(missionStore.createGeneratedFixFeature).not.toHaveBeenCalled(); + expect(missionStore.completeValidatorRun).toHaveBeenCalledWith( + expect.any(String), + "blocked", + expect.stringContaining("predates the merged code"), + ); + expect(emitSpy).toHaveBeenCalledWith( + "validation:inconclusive", + expect.objectContaining({ + featureId: "F-001", + reason: expect.stringContaining("predates the merged code"), + }), + ); + expect(emitSpy).not.toHaveBeenCalledWith("validation:failed", expect.anything()); + }, + ); + + (hasGit ? it : it.skip)( + "should run the normal fail path when the merged commit is an ancestor of HEAD", + async () => { + primeFeature(); + primeFailVerdict(); + + // rootDir HEAD advanced PAST the merged commit — the workspace is fresh, + // so the fail is real and must mint a Fix Feature. + const repo = makeGitRepo(); + const baseSha = git(repo, "git rev-parse HEAD"); + writeFileSync(join(repo, "foo.ts"), "line1\nadvanced\n"); + git(repo, "git add foo.ts && git commit -m advance"); + + taskStore._setTask({ id: "FN-001", title: "Test", description: "d", log: [], column: "done", integrationSha: baseSha } as any); + + loop = new MissionExecutionLoop({ + taskStore: taskStore as any, + missionStore: missionStore as any, + rootDir: repo, + }); + const emitSpy = vi.spyOn(loop, "emit"); + loop.start(); + + await loop.processTaskOutcome("FN-001"); + + expect(missionStore.createGeneratedFixFeature).toHaveBeenCalled(); + expect(emitSpy).toHaveBeenCalledWith( + "validation:failed", + expect.objectContaining({ featureId: "F-001" }), + ); + expect(emitSpy).not.toHaveBeenCalledWith("validation:inconclusive", expect.anything()); + }, + ); + + it("should fail open (normal fail) when the task carries no integration SHA", async () => { + primeFeature(); + primeFailVerdict(); + + // Done, but no integrationSha/baseCommit → no evidence of staleness. + taskStore._setTask({ id: "FN-001", title: "Test", description: "d", log: [], column: "done" }); + + loop = new MissionExecutionLoop({ + taskStore: taskStore as any, + missionStore: missionStore as any, + rootDir: "/tmp", + }); + const emitSpy = vi.spyOn(loop, "emit"); + loop.start(); + + await loop.processTaskOutcome("FN-001"); + + expect(missionStore.createGeneratedFixFeature).toHaveBeenCalled(); + expect(emitSpy).toHaveBeenCalledWith( + "validation:failed", + expect.objectContaining({ featureId: "F-001" }), + ); + expect(emitSpy).not.toHaveBeenCalledWith("validation:inconclusive", expect.anything()); + }); + + (hasGit ? it : it.skip)( + "should fail open (normal fail) when the integration SHA is an unknown object", + async () => { + primeFeature(); + primeFailVerdict(); + + // A bogus SHA makes `git merge-base --is-ancestor` exit 128 (bad object), + // which is unknown → must NOT suppress the fail. + const repo = makeGitRepo(); + taskStore._setTask({ id: "FN-001", title: "Test", description: "d", log: [], column: "done", integrationSha: "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef" } as any); + + loop = new MissionExecutionLoop({ + taskStore: taskStore as any, + missionStore: missionStore as any, + rootDir: repo, + }); + const emitSpy = vi.spyOn(loop, "emit"); + loop.start(); + + await loop.processTaskOutcome("FN-001"); + + expect(missionStore.createGeneratedFixFeature).toHaveBeenCalled(); + expect(emitSpy).toHaveBeenCalledWith( + "validation:failed", + expect.objectContaining({ featureId: "F-001" }), + ); + expect(emitSpy).not.toHaveBeenCalledWith("validation:inconclusive", expect.anything()); + }, + ); }); // ── handleValidationBlocked ─────────────────────────────────────────────── diff --git a/packages/engine/src/mission-execution-loop.ts b/packages/engine/src/mission-execution-loop.ts index e2265f48d..d6c7c2111 100644 --- a/packages/engine/src/mission-execution-loop.ts +++ b/packages/engine/src/mission-execution-loop.ts @@ -36,6 +36,17 @@ import { createLogger } from "./logger.js"; import { createFallbackModelObserver } from "./fallback-model-observer.js"; import { resolveMcpServersForStore } from "./mcp-resolution.js"; import { createRunAuditor, generateSyntheticRunId } from "./run-audit.js"; +import { exec } from "node:child_process"; +import { promisify } from "node:util"; + +const execAsync = promisify(exec); + +/** Shell-quote a single argument for a `git` invocation (mirror of the local + * helper in branch-conflicts.ts — kept local rather than shared per repo + * convention). */ +function quoteShellArg(value: string): string { + return `'${value.replace(/'/g, `'\\''`)}'`; +} /** Logger for the mission execution loop subsystem. */ export const loopLog = createLogger("mission-loop"); @@ -512,6 +523,18 @@ export class MissionExecutionLoop extends EventEmitter { run.id, `linked task ${feature.taskId} is still "${premergeColumn}" (code not merged yet) — validation deferred`, ); + } else if (await this.isValidationWorkspaceStale(feature)) { + // Even when the task column shows "done", the judge session ran in + // this.rootDir — if that working copy never fetched/reset to the + // merged commit (merge landed on remote or another worktree), the + // validator read PRE-merge files → a spurious fail. Defer instead of + // minting a bogus Fix Feature; a later validation judges the merged + // code. Only affirmative staleness evidence defers (fail-open). + await this.handleValidationInconclusive( + feature.id, + run.id, + `validation workspace predates the merged code for ${feature.taskId} — validation deferred`, + ); } else { await this.handleValidationFail(feature.id, run.id, result); } @@ -547,6 +570,30 @@ export class MissionExecutionLoop extends EventEmitter { return column; } + /** + * Affirmative-evidence check that the judged checkout (this.rootDir HEAD) + * predates the linked task's merged code. True ONLY when the integration SHA + * is resolvable AND is NOT an ancestor of rootDir HEAD. Fails open (returns + * false = trust the fail) on unresolvable SHA / unknown object / any git + * error — the guard may only ever defer a fail, never suppress one. + */ + private async isValidationWorkspaceStale(feature: MissionFeature): Promise { + const integrationSha = await this.resolveIntegrationSha(feature); + if (!integrationSha) return false; // no evidence → trust the fail + try { + await execAsync(`git merge-base --is-ancestor ${quoteShellArg(integrationSha)} HEAD`, { + cwd: this.rootDir, + timeout: 30_000, + }); + return false; // exit 0 → ancestor → workspace is fresh + } catch (err) { + // `--is-ancestor` exits 1 = NOT an ancestor (affirmatively stale); exit + // 128 = bad object / not a repo (unknown → fail-open). execAsync's thrown + // error carries `.code` = process exit code. ONLY code === 1 defers. + return (err as { code?: number })?.code === 1; + } + } + /** * Run the validation AI session for a feature. *