Files
alkcall/docs/reviews/006-channel-open-establishment-gap-review.md
T
glm-5.3-flash 36e74cda11 feat(review 006 Unit 3): teardown-race log, early-arrival bound docs, count accessor (E-03, E-04, N-2)
- E-03: the open wrapper's handler-exit teardown no longer discards
  UnknownChannel silently — debug log + benign-race pinning comment
  (ledger take is the atomic gate; no double-decrement)
- E-04: consumer-facing doc note on the 64-parked-chunks observable
  bound (channel-client.md + EARLY_ARRIVAL_CAP const doc) for
  tunnel-style push-first producers
- N-2: ChannelManager::early_arrival_count() accessor (the
  observability choice over removing the write-only counter),
  documented monotonic, with a park/adopt-drain monotonicity test
- review 006: Unit 3 marked implemented in Status and remediation plan

Verification: 617 tests pass; clippy -D warnings clean (host +
wasm32 check); fmt clean; doc clean
2026-09-06 19:35:18 +00:00

30 KiB
Raw Blame History

Review 006 — Channel-Open Establishment Gap (from alktunnels Phase 0)

Status

Resolved-by-ADR (E-01, N-1) / Implemented (E-02, E-03, E-04, N-2). Findings filed from the alktunnels Phase 0 research pass (2026-09-06). This is a design review, not a code-defect review: the establishment gap (E-01) is real, POC-observable, and load-bearing for the next downstream crate; the remaining findings are smaller mechanism/coverage gaps noticed in the same sweep.

2026-09-06 verification + remediation pass. All four findings were independently re-verified against source at tree 88e3f5e (0.4.1 + this review's own commit) — verdicts CONFIRMED for E-01..E-04, with one correction to E-02's cost estimate (see the verification appendix at the bottom of this file). Three additional findings filed from the same sweep: N-1 (client error-type gap — blocking for E-01's consumer visibility), N-2 (dead counter), N-3 (panic posture, pinned). E-01 + N-1 are resolved by ADR-049 (docs/architecture/decisions/049-channel-open-establishment-phase.md — split-hook OpenEstablisher, bounded await, channel:open_failed typed error). Unit 1 (E-01 + N-1) is implemented in alkcall 0.5.0 (OpenEstablisher + register_openable_with_establisher, wrapper establishment phase with bounded await, channel:open_failed typed error, ChannelOpenError typed client error — all four verification gates landed as tests; two implementation-shape notes recorded in ADR-049's amendment: the establisher does not receive the channel Connection (yield-once BiStream belongs to the pump handler), and the bound is the earlier of dispatch deadline and per-registration timeout). Unit 2 (E-02) is implemented in alkcall 0.5.0 (OperationSpec.description: Option<String> + with_description, spec_to_json_pub emits it when set, rebuild_spec_for parses it (shared by from_call and op/register), services/list and services/list-peers local listings emit it when set; the ADR-047 §6 amendment below records the discovery decision). Unit 3 (E-03, E-04, N-2) is implemented in alkcall 0.5.0 (E-03: the wrapper's handler-exit teardown discards UnknownChannel through a debug log + a benign-race pinning comment; E-04: the 64-parked-chunks observable bound is documented consumer-side in channel-client.md § "The early-arrival park bound (push-first producers)" and on the EARLY_ARRIVAL_CAP const; N-2: early_arrival_count is exposed via ChannelManager::early_arrival_count() — the observability choice, paired with dropped_unknown_chunks — with a monotonicity test). The original remediation sketch below is superseded by the "Remediation plan (post-verification)" section; the original sketch is retained for the record.

Findings continue the review numbering with prefix E (001–005 used P/C/R/A/B/C/D/F/G — each review numbers independently).

Scope

The channels open-op path (run_open_wrapper and the OpenHandler contract) reviewed from the perspective of the next consumer crate (alktunnels — arbitrary TCP/UDP tunnels over channels), cross-checked against the two existing consumers (alktty, alkhttp) and the SSH forwarding prior art recorded in /workspace/@alkdev/alktunnels/docs/research/ssh-socks5-survey.md.

Everything below was verified directly in source at tree a22b2b8 (0.4.1 + the early-arrival park fix). No code changes were made in this repo by this review. The 2026-09-06 re-verification pass (verification appendix) re-traced all findings at tree 88e3f5e — no source changes touched the reviewed paths between the two trees (the only delta is this review's own commit).

Verified against: alkcall a22b2b8 (0.4.1); re-verified at 88e3f5e
Reading list: src/channels/operations.rs (run_open_wrapper, OpenHandler,
  make_open_handler_once/stream/sink), src/channels/manager.rs
  (open_channel, teardown_channel, route_payload, early-arrival park),
  src/channels/reassembly.rs (MpscSendStream::poll_shutdown, Drop,
  MpscRecvStream::poll_read EOF arms), src/channels/mux.rs (implicit-EOF
  pump path), src/protocol/wire.rs (CallError), src/registry/
  registration.rs (invoke/invoke_streaming gates), src/registry/
  discovery.rs (services/list, spec_to_json_pub), src/client/from_call.rs
  (rebuild_spec_for), docs/architecture/decisions/047,
  docs/architecture/channel-operations.md

Severity legend

Same scale as reviews 004/005:

  • [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 — The open-op establishment gap

E-01 [major] — The open op cannot fail after allocation: the consumer receives a live channel for a tunnel whose establishment failed, with no error channel

Verified: YES, by code trace and mechanism analysis.

The mechanics

OpenHandler (ADR-047 §3) is the ALPN crate's hook for "validate params, consult ownership, prepare the backend" — i.e., the establishment phase of a channel. But the wrapper does not await it:

  1. run_open_wrapper (src/channels/operations.rs:483-528): policy.check_open → manager.open_channel(alpn, opener_id, None) → build the channel Connection → open_handler(input, channel_conn, auth) spawns the handler and collects its JoinHandle → ResponseEnvelope::ok(request_id, json!({ "channel_id": channel_id })) (operations.rs:503-527, 537). The reply is written to the wire the moment the handler task is spawned, not when the handler has done its establishment work.
  2. The OpenHandler type (operations.rs:333-334) returns tokio::task::JoinHandle<()> — there is no result, no error variant, no establishment phase the wrapper can consult. Any failure inside the handler (params valid at the schema level but semantically rejected, backend lookup failure, target dial failure for a direct-tcpip-shaped tunnel, resource no longer available) is invisible to the open op's reply.
  3. What the consumer observes on handler-side failure: the call op succeeds with {channel_id}, the channel is adopted (ChannelClient::open_channel, src/channels/client.rs:227-250), and then the channel EOFs — the handler wrote nothing before exiting, the mux pump writes the implicit-EOF chunk on receiver end (src/channels/mux.rs:78-81, REQ-CH-01 implicit-EOF path), and MpscRecvStream::poll_read returns clean EOF for both the sentinel and sender-drop arms (src/channels/reassembly.rs:111-135). A dial-failure EOF is byte-for-byte indistinguishable from a target that closed immediately after connecting — the two most different failure/success stories map to the same consumer-visible event.

Why this is the wrong shape (SSH's semantics, prior-art checked)

Every established tunnel/forwarding protocol puts establishment failure in the open reply, not in the data stream:

  • SSH (RFC 4254 §5.1): SSH_MSG_CHANNEL_OPEN_FAILURE is a first-class reply to the open, carrying a reason code (ADMINISTRATIVELY_PROHIBITED / CONNECT_FAILED / UNKNOWN_CHANNEL_TYPE / RESOURCE_SHORTAGE) plus a description string; the channel never exists on the opener's side afterward (verified end-to-end in russh: src/server/encrypted.rs:1244-1278, src/client/encrypted.rs:414-434 — see /workspace/@alkdev/alktunnels/docs/research/ssh-socks5-survey.md §"Open-failure path").
  • SOCKS5 (RFC 1928 §6): the reply carries REP codes 0x01–0x08 and the connection closes within 10s on failure — error-then-close, never "success then in-stream error."
  • udpgw (tun2proxy/src/udpgw.rs:21-26) is the counterexample: an opaque ERR bit with zero reason information — the survey flags it as the vocabulary to avoid.
  • alktty was forced to reinvent the missing mechanism in-band. The direct-ALPN path answers the negotiation with a length-prefixed JSON error frame on the stream (send_negotiation_error, alktty/src/adapter.rs:177-187; the 0x00-prefix first-byte disambiguation trick, alktty/docs/architecture/tty-adapter.md:186-197), and the channels path retains the same error-frame shape even though the open op's input is the negotiation (alktty ADR-009) — because post-allocation failures have nowhere better to go. That is per-crate reinvention of a protocol-level capability every ALPN crate will need: a structured, typed, establishment-failure reply to the open op.

The cost today, concretely

  • A consumer cannot distinguish "ACL denied" (call error, channel:forbidden — never allocated) from "dial refused" (open succeeded, instant EOF) from "target accepted then instantly closed" (open succeeded, instant EOF). Retry policy, client UX, and error reporting are impossible on the second and third.
  • ADR-016's typed error details (CallError.details) — which the wrapper already uses for channel:too_many_channels with {count, max} details (operations.rs:530-545) — cannot carry dial-failure reasons.
  • The alktty in-band error-frame path exists only because the wrapper can't fail the open post-allocation; every future ALPN crate faces the same fork: reinvent an in-band error vocabulary or silently-EOF.

The ask (proposed shape, for the ADR — not a prescriptive API)

Give the open-op wrapper an establishment phase it awaits before replying. Minimal, backward-compatible shape:

  1. OpenHandler gains an establishment result. Two candidate shapes:
    • Split the hook: OpenEstablisher (async, awaited by the wrapper — validates params semantically, prepares/dials the backend, returns Result<Establishment, EstablishmentError>) followed by OpenHandler (spawned on the established channel, as today). The dial is the natural establisher step for tunnels; the pumps remain the spawned handler.
    • Await-and-inspect: keep the single OpenHandler returning JoinHandle<OpenResult>; the wrapper awaits a bounded establishment phase (a JoinHandle::timeout equivalent — select on the handle vs an establishment deadline) before replying. The handler signals "established, continue" via an agreed value (e.g. the handler resolves a first Result<(), HandlerError> promptly, or the wrapper watches a oneshot the handler signals).
  2. On establishment failure: the wrapper tears down the just-allocated channel (teardown_channel — the ledger/policy paths already handle this atomically) and replies with a new typed CallError, e.g. channel:open_failed, with details carrying a reason code + message. Reason-code vocabulary per the SSH four (the survey's finding): policy-denied (already distinct — channel:forbidden), dial-failed, unknown-resource-or-substrate, resource-shortage — mapping 1:1 onto what an open handler can actually produce. Wire addition is additive (new error code string + optional details shape), no existing consumer breaks.
  3. Backward compatibility: the existing OpenHandler signature is preserved if the split shape is chosen (old handlers still compile — the establisher is a new, separately-registered hook, defaulting to an always-OK establisher for the no-establishment-work case).

Alternative considered and rejected

An in-band establishment/error frame (alktty-style, on the channel stream, alktunnels' original OQ-TN-09 direction) works without an upstream change, but: (a) it forces every ALPN crate to define a frame vocabulary and a disambiguation scheme (alktty's 0x00 peek is exactly this cost, paid once per crate); (b) it cannot carry typed CallError.details or participate in ADR-016 error schemas; (c) it leaves the phantom-opened channel in the manager (allocation/ledger/ policy all fire for a channel that never carried data); (d) SSH's semantics — the failure is the open's reply, the channel never exists opener-side — are the cleaner contract, and channels is at exactly the maturity point (three downstream dependents, alkcall 0.4.x) to fix it upstream cheaply.

Severity

[major] — not [critical]: no data corruption, and the capability is reachable via per-crate workarounds (alktty proves it). But it is the shape every future protocol crate will fight, the fix is cheaper now than after the next consumer, and the workaround path (in-band frames) bakes in a wire format that would then need to stay stable per crate.


Part B — Smaller findings from the same sweep

E-02 [minor] — services/list discloses no per-op metadata; openable resources are indistinguishable by name alone

services_list_handler (src/registry/discovery.rs:247-269) maps list_operations() to {name, namespace, op_type} only. For a tunnel registry, the consumer's discovery question is "which tunnel resources may I open" — and the answer arrives as a bare list of op names (channels/tunnel/sub...). Per-resource identity (which target, which substrate, human description) has no field to live in:

  • services/schema (spec_to_json_pub, discovery.rs:211-236) can disclose it per op (input schema, access_control), so the data path exists — but it is N+1 round-trips, and the input schema describes the open params contract, not the set of produced resources (a tunnel producer registers one op and N resources).
  • ADR-047 §6 already anticipated the dynamic half: channel/resources/subscribe aggregates per-ALPN resource enumerators — but the handler is a not-implemented stub (channel:resources_not_implemented, OQ-40, operations.rs:274-299).

This is not an alkcall defect — OQ-40 is decided-deferred and the stub fails loudly by design. Filing it because alktunnels' discovery need (OQ-TN-08 resolution: "the ops listing IS tunnel-resource discovery") makes OQ-40 load-bearing for the first time: a consumer UI cannot distinguish produced tunnel resources without either the enumerator aggregation or a listing enrichment. Recommend deciding (small ADR or OQ-40 update) whether:

  1. channel/resources/subscribe lands (per-ALPN enumerators — the decided shape), or
  2. services/list gains an additive per-op description/metadata field (static, registry-side — cheap, but describes the op, not the resource set), or
  3. Both: subscribe for live resource sets, list enriched for op descriptions.

alktunnels Phase 1 can proceed with (3)'s shape assumed; the minimal v1 consumer can also just know the op name out-of-band (config), which is why this is [minor] today.

E-03 [minor] — OpenHandler-exit teardown races channel/close's awaited teardown, but the ledger take makes it benign (verify + document)

run_open_wrapper's spawned teardown task (operations.rs:505-518) and the channel/close handler's teardown path (make_close_handler, operations.rs:170-238 — 5s await on the handler task, then ledger take + on_close) both walk opener_ledger().take(id) → policy.on_close(opener). The take is atomic (first caller removes the entry), so no double-decrement — the cap cannot drift upward. But the loser of the race:

  • The wrapper task's teardown_channel(id) returns Err(UnknownChannel) (logged? no — let _ = discard, operations.rs:509) and skips the ledger/policy half silently.
  • The close handler's Ok(task) arm then awaits a task that is already exiting.

Verified benign for the cap invariant (take is the gate — operations.rs:510, operations.rs:210), but the let _ = discard at operations.rs:509 means a real UnknownChannel after a set_handler_task failure (operations.rs:520-527 — the "channel vanished between open and handler-task install" warn) is indistinguishable from a benign race. Recommend either a debug log on the discard or a comment pinning the race as designed. No correctness impact; filing for the record since it was checked during the E-01 trace.

E-04 [minor] — Early-arrival park cap (64) interacts with push-first producers under slow adopters — cap-drop is silent at the consumer

The early-arrival buffer (EARLY_ARRIVAL_CAP = 64, manager.rs:111, park_early_arrival manager.rs:440-462) is per-channel and drops past the cap with a debug log + counter — correct per the adopt-race design. But for a tunnel producer whose handler dials then immediately pumps (the common case), 64 chunks can arrive before the consumer's adopt_channel runs if the open-op response is delayed (relay hops, scheduling). The drops are counted (dropped_unknown_chunks) but not visible to the channel's consumer — data loss presents as a truncated stream with clean framing elsewhere. Not a correctness bug (the cap is the documented behavior; the fix shipped in a22b2b8 is the right shape), but worth a note in the tunnel-crate's POC checklist: the UDP POC's MTU-vs-buffer sizing should treat 64 parked chunks as the observable bound. No alkcall change requested; filed so the constraint is visible to the next consumer.

Non-findings (verified correct, recorded to bound the re-review)

  • The open-op ACL path is complete: invoke_streaming runs the same visibility + AccessControl::check + input_schema gates as invoke (registration.rs:380-404 vs :347+); the Sub-typed open op's ACL failure is a call.error on the open, no channel allocated. Verified against the invoke_streaming_acl_denied_yields_ forbidden test (registration.rs:1628).
  • input_schema enforcement covers open params (0.4.0, the alktty-review-L1 fix): check_input_schema runs before the handler in all three dispatch entry points; schema-invalid open params are INVALID_INPUT call errors — never a phantom channel. The semantic-beyond-schema gap is E-01's subject and is properly post-schema.
  • EOF arms are unified and clean: both the sentinel (Bytes::new) and sender-drop arms of MpscRecvStream::poll_read yield clean EOF (reassembly.rs:111-135); the mux pump writes the implicit-EOF chunk for handler-drop-without-shutdown (mux.rs:78-81 + mux_pump_writes_eof_on_implicit_close test). The two-pump shutdown contract (alknet ADR-078) has its upstream half in place.
  • The channel_open marker survives the wire round-trip (spec_to_json_pub emits the boolean; rebuild_spec_for re-derives the ALPN from the op name — from_call.rs:265-280, :298-314), including the custom/proto multi-segment case. The G-04 resource_id_path fix is present (from_call.rs:260, round-trip test from_call.rs:610-617).
  • The opener-ledger cap decrement is atomic with removal (opener_ledger().take gates every on_close path); the double-decrement concern from ADR-047 §7 is closed at every call site traced.

Remediation sketch (original, superseded — see "Remediation plan (post-verification)")

Unit 1 — E-01 (the establishment phase). Decided-shape ADR first (this is ADR-047 §3 contract territory — the OpenHandler type shape is a cross-crate API surface, and both existing consumers' handlers must be considered; alktty's TtyOpenHandler is the migration prototype). Implementation sketch: ChannelCore::register_openable gains an optional establisher hook; run_open_wrapper awaits it bounded (establishment deadline — a constant, e.g. 10s, or per-spec), tears down on failure, replies channel:open_failed with {reason, message} details on failure and {channel_id} on success. alktty migrates its channels-path error-frame to the call-error path where applicable (its direct-ALPN path keeps the in-band frame — two transports, two contracts). alkhttp unaffected (no openable ops).

Unit 2 — E-02 (discovery enrichment). Decide via OQ-40 update + small ADR amendment: recommend (3) — subscribe for live resource sets (the decided shape, now load-bearing), plus an additive description field on the listing (cheap, immediately useful). Implemented 2026-09-06 (alkcall 0.5.0): the listing half landed (OperationSpec.description, four touchpoints as the appendix corrected); the subscribe half stays deferred (OQ-40, now with the load-bearing note).

Unit 3 — E-03/E-04. E-03: log-or-comment; E-04: doc note. Trivial.

Remediation plan (post-verification)

Firm ordering, decided 2026-09-06. E-01's ADR is ADR-049 (docs/architecture/decisions/049-channel-open-establishment-phase.md) — decided shape: the split hook (OpenEstablisher awaited bounded by the wrapper, OpenHandler spawned unchanged after success), chosen over await-and-inspect because it preserves the OpenHandler type (backward compatible by construction), keeps establishment and pump lifecycles separate (SSH semantics: the dial is synchronous with the open reply), and restores ADR-047 §3's original "channel plan" wrapper shape. N-1 rides the same unit — without the client error-type fix, channel:open_failed's typed reason is unreachable through the primary client path.

Unit 1 — E-01 + N-1 (alkcall 0.5.0). OpenEstablisher + register_openable_with_establisher (no-establisher = always-OK, existing registrations compile unchanged); wrapper flow: check_open → open_channel → await establisher bounded (dispatch deadline when Some, else ESTABLISHMENT_TIMEOUT = 10s) → on failure: teardown_channel + ledger take + policy.on_close + reply channel:open_failed with details: {reason, message} (reason ∈ dial_failed / unknown_resource / resource_shortage / handler_error / timeout); on success: spawn pumps, set_handler_task, reply {channel_id}. ChannelClient::open_channel returns a typed error carrying the CallError. Concurrency is safe: the dispatcher spawns Once invocations as independent tasks (dispatch.rs spawn_once_dispatch), so an awaited establisher does not head-of-line-block channel 0. Version 0.5.0 (the open_channel error-type change is semver-relevant). Status: IMPLEMENTED (2026-09-06) — with two implementation-shape notes recorded in ADR-049's amendment: the establisher takes (input, auth) only (the yield-once channel BiStream belongs exclusively to the pump handler), and the effective bound is min(dispatch deadline, registration timeout | ESTABLISHMENT_TIMEOUT). All four verification gates below are tests in the crate.

Unit 2 — E-02 (ride the 0.5.0 release). Additive description: Option<String> on OperationSpec — note this is four touchpoints, not one (see the verification appendix's E-02 correction): struct field + spec_to_json_pub emit + rebuild_spec_for parse + the services/list output-schema doc. services/list emits it when set. OQ-40 stays deferred but gains a "load-bearing for alktunnels discovery UI" note; the channel/resources/subscribe half stays deferred (alktunnels v1 uses config-known op names). Status: IMPLEMENTED (2026-09-06) — the discovery decision (3: enriched listing now, subscribe deferred) is recorded as an amendment to ADR-047 §6.

Unit 3 — E-03/E-04/N-2 (trivial batch, same PR series as Unit 1). E-03: debug log (or pinning comment) on the discarded UnknownChannel at the wrapper's teardown discard. E-04: doc note on EARLY_ARRIVAL_CAP (the 64-parked-chunks observable bound) for tunnel-crate-facing consumers. N-2: expose an accessor for early_arrival_count or remove the write-only counter. Status: IMPLEMENTED (2026-09-06) — E-03: debug log + benign-race pinning comment on the wrapper's handler-exit teardown (mirroring the sibling log the Unit 1 establisher-failure teardown already carries); E-04: consumer-side doc note in channel-client.md plus the const doc; N-2: accessor (ChannelManager::early_arrival_count(), documented as monotonic — adopt-drain does not decrement) + test.

Sequencing: ADR-049 (landed) → alkcall 0.5.0 (Units 1+2+3) → alktty migration (channels-path semantic failures move into an establisher; the direct-ALPN in-band error frame is retained — two transports, two contracts) → alkhttp mechanical pass (OpenableAlpn.establisher: Option<...>, default None). alktunnels Phase 1 blocks only on the ADR decision (landed), not the implementation: its OQ-TN-09 direction ("self-contained control frame") shrinks to post-establishment control only once channel:open_failed exists.

Verification gates for the E-01 remediation

  • A channels end-to-end test: producer handler whose establisher fails after channel allocation → consumer's open_channel resolves Err with channel:open_failed + reason details; no channel in the manager's channel_ids() afterward (the SSH "channel never exists opener-side" property, consumer-visible as "no channel_id was ever returned").
  • A bounded-establishment test: establisher that never completes → open op fails with a timeout-flavored error within the deadline, channel torn down, ledger decremented.
  • An alktty-migration test: the existing open_via_channels_surfaces_negotiation_rejected scenario resolves via call error (or retained error-frame — per the ADR's chosen compat shape) unchanged in behavior.
  • (Added post-verification) A compat gate: a no-establisher registration behaves exactly as today — {channel_id} reply, handler spawned, teardown on handler exit.

Verification appendix (2026-09-06 re-verification pass)

All findings re-verified by independent source trace at tree 88e3f5e. Verdicts, corrections, and the additional findings:

  • E-01 CONFIRMED — and strengthened. The wrapper's spawn-then-reply flow is as described (src/channels/operations.rs run_open_wrapper: handler spawned, reply written before the handler performs any work). New supporting evidence the original pass missed: ADR-047 §3's decision text describes the ALPN open handler as "validate params, consult ownership, prepare the backend, return a 'channel plan'" — an awaited preparation the wrapper consults before replying. The implemented OpenHandler (JoinHandle<()>) collapsed that phase into a fire-and-forget spawn. E-01 is therefore drift from ADR-047 §3's own wrapper shape, not merely a missing convenience — the remediation restores the decided design, which lowers the ADR's contention cost. Also verified: the serving loop spawns Once invocations as independent tasks, so an awaited establishment phase does not head-of-line-block other calls on channel 0 (a fix-feasibility question the original pass did not address).
  • E-02 CONFIRMED — one correction. services_list_handler emits {name, namespace, op_type} only, as described. But the "cheap, additive description field" framing understates the work: OperationSpec has no description field at all (the description in spec.rs is on ErrorDefinition). The change is four touchpoints: struct field, spec_to_json_pub emit, rebuild_spec_for parse, and the services/list output-schema doc. Still small; still [minor].
  • E-03 CONFIRMED. Both teardown paths gate on the atomic opener_ledger().take(id); the race loser gets UnknownChannel / take → None — no double-decrement. The let _ = discard on the wrapper's teardown_channel result is as described. Also traced: the set_handler_task failure path still runs the wrapper task to completion (self-teardown), so no leak in that arm either. The log-or-comment remedy stands.
  • E-04 CONFIRMED. EARLY_ARRIVAL_CAP = 64, per-channel, FIFO, drop-past-cap with debug log + dropped_unknown_chunks counter. Doc-note-only remedy stands.
  • N-1 [minor today, blocking for E-01's consumer visibility] — ChannelClient::open_channel erases the typed error. (src/channels/client.rs) It flattens the CallError into a String via format!("open op failed: {e:?}"). Even once the wrapper replies channel:open_failed with reason details, a consumer cannot branch on the reason — the error type destroys it. E-01's verification gate ("consumer's open_channel resolves Err with channel:open_failed + reason details") is unreachable without changing this error type, so the fix is in scope for the E-01 unit (ADR-049 §4), not deferred.
  • N-2 [trivial] — early_arrival_count is write-only. Incremented in park_early_arrival, never read anywhere (no accessor; only dropped_unknown_chunks is exposed). Either expose an accessor (observability for the E-04 bound) or remove the counter.
  • N-3 [observation, pinned by ADR-049 §6] — panicked pump handlers are EOF-shaped by design. The wrapper's teardown task swallows the pump handler's JoinError (let _ = raw_task.await). A panicked handler = phantom channel + instant EOF, indistinguishable from a clean short-lived channel. With the establisher split, the pump-phase panic stays in this category (correct — there is no mid-stream error channel by design; establishment errors are the only kind that belong in the open reply). ADR-049 §6 pins this posture; no change.

References

  • alknet ADR-078 / channels two-pump contract — the teardown half this review's EOF-arms non-finding confirms upstream.
  • alkcall ADR-047 (openable ALPNs are operations), ADR-016 (typed error schemas — the vehicle for channel:open_failed details), ADR-040/041 (backpressure/caps — untouched by E-01).
  • alktty ADR-009 (the open op's input is the negotiation) + review #001 L1/L3 resolution — the per-crate workaround E-01 obsoletes; send_negotiation_error (alktty/src/adapter.rs:177-187) and the 0x00-peek (alktty/docs/architecture/tty-adapter.md:186-197) are the in-band mechanism the upstream establisher replaces for the channels path.
  • alktunnels Phase 0 (docs/research/phase-0-findings.md OQ-TN-09) and the SSH/SOCKS5 survey (docs/research/ssh-socks5-survey.md §"Open-failure path", §"Comparison") — the prior art motivating E-01's reason-code vocabulary.
  • The consumer-findings ledger convention (docs/reviews/consumer- findings-ledger.md) — findings here follow the same spirit (downstream-discovered, filed for upstream action); E-01..E-04 are alktunnels-discovered but numbered in alkcall's review series since they are alkcall findings.
  • ADR-049 (docs/architecture/decisions/049-channel-open- establishment-phase.md) — the E-01/N-1 resolution (split-hook OpenEstablisher, bounded establishment, channel:open_failed typed error, client error-type fix).