Skip to content

ci(pr-readiness): only assess PRs targeting the default branch - #16821

Open
Joibel wants to merge 1 commit into
argoproj:mainfrom
Joibel:pr-readiness-default-branch-only
Open

ci(pr-readiness): only assess PRs targeting the default branch#16821
Joibel wants to merge 1 commit into
argoproj:mainfrom
Joibel:pr-readiness-default-branch-only

Conversation

@Joibel

@Joibel Joibel commented Aug 26, 2026

Copy link
Copy Markdown
Member
  • Ran make pre-commit -B (n/a — .github/pr-readiness only; ran npm run typecheck and npm test)
  • Signed-off commits with Conventional Commit messages
  • PR title is a conventional commit message (it becomes the release notes entry)
  • Unit or e2e tests cover the change
  • Opened as draft; will mark "Ready for review" once builds are green

Motivation

The PR readiness helper currently assesses every PR that has CI complete on it, including backports to release-* branches (e.g. #16813 / #16814). Backports follow their own process, and drafting or commenting on them is noise.

Modifications

  • classify.ts: new pure function targetsDefaultBranch(pr, openPrs, defaultBranch). A PR is in scope when its base is the default branch, or its base is the head branch of another open PR in the same repository that (transitively) targets the default branch — so stacked PRs heading for main are still assessed. Dead ends (a release branch, or a base whose PR has been closed/merged) and cycles are out of scope.
  • main.ts: after resolving the PR by head SHA, skip it unless targetsDefaultBranch holds. The default branch comes from context.payload.repository.default_branch rather than being hard-coded. The open-PR list was already fetched for head-SHA resolution, so this adds no API calls.
  • types.ts: StackablePr — the PR subset the walk needs.
  • README: documents the new skip rule.

workflow_run triggers can't filter on the PR's base branch (the branches: filter applies to the run's head branch), which is why this lives in the script rather than the workflow's if:.

Verification

  • Six new unit tests in test/classify.test.ts: direct main target, release-branch target, three-deep stack, stack ending on a release branch / closed PR, a fork head sharing the base's branch name (must not be treated as part of the stack), and a cycle.
  • npm run typecheck clean; npm test 43/43 passing. pr-readiness-test.yaml runs the same in CI.

Documentation

.github/pr-readiness/README.md updated.

AI

Claude Code was used to draft the change, tests and this description; reviewed by me.

Summary by CodeRabbit

  • Bug Fixes

    • Pull requests targeting non-default branches, including release branches, are now excluded from readiness checks.
    • Stacked pull requests targeting the default branch continue to be evaluated correctly.
    • Added safeguards for forked branches and circular stacked-PR relationships.
  • Tests

    • Added coverage for direct, stacked, release-branch, forked, closed, and circular pull-request scenarios.

The PR readiness helper fired on every PR that had CI complete on it,
including backports to release-* branches, which follow their own
process and should not be drafted or commented on.

Resolve the PR as before, then skip it unless it is bound for the
default branch. A stacked PR whose base is the head branch of another
open PR in the same repository is followed up the chain of open PRs,
so a stack that ends on the default branch is still assessed, while
one that ends on a release branch, a closed PR, or a cycle is not. The
open-PR list is already fetched, so this adds no API calls.

The default branch is read from the workflow_run payload rather than
hard-coded.

Signed-off-by: Alan Clucas <alan@clucas.org>
@Joibel
Joibel marked this pull request as ready for review August 26, 2026 08:21
@Joibel
Joibel requested a review from a team as a code owner August 26, 2026 08:21
@coderabbitai

coderabbitai Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: bcef21e5-bb4d-4d34-b681-25e7fe0c4b67

📥 Commits

Reviewing files that changed from the base of the PR and between 8f0d280 and 0e7f7fd.

📒 Files selected for processing (5)
  • .github/pr-readiness/README.md
  • .github/pr-readiness/classify.ts
  • .github/pr-readiness/main.ts
  • .github/pr-readiness/test/classify.test.ts
  • .github/pr-readiness/types.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

The PR-readiness workflow now reads the repository default branch, resolves stacked PRs that lead to it, and skips PRs targeting release branches or unresolved stacks. Tests cover direct, stacked, forked-head, dead-end, and cyclic cases.

Changes

Default-branch PR eligibility

Layer / File(s) Summary
Stack resolution classifier
.github/pr-readiness/types.ts, .github/pr-readiness/classify.ts, .github/pr-readiness/test/classify.test.ts
Adds StackablePr and targetsDefaultBranch. The classifier follows matching open PR stacks, rejects release-branch endpoints and cycles, and tests these cases.
Workflow eligibility gate
.github/pr-readiness/main.ts, .github/pr-readiness/README.md
Reads repository.default_branch and stops processing PRs that do not target it directly or through an eligible open stack. The README documents the rule.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 0e7f7

This change limits PR-readiness assessments to PRs targeting the default branch while preserving support for stacked PRs, with tests and type checks reported as passing. No actionable merge-blocking risk remains.

Suggested reviewers: isubasinghe

Sequence Diagram(s)

sequenceDiagram
  participant PullRequestEvent
  participant ReadinessWorkflow
  participant targetsDefaultBranch
  PullRequestEvent->>ReadinessWorkflow: provide base branch and default branch
  ReadinessWorkflow->>targetsDefaultBranch: resolve PR stack
  targetsDefaultBranch-->>ReadinessWorkflow: return eligibility
  ReadinessWorkflow-->>PullRequestEvent: continue or stop processing
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 4 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: limiting PR readiness checks to PRs targeting the default branch.
Description check ✅ Passed The description follows the repository template, explains the motivation and modifications, documents verification results, identifies the documentation update, and declares AI use. The feature-specif…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description follows the repository template, explains the motivation and modifications, documents verification results, identifies the documentation update, and declares AI use. The feature-specific issue requirement does not apply to this change.

Full details: Docstring Coverage

Explanation

Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 4 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant