test(#630): freeze native Fetch/Response/cancellation semantics (phase 1) - #640
Conversation
…hase 1 Characterization-only unit for issue #630 Phase 1: strengthens the existing ClickHouseTransport contract/implementation unit suites (strict Response identity, exact pre-abort Fetch count, pathological SQL/Authorization matrix, invalid-UTF-8 byte safety, live fetch()/origin() accessors, exact zero/empty/reserved-value URL serialization) and adds a real Chromium/WebKit browser proof (tests/e2e/clickhouse-http-transport.{html,spec.js}) driving the actual createHttpTransport against a real cross-origin fault server for native cancellation-lifetime scenarios (pre-abort, awaiting-header abort, post-header pending-read abort, streamLines() callback shutdown, concurrent A/B isolation, abort-after-completion). fault-server.mjs gains byte-safe request-body capture (Buffer.concat instead of per-chunk string concatenation), an opt-in browser/CORS mode, and a deterministic post-header-abort-hold fixture — all additive, existing callers unaffected. The CI e2e path filter now tracks this one shared fixture file since the new root spec depends on it. No production transport/auth/consumer file is touched — this phase only characterizes and strengthens proof around the existing boundary.
Author-side readiness review (per per-issue-cycle.md step 3) found the opt-in CORS writeHead wrapper added in the previous commit assumed Node's 2-arg writeHead(status, headers) call shape; Node's real signature also allows writeHead(status, statusMessage, headers), which the wrapper would have silently mishandled (spreading a string into object keys) had any fixture used it. No current fixture does, so this was latent, not an active bug — hardened defensively since it's shared test infrastructure. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
ChatGPT review pass 1Reviewed head: Verdict: REVISE Findings
Verified
Verification limitationThis runtime could not resolve |
- Strengthen invariant 5's Authorization matrix with a genuinely nonstandard scheme (XAuth) alongside Bearer/Basic/Digest, so a transport that special-cases a closed allowlist of known schemes can no longer pass a matrix built only from IANA-registered schemes. - Scope fault-server.mjs's ServerResponse error-suppression handler to the opt-in cors:true path only, matching its own docstring that cors defaults off and every pre-existing no-option caller keeps today's behavior. Add getLastErrorListenerCount() introspection and a parity.test.ts regression pair proving the scoping. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LwFPT465eDJqYcRa8HGNLz
ChatGPT review pass 2Previously reviewed head: Reviewed head: Verdict: SHIP Pass-1 finding reassessment
Complete updated-PR regression scan
Verification limitationThis runtime still cannot independently clone the repository or rerun the caller-reported local WebKit/sabotage suite. I reviewed the canonical GitHub full diff and history, the exact old→new delta, current file contents/blob identities, and the exact-head GitHub Actions run. I found no remaining actionable finding or regression in the updated PR. |
What & why
Part of #630 — replacing the rejected
@clickhouse/client-webadoption path (#585) witha first-party, Fetch-native
@altinity/clickhouse-httppackage extracted from genericprotocol code SQL Browser already maintains.
This is Phase 1 of 8: a characterization/test-infrastructure-only unit. It does not
create the package, refactor transport, or touch any production source. It strengthens
the existing transport contract and adds a real-browser (Chromium/WebKit) fault harness
proving the invariants that phases 2+ will freeze behind the new package boundary.
Claims: A1 (native Fetch/Response/cancellation contract characterized in unit and
Chromium/WebKit tests), A3 (low-level request preserves native Response identity, exact
SQL/Auth, raw bytes, one Fetch call, caller-signal lifetime). A2 and A4–A18 remain
deferred to later phases.
Approved plan: see the ship-log comment on #630 for the plan-review conversation link
(ChatGPT-author / Fable-high-approve loop, approved pass 3/5).
Contract coverage
All 14 characterization bullets from the issue's Phase 1 section are covered — see the
invariant map below. Every one of the 9 hard invariants this phase characterizes has an
enforcement mechanism, a test/compile-time proof, and a sabotage case that fails as
expected (verified independently twice — once by the implementer, once by a separate
high-effort pre-PR review — with no sabotage surviving in either pass).
send()returns the fetchResponsedirectlyclickhouse-transport-contract.tsidentity asserts (2xx/non-2xx) + e2e Scenario 1/2 page-realm===.clone()/rewrap → identity asserts failsend()never reads the bodybodyUsed === falsepre-consumption asserts.text()→ asserts faildeps.fetch()(...)call, no retrybody: request.sqlverbatim.trim()/rewrite → exactness failsorigin()/fetch()accessorssend()AbortSignalpassed directly to native fetchsend()never decodes/re-encodes.text()+reconstruct → byte proof failsTests
npm run check:types && npm run check:arch && npm run check:schemas && npm run check:examples && npm test && npm run build— green.npx vitest run tests/unit/clickhouse-http-transport.test.ts --config tests/vitest.config.ts— green.npm run test:client-spike— 116 passed, 19 skipped, 0 failed.npm run test:e2e -- tests/e2e/clickhouse-http-transport.spec.js --project=chromium --project=webkit— 18/18 passed.--project=chromium --project=webkit— 432 passed, 4 skipped (pre-existing, unrelated).test.skip(({browserName}) => browserName === 'firefox', ...)) per repo policy (Firefox can't launch locally; CI supplies it for the full nightly/manual matrix, not this PR's Chromium-only e2e job).Build
npm run buildsucceeds — single-filedist/sql.html(2,126,288 bytes), no new runtime dependency.Invariant/sabotage verification
Independently confirmed twice: once during implementation (all 9 sabotage mutations
caught, then reverted from saved bytes), and once by a separate high-effort read-only
review over the complete branch diff, which additionally re-ran 4 of the sabotage
mutations itself (shared-controller cancellation bug, swallowed read-rejection in
streamLines,chUrlordering swap,Response.clone()) — all caught, none survived.Checklist
npm testpasses (the per-file coverage gate is non-negotiable)npm run buildsucceeds (single-filedist/sql.html)tests/**and one CI path-filter line — zero production source changedCHANGELOG.md([Unreleased]) updated## Phaseschecklist is ticked only after merge is verified onorigin/main, per its explicit stated conventionPart of #630.