Retry engine and error classification: IoError rule, non-finite delayOverride, backoff NaN (#78)

docs/deviations.md

@@ -433,22 +433,38 @@ extend `DexpaceError` **directly** (lines 29, 51, 67, 83) and are grouped by the
 three-level branch.
 
 **Why it cannot be corrected.** `TRANSPORT-20` requires `TransportFailureError` to *be* an `IoError` — the
-subtyping is the requirement, not an artifact of modelling. It is also load-bearing: `classify.ts`'s
-cause-walk tests `current instanceof IoError` (`packages/core/src/retry/classify.ts:73`), so the `extends`
-is what makes a no-response transport failure retryable with zero edits to the retry layer. A flat sibling
+subtyping is the requirement, not an artifact of modelling. It is also load-bearing, and **what it bears is
+a boundary, not a category**: `classify.ts`'s cause-walk tests `current instanceof IoError`
+(`packages/core/src/retry/classify.ts:90`), and because the tree is flat that branch matches exactly two of
+the six classes in `io/errors.ts` — `IoError` itself and `TransportFailureError`. It does **not** match the
+four leaves `isIoError` groups. That is the intended reading, decided by audit #67 / #78 and stated here
+because it is this deviation's own consequence: the branch means "the wire failed". A send that produced no
+response is `RETRY-4`'s unconditionally-retryable condition, `TRANSPORT-20` names `TransportFailureError` as
+what carries it, and the `extends` is what routes it to the retry layer with no edit there. A flat sibling
 would have to be enumerated by hand in the retry classifier, and again for every transport added later —
 trading one level of depth for an open-ended maintenance obligation that the styleguide's own rule exists
 to prevent. Held at exactly three; a fourth level is not sanctioned by this entry.
 
-> **Anchor correction 2026-09-04 (audit #67 / #68); the rule itself is not decided here.** The line
-> numbers above were stale and are re-derived. The substantive point this paragraph used to make — "the
-> cause-walk returns retryable for any `IoError`" — reads as covering all five I/O classes, and it does
-> not: because the tree is flat, `instanceof IoError` at `classify.ts:73` matches `IoError` itself and
-> `TransportFailureError` only. `EndOfStreamError`, `SourceContractViolationError`,
-> `ClosedResourceError` and `AllocationLimitError` are **not** retryable through that branch. Whether
-> they should be — and therefore whether the classifier tests `instanceof IoError` or the `isIoError`
-> predicate — is decided by audit subtask #78, and this row's rationale is rewritten there. Nothing in
-> the deviation itself (the three-level branch, and why it stays) turns on that answer.
+**What the other four leaves are, and why they stay outside the branch.** `EndOfStreamError`,
+`SourceContractViolationError`, `ClosedResourceError` and `AllocationLimitError` are this package's own
+contract and lifecycle failures, and every one of them is deterministic on re-send: a closed resource
+(`IO-42`) and a source that returned zero bytes for a positive read (`IO-17`) are caller programming errors,
+an allocation cap (`IO-9`) is a limit the same request hits again, and `EndOfStreamError` is the
+exact-length-copy contract inside `io/` — a *wire* truncation is the transport's to report, as a
+`TransportFailureError`, which is the layer that can tell one from a complete short body. `RETRY-2`'s "an
+I/O error" is read as that boundary. Widening the branch to `isIoError` would retry all four; only
+`EndOfStreamError` was ever a candidate, and `io/` is the wrong layer to decide whether a stream ended early
+because the wire broke. Six cases in `packages/core/src/retry/classify.test.ts` pin one answer per class,
+each asserting both what `isIoError` says and what the classifier says, so the disagreement is recorded as a
+decision rather than an oversight — and re-parenting any leaf under `IoError` turns four of them red.
+
+> **Anchor correction 2026-09-04 (audit #67 / #68), rule supplied 2026-09-05 (audit #67 / #78).** The line
+> numbers in this section were stale on 2026-09-04 and were re-derived then; `classify.ts`'s branch moved
+> from `:73` to `:90` on 2026-09-05 when the paragraph above it was written. The substantive point this
+> section used to make — "the cause-walk returns retryable for any `IoError`" — read as covering all five
+> classes `isIoError` names, and it never did. #68 recorded the gap and left the answer to #78; #78 chose to
+> keep `instanceof IoError` and to say what it means, which is the two paragraphs above. Nothing in the
+> deviation itself (the three-level branch, and why it stays) ever turned on that answer.
 
 ## Deviations recorded outside a phase
 

packages/core/src/io/index.ts

@@ -13,10 +13,20 @@
 // U9 pass promoted `EndOfStreamError` — it was the subject of four `@throws` tags on public symbols
 // with no class a caller could catch. `1f48926` finished the set: `isIoError`,
 // `AllocationLimitError`, `ClosedResourceError` and `SourceContractViolationError` are exported as
-// well, so every error symbol re-exported below is also on `packages/core/src/index.ts:39-47` and in
+// well, so every error symbol re-exported below is also on `packages/core/src/index.ts:39-48` and in
 // `packages/core/etc/core.api.md`. Nothing in this file's error block is internal any more
 // (docs/work/mvp/2026-09-04-open-items-dissolution.md H8, whose remaining sub-item was the category
 // catch `isIoError` now provides).
+//
+// "Load-bearing on it" is narrower than it sounds, and the difference is a decision rather than an
+// accident. The cause-walk at `../retry/classify.ts:90` tests `instanceof IoError`, so it matches
+// `IoError` and `TransportFailureError` — and NOT the four leaves below, which extend `DexpaceError`
+// directly and are grouped only by `isIoError`. That branch means "the wire failed": a send that
+// produced no response is retryable (RETRY-4, TRANSPORT-20), while a violated source contract, a
+// closed resource, an allocation cap and a short exact-length copy are this package's own failures
+// and repeat identically on the next attempt. Audit #67 / #78 decided it; `docs/deviations.md`
+// item 17 carries the rationale and `../retry/classify.test.ts` pins one answer per class. Do not
+// re-parent a leaf under `IoError` to tidy the tree — that silently makes it retryable.
 export {BufferedSink} from './buffered-sink.js';
 export {BufferedSource} from './buffered-source.js';
 export {ByteQueue, copyBytes} from './byte-queue.js';

packages/core/src/retry/backoff.test.ts

@@ -2,7 +2,8 @@
 // packages/core/src/retry/backoff.test.ts
 // Exercises: RETRY-9 (initialDelay * multiplier^(attempt-1), 1-indexed, capped), RETRY-10 (symmetric
 // jitter bounds, midpoint, j=0 identity, negative floors to zero), RETRY-11 (attempt < 1 rejected,
-// overflow saturates), RETRY-43 (fixed delay disables backoff AND jitter).
+// overflow saturates -- INCLUDING at a zero initial delay, where `0 * Infinity` used to give NaN;
+// audit #67 / #78), RETRY-43 (fixed delay disables backoff AND jitter).
 import {describe, expect, test} from 'bun:test';
 import fc from 'fast-check';
 import {computeDelay, type BackoffSettings} from './backoff.js';
@@ -41,6 +42,69 @@ describe('exponential schedule', () => {
   });
 });
 
+/**
+ * RETRY-11's "saturating rather than throwing" has one hole, and it is the zero base. Every other
+ * accepted setting overflows into `Math.min`'s cap; `0 * Infinity` overflows into `NaN`, which
+ * `Math.min` propagates. Audit #67 / #78.
+ */
+describe('a zero initial delay (RETRY-11)', () => {
+  test('stays zero where the power overflows', () => {
+    // Downstream, NaN is worse than a large number: `overshootsBudget` reads false for it, the
+    // engine's `delayMs <= 0` guard reads false for it, and it lands in `Clock.sleep` as a
+    // RangeError that replaces the failure being retried. `retrySettings()` accepts both settings
+    // below (`initialDelayMs >= 0`, finite `multiplier >= 1`), so RETRY-11 covers them.
+    const hugeMultiplier: BackoffSettings = {
+      ...SETTINGS,
+      initialDelayMs: 0,
+      multiplier: 1e200,
+    };
+    expect(computeDelay(3, hugeMultiplier, never)).toBe(0);
+
+    const manyAttempts: BackoffSettings = {...SETTINGS, initialDelayMs: 0};
+    // 2 ** 1099 is Infinity: the first attempt at which the doubling schedule overflows a double.
+    expect(computeDelay(1100, manyAttempts, never)).toBe(0);
+  });
+
+  test('stays zero under jitter too (RETRY-10)', () => {
+    const jitteredZero: BackoffSettings = {
+      ...SETTINGS,
+      initialDelayMs: 0,
+      multiplier: 1e200,
+      jitter: 1,
+    };
+    expect(computeDelay(4, jitteredZero, () => 0)).toBe(0);
+    expect(computeDelay(4, jitteredZero, () => 1)).toBe(0);
+  });
+
+  test('property: every accepted schedule is finite and non-negative (RETRY-11)', () => {
+    // The ranges are exactly what `retrySettings()` admits, so a passing property means no
+    // configuration a caller can build reaches the engine as a non-finite delay. `initialDelayMs`
+    // is drawn through an explicit `constant(0)` arm: the failing region needs a zero base AND an
+    // overflowing power together, and 100 runs of an unbiased double never produced the pair.
+    const accepted = fc.record({
+      initialDelayMs: fc.oneof(
+        fc.constant(0),
+        fc.double({min: 0, max: 1e9, noNaN: true}),
+      ),
+      multiplier: fc.double({min: 1, max: 1e300, noNaN: true}),
+      maxDelayMs: fc.double({min: 0, max: 1e9, noNaN: true}),
+      jitter: fc.double({min: 0, max: 1, noNaN: true}),
+    });
+
+    fc.assert(
+      fc.property(
+        fc.integer({min: 1, max: 5000}),
+        accepted,
+        (attempt, settings) => {
+          const delay = computeDelay(attempt, settings, never);
+          expect(Number.isFinite(delay)).toBe(true);
+          expect(delay).toBeGreaterThanOrEqual(0);
+        },
+      ),
+    );
+  });
+});
+
 describe('symmetric jitter', () => {
   const jittered: BackoffSettings = {...SETTINGS, jitter: 0.2};
 

packages/core/src/retry/backoff.ts

@@ -49,6 +49,17 @@ function applyJitter(
  * Overflow-safe by construction (RETRY-11): a large attempt makes `**` return `Infinity`, which
  * `Math.min` absorbs into the cap. It saturates; it never throws.
  *
+ * Except at a zero base, where the saturation does not hold and the guard below is what supplies it.
+ * `0 * Infinity` is `NaN`, and `Math.min` propagates `NaN` rather than clamping it -- so
+ * `initialDelayMs: 0` with any multiplier above 1 produced a `NaN` delay at the attempt where the
+ * power overflows (`multiplier: 2` reaches it at attempt 1100; `multiplier: 1e200` at attempt 3).
+ * `retrySettings()` accepts both configurations. Downstream, `NaN` is worse than a large number: it
+ * fails every comparison, so the engine's budget check, its overshoot check and its `delayMs <= 0`
+ * short-circuit all read false and it arrives at `Clock.sleep`, which rejects with a `RangeError`
+ * that replaces the failure being retried. Short-circuiting the zero base before the power is taken
+ * is exact rather than a repair: the schedule's value there is `0` at every attempt, and jitter
+ * around `0` is `0` for any sample (audit #67 / #78).
+ *
  * `random` is injected so jitter is assertable rather than statistical -- the same determinism seam
  * CFG-15 wants for the clock.
  *
@@ -71,6 +82,8 @@ export function computeDelay(
     `retry attempt must be 1-indexed and >= 1, got ${String(attempt)}`,
   );
   if (settings.fixedDelayMs !== undefined) return settings.fixedDelayMs;
+  // Before the power, not after: `0 * Infinity` is the one product `Math.min` cannot absorb.
+  if (settings.initialDelayMs === 0) return 0;
   const growth = settings.initialDelayMs * settings.multiplier ** (attempt - 1);
   return applyJitter(
     Math.min(growth, settings.maxDelayMs),

packages/core/src/retry/classify.test.ts

@@ -1,10 +1,12 @@
 // SPDX-License-Identifier: MIT
 // packages/core/src/retry/classify.test.ts
 // Exercises: RETRY-1 (single-sourced status set, 501/505 excluded), RETRY-2 (iterative
-// identity-tracking cause walk, cycle-safe), RETRY-3 (retryability derived from status, not a stored
-// flag), RETRY-4 (transport failures always retryable), RETRY-5/6/7 (re-sendability), RETRY-8 (both
-// axes required), RETRY-23/24 (cancellation vs timeout), RETRY-25 (allow-list makes the fatal
-// exclusion vacuous), RETRY-37 (configured set is authoritative -- widens AND narrows),
+// identity-tracking cause walk, cycle-safe; and the I/O boundary the walk tests -- one case per
+// error class in `io/errors.ts`, see the block below), RETRY-3 (retryability derived from status,
+// not a stored flag), RETRY-4 (transport failures always retryable), RETRY-5/6/7 (re-sendability),
+// RETRY-8 (both axes required), RETRY-23/24 (cancellation vs timeout), RETRY-25 (allow-list makes
+// the fatal exclusion vacuous), RETRY-37 (configured set is authoritative -- widens AND narrows),
+// TRANSPORT-20 (a no-response send surfaces as a retryable I/O subtype),
 // XCUT-5 (the baked retryability flag comes from ONE shared status classifier covering 408/429/all
 // 5xx except 501 and 505 -- asserted below. This port has no separately-cached boolean field: the
 // classifier is a pure function of HttpStatusError.status, which never changes post-construction
@@ -16,7 +18,15 @@ import {stringBody} from '../body/simple-bodies.js';
 import {streamBody} from '../body/stream-body.js';
 import type {Body} from '../body/body.js';
 import {Request} from '../http/request.js';
-import {IoError} from '../io/errors.js';
+import {
+  AllocationLimitError,
+  ClosedResourceError,
+  EndOfStreamError,
+  IoError,
+  isIoError,
+  SourceContractViolationError,
+  TransportFailureError,
+} from '../io/errors.js';
 import {CancellationError} from '../seams/transport.js';
 import {
   RETRYABLE_STATUSES,
@@ -119,6 +129,75 @@ describe('isRetryableFailure', () => {
   });
 });
 
+/**
+ * One case per class in `io/errors.ts`, pinning the boundary RETRY-2's "an I/O error" is read as.
+ *
+ * `isIoError` accepts all six classes the file declares; `classify.ts`'s walk tests
+ * `instanceof IoError`, which two of them satisfy. That gap was undecided until audit #67 / #78
+ * decided it (`docs/deviations.md` item 17): the branch means "the wire failed", so `IoError` and
+ * `TransportFailureError` retry and the four flat leaves do not. Each leaf case asserts BOTH halves
+ * -- that `isIoError` accepts the value, and what the classifier answers for it -- because the two
+ * disagreeing is the decision, and a test that only asserted the classifier would read as an
+ * oversight rather than a choice.
+ *
+ * These are the guard on re-parenting: moving any leaf back under `IoError`, or switching the branch
+ * to `isIoError`, turns four of them red instead of quietly making a deterministic failure retryable.
+ * Measured 2026-09-05 by making that one-line change: exactly these four fail.
+ */
+describe('the I/O boundary the cause-walk tests (RETRY-2/RETRY-4, TRANSPORT-20)', () => {
+  test('IoError itself is retryable', () => {
+    const error = new IoError('connection refused');
+    expect(isIoError(error)).toBe(true);
+    expect(isRetryableFailure(error, RETRYABLE_STATUSES)).toBe(true);
+  });
+
+  test('TransportFailureError is retryable (TRANSPORT-20)', () => {
+    // The one class TRANSPORT-20 requires to BE an IoError. A send that produced no response is the
+    // canonical retryable condition (RETRY-4), and the `extends` is what carries it here.
+    const error = new TransportFailureError('ECONNREFUSED');
+    expect(error).toBeInstanceOf(IoError);
+    expect(isRetryableFailure(error, RETRYABLE_STATUSES)).toBe(true);
+  });
+
+  test('EndOfStreamError is NOT retryable, buried in a cause chain either', () => {
+    // The exact-length-copy contract inside io/, not a wire truncation: a short copy repeats on the
+    // next attempt. A truncated response is the transport's to report, as TransportFailureError.
+    const error = new EndOfStreamError(3, 8);
+    expect(isIoError(error)).toBe(true);
+    expect(isRetryableFailure(error, RETRYABLE_STATUSES)).toBe(false);
+    // Asserted through a wrapper too: the walk is what would rescue it if the branch widened, so the
+    // shallow case alone would not catch a change made one hop up.
+    expect(
+      isRetryableFailure(
+        new Error('read failed', {cause: error}),
+        RETRYABLE_STATUSES,
+      ),
+    ).toBe(false);
+  });
+
+  test('SourceContractViolationError is NOT retryable', () => {
+    // A foreign source that returned zero bytes for a positive read (IO-17) is a programming error
+    // in the source, deterministic on re-send.
+    const error = new SourceContractViolationError('source returned 0 bytes');
+    expect(isIoError(error)).toBe(true);
+    expect(isRetryableFailure(error, RETRYABLE_STATUSES)).toBe(false);
+  });
+
+  test('ClosedResourceError is NOT retryable', () => {
+    // Using a closed resource (IO-42) is a caller lifecycle error; the resource stays closed.
+    const error = new ClosedResourceError('response body');
+    expect(isIoError(error)).toBe(true);
+    expect(isRetryableFailure(error, RETRYABLE_STATUSES)).toBe(false);
+  });
+
+  test('AllocationLimitError is NOT retryable', () => {
+    // A cap the same request hits again (IO-9); retrying spends the budget to fail identically.
+    const error = new AllocationLimitError(2 ** 32, 2 ** 31 - 1);
+    expect(isIoError(error)).toBe(true);
+    expect(isRetryableFailure(error, RETRYABLE_STATUSES)).toBe(false);
+  });
+});
+
 describe('isRetryableFailure -- cancellation, timeouts, and the allow-list', () => {
   test('a user abort is never retryable (RETRY-23)', () => {
     const controller = new AbortController();

packages/core/src/retry/classify.ts

@@ -53,6 +53,22 @@ function causeOf(value: unknown): unknown {
  * surfaces as an `IoError` subclass and is therefore retryable unconditionally at this level
  * (RETRY-4).
  *
+ * **The I/O branch tests `instanceof IoError`, not `isIoError`, and the difference is the rule.**
+ * `io/errors.ts` groups six classes under `isIoError`, but only two of them descend from `IoError`:
+ * `IoError` itself, and `TransportFailureError` -- the class TRANSPORT-20 requires a send that
+ * produced no response to surface, and the reason that `extends` is a requirement rather than a
+ * modelling choice (`docs/deviations.md` item 17). Those two mean "the wire failed", and RETRY-2's
+ * "an I/O error" is read as exactly that boundary. The other four -- `EndOfStreamError`,
+ * `SourceContractViolationError`, `ClosedResourceError`, `AllocationLimitError` -- extend
+ * `DexpaceError` directly and are deliberately outside it: they are this package's own contract and
+ * lifecycle failures and are deterministic on re-send. A closed resource or a violated source
+ * contract is a caller programming error, an allocation cap is a limit the same request hits again,
+ * and `EndOfStreamError` is the exact-length-copy contract inside `io/` -- a *wire* truncation is the
+ * transport's to report, as a `TransportFailureError`. Widening this branch to `isIoError` would
+ * retry all four. Decided by audit #67 / #78; one case per class in `classify.test.ts` pins the
+ * answer, so a later re-parenting of any leaf under `IoError` changes a test rather than passing
+ * silently.
+ *
  * @param error - whatever was thrown; any value, not necessarily an `Error`.
  * @param statuses - the CONFIGURED set, authoritative on its own -- it both widens and narrows
  *   relative to `RETRYABLE_STATUSES`, and the built-in classifier is not AND-ed in (RETRY-37).
@@ -70,6 +86,7 @@ export function isRetryableFailure(
     seen.add(current);
     // RETRY-3: derived from the carried status at classification time, never a stored per-subclass flag.
     if (current instanceof HttpStatusError) return statuses.has(current.status);
+    // Deliberately `instanceof IoError`, never `isIoError` -- see the boundary paragraph above.
     if (current instanceof IoError) return true;
     if (isTimeoutAbort(current)) return true;
     current = causeOf(current);