5de083ef08
Fourth PR of **U8 — the graph owns execution**. This is the IR half of
the pending-review routing move. **Inert: no behavior change.** The
behavior half is deliberately NOT in this PR, for a measured reason
below.
## What lands
A `review-handoff` seam node (`review-pending-handoff`, column
`in-review`) in `BUILTIN_CODING_WORKFLOW_IR`, with:
```
execute --outcome:review-pending--> review-pending-handoff --success--> end
```
An implementation session can end because a step is blocked on a pending
review: the agent cannot continue, and the card belongs in review rather
than in an error bucket (`status: failed` on an `in-review` row
deadlocks the merge queue). Today the **executor** performs that
transition inline, mid-session, and the graph finds out afterwards —
which is why `handleGraphFailure` carries `alreadyFinalizedToReview`, a
classifier whose only job is recognising a move the graph did not make.
Two design points worth recording, both verified against the interpreter
rather than assumed:
- **The edge goes to `end`, not to `review`.** Routing to the ordinary
`review` node would have continued the run into `merge-gate` and
`merge-attempt` on work whose steps are incomplete. "Hand off and stop"
is what the inline handoff does; the edge to `end` is what preserves it.
- **`outcome:` edges match on the node's VALUE and take priority over
generic `success`/`failure` edges** (`shouldTraverseEdge` /
`traverseChildren`). So this claims only the pending-review ending, and
a workflow that does not declare the edge falls through to its generic
`failure` edge — exactly today's behavior. That is what makes the
eventual move safe for user-authored graphs.
## Why the behavior half is not here — a measured finding
I implemented it, and backed it out. The record matters more than the
diff:
1. **`BUILTIN_CODING_WORKFLOW_IR` is not the default workflow.** It
backs `builtin:legacy-coding`; `builtin:coding` uses the
*stepwise-final-review* IR, which has no `execute` node — its
implementation runs as a `foreach` of `step-execute`.
2. **The foreach mechanism would work.** `runForeach` propagates a
failing instance's `value` up as the foreach node's own value, so a
`steps` node could carry an `outcome:review-pending` edge.
3. **But `stepExecute` flattens it first.** The seam returns `value:
result.outcome === "success" ? "step-done" : "step-failed"`, discarding
the exit before it can reach any edge.
So on the default workflow the exit cannot reach an edge, and a compat
classifier in `handleGraphFailure` keyed on the failure value cannot see
it either. **Removing the inline handoff therefore regressed the default
path**: the card stopped reaching `in-review` at all.
`executor-step-session.test.ts`'s FN-5436 case caught it —
```
FAIL FN-5436: pending-review skip on no-fn_task_done exit
> parks in-review when review request has no subsequent verdict
expected "moveTask" to be called with [ 'FN-5436-B', 'in-review' ]
Number of calls: 0
```
I could have made that green by relaxing the assertion. That would have
been appeasement of a test that was telling the truth, so the behavior
commit came out instead.
**Also caught, and worth noting as the ratchets earning their keep:**
the PR1 ownership ledger flagged the change as `runImplementation` 3 → 2
review handoffs and `handleGraphFailure` 0 → 1 — i.e. a *relocation*,
not an elimination, for every non-plain-`execute` shape. That number is
what turned "this move is good" into "this move is only good for one
workflow shape". And PR3's routing-unchanged pin plus its out-of-band
adjacency ratchet both fired, forcing the routing change to be declared
rather than slipping in.
## PR5
Thread the implementation exit through the step-session chain
(`runImplementationPhase` → `graphStepRunOnce` → `runGraphTaskStep` →
`runProjectedGraphTaskStep` → `stepExecute`) so the seam can return
`review-pending` instead of flattening to `step-failed`; add the node +
edge to the stepwise IRs; then flip the execute seam and delete the
inline handoff **in one correct step** for every built-in shape at once.
The compat path for user-authored graphs is then a single named
classifier rather than a call buried two thousand lines into a session
loop.
## Verification
- `builtin-coding-workflow-ir` + `builtin-workflows` — 76 tests green
(the layout-completeness contract required a layout entry for the new
node; it is placed off the main line because the park is an exit, not a
stage)
- `executor-step-session` + ownership ledger + exit events — 50 tests
green, unchanged
- `pnpm test:gate` green (309/10/71); `pnpm lint` clean
- Changeset included (`patch`, `internal`)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
200 lines
11 KiB
TypeScript
200 lines
11 KiB
TypeScript
import type { WorkflowIr } from "./workflow-ir-types.js";
|
|
import { parseWorkflowIr } from "./workflow-ir.js";
|
|
import { BUILTIN_WORKFLOW_SETTINGS } from "./builtin-workflow-settings.js";
|
|
import { builtinPromptConfig } from "./builtin-workflow-prompts.js";
|
|
import { browserVerificationOptionalGroupNode } from "./builtin-browser-verification-group.js";
|
|
import { codeReviewOptionalGroupNode } from "./builtin-code-review-group.js";
|
|
import { completionSummaryNode } from "./builtin-completion-summary-node.js";
|
|
import { postMergeVerificationOptionalGroupNode } from "./builtin-post-merge-group.js";
|
|
import { planReviewOptionalGroupNode } from "./builtin-plan-review-group.js";
|
|
import {
|
|
browserVerificationRemediationNode,
|
|
codeReviewRemediationNode,
|
|
planReplanNode,
|
|
} from "./builtin-workflow-remediation-nodes.js";
|
|
|
|
/**
|
|
* The built-in default workflow as a v2 IR. Its six columns have ids that are
|
|
* EXACTLY the legacy enum values in legacy order (KTD-1), so a task with no
|
|
* workflow selection resolves here and its stored `column` value is already a
|
|
* valid column id — migration rewrites zero task rows.
|
|
*
|
|
* Trait ids are plain strings (the trait registry ships in U2); the mapping
|
|
* reproduces legacy behavior verbatim (R12):
|
|
* triage = intake
|
|
* todo = hold(capacity) + reset-on-entry
|
|
* in-progress = wip + abort-on-exit + timing
|
|
* in-review = merge-blocker + human-review + stall-detection + merge
|
|
* done = complete
|
|
* archived = archived
|
|
*
|
|
* The lifecycle seam nodes are placed in their columns. Planning is explicit so
|
|
* the built-in workflow owns the specification phase rather than relying on
|
|
* triage code that runs outside the graph; execute/review/merge keep the same
|
|
* observable pipeline and failure routing.
|
|
*
|
|
* FNXC:WorkflowOptionalGroup 2026-06-21-15:10:
|
|
* The pre-merge optional `browser-verification` step is now an `optional-group`
|
|
* container node (default OFF) sitting on the success path between execute and
|
|
* review — REPLACING the legacy `workflow-step` seam node + the execution-inert
|
|
* `optionalSteps: [{ templateId: "browser-verification" }]` declaration (U6). A
|
|
* task whose `enabledWorkflowSteps` includes the group id runs browser
|
|
* verification pre-merge exactly as before; a task with it off bypasses it.
|
|
*/
|
|
const RAW_BUILTIN_CODING_WORKFLOW_IR: WorkflowIr = {
|
|
version: "v2",
|
|
name: "builtin-coding-workflow",
|
|
columns: [
|
|
// FNXC:Workflows 2026-07-05-00:00: Default-workflow intake column now displays as "Planning" while keeping the `triage` id for lifecycle/DB/type stability (FN-7599).
|
|
{ id: "triage", name: "Planning", traits: [{ trait: "intake" }] },
|
|
{
|
|
id: "todo",
|
|
name: "Todo",
|
|
traits: [{ trait: "hold", config: { release: "capacity" } }, { trait: "reset-on-entry" }],
|
|
},
|
|
{
|
|
id: "in-progress",
|
|
name: "In progress",
|
|
traits: [
|
|
{ trait: "wip", config: { limitSetting: "maxConcurrent", countPending: true } },
|
|
{ trait: "abort-on-exit" },
|
|
{ trait: "timing" },
|
|
],
|
|
},
|
|
{
|
|
id: "in-review",
|
|
name: "In review",
|
|
traits: [{ trait: "merge-blocker" }, { trait: "human-review" }, { trait: "stall-detection" }, { trait: "merge" }],
|
|
},
|
|
{ id: "done", name: "Done", traits: [{ trait: "complete" }] },
|
|
{ id: "archived", name: "Archived", traits: [{ trait: "archived" }] },
|
|
],
|
|
nodes: [
|
|
{ id: "start", kind: "start", column: "triage" },
|
|
{
|
|
id: "planning",
|
|
kind: "prompt",
|
|
column: "todo",
|
|
config: builtinPromptConfig("planning", "Plan / specify"),
|
|
},
|
|
// FNXC:PlanReviewStep 2026-07-26-17:10: plan-in-place — the specification phase runs in the
|
|
// planning lane (`todo`) before the card takes an implementation slot. See the placement note in
|
|
// builtin-stepwise-coding-workflow-ir.ts.
|
|
planReviewOptionalGroupNode("todo"),
|
|
planReplanNode("todo"),
|
|
{
|
|
id: "execute",
|
|
kind: "prompt",
|
|
column: "in-progress",
|
|
config: { ...builtinPromptConfig("execute", "Execute"), maxRetries: 2 },
|
|
},
|
|
// Pre-merge optional browser-verification (optional-group, default OFF).
|
|
browserVerificationOptionalGroupNode("in-progress"),
|
|
browserVerificationRemediationNode("in-progress"),
|
|
// FNXC:CodeReviewStep 2026-06-25-15:00:
|
|
// Pre-merge Code Review as a DEFAULT-ON optional-group (blocking gate), on the success path
|
|
// between browser-verification and review (execute → browser-verification →
|
|
// code-review → review). Runs for every coding task by default (defaultOn:true) but is
|
|
// toggleable off per task; disabled → byte-inert pass-through.
|
|
codeReviewOptionalGroupNode("in-progress"),
|
|
codeReviewRemediationNode("in-progress"),
|
|
completionSummaryNode("in-review"),
|
|
{ id: "review", kind: "prompt", column: "in-review", config: builtinPromptConfig("review", "Review") },
|
|
/*
|
|
FNXC:WorkflowExecutionOwnership 2026-07-29-09:20 (U8 / R4 — workflow-owned lifecycle):
|
|
THE PENDING-REVIEW PARK, as a graph node.
|
|
|
|
An implementation session can end because a step is blocked on a pending review: the agent
|
|
cannot continue, and the card belongs in review rather than in an error bucket (marking it
|
|
failed deadlocks a row that is both `in-review` and `failed`). Until now the EXECUTOR
|
|
performed that transition inline, mid-session, and the graph found out afterwards — which is
|
|
why `handleGraphFailure` carries `alreadyFinalizedToReview`, a classifier whose only job is
|
|
recognising a move the graph did not make.
|
|
|
|
Declaring it here makes the ending a routed outcome instead of a side effect. It is a
|
|
`review-handoff` seam (a pure lifecycle handoff, no reviewer invocation) and its only edge is
|
|
to `end`, which is what preserves today's semantics exactly: hand the card to review and STOP.
|
|
Routing to the ordinary `review` node instead would have been wrong in a way worth recording —
|
|
the run would continue into merge-gate and merge-attempt on work whose steps are incomplete.
|
|
*/
|
|
{
|
|
id: "review-pending-handoff",
|
|
kind: "prompt",
|
|
column: "in-review",
|
|
config: builtinPromptConfig("review-handoff", "Park for pending review"),
|
|
},
|
|
{ id: "merge-gate", kind: "merge-gate", column: "in-review", config: { gate: "auto-merge" } },
|
|
{ id: "merge-retry", kind: "retry-backoff", column: "in-review", config: { policy: "merge", maxAttempts: 3 } },
|
|
{ id: "merge-manual-hold", kind: "manual-merge-hold", column: "in-review", config: { release: "manual" } },
|
|
{
|
|
id: "branch-group-member-integration",
|
|
kind: "branch-group-member-integration",
|
|
column: "in-review",
|
|
config: { reworkRegion: true, maxReworkCycles: 3 },
|
|
},
|
|
{ id: "branch-group-promotion", kind: "branch-group-promotion", column: "in-review" },
|
|
{
|
|
id: "merge-attempt",
|
|
kind: "merge-attempt",
|
|
column: "in-review",
|
|
config: { capability: "task-merge", reworkRegion: true, maxReworkCycles: 3 },
|
|
},
|
|
{ id: "recovery-router", kind: "recovery-router", column: "in-review", config: { surfaces: ["merge", "retry"] } },
|
|
postMergeVerificationOptionalGroupNode("done"),
|
|
{ id: "end", kind: "end", column: "done" },
|
|
],
|
|
edges: [
|
|
{ from: "start", to: "planning" },
|
|
{ from: "planning", to: "plan-review", condition: "success" },
|
|
{ from: "plan-review", to: "execute", condition: "success" },
|
|
// execute → browser-verification (optional-group) → review. When the group is
|
|
// disabled it passes through with outcome=success and routes straight to review.
|
|
{ from: "execute", to: "browser-verification", condition: "success" },
|
|
// browser-verification → code-review → review. Each optional-group passes through with
|
|
// outcome=success when disabled, so a task with both off routes straight to review.
|
|
{ from: "browser-verification", to: "code-review", condition: "success" },
|
|
{ from: "code-review", to: "completion-summary", condition: "success" },
|
|
{ from: "completion-summary", to: "review", condition: "success" },
|
|
{ from: "review", to: "merge-gate", condition: "success" },
|
|
{ from: "merge-gate", to: "branch-group-member-integration", condition: "outcome:auto-on" },
|
|
{ from: "merge-gate", to: "merge-manual-hold", condition: "outcome:auto-off" },
|
|
{ from: "merge-retry", to: "merge-attempt", condition: "success", kind: "rework" },
|
|
{ from: "merge-manual-hold", to: "branch-group-member-integration", condition: "success", kind: "rework" },
|
|
{ from: "branch-group-member-integration", to: "branch-group-promotion", condition: "success" },
|
|
{ from: "branch-group-member-integration", to: "merge-manual-hold", condition: "outcome:manual-required" },
|
|
{ from: "branch-group-promotion", to: "merge-attempt", condition: "success" },
|
|
{ from: "branch-group-promotion", to: "merge-manual-hold", condition: "outcome:manual-required" },
|
|
{ from: "merge-attempt", to: "post-merge-verification", condition: "success" },
|
|
{ from: "post-merge-verification", to: "end", condition: "success" },
|
|
{ from: "merge-attempt", to: "merge-retry", condition: "outcome:transient-failure" },
|
|
{ from: "merge-attempt", to: "merge-manual-hold", condition: "outcome:manual-required" },
|
|
{ from: "recovery-router", to: "merge-attempt", condition: "outcome:wake-merge", kind: "rework" },
|
|
{ from: "planning", to: "end", condition: "failure" },
|
|
{ from: "plan-review", to: "plan-replan", condition: "failure" },
|
|
{ from: "plan-replan", to: "plan-review", condition: "success", kind: "rework" },
|
|
/*
|
|
FNXC:WorkflowExecutionOwnership 2026-07-29-09:25 (U8 / R4):
|
|
`outcome:` edges are matched on the node's VALUE and take priority over the generic
|
|
success/failure edges (`shouldTraverseEdge` / `traverseChildren`), so this claims the
|
|
pending-review ending without disturbing the `failure -> end` edge below it for every other
|
|
failure value. A workflow that does NOT declare this edge falls through to that generic
|
|
failure edge — i.e. exactly the pre-existing behavior — which is what lets custom workflows
|
|
keep working unchanged while the built-in routes the ending properly.
|
|
*/
|
|
{ from: "execute", to: "review-pending-handoff", condition: "outcome:review-pending" },
|
|
{ from: "review-pending-handoff", to: "end", condition: "success" },
|
|
{ from: "execute", to: "end", condition: "failure" },
|
|
{ from: "browser-verification", to: "browser-verification-remediation", condition: "failure" },
|
|
{ from: "browser-verification-remediation", to: "browser-verification", condition: "success", kind: "rework" },
|
|
{ from: "code-review", to: "code-review-remediation", condition: "failure" },
|
|
{ from: "code-review-remediation", to: "code-review", condition: "success", kind: "rework" },
|
|
{ from: "review", to: "end", condition: "failure" },
|
|
{ from: "merge-attempt", to: "end", condition: "failure" },
|
|
],
|
|
// Workflow-settings (U1, R4): declare the full moved-key catalog with defaults
|
|
// byte-equal to today's DEFAULT_PROJECT_SETTINGS literals. Inert until U3.
|
|
settings: BUILTIN_WORKFLOW_SETTINGS,
|
|
};
|
|
|
|
export const BUILTIN_CODING_WORKFLOW_IR = parseWorkflowIr(RAW_BUILTIN_CODING_WORKFLOW_IR);
|