docs(review 003): WS data-channel wiring gap drill-down

Focused review of the OQ-05 deferral (review-001 WS-03): what is
missing to wire browser/native data channels over WS, the deferral
rationale check, and the design gaps found behind the 'nothing new
to design' assumption.

Findings (continuing review-002 numbering):
- WS-20..22 [major]: mechanical wiring gap — install_channel_zero
  discards the ChannelManager, generic channel lifecycle ops never
  registered, no openable-ALPN deployment surface
- WS-23 [minor]: no browser-opened-channel e2e test; OQ-05 reopen
  trigger reframed (native WS consumers are not blocked on OQ-04)
- 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 (needs an alkcall decision)
- WS-25 [major]: no wire mechanism for client-side op registration —
  ADR-048 browser-side bidirectionality is undesignable as specified
- WS-26 [major]: no retained live connection handle; hub cannot
  reach a session's overlay or push toward it
- WS-27 [minor]: dead-code-gated test hook cleanup

Includes a non-findings section (channel-id split, demux hardening,
open-op wrapper completeness, idle-knob posture) and a 4-unit
remediation plan sequenced so the alkcall decision task (Unit 1)
gates the alkhttp wiring shape (Unit 2).

Verification: cargo test (453) / --all-features (575), clippy
(all-targets, both feature sets), fmt — all clean at 58665f2.
This commit is contained in:
2026-09-03 08:31:44 +00:00
parent 58665f2061
commit 19e30c9769
@@ -0,0 +1,467 @@
# 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<CallConnection>` 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.
## Unit 1 — alkcall decision task (WS-24, WS-25)
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.
- WS-24: pick (a) per-session base registry (amend ADR-047 §4's
mechanism wording; add the `OperationRegistry` fork/clone surface)
or (b) overlay-aware top-level dispatch. Gate: an alkcall e2e test
where an open op registered per the *amended* mechanism resolves
via `run_loop_single_stream` dispatch over a channels connection.
- WS-25: decide the client-registration story (new envelope kind /
channel-0 bootstrap op / explicit scope cut). Gate: the ADR (or
scope-cut note) exists and OQ-05/ADR-048 point at it.
## 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<CallConnection>` 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/<alpn>/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.