FN-7293: scope external evidence to per-step review

This commit is contained in:
gsxdsm
2026-06-30 12:25:43 -07:00
parent 5f67c85fea
commit 658b3511c7
10 changed files with 270 additions and 21 deletions

View File

@@ -0,0 +1,7 @@
---
"@runfusion/fusion": patch
---
summary: Scope external-integration plan evidence checks to Coding (per-step review).
category: fix
dev: Triage no longer blocks generated plans for missing external evidence; the per-step review Plan Review gate does.

View File

@@ -9,6 +9,7 @@ import {
isBuiltinWorkflowPluginGated,
} from "../builtin-workflows.js";
import { BUILTIN_CODING_WORKFLOW_IR } from "../builtin-coding-workflow-ir.js";
import { BUILTIN_STEPWISE_CODING_WORKFLOW_IR } from "../builtin-stepwise-coding-workflow-ir.js";
import { BROWSER_VERIFICATION_GROUP_ID, BROWSER_VERIFICATION_STEP_NODE_ID } from "../builtin-browser-verification-group.js";
import { CODE_REVIEW_STEP_NODE_ID } from "../builtin-code-review-group.js";
import { PLAN_REVIEW_GROUP_ID, PLAN_REVIEW_STEP_NODE_ID } from "../builtin-plan-review-group.js";
@@ -101,6 +102,18 @@ describe("built-in workflows", () => {
}
});
it("scopes deterministic external-integration plan validation to Coding (per-step review)", () => {
const perStepPlanReview = planReviewInnerConfig(BUILTIN_STEPWISE_CODING_WORKFLOW_IR);
const defaultCodingPlanReview = planReviewInnerConfig(BUILTIN_CODING_WORKFLOW_IR);
const legacyCodingPlanReview = planReviewInnerConfig(getBuiltinWorkflow("builtin:legacy-coding")!.ir);
const quickFixPlanReview = planReviewInnerConfig(getBuiltinWorkflow("builtin:quick-fix")!.ir);
expect(perStepPlanReview.requireExternalIntegrationEvidence).toBe(true);
expect(defaultCodingPlanReview.requireExternalIntegrationEvidence).toBeUndefined();
expect(legacyCodingPlanReview.requireExternalIntegrationEvidence).toBeUndefined();
expect(quickFixPlanReview.requireExternalIntegrationEvidence).toBeUndefined();
});
it("all built-in Code Review optional groups are blocking gates", () => {
for (const workflow of BUILTIN_WORKFLOWS) {
const codeReview = workflow.ir.nodes.find((node) => node.id === "code-review");

View File

@@ -41,8 +41,23 @@ Be specific: cite the plan section or file path for every finding and explain th
/** Build the `plan-review` optional-group node placed between planning and execution. */
export function planReviewOptionalGroupNode(
column: string,
options: { defaultOn?: boolean; maxRevisions?: number | "unbounded" } = {},
options: { defaultOn?: boolean; maxRevisions?: number | "unbounded"; requireExternalIntegrationEvidence?: boolean } = {},
): WorkflowIrNode {
const promptConfig: Record<string, unknown> = {
name: PLAN_REVIEW_NAME,
description: PLAN_REVIEW_DESCRIPTION,
prompt: PLAN_REVIEW_PROMPT,
toolMode: "readonly",
gateMode: "gate",
};
if (options.requireExternalIntegrationEvidence === true) {
/*
* FNXC:PlanValidation 2026-06-30-08:56:
* Only Coding (per-step review) keeps deterministic external-integration evidence as part of Plan Review. Default Coding must not inherit this pre-review blocker; graph execution reads this flag from the Plan Review template node and turns missing evidence into a REVISE outcome.
*/
promptConfig.requireExternalIntegrationEvidence = true;
}
return {
id: PLAN_REVIEW_GROUP_ID,
kind: "optional-group",
@@ -66,13 +81,7 @@ export function planReviewOptionalGroupNode(
{
id: PLAN_REVIEW_STEP_NODE_ID,
kind: "prompt",
config: {
name: PLAN_REVIEW_NAME,
description: PLAN_REVIEW_DESCRIPTION,
prompt: PLAN_REVIEW_PROMPT,
toolMode: "readonly",
gateMode: "gate",
},
config: promptConfig,
},
],
edges: [],

View File

@@ -86,7 +86,7 @@ const RAW_BUILTIN_STEPWISE_CODING_WORKFLOW_IR: WorkflowIr = {
{ id: "start", kind: "start", column: "triage" },
// Planning seam: produces PROMPT.md (the declared step-source artifact).
{ id: "plan", kind: "prompt", column: "in-progress", config: builtinPromptConfig("planning", "Plan") },
planReviewOptionalGroupNode("in-progress"),
planReviewOptionalGroupNode("in-progress", { requireExternalIntegrationEvidence: true }),
planReplanNode("triage"),
// KTD-12: parse the planned PROMPT.md into the task step list. This node must
// dominate the foreach (validator-enforced).

View File

@@ -33,6 +33,19 @@ const RAW_BUILTIN_STEPWISE_FINAL_REVIEW_CODING_WORKFLOW_IR: WorkflowIr = (() =>
throw new Error("stepwise final-review built-in requires the stepwise foreach template");
}
const inheritedPlanReview = ir.nodes.find((node) => node.id === "plan-review");
const inheritedPlanReviewTemplate = inheritedPlanReview?.config?.template as
| { nodes?: Array<{ id: string; config?: Record<string, unknown> }> }
| undefined;
const inheritedPlanReviewStep = inheritedPlanReviewTemplate?.nodes?.find((node) => node.id === "plan-review-step");
if (inheritedPlanReviewStep?.config) {
/*
* FNXC:PlanValidation 2026-06-30-09:00:
* Default Coding is cloned from Coding (per-step review), but the deterministic external-integration evidence check belongs only to the review-heavy/per-step workflow. Remove the inherited flag here so default Coding relies on the normal Plan Review agent rather than pre-agent deterministic rejection.
*/
delete inheritedPlanReviewStep.config.requireExternalIntegrationEvidence;
}
const planIndex = ir.nodes.findIndex((node) => node.id === "plan");
if (planIndex < 0) {
throw new Error("stepwise final-review built-in requires a plan node");

View File

@@ -95,6 +95,23 @@ function browserVerificationStep(overrides: Record<string, unknown> = {}) {
};
}
function planReviewStep(overrides: Record<string, unknown> = {}) {
return {
id: "graph:plan-review-step",
name: "Plan Review",
description: "",
mode: "prompt",
phase: "pre-merge",
gateMode: "gate",
prompt: "Review the plan.",
toolMode: "readonly",
enabled: true,
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
...overrides,
};
}
describe("browser-verification workflow-step browser capability", () => {
beforeEach(() => {
resetExecutorMocks();
@@ -264,4 +281,34 @@ describe("browser-verification workflow-step browser capability", () => {
expect(store.logEntry.mock.calls.some(([, message]: [string, string]) => message.includes("[browser-verification]"))).toBe(false);
expect(store.appendAgentLog.mock.calls.some(([, message]: [string, string]) => message.includes("[browser-verification]"))).toBe(false);
});
it("returns a Plan Review revision for flagged external-integration evidence gaps without launching a session", async () => {
const store = createMockStore();
store.getTask.mockResolvedValue({
...baseTask(),
prompt: "## Mission\nAdd an external CLI.\n\n## Steps\n- Download and run `wt` from https://github.com/worktrunk/worktrunk/releases/latest/download/wt-linux-x64.tar.gz\n",
});
const executor = makeExecutor(store);
const result = await (executor as any).executeWorkflowStep(
baseTask(),
planReviewStep({ requireExternalIntegrationEvidence: true }),
"/tmp/wt",
{},
undefined,
undefined,
);
expect(result).toMatchObject({
success: false,
revisionRequested: true,
verdict: "REVISE",
});
expect(result.notes).toContain("External-integration evidence gaps");
expect(mockedCreateFnAgent).not.toHaveBeenCalled();
expect(store.logEntry).toHaveBeenCalledWith(
"FN-7130",
expect.stringContaining("Plan Review deterministic external-integration evidence check requested revision"),
);
});
});

View File

@@ -239,6 +239,16 @@ vi.mock("../step-session-executor.js", () => ({
steerActiveSessions: mockSteerActiveSessions,
};
}),
extractSection: (prompt: string, sectionName: string) => {
const escaped = sectionName.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
const match = new RegExp(`^## ${escaped}\\s*$`, "m").exec(prompt);
if (!match) return "";
const start = match.index;
const afterStart = start + match[0].length;
const nextHeading = prompt.indexOf("\n## ", afterStart);
const end = nextHeading === -1 ? prompt.length : nextHeading;
return prompt.slice(start, end).trim();
},
}));
vi.mock("../rate-limit-retry.js", () => ({

View File

@@ -4,6 +4,7 @@ import { join } from "node:path";
import { tmpdir } from "node:os";
import type { TaskStore, TaskDetail, Settings } from "@fusion/core";
import { TriageProcessor } from "../triage.js";
import { reviewStep } from "../reviewer.js";
vi.mock("@fusion/core", async (importOriginal) => {
const { createEngineCoreMock } = await import("../test/mockCore.js");
@@ -12,6 +13,10 @@ vi.mock("@fusion/core", async (importOriginal) => {
});
});
vi.mock("../reviewer.js", () => ({
reviewStep: vi.fn().mockResolvedValue({ verdict: "APPROVE", review: "ok", summary: "ok" }),
}));
function createMockStore(overrides: Partial<TaskStore> = {}): TaskStore {
return {
getTask: vi.fn(),
@@ -58,7 +63,7 @@ const mockTaskDetail: TaskDetail = {
};
describe("triage deterministic plan validation for external integration evidence", () => {
it("rejects incomplete evidence without invoking reviewer", async () => {
it("does not reject incomplete evidence during deterministic triage validation", async () => {
const rootDir = await mkdtemp(join(tmpdir(), "fusion-triage-ext-evidence-"));
try {
const taskId = "FN-5321";
@@ -71,7 +76,11 @@ describe("triage deterministic plan validation for external integration evidence
`## Mission\nAdd third-party external binary integration.\n## Steps\n- install and probe \`worktrunk\` from release URL https://github.com/${fabricatedRepo}/releases/latest/download/worktrunk.tar.gz\n`,
);
expect(failure).toContain("External-integration evidence gaps");
expect(failure).toBeNull();
expect(store.logEntry).not.toHaveBeenCalledWith(
taskId,
expect.stringContaining("external-integration evidence gaps"),
);
} finally {
await rm(rootDir, { recursive: true, force: true });
}
@@ -110,4 +119,58 @@ describe("triage deterministic plan validation for external integration evidence
await rm(rootDir, { recursive: true, force: true });
}
});
it("blocks the per-step review workflow during Plan Review when external evidence is missing", async () => {
const rootDir = await mkdtemp(join(tmpdir(), "fusion-triage-ext-evidence-plan-review-"));
try {
const taskId = "FN-5321";
const prompt = "## Mission\nAdd an external CLI.\n\n## Steps\n- Download and run `wt` from https://github.com/worktrunk/worktrunk/releases/latest/download/wt-linux-x64.tar.gz\n";
const task = {
...mockTaskDetail,
id: taskId,
enabledWorkflowSteps: ["plan-review"],
workflowStepResults: [],
};
const store = createMockStore({
getTask: vi.fn().mockResolvedValue(task),
getTaskWorkflowSelection: vi.fn().mockReturnValue({ workflowId: "builtin:stepwise-coding", stepIds: ["plan-review"] }),
getWorkflowDefinition: vi.fn().mockResolvedValue({
id: "builtin:stepwise-coding",
ir: {
nodes: [
{
id: "plan-review",
kind: "optional-group",
config: {
template: {
nodes: [
{
id: "plan-review-step",
kind: "prompt",
config: { requireExternalIntegrationEvidence: true },
},
],
},
},
},
],
},
}),
} as Partial<TaskStore>);
const processor = new TriageProcessor(store, rootDir);
const result = await (processor as any).runPlanReviewBeforeExecution(task, prompt, {} as Settings);
expect(result).toBe("blocked");
expect(reviewStep).not.toHaveBeenCalled();
expect(store.updateTask).toHaveBeenCalledWith(taskId, expect.objectContaining({ status: "needs-replan" }));
expect(store.logEntry).toHaveBeenCalledWith(
taskId,
"[pre-merge] Workflow step failed: Plan Review",
expect.stringContaining("External-integration evidence gaps"),
);
} finally {
await rm(rootDir, { recursive: true, force: true });
}
});
});

View File

@@ -148,6 +148,10 @@ import { TokenCapDetector } from "./token-cap-detector.js";
import { isUsageLimitError, checkSessionError, type UsageLimitPauser } from "./usage-limit-detector.js";
import { isNonContinuableSessionError, isTransientError, isSilentTransientError } from "./transient-error-detector.js";
import { withRateLimitRetry } from "./rate-limit-retry.js";
import {
detectExternalIntegrationEvidenceGaps,
formatExternalIntegrationEvidenceDiagnostic,
} from "./spec-validation/external-integration-evidence.js";
import { computeRecoveryDecision, formatDelay, MAX_RECOVERY_RETRIES } from "./recovery-policy.js";
import type { StuckTaskDetector, StuckTaskEvent } from "./stuck-task-detector.js";
import type { PluginRunner } from "./plugin-runner.js";
@@ -7174,6 +7178,9 @@ export class TaskExecutor {
if (cfg.summaryTarget === "task") {
(step as WorkflowStep & { summaryTarget?: "task" }).summaryTarget = "task";
}
if (cfg.requireExternalIntegrationEvidence === true) {
(step as WorkflowStep & { requireExternalIntegrationEvidence?: boolean }).requireExternalIntegrationEvidence = true;
}
// (U8a) Thread the plugin-injected runtime env (FUSION_CE_SKILLS_DIR /
// FUSION_CE_AGENTS_DIR + PATH contribution) into prompt-mode skill/model
@@ -13945,6 +13952,34 @@ ${scopeGuard}
// only (default false = board run); see runGraphCustomNode / KTD-3.
const unattended = stepOptions?.unattended === true;
const isPlanReviewStep = workflowStep.id === "graph:plan-review-step" || workflowStep.name === "Plan Review";
const requireExternalIntegrationEvidence =
(workflowStep as WorkflowStep & { requireExternalIntegrationEvidence?: boolean }).requireExternalIntegrationEvidence === true;
if (isPlanReviewStep && requireExternalIntegrationEvidence) {
/*
* FNXC:PlanValidation 2026-06-30-09:03:
* Coding (per-step review) intentionally keeps external-integration evidence as a Plan Review gate. Enforce it here, not in triage, so only workflows that set `requireExternalIntegrationEvidence` block and failures route through the graph's normal plan-replan loop.
*/
const promptContent = await this.readTaskArtifact(task.id, "PROMPT.md");
const evidenceGaps = detectExternalIntegrationEvidenceGaps({
promptContent: typeof promptContent === "string" ? promptContent : "",
});
if (evidenceGaps.length > 0) {
const diagnostic = formatExternalIntegrationEvidenceDiagnostic(evidenceGaps);
const output = `REVISE: ${diagnostic}`;
await this.store.logEntry(
task.id,
`[pre-merge] Plan Review deterministic external-integration evidence check requested revision: ${diagnostic}`,
);
return {
success: false,
revisionRequested: true,
output,
verdict: "REVISE",
notes: diagnostic,
};
}
}
// Compute the diff scope so the workflow step agent reviews only what THIS
// task changed — not unrelated files it might wander into. Without this,

View File

@@ -7,6 +7,7 @@ import type {
TaskAttachment,
Settings,
WorkflowStepResult,
WorkflowIr,
} from "@fusion/core";
import {
DUPLICATE_OF_METADATA_KEY,
@@ -36,6 +37,8 @@ import {
type NearDuplicateCandidate,
} from "@fusion/core";
const PLAN_REVIEW_TEMPLATE_STEP_NODE_ID = "plan-review-step";
type TaskListClamp = (lines: string[], opts?: { maxChars?: number }) => string;
type TaskListFormatter = (
lines: string[],
@@ -1837,6 +1840,9 @@ export class TriageProcessor {
/*
FNXC:PlanReview 2026-06-29-01:52:
Triage owns only deterministic PROMPT.md hygiene. AI plan quality review is graph-owned by the optional Plan Review step, so this helper must never call reviewer agents or require a fn_review_spec APPROVE verdict.
FNXC:PlanValidation 2026-06-30-08:42:
External-integration evidence is a planning/review expectation, not a deterministic triage blocker. Operators saw valid generated plans fail before Plan Review with "Generated plan failed deterministic validation"; keep this local validator limited to structural task-file references the engine can prove.
*/
if (!promptContent.trim()) {
return "PROMPT.md file not found or empty";
@@ -1853,16 +1859,6 @@ export class TriageProcessor {
return diagnostic;
}
const evidenceGaps = detectExternalIntegrationEvidenceGaps({
promptContent,
});
if (evidenceGaps.length > 0) {
const diagnostic = formatExternalIntegrationEvidenceDiagnostic(evidenceGaps);
planLog.warn(`${taskId}: ${diagnostic}`);
await this.store.logEntry(taskId, "Generated plan validation failed: external-integration evidence gaps");
return diagnostic;
}
return null;
}
@@ -1874,6 +1870,30 @@ export class TriageProcessor {
return Array.isArray(task.enabledWorkflowSteps) && task.enabledWorkflowSteps.includes(PLAN_REVIEW_GROUP_ID);
}
private async shouldRequireExternalIntegrationEvidenceForPlanReview(task: Task): Promise<boolean> {
/*
* FNXC:PlanValidation 2026-06-30-09:20:
* Triage may run Plan Review before the graph reaches `plan-review`; the graph later skips an already-passed Plan Review result. Read the selected workflow's Plan Review template flag here so Coding (per-step review) enforces external-integration evidence in the same Plan Review gate, while default Coding and other workflows stay unblocked.
*/
const selection = typeof this.store.getTaskWorkflowSelection === "function"
? this.store.getTaskWorkflowSelection(task.id)
: undefined;
const workflowId = selection?.workflowId;
if (!workflowId || typeof this.store.getWorkflowDefinition !== "function") return false;
const definition = await this.store.getWorkflowDefinition(workflowId).catch((error: unknown) => {
const message = error instanceof Error ? error.message : String(error);
planLog.warn(`${task.id}: failed to resolve workflow '${workflowId}' for Plan Review evidence policy: ${message}`);
return undefined;
});
const ir = definition?.ir as WorkflowIr | undefined;
const planReview = ir?.nodes.find((node) => node.id === PLAN_REVIEW_GROUP_ID);
const template = planReview?.config?.template as
| { nodes?: Array<{ id: string; config?: Record<string, unknown> }> }
| undefined;
const planReviewStep = template?.nodes?.find((node) => node.id === PLAN_REVIEW_TEMPLATE_STEP_NODE_ID);
return planReviewStep?.config?.requireExternalIntegrationEvidence === true;
}
private async recordPlanReviewWorkflowResult(task: Task, result: WorkflowStepResult): Promise<void> {
const live = await this.store.getTask(task.id).catch((error: unknown) => {
const message = error instanceof Error ? error.message : String(error);
@@ -1911,6 +1931,38 @@ export class TriageProcessor {
});
await this.store.logEntry(task.id, "[pre-merge] Starting workflow step: Plan Review");
if (await this.shouldRequireExternalIntegrationEvidenceForPlanReview(task)) {
const evidenceGaps = detectExternalIntegrationEvidenceGaps({ promptContent });
if (evidenceGaps.length > 0) {
const completedAt = new Date().toISOString();
const diagnostic = formatExternalIntegrationEvidenceDiagnostic(evidenceGaps);
await this.recordPlanReviewWorkflowResult(task, {
workflowStepId: PLAN_REVIEW_GROUP_ID,
workflowStepName: "Plan Review",
phase: "pre-merge",
status: "failed",
verdict: "REVISE",
output: diagnostic,
notes: diagnostic,
startedAt,
completedAt,
});
await this.store.logEntry(task.id, "[pre-merge] Workflow step failed: Plan Review", diagnostic);
await this.store.logEntry(
task.id,
"AI spec revision requested",
`Plan Review deterministic external-integration evidence check requested a planning revision before execution.\n\nFeedback:\n${diagnostic}`,
);
await this.store.updateTask(task.id, {
status: "needs-replan",
error: null,
recoveryRetryCount: null,
nextRecoveryAt: null,
});
return "blocked";
}
}
const review = await reviewStep(
this.rootDir,
task.id,