chore(harness): make /pr-review a true verification twin (adversarial Phase 6 + /second-opinion)

.claude/skills/_shared/verification-protocol.md

@@ -0,0 +1,117 @@
+<!--
+Mirrored (body verbatim) from orbit-ui-mobile/.claude/skills/_shared/verification-protocol.md
+— that copy is the source of truth. Keep the two in lockstep: edit the ui-mobile original,
+then re-sync here (a plain `diff` past this note must be empty). In orbit-api the only
+consumer is `/pr-review`; the `/audit-*` rows below describe the ui-mobile skill family and
+don't run here — kept verbatim on purpose, not forked. Paired with the /second-opinion PR.
+-->
+# Orbit Review/Audit Verification Protocol
+
+The shared reliability layer for every review and audit skill in this repo —
+`/pr-review`, `/audit-security`, `/audit-tests`, `/audit-performance`,
+`/audit-code-quality`, and `/prod-readiness`. Where `pr-review/rubric.md` defines *what*
+these skills look for, this file defines *how* they stay trustworthy: nothing in scope is
+silently skipped, no serious finding ships without surviving a challenge, and the report
+states what it did **not** do.
+
+One file, every skill reads it — so the reliability bar can never drift between them (the
+same no-drift principle as the shared rubric). Each consuming skill names, in its own
+pipeline, a **Verify** phase and a **Deferred ledger** section that apply the mechanisms
+below, and declares which it runs — a bounded diff needs less than a repo-wide sweep (see
+**Calibration**).
+
+---
+
+## 1. Coverage contract — inventory first, then nothing falls through
+
+Before judging anything, enumerate the full scope as a concrete list: every
+file / endpoint / surface / test-area / hot-zone the skill is responsible for. That list
+is **binding**. At the end, every item is either **(a) covered with a verdict** or **(b)
+in the Deferred ledger with a one-line reason**. There is no third "silently never looked
+at" bucket — that bucket is the failure this whole protocol exists to kill.
+
+- Build the inventory from the skill's own scope phase (the diff's changed files, the
+  repo's source globs, the attack surfaces, the critical paths).
+- **Rank it worst-first** so the highest-blast-radius items are examined even if the run
+  is cut short: by tier (security), critical-path (tests), scaling impact (performance),
+  blast-radius × churn (code-quality), or touched-surface severity (pr-review). A hot,
+  frequently-edited file outranks a stable leaf carrying the same smell.
+- If the scope is too large for one pass, that is a Deferred-ledger entry naming exactly
+  what was left and why — never an unstated gap.
+
+## 2. Adversarial verification — every serious finding survives a challenge
+
+A finding that reaches the report has **survived an independent attempt to refute it.**
+False positives are the fastest way to get an audit ignored; this is the mechanism that
+kills them.
+
+- For each **Critical / High** finding (**Tier 1 / Tier 2** for security), spawn an
+  independent skeptic subagent whose only job is to **disprove** it: read the cited
+  `file:line` in full context and argue why the code is actually correct — the path is
+  unreachable, the input already validated, the finding a duplicate, the severity
+  inflated.
+- The skeptic **defaults to refuted when uncertain.** The burden is on the finding to
+  prove it is real, not on the skeptic to prove it isn't.
+- A finding the skeptic refutes with evidence is **dropped** (or downgraded, with the
+  skeptic's note attached). A finding that survives ships with confidence.
+- Run skeptics concurrently (respect the 3-subagent cap). **Diverse lenses beat
+  repetition:** where a finding can fail more than one way, give skeptics distinct angles
+  — is it reachable? is the input trusted? is it already handled elsewhere? is the
+  severity right?
+- Medium / Low / Info findings don't each need a skeptic, but the honesty bar (§5) still
+  applies — don't pad.
+
+## 3. Loop-until-dry + completeness critic — repo-wide audits only
+
+A single pass misses the tail. For an unbounded scope, the skill does not stop after one
+sweep — it asks what it missed and goes again until a round comes back empty.
+
+- After the first finding pass, run a **completeness critic**: a fresh agent asked *"what
+  did this audit NOT examine — a surface never swept, a file skipped, a pattern only
+  half-searched, a claim left unverified?"* Its output is the next round's work-list.
+- Spawn a focused finder round against each gap the critic names. Repeat until a round
+  surfaces **nothing new**, or a stated bound is hit (e.g. 2 consecutive dry rounds) —
+  **log the bound** so a cap never reads as completeness.
+- Gated to **repo-wide** scopes. A bounded diff (`/pr-review`) is its own completeness
+  boundary: one completeness pass over the changed surface is enough — do not loop.
+
+## 4. Deferred ledger — say what you did NOT do
+
+The report carries an explicit **Deferred** section. Every item that was in scope but
+given no verdict — out-of-scope by tier, N/A by gate, skipped for size, unverifiable in
+CI — is listed there with a one-line reason. **Silence reads as coverage**, so "not
+examined" is stated, never disguised as "clean."
+
+- If the run bounded itself (top-N, sampled, no sibling repo in CI, capped findings), the
+  ledger names exactly what that dropped.
+- A Tier-3 / enterprise-only control acknowledged-and-skipped is one line here, not a
+  finding each.
+
+## 5. Honesty clause
+
+Coverage you did not achieve is never implied. Severity you cannot justify is downgraded.
+A finding with no concrete `file:line` + reproduction is not reported. A clean area earns
+a plain "None," not an invented nit. These bind every mechanism above — and a skill that
+manufactures findings to look thorough has failed this protocol, not passed it.
+
+---
+
+## Calibration — which mechanisms each skill runs
+
+| Skill | Coverage contract | Adversarial verify | Loop-until-dry | Deferred ledger |
+|---|---|---|---|---|
+| `/pr-review` (bounded diff) | changed files, ranked by surface severity | each Critical/High before posting | one completeness pass, no loop | N/A dimensions + out-of-diff defers |
+| `/audit-security` (repo-wide) | attack surfaces, by tier | each Tier 1/2 finding | yes, until dry | Tier 3 + unswept surfaces |
+| `/audit-tests` (repo-wide) | critical paths, criticality-first | each Critical/High gap | yes, until dry | non-critical paths + policy-excluded suites |
+| `/audit-performance` (repo-wide) | hot zones, by scaling impact | each High finding | yes, until dry | enterprise-only tuning |
+| `/audit-code-quality` (repo-wide) | code slices, blast-radius × churn | each Critical/High finding | yes, until dry | dimensions deferred to other audits |
+| `/prod-readiness` (orchestrator) | the four audits + ops checks | inherits each child audit's verify | inherits each child's loop | merges every child ledger |
+
+---
+
+## Self-application
+
+This protocol is held to its own bar. A skill that claims to run it must actually emit the
+**Verify** phase and the **Deferred ledger** in its output — otherwise it is not running
+it, it is only saying so. An audit that skips its own coverage contract is the exact
+failure this file exists to prevent.

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

@@ -16,6 +16,19 @@ severity-ranked report — posted to the PR when the scope is a PR.
 This skill subsumes the old `/review` and `/security-review` commands: it does
 everything both did and adds the backward-compat guard and a single shared rubric.
 
+<!--
+Lockstep twin of orbit-ui-mobile/.claude/skills/pr-review/SKILL.md — keep them behavior-
+aligned (Stage-6 rule: the copies can't be deduped across two repos + CIs, so they stay
+aligned by hand). Both walk the shared rubric, run the adversarial Phase 6
+(`.claude/skills/_shared/verification-protocol.md`) with the cross-model `/second-opinion`
+step on surviving Critical findings, and end decisively APPROVE / NEEDS WORK. Sanctioned
+differences: default repo (api here / ui there) + the mirror-image `ui#`/`api#` selector, the
+subagent set (security-reviewer + contract-aligner here; parity/i18n/design are ui-only), and
+`dotnet` validate vs the ui `/validate` skill. `opencode` is absent in orbit-api CI (this
+copy's only runtime), so the second-opinion step degrades to UNAVAILABLE here — identical to
+how the ui copy behaves in ui CI.
+-->
+
 **Golden rule**: every finding is constructive and actionable — a clear fix, a file:line,
 and the rule it traces to. Severity is about blast radius, not which dimension raised it.
 
@@ -86,6 +99,8 @@ In parallel:
 - The plan in `.claude/plans/completed/` if the PR body references one.
 - **`.claude/skills/pr-review/rubric.md`** — the dimensions, severities, and finding
   template this review walks.
+- **`.claude/skills/_shared/verification-protocol.md`** — the shared reliability contract;
+  its Verify phase and Deferred ledger run below.
 
 Understand intent: for a PR read the title, body, and linked issue; for a file
 understand its role; for staged changes, what is in flight.
@@ -108,6 +123,12 @@ FEATURES.md parity.
 
 Focus on changed code, not pre-existing issues — unless a pre-existing issue is Critical.
 
+**Coverage contract (verification protocol §1):** the diff's changed files are the binding
+inventory — rank them worst-first (highest-blast-radius / most-churned files and the
+trust-boundary + contract surfaces before stable leaves) so the riskiest code is reviewed
+even under pressure, and every changed file ends with a verdict or in the Deferred ledger.
+Nothing changed is silently skipped.
+
 Apply the rubric's **Signal gate**: post Critical/High and concretely-actionable Medium only — drop Low/Info nits and style preferences (manufacturing nits to avoid approving is a defect). The outcome is deterministic: **NEEDS WORK** iff any Critical/High finding survives, otherwise **APPROVE**.
 
 ---
@@ -165,7 +186,49 @@ not over-claim completeness here.
 
 ---
 
-## Phase 6 — Validate
+## Phase 6 — Verify findings (adversarial)
+
+Run `.claude/skills/_shared/verification-protocol.md` before validating — every finding
+that will decide the outcome has to survive a challenge first.
+
+1. **Adversarial pass (§2).** For every **Critical / High** finding (including any
+   `⚠️ breaks old mobile clients`), spawn an independent skeptic subagent (3 concurrent)
+   whose only job is to *refute* it — read the cited `file:line` in full diff context and
+   argue it is a false positive (the path is unreachable, the value already validated, the
+   field actually still present or optional-and-unused with the grep to prove it, a
+   duplicate, the severity inflated). Default to refuted when uncertain. Drop or downgrade
+   anything the skeptic disproves — a false Critical that blocks a clean PR is as costly as
+   a missed one. The survivors decide the recommendation.
+2. **Cross-model second opinion (§2, Critical survivors — interactive only).** For each
+   **Critical** finding that survives step 1 (including any `⚠️ breaks old mobile clients`),
+   fire **`/second-opinion`** so a *different* model (GLM-5.2 via opencode) independently
+   judges it — pipe the finding dossier (title · severity · `repo/path:line` · the claimed
+   defect · the cited code hunk) to `node .claude/skills/second-opinion/second-opinion.mjs`
+   and apply the JSON verdict it prints:
+   - **AGREE** → the finding is cross-model corroborated; keep the severity, note the
+     confirmation.
+   - **DISAGREE** → tag the finding **`CONTESTED`** and record GLM's `reasoning` beside
+     Claude's; surface **both** verdicts in the report. It stays Critical — the
+     disagreement is the human's to resolve. **Never** let it force a merge or silently drop
+     the finding (the skeptic in step 1 already owns the drop decision).
+   - **UNSURE** → note it; the finding stands as step 1 left it.
+   - **UNAVAILABLE** (opencode absent — **always the case in CI**, or capped / offline) →
+     skip the second opinion, leave the finding unchanged, and state it in one line. Never
+     read "couldn't ask" as agreement. This graceful-degradation path keeps the CI review
+     (no opencode) byte-for-byte identical to today.
+   Scope to **Critical only** (not High) — cross-model time/cost is reserved for the findings
+   that actually block, per the on-demand-diversity budget (research.md). CONTESTED never
+   changes the deterministic recommendation: a surviving Critical still means NEEDS WORK.
+3. **Completeness pass (§3).** One pass only — a diff is its own boundary, so no loop: ask
+   *"what changed file or hunk did I not give a verdict, what dimension did I mark N/A
+   without checking its surface?"* and close the gap before reporting.
+4. **Deferred ledger (§4).** Every dimension marked N/A and every changed file not
+   verdicted goes into the report's **Deferred** line with a one-line reason — so "clean"
+   never hides "not looked at."
+
+---
+
+## Phase 7 — Validate
 
 Run the backend checks from the orbit-api root:
 
@@ -180,7 +243,7 @@ phase is skipped — Build / Unit Tests run as separate required checks.
 
 ---
 
-## Phase 7 — Report
+## Phase 8 — Report
 
 Write the report, then post it to the PR when the scope is a PR.
 
@@ -203,7 +266,10 @@ mkdir -p .claude/reviews
 ## Findings
 
 ### Critical
-{findings in the rubric template, or "None" — `⚠️ breaks old mobile clients` findings sort here first}
+{findings in the rubric template, or "None" — `⚠️ breaks old mobile clients` findings sort here first.
+A finding a cross-model second opinion disputed carries a **`CONTESTED`** tag with both
+verdicts inline — e.g. "Claude: Critical · GLM-5.2: DISAGREE — {GLM's reasoning}" — so the
+human sees the disagreement. It stays Critical; the tag never downgrades it.}
 
 ### High
 {… or "None"}
@@ -228,6 +294,12 @@ mkdir -p .claude/reviews
 | Build (dotnet) | PASS / FAIL / N/A |
 | Tests (dotnet) | PASS / FAIL / N/A |
 
+## Deferred — N/A dimensions & files not verdicted
+
+{Per the verification protocol §4: each dimension marked N/A (with why its surface wasn't
+touched) and any changed file not given a verdict — one line each. "Nothing deferred" if
+every dimension and file got a verdict.}
+
 ## What's good
 
 {positive observations}
@@ -256,8 +328,10 @@ endpoint / `mcp__github_inline_comment__create_inline_comment`.
 
 - **CI wrapper** (`.github/workflows/claude-review.yml`) invokes this skill: it owns the
   single decisive post — produce the report + recommendation and let it submit (skip this
-  posting step). In CI also skip Phase 6, and mark any dimension that needs the
-  un-checked-out sibling repo as "not verifiable in CI".
+  posting step). In CI also skip Phase 7 (Validate) — Build / Unit Tests run as separate
+  required checks — and mark any dimension that needs the un-checked-out sibling repo as
+  "not verifiable in CI". The Phase 6 adversarial pass still runs; its `/second-opinion` step
+  returns UNAVAILABLE (no `opencode` in CI) and the findings stand.
 - **Local, a PR you do NOT own**: post the decisive review yourself per the recommendation.
 - **Local, your OWN PR** (GitHub blocks self-approval): write the report and post it with
   `--comment` instead — do not fail trying to `--approve`.