From b39d66c0027020b1b7562546d5faaefb10e7268f Mon Sep 17 00:00:00 2001 From: Fusion Agent Date: Tue, 25 Aug 2026 20:40:10 +0000 Subject: [PATCH] test(FN-WF): settle at the manual-merge hold instead of guessing from a snapshot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `driveToManualMergeHold` returned on the first turn that merely LOOKED parked (`column === "in-review" && reviewPassed`). That is a snapshot, and a review-column workflow invalidates it one turn later: a Code Review REVISE appends remediation steps and sends the card back to in-progress, so the caller's merge then hit the engine's correct refusal ("task is in 'in-progress', must be in 'in-review'"). The engine was right and the driver was wrong. Suppressing that refusal would have reproduced FN-175 exactly, which is why the earlier attempts to swallow it were reverted rather than kept. It now returns immediately on an authoritative `manual-required` work item, and otherwise keeps turning until the observable signature (column, status, step statuses, review statuses) stops changing across consecutive turns. That is a property of the graph rather than of how fast the suite happens to run — which is why the scenario was intermittent only under full-lane load. S05 is deliberately NOT extended to builtin:coding-ideas-v2 in this change. It passes 22/22 when its file runs alone but fails in the full lane, and the evidence says the cause is harness isolation, not the product: the task is handed a worktree belonging to a DIFFERENT fixture (observed .../fusion-pipeline-smoke-eXXRLy/..., expected .../fusion-pipeline-smoke-K2iaTm/...). The engine detects this and refuses it — `outside_worktrees_dir`, retried, budget exhausted — which is the correct behaviour. Shipping that as a red scenario would be shipping a known flake, so the coverage waits for the isolation fix. pnpm lint 0 errors, test:gate, verify:fast, and three consecutive full runs: 139.2s, 135.1s, 139.9s of the 150s budget. --- .../pipeline-smoke/_pipeline-harness.ts | 34 +++++++++++++++++-- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/packages/engine/src/__tests__/pipeline-smoke/_pipeline-harness.ts b/packages/engine/src/__tests__/pipeline-smoke/_pipeline-harness.ts index 81e34051ae..d2ec3755e7 100644 --- a/packages/engine/src/__tests__/pipeline-smoke/_pipeline-harness.ts +++ b/packages/engine/src/__tests__/pipeline-smoke/_pipeline-harness.ts @@ -956,19 +956,47 @@ export class PipelineSmokeHarness { * Drive the selected task through real Planning, Plan Review, scheduler release, execution, * and Code Review, then stop at the graph's manual-merge hold rather than pre-seeding review. */ + /* + FNXC:PipelineSmoke 2026-08-25-04:20: + Return only once the graph has QUIESCED at the hold, never on the first turn that merely LOOKS + parked. The old exit condition `column === "in-review" && reviewPassed` is a snapshot, and a + review-column workflow invalidates it one turn later: a Code Review REVISE appends remediation + steps and sends the card back to in-progress, so the caller's merge then hit the engine's correct + refusal ("task is in 'in-progress', must be in 'in-review'"). That is a REAL engine verdict, and + suppressing it would have reproduced FN-175; the driver was wrong, not the engine. + A `manual-required` work item is authoritative and returns immediately. Absent one, the loop keeps + turning until the observable state stops changing across consecutive turns, which is a property of + the graph rather than of how fast the suite happens to run — the reason this scenario was + intermittent only under full-lane load. + */ async driveToManualMergeHold(taskId: string, behavior: PipelineScriptedMergeBehavior = {}): Promise { await this.store.updateSettings({ autoMerge: false }); - for (let attempt = 0; attempt < 8; attempt += 1) { + const signature = (task: Task): string => JSON.stringify({ + column: task.column, + status: task.status ?? null, + steps: (task.steps ?? []).map((step) => `${step.id}:${step.status}`), + reviews: (task.workflowStepResults ?? []).map((result) => `${result.workflowStepId}:${result.status}`), + }); + let stableSignature: string | undefined; + let stableTurns = 0; + for (let attempt = 0; attempt < 40; attempt += 1) { await this.runProductionTurn(taskId, behavior); const current = await this.freshTask(taskId); if (current.column === "done" || current.mergeDetails?.mergeConfirmed) { throw new Error(`${taskId} merged before its reviewed branch reached the manual hold.`); } const active = await this.store.listWorkflowWorkItemsForTask(taskId, { kinds: ["task"] }); + const manualHeld = active.some((item) => item.state === "manual-required" || item.nodeId === "merge-manual-hold"); const reviewPassed = (current.workflowStepResults ?? []).some((result) => result.workflowStepId === "code-review" && result.status === "passed") || !(current.enabledWorkflowSteps ?? []).includes("code-review"); - if (active.some((item) => item.state === "manual-required" || item.nodeId === "merge-manual-hold") - || (current.column === "in-review" && reviewPassed)) { + + const currentSignature = signature(current); + stableTurns = currentSignature === stableSignature ? stableTurns + 1 : 0; + stableSignature = currentSignature; + + // Two unchanged turns means the graph has nothing left to advance on its own. + const settled = current.column === "in-review" && reviewPassed && stableTurns >= 2; + if (manualHeld || settled) { await this.assertProductionStageEvidence(taskId, { planReview: (current.enabledWorkflowSteps ?? []).includes("plan-review"), codeReview: (current.enabledWorkflowSteps ?? []).includes("code-review"),