Fix URL scan failure exit codes

Return exit 1 when browser setup or a URL scan fails, including partial multi-target scans, while preserving JSON findings output. Document the detector exit contract and cover isolated installs without Puppeteer.\n\nAI assistance disclosure: Codex implemented and tested this fix under maintainer direction.
This commit is contained in:
Paul Bakaus
2026-09-02 18:39:31 -07:00
parent 1e36c86315
commit 7437368526
4 changed files with 94 additions and 7 deletions
+1 -1
View File
@@ -427,7 +427,7 @@ 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.
Human-readable findings are diagnostics written to stderr, so redirect them with `2> findings.txt`. Use `--json` for machine-readable results on stdout. Exit `0` means the scan completed without primary findings, exit `2` means it completed with primary findings, and exit `1` means at least one requested target could not be scanned; operational failure takes precedence for a partial multi-target scan. 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.
+5 -2
View File
@@ -60,8 +60,11 @@ npx impeccable detect --fast src/
## Exit Codes
- `0`: no issues found
- `2`: anti-patterns detected
- `0`: scan completed with no primary findings (advisories may still be listed)
- `1`: at least one requested target could not be scanned
- `2`: scan completed with primary findings
Operational failure takes precedence when a multi-target scan is partial. In JSON mode, stdout remains a findings array and diagnostics are written to stderr.
## Options
+29 -4
View File
@@ -189,6 +189,12 @@ Output streams:
Human-readable findings go to stderr so stdout stays available for structured
output. Use --json for JSON on stdout, or redirect text with 2> findings.txt.
Exit status:
0 Scan completed with no primary findings (advisories may still be listed)
1 At least one requested target could not be scanned
2 Scan completed with primary findings
Operational failure takes precedence when a multi-target scan is partial.
Project config:
Respects .impeccable/config.json and .impeccable/config.local.json detector
settings: detector.ignoreRules, detector.ignoreFiles, detector.ignoreValues,
@@ -309,6 +315,7 @@ async function detectCli() {
if (helpMode) { printUsage(); process.exit(0); }
let allFindings = [];
let hadOperationalFailure = false;
if (!process.stdin.isTTY && targets.length === 0) {
allFindings = await handleStdin(scanOptionsFor);
@@ -319,11 +326,22 @@ async function detectCli() {
// browser-grade scan of a local artifact can pass file:///abs/path.html
// instead of the bare path (which stays on the static engine).
const urlTargetCount = paths.filter(target => URL_TARGET_RE.test(target)).length;
const browserDetector = urlTargetCount > 1 ? await createBrowserDetector() : null;
let browserDetector = null;
let browserSetupFailed = false;
if (urlTargetCount > 1) {
try {
browserDetector = await createBrowserDetector();
} catch (e) {
browserSetupFailed = true;
hadOperationalFailure = true;
process.stderr.write(`Error: ${e.message}\n`);
}
}
try {
for (const target of paths) {
if (URL_TARGET_RE.test(target)) {
if (browserSetupFailed) continue;
// A file:// URL points at a local artifact, so its design system
// resolves from that file's project. A remote http(s) URL has no
// local project — it gets base options (no design system), never
@@ -336,7 +354,10 @@ async function detectCli() {
? (url) => browserDetector.detectUrl(url, urlOptions)
: (url) => detectUrl(url, urlOptions);
allFindings.push(...await scanner(target));
} catch (e) { process.stderr.write(`Error: ${e.message}\n`); }
} catch (e) {
hadOperationalFailure = true;
process.stderr.write(`Error: ${e.message}\n`);
}
continue;
}
@@ -433,6 +454,10 @@ async function detectCli() {
// advisory-only scan still prints its notes but exits 0 (a clean pass), so
// advisory rules never break CI or block automation.
const { primary, advisory } = partitionAdvisory(allFindings);
// Exit 1 means at least one requested scan could not complete. It takes
// precedence over exit 2 because findings from the remaining targets do not
// turn a partial scan into a complete one.
const exitCode = hadOperationalFailure ? 1 : (primary.length > 0 ? 2 : 0);
if (allFindings.length > 0) {
if (jsonMode) process.stdout.write(formatFindings(allFindings, true) + '\n');
@@ -443,10 +468,10 @@ async function detectCli() {
}
}
else process.stderr.write(formatFindings(allFindings, false) + '\n');
process.exit(primary.length > 0 ? 2 : 0);
process.exit(exitCode);
}
if (jsonMode) process.stdout.write('[]\n');
process.exit(0);
process.exit(exitCode);
}
export { formatFindings, handleStdin, confirm, printUsage, detectCli };
+59
View File
@@ -1,7 +1,14 @@
import { describe, test, expect, afterEach } from 'bun:test';
import { spawnSync } from 'node:child_process';
import fs from 'node:fs';
import http from 'node:http';
import os from 'node:os';
import path from 'node:path';
import { fileURLToPath } from 'node:url';
import { launchBrowser, detectUrl, splitScanUrl } from '../cli/engine/engines/browser/detect-url.mjs';
const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..');
// launchBrowser prefers the system-installed Chrome on Windows to dodge the
// bundled-Chrome GPU crash-loop (issue #372), and keeps the pinned bundled
// build everywhere else. The function takes the puppeteer module as a
@@ -38,6 +45,28 @@ function makePuppeteer({ failChannel = false } = {}) {
};
}
function runWithoutPuppeteer(args, files = {}) {
const isolatedRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'impeccable-no-puppeteer-'));
try {
const cliRoot = path.join(isolatedRoot, 'cli');
fs.mkdirSync(cliRoot, { recursive: true });
fs.cpSync(path.join(ROOT, 'cli', 'engine'), path.join(cliRoot, 'engine'), { recursive: true });
fs.cpSync(path.join(ROOT, 'cli', 'lib'), path.join(cliRoot, 'lib'), { recursive: true });
for (const [relativePath, contents] of Object.entries(files)) {
const filePath = path.join(isolatedRoot, relativePath);
fs.mkdirSync(path.dirname(filePath), { recursive: true });
fs.writeFileSync(filePath, contents);
}
return spawnSync(
'node',
[path.join(cliRoot, 'engine', 'detect-antipatterns.mjs'), '--json', ...args],
{ cwd: isolatedRoot, encoding: 'utf8' },
);
} finally {
fs.rmSync(isolatedRoot, { recursive: true, force: true });
}
}
describe('launchBrowser', () => {
test('Windows: prefers system Chrome via channel:chrome', async () => {
setPlatform('win32');
@@ -81,6 +110,36 @@ describe('launchBrowser', () => {
});
});
describe('detect CLI browser failures', () => {
test('exits 1 with valid empty JSON when Puppeteer is unavailable', () => {
const result = runWithoutPuppeteer(['https://example.com']);
expect(result.status).toBe(1);
expect(result.stdout).toBe('[]\n');
expect(result.stderr).toContain('puppeteer is required for URL scanning');
});
test('reports a shared multi-URL setup failure once and exits 1', () => {
const result = runWithoutPuppeteer(['https://example.com', 'https://example.org']);
expect(result.status).toBe(1);
expect(result.stdout).toBe('[]\n');
expect(result.stderr.match(/puppeteer is required for URL scanning/g)).toHaveLength(1);
});
test('operational failure takes precedence over findings from another target', () => {
const result = runWithoutPuppeteer(
['https://example.com', 'page.css'],
{ 'page.css': '.hero { animation: bounce 1s linear infinite; }\n' },
);
const findings = JSON.parse(result.stdout);
expect(result.status).toBe(1);
expect(findings.some(finding => finding.antipattern === 'bounce-easing')).toBe(true);
expect(result.stderr).toContain('puppeteer is required for URL scanning');
});
});
describe('splitScanUrl', () => {
test('strips http(s) userinfo and returns credentials', () => {
expect(splitScanUrl('https://user:pass@example.com')).toEqual({