From 28878d8125197ba042321d5d1aa32fc91dcceeb3 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 17 Jul 2026 10:00:18 -0700 Subject: [PATCH] FN-8206: validate landed merge checkout Validate mission assertions against the delivered merge revision rather than an ambient working branch. - Materialize and dispose a detached checkout at mergeDetails.commitSha for validator sessions. - Defer failed validation when landed-code ancestry cannot be proven, preventing spurious fix features. - Link validator runs to their board tasks and document the inspection behavior. Files changed: .changeset/fn-8206-mission-validator-inspection-root.md | 7 + docs/missions.md | 2 + packages/engine/src/__tests__/mission-execution-loop.test.ts | 278 +++++++++++++-------- packages/engine/src/mission-execution-loop.ts | 160 ++++++++---- 4 files changed, 298 insertions(+), 149 deletions(-) Fusion-Task-Id: FN-8206 Fusion-Task-Lineage: dfd1eb21-fcdf-4722-acb8-416750f4f40b Co-authored-by: Fusion (runfusion.ai) --- ...-8206-mission-validator-inspection-root.md | 7 + docs/missions.md | 2 + .../__tests__/mission-execution-loop.test.ts | 278 +++++++++++------- packages/engine/src/mission-execution-loop.ts | 160 ++++++---- 4 files changed, 298 insertions(+), 149 deletions(-) create mode 100644 .changeset/fn-8206-mission-validator-inspection-root.md diff --git a/.changeset/fn-8206-mission-validator-inspection-root.md b/.changeset/fn-8206-mission-validator-inspection-root.md new file mode 100644 index 0000000000..c5d85214d7 --- /dev/null +++ b/.changeset/fn-8206-mission-validator-inspection-root.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Mission feature validator now inspects the merged commit and defers instead of false-failing on branch divergence. +category: fix +dev: runValidation materializes a disposable detached checkout of mergeDetails.commitSha (never baseCommitSha) and computes the stale-workspace ancestry guard against that inspection root before disposal; startValidatorRun now carries taskId. diff --git a/docs/missions.md b/docs/missions.md index 98a6b696c4..16fe5084e4 100644 --- a/docs/missions.md +++ b/docs/missions.md @@ -490,6 +490,8 @@ On task completion, the scheduler calls `MissionExecutionLoop.processTaskOutcome 5. Apply the **behavioral-verification posture** (see below): static assertions keep the judge's verdict; behavioral/bug assertions default to fail until a bounded, non-mutating verification run confirms them 6. Record `MissionValidatorRun` metadata for the validation attempt (per-assertion failures are stored separately in `MissionAssertionFailureRecord` rows) +For a linked task with a recorded `mergeDetails.commitSha`, the read-only judge runs from a disposable detached checkout of that landed merge revision rather than the ambient project checkout. If that checkout cannot be materialized, the judge falls back to the project root; a fail is deferred to **inconclusive** when the landed commit is not reachable from that same inspected root, or when the landed revision/its ancestry cannot be verified, preventing a branch-divergence false failure. The task worktree fork point (`baseCommitSha`) is never used as an inspection revision. + **Behavioral-verification posture (adversarial default-to-fail).** A Contract Assertion now carries a `type` (`static` | `behavioral`). The validator no longer grades a Feature "done" purely from the diff's apparent intent: - **Static assertions** (e.g. "documented in README") keep today's read-only static judging — no added cost or strictness. diff --git a/packages/engine/src/__tests__/mission-execution-loop.test.ts b/packages/engine/src/__tests__/mission-execution-loop.test.ts index dcd8d9c1a3..f6f8ed026f 100644 --- a/packages/engine/src/__tests__/mission-execution-loop.test.ts +++ b/packages/engine/src/__tests__/mission-execution-loop.test.ts @@ -972,12 +972,15 @@ describe("MissionExecutionLoop", () => { missionStore: missionStore as any, rootDir: "/tmp", }); - vi.spyOn(loop as any, "runValidation").mockResolvedValue({ status: "pass", summary: "ok" }); + vi.spyOn(loop as any, "runValidation").mockResolvedValue({ + result: { status: "pass", summary: "ok" }, + inspection: { inspectionRoot: "/tmp", landedSha: undefined, fallbackUsed: true, workspaceStale: false }, + }); loop.start(); await loop.processTaskOutcome("FN-001"); - expect(missionStore.startValidatorRun).toHaveBeenCalledWith("F-001", "task_completion"); + expect(missionStore.startValidatorRun).toHaveBeenCalledWith("F-001", "task_completion", "FN-001"); }); it("requeues needs_fix features back through validation", async () => { @@ -1035,13 +1038,16 @@ describe("MissionExecutionLoop", () => { rootDir: "/tmp", }); const emitSpy = vi.spyOn(loop, "emit"); - vi.spyOn(loop as any, "runValidation").mockResolvedValue({ status: "pass", summary: "ok" }); + vi.spyOn(loop as any, "runValidation").mockResolvedValue({ + result: { status: "pass", summary: "ok" }, + inspection: { inspectionRoot: "/tmp", landedSha: undefined, fallbackUsed: true, workspaceStale: false }, + }); loop.start(); await loop.processTaskOutcome("FN-001"); expect(missionStore.ensureFeatureAssertionLinked).toHaveBeenCalledWith("F-001"); - expect(missionStore.startValidatorRun).toHaveBeenCalledWith("F-001", "task_completion"); + expect(missionStore.startValidatorRun).toHaveBeenCalledWith("F-001", "task_completion", "FN-001"); expect(emitSpy).toHaveBeenCalledWith( "validation:passed", expect.objectContaining({ featureId: "F-001" }), @@ -1108,7 +1114,7 @@ describe("MissionExecutionLoop", () => { await loop.processTaskOutcome("FN-LATER"); expect(missionStore.listAssertionsForFeature).toHaveBeenCalledWith("F-LATER"); - expect(missionStore.startValidatorRun).toHaveBeenCalledWith("F-LATER", "task_completion"); + expect(missionStore.startValidatorRun).toHaveBeenCalledWith("F-LATER", "task_completion", "FN-LATER"); }); it("threads milestone acceptance criteria into validator prompts", () => { @@ -1168,7 +1174,7 @@ describe("MissionExecutionLoop", () => { await loop.processTaskOutcome("FN-001"); expect(taskStore.createTask).toHaveBeenCalledTimes(0); - expect(missionStore.startValidatorRun).toHaveBeenCalledWith("F-001", "task_completion"); + expect(missionStore.startValidatorRun).toHaveBeenCalledWith("F-001", "task_completion", "FN-001"); }); it("does NOT set mission-validation status on any task", async () => { @@ -1226,7 +1232,7 @@ describe("MissionExecutionLoop", () => { ); }); - it("calls startValidatorRun without a board task ID", async () => { + it("threads the linked board task ID into startValidatorRun", async () => { const feature = createMockFeature({ loopState: "implementing", taskId: "FN-001", sliceId: "SL-001" }); missionStore._setFeature(feature); taskStore._setTask({ id: "FN-001", title: "Test", description: "Test task", log: [] }); @@ -1247,6 +1253,7 @@ describe("MissionExecutionLoop", () => { expect(missionStore.startValidatorRun).toHaveBeenCalledWith( "F-001", "task_completion", + "FN-001", ); }); @@ -1417,14 +1424,14 @@ describe("MissionExecutionLoop", () => { await loop.processTaskOutcome("FN-ASSERT-PASS"); - expect(missionStore.startValidatorRun).toHaveBeenCalledWith("F-001", "task_completion"); + expect(missionStore.startValidatorRun).toHaveBeenCalledWith("F-001", "task_completion", "FN-ASSERT-PASS"); expect(missionStore.completeValidatorRun).toHaveBeenCalledWith(expect.any(String), "passed", expect.any(String)); expect(missionStore.getFeature("F-001")?.loopState).toBe("passed"); expect(missionStore.getFeature("F-001")?.lastValidatorStatus).toBe("passed"); expect(missionStore.updateFeatureStatus).toHaveBeenCalledWith("F-001", "done"); }); - it("routes failed assertion validation to fix flow and does not pass feature", async () => { + it("defers failed assertion validation when landed-code inspection is unavailable", async () => { const feature = createMockFeature({ loopState: "implementing", taskId: "FN-ASSERT-FAIL", status: "in-progress" }); missionStore._setFeature(feature); missionStore.getFeatureByTaskId = vi.fn().mockReturnValue(feature); @@ -1443,10 +1450,10 @@ describe("MissionExecutionLoop", () => { await loop.processTaskOutcome("FN-ASSERT-FAIL"); - expect(missionStore.completeValidatorRun).toHaveBeenCalledWith(expect.any(String), "failed", expect.any(String)); - expect(missionStore.createGeneratedFixFeature).toHaveBeenCalled(); - expect(missionStore.getFeature("F-001")?.lastValidatorStatus).toBe("failed"); - expect(missionStore.getFeature("F-001")?.loopState).toBe("implementing"); + expect(missionStore.completeValidatorRun).toHaveBeenCalledWith(expect.any(String), "blocked", expect.any(String)); + expect(missionStore.createGeneratedFixFeature).not.toHaveBeenCalled(); + expect(missionStore.getFeature("F-001")?.lastValidatorStatus).toBe("blocked"); + expect(missionStore.getFeature("F-001")?.loopState).toBe("blocked"); expect(missionStore.getFeature("F-001")?.status).not.toBe("done"); }); }); @@ -1669,24 +1676,21 @@ describe("MissionExecutionLoop", () => { await loop.processTaskOutcome("FN-001"); - // Should emit validation:failed + // The parser still returns fail, but routing must defer because this + // fixture has no verifiable landed merge revision. expect(emitSpy).toHaveBeenCalledWith( - "validation:failed", + "validation:inconclusive", expect.objectContaining({ featureId: "F-001" }), ); - - // recordValidatorFailures should be called - expect(missionStore.recordValidatorFailures).toHaveBeenCalled(); - - // completeValidatorRun should be called with failed + expect(missionStore.recordValidatorFailures).not.toHaveBeenCalled(); expect(missionStore.completeValidatorRun).toHaveBeenCalledWith( expect.any(String), - "failed", + "blocked", expect.any(String), ); - // createGeneratedFixFeature should be called - expect(missionStore.createGeneratedFixFeature).toHaveBeenCalled(); + // No remediation is created until the judge can inspect landed code. + expect(missionStore.createGeneratedFixFeature).not.toHaveBeenCalled(); expectNoValidationBoardTaskMutation(taskStore); }); @@ -1772,13 +1776,16 @@ describe("MissionExecutionLoop", () => { }, }); const emitSpy = vi.spyOn(loop, "emit"); - vi.spyOn(loop as any, "runValidation").mockResolvedValue({ status: "pass", summary: "ok" }); + vi.spyOn(loop as any, "runValidation").mockResolvedValue({ + result: { status: "pass", summary: "ok" }, + inspection: { inspectionRoot: "/tmp", landedSha: undefined, fallbackUsed: true, workspaceStale: false }, + }); loop.start(); await loop.processTaskOutcome("FN-001"); expect(missionStore.ensureFeatureAssertionLinked).toHaveBeenCalledWith("F-001"); - expect(missionStore.startValidatorRun).toHaveBeenCalledWith("F-001", "task_completion"); + expect(missionStore.startValidatorRun).toHaveBeenCalledWith("F-001", "task_completion", "FN-001"); // validation:passed event emitted expect(emitSpy).toHaveBeenCalledWith( @@ -1885,43 +1892,21 @@ describe("MissionExecutionLoop", () => { await loop.processTaskOutcome("FN-001"); - // recordValidatorFailures called - expect(missionStore.recordValidatorFailures).toHaveBeenCalled(); - - // completeValidatorRun called with failed + expect(missionStore.recordValidatorFailures).not.toHaveBeenCalled(); expect(missionStore.completeValidatorRun).toHaveBeenCalledWith( expect.any(String), - "failed", + "blocked", expect.any(String), ); - - // createGeneratedFixFeature called (U6: now also receives the - // observed-vs-expected failure reason as a 4th argument, R6). - expect(missionStore.createGeneratedFixFeature).toHaveBeenCalledWith( - "F-001", - expect.any(String), - expect.arrayContaining(["CA-1"]), - expect.any(String), - ); - - // triageFeature called for the fix feature - expect(missionStore.triageFeature).toHaveBeenCalledWith( - expect.stringContaining("FIX-"), - ); - - // validation:failed event emitted + expect(missionStore.createGeneratedFixFeature).not.toHaveBeenCalled(); + expect(missionStore.triageFeature).not.toHaveBeenCalled(); expect(emitSpy).toHaveBeenCalledWith( - "validation:failed", - expect.objectContaining({ - featureId: "F-001", - failures: expect.arrayContaining([ - expect.objectContaining({ assertionId: "CA-1" }), - ]), - }), + "validation:inconclusive", + expect.objectContaining({ featureId: "F-001" }), ); }); - it("should emit validation:failed even if triageFeature throws", async () => { + it("does not triage a fix when landed-code inspection is unavailable", async () => { const assertions: MissionContractAssertion[] = [ { id: "CA-1", @@ -1972,15 +1957,10 @@ describe("MissionExecutionLoop", () => { await loop.processTaskOutcome("FN-001"); - // triageFeature was called but threw - expect(missionStore.triageFeature).toHaveBeenCalledWith(expect.stringContaining("FIX-")); - - // validation:failed event should still be emitted + expect(missionStore.triageFeature).not.toHaveBeenCalled(); expect(emitSpy).toHaveBeenCalledWith( - "validation:failed", - expect.objectContaining({ - featureId: "F-001", - }), + "validation:inconclusive", + expect.objectContaining({ featureId: "F-001" }), ); }); }); @@ -2070,7 +2050,7 @@ describe("MissionExecutionLoop", () => { ); }); - it("should run the normal fail path once the linked task is done", async () => { + it("defers a done-task fail until the landed merge can be verified", async () => { primeFeature(); primeFailVerdict(); @@ -2086,20 +2066,14 @@ describe("MissionExecutionLoop", () => { await loop.processTaskOutcome("FN-001"); - expect(missionStore.createGeneratedFixFeature).toHaveBeenCalledWith( - "F-001", - expect.any(String), - expect.arrayContaining(["CA-1"]), - expect.any(String), - ); + expect(missionStore.createGeneratedFixFeature).not.toHaveBeenCalled(); expect(emitSpy).toHaveBeenCalledWith( - "validation:failed", + "validation:inconclusive", expect.objectContaining({ featureId: "F-001" }), ); - expect(emitSpy).not.toHaveBeenCalledWith("validation:inconclusive", expect.anything()); }); - it("should fail open (normal fail path) when the linked task cannot be read", async () => { + it("defers a fail when the linked task's landed revision cannot be resolved", async () => { primeFeature(); loop = new MissionExecutionLoop({ @@ -2110,9 +2084,18 @@ describe("MissionExecutionLoop", () => { // Bypass the AI session (runValidation also reads the task, without a // catch) so the rejecting getTask below only exercises the guard. vi.spyOn(loop as any, "runValidation").mockResolvedValue({ - status: "fail", - assertions: [{ assertionId: "CA-1", passed: false, message: "Failed", expected: "ok", actual: "not ok" }], - summary: "Assertion failed", + result: { + status: "fail", + assertions: [{ assertionId: "CA-1", passed: false, message: "Failed", expected: "ok", actual: "not ok" }], + summary: "Assertion failed", + }, + inspection: { + inspectionRoot: "/tmp", + landedSha: undefined, + fallbackUsed: true, + workspaceStale: false, + inspectionUnavailableReason: "landed merge SHA is unavailable", + }, }); taskStore.getTask = vi.fn().mockRejectedValue(new Error("store unavailable")); @@ -2121,12 +2104,10 @@ describe("MissionExecutionLoop", () => { await loop.processTaskOutcome("FN-001"); - // Unknown task state must never suppress a fail — only defer on - // affirmative evidence of an unmerged column. - expect(missionStore.createGeneratedFixFeature).toHaveBeenCalled(); + expect(missionStore.createGeneratedFixFeature).not.toHaveBeenCalled(); expect(emitSpy).toHaveBeenCalledWith( - "validation:failed", - expect.objectContaining({ featureId: "F-001" }), + "validation:inconclusive", + expect.objectContaining({ featureId: "F-001", reason: expect.stringContaining("could not prove") }), ); }); @@ -2150,12 +2131,23 @@ describe("MissionExecutionLoop", () => { // 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); + taskStore._setTask({ + id: "FN-001", + title: "Test", + description: "d", + log: [], + column: "done", + mergeDetails: { commitSha: mergedSha }, + } as any); loop = new MissionExecutionLoop({ taskStore: taskStore as any, missionStore: missionStore as any, rootDir: repo, + checkoutMaterializer: { + materialize: vi.fn().mockRejectedValue(new Error("simulated checkout failure")), + assertSourceClean: vi.fn(), + }, }); const emitSpy = vi.spyOn(loop, "emit"); loop.start(); @@ -2192,7 +2184,7 @@ describe("MissionExecutionLoop", () => { 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); + taskStore._setTask({ id: "FN-001", title: "Test", description: "d", log: [], column: "done", mergeDetails: { commitSha: baseSha } } as any); loop = new MissionExecutionLoop({ taskStore: taskStore as any, @@ -2213,11 +2205,12 @@ describe("MissionExecutionLoop", () => { }, ); - it("should fail open (normal fail) when the task carries no integration SHA", async () => { + it("defers a fail when the task has no landed merge SHA", async () => { primeFeature(); primeFailVerdict(); - // Done, but no integrationSha/baseCommit → no evidence of staleness. + // Done, but no mergeDetails.commitSha means the inspected tree cannot + // be proven to contain delivered code. taskStore._setTask({ id: "FN-001", title: "Test", description: "d", log: [], column: "done" }); loop = new MissionExecutionLoop({ @@ -2230,24 +2223,111 @@ describe("MissionExecutionLoop", () => { await loop.processTaskOutcome("FN-001"); - expect(missionStore.createGeneratedFixFeature).toHaveBeenCalled(); + expect(missionStore.createGeneratedFixFeature).not.toHaveBeenCalled(); expect(emitSpy).toHaveBeenCalledWith( - "validation:failed", - expect.objectContaining({ featureId: "F-001" }), + "validation:inconclusive", + expect.objectContaining({ featureId: "F-001", reason: expect.stringContaining("could not prove") }), ); - expect(emitSpy).not.toHaveBeenCalledWith("validation:inconclusive", expect.anything()); + }); + + it("pins the judge and stale check to the disposable landed-merge checkout", async () => { + primeFeature(); + primeFailVerdict(); + const dispose = vi.fn().mockResolvedValue(undefined); + const materialize = vi.fn().mockResolvedValue({ dir: "/inspection/landed", dispose }); + taskStore._setTask({ + id: "FN-001", + title: "Test", + description: "d", + log: [], + column: "done", + mergeDetails: { commitSha: "landed-sha" }, + } as any); + loop = new MissionExecutionLoop({ + taskStore: taskStore as any, + missionStore: missionStore as any, + rootDir: "/ambient-root", + checkoutMaterializer: { materialize, assertSourceClean: vi.fn() }, + }); + const staleCheck = vi.spyOn(loop as any, "isValidationWorkspaceStale").mockResolvedValue({ workspaceStale: false }); + loop.start(); + + await loop.processTaskOutcome("FN-001"); + + expect(materialize).toHaveBeenCalledWith("/ambient-root", "landed-sha"); + expect(createResolvedAgentSession).toHaveBeenCalledWith(expect.objectContaining({ cwd: "/inspection/landed" })); + expect(staleCheck).toHaveBeenCalledWith("landed-sha", "/inspection/landed"); + expect(dispose).toHaveBeenCalledOnce(); + }); + + it("defers a fallback-root fail when its landed merge is absent", async () => { + primeFeature(); + primeFailVerdict(); + taskStore._setTask({ + id: "FN-001", + title: "Test", + description: "d", + log: [], + column: "done", + mergeDetails: { commitSha: "landed-sha" }, + } as any); + loop = new MissionExecutionLoop({ + taskStore: taskStore as any, + missionStore: missionStore as any, + rootDir: "/ambient-root", + checkoutMaterializer: { materialize: vi.fn().mockRejectedValue(new Error("no checkout")), assertSourceClean: vi.fn() }, + }); + const staleCheck = vi.spyOn(loop as any, "isValidationWorkspaceStale").mockResolvedValue({ workspaceStale: true }); + const failHandler = vi.spyOn(loop as any, "handleValidationFail"); + const inconclusiveHandler = vi.spyOn(loop as any, "handleValidationInconclusive"); + loop.start(); + + await loop.processTaskOutcome("FN-001"); + + expect(createResolvedAgentSession).toHaveBeenCalledWith(expect.objectContaining({ cwd: "/ambient-root" })); + expect(staleCheck).toHaveBeenCalledWith("landed-sha", "/ambient-root"); + expect(inconclusiveHandler).toHaveBeenCalled(); + expect(failHandler).not.toHaveBeenCalled(); + }); + + it("never materializes the task fork point as delivered code", async () => { + primeFeature(); + primeFailVerdict(); + const materialize = vi.fn(); + taskStore._setTask({ + id: "FN-001", + title: "Test", + description: "d", + log: [], + column: "done", + baseCommitSha: "fork-point", + } as any); + loop = new MissionExecutionLoop({ + taskStore: taskStore as any, + missionStore: missionStore as any, + rootDir: "/ambient-root", + checkoutMaterializer: { materialize, assertSourceClean: vi.fn() }, + }); + const staleCheck = vi.spyOn(loop as any, "isValidationWorkspaceStale").mockResolvedValue({ workspaceStale: false, inspectionUnavailableReason: "landed merge SHA is unavailable" }); + loop.start(); + + await loop.processTaskOutcome("FN-001"); + + expect(materialize).not.toHaveBeenCalled(); + expect(createResolvedAgentSession).toHaveBeenCalledWith(expect.objectContaining({ cwd: "/ambient-root" })); + expect(staleCheck).toHaveBeenCalledWith(undefined, "/ambient-root"); }); (hasGit ? it : it.skip)( - "should fail open (normal fail) when the integration SHA is an unknown object", + "defers a fail when landed merge ancestry is unavailable", async () => { primeFeature(); primeFailVerdict(); // A bogus SHA makes `git merge-base --is-ancestor` exit 128 (bad object), - // which is unknown → must NOT suppress the fail. + // so the judge's inspection cannot be proven to contain delivered code. const repo = makeGitRepo(); - taskStore._setTask({ id: "FN-001", title: "Test", description: "d", log: [], column: "done", integrationSha: "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef" } as any); + taskStore._setTask({ id: "FN-001", title: "Test", description: "d", log: [], column: "done", mergeDetails: { commitSha: "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef" } } as any); loop = new MissionExecutionLoop({ taskStore: taskStore as any, @@ -2259,12 +2339,11 @@ describe("MissionExecutionLoop", () => { await loop.processTaskOutcome("FN-001"); - expect(missionStore.createGeneratedFixFeature).toHaveBeenCalled(); + expect(missionStore.createGeneratedFixFeature).not.toHaveBeenCalled(); expect(emitSpy).toHaveBeenCalledWith( - "validation:failed", - expect.objectContaining({ featureId: "F-001" }), + "validation:inconclusive", + expect.objectContaining({ featureId: "F-001", reason: expect.stringContaining("could not prove") }), ); - expect(emitSpy).not.toHaveBeenCalledWith("validation:inconclusive", expect.anything()); }, ); }); @@ -2505,9 +2584,10 @@ describe("MissionExecutionLoop", () => { await loop.processTaskOutcome("FN-001"); - // When budget exhausted, validation:budget_exhausted event should be emitted + // The missing landed revision is an inspection failure, so no retry + // budget is consumed by a Fix Feature loop. expect(emitSpy).toHaveBeenCalledWith( - "validation:budget_exhausted", + "validation:inconclusive", expect.objectContaining({ featureId: "F-001" }), ); }); @@ -2566,9 +2646,9 @@ describe("MissionExecutionLoop", () => { await loop.processTaskOutcome("FN-001"); - // Should emit budget_exhausted when at custom max + // No Fix Feature budget is consumed while inspection is unverifiable. expect(emitSpy).toHaveBeenCalledWith( - "validation:budget_exhausted", + "validation:inconclusive", expect.objectContaining({ featureId: "F-001" }), ); }); diff --git a/packages/engine/src/mission-execution-loop.ts b/packages/engine/src/mission-execution-loop.ts index 41e956c965..5f87566c97 100644 --- a/packages/engine/src/mission-execution-loop.ts +++ b/packages/engine/src/mission-execution-loop.ts @@ -25,7 +25,7 @@ import type { Mission, } from "@fusion/core"; import { normalizeMissionAssertionType } from "@fusion/core"; -import type { VerificationOutcome } from "./mission-verification.js"; +import { GitCheckoutMaterializer, type CheckoutMaterializer, type VerificationOutcome } from "./mission-verification.js"; import { createFnAgent, promptWithFallback, type AgentResult } from "./pi.js"; import { mergeEffectiveSettings } from "./effective-settings.js"; import { @@ -60,6 +60,25 @@ const VALIDATION_TIMEOUT_MS = 10 * 60 * 1000; // 10 minutes * The agent evaluates each linked assertion and returns pass/fail/blocked * per assertion plus an overall status. */ +interface ValidationInspection { + inspectionRoot: string; + landedSha: string | undefined; + fallbackUsed: boolean; + workspaceStale: boolean; + /** Why the inspected tree could not be proven to contain landed code. */ + inspectionUnavailableReason?: string; +} + +interface ValidationWorkspaceStaleness { + workspaceStale: boolean; + inspectionUnavailableReason?: string; +} + +interface ValidationExecution { + result: ValidationResult; + inspection: ValidationInspection; +} + export interface ValidationResult { /** * Overall validation status. @@ -110,6 +129,8 @@ export interface MissionExecutionLoopOptions { * preserving the behavior of existing construction sites that inject nothing. */ verificationCapability?: import("./mission-verification.js").VerificationCapability; + /** Injectable disposable-checkout seam for validator inspection tests. */ + checkoutMaterializer?: CheckoutMaterializer; } export class MissionExecutionLoop extends EventEmitter { @@ -122,6 +143,7 @@ export class MissionExecutionLoop extends EventEmitter { private pluginRunner?: MissionExecutionLoopOptions["pluginRunner"]; private agentStore?: MissionExecutionLoopOptions["agentStore"]; private verificationCapability?: MissionExecutionLoopOptions["verificationCapability"]; + private checkoutMaterializer: CheckoutMaterializer; private activeValidations = new Set(); // feature IDs currently being validated constructor(options: MissionExecutionLoopOptions) { @@ -134,6 +156,7 @@ export class MissionExecutionLoop extends EventEmitter { this.pluginRunner = options.pluginRunner; this.agentStore = options.agentStore; this.verificationCapability = options.verificationCapability; + this.checkoutMaterializer = options.checkoutMaterializer ?? new GitCheckoutMaterializer(); loopLog.log("MissionExecutionLoop created"); } @@ -519,12 +542,15 @@ export class MissionExecutionLoop extends EventEmitter { try { loopLog.log(`Running internal validation for feature ${feature.id} — no board task created (policy: docs/missions.md)`); - // Start the validator run (no board task per docs/missions.md) - const run = await this.missionStore.startValidatorRun(feature.id, "task_completion"); + // FNXC:MissionValidation 2026-07-16-12:00: + // Validator runs retain task linkage, while routing consumes inspection + // provenance calculated in the exact root the judge read. + const run = feature.taskId + ? await this.missionStore.startValidatorRun(feature.id, "task_completion", feature.taskId) + : await this.missionStore.startValidatorRun(feature.id, "task_completion"); loopLog.log(`Started validator run ${run.id} for feature ${feature.id}`); - // Run the validation - const result = await this.runValidation(feature, assertions, run); + const { result, inspection } = await this.runValidation(feature, assertions, run); // Handle the result if (result.status === "pass") { @@ -544,18 +570,16 @@ 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 if (inspection.workspaceStale || inspection.inspectionUnavailableReason) { + // FNXC:MissionValidation 2026-07-16-14:00: + // A FAIL can create a Fix Feature only after the judge's inspection + // root is proven to contain the landed code. A stale root, unresolved + // merge SHA, or unavailable ancestry result is inconclusive instead; + // this prevents a wrong checkout from restarting implementation. + const reason = inspection.workspaceStale + ? `validation workspace predates the merged code for ${feature.taskId} — validation deferred` + : `validation could not prove the inspected workspace contains merged code (${inspection.inspectionUnavailableReason}) — validation deferred`; + await this.handleValidationInconclusive(feature.id, run.id, reason); } else { await this.handleValidationFail(feature.id, run.id, result); } @@ -592,26 +616,29 @@ export class MissionExecutionLoop extends EventEmitter { } /** - * 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. + * Determine whether the exact inspection root proves it contains landed code. + * Exit 1 means the root is stale; missing SHA, bad objects, and other git + * failures are unproven inspections and must defer a FAIL rather than mint a + * remediation task from an unverifiable checkout. */ - private async isValidationWorkspaceStale(feature: MissionFeature): Promise { - const integrationSha = await this.resolveIntegrationSha(feature); - if (!integrationSha) return false; // no evidence → trust the fail + private async isValidationWorkspaceStale( + landedSha: string | undefined, + inspectionRoot: string, + ): Promise { + if (!landedSha) { + return { workspaceStale: false, inspectionUnavailableReason: "landed merge SHA is unavailable" }; + } try { - await execAsync(`git merge-base --is-ancestor ${quoteShellArg(integrationSha)} HEAD`, { - cwd: this.rootDir, + await execAsync(`git merge-base --is-ancestor ${quoteShellArg(landedSha)} HEAD`, { + cwd: inspectionRoot, timeout: 30_000, }); - return false; // exit 0 → ancestor → workspace is fresh + return { workspaceStale: 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; + // `--is-ancestor` exits 1 = NOT an ancestor (affirmatively stale). A bad + // object/non-repo (usually 128) cannot prove the judge saw delivered code. + if ((err as { code?: number })?.code === 1) return { workspaceStale: true }; + return { workspaceStale: false, inspectionUnavailableReason: "landed merge ancestry is unavailable" }; } } @@ -626,7 +653,7 @@ export class MissionExecutionLoop extends EventEmitter { feature: MissionFeature, assertions: MissionContractAssertion[], _run: MissionValidatorRun, - ): Promise { + ): Promise { loopLog.log(`Running validation for feature ${feature.id} with ${assertions.length} assertions`); const milestone = await this.resolveFeatureMilestone(feature); @@ -656,6 +683,25 @@ export class MissionExecutionLoop extends EventEmitter { ); let session: AgentResult | null = null; + let checkout: Awaited> | undefined; + const landedSha = await this.resolveIntegrationSha(feature); + let inspectionRoot = this.rootDir; + let fallbackUsed = !landedSha; + + // FNXC:MissionValidation 2026-07-16-12:00: + // Issue #2168 requires the read-only judge to inspect the landed merge + // checkout, not ambient rootDir whose branch can diverge. If checkout + // materialization fails, retain rootDir behavior and evaluate staleness in + // that exact fallback root before the disposable checkout is disposed. + if (landedSha) { + try { + checkout = await this.checkoutMaterializer.materialize(this.rootDir, landedSha); + inspectionRoot = checkout.dir; + fallbackUsed = false; + } catch (err) { + loopLog.warn(`Unable to materialize validation checkout for ${feature.id}; using rootDir fallback:`, err); + } + } try { // Create validation agent session @@ -670,7 +716,7 @@ export class MissionExecutionLoop extends EventEmitter { sessionPurpose: "validation", runtimeHint: validationRuntimeHint, pluginRunner: this.pluginRunner, - cwd: this.rootDir, + cwd: inspectionRoot, systemPrompt: this.buildValidationSystemPrompt(feature, assertions, taskContext, milestone), tools: "readonly", defaultProvider: validationSessionModel.provider, @@ -723,21 +769,28 @@ export class MissionExecutionLoop extends EventEmitter { // (or refuted) by a non-mutating verification run instead. const result = await this.applyBehavioralPosture(feature, assertions, judgeResult); + const workspace = await this.isValidationWorkspaceStale(landedSha, inspectionRoot); loopLog.log(`Validation completed for feature ${feature.id}: ${result.status}`); - return result; + return { + result, + inspection: { inspectionRoot, landedSha, fallbackUsed, ...workspace }, + }; } catch (err) { const message = err instanceof Error ? err.message : String(err); loopLog.error(`Validation error for feature ${feature.id}:`, message); // Return an error result - the loop will handle it return { - status: "error", - assertions: assertions.map((a) => ({ - assertionId: a.id, - passed: false, - message: `Validation error: ${message}`, - })), - summary: `Validation failed due to error: ${message}`, + result: { + status: "error", + assertions: assertions.map((a) => ({ + assertionId: a.id, + passed: false, + message: `Validation error: ${message}`, + })), + summary: `Validation failed due to error: ${message}`, + }, + inspection: { inspectionRoot, landedSha, fallbackUsed, workspaceStale: false }, }; } finally { // Always dispose the session @@ -749,6 +802,13 @@ export class MissionExecutionLoop extends EventEmitter { loopLog.warn(`Error disposing validation session for ${feature.id}:`, disposeErr); } } + if (checkout) { + try { + await checkout.dispose(); + } catch (disposeErr) { + loopLog.warn(`Error disposing validation checkout for ${feature.id}:`, disposeErr); + } + } } } @@ -888,20 +948,20 @@ export class MissionExecutionLoop extends EventEmitter { } /** - * Resolve the trusted revision (integration SHA) whose disposable checkout the - * verification run executes against. The live task worktree is pruned before - * the done-transition that triggers validation, so it cannot be used. + * Resolve the verified landed merge revision for a feature's linked task. * - * In this unit we read it from the linked task when available; callers that do - * not supply a resolvable SHA cause the verification run to resolve to - * inconclusive (fail-closed). A richer derivation is owned by a later unit. + * FNXC:MissionValidation 2026-07-16-12:00: + * `mergeDetails.commitSha` is the only delivered-code revision: it is the + * landed merge tip. `baseCommitSha` is the task worktree fork point and must + * never be inspected as delivered code; Task has no `integrationSha` or + * `baseCommit` fields. Inspection-root pinning, stale checking, and + * behavioral verification all consume this same revision. */ private async resolveIntegrationSha(feature: MissionFeature): Promise { if (!feature.taskId) return undefined; try { const task = await this.taskStore.getTask(feature.taskId); - const candidate = (task as { integrationSha?: string; baseCommit?: string } | undefined); - return candidate?.integrationSha ?? candidate?.baseCommit ?? undefined; + return task?.mergeDetails?.commitSha; } catch { return undefined; }