From f4bf3ae2cb77745ecae8631ce09b03c28f207345 Mon Sep 17 00:00:00 2001 From: Kjell Tore Guttormsen Date: Tue, 30 Jun 2026 10:09:10 +0200 Subject: [PATCH] fix(acr): feature-gap scopes presence checks to authored config + reads settings cascade (M-BUG-13) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) Claude-Session: https://claude.ai/code/session_01683eAqVecv9VZfQzL8CQ9h --- docs/scanner-internals.md | 33 +++++++ scanners/feature-gap-scanner.mjs | 82 ++++++++++++++++- tests/scanners/feature-gap-scanner.test.mjs | 97 ++++++++++++++++++++- 3 files changed, 207 insertions(+), 5 deletions(-) diff --git a/docs/scanner-internals.md b/docs/scanner-internals.md index befa00d..a59e1f6 100644 --- a/docs/scanner-internals.md +++ b/docs/scanner-internals.md @@ -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//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 diff --git a/scanners/feature-gap-scanner.mjs b/scanners/feature-gap-scanner.mjs index 00a5c23..0a68c6f 100644 --- a/scanners/feature-gap-scanner.mjs +++ b/scanners/feature-gap-scanner.mjs @@ -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>} + */ +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(), diff --git a/tests/scanners/feature-gap-scanner.test.mjs b/tests/scanners/feature-gap-scanner.test.mjs index ed9cf45..c31730a 100644 --- a/tests/scanners/feature-gap-scanner.test.mjs +++ b/tests/scanners/feature-gap-scanner.test.mjs @@ -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(' | ')}`, + ); + }); +});