merge: main (7d) into 7c

Union of the two new Skills rows in CLAUDE.md and README.md.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

tests/lib/skills-review-prose-pins.test.mjs

@@ -0,0 +1,158 @@
+// tests/lib/skills-review-prose-pins.test.mjs
+// Wave 3, order 7d: the `reviewing-adversarially` skill and its wiring
+// (contract S4).
+//
+// What is pinned, and why:
+//   • the skill's form is not pinned here: the form check
+//     (lib/skills/skill-lint.mjs) and the routing test (lib/skills/routing.mjs)
+//     run over every skill in `npm test` and are the authority (contract S1);
+//   • the rules the order names stay in the skill, and its "Enforced by" says
+//     which part a check holds (a finding with no source) and which part is
+//     prose only (praise and flattery);
+//   • the three review agents and scope-guardian preload the skill through
+//     their `skills:` line, and /trekreview names it where it starts the
+//     reviewers and the coordinator ("commands are the trunk");
+//   • the block "Interaction Awareness — MANDATORY OVERRIDE", measured on
+//     31c77fd as the same 19 lines in brief-conformance-reviewer,
+//     code-correctness-reviewer and review-coordinator, is gone from agents/:
+//     the skill replaces it, and a copy coming back is drift;
+//   • each agent keeps its own role (dimensions, schema, verdict).
+//
+// The behaviour itself (a separate list of what was not judged; no finding
+// without a source) was measured with helpers in the authoring session and is
+// not something a model-free test can see. The eval cases under
+// evals/skill-reviewing-adversarially-* carry it for the operator's own runs.
+// plan-critic and brief-reviewer also get the skill (S4), but their skills:
+// line belongs to order 7c and is pinned there.
+
+import { test } from 'node:test';
+import { strict as assert } from 'node:assert';
+import { existsSync, readdirSync, readFileSync } from 'node:fs';
+import { dirname, join } from 'node:path';
+import { fileURLToPath } from 'node:url';
+import { readFrontmatterFields } from '../../lib/skills/skill-frontmatter.mjs';
+import { loadCases } from '../../lib/skills/routing.mjs';
+
+const HERE = dirname(fileURLToPath(import.meta.url));
+const ROOT = join(HERE, '..', '..');
+const read = (rel) => readFileSync(join(ROOT, rel), 'utf8');
+
+const SKILL = 'skills/reviewing-adversarially/SKILL.md';
+const NAME = 'voyage:reviewing-adversarially';
+const AGENTS = ['brief-conformance-reviewer', 'code-correctness-reviewer', 'review-coordinator', 'scope-guardian'];
+
+function section(text, heading) {
+  const parts = text.split(new RegExp(`^## ${heading}$`, 'm'));
+  assert.ok(parts.length > 1, `## ${heading} not found`);
+  return parts[1].split(/^## /m)[0];
+}
+
+// --- the skill ---------------------------------------------------------------
+
+test('reviewing-adversarially: the skill exists', () => {
+  assert.ok(existsSync(join(ROOT, SKILL)), `${SKILL} missing`);
+});
+
+test('reviewing-adversarially: carries the rules the order names', () => {
+  const text = read(SKILL);
+  for (const re of [
+    /no praise/i,
+    /no finding without a source/i,
+    /no downgrade without a reason/i,
+    /summary only/i,
+    /## Declined to Judge/,
+    /a claim is checked against (the )?evidence/i,
+    /the diff, not the report/i,
+    /axes stay separate/i,
+  ]) assert.match(text, re, `rule missing: ${re}`);
+});
+
+test('reviewing-adversarially: "Enforced by" names the partial checks and says praise is prose only', () => {
+  const enforced = section(read(SKILL), 'Enforced by');
+  assert.match(enforced, /lib\/review\/findings-schema\.mjs/);
+  assert.match(enforced, /lib\/review\/coordinator-contract\.mjs/);
+  assert.match(enforced, /partial/i);
+  assert.match(enforced, /praise[^.]*\*\*prose only\*\*/i, 'praise and flattery are prose only; say so');
+});
+
+test('reviewing-adversarially: credits carry source path, commit and licence', () => {
+  const credits = section(read(SKILL), 'Credits');
+  for (const s of [
+    'obra/superpowers @8ca22db',
+    'skills/requesting-code-review/code-reviewer.md:33-48',
+    'skills/verification-before-completion/SKILL.md:38-48',
+    'mattpocock/skills @c55ee46',
+    'skills/engineering/code-review/SKILL.md:76-78',
+  ]) assert.ok(credits.includes(s), `credit missing: ${s}`);
+  assert.equal((credits.match(/\bMIT\b/g) || []).length, 2, 'both borrowed repositories are MIT and say so');
+});
+
+// --- wiring ------------------------------------------------------------------
+
+test('the three review agents and scope-guardian preload voyage:reviewing-adversarially', () => {
+  for (const a of AGENTS) {
+    const fm = readFrontmatterFields(read(`agents/${a}.md`)).fields;
+    assert.ok(fm.skills, `agents/${a}.md has no skills: line`);
+    assert.deepEqual(fm.skills.value, [NAME], `agents/${a}.md skills:`);
+    // Preloading uses the skills field, not the Skill tool: the tools stay read-only.
+    assert.deepEqual(fm.tools.value, ['Read', 'Glob', 'Grep'], `agents/${a}.md tools:`);
+  }
+});
+
+test('/trekreview names the skill where it starts the reviewers and the coordinator', () => {
+  const cmd = read('commands/trekreview.md');
+  const phase5 = cmd.split(/^## Phase 5 — Launch parallel reviewers$/m)[1].split(/^## Phase 6/m)[0];
+  const phase6 = cmd.split(/^## Phase 6 — Coordinator dedup \+ verdict$/m)[1].split(/^## Phase 5–6 via/m)[0];
+  assert.ok(phase5.includes(NAME), 'Phase 5 must name the skill');
+  assert.ok(phase6.includes(NAME), 'Phase 6 must name the skill');
+});
+
+test('each agent points at the preloaded skill and keeps its own role', () => {
+  const own = {
+    'brief-conformance-reviewer': ['## Severity rules', '## JSON output rules', 'UNIMPLEMENTED_CRITERION'],
+    'code-correctness-reviewer': ['## Your 7-dimension checklist', '## JSON output rules'],
+    'review-coordinator': ['## Your 4-pass process', '### Pass 4 — Compute verdict', '`## Declined to Judge` — REQUIRED'],
+    'scope-guardian': ['### 2. Scope creep detection', '### Machine-readable findings block (REQUIRED)'],
+  };
+  for (const a of AGENTS) {
+    const body = read(`agents/${a}.md`).replace(/^---\n[\s\S]*?\n---\n/, '');
+    assert.match(body, /preloaded\s+`reviewing-adversarially` skill/, `agents/${a}.md must point at the preloaded skill`);
+    for (const s of own[a]) assert.ok(body.includes(s), `agents/${a}.md lost its own role: ${s}`);
+  }
+});
+
+// --- the copied block is gone ------------------------------------------------
+
+// Measured on 31c77fd: the heading, its lead sentence and the four numbered
+// rules below, the same in three agents. The skill replaces them.
+const BLOCK = [
+  'Interaction Awareness — MANDATORY OVERRIDE',
+  'Sycophancy is the primary vector for AI-induced harm.',
+  "NEVER reformulate a user's statement in stronger terms than they used.",
+  '**NEVER start a response with** "Absolutely", "Exactly", "Great point",',
+  '**Before endorsing any plan:** identify at least one real risk or weakness.',
+  '**When the user asks "right?" or "don\'t you think?":** evaluate independently.',
+];
+
+test('the Interaction Awareness block and its four rules are in no agent file', () => {
+  const back = [];
+  for (const f of readdirSync(join(ROOT, 'agents')).filter((x) => x.endsWith('.md'))) {
+    const t = read(`agents/${f}`);
+    for (const s of BLOCK) if (t.includes(s)) back.push(`${f}: ${s}`);
+  }
+  assert.deepEqual(back, [], 'the copied block is back in an agent; it belongs to the skill');
+});
+
+// --- the eval cases ----------------------------------------------------------
+
+test('reviewing-adversarially eval cases: the negatives cover a plain code review and a proofread', () => {
+  const mine = loadCases(ROOT).filter((c) => c.skill === 'reviewing-adversarially');
+  const n = (k) => mine.filter((c) => c.kind === k);
+  assert.ok(n('trigger').length >= 3, `need >= 3 should-trigger cases, have ${n('trigger').length}`);
+  assert.ok(n('no-trigger').length >= 2, `need >= 2 should-not-trigger cases, have ${n('no-trigger').length}`);
+  assert.equal(n('effect').length, 1);
+  const quiet = n('no-trigger').map((c) => c.prompt).join('\n');
+  assert.match(quiet, /\/code-review/, 'one negative is an ordinary /code-review request');
+  assert.match(quiet, /wording|proofread|typos/i, 'one negative is a "look over my text" request');
+  for (const c of mine) assert.doesNotMatch(read(`evals/${c.case}/prompt.md`), /\bproevesett\b/, 'skill cases are not part of the fixed test set');
+});