diff --git a/cli/engine/browser/injected/index.mjs b/cli/engine/browser/injected/index.mjs index ed91c8fb1..0d0c26a16 100644 --- a/cli/engine/browser/injected/index.mjs +++ b/cli/engine/browser/injected/index.mjs @@ -1473,6 +1473,14 @@ if (IS_BROWSER) { } function collectBrowserFindings() { + // A page matched by detector.ignoreFiles is waived wholesale: answer the + // scan with the empty shape so the badge and toast read zero. Mirrors + // shouldIgnoreDetectionFile in cli/lib/impeccable-config.mjs; the live + // overlay resolves the globs per page (live-browser-ignores.js) and + // forwards the verdict as config.skipScan. + if (EXTENSION_MODE && window.__IMPECCABLE_CONFIG__?.skipScan === true) { + return { groupMap: new Map(), allFindings: [], pageLevelFindings: [] }; + } const groupMap = new Map(); const _disabled = EXTENSION_MODE ? (window.__IMPECCABLE_CONFIG__?.disabledRules || []) : []; const _ruleOk = (id) => !_disabled.length || !_disabled.includes(id); @@ -1702,15 +1710,20 @@ if (IS_BROWSER) { ]); // 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). + // sentence. One CLI matcher is not mirrored here: the motion extractor + // (a value-scoped bounce-easing waiver only matches when the finding + // carries ignoreValue directly). The CLI's [?&]family= URL fallback is + // also omitted on purpose: browser findings for these rules always + // carry ignoreValue or a "Primary font:" / "Google Fonts:" / + // font-family sentence, so it is unreachable here. const _findingValue = (f) => { if (!f || !_directValueRules.has(f.type || f.id)) return ''; const direct = f.ignoreValue || f.value; if (direct) return _normValue(direct); + // The CLI routes bounce-easing through extractMotionIgnoreValue and + // never the font regexes; without a direct ignoreValue there is no + // value to match, so do not invent one from unrelated CSS text. + if ((f.type || f.id) === 'bounce-easing') return ''; for (const text of [f.detail, f.snippet]) { if (typeof text !== 'string' || !text) continue; const primary = text.match(/Primary font:\s*([^()\n;]+)/i); @@ -1722,11 +1735,56 @@ if (IS_BROWSER) { } return ''; }; + // design-system-color compares by color value, not by spelling: the + // browser reports computed rgb(...) strings while waivers are usually + // written as hex. Mirrors ignoreValueMatches -> colorIgnoreKey in + // cli/lib/impeccable-config.mjs for the hex and rgb()/rgba() forms; + // hsl stays CLI-only. + const _colorKey = (value) => { + const text = String(value || '').trim().toLowerCase(); + const hex = text.match(/^#([0-9a-f]{3,4}|[0-9a-f]{6}|[0-9a-f]{8})$/); + if (hex) { + const expanded = hex[1].length <= 4 ? [...hex[1]].map(d => d + d).join('') : hex[1]; + const [r, g, b, a = 255] = expanded.match(/../g).map(ch => parseInt(ch, 16)); + return `${r},${g},${b},${a}`; + } + const rgb = text.match(/^rgba?\((.*)\)$/); + if (!rgb) return ''; + const body = rgb[1].trim().replace(/\s*\/\s*/g, ' / '); + let parts; + if (body.includes(',')) { + parts = body.split(',').map(p => p.trim()).filter(Boolean); + const last = parts[parts.length - 1]; + if (last && last.includes('/')) { + parts = [...parts.slice(0, -1), ...last.split('/').map(p => p.trim()).filter(Boolean)]; + } + } else { + parts = body.split(/\s+/).filter(p => p && p !== '/'); + } + if (parts.length < 3 || parts.length > 4) return ''; + const channel = (raw, isAlpha) => { + const m = String(raw).trim().match(/^(-?\d*\.?\d+)(%)?$/); + if (!m) return null; + let v = parseFloat(m[1]); + if (m[2]) v = isAlpha ? v / 100 : v * 2.55; + const max = isAlpha ? 1 : 255; + if (!Number.isFinite(v) || v < 0 || v > max) return null; + return isAlpha ? v : Math.round(v); + }; + const r = channel(parts[0], false); + const g = channel(parts[1], false); + const b = channel(parts[2], false); + const a = parts[3] === undefined ? 1 : channel(parts[3], true); + if ([r, g, b, a].some(v => v === null)) return ''; + return `${r},${g},${b},${Math.round(a * 255)}`; + }; 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); + return _disabledValues.some(e => e.rule === rule && (e.value === value + || (rule === 'design-system-color' + && _colorKey(e.value) !== '' && _colorKey(e.value) === _colorKey(value)))); }; for (const [el, list] of [...groupMap.entries()]) { const kept = list.filter(f => !_valueIgnored(f)); diff --git a/cli/engine/detect-antipatterns-browser.js b/cli/engine/detect-antipatterns-browser.js index f40a768f7..281e49810 100644 --- a/cli/engine/detect-antipatterns-browser.js +++ b/cli/engine/detect-antipatterns-browser.js @@ -8132,6 +8132,14 @@ if (IS_BROWSER) { } function collectBrowserFindings() { + // A page matched by detector.ignoreFiles is waived wholesale: answer the + // scan with the empty shape so the badge and toast read zero. Mirrors + // shouldIgnoreDetectionFile in cli/lib/impeccable-config.mjs; the live + // overlay resolves the globs per page (live-browser-ignores.js) and + // forwards the verdict as config.skipScan. + if (EXTENSION_MODE && window.__IMPECCABLE_CONFIG__?.skipScan === true) { + return { groupMap: new Map(), allFindings: [], pageLevelFindings: [] }; + } const groupMap = new Map(); const _disabled = EXTENSION_MODE ? (window.__IMPECCABLE_CONFIG__?.disabledRules || []) : []; const _ruleOk = (id) => !_disabled.length || !_disabled.includes(id); @@ -8361,15 +8369,20 @@ if (IS_BROWSER) { ]); // 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). + // sentence. One CLI matcher is not mirrored here: the motion extractor + // (a value-scoped bounce-easing waiver only matches when the finding + // carries ignoreValue directly). The CLI's [?&]family= URL fallback is + // also omitted on purpose: browser findings for these rules always + // carry ignoreValue or a "Primary font:" / "Google Fonts:" / + // font-family sentence, so it is unreachable here. const _findingValue = (f) => { if (!f || !_directValueRules.has(f.type || f.id)) return ''; const direct = f.ignoreValue || f.value; if (direct) return _normValue(direct); + // The CLI routes bounce-easing through extractMotionIgnoreValue and + // never the font regexes; without a direct ignoreValue there is no + // value to match, so do not invent one from unrelated CSS text. + if ((f.type || f.id) === 'bounce-easing') return ''; for (const text of [f.detail, f.snippet]) { if (typeof text !== 'string' || !text) continue; const primary = text.match(/Primary font:\s*([^()\n;]+)/i); @@ -8381,11 +8394,56 @@ if (IS_BROWSER) { } return ''; }; + // design-system-color compares by color value, not by spelling: the + // browser reports computed rgb(...) strings while waivers are usually + // written as hex. Mirrors ignoreValueMatches -> colorIgnoreKey in + // cli/lib/impeccable-config.mjs for the hex and rgb()/rgba() forms; + // hsl stays CLI-only. + const _colorKey = (value) => { + const text = String(value || '').trim().toLowerCase(); + const hex = text.match(/^#([0-9a-f]{3,4}|[0-9a-f]{6}|[0-9a-f]{8})$/); + if (hex) { + const expanded = hex[1].length <= 4 ? [...hex[1]].map(d => d + d).join('') : hex[1]; + const [r, g, b, a = 255] = expanded.match(/../g).map(ch => parseInt(ch, 16)); + return `${r},${g},${b},${a}`; + } + const rgb = text.match(/^rgba?\((.*)\)$/); + if (!rgb) return ''; + const body = rgb[1].trim().replace(/\s*\/\s*/g, ' / '); + let parts; + if (body.includes(',')) { + parts = body.split(',').map(p => p.trim()).filter(Boolean); + const last = parts[parts.length - 1]; + if (last && last.includes('/')) { + parts = [...parts.slice(0, -1), ...last.split('/').map(p => p.trim()).filter(Boolean)]; + } + } else { + parts = body.split(/\s+/).filter(p => p && p !== '/'); + } + if (parts.length < 3 || parts.length > 4) return ''; + const channel = (raw, isAlpha) => { + const m = String(raw).trim().match(/^(-?\d*\.?\d+)(%)?$/); + if (!m) return null; + let v = parseFloat(m[1]); + if (m[2]) v = isAlpha ? v / 100 : v * 2.55; + const max = isAlpha ? 1 : 255; + if (!Number.isFinite(v) || v < 0 || v > max) return null; + return isAlpha ? v : Math.round(v); + }; + const r = channel(parts[0], false); + const g = channel(parts[1], false); + const b = channel(parts[2], false); + const a = parts[3] === undefined ? 1 : channel(parts[3], true); + if ([r, g, b, a].some(v => v === null)) return ''; + return `${r},${g},${b},${Math.round(a * 255)}`; + }; 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); + return _disabledValues.some(e => e.rule === rule && (e.value === value + || (rule === 'design-system-color' + && _colorKey(e.value) !== '' && _colorKey(e.value) === _colorKey(value)))); }; for (const [el, list] of [...groupMap.entries()]) { const kept = list.filter(f => !_valueIgnored(f)); diff --git a/scripts/test-suites.mjs b/scripts/test-suites.mjs index 5d2d1f227..5be924d3e 100644 --- a/scripts/test-suites.mjs +++ b/scripts/test-suites.mjs @@ -153,6 +153,7 @@ export const SUITES = { 'tests/live-insert-ui.test.mjs', 'tests/live-manual-edits-buffer.test.mjs', 'tests/live-poll.test.mjs', + 'tests/live-project-ignores.test.mjs', 'tests/live-poll-lanes.test.mjs', 'tests/live-poll-stream.test.mjs', 'tests/live-recovery-commands.test.mjs', diff --git a/skill/scripts/live-browser-ignores.js b/skill/scripts/live-browser-ignores.js index 92df1132b..1c8514c7b 100644 --- a/skill/scripts/live-browser-ignores.js +++ b/skill/scripts/live-browser-ignores.js @@ -16,6 +16,20 @@ * 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. + * 4. `ignoreFiles` globs that name the page waive it wholesale: the + * resolver reports `skipScan: true` and the detector answers the scan + * with zero findings, mirroring shouldIgnoreDetectionFile in the CLI + * and the edit hook's own ignoreFiles gate. + * + * `pageFiles`, when the server could resolve it, lists the real project + * files the inject config serves. A URL that suffix-matches exactly one of + * them takes that file as its only project identity; an ambiguous or absent + * match falls back to the served-root common ancestor below. + * + * Known gap, unchanged from PR #645: framework apps inject into source files + * (src/routes/about/+page.svelte) while scans see route URLs (/about), so + * entries scoped to source or asset paths never match a page candidate and + * are dropped. That shows the finding, which is the conservative direction. * * 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 @@ -101,7 +115,7 @@ // alternatives at all, and demanding a waiver match under both stops // prototype/index.html from applying anywhere. When the globs share no // common root, no prefix is asserted and only the URL path itself matches. - function pageCandidates(pathname, roots) { + function pageCandidates(pathname, roots, pageFiles) { let pagePath = String(pathname || ''); try { pagePath = decodeURIComponent(pagePath); @@ -113,6 +127,32 @@ // name files. Without this, /news/ never matches prototype/news/index.html. if (pagePath === '' || pagePath.endsWith('/')) pagePath += 'index.html'; + const candidates = new Set(); + const addSuffixes = (fullPath) => { + const parts = fullPath.split('/').filter(Boolean); + for (let i = 0; i < parts.length; i++) { + candidates.add(parts.slice(i).join('/')); + } + }; + addSuffixes(pagePath); + + // The served page list names the real files the inject config serves. + // A URL that suffix-matches exactly one of them has an unambiguous + // project identity; assert that identity and stop guessing from roots + // (PR #645 review: with src/ and public/ both served, /foo.html must not + // borrow src/foo.html's waivers while actually serving public/foo.html). + // Zero matches or several fall through to the common-ancestor fallback: + // ambiguity resolves toward showing the finding. + const knownPages = []; + for (const entry of Array.isArray(pageFiles) ? pageFiles : []) { + if (typeof entry !== 'string' || !entry) continue; + if (entry === pagePath || entry.endsWith('/' + pagePath)) knownPages.push(entry); + } + if (knownPages.length === 1) { + addSuffixes(knownPages[0]); + return [...candidates]; + } + const prefixes = []; for (const entry of Array.isArray(roots) ? roots : []) { if (typeof entry !== 'string') continue; @@ -125,14 +165,6 @@ common = common.slice(0, i); } - const candidates = new Set(); - const addSuffixes = (fullPath) => { - const parts = fullPath.split('/').filter(Boolean); - for (let i = 0; i < parts.length; i++) { - candidates.add(parts.slice(i).join('/')); - } - }; - addSuffixes(pagePath); if (common.length > 0) addSuffixes(common.join('/') + '/' + pagePath); return [...candidates]; } @@ -158,12 +190,21 @@ * 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}> }} + * @returns {{ disabledRules: string[], disabledValues: Array<{rule: string, value: string}>, skipScan: boolean }} */ 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 candidates = pageCandidates(pathname, config.roots, config.pageFiles); + + // detector.ignoreFiles waives whole files. When any glob names this + // page, the scan itself is skipped; rule and value lists are returned + // empty because nothing will run. + const ignoreFileGlobs = asArray(config.ignoreFiles) + .filter((glob) => typeof glob === 'string' && glob.trim()); + if (ignoreFileGlobs.length > 0 && matchesScope(ignoreFileGlobs, candidates)) { + return { disabledRules: [], disabledValues: [], skipScan: true }; + } const disabledRules = new Set( asArray(config.ignoreRules) @@ -191,7 +232,7 @@ disabledValues.push({ rule, value }); } - return { disabledRules: [...disabledRules], disabledValues }; + return { disabledRules: [...disabledRules], disabledValues, skipScan: false }; } root.__IMPECCABLE_LIVE_IGNORES__ = { diff --git a/skill/scripts/live-browser.js b/skill/scripts/live-browser.js index e9435f8c2..2b38ec62e 100644 --- a/skill/scripts/live-browser.js +++ b/skill/scripts/live-browser.js @@ -11147,20 +11147,32 @@ void main() { // 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. + // rule in the files they name, ignoreFiles that name the page skip the + // scan wholesale, and the rest match on the finding's own value inside + // the detector. Guarded twice: a stale cached live.js without the + // resolver part still scans, and a resolver that throws must not brick + // the detect toggle; both degrade to an unfiltered scan. const ignoresApi = window.__IMPECCABLE_LIVE_IGNORES__; - const ignores = typeof ignoresApi?.resolveDetectIgnores === 'function' - ? ignoresApi.resolveDetectIgnores({ - ignores: window.__IMPECCABLE_PROJECT_IGNORES__, - pathname: location.pathname, - }) - : { disabledRules: [], disabledValues: [] }; + let ignores = { disabledRules: [], disabledValues: [], skipScan: false }; + if (typeof ignoresApi?.resolveDetectIgnores === 'function') { + try { + ignores = ignoresApi.resolveDetectIgnores({ + ignores: window.__IMPECCABLE_PROJECT_IGNORES__, + pathname: location.pathname, + }) || ignores; + } catch (e) { + ignores = { disabledRules: [], disabledValues: [], skipScan: false }; + } + } window.postMessage({ source: 'impeccable-command', action: 'scan', - config: { scanId, disabledRules: ignores.disabledRules, disabledValues: ignores.disabledValues }, + config: { + scanId, + disabledRules: ignores.disabledRules || [], + disabledValues: ignores.disabledValues || [], + skipScan: ignores.skipScan === true, + }, }, '*'); } diff --git a/skill/scripts/live-server.mjs b/skill/scripts/live-server.mjs index ab298843b..ebdb8f4d8 100644 --- a/skill/scripts/live-server.mjs +++ b/skill/scripts/live-server.mjs @@ -22,7 +22,6 @@ 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, @@ -46,10 +45,10 @@ import { readLiveServerInfo, removeLiveServerInfo, resolveDesignSidecarPath, - resolveLiveConfigPath, writeLiveServerInfo, } from './lib/impeccable-paths.mjs'; import { countByPage as countPendingByPage } from './live/manual-edits-buffer.mjs'; +import { collectProjectDetectorIgnores } from './live/project-ignores.mjs'; import { createManualApplyController, summarizeManualApplyFailures, @@ -696,51 +695,6 @@ 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}`); @@ -802,8 +756,16 @@ function createRequestHandler({ detectScript, liveScriptParts }) { 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(), + // reloading the tab is enough to pick up a new waiver. Config comes + // from every root the session spans (appRoot, contextRoot, repoRoot): + // in a monorepo the hook and the CLI key it at the repo root, which + // is not the appRoot this process chdir'd onto. + projectIgnores: collectProjectDetectorIgnores({ + appRoot: process.cwd(), + contextRoot: LIVE_ROOTS?.contextRoot, + repoRoot: LIVE_ROOTS?.repoRoot, + scriptsDir: __dirname, + }), }); res.writeHead(200, { 'Content-Type': 'application/javascript', diff --git a/skill/scripts/live/project-ignores.mjs b/skill/scripts/live/project-ignores.mjs new file mode 100644 index 000000000..e99acaed1 --- /dev/null +++ b/skill/scripts/live/project-ignores.mjs @@ -0,0 +1,139 @@ +/** + * Project detector waivers for the live overlay (issue #639, hardened in the + * PR #645 follow-up). One place decides what the /live.js prelude serializes + * as window.__IMPECCABLE_PROJECT_IGNORES__: + * + * ignoreRules detector.ignoreRules, unioned across every live root. + * ignoreValues detector.ignoreValues entries ({rule, value, files?}), + * deduped across roots; createdAt/reason stay local. + * ignoreFiles detector.ignoreFiles globs, unioned across roots, so a + * wholly waived page scans to zero findings in the overlay + * just as it reports nothing through the CLI and the hook. + * roots served-root prefixes derived from the inject config's own + * `files` globs. Never derived from the ignore globs: one + * entry scoped to prototype/library/** would lend + * prototype/library/ as a candidate prefix to every page, + * and that rule would suppress site-wide (issue #639). + * pageFiles the inject config's `files` expanded to real project + * files, so the browser can resolve a URL to the one file it + * actually serves instead of trying every root (PR #645 + * review: with src/ and public/ both served, /foo.html must + * not borrow src/foo.html's waivers while actually serving + * public/foo.html). + * + * Config is read from every root the live session spans: the appRoot the + * server chdir'd onto, plus contextRoot and repoRoot when they differ. The + * edit hook keys the same config at the session cwd (the repo root in a + * monorepo, via resolveCacheCwd), and `impeccable detect` reads it from its + * invocation cwd, so reading only the appRoot silently dropped every waiver + * in exactly the monorepo layouts the roots manifest exists for. Reading is + * additive across roots, matching readConfig's own union of config.json and + * config.local.json. + * + * In a monorepo, roots and pageFiles are serialized repo-relative (the + * appRoot's path inside the repo is prefixed), so waivers spelled from + * either root match through the resolver's suffix expansion. + */ +import fs from 'node:fs'; +import path from 'node:path'; +import { readConfig } from '../hook-lib.mjs'; +import { resolveFiles } from '../live-inject.mjs'; +import { resolveLiveConfigPath } from '../lib/impeccable-paths.mjs'; + +// Serializing thousands of page identities into every /live.js response +// helps nobody; past this cap pageFiles is omitted and the resolver falls +// back to the served-root common ancestor, which is correct, just less +// precise about cross-root duplicates. +const PAGE_FILES_CAP = 500; + +export function collectProjectDetectorIgnores({ appRoot, contextRoot, repoRoot, scriptsDir } = {}) { + const configRoots = []; + for (const dir of [appRoot, contextRoot, repoRoot]) { + if (typeof dir !== 'string' || !dir) continue; + const resolved = path.resolve(dir); + if (!configRoots.includes(resolved)) configRoots.push(resolved); + } + if (configRoots.length === 0) configRoots.push(process.cwd()); + + const ignoreRules = new Set(); + const ignoreFiles = new Set(); + const valueEntries = new Map(); + for (const dir of configRoots) { + // 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 = readConfig(dir); + for (const rule of Array.isArray(config.ignoreRules) ? config.ignoreRules : []) { + if (typeof rule === 'string' && rule.trim()) ignoreRules.add(rule); + } + for (const glob of Array.isArray(config.ignoreFiles) ? config.ignoreFiles : []) { + if (typeof glob === 'string' && glob.trim()) ignoreFiles.add(glob); + } + for (const entry of Array.isArray(config.ignoreValues) ? config.ignoreValues : []) { + if (!entry || typeof entry !== 'object') continue; + // readConfig already normalized rule/value and folded `file` into + // `files`; serve only what the browser matches on. + const serialized = { + rule: entry.rule, + value: entry.value, + ...(Array.isArray(entry.files) && entry.files.length > 0 ? { files: entry.files } : {}), + }; + const key = JSON.stringify([serialized.rule, serialized.value, + Array.isArray(serialized.files) ? [...serialized.files].sort() : []]); + if (!valueEntries.has(key)) valueEntries.set(key, serialized); + } + } + + const served = readLiveServedPages({ appRoot: configRoots[0], repoRoot, scriptsDir }); + return { + ignoreRules: [...ignoreRules], + ignoreValues: [...valueEntries.values()], + ignoreFiles: [...ignoreFiles], + roots: served.roots, + pageFiles: served.pageFiles, + }; +} + +function readLiveServedPages({ appRoot, repoRoot, scriptsDir }) { + let live = null; + try { + const configPath = resolveLiveConfigPath({ cwd: appRoot, scriptsDir }); + live = JSON.parse(fs.readFileSync(configPath, 'utf-8')); + } catch { + // No readable inject config: the browser matches URL paths as-is. + return { roots: [], pageFiles: [] }; + } + const files = Array.isArray(live?.files) + ? live.files.filter((glob) => typeof glob === 'string' && glob) + : []; + + // A monorepo appRoot serializes identities repo-relative, so waivers + // spelled from either root match through the resolver's suffix expansion. + let prefix = ''; + if (typeof repoRoot === 'string' && repoRoot) { + const rel = path.relative(path.resolve(repoRoot), path.resolve(appRoot)).split(path.sep).join('/'); + if (rel && !rel.startsWith('..') && !path.isAbsolute(rel)) prefix = `${rel}/`; + } + + const roots = [...new Set(files.map((glob) => { + const wildcardAt = glob.search(/[*?{]/); + const head = wildcardAt === -1 ? glob : glob.slice(0, wildcardAt); + const cut = head.lastIndexOf('/'); + return prefix + (cut > -1 ? head.slice(0, cut + 1) : ''); + }))]; + + let pageFiles = []; + try { + pageFiles = resolveFiles(appRoot, { ...live, files }) + .filter((rel) => { + // resolveFiles passes literal entries through even when they do not + // exist; a missing file is nobody's identity. + try { return fs.statSync(path.join(appRoot, rel)).isFile(); } catch { return false; } + }) + .map((rel) => prefix + rel); + } catch { + pageFiles = []; + } + if (pageFiles.length > PAGE_FILES_CAP) pageFiles = []; + + return { roots, pageFiles }; +} diff --git a/tests/detect-antipatterns-browser.test.mjs b/tests/detect-antipatterns-browser.test.mjs index ffffd3bd7..0ac7fe05e 100644 --- a/tests/detect-antipatterns-browser.test.mjs +++ b/tests/detect-antipatterns-browser.test.mjs @@ -1044,7 +1044,7 @@ describe('detectUrl — browser-only fixtures', () => { }); }); await page.evaluate(browserScript); - const scan = (scanId, disabledValues) => page.evaluate(async (config) => { + const scan = (scanId, disabledValues, extraConfig = {}) => page.evaluate(async (config) => { window.postMessage({ source: 'impeccable-command', action: 'scan', config }, '*'); const deadline = Date.now() + 2000; while ( @@ -1060,11 +1060,14 @@ describe('detectUrl — browser-only fixtures', () => { return { total: flat.length, colors: flat.filter(finding => finding.type === 'design-system-color').length, + colorValues: flat + .filter(finding => finding.type === 'design-system-color') + .map(finding => finding.ignoreValue || ''), fonts: flat .filter(finding => finding.type === 'design-system-font') .map(finding => finding.ignoreValue || ''), }; - }, { scanId, visualContrast: false, designSystem, ...(disabledValues ? { disabledValues } : {}) }); + }, { scanId, visualContrast: false, designSystem, ...(disabledValues ? { disabledValues } : {}), ...extraConfig }); const unfiltered = await scan('scan-dv-1'); assert.ok( @@ -1089,6 +1092,39 @@ describe('detectUrl — browser-only fixtures', () => { unfiltered.colors, `expected unrelated design-system findings to survive, got: ${JSON.stringify({ unfiltered, filtered })}`, ); + + // Color waivers match by value, not by spelling: the browser reports + // computed rgb(...) strings, the waiver is written as hex (mirrors + // ignoreValueMatches -> colorIgnoreKey in cli/lib/impeccable-config.mjs). + const rgbToHex = (value) => { + const m = String(value).match(/^rgb\((\d+),\s*(\d+),\s*(\d+)\)$/i); + if (!m) return null; + return `#${[m[1], m[2], m[3]].map(n => Number(n).toString(16).padStart(2, '0')).join('')}`; + }; + const rgbColor = unfiltered.colorValues.find(value => rgbToHex(value)); + assert.ok( + rgbColor, + `expected an rgb()-reported design-system-color finding, got: ${JSON.stringify(unfiltered.colorValues)}`, + ); + const hexWaiver = rgbToHex(rgbColor); + const colorFiltered = await scan('scan-dv-3', [{ rule: 'design-system-color', value: hexWaiver }]); + const waivedColorCount = unfiltered.colorValues.filter(value => value === rgbColor).length; + assert.ok(waivedColorCount > 0); + assert.equal( + colorFiltered.colors, + unfiltered.colors - waivedColorCount, + `expected the hex waiver ${hexWaiver} to suppress the ${rgbColor} findings, got: ${JSON.stringify({ colorValues: unfiltered.colorValues, colorFiltered })}`, + ); + assert.equal( + colorFiltered.fonts.some(value => /poppins/i.test(value)), + true, + `expected unrelated font findings to survive the color waiver, got: ${JSON.stringify(colorFiltered)}`, + ); + + // A page waived wholesale by detector.ignoreFiles arrives with + // config.skipScan and must scan to nothing at all. + const skipped = await scan('scan-dv-4', null, { skipScan: true }); + assert.equal(skipped.total, 0, `expected skipScan to empty the scan, got: ${JSON.stringify(skipped)}`); await page.close(); } finally { await browser.close().catch(() => {}); diff --git a/tests/live-browser-ignores.test.mjs b/tests/live-browser-ignores.test.mjs index 811eed6d7..46745fa3e 100644 --- a/tests/live-browser-ignores.test.mjs +++ b/tests/live-browser-ignores.test.mjs @@ -21,7 +21,7 @@ function loadIgnoresApi() { const resolve = loadIgnoresApi().resolveDetectIgnores; -const EMPTY = { disabledRules: [], disabledValues: [] }; +const EMPTY = { disabledRules: [], disabledValues: [], skipScan: false }; describe('live-browser-ignores resolver', () => { it('registers a versioned API on the root', () => { @@ -226,10 +226,106 @@ describe('live-browser-ignores resolver', () => { ); }); + it('resolves a URL to its one served file when pageFiles knows it', () => { + // PR #645 review discussion r3840011436: with src/ and public/ both + // served, /foo.html used to borrow identities from every root. The + // served page list disambiguates: this URL serves public/foo.html, so + // src-scoped waivers must not apply. + const ignores = { + roots: ['src/', 'public/'], + pageFiles: ['src/other.html', 'public/foo.html'], + ignoreValues: [ + { rule: 'dark-glow', value: '*', files: ['src/foo.html'] }, + { rule: 'gradient-text', value: '*', files: ['public/foo.html'] }, + ], + }; + const out = resolve({ ignores, pathname: '/foo.html' }); + assert.deepEqual(out.disabledRules, ['gradient-text']); + }); + + it('keeps ambiguity conservative when served files share the URL suffix', () => { + const ignores = { + roots: ['src/', 'public/'], + pageFiles: ['src/foo.html', 'public/foo.html'], + ignoreValues: [ + { rule: 'dark-glow', value: '*', files: ['src/foo.html'] }, + { rule: 'gradient-text', value: '*', files: ['public/foo.html'] }, + { rule: 'em-dash-overuse', value: '*', files: ['foo.html'] }, + ], + }; + const out = resolve({ ignores, pathname: '/foo.html' }); + // Neither root-scoped waiver can claim the page; the bare spelling + // still applies whichever root serves it. + assert.deepEqual(out.disabledRules, ['em-dash-overuse']); + }); + + it('falls back to the common ancestor when index files collide across depths', () => { + // /index.html suffix-matches both served index files; the ambiguity + // resolves through the common ancestor, which still yields the correct + // shallow identity and never the deep one. + const ignores = { + roots: ['prototype/', 'prototype/library/'], + pageFiles: ['prototype/index.html', 'prototype/library/index.html'], + ignoreValues: [ + { rule: 'dark-glow', value: '*', files: ['prototype/index.html'] }, + { rule: 'gradient-text', value: '*', files: ['prototype/library/index.html'] }, + ], + }; + const out = resolve({ ignores, pathname: '/index.html' }); + assert.deepEqual(out.disabledRules, ['dark-glow']); + }); + + it('skips the scan on pages named by ignoreFiles', () => { + const ignores = { + roots: ['prototype/'], + ignoreFiles: ['prototype/library/**'], + ignoreRules: ['dark-glow'], + }; + const waived = resolve({ ignores, pathname: '/library/buttons.html' }); + assert.deepEqual(waived, { disabledRules: [], disabledValues: [], skipScan: true }); + const scanned = resolve({ ignores, pathname: '/index.html' }); + assert.equal(scanned.skipScan, false); + assert.deepEqual(scanned.disabledRules, ['dark-glow']); + }); + + it('matches ignoreFiles by basename like the CLI glob matcher', () => { + const out = resolve({ + ignores: { ignoreFiles: ['buttons.html'], roots: ['prototype/'] }, + pathname: '/library/buttons.html', + }); + assert.equal(out.skipScan, true); + }); + + it('treats a malformed ignoreFiles value as no waiver at all', () => { + const out = resolve({ + ignores: { ignoreFiles: 'prototype/**', roots: [] }, + pathname: '/index.html', + }); + assert.equal(out.skipScan, false); + }); + + it('drops entries scoped to source paths that no route URL can match', () => { + // Framework apps inject into source files while scans see route URLs; a + // source-scoped entry must fail conservative (finding shown), never + // suppress by accident. Pinned so a refactor cannot flip the direction. + const ignores = { + roots: ['src/'], + pageFiles: ['src/routes/about/+page.svelte'], + ignoreValues: [ + { rule: 'dark-glow', value: '*', files: ['src/routes/about/+page.svelte'] }, + ], + }; + const out = resolve({ ignores, pathname: '/about' }); + assert.deepEqual(out.disabledRules, []); + assert.equal(out.skipScan, false); + }); + it('survives malformed roots and percent-escapes without throwing', () => { const out = resolve({ ignores: { roots: 7, + pageFiles: 'not-a-list', + ignoreFiles: [null, 42], ignoreRules: ['dark-glow'], ignoreValues: [{ rule: 'gradient-text', value: '*', files: ['broken%.html'] }], }, diff --git a/tests/live-browser-regression.test.mjs b/tests/live-browser-regression.test.mjs index 5c984c47f..7796404e3 100644 --- a/tests/live-browser-regression.test.mjs +++ b/tests/live-browser-regression.test.mjs @@ -714,14 +714,19 @@ 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,1400}?config: \{ scanId, disabledRules: ignores\.disabledRules, disabledValues: ignores\.disabledValues \}/, + /function requestDetectScan\(\)[\s\S]{0,240}?const scanId = String\(\+\+detectScanSeq\);[\s\S]{0,80}?activeDetectScanId = scanId;[\s\S]{0,2200}?config: \{\s*scanId,\s*disabledRules: ignores\.disabledRules \|\| \[\],\s*disabledValues: ignores\.disabledValues \|\| \[\],\s*skipScan: ignores\.skipScan === true,\s*\},/, '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: \[\] \}/, + /let ignores = \{ disabledRules: \[\], disabledValues: \[\], skipScan: false \};\s*if \(typeof ignoresApi\?\.resolveDetectIgnores === 'function'\) \{\s*try \{/, 'a cached live.js without the ignores resolver part must still scan, just unfiltered', ); + assert.match( + SOURCE, + /\} catch \(e\) \{\s*ignores = \{ disabledRules: \[\], disabledValues: \[\], skipScan: false \};\s*\}/, + 'a throwing ignores resolver must degrade to an unfiltered scan, not break the detect toggle', + ); assert.match( SOURCE, /if \(!detectActive\) return;[\s\S]{0,80}?if \(activeDetectScanId && e\.data\.scanId !== activeDetectScanId\) return;/, diff --git a/tests/live-browser-script-parts.test.mjs b/tests/live-browser-script-parts.test.mjs index 930f76fc9..9c1a7a9ab 100644 --- a/tests/live-browser-script-parts.test.mjs +++ b/tests/live-browser-script-parts.test.mjs @@ -56,7 +56,7 @@ describe('live browser script parts', () => { { 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/'] }, + projectIgnores: { ignoreRules: ['dark-glow'], ignoreValues: [], ignoreFiles: [], roots: ['prototype/'], pageFiles: ['prototype/index.html'] }, }); const tokenIndex = script.indexOf('window.__IMPECCABLE_TOKEN__'); @@ -79,7 +79,7 @@ describe('live browser script parts', () => { assert.ok(sessionIndex < domIndex); assert.ok(domIndex < ignoresIndex); assert.ok(ignoresIndex < browserIndex); - assert.match(script, /window\.__IMPECCABLE_PROJECT_IGNORES__ = \{"ignoreRules":\["dark-glow"\],"ignoreValues":\[\],"roots":\["prototype\/"\]\};/); + assert.match(script, /window\.__IMPECCABLE_PROJECT_IGNORES__ = \{"ignoreRules":\["dark-glow"\],"ignoreValues":\[\],"ignoreFiles":\[\],"roots":\["prototype\/"\],"pageFiles":\["prototype\/index\.html"\]\};/); 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\)/); diff --git a/tests/live-project-ignores.test.mjs b/tests/live-project-ignores.test.mjs new file mode 100644 index 000000000..3669d63f2 --- /dev/null +++ b/tests/live-project-ignores.test.mjs @@ -0,0 +1,145 @@ +import { describe, it, after } from 'node:test'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { collectProjectDetectorIgnores } from '../skill/scripts/live/project-ignores.mjs'; + +const REPO_ROOT = path.join(path.dirname(fileURLToPath(import.meta.url)), '..'); +const SCRIPTS_DIR = path.join(REPO_ROOT, 'skill', 'scripts'); + +const tempDirs = []; +function makeTemp() { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'impeccable-project-ignores-')); + tempDirs.push(dir); + return dir; +} +after(() => { + for (const dir of tempDirs) { + try { fs.rmSync(dir, { recursive: true, force: true }); } catch { /* best effort */ } + } +}); + +function write(root, rel, content) { + const filePath = path.join(root, rel); + fs.mkdirSync(path.dirname(filePath), { recursive: true }); + fs.writeFileSync(filePath, content); +} + +function writeDetectorConfig(root, detector) { + write(root, '.impeccable/config.json', JSON.stringify({ detector }, null, 2)); +} + +function writeLiveConfig(root, files) { + write(root, '.impeccable/live/config.json', JSON.stringify({ + files, + insertBefore: '', + commentSyntax: 'html', + }, null, 2)); +} + +describe('collectProjectDetectorIgnores', () => { + it('collects waivers, roots, and pageFiles from a single-root project', () => { + const app = makeTemp(); + write(app, 'package.json', '{"name":"single","private":true}\n'); + writeDetectorConfig(app, { + ignoreRules: ['ai-color-palette'], + ignoreFiles: ['prototype/legacy/**'], + ignoreValues: [ + { rule: 'gradient-text', value: '*', files: ['prototype/library/**'], reason: 'stays local' }, + ], + }); + writeLiveConfig(app, ['prototype/index.html', 'prototype/library/buttons.html']); + write(app, 'prototype/index.html', ''); + write(app, 'prototype/library/buttons.html', ''); + + const out = collectProjectDetectorIgnores({ appRoot: app, scriptsDir: SCRIPTS_DIR }); + assert.deepEqual(out.ignoreRules, ['ai-color-palette']); + assert.deepEqual(out.ignoreFiles, ['prototype/legacy/**']); + // createdAt/reason stay local; only rule/value/files ride to the browser. + assert.deepEqual(out.ignoreValues, [ + { rule: 'gradient-text', value: '*', files: ['prototype/library/**'] }, + ]); + assert.deepEqual(out.roots.sort(), ['prototype/', 'prototype/library/']); + assert.deepEqual(out.pageFiles.sort(), ['prototype/index.html', 'prototype/library/buttons.html']); + }); + + it('reads waivers keyed at the repo root, where the hook and the CLI put them', () => { + // The monorepo shape from the PR #645 review: the live server chdirs + // onto the child appRoot, while resolveCacheCwd keys the hook's config + // at the session cwd, which is the repo root. + const repo = makeTemp(); + const app = path.join(repo, 'site'); + fs.mkdirSync(path.join(repo, '.git'), { recursive: true }); + write(app, 'package.json', '{"name":"site","private":true}\n'); + writeDetectorConfig(repo, { + ignoreRules: ['ai-color-palette'], + ignoreValues: [{ rule: 'overused-font', value: 'space grotesk' }], + }); + writeLiveConfig(app, ['prototype/index.html']); + write(app, 'prototype/index.html', ''); + + const out = collectProjectDetectorIgnores({ appRoot: app, repoRoot: repo, scriptsDir: SCRIPTS_DIR }); + assert.deepEqual(out.ignoreRules, ['ai-color-palette']); + assert.deepEqual(out.ignoreValues, [{ rule: 'overused-font', value: 'space grotesk' }]); + // Identities serialize repo-relative so waivers spelled from either root + // match through the resolver's suffix expansion. + assert.deepEqual(out.roots, ['site/prototype/']); + assert.deepEqual(out.pageFiles, ['site/prototype/index.html']); + }); + + it('unions configs across roots and dedupes identical value entries', () => { + const repo = makeTemp(); + const app = path.join(repo, 'site'); + fs.mkdirSync(path.join(repo, '.git'), { recursive: true }); + write(app, 'package.json', '{"name":"site","private":true}\n'); + writeDetectorConfig(repo, { + ignoreRules: ['ai-color-palette'], + ignoreValues: [{ rule: 'overused-font', value: 'space grotesk' }], + }); + writeDetectorConfig(app, { + ignoreRules: ['gradient-text', 'ai-color-palette'], + ignoreValues: [{ rule: 'overused-font', value: 'space grotesk' }], + }); + writeLiveConfig(app, ['prototype/index.html']); + write(app, 'prototype/index.html', ''); + + const out = collectProjectDetectorIgnores({ appRoot: app, repoRoot: repo, scriptsDir: SCRIPTS_DIR }); + assert.deepEqual(out.ignoreRules.sort(), ['ai-color-palette', 'gradient-text']); + assert.deepEqual(out.ignoreValues, [{ rule: 'overused-font', value: 'space grotesk' }]); + }); + + it('expands glob file entries to existing files and drops missing literals', () => { + const app = makeTemp(); + write(app, 'package.json', '{"name":"globs","private":true}\n'); + writeLiveConfig(app, ['prototype/**/*.html', 'prototype/not-created-yet.html']); + write(app, 'prototype/index.html', ''); + write(app, 'prototype/library/buttons.html', ''); + + const out = collectProjectDetectorIgnores({ appRoot: app, scriptsDir: SCRIPTS_DIR }); + assert.deepEqual(out.pageFiles.sort(), ['prototype/index.html', 'prototype/library/buttons.html']); + assert.deepEqual(out.roots.sort(), ['prototype/']); + }); + + it('degrades to empty arrays when nothing is configured', () => { + const app = makeTemp(); + write(app, 'package.json', '{"name":"bare","private":true}\n'); + const out = collectProjectDetectorIgnores({ appRoot: app, scriptsDir: SCRIPTS_DIR }); + assert.deepEqual(out, { ignoreRules: [], ignoreValues: [], ignoreFiles: [], roots: [], pageFiles: [] }); + }); + + it('survives a malformed detector config without throwing', () => { + const app = makeTemp(); + write(app, 'package.json', '{"name":"broken","private":true}\n'); + write(app, '.impeccable/config.json', '{"detector":{"ignoreRules":"foo","ignoreValues":[null,7],"ignoreFiles":{}}}'); + writeLiveConfig(app, ['prototype/index.html']); + write(app, 'prototype/index.html', ''); + + const out = collectProjectDetectorIgnores({ appRoot: app, scriptsDir: SCRIPTS_DIR }); + assert.deepEqual(out.ignoreRules, []); + assert.deepEqual(out.ignoreValues, []); + assert.deepEqual(out.ignoreFiles, []); + assert.deepEqual(out.pageFiles, ['prototype/index.html']); + }); +});