From 41d415fe705943c1b9ce1ce858e2ac9b4d7c0d06 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 14 Aug 2026 21:51:45 -0700 Subject: [PATCH] FN-9051: report blocked workspace finalization Prevent workspace merge entry points from treating an unfinalized task as a successful merge. - Add a typed blocked-finalization result and engine handling that avoids retries, failure parking, and branch promotion. - Return blocked outcomes through dashboard and CLI merge surfaces. - Cover blocked finalization behavior and publish a patch changeset. Files changed: .changeset/fn-9051-workspace-blocked-finalize.md | 7 ++ packages/cli/src/commands/__tests__/task.test.ts | 38 ++++++++ packages/cli/src/commands/dashboard.ts | 22 +++-- packages/cli/src/commands/task.ts | 19 ++-- .../engine/src/__tests__/project-engine.test.ts | 105 ++++++++++++++++++++- .../engine/src/__tests__/workspace-merger.test.ts | 1 + packages/engine/src/index.ts | 1 + packages/engine/src/merge/merger-ai.ts | 28 +++++- packages/engine/src/project-engine.ts | 42 ++++++++- 9 files changed, 240 insertions(+), 23 deletions(-) Fusion-Task-Id: FN-9051 Fusion-Task-Lineage: 89d5fcbb-1a00-4429-87ea-fc3e34db2706 Co-authored-by: Fusion (runfusion.ai) --- .../fn-9051-workspace-blocked-finalize.md | 7 ++ .../cli/src/commands/__tests__/task.test.ts | 38 +++++++ packages/cli/src/commands/dashboard.ts | 22 ++-- packages/cli/src/commands/task.ts | 19 +++- .../src/__tests__/project-engine.test.ts | 105 +++++++++++++++++- .../src/__tests__/workspace-merger.test.ts | 1 + packages/engine/src/index.ts | 1 + packages/engine/src/merge/merger-ai.ts | 28 ++++- packages/engine/src/project-engine.ts | 42 ++++++- 9 files changed, 240 insertions(+), 23 deletions(-) create mode 100644 .changeset/fn-9051-workspace-blocked-finalize.md diff --git a/.changeset/fn-9051-workspace-blocked-finalize.md b/.changeset/fn-9051-workspace-blocked-finalize.md new file mode 100644 index 0000000000..d86c82e521 --- /dev/null +++ b/.changeset/fn-9051-workspace-blocked-finalize.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Workspace merges no longer report success when finalization is blocked. +category: fix +dev: Adds WorkspaceFinalizeBlockedError and requires finalized/finalizeBlockedReason for workspace merge success. diff --git a/packages/cli/src/commands/__tests__/task.test.ts b/packages/cli/src/commands/__tests__/task.test.ts index ae4002b6ec..a67fdd53ae 100644 --- a/packages/cli/src/commands/__tests__/task.test.ts +++ b/packages/cli/src/commands/__tests__/task.test.ts @@ -1410,6 +1410,44 @@ describe("project-aware task command behavior", () => { expect(refineTask).toHaveBeenCalledWith("FN-123", "more tests"); }); + it("exits non-zero when a workspace finalize is blocked after all repos landed", async () => { + const getTask = vi.fn().mockResolvedValue(makeTask({ + id: "FN-WS-BLOCKED", + column: "in-review", + workspaceWorktrees: { "repo-a": { worktreePath: "/tmp/a", branch: "fusion/fn-ws-blocked" } }, + })); + const resolvedStore = { getTask } as unknown as TaskStore; + vi.mocked(resolveProject).mockResolvedValue({ + projectId: "proj_test", + projectPath: "/test", + projectName: "demo-project", + isRegistered: true, + store: resolvedStore, + }); + vi.mocked(landWorkspaceTask).mockResolvedValue({ + allLanded: true, + finalized: false, + finalizeBlockedReason: "operator review required", + repos: [{ repo: "repo-a", status: "empty", integrationBranch: "main" }], + } as never); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => undefined); + const exitSpy = vi.spyOn(process, "exit").mockImplementation((((code?: number) => { + throw new Error(`process.exit:${code}`); + }) as unknown) as (code?: string | number | null | undefined) => never); + + let output = ""; + try { + await expect(runTaskMerge("FN-WS-BLOCKED", "demo-project")).rejects.toThrow("process.exit:1"); + output = logSpy.mock.calls.flat().join(" "); + } finally { + exitSpy.mockRestore(); + logSpy.mockRestore(); + } + + expect(output).toContain("Merge blocked — operator review required"); + expect(output).not.toContain("task finalized to done"); + }); + it("routes GitHub import commands through the resolved project store", async () => { const listTasks = vi.fn().mockResolvedValue([]); const createTask = vi.fn().mockResolvedValue(makeTask({ id: "FN-200" })); diff --git a/packages/cli/src/commands/dashboard.ts b/packages/cli/src/commands/dashboard.ts index 199d04e7f4..bb2216195f 100644 --- a/packages/cli/src/commands/dashboard.ts +++ b/packages/cli/src/commands/dashboard.ts @@ -1635,22 +1635,24 @@ export async function runDashboard(port: number, opts: { paused?: boolean; dev?: pluginRunner: undefined, }); const latest = await store.getTask(taskId).catch(() => mergeTask!); - // FNXC:Workspace 2026-06-22-05:10 (Phase C review B3): - // landWorkspaceTask now finalizes the workspace task to done on allLanded (Phase C U2), - // so the merge door must report merged=true when the workspace fully landed — mirroring - // the engine dispatch's MergeResult. The first landed sub-repo's landedSha is the recorded - // commitSha (same convention finalizeWorkspaceTask uses). On a partial land, merged stays - // false and the partial-land error surfaces on the task log. + /* + FNXC:Workspace 2026-08-15-04:22: + `finalized`, not `allLanded`, is the UI-only merge signal. A blocked finalize is already + parked, so never return merged/confirmed metadata for it; expose its reason instead. + */ const landedSha = workspaceResult.repos.find((r) => r.status === "landed")?.landedSha; + const workspaceMerged = workspaceResult.allLanded && workspaceResult.finalized; return { task: latest ?? mergeTask!, branch: getTaskBranchName(taskId), - merged: workspaceResult.allLanded, - mergeConfirmed: workspaceResult.allLanded || undefined, - commitSha: workspaceResult.allLanded ? landedSha : undefined, + merged: workspaceMerged, + mergeConfirmed: workspaceMerged || undefined, + commitSha: workspaceMerged ? landedSha : undefined, worktreeRemoved: false, branchDeleted: false, - error: workspaceResult.allLanded ? undefined : "partial workspace land — see task log", + error: workspaceMerged + ? undefined + : workspaceResult.finalizeBlockedReason ?? "partial workspace land — see task log", }; } diff --git a/packages/cli/src/commands/task.ts b/packages/cli/src/commands/task.ts index 91dae0a8b7..c9307c60b0 100644 --- a/packages/cli/src/commands/task.ts +++ b/packages/cli/src/commands/task.ts @@ -1256,14 +1256,21 @@ export async function runTaskMerge(id: string, projectName?: string) { : `failed: ${repo.error ?? "unknown"}`; console.log(` ${repo.status === "failed" ? "✗" : "✓"} ${repo.repo}: ${label}`); } - // FNXC:Workspace 2026-06-22-05:10 (Phase C review B3): - // landWorkspaceTask now finalizes the workspace task to done on allLanded (Phase C U2), - // so report it as merged rather than "remains in review until U2". A partial land leaves - // the task in review (landed repos stay landed locally) and exits non-zero. + /* + FNXC:Workspace 2026-08-15-04:22: + `finalized`, not `allLanded`, is the merged signal. A blocked finalize is already parked + with progress preserved, so the CLI must report it as blocked and exit non-zero rather than + claiming success for sub-repos that landed without the task reaching `done`. + */ + const workspaceMerged = workspaceResult.allLanded && workspaceResult.finalized; console.log( - `\n ${workspaceResult.allLanded ? "✓ All sub-repos landed — task finalized to done" : "✗ Partial land — see failures above (task remains in review; landed repos stay landed locally)"}\n`, + `\n ${workspaceMerged + ? "✓ All sub-repos landed — task finalized to done" + : workspaceResult.allLanded + ? `✗ Merge blocked — ${workspaceResult.finalizeBlockedReason ?? "workspace finalize was blocked"} (task moved back with progress preserved)` + : "✗ Partial land — see failures above (task remains in review; landed repos stay landed locally)"}\n`, ); - if (!workspaceResult.allLanded) await closeBoardContextAndExit(context, 1); + if (!workspaceMerged) await closeBoardContextAndExit(context, 1); return; } diff --git a/packages/engine/src/__tests__/project-engine.test.ts b/packages/engine/src/__tests__/project-engine.test.ts index 2ca3fe9e24..dc74ebb79b 100644 --- a/packages/engine/src/__tests__/project-engine.test.ts +++ b/packages/engine/src/__tests__/project-engine.test.ts @@ -10,7 +10,11 @@ import { ProjectEngine, __resetDeterministicMergerModeDeprecationWarned } from " import { AgentSemaphore, projectAdmissionCoordinator} from "../concurrency/concurrency.js"; // Resolves to the vi.mock factory above (the mocked merger-ai exports the real-shaped // workspace land error classes so the dispatch's `instanceof` matching is exercised). -import { WorkspacePartialLandError, WorkspaceRepoLandBusyError } from "../merge/merger-ai.js"; +import { + WorkspaceFinalizeBlockedError, + WorkspacePartialLandError, + WorkspaceRepoLandBusyError, +} from "../merge/merger-ai.js"; import { runtimeLog } from "../logger.js"; import { TunnelProcessManager } from "../remote-access/tunnel-process-manager.js"; import { NtfyNotifier } from "../util/notifier.js"; @@ -122,11 +126,21 @@ vi.mock("../merge/merger-ai.js", () => { this.name = "WorkspacePartialLandError"; } } + class WorkspaceFinalizeBlockedError extends Error { + constructor( + public readonly taskId: string, + public readonly reason: string, + ) { + super(`Workspace finalize blocked for ${taskId}: ${reason}`); + this.name = "WorkspaceFinalizeBlockedError"; + } + } return { runAiMerge: mocks.runAiMerge, landWorkspaceTask: mocks.landWorkspaceTask, WorkspaceRepoLandBusyError, WorkspacePartialLandError, + WorkspaceFinalizeBlockedError, }; }); @@ -1496,6 +1510,7 @@ describe("ProjectEngine U0 merge unification dispatch", () => { mocks.currentStore = mockStore.store; mocks.landWorkspaceTask.mockResolvedValue({ allLanded: true, + finalized: true, repos: [ { repo: "repo-a", status: "landed", landedSha: "aaaa1111", integrationBranch: "main" }, { repo: "repo-b", status: "landed", landedSha: "bbbb2222", integrationBranch: "main" }, @@ -1538,6 +1553,7 @@ describe("ProjectEngine U0 merge unification dispatch", () => { mocks.currentStore = mockStore.store; mocks.landWorkspaceTask.mockResolvedValue({ allLanded: true, + finalized: true, repos: [ { repo: "repo-a", status: "landed", landedSha: "aaaa1111", integrationBranch: "main" }, { repo: "repo-b", status: "landed", landedSha: "bbbb2222", integrationBranch: "main" }, @@ -1570,6 +1586,7 @@ describe("ProjectEngine U0 merge unification dispatch", () => { mocks.currentStore = mockStore.store; mocks.landWorkspaceTask.mockResolvedValue({ allLanded: true, + finalized: true, repos: [{ repo: "repo-c", status: "landed", landedSha: "cccc3333", integrationBranch: "main" }], } as any); @@ -1602,6 +1619,7 @@ describe("ProjectEngine U0 merge unification dispatch", () => { // reports allLanded:true and finalizes gracefully. mocks.landWorkspaceTask.mockResolvedValue({ allLanded: true, + finalized: true, repos: [{ repo: "repo-d", status: "empty", integrationBranch: "main" }], } as any); @@ -1679,6 +1697,91 @@ describe("ProjectEngine workspace merge dispatch hardening (Phase C review)", () ...overrides, }); + it("rejects a manual workspace merge when finalization is blocked without laundering success", async () => { + const mockStore = createMockStore({ ...baseSettings, autoMerge: true }); + const task = workspaceTask({ + branchContext: { assignmentMode: "shared", groupId: "BG-9051", source: "planning" }, + }); + mockStore.store.getTask.mockResolvedValue(task as any); + mocks.currentStore = mockStore.store; + mocks.landWorkspaceTask.mockResolvedValue({ + allLanded: true, + finalized: false, + finalizeBlockedReason: "operator review required", + repos: [{ repo: "repo-a", status: "empty", integrationBranch: "main" }], + } as any); + const mergedLogSpy = vi.spyOn(runtimeLog, "log").mockImplementation(() => undefined); + const engine = createEngine({ createGroupPr: vi.fn() }); + await engine.start(); + + await expect(engine.onMerge("FN-WSH")).rejects.toMatchObject({ + name: "WorkspaceFinalizeBlockedError", + reason: "operator review required", + }); + + expect(mockStore.store.updateTask.mock.calls.some(([, patch]) => + typeof (patch as { mergeRetries?: unknown }).mergeRetries === "number", + )).toBe(false); + expect(mockStore.store.updateTask).not.toHaveBeenCalledWith( + "FN-WSH", + expect.objectContaining({ status: "failed" }), + ); + expect(mockStore.store.getBranchGroup).not.toHaveBeenCalled(); + expect(mergedLogSpy.mock.calls.flat().join(" ")).not.toContain("merge merged: FN-WSH"); + + mergedLogSpy.mockRestore(); + await engine.stop(); + }); + + it("keeps a blocked workspace finalize out of the auto retry and failure paths", async () => { + const mockStore = createMockStore({ ...baseSettings, autoMerge: true }); + mockStore.store.getTask.mockResolvedValue(workspaceTask() as any); + mocks.currentStore = mockStore.store; + mocks.landWorkspaceTask.mockResolvedValue({ + allLanded: true, + finalized: false, + finalizeBlockedReason: "operator review required", + repos: [{ repo: "repo-a", status: "empty", integrationBranch: "main" }], + } as any); + const engine = createEngine(); + await engine.start(); + engine.enqueueMerge("FN-WSH"); + + await vi.waitFor(() => { + expect(mockStore.store.logEntry).toHaveBeenCalledWith( + "FN-WSH", + expect.stringContaining("operator review required"), + "WorkspaceFinalizeBlocked", + ); + }); + + expect(mockStore.store.updateTask.mock.calls.some(([, patch]) => + typeof (patch as { mergeRetries?: unknown }).mergeRetries === "number" + || (patch as { status?: unknown }).status === "failed", + )).toBe(false); + await engine.stop(); + }); + + it("uses the generic blocked reason when the workspace finalize fence is orphaned", async () => { + const mockStore = createMockStore({ ...baseSettings, autoMerge: true }); + mockStore.store.getTask.mockResolvedValue(workspaceTask() as any); + mocks.currentStore = mockStore.store; + mocks.landWorkspaceTask.mockResolvedValue({ + allLanded: true, + finalized: false, + repos: [{ repo: "repo-a", status: "empty", integrationBranch: "main" }], + } as any); + const engine = createEngine(); + await engine.start(); + + await expect(engine.onMerge("FN-WSH")).rejects.toMatchObject({ + name: "WorkspaceFinalizeBlockedError", + reason: expect.stringContaining("workspace finalize was blocked"), + }); + + await engine.stop(); + }); + // B1: getTask returning null in the partial-land catch must FAIL CLOSED — no retry timer. it("B1: partial land with getTask null fails closed (parks failed, no retry timer)", async () => { vi.useFakeTimers(); diff --git a/packages/engine/src/__tests__/workspace-merger.test.ts b/packages/engine/src/__tests__/workspace-merger.test.ts index e7f0de4bf4..fd6da8483c 100644 --- a/packages/engine/src/__tests__/workspace-merger.test.ts +++ b/packages/engine/src/__tests__/workspace-merger.test.ts @@ -321,6 +321,7 @@ describeIfGit("landWorkspaceTask — per-repo merge loop (Phase C U1)", () => { // No sub-repo FAILED, but nothing landed → blocked, NOT finalized. expect(result.allLanded).toBe(true); expect(result.finalized).toBe(false); + expect(result.finalizeBlockedReason).toEqual(expect.stringContaining("operator review required")); for (const r of result.repos) expect(r.status).toBe("empty"); // Moved back to todo with error set; never moved done and never emitted task:merged. diff --git a/packages/engine/src/index.ts b/packages/engine/src/index.ts index 01235515a6..8ca5ab6b0e 100644 --- a/packages/engine/src/index.ts +++ b/packages/engine/src/index.ts @@ -387,6 +387,7 @@ export { // re-exported so the engine dispatch can switch to instanceof in the separate pass. WorkspaceRepoLandBusyError, WorkspacePartialLandError, + WorkspaceFinalizeBlockedError, type WorkspaceMergeResult, type WorkspaceRepoLandResult, type LandOneRepoResult, diff --git a/packages/engine/src/merge/merger-ai.ts b/packages/engine/src/merge/merger-ai.ts index 32f1aefcd5..226e32d14c 100644 --- a/packages/engine/src/merge/merger-ai.ts +++ b/packages/engine/src/merge/merger-ai.ts @@ -1778,11 +1778,13 @@ export interface WorkspaceMergeResult { /** True iff every acquired sub-repo landed (or was empty) with no failure. */ allLanded: boolean; /** - * FNXC:Workspace 2026-06-22-00:30 (Phase C U2, KTD3): - * True iff the finalize-once move-to-done ran this call (only when `allLanded`). - * False on a partial land (the task stays put for the engine dispatch's auto-retry). + * FNXC:Workspace 2026-08-15-04:22: + * `allLanded` means no sub-repo failed, but `finalized` is the ONLY proof this call + * reached `done`. When all repos land but finalization is blocked, expose the + * operator-facing reason so every merge door reports a blocked outcome honestly. */ finalized: boolean; + finalizeBlockedReason?: string; } /* @@ -1894,6 +1896,22 @@ export class WorkspacePartialLandError extends Error { } } +/* +FNXC:Workspace 2026-08-15-04:22: +A blocked workspace finalize is non-retryable because the empty-merge guard has already +parked the task with `task.error`. Keep it distinct from retryable partial lands so callers +never consume merge retries or report a merge success for work that did not reach `done`. +*/ +export class WorkspaceFinalizeBlockedError extends Error { + constructor( + public readonly taskId: string, + public readonly reason: string, + ) { + super(`Workspace finalize blocked for ${taskId}: ${reason}`); + this.name = "WorkspaceFinalizeBlockedError"; + } +} + export async function landWorkspaceTask( store: TaskStore, task: Task, @@ -2160,7 +2178,7 @@ export async function landWorkspaceTask( const reason = "branch had no net changes vs main — work may have been reverted or lost; operator review required"; await fence.write("lifecycle", () => store.updateTask(taskId, { error: reason })); - if (fence.isOrphaned()) return { taskId, repos, allLanded, finalized: false }; + if (fence.isOrphaned()) return { taskId, repos, allLanded, finalized: false, finalizeBlockedReason: reason }; const reboundColumn = await resolveFinalizeReboundColumn(store, taskId); await fence.write("log", () => store.logEntry( taskId, @@ -2173,7 +2191,7 @@ export async function landWorkspaceTask( metadata: { reason, lane: "ai-empty-merge-workspace", repoCount: repos.length, landedCount, hadPriorNoOpProof: false }, }).catch(() => undefined); await fence.write("lifecycle", () => store.moveTask(taskId, reboundColumn, { preserveProgress: true, moveSource: "engine" } as Parameters[2])); - return { taskId, repos, allLanded, finalized: false }; + return { taskId, repos, allLanded, finalized: false, finalizeBlockedReason: reason }; } const finalized = await finalizeWorkspaceTask(store, taskId, task, repos, fence); return { taskId, repos, allLanded, finalized }; diff --git a/packages/engine/src/project-engine.ts b/packages/engine/src/project-engine.ts index 93f0d256fa..ae5931e41d 100644 --- a/packages/engine/src/project-engine.ts +++ b/packages/engine/src/project-engine.ts @@ -79,7 +79,13 @@ import { createFusionAuthStorage, getFusionOAuthAlertStatePath } from "./auth/au import { CronRunner, createAiPromptExecutor } from "./scheduling/cron-runner.js"; import type { RoutineRunner } from "./scheduling/routine-runner.js"; import { sweepStaleAutostashes, VerificationError } from "./merger.js"; -import { runAiMerge, landWorkspaceTask, WorkspacePartialLandError, WorkspaceRepoLandBusyError } from "./merge/merger-ai.js"; +import { + runAiMerge, + landWorkspaceTask, + WorkspaceFinalizeBlockedError, + WorkspacePartialLandError, + WorkspaceRepoLandBusyError, +} from "./merge/merger-ai.js"; import { promoteBranchGroup, type BranchGroupPromotionResult, type CreateGroupPrFn, type SyncGroupPrFn } from "./merge/group-merge-coordinator.js"; import { formatAdmissionCapacityQueuedReason, @@ -4438,6 +4444,20 @@ export class ProjectEngine { `Workspace partial land for ${taskId}: ${landedCount} repo(s) landed, ${failed.length} failed — ${detail}`, ); } + /* + FNXC:Workspace 2026-08-15-04:22: + `allLanded` only proves no sub-repo failed; `finalized` proves the task reached + `done`. A blocked finalize is already parked and non-retryable, so never let it + reach the success path that resets retries, resolves manual waiters, or promotes + a branch group. + */ + if (!workspaceResult.finalized) { + throw new WorkspaceFinalizeBlockedError( + taskId, + workspaceResult.finalizeBlockedReason + ?? "workspace finalize was blocked after all sub-repos landed; task progress was preserved", + ); + } // Finalized to done by landWorkspaceTask; report the merge as merged so // the success path (retry reset + branch-group promotion) runs normally. const latest = await store.getTask(taskId).catch(() => mergeTask!); @@ -4611,6 +4631,26 @@ export class ProjectEngine { continue; } + /* + FNXC:Workspace 2026-08-15-04:22: + A workspace finalize blocked after all repos landed is NOT merge success and is NOT + retryable: the producer already parked it with task.error. Do not reset or consume + mergeRetries, double-park it as failed, resolve a manual waiter with ok:true, or promote + a branch group. Clear transient busy bookkeeping and surface the real blocked reason. + */ + if (err instanceof WorkspaceFinalizeBlockedError) { + runtimeLog.error( + `${hasManualResolver ? "Manual" : "Auto"}-merge blocked for ${taskId}: ${err.reason}`, + ); + await store.logEntry(taskId, `Workspace finalize blocked: ${err.reason}`, "WorkspaceFinalizeBlocked") + .catch(() => undefined); + if (hasManualResolver) { + this.rejectMergeResolvers(taskId, err); + } + this.workspaceBusyReenqueues.delete(taskId); + continue; + } + // FNXC:Workspace 2026-06-22-00:30 (Phase C U2, KTD3): // Workspace PARTIAL-LAND auto-retry-then-park (user decision). Unlike the R7 // WorkspaceTaskMergeError above (a permanent config error that must NOT burn