From 5330fa358e1eb6c067dca3c9c39323ba48dc5c1f Mon Sep 17 00:00:00 2001 From: Guitaraholic Date: Mon, 24 Aug 2026 00:57:13 +0100 Subject: [PATCH] Fix: honour .impeccable detector ignores in the live overlay (#639) The live overlay's detect scan ran unfiltered: requestDetectScan() posted only { scanId }, so detector.ignoreRules and detector.ignoreValues in .impeccable/config.json reached impeccable detect and the edit hook but never the surface a designer actually watches. The server now serializes the project's detector waivers into the /live.js prelude (window.__IMPECCABLE_PROJECT_IGNORES__), read per request through hook-lib's readConfig so config.local.json wins and edits land on the next tab reload. A new script part, live-browser-ignores.js, resolves that config against the page URL when a scan starts: ignoreRules suppress outright, wildcard ignoreValues suppress their rule in the files their globs name, and the remaining entries ride along as disabledValues for the detector to match on each finding's own value. The detector bundle applies those where the findings are assembled, since the overlay draws its own markers from the collected findings. Scope resolution mirrors cli/lib/impeccable-config.mjs deliberately: the same glob dialect (globToRegex, including {a,b} alternation), the same path-suffix matching as findingMatchesScopedIgnoreFile, and the same refusal to apply an unscoped wildcard entry. The served-root prefixes that bridge project-relative globs and site-relative URLs come from the inject config's own files globs, never from the ignore globs; deriving them from the ignore globs lets one entry scoped to prototype/library/** lend its prefix to every page and suppress site-wide, which looks like success because the numbers go down. Known gaps, recorded in the detector comment: the motion value extractor is not mirrored, so a value-scoped bounce-easing waiver only matches when the finding carries ignoreValue directly, and design-system-color matches on the normalized string without the CLI's color-equality fallback. Tests: unit tests for the resolver part (stale globals, string ignoreRules, malformed entries, directory URLs, percent-escapes, glob metacharacters, the roots trap), an extension-mode puppeteer test that disabledValues suppress exactly the waived findings, and the live-browser regression pin now asserts the new scan config shape instead of { scanId }. Co-Authored-By: Claude Fable 5 --- cli/engine/browser/injected/index.mjs | 63 +++++++ cli/engine/detect-antipatterns-browser.js | 63 +++++++ scripts/test-suites.mjs | 1 + skill/scripts/live-browser-ignores.js | 181 ++++++++++++++++++ skill/scripts/live-browser.js | 16 +- skill/scripts/live-server.mjs | 50 +++++ skill/scripts/live/browser-script-parts.mjs | 9 +- tests/detect-antipatterns-browser.test.mjs | 114 +++++++++++ tests/live-browser-ignores.test.mjs | 197 ++++++++++++++++++++ tests/live-browser-regression.test.mjs | 9 +- tests/live-browser-script-parts.test.mjs | 32 +++- 11 files changed, 726 insertions(+), 9 deletions(-) create mode 100644 skill/scripts/live-browser-ignores.js create mode 100644 tests/live-browser-ignores.test.mjs diff --git a/cli/engine/browser/injected/index.mjs b/cli/engine/browser/injected/index.mjs index 6eb971450..ed91c8fb1 100644 --- a/cli/engine/browser/injected/index.mjs +++ b/cli/engine/browser/injected/index.mjs @@ -1675,6 +1675,69 @@ if (IS_BROWSER) { addBrowserFindings(groupMap, document.body, mapped); } + // Value-level suppression (issue #639). `disabledRules` above handles + // whole rules; this applies the config's remaining ignoreValues entries, + // which the CLI filters through isIgnoredFindingValue in + // cli/lib/impeccable-config.mjs, so a project waiver like + // overused-font = "geist mono" reaches the overlay and extension too. + const _normValue = (v) => String(v || '').trim().replace(/^["']|["']$/g, '') + .replace(/\+/g, ' ').replace(/\s+/g, ' ').toLowerCase(); + const _disabledValues = EXTENSION_MODE + ? (Array.isArray(window.__IMPECCABLE_CONFIG__?.disabledValues) ? window.__IMPECCABLE_CONFIG__.disabledValues : []) + .filter(e => e && typeof e === 'object' && e.rule && e.value) + .map(e => ({ rule: String(e.rule).trim().toLowerCase(), value: _normValue(e.value) })) + : []; + if (_disabledValues.length > 0) { + // The six rules whose findings carry a matchable value; keep in step + // with extractFindingIgnoreValue in cli/lib/impeccable-config.mjs. + // Everything else is suppressed by rule or by file scope, both already + // resolved into disabledRules before the scan message was sent. + const _directValueRules = new Set([ + 'overused-font', + 'bounce-easing', + 'design-system-font', + 'design-system-color', + 'design-system-radius', + 'design-system-font-size', + ]); + // The design-system checks set `ignoreValue` on their findings; the + // detail fallbacks catch overused-font, whose value lives in its + // sentence. Two CLI matchers are not mirrored here: the motion + // extractor (a value-scoped bounce-easing waiver only matches when the + // finding carries ignoreValue directly) and the design-system-color + // color-equality fallback (a waiver written as rgb() will not match a + // finding reported as hex; store the reported form). + const _findingValue = (f) => { + if (!f || !_directValueRules.has(f.type || f.id)) return ''; + const direct = f.ignoreValue || f.value; + if (direct) return _normValue(direct); + for (const text of [f.detail, f.snippet]) { + if (typeof text !== 'string' || !text) continue; + const primary = text.match(/Primary font:\s*([^()\n;]+)/i); + if (primary) return _normValue(primary[1]); + const google = text.match(/Google Fonts:\s*([^()\n;]+)/i); + if (google) return _normValue(google[1]); + const family = text.match(/font-family\s*:\s*["']?([^'",;\n]+)/i); + if (family) return _normValue(family[1]); + } + return ''; + }; + const _valueIgnored = (f) => { + const value = _findingValue(f); + if (!value) return false; + const rule = f.type || f.id; + return _disabledValues.some(e => e.rule === rule && e.value === value); + }; + for (const [el, list] of [...groupMap.entries()]) { + const kept = list.filter(f => !_valueIgnored(f)); + if (kept.length > 0) groupMap.set(el, kept); + else groupMap.delete(el); + } + for (let i = pageLevelFindings.length - 1; i >= 0; i--) { + if (_valueIgnored(pageLevelFindings[i])) pageLevelFindings.splice(i, 1); + } + } + return { groupMap, allFindings: browserFindingsFromMap(groupMap), diff --git a/cli/engine/detect-antipatterns-browser.js b/cli/engine/detect-antipatterns-browser.js index 8893bb323..f40a768f7 100644 --- a/cli/engine/detect-antipatterns-browser.js +++ b/cli/engine/detect-antipatterns-browser.js @@ -8334,6 +8334,69 @@ if (IS_BROWSER) { addBrowserFindings(groupMap, document.body, mapped); } + // Value-level suppression (issue #639). `disabledRules` above handles + // whole rules; this applies the config's remaining ignoreValues entries, + // which the CLI filters through isIgnoredFindingValue in + // cli/lib/impeccable-config.mjs, so a project waiver like + // overused-font = "geist mono" reaches the overlay and extension too. + const _normValue = (v) => String(v || '').trim().replace(/^["']|["']$/g, '') + .replace(/\+/g, ' ').replace(/\s+/g, ' ').toLowerCase(); + const _disabledValues = EXTENSION_MODE + ? (Array.isArray(window.__IMPECCABLE_CONFIG__?.disabledValues) ? window.__IMPECCABLE_CONFIG__.disabledValues : []) + .filter(e => e && typeof e === 'object' && e.rule && e.value) + .map(e => ({ rule: String(e.rule).trim().toLowerCase(), value: _normValue(e.value) })) + : []; + if (_disabledValues.length > 0) { + // The six rules whose findings carry a matchable value; keep in step + // with extractFindingIgnoreValue in cli/lib/impeccable-config.mjs. + // Everything else is suppressed by rule or by file scope, both already + // resolved into disabledRules before the scan message was sent. + const _directValueRules = new Set([ + 'overused-font', + 'bounce-easing', + 'design-system-font', + 'design-system-color', + 'design-system-radius', + 'design-system-font-size', + ]); + // The design-system checks set `ignoreValue` on their findings; the + // detail fallbacks catch overused-font, whose value lives in its + // sentence. Two CLI matchers are not mirrored here: the motion + // extractor (a value-scoped bounce-easing waiver only matches when the + // finding carries ignoreValue directly) and the design-system-color + // color-equality fallback (a waiver written as rgb() will not match a + // finding reported as hex; store the reported form). + const _findingValue = (f) => { + if (!f || !_directValueRules.has(f.type || f.id)) return ''; + const direct = f.ignoreValue || f.value; + if (direct) return _normValue(direct); + for (const text of [f.detail, f.snippet]) { + if (typeof text !== 'string' || !text) continue; + const primary = text.match(/Primary font:\s*([^()\n;]+)/i); + if (primary) return _normValue(primary[1]); + const google = text.match(/Google Fonts:\s*([^()\n;]+)/i); + if (google) return _normValue(google[1]); + const family = text.match(/font-family\s*:\s*["']?([^'",;\n]+)/i); + if (family) return _normValue(family[1]); + } + return ''; + }; + const _valueIgnored = (f) => { + const value = _findingValue(f); + if (!value) return false; + const rule = f.type || f.id; + return _disabledValues.some(e => e.rule === rule && e.value === value); + }; + for (const [el, list] of [...groupMap.entries()]) { + const kept = list.filter(f => !_valueIgnored(f)); + if (kept.length > 0) groupMap.set(el, kept); + else groupMap.delete(el); + } + for (let i = pageLevelFindings.length - 1; i >= 0; i--) { + if (_valueIgnored(pageLevelFindings[i])) pageLevelFindings.splice(i, 1); + } + } + return { groupMap, allFindings: browserFindingsFromMap(groupMap), diff --git a/scripts/test-suites.mjs b/scripts/test-suites.mjs index 1c1f64c58..5d2d1f227 100644 --- a/scripts/test-suites.mjs +++ b/scripts/test-suites.mjs @@ -131,6 +131,7 @@ export const SUITES = { 'tests/live-accept-css.test.mjs', 'tests/live-accept-scrub.test.mjs', 'tests/live-browser-dom.test.mjs', + 'tests/live-browser-ignores.test.mjs', 'tests/live-browser-script-parts.test.mjs', 'tests/live-browser-regression.test.mjs', 'tests/live-browser-session.test.mjs', diff --git a/skill/scripts/live-browser-ignores.js b/skill/scripts/live-browser-ignores.js new file mode 100644 index 000000000..fd78fe21b --- /dev/null +++ b/skill/scripts/live-browser-ignores.js @@ -0,0 +1,181 @@ +/** + * Browser-side resolution of project detector waivers for Impeccable live mode. + * + * The live server serializes `.impeccable/config.json` + `config.local.json` + * detector ignores (plus the served-root prefixes from the inject config's + * `files` globs) into `window.__IMPECCABLE_PROJECT_IGNORES__`. This part + * resolves that config against the current page's URL path when a detect scan + * starts, so the overlay suppresses the same findings the CLI and the edit + * hook do (issue #639). + * + * Mirrors filterDetectionFindings in cli/lib/impeccable-config.mjs: + * 1. `ignoreRules` suppress a rule project-wide. + * 2. `ignoreValues` entries with `value: "*"` suppress their rule in the + * files their globs name. The CLI never applies an unscoped wildcard + * (isIgnoredFindingValue returns false for it), so neither does this. + * 3. Remaining `ignoreValues` entries match on the finding's own value; + * those are forwarded as `disabledValues` for the detector bundle to + * apply where the findings are assembled. + * + * Kept separate from live-browser.js so the glob and page-scope logic can be + * unit tested in Node (tests/live-browser-ignores.test.mjs) without the full + * overlay UI bundle. + */ +(function (root) { + 'use strict'; + if (!root) return; + + // Keep in step with normalizeIgnoreRule / normalizeIgnoreValue in + // cli/lib/impeccable-config.mjs. + function normalizeIgnoreRule(rule) { + return String(rule || '').trim().toLowerCase(); + } + + function normalizeIgnoreValue(value) { + return String(value || '') + .trim() + .replace(/^["']|["']$/g, '') + .replace(/\+/g, ' ') + .replace(/\s+/g, ' ') + .toLowerCase(); + } + + // Glob -> RegExp. Supports `**`, `*`, `?`, and `{a,b}` alternation. + // Keep in step with globToRegex in cli/lib/impeccable-config.mjs. + function globToRegex(glob) { + let re = '^'; + let i = 0; + while (i < glob.length) { + const c = glob[i]; + if (c === '*') { + if (glob[i + 1] === '*') { + re += '.*'; + i += 2; + if (glob[i] === '/') i += 1; + } else { + re += '[^/]*'; + i += 1; + } + } else if (c === '?') { + re += '[^/]'; + i += 1; + } else if (c === '{') { + const end = glob.indexOf('}', i); + if (end === -1) { re += '\\{'; i += 1; continue; } + const parts = glob.slice(i + 1, end).split(',').map((p) => p.replace(/[.+^$()|[\]\\]/g, '\\$&')); + re += `(?:${parts.join('|')})`; + i = end + 1; + } else if (/[.+^$()|[\]\\]/.test(c)) { + re += `\\${c}`; + i += 1; + } else { + re += c; + i += 1; + } + } + re += '$'; + return new RegExp(re); + } + + // The project-relative paths this page could be known as. Ignore globs are + // project-relative (prototype/foo.html) and the URL is site-relative + // (/foo.html), because a static server's root usually sits inside the + // project; `roots` carries that prefix. The server reads it from the inject + // config's own `files` globs, which already state where the served pages + // are. Do not derive it from the ignore globs: a single entry scoped to + // prototype/library/** would then lend prototype/library/ as a candidate + // prefix to every page, and that rule would suppress site-wide. + // + // Each prefixed path also contributes its slash suffixes, mirroring + // findingMatchesScopedIgnoreFile in cli/lib/impeccable-config.mjs (which + // matches globs against every path suffix of the finding's file). + function pageCandidates(pathname, roots) { + let pagePath = String(pathname || ''); + try { + pagePath = decodeURIComponent(pagePath); + } catch { + // Malformed percent-escape: match on the raw path rather than throwing. + } + pagePath = pagePath.replace(/^\/+/, ''); + // A directory URL serves that directory's index, and the ignore globs + // name files. Without this, /news/ never matches prototype/news/index.html. + if (pagePath === '' || pagePath.endsWith('/')) pagePath += 'index.html'; + + const prefixes = ['']; + for (const entry of Array.isArray(roots) ? roots : []) { + if (typeof entry !== 'string') continue; + prefixes.push(entry === '' || entry.endsWith('/') ? entry : entry + '/'); + } + + const candidates = new Set(); + for (const prefix of prefixes) { + const full = prefix + pagePath; + const parts = full.split('/').filter(Boolean); + for (let i = 0; i < parts.length; i++) { + candidates.add(parts.slice(i).join('/')); + } + } + return [...candidates]; + } + + function matchesScope(globs, candidates) { + return globs.some((glob) => { + let re; + try { + re = globToRegex(String(glob)); + } catch { + return false; + } + return candidates.some((candidate) => re.test(candidate)); + }); + } + + /** + * Resolve the serialized project ignores for one page. + * + * @param {object} options + * @param {object} options.ignores window.__IMPECCABLE_PROJECT_IGNORES__, + * in whatever state it arrived: absent, null, or hand-edited into the + * wrong shape. Every read tolerates that and degrades to no filtering. + * @param {string} options.pathname location.pathname of the scanned page. + * @returns {{ disabledRules: string[], disabledValues: Array<{rule: string, value: string}> }} + */ + function resolveDetectIgnores({ ignores, pathname } = {}) { + const config = ignores && typeof ignores === 'object' ? ignores : {}; + const asArray = (value) => (Array.isArray(value) ? value : []); + const candidates = pageCandidates(pathname, config.roots); + + const disabledRules = new Set( + asArray(config.ignoreRules) + .filter((rule) => typeof rule === 'string') + .map(normalizeIgnoreRule) + .filter(Boolean), + ); + const disabledValues = []; + + for (const entry of asArray(config.ignoreValues)) { + if (!entry || typeof entry !== 'object') continue; + const rule = normalizeIgnoreRule(entry.rule); + const value = normalizeIgnoreValue(entry.value); + if (!rule || !value) continue; + const files = [ + ...(typeof entry.file === 'string' && entry.file.trim() ? [entry.file.trim()] : []), + ...asArray(entry.files).filter((glob) => typeof glob === 'string' && glob.trim()), + ]; + if (value === '*') { + // Wildcards suppress their rule only inside the files they name. + if (files.length > 0 && matchesScope(files, candidates)) disabledRules.add(rule); + continue; + } + if (files.length > 0 && !matchesScope(files, candidates)) continue; + disabledValues.push({ rule, value }); + } + + return { disabledRules: [...disabledRules], disabledValues }; + } + + root.__IMPECCABLE_LIVE_IGNORES__ = { + version: 1, + resolveDetectIgnores, + }; +})(typeof window !== 'undefined' ? window : globalThis); diff --git a/skill/scripts/live-browser.js b/skill/scripts/live-browser.js index 69d227b0a..e9435f8c2 100644 --- a/skill/scripts/live-browser.js +++ b/skill/scripts/live-browser.js @@ -11143,10 +11143,24 @@ void main() { const scanId = String(++detectScanSeq); activeDetectScanId = scanId; pendingDetectScanId = scanId; + // Send the project's detector waivers with the scan so the overlay + // filters the same findings the CLI and the edit hook do (issue #639). + // live-browser-ignores.js resolves .impeccable config for this page: + // ignoreRules suppress outright, wildcard ignoreValues suppress their + // rule in the files they name, and the rest match on the finding's own + // value inside the detector. Guarded so a stale cached live.js without + // the resolver part still scans, just unfiltered as before. + const ignoresApi = window.__IMPECCABLE_LIVE_IGNORES__; + const ignores = typeof ignoresApi?.resolveDetectIgnores === 'function' + ? ignoresApi.resolveDetectIgnores({ + ignores: window.__IMPECCABLE_PROJECT_IGNORES__, + pathname: location.pathname, + }) + : { disabledRules: [], disabledValues: [] }; window.postMessage({ source: 'impeccable-command', action: 'scan', - config: { scanId }, + config: { scanId, disabledRules: ignores.disabledRules, disabledValues: ignores.disabledValues }, }, '*'); } diff --git a/skill/scripts/live-server.mjs b/skill/scripts/live-server.mjs index dafa8bd0c..ab298843b 100644 --- a/skill/scripts/live-server.mjs +++ b/skill/scripts/live-server.mjs @@ -22,6 +22,7 @@ import net from 'node:net'; import { fileURLToPath } from 'node:url'; import { parseDesignMd } from './lib/design-parser.mjs'; import { loadContext } from './context.mjs'; +import { readConfig as readHookConfig } from './hook-lib.mjs'; import { assembleLiveBrowserScript, assertLiveBrowserScriptParts, @@ -45,6 +46,7 @@ import { readLiveServerInfo, removeLiveServerInfo, resolveDesignSidecarPath, + resolveLiveConfigPath, writeLiveServerInfo, } from './lib/impeccable-paths.mjs'; import { countByPage as countPendingByPage } from './live/manual-edits-buffer.mjs'; @@ -694,6 +696,51 @@ function isLoopbackOrigin(origin) { // HTTP request handler // --------------------------------------------------------------------------- +// Project detector waivers for the browser overlay (issue #639). The CLI and +// the edit hook filter findings through .impeccable/config.json; the overlay +// scans in the browser, so the same config rides along in the /live.js +// prelude and live-browser-ignores.js applies it per page at scan time. +function readProjectDetectorIgnores() { + // readConfig merges config.json with the gitignored config.local.json and + // type-checks both, exactly as the edit hook reads the same pair. + const config = readHookConfig(process.cwd()); + return { + ignoreRules: Array.isArray(config.ignoreRules) ? config.ignoreRules : [], + // Serve only what the browser matches on; createdAt/reason stay local. + ignoreValues: (Array.isArray(config.ignoreValues) ? config.ignoreValues : []).map((entry) => ({ + rule: entry.rule, + value: entry.value, + ...(Array.isArray(entry.files) && entry.files.length > 0 ? { files: entry.files } : {}), + })), + roots: readLiveServedRoots(), + }; +} + +// Where the served pages live inside the project. Ignore globs are +// project-relative and the browser only knows its URL path, so it needs the +// prefix; the inject config's own `files` globs are the authority on it. +// Deriving it from the ignore globs instead fails silently: one entry scoped +// to prototype/library/** would lend prototype/library/ as a candidate prefix +// to every page, and that rule would suppress site-wide. +function readLiveServedRoots() { + try { + const configPath = resolveLiveConfigPath({ cwd: process.cwd(), scriptsDir: __dirname }); + const live = JSON.parse(fs.readFileSync(configPath, 'utf-8')); + const files = Array.isArray(live?.files) ? live.files : []; + return [...new Set(files + .filter((glob) => typeof glob === 'string' && glob) + .map((glob) => { + const wildcardAt = glob.search(/[*?{]/); + const head = wildcardAt === -1 ? glob : glob.slice(0, wildcardAt); + const cut = head.lastIndexOf('/'); + return cut > -1 ? head.slice(0, cut + 1) : ''; + }))]; + } catch { + // No readable inject config: the browser matches URL paths as-is. + return []; + } +} + function createRequestHandler({ detectScript, liveScriptParts }) { return (req, res) => { const url = new URL(req.url, `http://localhost:${state.port}`); @@ -754,6 +801,9 @@ function createRequestHandler({ detectScript, liveScriptParts }) { commandPrefix: IMPECCABLE_COMMAND_PREFIX, appRoot: process.cwd(), parts, + // Read per request rather than cached, so editing the config and + // reloading the tab is enough to pick up a new waiver. + projectIgnores: readProjectDetectorIgnores(), }); res.writeHead(200, { 'Content-Type': 'application/javascript', diff --git a/skill/scripts/live/browser-script-parts.mjs b/skill/scripts/live/browser-script-parts.mjs index 720709a99..347a0f3f1 100644 --- a/skill/scripts/live/browser-script-parts.mjs +++ b/skill/scripts/live/browser-script-parts.mjs @@ -6,6 +6,7 @@ import { LIVE_CHROME_MOUNT_CONTRACT, LIVE_UI_SURFACES } from './ui-surfaces.mjs' export const LIVE_BROWSER_SCRIPT_PARTS = Object.freeze([ Object.freeze({ name: 'session-state', file: 'live-browser-session.js' }), Object.freeze({ name: 'dom-helpers', file: 'live-browser-dom.js' }), + Object.freeze({ name: 'project-ignores', file: 'live-browser-ignores.js' }), Object.freeze({ name: 'browser-ui', file: 'live-browser.js' }), ]); @@ -47,6 +48,11 @@ export function assembleLiveBrowserScript({ // so tests can assemble with a stand-in. uiSurfaces = LIVE_UI_SURFACES, mountContract = LIVE_CHROME_MOUNT_CONTRACT, + // Project detector waivers ({ ignoreRules, ignoreValues, roots }), read from + // .impeccable config by live-server.mjs. live-browser-ignores.js resolves + // them against the page when a detect scan starts, so the overlay filters + // the same findings the CLI and the edit hook do (issue #639). + projectIgnores = null, }) { const prelude = `window.__IMPECCABLE_TOKEN__ = '${token}';\n` + @@ -66,7 +72,8 @@ export function assembleLiveBrowserScript({ // repo's tests, the impeccable-site Live UI lab) import the module directly, // which is what keeps the two from drifting. `window.__IMPECCABLE_LIVE_UI_SURFACES__ = ${JSON.stringify(uiSurfaces)};\n` + - `window.__IMPECCABLE_LIVE_MOUNT_CONTRACT__ = ${JSON.stringify(mountContract)};\n`; + `window.__IMPECCABLE_LIVE_MOUNT_CONTRACT__ = ${JSON.stringify(mountContract)};\n` + + `window.__IMPECCABLE_PROJECT_IGNORES__ = ${JSON.stringify(projectIgnores)};\n`; const body = parts.map((part) => { const file = part.file || path.basename(part.path || ''); diff --git a/tests/detect-antipatterns-browser.test.mjs b/tests/detect-antipatterns-browser.test.mjs index e77d93231..ffffd3bd7 100644 --- a/tests/detect-antipatterns-browser.test.mjs +++ b/tests/detect-antipatterns-browser.test.mjs @@ -981,6 +981,120 @@ describe('detectUrl — browser-only fixtures', () => { } }); + it('extension mode suppresses disabledValues entries from scan config', async () => { + // The live overlay resolves .impeccable ignoreValues per page and sends + // the survivors as config.disabledValues (issue #639); the detector must + // filter them where the findings are assembled, since the overlay draws + // its own markers from the collected findings. + const normalized = normalizeDesignSystem({ + frontmatter: { + typography: { + display: { fontFamily: 'Avenir Next, Georgia, serif' }, + body: { fontFamily: 'IBM Plex Sans, Arial, sans-serif' }, + }, + colors: { + ink: '#241f1a', + paper: '#f7f4ee', + surface: '#ffffff', + accent: '#b8422e', + border: '#d4c7b9', + }, + rounded: { + sm: '4px', + md: '8px', + '"2xl"': '32px', + full: '999px', + }, + }, + }); + // The JSON-safe payload shape the extension panel and detectUrl inject as + // __IMPECCABLE_CONFIG__.designSystem (serializeDesignSystemForBrowser in + // cli/engine/engines/browser/detect-url.mjs). + const designSystem = { + present: true, + hasFonts: normalized.hasFonts === true, + allowedFonts: Array.from(normalized.allowedFonts || []), + hasColors: normalized.hasColors === true, + allowedColors: Array.from(normalized.allowedColorKeys?.values?.() || []) + .map(entry => entry?.color) + .filter(color => color && Number.isFinite(color.r) && Number.isFinite(color.g) && Number.isFinite(color.b)) + .map(color => ({ r: color.r, g: color.g, b: color.b })), + hasRadii: normalized.hasRadii === true, + allowedRadii: (normalized.allowedRadii || []) + .map(entry => Number(entry?.px)) + .filter(px => Number.isFinite(px)), + hasPillRadius: normalized.hasPillRadius === true, + }; + const puppeteer = await import('puppeteer'); + const browser = await puppeteer.default.launch({ + headless: true, + args: process.env.CI ? ['--no-sandbox', '--disable-setuid-sandbox'] : [], + }); + try { + const page = await browser.newPage(); + await page.setViewport({ width: 1280, height: 800 }); + await page.goto(`${baseUrl}/fixtures/antipatterns/design-system.html`, { waitUntil: 'load' }); + const browserScript = fs.readFileSync(path.join(ROOT, 'cli/engine/detect-antipatterns-browser.js'), 'utf-8'); + await page.evaluate(() => { + document.documentElement.dataset.impeccableExtension = 'true'; + window.__impeccableMessages = []; + window.addEventListener('message', event => { + if (event.source !== window || !event.data?.source?.startsWith('impeccable-')) return; + window.__impeccableMessages.push(event.data); + }); + }); + await page.evaluate(browserScript); + const scan = (scanId, disabledValues) => page.evaluate(async (config) => { + window.postMessage({ source: 'impeccable-command', action: 'scan', config }, '*'); + const deadline = Date.now() + 2000; + while ( + Date.now() < deadline && + !window.__impeccableMessages.some(message => + message.source === 'impeccable-results' && message.scanId === config.scanId) + ) { + await new Promise(resolve => setTimeout(resolve, 25)); + } + const resultMessage = window.__impeccableMessages.find(message => + message.source === 'impeccable-results' && message.scanId === config.scanId); + const flat = (resultMessage?.findings || []).flatMap(group => group.findings || []); + return { + total: flat.length, + colors: flat.filter(finding => finding.type === 'design-system-color').length, + fonts: flat + .filter(finding => finding.type === 'design-system-font') + .map(finding => finding.ignoreValue || ''), + }; + }, { scanId, visualContrast: false, designSystem, ...(disabledValues ? { disabledValues } : {}) }); + + const unfiltered = await scan('scan-dv-1'); + assert.ok( + unfiltered.fonts.some(value => /poppins/i.test(value)), + `expected an undocumented poppins font finding, got: ${JSON.stringify(unfiltered)}`, + ); + + const filtered = await scan('scan-dv-2', [{ rule: 'design-system-font', value: 'poppins' }]); + assert.equal( + filtered.fonts.some(value => /poppins/i.test(value)), + false, + `expected the poppins waiver to suppress its finding, got: ${JSON.stringify(filtered)}`, + ); + const waivedCount = unfiltered.fonts.filter(value => /poppins/i.test(value)).length; + assert.equal( + filtered.total, + unfiltered.total - waivedCount, + `expected exactly the waived findings to disappear, got: ${JSON.stringify({ unfiltered, filtered })}`, + ); + assert.equal( + filtered.colors, + unfiltered.colors, + `expected unrelated design-system findings to survive, got: ${JSON.stringify({ unfiltered, filtered })}`, + ); + await page.close(); + } finally { + await browser.close().catch(() => {}); + } + }); + it('browser API: impeccableDetect is pure, impeccableScan decorates', async () => { const puppeteer = await import('puppeteer'); const browser = await puppeteer.default.launch({ diff --git a/tests/live-browser-ignores.test.mjs b/tests/live-browser-ignores.test.mjs new file mode 100644 index 000000000..38fa4f718 --- /dev/null +++ b/tests/live-browser-ignores.test.mjs @@ -0,0 +1,197 @@ +import { describe, it } from 'node:test'; +import assert from 'node:assert/strict'; +import { readFileSync } from 'node:fs'; +import { join } from 'node:path'; +import vm from 'node:vm'; + +const REPO_ROOT = process.cwd(); +const SCRIPT = join(REPO_ROOT, 'skill/scripts/live-browser-ignores.js'); + +// Evaluated in the test realm (not a vm context) so the arrays the resolver +// returns share this realm's prototypes and deepEqual compares them plainly. +function loadIgnoresApi() { + const source = readFileSync(SCRIPT, 'utf-8'); + const factory = vm.runInThisContext( + `(function (window) {\n${source}\nreturn window.__IMPECCABLE_LIVE_IGNORES__;\n})`, + { filename: SCRIPT }, + ); + return factory({}); +} + +const resolve = loadIgnoresApi().resolveDetectIgnores; + +const EMPTY = { disabledRules: [], disabledValues: [] }; + +describe('live-browser-ignores resolver', () => { + it('registers a versioned API on the root', () => { + const api = loadIgnoresApi(); + assert.equal(api.version, 1); + assert.equal(typeof api.resolveDetectIgnores, 'function'); + }); + + it('degrades to an empty filter when the config global is missing or malformed', () => { + assert.deepEqual(resolve(), EMPTY); + assert.deepEqual(resolve({ ignores: undefined, pathname: '/index.html' }), EMPTY); + assert.deepEqual(resolve({ ignores: null, pathname: '/index.html' }), EMPTY); + assert.deepEqual(resolve({ ignores: 'nonsense', pathname: '/index.html' }), EMPTY); + assert.deepEqual(resolve({ ignores: {}, pathname: '/index.html' }), EMPTY); + }); + + it('does not spread a string ignoreRules into characters', () => { + // `"ignoreRules": "foo"` in a hand-edited config must disable nothing, + // not look like it disabled three one-letter rules. + const out = resolve({ ignores: { ignoreRules: 'foo' }, pathname: '/index.html' }); + assert.deepEqual(out, EMPTY); + }); + + it('forwards ignoreRules normalized and deduplicated', () => { + const out = resolve({ + ignores: { ignoreRules: ['Dark-Glow', 'dark-glow', ' gradient-text ', '', 42, null] }, + pathname: '/index.html', + }); + assert.deepEqual(out.disabledRules, ['dark-glow', 'gradient-text']); + }); + + it('forwards unscoped value entries and drops malformed ones', () => { + const out = resolve({ + ignores: { + ignoreValues: [ + { rule: 'overused-font', value: 'Geist+Mono' }, + null, + 'not-an-entry', + { rule: '', value: 'x' }, + { rule: 'overused-font', value: '' }, + ], + }, + pathname: '/index.html', + }); + assert.deepEqual(out.disabledValues, [{ rule: 'overused-font', value: 'geist mono' }]); + }); + + it('applies wildcard entries only on pages their globs name', () => { + const ignores = { + roots: ['prototype/'], + ignoreValues: [ + { rule: 'dark-glow', value: '*', files: ['prototype/attack-the-soc.html'] }, + ], + }; + const onPage = resolve({ ignores, pathname: '/attack-the-soc.html' }); + assert.deepEqual(onPage.disabledRules, ['dark-glow']); + const elsewhere = resolve({ ignores, pathname: '/index.html' }); + assert.deepEqual(elsewhere.disabledRules, []); + }); + + it('never applies an unscoped wildcard entry, matching the CLI', () => { + // isIgnoredFindingValue in cli/lib/impeccable-config.mjs returns false + // for a wildcard entry with no files; project-wide suppression is + // ignoreRules' job. + const out = resolve({ + ignores: { ignoreValues: [{ rule: 'dark-glow', value: '*' }] }, + pathname: '/index.html', + }); + assert.deepEqual(out, EMPTY); + }); + + it('drops scoped value entries on pages outside their globs', () => { + const ignores = { + roots: ['prototype/'], + ignoreValues: [ + { rule: 'overused-font', value: 'geist mono', files: ['prototype/mgmt-demo.html'] }, + ], + }; + const onPage = resolve({ ignores, pathname: '/mgmt-demo.html' }); + assert.deepEqual(onPage.disabledValues, [{ rule: 'overused-font', value: 'geist mono' }]); + const elsewhere = resolve({ ignores, pathname: '/index.html' }); + assert.deepEqual(elsewhere.disabledValues, []); + }); + + it('does not lend one entry\'s glob prefix to other pages', () => { + // The trap from issue #639: prefixes come only from `roots`, never from + // the ignore globs themselves. An entry scoped to prototype/library/** + // must not suppress on prototype/index.html. + const ignores = { + roots: ['prototype/'], + ignoreValues: [ + { rule: 'em-dash-overuse', value: '*', files: ['prototype/library/**'] }, + ], + }; + const inside = resolve({ ignores, pathname: '/library/buttons.html' }); + assert.deepEqual(inside.disabledRules, ['em-dash-overuse']); + const outside = resolve({ ignores, pathname: '/index.html' }); + assert.deepEqual(outside.disabledRules, []); + }); + + it('resolves root and directory URLs to their index file', () => { + const ignores = { + roots: ['prototype/'], + ignoreValues: [ + { rule: 'dark-glow', value: '*', files: ['prototype/index.html'] }, + { rule: 'gradient-text', value: '*', files: ['prototype/news/index.html'] }, + ], + }; + assert.deepEqual(resolve({ ignores, pathname: '/' }).disabledRules, ['dark-glow']); + assert.deepEqual(resolve({ ignores, pathname: '/news/' }).disabledRules, ['gradient-text']); + }); + + it('matches path suffixes like the CLI scoped-file matcher', () => { + // findingMatchesScopedIgnoreFile tries every path suffix of the finding's + // file, so `library/**` written without the prototype/ prefix still + // scopes to the library pages. + const ignores = { + roots: ['prototype/'], + ignoreValues: [ + { rule: 'em-dash-overuse', value: '*', files: ['library/**'] }, + { rule: 'dark-glow', value: '*', files: ['buttons.html'] }, + ], + }; + const out = resolve({ ignores, pathname: '/library/buttons.html' }); + assert.deepEqual(out.disabledRules.sort(), ['dark-glow', 'em-dash-overuse']); + }); + + it('supports the CLI glob dialect, including alternation', () => { + const ignores = { + roots: ['prototype/'], + ignoreValues: [ + { rule: 'dark-glow', value: '*', files: ['prototype/{index,about}.html'] }, + { rule: 'gradient-text', value: '*', files: ['prototype/page-?.html'] }, + ], + }; + assert.deepEqual(resolve({ ignores, pathname: '/about.html' }).disabledRules, ['dark-glow']); + assert.deepEqual(resolve({ ignores, pathname: '/page-3.html' }).disabledRules, ['gradient-text']); + assert.deepEqual(resolve({ ignores, pathname: '/page-33.html' }).disabledRules, []); + }); + + it('treats glob metacharacters in filenames literally', () => { + const ignores = { + ignoreValues: [ + { rule: 'dark-glow', value: '*', files: ['pricing (v2).html'] }, + ], + }; + const out = resolve({ ignores, pathname: '/pricing (v2).html' }); + assert.deepEqual(out.disabledRules, ['dark-glow']); + const near = resolve({ ignores, pathname: '/pricing xv2y.html' }); + assert.deepEqual(near.disabledRules, []); + }); + + it('accepts a single `file` string alongside `files`', () => { + const out = resolve({ + ignores: { + ignoreValues: [{ rule: 'dark-glow', value: '*', file: 'index.html' }], + }, + pathname: '/index.html', + }); + assert.deepEqual(out.disabledRules, ['dark-glow']); + }); + + it('survives malformed roots and percent-escapes without throwing', () => { + const out = resolve({ + ignores: { + roots: 7, + ignoreRules: ['dark-glow'], + ignoreValues: [{ rule: 'gradient-text', value: '*', files: ['broken%.html'] }], + }, + pathname: '/broken%.html', + }); + assert.deepEqual(out.disabledRules.sort(), ['dark-glow', 'gradient-text']); + }); +}); diff --git a/tests/live-browser-regression.test.mjs b/tests/live-browser-regression.test.mjs index 6be47eea7..5c984c47f 100644 --- a/tests/live-browser-regression.test.mjs +++ b/tests/live-browser-regression.test.mjs @@ -714,8 +714,13 @@ describe('live-browser.js regression guards', () => { ); assert.match( SOURCE, - /function requestDetectScan\(\)[\s\S]{0,240}?const scanId = String\(\+\+detectScanSeq\);[\s\S]{0,80}?activeDetectScanId = scanId;[\s\S]{0,160}?config: \{ scanId \}/, - 'Detect scans must send a fresh scan id to the detector', + /function requestDetectScan\(\)[\s\S]{0,240}?const scanId = String\(\+\+detectScanSeq\);[\s\S]{0,80}?activeDetectScanId = scanId;[\s\S]{0,1400}?config: \{ scanId, disabledRules: ignores\.disabledRules, disabledValues: ignores\.disabledValues \}/, + 'Detect scans must send a fresh scan id plus the resolved project waivers to the detector', + ); + assert.match( + SOURCE, + /typeof ignoresApi\?\.resolveDetectIgnores === 'function'[\s\S]{0,300}?: \{ disabledRules: \[\], disabledValues: \[\] \}/, + 'a cached live.js without the ignores resolver part must still scan, just unfiltered', ); assert.match( SOURCE, diff --git a/tests/live-browser-script-parts.test.mjs b/tests/live-browser-script-parts.test.mjs index b8db88631..930f76fc9 100644 --- a/tests/live-browser-script-parts.test.mjs +++ b/tests/live-browser-script-parts.test.mjs @@ -12,13 +12,15 @@ describe('live browser script parts', () => { it('resolves the canonical browser script order', () => { const parts = resolveLiveBrowserScriptParts('/repo/skill/scripts'); - assert.deepEqual(parts.map((part) => part.name), ['session-state', 'dom-helpers', 'browser-ui']); + assert.deepEqual(parts.map((part) => part.name), ['session-state', 'dom-helpers', 'project-ignores', 'browser-ui']); assert.equal(parts[0].file, 'live-browser-session.js'); assert.equal(parts[1].file, 'live-browser-dom.js'); - assert.equal(parts[2].file, 'live-browser.js'); + assert.equal(parts[2].file, 'live-browser-ignores.js'); + assert.equal(parts[3].file, 'live-browser.js'); assert.equal(parts[0].path, path.join('/repo/skill/scripts', 'live-browser-session.js')); assert.equal(parts[1].path, path.join('/repo/skill/scripts', 'live-browser-dom.js')); - assert.equal(parts[2].path, path.join('/repo/skill/scripts', 'live-browser.js')); + assert.equal(parts[2].path, path.join('/repo/skill/scripts', 'live-browser-ignores.js')); + assert.equal(parts[3].path, path.join('/repo/skill/scripts', 'live-browser.js')); }); it('asserts missing script parts by name', () => { @@ -37,6 +39,7 @@ describe('live browser script parts', () => { assert.deepEqual(loaded.map((part) => part.source), [ 'source:live-browser-session.js', 'source:live-browser-dom.js', + 'source:live-browser-ignores.js', 'source:live-browser.js', ]); }); @@ -50,16 +53,20 @@ describe('live browser script parts', () => { parts: [ { name: 'session-state', file: 'live-browser-session.js', source: 'window.__SESSION_PART__ = true;' }, { name: 'dom-helpers', file: 'live-browser-dom.js', source: 'window.__DOM_PART__ = true;' }, + { name: 'project-ignores', file: 'live-browser-ignores.js', source: 'window.__IGNORES_PART__ = true;' }, { name: 'browser-ui', file: 'live-browser.js', source: 'window.__BROWSER_PART__ = true;' }, ], + projectIgnores: { ignoreRules: ['dark-glow'], ignoreValues: [], roots: ['prototype/'] }, }); const tokenIndex = script.indexOf('window.__IMPECCABLE_TOKEN__'); const portIndex = script.indexOf('window.__IMPECCABLE_PORT__'); const commandPrefixIndex = script.indexOf('window.__IMPECCABLE_COMMAND_PREFIX__'); const vocabIndex = script.indexOf('window.__IMPECCABLE_VOCAB__'); + const projectIgnoresIndex = script.indexOf('window.__IMPECCABLE_PROJECT_IGNORES__'); const sessionIndex = script.indexOf('window.__SESSION_PART__'); const domIndex = script.indexOf('window.__DOM_PART__'); + const ignoresIndex = script.indexOf('window.__IGNORES_PART__'); const browserIndex = script.indexOf('window.__BROWSER_PART__'); assert.ok(tokenIndex !== -1); @@ -67,11 +74,26 @@ describe('live browser script parts', () => { assert.ok(portIndex < commandPrefixIndex); assert.ok(commandPrefixIndex < vocabIndex); assert.match(script, /window\.__IMPECCABLE_COMMAND_PREFIX__ = "\$"/); - assert.ok(vocabIndex < sessionIndex); + assert.ok(vocabIndex < projectIgnoresIndex); + assert.ok(projectIgnoresIndex < sessionIndex); assert.ok(sessionIndex < domIndex); - assert.ok(domIndex < browserIndex); + assert.ok(domIndex < ignoresIndex); + assert.ok(ignoresIndex < browserIndex); + assert.match(script, /window\.__IMPECCABLE_PROJECT_IGNORES__ = \{"ignoreRules":\["dark-glow"\],"ignoreValues":\[\],"roots":\["prototype\/"\]\};/); assert.match(script, /impeccable live script part: session-state \(live-browser-session\.js\)/); assert.match(script, /impeccable live script part: dom-helpers \(live-browser-dom\.js\)/); + assert.match(script, /impeccable live script part: project-ignores \(live-browser-ignores\.js\)/); assert.match(script, /impeccable live script part: browser-ui \(live-browser\.js\)/); }); + + it('serializes null project ignores when none are passed', () => { + const script = assembleLiveBrowserScript({ + token: 'token-a', + port: 8421, + vocabulary: [], + parts: [], + }); + + assert.match(script, /window\.__IMPECCABLE_PROJECT_IGNORES__ = null;/); + }); });