From 324c67d16ccaf7816f4ba3afa271ecc784a5570e Mon Sep 17 00:00:00 2001 From: Fusion Agent Date: Tue, 25 Aug 2026 01:58:00 +0000 Subject: [PATCH] fix(FN-WF): make V2 rework converge, and cover S07 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause of the stalled rework: named remediation (`review-remediation-steps`) is UNAVAILABLE to a foreach-executed workflow. The parse node preserves an appended step and then answers `already-expanded`, because the foreach is PINNED to the step list it first expanded — so a step appended afterwards never receives an instance and stays `pending` forever. The merge boundary's foreach coverage then never completes and the card terminalizes with `merge-boundary-unproven` ("no pre-merge node result recorded"), measured on S05 as `steps=["Implement deterministic pipeline output:pending"]` in the review lane. `implementationOnlySteps` + `preserveRemediationSteps` on the parse node is the pair that selects that mechanism, so V2 no longer sets it and keeps the inherited "reopen-trailing" policy, which re-runs instances the foreach already owns. The planner constraint is unaffected: it lives in the seam PROMPT, while `implementationOnlySteps` only audits leakage by its own design. Code Review rework accordingly returns to `code-review` as the inherited graph does. Stated cost: a Code Review REVISE no longer regenerates the documentation. Verification rework still re-enters `verification` and replays the doc node with it, because a failing test needs re-running rather than new implementation steps. Also fixes the smoke mock: gate routing intercepted the writable Code Review Remediation session and returned a bare approval, skipping the branch that completes the steps a REVISE reopened. S07 ("unactionable Code Review rejection") now passes on builtin:coding-ideas-v2, bringing it to 15 of 19 scenarios plus the multi-repository workspace drive. S05 still does not converge and stays on its proven workflows. Lane green twice: 6 files, 82 tests, 19/19 scenarios, 115.1s and 118.4s of 150s. --- .../builtin-coding-ideas-v2-workflow.test.ts | 53 +++++++++++++------ .../builtin-coding-ideas-v2-workflow-ir.ts | 44 ++++++++++----- .../pipeline-smoke/_pipeline-harness.ts | 10 +++- .../pipeline-smoke/_pipeline-mock-scripts.ts | 15 ++++++ .../pipeline-smoke/_pipeline-scenarios.ts | 2 +- 5 files changed, 93 insertions(+), 31 deletions(-) diff --git a/packages/core/src/__tests__/builtin-coding-ideas-v2-workflow.test.ts b/packages/core/src/__tests__/builtin-coding-ideas-v2-workflow.test.ts index a6523dcd6e..fe659cd4b7 100644 --- a/packages/core/src/__tests__/builtin-coding-ideas-v2-workflow.test.ts +++ b/packages/core/src/__tests__/builtin-coding-ideas-v2-workflow.test.ts @@ -72,14 +72,23 @@ describe("builtin:coding-ideas-v2", () => { replay documentation-delivery so the docs and changeset are regenerated to include what the review demanded; re-entering at the review would merge documentation describing a superseded tree. */ - it("replays documentation on rework by re-entering upstream of it", () => { + /* + FNXC:ReviewGatedRemediation 2026-08-24-20:10: + Verification rework re-enters `verification` — a failing test needs re-running, and the doc node + downstream is replayed with it. Code Review rework returns to `code-review`, as the inherited graph + does: its remediation node is a coding session that completes the trailing steps the REVISE + reopened, and routing it through `verification` walked past the foreach so the reopened step was + never re-executed and the merge boundary refused with `merge-boundary-unproven` (measured on S05). + Cost, stated rather than hidden: a Code Review REVISE does not regenerate the documentation. + */ + it("routes each rework to the stage that can actually redo the work", () => { expect(BUILTIN_CODING_IDEAS_V2_WORKFLOW_IR.edges).toEqual(expect.arrayContaining([ { from: "verification", to: "verification-remediation", condition: "failure" }, { from: "code-review", to: "code-review-remediation", condition: "failure" }, { from: "verification-remediation", to: "verification", condition: "success", kind: "rework" }, - { from: "code-review-remediation", to: "verification", condition: "success", kind: "rework" }, + { from: "code-review-remediation", to: "code-review", condition: "success", kind: "rework" }, ])); - // Re-entering at `verification` only replays the docs because the doc node sits downstream of it. + // Verification rework replays the docs, because the doc node sits downstream of it. expect(successChainFrom("verification")).toContain("documentation-delivery"); for (const remediationId of ["verification-remediation", "code-review-remediation"]) { @@ -105,19 +114,25 @@ describe("builtin:coding-ideas-v2", () => { nothing left to execute. Inheriting Coding (Ideas)' `pre-merge-remediation` stalled the card after a Code Review REVISE. */ - it("appends named remediation steps for both review gates", () => { - for (const [remediationId, gateId] of [ - ["verification-remediation", "verification"], - ["code-review-remediation", "code-review"], - ]) { - const config = BUILTIN_CODING_IDEAS_V2_WORKFLOW_IR.nodes.find((node) => node.id === remediationId)?.config; - expect(config?.workflowAction, `${remediationId} must append named steps`).toBe("review-remediation-steps"); - expect(config?.forWorkflowStepId).toBe(gateId); - } + /* + FNXC:ReviewGatedRemediation 2026-08-24-20:10: + Named remediation (`review-remediation-steps`) is UNAVAILABLE to a foreach-executed workflow: the + parse node preserves an appended step and then answers `already-expanded`, because the foreach is + pinned to the list it first expanded, so that step never receives an instance and stays `pending` + forever. Code Review therefore keeps the inherited `pre-merge-remediation`, which reopens trailing + steps the foreach already owns. Change this only together with a foreach that can re-expand. + */ + it("keeps Code Review on reopen-trailing remediation the foreach can execute", () => { + const parse = BUILTIN_CODING_IDEAS_V2_WORKFLOW_IR.nodes.find((node) => node.id === "parse")?.config; + expect(parse?.preserveRemediationSteps).toBeUndefined(); + expect(parse?.implementationOnlySteps).toBeUndefined(); - // The inherited workflow reopens trailing steps instead, so it keeps its own send-back. - expect(BUILTIN_CODING_IDEAS_WORKFLOW_IR.nodes.find((node) => node.id === "code-review-remediation")?.config?.workflowAction) - .toBe("pre-merge-remediation"); + const codeReview = BUILTIN_CODING_IDEAS_V2_WORKFLOW_IR.nodes.find((node) => node.id === "code-review-remediation")?.config; + expect(codeReview?.workflowAction).toBe("pre-merge-remediation"); + + // Verification remediation appends named steps because it re-runs commands, not the foreach. + const verification = BUILTIN_CODING_IDEAS_V2_WORKFLOW_IR.nodes.find((node) => node.id === "verification-remediation")?.config; + expect(verification?.workflowAction).toBe("review-remediation-steps"); }); /* @@ -147,7 +162,13 @@ describe("builtin:coding-ideas-v2", () => { // The base workflow must keep the ordinary template. const basePlan = BUILTIN_CODING_IDEAS_WORKFLOW_IR.nodes.find((node) => node.id === "plan"); expect(basePlan?.config?.prompt).toContain("### Step {N}: Documentation & Delivery"); - expect(parse?.config).toMatchObject({ implementationOnlySteps: true, preserveRemediationSteps: true }); + /* + The constraint lives in the SEAM PROMPT, not in the parse node. `implementationOnlySteps` only + audits leakage ("Detection is deliberately non-destructive"), and pairing it with + `preserveRemediationSteps` would select named remediation, which a foreach-executed workflow + cannot run — see the reopen-trailing test below. + */ + expect(parse?.config?.implementationOnlySteps).toBeUndefined(); }); /* diff --git a/packages/core/src/workflows/builtin-coding-ideas-v2-workflow-ir.ts b/packages/core/src/workflows/builtin-coding-ideas-v2-workflow-ir.ts index 7ea43882a3..403a854ab8 100644 --- a/packages/core/src/workflows/builtin-coding-ideas-v2-workflow-ir.ts +++ b/packages/core/src/workflows/builtin-coding-ideas-v2-workflow-ir.ts @@ -3,7 +3,7 @@ import { parseWorkflowIr } from "./workflow-ir.js"; import { BUILTIN_CODING_IDEAS_WORKFLOW_IR } from "./builtin-coding-ideas-workflow-ir.js"; import { verificationOptionalGroupNode } from "./builtin-verification-gate-group.js"; import { documentationDeliveryOptionalGroupNode } from "./builtin-documentation-delivery-group.js"; -import { codeReviewRemediationStepsNode, verificationRemediationNode } from "./builtin-workflow-remediation-nodes.js"; +import { verificationRemediationNode } from "./builtin-workflow-remediation-nodes.js"; import { builtinPromptConfig, builtinSeamPrompt } from "./builtin-workflow-prompts.js"; import { applyImplementationOnlyStepReview } from "./builtin-plan-review-group.js"; @@ -59,8 +59,21 @@ const RAW_BUILTIN_CODING_IDEAS_V2_WORKFLOW_IR: WorkflowIr = (() => { if (plan) plan.config = { ...plan.config, ...builtinPromptConfig("planning", "Plan"), prompt: builtinSeamPrompt("planning-implementation-only") }; const planReview = ir.nodes.find((node) => node.id === "plan-review"); if (planReview) applyImplementationOnlyStepReview(planReview); - const parse = ir.nodes.find((node) => node.id === "parse"); - if (parse) parse.config = { ...parse.config, implementationOnlySteps: true, preserveRemediationSteps: true }; + /* + FNXC:ReviewGatedRemediation 2026-08-24-20:10: + This workflow deliberately does NOT set the parse node's `implementationOnlySteps` + + `preserveRemediationSteps`, so `resolveStepReopenPolicy` keeps the inherited "reopen-trailing". + That pair selects named remediation (`review-remediation-steps`), which cannot execute here: the + parse node preserves the appended step and then answers `already-expanded`, because the foreach is + PINNED to the step list it first expanded. A step appended afterwards never receives an instance, + so it stays `pending` forever — measured on S05, where the card advanced to review with + `steps=["done","pending"]` and the merge boundary refused with `merge-boundary-unproven`. + Reopening trailing steps re-runs instances the foreach already owns, which is why the inherited + Coding (Ideas) rework converges. Named remediation stays unavailable to foreach-executed workflows + until the foreach can re-expand; builtin:review-gated-coding pairs them too and never reached a + merge to expose it. + The planner is still constrained — that is the SEAM PROMPT's job, not this flag, which only audits. + */ const codeReviewIndex = ir.nodes.findIndex((node) => node.id === "code-review"); if (codeReviewIndex < 0) throw new Error("coding-ideas-v2 requires the inherited code-review gate"); @@ -81,18 +94,11 @@ const RAW_BUILTIN_CODING_IDEAS_V2_WORKFLOW_IR: WorkflowIr = (() => { the harness. The node id is kept so the inherited edges stay valid; `appendReviewRemediationSteps` keys on the failing GATE id, not on this node's id. */ - const codeReviewRemediation = ir.nodes.find((node) => node.id === "code-review-remediation"); - if (codeReviewRemediation) { - codeReviewRemediation.config = { - ...codeReviewRemediation.config, - ...codeReviewRemediationStepsNode().config, - name: "Code Review Remediation", - }; - } + /* Code Review keeps the inherited `pre-merge-remediation`, which reopens trailing steps the + foreach already owns. See the parse-node note above. */ ir.edges = ir.edges.filter((edge) => !( (edge.from === "steps" && edge.to === "completion-summary") - || (edge.from === "code-review-remediation" && edge.to === "code-review") )); /* @@ -112,7 +118,19 @@ const RAW_BUILTIN_CODING_IDEAS_V2_WORKFLOW_IR: WorkflowIr = (() => { { from: "code-review", to: "merge-gate", condition: "success" }, { from: "verification", to: "verification-remediation", condition: "failure" }, { from: "verification-remediation", to: "verification", condition: "success", kind: "rework" }, - { from: "code-review-remediation", to: "verification", condition: "success", kind: "rework" }, + /* + FNXC:ReviewGatedRemediation 2026-08-24-20:10: + Code Review rework returns to `code-review`, exactly as the inherited Coding (Ideas) graph does. + The remediation node is itself a coding session that fixes the findings and completes the + trailing steps it reopened; routing the rework through `verification` instead walked the graph + forward past the foreach, so the reopened step was never re-executed, the merge boundary's + foreach coverage stayed incomplete, and the card terminalized with + `merge-boundary-unproven` / "no pre-merge node result recorded" — measured on S05. + Cost, stated: a Code Review REVISE does NOT replay Verification or Documentation & Delivery, so + docs written before the review are not regenerated from its findings. Convergence wins over + freshness here; re-running them requires the foreach to re-expand, which it cannot yet do. + */ + { from: "code-review-remediation", to: "code-review", condition: "success", kind: "rework" }, ); return ir; })(); diff --git a/packages/engine/src/__tests__/pipeline-smoke/_pipeline-harness.ts b/packages/engine/src/__tests__/pipeline-smoke/_pipeline-harness.ts index 9da8024ef4..6c7d150f78 100644 --- a/packages/engine/src/__tests__/pipeline-smoke/_pipeline-harness.ts +++ b/packages/engine/src/__tests__/pipeline-smoke/_pipeline-harness.ts @@ -857,7 +857,15 @@ export class PipelineSmokeHarness { () => this.observe(taskId), () => this.runProductionTurn(taskId, behavior), { - maxIterations: 16, + /* + FNXC:PipelineSmoke 2026-08-24-20:10: + A turn budget, not a timeout: it bounds how many explicit graph dispatches a scenario may + 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. + */ + maxIterations: 32, signature: (state) => JSON.stringify({ column: state.column, status: state.status, diff --git a/packages/engine/src/__tests__/pipeline-smoke/_pipeline-mock-scripts.ts b/packages/engine/src/__tests__/pipeline-smoke/_pipeline-mock-scripts.ts index 8301373cea..5e8575a7ec 100644 --- a/packages/engine/src/__tests__/pipeline-smoke/_pipeline-mock-scripts.ts +++ b/packages/engine/src/__tests__/pipeline-smoke/_pipeline-mock-scripts.ts @@ -216,6 +216,21 @@ export function installPipelineMockScripts(input: { if (gateStep) { if (/^Code Review$/i.test(gateStep)) { await emitReview(context, "code"); return; } if (/^Plan Review$/i.test(gateStep)) { await emitReview(context, "plan"); return; } + /* + FNXC:PipelineSmoke 2026-08-24-20:10: + A non-review gate that can WRITE must still finish the work it was handed. Code Review + Remediation is exactly that: a coding session whose job is to complete the trailing steps the + REVISE reopened. Returning a bare approval here skipped that, so the reopened step stayed + `pending`, the merge boundary's foreach coverage never completed, and S05 terminalized with + `merge-boundary-unproven`. + */ + if (hasTaskUpdateTool) { + const pending = await input.readTaskSteps(); + for (let index = 0; index < pending.length; index += 1) { + if (pending[index] !== "pending" && pending[index] !== "in-progress") continue; + await context.invokeTool("fn_task_update", { step: index, status: "done" }); + } + } context.options.onText?.(JSON.stringify({ verdict: "APPROVE", notes: `Mock gate completed ${gateStep}.`, findings: [] })); return; } diff --git a/packages/engine/src/__tests__/pipeline-smoke/_pipeline-scenarios.ts b/packages/engine/src/__tests__/pipeline-smoke/_pipeline-scenarios.ts index 1f001f881e..7c81d509d7 100644 --- a/packages/engine/src/__tests__/pipeline-smoke/_pipeline-scenarios.ts +++ b/packages/engine/src/__tests__/pipeline-smoke/_pipeline-scenarios.ts @@ -112,7 +112,7 @@ export const PIPELINE_SCENARIOS: readonly PipelineScenario[] = [ { id: "S07", title: "Unactionable review rejection parks then recovers", - workflows: ["builtin:coding-ideas"], + workflows: ["builtin:coding-ideas", "builtin:coding-ideas-v2"], expectedTerminal: "parked", arrange: PIPELINE_SCENARIO_DRIVERS.s07Arrange, act: PIPELINE_SCENARIO_DRIVERS.s07Act,