feat(skills): add nemoclaw-maintainer-quick-wins skill

.agents/skills/nemoclaw-maintainer-quick-wins/JUDGMENT-CHAIN.md

@@ -0,0 +1,96 @@
+# Judgment Chain — two-lens review
+
+Run per candidate, fail-fast. A failure here skips Karpathy + testing and routes to the appropriate outcome.
+
+This chain encodes two complementary review lenses that surface different classes of problem:
+
+- **Scope & Coverage Lens** — Is the PR doing one thing, and is the risky path tested? Catches grab-bag PRs, scope drift, and untested behavior changes on critical code.
+- **Substrate Sequencing Lens** — Is the PR the right *size* and shape? Catches PRs that should be split into substrate-first, then-fix steps (extract helper → test → land fix on top).
+
+The labels "Scope Lens" and "Sequencing Lens" are how this skill refers to them. Each team will recognize their own version of these two reviewer archetypes.
+
+## 1. Scope check (Scope Lens)
+
+**Question:** Does the PR have one clear objective, or is it a grab-bag?
+
+**Red flags:**
+
+- Unrelated config churn (editor settings, tsconfig tweaks)
+- Drive-by refactors in files tangential to the stated fix
+- Tool-setting diffs bundled with a behavior fix
+- Multiple "bonus" fixes in the body
+
+**Outcome:** Grab-bag → `RESHAPE`. Ask the author to revert the extraneous changes to main and keep one objective.
+
+**Reference example:** A PR stated "reject symlinks on `~/.nemoclaw`" but also migrated 8 call sites across two packages and did a `process.env.HOME` cleanup. Routed to `SEQUENCE` (see check 4).
+
+## 2. Intent preservation
+
+**Question:** Does the diff match what the contributor / linked issue described?
+
+**Red flags:**
+
+- Semantic drift (body says "fix X", diff changes Y)
+- Test plan checklist items unchecked `[ ]` for behavior claims
+- Linked issue's acceptance criteria not addressed
+
+**Outcome:** Semantic drift → stop, flag, ask. Don't proceed to coverage/size checks; the reshape decision depends on what the author actually meant.
+
+## 3. Coverage-first framing (Scope Lens)
+
+**Question:** Are the risky code paths covered by some test, in this PR or pre-existing?
+
+**Risky paths** come from `.agents/skills/nemoclaw-maintainer-day/RISKY-AREAS.md` (or the equivalent risky-paths registry in your repo):
+
+- Installer / bootstrap shell (`install.sh`, `setup.sh`, `scripts/*.sh`)
+- Onboarding / host glue (`src/lib/onboard.ts`, CLI launcher)
+- Sandbox / policy / SSRF (security-critical paths)
+- Workflow / enforcement (`.github/workflows/`, prek hooks, DCO)
+- Credentials / inference / network (credential helpers, inference routing, approval flows)
+
+**Extended set** — credential-adjacent paths like `src/lib/config-io.ts`, `src/lib/safe-dir.ts` — anything that reads/writes under `~/.<your-tool>`.
+
+**Outcome:** Risky path touched with no test → `BLOCK` regardless of how clean the diff looks. The underlying principle: **automated behavioral verification is just testing.** If the behavior isn't tested, the team can't tell whether a future refactor broke it.
+
+**Reference example:** A PR touched `src/lib/agent-onboard.ts` (risky: onboarding). Zero tests added for the new behavior even though sibling tests for the same function existed. Verdict: `BLOCK`.
+
+## 4. Substrate-first slicing (Sequencing Lens)
+
+**Question:** Is the PR the right *size*?
+
+**Principle:** Extract helper → add tests for current behavior → land fix on top. One file cluster per pass.
+
+**Red flags:**
+
+- A "fix" that's actually a redesign
+- New utility + migration of many call sites in one PR
+- Cross-package changes without a clear split
+
+**Outcome:** Too big / multi-step disguised as single → `SEQUENCE`. Propose the split explicitly.
+
+**Reference example:** A PR was right-intent / wrong-shape. Proposed split:
+
+1. PR 1: add `safe-dir.ts` + unit tests (for both packages)
+2. PR 2: migrate 7 non-`config-io` call sites
+3. PR 3 (optional): orthogonal `process.env.HOME → os.homedir()` cleanup
+
+## Output format
+
+Per candidate, produce a pass/fail table:
+
+| Check | Result | Notes |
+|-------|--------|-------|
+| Scope | ✅ / ❌ | — |
+| Intent | ✅ / ❌ | — |
+| Coverage | ✅ / ❌ | risky paths: ... tests: ... |
+| Size / Sequencing | ✅ / ❌ | lines, files, split proposal if needed |
+
+Then the routing decision (APPROVE-path / RESHAPE / BLOCK / SEQUENCE) and whether to proceed to the Karpathy lens.
+
+## Note on the team's review philosophy
+
+The two-lens framing here encodes the consensus the team has formed about what makes a mergeable PR — scope discipline + size discipline. Each team will have its own variant. If your repo maintains a written log of review principles (decisions made over time about what's blocking vs. nit, what's risky-area, what's an acceptable scope), this skill should treat that log as the source of truth and adapt the lens questions to match.
+
+## New verdicts added post-spec
+
+- `CLOSE-AS-SUPERSEDED` — added after a PR turned out to already be fixed in a sibling merged PR. The same-fix-check step runs *before* the judgment chain to catch this early.

.agents/skills/nemoclaw-maintainer-quick-wins/KARPATHY-LENS.md

@@ -0,0 +1,105 @@
+# Karpathy Review Lens
+
+Applied to candidates that survive the judgment chain. Adapted from `andrej-karpathy-skills:karpathy-guidelines`. Same four checks, applied to someone else's code (PR mode) or our own (issue mode / salvage mode).
+
+## 1. Hidden assumption scan
+
+**Instruction:** Read the diff line-by-line. List every assumption the contributor made that **isn't stated** in the body or comments.
+
+For each assumption, do one of:
+
+- **Prove it safe** by grep/read of the other side of the boundary. Note the verification in conversation but don't dump into the PR comment.
+- **Flag as a gap** if you can't prove it safe. Gap goes into the draft PR comment.
+
+**Common assumption types:**
+
+- "Assumes X exists" (file, env var, config key) — grep for the reader/creator
+- "Assumes X is already validated/sanitized" — trace the input
+- "Assumes this runs before/after Y" — check ordering in caller
+- "Assumes no concurrent access" — check locking
+- "Assumes default value is safe" — read the factory/constructor
+
+**Live example (PR #2290):**
+
+- Assumption: `$HOME` is not itself a symlink. Flagged to contributor as a limitation to document.
+- Assumption: `lstatSync` check + subsequent `mkdirSync` is atomic. Proved unsafe (TOCTOU), documented as accepted tradeoff.
+- Assumption: `shellQuote` is imported. Verified via `grep -n "shellQuote" src/lib/config-io.ts`. Safe.
+
+## 2. Simplicity check
+
+**Instruction:** Is there a smaller version that does the same thing? Count lines that **don't trace to the stated objective**.
+
+**Flag:**
+
+- Speculative flexibility / config knobs for no current user
+- Error handling for impossible cases
+- Premature abstractions
+- New classes/utilities where a plain function would do
+- Renaming done opportunistically
+- Dead code added "for future use"
+
+**Reminder:** The repo CLAUDE.md says: "No features beyond what was asked." Hold PRs to the same bar the contributor is held to.
+
+**Live example (PR #1954):**
+
+- `Math.min(token.length - 4, 20)` caps asterisks at 20. Description says "rest replaced with asterisks." Deviation from description is unexplained — either remove the cap or comment why. Flagged.
+
+## 3. Surgical-changes check
+
+**Instruction:** Scan for drive-by improvements **within in-scope files** — lines that don't trace to the stated objective but aren't in tangential files.
+
+**Flag:**
+
+- Formatting changes adjacent to the real edit
+- Comment rewrites
+- Variable renames not required by the fix
+- "While I was here" deletions of code not referenced by the fix
+- Whitespace-only diffs
+
+**Distinction from scope check:**
+
+- The Scope-Lens check catches *files* that don't belong.
+- The Karpathy surgical check catches *lines within scope files* that don't belong.
+
+## 4. Goal-driven verification
+
+**Instruction:** Translate the PR/issue objective into a test-shaped verifiable goal. Example transformations:
+
+| Stated objective | Verifiable goal |
+|------------------|-----------------|
+| "Add validation" | "A test with invalid input fails before this PR and passes after" |
+| "Fix crash when X" | "A test reproducing X fails before, passes after" |
+| "Redact token in URL" | "A test asserts the URL contains `****` after redaction and full token without" |
+
+Then check: does such a test **actually exist** in the PR?
+
+- If yes: verifiable. ✓
+- If no: that becomes a required new test. Route it to tier 1 as a `missing-test`.
+
+**Live example (PR #1954):**
+Stated: "Redact the gateway auth token in dashboard URLs printed to stdout."
+Verifiable goal: `expect(buildControlUiUrls("abcdefghij", 18789, true)[0]).toMatch(/#token=abcd\*+$/)`.
+Existing tests: 7 in `dashboard.test.ts`, none cover `forDisplay=true`. Missing-test → tier-1 required → coverage-first failure → `BLOCK`.
+
+## Output format
+
+Per candidate surviving the judgment chain:
+
+| Lens | Finding |
+|------|---------|
+| Hidden assumption | (file:line): ... |
+| Simpler | ... |
+| Surgical | ... |
+| Verifiable goal | test exists / missing test: ... |
+
+Feed the `missing-test` column directly into tier 1 testing as a required new test.
+
+## Interaction with salvage
+
+In salvage mode (we're writing the fix locally, not reviewing someone else's), the Karpathy lens applies to **our own diff** before calling it done. This is the cleanest form of the skill — self-review.
+
+Rules are stricter in salvage mode because we control the outcome:
+
+- Any missing-test goes in immediately, not as a "suggest for follow-up"
+- Any hidden assumption gets a comment or a test
+- Drive-by improvements are a firing offense — if we're salvaging, salvage only

.agents/skills/nemoclaw-maintainer-quick-wins/LESSONS-LEARNED.md

@@ -0,0 +1,137 @@
+# Lessons Learned — Live-Run Findings
+
+Each entry is a concrete finding from running the workflow. Read this first when invoking the skill; it encodes tradeoffs that a cold spec couldn't predict.
+
+## First live run
+
+**Candidates picked:** 3 PRs (mode D, top 3 from a 147-PR pool).
+**Time to verdict:** ~90 minutes of conversation for all three.
+**Outcomes:** 1 APPROVE + RFR, 1 BLOCK + salvage offer, 1 CLOSE-AS-SUPERSEDED. Then a 4th added → SEQUENCE.
+
+### Finding 1 — Same-fix-already-merged detection is mandatory
+
+**What happened:** A 1-line Dockerfile fix ranked in the top 10 because labels looked good and diff was tiny. Turned out the same fix had already shipped as a sibling PR merged that same day. Wasted ~15 min of review before catching it.
+
+**Fix to bake in:** Before the judgment chain, for every candidate PR, search merged PRs in the last 14 days. Match on:
+
+- Title token overlap ≥70%, OR
+- Same linked issue number in body
+
+```bash
+gh pr list --repo <owner>/<repo> --state merged --search "merged:>=$(date -v-14d +%Y-%m-%d)" \
+  --json number,title,body,mergedAt
+```
+
+If hit: verdict `CLOSE-AS-SUPERSEDED`, skip all further tiers.
+
+### Finding 2 — GraphQL 502 on heavy fields in bulk queries
+
+**What happened:** `gh pr list ... --json ...statusCheckRollup,mergeStateStatus,files` 502'd repeatedly. Dropping those fields made it work.
+
+**Fix to bake in:** Two-pass fetch.
+
+- Pass 1 (bulk, lightweight): `number,title,author,labels,createdAt,additions,deletions,isDraft`
+- Pass 2 (per-finalist, heavy): `body,mergeStateStatus,statusCheckRollup,files,reviewDecision`
+
+Don't combine. Don't retry the heavy bulk query more than once.
+
+### Finding 3 — Worktree `npm install` can hit semver bugs
+
+**What happened:** `npm install` in `/tmp/<repo>-pr-<PR>` threw `Invalid Version:` from a wasm-binding dedupe. Fresh install from cache was corrupted. Also: `prepare` script runs `npm install --omit=dev` which skips vitest.
+
+**Fix to bake in:** Skip per-worktree install. Symlink `node_modules` from the main checkout:
+
+```bash
+rm -rf /tmp/<repo>-pr-<PR>/node_modules
+ln -s <main-checkout>/node_modules /tmp/<repo>-pr-<PR>/node_modules
+```
+
+Works when lockfile is close to identical. If the PR changed `package-lock.json`, try `<INSTALLING_FLAG>=1 npm install --include=dev --no-audit --no-fund --ignore-scripts` once, fall back to symlink.
+
+### Finding 4 — Tier 2 routing list is too narrow
+
+**What happened:** A PR modified credential directory handling. Not in tier-2 routing list, so tier 2 was skipped. Got away with it because tests were thorough and Karpathy surfaced no gaps, but a less-tested PR at this level would slip through.
+
+**Fix to bake in:** Extend tier 2 triggers to include any path that reads/writes under `~/.<your-tool>`. Specifically: `config-io.ts`, `safe-dir.ts`, `onboard-session.ts`, `registry.ts`, `usage-notice.ts`.
+
+Better long-term: derive tier 2 from `RISKY-AREAS.md` (or equivalent) directly instead of a hardcoded list.
+
+### Finding 5 — Baseline diff on failures is not optional
+
+**What happened:** Tier 1 full suite reported 15 failures in `test/install-preflight.test.ts`. Looked like regressions. Ran the same file on `origin/main` — 15 failures reproduced. Pre-existing infra (curl-pipe tests fail with exit 127 in restricted shells).
+
+**Fix to bake in:** When tier-1 total failures > 0, automatically create a second worktree at `origin/main` and re-run the failing test files. Any tests that fail on main too are pre-existing, not regressions. Report them separately from actual regressions.
+
+### Finding 6 — Unchecked `[ ]` test-plan boxes are a risk signal
+
+**What happened:** A PR's body had:
+
+```text
+- [x] npm run build:cli — compiles cleanly
+- [ ] Manual: nemoclaw onboard shows redacted token
+- [ ] Manual: full token still retrievable
+```
+
+Two unchecked items for behavior claims, on a risky-area path. That's a coverage-lens failure surfaceable from the body alone.
+
+**Fix to bake in:** Parse the PR body for `[ ]` patterns. If any unchecked test-plan item AND the PR touches a risky-area path → auto-flag for coverage review, before even reading the diff. Counts as evidence for the Coverage check 3 verdict.
+
+### Finding 7 — New verdict: `CLOSE-AS-SUPERSEDED`
+
+Not in the original spec. Needed because same-fix-already-merged is a real category that isn't BLOCK (PR isn't wrong) or RESHAPE (there's nothing to reshape). Now in the verdict table.
+
+### Finding 8 — New verdict: `SEQUENCE` is load-bearing
+
+**What happened:** A PR was right-intent / wrong-shape: a new utility + migration of 8 call sites + a bonus cleanup, all in one PR. Not a BLOCK (not broken). Not a RESHAPE (can't just revert one file). The correct response is "split this into 3 PRs."
+
+**Fix to bake in:** When routing to SEQUENCE, the draft comment **must propose a concrete split**. Template:
+
+```text
+Agree with the intent. Asking to split this before merge:
+1. PR 1: <substrate — new utility + tests>
+2. PR 2: <migration — call sites in bounded cluster>
+3. PR 3 (optional): <orthogonal cleanup>
+```
+
+Without the proposed split, the SEQUENCE verdict isn't actionable.
+
+### Finding 9 — Status ledger needs clickable links
+
+**Maintainer feedback:** "add links to all these" — referring to the ledger and every PR mention.
+
+**Fix to bake in:** Every PR number in every output (ledger, findings tables, draft comments, workflow summaries) renders as `[#NNNN](url)`. Never use bare `#NNNN`.
+
+### Finding 10 — RFR format is different from the draft PR comment
+
+**Maintainer feedback:** First RFR attempt included line counts, test counts, CI status. Correction: "brief but impactful."
+
+**Rule:**
+
+- Draft PR comment (to the author): ≤30 lines, includes specifics (verdict, blockers, suggestions, test details) because the author needs actionable context.
+- RFR (to peer reviewers in chat / Slack): 2 lines total, impact-first, NO engineering details. Reviewers click through to the PR for details.
+
+Different audiences, different density.
+
+### Finding 11 — "Merge now" vs "fix first" must be unambiguous
+
+**Maintainer feedback:** Draft comment combined "LGTM" with "two minor follow-ups (non-blocking)." Author can't tell whether to merge or address the notes first.
+
+**Rule:** Pick one of two shapes. Don't mix.
+
+- **APPROVE, merge-as-is:** no listed follow-ups in the comment. File any genuinely-useful thoughts as a follow-up issue instead.
+- **REQUEST_CHANGES, fix first:** explicit list, no LGTM.
+
+The ambiguous middle ("approve with concerns") creates round-trips and wastes the contributor's time.
+
+---
+
+## Improvement backlog (not yet baked in)
+
+Flag these as TODO next time this skill runs:
+
+- **Pre-baked CI runner image** — Cuts tier 3 wall time from ~18 min to ~3 min when it lands. Blocks on the team's container-image-publishing plan.
+- **Behavioral-test auto-selection** — the previous skill iteration's e2e auto-selection logic is referenced but not present. Useful for tier 3 auto-suite-pick.
+- **Weight tuning for mode D** — first-run weights (`w1=2, w2=1.5, ...`) produced reasonable rankings but an obsolete PR landed at score 4.85 in the top 10, proving label signal alone isn't enough. After the same-fix check is in, re-tune.
+- **Auto-detect RESHAPE vs SEQUENCE** — currently both are made by eye. A rough heuristic: changed-files count > 5 AND new-file count > 0 → likely SEQUENCE. Changed-files includes unrelated areas → likely RESHAPE. Prototype and check against past judgments.
+- **Posting-to-GitHub path** — when the maintainer authorizes, add a `--post` flag that uses the draft comment / RFR / close-as-superseded outputs. Currently everything stays local.
+- **Issue mode (E) reactivation** — parked. Build `ISSUE-MODE.md` and re-enable for issue-first quick-wins when the team wants it.

.agents/skills/nemoclaw-maintainer-quick-wins/MULTI-MODEL-TESTING.md

@@ -0,0 +1,28 @@
+# Multi-model test plan — quick-wins
+
+## Models in scope
+
+| Model | Check |
+|---|---|
+| Claude Haiku 4.5 | Does Haiku follow Mode D scoring formula correctly without dropping terms? |
+| Claude Sonnet 4.6 | Does Sonnet apply the two-lens judgment chain (Scope/Coverage + Sequencing) in order? |
+| Claude Opus 4.7 (1M) | Does Opus over-elaborate the verdict beyond a ≤30-line draft PR comment? |
+
+## Pass criteria
+
+- Top-10 candidates ranked by Mode D formula (no drift)
+- Same-fix-already-merged check fires BEFORE judgment chain (avoids re-reviewing duplicates)
+- CODEOWNERS resolution: Python parser correctly handles last-match-wins and prefix patterns
+- Reviewer-load awareness: counts open review-requests, warns above threshold
+- For APPROVE verdicts: separate draft PR comment + draft RFR, never merged
+- For SEQUENCE verdicts: proposes a concrete split, not just "split this"
+
+## Known risks
+
+- Haiku may invoke tier 2 / tier 3 without justification; tighten the routing decision tree wording.
+- Sonnet may inline the RFR into the conversation as plain text instead of as a copy-paste block; ensure "Two outputs, kept separate" framing is prominent.
+- Opus may produce verbose Karpathy-lens findings; cap the findings table at the per-lens columns described.
+
+## How to run
+
+Same eval-iteration pattern. Verify the JSON sidecar's `judgment_chain`, `karpathy_findings`, and `reviewers_resolved` fields are populated correctly per the documented schema.