fix(FN-7235): scope workflow remediation recovery
Keep merge finalization from stranding scoped workflow tasks on out-of-scope branch residue, move built-in optional gate remediation attempts into workflow config, and guard review remediation against out-of-scope fixes. Also covers FN-7236 review-remediation scope drift.
This commit is contained in:
7
.changeset/fn-7235-7236-workflow-remediation-proof.md
Normal file
7
.changeset/fn-7235-7236-workflow-remediation-proof.md
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
summary: Prevent scoped workflow tasks from getting stranded by unrelated branch residue.
|
||||
category: fix
|
||||
dev: Built-in optional workflow gates now default to three remediation attempts and review fixes carry File Scope guardrails.
|
||||
@@ -52,6 +52,7 @@ describe("codeReviewOptionalGroupNode", () => {
|
||||
expect(node.config?.name).toBe("Code Review");
|
||||
// Default-ON (runs by default), but still an optional-group → toggleable per task.
|
||||
expect(node.config?.defaultOn).toBe(true);
|
||||
expect(node.config?.maxRevisions).toBe(3);
|
||||
|
||||
const template = node.config?.template as { nodes: { id: string; kind: string; config?: Record<string, unknown> }[] };
|
||||
expect(template.nodes).toHaveLength(1);
|
||||
@@ -62,6 +63,11 @@ describe("codeReviewOptionalGroupNode", () => {
|
||||
expect(inner.config?.gateMode).toBe("gate");
|
||||
expect(String(inner.config?.prompt)).toMatch(/"verdict":"APPROVE\|APPROVE_WITH_NOTES\|REVISE"/);
|
||||
});
|
||||
|
||||
it("lets workflows override the default remediation attempt budget", () => {
|
||||
expect(codeReviewOptionalGroupNode("in-progress", { maxRevisions: 1 }).config?.maxRevisions).toBe(1);
|
||||
expect(codeReviewOptionalGroupNode("in-progress", { maxRevisions: "unbounded" }).config?.maxRevisions).toBe("unbounded");
|
||||
});
|
||||
});
|
||||
|
||||
describe("built-in coding + stepwise workflows wire code-review as a default-ON optional group", () => {
|
||||
|
||||
@@ -139,6 +139,7 @@ describe("built-in workflows", () => {
|
||||
expect(workflow.ir.nodes.find((node) => node.id === gate)?.config, `${workflow.id}:${gate}:reworkRegion`).toMatchObject({
|
||||
reworkRegion: true,
|
||||
maxReworkCycles: 3,
|
||||
maxRevisions: 3,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
@@ -86,7 +86,7 @@ Note: Refs (@e1, @e2) are invalidated after page navigation. Re-snapshot after c
|
||||
*/
|
||||
export function browserVerificationOptionalGroupNode(
|
||||
column: string,
|
||||
options: { defaultOn?: boolean } = {},
|
||||
options: { defaultOn?: boolean; maxRevisions?: number | "unbounded" } = {},
|
||||
): WorkflowIrNode {
|
||||
return {
|
||||
id: BROWSER_VERIFICATION_GROUP_ID,
|
||||
@@ -97,6 +97,11 @@ export function browserVerificationOptionalGroupNode(
|
||||
defaultOn: options.defaultOn ?? false,
|
||||
reworkRegion: true,
|
||||
maxReworkCycles: 3,
|
||||
/*
|
||||
* FNXC:WorkflowRemediationBudget 2026-06-29-13:56:
|
||||
* Built-in browser verification owns its remediation attempt policy. Default to three workflow-scoped attempts, with custom workflow `maxRevisions` values able to override this node config.
|
||||
*/
|
||||
maxRevisions: options.maxRevisions ?? 3,
|
||||
template: {
|
||||
nodes: [
|
||||
{
|
||||
|
||||
@@ -80,7 +80,7 @@ Be specific: cite \`file:line\` for every finding and explain the concrete failu
|
||||
*/
|
||||
export function codeReviewOptionalGroupNode(
|
||||
column: string,
|
||||
options: { defaultOn?: boolean } = {},
|
||||
options: { defaultOn?: boolean; maxRevisions?: number | "unbounded" } = {},
|
||||
): WorkflowIrNode {
|
||||
return {
|
||||
id: CODE_REVIEW_GROUP_ID,
|
||||
@@ -93,6 +93,11 @@ export function codeReviewOptionalGroupNode(
|
||||
defaultOn: options.defaultOn ?? true,
|
||||
reworkRegion: true,
|
||||
maxReworkCycles: 3,
|
||||
/*
|
||||
* FNXC:WorkflowRemediationBudget 2026-06-29-13:56:
|
||||
* Built-in workflows own their optional-step remediation policy. Default Code Review to three fix→review attempts while preserving workflow-authored overrides through `config.maxRevisions`.
|
||||
*/
|
||||
maxRevisions: options.maxRevisions ?? 3,
|
||||
template: {
|
||||
nodes: [
|
||||
{
|
||||
|
||||
@@ -41,7 +41,7 @@ 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 } = {},
|
||||
options: { defaultOn?: boolean; maxRevisions?: number | "unbounded" } = {},
|
||||
): WorkflowIrNode {
|
||||
return {
|
||||
id: PLAN_REVIEW_GROUP_ID,
|
||||
@@ -56,6 +56,11 @@ export function planReviewOptionalGroupNode(
|
||||
*/
|
||||
reworkRegion: true,
|
||||
maxReworkCycles: 3,
|
||||
/*
|
||||
* FNXC:WorkflowRemediationBudget 2026-06-29-13:56:
|
||||
* Built-in Plan Review owns the pre-execution replan budget. Default to three graph-mediated revise→replan→review attempts while allowing workflow authors to override `maxRevisions`.
|
||||
*/
|
||||
maxRevisions: options.maxRevisions ?? 3,
|
||||
template: {
|
||||
nodes: [
|
||||
{
|
||||
|
||||
@@ -59,6 +59,11 @@ function ceCodeReviewOptionalGroupNode(column: string): WorkflowIrNode {
|
||||
defaultOn: true,
|
||||
reworkRegion: true,
|
||||
maxReworkCycles: 3,
|
||||
/*
|
||||
* FNXC:WorkflowRemediationBudget 2026-06-29-13:56:
|
||||
* The CE Code Review group is custom because it invokes the CE skill, but its workflow-owned remediation budget must match the other built-in optional gates by defaulting to three attempts while remaining editable in workflow config.
|
||||
*/
|
||||
maxRevisions: 3,
|
||||
template: {
|
||||
nodes: [
|
||||
{
|
||||
|
||||
@@ -440,6 +440,78 @@ describe("auto-merge proven finalization helper", () => {
|
||||
expect(store.moveTask).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("allows workflow finalization when missing branch proof is outside the declared File Scope", async () => {
|
||||
const strandedTask = {
|
||||
id: "FN-SCOPED-PROOF",
|
||||
title: "Scoped proof",
|
||||
description: "Test",
|
||||
column: "in-progress",
|
||||
branch: "fusion/fn-scoped-proof",
|
||||
baseBranch: "main",
|
||||
dependencies: [],
|
||||
steps: [{ status: "done" }],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
sourceMetadata: {
|
||||
fileScope: [
|
||||
"packages/dashboard/app/components/EngineControlMenu.tsx",
|
||||
"packages/dashboard/app/components/__tests__/EngineControlMenu.test.tsx",
|
||||
"docs/dashboard-guide.md",
|
||||
".changeset/*.md",
|
||||
],
|
||||
},
|
||||
mergeDetails: {
|
||||
mergeConfirmed: true,
|
||||
commitSha: "abc123",
|
||||
landedFiles: [
|
||||
"packages/dashboard/app/components/EngineControlMenu.tsx",
|
||||
"packages/dashboard/app/components/__tests__/EngineControlMenu.test.tsx",
|
||||
"docs/dashboard-guide.md",
|
||||
".changeset/fn-7235-footer-concurrency-marker.md",
|
||||
],
|
||||
},
|
||||
} as Task;
|
||||
const store = createMockStore(strandedTask) as unknown as TaskStore & {
|
||||
getTask: ReturnType<typeof vi.fn>;
|
||||
updateTask: ReturnType<typeof vi.fn>;
|
||||
moveTask: ReturnType<typeof vi.fn>;
|
||||
recordRunAuditEvent: ReturnType<typeof vi.fn>;
|
||||
};
|
||||
store.getTask.mockResolvedValue(strandedTask);
|
||||
mockedExecSync.mockImplementation((cmd: any) => {
|
||||
const command = String(cmd);
|
||||
if (command.includes("rev-parse --verify")) return "ok\n" as any;
|
||||
if (command.includes("git diff --name-only") && command.includes("main...fusion/fn-scoped-proof")) {
|
||||
return [
|
||||
"packages/dashboard/app/components/EngineControlMenu.tsx",
|
||||
"packages/dashboard/app/components/__tests__/EngineControlMenu.test.tsx",
|
||||
"docs/dashboard-guide.md",
|
||||
".changeset/fn-7235-footer-concurrency-marker.md",
|
||||
"packages/engine/src/triage.ts",
|
||||
].join("\n") as any;
|
||||
}
|
||||
return "" as any;
|
||||
});
|
||||
|
||||
const result = await finalizeProvenAutoMergeTask({
|
||||
store,
|
||||
taskId: "FN-SCOPED-PROOF",
|
||||
result: { task: strandedTask, ok: true, merged: true, commitSha: "abc123", mergeConfirmed: true } as MergeResult,
|
||||
source: "workflow-graph-merge-finalize",
|
||||
rootDir: "/repo",
|
||||
});
|
||||
|
||||
expect(result).toEqual(expect.objectContaining({ outcome: "done" }));
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-SCOPED-PROOF", "done", expect.objectContaining({
|
||||
moveSource: "engine",
|
||||
preserveProgress: true,
|
||||
recoveryRehome: true,
|
||||
}));
|
||||
expect(store.updateTask).not.toHaveBeenCalledWith("FN-SCOPED-PROOF", expect.objectContaining({ status: "failed" }));
|
||||
});
|
||||
|
||||
it("treats already-done landed rows as idempotent success", async () => {
|
||||
const doneTask = {
|
||||
id: "FN-DONE",
|
||||
|
||||
@@ -164,7 +164,7 @@ describe("TaskExecutor pre-merge optional-step fix seam", () => {
|
||||
expect((executor as any).pausedAborted.has("FN-7066")).toBe(false);
|
||||
});
|
||||
|
||||
it("clears stale pause-abort provenance before a fresh unpaused execution dispatch", async () => {
|
||||
it("clears stale pause-abort provenance silently before a fresh unpaused execution dispatch", async () => {
|
||||
const store = createMockStore();
|
||||
const liveTask = task({ column: "todo", paused: false, userPaused: false });
|
||||
store.getSettings.mockResolvedValue({ globalPause: false });
|
||||
@@ -174,12 +174,7 @@ describe("TaskExecutor pre-merge optional-step fix seam", () => {
|
||||
await (executor as any).clearStalePauseAbortBeforeDispatch(liveTask);
|
||||
|
||||
expect((executor as any).pausedAborted.has("FN-7066")).toBe(false);
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
"FN-7066",
|
||||
"Cleared stale pause-abort marker before unpaused execution dispatch",
|
||||
undefined,
|
||||
undefined,
|
||||
);
|
||||
expect(store.logEntry).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("clears pause-abort provenance for manual retry", () => {
|
||||
@@ -275,6 +270,28 @@ describe("TaskExecutor pre-merge optional-step fix seam", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("adds declared File Scope boundaries to optional-step remediation instructions", () => {
|
||||
const store = createMockStore();
|
||||
const executor = new TaskExecutor(store, "/tmp/test");
|
||||
const guard = (executor as any).buildWorkflowFailureScopeGuard(
|
||||
task({ sourceMetadata: { fileScope: ["packages/dashboard/app/components/WorkflowTabs.tsx"] } }),
|
||||
[
|
||||
"# Task",
|
||||
"",
|
||||
"## File Scope",
|
||||
"- `packages/dashboard/app/components/WorkflowTabs.css`",
|
||||
"",
|
||||
"## Steps",
|
||||
"- Implement",
|
||||
].join("\n"),
|
||||
);
|
||||
|
||||
expect(guard).toContain("Treat the declared File Scope as the remediation boundary");
|
||||
expect(guard).toContain("packages/dashboard/app/components/WorkflowTabs.css");
|
||||
expect(guard).toContain("packages/dashboard/app/components/WorkflowTabs.tsx");
|
||||
expect(guard).toContain("split them into a separate task");
|
||||
});
|
||||
|
||||
it("honors unbounded and zero per-step maxRevisions states", async () => {
|
||||
const unboundedStore = createMockStore();
|
||||
const exhaustedTask = task({ postReviewFixCount: 99 });
|
||||
|
||||
@@ -46,6 +46,94 @@ function hasIncompleteWorkflowSteps(task: Task): boolean {
|
||||
return (task.steps ?? []).some((step) => step.status !== "done" && step.status !== "skipped");
|
||||
}
|
||||
|
||||
function cleanScopeEntry(entry: string): string {
|
||||
let cleaned = entry.trim().replace(/^[-*]\s+/, "");
|
||||
const codeSpan = cleaned.match(/`([^`]+)`/);
|
||||
if (codeSpan) cleaned = codeSpan[1];
|
||||
return cleaned
|
||||
.replace(/^<rootDir>\//, "")
|
||||
.replace(/\s+\((new|modified|existing)\)\s*$/i, "")
|
||||
.trim();
|
||||
}
|
||||
|
||||
function extractMarkdownSection(prompt: string, heading: string): string {
|
||||
const escaped = heading.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
|
||||
const headingPattern = new RegExp(`^##\\s+${escaped}\\s*:?\\s*$`, "i");
|
||||
const lines = prompt.split(/\r?\n/);
|
||||
const start = lines.findIndex((line) => headingPattern.test(line.trim()));
|
||||
if (start === -1) return "";
|
||||
const sectionLines: string[] = [];
|
||||
for (let i = start + 1; i < lines.length; i++) {
|
||||
if (/^##\s+/.test(lines[i].trim())) break;
|
||||
sectionLines.push(lines[i]);
|
||||
}
|
||||
return sectionLines.join("\n");
|
||||
}
|
||||
|
||||
function extractScopeEntriesFromPrompt(prompt: string | undefined): string[] {
|
||||
if (!prompt) return [];
|
||||
return extractMarkdownSection(prompt, "File Scope")
|
||||
.split(/\r?\n/)
|
||||
.map(cleanScopeEntry)
|
||||
.filter(Boolean);
|
||||
}
|
||||
|
||||
function getTaskFileScope(task: Task): string[] {
|
||||
const metadataScope = Array.isArray(task.sourceMetadata?.fileScope)
|
||||
? task.sourceMetadata.fileScope.filter((entry): entry is string => typeof entry === "string")
|
||||
: [];
|
||||
return Array.from(new Set([...metadataScope, ...extractScopeEntriesFromPrompt(task.prompt)].map(cleanScopeEntry).filter(Boolean)));
|
||||
}
|
||||
|
||||
function globToRegex(pattern: string): RegExp {
|
||||
let source = "";
|
||||
for (let i = 0; i < pattern.length; i++) {
|
||||
const char = pattern[i];
|
||||
if (char === "*") {
|
||||
if (pattern[i + 1] === "*") {
|
||||
source += ".*";
|
||||
i++;
|
||||
} else {
|
||||
source += "[^/]*";
|
||||
}
|
||||
continue;
|
||||
}
|
||||
source += char.replace(/[.+^${}()|[\]\\]/g, "\\$&");
|
||||
}
|
||||
return new RegExp(`^${source}$`);
|
||||
}
|
||||
|
||||
function matchesFileScope(filePath: string, scopeEntry: string): boolean {
|
||||
const file = filePath.replace(/^\.\/+/, "");
|
||||
const scope = scopeEntry.replace(/^\.\/+/, "");
|
||||
if (!scope || /\b(no source|no code|task document|read-only)\b/i.test(scope)) return false;
|
||||
if (file === scope) return true;
|
||||
if (scope.endsWith("/")) return file.startsWith(scope);
|
||||
if (scope.endsWith("/**")) return file.startsWith(scope.slice(0, -2));
|
||||
if (scope.includes("*")) return globToRegex(scope).test(file);
|
||||
return file.startsWith(`${scope}/`);
|
||||
}
|
||||
|
||||
function branchDiffFilesMissingFromMergeProof(task: Task, branchFiles: string[], landedFiles: string[]): {
|
||||
blockingMissing: string[];
|
||||
ignoredOutOfScopeMissing: string[];
|
||||
} {
|
||||
const landed = new Set(landedFiles);
|
||||
const missing = branchFiles.filter((file) => !landed.has(file));
|
||||
const scope = getTaskFileScope(task);
|
||||
if (scope.length === 0) return { blockingMissing: missing, ignoredOutOfScopeMissing: [] };
|
||||
|
||||
/*
|
||||
* FNXC:WorkflowMergeFinalization 2026-06-29-13:56:
|
||||
* Scoped squash merges may intentionally land only the task's declared File Scope while a stale task branch still carries unrelated residue from a previous remediation or contaminated branch. Finalization must still block any in-scope branch diff missing from durable merge proof, but out-of-scope residue should not strand an already-landed workflow task in review forever.
|
||||
*/
|
||||
const blockingMissing = missing.filter((file) => scope.some((entry) => matchesFileScope(file, entry)));
|
||||
return {
|
||||
blockingMissing,
|
||||
ignoredOutOfScopeMissing: missing.filter((file) => !blockingMissing.includes(file)),
|
||||
};
|
||||
}
|
||||
|
||||
async function readBranchDiffFiles(rootDir: string, task: Task): Promise<string[] | null> {
|
||||
const branch = task.branch;
|
||||
if (!branch) return null;
|
||||
@@ -85,13 +173,18 @@ export async function validateWorkflowDoneMergeProof(
|
||||
if (noOp) {
|
||||
return { ok: false, reason: "noop-merge-branch-still-has-diff", metadata: { branchFiles: branchFiles.length } };
|
||||
}
|
||||
const landed = new Set(landedFiles);
|
||||
const missing = branchFiles.filter((file) => !landed.has(file));
|
||||
if (missing.length > 0) {
|
||||
const { blockingMissing, ignoredOutOfScopeMissing } = branchDiffFilesMissingFromMergeProof(task, branchFiles, landedFiles);
|
||||
if (blockingMissing.length > 0) {
|
||||
return {
|
||||
ok: false,
|
||||
reason: "branch-diff-missing-from-merge-proof",
|
||||
metadata: { missingFiles: missing.slice(0, 10), missingCount: missing.length, branchFiles: branchFiles.length },
|
||||
metadata: {
|
||||
missingFiles: blockingMissing.slice(0, 10),
|
||||
missingCount: blockingMissing.length,
|
||||
ignoredOutOfScopeMissingFiles: ignoredOutOfScopeMissing.slice(0, 10),
|
||||
ignoredOutOfScopeMissingCount: ignoredOutOfScopeMissing.length,
|
||||
branchFiles: branchFiles.length,
|
||||
},
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
@@ -13206,6 +13206,7 @@ ${failureContext.output.slice(0, VERIFICATION_LOG_MAX_CHARS)}
|
||||
|
||||
const remainingRetries = MAX_WORKFLOW_STEP_RETRIES - retryCount;
|
||||
const failureSectionHeader = "## Workflow Step Failure";
|
||||
const scopeGuard = this.buildWorkflowFailureScopeGuard(task, content);
|
||||
const failureSectionContent = `${failureSectionHeader}
|
||||
|
||||
The following workflow step failed and requires implementation fixes:
|
||||
@@ -13215,6 +13216,8 @@ The following workflow step failed and requires implementation fixes:
|
||||
**Failure Feedback:**
|
||||
${failureFeedback}
|
||||
|
||||
${scopeGuard}
|
||||
|
||||
**Retry:** ${retryCount}/${MAX_WORKFLOW_STEP_RETRIES} (${remainingRetries} remaining)
|
||||
|
||||
**Important:** This is a workflow step failure — fix the issues above by making the necessary code changes. The task has been sent back to in-progress for remediation. The executor will attempt to fix the issues on the next pass.
|
||||
@@ -13265,6 +13268,27 @@ ${failureFeedback}
|
||||
}
|
||||
}
|
||||
|
||||
private buildWorkflowFailureScopeGuard(task: Task, promptContent: string): string {
|
||||
const promptScopeEntries = extractPromptListEntries(extractPromptSection(promptContent, "File Scope"));
|
||||
const metadataScope = Array.isArray(task.sourceMetadata?.fileScope)
|
||||
? task.sourceMetadata.fileScope.filter((entry): entry is string => typeof entry === "string")
|
||||
: [];
|
||||
const declaredScope = Array.from(new Set([...promptScopeEntries, ...metadataScope].map((entry) => entry.trim()).filter(Boolean)));
|
||||
/*
|
||||
* FNXC:WorkflowRemediationScope 2026-06-29-13:56:
|
||||
* Review remediation must not let one task silently implement unrelated behavior. If reviewer feedback points outside the declared File Scope, the executor should remove/split the unrelated work instead of expanding the task, while still allowing already-scoped fixes to proceed automatically.
|
||||
*/
|
||||
if (declaredScope.length === 0) {
|
||||
return "**Scope Guard:** Keep remediation limited to this task's stated mission and existing implementation surface. If the feedback requires unrelated behavior, remove or split that work instead of implementing it here.";
|
||||
}
|
||||
return [
|
||||
"**Scope Guard:** Treat the declared File Scope as the remediation boundary. Fix only the scoped files unless PROMPT.md already authorizes a scope expansion. If the feedback requires unrelated behavior outside this scope, remove those unrelated changes or split them into a separate task instead of implementing them here.",
|
||||
"",
|
||||
"**Declared File Scope:**",
|
||||
...declaredScope.map((entry) => `- ${entry}`),
|
||||
].join("\n");
|
||||
}
|
||||
|
||||
private async captureBaseCommitSha(
|
||||
task: Task,
|
||||
worktreePath: string,
|
||||
|
||||
Reference in New Issue
Block a user