From e4438ea586256ffcb3d8e68f546f1ad08420897b Mon Sep 17 00:00:00 2001 From: Paul Bakaus Date: Wed, 2 Sep 2026 09:03:34 -0700 Subject: [PATCH] Fix detector URL and advisory handling Recover joined URL arguments without splitting local paths, derive advisory behavior from registry severity across consumers, inspect readable linked CSS in URL scans, and report only the dominant primary font. AI assistance disclosure: Implemented and verified with Codex under maintainer direction. --- README.md | 2 + cli/engine/browser/injected/index.mjs | 38 +++++++++- cli/engine/cli/main.mjs | 32 ++++++-- cli/engine/detect-antipatterns-browser.js | 57 +++++++++++--- cli/engine/findings.mjs | 4 +- cli/engine/registry/antipatterns.mjs | 7 +- cli/engine/rules/checks.mjs | 17 +++-- skill/scripts/hook-lib.mjs | 13 ++-- tests/detect-antipatterns-browser.test.mjs | 74 +++++++++++++++++++ tests/detect-antipatterns.test.js | 35 +++++++-- tests/detect-cli-stdin-dispatch.test.mjs | 10 ++- .../antipatterns/linked-url-patterns.css | 11 +++ .../antipatterns/linked-url-patterns.html | 14 ++++ tests/hook.test.mjs | 3 + 14 files changed, 272 insertions(+), 45 deletions(-) create mode 100644 tests/fixtures/antipatterns/linked-url-patterns.css create mode 100644 tests/fixtures/antipatterns/linked-url-patterns.html diff --git a/README.md b/README.md index b957b7e5b..69cb68639 100644 --- a/README.md +++ b/README.md @@ -427,6 +427,8 @@ npx impeccable ignores add-value overused-font Inter --reason "Brand font" The detector catches 61 deterministic issues across AI slop (side-tab borders, purple gradients, bounce easing, dark glows) and general design quality (line length, cramped padding, small touch targets, skipped headings, and more). +Human-readable findings are diagnostics written to stderr, so redirect them with `2> findings.txt`. Use `--json` for machine-readable results on stdout. URL scans inspect the rendered DOM, computed layout, and accessible linked stylesheets; browser security still prevents reading cross-origin CSS without CORS. A clean detector run is evidence, not proof of visual or accessibility quality: it does not replace inspecting the rendered experience across relevant viewports. + By default, `detect` respects the same `.impeccable/config.json` and `.impeccable/config.local.json` detector config as the design hook: `detector.ignoreRules`, `detector.ignoreFiles`, `detector.ignoreValues`, and `detector.designSystem.enabled`. Hook lifecycle settings such as `hook.enabled` only affect automatic hook execution. For a waiver that should travel with one file instead of the repo config, add an inline comment in the file: ``. The marker works in any comment syntax, scopes to the whole file (or one line with `impeccable-disable-line` / `impeccable-disable-next-line`), and is bypassed by `--no-inline-ignores` or `--no-config`. diff --git a/cli/engine/browser/injected/index.mjs b/cli/engine/browser/injected/index.mjs index febf7f297..a9b9fd18a 100644 --- a/cli/engine/browser/injected/index.mjs +++ b/cli/engine/browser/injected/index.mjs @@ -1235,7 +1235,7 @@ if (IS_BROWSER) { // Advisory findings (em-dash overuse, etc.) are surfaced but never // treated as failures; carry the flag so the overlay/extension can // render them with the mildest affordance and consumers can filter. - advisory: (ap && ap.advisory === true) || f.advisory === true, + advisory: ap?.severity === 'advisory' || f.severity === 'advisory' || f.advisory === true, detail: f.detail || f.snippet, ignoreValue: f.ignoreValue || f.value || '', name: ap ? ap.name : (f.type || f.id), @@ -1277,6 +1277,36 @@ if (IS_BROWSER) { else groupMap.set(el, [...kept]); } + // Read CSS that is absent from document.outerHTML. Inline
${primary}${secondary}
`); + await fontPage.evaluate(() => { window.__IMPECCABLE_CONFIG__ = { autoScan: false }; }); + await fontPage.evaluate(browserScript); + const fontFindings = await fontPage.evaluate(() => window.impeccableDetect({ serialize: true }) + .flatMap(group => group.findings || []) + .filter(finding => finding.type === 'overused-font')); + assert.equal(fontFindings.length, 1, JSON.stringify(fontFindings)); + assert.match(fontFindings[0].detail, /Primary font: geist \(82% of text\)/i); + assert.doesNotMatch(fontFindings[0].detail, /geist mono/i); + await fontPage.close(); + } finally { + await browser.close().catch(() => {}); + } + }); + // Only a real browser reproduces this one: Chrome keeps oklch(), lch(), and // color(srgb ...) verbatim in getComputedStyle output, so a detector that // cannot parse those reads every surface as unset, walks out of the page, diff --git a/tests/detect-antipatterns.test.js b/tests/detect-antipatterns.test.js index 9ad3a7a47..30c5e8f18 100644 --- a/tests/detect-antipatterns.test.js +++ b/tests/detect-antipatterns.test.js @@ -2717,23 +2717,30 @@ describe('CLI', () => { expect(code).toBe(0); expect(stdout).toContain('Usage:'); expect(stdout).toContain('--quiet'); + expect(stdout).toContain('Human-readable findings go to stderr'); expect(stdout).not.toContain('--gpt'); expect(stdout).not.toContain('--gemini'); }); - test('generated-UI tells run by default in the CLI', () => { + test('severity advisory is non-blocking, flagged in JSON, and suppressible', () => { const { stdout, code } = run('--json', path.join(FIXTURES, 'gpt-tells.html')); - expect(code).toBe(2); - const ids = JSON.parse(stdout).map(f => f.antipattern); + expect(code).toBe(0); + const findings = JSON.parse(stdout); + const ids = findings.map(f => f.antipattern); expect(ids).toContain('gpt-thin-border-wide-shadow'); expect(ids).toContain('repeating-stripes-gradient'); expect(ids).toContain('codex-grid-background'); expect(ids).toContain('theater-slop-phrase'); + expect(findings.every(f => f.severity === 'advisory' && f.advisory === true)).toBe(true); + + const hidden = run('--json', '--no-advisory', path.join(FIXTURES, 'gpt-tells.html')); + expect(hidden.code).toBe(0); + expect(JSON.parse(hidden.stdout)).toEqual([]); }); test('legacy provider flags are accepted as deprecated no-ops', () => { const { stdout, stderr, code } = run('--gpt', '--json', path.join(FIXTURES, 'gpt-tells.html')); - expect(code).toBe(2); + expect(code).toBe(0); expect(stderr).toContain('--gpt and --gemini are deprecated and ignored'); expect(JSON.parse(stdout).some(f => f.antipattern === 'codex-grid-background')).toBe(true); }); @@ -2744,14 +2751,30 @@ describe('CLI', () => { expect(stderr).not.toContain('cannot access detect'); }); + test('keeps a local path containing spaces as one scan target', () => { + const fixture = writeStaticFixture({ + 'page with spaces.html': '

Plain page

', + }); + const file = path.join(fixture.dir, 'page with spaces.html'); + try { + const { stdout, stderr, code } = run('--json', file); + expect(code).toBe(0); + expect(JSON.parse(stdout)).toEqual([]); + expect(stderr).not.toContain('cannot access'); + } finally { + fs.rmSync(fixture.dir, { recursive: true, force: true }); + } + }); + test('should-pass exits 0', () => { const { code } = run(path.join(FIXTURES, 'should-pass.html')); expect(code).toBe(0); }); test('should-flag exits 2 with findings', () => { - const { code, stderr } = run(path.join(FIXTURES, 'should-flag.html')); + const { stdout, code, stderr } = run(path.join(FIXTURES, 'should-flag.html')); expect(code).toBe(2); + expect(stdout).toBe(''); expect(stderr).toContain('side-tab'); }); @@ -2899,7 +2922,7 @@ colors: `); const full = runIn(dir, '--json', 'index.css'); - expect(full.code).toBe(2); + expect(full.code).toBe(0); const fullIds = JSON.parse(full.stdout).map((finding) => finding.antipattern); expect(fullIds).toContain('design-system-font-size'); expect(fullIds).toContain('design-system-color'); diff --git a/tests/detect-cli-stdin-dispatch.test.mjs b/tests/detect-cli-stdin-dispatch.test.mjs index 2481d00f8..a0ee03e34 100644 --- a/tests/detect-cli-stdin-dispatch.test.mjs +++ b/tests/detect-cli-stdin-dispatch.test.mjs @@ -10,7 +10,7 @@ const root = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..'); const cli = path.join(root, 'cli', 'bin', 'cli.js'); const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'impeccable-stdin-dispatch-')); -function detectStdinFile(filePath) { +function detectStdinFile(filePath, expectedStatus = 2) { const result = spawnSync( process.execPath, [cli, 'detect', '--json', '--no-config', '--no-design-system'], @@ -19,7 +19,7 @@ function detectStdinFile(filePath) { encoding: 'utf8', }, ); - assert.equal(result.status, 2, result.stderr); + assert.equal(result.status, expectedStatus, result.stderr); return JSON.parse(result.stdout); } @@ -57,9 +57,11 @@ describe('detect CLI stdin file dispatch', () => { } `); - const findings = detectStdinFile(filePath); + const findings = detectStdinFile(filePath, 0); assert.ok(findings.some( - (item) => item.file === filePath && item.antipattern === 'codex-grid-background', + (item) => item.file === filePath + && item.antipattern === 'codex-grid-background' + && item.advisory === true, )); }); }); diff --git a/tests/fixtures/antipatterns/linked-url-patterns.css b/tests/fixtures/antipatterns/linked-url-patterns.css new file mode 100644 index 000000000..b4d8853fa --- /dev/null +++ b/tests/fixtures/antipatterns/linked-url-patterns.css @@ -0,0 +1,11 @@ +.flag-linked-stripes { + width: 160px; + height: 80px; + background: repeating-linear-gradient(45deg, #eee, #eee 10px, #fafafa 10px, #fafafa 20px); +} + +/* A shipped but unused selector must remain outside live URL findings. */ +.unused-linked-grid { + background: linear-gradient(90deg, #d9d9d9 1px, transparent 1px), linear-gradient(180deg, #d9d9d9 1px, transparent 1px); + background-size: 72px 72px; +} diff --git a/tests/fixtures/antipatterns/linked-url-patterns.html b/tests/fixtures/antipatterns/linked-url-patterns.html new file mode 100644 index 000000000..4aceecd16 --- /dev/null +++ b/tests/fixtures/antipatterns/linked-url-patterns.html @@ -0,0 +1,14 @@ + + + + + Linked URL pattern detection + + + +
+

Linked stylesheet pattern

+
Rendered repeating stripes
+
+ + diff --git a/tests/hook.test.mjs b/tests/hook.test.mjs index ee12a5876..660b43817 100644 --- a/tests/hook.test.mjs +++ b/tests/hook.test.mjs @@ -665,6 +665,7 @@ describe('filterFindings()', () => { const filtered = filterFindings(findings, content, '.ts', { ignoreRules: ['side-tab'], minSeverity: 'error', + advisoryRules: 'include', limits: DEFAULT_CONFIG.limits, }); assert.deepEqual(filtered.map((f) => f.antipattern), ['gradient-text', 'overused-font']); @@ -674,6 +675,7 @@ describe('filterFindings()', () => { const findings = [ finding('side-tab', 1), finding('em-dash-overuse', 2), + finding('design-system-radius', 3, { severity: 'advisory' }), finding('gradient-text', 3), ]; const filtered = filterFindings(findings, '', '.html', { @@ -700,6 +702,7 @@ describe('filterFindings()', () => { assert.ok(ADVISORY_RULES.has('em-dash-overuse')); assert.equal(isAdvisoryFinding(finding('em-dash-overuse', 1)), true); assert.equal(isAdvisoryFinding({ antipattern: 'anything', advisory: true }), true); + assert.equal(isAdvisoryFinding({ antipattern: 'anything', severity: 'advisory' }), true); assert.equal(isAdvisoryFinding(finding('side-tab', 1)), false); });