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) <noreply@runfusion.ai>
This commit is contained in:
7
.changeset/fn-8206-mission-validator-inspection-root.md
Normal file
7
.changeset/fn-8206-mission-validator-inspection-root.md
Normal file
@@ -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.
|
||||
@@ -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.
|
||||
|
||||
@@ -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" }),
|
||||
);
|
||||
});
|
||||
|
||||
@@ -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<string>(); // 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<boolean> {
|
||||
const integrationSha = await this.resolveIntegrationSha(feature);
|
||||
if (!integrationSha) return false; // no evidence → trust the fail
|
||||
private async isValidationWorkspaceStale(
|
||||
landedSha: string | undefined,
|
||||
inspectionRoot: string,
|
||||
): Promise<ValidationWorkspaceStaleness> {
|
||||
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<ValidationResult> {
|
||||
): Promise<ValidationExecution> {
|
||||
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<ReturnType<CheckoutMaterializer["materialize"]>> | 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<string | undefined> {
|
||||
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;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user