|
| 1 | +--- |
| 2 | +name: review-pr |
| 3 | +description: Review a PR on metalk8s (an opinionated Kubernetes distribution for long-term on-prem deployments) |
| 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 | +| Go error handling | Use `fmt.Errorf("...: %w", err)` for wrapping, not `%v`; no swallowed errors | |
| 42 | +| Go context propagation | Pass `context.Context` through call chains; respect cancellation | |
| 43 | +| Go goroutine leaks | Goroutines must have exit conditions; use `errgroup` where appropriate | |
| 44 | +| Kubernetes operator | RBAC scoping (least-privilege), reconciler idempotency, status subresource updates only via `Status().Update()` | |
| 45 | +| Interface compliance | Verify implementations satisfy interfaces at compile time (e.g. `var _ SomeInterface = &MyType{}`) | |
| 46 | +| TypeScript/React | Prop changes (missing/wrong types), missing `key` props in lists, proper hook usage, no `console.log` in production code | |
| 47 | +| Scality dep pinning | `@scality/core-ui`, `@scality/module-federation`, `@kubernetes/client-node` must be pinned to a specific tag/commit, not a branch | |
| 48 | +| Salt states | Correct use of requisites (`require`, `watch`, `onchanges`); no hardcoded credentials; proper Jinja templating | |
| 49 | +| Helm/Kustomize | Resource limits set, security contexts defined, proper label selectors, upgrade path preserved | |
| 50 | +| Python | No bare `except:`; specific exception types; type hints on new functions; no blocking calls in async context | |
| 51 | +| Security | No secrets/tokens in plain text; no OWASP-relevant issues (injection, XSS, insecure defaults) | |
| 52 | +| Breaking changes | Public API changes, CRD schema changes, Salt pillar schema changes, UI prop interface changes | |
| 53 | + |
| 54 | +4. **Deliver your review:** |
| 55 | + |
| 56 | +### If CI mode: post to GitHub |
| 57 | + |
| 58 | +#### Part A: Inline file comments |
| 59 | + |
| 60 | +For each specific issue, post a comment on the exact file and line: |
| 61 | + |
| 62 | +```bash |
| 63 | +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>" |
| 64 | +``` |
| 65 | + |
| 66 | +**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. |
| 67 | + |
| 68 | +Each inline comment must: |
| 69 | +- Be short and direct — say what's wrong, why it's wrong, and how to fix it in 1-3 sentences |
| 70 | +- No filler, no complex words, no long explanations |
| 71 | +- When the fix is a concrete line change (not architectural), include a GitHub suggestion block so the author can apply it in one click: |
| 72 | + ```` |
| 73 | + ```suggestion |
| 74 | + corrected-line-here |
| 75 | + ``` |
| 76 | + ```` |
| 77 | + Only suggest when you can show the exact replacement. For architectural or design issues, just describe the problem. |
| 78 | + Example with a suggestion block: |
| 79 | + ```bash |
| 80 | + 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' ... |
| 81 | + ``` |
| 82 | +- When the comment contains a suggestion block, use `$'...'` quoting with `\n` for code fence boundaries. Escape single quotes as `\'` (e.g., `don\'t`) |
| 83 | +- End with: `— Claude Code` |
| 84 | + |
| 85 | +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. |
| 86 | + |
| 87 | +#### Part B: Summary comment |
| 88 | + |
| 89 | +```bash |
| 90 | +gh pr comment <number> --repo <owner/repo> --body "LGTM<br><br>Review by Claude Code" |
| 91 | +``` |
| 92 | + |
| 93 | +**The command must stay on a single bash line.** Never use newlines in bash commands — use `<br>` for line breaks in comment bodies. |
| 94 | + |
| 95 | +Do not describe or summarize the PR. For each issue, state the problem on one line, then list one or more suggestions below it: |
| 96 | + |
| 97 | +``` |
| 98 | +- <issue> |
| 99 | + - <suggestion> |
| 100 | + - <suggestion> |
| 101 | +``` |
| 102 | + |
| 103 | +If no issues: just say "LGTM". End with: `Review by Claude Code` |
| 104 | + |
| 105 | +### If local mode: output the review as text |
| 106 | + |
| 107 | +Do NOT post anything to GitHub. Instead, output the review directly as text. |
| 108 | + |
| 109 | +For each issue found, output: |
| 110 | + |
| 111 | +``` |
| 112 | +**<file_path>:<line_number>** — <what's wrong and how to fix it> |
| 113 | +``` |
| 114 | + |
| 115 | +When the fix is a concrete line change, include a fenced code block showing the suggested replacement. |
| 116 | + |
| 117 | +At the end, output a summary section listing all issues. If no issues: just say "LGTM". |
| 118 | + |
| 119 | +End with: `Review by Claude Code` |
| 120 | + |
| 121 | +## What NOT to do |
| 122 | + |
| 123 | +- Do not comment on markdown formatting preferences |
| 124 | +- Do not suggest refactors unrelated to the PR's purpose |
| 125 | +- Do not praise code — only flag problems or stay silent |
| 126 | +- If no issues are found, post only a summary saying "LGTM" |
| 127 | +- Do not flag style issues already covered by the project's linter (biome, golangci-lint, pylint) |
0 commit comments