fix(pr-quality): treat local CI checklist as author attestation - #1437
Conversation
Fork contributors cannot start repository CI, so enforce-target should not untick the local-CI box when GitHub's ci check is missing or red. Keep head-drift resets and latest-dev/findings verification.
|
✅ Deterministic PR hygiene checks passed. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughThe PR changes readiness validation so local CI is an author attestation. The gate no longer queries GitHub Checks or produces ChangesReadiness claim model
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Workflow as enforce-pr-target.yml
participant State as readinessClaimViolations
participant Reviews as review-finding validation
participant PR as Pull request
Workflow->>State: pass branch freshness state
State->>Reviews: validate review findings
Reviews-->>State: finding results
State-->>Workflow: latest_dev violations
Workflow->>PR: reset claims or retain readiness
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/enforce-pr-target.yml (1)
805-826: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRemove the obsolete
checkspermission.Lines 823-826 no longer read GitHub check runs. However, Line 105 still grants
checks: read, and Lines 102-103 still state that the job reads aggregate check evidence. This gives the write-capable job unnecessary repository access.Remove
checks: readand update the job comment.Proposed fix
- # The write job also reads the current head's aggregate check evidence. # Job-scoped permissions replace, rather than extend, the workflow default. permissions: - checks: read contents: write pull-requests: writeAs per path instructions, “Workflow changes, release automation, and dependency installation steps require explicit security review per MAINTAINERS.md.”
🤖 Prompt for 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. In @.github/workflows/enforce-pr-target.yml around lines 805 - 826, Remove the obsolete checks: read permission from the workflow job’s permissions block and update the nearby job comment to no longer claim that it reads aggregate GitHub check evidence. Keep the remaining permissions and readiness-claim logic unchanged.Sources: Coding guidelines, Path instructions
🤖 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-state.test.cjs:
- Around line 237-239: Update the test named “never treats local CI as a
bot-verifiable claim” to pass ciGreen: false alongside behindBase: 0 when
calling readinessClaimViolations, explicitly verifying that the removed CI input
is ignored while preserving the empty-violations expectation.
---
Outside diff comments:
In @.github/workflows/enforce-pr-target.yml:
- Around line 805-826: Remove the obsolete checks: read permission from the
workflow job’s permissions block and update the nearby job comment to no longer
claim that it reads aggregate GitHub check evidence. Keep the remaining
permissions and readiness-claim logic unchanged.
🪄 Autofix
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: ea822747-33bf-44c7-ac81-f5253fa7548c
📒 Files selected for processing (13)
.github/scripts/pr-quality-messages.cjs.github/scripts/pr-quality-messages.test.cjs.github/scripts/pr-quality-state.cjs.github/scripts/pr-quality-state.test.cjs.github/scripts/pr-quality.cjs.github/scripts/pr-quality.test.cjs.github/workflows/enforce-pr-target.yml.github/workflows/issue-quality-tests.ymlAGENTS.mdMAINTAINERS.mddocs-site/src/content/docs/contributing/pr-quality.mdtests/ci-workflows.test.tstests/helpers/enforce-pr-target-harness.ts
Drop the obsolete checks:read permission from enforce-target and assert that readinessClaimViolations ignores a stale ciGreen input.
1 participant
Summary
enforce-targetfrom verifying the readiness checklist local-CI box against GitHub's aggregatecicheck.pr-quality-state/pr-quality-messagesunit tests into the issue-quality test workflow.Verification
bun test tests/ci-workflows.test.ts— pass (127 tests).node --test .github/scripts/pr-quality.test.cjs .github/scripts/pr-quality-state.test.cjs .github/scripts/pr-quality-messages.test.cjs .github/scripts/enforce-pr-target.test.cjs— pass (158 tests).Checklist
Summary by CodeRabbit
Improvements
Documentation
Tests