diff --git a/.changeset/fn-7381-auto-plan-approval.md b/.changeset/fn-7381-auto-plan-approval.md new file mode 100644 index 0000000000..4e4b2d3913 --- /dev/null +++ b/.changeset/fn-7381-auto-plan-approval.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Honor project auto plan approval across task finalization paths. +category: fix +dev: Ensures planApprovalMode=auto-approve-all wins over workflow requirePlanApproval for ordinary plan approval. diff --git a/docs/settings-reference.md b/docs/settings-reference.md index 6a79794de1..621d04967b 100644 --- a/docs/settings-reference.md +++ b/docs/settings-reference.md @@ -397,7 +397,7 @@ Security-sensitive file-browser escape hatches are project-only. `allowAbsoluteF | `overlapIgnorePaths` | `string[]` | `[]` | Optional project-relative file or directory paths to exclude from overlap blocking (for example `docs` or `generated/openapi.json`). Entries are trimmed, deduplicated, and must not be absolute or contain `..` traversal. | | `allowAbsoluteFileBrowserPaths` | `boolean` | `false` | Project-scoped Settings → General toggle for the workspace file browser. When enabled, slash-prefixed paths such as `/tmp` can be listed/read/written/downloaded through workspace file-browser routes while keeping existing file-size, binary, type, null-byte, traversal, and permission checks. Windows drive-letter paths remain blocked, and task-local file routes, memory APIs, worktree-copy validation, plugin bundle paths, and other validators are unchanged. | | `autoMerge` | `boolean` | `true` | Auto-finalize tasks from `in-review`. Tasks can override this per-task (including at create time in New Task modal via **Auto-merge** = Default/Enabled/Disabled); explicit overrides are tagged with `autoMergeProvenance: "user"`, while tasks left at **Default** keep following the live global setting and do not snapshot it when entering review. Legacy pre-FN-6245 in-review rows that were stamped `autoMerge: true` are marked `autoMergeProvenance: "legacy-stamp"` on startup and can be inspected/cleared with Settings → Merge → **Legacy auto-merge stamp cleanup**, `fn pr automerge-cleanup [--apply] [--json]`, or `reconcileLegacyAutoMergeStamps({ apply: true })` after operator review. For grouped branch flows, per-task `autoMerge` governs member→group-integration landing while group `autoMerge` governs group→default-branch promotion eligibility. | -| `planApprovalMode` | `"workflow" \| "auto-approve-all" \| "require-all"` | `"workflow"` | Project-scoped override for the planning approval gate. `"workflow"` preserves the workflow-resolved `requirePlanApproval`; `"auto-approve-all"` moves every specified task to todo without manual approval; `"require-all"` parks every specified task at `status: "awaiting-approval"` regardless of workflow settings. | +| `planApprovalMode` | `"workflow" \| "auto-approve-all" \| "require-all"` | `"workflow"` | Project-scoped override for the manual planning approval gate. `"workflow"` preserves the workflow-resolved `requirePlanApproval`; `"auto-approve-all"` moves every successfully specified task to `todo` without manual plan approval even when the selected workflow or stored workflow setting has `requirePlanApproval: true`; `"require-all"` parks every specified task at `status: "awaiting-approval"` regardless of workflow settings. This does not disable Workflow Plan Review, release authorization, or other non-plan safety gates. | | `maxAutoMergeRetries` | `number` | `3` | Project-scoped positive-integer cap for auto-merge conflict-resolution retries before Fusion parks or bounces a task for human/recovery handling. Unset, non-finite, zero, or negative values fall back to `3` to preserve historical behavior. | | `mergeRequestContractShadowEnabled` | `boolean` | `false` | Phase-1 FN-5741 write-only shadow flag (project/global setting). When enabled, executor/self-healing/merger persist merge-request records and `completion_handoff_accepted` markers for observation only; legacy mergeQueue + lifecycle remains authoritative. | | `mergeStrategy` | `"direct" \| "pull-request"` | `"direct"` | Completion mode (local direct merge vs PR-first). | diff --git a/docs/workflow-steps.md b/docs/workflow-steps.md index 290d9aa129..7b124b4732 100644 --- a/docs/workflow-steps.md +++ b/docs/workflow-steps.md @@ -210,6 +210,8 @@ The default built-in catalog entry `builtin:coding` is backed by a Stepwise-deri If the Plan Review reviewer is unavailable before producing a verdict, the task stays in triage as `status: "plan-review-unavailable"` with a short backoff. That retry state is not a replan: Fusion rereads the existing non-empty `PROMPT.md`, preserves it unchanged, and reruns only Plan Review/finalization while holding a global agent concurrency slot for the reviewer lane. A reviewer revision verdict moves the task to `needs-replan`; missing/empty/invalid prompt content fails clearly instead of restarting the planner. +Workflow Plan Review is separate from manual plan approval. Project `planApprovalMode: "auto-approve-all"` bypasses only the final manual `awaiting-approval` plan gate after the plan is specified and any enabled Plan Review passes; it does not disable Plan Review, release authorization, or other explicit safety gates. + `builtin:legacy-coding` is backed by the original monolithic `BUILTIN_CODING_WORKFLOW_IR`: `planning` → `execute` → optional quality gates → `review` → merge region. `builtin:stepwise-coding` displays as Coding (per-step review). It is backed by `BUILTIN_STEPWISE_CODING_WORKFLOW_IR`; it keeps the same lifecycle columns/traits while adding the default-on optional Plan Review before `parse-steps`, modeling per-step parse/execute/review/rework as authored graph structure, and retaining the post-foreach optional Code Review gate before its final review/merge region. diff --git a/packages/engine/src/__tests__/effective-settings-merge.test.ts b/packages/engine/src/__tests__/effective-settings-merge.test.ts index f63391da1a..9b8e11428f 100644 --- a/packages/engine/src/__tests__/effective-settings-merge.test.ts +++ b/packages/engine/src/__tests__/effective-settings-merge.test.ts @@ -10,6 +10,7 @@ function baseSettings(): Settings { return { workflowStepTimeoutMs: 900_000, requirePrApproval: false, + requirePlanApproval: false, runStepsInNewSessions: false, // A real project value for an absent-default lane — must NOT be clobbered. executionProvider: "project-anthropic", @@ -83,6 +84,17 @@ describe("mergeEffectiveSettings (engine entry merge, U3/KTD-3)", () => { expect(merged.executionModelId).toBe("claude-project"); }); + it("preserves project planApprovalMode while applying stored workflow requirePlanApproval", async () => { + const store = makeStore({ + workflowId: "builtin:coding", + values: { requirePlanApproval: true }, + }); + const base = { ...baseSettings(), planApprovalMode: "auto-approve-all", requirePlanApproval: false } as unknown as Settings; + const merged = await mergeEffectiveSettings(store as any, { id: "t1" }, base); + expect(merged.planApprovalMode).toBe("auto-approve-all"); + expect(merged.requirePlanApproval).toBe(true); + }); + it("returns a NEW object; the base is not mutated", async () => { const store = makeStore({ workflowId: "builtin:coding", values: { workflowStepTimeoutMs: 1 } }); const base = baseSettings(); diff --git a/packages/engine/src/__tests__/triage-plan-review-unavailable-retry.test.ts b/packages/engine/src/__tests__/triage-plan-review-unavailable-retry.test.ts index 866d410a76..3d911d8e89 100644 --- a/packages/engine/src/__tests__/triage-plan-review-unavailable-retry.test.ts +++ b/packages/engine/src/__tests__/triage-plan-review-unavailable-retry.test.ts @@ -54,7 +54,7 @@ function createRetryTask(overrides: Partial = {}): Task { } as Task; } -function createStore(task: Task): TaskStore { +function createStore(task: Task, settingsOverrides: Partial = {}): TaskStore { return { getTask: vi.fn().mockResolvedValue(task), listTasks: vi.fn().mockResolvedValue([task]), @@ -65,6 +65,7 @@ function createStore(task: Task): TaskStore { groupOverlappingFiles: false, autoMerge: true, requirePlanApproval: false, + ...settingsOverrides, } as Settings), updateTask: vi.fn().mockResolvedValue(undefined), moveTask: vi.fn().mockResolvedValue(undefined), @@ -79,6 +80,10 @@ function createStore(task: Task): TaskStore { mergeTask: vi.fn(), updateSettings: vi.fn(), addSteeringComment: vi.fn(), + getTaskWorkflowSelection: vi.fn().mockReturnValue({ workflowId: "builtin:coding", stepIds: [] }), + getWorkflowDefinition: vi.fn().mockResolvedValue(undefined), + getWorkflowSettingValues: vi.fn().mockReturnValue({}), + getWorkflowSettingsProjectId: vi.fn().mockReturnValue("project-plan-review-retry"), on: vi.fn(), emit: vi.fn(), } as unknown as TaskStore; @@ -138,6 +143,26 @@ describe("Plan Review unavailable retry", () => { expect(store.moveTask).toHaveBeenCalledWith(task.id, "todo"); }); + it("moves an approved retry to todo when project auto approval overrides workflow approval", async () => { + const rootDir = await createFixtureRoot(); + roots.push(rootDir); + const task = createRetryTask({ id: "FN-PLAN-RETRY-AUTO-APPROVE" }); + const prompt = `# Task: ${task.id} - Existing draft\n\n## Mission\n\nKeep this exact text.\n`; + const promptPath = await writePrompt(rootDir, task.id, prompt); + const store = createStore(task, { planApprovalMode: "auto-approve-all", requirePlanApproval: false }); + (store.getWorkflowSettingValues as ReturnType).mockReturnValue({ requirePlanApproval: true }); + mockReviewStep.mockResolvedValue({ verdict: "APPROVE", review: "Approved.", summary: "Ready." }); + + await retryTask(rootDir, task, store); + + expect(mockCreateFnAgent).not.toHaveBeenCalled(); + expect(readFileSync(promptPath, "utf-8")).toBe(prompt); + expect(store.moveTask).toHaveBeenCalledWith(task.id, "todo"); + expect(store.updateTask).not.toHaveBeenCalledWith(task.id, expect.objectContaining({ status: "awaiting-approval" })); + const logActions = (store.logEntry as ReturnType).mock.calls.map(([, action]) => action); + expect(logActions).not.toContain("Specification approved by AI — awaiting manual approval"); + }); + it.each([ { name: "unavailable verdict", diff --git a/packages/engine/src/__tests__/triage.test.ts b/packages/engine/src/__tests__/triage.test.ts index f38b18823a..0e5cc23ed2 100644 --- a/packages/engine/src/__tests__/triage.test.ts +++ b/packages/engine/src/__tests__/triage.test.ts @@ -2843,6 +2843,48 @@ Forbidden paths / non-goals: expect(store.moveTask).toHaveBeenCalledWith("FN-001", "todo"); }); + it("moves recovered task to todo when project auto approval overrides stored workflow approval", async () => { + const store = createMockStore({ + getSettings: vi.fn().mockResolvedValue({ + maxConcurrent: 2, + maxWorktrees: 4, + pollIntervalMs: 10000, + groupOverlappingFiles: false, + autoMerge: true, + planApprovalMode: "auto-approve-all", + requirePlanApproval: false, + } as Settings), + getTaskWorkflowSelection: vi.fn().mockReturnValue({ workflowId: "builtin:coding", stepIds: [] }), + getWorkflowDefinition: vi.fn().mockResolvedValue(undefined), + getWorkflowSettingValues: vi.fn().mockReturnValue({ requirePlanApproval: true }), + getWorkflowSettingsProjectId: vi.fn().mockReturnValue("project-auto-approval"), + } as Partial); + + const processor = new TriageProcessor(store, rootDir); + const recovered = await processor.recoverApprovedTask({ + id: "FN-001", + description: "Recovered triage task", + column: "triage", + status: "planning", + dependencies: [], + steps: [], + currentStep: 0, + log: [ + { timestamp: "2026-01-01T00:00:00.000Z", action: "Spec review: APPROVE" }, + ], + createdAt: "2026-01-01T00:00:00.000Z", + updatedAt: "2026-01-01T00:02:00.000Z", + }); + + expect(recovered).toBe(true); + expect(store.moveTask).toHaveBeenCalledWith("FN-001", "todo"); + expect(store.updateTask).not.toHaveBeenCalledWith("FN-001", expect.objectContaining({ status: "awaiting-approval" })); + expect(store.logEntry).toHaveBeenCalledWith( + "FN-001", + "Auto-recovered specified task stuck in planning — moved to todo", + ); + }); + it("moves approved planning task to awaiting-approval when manual approval is required", async () => { const store = createMockStore({ getSettings: vi.fn().mockResolvedValue({ diff --git a/packages/engine/src/triage.ts b/packages/engine/src/triage.ts index e2979440d3..4c6d8287c8 100644 --- a/packages/engine/src/triage.ts +++ b/packages/engine/src/triage.ts @@ -548,8 +548,10 @@ export class TriageProcessor { return false; } - // Merge per-task effective workflow settings (U3, KTD-3) so requirePlanApproval - // resolves from the workflow. Behavior-inert when nothing is customized. + /* + FNXC:PlanApproval 2026-07-01-08:12: + Recovery finalizes an already-written PROMPT.md and must use the same merged project/workflow settings as fresh triage. The project planApprovalMode value stays project-scoped while workflow requirePlanApproval may overlay, so auto-approve-all still wins for ordinary plan approval. + */ const settings = await mergeEffectiveSettings(this.store, task, await this.store.getSettings()); const approvalRequired = resolvePlanApprovalRequired(settings); const promptPath = join(this.rootDir, ".fusion", "tasks", task.id, "PROMPT.md"); @@ -2456,7 +2458,7 @@ export class TriageProcessor { await this.store.updateTask(task.id, approvalUpdates); await this.store.logEntry( task.id, - "Release authorization required — leaving task in triage awaiting manual approval", + "Release authorization required — leaving task in triage awaiting release authorization", details, ); try { @@ -2475,7 +2477,7 @@ export class TriageProcessor { const message = activityError instanceof Error ? activityError.message : String(activityError); planLog.warn(`${task.id}: failed to record release-authorization-required activity (${message})`); } - planLog.log(`${task.id} release authorization required — leaving in triage awaiting manual approval (${signals})`); + planLog.log(`${task.id} release authorization required — leaving in triage awaiting release authorization (${signals})`); return; } } catch (error: unknown) { @@ -2504,6 +2506,9 @@ export class TriageProcessor { /* FNXC:PlanApproval 2026-06-26-00:00: Project planApprovalMode has precedence over the workflow-resolved requirePlanApproval value so operators can force auto-approval or manual approval for every task in this project. + + FNXC:PlanApproval 2026-07-01-08:12: + This is the ordinary manual plan-approval gate only, after release authorization and Workflow Plan Review have already made their independent decisions. Always call resolvePlanApprovalRequired with the merged settings object so project auto-approve-all can override workflow requirePlanApproval without weakening non-plan safety gates. */ if (resolvePlanApprovalRequired(settings)) { const approvalUpdates: Record = { status: "awaiting-approval" };