|
| 1 | +--- |
| 2 | +name: review-pr |
| 3 | +description: Review a PR on artifacts (OpenResty/nginx+Lua artifact proxy serving GCS S3-compatible storage) |
| 4 | +argument-hint: <pr-number-or-url> |
| 5 | +disable-model-invocation: true |
| 6 | +allowed-tools: Read, 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: $ARGUMENTS |
| 12 | + |
| 13 | +## Determine PR target |
| 14 | + |
| 15 | +Parse `$ARGUMENTS` 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 | +## Steps |
| 27 | + |
| 28 | +1. **Fetch PR details:** |
| 29 | + |
| 30 | +```bash |
| 31 | +gh pr view <number> --repo <owner/repo> --json title,body,headRefOid,author,files |
| 32 | +gh pr diff <number> --repo <owner/repo> |
| 33 | +``` |
| 34 | + |
| 35 | +2. **Read changed files** to understand the full context around each change (not just the diff hunks). |
| 36 | + |
| 37 | +3. **Analyze the changes** against these criteria: |
| 38 | + |
| 39 | +| Area | What to check | |
| 40 | +|------|---------------| |
| 41 | +| Lua correctness | Nil checks before indexing ngx.var/env vars; proper use of `ngx.exit()` vs `return`; no blocking I/O calls (use `ngx.location.capture` or cosocket) | |
| 42 | +| S3 signature / request signing | Correct HMAC-SHA256 construction, canonical request, string-to-sign; URL encoding of keys and prefixes matches S3 spec | |
| 43 | +| GitHub auth logic | Auth cache invalidation on revoked access; bot credentials properly gated; restriction lists correctly parsed | |
| 44 | +| nginx config template | Variable substitution correctness; no broken `${}` references; upstream and proxy_pass consistency | |
| 45 | +| Shell scripts | Unquoted variable expansions that could break on spaces or empty values; proper error handling with `set -e` or explicit checks | |
| 46 | +| Python tests | Tests actually assert on response content, not just status codes; boto3 calls use the right bucket/key paths | |
| 47 | +| Helm chart | Resource limits present; env var names match what `start.sh` and Lua scripts expect; no hardcoded secrets | |
| 48 | +| Security | Credentials or tokens in plain text; path traversal via unchecked user input in Lua; overly permissive CORS or auth bypass | |
| 49 | +| Breaking changes | Env var renames that break existing deployments; nginx directive changes that alter behavior for existing clients | |
| 50 | +| Docker image | Base image pin still uses a specific tag (not `latest`); new runtime deps added to `apk add` | |
| 51 | + |
| 52 | +4. **Deliver your review:** |
| 53 | + |
| 54 | +### If CI mode: post to GitHub |
| 55 | + |
| 56 | +#### Part A: Inline file comments |
| 57 | + |
| 58 | +For each specific issue, post a comment on the exact file and line: |
| 59 | + |
| 60 | +```bash |
| 61 | +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>" |
| 62 | +``` |
| 63 | + |
| 64 | +**The command must stay on a single bash line.** Never use newlines in bash commands — use `<br>` for line breaks in comment bodies. Never put `<br>` inside code blocks or suggestion blocks. |
| 65 | + |
| 66 | +Each inline comment must: |
| 67 | +- Be short and direct — say what's wrong, why it's wrong, and how to fix it in 1-3 sentences |
| 68 | +- No filler, no complex words, no long explanations |
| 69 | +- When the fix is a concrete line change (not architectural), include a GitHub suggestion block so the author can apply it in one click: |
| 70 | + ```` |
| 71 | + ```suggestion |
| 72 | + corrected-line-here |
| 73 | + ``` |
| 74 | + ```` |
| 75 | + Only suggest when you can show the exact replacement. For architectural or design issues, just describe the problem. |
| 76 | + Example with a suggestion block: |
| 77 | + ```bash |
| 78 | + gh api ... -f body=$'Missing the shared-guidelines update command.<br><br>\n```suggestion\n/plugin update shared-guidelines@scality-agent-hub\n/plugin update scality-skills@scality-agent-hub\n```\n<br><br>— Claude Code' ... |
| 79 | + ``` |
| 80 | +- When the comment contains a suggestion block, use `$'...'` quoting with `\n` for code fence boundaries. Escape single quotes as `\'` (e.g., `don\'t`) |
| 81 | +- End with: `— Claude Code` |
| 82 | + |
| 83 | +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. |
| 84 | + |
| 85 | +#### Part B: Summary comment |
| 86 | + |
| 87 | +```bash |
| 88 | +gh pr comment <number> --repo <owner/repo> --body "LGTM<br><br>Review by Claude Code" |
| 89 | +``` |
| 90 | + |
| 91 | +**The command must stay on a single bash line.** Never use newlines in bash commands — use `<br>` for line breaks in comment bodies. |
| 92 | + |
| 93 | +Do not describe or summarize the PR. For each issue, state the problem on one line, then list one or more suggestions below it: |
| 94 | + |
| 95 | +``` |
| 96 | +- <issue> |
| 97 | + - <suggestion> |
| 98 | + - <suggestion> |
| 99 | +``` |
| 100 | + |
| 101 | +If no issues: just say "LGTM". End with: `Review by Claude Code` |
| 102 | + |
| 103 | +### If local mode: output the review as text |
| 104 | + |
| 105 | +Do NOT post anything to GitHub. Instead, output the review directly as text. |
| 106 | + |
| 107 | +For each issue found, output: |
| 108 | + |
| 109 | +``` |
| 110 | +**<file_path>:<line_number>** — <what's wrong and how to fix it> |
| 111 | +``` |
| 112 | + |
| 113 | +When the fix is a concrete line change, include a fenced code block showing the suggested replacement. |
| 114 | + |
| 115 | +At the end, output a summary section listing all issues. If no issues: just say "LGTM". |
| 116 | + |
| 117 | +End with: `Review by Claude Code` |
| 118 | + |
| 119 | +## What NOT to do |
| 120 | + |
| 121 | +- Do not comment on markdown formatting preferences |
| 122 | +- Do not suggest refactors unrelated to the PR's purpose |
| 123 | +- Do not praise code — only flag problems or stay silent |
| 124 | +- If no issues are found, post only a summary saying "LGTM" |
| 125 | +- Do not flag style issues already covered by the project's linter |
0 commit comments