fix(acr): feature-gap scopes presence checks to authored config + reads settings cascade (M-BUG-13)
The GAP scanner's 25 presence checks ran over the full includeGlobal discovery, so this plugin's own examples/optimal-setup (vendored across plugin-cache versions) satisfied every tier-3 check — masking real feature gaps to GAP=0 on ANY target. And the real ~/.claude/settings.json is invisible to the settings-key checks (includeGlobal gotcha + maxFiles cap), which would flip statusLine/autoMode to false positives once the maskers were removed. - isAuthoredConfig: exclude plugin-bundled (~/.claude/plugins/) + nested examples/ and tests/fixtures/ (relPath-relative, so a fixture scanned AS the target keeps its own files) from ctx.files + parsedSettings. - readSettingsCascade: read the user->project->local settings cascade directly and merge into parsedSettings — immune to the discovery cap/gotcha. Empty target: ~0 (masked) -> 18 humanized opportunities; no statusLine/autoMode false positives. Frozen v5.0.0 snapshots + SC-5/6/7 byte-stable (marketplace-medium has no nested demo trees; hermetic-HOME cascade adds nothing). Suite 1350->1355/0. Found by dogfooding feature-gap against the machine. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01683eAqVecv9VZfQzL8CQ9h
This commit is contained in:
parent
b58393099a
commit
f4bf3ae2cb
3 changed files with 207 additions and 5 deletions
|
|
@ -254,6 +254,39 @@ model-switch is a *runtime* behaviour, not static config a scanner can reliably
|
|||
automation. "No overstated behavioral finding ships" — so even the permitted opusplan *info*-advisory was
|
||||
left out; the verified @import extension is the whole of B6.
|
||||
|
||||
### GAP scanner — authored-config scoping + direct cascade read (M-BUG-13)
|
||||
|
||||
The 25 presence checks ask "does the user's effective config have feature X?" and GAP **always**
|
||||
runs `includeGlobal: true`. Two failure modes made the answer wrong on a real machine, both surfaced
|
||||
by dogfooding `feature-gap`/`posture --global`:
|
||||
|
||||
1. **Demo/vendored config masks real gaps.** This plugin's own `examples/optimal-setup/` is a complete
|
||||
config (sets `outputStyle`/`statusLine`/`worktree`/`model`/`keybindings.json`/`.lsp.json`), and its
|
||||
copies vendored under `~/.claude/plugins/cache/.../config-audit/<ver>/examples/` are pulled into the
|
||||
includeGlobal discovery. Because `anySettingsHas`/`files.some(...)` accept ANY discovered file, that
|
||||
one demo file drove every tier-3 check to "present" → **GAP=0 on any target** (false negative).
|
||||
Fix: `isAuthoredConfig` filters `ctx.files`/`parsedSettings` to the user's authored cascade —
|
||||
excludes `~/.claude/plugins/` (absPath marker, mirrors CNF's M-BUG-2 exclusion) and any file whose
|
||||
path **relative to the scan target** sits under `examples/` or `tests/fixtures/`. relPath (not
|
||||
absPath) is deliberate: a fixture scanned AS the target keeps its own files, so the frozen v5.0.0
|
||||
snapshots (scanned from `tests/fixtures/marketplace-medium`, which has no such nested trees) are
|
||||
byte-stable.
|
||||
|
||||
2. **The real `~/.claude/settings.json` is invisible to the settings-key checks.** Discovery misses it
|
||||
(its relPath carries no `.claude` segment when the walk root IS `~/.claude` — the gotcha) AND, when
|
||||
vendored plugins flood the walk, the `maxFiles=2000` cap drops it. After (1) removed the demo
|
||||
maskers, `statusLine`/`autoMode`/`permissions` (which the user HAS) would flip to false **positives**.
|
||||
Fix: `readSettingsCascade` reads the four canonical cascade paths (user `settings.json`/`.local`,
|
||||
project `settings.json`/`.local`) directly and merges them INTO `parsedSettings` — immune to the
|
||||
cap and the gotcha. Merge (not replace) keeps non-canonical project settings and leaves the snapshot
|
||||
(hermetic empty HOME → cascade adds nothing new) byte-stable.
|
||||
|
||||
Net: an empty target now surfaces ~18 humanized opportunities (was masked to ~0); config-audit's own
|
||||
repo still shows 0 in output via its intentional `.config-audit-ignore` `CA-GAP-*` self-suppression
|
||||
(a plugin repo legitimately lacks user-project features) — suppression is an envelope-layer concern,
|
||||
orthogonal to this scanner fix. Scoped GAP-local; the includeGlobal discovery gotcha itself is left
|
||||
to other consumers (see auto-memory `discovery-includeglobal-user-settings-gotcha`).
|
||||
|
||||
### CML scanner — context-window-scaled char budget
|
||||
|
||||
Beyond the line-count checks (200/500 lines, both MEDIUM), the CML scanner mirrors
|
||||
|
|
|
|||
|
|
@ -7,7 +7,7 @@
|
|||
* Finding IDs: CA-GAP-NNN
|
||||
*/
|
||||
|
||||
import { resolve } from 'node:path';
|
||||
import { resolve, join, sep } from 'node:path';
|
||||
import { readTextFile, discoverConfigFiles } from './lib/file-discovery.mjs';
|
||||
import { finding, scannerResult } from './lib/output.mjs';
|
||||
import { SEVERITY } from './lib/severity.mjs';
|
||||
|
|
@ -46,6 +46,68 @@ function isTargetLocal(ctx, f) {
|
|||
return f.absPath.startsWith(ctx.targetPath);
|
||||
}
|
||||
|
||||
// Files that are test/demo/vendored config — NOT part of the user's authored
|
||||
// cascade — must not satisfy "is feature X present?" checks, or they mask real
|
||||
// gaps. The canonical case: this plugin's own examples/optimal-setup sets
|
||||
// outputStyle/statusLine/worktree/model/keybindings/.lsp.json, and (because GAP
|
||||
// always runs includeGlobal) its copies vendored under ~/.claude/plugins/cache
|
||||
// drive every tier-3 presence check to "present" — hiding the user's real gaps
|
||||
// on ANY target. Two classes to exclude:
|
||||
// - plugin-bundled: anything under ~/.claude/plugins/ (absPath marker, mirrors
|
||||
// the CNF conflict-detector exclusion from M-BUG-2).
|
||||
// - nested demo/test data: a file whose path RELATIVE TO THE SCAN TARGET sits
|
||||
// under an examples/ or tests/fixtures/ subtree. relPath (not absPath) is
|
||||
// deliberate: a fixture scanned AS the target keeps its own files, so the
|
||||
// frozen v5.0.0 byte-snapshots (scanned from tests/fixtures/marketplace-medium)
|
||||
// are untouched. (M-BUG-13)
|
||||
const PLUGIN_TREE_MARKER = `.claude${sep}plugins${sep}`;
|
||||
|
||||
/**
|
||||
* @param {import('./lib/file-discovery.mjs').ConfigFile} file
|
||||
* @returns {boolean} true if the file is part of the user's authored config
|
||||
*/
|
||||
function isAuthoredConfig(file) {
|
||||
if (file.absPath.includes(PLUGIN_TREE_MARKER)) return false;
|
||||
const segs = (file.relPath || '').split(sep);
|
||||
if (segs.includes('examples')) return false;
|
||||
const ti = segs.indexOf('tests');
|
||||
if (ti !== -1 && segs[ti + 1] === 'fixtures') return false;
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Read the user→project→local settings cascade directly from the filesystem.
|
||||
* The settings-key gap checks ask "does the USER's resolved config set X?" — a
|
||||
* question the includeGlobal discovery answers unreliably on a real machine: the
|
||||
* top-level ~/.claude/settings.json is missed (its relPath carries no `.claude`
|
||||
* segment when the walk root IS ~/.claude) and, when many vendored plugins flood
|
||||
* the walk, dropped by the discovery file cap. Reading the canonical cascade
|
||||
* paths directly is immune to both. Merged INTO (not replacing) the discovery
|
||||
* settings so any non-canonical project settings still count and the frozen
|
||||
* snapshots stay byte-stable. (M-BUG-13)
|
||||
* @param {string} targetPath
|
||||
* @returns {Promise<Array<{ key: string, parsed: object }>>}
|
||||
*/
|
||||
async function readSettingsCascade(targetPath) {
|
||||
const home = process.env.HOME || process.env.USERPROFILE || '';
|
||||
const paths = [];
|
||||
if (home) {
|
||||
paths.push(['user', join(home, '.claude', 'settings.json')]);
|
||||
paths.push(['user-local', join(home, '.claude', 'settings.local.json')]);
|
||||
}
|
||||
paths.push(['project', join(targetPath, '.claude', 'settings.json')]);
|
||||
paths.push(['local', join(targetPath, '.claude', 'settings.local.json')]);
|
||||
|
||||
const out = [];
|
||||
for (const [scope, p] of paths) {
|
||||
const content = await readTextFile(p);
|
||||
if (!content) continue;
|
||||
const parsed = parseJson(content);
|
||||
if (parsed && typeof parsed === 'object') out.push({ key: `cascade:${scope}:${p}`, parsed });
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
const TIER_SEVERITY = {
|
||||
t1: SEVERITY.medium,
|
||||
t2: SEVERITY.low,
|
||||
|
|
@ -479,18 +541,30 @@ export async function scan(targetPath, sharedDiscovery) {
|
|||
? sharedDiscovery
|
||||
: await discoverConfigFiles(resolve(targetPath), { includeGlobal: true });
|
||||
|
||||
// Parse all settings files upfront
|
||||
// Presence checks ("does the user have feature X?") must see only the user's
|
||||
// authored cascade — not bundled/vendored/demo config, which masks real gaps
|
||||
// (M-BUG-13, see isAuthoredConfig).
|
||||
const authoredFiles = discovery.files.filter(isAuthoredConfig);
|
||||
|
||||
// Parse all settings files upfront (authored discovery files) ...
|
||||
const parsedSettings = new Map();
|
||||
for (const file of discovery.files.filter(f => f.type === 'settings-json')) {
|
||||
for (const file of authoredFiles.filter(f => f.type === 'settings-json')) {
|
||||
const content = await readTextFile(file.absPath);
|
||||
if (content) {
|
||||
const parsed = parseJson(content);
|
||||
parsedSettings.set(`${file.scope}:${file.relPath}`, parsed);
|
||||
}
|
||||
}
|
||||
// ... plus the real user→project→local cascade read directly, so settings-key
|
||||
// checks see the true resolved config regardless of the discovery cap/gotcha
|
||||
// (M-BUG-13). Merged, not replacing — keeps non-canonical project settings and
|
||||
// the frozen byte-snapshots unchanged.
|
||||
for (const { key, parsed } of await readSettingsCascade(resolve(targetPath))) {
|
||||
parsedSettings.set(key, parsed);
|
||||
}
|
||||
|
||||
const ctx = {
|
||||
files: discovery.files,
|
||||
files: authoredFiles,
|
||||
targetPath: resolve(targetPath),
|
||||
parsedSettings,
|
||||
fileContents: new Map(),
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
import { describe, it, beforeEach } from 'node:test';
|
||||
import assert from 'node:assert/strict';
|
||||
import { resolve, join } from 'node:path';
|
||||
import { resolve, join, dirname } from 'node:path';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
import { mkdtemp, mkdir, writeFile, rm } from 'node:fs/promises';
|
||||
import { tmpdir } from 'node:os';
|
||||
|
|
@ -411,3 +411,98 @@ describe('GAP scanner — filter-before lever wiring (chatty hook fixture)', ()
|
|||
assert.equal(lever, undefined, `expected no filter-before lever; got id=${lever?.id}`);
|
||||
});
|
||||
});
|
||||
|
||||
describe('GAP scanner — test/demo data must not mask real gaps (M-BUG-13)', () => {
|
||||
// Build a throwaway project (+ optional hermetic HOME settings) and run GAP
|
||||
// exactly as posture --global does: includeGlobal discovery + scan().
|
||||
async function runGap({ projectFiles = {}, homeSettings = null } = {}) {
|
||||
const project = await mkdtemp(join(tmpdir(), 'config-audit-gap-mask-proj-'));
|
||||
const home = await mkdtemp(join(tmpdir(), 'config-audit-gap-mask-home-'));
|
||||
for (const [rel, content] of Object.entries(projectFiles)) {
|
||||
const abs = join(project, rel);
|
||||
await mkdir(dirname(abs), { recursive: true });
|
||||
await writeFile(abs, content);
|
||||
}
|
||||
if (homeSettings) {
|
||||
await mkdir(join(home, '.claude'), { recursive: true });
|
||||
await writeFile(join(home, '.claude', 'settings.json'), JSON.stringify(homeSettings));
|
||||
}
|
||||
const original = process.env.HOME;
|
||||
process.env.HOME = home;
|
||||
try {
|
||||
resetCounter();
|
||||
const discovery = await discoverConfigFiles(project, { includeGlobal: true });
|
||||
const result = await scan(project, discovery);
|
||||
return result;
|
||||
} finally {
|
||||
process.env.HOME = original;
|
||||
await rm(project, { recursive: true, force: true });
|
||||
await rm(home, { recursive: true, force: true });
|
||||
}
|
||||
}
|
||||
|
||||
const hasGap = (result, titleRe) =>
|
||||
result.findings.some(f => f.scanner === 'GAP' && titleRe.test(f.title || ''));
|
||||
|
||||
it('a nested examples/ settings.json does NOT satisfy the outputStyle check (settings-key)', async () => {
|
||||
const result = await runGap({
|
||||
projectFiles: {
|
||||
'.claude/CLAUDE.md': '# proj',
|
||||
'examples/optimal-setup/.claude/settings.json': JSON.stringify({ outputStyle: 'Explanatory' }),
|
||||
},
|
||||
});
|
||||
assert.ok(
|
||||
hasGap(result, /output style/i),
|
||||
`example settings.json must not mask the outputStyle gap; got: ${result.findings.map(f => f.title).join(' | ')}`,
|
||||
);
|
||||
});
|
||||
|
||||
it('a nested examples/ keybindings.json does NOT satisfy the keybindings check (file-type)', async () => {
|
||||
const result = await runGap({
|
||||
projectFiles: {
|
||||
'.claude/CLAUDE.md': '# proj',
|
||||
'examples/optimal-setup/.claude/keybindings.json': JSON.stringify({}),
|
||||
},
|
||||
});
|
||||
assert.ok(
|
||||
hasGap(result, /keybinding/i),
|
||||
`example keybindings.json must not mask the keybindings gap; got: ${result.findings.map(f => f.title).join(' | ')}`,
|
||||
);
|
||||
});
|
||||
|
||||
it('a nested tests/fixtures/ settings.json does NOT satisfy the model check', async () => {
|
||||
const result = await runGap({
|
||||
projectFiles: {
|
||||
'.claude/CLAUDE.md': '# proj',
|
||||
'tests/fixtures/demo/.claude/settings.json': JSON.stringify({ model: 'opus' }),
|
||||
},
|
||||
});
|
||||
assert.ok(
|
||||
hasGap(result, /model config/i),
|
||||
`fixture settings.json must not mask the model gap; got: ${result.findings.map(f => f.title).join(' | ')}`,
|
||||
);
|
||||
});
|
||||
|
||||
it("the project's OWN real .claude/settings.json IS counted (not over-excluded)", async () => {
|
||||
const result = await runGap({
|
||||
projectFiles: {
|
||||
'.claude/CLAUDE.md': '# proj',
|
||||
'.claude/settings.json': JSON.stringify({ outputStyle: 'Explanatory' }),
|
||||
},
|
||||
});
|
||||
assert.ok(
|
||||
!hasGap(result, /output style/i),
|
||||
`real project settings.json must satisfy outputStyle; got: ${result.findings.map(f => f.title).join(' | ')}`,
|
||||
);
|
||||
});
|
||||
|
||||
it('the real ~/.claude/settings.json IS seen for settings-key checks (gotcha fix end-to-end)', async () => {
|
||||
const result = await runGap({
|
||||
homeSettings: { statusLine: { type: 'command', command: 'x' } },
|
||||
});
|
||||
assert.ok(
|
||||
!hasGap(result, /status line/i),
|
||||
`~/.claude/settings.json statusLine must be discovered; got: ${result.findings.map(f => f.title).join(' | ')}`,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue