From ad468813d5944a52d751e3a2fbbda56a0b58d9ac Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Wed, 3 Jun 2026 10:18:24 -0700 Subject: [PATCH 1/3] fix(engine): honor per-task auto-merge override when global auto-merge is off MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tasks with autoMerge explicitly enabled never auto-merged when the project-level setting was disabled: the merge enqueue gate (allowInReviewMergeProcessing) and all 19 in-review self-healing sweeps checked only settings.autoMerge, and the board stall-signal hydration passed the raw global into the diagnostic gates. Introduce allowsAutoMergeProcessing(task, settings) in core — additive relative to the global setting so configs with global auto-merge ON are unchanged (explicit autoMerge:false tasks still flow to the merger's manual-required parking) — and use it at the enqueue gate, every self-healing sweep, and the store's stall/stalled signal contexts. --- .changeset/per-task-automerge-override.md | 5 + .../core/src/__tests__/task-merge.test.ts | 18 +++ packages/core/src/index.ts | 1 + packages/core/src/store.ts | 13 +- packages/core/src/task-merge.ts | 17 +++ .../src/__tests__/project-engine.test.ts | 26 ++++ .../engine/src/__tests__/self-healing.test.ts | 88 ++++++++++- packages/engine/src/project-engine.ts | 6 +- packages/engine/src/self-healing.ts | 141 ++++++++++-------- 9 files changed, 241 insertions(+), 74 deletions(-) create mode 100644 .changeset/per-task-automerge-override.md diff --git a/.changeset/per-task-automerge-override.md b/.changeset/per-task-automerge-override.md new file mode 100644 index 0000000000..ae495efe7e --- /dev/null +++ b/.changeset/per-task-automerge-override.md @@ -0,0 +1,5 @@ +--- +"@runfusion/fusion": patch +--- + +Respect per-task auto-merge overrides when the global auto-merge setting is off. Tasks with auto-merge explicitly enabled now get enqueued for merge and covered by the in-review self-healing sweeps (stall surfacing, merged-task finalization, retry recovery) even when the project-level setting is disabled; tasks without an explicit override keep the PR-based/manual review flow untouched. diff --git a/packages/core/src/__tests__/task-merge.test.ts b/packages/core/src/__tests__/task-merge.test.ts index b9a3854406..c7e26e981f 100644 --- a/packages/core/src/__tests__/task-merge.test.ts +++ b/packages/core/src/__tests__/task-merge.test.ts @@ -8,6 +8,7 @@ import { getTaskHardMergeBlocker, getTaskMergeBlocker, isTaskReadyForMerge, + allowsAutoMergeProcessing, isSharedBranchGroupMemberIntegration, resolveEffectiveAutoMerge, resolveEffectiveGroupAutoMerge, @@ -46,6 +47,23 @@ describe("resolveEffectiveAutoMerge", () => { }); }); +describe("allowsAutoMergeProcessing", () => { + it("lets explicit per-task true through when the global setting is off (FN per-task override)", () => { + expect(allowsAutoMergeProcessing({ autoMerge: true }, { autoMerge: false })).toBe(true); + }); + + it("blocks tasks without an explicit override when the global setting is off", () => { + expect(allowsAutoMergeProcessing({ autoMerge: undefined }, { autoMerge: false })).toBe(false); + expect(allowsAutoMergeProcessing({ autoMerge: false }, { autoMerge: false })).toBe(false); + }); + + it("lets everything through when the global setting is on — explicit false still flows so the merger can park it manual-required", () => { + expect(allowsAutoMergeProcessing({ autoMerge: undefined }, { autoMerge: true })).toBe(true); + expect(allowsAutoMergeProcessing({ autoMerge: true }, { autoMerge: true })).toBe(true); + expect(allowsAutoMergeProcessing({ autoMerge: false }, { autoMerge: true })).toBe(true); + }); +}); + describe("resolveEffectiveGroupAutoMerge", () => { it("prefers explicit true over global false", () => { expect(resolveEffectiveGroupAutoMerge({ autoMerge: true }, { autoMerge: false })).toBe(true); diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index a00a01622f..a362011a06 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -318,6 +318,7 @@ export { getTaskHardMergeBlocker, getTaskCompletionBlocker, isTaskReadyForMerge, + allowsAutoMergeProcessing, isSharedBranchGroupMemberIntegration, resolveEffectiveAutoMerge, resolveEffectiveGroupAutoMerge, diff --git a/packages/core/src/store.ts b/packages/core/src/store.ts index d5ff59dcb9..f5684fbc5c 100644 --- a/packages/core/src/store.ts +++ b/packages/core/src/store.ts @@ -9,6 +9,7 @@ import { VALID_TRANSITIONS, DEFAULT_SETTINGS, isGlobalOnlySettingsKey, WORKFLOW_ import { DEFAULT_PROJECT_SETTINGS } from "./settings-schema.js"; import { resolveWorktrunkSettings, validateWorktrunkSettings } from "./worktrunk-settings.js"; import { normalizeTaskPriority } from "./task-priority.js"; +import { allowsAutoMergeProcessing } from "./task-merge.js"; import { canAgentTakeImplementationTaskForExplicitRouting } from "./agent-role-policy.js"; import { GlobalSettingsStore } from "./global-settings.js"; import { Database, SCHEMA_VERSION, toJson, toJsonNullable, fromJson } from "./db.js"; @@ -4597,7 +4598,7 @@ export class TaskStore extends EventEmitter { const task = this.rowToTask(row); task.inReviewStall = getInReviewStallReason(task, { now, - autoMerge: settings.autoMerge, + autoMerge: allowsAutoMergeProcessing(task, settings), engineActiveSinceMs: settings.engineActiveSinceMs, engineActivationGraceMs: settings.engineActivationGraceMs, }); @@ -4610,7 +4611,7 @@ export class TaskStore extends EventEmitter { task.inReviewStalled = getInReviewStalledSignal(task, { now, thresholdMs: settings.inReviewStalledThresholdMs, - autoMerge: settings.autoMerge, + autoMerge: allowsAutoMergeProcessing(task, settings), engineActiveSinceMs: settings.engineActiveSinceMs, engineActivationGraceMs: settings.engineActivationGraceMs, }); @@ -4853,7 +4854,7 @@ export class TaskStore extends EventEmitter { const task = this.rowToTask(row); task.inReviewStall = getInReviewStallReason(task, { now, - autoMerge: settings.autoMerge, + autoMerge: allowsAutoMergeProcessing(task, settings), engineActiveSinceMs: settings.engineActiveSinceMs, engineActivationGraceMs: settings.engineActivationGraceMs, }); @@ -4866,7 +4867,7 @@ export class TaskStore extends EventEmitter { task.inReviewStalled = getInReviewStalledSignal(task, { now, thresholdMs: settings.inReviewStalledThresholdMs, - autoMerge: settings.autoMerge, + autoMerge: allowsAutoMergeProcessing(task, settings), engineActiveSinceMs: settings.engineActiveSinceMs, engineActivationGraceMs: settings.engineActivationGraceMs, }); @@ -5016,7 +5017,7 @@ export class TaskStore extends EventEmitter { const task = this.rowToTask(row); task.inReviewStall = getInReviewStallReason(task, { now, - autoMerge: settings.autoMerge, + autoMerge: allowsAutoMergeProcessing(task, settings), engineActiveSinceMs: settings.engineActiveSinceMs, engineActivationGraceMs: settings.engineActivationGraceMs, }); @@ -5029,7 +5030,7 @@ export class TaskStore extends EventEmitter { task.inReviewStalled = getInReviewStalledSignal(task, { now, thresholdMs: settings.inReviewStalledThresholdMs, - autoMerge: settings.autoMerge, + autoMerge: allowsAutoMergeProcessing(task, settings), engineActiveSinceMs: settings.engineActiveSinceMs, engineActivationGraceMs: settings.engineActivationGraceMs, }); diff --git a/packages/core/src/task-merge.ts b/packages/core/src/task-merge.ts index 479a965a98..0caaac894d 100644 --- a/packages/core/src/task-merge.ts +++ b/packages/core/src/task-merge.ts @@ -47,6 +47,23 @@ export function resolveEffectiveAutoMerge( return task.autoMerge ?? settings.autoMerge; } +/** + * Gate for auto-merge *processing* (engine enqueue + self-healing sweeps). + * Additive relative to the global setting: when `settings.autoMerge` is on, + * every task flows through — tasks with an explicit `autoMerge: false` are + * parked as `manual-required` downstream by the merger, not silently skipped + * here. When the global setting is off, only tasks with an explicit per-task + * `autoMerge: true` override proceed. Distinct from + * `resolveEffectiveAutoMerge`, which resolves the effective boolean and would + * (incorrectly for processing gates) starve the manual-required parking path. + */ +export function allowsAutoMergeProcessing( + task: Pick, + settings: Pick, +): boolean { + return settings.autoMerge !== false || task.autoMerge === true; +} + // Resolves group → default-branch PROMOTION auto-merge. See resolveEffectiveAutoMerge for the per-task member→group-integration step; the two are distinct and must not be conflated. export function resolveEffectiveGroupAutoMerge( group: Pick, diff --git a/packages/engine/src/__tests__/project-engine.test.ts b/packages/engine/src/__tests__/project-engine.test.ts index 4d9774959a..4c46d088ac 100644 --- a/packages/engine/src/__tests__/project-engine.test.ts +++ b/packages/engine/src/__tests__/project-engine.test.ts @@ -2670,3 +2670,29 @@ describe("ProjectEngine stale mergeActive rescue (FN-3900)", () => { await engine.stop(); }); }); + +describe("allowInReviewMergeProcessing per-task autoMerge override", () => { + const gate = (task: Partial, settings: { autoMerge: boolean }) => + (createEngine() as any).allowInReviewMergeProcessing(task, settings) as boolean; + + it("lets an explicit per-task autoMerge:true through when the global setting is off", () => { + expect(gate({ autoMerge: true }, { autoMerge: false })).toBe(true); + }); + + it("blocks tasks without a per-task override when the global setting is off", () => { + expect(gate({}, { autoMerge: false })).toBe(false); + expect(gate({ autoMerge: false }, { autoMerge: false })).toBe(false); + }); + + it("keeps everything flowing when the global setting is on — explicit autoMerge:false is parked manual-required downstream", () => { + expect(gate({}, { autoMerge: true })).toBe(true); + expect(gate({ autoMerge: false }, { autoMerge: true })).toBe(true); + }); + + it("still exempts shared-branch-group member integration when the global setting is off", () => { + expect(gate( + { branchContext: { assignmentMode: "shared", groupId: "grp-1" } as Task["branchContext"] }, + { autoMerge: false }, + )).toBe(true); + }); +}); diff --git a/packages/engine/src/__tests__/self-healing.test.ts b/packages/engine/src/__tests__/self-healing.test.ts index 18e547aa53..87db2fbb2f 100644 --- a/packages/engine/src/__tests__/self-healing.test.ts +++ b/packages/engine/src/__tests__/self-healing.test.ts @@ -3365,7 +3365,8 @@ describe("SelfHealingManager", () => { const result = await managerWithRecovery.recoverMergeableReviewTasks(); expect(result).toBe(0); - expect(store.listTasks).not.toHaveBeenCalled(); + // The sweep may list tasks to discover per-task autoMerge overrides, + // but must not merge or enqueue anything without one. expect(store.mergeTask).not.toHaveBeenCalled(); expect(enqueueMerge).not.toHaveBeenCalled(); @@ -3747,7 +3748,10 @@ describe("SelfHealingManager", () => { const result = await managerWithRecovery.finalizeNoOpReviewTasks(); expect(result).toBe(0); - expect(store.listTasks).not.toHaveBeenCalled(); + // The sweep may list tasks to discover per-task autoMerge overrides, + // but must not finalize anything without one. + expect(store.moveTask).not.toHaveBeenCalled(); + expect(store.updateTask).not.toHaveBeenCalled(); managerWithRecovery.stop(); }); @@ -8227,26 +8231,98 @@ describe("autoMerge gating for mutating in-review sweeps (FN-5147)", () => { "recoverMissingWorktreeReviewFailures", "recoverPartialProgressNoTaskDoneFailures", "reclaimSelfOwnedBranchConflicts", - ] as const)("skips entirely when autoMerge is disabled (respects PR-based review flow): %s", async (methodName) => { + ] as const)("performs no mutations when autoMerge is disabled and no per-task override exists: %s", async (methodName) => { if (methodName === "recoverReviewTasksWithFailedPreMergeSteps") { manager = new SelfHealingManager(store, { rootDir: "/tmp/test-project", recoverFailedPreMergeStep: vi.fn() }); } const result = await (manager as any)[methodName](); expect(result).toBe(0); - expect(store.listTasks).not.toHaveBeenCalled(); + // The sweep may list tasks to discover per-task autoMerge overrides, + // but must not mutate anything without one (respects PR-based review flow). expect(store.moveTask).not.toHaveBeenCalled(); expect(store.updateTask).not.toHaveBeenCalled(); expect(store.logEntry).not.toHaveBeenCalled(); }); - it("skips entirely when autoMerge is disabled (respects PR-based review flow): recoverCompletionHandoffLimbo", async () => { + it("performs no mutations when autoMerge is disabled and no per-task override exists: recoverCompletionHandoffLimbo", async () => { const result = await manager.recoverCompletionHandoffLimbo(); expect(result).toBeUndefined(); - expect(store.listTasks).not.toHaveBeenCalled(); expect(store.moveTask).not.toHaveBeenCalled(); expect(store.updateTask).not.toHaveBeenCalled(); expect(store.logEntry).not.toHaveBeenCalled(); }); + + it("surfaces in-review stalls for tasks with an explicit autoMerge:true override when the global setting is off", async () => { + vi.useFakeTimers(); + try { + vi.setSystemTime(new Date("2026-01-01T00:10:00.000Z")); + (store.getSettings as ReturnType).mockResolvedValue({ + autoMerge: false, + globalPause: false, + enginePaused: false, + taskStuckTimeoutMs: 60_000, + }); + (store.listTasks as ReturnType).mockResolvedValue([ + { + id: "FN-OVERRIDE", + column: "in-review", + paused: false, + status: "merging", + autoMerge: true, + steps: [], + log: [], + updatedAt: new Date(Date.parse("2026-01-01T00:10:00.000Z") - 600_000).toISOString(), + columnMovedAt: new Date(Date.parse("2026-01-01T00:10:00.000Z") - 600_000).toISOString(), + }, + ]); + + const surfaced = await manager.surfaceInReviewStalls(); + + expect(surfaced).toBe(1); + expect(store.logEntry).toHaveBeenCalledWith( + "FN-OVERRIDE", + expect.stringContaining("In-review stall surfaced ["), + ); + } finally { + vi.useRealTimers(); + } + }); + + it("keeps skipping override-less siblings while processing the override task", async () => { + vi.useFakeTimers(); + try { + vi.setSystemTime(new Date("2026-01-01T00:10:00.000Z")); + const staleFields = { + column: "in-review", + paused: false, + status: "merging", + steps: [], + log: [], + updatedAt: new Date(Date.parse("2026-01-01T00:10:00.000Z") - 600_000).toISOString(), + columnMovedAt: new Date(Date.parse("2026-01-01T00:10:00.000Z") - 600_000).toISOString(), + }; + (store.getSettings as ReturnType).mockResolvedValue({ + autoMerge: false, + globalPause: false, + enginePaused: false, + taskStuckTimeoutMs: 60_000, + }); + (store.listTasks as ReturnType).mockResolvedValue([ + { id: "FN-OVERRIDE", autoMerge: true, ...staleFields }, + { id: "FN-MANUAL", ...staleFields }, + ]); + + const surfaced = await manager.surfaceInReviewStalls(); + + expect(surfaced).toBe(1); + expect(store.logEntry).not.toHaveBeenCalledWith( + "FN-MANUAL", + expect.stringContaining("In-review stall surfaced ["), + ); + } finally { + vi.useRealTimers(); + } + }); }); describe("FN-5335 triple-proof no-action unit coverage", () => { diff --git a/packages/engine/src/project-engine.ts b/packages/engine/src/project-engine.ts index cfa8d5ca20..fadda27a09 100644 --- a/packages/engine/src/project-engine.ts +++ b/packages/engine/src/project-engine.ts @@ -10,7 +10,7 @@ import type { ScheduledTask, AutomationRunResult, } from "@fusion/core"; -import { compareTasksByPriorityThenAgeAndId, getTaskHardMergeBlocker, isSharedBranchGroupMemberIntegration, normalizeMergerMode, sortTasksByPriorityThenAgeAndId } from "@fusion/core"; +import { allowsAutoMergeProcessing, compareTasksByPriorityThenAgeAndId, getTaskHardMergeBlocker, isSharedBranchGroupMemberIntegration, normalizeMergerMode, sortTasksByPriorityThenAgeAndId } from "@fusion/core"; import { execFile } from "node:child_process"; import { promisify } from "node:util"; import { InProcessRuntime } from "./runtimes/in-process-runtime.js"; @@ -1383,8 +1383,8 @@ export class ProjectEngine { * pushed wins. listTasks returns createdAt ASC — without this sort an * older low-priority task would start before a later urgent one. */ - private allowInReviewMergeProcessing(task: Pick, settings: Pick): boolean { - return settings.autoMerge || isSharedBranchGroupMemberIntegration(task); + private allowInReviewMergeProcessing(task: Pick, settings: Pick): boolean { + return allowsAutoMergeProcessing(task, settings) || isSharedBranchGroupMemberIntegration(task); } private enqueueEligibleInReviewTasks(tasks: readonly Task[], settings: Pick): number { diff --git a/packages/engine/src/self-healing.ts b/packages/engine/src/self-healing.ts index 4f73c4410a..929dbaea75 100644 --- a/packages/engine/src/self-healing.ts +++ b/packages/engine/src/self-healing.ts @@ -28,7 +28,7 @@ import { promisify } from "node:util"; import { setImmediate as setImmediateCb } from "node:timers"; import { existsSync, mkdirSync, readdirSync, readFileSync, realpathSync, rmSync, statSync, writeFileSync } from "node:fs"; import { isAbsolute, join, relative, resolve } from "node:path"; -import { IN_REVIEW_STALL_DEADLOCK_LOG_PREFIX, IN_REVIEW_STALL_LOG_PREFIX, countRecentIdenticalStallEntries, detectDependencyCycle, detectSelfDefeatingDependency, getInReviewStalledSignal, getInReviewStallReason, getPrimaryPrInfo, getStalePausedReviewSignal, getStalePausedTodoSignal, getTaskHardMergeBlocker, getTaskMergeBlocker, isEphemeralAgent, isMergeRequestContractShadowEnabled, isSharedBranchGroupMemberIntegration, parseExplicitDuplicateMarker, type AgentStore, type ChatStore, type MessageStore, type TaskStore, type Settings, type Task, type MergeDetails, type TaskPriority, type MergeResult } from "@fusion/core"; +import { IN_REVIEW_STALL_DEADLOCK_LOG_PREFIX, IN_REVIEW_STALL_LOG_PREFIX, allowsAutoMergeProcessing, countRecentIdenticalStallEntries, detectDependencyCycle, detectSelfDefeatingDependency, getInReviewStalledSignal, getInReviewStallReason, getPrimaryPrInfo, getStalePausedReviewSignal, getStalePausedTodoSignal, getTaskHardMergeBlocker, getTaskMergeBlocker, isEphemeralAgent, isMergeRequestContractShadowEnabled, isSharedBranchGroupMemberIntegration, parseExplicitDuplicateMarker, type AgentStore, type ChatStore, type MessageStore, type TaskStore, type Settings, type Task, type MergeDetails, type TaskPriority, type MergeResult } from "@fusion/core"; import type { MeshLeaseManager } from "./mesh-lease-manager.js"; import { createLogger, schedulerLog } from "./logger.js"; import { RemovalReason, classifyTaskWorktree, getRegisteredWorktreeBranchMap, getRegisteredWorktreePaths, isUsableTaskWorktree, removeWorktree, resolveWorktreeBackend, scanIdleWorktrees, scanOrphanedBranches } from "./worktree-pool.js"; @@ -2302,14 +2302,14 @@ export class SelfHealingManager { * Backward lifecycle move gated on triple proof (FN-5335). * When the predicate fails, emits `task:reclaim-self-owned-branch-conflict-no-action` and skips lifecycle mutation. * - * No-op when `settings.autoMerge === false` — PR-based review flow owns lifecycle until human merge. + * Skips tasks not eligible for auto-merge processing (global `autoMerge` + * off without an explicit per-task `autoMerge: true` override) — PR-based + * review flow owns lifecycle until human merge. */ async reclaimSelfOwnedBranchConflicts(): Promise { try { const settings = await this.store.getSettings(); if (settings.globalPause || settings.enginePaused) return 0; - if (settings.autoMerge === false) return 0; - const todoCandidates = await this.store.listTasks({ column: "todo", slim: true }); const inProgressCandidates = await this.store.listTasks({ column: "in-progress", slim: true }); const inProgressByWorktree = new Map(); @@ -2320,7 +2320,8 @@ export class SelfHealingManager { } const inReviewPausedCandidates = (await this.store.listTasks({ column: "in-review", slim: true })) .filter((task) => task.paused === true && task.pausedReason === "branch-conflict-unrecoverable"); - const candidates = [...todoCandidates, ...inProgressCandidates, ...inReviewPausedCandidates]; + const candidates = [...todoCandidates, ...inProgressCandidates, ...inReviewPausedCandidates] + .filter((task) => allowsAutoMergeProcessing(task, settings)); const activeTaskIds = new Set(); if (this.options.agentStore) { @@ -4484,17 +4485,18 @@ export class SelfHealingManager { * Backward lifecycle move gated on triple proof (FN-5335). * When the unproven fallback predicate fails, emits `task:finalize-no-op-review-no-action` and skips lifecycle mutation. * - * No-op when `settings.autoMerge === false` — PR-based review flow owns lifecycle until human merge. + * Skips tasks not eligible for auto-merge processing (global `autoMerge` + * off without an explicit per-task `autoMerge: true` override) — PR-based + * review flow owns lifecycle until human merge. */ async finalizeNoOpReviewTasks(): Promise { try { const settings = await this.store.getSettings(); if (settings.globalPause || settings.enginePaused) return 0; - if (settings.autoMerge === false) return 0; - const tasks = await this.store.listTasks({ column: "in-review", slim: true }); const candidates = tasks.filter((t) => t.column === "in-review" && + allowsAutoMergeProcessing(t, settings) && !t.paused && !isSharedBranchGroupMemberIntegration(t) && Boolean(t.worktree) && @@ -4793,12 +4795,11 @@ export class SelfHealingManager { // "pull-request"`) — see GitHub issue #21. const settings = await this.store.getSettings(); if (settings.globalPause || settings.enginePaused) return 0; - if (settings.autoMerge === false) return 0; - const tasks = await this.store.listTasks({ column: "in-review", slim: true }); const mergeable = tasks.filter((t) => t.column === "in-review" && + allowsAutoMergeProcessing(t, settings) && !t.paused && t.status !== "failed" && // Exclude transient merge statuses. Active merges should be left alone; @@ -4898,7 +4899,9 @@ export class SelfHealingManager { * per-task `postReviewFixCount` so a persistently-failing verifier cannot * ping-pong a task forever. * - * No-op when `settings.autoMerge === false` — PR-based review flow owns lifecycle until human merge. + * Skips tasks not eligible for auto-merge processing (global `autoMerge` + * off without an explicit per-task `autoMerge: true` override) — PR-based + * review flow owns lifecycle until human merge. * @returns Number of tasks sent back for fix */ async recoverReviewTasksWithFailedPreMergeSteps(): Promise { @@ -4908,7 +4911,6 @@ export class SelfHealingManager { try { const settings = await this.store.getSettings(); if (settings.globalPause || settings.enginePaused) return 0; - if (settings.autoMerge === false) return 0; const maxFixes = settings.maxPostReviewFixes ?? 1; if (!Number.isFinite(maxFixes) || maxFixes <= 0) return 0; @@ -4917,6 +4919,7 @@ export class SelfHealingManager { const candidates = tasks.filter((task) => { if (task.column !== "in-review") return false; + if (!allowsAutoMergeProcessing(task, settings)) return false; if (task.paused) return false; // Preserve terminal/human-handoff statuses (failed, awaiting-user-review, // merging, etc.). Only revive tasks that are otherwise idle. @@ -4994,13 +4997,14 @@ export class SelfHealingManager { * incomplete step instead of leaving the task stranded in review. * Backward lifecycle move gated on triple proof (FN-5335). * When the predicate fails, emits `task:stale-incomplete-review-no-action` and skips lifecycle mutation. - * No-op when `settings.autoMerge === false` — PR-based review flow owns lifecycle until human merge. + * Skips tasks not eligible for auto-merge processing (global `autoMerge` + * off without an explicit per-task `autoMerge: true` override) — PR-based + * review flow owns lifecycle until human merge. */ async recoverStaleIncompleteReviewTasks(): Promise { try { const settings = await this.store.getSettings(); if (settings.globalPause || settings.enginePaused) return 0; - if (settings.autoMerge === false) return 0; const timeoutMs = settings.taskStuckTimeoutMs; if (!timeoutMs || timeoutMs <= 0) return 0; @@ -5008,6 +5012,7 @@ export class SelfHealingManager { const tasks = await this.store.listTasks({ column: "in-review", slim: true }); const staleIncomplete = tasks.filter((task) => task.column === "in-review" && + allowsAutoMergeProcessing(task, settings) && !task.paused && !task.status && task.steps.length > 0 && @@ -5056,8 +5061,9 @@ export class SelfHealingManager { * Final-fallback recovery for `in-review` tasks that fell through every other * scan and have sat untouched longer than `taskStuckTimeoutMs`. * - * When `settings.autoMerge` is disabled, this sweep is a no-op because - * PR-based manual review intentionally leaves tasks in `in-review`. + * Tasks not eligible for auto-merge processing (global `autoMerge` off + * without an explicit per-task `autoMerge: true` override) are skipped + * because PR-based manual review intentionally leaves them in `in-review`. * * The other review-recovery scans each require a specific shape (failed * pre-merge step, incomplete steps, mergeable + worktree present, confirmed @@ -5078,8 +5084,10 @@ export class SelfHealingManager { * each kick refreshes `updatedAt`, so a task that re-enters review and gets * stuck again can only be kicked once per `taskStuckTimeoutMs` window. * - * When `settings.autoMerge === false`, this sweep is a no-op because those - * projects intentionally use PR-based/manual in-review ownership. + * Tasks not eligible for auto-merge processing (global `autoMerge` off + * without an explicit per-task `autoMerge: true` override) are skipped + * because those projects intentionally use PR-based/manual in-review + * ownership. * * @returns Number of tasks kicked back to todo */ @@ -5087,8 +5095,6 @@ export class SelfHealingManager { try { const settings = await this.store.getSettings(); if (settings.globalPause || settings.enginePaused) return 0; - if (settings.autoMerge === false) return 0; - const cycleStartMs = Date.now(); const timeoutMs = settings.taskStuckTimeoutMs; if (!timeoutMs || timeoutMs <= 0) return 0; @@ -5100,6 +5106,7 @@ export class SelfHealingManager { for (const task of tasks) { if (task.deletedAt) continue; + if (!allowsAutoMergeProcessing(task, settings)) continue; const signal = getInReviewStallReason(task, { now: cycleStartMs, activeMergeTaskId, @@ -5217,14 +5224,14 @@ export class SelfHealingManager { * - `surfaceStalePausedReviews()` owns paused in-review tasks. * - `surfaceInReviewStalls()` owns reason-driven in-review stalls. * - * No-op when `settings.autoMerge === false` — PR-based review flow owns lifecycle until human merge. + * Skips tasks not eligible for auto-merge processing (global `autoMerge` + * off without an explicit per-task `autoMerge: true` override) — PR-based + * review flow owns lifecycle until human merge. */ async surfaceInReviewStalled(): Promise { try { const settings = await this.store.getSettings(); if (settings.globalPause || settings.enginePaused) return 0; - if (settings.autoMerge === false) return 0; - const cycleStartMs = Date.now(); const thresholdMs = settings.inReviewStalledThresholdMs; if (!thresholdMs || thresholdMs <= 0) return 0; @@ -5236,6 +5243,7 @@ export class SelfHealingManager { for (const task of tasks) { if (task.deletedAt) continue; + if (!allowsAutoMergeProcessing(task, settings)) continue; if (task.paused === true) continue; if (task.id === activeMergeTaskId || executingTaskIds.has(task.id)) continue; @@ -5389,13 +5397,14 @@ export class SelfHealingManager { * Backward lifecycle move gated on triple proof (FN-5335). * When the predicate fails, emits `task:ghost-review-no-action` and skips lifecycle mutation. * - * No-op when `settings.autoMerge === false` — PR-based review flow owns lifecycle until human merge. + * Skips tasks not eligible for auto-merge processing (global `autoMerge` + * off without an explicit per-task `autoMerge: true` override) — PR-based + * review flow owns lifecycle until human merge. */ async recoverGhostReviewTasks(): Promise { try { const settings = await this.store.getSettings(); if (settings.globalPause || settings.enginePaused) return 0; - if (settings.autoMerge === false) return 0; const timeoutMs = settings.taskStuckTimeoutMs; if (!timeoutMs || timeoutMs <= 0) return 0; @@ -5404,6 +5413,7 @@ export class SelfHealingManager { const tasks = await this.store.listTasks({ column: "in-review", slim: true }); const ghosts = tasks.filter((task) => task.column === "in-review" && + allowsAutoMergeProcessing(task, settings) && !task.paused && !executingIds.has(task.id) && !(task.status && GHOST_REVIEW_PRESERVED_STATUSES.has(task.status)) && @@ -5465,7 +5475,9 @@ export class SelfHealingManager { * If no landed commit is found, it only clears the stale transient status so * the normal mergeable-review recovery can retry the merge. * - * No-op when `settings.autoMerge === false` — PR-based review flow owns lifecycle until human merge. + * Skips tasks not eligible for auto-merge processing (global `autoMerge` + * off without an explicit per-task `autoMerge: true` override) — PR-based + * review flow owns lifecycle until human merge. * @returns Number of tasks finalized or unblocked */ /** @@ -5486,8 +5498,9 @@ export class SelfHealingManager { * parked as failed and emit `merger:transient-failure-budget-exhausted` * once for diagnostic visibility. * - * No-op when `settings.autoMerge === false`, no `requeueForAutoMerge` - * callback is wired, or global/engine pause is active. + * Skips tasks not eligible for auto-merge processing (global `autoMerge` + * off without a per-task `autoMerge: true` override). No-op when no + * `requeueForAutoMerge` callback is wired or global/engine pause is active. * * @returns Number of tasks recovered */ @@ -5496,12 +5509,12 @@ export class SelfHealingManager { if (!requeue) return 0; try { const settings = await this.store.getSettings(); - if (settings.autoMerge === false) return 0; if (settings.globalPause || settings.enginePaused) return 0; const slim = await this.store.listTasks({ column: "in-review", slim: true }); const candidates = slim.filter((t) => t.column === "in-review" + && allowsAutoMergeProcessing(t, settings) && t.status === "failed" && (t.mergeRetries ?? 0) >= MAX_AUTO_MERGE_RETRIES && typeof t.error === "string" @@ -5642,13 +5655,13 @@ export class SelfHealingManager { try { const settings = await this.store.getSettings(); if (settings.globalPause || settings.enginePaused) return 0; - if (settings.autoMerge === false) return 0; const timeoutMs = settings.taskStuckTimeoutMs; if (!timeoutMs || timeoutMs <= 0) return 0; const tasks = await this.store.listTasks({ column: "in-review", slim: true }); const candidates = tasks.filter((task) => task.column === "in-review" && + allowsAutoMergeProcessing(task, settings) && !task.paused && Boolean(task.status && ACTIVE_MERGE_STATUSES.has(task.status)) && this.isPastInterruptedMergeGrace(task, timeoutMs), @@ -5956,20 +5969,21 @@ export class SelfHealingManager { * but a later transition failed or another process moved the task before the * final `in-review` → `done` update completed. * - * No-op when `settings.autoMerge === false` — PR-based review flow owns lifecycle until human merge. + * Skips tasks not eligible for auto-merge processing (global `autoMerge` + * off without an explicit per-task `autoMerge: true` override) — PR-based + * review flow owns lifecycle until human merge. * @returns Number of tasks recovered */ async recoverMergedReviewTasks(): Promise { try { const settings = await this.store.getSettings(); if (settings.globalPause || settings.enginePaused) return 0; - if (settings.autoMerge === false) return 0; - const tasks = await this.store.listTasks({ column: "in-review", slim: true }); const mergedButNotDone = tasks.filter((t) => !t.deletedAt && t.column === "in-review" && + allowsAutoMergeProcessing(t, settings) && t.mergeDetails?.mergeConfirmed === true, ); @@ -6087,14 +6101,14 @@ export class SelfHealingManager { * When the no-landed predicate fails, emits `task:stuck-merge-deadlock-no-action` and skips lifecycle mutation. */ /** - * No-op when `settings.autoMerge === false` — PR-based review flow owns lifecycle until human merge. + * Skips tasks not eligible for auto-merge processing (global `autoMerge` + * off without an explicit per-task `autoMerge: true` override) — PR-based + * review flow owns lifecycle until human merge. */ async recoverStuckMergeDeadlocks(): Promise { try { const settings = await this.store.getSettings(); if (settings.globalPause || settings.enginePaused) return 0; - if (settings.autoMerge === false) return 0; - const now = Date.now(); const inReview = await this.store.listTasks({ column: "in-review", slim: true }); const triage = await this.store.listTasks({ column: "triage", slim: true }); @@ -6117,6 +6131,7 @@ export class SelfHealingManager { (dep) => dep.column === "triage" || dep.column === "todo", ); return task.column === "in-review" && + allowsAutoMergeProcessing(task, settings) && !task.paused && task.status === "failed" && (task.mergeRetries ?? 0) >= MAX_AUTO_MERGE_RETRIES && @@ -6278,18 +6293,19 @@ export class SelfHealingManager { } /** - * No-op when `settings.autoMerge === false` — PR-based review flow owns lifecycle until human merge. + * Skips tasks not eligible for auto-merge processing (global `autoMerge` + * off without an explicit per-task `autoMerge: true` override) — PR-based + * review flow owns lifecycle until human merge. */ async recoverOrphanOnlyScopeViolations(): Promise { try { const settings = await this.store.getSettings(); if (settings.globalPause || settings.enginePaused) return 0; - if (settings.autoMerge === false) return 0; - const executingIds = this.options.getExecutingTaskIds?.() ?? new Set(); const tasks = await this.store.listTasks({ column: "in-review", slim: true }); const candidates = tasks.filter((task) => task.column === "in-review" && + allowsAutoMergeProcessing(task, settings) && task.status === "failed" && task.scopeOverride !== true && task.mergeDetails?.mergeConfirmed !== true && @@ -6442,19 +6458,20 @@ export class SelfHealingManager { * * Idempotency: recovered tasks are moved to `done`, status/error are cleared, * and mergeRetries reset to 0, so subsequent sweeps will not match them. - * No-op when `settings.autoMerge === false` — PR-based review flow owns lifecycle until human merge. + * Skips tasks not eligible for auto-merge processing (global `autoMerge` + * off without an explicit per-task `autoMerge: true` override) — PR-based + * review flow owns lifecycle until human merge. */ async recoverAlreadyMergedReviewTasks(): Promise { try { const settings = await this.store.getSettings(); if (settings.globalPause || settings.enginePaused) return 0; - if (settings.autoMerge === false) return 0; - const executingIds = this.options.getExecutingTaskIds?.() ?? new Set(); const tasks = await this.store.listTasks({ column: "in-review", slim: true }); const candidates = tasks.filter((task) => !task.deletedAt && task.column === "in-review" && + allowsAutoMergeProcessing(task, settings) && task.status === "failed" && (task.mergeRetries ?? 0) >= MAX_AUTO_MERGE_RETRIES && task.mergeDetails?.mergeConfirmed !== true && @@ -6591,19 +6608,20 @@ export class SelfHealingManager { * Recover completed in-review tasks wedged as failed only because a post-done * session continuation hit a non-continuable signature. * - * No-op when `settings.autoMerge === false` — PR-based review flow owns lifecycle until human merge. + * Skips tasks not eligible for auto-merge processing (global `autoMerge` + * off without an explicit per-task `autoMerge: true` override) — PR-based + * review flow owns lifecycle until human merge. */ async recoverPostDoneNonContinuableWedge(): Promise { try { const settings = await this.store.getSettings(); if (settings.globalPause || settings.enginePaused) return 0; - if (settings.autoMerge === false) return 0; - const tasks = await this.store.listTasks({ column: "in-review", slim: false }); let recovered = 0; for (const task of tasks) { if (task.column !== "in-review" || task.deletedAt) continue; + if (!allowsAutoMergeProcessing(task, settings)) continue; if (task.paused || task.userPaused) continue; if (task.status !== "failed") continue; if (this.options.isTaskActive?.(task.id)) continue; @@ -6664,18 +6682,19 @@ export class SelfHealingManager { } /** - * No-op when `settings.autoMerge === false` — PR-based review flow owns lifecycle until human merge. + * Skips tasks not eligible for auto-merge processing (global `autoMerge` + * off without an explicit per-task `autoMerge: true` override) — PR-based + * review flow owns lifecycle until human merge. */ async recoverCompletionHandoffLimbo(): Promise { const settings = await this.store.getSettings(); if (settings.globalPause || settings.enginePaused) return; - if (settings.autoMerge === false) return; - const tasks = await this.store.listTasks({ column: "in-review", slim: false }); const now = Date.now(); for (const task of tasks) { if (task.column !== "in-review" || task.paused) continue; + if (!allowsAutoMergeProcessing(task, settings)) continue; if (task.status != null || task.mergeDetails != null || task.review != null || task.reviewState != null) continue; if (this.options.isTaskActive?.(task.id)) continue; if (getTaskMergeBlocker(task) !== undefined) continue; @@ -6889,20 +6908,21 @@ export class SelfHealingManager { } /** - * No-op when `settings.autoMerge === false` — PR-based review flow owns lifecycle until human merge. + * Skips tasks not eligible for auto-merge processing (global `autoMerge` + * off without an explicit per-task `autoMerge: true` override) — PR-based + * review flow owns lifecycle until human merge. */ async recoverForeignOnlyContaminatedInReviewTasks(): Promise { try { const settings = await this.store.getSettings(); if (settings.globalPause || settings.enginePaused) return 0; - if (settings.autoMerge === false) return 0; - const executingIds = this.options.getExecutingTaskIds?.() ?? new Set(); const inReview = await this.store.listTasks({ column: "in-review", slim: true }); const inProgress = await this.store.listTasks({ column: "in-progress", slim: true }); const candidates = [ ...inReview.filter((task) => task.column === "in-review" && + allowsAutoMergeProcessing(task, settings) && Boolean(task.branch) && Boolean(task.worktree) && task.mergeDetails?.mergeConfirmed !== true && @@ -6911,6 +6931,7 @@ export class SelfHealingManager { ), ...inProgress.filter((task) => task.column === "in-progress" && + allowsAutoMergeProcessing(task, settings) && task.paused === true && (task.pausedReason === "branch-cross-contamination" || task.pausedReason === "branch-conflict-unrecoverable") && Boolean(task.branch) && @@ -7730,18 +7751,19 @@ export class SelfHealingManager { * `restart-recovery-coordinator.ts`. * We clear stale worktree metadata and failure state, keep step progress and * retry counters, then requeue to todo for a clean retry. - * No-op when `settings.autoMerge === false` — PR-based review flow owns lifecycle until human merge. + * Skips tasks not eligible for auto-merge processing (global `autoMerge` + * off without an explicit per-task `autoMerge: true` override) — PR-based + * review flow owns lifecycle until human merge. */ async recoverMissingWorktreeReviewFailures(): Promise { try { const settings = await this.store.getSettings(); if (settings.globalPause || settings.enginePaused) return 0; - if (settings.autoMerge === false) return 0; - const tasks = await this.store.listTasks({ column: "in-review", slim: true }); const candidates = tasks.filter((task) => - isRecoverableMissingWorktreeReviewFailureWithProgress(task) - || isRecoverableMissingWorktreeReviewFailureNoProgress(task), + allowsAutoMergeProcessing(task, settings) + && (isRecoverableMissingWorktreeReviewFailureWithProgress(task) + || isRecoverableMissingWorktreeReviewFailureNoProgress(task)), ); if (candidates.length === 0) return 0; @@ -7813,19 +7835,20 @@ export class SelfHealingManager { * - `recoverNoProgressNoTaskDoneFailures`: `in-progress` with zero progress → clean requeue. * - This one: `in-review` with partial progress → bounded requeue preserving work. * - * No-op when `settings.autoMerge === false` — PR-based review flow owns lifecycle until human merge. + * Skips tasks not eligible for auto-merge processing (global `autoMerge` + * off without an explicit per-task `autoMerge: true` override) — PR-based + * review flow owns lifecycle until human merge. * @returns Number of tasks requeued for retry */ async recoverPartialProgressNoTaskDoneFailures(): Promise { try { const settings = await this.store.getSettings(); if (settings.globalPause || settings.enginePaused) return 0; - if (settings.autoMerge === false) return 0; - const tasks = await this.store.listTasks({ column: "in-review", slim: true }); const candidates = tasks.filter((task) => task.column === "in-review" && + allowsAutoMergeProcessing(task, settings) && task.status === "failed" && isNoTaskDoneFailure(task) && !task.paused && From ff1bb20b8f01a07347db5363e9ee0f3da81ec5ff Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Wed, 3 Jun 2026 10:55:26 -0700 Subject: [PATCH 2/3] docs: capture per-task auto-merge override learning and seed CONCEPTS.md Document the trigger-layer gating bug fixed in this PR under docs/solutions/logic-errors/, seed CONCEPTS.md with the merge-lifecycle vocabulary, and surface both knowledge stores in AGENTS.md's reference docs index. --- AGENTS.md | 2 + CONCEPTS.md | 32 +++++ ...merge-override-ignored-by-trigger-gates.md | 112 ++++++++++++++++++ 3 files changed, 146 insertions(+) create mode 100644 CONCEPTS.md create mode 100644 docs/solutions/logic-errors/per-task-auto-merge-override-ignored-by-trigger-gates.md diff --git a/AGENTS.md b/AGENTS.md index 266a5e1053..510c306842 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -178,6 +178,8 @@ Scoped exception (FN-5819): shared-branch-group members (`branchContext.assignme - `./docs/soft-delete-verification-matrix.md` — mandatory soft-delete verification matrix. - `./docs/cli-reference.md` — CLI and terminal UI reference. - `./docs/contributing.md` — contributing conventions and release-adjacent context. +- `./docs/solutions/` — documented solutions to past problems (bugs, patterns, conventions), organized by category with YAML frontmatter (`module`, `tags`, `problem_type`). Relevant when implementing or debugging in documented areas. +- `./CONCEPTS.md` — shared domain vocabulary (entities, named processes, status concepts). Relevant when orienting to the codebase or discussing domain concepts. ### Lazy-Loaded Heavy Views diff --git a/CONCEPTS.md b/CONCEPTS.md new file mode 100644 index 0000000000..9f015d0e39 --- /dev/null +++ b/CONCEPTS.md @@ -0,0 +1,32 @@ +# Concepts + +Shared domain vocabulary for this project — entities, named processes, and status concepts with project-specific meaning. Seeded with core domain vocabulary, then accretes as ce-compound and ce-compound-refresh process learnings; direct edits are fine. Glossary only, not a spec or catch-all. + +## Merge lifecycle + +### Task +The core board entity: a unit of work that moves through columns (triage, todo, in-progress, in-review, done, archived) and is executed by agents. A Task carries its own per-task settings that can override project-level defaults. + +### Auto-merge +The named process that automatically lands a completed Task's branch onto its merge target once the Task reaches In-review and passes its merge blockers. Gated twice: a project-level setting enables it globally, and each Task may carry an explicit per-task override. + +The per-task override takes precedence in both directions: an explicit per-task enable proceeds even when the global setting is off, and an explicit per-task disable routes the merge to Manual-required even when the global setting is on. Trigger-layer gates (enqueue, Self-healing sweeps) must evaluate additively — global on lets everything through for downstream routing; global off admits only explicit per-task enables — rather than collapsing the override to a single effective value, which would starve Manual-required routing. + +### In-review +The Task status column between execution and completion: work is done and the branch awaits merging. An In-review Task either auto-merges, waits for a human merge (PR-based/manual flow), or surfaces a stall diagnostic when it sits unprocessed longer than expected. Tasks not eligible for Auto-merge processing intentionally remain In-review until a human acts — recovery sweeps must not move them. + +### Merge queue +The ordered line of In-review Tasks awaiting Auto-merge, with a single merge active at a time. Tasks enter only through trigger gates (engine startup sweep, periodic retry, unpause, and the moved-to-review fast path); a Task filtered out at a gate is invisible to the merger regardless of its own settings. + +### Manual-required +The merge-request state for a Task whose merge needs an explicit human go-ahead — typically a Task with auto-merge explicitly disabled under a globally-enabled project. Reaching this state requires the Task to flow through the Merge queue trigger gates; upstream filtering that excludes such Tasks strands them In-review instead of parking them here. + +### Self-healing sweep +A recurring background scan that detects and repairs stuck Task states — stalled In-review Tasks, confirmed merges never finalized, ghost or limbo states, exhausted retries. Sweeps respect the same Auto-merge eligibility as the Merge queue: they may inspect any Task but mutate only those eligible for auto-merge processing. + +### Shared branch group +A set of Tasks integrating into a common shared branch instead of each merging straight to the project's default branch. Member integration (task branch → shared branch) is a soft pre-integration step exempt from the global auto-merge gate; promotion (shared branch → default branch) is gated separately. + +## Flagged ambiguities + +- "Merging" a shared-branch-group Task had been used for both member integration and group promotion — these are distinct steps with independent gating and must not be conflated. diff --git a/docs/solutions/logic-errors/per-task-auto-merge-override-ignored-by-trigger-gates.md b/docs/solutions/logic-errors/per-task-auto-merge-override-ignored-by-trigger-gates.md new file mode 100644 index 0000000000..926d0f86ba --- /dev/null +++ b/docs/solutions/logic-errors/per-task-auto-merge-override-ignored-by-trigger-gates.md @@ -0,0 +1,112 @@ +--- +title: Per-task auto-merge override ignored by trigger-layer gates +date: 2026-06-03 +category: logic-errors +module: engine +problem_type: logic_error +component: background_job +symptoms: + - "Tasks with per-task autoMerge:true never auto-merged when global settings.autoMerge was off" + - "Override tasks reached in-review and sat there indefinitely with no error surfaced" + - "In-review self-healing sweeps short-circuited on the global setting and never enqueued the merge" +root_cause: logic_error +resolution_type: code_fix +severity: high +related_components: + - merger + - self-healing + - store +tags: + - auto-merge + - per-task-override + - merge-queue + - self-healing + - engine + - trigger-gate +--- + +# Per-task auto-merge override ignored by trigger-layer gates + +## Problem + +A per-task `autoMerge: true` override was honored only by the merger itself, but every *trigger-layer* gate (engine enqueue, 19 self-healing sweeps, store stall-signal hydration) checked the global `settings.autoMerge` alone. With global auto-merge OFF, override tasks were never enqueued and sat in `in-review` forever. Fixed in PR Runfusion/Fusion#1356. + +## Symptoms + +- User disabled auto-merge globally but enabled it on individual tasks. +- Those individually-enabled tasks reached `in-review` and stayed there indefinitely — never picked up, never merged. +- No error surfaced: the tasks were simply never *triggered* into the merge pipeline, so the merger's per-task handling never ran. + +## What Didn't Work + +- **Assuming the downstream merger check was enough.** The only code consulting `task.autoMerge` was the merger (`packages/engine/src/merger.ts` ~7958: `task.autoMerge === false` → `manual-required`). That runs *after* enqueue. The enqueue gate `allowInReviewMergeProcessing` (`packages/engine/src/project-engine.ts:1386`) and 19 self-healing sweeps short-circuited on `settings.autoMerge` before the task ever reached the merger — so the per-task flag was dead code from the user's perspective. Notably, the feature issues (Runfusion/Fusion#1150, #1152, #1153) shipped the data model, a resolver (`resolveEffectiveAutoMerge`), and the dashboard control — #1152 even claimed engine merge-gating used the resolved value — but no trigger gate actually consulted it. +- **Reaching for `resolveEffectiveAutoMerge` at the gates.** The existing resolver `task.autoMerge ?? settings.autoMerge` (`packages/core/src/task-merge.ts`) looks like the natural gate, but using it would *regress* the global-ON + `autoMerge:false` case: those tasks must still flow into the merger so it can park them as `manual-required` (and so merged-task finalization sweeps still finalize them). Plain resolution would skip them at the trigger, stranding manually-merged tasks in `in-review`. +- **Slim-projection gotcha.** Per-task gating reads `task.autoMerge` off rows from slim task projections. If the `autoMerge` column were missing from `getTaskSelectClause` (`packages/core/src/store.ts` ~1976), the gate would silently see `undefined` and the override would fail with no error. (Verified present — but a real trap when adding per-row predicates.) + +## Solution + +New core predicate, **additive** to the global setting (`packages/core/src/task-merge.ts`): + +```ts +export function allowsAutoMergeProcessing( + task: Pick, + settings: Pick, +): boolean { + return settings.autoMerge !== false || task.autoMerge === true; +} +``` + +Applied at three trigger layers: + +1. **Enqueue gate** (`project-engine.ts:1386`), which fronts all four enqueue paths (startup sweep, periodic retry, unpause, task-moved fast path): + + ```ts + // before + private allowInReviewMergeProcessing(task: Pick, settings: Pick): boolean { + return settings.autoMerge || isSharedBranchGroupMemberIntegration(task); + } + // after + private allowInReviewMergeProcessing(task: Pick, settings: Pick): boolean { + return allowsAutoMergeProcessing(task, settings) || isSharedBranchGroupMemberIntegration(task); + } + ``` + +2. **All 19 self-healing sweeps** (`self-healing.ts`): the function-level early returns (`if (settings.autoMerge === false) return 0;`) were replaced by per-task filtering inside each sweep's candidate set, e.g.: + + ```ts + const candidates = tasks.filter((t) => + t.column === "in-review" && + allowsAutoMergeProcessing(t, settings) && + !t.paused && /* ... */); + ``` + +3. **Store stall-signal hydration** (`store.ts`, 6 sites): `autoMerge: settings.autoMerge` → `autoMerge: allowsAutoMergeProcessing(task, settings)` in the `getInReviewStallReason` / `getInReviewStalledSignal` contexts, so board diagnostics reflect that override tasks *are* being processed. + +The self-healing contract also changed: from "skip the whole sweep when global is off" to "list tasks, but mutate nothing without a per-task override." FN-5147 tests that asserted `listTasks` was never called were updated to assert the mutation-free guarantee instead. This extends — and stays consistent with — the AGENTS.md `autoMerge: false` callout (FN-5147): self-healing still never moves override-less `in-review` tasks when auto-merge is off. + +## Why This Works + +The root cause was a flag consulted only where the *action* runs, not where processing is *triggered*. Adding the override evaluation to every trigger gate closes the gap. + +Additive (`settings.autoMerge !== false || task.autoMerge === true`) is deliberately chosen over resolution (`task.autoMerge ?? settings.autoMerge`): + +- **Global ON:** `settings.autoMerge !== false` is already `true`, so the predicate is a no-op — every task flows through exactly as before, including `autoMerge:false` tasks that the merger then parks as `manual-required`. Resolution would have excluded those, breaking manual-required parking and finalization. +- **Global OFF:** the first term is `false`, so only `task.autoMerge === true` tasks proceed — exactly the missing override path. + +It changes nothing when global is ON and adds only the explicit-true path when global is OFF. + +## Prevention + +When adding a per-entity override to a behavior that's gated on a global setting, the override must be consulted **where the behavior is TRIGGERED, not just where the action runs.** A check at the merger (the action) is invisible if upstream enqueue/sweep gates already filtered the entity out. + +- **Grep every gate on the global setting** before declaring the override wired: here `settings.autoMerge` appeared at 1 enqueue gate, 19 sweep guards, and 6 hydration sites — all needed updating. A search for the global key, not just the new override field, surfaces the dead-flag sites. +- **Prefer additive gating over effective-value resolution for *processing* gates.** Resolution collapses three states (global-on/off × per-task true/false/unset) into one boolean and can starve a needed downstream branch (the manual-required parking path). Gate on "should this be processed at all," resolve the actual behavior later. +- **Watch slim projections:** per-row predicates require the override column in the SELECT clause, or they silently read `undefined`. +- **Test matrix must cross global × per-task.** The fix shipped red-first unit tests for the predicate (`packages/core/src/__tests__/task-merge.test.ts`), the gate including the shared-group exemption (`packages/engine/src/__tests__/project-engine.test.ts`), and a self-healing test proving an **override task is processed while an override-less sibling stays skipped** (`packages/engine/src/__tests__/self-healing.test.ts`) — the latter is the canonical shape: two tasks differing only in `autoMerge` under global-OFF, asserting divergent outcomes. + +## Related Issues + +- Runfusion/Fusion#1356 — the fix PR (commit `ad468813d`) +- Runfusion/Fusion#1150, Runfusion/Fusion#1152, Runfusion/Fusion#1153 — the per-task auto-merge feature trio (data model + resolver, engine gating, dashboard control); #1152's gating claim is the gap this bug exposed +- Runfusion/Fusion#753 (FN-5147), Runfusion/Fusion#690 (FN-5052) — prior global `autoMerge:false` stall/lifecycle handling that the sweeps' guards came from +- AGENTS.md → "`autoMerge: false` callout (FN-5147)" — standing lifecycle rule this fix extends to per-task granularity From e00bc0235b6c07b4b23fa2bf57e392d126c3fe22 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Wed, 3 Jun 2026 13:08:06 -0700 Subject: [PATCH 3/3] Address PR review feedback (#1356) - Add behavior-level tests for the shared merge-enqueue funnel (enqueueEligibleInReviewTasks) with a Surface Enumeration of all in-review entry surfaces, per review - Seed real stale in-review fixtures in the FN-5147 no-mutation regression block so sweeps enumerate candidates and the assertions are non-vacuous - Keep per-task auto-merge gating uniform across reclaim/contamination candidate columns: the suggested in-review-only scoping broke the FN-5704 regression contract (reclaim short-circuits when autoMerge is off); documented the tension in code comments and the learning doc - Drop hardcoded commit hash from the learning doc --- ...merge-override-ignored-by-trigger-gates.md | 3 +- .../src/__tests__/project-engine.test.ts | 68 ++++++++ .../engine/src/__tests__/self-healing.test.ts | 160 +++++++++++++++++- packages/engine/src/self-healing.ts | 10 ++ 4 files changed, 237 insertions(+), 4 deletions(-) diff --git a/docs/solutions/logic-errors/per-task-auto-merge-override-ignored-by-trigger-gates.md b/docs/solutions/logic-errors/per-task-auto-merge-override-ignored-by-trigger-gates.md index 926d0f86ba..4aeb77af47 100644 --- a/docs/solutions/logic-errors/per-task-auto-merge-override-ignored-by-trigger-gates.md +++ b/docs/solutions/logic-errors/per-task-auto-merge-override-ignored-by-trigger-gates.md @@ -101,12 +101,13 @@ When adding a per-entity override to a behavior that's gated on a global setting - **Grep every gate on the global setting** before declaring the override wired: here `settings.autoMerge` appeared at 1 enqueue gate, 19 sweep guards, and 6 hydration sites — all needed updating. A search for the global key, not just the new override field, surfaces the dead-flag sites. - **Prefer additive gating over effective-value resolution for *processing* gates.** Resolution collapses three states (global-on/off × per-task true/false/unset) into one boolean and can starve a needed downstream branch (the manual-required parking path). Gate on "should this be processed at all," resolve the actual behavior later. +- **Check existing regression contracts before re-scoping a gate.** Review of the fix PR suggested exempting `todo`/`in-progress` candidates (execution-stage repair) from the auto-merge gate — but the repo's FN-5704 regression test ("short-circuits reclaim when autoMerge is false") deliberately keeps execution-stage reclaim inert in manual-review projects. Per-task gating applied uniformly preserves that contract while enabling overrides; exempting execution-stage recovery would be a separate, deliberate behavior change. - **Watch slim projections:** per-row predicates require the override column in the SELECT clause, or they silently read `undefined`. - **Test matrix must cross global × per-task.** The fix shipped red-first unit tests for the predicate (`packages/core/src/__tests__/task-merge.test.ts`), the gate including the shared-group exemption (`packages/engine/src/__tests__/project-engine.test.ts`), and a self-healing test proving an **override task is processed while an override-less sibling stays skipped** (`packages/engine/src/__tests__/self-healing.test.ts`) — the latter is the canonical shape: two tasks differing only in `autoMerge` under global-OFF, asserting divergent outcomes. ## Related Issues -- Runfusion/Fusion#1356 — the fix PR (commit `ad468813d`) +- Runfusion/Fusion#1356 — the fix PR - Runfusion/Fusion#1150, Runfusion/Fusion#1152, Runfusion/Fusion#1153 — the per-task auto-merge feature trio (data model + resolver, engine gating, dashboard control); #1152's gating claim is the gap this bug exposed - Runfusion/Fusion#753 (FN-5147), Runfusion/Fusion#690 (FN-5052) — prior global `autoMerge:false` stall/lifecycle handling that the sweeps' guards came from - AGENTS.md → "`autoMerge: false` callout (FN-5147)" — standing lifecycle rule this fix extends to per-task granularity diff --git a/packages/engine/src/__tests__/project-engine.test.ts b/packages/engine/src/__tests__/project-engine.test.ts index 4c46d088ac..849875483a 100644 --- a/packages/engine/src/__tests__/project-engine.test.ts +++ b/packages/engine/src/__tests__/project-engine.test.ts @@ -2696,3 +2696,71 @@ describe("allowInReviewMergeProcessing per-task autoMerge override", () => { )).toBe(true); }); }); + +// ## Surface Enumeration +// +// Known in-review merge entry surfaces in ProjectEngine, and how each enforces +// the per-task `autoMerge` override invariant (a task with `autoMerge:true` must +// still be enqueued for merge even when the global `autoMerge` setting is off): +// +// 1. Startup merge sweep (project-engine.ts ~:2857) ─┐ +// 2. Periodic merge retry sweep (project-engine.ts ~:2916) ─┼─ all call +// 3. Resume-after-unpause sweep (project-engine.ts ~:2977) ─┘ enqueueEligibleInReviewTasks(...) +// 4. task:moved fast path (project-engine.ts ~:1506) ─── inline allowInReviewMergeProcessing(...) +// +// Surfaces 1–3 funnel through `enqueueEligibleInReviewTasks`, whose filter is +// `!t.paused && canMergeTask(t) && allowInReviewMergeProcessing(t, settings)`. +// The behavior tests below exercise that shared funnel directly on a real engine +// instance (with `internalEnqueueMerge` stubbed), so a regression in any of the +// three sweep wrappers (wireAutoMerge / startupMergeSweep / scheduleMergeRetry / +// resumeAfterUnpauseAndSweepInReview) that still routes through the funnel is +// caught. Surface 4 (the task:moved fast path) shares the same +// `allowInReviewMergeProcessing` gate, which is covered by the direct helper +// tests above. + +describe("enqueueEligibleInReviewTasks honors per-task autoMerge override (shared sweep funnel)", () => { + const inReview = (id: string, overrides: Partial = {}): Task => + ({ + id, + column: "in-review", + paused: false, + mergeRetries: 0, + status: null, + ...overrides, + }) as unknown as Task; + + const setup = () => { + const engine = createEngine() as any; + const enqueueSpy = vi + .spyOn(engine, "internalEnqueueMerge") + .mockImplementation(() => true); + const run = (tasks: Task[], settings: { autoMerge: boolean }): number => + engine.enqueueEligibleInReviewTasks(tasks, settings) as number; + return { engine, enqueueSpy, run }; + }; + + it("enqueues an in-review task with autoMerge:true even when the global setting is off", () => { + const { enqueueSpy, run } = setup(); + const count = run([inReview("FN-override", { autoMerge: true })], { autoMerge: false }); + expect(count).toBe(1); + expect(enqueueSpy).toHaveBeenCalledWith("FN-override"); + }); + + it("does not enqueue a sibling task without an override in the same sweep when the global setting is off", () => { + const { enqueueSpy, run } = setup(); + const count = run( + [inReview("FN-override", { autoMerge: true }), inReview("FN-plain")], + { autoMerge: false }, + ); + expect(count).toBe(1); + expect(enqueueSpy).toHaveBeenCalledWith("FN-override"); + expect(enqueueSpy).not.toHaveBeenCalledWith("FN-plain"); + }); + + it("still enqueues a task with autoMerge:false when the global setting is on (parked manual-required downstream)", () => { + const { enqueueSpy, run } = setup(); + const count = run([inReview("FN-explicit-false", { autoMerge: false })], { autoMerge: true }); + expect(count).toBe(1); + expect(enqueueSpy).toHaveBeenCalledWith("FN-explicit-false"); + }); +}); diff --git a/packages/engine/src/__tests__/self-healing.test.ts b/packages/engine/src/__tests__/self-healing.test.ts index 87db2fbb2f..1b37da510f 100644 --- a/packages/engine/src/__tests__/self-healing.test.ts +++ b/packages/engine/src/__tests__/self-healing.test.ts @@ -97,7 +97,7 @@ vi.mock("../merger.js", () => ({ classifyOwnedLandedEvidence: vi.fn(), })); -import { SelfHealingManager, isBranchAheadOfBase } from "../self-healing.js"; +import { SelfHealingManager, isBranchAheadOfBase, MAX_AUTO_MERGE_RETRIES } from "../self-healing.js"; import type { TaskStore, Settings, Task, AgentStore, Agent, NotificationProvider } from "@fusion/core"; import { EventEmitter } from "node:events"; import { execSync } from "node:child_process"; @@ -8212,6 +8212,153 @@ describe("autoMerge gating for mutating in-review sweeps (FN-5147)", () => { taskStuckTimeoutMs: 1_000, maxPostReviewFixes: 1, }); + + // Seed real, stale in-review sweep candidates with NO per-task autoMerge + // override. Each fixture matches a distinct covered sweep's candidate shape + // and would be mutated if the per-task gate (allowsAutoMergeProcessing) were + // ignored. Because the global setting is autoMerge:false and none of these + // carry autoMerge:true, every sweep must enumerate them and skip them solely + // due to the gate — which is the regression under test. The gate is the + // first/early filter in each sweep, so candidates are dropped before any + // store.getTask / git helper is reached. + const stale = new Date(Date.now() - 600_000).toISOString(); + const seededInReviewCandidates = [ + // recoverStaleIncompleteReviewTasks + recoverGhostReviewTasks: + // idle in-review with incomplete steps, stale. + { + id: "FN-GATE-INCOMPLETE", + column: "in-review", + paused: false, + steps: [{ status: "pending" }], + log: [], + updatedAt: stale, + columnMovedAt: stale, + }, + // recoverInterruptedMergingTasks: stale `merging` status. + { + id: "FN-GATE-MERGING", + column: "in-review", + paused: false, + status: "merging", + steps: [], + log: [], + updatedAt: stale, + columnMovedAt: stale, + }, + // recoverMergedReviewTasks + recoverGhostReviewTasks(skip merge-confirmed): + // mergeConfirmed:true stuck in in-review. + { + id: "FN-GATE-MERGED", + column: "in-review", + paused: false, + steps: [], + log: [], + mergeDetails: { mergeConfirmed: true }, + updatedAt: stale, + columnMovedAt: stale, + }, + // recoverStuckMergeDeadlocks + recoverAlreadyMergedReviewTasks + + // recoverOrphanOnlyScopeViolations: failed in-review, retries exhausted, + // worktree present. + { + id: "FN-GATE-FAILED", + column: "in-review", + paused: false, + status: "failed", + steps: [], + log: [], + mergeRetries: MAX_AUTO_MERGE_RETRIES, + worktree: "/tmp/test-project/.worktrees/FN-GATE-FAILED", + branch: "fn/FN-GATE-FAILED", + updatedAt: stale, + columnMovedAt: stale, + }, + // recoverReviewTasksWithFailedPreMergeSteps: idle in-review whose merge is + // blocked specifically by a failed pre-merge workflow step, worktree set. + { + id: "FN-GATE-PREMERGE", + column: "in-review", + paused: false, + steps: [], + log: [], + worktree: "/tmp/test-project/.worktrees/FN-GATE-PREMERGE", + workflowStepResults: [{ phase: "pre-merge", status: "failed" }], + updatedAt: stale, + columnMovedAt: stale, + }, + // recoverMissingWorktreeReviewFailures: failed by missing-worktree session + // start, with step progress. + { + id: "FN-GATE-MISSINGWT", + column: "in-review", + paused: false, + status: "failed", + error: "Refusing to start coding agent in missing worktree: /tmp/gone", + steps: [{ status: "done" }], + log: [], + updatedAt: stale, + columnMovedAt: stale, + }, + // recoverPartialProgressNoTaskDoneFailures: failed without fn_task_done, + // partial step progress, not work-complete, retries available. + { + id: "FN-GATE-NOTASKDONE", + column: "in-review", + paused: false, + status: "failed", + error: "Agent finished without calling fn_task_done", + steps: [{ status: "done" }, { status: "pending" }], + log: [], + updatedAt: stale, + columnMovedAt: stale, + }, + // recoverForeignOnlyContaminatedInReviewTasks: in-review with branch + + // worktree, not merge-confirmed. + { + id: "FN-GATE-FOREIGN", + column: "in-review", + paused: false, + branch: "fn/FN-GATE-FOREIGN", + worktree: "/tmp/test-project/.worktrees/FN-GATE-FOREIGN", + steps: [], + log: [], + updatedAt: stale, + columnMovedAt: stale, + }, + // recoverCompletionHandoffLimbo: idle in-review with no status/mergeDetails/ + // review, an aged "Task marked done by agent" log marker, no merge blocker. + { + id: "FN-GATE-HANDOFF", + column: "in-review", + paused: false, + steps: [], + log: [{ action: "Task marked done by agent", timestamp: stale }], + updatedAt: stale, + columnMovedAt: stale, + }, + // reclaimSelfOwnedBranchConflicts: in-review branch-conflict-unrecoverable. + // (No worktree, so even absent the gate it is skipped before any git call; + // the gate is what the assertions verify.) + { + id: "FN-GATE-RECLAIM", + column: "in-review", + paused: true, + pausedReason: "branch-conflict-unrecoverable", + branch: "fn/FN-GATE-RECLAIM", + steps: [], + log: [], + updatedAt: stale, + columnMovedAt: stale, + }, + ] as unknown as Task[]; + + // Resolve fixtures only for the in-review column the sweeps enumerate; other + // columns (todo / in-progress / triage) stay empty so the non-auto-merge- + // gated branches of reclaim/foreign-only sweeps don't reach git helpers. + (store.listTasks as ReturnType).mockImplementation( + async (opts?: { column?: string }) => + opts?.column === "in-review" ? seededInReviewCandidates : [], + ); }); afterEach(() => { @@ -8237,8 +8384,11 @@ describe("autoMerge gating for mutating in-review sweeps (FN-5147)", () => { } const result = await (manager as any)[methodName](); expect(result).toBe(0); - // The sweep may list tasks to discover per-task autoMerge overrides, - // but must not mutate anything without one (respects PR-based review flow). + // Enumeration must have happened: the sweep listed real, stale in-review + // candidates seeded above. Mutations are skipped solely because of the + // per-task auto-merge gate (respects PR-based review flow) — so these + // assertions are non-vacuous. + expect(store.listTasks).toHaveBeenCalled(); expect(store.moveTask).not.toHaveBeenCalled(); expect(store.updateTask).not.toHaveBeenCalled(); expect(store.logEntry).not.toHaveBeenCalled(); @@ -8247,6 +8397,10 @@ describe("autoMerge gating for mutating in-review sweeps (FN-5147)", () => { it("performs no mutations when autoMerge is disabled and no per-task override exists: recoverCompletionHandoffLimbo", async () => { const result = await manager.recoverCompletionHandoffLimbo(); expect(result).toBeUndefined(); + // The seeded FN-GATE-HANDOFF candidate carries an aged "Task marked done by + // agent" marker and no merge blocker, so the sweep enumerates it and would + // requeue/fail it absent the per-task gate. + expect(store.listTasks).toHaveBeenCalled(); expect(store.moveTask).not.toHaveBeenCalled(); expect(store.updateTask).not.toHaveBeenCalled(); expect(store.logEntry).not.toHaveBeenCalled(); diff --git a/packages/engine/src/self-healing.ts b/packages/engine/src/self-healing.ts index 929dbaea75..2c60cd7c81 100644 --- a/packages/engine/src/self-healing.ts +++ b/packages/engine/src/self-healing.ts @@ -2320,6 +2320,12 @@ export class SelfHealingManager { } const inReviewPausedCandidates = (await this.store.listTasks({ column: "in-review", slim: true })) .filter((task) => task.paused === true && task.pausedReason === "branch-conflict-unrecoverable"); + // Per-task auto-merge gating applies to ALL candidate columns, not just + // in-review: the FN-5704 regression contract ("short-circuits reclaim + // when autoMerge is false") deliberately keeps execution-stage reclaim + // and resume-limbo escalation inert in manual-review projects. The + // per-task override preserves that for override-less tasks while letting + // explicit autoMerge:true tasks recover. const candidates = [...todoCandidates, ...inProgressCandidates, ...inReviewPausedCandidates] .filter((task) => allowsAutoMergeProcessing(task, settings)); @@ -6929,6 +6935,10 @@ export class SelfHealingManager { !task.userPaused && !executingIds.has(task.id), ), + // The paused in-progress contamination branch is gated per-task too: + // pre-existing behavior kept this sweep fully inert in manual-review + // projects (mirroring the FN-5704 reclaim contract), so override-less + // tasks stay untouched while explicit autoMerge:true tasks recover. ...inProgress.filter((task) => task.column === "in-progress" && allowsAutoMergeProcessing(task, settings) &&