FN-8597: restore executor task-done invariant coverage

Restore the quarantined executor graph-completion invariant suite with real foreach projections.

- Exercise complete and partial expanded workflow-step projections at the merge boundary.
- Remove the rescued invariant suite from Vitest quarantine and clear its ledger entry.
- Extend the shared executor logger mock with the debug method required by the integration tip.

Files changed:
 .../__tests__/executor-task-done-invariant.test.ts | 267 +++++++++++++++++++--
 .../engine/src/__tests__/executor-test-helpers.ts  |   7 +
 packages/engine/vitest.config.ts                   |   7 -
 scripts/lib/test-quarantine.json                   |   8 +-
 4 files changed, 254 insertions(+), 35 deletions(-)

Fusion-Task-Id: FN-8597

Fusion-Task-Lineage: 05a08e31-7da0-4c93-86a0-9baf8db7ce52

Co-authored-by: Fusion (runfusion.ai) <noreply@runfusion.ai>
This commit is contained in:
gsxdsm
2026-07-26 10:04:36 -07:00
parent f157bf7460
commit c7fa02f370
4 changed files with 254 additions and 35 deletions

View File

@@ -4,7 +4,7 @@ import { join } from "node:path";
import "./executor-test-helpers.js";
import { TaskExecutor } from "../executor.js";
import * as worktreePool from "../worktree-pool.js";
import { type TaskStore } from "@fusion/core";
import { getBuiltinWorkflow, instanceNodeId, type TaskStore } from "@fusion/core";
import { createTaskStoreForTest, pgDescribe, type PgTestHarness } from "../../../core/src/__test-utils__/pg-test-harness.js";
import { captureNamedTool, createMockStore, mockedCreateFnAgent, mockedExec, mockedExecSync, resetExecutorMocks } from "./executor-test-helpers.js";
@@ -814,6 +814,15 @@ pgDescribe("FN-5241 executor handoff auditing", () => {
harness = await createTaskStoreForTest({ prefix: "fusion_executor_handoff_audit" });
rootDir = harness.rootDir;
store = harness.store;
/*
FNXC:EngineTests 2026-07-26-08:18:
The PG harness deliberately has no global directory. FN-8597's graph test has no MCP
configuration, so use its no-MCP secret-reader seam instead of resolving a real global store.
This isolates the executor handoff invariant from environment-specific secrets initialization.
*/
vi.spyOn(store, "getSecretsStore").mockResolvedValue({
revealSecret: vi.fn(),
} as any);
vi.spyOn(worktreePool, "isUsableTaskWorktree").mockResolvedValue(true);
});
@@ -821,7 +830,7 @@ pgDescribe("FN-5241 executor handoff auditing", () => {
await harness?.teardown();
});
async function createExecutorTask(taskDoneRetryCount = 0) {
async function createExecutorTask(taskDoneRetryCount = 0, stepNames = ["Implement"]) {
const created = await store.createTask({ description: "Invariant test", priority: "high" });
await store.moveTask(created.id, "todo");
await store.moveTask(created.id, "in-progress");
@@ -829,11 +838,12 @@ pgDescribe("FN-5241 executor handoff auditing", () => {
mkdirSync(worktreePath, { recursive: true });
const branch = `fusion/${created.id.toLowerCase()}`;
/*
FNXC:EngineTests 2026-07-20-23:55:
Graph-native completion falls back to task.prompt when no PROMPT.md task-document exists.
Seed an executable step-heading prompt so parse/execute can finish and reach merge.
FNXC:EngineTests 2026-07-26-08:18:
FN-8597 exercises the real graph expansion from task-document step headings.
Derive the heading from the requested projection so graph-owned execution, rather than
manually seeded workflow-step results, determines the active foreach node ids.
*/
const prompt = "# Test\n## Steps\n### Step 0: Implement\n- [ ] check\n";
const prompt = `# Test\n## Steps\n${stepNames.map((name, index) => `### Step ${index}: ${name}\n- [ ] check`).join("\n")}\n`;
const taskDir = join(rootDir, ".fusion", "tasks", created.id);
mkdirSync(taskDir, { recursive: true });
writeFileSync(join(taskDir, "PROMPT.md"), prompt, "utf8");
@@ -842,8 +852,8 @@ pgDescribe("FN-5241 executor handoff auditing", () => {
branch,
baseCommitSha: "abc123",
taskDoneRetryCount,
// Mark implementation complete so graph foreach step-execute can advance to merge after fn_task_done.
steps: [{ name: "Implement", status: "done" }],
// Start pending so graph-owned fn_task_done completes the real foreach projection.
steps: stepNames.map((name) => ({ name, status: "pending" })),
currentStep: 0,
prompt,
} as any);
@@ -857,6 +867,86 @@ pgDescribe("FN-5241 executor handoff auditing", () => {
};
}
/*
FNXC:EngineTests 2026-07-27-08:50:
FN-8597's merge-proof regression coverage must inspect the persisted foreach
instances, not `workflowStepResults`: that field belongs to optional-group
review gates, while `steps#N:step-execute` state is stored under the live
graph run id. Reading the rows derived by the runner proves the partial and
complete projections use the actual expanded node identities rather than
guessed synthetic result ids.
*/
async function loadForeachInstances(taskId: string) {
return store.loadWorkflowRunStepInstancesAsync(taskId, `${taskId}:builtin:coding`);
}
async function advanceMergeBoundary(executor: TaskExecutor, task: unknown, mergeBoundaryNodeId: string) {
return (executor as unknown as {
ensureWorkflowMergeBoundaryTask: (candidate: unknown, metadata: unknown) => Promise<unknown>;
}).ensureWorkflowMergeBoundaryTask(task, {
reason: "fn-8597-workflow-step-result-proof",
nodeId: mergeBoundaryNodeId,
workflowId: "builtin:coding",
runId: `${(task as { id: string }).id}:builtin:coding`,
});
}
type GraphCompletionProjection = {
parentIds: string[];
groupIds: string[];
leafIds: string[];
mergeBoundaryNodeId: string;
};
/*
FNXC:EngineTests 2026-07-27-11:05:
FN-8597 must seed the merge proof from the live foreach expansion, not guessed
`steps#N` strings. Derive the foreach parents, expanded groups, leaves, and
merge boundary from builtin:coding so a graph topology change cannot leave this
PG fixture proving an obsolete projection.
*/
async function deriveGraphCompletionProjection(taskId: string): Promise<GraphCompletionProjection> {
const instances = await loadForeachInstances(taskId);
const graph = getBuiltinWorkflow("builtin:coding")?.ir;
expect(graph).toBeDefined();
const groupIds = [...new Set(instances.map((instance) => instance.foreachNodeId))].sort();
const parentIds = [...new Set(
graph!.edges
.filter((edge) => groupIds.includes(edge.to))
.map((edge) => edge.from),
)].sort();
const leafIds = instances
.filter((instance) => instance.status === "completed")
.map((instance) => {
const foreach = graph!.nodes.find((node) => node.id === instance.foreachNodeId && node.kind === "foreach");
const template = foreach?.config?.template;
const stepExecute = template?.nodes.find((node) => node.config?.seam === "step-execute");
expect(stepExecute?.id).toBeTypeOf("string");
return instanceNodeId(instance.foreachNodeId, instance.stepIndex, stepExecute!.id);
})
.sort();
const mergeBoundary = graph!.nodes.find((node) => node.kind === "merge-gate");
expect(parentIds).not.toHaveLength(0);
expect(groupIds).not.toHaveLength(0);
expect(mergeBoundary?.id).toBeTypeOf("string");
expect(leafIds).toHaveLength(instances.length);
return { parentIds, groupIds, leafIds, mergeBoundaryNodeId: mergeBoundary!.id };
}
function completedProjectionResults(projection: GraphCompletionProjection) {
return [
...projection.parentIds,
...projection.groupIds,
...projection.leafIds,
].map((workflowStepId) => ({
workflowStepId,
workflowStepName: workflowStepId,
source: "node" as const,
phase: "pre-merge" as const,
status: "passed" as const,
}));
}
/*
FNXC:WorkflowLifecycle 2026-07-01-22:10:
workflowGraphExecutor is default-on, so the FN-5241 "atomic in-review handoff seam" auditing was
@@ -873,21 +963,16 @@ pgDescribe("FN-5241 executor handoff auditing", () => {
*/
/*
FNXC:EngineTests 2026-07-20-23:55:
Graph-native step-execute still terminates before the merge boundary under this PG + mock-agent fixture after U10b (steps#0:step-execute failure). Skip until a dedicated graph completion harness is written; do not appease with timeouts. Same failure class observed on origin/main full-suite.
FNXC:EngineTests 2026-07-26-08:18:
FN-8597 found the old `steps#0:step-execute` failure was an unconfigured PG-harness
secrets-store access, not a graph-completion defect. Do not seed guessed result ids:
completion must traverse the live single-instance foreach expansion and merge boundary.
*/
it.skip("moves a cleanly completed task to in-review via the merge-node boundary", async () => {
it("moves a cleanly completed task to in-review via the merge-node boundary", async () => {
const { task, worktreePath } = await createExecutorTask();
// Graph-native implementation proof: the mock agent signals completion via fn_task_done without running
// the foreach step-execute nodes, so the merge-boundary FN-7260/FN-7271 proof gate needs an explicit
// node-source pre-merge pass to model a genuinely-implemented task reaching the merge boundary.
await store.updateTask(task.id, {
workflowStepResults: [
{ workflowStepId: "execute", workflowStepName: "Execute", phase: "pre-merge", source: "node", status: "passed" },
{ workflowStepId: "steps#0:step-execute", workflowStepName: "Implement", phase: "pre-merge", source: "node", status: "passed" },
{ workflowStepId: "steps", workflowStepName: "Steps", phase: "pre-merge", source: "node", status: "passed" },
],
} as any);
mockedExec.mockImplementation(((cmd: string, _opts: unknown, cb?: (err: Error | null, stdout: string, stderr: string) => void) => {
if (!cb) return undefined as any;
if (cmd.includes("rev-parse --show-toplevel")) return cb(null, `${worktreePath}\n`, "");
@@ -905,7 +990,7 @@ pgDescribe("FN-5241 executor handoff auditing", () => {
await taskDoneTool.execute("tool-1", {});
return;
}
// FNXC:EngineTests 2026-07-20-23:55: step-execute / review sessions need a clean prompt completion when fn_task_done is absent.
// FNXC:EngineTests 2026-07-26-08:18: graph step sessions without task_done use the native step-completion tool.
const stepDone = tools.find((tool: any) => tool.name === "fn_task_step_done" || tool.name === "fn_step_done");
if (stepDone) {
await stepDone.execute("tool-step", {});
@@ -927,7 +1012,15 @@ pgDescribe("FN-5241 executor handoff auditing", () => {
await executor.execute(task as any);
// CURRENT completion mechanism: task ends in-review via the merge-node boundary moveTask.
expect((await store.getTask(task.id))?.column).toBe("in-review");
const latest = await store.getTask(task.id);
expect(latest?.column).toBe("in-review");
// No synthetic optional-group result can bypass the proof: the live foreach traversal
// completes the actual task-step projection before the merge boundary advances.
expect(latest?.steps.map((step) => step.status)).toEqual(["done"]);
const instances = await loadForeachInstances(task.id);
expect(instances).toEqual(expect.arrayContaining([
expect.objectContaining({ foreachNodeId: "steps", stepIndex: 0, status: "completed" }),
]));
// Forensic intent preserved via the mechanism that actually fires now: the merge boundary records a
// task:move into in-review (the review-seam task:handoff("workflow-graph-review") event and the
// review-seam merge-queue enqueue no longer occur on this path).
@@ -938,7 +1031,139 @@ pgDescribe("FN-5241 executor handoff auditing", () => {
expect(await store.getRunAuditEventsAsync({ taskId: task.id, mutationType: "task:handoff", limit: 10 })).toHaveLength(0);
});
it.skip("fails a no-fn_task_done retry-budget-exhausted run in place without moving to in-review", async () => {
/*
FNXC:EngineTests 2026-07-27-08:35:
FN-8597's rescue must protect the foreach projection rather than only its former
single-step reproduction. A partial multi-step run has no implementation proof for
the unvisited instance, so it must fail before the graph merge boundary can move the
task to in-review.
*/
it("does not move a partial foreach expansion to in-review", async () => {
const { task, worktreePath } = await createExecutorTask(0, ["Implement", "Verify"]);
mockedExec.mockImplementation(((cmd: string, _opts: unknown, cb?: (err: Error | null, stdout: string, stderr: string) => void) => {
if (!cb) return undefined as any;
if (cmd.includes("rev-parse --show-toplevel")) return cb(null, `${worktreePath}\n`, "");
if (cmd.includes("rev-parse --abbrev-ref HEAD")) return cb(null, `${task.branch}\n`, "");
if (cmd.includes("rev-list --count")) return cb(null, "1\n", "");
if (cmd.includes("rev-parse HEAD")) return cb(null, "def456\n", "");
return cb(null, "", "");
}) as any);
const executor = new TaskExecutor(store as any, rootDir);
// The graph's per-instance seam is the narrow deterministic boundary: complete
// only expanded instance 0 in the real PG projection, then reject instance 1.
// This makes the persisted result set partial without depending on agent timing.
vi.spyOn(executor as any, "runGraphTaskStep").mockImplementation(async (_task: unknown, stepIndex: number) => {
if (stepIndex !== 0) return { success: false, error: "intentionally partial projection" };
await store.updateStep(task.id, stepIndex, "done", { source: "graph" });
return { success: true };
});
await executor.execute(task as any);
const latest = await store.getTask(task.id);
expect(latest?.column).not.toBe("in-review");
// The failed second instance is left in-progress for graph-owned recovery; its
// non-terminal projection is the condition that must keep merge unavailable.
expect(latest?.steps.map((step) => step.status)).toEqual(["done", "in-progress"]);
// The graph persisted the actual first expanded node as complete but its sibling
// `steps#1:step-execute` as failed; this partial result set must not cross merge.
const instances = await loadForeachInstances(task.id);
expect(instances).toEqual(expect.arrayContaining([
expect.objectContaining({ foreachNodeId: "steps", stepIndex: 0, status: "completed" }),
expect.objectContaining({ foreachNodeId: "steps", stepIndex: 1, status: "failed" }),
]));
expect((await store.getRunAuditEventsAsync({ taskId: task.id, mutationType: "task:move", limit: 20 }))
.some((event) => (event.metadata as { to?: string })?.to === "in-review")).toBe(false);
expect(await store.getRunAuditEventsAsync({ taskId: task.id, mutationType: "task:handoff", limit: 10 })).toHaveLength(0);
expect(await store.peekMergeQueue()).toEqual([]);
});
it("requires every expanded foreach step before the merge boundary advances", async () => {
const { task, worktreePath } = await createExecutorTask(0, ["Implement", "Verify"]);
mockedExec.mockImplementation(((cmd: string, _opts: unknown, cb?: (err: Error | null, stdout: string, stderr: string) => void) => {
if (!cb) return undefined as any;
if (cmd.includes("rev-parse --show-toplevel")) return cb(null, `${worktreePath}\n`, "");
if (cmd.includes("rev-parse --abbrev-ref HEAD")) return cb(null, `${task.branch}\n`, "");
if (cmd.includes("rev-list --count")) return cb(null, "1\n", "");
if (cmd.includes("rev-parse HEAD")) return cb(null, "def456\n", "");
return cb(null, "", "");
}) as any);
const executor = new TaskExecutor(store as any, rootDir);
// Complete each runtime-expanded instance through the same projection seam.
// The runner, not this test, creates and persists `steps#0` and `steps#1`.
vi.spyOn(executor as any, "runGraphTaskStep").mockImplementation(async (_task: unknown, stepIndex: number) => {
await store.updateStep(task.id, stepIndex, "done", { source: "graph" });
return { success: true };
});
executor.setMergeRequester(async () => ({ merged: false, noOp: false, reason: "queued" }) as any);
await executor.execute(task as any);
const latest = await store.getTask(task.id);
// A graph run uses one shared implementation pass for sequential foreach instances.
// The proof is the complete live projection, paired with the partial case above:
// merge can advance only after both expanded task steps are terminal.
expect(latest?.steps.map((step) => step.status)).toEqual(["done", "done"]);
// Complete the same real expansion: both persisted `steps#N:step-execute`
// identities must be complete before this run can enter the merge boundary.
const instances = await loadForeachInstances(task.id);
expect(instances).toEqual(expect.arrayContaining([
expect.objectContaining({ foreachNodeId: "steps", stepIndex: 0, status: "completed" }),
expect.objectContaining({ foreachNodeId: "steps", stepIndex: 1, status: "completed" }),
]));
expect(instances).toHaveLength(2);
expect(latest?.column).toBe("in-review");
expect((await store.getRunAuditEventsAsync({ taskId: task.id, mutationType: "task:move", limit: 20 }))
.some((event) => (event.metadata as { to?: string })?.to === "in-review")).toBe(true);
expect(await store.getRunAuditEventsAsync({ taskId: task.id, mutationType: "task:handoff", limit: 10 })).toHaveLength(0);
});
/*
FNXC:EngineTests 2026-07-27-10:30:
FN-8597's rescue proves the merge boundary sees the complete live expansion:
its execution parent, foreach group, and every `steps#N:step-execute` leaf.
A partial leaf projection must not enter review even when parent/group results
and persisted PG instances are present.
*/
it("blocks partial workflowStepResults and advances a complete multi-instance merge proof", async () => {
const { task } = await createExecutorTask(0, ["Implement", "Verify"]);
const runId = `${task.id}:builtin:coding`;
for (const stepIndex of [0, 1]) {
await store.saveWorkflowRunStepInstanceAsync({
taskId: task.id,
runId,
foreachNodeId: "steps",
stepIndex,
pinnedStepCount: 2,
currentNodeId: "step-execute",
status: "completed",
reworkCount: 0,
});
}
const projection = await deriveGraphCompletionProjection(task.id);
const completeResults = completedProjectionResults(projection);
const partialResults = completeResults.filter((result) => result.workflowStepId !== projection.leafIds[1]);
await store.updateTask(task.id, { workflowStepResults: partialResults } as any);
const executor = new TaskExecutor(store as any, rootDir);
await advanceMergeBoundary(executor, task, projection.mergeBoundaryNodeId);
let latest = (await store.getTask(task.id))!;
expect(latest.column).not.toBe("in-review");
expect(latest.workflowStepResults.map((result) => result.workflowStepId).sort()).toEqual(
partialResults.map((result) => result.workflowStepId).sort(),
);
expect(latest.steps.map((step) => step.status)).toEqual(["pending", "pending"]);
await store.updateTask(task.id, { workflowStepResults: completeResults } as any);
latest = (await store.getTask(task.id))!;
await advanceMergeBoundary(executor, latest, projection.mergeBoundaryNodeId);
latest = (await store.getTask(task.id))!;
expect(latest.column).toBe("in-review");
expect(latest.workflowStepResults.map((result) => result.workflowStepId).sort()).toEqual(
completeResults.map((result) => result.workflowStepId).sort(),
);
expect(latest.steps.every((step) => step.status === "done")).toBe(true);
});
it("fails a no-fn_task_done retry-budget-exhausted run in place without moving to in-review", async () => {
const { task, worktreePath } = await createExecutorTask(3);
mockedExec.mockImplementation(((cmd: string, _opts: unknown, cb?: (err: Error | null, stdout: string, stderr: string) => void) => {
if (!cb) return undefined as any;

View File

@@ -37,7 +37,14 @@ vi.mock("../logger.js", () => {
const probe = process.env.FUSION_TEST_LOG_PROBE === "1"
? (...a: unknown[]) => console.error("[probe]", ...a)
: undefined;
/*
FNXC:EngineTests 2026-07-26-09:50:
Executor construction now emits debug-only dispatch bookkeeping. The rescued
fn_task_done invariant suite imports this logger mock, so preserve its full
logger contract when it re-enters the default test suite.
*/
const createMockLogger = () => ({
debug: vi.fn(probe),
log: vi.fn(probe),
warn: vi.fn(probe),
error: vi.fn(probe),

View File

@@ -244,13 +244,6 @@ export default defineConfig({
// / `test:all` invoked from the root `test:full` script.
"src/**/*.slow.test.ts",
/*
FNXC:EngineTests 2026-07-22-03:15:
Quarantine ledger lockstep for executor-task-done-invariant (FN-5241 graph handoff
under PG mock-agent after U10b). Matching entry in scripts/lib/test-quarantine.json;
delete with the ledger row after rescue — never timeout appeasement.
*/
"src/__tests__/executor-task-done-invariant.test.ts",
/*
FNXC:EngineTests 2026-06-26-13:15:
FN-7068 rescued the 2026-06-25 self-healing quarantine batch by completing the local TaskStore fakes for the FN-5488 overlap path. Keep both files active in engine-default so fake drift around clearStaleBlockedBy() is caught before the deletion ratchet expires.
*/

View File

@@ -1,10 +1,4 @@
{
"$comment": "Flaky-test quarantine ledger (deletion ratchet \u2014 see AGENTS.md 'Flaky tests: quarantine on sight' and docs/testing.md 'Quarantine ledger and the deletion ratchet'). A test observed failing without a corresponding real bug is quarantined ON SIGHT: add an entry here AND a matching one-line `exclude` entry in that package's vitest config, in the same commit. Every entry needs a non-empty `reason` (link the failing run) and a `quarantinedAt` date \u2014 the entry expires 14 days later, at which point the test file is DELETED unless someone rescues it with evidence it catches real regressions plus a root-cause fix (never appeasement). There is deliberately no loader module and no automation around this file: it is a dated record, the vitest config is the enforcement.",
"entries": [
{
"file": "packages/engine/src/__tests__/executor-task-done-invariant.test.ts",
"reason": "FN-5241 handoff pair skipped in-file (it.skip): graph fails at steps#0:step-execute under PG mock-agent fixture after U10b; same class red on origin/main full-suite https://github.com/Runfusion/Fusion/actions/runs/29805577293. Rewrite graph completion harness before rescue. quarantinedAt 2026-07-20.",
"quarantinedAt": "2026-07-20"
}
]
"entries": []
}