open/config-audit
1
0
Fork 0
Code Issues Releases 1 Activity

fix(commands): router dogfood — five seam defects, plus the placeholder class

Browse source 
Dogfooding `/config-audit` (the router) against the repo, fasit written before
any run (docs/router-fasit.local.md, untouched). Every claim below is measured
behaviour, not a reading of the source.

1. Bare `<target-path>` inside the step-3 fence is a shell REDIRECTION, not an
   argument. Measured in zsh: both CLIs failed before starting, no output file
   was written, and the echoed status was 1 — inside the band the router's own
   gate calls "continue normally". Quoting makes an unsubstituted placeholder
   reach argv, so it fails in the CLI where the exit code means something.
   Swept the whole class: 30 sites across 12 further command files, since a
   defect in one file is a class until the opposite is measured. New guard:
   command-placeholder-shell-safety.test.mjs.

2. The orchestrator's exit code was discarded. Two commands on one line share a
   single trailing `echo $?`, which reports only the last: measured, an
   orchestrator exit 3 echoed as posture's 0, so the "3 -> stop" gate could
   never fire. Both statuses are now captured and echoed.

3. "Running 12 configuration scanners" — the orchestrator registers 16. The new
   test binds the narrated count to the registry so the next scanner added
   cannot re-stale it silently.

4. The Area Breakdown table hardcoded 7 rows; posture emits 9 quality areas.
   Token Efficiency (a B on this repo) and Plugin Hygiene never reached the
   user. Rows added, and the row set is now asserted against lib/scoring.mjs.
   Label aligned: "MCP Servers" -> "MCP", as posture emits it.

5. Step 6 rendered "the headline line from the humanized stderr scorecard" and
   forbade deriving a replacement — while step 3 sent posture's stderr to
   /dev/null, as UX rule 2 requires, and the prose is absent from the JSON
   payload (measured). The slot could only be improvised. posture's stderr now
   goes to a file in the session dir, as commands/posture.md already did; the
   user still never sees raw scanner output.

Also: `grep -q -- "--raw"` matched any argument CONTAINING --raw (measured on
`--rawdog` and on a path with --raw in it) — anchored to whole arguments.
SCOPE_FLAGS renamed SCOPE_FLAG, since zsh does not word-split and the plural
invited the M-BUG-45 shape.

command-shell-state-shape.test.mjs only recognised line-initial assignments, so
it reported the idiomatic `node …; STATUS=$?` capture as never assigned. Widened
to assignments after a separator; verified it still fails on a real cross-block
reference before trusting it.

Suite 1477 -> 1483, frozen v5.0.0 snapshots untouched.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YDAwy1ZXRpZxht1wyCeSbF
This commit is contained in:
Kjell Tore Guttormsen 2026-08-09 21:18:05 +02:00
commit 0b763f25c1
16 changed files with 320 additions and 41 deletions
Download patch file Download diff file Expand all files Collapse all files

102
tests/commands/command-placeholder-shell-safety.test.mjs Normal file
View file

@ -0,0 +1,102 @@
/**
* Session #56 — placeholders inside runnable bash fences must not be shell-active.
*
* Dogfooding the ROUTER (`commands/config-audit.md`) surfaced a defect one layer
* below the CLIs: the fence on the router's step 3 carries the placeholder
* `<target-path>` **bare** — unquoted, preceded by a space. `<` and `>` are
* redirection operators. Measured under the shell the Bash tool actually runs
* (zsh), in a scratch directory:
*
* $ node …/scan-orchestrator.mjs <target-path> --output-file scan.json \
* >/dev/null 2>/dev/null; node …/posture.mjs <target-path> \
* --output-file posture.json 2>/dev/null; echo $?
* zsh:1: no such file or directory: target-path
* zsh:1: no such file or directory: target-path
* 1
* → neither file written, neither CLI ever started
*
* Three properties make this worse than a plain typo:
*
* 1. **The CLI never runs.** Redirection is resolved by the shell before the
* command is executed, so the CLI's own argument validation — the layer that
* `cli-unknown-flag-rejection.test.mjs` hardened — never sees it.
* 2. **The failure is quiet where it counts.** The echoed status is `1`, and
* `1` is inside the band the router's own step 3 classifies as
* "continue normally" (0/1/2 = PASS/WARNING/FAIL; only 3 is a real error).
* A total non-execution is indistinguishable from a healthy WARNING run.
* 3. **The file already knew about the neighbouring hazard.** Two lines above
* the offending call sits a comment warning that a *square-bracket*
* placeholder "does not start with a dash, so both CLIs' arg loops would
* take it as the TARGET PATH instead of a flag" — awareness of the
* placeholder class, while carrying a strictly worse member of it.
*
* The invariant is not "substitute your placeholders" (a template cannot enforce
* that). It is that an UNSUBSTITUTED placeholder must fail **loudly, in the
* CLI**, not silently in the shell. Quoting achieves exactly that: `"<path>"`
* reaches argv as a literal, the CLI reports an unreadable target, and the exit
* code means what the router thinks it means.
*
* Measured breadth at the time of writing: 13 of 21 command files, 33 sites.
*/
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 COMMANDS_DIR = resolve(__dirname, '..', '..', 'commands');
async function commandFiles() {
const entries = await readdir(COMMANDS_DIR);
return entries.filter((e) => e.endsWith('.md')).sort();
}
/**
* Yield the lines that live inside ```bash fences, with their 1-based file line
* numbers. Only bash-tagged fences count: an untagged or json fence is prose.
*/
function bashFenceLines(content) {
const out = [];
let inFence = false;
content.split('\n').forEach((line, i) => {
if (/^\s*```/.test(line)) {
inFence = /^\s*```bash\s*$/.test(line);
return;
}
if (inFence) out.push({ line, n: i + 1 });
});
return out;
}
/**
* A placeholder that the shell would read as a redirection: `<` at the start of
* a word (start of line, or after whitespace / `;` / `|` / `&`), a lowercase
* placeholder name, then `>`. A quoted placeholder (`"<path>"`) is excluded by
* construction — the `<` is preceded by a quote, not a word boundary.
*/
const BARE_PLACEHOLDER = /(?:^|[\s;|&(])(<[a-z][a-z0-9._-]*>)/;
test('no runnable bash fence carries a bare (shell-active) angle-bracket placeholder', async () => {
const offenders = [];
for (const file of await commandFiles()) {
const content = await readFile(resolve(COMMANDS_DIR, file), 'utf-8');
for (const { line, n } of bashFenceLines(content)) {
const m = line.match(BARE_PLACEHOLDER);
if (m) offenders.push(`${file}:${n} ${m[1]} in: ${line.trim().slice(0, 90)}`);
}
}
assert.deepEqual(
offenders,
[],
'A bare `<name>` inside a bash fence is a REDIRECTION, not an argument. Left\n' +
'unsubstituted it fails in the shell before the CLI starts — no output file,\n' +
'no CLI diagnostics, and an exit code (1) that the router reads as a normal\n' +
'WARNING run. Quote the placeholder (`"<name>"`) so an unsubstituted template\n' +
'fails loudly in the CLI instead.\n' +
'Offending sites:\n ' + offenders.join('\n '),
);
});

14
tests/commands/command-shell-state-shape.test.mjs
View file

@ -82,12 +82,18 @@ test('Shell state: no variable is referenced outside the block that assigned it'
const assignedIn = new Map();
lines.forEach((raw, i) => {
const m = stripComment(raw).match(/^\s*([A-Z_][A-Z0-9_]*)=/);
if (!m) return;
const bi = blockIndexOf(i);
if (bi < 0) return;
if (!assignedIn.has(m[1])) assignedIn.set(m[1], new Set());
assignedIn.get(m[1]).add(bi);
// An assignment starts a command, and a command starts at the beginning of
// the line OR after a separator. Capturing only the line-initial form would
// miss the idiomatic status capture `node …; STATUS=$?` — the one form that
// MUST trail a command — and report the variable as never assigned.
const re = /(?:^|[;&|]|&&|\|\|)\s*([A-Z_][A-Z0-9_]*)=/g;
let m;
while ((m = re.exec(stripComment(raw))) !== null) {
if (!assignedIn.has(m[1])) assignedIn.set(m[1], new Set());
assignedIn.get(m[1]).add(bi);
}
});
lines.forEach((raw, i) => {

161
tests/commands/router-shape.test.mjs Normal file
View file

@ -0,0 +1,161 @@
/**
* Session #56 — shape invariants for the ROUTER (`commands/config-audit.md`).
*
* `/config-audit` with no arguments is the plugin's front door: it auto-detects
* scope, runs `scan-orchestrator` + `posture`, and renders the result. Dogfooding
* it surfaced four defects that a scanner test could not see, because each lives
* at the seam between the command template and something it does not own — the
* shell, the scanner registry, the area registry, or the stderr stream.
*
* 1. **The orchestrator's exit code is discarded.** Step 3 runs both CLIs on one
* line and appends a single `echo $?`, which reports only the LAST command.
* Measured:
*
* $ node -e "process.exitCode=3" >/dev/null 2>/dev/null; \
* node -e "process.exitCode=0" 2>/dev/null; echo $?
* 0
*
* An orchestrator hard-error (exit 3) is therefore invisible to the very gate
* step 3 defines for it ("3 → tell user … and stop").
*
* 2. **The narrated scanner count is stale.** Step 3 tells the user "Running 12
* configuration scanners"; the orchestrator registers and runs 16. Asserted
* against the registry rather than a literal, so the next scanner addition
* cannot re-stale it silently.
*
* 3. **The Area Breakdown table drops areas.** Posture emits 10 areas (9 quality
* areas plus Feature Coverage, which the template excludes by design). The
* table hardcodes 7 rows, so `Token Efficiency` and `Plugin Hygiene` — both
* real, both graded, one of them a B on this very repo — never reach the
* user. Asserted against the area registry in `lib/scoring.mjs`.
*
* 4. **The template consumes a stream the same file mandates be thrown away.**
* Step 6 instructs: "Use the headline line from the humanized stderr
* scorecard … Avoid hardcoding a separate per-grade prose ladder." That
* headline (`Health: A (93/100) — Healthy setup, only minor polish needed`)
* exists ONLY on posture's stderr — measured absent from the JSON payload —
* and step 3's fence sends posture's stderr to /dev/null, as UX rule 2
* requires. So the router is told to render something it cannot obtain and
* forbidden from deriving a replacement; the slot can only be improvised.
* `commands/posture.md` already shows the fix in-repo: redirect stderr to a
* FILE (`2>/tmp/…-stderr.txt`), which satisfies "the user never sees it"
* while keeping the text readable.
*
* This is the mirror image of [[stderr-only-warnings-invisible-to-commands]]:
* there a WARNING was lost to /dev/null, here a REQUIRED INPUT is.
*/
import { test } from 'node:test';
import { strict as assert } from 'node:assert';
import { readFile } 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 ROUTER = resolve(ROOT, 'commands', 'config-audit.md');
const router = await readFile(ROUTER, 'utf-8');
/** Scanner ids the orchestrator actually registers, read from its registry. */
async function registeredScanners() {
const src = await readFile(resolve(ROOT, 'scanners', 'scan-orchestrator.mjs'), 'utf-8');
const block = src.slice(src.indexOf('const SCANNERS = ['));
const arr = block.slice(0, block.indexOf('\n];'));
return [...arr.matchAll(/\{\s*name:\s*'([A-Z]+)'/g)].map((m) => m[1]);
}
/** Distinct posture area names, read from the scanner→area registry. */
async function registeredAreas() {
const src = await readFile(resolve(ROOT, 'scanners', 'lib', 'scoring.mjs'), 'utf-8');
const block = src.slice(src.indexOf('const SCANNER_AREA_MAP = {'));
const arr = block.slice(0, block.indexOf('\n};'));
return [...new Set([...arr.matchAll(/:\s*'([^']+)'/g)].map((m) => m[1]))];
}
test('the router surfaces the exit code of BOTH scanner invocations', async () => {
const fence = router.slice(router.indexOf('### Step 3'), router.indexOf('### Step 4'));
const chained = fence
.split('\n')
.filter((l) => (l.match(/\bnode \$\{CLAUDE_PLUGIN_ROOT\}/g) || []).length > 1);
assert.deepEqual(
chained,
[],
'Step 3 runs scan-orchestrator and posture on ONE line with a single trailing\n' +
'`echo $?`, which reports only the LAST command. An orchestrator exit 3 echoes\n' +
'as posture\'s 0 and the "3 → stop" gate never fires. Capture each status into\n' +
'its own variable and echo both.\n' +
'Offending line(s):\n ' + chained.map((l) => l.trim().slice(0, 120)).join('\n '),
);
assert.match(
fence,
/echo "\$[A-Z_]+ \$[A-Z_]+"/,
'Step 3 must echo both captured exit codes (e.g. `echo "$ORCH_STATUS $POSTURE_STATUS"`)\n' +
'so the gate can act on either scanner failing.',
);
});
test('the scanner count the router narrates matches the orchestrator registry', async () => {
const scanners = await registeredScanners();
const narrated = [...router.matchAll(/Running (\d+) configuration scanners/g)].map((m) =>
Number(m[1]),
);
assert.ok(narrated.length > 0, 'The router should still narrate how many scanners are running.');
assert.deepEqual(
narrated,
narrated.map(() => scanners.length),
`The router tells the user it is running ${narrated.join('/')} scanners; the ` +
`orchestrator registers ${scanners.length} (${scanners.join(', ')}). A user-facing ` +
'count that no test binds to the registry goes stale on the next scanner added.',
);
});
test('the Area Breakdown table has a row for every quality area posture emits', async () => {
const areas = (await registeredAreas()).filter((a) => a !== 'Feature Coverage');
const table = router.slice(router.indexOf('### Area Breakdown'), router.indexOf('{For the status column'));
const missing = areas.filter((a) => !table.includes(`| ${a} |`));
assert.deepEqual(
missing,
[],
'Posture grades these areas but the router\'s table has no row for them, so they\n' +
'are silently dropped from the user\'s results. (Feature Coverage is excluded by\n' +
'design — it is reported as opportunities, not as a quality grade.)\n' +
`Areas emitted: ${areas.join(', ')}\n` +
`Missing rows: ${missing.join(', ')}`,
);
});
test('the router does not consume a stderr scorecard it discards', async () => {
const fence = router.slice(router.indexOf('### Step 3'), router.indexOf('### Step 4'));
const dependsOnStderr = /stderr scorecard/i.test(router);
const postureDiscardsStderr = /posture\.mjs[^\n]*2>\/dev\/null/.test(fence);
assert.ok(
!(dependsOnStderr && postureDiscardsStderr),
'Step 6 renders "the headline line from the humanized stderr scorecard" and forbids\n' +
'deriving a grade-prose ladder instead — but step 3 sends posture\'s stderr to\n' +
'/dev/null, and the prose is absent from the JSON payload (measured). The slot can\n' +
'only be improvised. Capture posture stderr to a FILE and read the headline from\n' +
'it, as commands/posture.md already does.',
);
});
test('the --raw detection is not substring-based', async () => {
const substringMatch = /grep -q -- "--raw"/.test(router);
assert.ok(
!substringMatch,
'Measured: `echo "$ARGUMENTS" | grep -q -- "--raw"` turns raw mode ON for `--rawdog`\n' +
'and for any path containing `--raw`. Anchor the match to whole arguments.',
);
assert.match(
router,
/grep -qE -- '\(\^\| \)--raw\( \|\$\)'/,
'The --raw check must match a whole argument, not a substring.',
);
});