[TRTLLM-12092][infra] Add PR Base Freshness Check Action#13430
[TRTLLM-12092][infra] Add PR Base Freshness Check Action#13430crazydemo merged 1 commit intoNVIDIA:mainfrom
Conversation
📝 WalkthroughWalkthroughIntroduces a new GitHub Actions-based PR base freshness validation system consisting of a Python script that evaluates whether a PR's base branch is sufficiently fresh relative to the PR head by comparing commits-behind count and merge-base age against configured thresholds, and a workflow that orchestrates the check on PR lifecycle events with optional enforcement. Changes
Sequence DiagramsequenceDiagram
actor GHA as GitHub Actions
participant WF as pr-base-freshness.yml
participant GIT as Git
participant PY as pr_base_freshness_check.py
participant ENV as Environment/Report
GHA->>WF: Trigger on PR events
WF->>GIT: Fetch PR head ref
WF->>GIT: Deepen target branch history<br/>(iterative fetch)
WF->>GIT: Compute merge-base
GIT-->>WF: merge-base SHA
WF->>PY: Run script with PR_HEAD_SHA,<br/>TARGET_REF, thresholds
PY->>GIT: Get merge-base commits
GIT-->>PY: Commits between merge-base<br/>and target ref
PY->>PY: Calculate staleness<br/>(commits behind + base age)
PY->>ENV: Write report to<br/>GITHUB_STEP_SUMMARY
PY->>GHA: Exit with status<br/>(0=pass/warn, 1=fail)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/scripts/pr_base_freshness_check.py (1)
28-29: Missing error handling for git command failures.If any
gitcommand fails (e.g., invalid ref, corrupted repo state),subprocess.run(..., check=True)raisesCalledProcessError, which propagates as an unhandled exception with a less informative traceback rather than a clean::error::message.Regarding static analysis hints (S603/S607): These are acceptable here since the inputs are controlled by the workflow environment and
gitis guaranteed to be in PATH on GitHub-hosted runners.♻️ Suggested improvement for cleaner error reporting
def _git(*args: str) -> str: - return subprocess.run(["git", *args], capture_output=True, text=True, check=True).stdout.strip() + result = subprocess.run(["git", *args], capture_output=True, text=True) + if result.returncode != 0: + print(f"::error::git {' '.join(args)} failed: {result.stderr.strip()}") + raise SystemExit(1) + return result.stdout.strip()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/scripts/pr_base_freshness_check.py around lines 28 - 29, Change the helper _git to catch subprocess.CalledProcessError and emit a GitHub Actions friendly error message instead of letting an unhandled traceback percolate: wrap the subprocess.run call in try/except for subprocess.CalledProcessError, and in the except block print a formatted ::error:: line that includes the failed git command (args), the return code, and the captured stderr/stdout for diagnostics, then either re-raise or call sys.exit(1) to fail the workflow cleanly; reference the _git(...) function to locate where to add this handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/scripts/pr_base_freshness_check.py:
- Around line 28-29: Change the helper _git to catch
subprocess.CalledProcessError and emit a GitHub Actions friendly error message
instead of letting an unhandled traceback percolate: wrap the subprocess.run
call in try/except for subprocess.CalledProcessError, and in the except block
print a formatted ::error:: line that includes the failed git command (args),
the return code, and the captured stderr/stdout for diagnostics, then either
re-raise or call sys.exit(1) to fail the workflow cleanly; reference the
_git(...) function to locate where to add this handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 9af26d39-a7b9-4981-81e8-c3260ca70d89
📒 Files selected for processing (2)
.github/scripts/pr_base_freshness_check.py.github/workflows/pr-base-freshness.yml
Post-merge actions required by repo adminsThis PR ships the freshness check in warn-only mode. Merging this PR alone is not enough to block stale-base PRs — the workflow Step 1 (recommended): Observe in warn-only mode for ~2 weeksLet the workflow run as-is. The computed Step 2 (optional): Tune thresholdsThresholds are read from repo-level Actions variables so they can be changed without another PR. Under: Settings → Secrets and variables → Actions →
Leave them unset to use the fallbacks. Step 3: Turn on enforcement (two actions required — both needed)(a) Set the enforcement variable in the same Repository variables page: This flips the workflow exit code from (b) Add the check to branch protection for Settings → Rules → Rulesets → the ruleset targeting (the check must have run at least once before it shows up in the picker — any PR opened after this one merges will suffice.)
RollbackIf enforcement causes unforeseen problems, set |
|
/bot run |
|
PR_Github #45387 [ run ] triggered by Bot. Commit: |
|
PR_Github #45387 [ run ] completed with state
|
|
/bot run |
|
PR_Github #45404 [ run ] triggered by Bot. Commit: |
|
PR_Github #45404 [ run ] completed with state
|
|
/bot run |
|
PR_Github #45470 [ run ] triggered by Bot. Commit: |
|
PR_Github #45470 [ run ] completed with state
|
|
/bot help |
GitHub Bot Help
Provide a user friendly way for developers to interact with a Jenkins server. Run See details below for each supported subcommand. Details
Launch build/test pipelines. All previously running jobs will be killed.
kill
Kill all running builds associated with pull request. skip
Skip testing for latest commit on pull request. reuse-pipeline
Reuse a previous pipeline to validate current commit. This action will also kill all currently running builds associated with the pull request. IMPORTANT NOTE: This is dangerous since lack of user care and validation can cause top of tree to break. |
Add a GitHub Actions workflow that detects PRs whose merge base is too far behind the target branch and fails the check so the author rebases before merging. This catches the "invisible conflict" class of breakage: a PR can merge textually clean yet break main after merge because semantic changes (renamed helper, tightened invariant, replaced registration, etc.) landed on main while the PR was open. The check is lightweight (pure git): compute merge-base, count commits_behind, compute merge-base age, compare to thresholds. Configuration is via repo-level Actions variables so thresholds and enforcement can be tuned without another PR: PR_BASE_FRESHNESS_COMMITS_LIMIT (fallback: 150) PR_BASE_FRESHNESS_AGE_LIMIT_DAYS (fallback: 10) PR_BASE_FRESHNESS_ENFORCE (fallback: false, warn-only) Implementation notes: - Shallow (depth 500) + sparse checkout of the target branch materializes only the check script. Progressive deepening covers very old PRs without paying the cost up front. - refs/pull/N/head is used to reach the PR head commit; no PR-side code is executed, so fork PRs are safe. - auto_merge_enabled is an explicit trigger so a PR that sits in review long enough to go stale cannot auto-merge on a cached green result. - Bootstrap-safe: if the check script is not yet present on the target branch (true for the PR that introduces this workflow), the step exits 0 with a notice instead of failing. Rollout: - Ships in warn-only mode; a stale PR logs ::warning:: and is not blocked. - To enforce: set PR_BASE_FRESHNESS_ENFORCE='true' in repo variables AND add "PR Base Freshness" to main's required status checks under Settings -> Rules. Both steps are needed: exit 1 alone does not block merge without the required-check wiring. Signed-off-by: Ivy Zhang <25222398+crazydemo@users.noreply.github.com>
|
/bot run --skip-test |
|
PR_Github #45635 [ run ] triggered by Bot. Commit: |
|
PR_Github #45635 [ run ] completed with state |
|
/bot reuse-pipeline |
|
PR_Github #45662 [ reuse-pipeline ] triggered by Bot. Commit: |
|
PR_Github #45662 [ reuse-pipeline ] completed with state |
niukuo
left a comment
niukuo
left a comment
There was a problem hiding this comment.
LGTM. Will add PR_BASE_FRESHNESS_COMMITS_LIMIT to variables on demand
4 participants
Summary by CodeRabbit
Release Notes
Description
Test Coverage
PR Checklist
Please review the following before submitting your PR:
PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.
PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.
Test cases are provided for new code paths (see test instructions)
Any new dependencies have been scanned for license and vulnerabilities
CODEOWNERS updated if ownership changes
Documentation updated as needed
Update tava architecture diagram if there is a significant design change in PR.
The reviewers assigned automatically/manually are appropriate for the PR.
Please check this after reviewing the above items as appropriate for this PR.
GitHub Bot Help
To see a list of available CI bot commands, please comment
/bot help.