fix(commands): close the promises the command templates could not keep

DEL B chunk `interview` (+ discover/status/cleanup/help). Fasit written before
the run predicted 8 defects and refuted 4 candidates; all 8 confirmed, all 4
refutations held, and three predictions turned out too narrow.

- M-BUG-36: `drift --list` reached the command as 0 bytes. drift-cli accepted
  --output-file but list mode ignored it, and the listing goes to stderr, which
  the command discards per ux-rules rule 2. Fixing the caller alone would not
  have helped.
- M-BUG-37: feature-gap's "Create backup" step ran fix-cli without --apply.
  Dry-run is the default, so no backup existed (backupId: null) while the
  command went on to edit config believing it could roll back.
- M-BUG-38: fix-cli told users to recover with scanners/rollback-cli.mjs, which
  does not exist. Dead reference in the one message read after a bad fix.
- M-BUG-21 fourth arm: five templates carried literal [--global]/[--full-machine]
  inside executable bash blocks. A bracketed placeholder does not start with a
  dash, so every scanner's arg loop takes it as the scan target.
- interview and analyze never said which session they act on; interview could
  rewind a finished session; cleanup interpolated an unvalidated id into rm -rf
  (an empty id deletes every session); status advertised a `resume` command that
  does not exist and documented an `all` argument it never parsed.

TDD: 9 red tests first, including a machine sweep for dead /config-audit
references and for bracketed flags in bash blocks. Suite 1432 -> 1441/0.
Frozen v5.0.0 snapshots untouched; --raw/--json contracts unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UGvA1uUQn2hPBPMaCKK6x3
This commit is contained in:
Kjell Tore Guttormsen 2026-07-31 21:27:07 +02:00
commit b85919f2ec
17 changed files with 318 additions and 28 deletions

View file

@ -0,0 +1,151 @@
/**
* DEL B chunk `interview` (session #47) command-contract shape tests.
*
* Every assertion here encodes a defect that was MEASURED against the shipped
* command templates before the fix, per the fasit in
* `docs/interview-fasit.local.md`:
*
* - interview.md / analyze.md referenced `{session-id}` with no rule for
* resolving WHICH session every other session-aware command has one.
* - interview.md could push a finished session backwards to `interview`
* with no guard (state-management.md mandates the write, nothing bounded it).
* - status.md advertised `/config-audit resume {session-id}`; no resume
* command exists. ux-rules: "Never reference commands that don't exist."
* - status.md documented `/config-audit status all` but its flag-parse step
* knew only `--raw`.
* - cleanup.md ran `rm -rf .../sessions/{session-id}/` with no guard against
* an empty/malformed id an empty expansion deletes every session.
* - discover.md carried literal `[--full-machine] [--global]` inside an
* executable bash block; `[` is not `-`, so scan-orchestrator's arg loop
* turns it into the SCAN TARGET (M-BUG-21 class).
* - feature-gap.md called `fix-cli.mjs --json` under the heading
* "Create backup". fix-cli is dry-run by default: no backup was created,
* `backupId` came back null, and the command then edited config believing
* it could roll back (M-BUG-31 class).
* - fix-cli.mjs told the user to recover with `node scanners/rollback-cli.mjs`
* a file that does not exist.
*/
import { test } from 'node:test';
import { strict as assert } from 'node:assert';
import { readFileSync, existsSync, readdirSync } from 'node:fs';
import { resolve, dirname } from 'node:path';
import { fileURLToPath } from 'node:url';
const __dirname = dirname(fileURLToPath(import.meta.url));
const ROOT = resolve(__dirname, '..', '..');
const COMMANDS_DIR = resolve(ROOT, 'commands');
const read = (name) => readFileSync(resolve(COMMANDS_DIR, name), 'utf-8');
/** Bash fenced blocks only — prose may legitimately use bracket notation. */
function bashBlocks(text) {
return [...text.matchAll(/```bash\n([\s\S]*?)```/g)].map((m) => m[1]);
}
test('interview.md and analyze.md resolve WHICH session before using {session-id}', () => {
for (const file of ['interview.md', 'analyze.md']) {
const text = read(file);
assert.match(
text,
/sessions\/\*\/state\.yaml/,
`${file} references {session-id} but never says how to find the session`
);
assert.match(
text,
/most recent|most recently modified/i,
`${file} must state which session wins when several exist`
);
}
});
test('interview.md guards against pushing a finished session backwards', () => {
const text = read('interview.md');
assert.match(
text,
/completed_phases/,
'interview.md must reason about completed_phases before rewriting current_phase'
);
assert.match(
text,
/do not (re-?add|add)|already contains|leave .*completed_phases/i,
'interview.md must forbid re-adding a phase already in completed_phases'
);
});
test('no command references a /config-audit subcommand that does not exist', () => {
// Scope words are arguments to the bare router command, not subcommands.
const SCOPE_WORDS = new Set(['current', 'repo', 'home', 'full']);
const dead = [];
for (const file of readdirSync(COMMANDS_DIR).filter((f) => f.endsWith('.md'))) {
const text = readFileSync(resolve(COMMANDS_DIR, file), 'utf-8');
for (const m of text.matchAll(/\/config-audit ([a-z][a-z-]+)/g)) {
const word = m[1];
if (SCOPE_WORDS.has(word)) continue;
if (!existsSync(resolve(COMMANDS_DIR, `${word}.md`))) dead.push(`${file}: /config-audit ${word}`);
}
}
assert.deepEqual(dead, [], `dead command references: ${dead.join(', ')}`);
});
test('status.md documents how the `all` argument is parsed', () => {
const text = read('status.md');
assert.match(
text,
/ALL_FLAG|contains? `all`|\ball\b[^\n]*\$ARGUMENTS|\$ARGUMENTS[^\n]*\ball\b/i,
'status.md advertises `/config-audit status all` but never parses `all`'
);
});
test('cleanup.md guards its rm -rf against an empty or malformed session id', () => {
const text = read('cleanup.md');
// The destructive line lives in prose, not a bash block, so scan the whole
// file. An earlier version of this test matched the example session ids in
// the display table and went green without any guard existing.
assert.ok(/rm -rf/.test(text), 'cleanup.md is expected to document a deletion step');
assert.match(
text,
/\^\[0-9\]\{8\}_\[0-9\]\{6\}\$|\^\\d\{8\}_\\d\{6\}\$/,
'cleanup.md must state the exact session-id pattern it validates against before deleting'
);
assert.match(
text,
/(refuse|abort|skip)[^\n]*(delet|remov)|never[^\n]*empty[^\n]*(id|path)/i,
'cleanup.md must say what happens when the id fails validation'
);
});
test('no executable bash block passes a bracketed placeholder flag', () => {
// `[--full-machine]` does not start with `-`, so the scanner arg loops take
// it as the scan target and silently scan a path that does not exist.
const offenders = [];
for (const file of readdirSync(COMMANDS_DIR).filter((f) => f.endsWith('.md'))) {
for (const block of bashBlocks(readFileSync(resolve(COMMANDS_DIR, file), 'utf-8'))) {
if (/\[--[a-z-]+\]/.test(block)) offenders.push(file);
}
}
assert.deepEqual([...new Set(offenders)], [], `bracketed flags inside bash blocks: ${offenders.join(', ')}`);
});
test('feature-gap.md does not present a dry-run fix-cli call as a backup', () => {
const text = read('feature-gap.md');
for (const block of bashBlocks(text)) {
if (!/fix-cli\.mjs/.test(block)) continue;
assert.match(
block,
/--apply/,
'feature-gap.md calls fix-cli without --apply; dry-run creates no backup'
);
}
});
test('fix-cli.mjs recovery hint points at a command that exists', () => {
const text = readFileSync(resolve(ROOT, 'scanners', 'fix-cli.mjs'), 'utf-8');
const hints = [...text.matchAll(/scanners\/([a-z-]+\.mjs)/g)].map((m) => m[1]);
for (const hint of hints) {
assert.ok(
existsSync(resolve(ROOT, 'scanners', hint)),
`fix-cli.mjs tells the user to run scanners/${hint}, which does not exist`
);
}
});

View file

@ -46,6 +46,21 @@ describe('drift-cli --list', () => {
const found = output.baselines.find(b => b.name === TEST_BASELINE);
assert.ok(found, 'Should find test baseline in list');
});
// commands/drift.md runs `--list` with `2>/dev/null` (ux-rules rule 2), so a
// stderr-only listing reaches the command as 0 bytes and it can show nothing.
// --output-file was accepted by the arg parser but ignored by list mode.
it('writes the listing to --output-file so the command can read it', () => {
execFileSync('node', [DRIFT_CLI, HEALTHY, '--save', '--name', TEST_BASELINE], RUN);
const outFile = join(mkdtempSync(join(tmpdir(), 'drift-list-')), 'list.json');
execFileSync('node', [DRIFT_CLI, '--list', '--output-file', outFile], RUN);
assert.ok(existsSync(outFile), '--list must honour --output-file');
const payload = JSON.parse(readFileSync(outFile, 'utf-8'));
assert.ok(Array.isArray(payload.baselines));
assert.ok(payload.baselines.find(b => b.name === TEST_BASELINE));
});
});
describe('drift-cli compare', () => {