root/fusion
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactor(FN-WF): classify workflow gates structurally, not by display name

Browse Source 
Three defects of the same family, all of which let a LABEL decide BEHAVIOUR.

1. `workflowNodeRequiresWorktree` matched `/(?:^|\b)(?:review|verification)(?:\b|$)/i`
   against `config.name`. A deterministic verification gate — exit codes only, no
   mutation path — was therefore classified write-capable purely because it is
   called "Verification", and the review seal refused it on every post-approval
   replay. It now keys on `reviewKind`, `workflowAction` and the optional-group id.

2. The review seal's `isCodeReview` also matched `/code review/i`, so its central
   question — "is this THE review that seals the tree?" — depended on a name an
   operator may change. Renaming the gate to "Final Review" would have silently
   stopped it being recognised while every other gate kept being sealed against it.
   That is why the rename was blocked; it no longer is.

3. `getRunningOptionalGateBadge` gated on a closed list of three step ids, so gates
   a workflow adds showed no badge at all: the operator watched an apparently idle
   card until "Merging" appeared at the end. It now asks whether the running step
   is a lane-owned gate.

Also: task cards arrive in the review lane with their step list EXPANDED, as they
already were in in-progress. The initial state is computed once per mount and a
column move remounts the card, so a card the operator had open collapsed itself
exactly when its review gates started running.

Lifecycle-column ratchet ceilings lowered to the measured counts (todo 64→12,
in-progress 197→72, in-review 213→28). They had drifted so far above reality that
the ratchet was inert for the thing it exists to stop — the TaskCard guard that hid
review-lane progress was one of those tolerated sites, and it had no way to notice.

Tests updated to the new truth rather than around it: the seal ratchet now asserts
a deterministic gate must NOT be write-capable, and the badge tests assert the
expanded list.

pnpm lint 0 errors, test:gate, verify:fast, dashboard 695, core 38, engine 8, and
three consecutive smoke runs: 122.4s, 126.3s, 124.8s of the 150s budget.
This commit is contained in:
Fusion Agent
2026-08-25 12:31:43 +00:00
parent 0c2f204acc
commit 4750b689ea
8 changed files with 89 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
.changeset/structural-workflow-predicates.md Normal file
View File

@@ -0,0 +1,7 @@
---
"@runfusion/fusion": patch
---
summary: Workflow gates are classified by what they are, not by what they are called.
category: fix
dev: Three name/id-coupling defects removed. `workflowNodeRequiresWorktree` matched `/review|verification/i` against `config.name`, so a deterministic verification gate was classified write-capable purely because of its label and the review seal refused it on every post-approval replay; it now keys on `reviewKind`, `workflowAction` and the optional-group id. The review seal's `isCodeReview` likewise matched `/code review/i`, which would have silently unrecognised a gate renamed to "Final Review". `getRunningOptionalGateBadge` replaced a closed three-id allowlist with `isNonImplementationWorkflowStepId`, so review-lane gates added by a workflow surface a running badge instead of leaving the card apparently idle until "Merging". Lifecycle-column ratchet ceilings lowered to measured counts (todo 64→12, in-progress 197→72, in-review 213→28).

14
packages/core/src/__tests__/no-hardcoded-lifecycle-columns.test.ts
View File

@@ -220,11 +220,19 @@ these values the same single-guard probe takes the gate red.
LOWER these as conversions land; a raise means a literal came back OR detection improved — and if it
is the latter, say which sites in the commit, as previous revisions of this file did.
*/
/*
FNXC:LifecycleColumnRatchet 2026-08-25-02:10:
Ceilings lowered to the MEASURED counts. They had drifted far above reality (todo 64 vs 12,
in-progress 197 vs 72, in-review 213 vs 28), which made the ratchet inert for the very thing it
exists to stop: a new hardcoded guard could be added freely inside that headroom. Not hypothetical —
the TaskCard guard that hid review-lane progress from operators was one of those tolerated sites,
and the ratchet had no way to notice it. A ceiling that sits far above the count measures nothing.
*/
const CEILINGS: Record<string, number> = {
triage: 11,
todo: 64,
"in-progress": 197,
"in-review": 213,
todo: 12,
"in-progress": 72,
"in-review": 28,
};
describe("lifecycle-column literal ratchet (AST)", () => {

7
packages/dashboard/app/components/TaskCard.tsx
View File

@@ -1115,8 +1115,15 @@ function TaskCardComponent({
: TIME_INDICATOR_COLUMNS.has(task.column);
const [isSaving, setIsSaving] = useState(false);
/*
FNXC:TaskCardWorkflowProgress 2026-08-25-02:10:
The review lane starts expanded too. This initial state is computed once per mount, and moving a
card between columns remounts it, so a card that was expanded in in-progress collapsed the moment
it reached review — exactly where a review-column workflow has gates worth watching.
*/
const [showSteps, setShowSteps] = useState(
isWipColumn ||
isReviewColumn ||
(isIntakeColumn && task.steps.some(s => s.status === "done" || s.status === "skipped"))
);
const [missionTitle, setMissionTitle] = useState<string | null>(null);

9
packages/dashboard/app/components/__tests__/TaskCard.test.tsx
View File

@@ -2797,8 +2797,13 @@ describe("TaskCard", () => {
distinct, additive affordance — it is not replaced by the bar.
*/
expect(container.querySelector(".card-progress")).not.toBeNull();
// The expandable step list stays collapsed until the operator opens it.
expect(container.querySelector(".card-steps-list")).toBeNull();
/*
FNXC:TaskCardWorkflowProgress 2026-08-25-02:10:
The list is EXPANDED on arrival in the review lane, as it already was in in-progress. The
initial state is computed once per mount and a column move remounts the card, so a card the
operator had open collapsed itself at exactly the point its review gates start running.
*/
expect(container.querySelector(".card-steps-list")).not.toBeNull();
});
/*

19
packages/dashboard/app/utils/taskProgress.ts
View File

@@ -288,13 +288,18 @@ export function getRunningOptionalGateBadge(
}
if (!(columnFlags ? isReviewColumnRole(columnFlags, task.column) : REVIEW_LANE_COLUMNS.has(task.column))) return undefined;
if (
workflowStepId !== "code-review"
&& workflowStepId !== "browser-verification"
&& workflowStepId !== "post-merge-verification"
) {
return undefined;
}
/*
FNXC:TaskCardOptionalGateBadge 2026-08-25-02:10:
Badge whatever review-lane gate is RUNNING, instead of a closed list of three ids. The old test
named `code-review`, `browser-verification` and `post-merge-verification` explicitly, so a
workflow that adds gates — builtin:coding-ideas-v2 runs Verification and Documentation & Delivery
in review — showed no badge at all for them: the operator watched an apparently idle card until
"Merging" appeared at the very end. The running gate's own state is the signal; a hardcoded id
list can only ever describe the gates that existed when it was written.
`isNonImplementationWorkflowStepId` already distinguishes a lane-owned gate from an
implementation step, and the review-lane check above bounds this to the right column.
*/
if (!isNonImplementationWorkflowStepId(workflowStepId)) return undefined;
return {
workflowStepId,

23
packages/engine/src/__tests__/coding-ideas-v2-review-seal.test.ts
View File

@@ -58,13 +58,22 @@ describe("builtin:coding-ideas-v2 review seal", () => {
});
it("keeps both write-capable gates strictly before Code Review", () => {
// If these ever stop being write-capable the ordering constraint is moot and this
// whole ratchet is measuring nothing — so assert the premise, not just the outcome.
for (const gateId of ["verification", "documentation-delivery"]) {
const gate = ir.nodes.find((node) => node.id === gateId);
expect(gate, `${gateId} is missing`).toBeDefined();
expect(isWriteCapable(gate!), `${gateId} is expected to be write-capable`).toBe(true);
}
/*
FNXC:WorkflowReviewSeal 2026-08-25-02:10:
Assert the PREMISE, not just the outcome, or this ratchet measures nothing.
`documentation-delivery` is genuinely write-capable (`toolMode: "coding"`) and is what the seal
exists to order. `verification` is NOT: it is a deterministic gate that runs commands and reads
exit codes, and it was only ever classified write-capable because the old predicate matched its
display NAME. It still runs before the review — not for the seal, but because anything between
the review and the merge invalidates the review-diff fingerprint.
*/
const documentation = ir.nodes.find((node) => node.id === "documentation-delivery");
expect(documentation, "documentation-delivery is missing").toBeDefined();
expect(isWriteCapable(documentation!), "documentation-delivery must be write-capable").toBe(true);
const verification = ir.nodes.find((node) => node.id === "verification");
expect(verification, "verification is missing").toBeDefined();
expect(isWriteCapable(verification!), "a deterministic gate must not be classified write-capable").toBe(false);
const chain = successChainFrom(ir, "steps").map((node) => node.id);
expect(chain.indexOf("verification")).toBeLessThan(chain.indexOf("code-review"));

14
packages/engine/src/executor/execute-workflow-graph.ts
View File

@@ -684,8 +684,18 @@ export async function executeWorkflowGraph(
);
if (principalAdmission) return principalAdmission;
const live = await deps.store.getTask(nodeTask.id);
const name = typeof node.config?.name === "string" ? node.config.name : "";
const isCodeReview = node.id === "code-review" || node.config?.reviewKind === "code" || /code review/i.test(name);
/*
FNXC:WorkflowReviewSeal 2026-08-25-02:10:
Structural signals only. The old test also matched `/code review/i` against the display
name, which made the seal's central question — "is this THE review that seals the tree?" —
depend on a label an operator is free to change. Renaming the gate to "Final Review" would
have silently stopped it being recognised as the sealing review while every other gate kept
being sealed against it. `reviewKind: "code"` and the node/group id are carried by every
built-in and are what the rest of the merge path already keys on.
*/
const isCodeReview = node.id === "code-review"
|| node.id === "code-review-step"
|| node.config?.reviewKind === "code";
/*
FNXC:WorkflowReviewSeal 2026-08-24-16:20:
A DETERMINISTIC verification gate is not a writer. It needs a worktree because it runs the

20
packages/engine/src/workflows/workflow-node-execution-needs.ts
View File

@@ -25,14 +25,28 @@ export function workflowNodeRequiresWorktree(
const rawCliCommand = executorKind === "cli" && typeof cfg.cliCommand === "string" && cfg.cliCommand.trim()
? cfg.cliCommand
: undefined;
const nodeName = typeof cfg.name === "string" && cfg.name.trim() ? cfg.name.trim() : node.id;
const isPlanReview = node.id === "plan-review-step" || nodeName === "Plan Review" || optionalGroupId === "plan-review";
/*
FNXC:WorkflowNodeNeeds 2026-08-25-02:10:
Classify by STRUCTURE, never by display name. The old test matched
`/(?:^|\b)(?:review|verification)(?:\b|$)/i` against `config.name`, which made behaviour hostage to
a label: a DETERMINISTIC verification gate — exit codes only, no mutation path whatsoever — was
classified write-capable purely because it is called "Verification", and the review seal then
refused it on every post-approval replay. It also silently blocked renaming a gate, since
"Final Review" and "Code Review" would classify differently for no structural reason.
`reviewKind`, `workflowAction` and the optional-group id are the real signals and are already
carried by every built-in node; a hand-authored node opts in explicitly with `reviewCanFixInline`.
*/
const isPlanReview = node.id === "plan-review-step"
|| optionalGroupId === "plan-review"
|| cfg.reviewKind === "plan";
const isDeterministicGate = cfg.workflowAction === "deterministic-verification";
const isInlineFixReview = reviewerInlineFixes !== false
&& executorKind !== "cli"
&& !isPlanReview
&& !isDeterministicGate
&& (
cfg.reviewCanFixInline === true
|| /(?:^|\b)(?:review|verification)(?:\b|$)/i.test(nodeName)
|| cfg.reviewKind === "code"
|| optionalGroupId === "code-review"
|| optionalGroupId === "browser-verification"
);