From 0a2fe913c69ddae33742a02f7be389510d8b8fa3 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Sat, 8 Aug 2026 23:34:41 -0700 Subject: [PATCH] FN-8863: fix standalone pre-merge remediation holds Allow standalone tasks to recover from failed pre-merge steps when project auto-merge is disabled. - Add a remediation-specific auto-merge hold for shared members and explicit user holds. - Restore Plan Review replans and Code Review fix handoffs for standalone tasks. - Cover hold behavior and document the operator-consent policy. Files changed: .changeset/fn-8863-remediation-auto-merge-hold.md | 7 ++ docs/dashboard-guide.md | 2 +- packages/core/src/__tests__/task-merge.test.ts | 38 ++++++++ packages/core/src/index.gate.ts | 1 + packages/core/src/index.ts | 1 + packages/core/src/merge/task-merge.ts | 18 ++++ ...cutor-live-branch-group-auto-merge-hold.test.ts | 27 ++++++ .../workflow-graph-optional-step-fix.test.ts | 106 +++++++++++++++++++++ packages/engine/src/executor.ts | 16 +++- 9 files changed, 212 insertions(+), 4 deletions(-) Fusion-Task-Id: FN-8863 Fusion-Task-Lineage: 930bf37a-3173-4a3b-84ca-575f7f5d94b9 Co-authored-by: Fusion (runfusion.ai) --- .../fn-8863-remediation-auto-merge-hold.md | 7 ++ docs/dashboard-guide.md | 2 +- .../core/src/__tests__/task-merge.test.ts | 38 +++++++ packages/core/src/index.gate.ts | 1 + packages/core/src/index.ts | 1 + packages/core/src/merge/task-merge.ts | 18 +++ ...-live-branch-group-auto-merge-hold.test.ts | 27 +++++ .../workflow-graph-optional-step-fix.test.ts | 106 ++++++++++++++++++ packages/engine/src/executor.ts | 16 ++- 9 files changed, 212 insertions(+), 4 deletions(-) create mode 100644 .changeset/fn-8863-remediation-auto-merge-hold.md diff --git a/.changeset/fn-8863-remediation-auto-merge-hold.md b/.changeset/fn-8863-remediation-auto-merge-hold.md new file mode 100644 index 0000000000..9f2d98323e --- /dev/null +++ b/.changeset/fn-8863-remediation-auto-merge-hold.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Restore plan-review replan and review fix handoffs in projects with auto-merge off. +category: fix +dev: Uses hasPreMergeRemediationAutoMergeHold at the two executor pre-merge remediation seams. diff --git a/docs/dashboard-guide.md b/docs/dashboard-guide.md index 5fa50c2fbe..a944930dc6 100644 --- a/docs/dashboard-guide.md +++ b/docs/dashboard-guide.md @@ -670,7 +670,7 @@ Rules: - `auto-new` creates a branch after task creation using `fusion/{task-id}-{short-name}` (for example `fusion/fn-5671-branch-strategy-dropdown`). - `Merge target / base branch` stays optional for all modes and uses the same branch-dropdown + `Custom…` fallback behavior as Planning Mode. - In **More options → Model Configuration**, **Auto-merge** is a per-task override with three states: **Default** (follow project setting), **Enabled**, or **Disabled**. -- A live mission/shared-branch task uses one operator-consent precedence at every member-integration boundary: explicit task **Enabled** opts in; otherwise project **Auto-merge Off** holds every task value (unset, user Off, mission, legacy, or inherited false) in Review. With project auto-merge On, only explicit user **Disabled** holds; engine-authored false values may use the live intermediate-group integration path. This does not change the separate shared-branch → default-branch promotion policy. In Task Detail edit mode, **Merge target / base branch** uses the existing branch control; clear it to return to the project default. +- A live mission/shared-branch task uses one operator-consent precedence at every member-integration boundary: explicit task **Enabled** opts in; otherwise project **Auto-merge Off** holds every task value (unset, user Off, mission, legacy, or inherited false) in Review. With project auto-merge On, only explicit user **Disabled** holds; engine-authored false values may use the live intermediate-group integration path. This governs merge/promotion and shared-member integration; standalone pre-merge remediation remains available unless the operator explicitly disables auto-merge for that task. This does not change the separate shared-branch → default-branch promotion policy. In Task Detail edit mode, **Merge target / base branch** uses the existing branch control; clear it to return to the project default. - In **More options → Model Configuration**, task and agent model pickers expose **Thinking Level** inside the same model dropdown panel instead of as a separate adjacent selector. Task pickers offer **Default (project setting)** plus **Off**, **Minimal**, **Low**, **Medium**, **High**, and **Very High**; agent creation, Agent Onboarding review, and Agent Detail built-in-model settings are concrete-only and start/fall back to **Off**. - Shared model dropdowns keep the active provider header visible while scrolling. Use the provider chevron to collapse a provider's model rows; this dashboard-local preference persists across sessions, while filtering temporarily shows matching rows from collapsed providers. - In **More options → Model Configuration**, **Planner oversight** is a per-task override of the workflow-native `plannerOversightLevel` setting (FN-7508): **Inherit from workflow** (default) plus **Off**, **Observe**, **Steer**, and **Autonomous recovery**. This selector appears in both the New Task dialog and the Task Detail edit form (same shared control). Selecting **Inherit from workflow** clears the per-task override (sent as `null` on edit, omitted on create) so the task falls back to the effective `plannerOversightLevel` configured on its workflow — set project/global defaults for this in the **Workflow Editor → Values** tab, not in Project Settings; it is workflow-native, not a project setting. diff --git a/packages/core/src/__tests__/task-merge.test.ts b/packages/core/src/__tests__/task-merge.test.ts index 4eacbda3e4..12cff58bcd 100644 --- a/packages/core/src/__tests__/task-merge.test.ts +++ b/packages/core/src/__tests__/task-merge.test.ts @@ -17,6 +17,7 @@ import { isSharedBranchGroupMemberIntegration, isLiveSharedBranchGroupMemberIntegration, hasSharedBranchMemberAutoMergeHold, + hasPreMergeRemediationAutoMergeHold, hasUserAutoMergeHold, resolveEffectiveAutoMerge, resolveEffectiveGroupAutoMerge, @@ -99,6 +100,43 @@ describe("hasSharedBranchMemberAutoMergeHold", () => { }); }); +describe("hasPreMergeRemediationAutoMergeHold", () => { + const taskValues = [undefined, true, false] as const; + const provenances = [undefined, "user", "mission", "legacy-stamp"] as const; + + it.each([false, true] as const)("uses the user hold for standalone tasks when project autoMerge is %s", (projectAutoMerge) => { + for (const autoMerge of taskValues) { + for (const autoMergeProvenance of provenances) { + expect(hasPreMergeRemediationAutoMergeHold( + { autoMerge, autoMergeProvenance }, + { autoMerge: projectAutoMerge }, + )).toBe(autoMerge === false && autoMergeProvenance === "user"); + } + } + }); + + it.each([false, true] as const)("matches the shared-member hold when project autoMerge is %s", (projectAutoMerge) => { + for (const autoMerge of taskValues) { + for (const autoMergeProvenance of provenances) { + const task = { + autoMerge, + autoMergeProvenance, + branchContext: { assignmentMode: "shared" as const, groupId: "BG-1" }, + }; + expect(hasPreMergeRemediationAutoMergeHold(task, { autoMerge: projectAutoMerge })) + .toBe(hasSharedBranchMemberAutoMergeHold(task, { autoMerge: projectAutoMerge })); + } + } + }); + + it.each(["", " "])("treats a blank shared group id as standalone (%j)", (groupId) => { + expect(hasPreMergeRemediationAutoMergeHold({ + autoMerge: undefined, + branchContext: { assignmentMode: "shared", groupId }, + }, { autoMerge: false })).toBe(false); + }); +}); + describe("hasUserAutoMergeHold", () => { it.each([ [{ autoMerge: false, autoMergeProvenance: "user" }, true], diff --git a/packages/core/src/index.gate.ts b/packages/core/src/index.gate.ts index f28d746f9c..38d5c21cf5 100644 --- a/packages/core/src/index.gate.ts +++ b/packages/core/src/index.gate.ts @@ -1020,6 +1020,7 @@ export { isTaskReadyForMerge, allowsAutoMergeProcessing, hasSharedBranchMemberAutoMergeHold, + hasPreMergeRemediationAutoMergeHold, hasUserAutoMergeHold, isSharedBranchGroupMemberIntegration, isLiveSharedBranchGroupMemberIntegration, diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index d663829bb3..afae7f87ec 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -1162,6 +1162,7 @@ export { isTaskReadyForMerge, allowsAutoMergeProcessing, hasSharedBranchMemberAutoMergeHold, + hasPreMergeRemediationAutoMergeHold, hasUserAutoMergeHold, isSharedBranchGroupMemberIntegration, isLiveSharedBranchGroupMemberIntegration, diff --git a/packages/core/src/merge/task-merge.ts b/packages/core/src/merge/task-merge.ts index 197c022ffd..5c68539516 100644 --- a/packages/core/src/merge/task-merge.ts +++ b/packages/core/src/merge/task-merge.ts @@ -125,6 +125,24 @@ export function hasUserAutoMergeHold( return task.autoMerge === false && task.autoMergeProvenance === "user"; } +/** + * FNXC:SharedBranchMemberHold 2026-08-09-06:11: + * FN-8863 narrows FN-8823's project-Off consent rule to shared members at + * pre-merge remediation seams. Applying it to standalone tasks disabled Plan + * Review replans, required-artifact recovery, provider-failure diagnostics, + * and Code Review fix handoffs in every auto-merge-Off project. Remediation + * reopens implementation rather than merging, so standalone tasks are fenced + * only by an operator-authored task-level Off. + */ +export function hasPreMergeRemediationAutoMergeHold( + task: Pick, + settings: Pick, +): boolean { + return isSharedBranchGroupMemberIntegration(task) + ? hasSharedBranchMemberAutoMergeHold(task, settings) + : hasUserAutoMergeHold(task); +} + /** * Gate for auto-merge *processing* (engine enqueue + self-healing sweeps). * Additive relative to the global setting: when `settings.autoMerge` is on, diff --git a/packages/engine/src/__tests__/executor-live-branch-group-auto-merge-hold.test.ts b/packages/engine/src/__tests__/executor-live-branch-group-auto-merge-hold.test.ts index d294e807c6..73cd73601b 100644 --- a/packages/engine/src/__tests__/executor-live-branch-group-auto-merge-hold.test.ts +++ b/packages/engine/src/__tests__/executor-live-branch-group-auto-merge-hold.test.ts @@ -135,6 +135,33 @@ describe("executor shared-branch autoMerge:false liveness gates", () => { )).resolves.toBe(true); }); + it("holds an unset shared member at both pre-merge remediation seams when project auto-merge is Off", async () => { + const { executor, store } = makeExecutor({ status: "open", branchName: "mission/M-1980" }); + const task = makeInReviewTask({ + workflowStepResults: [{ + workflowStepId: "code-review", + workflowStepName: "Code Review", + phase: "pre-merge", + status: "failed", + output: "Please revise", + }], + }); + store.getTask.mockResolvedValue(task); + const sendBack = vi.spyOn(executor as any, "sendTaskBackForFix"); + + await expect((executor as any).requestPreMergeOptionalStepFix(task.id, task, { + stepName: "Code Review", + feedback: "Please revise", + phase: "pre-merge", + status: "failed", + verdict: "REVISE", + nodeId: "code-review", + })).resolves.toBe(false); + await expect(executor.recoverFailedPreMergeWorkflowStep(task)).resolves.toBe(false); + + expect(sendBack).not.toHaveBeenCalled(); + }); + it("does not let live pre-merge remediation reopen an operator-held member", async () => { const { executor, store } = makeExecutor({ status: "open", branchName: "mission/M-1980" }); const task = makeInReviewTask({ autoMerge: false, autoMergeProvenance: "user" }); 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 f05884cdf6..cc740237d3 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 @@ -369,6 +369,66 @@ describe("TaskExecutor pre-merge optional-step fix seam", () => { expect((executor as any).pausedAborted.has("FN-7066")).toBe(false); }); + it("replans a standalone Plan Review REVISE with the default project-Off settings", async () => { + const store = createMockStore(); + const liveTask = task({ column: "in-progress", status: null }); + store.getTask.mockResolvedValue(liveTask); + const executor = new TaskExecutor(store, "/tmp/test"); + + await expect((executor as any).requestPreMergeOptionalStepFix(liveTask.id, liveTask, { + stepName: "Plan Review", + feedback: "Revise the task specification.", + phase: "pre-merge", + status: "failed", + verdict: "REVISE", + nodeId: "plan-review", + })).resolves.toBe(true); + + expect(store.moveTask).toHaveBeenCalledWith(liveTask.id, "todo", { preserveWorktree: true }); + expect(store.updateTask).toHaveBeenCalledWith(liveTask.id, expect.objectContaining({ status: "needs-replan" }), undefined); + }); + + it("holds shared members and user-held standalone tasks during project-Off Plan Review remediation", async () => { + for (const overrides of [ + { branchContext: { assignmentMode: "shared" as const, groupId: "BG-1" } }, + { autoMerge: false, autoMergeProvenance: "user" as const }, + ]) { + const store = createMockStore(); + const liveTask = task({ column: "in-progress", status: null, ...overrides }); + store.getTask.mockResolvedValue(liveTask); + const executor = new TaskExecutor(store, "/tmp/test"); + + await expect((executor as any).requestPreMergeOptionalStepFix(liveTask.id, liveTask, { + stepName: "Plan Review", + feedback: "Revise the task specification.", + phase: "pre-merge", + status: "failed", + verdict: "REVISE", + nodeId: "plan-review", + })).resolves.toBe(false); + + expect(store.moveTask).not.toHaveBeenCalled(); + expect(store.updateTask).not.toHaveBeenCalled(); + } + }); + + it("remediates a mission-policy shared member when project auto-merge is On", async () => { + const store = createMockStore(); + const liveTask = task({ + branchContext: { assignmentMode: "shared", groupId: "BG-1" }, + autoMerge: false, + autoMergeProvenance: "mission", + }); + store.getTask.mockResolvedValue(liveTask); + store.getSettings.mockResolvedValue({ autoMerge: true }); + const executor = new TaskExecutor(store, "/tmp/test"); + const sendBack = vi.spyOn(executor as any, "sendTaskBackForFix").mockResolvedValue(undefined); + + await expect((executor as any).requestPreMergeOptionalStepFix(liveTask.id, liveTask, reviseInfo)).resolves.toBe(true); + + expect(sendBack).toHaveBeenCalledOnce(); + }); + it("does not hard-cancel the graph that performs its own Plan Review replan move", async () => { const store = createMockStore(); const liveTask = task({ postReviewFixCount: 0, column: "in-progress", status: null }); @@ -748,6 +808,52 @@ describe("TaskExecutor pre-merge optional-step fix seam", () => { } }); + it("recovers a standalone failed Code Review with the default project-Off settings", async () => { + const store = createMockStore(); + const liveTask = task({ + column: "in-review", + workflowStepResults: [{ + workflowStepId: "code-review", + workflowStepName: "Code Review", + phase: "pre-merge", + status: "failed", + output: "Fix the review finding.", + }], + }); + const executor = new TaskExecutor(store, "/tmp/test"); + const sendBack = vi.spyOn(executor as any, "sendTaskBackForFix").mockResolvedValue(undefined); + + await expect(executor.recoverFailedPreMergeWorkflowStep(liveTask)).resolves.toBe(true); + + expect(sendBack).toHaveBeenCalledOnce(); + }); + + it("does not recover shared members or user-held standalone tasks during project-Off failed-step recovery", async () => { + for (const overrides of [ + { branchContext: { assignmentMode: "shared" as const, groupId: "BG-1" } }, + { autoMerge: false, autoMergeProvenance: "user" as const }, + ]) { + const store = createMockStore(); + const liveTask = task({ + column: "in-review", + ...overrides, + workflowStepResults: [{ + workflowStepId: "code-review", + workflowStepName: "Code Review", + phase: "pre-merge", + status: "failed", + output: "Fix the review finding.", + }], + }); + const executor = new TaskExecutor(store, "/tmp/test"); + const sendBack = vi.spyOn(executor as any, "sendTaskBackForFix"); + + await expect(executor.recoverFailedPreMergeWorkflowStep(liveTask)).resolves.toBe(false); + + expect(sendBack).not.toHaveBeenCalled(); + } + }); + it("keeps the retry presentation aligned with the next attempt during failed-step recovery", async () => { const store = createMockStore(); const liveTask = task({ diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index efe4e3f0d5..25054cdd48 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -16,7 +16,7 @@ import { DEFAULT_PROVIDER_INSTANCE_ID, type ProviderInstanceRef, type TaskStore, import { getUnmetSchedulingDependencies } from "./scheduler.js"; import type { ImplementationExit, ImplementationExitReporter } from "./executor/implementation-exit.js"; import { emitWorkflowLifecycleEvent } from "@fusion/core"; -import { resolveTaskLifecycleColumns, resolveProjectColumnsForRoles, resolveWipTargetForTask, resolveTerminalColumns, RetryStormError, serializeRetryStormError, evaluateCompletedPromotionFailureProvenance, evaluateSkipBypassTaint, resolveWorkflowIrForTask, columnsWithFlag, evaluateForeachMergeProof, resolveCompleteColumn, resolveMergeOrchestrationColumn, resolveReboundTarget, resolveLifecycleColumns, resolveColumnAgentBinding, resolveEffectiveAgent, instanceNodeId, getWorkflowExtensionRegistry, getBuiltinWorkflow, parseNoOpCompletionMarker, allowsAutoMergeProcessing, hasSharedBranchMemberAutoMergeHold, resolveEffectiveAutoMerge, isLiveSharedBranchGroupMemberIntegration, resolveMaxAutoMergeRetries, resolveMaxConsecutiveToolFailureRetries, resolveConsecutiveToolFailureRetryBackoffMs, resolveConsecutiveToolFailureThreshold, resolveExecutorEscalationTarget, resolveOptionalStepRevisionBudget, resolveOptionalReviewRevisionBudget, DEFAULT_MAX_POST_REVIEW_FIXES, COMPLETION_SUMMARY_NODE_ID, PLAN_REVIEW_GROUP_ID, upsertWorkflowStepResult, normalizeWorkflowReviewFindings, AWAITING_APPROVAL_PAUSE_REASON, THINKING_LEVELS, ACTIVE_WORKFLOW_WORK_ITEM_STATES, AgentStore, classifyWorkflowAgentNode, isWorkflowAgentRole, resolveExecutorFallbackModel, resolveValidatorFallbackModel, resolveExplicitDuplicateMarker, nonExecutableDuplicateRedirectReason } from "@fusion/core"; +import { resolveTaskLifecycleColumns, resolveProjectColumnsForRoles, resolveWipTargetForTask, resolveTerminalColumns, RetryStormError, serializeRetryStormError, evaluateCompletedPromotionFailureProvenance, evaluateSkipBypassTaint, resolveWorkflowIrForTask, columnsWithFlag, evaluateForeachMergeProof, resolveCompleteColumn, resolveMergeOrchestrationColumn, resolveReboundTarget, resolveLifecycleColumns, resolveColumnAgentBinding, resolveEffectiveAgent, instanceNodeId, getWorkflowExtensionRegistry, getBuiltinWorkflow, parseNoOpCompletionMarker, allowsAutoMergeProcessing, hasSharedBranchMemberAutoMergeHold, hasPreMergeRemediationAutoMergeHold, resolveEffectiveAutoMerge, isLiveSharedBranchGroupMemberIntegration, resolveMaxAutoMergeRetries, resolveMaxConsecutiveToolFailureRetries, resolveConsecutiveToolFailureRetryBackoffMs, resolveConsecutiveToolFailureThreshold, resolveExecutorEscalationTarget, resolveOptionalStepRevisionBudget, resolveOptionalReviewRevisionBudget, DEFAULT_MAX_POST_REVIEW_FIXES, COMPLETION_SUMMARY_NODE_ID, PLAN_REVIEW_GROUP_ID, upsertWorkflowStepResult, normalizeWorkflowReviewFindings, AWAITING_APPROVAL_PAUSE_REASON, THINKING_LEVELS, ACTIVE_WORKFLOW_WORK_ITEM_STATES, AgentStore, classifyWorkflowAgentNode, isWorkflowAgentRole, resolveExecutorFallbackModel, resolveValidatorFallbackModel, resolveExplicitDuplicateMarker, nonExecutableDuplicateRedirectReason } from "@fusion/core"; import { BLOCKED_THRASH_LIMIT, buildExternalBlockMetadataPatch, @@ -5993,8 +5993,13 @@ export class TaskExecutor { * an auto-merge admission preference. Pre-merge remediation must not reopen * implementation and thereby bypass that checkpoint before the operator * releases or revises the held member. + * + * FNXC:SharedBranchMemberHold 2026-08-09-06:11: + * FN-8863 confines FN-8823's project-Off consent arm to shared members at + * this remediation seam. Standalone remediation reopens implementation, + * not a merge, so only an operator-authored task Off holds it. */ - if (hasSharedBranchMemberAutoMergeHold(liveTask, await this.store.getSettings())) return false; + if (hasPreMergeRemediationAutoMergeHold(liveTask, await this.store.getSettings())) return false; const missingArtifactKeys = parseRequiredArtifactMissingValue(info.failureValue); if (missingArtifactKeys) { await this.recoverMissingRequiredArtifacts(liveTask, missingArtifactKeys, { @@ -6319,8 +6324,13 @@ export class TaskExecutor { * Startup/self-healing recovery is another pre-merge remediation requester. * Do not let it send a user-held member back to execution: only an explicit * operator release or revision may advance that manual checkpoint. + * + * FNXC:SharedBranchMemberHold 2026-08-09-06:11: + * FN-8863 keeps the project-Off consent hold for shared members while + * allowing standalone failed-step recovery unless the operator authored a + * task-level Off; recovery reopens implementation and never merges. */ - if (hasSharedBranchMemberAutoMergeHold(task, await this.store.getSettings())) return false; + if (hasPreMergeRemediationAutoMergeHold(task, await this.store.getSettings())) return false; /* FNXC:WorkflowPostMerge 2026-06-26-14:00: U7c: gate-ness is now sourced from the recorded `WorkflowStepResult.status`, NOT a