diff --git a/.changeset/gh-4-pr-merge-repo-resolution.md b/.changeset/gh-4-pr-merge-repo-resolution.md new file mode 100644 index 000000000..f2c43d03b --- /dev/null +++ b/.changeset/gh-4-pr-merge-repo-resolution.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Fix PR-mode auto-merge failing with "Could not determine repository" in centrally-installed multi-project deployments. +category: fix +dev: processPullRequestMergeTask, createGroupPrCallback, and createPrNodeGithubOps now pass explicit owner/repo (resolved from the per-project cwd / task worktree) into findPrForBranch/createPr/mergePr instead of relying on GitHubClient.resolveRepo's process.cwd() fallback; the engine's buildRespondCallback resolves the review-response run cwd from the task's recorded worktree. Fixes Tchori-Labs/Fusion#4; non-workspace sibling of upstream #1924/FN-7610. diff --git a/packages/cli/src/commands/__tests__/task-lifecycle.test.ts b/packages/cli/src/commands/__tests__/task-lifecycle.test.ts index 49cdd75e8..4a24f81e0 100644 --- a/packages/cli/src/commands/__tests__/task-lifecycle.test.ts +++ b/packages/cli/src/commands/__tests__/task-lifecycle.test.ts @@ -4,10 +4,12 @@ import { EventEmitter } from "node:events"; // Mock child_process so we can intercept the `git push -u origin ` // call that processPullRequestMergeTask issues before createPr. const execMock = vi.hoisted(() => vi.fn()); -// Records raw (file, args[]) tuples for execFile so tests can assert a no-shell -// invocation (Fix #11) — i.e. the branch is a discrete argv entry, not shell- -// interpolated. -const execFileCalls = vi.hoisted(() => [] as Array<{ file: string; args: string[] }>); +// Records raw (file, args[], cwd) tuples for execFile so tests can assert a +// no-shell invocation (Fix #11) — i.e. the branch is a discrete argv entry, not +// shell-interpolated — and that git ops target the task worktree cwd (gh-4). +const execFileCalls = vi.hoisted( + () => [] as Array<{ file: string; args: string[]; cwd: string | undefined }>, +); vi.mock("node:child_process", () => ({ exec: (cmd: string, opts: unknown, cb: (err: Error | null, stdout: string, stderr: string) => void) => { try { @@ -19,7 +21,7 @@ vi.mock("node:child_process", () => ({ }, execFile: (file: string, args: string[] | undefined, opts: unknown, cb: (err: Error | null, stdout: string, stderr: string) => void) => { try { - execFileCalls.push({ file, args: args ?? [] }); + execFileCalls.push({ file, args: args ?? [], cwd: (opts as { cwd?: string } | undefined)?.cwd }); const result = execMock(`${file} ${(args ?? []).join(" ")}`.trim(), opts); cb(null, typeof result === "string" ? result : "", ""); } catch (err) { @@ -41,6 +43,7 @@ import { activeSessionRegistry } from "@fusion/engine"; import { cleanupMergedTaskArtifacts, createGroupPrCallback, + createPrNodeGithubOps, processPullRequestMergeTask, getTaskBranchName, syncGroupPrCallback, @@ -157,6 +160,141 @@ describe("processPullRequestMergeTask", () => { vi.mocked(getCurrentRepo).mockReturnValue({ owner: "owner", repo: "repo" }); }); + describe("central-install repo threading (gh-4)", () => { + // FNXC:PrMergeAutoMerge 2026-07-17-19:18 (gh-4): + // Simulate a centrally-installed multi-project daemon: process.cwd() is NOT + // a git repo, so cwd-less getCurrentRepo() returns null; only the + // per-project cwd resolves. Any PR call that relies on the client's + // process-cwd fallback would blow up with "Could not determine repository". + beforeEach(() => { + vi.mocked(getCurrentRepo).mockImplementation(((cwd?: string) => + cwd ? { owner: "central-owner", repo: "central-repo" } : null) as never); + }); + + it("threads explicit owner/repo into findPrForBranch, createPr, and mergePr on the per-task path", async () => { + const task: MockTask = { id: "FN-9401", title: "t", description: "d", column: "in-review" }; + const branch = getTaskBranchName(task.id); + const store = makeStatefulStore(task); + execMock.mockReturnValue(""); + + const github = { + findPrForBranch: vi.fn(async () => null), + createPr: vi.fn(async () => ({ + number: 77, + url: "https://github.com/central-owner/central-repo/pull/77", + status: "open" as const, + headBranch: branch, + baseBranch: "main", + })), + getPrMergeStatus: vi.fn(async () => ({ + prInfo: { + number: 77, + url: "https://github.com/central-owner/central-repo/pull/77", + status: "open" as const, + }, + reviewDecision: null, + checks: [], + mergeReady: true, + blockingReasons: [], + })), + mergePr: vi.fn(async () => ({ + number: 77, + url: "https://github.com/central-owner/central-repo/pull/77", + status: "merged" as const, + })), + }; + + const result = await processPullRequestMergeTask( + store as never, + "/projects/repo-a", + task.id, + github as never, + () => undefined, + ); + + expect(result).toBe("merged"); + expect(github.findPrForBranch).toHaveBeenCalledWith( + expect.objectContaining({ owner: "central-owner", repo: "central-repo", head: branch }), + ); + expect(github.createPr).toHaveBeenCalledWith( + expect.objectContaining({ owner: "central-owner", repo: "central-repo", head: branch }), + ); + expect(github.mergePr).toHaveBeenCalledWith( + expect.objectContaining({ owner: "central-owner", repo: "central-repo", number: 77, method: "squash" }), + ); + }); + + it("threads explicit owner/repo into findPrForBranch, createPr, and mergePr on the shared-branch-group path", async () => { + const task: MockTask = { + id: "FN-9402", + title: "t", + description: "d", + column: "in-review", + branchContext: { groupId: "planning:g4", source: "planning", assignmentMode: "shared" }, + }; + const store = makeStore(task, { baseBranch: "main" }); + (store.getBranchGroup as ReturnType).mockReturnValue({ + id: "BG-4", + sourceType: "planning", + sourceId: "planning:g4", + branchName: "fusion/groups/planning-g4", + autoMerge: true, + prState: "none", + status: "open", + createdAt: Date.now(), + updatedAt: Date.now(), + }); + (store.listTasksByBranchGroup as ReturnType).mockResolvedValue([task]); + execMock.mockImplementation((cmd: string) => (cmd.includes("rev-list --count") ? "1\n" : "")); + + const github = { + findPrForBranch: vi.fn(async () => null), + createPr: vi.fn(async () => ({ + number: 88, + url: "https://github.com/central-owner/central-repo/pull/88", + status: "open" as const, + headBranch: "fusion/groups/planning-g4", + baseBranch: "main", + })), + getPrMergeStatus: vi.fn(async () => ({ + prInfo: { + number: 88, + url: "https://github.com/central-owner/central-repo/pull/88", + status: "open" as const, + }, + reviewDecision: null, + checks: [], + mergeReady: true, + blockingReasons: [], + })), + mergePr: vi.fn(async () => ({ + number: 88, + url: "https://github.com/central-owner/central-repo/pull/88", + status: "merged" as const, + })), + }; + + const result = await processPullRequestMergeTask( + store as never, + "/projects/repo-a", + task.id, + github as never, + () => undefined, + ); + + expect(result).toBe("merged"); + expect(github.findPrForBranch).toHaveBeenCalledWith( + expect.objectContaining({ owner: "central-owner", repo: "central-repo", head: "fusion/groups/planning-g4" }), + ); + expect(github.createPr).toHaveBeenCalledWith( + expect.objectContaining({ owner: "central-owner", repo: "central-repo", head: "fusion/groups/planning-g4" }), + ); + expect(github.mergePr).toHaveBeenCalledWith( + expect.objectContaining({ owner: "central-owner", repo: "central-repo", number: 88, method: "squash" }), + ); + }); + }); + it("pushes the per-task branch to origin before creating a new PR", async () => { const task: MockTask = { id: "FN-9001", @@ -1103,7 +1241,7 @@ describe("processPullRequestMergeTask", () => { ); expect(result).toBe("merged"); - expect(github.mergePr).toHaveBeenCalledWith({ number: 124, method: "squash" }); + expect(github.mergePr).toHaveBeenCalledWith({ owner: "owner", repo: "repo", number: 124, method: "squash" }); expect(github.getPrMergeStatus).toHaveBeenCalledTimes(2); expect(github.getPrMergeStatus).toHaveBeenNthCalledWith(1, "owner", "repo", 124); expect(github.getPrMergeStatus).toHaveBeenNthCalledWith(2, "owner", "repo", 124); @@ -1174,7 +1312,7 @@ describe("processPullRequestMergeTask", () => { ), ).rejects.toThrow(mergeError.message); - expect(github.mergePr).toHaveBeenCalledWith({ number: 125, method: "squash" }); + expect(github.mergePr).toHaveBeenCalledWith({ owner: "owner", repo: "repo", number: 125, method: "squash" }); expect(github.getPrMergeStatus).toHaveBeenCalledTimes(2); expect(store.updatePrInfo).not.toHaveBeenCalledWith("FN-9105", expect.objectContaining({ status: "merged" })); expect(store.moveTask).not.toHaveBeenCalled(); @@ -1231,7 +1369,7 @@ describe("processPullRequestMergeTask", () => { ), ).rejects.toThrow(mergeError.message); - expect(github.mergePr).toHaveBeenCalledWith({ number: 126, method: "squash" }); + expect(github.mergePr).toHaveBeenCalledWith({ owner: "owner", repo: "repo", number: 126, method: "squash" }); expect(github.getPrMergeStatus).toHaveBeenCalledTimes(2); expect(store.moveTask).not.toHaveBeenCalled(); }); @@ -1398,7 +1536,7 @@ describe("processPullRequestMergeTask", () => { ); expect(result).toBe("merged"); - expect(github.mergePr).toHaveBeenCalledWith({ number: 100, method: "squash" }); + expect(github.mergePr).toHaveBeenCalledWith({ owner: "owner", repo: "repo", number: 100, method: "squash" }); }); it("preserves existing behavior when requirePrApproval is false", async () => { @@ -1654,7 +1792,12 @@ describe("createGroupPrCallback", () => { baseBranch: "main", }); - expect(github.findPrForBranch).toHaveBeenCalledWith({ head: group.branchName, state: "open" }); + expect(github.findPrForBranch).toHaveBeenCalledWith({ + owner: "owner", + repo: "repo", + head: group.branchName, + state: "open", + }); }); it("does not reuse a closed PR from a prior group — creates a fresh one", async () => { @@ -1679,7 +1822,12 @@ describe("createGroupPrCallback", () => { baseBranch: "main", }); - expect(github.findPrForBranch).toHaveBeenCalledWith({ head: group.branchName, state: "open" }); + expect(github.findPrForBranch).toHaveBeenCalledWith({ + owner: "owner", + repo: "repo", + head: group.branchName, + state: "open", + }); expect(github.createPr).toHaveBeenCalledTimes(1); expect(result).toEqual({ prNumber: 123, @@ -1687,5 +1835,121 @@ describe("createGroupPrCallback", () => { prState: "open", }); }); + + it("resolves the repo from the callback cwd and threads owner/repo into findPrForBranch/createPr (gh-4)", async () => { + vi.mocked(getCurrentRepo).mockImplementation(((cwd?: string) => + cwd ? { owner: "central-owner", repo: "central-repo" } : null) as never); + execMock.mockReturnValue(""); + + const github = { + findPrForBranch: vi.fn(async () => null), + createPr: vi.fn(async () => ({ + number: 5, + url: "https://github.com/central-owner/central-repo/pull/5", + status: "open" as const, + })), + }; + + const callback = createGroupPrCallback(github as never); + const result = await callback({ + cwd: "/projects/repo-a", + group: { id: "BG-9", branchName: "fusion/groups/g9" } as never, + members: [{ id: "FN-9501", title: "m1" }] as never, + headBranch: "fusion/groups/g9", + baseBranch: "main", + }); + + expect(vi.mocked(getCurrentRepo)).toHaveBeenCalledWith("/projects/repo-a"); + expect(github.findPrForBranch).toHaveBeenCalledWith( + expect.objectContaining({ owner: "central-owner", repo: "central-repo", head: "fusion/groups/g9" }), + ); + expect(github.createPr).toHaveBeenCalledWith( + expect.objectContaining({ owner: "central-owner", repo: "central-repo", head: "fusion/groups/g9" }), + ); + expect(result.prNumber).toBe(5); + }); +}); + +describe("createPrNodeGithubOps repo resolution (gh-4)", () => { + beforeEach(() => { + execMock.mockReset(); + execFileCalls.length = 0; + vi.mocked(getCurrentRepo).mockImplementation(((cwd?: string) => + cwd ? { owner: "central-owner", repo: "central-repo" } : null) as never); + }); + + const githubStub = () => ({ + createPr: vi.fn(async () => ({ number: 9, url: "https://github.com/central-owner/central-repo/pull/9", status: "open" as const })), + mergePr: vi.fn(async () => ({ number: 9, url: "https://github.com/central-owner/central-repo/pull/9", status: "merged" as const })), + getPrStatus: vi.fn(), + replyToReviewThread: vi.fn(), + resolveReviewThread: vi.fn(), + getViewerLogin: vi.fn(), + getPrReviewThreadsDetailed: vi.fn(), + }); + + it("resolvePrSource resolves the repo slug from the task worktree, not process.cwd()", async () => { + const ops = createPrNodeGithubOps(githubStub() as never); + const source = await ops.resolvePrSource( + { id: "FN-9601", worktree: "/projects/repo-a/.worktrees/fn-9601" } as never, + {} as never, + ); + expect(source.repo).toBe("central-owner/central-repo"); + expect(vi.mocked(getCurrentRepo)).toHaveBeenCalledWith("/projects/repo-a/.worktrees/fn-9601"); + }); + + it("resolvePrSource treats the configured getTaskWorktree resolver as authoritative over task.worktree", async () => { + const ops = createPrNodeGithubOps(githubStub() as never, { + getTaskWorktree: (taskId) => `/resolved/${taskId}`, + }); + const source = await ops.resolvePrSource( + { id: "FN-9601", worktree: "/stale/recorded/worktree" } as never, + {} as never, + ); + expect(source.repo).toBe("central-owner/central-repo"); + expect(vi.mocked(getCurrentRepo)).toHaveBeenCalledWith("/resolved/FN-9601"); + }); + + it("resolvePrSource throws instead of persisting entity.repo as '' when no repo resolves (central install, no worktree)", async () => { + // Worktree-less task in a central install: every cwd candidate (including + // process.cwd(), the install dir) fails to resolve a repo. Persisting "" + // would poison downstream splitRepoSlug consumers into the client's + // process-cwd fallback — the exact gh-4 failure mode. + vi.mocked(getCurrentRepo).mockReturnValue(null as never); + const ops = createPrNodeGithubOps(githubStub() as never); + expect(() => ops.resolvePrSource({ id: "FN-9601" } as never, {} as never)).toThrow( + /pr-create: could not determine repository for task FN-9601/, + ); + }); + + it("createPr pushes from the task worktree and passes owner/repo parsed from entity.repo", async () => { + execMock.mockReturnValue(""); + const github = githubStub(); + const ops = createPrNodeGithubOps(github as never); + const result = await ops.createPr({ + task: { id: "FN-9601", title: "t", description: "d", worktree: "/projects/repo-a/.worktrees/fn-9601" }, + entity: { id: "e1", sourceId: "FN-9601", repo: "central-owner/central-repo", headBranch: "fusion/fn-9601", baseBranch: "main" }, + } as never); + expect(github.createPr).toHaveBeenCalledWith( + expect.objectContaining({ owner: "central-owner", repo: "central-repo", head: "fusion/fn-9601" }), + ); + const pushCall = execFileCalls.find((c) => c.file === "git" && c.args[0] === "push"); + expect(pushCall).toBeDefined(); + // The push must run in the task worktree, not process.cwd() (gh-4). + expect(pushCall?.cwd).toBe("/projects/repo-a/.worktrees/fn-9601"); + expect(result.prNumber).toBe(9); + }); + + it("mergePr passes owner/repo parsed from entity.repo", async () => { + const github = githubStub(); + const ops = createPrNodeGithubOps(github as never); + const result = await ops.mergePr({ + entity: { id: "e1", sourceId: "FN-9601", repo: "central-owner/central-repo", prNumber: 9, headOid: "abc123" }, + } as never); + expect(result).toEqual({ status: "merged-requested" }); + expect(github.mergePr).toHaveBeenCalledWith( + expect.objectContaining({ owner: "central-owner", repo: "central-repo", number: 9, method: "squash", expectedHeadOid: "abc123" }), + ); + }); }); diff --git a/packages/cli/src/commands/task-lifecycle.ts b/packages/cli/src/commands/task-lifecycle.ts index 40cc07a9c..7cd659644 100644 --- a/packages/cli/src/commands/task-lifecycle.ts +++ b/packages/cli/src/commands/task-lifecycle.ts @@ -49,8 +49,8 @@ import type { * Defined locally to avoid importing from @fusion/dashboard. */ interface GitHubOperations { - findPrForBranch(params: { head: string; state?: "open" | "closed" | "all" }): Promise; - createPr(params: { title: string; body: string; head: string; base?: string }): Promise; + findPrForBranch(params: { owner?: string; repo?: string; head: string; state?: "open" | "closed" | "all" }): Promise; + createPr(params: { owner?: string; repo?: string; title: string; body: string; head: string; base?: string }): Promise; getPrMergeStatus(owner?: string, repo?: string, number?: number): Promise<{ prInfo: PrInfo; reviewDecision: string | null; @@ -58,7 +58,7 @@ interface GitHubOperations { mergeReady: boolean; blockingReasons: string[]; }>; - mergePr(params: { number: number; method?: "merge" | "squash" | "rebase"; expectedHeadOid?: string }): Promise; + mergePr(params: { owner?: string; repo?: string; number: number; method?: "merge" | "squash" | "rebase"; expectedHeadOid?: string }): Promise; getPrStatus(owner: string, repo: string, number: number): Promise; /** Reply to a specific review thread (U2). */ replyToReviewThread(threadId: string, body: string): Promise; @@ -256,12 +256,24 @@ function toBranchGroupPrState(prInfo: PrInfo | null): BranchGroupPrState { * Idempotency: reuses an existing PR for the group head branch on GitHub. The * coordinator additionally skips this call when a `prNumber` is already persisted, * so a re-promotion never opens a second PR. + * + * Repo identity is resolved from the per-project cwd in the callback input, mirroring + * syncGroupPrCallback. */ export function createGroupPrCallback( github: Pick, ): CreateGroupPrFn { return async ({ cwd, group, members, headBranch, baseBranch }) => { - const existing = await github.findPrForBranch({ head: headBranch, state: "open" }); + // FNXC:PrMergeAutoMerge 2026-07-17-16:50 (gh-4): + // Resolve the repo from the PROJECT cwd, not the process cwd (same T4 + // requirement as syncGroupPrCallback below) — in a centrally-installed + // multi-project daemon process.cwd() is not the project repo, so the + // client's fallback would throw or target the wrong repository. + const repo = getCurrentRepo(cwd); + if (!repo) { + throw new Error("createGroupPr: could not determine repository"); + } + const existing = await github.findPrForBranch({ owner: repo.owner, repo: repo.repo, head: headBranch, state: "open" }); if (existing) { return { prNumber: existing.number, prUrl: existing.url, prState: toBranchGroupPrState(existing) }; } @@ -273,6 +285,8 @@ export function createGroupPrCallback( branchName: getTaskBranchName(member.id), })); const created = await github.createPr({ + owner: repo.owner, + repo: repo.repo, title: buildGroupPullRequestTitle(group, members), body: buildGroupPullRequestBody(group, membersWithBranch), head: headBranch, @@ -418,20 +432,41 @@ export function createPrNodeGithubOps( return { resolvePrSource: (task) => { - const repo = getCurrentRepo(); - const repoSlug = repo ? `${repo.owner}/${repo.repo}` : ""; + // FNXC:PrMergeAutoMerge 2026-07-17-19:18 (gh-4): + // Resolve the repo from the task's worktree (a checkout of the project + // repo), not process.cwd() — in a centrally-installed multi-project + // server process.cwd() is not a repo, which persisted entity.repo as "" + // and poisoned every downstream splitRepoSlug consumer. The configured + // getTaskWorktree resolver is authoritative (same precedence as createPr), + // and an unresolvable repo now throws instead of persisting "" — the + // pr-create node maps the throw to a routable `source-error` failure, + // and "" could only ever succeed by silently targeting whatever repo + // process.cwd() happens to sit in. + const cwd = options.getTaskWorktree?.(task.id) ?? task.worktree ?? process.cwd(); + const repo = getCurrentRepo(cwd); + if (!repo) { + throw new Error( + `pr-create: could not determine repository for task ${task.id} (no GitHub remote resolved from ${cwd})`, + ); + } return { sourceType: "task", sourceId: task.id, - repo: repoSlug, + repo: `${repo.owner}/${repo.repo}`, headBranch: getTaskBranchName(task.id), }; }, createPr: async ({ task, entity }) => { - const cwd = process.cwd(); + // FNXC:PrMergeAutoMerge 2026-07-17-19:18 (gh-4): + // Git ops run in the task worktree when known; process.cwd() only as the + // single-project fallback. + const cwd = options.getTaskWorktree?.(entity.sourceId) ?? task.worktree ?? process.cwd(); const headBranch = entity.headBranch || getTaskBranchName(task.id); await pushTaskBranchToOrigin(cwd, headBranch); + const { owner, name } = splitRepoSlug(entity.repo); const created = await github.createPr({ + owner, + repo: name, title: task.title ?? `Task ${task.id}`, body: task.description ?? "", head: headBranch, @@ -444,8 +479,11 @@ export function createPrNodeGithubOps( if (entity.prNumber == null) { throw new Error(`pr-merge: entity ${entity.id} has no persisted prNumber`); } + const { owner, name } = splitRepoSlug(entity.repo); try { await github.mergePr({ + owner, + repo: name, number: entity.prNumber, method: "squash", expectedHeadOid: entity.headOid, @@ -735,6 +773,9 @@ export async function processPullRequestMergeTask( /* * 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. + * + * FNXC:PrMergeAutoMerge 2026-07-17-16:46 (gh-4): + * The same requirement extends to EVERY GitHub call in this path: findPrForBranch/createPr/mergePr must carry prRepo explicitly, because GitHubClient.resolveRepo's fallback resolves from process.cwd(), which in a centrally-installed multi-project server is the install dir (throws "Could not determine repository") or, worse, some unrelated repo (silently targets the wrong repository). */ const prRepo = getCurrentRepo(cwd); if (!prRepo) { @@ -794,11 +835,13 @@ export async function processPullRequestMergeTask( // terminal PR for this head branch must NOT be reattached (that reintroduces // the terminal-PR reuse bug createGroupPrCallback fixed); treat it as // not-found and fall through to push + createPr for a fresh open PR. - groupPrInfo = await github.findPrForBranch({ head: branchGroup.branchName, state: "open" }); + groupPrInfo = await github.findPrForBranch({ owner: prRepo.owner, repo: prRepo.repo, head: branchGroup.branchName, state: "open" }); if (!groupPrInfo) { await pushTaskBranchToOrigin(cwd, branchGroup.branchName); try { groupPrInfo = await github.createPr({ + owner: prRepo.owner, + repo: prRepo.repo, title: buildGroupPullRequestTitle(branchGroup, members), body: buildGroupPullRequestBody(branchGroup, membersWithCommits), head: branchGroup.branchName, @@ -867,7 +910,7 @@ export async function processPullRequestMergeTask( } await store.updateTask(task.id, { status: "merging-pr" }); - const mergedPr = await github.mergePr({ number: refreshedPrInfo.number, method: "squash" }); + const mergedPr = await github.mergePr({ owner: prRepo.owner, repo: prRepo.repo, number: refreshedPrInfo.number, method: "squash" }); await store.updateBranchGroup(branchGroup.id, { prNumber: mergedPr.number, prUrl: mergedPr.url, @@ -894,7 +937,7 @@ export async function processPullRequestMergeTask( if (!prInfo) { await store.updateTask(task.id, { status: "creating-pr" }); - const existingPr = await github.findPrForBranch({ head: branch, state: "all" }); + const existingPr = await github.findPrForBranch({ owner: prRepo.owner, repo: prRepo.repo, head: branch, state: "all" }); if (!existingPr) { // gh pr create / GitHub REST require the head branch to exist on // origin. Nothing else in the merge path publishes the per-task @@ -903,6 +946,8 @@ export async function processPullRequestMergeTask( } try { prInfo = existingPr ?? await github.createPr({ + owner: prRepo.owner, + repo: prRepo.repo, title: buildPullRequestTitle(task), body: buildPullRequestBody(task), head: branch, @@ -987,7 +1032,7 @@ export async function processPullRequestMergeTask( await store.updateTask(task.id, { status: "merging-pr" }); let mergedPr: PrInfo; try { - mergedPr = await github.mergePr({ number: prInfo.number, method: "squash" }); + mergedPr = await github.mergePr({ owner: prRepo.owner, repo: prRepo.repo, number: prInfo.number, method: "squash" }); } catch (err: unknown) { let refreshedStatus: Awaited>; try { diff --git a/packages/engine/src/__tests__/pr-respond-cwd-resolution.test.ts b/packages/engine/src/__tests__/pr-respond-cwd-resolution.test.ts new file mode 100644 index 000000000..99fa8ff10 --- /dev/null +++ b/packages/engine/src/__tests__/pr-respond-cwd-resolution.test.ts @@ -0,0 +1,127 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +// Capture the cwd + store handed to the response agent runner and the git-ops +// resolver, so these tests can assert which directory the respond run actually +// targets and that getTask-less structural stores are withheld from the runner. +const agentRunnerCalls = vi.hoisted( + () => [] as Array<{ taskId: string; cwd: string; store: unknown }>, +); +const gitOpsResolvers = vi.hoisted(() => [] as Array<(entity: unknown) => string>); + +vi.mock("../pr-response-run-ops.js", () => ({ + makePrResponseAgentRunner: vi.fn( + (_settings: unknown, taskId: string, cwd: string, store: unknown) => { + agentRunnerCalls.push({ taskId, cwd, store }); + return vi.fn(); + }, + ), + makePrResponseGitOps: vi.fn((getCwd: (entity: unknown) => string) => { + gitOpsResolvers.push(getCwd); + return { + getChangedContent: vi.fn(), + getWorktreeHeadOid: vi.fn(), + fetchAndFastForwardPush: vi.fn(), + }; + }), +})); + +vi.mock("../pr-response-run.js", () => ({ + runPrResponseRun: vi.fn(async () => ({ value: "resolved-all" })), +})); + +import { buildRespondCallback } from "../pr-nodes.js"; + +const entity = { + id: "pr-entity-1", + sourceType: "task", + sourceId: "FN-1", + repo: "owner/repo", + headBranch: "fusion/fn-1", + state: "open", + autoMerge: false, + unverified: false, + responseRounds: 0, + createdAt: 1, + updatedAt: 1, + prNumber: 7, +} as never; + +function makeOps(getCwd: (e: unknown) => string) { + return { + getReviewThreads: vi.fn(async () => []), + getViewerLogin: vi.fn(async () => "viewer"), + checkPrStillOpen: vi.fn(async () => ({ open: true, headOid: null })), + replyToThread: vi.fn(), + resolveThread: vi.fn(), + getCwd, + getTaskId: () => "FN-1", + } as never; +} + +function makeStore(worktree?: string, opts?: { getTaskThrows?: boolean }) { + return { + getSettings: vi.fn(async () => ({})), + getTask: opts?.getTaskThrows + ? vi.fn(async () => { + throw new Error("missing task"); + }) + : vi.fn(async () => ({ id: "FN-1", worktree })), + }; +} + +beforeEach(() => { + agentRunnerCalls.length = 0; + gitOpsResolvers.length = 0; +}); + +describe("buildRespondCallback cwd resolution (gh-4)", () => { + it("prefers the task's recorded worktree over the CLI getCwd resolver (process.cwd() in central installs)", async () => { + const getCwd = vi.fn(() => "/central/install-dir"); + const store = makeStore("/projects/repo-a/.worktrees/fn-1"); + const respond = buildRespondCallback(() => store as never, makeOps(getCwd)); + + const result = await respond({ entity } as never); + + expect(result).toEqual({ value: "resolved-all" }); + expect(agentRunnerCalls).toEqual([ + { taskId: "FN-1", cwd: "/projects/repo-a/.worktrees/fn-1", store }, + ]); + expect(gitOpsResolvers).toHaveLength(1); + expect(gitOpsResolvers[0](entity)).toBe("/projects/repo-a/.worktrees/fn-1"); + }); + + it("falls back to the CLI getCwd resolver when the task has no recorded worktree", async () => { + const getCwd = vi.fn(() => "/single-project/checkout"); + const store = makeStore(undefined); + const respond = buildRespondCallback(() => store as never, makeOps(getCwd)); + + await respond({ entity } as never); + + expect(agentRunnerCalls).toEqual([{ taskId: "FN-1", cwd: "/single-project/checkout", store }]); + expect(gitOpsResolvers[0](entity)).toBe("/single-project/checkout"); + }); + + it("propagates a real store read failure instead of silently running in the fallback cwd (routable respond-error at the node)", async () => { + const getCwd = vi.fn(() => "/single-project/checkout"); + const respond = buildRespondCallback( + () => makeStore(undefined, { getTaskThrows: true }) as never, + makeOps(getCwd), + ); + + await expect(respond({ entity } as never)).rejects.toThrow("missing task"); + expect(agentRunnerCalls).toEqual([]); + expect(gitOpsResolvers).toEqual([]); + }); + + it("falls back to the CLI getCwd resolver on structural stores without getTask, and withholds the store from the agent runner", async () => { + const getCwd = vi.fn(() => "/single-project/checkout"); + const storeWithoutGetTask = { getSettings: vi.fn(async () => ({})) }; + const respond = buildRespondCallback(() => storeWithoutGetTask as never, makeOps(getCwd)); + + await respond({ entity } as never); + + expect(agentRunnerCalls).toEqual([ + { taskId: "FN-1", cwd: "/single-project/checkout", store: undefined }, + ]); + }); +}); diff --git a/packages/engine/src/pr-nodes.ts b/packages/engine/src/pr-nodes.ts index 9e9371720..45ef3ae09 100644 --- a/packages/engine/src/pr-nodes.ts +++ b/packages/engine/src/pr-nodes.ts @@ -222,7 +222,6 @@ export function buildRespondCallback( */ pluginRunner?: import("./plugin-runner.js").PluginRunner, ): NonNullable { - const gitOps = makePrResponseGitOps(ops.getCwd); return async ({ entity }) => { const store = getStore(); // The engine owns a concrete TaskStore behind the structural PrNodeStore; the @@ -247,8 +246,34 @@ export function buildRespondCallback( } const taskId = ops.getTaskId(entity); - const cwd = ops.getCwd(entity); - const runAgent = makePrResponseAgentRunner(settings, taskId, cwd, fullStore, pluginRunner); + /* + * FNXC:PrMergeAutoMerge 2026-07-17-19:18 (gh-4): + * Resolve the review-response run cwd from the task's recorded worktree first. + * The CLI-injected ops.getCwd defaults to process.cwd() (no CLI composition + * site wires getTaskWorktree), which in a centrally-installed multi-project + * server is the install dir — git ops and the response agent would run outside + * the project repo. The engine owns the store, so it recovers the worktree + * here; ops.getCwd stays the fallback ONLY for structural stores without + * getTask (PrNodeStore does not declare it — tests/specialized wiring). A + * real getTask rejection (store failure, deleted task) propagates instead: + * the pr-respond node maps it to a routable `respond-error`, which beats + * running a mutating agent + git push in a cwd that may not even be the + * project repo. Structural stores are also withheld from the agent runner, + * whose store parameter is optional but assumed to have getTask. + */ + const hasGetTask = typeof (fullStore as Partial).getTask === "function"; + const respondTask: { worktree?: string } | null = hasGetTask + ? await fullStore.getTask(taskId) + : null; + const cwd = respondTask?.worktree || ops.getCwd(entity); + const gitOps = makePrResponseGitOps(() => cwd); + const runAgent = makePrResponseAgentRunner( + settings, + taskId, + cwd, + hasGetTask ? fullStore : undefined, + pluginRunner, + ); const result = await runPrResponseRun({ entity,