From 7813bdca130aec0d3a8787c071a0147483903d27 Mon Sep 17 00:00:00 2001 From: qnbs <155236708+qnbs@users.noreply.github.com> Date: Sat, 13 Jun 2026 13:45:19 +0200 Subject: [PATCH 1/3] feat(ai): error taxonomy + fail-fast retry classification MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 1 · Batch 1.1 (Master Prompt v1.1 engagement). - services/ai/aiErrorTaxonomy.ts: pure classifyAiError(err) -> {category, retryable, messageKey} across transient/rateLimit/auth/network/ offline/policy/invalidRequest/permanent (HTTP status + aiPolicy 'blocked' markers + offline short-circuit). Table-driven tests (26). - services/ai/aiRetry.ts: withTransientRetry now fails fast on non-retryable errors (auth/policy/invalid-request/offline) via a shouldRetry default, instead of backing off a doomed call. Transient/rateLimit/network still retry (Retry-After honored). One structured ai.retry log line per decision with a per-call correlation id (no payloads/keys). Backward compatible. - Tests: fail-fast cases + custom shouldRetry override; logger stubbed in the 3 retry test files for deterministic output. - Docs: living plan batch log + CHANGELOG [Unreleased]. FU-1 (plugin-worker guard restoration) SPLIT from this batch after a timeboxed probe: the Function.prototype.constructor leak is real but its mechanism is subtler than a symmetric defineProperty fix; documented in the living plan for a dedicated fix. Benign (worker compiles via captured GlobalFunction). i18n/UI mapping of messageKey deferred to Batch 1.2. Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 5 + docs/AUDIT-PERFECTION-PLAN-v1.23.md | 36 ++++-- services/ai/aiErrorTaxonomy.ts | 121 +++++++++++++++++ services/ai/aiRetry.ts | 44 ++++++- tests/unit/aiRetry.test.ts | 15 +++ tests/unit/aiRetryChain.test.ts | 15 +++ tests/unit/services/aiErrorTaxonomy.test.ts | 136 ++++++++++++++++++++ tests/unit/services/aiRetry.test.ts | 79 ++++++++++++ 8 files changed, 436 insertions(+), 15 deletions(-) create mode 100644 services/ai/aiErrorTaxonomy.ts create mode 100644 tests/unit/services/aiErrorTaxonomy.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index cb4add40d..bad006593 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - **Plugin sandbox adversarial test coverage:** `tests/unit/workers/plugin.worker.test.ts` gains WebAssembly-denial, `GeneratorFunction`/`AsyncGeneratorFunction` constructor-escape, and guard-restoration (success + error path) tests for the v1.22 plugin-isolation hardening. New living audit artifact `docs/AUDIT-PERFECTION-PLAN-v1.23.md` tracks the 6-phase perfection engagement and its follow-ups. +- **AI error taxonomy (`services/ai/aiErrorTaxonomy.ts`):** pure `classifyAiError(err)` → `{ category, retryable, messageKey }` across transient / rateLimit / auth / network / offline / policy / invalidRequest / permanent. Consumed by the retry layer; a stable `messageKey` is exposed for the upcoming UI recovery mapping (Batch 1.2). + +### Fixed + +- **AI retry no longer backs off doomed calls:** `withTransientRetry` now classifies the error and **fails fast** on non-retryable categories (invalid API key, policy block, malformed request, offline) instead of retrying with exponential backoff. Transient / rate-limit / network errors still retry (honoring `Retry-After`). Each retry decision emits a structured `ai.retry` log line with a per-call correlation id (no payloads or keys). A `shouldRetry` option allows callers to override the default. ## [1.22.0] — 2026-06-11 diff --git a/docs/AUDIT-PERFECTION-PLAN-v1.23.md b/docs/AUDIT-PERFECTION-PLAN-v1.23.md index aaf726602..2cca2e5fa 100644 --- a/docs/AUDIT-PERFECTION-PLAN-v1.23.md +++ b/docs/AUDIT-PERFECTION-PLAN-v1.23.md @@ -28,8 +28,8 @@ the following are already shipped — do **not** re-plan them as gaps: | Phase | Theme | Status | |---|---|---| -| **0** | v1.23 foundation: tracker sync, dependency hygiene, plugin sandbox validation | ✅ **Closing** (PR #118) | -| 1 | Reliability/Observability + Local-AI/Voice low-end hardening | ⬜ Planned | +| **0** | v1.23 foundation: tracker sync, dependency hygiene, plugin sandbox validation | ✅ **Merged** (PR #118 → `8f94178`) | +| **1** | Reliability/Observability + Local-AI/Voice low-end hardening | 🔄 **In progress** (Batch 1.1) | | 2 | Coverage elevation (L≥85 / B≥75 / F≥80) in AI routing, Copilot v2, Voice, collab, PlotBoard | ⬜ Planned | | 3 | Tauri desktop: signing / notarization / updater pipeline + UX | ⬜ Planned | | 4 | WCAG 2.2 AA manual audit + i18n sustainability (<2% placeholders) | ⬜ Planned | @@ -56,18 +56,34 @@ a CodeAnt-caught regression. (prefix / length / `..` traversal / path separators / empty suffix) **and** the 2 MiB value-size cap — no new registry tests needed. +### Batch 1.1 — AI error taxonomy + fail-fast retry (Phase 1) +- New `services/ai/aiErrorTaxonomy.ts` — pure `classifyAiError(err)` → + `{category, retryable, messageKey}` over transient/rateLimit/auth/network/offline/policy/ + invalidRequest/permanent (reuses the `aiPolicy.ts` "blocked" markers + HTTP status). +- `services/ai/aiRetry.ts` `withTransientRetry` now **fails fast** on non-retryable errors + (auth/policy/invalid-request/offline) via a `shouldRetry` default, and emits a structured + `createLogger('ai.retry')` line per decision with a per-call correlation id. Backward + compatible (unknown errors still retry). +33 tests (`aiErrorTaxonomy.test.ts` table-driven, + fail-fast cases in `aiRetry.test.ts`). +- **i18n / UI mapping of `messageKey` deferred to Batch 1.2** (avoids 11-locale churn here). +- **FU-1 split out of this batch** (see follow-ups). + --- ## 4. Decisions & follow-ups -- **FU-1 (Phase 1 candidate, low impact):** `workers/plugin.worker.ts` - `restoreRuntimeGuards` restores the `self.Function/eval/WebAssembly` bindings correctly, - but `Function.prototype.constructor` does **not** round-trip to its pre-call value across - runs (each run leaves a fresh denied constructor). Benign in production — - `createSandboxedRunner` compiles via the captured `GlobalFunction`, not - `Function.prototype.constructor` — but the install/restore pair is asymmetric and worth a - small source fix + assertion. Not fixed in the test-only Phase 0 PR by policy (no source - changes smuggled into a test batch). +- **FU-1 (open — needs a dedicated fix; SPLIT from Batch 1.1):** `workers/plugin.worker.ts` + `restoreRuntimeGuards` restores `self.Function/eval/WebAssembly` correctly, but + `Function.prototype.constructor` does **not** round-trip — after a single isolated run it is + left as the **`Function`-variant `deniedConstructor`** (confirmed by probe: + `beforeIsNative=true`, `afterIsNative=false`, leaked message "Function constructor is + disabled"; the async/generator prototypes are genuinely distinct, ruling out shared-prototype + aliasing). A naive symmetric `Object.defineProperty` restore did **not** fix it, so the + mechanism is subtler (likely a snapshot-capture / property-attribute interaction) and needs a + focused fix with its own assertion. **Benign in production** — `createSandboxedRunner` + compiles via the captured `GlobalFunction`, not `Function.prototype.constructor` — so it does + not weaken isolation; it is a worker-hygiene leak. Timeboxed out of Batch 1.1 per plan to keep + the taxonomy PR clean. --- diff --git a/services/ai/aiErrorTaxonomy.ts b/services/ai/aiErrorTaxonomy.ts new file mode 100644 index 000000000..1c30d98c8 --- /dev/null +++ b/services/ai/aiErrorTaxonomy.ts @@ -0,0 +1,121 @@ +/** + * AI error taxonomy. + * QNBS-v3: P1 (Batch 1.1) — classify AI/provider errors so the retry layer can fail fast on + * non-retryable errors (auth, policy, invalid request, offline) instead of backing off + * a doomed call, and so the UI (Batch 1.2) can map a stable `messageKey` to an + * actionable hint. Pure + dependency-free; consumed by `aiRetry.withTransientRetry`. + */ + +export type AiErrorCategory = + | 'transient' + | 'rateLimit' + | 'auth' + | 'network' + | 'offline' + | 'policy' + | 'invalidRequest' + | 'permanent'; + +export interface AiErrorClassification { + readonly category: AiErrorCategory; + /** Whether retrying the same call could plausibly succeed. */ + readonly retryable: boolean; + /** Stable i18n key — wired into locales + UI recovery in Batch 1.2. */ + readonly messageKey: string; +} + +// QNBS-v3: Only connection-class failures are worth a retry. auth/policy/invalidRequest are +// deterministic (a retry repeats the same failure); offline is doomed until connectivity returns. +const RETRYABLE: ReadonlySet = new Set([ + 'transient', + 'rateLimit', + 'network', +]); + +function classificationFor(category: AiErrorCategory): AiErrorClassification { + return { category, retryable: RETRYABLE.has(category), messageKey: `errors.ai.${category}` }; +} + +/** Best-effort numeric HTTP status from common provider/SDK error shapes. */ +function extractStatus(err: Record): number | undefined { + for (const key of ['status', 'statusCode'] as const) { + const v = err[key]; + if (typeof v === 'number' && Number.isFinite(v)) return v; + } + const response = err['response']; + if (response && typeof response === 'object') { + const s = (response as Record)['status']; + if (typeof s === 'number' && Number.isFinite(s)) return s; + } + return undefined; +} + +function categoryFromStatus(status: number): AiErrorCategory | undefined { + if (status === 429) return 'rateLimit'; + if (status === 401 || status === 403) return 'auth'; + if (status === 408) return 'transient'; // request timeout — worth a retry + if (status >= 400 && status < 500) return 'invalidRequest'; + if (status >= 500) return 'transient'; + return undefined; +} + +function categoryFromMessage(message: string): AiErrorCategory | undefined { + const m = message.toLowerCase(); + // Policy-gate markers from services/ai/aiPolicy.ts all read "...blocked...". + if (m.includes('blocked')) return 'policy'; + if (/(rate limit|too many requests|\b429\b)/.test(m)) return 'rateLimit'; + if (/(unauthorized|forbidden|invalid api key|api key|authentication|\b401\b|\b403\b)/.test(m)) { + return 'auth'; + } + if (/(invalid request|bad request|unprocessable|\b400\b|\b404\b|\b422\b)/.test(m)) { + return 'invalidRequest'; + } + if (/(timeout|timed out|etimedout|econnreset|socket hang up|\baborted\b)/.test(m)) { + return 'transient'; + } + if (/(failed to fetch|networkerror|network error|enotfound|econnrefused|fetch failed)/.test(m)) { + return 'network'; + } + return undefined; +} + +function isOffline(): boolean { + return ( + typeof navigator !== 'undefined' && + typeof navigator.onLine === 'boolean' && + navigator.onLine === false + ); +} + +/** + * Classify an arbitrary thrown value from the AI/provider layer. + * Ordering: offline (device-level) → HTTP status → message markers → conservative default. + * Unknown/unclassifiable errors default to retryable `transient` to preserve the historical + * "retry on failure" behavior; only confidently-classified deterministic errors fail fast. + */ +export function classifyAiError(err: unknown): AiErrorClassification { + // Offline trumps everything — retrying a cloud call while the device is offline is doomed. + if (isOffline()) return classificationFor('offline'); + + if (typeof err !== 'object' || err === null) { + return classificationFor('transient'); + } + const e = err as Record; + + const status = extractStatus(e); + if (status !== undefined) { + const byStatus = categoryFromStatus(status); + if (byStatus) return classificationFor(byStatus); + } + + const message = typeof e['message'] === 'string' ? e['message'] : ''; + if (message) { + const byMessage = categoryFromMessage(message); + if (byMessage) return classificationFor(byMessage); + } + + // A `fetch` network failure is commonly a bare TypeError mentioning fetch. + if (e['name'] === 'TypeError' && /fetch/i.test(message)) return classificationFor('network'); + + return classificationFor('transient'); +} diff --git a/services/ai/aiRetry.ts b/services/ai/aiRetry.ts index c6d33c36e..6f94b49be 100644 --- a/services/ai/aiRetry.ts +++ b/services/ai/aiRetry.ts @@ -3,8 +3,18 @@ * QNBS-v3: P1-F5 — exponential backoff with full jitter + `Retry-After` parsing (was linear). * Cloud providers (429/503) get backed off with jitter to avoid thundering-herd; * a server-supplied `Retry-After` always takes precedence over the computed delay. + * P1 Batch 1.1 — fail fast on non-retryable errors (auth/policy/invalid request/offline) + * via {@link classifyAiError}, instead of backing off a call that cannot succeed. */ +import { createLogger } from '../logger'; +import { classifyAiError } from './aiErrorTaxonomy'; + +const log = createLogger('ai.retry'); +// QNBS-v3: Ties the log lines of one retry chain together. Per-call sequence id (no Date/random) +// — full cross-thread correlation-id propagation is a later Phase 1 increment. +let retrySeq = 0; + export const DEFAULT_AI_RETRY_ATTEMPTS = 2; export const AI_RETRY_BASE_DELAY_MS = 400; /** Cap for the computed exponential delay (before honoring a server Retry-After). */ @@ -21,6 +31,12 @@ export interface RetryOptions { jitter?: boolean; /** Injectable RNG (0..1) — override for deterministic tests. Default Math.random. */ rng?: () => number; + /** + * Predicate deciding whether a thrown error is worth retrying. + * Default: `classifyAiError(err).retryable` — auth/policy/invalid-request/offline fail fast, + * transient/rate-limit/network back off and retry. Pass a custom predicate to override. + */ + shouldRetry?: (err: unknown) => boolean; } function delay(ms: number): Promise { @@ -103,18 +119,36 @@ function clampRetryAfter(ms: number): number { export async function withTransientRetry(fn: () => Promise, opts?: RetryOptions): Promise { const attempts = opts?.attempts ?? DEFAULT_AI_RETRY_ATTEMPTS; + const shouldRetry = opts?.shouldRetry ?? ((err: unknown) => classifyAiError(err).retryable); + const correlationId = `air-${++retrySeq}`; let lastError: unknown; for (let i = 0; i < attempts; i++) { try { return await fn(); } catch (err) { lastError = err; - if (i < attempts - 1) { - // QNBS-v3: server Retry-After wins over the computed backoff. - const retryAfter = parseRetryAfterMs(err); - const waitMs = retryAfter ?? computeRetryDelayMs(i, opts); - await delay(waitMs); + if (i >= attempts - 1) break; + // QNBS-v3: fail fast — a non-retryable error (auth/policy/invalid request/offline) won't + // succeed on retry; surface it immediately instead of backing off a doomed call. + if (!shouldRetry(err)) { + log + .withContext({ correlationId, category: classifyAiError(err).category }) + .info('AI error is non-retryable; failing fast'); + break; } + // QNBS-v3: server Retry-After wins over the computed backoff. + const retryAfter = parseRetryAfterMs(err); + const waitMs = retryAfter ?? computeRetryDelayMs(i, opts); + log + .withContext({ + correlationId, + attempt: i + 1, + of: attempts, + category: classifyAiError(err).category, + waitMs: Math.round(waitMs), + }) + .info('retrying transient AI error'); + await delay(waitMs); } } throw lastError instanceof Error ? lastError : new Error(String(lastError)); diff --git a/tests/unit/aiRetry.test.ts b/tests/unit/aiRetry.test.ts index e36d05413..841645e20 100644 --- a/tests/unit/aiRetry.test.ts +++ b/tests/unit/aiRetry.test.ts @@ -4,6 +4,21 @@ */ import { describe, expect, it, vi } from 'vitest'; + +// QNBS-v3: aiRetry emits structured logs on retry decisions — stub the logger to keep test +// output clean and deterministic. +vi.mock('../../services/logger', () => { + const noop = (): void => {}; + const make = (): Record => ({ + debug: noop, + info: noop, + warn: noop, + error: noop, + withContext: () => make(), + }); + return { createLogger: () => make() }; +}); + import { AI_RETRY_MAX_RETRY_AFTER_MS, computeRetryDelayMs, diff --git a/tests/unit/aiRetryChain.test.ts b/tests/unit/aiRetryChain.test.ts index 599505d19..69858f417 100644 --- a/tests/unit/aiRetryChain.test.ts +++ b/tests/unit/aiRetryChain.test.ts @@ -1,4 +1,19 @@ import { describe, expect, it, vi } from 'vitest'; + +// QNBS-v3: aiRetry emits structured logs on retry decisions — stub the logger to keep test +// output clean and deterministic. +vi.mock('../../services/logger', () => { + const noop = (): void => {}; + const make = (): Record => ({ + debug: noop, + info: noop, + warn: noop, + error: noop, + withContext: () => make(), + }); + return { createLogger: () => make() }; +}); + import { withTransientRetry } from '../../services/ai/aiRetry'; describe('withTransientRetry', () => { diff --git a/tests/unit/services/aiErrorTaxonomy.test.ts b/tests/unit/services/aiErrorTaxonomy.test.ts new file mode 100644 index 000000000..40d460213 --- /dev/null +++ b/tests/unit/services/aiErrorTaxonomy.test.ts @@ -0,0 +1,136 @@ +/** + * Tests for services/ai/aiErrorTaxonomy.ts + * QNBS-v3: Table-driven classification across every category + the offline short-circuit. + */ + +import { afterEach, describe, expect, it } from 'vitest'; +import { type AiErrorCategory, classifyAiError } from '../../../services/ai/aiErrorTaxonomy'; + +interface Case { + readonly name: string; + readonly err: unknown; + readonly category: AiErrorCategory; + readonly retryable: boolean; +} + +const CASES: readonly Case[] = [ + // --- by HTTP status ------------------------------------------------------- + { name: '429 status', err: { status: 429 }, category: 'rateLimit', retryable: true }, + { name: '401 status', err: { status: 401 }, category: 'auth', retryable: false }, + { name: '403 status', err: { status: 403 }, category: 'auth', retryable: false }, + { name: '408 status', err: { status: 408 }, category: 'transient', retryable: true }, + { name: '400 status', err: { status: 400 }, category: 'invalidRequest', retryable: false }, + { name: '404 status', err: { status: 404 }, category: 'invalidRequest', retryable: false }, + { name: '422 status', err: { status: 422 }, category: 'invalidRequest', retryable: false }, + { name: '500 status', err: { status: 500 }, category: 'transient', retryable: true }, + { name: '503 status', err: { status: 503 }, category: 'transient', retryable: true }, + { name: 'statusCode field', err: { statusCode: 401 }, category: 'auth', retryable: false }, + { + name: 'nested response.status', + err: { response: { status: 429 } }, + category: 'rateLimit', + retryable: true, + }, + // --- by message markers --------------------------------------------------- + { + name: 'policy: local-only block', + err: new Error('Cloud provider blocked: local-only mode is active.'), + category: 'policy', + retryable: false, + }, + { + name: 'policy: LoRA training block', + err: new Error('LoRA training blocked: model "openai/x" appears to be cloud-hosted.'), + category: 'policy', + retryable: false, + }, + { + name: 'rate limit message', + err: new Error('Rate limit exceeded, try again later'), + category: 'rateLimit', + retryable: true, + }, + { + name: 'unauthorized message', + err: new Error('Unauthorized'), + category: 'auth', + retryable: false, + }, + { + name: 'invalid api key message', + err: new Error('Invalid API key provided'), + category: 'auth', + retryable: false, + }, + { + name: 'timeout message', + err: new Error('Request timed out'), + category: 'transient', + retryable: true, + }, + { + name: 'socket hang up message', + err: new Error('socket hang up'), + category: 'transient', + retryable: true, + }, + { + name: 'failed to fetch message', + err: new Error('Failed to fetch'), + category: 'network', + retryable: true, + }, + { + name: 'ENOTFOUND message', + err: new Error('getaddrinfo ENOTFOUND api.openai.com'), + category: 'network', + retryable: true, + }, + { + name: 'fetch TypeError', + err: new TypeError('NetworkError when attempting to fetch resource'), + category: 'network', + retryable: true, + }, + // --- conservative defaults ------------------------------------------------ + { + name: 'unknown Error → transient', + err: new Error('something weird happened'), + category: 'transient', + retryable: true, + }, + { name: 'bare string → transient', err: 'boom', category: 'transient', retryable: true }, + { name: 'null → transient', err: null, category: 'transient', retryable: true }, +]; + +describe('classifyAiError', () => { + for (const c of CASES) { + it(`classifies ${c.name} as ${c.category} (retryable=${c.retryable})`, () => { + const result = classifyAiError(c.err); + expect(result.category).toBe(c.category); + expect(result.retryable).toBe(c.retryable); + expect(result.messageKey).toBe(`errors.ai.${c.category}`); + }); + } + + it('status takes precedence over a misleading message', () => { + // 401 status with a generic message must classify as auth, not transient. + const result = classifyAiError({ status: 401, message: 'temporary glitch' }); + expect(result.category).toBe('auth'); + expect(result.retryable).toBe(false); + }); + + describe('offline short-circuit', () => { + afterEach(() => { + Object.defineProperty(navigator, 'onLine', { value: true, configurable: true }); + }); + + it('returns offline (non-retryable) when navigator is offline, even for a 429', () => { + Object.defineProperty(navigator, 'onLine', { value: false, configurable: true }); + const result = classifyAiError({ status: 429 }); + expect(result.category).toBe('offline'); + expect(result.retryable).toBe(false); + expect(result.messageKey).toBe('errors.ai.offline'); + }); + }); +}); diff --git a/tests/unit/services/aiRetry.test.ts b/tests/unit/services/aiRetry.test.ts index 48acc3899..75d26ee23 100644 --- a/tests/unit/services/aiRetry.test.ts +++ b/tests/unit/services/aiRetry.test.ts @@ -5,6 +5,21 @@ */ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +// QNBS-v3: aiRetry now emits structured logs on retry decisions — stub the logger so tests stay +// deterministic and free of IDB/console side effects. +vi.mock('../../../services/logger', () => { + const noop = (): void => {}; + const make = (): Record => ({ + debug: noop, + info: noop, + warn: noop, + error: noop, + withContext: () => make(), + }); + return { createLogger: () => make() }; +}); + import { AI_RETRY_BASE_DELAY_MS, AI_RETRY_MAX_DELAY_MS, @@ -192,4 +207,68 @@ describe('aiRetry', () => { expect(parseRetryAfterMs({ retryAfter: 10 ** 6 })).toBe(AI_RETRY_MAX_RETRY_AFTER_MS); }); }); + + describe('withTransientRetry — fail-fast classification', () => { + it('does not retry a non-retryable auth error (401)', async () => { + const err = Object.assign(new Error('Unauthorized'), { status: 401 }); + const fn = vi.fn().mockRejectedValue(err); + await expect(withTransientRetry(fn, { attempts: 3, baseDelayMs: 0 })).rejects.toThrow( + 'Unauthorized', + ); + // QNBS-v3: classified as auth → fails fast; fn invoked exactly once, no backoff. + expect(fn).toHaveBeenCalledTimes(1); + }); + + it('does not retry a policy-blocked error', async () => { + const fn = vi + .fn() + .mockRejectedValue(new Error('Cloud provider blocked: local-only mode is active.')); + await expect(withTransientRetry(fn, { attempts: 3, baseDelayMs: 0 })).rejects.toThrow( + /blocked/, + ); + expect(fn).toHaveBeenCalledTimes(1); + }); + + it('still retries a transient error by default classification', async () => { + vi.useFakeTimers(); + try { + const fn = vi + .fn() + .mockRejectedValueOnce(new Error('socket hang up')) + .mockResolvedValueOnce('ok'); + const check = expect( + withTransientRetry(fn, { attempts: 2, baseDelayMs: 10 }), + ).resolves.toBe('ok'); + await vi.runAllTimersAsync(); + await check; + expect(fn).toHaveBeenCalledTimes(2); + } finally { + vi.useRealTimers(); + } + }); + + it('a custom shouldRetry=false short-circuits an otherwise-retryable error', async () => { + const fn = vi.fn().mockRejectedValue(new Error('socket hang up')); // transient → retryable + await expect( + withTransientRetry(fn, { attempts: 3, baseDelayMs: 0, shouldRetry: () => false }), + ).rejects.toThrow('socket hang up'); + expect(fn).toHaveBeenCalledTimes(1); + }); + + it('a custom shouldRetry=true forces retry of an otherwise-non-retryable error', async () => { + vi.useFakeTimers(); + try { + const err = Object.assign(new Error('Unauthorized'), { status: 401 }); + const fn = vi.fn().mockRejectedValueOnce(err).mockResolvedValueOnce('recovered'); + const check = expect( + withTransientRetry(fn, { attempts: 2, baseDelayMs: 10, shouldRetry: () => true }), + ).resolves.toBe('recovered'); + await vi.runAllTimersAsync(); + await check; + expect(fn).toHaveBeenCalledTimes(2); + } finally { + vi.useRealTimers(); + } + }); + }); }); From b07c9b27f0ab2317b3a91654c0959527a824d1e1 Mon Sep 17 00:00:00 2001 From: qnbs <155236708+qnbs@users.noreply.github.com> Date: Sat, 13 Jun 2026 13:50:28 +0200 Subject: [PATCH 2/3] test(ai): use placeholder domain in taxonomy network-error fixture MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeAnt (Minor): avoid embedding a real service hostname in test text. api.openai.com -> api.example.invalid (RFC 2606/6761 reserved). The hostname is not asserted on — classification keys off the ENOTFOUND marker — so behavior is unchanged; 26/26 still green. Co-Authored-By: Claude Opus 4.8 --- tests/unit/services/aiErrorTaxonomy.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/services/aiErrorTaxonomy.test.ts b/tests/unit/services/aiErrorTaxonomy.test.ts index 40d460213..c1075dcab 100644 --- a/tests/unit/services/aiErrorTaxonomy.test.ts +++ b/tests/unit/services/aiErrorTaxonomy.test.ts @@ -82,7 +82,7 @@ const CASES: readonly Case[] = [ }, { name: 'ENOTFOUND message', - err: new Error('getaddrinfo ENOTFOUND api.openai.com'), + err: new Error('getaddrinfo ENOTFOUND api.example.invalid'), category: 'network', retryable: true, }, From cf465db58bf3648abcdc235f345100c292ff00aa Mon Sep 17 00:00:00 2001 From: qnbs <155236708+qnbs@users.noreply.github.com> Date: Sat, 13 Jun 2026 13:58:40 +0200 Subject: [PATCH 3/3] fix(ai): never retry deliberate cancellations (AbortError) CodeAnt: classifying messages containing 'aborted' as transient made user/timeout AbortController cancellations retryable. Add a non-retryable 'canceled' category: detect DOMException AbortError (name/code 20) and abort/cancel messages; remove 'aborted' from the transient regex. +4 tests (30 taxonomy / 58 total green). Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 2 +- docs/AUDIT-PERFECTION-PLAN-v1.23.md | 3 ++- services/ai/aiErrorTaxonomy.ts | 13 ++++++++++- tests/unit/services/aiErrorTaxonomy.test.ts | 25 +++++++++++++++++++++ 4 files changed, 40 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bad006593..0c5cad2f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - **Plugin sandbox adversarial test coverage:** `tests/unit/workers/plugin.worker.test.ts` gains WebAssembly-denial, `GeneratorFunction`/`AsyncGeneratorFunction` constructor-escape, and guard-restoration (success + error path) tests for the v1.22 plugin-isolation hardening. New living audit artifact `docs/AUDIT-PERFECTION-PLAN-v1.23.md` tracks the 6-phase perfection engagement and its follow-ups. -- **AI error taxonomy (`services/ai/aiErrorTaxonomy.ts`):** pure `classifyAiError(err)` → `{ category, retryable, messageKey }` across transient / rateLimit / auth / network / offline / policy / invalidRequest / permanent. Consumed by the retry layer; a stable `messageKey` is exposed for the upcoming UI recovery mapping (Batch 1.2). +- **AI error taxonomy (`services/ai/aiErrorTaxonomy.ts`):** pure `classifyAiError(err)` → `{ category, retryable, messageKey }` across transient / rateLimit / auth / network / offline / policy / invalidRequest / canceled / permanent (cancellations via `AbortError` fail fast — never retried). Consumed by the retry layer; a stable `messageKey` is exposed for the upcoming UI recovery mapping (Batch 1.2). ### Fixed diff --git a/docs/AUDIT-PERFECTION-PLAN-v1.23.md b/docs/AUDIT-PERFECTION-PLAN-v1.23.md index 2cca2e5fa..9185b6348 100644 --- a/docs/AUDIT-PERFECTION-PLAN-v1.23.md +++ b/docs/AUDIT-PERFECTION-PLAN-v1.23.md @@ -59,7 +59,8 @@ a CodeAnt-caught regression. ### Batch 1.1 — AI error taxonomy + fail-fast retry (Phase 1) - New `services/ai/aiErrorTaxonomy.ts` — pure `classifyAiError(err)` → `{category, retryable, messageKey}` over transient/rateLimit/auth/network/offline/policy/ - invalidRequest/permanent (reuses the `aiPolicy.ts` "blocked" markers + HTTP status). + invalidRequest/canceled/permanent (reuses the `aiPolicy.ts` "blocked" markers + HTTP status; + `AbortError` cancellations fail fast — CodeAnt-flagged). - `services/ai/aiRetry.ts` `withTransientRetry` now **fails fast** on non-retryable errors (auth/policy/invalid-request/offline) via a `shouldRetry` default, and emits a structured `createLogger('ai.retry')` line per decision with a per-call correlation id. Backward diff --git a/services/ai/aiErrorTaxonomy.ts b/services/ai/aiErrorTaxonomy.ts index 1c30d98c8..d667bd35d 100644 --- a/services/ai/aiErrorTaxonomy.ts +++ b/services/ai/aiErrorTaxonomy.ts @@ -14,6 +14,7 @@ export type AiErrorCategory = | 'offline' | 'policy' | 'invalidRequest' + | 'canceled' | 'permanent'; export interface AiErrorClassification { @@ -70,7 +71,11 @@ function categoryFromMessage(message: string): AiErrorCategory | undefined { if (/(invalid request|bad request|unprocessable|\b400\b|\b404\b|\b422\b)/.test(m)) { return 'invalidRequest'; } - if (/(timeout|timed out|etimedout|econnreset|socket hang up|\baborted\b)/.test(m)) { + // QNBS-v3: deliberate cancellation must fail fast — never retry a user/timeout abort. + if (/(\baborted\b|operation was aborted|\bcancell?ed\b)/.test(m)) { + return 'canceled'; + } + if (/(timeout|timed out|etimedout|econnreset|socket hang up)/.test(m)) { return 'transient'; } if (/(failed to fetch|networkerror|network error|enotfound|econnrefused|fetch failed)/.test(m)) { @@ -102,6 +107,12 @@ export function classifyAiError(err: unknown): AiErrorClassification { } const e = err as Record; + // QNBS-v3: a deliberate AbortController cancellation surfaces as DOMException 'AbortError' + // (or legacy code 20) — fail fast, retrying a cancelled request is never correct. + if (e['name'] === 'AbortError' || e['code'] === 20) { + return classificationFor('canceled'); + } + const status = extractStatus(e); if (status !== undefined) { const byStatus = categoryFromStatus(status); diff --git a/tests/unit/services/aiErrorTaxonomy.test.ts b/tests/unit/services/aiErrorTaxonomy.test.ts index c1075dcab..d593de01d 100644 --- a/tests/unit/services/aiErrorTaxonomy.test.ts +++ b/tests/unit/services/aiErrorTaxonomy.test.ts @@ -92,6 +92,31 @@ const CASES: readonly Case[] = [ category: 'network', retryable: true, }, + // --- cancellation (must fail fast, never retry) --------------------------- + { + name: 'AbortError by name', + err: Object.assign(new Error('signal aborted without reason'), { name: 'AbortError' }), + category: 'canceled', + retryable: false, + }, + { + name: 'DOMException abort code 20', + err: { code: 20, message: 'aborted' }, + category: 'canceled', + retryable: false, + }, + { + name: 'operation was aborted message', + err: new Error('The operation was aborted'), + category: 'canceled', + retryable: false, + }, + { + name: 'cancelled message', + err: new Error('Request cancelled by user'), + category: 'canceled', + retryable: false, + }, // --- conservative defaults ------------------------------------------------ { name: 'unknown Error → transient',