From add48a65554aa63e2dd1978f915d880e776c5daa Mon Sep 17 00:00:00 2001 From: Kristina Pathak Date: Mon, 20 Jul 2026 23:02:25 -0700 Subject: [PATCH] docs: require GitHub issue references for PRs Refs: NVIDIA/nvcf#286 Signed-off-by: Kristina Pathak --- .github/PULL_REQUEST_TEMPLATE.md | 13 +- .github/workflows/pr-issue-check.yml | 19 +-- AGENTS.md | 55 +++----- ci/check-pr-issue.sh | 186 +++++++++++++++++++++------ ci/test/check-pr-issue.test.sh | 157 ++++++++++++++++++++++ 5 files changed, 341 insertions(+), 89 deletions(-) create mode 100755 ci/test/check-pr-issue.test.sh diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 2527cd481..aa3df6ec8 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -18,13 +18,12 @@ ## Issues diff --git a/.github/workflows/pr-issue-check.yml b/.github/workflows/pr-issue-check.yml index 810e4ed41..5b68dbbc0 100644 --- a/.github/workflows/pr-issue-check.yml +++ b/.github/workflows/pr-issue-check.yml @@ -1,15 +1,15 @@ # SPDX-FileCopyrightText: Copyright (c) NVIDIA CORPORATION & AFFILIATES. All rights reserved. # SPDX-License-Identifier: Apache-2.0 # -# Require every pull request to reference an associated issue (or the NO-REF -# escape hatch). Make it a required status check on main via branch protection -# to actually block merge. +# Require every pull request to reference an associated GitHub issue or the +# NO-REF last-resort value. Make this a required status check on main via branch +# protection to actually block merge. # # Trigger is pull_request_target (not pull_request) on purpose: the workflow # and the checked-out ci/check-pr-issue.sh both come from the BASE branch, so a # fork PR cannot swap the script to `exit 0` and bypass the gate. This is safe -# because we run NO code from the PR head and only read the PR title/body from -# the event payload; the token is restricted to contents: read. +# because we run NO code from the PR head and only read the PR body from the +# event payload; the token is restricted to read-only permissions. name: pr-issue-check @@ -20,6 +20,8 @@ on: permissions: contents: read + issues: read + pull-requests: read concurrency: group: pr-issue-${{ github.event.pull_request.number }} @@ -34,9 +36,10 @@ jobs: # main's ci/check-pr-issue.sh, not the PR author's version. - uses: actions/checkout@v4 - name: Check for an associated issue - # PR title/body passed via env (not inline ${{ }}) to avoid script - # injection from the attacker-controlled PR body. + # PR body passed via env (not inline ${{ }}) to avoid script injection + # from attacker-controlled PR text. env: - PR_TITLE: ${{ github.event.pull_request.title }} PR_BODY: ${{ github.event.pull_request.body }} + GITHUB_TOKEN: ${{ github.token }} + GITHUB_REPOSITORY: ${{ github.repository }} run: ./ci/check-pr-issue.sh diff --git a/AGENTS.md b/AGENTS.md index d7cbcc810..37b89e3e0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -40,8 +40,6 @@ For self-managed stack ownership, deployment order, chart/image-source mapping, or "which subtree owns this" questions, use: - `.cursor/skills/nvcf-explore-stack/SKILL.md` for the in-repo Helmfile, dependency, chart, hook, and image-source map -- the `nvcf/nvcf-internal` repository when private workspace routing metadata - is relevant Do not copy workspace routing boilerplate into subtree `AGENTS.md` files. Local guidance should only record subtree-specific ownership exceptions, adjacent @@ -57,10 +55,9 @@ names, registry endpoints, vault endpoints, or debugging context that external readers cannot access. Keep private context in the internal Merge Request/Pull Request description -outside the public commit section, in the `nvcf/nvcf-internal` repository, or -in a non-allowlisted runbook. If a change requires public wording, generalize -it to the user-visible behavior and remove the private evidence trail from the -allowlisted file. +outside the public commit section or in a non-allowlisted runbook. If a change +requires public wording, generalize it to the user-visible behavior and remove +the private evidence trail from the allowlisted file. ## Local QA and Testing Environment Safety @@ -71,7 +68,6 @@ local artifact state, then ask the user whether cleanup or a net-new isolated environment is appropriate. Never delete clusters, Helm releases, worktrees, secrets, or artifact directories without explicit user confirmation. -Use the `nvcf/nvcf-internal` repository for detailed internal k3d workflows. If creating a net-new environment, use unique cluster names, ports, Helmfile environments, secrets files, CLI configs, and artifact directories. @@ -144,15 +140,15 @@ Skills are split by visibility and audience: - `ai-tooling/user/skills/`: public user-facing NVCF skills. - `ai-tooling/dev/skills/`: public developer workflow skills. -- Private skill source trees follow the same `user/skills/` and `dev/skills/` - split in the `nvcf/nvcf-internal` repository. +- Private skill source trees use the same `user/skills/` and `dev/skills/` + split outside this public snapshot. - `.cursor/skills/`: root dev-skill fanout only. Each entry is a symlink to a public dev skill source directory in this repository. -Cross-tool symlinks make root dev skills available to all agents. The root fanout directories must contain symlinks only, never source skill directories or regular skill files: -- `.cursor/skills/` -> symlink to the dev skill source directory. -- `.codex/skills/` -> symlink to the same dev skill source directory. -- `.claude/skills/` -> symlink to the same dev skill source directory. +Cross-tool symlinks make root dev skills available to all agents. The root +fanout directories (`.cursor/skills/`, `.codex/skills/`, `.claude/skills/`) +must contain symlinks only, never source skill directories or regular skill +files. The project hook `.cursor/hooks/validate-skill-fanout.py` audits this before an agent finishes. If it reports a fanout error, fix the symlinks or source placement before responding. @@ -179,23 +175,12 @@ is stale. Do not edit this file by hand. ### Where hooks live -Hooks follow the same source-and-fanout pattern as root dev skills. The root -hook directories are agent-facing fanouts only; hook implementation scripts -must live in their owning public source tree in this repository. - -- `ai-tooling/dev/hooks/`: public hook source scripts. -- `.cursor/hooks/`: Cursor hook script fanout only. Entries must be symlinks. -- `.codex/hooks/`: Codex hook script fanout only. Entries must be symlinks. -- `.claude/hooks/`: Claude hook script fanout only. Entries must be symlinks. - -Tool-specific hook config files, such as `.cursor/hooks.json`, -`.codex/hooks.json`, `.codex/config.toml`, and `.claude/settings.json`, may be -regular files. Hook implementation scripts must live in `ai-tooling/dev/hooks/` -and be exposed through matching symlinks in all three root hook fanouts. - -Internal-only stop hooks and private snapshot hygiene checks live in the -`nvcf/nvcf-internal` repository. Still follow the OSS Snapshot Hygiene policy -manually before finishing. +Hooks follow the same source-and-fanout pattern as root dev skills. +Implementation scripts live in `ai-tooling/dev/hooks/`; `.cursor/hooks/`, +`.codex/hooks/`, and `.claude/hooks/` are symlink fanouts only. Tool-specific +hook configs may be regular files. Internal-only stop hooks and private +snapshot hygiene checks live outside this public snapshot; still follow OSS +Snapshot Hygiene manually before finishing. ### Public skills @@ -229,7 +214,7 @@ belong in `tools/ci/helm-validate-values/`, not under chart `ci/` dirs. ## Commit Messages -Use Conventional Commits v1.0.0. Include issue references in the footer when required by the change type; use `NO-REF` only when acceptable. +Use Conventional Commits v1.0.0. Include issue references in the footer when required by the change type. For GitHub PR work, use a public `NVIDIA/nvcf` issue reference. Format: @@ -250,9 +235,11 @@ Use Conventional Commit format for Merge Request/Pull Request titles, as described in the Commit Messages section. Release automation depends on this format. Examples: `feat:`, `fix:`, `chore:`, `docs:`. -Before creating a Merge Request/Pull Request, confirm there is a bug, issue, or -ticket reference. If none was provided, ask for one. Use `None` or `NO-REF` only -after explicit confirmation that no issue exists or one is not required. +For GitHub PRs, agents must create or link a public `NVIDIA/nvcf` issue in +`## Issues` using `Closes #123`, `Fixes #123`, `Resolves NVIDIA/nvcf#123`, or +`Relates to #123`. If context is private, create a generic public issue. Do not +use `NO-REF`, Jira keys, private tracker or bug IDs, private URLs, title-only +refs, other-repo issues, or invented issue numbers for agent-authored PRs. Use a structured Merge Request/Pull Request description. Subtree repos may define their own Merge Request/Pull Request template; fall back to this shape when none diff --git a/ci/check-pr-issue.sh b/ci/check-pr-issue.sh index 946cfd6e2..67e69ca2d 100755 --- a/ci/check-pr-issue.sh +++ b/ci/check-pr-issue.sh @@ -4,54 +4,160 @@ # Fail a pull request that does not reference an associated issue. # -# A PR passes if its title or body (with HTML comments stripped, so the PR -# template's own guidance/examples do not count) contains any of: -# - a GitHub issue reference: "#123", "Closes/Fixes/Resolves/Relates to #123", -# or "NVIDIA/nvcf#123" -# - a tracker key: NVCF-1234, NVCFCLUST-1234, NVCT-1234, NVBUG-1234, NVBUGS-1234 -# - the escape hatch "NO-REF" (use only when no issue genuinely applies) +# A PR passes if its body's ## Issues section contains a valid NVIDIA/nvcf +# GitHub issue reference or the NO-REF last-resort value: +# - action keyword + optional NVIDIA/nvcf repo: "Closes #123", +# "Fixes NVIDIA/nvcf#123", "Resolves #123", "Relates to #123" +# - "NO-REF" # # Inputs (from the workflow, passed via env to avoid shell injection from the # attacker-controlled PR body): -# PR_TITLE, PR_BODY +# PR_BODY, GITHUB_TOKEN, GITHUB_REPOSITORY, GITHUB_API_URL set -euo pipefail -# Strip HTML comments so the template's guidance and examples (which live inside -# and mention NO-REF / example keys) are not mistaken for a real ref. -visible="$( - PR_TITLE="${PR_TITLE:-}" PR_BODY="${PR_BODY:-}" python3 - <<'PY' -import os, re -text = os.environ.get("PR_TITLE", "") + "\n" + os.environ.get("PR_BODY", "") -print(re.sub(r"", "", text, flags=re.S)) -PY -)" +# Strip HTML comments so the template's guidance and examples are not mistaken +# for a real ref. Only the body ## Issues section is checked. +if output="$( + PR_BODY="${PR_BODY:-}" \ + GITHUB_TOKEN="${GITHUB_TOKEN:-}" \ + GITHUB_REPOSITORY="${GITHUB_REPOSITORY:-NVIDIA/nvcf}" \ + SKIP_GITHUB_ISSUE_EXISTS_CHECK="${SKIP_GITHUB_ISSUE_EXISTS_CHECK:-}" \ + python3 - <<'PY' +import json +import os +import re +import sys +import urllib.error +import urllib.request -# Escape hatch: explicit NO-REF written by the author. -if grep -qE '(^|[^A-Za-z0-9])NO-REF([^A-Za-z0-9]|$)' <<<"$visible"; then - echo "check-pr-issue: NO-REF present; issue requirement waived." - exit 0 -fi +repo = os.environ.get("GITHUB_REPOSITORY", "NVIDIA/nvcf") +if repo.lower() != "nvidia/nvcf": + print(f"ERROR: unsupported repository {repo}; expected NVIDIA/nvcf.") + sys.exit(1) -# Accepted issue references. A bare "#N" is intentionally NOT accepted on its -# own (it false-matches prose like "#1 priority" or "#3 steps"); an explicit -# action keyword, a cross-repo reference, or a tracker key is required. -# - action keyword + optional owner/repo + issue: "Closes #123", -# "Fixes NVIDIA/nvcf#123", "Resolves #123", "Relates to #123" -# - explicit cross-repo GitHub issue: "NVIDIA/nvcf#123" -# - tracker key: NVCF-1234, NVCFCLUST-1234, NVCT-1234, NVBUG-1234, NVBUGS-1234 -if grep -qiE '(close[sd]?|fix(e[sd])?|resolve[sd]?|relate[sd]?[[:space:]]+to)[[:space:]:]+([A-Za-z0-9._/-]+)?#[0-9]+' <<<"$visible" \ - || grep -qE '\bNVIDIA/nvcf#[0-9]+' <<<"$visible" \ - || grep -qE '\b(NVCF|NVCFCLUST|NVCT|NVBUG|NVBUGS)-[0-9]+\b' <<<"$visible"; then - echo "check-pr-issue: PR references an associated issue." +body = os.environ.get("PR_BODY", "") +visible = re.sub(r"|$)", "", body, flags=re.S) + +section_match = re.search( + r"(?ims)^##[ \t]+Issues[ \t]*\n(?P
.*?)(?=^##[ \t]+|\Z)", + visible, +) +if not section_match: + print("ERROR: PR body is missing a visible ## Issues section.") + sys.exit(1) + +section = section_match.group("section") + +disallowed_repo = re.search( + r"\b(?!NVIDIA/nvcf\b)[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+#[0-9]+\b", + section, + flags=re.I, +) +if disallowed_repo: + print( + "ERROR: PR issue references must point to NVIDIA/nvcf; " + f"found {disallowed_repo.group(0)}." + ) + sys.exit(1) + +disallowed_tracker = re.search( + r"\b(?:NVCF|NVCFCLUST|NVCT|NVBUG|NVBUGS)-[0-9]+\b", + section, + flags=re.I, +) +if disallowed_tracker: + print( + "ERROR: Jira keys, private tracker keys, and private bug IDs do not " + "satisfy this check. Create a generic public issue instead." + ) + sys.exit(1) + +disallowed_url = re.search(r"https?://\S+", section, flags=re.I) +if disallowed_url: + print( + "ERROR: URLs do not satisfy this check. Use an action keyword " + "NVIDIA/nvcf issue reference instead." + ) + sys.exit(1) + +if re.search(r"(^|[^A-Za-z0-9])NO-REF([^A-Za-z0-9]|$)", section): + print("check-pr-issue: NO-REF present in ## Issues.") + sys.exit(0) + +action = r"(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?|relate[sd]?[ \t]+to)" +candidates = set() +for match in re.finditer( + action + r"[ \t:]+(?:(?PNVIDIA/nvcf))?#(?P[0-9]+)\b", + section, + flags=re.I, +): + candidates.add(match.group("num")) + +if not candidates: + print("ERROR: this pull request does not reference an associated issue.") + print() + print("Add a GitHub issue to the ## Issues section, for example:") + print(' - "Closes #123" or "Fixes NVIDIA/nvcf#123"') + print(' - or "NO-REF" only as a last resort') + print() + print("If private context exists, create a generic public issue instead of") + print("using a Jira key, private tracker key, private bug ID, or private URL.") + sys.exit(1) + +if os.environ.get("SKIP_GITHUB_ISSUE_EXISTS_CHECK") == "1": + print( + "check-pr-issue: PR references an associated NVIDIA/nvcf issue " + "(existence check skipped)." + ) + sys.exit(0) + +token = os.environ.get("GITHUB_TOKEN") +if not token: + print("ERROR: GITHUB_TOKEN is required to validate issue existence.") + sys.exit(1) + +headers = { + "Accept": "application/vnd.github+json", + "Authorization": f"Bearer {token}", + "User-Agent": "nvcf-pr-issue-check", + "X-GitHub-Api-Version": "2022-11-28", +} +api_url = (os.environ.get("GITHUB_API_URL") or "https://api.github.com").rstrip("/") + +for number in sorted(candidates, key=int): + req = urllib.request.Request( + f"{api_url}/repos/NVIDIA/nvcf/issues/{number}", + headers=headers, + ) + try: + with urllib.request.urlopen(req, timeout=10) as response: + data = json.loads(response.read().decode("utf-8")) + except urllib.error.HTTPError as err: + if err.code == 404: + continue + print(f"ERROR: failed to validate NVIDIA/nvcf#{number}: HTTP {err.code}.") + sys.exit(1) + except Exception as err: + print(f"ERROR: failed to validate NVIDIA/nvcf#{number}: {err}.") + sys.exit(1) + + if "pull_request" in data: + print(f"ERROR: NVIDIA/nvcf#{number} is a pull request, not an issue.") + sys.exit(1) + + print(f"check-pr-issue: PR references NVIDIA/nvcf#{number}.") + sys.exit(0) + +print( + "ERROR: referenced NVIDIA/nvcf issue number was not found or is not " + "accessible." +) +sys.exit(1) +PY +)"; then + echo "$output" exit 0 fi -cat >&2 <<'MSG' -ERROR: this pull request does not reference an associated issue. - -Add one of the following to the PR description: - - a GitHub issue, e.g. "Closes #123" or "Fixes NVIDIA/nvcf#123" - - a tracker key, e.g. "NVCF-1234" - - or "NO-REF" if no issue genuinely applies (use sparingly, per AGENTS.md) -MSG +echo "$output" >&2 exit 1 diff --git a/ci/test/check-pr-issue.test.sh b/ci/test/check-pr-issue.test.sh new file mode 100755 index 000000000..7193485cb --- /dev/null +++ b/ci/test/check-pr-issue.test.sh @@ -0,0 +1,157 @@ +#!/usr/bin/env bash +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +# Focused tests for ci/check-pr-issue.sh. +set -uo pipefail + +here="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +script="${here}/check-pr-issue.sh" +fails=0 +tmpdir="$(mktemp -d)" +server_pid="" + +cleanup() { + if [ -n "${server_pid}" ]; then + kill "${server_pid}" >/dev/null 2>&1 || true + fi + rm -rf "${tmpdir}" +} +trap cleanup EXIT + +issue_body() { + printf '## Issues\n%s\n\n## Checklist\n' "$1" +} + +run_check() { + local body="$1" + local skip="${2-1}" + local api_url="${3:-}" + + PR_BODY="${body}" \ + GITHUB_TOKEN="test-token" \ + GITHUB_REPOSITORY="NVIDIA/nvcf" \ + SKIP_GITHUB_ISSUE_EXISTS_CHECK="${skip}" \ + GITHUB_API_URL="${api_url}" \ + bash "${script}" >/dev/null 2>&1 +} + +pass_case() { + local name="$1" + local body="$2" + + if run_check "${body}"; then + echo "ok (accept) ${name}" + else + echo "FAIL (should accept) ${name}"; fails=$((fails+1)) + fi +} + +fail_case() { + local name="$1" + local body="$2" + + if run_check "${body}"; then + echo "FAIL (should reject) ${name}"; fails=$((fails+1)) + else + echo "ok (reject) ${name}" + fi +} + +start_api() { + local server_py="${tmpdir}/mock_github_api.py" + cat > "${server_py}" <<'PY' +import http.server +import json +import socketserver + +class Handler(http.server.BaseHTTPRequestHandler): + def do_GET(self): + if self.path == "/repos/NVIDIA/nvcf/issues/286": + self.send_json(200, {"number": 286}) + elif self.path == "/repos/NVIDIA/nvcf/issues/287": + self.send_json(200, {"number": 287, "pull_request": {}}) + elif self.path == "/repos/NVIDIA/nvcf/issues/404": + self.send_json(404, {"message": "Not Found"}) + else: + self.send_json(500, {"message": f"unexpected path {self.path}"}) + + def send_json(self, status, payload): + body = json.dumps(payload).encode("utf-8") + self.send_response(status) + self.send_header("Content-Type", "application/json") + self.send_header("Content-Length", str(len(body))) + self.end_headers() + self.wfile.write(body) + + def log_message(self, format, *args): + return + +with socketserver.TCPServer(("127.0.0.1", 0), Handler) as httpd: + print(httpd.server_address[1], flush=True) + httpd.serve_forever() +PY + + python3 "${server_py}" > "${tmpdir}/api.port" 2> "${tmpdir}/api.log" & + server_pid=$! + for _ in {1..50}; do + if [ -s "${tmpdir}/api.port" ]; then + return 0 + fi + sleep 0.1 + done + echo "FAIL mock GitHub API did not start"; fails=$((fails+1)) + return 1 +} + +pass_case_api() { + local name="$1" + local body="$2" + local api_url="$3" + + if run_check "${body}" "" "${api_url}"; then + echo "ok (api accept) ${name}" + else + echo "FAIL (api should accept) ${name}"; fails=$((fails+1)) + fi +} + +fail_case_api() { + local name="$1" + local body="$2" + local api_url="$3" + + if run_check "${body}" "" "${api_url}"; then + echo "FAIL (api should reject) ${name}"; fails=$((fails+1)) + else + echo "ok (api reject) ${name}" + fi +} + +pass_case "action keyword short ref" "$(issue_body 'Closes #286')" +pass_case "action keyword repo ref" "$(issue_body 'Fixes NVIDIA/nvcf#286')" +pass_case "NO-REF last resort" "$(issue_body 'NO-REF')" + +fail_case "missing Issues section" $'## TL;DR\nCloses #286\n' +fail_case "reference outside Issues" $'## TL;DR\nCloses #286\n\n## Issues\n\n## Checklist\n' +fail_case "HTML comment is hidden" "$(issue_body '')" +fail_case "unterminated HTML comment is hidden" $'## Issues\n