feat(skills): add autonomous PR comparator for issues with competing fixes

.agents/skills/nemoclaw-maintainer-pr-comparator/SKILL.md

@@ -0,0 +1,121 @@
+---
+name: nemoclaw-maintainer-pr-comparator
+description: Compares competing PRs that target the same issue and recommends which one to merge. Runs gate, correctness, and quality checks; outputs a deterministic scorecard with reasoning trace. Use when an issue has two or more open PRs and a maintainer needs to decide which to merge.
+user_invocable: true
+---
+
+# PR Comparator
+
+Picks the merge winner among competing PRs for a single issue. Tier 0 gates eliminate plumbing failures; Tiers 1-2 score correctness and quality; Tier 3 applies deterministic tiebreakers. Degraded mode handles the case where no PR passes gates.
+
+## Prerequisites
+
+- `gh` CLI installed and authenticated
+- A target repository with an issue that has 2+ open PRs
+
+## Repo policy
+
+Defaults assume NemoClaw conventions (security CODEOWNERS, DCO, CodeRabbit, `docs/` directory). For other repos, edit `repo-policy.md` to override.
+
+## Workflow
+
+Copy this checklist into your response and check off each step:
+
+```text
+PR Comparison Progress:
+- [ ] Step 1: Parse issue (body + comments) for acceptance criteria
+- [ ] Step 2: Discover candidate PRs (default-order search with stop conditions)
+- [ ] Step 3: Detect supersession (parse PR bodies)
+- [ ] Step 4: Run Tier 0 gates per PR
+- [ ] Step 5: Run Tier 1 correctness checks per PR
+- [ ] Step 6: Run Tier 2 quality checks per PR
+- [ ] Step 7: Compute weighted scores
+- [ ] Step 8: Apply Tier 3 ranking (happy path or degraded mode)
+- [ ] Step 9: Emit verdict using templates/verdict.md
+```
+
+### Step 1: Parse issue
+
+Extract acceptance criteria from issue body **and all comments**:
+
+```bash
+gh issue view <issue-number> --json title,body,comments
+```
+
+Read every comment — commenters often add asks the body doesn't capture.
+
+### Step 2: Discover candidate PRs
+
+```bash
+scripts/find-candidates.sh <issue-number>
+```
+
+Applies a single default order with stop conditions.
+
+### Step 3: Detect supersession
+
+```bash
+scripts/parse-supersession.sh <pr-number-1> <pr-number-2> ...
+```
+
+Parses each PR body for `supersedes #N`, `replaces #N`, `closes in favor of #N`, `folds in #N`. A PR that supersedes another wins ties immediately.
+
+### Step 4: Tier 0 gates
+
+```bash
+scripts/collect-gates.sh <pr-number>
+scripts/check-coderabbit-threads.sh <pr-number>
+```
+
+Five gates, all mandatory. See `checks/tier-0-gates.md` for the full list and interpretation.
+
+### Step 5: Tier 1 correctness
+
+Six checks, all LLM judgments. See `checks/tier-1-correctness.md` for evidence requirements per check.
+
+### Step 6: Tier 2 quality
+
+Three checks, all LLM judgments. See `checks/tier-2-quality.md`.
+
+### Step 7: Weighted score
+
+- Each pass = full points
+- Each yellow = half points
+- Each fail = zero
+- Tier 1 weight: 2.0× per check
+- Tier 2 weight: 1.0× per check
+
+### Step 8: Tier 3 ranking
+
+Branch on whether any PR passes all Tier 0 gates. See `tiebreakers.md` for happy-path tiebreakers, degraded-mode distance-to-ready ranking, and the behavior-coverage matrix.
+
+### Step 9: Emit verdict
+
+Use `templates/verdict.md`. Every judgment must carry evidence (file:line refs, diff snippets), reasoning chain, and the score it contributed.
+
+## Reference files
+
+- `repo-policy.md` — configurable defaults per target repo
+- `checks/tier-0-gates.md` — plumbing gates
+- `checks/tier-1-correctness.md` — six correctness checks
+- `checks/tier-2-quality.md` — three quality checks
+- `tiebreakers.md` — Tier 3 ranking and degraded mode
+- `templates/verdict.md` — output template
+- `validation/backtest.md` — backtest the skill against historical cases
+
+## Scripts (execute, do not read)
+
+- `scripts/find-candidates.sh` — PR discovery
+- `scripts/collect-gates.sh` — Tier 0 gate evaluation
+- `scripts/check-coderabbit-threads.sh` — GraphQL thread resolution
+- `scripts/parse-supersession.sh` — body parsing for supersession refs
+- `scripts/render-verdict.py` — verdict scorecard renderer
+
+## What this skill does NOT do
+
+These require infrastructure beyond GitHub API + LLM and are deferred to v2 modules:
+
+- Running each PR's code against adversarial inputs (sandboxed execution)
+- Cross-issue regression sweep (separate skill)
+- Revert simulation against neighbor PRs
+- Static analyzer integration (CodeQL, Semgrep)

.agents/skills/nemoclaw-maintainer-pr-comparator/checks/tier-0-gates.md

@@ -0,0 +1,61 @@
+# Tier 0 — Plumbing Gates
+
+Mandatory prerequisites. Any gate failure means the PR cannot be merged in its current state. Five gates total. Run `scripts/collect-gates.sh <pr>` to evaluate gates 1-4 mechanically; run `scripts/check-coderabbit-threads.sh <pr>` for gate 5.
+
+## Contents
+
+- Gate 1: PR state OPEN
+- Gate 2: CI green on latest head SHA
+- Gate 3: Mergeable, no conflicts
+- Gate 4: Branch protection satisfied
+- Gate 5: Automated reviewer threads resolved
+
+## Gate 1: PR state OPEN
+
+The PR's `state` must be `OPEN`. A `CLOSED` or `MERGED` PR is not a valid merge candidate regardless of its other properties.
+
+**Why this is a hard kill:** A closed PR cannot merge no matter how high it scores. Even degraded mode skips closed PRs entirely.
+
+**Failure mode this catches:** When two PRs are byte-identical and the older one was closed before review completed, first-mover would otherwise pick the closed one. This gate prevents that.
+
+## Gate 2: CI green on latest head SHA
+
+The CI rollup must show all required checks passing on the **latest** head SHA, not on a stale ancestor.
+
+**Why "latest" matters:** If the author force-pushed after CI ran, the green checks are on the old commit. The new commit may have introduced regressions that haven't been re-checked.
+
+**How to evaluate:** `scripts/collect-gates.sh` returns the head SHA and a per-check status. Cross-reference each required check against `repo-policy.md`'s required-checks list.
+
+## Gate 3: Mergeable, no conflicts
+
+`mergeable: MERGEABLE` and `mergeStateStatus: CLEAN`. The PR must merge cleanly into its base branch.
+
+**Common failure modes:**
+
+- `CONFLICTING` — base branch has diverged
+- `DIRTY` — staged changes block merge
+- `BLOCKED` — required checks failing or reviews missing
+
+## Gate 4: Branch protection satisfied
+
+`reviewDecision: APPROVED`, plus all branch-protection requirements (CODEOWNERS, DCO, required hooks). The skill defers to branch protection — it does NOT separately verify CODEOWNERS membership or DCO sign-off.
+
+**Why defer:** Branch protection rules are the source of truth. Re-implementing the check in the skill would drift from repo policy. If your repo doesn't enforce CODEOWNERS via branch protection, set `codeowners_enforced_via_branch_protection: false` in `repo-policy.md` and add explicit team checks.
+
+## Gate 5: Automated reviewer threads resolved
+
+All threads created by automated reviewers (e.g., CodeRabbit) must be in `resolved: true` state. **Zero unresolved threads is the bar.**
+
+**Why GraphQL, not REST:** GitHub's REST `/comments` endpoint exposes individual review comments without thread-resolution state. To check whether a thread is resolved, query `pullRequest.reviewThreads.isResolved` via GraphQL. This is what `scripts/check-coderabbit-threads.sh` does.
+
+**Configurable:** Add bot logins to `repo-policy.md` under `auto_reviewers`. Defaults to `coderabbitai` (CodeRabbit's bot login).
+
+## Output
+
+For each gate, the skill records:
+
+- Pass/fail
+- Evidence (head SHA, check names, mergeable state, thread IDs)
+- Whether the failure is **trivial** (auto-fixable: rebase, push sign-off) or **substantive** (real work: CI red, conflicts, missing approvals)
+
+The trivial/substantive split feeds degraded mode (see `tiebreakers.md`).

.agents/skills/nemoclaw-maintainer-pr-comparator/checks/tier-1-correctness.md

@@ -0,0 +1,86 @@
+# Tier 1 — Correctness Checks
+
+Six 1%-clever LLM judgments. Each catches a failure mode CI cannot see. Score: pass = 1, yellow = 0.5, fail = 0; weight 2.0× per check. Every judgment must carry file:line evidence.
+
+## Contents
+
+- 1.1 Test exercises bug path
+- 1.2 Comment-as-spec coverage
+- 1.3 Negative test coverage
+- 1.4 Coverage shape
+- 1.5 Refactor-vs-behavior scan
+- 1.6 Mocking purity
+
+## 1.1 Test exercises bug path
+
+The PR's new/modified test must, when run on the pre-fix code, fail. A test that passes both before and after the fix proves nothing.
+
+**How to evaluate:** Read each new test in the diff. For each assertion, ask: "Would this assertion have held on the pre-fix code?" If yes, the test doesn't exercise the bug.
+
+**Common false-positive patterns to flag as yellow:**
+
+- Test calls the function but only asserts "no exception thrown"
+- Test asserts on output that's unrelated to the bug
+- Test mocks the very behavior the bug was in
+
+**Evidence to record:** Diff line of the assertion + the bug's pre-fix behavior + reasoning that the assertion would have failed pre-fix.
+
+## 1.2 Comment-as-spec coverage
+
+Acceptance criteria come from the issue body **and every comment**. Commenters often add asks the body doesn't capture: "and don't break Y while you're at it." All asks must map to a fix or test in the diff.
+
+**How to evaluate:** From the issue's parsed criteria checklist (Step 1 of the workflow), check each item against:
+
+- Files changed in the diff
+- New tests in the diff
+- PR body's "Changes" section
+
+**Yellow if:** Some criteria are addressed but one or two are missing without explanation.
+**Fail if:** Half or more criteria are unaddressed.
+
+## 1.3 Negative test coverage
+
+The fix must have tests for invalid/edge inputs, not just the happy path that matches the reported bug.
+
+**Look for assertions on:**
+
+- Empty / null / undefined inputs
+- Boundaries (0, max, min, off-by-one)
+- Type confusion (string where number expected)
+- Malformed input
+- Whitespace-only / non-ASCII / unicode
+
+**Adapt for the input domain:** A Dockerfile change needs different "negative cases" than an HTTP handler. For infrastructure changes, package-presence assertions and version-pin assertions count.
+
+**Yellow if** only happy-path is tested. **Fail if** the bug class has obvious negative cases and none are covered.
+
+## 1.4 Coverage shape
+
+Every new code path in the diff has a test. Standard coverage % does not catch this — coverage can stay flat while new branches are untested if they're hit incidentally by unrelated tests.
+
+**How to evaluate:** For each new branch (`if`, `else`, `try`/`catch`, `switch` arm) in the diff, find a test that exercises that branch specifically. A new `else` branch with no test is a yellow.
+
+## 1.5 Refactor-vs-behavior scan
+
+If the PR's title or description claims `refactor` / `rename` / `extract` / `move`, the diff must be net-zero in:
+
+- Conditional adds (`if(`, `?`, `&&`, `||`)
+- New `throw new Error(`
+- Changed `process.exit(` codes
+- Changed return values
+
+**How to evaluate:** Run a count over the diff for these tokens. Net-zero is normal — code moves around. Net-positive in any of these = hidden behavior change inside what claims to be a refactor → yellow or fail depending on magnitude.
+
+**Why this catches real bugs:** Authors sometimes "fix while refactoring" without flagging it. The fix may be correct, but landing it as a refactor bypasses the review attention a behavior change would get.
+
+## 1.6 Mocking purity
+
+Tests must isolate **external** dependencies (network, filesystem, time, randomness, third-party APIs), not replace the **unit under test**. If a test for `validateInput()` mocks `validateInput()` itself, the test proves nothing.
+
+**How to evaluate:** Read each new test's mock setup. For each mock, ask: "Is this mock replacing an external dependency, or is it replacing the function this test is supposed to verify?" If the latter, fail.
+
+**Common red flags:**
+
+- Mocking the function whose name appears in the test description
+- Mocking a function and asserting only that the mock was called (without verifying the calling code's logic)
+- Mocking deep into the unit under test's call graph rather than at the external boundary

.agents/skills/nemoclaw-maintainer-pr-comparator/checks/tier-2-quality.md

@@ -0,0 +1,64 @@
+# Tier 2 — Code Quality Checks
+
+Four 1%-clever LLM judgments. Score: pass = 1, yellow = 0.5, fail = 0; weight 1.0× per check.
+
+## Contents
+
+- 2.1 Description-vs-diff drift
+- 2.2 Migration completion
+- 2.3 Public surface preservation
+- 2.4 Workaround-vs-root-cause
+
+## 2.1 Description-vs-diff drift
+
+Every touched file must be named or implied by the PR description's "Changes" section. Files outside the stated scope are "while I'm here" tweaks — common in long stacks where authors are heads-down and unrelated cleanups drift in.
+
+**How to evaluate:** From `gh pr view <pr> --json files,body`, build a set of touched files. Cross-reference against files named in the description. Files not named or implied by the description = yellow flag.
+
+**Implied counts as covered:** If the description says "extracts onboard parsing into a new module," touching both `onboard.ts` and a new `onboard-parser.ts` is implied. But touching `unrelated-helper.ts` is not.
+
+## 2.2 Migration completion
+
+If the PR adds a new path (oclif version, v2 helper, replacement function, new format), the old path must be either:
+
+- Deleted in this PR, OR
+- Linked to a follow-up PR/issue in the body
+
+Both surviving with no follow-up link = incomplete migration → yellow.
+
+**How to evaluate:** Look for diff additions that name a "v2" / "new" / "oclif" version of an existing symbol. Then grep the post-PR codebase for the old symbol's usages. If callers still use the old path AND the body has no follow-up link, flag.
+
+**Why this catches real bugs:** Half-migrations create maintenance debt. The new path drifts ahead, the old path bit-rots, callers get confused about which to use.
+
+## 2.3 Public surface preservation
+
+For any content **change** (not pure move) in:
+
+- Flag definitions (`--name`, `Flags.<x>(`, oclif flag schemas)
+- Help/usage strings (`Usage:`, `description:`, `summary:`)
+- Error messages (`throw new Error(`, `console.error`)
+- Exit codes (`process.exit(`)
+
+…the PR body must have a Notes section explaining the change, AND the corresponding docs files (per `repo-policy.md`'s `docs_dir`) must be updated.
+
+**Distinguishing moves from changes:** Pure moves (added in one file, removed in another with same string content) are fine — no Notes or docs update needed. The check is for **content changes**: adding a new flag, renaming an existing one, rewriting an error message.
+
+**Yellow if:** Content changes are present but no Notes section.
+**Fail if:** Content changes change user-facing behavior AND no Notes AND no docs update.
+
+**Why this catches real bugs:** Authors often make small UX changes (error message wording, help format) without realizing they're public-surface changes. End users notice. Forcing a Notes section forces awareness.
+
+## 2.4 Workaround-vs-root-cause
+
+Grep the diff for symptom-suppression patterns:
+
+- `try { ... } catch { /* empty or swallow */ }` blocks
+- `catch (err) { return; }` with no rethrow or logging
+- `if (err.code === '<errno>') return` (errno-specific silent ignores like EACCES, ENOENT, EEXIST)
+- Defensive returns in error paths that hide failures from callers
+
+If any are added in the diff, the PR body must (a) link to a follow-up issue for the root-cause fix, OR (b) explain why the suppression is the correct behavior (e.g., "expected during shutdown, callers handle absence elsewhere"). Without (a) or (b) → yellow.
+
+**Why this catches real bugs:** Symptom-suppression hides bugs without fixing them. The same code can fail in production for a different reason and now no one sees it. Forcing a justification or follow-up makes the cost-of-suppression visible.
+
+**Score key:** Tier 2 has 4 checks total. Max contribution per PR = 4 × 1.0 = 4.0 points. Combined with Tier 1's max of 12.0 (6 checks × 2.0), the overall max weighted score is **16.0**.