diff --git a/.impeccable/config.json b/.impeccable/config.json index 45ca0e911..b2df55f0d 100644 --- a/.impeccable/config.json +++ b/.impeccable/config.json @@ -71,6 +71,15 @@ ], "createdAt": "2026-06-15T23:37:38.170Z", "reason": "Generic slop card intentionally uses Inter for the before-state comparison" + }, + { + "rule": "design-system-font-size", + "value": "*", + "files": [ + "skill/scripts/live-browser.js" + ], + "createdAt": "2026-07-17T00:00:00.000Z", + "reason": "Live overlay chrome is injected over arbitrary host pages and builds a self-contained UI with its own small type scale; DESIGN.md's ramp describes the impeccable website, not this widget" } ] }, diff --git a/cli/bin/commands/ignores.mjs b/cli/bin/commands/ignores.mjs index 9eb4696ed..dc1d6a376 100644 --- a/cli/bin/commands/ignores.mjs +++ b/cli/bin/commands/ignores.mjs @@ -78,6 +78,20 @@ function parseScope(args, { allowAll = false } = {}) { return { local, all, rest }; } +// An empty glob used to be dropped by filter(Boolean), so `--file=` reported +// success and wrote an entry with no files: the user asked to scope a rule to one +// file and silently got the project-wide suppression instead. Refuse it. +function requireGlob(raw, flag) { + const glob = String(raw ?? '').trim(); + if (!glob) throw new Error(`${flag} requires a non-empty glob`); + // A following flag is not a glob. `--file --reason "why"` consumed `--reason` + // as the scope and left the reason text to fold into the value, storing + // value="* why" files=["--reason"] and reporting success. Same silent-no-op + // class as an unknown flag folding into the value; refuse it the same way. + if (glob.startsWith('--')) throw new Error(`${flag} requires a glob, got the flag ${glob}`); + return glob; +} + function parseValueArgs(args, { allowUnscopedWildcard = false } = {}) { const positionals = []; const files = []; @@ -93,11 +107,11 @@ function parseValueArgs(args, { allowUnscopedWildcard = false } = {}) { reason = arg.slice('--reason='.length).trim(); } else if (arg === '--file' || arg === '--files') { if (i + 1 >= args.length) throw new Error(`${arg} requires a glob`); - files.push(String(args[++i]).trim()); + files.push(requireGlob(args[++i], arg)); } else if (arg.startsWith('--file=')) { - files.push(arg.slice('--file='.length).trim()); + files.push(requireGlob(arg.slice('--file='.length), '--file')); } else if (arg.startsWith('--files=')) { - files.push(arg.slice('--files='.length).trim()); + files.push(requireGlob(arg.slice('--files='.length), '--files')); } else if (arg.startsWith('--')) { throw new Error(`Unknown add-value flag: ${arg}`); } else { @@ -108,7 +122,9 @@ function parseValueArgs(args, { allowUnscopedWildcard = false } = {}) { const [rule, ...valueParts] = positionals; const value = normalizeIgnoreValue(valueParts.join(' ')); if (!rule || !value) throw new Error('Pass a rule id and value, e.g. impeccable ignores add-value overused-font Inter'); - const scopedFiles = Array.from(new Set(files.filter(Boolean))); + // Sorted: the dedup key compares the files array, so an unsorted scope made + // `--file b.css --file a.css` a different entry from `--file a.css --file b.css`. + const scopedFiles = Array.from(new Set(files.filter(Boolean))).sort(); if (value === '*' && scopedFiles.length === 0 && !allowUnscopedWildcard) { throw new Error('Wildcard value ignores must be scoped with --file .'); } @@ -226,12 +242,14 @@ function addValue(cwd, args) { if (parsed.reason) existing.reason = parsed.reason; if (parsed.files.length) existing.files = parsed.files; } else { + // rule, value, files, createdAt, reason — the same order the normalizers emit, + // so a fresh entry survives the next write untouched. const entry = { rule: parsed.rule, value: parsed.value, - createdAt: new Date().toISOString(), }; if (parsed.files.length) entry.files = parsed.files; + entry.createdAt = new Date().toISOString(); if (parsed.reason) entry.reason = parsed.reason; config.ignoreValues.push(entry); } @@ -299,7 +317,10 @@ function clear(cwd, args) { } function ignoreValueKey(entry) { - const files = Array.isArray(entry.files) && entry.files.length ? entry.files.join('\x1f') : ''; + // Sorted: a file scope is a set. Comparing stored order made an on-disk scope + // miss the sorted argv form, so a re-add duplicated the entry and a remove + // silently failed. Every key that hashes `files` must sort — there are four. + const files = Array.isArray(entry.files) && entry.files.length ? [...entry.files].sort().join('\x1f') : ''; return `${String(entry.rule || '').trim().toLowerCase()}\0${normalizeIgnoreValue(entry.value)}\0${files}`; } diff --git a/cli/lib/impeccable-config.mjs b/cli/lib/impeccable-config.mjs index 45c65e65a..a62de8e3c 100644 --- a/cli/lib/impeccable-config.mjs +++ b/cli/lib/impeccable-config.mjs @@ -346,12 +346,16 @@ export function normalizeIgnoreValueEntries(entries) { ...(Array.isArray(entry.files) ? entry.files.filter(v => typeof v === 'string' && v.trim()).map(v => v.trim()) : []), ]); if (files.length > 0) normalized.files = files; - if (typeof entry.reason === 'string' && entry.reason.trim()) { - normalized.reason = entry.reason.trim(); - } + // Key order is rule, value, files, createdAt, reason and must stay that way: + // normalizing runs on every write, so emitting a different order than the one + // already on disk rewrites every untouched entry and churns the diff. Keep in + // step with normalizeIgnoreValueEntries in skill/scripts/hook-lib.mjs. if (typeof entry.createdAt === 'string' && entry.createdAt.trim()) { normalized.createdAt = entry.createdAt.trim(); } + if (typeof entry.reason === 'string' && entry.reason.trim()) { + normalized.reason = entry.reason.trim(); + } out.push(normalized); } return out; @@ -369,7 +373,9 @@ function mergeIgnoreValues(existing, incoming) { } function ignoreValueFilesKey(files) { - return Array.isArray(files) && files.length > 0 ? files.join('\x1f') : ''; + // Sort before joining: a scope is a set, so an entry already on disk in another + // order must compare equal rather than dedup as two distinct entries. + return Array.isArray(files) && files.length > 0 ? [...files].sort().join('\x1f') : ''; } // Glob -> RegExp. Supports `**`, `*`, `?`, and `{a,b}` alternation. diff --git a/skill/reference/hooks.md b/skill/reference/hooks.md index b7f6033c3..606e49150 100644 --- a/skill/reference/hooks.md +++ b/skill/reference/hooks.md @@ -23,10 +23,11 @@ The first argument is the action. Defaults to `status`. | `status` | Print current state, shared/local config paths, ignored rules / files / values, env override. | | `on` | Set `enabled: true` in `.impeccable/config.json`, record local hook consent as accepted, and install/repair provider hook manifests when the skill is installed. | | `off` | Set `enabled: false` in `.impeccable/config.json`. | -| `ignore-rule ` | Append `` to `detector.ignoreRules`; for `overused-font`, requires `--all-values`. | -| `ignore-file ` | Append `` to `detector.ignoreFiles`. | +| `ignore-rule ` | Append `` to `detector.ignoreRules`; for `overused-font`, requires `--all-values`. Suppresses the rule across the whole project. | +| `ignore-file ` | Append `` to `detector.ignoreFiles`. Suppresses **every** rule for matching files. | | `ignore-value [--shared] [--reason "..."]` | Append a rule/value suppression to shared `.impeccable/config.json`. | | `ignore-value --local [--reason "..."]` | Append a private rule/value suppression to `.impeccable/config.local.json`. | +| `ignore-value "*" --file [--file ...]` | Turn one rule off in matching files only, leaving it active everywhere else. Repeat `--file`, or use `--file=` / `--files=`. A bare `"*"` with no `--file` is refused: use `ignore-rule ` if you really mean project-wide. | | `reset` | Delete the project config, dedup cache, and Cursor pending queue. | ## Flow @@ -51,7 +52,8 @@ Prefer the narrowest exception: - If the finding line shows an exact `ignore-value` command, run that command. This writes shared `.impeccable/config.json` by default. - For value-specific findings such as `overused-font` and `bounce-easing`, use `ignore-value` when the user confirms the specific value. Do not use `ignore-rule overused-font` for a specific font. -- If the finding has no value-specific command, such as `side-tab`, prefer `ignore-file ` for the current file. +- If the finding has no value-specific command, such as `side-tab`, scope that one rule to the file: `ignore-value "*" --file `. Run `npx impeccable detect ` first to see what actually fires there. +- Reach for `ignore-file ` only when the whole file is out of scope for design review: a fixture, a generated artifact, a deliberate slop demo. It silences every rule for that file permanently, including rules that have not been written yet. A real UI surface with one noisy rule wants the file-scoped value ignore above. - Use `ignore-rule ` only when the user asks to suppress that whole rule across the project. For broad overused-font suppression, use `ignore-rule overused-font --all-values` only when the user asks to ignore overused fonts generally. - Prefer config ignores (the commands above) by default; they keep suppressions in one reviewable place. Reach for an inline comment only when the waiver must travel with a single file that leaves the repo (a generated/exported standalone document, an emailed HTML file). The supported marker is `impeccable-disable ` (whole file) or `impeccable-disable-line` / `impeccable-disable-next-line` (one line), in any comment syntax, with an optional reason after `:` or `--`. The detector honors it by default; `--no-inline-ignores` or `--no-config` bypasses it. @@ -73,7 +75,14 @@ Example whole-rule font exception: node {{scripts_path}}/hook-admin.mjs ignore-rule overused-font --all-values --reason "User asked to ignore overused fonts generally" ``` -Example file-scoped exception: +Example one-rule-in-one-file exception, for a file that is still worth reviewing +for everything else: + +```bash +node {{scripts_path}}/hook-admin.mjs ignore-value design-system-font-size "*" --file "src/overlay/widget.js" --reason "Injected widget builds its own type scale; DESIGN.md's ramp describes the site" +``` + +Example whole-file exception, for a file that is out of scope entirely: ```bash node {{scripts_path}}/hook-admin.mjs ignore-file "src/legacy/Card.tsx" diff --git a/skill/scripts/hook-admin.mjs b/skill/scripts/hook-admin.mjs index 8b37b1f3b..12cf004b9 100644 --- a/skill/scripts/hook-admin.mjs +++ b/skill/scripts/hook-admin.mjs @@ -13,6 +13,8 @@ * node hook-admin.mjs ignore-file # append to ignoreFiles * node hook-admin.mjs ignore-value # append to shared ignoreValues * node hook-admin.mjs ignore-value --local + * node hook-admin.mjs ignore-value "*" --file # rule off in only + * node hook-admin.mjs ignore-value "*" # refused: scope it or use ignore-rule * node hook-admin.mjs reset # remove all config + cache * * Designed to be invoked by the LLM from the reference/hooks.md flow. @@ -265,7 +267,10 @@ function mergeIgnoreValueEntries(existing, incoming) { } function ignoreValueEntryKey(entry) { - const files = Array.isArray(entry.files) && entry.files.length > 0 ? entry.files.join('\x1f') : ''; + // Sorted: a file scope is a set. Comparing stored order made an on-disk scope + // miss the sorted argv form, so a re-add duplicated the entry and a remove + // silently failed. Every key that hashes `files` must sort — there are four. + const files = Array.isArray(entry.files) && entry.files.length > 0 ? [...entry.files].sort().join('\x1f') : ''; return `${entry.rule}\0${entry.value}\0${files}`; } @@ -283,7 +288,14 @@ function statusReport(cwd) { if (info.exists) return relPath; return `${relPath} (${absent})`; }; - const ignoreValues = cfg.ignoreValues.map((entry) => `${entry.rule}=${entry.value}`); + // Show the file scope. Dropping it rendered a file-scoped entry as + // `design-system-font-size=*`, which reads as the project-wide wildcard this + // command refuses — the opposite of what is on disk. Matches the + // `rule=value [files]` shape `impeccable ignores list` already prints. + const ignoreValues = cfg.ignoreValues.map((entry) => { + const scope = Array.isArray(entry.files) && entry.files.length ? ` [${entry.files.join(', ')}]` : ''; + return `${entry.rule}=${entry.value}${scope}`; + }); const lines = [ `Impeccable design hook`, @@ -532,14 +544,29 @@ function addIgnoreFile(cwd, glob) { return `Added "${glob}" to detector.ignoreFiles. Current: ${config.ignoreFiles.join(', ')}`; } +// An empty glob used to be dropped by filter(Boolean), so `--file=` reported +// success and wrote an entry with no files: the user asked to scope a rule to one +// file and silently got the project-wide suppression instead. Refuse it. +function requireGlob(raw, flag) { + const glob = String(raw ?? '').trim(); + if (!glob) throw new Error(`${flag} requires a non-empty glob`); + // A following flag is not a glob. `--file --reason "why"` consumed `--reason` + // as the scope and left the reason text to fold into the value, storing + // value="* why" files=["--reason"] and reporting success. Same silent-no-op + // class as an unknown flag folding into the value; refuse it the same way. + if (glob.startsWith('--')) throw new Error(`${flag} requires a glob, got the flag ${glob}`); + return glob; +} + function parseIgnoreValueArgs(args) { const positionals = []; + const files = []; let shared = false; let local = false; let reason = ''; for (let i = 0; i < args.length; i++) { - const arg = args[i]; + const arg = String(args[i] || ''); if (arg === '--shared') { shared = true; } else if (arg === '--local') { @@ -550,8 +577,20 @@ function parseIgnoreValueArgs(args) { chunks.push(args[++i]); } reason = chunks.join(' ').trim(); - } else if (String(arg).startsWith('--reason=')) { - reason = String(arg).slice('--reason='.length).trim(); + } else if (arg.startsWith('--reason=')) { + reason = arg.slice('--reason='.length).trim(); + } else if (arg === '--file' || arg === '--files') { + if (i + 1 >= args.length) throw new Error(`${arg} requires a glob`); + files.push(requireGlob(args[++i], arg)); + } else if (arg.startsWith('--file=')) { + files.push(requireGlob(arg.slice('--file='.length), '--file')); + } else if (arg.startsWith('--files=')) { + files.push(requireGlob(arg.slice('--files='.length), '--files')); + } else if (arg.startsWith('--')) { + // Otherwise a typo folds into the value: `ignore-value overused-font Inter + // --shard` stored the value "inter --shard", which matches no finding, and + // reported success. Matches `impeccable ignores add-value`. + throw new Error(`Unknown ignore-value flag: ${arg}`); } else { positionals.push(arg); } @@ -561,6 +600,9 @@ function parseIgnoreValueArgs(args) { return { rule: String(rule || '').trim().toLowerCase(), value: normalizeIgnoreValue(valueParts.join(' ')), + // Sorted: the dedup key compares the files array, so an unsorted scope made + // `--file b.css --file a.css` a different entry from `--file a.css --file b.css`. + files: Array.from(new Set(files.filter(Boolean))).sort(), shared, local, reason, @@ -577,10 +619,24 @@ function addIgnoreValue(cwd, args) { throw new Error('Pass only one scope flag: --shared or --local'); } + // A bare `*` would suppress the rule everywhere, which is ignore-rule's job and + // not what a finding in one file justifies. detector.ignoreValues honours a + // `files` scope, so require one — matching `impeccable ignores add-value`. + if (parsed.value === '*' && parsed.files.length === 0) { + // `ignore-rule overused-font` refuses on its own without --all-values, so + // naming the bare form here would hand the user a second error. + const projectWide = parsed.rule === 'overused-font' + ? `${IMPECCABLE_COMMAND} hooks ignore-rule ${parsed.rule} --all-values` + : `${IMPECCABLE_COMMAND} hooks ignore-rule ${parsed.rule}`; + throw new Error(`Wildcard value ignores must be scoped with --file , e.g. ${IMPECCABLE_COMMAND} hooks ignore-value design-system-font-size "*" --file "src/widget.js". To suppress the rule project-wide use ${projectWide}.`); + } + const local = parsed.local; const config = mergeDetectorConfig(readRawDetectorConfig(cwd, { local })); - const key = `${parsed.rule}\0${parsed.value}`; - const existing = config.ignoreValues.find((entry) => `${entry.rule}\0${entry.value}` === key); + // Key on the file scope too: the same rule/value legitimately appears more than + // once with different scopes, and a rule+value-only key overwrote them. + const key = ignoreValueEntryKey({ rule: parsed.rule, value: parsed.value, files: parsed.files }); + const existing = config.ignoreValues.find((entry) => ignoreValueEntryKey(entry) === key); if (existing) { if (parsed.reason) existing.reason = parsed.reason; @@ -588,15 +644,17 @@ function addIgnoreValue(cwd, args) { const entry = { rule: parsed.rule, value: parsed.value, - createdAt: new Date().toISOString(), }; + if (parsed.files.length) entry.files = parsed.files; + entry.createdAt = new Date().toISOString(); if (parsed.reason) entry.reason = parsed.reason; config.ignoreValues.push(entry); } const target = writeDetectorConfig(cwd, config, { local }); const scope = local ? 'local detector.ignoreValues' : 'shared detector.ignoreValues'; - return `Added ${parsed.rule}=${parsed.value} to ${scope} (${path.relative(cwd, target) || target}).`; + const scopeSuffix = parsed.files.length ? ` scoped to ${parsed.files.join(', ')}` : ''; + return `Added ${parsed.rule}=${parsed.value}${scopeSuffix} to ${scope} (${path.relative(cwd, target) || target}).`; } function reset(cwd) { diff --git a/skill/scripts/hook-lib.mjs b/skill/scripts/hook-lib.mjs index 0d9722953..2893fa4f3 100644 --- a/skill/scripts/hook-lib.mjs +++ b/skill/scripts/hook-lib.mjs @@ -502,12 +502,15 @@ export function normalizeIgnoreValueEntries(entries) { ...(Array.isArray(entry.files) ? entry.files.filter(v => typeof v === 'string' && v.trim()).map(v => v.trim()) : []), ]); if (files.length > 0) normalized.files = files; - if (typeof entry.reason === 'string' && entry.reason.trim()) { - normalized.reason = entry.reason.trim(); - } + // Key order is rule, value, files, createdAt, reason and must stay that way: + // normalizing runs on every write, so emitting a different order than the one + // already on disk rewrites every untouched entry and churns the diff. if (typeof entry.createdAt === 'string' && entry.createdAt.trim()) { normalized.createdAt = entry.createdAt.trim(); } + if (typeof entry.reason === 'string' && entry.reason.trim()) { + normalized.reason = entry.reason.trim(); + } out.push(normalized); } return out; @@ -525,7 +528,9 @@ function mergeIgnoreValues(existing, incoming) { } function ignoreValueFilesKey(files) { - return Array.isArray(files) && files.length > 0 ? files.join('\x1f') : ''; + // Sort before joining: a scope is a set, so an entry already on disk in another + // order must compare equal rather than dedup as two distinct entries. + return Array.isArray(files) && files.length > 0 ? [...files].sort().join('\x1f') : ''; } export function readCache(cwd) { @@ -1465,16 +1470,17 @@ export function appendDesignSystemNote(text, scanOptions) { // raw envelope. Asking the model to surface the resolution in its // reply is the cheapest way to make the feedback loop visible. function directiveFooter(display, opts = {}) { - const ignoreFileCommand = `${IMPECCABLE_COMMAND} hooks ignore-file ${quoteCommandArg(display)}`; - const fileIgnoreGuidance = opts.grouped - ? `run \`${IMPECCABLE_COMMAND} hooks ignore-file \` for the specific file` - : `run \`${ignoreFileCommand}\``; + // Offer the rule-scoped-to-file form first. `ignore-file` silences every rule + // for the path forever, which is far more than one noisy rule on a real UI + // surface justifies, and it was previously the only option named here. + const target = opts.grouped ? '' : quoteCommandArg(display); + const fileIgnoreGuidance = `run \`${IMPECCABLE_COMMAND} hooks ignore-value "*" --file ${target}\` to scope just that rule to the file, or \`${IMPECCABLE_COMMAND} hooks ignore-file ${target}\` only when the whole file is out of scope for design review (a fixture, a generated artifact, a deliberate demo)`; return [ 'Handle these before finalizing: fix findings that are real design problems, or explicitly classify contextually intentional findings as false positives. Acknowledge what you changed or why you are leaving a finding unchanged.', '', 'Use context judgment before editing. A finding is not automatically a defect; literal or domain-appropriate motion, intentional demos or fixtures, documentation of bad design, and user-confirmed choices can be valid as-is.', '', - `Do not change intentional design just to satisfy the hook, and do not silence a real finding with an inline ignore comment to skip fixing it. Suppress a finding only after the user explicitly confirms it is intentional. Prefer a config ignore (one reviewable place, the commands below); reach for an inline \`impeccable-disable \` comment only when the waiver must travel with a file that leaves the repo, such as an exported or standalone document. Prefer the narrowest persisted exception: run the exact \`${IMPECCABLE_COMMAND} hooks ignore-value ... --shared\` command shown next to a value-specific finding. For \`overused-font\`, use \`ignore-value\` for a specific font and use \`${IMPECCABLE_COMMAND} hooks ignore-rule overused-font --all-values\` only when the user asks to ignore overused fonts generally. For file-specific findings without an ignore-value command, ${fileIgnoreGuidance}; use \`${IMPECCABLE_COMMAND} hooks ignore-rule \` only when the user asks to suppress the whole non-value-specific rule. Run ${IMPECCABLE_COMMAND} audit for the full pass.`, + `Do not change intentional design just to satisfy the hook, and do not silence a real finding with an inline ignore comment to skip fixing it. Suppress a finding only after the user explicitly confirms it is intentional. Prefer a config ignore (one reviewable place, the commands below); reach for an inline \`impeccable-disable \` comment only when the waiver must travel with a file that leaves the repo, such as an exported or standalone document. Prefer the narrowest persisted exception: run the exact \`${IMPECCABLE_COMMAND} hooks ignore-value ... --shared\` command shown next to a value-specific finding. For \`overused-font\`, use \`ignore-value\` for a specific font and use \`${IMPECCABLE_COMMAND} hooks ignore-rule overused-font --all-values\` only when the user asks to ignore overused fonts generally. For a finding whose line shows no exact ignore-value command, such as \`side-tab\`, ${fileIgnoreGuidance}; use \`${IMPECCABLE_COMMAND} hooks ignore-rule \` only when the user asks to suppress the whole non-value-specific rule. Run ${IMPECCABLE_COMMAND} audit for the full pass.`, ].join('\n'); } diff --git a/tests/hook.test.mjs b/tests/hook.test.mjs index 3fe0000ba..0bf3317c7 100644 --- a/tests/hook.test.mjs +++ b/tests/hook.test.mjs @@ -53,7 +53,9 @@ import { extractFindingIgnoreValue, resolveProjectPlatform, isNativePlatform, + normalizeIgnoreValueEntries, } from '../skill/scripts/hook-lib.mjs'; +import { normalizeIgnoreValueEntries as normalizeIgnoreValueEntriesCli } from '../cli/lib/impeccable-config.mjs'; import { detectHtml, detectText } from '../cli/engine/detect-antipatterns.mjs'; function mkTmp() { @@ -525,6 +527,72 @@ describe('hook-admin.mjs', () => { }); } + it('refuses an empty --file glob instead of silently writing a project-wide ignore', () => { + // `--file=` was dropped by filter(Boolean), so this reported success and + // stored an entry with no files: a broader suppression than was asked for. + for (const args of [['--file='], ['--file', ''], ['--files=']]) { + assert.throws( + () => runAdmin(['ignore-value', 'overused-font', 'Inter', ...args]), + /requires a non-empty glob/, + `empty glob via ${args.join(' ')} must error`, + ); + } + // `--file --reason "why"` consumed --reason as the scope and let the reason + // text fold into the value: stored value="* why" files=["--reason"], success. + assert.throws( + () => runAdmin(['ignore-value', 'design-system-font-size', '*', '--file', '--reason', 'why']), + /requires a glob, got the flag --reason/, + 'a following flag is not a glob', + ); + assert.equal(fs.existsSync(path.join(cwd, '.impeccable', 'config.json')), false, 'nothing may be written'); + }); + + it('matches an on-disk scope whose glob order differs from the sorted argv form', () => { + // Storage is canonical now, but configs written before that are not. Every key + // that hashes `files` must sort or a re-add duplicates the entry. + fs.mkdirSync(path.join(cwd, '.impeccable'), { recursive: true }); + fs.writeFileSync(path.join(cwd, '.impeccable', 'config.json'), JSON.stringify({ + detector: { ignoreValues: [ + { rule: 'design-system-font-size', value: '*', files: ['b.css', 'a.css'], createdAt: '2026-01-01T00:00:00.000Z' }, + ] }, + })); + runAdmin(['ignore-value', 'design-system-font-size', '*', '--file', 'a.css', '--file', 'b.css', '--reason', 're-add']); + const cfg = JSON.parse(fs.readFileSync(path.join(cwd, '.impeccable', 'config.json'), 'utf-8')); + const entries = cfg.detector.ignoreValues.filter((e) => e.rule === 'design-system-font-size'); + assert.equal(entries.length, 1, 'an unsorted on-disk scope must match the sorted argv form, not duplicate'); + assert.equal(entries[0].reason, 're-add', 'the existing entry is the one updated'); + }); + + it('stores a multi-file scope in canonical order so argv order cannot duplicate it', () => { + runAdmin(['ignore-value', 'design-system-font-size', '*', '--file', 'b.css', '--file', 'a.css']); + runAdmin(['ignore-value', 'design-system-font-size', '*', '--file', 'a.css', '--file', 'b.css']); + const cfg = JSON.parse(fs.readFileSync(path.join(cwd, '.impeccable', 'config.json'), 'utf-8')); + const entries = cfg.detector.ignoreValues.filter((e) => e.rule === 'design-system-font-size'); + assert.equal(entries.length, 1, 'the same scope in a different order is one entry, not two'); + assert.deepEqual(entries[0].files, ['a.css', 'b.css']); + }); + + it('status shows the file scope of a scoped wildcard ignore', () => { + runAdmin(['ignore-value', 'design-system-font-size', '*', '--file', 'src/widget.js']); + const out = runAdmin(['status']); + // Printing `design-system-font-size=*` bare reads as the project-wide + // wildcard this command refuses, which is the opposite of what is on disk. + assert.match(out, /design-system-font-size=\*\s*\[src\/widget\.js\]/); + }); + + it('refuses a bare wildcard and names a project-wide command that actually works', () => { + assert.throws( + () => runAdmin(['ignore-value', 'design-system-font-size', '*']), + (err) => /--file/.test(String(err.stderr)) && /ignore-rule design-system-font-size\./.test(String(err.stderr)), + ); + // ignore-rule overused-font refuses on its own without --all-values, so the + // suggestion must carry the flag or it hands the user a second error. + assert.throws( + () => runAdmin(['ignore-value', 'overused-font', '*']), + (err) => /ignore-rule overused-font --all-values/.test(String(err.stderr)), + ); + }); + it('ignore-value writes shared config by default without creating local config', () => { const out = runAdmin(['ignore-value', 'overused-font', 'Inter', '--reason', 'User confirmed Inter']); assert.match(out, /overused-font=inter/); @@ -564,6 +632,132 @@ describe('hook-admin.mjs', () => { assert.match(status, /ignoreValues:\s+overused-font=inter/); }); + // detector.ignoreValues honours a `files` scope, which is the narrowest way to + // silence one noisy rule on one file. hook-admin could not write it, so the + // only reachable option was ignore-file, which silences every rule for that + // file forever. + it('ignore-value scopes a wildcard to files via --file', () => { + const out = runAdmin([ + 'ignore-value', 'design-system-font-size', '*', + '--file', 'src/overlay/widget.js', + '--reason', 'Widget builds its own type scale', + ]); + assert.match(out, /scoped to src\/overlay\/widget\.js/); + const shared = JSON.parse(fs.readFileSync(getConfigPath(cwd), 'utf-8')).detector; + assert.deepEqual(shared.ignoreValues, [{ + rule: 'design-system-font-size', + value: '*', + files: ['src/overlay/widget.js'], + createdAt: shared.ignoreValues[0].createdAt, + reason: 'Widget builds its own type scale', + }]); + }); + + it('ignore-value accepts --file=, --files= and repeated --file', () => { + runAdmin(['ignore-value', 'side-tab', '*', '--file=a.css']); + runAdmin(['ignore-value', 'side-tab', '*', '--files=b.css']); + runAdmin(['ignore-value', 'low-contrast', '*', '--file', 'c.css', '--file', 'd.css']); + const shared = JSON.parse(fs.readFileSync(getConfigPath(cwd), 'utf-8')).detector; + assert.deepEqual( + shared.ignoreValues.map(({ rule, files }) => ({ rule, files })), + [ + { rule: 'side-tab', files: ['a.css'] }, + { rule: 'side-tab', files: ['b.css'] }, + { rule: 'low-contrast', files: ['c.css', 'd.css'] }, + ], + 'each distinct file scope is its own entry; a rule+value-only key overwrote them', + ); + }); + + it('ignore-value refuses a wildcard with no file scope', () => { + assert.throws( + () => runAdmin(['ignore-value', 'design-system-font-size', '*']), + /Wildcard value ignores must be scoped with --file/, + 'a bare wildcard is ignore-rule\'s job, not a per-file waiver', + ); + assert.equal(fs.existsSync(getConfigPath(cwd)), false, 'a refused ignore must not write config'); + }); + + it('ignore-value --file requires a glob', () => { + assert.throws( + () => runAdmin(['ignore-value', 'side-tab', '*', '--file']), + /--file requires a glob/, + ); + }); + + it('ignore-value rejects an unknown flag instead of folding it into the value', () => { + // `--shard` (a typo for --shared) used to store the value "inter --shard", + // which matches nothing, while reporting a successful suppression. + assert.throws( + () => runAdmin(['ignore-value', 'overused-font', 'Inter', '--shard']), + /Unknown ignore-value flag: --shard/, + ); + assert.equal(fs.existsSync(getConfigPath(cwd)), false); + }); + + // Every write runs the entries through normalizeIgnoreValueEntries. Emitting a + // different key order than the one on disk rewrote all untouched entries. + it('an unrelated edit leaves existing ignoreValues byte-identical', () => { + fs.mkdirSync(path.join(cwd, '.impeccable'), { recursive: true }); + const seeded = { + detector: { + ignoreRules: [], + ignoreFiles: [], + ignoreValues: [ + { + rule: 'bounce-easing', + value: 'bounce-ball', + createdAt: '2026-06-15T04:15:03.164Z', + reason: 'Intentional', + }, + { + rule: 'design-system-color', + value: '*', + files: ['site/styles/demo.css'], + createdAt: '2026-06-15T23:37:38.170Z', + reason: 'Deliberate off-system demo', + }, + ], + }, + }; + fs.writeFileSync(getConfigPath(cwd), JSON.stringify(seeded, null, 2) + '\n'); + const before = JSON.parse(fs.readFileSync(getConfigPath(cwd), 'utf-8')).detector.ignoreValues; + + runAdmin(['ignore-file', 'some/other/**']); + + const after = JSON.parse(fs.readFileSync(getConfigPath(cwd), 'utf-8')).detector; + assert.deepEqual(after.ignoreFiles, ['some/other/**'], 'the intended change still lands'); + assert.equal( + JSON.stringify(after.ignoreValues), + JSON.stringify(before), + 'untouched ignoreValues must keep their exact key order, or every config diff churns', + ); + }); + + // hook-lib.mjs (skill, ships into harness dirs) and cli/lib/impeccable-config.mjs + // (CLI + Pages functions) carry independent copies of this normalizer by + // necessity. They write the same file, so a key-order drift between them makes + // the config churn depending on which tool touched it last. + it('both config normalizers emit identical entries', () => { + const input = [ + { rule: 'BOUNCE-EASING', value: 'Bounce-Ball', reason: ' r ', createdAt: '2026-01-01T00:00:00.000Z' }, + { rule: 'design-system-color', value: '*', files: [' a.css ', 'b.css', 'a.css'], createdAt: '2026-02-02T00:00:00.000Z' }, + { rule: 'side-tab', value: '*', file: 'legacy.css' }, + { rule: '', value: 'dropped' }, + ]; + assert.equal( + JSON.stringify(normalizeIgnoreValueEntries(input)), + JSON.stringify(normalizeIgnoreValueEntriesCli(input)), + 'skill/scripts/hook-lib.mjs and cli/lib/impeccable-config.mjs must agree, key order included', + ); + // And pin the canonical order itself, which is what the config on disk uses. + const full = { rule: 'side-tab', value: '*', files: ['a.css'], createdAt: '2026-01-01T00:00:00.000Z', reason: 'r' }; + assert.deepEqual( + Object.keys(normalizeIgnoreValueEntries([full])[0]), + ['rule', 'value', 'files', 'createdAt', 'reason'], + ); + }); + it('a /impeccable hooks edit preserves sibling hook fields (consent, quiet)', () => { fs.mkdirSync(path.join(cwd, '.impeccable'), { recursive: true }); // A recorded per-developer consent in the local file...