On merge-to-main, rebase other open PRs that just lost mergeability #40

New issue

Closed

opened 2026-04-18 11:18:05 +00:00 by claude-desktop · 0 comments

claude-desktop commented

2026-04-18 11:18:05 +00:00

Collaborator

User story

As a maintainer, when one PR merges to main and breaks the mergeability of other open PRs targeting the same base, I want the webhook to dispatch the other PRs' authors to rebase automatically, so that a merge conflict on PR B doesn't leave B silently stalled with no agent touching it.

Context

Forgejo fires no webhook event for "PR mergeability changed". So today:

PRs A and B are both approved, mergeable: true.
Boss squash-merges A → main advances.
PR B now has conflicts against the new main — mergeable flips to false.
Nothing triggers. No synchronize, no status, no action_run_* event for B. B's approval still stands, its head SHA is unchanged, CI is unchanged. The only way forward is a human closing the loop manually.

If boss later attempts to merge B (e.g. from a lingering pull_request_approved dispatch), the patched merge_pull_request will surface the conflict as an error and boss's merge.md skill tells it to "post a comment asking the author to rebase, exit" — but nothing then dispatches the author. B stays open with a comment no one reacts to.

Acceptance criteria

New post-merge handler

In the pull_request case, when action=closed and merged=true, call a new handler handlePostMergeRebase(repo, mergedPr) (name TBD; can live in webhook-ci.ts after #34 lands).
The handler lists open PRs on the same base.ref as the merged PR, excluding the just-merged one.
For each candidate PR:
- Fetch the PR detail and read mergeable.
- If mergeable === false and the PR author is a configured agent (has a token in config.agents), dispatch the author with the existing rebase skill (same codepath as dispatchRebaseIfNotMergeable).
- If mergeable === true or the PR is not authored by an agent: skip silently.
Dedup: if we've already dispatched a rebase for this (repo, pr.number, pr.head.sha) tuple recently (e.g. within 10 minutes), skip. Reuse the BoundedMap from #35 if it's landed.

Existing flow composes cleanly

After the rebase push, the existing pull_request action=synchronize(d) handler fires (new head SHA) → no-workflows fast path or CI fallback → reviewer re-requested.
Reviewer's prior APPROVED verdict is automatically stale because requestReviewIfFresh compares commit_id === sha — so the reviewer is re-asked on the new SHA and can re-approve.
Approval → boss → merge → loop closes.

Logging

One log line per candidate PR evaluated: either [webhook] post-merge: <repo>#<n> still mergeable, skipping or [webhook] post-merge: <repo>#<n> lost mergeability, dispatching rebase to <author>.

Tests

handlePostMergeRebase:
- (a) No open PRs → no dispatches.
- (b) Two open PRs, one mergeable: true and one mergeable: false → exactly one rebase dispatch, to the second PR's author.
- (c) mergeable: false PR but author is not in config.agents → no dispatch, log only.
- (d) Dedup: same PR + same head SHA twice in quick succession → exactly one dispatch.

Out of scope

Pre-emptively rebasing mergeable: true PRs (noise, no benefit — rebasing a mergeable PR just churns CI).
Cross-base handling (e.g. PR C targeting develop). This issue only handles PRs that share the merged PR's base.ref.
Forgejo-side mergeability event (upstream feature, out of our control).
Changing merge.md to skip the "post comment, exit" behavior. The comment + exit stays; this new handler makes the comment informational rather than load-bearing.

References

Edge case surfaced 2026-04-18 during refactor kickoff (see conversation history).
Related webhook handlers: handlePullRequestClosed, handlePullRequestOpened, dispatchRebaseIfNotMergeable.

Dependencies

Helped by (not blocked by): #34 (cleaner seams for the new handler), #35 (BoundedMap for dedup).
Not blocked by: anything strictly — can be implemented in either pre- or post-refactor layout.
Branch off: main (or whichever branch #34 merges to if it lands first).

## User story As a **maintainer**, when one PR merges to `main` and breaks the mergeability of other open PRs targeting the same base, I want the webhook to dispatch the other PRs' authors to rebase automatically, so that a merge conflict on PR B doesn't leave B silently stalled with no agent touching it. ## Context Forgejo fires no webhook event for "PR mergeability changed". So today: 1. PRs A and B are both approved, `mergeable: true`. 2. Boss squash-merges A → `main` advances. 3. PR B now has conflicts against the new `main` — `mergeable` flips to `false`. 4. **Nothing triggers.** No `synchronize`, no `status`, no `action_run_*` event for B. B's approval still stands, its head SHA is unchanged, CI is unchanged. The only way forward is a human closing the loop manually. If boss later attempts to merge B (e.g. from a lingering `pull_request_approved` dispatch), the patched `merge_pull_request` will surface the conflict as an error and boss's `merge.md` skill tells it to "post a comment asking the author to rebase, exit" — but nothing then dispatches the author. B stays open with a comment no one reacts to. ## Acceptance criteria ### New post-merge handler - [ ] In the `pull_request` case, when `action=closed` and `merged=true`, call a new handler `handlePostMergeRebase(repo, mergedPr)` (name TBD; can live in `webhook-ci.ts` after #34 lands). - [ ] The handler lists open PRs on the same `base.ref` as the merged PR, excluding the just-merged one. - [ ] For each candidate PR: - Fetch the PR detail and read `mergeable`. - If `mergeable === false` and the PR author is a configured agent (has a token in `config.agents`), dispatch the author with the existing `rebase` skill (same codepath as `dispatchRebaseIfNotMergeable`). - If `mergeable === true` or the PR is not authored by an agent: skip silently. - [ ] Dedup: if we've already dispatched a rebase for this `(repo, pr.number, pr.head.sha)` tuple recently (e.g. within 10 minutes), skip. Reuse the `BoundedMap` from #35 if it's landed. ### Existing flow composes cleanly - [ ] After the rebase push, the existing `pull_request action=synchronize(d)` handler fires (new head SHA) → no-workflows fast path or CI fallback → reviewer re-requested. - [ ] Reviewer's prior APPROVED verdict is automatically stale because `requestReviewIfFresh` compares `commit_id === sha` — so the reviewer is re-asked on the new SHA and can re-approve. - [ ] Approval → boss → merge → loop closes. ### Logging - [ ] One log line per candidate PR evaluated: either `[webhook] post-merge: <repo>#<n> still mergeable, skipping` or `[webhook] post-merge: <repo>#<n> lost mergeability, dispatching rebase to <author>`. ### Tests - [ ] `handlePostMergeRebase`: - (a) No open PRs → no dispatches. - (b) Two open PRs, one `mergeable: true` and one `mergeable: false` → exactly one rebase dispatch, to the second PR's author. - (c) `mergeable: false` PR but author is not in `config.agents` → no dispatch, log only. - (d) Dedup: same PR + same head SHA twice in quick succession → exactly one dispatch. ## Out of scope - Pre-emptively rebasing `mergeable: true` PRs (noise, no benefit — rebasing a mergeable PR just churns CI). - Cross-base handling (e.g. PR C targeting `develop`). This issue only handles PRs that share the merged PR's `base.ref`. - Forgejo-side mergeability event (upstream feature, out of our control). - Changing `merge.md` to skip the "post comment, exit" behavior. The comment + exit stays; this new handler makes the comment informational rather than load-bearing. ## References - Edge case surfaced 2026-04-18 during refactor kickoff (see conversation history). - Related webhook handlers: `handlePullRequestClosed`, `handlePullRequestOpened`, `dispatchRebaseIfNotMergeable`. ## Dependencies - **Helped by (not blocked by):** `#34` (cleaner seams for the new handler), `#35` (BoundedMap for dedup). - **Not blocked by:** anything strictly — can be implemented in either pre- or post-refactor layout. - **Branch off:** `main` (or whichever branch `#34` merges to if it lands first).

claude-desktop added the

area:webhook

type:user-story

labels

2026-04-18 11:18:10 +00:00