fix(FN-WF): prove fix steps reach the card, and clear the V2 rework's leftovers
FIX STEPS, asserted on `task.steps` rather than on a spy. A failing FINAL verification and a Code Review REVISE each append pending named steps carrying their gate provenance, and the card is re-dispatched to run them; completed implementation steps stay done, because remediation appends and never reopens. A review failure with NO REVISE verdict appends nothing — a transport error must not manufacture work. And no node id other than those two gates can reach the appender, which is what keeps a red test INSIDE a step the step's own problem: the executor fixes it there instead of littering the checklist. The new tests drive the real routing seam and the real appender against the real built-in registry — an injected IR is resolved away by workflow id and would have proved nothing. CATALOG. `builtin-workflows-lifecycle.test.ts` never received an EXPECTATIONS entry when V2 was registered, so its catalog-coverage assertion has been red on main since. The merge gate does not run that file, which is why it survived. Its trail is identical to builtin:coding-ideas by design: a read-only review lane changes what happens inside the working columns, not where the card goes. REGISTRY. The description still advertised "verify … summarize", steps that no longer exist, and the layout still positioned four deleted nodes plus drew Documentation to the LEFT of Code Review — so the editor rendered the review lane backwards against its own edges. Both now match the graph. AUDIT. `implementation-only-leakage` no longer flags `testing|verification`. That regex belonged to the revision where a review gate ran the tests; testing came back to the executor, so the planner emits that step on purpose and every V2 card was reporting leakage against its own intended plan. Documentation and delivery are still flagged. pnpm lint 0 errors, test:gate green, core + engine typecheck clean, 227 tests across the touched files.
This commit is contained in:
Fusion Agent
7
.changeset/v2-fix-steps-and-catalog-repair.md
Normal file
7
.changeset/v2-fix-steps-and-catalog-repair.md
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
summary: A failed final check or review now shows named fix steps on the card; per-step failures stay in their step.
|
||||
category: fix
|
||||
dev: Adds `fix-steps-from-failed-gates.test.ts`, driving the real `requestPreMergeOptionalStepFix` and `appendReviewRemediationSteps` against the real built-in registry and asserting on `task.steps`: a `verification` failure and a `code-review` REVISE each append pending named steps with remediation provenance, a review failure with no REVISE verdict appends nothing, no other node id can reach the appender (so a per-step test failure is fixed inside its step), and `builtin:coding-ideas` keeps reopen-trailing. Also repairs three leftovers from the V2 rework: the missing `builtin:coding-ideas-v2` entry in `builtin-workflows-lifecycle.test.ts` EXPECTATIONS (catalog-coverage assertion was red on main), the registry description and layout (ghost `verification`/`verification-remediation`/`completion-summary`/`post-merge-verification` keys removed, `documentation-delivery` repositioned after `code-review` so the editor diagram matches the graph), and the `implementation-only-leakage` audit, which no longer flags `testing|verification` now that the planner emits that step deliberately.
|
||||
@@ -9,30 +9,29 @@ import { codeReviewRemediationStepsNode } from "./builtin-workflow-remediation-n
|
||||
const clone = (ir: WorkflowIr): WorkflowIr => JSON.parse(JSON.stringify(ir)) as WorkflowIr;
|
||||
|
||||
/*
|
||||
FNXC:CodingIdeasV2Workflow 2026-08-24-05:35:
|
||||
FNXC:CodingIdeasV2Workflow 2026-08-26-05:56:
|
||||
Operator intent: keep the Coding (Ideas) board exactly as it is (manual "Ideas" intake, autoTriage
|
||||
false), but stop hiding testing and documentation inside the implementation checklist. They become
|
||||
VISIBLE review-column gates, and the merge is the last thing that happens after delivery.
|
||||
false), and enforce ONE rule the inherited board does not — nothing in `in-review` writes code.
|
||||
|
||||
in-progress : steps = implementation only
|
||||
in-review : verification -> documentation-delivery -> completion-summary -> code-review -> merge
|
||||
in-progress : steps (implementation + its tests) -> the executor's own final verification
|
||||
in-review : code-review -> documentation-delivery -> merge
|
||||
|
||||
Ordering is NOT cosmetic. `execute-workflow-graph.ts` refuses any write-capable node once a Code
|
||||
Review APPROVE exists (`workspace-review-seal-required`): a passed review seals the tree so nothing
|
||||
unreviewed can reach main. `verification-step` (its name matches the write-capable classifier) and
|
||||
`documentation-delivery-step` (`toolMode: "coding"`) are both write-capable, so both MUST precede
|
||||
`code-review`. builtin:review-gated-coding places them after it and therefore deadlocks on every
|
||||
task the moment the review approves — that defect is the reason this ordering is explicit here.
|
||||
Work and proof of work both finish in `in-progress`. The executor plans and runs its own tests, then
|
||||
the FN-3345 gate re-runs the project's configured test/build commands as an independent measurement;
|
||||
a red result appends NAMED fix steps to the card instead of letting it advance (see
|
||||
bounce-verification-failure.ts). A card only crosses into review once that is green.
|
||||
|
||||
`completion-summary` runs BEFORE `code-review`, matching the inherited graph. It escapes the review
|
||||
seal (it is `toolMode: "readonly"`, so the write-capable classifier ignores it), which made "summary
|
||||
last, so it can describe the approved state" look correct — and it is wrong. The node still acquires
|
||||
a task worktree, and ANY node running between the review and the merge changes the tree the review
|
||||
approved, so `canMergeTask` refuses with "task has no provable approval for the content being
|
||||
merged" (FN-180's review-diff fingerprint). Measured: the pipeline-smoke S01 run on this workflow
|
||||
failed exactly there, then looped through verification-remediation. The seal is not the only thing
|
||||
ordering these nodes; the merge fingerprint is the other, and it is stricter.
|
||||
It stays best-effort with a success-only edge — a summary failure must never wedge a task.
|
||||
`in-review` is then three read-only milestones. Code Review judges the work — it is the only gate
|
||||
that can hold the card. Documentation reports on it. The merge is the last thing that happens.
|
||||
|
||||
Ordering is still NOT cosmetic, for a stricter reason than the review seal: ANY node that runs
|
||||
between the review and the merge changes the tree the review approved, so `canMergeTask` refuses
|
||||
with "task has no provable approval for the content being merged" (FN-180's review-diff
|
||||
fingerprint). Measured on pipeline-smoke S01, where a readonly completion-summary node placed after
|
||||
the review failed exactly there — readonly was not enough, because it still acquired a worktree.
|
||||
Documentation may follow the review ONLY because it writes nothing at all: no repository files, no
|
||||
worktree-visible change. The write-capable seal (`workspace-review-seal-required`) is the second,
|
||||
weaker constraint on this ordering.
|
||||
*/
|
||||
const RAW_BUILTIN_CODING_IDEAS_V2_WORKFLOW_IR: WorkflowIr = (() => {
|
||||
const ir = clone(BUILTIN_CODING_IDEAS_WORKFLOW_IR);
|
||||
@@ -177,21 +176,11 @@ unhandled failure modes.`;
|
||||
/* Every inherited edge touching `completion-summary` dies with the node; the lane is rebuilt below. */
|
||||
ir.edges = ir.edges.filter((edge) => edge.from !== "completion-summary" && edge.to !== "completion-summary");
|
||||
|
||||
/*
|
||||
FNXC:CodingIdeasV2Workflow 2026-08-24-05:35:
|
||||
Both remediation loops re-enter at `verification`, never directly at `code-review`. That is what
|
||||
keeps the documentation honest: a REVISE sends the fix back to in-progress, then the walk replays
|
||||
verification AND documentation-delivery, so the docs and changeset are regenerated to include what
|
||||
the review demanded before it re-reads them. Re-entering at `code-review` would leave the docs
|
||||
describing a tree that no longer exists. `verification` is the rework-region head (`reworkRegion:
|
||||
true`, `maxReworkCycles: 3`), which is what makes these edges legal.
|
||||
*/
|
||||
/*
|
||||
FNXC:CodingIdeasV2Workflow 2026-08-24-20:40:
|
||||
Push ONLY the genuinely new edges. `completion-summary -> code-review`, `code-review -> merge-gate`
|
||||
and the code-review rework are inherited from Coding (Ideas) and were being re-pushed, so the
|
||||
graph carried each of them twice — a duplicated success edge out of a review gate is a second,
|
||||
competing traversal of the same lane.
|
||||
Push ONLY the genuinely new edges. `code-review -> merge-gate` and the code-review rework are
|
||||
inherited from Coding (Ideas) and were being re-pushed, so the graph carried each of them twice —
|
||||
a duplicated success edge out of a review gate is a second, competing traversal of the same lane.
|
||||
*/
|
||||
/* `code-review -> merge-gate` is inherited and must not survive: Documentation now sits between them. */
|
||||
ir.edges = ir.edges.filter((edge) => !(edge.from === "code-review" && edge.to === "merge-gate"));
|
||||
|
||||
@@ -589,17 +589,21 @@ export const BUILTIN_WORKFLOWS: WorkflowDefinition[] = [
|
||||
updatedAt: BUILTIN_TS,
|
||||
},
|
||||
/*
|
||||
* FNXC:CodingIdeasV2Workflow 2026-08-24-05:35:
|
||||
* Same Ideas board as builtin:coding-ideas, but testing and documentation stop being hidden
|
||||
* checklist items inside the implementation steps and become visible in-review gates, with the
|
||||
* merge last. Every write-capable gate runs BEFORE Code Review because a passed review seals the
|
||||
* tree (`workspace-review-seal-required`); the readonly completion summary runs after it.
|
||||
* FNXC:CodingIdeasV2Workflow 2026-08-26-05:56:
|
||||
* Same Ideas board as builtin:coding-ideas, with one rule: in-review NEVER writes code.
|
||||
* Implementation and its tests finish in `in-progress` — the executor's own final verification
|
||||
* runs the project's configured test/build commands there, and a red result appends named fix
|
||||
* steps rather than letting the card advance. `in-review` is then three read-only milestones:
|
||||
* Code Review judges, Documentation reports, and the merge is the last thing that happens.
|
||||
* The layout is ordered left-to-right in that same sequence so the editor's diagram reads the way
|
||||
* the graph runs; nodes deleted from the graph must be deleted from this map too, or the editor
|
||||
* keeps positioning ghosts.
|
||||
*/
|
||||
{
|
||||
id: "builtin:coding-ideas-v2",
|
||||
name: "Coding (Ideas) V2",
|
||||
description:
|
||||
"Capture-first coding pipeline with visible review-column gates: park ideas in a manual intake, plan, implement per step, then verify, document, review, summarize, and merge.",
|
||||
"Capture-first coding pipeline with a read-only review lane: park ideas in a manual intake, plan, implement and test per step, then review, document, and merge.",
|
||||
kind: "workflow",
|
||||
ir: BUILTIN_CODING_IDEAS_V2_WORKFLOW_IR,
|
||||
layout: {
|
||||
@@ -611,21 +615,17 @@ export const BUILTIN_WORKFLOWS: WorkflowDefinition[] = [
|
||||
parse: { x: 570, y: 160 },
|
||||
steps: { x: 740, y: 160 },
|
||||
"review-pending-handoff": { x: 740, y: 320 },
|
||||
verification: { x: 910, y: 160 },
|
||||
"verification-remediation": { x: 910, y: 320 },
|
||||
"code-review": { x: 910, y: 160 },
|
||||
"code-review-remediation": { x: 910, y: 320 },
|
||||
"documentation-delivery": { x: 1080, y: 160 },
|
||||
"code-review": { x: 1250, y: 160 },
|
||||
"code-review-remediation": { x: 1250, y: 320 },
|
||||
"completion-summary": { x: 1420, y: 160 },
|
||||
"merge-gate": { x: 1590, y: 160 },
|
||||
"branch-group-member-integration": { x: 1760, y: 80 },
|
||||
"branch-group-promotion": { x: 1930, y: 80 },
|
||||
"merge-attempt": { x: 2100, y: 160 },
|
||||
"merge-retry": { x: 2270, y: 80 },
|
||||
"recovery-router": { x: 2270, y: 240 },
|
||||
"merge-manual-hold": { x: 1760, y: 240 },
|
||||
"post-merge-verification": { x: 2440, y: 160 },
|
||||
end: { x: 2610, y: 160 },
|
||||
"merge-gate": { x: 1250, y: 160 },
|
||||
"branch-group-member-integration": { x: 1420, y: 80 },
|
||||
"branch-group-promotion": { x: 1590, y: 80 },
|
||||
"merge-manual-hold": { x: 1420, y: 240 },
|
||||
"merge-attempt": { x: 1760, y: 160 },
|
||||
"merge-retry": { x: 1930, y: 80 },
|
||||
"recovery-router": { x: 1930, y: 240 },
|
||||
end: { x: 2100, y: 160 },
|
||||
},
|
||||
createdAt: BUILTIN_TS,
|
||||
updatedAt: BUILTIN_TS,
|
||||
|
||||
@@ -328,6 +328,26 @@ const EXPECTATIONS: BuiltinExpectation[] = [
|
||||
finalColumn: "done",
|
||||
leasedGates: ["plan-review", "code-review"],
|
||||
},
|
||||
/*
|
||||
FNXC:CodingIdeasV2Workflow 2026-08-26-05:56:
|
||||
Same board and same crossings as builtin:coding-ideas — the difference is entirely INSIDE the two
|
||||
working columns, so the trail is identical and that is the point: a read-only review lane must not
|
||||
change where the card goes, only what happens while it is there.
|
||||
This entry was missing when the workflow was registered, which left the catalog-coverage assertion
|
||||
red on main while every other test in this file passed.
|
||||
*/
|
||||
{
|
||||
id: "builtin:coding-ideas-v2",
|
||||
entryColumn: "ideas",
|
||||
trail: [
|
||||
["ideas", "todo", "graph"],
|
||||
["todo", "in-progress", "scheduler"],
|
||||
["in-progress", "in-review", "graph"],
|
||||
["in-review", "done", "graph"],
|
||||
],
|
||||
finalColumn: "done",
|
||||
leasedGates: ["plan-review", "code-review"],
|
||||
},
|
||||
{
|
||||
id: "builtin:legacy-coding",
|
||||
entryColumn: "triage",
|
||||
|
||||
@@ -0,0 +1,215 @@
|
||||
/*
|
||||
FNXC:VerificationRemediation 2026-08-26-05:56:
|
||||
The operator contract for Coding (Ideas) V2, stated as behaviour a card shows:
|
||||
|
||||
- A failing test INSIDE a step is the step's own problem. The executor fixes it there, and it must
|
||||
never become a separate fix step \u2014 otherwise every red test during implementation would litter
|
||||
the checklist with work the session is already doing.
|
||||
- A failing FINAL verification (the project's configured test/build commands, run once after every
|
||||
planned step succeeds) DOES become fix steps: the session that could have fixed it in place is
|
||||
over, so the work has to be visible on the card and re-dispatched.
|
||||
- A Code Review REVISE does the same, from the reviewer's findings.
|
||||
|
||||
These tests drive the REAL routing seam (`requestPreMergeOptionalStepFix`) and the REAL appender
|
||||
against the REAL V2 workflow IR, and assert on `task.steps` \u2014 the list an operator reads on the card.
|
||||
A spy on the appender would prove only that a function was called; the appender's `Verification`
|
||||
branch was present, correct, and caller-less for days, which is exactly the failure a spy misses.
|
||||
*/
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import type { Task, TaskStep } from "@fusion/core";
|
||||
import { getBuiltinWorkflow } from "@fusion/core";
|
||||
|
||||
import { appendReviewRemediationSteps } from "../executor/append-review-remediation-steps.js";
|
||||
import { requestPreMergeOptionalStepFix } from "../executor/request-pre-merge-optional-step-fix.js";
|
||||
|
||||
const FAILING_OUTPUT = [
|
||||
"test command `pnpm test` failed (exit 1):",
|
||||
" FAIL packages/engine/src/retry.ts:42",
|
||||
" expected 3 retries, received 1",
|
||||
].join("\n");
|
||||
|
||||
const PROMPT = [
|
||||
"# Task: FN-VR-2",
|
||||
"",
|
||||
"## File Scope",
|
||||
"",
|
||||
"- `packages/engine/src/*`",
|
||||
"",
|
||||
"## Steps",
|
||||
"",
|
||||
"### Step 1: Add the retry guard",
|
||||
"",
|
||||
"### Step 2: Testing & Verification",
|
||||
"",
|
||||
].join("\n");
|
||||
|
||||
/*
|
||||
A card that has finished every planned step and is at the verification boundary.
|
||||
The workflow is SELECTED by id and resolved through the real built-in registry, because that is what
|
||||
decides the bounce shape — injecting an IR object here would be resolved away and prove nothing.
|
||||
*/
|
||||
function harness(workflowId = "builtin:coding-ideas-v2") {
|
||||
const task = {
|
||||
id: "FN-VR-2",
|
||||
column: "in-progress",
|
||||
worktree: "/tmp/fn-vr-2",
|
||||
prompt: PROMPT,
|
||||
modifiedFiles: ["packages/engine/src/retry.ts"],
|
||||
steps: [
|
||||
{ name: "Add the retry guard", status: "done" },
|
||||
{ name: "Testing & Verification", status: "done" },
|
||||
] as TaskStep[],
|
||||
} as Task;
|
||||
|
||||
const store = {
|
||||
getTask: vi.fn(async () => task),
|
||||
getSettings: vi.fn(async () => ({ autoMerge: true })),
|
||||
logEntry: vi.fn(async () => undefined),
|
||||
updateTask: vi.fn(async (_id: string, patch: Partial<Task>) => { Object.assign(task, patch); return task; }),
|
||||
appendRemediationSteps: vi.fn(async (_id: string, steps: readonly TaskStep[], options: { wave?: number }) => {
|
||||
const appended = steps.map((step) => ({ ...step, status: "pending" as const }));
|
||||
task.steps = [...(task.steps ?? []), ...appended];
|
||||
return { task, appended, appendedCount: appended.length, wave: options.wave ?? 1 };
|
||||
}),
|
||||
getTaskWorkflowSelectionAsync: vi.fn(async () => ({ workflowId })),
|
||||
getWorkflowDefinition: vi.fn(async (id: string) => {
|
||||
const workflow = getBuiltinWorkflow(id);
|
||||
return workflow ? { ir: workflow.ir } : undefined;
|
||||
}),
|
||||
};
|
||||
|
||||
const sendTaskBackForFix = vi.fn(async () => undefined);
|
||||
const deps = {
|
||||
store: store as never,
|
||||
getRunContextFor: () => undefined,
|
||||
recoverMissingRequiredArtifacts: vi.fn(async () => undefined),
|
||||
parkPlanReviewReplanCapExhausted: vi.fn(async () => undefined),
|
||||
clearPausedAborted: vi.fn(),
|
||||
readTaskArtifact: async () => task.prompt,
|
||||
appendReviewRemediationSteps: (live: Task, info: never) => appendReviewRemediationSteps(
|
||||
{ store: store as never, readTaskArtifact: async () => task.prompt, sendTaskBackForFix },
|
||||
live,
|
||||
info,
|
||||
),
|
||||
workflowLifecycleMovesInFlight: new Set<string>(),
|
||||
sendTaskBackForFix,
|
||||
};
|
||||
|
||||
const pending = () => (task.steps ?? []).filter((step) => step.status === "pending");
|
||||
return { task, store, deps, sendTaskBackForFix, pending };
|
||||
}
|
||||
|
||||
describe("fix steps appear on the card when a gate fails", () => {
|
||||
it("turns a failing FINAL verification into named work on the card", async () => {
|
||||
const { deps, task, pending, sendTaskBackForFix } = harness();
|
||||
|
||||
const scheduled = await requestPreMergeOptionalStepFix(deps as never, task.id, task, {
|
||||
stepName: "Verification (test)",
|
||||
feedback: FAILING_OUTPUT,
|
||||
phase: "pre-merge",
|
||||
status: "failed",
|
||||
nodeId: "verification",
|
||||
});
|
||||
|
||||
expect(scheduled).toBe(true);
|
||||
expect(pending()).toHaveLength(1);
|
||||
expect(pending()[0]!.name).toContain("packages/engine/src/retry.ts");
|
||||
expect(pending()[0]!.remediation).toMatchObject({ gate: "Verification", wave: 1 });
|
||||
// Completed implementation work is preserved: remediation appends, it never reopens.
|
||||
expect(task.steps?.slice(0, 2).map((step) => step.status)).toEqual(["done", "done"]);
|
||||
// And the card is actually re-dispatched to run that step.
|
||||
expect(sendTaskBackForFix).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("turns a Code Review REVISE into named work on the card", async () => {
|
||||
const { deps, task, pending, sendTaskBackForFix } = harness();
|
||||
|
||||
const scheduled = await requestPreMergeOptionalStepFix(deps as never, task.id, task, {
|
||||
stepName: "Code Review",
|
||||
feedback: "The guard is inverted.",
|
||||
phase: "pre-merge",
|
||||
status: "failed",
|
||||
verdict: "REVISE",
|
||||
nodeId: "code-review",
|
||||
findings: [{
|
||||
id: "finding-1",
|
||||
title: "inverted guard",
|
||||
body: "Reverse the retry guard condition",
|
||||
filePath: "packages/engine/src/retry.ts",
|
||||
line: 42,
|
||||
severity: "critical",
|
||||
}],
|
||||
});
|
||||
|
||||
expect(scheduled).toBe(true);
|
||||
expect(pending()).toHaveLength(1);
|
||||
expect(pending()[0]!.name).toContain("Reverse the retry guard condition");
|
||||
expect(pending()[0]!.remediation).toMatchObject({
|
||||
gate: "Code Review",
|
||||
findingId: "finding-1",
|
||||
filePath: "packages/engine/src/retry.ts",
|
||||
});
|
||||
expect(sendTaskBackForFix).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
/*
|
||||
A gate that failed for transport/provider reasons produced no verdict and therefore no findings.
|
||||
Manufacturing work from it would hand the executor an invented task.
|
||||
*/
|
||||
it("creates nothing from a Code Review failure that carries no REVISE verdict", async () => {
|
||||
const { deps, task, pending, sendTaskBackForFix } = harness();
|
||||
|
||||
const scheduled = await requestPreMergeOptionalStepFix(deps as never, task.id, task, {
|
||||
stepName: "Code Review",
|
||||
feedback: "reviewer session aborted",
|
||||
phase: "pre-merge",
|
||||
status: "failed",
|
||||
nodeId: "code-review",
|
||||
});
|
||||
|
||||
expect(scheduled).toBe(false);
|
||||
expect(pending()).toHaveLength(0);
|
||||
expect(sendTaskBackForFix).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
/*
|
||||
THE RULE THIS PROTECTS: a red test inside a step is fixed inside that step. Only the two named
|
||||
gates may append work, so no per-step failure can reach the appender and litter the checklist.
|
||||
*/
|
||||
it("never creates a fix step for a per-step failure", async () => {
|
||||
const { task, pending, sendTaskBackForFix, store } = harness();
|
||||
|
||||
for (const nodeId of ["step-execute", "steps", "parse", undefined]) {
|
||||
const appended = await appendReviewRemediationSteps(
|
||||
{ store: store as never, readTaskArtifact: async () => task.prompt, sendTaskBackForFix },
|
||||
task,
|
||||
{ stepName: "Step 1", feedback: FAILING_OUTPUT, phase: "pre-merge", status: "failed", nodeId } as never,
|
||||
);
|
||||
expect(appended, `nodeId ${String(nodeId)} must not be able to append work`).toBe(false);
|
||||
}
|
||||
|
||||
expect(pending()).toHaveLength(0);
|
||||
expect(store.appendRemediationSteps).not.toHaveBeenCalled();
|
||||
expect(sendTaskBackForFix).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
/*
|
||||
The appended shape is selected by the WORKFLOW, not by this seam: Coding (Ideas) reopens its
|
||||
trailing step instead, and must keep doing so.
|
||||
*/
|
||||
it("leaves the inherited Coding (Ideas) workflow on its reopen-trailing bounce", async () => {
|
||||
const { deps, task, pending } = harness("builtin:coding-ideas");
|
||||
|
||||
await requestPreMergeOptionalStepFix(deps as never, task.id, task, {
|
||||
stepName: "Code Review",
|
||||
feedback: "The guard is inverted.",
|
||||
phase: "pre-merge",
|
||||
status: "failed",
|
||||
verdict: "REVISE",
|
||||
nodeId: "code-review",
|
||||
findings: [{ id: "f1", title: "t", body: "b", filePath: "packages/engine/src/retry.ts", severity: "critical" }],
|
||||
});
|
||||
|
||||
expect(pending(), "named remediation belongs to V2, not to the inherited board").toHaveLength(0);
|
||||
});
|
||||
});
|
||||
@@ -29,4 +29,23 @@ describe("review-gated parse-step preservation", () => {
|
||||
expect(writes).toEqual([[{ name: "Wire documentation link resolver", status: "pending" }, { name: "Testing & Verification", status: "pending" }]]);
|
||||
expect(audits).toContain("implementation-only-leakage");
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:PlanningDocumentationStep 2026-08-26-05:56:
|
||||
A planned `Testing & Verification` step is the INTENDED plan on these workflows — the executor owns
|
||||
testing because a readonly reviewer cannot run commands. Auditing it as review-gate leakage made
|
||||
every card report a problem with its own correct plan, which is how a signal stops being read.
|
||||
*/
|
||||
it("does not report leakage for the testing step the planner is meant to emit", async () => {
|
||||
const writes: TaskStep[][] = [];
|
||||
const audits: string[] = [];
|
||||
const runner = new ParseStepsNodeRunner({
|
||||
readArtifact: async () => "### Step 1: Add the retry guard\n### Step 2: Testing & Verification",
|
||||
writeSteps: async (_task, steps) => { writes.push(steps); },
|
||||
audit: (reason) => audits.push(reason),
|
||||
});
|
||||
await runner.run(node({ artifact: "PROMPT.md", parser: "step-headings", implementationOnlySteps: true }), { task: task(), context: {} });
|
||||
expect(writes[0]?.map((step) => step.name)).toEqual(["Add the retry guard", "Testing & Verification"]);
|
||||
expect(audits).not.toContain("implementation-only-leakage");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -133,9 +133,20 @@ export class ParseStepsNodeRunner implements WorkflowNodeRunner {
|
||||
return step;
|
||||
});
|
||||
if (cfg.implementationOnlySteps === true) {
|
||||
const leakage = steps.filter((step) => /(^|[^a-z])(testing|verification|documentation|delivery)([^a-z]|$)/i.test(step.name));
|
||||
/*
|
||||
FNXC:PlanningDocumentationStep 2026-08-26-05:56:
|
||||
TESTING IS NO LONGER LEAKAGE. This audit used to flag `testing|verification` too, from the
|
||||
revision that moved test execution into a review-column gate. That reversed: a readonly
|
||||
reviewer cannot run commands, so testing belongs to the executor and the planner emits a
|
||||
`Testing & Verification` step ON PURPOSE. Flagging it made every card on such a workflow report
|
||||
review-gate leakage for its own intended plan, which trains an operator to ignore the signal.
|
||||
Documentation and delivery ARE still leakage: those are produced by the in-review Documentation
|
||||
milestone, and a step planning them is duplicated work (see stripDocumentationDeliveryStep).
|
||||
Detection stays deliberately non-destructive — an implementation step name can legitimately
|
||||
contain these words.
|
||||
*/
|
||||
const leakage = steps.filter((step) => /(^|[^a-z])(documentation|delivery)([^a-z]|$)/i.test(step.name));
|
||||
if (leakage.length > 0) {
|
||||
// Detection is deliberately non-destructive: implementation names can legitimately contain these words.
|
||||
this.audit("implementation-only-leakage", `parse-steps node '${node.id}' detected possible review-gate work: ${leakage.map((step) => step.name).join(", ")}`);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user