From 21afa84b2b97b8e9ad37b798d60876f061818ddd Mon Sep 17 00:00:00 2001 From: Phil Larson Date: Thu, 11 Jun 2026 18:57:56 -0700 Subject: [PATCH 1/9] test(FN-343): add regression for temp cleanup classification Fusion-Task-Id: FN-343 Co-authored-by: Fusion --- .../src/__tests__/worktree-backend.test.ts | 94 +++++++++++++++++++ 1 file changed, 94 insertions(+) diff --git a/packages/engine/src/__tests__/worktree-backend.test.ts b/packages/engine/src/__tests__/worktree-backend.test.ts index a227a49f33..21eb85e9b6 100644 --- a/packages/engine/src/__tests__/worktree-backend.test.ts +++ b/packages/engine/src/__tests__/worktree-backend.test.ts @@ -911,6 +911,100 @@ describe("removeWorktree", () => { expect(audit.git).toHaveBeenCalledWith({ type: "worktree:remove", target: "/repo/.worktrees/fn-1" }); }); + it("classifies FN-343 nonstandard temp merge worktree remove failures as harmless when porcelain is absent after prune", async () => { + const tempPath = "/var/folders/demo/T/fusion-ai-merge-fn-327-A5uY3j"; + const validationError = { + message: `Command failed: git worktree remove --force ${tempPath}`, + stderr: `fatal: validation failed, cannot remove working tree: '${tempPath}/.git' is not a .git file, error code 2`, + status: 2, + }; + execMock + .mockRejectedValueOnce(validationError) + .mockResolvedValueOnce({ stdout: "", stderr: "" }) + .mockResolvedValueOnce({ stdout: "worktree /repo\nbranch refs/heads/main\n", stderr: "" }); + const audit = { git: vi.fn().mockResolvedValue(undefined) } as any; + + // A real-git fixture for this exact macOS temp shape is git-version sensitive: + // some versions prune the malformed admin entry before emitting the validation + // string. Keep the classifier deterministic by simulating the exact FN-327 + // command stderr, then assert the porcelain proof that no registered worktree + // remains for the temp path. + await expect( + removeWorktree({ + rootDir: "/repo", + worktreePath: tempPath, + settings: {}, + audit, + taskId: "FN-327", + reason: RemovalReason.MergerCleanup, + }), + ).resolves.toBeUndefined(); + + expect(execMock).toHaveBeenNthCalledWith( + 2, + "git worktree prune", + expect.objectContaining({ cwd: "/repo" }), + ); + expect(execMock).toHaveBeenNthCalledWith( + 3, + "git worktree list --porcelain", + expect.objectContaining({ cwd: "/repo" }), + ); + expect(audit.git).toHaveBeenCalledWith( + expect.objectContaining({ + type: "worktree:remove-classified-harmless", + target: tempPath, + metadata: expect.objectContaining({ + reason: RemovalReason.MergerCleanup, + classification: "not-registered-after-prune", + registeredAfterPrune: false, + stderrPreview: expect.stringContaining("is not a .git file"), + }), + }), + ); + }); + + it("keeps FN-343 remove failures visible when the temp path remains registered after prune", async () => { + const tempPath = "/var/folders/demo/T/fusion-ai-merge-fn-327-A5uY3j"; + const validationError = { + message: `Command failed: git worktree remove --force ${tempPath}`, + stderr: `fatal: validation failed, cannot remove working tree: '${tempPath}/.git' is not a .git file, error code 2`, + status: 2, + }; + execMock + .mockRejectedValueOnce(validationError) + .mockResolvedValueOnce({ stdout: "", stderr: "" }) + .mockResolvedValueOnce({ + stdout: `worktree /repo\nbranch refs/heads/main\n\nworktree ${tempPath}\nbranch refs/heads/fusion/fn-327\n`, + stderr: "", + }); + const audit = { git: vi.fn().mockResolvedValue(undefined) } as any; + + await expect( + removeWorktree({ + rootDir: "/repo", + worktreePath: tempPath, + settings: {}, + audit, + taskId: "FN-327", + reason: RemovalReason.MergerCleanup, + }), + ).rejects.toMatchObject({ stderr: expect.stringContaining("is not a .git file") }); + + expect(execMock).toHaveBeenNthCalledWith(2, "git worktree prune", expect.objectContaining({ cwd: "/repo" })); + expect(execMock).toHaveBeenNthCalledWith(3, "git worktree list --porcelain", expect.objectContaining({ cwd: "/repo" })); + expect(audit.git).toHaveBeenCalledWith( + expect.objectContaining({ + type: "worktree:remove-leaked-registered-worktree", + target: tempPath, + metadata: expect.objectContaining({ + reason: RemovalReason.MergerCleanup, + registeredAfterPrune: true, + }), + }), + ); + }); + it("uses worktrunk remove and emits worktree:worktrunk-remove", async () => { execMock.mockResolvedValue({ stdout: "", stderr: "" }); const audit = { git: vi.fn().mockResolvedValue(undefined) } as any; From 3cfc3972a59392d5f3afbf2ff6fc4180719adc49 Mon Sep 17 00:00:00 2001 From: Phil Larson Date: Thu, 11 Jun 2026 19:02:47 -0700 Subject: [PATCH 2/9] fix(FN-343): classify pruned temp merge worktree removals Fusion-Task-Id: FN-343 Co-authored-by: Fusion --- .../src/__tests__/worktree-backend.test.ts | 29 +++- packages/engine/src/merger.ts | 12 +- packages/engine/src/run-audit.ts | 2 + packages/engine/src/worktree-backend.ts | 145 +++++++++++++++++- 4 files changed, 180 insertions(+), 8 deletions(-) diff --git a/packages/engine/src/__tests__/worktree-backend.test.ts b/packages/engine/src/__tests__/worktree-backend.test.ts index 21eb85e9b6..90df828ba0 100644 --- a/packages/engine/src/__tests__/worktree-backend.test.ts +++ b/packages/engine/src/__tests__/worktree-backend.test.ts @@ -938,7 +938,12 @@ describe("removeWorktree", () => { taskId: "FN-327", reason: RemovalReason.MergerCleanup, }), - ).resolves.toBeUndefined(); + ).resolves.toMatchObject({ + removed: false, + harmless: true, + classification: "not-registered-after-prune", + message: expect.stringContaining("no registered worktree remains after prune"), + }); expect(execMock).toHaveBeenNthCalledWith( 2, @@ -964,6 +969,28 @@ describe("removeWorktree", () => { ); }); + it("does not downgrade non-temp merger cleanup failures even when porcelain would be absent", async () => { + const worktreePath = "/repo/.worktrees/fn-327"; + const validationError = { + message: `Command failed: git worktree remove --force ${worktreePath}`, + stderr: `fatal: validation failed, cannot remove working tree: '${worktreePath}/.git' is not a .git file, error code 2`, + status: 2, + }; + execMock.mockRejectedValueOnce(validationError); + + await expect( + removeWorktree({ + rootDir: "/repo", + worktreePath, + settings: {}, + taskId: "FN-327", + reason: RemovalReason.MergerCleanup, + }), + ).rejects.toBe(validationError); + + expect(execMock).toHaveBeenCalledTimes(1); + }); + it("keeps FN-343 remove failures visible when the temp path remains registered after prune", async () => { const tempPath = "/var/folders/demo/T/fusion-ai-merge-fn-327-A5uY3j"; const validationError = { diff --git a/packages/engine/src/merger.ts b/packages/engine/src/merger.ts index 4271c6616d..79fe7deb54 100644 --- a/packages/engine/src/merger.ts +++ b/packages/engine/src/merger.ts @@ -7314,13 +7314,16 @@ async function removePostMergeWorktree( settings: Partial, ): Promise { try { - await removeWorktree({ + const outcome = await removeWorktree({ rootDir, worktreePath: postMergeWorktree, settings, taskId, reason: RemovalReason.MergerPostMerge, }); + if (outcome && "harmless" in outcome && outcome.harmless) { + mergerLog.warn(`${taskId}: post-merge worktree cleanup remove failed, but no registered worktree remains after prune for ${postMergeWorktree}: ${outcome.message}`); + } } catch (err: unknown) { mergerLog.warn(`${taskId}: failed to remove post-merge worktree ${postMergeWorktree}: ${getCommandErrorMessage(err)}`); } @@ -10569,7 +10572,7 @@ export async function aiMergeTask( metadata: { taskId, reason: RemovalReason.MergerCleanup, kind: "merger" }, }); } else { - await removeWorktree({ + const outcome = await removeWorktree({ rootDir, worktreePath, settings, @@ -10577,7 +10580,10 @@ export async function aiMergeTask( audit, reason: RemovalReason.MergerCleanup, }); - result.worktreeRemoved = true; + if (outcome && "harmless" in outcome && outcome.harmless) { + mergerLog.warn(`${taskId}: merge worktree cleanup remove failed, but no registered worktree remains after prune for ${worktreePath}: ${outcome.message}`); + } + result.worktreeRemoved = outcome?.removed ?? true; } if (result.worktreeRemoved) { try { diff --git a/packages/engine/src/run-audit.ts b/packages/engine/src/run-audit.ts index ee131c30b6..fcdd1d19ee 100644 --- a/packages/engine/src/run-audit.ts +++ b/packages/engine/src/run-audit.ts @@ -92,6 +92,8 @@ export type GitMutationType = | "worktree:create" | "worktree:remove" | "worktree:remove-fallback" + | "worktree:remove-classified-harmless" + | "worktree:remove-leaked-registered-worktree" | "worktree:reuse" | "worktree:incomplete-detected" | "worktree:reanchored" diff --git a/packages/engine/src/worktree-backend.ts b/packages/engine/src/worktree-backend.ts index 6abc8dbd07..74a811cfac 100644 --- a/packages/engine/src/worktree-backend.ts +++ b/packages/engine/src/worktree-backend.ts @@ -30,6 +30,133 @@ const NATIVE_TIMEOUT_MS = 120_000; const REMOVE_TIMEOUT_MS = 60_000; const MAX_BUFFER = 10 * 1024 * 1024; + +export type WorktreeRemoveOutcome = + | { removed: true; classification: "removed" } + | { + removed: false; + harmless: true; + classification: "not-registered-after-prune"; + message: string; + stderrPreview: string; + pathExists: boolean; + gitFileExists: boolean; + }; + +const HARMLESS_MERGE_REMOVE_ERROR_PATTERNS = [ + /validation failed, cannot remove working tree/i, + /is not a \.git file/i, + /is not a working tree/i, + /not a git repository/i, + /No such file or directory/i, +] as const; + +function previewError(error: unknown): string { + const stderr = getErrorStderr(error); + const message = error instanceof Error ? error.message : String(error); + return (stderr || message).slice(0, 4096); +} + +function normalizeComparablePath(value: string): string { + const resolved = resolve(value); + return resolved.startsWith("/private/var/") ? resolved.slice("/private".length) : resolved; +} + +function porcelainContainsWorktree(stdout: string, worktreePath: string): boolean { + const target = normalizeComparablePath(worktreePath); + const privateTarget = target.startsWith("/var/") ? `/private${target}` : target; + for (const line of stdout.split("\n")) { + if (!line.startsWith("worktree ")) continue; + const candidate = normalizeComparablePath(line.slice("worktree ".length).trim()); + if (candidate === target || candidate === privateTarget) return true; + } + return false; +} + +function isMergeTempCleanupCandidate(input: { worktreePath: string; reason: RemovalReason }, error: unknown): boolean { + if (input.reason !== RemovalReason.MergerCleanup && input.reason !== RemovalReason.MergerPostMerge) return false; + const base = basename(input.worktreePath); + const looksLikeFusionMergeTemp = base.startsWith("fusion-ai-merge-") || base.startsWith("post-merge-"); + if (!looksLikeFusionMergeTemp) return false; + const detail = previewError(error); + return HARMLESS_MERGE_REMOVE_ERROR_PATTERNS.some((pattern) => pattern.test(detail)); +} + +async function classifyHarmlessMergeRemoveFailure(input: { + rootDir: string; + worktreePath: string; + reason: RemovalReason; + taskId?: string; + audit?: RunAuditor; +}, error: unknown): Promise { + if (!isMergeTempCleanupCandidate(input, error)) return null; + + const stderrPreview = previewError(error); + const pathExists = existsSync(input.worktreePath); + const gitFileExists = existsSync(resolve(input.worktreePath, ".git")); + + await execAsync("git worktree prune", { + cwd: input.rootDir, + encoding: "utf-8", + timeout: NATIVE_TIMEOUT_MS, + maxBuffer: MAX_BUFFER, + }); + + const { stdout } = await execAsync("git worktree list --porcelain", { + cwd: input.rootDir, + encoding: "utf-8", + timeout: 10_000, + maxBuffer: MAX_BUFFER, + }); + const registeredAfterPrune = porcelainContainsWorktree(String(stdout ?? ""), input.worktreePath); + + if (registeredAfterPrune) { + await input.audit?.git({ + type: "worktree:remove-leaked-registered-worktree", + target: input.worktreePath, + metadata: { + taskId: input.taskId, + reason: input.reason, + registeredAfterPrune: true, + stderrPreview, + pathExists, + gitFileExists, + }, + }); + return null; + } + + const message = pathExists + ? "cleanup remove failed, but no registered worktree remains after prune; leftover directory was not deleted automatically" + : "cleanup remove failed, but no registered worktree remains after prune"; + await input.audit?.git({ + type: "worktree:remove-classified-harmless", + target: input.worktreePath, + metadata: { + taskId: input.taskId, + reason: input.reason, + classification: "not-registered-after-prune", + registeredAfterPrune: false, + stderrPreview, + pathExists, + gitFileExists, + nextAction: pathExists + ? "inspect the leftover temp directory before deleting filesystem residue" + : "no operator action required", + }, + }); + + return { + removed: false, + harmless: true, + classification: "not-registered-after-prune", + message, + stderrPreview, + pathExists, + gitFileExists, + }; +} + /** * worktrunk CLI mapping (verified 2026-05-15 from README + worktrunk.dev docs): * - create -> `wt switch --create [--base ]` @@ -822,7 +949,7 @@ export async function removeWorktree(input: { liveOwnerProbe?: LiveBindingProbe; processActiveProbe?: ProcessActiveProbe; reconcileMinIdleMs?: number; -}): Promise { +}): Promise { const logger = { log: (_message: string): void => {}, warn: (_message: string): void => {}, @@ -896,8 +1023,11 @@ export async function removeWorktree(input: { target: input.worktreePath, }); } - return; + return { removed: true, classification: "removed" }; } catch (error) { + const classified = await classifyHarmlessMergeRemoveFailure(input, error); + if (classified) return classified; + if (!(error instanceof WorktrunkOperationError) || input.settings.worktrunk?.onFailure !== "fallback-native") { throw error; } @@ -915,8 +1045,15 @@ export async function removeWorktree(input: { }); const native = new NativeWorktreeBackend({ logger, settings: input.settings }); - await native.remove(removeInput); - await input.audit?.git({ type: "worktree:remove", target: input.worktreePath }); + try { + await native.remove(removeInput); + await input.audit?.git({ type: "worktree:remove", target: input.worktreePath }); + return { removed: true, classification: "removed" }; + } catch (nativeError) { + const classified = await classifyHarmlessMergeRemoveFailure(input, nativeError); + if (classified) return classified; + throw nativeError; + } } } From 92c5503f88509b217989ed7328a44b8c50d1899a Mon Sep 17 00:00:00 2001 From: Phil Larson Date: Thu, 11 Jun 2026 19:06:46 -0700 Subject: [PATCH 3/9] test(FN-343): cover merger temp cleanup visibility Fusion-Task-Id: FN-343 Co-authored-by: Fusion --- .../worktrunk-worktree-removal.test.ts | 66 ++++++++++++++++++- packages/engine/src/worktree-backend.ts | 5 +- 2 files changed, 68 insertions(+), 3 deletions(-) diff --git a/packages/engine/src/__tests__/reliability-interactions/worktrunk-worktree-removal.test.ts b/packages/engine/src/__tests__/reliability-interactions/worktrunk-worktree-removal.test.ts index c93459d8f2..5de6428a52 100644 --- a/packages/engine/src/__tests__/reliability-interactions/worktrunk-worktree-removal.test.ts +++ b/packages/engine/src/__tests__/reliability-interactions/worktrunk-worktree-removal.test.ts @@ -1,4 +1,4 @@ -import { beforeEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { EventEmitter } from "node:events"; import type { Settings, TaskStore, Task } from "@fusion/core"; import { cleanupOrphanedWorktrees } from "../../worktree-pool.js"; @@ -41,6 +41,10 @@ describe("reliability interactions: worktrunk worktree removal routing", () => { existsSpy.mockReturnValue(true); }); + afterEach(() => { + vi.restoreAllMocks(); + }); + it("merger post-merge cleanup calls worktrunk backend remove and avoids native git remove", async () => { const removeSpy = vi.spyOn(WorktrunkWorktreeBackend.prototype, "remove").mockResolvedValue(undefined); @@ -52,6 +56,66 @@ describe("reliability interactions: worktrunk worktree removal routing", () => { expect(execSpy.mock.calls.some((call) => String(call[0]).includes("git worktree remove"))).toBe(false); }); + it("merger post-merge cleanup logs harmless classified temp residue when porcelain is absent after prune", async () => { + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => undefined); + const postMergePath = "/repo/.worktrees/post-merge-FN-343-abcd1234"; + execSpy.mockImplementation((cmd: string, _opts: unknown, cb: (err: any, stdout: string, stderr: string) => void) => { + if (cmd.includes("git worktree remove")) { + const stderr = `fatal: validation failed, cannot remove working tree: '${postMergePath}/.git' is not a .git file, error code 2`; + cb(Object.assign(new Error(stderr), { stderr, status: 2 }), "", stderr); + return; + } + if (cmd === "git worktree prune") { + cb(null, "", ""); + return; + } + if (cmd === "git worktree list --porcelain") { + cb(null, "worktree /repo\nbranch refs/heads/main\n", ""); + return; + } + cb(null, "", ""); + }); + + await mergerTestHooks.removePostMergeWorktree("/repo", postMergePath, "FN-343", {}); + + expect(execSpy.mock.calls.map((call) => String(call[0]))).toEqual([ + `git worktree remove --force "${postMergePath}"`, + "git worktree prune", + "git worktree list --porcelain", + ]); + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining("post-merge worktree cleanup remove failed, but no registered worktree remains after prune"), + ); + }); + + it("merger post-merge cleanup keeps still-registered temp worktree failures visible", async () => { + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => undefined); + const postMergePath = "/repo/.worktrees/post-merge-FN-343-abcd1234"; + execSpy.mockImplementation((cmd: string, _opts: unknown, cb: (err: any, stdout: string, stderr: string) => void) => { + if (cmd.includes("git worktree remove")) { + const stderr = `fatal: validation failed, cannot remove working tree: '${postMergePath}/.git' is not a .git file, error code 2`; + cb(Object.assign(new Error(stderr), { stderr, status: 2 }), "", stderr); + return; + } + if (cmd === "git worktree prune") { + cb(null, "", ""); + return; + } + if (cmd === "git worktree list --porcelain") { + cb(null, `worktree /repo\nbranch refs/heads/main\n\nworktree ${postMergePath}\nbranch refs/heads/fusion/fn-343\n`, ""); + return; + } + cb(null, "", ""); + }); + + await mergerTestHooks.removePostMergeWorktree("/repo", postMergePath, "FN-343", {}); + + expect(execSpy.mock.calls.map((call) => String(call[0]))).toContain("git worktree list --porcelain"); + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining(`failed to remove post-merge worktree ${postMergePath}`), + ); + }); + it("self-healing recover path calls worktrunk backend remove and not native remove", async () => { const removeSpy = vi.spyOn(WorktrunkWorktreeBackend.prototype, "remove").mockResolvedValue(undefined); const task = { diff --git a/packages/engine/src/worktree-backend.ts b/packages/engine/src/worktree-backend.ts index 74a811cfac..b8caf8552d 100644 --- a/packages/engine/src/worktree-backend.ts +++ b/packages/engine/src/worktree-backend.ts @@ -102,13 +102,14 @@ async function classifyHarmlessMergeRemoveFailure(input: { maxBuffer: MAX_BUFFER, }); - const { stdout } = await execAsync("git worktree list --porcelain", { + const listResult = await execAsync("git worktree list --porcelain", { cwd: input.rootDir, encoding: "utf-8", timeout: 10_000, maxBuffer: MAX_BUFFER, }); - const registeredAfterPrune = porcelainContainsWorktree(String(stdout ?? ""), input.worktreePath); + const stdout = typeof listResult === "string" ? listResult : String(listResult.stdout ?? ""); + const registeredAfterPrune = porcelainContainsWorktree(stdout, input.worktreePath); if (registeredAfterPrune) { await input.audit?.git({ From e35f3dd2b8630e9acaff588f8f211a687cb86901 Mon Sep 17 00:00:00 2001 From: Phil Larson Date: Thu, 11 Jun 2026 19:08:57 -0700 Subject: [PATCH 4/9] =?UTF-8?q?feat(FN-343):=20complete=20Step=205=20?= =?UTF-8?q?=E2=80=94=20document=20cleanup=20diagnostics=20and=20changeset?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fusion-Task-Id: FN-343 Co-authored-by: Fusion --- .changeset/fn-343-merge-worktree-cleanup.md | 5 +++++ docs/diagnostics.md | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+) create mode 100644 .changeset/fn-343-merge-worktree-cleanup.md diff --git a/.changeset/fn-343-merge-worktree-cleanup.md b/.changeset/fn-343-merge-worktree-cleanup.md new file mode 100644 index 0000000000..2b6d3982fc --- /dev/null +++ b/.changeset/fn-343-merge-worktree-cleanup.md @@ -0,0 +1,5 @@ +--- +"@fusion/engine": patch +--- + +Classify harmless temporary merge worktree cleanup failures after `git worktree prune`/porcelain inspection while keeping still-registered worktree leaks visible in merger diagnostics. diff --git a/docs/diagnostics.md b/docs/diagnostics.md index 176cea2633..1e67cbaff2 100644 --- a/docs/diagnostics.md +++ b/docs/diagnostics.md @@ -140,3 +140,21 @@ FN-5416 extends resume-correlation coverage to stream-focused hooks and their pr - Route shells - `DevServerView`: `remount` / `route-active` / `route-inactive` - `ResearchView`: `remount` / `route-active` / `route-inactive` + +## Merge temp worktree cleanup classification (`[merger]`) + +Fusion merge cleanup treats a narrow class of temporary merge/post-merge worktree removal failures as non-fatal only after Git admin state proves there is no registered worktree leak. + +- Applies to Fusion-created temp merge paths such as `fusion-ai-merge-*` and post-merge paths such as `post-merge-*` during `merger-cleanup` / `merger-post-merge` removal. +- Trigger shape: `git worktree remove --force ` fails with validation text such as `fatal: validation failed, cannot remove working tree: '/.git' is not a .git file`. +- Recovery proof: Fusion runs `git worktree prune`, then inspects `git worktree list --porcelain`. +- Harmless classification: if the target path is absent from porcelain after prune, the merger logs that cleanup remove failed but no registered worktree remains. If a directory still exists, Fusion reports it as residue for operator inspection; it does not delete arbitrary `/var/folders` content. +- Leak classification: if the target path is still present in porcelain after prune, the cleanup failure remains visible as a real registered-worktree leak. + +Operator verification command: + +```bash +git worktree list --porcelain | grep -F "" +``` + +No output means Git no longer registers that temp path; matching `worktree ` output means the leak is still registered and needs operator cleanup. From 2c8e9c6439f748b75539c89aec0c595c99dcee97 Mon Sep 17 00:00:00 2001 From: Phil Larson Date: Thu, 11 Jun 2026 17:33:00 -0700 Subject: [PATCH 5/9] fix(dashboard): map merge workflow nodes in editor --- packages/dashboard/app/components/workflow-flow-mapping.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/dashboard/app/components/workflow-flow-mapping.ts b/packages/dashboard/app/components/workflow-flow-mapping.ts index cd93d61789..63dbc81641 100644 --- a/packages/dashboard/app/components/workflow-flow-mapping.ts +++ b/packages/dashboard/app/components/workflow-flow-mapping.ts @@ -173,7 +173,6 @@ function isSameKindEditorNodeKind( function editorKind(node: WorkflowIr["nodes"][number]): WorkflowEditorNodeKind { const seam = node.config?.seam; if (seam === "merge") return "merge"; - const mapped = GRAPH_ONLY_EDITOR_KIND[node.kind]; if (mapped) return mapped; From 16d71e6773baf5e3b2f837c4002aced51bbd8df4 Mon Sep 17 00:00:00 2001 From: Phil Larson Date: Thu, 11 Jun 2026 19:55:58 -0700 Subject: [PATCH 6/9] fix(FN-343): address PR review feedback --- .../__tests__/workflow-flow-mapping.test.ts | 29 ++++++++++ .../app/components/workflow-flow-mapping.ts | 25 +++++++-- .../worktrunk-worktree-removal.test.ts | 54 ++++++++----------- .../src/__tests__/worktree-backend.test.ts | 39 ++++++++++++++ packages/engine/src/run-audit.ts | 1 + packages/engine/src/worktree-backend.ts | 43 ++++++++++----- 6 files changed, 141 insertions(+), 50 deletions(-) diff --git a/packages/dashboard/app/components/__tests__/workflow-flow-mapping.test.ts b/packages/dashboard/app/components/__tests__/workflow-flow-mapping.test.ts index 698a8c8f89..8d52db7b60 100644 --- a/packages/dashboard/app/components/__tests__/workflow-flow-mapping.test.ts +++ b/packages/dashboard/app/components/__tests__/workflow-flow-mapping.test.ts @@ -198,6 +198,35 @@ describe("workflow-flow-mapping v2 round-trip", () => { expect(byId.j1.config?.onBranchFailure).toBe("fail-fast"); }); + + it("preserves aliased IR node kinds when round-tripping through editor render kinds", () => { + const ir: WorkflowDefinition["ir"] = { + version: "v2", + name: "merge aliases", + columns: [{ id: "in-progress", name: "In progress", traits: [] }], + nodes: [ + { id: "gate", kind: "merge-gate", column: "in-progress", config: { name: "Gate" } }, + { id: "attempt", kind: "merge-attempt", column: "in-progress" }, + { id: "hold", kind: "manual-merge-hold", column: "in-progress", config: { release: "manual" } }, + { id: "retry", kind: "retry-backoff", column: "in-progress", config: { maxIterations: 2, template: { nodes: [], edges: [] } } }, + ] as WorkflowDefinition["ir"]["nodes"], + edges: [], + }; + + const { nodes, edges } = irToFlow(v2Def(ir)); + expect(nodes.find((node) => node.id === "gate")?.type).toBe("merge"); + expect(nodes.find((node) => node.id === "hold")?.type).toBe("hold"); + expect(nodes.find((node) => node.id === "retry")?.type).toBe("loop"); + + const { ir: out } = flowToIr("merge aliases", nodes, edges, columnsOf(v2Def(ir))); + if (out.version !== "v2") throw new Error("expected v2"); + const byId = Object.fromEntries(out.nodes.map((node) => [node.id, node])); + expect(byId.gate.kind).toBe("merge-gate"); + expect(byId.attempt.kind).toBe("merge-attempt"); + expect(byId.hold.kind).toBe("manual-merge-hold"); + expect(byId.retry.kind).toBe("retry-backoff"); + }); + it("emits swimlane band group nodes that flowToIr strips back out", () => { const { nodes } = irToFlow(v2Def(ir)); const bands = nodes.filter((n) => isColumnBandNode(n.id)); diff --git a/packages/dashboard/app/components/workflow-flow-mapping.ts b/packages/dashboard/app/components/workflow-flow-mapping.ts index 63dbc81641..14532b17ff 100644 --- a/packages/dashboard/app/components/workflow-flow-mapping.ts +++ b/packages/dashboard/app/components/workflow-flow-mapping.ts @@ -188,6 +188,14 @@ function nodeLabel(node: WorkflowIr["nodes"][number]): string { return node.id; } +function dataIrKind(node: WorkflowIrNode, editorNodeKind: WorkflowEditorNodeKind): Partial { + return node.kind === editorNodeKind ? {} : { irKind: node.kind }; +} + +function preservedIrKind(data: WorkflowFlowNodeData): WorkflowIrNode["kind"] | undefined { + return typeof data.irKind === "string" ? (data.irKind as WorkflowIrNode["kind"]) : undefined; +} + /** Build React Flow swimlane band group nodes from the workflow's columns. */ export function columnsToBandNodes(columns: WorkflowIrColumn[]): FlowNode[] { return columns.map((col, index): FlowNode => ({ @@ -313,7 +321,7 @@ export function irToFlow(def: WorkflowDefinition): { position: childPos, parentId: node.id, extent: "parent", - data: { kind: innerKind, label: nodeLabel(inner), config: { ...(inner.config ?? {}) } }, + data: { kind: innerKind, ...dataIrKind(inner, innerKind), label: nodeLabel(inner), config: { ...(inner.config ?? {}) } }, deletable: true, zIndex: WF_STEP_NODE_Z_INDEX, }); @@ -329,6 +337,7 @@ export function irToFlow(def: WorkflowDefinition): { position: pos ?? { x: 80 + index * 180, y: fallbackY }, data: { kind, + ...dataIrKind(node, kind), label: nodeLabel(node), config: { ...restCfg }, column, @@ -346,6 +355,7 @@ export function irToFlow(def: WorkflowDefinition): { position: pos ?? { x: 80 + index * 180, y: fallbackY }, data: { kind, + ...dataIrKind(node, kind), label: nodeLabel(node), config: { ...(node.config ?? {}) }, column, @@ -418,7 +428,11 @@ export function flowToIr( function toIrNode(node: FlowNode, localId: string): WorkflowIrNode { const data = node.data; const config = nodeConfig(node); + const originalKind = preservedIrKind(data); if (data.kind === "merge") { + if (originalKind) { + return { id: localId, kind: originalKind, config: config && Object.keys(config).length ? config : undefined }; + } return { id: localId, kind: "prompt", config: { ...(config ?? {}), seam: "merge" } }; } if (data.kind === "foreach" || data.kind === "loop") { @@ -436,13 +450,13 @@ export function flowToIr( const baseCfg = (config ?? {}) as Record; return { id: localId, - kind: data.kind, + kind: originalKind ?? data.kind, config: { ...baseCfg, template: { nodes: templateNodes, edges: templateEdges } }, }; } return { id: localId, - kind: data.kind as WorkflowIrNode["kind"], + kind: originalKind ?? (data.kind as WorkflowIrNode["kind"]), config: config && Object.keys(config).length ? config : undefined, }; } @@ -988,7 +1002,7 @@ function irNodeToFlowNode( id, type: kind, position, - data: { kind, label: nodeLabel(node), config: { ...(node.config ?? {}) } }, + data: { kind, ...dataIrKind(node, kind), label: nodeLabel(node), config: { ...(node.config ?? {}) } }, deletable: node.kind !== "start" && node.kind !== "end", zIndex: WF_STEP_NODE_Z_INDEX, }; @@ -1066,7 +1080,7 @@ export function insertFragment( position: childPos, parentId: id, extent: "parent", - data: { kind: innerKind, label: nodeLabel(inner), config: { ...(inner.config ?? {}) } }, + data: { kind: innerKind, ...dataIrKind(inner, innerKind), label: nodeLabel(inner), config: { ...(inner.config ?? {}) } }, deletable: true, zIndex: WF_STEP_NODE_Z_INDEX, }); @@ -1082,6 +1096,7 @@ export function insertFragment( position: pos, data: { kind: groupKind, + ...dataIrKind(node, groupKind), label: nodeLabel(node), config: { ...restCfg }, templateEmpty: template.nodes.length === 0, diff --git a/packages/engine/src/__tests__/reliability-interactions/worktrunk-worktree-removal.test.ts b/packages/engine/src/__tests__/reliability-interactions/worktrunk-worktree-removal.test.ts index 5de6428a52..715756dc52 100644 --- a/packages/engine/src/__tests__/reliability-interactions/worktrunk-worktree-removal.test.ts +++ b/packages/engine/src/__tests__/reliability-interactions/worktrunk-worktree-removal.test.ts @@ -22,6 +22,26 @@ vi.mock("node:fs", async (importOriginal) => { return { ...actual, existsSync: existsSpy, readdirSync: readdirSpy }; }); + +function mockWorktreeRemoveFailure(postMergePath: string, porcelainOutput: string): void { + execSpy.mockImplementation((cmd: string, _opts: unknown, cb: (err: any, stdout: string, stderr: string) => void) => { + if (cmd.includes("git worktree remove")) { + const stderr = `fatal: validation failed, cannot remove working tree: '${postMergePath}/.git' is not a .git file, error code 2`; + cb(Object.assign(new Error(stderr), { stderr, status: 2 }), "", stderr); + return; + } + if (cmd === "git worktree prune") { + cb(null, "", ""); + return; + } + if (cmd === "git worktree list --porcelain") { + cb(null, porcelainOutput, ""); + return; + } + cb(null, "", ""); + }); +} + function storeForSelfHealing(settings: Partial, task: Partial): TaskStore & EventEmitter { const emitter = new EventEmitter(); return Object.assign(emitter, { @@ -59,22 +79,7 @@ describe("reliability interactions: worktrunk worktree removal routing", () => { it("merger post-merge cleanup logs harmless classified temp residue when porcelain is absent after prune", async () => { const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => undefined); const postMergePath = "/repo/.worktrees/post-merge-FN-343-abcd1234"; - execSpy.mockImplementation((cmd: string, _opts: unknown, cb: (err: any, stdout: string, stderr: string) => void) => { - if (cmd.includes("git worktree remove")) { - const stderr = `fatal: validation failed, cannot remove working tree: '${postMergePath}/.git' is not a .git file, error code 2`; - cb(Object.assign(new Error(stderr), { stderr, status: 2 }), "", stderr); - return; - } - if (cmd === "git worktree prune") { - cb(null, "", ""); - return; - } - if (cmd === "git worktree list --porcelain") { - cb(null, "worktree /repo\nbranch refs/heads/main\n", ""); - return; - } - cb(null, "", ""); - }); + mockWorktreeRemoveFailure(postMergePath, "worktree /repo\nbranch refs/heads/main\n"); await mergerTestHooks.removePostMergeWorktree("/repo", postMergePath, "FN-343", {}); @@ -91,22 +96,7 @@ describe("reliability interactions: worktrunk worktree removal routing", () => { it("merger post-merge cleanup keeps still-registered temp worktree failures visible", async () => { const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => undefined); const postMergePath = "/repo/.worktrees/post-merge-FN-343-abcd1234"; - execSpy.mockImplementation((cmd: string, _opts: unknown, cb: (err: any, stdout: string, stderr: string) => void) => { - if (cmd.includes("git worktree remove")) { - const stderr = `fatal: validation failed, cannot remove working tree: '${postMergePath}/.git' is not a .git file, error code 2`; - cb(Object.assign(new Error(stderr), { stderr, status: 2 }), "", stderr); - return; - } - if (cmd === "git worktree prune") { - cb(null, "", ""); - return; - } - if (cmd === "git worktree list --porcelain") { - cb(null, `worktree /repo\nbranch refs/heads/main\n\nworktree ${postMergePath}\nbranch refs/heads/fusion/fn-343\n`, ""); - return; - } - cb(null, "", ""); - }); + mockWorktreeRemoveFailure(postMergePath, `worktree /repo\nbranch refs/heads/main\n\nworktree ${postMergePath}\nbranch refs/heads/fusion/fn-343\n`); await mergerTestHooks.removePostMergeWorktree("/repo", postMergePath, "FN-343", {}); diff --git a/packages/engine/src/__tests__/worktree-backend.test.ts b/packages/engine/src/__tests__/worktree-backend.test.ts index 90df828ba0..b1cc8406a3 100644 --- a/packages/engine/src/__tests__/worktree-backend.test.ts +++ b/packages/engine/src/__tests__/worktree-backend.test.ts @@ -1032,6 +1032,45 @@ describe("removeWorktree", () => { ); }); + + it("preserves the original remove failure when classification probes fail", async () => { + const tempPath = "/var/folders/demo/T/fusion-ai-merge-fn-327-A5uY3j"; + const validationError = { + message: `Command failed: git worktree remove --force ${tempPath}`, + stderr: `fatal: validation failed, cannot remove working tree: '${tempPath}/.git' is not a .git file, error code 2`, + status: 2, + }; + const probeError = new Error("git worktree prune failed"); + execMock + .mockRejectedValueOnce(validationError) + .mockRejectedValueOnce(probeError); + const audit = { git: vi.fn().mockResolvedValue(undefined) } as any; + + await expect( + removeWorktree({ + rootDir: "/repo", + worktreePath: tempPath, + settings: {}, + audit, + taskId: "FN-327", + reason: RemovalReason.MergerCleanup, + }), + ).rejects.toBe(validationError); + + expect(execMock).toHaveBeenNthCalledWith(2, "git worktree prune", expect.objectContaining({ cwd: "/repo" })); + expect(audit.git).toHaveBeenCalledWith( + expect.objectContaining({ + type: "worktree:remove-classification-probe-failed", + target: tempPath, + metadata: expect.objectContaining({ + reason: RemovalReason.MergerCleanup, + stderrPreview: expect.stringContaining("is not a .git file"), + probeError: expect.stringContaining("git worktree prune failed"), + }), + }), + ); + }); + it("uses worktrunk remove and emits worktree:worktrunk-remove", async () => { execMock.mockResolvedValue({ stdout: "", stderr: "" }); const audit = { git: vi.fn().mockResolvedValue(undefined) } as any; diff --git a/packages/engine/src/run-audit.ts b/packages/engine/src/run-audit.ts index fcdd1d19ee..9cf7077e64 100644 --- a/packages/engine/src/run-audit.ts +++ b/packages/engine/src/run-audit.ts @@ -93,6 +93,7 @@ export type GitMutationType = | "worktree:remove" | "worktree:remove-fallback" | "worktree:remove-classified-harmless" + | "worktree:remove-classification-probe-failed" | "worktree:remove-leaked-registered-worktree" | "worktree:reuse" | "worktree:incomplete-detected" diff --git a/packages/engine/src/worktree-backend.ts b/packages/engine/src/worktree-backend.ts index b8caf8552d..56a5867cd3 100644 --- a/packages/engine/src/worktree-backend.ts +++ b/packages/engine/src/worktree-backend.ts @@ -95,20 +95,37 @@ async function classifyHarmlessMergeRemoveFailure(input: { const pathExists = existsSync(input.worktreePath); const gitFileExists = existsSync(resolve(input.worktreePath, ".git")); - await execAsync("git worktree prune", { - cwd: input.rootDir, - encoding: "utf-8", - timeout: NATIVE_TIMEOUT_MS, - maxBuffer: MAX_BUFFER, - }); + let stdout: string; + try { + await execAsync("git worktree prune", { + cwd: input.rootDir, + encoding: "utf-8", + timeout: NATIVE_TIMEOUT_MS, + maxBuffer: MAX_BUFFER, + }); - const listResult = await execAsync("git worktree list --porcelain", { - cwd: input.rootDir, - encoding: "utf-8", - timeout: 10_000, - maxBuffer: MAX_BUFFER, - }); - const stdout = typeof listResult === "string" ? listResult : String(listResult.stdout ?? ""); + const listResult = await execAsync("git worktree list --porcelain", { + cwd: input.rootDir, + encoding: "utf-8", + timeout: 10_000, + maxBuffer: MAX_BUFFER, + }); + stdout = typeof listResult === "string" ? listResult : String(listResult.stdout ?? ""); + } catch (probeError) { + await input.audit?.git({ + type: "worktree:remove-classification-probe-failed", + target: input.worktreePath, + metadata: { + taskId: input.taskId, + reason: input.reason, + stderrPreview, + probeError: previewError(probeError), + pathExists, + gitFileExists, + }, + }); + return null; + } const registeredAfterPrune = porcelainContainsWorktree(stdout, input.worktreePath); if (registeredAfterPrune) { From 834ea3c22778478850d8b2c7448ed8e6b2629244 Mon Sep 17 00:00:00 2001 From: Phil Larson Date: Thu, 11 Jun 2026 20:18:41 -0700 Subject: [PATCH 7/9] fix(FN-343): address follow-up review findings --- .../__tests__/workflow-flow-mapping.test.ts | 22 ++++++++++++++++++- .../app/components/workflow-flow-mapping.ts | 3 +++ .../worktrunk-worktree-removal.test.ts | 2 +- packages/engine/src/merger.ts | 14 +++++++----- packages/engine/src/worktree-backend.ts | 2 +- 5 files changed, 34 insertions(+), 9 deletions(-) diff --git a/packages/dashboard/app/components/__tests__/workflow-flow-mapping.test.ts b/packages/dashboard/app/components/__tests__/workflow-flow-mapping.test.ts index 8d52db7b60..1ffb1c4a8d 100644 --- a/packages/dashboard/app/components/__tests__/workflow-flow-mapping.test.ts +++ b/packages/dashboard/app/components/__tests__/workflow-flow-mapping.test.ts @@ -208,7 +208,18 @@ describe("workflow-flow-mapping v2 round-trip", () => { { id: "gate", kind: "merge-gate", column: "in-progress", config: { name: "Gate" } }, { id: "attempt", kind: "merge-attempt", column: "in-progress" }, { id: "hold", kind: "manual-merge-hold", column: "in-progress", config: { release: "manual" } }, - { id: "retry", kind: "retry-backoff", column: "in-progress", config: { maxIterations: 2, template: { nodes: [], edges: [] } } }, + { + id: "retry", + kind: "retry-backoff", + column: "in-progress", + config: { + maxIterations: 2, + template: { + nodes: [{ id: "retry-step", kind: "prompt", config: { prompt: "try again" } }], + edges: [{ from: "retry-step", to: "retry-step", condition: "retry", kind: "rework" }], + }, + }, + }, ] as WorkflowDefinition["ir"]["nodes"], edges: [], }; @@ -222,9 +233,18 @@ describe("workflow-flow-mapping v2 round-trip", () => { if (out.version !== "v2") throw new Error("expected v2"); const byId = Object.fromEntries(out.nodes.map((node) => [node.id, node])); expect(byId.gate.kind).toBe("merge-gate"); + expect(byId.gate.config?.name).toBe("Gate"); expect(byId.attempt.kind).toBe("merge-attempt"); expect(byId.hold.kind).toBe("manual-merge-hold"); + expect(byId.hold.config?.release).toBe("manual"); expect(byId.retry.kind).toBe("retry-backoff"); + expect(byId.retry.config).toEqual({ + maxIterations: 2, + template: { + nodes: [{ id: "retry-step", kind: "prompt", config: { prompt: "try again" } }], + edges: [{ from: "retry-step", to: "retry-step", condition: "retry", kind: "rework" }], + }, + }); }); it("emits swimlane band group nodes that flowToIr strips back out", () => { diff --git a/packages/dashboard/app/components/workflow-flow-mapping.ts b/packages/dashboard/app/components/workflow-flow-mapping.ts index 14532b17ff..6358adfad0 100644 --- a/packages/dashboard/app/components/workflow-flow-mapping.ts +++ b/packages/dashboard/app/components/workflow-flow-mapping.ts @@ -436,6 +436,9 @@ export function flowToIr( return { id: localId, kind: "prompt", config: { ...(config ?? {}), seam: "merge" } }; } if (data.kind === "foreach" || data.kind === "loop") { + if (originalKind && originalKind !== "foreach" && originalKind !== "loop") { + return { id: localId, kind: originalKind, config: config && Object.keys(config).length ? config : undefined }; + } // Reassemble the template from this group's children. const children = childrenByGroup.get(node.id) ?? []; const templateNodes: WorkflowIrNode[] = children.map((c) => { diff --git a/packages/engine/src/__tests__/reliability-interactions/worktrunk-worktree-removal.test.ts b/packages/engine/src/__tests__/reliability-interactions/worktrunk-worktree-removal.test.ts index 715756dc52..04f2827379 100644 --- a/packages/engine/src/__tests__/reliability-interactions/worktrunk-worktree-removal.test.ts +++ b/packages/engine/src/__tests__/reliability-interactions/worktrunk-worktree-removal.test.ts @@ -89,7 +89,7 @@ describe("reliability interactions: worktrunk worktree removal routing", () => { "git worktree list --porcelain", ]); expect(warnSpy).toHaveBeenCalledWith( - expect.stringContaining("post-merge worktree cleanup remove failed, but no registered worktree remains after prune"), + expect.stringContaining("post-merge worktree cleanup classified harmless"), ); }); diff --git a/packages/engine/src/merger.ts b/packages/engine/src/merger.ts index 79fe7deb54..1cb1390870 100644 --- a/packages/engine/src/merger.ts +++ b/packages/engine/src/merger.ts @@ -7312,6 +7312,7 @@ async function removePostMergeWorktree( postMergeWorktree: string, taskId: string, settings: Partial, + audit?: RunAuditor, ): Promise { try { const outcome = await removeWorktree({ @@ -7320,9 +7321,10 @@ async function removePostMergeWorktree( settings, taskId, reason: RemovalReason.MergerPostMerge, + audit, }); - if (outcome && "harmless" in outcome && outcome.harmless) { - mergerLog.warn(`${taskId}: post-merge worktree cleanup remove failed, but no registered worktree remains after prune for ${postMergeWorktree}: ${outcome.message}`); + if ("harmless" in outcome && outcome.harmless) { + mergerLog.warn(`${taskId}: post-merge worktree cleanup classified harmless for ${postMergeWorktree}: ${outcome.message}`); } } catch (err: unknown) { mergerLog.warn(`${taskId}: failed to remove post-merge worktree ${postMergeWorktree}: ${getCommandErrorMessage(err)}`); @@ -10520,7 +10522,7 @@ export async function aiMergeTask( // Non-fatal — task still moves to done } finally { if (postMergeWorktree) { - await removePostMergeWorktree(rootDir, postMergeWorktree, taskId, settings); + await removePostMergeWorktree(rootDir, postMergeWorktree, taskId, settings, audit); } } } @@ -10580,10 +10582,10 @@ export async function aiMergeTask( audit, reason: RemovalReason.MergerCleanup, }); - if (outcome && "harmless" in outcome && outcome.harmless) { - mergerLog.warn(`${taskId}: merge worktree cleanup remove failed, but no registered worktree remains after prune for ${worktreePath}: ${outcome.message}`); + if ("harmless" in outcome && outcome.harmless) { + mergerLog.warn(`${taskId}: merge worktree cleanup classified harmless for ${worktreePath}: ${outcome.message}`); } - result.worktreeRemoved = outcome?.removed ?? true; + result.worktreeRemoved = outcome.removed || ("harmless" in outcome && outcome.harmless); } if (result.worktreeRemoved) { try { diff --git a/packages/engine/src/worktree-backend.ts b/packages/engine/src/worktree-backend.ts index 56a5867cd3..55e85f4686 100644 --- a/packages/engine/src/worktree-backend.ts +++ b/packages/engine/src/worktree-backend.ts @@ -967,7 +967,7 @@ export async function removeWorktree(input: { liveOwnerProbe?: LiveBindingProbe; processActiveProbe?: ProcessActiveProbe; reconcileMinIdleMs?: number; -}): Promise { +}): Promise { const logger = { log: (_message: string): void => {}, warn: (_message: string): void => {}, From 3a729f53a9566f858b9bc9a86931fc3c48c08e47 Mon Sep 17 00:00:00 2001 From: Phil Larson Date: Thu, 11 Jun 2026 20:51:40 -0700 Subject: [PATCH 8/9] fix(FN-352): allow no-commit coordination completion --- .changeset/fn-352-no-commit-coordination.md | 5 + docs/task-management.md | 3 + .../app/components/workflow-flow-mapping.ts | 11 +- .../executor-task-done-invariant.test.ts | 118 +++++++++++++++++- packages/engine/src/executor.ts | 62 ++++++++- 5 files changed, 191 insertions(+), 8 deletions(-) create mode 100644 .changeset/fn-352-no-commit-coordination.md diff --git a/.changeset/fn-352-no-commit-coordination.md b/.changeset/fn-352-no-commit-coordination.md new file mode 100644 index 0000000000..3b216bd917 --- /dev/null +++ b/.changeset/fn-352-no-commit-coordination.md @@ -0,0 +1,5 @@ +--- +"@runfusion/fusion": patch +--- + +Allow narrowly-scoped Review Level 1 coordination tasks with board-only file scope and explicit no-source intent to complete without commits while preserving the missing-commit guard for implementation tasks. diff --git a/docs/task-management.md b/docs/task-management.md index bf30bdbf28..cefe2526f3 100644 --- a/docs/task-management.md +++ b/docs/task-management.md @@ -875,7 +875,10 @@ Use `noCommitsExpected: true` for tasks where the deliverable is a decision/repo - Meaning: executor allows `fn_task_done` with zero commits for that task. - Triage auto-sets it only when the task is clearly decision-shaped (e.g. "Decide whether...", "Evaluate...", "Verify...", "Audit...") with explicitly observational acceptance criteria and explicit no-code language. +- Review Level 1 coordination/routing tasks that are board-only, explicitly say not to change source, and scope only task documents/metadata can also complete without commits even if older prompts omitted the explicit flag. This fallback is intentionally narrow and exists to recover plan-only coordination work; it does not bypass wrong-worktree or wrong-branch checks. - Ambiguous/forked tasks (e.g. "Investigate..." or "Investigate and fix if needed") leave it unset by default. +- Implementation, feature, bug-fix, source-docs, test, config, or broad investigation tasks still require commits unless they have an explicit and valid no-commit contract. +- If a legacy coordination task is stuck with `fn_task_done refused: no_commits`, prefer setting/verifying `noCommitsExpected` and re-running normal no-op finalization rather than editing `.fusion/fusion.db` directly. - You can manually set/clear it in Task Detail via **No commits expected (decision-only task)**. - Task cards show a **decision-only** badge when enabled. - Finalization still uses the existing no-op review/merge path (`mergeDetails.noOpMerge: true`, `mergeConfirmed: true`); no synthetic merge strategy values are introduced. diff --git a/packages/dashboard/app/components/workflow-flow-mapping.ts b/packages/dashboard/app/components/workflow-flow-mapping.ts index 6358adfad0..a4f1d928f9 100644 --- a/packages/dashboard/app/components/workflow-flow-mapping.ts +++ b/packages/dashboard/app/components/workflow-flow-mapping.ts @@ -226,7 +226,7 @@ function foreachConfigOf(node: WorkflowIrNode): WorkflowForeachConfig | undefine } function loopConfigOf(node: WorkflowIrNode): WorkflowLoopConfig | undefined { - if (node.kind !== "loop") return undefined; + if (node.kind !== "loop" && node.kind !== "retry-backoff") return undefined; const cfg = node.config as Partial | undefined; if (!cfg || !cfg.template) return undefined; return cfg as WorkflowLoopConfig; @@ -375,7 +375,11 @@ export function irToFlow(def: WorkflowDefinition): { function nodeConfig(node: FlowNode): Record | undefined { const data = node.data; const config: Record = { ...(data.config ?? {}) }; - const fallbackLabel = data.kind === "merge" ? "Merge boundary" : node.id; + const fallbackLabel = data.kind === "merge" + ? "Merge boundary" + : node.parentId + ? templateNodeIdFromChild(node.parentId, node.id) + : node.id; if (data.kind !== "start" && data.kind !== "end" && data.label && data.label !== fallbackLabel) { config.name = data.label; } else { @@ -436,9 +440,6 @@ export function flowToIr( return { id: localId, kind: "prompt", config: { ...(config ?? {}), seam: "merge" } }; } if (data.kind === "foreach" || data.kind === "loop") { - if (originalKind && originalKind !== "foreach" && originalKind !== "loop") { - return { id: localId, kind: originalKind, config: config && Object.keys(config).length ? config : undefined }; - } // Reassemble the template from this group's children. const children = childrenByGroup.get(node.id) ?? []; const templateNodes: WorkflowIrNode[] = children.map((c) => { diff --git a/packages/engine/src/__tests__/executor-task-done-invariant.test.ts b/packages/engine/src/__tests__/executor-task-done-invariant.test.ts index dcf3710d2b..0670657153 100644 --- a/packages/engine/src/__tests__/executor-task-done-invariant.test.ts +++ b/packages/engine/src/__tests__/executor-task-done-invariant.test.ts @@ -48,7 +48,7 @@ async function setup(overrides: Record = {}) { }); const executor = new TaskExecutor(store as any, "/repo"); - await executor.execute(baseTask() as any); + await executor.execute(task as any); return { store, tool, setTask: (next: any) => (task = { ...task, ...next }) }; } @@ -111,6 +111,122 @@ describe("FN-4114 fn_task_done invariants", () => { expect(store.moveTask).toHaveBeenCalledWith("FN-4114", "todo", { preserveProgress: true }); }); + it("FN-350 allows Review Level 1 coordination completion with zero commits when no source files are scoped", async () => { + const fn350Prompt = `# Task: FN-350 - Route Ready Swift Tasks to Executor Owner + +**Created:** 2026-06-12 +**Size:** S + +## Review Level: 1 (Plan Only) + +**Assessment:** This is a coordination/routing task that should not change product source, but it can affect execution ordering and owner assignment for active Swift implementation work. Risk is low if the executor follows the existing coordinator handoff policy, routes at most one existing ready task, and records clear evidence instead of creating duplicate implementation work. + +## Mission + +Route exactly one existing ready Swift implementation task to the durable executor owner, or record the intentional block if no safe candidate exists. Do not change product source. + +## File Scope + +Atlas Notes task-board artifacts only: + +- FN-350 task document \`docs\` via \`fn_task_document_write\` +- Board task metadata and logs via Fusion task tools + +## Steps + +### Step 0: Preflight +- [x] Required board records exist. + +### Step 1: Re-check live candidate readiness +- [x] Candidate readiness inspected. + +### Step 2: Select exactly one routing action +- [x] One routing action selected. + +### Step 3: Perform safe routing or record intentional block +- [x] Routing evidence recorded. + +### Step 4: Testing & Verification +- [x] Board-only verification recorded. + +### Step 5: Documentation & Delivery +- [x] Final documentation saved. + +## Do NOT + +- Do not edit product source. +- Do not create duplicate implementation tasks. +`; + const { store, tool } = await setup({ + id: "FN-350", + title: "Route Ready Swift Tasks to Executor Owner", + description: "Coordination/routing task with task-document evidence only.", + prompt: fn350Prompt, + branch: "fusion/fn-350", + noCommitsExpected: undefined, + steps: [ + { name: "Preflight", status: "done" as const }, + { name: "Re-check live candidate readiness", status: "done" as const }, + { name: "Select exactly one routing action", status: "done" as const }, + { name: "Perform safe routing or record intentional block", status: "done" as const }, + { name: "Testing & Verification", status: "done" as const }, + { name: "Documentation & Delivery", status: "in-progress" as const }, + ], + currentStep: 5, + }); + mockedExecSync.mockImplementation((cmd: string) => { + if (cmd.includes("rev-parse --show-toplevel")) return Buffer.from("/repo/.worktrees/swift-falcon\n"); + if (cmd.includes("rev-parse --abbrev-ref HEAD")) return Buffer.from("fusion/fn-350\n"); + if (cmd.includes("rev-list --count")) return Buffer.from("0\n"); + if (cmd.includes("rev-parse HEAD")) return Buffer.from("def456\n"); + return Buffer.from(""); + }); + + store.moveTask.mockClear(); + const result = await tool.execute("id", { summary: "Recorded routing evidence in task documents and logs." }); + + expect(result.content[0].text).toContain("Task marked complete"); + expect(result.content[0].text).not.toContain("fn_task_done refused: no_commits"); + expect(store.moveTask.mock.calls).toEqual([["FN-350", "in-progress"]]); + expect(store.handoffToReview).not.toHaveBeenCalled(); + }); + + it("FN-4114 still refuses source-changing implementation tasks with zero commits and no explicit no-commit contract", async () => { + const implementationPrompt = `# Task: FN-4114 - Implement source change + +**Size:** M + +## Review Level: 2 (Plan and Code) + +## Mission + +Implement a bug fix in the engine. + +## File Scope + +- packages/engine/src/executor.ts +- packages/engine/src/__tests__/executor-task-done-invariant.test.ts + +## Steps + +### Step 1: Implement +- [ ] Change source code and tests. +`; + const { store, tool } = await setup({ prompt: implementationPrompt, noCommitsExpected: undefined }); + mockedExecSync.mockImplementation((cmd: string) => { + if (cmd.includes("rev-parse --show-toplevel")) return Buffer.from("/repo/.worktrees/swift-falcon\n"); + if (cmd.includes("rev-parse --abbrev-ref HEAD")) return Buffer.from("fusion/fn-4114\n"); + if (cmd.includes("rev-list --count")) return Buffer.from("0\n"); + if (cmd.includes("rev-parse HEAD")) return Buffer.from("def456\n"); + return Buffer.from(""); + }); + + const result = await tool.execute("id", {}); + + expect(result.content[0].text).toContain("fn_task_done refused: no_commits"); + expect(store.moveTask).toHaveBeenCalledWith("FN-4114", "todo", { preserveProgress: true }); + }); + it("FN-4114 allows no-commit completion when noCommitsExpected is true", async () => { const { store, tool } = await setup({ noCommitsExpected: true }); mockedExecSync.mockImplementation((cmd: string) => { diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index 7ce9123200..1ad795e847 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -231,6 +231,63 @@ export { const yieldEventLoop = (): Promise => new Promise((resolve) => setImmediateCb(resolve)); +function getPromptSection(prompt: string, heading: string): string { + const escapedHeading = heading.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + const match = prompt.match(new RegExp(`^##\\s+${escapedHeading}\\s*$([\\s\\S]*?)(?=^##\\s+|$(?![\\s\\S]))`, "im")); + return match?.[1]?.trim() ?? ""; +} + +function promptDeclaresReviewLevelOnePlanOnly(prompt: string): boolean { + return /^##\s+Review Level:\s*1\b[^\n]*\bPlan Only\b/im.test(prompt); +} + +function promptDeclaresNoSourceChangeIntent(prompt: string): boolean { + const normalized = prompt.toLowerCase(); + return [ + /should\s+not\s+change\s+(?:product\s+)?source/, + /do\s+not\s+(?:edit|modify|change)\s+(?:product\s+)?source/, + /no\s+(?:source|code)\s+changes?\s+(?:are\s+)?(?:expected|required|needed|allowed)/, + /must\s+not\s+(?:edit|modify|change)\s+(?:product\s+)?(?:source|code)/, + ].some((pattern) => pattern.test(normalized)); +} + +function promptLooksCoordinationOnly(prompt: string): boolean { + const titleMatch = prompt.match(/^#\s+Task:\s+[^\n]+/im)?.[0] ?? ""; + const mission = getPromptSection(prompt, "Mission"); + const assessment = prompt.match(/^\*\*Assessment:\*\*\s*([^\n]+)/im)?.[1] ?? ""; + const coordinationText = `${titleMatch}\n${mission}\n${assessment}`.toLowerCase(); + const hasCoordinationIntent = /\b(coordination|routing|route|handoff|assign(?:ment)?|owner|triage|select exactly one|record (?:the )?intentional block)\b/.test(coordinationText); + const hasImplementationDirective = /\b(implement|fix|add|change|modify|refactor|build|create|delete|remove)\b/.test(mission.toLowerCase()) && !/record (?:the )?intentional block/.test(mission.toLowerCase()); + return hasCoordinationIntent && !hasImplementationDirective; +} + +function promptFileScopeIsBoardOnly(prompt: string): boolean { + const fileScope = getPromptSection(prompt, "File Scope"); + if (!fileScope.trim()) return false; + const normalized = fileScope.toLowerCase(); + const sourcePathPattern = /(?:^|[\s`'"(])(?:packages|src|source|sources|app|apps|lib|libs|components|scripts|docs|\.github|config|test|tests|__tests__)\//m; + const sourceExtensionPattern = /\.(?:ts|tsx|js|jsx|mjs|cjs|swift|kt|java|py|go|rs|rb|php|cs|cpp|c|h|hpp|json|ya?ml|toml|mdx?|css|scss|html|sql|sh)\b/m; + if (sourcePathPattern.test(normalized) || sourceExtensionPattern.test(normalized)) return false; + const allowedBoardOnlyPattern = /\b(task[- ]?board|board task|task document|task documents|task metadata|task logs|fusion task tools|fn_task_|\.fusion\/tasks|attachments?)\b/; + return allowedBoardOnlyPattern.test(normalized); +} + +function getNoCommitEligibilityReason(task: Task): "explicit noCommitsExpected=true" | "prompt-derived coordination-only no-source scope" | null { + if (task.noCommitsExpected === true) return "explicit noCommitsExpected=true"; + const rawPrompt = (task as { prompt?: unknown }).prompt; + const prompt = typeof rawPrompt === "string" ? rawPrompt : ""; + if (!prompt.trim()) return null; + if ( + promptDeclaresReviewLevelOnePlanOnly(prompt) && + promptLooksCoordinationOnly(prompt) && + promptDeclaresNoSourceChangeIntent(prompt) && + promptFileScopeIsBoardOnly(prompt) + ) { + return "prompt-derived coordination-only no-source scope"; + } + return null; +} + /** * How long to wait after engine startup before spawning AI agent sessions for * orphaned in-progress tasks. The work itself (worktree setup, pi-coding-agent @@ -9419,8 +9476,9 @@ export class TaskExecutor { }; } - if (task.noCommitsExpected === true) { - executorLog.log(`${task.id}: fn_task_done no_commits guard skipped (noCommitsExpected=true)`); + const noCommitEligibilityReason = getNoCommitEligibilityReason(task); + if (noCommitEligibilityReason) { + executorLog.log(`${task.id}: fn_task_done no_commits guard skipped (${noCommitEligibilityReason})`); return { ok: true }; } From c6bcbe26a2429139de2995c29a9cc17708bff262 Mon Sep 17 00:00:00 2001 From: Phil Larson Date: Fri, 12 Jun 2026 15:54:47 -0700 Subject: [PATCH 9/9] fix(FN-352): address no-commit review feedback --- .changeset/fn-343-merge-worktree-cleanup.md | 2 +- packages/core/src/types.ts | 2 + .../executor-task-done-invariant.test.ts | 42 +++++++++++++++++++ packages/engine/src/executor.ts | 10 +++-- 4 files changed, 52 insertions(+), 4 deletions(-) diff --git a/.changeset/fn-343-merge-worktree-cleanup.md b/.changeset/fn-343-merge-worktree-cleanup.md index 2b6d3982fc..d5386b6638 100644 --- a/.changeset/fn-343-merge-worktree-cleanup.md +++ b/.changeset/fn-343-merge-worktree-cleanup.md @@ -1,5 +1,5 @@ --- -"@fusion/engine": patch +"@runfusion/fusion": patch --- Classify harmless temporary merge worktree cleanup failures after `git worktree prune`/porcelain inspection while keeping still-registered worktree leaks visible in merger diagnostics. diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index 12bad910f7..125f458a40 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -2368,6 +2368,8 @@ export interface Task { sourceMessageId?: string; sourceParentTaskId?: string; sourceMetadata?: Record; + /** Reconstructed task prompt content when available on in-memory execution tasks. */ + prompt?: string; /** Explicitly assigned user ID for task-user linking. Used during review handoff to indicate * which user should review the task. The sentinel value "requesting-user" indicates the * user who created or steered the task. */ diff --git a/packages/engine/src/__tests__/executor-task-done-invariant.test.ts b/packages/engine/src/__tests__/executor-task-done-invariant.test.ts index 0670657153..9c3d68a9b6 100644 --- a/packages/engine/src/__tests__/executor-task-done-invariant.test.ts +++ b/packages/engine/src/__tests__/executor-task-done-invariant.test.ts @@ -191,6 +191,48 @@ Atlas Notes task-board artifacts only: expect(store.handoffToReview).not.toHaveBeenCalled(); }); + it("FN-350 refuses contradictory implementation plus coordination fallback prompts", async () => { + const prompt = `# Task: FN-350 - Route Ready Swift Tasks to Executor Owner + +## Review Level: 1 (Plan Only) + +**Assessment:** This is a coordination/routing task that should not change product source. + +## Mission +Implement the source fix if possible, or record the intentional block if no safe candidate exists. Do not change product source. + +## File Scope + +- FN-350 task document \`docs\` via \`fn_task_document_write\` + +## Steps + +### Step 1: Decide +- [x] Decision recorded. +`; + const { store, tool } = await setup({ + id: "FN-350", + title: "Route Ready Swift Tasks to Executor Owner", + description: "Coordination/routing task with task-document evidence only.", + prompt, + branch: "fusion/fn-350", + noCommitsExpected: undefined, + steps: [{ name: "Decide", status: "done" as const }], + }); + mockedExecSync.mockImplementation((cmd: string) => { + if (cmd.includes("rev-parse --show-toplevel")) return Buffer.from("/repo/.worktrees/swift-falcon\n"); + if (cmd.includes("rev-parse --abbrev-ref HEAD")) return Buffer.from("fusion/fn-350\n"); + if (cmd.includes("rev-list --count")) return Buffer.from("0\n"); + if (cmd.includes("rev-parse HEAD")) return Buffer.from("def456\n"); + return Buffer.from(""); + }); + + const result = await tool.execute("id", {}); + + expect(result.content[0].text).toContain("fn_task_done refused: no_commits"); + expect(store.moveTask).toHaveBeenCalledWith("FN-350", "todo", { preserveProgress: true }); + }); + it("FN-4114 still refuses source-changing implementation tasks with zero commits and no explicit no-commit contract", async () => { const implementationPrompt = `# Task: FN-4114 - Implement source change diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index 1ad795e847..8450ae2bcd 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -257,7 +257,11 @@ function promptLooksCoordinationOnly(prompt: string): boolean { const assessment = prompt.match(/^\*\*Assessment:\*\*\s*([^\n]+)/im)?.[1] ?? ""; const coordinationText = `${titleMatch}\n${mission}\n${assessment}`.toLowerCase(); const hasCoordinationIntent = /\b(coordination|routing|route|handoff|assign(?:ment)?|owner|triage|select exactly one|record (?:the )?intentional block)\b/.test(coordinationText); - const hasImplementationDirective = /\b(implement|fix|add|change|modify|refactor|build|create|delete|remove)\b/.test(mission.toLowerCase()) && !/record (?:the )?intentional block/.test(mission.toLowerCase()); + const missionLower = mission.toLowerCase() + .replace(/do\s+not\s+(?:edit|modify|change)\s+(?:product\s+)?source/g, "") + .replace(/should\s+not\s+change\s+(?:product\s+)?source/g, "") + .replace(/must\s+not\s+(?:edit|modify|change)\s+(?:product\s+)?(?:source|code)/g, ""); + const hasImplementationDirective = /\b(implement|fix|add|change|modify|refactor|build|create|delete|remove)\b/.test(missionLower); return hasCoordinationIntent && !hasImplementationDirective; } @@ -268,13 +272,13 @@ function promptFileScopeIsBoardOnly(prompt: string): boolean { const sourcePathPattern = /(?:^|[\s`'"(])(?:packages|src|source|sources|app|apps|lib|libs|components|scripts|docs|\.github|config|test|tests|__tests__)\//m; const sourceExtensionPattern = /\.(?:ts|tsx|js|jsx|mjs|cjs|swift|kt|java|py|go|rs|rb|php|cs|cpp|c|h|hpp|json|ya?ml|toml|mdx?|css|scss|html|sql|sh)\b/m; if (sourcePathPattern.test(normalized) || sourceExtensionPattern.test(normalized)) return false; - const allowedBoardOnlyPattern = /\b(task[- ]?board|board task|task document|task documents|task metadata|task logs|fusion task tools|fn_task_|\.fusion\/tasks|attachments?)\b/; + const allowedBoardOnlyPattern = /(?:^|[^\w/])(?:task[- ]?board|board task|task document|task documents|task metadata|task logs|fusion task tools|fn_task_[\w-]*|\.fusion\/tasks|attachments?)(?=$|[^\w/-])/; return allowedBoardOnlyPattern.test(normalized); } function getNoCommitEligibilityReason(task: Task): "explicit noCommitsExpected=true" | "prompt-derived coordination-only no-source scope" | null { if (task.noCommitsExpected === true) return "explicit noCommitsExpected=true"; - const rawPrompt = (task as { prompt?: unknown }).prompt; + const rawPrompt = task.prompt; const prompt = typeof rawPrompt === "string" ? rawPrompt : ""; if (!prompt.trim()) return null; if (