fix(core): isolate rule failures so one crashing rule no longer kills the whole run

.changeset/rule-failure-isolation.md

@@ -0,0 +1,7 @@
+---
+'@svelte-vitals/core': patch
+'svelte-vitals': patch
+'@svelte-vitals/vite': patch
+---
+
+A rule that throws no longer kills the analysis: the run completes without it, its id and error surface as a warning, and its weight is removed from that run's Health denominator so the score is not silently inflated — in both the CLI and the vite plugin's build mode. Previously the CLI died with exit 2 and the vite plugin skipped the entire analysis (and its build gate) with a single "analysis failed" warning; both now finish with real results for every other rule.

packages/cli/src/index.ts

@@ -18,6 +18,7 @@ import {
   applyRuleSeverities,
   applyOverrides,
   settingSeverity,
+  withFailedRulesOff,
   type Severity,
   type RuleSetting,
   type RuleOverride,
@@ -261,6 +262,15 @@ function skippedFileWarnings(facts: { file: string; parseFailed?: true }[]): str
   ];
 }
 
+/**
+ * Warn about rules `runRules` caught throwing (dev tooling must never throw): the run completes
+ * without them, so any findings they would have produced are simply missing rather than reported
+ * as clean. Message capped to its first line so a multi-line stack trace can't flood the terminal.
+ */
+function failedRuleWarnings(failedRules: { id: string; message: string }[]): string[] {
+  return failedRules.map((f) => `rule ${f.id} failed and was skipped: ${f.message.split('\n')[0]}`);
+}
+
 /**
  * Run static-mode analysis and return the structured findings + resolved config.
  * Throws ProjectError when `cwd` is not a SvelteKit project. Also throws when a
@@ -307,7 +317,11 @@ export async function analyzeProject(opts: AnalyzeOptions = {}): Promise<Analyze
   });
   const selected = selectRules(allRules, config);
   const rules = opts.categories ? selected.filter((r) => opts.categories!.includes(r.category)) : selected;
-  const { results: rawResults, examined } = await runRules(rules, {
+  const {
+    results: rawResults,
+    examined,
+    failedRules
+  } = await runRules(rules, {
     heads,
     images,
     headings,
@@ -318,13 +332,21 @@ export async function analyzeProject(opts: AnalyzeOptions = {}): Promise<Analyze
     sourceFiles
   });
   const results = applyOverrides(applyRuleSeverities(rawResults, config), config);
+  // A failed rule examined nothing, so its weight must not stay in the Health denominator — else it
+  // would score as if it had run clean. Returned as the config this function hands back (not just a
+  // local copy) so every downstream consumer — CLI health/exit-code checks and the reporters, which
+  // each recompute Health from `config` — agrees on the same score.
+  const scoringConfig = withFailedRulesOff(
+    config,
+    failedRules.map((f) => f.id)
+  );
   return {
     results,
-    config,
+    config: scoringConfig,
     version: readPackageVersion(),
     ruleIds: rules.map((r) => r.id),
     examined,
-    warnings: [...warnings, ...skippedFileWarnings([...components, ...kitModules])],
+    warnings: [...warnings, ...skippedFileWarnings([...components, ...kitModules]), ...failedRuleWarnings(failedRules)],
     loadedConfig: loaded
   };
 }

packages/cli/test/rule-failure-isolation.test.ts

@@ -0,0 +1,59 @@
+import { describe, it, expect, vi } from 'vitest';
+import { fileURLToPath } from 'node:url';
+import { dirname, join } from 'node:path';
+
+const here = dirname(fileURLToPath(import.meta.url));
+const fixtureDir = join(here, 'fixtures', 'basic-project');
+
+const THROWN_MESSAGE = 'synthetic rule failure (test)\nwith a second line a warning must not print';
+
+// Replaces one real rule's `check` with a throwing stub, keeping the rest of the registry (and
+// every other export) untouched — the smallest way to prove `run()` survives a crashed rule
+// end-to-end without hand-rolling a fake analysis pipeline.
+vi.mock('@svelte-vitals/core', async (importOriginal) => {
+  const actual = await importOriginal<typeof import('@svelte-vitals/core')>();
+  const allRules = actual.allRules.map((rule) =>
+    rule.id === 'seo/title-presence'
+      ? {
+          ...rule,
+          check: async () => {
+            throw new Error(THROWN_MESSAGE);
+          }
+        }
+      : rule
+  );
+  return { ...actual, allRules };
+});
+
+const { run } = await import('../src/index.js');
+
+function capture() {
+  const out: string[] = [];
+  const err: string[] = [];
+  return { out, err, log: (line: string) => out.push(line), errorLog: (line: string) => err.push(line) };
+}
+
+describe('rule-failure isolation (audit 2608-CORE-06)', () => {
+  it('a crashed rule does not kill the run: a clean run with only the crashed rule selected exits 0', async () => {
+    const cap = capture();
+    // --rules restricts the run to exactly the crashed rule, so nothing else can contribute a
+    // finding — isolating whether the crash itself changes the exit code.
+    const code = await run({
+      cwd: fixtureDir,
+      log: cap.log,
+      errorLog: cap.errorLog,
+      env: {},
+      allowRules: ['seo/title-presence']
+    });
+    expect(code).toBe(0);
+  });
+
+  it('warns on stderr with the rule id and only the first line of its message', async () => {
+    const cap = capture();
+    await run({ cwd: fixtureDir, log: cap.log, errorLog: cap.errorLog, env: {}, allowRules: ['seo/title-presence'] });
+    expect(cap.err).toContainEqual(
+      'svelte-vitals: rule seo/title-presence failed and was skipped: synthetic rule failure (test)'
+    );
+    expect(cap.err.some((l) => l.includes('with a second line'))).toBe(false);
+  });
+});

packages/core/src/config-apply.ts

@@ -18,6 +18,24 @@ export function selectRules(rules: Rule[], config: Config): Rule[] {
   return rules.filter((rule) => settingSeverity(config.rules[rule.id]) !== 'off');
 }
 
+/**
+ * `config` with `failedRuleIds` (from `runRules`' `failedRules`) forced `'off'`: a rule that threw
+ * examined nothing, so leaving it in the inventory would score it as if it had run clean, silently
+ * inflating Health. Reuses the exact mechanism a `rules: { id: 'off' }` config entry already gets —
+ * `selectRules`/`buildInventory` both drop an `'off'` id from the denominator — rather than adding a
+ * second, parallel notion of "not counted" for callers to keep in sync.
+ */
+export function withFailedRulesOff(config: Config, failedRuleIds: readonly string[]): Config {
+  if (failedRuleIds.length === 0) return config;
+  return {
+    ...config,
+    rules: {
+      ...config.rules,
+      ...Object.fromEntries(failedRuleIds.map((id): [string, RuleSetting] => [id, 'off']))
+    }
+  };
+}
+
 /** Apply per-rule severity overrides to results (design §6). */
 export function applyRuleSeverities(results: Result[], config: Config): Result[] {
   return results.map((result) => {

packages/core/src/engine.ts

@@ -1,20 +1,41 @@
 import type { Result } from './types.js';
 import type { Rule, RuleContext } from './rule.js';
 
+export interface FailedRule {
+  id: string;
+  message: string;
+}
+
 /**
  * Run a set of rules against a shared context and collect their findings.
  * Rules are independent, so they run concurrently; results are flattened in
- * rule order for stable output.
+ * rule order for stable output. A rule that throws (sync or async) contributes
+ * no results instead of taking the whole run down with it — dev tooling must
+ * never throw — and is reported in `failedRules` instead.
  */
 export async function runRules(
   rules: Rule[],
   ctx: RuleContext
-): Promise<{ results: Result[]; examined: Record<string, Record<string, number>> }> {
+): Promise<{ results: Result[]; examined: Record<string, Record<string, number>>; failedRules: FailedRule[] }> {
   const examined: Record<string, Record<string, number>> = {};
   // The engine supplies the sink rather than each caller: three call sites thread this context, and a
   // caller that forgot would drop the counts silently — the failure this feature exists to remove.
+  // Each entry resolves to either its results or its failure — never rejects — so `Promise.all`
+  // preserves rule order regardless of completion order, matching `results`' own ordering guarantee.
   const perRule = await Promise.all(
-    rules.map((rule) => rule.check({ ...ctx, recordExamined: (counts) => void (examined[rule.id] = counts) }))
+    rules.map(async (rule): Promise<Result[] | FailedRule> => {
+      try {
+        return await rule.check({ ...ctx, recordExamined: (counts) => void (examined[rule.id] = counts) });
+      } catch (err) {
+        return { id: rule.id, message: err instanceof Error ? err.message : String(err) };
+      }
+    })
   );
-  return { results: perRule.flat(), examined };
+  const results: Result[] = [];
+  const failedRules: FailedRule[] = [];
+  for (const outcome of perRule) {
+    if (Array.isArray(outcome)) results.push(...outcome);
+    else failedRules.push(outcome);
+  }
+  return { results, examined, failedRules };
 }

packages/core/src/index.ts

@@ -64,6 +64,7 @@ export type { Rule, RuleContext } from './rule.js';
 export { isPenalized, docsUrlFor } from './rule.js';
 
 export { runRules } from './engine.js';
+export type { FailedRule } from './engine.js';
 export {
   allRules,
   explainRule,
@@ -170,7 +171,8 @@ export {
   compileOverrides,
   overrideMatches,
   settingSeverity,
-  settingOptions
+  settingOptions,
+  withFailedRulesOff
 } from './config-apply.js';
 export type { CompiledOverride } from './config-apply.js';
 

packages/core/src/rules/correctness/orphan-effect.ts

@@ -12,7 +12,8 @@ export const correctnessOrphanEffect = componentRule({
     'An $effect created outside component initialisation throws effect_orphan at runtime. The compiler does not catch it — the server compiler deletes $effect calls entirely, so SSR renders without error — and the crash happens client-side, when the module evaluates in the browser, breaking hydration rather than producing a server error.',
   // `orphanEffects` is typed required, but a facts object built by an older/external
   // constructor may omit it — default to empty rather than let `applies` throw and
-  // take the whole `runRules` Promise.all down with it.
+  // surface this rule as failed (the engine isolates a throwing rule, but this one
+  // can just work instead of getting flagged).
   applies: (c) => (c.orphanEffects ?? []).length > 0,
   bad: (c) =>
     (c.orphanEffects ?? []).map((o) => ({

packages/core/test/config-apply.test.ts

@@ -6,6 +6,7 @@ import {
   compileOverrides,
   overrideMatches,
   defineConfig,
+  withFailedRulesOff,
   type Rule,
   type Result
 } from '../src/index.js';
@@ -167,6 +168,24 @@ describe('config application', () => {
   });
 });
 
+describe('withFailedRulesOff', () => {
+  it('returns the same config reference when nothing failed', () => {
+    const config = defineConfig({ rules: { 'seo/json-ld': 'critical' } });
+    expect(withFailedRulesOff(config, [])).toBe(config);
+  });
+  it('forces the given ids off without touching other rules’ settings', () => {
+    const config = defineConfig({ rules: { 'seo/json-ld': 'critical' } });
+    const out = withFailedRulesOff(config, ['seo/title-presence']);
+    expect(out.rules['seo/json-ld']).toBe('critical');
+    expect(out.rules['seo/title-presence']).toBe('off');
+  });
+  it('drops a failed rule from selectRules, same as an explicit off setting', () => {
+    const config = withFailedRulesOff(defineConfig({}), ['seo/json-ld']);
+    const kept = selectRules([ruleA, ruleB], config);
+    expect(kept.map((r) => r.id)).toEqual(['seo/title-presence']);
+  });
+});
+
 describe('override matching', () => {
   const config = defineConfig({
     overrides: [{ files: 'src/lib/**', rules: { 'architecture/prop-count': 'off' } }]

packages/core/test/engine.test.ts

@@ -41,6 +41,48 @@ function ruleThatDoesNot(id: string): Rule {
   } as unknown as Rule;
 }
 
+function ruleThatFindsOne(id: string): Rule {
+  return {
+    id,
+    title: id,
+    category: 'architecture',
+    severity: 'info',
+    scope: 'component',
+    rationale: '',
+    async check() {
+      return [
+        {
+          id,
+          category: 'architecture',
+          severity: 'info',
+          message: 'x',
+          detection: { presence: 'none', value: 'absent' }
+        }
+      ];
+    }
+  } as unknown as Rule;
+}
+
+/** `sync` throws before ever returning a promise; the default (async) rejects the promise `check` returns. */
+function ruleThatThrows(id: string, message: string, mode: 'sync' | 'async' = 'async'): Rule {
+  return {
+    id,
+    title: id,
+    category: 'architecture',
+    severity: 'info',
+    scope: 'component',
+    rationale: '',
+    check:
+      mode === 'sync'
+        ? () => {
+            throw new Error(message);
+          }
+        : async () => {
+            throw new Error(message);
+          }
+  } as unknown as Rule;
+}
+
 describe('runRules examined counts', () => {
   it('keys a rule’s counts by its id', async () => {
     const { examined } = await runRules([ruleThatCounts('a/one', { 'x → y': 3 })], ctx);
@@ -107,3 +149,44 @@ describe('runRules examined counts — the four architecture directory rules tog
     expect(examined['architecture/unit-entry-file']?.['src/lib/svc/*']).toBe(1);
   });
 });
+
+describe('runRules — a throwing rule is isolated', () => {
+  it('does not throw itself, and leaves the healthy rules’ results intact', async () => {
+    const { results, failedRules } = await runRules(
+      [ruleThatFindsOne('a/before'), ruleThatThrows('a/boom', 'kaboom'), ruleThatFindsOne('a/after')],
+      ctx
+    );
+    expect(results.map((r) => r.id)).toEqual(['a/before', 'a/after']);
+    expect(failedRules).toEqual([{ id: 'a/boom', message: 'kaboom' }]);
+  });
+
+  it('reports the failed rule’s id and message in failedRules', async () => {
+    const { failedRules } = await runRules([ruleThatThrows('a/boom', 'kaboom')], ctx);
+    expect(failedRules).toEqual([{ id: 'a/boom', message: 'kaboom' }]);
+  });
+
+  it('keeps a healthy rule’s examined counts when a sibling throws', async () => {
+    const { examined, failedRules } = await runRules(
+      [ruleThatCounts('a/fine', { g: 1 }), ruleThatThrows('a/boom', 'kaboom')],
+      ctx
+    );
+    expect(examined).toEqual({ 'a/fine': { g: 1 } });
+    expect(failedRules.map((f) => f.id)).toEqual(['a/boom']);
+  });
+
+  it('isolates a rule that throws synchronously, not just one that rejects its promise', async () => {
+    const { results, failedRules } = await runRules([ruleThatThrows('a/sync-boom', 'sync kaboom', 'sync')], ctx);
+    expect(results).toEqual([]);
+    expect(failedRules).toEqual([{ id: 'a/sync-boom', message: 'sync kaboom' }]);
+  });
+
+  it('isolates a rule that rejects its promise (the default async throw)', async () => {
+    const { failedRules } = await runRules([ruleThatThrows('a/async-boom', 'async kaboom', 'async')], ctx);
+    expect(failedRules).toEqual([{ id: 'a/async-boom', message: 'async kaboom' }]);
+  });
+
+  it('gives failedRules an empty array when nothing failed', async () => {
+    const { failedRules } = await runRules([ruleThatDoesNot('a/fine')], ctx);
+    expect(failedRules).toEqual([]);
+  });
+});

packages/core/test/score.test.ts

@@ -1,6 +1,6 @@
 // Scores are floored, not rounded (2026-07-31): a displayed 100 means the deduction was exactly zero.
 import { describe, it, expect } from 'vitest';
-import { computeScore, defineConfig, scoresByCategory, type Result } from '../src/index.js';
+import { computeScore, defineConfig, scoresByCategory, withFailedRulesOff, type Result } from '../src/index.js';
 import type { Rule } from '../src/rule.js';
 import { buildInventory, DEDUCTION } from '../src/scoring/inventory.js';
 import { INVENTORY_FLOOR } from '../src/scoring/score.js';
@@ -543,6 +543,26 @@ describe('computeScore — a disabled rule injected via options.rules', () => {
   });
 });
 
+describe('computeScore — a rule that threw at runtime (withFailedRulesOff)', () => {
+  // Six warning rules share a pair (inventory 30, above the floor). Rule a/5 "crashed" — it
+  // produced no results, exactly like one that ran clean, so its own absence from `results`
+  // can't distinguish the two cases; only the config it's scored under can.
+  const rules = Array.from({ length: 6 }, (_, i) => r(`a/${i}`, 'architecture', 'component', 'warning'));
+  const results = [fail('a/0', 'src/A.svelte', 'warning')];
+
+  it('WITH the crashed rule still counted in the inventory (pre-fix), it scores as if a/5 passed', () => {
+    const config = defineConfig({});
+    const { score } = computeScore(results, config, { rules, applyCriticalCap: false });
+    expect(score).toBe(83); // 100 - 500/30
+  });
+
+  it('WITHOUT it — withFailedRulesOff drops its weight — the same failure scores lower, not inflated', () => {
+    const config = withFailedRulesOff(defineConfig({}), ['a/5']);
+    const { score } = computeScore(results, config, { rules, applyCriticalCap: false });
+    expect(score).toBe(80); // 100 - 500/25 (inventory 25, floor)
+  });
+});
+
 describe('computeScore — inventory floor', () => {
   const config = defineConfig({});