From 281f30e5ccc8da1acb0e03a355f57a1d4b9e5366 Mon Sep 17 00:00:00 2001 From: Abdul Wahab Date: Fri, 28 Aug 2026 12:20:02 +0500 Subject: [PATCH] Fix: skipScan must cover the visual contrast stage too Bugbot on PR #665: the skipScan guard emptied only the analytic collectBrowserFindings pass, and scan()'s detached visual-contrast stage then repopulated an ignoreFiles-waived page with contrast markers and a second non-zero results post. Hoist the guard into skipScanActive() and honor it in scan() and the async collector; regenerate the browser bundle. Adds a browser-backed regression test that reproduces the leak (second results post carrying low-contrast findings) and pins the zero contract; drops a tautological assert flagged in review. AI-assisted change: implemented with Claude Code under maintainer direction. Co-Authored-By: Claude Fable 5 --- cli/engine/browser/injected/index.mjs | 24 +++++-- cli/engine/detect-antipatterns-browser.js | 24 +++++-- tests/detect-antipatterns-browser.test.mjs | 79 +++++++++++++++++++++- 3 files changed, 112 insertions(+), 15 deletions(-) diff --git a/cli/engine/browser/injected/index.mjs b/cli/engine/browser/injected/index.mjs index 0d0c26a16..febf7f297 100644 --- a/cli/engine/browser/injected/index.mjs +++ b/cli/engine/browser/injected/index.mjs @@ -1472,13 +1472,17 @@ if (IS_BROWSER) { return findings; } + // A page matched by detector.ignoreFiles is waived wholesale: every scan + // stage answers empty 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. + function skipScanActive() { + return EXTENSION_MODE && window.__IMPECCABLE_CONFIG__?.skipScan === true; + } + 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) { + if (skipScanActive()) { return { groupMap: new Map(), allFindings: [], pageLevelFindings: [] }; } const groupMap = new Map(); @@ -2013,6 +2017,12 @@ if (IS_BROWSER) { async function collectBrowserFindingsAsync(options = {}, runtime = {}) { const collected = collectBrowserFindings(); + // The visual pass walks the DOM on its own; on a skipScan page it would + // repopulate the emptied scan, so it is skipped with everything else. + if (skipScanActive()) { + lastVisualContrastAnalyses = []; + return { ...collected, allFindings: [], visualContrastAnalyses: [] }; + } await addVisualContrastFindings(collected.groupMap, options, runtime); return { ...collected, @@ -2066,7 +2076,7 @@ if (IS_BROWSER) { const generation = scanGeneration; const collected = collectBrowserFindings(); const allFindings = renderBrowserFindings(collected, options); - if (shouldRunVisualContrast(options)) { + if (!skipScanActive() && shouldRunVisualContrast(options)) { addVisualContrastFindings(collected.groupMap, options, { decorate: true, generation }) .then(() => { if (generation === scanGeneration) postSerializedFindings(collected.groupMap, options); diff --git a/cli/engine/detect-antipatterns-browser.js b/cli/engine/detect-antipatterns-browser.js index 281e49810..7df671eb5 100644 --- a/cli/engine/detect-antipatterns-browser.js +++ b/cli/engine/detect-antipatterns-browser.js @@ -8131,13 +8131,17 @@ if (IS_BROWSER) { return findings; } + // A page matched by detector.ignoreFiles is waived wholesale: every scan + // stage answers empty 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. + function skipScanActive() { + return EXTENSION_MODE && window.__IMPECCABLE_CONFIG__?.skipScan === true; + } + 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) { + if (skipScanActive()) { return { groupMap: new Map(), allFindings: [], pageLevelFindings: [] }; } const groupMap = new Map(); @@ -8672,6 +8676,12 @@ if (IS_BROWSER) { async function collectBrowserFindingsAsync(options = {}, runtime = {}) { const collected = collectBrowserFindings(); + // The visual pass walks the DOM on its own; on a skipScan page it would + // repopulate the emptied scan, so it is skipped with everything else. + if (skipScanActive()) { + lastVisualContrastAnalyses = []; + return { ...collected, allFindings: [], visualContrastAnalyses: [] }; + } await addVisualContrastFindings(collected.groupMap, options, runtime); return { ...collected, @@ -8725,7 +8735,7 @@ if (IS_BROWSER) { const generation = scanGeneration; const collected = collectBrowserFindings(); const allFindings = renderBrowserFindings(collected, options); - if (shouldRunVisualContrast(options)) { + if (!skipScanActive() && shouldRunVisualContrast(options)) { addVisualContrastFindings(collected.groupMap, options, { decorate: true, generation }) .then(() => { if (generation === scanGeneration) postSerializedFindings(collected.groupMap, options); diff --git a/tests/detect-antipatterns-browser.test.mjs b/tests/detect-antipatterns-browser.test.mjs index 0ac7fe05e..9a976fa2a 100644 --- a/tests/detect-antipatterns-browser.test.mjs +++ b/tests/detect-antipatterns-browser.test.mjs @@ -1109,7 +1109,6 @@ describe('detectUrl — browser-only fixtures', () => { 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, @@ -1131,6 +1130,84 @@ describe('detectUrl — browser-only fixtures', () => { } }); + it('extension scan: skipScan suppresses the visual contrast stage too', async () => { + 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(); + // Keep failing visual-contrast cards inside the no-scroll viewport. + await page.setViewport({ width: 1280, height: 1000 }); + await page.goto(`${baseUrl}/fixtures/antipatterns/visual-contrast.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 resultsFor = (scanId) => page.evaluate((id) => ( + (window.__impeccableMessages || []) + .filter(m => m.source === 'impeccable-results' && m.scanId === id) + .map(m => ({ + count: m.count, + types: (m.findings || []).flatMap(g => (g.findings || []).map(f => f.type || f.id)), + })) + ), scanId); + + // Control: the visual pass runs after the analytic scan and re-posts + // results carrying its low-contrast findings. This is exactly what an + // ignoreFiles-waived page must not do. + await page.evaluate(() => { + window.postMessage({ + source: 'impeccable-command', + action: 'scan', + config: { scanId: 'vc-skip-1', visualContrast: true, visualContrastMaxCandidates: 20 }, + }, '*'); + }); + const controlDeadline = Date.now() + 8000; + let control = []; + while (Date.now() < controlDeadline) { + control = await resultsFor('vc-skip-1'); + if (control.some(r => r.types.includes('low-contrast'))) break; + await new Promise(resolve => setTimeout(resolve, 100)); + } + assert.ok( + control.some(r => r.types.includes('low-contrast')), + `expected the control scan's visual pass to report low-contrast, got: ${JSON.stringify(control)}`, + ); + + // skipScan: a page waived wholesale by detector.ignoreFiles must stay + // at zero through the async visual stage as well: no results post with + // findings, no markers. + await page.evaluate(() => { + window.postMessage({ + source: 'impeccable-command', + action: 'scan', + config: { scanId: 'vc-skip-2', visualContrast: true, visualContrastMaxCandidates: 20, skipScan: true }, + }, '*'); + }); + await new Promise(resolve => setTimeout(resolve, 2500)); + const skipped = await resultsFor('vc-skip-2'); + assert.ok(skipped.length >= 1, `expected the skipScan scan to post results, got: ${JSON.stringify(skipped)}`); + assert.ok( + skipped.every(r => r.count === 0 && r.types.length === 0), + `expected every skipScan results post to stay empty, got: ${JSON.stringify(skipped)}`, + ); + const overlays = await page.evaluate(() => + document.querySelectorAll('.impeccable-overlay, .impeccable-label').length); + assert.equal(overlays, 0, `expected no markers on a skipScan page, got ${overlays}`); + 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({