diff --git a/.changeset/fn-7235-7236-workflow-remediation-proof.md b/.changeset/fn-7235-7236-workflow-remediation-proof.md new file mode 100644 index 0000000000..74dacc22d5 --- /dev/null +++ b/.changeset/fn-7235-7236-workflow-remediation-proof.md @@ -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. diff --git a/packages/core/src/__tests__/builtin-code-review-group.test.ts b/packages/core/src/__tests__/builtin-code-review-group.test.ts index ee0d00bb83..5f1fbf7cad 100644 --- a/packages/core/src/__tests__/builtin-code-review-group.test.ts +++ b/packages/core/src/__tests__/builtin-code-review-group.test.ts @@ -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 }[] }; 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", () => { diff --git a/packages/core/src/__tests__/builtin-workflows.test.ts b/packages/core/src/__tests__/builtin-workflows.test.ts index 51ff8c235f..dc8d4f2548 100644 --- a/packages/core/src/__tests__/builtin-workflows.test.ts +++ b/packages/core/src/__tests__/builtin-workflows.test.ts @@ -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, }); } } diff --git a/packages/core/src/builtin-browser-verification-group.ts b/packages/core/src/builtin-browser-verification-group.ts index f684f72ca6..6b9cf459ed 100644 --- a/packages/core/src/builtin-browser-verification-group.ts +++ b/packages/core/src/builtin-browser-verification-group.ts @@ -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: [ { diff --git a/packages/core/src/builtin-code-review-group.ts b/packages/core/src/builtin-code-review-group.ts index 403151d17b..d5f756cf6f 100644 --- a/packages/core/src/builtin-code-review-group.ts +++ b/packages/core/src/builtin-code-review-group.ts @@ -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: [ { diff --git a/packages/core/src/builtin-plan-review-group.ts b/packages/core/src/builtin-plan-review-group.ts index 7dc9da2004..8f008f96f2 100644 --- a/packages/core/src/builtin-plan-review-group.ts +++ b/packages/core/src/builtin-plan-review-group.ts @@ -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: [ { diff --git a/packages/core/src/builtin-workflows.ts b/packages/core/src/builtin-workflows.ts index 4bcc08137f..a32a1782eb 100644 --- a/packages/core/src/builtin-workflows.ts +++ b/packages/core/src/builtin-workflows.ts @@ -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: [ { diff --git a/packages/engine/src/__tests__/merger-merge-lifecycle.test.ts b/packages/engine/src/__tests__/merger-merge-lifecycle.test.ts index 256e9df561..7b7a9da463 100644 --- a/packages/engine/src/__tests__/merger-merge-lifecycle.test.ts +++ b/packages/engine/src/__tests__/merger-merge-lifecycle.test.ts @@ -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; + updateTask: ReturnType; + moveTask: ReturnType; + recordRunAuditEvent: ReturnType; + }; + 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", diff --git a/packages/engine/src/__tests__/workflow-graph-optional-step-fix.test.ts b/packages/engine/src/__tests__/workflow-graph-optional-step-fix.test.ts index 1140b35d7f..b3737e8490 100644 --- a/packages/engine/src/__tests__/workflow-graph-optional-step-fix.test.ts +++ b/packages/engine/src/__tests__/workflow-graph-optional-step-fix.test.ts @@ -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 }); diff --git a/packages/engine/src/auto-merge-finalization.ts b/packages/engine/src/auto-merge-finalization.ts index 224f727ee7..df33f432db 100644 --- a/packages/engine/src/auto-merge-finalization.ts +++ b/packages/engine/src/auto-merge-finalization.ts @@ -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(/^\//, "") + .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 { 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, + }, }; } } diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index 1853da00c9..587e8adcbd 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -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,