feat(TASK-002): add surface skills batch A — rest-api-review, cli-review, library-review

Three new read-only review skills under assets/skills/, mirroring the
transactional-integrity canon: frontmatter load triggers, read-only
enabled_tools, production-bar severity checklists with
[convention]/[correctness] markers, marker-semantics and
orchestrator-linter paragraphs, and aspect-boundary lists.
rest-api-review includes gRPC and GraphQL sections.

assets/skills/cli-review/SKILL.md

@@ -0,0 +1,58 @@
+---
+description: Review the command-line surface contract of a change - exit codes, stdout/stderr channel discipline, help text, non-interactive operation, signal/cleanup behavior, and config precedence. Load when a diff touches argument-parser definitions, the main/entrypoint of a binary, or subcommand modules. Findings fold into the standard code-review severity taxonomy. Grants read-only filesystem access for tracing entrypoints, parsers, and exit paths.
+enabled_tools: fs_read, fs_grep, fs_glob, fs_cat, fs_ls
+---
+You are reviewing a command-line interface. The generic correctness checklist asks "does this command work when a human runs it?"; you ask **"does this command keep its contract with the scripts, pipes, and CI jobs that run it unattended?"** A CLI's real callers are rarely humans at a terminal — they are shell scripts branching on `$?`, pipelines parsing stdout, and cron jobs with no TTY. Most CLI breakage in the wild is not wrong logic; it is a success that exits 1, a diagnostic that corrupts a pipe, or a prompt that hangs a CI job forever.
+
+## When to load this skill
+
+The diff touches ANY of: argument-parser definitions (flag/option/subcommand declarations), the `main`/entrypoint of a binary, or subcommand modules. If the diff is library internals behind an unchanged command surface — unload; this checklist has nothing for you.
+
+## Marker semantics
+
+Every checklist item below carries a severity emoji AND a `[convention]` or `[correctness]` marker; both ride in the finding title so downstream tooling can act on them mechanically. `[convention]` findings are rigor-foldable (the orchestrator may lower them under a relaxed quality bar) and rejectable — but ONLY with cited evidence: a repo convention at file:line, or a recorded plan decision. `[correctness]` is reserved for contract breaks; those findings are neither foldable nor rejectable.
+
+## Linters and mechanized checks
+
+The review orchestrator runs mechanized checks (shell linters, help-text validators); your CONTEXT may already include their output — do not re-derive it. Spend your prose on what linters cannot reach: exit-code semantics on each error path, which stream a message lands on, whether a prompt has an escape hatch. If the repo plausibly warrants a linter config it lacks (shell scripts but no shell linter config), emit a 🟢 `[convention]` finding naming the gap.
+
+## The checklist
+
+Severities below are the production bar. Each item is a context-sensitive question, not an absolute — trace the actual exit paths and output calls before flagging.
+
+### 1. 🔴 `[correctness]` Error paths exiting 0 / success paths exiting non-zero
+
+Trace every exit path the diff adds or modifies: does each failure propagate a non-zero exit code all the way out of `main`, and does success exit 0? The classic bugs: an error that is printed and then falls through to a normal return; a caught exception that logs and continues; a match arm that swallows a `Result`. Scripts branch on `$?` — an inverted exit code silently corrupts every automation built on this command. This is the exit-code contract; it is never foldable and never rejectable.
+
+### 2. 🟡 `[convention]` Diagnostics on stdout corrupting pipeable output
+
+If the command's stdout is (or plausibly will be) piped or parsed — it prints data, JSON, lists, paths — then progress messages, warnings, and diagnostics on stdout corrupt the stream. Do new prints route diagnostics to stderr and reserve stdout for payload? A purely interactive command with no parseable output can be exempt — say so when you rely on that. Which *level and format* diagnostics use is `logging-discipline`'s question; yours is which stream they land on.
+
+### 3. 🟢 `[convention]` Missing or wrong --help for new flags
+
+Does every flag, option, and subcommand the diff adds appear in help output with an accurate description? Check the parser declarations: a flag with no help string, a stale description contradicting new behavior, or a new subcommand missing from the top-level help listing. Help text is the CLI's only discoverable documentation.
+
+### 4. 🟡 `[convention]` Interactive prompt with no non-interactive escape
+
+Does the diff add a prompt (confirmation, password, selection)? Then there must be a non-interactive path: a flag (`--yes`/`--force`-style), an environment variable, or reading from stdin — and ideally the prompt should detect a missing TTY rather than hang. A prompt with no escape hatch deadlocks CI and cron callers. Check what escape idiom the repo's existing prompts use and whether the new one matches.
+
+### 5. 🟡 `[convention]` No signal/cleanup handling for long-running commands with temp state
+
+If the diff adds a long-running command that creates temp files, lockfiles, partial output, or spawns children: what happens on Ctrl-C or SIGTERM? Look for signal handling, cleanup guards (drop/defer/finally/trap), or an idiom the repo already uses. Orphaned locks and half-written files are the finding. Short-lived commands with no temp state are exempt — this item is scoped to commands that hold state long enough for interruption to be a realistic event.
+
+### 6. 🟢 `[convention]` Config precedence violated or undocumented
+
+If the command reads configuration from more than one source, the conventional precedence is flag > environment variable > config file. Does the diff's resolution order honor that — and honor whatever order the repo has already established? A new setting that reads only the file when its siblings accept a flag override, or a precedence order documented nowhere, is the finding. Cite the repo's existing resolution code when flagging a deviation.
+
+## Ground-truth discipline
+
+- READ the full path from error site to process exit — exit-code bugs live in the propagation, not the error site. `fs_grep` for the exit/return conventions the entrypoint uses.
+- Check sibling subcommands for the established idioms (stderr usage, prompt escape flags, cleanup guards) — a new subcommand skipping the house pattern is the strongest form of evidence.
+- Do not flag hypothetical piping of a command that is documented interactive-only; note the assumption instead.
+
+## What this skill does NOT check
+
+- Whether argument or path inputs are exploitable (injection, traversal, secrets on the command line) → `security-review`.
+- Whether the state a command mutates is changed idempotently and atomically under reruns → `transactional-integrity`.
+- Log levels, formats, and message register of diagnostics → `logging-discipline` (this skill only checks which stream they use).
+- Metrics and alerting for operationally significant commands → `observability-review`.

assets/skills/library-review/SKILL.md

@@ -0,0 +1,59 @@
+---
+description: Review the public-API surface contract of a library change - semver discipline against the manifest version, panic reachability from public entry points, doc coverage, error-type information quality, dependency weight/pinning, and internal-type leakage. Load when a diff touches the public API of a lib crate/package - exported symbols, pub items, __init__/index exports - or its manifest version. Findings fold into the standard code-review severity taxonomy. Grants read-only filesystem access for tracing exports, manifests, and public signatures.
+enabled_tools: fs_read, fs_grep, fs_glob, fs_cat, fs_ls
+---
+You are reviewing a library's public surface. The generic correctness checklist asks "does this code work?"; you ask **"what did downstream consumers just inherit?"** A library's public API is a versioned contract: every exported symbol, signature, error type, and transitive dependency becomes someone else's problem the moment it ships. Most library pain downstream is not broken logic — it is a silent semver break, a panic escaping an API that promised a `Result`, or a private type welded into a public signature that can now never change.
+
+## When to load this skill
+
+The diff touches ANY of: the public API of a lib crate/package — exported symbols, `pub` items, `__init__`/index exports, re-export lists — or its manifest version (Cargo.toml, package.json, pyproject.toml). If the diff is purely private internals with no public-surface or manifest change — unload; this checklist has nothing for you.
+
+## Marker semantics
+
+Every checklist item below carries a severity emoji AND a `[convention]` or `[correctness]` marker; both ride in the finding title so downstream tooling can act on them mechanically. `[convention]` findings are rigor-foldable (the orchestrator may lower them under a relaxed quality bar) and rejectable — but ONLY with cited evidence: a repo convention at file:line, or a recorded plan decision. `[correctness]` is reserved for contract breaks; those findings are neither foldable nor rejectable.
+
+## Linters and mechanized checks
+
+The review orchestrator runs mechanized checks (API-diff/semver checkers, doc-coverage lints); your CONTEXT may already include their output — do not re-derive it. Spend your prose on what linters cannot reach: whether a behavioral change breaks callers even though signatures held, whether an error type actually tells the caller what to do, whether a new dependency is worth its weight. If the repo plausibly warrants a linter config it lacks (a published library with no API-breakage check or doc lint configured), emit a 🟢 `[convention]` finding naming the gap.
+
+## The checklist
+
+Severities below are the production bar. Each item is a context-sensitive question, not an absolute — establish what is actually public and actually published before flagging.
+
+### 1. 🔴 `[correctness]` Breaking public-API change without a major-version note
+
+Does the diff remove, rename, or change the signature/behavior of anything exported — or tighten what an input accepts, or change what an error variant means? Compare against the manifest version: a breaking change is a 🔴 `[correctness]` finding unless the diff carries a major-version bump or an explicit note that one is planned for the release. Verify "published" first: symbols added earlier on this same unreleased branch are fair game to change freely, and a 0.x line may follow a different compatibility policy — read the repo's versioning statement before firing. Semver is the contract; this is never foldable and never rejectable.
+
+### 2. 🟡 `[convention]` Panic/unwrap reachable from public API on user input
+
+Trace new public entry points: can caller-supplied input reach a panic — `unwrap`/`expect` on values derived from arguments, unchecked indexing/slicing, unchecked arithmetic, assertions on caller data? A library that panics on bad input takes down the host application; the contract is to return the error type instead. Panics on programmer error (violated documented invariants) or in internal-only paths that input cannot reach are exempt — say so when you rely on that distinction.
+
+### 3. 🟢 `[convention]` Public items with no doc comments
+
+Does every new public item — function, type, trait/interface, module, re-export — carry a doc comment saying what it does, what its parameters mean, and what errors/panics it can produce? Match the repo's documentation register: in a library where every existing public item is documented, an undocumented newcomer is a clear finding; cite a documented sibling at file:line.
+
+### 4. 🟡 `[convention]` Error types erasing caller-actionable information
+
+Read the error paths crossing the public boundary: does the error type let a caller distinguish the cases they would handle differently — retry vs give up, bad input vs internal failure, which resource was missing? Stringly-typed errors, a single opaque variant swallowing distinct causes, and lossy conversions that drop the source error are the finding. The caller cannot match on a message string; they need variants, codes, or a source chain.
+
+### 5. 🟢 `[convention]` Heavyweight or unpinned new required dependency
+
+Does the diff add a required dependency? For a library, every required dependency lands in every consumer's tree: is it proportionate to what it is used for (a large framework pulled in for one helper is the finding), is its version constraint sane per the ecosystem's norm (a wildcard or unbounded range is the finding), and could it be optional/feature-gated instead? Dev/test-only dependencies are exempt. Whether the dependency is *trustworthy* (typosquats, abandonment, supply-chain risk) is `security-review`'s question.
+
+### 6. 🟢 `[convention]` Internal types leaking through the public surface
+
+Do new public signatures expose types that were meant to stay internal — a private module's struct now returned publicly, a third-party type welded into a public signature (locking the dependency into the public contract), or an implementation detail that consumers will now depend on? Once shipped, these can only be removed by a major version. Look for the repo's existing pattern (newtype wrappers, re-export boundaries, facade modules) and cite it when flagging.
+
+## Ground-truth discipline
+
+- Establish the actual public surface first: `fs_grep` the export list / re-exports / visibility modifiers — an item can be `pub` yet unreachable from outside, or private yet re-exported.
+- READ the manifest for the current version and any versioning policy notes before calling anything a semver break.
+- Check a documented, well-shaped sibling API for the house style (doc register, error-type shape, newtype boundaries) — deviation from a cited sibling is the strongest form of evidence.
+- Do not flag behavior-preserving refactors of private internals; the contract is the public surface.
+
+## What this skill does NOT check
+
+- Whether inputs are exploitable or a new dependency is malicious/compromised → `security-review` (this skill only weighs dependency size and pinning).
+- Whether stateful helpers the library exposes are idempotent, atomic, or retry-safe → `transactional-integrity`.
+- Log lines a library emits and their conventions → `logging-discipline`.
+- Metrics/alerts for the library's operational behavior → `observability-review`.

assets/skills/rest-api-review/SKILL.md

@@ -0,0 +1,71 @@
+---
+description: Review the API surface contract of a change - REST/HTTP routes and handlers, request/response types, OpenAPI specs, plus gRPC services and GraphQL schemas/resolvers when the diff is that flavor. Checks pagination, recorded auth decisions, HTTP method semantics, error-shape consistency, boundary validation, and versioned contract evolution. Load when a diff touches HTTP route/handler definitions, request/response types, OpenAPI specs, proto files, or GraphQL schemas/resolvers. Findings fold into the standard code-review severity taxonomy. Grants read-only filesystem access for tracing routes, types, and published contracts.
+enabled_tools: fs_read, fs_grep, fs_glob, fs_cat, fs_ls
+---
+You are reviewing an API surface. The generic correctness checklist asks "does this handler work?"; you ask **"does this endpoint honor the contract its callers already depend on — and did anyone record the decisions callers will need?"** An API is a promise: clients you cannot see build against the shapes, semantics, and error formats you ship. Most API pain in production is not broken logic — it is a silently changed shape, an unbounded list that grew, or an error format that differs from every sibling endpoint.
+
+## When to load this skill
+
+The diff touches ANY of: HTTP route or handler definitions, request/response types, OpenAPI/Swagger specs, gRPC `.proto` files or service implementations, GraphQL schemas or resolvers. If the diff is internal logic behind an unchanged API surface — unload; this checklist has nothing for you.
+
+## Marker semantics
+
+Every checklist item below carries a severity emoji AND a `[convention]` or `[correctness]` marker; both ride in the finding title so downstream tooling can act on them mechanically. `[convention]` findings are rigor-foldable (the orchestrator may lower them under a relaxed quality bar) and rejectable — but ONLY with cited evidence: a repo convention at file:line, or a recorded plan decision. `[correctness]` is reserved for contract breaks; those findings are neither foldable nor rejectable.
+
+## Linters and mechanized checks
+
+The review orchestrator runs mechanized checks (spec linters, schema diff tools, breaking-change detectors); your CONTEXT may already include their output — do not re-derive it. Spend your prose on what linters cannot reach: whether pagination is warranted, whether the auth decision was recorded, whether an evolution is actually compatible for real callers. If the repo plausibly warrants a linter config it lacks (an OpenAPI spec but no spec linter, protos but no breaking-change check), emit a 🟢 `[convention]` finding naming the gap.
+
+## The checklist (REST/HTTP)
+
+Severities below are the production bar. Each item is a context-sensitive question, not an absolute — read the surrounding code and the callers before flagging.
+
+### 1. 🟡 `[convention]` Unbounded collection endpoint without pagination
+
+Does the diff add or modify an endpoint that returns a collection? If the collection can grow without bound (rows in a table, user-generated items), missing pagination is a finding: name the endpoint, the backing query, and the growth vector. A provably bounded small set is exempt — an enum-backed list, a fixed config table, a per-user set with a hard cap — and when you rely on that exemption, say so explicitly in your notes so the next reviewer sees it was considered, not missed.
+
+### 2. 🟡 `[convention]` Route without a recorded authn/z decision
+
+For every new or changed route: is there a recorded decision that this route is public, authenticated, or role-gated? "Recorded" means visible in code or spec — middleware attached, an annotation, a spec `security` block, or an explicit comment for deliberately public routes. A route with no discernible decision is the finding. Whether the auth implementation is *bypassable* is not your question — that belongs to `security-review`; you only verify the decision exists and is stated.
+
+### 3. 🟡 `[convention]` Non-idempotent PUT/DELETE semantics
+
+PUT and DELETE carry idempotency promises by HTTP contract: repeating a PUT must converge on the same state; repeating a DELETE must not error in a way that breaks retrying clients (a second DELETE returning 404 or 204 is fine; returning 500 is not). Does the diff's handler honor the method it is mounted on — or should it be a POST? You check the *declared method semantics*; whether the state change is mechanically idempotent under retries and concurrency belongs to `transactional-integrity`.
+
+### 4. 🟡/🟢 `[convention]` Error responses leaking internals or inconsistent error shape
+
+Read the error paths: do responses leak internals — stack traces, SQL fragments, internal hostnames, framework default error pages (🟡)? Do they match the error shape the repo's sibling endpoints already return — same envelope, same code/message fields (🟢 when merely inconsistent)? `fs_grep` a sibling handler's error response to establish the house shape before flagging. Whether a leak is *exploitable* is `security-review`'s call; yours is the contract and consistency question.
+
+### 5. 🔴 `[correctness]` Breaking a published request/response shape without versioning
+
+Does the diff remove or rename a field, change a type, tighten accepted input, or change status codes on an endpoint that is already published (in a released spec, consumed by known clients, or exposed beyond this repo)? That is a contract break — a 🔴 `[correctness]` finding unless the change ships behind a new version (path version, header version, or an additive evolution that old clients tolerate). Verify "published" before firing: an endpoint added earlier in this same unreleased branch is not published, and changing it freely is fine.
+
+### 6. 🟡 `[convention]` Missing boundary input validation
+
+At the request boundary, is input validated at all — types enforced, required fields checked, sizes/ranges bounded — before it flows inward? Absence of any validation on a new input path is the finding. Whether unvalidated input is *exploitable* (injection, traversal) is deferred to `security-review`; you flag the missing guardrail, not the attack.
+
+## gRPC section (apply when the diff touches protos or gRPC services)
+
+- 🟡 `[convention]` **Deadlines** — do new client calls set deadlines, and do servers propagate the caller's deadline to their own outbound calls? A call chain with no deadline anywhere hangs forever on a stuck dependency.
+- 🟡 `[convention]` **Status-code discipline** — do handlers return meaningful gRPC status codes (`NOT_FOUND`, `INVALID_ARGUMENT`, `ALREADY_EXISTS`) rather than collapsing every failure into `UNKNOWN`/`INTERNAL`? Callers branch on these codes; a flattened code space breaks their error handling.
+- 🔴 `[correctness]` **Backwards-compatible proto evolution** — on a published proto: no field-number reuse, no type changes on existing fields, no renumbering, new fields optional with fresh numbers. Violating any of these silently corrupts data for old clients — same contract-break bar as item 5.
+- 🟢 `[convention]` **Field deprecation** — removed fields should be `reserved` (number and name) and deprecations marked with the `deprecated` option, not deleted outright, so the number can never be reused.
+
+## GraphQL section (apply when the diff touches schemas or resolvers)
+
+- 🟡 `[convention]` **Resolver N+1** — does a new list-field resolver fetch per-item (a query inside a loop, or a per-parent resolver hitting the DB)? Look for a dataloader/batching layer; its absence on a list path is the finding.
+- 🟡 `[convention]` **Depth/complexity limits** — if the diff grows the schema's reachable graph (new nested relations), is there a depth or complexity limit configured anywhere? An unlimited schema is a self-service DoS invitation; check the server setup before assuming.
+- 🟡 `[convention]` **Connection-style pagination** — list fields over unbounded collections should use the repo's established pagination idiom (connections/edges or equivalent). The same bounded-set exemption as REST item 1 applies — and state it when you use it. Breaking a published schema field without a deprecation cycle falls under item 5's 🔴 `[correctness]` bar.
+
+## Ground-truth discipline
+
+- READ the route registration and middleware chain, not just the handler — auth decisions and pagination defaults often live up-stack.
+- `fs_grep` for the spec file (OpenAPI, proto, GraphQL schema) that publishes the shape the diff changes; the spec, not the struct, is the contract.
+- Check sibling endpoints for the house error shape, pagination idiom, and auth annotation style before flagging deviation — the strongest finding cites the sibling at file:line.
+
+## What this skill does NOT check
+
+- Whether auth is bypassable, input is exploitable, or errors leak abusable secrets → `security-review`.
+- Whether state-changing handlers are mechanically idempotent, atomic, or retry-safe → `transactional-integrity`.
+- Log lines, levels, and message conventions in handlers → `logging-discipline`.
+- Metrics, alerts, and dashboards for new endpoints → `observability-review`.