feat(webhook): linear cascade rebase via dependency markers (B13) #438

Merged
code-lead merged 1 commit from boss/429 into main 2026-04-27 10:06:24 +00:00
Collaborator

PRs can opt out of the O(N²) post-merge fan-out by declaring Stacked on #M or Depends on #M in the body — only the named parent's merge or re-push triggers a rebase. Marker-less PRs keep the legacy mergeable: false cascade. Cycles are rejected with a one-line comment; cross-repo refs are silently dropped.

Closes #429

Test plan

  • just qa clean (typecheck + Biome + 2056 server tests)
  • Linear-chain test: merge #1 → only #2 rebases (#3 stays)
  • Stacked-rebase test: #2 synchronize → #3 rebases
  • Cycle-detection test: A → B → A → comment posted, edges intact
  • Marker-less PR test: legacy fan-out preserved
PRs can opt out of the O(N²) post-merge fan-out by declaring `Stacked on #M` or `Depends on #M` in the body — only the named parent's merge or re-push triggers a rebase. Marker-less PRs keep the legacy `mergeable: false` cascade. Cycles are rejected with a one-line comment; cross-repo refs are silently dropped. Closes #429 ## Test plan - [ ] `just qa` clean (typecheck + Biome + 2056 server tests) - [ ] Linear-chain test: merge `#1` → only `#2` rebases (`#3` stays) - [ ] Stacked-rebase test: `#2` synchronize → `#3` rebases - [ ] Cycle-detection test: `A → B → A` → comment posted, edges intact - [ ] Marker-less PR test: legacy fan-out preserved
feat(webhook): linear cascade rebase via dependency markers (B13)
All checks were successful
qa / qa (pull_request) Successful in 7m47s
qa / dockerfile (pull_request) Successful in 13s
189d51825e
PR bodies can mark a stacked relationship with `Stacked on #M` or
`Depends on #M` (case-insensitive, comma-list accepted). Marked PRs only
rebase when their named parent merges or re-pushes; un-marked PRs keep
the legacy fan-out. Parser runs on `pull_request.opened`, `reopened`,
`edited`. Cycle detection rejects + posts a one-line comment, leaving
the prior edge set intact. Cross-repo refs are silently dropped.

Closes #429

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
reviewer approved these changes 2026-04-27 09:28:05 +00:00
reviewer left a comment

CI green. All five AC blocks verified.

ParserparseP rDependencies correctly handles both phrasings, comma-list continuation, case-insensitivity, cross-repo drop, and \b word-boundary guards. Dedupes + sorts on return. Fires on opened / reopened / edited in webhook.ts (raw payload.action, correct rationale in the comment).

DBpr_dependencies schema, setPrDependencies (atomic delete+insert transaction), getPrDependencies, getPrsDependingOn, hasPrDependencies, clearPrDependencies, listPrDependencies all look correct. Transaction ensures no half-stale edge sets.

Dispatch — Three-way routing in handlePostMergeRebase is correct: named-parent → unconditional dispatch; named-different-parent → skip; no marker → legacy mergeable fan-out. handleStackedRebaseCascade correctly fires on synchronized and propagates one level (recursive propagation happens naturally as each child's rebase push triggers another synchronized).

Cycle detection — DFS from prNumber with replacement semantics (prior edges from prNumber excluded before walk). Self-loop, A→B→A, and longer cycles all tested. Comment body matches AC verbatim.

Closed-PR cleanuphandlePullRequestClosed clears outgoing edges; incoming refs (other PRs depending on the closed one) are intentionally left — correct and documented.

Tests — All four AC-mandated unit tests are present and pass: linear chain with only #2 rebasing, stacked-rebase propagation on synchronized, cycle rejection + comment, marker-less fan-out preserved.

Nit (not blocking): handlePostMergeRebase calls forge.getPullRequest for every open PR before checking the marker early-exit at line 880. Moving getPrDependencies + the skip guard to before the detail fetch would eliminate the wasted API call for marker-bound PRs that point elsewhere — e.g. in a 10-PR linear chain merging #1, PRs #3-10 each incur a getPullRequest call before being skipped. Pre-existing O(N) pattern rather than a B13 regression, so not blocking, but worth a follow-up.

CI green. All five AC blocks verified. **Parser** — `parseP rDependencies` correctly handles both phrasings, comma-list continuation, case-insensitivity, cross-repo drop, and `\b` word-boundary guards. Dedupes + sorts on return. Fires on `opened` / `reopened` / `edited` in `webhook.ts` (raw `payload.action`, correct rationale in the comment). **DB** — `pr_dependencies` schema, `setPrDependencies` (atomic delete+insert transaction), `getPrDependencies`, `getPrsDependingOn`, `hasPrDependencies`, `clearPrDependencies`, `listPrDependencies` all look correct. Transaction ensures no half-stale edge sets. **Dispatch** — Three-way routing in `handlePostMergeRebase` is correct: named-parent → unconditional dispatch; named-different-parent → skip; no marker → legacy `mergeable` fan-out. `handleStackedRebaseCascade` correctly fires on `synchronized` and propagates one level (recursive propagation happens naturally as each child's rebase push triggers another `synchronized`). **Cycle detection** — DFS from `prNumber` with replacement semantics (prior edges from `prNumber` excluded before walk). Self-loop, A→B→A, and longer cycles all tested. Comment body matches AC verbatim. **Closed-PR cleanup** — `handlePullRequestClosed` clears outgoing edges; incoming refs (other PRs depending on the closed one) are intentionally left — correct and documented. **Tests** — All four AC-mandated unit tests are present and pass: linear chain with only #2 rebasing, stacked-rebase propagation on `synchronized`, cycle rejection + comment, marker-less fan-out preserved. Nit (not blocking): `handlePostMergeRebase` calls `forge.getPullRequest` for every open PR before checking the marker early-exit at line 880. Moving `getPrDependencies` + the skip guard to before the `detail` fetch would eliminate the wasted API call for marker-bound PRs that point elsewhere — e.g. in a 10-PR linear chain merging #1, PRs #3-10 each incur a `getPullRequest` call before being skipped. Pre-existing O(N) pattern rather than a B13 regression, so not blocking, but worth a follow-up.
code-lead force-pushed boss/429 from 189d51825e
All checks were successful
qa / qa (pull_request) Successful in 7m47s
qa / dockerfile (pull_request) Successful in 13s
to c5e8ac30c7
All checks were successful
qa / qa (pull_request) Successful in 8m51s
qa / dockerfile (pull_request) Successful in 12s
2026-04-27 09:31:40 +00:00
Compare
code-lead deleted branch boss/429 2026-04-27 10:06:25 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
charles/claude-hooks!438
No description provided.