Fix the test, tooling, and skill defects that the #98 benchmark found in #96

plugins/docks/skills/AGENTS.md

@@ -104,7 +104,7 @@ rule and the command that finds every copy.
 
 ## Report section (every shipped skill)
 
-Every shipped skill of the docks and plan-lifecycle plugins ends with one byte-identical `## Report a problem with this skill` section, so an agent that uses a skill sees where and how to report a problem with it. The section points to the docks `report-skill-issue` skill. For a runtime without docks, it allows only a draft for the user, because the publication safeguards live in that skill. A new skill copies the section unchanged; a change to the section changes every copy in the same commit. Verify: `for f in plugins/docks/skills/*/*/SKILL.md plugins/plan-lifecycle/skills/*/*/SKILL.md; do sed -n '/^## Report a problem with this skill$/,$p' "$f" | sha256sum; done | sort | uniq -c` must print one line whose count equals the number of shipped skills.
+Every shipped skill of the docks and plan-lifecycle plugins ends with one byte-identical `## Report a problem with this skill` section, so an agent that uses a skill sees where and how to report a problem with it. The section points to the docks `report-skill-issue` skill. For a runtime without docks, it allows only a draft for the user, because the publication safeguards live in that skill. A new skill copies the section unchanged. The skills guard checks every copy against `scripts/config/report-section.md` (verify: `bun scripts/skills/guard.mjs plugins/docks/skills` and `bun scripts/skills/guard.mjs plugins/plan-lifecycle/skills`); a change to the section changes that file and every copy in the same commit.
 
 ## Cross-tool wording (Claude Code + Codex)
 

scripts/AGENTS.md

@@ -48,7 +48,7 @@ Plugin behavior stays registry-driven: extend descriptor capabilities rather tha
 | Script | Purpose | Floor |
 |---|---|---|
 | `ci.mjs` | the authoritative gate entry point — without `--plugin`, runs repo-wide checks once and gates every registry entry; `--plugin <name>` skips repo-wide sections and runs only that plugin's owned author checks, shell-hook lint, and `gatePlugin` validation (manifest/version coherence, `claude plugin validate`, Codex parity, and the checks below) | — |
-| `skills/guard.mjs` | runs the skill frontmatter validators (codex + claude via `lib/validate-skills.mjs`) + `codex-facts.mjs` + `refs-guard.mjs` | pass/fail |
+| `skills/guard.mjs` | runs the skill frontmatter validators (codex + claude via `lib/validate-skills.mjs`) + `codex-facts.mjs` + `refs-guard.mjs`, then checks that every `SKILL.md` ends with the Report section of `config/report-section.md`, byte for byte | pass/fail |
 | `lib/validate-skills.mjs` | skill frontmatter per runtime — name/description, 1024-char cap, no `#` truncation, CSO `Use when` prefix, `user-invocable`, `metadata.updated`, `references/` one level deep | pass/fail |
 | `skills/codex-facts.mjs` | pins canonical Codex model ids / `sandbox_mode` / `model_reasoning_effort` + the `agents.max_depth` fact in the skill-agent-pipeline refs (self-skips when absent) | pass/fail |
 | `skills/refs-guard.mjs` | reference hygiene: broken local `references/`/`assets/` links, orphan reference files, missing `## Contents` TOC on `references/*.md` > 100 lines with ≥3 doc-level headings | pass/fail |
@@ -65,8 +65,8 @@ Plugin behavior stays registry-driven: extend descriptor capabilities rather tha
 | `tests/idempotency.mjs` | content-hash determinism + every stored hash in sync | pass/fail |
 | `tests/plan-cli.mjs` | runs the plan-CLI case files in `tests/plan-cli/` in parallel through Node's test runner against the shipped plan helper (`plan.mjs`) | pass/fail |
 | `tests/ci-observability.mjs` | validates command timing records, wall-time reconstruction, and CI host metadata | pass/fail |
-| `tests/test-contracts.mjs` | validates the closed test-contract registry and its discovered, registered, selected, and executed sets | pass/fail |
-| `tests/author-tooling.mjs` | author-tool contracts: Biome rejects a syntax defect, combined skill validation, tree/guard operational failures, skills-guard spawn failure; this table lists every `scripts/**/*.mjs` outside `lib/`, `tests/unit/`, `tests/plan-cli/`, and the non-validator entry points `release.mjs`/`ci-target.mjs`/`capture-tdd-red.mjs`, and every first-column `.mjs` path resolves from `scripts/` or the repo root | pass/fail |
+| `tests/test-contracts.mjs` | validates the closed test-contract registry and its discovered, registered, selected, and executed sets; `--registry <path>` checks another registry file (author-tooling uses it for the rejection cases) | pass/fail |
+| `tests/author-tooling.mjs` | author-tool contracts: Biome rejects a syntax defect, combined skill validation, tree/guard operational failures, skills-guard spawn failure, registry toolchain rejections, and skills-guard Report section rejections; this table lists every `scripts/**/*.mjs` outside `lib/`, `tests/unit/`, `tests/plan-cli/`, and the non-validator entry points `release.mjs`/`ci-target.mjs`/`capture-tdd-red.mjs`, and every first-column `.mjs` path resolves from `scripts/` or the repo root | pass/fail |
 | `tests/ci-plugin-targeting.mjs` | CI targeting and release contracts: shard selection, `ci.yml` trigger block, release module and dry-run safety (`--unit` in the gate) | pass/fail |
 | shellcheck (target-selected) | `-S warning` over selected plugins' `hooks/*.sh`, via `shellHooks(p)`; a full invocation selects every plugin | pass/warn |
 
@@ -116,6 +116,11 @@ repo-wide guards section.
 When one suite has multiple gate-selected cases, its selector joins the sorted case values with `|`, and
 `expected_min` records the number of distinct invocations.
 
+Each row's `toolchains` must equal the toolchains of its discovered runners: the
+runners that `scripts/ci.mjs` uses for the suite, plus the `#!` interpreter of each
+executable fixture under `scripts/tests/fixtures/` that the suite's files name. A
+missing or an extra toolchain fails with the row id and the evidence.
+
 ### Host-derived resource envelope
 
 `scripts/lib/host-resources.mjs` sizes the gate to the host that actually runs it.

scripts/config/report-section.md

@@ -0,0 +1,3 @@
+## Report a problem with this skill
+
+When this skill gives a wrong, outdated, or unsafe instruction, or a script that it ships fails, report the problem in the public DocksDocks/docks repository. Use the docks `report-skill-issue` skill for the report. If that skill is not installed, do not file anything: give the user a draft instead. The draft names this skill and its plugin version, quotes the instruction, and states what happened and what you expected. Leave out secrets and private names.

scripts/skills/guard.mjs

@@ -1,12 +1,17 @@
 #!/usr/bin/env bun
 // guard.mjs — run the combined skill frontmatter validator, the Codex-fact
-// drift guard, and the reference-hygiene guard. Usage: guard.mjs [skills-dir]
+// drift guard, and the reference-hygiene guard, then check that every skill
+// ends with the canonical Report section. Usage: guard.mjs [skills-dir]
 import { spawnSync } from 'node:child_process';
+import fs from 'node:fs';
 import path from 'node:path';
+import { findSkillFiles } from '../lib/skills-walk.mjs';
 
 const SCRIPT_DIR = path.dirname(new URL(import.meta.url).pathname);
 const REPO_DIR = path.resolve(SCRIPT_DIR, '../..');
 const DIR = process.argv[2] || '';
+// One canonical copy, so that the copies in all plugins cannot drift apart unseen.
+const REPORT_SECTION = 'scripts/config/report-section.md';
 
 function run(args) {
   const result = spawnSync('bun', args, { stdio: 'inherit' });
@@ -26,10 +31,51 @@ function run(args) {
   if (result.status !== 0) process.exit(result.status);
 }
 
+// Every shipped skill ends with the same Report section, so that an agent that uses any skill
+// can find how to report a problem with it. Read the files as latin1 (one character per byte),
+// so that the comparison is byte for byte, also for line endings and trailing newlines.
+function reportSectionFailures(skillsRoot) {
+  const canonicalFile = path.join(REPO_DIR, REPORT_SECTION);
+  let canonical;
+  try {
+    canonical = fs.readFileSync(canonicalFile, 'latin1');
+  } catch (error) {
+    console.error(`FAIL: cannot read the canonical Report section ${canonicalFile}: ${error.message}`);
+    process.exit(2);
+  }
+  const heading = canonical.split('\n', 1)[0];
+  if (!heading.startsWith('## ')) {
+    console.error(`FAIL: ${REPORT_SECTION}: the first line must be the \`## \` heading of the Report section`);
+    process.exit(2);
+  }
+  const escapedHeading = heading.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
+  const headingLine = new RegExp(`^${escapedHeading}$`, 'gm');
+  const failures = [];
+  for (const file of findSkillFiles(skillsRoot)) {
+    const content = fs.readFileSync(file, 'latin1');
+    // The last heading starts the section: a skill can quote the heading in its body.
+    const start = [...content.matchAll(headingLine)].at(-1)?.index;
+    const shown = path.relative(process.cwd(), file) || file;
+    if (start === undefined) {
+      failures.push(`${shown}: missing the Report section; end the file with the text of ${REPORT_SECTION}`);
+    } else if (content.slice(start) !== canonical) {
+      failures.push(`${shown}: the Report section differs from ${REPORT_SECTION}; copy that file's text over it`);
+    }
+  }
+  return failures;
+}
+
 const lib = path.join(REPO_DIR, 'scripts/lib/validate-skills.mjs');
 const target = DIR || path.join(REPO_DIR, 'plugins/docks/skills');
 run([lib, '--runtime', 'all', target]);
 run([path.join(REPO_DIR, 'scripts/skills/codex-facts.mjs')]);
 run([path.join(REPO_DIR, 'scripts/skills/refs-guard.mjs'), ...(DIR ? [DIR] : [])]);
 
-console.log('Guard PASSED: skills match Codex and Claude conventions');
+const reportFailures = reportSectionFailures(target);
+if (reportFailures.length > 0) {
+  for (const failure of reportFailures) console.error(`FAIL: ${failure}`);
+  console.error(`Guard FAILED: ${reportFailures.length} skill file(s) do not end with the canonical Report section`);
+  process.exit(1);
+}
+
+console.log('Guard PASSED: skills match Codex and Claude conventions and end with the canonical Report section');

scripts/tests/author-tooling.mjs

@@ -5,12 +5,15 @@ import fs from 'node:fs';
 import os from 'node:os';
 import path from 'node:path';
 import { fileURLToPath } from 'node:url';
+import { shebangInterpreter } from './test-contracts.mjs';
 
 const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..');
 const BIOME = path.join(ROOT, 'node_modules', '.bin', process.platform === 'win32' ? 'biome.cmd' : 'biome');
 const VALIDATE_SKILLS = path.join(ROOT, 'scripts/lib/validate-skills.mjs');
 const TREE_GUARD = path.join(ROOT, 'scripts/tree/guard.mjs');
 const SKILLS_GUARD = path.join(ROOT, 'scripts/skills/guard.mjs');
+const TEST_CONTRACTS = path.join(ROOT, 'scripts/tests/test-contracts.mjs');
+const REPORT_SECTION = path.join(ROOT, 'scripts/config/report-section.md');
 
 function runNode(script, args, options = {}) {
   return spawnSync(process.execPath, [script, ...args], {
@@ -26,6 +29,10 @@ function assertStarted(result) {
   assert.equal(result.signal, null);
 }
 
+function escapeRegExp(text) {
+  return text.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
+}
+
 function testBiomeRejectsSyntaxDefect() {
   const fixtureRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'docks-author-tooling-biome-'));
   const fixture = path.join(fixtureRoot, 'broken-syntax.mjs');
@@ -250,9 +257,158 @@ function testSkillsGuardSpawnFailure() {
   );
 }
 
+function runTestContracts(fixtureRoot, name, registry) {
+  const file = path.join(fixtureRoot, name);
+  fs.writeFileSync(file, `${JSON.stringify(registry, null, 2)}\n`);
+  return runNode(TEST_CONTRACTS, ['--registry', file]);
+}
+
+// The registry check must reject a row that does not list a toolchain that its suite needs, and a row that lists a
+// toolchain that nothing needs. Each negative case changes one row in a copy of the registry.
+function testTestContractToolchains() {
+  const registry = JSON.parse(fs.readFileSync(path.join(ROOT, 'scripts/config/test-contracts.json'), 'utf8'));
+  const passed = runNode(TEST_CONTRACTS, []);
+  assertStarted(passed);
+  assert.equal(passed.status, 0, passed.stderr);
+  assert.match(passed.stdout, /^PASS test_contracts /);
+
+  // The plan CLI suite runs with Node, and its cases run the fake gh, which runs with Bun.
+  const planCli = registry.contracts.find(({ id }) => id === 'repo-plan-cli');
+  assert.ok(planCli, 'the registry must hold the repo-plan-cli row');
+  const bunToken = planCli.toolchains.find((token) => token.startsWith('bun-'));
+  const nodeToken = planCli.toolchains.find((token) => token.startsWith('node-'));
+  assert.ok(bunToken && nodeToken, `repo-plan-cli must list a Bun and a Node toolchain: ${planCli.toolchains}`);
+  const bunOnly = registry.contracts.find(({ toolchains }) => toolchains.length === 1 && toolchains[0] === bunToken);
+  assert.ok(bunOnly, `the registry must hold a row that lists only ${bunToken}`);
+
+  const fixtureRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'docks-author-tooling-contracts-'));
+  try {
+    const lacksBun = structuredClone(registry);
+    const lacksBunRow = lacksBun.contracts.find(({ id }) => id === planCli.id);
+    lacksBunRow.toolchains = lacksBunRow.toolchains.filter((token) => token !== bunToken);
+    const missing = runTestContracts(fixtureRoot, 'lacks-bun.json', lacksBun);
+    assertStarted(missing);
+    assert.notEqual(missing.status, 0);
+    assert.match(
+      `${missing.stdout}\n${missing.stderr}`,
+      new RegExp(
+        `repo-plan-cli\\.toolchains must list ${escapeRegExp(bunToken)}: .*scripts/tests/fixtures/gh-stub/gh runs with bun`,
+      ),
+    );
+
+    const listsNode = structuredClone(registry);
+    const listsNodeRow = listsNode.contracts.find(({ id }) => id === bunOnly.id);
+    listsNodeRow.toolchains = [bunToken, nodeToken].sort();
+    const extra = runTestContracts(fixtureRoot, 'lists-node.json', listsNode);
+    assertStarted(extra);
+    assert.notEqual(extra.status, 0);
+    // The line names the runners that discovery found, so that the reader sees why the row does not need the token.
+    const runnerEvidence = `\\(discovered: [^\\n]*\\b${escapeRegExp(bunOnly.owner.suite)} with bun\\b[^\\n]*\\)`;
+    assert.match(
+      `${extra.stdout}\n${extra.stderr}`,
+      new RegExp(
+        `${escapeRegExp(bunOnly.id)}\\.toolchains lists ${escapeRegExp(nodeToken)}, which no discovered runner needs ${runnerEvidence}`,
+      ),
+    );
+  } finally {
+    fs.rmSync(fixtureRoot, { recursive: true, force: true });
+  }
+}
+
+// A fixture program runs with the interpreter of its #! line, and that interpreter decides which toolchain a suite
+// needs. `env` reads its options, their operands, and NAME=VALUE words before the interpreter, so none of them is it.
+function testShebangInterpreter() {
+  const cases = [
+    ['#!/usr/bin/env bun\n', 'bun'],
+    ['#!/usr/local/bin/bun\n', 'bun'],
+    ['#!/usr/bin/env -S -u NODE_OPTIONS bun\n', 'bun'],
+    ['#!/usr/bin/env -S -uNODE_OPTIONS bun --smol\n', 'bun'],
+    ['#!/usr/bin/env --split-string=-u NODE_OPTIONS bun\n', 'bun'],
+    ['#!/usr/bin/env --unset=NODE_OPTIONS node\n', 'node'],
+    ['#!/usr/bin/env --unset NODE_OPTIONS node\n', 'node'],
+    ['#!/usr/bin/env -i FORCE_COLOR=0 node\n', 'node'],
+    ['#!/usr/bin/env - --ignore-environment node\n', 'node'],
+    ['#!/usr/bin/env -iu NODE_OPTIONS /opt/bun/bin/bun\n', 'bun'],
+    ['#!/usr/bin/env -S -- FORCE_COLOR=0 bun\n', 'bun'],
+    ['#!/usr/bin/env -- bun\n', 'bun'],
+    ['#!/usr/bin/env -S -u NODE_OPTIONS\n', ''],
+    ['#!\n', ''],
+    ['console.log(1);\n', null],
+  ];
+  for (const [source, interpreter] of cases) {
+    assert.equal(shebangInterpreter(source), interpreter, `interpreter of ${JSON.stringify(source)}`);
+  }
+}
+
+// Every docks skill must end with the canonical Report section. The cases change a full copy of the docks skills: the
+// copy passes the sub-guards that run before the Report check, so that each run gets to that check.
+function testSkillsGuardReportSection() {
+  const canonical = fs.readFileSync(REPORT_SECTION, 'utf8');
+  const fixtureRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'docks-author-tooling-report-'));
+  try {
+    const skillsRoot = path.join(fixtureRoot, 'skills');
+    fs.cpSync(path.join(ROOT, 'plugins/docks/skills'), skillsRoot, { recursive: true });
+    const skillFiles = fs
+      .readdirSync(skillsRoot, { recursive: true })
+      .filter((relative) => path.basename(relative) === 'SKILL.md')
+      .sort();
+    assert.ok(skillFiles.length >= 2, `the fixture needs two skills, found ${skillFiles.length}`);
+    const originals = new Map(
+      skillFiles.map((relative) => [relative, fs.readFileSync(path.join(skillsRoot, relative), 'utf8')]),
+    );
+    for (const [relative, content] of originals)
+      assert.ok(content.endsWith(canonical), `${relative} must end with the canonical Report section`);
+    const write = (relative, content) => fs.writeFileSync(path.join(skillsRoot, relative), content);
+    const withoutSection = (relative) => originals.get(relative).slice(0, -canonical.length);
+    const failLine = (relative, reason) => new RegExp(`^FAIL: .*/${escapeRegExp(relative)}: ${reason}`, 'm');
+    const failedCount = (count) =>
+      new RegExp(`Guard FAILED: ${count} skill file\\(s\\) do not end with the canonical Report section\\n$`);
+
+    const unchanged = runNode(SKILLS_GUARD, [skillsRoot]);
+    assertStarted(unchanged);
+    assert.equal(unchanged.status, 0, unchanged.stderr);
+    assert.match(
+      unchanged.stdout,
+      /Guard PASSED: skills match Codex and Claude conventions and end with the canonical Report section\n$/,
+    );
+
+    const [missingSkill, changedSkill] = skillFiles;
+    write(missingSkill, withoutSection(missingSkill));
+    const missing = runNode(SKILLS_GUARD, [skillsRoot]);
+    assertStarted(missing);
+    assert.equal(missing.status, 1);
+    assert.match(missing.stderr, failLine(missingSkill, 'missing the Report section'));
+    assert.match(missing.stderr, failedCount(1));
+    write(missingSkill, originals.get(missingSkill));
+
+    // The byte goes before the last byte of the file, which is in the section.
+    const changedOriginal = originals.get(changedSkill);
+    write(changedSkill, `${changedOriginal.slice(0, -1)}x${changedOriginal.slice(-1)}`);
+    const changed = runNode(SKILLS_GUARD, [skillsRoot]);
+    assertStarted(changed);
+    assert.equal(changed.status, 1);
+    assert.match(changed.stderr, failLine(changedSkill, 'the Report section differs from'));
+    assert.match(changed.stderr, failedCount(1));
+
+    // No skill has the section: a check that only looks for a file that differs finds nothing to report here.
+    for (const relative of skillFiles) write(relative, withoutSection(relative));
+    const allMissing = runNode(SKILLS_GUARD, [skillsRoot]);
+    assertStarted(allMissing);
+    assert.equal(allMissing.status, 1);
+    for (const relative of skillFiles)
+      assert.match(allMissing.stderr, failLine(relative, 'missing the Report section'));
+    assert.match(allMissing.stderr, failedCount(skillFiles.length));
+  } finally {
+    fs.rmSync(fixtureRoot, { recursive: true, force: true });
+  }
+}
+
 testBiomeRejectsSyntaxDefect();
 testCombinedSkillValidation();
 testTreeGuardOperationalFailures();
 testSkillsGuardSpawnFailure();
+testSkillsGuardReportSection();
+testTestContractToolchains();
+testShebangInterpreter();
 testValidatorTable();
 console.log('author tooling contracts passed');