docs: require GitHub issue references for PRs#287
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe changes align repository guidance, the PR template, workflow configuration, and ChangesIssue Reference Policy
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant PullRequest
participant PRIssueWorkflow
participant CheckPRIssue
participant GitHubIssuesAPI
PullRequest->>PRIssueWorkflow: provide PR body
PRIssueWorkflow->>CheckPRIssue: run with PR_BODY
CheckPRIssue->>CheckPRIssue: parse visible Issues section
CheckPRIssue->>GitHubIssuesAPI: validate NVIDIA/nvcf issue
GitHubIssuesAPI-->>CheckPRIssue: issue or pull request response
CheckPRIssue-->>PRIssueWorkflow: pass or fail
Possibly related issues
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@AGENTS.md`:
- Around line 256-260: Update ci/check-pr-issue.sh and its PR-body reference
validation to accept only NVIDIA/nvcf issue references, including the
repository-qualified form, and reject arbitrary cross-repository references;
additionally validate that the referenced NVIDIA/nvcf issue exists before
allowing the check to pass.
In `@ci/check-pr-issue.sh`:
- Around line 55-57: Update the PR validation logic in ci/check-pr-issue.sh to
inspect PR_BODY alone when enforcing the required issue-reference policy, rather
than allowing PR_TITLE content to satisfy the check. Preserve the existing
accepted reference formats and ensure the reference is validated within the PR
body’s ## Issues section.
- Around line 12-13: Update the validation logic in ci/check-pr-issue.sh so
NO-REF and tracker-key bypasses cannot succeed based solely on visible text.
Require a trusted maintainer approval signal, such as an approved label or
maintainer review, and private-context authorization where applicable; otherwise
remove these automatic-success escape hatches. Ensure the comments describing
lines 12-13 and 32 accurately reflect the enforced behavior.
🪄 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: CHILL
Plan: Enterprise
Run ID: a496d4a0-ead1-43ed-9b1f-4ead2bbdc9f9
📒 Files selected for processing (4)
.github/PULL_REQUEST_TEMPLATE.md.github/workflows/pr-issue-check.ymlAGENTS.mdci/check-pr-issue.sh
3b83773 to
1182853
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@ci/check-pr-issue.sh`:
- Around line 41-42: Update the PR reference regex in the visible grep condition
to accept only valid NVIDIA/nvcf issue references, rejecting arbitrary
repositories. Add word boundaries around the closing/fixing/resolving/relating
keyword and the issue number, while preserving support for the accepted keyword
variants and NVIDIA/nvcf#123 format.
🪄 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: CHILL
Plan: Enterprise
Run ID: 43d073b4-7bfb-4b3a-a907-07b737dc9287
📒 Files selected for processing (4)
.github/PULL_REQUEST_TEMPLATE.md.github/workflows/pr-issue-check.ymlAGENTS.mdci/check-pr-issue.sh
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/pr-issue-check.yml
- AGENTS.md
1182853 to
404fc5d
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@AGENTS.md`:
- Around line 257-260: Align the PR guidance in the Issues section with the
formats accepted by pr-issue-check and ci/check-pr-issue.sh: either document
standalone NVIDIA/nvcf#123 references alongside the action-keyword examples, or
update the checker to reject them. Keep the documented and enforced formats
consistent.
- Around line 255-272: Keep AGENTS.md below 400 lines by consolidating or
removing redundant existing guidance before retaining the new GitHub
issue-linking policy. Ensure the final file remains under the mandated limit and
preserves the required issue-reference behavior.
In `@ci/check-pr-issue.sh`:
- Around line 75-84: Update ci/check-pr-issue.sh to recognize NO-REF only when a
trusted maintainer-approved waiver is validated; otherwise retain the existing
issue-reference failure. Update .github/workflows/pr-issue-check.yml lines 40-43
to provide the trusted approval input or PR identity required for validation.
Update AGENTS.md lines 267-270 to document the approval procedure and state that
ordinary pull requests still require an issue reference.
🪄 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: CHILL
Plan: Enterprise
Run ID: 6cd4ffb8-4bc5-4d53-8495-91c998849cb0
📒 Files selected for processing (4)
.github/PULL_REQUEST_TEMPLATE.md.github/workflows/pr-issue-check.ymlAGENTS.mdci/check-pr-issue.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/PULL_REQUEST_TEMPLATE.md
1b1ea2d to
3245cc7
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
ci/check-pr-issue.sh (1)
118-133: 🩺 Stability & Availability | 🔵 TrivialInstrument this only if CI can carry a Python OTel dependency path. This helper is a stdlib-only Python snippet, so adding spans here would introduce a new package dependency for a single GitHub API request.
🤖 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 `@ci/check-pr-issue.sh` around lines 118 - 133, Leave the GitHub API validation loop unchanged and do not add OpenTelemetry instrumentation or dependencies to this stdlib-only helper. Only instrument this flow if CI already provides a supported Python OTel dependency path; otherwise preserve the existing urllib-based behavior and error handling.Source: Coding guidelines
🤖 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 `@ci/check-pr-issue.sh`:
- Around line 20-147: Add committed automated regression tests for the policy
logic implemented in the embedded Python check, covering HTML comment stripping,
## Issues section boundaries, rejected repository and tracker references, NO-REF
handling, and GitHub API response cases. Use the repository’s native test runner
and record the result; if testing is genuinely inapplicable, document that
rationale in the pull request instead.
- Around line 63-73: Extend the validation around disallowed_tracker in the PR
issue-check logic to detect private URLs in section, not only selected tracker
ID formats. Reject any URL unless it matches an explicitly permitted public
issue URL pattern, using the same error message and exit behavior for
violations.
- Line 39: Update the HTML-comment removal in the visible-body processing around
re.sub so comments beginning with <!-- are removed through the closing --> or,
when unterminated, through the end of body. Preserve removal of normal comments
while ensuring hidden issue references cannot remain in visible.
---
Nitpick comments:
In `@ci/check-pr-issue.sh`:
- Around line 118-133: Leave the GitHub API validation loop unchanged and do not
add OpenTelemetry instrumentation or dependencies to this stdlib-only helper.
Only instrument this flow if CI already provides a supported Python OTel
dependency path; otherwise preserve the existing urllib-based behavior and error
handling.
🪄 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: CHILL
Plan: Enterprise
Run ID: e7c531cb-48a0-487d-9046-318713e05b4e
📒 Files selected for processing (4)
.github/PULL_REQUEST_TEMPLATE.md.github/workflows/pr-issue-check.ymlAGENTS.mdci/check-pr-issue.sh
🚧 Files skipped from review as they are similar to previous changes (3)
- .github/PULL_REQUEST_TEMPLATE.md
- AGENTS.md
- .github/workflows/pr-issue-check.yml
Refs: #286 Signed-off-by: Kristina Pathak <kristina.m.pathak@gmail.com>
3245cc7 to
add48a6
Compare
TL;DR
NVIDIA/nvcfissue for GitHub PRs.NO-REFaccepted by the gate only as a last resort.Additional Details
AGENTS.mdwith concise GitHub issue guidance and removes public references to internal repo names.## Issues.For the Reviewer
Start with the
Merge Requests/Pull Requestssection inAGENTS.md.For QA
git diff --checkbash -n ci/check-pr-issue.sh ci/test/check-pr-issue.test.shci/test/check-pr-issue.test.shci/test/check-conventional-title.test.shgrep -n "nvcf/nvcf-internal\|For non-GitHub" AGENTS.mdreturns no matcheswc -l AGENTS.md-> 391ruby -e 'require "yaml"; YAML.load_file(".github/workflows/pr-issue-check.yml")'Issues
Closes #286
Checklist