feat(download): @stitchapi/download batch downloader + core cause-carry + download rig (M1–M5) - #451
Open
rejifald wants to merge 8 commits into
Open
feat(download): @stitchapi/download batch downloader + core cause-carry + download rig (M1–M5)#451rejifald wants to merge 8 commits into
rejifald wants to merge 8 commits into
Conversation
This was referenced Jul 7, 2026
rejifald
force-pushed
the
claude/download-consolidate
branch
from
August 4, 2026 21:14
adecae9 to
f9a56b5
Compare
The `download` surface's `interpret` accepted any sub-400 status as a complete Blob, so a stray `206 Partial Content` (a range-serving proxy/CDN answering a request that never sent a Range) was handed back as if it were the whole file. `interpret` now accepts only a complete body (200/204) and rejects everything else. Adds a controllable download test rig (extending the zero-dep node:http mock server) that reproduces real-world server/network behavior and pins the download surface against it: - byte accounting: stray-206 rejection; Content-Length-mismatch truncation (undici hangs on a clean FIN, cured by `timeout`) - network faults: mid-body RST, idle stall, slow TTFB, steady throttle - redirects: cross-origin credential strip proven end-to-end through download() (the signed-URL -> CDN pattern), same-origin no-over-strip, redirect-loop -> interpret-reject, retry x redirect No engine change; the rig is test-only. Two findings recorded in the specs for the future @stitchapi/download package: the engine has no idle/forward-progress timeout (a healthy-but-slow download dies like a stall), and the transport `.cause` (UND_ERR_SOCKET) is stripped before callers see it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ost-pooling
Milestone 4 of the download test rig — pin the substrate the future
@stitchapi/download batch downloader stands on: per-request concurrency,
host-pooled budgets, partial-failure isolation, multi-host fairness, and
stall isolation. All test-only; no src/ change.
- MockServer concurrency probe: maxOpen/openNow/arrivals(path?) — the count of
requests SIMULTANEOUSLY open on the wire (high-water mark), marked at handler
entry and released on res 'close', so a test asserts ACTUAL wire overlap.
- 5 download-concurrency-*.spec.ts, all asserting overlap COUNT (a hard limiter
invariant), never a timing gap:
ceiling — <= k requests open at once under a shared host throttle
pool — pool:'host' shares one budget vs pool:'stitch' independent
partial-failure — allSettled: 200 + 404 + RST each settle on their own
multihost — two hosts = two budgets; a saturated host can't stall the other
stall-isolation — one stalled item times out alone; siblings' bytes intact
- Root-cause + fix the flaky throttle-host-pooling spec: it measured a
server-arrival wall-clock gap (conflated the 500ms rate spacing with first-fetch
cold-start / event-loop lag). Rewritten on mockAdapter + a shared manualClock,
proving pooling by virtual-time gating — deterministic, zero wall-clock.
Deferred to the unbuilt batch orchestrator (each noted in a // Deferred: comment):
FIFO admission order, cancel one/all, aggregate progress/ETA, same-URL dedupe.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ing, size, range (M5)
Completes the download test rig beyond M4: the connection-level faults, status/retry
semantics, content-encoding, size boundaries, and Range scaffolding a downloader must
survive. All test-only; no src/ change.
Infra:
- test/support/hostile-net.ts — raw net.Server escape hatch (unusedPort, startRawServer)
for faults node:http normalizes away (ECONNREFUSED, immediate FIN, accept-then-silence,
broken TLS handshake).
- mock-server.ts — contentEncoding (gzip/br via node:zlib), etag/lastModified/acceptRanges
validators, forceStatusOnRange (416 / 200-ignore-Range). Plus a fix: reset() now destroys
ONLY sockets with an in-flight response, leaving idle keep-alive sockets alive — destroying
them made undici reuse a dead connection on the next test's first request (the N12
stale-keep-alive "fetch failed"), a latent flake across every multi-test spec.
Specs (real-timer, loose bounds; manualClock for the retry-after delta):
- download-conn-{dns,refused,hangup,no-response,tls} — N7–N11: DNS surfaces generically with
no host leak; ECONNREFUSED is prompt; a FIN / no-response is cut by the timeout; a TLS
handshake failure rejects cleanly.
- download-retry-{transient,after,policy} — X9–X11: a 503 is retried and the WHOLE file
re-downloaded (no Range — buffered surface has no resume); a 429 Retry-After waits the exact
delta (manualClock); 500 and 408 are NOT retried by default, opt-in via retry.on restores it.
- download-{content-encoding,size-boundaries,html-error-page,range-416} — B7/B8 gzip+br
"progress lies" (loaded > compressed total, body intact); Z1–Z3 + 204 boundaries; Z4 a 200
HTML error page returns as a Blob (caller must check); R4 a 416 is rejected.
Deferred (noted per spec) to the future @stitchapi/download package / resume feature:
client-side Range/If-Range resume, plus the M4 batch-orchestrator properties.
Verified: check:types 0, full core suite 1248 green, download+throttle 5x green
(28 files / 35 tests), build+size unchanged (24.58/19.80 — subpath-only), eslint+prettier clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…sees (#450) When a transport call fails (undici `TypeError: fetch failed` whose `.cause` is a `SocketError { code: 'UND_ERR_SOCKET' }`, a DNS/connect failure, an AbortError, …), the engine reduced it to its top-level MESSAGE on the error event and rebuilt a `StitchError('fetch failed', …)` with NO `.cause` — so a caller (both the awaited and `.safe()` paths) saw only the generic message and could not tell a socket reset from any other transport failure (`err.cause === undefined`). Surfaced by the download rig's download-reset-midbody spec. Fix, entirely on the existing non-enumerable ERROR_SOURCE channel: - engine.ts `errEvt`: pin the live error for a bare transport/internal error too (not just RateLimitError / HTTP `.response` failures). A thrown `StitchError` stays excluded, so it keeps being rebuilt from the event (unchanged behaviour). - stitch.ts `rebuildError`: re-surface RateLimitError / contract-violation StitchError UNCHANGED (as before), and otherwise build a StitchError carrying the pinned transport error as `cause`. `cause` is non-enumerable, so it never leaks into a trace sink — only the enumerable `status`/`message` do. This is what lets @stitchapi/download's classifier read `err.cause`/`.code` to distinguish an RST from a generic "fetch failed". download-reset-midbody.spec.ts turns from pinning the gap to pinning the fix: it now asserts `err.cause` is defined and `UND_ERR_SOCKET` is reachable through the cause chain, on both the throwing and `.safe()` paths. Verified: core typecheck 0; full core suite 1248/1248 green; bundle 24.61/19.84 within the 24.80/20.00 budget. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
… ETA, idle-timeout, dedupe) (#449) * feat(download): @stitchapi/download — batch downloader (FIFO, cancel, ETA, idle-timeout, dedupe) A new zero-runtime-dep, browser-first workspace package that sequences a LIST of downloads on top of core's buffered `download()` surface — the niceties a single `download()` call can't express, which the download rig (PRs #447/#448) pinned as DEFERRED. `downloadAll(items, opts)` (one-shot, awaitable + controllable) and the stateful `DownloadManager` it's built on. Its own FIFO concurrency scheduler over `download()` adds, and proves against the rig: - FIFO admission order — queued items start in enqueue order as slots free (P6/P14). - Per-item settling — the batch promise NEVER rejects; one 404/RST never fails a sibling (P4). - Cancellation — cancel-one (slot returns to the queue), cancel-queued (frees no slot, no skip), cancel-all (in-flight abort, queue drains, pool:'host' left clean) (P10-P13). - Aggregate progress + ETA across N concurrent streams under a known byte schedule, driven by the injected Clock (manualClock) so the math is deterministic; a stalled sibling never distorts it (P8/P9/P17). - Same-URL dedupe — independent fetches by default, opt-in `dedupe:true` collapses onto one in-flight request (P18). Solves the two findings the rig surfaced, in the package (core untouched): - Idle / forward-progress timeout that resets on each onProgress chunk — a slow-but-alive stream survives while a dead stall is cut (retryable IDLE_TIMEOUT). The engine's wall-clock timeout cannot make this distinction. - Error classification — captures the RAW transport error via a per-item hooks.onError seam (the engine drops the transport .cause before the caller sees it) and reports retryable + a machine code (UND_ERR_SOCKET / HTTP_404 / IDLE_TIMEOUT). Proven by 7 test/gaps/download-batch-*.spec.ts (15 tests) that re-export the core rig (mock-server + hostile-net) and mirror the existing download-* patterns exactly: assert on-wire overlap via the server's maxOpen probe, manualClock for ETA math, real timers with loose bounds for socket tests. Zero runtime deps; own bundle-size gate (2.46 KB gzip / 2.65 KB budget); core bundle unchanged at 24.58/19.80. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(download): register @stitchapi/download in the README package tables (fix drift) The `drift-executable` CI job (yakir `readme-packages` + `core-integrations` tethers) flagged the new package: `gen-readme-metrics.mjs` auto-discovers every non-private workspace package, so adding `@stitchapi/download` changed the generator's emit while the committed README regions stayed stale — a two-site conflict. Register the package (a group + a curated one-liner, per the generator's own convention) and regenerate: - scripts/gen-readme-metrics.mjs: add `download` to GROUP_BY_DIR ('surface', alongside shell) and TABLE_DESCRIPTIONS. - README.md + packages/core/README.md: regenerated regions now list @stitchapi/download under "Surfaces". Verified: `yakir check --tier executable` AND `--tier token` both exit 0 (5 ok / 3 ok, 0 conflict); the 3 changed files are prettier-clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
… (unblocks lint)
The repo's `no-restricted-syntax` rule forbids `...(x !== undefined ? { k: x } : {})`
in favour of `compact({ ...obj, k: x })` — my cause-carry change tripped it at
stitch.ts, failing core `check:lint` and blocking the `verify` job.
- core rebuildError: `compact({ status, attempts, cause })`.
- @stitchapi/download classify.ts: `compact({ retryable, code })` (compact is re-exported
from `stitchapi`) — same pattern, kept consistent even though the package isn't linted.
Behavior-identical (compact drops undefined keys). Verified: core check:lint 0,
check:types 0, full core suite 1248/1248, @stitchapi/download 15/15, prettier clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@rc install The consolidation PR's `verify` job surfaced two ratchets #447 never reached (it died earlier at check:lint, now fixed): - check:contract (R2/P17): duration fields must not carry an `Ms` suffix — ms is the house unit (cf. core's `waited`/`elapsed`). Renamed `BatchProgress.etaMs` → `eta` and `DownloadIdleTimeoutError.idleMs` → `idle` (+ usages, README, spec). - check:release: every publishable package ships a LICENSE, and an rc package's README install command must target `@rc`. Added packages/download/LICENSE (Apache-2.0) and changed the install line to `@stitchapi/download@rc stitchapi@rc`. Verified locally: check:contract 0, check:release 0, package check:types 0, 15/15 tests, prettier clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`claude/download-consolidate` was cut 2026-07-06 with a 2026-07-23 merge base. Main has since landed the pre-GA breaking sweep, so replaying the seven commits was only half the work — the other half is what the ratchets then demanded. Core - `download`'s `interpret` composes `verdictOf(res, cfg)` (ADR 0022 Decision 4) and yields `data`, not `value`. The M1 complete-body rule sits AFTER the composed verdict, as the surface's own, and now reads `verdict.accept`: `classifyStatus` consults it only at `>= 400`, so without that read a caller who declared `206` NORMAL would still be rejected — the surface overriding an explicit declaration rather than ruling where none was made. Second test in download-stray-206.spec.ts pins it. - `rebuildError`'s cause-carry lands on top of main's body/url arm instead of replacing it. Test rig + specs - `throttle-host-pooling` keeps the deterministic manual-clock rewrite (the wall-clock flake fix) minus its `scope`-alias half — #490 removed the alias. ADR 0023 Decision 1 ratified exactly the 500ms in-process spacing it asserts. - Rig duration fields drop the `Ms` suffix (D3/P17), matching main's own `stream.chunkDelay`: `ttfbDelayMs` → `ttfbDelay`, `chunkDelayMs` → `chunkDelay`. The rig keeps main's `retryAfterSeconds` (the header's native unit) over the branch's `retryAfter`; four existing specs already depend on it. - Spec config: auth moved to `stitchapi/auth` (ADR 0021) and `apiKey` takes `secret` (#580); `retry.baseMs`/`baseDelay` fold into `retry.backoff.base` (#513); `respectRetryAfter` → `retry.respect` (#608); `AdapterProgress.phase` → `direction`. @stitchapi/download - `DownloadBatch.cancel`/`cancelAll` fold into `cancel(id?)` — R8 landed after this branch was cut and flags the shared prefix (P24). For a verb the optional parameter IS the collapse; an envelope would be the same pair one level deeper. - `hooks` is built unannotated so `onError` infers REQUIRED: the slot is `AtLeastOne<Hooks>` now (P20, #507), which an all-optional annotation cannot satisfy. - Version + vitest to the rc.7 lockstep; lockfile regenerated from main's. - classify.ts and the README no longer claim the engine drops the transport cause — this branch's own core fix carries it through. The hook seam stays: it is strictly wider (it also catches a thrown non-`Error`). Verify: 40/40 workspace projects green (core 1460 passed, download 15/15); check:lint / types / types-d / exports / exports:companions / contract / unknown-keys / release / docs-links / format all clean; core size 23.45 gzip (budget 23.50) and 20.87 for `import { stitch }` (20.90), @stitchapi/download 2.45/2.65 KB; `yakir check` both tiers 0 drift. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
rejifald
force-pushed
the
claude/download-consolidate
branch
from
August 4, 2026 21:19
f9a56b5 to
fa92962
Compare
rejifald
added a commit
that referenced
this pull request
Aug 5, 2026
…ipt (#646) `pnpm -r check:lint` fans out only to packages that DEFINE a `check:lint` script. Exactly one did — core — so ESLint had never run against the other 35 `packages/*`, including the repo's own `no-restricted-syntax` ratchet. A banned `...(x !== undefined ? { k: x } : {})` spread in a companion passed every local gate (#451); the identical pattern in core failed, purely because core happened to own the script. `check:lint` is now `node scripts/check-lint.mjs`, which enumerates `packages/*` on DISK and lints each one's `src/` and `test/`. Coverage no longer depends on a package opting in, so a new package is gated the moment it exists. Verified by probe: re-introducing the #451 spread in `@stitchapi/redis` fails the gate, and passes again when reverted. Rules move from `packages/core/eslint.config.ts` to a root `eslint.config.ts`, unchanged. Core's `eslint-suppressions.json` regenerates byte-for-byte identical under the new setup, which is the check that the move preserved its semantics exactly. One eslint process per package, not one whole-workspace pass. The rules are type-aware, so a single pass has to hold all 36 TypeScript programs in one heap: that dies with SIGABRT at ~4.7 GB RSS under Node's default heap — with a tsconfig glob AND with `projectService` — which would have made the gate unrunnable in CI. Sharded, it peaks at 1.3 GB and finishes in ~10s (faster than the single pass, since the shards run four-up). Per-package cwd also anchors `parserOptions.project` and the `eslint-suppressions.json` lookup, which is why core's baseline keeps working untouched. The pass surfaced 554 violations. 211 are fixed here: - The `no-restricted-syntax` spread in `@stitchapi/vercel-ai` — the #451 class. A pre-existing comment already explained that `compact` is wrong here (`parameters`/`inputSchema` are required `unknown` keys it would optionalize), so it takes the inline disable core's config prescribes for that case and already uses at engine.ts:1380. - `require-yield` in rtk-query's never-ending stream fake; `no-prototype-builtins` in sandbox-sim (→ `Object.hasOwn`); a dead `no-eval` directive in openapi; an unused import and an unused `TextEncoder` in sandbox-sim. - ~200 mechanical fixes from a `--fix` pass restricted to fixers that cannot change behaviour (`array-type`, `consistent-type-definitions`, `consistent-type-imports`, `no-unnecessary-type-arguments`/`-assertion`/ `-conversion`, `prefer-optional-chain`/`-includes`/`-regexp-exec`), plus the same optional-chain fix hand-applied to seven copies of one test helper. `--fix-type` excludes `directive`, because a plain `--fix` deletes `eslint-disable` comments it considers unused and ate ten of them — core's included — on the first attempt. The `--fix` also rewrote elysia's `StitchContext` from a `type` to an `interface`, breaking the constraint the comment directly above it warns about (an interface gets no implicit index signature, so it fails Elysia's `SingletonBase`). Reverted, with the disable that comment always implied. `pnpm check:types` catches it — elysia is green on main and was red with the rewrite. The remaining 343 are baselined per package with ESLint's bulk-suppressions ratchet — the mechanism core already used, and which the history shows being burned down rather than grown. They are dominated by work that needs real per-package decisions, not lint edits: 120 `no-unsafe-member-access` from the fingerprint packages walking untyped third-party schema internals, and 41 `only-throw-error` whose fix would change what those packages throw, i.e. public behaviour. New violations fail; these do not. Two rule options rather than suppressions, both because the alternative was editing code that is already correct: - `no-unused-vars` gets `^_` ignore patterns. Every package compiles under `noUnusedLocals`/`noUnusedParameters`, which TypeScript itself exempts `_`-prefixed bindings from — so 54 of the 55 hits were the compiler's own convention, and the two gates disagreed about the same files. - `vitest/expect-expect` learns `assertConformance`, the shared store-contract runner, so those suites stop reading as assertion-free. Also here because the gate needs them: `sandbox-sim/tsconfig.test.json` gets `"exclude": []` — `exclude` is inherited through `extends`, so the base config's `**/*.test.ts` carried over and the project resolved to ZERO test files, making it dead config that nothing ran. It now covers its 9 tests and typechecks clean. Plain-JS packages (`completions-plugin`, .mjs with no tsconfig) drop to the syntactic rule set instead of being excluded, since the type-aware rules throw rather than skip when there is no program. CI and the lefthook hooks already call `pnpm check:lint`, so nothing in verify.yml or lefthook.yml changes. Closes #457 Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Owner
Author
|
Not touching this branch — #639 is the rebase of this same work, so it's the one carrying the fix. Context, since it applies to both: The duplication was a symptom rather than the bug. The two classes were siblings, so every dispatch site had to name both — and |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The whole download workstream, consolidated onto
mainin one clean, lint-passing branch.Why this PR exists
The stack
#447 (M1–M3) ← #448 (M4–M5) ← {#449 package, #450 core-fix}merged between branches, but the squash-merge order stranded the package: #448 carrieddownload-m4-batchup to#447's branch at 20:53, but #449 (the@stitchapi/downloadpackage) only landed ondownload-m4-batchat 21:09 — after it had been carried up. So #447 contains the rig + core-fix but not the package, and nothing reachedmain. #447 was also failingverifyon ano-restricted-syntaxlint error introduced by the core cause-fix.claude/download-consolidate=download-m4-batch(which does contain everything, as clean linear commits onmain) + the lint fix. It supersedes the #447/#448 stack.What's included
#447+#448): the zero-depnode:httpmock server +hostile-net, the M1interpret206-rejection fix, and 27test/gaps/download-*specs.@stitchapi/download(#449): the batch downloader —downloadAll+DownloadManager, FIFO admission, per-item settling, cancel (one/queued/all), aggregate progress+ETA, idle-timeout, dedupe. Zero runtime deps; own size gate (2.46 KB / 2.65 budget). Plus its README registration (drift fix).fix(core): carry the transport cause (#450):err.cause(undiciUND_ERR_SOCKET…) now reaches awaited &.safe()callers, on the non-enumerableERROR_SOURCEchannel....(x !== undefined ? {k} : {})conditional-spread withcompact({…})inrebuildError(+ the package's classifier), which is whatno-restricted-syntaxrequires — the fix that unblocksverify.Verification (local)
check:lint0 (the blocker),check:types0, full core suite 1248/1248.@stitchapi/download:check:types0, 15/15 tests, build + size 2.46/2.65 KB.yakir check(drift): both tiers pass (the README/integrations regions list@stitchapi/download).Housekeeping
Supersedes #447 (and the merged #448/#449/#450) — close #447 once this lands. Deferred by design (not in this PR): client-side HTTP Range / byte-offset resume.
🤖 Generated with Claude Code