diff --git a/cli/bin/commands/ignores.mjs b/cli/bin/commands/ignores.mjs index 9eb4696ed..0d1393d0f 100644 --- a/cli/bin/commands/ignores.mjs +++ b/cli/bin/commands/ignores.mjs @@ -226,12 +226,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); } diff --git a/cli/lib/impeccable-config.mjs b/cli/lib/impeccable-config.mjs index 45c65e65a..431e4cad0 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; 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..cd40aff6b 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. @@ -534,12 +536,13 @@ function addIgnoreFile(cwd, 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 +553,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(String(args[++i]).trim()); + } else if (arg.startsWith('--file=')) { + files.push(arg.slice('--file='.length).trim()); + } else if (arg.startsWith('--files=')) { + files.push(arg.slice('--files='.length).trim()); + } 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 +576,7 @@ function parseIgnoreValueArgs(args) { return { rule: String(rule || '').trim().toLowerCase(), value: normalizeIgnoreValue(valueParts.join(' ')), + files: Array.from(new Set(files.filter(Boolean))), shared, local, reason, @@ -577,10 +593,19 @@ 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) { + 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 ${IMPECCABLE_COMMAND} hooks ignore-rule ${parsed.rule}.`); + } + 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 +613,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..21fcd6b68 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; @@ -1465,16 +1468,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..c2fa43f87 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() { @@ -564,6 +566,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...