From 60081fb1f4253852a6c4a8b0e88cdd0d60e31434 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Sun, 5 Jul 2026 21:04:45 -0700 Subject: [PATCH] FN-7610: route workspace-mode tasks around PR-merge auto-merge strategy Fixes workspace-mode (workspaceWorktrees) tasks failing auto-merge under mergeStrategy=pull-request, where processPullRequestMergeTask threw "could not determine repository" because the workspace root is a container of independent git sub-repos, not itself a git repo. - Hoist an isWorkspaceTask check in ProjectEngine's merge dispatch (project-engine.ts) before the mergeStrategy branch, so workspace tasks always fall through to the existing direct/landWorkspaceTask path regardless of configured mergeStrategy. - Add processPullRequestMergeTask and syncGroupPrCallback defense-in-depth guards (task-lifecycle.ts) that throw the new named WorkspaceTaskMergeError if a workspace task ever reaches the PR-merge path. - Add engine tests covering multi-repo, single-repo, and zero-commit no-op workspace tasks under mergeStrategy=pull-request, plus a non-regression test for the legacy single-worktree PR path. - Add CLI tests asserting the new guards throw WorkspaceTaskMergeError. - Add a patch changeset describing the fix. Files changed: .changeset/fn-7610-workspace-pr-merge-routing.md | 7 ++ .../src/commands/__tests__/task-lifecycle.test.ts | 56 +++++++++ packages/cli/src/commands/task-lifecycle.ts | 33 ++++- .../engine/src/__tests__/project-engine.test.ts | 140 +++++++++++++++++++++ packages/engine/src/project-engine.ts | 18 ++- 5 files changed, 252 insertions(+), 2 deletions(-) Fusion-Task-Id: FN-7610 Fusion-Task-Lineage: 31768b77-d9a9-4a79-a055-bbc6b228a1c4 Co-authored-by: Fusion (runfusion.ai) --- .../fn-7610-workspace-pr-merge-routing.md | 7 + .../commands/__tests__/task-lifecycle.test.ts | 56 +++++++ packages/cli/src/commands/task-lifecycle.ts | 33 ++++- .../src/__tests__/project-engine.test.ts | 140 ++++++++++++++++++ packages/engine/src/project-engine.ts | 18 ++- 5 files changed, 252 insertions(+), 2 deletions(-) create mode 100644 .changeset/fn-7610-workspace-pr-merge-routing.md diff --git a/.changeset/fn-7610-workspace-pr-merge-routing.md b/.changeset/fn-7610-workspace-pr-merge-routing.md new file mode 100644 index 0000000000..53a8deb789 --- /dev/null +++ b/.changeset/fn-7610-workspace-pr-merge-routing.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Fix workspace-mode tasks failing auto-merge under the pull-request merge strategy. +category: fix +dev: Engine merge dispatch now checks isWorkspaceTask before the mergeStrategy branch, routing workspace tasks to landWorkspaceTask instead of processPullRequestMerge (which threw "could not determine repository" against the non-git workspace root). processPullRequestMergeTask/syncGroupPrCallback now throw the named WorkspaceTaskMergeError for workspace tasks as defense-in-depth. diff --git a/packages/cli/src/commands/__tests__/task-lifecycle.test.ts b/packages/cli/src/commands/__tests__/task-lifecycle.test.ts index f6d148c97b..a3b1fa2335 100644 --- a/packages/cli/src/commands/__tests__/task-lifecycle.test.ts +++ b/packages/cli/src/commands/__tests__/task-lifecycle.test.ts @@ -463,6 +463,43 @@ describe("processPullRequestMergeTask", () => { expect(github.createPr).not.toHaveBeenCalled(); }); + // FNXC:Workspace 2026-07-05-00:00 (FN-7610, defense-in-depth): + // A workspace-mode task (non-empty workspaceWorktrees) must never reach + // getCurrentRepo here — the engine merge dispatch is the primary fix that + // routes workspace tasks around this function entirely, but if a future + // caller forgets that guard, this must fail with the named + // WorkspaceTaskMergeError BEFORE getCurrentRepo is even called (never the + // generic "could not determine repository"). + it("rejects with the named WorkspaceTaskMergeError for a workspace-mode task, before resolving the repository", async () => { + const getCurrentRepoMock = vi.mocked(getCurrentRepo); + getCurrentRepoMock.mockClear(); + const task: MockTask & { workspaceWorktrees: Record } = { + id: "FN-7610-WS", + title: "test", + description: "desc", + column: "in-review", + workspaceWorktrees: { + "repo-a": { worktreePath: "/tmp/a", branch: "fusion/fn-7610-ws-a" }, + }, + }; + const store = makeStore(task as never); + const github = { + findPrForBranch: vi.fn(), + createPr: vi.fn(), + getPrMergeStatus: vi.fn(), + mergePr: vi.fn(), + }; + + const rejection = processPullRequestMergeTask(store as never, "/repo", task.id, github as never, () => undefined); + await expect(rejection).rejects.toMatchObject({ name: "WorkspaceTaskMergeError" }); + await expect(rejection).rejects.not.toThrow("could not determine repository"); + + expect(getCurrentRepoMock).not.toHaveBeenCalled(); + expect(github.getPrMergeStatus).not.toHaveBeenCalled(); + expect(github.findPrForBranch).not.toHaveBeenCalled(); + expect(github.createPr).not.toHaveBeenCalled(); + }); + it("finalizes branch group and member tasks when shared group PR is already merged", async () => { const taskA: MockTask = { id: "FN-9015", @@ -1463,6 +1500,25 @@ describe("syncGroupPrCallback (U6)", () => { await expect(sync({ cwd: "/tmp/project", group: { ...group, prNumber: undefined } as never, members })).rejects.toThrow(/no persisted prNumber/); }); + // FNXC:Workspace 2026-07-05-00:00 (FN-7610, defense-in-depth): + // A workspace-mode shared-group member has no single git repo to resolve a + // PR against here. Assert the named WorkspaceTaskMergeError fires BEFORE + // getPrStatus/getCurrentRepo resolution is attempted. + it("rejects with the named WorkspaceTaskMergeError when a group member is a workspace-mode task, before resolving the repository", async () => { + const getCurrentRepoMock = vi.mocked(getCurrentRepo); + getCurrentRepoMock.mockClear(); + const workspaceMembers = [ + { id: "FN-A", title: "Alpha" }, + { id: "FN-B", title: "Beta", workspaceWorktrees: { "repo-a": { worktreePath: "/tmp/a", branch: "fusion/fn-b-a" } } }, + ] as never[]; + const github = { getPrStatus: vi.fn(), updatePr: vi.fn() }; + const sync = syncGroupPrCallback(github as never); + const rejection = sync({ cwd: "/tmp/project", group: group as never, members: workspaceMembers }); + await expect(rejection).rejects.toMatchObject({ name: "WorkspaceTaskMergeError" }); + expect(getCurrentRepoMock).not.toHaveBeenCalled(); + expect(github.getPrStatus).not.toHaveBeenCalled(); + }); + /* FNXC:BranchGroupCompletion 2026-07-04-00:00: FN-7532 surface-parity regression: the PR-body checklist must use the SAME diff --git a/packages/cli/src/commands/task-lifecycle.ts b/packages/cli/src/commands/task-lifecycle.ts index a4c9006121..1f90e5907f 100644 --- a/packages/cli/src/commands/task-lifecycle.ts +++ b/packages/cli/src/commands/task-lifecycle.ts @@ -24,7 +24,15 @@ const execAsync = promisify(exec); const execFileAsync: (file: string, args: string[], opts?: import("node:child_process").ExecFileOptions) => Promise<{ stdout: string; stderr: string }> = (file, args, opts) => (promisify(childProcess.execFile) as (f: string, a: string[], o?: object) => Promise<{ stdout: string; stderr: string }>)(file, args, opts); import type { TaskStore } from "@fusion/core"; -import { resolveTaskMergeTarget, getCurrentRepo, isBranchGroupMemberLanded, resolveEffectiveSettings } from "@fusion/core"; +import { + resolveTaskMergeTarget, + getCurrentRepo, + isBranchGroupMemberLanded, + resolveEffectiveSettings, + isWorkspaceTask, + assertNotWorkspaceTaskMerge, + WorkspaceTaskMergeError, +} from "@fusion/core"; import type { Settings, TaskDetail, PrInfo, MergeResult, BranchGroup, BranchGroupPrState, Task } from "@fusion/core"; import { activeSessionRegistry, resolveIntegrationBranch } from "@fusion/engine"; import type { @@ -312,6 +320,18 @@ export function syncGroupPrCallback( if (group.prNumber == null) { throw new Error(`syncGroupPr: group ${group.id} has no persisted prNumber`); } + // FNXC:Workspace 2026-07-05-00:00 (FN-7610, defense-in-depth): + // A workspace-mode task (non-empty workspaceWorktrees) as a shared-group + // member has no single git repo to resolve a PR against here — the primary + // fix routes workspace tasks around the PR-merge branch entirely in the + // engine dispatch (project-engine.ts drainMergeQueue), but this callback + // must fail loudly with the named WorkspaceTaskMergeError (not the generic + // "could not determine repository") if it is ever reached for one anyway. + if (members.some((m) => isWorkspaceTask(m))) { + throw new WorkspaceTaskMergeError( + `syncGroupPr: group ${group.id} has a workspace-mode member; group PR sync is not supported for workspace tasks`, + ); + } // T4: resolve the repo from the PROJECT cwd, not the process cwd. In a // multi-project daemon the process cwd is not the project dir, so // `getCurrentRepo()` (no arg) would resolve the wrong repository. @@ -667,6 +687,17 @@ export async function processPullRequestMergeTask( return "skipped"; } + // FNXC:Workspace 2026-07-05-00:00 (FN-7610, defense-in-depth): + // The engine merge dispatch (project-engine.ts drainMergeQueue) is the + // primary fix: it hoists an isWorkspaceTask check before the mergeStrategy + // branch so a workspace-mode task never reaches this function under + // mergeStrategy:"pull-request". This assert is a second line of defense — + // any future caller that forgets that guard fails with the named, + // actionable WorkspaceTaskMergeError instead of the generic + // "could not determine repository" (the workspace root is a plain container + // of independent git sub-repos, not itself a git repo). + assertNotWorkspaceTaskMerge(task); + /* * FNXC:PrMergeAutoMerge 2026-06-27-13:14: * FN-7133 requires PR-mode merge status to resolve owner/repo from the project cwd because multi-project daemons cannot rely on process cwd. Never pass branch names into gh pr view --repo; getPrMergeStatus forwards its first two args as the repository slug. diff --git a/packages/engine/src/__tests__/project-engine.test.ts b/packages/engine/src/__tests__/project-engine.test.ts index 160fd6fa2e..796b295c22 100644 --- a/packages/engine/src/__tests__/project-engine.test.ts +++ b/packages/engine/src/__tests__/project-engine.test.ts @@ -1373,6 +1373,146 @@ describe("ProjectEngine U0 merge unification dispatch", () => { expect(result.merged).toBe(true); await engine.stop(); }); + + // FNXC:Workspace 2026-07-05-00:00 (FN-7610): + // A workspace-mode task must route to landWorkspaceTask EVEN WHEN the project + // is configured with mergeStrategy:"pull-request" (getMergeStrategy resolves + // "pull-request") — the engine dispatch hoists an isWorkspaceTask check before + // the mergeStrategy branch so processPullRequestMerge (which would call + // getCurrentRepo against the non-git workspace root and throw "could not + // determine repository") is never reached for workspace tasks. Covers + // multi-repo, single-repo, and true-zero-commit no-op variants, plus asserts + // no regression to the legacy singular-worktree PR path. + describe("workspace tasks bypass PR-merge strategy (FN-7610)", () => { + it("multi-repo workspaceWorktrees + mergeStrategy=pull-request routes to landWorkspaceTask, never processPullRequestMerge", async () => { + const mockStore = createMockStore({ ...baseSettings, autoMerge: true }); + mockStore.store.getTask.mockResolvedValue({ + id: "FN-WS-PR-MULTI", + column: "in-review", + paused: false, + mergeRetries: 0, + status: "queued", + branch: "fusion/fn-ws-pr-multi", + workspaceWorktrees: { + "repo-a": { worktreePath: "/tmp/a", branch: "fusion/fn-ws-pr-multi-a" }, + "repo-b": { worktreePath: "/tmp/b", branch: "fusion/fn-ws-pr-multi-b" }, + }, + } as any); + mocks.currentStore = mockStore.store; + mocks.landWorkspaceTask.mockResolvedValue({ + allLanded: true, + repos: [ + { repo: "repo-a", status: "landed", landedSha: "aaaa1111", integrationBranch: "main" }, + { repo: "repo-b", status: "landed", landedSha: "bbbb2222", integrationBranch: "main" }, + ], + } as any); + + const processPullRequestMerge = vi.fn(async () => "merged" as const); + const engine = createEngine({ processPullRequestMerge, getMergeStrategy: () => "pull-request" }); + await engine.start(); + const result = await engine.onMerge("FN-WS-PR-MULTI"); + expect(processPullRequestMerge).not.toHaveBeenCalled(); + expect(mocks.landWorkspaceTask).toHaveBeenCalled(); + expect(result.merged).toBe(true); + await engine.stop(); + }); + + it("single-key workspaceWorktrees + mergeStrategy=pull-request routes to landWorkspaceTask, never processPullRequestMerge", async () => { + const mockStore = createMockStore({ ...baseSettings, autoMerge: true }); + mockStore.store.getTask.mockResolvedValue({ + id: "FN-WS-PR-SINGLE", + column: "in-review", + paused: false, + mergeRetries: 0, + status: "queued", + branch: "fusion/fn-ws-pr-single", + workspaceWorktrees: { + "repo-c": { worktreePath: "/tmp/c", branch: "fusion/fn-ws-pr-single-c" }, + }, + } as any); + mocks.currentStore = mockStore.store; + mocks.landWorkspaceTask.mockResolvedValue({ + allLanded: true, + repos: [{ repo: "repo-c", status: "landed", landedSha: "cccc3333", integrationBranch: "main" }], + } as any); + + const processPullRequestMerge = vi.fn(async () => "merged" as const); + const engine = createEngine({ processPullRequestMerge, getMergeStrategy: () => "pull-request" }); + await engine.start(); + const result = await engine.onMerge("FN-WS-PR-SINGLE"); + expect(processPullRequestMerge).not.toHaveBeenCalled(); + expect(mocks.landWorkspaceTask).toHaveBeenCalled(); + expect(result.merged).toBe(true); + await engine.stop(); + }); + + it("true-zero-commit no-op workspace task under mergeStrategy=pull-request finalizes without calling processPullRequestMerge and does not park failed", async () => { + const mockStore = createMockStore({ ...baseSettings, autoMerge: true }); + mockStore.store.getTask.mockResolvedValue({ + id: "FN-WS-PR-NOOP", + column: "in-review", + paused: false, + mergeRetries: 0, + status: "queued", + branch: "fusion/fn-ws-pr-noop", + noCommitsExpected: true, + workspaceWorktrees: { + "repo-d": { worktreePath: "/tmp/d", branch: "fusion/fn-ws-pr-noop-d" }, + }, + } as any); + mocks.currentStore = mockStore.store; + // All repos land with no real commit (empty/no-op) — landWorkspaceTask still + // reports allLanded:true and finalizes gracefully. + mocks.landWorkspaceTask.mockResolvedValue({ + allLanded: true, + repos: [{ repo: "repo-d", status: "empty", integrationBranch: "main" }], + } as any); + + const processPullRequestMerge = vi.fn(async () => "merged" as const); + const engine = createEngine({ processPullRequestMerge, getMergeStrategy: () => "pull-request" }); + await engine.start(); + const result = await engine.onMerge("FN-WS-PR-NOOP"); + expect(processPullRequestMerge).not.toHaveBeenCalled(); + expect(mocks.landWorkspaceTask).toHaveBeenCalled(); + expect(result.ok).not.toBe(false); + await engine.stop(); + }); + + it("non-workspace task under mergeStrategy=pull-request still uses the PR path (no regression)", async () => { + const mockStore = createMockStore({ ...baseSettings, autoMerge: true }); + mockStore.store.getTask + .mockResolvedValueOnce({ + id: "FN-WS-PR-LEGACY", + column: "in-review", + paused: false, + mergeRetries: 0, + status: null, + branch: "fusion/fn-ws-pr-legacy", + }) + .mockResolvedValue({ + id: "FN-WS-PR-LEGACY", + column: "done", + paused: false, + mergeRetries: 0, + status: null, + branch: "fusion/fn-ws-pr-legacy", + mergeDetails: { mergeConfirmed: true, mergedAt: "2026-07-05T00:00:00.000Z", mergeTargetBranch: "main" }, + }); + mocks.currentStore = mockStore.store; + + const processPullRequestMerge = vi.fn(async () => "merged" as const); + const engine = createEngine({ processPullRequestMerge, getMergeStrategy: () => "pull-request" }); + await engine.start(); + engine.enqueueMerge("FN-WS-PR-LEGACY"); + + await vi.waitFor(() => { + expect(processPullRequestMerge).toHaveBeenCalled(); + }); + expect(mocks.landWorkspaceTask).not.toHaveBeenCalled(); + + await engine.stop(); + }); + }); }); /* diff --git a/packages/engine/src/project-engine.ts b/packages/engine/src/project-engine.ts index b741227e4d..730a3dea6b 100644 --- a/packages/engine/src/project-engine.ts +++ b/packages/engine/src/project-engine.ts @@ -2922,7 +2922,23 @@ export class ProjectEngine { } }; - if (mergeStrategy === "pull-request" && this.options.processPullRequestMerge) { + // FNXC:Workspace 2026-07-05-00:00 (FN-7610): + // The PR-merge branch previously had NO isWorkspaceTask guard, so a + // workspace-mode task (non-empty task.workspaceWorktrees) reaching + // auto-merge under project mergeStrategy:"pull-request" would + // unconditionally call processPullRequestMerge -> getCurrentRepo(cwd), + // which throws "could not determine repository" because the workspace + // root is a plain container of independent git sub-repos, not itself a + // git repo. That looped in-review <-> failed until retries exhausted. + // Hoist the workspace check here so workspace tasks ALWAYS fall through + // to the existing direct/else `rawMerge` branch below, whose + // isWorkspaceTask(mergeTask) routing already calls landWorkspaceTask + // correctly, regardless of the configured mergeStrategy — until true + // per-repo PR merge for workspace tasks (master-plan U6) ships. + const mergeCandidate = await store.getTask(taskId).catch(() => null); + const routeWorkspaceDirect = !!mergeCandidate && isWorkspaceTask(mergeCandidate); + + if (mergeStrategy === "pull-request" && this.options.processPullRequestMerge && !routeWorkspaceDirect) { this.activeMergeTaskId = taskId; runtimeLog.log(`${hasManualResolver ? "Manual" : "Auto"}-merge processing PR flow for ${taskId}...`); const result = await this.options.processPullRequestMerge(