Merge pull request #268 from claude/auto-merge-agent-rule
Branch rules can auto-merge chosen agents' pull requests by themselves
cnbrkdmrci authored 2 parents 0d85d7d139e858commit 5d1cb0c7b77b4b8475102b41ee15d494cf106bc6Browse files
- CI / Typecheck and bundleFailed
- CI / Unit tests (1/2)Failed
- CI / Unit tests (2/2)Failed
- End-to-end / End-to-end (agent reviews, tasks, Today)Failed
- End-to-end / End-to-end (git, PRs, Actions)Failed
- End-to-end / End-to-end (large pushes)Failed
- End-to-end / End-to-end (passkeys, registry, email)Failed
- End-to-end / End-to-end (real histories)Failed
- End-to-end / End-to-end (runner Docker)Failed
- End-to-end / End-to-end (security)Failed
14 files changed+470−37
Files (14)
- apps/diffinery/docs/agents/overview.md+2−2
- apps/diffinery/docs/collaboration/pull-requests.md+4−3
- apps/diffinery/docs/repositories/branch-protection.md+43−0
- apps/diffinery/docs/security/audit-log.md+3−2
- apps/diffinery/src/docs/api-meta.ts+1−1
- apps/diffinery/src/http/api.ts+6−4
- apps/diffinery/src/repo/RepoDO.ts+58−9
- apps/diffinery/src/repo/auto-merge.ts+61−10
- apps/diffinery/src/repo/collab.ts+8−1
- apps/diffinery/src/repo/types.ts+15−0
- apps/diffinery/src/web/collab.tsx+11−1
- apps/diffinery/src/web/repo-settings.tsx+38−4
- apps/diffinery/test/e2e/security.test.mjs+126−0
- apps/diffinery/test/unit/auto-merge.test.ts+94−0
modifiedapps/diffinery/docs/agents/overview.md+2−2View file
| @@ -123,7 +123,7 @@ | ||
| 123 | 123 | | --- | --- | --- | |
| 124 | 124 | | No pushes to the default branch or protected branches | Pushes, `commit_files`, the contents API, the git data refs API, the merges API, update-branch and branch updates are refused with `agents may not push to protected or default branches; open a pull request`. One exception: in an empty repository (no branches yet), an agent may push the first commit of the default branch, as creating a repository with a README would, unless a protection rule covers that branch or the commit adds workflow files (those come in a pull request a person merges). | Tick **Allow agents to push directly** (`agentsMayPush`) on that branch's protection rule. | |
| 125 | 125 | | No merges into the default branch or protected branches | The merge blocker `agents may not merge into this branch; a human must merge`. Agents can't add pull requests to a merge queue or turn on auto-merge there either. | An agent from **Settings → Agent setup** may merge what a person approved, with **Can merge pull requests** (on by default; see [Merging](/docs/connect/cloud-agents#merging)). Or tick **Allow agents to merge pull requests** (`agentsMayMerge`) on the rule, or, so that CI decides instead of the agent, **Allow agents to land pull requests through auto-merge** (`allowAgentAutoMerge`, see [Agents landing changes](/docs/repositories/branch-protection#agents-landing-changes)); the agent then turns on [auto-merge](/docs/collaboration/pull-requests#auto-merge-for-agents-and-workflow-tokens) with `enable_auto_merge` or `PUT /repos/{owner}/{repo}/pulls/{number}/auto-merge`. | |
| 126 | -| Pull requests opened by agents need a human approval | At least one approval from a person, even on a branch with no protection rule. When an agent has Diffinery land its pull request on the default branch or a protected branch through auto-merge or the merge queue, the approval counts only for the commit it approved. | Only for landings an agent asked for: **Let agents' auto-merges land without an approving review** (`agentAutoMergeWithoutApproval`), which lets green required checks stand in for the approval. See [Waiving approvals for agents](/docs/repositories/branch-protection#waiving-approvals-for-agents). | | |
| 126 | +| Pull requests opened by agents need a human approval | At least one approval from a person, even on a branch with no protection rule. When an agent has Diffinery land its pull request on the default branch or a protected branch through auto-merge or the merge queue, the approval counts only for the commit it approved. | Only for landings an agent asked for: **Let agents' auto-merges land without an approving review** (`agentAutoMergeWithoutApproval`), which lets green required checks stand in for the approval. See [Waiving approvals for agents](/docs/repositories/branch-protection#waiving-approvals-for-agents). To have Diffinery turn auto-merge on for chosen agents' pull requests by itself, name them in **Auto-merge pull requests from these agents** (`autoMergeAgents`, see [Auto-merging agents' pull requests](/docs/repositories/branch-protection#auto-merging-agents-pull-requests)). | | |
| 127 | 127 | | Agent approvals don't count | An agent's Approve review is shown but never counts toward required approvals. An agent's Request changes review still blocks. | None. | |
| 128 | 128 | | No branch protection changes | `agents cannot change branch protection` | None. | |
| 129 | 129 | | No secret management | Setting or deleting repository or environment secrets fails with `agents cannot manage secrets`, and agents can't use the organization secrets endpoints, whatever their role. | None. | |
| @@ -283,7 +283,7 @@ | ||
| 283 | 283 | ## Differences from GitHub |
| 284 | 284 | |
| 285 | 285 | - Agents are accounts owned by a person or an organization, rather than GitHub Apps with installations, and authenticate with ordinary `dfy_` tokens. |
| 286 | -- Agents are held to built-in guardrails (no pushes or merges to default and protected branches, human approval for their pull requests, agent approvals not counted) that you relax per branch with `agentsMayPush`, `agentsMayMerge`, `allowAgentAutoMerge` and `agentAutoMergeWithoutApproval`. The guardrails also hold for workflow runs of code an agent wrote, so an agent can't use a workflow's `GITHUB_TOKEN` to get around them. | |
| 286 | +- Agents are held to built-in guardrails (no pushes or merges to default and protected branches, human approval for their pull requests, agent approvals not counted) that you relax per branch with `agentsMayPush`, `agentsMayMerge`, `allowAgentAutoMerge` and `agentAutoMergeWithoutApproval`; `autoMergeAgents` has Diffinery turn auto-merge on for chosen agents by itself. The guardrails also hold for workflow runs of code an agent wrote, so an agent can't use a workflow's `GITHUB_TOKEN` to get around them. | |
| 287 | 287 | - Commit statuses and check runs reported by agents never satisfy required checks, and agents can't manage environments, secrets or Actions variables, or dismiss reviews. |
| 288 | 288 | - Agents can run GitHub imports, within the import rules above. |
| 289 | 289 | - Commits Diffinery writes for agents carry a `diffinery-agent ... on-behalf-of ...` header. |
modifiedapps/diffinery/docs/collaboration/pull-requests.md+4−3View file
| @@ -739,12 +739,13 @@ | ||
| 739 | 739 | - A pull request that adds, changes or deletes a workflow file can't land this way; a person must merge it. |
| 740 | 740 | - A workflow's `GITHUB_TOKEN` is held to the same rule, even when the run's code didn't come from an agent. |
| 741 | 741 | - On branches that are neither the default branch nor protected, agents and workflow tokens can use auto-merge without any rule. |
| 742 | +- An admin can also name agents in **Auto-merge pull requests from these agents** (`autoMergeAgents`) on the rule. Diffinery then turns auto-merge on by itself, as the agent, for each pull request those agents open, mark ready or push to, so neither the agent nor a person has to. Every point above still applies. See [Auto-merging agents' pull requests](/docs/repositories/branch-protection#auto-merging-agents-pull-requests). | |
| 742 | 743 | |
| 743 | 744 | A typical agent flow over MCP: |
| 744 | 745 | |
| 745 | 746 | 1. `commit_files` with `create_branch_from` to put the change on a new branch. |
| 746 | 747 | 2. `create_pull_request` with an `intent`. |
| 747 | -3. `enable_auto_merge` with `merge_method: "squash"`. If the answer is `isError: true` with `agents may not have pull requests merged into main automatically ...`, the branch doesn't allow it: ask a person to merge, or an admin to set `allowAgentAutoMerge`. | |
| 748 | +3. `enable_auto_merge` with `merge_method: "squash"` (not needed where the rule names the agent in `autoMergeAgents`: `get_pull_request` then already shows `issue.pull.autoMerge` with `byRule`). If the answer is `isError: true` with `agents may not have pull requests merged into main automatically ...`, the branch doesn't allow it: ask a person to merge, or an admin to set `allowAgentAutoMerge`. | |
| 748 | 749 | 4. Follow CI with `list_workflow_runs`, and fix failures with more commits. The agent's own pushes keep auto-merge on. |
| 749 | 750 | 5. `get_pull_request` shows `issue.pull.autoMerge` while it waits, and `issue.pull.merged` once Diffinery merged it. |
| 750 | 751 | |
| @@ -752,14 +753,14 @@ | ||
| 752 | 753 | |
| 753 | 754 | ### Auto-merge events and audit log |
| 754 | 755 | |
| 755 | -- **Timeline:** turning auto-merge on or off adds an `auto_merge_enabled` or `auto_merge_disabled` event to the pull request (GitHub's names), returned by `GET /repos/{owner}/{repo}/issues/{number}/events` and the timeline. | |
| 756 | +- **Timeline:** turning auto-merge on or off adds an `auto_merge_enabled` or `auto_merge_disabled` event to the pull request (GitHub's names), returned by `GET /repos/{owner}/{repo}/issues/{number}/events` and the timeline. When a branch rule's `autoMergeAgents` turned it on, the event's actor is `diffinery-auto-merge[bot]`. | |
| 756 | 757 | - **Webhooks:** a `pull_request` webhook with the action `auto_merge_enabled` or `auto_merge_disabled` is sent. The sender is who turned it on or off; when auto-merge turned itself off, it is the pusher, the person who converted the pull request to a draft, or `diffinery-auto-merge[bot]`. See [Webhook events](/docs/reference/webhook-events#pull_request). |
| 757 | 758 | - **Workflows and email:** these actions don't start `pull_request` workflows and send no notification emails. The merge itself does everything a merge by hand does (see [After the merge](#after-the-merge)). |
| 758 | 759 | - **Audit log:** each step is recorded with the pull request (`acme/website#12`) as the target: |
| 759 | 760 | |
| 760 | 761 | | Action | When | Detail | |
| 761 | 762 | | --- | --- | --- | |
| 762 | -| `pull_request.auto_merge_enabled` | Auto-merge is turned on (or changed) | `base`, `method`, `viaAgentRule` (`true` when it relies on `allowAgentAutoMerge`) | | |
| 763 | +| `pull_request.auto_merge_enabled` | Auto-merge is turned on (or changed) | `base`, `method`, `viaAgentRule` (`true` when it relies on `allowAgentAutoMerge`); with `byRule` (the branch rule's pattern) and `agent` when a rule's `autoMergeAgents` turned it on, by `diffinery-auto-merge[bot]` | | |
| 763 | 764 | | `pull_request.auto_merge_disabled` | Auto-merge is turned off, by someone or by itself | `enabledBy`, and `reason` when it turned itself off | |
| 764 | 765 | | `pull_request.auto_merged` | Auto-merge merged the pull request; a `pull.merge` entry is written too | `base`, `method`, `sha` | |
| 765 | 766 | | `pull_request.auto_merge_enqueued` | Auto-merge added the pull request to the merge queue | `base`, `state` | |
modifiedapps/diffinery/docs/repositories/branch-protection.md+43−0View file
| @@ -67,6 +67,8 @@ | ||
| 67 | 67 | | `agentsMayMerge` | Allow agents to merge pull requests | boolean | `false` | Lets agents merge pull requests into matching branches. | |
| 68 | 68 | | `allowAgentAutoMerge` | Allow agents to land pull requests through auto-merge | boolean | `false` | Lets agents (and workflow tokens) turn on auto-merge or add pull requests to the merge queue for matching branches, so Diffinery merges once every rule is met. Needs at least one required check. See [Agents landing changes](#agents-landing-changes). | |
| 69 | 69 | | `agentAutoMergeWithoutApproval` | Let agents' auto-merges land without an approving review | boolean | `false` | With `allowAgentAutoMerge`: what Diffinery lands for agents needs no approval from a person, only green required checks. Requested changes still block it, and a workflow change still needs a person. See [Waiving approvals for agents](#waiving-approvals-for-agents). | |
| 70 | +| `autoMergeAgents` | Auto-merge pull requests from these agents | array of agent logins | none | With `allowAgentAutoMerge`: Diffinery turns auto-merge on by itself for pull requests these agents open into matching branches, so nobody has to click Merge or turn auto-merge on. Stored as `{ "id", "login" }` objects. See [Auto-merging agents' pull requests](#auto-merging-agents-pull-requests). | | |
| 71 | +| `autoMergeMethod` | Their merge method | `merge`, `squash` or `rebase` | as the branch merges | How what `autoMergeAgents` turns on merges. Without it: the merge queue's method, else squash with `requireLinearHistory`, else a merge commit. | | |
| 70 | 72 | | `mergeQueue` | Require a merge queue | object | none | Pull requests merge through a merge queue. See [Merge queue settings](#merge-queue-settings). | |
| 71 | 73 | |
| 72 | 74 | ### Required checks |
| @@ -162,6 +164,47 @@ | ||
| 162 | 164 | - It is set per branch rule, only by people with the admin role, and never comes in with an import. Turning `allowAgentAutoMerge` off turns this off too. Every change is audited as `repo.protection.agent_approval_waiver`. |
| 163 | 165 | - What lands this way is the agent's push, so later runs on the branch are runs of agent code (see [Workflow permissions](/docs/actions/workflow-permissions#runs-whose-code-came-from-an-agent)). They get no repository or organization secrets unless an admin turns on **Give runs of agent code the repository's and organization's secrets**, and they do get the secrets of environments limited to protected branches. A deploy that runs after an agent's merge needs one of the two; see [Which runs receive secrets](/docs/actions/secrets-and-variables#which-runs-receive-secrets). |
| 164 | 166 | |
| 167 | +#### Auto-merging agents' pull requests | |
| 168 | + | |
| 169 | +`allowAgentAutoMerge` still needs someone to turn auto-merge on for each pull request. Where a repository's agents should land their work with nobody acting at all, an admin names those agents on the branch's rule, in **Auto-merge pull requests from these agents** (`autoMergeAgents`). Diffinery then turns auto-merge on by itself, as the agent, when one of them: | |
| 170 | + | |
| 171 | +- opens a pull request into a matching branch (not a draft), | |
| 172 | +- marks its draft pull request ready for review, or | |
| 173 | +- pushes to a pull request it opened, which is how a pull request opened before the agent was named gets picked up. | |
| 174 | + | |
| 175 | +Each such pull request then lands exactly as the agent's own auto-merge would, with nothing loosened: | |
| 176 | + | |
| 177 | +- The rule needs `allowAgentAutoMerge` and at least one required check (`422` otherwise), and the required checks must pass on the head commit. | |
| 178 | +- An approval from a person is still needed unless the rule also has `agentAutoMergeWithoutApproval`. With it, green required checks are enough: that pairing is what lands agents' work with no human step. | |
| 179 | +- A person's requested changes still block it. | |
| 180 | +- A pull request that adds, changes or deletes a workflow file never turns on this way. Diffinery says so in a comment (``Auto-merge didn't turn on by itself (branch rule `main` auto-merges my-agent's pull requests): the pull request changes workflow file ...``), and a person merges it. | |
| 181 | +- The agent's write access is checked again when the merge happens, and the merge is made as the agent, on behalf of its owner. | |
| 182 | +- Once auto-merge on a pull request is turned off, by anyone or by Diffinery (for example because a person pushed to the branch), the rule never turns it back on for that pull request. A person can still turn it on, or merge. | |
| 183 | +- Pull requests from other agents, and from people, are left alone. | |
| 184 | + | |
| 185 | +To turn it on for `main`: | |
| 186 | + | |
| 187 | +1. Open the repository's **Settings**, then **Branch protection** in the sidebar (`/{owner}/{repo}/settings/branches`) and expand the rule for `main` (or add one). | |
| 188 | +2. Make sure **Required status checks** lists your CI checks, and tick **Allow agents to land pull requests through auto-merge**. | |
| 189 | +3. To land without an approving review, also tick **Let agents' auto-merges land without an approving review**. | |
| 190 | +4. In **Auto-merge pull requests from these agents**, enter the agent logins, one per line, and pick **Their merge method** if it should differ from how the branch merges. | |
| 191 | +5. Click **Save rule**. | |
| 192 | + | |
| 193 | +With the API, the same rule is: | |
| 194 | + | |
| 195 | +```bash | |
| 196 | +curl -X PUT https://diffinery.com/api/v3/repos/acme/website/branches/main/protection \ | |
| 197 | + -H "Authorization: Bearer $DIFFINERY_TOKEN" \ | |
| 198 | + -d '{"requirePullRequest": true, "requiredChecks": ["ci"], "allowAgentAutoMerge": true, | |
| 199 | + "agentAutoMergeWithoutApproval": true, "autoMergeAgents": ["my-agent"], "autoMergeMethod": "squash"}' | |
| 200 | +``` | |
| 201 | + | |
| 202 | +The answer shows the agents as stored, `"autoMergeAgents": [{ "id": "...", "login": "my-agent" }]`, and that form can be sent back as is. Only accounts that are agents can be named (`422`: `autoMergeAgents: olive is not an agent; ...`), at most 20 per rule. | |
| 203 | + | |
| 204 | +- Only people with the admin role change it, never agents or workflow tokens, and it never comes in with an import. Turning `allowAgentAutoMerge` off removes it. | |
| 205 | +- Every change to the agents or the method is audited as `repo.protection.agent_auto_merge_rule` (`pattern`, `enabled`, `agents`, `method`, `withoutApproval`, `requiredChecks`). Each pull request it turns auto-merge on for is audited as `pull_request.auto_merge_enabled` by `diffinery-auto-merge[bot]`, with `byRule` (the rule's pattern) and `agent`, and the merge as `pull_request.auto_merged` by the agent. | |
| 206 | +- The pull request page says **Auto-merge is on · turned on by the branch rule `main` for my-agent**, with the usual **Disable auto-merge** button. | |
| 207 | + | |
| 165 | 208 | ### Workflow tokens |
| 166 | 209 | |
| 167 | 210 | A workflow job's `GITHUB_TOKEN` is held to branch protection like a person with the write role: |
modifiedapps/diffinery/docs/security/audit-log.md+3−2View file
| @@ -168,6 +168,7 @@ | ||
| 168 | 168 | | `repo.protection.set`, `repo.protection.delete` | A branch protection rule is saved or deleted on **Branch protection**, or with `PUT`/`DELETE /api/v3/repos/{owner}/{repo}/branches/{branch}/protection` | `pattern`, and on the web `rules` when saved | |
| 169 | 169 | | `repo.protection.agent_auto_merge` | **Allow agents to land pull requests through auto-merge** (`allowAgentAutoMerge`) is turned on or off for a rule, from the web UI or the API | `pattern`, `enabled`, `requiredChecks` | |
| 170 | 170 | | `repo.protection.agent_approval_waiver` | **Let agents' auto-merges land without an approving review** (`agentAutoMergeWithoutApproval`) is turned on or off for a rule, from the web UI or the API | `pattern`, `enabled`, `requiredChecks` | |
| 171 | +| `repo.protection.agent_auto_merge_rule` | **Auto-merge pull requests from these agents** (`autoMergeAgents`) or their merge method changes for a rule, from the web UI or the API (see [Auto-merging agents' pull requests](/docs/repositories/branch-protection#auto-merging-agents-pull-requests)) | `pattern`, `enabled`, `agents`, `method`, `withoutApproval`, `requiredChecks` | | |
| 171 | 172 | | `repo.secret.set`, `repo.secret.delete` | An Actions secret is saved or deleted on **Secrets and variables**, or with `PUT`/`DELETE /api/v3/repos/{owner}/{repo}/actions/secrets/{name}` | `name` (never the value) | |
| 172 | 173 | | `repo.variable.set`, `repo.variable.delete` | An Actions variable is saved or deleted on **Secrets and variables**, or created, updated, renamed or deleted through `/api/v3/repos/{owner}/{repo}/actions/variables` | `name`, and `renamed_from` when a `PATCH` renamed it | |
| 173 | 174 | | `repo.webhook.create`, `repo.webhook.delete` | A webhook is added or deleted on **Webhooks**, or with `POST /api/v3/repos/{owner}/{repo}/hooks` or `DELETE /api/v3/repos/{owner}/{repo}/hooks/{id}` | Web: `url` and `events`, or `id`. API: `id` and `url`, or `id` | |
| @@ -201,7 +202,7 @@ | ||
| 201 | 202 | | `git.delete_ref` | A branch or tag is deleted | as above | `before`, `after` | |
| 202 | 203 | | `pull.merge` | A pull request is merged, directly, by the merge queue or by auto-merge | `acme/website#42` | none | |
| 203 | 204 | | `pull_request.merge_bypassed` | A repository admin merges a pull request past branch rules by asking for a bypass: with `"bypass_rules": true` (or **Merge without waiting for requirements (bypass)** in the web UI) past unmet blockers or a linear-history rule (a merge commit), or with `"bypass_merge_queue": true` (or **Merge now (bypass the queue)**) past a required merge queue. `diffinery pr merge --admin` sends both. A `pull.merge` entry follows. | `acme/website#42` | `blockers` (what was bypassed: the unmet blockers, such as `needs 1 more approving review(s)`, plus `linear history required` or `merge queue` when those were bypassed), `base` (the base branch) | |
| 204 | -| `pull_request.auto_merge_enabled` | Someone turns on [auto-merge](/docs/collaboration/pull-requests#auto-merge) for a pull request, or changes its method (web UI, `PUT /api/v3/repos/{owner}/{repo}/pulls/{number}/auto-merge` or the `enable_auto_merge` MCP tool) | `acme/website#42` | `base`, `method`, `viaAgentRule` (`true` when an agent or workflow token relies on `allowAgentAutoMerge`) | | |
| 205 | +| `pull_request.auto_merge_enabled` | Someone turns on [auto-merge](/docs/collaboration/pull-requests#auto-merge) for a pull request, or changes its method (web UI, `PUT /api/v3/repos/{owner}/{repo}/pulls/{number}/auto-merge` or the `enable_auto_merge` MCP tool) | `acme/website#42` | `base`, `method`, `viaAgentRule` (`true` when an agent or workflow token relies on `allowAgentAutoMerge`); `byRule` and `agent` when a branch rule's `autoMergeAgents` turned it on, recorded as `diffinery-auto-merge[bot]` | | |
| 205 | 206 | | `pull_request.auto_merge_disabled` | Auto-merge is turned off: by someone (**Disable auto-merge**, `DELETE .../auto-merge` or `disable_auto_merge`), by a push from someone else, by converting the pull request to a draft, or by itself when it can't go through. The actor is who turned it off, the pusher, or `diffinery-auto-merge[bot]`. | `acme/website#42` | `enabledBy` (login), and `reason` when it turned itself off | |
| 206 | 207 | | `pull_request.auto_merged` | Auto-merge merges the pull request, as the account that turned it on. A `pull.merge` entry is written too. | `acme/website#42` | `base`, `method`, `sha` | |
| 207 | 208 | | `pull_request.auto_merge_enqueued` | Auto-merge adds the pull request to the base branch's merge queue | `acme/website#42` | `base`, `state` | |
| @@ -261,5 +262,5 @@ | ||
| 261 | 262 | | Symptom | Cause | |
| 262 | 263 | |---|---| |
| 263 | 264 | | You don't see **Audit log** in the settings sidebar | Only site administrators can view it. Ask a site admin; see [Site administration](/docs/admin/site-administration). | |
| 264 | -| A push or merge appears a few seconds late | `git.push`, `git.force_push`, `git.delete_ref`, `pull.merge`, `pull_request.merge_bypassed`, the `pull_request.auto_merge_*` and `pull_request.auto_merged` entries, `repo.protection.agent_auto_merge`, `repo.protection.agent_approval_waiver`, `repo.actions_workflow_settings` and push protection entries (including `api.blocked_secret` and `api.secret_bypassed`) are written by the event queue shortly after the action, not in the request itself. | | |
| 265 | +| A push or merge appears a few seconds late | `git.push`, `git.force_push`, `git.delete_ref`, `pull.merge`, `pull_request.merge_bypassed`, the `pull_request.auto_merge_*` and `pull_request.auto_merged` entries, `repo.protection.agent_auto_merge`, `repo.protection.agent_approval_waiver`, `repo.protection.agent_auto_merge_rule`, `repo.actions_workflow_settings` and push protection entries (including `api.blocked_secret` and `api.secret_bypassed`) are written by the event queue shortly after the action, not in the request itself. | | |
| 265 | 266 | | An older entry is missing from the page | The page shows only the 200 most recent entries. | |
modifiedapps/diffinery/src/docs/api-meta.ts+1−1View file
| @@ -713,7 +713,7 @@ | ||
| 713 | 713 | "PUT /repos/{owner}/{repo}/branches/{branch}/protection": { |
| 714 | 714 | summary: "Create or replace a protection rule", |
| 715 | 715 | description: |
| 716 | - "`branch` is a branch name or pattern. People only: agents and workflow tokens can't change protection. `allowAgentAutoMerge` (boolean) lets agents land pull requests through auto-merge or the merge queue; turning it on or off is audited as `repo.protection.agent_auto_merge`. `agentAutoMergeWithoutApproval` (boolean, needs `allowAgentAutoMerge`) lets those landings go through on green required checks without a person's approval, audited as `repo.protection.agent_approval_waiver`. See the branch protection guide for every field.", | |
| 716 | + "`branch` is a branch name or pattern. People only: agents and workflow tokens can't change protection. `allowAgentAutoMerge` (boolean) lets agents land pull requests through auto-merge or the merge queue; turning it on or off is audited as `repo.protection.agent_auto_merge`. `agentAutoMergeWithoutApproval` (boolean, needs `allowAgentAutoMerge`) lets those landings go through on green required checks without a person's approval, audited as `repo.protection.agent_approval_waiver`. `autoMergeAgents` (agent logins, needs `allowAgentAutoMerge`) has Diffinery turn auto-merge on by itself, as the agent, for pull requests those agents open, mark ready or push to, with `autoMergeMethod` (`merge`, `squash` or `rebase`) for how they merge; the answer stores them as `{ id, login }`, and changes are audited as `repo.protection.agent_auto_merge_rule`. See the branch protection guide for every field.", | |
| 717 | 717 | access: "admin", |
| 718 | 718 | }, |
| 719 | 719 | "DELETE /repos/{owner}/{repo}/branches/{branch}/protection": { summary: "Delete a protection rule", description: "Audited as `repo.protection.delete`.", access: "admin", status: 204 }, |
modifiedapps/diffinery/src/http/api.ts+6−4View file
| @@ -12,7 +12,7 @@ | ||
| 12 | 12 | import { publicJwk } from "../lib/jwt.js"; |
| 13 | 13 | import { visibleTo } from "../lib/moderation.js"; |
| 14 | 14 | import { HttpError, b64, fromB64 } from "../lib/util.js"; |
| 15 | -import { MAX_LOG_DEPTH, atLeast, ROLE_LEVEL, type ProtectionRules, type Role } from "../repo/types.js"; | |
| 15 | +import { MAX_LOG_DEPTH, atLeast, ROLE_LEVEL, type ProtectionRulesInput, type Role } from "../repo/types.js"; | |
| 16 | 16 | import type { JobInfo, RunInfo } from "../repo/actions.js"; |
| 17 | 17 | import type { CommitInfo } from "../repo/RepoDO.js"; |
| 18 | 18 | import type { QueueEntry } from "../repo/queue.js"; |
| @@ -768,10 +768,12 @@ | ||
| 768 | 768 | api.put("/repos/:owner/:repo/branches/:branch/protection", async (c) => { |
| 769 | 769 | const { stub, caller, repo } = await loadRepo(c, P(c).owner, P(c).repo, "admin"); |
| 770 | 770 | if (caller.kind === "agent") throw new HttpError(403, "agents cannot change branch protection"); |
| 771 | - const rules = await body<ProtectionRules>(c); | |
| 772 | - await stub.setProtection(c.req.param("branch"), rules, caller); | |
| 771 | + const input = await body<ProtectionRulesInput>(c); | |
| 772 | + await stub.setProtection(c.req.param("branch"), input, caller); | |
| 773 | 773 | auditRepo(c, caller, repo, "repo.protection.set", { pattern: c.req.param("branch") }); |
| 774 | - return c.json({ pattern: c.req.param("branch"), rules }); | |
| 774 | + // As stored: autoMergeAgents named by login come back as { id, login }. | |
| 775 | + const saved = (await stub.listProtections()).find((p) => p.pattern === c.req.param("branch")); | |
| 776 | + return c.json({ pattern: c.req.param("branch"), rules: saved?.rules ?? input }); | |
| 775 | 777 | }); |
| 776 | 778 | api.delete("/repos/:owner/:repo/branches/:branch/protection", async (c) => { |
| 777 | 779 | const { stub, caller, repo } = await loadRepo(c, P(c).owner, P(c).repo, "admin"); |
modifiedapps/diffinery/src/repo/RepoDO.ts+58−9View file
| @@ -52,6 +52,7 @@ | ||
| 52 | 52 | } from "../git/protocol.js"; |
| 53 | 53 | import { memorySource, packedContents, readStagedHead, repositoryBases, stagedContents, streamedContents, type PushContents } from "../git/push-contents.js"; |
| 54 | 54 | import { scanForSecrets, secretBypassReason, type SecretHit } from "../git/secret-scan.js"; |
| 55 | +import { Directory } from "../lib/directory.js"; | |
| 55 | 56 | import { GHOST, HttpError, isValidRefName, now } from "../lib/util.js"; |
| 56 | 57 | import { R2Sink } from "../lib/r2-sink.js"; |
| 57 | 58 | import { archivePrefix, deletePrefixes, pushBodyKey, repoPrefixes, type SpilledBody } from "../lib/storage-keys.js"; |
| @@ -74,9 +75,11 @@ | ||
| 74 | 75 | import { repoToday } from "../agentic/today-repo.js"; |
| 75 | 76 | import { Social } from "./social.js"; |
| 76 | 77 | import { Statuses } from "./statuses.js"; |
| 77 | -import { MAX_LOG_DEPTH, atLeast, globToRegExp, type Caller, type ProtectionRules, type RepoMeta } from "./types.js"; | |
| 78 | +import { MAX_LOG_DEPTH, atLeast, globToRegExp, type AutoMergeAgent, type Caller, type ProtectionRules, type ProtectionRulesInput, type RepoMeta } from "./types.js"; | |
| 78 | 79 | |
| 79 | 80 | const MAX_PUSH_BYTES = 100 * 1024 * 1024; |
| 81 | +/** Agents one branch rule may auto-merge pull requests from (ProtectionRules.autoMergeAgents). */ | |
| 82 | +const MAX_AUTO_MERGE_AGENTS = 20; | |
| 80 | 83 | /** Pushed blobs this large are hashed, scanned and stored from their compressed bytes, never held inflated. */ |
| 81 | 84 | const LARGE_OBJECT_BYTES = 4 * 1024 * 1024; |
| 82 | 85 | /** |
| @@ -771,9 +774,14 @@ | ||
| 771 | 774 | |
| 772 | 775 | /** As on GitHub: a rule naming the branch exactly wins; otherwise the oldest matching pattern. */ |
| 773 | 776 | protectionFor(branch: string): ProtectionRules | null { |
| 777 | + return this.protectionRuleFor(branch)?.rules ?? null; | |
| 778 | + } | |
| 779 | + | |
| 780 | + /** The rule that applies to `branch`, with its pattern: an exact name first, else the first matching pattern. */ | |
| 781 | + protectionRuleFor(branch: string): { pattern: string; rules: ProtectionRules } | null { | |
| 774 | 782 | const rows = this.sql.exec<{ pattern: string; rules: string }>("SELECT pattern, rules FROM protections ORDER BY rowid").toArray(); |
| 775 | 783 | const row = rows.find((r) => r.pattern === branch) ?? rows.find((r) => globToRegExp(r.pattern).test(branch)); |
| 776 | - return row ? (JSON.parse(row.rules) as ProtectionRules) : null; | |
| 784 | + return row ? { pattern: row.pattern, rules: JSON.parse(row.rules) as ProtectionRules } : null; | |
| 777 | 785 | } |
| 778 | 786 | |
| 779 | 787 | /** |
| @@ -1090,11 +1098,13 @@ | ||
| 1090 | 1098 | |
| 1091 | 1099 | /** |
| 1092 | 1100 | * Create, replace or (with null) delete a rule. Only people who administer the repository change |
| 1093 | - * branch protection: never agents, never a workflow's token. Turning `allowAgentAutoMerge` or | |
| 1094 | - * `agentAutoMergeWithoutApproval` on or off is audited on its own, since they let agents land changes. | |
| 1101 | + * branch protection: never agents, never a workflow's token. Turning `allowAgentAutoMerge`, | |
| 1102 | + * `agentAutoMergeWithoutApproval` or `autoMergeAgents` on or off is audited on its own, since they let | |
| 1103 | + * agents land changes. | |
| 1095 | 1104 | */ |
| 1096 | - setProtection(pattern: string, rules: ProtectionRules | null, caller: Caller): void { | |
| 1105 | + async setProtection(pattern: string, input: ProtectionRulesInput | null, caller: Caller): Promise<void> { | |
| 1097 | 1106 | if (!isPerson(caller) || caller.role !== "admin") throw new HttpError(403, caller.kind === "agent" ? "agents cannot change branch protection" : "only people who administer the repository can change branch protection"); |
| 1107 | + let rules = input ? await this.resolveAutoMergeAgents(input) : null; | |
| 1098 | 1108 | if (rules?.mergeQueue) { |
| 1099 | 1109 | const asked = typeof rules.mergeQueue === "object" ? rules.mergeQueue.method : undefined; |
| 1100 | 1110 | const mergeQueue = normalizeMergeQueue(rules.mergeQueue); |
| @@ -1111,18 +1121,26 @@ | ||
| 1111 | 1121 | if (rules && rules.allowAgentAutoMerge !== undefined && typeof rules.allowAgentAutoMerge !== "boolean") throw new HttpError(422, "allowAgentAutoMerge must be a boolean"); |
| 1112 | 1122 | if (rules && rules.agentAutoMergeWithoutApproval !== undefined && typeof rules.agentAutoMergeWithoutApproval !== "boolean") throw new HttpError(422, "agentAutoMergeWithoutApproval must be a boolean"); |
| 1113 | 1123 | if (rules?.agentAutoMergeWithoutApproval && !rules.allowAgentAutoMerge) throw new HttpError(422, "agentAutoMergeWithoutApproval needs allowAgentAutoMerge: it only applies to what Diffinery lands for agents"); |
| 1124 | + if (rules?.autoMergeAgents?.length && !rules.allowAgentAutoMerge) throw new HttpError(422, "autoMergeAgents needs allowAgentAutoMerge: Diffinery turns on auto-merge as those agents, under the same rules as their own"); | |
| 1125 | + if (rules?.autoMergeMethod !== undefined && !(["merge", "squash", "rebase"] as const).includes(rules.autoMergeMethod)) throw new HttpError(422, "autoMergeMethod must be merge, squash or rebase"); | |
| 1126 | + if (rules?.autoMergeMethod === "merge" && rules.requireLinearHistory) throw new HttpError(422, "this branch requires linear history, so autoMergeMethod must be squash or rebase"); | |
| 1114 | 1127 | if (rules && !rules.allowAgentAutoMerge) { |
| 1115 | - const { allowAgentAutoMerge: _, agentAutoMergeWithoutApproval: _w, ...rest } = rules; | |
| 1128 | + const { allowAgentAutoMerge: _, agentAutoMergeWithoutApproval: _w, autoMergeAgents: _a, autoMergeMethod: _m, ...rest } = rules; | |
| 1116 | 1129 | rules = rest; |
| 1117 | 1130 | } |
| 1118 | 1131 | if (rules && !rules.agentAutoMergeWithoutApproval) { |
| 1119 | 1132 | const { agentAutoMergeWithoutApproval: _, ...rest } = rules; |
| 1120 | 1133 | rules = rest; |
| 1121 | 1134 | } |
| 1135 | + if (rules && !rules.autoMergeAgents?.length) { | |
| 1136 | + const { autoMergeAgents: _, autoMergeMethod: _m, ...rest } = rules; | |
| 1137 | + rules = rest; | |
| 1138 | + } | |
| 1122 | 1139 | const before = this.sql.exec<{ rules: string }>("SELECT rules FROM protections WHERE pattern = ?", pattern).toArray()[0]; |
| 1123 | 1140 | const old = before ? (JSON.parse(before.rules) as ProtectionRules) : null; |
| 1124 | 1141 | const was = !!old?.allowAgentAutoMerge; |
| 1125 | 1142 | const waived = !!old?.agentAutoMergeWithoutApproval; |
| 1143 | + const agentsKey = (r: ProtectionRules | null) => (r?.autoMergeAgents?.length ? `${r.autoMergeAgents.map((a) => a.id).join(",")}|${r.autoMergeMethod ?? ""}` : ""); | |
| 1126 | 1144 | if (rules) this.sql.exec("INSERT INTO protections (pattern, rules) VALUES (?, ?) ON CONFLICT(pattern) DO UPDATE SET rules = excluded.rules", pattern, JSON.stringify(rules)); |
| 1127 | 1145 | else this.sql.exec("DELETE FROM protections WHERE pattern = ?", pattern); |
| 1128 | 1146 | if (was !== !!rules?.allowAgentAutoMerge) { |
| @@ -1133,11 +1151,41 @@ | ||
| 1133 | 1151 | const m = this.meta; |
| 1134 | 1152 | this.emit({ type: "audit", action: "repo.protection.agent_approval_waiver", actor: { id: caller.id, login: caller.login, kind: caller.kind, onBehalfOf: caller.onBehalfOf }, target: `${m.owner}/${m.name}`, detail: { pattern, enabled: !!rules?.agentAutoMergeWithoutApproval, requiredChecks: rules?.requiredChecks ?? [] }, at: now() }); |
| 1135 | 1153 | } |
| 1154 | + if (agentsKey(old) !== agentsKey(rules)) { | |
| 1155 | + const m = this.meta; | |
| 1156 | + this.emit({ | |
| 1157 | + type: "audit", | |
| 1158 | + action: "repo.protection.agent_auto_merge_rule", | |
| 1159 | + actor: { id: caller.id, login: caller.login, kind: caller.kind, onBehalfOf: caller.onBehalfOf }, | |
| 1160 | + target: `${m.owner}/${m.name}`, | |
| 1161 | + detail: { pattern, enabled: !!rules?.autoMergeAgents?.length, agents: (rules?.autoMergeAgents ?? []).map((a) => a.login), method: rules?.autoMergeMethod ?? null, withoutApproval: !!rules?.agentAutoMergeWithoutApproval, requiredChecks: rules?.requiredChecks ?? [] }, | |
| 1162 | + at: now(), | |
| 1163 | + }); | |
| 1164 | + } | |
| 1136 | 1165 | // Turning a queue off (or changing its checks) affects what is queued, and what may auto-merge. |
| 1137 | 1166 | this.queue.schedule(); |
| 1138 | 1167 | this.autoMerge.schedule(); |
| 1139 | 1168 | } |
| 1140 | 1169 | |
| 1170 | + /** `autoMergeAgents` names agent accounts, by login or as stored ({ id, login }); store them by id, with the login as it is now. */ | |
| 1171 | + private async resolveAutoMergeAgents(input: ProtectionRulesInput): Promise<ProtectionRules> { | |
| 1172 | + const { autoMergeAgents: names, ...rest } = input; | |
| 1173 | + if (names === undefined || names === null) return rest; | |
| 1174 | + if (!Array.isArray(names)) throw new HttpError(422, "autoMergeAgents must be a list of agent logins"); | |
| 1175 | + if (names.length > MAX_AUTO_MERGE_AGENTS) throw new HttpError(422, `autoMergeAgents can name at most ${MAX_AUTO_MERGE_AGENTS} agents`); | |
| 1176 | + const dir = new Directory(this.env); | |
| 1177 | + const agents: AutoMergeAgent[] = []; | |
| 1178 | + for (const n of names) { | |
| 1179 | + const login = typeof n === "string" ? n.trim().replace(/^@/, "") : n && typeof n === "object" && typeof n.login === "string" ? n.login : null; | |
| 1180 | + if (!login) throw new HttpError(422, "autoMergeAgents must be a list of agent logins"); | |
| 1181 | + const account = typeof n === "object" && typeof n.id === "string" ? ((await dir.account(n.id)) ?? (await dir.accountByLogin(login))) : await dir.accountByLogin(login); | |
| 1182 | + if (!account) throw new HttpError(422, `autoMergeAgents: there is no account named ${login}`); | |
| 1183 | + if (account.kind !== "agent") throw new HttpError(422, `autoMergeAgents: ${account.login} is not an agent; only agents' pull requests can be auto-merged by a rule`); | |
| 1184 | + if (!agents.some((a) => a.id === account.id)) agents.push({ id: account.id, login: account.login }); | |
| 1185 | + } | |
| 1186 | + return agents.length ? { ...rest, autoMergeAgents: agents } : rest; | |
| 1187 | + } | |
| 1188 | + | |
| 1141 | 1189 | // ---------------------------------------------------------------- RPC passthroughs (collab + actions) |
| 1142 | 1190 | // Workers RPC only exposes methods on the class itself, so the sub-modules are wrapped here. |
| 1143 | 1191 | |
| @@ -1235,8 +1283,9 @@ | ||
| 1235 | 1283 | mergeQueueSettings(...a: Parameters<MergeQueue["settings"]>) { |
| 1236 | 1284 | return this.queue.settings(...a); |
| 1237 | 1285 | } |
| 1238 | - enableAutoMerge(...a: Parameters<AutoMerge["enable"]>) { | |
| 1239 | - return this.autoMerge.enable(...a); | |
| 1286 | + /** A caller's own auto-merge; only the server turns one on for a branch rule (AutoMerge.applyRule). */ | |
| 1287 | + enableAutoMerge(number: number, input: Parameters<AutoMerge["enable"]>[1], caller: Caller) { | |
| 1288 | + return this.autoMerge.enable(number, input, caller); | |
| 1240 | 1289 | } |
| 1241 | 1290 | disableAutoMerge(...a: Parameters<AutoMerge["disable"]>) { |
| 1242 | 1291 | return this.autoMerge.disable(...a); |
| @@ -1881,7 +1930,7 @@ | ||
| 1881 | 1930 | importProtection(pattern: string, rules: ProtectionRules | null, importId: string) { |
| 1882 | 1931 | // What agents may do is a person's decision here, never something an import brings in. |
| 1883 | 1932 | if (rules) { |
| 1884 | - const { allowAgentAutoMerge: _a, agentAutoMergeWithoutApproval: _w, agentsMayPush: _p, agentsMayMerge: _m, ...rest } = rules; | |
| 1933 | + const { allowAgentAutoMerge: _a, agentAutoMergeWithoutApproval: _w, autoMergeAgents: _g, autoMergeMethod: _gm, agentsMayPush: _p, agentsMayMerge: _m, ...rest } = rules; | |
| 1885 | 1934 | rules = rest; |
| 1886 | 1935 | } |
| 1887 | 1936 | if (rules?.mergeQueue) rules = { ...rules, mergeQueue: normalizeMergeQueue(rules.mergeQueue) }; |
modifiedapps/diffinery/src/repo/auto-merge.ts+61−10View file
| @@ -15,7 +15,9 @@ | ||
| 15 | 15 | * |
| 16 | 16 | * It is also how agents land changes on protected branches without a bypass: they never |
| 17 | 17 | * merge there themselves, but a person who administers the repository can let them have |
| 18 | - * Diffinery do it (`allowAgentAutoMerge`), on a branch whose required checks gate it. | |
| 18 | + * Diffinery do it (`allowAgentAutoMerge`), on a branch whose required checks gate it. With | |
| 19 | + * `autoMergeAgents` on the rule too, Diffinery turns it on by itself for those agents' pull | |
| 20 | + * requests (applyRule), so no agent or person has to at merge time. | |
| 19 | 21 | */ |
| 20 | 22 | |
| 21 | 23 | export type MergeMethod = "merge" | "squash" | "rebase"; |
| @@ -32,10 +34,12 @@ | ||
| 32 | 34 | permissions?: Caller["permissions"]; |
| 33 | 35 | /** Delete the head branch once it lands, as a manual merge's `delete_branch` does (collab.ts completeMerge says which branches). */ |
| 34 | 36 | deleteBranch?: boolean; |
| 37 | + /** The branch rule pattern whose `autoMergeAgents` turned it on, for the agent in `enabledBy`. */ | |
| 38 | + byRule?: string; | |
| 35 | 39 | }; |
| 36 | 40 | |
| 37 | 41 | /** What the API and the web show: who turned it on, and how it will merge. */ |
| 38 | -export type AutoMergeInfo = Pick<AutoMergeState, "method" | "commitTitle" | "commitMessage" | "enabledBy" | "enabledAt"> & { deleteBranch: boolean }; | |
| 42 | +export type AutoMergeInfo = Pick<AutoMergeState, "method" | "commitTitle" | "commitMessage" | "enabledBy" | "enabledAt" | "byRule"> & { deleteBranch: boolean }; | |
| 39 | 43 | |
| 40 | 44 | export type AutoMergeResult = { state: "enabled" | "queued" | "merged"; autoMerge: AutoMergeInfo | null; mergeCommit?: string | null }; |
| 41 | 45 | |
| @@ -119,11 +123,15 @@ | ||
| 119 | 123 | } |
| 120 | 124 | |
| 121 | 125 | static info(s: AutoMergeState | null): AutoMergeInfo | null { |
| 122 | - return s ? { method: s.method, commitTitle: s.commitTitle, commitMessage: s.commitMessage, enabledBy: s.enabledBy, enabledAt: s.enabledAt, deleteBranch: !!s.deleteBranch } : null; | |
| 126 | + return s ? { method: s.method, commitTitle: s.commitTitle, commitMessage: s.commitMessage, enabledBy: s.enabledBy, enabledAt: s.enabledAt, deleteBranch: !!s.deleteBranch, ...(s.byRule ? { byRule: s.byRule } : {}) } : null; | |
| 123 | 127 | } |
| 124 | 128 | |
| 125 | - /** Turn auto-merge on (or change how it merges), then merge or enqueue right away if the pull request is ready. */ | |
| 126 | - async enable(number: number, input: { method?: string; commitTitle?: string | null; commitMessage?: string | null; deleteBranch?: boolean; expectedHeadSha?: string }, caller: Caller): Promise<AutoMergeResult> { | |
| 129 | + /** | |
| 130 | + * Turn auto-merge on (or change how it merges), then merge or enqueue right away if the pull request is ready. | |
| 131 | + * `opts.rule`: the server turns it on for `caller`, the pull request's agent, because that branch rule names it | |
| 132 | + * (applyRule); it is then recorded as the server's doing. | |
| 133 | + */ | |
| 134 | + async enable(number: number, input: { method?: string; commitTitle?: string | null; commitMessage?: string | null; deleteBranch?: boolean; expectedHeadSha?: string }, caller: Caller, opts: { rule?: string } = {}): Promise<AutoMergeResult> { | |
| 127 | 135 | const { issue } = this.repo.collab.get(number); |
| 128 | 136 | const pull = issue.pull; |
| 129 | 137 | if (!pull) throw new HttpError(404, "not a pull request"); |
| @@ -153,16 +161,54 @@ | ||
| 153 | 161 | role: caller.role, |
| 154 | 162 | ...(caller.permissions ? { permissions: caller.permissions } : {}), |
| 155 | 163 | ...(input.deleteBranch === true ? { deleteBranch: true } : {}), |
| 164 | + ...(opts.rule ? { byRule: opts.rule } : {}), | |
| 156 | 165 | }; |
| 166 | + // Turned on by a branch rule: the server did it (for the agent it merges as), and the record says so. | |
| 167 | + const by = opts.rule ? AUTO_MERGE_BOT : caller; | |
| 157 | 168 | this.sql.exec("UPDATE pulls SET auto_merge = ? WHERE number = ?", JSON.stringify(state), number); |
| 158 | - this.repo.social.record(number, "auto_merge_enabled", caller, { merge_method: method }); | |
| 159 | - this.event(number, "auto_merge_enabled", caller); | |
| 160 | - this.audit("pull_request.auto_merge_enabled", caller, number, { base: pull.base, method, viaAgentRule: landsByAgentRule(caller, rules, pull.base === this.repo.meta.defaultBranch) }); | |
| 169 | + this.repo.social.record(number, "auto_merge_enabled", by, { merge_method: method, ...(opts.rule ? { rule: opts.rule, agent: caller.login } : {}) }); | |
| 170 | + this.event(number, "auto_merge_enabled", by); | |
| 171 | + this.audit("pull_request.auto_merge_enabled", by, number, { base: pull.base, method, viaAgentRule: landsByAgentRule(caller, rules, pull.base === this.repo.meta.defaultBranch), ...(opts.rule ? { byRule: opts.rule, agent: caller.login } : {}) }); | |
| 161 | 172 | this.repo.broadcast(`pr:${number}`, { action: "auto_merge" }); |
| 162 | 173 | await this.process({ number }); |
| 163 | 174 | return this.result(number); |
| 164 | 175 | } |
| 165 | 176 | |
| 177 | + /** | |
| 178 | + * A branch rule's `autoMergeAgents`: when one of those agents opens a pull request into a matching branch, marks | |
| 179 | + * it ready, or pushes to it, Diffinery turns auto-merge on by itself, as that agent. It then lands exactly as the | |
| 180 | + * agent's own auto-merge would: `allowAgentAutoMerge` and a required check, the agent's write access at merge | |
| 181 | + * time, approvals unless the rule waives them, no requested changes, and a person for a workflow change. | |
| 182 | + * Never again on a pull request where anyone turned auto-merge off, or the server did. `announce`: when it | |
| 183 | + * can't turn on, say why in a comment (once, when the pull request is opened or marked ready). | |
| 184 | + */ | |
| 185 | + async applyRule(number: number, announce: boolean): Promise<void> { | |
| 186 | + const r = this.sql | |
| 187 | + .exec<{ author_id: string; author_kind: string; state: string; draft: number; merged: number; auto_merge: string | null; base_ref: string }>( | |
| 188 | + "SELECT i.author_id, i.author_kind, i.state, p.draft, p.merged, p.auto_merge, p.base_ref FROM issues i JOIN pulls p ON p.number = i.number WHERE i.number = ?", | |
| 189 | + number, | |
| 190 | + ) | |
| 191 | + .toArray()[0]; | |
| 192 | + if (!r || r.state !== "open" || r.merged || r.draft || r.auto_merge || r.author_kind !== "agent") return; | |
| 193 | + const rule = this.repo.protectionRuleFor(r.base_ref); | |
| 194 | + if (!rule?.rules.allowAgentAutoMerge || !rule.rules.autoMergeAgents?.some((a) => a.id === r.author_id)) return; | |
| 195 | + if (this.repo.queue.isQueued(number)) return; | |
| 196 | + if (this.sql.exec("SELECT 1 FROM issue_events WHERE number = ? AND event = 'auto_merge_disabled' LIMIT 1", number).toArray().length) return; | |
| 197 | + const caller = await this.accountCaller(r.author_id, "admin"); | |
| 198 | + if (!caller || caller.kind !== "agent") return; | |
| 199 | + try { | |
| 200 | + await this.enable(number, { method: rule.rules.autoMergeMethod }, caller, { rule: rule.pattern }); | |
| 201 | + } catch (e) { | |
| 202 | + if (!announce) return; | |
| 203 | + const why = e instanceof HttpError ? e.detail : "it could not be turned on"; | |
| 204 | + try { | |
| 205 | + this.repo.collab.addComment(number, `Auto-merge didn't turn on by itself (branch rule \`${rule.pattern}\` auto-merges ${caller.login}'s pull requests): ${why}. A person can merge it.`, AUTO_MERGE_BOT); | |
| 206 | + } catch (err) { | |
| 207 | + console.warn("auto-merge rule comment failed", err); | |
| 208 | + } | |
| 209 | + } | |
| 210 | + } | |
| 211 | + | |
| 166 | 212 | private result(number: number): AutoMergeResult { |
| 167 | 213 | const p = this.sql.exec<{ merged: number; merge_commit: string | null }>("SELECT merged, merge_commit FROM pulls WHERE number = ?", number).one(); |
| 168 | 214 | if (p.merged) return { state: "merged", autoMerge: null, mergeCommit: p.merge_commit }; |
| @@ -308,14 +354,19 @@ | ||
| 308 | 354 | private async callerOf(s: AutoMergeState): Promise<Caller | null> { |
| 309 | 355 | const by = s.enabledBy; |
| 310 | 356 | if (isJobCaller(by)) return { ...by, role: s.role, permissions: s.permissions }; |
| 357 | + return this.accountCaller(by.id, s.role); | |
| 358 | + } | |
| 359 | + | |
| 360 | + /** An account as a caller in this repository, with its access as it is now, never above `cap`. */ | |
| 361 | + private async accountCaller(id: string, cap: Role): Promise<Caller | null> { | |
| 311 | 362 | const dir = new Directory(this.env); |
| 312 | - const [account, repo] = await Promise.all([dir.account(by.id), dir.repoById(this.repo.meta.repoId)]); | |
| 363 | + const [account, repo] = await Promise.all([dir.account(id), dir.repoById(this.repo.meta.repoId)]); | |
| 313 | 364 | if (!account || !repo) return null; |
| 314 | 365 | const role = await dir.roleFor(account, repo); |
| 315 | 366 | if (!role) return null; |
| 316 | 367 | const kind = account.kind === "agent" ? "agent" : "user"; |
| 317 | 368 | const onBehalfOf = kind === "agent" && account.owner_id ? (await dir.account(account.owner_id))?.login : undefined; |
| 318 | - return { id: account.id, login: account.login, kind, ...(onBehalfOf ? { onBehalfOf } : {}), role: minRole(role, s.role), ...(agentMayMerge(account) ? { mayMerge: true } : {}) }; | |
| 369 | + return { id: account.id, login: account.login, kind, ...(onBehalfOf ? { onBehalfOf } : {}), role: minRole(role, cap), ...(agentMayMerge(account) ? { mayMerge: true } : {}) }; | |
| 319 | 370 | } |
| 320 | 371 | |
| 321 | 372 | private event(number: number, action: "auto_merge_enabled" | "auto_merge_disabled", by: Caller): void { |
modifiedapps/diffinery/src/repo/collab.ts+8−1View file
| @@ -494,7 +494,10 @@ | ||
| 494 | 494 | this.repo.background(this.repo.actions.onPullRequest(action, number, caller).catch((e) => console.error("pr workflow trigger", e))); |
| 495 | 495 | // As on GitHub, a draft can't auto-merge; one marked ready may now be mergeable. |
| 496 | 496 | if (draft) this.repo.autoMerge.cancel(number, caller, "the pull request was converted to a draft"); |
| 497 | - else this.repo.autoMerge.schedule({ number }); | |
| 497 | + else { | |
| 498 | + this.repo.autoMerge.schedule({ number }); | |
| 499 | + this.repo.background(this.repo.autoMerge.applyRule(number, true)); | |
| 500 | + } | |
| 498 | 501 | this.repo.broadcast(`pr:${number}`, { action: "updated" }); |
| 499 | 502 | return this.toIssue(this.row(number)); |
| 500 | 503 | } |
| @@ -596,6 +599,8 @@ | ||
| 596 | 599 | const m = this.repo.meta; |
| 597 | 600 | this.repo.emit({ type: "pull_request", action: "opened", repoId: m.repoId, owner: m.owner, repo: m.name, number: n, headRef: input.head, headSha, baseRef: base, actor: actorOf(caller), at: t }); |
| 598 | 601 | await this.repo.actions.onPullRequest("opened", n, caller).catch((e) => console.error("pr workflow trigger", e)); |
| 602 | + // A branch rule may auto-merge this agent's pull requests (auto-merge.ts applyRule). | |
| 603 | + if (!input.draft) this.repo.background(this.repo.autoMerge.applyRule(n, true)); | |
| 599 | 604 | return this.toIssue(this.row(n)); |
| 600 | 605 | } |
| 601 | 606 | |
| @@ -660,6 +665,8 @@ | ||
| 660 | 665 | await this.repo.actions.onPullRequest("synchronize", p.number, caller).catch((e) => console.error("pr workflow trigger", e)); |
| 661 | 666 | if (this.repo.queue.isQueued(p.number)) this.repo.queue.schedule(p.base_ref); |
| 662 | 667 | this.repo.autoMerge.schedule({ number: p.number }); |
| 668 | + // The agent that opened it pushed again: a branch rule may auto-merge its pull requests (opened before the rule, say). | |
| 669 | + if (this.row(p.number).author_id === caller.id) this.repo.background(this.repo.autoMerge.applyRule(p.number, false)); | |
| 663 | 670 | this.repo.broadcast(`pr:${p.number}`, { action: "synchronize", sha }); |
| 664 | 671 | out.push(p.number); |
| 665 | 672 | } |
modifiedapps/diffinery/src/repo/types.ts+15−0View file
| @@ -75,10 +75,25 @@ | ||
| 75 | 75 | * Only people who administer the repository change it, and every change is audited. Default false. |
| 76 | 76 | */ |
| 77 | 77 | agentAutoMergeWithoutApproval?: boolean; |
| 78 | + /** | |
| 79 | + * With `allowAgentAutoMerge`: Diffinery turns auto-merge on by itself, as the agent, for pull requests | |
| 80 | + * these agents open into matching branches (repo/auto-merge.ts applyRule), so no one has to at merge | |
| 81 | + * time. Everything an agent's own auto-merge needs still applies. Stored by account id; the API also | |
| 82 | + * takes logins. Only people who administer the repository change it, and every change is audited. | |
| 83 | + */ | |
| 84 | + autoMergeAgents?: AutoMergeAgent[]; | |
| 85 | + /** How what `autoMergeAgents` turns on merges. Default: the merge queue's method, else squash with linear history, else a merge commit. */ | |
| 86 | + autoMergeMethod?: "merge" | "squash" | "rebase"; | |
| 78 | 87 | /** Pull requests merge through a merge queue (like GitHub's "Require merge queue"). */ |
| 79 | 88 | mergeQueue?: MergeQueueSettings; |
| 80 | 89 | } |
| 81 | 90 | |
| 91 | +/** An agent account whose pull requests a branch rule auto-merges. */ | |
| 92 | +export type AutoMergeAgent = { id: string; login: string }; | |
| 93 | + | |
| 94 | +/** Branch protection as the API and the settings form send it: `autoMergeAgents` may name agents by login. */ | |
| 95 | +export type ProtectionRulesInput = Omit<ProtectionRules, "autoMergeAgents"> & { autoMergeAgents?: Array<string | AutoMergeAgent> }; | |
| 96 | + | |
| 82 | 97 | export interface MergeQueueSettings { |
| 83 | 98 | /** How each queued pull request is merged. Default "merge". */ |
| 84 | 99 | method?: "merge" | "squash" | "rebase"; |
modifiedapps/diffinery/src/web/collab.tsx+11−1View file
| @@ -881,7 +881,17 @@ | ||
| 881 | 881 | <b>Auto-merge is on</b> |
| 882 | 882 | <span class="muted"> |
| 883 | 883 | {" "} |
| 884 | - · 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`} | |
| 884 | + ·{" "} | |
| 885 | + {a.byRule ? ( | |
| 886 | + <> | |
| 887 | + turned on by the branch rule <span class="mono">{a.byRule}</span> for <Actor login={by.login} kind={by.kind} onBehalfOf={by.onBehalfOf} avatar={false} /> | |
| 888 | + </> | |
| 889 | + ) : ( | |
| 890 | + <> | |
| 891 | + turned on by <Actor login={by.login} kind={by.kind} onBehalfOf={by.onBehalfOf} avatar={false} /> | |
| 892 | + </> | |
| 893 | + )}{" "} | |
| 894 | + · {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`} | |
| 885 | 895 | {a.deleteBranch ? ( |
| 886 | 896 | <> |
| 887 | 897 | {" "} |
modifiedapps/diffinery/src/web/repo-settings.tsx+38−4View file
| @@ -8,7 +8,7 @@ | ||
| 8 | 8 | import { MAX_REVIEWERS, MAX_WAIT_TIMER, type Environment } from "../repo/deployments.js"; |
| 9 | 9 | import { resolveReviewers } from "../http/environment-reviews.js"; |
| 10 | 10 | import { parseCheckList } from "../repo/required.js"; |
| 11 | -import { atLeast, type ProtectionRules, type Role } from "../repo/types.js"; | |
| 11 | +import { atLeast, type ProtectionRules, type ProtectionRulesInput, type Role } from "../repo/types.js"; | |
| 12 | 12 | import { ICONS } from "./assets.js"; |
| 13 | 13 | import { KCheckbox, KRadio, KSelect, type KOption } from "./kumo.js"; |
| 14 | 14 | import { csrfToken, type FormData } from "./csrf.js"; |
| @@ -88,7 +88,7 @@ | ||
| 88 | 88 | |
| 89 | 89 | // ---------------------------------------------------------------- protection form |
| 90 | 90 | |
| 91 | -const RULE_FLAGS: Array<[keyof ProtectionRules, string, string]> = [ | |
| 91 | +const RULE_FLAGS: Array<[keyof ProtectionRules & keyof ProtectionRulesInput, string, string]> = [ | |
| 92 | 92 | ["requirePullRequest", "Require a pull request before merging", "Direct pushes are rejected; changes land through reviewed pull requests."], |
| 93 | 93 | ["dismissStaleReviews", "Dismiss stale approvals when new commits are pushed", ""], |
| 94 | 94 | ["requireLinearHistory", "Require linear history", "Merge commits are rejected; use squash or rebase merges."], |
| @@ -148,6 +148,31 @@ | ||
| 148 | 148 | ))} |
| 149 | 149 | </div> |
| 150 | 150 | </div> |
| 151 | + <div class="row" style="align-items:flex-start"> | |
| 152 | + <div class="field grow"> | |
| 153 | + <label>Auto-merge pull requests from these agents</label> | |
| 154 | + <textarea name="autoMergeAgents" class="mono" style="min-height:60px" autocomplete="off" spellcheck={false} placeholder="my-agent"> | |
| 155 | + {(rules.autoMergeAgents ?? []).map((a) => a.login).join("\n")} | |
| 156 | + </textarea> | |
| 157 | + <div class="hint"> | |
| 158 | + One agent login per line. Needs <b>Allow agents to land pull requests through auto-merge</b>. Diffinery turns auto-merge on by itself for their pull requests, so nobody has to click Merge: each lands once every rule above is met (an approval too, unless agents' auto-merges are allowed to land without one). A change to a workflow file still needs a person. | |
| 159 | + </div> | |
| 160 | + </div> | |
| 161 | + <div class="field" style="width:240px"> | |
| 162 | + <KSelect | |
| 163 | + name="autoMergeMethod" | |
| 164 | + label="Their merge method" | |
| 165 | + block | |
| 166 | + value={rules.autoMergeMethod ?? "default"} | |
| 167 | + options={[ | |
| 168 | + { value: "default", label: "As the branch merges" }, | |
| 169 | + { value: "merge", label: "Create a merge commit" }, | |
| 170 | + { value: "squash", label: "Squash and merge" }, | |
| 171 | + { value: "rebase", label: "Rebase and merge" }, | |
| 172 | + ]} | |
| 173 | + /> | |
| 174 | + </div> | |
| 175 | + </div> | |
| 151 | 176 | <div class="field"> |
| 152 | 177 | <KCheckbox |
| 153 | 178 | name="mergeQueue" |
| @@ -187,13 +212,21 @@ | ||
| 187 | 212 | </Form> |
| 188 | 213 | ); |
| 189 | 214 | |
| 190 | -function rulesFrom(f: FormData): ProtectionRules { | |
| 191 | - const rules: ProtectionRules = {}; | |
| 215 | +function rulesFrom(f: FormData): ProtectionRulesInput { | |
| 216 | + const rules: ProtectionRulesInput = {}; | |
| 192 | 217 | for (const [key] of RULE_FLAGS) if (f.b(key)) (rules as Record<string, unknown>)[key] = true; |
| 193 | 218 | const approvals = Math.max(0, Math.min(10, parseInt(f.s("requiredApprovals"), 10) || 0)); |
| 194 | 219 | if (approvals) rules.requiredApprovals = approvals; |
| 195 | 220 | const checks = parseCheckList(f.s("requiredChecks")); |
| 196 | 221 | if (checks.length) rules.requiredChecks = checks; |
| 222 | + const agents = f | |
| 223 | + .s("autoMergeAgents") | |
| 224 | + .split(/[\s,]+/) | |
| 225 | + .map((x) => x.trim().replace(/^@/, "")) | |
| 226 | + .filter(Boolean); | |
| 227 | + if (agents.length) rules.autoMergeAgents = agents; | |
| 228 | + const agentMethod = f.s("autoMergeMethod"); | |
| 229 | + if (agents.length && (agentMethod === "merge" || agentMethod === "squash" || agentMethod === "rebase")) rules.autoMergeMethod = agentMethod; | |
| 197 | 230 | if (f.b("mergeQueue")) { |
| 198 | 231 | const method = f.s("mqMethod"); |
| 199 | 232 | rules.mergeQueue = { |
| @@ -447,6 +480,7 @@ | ||
| 447 | 480 | r.rules.requiredChecks?.length ? `${r.rules.requiredChecks.length} required check${r.rules.requiredChecks.length === 1 ? "" : "s"}` : "", |
| 448 | 481 | r.rules.requireLinearHistory ? "linear history" : "", |
| 449 | 482 | r.rules.mergeQueue ? "merge queue" : "", |
| 483 | + r.rules.autoMergeAgents?.length ? `auto-merges ${r.rules.autoMergeAgents.map((a) => a.login).join(", ")}` : "", | |
| 450 | 484 | r.rules.enforceAdmins ? "admins included" : "", |
| 451 | 485 | ] |
| 452 | 486 | .filter(Boolean) |
modifiedapps/diffinery/test/e2e/security.test.mjs+126−0View file
| @@ -1020,6 +1020,132 @@ | ||
| 1020 | 1020 | await until("merged after the approval", async () => (await pull(after.number)).merged); |
| 1021 | 1021 | }); |
| 1022 | 1022 | |
| 1023 | +test("security: a branch rule has Diffinery auto-merge chosen agents' pull requests by itself, under every rule their own auto-merge has", async () => { | |
| 1024 | + const R = `/repos/${OWNER}/ruled`; | |
| 1025 | + const mainSha = await repoWithReadme("ruled"); | |
| 1026 | + await status(204, "PUT", `${R}/collaborators/olive-bot`, { permission: "write" }); | |
| 1027 | + await status(204, "PUT", `${R}/collaborators/olive-helper`, { permission: "write" }); | |
| 1028 | + const pull = (n) => ok("GET", `${R}/pulls/${n}`); | |
| 1029 | + const said = async (n) => (await ok("GET", `${R}/issues/${n}/comments`)).map((c) => c.body); | |
| 1030 | + const opened = async (branch, as, { from = mainSha, path = `${branch}.txt`, draft = false } = {}) => { | |
| 1031 | + await ok("POST", `${R}/git/refs`, { ref: `refs/heads/${branch}`, sha: from }, as); | |
| 1032 | + const put = await ok("PUT", `${R}/contents/${path}`, { message: branch, content: b64(`${branch}\n`), branch }, as); | |
| 1033 | + const pr = await ok("POST", `${R}/pulls`, { title: branch, head: branch, base: "main", draft }, as); | |
| 1034 | + return { number: pr.number, head: put.commit.sha, branch }; | |
| 1035 | + }; | |
| 1036 | + const pushed = async (pr, as) => (await ok("PUT", `${R}/contents/${pr.branch}-2.txt`, { message: "more", content: b64("more\n"), branch: pr.branch }, as)).commit.sha; | |
| 1037 | + const green = (head) => ok("POST", `${R}/check-runs`, { name: "ci", head_sha: head, status: "completed", conclusion: "success" }); | |
| 1038 | + const rule = { requirePullRequest: true, requiredChecks: ["ci"], requiredApprovals: 1, allowAgentAutoMerge: true, agentAutoMergeWithoutApproval: true }; | |
| 1039 | + | |
| 1040 | + // ------------------------------------------------------------ only an admin sets it, only for agents, only alongside allowAgentAutoMerge | |
| 1041 | + await status(422, "PUT", `${R}/branches/main/protection`, { requirePullRequest: true, requiredChecks: ["ci"], autoMergeAgents: ["olive-bot"] }); | |
| 1042 | + await status(422, "PUT", `${R}/branches/main/protection`, { ...rule, autoMergeAgents: ["no-such-agent"] }); | |
| 1043 | + assert.match((await status(422, "PUT", `${R}/branches/main/protection`, { ...rule, autoMergeAgents: [OWNER] })).message, /not an agent/); | |
| 1044 | + await status(422, "PUT", `${R}/branches/main/protection`, { ...rule, autoMergeAgents: "olive-bot" }); | |
| 1045 | + await status(422, "PUT", `${R}/branches/main/protection`, { ...rule, requireLinearHistory: true, autoMergeAgents: ["olive-bot"], autoMergeMethod: "merge" }); | |
| 1046 | + await status(422, "PUT", `${R}/branches/main/protection`, { ...rule, autoMergeAgents: ["olive-bot"], autoMergeMethod: "fast" }); | |
| 1047 | + await status(403, "PUT", `${R}/branches/main/protection`, { ...rule, autoMergeAgents: ["olive-bot"] }, T.bot); | |
| 1048 | + | |
| 1049 | + // ------------------------------------------------------------ a pull request opened before the rule names the agent isn't swept in | |
| 1050 | + await ok("PUT", `${R}/branches/main/protection`, rule); | |
| 1051 | + const early = await opened("agent-early", T.bot); | |
| 1052 | + const saved = await ok("PUT", `${R}/branches/main/protection`, { ...rule, autoMergeAgents: ["@olive-bot", "olive-bot"], autoMergeMethod: "squash" }); | |
| 1053 | + assert.deepEqual(saved.rules.autoMergeAgents.map((a) => a.login), ["olive-bot"], "stored once, by account"); | |
| 1054 | + assert.equal(typeof saved.rules.autoMergeAgents[0].id, "string"); | |
| 1055 | + await until("rule audit", async () => | |
| 1056 | + auditLog("repo.protection.agent_auto_merge_rule").some((e) => { | |
| 1057 | + const d = JSON.parse(e.detail); | |
| 1058 | + return e.actor_login === OWNER && d.enabled === true && d.agents.includes("olive-bot") && d.method === "squash"; | |
| 1059 | + }), | |
| 1060 | + ); | |
| 1061 | + // An unchanged save doesn't audit again; the stored form (ids and logins) round-trips. | |
| 1062 | + await ok("PUT", `${R}/branches/main/protection`, saved.rules); | |
| 1063 | + await sleep(1000); | |
| 1064 | + assert.equal(auditLog("repo.protection.agent_auto_merge_rule").filter((e) => e.target === `${OWNER}/ruled`).length, 1); | |
| 1065 | + assert.equal((await pull(early.number)).auto_merge, null); | |
| 1066 | + | |
| 1067 | + // ------------------------------------------------------------ a named agent's new pull request lands on green checks, with nobody acting | |
| 1068 | + const one = await opened("agent-one", T.bot); | |
| 1069 | + const on = await until("auto-merge turned on by the rule", async () => (await pull(one.number)).auto_merge); | |
| 1070 | + assert.equal(on.enabled_by.login, "olive-bot", "it merges as the agent"); | |
| 1071 | + assert.equal(on.merge_method, "squash"); | |
| 1072 | + await sleep(1000); | |
| 1073 | + assert.equal((await pull(one.number)).merged, false, "the required check still gates it"); | |
| 1074 | + await green(one.head); | |
| 1075 | + const merged = await until("merged by the rule", async () => { | |
| 1076 | + const x = await pull(one.number); | |
| 1077 | + return x.merged ? x : null; | |
| 1078 | + }); | |
| 1079 | + assert.equal(merged.merged_by.login, "olive-bot"); | |
| 1080 | + const enabledAudit = auditLog("pull_request.auto_merge_enabled").find((e) => e.target === `${OWNER}/ruled#${one.number}`); | |
| 1081 | + assert.equal(enabledAudit.actor_login, "diffinery-auto-merge[bot]"); | |
| 1082 | + assert.equal(JSON.parse(enabledAudit.detail).byRule, "main"); | |
| 1083 | + assert.equal(JSON.parse(enabledAudit.detail).agent, "olive-bot"); | |
| 1084 | + assert.ok(auditLog("pull_request.auto_merged").some((e) => e.target === `${OWNER}/ruled#${one.number}` && e.actor_login === "olive-bot")); | |
| 1085 | + const from = merged.merge_commit_sha; | |
| 1086 | + | |
| 1087 | + // ------------------------------------------------------------ the agent pushing to its older pull request picks it up | |
| 1088 | + await green(await pushed(early, T.bot)); | |
| 1089 | + await until("early pull request merged after the agent pushed", async () => (await pull(early.number)).merged); | |
| 1090 | + | |
| 1091 | + // ------------------------------------------------------------ a workflow change still needs a person, and the pull request says so | |
| 1092 | + const wf = await opened("agent-workflow", T.bot, { from, path: ".github/workflows/ci.yml" }); | |
| 1093 | + await until("workflow comment", async () => (await said(wf.number)).some((b) => /Auto-merge didn't turn on by itself/.test(b) && /workflow file/.test(b))); | |
| 1094 | + await green(wf.head); | |
| 1095 | + await sleep(1000); | |
| 1096 | + const held = await pull(wf.number); | |
| 1097 | + assert.equal(held.auto_merge, null); | |
| 1098 | + assert.equal(held.merged, false); | |
| 1099 | + | |
| 1100 | + // ------------------------------------------------------------ a person's requested changes still block it | |
| 1101 | + const objected = await opened("agent-objected", T.bot, { from }); | |
| 1102 | + await until("objected auto-merge on", async () => (await pull(objected.number)).auto_merge); | |
| 1103 | + await ok("POST", `${R}/pulls/${objected.number}/reviews`, { event: "REQUEST_CHANGES", body: "not like this" }); | |
| 1104 | + await green(objected.head); | |
| 1105 | + await sleep(1500); | |
| 1106 | + assert.equal((await pull(objected.number)).merged, false); | |
| 1107 | + | |
| 1108 | + // ------------------------------------------------------------ other agents, people and drafts are left alone until marked ready | |
| 1109 | + const other = await opened("helper-change", T.helper, { from }); | |
| 1110 | + const mine = await opened("person-change", undefined, { from }); | |
| 1111 | + const draft = await opened("agent-draft", T.bot, { from, draft: true }); | |
| 1112 | + await sleep(1500); | |
| 1113 | + for (const n of [other.number, mine.number, draft.number]) assert.equal((await pull(n)).auto_merge, null, `#${n}`); | |
| 1114 | + await ok("PATCH", `${R}/pulls/${draft.number}`, { draft: false }); | |
| 1115 | + await until("draft marked ready turns it on", async () => (await pull(draft.number)).auto_merge); | |
| 1116 | + | |
| 1117 | + // ------------------------------------------------------------ once a person turns it off, the rule never turns it back on | |
| 1118 | + const stopped = await opened("agent-stopped", T.bot, { from }); | |
| 1119 | + await until("stopped auto-merge on", async () => (await pull(stopped.number)).auto_merge); | |
| 1120 | + await status(204, "DELETE", `${R}/pulls/${stopped.number}/auto-merge`); | |
| 1121 | + await green(await pushed(stopped, T.bot)); | |
| 1122 | + await sleep(1500); | |
| 1123 | + const still = await pull(stopped.number); | |
| 1124 | + assert.equal(still.auto_merge, null); | |
| 1125 | + assert.equal(still.merged, false); | |
| 1126 | + // And a person's push to an agent's pull request turns it off for good too. | |
| 1127 | + const touched = await opened("agent-touched", T.bot, { from }); | |
| 1128 | + await until("touched auto-merge on", async () => (await pull(touched.number)).auto_merge); | |
| 1129 | + await pushed(touched, T.olive); | |
| 1130 | + await until("a person's push turns it off", async () => (await pull(touched.number)).auto_merge === null); | |
| 1131 | + await ok("PUT", `${R}/contents/${touched.branch}-3.txt`, { message: "agent again", content: b64("x\n"), branch: touched.branch }, T.bot); | |
| 1132 | + await sleep(1500); | |
| 1133 | + assert.equal((await pull(touched.number)).auto_merge, null); | |
| 1134 | + | |
| 1135 | + // ------------------------------------------------------------ the settings page shows it; turning allowAgentAutoMerge off removes it | |
| 1136 | + const settings = await (await fetch(`${base}/${OWNER}/ruled/settings/branches`, { headers: { cookie: `__Host-dfy_session=${S.olive}` } })).text(); | |
| 1137 | + assert.match(settings, /Auto-merge pull requests from these agents/); | |
| 1138 | + assert.match(settings, /auto-merges olive-bot/); | |
| 1139 | + assert.doesNotMatch(settings, /<select|type="checkbox"/); | |
| 1140 | + await ok("PUT", `${R}/branches/main/protection`, { requirePullRequest: true, requiredChecks: ["ci"], requiredApprovals: 1, autoMergeAgents: [] }); | |
| 1141 | + const off = await ok("GET", `${R}/branches/main/protection`); | |
| 1142 | + assert.equal(off.rules.autoMergeAgents, undefined); | |
| 1143 | + await until("rule off audit", async () => auditLog("repo.protection.agent_auto_merge_rule").some((e) => JSON.parse(e.detail).enabled === false)); | |
| 1144 | + const after = await opened("agent-after", T.bot, { from }); | |
| 1145 | + await sleep(1500); | |
| 1146 | + assert.equal((await pull(after.number)).auto_merge, null); | |
| 1147 | +}); | |
| 1148 | + | |
| 1023 | 1149 | test("security: an archived repository lands nothing by itself: archiving ends auto-merge and empties the merge queue", async () => { |
| 1024 | 1150 | const R = `/repos/${OWNER}/shelved`; |
| 1025 | 1151 | const mainSha = await repoWithReadme("shelved"); |
modifiedapps/diffinery/test/unit/auto-merge.test.ts+94−0View file
| @@ -211,3 +211,97 @@ | ||
| 211 | 211 | expect(kept.enqueues[0]).toMatchObject({ deleteBranch: false }); |
| 212 | 212 | }); |
| 213 | 213 | }); |
| 214 | + | |
| 215 | +// ---------------------------------------------------------------- a branch rule's autoMergeAgents | |
| 216 | + | |
| 217 | +/** Pull request #1 by agent a1 into main, with auto-merge off, and a rule for main; the agent has the write role. */ | |
| 218 | +function ruleHarness(o: { rules?: ProtectionRules; authorKind?: string; draft?: boolean; workflow?: string; role?: "read" | "write" } = {}) { | |
| 219 | + const sql = sqlStorage(); | |
| 220 | + const at = new Date().toISOString(); | |
| 221 | + sql.exec("INSERT INTO issues (number, kind, title, author_id, author_login, author_kind, created_at, updated_at) VALUES (1, 'pull', 'T', 'a1', 'olive-bot', ?, ?, ?)", o.authorKind ?? "agent", at, at); | |
| 222 | + sql.exec("INSERT INTO pulls (number, head_ref, head_sha, base_ref, draft) VALUES (1, 'topic', 'h1', 'main', ?)", o.draft ? 1 : 0); | |
| 223 | + const rules: ProtectionRules = o.rules ?? { requiredChecks: ["ci"], allowAgentAutoMerge: true, autoMergeAgents: [{ id: "a1", login: "olive-bot" }], autoMergeMethod: "squash" }; | |
| 224 | + const comments: Array<{ body: string; by: string }> = []; | |
| 225 | + const events: Array<{ type: string; action?: string; actor?: { login: string }; detail?: Record<string, unknown> }> = []; | |
| 226 | + const repo = { | |
| 227 | + sql, | |
| 228 | + meta: { repoId: "r1", ownerId: "u1", owner: "olive", name: "n", defaultBranch: "main", ownerKind: "user", archived: false }, | |
| 229 | + isArchived: async () => false, | |
| 230 | + protectionFor: () => rules, | |
| 231 | + protectionRuleFor: () => ({ pattern: "main", rules }), | |
| 232 | + collab: { | |
| 233 | + get: () => ({ issue: { number: 1, state: "open", pull: { merged: false, draft: !!o.draft, base: "main", head: "topic", headSha: "h1" } } }), | |
| 234 | + mergeability: async () => ({ mergeable: false, blockers: ["required check 'ci' has not run"] }), | |
| 235 | + addComment: (_n: number, body: string, by: { login: string }) => comments.push({ body, by: by.login }), | |
| 236 | + changedWorkflow: async () => o.workflow ?? null, | |
| 237 | + }, | |
| 238 | + queue: { isQueued: () => false }, | |
| 239 | + social: { | |
| 240 | + record: (n: number, event: string, actor: { id: string; login: string; kind: string }) => | |
| 241 | + sql.exec("INSERT INTO issue_events (number, event, actor_id, actor_login, actor_kind, detail, created_at) VALUES (?, ?, ?, ?, ?, '{}', ?)", n, event, actor.id, actor.login, actor.kind, at), | |
| 242 | + }, | |
| 243 | + emit: (e: (typeof events)[number]) => events.push(e), | |
| 244 | + broadcast: () => undefined, | |
| 245 | + background: () => undefined, | |
| 246 | + }; | |
| 247 | + const am = new AutoMerge(repo as unknown as RepoDO, {} as Env); | |
| 248 | + // The agent's access, as the directory would give it. | |
| 249 | + (am as unknown as { accountCaller: (id: string) => Promise<unknown> }).accountCaller = async (id: string) => (id === "a1" ? { id: "a1", login: "olive-bot", kind: "agent", role: o.role ?? "write" } : null); | |
| 250 | + return { am, comments, events, sql }; | |
| 251 | +} | |
| 252 | + | |
| 253 | +describe("a branch rule that auto-merges chosen agents' pull requests", () => { | |
| 254 | + it("turns auto-merge on as the agent, recorded as the server's doing", async () => { | |
| 255 | + const h = ruleHarness(); | |
| 256 | + await h.am.applyRule(1, true); | |
| 257 | + const s = h.am.state(1); | |
| 258 | + expect(s).toMatchObject({ method: "squash", byRule: "main", enabledBy: { id: "a1", login: "olive-bot", kind: "agent" }, role: "write" }); | |
| 259 | + const audit = h.events.find((e) => e.action === "pull_request.auto_merge_enabled"); | |
| 260 | + expect(audit?.actor?.login).toBe(AUTO_MERGE_BOT.login); | |
| 261 | + expect(audit?.detail).toMatchObject({ byRule: "main", agent: "olive-bot", viaAgentRule: true }); | |
| 262 | + expect(AutoMerge.info(s)?.byRule).toBe("main"); | |
| 263 | + }); | |
| 264 | + | |
| 265 | + it("leaves other agents, people, drafts and rules without allowAgentAutoMerge alone", async () => { | |
| 266 | + for (const h of [ | |
| 267 | + ruleHarness({ rules: { requiredChecks: ["ci"], allowAgentAutoMerge: true, autoMergeAgents: [{ id: "a2", login: "other-bot" }] } }), | |
| 268 | + ruleHarness({ authorKind: "user" }), | |
| 269 | + ruleHarness({ draft: true }), | |
| 270 | + ruleHarness({ rules: { requiredChecks: ["ci"], autoMergeAgents: [{ id: "a1", login: "olive-bot" }] } }), | |
| 271 | + ruleHarness({ rules: { requiredChecks: ["ci"], allowAgentAutoMerge: true } }), | |
| 272 | + ]) { | |
| 273 | + await h.am.applyRule(1, true); | |
| 274 | + expect(h.am.state(1)).toBeNull(); | |
| 275 | + expect(h.comments).toHaveLength(0); | |
| 276 | + } | |
| 277 | + }); | |
| 278 | + | |
| 279 | + it("never turns it back on once it was turned off", async () => { | |
| 280 | + const h = ruleHarness(); | |
| 281 | + await h.am.applyRule(1, false); | |
| 282 | + h.am.disable(1, { id: "u1", login: "olive", kind: "user", role: "admin" }); | |
| 283 | + expect(h.am.state(1)).toBeNull(); | |
| 284 | + await h.am.applyRule(1, true); | |
| 285 | + expect(h.am.state(1)).toBeNull(); | |
| 286 | + }); | |
| 287 | + | |
| 288 | + it("keeps every refusal of the agent's own auto-merge, and says why when asked to", async () => { | |
| 289 | + const wf = ruleHarness({ workflow: ".github/workflows/ci.yml" }); | |
| 290 | + await wf.am.applyRule(1, true); | |
| 291 | + expect(wf.am.state(1)).toBeNull(); | |
| 292 | + expect(wf.comments[0]?.body).toMatch(/^Auto-merge didn't turn on by itself \(branch rule `main` auto-merges olive-bot's pull requests\): the pull request changes workflow file `\.github\/workflows\/ci\.yml`/); | |
| 293 | + const quiet = ruleHarness({ workflow: ".github/workflows/ci.yml" }); | |
| 294 | + await quiet.am.applyRule(1, false); | |
| 295 | + expect(quiet.comments).toHaveLength(0); | |
| 296 | + // No required check: allowAgentAutoMerge refuses, so the rule does too. | |
| 297 | + const unchecked = ruleHarness({ rules: { allowAgentAutoMerge: true, autoMergeAgents: [{ id: "a1", login: "olive-bot" }] } }); | |
| 298 | + await unchecked.am.applyRule(1, true); | |
| 299 | + expect(unchecked.am.state(1)).toBeNull(); | |
| 300 | + expect(unchecked.comments[0]?.body).toMatch(/no required status checks/); | |
| 301 | + // An agent that lost write access. | |
| 302 | + const reader = ruleHarness({ role: "read" }); | |
| 303 | + await reader.am.applyRule(1, true); | |
| 304 | + expect(reader.am.state(1)).toBeNull(); | |
| 305 | + expect(reader.comments[0]?.body).toMatch(/write access is required/); | |
| 306 | + }); | |
| 307 | +}); |