diff --git a/skill/scripts/live-browser-ignores.js b/skill/scripts/live-browser-ignores.js index 649302fa6..92df1132b 100644 --- a/skill/scripts/live-browser-ignores.js +++ b/skill/scripts/live-browser-ignores.js @@ -90,12 +90,17 @@ // findingMatchesScopedIgnoreFile in cli/lib/impeccable-config.mjs (which // matches globs against every path suffix of the finding's file). // - // With several roots configured, one URL has several possible identities - // and the browser cannot tell which root actually serves it. The rooted - // groups are therefore kept separate: matchesScope treats a root-prefixed - // match as valid only when it holds under every root, so a waiver scoped - // to src/foo.html never hides a finding on a page served from - // public/foo.html. With a single root this reduces to plain matching. + // One live session is served by one server, so a single document root must + // sit at or above every configured page. The only prefix that can safely + // be asserted is therefore the deepest common ancestor of the glob roots. + // Treating each glob's own prefix as an identity goes wrong in both + // directions: disjoint roots (src/ and public/) invent simultaneous + // identities for one URL, so a waiver scoped to src/foo.html hides a + // finding on a page served from public/foo.html; nested roots (prototype/ + // and prototype/library/, from globs at two depths in one tree) are not + // 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) { let pagePath = String(pathname || ''); try { @@ -108,39 +113,41 @@ // name files. Without this, /news/ never matches prototype/news/index.html. if (pagePath === '' || pagePath.endsWith('/')) pagePath += 'index.html'; - const suffixesOf = (fullPath) => { - const parts = fullPath.split('/').filter(Boolean); - const out = []; - for (let i = 0; i < parts.length; i++) { - out.push(parts.slice(i).join('/')); - } - return out; - }; - - const rooted = []; + const prefixes = []; for (const entry of Array.isArray(roots) ? roots : []) { if (typeof entry !== 'string') continue; - const prefix = entry === '' || entry.endsWith('/') ? entry : entry + '/'; - rooted.push(suffixesOf(prefix + pagePath)); + prefixes.push(entry.split('/').filter(Boolean)); } - return { bare: suffixesOf(pagePath), rooted }; + let common = prefixes.length > 0 ? prefixes[0] : []; + for (const segments of prefixes.slice(1)) { + let i = 0; + while (i < common.length && i < segments.length && common[i] === segments[i]) i += 1; + 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]; } function matchesScope(globs, candidates) { - const regexes = []; - for (const glob of globs) { + return globs.some((glob) => { + let re; try { - regexes.push(globToRegex(String(glob))); + re = globToRegex(String(glob)); } catch { // Malformed glob: skip it, as matchesAnyGlob does in the CLI. + return false; } - } - if (regexes.length === 0) return false; - const hits = (paths) => paths.some((path) => regexes.some((re) => re.test(path))); - // Matches on the URL path itself hold whichever root serves the page. - if (hits(candidates.bare)) return true; - // Root-prefixed matches only hold if no possible identity disagrees. - return candidates.rooted.length > 0 && candidates.rooted.every(hits); + return candidates.some((candidate) => re.test(candidate)); + }); } /** diff --git a/tests/live-browser-ignores.test.mjs b/tests/live-browser-ignores.test.mjs index 69586d86a..811eed6d7 100644 --- a/tests/live-browser-ignores.test.mjs +++ b/tests/live-browser-ignores.test.mjs @@ -1,10 +1,11 @@ import { describe, it } from 'node:test'; import assert from 'node:assert/strict'; import { readFileSync } from 'node:fs'; -import { join } from 'node:path'; +import { dirname, join } from 'node:path'; +import { fileURLToPath } from 'node:url'; import vm from 'node:vm'; -const REPO_ROOT = process.cwd(); +const REPO_ROOT = join(dirname(fileURLToPath(import.meta.url)), '..'); 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 @@ -183,35 +184,46 @@ describe('live-browser-ignores resolver', () => { assert.deepEqual(out.disabledRules, ['dark-glow']); }); - it('does not apply a waiver scoped to one root when several roots could serve the URL', () => { - // With src/**/*.html and public/**/*.html both configured, /foo.html - // could be served from either root. A waiver naming only src/foo.html - // must not hide a finding on a page actually served from - // public/foo.html; ambiguity resolves to showing the finding. + it('asserts no prefix when the configured roots share no common ancestor', () => { + // With src/**/*.html and public/**/*.html both configured, no single + // document root maps /foo.html to a unique project file, so no prefix + // is asserted. A waiver naming src/foo.html must not hide a finding on + // a page served from public/foo.html; ambiguity resolves to showing + // the finding. Bare-path spellings still apply whichever root serves it. const ignores = { roots: ['src/', 'public/'], ignoreValues: [ { rule: 'dark-glow', value: '*', files: ['src/foo.html'] }, - { rule: 'gradient-text', value: 'teal', files: ['src/foo.html'] }, - ], - }; - const out = resolve({ ignores, pathname: '/foo.html' }); - assert.deepEqual(out, EMPTY); - }); - - it('applies a scoped waiver under several roots when every identity matches', () => { - const ignores = { - roots: ['src/', 'public/'], - ignoreValues: [ - // Two globs covering both identities. - { rule: 'dark-glow', value: '*', files: ['src/foo.html', 'public/foo.html'] }, - // A bare-path glob holds whichever root serves the page. + { rule: 'clipped-overflow-container', value: '*', files: ['src/foo.html', 'public/foo.html'] }, { rule: 'em-dash-overuse', value: '*', files: ['foo.html'] }, { rule: 'gradient-text', value: '*', files: ['**/foo.html'] }, ], }; const out = resolve({ ignores, pathname: '/foo.html' }); - assert.deepEqual(out.disabledRules.sort(), ['dark-glow', 'em-dash-overuse', 'gradient-text']); + assert.deepEqual(out.disabledRules.sort(), ['em-dash-overuse', 'gradient-text']); + }); + + it('reduces nested roots to their common ancestor so normal waivers keep applying', () => { + // Globs at two depths in one tree (prototype/*.html plus + // prototype/library/**/*.html) derive the prefixes prototype/ and + // prototype/library/. Those are not alternative identities: one server + // serves both, so the document root sits at their common ancestor and + // a project-relative waiver like prototype/index.html must apply. + const ignores = { + roots: ['prototype/', 'prototype/library/'], + ignoreValues: [ + { rule: 'dark-glow', value: '*', files: ['prototype/index.html'] }, + { rule: 'em-dash-overuse', value: '*', files: ['prototype/library/**'] }, + ], + }; + assert.deepEqual( + resolve({ ignores, pathname: '/index.html' }).disabledRules, + ['dark-glow'], + ); + assert.deepEqual( + resolve({ ignores, pathname: '/library/buttons.html' }).disabledRules, + ['em-dash-overuse'], + ); }); it('survives malformed roots and percent-escapes without throwing', () => {