From 48ceeba55cea1d336da98df90e977022aa7f1076 Mon Sep 17 00:00:00 2001 From: "glm-5.3-flash" Date: Sun, 6 Sep 2026 10:57:20 +0000 Subject: [PATCH] docs: ADR-049 channel-open establishment phase; verify review 006 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Verify review 006's findings against source at 88e3f5e (E-01..E-04 all confirmed; E-02 cost corrected — OperationSpec has no description field, four touchpoints) and file three additional findings from the same sweep (N-1 client error-type gap, N-2 write-only early-arrival counter, N-3 pump-panic posture). ADR-049 resolves E-01 + N-1: split-hook OpenEstablisher awaited bounded by the open-op wrapper (restoring ADR-047 §3's "channel plan" shape), teardown + typed channel:open_failed reply on establishment failure, ChannelClient::open_channel typed error. Review 006 gains the post-verification remediation plan and verdict appendix. Verification: cargo test (597 passed), cargo doc --no-deps clean. --- .../049-channel-open-establishment-phase.md | 316 ++++++++++++++++++ ...6-channel-open-establishment-gap-review.md | 154 ++++++++- 2 files changed, 465 insertions(+), 5 deletions(-) create mode 100644 docs/architecture/decisions/049-channel-open-establishment-phase.md diff --git a/docs/architecture/decisions/049-channel-open-establishment-phase.md b/docs/architecture/decisions/049-channel-open-establishment-phase.md new file mode 100644 index 0000000..a17712d --- /dev/null +++ b/docs/architecture/decisions/049-channel-open-establishment-phase.md @@ -0,0 +1,316 @@ +# ADR-049: Channel-Open Establishment Phase (`OpenEstablisher`) + +## Status + +Accepted (amends ADR-047 §3 — the open-op wrapper gains an awaited +establishment phase ahead of the spawned pump handler; resolves review +006 E-01 and N-1) + +## Context + +ADR-047 §3 made openable ALPNs operations: the ALPN crate supplies an +open handler, and the channels wrapper does the channel machinery +(allocation, ledger, policy, spawn). The §3 decision text describes the +handler's job as "validate params, consult ownership, prepare the +backend, return a 'channel plan'" — an awaited preparation step the +wrapper consults **before** replying. The implemented `OpenHandler` +type (`Arc JoinHandle<()>`) +collapsed that preparation into a fire-and-forget spawn: the wrapper +collects the `JoinHandle`, records it for teardown, and writes +`{ "channel_id": }` to the wire the moment the handler task is +*spawned* (`run_open_wrapper`, `src/channels/operations.rs`). The +establishment phase the ADR described never became a thing the wrapper +could consult. + +The consequence (review 006 E-01, verified at tree `88e3f5e`): the open +op **cannot fail after allocation**. Any establishment failure inside +the handler — params valid at the schema level but semantically +rejected, backend lookup failure, a target dial refused for a +`direct-tcpip`-shaped tunnel, a resource no longer available — is +invisible to the open reply. The consumer observes: the call op +succeeds with `{channel_id}`, the channel is adopted, and then the +channel EOFs (the handler exits without writing; the mux pump writes +the implicit-EOF chunk; `MpscRecvStream::poll_read` returns clean EOF +for both the sentinel and sender-drop arms). A dial failure 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. + +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 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. +- **SOCKS5** (RFC 1928 §6): the reply carries REP codes 0x01–0x08; + error-then-close, never "success then in-stream error." +- **udpgw** (tun2proxy) is the counterexample: an opaque ERR bit with + zero reason information — the vocabulary to avoid. + +The per-crate workaround proves the gap is load-bearing: alktty's +channels path answers establishment failures with a length-prefixed +JSON error frame **on the channel stream** +(`send_negotiation_error`, `alktty/src/adapter.rs`), disambiguated from +data by a `0x00` first-byte peek. That is a per-crate reinvention of a +protocol-level capability every ALPN crate will need: a structured, +typed, **establishment-failure reply to the open op**. It also forces +the phantom-opened channel to exist in the manager — allocation, ledger, +and policy all fire for a channel that never carries data. + +alktunnels (the next consumer, arbitrary TCP/UDP tunnels over channels) +hits this on day one: its producer dials the tunnel target inside the +open op, and dial failure is the *common* case, not the edge case +(review 006 E-01; alktunnels OQ-TN-09). + +This is the cheapest moment to fix upstream: three downstream crates +(alktty, alkhttp, alktunnels-in-progress), alkcall 0.4.x, no published +consumer depends on the phantom-open shape. + +## Decision + +### 1. The open-op wrapper gains an awaited establishment phase + +`ChannelCore::register_openable` accepts an optional +**establisher** alongside the existing `OpenHandler`: + +```rust +/// The establishment result. `Establishment` carries what the +/// pump phase needs (today: nothing — reserved for a channel plan). +/// `EstablishmentError` carries the reason code + message the +/// wrapper puts in the open reply's `details`. +pub type OpenEstablisher = Arc< + dyn Fn(Value, Connection, AuthContext) + -> BoxFuture<'static, Result> + + Send + + Sync, +>; +``` + +The establisher is **awaited by the wrapper, bounded** — before the +reply is written, before the pump handler is spawned. The pump handler +(the existing `OpenHandler`, unchanged) is spawned only on +establishment success. The dial is the natural establisher step for +tunnels; the pumps remain the spawned handler. This restores ADR-047 +§3's original "channel plan" shape: the establisher is the awaited +preparation, the wrapper consults its result, the pumps are the +spawned protocol. + +**Why split-hook, not await-and-inspect** (the two candidates review +006 proposed): + +- The split preserves the `OpenHandler` type exactly. Await-and-inspect + changes `OpenHandler`'s return type (`JoinHandle<()>` → + `JoinHandle`), breaking all three consumers' handlers and + the alkhttp `OpenableAlpn` ferry for no compensating gain. +- Establishment and pump are genuinely different lifecycles. The dial + is synchronous with the open reply (SSH's semantics: the failure is + the open's reply); the pumps outlive the reply. Coupling the + establishment signal to the pump task's lifecycle (watching a + `JoinHandle` for a first resolution) conflates them and makes the + "established, continue" signal an out-of-band convention (a sentinel + `Result` value, a oneshot the handler must remember to signal) — + more protocol per crate, the thing this fix exists to remove. +- The split is backward compatible by construction: no establisher + registered = an always-OK establisher. Existing registrations compile + and behave unchanged. + +### 2. The deadline bounds the establisher, not the pumps + +The establisher await is bounded by the dispatch deadline when the +`OperationContext` carries one (`context.deadline`), else by a crate +constant (`ESTABLISHMENT_TIMEOUT`, 10s default; overridable per +registration via a `Duration` argument on the establisher-taking +`register_openable` variant). On deadline expiry the wrapper treats it +as establishment failure with reason `timeout`. + +The bound applies **only** to the establisher. The spawned pump +handler's lifetime is governed by the existing teardown machinery +(`channel/close`, connection drop, handler exit) — unchanged. + +Head-of-line safety is already proven: the serving loop spawns Once +invocations as independent tasks +(`Dispatcher::spawn_once_dispatch`), so a slow establisher on one open +op does not block other calls on channel 0. + +### 3. Establishment failure: teardown + typed `channel:open_failed` + +On establishment failure (error or deadline), the wrapper: + +1. Tears down the just-allocated channel + (`teardown_channel` — drops the demux sender, returns the not-yet- + installed handler task handle if any), +2. Takes the opener-ledger entry and calls `policy.on_close(opener)` + (the same un-increment path the allocation-failure arms already + run — the ledger `take` is the atomic gate, ADR-047 §7), +3. Replies with a new typed error: + +``` +code: "channel:open_failed" +message: human-readable establishment failure description +retryable: false +details: { "reason": , "message": } +``` + +The reason-code vocabulary maps 1:1 onto what an establisher can +actually produce (per the SSH four; the survey's finding): + +| reason | meaning | +|---|---| +| `dial_failed` | the backend/target could not be reached or refused | +| `unknown_resource` | the requested resource does not exist | +| `resource_shortage` | the backend is out of capacity (ports, fds, slots) | +| `handler_error` | establisher-internal failure not covered above | +| `timeout` | establishment exceeded the deadline | + +Policy denial stays `channel:too_many_channels` (pre-allocation, +unchanged); ACL denial stays `FORBIDDEN` (registry gate, unchanged). +The new code is an additive wire addition (new error-code string + +optional `details` shape); no existing consumer breaks. ALPN crates' +open-op specs gain matching `ErrorDefinition` entries per ADR-016 so +`services/schema` discloses the failure contract. + +The SSH "channel never exists opener-side" property is the contract: +the consumer's open resolves `Err` and no `channel_id` was ever +returned. (The allocation still happened accept-side momentarily — +that is invisible to the consumer and is what the teardown in step 1 +cleans up.) + +### 4. `ChannelClient::open_channel` stops erasing the error (review 006 N-1) + +`ChannelClient::open_channel` currently flattens the `CallError` into a +`String` (`format!("open op failed: {e:?}")`), which would make the +typed reason invisible to consumers — the E-01 fix would be unreachable +end-to-end through the primary client path. It changes to return a +typed error carrying the `CallError` (a new +`ChannelOpenError { error: CallError }` or equivalent), so the +consumer branches on `channel:open_failed` + `details.reason`. + +This is a breaking change to a method signature introduced in this +crate's 0.4.x — acceptable at 0.5.0 (see Consequences), and it is the +point of the change: the reason must be consumer-usable. + +### 5. Compatibility and migration + +- `OpenHandler`'s type is unchanged. Existing registrations compile + unchanged. +- `ChannelCore::register_openable` keeps its current signature + (no establisher = always-OK); a new + `register_openable_with_establisher(spec, establisher, open_handler, + registry, auth)` variant adds the hook. alkhttp's `OpenableAlpn` + gains an optional `establisher` field (default `None`) — the ferry + passes it through mechanically. +- alktty migrates its **channels path** semantic failures (unknown + backend, `carriage != "raw"`, `allocate_failed`, ownership denial — + currently post-open error frames) into the establisher, resolving + them as `channel:open_failed`. Its direct-ALPN path **keeps** the + in-band error frame (two transports, two contracts; the direct path + has no open op to fail). The `0x00`-peek disambiguation stays for + the direct path only. +- alkhttp is unaffected (no openable ops in the default surface; the + `OpenableAlpn` change is additive). + +### 6. Panicked pump handlers stay EOF-shaped (pinned as designed) + +The wrapper's teardown task swallows the pump handler's `JoinError` +(`let _ = raw_task.await`). A panicked pump = instant EOF, which is +the correct consumer-visible outcome for a mid-stream handler crash +(indistinguishable from an abrupt close — there is no error channel +mid-stream by design; establishment errors are the only kind that +belong in the open reply). This ADR pins that as intended; no change. +The establisher, by contrast, runs pre-reply — its panic (a future +that panics when polled) surfaces as the spawned Once task's panic, +which the serving loop already tolerates (the call never resolves; +the deadline / client timeout is the bound). Establisher +implementations return `EstablishmentError` instead of panicking, per +this crate's no-panic convention. + +## Consequences + +**Positive:** + +- Establishment failure reaches the consumer as a typed, branchable + call error — retry policy, client UX, and error reporting become + possible for dial-refused, unknown-resource, and shortage cases + (previously: instant-EOF ambiguity). +- The SSH contract ("the channel never exists opener-side") holds + consumer-visibly: a failed open never returns a `channel_id`. +- No phantom channels: the ledger, policy count, and manager state are + restored atomically on failure — allocation and teardown balance. +- alktty's per-crate in-band error vocabulary is retired on the + channels path; every future ALPN crate (alktunnels first) gets the + establishment reply for free. +- ADR-047 §3's "channel plan" shape is realized: awaited preparation + before reply, spawned pumps after. + +**Negative:** + +- `channel:open_failed` + the reason vocabulary is a new wire-visible + error surface — additive, but it joins the stable error set + consumers may branch on (per ADR-016, `details` shapes are + discoverable via `services/schema`). +- `ChannelClient::open_channel`'s error type changes (breaking at + 0.5.0; mechanical for consumers — the `String` was a + debug-formatting wrapper anyway). +- `OpenableAlpn` (alkhttp) gains a field; its two construction sites + add `None` (mechanical). +- The establisher await adds a bounded latency to open-op replies + where handlers previously replied instantly (the spawn). The 10s + default is the worst case for a hung establisher; real establishers + (dial, lookup) complete in dial-time. Consumers already tolerate + call-op latency; the deadline is the bound. + +## Door type + +**One-way (wire-visible error surface).** `channel:open_failed` and its +`details.reason` vocabulary join the stable error set: once consumers +branch on reason codes, changing the vocabulary requires a migration +(the same one-way-ness ADR-016 gives typed error details). The +establisher hook shape itself — `OpenEstablisher`, the +`register_openable_with_establisher` variant, the +`Establishment`/`EstablishmentError` types — is a **two-way-door +implementation detail** within the one-way decision (the wrapper shape, +per ADR-047 §3's own door-type note). The `OpenHandler` type is +untouched, which is what keeps the split cheap to revise. + +## Implementation units + +1. **alkcall 0.5.0** — `OpenEstablisher` + + `register_openable_with_establisher`; wrapper flow (await bounded → + teardown-on-failure → `channel:open_failed` with details); + `ChannelClient::open_channel` typed error (N-1); tests: + - establisher fails after allocation → consumer's `open_channel` + resolves `Err(channel:open_failed)` + reason details; channel + absent from `channel_ids()` afterward; + - establisher never completes → `timeout`-reason failure within the + deadline, channel torn down, ledger decremented; + - no-establisher registration behaves exactly as today (compat + gate); + - establisher success spawns pumps and replies `{channel_id}` + unchanged. +2. **alktty migration** — channels-path semantic failures move into an + establisher; `open_via_channels_surfaces_negotiation_rejected` + resolves via call error; the direct-ALPN error-frame path is + retained. +3. **alkhttp pass** — `OpenableAlpn.establisher: Option<...>` (default + `None`), threaded through the session fork (mechanical). + +## References + +- Review 006 E-01 (the establishment gap — findings and prior-art + survey), N-1 (the client error-type gap this ADR also resolves), + E-03/E-04 (adjacent teardown/early-arrival notes, filed separately + from this ADR's scope) +- ADR-047 §3 (openable ALPNs are operations — the "channel plan" + wrapper shape this ADR restores; §7 opener ledger — the teardown + un-increment path) +- ADR-016 (typed error schemas — the `details` vehicle) +- ADR-040/041 (backpressure/caps — untouched; the teardown path keeps + the ledger `take` as the atomic gate) +- alktty ADR-009 (the open op's input is the negotiation) + review 001 + L1/L3 — the in-band mechanism retired on the channels path +- alktunnels OQ-TN-09 (dial-failure reporting — the first consumer of + the new error) and `docs/research/ssh-socks5-survey.md` + §"Open-failure path" (the reason-code prior art) +- RFC 4254 §5.1, RFC 1928 §6 — SSH/SOCKS5 open-failure semantics \ No newline at end of file diff --git a/docs/reviews/006-channel-open-establishment-gap-review.md b/docs/reviews/006-channel-open-establishment-gap-review.md index 2390629..9d33b18 100644 --- a/docs/reviews/006-channel-open-establishment-gap-review.md +++ b/docs/reviews/006-channel-open-establishment-gap-review.md @@ -2,12 +2,27 @@ ## Status -Open — findings filed from the alktunnels Phase 0 research pass +**Resolved-by-ADR (E-01, N-1) / Verified + planned (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). 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). @@ -22,10 +37,13 @@ forwarding prior art recorded in 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. +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) +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), @@ -315,7 +333,7 @@ consumer. --- -# Remediation sketch (for the ADR discussion, not landed) +# 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 @@ -337,6 +355,63 @@ small ADR amendment: recommend (3) — subscribe for live resource sets **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). + +**Unit 2 — E-02 (ride the 0.5.0 release).** Additive +`description: Option` 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). + +**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. + +**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 @@ -352,6 +427,71 @@ small ADR amendment: recommend (3) — subscribe for live resource sets `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 @@ -374,4 +514,8 @@ small ADR amendment: recommend (3) — subscribe for live resource sets 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. \ No newline at end of file + 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). \ No newline at end of file