root/fusion
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(engine): defer validator fail when the judged workspace predates the merged code

Browse Source 
The mission validator runs its read-only judge session with cwd:
this.rootDir — the engine's main working copy. When a task's merge landed
on the remote or in another worktree and rootDir was never fetched/reset to
it, the judge reads PRE-merge files and returns a spurious `fail`. #1917's
premerge column guard doesn't catch this: the task column is already `done`,
so it falls through to handleValidationFail and mints a bogus Fix Feature.

Add a symmetric second guard in the fail branch, after the premerge column
check: isValidationWorkspaceStale resolves the task's integration SHA and
runs `git merge-base --is-ancestor <sha> HEAD` in rootDir. Only affirmative
staleness evidence (exit 1 = NOT an ancestor) defers the fail to
inconclusive; exit 0 (ancestor/fresh), a missing SHA, or a bad object
(exit 128) all trust the fail. Fail-open: a guard may DEFER a fail, never
SUPPRESS one on missing or unreadable data.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
fusion-merge-train
2026-07-06 08:23:07 +02:00
parent 6c9989c847
commit 58683f9610
2 changed files with 218 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

171
packages/engine/src/__tests__/mission-execution-loop.test.ts
View File

@@ -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<typeof create
// ── Tests ───────────────────────────────────────────────────────────────────
// ── real-git fixtures (for the stale-workspace guard) ────────────────────────
const hasGit = spawnSync("git", ["--version"], { stdio: "pipe" }).status === 0;
function git(repo: string, command: string): string {
return execSync(command, { cwd: repo, encoding: "utf-8", stdio: ["pipe", "pipe", "pipe"] }).trim();
}
/** A throwaway git repo with one initial commit; HEAD on `main`. */
function initGitRepo(): string {
const repo = mkdtempSync(join(tmpdir(), "fn-stale-ws-"));
git(repo, "git init -b main");
git(repo, 'git config user.email "test@example.com"');
git(repo, 'git config user.name "Test User"');
git(repo, "git config commit.gpgsign false");
writeFileSync(join(repo, "foo.ts"), "line1\n");
git(repo, "git add foo.ts && git commit -m init");
return repo;
}
describe("MissionExecutionLoop", () => {
let loop: MissionExecutionLoop;
let missionStore: ReturnType<typeof createMockMissionStore>;
@@ -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 ───────────────────────────────────────────────

47
packages/engine/src/mission-execution-loop.ts
View File

@@ -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<boolean> {
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.
*