ci: update NVSkills CI request template and add require-nvskills-status workflow - #1690
Conversation
ramakrishnap-nv
added
non-breaking
📝 WalkthroughWalkthroughThe workflow validates NVSkills requests from pull requests, comments, and signature pushes. It resolves pull-request metadata, checks watched files, and dispatches ChangesNVSkills CI integration
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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.
Actionable comments posted: 5
🧹 Nitpick comments (2)
.github/workflows/request-nvskills-ci.yml (2)
185-188: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd a timeout and retry to the cross-repository dispatch.
The
curlcall has no--max-timeand no--retry. If the connection hangs, the job holds the runner until the workflow limit. If GitHub returns a transient 5xx, the dispatch fails and the requester must comment/nvskills-ciagain.The request is a workflow dispatch, so a retried call only starts a duplicate run at worst. The job concurrency group at line 40 already cancels superseded requests.
♻️ Proposed timeout and retry
- curl -fsSL -X POST \ + curl -fsSL -X POST \ + --max-time 30 \ + --retry 3 --retry-delay 5 --retry-connrefused \ -H "Authorization: Bearer ${DISPATCH_TOKEN}" \ -H "Accept: application/vnd.github+json" \ + -H "X-GitHub-Api-Version: 2022-11-28" \ "https://api.github.com/repos/NVIDIA/nvskills-ci/actions/workflows/nvskills-ci.yml/dispatches" \🤖 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/request-nvskills-ci.yml around lines 185 - 188, Update the curl workflow-dispatch call to include a finite maximum request timeout and retries for transient failures, while preserving its existing authorization, headers, and dispatch endpoint. Configure retry behavior appropriate for transient HTTP errors and ensure the command still fails after exhausting retries.
107-112: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConsider rejecting fork pull requests early.
The step reads
.head.shaand.base.refbut not.head.repo.full_name. The consumer.github/workflows/require-nvskills-status.yml(lines 274-300) states that/nvskills-cidoes not work on forks and asks the author to move the changes to a branch inNVIDIA/skills.For a fork pull request this workflow still dispatches NVSkills CI. The run then fails without a clear reason in this repository.
Read
.head.repo.full_name, compare it toREPO, and write the fork guidance to the step summary before you skip the dispatch.♻️ Proposed fork check
head_sha="$(printf '%s' "${pr_json}" | jq -r '.head.sha')" base_ref="$(printf '%s' "${pr_json}" | jq -r '.base.ref')" + head_repo="$(printf '%s' "${pr_json}" | jq -r '.head.repo.full_name // ""')" + + if [ "${head_repo}" != "${REPO}" ]; then + { + echo "## NVSkills CI request" + echo + echo "Skipped: PR #${pr_number} comes from fork \`${head_repo}\`." + echo "Move the changes to a branch in \`${REPO}\` first." + } >> "${GITHUB_STEP_SUMMARY}" + exit 0 + fi🤖 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/request-nvskills-ci.yml around lines 107 - 112, Update the pull-request metadata handling around pr_json, head_sha, and base_ref to also read .head.repo.full_name and compare it with REPO; when they differ, write the existing fork guidance to the step summary and exit the step before dispatching NVSkills CI, while preserving the current dispatch path for same-repository pull requests.
🤖 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/workflows/request-nvskills-ci.yml:
- Around line 81-89: Update the push title validation in the dispatch step to
use the same prefix-matching rule as the job condition, allowing commit titles
that begin with SIGNATURE_COMMIT_TITLE while retaining the existing actor check
and skip behavior.
- Around line 129-136: Update the jq watched-file check in the workflow
condition to evaluate both .filename and .previous_filename against the existing
skills/, team-skills/, rules/team-rules/, and plugins/ prefixes, matching the
classification used by the downstream status workflow.
- Line 166: Document the NVSkills configuration associated with the workflow:
add documentation for secrets.NVSKILLS_CI_DISPATCH_TOKEN, including the
permissions required to dispatch nvskills-ci.yml in NVIDIA/nvskills-ci, and
document vars.NVSKILLS_SIGNATURE_PUSH_ACTOR and
vars.NVSKILLS_SIGNATURE_COMMIT_TITLE with their default values. Place the
documentation alongside the repository’s existing configuration documentation
and do not change the workflow behavior.
In @.github/workflows/require-nvskills-status.yml:
- Around line 141-161: Replace the paginated pull-request files scan with a
complete commit-range query that detects watched changes across the entire PR,
avoiding GitHub’s files and commits result caps. In the workflow logic
surrounding has_watched_change, verify the complete range was retrieved; if
retrieval fails or is incomplete, write a summary and exit nonzero rather than
approving the PR.
- Around line 56-64: Restrict the bot-managed branch exemption in the workflow’s
HEAD_REF case to pull requests whose head repository matches github.repository
and whose actor is nv-skills-ci[bot]. Update the pull-request files and commits
pagination logic to fail closed when GitHub’s documented 3,000-file or
250-commit response caps are reached, ensuring the required status is not
skipped when watched changes may exist beyond those limits.
---
Nitpick comments:
In @.github/workflows/request-nvskills-ci.yml:
- Around line 185-188: Update the curl workflow-dispatch call to include a
finite maximum request timeout and retries for transient failures, while
preserving its existing authorization, headers, and dispatch endpoint. Configure
retry behavior appropriate for transient HTTP errors and ensure the command
still fails after exhausting retries.
- Around line 107-112: Update the pull-request metadata handling around pr_json,
head_sha, and base_ref to also read .head.repo.full_name and compare it with
REPO; when they differ, write the existing fork guidance to the step summary and
exit the step before dispatching NVSkills CI, while preserving the current
dispatch path for same-repository pull requests.
🪄 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: CHILL
Plan: Enterprise
Run ID: 1fd9ae10-99f8-4638-b5b4-d52efc092269
📒 Files selected for processing (2)
.github/workflows/request-nvskills-ci.yml.github/workflows/require-nvskills-status.yml
| - name: Dispatch NVSkills CI | ||
| if: steps.context.outputs.should_dispatch == 'true' | ||
| env: | ||
| DISPATCH_TOKEN: ${{ secrets.NVSKILLS_CI_DISPATCH_TOKEN }} |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether the new secret and vars are documented anywhere in the repo.
set -euo pipefail
for name in NVSKILLS_CI_DISPATCH_TOKEN NVSKILLS_SIGNATURE_PUSH_ACTOR NVSKILLS_SIGNATURE_COMMIT_TITLE; do
echo "=== ${name} ==="
rg -n --hidden -g '!.git' "${name}" || echo " (no references)"
done
echo "=== docs/README mentions of nvskills ==="
rg -ni --hidden -g '!.git' -g '*.md' 'nvskills' || echo " (no markdown references)"Repository: NVIDIA/cuopt
Length of output: 317
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== workflow context ==="
sed -n '130,185p' .github/workflows/request-nvskills-ci.yml
echo "=== workflow references and repository documentation files ==="
rg -n --hidden -g '!.git' \
'NVSKILLS|repository_dispatch|workflow_dispatch|DISPATCH_TOKEN|SIGNATURE_PUSH|SIGNATURE_COMMIT' \
.github README* docs 2>/dev/null || true
echo "=== tracked documentation/configuration files ==="
git ls-files '*.md' '*.yaml' '*.yml' '*.json' | sed -n '1,160p'
echo "=== available repository changes ==="
git diff --stat
git diff -- .github/workflows/request-nvskills-ci.ymlRepository: NVIDIA/cuopt
Length of output: 10741
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== complete dispatch step ==="
sed -n '1,115p' .github/workflows/request-nvskills-ci.yml
sed -n '160,225p' .github/workflows/request-nvskills-ci.yml
echo "=== all documentation/configuration references to related setup terms ==="
rg -ni --hidden -g '!.git' -g '*.md' -g '*.yaml' -g '*.yml' \
'NVSkills|nvskills-ci|repository dispatch|repository_dispatch|workflow dispatch|workflow_dispatch|fine-grained|contents: write|actions: write|personal access token|PAT|secret|vars\.' \
.github README.md CONTRIBUTING.md CONVENTIONS.md docs 2>/dev/null || trueRepository: NVIDIA/cuopt
Length of output: 34831
🌐 Web query:
GitHub REST API create a repository dispatch event fine-grained token required permissions Actions workflows NVIDIA nvskills-ci
💡 Result:
To create a repository dispatch event using the GitHub REST API, you must send a POST request to the /repos/{owner}/{repo}/dispatches endpoint [1][2]. When using a fine-grained personal access token (PAT), the following permissions are required on the target repository: - Contents: Read and write [3][4][5] - Metadata: Read-only (this is automatically selected when you select Contents: Read and write) [3][4] If your workflow also needs to discover the Run ID of the dispatched workflow, you must additionally include Actions: read permission [5]. Important Considerations: - Repository dispatch events will only trigger a workflow run if the workflow file is committed to the default branch of the repository [4][6]. - While a classic PAT requires the full repo scope [1][7][3], fine-grained PATs allow for more limited, repository-specific access [3][8]. - If you are dispatching a workflow within the same repository that is executing the action, you can use the default GITHUB_TOKEN [3][4]. For cross-repository dispatching, you must use a PAT or a GitHub App installation token [3][9][5].
Citations:
- 1: https://docs.github.com/en/rest/repos/repos?api=
- 2: https://docs.github.com/rest/repos/repos
- 3: https://github.com/peter-evans/repository-dispatch/
- 4: https://github.com/peter-evans/repository-dispatch?tab=readme-ov-file
- 5: https://github.com/step-security/dispatch-workflow
- 6: https://github.com/marketplace/actions/repository-dispatch
- 7: https://dev.to/ngnijland/how-to-trigger-a-github-action-workflow-in-another-repository-k3k
- 8: https://www.eliostruyf.com/dispatch-github-action-fine-grained-personal-access-token/
- 9: https://docs.github.com/en/actions/how-tos/write-workflows/choose-when-workflows-run/trigger-a-workflow
🌐 Web query:
site:docs.github.com/en/rest/actions/workflows "Create a workflow dispatch event" fine-grained access tokens Actions write
💡 Result:
To trigger a workflow dispatch event using the GitHub REST API, you must use a token that has the repository scope [1]. Official documentation indicates that personal access tokens (classic) require the repo scope to interact with the Create a workflow dispatch event endpoint [1]. While fine-grained personal access tokens allow for more granular control, the current documentation for the workflow dispatch endpoint specifically emphasizes the necessity of the repo scope for classic tokens [1]. Fine-grained tokens are generally managed through specific repository permissions; if a fine-grained token does not support the workflow dispatch event, it is because the required Actions-related permissions for that specific endpoint are not mapped to that token type. In such cases, a classic personal access token with the repo scope remains the standard requirement for programmatically triggering workflows [1]. To trigger the event via the API, you would typically use a request similar to the following: POST /repos/{owner}/{repo}/actions/workflows/{workflow_id}/dispatches The request body must include the ref (the git reference, such as a branch or tag) and any required inputs defined in your workflow [1].
Citations:
Document the NVSkills configuration
Document secrets.NVSKILLS_CI_DISPATCH_TOKEN and its required permissions for dispatching nvskills-ci.yml in NVIDIA/nvskills-ci. Document vars.NVSKILLS_SIGNATURE_PUSH_ACTOR and vars.NVSKILLS_SIGNATURE_COMMIT_TITLE, including their default values.
🤖 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/request-nvskills-ci.yml at line 166, Document the NVSkills
configuration associated with the workflow: add documentation for
secrets.NVSKILLS_CI_DISPATCH_TOKEN, including the permissions required to
dispatch nvskills-ci.yml in NVIDIA/nvskills-ci, and document
vars.NVSKILLS_SIGNATURE_PUSH_ACTOR and vars.NVSKILLS_SIGNATURE_COMMIT_TITLE with
their default values. Place the documentation alongside the repository’s
existing configuration documentation and do not change the workflow behavior.
Source: Path instructions
| case "${HEAD_REF}" in | ||
| automated/sync-skills|bot/regenerate-skill-metadata) | ||
| append_summary \ | ||
| "## NVSkills CI required status" \ | ||
| "" \ | ||
| "Skipped: bot-managed branch \`${HEAD_REF}\` is exempt from this check." | ||
| exit 0 | ||
| ;; | ||
| esac |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
file=".github/workflows/require-nvskills-status.yml"
wc -l "$file"
cat -n "$file" | sed -n '1,220p'
printf '\n--- workflow references ---\n'
rg -n '(^|[[:space:]])(run:|uses:|needs:|secrets:|env:)' "$file"
printf '\n--- repository context ---\n'
rg -n 'nv-skills-ci|automated/sync-skills|bot/regenerate-skill-metadata|require-nvskills-status|list-pull-request-files|list-pull-request-commits' .github README.md 2>/dev/null || trueRepository: NVIDIA/cuopt
Length of output: 10557
🏁 Script executed:
#!/bin/bash
set -e
file=".github/workflows/request-nvskills-ci.yml"
wc -l "$file"
cat -n "$file" | sed -n '1,130p'
printf '\n--- all workflow callers and actor configuration ---\n'
rg -n -C 4 'require-nvskills-status|SIGNATURE_PUSH_ACTOR|NVSKILLS_SIGNATURE_PUSH_ACTOR|nv-skills-ci\[bot\]|pull_request_target|pull_request:' .github README.md . 2>/dev/null | head -300
printf '\n--- branch names and bot identity ---\n'
rg -n -C 3 'automated/sync-skills|bot/regenerate-skill-metadata|nv-skills-ci' . ':!*.lock' 2>/dev/null | head -300Repository: NVIDIA/cuopt
Length of output: 11083
🌐 Web query:
GitHub REST API pull request files commits pagination maximum results list pull request files list pull request commits documentation
💡 Result:
For both the List pull requests files and List commits on a pull request endpoints, pagination is supported using the per_page and page query parameters [1][2]. 1. Pagination Limits: For most GitHub REST API endpoints, the maximum value for the per_page parameter is 100 [3][4]. If you request a value higher than 100, the API will automatically cap it at the maximum without returning an error [3][4]. You can control pagination by adjusting the per_page (results per page, default 30) and page (page number, default 1) parameters [1][2]. 2. Specific Endpoint Constraints: - List pull requests files (GET /repos/{owner}/{repo}/pulls/{pull_number}/files): Responses are limited to a maximum of 3,000 files [1][2]. - List commits on a pull request (GET /repos/{owner}/{repo}/pulls/{pull_number}/commits): This endpoint is limited to a maximum of 250 commits [5][2]. To retrieve a complete list of commits for pull requests exceeding 250, GitHub recommends using the List commits endpoint (GET /repos/{owner}/{repo}/commits) instead [5][2]. To fetch all results, you should inspect the Link header in the API response, which provides the URLs for subsequent pages [3][4]. Using official client libraries like Octokit (e.g., octokit.paginate) is recommended to handle the pagination logic automatically [3][4].
Citations:
- 1: https://docs.github.com/en/rest/pulls/pulls
- 2: https://docs.github.com/en/rest/pulls/pulls?apiVersion=2026-03-10
- 3: https://docs.github.com/en/rest/using-the-rest-api/using-pagination-in-the-rest-api?apiVersion=2026-03-10
- 4: https://docs.github.com/rest/using-the-rest-api/using-pagination-in-the-rest-api
- 5: https://docs.github.com/en/enterprise-cloud@latest/rest/pulls/pulls
🏁 Script executed:
#!/bin/bash
set -e
python3 - <<'PY'
PAGE_SIZE = 100
def fetched_pages(total_returned_by_api):
pages = []
page = 1
while True:
start = (page - 1) * PAGE_SIZE
current = max(0, min(PAGE_SIZE, total_returned_by_api - start))
pages.append(current)
if current < PAGE_SIZE:
return pages
page += 1
for name, api_cap in (("PR files", 3000), ("PR commits", 250)):
pages = fetched_pages(api_cap)
print(f"{name}: pages={len(pages)}, lengths={pages}, fetched={sum(pages)}")
print(f"{name}: item after API cap is unavailable to this workflow")
print("\nExemption predicate examples:")
cases = [
("automated/sync-skills", "NVIDIA/cuopt", "nv-skills-ci[bot]"),
("automated/sync-skills", "attacker/fork", "nv-skills-ci[bot]"),
("automated/sync-skills", "NVIDIA/cuopt", "contributor"),
("feature", "NVIDIA/cuopt", "nv-skills-ci[bot]"),
]
for head_ref, head_repo, actor in cases:
exempt = (
head_repo == "NVIDIA/cuopt"
and actor == "nv-skills-ci[bot]"
and head_ref in {"automated/sync-skills", "bot/regenerate-skill-metadata"}
)
print(f"head_ref={head_ref!r}, head_repo={head_repo!r}, actor={actor!r} => exempt={exempt}")
PY
printf '\n--- exact relevant workflow lines ---\n'
cat -n .github/workflows/require-nvskills-status.yml | sed -n '26,66p;141,215p'Repository: NVIDIA/cuopt
Length of output: 6229
🌐 Web query:
site:docs.github.com GitHub pull_request event github.actor head.repo.full_name head.ref pull request webhook payload
💡 Result:
In a GitHub pull_request webhook payload, the fields you requested are located within the nested structure of the pull request object [1][2]. The payload root does not contain github.actor directly, but the user who performed the action triggering the webhook is typically found in the sender field at the root of the JSON payload. The specific details regarding the head of the pull request are located inside the pull_request object [3]. The hierarchy is as follows: 1. github.actor (Action Triggerer): Found at the root level of the payload as sender.login [3]. 2. head.repo.full_name: Located at payload.pull_request.head.repo.full_name [4][3]. 3. head.ref: Located at payload.pull_request.head.ref [4]. In summary, to access these values in a webhook receiver, you would typically reference them as follows: - Actor: payload['sender']['login'] [3] - Head Repository Full Name: payload['pull_request']['head']['repo']['full_name'] [3] - Head Reference (Branch Name): payload['pull_request']['head']['ref'] [4] These fields are standard parts of the pull_request event payload sent by GitHub [1][2]. Always ensure you are parsing the incoming JSON payload correctly before accessing these nested keys [3].
Citations:
- 1: https://docs.github.com/en/webhooks/webhook-events-and-payloads
- 2: https://docs.github.com/en/enterprise-server@3.21/webhooks/webhook-events-and-payloads
- 3: https://docs.github.com/en/rest/guides/delivering-deployments?apiVersion=2026-03-10
- 4: https://docs.github.com/en/actions/reference/security/securely-using-pull_request_target
Restrict the branch exemption to the trusted repository and actor.
A fork can use automated/sync-skills or bot/regenerate-skill-metadata and bypass the required status. Require github.event.pull_request.head.repo.full_name == github.repository and github.actor == 'nv-skills-ci[bot]' before this exemption.
Fail closed when the pull-request files or commits exceed GitHub’s documented response caps of 3,000 files or 250 commits. The current pagination loops can miss watched changes beyond those caps and then skip the required status.
🤖 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/require-nvskills-status.yml around lines 56 - 64, Restrict
the bot-managed branch exemption in the workflow’s HEAD_REF case to pull
requests whose head repository matches github.repository and whose actor is
nv-skills-ci[bot]. Update the pull-request files and commits pagination logic to
fail closed when GitHub’s documented 3,000-file or 250-commit response caps are
reached, ensuring the required status is not skipped when watched changes may
exist beyond those limits.
| has_watched_change=false | ||
| page=1 | ||
| while true; do | ||
| files_json="$(github_get "https://api.github.com/repos/${owner}/${repo}/pulls/${pr_number}/files?per_page=100&page=${page}")" | ||
| if printf '%s' "${files_json}" | jq -e ' | ||
| def watched: | ||
| (. // "") | | ||
| startswith("skills/") or | ||
| startswith("team-skills/") or | ||
| startswith("rules/team-rules/") or | ||
| startswith("plugins/"); | ||
| any(.[]; (.filename | watched) or (.previous_filename? | watched)) | ||
| ' >/dev/null; then | ||
| has_watched_change=true | ||
| break | ||
| fi | ||
| if [ "$(printf '%s' "${files_json}" | jq 'length')" -lt 100 ]; then | ||
| break | ||
| fi | ||
| page=$((page + 1)) | ||
| done |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
file=".github/workflows/require-nvskills-status.yml"
printf '%s\n' "== workflow structure =="
sed -n '1,230p' "$file"
printf '%s\n' "== referenced helpers and endpoints =="
rg -n -C 5 'github_get|/pulls/.*/files|/pulls/.*/commits|has_watched_change|watched|nvskills' "$file"
printf '%s\n' "== repository state and workflow presence =="
git status --short
git ls-files "$file"Repository: NVIDIA/cuopt
Length of output: 18590
🌐 Web query:
GitHub REST API pull request files endpoint 3000 files pull request commits endpoint 250 commits pagination documentation
💡 Result:
The GitHub REST API enforces specific limitations and pagination behaviors for pull request files and commits. Pull Request Files The List pull requests files endpoint (GET /repos/{owner}/{repo}/pulls/{pull_number}/files) returns a maximum of 3,000 files [1][2]. By default, the API returns 30 files per page, but you can adjust this using the per_page query parameter, which accepts a maximum value of 100 [1][3]. Pull Request Commits The List commits on a pull request endpoint (GET /repos/{owner}/{repo}/pulls/{pull_number}/commits) is limited to returning a maximum of 250 commits [4][5]. If a pull request contains more than 250 commits, you must use the List commits on a repository endpoint (GET /repos/{owner}/{repo}/commits) to retrieve the complete list [4][6][5]. General Pagination Most GitHub REST API endpoints support pagination via the per_page and page query parameters [3][7]. For endpoints that support these parameters, the per_page value is capped at 100; requesting a value higher than this will result in the API automatically defaulting to the maximum of 100 without returning an error [3][8]. You can navigate through paginated results using the Link header provided in the API response or by using the octokit.paginate method if working with the GitHub Octokit library [3][9][10].
Citations:
- 1: https://docs.github.com/en/rest/pulls/pulls?apiVersion=2026-03-10
- 2: https://docs.github.com/en/rest/pulls/pulls
- 3: https://docs.github.com/rest/using-the-rest-api/using-pagination-in-the-rest-api
- 4: https://github.com/github/developer.github.com/blob/master/content/v3/pulls.md
- 5: https://docs.github.com/en/enterprise-cloud@latest/rest/pulls/pulls
- 6: https://stackoverflow.com/questions/63584999/list-commits-on-a-pull-request
- 7: https://docs.github.com/en/rest/using-the-rest-api/getting-started-with-the-rest-api?tool=cli
- 8: https://docs.github.com/en/rest/using-the-rest-api/using-pagination-in-the-rest-api?apiVersion=2026-03-10
- 9: https://github.com/github/docs/blob/main/content/rest/using-the-rest-api/using-pagination-in-the-rest-api.md
- 10: https://docs.github.com/en/enterprise-cloud@latest/rest/guides/scripting-with-the-rest-api-and-javascript?apiVersion=2022-11-28
🏁 Script executed:
#!/bin/bash
set -eu
file=".github/workflows/require-nvskills-status.yml"
printf '%s\n' "== remaining workflow logic =="
sed -n '217,330p' "$file"
printf '%s\n' "== all workflow files and callers =="
git ls-files '.github/workflows'
rg -n -C 4 'require-nvskills-status|workflow_call|pull_request_target|pull_request:' .github/workflows
printf '%s\n' "== pagination-related repository guidance =="
rg -n -i -C 3 '3000|250 commits|pulls/.*/files|pulls/.*/commits|compare/.*\\.\\.\\.|commit range' .github README.md 2>/dev/null || trueRepository: NVIDIA/cuopt
Length of output: 12820
🌐 Web query:
site:docs.github.com REST "List commits on a pull request" chronological order 250 commits
💡 Result:
To list commits on a GitHub pull request, use the endpoint: GET /repos/{owner}/{repo}/pulls/{pull_number}/commits [1][2]. Regarding your requirements: 1. Chronological Order: The GitHub REST API returns commits for a pull request in chronological order (the order they were created) [3][4]. 2. 250 Commits: You cannot request 250 commits in a single page request. The per_page parameter has a maximum value of 100 [1][5][6]. To retrieve more than 100 commits, you must use pagination (e.g., requesting page 1 with per_page=100, then page 2, and so on) using the Link header provided in the API response [6]. 3. Limit Note: While the endpoint documentation mentions it can list a maximum of 250 commits in a single response for some internal contexts, the API explicitly restricts the per_page query parameter to a maximum of 100 for standard pagination [1][5][6]. For pull requests with more than 250 commits, you should paginate through the results to ensure you retrieve the complete list [1].
Citations:
- 1: https://docs.github.com/en/rest/pulls/pulls
- 2: https://docs.github.com/en/rest/pulls/pulls?apiVersion=2026-03-10
- 3: https://docs.github.com/en/rest/commits/commits
- 4: https://docs.github.com/en/rest/commits/commits?apiVersion=2026-03-10
- 5: https://docs.github.com/en/enterprise-cloud@latest/rest/pulls/pulls
- 6: https://docs.github.com/en/rest/using-the-rest-api/using-pagination-in-the-rest-api
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
# Read-only model of the pagination conditions in
# .github/workflows/require-nvskills-status.yml.
def pages_for_files(total_files, api_cap=3000, per_page=100):
visible = min(total_files, api_cap)
pages = []
offset = 0
while True:
page = min(per_page, max(visible - offset, 0))
pages.append(page)
if page < per_page:
return pages
offset += per_page
def pages_for_commits(total_commits, api_cap=250, per_page=100):
visible = min(total_commits, api_cap)
pages = []
offset = 0
while True:
page = min(per_page, max(visible - offset, 0))
pages.append(page)
if page < per_page:
return pages
offset += per_page
for total in (3000, 3001, 3100):
print(f"files total={total}: pages={pages_for_files(total)}")
for total in (250, 251, 300, 301):
print(f"commits total={total}: pages={pages_for_commits(total)}")
print("At the API caps, the loops stop without an explicit completeness check.")
PYRepository: NVIDIA/cuopt
Length of output: 916
Fail closed when GitHub truncates pull-request results.
The files endpoint is capped at 3,000 files, and the commits endpoint is capped at 250 commits. These loops stop on a short page and can miss later watched changes. Use a complete commit-range query. If the complete range cannot be retrieved, write a summary and exit nonzero.
🤖 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/require-nvskills-status.yml around lines 141 - 161,
Replace the paginated pull-request files scan with a complete commit-range query
that detects watched changes across the entire PR, avoiding GitHub’s files and
commits result caps. In the workflow logic surrounding has_watched_change,
verify the complete range was retrieved; if retrieval fails or is incomplete,
write a summary and exit nonzero rather than approving the PR.
CI Test Summary⏭️ All 5 test job(s) skipped. |
…ls-status from NVIDIA/skills
jameslamb
left a comment
jameslamb
left a comment
There was a problem hiding this comment.
Looks fine, do what you need to. I left some small suggestions.
This is also quite a lot of shell code to have in workflow files... IMO it'd be better in scripts in the repo, where it'd go through shellcheck and other linting.
| role_name="$(curl -fsSL \ | ||
| -H "Authorization: Bearer ${GH_TOKEN}" \ | ||
| -H "Accept: application/vnd.github+json" \ | ||
| "https://api.github.com/repos/${REPO}/collaborators/${ACTOR}/permission" \ | ||
| | jq -r '.role_name // ""')" |
There was a problem hiding this comment.
You might find it easier to use gh api with with the GitHub CLI than this curl. That'd keep GH_TOKEN out of the code entirely (gh would just silently read the variable and use it).
| curl -fsSL -X POST \ | ||
| -H "Authorization: Bearer ${DISPATCH_TOKEN}" \ | ||
| -H "Accept: application/vnd.github+json" \ | ||
| "https://api.github.com/repos/NVIDIA/nvskills-ci/actions/workflows/nvskills-ci.yml/dispatches" \ |
There was a problem hiding this comment.
The GitHub API has a gh workflow run which would be simpler than curl-ing against api.github.com, I think you should consider that.
|
/merge |
2 participants
Summary
Updates `.github/workflows/request-nvskills-ci.yml` to the latest NVSkills CI request template as requested by the NVSkills team.
Changes vs the old template:
Testing
Pre-commit passes (yamllint, zizmor, verify-copyright).
Docs
No doc changes needed.