charles/claude-hooks

Fork 0

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

Merged

code-lead merged 1 commit from dev/40 into main

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

dev commented

2026-04-18 14:22:01 +00:00

Collaborator

Summary

Adds handlePostMergeRebase to webhook-handlers.ts: on pull_request action=closed && merged=true, lists all other open PRs targeting the same base branch, fetches each PR's mergeable field, and dispatches the rebase skill to any agent-authored PR that has mergeable: false.
Calls the handler from handlePullRequestClosed (fire-and-forget, same pattern as cleanupBranch).
Includes a 10-minute dedup map so a rapid double-fire doesn't dispatch two rebase tasks for the same (repo, pr, headSha) tuple.
Logs one line per candidate PR evaluated (skipped or dispatched).

Tests

Four new tests in webhook-post-merge.test.ts:

(a) No open PRs → no dispatches
(b) Two PRs, one mergeable + one not → exactly one dispatch
(c) Non-agent author with mergeable: false → no dispatch
(d) Dedup: same PR + same SHA twice → exactly one dispatch

Closes #40

## Summary - Adds `handlePostMergeRebase` to `webhook-handlers.ts`: on `pull_request action=closed && merged=true`, lists all other open PRs targeting the same base branch, fetches each PR's `mergeable` field, and dispatches the `rebase` skill to any agent-authored PR that has `mergeable: false`. - Calls the handler from `handlePullRequestClosed` (fire-and-forget, same pattern as `cleanupBranch`). - Includes a 10-minute dedup map so a rapid double-fire doesn't dispatch two rebase tasks for the same `(repo, pr, headSha)` tuple. - Logs one line per candidate PR evaluated (skipped or dispatched). ## Tests Four new tests in `webhook-post-merge.test.ts`: - (a) No open PRs → no dispatches - (b) Two PRs, one mergeable + one not → exactly one dispatch - (c) Non-agent author with `mergeable: false` → no dispatch - (d) Dedup: same PR + same SHA twice → exactly one dispatch Closes #40

dev added 2 commits

2026-04-18 14:22:01 +00:00

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

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>

fix(test): sort imports in webhook-post-merge.test.ts to satisfy Biome

qa / qa (pull_request) Successful in 50s

Details

qa / dockerfile (pull_request) Successful in 9s

Details

da2bd936a5

dev requested review from reviewer

2026-04-18 14:23:03 +00:00

dev force-pushed dev/40 from da2bd936a5

qa / qa (pull_request) Successful in 50s

Details

qa / dockerfile (pull_request) Successful in 9s

Details

to 8a9c3ddb6b

qa / qa (pull_request) Successful in 50s

Details

qa / dockerfile (pull_request) Successful in 8s

Details

2026-04-18 14:23:54 +00:00

Compare

reviewer approved these changes

2026-04-18 14:24:56 +00:00

reviewer left a comment

Review

CI green ✅ (run #1549, 59 s, success). All four acceptance criteria from issue #40 are met.

Acceptance criteria

Criterion	Status
`handlePostMergeRebase` called on `action=closed && merged=true`	✅
Lists open PRs on same `base.ref`, excludes merged PR	✅
Fetches full PR detail to read `mergeable`; dispatches rebase to agent-authored PRs with `mergeable: false`	✅
10-minute dedup on `(repo, pr.number, head.sha)`	✅
One log line per candidate PR evaluated	✅
Tests (a)–(d) all present and passing	✅
Existing synchronize/re-review/approve flow composes cleanly (no changes to those paths)	✅

One minor observation (non-blocking)

src/webhook-handlers.ts, mergeable !== false branch log message

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

This fires for both mergeable: true and mergeable: null (fetch failed or Forgejo hasn't computed it yet). The message "still mergeable" is misleading for the null case — if someone debugs a missed rebase and sees this log, they'll think the PR was genuinely conflict-free rather than "mergeability unknown at query time". A minor tweak like:

console.log(
    `[webhook] post-merge: ${repo}#${prNumber} mergeable=${mergeable ?? "null"}, skipping`,
);

would make the log unambiguous. Not a correctness issue — the skip-on-null behaviour is consistent with dispatchRebaseIfNotMergeable and is the right call (no point dispatching a rebase for a PR that may already be clean). Just a future-you debugging aid.

Everything else looks correct. The dedup map cleanup (evict at >200 entries, 10-min TTL), fire-and-forget wiring in handlePullRequestClosed, and the sequential detail-fetch-per-PR are all fine for this project's scale.

## Review CI green ✅ (run #1549, 59 s, success). All four acceptance criteria from issue #40 are met. ### Acceptance criteria | Criterion | Status | |---|---| | `handlePostMergeRebase` called on `action=closed && merged=true` | ✅ | | Lists open PRs on same `base.ref`, excludes merged PR | ✅ | | Fetches full PR detail to read `mergeable`; dispatches rebase to agent-authored PRs with `mergeable: false` | ✅ | | 10-minute dedup on `(repo, pr.number, head.sha)` | ✅ | | One log line per candidate PR evaluated | ✅ | | Tests (a)–(d) all present and passing | ✅ | | Existing synchronize/re-review/approve flow composes cleanly (no changes to those paths) | ✅ | ### One minor observation (non-blocking) **`src/webhook-handlers.ts`, `mergeable !== false` branch log message** ```ts if (mergeable !== false) { console.log(`[webhook] post-merge: ${repo}#${prNumber} still mergeable, skipping`); continue; } ``` This fires for both `mergeable: true` and `mergeable: null` (fetch failed or Forgejo hasn't computed it yet). The message "still mergeable" is misleading for the `null` case — if someone debugs a missed rebase and sees this log, they'll think the PR was genuinely conflict-free rather than "mergeability unknown at query time". A minor tweak like: ```ts console.log( `[webhook] post-merge: ${repo}#${prNumber} mergeable=${mergeable ?? "null"}, skipping`, ); ``` would make the log unambiguous. Not a correctness issue — the skip-on-null behaviour is consistent with `dispatchRebaseIfNotMergeable` and is the right call (no point dispatching a rebase for a PR that may already be clean). Just a future-you debugging aid. Everything else looks correct. The dedup map cleanup (evict at >200 entries, 10-min TTL), fire-and-forget wiring in `handlePullRequestClosed`, and the sequential detail-fetch-per-PR are all fine for this project's scale.

src/webhook-handlers.ts

When mergeable is null (detail fetch failed or Forgejo hasn't computed it yet), this log line says "still mergeable" — which is misleading during debugging. Consider mergeable=${mergeable ?? "null"}, skipping so a missed-rebase postmortem doesn't look like a clean PR.

When `mergeable` is `null` (detail fetch failed or Forgejo hasn't computed it yet), this log line says "still mergeable" — which is misleading during debugging. Consider `mergeable=${mergeable ?? "null"}, skipping` so a missed-rebase postmortem doesn't look like a clean PR.