# Review 003 — WebSocket data-channel wiring gap (the OQ-05 drill-down) ## Status Verified, open for remediation. ## Scope Focused review of the deferred half of the WS path: the data-channel wiring recorded as OQ-05 (review-001 WS-03, deferred 2026-08-29), plus the bidirectionality half of ADR-048 it leans on. The trigger for this pass was a session-level drill-down into [websocket.md](../architecture/websocket.md) §"Data channels for browsers" and the OQ-05 deferral: what exactly is missing, whether the deferral rationale still holds, and whether the "nothing new to design" assumption recorded in OQ-05 is actually true. The pass covered: - the WS accept-side wiring end to end (`src/websocket/upgrade.rs`, the alkcall `ChannelsAdapter` accept path it drives), - the alkcall channel machinery the deferred wiring is expected to reuse (`ChannelCore::register_openable`, `ChannelOperations`, `ChannelManager` open/adopt/ledger, the demux loop), including alkcall's own end-to-end test of the exact wiring shape, - the dispatch path that would resolve an open op on channel 0 (`Dispatcher::dispatch` / `run_loop_single_stream`, `compose_root_env`, the connection overlay), - the deployment surface (`HttpAdapter` builders, `SessionState`, `RouterState`) an openable-ALPN declaration would ride, - the spec record (ADR-047 §4 amendment, ADR-067 v1-cut note, ADR-048 reconciliation note, OQ-05, alkcall ADR-047 §6 discovery split), and - the consumer framing (OQ-05's "reopened when a browser consumer lands" trigger) against the clarified product position: WS is the browser path **and** the native-client fallback behind hostile NAT/firewall — the consumer set is wider than browsers, and the wasm-capable protocol stack means a browser client needs only binary-frame parsing and byte reassembly before handing bytes to alkcall-wasm. Cross-crate findings (alkcall-side) are flagged as such. Findings continue review-002's numbering (WS-20..). ## Baseline verification (this pass) ``` cargo test → 453 passed, 0 failed cargo test --all-features → 575 passed, 0 failed cargo clippy --all-targets -- -D warnings → clean cargo clippy --all-features --all-targets -- -D warnings → clean cargo fmt --check → clean ``` Verified against tree `58665f2`. No source changes were made in this pass; the findings are analysis-verified against source (file:line below) rather than introduced by new code. ## Verdict - **OQ-05's core assumption is now known to be wrong, and that is the headline.** OQ-05 records the deferral as *"the design is decided (ADR-067: the browser opens data channels exactly as any channels consumer — nothing new to design); what is deferred is the wiring."* That held for the alkcall-side machinery — which is genuinely complete and well tested — but two wiring-blockers turned out to be **design gaps, not mechanical work**: - **WS-24 (cross-crate):** the ADR-047 §4 amendment's registration mechanism ("`register_openable` is called on the connection's overlay registry, Layer 2") cannot resolve an open op on the wire — the top-level dispatch consults only the dispatcher's base registry and never the connection overlay. The only shape proven to work (alkcall's own e2e test) registers open ops on a per-connection registry installed **as the dispatcher's base registry** — a shape the ADR does not describe and that has no fork/clone surface on `OperationRegistry` to support. - **WS-25:** there is no wire mechanism for a WS client to register an op at all — the call protocol's envelope kinds are `call.requested`/`responded`/`completed`/`aborted`/`published`/ `error` only. ADR-048's browser-side bidirectionality ("the hub can call browser-registered ops") is not designable until alkcall defines client-side op registration (or amends the promise). Every overlay test to date pokes `register_imported` in-process. - **The mechanical half of the gap is real but small** (WS-20/21/22): the accept-side hook discards the `ChannelManager` alkcall hands it, the generic channel lifecycle ops are never registered, and no deployment surface exists to declare openable ALPNs. The alkcall machinery these would consume (`ChannelCore`, `ChannelOperations`, opener ledger, demux, odd/even id split, cap policy) is built, tested, and ready — alkcall even ships an end-to-end test of the exact wiring over duplex. - **The deferral trigger no longer matches the consumer set.** OQ-05 says "reopened when a browser consumer lands," but WS is also the native fallback behind ugly NAT/firewall; a Rust WS consumer (exactly the `test_support` client shape) is a legitimate first consumer and needs no JS library — OQ-04 does not block this wiring (WS-23). - No regressions and no new robustness holes were found in the channel-0 path itself; the WS-13/17/18-era pump work is sound and the idle-knob/data-channel interplay (silent-but-alive SSH-style channels) is already documented with a deployment posture. ## Severity legend - **[critical]** — the protocol cannot function end-to-end as committed; or a decided spec invariant is violated in a way that corrupts data or causes silent permanent state damage. - **[major]** — a decided behavior is missing, wrong, or a real reliability hazard; works in the happy path but fails a spec-required edge case, or is a realistic security weakness. - **[minor]** — drift, convention violation, dead code, or a doc/spec inconsistency with no correctness impact. --- # Part A — The deferred wiring (mechanical) ## WS-20 [major] — `install_channel_zero` discards the `ChannelManager`; no `ChannelCore`, no open ops, ever **Residual of:** review-001 WS-03 / OQ-05 (the v1 cut itself). **Verified:** YES. The hook alkhttp supplies to `ChannelsAdapter::new` ignores the manager: `Arc::new(move |_manager, channel0_conn, auth| ...)` — `src/websocket/upgrade.rs:255`. alkcall's accept path passes the per-connection `ChannelManager` precisely so the hook can build a `ChannelCore` and register openable ALPNs on it (`alkcall/src/channels/adapter.rs:268` — `(self.install_channel_zero)(manager.clone(), channel0_conn, auth.clone())`; the `InstallChannelZero` doc at `adapter.rs:50-54` says the manager is reachable via the hook "e.g., to register channel lifecycle ops' `ChannelCore`"). The reference wiring exists in alkcall's e2e test (`channels/client.rs:780-837`): the hook builds `ChannelCore::new(manager, policy)`, registers a per-ALPN open op via `register_openable`, and dispatches channel 0 with it. In alkhttp the manager is dropped on the floor, so a browser/native WS client can never open a data channel — the capability ADR-067 says the channels design exists to provide, and the only part of the WS path the HTTP gateway cannot already do. **Fix (Unit 2):** rework the hook to take the manager, build the per-connection `ChannelCore`, register the generic ops (WS-21) and the deployment's openable ALPNs (WS-22), then run the dispatcher over the per-session registry — shape per the WS-24 decision. The policy is already threaded (`ChannelsPolicy` extension, review-001 SRV-10's fix); the openables ride the same path. ## WS-21 [major] — Generic channel lifecycle ops are never registered: no `channel/close`, `channel/control`, `channel/resources/subscribe` on WS sessions **Verified:** YES. alkcall's `ChannelOperations::register_on` (`alkcall/src/channels/operations.rs:42-98`) registers the three generic ops — close (with drain-before-close + opener-ledger decrement), control (loud `not_implemented` per alkcall OQ-39), and resources/subscribe (loud `not_implemented` per alkcall OQ-40). Grep-verified: nothing in alkhttp calls it, and the per-session hook (WS-20) never registers them. Consequence: even if open ops were wired today, a WS client could not close a channel (`channel/close` → `NOT_FOUND`) or observe resources — the open ledger would only drain via connection teardown. This is also the discovery seam for the dynamic half of alkcall ADR-047 §6's discovery split. **Fix:** fold into the WS-20 hook rework — `register_on` on the per-session registry next to the openables. Note the `not_implemented` ops are correctly loud today; registering them is still right (fail loudly at the op, not `NOT_FOUND` at discovery). ## WS-22 [major] — No deployment surface to declare openable ALPNs for the WS path **Verified:** YES. The `HttpAdapter` builder surface is `with_decoy` / `with_extra_routes` / `with_ws_max_sessions` / `with_ws_idle_timeout` (`src/server/adapter.rs:158-233`) — nothing expresses "these ALPNs are openable on WS sessions." `SessionState` carries registry/sessions/slots/idle only (`upgrade.rs:89-97`), and `RouterState` likewise. The deployment decision ADR-067 requires ("the deployment decides which ALPNs are openable — the browser reaches exactly what a Rust consumer on an in-line connection would reach") has no expression point. Note the distinction from the existing `ChannelsPolicy` extension: that is the *cap* policy (`ChannelLifecyclePolicy::check_open`); the *openable set* (per-ALPN `OperationSpec` with the `channel_open` marker + `OpenHandler`) is a separate, currently nonexistent input. **Fix (Unit 2):** an `OpenableAlpn { spec, open_handler }` list + `HttpAdapter::with_ws_openable_alpns(...)`, threaded `RouterState` → `SessionState` → `run_channels_session` → hook, with a request-extension fallback mirroring `ChannelsPolicy` so bare-registry/custom upgrade routes can pass their own set. The ALPN-specific `OpenHandler` implementations stay in the ALPN crates (alktty et al.); alkhttp only ferries the registrations. ## WS-23 [minor] — No browser-opened-channel e2e test; OQ-05's reopen trigger underestimates the consumer set **Verified:** YES. The test suite covers channel-0 dispatch (`tests/ws_*`), overlay mechanics (in-process pokes, `tests/ws_overlay_ops.rs`), and pump robustness — but no test opens a data channel through a live WS session (the OQ-05 acceptance item "browser-opened-channel end-to-end test" exists precisely because none does). Everything needed already ships behind `test-support`: `WsClient`, `frame_channel0_chunk`, `ChunkAssembler`, `FrameAssembler` (`src/websocket/upgrade.rs:444-709`) — a Rust WS client speaking raw channels framing, which is also exactly the shape of the native-client-behind-a-firewall consumer. alkcall has the analogous e2e test over duplex (`channels/client.rs:767-870`) — the alkhttp one is the missing gate. **Also record here (product framing, no code change):** OQ-05's status — `deferred(scope: v1 cut — no browser consumer yet; reopened when one lands)` — frames the consumer as a browser needing the JS/TS library (OQ-04). The clarified position: WS is the bidirectional path for browsers **and** the fallback for native clients behind hostile NAT/firewall/proxy; a native WS consumer is feasible today with zero new library work (Rust client, same framing as any in-line transport), and a browser consumer is mostly byte parsing + reassembly ahead of wasm-targeted alkcall (WebTransport remains out of scope per ADR-069). The wiring is therefore not blocked on OQ-04, and the reopen trigger should be reframed accordingly when OQ-05 is picked up. **Fix:** the Unit-3 e2e test (open → `{channel_id}` → chunks both ways → handler sees bytes → `channel/close` → cap denial → teardown decrements), plus the OQ-05 reframe when the task lands. --- # Part B — Design/spec mismatches (planning bugs; need decisions before wiring) ## WS-24 [major, cross-crate] — Top-level dispatch never consults the connection overlay; the ADR-047 §4 amendment mechanism cannot resolve open ops on the wire **Verified:** YES. The amendment (`alkcall/docs/architecture/decisions/047-openable-alpns-are-operations.md`, "Amendment (§4 per-connection registration, 2026-08-13)") decides: *"open ops are registered per-connection … `register_openable` is called on the connection overlay registry (Layer 2 per ADR-019), closing over a per-connection `ChannelCore`."* But the dispatcher's top-level resolution never looks at that layer: `Dispatcher::dispatch` reads the op's type from `self.registry.registration(...)` (`dispatch.rs:316-320`) and invokes via `self.registry.invoke/invoke_streaming/invoke_sink` (`dispatch.rs:330-341`) — the base registry only. The connection overlay (`CallConnection::overlay_env()`, `connection.rs:174`; `OverlayOperationEnv`) is reachable solely as a layer of `context.env` via `compose_root_env` (`dispatch.rs:195-215`) — i.e., for *nested* invocation by handlers, not for resolving the incoming `call.requested` itself. An open op registered per the amendment's mechanism would resolve `NOT_FOUND` on the wire. The only shape proven to work is alkcall's own e2e test (`channels/client.rs:767-870`): the hook builds a **fresh per-connection registry containing the open op and passes it as the dispatcher's base registry** (`Dispatcher::new(registry, ...)` at `:837`). That shape works but implies a per-session fork of the deployment's base registry (base ops + generic channel ops + openables) — and `OperationRegistry`/`HandlerRegistration` have no `Clone`/fork surface (registration.rs:68-75, 97-100; the inner payloads `OperationSpec`/`HandlerKind` do derive `Clone`), so the fork needs an alkcall-side addition or a re-registration walk. It also implies per-session re-registration of `services/list` / `services/schema` closed over the fork, or per-session open ops stay undiscoverable (alkcall ADR-047 §6's "what may I open" half). This is the "planning bug": OQ-05 says *nothing new to design*; mechanically the alkcall machinery is done, but the registration *mechanism* the ADR names does not dispatch. Either resolution is fine — the decision just needs to be made deliberately: - **(a)** per-session base registry (the proven shape): alkcall amends ADR-047 §4's mechanism wording and adds a fork/clone surface on `OperationRegistry` (most registration payloads already `Clone`); alkhttp builds the per-session fork in the hook. No dispatch-path change; discovery re-registered per session. - **(b)** overlay-aware top-level dispatch: alkcall adds an overlay fallback in `dispatch`/`run_loop_single_stream` (consult `connection.overlay_env()` when the base registry misses). Larger blast radius (touches the shared dispatch loop); keeps one static registry; makes the ADR-047 amendment's wording true as written. Cross-crate: the decision and (for (a)) the fork surface live in alkcall; alkhttp consumes whichever lands. ## WS-25 [major] — No wire mechanism for a WS client to register an op; ADR-048's browser-side bidirectionality is undesignable as specified **Verified:** YES. The call protocol's envelope kinds are `call.requested` / `call.responded` / `call.completed` / `call.aborted` / `call.published` / `call.error` (`alkcall/src/protocol/wire.rs:12-17`) — there is no registration/announce kind, and grep-verified no wire op by which a client could register a handler spec. Client-side registration onto an overlay (`register_imported` / `register_imported_all`, `connection.rs:162-172`) is reached only in-process — the `from_call`/`from_wss` import path (a Rust consumer importing a remote peer's ops) and tests. Every "browser-registered op" test pokes the overlay directly in-process (`tests/ws_overlay_ops.rs:210-251`; the alknet-http `overlay.rs` prior art is the same shape). So the decided behavior — *the hub can call browser-registered ops over the same session* (websocket.md §"Bidirectionality"; ADR-048's reconciliation note records it as decided design, deferred) — has no wire contract for the browser half: nothing tells the hub the op exists, and the browser cannot run a Rust-side `import()`. This is an alkcall-level protocol design gap (a new envelope kind / handshake, an explicit "not supported for WS-native clients" scope cut, or a channel-0 bootstrap op). It does **not** block the data-channel half (open ops are hub-registered; the browser only calls them), which is why Unit 1 can decide WS-24 first and WS-25 in parallel — but ADR-048's promise should not be treated as implementable by wiring alone. ## WS-26 [major] — The upgrade path retains no live connection handle; the hub cannot reach a session's overlay even once registration exists (and cannot push anything, ever) **Verified:** YES. `run_channels_session` hands channel 0's `Connection` into the hook, which moves it into the spawned dispatcher task (`upgrade.rs:255-287`) — nothing retains a handle the assembly layer can reach. `WsSessions` tracks pump handles only (the WS-08 abort lever). Consequences: ADR-048's "the hub holds that connection as a live handle" and "the hub reaches browser ops through the connection handle, not through `PeerRef::Specific`" (websocket.md §"Browsers are not alk peers") has no object to hold — the deployment cannot enumerate live WS sessions, cannot hub→browser-call an overlay op even after WS-25 defines registration, and cannot push-initiate anything toward a session. The alknet prior art retained the handle (`alknet-http/src/websocket/upgrade.rs:104` — `Arc::new(CallConnection::new_overlay_only(identity))` held by the session state). **Fix:** rides the WS-20 rework — the hook (or the session) retains an `Arc` in a deployment-visible registry (shape decided in Unit 2 alongside the openables surface; eviction hooks into `WsSessions` so handles die with the session). ## WS-27 [minor] — `adapter_install_channel_zero` is compiled unconditionally on `server` under `#[allow(dead_code)]`; its only caller is test-gated **Verified:** YES. `src/websocket/upgrade.rs:290-295` exports the `pub(crate)` wrapper; `mod.rs:36-37` re-exports it under `#[cfg(feature = "server")]`; the sole caller is `from_wss`'s `#[cfg(all(test, feature = "server"))]` test server (`src/adapters/from_wss.rs:352,394`). The `#[allow(dead_code)]` documents the current shape but the cleaner cut is gating the wrapper with the same cfg as its caller — moot after WS-20's rework, which will give both the upgrade path and the test server a real hook; fold the cleanup into that task. --- # Non-findings (verified correct, recorded to bound the re-review) - **Channel-id collision:** accept side allocates even ids starting at 2, connect side odd starting at 1 (`alkcall/src/channels/manager.rs:105-125`); `adopt_channel` is the non-allocating side's install and returns `ChannelExists` on collision (`manager.rs:273-309`). The WS browser (connect side) and hub (accept side) allocation split is safe. - **Demux hardening already landed** (review-001 WS-12's cross-crate fix): `TooLarge` skip with a 64 KiB stack buffer, cumulative 256 MiB skipped-bytes budget, EOF-mid-skip ends the loop (`alkcall/src/channels/adapter.rs:131-222`) — the DoS shape is bounded before browser traffic ever hits it. - **Open-op wrapper completeness:** `register_openable` covers Query/Mutation (`Once`), Sub (`Stream`, one envelope then out-of-band data), Pub (loud `not_implemented` stub pending the channel-adoption path, alkcall C-08) — cap check → allocation → ledger → spawn → teardown decrement all wired (`alkcall/src/channels/operations.rs:399-453, 483-555`). - **Idle-knob vs silent data channels** is already documented with a deployment posture (websocket.md §"Idle-read timeout": progress- based deadline, `with_ws_idle_timeout(None)` posture, no keepalive by decision). Wiring data channels makes the 60 s default bite more often (an idle SSH-style channel is silent-but-alive); the remediation plan should surface that knob in the task's docs note, not change the semantics. - **`NoCap` default on the built-in surface** is review-001 SRV-10's recorded resolution (`ChannelsPolicy` extension exists for deployments; `upgrade.rs:308-317, 391-393`) — unchanged by this review. --- # Remediation plan Sequenced by dependency; Unit 1 is a decision task and gates the shape of Unit 2. Upstream findings are tracked in parallel as alkcall review 004 (`/workspace/@alkdev/alkcall/docs/reviews/004-*.md`), where the decision work actually lives; the alkcall remediation units there carry the acceptance gates for WS-24/WS-25. The plan below is the alkhttp-side consequence of whichever upstream resolution lands. ## Unit 1 — alkcall decision task (WS-24, WS-25) → tracked upstream Decide the open-op dispatch-resolution mechanism and the client-side op-registration question; record as alkcall ADR amendments (ADR-047 §4 mechanism; ADR-048's bidirectionality scope) with alkhttp-side consequences noted. **Full analysis and remediation units live in alkcall review 004**; the alkhttp summary: - WS-24: the leading candidate is **option (a), per-session base registry** — the only shape proven to dispatch on the wire (alkcall's e2e test). It requires the alkcall fork/clone surface (`OperationRegistry` fork; all inner payload types verified `Clone`-able — see review 004 F-03). - WS-25: the leading candidate is the **channel-0 bootstrap op** (`op/register` as an assumed-op the hub serves per-session, whose handler writes into the connection-local overlay via `register_imported`) — no new envelope kind, the wire stays the six `call.*` kinds. It requires the alkcall **client serving half** (F-04): the connect-side read pump currently resolves responses only and silently drops inbound `call.requested`. It composes with the bootstrap-discovery split: `services/list` stays the base-registry listing; peer-registered op discovery rides the already-built `services/list-peers` (which reads `ctx.env.peer_ids()`/`peer_operations()` — populated from the connection overlay by `compose_root_env`). Gate: the alkcall ADRs (or amendments) recorded per review 004, and review 004's Units 1–3 landed; OQ-05/ADR-048 in this repo point at the resolution. ## Unit 2 — alkhttp wiring (WS-20, WS-21, WS-22, WS-26) - `OpenableAlpn` list + `HttpAdapter::with_ws_openable_alpns(...)`, threaded through `RouterState`/`SessionState` with a request-extension fallback (mirroring `ChannelsPolicy`). - Rework `install_channel_zero`: take the manager + policy + openables; build the per-connection `ChannelCore`; register `ChannelOperations` generic ops + the openables (per Unit 1's shape); retain the `Arc` in a deployment-visible, `WsSessions`-evicted handle registry. - `adapter_install_channel_zero` cleanup (WS-27) rides this rework. - Gate: a live WS session lists `channels//sub` in `services/list` (per Unit 1's discovery shape) and an open op on channel 0 returns a `channel_id`. ## Unit 3 — e2e tests (WS-23) Using `test_support::WsClient` (the Rust-native-consumer shape): 1. open → `{channel_id}` → chunks flow both directions on that channel → the `OpenHandler`'s `Connection` sees the bytes. 2. `channel/close` → handler EOF + opener-ledger decrement (`policy` count drops). 3. per-identity cap denial (`channel:too_many_channels`) and per-connection cap (`channel:too_many_channels` from the manager). 4. disconnect mid-open → teardown cascade, no leak. 5. `TooLarge` chunk on a data channel survives (demux resync) — already proven alkcall-side; assert it through the WS path. ## Unit 4 — spec reconciliation (docs) - OQ-05 → resolved (or narrowed to the remaining piece), with the consumer-set reframe recorded (WS-23): browser + native fallback; OQ-04 not blocking. - ADR-067 v1-cut note and ADR-048 reconciliation note updated to the landed state; websocket.md §"Data channels for browsers" status block removed; the alknet-ADR-044 §5 pointer checked for staleness. - websocket.md gains the idle-knob deployment note for silent data channels (already true, just surfaced where a deployer reads). --- ## Verification log (this pass) - All findings carry `file:line` references verified against tree `58665f2`. - The WS-24 dispatch claim was triple-checked: `dispatch()`'s op-type lookup (`dispatch.rs:316-320`) and invoke paths (`:330-341`) consult `self.registry` only; `OverlayOperationEnv` (`connection.rs:731-770`) is reachable only via `compose_root_env` → `context.env` layers (`dispatch.rs:195-215`); alkcall's e2e test passes the per-connection registry **as** the dispatcher's base registry (`channels/client.rs:789-837`), not as an overlay. - The WS-25 claim was grep-verified across alkcall `src/`: the only `register_imported*` callers are in-process import paths (`protocol/adapter.rs:541,610`, `client/from_call.rs:71` doc); the wire kind set is closed over the six `call.*` constants. - The WS-20 reference shape was read in full (`channels/client.rs:767-870`), including the fresh-registry detail that motivates the fork-surface ask. - `adapter_install_channel_zero`'s caller census: `from_wss.rs:394` only (cfg(test) module). - Baseline gates re-run for this pass (see above): cargo test (default + all-features), clippy (both), fmt. No coverage run — no source changed.