fix(agents): an agent cannot promise what its tools forbid

R4 — `verifier-agent.md` carried two contracts at once: §Output Format said
"Append to: implementation-log.md", §Read-Only Guarantee said "never modifies
any files", and `tools:` granted only Read/Glob/Grep. Which one wins is
nondeterministic, and the loss is not a blocked write but a full-file Write on
the log the agent SHARES with the parallel implementer agents — the defect
`implement-log-append.test.mjs` exists to prevent, entering through the one file
that test does not read. The orchestrator half was already right
(`implement.md` Step 5 appends with Bash `>>` and tells the agent not to write),
so the fix is one-way: the agent file now returns its report inline and names
who appends it, and why a Write there would clobber.

The guard is the blanket invariant over the catalogue, not a fact about one
file: any agent whose tools grant no write capability must instruct no write AND
say positively that it returns findings inline. Tools and body are both read, so
stripping `Write` from any agent whose body still writes turns it red. Measured
1 of 7 agents carried the defect; the sweep asserts a write-tool-less agent
exists so the invariant cannot pass vacuously.

R6 — both "Required Frontmatter" rules were enforced by nothing, and the only
test reading agent frontmatter checked `name:` against a hand-written 3-of-7
list. The new guard takes nothing by hand: required keys are parsed from each
rule's own yaml block, the swept files from each rule's own `paths:`, the plugin
name from plugin.json — add a key to a rule and it is enforced next run. The
repo was already 7/7 and 21/21 compliant, so a green first run proves nothing:
all seven arms were seen red against a temporarily introduced defect one at a
time, including emptying a rule's yaml block to show the derivation is not
vacuous.

The color enum is deliberately NOT guarded: the official subagent docs list
red/blue/green/yellow/purple/orange/pink/cyan (no magenta) while issue 19292
lists magenta but neither purple nor orange, and this plugin ships both. Pinning
an unsettled set would encode an unverified premise rather than measure one.

Suite 1777 -> 1785, 0 fail. Frozen tests/snapshots/v5.0.0/ untouched.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MEMKCAyVzYTMzLaqPcULVr
This commit is contained in:
Kjell Tore Guttormsen 2026-08-20 22:59:51 +02:00
commit d5714261d1
4 changed files with 289 additions and 3 deletions

View file

@ -83,6 +83,37 @@ Finding ID format: `CA-{SCANNER}-{NNN}` — e.g. `CA-CML-001`, `CA-SET-003`, `CA
**`{NNN}` names the CHECK, never the emission position (invariant).** `scanners/lib/finding-codes.mjs` is the single authority: every `finding()` call passes a `code`, and an undeclared or missing one **throws** — there is no counter fallback, because a fallback lets a half-converted scanner ship IDs that look valid. Adding a check takes the next free number for that scanner, never the next source-order position; removing one moves its key to `RETIRED_CODES` and its number is never reissued. IDs are therefore **not unique per finding** — one check failing in three files emits three findings sharing an ID, and `(id, file, line)` is the instance key that `fix-engine` verification uses. Frozen `v5.0.0` baselines mask IDs (`tests/helpers/mask-finding-ids.mjs`) instead of re-deriving them; the check→number pairs are pinned exhaustively in `tests/lib/finding-codes.test.mjs`. **`{NNN}` names the CHECK, never the emission position (invariant).** `scanners/lib/finding-codes.mjs` is the single authority: every `finding()` call passes a `code`, and an undeclared or missing one **throws** — there is no counter fallback, because a fallback lets a half-converted scanner ship IDs that look valid. Adding a check takes the next free number for that scanner, never the next source-order position; removing one moves its key to `RETIRED_CODES` and its number is never reissued. IDs are therefore **not unique per finding** — one check failing in three files emits three findings sharing an ID, and `(id, file, line)` is the instance key that `fix-engine` verification uses. Frozen `v5.0.0` baselines mask IDs (`tests/helpers/mask-finding-ids.mjs`) instead of re-deriving them; the check→number pairs are pinned exhaustively in `tests/lib/finding-codes.test.mjs`.
**A file's contract cannot exceed its tools (invariant).** `agents/verifier-agent.md` said both
"Append to: implementation-log.md" (§Output Format) and "never modifies any files" (§Read-Only
Guarantee) while granting only `Read, Glob, Grep`. The failure mode is not a blocked write — it is
the agent improvising a full-file `Write` on the log it *shares* with the parallel implementer
agents, which is the exact defect `implement-log-append.test.mjs` exists to prevent, entering
through the one file that test does not read. `tests/agents/agent-write-contract.test.mjs` asserts
the blanket invariant over the whole catalogue rather than a fact about one file: an agent whose
`tools:` grant no write capability (`Write`/`Edit`/`NotebookEdit`/`Bash`) must instruct no file
write **and** state positively that it returns its findings inline. Both sides are read from the
file, so stripping `Write` from any agent whose body still instructs a write turns it red, and the
sweep asserts a write-tool-less agent exists so the invariant cannot pass vacuously. Measured
2026-08-20: **1 of 7** agents carried the defect; the other six all hold `Write`. The orchestrator
half was already correct — `implement.md` Step 5 appends with Bash `>>` and tells the agent not to
write — so this was a one-way fix in the agent file, not a two-sided one.
**Frontmatter contracts are derived, never listed (invariant).** `.claude/rules/agent-development.md`
and `.claude/rules/command-development.md` state which keys an agent/command MUST carry and that
agent colors are unique; nothing enforced any of it, and the only test that read agent frontmatter
checked `name:` against a hand-written 3-of-7 list. `tests/agents/frontmatter-contract.test.mjs`
takes nothing by hand: required keys are parsed from each rule's own fenced `yaml` block, the files swept
are resolved from each rule's own `paths:`, and the plugin name is read from
`.claude-plugin/plugin.json` — add a key to a rule and it is enforced on the next run with no test
edit, because a hand-kept list of what to sweep is a premise, not a measurement. The repo was
already compliant (7/7 agents, 21/21 commands, 0 duplicate colors), so a green first run proves
nothing: every arm was seen red against a temporarily introduced defect **one at a time**,
including emptying a rule's yaml block to show the derivation is not vacuous. The color **enum** is
deliberately *not* guarded — the official subagent docs list `red/blue/green/yellow/purple/orange/
pink/cyan` (no `magenta`) while issue #19292 lists `magenta` but neither `purple` nor `orange`, and
this plugin ships both `magenta` and `orange`; pinning an unsettled set in a guard would encode an
unverified premise rather than measure one.
## Conventions ## Conventions
Enforced conventions live in `.claude/rules/` (auto-loaded as project instructions): Enforced conventions live in `.claude/rules/` (auto-loaded as project instructions):

View file

@ -140,7 +140,15 @@ Checking for secrets...
## Output Format ## Output Format
Append to: `~/.claude/config-audit/sessions/{session-id}/implementation-log.md` Return the report below as your final message. Do NOT write it to a file: this
agent is read-only by design (`tools: Read, Glob, Grep`) and has no write tool,
so a write instruction here would be a contract it cannot keep.
The orchestrator appends what you return to
`~/.claude/config-audit/sessions/{session-id}/implementation-log.md` itself,
with Bash `>>` (`commands/implement.md` Step 5) — never the Write tool. That log
is shared with the implementer agents running in parallel, and a full-file Write
on it silently clobbers their entries.
```markdown ```markdown
## Verification Report ## Verification Report
@ -243,8 +251,8 @@ Optional: Generate before/after comparison:
This agent: This agent:
- Only uses Read, Glob, Grep tools - Only uses Read, Glob, Grep tools
- Never modifies any files - Never modifies any files, including the shared implementation log
- Reports findings without taking action - Reports findings without taking action — every result is returned inline
- Safe to run multiple times - Safe to run multiple times
## Model policy ## Model policy

View file

@ -0,0 +1,109 @@
/**
* R4 an agent file must not carry a contract its `tools:` cannot keep.
*
* `agents/verifier-agent.md` said both things at once: §Output Format
* ("Append to: implementation-log.md") and §Read-Only Guarantee ("never
* modifies any files"), while its frontmatter granted only Read/Glob/Grep.
* Which instruction wins is nondeterministic, and the failure mode is not
* "the write fails" it is the agent improvising a full-file Write on the
* SHARED implementation log, clobbering parallel implementer entries. That is
* exactly the defect `implement-log-append.test.mjs` exists to prevent,
* entering through the one file that test does not read.
*
* The guard is the blanket invariant over the whole agents/ catalogue, not a
* statement about verifier-agent: any agent whose tools grant no write
* capability must (a) instruct no file write, and (b) say positively that it
* returns its findings inline. Both sides are read from the file the tools
* list AND the body so removing a write tool from any agent whose body still
* instructs a write turns this red.
*/
import { test } from 'node:test';
import { strict as assert } from 'node:assert';
import { readFile, readdir } from 'node:fs/promises';
import { resolve, dirname } from 'node:path';
import { fileURLToPath } from 'node:url';
const __dirname = dirname(fileURLToPath(import.meta.url));
const ROOT = resolve(__dirname, '..', '..');
const AGENTS_DIR = resolve(ROOT, 'agents');
/** Tools that can put bytes on disk. Bash counts: `>>` is a write. */
const WRITE_TOOLS = new Set(['Write', 'Edit', 'NotebookEdit', 'Bash']);
/**
* Directive lines that tell the agent to put its output in a file.
* Anchored to line start so prose ABOUT writing ("do not write it to a file")
* is not caught the defect is an instruction, not a mention.
*/
const WRITE_DIRECTIVE_RE =
/^(?:\*\*)?(?:Append|Write|Save|Output|Persist)\b(?![^\n]*\bnot\b)[^\n]*?(?:\bto\b|`[^`\n]+\.(?:md|ya?ml|json)`)/mi;
/** The positive half: the file must say the findings come back inline. */
const RETURN_INLINE_RE =
/\breturn\b[^.]{0,120}?\b(?:as\s+your\s+final\s+message|inline)\b/i;
function frontmatterOf(content) {
const m = content.match(/^---\r?\n([\s\S]*?)\r?\n---\r?\n/);
return m ? m[1] : '';
}
function bodyOf(content) {
const m = content.match(/^---\r?\n[\s\S]*?\r?\n---\r?\n/);
return m ? content.slice(m[0].length) : content;
}
function toolsOf(frontmatter) {
const m = frontmatter.match(/^tools:\s*(.+)$/m);
if (!m) return [];
return (m[1].match(/[A-Za-z_][A-Za-z0-9_]*/g) || []);
}
async function loadAgents() {
const names = (await readdir(AGENTS_DIR)).filter((n) => n.endsWith('.md')).sort();
return Promise.all(
names.map(async (name) => {
const content = await readFile(resolve(AGENTS_DIR, name), 'utf-8');
const frontmatter = frontmatterOf(content);
const tools = toolsOf(frontmatter);
return {
name,
body: bodyOf(content),
tools,
canWrite: tools.some((t) => WRITE_TOOLS.has(t)),
};
}),
);
}
test('R4 sweep is not vacuous: the agents catalogue is read and at least one agent has no write tool', async () => {
const agents = await loadAgents();
assert.ok(agents.length >= 7,
`expected the agents/ catalogue to be swept, got ${agents.length} files`);
assert.ok(agents.every((a) => a.tools.length > 0),
`every agent must declare tools:, missing in ${agents.filter((a) => !a.tools.length).map((a) => a.name).join(', ')}`);
const writeless = agents.filter((a) => !a.canWrite);
assert.ok(writeless.length >= 1,
'no write-tool-less agent found — the invariant below would be vacuously green');
});
test('R4: no agent instructs a file write its tools cannot perform', async () => {
const agents = await loadAgents();
for (const agent of agents.filter((a) => !a.canWrite)) {
const offending = agent.body.match(WRITE_DIRECTIVE_RE);
assert.ok(
offending === null,
`${agent.name} grants no write tool (tools: ${agent.tools.join(', ')}) but instructs a write: ${JSON.stringify(offending && offending[0])}`,
);
}
});
test('R4: a write-tool-less agent states positively that it returns findings inline', async () => {
const agents = await loadAgents();
for (const agent of agents.filter((a) => !a.canWrite)) {
assert.ok(
RETURN_INLINE_RE.test(agent.body),
`${agent.name} has no write tool, so it must say its findings are returned as its final message`,
);
}
});

View file

@ -0,0 +1,138 @@
/**
* R6 the two "Required Frontmatter" contracts are enforced by something.
*
* `.claude/rules/agent-development.md` and `.claude/rules/command-development.md`
* write down which frontmatter keys an agent/command MUST carry, and that agent
* colors must be unique within the plugin. Nothing enforced any of it: the only
* test that looked at agent frontmatter (`agent-prompt-shape`) asserted `name:`
* on a HAND-WRITTEN list of 3 of the 7 agents. Everything was compliant and
* unwatched the state in which a rule becomes fiction one file at a time.
*
* Nothing here is hand-maintained, because a hand-kept list of what to sweep is
* a premise, not a measurement (the #57 shape):
* - the required KEYS are parsed out of each rule's own ```yaml block;
* - the files swept are resolved from each rule's own `paths:` frontmatter;
* - the plugin name is read from `.claude-plugin/plugin.json`.
* Add a key to a rule and it is enforced on the next run, with no test edit.
*/
import { test } from 'node:test';
import { strict as assert } from 'node:assert';
import { readFile, readdir } from 'node:fs/promises';
import { resolve, dirname } from 'node:path';
import { fileURLToPath } from 'node:url';
const __dirname = dirname(fileURLToPath(import.meta.url));
const ROOT = resolve(__dirname, '..', '..');
const RULES_DIR = resolve(ROOT, '.claude', 'rules');
function frontmatterOf(content) {
const m = content.match(/^---\r?\n([\s\S]*?)\r?\n---\r?\n/);
return m ? m[1] : '';
}
/** Top-level keys of a frontmatter block, in source order. */
function topLevelKeys(frontmatter) {
return frontmatter
.split('\n')
.map((line) => line.match(/^([A-Za-z][A-Za-z0-9_-]*):/))
.filter(Boolean)
.map((m) => m[1]);
}
function valueOf(frontmatter, key) {
const m = frontmatter.match(new RegExp(`^${key}:[ \\t]*(.*)$`, 'm'));
return m ? m[1].trim() : null;
}
/**
* A rule file is the contract. Read the required keys out of the ```yaml block
* under "## Required Frontmatter", and the swept directory out of `paths:`.
*/
async function loadRule(fileName) {
const content = await readFile(resolve(RULES_DIR, fileName), 'utf-8');
const paths = valueOf(frontmatterOf(content), 'paths');
const fence = content.match(/##\s+Required Frontmatter[\s\S]*?```ya?ml\r?\n([\s\S]*?)```/);
const requiredKeys = fence ? topLevelKeys(fence[1]).filter((k) => k !== '---') : [];
const dir = paths ? paths.split('/')[0] : null;
return { fileName, paths, dir, requiredKeys };
}
async function loadTargets(rule) {
const dirAbs = resolve(ROOT, rule.dir);
const names = (await readdir(dirAbs)).filter((n) => n.endsWith('.md')).sort();
return Promise.all(
names.map(async (name) => {
const frontmatter = frontmatterOf(await readFile(resolve(dirAbs, name), 'utf-8'));
return { name, frontmatter, keys: topLevelKeys(frontmatter) };
}),
);
}
const AGENT_RULE = 'agent-development.md';
const COMMAND_RULE = 'command-development.md';
test('R6 derivation is not vacuous: both rules yield required keys and a non-empty file set', async () => {
for (const fileName of [AGENT_RULE, COMMAND_RULE]) {
const rule = await loadRule(fileName);
assert.ok(rule.requiredKeys.length > 0,
`${fileName}: no required keys parsed from its "Required Frontmatter" yaml block — every assertion below would be vacuously green`);
assert.ok(rule.dir, `${fileName}: no paths: frontmatter to resolve a file set from`);
const targets = await loadTargets(rule);
assert.ok(targets.length > 0,
`${fileName}: paths: ${rule.paths} resolved to 0 files — the sweep would prove nothing`);
}
});
test('R6: every agent and command carries the keys its rule requires, non-empty', async () => {
for (const fileName of [AGENT_RULE, COMMAND_RULE]) {
const rule = await loadRule(fileName);
const targets = await loadTargets(rule);
for (const target of targets) {
for (const key of rule.requiredKeys) {
assert.ok(target.keys.includes(key),
`${rule.dir}/${target.name} is missing required frontmatter key "${key}" (required by .claude/rules/${fileName}; ${targets.length} files swept)`);
const value = valueOf(target.frontmatter, key);
assert.ok(value !== null && value !== '',
`${rule.dir}/${target.name} has an empty "${key}" — a present-but-empty key satisfies no contract`);
}
}
}
});
test('R6: agent colors are unique within the plugin', async () => {
const rule = await loadRule(AGENT_RULE);
const targets = await loadTargets(rule);
const seen = new Map();
for (const target of targets) {
const color = valueOf(target.frontmatter, 'color');
if (seen.has(color)) {
assert.fail(`duplicate agent color "${color}": ${seen.get(color)} and ${target.name} (.claude/rules/${AGENT_RULE}: "Color must be unique within the plugin")`);
}
seen.set(color, target.name);
}
assert.equal(seen.size, targets.length, `expected ${targets.length} distinct colors, got ${seen.size}`);
});
test('R6: agent names are kebab-case with the -agent suffix', async () => {
const rule = await loadRule(AGENT_RULE);
for (const target of await loadTargets(rule)) {
const name = valueOf(target.frontmatter, 'name');
assert.match(name, /^[a-z0-9]+(?:-[a-z0-9]+)*-agent$/,
`agents/${target.name}: name "${name}" must be kebab-case with an -agent suffix`);
}
});
test('R6: command names are plugin:action, or the bare plugin name for the router', async () => {
const plugin = JSON.parse(await readFile(resolve(ROOT, '.claude-plugin', 'plugin.json'), 'utf-8')).name;
const rule = await loadRule(COMMAND_RULE);
const targets = await loadTargets(rule);
const routers = [];
for (const target of targets) {
const name = valueOf(target.frontmatter, 'name');
if (name === plugin) { routers.push(target.name); continue; }
assert.match(name, new RegExp(`^${plugin}:[a-z0-9]+(?:-[a-z0-9]+)*$`),
`commands/${target.name}: name "${name}" must be "${plugin}:action" (or the bare "${plugin}" router)`);
}
assert.equal(routers.length, 1, `expected exactly one bare-"${plugin}" router command, got ${routers.length}: ${routers.join(', ')}`);
});