alkcall/docs/reviews/007-establishment-follow-ups-review.md

Review 007 — Establishment Follow-Ups (from the alktunnels UDP POC)

Status

Resolved — all three units landed in alkcall 0.6.0 (2026-09-07): Unit 1 dc4ad2b (R-01 + R-02's doc notes), Unit 2 8f122b0 (R-02's telemetry), Unit 3 9c6fec1 (R-03 + ADR-050). Two deviations from the sketches below, both recorded in the ADRs: (1) R-01's plan is typed-opaque (ChannelPlan = Arc<dyn Any + Send + Sync>), not Option<Value> — the sketch could not satisfy this review's own verification gate, because the payloads establishers hand off are live handles (dialed sockets, TTY handles) with no JSON representation; (2) R-03's helper returns (u64, u64), not io::Result<(u64, u64)> — both pumps swallow copy errors by the ADR-078 contract (mid-stream error = abrupt close), so an Err state would be dead code. Original findings below, retained as filed (verified against tree 36e74cd, 0.5.0).

Open — findings filed from the alktunnels UDP POC pass (alktunnels-udp-poc, 2026-09-06; summary at /workspace/@alkdev/alktunnels/docs/research/poc-summary.md). This review exists to prevent a second fix→publish→update-dependents cycle: every finding below is either (a) guaranteed to be needed by alktunnels Phase 1 implementation, or (b) a documented contract gap that will bite the next OpenHandler author the way it bit the POC. Nothing here is speculative — each finding was reached by writing working code against 0.5.0 and finding the shape insufficient.

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

Post-remediation note (2026-09-07): the "Open —" paragraph above is the original filing state, retained for the record. All units landed; see Status above and ADR-049 amendment 2 + ADR-050.

Scope

The ADR-049 establishment surface (OpenEstablisher, Establishment, EstablishmentError, register_openable_with_establisher) and the OpenHandler contract, reviewed from the alktunnels UDP POC's consumer/producer implementations. Everything verified against source at tree 36e74cd (0.5.0). Cross-references: alkcall review 006 (E-01..E-04), alkcall ADR-049, alktunnels poc-summary.md (§Issues Surfaced), alktty's channels establisher (make_tty_establisher).

Verified against: alkcall 36e74cd (0.5.0)
Reading list: src/channels/operations.rs (OpenEstablisher, Establishment,
  run_open_wrapper, teardown_failed_channel), src/channels/client.rs
  (ChannelOpenError::establishment_reason), src/channels/manager.rs
  (teardown_channel, adopt_channel), docs/architecture/decisions/
  047 + 049, and the consumers: alktty src/channels.rs (make_tty_
  establisher / make_tty_open_handler), alkhttp src/websocket/
  upgrade.rs (OpenableAlpn ferry), alktunnels-udp-poc src/producer.rs

Severity legend

Same scale as reviews 004–006.

---

Part A — The findings

R-01 [major] — Establishment is payloadless, but the channel plan is exactly what establishers need to hand to the pump handler

Verified: YES — by building against the API. The ADR-049 §1 text itself anticipates this: "Reserved for a channel plan — today the wrapper consults only success/failure, so () carries no data" (src/channels/operations.rs:341-345, the empty pub struct Establishment {}).

The mechanism

The establisher is pre-data-plane: it cannot see the channel's yield-once BiStream (ADR-049 amendment), so anything it establishes — a dialed socket, an allocated handle — must cross to the pump handler through a side channel the ALPN crate invents. The alktunnels POC's workaround is a resource-keyed Mutex<HashMap<String, SubstrateHandle>> + a poll-loop take (producer.rs::HandleHandoff) that works only because the wrapper guarantees establisher-before-handler ordering. The costs:

1. Concurrent same-resource opens race the slot. Two opens of the same resource: the second establisher's deliver overwrites the first handler's not-yet-taken handle. The POC documents this as a simplification; a real crate cannot.
2. alktty hit the same wall and documented it as a limitation. alktty/src/channels.rs:253-257: the establisher cannot carry the allocated TtyHandle across, so backend allocation stays post-open in the pump handler (allocate_failed remains an in-band negotiation error frame — ADR-010) — exactly the phantom-channel-ish shape ADR-049 exists to eliminate, still alive one layer down. TTY's establisher can only validate/lookup/ ownership-check; the one thing that can actually fail with a runtime error (allocate) is unreachable from the typed error path.
3. Every ALPN crate pays the side-channel tax again. Handoffs, slots, poll loops or oneshots — per crate, per resource key, all with the same concurrency caveat.

The ask

Give Establishment its payload — the channel plan ADR-047 §3 originally described ("return a 'channel plan'... the wrapper consults its result"). Minimal shape:

pub struct Establishment {
    /// ALPN-defined plan data — the dialed handle, an allocation
    /// ticket, whatever the pump phase needs. Opaque to alkcall.
    pub plan: Option<Value>,
}

or, keeping it typed at the boundary:

pub struct Establishment { pub plan: Option<Value> }

with the wrapper passing establishment.plan (or Null) to the OpenHandler's input — e.g. as a well-known key the pump handler reads, or as a third callback parameter. The wire surface is unchanged (the plan is process-local: establisher → wrapper → handler in the same process on the producing side; nothing crosses the transport that isn't already the open op's input). Consumers' Ok(Establishment {}) construction sites (alktty has two; alkhttp's ferry has none) break mechanically at 0.6 — Establishment::new(plan) / Establishment::default() make the migration one-liners.

The break is the point of doing this now: 0.5.0 published yesterday with Establishment {} documented as reserved. The next consumer (alktunnels) needs the payload in Phase 1 — implementing Phase 1 without it means shipping the POC's side-channel handoff into the real crate, with its same-resource race, and then the payload lands later anyway as another breaking release. Filling the reserved field now is the cheap moment; the alternative is paying the breaking change twice.

Severity

[major] — the capability gap is structural for any establisher whose backend produces a handle (tunnels: dial; TTY: allocate), not a polish item. Not [critical] because workarounds exist (the POC proves one), but every workaround carries the same-resource race or forces failure classes back into per-crate in-band frames — the exact cost ADR-049 was written to remove.

---

R-02 [minor] — The OpenHandler JoinHandle lifetime contract is undocumented and load-bearing (found empirically by the POC)

The POC's first pump implementation returned a wrapper task that spawned the pump as a nested fire-and-forget task. The result: every tunnel connected then instantly EOF'd with zero bytes — the wrapper awaited the (already-complete) handler task, tore the channel down at birth, and both pumps saw immediate EOF. Everything upstream (establisher, open reply, channel routing) looked healthy; the bug was purely in the handler's return-value semantics.

The contract, as implemented: the wrapper awaits the returned JoinHandle and that completion is the teardown trigger (run_open_wrapper's spawned task: let _ = raw_task.await; → teardown_channel(id) → drop the demux sender → EOF to the handler's read half). Therefore:

• The returned JoinHandle must track the data-plane lifetime. A handler that returns before its pumps finish tears the channel down at birth. The pump must be awaited inline inside the handler task (let _ = pump_halves(...).await;), not spawned-and-forgotten.
• Half-open semantics fall out of this correctly (one pump finishing shuts down the opposite sink per ADR-078; the handler's task completes when both pumps finish — which is when teardown should happen). The contract is right; only its documentation is missing.

The current type docs (operations.rs:325-339) say the handle is "recorded for teardown (abort on channel/close / connection drop)" — abort semantics — but never say early return = teardown-at-birth. alktty got this right by accident of shape (drive_session_pre_negotiated awaited inline in its single spawned task — channels.rs:355-395); the POC got it wrong the natural way. The next consumer will write the wrong shape too, because the natural reading of "spawn your protocol and return the JoinHandle" is a task-spawner, not a lifecycle promise.

The ask: doc note on OpenHandler and ChannelCore::register_openable* — "the returned JoinHandle must track the data-plane lifetime: the wrapper awaits it and tears the channel down on completion; return a task that runs the handler to completion, never a spawner that exits early" — plus the ADR-049 amendment (§1 or §6) recording the semantics. Optional hardening (not required): a debug! in run_open_wrapper when the awaited handler exits without the channel's BiStream having been accepted (a telemetry hint for the birth-teardown pattern; the accept is observable in-process). Doc-only; no break.

---

R-03 [minor, optional unit] — The two-pump helper's convergence test is satisfied; extracting it now removes the last reason for a later sweep

alknet ADR-078 deferred helper extraction until a second two-pump consumer existed and the shapes converged. The POC supplies both halves of that test:

• Producer side: the alktunnels POC's pump_halves — two tokio::io::copy pumps over split channel-vs-substrate halves, each pump shutting down the opposite sink on completion, joined.
• Consumer side: TunnelSession::take_halves + the assembly layer's copy — the same shape modulo channel side.

The shapes converged. The helper (pump_bidi or similar) is a candidate for this same 0.6 sweep — additive (a new pub fn in channels or core), zero breaking change, and it pins the ADR-078 contract in one place instead of three. The natural signature follows the POC:

pub async fn pump_bidi<A, B>(a: A, b: B) -> io::Result<(u64, u64)>
where
    A: AsyncRead + AsyncWriteExt + Send + Unpin,
    B: AsyncRead + AsyncWriteExt + Send + Unpin,

(join'd two-pump with shutdown-on-completion; returns the copy counts for observability). alktty's three-pump session does not fit it (the exit future is a third signal) — that is fine; the helper serves the two-pump shape, TTY stays as-is.

The ask: decide in this sweep — either extract in 0.6 (additive; recommended, since alktunnels Phase 1 will implement the pattern anyway and an upstream helper makes the third consumer free), or record the explicit decision to keep it per-crate. Do not leave it half-decided: an un-extracted helper is not itself a break, but finding this out during Phase 1 would be the same fix→publish→update treadmill for a purely additive change.

---

Part B — Verified non-issues (bounded so the next review doesn't re-check)

• The reverse-flow (-R) story needs no upstream change. A hub proxy opening a channel toward a worker is the worker serving its own open op on the connect side — supported by ChannelClient::from_connection_with_serving (ADR-022 §2 both-sides semantics) + ChannelOperations::register_on on the serving registry + the wrapper allocating on the serving side (ADR-047 §5 "the side that holds the ChannelManager allocates" — both sides do, with odd/even split per ADR-047 §5). Verified by trace; no API gap. alktunnels' reverse-flow POC (OQ-TN-10 #2) will exercise this end-to-end, but no upstream mechanism is missing.
• The establisher receives registry-validated input — invoke_streaming's input_schema check runs before the wrapper (registration.rs:397); the establisher's input.clone() (operations.rs:718) is post-schema. Confirmed; no gap.
• Typed establishment errors are complete on the wire — establishment_error_to_call_error carries both reason and message in details (operations.rs:646-653); the client-side establishment_reason() branches on it (client.rs:75-83); alktty's open spec declares the matching ErrorDefinition (ADR-016). The vocabulary needs no extension for tunnels (dial_failed / unknown_resource / resource_shortage / handler_error / timeout cover every tunnel establisher failure class — verified by the POC's three typed-error tests).
• The early-arrival park cap (64) — reviewed in 006 E-04; the POC treated it as a sizing constraint, not a defect. Unchanged.
• EstablishmentError's reason set is sufficient — the POC's establisher used three of four reasons; handler_error covers params-parse failures (the TTY pattern). No new reason needed.

---

Remediation plan

Unit 1 — Establishment carries the channel plan (R-01)

1. ADR-049 amendment (or ADR-050-adjacent amendment): Establishment gains plan: Option<Value> (ALPN-opaque); the wrapper threads plan into the pump handler (third OpenHandler parameter, or merged into input under a reserved key — decide in the ADR; the separate-parameter shape avoids colliding with the input schema).
2. Bump to 0.6.0 (breaks Ok(Establishment {}) construction sites — alktty, two sites; mechanical Establishment::default() or Establishment { plan: None }).
3. Migration: alktty can then move backend allocate into its establisher (killing the in-band allocate_failed frame on the channels path — ADR-010's reason for that frame disappears); alktunnels Phase 1 uses plan for the dialed handle; alkhttp's ferry passes Option through unchanged.

Unit 2 — OpenHandler lifetime doc note (R-02)

Doc-only (type docs + ADR-049 amendment). Optional debug warning. No break.

Unit 3 (optional) — pump_bidi helper extraction (R-03)

Additive pub fn + tests. No break. Do it in the same 0.6 or record the decision not to.

What must NOT ride along

Nothing else. The remaining alktunnels Phase 1 needs (UDP codec ADR, reverse-flow lifecycle, access-policy details) are alktunnels-local spec work — the POC validated the mechanics and found no further upstream gaps. The deliberate goal of this review: 0.6 is the last breaking sweep forced by known work; anything after it should be a genuinely new discovery, not a known-dangling item.

Verification gates

• Unit 1: end-to-end test — establisher dials, plan carries the handle, pump handler receives it; concurrent same-resource opens each get their own handle (the race the POC documented is unreachable); alktty migration test (allocate in the establisher → allocate_failed is a call error, no in-band frame).
• Unit 2: doc gate only (cargo doc clean; the note present on both the type and the registration entry points).
• Unit 3: helper test — the POC's two-pump semantics (EOF from one direction completes the other; shutdown-on-completion) reproduced through the helper.

References

• alkcall review 006 (E-01 establishment gap; R-01 is the payload half of E-01's own remediation — ADR-047 §3's "channel plan" through the reserved Establishment field) and ADR-049 (the establishment phase; §1's original sketch already described the plan-carrying shape).
• alkcall ADR-047 §3 ("return a 'channel plan'" — the original shape this review restores), ADR-022 §2 (both-sides serving — the reverse-flow non-finding), ADR-047 §5 (odd/even allocation — reverse-flow allocation on the serving side).
• alknet ADR-078 (the two-pump contract; R-03's helper pins it upstream).
• alktunnels docs/research/poc-summary.md (§Issues Surfaced #1/#2 — the empirical findings; §#4 — the convergence input for R-03) and the POC crate (/workspace/alktunnels-udp-poc, 17 tests — the empirical basis for every finding here).
• alktty src/channels.rs:253-257 — the existing documentation of the R-01 cost on the TTY side (allocate cannot cross; in-band allocate_failed remains), the second consumer confirming the gap is not tunnel-specific.