Report unreadable detector directories

AI assistance disclosure: Codex implemented and tested this fix under maintainer direction.

README.md

@@ -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.
 

README.npm.md

@@ -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
 

cli/engine/cli/main.mjs

@@ -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,11 @@ async function detectCli() {
   if (helpMode) { printUsage(); process.exit(0); }
 
   let allFindings = [];
+  let hadOperationalFailure = false;
+  const reportLocalScanFailure = (target, error) => {
+    hadOperationalFailure = true;
+    process.stderr.write(`Error: cannot scan ${target}: ${error.message}\n`);
+  };
 
   if (!process.stdin.isTTY && targets.length === 0) {
     allFindings = await handleStdin(scanOptionsFor);
@@ -319,11 +330,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,14 +358,21 @@ 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;
         }
 
         const resolved = path.resolve(target);
         let stat;
         try { stat = fs.statSync(resolved); }
-        catch { process.stderr.write(`Warning: cannot access ${target}\n`); continue; }
+        catch {
+          hadOperationalFailure = true;
+          process.stderr.write(`Warning: cannot access ${target}\n`);
+          continue;
+        }
 
         if (stat.isDirectory()) {
           // Check for framework dev server config (skip in JSON/quiet modes to avoid polluting output)
@@ -372,7 +401,7 @@ async function detectCli() {
             }
           }
 
-          const files = walkDir(resolved)
+          const files = walkDir(resolved, reportLocalScanFailure)
             .filter(file => !shouldIgnoreDetectionFile(file, process.cwd(), detectionConfig));
           const htmlCount = files.filter(f => HTML_EXTENSIONS.has(path.extname(f).toLowerCase())).length;
 
@@ -388,7 +417,11 @@ async function detectCli() {
           }
 
           // Build import graph for multi-file awareness
-          const graph = buildImportGraph(files);
+          const unreadableFiles = new Set();
+          const graph = buildImportGraph(files, (file, error) => {
+            unreadableFiles.add(file);
+            reportLocalScanFailure(file, error);
+          });
           // Build reverse map: file -> set of files that import it
           const importedByMap = new Map();
           for (const [importer, imports] of graph) {
@@ -399,24 +432,33 @@ async function detectCli() {
           }
 
           for (const file of files) {
-            // Each file resolves its own project design system (cached by root),
-            // so a scan spanning sibling projects applies the right rules per file.
-            const fileOptions = scanOptionsFor(file);
-            const fileFindings = await detectLocalFile(file, fileOptions);
-            // Annotate findings with import context
-            const importers = importedByMap.get(file);
-            if (importers && importers.size > 0) {
-              const importerNames = [...importers].map(f => path.basename(f));
-              for (const f of fileFindings) {
-                f.importedBy = importerNames;
+            if (unreadableFiles.has(file)) continue;
+            try {
+              // Each file resolves its own project design system (cached by root),
+              // so a scan spanning sibling projects applies the right rules per file.
+              const fileOptions = scanOptionsFor(file);
+              const fileFindings = await detectLocalFile(file, fileOptions);
+              // Annotate findings with import context
+              const importers = importedByMap.get(file);
+              if (importers && importers.size > 0) {
+                const importerNames = [...importers].map(f => path.basename(f));
+                for (const f of fileFindings) {
+                  f.importedBy = importerNames;
+                }
               }
+              allFindings.push(...fileFindings);
+            } catch (error) {
+              reportLocalScanFailure(file, error);
             }
-            allFindings.push(...fileFindings);
           }
         } else if (stat.isFile()) {
           if (shouldIgnoreDetectionFile(resolved, process.cwd(), detectionConfig)) continue;
-          const fileOptions = scanOptionsFor(resolved);
-          allFindings.push(...await detectLocalFile(resolved, fileOptions));
+          try {
+            const fileOptions = scanOptionsFor(resolved);
+            allFindings.push(...await detectLocalFile(resolved, fileOptions));
+          } catch (error) {
+            reportLocalScanFailure(target, error);
+          }
         }
       }
     } finally {
@@ -433,6 +475,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 +489,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 };

cli/engine/node/file-system.mjs

@@ -46,15 +46,20 @@ const IMPORT_SPECIFIER_PATTERNS = [
   /@(?:use|forward)\s+['"]([^'"]+)['"]/g,
 ];
 
-function walkDir(dir) {
+function walkDir(dir, onReadError = null) {
   const files = [];
   let entries;
-  try { entries = fs.readdirSync(dir, { withFileTypes: true }); } catch { return files; }
+  try {
+    entries = fs.readdirSync(dir, { withFileTypes: true });
+  } catch (error) {
+    if (onReadError) onReadError(dir, error);
+    return files;
+  }
   for (const entry of entries) {
     if (SKIP_DIRS.has(entry.name)) continue;
     if (entry.isDirectory() && entry.name.startsWith('.') && !HIDDEN_SOURCE_DIRS.has(entry.name)) continue;
     const full = path.join(dir, entry.name);
-    if (entry.isDirectory()) files.push(...walkDir(full));
+    if (entry.isDirectory()) files.push(...walkDir(full, onReadError));
     else if (hasScannableExtension(entry.name)) files.push(full);
   }
   return files;
@@ -81,12 +86,19 @@ function resolveImport(specifier, fromDir, fileSet) {
   return null;
 }
 
-function buildImportGraph(files) {
+function buildImportGraph(files, onReadError = null) {
   const fileSet = new Set(files);
   const graph = new Map();
 
   for (const file of files) {
-    const content = fs.readFileSync(file, 'utf-8');
+    let content;
+    try {
+      content = fs.readFileSync(file, 'utf-8');
+    } catch (error) {
+      if (!onReadError) throw error;
+      onReadError(file, error);
+      continue;
+    }
     const dir = path.dirname(file);
     const imports = new Set();
 

tests/detect-antipatterns.test.js

@@ -2648,6 +2648,21 @@ describe('walkDir', () => {
     expect(walkDir('/nonexistent/path/12345')).toHaveLength(0);
   });
 
+  test('reports directory traversal errors to an optional handler', () => {
+    const parent = fs.mkdtempSync(path.join(os.tmpdir(), 'impeccable-walk-error-'));
+    try {
+      const missing = path.join(parent, 'missing');
+      const errors = [];
+
+      expect(walkDir(missing, (dir, error) => {
+        errors.push({ dir, code: error.code });
+      })).toHaveLength(0);
+      expect(errors).toEqual([{ dir: missing, code: 'ENOENT' }]);
+    } finally {
+      fs.rmSync(parent, { recursive: true, force: true });
+    }
+  });
+
   // Issue #303: when impeccable (or any agent tool) is installed into a
   // project's .claude/.cursor/etc. tree, a root scan descended into the
   // vendored skill code and reported the detector's own example strings as
@@ -3684,6 +3699,23 @@ describe('buildImportGraph', () => {
       expect(imp).toContain(MF);
     }
   });
+
+  test('reports unreadable files and continues building the graph', async () => {
+    await withStaticFixture({
+      'readable.css': '@import "./missing.css";\n',
+    }, ({ dir }) => {
+      const readable = path.join(dir, 'readable.css');
+      const missing = path.join(dir, 'missing.css');
+      const errors = [];
+      const graph = buildImportGraph([missing, readable], (file, error) => {
+        errors.push({ file, code: error.code });
+      });
+
+      expect(errors).toEqual([{ file: missing, code: 'ENOENT' }]);
+      expect(graph.get(readable)).toEqual(new Set([missing]));
+      expect(graph.has(missing)).toBe(false);
+    });
+  });
 });
 
 describe('resolveImport', () => {

tests/detect-url-launch.test.mjs

@@ -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,50 @@ function makePuppeteer({ failChannel = false } = {}) {
   };
 }
 
+function runWithoutPuppeteer(args, files = {}, { nodeArgs = [] } = {}) {
+  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',
+      [...nodeArgs, path.join(cliRoot, 'engine', 'detect-antipatterns.mjs'), '--json', ...args],
+      { cwd: isolatedRoot, encoding: 'utf8' },
+    );
+  } finally {
+    fs.rmSync(isolatedRoot, { recursive: true, force: true });
+  }
+}
+
+const DENY_FS_PRELOAD = `
+const fs = require('node:fs');
+const originalReadFileSync = fs.readFileSync;
+const originalReaddirSync = fs.readdirSync;
+fs.readFileSync = function (file, ...args) {
+  if (String(file).endsWith('unreadable.css')) {
+    const error = new Error('simulated EACCES');
+    error.code = 'EACCES';
+    throw error;
+  }
+  return originalReadFileSync.call(this, file, ...args);
+};
+fs.readdirSync = function (dir, ...args) {
+  if (String(dir).endsWith('unreadable-dir')) {
+    const error = new Error('simulated directory EACCES');
+    error.code = 'EACCES';
+    throw error;
+  }
+  return originalReaddirSync.call(this, dir, ...args);
+};
+`;
+
 describe('launchBrowser', () => {
   test('Windows: prefers system Chrome via channel:chrome', async () => {
     setPlatform('win32');
@@ -81,6 +132,122 @@ 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');
+  });
+
+  test('exits 1 when an explicitly requested local target cannot be accessed', () => {
+    const result = runWithoutPuppeteer(['missing.css']);
+
+    expect(result.status).toBe(1);
+    expect(result.stdout).toBe('[]\n');
+    expect(result.stderr).toContain('Warning: cannot access missing.css');
+  });
+
+  test('missing local target takes precedence over findings from another target', () => {
+    const result = runWithoutPuppeteer(
+      ['missing.css', '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('Warning: cannot access missing.css');
+  });
+
+  test('unreadable local file exits 1 with valid empty JSON and no stack trace', () => {
+    const result = runWithoutPuppeteer(
+      ['unreadable.css'],
+      {
+        'deny-fs.cjs': DENY_FS_PRELOAD,
+        'unreadable.css': '.hero { color: red; }\n',
+      },
+      { nodeArgs: ['--require=./deny-fs.cjs'] },
+    );
+
+    expect(result.status).toBe(1);
+    expect(result.stdout).toBe('[]\n');
+    expect(result.stderr).toContain('Error: cannot scan unreadable.css: simulated EACCES');
+    expect(result.stderr).not.toContain('at detectLocalFile');
+  });
+
+  test('unreadable directory file preserves findings from readable siblings', () => {
+    const result = runWithoutPuppeteer(
+      ['styles'],
+      {
+        'deny-fs.cjs': DENY_FS_PRELOAD,
+        'styles/unreadable.css': '.hero { color: red; }\n',
+        'styles/page.css': '.hero { animation: bounce 1s linear infinite; }\n',
+      },
+      { nodeArgs: ['--require=./deny-fs.cjs'] },
+    );
+    const findings = JSON.parse(result.stdout);
+
+    expect(result.status).toBe(1);
+    expect(findings.some(finding => finding.antipattern === 'bounce-easing')).toBe(true);
+    expect(result.stderr.match(/simulated EACCES/g)).toHaveLength(1);
+  });
+
+  test('unreadable local directory exits 1 with valid empty JSON', () => {
+    const result = runWithoutPuppeteer(
+      ['unreadable-dir'],
+      {
+        'deny-fs.cjs': DENY_FS_PRELOAD,
+        'unreadable-dir/page.css': '.hero { color: red; }\n',
+      },
+      { nodeArgs: ['--require=./deny-fs.cjs'] },
+    );
+
+    expect(result.status).toBe(1);
+    expect(result.stdout).toBe('[]\n');
+    expect(result.stderr).toContain('Error: cannot scan');
+    expect(result.stderr).toContain('unreadable-dir: simulated directory EACCES');
+  });
+
+  test('unreadable nested directory preserves findings from readable siblings', () => {
+    const result = runWithoutPuppeteer(
+      ['project'],
+      {
+        'deny-fs.cjs': DENY_FS_PRELOAD,
+        'project/unreadable-dir/hidden.css': '.hero { color: red; }\n',
+        'project/page.css': '.hero { animation: bounce 1s linear infinite; }\n',
+      },
+      { nodeArgs: ['--require=./deny-fs.cjs'] },
+    );
+    const findings = JSON.parse(result.stdout);
+
+    expect(result.status).toBe(1);
+    expect(findings.some(finding => finding.antipattern === 'bounce-easing')).toBe(true);
+    expect(result.stderr.match(/simulated directory EACCES/g)).toHaveLength(1);
+  });
+});
+
 describe('splitScanUrl', () => {
   test('strips http(s) userinfo and returns credentials', () => {
     expect(splitScanUrl('https://user:pass@example.com')).toEqual({