docs(review 004): per-connection dispatch resolution + client-side op serving

Focused design-mismatch review found via alkhttp's WS data-channel
drill-down (alkhttp review 003 WS-24/WS-25). The channel machinery is
done and proven; the gaps are the dispatch-resolution mechanism and
the connect-side serving half — both upstream of any transport.

Findings:
- F-01 [major]: top-level dispatch consults only the dispatcher's
  base registry — ops registered per the ADR-047 §4 amendment's
  overlay mechanism resolve NOT_FOUND on the wire; the only proven
  shape (per-session registry as the dispatcher's base) differs from
  the ADR's wording
- F-02 [major]: no Clone/fork surface on OperationRegistry or
  HandlerRegistration — every inner payload type IS Clone-able
  (verified type-by-type, incl. jsonschema::Validator and
  Capabilities), so the fork is a small addition
- F-03 [minor]: fork must carry handlers + validators, not just specs
- F-04 [major]: connect-side channel-0 read pump resolves responses
  only; inbound call.requested frames are silently dropped — no
  serving half on the single-stream shape (ADR-022/AGENTS §8
  bidirectionality unreachable from the connect side)
- F-05 [major]: no wire mechanism announces client-side ops; the
  six call.* kinds are closed. Resolution candidate: bootstrap op
  (op/register) served per-session, handler writes into the
  connection-local overlay; discovery rides services/list-peers
- F-06 [minor]: per-session fork must carry bootstrap discovery ops
  for per-session openables to be discoverable

Includes a non-findings section (e2e reference shape,
register_openable completeness, channel-id split, envelope-kind
closure, wasm-cleanliness) and a 4-unit plan: ADR decisions (Unit 1)
-> fork surface (Unit 2) -> client serving + bootstrap op (Unit 3)
-> alkhttp wiring downstream (Unit 4, tracked in alkhttp review 003).

Verification: cargo test (565), clippy --all-targets -D warnings,
fmt, doc --no-deps — all clean at c16b069. No source changes.
This commit is contained in:
glm-5.3-flash committed 2026-09-03 14:42:57 +00:00
1 parent c16b0697e3
commit c0dbf82518
1 file changed
+447
@@ -0,0 +1,447 @@
# Review 004 — Per-Connection Dispatch Resolution and Client-Side Op Serving (found via alkhttp's WS data-channel drill-down)
## Status
Verified, open for remediation.
## Scope
Focused review of two design/spec mismatches in the call + channels
integration, uncovered while drilling into the deferred half of
alkhttp's WebSocket path (alkhttp review 003, findings WS-24/WS-25 —
the OQ-05 "browser data channels" deferral). The drill-down
established that the alkhttp-side gap is mostly wiring, but two
assumptions underneath it resolve to *this* crate:
1. **Dispatch resolution for per-connection operations.** ADR-047's
§4 amendment (2026-08-13) decided that openable-ALPN open ops are
registered per-connection on the connection overlay registry
(Layer 2). The top-level dispatch path never consults that layer,
so an op registered per the amendment's mechanism cannot be
invoked over the wire. The only shape proven to dispatch is
different from the one the ADR describes.
2. **Client-side operation serving.** The connection-local overlay
(`CallConnection::register_imported` → `overlay_env()`) and the
session overlay (`compose_root_env` attaching peer-keyed overlays)
are fully built and tested — but only the *accept side* has a loop
that serves inbound `call.requested` frames. The connect side's
read pump resolves responses and silently drops requests. There
is also no wire mechanism by which a client could announce an op
in the first place. Together these leave the "both sides can be
both" symmetry (AGENTS.md §8; ADR-022 §direction semantics;
alkhttp ADR-048's browser bidirectionality) unreachable on the
single-stream channel-0 shape.
The pass re-verified every claim directly in source at tree
`c16b069`. Cross-repo context: these findings block alkhttp review
003's Unit 2 (data-channel wiring for WS sessions, both browser and
native-fallback consumers); the decision work and the fixes live
here. Findings continue alkcall review numbering (F-01..; reviews
001–003 used P/C/R/A/B/C/D prefixes — F is the new prefix for this
focused review, findings numbered independently).
## Baseline verification (this pass)
```
cargo test → 565 passed, 0 failed
cargo clippy --all-targets -- -D warnings → clean
cargo fmt --check → clean
```
The suite is green; these findings are design/spec mismatches, not
regressions. Notably, one of them (F-04's serving gap) is invisible
to the suite *because* the connect side has never been asked to
serve — the existing e2e tests all put the dispatcher on the accept
side.
## Verdict
- **The channel machinery is done and proven.** `ChannelCore` /
`register_openable` / `ChannelOperations`, the opener ledger, the
odd/even id split, the demux hardening, and the full e2e wiring
test (`channels/client.rs:767-870`) are all in place and tested.
Nothing in Part A below re-opens that machinery.
- **The gap is the dispatch-resolution *mechanism* (F-01/F-02/F-03)
and the connect-side serving half (F-04)** — both upstream of any
transport (TCP+TLS, WS, whatever). Fixing them here unblocks
alkhttp's data-channel wiring, and — since the crate is
wasm-targetable and not yet published to consumers — the fixes are
cheap now and required regardless of alkhttp.
- **The recommended resolution is assembly, not invention:** per-
session registry forks (F-02/F-03) + a bootstrap `op/register` op
(F-01's mechanism choice) + a connect-side serving loop (F-04).
Three of the four pieces have working reference shapes in-tree;
the genuinely new protocol surface is one op handler and one read
loop.
## Severity legend
- **[critical]** — a decided spec invariant is violated in a way that
makes a promised capability unreachable end-to-end; or corrupts
data.
- **[major]** — a core protocol path cannot serve a decided behavior;
works only via shapes the spec does not describe.
- **[minor]** — drift, doc/spec inconsistency, or a missing
convenience with no correctness impact.
---
# Part A — Dispatch resolution vs the ADR-047 §4 amendment
## F-01 [major] — Top-level dispatch never consults the connection overlay; ops registered per the ADR-047 §4 amendment mechanism resolve `NOT_FOUND` on the wire
**ADR drift:** ADR-047 "Amendment (§4 per-connection registration,
2026-08-13)": *"open ops are registered per-connection …
`register_openable` is called on the connection overlay registry
(Layer 2 per ADR-019), closing over the per-connection `ChannelCore`."*
**Verified:** YES, three ways:
1. `Dispatcher::dispatch` reads the operation's `op_type` from
`self.registry.registration(...)` only
(`src/protocol/dispatch.rs:316-320`) and invokes via
`self.registry.invoke` / `invoke_streaming` / `resolve_sink_handler`
(`:330-357`) — the dispatcher's **base registry** only. There is
no fallback to `connection.overlay_env()`.
2. The connection overlay is reachable solely as a layer of
`context.env`: `compose_root_env` attaches it via
`env.attach_peer(peer_id, connection.overlay_env())`
(`dispatch.rs:195-215`). `context.env` is consulted for *nested*
invocations (a handler calling `context.env.invoke(...)`) — not
for resolving the incoming `call.requested` itself. An open op
registered on the overlay resolves `NOT_FOUND` when the wire
request arrives.
3. The one place this wiring is proven end-to-end uses a different
shape than the amendment describes: alkcall's own e2e test
(`src/channels/client.rs:767-870`) has the `install_channel_zero`
hook build a **fresh per-connection registry containing the open
op and pass it as the dispatcher's base registry**
(`Dispatcher::new(registry, ...)` at `:837`) — not as an overlay.
That shape works; the amendment's shape cannot.
Consequence: ADR-047's data-plane promise (per-connection open ops
invocable on channel 0) is unreachable through the mechanism the
amendment names. Every consumer of `register_openable` must either
adopt the undocumented per-session-base-registry shape or fail.
**Fix (Unit 1):** decide the mechanism deliberately (see F-02) and
amend ADR-047 §4's wording to describe the mechanism that actually
dispatches. The amendment's *rationale* (per-connection state, no
`context.env` downcast) stands under either option.
## F-02 [major] — `OperationRegistry` has no fork/clone surface; the proven per-session-registry shape requires one
**Verified:** YES. `OperationRegistry`
(`src/registry/registration.rs:97-100`) is
`HashMap<String, HandlerRegistration>` + a validators map, with
`new`/`register`/`registration`/`publish_validator`/`list_operations`
(`:102-180`) — no `Clone` impl and no fork API. Every type inside a
`HandlerRegistration` *is* clone-able (verified):
- `HandlerKind` — `#[derive(Clone)]` (`registration.rs:51`)
- `OperationSpec` — `#[derive(Debug, Clone, PartialEq)]`
(`spec.rs:174-175`)
- `OperationProvenance` — `#[derive(Debug, Clone, Copy, PartialEq,
Eq)]` (`registration.rs:58-59`)
- `CompositionAuthority` — `#[derive(Debug, Clone)]`
(`context.rs:82-83`)
- `Capabilities` — manual `impl Clone` (`core/types.rs:75-80`)
- `jsonschema::Validator` — `#[derive(Clone, Debug)]`
(jsonschema 0.46.10 `validator.rs:294`)
So the fork is a small addition (derive `Clone` on
`HandlerRegistration` + `OperationRegistry`, or an explicit
`fork() -> OperationRegistry` that copies both maps). Two shape
options:
- **(a) per-session base registry (recommended).** The
`install_channel_zero` hook (and any future session-establishment
seam) forks the deployment's base registry, registers the
per-connection operations on the fork (generic channel ops, the
openable ALPNs, and — with F-05 — the bootstrap op), and dispatches
channel 0 over the fork. This is the proven shape; the ADR wording
moves from "overlay registry" to "per-connection registry installed
as the session's dispatch registry." Cost: `services/list` /
`services/schema` registered on the base registry must be
re-registered (or inherited via fork) per session to stay
discoverable — the fork makes that free if the bootstrap ops are
part of the fork source.
- **(b) overlay-aware top-level dispatch.** Add an overlay fallback
in `dispatch` / `run_loop_single_stream`: when the base registry
misses, consult `connection.overlay_env()`. Keeps one static
registry and makes the amendment's wording true as written, but
touches the shared dispatch loop (higher blast radius, and the
overlay's `invoke_with_policy` shape — namespace-scoped, parent-
context-driven — does not match the top-level call shape, so the
fallback needs care: op-type lookup, ACL, visibility).
Recommendation: (a). It is the only shape with an end-to-end proof,
it needs no dispatch-loop change, and it matches ADR-019's layering
(the overlay stays what it is today: the nested-invocation landing
zone for imported ops — which F-05 needs).
**Fix (Unit 2):** `Clone`/fork surface + the ADR wording amendment;
acceptance = the review-001-style e2e gate (open op resolves over a
live channels connection through the *fork*).
## F-03 [minor] — Per-session fork + static base: no `Default`/builder seam exists to compose "base ops + per-session ops" without re-registration
**Verified:** YES. Even with a `Clone` surface (F-02), the hook must
compose three sources into the per-session registry: the deployment's
base ops (already registered, not retained anywhere as a "source"
list), the generic channel ops (`ChannelOperations::register_on`),
and the openables. A fork of the base registry makes the first source
free — this finding exists only if (a) is chosen *without* the fork
(i.e., rebuilding per-session registries from scratch). Record it as
a constraint on the fork API: it must clone *registered handlers*
(`HandlerKind` clones carry their closures), not just specs, so
`services/list` keeps its closure over the base registry.
`publish_validator`'s `validators` map must be carried too (schema
enforcement would silently vanish otherwise).
**Fix:** fold into F-02's fork surface; the acceptance test covers it
(`services/schema` on the fork still validates input).
---
# Part B — The connect side cannot serve; no wire path announces ops
## F-04 [major] — The connect side's channel-0 read pump resolves responses only; inbound `call.requested` frames are silently dropped — there is no serving half on channel 0
**Verified:** YES. `ChannelClient::from_connection`
(`src/channels/client.rs:85-134`) spawns the read pump as
`read_single_stream_until_closed(single_stream_reader, &pending_map)`
(`connection.rs:681-686`), whose only job is resolving the
`PendingRequestMap` — its frame handler (`dispatch_envelope`,
`connection.rs:705-...`) branches on `call.responded` /
`call.completed` / `call.aborted` / `call.error` and **has no arm for
`call.requested`**. The accept side's mirror
(`Dispatcher::run_loop_single_stream`, `dispatch.rs:727-850`) serves
requests and has no pending-resolution arm. Each side silently drops
the other half's frames.
Consequences:
- A consumer that dials a producer via `ChannelClient` can call ops
and open channels, but if the producer later calls an op *the
consumer serves*, the request is dropped (the caller's pending
hangs until the sweeper or connection close). This is the exact
shape ADR-022 §connection-direction-independence promises ("both
sides can be both simultaneously" — AGENTS.md §8), and ADR-036's
channel-0 pre-negotiation makes channel 0 the single control
plane where it must work.
- It blocks the bootstrap registration story (F-05): the hub cannot
call anything the WS/native client serves, and the client cannot
even receive the call that would let it register.
The fix is well-understood and symmetric: a full-duplex
single-stream read loop that branches both ways — `call.requested`
→ dispatch and write a response frame (the `run_loop_single_stream`
arms), `call.responded`/`completed`/`aborted`/`published` → resolve
pendings and route sink chunks (the read-pump arms). IDs are
UUID-generated on each side (`generate_request_id()`), so
cross-correlation is not a hazard; the abort arm needs to try both
tables (in-flight sink aborts *and* the pending map's cascade), which
`run_loop_single_stream`'s `EVENT_ABORTED` arm already does within
its own scope. The wasm-targetable browser story compounds the
value: the same serving loop is what a wasm alkcall browser
compiles.
**Fix (Unit 3):** a serving-capable single-stream loop usable from
the connect side (e.g., a `Dispatcher` variant or a
`CallConnection::serve_single_stream` that composes both frame
directions), wired into `ChannelClient` behind an opt-in (a serving
registry is not always present — a pure consumer may have none).
Acceptance gates: hub→consumer call over an existing `ChannelClient`
session resolves; consumer-served op participates in ACL (its
`AccessControl` gates the hub's call); disconnect mid-call still
fails pendings on both sides.
## F-05 [major] — No wire mechanism announces client-side ops; ADR-022's import flow is hub→consumer only and the browser-native path has none
**Verified:** YES. The envelope kind set is closed at six —
`call.requested` / `call.responded` / `call.completed` /
`call.aborted` / `call.error` / `call.published`
(`src/protocol/wire.rs:12-17`). Op registration onto an overlay
(`register_imported` / `register_imported_all`,
`connection.rs:162-172`) happens only in-process: `from_call`
(`client/from_call.rs:82-91`) builds bundles the *local* process
registers after *it* dialed out — the consumer importing a hub's
ops. There is no op, frame, or handshake by which a connected peer
announces "here are the ops I serve." Every consumer-facing
registration surface today requires the registrant to hold the
local `CallConnection` handle in-process.
Consequence: the decided bidirectionality model — either side
registers ops the other can call (AGENTS.md §8; ADR-022 §direction
semantics; alkhttp ADR-048's connection-local overlay for
browser-registered ops) — has no on-the-wire expression for the
non-in-process side. It is exactly the assumed-bootstrap-op set the
protocol already leans on (`services/list` is dialed and called by
`from_call` on every import; each side is *expected* to serve it)
extended by one op: e.g. `op/register` carrying the
`HandlerRegistration`'s serializable parts (spec + provenance +
access control), whose hub-side per-session handler writes the
registration into that connection's overlay via
`register_imported`. The overlay is already the landing zone
(`compose_root_env` attaches it keyed by identity,
`dispatch.rs:211-213`), and discovery of what a peer registered is
already built: `services/list-peers` reads
`ctx.env.peer_ids()` / `peer_operations()`
(`registry/discovery.rs:260-307`) — the same overlay-populated env.
Scope decision this forces (record it in the ADR, either way):
- **Design it** (recommended): `op/register` (+ a deregister or
replace semantics for the reconnect path) as an assumed op in the
bootstrap set, served per-session; the wire stays six kinds. The
`HandlerRegistration`'s `Handler` closures cannot cross the wire —
the client sends spec + a call-forwarding contract, and the
hub-side handler wraps it as a forwarding handler that issues a
nested `call.requested` back over channel 0 (the same shape
`from_call`'s imported bundles use — `protocol/adapter.rs:541,610`
show the in-process twin).
- **Or scope-cut**: amend ADR-022/alkhttp ADR-048 to say peer-side
op registration is hub-initiated-import only until a contract
exists. Cheaper, but it re-opens the same hedge the deferral was
criticized for — the promise would remain written-but-unmechanized.
Recommendation: design it. The pieces are all present; the cost is
one op handler, an `AccessControl`-gated registration surface (the
registry's existing ACL path covers it — an op with restrictive
`AccessControl` cannot be overwritten by an unprivileged peer), and
the F-04 serving loop to make it reachable in both directions.
**Fix (Unit 3):** with F-04 — bootstrap op + client serving loop +
ADR-022 amendment naming the bootstrap op set (assumed ops each side
may serve: `services/list`, `services/schema`, and `op/register`).
## F-06 [minor] — `services/list` on a per-session fork needs a closed-over fork reference, or per-session ops are undiscoverable
**Verified:** YES. `services_list_handler` closes over a *specific*
`Arc<OperationRegistry>` (`registry/discovery.rs:233-258`) — it
lists that registry's ops. Under the F-02(a) shape, per-session
openables live on the fork, so a `services/list` handler closed over
the base registry cannot see them. Two clean outs, both cheap: (i)
fork *before* registering the bootstrap discovery ops (then the
handler closes over the fork), or (ii) the fork API registers
base-registry bootstrap ops fresh on the fork. Either way the
constraint is: discovery ops must be registered against the
per-session registry, not the static one. Also note `services/list`
on the fork still ACL-filters (the handler re-checks per-caller) —
no privilege regression.
**Fix:** fold into Unit 2 (the fork seam registers bootstrap ops on
the fork); acceptance: a per-session openable appears in
`services/list` for an authorized caller and not for an unauthorized
one.
---
# Non-findings (verified correct, recorded to bound the re-review)
- **The e2e reference shape is real and passes** —
`channels/client.rs:767-870` wires `ChannelClient` ↔
`ChannelsAdapter` over duplex with an open op registered on the
per-connection registry, invoked end-to-end, quota reserved. The
F-02(a) shape is not speculative; it is in-tree.
- **`register_openable`'s wrapper machinery is complete** — ACL via
the registry's normal invoke path, `check_open` → `open_channel` →
ledger → spawn → teardown decrement, with `Once`/`Stream`/`Sink`
arms (`channels/operations.rs:399-453, 483-555`). Nothing in F-01
re-opens it; the ops it registers just need a dispatch-reachable
home.
- **Channel-id allocation is collision-safe** for the bidirectional
open story (accept side even from 2, connect side odd from 1,
`manager.rs:105-125`; `adopt_channel` for the non-allocating side,
`manager.rs:273-309`).
- **The `call.*` envelope kind set staying closed is correct** —
AGENTS.md §7 allows *adding* kinds but F-05's resolution does not
need one; bootstrap ops over channel 0 are the lighter door.
- **Wasm-cleanliness is unaffected** — the recommended fixes (fork +
handler + read-loop branch) touch no transport, no fs/net tokio
features; `cargo check --target wasm32-unknown-unknown` remains
the release gate.
---
# Remediation plan
Sequenced by dependency. Units 1–3 are alkcall work; the alkhttp
consequences are tracked as alkhttp review 003 Unit 1 (updated to
point here).
## Unit 1 — Decision recording (F-01, F-02 direction)
ADR work only:
- ADR-047 §4 amendment #2: the open-op registration mechanism is the
**per-connection registry installed as the session's dispatch
registry** (option (a)); the overlay registry remains the landing
zone for peer-announced ops (F-05) and nested invocation.
- ADR-022 amendment: the bootstrap-op set (each side may serve
`services/list`, `services/schema`, `op/register`), and the
direction-semantics note that serving on the connect side is
opt-in (F-04).
- alkhttp ADR-048's bidirectionality promise is re-pointed at these
ADRs instead of standing as an unmechanized commitment.
Gate: ADRs recorded; alkhttp OQ-05 and ADR-048 cross-reference them.
## Unit 2 — Fork surface + per-session composition (F-02, F-03, F-06)
- `Clone` (or explicit fork) on `HandlerRegistration` +
`OperationRegistry`, carrying handlers and validators.
- A composition helper (e.g. `OperationRegistry::fork_with(...)`) or
documented pattern: fork base → register generic channel ops +
openables + bootstrap ops against the fork.
- Gate: e2e over a live channels connection — open op resolves
through the fork; `services/list` shows per-session openables
(ACL-filtered); `services/schema` on the fork still validates.
## Unit 3 — Client serving half + bootstrap registration (F-04, F-05)
- Serving-capable single-stream loop (compose the read-pump and
dispatch arms); wired into `ChannelClient` opt-in.
- `op/register` bootstrap op (per-session handler →
`register_imported` into the connection overlay), with
`AccessControl` gating and reconnect/replace semantics decided in
the ADR.
- Gates: hub→consumer call over an existing session resolves;
peer-registered op is discoverable via `services/list-peers` and
callable back over channel 0; unregistered/unauthorized
registration attempts fail loudly; disconnect drops the overlay
and fails both sides' pendings.
## Unit 4 — alkhttp wiring (downstream, after Units 1–3)
Tracked in alkhttp review 003 (Units 2–4 there): openables surface,
`install_channel_zero` rework over the fork, retained session
handle, e2e tests, spec reconciliation (OQ-05, ADR-067/048 v1-cut
notes).
---
## Verification log (this pass)
- All claims verified at tree `c16b069`; the F-01 dispatch claim was
checked against both dispatch paths (`run_loop` stream-per-request
reads `handle_stream` → same base-registry-only resolution;
`run_loop_single_stream` at `dispatch.rs:727-850`).
- The F-04 claim was verified by reading both loops' frame arms:
`read_single_stream_until_closed` → `dispatch_envelope`
(`connection.rs:681-705`, no `EVENT_REQUESTED` arm) vs
`run_loop_single_stream` (`dispatch.rs:754-850`, no
pending-resolution arm).
- The F-02 clone claims were verified type-by-type, including the
third-party `jsonschema::Validator` (0.46.10) and the vendored
`Capabilities` (manual `Clone` preserving `Secret`'s clone).
- The F-05 wire-closure claim was verified against the full kind set
(`wire.rs:12-17`) and grep over `src/` for any wire-side
registration path (none; `register_imported*` callers are
in-process only).
- `services/list-peers`' overlay-backed discovery verified at
`registry/discovery.rs:260-307` (reads `ctx.env.peer_ids()` /
`peer_operations()`, populated by `compose_root_env` at
`dispatch.rs:211-213`).
- Baseline gates re-run for this pass: cargo test (565), clippy
(all-targets), fmt — all clean. No source changes.