claude-plugins: move pr-review skill into base, not its own plugin

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

@@ -0,0 +1,90 @@
+---
+name: pr-review
+description: Generic mechanics and conventions for reviewing a pull request - label every finding with a severity (block/note/good), don't approve with an unresolved one-line fix outstanding, check correctness/security/consistency/tests/docs, resolve any opaque reference before approving, and submit a formal approve/request-changes verdict rather than a plain comment. Use this whenever you're about to review someone else's pull request, from first read to submitting your verdict.
+---
+
+# PR Review
+
+A review's job is to catch what the author missed, not to rewrite their
+work in your own style. That framing should hold throughout.
+
+## Before you start
+
+1. **Read the linked issue/task** - understand *why* the change exists,
+   not just what it does.
+2. **Check CI status first.** A failing build or test suite is an
+   automatic block - don't write a detailed review of code that doesn't
+   even pass its own checks. Unblock the build first.
+3. **Pull the full source when context matters**, not just the diff -
+   diffs lie by omission.
+
+## Every finding needs a severity
+
+- **block** - must fix before merge: a bug, a security issue, broken
+  behavior, a failing test, a hardcoded value that will rot.
+- **note** - worth fixing, author's call: an inconsistency, a missing
+  edge case, a stale comment, copy-paste smell.
+- **good** - explicitly call out what's done well (optional, but
+  appreciated - a review that's only ever critical trains authors to
+  dread review instead of valuing it).
+
+Mixing blocks and notes without labeling them forces the author to
+guess what's actually required before merge.
+
+## Don't approve with an outstanding one-line fix
+
+If a finding is a trivial fix, it belongs in the PR before merge - don't
+approve with it left as a dangling nit. Request changes when the fix is
+that cheap. Approving with unresolved issues trains authors to treat
+review comments as optional.
+
+## What to check
+
+- **Correctness** - does it do what the issue says? Handle edge cases
+  (empty input, off-by-one, concurrent access)? Any silent failure paths
+  (swallowed errors, unchecked returns)?
+- **Security** - credentials/tokens ever logged or echoed? User-supplied
+  input injected into a shell command, query, or DOM? File paths
+  validated before use?
+- **Consistency** - do new identifiers match the surrounding file's
+  conventions? Same error-handling pattern as neighboring code? Doc
+  comments in the same style and tense as existing ones?
+- **Tests** - do they exercise the actual new behavior, including
+  failure paths, not just the happy path? Are names descriptive enough
+  to diagnose a failure without reading the body?
+- **Documentation** - does any public interface, flag, or config option
+  need a doc update? Are stale docs cleaned up alongside the change?
+
+## Opaque references
+
+Any comment or identifier that references something by name (a ticket
+number, an internal codename, a `NOTE(foo)`-style marker) is only useful
+if it's actually resolvable. Search for the referenced thing before
+approving; if you can't resolve it, flag it - missing context isn't a
+reason to wave a change through.
+
+## Submit a formal verdict, not just a comment
+
+A plain comment does **not** set a PR's review state and won't
+block or unblock a merge on most forges. Every review should end with
+an explicit **approve** or **request-changes** verdict through
+whatever formal review mechanism your forge tooling exposes - not just
+prose in a comment that looks like a verdict but doesn't function as one.
+
+## Format
+
+Keep it tight - one finding per bullet, each labeled with its severity,
+two sentences max per finding. If a finding needs more than that, it's
+architectural enough to be its own comment, not a review bullet.
+
+## Tone
+
+Precise, not harsh. "This will panic on an empty slice" is useful. "This
+is bad" is not - point at the exact spot, describe the failure mode,
+suggest the fix. Don't soften a real block into a vague nit to be nice;
+a watered-down block is just a block that slips into production.
+
+If your project ships its own domain-specific review checklist (gates
+that differ by author/area, project-specific tooling commands), treat
+that as the canonical addition on top of this - project specifics don't
+belong duplicated in here.