Auto-merge review fixes: CLI --auto, archived repositories, queue refusals, agent blockers, branch deletion (#58)
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AizKXSsengXZRegJxRATkL
Canberk Demirci authored · GitHub committed signed1 parent 3857f8ccommit 7dd08e9ce30964280efde862e4b3d4c1cec0895fBrowse files
20 files changed+487−41
Files (20)
- apps/cli/src/commands/pr.ts+32−5
- apps/cli/test/pr.test.ts+46−1
- apps/diffinery/docs/agents/mcp.md+3−3
- apps/diffinery/docs/collaboration/merge-queue.md+1−1
- apps/diffinery/docs/collaboration/pull-requests.md+9−6
- apps/diffinery/docs/reference/rest-api.md+3−1
- apps/diffinery/docs/repositories/branch-protection.md+5−2
- apps/diffinery/src/http/api.ts+4−2
- apps/diffinery/src/http/gh.ts+3−2
- apps/diffinery/src/http/ghctx.ts+5−1
- apps/diffinery/src/lib/directory.ts+3−1
- apps/diffinery/src/mcp/server.ts+9−4
- apps/diffinery/src/repo/RepoDO.ts+23−0
- apps/diffinery/src/repo/auto-merge.ts+29−5
- apps/diffinery/src/repo/collab.ts+14−2
- apps/diffinery/src/repo/queue.ts+15−3
- apps/diffinery/src/repo/types.ts+5−0
- apps/diffinery/src/web/collab.tsx+16−1
- apps/diffinery/test/e2e/security.test.mjs+127−0
- apps/diffinery/test/unit/auto-merge.test.ts+135−1
modifiedapps/cli/src/commands/pr.ts+32−5View file
| @@ -270,7 +270,8 @@ | ||
| 270 | 270 | ...bodyOpts, |
| 271 | 271 | "match-head-commit": { type: "string", desc: "Only merge if the head is still at this SHA", arg: "SHA" }, |
| 272 | 272 | admin: { type: "boolean", desc: "Merge now, bypassing unmet branch rules and any merge queue (repository admins; audited)" }, |
| 273 | - "disable-auto": { type: "boolean", desc: "Remove the pull request from the merge queue" }, | |
| 273 | + auto: { type: "boolean", desc: "Turn on auto-merge: merge (or join the merge queue) once every requirement is met" }, | |
| 274 | + "disable-auto": { type: "boolean", desc: "Turn off auto-merge (and take the pull request out of the merge queue if it is in it)" }, | |
| 274 | 275 | }, |
| 275 | 276 | repo: true, |
| 276 | 277 | json: true, |
| @@ -281,12 +282,38 @@ | ||
| 281 | 282 | const method = exactlyOne(o, ["merge", "squash", "rebase"], "merge"); |
| 282 | 283 | const repo = await ctx.repo(); |
| 283 | 284 | const api = await ctx.authedApi(); |
| 285 | + const explicit = ["merge", "squash", "rebase"].some((k) => o[k] === true); | |
| 286 | + if (o.auto === true && o["disable-auto"] === true) throw new CliError("use only one of --auto, --disable-auto", 2); | |
| 287 | + if (o.auto === true && o.admin === true) throw new CliError("--admin merges now; it can't be combined with --auto", 2); | |
| 284 | 288 | if (o["disable-auto"] === true) { |
| 285 | - await api.delete(`${repoPath(repo)}/pulls/${n}/merge-queue`); | |
| 286 | - if (ctx.json) return json(ctx.io, { removed: true }); | |
| 287 | - return ctx.out(`${ctx.c.green("✓")} Removed #${n} from the merge queue`); | |
| 289 | + // Like `gh pr merge --disable-auto`: turn auto-merge off. Diffinery should not land it by | |
| 290 | + // itself afterwards, so a pull request auto-merge (or someone) already queued leaves the queue too. | |
| 291 | + await api.delete(`${repoPath(repo)}/pulls/${n}/auto-merge`); | |
| 292 | + let dequeued = true; | |
| 293 | + try { | |
| 294 | + await api.delete(`${repoPath(repo)}/pulls/${n}/merge-queue`); | |
| 295 | + } catch (e) { | |
| 296 | + if (!(e instanceof ApiError) || e.status !== 404) throw e; | |
| 297 | + dequeued = false; | |
| 298 | + } | |
| 299 | + if (ctx.json) return json(ctx.io, { auto_merge: null, dequeued }); | |
| 300 | + return ctx.out(`${ctx.c.green("✓")} Turned off auto-merge for #${n}${dequeued ? " and removed it from the merge queue" : ""}`); | |
| 288 | 301 | } |
| 289 | - const explicit = ["merge", "squash", "rebase"].some((k) => o[k] === true); | |
| 302 | + if (o.auto === true) { | |
| 303 | + // Like `gh pr merge --auto`: PUT /pulls/{n}/auto-merge. Without a method flag the server picks | |
| 304 | + // the merge queue's method, squash where linear history is required, else a merge commit. | |
| 305 | + const r = await api.put<{ state: "enabled" | "queued" | "merged"; auto_merge: { merge_method: string } | null; merge_commit_sha: string | null }>(`${repoPath(repo)}/pulls/${n}/auto-merge`, { | |
| 306 | + merge_method: explicit ? method : undefined, | |
| 307 | + delete_branch: o["delete-branch"] === true ? true : undefined, | |
| 308 | + commit_title: str(o, "subject"), | |
| 309 | + commit_message: await bodyFrom(ctx, o), | |
| 310 | + sha: str(o, "match-head-commit"), | |
| 311 | + }); | |
| 312 | + if (ctx.json) return json(ctx.io, r); | |
| 313 | + if (r.state === "merged") return ctx.out(`${ctx.c.green("✓")} Merged #${n} ${ctx.c.dim(String(r.merge_commit_sha ?? "").slice(0, 12))}`); | |
| 314 | + if (r.state === "queued") return ctx.out(`${ctx.c.green("✓")} Added #${n} to the merge queue`); | |
| 315 | + return ctx.out(`${ctx.c.green("✓")} Auto-merge is on for #${n} (${r.auto_merge?.merge_method ?? method}): it merges once every requirement is met`); | |
| 316 | + } | |
| 290 | 317 | const payload = { |
| 291 | 318 | merge_method: method, |
| 292 | 319 | delete_branch: o["delete-branch"] === true ? true : undefined, |
modifiedapps/cli/test/pr.test.ts+46−1View file
| @@ -85,13 +85,58 @@ | ||
| 85 | 85 | expect(r.code).toBe(0); |
| 86 | 86 | expect(r.stdout).toContain("Added #6 to the merge queue (position 2)"); |
| 87 | 87 | expect(srv.find("PUT", "/api/v3/repos/o/r/pulls/6/merge-queue")[0]!.body).toEqual({ delete_branch: true }); |
| 88 | + // Like gh, --disable-auto turns auto-merge off; a pull request already in the queue leaves it too. | |
| 89 | + srv.on("DELETE", "/api/v3/repos/o/r/pulls/6/auto-merge", { status: 204 }); | |
| 88 | 90 | srv.on("DELETE", "/api/v3/repos/o/r/pulls/6/merge-queue", { status: 204 }); |
| 89 | - expect((await run(["pr", "merge", "6", "-R", "o/r", "--disable-auto"], { env })).stdout).toContain("Removed #6 from the merge queue"); | |
| 91 | + const off = await run(["pr", "merge", "6", "-R", "o/r", "--disable-auto"], { env }); | |
| 92 | + expect(off.code).toBe(0); | |
| 93 | + expect(off.stdout).toContain("Turned off auto-merge for #6 and removed it from the merge queue"); | |
| 94 | + expect(srv.find("DELETE", "/api/v3/repos/o/r/pulls/6/auto-merge")).toHaveLength(1); | |
| 90 | 95 | const admin = await run(["pr", "merge", "6", "-R", "o/r", "--admin"], { env }); |
| 91 | 96 | expect(admin.code).toBe(1); |
| 92 | 97 | expect(srv.find("PUT", "/api/v3/repos/o/r/pulls/6/merge").at(-1)!.body).toEqual({ merge_method: "merge", bypass_merge_queue: true, bypass_rules: true }); |
| 93 | 98 | }); |
| 94 | 99 | |
| 100 | + it("pr merge --auto enables auto-merge like gh, and --disable-auto turns it off", async () => { | |
| 101 | + srv.on("PUT", "/api/v3/repos/o/r/pulls/8/auto-merge", { body: { state: "enabled", auto_merge: { merge_method: "squash", enabled_by: { login: "alice" } }, merge_commit_sha: null } }); | |
| 102 | + const r = await run(["pr", "merge", "8", "-R", "o/r", "--auto", "--squash", "-d", "-t", "Ship it", "--match-head-commit", "abc"], { env }); | |
| 103 | + expect(r.stderr).toBe(""); | |
| 104 | + expect(r.code).toBe(0); | |
| 105 | + expect(r.stdout).toContain("Auto-merge is on for #8 (squash)"); | |
| 106 | + expect(srv.find("PUT", "/api/v3/repos/o/r/pulls/8/auto-merge")[0]!.body).toEqual({ merge_method: "squash", delete_branch: true, commit_title: "Ship it", sha: "abc" }); | |
| 107 | + // It never tries a direct merge or the queue itself. | |
| 108 | + expect(srv.find("PUT", "/api/v3/repos/o/r/pulls/8/merge")).toHaveLength(0); | |
| 109 | + expect(srv.find("PUT", "/api/v3/repos/o/r/pulls/8/merge-queue")).toHaveLength(0); | |
| 110 | + // Without a method flag the server picks one (the queue's, or squash under linear history). | |
| 111 | + expect((await run(["pr", "merge", "8", "-R", "o/r", "--auto"], { env })).code).toBe(0); | |
| 112 | + expect(srv.find("PUT", "/api/v3/repos/o/r/pulls/8/auto-merge")[1]!.body).toEqual({}); | |
| 113 | + // When it is ready already, it merges (or joins the queue) at once. | |
| 114 | + srv.on("PUT", "/api/v3/repos/o/r/pulls/9/auto-merge", { body: { state: "merged", auto_merge: null, merge_commit_sha: "0123456789abcdef" } }); | |
| 115 | + expect((await run(["pr", "merge", "9", "-R", "o/r", "--auto"], { env })).stdout).toContain("Merged #9 0123456789ab"); | |
| 116 | + srv.on("PUT", "/api/v3/repos/o/r/pulls/10/auto-merge", { body: { state: "queued", auto_merge: null, merge_commit_sha: null } }); | |
| 117 | + expect((await run(["pr", "merge", "10", "-R", "o/r", "--auto"], { env })).stdout).toContain("Added #10 to the merge queue"); | |
| 118 | + const json = await run(["pr", "merge", "8", "-R", "o/r", "--auto", "--json"], { env }); | |
| 119 | + expect(JSON.parse(json.stdout).state).toBe("enabled"); | |
| 120 | + // --auto can't be combined with --admin (a bypass) or --disable-auto. | |
| 121 | + expect((await run(["pr", "merge", "8", "-R", "o/r", "--auto", "--admin"], { env })).code).toBe(2); | |
| 122 | + expect((await run(["pr", "merge", "8", "-R", "o/r", "--auto", "--disable-auto"], { env })).code).toBe(2); | |
| 123 | + // --disable-auto sends DELETE to the same path; not being in a queue is fine. | |
| 124 | + srv.on("DELETE", "/api/v3/repos/o/r/pulls/8/auto-merge", { status: 204 }); | |
| 125 | + srv.on("DELETE", "/api/v3/repos/o/r/pulls/8/merge-queue", { status: 404, body: { message: "#8 is not in the merge queue" } }); | |
| 126 | + const off = await run(["pr", "merge", "8", "-R", "o/r", "--disable-auto"], { env }); | |
| 127 | + expect(off.stderr).toBe(""); | |
| 128 | + expect(off.code).toBe(0); | |
| 129 | + expect(off.stdout).toContain("Turned off auto-merge for #8"); | |
| 130 | + expect(off.stdout).not.toContain("merge queue"); | |
| 131 | + expect(srv.find("DELETE", "/api/v3/repos/o/r/pulls/8/auto-merge")).toHaveLength(1); | |
| 132 | + expect(JSON.parse((await run(["pr", "merge", "8", "-R", "o/r", "--disable-auto", "--json"], { env })).stdout)).toEqual({ auto_merge: null, dequeued: false }); | |
| 133 | + // Other refusals still surface. | |
| 134 | + srv.on("DELETE", "/api/v3/repos/o/r/pulls/11/auto-merge", { status: 403, body: { message: "write access is required to disable auto-merge" } }); | |
| 135 | + const denied = await run(["pr", "merge", "11", "-R", "o/r", "--disable-auto"], { env }); | |
| 136 | + expect(denied.code).toBe(1); | |
| 137 | + expect(denied.stderr).toContain("write access is required"); | |
| 138 | + }); | |
| 139 | + | |
| 95 | 140 | it("surfaces the server's { message } on errors", async () => { |
| 96 | 141 | srv.on("PUT", "/api/v3/repos/o/r/pulls/4/merge", { status: 405, body: { message: "pull request is not mergeable: 1 approval required" } }); |
| 97 | 142 | const r = await run(["pr", "merge", "4", "-R", "o/r"], { env }); |
modifiedapps/diffinery/docs/agents/mcp.md+3−3View file
| @@ -157,12 +157,12 @@ | ||
| 157 | 157 | | Tool | Read-only | What it does | Minimum role | |
| 158 | 158 | | --- | --- | --- | --- | |
| 159 | 159 | | `list_pull_requests` | Yes | Up to 100 pull requests by `state` (`open`, `closed`, `all`) | read | |
| 160 | -| `get_pull_request` | Yes | A pull request with its comments, reviews and mergeability (including blockers); `include_diff: true` adds the files and patches | read | | |
| 160 | +| `get_pull_request` | Yes | A pull request with its comments, reviews and mergeability (including blockers); `include_diff: true` adds the files and patches. `mergeability.blockers` is what stops the caller merging it directly now. For agents and workflow tokens it also returns `auto_merge_blockers`: what stops Diffinery landing it through auto-merge or the merge queue (`[]` means it would land now). An agent on a protected branch sees `agents may not merge into this branch` in the first list but not in the second when the rule allows auto-merge. | read | | |
| 161 | 161 | | `create_pull_request` | No | Open a pull request: `title`, `head`, optional `base`, `body`, `draft`, `intent`, `reviewers` | write | |
| 162 | 162 | | `comment` | No | Comment on an issue or pull request | triage | |
| 163 | 163 | | `review_pull_request` | No | Submit an `APPROVE`, `REQUEST_CHANGES` or `COMMENT` review, with optional line `comments` | read (write to approve or request changes) | |
| 164 | 164 | | `merge_pull_request` | No | Merge with `method`, `expected_head_sha`, `delete_branch`; queues the pull request when the base branch uses a merge queue. Agents can't merge into the default branch or protected branches this way: they use `enable_auto_merge` | write | |
| 165 | -| `enable_auto_merge` | No | Turn on auto-merge, optionally with `merge_method`, `commit_title` and `commit_message`: Diffinery merges the pull request (or adds it to the merge queue) once every branch rule is met. Returns `state` (`enabled`, `queued` or `merged`), `autoMerge` and, once merged, `mergeCommit`. See [Auto-merge](/docs/collaboration/pull-requests#auto-merge) | write | | |
| 165 | +| `enable_auto_merge` | No | Turn on auto-merge, optionally with `merge_method`, `commit_title`, `commit_message`, `delete_branch` and `expected_head_sha`: Diffinery merges the pull request (or adds it to the merge queue) once every branch rule is met. Returns `state` (`enabled`, `queued` or `merged`), `autoMerge` and, once merged, `mergeCommit`. See [Auto-merge](/docs/collaboration/pull-requests#auto-merge) and [Agents landing changes](/docs/repositories/branch-protection#agents-landing-changes) | write | | |
| 166 | 166 | | `disable_auto_merge` | No | Turn auto-merge off; returns `{"auto_merge": null}` | write | |
| 167 | 167 | | `dequeue_pull_request` | No | Remove a pull request from the merge queue | write | |
| 168 | 168 | |
| @@ -449,7 +449,7 @@ | ||
| 449 | 449 | 5. `sandbox_create` and `sandbox_exec` to build and run tests when needed. |
| 450 | 450 | 6. `create_pull_request` with an `intent` that states the goal. |
| 451 | 451 | 7. `list_workflow_runs`, `get_workflow_run` and `get_job_logs` to follow CI, fixing failures with more commits. |
| 452 | -8. `get_pull_request` to read reviews and the remaining merge blockers. A person approves and merges. Or the agent calls `enable_auto_merge` where the branch allows it (`allowAgentAutoMerge`), and Diffinery merges once the checks pass and a person has approved, or merges with `merge_pull_request` where the branch allows agents to merge. | |
| 452 | +8. `get_pull_request` to read reviews and the remaining merge blockers. A person approves and merges. Or the agent calls `enable_auto_merge` where the branch allows it (`allowAgentAutoMerge`), and Diffinery merges once the checks pass and a person has approved (or without the approval where an admin turned on `agentAutoMergeWithoutApproval`); `auto_merge_blockers` lists what auto-merge still needs. Or it merges with `merge_pull_request` where the branch allows agents to merge. | |
| 453 | 453 | |
| 454 | 454 | ## Limitations |
| 455 | 455 |
modifiedapps/diffinery/docs/collaboration/merge-queue.md+1−1View file
| @@ -247,7 +247,7 @@ | ||
| 247 | 247 | ## Remove a pull request from the queue |
| 248 | 248 | |
| 249 | 249 | - **Web UI:** click **Remove from queue** in the merge box. You see `Removed from the merge queue.` |
| 250 | -- **CLI:** `diffinery pr merge 12 --disable-auto` | |
| 250 | +- **CLI:** `diffinery pr merge 12 --disable-auto`. Like `gh`, it turns auto-merge off, and a pull request that is in the queue also leaves it. | |
| 251 | 251 | - **REST API:** `DELETE /repos/{owner}/{repo}/pulls/{number}/merge-queue` (returns `204`, or `404 #12 is not in the merge queue`). |
| 252 | 252 | - **MCP:** the `dequeue_pull_request` tool. |
| 253 | 253 |
modifiedapps/diffinery/docs/collaboration/pull-requests.md+9−6View file
| @@ -486,13 +486,16 @@ | ||
| 486 | 486 | | `-m, --merge` | Create a merge commit (default) | |
| 487 | 487 | | `-s, --squash` | Squash into one commit | |
| 488 | 488 | | `-r, --rebase` | Fast-forward the base to the head | |
| 489 | -| `-d, --delete-branch` | Delete the head branch after merging | | |
| 489 | +| `-d, --delete-branch` | Delete the head branch after merging (with `--auto`, once it lands). A merge never deletes the default branch or a protected branch. | | |
| 490 | 490 | | `-t, --subject TEXT` | Commit title | |
| 491 | 491 | | `-b, --body TEXT`, `--body-file FILE` | Commit message body | |
| 492 | 492 | | `--match-head-commit SHA` | Only merge if the head is still at this commit | |
| 493 | 493 | | `--admin` | Merge now, bypassing unmet branch rules and any merge queue (repository admins; audited). Sends `bypass_rules` and `bypass_merge_queue`. See [Admin bypass](#admin-bypass). | |
| 494 | -| `--disable-auto` | Remove the pull request from the merge queue. It doesn't turn off [auto-merge](#auto-merge). | | |
| 494 | +| `--auto` | Turn on auto-merge instead of merging now: Diffinery merges (or adds it to the merge queue) once every requirement is met. Sends `PUT /repos/{owner}/{repo}/pulls/{number}/auto-merge` with the method flag, `--delete-branch`, `--subject`, `--body` and `--match-head-commit` you pass. Can't be combined with `--admin`. | | |
| 495 | +| `--disable-auto` | Turn auto-merge off (`DELETE /repos/{owner}/{repo}/pulls/{number}/auto-merge`). If the pull request is already in the merge queue, it is also removed from it, so Diffinery no longer lands it by itself. | | |
| 495 | 496 | |
| 497 | +Like `gh pr merge --auto`, `diffinery pr merge 12 --auto --squash` prints `✓ Auto-merge is on for #12 (squash): it merges once every requirement is met`, or `✓ Merged #12` / `✓ Added #12 to the merge queue` when the pull request was ready already. Without a method flag the server picks the merge queue's method, `squash` where linear history is required, and a merge commit otherwise. `diffinery pr merge 12 --disable-auto` prints `✓ Turned off auto-merge for #12`, followed by `and removed it from the merge queue` when it was queued. | |
| 498 | + | |
| 496 | 499 | ### Merge with the REST API |
| 497 | 500 | |
| 498 | 501 | ```sh |
| @@ -698,18 +701,18 @@ | ||
| 698 | 701 | |
| 699 | 702 | ### Turn on auto-merge with the CLI |
| 700 | 703 | |
| 701 | -The CLI has no auto-merge command or flag yet (`diffinery pr merge --disable-auto` removes a pull request from the merge queue; it doesn't touch auto-merge). Call the REST endpoints through `diffinery api`: | |
| 704 | +`diffinery pr merge --auto` turns auto-merge on, with the same flags as a merge, and `--disable-auto` turns it off: | |
| 702 | 705 | |
| 703 | 706 | ```sh |
| 704 | -diffinery api PUT /repos/acme/website/pulls/12/auto-merge -f merge_method=squash | |
| 705 | -diffinery api DELETE /repos/acme/website/pulls/12/auto-merge | |
| 707 | +diffinery pr merge 12 --auto --squash --delete-branch | |
| 708 | +diffinery pr merge 12 --disable-auto | |
| 706 | 709 | ``` |
| 707 | 710 | |
| 708 | 711 | `diffinery pr view 12` lists the merge blockers auto-merge is waiting on. |
| 709 | 712 | |
| 710 | 713 | ### Turn on auto-merge over MCP |
| 711 | 714 | |
| 712 | -`enable_auto_merge` takes `owner`, `repo`, `number`, and optionally `merge_method`, `commit_title` and `commit_message`. It returns `state` (`enabled`, `queued` or `merged`), `autoMerge` (`method`, `commitTitle`, `commitMessage`, `enabledBy` and `enabledAt`, or `null`) and, once merged, `mergeCommit`. `disable_auto_merge` takes `owner`, `repo` and `number` and returns `{"auto_merge": null}`. `get_pull_request` shows the current setting as `issue.pull.autoMerge`. Both tools need a `write` token. See [MCP server](/docs/agents/mcp). | |
| 715 | +`enable_auto_merge` takes `owner`, `repo`, `number`, and optionally `merge_method`, `commit_title`, `commit_message`, `delete_branch` and `expected_head_sha`. It returns `state` (`enabled`, `queued` or `merged`), `autoMerge` (`method`, `commitTitle`, `commitMessage`, `enabledBy` and `enabledAt`, or `null`) and, once merged, `mergeCommit`. `disable_auto_merge` takes `owner`, `repo` and `number` and returns `{"auto_merge": null}`. `get_pull_request` shows the current setting as `issue.pull.autoMerge`. Both tools need a `write` token. See [MCP server](/docs/agents/mcp). | |
| 713 | 716 | |
| 714 | 717 | ### When auto-merge turns itself off |
| 715 | 718 |
modifiedapps/diffinery/docs/reference/rest-api.md+3−1View file
| @@ -64,7 +64,8 @@ | ||
| 64 | 64 | |
| 65 | 65 | | Object | Extra fields | |
| 66 | 66 | | --- | --- | |
| 67 | -| Pull request (`GET /pulls/{n}`) | `merge_blockers` (why it can't merge yet), `approvals`, `required_approvals`, `checks`, `merge_queue` | | |
| 67 | +| Pull request (`GET /pulls/{n}`) | `merge_blockers` (why the caller can't merge it directly yet), `approvals`, `required_approvals`, `checks`, `merge_queue`; for agents and workflow tokens also `auto_merge_blockers` (why auto-merge or the merge queue can't land it for them yet; `[]` means it would now). People merge and auto-merge on the same terms, so they don't get it. | | |
| 68 | +| Pull request `auto_merge` | `delete_branch`: whether the head branch is deleted once auto-merge lands it | | |
| 68 | 69 | | Pull request (every response) | `intent`, a short statement of what the change is for, written for reviewers | |
| 69 | 70 | | Repository (`GET /repos/{owner}/{repo}`) | `stats` (counts of open issues and pull requests and more), and `permissions.role` | |
| 70 | 71 | | Workflow run | `sha` and `ref` (what the run checked out: the test merge commit for `pull_request` runs) and `pull_request_number` | |
| @@ -640,6 +641,7 @@ | ||
| 640 | 641 | - `POST /api/v3/user/orgs` creates an organization; `POST /api/v3/invitations` (site administrators) and `POST /api/v3/orgs/{org}/invitations` invite people by email. `GET /api/v3/orgs/{org}` and `PATCH /api/v3/orgs/{org}` (owners) follow GitHub: `default_repository_permission` is the organization's base permission (`none`, `read`, `write` or `admin`), shown to members and site administrators, and `PATCH` also takes the display `name`. See [Base permission](/docs/accounts/organizations#base-permission). |
| 641 | 642 | - `PUT /api/v3/user/avatar` sets your profile picture from the raw image in the request body (JPEG, PNG or WebP, up to 5 MB and 3072×3072 pixels), and `DELETE` goes back to generated initials. `PUT` and `DELETE /api/v3/orgs/{org}/avatar` do the same for an organization (owners only). Both return `{"avatar_url": ...}`. `GET /api/avatars/{login}?s=64|128|256` serves the picture to anyone, as WebP, or initials as SVG; it is the `avatar_url` of every user object. |
| 642 | 643 | - `PUT`, `DELETE /repos/{owner}/{repo}/pulls/{n}/merge-queue` and `GET /repos/{owner}/{repo}/merge-queue/{branch}` drive the merge queue. |
| 644 | +- `PUT /repos/{owner}/{repo}/pulls/{n}/auto-merge` turns on auto-merge (GitHub's `enablePullRequestAutoMerge`, which it offers through GraphQL only) with `merge_method`, `commit_title`, `commit_message`, `delete_branch` and `sha` (`409` if the head moved); `DELETE` turns it off (`diffinery pr merge --auto` and `--disable-auto`). It returns `{"state": "enabled" | "queued" | "merged", "auto_merge": ..., "merge_commit_sha": ...}`. | |
| 643 | 645 | - `POST /repos/{owner}/{repo}/actions/runners/claim` is how self-hosted runners pick up jobs (see [Runners](/docs/actions/runners)). |
| 644 | 646 | - `GET /repos/{owner}/{repo}/raw/{ref}/{path}` returns a file's raw bytes. |
| 645 | 647 | - `GET /repos/{owner}/{repo}/packages` lists container images linked to a repository, and `PATCH` on a package changes its visibility or linked repository. |
modifiedapps/diffinery/docs/repositories/branch-protection.md+5−2View file
| @@ -133,7 +133,7 @@ | ||
| 133 | 133 | |
| 134 | 134 | ### Agents landing changes |
| 135 | 135 | |
| 136 | -`allowAgentAutoMerge` lets agents land their work on a protected or default branch without ever merging it themselves and without any bypass. An agent turns on auto-merge for its pull request (`PUT /repos/{owner}/{repo}/pulls/{number}/auto-merge`, or the MCP tool `enable_auto_merge`), and Diffinery merges it once every rule of the branch is met: required checks green on the head commit, approvals from people (an agent's pull request still needs one), no changes requested, linear history and an up-to-date branch where required. On a branch with a merge queue, the pull request joins the queue instead, and the queue's own checks gate it. | |
| 136 | +`allowAgentAutoMerge` lets agents land their work on a protected or default branch without ever merging it themselves and without any bypass. An agent turns on auto-merge for its pull request (`PUT /repos/{owner}/{repo}/pulls/{number}/auto-merge`, `diffinery pr merge 12 --auto`, or the MCP tool `enable_auto_merge`), and Diffinery merges it once every rule of the branch is met: required checks green on the head commit, approvals from people (an agent's pull request still needs one), no changes requested, linear history and an up-to-date branch where required. On a branch with a merge queue, the pull request joins the queue instead, and the queue's own checks gate it. | |
| 137 | 137 | |
| 138 | 138 | - The rule must set `allowAgentAutoMerge` and have at least one required check. Without the setting the agent gets `403` (`agents may not have pull requests merged into main automatically unless its branch protection allows it ...`); without a required check, `422` (`main has no required status checks: allowAgentAutoMerge needs at least one ...`). The same holds for adding a pull request to the merge queue. |
| 139 | 139 | - A pull request that adds, changes or deletes a workflow file can't land this way (`the pull request changes workflow file ...; a person must merge it`), since the workflows are what gate it. |
| @@ -141,7 +141,10 @@ | ||
| 141 | 141 | - Only people with the admin role change the setting, like every other rule; agents and workflow tokens never can. Turning it on or off is audited as `repo.protection.agent_auto_merge`. |
| 142 | 142 | - The merge is made as the agent, on behalf of its owner, and its commit carries the `diffinery-agent` header. Workflows that the merge triggers run as code from an agent, so their tokens are held to the agent rules too. |
| 143 | 143 | - A workflow's `GITHUB_TOKEN` is held to the same rule as agents (and needs `contents: write`), even when the run's code didn't come from an agent. |
| 144 | -- The agent's access is checked again when the merge happens, and auto-merge turns itself off, with a comment saying why, if the rule no longer allows it, or if someone else pushes to the pull request's branch. | |
| 144 | +- The agent's access is checked again when the merge happens, and auto-merge turns itself off, with a comment saying why, if the rule no longer allows it, if the merge queue refuses the pull request, or if someone else pushes to the pull request's branch. | |
| 145 | +- An archived repository lands nothing by itself. Archiving it turns every waiting auto-merge off (`Auto-merge was turned off: the repository is archived.`) and empties its merge queues (`This pull request was removed from the merge queue: the repository is archived.`); unarchiving doesn't turn them back on. | |
| 146 | +- With `delete_branch` (`diffinery pr merge 12 --auto --delete-branch`), the head branch is deleted once the pull request lands, directly or through the queue, as with a manual merge. A merge never deletes the default branch or a protected branch, even where its rule sets `allowDeletion`. | |
| 147 | +- `GET /repos/{owner}/{repo}/pulls/{number}` and the MCP tool `get_pull_request` show agents and workflow tokens `auto_merge_blockers`, what auto-merge still needs, next to the blockers of a direct merge (which keep saying `agents may not merge into this branch; a human must merge`). | |
| 145 | 148 | |
| 146 | 149 | Turning on auto-merge, turning it off and the merge itself are audited (`pull_request.auto_merge_enabled`, `pull_request.auto_merge_disabled`, `pull_request.auto_merged`, or `pull_request.auto_merge_enqueued` when it joins a merge queue). How to turn auto-merge on and off in the web UI, the API and MCP, and when it turns itself off, is in [Auto-merge](/docs/collaboration/pull-requests#auto-merge). |
| 147 | 150 |
modifiedapps/diffinery/src/http/api.ts+4−2View file
| @@ -691,8 +691,10 @@ | ||
| 691 | 691 | // Auto-merge (GitHub's enablePullRequestAutoMerge and disablePullRequestAutoMerge, which it exposes through GraphQL only). |
| 692 | 692 | api.put("/repos/:owner/:repo/pulls/:number/auto-merge", async (c) => { |
| 693 | 693 | const { stub, caller } = await loadRepo(c, P(c).owner, P(c).repo, "write"); |
| 694 | - const b = await body<{ merge_method?: string; commit_title?: string; commit_message?: string }>(c).catch(() => ({}) as Record<string, undefined>); | |
| 695 | - const r = await stub.enableAutoMerge(num(c), { method: b.merge_method, commitTitle: b.commit_title, commitMessage: b.commit_message }, caller); | |
| 694 | + const b = await body<{ merge_method?: string; commit_title?: string; commit_message?: string; delete_branch?: unknown; sha?: unknown }>(c).catch(() => ({}) as Record<string, undefined>); | |
| 695 | + if (b.delete_branch !== undefined && typeof b.delete_branch !== "boolean") throw new HttpError(422, "delete_branch must be a boolean", { errors: [{ resource: "PullRequest", code: "invalid", field: "delete_branch" }] }); | |
| 696 | + if (b.sha !== undefined && typeof b.sha !== "string") throw new HttpError(422, "sha must be a string", { errors: [{ resource: "PullRequest", code: "invalid", field: "sha" }] }); | |
| 697 | + const r = await stub.enableAutoMerge(num(c), { method: b.merge_method, commitTitle: b.commit_title, commitMessage: b.commit_message, deleteBranch: b.delete_branch, expectedHeadSha: b.sha || undefined }, caller); | |
| 696 | 698 | return c.json({ state: r.state, auto_merge: ghAutoMerge(baseOf(c), r.autoMerge), merge_commit_sha: r.mergeCommit ?? null }); |
| 697 | 699 | }); |
| 698 | 700 | api.delete("/repos/:owner/:repo/pulls/:number/auto-merge", async (c) => { |
modifiedapps/diffinery/src/http/gh.ts+3−2View file
| @@ -251,11 +251,12 @@ | ||
| 251 | 251 | } |
| 252 | 252 | |
| 253 | 253 | /** Auto-merge as `../repo/auto-merge.ts` reports it. */ |
| 254 | -export type AutoMergeLike = { method: string; commitTitle: string | null; commitMessage: string | null; enabledBy: Who }; | |
| 254 | +export type AutoMergeLike = { method: string; commitTitle: string | null; commitMessage: string | null; enabledBy: Who; deleteBranch?: boolean }; | |
| 255 | 255 | |
| 256 | 256 | /** GitHub's `auto_merge` object of a pull request (null while auto-merge is off). */ |
| 257 | 257 | export function ghAutoMerge(base: string, a: AutoMergeLike | null | undefined) { |
| 258 | - return a ? { enabled_by: ghUser(base, a.enabledBy), merge_method: a.method, commit_title: a.commitTitle, commit_message: a.commitMessage } : null; | |
| 258 | + // delete_branch is Diffinery's: whether the head branch is deleted once it lands. | |
| 259 | + return a ? { enabled_by: ghUser(base, a.enabledBy), merge_method: a.method, commit_title: a.commitTitle, commit_message: a.commitMessage, delete_branch: !!a.deleteBranch } : null; | |
| 259 | 260 | } |
| 260 | 261 | |
| 261 | 262 | /** Per-request lookups the issue builders need (labels by name, people by login, reactions by subject). */ |
modifiedapps/diffinery/src/http/ghctx.ts+5−1View file
| @@ -6,6 +6,7 @@ | ||
| 6 | 6 | import type { AppContext } from "../auth/principal.js"; |
| 7 | 7 | import type { Repo } from "../lib/directory.js"; |
| 8 | 8 | import { HttpError } from "../lib/util.js"; |
| 9 | +import { isPerson } from "../repo/auto-merge.js"; | |
| 9 | 10 | import type { Caller, Role } from "../repo/types.js"; |
| 10 | 11 | import { loadRepo, type repoStub } from "./common.js"; |
| 11 | 12 | import { baseOf, ghIssue, ghPull, ghRepo, type IssueCtx, type IssueLike, type Who } from "./gh.js"; |
| @@ -113,7 +114,8 @@ | ||
| 113 | 114 | const { issue, comments } = await stub.issue(number); |
| 114 | 115 | if (issue.kind !== "pull") throw new HttpError(404, "not a pull request"); |
| 115 | 116 | const [pull] = await pullsOut(c, repo, stub, [issue]); |
| 116 | - const m = await stub.mergeability(number, caller); | |
| 117 | + // For agents and workflow tokens, also what Diffinery itself would need to land it for them (auto-merge, the merge queue). | |
| 118 | + const [m, automatic] = await Promise.all([stub.mergeability(number, caller), isPerson(caller) ? null : stub.mergeability(number, caller, { automatic: true })]); | |
| 117 | 119 | const open = issue.state === "open"; |
| 118 | 120 | const commits = open || issue.pull!.merged ? (await stub.pullCommits(number)).length : 0; |
| 119 | 121 | const files = await stub.pullFiles(number).catch(() => null); |
| @@ -133,5 +135,7 @@ | ||
| 133 | 135 | required_approvals: m.requiredApprovals, |
| 134 | 136 | checks: m.checks, |
| 135 | 137 | merge_queue: m.mergeQueue, |
| 138 | + // Diffinery, agents and workflow tokens only: why auto-merge (or the merge queue) can't land it yet; [] means it would now. | |
| 139 | + ...(automatic ? { auto_merge_blockers: automatic.blockers } : {}), | |
| 136 | 140 | }; |
| 137 | 141 | } |
modifiedapps/diffinery/src/lib/directory.ts+3−1View file
| @@ -578,7 +578,7 @@ | ||
| 578 | 578 | // A new repository at a moved one's old name takes it over for good, as on GitHub. |
| 579 | 579 | ...this.claimName(owner.login, input.name), |
| 580 | 580 | ]); |
| 581 | - const meta: RepoMeta = { repoId: id, ownerId: owner.id, owner: owner.login, name: input.name, defaultBranch: branch, ownerKind: owner.kind === "org" ? "org" : "user" }; | |
| 581 | + const meta: RepoMeta = { repoId: id, ownerId: owner.id, owner: owner.login, name: input.name, defaultBranch: branch, ownerKind: owner.kind === "org" ? "org" : "user", archived: false }; | |
| 582 | 582 | const stub = this.env.REPO.get(this.env.REPO.idFromName(id)); |
| 583 | 583 | await stub.init(meta); |
| 584 | 584 | if (owner.kind === "org") { |
| @@ -608,6 +608,8 @@ | ||
| 608 | 608 | .run(); |
| 609 | 609 | } |
| 610 | 610 | if (patch.name || patch.defaultBranch) await this.env.REPO.get(this.env.REPO.idFromName(repo.id)).updateMeta({ name: patch.name ?? repo.name, defaultBranch: patch.defaultBranch ?? repo.default_branch }); |
| 611 | + // The repository's object lands pull requests by itself (auto-merge, the merge queue): it must know, and stop. | |
| 612 | + if (patch.archived !== undefined) await this.env.REPO.get(this.env.REPO.idFromName(repo.id)).setArchived(patch.archived); | |
| 611 | 613 | if (renamed) await this.forgetArchives(repo.id); |
| 612 | 614 | return (await this.repoById(repo.id))!; |
| 613 | 615 | } |
modifiedapps/diffinery/src/mcp/server.ts+9−4View file
| @@ -10,6 +10,7 @@ | ||
| 10 | 10 | import { AGENTIC_TOOLS } from "../agentic/mcp.js"; |
| 11 | 11 | import { ImportService } from "../imports/service.js"; |
| 12 | 12 | import { importJson } from "../http/imports.js"; |
| 13 | +import { isPerson } from "../repo/auto-merge.js"; | |
| 13 | 14 | |
| 14 | 15 | /** |
| 15 | 16 | * Model Context Protocol server (Streamable HTTP transport, JSON responses). |
| @@ -169,16 +170,18 @@ | ||
| 169 | 170 | }, |
| 170 | 171 | }, |
| 171 | 172 | get_pull_request: { |
| 172 | - description: "Get a pull request with its conversation, reviews, mergeability and (optionally) the full diff.", | |
| 173 | + description: | |
| 174 | + "Get a pull request with its conversation, reviews, mergeability and (optionally) the full diff. `mergeability.blockers` is what stops you merging it yourself now. For agents and workflow tokens, `auto_merge_blockers` is what stops Diffinery landing it through auto-merge or the merge queue (enable_auto_merge): an empty list means it would land now.", | |
| 173 | 175 | input: { ...OWNER_REPO, number: { type: "integer" }, include_diff: { type: "boolean" } }, |
| 174 | 176 | required: ["owner", "repo", "number"], |
| 175 | 177 | readOnly: true, |
| 176 | 178 | run: async (c, a) => { |
| 177 | 179 | const { stub, caller } = await repoOf(c, a); |
| 178 | 180 | const n = int(a, "number"); |
| 179 | - const [detail, mergeability] = await Promise.all([stub.issue(n), stub.mergeability(n, caller).catch(() => null)]); | |
| 181 | + // An agent may not merge into a protected branch itself, but auto-merge may land it for the agent: say what each needs. | |
| 182 | + const [detail, mergeability, automatic] = await Promise.all([stub.issue(n), stub.mergeability(n, caller).catch(() => null), isPerson(caller) ? null : stub.mergeability(n, caller, { automatic: true }).catch(() => null)]); | |
| 180 | 183 | const files = a.include_diff ? await stub.pullFiles(n) : undefined; |
| 181 | - return { ...detail, mergeability, files }; | |
| 184 | + return { ...detail, mergeability, ...(automatic ? { auto_merge_blockers: automatic.blockers } : {}), files }; | |
| 182 | 185 | }, |
| 183 | 186 | }, |
| 184 | 187 | create_pull_request: { |
| @@ -236,11 +239,13 @@ | ||
| 236 | 239 | merge_method: { type: "string", enum: ["merge", "squash", "rebase"], description: "Default: the merge queue's method, squash where linear history is required, else merge." }, |
| 237 | 240 | commit_title: { type: "string" }, |
| 238 | 241 | commit_message: { type: "string" }, |
| 242 | + delete_branch: { type: "boolean", description: "Delete the head branch once it lands (never a protected or default branch)." }, | |
| 243 | + expected_head_sha: { type: "string", description: "Only turn it on if the head is still at this commit." }, | |
| 239 | 244 | }, |
| 240 | 245 | required: ["owner", "repo", "number"], |
| 241 | 246 | run: async (c, a) => { |
| 242 | 247 | const { stub, caller } = await repoOf(c, a, "write"); |
| 243 | - return stub.enableAutoMerge(int(a, "number"), { method: optStr(a, "merge_method"), commitTitle: optStr(a, "commit_title"), commitMessage: optStr(a, "commit_message") }, caller); | |
| 248 | + return stub.enableAutoMerge(int(a, "number"), { method: optStr(a, "merge_method"), commitTitle: optStr(a, "commit_title"), commitMessage: optStr(a, "commit_message"), deleteBranch: a.delete_branch === true, expectedHeadSha: optStr(a, "expected_head_sha") }, caller); | |
| 244 | 249 | }, |
| 245 | 250 | }, |
| 246 | 251 | disable_auto_merge: { |
modifiedapps/diffinery/src/repo/RepoDO.ts+23−0View file
| @@ -139,6 +139,29 @@ | ||
| 139 | 139 | return next; |
| 140 | 140 | } |
| 141 | 141 | |
| 142 | + /** | |
| 143 | + * Whether the repository is archived (read-only). The Worker refuses writes to an archived | |
| 144 | + * repository, but work this object does by itself (auto-merge, the merge queue) must ask here. | |
| 145 | + */ | |
| 146 | + async isArchived(): Promise<boolean> { | |
| 147 | + const known = this.meta.archived; | |
| 148 | + if (typeof known === "boolean") return known; | |
| 149 | + const row = await this.env.DB.prepare("SELECT archived FROM repos WHERE id = ?").bind(this.meta.repoId).first<{ archived: number }>(); | |
| 150 | + // setArchived may have run while D1 answered; what it set wins. | |
| 151 | + if (typeof this.meta.archived === "boolean") return this.meta.archived; | |
| 152 | + const archived = !!row?.archived; | |
| 153 | + await this.updateMeta({ archived }); | |
| 154 | + return archived; | |
| 155 | + } | |
| 156 | + | |
| 157 | + /** The repository was archived or unarchived (D1 already says so). Archiving ends auto-merge and empties the merge queues. */ | |
| 158 | + async setArchived(archived: boolean): Promise<void> { | |
| 159 | + await this.updateMeta({ archived }); | |
| 160 | + if (!archived) return; | |
| 161 | + await this.autoMerge.repositoryArchived(); | |
| 162 | + await this.queue.repositoryArchived(); | |
| 163 | + } | |
| 164 | + | |
| 142 | 165 | /** Wipe all storage (repository deletion). */ |
| 143 | 166 | async destroy(): Promise<void> { |
| 144 | 167 | // Caches, artifacts, logs and archives live outside repos/<id>/; with the rows gone, nothing would remove them. |
modifiedapps/diffinery/src/repo/auto-merge.ts+29−5View file
| @@ -29,13 +29,18 @@ | ||
| 29 | 29 | /** The role (and a workflow token's permissions) it was enabled with: the merge never acts with more. */ |
| 30 | 30 | role: Role; |
| 31 | 31 | permissions?: Caller["permissions"]; |
| 32 | + /** Delete the head branch once it lands, as a manual merge's `delete_branch` does (collab.ts completeMerge says which branches). */ | |
| 33 | + deleteBranch?: boolean; | |
| 32 | 34 | }; |
| 33 | 35 | |
| 34 | 36 | /** What the API and the web show: who turned it on, and how it will merge. */ |
| 35 | -export type AutoMergeInfo = Pick<AutoMergeState, "method" | "commitTitle" | "commitMessage" | "enabledBy" | "enabledAt">; | |
| 37 | +export type AutoMergeInfo = Pick<AutoMergeState, "method" | "commitTitle" | "commitMessage" | "enabledBy" | "enabledAt"> & { deleteBranch: boolean }; | |
| 36 | 38 | |
| 37 | 39 | export type AutoMergeResult = { state: "enabled" | "queued" | "merged"; autoMerge: AutoMergeInfo | null; mergeCommit?: string | null }; |
| 38 | 40 | |
| 41 | +/** Why auto-merge turns itself off in an archived repository, which is read-only for everyone. */ | |
| 42 | +export const ARCHIVED_REASON = "the repository is archived"; | |
| 43 | + | |
| 39 | 44 | /** Comments about auto-merge the server wrote on its own (it was turned off because ...). */ |
| 40 | 45 | export const AUTO_MERGE_BOT: Caller = { id: "system:auto-merge", login: "diffinery-auto-merge[bot]", kind: "system", role: "admin" }; |
| 41 | 46 | |
| @@ -96,14 +101,15 @@ | ||
| 96 | 101 | } |
| 97 | 102 | |
| 98 | 103 | static info(s: AutoMergeState | null): AutoMergeInfo | null { |
| 99 | - return s ? { method: s.method, commitTitle: s.commitTitle, commitMessage: s.commitMessage, enabledBy: s.enabledBy, enabledAt: s.enabledAt } : null; | |
| 104 | + return s ? { method: s.method, commitTitle: s.commitTitle, commitMessage: s.commitMessage, enabledBy: s.enabledBy, enabledAt: s.enabledAt, deleteBranch: !!s.deleteBranch } : null; | |
| 100 | 105 | } |
| 101 | 106 | |
| 102 | 107 | /** Turn auto-merge on (or change how it merges), then merge or enqueue right away if the pull request is ready. */ |
| 103 | - async enable(number: number, input: { method?: string; commitTitle?: string | null; commitMessage?: string | null }, caller: Caller): Promise<AutoMergeResult> { | |
| 108 | + async enable(number: number, input: { method?: string; commitTitle?: string | null; commitMessage?: string | null; deleteBranch?: boolean; expectedHeadSha?: string }, caller: Caller): Promise<AutoMergeResult> { | |
| 104 | 109 | const { issue } = this.repo.collab.get(number); |
| 105 | 110 | const pull = issue.pull; |
| 106 | 111 | if (!pull) throw new HttpError(404, "not a pull request"); |
| 112 | + if (await this.repo.isArchived()) throw new HttpError(403, "repository is archived (read-only)"); | |
| 107 | 113 | if (pull.merged) throw new HttpError(422, "pull request is already merged"); |
| 108 | 114 | if (issue.state !== "open") throw new HttpError(422, "pull request is closed"); |
| 109 | 115 | if (pull.draft) throw new HttpError(422, "pull request is a draft; mark it ready for review first"); |
| @@ -119,6 +125,7 @@ | ||
| 119 | 125 | const method = (input.method as MergeMethod | undefined) ?? rules?.mergeQueue?.method ?? (rules?.requireLinearHistory ? "squash" : "merge"); |
| 120 | 126 | if (method === "merge" && rules?.requireLinearHistory) throw new HttpError(422, `${pull.base} requires linear history; use squash or rebase`); |
| 121 | 127 | if (this.repo.queue.isQueued(number)) throw new HttpError(422, "pull request is already in the merge queue"); |
| 128 | + if (input.expectedHeadSha && input.expectedHeadSha !== pull.headSha) throw new HttpError(409, "head branch was modified; review and try again"); | |
| 122 | 129 | const state: AutoMergeState = { |
| 123 | 130 | method, |
| 124 | 131 | commitTitle: typeof input.commitTitle === "string" && input.commitTitle ? input.commitTitle : null, |
| @@ -127,6 +134,7 @@ | ||
| 127 | 134 | enabledAt: now(), |
| 128 | 135 | role: caller.role, |
| 129 | 136 | ...(caller.permissions ? { permissions: caller.permissions } : {}), |
| 137 | + ...(input.deleteBranch === true ? { deleteBranch: true } : {}), | |
| 130 | 138 | }; |
| 131 | 139 | this.sql.exec("UPDATE pulls SET auto_merge = ? WHERE number = ?", JSON.stringify(state), number); |
| 132 | 140 | this.repo.social.record(number, "auto_merge_enabled", caller, { merge_method: method }); |
| @@ -181,6 +189,13 @@ | ||
| 181 | 189 | this.repo.broadcast(`pr:${number}`, { action: "auto_merge" }); |
| 182 | 190 | } |
| 183 | 191 | |
| 192 | + /** The repository was archived: nothing lands by itself any more, so every waiting auto-merge is turned off, saying why. */ | |
| 193 | + repositoryArchived(): Promise<void> { | |
| 194 | + return this.exclusive(async () => { | |
| 195 | + for (const n of this.pending({})) this.cancel(n, AUTO_MERGE_BOT, ARCHIVED_REASON); | |
| 196 | + }); | |
| 197 | + } | |
| 198 | + | |
| 184 | 199 | /** Re-evaluate in the background: after checks complete, reviews, protection changes and pushes. */ |
| 185 | 200 | schedule(filter: Filter = {}): void { |
| 186 | 201 | if (!this.pending(filter).length) return; |
| @@ -234,6 +249,8 @@ | ||
| 234 | 249 | const pull = issue.pull!; |
| 235 | 250 | if (pull.merged || issue.state !== "open") return this.clear(number); |
| 236 | 251 | if (this.repo.queue.isQueued(number)) return this.clear(number); |
| 252 | + // An archived repository is read-only: a check that finishes after the archive must not land anything. | |
| 253 | + if (await this.repo.isArchived()) return this.cancel(number, AUTO_MERGE_BOT, ARCHIVED_REASON); | |
| 237 | 254 | const caller = await this.callerOf(s); |
| 238 | 255 | if (!caller || !atLeast(caller.role, "write")) return this.cancel(number, AUTO_MERGE_BOT, `${s.enabledBy.login} no longer has write access`); |
| 239 | 256 | // The rules may have changed since it was turned on: an agent's auto-merge needs them to still allow it. |
| @@ -244,13 +261,20 @@ | ||
| 244 | 261 | const m = await this.repo.collab.mergeability(number, caller, { automatic: true }); |
| 245 | 262 | if (!m.mergeable) return; |
| 246 | 263 | if (rules?.mergeQueue) { |
| 247 | - const r = await this.repo.queue.enqueue(number, { method: s.method }, caller); | |
| 264 | + let r: Awaited<ReturnType<RepoDO["queue"]["enqueue"]>>; | |
| 265 | + try { | |
| 266 | + r = await this.repo.queue.enqueue(number, { method: s.method, deleteBranch: !!s.deleteBranch }, caller); | |
| 267 | + } catch (e) { | |
| 268 | + // Like a refused merge: the queue said no, so auto-merge is off and the comment says why. | |
| 269 | + if (e instanceof HttpError && e.status === 409) return; | |
| 270 | + return this.cancel(number, AUTO_MERGE_BOT, e instanceof HttpError ? e.detail : "the pull request could not be added to the merge queue"); | |
| 271 | + } | |
| 248 | 272 | this.clear(number); |
| 249 | 273 | this.audit("pull_request.auto_merge_enqueued", caller, number, { base: pull.base, state: r.state }); |
| 250 | 274 | return; |
| 251 | 275 | } |
| 252 | 276 | try { |
| 253 | - const r = await this.repo.collab.merge(number, { method: s.method, title: s.commitTitle ?? undefined, message: s.commitMessage ?? undefined, expectedHeadSha: pull.headSha }, caller, { automatic: true }); | |
| 277 | + const r = await this.repo.collab.merge(number, { method: s.method, title: s.commitTitle ?? undefined, message: s.commitMessage ?? undefined, expectedHeadSha: pull.headSha, deleteBranch: !!s.deleteBranch }, caller, { automatic: true }); | |
| 254 | 278 | this.audit("pull_request.auto_merged", caller, number, { base: pull.base, method: r.method, sha: r.sha }); |
| 255 | 279 | } catch (e) { |
| 256 | 280 | // The head or base moved under us: the next event looks again. Anything else won't fix itself. |
modifiedapps/diffinery/src/repo/collab.ts+14−2View file
| @@ -838,6 +838,8 @@ | ||
| 838 | 838 | ) { |
| 839 | 839 | const p = this.pullRow(number); |
| 840 | 840 | const r = this.row(number); |
| 841 | + // The Worker refuses writes to an archived repository; auto-merge, which runs here by itself, is refused here. | |
| 842 | + if (await this.repo.isArchived()) throw new HttpError(403, "repository is archived (read-only)"); | |
| 841 | 843 | if (input.expectedHeadSha && input.expectedHeadSha !== p.head_sha) throw new HttpError(409, "head branch was modified; review and try again"); |
| 842 | 844 | const m = await this.mergeability(number, caller, opts); |
| 843 | 845 | const rules = this.repo.protectionFor(p.base_ref) ?? {}; |
| @@ -887,6 +889,16 @@ | ||
| 887 | 889 | return { merged: true, sha: newSha, method }; |
| 888 | 890 | } |
| 889 | 891 | |
| 892 | + /** | |
| 893 | + * Whether landing the pull request may delete its head branch (a merge's `delete_branch`, also kept | |
| 894 | + * by auto-merge and the merge queue): only a branch of this repository, never the default branch, | |
| 895 | + * and never a protected one, whoever merges and whatever the rule's allowDeletion says. | |
| 896 | + */ | |
| 897 | + headBranchDeletable(number: number): boolean { | |
| 898 | + const p = this.pullRow(number); | |
| 899 | + return !this.repo.policy.isForkPull(number) && p.head_ref !== this.repo.meta.defaultBranch && !this.repo.protectionFor(p.head_ref); | |
| 900 | + } | |
| 901 | + | |
| 890 | 902 | /** Record a merge whose commit is already on the base branch (directly or through the merge queue). */ |
| 891 | 903 | async completeMerge(number: number, newSha: string, baseSha: string, caller: Caller, deleteBranch: boolean): Promise<void> { |
| 892 | 904 | const p = this.pullRow(number); |
| @@ -913,9 +925,9 @@ | ||
| 913 | 925 | /* unknown issue number */ |
| 914 | 926 | } |
| 915 | 927 | } |
| 916 | - if (deleteBranch && p.head_ref !== meta.defaultBranch) { | |
| 928 | + if (deleteBranch && this.headBranchDeletable(number)) { | |
| 917 | 929 | const headRef = `refs/heads/${p.head_ref}`; |
| 918 | - if (this.repo.getRef(headRef) === p.head_sha) this.repo.casRef(headRef, p.head_sha, ZERO_OID); | |
| 930 | + if (this.repo.casRef(headRef, p.head_sha, null)) await this.repo.afterRefUpdate(headRef, p.head_sha, ZERO_OID, serverCaller); | |
| 919 | 931 | } |
| 920 | 932 | this.repo.broadcast(`pr:${number}`, { action: "merged", sha: newSha }); |
| 921 | 933 | // pull_request workflows with `types: [closed]` (merged ones see github.event.pull_request.merged). |
modifiedapps/diffinery/src/repo/queue.ts+15−3View file
| @@ -1,6 +1,6 @@ | ||
| 1 | 1 | import { ZERO_OID, mergeBase, mergeTrees, readCommit, serializeCommit } from "@diffinery/git-core"; |
| 2 | 2 | import { squashSig } from "./collab.js"; |
| 3 | -import { automaticLandingRefusal } from "./auto-merge.js"; | |
| 3 | +import { ARCHIVED_REASON, automaticLandingRefusal } from "./auto-merge.js"; | |
| 4 | 4 | import { requiredCheck } from "./required.js"; |
| 5 | 5 | import type { Actor } from "@diffinery/shared"; |
| 6 | 6 | import { HttpError, now } from "../lib/util.js"; |
| @@ -155,6 +155,7 @@ | ||
| 155 | 155 | const { issue } = this.repo.collab.get(number); |
| 156 | 156 | const pull = issue.pull; |
| 157 | 157 | if (!pull) throw new HttpError(422, `#${number} is not a pull request`); |
| 158 | + if (await this.repo.isArchived()) throw new HttpError(403, "repository is archived (read-only)"); | |
| 158 | 159 | const settings = this.settings(pull.base); |
| 159 | 160 | if (!settings) throw new HttpError(422, `${pull.base} does not use a merge queue; merge the pull request directly`); |
| 160 | 161 | if (this.row(number)) return this.result(number); |
| @@ -203,6 +204,14 @@ | ||
| 203 | 204 | await this.process(r.base_ref); |
| 204 | 205 | } |
| 205 | 206 | |
| 207 | + /** The repository was archived: nothing merges any more, so every queue is emptied, saying why on each pull request. */ | |
| 208 | + repositoryArchived(): Promise<void> { | |
| 209 | + return this.exclusive(async () => { | |
| 210 | + for (const r of this.rows()) await this.remove(r, ARCHIVED_REASON); | |
| 211 | + this.armAlarm(); | |
| 212 | + }); | |
| 213 | + } | |
| 214 | + | |
| 206 | 215 | /** Re-evaluate in the background (after checks, pushes and pull request changes). */ |
| 207 | 216 | schedule(base?: string): void { |
| 208 | 217 | this.repo.background(this.process(base)); |
| @@ -232,8 +241,11 @@ | ||
| 232 | 241 | private async step(base: string): Promise<void> { |
| 233 | 242 | const settings = this.settings(base); |
| 234 | 243 | const baseRef = `refs/heads/${base}`; |
| 244 | + // An archived repository is read-only: a merge group's check that finishes after the archive merges nothing. | |
| 245 | + const archived = await this.repo.isArchived(); | |
| 235 | 246 | for (const r of this.rows(base)) { |
| 236 | - if (!settings) await this.remove(r, `${base} no longer uses a merge queue`); | |
| 247 | + if (archived) await this.remove(r, ARCHIVED_REASON); | |
| 248 | + else if (!settings) await this.remove(r, `${base} no longer uses a merge queue`); | |
| 237 | 249 | else if (!this.repo.getRef(baseRef)) await this.remove(r, `the base branch ${base} no longer exists`); |
| 238 | 250 | else { |
| 239 | 251 | const p = this.sql.exec<{ head_sha: string; merged: number; state: string }>("SELECT p.head_sha, p.merged, i.state FROM pulls p JOIN issues i ON i.number = p.number WHERE p.number = ?", r.number).toArray()[0]; |
| @@ -241,7 +253,7 @@ | ||
| 241 | 253 | else if (p.head_sha !== r.head_sha) await this.remove(r, "new commits were pushed to its branch"); |
| 242 | 254 | } |
| 243 | 255 | } |
| 244 | - if (!settings) return; | |
| 256 | + if (!settings || archived) return; | |
| 245 | 257 | |
| 246 | 258 | // Merge from the front: the front group is built on the current base and its checks passed. |
| 247 | 259 | for (;;) { |
modifiedapps/diffinery/src/repo/types.ts+5−0View file
| @@ -21,6 +21,11 @@ | ||
| 21 | 21 | defaultBranch: string; |
| 22 | 22 | /** Owner kind decides whether org secrets apply. */ |
| 23 | 23 | ownerKind: "user" | "org"; |
| 24 | + /** | |
| 25 | + * A copy of D1's repos.archived, which is the record (the Directory keeps it in sync, lib/directory.ts | |
| 26 | + * updateRepo). Absent in objects made before it existed: RepoDO.isArchived() then reads D1 once. | |
| 27 | + */ | |
| 28 | + archived?: boolean; | |
| 24 | 29 | } |
| 25 | 30 | |
| 26 | 31 | export interface ProtectionRules { |
modifiedapps/diffinery/src/web/collab.tsx+16−1View file
| @@ -842,6 +842,12 @@ | ||
| 842 | 842 | <span class="muted"> |
| 843 | 843 | {" "} |
| 844 | 844 | · turned on by <Actor login={by.login} kind={by.kind} onBehalfOf={by.onBehalfOf} avatar={false} /> · {queue ? `joins the merge queue (${METHOD_LABELS[a.method].toLowerCase()}) once every requirement is met` : `${METHOD_LABELS[a.method].toLowerCase()} once every requirement is met`} |
| 845 | + {a.deleteBranch ? ( | |
| 846 | + <> | |
| 847 | + {" "} | |
| 848 | + · then deletes <span class="mono">{x.issue.pull?.head}</span> | |
| 849 | + </> | |
| 850 | + ) : null} | |
| 845 | 851 | </span> |
| 846 | 852 | </span> |
| 847 | 853 | </> |
| @@ -866,6 +872,15 @@ | ||
| 866 | 872 | value={queueMethod ?? (linearHistory ? "squash" : "merge")} |
| 867 | 873 | options={(["merge", "squash", "rebase"] as const).filter((v) => !(linearHistory && v === "merge")).map((v) => ({ value: v, label: METHOD_LABELS[v] }))} |
| 868 | 874 | /> |
| 875 | + <KCheckbox | |
| 876 | + name="delete_branch" | |
| 877 | + checked | |
| 878 | + label={ | |
| 879 | + <> | |
| 880 | + Delete <span class="mono">{x.issue.pull?.head}</span> after merge | |
| 881 | + </> | |
| 882 | + } | |
| 883 | + /> | |
| 869 | 884 | <button class="btn">Enable auto-merge</button> |
| 870 | 885 | </Form> |
| 871 | 886 | ); |
| @@ -1410,7 +1425,7 @@ | ||
| 1410 | 1425 | const l = await forAction(c, "write"); |
| 1411 | 1426 | const n = issueNumber(c); |
| 1412 | 1427 | return act(c, `${repoUrl(l.repo)}/pull/${n}`, async () => { |
| 1413 | - const r = await l.stub.enableAutoMerge(n, { method: f.s("method") || undefined }, l.caller); | |
| 1428 | + const r = await l.stub.enableAutoMerge(n, { method: f.s("method") || undefined, deleteBranch: f.b("delete_branch") }, l.caller); | |
| 1414 | 1429 | flash(c, "notice", r.state === "merged" ? "Pull request merged." : r.state === "queued" ? "Added to the merge queue." : "Auto-merge is on: it merges once every requirement is met."); |
| 1415 | 1430 | }); |
| 1416 | 1431 | }); |
modifiedapps/diffinery/test/e2e/security.test.mjs+127−0View file
| @@ -1132,6 +1132,133 @@ | ||
| 1132 | 1132 | await until("merged after the approval", async () => (await pull(after.number)).merged); |
| 1133 | 1133 | }); |
| 1134 | 1134 | |
| 1135 | +test("security: an archived repository lands nothing by itself: archiving ends auto-merge and empties the merge queue", async () => { | |
| 1136 | + const R = `/repos/${OWNER}/shelved`; | |
| 1137 | + const mainSha = await repoWithReadme("shelved"); | |
| 1138 | + await ok("PUT", `${R}/branches/main/protection`, { requiredChecks: ["ci"] }); | |
| 1139 | + await ok("POST", `${R}/git/refs`, { ref: "refs/heads/train", sha: mainSha }); | |
| 1140 | + await ok("PUT", `${R}/branches/train/protection`, { requiredChecks: ["ci"], mergeQueue: { method: "squash" } }); | |
| 1141 | + const pull = (n) => ok("GET", `${R}/pulls/${n}`); | |
| 1142 | + const said = async (n) => (await ok("GET", `${R}/issues/${n}/comments`)).map((c) => c.body); | |
| 1143 | + const opened = async (branch, base) => { | |
| 1144 | + await ok("POST", `${R}/git/refs`, { ref: `refs/heads/${branch}`, sha: mainSha }); | |
| 1145 | + const put = await ok("PUT", `${R}/contents/${branch}.txt`, { message: branch, content: b64(`${branch}\n`), branch }); | |
| 1146 | + return { number: (await ok("POST", `${R}/pulls`, { title: branch, head: branch, base })).number, head: put.commit.sha }; | |
| 1147 | + }; | |
| 1148 | + // One pull request waits for its check with auto-merge on; another waits in the merge queue for its group's check. | |
| 1149 | + const waiting = await opened("waiting", "main"); | |
| 1150 | + assert.equal((await ok("PUT", `${R}/pulls/${waiting.number}/auto-merge`, {})).state, "enabled"); | |
| 1151 | + const queued = await opened("queued", "train"); | |
| 1152 | + await ok("POST", `${R}/check-runs`, { name: "ci", head_sha: queued.head, status: "completed", conclusion: "success" }); | |
| 1153 | + const entry = await status(201, "PUT", `${R}/pulls/${queued.number}/merge-queue`, {}); | |
| 1154 | + assert.ok(entry.entry.merge_group); | |
| 1155 | + | |
| 1156 | + // ------------------------------------------------------------ archiving turns both off, saying why | |
| 1157 | + assert.equal((await ok("PATCH", R, { archived: true })).archived, true); | |
| 1158 | + assert.equal((await pull(waiting.number)).auto_merge, null); | |
| 1159 | + assert.deepEqual((await ok("GET", `${R}/merge-queue/train`)).entries, []); | |
| 1160 | + assert.ok((await said(waiting.number)).includes("Auto-merge was turned off: the repository is archived."), JSON.stringify(await said(waiting.number))); | |
| 1161 | + assert.ok((await said(queued.number)).includes("This pull request was removed from the merge queue: the repository is archived."), JSON.stringify(await said(queued.number))); | |
| 1162 | + await status(403, "PUT", `${R}/pulls/${waiting.number}/auto-merge`, {}); | |
| 1163 | + | |
| 1164 | + // ------------------------------------------------------------ unarchived, nothing comes back by itself | |
| 1165 | + await ok("PATCH", R, { archived: false }); | |
| 1166 | + for (const n of [waiting.number, queued.number]) assert.equal((await pull(n)).merged, false); | |
| 1167 | + assert.equal((await pull(waiting.number)).auto_merge, null); | |
| 1168 | + assert.equal((await ok("PUT", `${R}/pulls/${waiting.number}/auto-merge`, {})).state, "enabled"); | |
| 1169 | + await ok("POST", `${R}/check-runs`, { name: "ci", head_sha: waiting.head, status: "completed", conclusion: "success" }); | |
| 1170 | + await until("auto-merged once unarchived", async () => (await pull(waiting.number)).merged); | |
| 1171 | +}); | |
| 1172 | + | |
| 1173 | +test("security: landing a pull request deletes its head branch only when asked, through auto-merge and the queue too, and never a protected one", async () => { | |
| 1174 | + const R = `/repos/${OWNER}/tidy`; | |
| 1175 | + const mainSha = await repoWithReadme("tidy"); | |
| 1176 | + await ok("PUT", `${R}/branches/main/protection`, { requiredChecks: ["ci"] }); | |
| 1177 | + const pull = (n) => ok("GET", `${R}/pulls/${n}`); | |
| 1178 | + const green = (head) => ok("POST", `${R}/check-runs`, { name: "ci", head_sha: head, status: "completed", conclusion: "success" }); | |
| 1179 | + const exists = async (branch) => (await api("GET", `${R}/branches/${branch}`)).status === 200; | |
| 1180 | + const opened = async (branch, base = "main", from = mainSha) => { | |
| 1181 | + await ok("POST", `${R}/git/refs`, { ref: `refs/heads/${branch}`, sha: from }); | |
| 1182 | + const put = await ok("PUT", `${R}/contents/${branch}.txt`, { message: branch, content: b64(`${branch}\n`), branch }); | |
| 1183 | + return { number: (await ok("POST", `${R}/pulls`, { title: branch, head: branch, base })).number, head: put.commit.sha }; | |
| 1184 | + }; | |
| 1185 | + | |
| 1186 | + // ------------------------------------------------------------ a protected head branch stays, even where the rule allows deleting it | |
| 1187 | + await ok("POST", `${R}/git/refs`, { ref: "refs/heads/release", sha: mainSha }); | |
| 1188 | + const rel = await ok("PUT", `${R}/contents/release.txt`, { message: "release", content: b64("release\n"), branch: "release" }); | |
| 1189 | + await ok("PUT", `${R}/branches/release/protection`, { allowDeletion: true }); | |
| 1190 | + const release = (await ok("POST", `${R}/pulls`, { title: "release", head: "release", base: "main" })).number; | |
| 1191 | + await green(rel.commit.sha); | |
| 1192 | + await ok("PUT", `${R}/pulls/${release}/merge`, { delete_branch: true }); | |
| 1193 | + assert.equal((await pull(release)).merged, true); | |
| 1194 | + assert.equal(await exists("release"), true, "a protected head branch is never deleted by a merge"); | |
| 1195 | + | |
| 1196 | + // ------------------------------------------------------------ auto-merge keeps delete_branch, like a manual merge | |
| 1197 | + const gone = await opened("auto-delete"); | |
| 1198 | + const on = await ok("PUT", `${R}/pulls/${gone.number}/auto-merge`, { delete_branch: true }); | |
| 1199 | + assert.deepEqual([on.state, on.auto_merge.delete_branch, (await pull(gone.number)).auto_merge.delete_branch], ["enabled", true, true]); | |
| 1200 | + await status(422, "PUT", `${R}/pulls/${gone.number}/auto-merge`, { delete_branch: "yes" }); | |
| 1201 | + await status(409, "PUT", `${R}/pulls/${gone.number}/auto-merge`, { sha: mainSha }); | |
| 1202 | + await green(gone.head); | |
| 1203 | + await until("auto-merged", async () => (await pull(gone.number)).merged); | |
| 1204 | + await until("its head branch deleted", async () => !(await exists("auto-delete"))); | |
| 1205 | + const kept = await opened("auto-keep"); | |
| 1206 | + await ok("PUT", `${R}/pulls/${kept.number}/auto-merge`, {}); | |
| 1207 | + await green(kept.head); | |
| 1208 | + await until("auto-merged, keeping the branch", async () => (await pull(kept.number)).merged); | |
| 1209 | + assert.equal(await exists("auto-keep"), true); | |
| 1210 | + | |
| 1211 | + // ------------------------------------------------------------ through the merge queue, by auto-merge | |
| 1212 | + await ok("POST", `${R}/git/refs`, { ref: "refs/heads/train", sha: mainSha }); | |
| 1213 | + await ok("PUT", `${R}/branches/train/protection`, { requiredChecks: ["ci"], mergeQueue: { method: "squash" } }); | |
| 1214 | + const q = await opened("queue-delete", "train"); | |
| 1215 | + await ok("PUT", `${R}/pulls/${q.number}/auto-merge`, { delete_branch: true }); | |
| 1216 | + await green(q.head); | |
| 1217 | + const entry = await until("queued by auto-merge", async () => (await ok("GET", `${R}/merge-queue/train`)).entries.find((e) => e.number === q.number && e.merge_group)); | |
| 1218 | + assert.equal(entry.delete_branch, true); | |
| 1219 | + await green(entry.merge_group.head_sha); | |
| 1220 | + await until("merged through the queue", async () => (await pull(q.number)).merged); | |
| 1221 | + await until("its head branch deleted", async () => !(await exists("queue-delete"))); | |
| 1222 | +}); | |
| 1223 | + | |
| 1224 | +test("security: agents see what auto-merge needs next to the refusal to merge directly", async () => { | |
| 1225 | + const R = `/repos/${OWNER}/insight`; | |
| 1226 | + const mainSha = await repoWithReadme("insight"); | |
| 1227 | + await status(204, "PUT", `${R}/collaborators/olive-bot`, { permission: "write" }); | |
| 1228 | + await ok("PUT", `${R}/branches/main/protection`, { requirePullRequest: true, requiredChecks: ["ci"], allowAgentAutoMerge: true }); | |
| 1229 | + await ok("POST", `${R}/git/refs`, { ref: "refs/heads/agent-insight", sha: mainSha }, T.bot); | |
| 1230 | + const put = await ok("PUT", `${R}/contents/insight.txt`, { message: "insight", content: b64("insight\n"), branch: "agent-insight" }, T.bot); | |
| 1231 | + const pr = (await ok("POST", `${R}/pulls`, { title: "insight", head: "agent-insight", base: "main" }, T.bot)).number; | |
| 1232 | + const viaMcp = async (as = T.bot) => (await mcpTool(as, "get_pull_request", { owner: OWNER, repo: "insight", number: pr })).data; | |
| 1233 | + | |
| 1234 | + // ------------------------------------------------------------ the agent may not merge, but auto-merge only waits for CI and a person | |
| 1235 | + let m = await viaMcp(); | |
| 1236 | + assert.ok(m.mergeability.blockers.some((b) => /agents may not merge/.test(b)), JSON.stringify(m.mergeability.blockers)); | |
| 1237 | + assert.ok(Array.isArray(m.auto_merge_blockers), JSON.stringify(Object.keys(m))); | |
| 1238 | + assert.ok(!m.auto_merge_blockers.some((b) => /agents may not merge/.test(b)), JSON.stringify(m.auto_merge_blockers)); | |
| 1239 | + assert.ok(m.auto_merge_blockers.some((b) => /required check 'ci' has not run/.test(b)), JSON.stringify(m.auto_merge_blockers)); | |
| 1240 | + assert.ok(m.auto_merge_blockers.some((b) => /needs 1 more approving review/.test(b)), JSON.stringify(m.auto_merge_blockers)); | |
| 1241 | + await ok("POST", `${R}/check-runs`, { name: "ci", head_sha: put.commit.sha, status: "completed", conclusion: "success" }); | |
| 1242 | + await ok("POST", `${R}/pulls/${pr}/reviews`, { event: "APPROVE" }); | |
| 1243 | + m = await viaMcp(); | |
| 1244 | + assert.deepEqual(m.auto_merge_blockers, []); | |
| 1245 | + assert.ok(m.mergeability.blockers.some((b) => /agents may not merge/.test(b)), "the refusal to merge directly still shows"); | |
| 1246 | + // The REST API says the same. | |
| 1247 | + const rest = await ok("GET", `${R}/pulls/${pr}`, undefined, T.bot); | |
| 1248 | + assert.deepEqual(rest.auto_merge_blockers, []); | |
| 1249 | + assert.ok(rest.merge_blockers.some((b) => /agents may not merge/.test(b))); | |
| 1250 | + // People merge and auto-merge on the same terms, so they get no second list. | |
| 1251 | + assert.equal((await ok("GET", `${R}/pulls/${pr}`)).auto_merge_blockers, undefined); | |
| 1252 | + assert.equal((await viaMcp(T.olive)).auto_merge_blockers, undefined); | |
| 1253 | + // Without the setting, auto-merge's own refusal shows there. | |
| 1254 | + await ok("PUT", `${R}/branches/main/protection`, { requirePullRequest: true, requiredChecks: ["ci"] }); | |
| 1255 | + assert.ok((await viaMcp()).auto_merge_blockers.some((b) => /allowAgentAutoMerge/.test(b))); | |
| 1256 | + assert.equal((await pull(pr)).merged, false); | |
| 1257 | + async function pull(n) { | |
| 1258 | + return ok("GET", `${R}/pulls/${n}`); | |
| 1259 | + } | |
| 1260 | +}); | |
| 1261 | + | |
| 1135 | 1262 | test("security: the queue's bypass button does what it says, and a merge queue can't contradict linear history", async () => { |
| 1136 | 1263 | const R = `/repos/${OWNER}/bypass`; |
| 1137 | 1264 | const mainSha = await repoWithReadme("bypass"); |
modifiedapps/diffinery/test/unit/auto-merge.test.ts+135−1View file
| @@ -1,5 +1,10 @@ | ||
| 1 | +import { DatabaseSync } from "node:sqlite"; | |
| 1 | 2 | import { describe, expect, it } from "vitest"; |
| 2 | -import { automaticLandingRefusal, isPerson, landsByAgentRule } from "../../src/repo/auto-merge.js"; | |
| 3 | +import type { Env } from "../../src/env.js"; | |
| 4 | +import { HttpError } from "../../src/lib/util.js"; | |
| 5 | +import { AUTO_MERGE_BOT, AutoMerge, automaticLandingRefusal, isPerson, landsByAgentRule } from "../../src/repo/auto-merge.js"; | |
| 6 | +import type { RepoDO } from "../../src/repo/RepoDO.js"; | |
| 7 | +import { REPO_SCHEMA, migrateRepoSchema } from "../../src/repo/schema.js"; | |
| 3 | 8 | import type { ProtectionRules } from "../../src/repo/types.js"; |
| 4 | 9 | |
| 5 | 10 | const person = { id: "u1", kind: "user" as const }; |
| @@ -51,3 +56,132 @@ | ||
| 51 | 56 | expect(landsByAgentRule(agent, null, false)).toBe(false); |
| 52 | 57 | }); |
| 53 | 58 | }); |
| 59 | + | |
| 60 | +// ---------------------------------------------------------------- what auto-merge does when it tries | |
| 61 | + | |
| 62 | +/** A minimal SqlStorage over node:sqlite with the repository schema. */ | |
| 63 | +function sqlStorage(): SqlStorage { | |
| 64 | + const db = new DatabaseSync(":memory:"); | |
| 65 | + db.exec(REPO_SCHEMA); | |
| 66 | + const sql = { | |
| 67 | + exec(query: string, ...bindings: unknown[]) { | |
| 68 | + const stmt = db.prepare(query); | |
| 69 | + if (/^\s*select/i.test(query)) { | |
| 70 | + const rows = stmt.all(...(bindings as never[])); | |
| 71 | + return { toArray: () => rows, one: () => rows[0] } as unknown as SqlStorageCursor<Record<string, SqlStorageValue>>; | |
| 72 | + } | |
| 73 | + stmt.run(...(bindings as never[])); | |
| 74 | + return { toArray: () => [], one: () => undefined } as unknown as SqlStorageCursor<Record<string, SqlStorageValue>>; | |
| 75 | + }, | |
| 76 | + } as unknown as SqlStorage; | |
| 77 | + migrateRepoSchema(sql); | |
| 78 | + return sql; | |
| 79 | +} | |
| 80 | + | |
| 81 | +/** | |
| 82 | + * Auto-merge over a fake repository: pull request #1 from `topic` into `base`, ready to merge, with | |
| 83 | + * auto-merge turned on by a workflow token (whose access needs no directory lookup). | |
| 84 | + */ | |
| 85 | +function harness(o: { rules?: ProtectionRules | null; archived?: boolean; deleteBranch?: boolean; enqueue?: () => Promise<unknown> } = {}) { | |
| 86 | + const sql = sqlStorage(); | |
| 87 | + const at = new Date().toISOString(); | |
| 88 | + sql.exec("INSERT INTO issues (number, kind, title, author_id, author_login, created_at, updated_at) VALUES (1, 'pull', 'T', 'u1', 'olive', ?, ?)", at, at); | |
| 89 | + const state = { method: "squash", commitTitle: null, commitMessage: null, enabledBy: { id: "job:j1", login: "github-actions[bot]", kind: "system" }, enabledAt: at, role: "write", permissions: { contents: "write" }, ...(o.deleteBranch ? { deleteBranch: true } : {}) }; | |
| 90 | + sql.exec("INSERT INTO pulls (number, head_ref, head_sha, base_ref, auto_merge) VALUES (1, 'topic', 'h1', 'base', ?)", JSON.stringify(state)); | |
| 91 | + const comments: Array<{ body: string; by: string }> = []; | |
| 92 | + const events: Array<{ type: string; action?: string; detail?: Record<string, unknown> }> = []; | |
| 93 | + const merges: Array<Record<string, unknown>> = []; | |
| 94 | + const enqueues: Array<Record<string, unknown>> = []; | |
| 95 | + const repo = { | |
| 96 | + sql, | |
| 97 | + meta: { repoId: "r1", ownerId: "u1", owner: "olive", name: "n", defaultBranch: "main", ownerKind: "user", ...(o.archived !== undefined ? { archived: o.archived } : {}) }, | |
| 98 | + isArchived: async () => !!o.archived, | |
| 99 | + protectionFor: () => o.rules ?? null, | |
| 100 | + collab: { | |
| 101 | + get: () => ({ issue: { number: 1, state: "open", pull: { merged: false, draft: false, base: "base", head: "topic", headSha: "h1" } } }), | |
| 102 | + mergeability: async () => ({ mergeable: true, blockers: [] }), | |
| 103 | + merge: async (_n: number, input: Record<string, unknown>) => { | |
| 104 | + merges.push(input); | |
| 105 | + sql.exec("UPDATE pulls SET merged = 1, auto_merge = NULL WHERE number = 1"); | |
| 106 | + return { merged: true, sha: "m1", method: "squash" }; | |
| 107 | + }, | |
| 108 | + addComment: (_n: number, body: string, by: { login: string }) => comments.push({ body, by: by.login }), | |
| 109 | + changedWorkflow: async () => null, | |
| 110 | + }, | |
| 111 | + queue: { | |
| 112 | + isQueued: () => false, | |
| 113 | + enqueue: async (_n: number, input: Record<string, unknown>) => { | |
| 114 | + enqueues.push(input); | |
| 115 | + return o.enqueue ? o.enqueue() : { state: "queued", entry: null }; | |
| 116 | + }, | |
| 117 | + }, | |
| 118 | + social: { record: () => undefined }, | |
| 119 | + emit: (e: { type: string; action?: string; detail?: Record<string, unknown> }) => events.push(e), | |
| 120 | + broadcast: () => undefined, | |
| 121 | + background: () => undefined, | |
| 122 | + }; | |
| 123 | + const am = new AutoMerge(repo as unknown as RepoDO, {} as Env); | |
| 124 | + return { am, comments, events, merges, enqueues, on: () => am.state(1) !== null }; | |
| 125 | +} | |
| 126 | + | |
| 127 | +const QUEUED: ProtectionRules = { requiredChecks: ["ci"], allowAgentAutoMerge: true, mergeQueue: { method: "squash" } }; | |
| 128 | + | |
| 129 | +describe("auto-merge at work", () => { | |
| 130 | + it("merges a ready pull request", async () => { | |
| 131 | + const h = harness(); | |
| 132 | + await h.am.process({ number: 1 }); | |
| 133 | + expect(h.merges).toHaveLength(1); | |
| 134 | + expect(h.on()).toBe(false); | |
| 135 | + }); | |
| 136 | + | |
| 137 | + it("never merges, or queues, in an archived repository: it turns itself off and says why", async () => { | |
| 138 | + for (const rules of [null, QUEUED]) { | |
| 139 | + const h = harness({ archived: true, rules }); | |
| 140 | + await h.am.process({ number: 1 }); | |
| 141 | + expect(h.merges).toHaveLength(0); | |
| 142 | + expect(h.enqueues).toHaveLength(0); | |
| 143 | + expect(h.on()).toBe(false); | |
| 144 | + expect(h.comments).toEqual([{ body: "Auto-merge was turned off: the repository is archived.", by: AUTO_MERGE_BOT.login }]); | |
| 145 | + } | |
| 146 | + }); | |
| 147 | + | |
| 148 | + it("can't be turned on in an archived repository", async () => { | |
| 149 | + const h = harness({ archived: true }); | |
| 150 | + h.am.clear(1); | |
| 151 | + await expect(h.am.enable(1, {}, { id: "u1", login: "olive", kind: "user", role: "admin" })).rejects.toMatchObject({ status: 403 }); | |
| 152 | + expect(h.on()).toBe(false); | |
| 153 | + }); | |
| 154 | + | |
| 155 | + it("turns itself off, with a comment, when the merge queue refuses the pull request", async () => { | |
| 156 | + const h = harness({ rules: QUEUED, enqueue: () => Promise.reject(new HttpError(405, "pull request cannot be added to the merge queue: required check 'ci' failed")) }); | |
| 157 | + await h.am.process({ number: 1 }); | |
| 158 | + expect(h.enqueues).toHaveLength(1); | |
| 159 | + expect(h.on()).toBe(false); | |
| 160 | + expect(h.comments).toEqual([{ body: "Auto-merge was turned off: pull request cannot be added to the merge queue: required check 'ci' failed.", by: AUTO_MERGE_BOT.login }]); | |
| 161 | + expect(h.events.find((e) => e.action === "pull_request.auto_merge_disabled")?.detail?.reason).toMatch(/cannot be added to the merge queue/); | |
| 162 | + // Anything else the queue throws turns it off too, rather than retrying on every event in silence. | |
| 163 | + const odd = harness({ rules: QUEUED, enqueue: () => Promise.reject(new Error("boom")) }); | |
| 164 | + await odd.am.process({ number: 1 }); | |
| 165 | + expect(odd.on()).toBe(false); | |
| 166 | + expect(odd.comments[0]?.body).toBe("Auto-merge was turned off: the pull request could not be added to the merge queue."); | |
| 167 | + }); | |
| 168 | + | |
| 169 | + it("leaves auto-merge on when the head moved under it (409): the next event looks again", async () => { | |
| 170 | + const h = harness({ rules: QUEUED, enqueue: () => Promise.reject(new HttpError(409, "head branch was modified")) }); | |
| 171 | + await h.am.process({ number: 1 }); | |
| 172 | + expect(h.on()).toBe(true); | |
| 173 | + expect(h.comments).toHaveLength(0); | |
| 174 | + }); | |
| 175 | + | |
| 176 | + it("deletes the head branch after it lands when asked to, directly or through the queue", async () => { | |
| 177 | + const direct = harness({ deleteBranch: true }); | |
| 178 | + await direct.am.process({ number: 1 }); | |
| 179 | + expect(direct.merges[0]).toMatchObject({ deleteBranch: true }); | |
| 180 | + const queued = harness({ deleteBranch: true, rules: QUEUED }); | |
| 181 | + await queued.am.process({ number: 1 }); | |
| 182 | + expect(queued.enqueues[0]).toMatchObject({ deleteBranch: true }); | |
| 183 | + const kept = harness({ rules: QUEUED }); | |
| 184 | + await kept.am.process({ number: 1 }); | |
| 185 | + expect(kept.enqueues[0]).toMatchObject({ deleteBranch: false }); | |
| 186 | + }); | |
| 187 | +}); |