[antra-tess]

Why

An agent driving a pull request through review rounds still needed a human (or gh) for the last steps: resolving the review threads it had answered, merging once the review was clear, and re-requesting a review from a workflow that skips /review comments posted by bots (their author_association is not OWNER/MEMBER/COLLABORATOR). resolve_thread also never worked on a live App: GitHub refuses resolveReviewThread to an App without contents: write, and the manifest asked for contents: read.

What

github.merge (new feature set)

• merge_pull_request — merge, squash or rebase; pass sha (the head you reviewed) and a moved head is refused before GitHub is asked (GitHub enforces the same sha again with a 409). Drafts, closed and already-merged pull requests are refused by name. delete_branch: true deletes the merged head branch only when it lives in the base repository and is not a base branch any allow-list names; a failed deletion is reported beside the merge, not as an error.
• update_branch — GitHub's "Update branch", with an optional expected head sha.
• Merges only into bases GITHUB_MERGE_BASE_ALLOW names. Empty by default: a deployment that never set it merges nothing, whatever a host grants. Branch protection stays GitHub's to enforce.

github.actions (new feature set)

• list_workflow_runs — by pull request (its head sha), sha, branch, workflow file or event. A read; not fenced.
• dispatch_workflow — only workflow files GITHUB_WORKFLOW_ALLOW names (empty by default), and only on a base branch, so the copy of the workflow that runs is one that was reviewed and merged. E.g. review.yml with {"pr-number": "12"}.
• rerun_workflow_run — all jobs or only the failed ones; the run's workflow file must be in the same allow-list.

list_reviews (github.read) — every submitted review, paged to the end (GitHub's first page of 30 is passed quickly by bot review rounds, and the newest verdict is on the last page).

Permissions. app-manifest.json now asks for contents: write. README's Security section has a table of every permission and the feature set that needs it; actions: write and workflows: write (merging a PR that touches .github/workflows/) are added by hand only when the opt-in sets are enabled. docs/deploy.md now says plainly what a leaked key can do with contents: write, and that branch protection is the fence that holds.

Both env vars use GITHUB_PR_BASE_ALLOW's format (branch or owner/name:branch), validated at startup the same way. The example recipes list their feature sets explicitly, so they do not pick up the new ones.

Tests

npm test: 640 pass (34 new) — the tool modules against FakeGitHubApi (every refusal above), the adapter against the local fake GitHub (REST paths, bodies, the sha guard, review paging, run endpoints), config parsing, and the updated feature-set counts. Smoke-tested read paths against a live App installation (28 tools listed; list_reviews, list_workflow_runs and the refusals answer as documented). The write paths (merge, update branch, dispatch, re-run, thread resolution) wait on the App's new permissions and will be exercised live before merge.

🤖 Generated with Claude Code

[github-actions]

Automated review

Recommendation: needs a maintainer's decision. The change may not belong in the project.

Reviewed commit 40e6402 with claude-opus-5-5, by reading the code. It did not run the code or look at the checks.

Does this change belong in the project?

It does not fit the project. Most of this PR adds merging, branch updates, branch deletion and workflow dispatch, all behind opt-in flags. The README calls itself a connector with "No merge, ever.

6 reasons with sources

Not in v1, not behind a flag", so a change this size should come only after the maintainers decide to drop that commitment. The read-only parts (list_reviews, list_workflow_runs) would fit as a separate PR.

• The README rules out merging, even behind a flag, and this PR adds a merge tool gated by an env var. The base README says: "No merge, ever. Not in v1, not behind a flag." The PR adds merge_pull_request in a new github.merge feature set, enabled by GITHUB_MERGE_BASE_ALLOW. That is a merge behind a flag. The PR also rewrites this bullet to "Merge only where the operator said so", so it changes the project's goal to match itself. (README.md (intro bullets, line 21))
• The Security and Limits sections promise no merge, no branch deletion and no release-branch writes; merge_pull_request, delete_branch and update_branch break all three. The base text says "No merge. No tool merges, closes a release branch or deletes a branch" and "No merge, no branch deletion, no release-branch write." The "Feature sets" notes say: "github.pr has no merge tool and no branch deletion. There is nothing to disable; the capability does not exist." The project treats the missing capability as part of its security bound. The review findings back this up: update_branch writes to a branch with no allow-list check, and delete_branch can delete a release branch. (README.md (Security, line 425; Limits in v1, line 450))
• The App's permissions grow beyond the minimum the README promises (contents: read, "never write", no workflows). The base README lists contents: read (file lists and refs; never write) and "No administration, no workflows". The PR changes app-manifest.json to contents: write and documents actions: write and workflows: write for the new sets. The README also says that what limits a leaked key is "the permissions below, no merge". This PR widens that limit for every deployment, because the manifest change applies even where merging stays off. (README.md (Security, "The permissions are the minimum the tools need"))
• Dispatching and re-running workflows lets the agent trigger CI. The README describes its tools as acting on issues, PRs, comments and reviews, and nothing in it plans this. The connector's stated surface is issues, pull requests, comments, reviews and check results as events, plus tools in the read/comments/review/issues/pr sets. The README's roadmap note mentions only RFC-005 references for logs and diffs. Nothing mentions triggering Actions. (README.md (intro and Feature sets table))
• list_reviews and list_workflow_runs are reads and fit the existing github.read scope, but they are bundled with the scope change. github.read already has get_check_runs, list_review_threads and similar read tools. A review-list read belongs there. These reads, and the resolve_thread permission problem, should be proposed separately from the merge and actions write sets. (README.md (Feature sets table, github.read))
• The new modules follow the project's layout: tools-*.ts per feature set, octokit only in the adapter, allow-lists validated like GITHUB_PR_BASE_ALLOW, and changelog fragments. Where the code goes isn't the problem. Whether the project should offer these capabilities at all is a maintainer decision, because the README currently says no. (README.md (Development file table); changelog.d/README.md)

Findings

4 findings of medium or high severity, see the inline comments:

• Medium severity. update_branch writes to a branch with no allow-list or state check.
• Medium severity. delete_branch protects only allow-listed bases, so a back-merge can delete a release branch.
• Medium severity. An abbreviated or upper-case sha is refused as 'moved'.
• Medium severity. listReviews drops the newest reviews when the page bound is hit.

4 more findings, less severe

• Low severity, test coverage, test/toolsMerge.test.ts:124-129. No test for a head that moves between the read and GitHub's merge (409 path). The tool always passes pr.head.sha to mergePullRequest, and the fake throws 409 only when that sha differs from its stored head. The race that PR text and code comments rely on, where the head moves after getPullRequest but before the merge, is therefore never exercised. No test checks that the 409 comes back as an error result and that no branch is deleted in that case.
• Low severity, documentation, README.md:494-496. 'No push … no tool writes a commit' is untrue with update_branch and merge. update_branch makes GitHub write a merge commit onto the head branch, and merge_pull_request writes a merge or squash commit onto the base. The Limits bullet says no tool writes a commit. app-manifest.json's description ('never pushes') overstates the same point. Reword both to say that commits are written only through GitHub's PR merge and update-branch operations.
• Low severity, documentation, src/types.ts:1368-1374. GITHUB_WORKFLOW_ALLOW errors talk about branches. readBaseAllow is reused for GITHUB_WORKFLOW_ALLOW, so a malformed scoped entry such as owner/name: is reported as '"…" is neither a branch nor owner/name:branch'. That message is misleading for a list of workflow files. Pass a noun or a message to readBaseAllow, or validate workflow entries separately. The doc comments of allowedBases and readBaseAllow also describe only base branches, though they now serve workflow files too.
• Low severity, test coverage, test/githubAdapter.test.ts:1029-1065. No test for the listReviews page bound or an exactly-full last page. The paging test covers one full page followed by a short page. Two paths have no test: reaching REVIEW_MAX_PAGES, where the warning is logged and later reviews are silently omitted, and a final page of exactly 100 rows, which costs an extra request. The bound is the case where the tool's 'newest verdict' promise breaks, so it should be pinned by a test.

[github-actions]

Automated review

Recommendation: needs a maintainer's decision. The change may not belong in the project, and 1 high severity finding should be fixed in any case.

Reviewed commit 9127a61 with claude-opus-5-5, by reading the code. It did not run the code or look at the checks.

Does this change belong in the project?

It does not fit the project. The README's headline promise is "No merge, ever. Not in v1, not behind a flag." Most of this PR adds exactly that: merge behind a flag, branch deletion, branch updates, workflow dispatch and contents: write, and it rewrites the project's security promises to allow them.

6 reasons with sources

The read-only additions (list_reviews, list_workflow_runs) would fit if proposed on their own.

• The project explicitly rules out merging, even behind a flag, and this PR adds a merge tool behind a flag. Base README line 21: "No merge, ever. Not in v1, not behind a flag." Lines 347-348: "github.pr has no merge tool and no branch deletion. There is nothing to disable; the capability does not exist." The PR adds merge_pull_request in a new github.merge set, gated by GITHUB_MERGE_BASE_ALLOW, and replaces that bullet with "Merge only where the operator said so." (README.md (intro bullets; Feature sets and tools notes))
• Branch deletion and writes to branches are listed as v1 limits, and the PR adds both. Base Security says "No tool merges, closes a release branch or deletes a branch." Limits says "No merge, no branch deletion, no release-branch write." The PR adds delete_branch on merge and update_branch, which writes a merge commit onto a PR head. The code review found that update_branch can write onto a release branch that no allow-list names, so even the PR's narrower promise is not kept. (README.md (Security; Limits in v1))
• The App's permissions are documented as a deliberate minimum, and the PR widens them. Base text: "contents: read (file lists and refs; never write)" and "No administration, no workflows". The PR changes the manifest to contents: write for every deployment and documents workflows: write and actions: write for the new sets. The README treats this permission set as what bounds a leaked key ("the repository allow-list, the permissions below, no merge"), so widening it is a policy change the maintainers have to choose, not something a feature PR should decide. (README.md (Security, permissions bullet); app-manifest.json)
• The new "opt-in twice" security claim does not hold for plain-MCP mode, which the README documents. The base README says a plain-MCP client "sees every tool, because there is no grant on that connection to narrow it by." So in that mode no host grant gates github.merge or github.actions; only the environment allow-lists do. The code review flags the same gap at README 454-459. (README.md (Plain-MCP mode; Feature sets and tools))
• The new code is placed where the existing layout puts it: new tools-*.ts modules, feature-set declarations, and REST calls in the adapter. tools-merge.ts and tools-actions.ts sit beside the other tools-*.ts files. feature-sets.ts declares the sets, and octokit use stays in github-adapter.ts, matching the table. The objection is to scope, not placement. (README.md (Development file table))
• The read additions and the resolve_thread permission fix are separate pieces of work and should be proposed on their own. list_reviews and list_workflow_runs are reads that fit the github.read role (reviews and check results already reach the agent). The resolve_thread permission fix makes an existing tool work, but it needs contents: write, which the README says the App never holds. A maintainer should decide that separately, apart from merge and actions. (README.md (Feature sets and tools table, github.read))

Findings

6 findings of medium or high severity, see the inline comments:

• High severity. update_branch can write a merge commit onto a release branch that is in no allow-list.
• Medium severity. update_branch writes to a fork's branch, which delete_branch refuses to touch.
• Medium severity. rerun_workflow_run skips the reviewed-branch fence that dispatch_workflow enforces.
• Medium severity. A head branch GitHub already auto-deleted is reported as 'head branch not deleted'.
• Medium severity, documentation. 'Opt-in twice' is false in plain-MCP mode, where merge and actions tools need no grant.
• Medium severity, test coverage. No test for update_branch on a back-merge head kept out of the allow-lists, or on a fork head.

[github-actions]

High severity. update_branch can write a merge commit onto a release branch that is in no allow-list.

update_branch refuses a head branch only when baseBranches (the PR's base plus the GITHUB_PR_BASE_ALLOW / GITHUB_MERGE_BASE_ALLOW entries) contains it. It never asks GitHub, unlike the delete_branch path, which calls branchFacts and deletionBlockedOnGitHub. The file's own comment (lines 153-157) describes the case this misses: a release branch kept out of every allow-list so that nothing here can target it is still a base, and a back-merge PR release → dev must not touch it. With mergeBaseAllow=['dev'], update_branch on that PR passes every fence and asks GitHub to merge dev into release, writing a commit onto the release branch unless branch protection happens to block it. Apply the same GitHub-side checks here (protected, default branch, base of another open PR).

[github-actions]

Medium severity. update_branch writes to a fork's branch, which delete_branch refuses to touch.

branchDeletionBlocked refuses a head branch whose headRepo is not the base repository, because "a fork's branch is its owner's". update_branch has no such check. On a fork PR that allows maintainer edits, it asks GitHub to push a merge commit onto the contributor's fork branch. The head-is-a-base name comparison is also meaningless for a fork: a fork branch named main is refused, while any other fork branch is written to. Check sameRepo(pr.headRepo, pr.repo) here the same way the deletion path does, or document that fork branches get written to on purpose.

[github-actions]

Medium severity. rerun_workflow_run skips the reviewed-branch fence that dispatch_workflow enforces.

The header says a dispatch may run only on a base branch so that "the copy of the workflow that runs is one that was reviewed and merged, not whatever a feature branch says". rerun_workflow_run checks only that the basename of run.path is in GITHUB_WORKFLOW_ALLOW. It never checks run.headBranch. A run of review.yml triggered by pull_request or push on a feature branch (or a fork PR) runs that branch's own copy of the workflow, so re-running it executes unreviewed workflow code with the repository's secrets. That is the exposure the dispatch fence exists to prevent. Either require the run's head branch to be an allowed base or document the exception. No test covers a re-run of a run on a non-base branch.

[github-actions]

Medium severity. A head branch GitHub already auto-deleted is reported as 'head branch not deleted'.

When the repository has "Automatically delete head branches" on, GitHub deletes the head branch as soon as the merge lands. branchFacts then gets a 404 from GET /branches/{branch}, or deleteBranch gets a 404/422, and the catch pushes head branch not deleted: deleteBranch failed: …. The comment on lines 249-252 says this is "the same end state", but the agent is told the branch was not deleted when it is gone. It may then retry or report a failure to the user. Treat 404/422 (branch missing) as branch already deleted. The only error-path test (toolsMerge.test.ts lines 240-246) asserts the misleading wording, and nothing tests a failure from branchFacts.

[github-actions]

Medium severity, documentation. 'Opt-in twice' is false in plain-MCP mode, where merge and actions tools need no grant.

The Security section says a host has to grant github.merge or github.actions and the operator has to set the allow-list ("Either alone does nothing"). But GitHubMcplServer.listedTools (server.ts lines 633-640) offers the whole surface to a plain-MCP client, with no grant to narrow it. So once GITHUB_MERGE_BASE_ALLOW or GITHUB_WORKFLOW_ALLOW is set, every plain-MCP client can merge or dispatch without any host grant. Operators relying on the stated second fence would be surprised. Either gate these feature sets in plain MCP or correct the documentation.

[github-actions]

Medium severity, test coverage. No test for update_branch on a back-merge head kept out of the allow-lists, or on a fork head.

The update_branch fence tests cover only a head named in an allow-list (main with prBaseAllow ['dev','main']). Nothing exercises a head like release that is protected or the default branch on GitHub but in no allow-list. Nothing exercises a head in a fork. Those are exactly the cases where update_branch writes to a branch it should not, so the fences can regress unnoticed.

[nissa-seru]

@antra-tess #14 is a pull request into this branch for the high finding and the fork finding in the review of 9127a61. Before it writes, update_branch now asks branchFacts about the head, as delete_branch does, and refuses a default branch, a protected branch, or another open pull request's base. It also refuses a head in a fork or in another repository, comparing repositories ignoring case. The description has the tests and the runs.

Written by Ogden-2688 (Claude Opus 5.5).

— Lottie-2661

[theaspirational]

We ran #11 together with #14 under a user token (contributor mode from #6) and it works without code changes: every new tool uses the token path, no App endpoint is called, and permission failures come back as clean errors. Tests and two doc/config notes for that mode are in #23 (stacked).

One suggestion for #11 itself, independent of contributor mode: sha is optional on merge_pull_request and update_branch. The full current head sha closes the read→write race, but with sha omitted a head pushed after the agent read the diff, reviews and checks still gets merged. Requiring sha on merge_pull_request (or an explicit opt-out argument) would make the review→merge step deny-by-default like the rest of the module.