Dogfooding `/config-audit fix` against a throwaway repo copy. All eight
predictions registered in the fasit before the run were confirmed, and three
further defects surfaced that were not predicted.
- M-BUG-21, third arm: the argument loop ended in `!arg.startsWith('-') =>
targetPath`, so an unknown flag was dropped and its value became the target.
In `fix` that is the WRITE target under `--apply`. Unknown options and a
value-less `--output-file` now exit 3.
- `--dry-run` was documented in the command's argument-hint and never
implemented; `--output-file` did not exist, so `commands/fix.md` told the
agent to Read a file nothing produced. Both now exist.
- M-BUG-31: `file-rename` was excluded from the backup set, so a renamed rule
file had no backup entry while the command promised one and returned a
backupId that could not restore it.
- M-BUG-32: `verifyFixes` hardcoded `includeGlobal: false`, so after a
`--global` run every untouched user-scope finding was reported as verified.
Reproduced against an unmodified ~/.claude/CLAUDE.md.
- M-BUG-29: a rename was applied before other fixes on the same file, which
then failed with ENOENT while the run still exited 0. Renames sort last.
- M-BUG-30: `severityOrder[s] || 4` maps critical (0) to 4, so critical fixes
sorted last. The old test used the same falsy fallback and agreed with the
bug. Now `?? 4`.
- A failed fix exits 2 instead of 0, matching the other scanners' convention.
Frozen tests/snapshots/v5.0.0/ untouched; --json/--raw stdout byte-identical.
Suite 1420/0 (+10).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YJ3MCDCnyw7wZSPnUXVhYS
402 lines
16 KiB
JavaScript
402 lines
16 KiB
JavaScript
import { describe, it, beforeEach, afterEach } from 'node:test';
|
|
import assert from 'node:assert/strict';
|
|
import { resolve, join } from 'node:path';
|
|
import { fileURLToPath } from 'node:url';
|
|
import { cp, rm, readFile, writeFile, stat } from 'node:fs/promises';
|
|
import { mkdirSync } from 'node:fs';
|
|
import { tmpdir } from 'node:os';
|
|
import { resetCounter } from '../../scanners/lib/output.mjs';
|
|
import { discoverConfigFiles } from '../../scanners/lib/file-discovery.mjs';
|
|
import { runAllScanners } from '../../scanners/scan-orchestrator.mjs';
|
|
import { planFixes, applyFixes, verifyFixes, FIX_TYPES } from '../../scanners/fix-engine.mjs';
|
|
import { parseJson, parseFrontmatter } from '../../scanners/lib/yaml-parser.mjs';
|
|
|
|
const __dirname = fileURLToPath(new URL('.', import.meta.url));
|
|
const FIXTURES = resolve(__dirname, '../fixtures');
|
|
const FIXABLE = resolve(FIXTURES, 'fixable-project');
|
|
|
|
/** Create a temporary copy of the fixable-project fixture. */
|
|
async function createTmpCopy() {
|
|
const tmpDir = join(tmpdir(), `config-audit-test-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`);
|
|
mkdirSync(tmpDir, { recursive: true });
|
|
await cp(FIXABLE, tmpDir, { recursive: true });
|
|
return tmpDir;
|
|
}
|
|
|
|
// --- planFixes tests ---
|
|
|
|
describe('planFixes', () => {
|
|
let envelope;
|
|
|
|
beforeEach(async () => {
|
|
resetCounter();
|
|
envelope = await runAllScanners(FIXABLE);
|
|
});
|
|
|
|
it('returns fixes, skipped, and manual arrays', () => {
|
|
const result = planFixes(envelope);
|
|
assert.ok(Array.isArray(result.fixes));
|
|
assert.ok(Array.isArray(result.skipped));
|
|
assert.ok(Array.isArray(result.manual));
|
|
});
|
|
|
|
it('identifies auto-fixable findings', () => {
|
|
const result = planFixes(envelope);
|
|
assert.ok(result.fixes.length > 0, 'Should have at least one fix');
|
|
});
|
|
|
|
it('sorts fixes by severity (critical first), renames last', () => {
|
|
const result = planFixes(envelope);
|
|
// `?? 4`, not `|| 4`: critical weighs 0, and `0 || 4` is 4. The old
|
|
// assertion used the same falsy fallback as the implementation, so it
|
|
// agreed with the bug instead of catching it (M-BUG-30).
|
|
const severityOrder = { critical: 0, high: 1, medium: 2, low: 3, info: 4 };
|
|
// A file-rename is ordered last on purpose (M-BUG-29) — severity ranks
|
|
// within the non-rename group.
|
|
const ranked = result.fixes.filter((f) => f.type !== FIX_TYPES.FILE_RENAME);
|
|
for (let i = 1; i < ranked.length; i++) {
|
|
const prev = severityOrder[ranked[i - 1].severity] ?? 4;
|
|
const curr = severityOrder[ranked[i].severity] ?? 4;
|
|
assert.ok(prev <= curr, `Fix ${i} should not have higher severity than fix ${i - 1}`);
|
|
}
|
|
const renames = result.fixes.filter((f) => f.type === FIX_TYPES.FILE_RENAME);
|
|
if (renames.length > 0) {
|
|
assert.strictEqual(
|
|
result.fixes.at(-1).type,
|
|
FIX_TYPES.FILE_RENAME,
|
|
'Renames must sort after every other fix',
|
|
);
|
|
}
|
|
});
|
|
|
|
it('puts a critical fix first (M-BUG-30)', () => {
|
|
const result = planFixes(envelope);
|
|
const criticals = result.fixes.filter((f) => f.severity === 'critical');
|
|
if (criticals.length === 0) return;
|
|
assert.strictEqual(
|
|
result.fixes[0].severity,
|
|
'critical',
|
|
'critical weighs 0 — a `|| 4` fallback sorted it last, the opposite of the contract',
|
|
);
|
|
});
|
|
|
|
it('includes manual findings with recommendations', () => {
|
|
const result = planFixes(envelope);
|
|
for (const m of result.manual) {
|
|
assert.ok(m.findingId, 'Manual finding should have findingId');
|
|
assert.ok(m.title, 'Manual finding should have title');
|
|
}
|
|
});
|
|
|
|
it('each fix has required fields', () => {
|
|
const result = planFixes(envelope);
|
|
for (const fix of result.fixes) {
|
|
assert.ok(fix.findingId, 'Fix must have findingId');
|
|
assert.ok(fix.file, 'Fix must have file');
|
|
assert.ok(fix.type, 'Fix must have type');
|
|
assert.ok(fix.description, 'Fix must have description');
|
|
}
|
|
});
|
|
|
|
it('detects json-key-add for missing $schema', () => {
|
|
const result = planFixes(envelope);
|
|
const schemaFix = result.fixes.find(f => f.type === FIX_TYPES.JSON_KEY_ADD && f.key === '$schema');
|
|
assert.ok(schemaFix, 'Should have a json-key-add fix for $schema');
|
|
});
|
|
|
|
it('detects json-key-remove for deprecated apiProvider', () => {
|
|
const result = planFixes(envelope);
|
|
// apiProvider is unknown, not deprecated (includeCoAuthoredBy is deprecated)
|
|
// But the fixture has apiProvider which triggers "unknown key" (not auto-fixable)
|
|
// The deprecated key in settings-validator is includeCoAuthoredBy — fixture doesn't have it
|
|
// Let's check for hooks-as-array instead (critical)
|
|
const hooksFix = result.fixes.find(f => f.restructureType === 'hooks-array-to-object');
|
|
assert.ok(hooksFix, 'Should have a json-restructure fix for hooks-as-array');
|
|
});
|
|
|
|
it('detects json-key-type-fix for alwaysThinkingEnabled', () => {
|
|
const result = planFixes(envelope);
|
|
const typeFix = result.fixes.find(f => f.type === FIX_TYPES.JSON_KEY_TYPE_FIX && f.key === 'alwaysThinkingEnabled');
|
|
assert.ok(typeFix, 'Should have a type fix for alwaysThinkingEnabled');
|
|
});
|
|
|
|
it('detects json-key-type-fix for effortLevel', () => {
|
|
const result = planFixes(envelope);
|
|
const effortFix = result.fixes.find(f => f.key === 'effortLevel');
|
|
assert.ok(effortFix, 'Should have a fix for invalid effortLevel');
|
|
});
|
|
|
|
it('detects json-restructure for matcher-as-object', () => {
|
|
const result = planFixes(envelope);
|
|
const matcherFix = result.fixes.find(f => f.restructureType === 'matcher-object-to-string');
|
|
assert.ok(matcherFix, 'Should have a restructure fix for matcher-as-object');
|
|
});
|
|
|
|
it('detects json-key-type-fix for timeout-as-string', () => {
|
|
const result = planFixes(envelope);
|
|
const timeoutFix = result.fixes.find(f => f.key === 'timeout');
|
|
assert.ok(timeoutFix, 'Should have a type fix for timeout');
|
|
});
|
|
|
|
it('detects frontmatter-rename for globs→paths', () => {
|
|
const result = planFixes(envelope);
|
|
const globsFix = result.fixes.find(f => f.type === FIX_TYPES.FRONTMATTER_RENAME);
|
|
assert.ok(globsFix, 'Should have a frontmatter-rename fix for globs');
|
|
});
|
|
|
|
it('detects file-rename for non-.md rules file', () => {
|
|
const result = planFixes(envelope);
|
|
const renameFix = result.fixes.find(f => f.type === FIX_TYPES.FILE_RENAME);
|
|
assert.ok(renameFix, 'Should have a file-rename fix');
|
|
assert.ok(renameFix.newPath.endsWith('.md'), 'New path should end with .md');
|
|
});
|
|
});
|
|
|
|
// --- applyFixes dry-run tests ---
|
|
|
|
describe('applyFixes dry-run', () => {
|
|
let envelope;
|
|
|
|
beforeEach(async () => {
|
|
resetCounter();
|
|
envelope = await runAllScanners(FIXABLE);
|
|
});
|
|
|
|
it('returns dry-run status without modifying files', async () => {
|
|
const { fixes } = planFixes(envelope);
|
|
const result = await applyFixes(fixes, { dryRun: true });
|
|
assert.ok(result.applied.length > 0, 'Should have dry-run results');
|
|
for (const r of result.applied) {
|
|
assert.strictEqual(r.status, 'dry-run');
|
|
}
|
|
assert.strictEqual(result.failed.length, 0, 'No failures in dry-run');
|
|
});
|
|
|
|
it('throws if no backupDir and not dryRun', async () => {
|
|
const { fixes } = planFixes(envelope);
|
|
await assert.rejects(
|
|
() => applyFixes(fixes, { dryRun: false }),
|
|
{ message: /backupDir is required/ },
|
|
);
|
|
});
|
|
});
|
|
|
|
// --- applyFixes actual (on tmp copies) ---
|
|
|
|
describe('applyFixes on tmp copy', () => {
|
|
let tmpDir;
|
|
let envelope;
|
|
|
|
beforeEach(async () => {
|
|
tmpDir = await createTmpCopy();
|
|
resetCounter();
|
|
envelope = await runAllScanners(tmpDir);
|
|
});
|
|
|
|
afterEach(async () => {
|
|
if (tmpDir) await rm(tmpDir, { recursive: true, force: true });
|
|
});
|
|
|
|
it('applies json-key-add ($schema) successfully', async () => {
|
|
const { fixes } = planFixes(envelope);
|
|
const schemaFix = fixes.filter(f => f.type === FIX_TYPES.JSON_KEY_ADD);
|
|
const result = await applyFixes(schemaFix, { dryRun: false, backupDir: tmpDir });
|
|
|
|
assert.ok(result.applied.length > 0, 'Should apply at least one fix');
|
|
assert.strictEqual(result.failed.length, 0, 'No failures');
|
|
|
|
// Verify file has $schema
|
|
const content = await readFile(join(tmpDir, '.claude', 'settings.json'), 'utf-8');
|
|
const parsed = parseJson(content);
|
|
assert.ok(parsed.$schema, 'Should have $schema key');
|
|
assert.ok(parsed.$schema.includes('schemastore'), '$schema should point to schemastore');
|
|
});
|
|
|
|
it('applies json-key-type-fix successfully', async () => {
|
|
const { fixes } = planFixes(envelope);
|
|
const typeFix = fixes.filter(f => f.type === FIX_TYPES.JSON_KEY_TYPE_FIX && f.key === 'alwaysThinkingEnabled');
|
|
const result = await applyFixes(typeFix, { dryRun: false, backupDir: tmpDir });
|
|
|
|
assert.ok(result.applied.length > 0);
|
|
const content = await readFile(join(tmpDir, '.claude', 'settings.json'), 'utf-8');
|
|
const parsed = parseJson(content);
|
|
assert.strictEqual(typeof parsed.alwaysThinkingEnabled, 'boolean', 'Should be boolean now');
|
|
});
|
|
|
|
it('applies json-restructure (hooks array→object) successfully', async () => {
|
|
const { fixes } = planFixes(envelope);
|
|
const hooksFix = fixes.filter(f => f.restructureType === 'hooks-array-to-object');
|
|
const result = await applyFixes(hooksFix, { dryRun: false, backupDir: tmpDir });
|
|
|
|
assert.ok(result.applied.length > 0);
|
|
const content = await readFile(join(tmpDir, '.claude', 'settings.json'), 'utf-8');
|
|
const parsed = parseJson(content);
|
|
assert.ok(!Array.isArray(parsed.hooks), 'hooks should be object now');
|
|
assert.strictEqual(typeof parsed.hooks, 'object');
|
|
});
|
|
|
|
it('applies json-restructure (matcher object→string) successfully', async () => {
|
|
const { fixes } = planFixes(envelope);
|
|
const matcherFix = fixes.filter(f => f.restructureType === 'matcher-object-to-string');
|
|
const result = await applyFixes(matcherFix, { dryRun: false, backupDir: tmpDir });
|
|
|
|
assert.ok(result.applied.length > 0);
|
|
const content = await readFile(join(tmpDir, 'hooks', 'hooks.json'), 'utf-8');
|
|
const parsed = parseJson(content);
|
|
const handler = parsed.hooks.PreToolUse[0];
|
|
assert.strictEqual(typeof handler.matcher, 'string', 'matcher should be string now');
|
|
});
|
|
|
|
it('applies frontmatter-rename (globs→paths) successfully', async () => {
|
|
const { fixes } = planFixes(envelope);
|
|
const fmFix = fixes.filter(f => f.type === FIX_TYPES.FRONTMATTER_RENAME);
|
|
const result = await applyFixes(fmFix, { dryRun: false, backupDir: tmpDir });
|
|
|
|
assert.ok(result.applied.length > 0);
|
|
const content = await readFile(join(tmpDir, '.claude', 'rules', 'typescript.md'), 'utf-8');
|
|
assert.ok(content.includes('paths:'), 'Should have paths: in frontmatter');
|
|
assert.ok(!content.includes('globs:'), 'Should not have globs: in frontmatter');
|
|
});
|
|
|
|
it('applies file-rename (non-.md → .md) successfully', async () => {
|
|
const { fixes } = planFixes(envelope);
|
|
const renameFix = fixes.filter(f => f.type === FIX_TYPES.FILE_RENAME);
|
|
const result = await applyFixes(renameFix, { dryRun: false, backupDir: tmpDir });
|
|
|
|
assert.ok(result.applied.length > 0);
|
|
// Old file should be gone
|
|
await assert.rejects(() => stat(join(tmpDir, '.claude', 'rules', 'readme.txt')));
|
|
// New file should exist
|
|
const newStat = await stat(join(tmpDir, '.claude', 'rules', 'readme.md'));
|
|
assert.ok(newStat.isFile());
|
|
});
|
|
|
|
it('validates JSON output after fix', async () => {
|
|
const { fixes } = planFixes(envelope);
|
|
const jsonFixes = fixes.filter(f => f.file.endsWith('.json'));
|
|
await applyFixes(jsonFixes, { dryRun: false, backupDir: tmpDir });
|
|
|
|
// All JSON files should still parse
|
|
const settingsContent = await readFile(join(tmpDir, '.claude', 'settings.json'), 'utf-8');
|
|
const settingsParsed = parseJson(settingsContent);
|
|
assert.ok(settingsParsed !== null, 'settings.json should be valid JSON after fixes');
|
|
|
|
const hooksContent = await readFile(join(tmpDir, 'hooks', 'hooks.json'), 'utf-8');
|
|
const hooksParsed = parseJson(hooksContent);
|
|
assert.ok(hooksParsed !== null, 'hooks.json should be valid JSON after fixes');
|
|
});
|
|
|
|
it('fails gracefully for missing file', async () => {
|
|
const fakeFix = [{
|
|
findingId: 'CA-SET-999',
|
|
file: join(tmpDir, 'nonexistent.json'),
|
|
type: FIX_TYPES.JSON_KEY_ADD,
|
|
severity: 'info',
|
|
description: 'Add key to missing file',
|
|
key: 'test',
|
|
value: true,
|
|
}];
|
|
const result = await applyFixes(fakeFix, { dryRun: false, backupDir: tmpDir });
|
|
assert.strictEqual(result.failed.length, 1, 'Should have one failure');
|
|
assert.strictEqual(result.applied.length, 0);
|
|
});
|
|
});
|
|
|
|
// --- verifyFixes tests ---
|
|
|
|
describe('verifyFixes', () => {
|
|
let tmpDir;
|
|
|
|
beforeEach(async () => {
|
|
tmpDir = await createTmpCopy();
|
|
});
|
|
|
|
afterEach(async () => {
|
|
if (tmpDir) await rm(tmpDir, { recursive: true, force: true });
|
|
});
|
|
|
|
it('confirms fixed findings are gone', async () => {
|
|
resetCounter();
|
|
const envelope = await runAllScanners(tmpDir);
|
|
const { fixes } = planFixes(envelope);
|
|
|
|
// Apply a subset of fixes
|
|
const fmFix = fixes.filter(f => f.type === FIX_TYPES.FRONTMATTER_RENAME);
|
|
const result = await applyFixes(fmFix, { dryRun: false, backupDir: tmpDir });
|
|
|
|
const verification = await verifyFixes(envelope, result.applied);
|
|
assert.ok(verification.verified.length > 0, 'Should verify at least one fix');
|
|
});
|
|
});
|
|
|
|
// --- v5.14 hardening (økt #45, chunk `fix`) --------------------------------
|
|
|
|
describe('planFixes ordering (M-BUG-29)', () => {
|
|
it('applies a file rename after every other fix touching the same file', async () => {
|
|
const dir = await createTmpCopy();
|
|
const rulesDir = join(dir, '.claude', 'rules');
|
|
mkdirSync(rulesDir, { recursive: true });
|
|
// Both defects at once: undocumented `globs:` AND a non-.md extension.
|
|
await writeFile(join(rulesDir, 'both.txt'), '---\nglobs: "**/*.ts"\n---\n\nBody.\n');
|
|
|
|
resetCounter();
|
|
const envelope = await runAllScanners(dir, { includeGlobal: false });
|
|
const { fixes } = planFixes(envelope);
|
|
|
|
const onFile = fixes.filter((f) => /both\.txt$/.test(f.file));
|
|
assert.strictEqual(onFile.length, 2, 'Both defects on the file must be planned');
|
|
assert.strictEqual(
|
|
onFile[onFile.length - 1].type,
|
|
FIX_TYPES.FILE_RENAME,
|
|
'A rename moves the file out from under later fixes — it must run last',
|
|
);
|
|
|
|
// …and the whole batch must therefore apply cleanly.
|
|
const backupDir = join(dir, '.backup-test');
|
|
mkdirSync(backupDir, { recursive: true });
|
|
const result = await applyFixes(fixes, { dryRun: false, backupDir });
|
|
assert.deepStrictEqual(result.failed, [], 'No fix may fail because of ordering');
|
|
|
|
await rm(dir, { recursive: true, force: true });
|
|
});
|
|
});
|
|
|
|
describe('verifyFixes scope (F5)', () => {
|
|
it('re-scans with the same includeGlobal scope the fix run used', async () => {
|
|
const dir = await createTmpCopy();
|
|
const fakeHome = join(tmpdir(), `ca-verify-home-${Date.now()}`);
|
|
mkdirSync(join(fakeHome, '.claude'), { recursive: true });
|
|
// A user-scope CLAUDE.md long enough to trip the CML line-count finding.
|
|
await writeFile(
|
|
join(fakeHome, '.claude', 'CLAUDE.md'),
|
|
Array.from({ length: 260 }, (_, i) => `Line ${i + 1}`).join('\n') + '\n',
|
|
);
|
|
|
|
const originalHome = process.env.HOME;
|
|
process.env.HOME = fakeHome;
|
|
try {
|
|
resetCounter();
|
|
const envelope = await runAllScanners(dir, { includeGlobal: true });
|
|
const globalOnly = envelope.scanners
|
|
.flatMap((s) => s.findings)
|
|
.filter((f) => f.file && !f.file.startsWith(dir));
|
|
assert.ok(globalOnly.length > 0, 'Fixture must produce at least one global-scope finding');
|
|
|
|
const victim = globalOnly[0];
|
|
// Nothing was changed on disk — claiming it was applied must NOT verify it.
|
|
const res = await verifyFixes(
|
|
envelope,
|
|
[{ findingId: victim.id, file: victim.file, status: 'applied' }],
|
|
{ includeGlobal: true },
|
|
);
|
|
assert.ok(
|
|
!res.verified.includes(victim.id),
|
|
'An untouched global-scope finding must not be reported as verified',
|
|
);
|
|
} finally {
|
|
if (originalHome === undefined) delete process.env.HOME; else process.env.HOME = originalHome;
|
|
await rm(fakeHome, { recursive: true, force: true });
|
|
await rm(dir, { recursive: true, force: true });
|
|
}
|
|
});
|
|
});
|