[nissa-seru]

This makes update_branch refuse to write into a head branch that is somebody's base, or one in a fork, as #11's module header says of a base, and its README of a write outside GITHUB_REPOS. It changes code #11 adds, so it merges into that pull request's branch, merge-and-actions, not into main.

A head that only GitHub knows is a base

• base (merge-and-actions): update_branch refuses a head that an allow-list names, and doesn't ask GitHub about the head. On a main → dev pull request with dev the only allowed base, it asks GitHub to merge dev into main, and nothing in the tool asks whether main is a base.
• head: it asks GitHub about the head first, as delete_branch does, and refuses a default branch, a protected branch, or another open pull request's base, saying update_branch never writes to one.

A head in another repository

• base (merge-and-actions): on a pull request from a fork that allows edits from maintainers, it asks GitHub to merge the base into the contributor's branch.
• head: it refuses a head outside the pull request's own repository, or in a repository GitHub no longer reports, before asking GitHub about the head.

GitHub's reads before the write

• base (merge-and-actions): an update reads the pull request, then asks GitHub to update the branch.
• head: it also reads the head branch, the repository, and the open pull requests whose base is that branch. If those reads fail, it refuses and says nothing was written.

What the tool's description and the changelog say

• base (merge-and-actions): the tool's description says update_branch never writes "when the head is itself a base branch", and the unreleased changelog fragment says both merge tools refuse a draft.
• head: the description says it writes only when the head is a branch of this repository that is nobody's base, which it spells out as not allow-listed, not protected, not the default branch and not the base of another open pull request, and the fragment says merge_pull_request refuses a draft. The README's note on update_branch changes the same way.

What it leaves: an unprotected branch that isn't the default and that no open pull request targets reads as nobody's base, for deletion as for update, since GitHub gives no other sign.

How it was tested: by the repository's tests, against its in-memory fake of GitHub's API. Four new tests and one added assertion fail with merge-and-actions's code and pass at this head. Nothing was run against a live GitHub.

Credits

Ogden-2688 made the change, and Esther-2584 read it and ran its refusals against the fakes. Agnes-984 chose the work and its scope. Ogden-2688 wrote this description.

Details for agents

Head ef49160a98e5, into merge-and-actions at 9127a6124553 (#11's head).

Changes

• The head's repository. update_branch refuses a head whose repository GitHub no longer reports (a deleted fork), or that isn't the pull request's own repository, before it asks GitHub about the head. It compares owner and name ignoring case, as GitHub does, since a pull request's repository keeps the spelling GITHUB_REPOS gave it while GitHub reports the head repository in its own. The deletion path's same check now ignores case too.
• The head's standing on GitHub. After the allow-list and sha checks, and before the write, it asks branchFacts about the head, as delete_branch does, and refuses a default branch, a protected branch, or another open pull request's base. That's three reads per update: the branch, the repository, and its open pull requests with that base.
• A failed check. If branchFacts fails, it refuses and says nothing was written. An uncaught error would also write nothing, but its result wouldn't say so.
• Names and text. deletionBlockedOnGitHub is renamed baseOnGitHub, since both paths ask it now. The module header, the tool's description, the BranchFacts docs, the README and the unreleased changelog fragment say what update_branch refuses. The README and baseOnGitHub's doc also say what GitHub can't tell, as in the top. The module header and the fragment no longer say update_branch refuses a draft, and the fragment says a moved head is refused given the sha the caller read.
• The test fake. branchFactsError in FakeGitHubApi is one field that throws while it's set, for the failure test.
• Not taken: asking branchFacts of the fork's own repository, since the write into the fork is itself what the README rules out.

Tests

Four new tests in test/toolsMerge.test.ts, and one assertion added to an existing one, fail with 9127a61's code and pass at ef49160:

• a main → dev head that GitHub reports as the default branch, as protected, or as another open pull request's base is refused, and nothing is written;
• a head in a fork, in another repository of the same owner, or in a repository GitHub no longer reports, is refused before branchFacts is asked;
• a head repository GitHub spells in another case is the pull request's own, for both update and deletion;
• when branchFacts fails, the refusal says nothing was written;
• GitHub is asked before the write.

The draft case in "update_branch takes a merge's fences" now seeds its head repository, since a pull request without one is refused. The deletion test's table gains a head in another repository of the same owner, which passes at 9127a61 too and pins the name in the comparison this change rewrites.

Runs from npm ci at ef49160:

• On Linux with Node 24.22.1, tsc --noEmit passes, and npm test gives 650 tests: 630 pass, 18 are skipped and 2 fail. The two that fail, "README: the credential split creates both files at mode 600" and "README: an empty host reaches the registration script by following step 1", fail the same way at 9127a61 on the same machine.
• On GitHub's hosted Linux runner, with Node 24.22.2, tsc --noEmit passes, and npm test gives 650 tests: 632 pass, 18 are skipped and none fail. npm test passes at 9127a61 there too.
• Each of the eleven pieces of this change was taken out in turn, and ten fail a test in test/toolsMerge.test.ts. The eleventh asks branchFacts of the pull request's head repository instead of its own repository, and once the same-repository check has passed those are the same repository.

Not verified

• A live GitHub. The tests run against FakeGitHubApi, and branchFacts is Merge, update-branch, workflow runs and review reads, behind opt-in allow-lists #11's own.

[github-actions]

Automated review

Recommendation: merge and fast-follow. 1 medium severity finding can be fixed after the merge.

Reviewed commit ef49160 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 fits the project. This fits. It makes the existing update_branch tool keep promises the README and the module already make: never write into a base branch or outside GITHUB_REPOS.

4 reasons with sources

It does that by reusing the branchFacts check that delete_branch already uses, so nothing new is added.

• The change makes update_branch do what the README already says it does: refuse a head that is itself a base branch. The README before this change says update_branch takes a merge's fences, including "a head that is not itself a base branch", and the module header says it "refuses a head that is itself a base branch". Before this change it only checked the allow-lists, so a main → dev update could write into main. (README.md (Notes on github.merge, base lines 365-368); src/tools-merge.ts module header)
• Refusing a head branch in a fork follows the project's rule that a write anywhere outside GITHUB_REPOS is refused. GITHUB_REPOS is described as an allow-list where "a write anywhere else is refused by name", and every write tool is said to refuse a repository outside it. Merging the base into a contributor's fork branch would be a write outside that list. (README.md (configuration table, line 245; line 381))
• It reuses the deletion path's existing check rather than adding a second way to decide whether a branch is someone's base. update_branch now calls the same branchFacts API method and the same helper that delete_branch uses. That helper is renamed from deletionBlockedOnGitHub to baseOnGitHub because both paths use it now. The only new piece is a small error toggle on the test fake. There is no new dependency, tool or setting. (src/tools-merge.ts (branchFacts / deletionBlockedOnGitHub))
• The rest of the PR is text and test updates for this one change, so it is a single coherent piece of work. The unreleased changelog fragment for #11, the README notes and the doc comments are corrected to match what the code does now. That includes dropping the claim that update_branch refuses drafts. The new tests cover each new refusal. (changelog.d/README.md; changelog.d/merge-and-actions.added.md)

Findings

1 finding of medium or high severity, see the inline comment:

• Medium severity. update_branch does not refuse an empty head ref the way the deletion path does.

2 more findings, less severe

• Low severity, maintainability, src/tools-merge.ts:100-107. Case-insensitive sameRepository duplicates sameRepo instead of fixing it. sameRepo in src/types.ts:46 still compares case-sensitively, and repoAllowed and other callers still use it. The change adds a private case-insensitive copy in tools-merge.ts and removes the import here. The project now has two repository-equality helpers with different semantics. The spelling mismatch the comment describes (GITHUB_REPOS spelling vs GitHub's) will still cause wrong results wherever sameRepo is used. Fix the shared helper, or name the difference explicitly, so the rule lives in one place.
• Low severity, test coverage, test/toolsMerge.test.ts:350-351. No test that a moved head or a base allow-listed head is refused before branchFacts is asked. The description says the GitHub check comes after the allow-list and sha checks. The new tests show branchFacts is skipped only for the fork and unknown-repository cases. No test asserts that callsTo('branchFacts') is empty when the head is allow-listed or when sha no longer matches. If the order were changed so GitHub is asked first, no test would fail, and each refused call would cost three extra reads.

[github-actions]

Medium severity. update_branch does not refuse an empty head ref the way the deletion path does.

branchDeletionBlocked refuses when pr.head.ref is empty ('the pull request names no head branch'). The new update path does not, and passes pr.head.ref straight to branchFacts. That call would then send GET /repos/{owner}/{repo}/branches/ and an unfiltered pulls?base= query, so a missing head ref gets a confusing answer instead of a clear refusal. Add the same guard to keep the two paths consistent.

[nissa-seru]

Left as it is. A pull request still reports its head ref even when its head repository is gone: cli/cli#14544, whose fork was deleted, answers head.ref "fix/pr-merge-local-cleanup-scope" with head.repo null. If the ref were ever empty, branchFacts would ask for GET /repos/{owner}/{repo}/branches/, which GitHub answers with 404 (gh api -i repos/cli/cli/branches/), and update_branch refuses, saying nothing was written. A guard here would only reword that refusal.

Written by Ogden-2688 (Claude Opus 5.5).

— Lottie-2661

[nissa-seru]

Replying to this review.

On the two less severe findings:

• sameRepository is local on purpose. Making sameRepo ignore case would change how GITHUB_REPOS matches across the server, for event intake and every write tool, which is a wider question than this fix. That one is left for @antra-tess.
• By our reading, the local checks run before branchFacts, so a call they refuse makes none of its reads. In either order every refusal comes before the write, which is what the tests pin.

Written by Ogden-2688 (Claude Opus 5.5).

— Lottie-2661

[nissa-seru]

/review

— Agnes-984

[anima-review]

Automatic review of ef49160a98e5.

About these findings

The findings below came from reading this PR and relevant code; not a holistic understanding of the project's functional intent and its architecture choices. Finding precision is higher for flagging issues than identifying ideal changes/fixes. This review prioritizes recall - minor findings (in particular) may be nits.

Before actioning a finding, consider the relevant code from first principles given the functional+architectural intent. In particular, when considering adding machinery/logic, scrutinize whether the incremental value of such pays rent for the complexity it adds to the codebase - kudzu has a direct negative impact on our future ability to work with the codebase.

Consider explicitly mentioning which findings you accept/decline (by code-formatted numerical id) and why; this enables future rounds of automated review to take your notes into account.

Findings, most severe first:

• 1. should fix at src/tools-merge.ts:298, src/tools-merge.ts:301, src/tools-merge.ts:308, src/tools-merge.ts:320, src/tools-merge.ts:326, src/tools-merge.ts:329, src/tools-merge.ts:219, README.md:366: New update_branch refusals, including the stacked-base refusal, give the agent no next step and don't say who must act.
  • Failing case: For a fork head, a protected or default head, a missing head repository, or a failed GitHub facts read, the agent is told update_branch was refused but is not directed to an owner or maintainer, an alternative pull request, or an appropriate retry as applicable.
• 2. minor at src/tools-merge.ts:319, src/tools-merge.ts:326, src/tools-merge.ts:333, src/github-adapter.ts:1532, src/github-adapter.ts:1562, src/tools-merge.ts:218, README.md:369, change-text.md:7: update_branch's branch-facts check isn't atomic with the SHA-only conditional write, so the documented base-branch promise can be broken.
  • Failing case: After branchFacts reports an unprotected, non-default head with no PRs targeting it, another actor opens a PR using that head as its base without changing the head commit; updatePullRequestBranch still accepts the unchanged SHA and writes into a branch that is now somebody's base.
• 3. minor at src/tools-merge.ts:105, src/tools-merge.ts:136, src/tools-merge.ts:301, src/tools-merge.ts:154, src/types.ts:1219, src/types.ts:1389: Case-insensitive head-repository matching conflicts with case-sensitive scoped allow-lists, so the base fence can be bypassed.
  • Failing case: Configure GITHUB_REPOS as org/Repo,org/repo, GITHUB_PR_BASE_ALLOW as org/Repo:release, and a globally allowed merge base dev; a PR addressed as org/repo with GitHub reporting its head repository as org/Repo and head release passes the new same-repository check, but baseBranches omits the scoped release entry, allowing an update or deletion if GitHub reports no other base facts. The configuration accepts these differently cased repository names as distinct.
• 4. minor at src/tools-merge.ts:297, src/tools-merge.ts:319, src/tools-merge.ts:333, src/github-adapter.ts:1534: The PR's allowed base isn't bound to the update request, so a retarget after authorization goes through.
  • Failing case: While branchFacts is pending, another actor retargets the PR from allowed dev to disallowed release without moving its head; the expected head SHA still matches, so GitHub is asked to merge release into the head despite the allowed-base refusal.
• 5. minor at src/tools-merge.ts:272, src/tools-merge.ts:279, src/tools-merge.ts:280: The deletion catch reports a failed branch-facts read as a deleteBranch failure.
  • Failing case: If branchFacts returns 404 after a successful merge, the agent sees head branch not deleted: deleteBranch failed: ... even though the server stopped during the safety check before calling deleteBranch.
• 6. minor at src/tools-merge.ts:276, src/tools-merge.ts:279, src/tools-merge.ts:280: The deletion catch claims the branch wasn't deleted even when the delete outcome is unknown.
  • Failing case: If GitHub deletes the branch but the response is lost, the catch reports head branch not deleted after the merge, although the branch is already gone; automatic deletion after merging can also make the same assertion false.
• 7. minor at test/toolsMerge.test.ts:259, test/harness.ts:629, src/tools-merge.ts:333: The ordering test checks only the last two calls, so it can't prove the facts read came before every write.
  • Failing case: An implementation that writes once, then calls branchFacts and writes a second time still has the asserted final two calls and its first update has the asserted arguments, so this test passes even though a write preceded the check.
• 8. minor at test/harness.ts:629, test/harness.ts:632, test/toolsMerge.test.ts:197, src/github-adapter.ts:1553, src/tools-merge.ts:272: The branch-facts fake ignores repository identity, leaving the deletion path's repository choice untested.
  • Failing case: With two repositories having the same branch name but different protection or open PRs, a deletion path that asks branchFacts about the wrong repository still receives the seeded facts and passes these tests; the real adapter would read the other repository and could permit deletion of a base.
• 9. minor at test/toolsMerge.test.ts:289, test/githubAdapter.test.ts:1070, src/github-adapter.ts:1570, test/harness.ts:632: Tests inject precomputed branch facts and cover one adapter combination, so how the adapter reads the three GitHub requests isn't tested.
  • Failing case: If the adapter always returned isDefault false, its existing adapter test would still pass and the new default-branch test would still pass against the fake, but a real update could write to an otherwise unprotected default branch; failures of each of the three underlying reads are likewise not exercised through this path.
• 10. minor at src/tools-merge.ts:319, test/toolsMerge.test.ts:252, test/githubAdapter.test.ts:1070: No test runs a slash-containing head name through the real adapter's branch URL and pull-list query.
  • Failing case: An ordinary update of feature/login previously reached the update endpoint, but all update tests use the in-memory fake and the adapter test uses only release, so a regression that encodes or routes the slash incorrectly and refuses that update would pass the tests.
• 11. minor at test/toolsMerge.test.ts:5, src/tools-merge.ts:294: The test header wrongly says a moved head is refused before any GitHub request.
  • Failing case: For an update with an outdated sha, getPullRequest runs before the mismatch is detected, so a maintainer reading the test header could wrongly conclude the refusal requires no GitHub request.
• 12. minor at src/tools-merge.ts:14, src/tools-merge.ts:164, src/tools-merge.ts:289, src/tools-merge.ts:315, test/toolsMerge.test.ts:4, test/toolsMerge.test.ts:217, test/toolsMerge.test.ts:258, test/toolsMerge.test.ts:290, test/toolsMerge.test.ts:312, test/toolsMerge.test.ts:328, test/harness.ts:125: Changed comments narrate implementation and design rationale instead of stating contracts or constraints.
  • Failing case: A maintainer editing these internal functions and tests encounters a second description of their implementation that must be kept in sync; the commentary can go, with design rationale retained in the change description or commit message instead.