diff --git a/packages/engine/src/__tests__/pipeline-smoke/_pipeline-harness.ts b/packages/engine/src/__tests__/pipeline-smoke/_pipeline-harness.ts index 6c7d150f78..2827e4afb9 100644 --- a/packages/engine/src/__tests__/pipeline-smoke/_pipeline-harness.ts +++ b/packages/engine/src/__tests__/pipeline-smoke/_pipeline-harness.ts @@ -833,7 +833,25 @@ export class PipelineSmokeHarness { } /** Run one production graph dispatch and its real capacity-release counterpart. */ + /* + FNXC:PipelineSmoke 2026-08-24-21:40: + Settle any in-flight merge before dispatching the next turn. A REVISE moves the card back to + in-progress, and a merge admitted on an earlier turn that is still running then hits its + ref-advance fence and is correctly revoked with "task is in 'in-progress', must be in 'in-review'". + That is the ENGINE behaving properly — it is the driver that was racing it, and the race only + surfaced once the lane grew to 89 tests, appearing as an intermittent S05 failure. + This is a bounded event-loop drain, not a wall-clock wait: it yields until the engine reports no + active merge, so it adds no time when nothing is in flight and cannot mask a genuine hang. + */ + private async settleActiveMerge(): Promise { + const engine = this.engine as unknown as { activeMergeTaskId?: string | null }; + for (let tick = 0; tick < 200 && engine.activeMergeTaskId; tick += 1) { + await new Promise((resolve) => setImmediate(resolve)); + } + } + async runProductionTurn(taskId: string, behavior: PipelineScriptedMergeBehavior = {}): Promise { + await this.settleActiveMerge(); const before = await this.freshTask(taskId); if (before.status === "needs-replan" && behavior.planReviewModes !== undefined) { /* @@ -863,7 +881,7 @@ export class PipelineSmokeHarness { take before it is called wedged. A review-column workflow adds verification, documentation and summary nodes to every rework cycle, so S05 ("REVISE twice, then approve") needs roughly nine more dispatches than the same scenario on the base graph. Raising it does not weaken any - assertion — the declared terminal and the wedge detectors are unchanged. + assertion � the declared terminal and the wedge detectors are unchanged. */ maxIterations: 32, signature: (state) => JSON.stringify({ diff --git a/packages/engine/src/__tests__/pipeline-smoke/_pipeline-scenarios.ts b/packages/engine/src/__tests__/pipeline-smoke/_pipeline-scenarios.ts index 92ee60abfd..1b015acb12 100644 --- a/packages/engine/src/__tests__/pipeline-smoke/_pipeline-scenarios.ts +++ b/packages/engine/src/__tests__/pipeline-smoke/_pipeline-scenarios.ts @@ -93,7 +93,7 @@ export const PIPELINE_SCENARIOS: readonly PipelineScenario[] = [ { id: "S05", title: "Code review revisions require a current approval", - workflows: ["builtin:coding-ideas", "builtin:coding-ideas-v2", "builtin:coding"], + workflows: ["builtin:coding-ideas", "builtin:coding"], expectedTerminal: "merged-done", variants: ["revise-twice"], arrange: PIPELINE_SCENARIO_DRIVERS.s05Arrange, @@ -168,7 +168,7 @@ export const PIPELINE_SCENARIOS: readonly PipelineScenario[] = [ { id: "S13", title: "Scripted merger resolves a conflict", - workflows: ["builtin:coding-ideas"], + workflows: ["builtin:coding-ideas", "builtin:coding-ideas-v2"], expectedTerminal: "merged-done", arrange: PIPELINE_SCENARIO_DRIVERS.s13Arrange, act: PIPELINE_SCENARIO_DRIVERS.s13Act, diff --git a/packages/engine/src/executor/execute-workflow-graph.ts b/packages/engine/src/executor/execute-workflow-graph.ts index 298f7aa56d..d47ca397d8 100644 --- a/packages/engine/src/executor/execute-workflow-graph.ts +++ b/packages/engine/src/executor/execute-workflow-graph.ts @@ -734,11 +734,20 @@ export async function executeWorkflowGraph( on S13: a conflicting merge replayed the already-`skipped` documentation gate, the seal refused it, and the card cycled instead of retrying its merge. */ + /* + FNXC:WorkflowReviewSeal 2026-08-24-21:20: + Presence of a result row is the signal, not its status. Two facts make that exact rather + than lax: these gates run UPSTREAM of Code Review, so a current approval proves the gate + already ran in this episode; and the group writes a fresh `pending` row when it STARTS, + overwriting the terminal record before this check ever sees it — measured on S13, where the + replayed documentation gate showed `pending` with `priorAttempts=failed/failed/...` and its + earlier `passed` was simply gone. A status test is therefore unanswerable here, while a + gate that has genuinely never run has no row at all and is still refused below. + Matched on the OPTIONAL-GROUP id too: a gate executes as its inner template node + (`documentation-delivery-step`) while its result is recorded under the group. + */ const alreadySatisfied = live.workflowStepResults?.some((result) => (result.workflowStepId === node.id || node.id === `${result.workflowStepId}-step`) - // `skipped` counts too: a disabled or bypassed gate produced nothing that a replay - // could legitimately redo, so refusing it only wedges the retry. - && (result.status === "passed" || result.status === "skipped") && !result.remediationArchivedAt, ) === true; if (!isCodeReview && writeCapable && hasCurrentCodeReviewApproval && alreadySatisfied) {