diff --git a/packages/core/src/config.ts b/packages/core/src/config.ts index 145a7b2cb5..3426482646 100644 --- a/packages/core/src/config.ts +++ b/packages/core/src/config.ts @@ -267,6 +267,8 @@ const ProjectConfigSchema = z.object({ orchestrator: RoleAgentConfigSchema, worker: RoleAgentConfigSchema, reactions: z.record(ReactionConfigSchema.partial()).optional(), + /** Merge strategy for the dashboard merge action. Default: "squash". */ + mergeMethod: z.enum(["merge", "squash", "rebase", "merge-with-ff"]).default("squash"), agentRules: z.string().optional(), agentRulesFile: z.string().optional(), orchestratorRules: z.string().optional(), diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index 86850228e6..c18b649d70 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -981,7 +981,15 @@ export const PR_STATE = { CLOSED: "closed" as const, } satisfies Record; -export type MergeMethod = "merge" | "squash" | "rebase"; +/** + * PR merge strategy. `merge` / `squash` / `rebase` map directly to GitHub's + * native merge-button methods. `merge-with-ff` is an AO composite strategy + * (not a native API value): when the PR branch is strictly ahead of the base + * (no divergence) it fast-forwards the base ref server-side via the Git Refs + * API (`force=false`); when the branches have diverged it falls back to a + * `merge` commit. Only the GitHub SCM implements `merge-with-ff`. + */ +export type MergeMethod = "merge" | "squash" | "rebase" | "merge-with-ff"; export interface SCMWebhookRequest { method: string; @@ -1579,6 +1587,9 @@ export interface ProjectConfig { /** Per-project reaction overrides */ reactions?: Record>; + /** Merge strategy for the dashboard merge action. Default: "squash". */ + mergeMethod?: MergeMethod; + /** Inline rules/instructions passed to every agent prompt */ agentRules?: string; diff --git a/packages/plugins/scm-github/src/index.ts b/packages/plugins/scm-github/src/index.ts index fa59cecdca..c8934dc838 100644 --- a/packages/plugins/scm-github/src/index.ts +++ b/packages/plugins/scm-github/src/index.ts @@ -526,6 +526,56 @@ function repoFlag(pr: PRInfo): string { return `${pr.owner}/${pr.repo}`; } +/** GitHub's `compare` endpoint status for base...head. */ +type CompareStatus = "ahead" | "behind" | "diverged" | "identical"; + +/** + * Compare the PR head branch against its base. Returns GitHub's comparison + * status: `"ahead"` means head is strictly ahead of base (fast-forwardable), + * `"identical"` means both refs point at the same commit, `"diverged"` means + * both sides have unique commits, `"behind"` means base is ahead of head. + */ +async function getCompareStatus(pr: PRInfo): Promise { + const raw = await gh(["api", `repos/${repoFlag(pr)}/compare/${pr.baseBranch}...${pr.branch}`]); + const data: { status: CompareStatus } = JSON.parse(raw); + return data.status; +} + +/** + * Fast-forward the base branch ref to the PR head, server-side, via the Git + * Refs API. `force=false` makes GitHub reject anything that isn't a true + * fast-forward. The head branch is then deleted to match `--delete-branch`. + * + * Note: the base ref must not be protected against direct updates. Protected + * branches reject Refs API writes by design, so `merge-with-ff` cannot + * fast-forward into them — use `merge`/`squash`/`rebase` for protected bases. + */ +async function fastForwardBase(pr: PRInfo): Promise { + const refRaw = await gh(["api", `repos/${repoFlag(pr)}/git/ref/heads/${pr.branch}`]); + const headSha = (JSON.parse(refRaw) as { object: { sha: string } }).object.sha; + try { + await gh([ + "api", + "-X", + "PATCH", + `repos/${repoFlag(pr)}/git/refs/heads/${pr.baseBranch}`, + "-f", + `sha=${headSha}`, + "-F", + "force=false", + ]); + } catch (err) { + throw new Error( + `Failed to fast-forward ${pr.baseBranch} to ${pr.branch} via the Git Refs API. ` + + `This typically means ${pr.baseBranch} is a protected branch (direct ref updates are ` + + `blocked) or it advanced since the comparison. Use mergeMethod "merge", "squash", or ` + + `"rebase" for protected branches.`, + { cause: err }, + ); + } + await gh(["api", "-X", "DELETE", `repos/${repoFlag(pr)}/git/refs/heads/${pr.branch}`]); +} + function prEventKey(pr: PRInfo): string { return `${repoFlag(pr)}#${pr.number}`; } @@ -839,9 +889,37 @@ function createGitHubSCM(): SCM { }, async mergePR(pr: PRInfo, method: MergeMethod = "squash"): Promise { - const flag = method === "rebase" ? "--rebase" : method === "merge" ? "--merge" : "--squash"; - - await gh(["pr", "merge", String(pr.number), "--repo", repoFlag(pr), flag, "--delete-branch"]); + if (method === "merge-with-ff") { + // Fast-forward when the branch is ahead of (or identical to) base; on + // divergence fall back to a merge commit. GitHub has no fast-forward + // merge button, so the FF path goes through the Git Refs API rather + // than `gh pr merge`. + const status = await getCompareStatus(pr); + if (status === "ahead" || status === "identical") { + await fastForwardBase(pr); + } else { + await gh([ + "pr", + "merge", + String(pr.number), + "--repo", + repoFlag(pr), + "--merge", + "--delete-branch", + ]); + } + } else { + const flag = method === "rebase" ? "--rebase" : method === "merge" ? "--merge" : "--squash"; + await gh([ + "pr", + "merge", + String(pr.number), + "--repo", + repoFlag(pr), + flag, + "--delete-branch", + ]); + } invalidatePRCache(pr); }, diff --git a/packages/plugins/scm-github/test/index.test.ts b/packages/plugins/scm-github/test/index.test.ts index 0812cf01fe..3b6e43ec4d 100644 --- a/packages/plugins/scm-github/test/index.test.ts +++ b/packages/plugins/scm-github/test/index.test.ts @@ -576,6 +576,90 @@ describe("scm-github plugin", () => { expect.any(Object), ); }); + + it("merge-with-ff fast-forwards the base ref when the branch is ahead", async () => { + ghMock.mockResolvedValueOnce({ stdout: JSON.stringify({ status: "ahead" }) }); // compare + ghMock.mockResolvedValueOnce({ stdout: JSON.stringify({ object: { sha: "abc123" } }) }); // ref + ghMock.mockResolvedValueOnce({ stdout: "" }); // PATCH update-ref + ghMock.mockResolvedValueOnce({ stdout: "" }); // DELETE head ref + await scm.mergePR(pr, "merge-with-ff"); + + expect(ghMock).toHaveBeenCalledWith( + expect.stringMatching(/(?:^|[\\/])gh(?:\.(?:exe|cmd|bat))?$/i), + ["api", "repos/acme/repo/compare/main...feat/my-feature"], + expect.any(Object), + ); + expect(ghMock).toHaveBeenCalledWith( + expect.stringMatching(/(?:^|[\\/])gh(?:\.(?:exe|cmd|bat))?$/i), + [ + "api", + "-X", + "PATCH", + "repos/acme/repo/git/refs/heads/main", + "-f", + "sha=abc123", + "-F", + "force=false", + ], + expect.any(Object), + ); + expect(ghMock).toHaveBeenCalledWith( + expect.stringMatching(/(?:^|[\\/])gh(?:\.(?:exe|cmd|bat))?$/i), + ["api", "-X", "DELETE", "repos/acme/repo/git/refs/heads/feat/my-feature"], + expect.any(Object), + ); + // The fast-forward path must not touch the gh pr merge button. + const usedPrMerge = ghMock.mock.calls.some( + (c) => Array.isArray(c[1]) && c[1][0] === "pr" && c[1][1] === "merge", + ); + expect(usedPrMerge).toBe(false); + }); + + it("merge-with-ff fast-forwards (no merge commit) when the branch is identical to base", async () => { + ghMock.mockResolvedValueOnce({ stdout: JSON.stringify({ status: "identical" }) }); // compare + ghMock.mockResolvedValueOnce({ stdout: JSON.stringify({ object: { sha: "abc123" } }) }); // ref + ghMock.mockResolvedValueOnce({ stdout: "" }); // PATCH update-ref + ghMock.mockResolvedValueOnce({ stdout: "" }); // DELETE head ref + await scm.mergePR(pr, "merge-with-ff"); + + expect(ghMock).toHaveBeenCalledWith( + expect.stringMatching(/(?:^|[\\/])gh(?:\.(?:exe|cmd|bat))?$/i), + [ + "api", + "-X", + "PATCH", + "repos/acme/repo/git/refs/heads/main", + "-f", + "sha=abc123", + "-F", + "force=false", + ], + expect.any(Object), + ); + // Identical branches must not produce a merge commit. + const usedPrMerge = ghMock.mock.calls.some( + (c) => Array.isArray(c[1]) && c[1][0] === "pr" && c[1][1] === "merge", + ); + expect(usedPrMerge).toBe(false); + }); + + it("merge-with-ff surfaces a clear error when the base ref update is rejected", async () => { + ghMock.mockResolvedValueOnce({ stdout: JSON.stringify({ status: "ahead" }) }); // compare + ghMock.mockResolvedValueOnce({ stdout: JSON.stringify({ object: { sha: "abc123" } }) }); // ref + ghMock.mockRejectedValueOnce(new Error("HTTP 403: Protected branch")); // PATCH update-ref + await expect(scm.mergePR(pr, "merge-with-ff")).rejects.toThrow(/protected branch/i); + }); + + it("merge-with-ff falls back to a merge commit when the branch has diverged", async () => { + ghMock.mockResolvedValueOnce({ stdout: JSON.stringify({ status: "diverged" }) }); // compare + ghMock.mockResolvedValueOnce({ stdout: "" }); // gh pr merge + await scm.mergePR(pr, "merge-with-ff"); + expect(ghMock).toHaveBeenLastCalledWith( + expect.stringMatching(/(?:^|[\\/])gh(?:\.(?:exe|cmd|bat))?$/i), + ["pr", "merge", "42", "--repo", "acme/repo", "--merge", "--delete-branch"], + expect.any(Object), + ); + }); }); // ---- closePR ----------------------------------------------------------- diff --git a/packages/plugins/scm-gitlab/src/index.ts b/packages/plugins/scm-gitlab/src/index.ts index 31402653a6..49da1725e0 100644 --- a/packages/plugins/scm-gitlab/src/index.ts +++ b/packages/plugins/scm-gitlab/src/index.ts @@ -528,6 +528,15 @@ function createGitLabSCM(config?: Record): SCM { }, async mergePR(pr: PRInfo, method: MergeMethod = "squash"): Promise { + if (method === "merge-with-ff") { + // "merge-with-ff" is a GitHub-only composite strategy (server-side + // fast-forward via the Git Refs API). GitLab has no equivalent + // per-merge flag, so fail loudly instead of silently ignoring it. + throw new Error( + 'mergeMethod "merge-with-ff" is not supported by the GitLab SCM; ' + + 'use "merge", "squash", or "rebase".', + ); + } const args = ["mr", "merge", String(pr.number), "--repo", repoFlag(pr)]; if (method === "squash") args.push("--squash"); else if (method === "rebase") args.push("--rebase"); diff --git a/packages/plugins/scm-gitlab/test/index.test.ts b/packages/plugins/scm-gitlab/test/index.test.ts index 4c5f87dfb6..0f791e3303 100644 --- a/packages/plugins/scm-gitlab/test/index.test.ts +++ b/packages/plugins/scm-gitlab/test/index.test.ts @@ -514,6 +514,13 @@ describe("scm-gitlab plugin", () => { expect(args).not.toContain("--squash"); expect(args).not.toContain("--rebase"); }); + + it("throws for the unsupported merge-with-ff method instead of silently ignoring it", async () => { + await expect(scm.mergePR(pr, "merge-with-ff")).rejects.toThrow( + /merge-with-ff.*not supported/i, + ); + expect(glabMock).not.toHaveBeenCalled(); + }); }); // ---- closePR ----------------------------------------------------------- diff --git a/packages/web/src/__tests__/api-routes.test.ts b/packages/web/src/__tests__/api-routes.test.ts index 157576fb43..feb71b0607 100644 --- a/packages/web/src/__tests__/api-routes.test.ts +++ b/packages/web/src/__tests__/api-routes.test.ts @@ -1466,6 +1466,31 @@ describe("API Routes", () => { const data = await res.json(); expect(data.error).toMatch(/merged/); }); + + it("defaults to squash when the project has no mergeMethod", async () => { + const req = makeRequest("/api/prs/432/merge", { method: "POST" }); + const res = await mergePOST(req, { params: Promise.resolve({ id: "432" }) }); + expect(res.status).toBe(200); + const data = await res.json(); + expect(data.method).toBe("squash"); + expect(mockSCM.mergePR).toHaveBeenCalledWith(expect.anything(), "squash"); + }); + + it("uses the project's configured mergeMethod", async () => { + const project = mockConfig.projects["my-app"]; + const original = project.mergeMethod; + project.mergeMethod = "merge-with-ff"; + try { + const req = makeRequest("/api/prs/432/merge", { method: "POST" }); + const res = await mergePOST(req, { params: Promise.resolve({ id: "432" }) }); + expect(res.status).toBe(200); + const data = await res.json(); + expect(data.method).toBe("merge-with-ff"); + expect(mockSCM.mergePR).toHaveBeenCalledWith(expect.anything(), "merge-with-ff"); + } finally { + project.mergeMethod = original; + } + }); }); describe("GET /api/observability", () => { diff --git a/packages/web/src/app/api/prs/[id]/merge/route.ts b/packages/web/src/app/api/prs/[id]/merge/route.ts index d4ea77d07a..4faed33845 100644 --- a/packages/web/src/app/api/prs/[id]/merge/route.ts +++ b/packages/web/src/app/api/prs/[id]/merge/route.ts @@ -89,7 +89,8 @@ export async function POST(_request: NextRequest, { params }: { params: Promise< ); } - await scm.mergePR(targetPR, "squash"); + const mergeMethod = project.mergeMethod ?? "squash"; + await scm.mergePR(targetPR, mergeMethod); recordApiObservation({ config, method: "POST", @@ -108,10 +109,10 @@ export async function POST(_request: NextRequest, { params }: { params: Promise< source: "api", kind: "api.pr_merge_requested", summary: `PR ${prNumber} merge requested`, - data: { prNumber, method: "squash" }, + data: { prNumber, method: mergeMethod }, }); return jsonWithCorrelation( - { ok: true, prNumber, method: "squash" }, + { ok: true, prNumber, method: mergeMethod }, { status: 200 }, correlationId, );