Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 / 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

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

Expand Down
37 changes: 27 additions & 10 deletions docs/AUDIT-PERFECTION-PLAN-v1.23.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand All @@ -56,18 +56,35 @@ 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/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
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.

---

Expand Down
132 changes: 132 additions & 0 deletions services/ai/aiErrorTaxonomy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
/**
* 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'
| 'canceled'
| '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<AiErrorCategory> = new Set<AiErrorCategory>([
'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<string, unknown>): 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<string, unknown>)['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';
}
// 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)) {
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<string, unknown>;

// 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);
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');
}
44 changes: 39 additions & 5 deletions services/ai/aiRetry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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). */
Expand All @@ -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<void> {
Expand Down Expand Up @@ -103,18 +119,36 @@ function clampRetryAfter(ms: number): number {

export async function withTransientRetry<T>(fn: () => Promise<T>, opts?: RetryOptions): Promise<T> {
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));
Expand Down
15 changes: 15 additions & 0 deletions tests/unit/aiRetry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, unknown> => ({
debug: noop,
info: noop,
warn: noop,
error: noop,
withContext: () => make(),
});
return { createLogger: () => make() };
});

import {
AI_RETRY_MAX_RETRY_AFTER_MS,
computeRetryDelayMs,
Expand Down
15 changes: 15 additions & 0 deletions tests/unit/aiRetryChain.test.ts
Original file line number Diff line number Diff line change
@@ -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<string, unknown> => ({
debug: noop,
info: noop,
warn: noop,
error: noop,
withContext: () => make(),
});
return { createLogger: () => make() };
});

import { withTransientRetry } from '../../services/ai/aiRetry';

describe('withTransientRetry', () => {
Expand Down
Loading
Loading