fix(core): reject partial (206) download responses + a download test rig

packages/core/src/download.ts

@@ -84,6 +84,25 @@ export const downloadSurface: Surface<StitchInput, DownloadResult> = {
         responseType: 'blob',
     }),
     interpret: (res): SurfaceOutcome<DownloadResult> => {
+        // A buffered download resolves to a WHOLE Blob, so only a complete-body status is a success:
+        // `200 OK`, or `204 No Content` (a legitimately empty body). The engine's attempt loop only
+        // throws on `>= 400` (engine.ts), so every other sub-400 status — most importantly `206
+        // Partial Content` — flows here. `download` never sends a `Range`, so a `206` is a partial
+        // body the server volunteered (a range-serving proxy/CDN, a resumed-and-mismatched cache); if
+        // it were accepted it would hand the caller a truncated file as if it were the whole thing.
+        // Reject anything that is not a complete 2xx.
+        if (res.status !== 200 && res.status !== 204)
+            return {
+                ok: false,
+                message: `download: expected a complete body (200/204) but got HTTP ${res.status}${res.status === 206 ? ' — a partial (206) response was not requested (no Range header is ever sent)' : ''}`,
+                status: res.status,
+            };
+        // No `blob.size` vs `content-length` cross-check here: a truncated body (Content-Length
+        // advertises more bytes than are sent) never reaches this hook. undici HANGS on a clean-FIN
+        // short read until the caller's `timeout` fires, and REJECTS on an abrupt socket close — so a
+        // truncation surfaces as a timeout / transport error, never as a short Blob delivered here. A
+        // length check would be dead code (and a gzip/br body legitimately has `size != content-length`
+        // once decoded — it would be a false positive). See test/gaps/download-truncation.spec.ts.
         const value: DownloadResult = { blob: res.body as Blob };
         const filename =
             filenameFromDisposition(res.headers['content-disposition']) ??

packages/core/src/engine.ts

@@ -368,6 +368,17 @@ function errEvt(err: unknown, name: string, attempts: number): StitchEvent {
             value: err,
             enumerable: false,
         });
+    } else if (err instanceof Error && !(err instanceof StitchError)) {
+        // Bare transport / internal error (undici UND_ERR_SOCKET / ECONNRESET / DNS, an AbortError, …):
+        // pin the live instance on the SAME non-enumerable channel so the awaited / `.safe()` path can
+        // carry its `.cause` through as `StitchError.cause` — letting a caller tell a socket reset from
+        // a generic "fetch failed" (previously only the top-level message survived). Non-enumerable ⇒
+        // it never serialises into a trace sink (only the enumerable `status`/`message` do). A thrown
+        // `StitchError` is excluded so it keeps being rebuilt from the event (unchanged behaviour).
+        Object.defineProperty(evt, ERROR_SOURCE, {
+            value: err,
+            enumerable: false,
+        });
     }
     return evt;
 }

packages/core/src/stitch.ts

@@ -353,13 +353,20 @@ function rebuildError(ev: Extract<StitchEvent, { type: 'error' }>): Error {
             cause: source,
         });
     }
-    return (
-        source ??
-        new StitchError(ev.message, {
-            status: ev.status,
-            attempts: ev.attempts,
-        })
-    );
+    // A pass-through terminal — a delegate-backoff RateLimitError or a contract-violation StitchError
+    // (`.inspect()` retain path) — is re-surfaced UNCHANGED.
+    if (source instanceof StitchError || source instanceof RateLimitError)
+        return source;
+    // Otherwise a StitchError from the event, carrying any bare transport/internal `source` the engine
+    // pinned (an undici UND_ERR_SOCKET / ECONNRESET, a DNS failure, an AbortError, …) as `cause` — so
+    // callers can read `err.cause` (and its `.code`) to tell a socket reset from a generic "fetch
+    // failed". Absent a source, no cause (unchanged). `cause` is non-enumerable, so it never leaks into
+    // a trace sink.
+    return new StitchError(ev.message, {
+        status: ev.status,
+        attempts: ev.attempts,
+        ...(source !== undefined ? { cause: source } : {}),
+    });
 }
 
 // Build the `Inspection` wrapper (ADR 0016 / ADR 0019): every field enumerable EXCEPT `raw`, which is

packages/core/test/gaps/download-concurrency-ceiling.spec.ts

@@ -0,0 +1,86 @@
+// Pins P1 (download-test-rig-spec §2.8): under a shared host throttle, no more than `k` download
+// requests are OPEN ON THE WIRE at any instant. Asserted from the mock server's own concurrency probe
+// (`maxOpen`) — ACTUAL on-the-wire overlap, not "we decided to be concurrent". This is the ceiling the
+// future @stitchapi/download batch layer stands on: it sequences a caller's LIST, but the per-request
+// admission cap is core `throttle`, and it must genuinely bound the wire.
+//
+// N INDEPENDENT download() stitches (a batch of distinct files) share ONE host-keyed concurrency
+// budget via `throttle: { concurrency: k, pool: 'host' }` — P2, the cross-instance pooling substrate
+// (resilience.ts `hostStates`, keyed by URL host). Each route holds its response with `ttfbDelayMs` so
+// the k admitted slots stay open together long enough to observe the peak.
+//
+// Real-timer, LOOSE bounds (a socket test): the hold (150ms) is wide vs. loopback dispatch jitter
+// (two near-simultaneous fetches land sub-ms apart), so the k concurrent slots reliably overlap; no
+// exact timing is asserted, only a COUNT. Held sockets are force-destroyed at teardown (the server
+// tracks live sockets), so the suite exits.
+//
+// Deferred (needs the unbuilt @stitchapi/download batch orchestrator, NOT raw download()+throttle):
+//   • FIFO admission ORDER — that queued items (k+1)…N start in enqueue order as slots free (P6). The
+//     limiter serves concurrency waiters FIFO, but PROVING per-item order needs the batch API's stable
+//     item identity + start events, which don't exist yet.
+//   • Aggregate progress / ETA across the N concurrent streams (P8) — a batch-level roll-up, not a
+//     property of a single download().
+// Here we pin only the wire CEILING + host pooling those features are built on.
+import { download } from '../../src/download';
+import { startMockServer } from '../support/mock-server';
+import type { MockServer } from '../support/mock-server';
+
+let server: MockServer;
+beforeAll(async () => {
+    server = await startMockServer();
+});
+afterAll(async () => {
+    await server.close();
+});
+beforeEach(() => {
+    server.reset();
+});
+
+const blobText = async (b: Blob): Promise<string> =>
+    new TextDecoder().decode(await b.arrayBuffer());
+
+test('≤ k download requests are open on the wire at once under a shared host throttle', async () => {
+    const K = 2;
+    const N = 6;
+    const paths = Array.from({ length: N }, (_, i) => `/file-${i}`);
+    for (const p of paths)
+        server.route('GET', p, {
+            statuses: [200],
+            rawBody: `contents-of${p}`,
+            ttfbDelayMs: 150, // hold each admitted slot open long enough to observe the overlap
+        });
+
+    // N INDEPENDENT download stitches (distinct files), all sharing one host-keyed budget of K.
+    const calls = paths.map((p) =>
+        download({
+            baseUrl: server.url,
+            path: p,
+            throttle: { concurrency: K, pool: 'host' },
+            retry: { attempts: 1 },
+        }),
+    );
+
+    // `Promise.all` subscribes to every cold StitchResult in the same tick → all N are launched
+    // concurrently; the throttle, not the test, decides how many actually reach the wire.
+    const results = await Promise.all(calls.map((c) => c()));
+
+    // THE CEILING: the server never saw more than K requests open on the wire at any instant.
+    expect(server.maxOpen()).toBeLessThanOrEqual(K);
+    // …and the budget was genuinely SATURATED (real concurrency, not accidental serialization) — else
+    // "≤ K" would pass vacuously at 1. With N > K and a 150ms hold, the peak must reach K.
+    expect(server.maxOpen()).toBe(K);
+    // Every file still downloaded correctly and completely — the cap PACES work, it never drops it.
+    expect(results).toHaveLength(N);
+    for (let i = 0; i < N; i++)
+        expect(await blobText(results[i]!.blob)).toBe(
+            `contents-of${paths[i]!}`,
+        );
+    // Sanity: all N requests actually reached the server (nothing was silently dropped by the gate).
+    expect(server.callCount()).toBe(N);
+    // The probe also records a per-request arrival timestamp, in receipt order (the observable the
+    // future batch layer's ETA/progress math will read); one per request, monotonically non-decreasing.
+    const ts = server.arrivals();
+    expect(ts).toHaveLength(N);
+    for (let i = 1; i < ts.length; i++)
+        expect(ts[i]!).toBeGreaterThanOrEqual(ts[i - 1]!);
+});

packages/core/test/gaps/download-concurrency-multihost.spec.ts

@@ -0,0 +1,106 @@
+// Pins P7 (download-test-rig-spec §2.8): multi-host fairness — a SATURATED host must not stall an
+// INDEPENDENT host's schedule. `pool:'host'` keys the budget by URL host, so two mock servers on
+// different ephemeral ports are two distinct hosts with two independent budgets (the same mechanism
+// download-redirect-cross-origin.spec.ts uses for a cross-origin pair). A batch that spans hosts must
+// let each host make progress at its own pace; host A being pinned at its cap can't starve host B.
+//
+// Setup: host A is deliberately throttled to concurrency 1 and given M slow items → it drains SERIALLY.
+// Host B has concurrency 2 and a couple of items → it runs them concurrently. All are fired together.
+// The oracles are COUNTS + ordering (robust), not a wall-clock gap:
+//   • B finishes while A is still draining  → A.callCount() < M at the moment B is done (B was not
+//     stuck behind A's saturated queue).
+//   • B genuinely ran its own concurrency    → B.maxOpen() reaches 2.
+//   • A really was capped at 1               → A.maxOpen() === 1.
+// Real-timer, LOOSE bounds: A's serial drain (M×150ms) dwarfs B's (~150ms), a wide margin; held
+// sockets are destroyed at teardown on BOTH servers.
+//
+// Deferred (batch-orchestrator scope): aggregate cross-host progress/ETA (P8) and priority/fair
+// SCHEDULING across hosts (P21, which core `throttle` has no knob for) — this pins only that the two
+// hosts' budgets are independent, the substrate any cross-host scheduler builds on.
+import { download } from '../../src/download';
+import { startMockServer } from '../support/mock-server';
+import type { MockServer } from '../support/mock-server';
+
+let a: MockServer; // the saturated host (concurrency 1)
+let b: MockServer; // the independent host that must not be stalled
+beforeAll(async () => {
+    a = await startMockServer();
+    b = await startMockServer();
+});
+afterAll(async () => {
+    await Promise.all([a.close(), b.close()]);
+});
+beforeEach(() => {
+    a.reset();
+    b.reset();
+});
+
+const blobText = async (blob: Blob): Promise<string> =>
+    new TextDecoder().decode(await blob.arrayBuffer());
+
+test('a saturated host does not stall an independent host’s schedule (per-host budgets)', async () => {
+    const M = 4; // host A items — drained one at a time (concurrency 1)
+    const aPaths = Array.from({ length: M }, (_, i) => `/a-${i}`);
+    const bPaths = ['/b-0', '/b-1'];
+
+    // Distinct ephemeral ports ⇒ two real, independent hosts under pool:'host'.
+    expect(new URL(a.url).port).not.toBe(new URL(b.url).port);
+
+    for (const p of aPaths)
+        a.route('GET', p, {
+            statuses: [200],
+            rawBody: `A${p}`,
+            ttfbDelayMs: 150, // each A item holds its single slot ~150ms → serial ~600ms total
+        });
+    for (const p of bPaths)
+        b.route('GET', p, {
+            statuses: [200],
+            rawBody: `B${p}`,
+            ttfbDelayMs: 50, // B is fast — it finishes well before A drains, a wide margin
+        });
+
+    const aRuns = aPaths.map((p) =>
+        download({
+            baseUrl: a.url,
+            path: p,
+            throttle: { concurrency: 1, pool: 'host' }, // A: saturated, serial
+            retry: { attempts: 1 },
+        })(),
+    );
+    const bRuns = bPaths.map((p) =>
+        download({
+            baseUrl: b.url,
+            path: p,
+            throttle: { concurrency: 2, pool: 'host' }, // B: its own budget, unaffected by A
+            retry: { attempts: 1 },
+        })(),
+    );
+
+    // StitchResults are COLD — they start on subscribe, not on creation. Launch A NOW (concurrently
+    // with B) by subscribing via `allSettled`, and keep the promise to await after we've measured —
+    // otherwise A would not even start until we awaited it, and the probe would read an idle host A.
+    const aDone = Promise.allSettled(aRuns);
+
+    // B completes on its OWN schedule — it does not wait behind A's saturated queue. (Reaching here
+    // means neither B call rejected — `Promise.all` would have thrown otherwise.)
+    const bResults = await Promise.all(bRuns);
+    expect(bResults).toHaveLength(2);
+    expect(await blobText(bResults[0]!.blob)).toBe(`B${bPaths[0]!}`);
+    expect(await blobText(bResults[1]!.blob)).toBe(`B${bPaths[1]!}`);
+
+    // At the instant B is done, A is STILL draining — proof B was never stalled behind A.
+    expect(a.callCount()).toBeLessThan(M);
+    // B genuinely ran its two downloads concurrently (its budget, not throttled down by A).
+    expect(b.maxOpen()).toBe(2);
+    // …and A really was pinned at its cap of 1 the whole time (the saturation was real).
+    expect(a.maxOpen()).toBe(1);
+
+    // Let A finish so teardown is clean; every A item still downloads correctly once its turn comes.
+    const aSettled = await aDone;
+    expect(aSettled.map((s) => s.status)).toEqual(Array(M).fill('fulfilled'));
+    for (let i = 0; i < M; i++) {
+        const r = aSettled[i] as PromiseFulfilledResult<{ blob: Blob }>;
+        expect(await blobText(r.value.blob)).toBe(`A${aPaths[i]!}`);
+    }
+    expect(a.maxOpen()).toBe(1); // never exceeded its cap across the whole drain
+});

packages/core/test/gaps/download-concurrency-partial-failure.spec.ts

@@ -0,0 +1,108 @@
+// Pins P4 (download-test-rig-spec §2.8): across a batch of INDEPENDENT downloads sharing a throttle,
+// each call SETTLES ON ITS OWN — one bad URL (a 404, or a mid-body RST) never fails the others, and a
+// successful item's bytes are complete and uncorrupted despite failing siblings sharing the budget.
+//
+// This is the isolation property the future @stitchapi/download batch API MUST preserve: a naive
+// `Promise.all` over the calls would reject the WHOLE batch on the first failure; a batch downloader
+// has to be per-item (the `Promise.allSettled` shape modelled here). We pin it at the raw
+// download()+throttle layer so the batch API inherits an already-proven substrate — the two failure
+// MODES compose with concurrency, not just in isolation.
+//
+// Deferred (an orchestrator CONTRACT, not a property of raw download()): same-URL DEDUPE (P18) —
+// whether enqueuing the same URL twice reuses one in-flight fetch or runs two independent ones.
+// StitchAPI's only coalescing is cache-gated (and download blobs are typically uncacheable), so a
+// dedupe map is NEW batch-layer code; every call here is independent by construction. The batch API
+// picks the contract (recommended default: independent fetches) and a future spec pins whichever.
+import { download } from '../../src/download';
+import type { DownloadResult } from '../../src/download';
+import { startMockServer } from '../support/mock-server';
+import type { MockServer } from '../support/mock-server';
+
+let server: MockServer;
+beforeAll(async () => {
+    server = await startMockServer();
+});
+afterAll(async () => {
+    await server.close();
+});
+beforeEach(() => {
+    server.reset();
+});
+
+const blobText = async (b: Blob): Promise<string> =>
+    new TextDecoder().decode(await b.arrayBuffer());
+
+// A mixed batch: good / 404 / good / RST / good — failures interleaved with successes, all sharing one
+// host-keyed budget of 2. `ok` items hold briefly so the batch genuinely overlaps failures & successes
+// under the gate (isolation must survive real concurrency, not just a serial run).
+const ITEMS = [
+    { path: '/ok-0', kind: 'ok' as const },
+    { path: '/bad-404', kind: 'notfound' as const },
+    { path: '/ok-1', kind: 'ok' as const },
+    { path: '/bad-rst', kind: 'reset' as const },
+    { path: '/ok-2', kind: 'ok' as const },
+];
+
+test('a partial-failure batch settles per-item — one bad URL never fails the others (404 + RST isolated)', async () => {
+    for (const it of ITEMS) {
+        if (it.kind === 'ok')
+            server.route('GET', it.path, {
+                statuses: [200],
+                rawBody: `ok-body${it.path}`,
+                ttfbDelayMs: 60,
+            });
+        else if (it.kind === 'notfound')
+            server.route('GET', it.path, {
+                statuses: [404],
+                body: { error: 'not_found' },
+            });
+        else
+            server.route('GET', it.path, {
+                statuses: [200],
+                rawBody: 'ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789', // 36 bytes advertised…
+                declaredLength: 36,
+                resetAfterBytes: 6, // …only 6 written, then a real ECONNRESET mid-body
+            });
+    }
+
+    const calls = ITEMS.map((it) =>
+        download({
+            baseUrl: server.url,
+            path: it.path,
+            throttle: { concurrency: 2, pool: 'host' },
+            retry: { attempts: 1 }, // failures terminal — no backoff to wait on
+        }),
+    );
+
+    // Per-item settling — the batch shape a downloader must use (NOT `Promise.all`, which would
+    // collapse on the first reject).
+    const settled = await Promise.allSettled(calls.map((c) => c()));
+
+    // Each item settled strictly on its OWN outcome; a sibling's failure never leaked across.
+    expect(settled.map((s) => s.status)).toEqual([
+        'fulfilled', // ok-0
+        'rejected', // bad-404
+        'fulfilled', // ok-1
+        'rejected', // bad-rst
+        'fulfilled', // ok-2
+    ]);
+
+    // The successful items carry their COMPLETE, correct bytes — not truncated or cross-contaminated
+    // by the failing siblings that shared the throttle budget.
+    for (const i of [0, 2, 4]) {
+        const r = settled[i] as PromiseFulfilledResult<DownloadResult>;
+        expect(await blobText(r.value.blob)).toBe(`ok-body${ITEMS[i]!.path}`);
+    }
+
+    // The failures are Errors, each its OWN distinct terminal shape — a 404 (engine throws `HTTP 404`
+    // before interpret) and a transport RST (undici "fetch failed") — never a resolved partial Blob.
+    const err404 = (settled[1] as PromiseRejectedResult).reason as Error;
+    const errRst = (settled[3] as PromiseRejectedResult).reason as Error;
+    expect(err404).toBeInstanceOf(Error);
+    expect(err404.message).toMatch(/404/);
+    expect(errRst).toBeInstanceOf(Error);
+    expect(errRst.message).toMatch(/fetch failed/i);
+
+    // All five requests reached the server — the gate paced them, it didn't drop the failing ones.
+    expect(server.callCount()).toBe(ITEMS.length);
+});