fix(FN-WF): repair the review-gated planning seam, prompt, and workspace gate boundary
Four defects found by pointing the FN-182 pipeline-smoke harness at a review-gated
workflow. Three of them also affected builtin:review-gated-coding, where they had
been latent because that graph dies earlier on the review seal.
1. `planning-implementation-only` is a PROMPT key, never an executable seam.
`resolveSeamName` accepts exactly seven seam names and throws
`Unsupported workflow seam` otherwise, so the `plan` node threw on every task:
the graph failed at `plan`, the card bounced to todo, and the board reported
"Execution dispatch refused — task is still unplanned" — pressing Start
appeared to do nothing. The seam is now `planning`; only the prompt differs.
2. The seam prompt contradicted itself. It was the full triage prompt — whose
template MANDATES `### Step {N-1}: Testing & Verification` and
`### Step {N}: Documentation & Delivery` — plus one appended line asking for
neither. The template won, so tasks emitted both steps and ran them in
in-progress, duplicating the review gates. The template region is now removed
and replaced by an explicit prohibition. The parse node's
`implementationOnlySteps` is not a backstop: it only audits, by design.
3. `requireImplementationOnlySteps` was inert when set on an already-built
plan-review node: the prompt is assembled by `planReviewOptionalGroupNode`
and no engine code reads the flag, so the reviewer never received its
criterion. Both derived workflows now call `applyImplementationOnlyStepReview`.
4. Write-capable graph nodes declared no session boundary on workspace tasks, so
the single-repo assertion resolved the task DIRECTORY (a container of per-repo
worktrees, no `.git`) as a worktree and refused: "Refusing to start coding
agent in incomplete worktree", failing the gate before a verdict and requeuing
the task. FN-158 gave Code Review the `workspace-task-dir` boundary but not the
generic prompt path. Extracted as a pure `resolveGraphNodeSessionBoundary`.
Also reorders coding-ideas-v2 to `verification -> documentation-delivery ->
completion-summary -> code-review -> merge`. The summary escapes the review seal
(readonly) but still acquires a worktree, and any node between the review and the
merge invalidates FN-180's review-diff fingerprint.
Known incomplete: builtin:coding-ideas-v2 still does not converge end to end —
pipeline-smoke S01 reaches merge and is refused with "task has no provable
approval for the content being merged". Not yet root-caused; the workflow must be
treated as unusable until it is.
This commit is contained in:
@@ -37,12 +37,20 @@ describe("builtin:coding-ideas-v2", () => {
|
||||
expect(intake?.traits).toEqual([{ trait: "intake", config: { autoTriage: false } }]);
|
||||
});
|
||||
|
||||
it("runs verify -> document -> review -> summarize -> merge in review", () => {
|
||||
/*
|
||||
FNXC:CodingIdeasV2Workflow 2026-08-24-06:45:
|
||||
`completion-summary` sits BEFORE `code-review`, like the inherited graph. Putting it after looks
|
||||
better (the blurb could describe the approved state) and passes the review seal, because a
|
||||
readonly node is not write-capable — but it still acquires a worktree, and anything running
|
||||
between the review and the merge invalidates FN-180's review-diff fingerprint:
|
||||
"task has no provable approval for the content being merged". Measured in pipeline-smoke S01.
|
||||
*/
|
||||
it("runs verify -> document -> summarize -> review -> merge in review", () => {
|
||||
expect(successChainFrom("steps")).toEqual([
|
||||
"verification",
|
||||
"documentation-delivery",
|
||||
"code-review",
|
||||
"completion-summary",
|
||||
"code-review",
|
||||
"merge-gate",
|
||||
]);
|
||||
|
||||
@@ -80,16 +88,56 @@ describe("builtin:coding-ideas-v2", () => {
|
||||
}
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:ReviewGatedPlanning 2026-08-24-06:30:
|
||||
Measured failure this guards: a task on V2 still emitted "Testing & Verification" and
|
||||
"Documentation & Delivery" steps and ran them in in-progress, duplicating the review gates. The
|
||||
seam appended a prohibition to a prompt whose template MANDATED both steps, and the parse node
|
||||
only audits. Assert the template region is genuinely gone, not merely contradicted.
|
||||
*/
|
||||
it("stops the planner emitting the gates as duplicate implementation steps", () => {
|
||||
const ir = BUILTIN_CODING_IDEAS_V2_WORKFLOW_IR;
|
||||
const plan = ir.nodes.find((node) => node.id === "plan");
|
||||
const parse = ir.nodes.find((node) => node.id === "parse");
|
||||
const planReview = ir.nodes.find((node) => node.id === "plan-review");
|
||||
const planReviewTemplate = planReview?.config.template as { nodes?: Array<{ config?: Record<string, unknown> }> };
|
||||
const prompt = plan?.config?.prompt;
|
||||
|
||||
expect(plan?.config?.seam).toBe("planning-implementation-only");
|
||||
/*
|
||||
FNXC:ReviewGatedPlanning 2026-08-24-06:45:
|
||||
The seam must stay `planning`: `resolveSeamName` accepts seven names and throws
|
||||
`Unsupported workflow seam` otherwise, which made the plan node fail on every task and the
|
||||
board report "Execution dispatch refused — task is still unplanned". Only the PROMPT differs.
|
||||
*/
|
||||
expect(plan?.config?.seam).toBe("planning");
|
||||
expect(typeof prompt).toBe("string");
|
||||
expect(prompt).not.toContain("### Step {N-1}: Testing & Verification");
|
||||
expect(prompt).not.toContain("### Step {N}: Documentation & Delivery");
|
||||
expect(prompt).toContain("OVERRIDES the step template above");
|
||||
// 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 });
|
||||
expect(planReviewTemplate.nodes?.[0]?.config).toMatchObject({ requireImplementationOnlySteps: true });
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:ReviewGatedPlanning 2026-08-24-06:30:
|
||||
Setting requireImplementationOnlySteps on an already-built plan-review node is inert: the prompt
|
||||
is assembled by planReviewOptionalGroupNode, and no engine code reads the flag. Assert the
|
||||
reviewer actually carries the criterion, not just the boolean.
|
||||
*/
|
||||
it("gives Plan Review the implementation-only criterion in its prompt, not just a flag", () => {
|
||||
const planReview = BUILTIN_CODING_IDEAS_V2_WORKFLOW_IR.nodes.find((node) => node.id === "plan-review");
|
||||
const reviewConfig = (planReview?.config.template as { nodes?: Array<{ config?: Record<string, unknown> }> })
|
||||
.nodes?.[0]?.config;
|
||||
|
||||
expect(reviewConfig?.requireImplementationOnlySteps).toBe(true);
|
||||
expect(reviewConfig?.prompt).toContain("## Review-gated implementation steps");
|
||||
|
||||
// The inherited workflow's reviewer must stay untouched.
|
||||
const baseReview = BUILTIN_CODING_IDEAS_WORKFLOW_IR.nodes.find((node) => node.id === "plan-review");
|
||||
const baseConfig = (baseReview?.config.template as { nodes?: Array<{ config?: Record<string, unknown> }> })
|
||||
.nodes?.[0]?.config;
|
||||
expect(baseConfig?.requireImplementationOnlySteps).toBeUndefined();
|
||||
expect(baseConfig?.prompt).not.toContain("## Review-gated implementation steps");
|
||||
});
|
||||
|
||||
it("never mutates the inherited Coding (Ideas) graph", () => {
|
||||
|
||||
@@ -4,7 +4,8 @@ import { BUILTIN_CODING_IDEAS_WORKFLOW_IR } from "./builtin-coding-ideas-workflo
|
||||
import { verificationOptionalGroupNode } from "./builtin-verification-gate-group.js";
|
||||
import { documentationDeliveryOptionalGroupNode } from "./builtin-documentation-delivery-group.js";
|
||||
import { verificationRemediationNode } from "./builtin-workflow-remediation-nodes.js";
|
||||
import { builtinPromptConfig } from "./builtin-workflow-prompts.js";
|
||||
import { builtinPromptConfig, builtinSeamPrompt } from "./builtin-workflow-prompts.js";
|
||||
import { applyImplementationOnlyStepReview } from "./builtin-plan-review-group.js";
|
||||
|
||||
const clone = (ir: WorkflowIr): WorkflowIr => JSON.parse(JSON.stringify(ir)) as WorkflowIr;
|
||||
|
||||
@@ -15,7 +16,7 @@ false), but stop hiding testing and documentation inside the implementation chec
|
||||
VISIBLE review-column gates, and the merge is the last thing that happens after delivery.
|
||||
|
||||
in-progress : steps = implementation only
|
||||
in-review : verification -> documentation-delivery -> code-review -> completion-summary -> merge
|
||||
in-review : verification -> documentation-delivery -> completion-summary -> code-review -> 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
|
||||
@@ -24,9 +25,15 @@ unreviewed can reach main. `verification-step` (its name matches the write-capab
|
||||
`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.
|
||||
|
||||
`completion-summary` is deliberately AFTER `code-review`: it is `toolMode: "readonly"`, so the seal
|
||||
does not apply, and writing it last lets the card blurb describe the state that was actually
|
||||
approved. It stays best-effort with a success-only edge — a summary failure must never wedge a task.
|
||||
`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.
|
||||
*/
|
||||
const RAW_BUILTIN_CODING_IDEAS_V2_WORKFLOW_IR: WorkflowIr = (() => {
|
||||
const ir = clone(BUILTIN_CODING_IDEAS_WORKFLOW_IR);
|
||||
@@ -38,11 +45,20 @@ const RAW_BUILTIN_CODING_IDEAS_V2_WORKFLOW_IR: WorkflowIr = (() => {
|
||||
are gates now, and leaving them in PROMPT.md would run the same work twice under the same names.
|
||||
`planning-implementation-only` is the seam that carries that instruction.
|
||||
*/
|
||||
/*
|
||||
FNXC:ReviewGatedPlanning 2026-08-24-06:45:
|
||||
The SEAM stays `planning`; only the PROMPT changes. `resolveSeamName`
|
||||
(engine/workflows/workflow-node-handlers.ts) accepts exactly seven seam names and throws
|
||||
`WorkflowIrError: Unsupported workflow seam` for anything else. Declaring
|
||||
`seam: "planning-implementation-only"` therefore made the `plan` node throw on every task: the
|
||||
graph failed at `plan`, the card bounced back to todo, and the board reported "Execution dispatch
|
||||
refused — task is still unplanned" — i.e. pressing Start appeared to do nothing.
|
||||
builtin:review-gated-coding still carries that unsupported seam; it is fixed there too.
|
||||
*/
|
||||
const plan = ir.nodes.find((node) => node.id === "plan");
|
||||
if (plan) plan.config = { ...plan.config, ...builtinPromptConfig("planning-implementation-only", "Plan") };
|
||||
if (plan) plan.config = { ...plan.config, ...builtinPromptConfig("planning", "Plan"), prompt: builtinSeamPrompt("planning-implementation-only") };
|
||||
const planReview = ir.nodes.find((node) => node.id === "plan-review");
|
||||
const planTemplate = planReview?.config?.template as { nodes?: Array<{ config?: Record<string, unknown> }> } | undefined;
|
||||
if (planTemplate?.nodes?.[0]?.config) planTemplate.nodes[0].config.requireImplementationOnlySteps = true;
|
||||
if (planReview) applyImplementationOnlyStepReview(planReview);
|
||||
const parse = ir.nodes.find((node) => node.id === "parse");
|
||||
if (parse) parse.config = { ...parse.config, implementationOnlySteps: true, preserveRemediationSteps: true };
|
||||
|
||||
@@ -53,8 +69,6 @@ const RAW_BUILTIN_CODING_IDEAS_V2_WORKFLOW_IR: WorkflowIr = (() => {
|
||||
|
||||
ir.edges = ir.edges.filter((edge) => !(
|
||||
(edge.from === "steps" && edge.to === "completion-summary")
|
||||
|| (edge.from === "completion-summary" && edge.to === "code-review")
|
||||
|| (edge.from === "code-review" && edge.to === "merge-gate")
|
||||
|| (edge.from === "code-review-remediation" && edge.to === "code-review")
|
||||
));
|
||||
|
||||
@@ -70,9 +84,9 @@ const RAW_BUILTIN_CODING_IDEAS_V2_WORKFLOW_IR: WorkflowIr = (() => {
|
||||
ir.edges.push(
|
||||
{ from: "steps", to: "verification", condition: "success" },
|
||||
{ from: "verification", to: "documentation-delivery", condition: "success" },
|
||||
{ from: "documentation-delivery", to: "code-review", condition: "success" },
|
||||
{ from: "code-review", to: "completion-summary", condition: "success" },
|
||||
{ from: "completion-summary", to: "merge-gate", condition: "success" },
|
||||
{ from: "documentation-delivery", to: "completion-summary", condition: "success" },
|
||||
{ from: "completion-summary", to: "code-review", condition: "success" },
|
||||
{ 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" },
|
||||
|
||||
@@ -1,4 +1,8 @@
|
||||
import type { WorkflowIrNode } from "./workflow-ir-types.js";
|
||||
|
||||
/** The single definition of the implementation-only criterion, shared by the builder and by
|
||||
* derived workflows that clone an already-built plan-review node. */
|
||||
const IMPLEMENTATION_ONLY_STEPS_CRITERION = "\n\n## Review-gated implementation steps\nREVISE when the proposed task-step list includes testing, verification, documentation, or delivery work. Those are review-column gates in this workflow, not implementation steps.";
|
||||
import { PLAN_REVIEW_COMPLETENESS_POLICY } from "../agents/planning-review-policy.js";
|
||||
import { REVIEW_REREVIEW_POLICY, REVIEW_SEVERITY_POLICY } from "../agents/review-severity-policy.js";
|
||||
|
||||
@@ -62,6 +66,23 @@ a wip slot.
|
||||
so those call sites omit it and `assignLinearNodeColumns` places the group in whatever planning
|
||||
column the preceding node established — `todo` in practice, the same lane by a different route.
|
||||
*/
|
||||
/*
|
||||
FNXC:ReviewGatedPlanning 2026-08-24-06:30:
|
||||
Setting `requireImplementationOnlySteps` on an ALREADY-BUILT plan-review node is inert: the prompt
|
||||
is assembled here, so a later `template.nodes[0].config.requireImplementationOnlySteps = true`
|
||||
changes a flag no engine code reads and leaves the reviewer prompt without its criterion. Both
|
||||
builtin:review-gated-coding and builtin:coding-ideas-v2 did exactly that. Derived workflows that
|
||||
clone a base IR must call this instead so the prompt and the flag stay together.
|
||||
*/
|
||||
export function applyImplementationOnlyStepReview(node: WorkflowIrNode): void {
|
||||
const template = node.config?.template as { nodes?: Array<{ config?: Record<string, unknown> }> } | undefined;
|
||||
const reviewConfig = template?.nodes?.[0]?.config;
|
||||
if (!reviewConfig || typeof reviewConfig.prompt !== "string") return;
|
||||
if (reviewConfig.requireImplementationOnlySteps === true) return;
|
||||
reviewConfig.prompt = `${reviewConfig.prompt}${IMPLEMENTATION_ONLY_STEPS_CRITERION}`;
|
||||
reviewConfig.requireImplementationOnlySteps = true;
|
||||
}
|
||||
|
||||
/** Build the `plan-review` optional-group node placed between planning and execution. */
|
||||
export function planReviewOptionalGroupNode(
|
||||
column?: string,
|
||||
@@ -80,7 +101,7 @@ export function planReviewOptionalGroupNode(
|
||||
* A reviewer can distinguish implementation work from a legitimate name containing
|
||||
* "verification"; parser regexes cannot, so only this workflow opts into the criterion.
|
||||
*/
|
||||
promptConfig.prompt = `${PLAN_REVIEW_PROMPT}\n\n## Review-gated implementation steps\nREVISE when the proposed task-step list includes testing, verification, documentation, or delivery work. Those are review-column gates in this workflow, not implementation steps.`;
|
||||
promptConfig.prompt = `${PLAN_REVIEW_PROMPT}${IMPLEMENTATION_ONLY_STEPS_CRITERION}`;
|
||||
promptConfig.requireImplementationOnlySteps = true;
|
||||
}
|
||||
if (options.requireExternalIntegrationEvidence === true) {
|
||||
|
||||
@@ -4,7 +4,8 @@ import { BUILTIN_STEPWISE_FINAL_REVIEW_CODING_WORKFLOW_IR } from "./builtin-step
|
||||
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 { builtinPromptConfig } from "./builtin-workflow-prompts.js";
|
||||
import { builtinPromptConfig, builtinSeamPrompt } from "./builtin-workflow-prompts.js";
|
||||
import { applyImplementationOnlyStepReview } from "./builtin-plan-review-group.js";
|
||||
|
||||
const clone = (ir: WorkflowIr): WorkflowIr => JSON.parse(JSON.stringify(ir)) as WorkflowIr;
|
||||
|
||||
@@ -17,11 +18,15 @@ const RAW_BUILTIN_REVIEW_GATED_CODING_WORKFLOW_IR: WorkflowIr = (() => {
|
||||
const ir = clone(BUILTIN_STEPWISE_FINAL_REVIEW_CODING_WORKFLOW_IR);
|
||||
ir.name = "builtin-review-gated-coding";
|
||||
|
||||
/* FNXC:ReviewGatedPlanning 2026-08-24-06:45: `planning-implementation-only` is a PROMPT key, not
|
||||
an executable seam — `resolveSeamName` throws for it, so the plan node failed on every task.
|
||||
Keep the seam `planning` and swap only the prompt. */
|
||||
const plan = ir.nodes.find((node) => node.id === "plan");
|
||||
if (plan) plan.config = builtinPromptConfig("planning-implementation-only", "Plan");
|
||||
if (plan) plan.config = { ...builtinPromptConfig("planning", "Plan"), prompt: builtinSeamPrompt("planning-implementation-only") };
|
||||
/* FNXC:ReviewGatedPlanning 2026-08-24-06:30: the flag alone was inert here too — see
|
||||
applyImplementationOnlyStepReview. */
|
||||
const planReview = ir.nodes.find((node) => node.id === "plan-review");
|
||||
const planTemplate = planReview?.config?.template as { nodes?: Array<{ config?: Record<string, unknown> }> } | undefined;
|
||||
if (planTemplate?.nodes?.[0]?.config) planTemplate.nodes[0].config.requireImplementationOnlySteps = true;
|
||||
if (planReview) applyImplementationOnlyStepReview(planReview);
|
||||
const parse = ir.nodes.find((node) => node.id === "parse");
|
||||
if (parse) parse.config = { ...parse.config, implementationOnlySteps: true, preserveRemediationSteps: true };
|
||||
|
||||
|
||||
@@ -6,12 +6,51 @@ const DEFAULT_TRIAGE_FAST_PROMPT = BUILTIN_AGENT_PROMPTS.find((prompt) => prompt
|
||||
const DEFAULT_REVIEWER_PROMPT = BUILTIN_AGENT_PROMPTS.find((prompt) => prompt.role === "reviewer")?.prompt ?? "";
|
||||
const DEFAULT_MERGER_PROMPT = BUILTIN_AGENT_PROMPTS.find((prompt) => prompt.role === "merger")?.prompt ?? "";
|
||||
|
||||
/*
|
||||
FNXC:ReviewGatedPlanning 2026-08-24-06:30:
|
||||
The review-gated seam used to be `DEFAULT_TRIAGE_PROMPT` plus one appended sentence. That does not
|
||||
work and was measured not working: the base prompt's PROMPT.md template MANDATES
|
||||
`### Step {N-1}: Testing & Verification` and `### Step {N}: Documentation & Delivery`, with full
|
||||
checklists, so a trailing line telling the planner to omit them is a self-contradicting prompt and
|
||||
the detailed template wins. Tasks kept emitting both steps, the executor ran them in in-progress,
|
||||
and the review-column gates then redid the same work under the same names.
|
||||
The parse node's `implementationOnlySteps` is NOT a backstop — it only audits, by design
|
||||
("Detection is deliberately non-destructive"), because a legitimate implementation step name can
|
||||
contain these words.
|
||||
So the template region is REMOVED and replaced by an explicit prohibition. If the base prompt is
|
||||
reworded and the anchors stop matching, the strip degrades to the old append rather than breaking
|
||||
planning at runtime; `builtin-workflow-prompts.test.ts` fails loudly on that drift.
|
||||
*/
|
||||
const REVIEW_GATE_STEP_TEMPLATE_START = "### Step {N-1}: Testing & Verification";
|
||||
const REVIEW_GATE_STEP_TEMPLATE_END = "## Documentation Requirements";
|
||||
|
||||
const REVIEW_GATED_STEP_CONTRACT = `## Review-gated step contract (OVERRIDES the step template above)
|
||||
|
||||
This workflow runs testing, verification, documentation, and delivery as REVIEW-COLUMN GATES after
|
||||
implementation. They are not task steps here.
|
||||
|
||||
- Do NOT emit a "Testing & Verification" step.
|
||||
- Do NOT emit a "Documentation & Delivery" step.
|
||||
- Emit implementation steps only, ending with the last implementation step.
|
||||
- Per-step verification bullets stay: each implementation step still runs its own targeted tests.
|
||||
|
||||
`;
|
||||
|
||||
export function applyReviewGatedStepContract(prompt: string): string {
|
||||
const start = prompt.indexOf(REVIEW_GATE_STEP_TEMPLATE_START);
|
||||
const end = prompt.indexOf(REVIEW_GATE_STEP_TEMPLATE_END);
|
||||
if (start < 0 || end < 0 || end <= start) {
|
||||
return `${prompt}\n\n${REVIEW_GATED_STEP_CONTRACT}`;
|
||||
}
|
||||
return `${prompt.slice(0, start)}${REVIEW_GATED_STEP_CONTRACT}${prompt.slice(end)}`;
|
||||
}
|
||||
|
||||
export const BUILTIN_SEAM_PROMPTS: Record<string, string> = {
|
||||
execute: DEFAULT_EXECUTOR_PROMPT,
|
||||
planning: DEFAULT_TRIAGE_PROMPT,
|
||||
"planning-fast": DEFAULT_TRIAGE_FAST_PROMPT,
|
||||
/* Review-gated tasks keep test and delivery work in review-column gates. */
|
||||
"planning-implementation-only": `${DEFAULT_TRIAGE_PROMPT}\n\n## Review-gated step contract\nProduce implementation steps only. Do not add Testing & Verification or Documentation & Delivery steps; those run as review-column gates after implementation.`,
|
||||
"planning-implementation-only": applyReviewGatedStepContract(DEFAULT_TRIAGE_PROMPT),
|
||||
"step-execute": DEFAULT_EXECUTOR_PROMPT,
|
||||
review: DEFAULT_REVIEWER_PROMPT,
|
||||
merge: DEFAULT_MERGER_PROMPT,
|
||||
|
||||
@@ -49,8 +49,9 @@ describe("builtin:coding-ideas-v2 review seal", () => {
|
||||
|
||||
it("has no write-capable node after Code Review", () => {
|
||||
const after = successChainFrom(ir, "code-review");
|
||||
// Guard the guard: an empty chain would make this assertion vacuously true.
|
||||
expect(after.map((node) => node.id)).toContain("completion-summary");
|
||||
// Guard the guard: an empty chain would make this assertion vacuously true. Everything the
|
||||
// review must cover now runs before it, so what remains downstream is the merge machinery.
|
||||
expect(after.map((node) => node.id)).toContain("merge-gate");
|
||||
|
||||
const offenders = after.filter(isWriteCapable).map((node) => node.id);
|
||||
expect(offenders).toEqual([]);
|
||||
@@ -68,6 +69,14 @@ describe("builtin:coding-ideas-v2 review seal", () => {
|
||||
const chain = successChainFrom(ir, "steps").map((node) => node.id);
|
||||
expect(chain.indexOf("verification")).toBeLessThan(chain.indexOf("code-review"));
|
||||
expect(chain.indexOf("documentation-delivery")).toBeLessThan(chain.indexOf("code-review"));
|
||||
/*
|
||||
Not seal-driven but merge-driven: `completion-summary` escapes the write-capable classifier
|
||||
(readonly) yet still acquires a worktree, and any node between the review and the merge
|
||||
invalidates FN-180's review-diff fingerprint ("no provable approval for the content being
|
||||
merged"). Nothing may sit between them.
|
||||
*/
|
||||
expect(chain.indexOf("completion-summary")).toBeLessThan(chain.indexOf("code-review"));
|
||||
expect(chain[chain.indexOf("code-review") + 1]).toBe("merge-gate");
|
||||
});
|
||||
|
||||
it("keeps the completion summary readonly so it may run after the seal", () => {
|
||||
|
||||
@@ -0,0 +1,65 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { resolveGraphNodeSessionBoundary } from "../executor/run-graph-custom-node.js";
|
||||
|
||||
/*
|
||||
FNXC:WorkspaceBoundary 2026-08-24-06:30:
|
||||
Measured failure this guards, on a multi-repo project:
|
||||
|
||||
Workflow node 'documentation-delivery-step' requires a task worktree — acquiring worktree
|
||||
[pre-merge] Workflow step failed: Documentation & Delivery
|
||||
Documentation & Delivery failed before producing a verdict:
|
||||
Refusing to start coding agent in incomplete worktree:
|
||||
.../multi-repo/.fusion/worktrees/mult-012
|
||||
Auto-recovered: session start refused unusable worktree — requeued to todo (attempt 1/3)
|
||||
|
||||
`mult-012` is the task DIRECTORY: a container whose per-repository worktrees (`mult-012/repo1`) hold
|
||||
the Git metadata. The generic graph prompt path declared no session boundary, so the single-repo
|
||||
assertion resolved that container as a worktree and refused. FN-158 gave Code Review the
|
||||
`workspace-task-dir` boundary but not this path, so every OTHER write-capable gate stayed broken on
|
||||
workspace projects — it simply went unnoticed because builtin:review-gated-coding died earlier on
|
||||
the review seal.
|
||||
*/
|
||||
describe("graph node workspace session boundary", () => {
|
||||
const base = {
|
||||
isWorkspace: true,
|
||||
writeCapable: true,
|
||||
legacyWorkspaceLayout: false,
|
||||
rootDir: "/ws",
|
||||
worktreePath: "/ws/.fusion/worktrees/mult-012",
|
||||
confirmedRepositories: ["repo1", "repo2"] as const,
|
||||
};
|
||||
|
||||
it("declares a workspace-task-dir boundary validating the per-repository children", () => {
|
||||
expect(resolveGraphNodeSessionBoundary(base)).toEqual({
|
||||
kind: "workspace-task-dir",
|
||||
writableRoot: "/ws/.fusion/worktrees/mult-012",
|
||||
projectRoot: "/ws",
|
||||
repoRoots: [
|
||||
{ repoRelPath: "repo1", repoRootDir: "/ws/repo1" },
|
||||
{ repoRelPath: "repo2", repoRootDir: "/ws/repo2" },
|
||||
],
|
||||
});
|
||||
});
|
||||
|
||||
it("leaves single-repository tasks on their existing implicit boundary", () => {
|
||||
expect(resolveGraphNodeSessionBoundary({ ...base, isWorkspace: false })).toBeUndefined();
|
||||
});
|
||||
|
||||
it("does not touch read-only nodes, which never run in the task directory", () => {
|
||||
expect(resolveGraphNodeSessionBoundary({ ...base, writeCapable: false })).toBeUndefined();
|
||||
});
|
||||
|
||||
it("keeps the legacy per-repo layout on its child-worktree boundary", () => {
|
||||
expect(resolveGraphNodeSessionBoundary({ ...base, legacyWorkspaceLayout: true })).toBeUndefined();
|
||||
});
|
||||
|
||||
/*
|
||||
A `workspace-task-dir` descriptor with zero repoRoots is itself refused by the session guard
|
||||
("Refusing workspace-task-dir session without declared repository roots"), so an unconfirmed scope
|
||||
must fall back rather than trade one refusal for another.
|
||||
*/
|
||||
it("falls back when the repository scope is not confirmed", () => {
|
||||
expect(resolveGraphNodeSessionBoundary({ ...base, confirmedRepositories: undefined })).toBeUndefined();
|
||||
expect(resolveGraphNodeSessionBoundary({ ...base, confirmedRepositories: [] })).toBeUndefined();
|
||||
});
|
||||
});
|
||||
@@ -37,6 +37,7 @@ import { parseAwaitInputSentinel } from "./await-input-parse.js";
|
||||
import { buildAgentPersona } from "./agent-binding-pure.js";
|
||||
import { reviewWorkspacePerRepo } from "./workspace-review-per-repo.js";
|
||||
import type { ReviewResult } from "../execution/reviewer.js";
|
||||
import type { SessionBoundaryDescriptor } from "../agents/agent-runtime.js";
|
||||
import { runDeterministicVerificationGate } from "../workflow-node-runners/verification-gate.js";
|
||||
|
||||
const WORKFLOW_THINKING_LEVEL_SET: ReadonlySet<string> = new Set(THINKING_LEVELS);
|
||||
@@ -64,6 +65,39 @@ export type RunGraphCustomNodeDeps = {
|
||||
runRawCliCommand: AnyFn;
|
||||
};
|
||||
|
||||
/*
|
||||
FNXC:WorkspaceBoundary 2026-08-24-06:30:
|
||||
Pure so the decision is testable without driving a whole graph run. A write-capable node on a
|
||||
workspace task runs from the task DIRECTORY, a container of per-repository worktrees with no `.git`
|
||||
of its own; with no declared boundary the session applies the single-repo assertion to that
|
||||
container and refuses to start ("Refusing to start coding agent in incomplete worktree"), so the
|
||||
gate fails before producing a verdict and the task requeues to todo. `workspace-task-dir` validates
|
||||
the per-repository children instead. `undefined` preserves the existing implicit boundary for
|
||||
single-repo tasks, for the legacy per-repo layout, and when the scope is unconfirmed (a
|
||||
zero-repoRoots descriptor is itself refused, so guessing only trades one refusal for another).
|
||||
*/
|
||||
export function resolveGraphNodeSessionBoundary(input: {
|
||||
isWorkspace: boolean;
|
||||
writeCapable: boolean;
|
||||
legacyWorkspaceLayout: boolean;
|
||||
rootDir: string;
|
||||
worktreePath: string;
|
||||
confirmedRepositories?: readonly string[];
|
||||
}): SessionBoundaryDescriptor | undefined {
|
||||
if (!input.isWorkspace || !input.writeCapable || input.legacyWorkspaceLayout) return undefined;
|
||||
const repoRoots = (input.confirmedRepositories ?? []).map((repoRelPath) => ({
|
||||
repoRelPath,
|
||||
repoRootDir: join(input.rootDir, repoRelPath),
|
||||
}));
|
||||
if (repoRoots.length === 0) return undefined;
|
||||
return {
|
||||
kind: "workspace-task-dir",
|
||||
writableRoot: input.worktreePath,
|
||||
projectRoot: input.rootDir,
|
||||
repoRoots,
|
||||
};
|
||||
}
|
||||
|
||||
export async function runGraphCustomNode(
|
||||
deps: RunGraphCustomNodeDeps,
|
||||
node: WorkflowIrNode,
|
||||
@@ -280,6 +314,30 @@ export async function runGraphCustomNode(
|
||||
const worktreePath = workspaceConfig && !writeCapable
|
||||
? deps.rootDir
|
||||
: executionTarget.worktree || legacyWorkspacePath || workspaceTaskDir!;
|
||||
/*
|
||||
FNXC:WorkspaceBoundary 2026-08-24-06:30:
|
||||
A write-capable graph node on a workspace task runs from the TASK DIRECTORY, which is a plain
|
||||
container of per-repository worktrees and carries no `.git` of its own. Without a declared
|
||||
boundary the session falls back to the single-repo assertion, which resolves that container as
|
||||
a worktree and refuses to start: "Refusing to start coding agent in incomplete worktree". The
|
||||
node then fails before producing a verdict and the task requeues to todo — measured on a
|
||||
Documentation & Delivery gate in a multi-repo project.
|
||||
FN-158 gave Code Review this boundary (see reviewBoundary below) but not the generic prompt
|
||||
path, so every OTHER write-capable gate stayed broken on workspace projects. `workspace-task-dir`
|
||||
validates the per-repository CHILDREN instead of the root, which is what makes the session legal.
|
||||
Single-repository tasks keep their existing implicit boundary; a workspace task whose scope is
|
||||
unconfirmed also keeps it, because `workspace-task-dir` with zero repoRoots is itself refused.
|
||||
*/
|
||||
const nodeSessionBoundary = resolveGraphNodeSessionBoundary({
|
||||
isWorkspace: Boolean(workspaceConfig),
|
||||
writeCapable,
|
||||
legacyWorkspaceLayout: Boolean(legacyWorkspacePath),
|
||||
rootDir: deps.rootDir,
|
||||
worktreePath,
|
||||
confirmedRepositories: executionTarget.repositoryScope?.state === "confirmed"
|
||||
? executionTarget.repositoryScope.repositories
|
||||
: undefined,
|
||||
});
|
||||
if (isDeterministicVerificationGate) {
|
||||
return runDeterministicVerificationGate({ store: deps.store }, node, live, settings, worktreePath);
|
||||
}
|
||||
@@ -631,7 +689,12 @@ export async function runGraphCustomNode(
|
||||
} else {
|
||||
outcome = mode === "script"
|
||||
? await deps.executeScriptWorkflowStep(live, step, worktreePath, settings, nodeEnv)
|
||||
: await deps.executeWorkflowStep(live, step, worktreePath, settings, nodeEnv, { unattended, principalAgentId, outputLanguage });
|
||||
: await deps.executeWorkflowStep(live, step, worktreePath, settings, nodeEnv, {
|
||||
unattended,
|
||||
principalAgentId,
|
||||
outputLanguage,
|
||||
...(nodeSessionBoundary ? { sessionBoundary: nodeSessionBoundary } : {}),
|
||||
});
|
||||
}
|
||||
/*
|
||||
* FNXC:WorkflowReviewFindings 2026-08-05-06:29:
|
||||
|
||||
Reference in New Issue
Block a user