docs(#585): revert ADR-0005 to Rejected after Phase 2 cancellation defect - #629
Merged
Merged
Conversation
…fect Phase 2's plan (implementing ClickHouseTransport with @clickhouse/client-web) went through 5 rounds of the /ship plan-review loop, hitting the pass cap without approval. All 23 accepted findings across those rounds traced to one mechanism: the design's abort/cancellation wiring ties the caller's own AbortSignal to a derived controller that's torn down when send() settles at response headers, before the body finishes streaming - silently breaking mid-stream Cancel for progressive queries and exports, a regression from the current transport's baseline of wiring the caller's signal directly into the real fetch for its whole lifetime. Two independent architecture reviews (ChatGPT and a separate Fable/high read-only reviewer, neither aware of the other's answer) verified this against the installed @clickhouse/client-web 1.23.1 sources and reached the same conclusion: the vendor's abort model is structurally incompatible with the Phase 1 transport contract, and after every other required correction (byte-exact SQL/Authorization restoration, a hand-written query-string serializer replacing the vendor's incompatible one), the vendor contributes no bytes or behavior to the actual wire request. This is exactly the revert condition the 2026-08-07 decision-methodology amendment itself named in advance. No implementation commit was ever made; the Phase 2 branch was cut and deleted with zero commits. ADR-0005, the issue's Phases checklist, the ship log, CHANGELOG, and wiki are reconciled to the reverted Rejected decision, with the full evidence trail (both independent reviews' findings) recorded in a new ADR addendum. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LwFPT465eDJqYcRa8HGNLz
29 tasks
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.
Summary
docs/ADR-0005-clickhouse-web-client.md) from Accepted back to Rejected, the same day (2026-08-07) it was amended to Accepted — the 2026-08-07 decision-methodology amendment authorized a Phase 2 implementation attempt, and that attempt's own plan review surfaced a disqualifying defect the amendment itself had pre-named as a revert condition.ClickHouseTransportimplemented with@clickhouse/client-web) went through 5 rounds of the/shipplan-review loop (23 verified findings, all real). Every real finding traced to one mechanism: the design's abort/cancellation wiring never lets the caller's ownAbortSignalreach the vendor's realfetchcall, and no variant of "tear the forwarding down at some point" is simultaneously correct — tearing down atsend()-settlement (response headers) silently breaks mid-streamCancelfor progressive queries/exports (a regression from the current transport's baseline); never tearing down leaks listeners indefinitely.@clickhouse/client-web@1.23.1sources from scratch and reached the same conclusion, additionally finding that once every other required correction is applied (byte-exact SQL, byte-exactAuthorization, a hand-written query-string serializer replacing the vendor's incompatible one), the vendor'sexec()call contributes no bytes or behavior to the actual wire request.feat/585-phase2-official-transport-implwas cut, received zero commits, and was deleted.docs/ADR-0005-clickhouse-web-client.md(new "Phase 2 cancellation-incompatibility addendum", status/decision/consequences/alternatives/phase-boundaries sections updated), issue ADR-0005: adopt @clickhouse/client-web behind the SQL Browser transport adapter #585's## Phaseschecklist and phase-scoping note, the<!-- ship-log -->comment,CHANGELOG.md, and.wiki/Decisions-and-Roadmap.md.docs/evidence/585/**anddocs/evidence/585/decision-table.mdare untouched — Phase 0's original ten-gate computation is unaffected and remains correctlyAcceptedas literal script output; this reversion is driven by an eleventh consideration (cancellation-model ownership) that Phase 0 never measured at all.No
src/**changes. Docs/wiki only.Test plan
npm run check:typesnpm run check:archnpm run check:schemasnpm run check:examplesnpm test(7152 tests, includingtests/unit/client-web-spike-policy.test.js's fail-closed ADR/wiki status-consistency check)npm run build🤖 Generated with Claude Code
https://claude.ai/code/session_01LwFPT465eDJqYcRa8HGNLz