fix(ci): credit takeover authors and exempt stacked PR bases - #742
Conversation
Release notes attributed maintainer takeovers to the landing author only, and enforce-pr-target drafted stacked children that correctly targeted a parent head. Rewrite takeover credits to name the original creator, skip wrong-base for open stacked parents, and drop the retired dual-track essay from AGENTS.md so agent guidance stays current-policy only.
|
Warning Review limit reached
Next review available in: 3 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR adds stacked-base exceptions to PR target enforcement and introduces release-note takeover-credit rewriting. It updates workflow permissions, test harnesses, CI assertions, branch-policy documentation, exported release-note helpers, and release creation integration. ChangesStacked PR enforcement
Release takeover credits
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Sequence Diagram(s)Stacked PR target detectionsequenceDiagram
participant EnforceTarget
participant GitHubPullsAPI
participant QualityValidation
EnforceTarget->>GitHubPullsAPI: list open pull requests
GitHubPullsAPI-->>EnforceTarget: return open PR heads
EnforceTarget->>QualityValidation: pass stackedBase when base matches an open head
QualityValidation-->>EnforceTarget: return quality failures
Release takeover-credit processingsequenceDiagram
participant ReleaseWorkflow
participant CreditTakeovers
participant GitHubAPI
ReleaseWorkflow->>CreditTakeovers: process carried and delta notes
CreditTakeovers->>GitHubAPI: resolve landing and source PR metadata
GitHubAPI-->>CreditTakeovers: return authors and takeover details
CreditTakeovers-->>ReleaseWorkflow: write rewritten release notes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2324c8d086
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }) { | ||
| const failures = []; | ||
| const wrongBase = !allowedBases.includes(baseRef); | ||
| const wrongBase = !allowedBases.includes(baseRef) && !stackedBase; |
There was a problem hiding this comment.
When stackedBase is true, this condition allows a PR targeting any non-allow-listed branch to pass the required target check, and the subsequent branch also skips ancestry enforcement. An open PR with a matching head therefore permits changes to be merged into a non-dev branch, contrary to the repository policy whose only stated exceptions are releases and maintainer promotions; retain wrong_base enforcement or obtain and document an explicit stacked-PR policy exception.
AGENTS.md reference: AGENTS.md:L132-L133
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Kept the stacked exemption (intentional review workflow, not a second integration line). Documented it in AGENTS.md: open-parent-head children skip wrong-base; after the parent lands/closes, retarget to dev.
Also addressed the P2s in this push: pull-requests: read on the release workflow, fail-closed takeover lookups (404 on source PR still leaves the line unchanged), and credit-takeovers on both carried + delta notes.
| bun scripts/release-notes.ts credit-takeovers \ | ||
| --repo "$GITHUB_REPOSITORY" \ | ||
| --in "$delta_file" \ | ||
| --out "$delta_file" |
There was a problem hiding this comment.
This new command calls GET /repos/{owner}/{repo}/pulls/{number}, but the workflow-level permissions at lines 32–35 grant only contents, actions, and id-token, so the GITHUB_TOKEN has no pull-requests: read permission. The pull-request lookups will be rejected, and because ghJson converts those errors to null, the release still succeeds with every takeover credit unchanged; add the least-privilege pull-requests: read permission and assert it in the workflow test.
Useful? React with 👍 / 👎.
| if (exitCode !== 0) { | ||
| console.error(`gh api ${path} failed: ${stderr.trim() || `exit ${exitCode}`}`); | ||
| return null; |
There was a problem hiding this comment.
When GitHub returns a transient error, rate limit, permission error, or malformed response for either the landing or source PR, this path logs the error and returns null; the resolvers then retain the old attribution and the command exits successfully, after which the workflow publishes release notes it never edits. Propagate lookup and JSON errors as a nonzero exit so the release can be retried instead of silently publishing incorrect credits.
AGENTS.md reference: scripts/AGENTS.md:L14-L18
Useful? React with 👍 / 👎.
| printf '%s\n' "$pr_notes" | sed '/^\*\*Full Changelog\*\*:/d' > "$delta_file" | ||
| bun scripts/release-notes.ts credit-takeovers \ | ||
| --repo "$GITHUB_REPOSITORY" \ | ||
| --in "$delta_file" \ |
There was a problem hiding this comment.
When a stable release carries a matching preview created before this helper was introduced, the takeover PR is excluded from the generated delta because notes_range_start advances to the preview tag. This invocation rewrites only delta_file, while the uncorrected carried_file goes directly into assemble, so the stable release still credits only the landing maintainer; run the rewrite over carried content or the final assembled notes as well.
Useful? React with 👍 / 👎.
Grant pull-requests:read for PR author lookups, fail closed on non-404 gh api errors, rewrite carried preview notes as well as the delta, and record the stacked-child wrong-base exemption in AGENTS.md.
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/scripts/pr-quality.test.cjs:
- Around line 258-265: Update the test fixture in the ancestry exemption case to
use the existing ancestry-failure values for behindBase and aheadMain instead of
zero, while keeping stackedBase enabled. Preserve the assertions that neither
wrong_base nor wrong_ancestry is reported so the test fails if the stackedBase
exemption is removed.
In `@scripts/release-notes.ts`:
- Around line 237-272: Update rewriteTakeoverCredits to pre-parse
match.groups.prefix with parseTakeoverSourcePr before calling resolveLanding.
When the title identifies a takeover, reuse that source PR and skip the network
lookup; otherwise fall back to resolveLanding for ambiguous or body-only
markers, preserving the existing author comparison and output behavior.
- Around line 450-487: Update the landing PR lookup callback in
rewriteTakeoverCredits to call ghJson with allowNotFound enabled, matching the
source PR lookup. Return null when the landing PR is missing so
rewriteTakeoverCredits can preserve the original line via its existing fallback,
while continuing to abort on other invalid lookup results; add a unit test
covering a null resolveLanding result if the test suite supports this flow.
In `@tests/helpers/enforce-pr-target-harness.ts`:
- Around line 555-559: Update tests/helpers/enforce-pr-target-harness.ts lines
555-559 to return the page-specific entry from an openPullPages fixture instead
of making every page after the first empty; update lines 92-96 to define and use
the openPullPages fixture type. In tests/ci-workflows.test.ts lines 1015-1043,
provide a full first page plus a matching parent on page two and assert that the
page-two parent remains exempt from wrong-base enforcement, ensuring pagination
is exercised.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 768a80a0-5c97-446e-a9b7-9b7a5c0566aa
📒 Files selected for processing (10)
.github/scripts/enforce-pr-target.test.cjs.github/scripts/pr-quality.cjs.github/scripts/pr-quality.test.cjs.github/workflows/enforce-pr-target.yml.github/workflows/release.ymlAGENTS.mdscripts/release-notes.tstests/ci-workflows.test.tstests/helpers/enforce-pr-target-harness.tstests/release-notes.test.ts
…d PRs Tighten stacked ancestry regression inputs, skip non-takeover gh lookups, tolerate missing landing PRs, and paginate open-PR fixtures so stacked parents beyond page one stay covered.
|
@coderabbitai review |
✅ Action performedReview finished.
|
Merge readyHead What landed
Review triage
Ready to merge into |
1 participant
Summary
generate-notes, rewrite maintainer-takeover changelog lines so the original PR author is credited (by @Original (takeover by @Landing) in …/pull/P), not only the landing maintainer.[WRONG BRANCH]/ draft). Orphan non-devbases stay enforced.dev2-godual-track essay so agent guidance only carries current branch policy (dev/main/preview). The historical retirement record stays inMAINTAINERS.mdand Contributing (all locales); those were left unchanged on purpose.Test plan
bun test tests/release-notes.test.tsbun test tests/ci-workflows.test.ts(includes stacked vs orphan base cases)node --test .github/scripts/pr-quality.test.cjsbun run typecheckSummary by CodeRabbit
New Features
Documentation
Bug Fixes