Add review-pr and review-open-prs skills

.claude/skills/review-open-prs/SKILL.md

@@ -0,0 +1,58 @@
+---
+name: review-open-prs
+description: Fan-out code review of multiple/all open PRs - one parallel worktree-isolated agent per PR, each following the review-pr skill protocol, then cross-PR verification, cleanup and summary. Use when asked to review all open PRs or several PRs at once; a single PR needs only review-pr.
+---
+
+# Multi-agent review of open PRs
+
+Orchestration only — the per-PR review protocol lives in
+`.claude/skills/review-pr/SKILL.md` and is NOT duplicated here.
+
+## 1. Discover the PRs and the stack topology
+
+```bash
+gh pr list --state open --json number,title,headRefName,baseRefName,url --limit 50
+```
+
+Note which PRs are stacked (`baseRefName` ≠ main) — pass each PR's base branch into its
+agent prompt so the "own diff only" rule is explicit.
+
+## 2. Launch one agent per PR
+
+Agent tool, all launches in a single message (parallel), each with
+`isolation: "worktree"` so agents can `gh pr checkout` and run the test suite without
+touching the main checkout or each other.
+
+Each agent's prompt:
+
+- "You are an expert Kubernetes/Helm code reviewer. Review PR #<N> (<title>, base
+  `<baseRefName>`) and post the review to GitHub."
+- "Follow the protocol in `.claude/skills/review-pr/SKILL.md` — read it first; the
+  **(parallel)** notes apply to you."
+- PR-specific hypotheses worth chasing (see the review-focus section of `review-pr`) —
+  tailor them per PR; this is what separates real findings from style nits.
+- "Return a severity-ranked findings report, verification results, and the posted
+  review URL."
+
+## 3. After all agents finish
+
+1. **Verify every review landed on the right PR** (the shared-scratchpad collision
+   described in `review-pr` makes this non-optional):
+
+   ```bash
+   for pr in <numbers>; do echo "=== PR #$pr ==="; \
+     gh api repos/shopsys/helm/pulls/$pr/reviews \
+       --jq '.[] | select(.body | contains("Claude Agent (CR)")) | "\(.id) [\(.state)]"'; done
+   ```
+
+2. **Clean up the agent worktrees** (a PR-branch checkout marks them "changed", so they
+   are not auto-removed):
+
+   ```bash
+   git worktree list | grep agent- | awk '{print $1}' \
+     | while read wt; do git worktree remove --force "$wt"; done
+   git branch | grep worktree-agent | xargs -r git branch -D
+   ```
+
+3. **Summarize for the user**: cross-PR severity ranking (lead with the worst finding
+   overall, not per-PR), per-PR test results, and links to all posted reviews.

.claude/skills/review-pr/SKILL.md

@@ -0,0 +1,102 @@
+---
+name: review-pr
+description: Detailed code review of ONE pull request - scope vs its (possibly stacked) base, verification-first findings (render + tests + web research), posting a GitHub review with anchored inline comments signed "Claude Agent (CR)". Use when asked to review a specific PR; for all open PRs at once use review-open-prs, which fans this out.
+---
+
+# Code review of a single PR
+
+Works standalone in the main checkout, or as the per-agent protocol inside a
+`review-open-prs` fan-out. When running as a parallel agent, the **(parallel)** notes apply.
+
+## 1. Context before judgment
+
+Read `CLAUDE.md` and `.claude/skills/helm-kubernetes-guidelines/SKILL.md` first. Most
+"obvious improvements" in this repo are intentional legacy-parity decisions — check
+`docs/migrating-from-shopsys-deployment.md` before flagging behavior as wrong.
+
+## 2. Scope the diff
+
+```bash
+gh pr view <N> --json title,body,baseRefName,files
+gh pr diff <N>
+```
+
+Check `baseRefName`: PRs here are often **stacked** (base = another PR's branch).
+Review ONLY this PR's own diff — `gh pr diff <N>` already diffs against the PR's base.
+Never attribute base-branch changes to this PR.
+
+Then `gh pr checkout <N>` to inspect full files in context — the diff alone hides
+interactions (hook ordering, helper includes, values layering).
+**(parallel)** Only ever checkout inside your own isolated worktree.
+
+## 3. Review focus
+
+- **Will it work** — render + runtime reasoning, not just YAML shape. Form PR-specific
+  hypotheses and chase them (e.g. "the hook that scales cron needs an API token — does it
+  still get one?", "does nginx actually start as non-root with a read-only rootfs?").
+  A pointed hypothesis finds real bugs; generic scanning finds style nits.
+- **Completeness** — values.schema.json, docs/values.md, deviations doc entry, golden
+  snapshots regenerated in the SAME commit, helm-unittest coverage for new values paths.
+- **Security** — least privilege, PSS "restricted" alignment, secret handling, footguns
+  that fail open (e.g. a mistyped toggle key silently ignored because the schema doesn't
+  know it).
+- **Improvements** — override/null-merge behavior (verify by actually rendering the
+  override), consistency with the standard component keys, docs accuracy.
+
+## 4. Verify, don't speculate
+
+```bash
+helm dependency build charts/shopsys-infra && helm dependency build charts/shopsys-app
+./tests/run-golden-tests.sh
+helm unittest charts/shopsys-app charts/shopsys-infra
+# manifest-shape changes: kubeconform the rendered golden output (see CLAUDE.md)
+```
+
+- Upstream claims (Kubernetes semantics, ingress-nginx behavior, CVEs, image UIDs) get
+  confirmed via WebSearch/WebFetch against primary docs, not memory.
+- Only verified findings get posted. Anything needing a real cluster is explicitly marked
+  unverifiable — never presented as fact.
+
+## 5. Post the review
+
+One review — summary body + inline comments in a single API call:
+
+```bash
+gh api repos/shopsys/helm/pulls/<N>/reviews -X POST --input review-pr<N>.json
+# payload: {"event":"COMMENT","body":"<summary>",
+#           "comments":[{"path":"...","line":L,"side":"RIGHT","body":"..."}]}
+```
+
+- `event: COMMENT` — never APPROVE/REQUEST_CHANGES from an agent.
+- Inline `line` values MUST be lines present in this PR's diff hunks (`side: RIGHT` for
+  added/context lines) — cross-check against `gh pr diff <N>` immediately before posting;
+  one bad anchor 422s the whole review. Unanchorable findings go into the summary body.
+- Summary = what was reviewed, what was verified (commands run, sources consulted),
+  findings ranked by severity. A clean review is still posted, saying so explicitly.
+- English (repo convention); EVERY inline comment and the summary end with the signature:
+  `— Claude Agent (CR)`.
+- **Unique payload filename** (`review-pr<N>.json`, never `payload.json`).
+  **(parallel)** Sibling agents share the scratchpad and WILL overwrite a generic name
+  between write and POST (observed: 422 with the other PR's unresolvable paths/lines) —
+  re-verify the file's anchors right before the POST.
+- Re-run? Check for an existing "Claude Agent (CR)" review first and post only findings
+  that are new — no duplicates.
+
+## 6. Report
+
+Return/deliver a severity-ranked findings list, the verification results (honest — failing
+tests are reported as failing), and the posted review URL:
+
+```bash
+gh api repos/shopsys/helm/pulls/<N>/reviews \
+  --jq '.[] | select(.body | contains("Claude Agent (CR)")) | "\(.id) [\(.state)]"'
+```
+
+Transient GitHub API `i/o timeout` happens — retry after a few seconds before concluding
+the review is missing.
+
+## Don'ts
+
+- Never resolve review threads (same rule as `watch-pr`).
+- Never review the stacked siblings' changes.
+- Never post speculation as fact.

CLAUDE.md

@@ -134,6 +134,10 @@ improvising:
   conventions (incl. stacked PRs)
 - `watch-pr` — CI watching, Copilot review triage protocol (incl. suppressed comments;
   never resolve threads yourself)
+- `review-pr` — detailed code review of one PR: stacked-base scoping, verification-first
+  findings, GitHub review with inline comments signed "Claude Agent (CR)"
+- `review-open-prs` — fan-out of `review-pr` over multiple/all open PRs via parallel
+  worktree-isolated agents, plus cross-PR verification, cleanup and summary
 - `implement-issue` — issue lifecycle: triage, scope-deviation records, closing retrospective
 - `create-issue` — backlog conventions: body structure, title prefixes, label taxonomy
 - `sync-upstream-deployment` — porting changes from the legacy shopsys/deployment package