From c5845ea26f8096c11bfc18f47fec336c7c07234c Mon Sep 17 00:00:00 2001 From: Eduardo Marquez <55303379+DocksDocks@users.noreply.github.com> Date: Mon, 24 Aug 2026 22:02:24 -0300 Subject: [PATCH] feat: harden author-side payload gating Implements the reviewed plan #33: - no-author-scripts matcher derives author tails from the on-disk scripts/ inventory and flags them under any prefix, with no exemption logic; bundled plugin-internal scripts never match because their filenames are not author tails - declared skills/agents roots that are missing now fail the gate instead of silently skipping - agent-scorer output is corroborated: exit status, row names vs on-disk agents, duplicates, non-finite scores, empty-root vacuity; seven-case unit suite registered as unit-agent-score-vacuity - ci-plugin-targeting stub scorer aligned with the real scorer's stripped-name output (fixture drift exposed by the new corroboration) Closes #33 --- scripts/ci.mjs | 108 ++++++++--- scripts/config/test-contracts.json | 19 ++ scripts/skills/no-author-scripts.mjs | 27 ++- scripts/tests/ci-plugin-targeting.mjs | 2 +- .../tests/unit/agent-score-vacuity.test.mjs | 177 ++++++++++++++++++ 5 files changed, 304 insertions(+), 29 deletions(-) create mode 100644 scripts/tests/unit/agent-score-vacuity.test.mjs diff --git a/scripts/ci.mjs b/scripts/ci.mjs index 3e265f55..dab9257e 100644 --- a/scripts/ci.mjs +++ b/scripts/ci.mjs @@ -427,33 +427,91 @@ function gatePlugin(p) { for (const f of p.extraJson) readJSON(f) ? ok(`${p.name} ${path.basename(f)} JSON valid`) : fail(`${p.name} ${f} JSON invalid`); - if (p.skills && fs.existsSync(p.skills)) gateSkills(p, manifest); - - if (p.agents && fs.existsSync(p.agents)) { - nodeOk(['scripts/agents/guard.mjs', p.agents]) - ? ok(`${p.name} agents/guard passed`) - : fail(`${p.name} agents/guard failed (run: node scripts/agents/guard.mjs ${p.agents})`); - const floor = floorOf('agents'); - const count = fs - .readdirSync(p.agents) - .filter((f) => f.endsWith('.md') && f !== 'AGENTS.md' && f !== 'CLAUDE.md').length; - const agentScoreRows = node(['scripts/agents/score.mjs', '--per-file', p.agents]) - .stdout.trim() - .split('\n') - .filter(Boolean) - .map((line) => ({ line, score: parseInt(line.split(' ').pop(), 10) })); - const total = agentScoreRows.reduce((sum, row) => sum + row.score, 0); - total >= count * floor - ? ok(`${p.name} agents score: ${total} (floor ${count * floor} = ${count} × ${floor})`) - : fail(`${p.name} agents score: ${total} below floor ${count * floor} (${count} × ${floor})`); - let aunder = 0; - for (const row of agentScoreRows) { - if (row.score < floor) { - fail(` ${p.name} agents:${row.line} below per-file floor ${floor}`); - aunder = 1; + if (p.skills) { + if (fs.existsSync(p.skills)) gateSkills(p, manifest); + else fail(`${p.name} declared skills root missing: ${p.skills}`); + } + + if (p.agents) { + if (!fs.existsSync(p.agents)) { + fail(`${p.name} declared agents root missing: ${p.agents}`); + } else { + nodeOk(['scripts/agents/guard.mjs', p.agents]) + ? ok(`${p.name} agents/guard passed`) + : fail(`${p.name} agents/guard failed (run: node scripts/agents/guard.mjs ${p.agents})`); + const floor = floorOf('agents'); + const agentFiles = fs + .readdirSync(p.agents) + .filter((name) => name.endsWith('.md') && name !== 'AGENTS.md' && name !== 'CLAUDE.md') + .sort(); + const onDisk = agentFiles.map((name) => name.replace(/\.md$/u, '')); + const scoreArgs = ['scripts/agents/score.mjs', '--per-file', p.agents]; + const scoreCommand = `node ${scoreArgs.join(' ')}`; + const scoreRun = node(scoreArgs); + if ((scoreRun.status ?? 1) !== 0) { + const detail = `${scoreRun.stdout ?? ''}${scoreRun.stderr ?? ''}`.trim(); + if (detail) console.error(detail); + fail(`${p.name} agent scorer exited ${scoreRun.status ?? 'null'} — no score set to gate (${scoreCommand})`); + } else { + const agentScoreRows = (scoreRun.stdout ?? '') + .trim() + .split('\n') + .filter(Boolean) + .map((line) => { + const match = /^(.+)\s+(\S+)$/u.exec(line); + return { line, name: match?.[1] ?? '', score: Number(match?.[2]) }; + }); + const nonFinite = agentScoreRows.filter((row) => !Number.isFinite(row.score)); + const scoredCounts = new Map(); + for (const row of agentScoreRows) scoredCounts.set(row.name, (scoredCounts.get(row.name) ?? 0) + 1); + const scored = new Set(scoredCounts.keys()); + const disk = new Set(onDisk); + const unscored = onDisk.filter((name) => !scored.has(name)); + const notOnDisk = [...scored].filter((name) => !disk.has(name)); + const duplicates = [...scoredCounts].filter(([, count]) => count > 1).map(([name]) => name); + const scoreSetMismatch = + agentScoreRows.length !== onDisk.length || + unscored.length > 0 || + notOnDisk.length > 0 || + duplicates.length > 0; + + if (onDisk.length === 0) { + fail(`${p.name} declared agents root is empty — no agent score set to gate: ${p.agents}`); + } else if (nonFinite.length > 0) { + fail( + `${p.name} agent scorer produced non-finite score(s): ${nonFinite.map((row) => row.line).join(', ')} (${scoreCommand})`, + ); + } else if (scoreSetMismatch) { + const mismatchDetails = [ + unscored.length > 0 ? `unscored on disk: ${unscored.join(', ')}` : null, + notOnDisk.length > 0 ? `not on disk: ${notOnDisk.join(', ')}` : null, + duplicates.length > 0 ? `duplicate score rows: ${duplicates.join(', ')}` : null, + ].filter(Boolean); + fail( + `${p.name} agent score set does not cover the tree: expected ${onDisk.length} agent(s) on disk, parsed ${agentScoreRows.length} score row(s) from ${scoreCommand}` + + (mismatchDetails.length > 0 ? ` — ${mismatchDetails.join(' — ')}` : ''), + ); + } else { + ok( + `${p.name} agent score set covers the tree: ${agentScoreRows.length} score rows for ${onDisk.length} agent files on disk`, + ); + const total = agentScoreRows.reduce((sum, row) => sum + row.score, 0); + total >= onDisk.length * floor + ? ok(`${p.name} agents score: ${total} (floor ${onDisk.length * floor} = ${onDisk.length} × ${floor})`) + : fail( + `${p.name} agents score: ${total} below floor ${onDisk.length * floor} (${onDisk.length} × ${floor})`, + ); + let aunder = 0; + for (const row of agentScoreRows) { + if (row.score < floor) { + fail(` ${p.name} agents:${row.line} below per-file floor ${floor}`); + aunder = 1; + } + } + if (!aunder) ok(`${p.name} agents per-file all ≥ ${floor}`); + } } } - if (!aunder) ok(`${p.name} agents per-file all ≥ ${floor}`); } if (p.distributionContract) { diff --git a/scripts/config/test-contracts.json b/scripts/config/test-contracts.json index 09957294..19054746 100644 --- a/scripts/config/test-contracts.json +++ b/scripts/config/test-contracts.json @@ -172,6 +172,25 @@ "replaces": [], "skips": [] }, + { + "id": "unit-agent-score-vacuity", + "version": 1, + "title": "Agent score gate vacuity contracts", + "owner": { + "suite": "scripts/tests/unit/agent-score-vacuity.test.mjs", + "layer": "unit" + }, + "selection": { + "kind": "node-test-file", + "selector": "scripts/tests/unit/agent-score-vacuity.test.mjs", + "expected_min": 7 + }, + "platforms": ["linux", "macos"], + "toolchains": ["node-24"], + "release_role": "gate", + "replaces": [], + "skips": [] + }, { "id": "unit-ci-background-task", "version": 1, diff --git a/scripts/skills/no-author-scripts.mjs b/scripts/skills/no-author-scripts.mjs index 1bf076c9..5c6638b7 100644 --- a/scripts/skills/no-author-scripts.mjs +++ b/scripts/skills/no-author-scripts.mjs @@ -20,8 +20,29 @@ const AGENTS_DIR = argSkills : path.join(REPO_DIR, 'plugins/docks/agents'); const ALLOWLIST = ['scaffold', 'write-skill']; -const PATTERN = - /(? value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +const scriptEntries = fs.readdirSync(path.join(REPO_DIR, 'scripts'), { withFileTypes: true }); +const topLevelScriptTails = scriptEntries + .filter((entry) => entry.isFile() && entry.name.endsWith('.mjs')) + .map((entry) => escapeRegex(entry.name)) + .sort(); +const firstLevelScriptDirs = scriptEntries + .filter((entry) => entry.isDirectory()) + .map((entry) => escapeRegex(entry.name)) + .sort(); +const authorScriptTails = [...topLevelScriptTails, `(?:${firstLevelScriptDirs.join('|')})/` + '[^\\s`]+\\.mjs']; +// The tails come from the real author-side scripts/ inventory, so bundled +// plugin-internal paths (write-skill/scripts/skill-guard.mjs, plan-lifecycle's +// scripts/plan.mjs) never match: their filenames are not author tails. Any +// prefix - bare, ./, ../, variables, substitutions, alternate clone roots - +// of a real author tail is therefore a violation, with no exemption logic. +const PATTERN = new RegExp( + `scripts/(?:${authorScriptTails.join('|')})\\b|tree/guard\\.sh|content-hash\\.sh|transform-guard\\.sh|no-author-scripts\\.sh|codex-facts\\.sh|guard-spec\\.sh`, +); + +function namesAuthorScript(line) { + return PATTERN.test(line); +} function walk(dir, filter, out = []) { let entries; @@ -50,7 +71,7 @@ for (const f of files) { if (ALLOWLIST.includes(skill)) continue; const lines = fs.readFileSync(f, 'utf8').split('\n'); lines.forEach((line, i) => { - if (PATTERN.test(line)) report.push(`${path.relative(REPO_DIR, f)}:${i + 1}:${line}`); + if (namesAuthorScript(line)) report.push(`${path.relative(REPO_DIR, f)}:${i + 1}:${line}`); }); } diff --git a/scripts/tests/ci-plugin-targeting.mjs b/scripts/tests/ci-plugin-targeting.mjs index fed76c47..c729a4a7 100755 --- a/scripts/tests/ci-plugin-targeting.mjs +++ b/scripts/tests/ci-plugin-targeting.mjs @@ -381,7 +381,7 @@ if (tool === 'node' && args[0] === 'plugins/docks/skills/productivity/write-skil if (rows.length) process.stdout.write(\`\${rows.join('\\n')}\\n\`); } if (tool === 'node' && args[0] === 'scripts/agents/score.mjs' && args[1] === '--per-file') { - process.stdout.write('code-reviewer.md 14\\nplan-reviewer.md 14\\n'); + process.stdout.write('code-reviewer 14\\nplan-reviewer 14\\n'); } if (tool === 'node' && args[0] === 'scripts/config/read-floor.mjs') process.stdout.write('10\\n'); process.exit(0); diff --git a/scripts/tests/unit/agent-score-vacuity.test.mjs b/scripts/tests/unit/agent-score-vacuity.test.mjs new file mode 100644 index 00000000..f0469343 --- /dev/null +++ b/scripts/tests/unit/agent-score-vacuity.test.mjs @@ -0,0 +1,177 @@ +import assert from 'node:assert/strict'; +import { spawnSync } from 'node:child_process'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import test from 'node:test'; + +// The agent-score branch used to trust the scorer's stdout completely: an empty directory, +// a crashed scorer, a same-size set of invented names, or NaN scores could all reach a green +// per-file all-clear. Drive the real `scripts/ci.mjs` with PATH shims, matching the skill-score +// vacuity contracts, so these cases exercise the shipped gate rather than a reimplementation. +const HERE = path.dirname(new URL(import.meta.url).pathname); +const REPO = path.resolve(HERE, '../../..'); +const TARGET = 'plan-lifecycle'; +const SKILLS_ROOT = `plugins/${TARGET}/skills`; +const AGENTS_ROOT = `plugins/${TARGET}/agents`; + +const agentNames = (root) => + fs + .readdirSync(path.join(REPO, root)) + .filter((name) => name.endsWith('.md') && name !== 'AGENTS.md' && name !== 'CLAUDE.md') + .sort() + .map((name) => name.replace(/\.md$/u, '')); +const EXPECTED = agentNames(AGENTS_ROOT); +assert.ok(EXPECTED.length >= 2, `${AGENTS_ROOT} must hold at least two agents for the mismatch case to mean anything`); + +function writeShim(directory, name) { + const script = `#!${process.execPath} +import fs from 'node:fs'; +import path from 'node:path'; +const tool = ${JSON.stringify(name)}; +const args = process.argv.slice(2); +if (tool === 'claude') process.stdout.write('Validation passed\\n'); +if (tool === 'node' && args[0] === 'scripts/config/read-floor.mjs') process.stdout.write('10\\n'); +if (tool === 'node' && args[0] === 'plugins/docks/skills/productivity/write-skill/scripts/skill-guard.mjs' && args[1] === 'score') { + const root = args.at(-1); + const rows = []; + for (const category of fs.readdirSync(root).sort()) { + const categoryPath = path.join(root, category); + if (!fs.statSync(categoryPath).isDirectory()) continue; + for (const skill of fs.readdirSync(categoryPath).sort()) { + const skillPath = path.join(categoryPath, skill); + if (fs.statSync(skillPath).isDirectory() && fs.existsSync(path.join(skillPath, 'SKILL.md'))) { + rows.push(\`\${category}/\${skill} 14\`); + } + } + } + if (rows.length) process.stdout.write(\`\${rows.join('\\n')}\\n\`); +} +if (tool === 'node' && args[0] === 'scripts/agents/score.mjs' && args[1] === '--per-file') { + const root = args.at(-1); + const rows = fs + .readdirSync(root) + .filter((entry) => entry.endsWith('.md') && entry !== 'AGENTS.md' && entry !== 'CLAUDE.md') + .sort() + .map((entry) => \`\${entry.replace(/\\.md$/u, '')} 14\`); + const mode = process.env.DOCKS_AGENT_SCORE_MODE; + if (mode === 'crash') { + process.stderr.write('agent-score: simulated scorer crash\\n'); + process.exit(3); + } + if (mode === 'mismatch' && rows.length) rows[0] = 'not-on-disk 14'; + if (mode === 'non-finite' && rows.length) rows[0] = \`\${rows[0].split(' ')[0]} NaN\`; + if (rows.length) process.stdout.write(\`\${rows.join('\\n')}\\n\`); +} +process.exit(0); +`; + fs.writeFileSync(path.join(directory, name), script, { mode: 0o755 }); +} + +const shimDir = fs.mkdtempSync(path.join(os.tmpdir(), 'docks-agent-score-vacuity-')); +for (const tool of ['node', 'bun', 'claude', 'shellcheck', 'cargo']) writeShim(shimDir, tool); + +// Make the parent gate observe either an empty agents directory or a declared root that does +// not exist. Child checks are shimmed; the same hook is harmless in those short-lived processes. +const fixtureHook = path.join(shimDir, 'payload-root-fixture.mjs'); +fs.writeFileSync( + fixtureHook, + `import fs from 'node:fs'; +const existsSync = fs.existsSync.bind(fs); +const readdirSync = fs.readdirSync.bind(fs); +fs.existsSync = (target) => target === process.env.DOCKS_MISSING_ROOT ? false : existsSync(target); +fs.readdirSync = (target, ...args) => + process.env.DOCKS_AGENT_SCORE_MODE === 'empty-directory' && target === ${JSON.stringify(AGENTS_ROOT)} + ? [] + : readdirSync(target, ...args); +`, +); + +const baseEnv = { ...process.env }; +delete baseEnv.CARGO_TARGET_DIR; +delete baseEnv.GITHUB_ACTIONS; +delete baseEnv.NODE_OPTIONS; + +const runCi = (mode) => { + const result = spawnSync(process.execPath, ['scripts/ci.mjs', '--plugin', TARGET], { + cwd: REPO, + encoding: 'utf8', + timeout: 180_000, + env: { + ...baseEnv, + PATH: `${shimDir}${path.delimiter}${process.env.PATH ?? ''}`, + DOCKS_AGENT_SCORE_MODE: mode, + ...(mode === 'empty-directory' || mode.startsWith('missing-') + ? { + NODE_OPTIONS: `--import=${fixtureHook}`, + DOCKS_MISSING_ROOT: + mode === 'missing-skills-root' ? SKILLS_ROOT : mode === 'missing-agents-root' ? AGENTS_ROOT : '', + } + : {}), + }, + }); + return { status: result.status, output: `${result.stdout ?? ''}${result.stderr ?? ''}` }; +}; + +const rx = (literal) => new RegExp(literal.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')); + +test.after(() => fs.rmSync(shimDir, { recursive: true, force: true })); + +test('a scorer covering every agent on disk keeps the gate green', () => { + const { status, output } = runCi('full'); + assert.equal(status, 0, output); + assert.match( + output, + rx( + `${TARGET} agent score set covers the tree: ${EXPECTED.length} score rows for ${EXPECTED.length} agent files on disk`, + ), + ); + assert.match(output, rx(`${TARGET} agents per-file all ≥ 10`)); +}); +test('a missing declared skills root fails explicitly', () => { + const { status, output } = runCi('missing-skills-root'); + assert.notEqual(status, 0); + assert.match(output, rx(`${TARGET} declared skills root missing: ${SKILLS_ROOT}`)); +}); + +test('a missing declared agents root fails explicitly', () => { + const { status, output } = runCi('missing-agents-root'); + assert.notEqual(status, 0); + assert.match(output, rx(`${TARGET} declared agents root missing: ${AGENTS_ROOT}`)); +}); + +test('an empty declared agents directory fails instead of passing vacuously', () => { + const { status, output } = runCi('empty-directory'); + assert.notEqual(status, 0); + assert.match(output, rx(`${TARGET} declared agents root is empty — no agent score set to gate: ${AGENTS_ROOT}`)); + assert.doesNotMatch(output, rx(`${TARGET} agents per-file all ≥ 10`)); +}); + +test('an agent scorer that exits non-zero fails the gate naming the command', () => { + const { status, output } = runCi('crash'); + assert.notEqual(status, 0); + assert.match(output, rx(`${TARGET} agent scorer exited 3 — no score set to gate`)); + assert.match(output, rx(`node scripts/agents/score.mjs --per-file ${AGENTS_ROOT}`)); + assert.doesNotMatch(output, rx(`${TARGET} agents per-file all ≥ 10`)); +}); + +test('a same-size score set with a wrong row name fails corroboration', () => { + const { status, output } = runCi('mismatch'); + assert.notEqual(status, 0); + assert.match( + output, + rx( + `${TARGET} agent score set does not cover the tree: expected ${EXPECTED.length} agent(s) on disk, parsed ${EXPECTED.length} score row(s)`, + ), + ); + assert.match(output, rx(`unscored on disk: ${EXPECTED[0]}`)); + assert.match(output, rx('not on disk: not-on-disk')); + assert.doesNotMatch(output, rx(`${TARGET} agents per-file all ≥ 10`)); +}); + +test('a non-finite agent score fails before floor arithmetic', () => { + const { status, output } = runCi('non-finite'); + assert.notEqual(status, 0); + assert.match(output, rx(`${TARGET} agent scorer produced non-finite score(s): ${EXPECTED[0]} NaN`)); + assert.doesNotMatch(output, rx(`${TARGET} agents per-file all ≥ 10`)); +});