fix: Make a sweep's one update after its merges, and only if it merged none

[mpaulosky]

Closes #177.

Why

Claude Review on mpaulosky/TicketManager#191 (the #176 re-Apply) found that a sweep could update an older ready BEHIND PR and then merge a newer CLEAN one in the same loop. The merge moves main, so the updated PR is BEHIND again and its CI run is wasted.

What changed

• pr-automerge.yml (Template and dotfiles' copy): during a sweep, bringUpToDate only records a ready BEHIND PR as a candidate. After every PR is evaluated, the sweep updates the oldest candidate it can (a 422 or a Dependabot PR already asked for its head still moves on to the next), but only if it merged nothing. When it merged one, that merge's push sweeps again. The update itself moves into updateBranchOf, which returns whether it asked for one. PR events still update their own PR straight away.
• Test: a sweep that merges a later PR leaves the earlier BEHIND one for the merge's push.
• references/automerge.md and ADR 0005 describe the ordering.

Not changed

The second thread on TicketManager#191 (the oldest PR keeps the slot without merging) is already handled: bringUpToDate skips a PR whose checks failed on its head, and one waiting on a review or a thread never reaches it, so the sweep moves to the next PR.

Testing

test.sh passes (134 auto-merge tests).

🤖 Generated with Claude Code

[github-actions]

Claude Review

The PR changes how a sweep (a push to main, the schedule, or a manual run) brings a PR up to date. Before, it updated the first ready BEHIND PR the moment it found one. Now it saves the ready BEHIND PRs as candidates while it checks every open PR. After the merge loop it updates the oldest candidate it can, and only if the sweep merged nothing. The idea is that a merge made later in the same sweep would leave the updated PR BEHIND again and waste its CI run. updateBranchOf now returns whether an update was asked for, so a 422 or an error moves on to the next candidate. The repo's own workflow and the template copy have the same blob hashes (8d8993c..842a2f7), so they match exactly. A test covers the case where a later PR merges, and the ADR and reference docs describe the new rule.

Verdict: the logic is sound. Because the merge loop runs first, the case the PR targets is handled, and a 422 or Dependabot result still falls through to the next candidate as before. Two robustness points come from making the update the sweep's last step; they are listed below and are low severity. Notes:

• I relied on the merge being made with a token whose push to main triggers this workflow (the PAT, not GITHUB_TOKEN). Otherwise "whose push sweeps again" is false, and a skipped candidate waits for the hourly schedule. The old comment already assumed this, but the PR now leans on it more, because it skips the update outright whenever there's a merge.
• I couldn't re-read the checked-out file. The line numbers come from the diff's hunk headers, and the two workflow copies are identical, so they hold for both.
• The new test covers only a merge that comes after the BEHIND PR. It doesn't cover the reverse: PR 7 merges, then PR 8 shows as BEHIND. That changes behaviour too, since PR 8 used to be updated in the same sweep and now waits for the push sweep. A test pinning that would help.

Every piece was read; nothing is left unreviewed. There is no CLAUDE.md or AGENTS.md.

[github-actions]

Claude Review

What changes: The PR changes how a sweep (a push to main, a scheduled run or a manual run) in pr-automerge.yml updates PRs that are ready but BEHIND main. The root copy and the template copy get identical changes.

• During the first pass, a ready BEHIND PR is only recorded as a candidate.
• After the pass, the sweep checks each candidate again, in order. It updates the first one that is still ready and BEHIND, but only if the sweep merged nothing. If it did merge something, the push from that merge starts another sweep.
• Each PR's evaluation is now wrapped in a try/catch. An error on one PR (a 5xx or a rate limit) is logged as a warning, the sweep carries on and makes its update, and the run is failed with core.setFailed at the end.
• The tests gain per-PR snapshots (byNumber) and read errors (readErrors). The ADR and automerge.md describe the new order.

Verdict: I found no correctness problems. I checked these paths:

• Events for a single PR, and workflow_run: these runs have collectingCandidates false, so they still update their PR directly.
• 422 and Dependabot already asked: both still return false and the sweep moves on to the next candidate.
• A candidate merged in the second pass: a candidate that is now CLEAN gets merged, and that sets mergedThisSweep, so the remaining candidates wait.
• An error in the second pass: it is caught and the next candidate is tried.
• Without RELEASE_PR_PAT: BEHIND PRs are never recorded as candidates, so the notice behaviour is unchanged.

Trade-off to accept knowingly: a merge earlier in the sweep, by a PR older than the BEHIND one, now also holds back the update, even though updating after that merge would not be wasted. The PR then waits for the sweep started by the merge's push.

What I couldn't check:

• I read only the diff, not the unchanged code around it: the body of mergeIfReady, the workflow's triggers, concurrency settings and token setup.
• Waiting for "the merge's push sweeps again" assumes merges are made with the PAT, so their push to main triggers the workflow. A push made with GITHUB_TOKEN starts no workflow runs. The existing ADR already relies on this.
• I assumed that evaluating a ready PR a second time has no side effects that aren't safe to repeat.
• The line numbers come from the diff's hunk headers. I didn't re-read them in the checked-out file.

No piece went unreviewed: INDEX lists one piece, 001.diff, and I read all of it. Neither AGENTS.md nor CLAUDE.md exists.

[github-actions]

Claude Review

What the PR changes. It changes the sweep in PR Auto-Merge (.github/workflows/pr-automerge.yml and its identical template copy). A sweep is a run triggered by a push to main, the schedule, or a manual run.

• Candidates first. During the sweep, a ready BEHIND PR is only noted as a candidate, no longer updated on the spot.
• One update, last. After every open PR has been evaluated, each candidate is evaluated again, oldest first. The first one that is still ready and BEHIND gets the sweep's single branch update. One that is now CLEAN is merged instead.
• No update after a merge. If the sweep merged any PR, no update is made. The merge's push to main starts another sweep.
• Errors don't stop the sweep. An exception on one PR is logged as a warning, the sweep goes on, and core.setFailed fails the run at the end.
• Refactor. The update call moves into updateBranchOf, which returns whether an update was asked for. updatedThisSweep is now set only in the caller.
• Tests and docs. The test harness gains per-PR snapshot sequences (byNumber) and failing reads (readErrors). New tests cover: a later merge holding back an earlier BEHIND PR, a candidate handed back before its update, a candidate that is CLEAN when read again, and errors on the first or the second read. The ADR and references/automerge.md describe the new order.

Paths checked:

• PR events and workflow_run events. collectingCandidates is false for them, so they still update their own PR directly.
• Ordering. A merge after the sweep's update can't happen: once updatedThisSweep is set, every remaining candidate is skipped.
• A candidate that changes before its update. If it gets a 422, the hand-back label, or a moved head, it is skipped and the next candidate is tried.
• Rebase requests. A Dependabot rebase request is not repeated for the same head, because of the marker check in askDependabotToRebase.
• No RELEASE_PR_PAT. That check comes before the candidate is recorded, so without the PAT nothing is ever collected.
• A failed run. Failing a sweep run doesn't affect any PR's checks or mergeStateStatus. A sweep is not a PR check run.
• The two workflow copies have the same blob hash (8abaf00), so they stay in sync.

What I relied on without checking:

• Which token merges. The "merge's push sweeps again" design needs the squash-merge to be made with a token whose push to main triggers workflows. A merge made with GITHUB_TOKEN would not trigger the push sweep. This PR leans on that more than before: an earlier ready BEHIND PR now waits for that push instead of being updated in the same sweep. It holds if the github-script client uses RELEASE_PR_PAT whenever it is set, which the HAS_RELEASE_PR_PAT gate suggests. I didn't verify this in the workflow's unchanged lines.
• Running the evaluation twice. I assumed the rest of mergeIfReady has no side effects that are a problem when a candidate is evaluated a second time.

Not reviewed: AGENTS.md and CLAUDE.md don't exist at the repository root. I read nothing beyond the diff piece and INDEX; the only piece, 001.diff, was read in full.

Verdict: no blocking problems found. The logic is consistent with its comments and docs, and the new tests cover the new paths.

No findings.