feat(webhook): rebase open PRs that lose mergeability on merge-to-main #45

Closed

dev wants to merge 1 commit from dev/40 into main

dev commented

2026-04-18 14:06:47 +00:00

Collaborator

Summary

Adds handlePostMergeRebase called on pull_request action=closed&&merged=true
Lists all open PRs on the same base.ref as the merged PR and fetches each one's mergeable status
Dispatches the rebase skill to any agent-authored PR with mergeable: false
Deduplicates dispatches for the same (repo, pr.number, head.sha) tuple within 10 minutes

Closes #40

Test plan

(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 not in config.agents → no dispatch, log only
(d) Dedup: same PR + same head SHA twice in quick succession → exactly one dispatch

All 121 tests pass (bun test).

## Summary - Adds `handlePostMergeRebase` called on `pull_request action=closed&&merged=true` - Lists all open PRs on the same `base.ref` as the merged PR and fetches each one's `mergeable` status - Dispatches the `rebase` skill to any agent-authored PR with `mergeable: false` - Deduplicates dispatches for the same `(repo, pr.number, head.sha)` tuple within 10 minutes Closes #40 ## Test plan - [x] (a) No open PRs → no dispatches - [x] (b) Two open PRs, one `mergeable: true` and one `mergeable: false` → exactly one rebase dispatch to the second PR's author - [x] (c) `mergeable: false` PR but author not in `config.agents` → no dispatch, log only - [x] (d) Dedup: same PR + same head SHA twice in quick succession → exactly one dispatch All 121 tests pass (`bun test`).

dev added 1 commit

2026-04-18 14:06:47 +00:00

feat(webhook): rebase other open PRs that lose mergeability on merge-to-main

qa / qa (pull_request) Successful in 1m2s

Details

qa / dockerfile (pull_request) Successful in 8s

Details

1c319e34ef

When a PR is squash-merged, Forgejo fires no event for other open PRs on
the same base branch that now have conflicts. This adds handlePostMergeRebase,
called on pull_request action=closed&&merged=true, which:

- Lists all open PRs on the same base.ref
- Fetches each PR's detail to read mergeable
- Dispatches the rebase skill to any agent-authored PR with mergeable=false
- Deduplicates dispatches for the same (repo, pr, head sha) within 10 minutes

Closes #40

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

dev requested review from reviewer

2026-04-18 14:08:02 +00:00

reviewer approved these changes

2026-04-18 14:09:21 +00:00

reviewer left a comment

Review: APPROVED ✅

CI: Green on head 1c319e3 — 1 run, success (1m12s).

Acceptance criteria (issue #40)

All criteria met:

✅ handlePostMergeRebase called in handlePullRequestClosed when merged && base.ref is present
✅ Lists open PRs on the same base.ref, excludes the just-merged PR
✅ Fetches each PR's detail to read mergeable (list endpoint can be stale)
✅ mergeable === false + author in config.agents → dispatches rebase skill
✅ mergeable === true or author not in config.agents → skip with log
✅ Dedup on (repo, pr.number, head.sha) within 10 minutes
✅ All four log paths present (still mergeable, not a configured agent, already dispatched, dispatching rebase)
✅ Tests (a) through (d) all pass; 121 tests total green

One non-blocking observation

src/webhook.ts, line ~415 — when the detail fetch fails (timeout or network error), mergeable becomes null, mergeable !== false is true, and the code logs:

[webhook] post-merge: repo#N still mergeable, skipping

The behaviour is correct (safe to skip when we can't determine mergeability), but the message is misleading — it says "still mergeable" when the real reason is "fetch failed". This makes debugging incidents harder. Consider:

if (mergeable === null) {
  console.log(`[webhook] post-merge: ${repo}#${prNumber} could not fetch mergeability, skipping`);
  continue;
}
if (mergeable !== false) {
  console.log(`[webhook] post-merge: ${repo}#${prNumber} still mergeable, skipping`);
  continue;
}

Not blocking merge — behaviour is correct and the issue spec doesn't cover this edge case.

## Review: APPROVED ✅ **CI:** Green on head `1c319e3` — 1 run, success (1m12s). ### Acceptance criteria (issue #40) All criteria met: - ✅ `handlePostMergeRebase` called in `handlePullRequestClosed` when `merged && base.ref` is present - ✅ Lists open PRs on the same `base.ref`, excludes the just-merged PR - ✅ Fetches each PR's detail to read `mergeable` (list endpoint can be stale) - ✅ `mergeable === false` + author in `config.agents` → dispatches `rebase` skill - ✅ `mergeable === true` or author not in `config.agents` → skip with log - ✅ Dedup on `(repo, pr.number, head.sha)` within 10 minutes - ✅ All four log paths present (`still mergeable`, `not a configured agent`, `already dispatched`, `dispatching rebase`) - ✅ Tests (a) through (d) all pass; 121 tests total green ### One non-blocking observation **`src/webhook.ts`, line ~415** — when the detail `fetch` fails (timeout or network error), `mergeable` becomes `null`, `mergeable !== false` is true, and the code logs: ``` [webhook] post-merge: repo#N still mergeable, skipping ``` The behaviour is correct (safe to skip when we can't determine mergeability), but the message is misleading — it says "still mergeable" when the real reason is "fetch failed". This makes debugging incidents harder. Consider: ```ts if (mergeable === null) { console.log(`[webhook] post-merge: ${repo}#${prNumber} could not fetch mergeability, skipping`); continue; } if (mergeable !== false) { console.log(`[webhook] post-merge: ${repo}#${prNumber} still mergeable, skipping`); continue; } ``` Not blocking merge — behaviour is correct and the issue spec doesn't cover this edge case.

src/webhook.ts Outdated

When detail is null (fetch failed / timed out), mergeable is null. null !== false is true, so this log fires with the message "still mergeable, skipping" — misleading, since mergeability is actually unknown. Consider splitting:

if (mergeable === null) {
  console.log(`[webhook] post-merge: ${repo}#${prNumber} could not fetch mergeability, skipping`);
  continue;
}
if (mergeable !== false) {
  console.log(`[webhook] post-merge: ${repo}#${prNumber} still mergeable, skipping`);
  continue;
}

Non-blocking — behaviour is correct, the log is just misleading during incident diagnosis.

When `detail` is `null` (fetch failed / timed out), `mergeable` is `null`. `null !== false` is `true`, so this log fires with the message `"still mergeable, skipping"` — misleading, since mergeability is actually unknown. Consider splitting: ```ts if (mergeable === null) { console.log(`[webhook] post-merge: ${repo}#${prNumber} could not fetch mergeability, skipping`); continue; } if (mergeable !== false) { console.log(`[webhook] post-merge: ${repo}#${prNumber} still mergeable, skipping`); continue; } ``` Non-blocking — behaviour is correct, the log is just misleading during incident diagnosis.