fix(FN-WF): make V2 rework converge, and cover S07
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.
This commit is contained in:
@@ -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();
|
||||
});
|
||||
|
||||
/*
|
||||
|
||||
@@ -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;
|
||||
})();
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user