Files
alkcall/docs/reviews/001-pub-and-channels-integration-review.md
glm-5.2 f1d6796436 docs: consolidated review 001 — Pub (ADR-046) + channels integration (ADR-047)
Consolidates three code-review passes (one main summary + sub-review A
on ADR-046 + sub-review B on channels ADRs 034–043/047) into a single
verified document under docs/reviews/. Every finding was re-verified
directly against the source at f305f8c with exact file:line refs.

Findings (all verified): 10 critical, 19 major, 7 minor.
- Pub end-to-end cannot work (chunks written to a different bi-stream
  than the request); publish() takes Vec<Value> not a Stream; 30s
  client timeout; abort-doesn't-cancel-Pub; dispatch re-implements
  invoke_sink inline; from_call sink forwarding swallows errors.
- Channel 0 dead in both directions; register_openable absent;
  resolve_channel_manager stub; backpressure drops chunks; buffer cap
  in messages not bytes; ledger decrement on 1/4 teardown paths; demux
  desyncs on oversized chunk; no channel-adoption/collision scheme.
- channel/control + resources/subscribe stubs; busy-wait spin; lost
  EOF sentinel; TOCTOU on max_channels; ~50 lines of abandoned
  deliberation in poll_write; cargo doc 4 warnings; spec docs
  inconsistent post-047.

Includes a 6-unit remediation plan sequenced by dependency, with
acceptance gates. Units 1/2/4 need no spec decisions; Units 3 and 5
each need one written ADR decision first (ADR-047 §4 env-resolution;
§5 channel-id adoption). The overarching acceptance gate is the
end-to-end ChannelClient ↔ ChannelsAdapter test whose absence let
both commits land green.

Two corrections to the original reviews noted in the verification log:
- C-17 (lost EOF sentinel): Sub Review B overstated "the pump does not
  write an EOF chunk" — the pump does write EOF when it receives the
  sentinel; the real bug is narrower (sentinel lost on full buffer →
  pump exits on recv→None without writing EOF).
- Warning count: main review said 5 cargo doc warnings; this pass sees
  4 (2× register_openable, 1× default_policy, 1× env module/macro).

Verification:
- cargo test → 432 passed (no source changed; docs-only commit)
- cargo clippy --all-targets -- -D warnings → clean
- cargo fmt --check → clean
- cargo doc --no-deps → 4 warnings (documented as C-09; not addressed
  here — fixing them requires the register_openable method to exist,
  which is Unit 3 of the remediation plan)
2026-08-12 13:03:36 +00:00

56 KiB
Raw Blame History

Review 001 — Pub (ADR-046) and Channels Integration (ADR-047)

Status

Verified, open for remediation.

Scope

Consolidated code review of the two commits that landed the Pub operation type / HandlerKind::Sink (ADR-046, commit ea66398) and the channels protocol + openable-ALPNs model (ADR-047, commit f305f8c). The review consolidates three passes (one main summary + two sub-reviews: one focused on ADR-046, one on the channels ADRs 034043/047) and was produced against the tree at f305f8c.

Every finding below was re-verified directly in the source during this consolidation pass (2026-08-12). Each finding carries: the ADR it drifts from (if any), a severity, the exact file:line reference(s), the verification status, and the unit-of-work bucket it belongs to. Findings that did not survive verification, or that the original reviews overstated, are called out as such.

Baseline verification (this pass)

cargo test                        → 432 passed, 0 failed
cargo clippy --all-targets -D warnings → clean
cargo fmt --check                 → clean
cargo doc --no-deps               → 4 warnings (see C-09)

The suite is green, but green is misleading here: all coverage is unit-level. Not a single test wires two sides of either new protocol together end-to-end (no CallConnection::publishDispatcher::handle_stream round-trip; no ChannelClientChannelsAdapter round-trip). The integration seams — which is exactly where ADR-047 lives — are where the real problems live, and the absence of end-to-end tests is what let both commits land green with multiple implementation-killing defects.

Verdict

  • ADR-046 registry half (types, HandlerKind::Sink validation, invoke_sink, builders, wire strings, discovery strings): solid, close to spec, well tested at the unit level.
  • ADR-046 protocol half: has one implementation-killing bug (P-01: publish chunks are written to a different bi-stream than the request — end-to-end publish cannot succeed on any transport), plus two unimplemented decided behaviors and an abort story that exists only in doc comments.
  • ADR-047: the components are good but the integration layer — the part ADR-047 actually decided — is scaffolding. As committed, the channels protocol cannot open, use, or account for a single data channel end-to-end, channel 0 is dead in both directions, and a publish() cannot succeed on any transport.

The architecture is right: no channels types leaked into the call crate, OperationEnv stayed a trait, the wire formats (envelope + 8-byte chunk header) match spec exactly, the channel_open marker plumbing (Gap F) is complete and tested, and auth stays on the one AccessControl path. What's missing is the connective tissue a tty/docker crate would actually call.

Severity legend

  • [critical] — the protocol cannot function end-to-end as committed on any transport; 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 stores wrong data.
  • [minor] — drift, convention violation, dead code, or a doc/spec inconsistency with no correctness impact.

Part A — Pub / HandlerKind::Sink (ADR-046) findings

P-01 [critical] — Publish chunks are written to the wrong stream; end-to-end publish cannot work

ADR drift: ADR-046 §5, §8 (the stream lifecycle and publish() client method). Verified: YES. src/protocol/connection.rs:285-311 (open stream A, write call.requested), src/protocol/connection.rs:334-362 (write_publish_chunks opens a fresh open_bi() at line 343-346 and writes all call.published + call.completed on stream B). Responder side: src/protocol/dispatch.rs:433-435, 516 (pump_sink reads chunks from the same stream the request arrived on — stream A). Stream B arrives as a fresh accept_bi whose frames hit the handle_stream "ignoring non-requested/non-aborted event" branch (src/protocol/dispatch.rs:443-444) and are silently discarded.

Consequences (verified):

  • Multi-stream transport (QUIC): chunks are lost; pump_sink blocks on stream A until the client's dropped write-half surfaces as reset/EOF — best case the handler sees an empty stream and returns a bogus result (e.g. count: 0), worst case it hangs until the 30s client-side timeout (P-05).
  • Single-stream transport (Connection::from_bidi): the second open_bi returns StreamClosed (src/core/types.rs), so every publish fails outright with "failed to open stream".

No end-to-end publish test exists (see P-12), which is how this landed green. Fix: keep stream A's write half and pump chunks on it. This is also what a Stream-typed publish (P-02) would force structurally. Acceptance gate: one end-to-end publish test that moves ≥2 chunks and asserts the responder sees them.

P-02 [major] — publish() takes Vec<Value>, not a Stream

ADR drift: ADR-046 §8 specifies stream: Pin<Box<dyn Stream<Item = Value> + Send>>. The implementation is chunks: Vec<Value>. Verified: YES. src/protocol/connection.rs:249-254 (publish) and :268-272 (publish_with_payload) both take chunks: Vec<Value>; write_publish_chunks (:334-362) iterates the Vec.

This buffers the entire publish in memory before anything is sent and makes the ADR's flagship use cases (telemetry ingest, live upload, drag-drop file streaming) impossible from this API — the chunk set must be fully materialized up front. Fix P-01 + P-02 together: hold stream A's write half, accept a Stream, pump it as call.published events. This also resolves P-05's tension (a streaming publish should register like subscribe with timeout: None, not the 30s Call deadline).

P-03 [major] — publish_schema per-chunk validation is written but never read

ADR drift: ADR-046 §4, and the field's own doc comment at src/registry/spec.rs:181-186 which says "the dispatch path validates each call.published event's payload.input against this schema before yielding it to the SinkHandler." Verified: YES. grep publish_schema outside src/registry/spec.rs matches nothing. The field is declared (spec.rs:186), defaults to None (:228), and has a builder (with_publish_schema, :236-239), and is never read anywhere. pump_sink forwards chunks as-is (src/protocol/dispatch.rs:527-535). It is also missing from spec_to_json, from operation_spec_schema(), and from from_call's rebuild_spec_for — so an imported Pub op loses the schema entirely. alktype is a declared dependency (Cargo.toml:21) and AGENTS.md §10 says to use it for exactly this; it is unused in src/. Dead field + documented-but-unimplemented behavior.

P-04 [major] — Initiator call.error does not terminate the publish stream

ADR drift: ADR-046 §5 event table and §6 ("Err(call_error) for an initiator-side error (a call.error event from the initiator)"); plus the SinkHandler/PublishStream doc contract (src/registry/registration.rs:26-31, 44-49). Verified: YES. pump_sink matches only "call.published", "call.completed", "call.aborted" and drops everything else into a debug-log ignore branch (src/protocol/dispatch.rs:526-550). A call.error frame is silently ignored; the only Err ever injected is the call.aborted case, and it's an ad-hoc CallError::internal("publish aborted by initiator") (dispatch.rs:538-543), not the initiator's error.

P-05 [major] — Client-side 30s timeout on Pub contradicts "long-lived, same as Sub"

ADR drift: ADR-046 §7 ("the stream may be long-lived, same as Sub") clears the deadline for Pub on the responder. Verified: YES. publish_with_payload registers via register_call with DEFAULT_CALL_TIMEOUT = 30s (src/protocol/connection.rs:32, 296-303). The sweeper resolves expired Call entries with TIMEOUT (src/protocol/pending.rs). subscribe avoids this with register_subscribe(timeout: None) (connection.rs:215-218). Any publish whose stream takes >30s wall-clock gets a spurious client-side TIMEOUT while the responder keeps consuming. Fix: register like subscribe with timeout: None (naturally falls out of the P-01/P-02 streaming rework).

P-06 [major] — Abort does not cancel a Pub

ADR drift: ADR-046 §5 table (call.aborted | either side | "Cancel the Pub"); ADR-020 (stream drop on abort; cascade through sink handlers). Verified: YES, both abort paths are broken:

  • Cross-stream (the path the shipped client exercises): CallConnection::abort() sends call.aborted on a new stream (src/protocol/connection.rs:236-243write_envelopeopen_bi at :388-392). On the responder that frame is handled by a different handle_stream task, whose handle_abort only mutates the PendingRequestMap (src/protocol/dispatch.rs:381-389). Inbound requests are never registered in that map, and nothing links the map to the spawned handle_stream/pump_sink task — the running SinkHandler and feed loop are untouched. The doc comments at dispatch.rs:494-498 (and :459-462 for pump_stream) describe a cancellation mechanism that does not exist.
  • Same-stream (dead-untested): pump_sink injects an Err and breaks the feed on call.aborted (dispatch.rs:538-543), then still tokio::join!s the handler to completion and writes the response back to the aborting initiator (:562-568). The handler future is never dropped.

Net: an initiator cannot actually cancel an in-flight Pub. (The pump_stream half of this predates ADR-046, but ADR-046 §5 explicitly promises abort-cancels-Pub, so it is in scope here.) Also: during a sink pump, call.aborted frames for other request IDs on the same stream are dropped by the id-mismatch guard before the type match (dispatch.rs:518-525) — a conforming peer that serializes calls on one stream loses requests. Same for a second call.requested multiplexed on that stream.

P-07 [major] — Handler-returns-early: response withheld until the initiator finishes

ADR drift: ADR-046 §7 (long-lived publish streams). Verified: YES. pump_sink uses tokio::join!(feed_fut, handler) and only writes the response after both complete (src/protocol/dispatch.rs:562-568). If the SinkHandler returns early (e.g. rejects the upload after chunk 1), the feed loop notices only when its next chunk_tx.send fails (dispatch.rs:533-535) — only when another chunk arrives. An initiator that pauses between chunks, or a long/infinite publish, never learns of the early error; the response is deferred indefinitely. Not a deadlock, but an unbounded stall on a path the ADR intends for long-lived streams.

P-08 [major] — The dispatch Pub branch re-implements invoke_sink inline

ADR drift: ADR-046 §6, §7 (Pubinvoke_sink(); dispatch() branches on op_type and routes Pub through invoke_sink). Verified: YES. The Pub branch of Dispatcher::dispatch re-implements the not-found / visibility / ACL / handler-kind checks inline (src/protocol/dispatch.rs:309-377) and invokes the SinkHandler directly, duplicating OperationRegistry::invoke_sink (src/registry/registration.rs:283-345) line for line. invoke_sink is only ever called from its own tests. Divergence hazard: any future fix in invoke_sink (e.g. adding the missing publish_schema validation, P-03) won't reach the wire path.

P-09 [major] — from_call sink forwarding is store-and-forward and swallows errors

ADR drift: ADR-046 §9 (sink forwarding); the SinkHandler contract (src/registry/registration.rs:29-31: an Err terminates the stream). Verified: YES. make_sink_forwarding_handler does publish_stream.filter_map(|item| async move { item.ok() }).collect::<Vec<_>>().await then one publish_with_payload call (src/client/from_call.rs:464-475). Three problems:

  1. The whole stream is buffered in memory — unbounded, contradicting the doc comment "No truncation — the full stream is forwarded end-to-end" (from_call.rs:452-453) in spirit (forwarded, but not streamed; a hub relaying a large upload holds it all in RAM).
  2. Err items are silently discarded and iteration continues (item.ok()None → filtered out), contradicting the contract that an Err terminates the stream — a partial/errored publish is forwarded upstream as if it completed cleanly.
  3. It inherits P-01, so the forwarded publish is broken anyway.

P-10 [minor] — String literals instead of wire constants in pump_sink

Verified: YES. pump_sink matches "call.published", "call.completed", "call.aborted" (src/protocol/dispatch.rs:527, 537, 538) instead of EVENT_PUBLISHED/EVENT_COMPLETED/EVENT_ABORTED (src/protocol/wire.rs:12-17), which the rest of the file imports and uses. Refactor hazard, not a correctness issue.

P-11 [minor] — ADR-046 §3 vs §6 SinkHandler stream-item-type inconsistency

Verified: YES (internal ADR inconsistency, not a code bug). ADR-046 §3 defines SinkHandler receiving Pin<Box<dyn Stream<Item = Value> + Send>>; §6 defines invoke_sink's publish_stream as Item = Result<Value, CallError>. The code resolves this in favor of §6 uniformly: SinkHandler itself takes Item = Result<Value, CallError> (src/registry/registration.rs:32-40), aliased as PublishStream (:49), and invoke_sink passes it straight through (:344). This contradicts §3's literal type but is arguably the right resolution (the handler must see initiator errors), and the ADR's Door-type section explicitly marks the concrete stream item type a two-way-door detail. Action: amend ADR-046 §3 text to match §6.

P-12 — Test coverage gaps (ADR-046)

Verified: YES — all 23 new Pub tests are either registry-level (invoke_sink with a synthetic stream), responder-side with hand-crafted frames on one stream, or builder/wire-shape tests. src/protocol/connection.rs has zero publish tests. Specific decided behaviors with no test:

  1. End-to-end publish through CallConnection::publishDispatcher::handle_stream (would have caught P-01 immediately).
  2. deadline: None for Pub — dispatch_pub_clears_deadline_to_none asserts nothing about the deadline; the test name is misleading.
  3. publish_schema validation (untested + unimplemented, P-03).
  4. Initiator call.error terminating the publish stream (P-04).
  5. Abort during a Pub, both directions (P-06 — same-stream path is dead-untested; no initiator-side abort() of an in-flight publish).
  6. from_call Pub forwarding — no test that build_bundles maps "pub"HandlerKind::Sink; no behavioral test of make_sink_forwarding_handler (Err-dropping, forwarded_for population).
  7. Backpressure — nothing exercises a full 64-slot chunk channel (PUBLISH_CHANNEL_BUFFER, dispatch.rs:45) with a slow handler, or the handler-returns-early feed-termination path.
  8. OverlayOperationEnv Sink arm (connection.rs:523-528) — the Sub arm has a test; Sink does not.

P-13 [minor] — SinkDispatch exposes chunk_tx publicly

Verified: YES. SinkDispatch exposes chunk_tx: futures::mpsc::Sender publicly (src/protocol/dispatch.rs:78-81). The channel type and buffer size are effectively public API now; consider opaque wrapping before crates.io.


Part B — Channels (ADR-034043, 047) findings

C-01 [critical] — Channel 0 is dead in both directions

ADR drift: ADR-036 (channel 0 pre-negotiated as alknet/call). Verified: YES, both sides.

  • Accept side: src/channels/adapter.rs:181-188 — channel 0's Connection is built and bound to _channel0_conn, which is never handed to anything. The InstallChannelZero hook (adapter.rs:41-42) receives only (&ChannelManager, &AuthContext) — it has no way to reach the channel-0 Connection or its send/recv halves (already consumed by install_channel_zero at adapter.rs:181, which can't be called again — it returns ChannelExists). Inbound call.requested chunks on channel 0 route into a receiver nobody reads. Channel 0 on the accept side is a black hole.
  • Connect side: src/channels/client.rs:64-65 wraps channel 0 in CallConnection, but CallConnection::call_with_payload opens a new bidi stream per request via connection.open_bi(), and ChannelBidiStreamSource::open_bi returns Err(StreamClosed) unconditionally (src/channels/source.rs:58-60). So every ChannelClient::call_open_op fails immediately with "failed to open stream."

The call protocol's stream-per-request model (Dispatcher::run_loop accepting streams in a loop, dispatch.rs:603-620; CallConnection opening one stream per call) is structurally incompatible with channel 0's single yield-once BiStream. This needs a single-stream call mode (multiplex all requests on one framed stream for both the client and the dispatch side), which is a real design gap, not just a bug fix. Acceptance gate: one end-to-end channel-0 test: ChannelClient::call_open_opChannelsAdapter accept-side call dispatch, one round-trip.

C-02 [critical] — register_openable / the ADR-047 §3 open flow does not exist

ADR drift: ADR-047 §3 (ChannelCore wrapper + register_openable(spec, open_handler, channel_core) helper; the ACL→cap→allocate→ledger→spawn→respond flow). Verified: YES. ChannelCore has only new/manager/check_open/ on_close (src/channels/operations.rs:280-311). Doc comments link to ChannelCore::register_openable (operations.rs:10, 62) — the rustdoc warnings in C-09 are those dangling links. There is currently no way to register a channel-open op at all; the channel:forbidden / channel:no_channels_session / channel:allocation_failed / channel:too_many_channels error-code mapping to CallError exists only in comments. The whole open flow is unreachable.

C-03 [critical] — resolve_channel_manager (ADR-047 §4) is a stub

ADR drift: ADR-047 §4 (per-connection ChannelManager resolution via the extension-trait downcast). Verified: YES. resolve_channel_manager unconditionally returns None (src/channels/env.rs:108-128). The comment at :111-125 admits a real downcast is impossible without Any-machinery and says "the implementation uses the ChannelCore's stored manager instead" — which contradicts ADR-047 §4's core point (a statically-registered handler closing over a static ChannelCore "has no way to know which channels connection invoked it"). ChannelsSessionEnv exists (env.rs:46) but nothing constructs it outside tests (env.rs:148 is the only construction site). channel:no_channels_session appears only in comments. Decision needed: either add as_any() to OperationEnv (or similar) to make §4 implementable, or amend ADR-047 to bless the "manager passed at registration via ChannelCore" shape. The current stub gives you neither per-connection resolution nor the amended spec.

C-04 [critical] — Backpressure is inverted: the demux drops chunks

ADR drift: ADR-040 DP-5 / REQ-CH-05 (lossless bounded-buffer backpressure — the demux stalls until the consumer drains). Verified: YES. route_payload uses try_send and on Full logs a warn and drops the chunk (src/channels/manager.rs:253-258). route_payload is sync (manager.rs:246), so it cannot await — the shape itself deviates. This is a reliability-layer protocol with no retransmit; a dropped mid-stream data chunk silently corrupts the channel's byte stream for the handler. Fix: make route_payload async (or use a blocking_send/reserve-equivalent) and await on full.

C-05 [critical] — The buffer bound is in messages, not bytes

ADR drift: ADR-040 ("1 MiB default"; "256 × 1 MiB = 256 MiB worst case per connection" memory bound). Verified: YES. DEFAULT_BUFFER_CAP = 1024 * 1024 (src/channels/reassembly.rs:35) is passed as the capacity of tokio::mpsc::channel (reassembly.rs:77, also manager.rs:180, 219, mux.rs:115), which counts messages, not bytes. The actual per-channel bound is 1,048,576 chunks × up to 16 MiB each ≈ 16 TiB, not 1 MiB. The doc comment at reassembly.rs:31-34 even claims "1 MiB" while the bound is ~6 orders of magnitude larger. ADR-040's per-connection memory bound effectively does not exist. Fix: a byte-budgeted bound (track accumulated payload.len() against the cap, not message count).

C-06 [critical] — Opener-ledger decrement happens on only 1 of 4 teardown paths

ADR drift: ADR-047 §7 (the self-DoS fix: decrement on close-received, close-sent, handler-exit, connection-drop). Verified: YES. Only close-received is wired (src/channels/operations.rs:197-204). On transport EOF, clear_all() returns the (channel_id, opener) list (src/channels/manager.rs:305-316) and the demux loop only logs its length (src/channels/adapter.rs:139-143) — on_close is never called. Close-sent locally: nothing sends channel/close locally, no decrement hook. Handler exit: a handler task that finishes naturally triggers no teardown at all. The exact "peer whose connection dies at cap is permanently at cap" self-DoS that ADR-047 §7 was written to fix is back. The auth-blind adapter has no way to reach the policy — there is no hook — so this is an architectural gap, not just a missed call.

C-07 [critical] — Demux desynchronizes permanently on an oversized chunk

ADR drift: channels-wire.md §MAX_CHUNK_LEN (resync by skipping length bytes; the parsed length is available in the error variant). Verified: YES. On ChunkError::TooLarge (and any header-parse error) the loop continues without skipping the payload bytes (src/channels/adapter.rs:104-110). The next 8-byte header read consumes the first 8 bytes of the oversized payload → permanent stream desync; every subsequent "header" is garbage routed to random channel_ids (lenient drop hides the corruption). ChunkError::TooLarge carries the parsed length (src/channels/wire.rs:95-98) — it is discarded. Fix: on TooLarge, read and discard length bytes before continuing.

C-08 [critical] — No channel-adoption path → channel-id agreement is impossible

ADR drift: implied by ADR-037/047 §5 (connection-owner allocates). Verified: YES. The side that did not allocate a channel_id has no way to install routing state for it: open_channel always allocates a fresh id (manager.rs:167), install_channel_zero is hardcoded to 0. A consumer that receives {channel_id: 7} from the responder cannot register 7 in its local manager/mux — inbound chunks for 7 are dropped as unknown (manager.rs:268-274) and there's nothing to write to. Additionally, both sides construct independent managers with next_id starting at 1 (manager.rs:98; adapter side adapter.rs:170; client side client.rs:55), so if both sides ever allocate on one connection (Sub opened by A + Pub opened by A, per ADR-047 §5's Pub case), IDs collide — one wire channel_id, two different channels. No odd/even split or any collision-avoidance scheme exists in either the ADRs (post-047) or the code. This needs a spec decision (odd/even split, or an adoption op), not just a code fix.

Verified: YES — 4 warnings (the original reviews said 5; this pass sees 4):

  • unresolved link to ChannelCore::register_openable ×2 (from operations.rs:10, 62 — the nonexistent helper, C-02).
  • unresolved link to default_policy (from operations.rs:49).
  • env is both a module and a macro (from the env module name colliding with the env! macro).

The commit message's "cargo doc generates" was technically true; clean it is not.

C-10 [major] — channel/control is a stub returning success for a dropped message

ADR drift: ADR-037 ("the channels layer routes message to the handler's control handle for channel_id"). Verified: YES. src/channels/operations.rs:236-248 parses channel_id, ignores message (_message), and returns {"ok": true}. There is no control-handle concept anywhere (no field on ChannelState). Returning success for a silently-discarded control message is worse than returning unimplemented.

C-11 [major] — channel/resources/subscribe emits a one-shot snapshot of the wrong data

ADR drift: ADR-037 (as amended by ADR-047 §6): live subscription of openable resources aggregated from ALPN-crate resource enumerators, with subsequent events on any change. Verified: YES, three problems (src/channels/operations.rs:257-277):

  1. futures::stream::once(...): emits only the initial snapshot, never "subsequent events on any change." ADR-037 explicitly rejected poll-shaped behavior; this is a one-shot poll wearing a Sub type.
  2. Wrong content: emits the ALPNs of currently-open channels, not the set of openable ALPNs aggregated from ALPN-crate resource enumerators ("which containers are running…"). A fresh connection reports an empty resource set even when TTY/tunnel ops are registered.
  3. (Positive) The access preview is correctly dropped per ADR-047 §6 (operations.rs:255; the output schema at :148-161 has no access).

C-12 [major] — No guard on channel/close with channel_id: 0

Verified: YES. channel/close has no guard against channel_id: 0 (src/channels/operations.rs:190). Any peer can tear down channel-0 state via teardown_channel(0) (manager.rs:284-298), killing the call channel's demux routing. Fix: reject channel_id: 0 in the channel/close handler (channel 0 is pre-negotiated and not closeable).

C-13 [major] — channel/close abort ≠ drain; REQ-CH-06 ordering absent

ADR drift: ADR-041 §3 (decrement "after the drain completes"); ADR-037 (responder drains and signals EOF to the handler); channels-wire.md REQ-CH-06 (exit-chunk-before-close ordering — "the channels layer owns the ordering guarantee"). Verified: YES. channel/close's handler task is abort()ed immediately and the decrement happens immediately (src/channels/operations.rs:190-204); abort ≠ EOF-and-drain. REQ-CH-06 (the close handler must observe data-pump completion) has no implementation anywhere.

C-14 [major] — Substrate modes: only in-line is implemented

ADR drift: ADR-034 §substrate modes, ADR-039 (QUIC-native multi-stream substrate). Verified: YES. handle() calls connection.accept_bi() once and never loops (src/channels/adapter.rs:159). The doc comment at adapter.rs:80-84 claims the multi-stream behavior the code doesn't have. The QUIC-native substrate ("accept remaining bidi streams… read headers off each") is not implemented.

C-15 [major] — ChannelClient API does not match ADR-043

ADR drift: ADR-043 (open_channel(...) -> Channel, Channel { channel_id, source }, subscribe_resources, call() accessor). Verified: YES. src/channels/client.rs has from_connection ✓ (transport-agnostic, correct per the amendment), but: no open_channel(...) -> Channel, no Channel { channel_id, source }, no subscribe_resources, no call() accessor (instead call_open_op

  • take_call_connection). The doc comment at client.rs:104-106 refers to manager.open_channel_stream(channel_id)no such method exists. A consumer who receives {channel_id: 7} from the responder has no way to obtain a BiStream for it (see C-08, channel adoption).

C-16 [major] — Busy-wait spin on full write buffer

Verified: YES. poll_write on TrySendError::Full does cx.waker().wake_by_ref(); Poll::Pending (src/channels/reassembly.rs:256-257) — a hot busy-loop that pegs a core until the mux drains (the waker is re-woken immediately, not registered against capacity). The correct shape is storing a reserve()/poll_ready-style future. The abandoned deliberation left in comments (reassembly.rs:205-255) documents that the author knew this (see C-20).

C-17 [major] — Lost EOF sentinel path → remote handler hang

Verified: YES, with a precision correction to Sub Review B. The mux pump does write an EOF chunk to the wire when it receives the zero-length sentinel (src/channels/mux.rs:124-133). The real bug: MpscSendStream::poll_shutdown (reassembly.rs:283-291) and Drop (:295-309) emit the sentinel via try_send, best-effort; if the buffer is full the sentinel is silently dropped. When the sentinel is lost, the pump's recv yields None (sender dropped, no sentinel) and the pump loop exits (mux.rs:122-147) without writing an EOF chunk. Result: the remote handler never sees channel EOF until full transport close. REQ-CH-01 violation on this path. The doc comments at reassembly.rs:283-291, 295-309 ("the peer's read will still EOF when the sender drops — REQ-CH-02") are wrong about which side of the wire this sender feeds (it feeds the local mux pump, not the peer's demux). The doc comment at mux.rs:109-112 ("the pump emits the EOF sentinel") is misleading because it depends on the Drop impl's try_send succeeding. Fix: the pump must write an EOF chunk when its receiver ends without a sentinel (treat recv → None as implicit EOF).

C-18 [major] — open_channel TOCTOU on max_channels

Verified: YES. src/channels/manager.rs:159-194 — the cap check (:161) and the insert (:188-189) are under separate lock acquisitions with mux.register(...) awaited in between (:170-176). N concurrent opens at len == max-1 all pass the check and all insert → cap exceeded by N-1. Fix: hold the channels lock across check + insert (release for the mux.register await, re-acquire for insert, re-check on re-acquire).

C-19 [minor] — Misc channels correctness issues

  • next_id wrap allocates channel 0: manager.rs:98 starts at 1; fetch_add(1, Relaxed) wraps u32::MAX → 0, colliding with the reserved channel 0. The ChannelExists defensive check catches the map collision (:189-193), but only after registering a mux pump that is never cleaned up. Effectively unreachable in practice (ADR-040), but the wrap story is not handled as stated.
  • Unbounded loop in resources snapshot: operations.rs:267-274for channel_id in 0..u32::MAX with a mutex lock per iteration, break keyed on resources.len() >= manager.open_count(). open_count() is re-read each iteration; a channel closing mid-iteration (or sparse ids after churn — monotonic ids guarantee sparseness) can iterate millions/billions of times.
  • Mux pump map leak + duplicate registration: mux.rs:148 — finished pump JoinHandles accumulate in self.pumps for the connection's lifetime. Duplicate register(channel_id) silently overwrites the handle while the old pump keeps running (two pumps, one channel id).
  • Mux shutdown aborts pumps mid-write_chunk: mux.rs:154-158 — pumps are abort()ed possibly mid-write_chunk while holding the writer lock (a half-written chunk header corrupts the wire; moot since the transport is going away, except the code then writes EOF sentinels onto that possibly-mid-chunk stream).
  • manager.rs:309: drop(state.demux_sender.clone()) clones a sender then drops the clone — a no-op (the real drop happens when drained falls out of scope; the code works by accident, the line is misleading).
  • manager.rs:176, 218: mux register failure (runner closed) mapped to ManagerError::ChannelExists — wrong error.
  • client.rs:61: install_channel_zero error mapped to StreamError::StreamClosed — conflation.
  • policy.rs:127-140: check_open increments the count (check-and-reserve). If allocation subsequently fails, no rollback API exists other than on_close(&Identity); a future open-op wrapper must remember to decrement on every post-check failure path or quota leaks. Not a bug today (nothing calls it), but a trap in the trait's contract vs ADR-041's "check_open = check, on_close = decrement after deallocation" framing.
  • REQ-CH-04's error counter (Demux::stats()): no counter, no stats surface; only a debug! log (manager.rs:268-274).
  • derive_alpn_from_op_name (src/client/from_call.rs:289-301): ADR-047 says non-alknet/* ALPNs "use their full ALPN string as the path segment." The implementation takes only the first path segment (rest.split('/').next() at :291), so channels/custom/proto/sub derives alknet/custom — wrong. The segment.starts_with("alknet/") branch at :295 is dead code (a single segment can't contain /), and segment == "alknet" yields the nonsense ALPN "alknet" (:296).
  • Box::leak per discovery (from_call.rs:312-314): bounded per unique ALPN in theory, but it leaks on every rediscovery of every marked op. Documented in-code as accepted; the Arc<str> refactor should happen before any long-lived hub uses from_call against churning peers.
  • from_call.rs:269 "SAFETY:" comment is factually wrong (describes a 'static-only return that isn't what derive_alpn_from_op_name does — it returns Option<String>; the leak happens in leak_alpn), and "SAFETY" is a loaded term that conventionally marks unsafe blocks; there's no unsafe here.
  • Ledger placement: ADR-047 §7 says "the ledger lives in channels-call, not channels-core," but OpenerLedger is defined in src/channels/mux.rs:169-207 and owned by ChannelManager (manager.rs:81) — the auth-blind core layer. Doc comment at manager.rs:62-69 argues for this; it's a deliberate deviation, but it is a deviation, and the "which-crate" boundary matters when the crate is split per ADR-044.

C-20 [major → convention] — Abandoned deliberation + pervasive inline comments

Convention drift: AGENTS.md §1 (no inline // comments unless asked; doc comments //////! are fine). Verified: YES. Worst instance: src/channels/reassembly.rs:205-255 — ~50 lines of abandoned stream-of-consciousness deliberation ("no, we need Sender::reserve_slot… Actually, the simplest approach…") shipped inside poll_write. Plus pervasive inline // comments across the channels module: adapter.rs:154-158, 163-165, 177-180, 189-194, client.rs:57, 67, 74-76, manager.rs:170, 177-179, 190-191, 196, 234-237, 268-269, 291-292, 314, env.rs:112-126, operations.rs:188-189, 195-196, 243-247, 262-266, from_call.rs:256-262, 269-274. A handful qualify as "non-obvious correctness constraint" (REQ references); most don't. Several comments are actively wrong (the mux EOF claim mux.rs:109-112, the abort-cancels claim dispatch.rs:495-498, the reassembly.rs:283-291 "peer's read will still EOF" wrong-side claim, the from_call.rs:269 "SAFETY:" comment marking no unsafe).

C-21 [minor → convention] — write_header panics on short buffers

Convention drift: AGENTS.md §2 (no panics in library code). Verified: YES. src/channels/wire.rs:110-114write_header slices out[..CHUNK_HEADER_LEN], panicking on short buffers; the doc comment at :106-107 declares the panic. All current callers pass [0u8; 8], but the API is &mut [u8]. Same class: a potential panic path documented instead of typed. Fix: return a Result<_, ChunkError::HeaderTooShort> (the error variant already exists for the parse side, wire.rs:87-90).

C-22 [minor → convention] — Filler tests padding the count

Verified: YES. src/channels/client.rs:131-136 — a PhantomData "smoke test" (let _ = std::marker::PhantomData::<ChannelClient>;). src/channels/env.rs:155assert!(env.contains("anything") || !env.contains("anything")) tautology. These pad the "66 new channels tests" / "23 new Pub tests" counts claimed in the commit messages.

C-23 [minor] — dispatch_requested Sink/Stream errors use empty request id

Verified: YES. dispatch_requested's Sink error uses String::new() as request id (src/protocol/dispatch.rs:244-247), producing an envelope with an empty id (shared flaw with the Stream arm at :236-243).

C-24 [minor] — Producer/consumer naming leaked into docs

Convention drift: AGENTS.md §8 (producer/consumer, not server/client). Verified: YES, minor. Prod code is largely fine; "client→server streaming" leaked from ADR-046 into docs/architecture/call-protocol.md:335 and the ADR-046 reference table at :584. No public API names offend; only test-local duplex variable names use client/server (mux.rs:217, 253, etc.) — acceptable.

C-25 — Test coverage gaps (channels)

Verified: YES — present coverage is unit-level only; no end-to-end test wires two sides together. Missing:

  1. Channel 0 end-to-end (ADR-036): adapter ↔ client, one call_open_op round-trip (would catch C-01 immediately).
  2. Demux resync on TooLarge (channels-wire.md §MAX_CHUNK_LEN): send oversized chunk + valid chunk; assert the valid one survives (C-07).
  3. Backpressure semantics (ADR-040/REQ-CH-05): slow reader on channel A, fast writer, assert no data loss and channel B unaffected (C-04, C-05, C-16).
  4. Policy decrement on connection drop / handler exit (ADR-047 §7): open at cap, drop transport, reopen (C-06).
  5. Per-connection max_channels via open_channel — no test fills 256 and asserts TooManyChannels (only Display is tested, manager.rs:446-452); no concurrent-open race test (C-18).
  6. REQ-CH-01 through the mux to the wire on drop-without-shutdown — mux_handle_register_and_write_round_trips_to_transport covers explicit shutdown only; the drop-with-full-buffer path is untested (C-17).
  7. REQ-CH-06 — untested (and unimplemented, C-13).
  8. channel/close / channel/control / resources/subscribe handler invocation — the handler bodies (make_close_handler etc.) have zero tests; only spec-shape and registration are tested (operations.rs:329-364).
  9. resolve_channel_manager — untested (it's a stub; a test asserting the ADR-047 §4 behavior would have flagged it, C-03).
  10. ID wrap → 0 — untested (C-19).
  11. Marker wire format: well tested ✓ (discovery.rs:807-839, from_call.rs:592-653, spec.rs:338-365).

C-26 — Spec docs internally inconsistent post-047

Verified: YES. Only channel-operations.md was updated. The other channels spec docs still describe the pre-047 model that ADR-047 removed, and carry stale alknet ADR numbers (071/075/076/093/094/079/ 080) that point to nonexistent files after the renumbering (alkcall decisions only go up to 047):

  • docs/architecture/channel-client.md:54-59, 94-98 still specifies open_channel(alpn, params, direction), ChannelDirection, and ResourceEntry.access — all removed by ADR-047.
  • docs/architecture/channels-wire.md:241 lifecycle table still says "channel/open call operation… responder allocates."
  • docs/architecture/channels-adapter.md:142-194 still describes the generic channel/open handler with channel:unknown_alpn.
  • Stale ADR-number references: channel-client.md:170-171, channels-wire.md:273-274, channels-adapter.md:294-299.

Cross-cutting: what is solid (verified)

These were checked and are correct/close to spec — listed so the remediation plan can focus on what's actually broken:

  • Wire formats: EventEnvelope shape and the five+one event types match ADR-014/046 exactly; the channels 8-byte chunk header [channel_id:u32 BE][length:u32 BE] matches ADR-034/035 exactly (src/channels/wire.rs:85-114). No stream_type anywhere in the channels layer (verified by grep). ✓
  • HandlerKind::Sink validation: Pub↔Sink both directions (src/registry/registration.rs:108-124, 371-411), make_sink_handler (:587-593), with_local_sink/with_leaf_sink/ with_leaf_sink_provenance (:507-552). ✓
  • Wire op_type strings: "query"/"mutation"/"sub"/"pub" emit (src/registry/discovery.rs:163-169) and parse with "subscription" correctly rejected (src/client/from_call.rs:316-326). ✓
  • call.published payload { "input": <chunk> }: src/protocol/wire.rs:54-56, correlated by envelope id. ✓
  • Responder-side deadline: None for Pub: dispatch.rs:310 (implemented; untested — P-12).
  • invoke() / invoke_streaming() on Pub → INVALID_OPERATION_TYPE: registration.rs:193-197, 262-271. ✓
  • Gap F (channel_open marker): spec_to_json emits "channel_open": true only when set (discovery.rs:213-215), schema documents it as optional (:145-148), rebuild_spec_for parses it and ignores false/non-channels names (from_call.rs:263-277). ChannelOpenSpec { alpn: &'static str } matches ADR-047 §2 (spec.rs:38-40). Well tested. ✓
  • Per-identity cap default 256: policy.rs:156, default_policy(), PerIdentityChannelPolicy::new(256) tested at policy.rs:252-259. ✓
  • Channel 0 pre-installed, skips the ledger: correct (manager.rs:234-237), ALPN recorded as alknet/call. ✓ (Functionally dead per C-01, but the bookkeeping is right.)
  • Connection-owner allocation mechanism: ChannelManager::open_channel allocating via next_id.fetch_add (manager.rs:167) — "whoever holds the manager allocates" ✓ — but nothing invokes it in the Sub flow (no open-op wrapper, C-02) and the Pub-case flow doesn't exist.
  • Layering held: no channels types leaked into the call crate, OperationEnv stayed a trait, auth stays on the one AccessControl path. ✓
  • Hub relay (ADR-042), broker (Gap B), from_call relay wrapper (Gap C): intentionally out of scope per ADR-047 — correctly not flagged as bugs. ADR-047 Gap C expects alkcall's from_call to reconstruct the marker so the consumer can branch — that half is implemented ✓.
  • No unwrap()/expect()/panics in the new library code outside the write_header short-buffer path (C-21) and the channels deliberation comments. No blocking I/O on async paths. Poisoning N/A (parking_lot). ✓

Remediation plan

The plan is split into six units of work, ordered by dependency and severity. Each unit is a independently shippable commit (or small sequence) with its own acceptance gate. The units are sized so that none of the critical bugs is blocked behind a spec decision except where noted (C-03, C-08). The overarching acceptance gate for the whole effort is the single end-to-end test both reviews identify as missing:

One end-to-end test: ChannelClientChannelsAdapter, open a channel via a registered op, move bytes, tear down, verify quota.

That test is the gate because its absence is what let both commits land green. Each unit below either moves toward it or removes a defect it would surface.

Unit 1 — Fix Pub end-to-end (P-01, P-02, P-05, P-08, P-12 #1)

Goal: a publish() that actually works on any transport. Files: src/protocol/connection.rs, src/protocol/dispatch.rs. Scope:

  • P-01: keep stream A's write half; pump call.published + call.completed on it (do not re-open open_bi in write_publish_chunks).
  • P-02: change publish/publish_with_payload to take Pin<Box<dyn Stream<Item = Value> + Send>> (ADR-046 §8 shape). Hold stream A's write half and pump the stream.
  • P-05: register the publish like subscribe (timeout: None), not with DEFAULT_CALL_TIMEOUT. Falls out of the streaming rework.
  • P-08: route the dispatch Pub branch through OperationRegistry::invoke_sink instead of re-implementing it inline (removes the divergence hazard; the invoke_sink path becomes the wire path).
  • P-12 #1: add the end-to-end publish test (≥2 chunks, assert the responder sees them and returns the right result). This is the acceptance gate for the unit.

Note: P-02 changes a public API (publish signature) — but per ADR-046 §8 the Stream shape is the spec, so the current Vec<Value> was the drift. This is correcting drift, not introducing a wire break (the wire format is unchanged). No deployments exist.

Unit 2 — Channel 0 single-stream call mode, both sides (C-01, C-25 #1)

Goal: channel 0 actually carries call traffic in both directions. Files: src/protocol/connection.rs, src/protocol/dispatch.rs, src/channels/source.rs, src/channels/client.rs, src/channels/adapter.rs. Scope: This is the real design gap, not a one-line fix. The call protocol's stream-per-request model is incompatible with channel 0's single yield-once BiStream. Options:

  • (a) A single-stream call mode: multiplex all requests/responses on one framed stream (length-prefixed EventEnvelopes on channel 0's one BiStream). Both CallConnection (client) and Dispatcher (accept side) gain a "single-stream mode" that reads/writes frames on the one BiStream instead of opening/accepting one stream per request. ChannelBidiStreamSource::open_bi stops being called.
  • (b) Make ChannelBidiStreamSource yield a fresh BiStream per accept_bi/open_bi by framing sub-streams over the one channel — more complex, probably wrong. Recommend (a). This unblocks C-01's connect side (every call_open_op no longer fails) and accept side (channel 0's Connection is actually driven by a call dispatch loop). Acceptance gate: C-25 #1 — one end-to-end channel-0 round-trip test. May require a short ADR or ADR-036 amendment recording the single-stream-mode decision (it's a two-way-door implementation detail per ADR-046/036, but worth pinning).

Unit 3 — register_openable + decide the §4 env-resolution question (C-02, C-03)

Goal: a channel-open op can actually be registered and invoked. Files: src/channels/operations.rs, src/channels/env.rs, src/registry/env.rs (possibly, if as_any() is added). Scope: This unit requires a spec decision first:

  • Option A (implement §4 as written): add as_any() (or equivalent downcast machinery) to OperationEnv, implement resolve_channel_manager for real, and have the open-op wrapper resolve the per-connection manager at invocation time.
  • Option B (amend ADR-047 §4): bless the "manager passed at registration via ChannelCore" shape that the stub's comment describes. This drops per-connection resolution but is simpler; it works only if a ChannelCore is registered per-connection (an overlay concern), which interacts with ADR-019's layering. Recommend Option A (it preserves the ADR's stated design and the session/connection overlay patterns from ADR-024). Whichever is chosen, amend or affirm ADR-047 §4 in writing before coding. Then:
  • Implement ChannelCore::register_openable(spec, open_handler, channel_core) wrapping the ALPN open handler with the ACL→check_open→allocate→ledger→spawn→respond flow (src/channels/operations.rs:280-311).
  • Map channel:forbidden / channel:no_channels_session / channel:allocation_failed / channel:too_many_channels to CallError.
  • Fix the broken rustdoc links (C-09's register_openable ×2) once the method exists.
  • Test: register a no-op open op via register_openable, invoke it end-to-end through channel 0 (depends on Unit 2), assert channel_id is returned and quota is reserved.

Unit 4 — Backpressure fixes (C-04, C-05, C-07, C-16, C-17, C-25 #2 #3 #6)

Goal: the channels layer is lossless and bounded as ADR-040 promises. Files: src/channels/manager.rs, src/channels/reassembly.rs, src/channels/adapter.rs, src/channels/mux.rs. Scope:

  • C-04: make route_payload async (or use blocking_send/reserve), await on full instead of dropping. The demux loop calls it on the read path — make the demux loop await it.
  • C-05: replace the message-count buffer cap with a byte-budgeted cap (track accumulated payload.len() against the cap). Update DEFAULT_BUFFER_CAP's doc comment to match reality.
  • C-07: on ChunkError::TooLarge, read and discard length bytes before continuing. The parsed length is in the error variant.
  • C-16: replace the wake_by_ref + Pending busy-loop with a stored reserve() future (or Poll-friendly ready notification). Remove the ~50 lines of abandoned deliberation while here (C-20 overlap).
  • C-17: the mux pump must write an EOF chunk when its receiver ends without a sentinel (treat recv → None as implicit EOF). Fix the wrong-side doc comments in reassembly.rs:283-291, 295-309 and the misleading mux.rs:109-112 claim.
  • C-25 #2 #3 #6: add the demux-resync test, the backpressure test, and the drop-without-shutdown EOF test.

Unit 5 — Ledger decrement on all teardown paths + channel-id adoption/collision decision (C-06, C-08, C-12, C-13, C-18)

Goal: quota accounting is correct and channel ids don't collide. Files: src/channels/adapter.rs, src/channels/manager.rs, src/channels/operations.rs, src/channels/mux.rs. Scope:

  • C-06: wire on_close into the three missing teardown paths: close-sent locally, handler-exit, connection-drop. On transport EOF, clear_all() already returns the (channel_id, opener) list — call policy.on_close(&opener) for each (the adapter needs a hook to the policy; this is the architectural gap Sub Review B flagged — add a DecrementOnDrop hook or have the adapter hold a ChannelCore).
  • C-08: spec decision needed — odd/even id split, or an adoption op so the non-allocating side can install routing for a remotely-assigned id. Recommend odd/even (initiator allocates odd, responder even, or vice versa) as the smaller change; record the decision in ADR-047 §5 or a follow-on ADR. Then implement the adoption path: the non-allocating side registers the remotely-assigned id in its manager/mux.
  • C-12: reject channel_id: 0 in the channel/close handler.
  • C-13: channel/close must drain (signal EOF to the handler, await pump completion) before abort/decrement; implement REQ-CH-06 ordering (the close handler observes data-pump completion).
  • C-18: hold the channels lock across the max_channels check + insert in open_channel (re-check on re-acquire after the mux.register await).
  • Test: C-25 #4 #5 — open-at-cap-then-drop-transport-then-reopen, and the concurrent-open race test.

Goal: conventions satisfied, docs consistent, the remaining decided-but-unimplemented behaviors either implemented or explicitly deferred with an OQ. Scope: This is the largest unit by item count but the lowest risk; do it last, after the integration is end-to-end functional. Items:

  • C-20: remove the ~50 lines of abandoned deliberation in reassembly.rs:205-255 and the pervasive inline // comments across the channels module that aren't carrying a non-obvious correctness constraint. Fix the actively-wrong comments (mux EOF claim, abort-cancels claim, wrong-side peer-EOF claim, "SAFETY:" comment marking no unsafe).
  • C-09: fix the remaining cargo doc warnings (default_policy link, env is both a module and a macro).
  • C-26: update channel-client.md, channels-wire.md, channels-adapter.md to the post-047 model (remove open_channel(..., direction), ChannelDirection, ResourceEntry.access, the generic channel/open handler, channel:unknown_alpn; renumber stale alknet ADR refs to alkcall ADR-001..047).
  • C-21: make write_header return Result<_, ChunkError> instead of panicking.
  • C-22: remove the filler tests (client.rs:131-136, env.rs:155) or replace them with real assertions.
  • C-23: give dispatch_requested's Sink/Stream error envelopes a real request id (or document why empty is correct).
  • C-24: replace "client→server streaming" in call-protocol.md:335 with producer/consumer phrasing; amend ADR-046 §3 to match §6 (P-11).
  • C-19 misc: fix next_id wrap, the unbounded resources-snapshot loop, the mux pump map leak / duplicate registration, the wrong ManagerError mapping on mux register failure, the misleading drop(state.demux_sender.clone()), the derive_alpn_from_op_name multi-segment bug + dead branch, the Box::leak Arc<str> refactor, the policy.rs:127-140 check-and-reserve rollback trap (document or add a rollback API), the Demux::stats() error counter (REQ-CH-04).
  • P-03: implement publish_schema per-chunk validation using alktype (the dependency is already declared and unused), and wire it through spec_to_json / operation_spec_schema() / rebuild_spec_for so imported Pub ops keep the schema. Or, if deferred, file an OQ and remove the misleading doc comment at spec.rs:181-186.
  • P-04: make pump_sink match call.error and inject the initiator's CallError as an Err item that terminates the stream (per ADR-046 §6).
  • P-06: implement abort-cancels-Pub. This likely needs the single-stream call mode from Unit 2 (so an abort on the same stream can reach the running pump) plus a mechanism to drop the handler future on abort (the doc comments at dispatch.rs:494-498 describe the target shape). Decide whether to fix the cross-stream path (abort() opening a new stream) or deprecate it in favor of same-stream abort.
  • P-07: fix the handler-returns-early stall — when the handler returns, the feed should be short-circuited and the response written immediately (don't join! to completion). Use tokio::select! or feed the handler's completion back into the feed loop.
  • P-09: fix make_sink_forwarding_handler to stream (not collect), and to terminate on Err (not filter_map it away). Depends on P-01/P-02.
  • P-10: use the EVENT_* wire constants in pump_sink instead of string literals.
  • P-13: opaque-wrap SinkDispatch::chunk_tx before crates.io.
  • C-10 / C-11: either implement channel/control routing and live resources/subscribe, or file OQs and make the stubs honest (return unimplemented-style errors, not fake success).
  • C-14 / C-15: either implement the QUIC-native substrate and the full ChannelClient API (open_channel, Channel, subscribe_resources), or file OQs and remove the misleading doc comments / nonexistent method references.

Suggested sequencing

Unit 1 (Pub end-to-end)        → unblocks the Pub flagship use cases
Unit 2 (channel 0 single-stream) → unblocks all channel-0 integration tests
Unit 3 (register_openable)     → needs a §4 spec decision first; unblocks opening channels
Unit 4 (backpressure)          → independent; can run in parallel with 2/3
Unit 5 (ledger + adoption)     → needs a §5 spec decision first; depends on 3
Unit 6 (cleanup)               → last; after the integration is end-to-end functional

Units 1, 2, 4 can proceed immediately with no spec decisions. Units 3 and 5 each need one written decision (ADR-047 §4 affirm/amend; ADR-047 §5 odd/even-or-adoption). Unit 6 is the long tail.

On the baseline objective

The layering held, the wire formats are right, the architecture is right. What's missing is the connective tissue a tty/docker crate would actually call: a working publish(), a working channel 0, register_openable, channel adoption, and a streaming publish(). A tty extraction cannot start on this foundation yet — treat the end-to-end ChannelClientChannelsAdapter test (Unit 2's gate, then Unit 3's gate) as the acceptance gate for the next round.


Verification log (this consolidation pass)

Findings re-verified directly in source on 2026-08-12 against tree f305f8c. Every file:line reference above was checked. Findings that did not survive verification or were overstated in the original reviews, corrected here:

  • Sub Review B's "the pump does not write an EOF chunk" (C-17): overstated. The pump does write EOF when it receives the sentinel (mux.rs:124-133). The real bug is narrower: when the sentinel is lost (full buffer try_send), the pump exits on recv → None without writing EOF. C-17 above states the corrected mechanism.
  • Warning count (C-09): the main review said 5; this pass sees 4 cargo doc warnings (2× register_openable, 1× default_policy, 1× env module/macro). Documented as 4.
  • P-11: flagged in Sub Review A as drift; on verification this is an internal ADR-046 inconsistency (§3 vs §6) that the code resolved correctly and consistently. Reclassified from "drift" to "amend the ADR §3 text."

All other findings (P-01 through P-10, C-01 through C-08, C-12 through C-26) verified as stated.