|
| 1 | +--- |
| 2 | +name: review-pr |
| 3 | +description: Review a PR on scality/workflows (reusable GitHub Actions workflows for the Scality org) |
| 4 | +argument-hint: <pr-number-or-url> |
| 5 | +disable-model-invocation: true |
| 6 | +allowed-tools: Bash(gh repo view *), Bash(gh pr view *), Bash(gh pr diff *), Bash(gh pr comment *), Bash(gh api *), Bash(git diff *), Bash(git log *), Bash(git show *) |
| 7 | +--- |
| 8 | + |
| 9 | +# Review GitHub PR |
| 10 | + |
| 11 | +You are an expert code reviewer. Review this PR: |
| 12 | + |
| 13 | +## Determine PR target |
| 14 | + |
| 15 | +Parse `` to extract the repo and PR number: |
| 16 | + |
| 17 | +- If arguments contain `REPO:` and `PR_NUMBER:` (CI mode), use those values directly. |
| 18 | +- If the argument is a GitHub URL (starts with `https://github.com/`), extract `owner/repo` and the PR number from it. |
| 19 | +- If the argument is just a number, use the current repo from `gh repo view --json nameWithOwner -q .nameWithOwner`. |
| 20 | + |
| 21 | +## Output mode |
| 22 | + |
| 23 | +- **CI mode** (arguments contain `REPO:` and `PR_NUMBER:`): post inline comments and summary to GitHub. |
| 24 | +- **Local mode** (all other cases): output the review as text directly. Do NOT post anything to GitHub. |
| 25 | + |
| 26 | +## Repo context |
| 27 | + |
| 28 | +This is the **Scality reusable GitHub Actions workflows repository**. It provides standardized CI/CD workflow templates consumed by downstream Scality repos. It contains: |
| 29 | + |
| 30 | +- Reusable workflow definitions (`.github/workflows/*.yaml`) — Docker builds, Trivy scanning, LFS warnings, Claude code review |
| 31 | +- MkDocs Material documentation (`docs/`, `mkdocs.yml`) |
| 32 | +- Test fixtures (`tests/docker/`) — Dockerfiles used to validate workflows |
| 33 | +- Dependabot configuration for automated dependency updates |
| 34 | +- Python dependencies only for docs tooling (`requirements.txt`) |
| 35 | + |
| 36 | +PRs typically involve: workflow YAML changes, action version bumps (Dependabot), documentation updates, and test Dockerfile modifications. |
| 37 | + |
| 38 | +## Steps |
| 39 | + |
| 40 | +1. **Fetch PR details:** |
| 41 | + |
| 42 | +```bash |
| 43 | +gh pr view <number> --repo <owner/repo> --json title,body,headRefOid,author,files |
| 44 | +gh pr diff <number> --repo <owner/repo> |
| 45 | +``` |
| 46 | + |
| 47 | +2. **Read changed files** to understand the full context around each change (not just the diff hunks). |
| 48 | + |
| 49 | +3. **Analyze the changes** against these criteria: |
| 50 | + |
| 51 | +| Area | What to check | |
| 52 | +|------|---------------| |
| 53 | +| Workflow syntax | Valid GitHub Actions YAML — correct `on` triggers, proper `uses` references, required `inputs`/`secrets` declarations, job dependency chains | |
| 54 | +| Action version pinning | Actions should pin to a specific major version tag (e.g., `@v6`), not `@main` or a full SHA without comment | |
| 55 | +| Secret exposure | No credentials, tokens, or keys in plain text; secrets passed only via `secrets:` blocks | |
| 56 | +| Permissions | Jobs use least-privilege `permissions:` — no unnecessary `write` scopes | |
| 57 | +| Breaking changes | Changes to workflow `inputs`, `secrets`, or `outputs` that would break downstream callers | |
| 58 | +| Backward compatibility | Renamed/removed inputs must have a migration path for consuming repos | |
| 59 | +| Docker best practices | Multi-stage builds, minimal base images, no unnecessary `RUN` layers, proper use of build cache | |
| 60 | +| Trivy/security scanning | Correct SARIF output, proper severity thresholds, rate-limiting mitigations | |
| 61 | +| Documentation sync | Workflow changes reflected in corresponding `docs/*.md` files | |
| 62 | +| MkDocs config | Valid `mkdocs.yml` navigation, no broken internal links | |
| 63 | +| Test coverage | New workflow features have corresponding test scenarios in `tests/` | |
| 64 | +| Security | OWASP-relevant issues — command injection in `run:` steps, untrusted input in expressions | |
| 65 | + |
| 66 | +4. **Deliver your review:** |
| 67 | + |
| 68 | +### If CI mode: post to GitHub |
| 69 | + |
| 70 | +#### Part A: Inline file comments |
| 71 | + |
| 72 | +For each specific issue, post a comment on the exact file and line: |
| 73 | + |
| 74 | +```bash |
| 75 | +gh api -X POST -H "Accept: application/vnd.github+json" "repos/<owner/repo>/pulls/<number>/comments" -f body="Your comment<br><br>— Claude Code" -f path="path/to/file" -F line=<line_number> -f side="RIGHT" -f commit_id="<headRefOid>" |
| 76 | +``` |
| 77 | + |
| 78 | +**Never use newlines in bash commands** — use `<br>` for line breaks in comment bodies. The command must stay on a single line. |
| 79 | + |
| 80 | +Each inline comment must: |
| 81 | +- Be short and direct — say what's wrong, why it's wrong, and how to fix it in 1-3 sentences |
| 82 | +- No filler, no complex words, no long explanations |
| 83 | +- When the fix is a concrete line change (not architectural), include a GitHub suggestion block so the author can apply it in one click: |
| 84 | + ```` |
| 85 | + ```suggestion |
| 86 | + corrected-line-here |
| 87 | + ``` |
| 88 | + ```` |
| 89 | + Only suggest when you can show the exact replacement. For architectural or design issues, just describe the problem. |
| 90 | +- Never put `<br>` inside code blocks or suggestion blocks — `<br>` renders as literal text in code. Use `<br>` only in regular comment text. |
| 91 | +- End with: `— Claude Code` |
| 92 | + |
| 93 | +Use the line number from the **new version** of the file (the line number you'd see after the PR is merged), which corresponds to the `line` parameter in the GitHub API. |
| 94 | + |
| 95 | +#### Part B: Summary comment |
| 96 | + |
| 97 | +```bash |
| 98 | +gh pr comment <number> --repo <owner/repo> --body "LGTM<br><br>Review by Claude Code" |
| 99 | +``` |
| 100 | + |
| 101 | +**Never use newlines in bash commands** — use `<br>` for line breaks in comment bodies. The command must stay on a single line. |
| 102 | + |
| 103 | +Do not describe or summarize the PR. For each issue, state the problem on one line, then list one or more suggestions below it: |
| 104 | + |
| 105 | +``` |
| 106 | +- <issue> |
| 107 | + - <suggestion> |
| 108 | + - <suggestion> |
| 109 | +``` |
| 110 | + |
| 111 | +If no issues: just say "LGTM". End with: `Review by Claude Code` |
| 112 | + |
| 113 | +### If local mode: output the review as text |
| 114 | + |
| 115 | +Do NOT post anything to GitHub. Instead, output the review directly as text. |
| 116 | + |
| 117 | +For each issue found, output: |
| 118 | + |
| 119 | +``` |
| 120 | +**<file_path>:<line_number>** — <what's wrong and how to fix it> |
| 121 | +``` |
| 122 | + |
| 123 | +When the fix is a concrete line change, include a fenced code block showing the suggested replacement. |
| 124 | + |
| 125 | +At the end, output a summary section listing all issues. If no issues: just say "LGTM". |
| 126 | + |
| 127 | +End with: `Review by Claude Code` |
| 128 | + |
| 129 | +## What NOT to do |
| 130 | + |
| 131 | +- Do not comment on markdown formatting preferences |
| 132 | +- Do not suggest refactors unrelated to the PR's purpose |
| 133 | +- Do not praise code — only flag problems or stay silent |
| 134 | +- If no issues are found, post only a summary saying "LGTM" |
| 135 | +- Do not flag style issues already covered by the project's linter (eslint, biome, pylint, golangci-lint) |
0 commit comments