alkcall/docs/reviews/008-tunnels-graduation-upstream-review.md

Review 008 — Graduation Upstream Asks (from the alktunnels graduation)

Status

Open — filed 2026-09-16 from the alktunnels graduation spec work (ADRs 007/008 at /workspace/@alkdev/alktunnels/docs/architecture/decisions/; research record at /workspace/@alkdev/alktunnels/docs/research/tunnels-graduation.md). Both asks are prerequisites for the alktunnels graduation implementation (the direct op + the listen-metadata shape), which is deliberately sequenced AFTER these land — alksocks (the first consumer) composes against the graduated surface, so the discovery and reply shapes must be final before it pins them. Verified against alkcall tree @ HEAD, 0.7.1 (all code references below cite current files/lines).

Findings continue the review numbering with prefix U.

Scope

The two wire-adjacent surfaces the graduation's ADRs depend on: (1) the open-op ↔ discovery relationship (alkcall ADR-047's op-name → ALPN derivation and its relay reconstruction), which flavor-form open op ids must survive; (2) the establishment → reply boundary (alkcall ADR-049's wrapper), which a bind-first listen establisher must be able to project metadata into. Both are additive; neither changes the channels data-plane wire format (ADR-034/071 untouched).

U-1: Flavor-form open-op ids in discovery derivation

Finding. ADR-047 §1 pins the open-op naming to exactly two shapes per ALPN — channels/<alpn>/sub and channels/<alpn>/pub — and Gap F's marker derivation (rebuild_spec_for, src/client/from_call.rs:209 + derive_alpn_from_op_name, src/client/from_call.rs:305) parses exactly those suffixes to reconstruct the channel_open marker after discovery (spec_to_json serializes the marker as a boolean, src/registry/discovery.rs:244 — the ALPN string itself does not survive the round trip). alktunnels' graduation pins two NEW Sub-typed open ops on the existing alk/tunnel ALPN with flavor-form op ids:

• channels/tunnel/direct (ADR-007 — dynamic-target egress; the ssh direct-tcpip capability, scope tunnel:direct)
• channels/tunnel/forwarded (ADR-008 — accept-as-open toward the listen-opener side; the ssh forwarded-tcpip capability, scope tunnel:forwarded)

Runtime is NOT blocked today: registration sets the ALPN marker explicitly (with_channel_open), dispatch never parses the op name (register_openable_with_establisher accepts any spec name with a marker — verified, src/channels/operations.rs:596), and the client takes the ALPN as an argument (open_channel(op_id, params, alpn)). The gap is discovery + hub relay: a flavor-form op id fails derive_alpn_from_op_name's suffix match, so rebuild_spec_for rebuilds the spec WITHOUT the channel_open marker, so a hub consuming through discovery (the from_call relay path, ADR-047 Gap C — the alknodes re-produce shape) treats the open op as a plain forwarding stub instead of wrapping it with relay machinery.

Prior art in-tree. derive_alpn_from_op_name already tolerates multi-segment ALPNs (segment.contains('/') → used verbatim, so channels/x-y/sub → alk/x-y); the shape is one suffix away from flavor support.

Requested change. Extend the op-name convention (ADR-047 amendment): channels/<alpn>/<flavor> is a valid open-op name where <flavor> is a bare path segment (no /); the derivation generalizes from "strip /sub or /pub" to "strip the LAST segment when it is a known op-type marker (sub, pub) or a flavor registered with an explicit ALPN marker." Two viable implementation shapes, either acceptable:

• (a) Suffix-set extension — strip_suffix against ["/sub", "/pub"] extended with a flavor allowlist threaded from registration. Minimal, but couples discovery to a registry.
• (b) Wire the flavor op's ALPN explicitly through discovery — add an optional channel_open_alpn string field to the services/schema payload (spec_to_json emits the ALPN string instead of true when the op name is not a standard …/sub/…/pub shape; rebuild_spec_for prefers the explicit string). This also future-proofs the boolean-only marker (Gap F) whose round-trip is lossy by construction for any non-derivable name.

alktunnels ADR-002 Amendment 1 pins the convention sentence ("new flavors are new op ids — additive; the …/sub op is never reused for a different meaning"); alkcall's amendment should mirror it. The boolean marker's wire meaning for standard-shape ops is unchanged.

Why one-way-door timing: the op ids are wire-stable from the first consumer (alksocks). If discovery's derivation ships after consumers exist, hubs deployed in between cannot relay the new ops — the failure is silent (plain forwarding stub instead of channel relay), which is the worst failure mode for a relay.

Verification gates:

1. rebuild_spec_for("channels/tunnel/direct" summary) reconstructs the spec WITH channel_open = alk/tunnel.
2. A hub-relay round trip over a real channels connection: consumer → hub (from_call + relay wrapper) → producer, opening channels/tunnel/direct end to end, the hub forwarding the channel (the ADR-042 relay path riding the reconstructed marker).
3. Old boolean-marker ops (channels/tty/sub) round-trip unchanged (no regression on the existing derivation).

U-2: Establisher → reply projection (Establishment contributing reply fields)

Finding. The open-op success reply is constructed by the wrapper and hardcoded to the channel id: ResponseEnvelope::ok(request_id, json!({ "channel_id": channel_id })) (src/channels/operations.rs:955 in run_open_wrapper). ADR-049 §1 explicitly reserved the establishment phase as the wire's pre-data moment and ADR-049 amendment 2 landed the typed-opaque plan (Establishment { plan }) for the establisher → handler direction — but the establisher has no path to contribute to the REPLY (the establisher → opener direction). alktunnels ADR-008's bind-first listen establisher needs exactly that: establishment ends at bind time, and the observed OS-chosen bound address must ride the open-op reply as an additive "bound" field (the SOCKS5 BIND reply#1 BND.ADDR fidelity ask; the alternative — an out-of-band query op — is a new wire surface, strictly worse than an optional reply field).

Shape note. #[non_exhaustive] on Establishment (ADR-049 amendment 2) anticipated exactly this carrier change. The sketch:

• Establishment gains optional reply fields — e.g. Establishment::new(plan).with_reply_field("bound", json!({...})) or reply_fields: Option<Map<String, Value>> (bikeshed: builder vs struct field; the map shape generalizes beyond bound without a second amendment).
• The wrapper merges them into the success output AFTER reserving channel_id (wrapper-owned key: an establisher-provided channel_id key is an establisher bug — reject or handler_error-class it loudly rather than shadowing).
• Absent fields → the reply is byte-identical to today's; no schema change on the wrapper side (the output schema is the op's own concern — alktunnels' listen-op spec documents bound as an optional output field).

Consumer-compat check: open_channel extracts channel_id from the response and ignores unknown fields (the client-side parse, src/channels/client.rs:932's contract: "respond { channel_id } — asserts the response carries a channel_id"), so old consumers are unaffected by a NEW field; new consumers reading bound against an OLD alkcall simply see the field absent — the additive posture alktunnels ADR-008 §2 pins ("until upstream lands, the field is simply absent").

Requested change (ADR-049 amendment 3): the reply projection as sketched above, with the channel_id reservation documented.

Verification gates:

1. An establisher returning a reply field produces { channel_id, bound } on the wire; one returning none produces { channel_id } (byte-identical to today's).
2. An establisher attempting to set channel_id fails loudly (the reservation holds).
3. The existing establishment tests (establisher-success pump round-trip, no-establisher compat) pass unchanged.

Non-asks (recorded to bound the review)

• The channels data plane is untouched — the forwarded op's data plane is the existing BiStream + pump_bidi; the bound field is call-plane JSON. ADR-034/071's one-way doors stay closed.
• No new alkcall op types — both new tunnel ops are Sub-typed open ops (the existing HandlerKind::Stream wrapper path).
• No identity/ACL changes — the new scopes (tunnel:direct, tunnel:forwarded) ride the existing op-spec required_scopes; the per-call opener identity (CF-005/CF-006) is the only identity surface the establisher/handler need (the forwarded op's peer param is producer-asserted data, not transport truth — alkcall need not validate it).

References

• alktunnels ADR-007 (docs/architecture/decisions/007-direct-open-op.md), ADR-008 (008-listen-metadata-forwarded-op.md) — the consumers of both asks, with the full context and alternatives
• alktunnels ADR-002 Amendment 1 — the flavor-form op-id convention sentence
• alkcall ADR-047 (open ops, Gap F, the derivation), ADR-049 (establishment; amendment 2 — the plan payload whose #[non_exhaustive] anticipates this carrier), ADR-042 (hub relay — the relay path U-1's gate 2 exercises)
• alksocks poc-bind-findings.md (the two fidelity asks with evidence; the upstream-ask ledger this review retires)
• Review 007 (the precedent for this filing: POC-driven upstream asks before the dependent implementation)

---

Remediation plan (adopted 2026-09-16 — findings verified, plan pinned)

Both findings verified against HEAD 3b36b40 (0.7.1) — all code citations check out, with one line drift: U-2's reply construction is src/channels/operations.rs:945 (review cites 955; the cf-006/007 commit shifted the wrapper down ten lines). One review-scope deviation from the as-filed text, adopted in planning:

• U-1's gate 2 escalates from "test harness" to an in-tree relay component. The as-filed review treated ADR-042's relay wrapper as consumer-side code (the ADR's §Scope note pins the implementation downstream). The consumer decision is that the re-produce/relay shape is general (alkhttp's fallback hub, alknodes, the vpn-like endgame all compose it), and a real consumer now requires it — so ADR-042 is amended: the relay implementation moves into alkcall as a reusable component, and gate 2 becomes a genuine in-tree e2e.

Design decisions pinned above the as-filed option lists:

• 2026-09-17 (Unit 3 planning session): Unit 3 split into 3a/3b/3c and design pinned in ADR-051 (docs/architecture/decisions/ 051-channel-relay-and-hub-leg.md) — the session's discussion expanded the unit's scope beyond the as-pinned sketch: the hub/spoke family (hub or spoke may relay; the hub re-exposes spoke services by ACL without binding ports) needs the call-half support too (the hub-leg install template — now Unit 3b). Key decisions, all in ADR-051: the relay holds Arc<CallConnection> + ChannelManager per producer leg (not a ChannelClient — take_call_connection's detach is wrong for a hub with three CallConnection claimants); the reason mapping preserves the spoke's code+message (timeout is the one non-1:1 case — it maps to dial_failed); the registration seam is two-phase (fork mechanism, §4); the relay map and channel/close translation dissolve (implicit mapping + EOF cascade, recorded as the ADR-042 amendment's mechanism supersessions); Pub-typed marked specs are a loud assembly error; establishment bounds compound per hop (noted, no fix).
• U-1 shape: combined (b) + (c). The as-filed option (a) is unsound as stated — the derivation runs consumer-side against a wire JSON payload, so an allowlist "threaded from registration" has no path to rebuild_spec_for (both its consumers — from_call, op/register — are wire-fed). The pinned shape: spec_to_json_pub emits an explicit channel_open_alpn string alongside the boolean when the op name is not a standard …/sub/…/pub shape (standard shapes stay byte-identical); rebuild_spec_for prefers the explicit string, else generalizes the derivation to strip the LAST segment when the boolean marker is present (the marker is the gate, so any bare-segment flavor works; op/register is fixed by the same parser).
• U-2 client read path: add the ergonomic accessor now — open_channel_with_reply returning the extra reply fields alongside the streams; open_channel unchanged. Keeps alkcall's primary surface intact while giving alktunnels' TunnelListener::open a first-class way to read bound.

Unit 1 — U-2: establisher reply projection (ADR-049 amendment 3)

Land first: the relay component (Unit 3a) projects a spoke's extra reply fields through Establishment, so the projection must exist before the e2e can assert bound flowing through a relay.

1. Establishment gains reply_fields: Option<Map<String, Value>> + builder (Establishment::new(plan).with_reply_field("bound", json!({...})); also with_reply_fields(map) / reply_fields() accessor). #[non_exhaustive] holds; additive, no break.
2. run_open_wrapper: on establisher success, merge reply_fields into the success output AFTER reserving channel_id. An establisher-supplied channel_id key is an establisher bug — fail loudly as EstablishmentError::HandlerError (reason handler_error, message naming the reserved key), tearing the channel down (the wrapper's existing failure path). No silent shadowing.
3. Client: ChannelClient::open_channel_with_reply(operation_id, input, alpn) -> Result<(u32, Value, MpscSendStream, MpscRecvStream), ChannelOpenError> — the full success output (the consumer reads bound from it); open_channel delegates with the fields discarded. Additive; no signature change.
4. Doc: ADR-049 amendment 3 (the projection, the channel_id reservation, the accessor). The output schema remains the op's own concern (alktunnels' listen-op spec documents bound as optional).

Verification gates (from the review, tightened):

• Establisher returning a reply field produces { channel_id, bound } on the wire; one returning none produces { channel_id } (byte-identical to today's — assert exact JSON).
• Establisher attempting channel_id → channel:open_failed with reason handler_error; channel torn down; ledger decremented.
• The existing establishment tests (establisher-success pump round-trip, plan-flow, no-establisher compat) pass unchanged.
• open_channel_with_reply returns the extra fields end-to-end over a real channels connection; open_channel behavior unchanged.

Unit 2 — U-1: flavor-form derivation + explicit ALPN field (ADR-047 amendment)

1. derive_alpn_from_op_name (src/client/from_call.rs:305) generalizes from strip-/sub|/pub to strip the LAST path segment (rsplit_once('/')), which is a strict superset of today's behavior (standard two-suffix shapes derive identically; multi-segment ALPNs survive the same way). The marker boolean remains the gate in rebuild_spec_for — the derivation is only consulted for marked ops, so plain ops named channels/x/y are unaffected (the channels/tty/query-returns-None unit test moves from the function to the marker-gated rebuild level).
2. spec_to_json_pub (src/registry/discovery.rs:222): when channel_open is set and the op name is NOT a standard channels/<seg>/(sub|pub) shape, emit "channel_open": true PLUS "channel_open_alpn": "<alpn>". Standard shapes keep the boolean only — byte-stable for every existing consumer. The advertised operation_spec_schema (discovery.rs:159) gains the optional string property (schema-type widening, additive).
3. rebuild_spec_for (from_call.rs:209): prefer the explicit channel_open_alpn string; else boolean → generalized derivation. Covers both wire consumers (from_call, op/register) by construction.
4. Doc: ADR-047 amendment — the convention sentence mirroring alktunnels ADR-002 Amendment 1 (channels/<alpn>/<flavor> is a valid open-op name, <flavor> a bare segment; flavors are additive, the …/sub op is never reused), the explicit-field wire shape, and the one-way-door note (the field joins the stable services/schema payload; the boolean's meaning for standard-shape ops is unchanged). Record the residual ambiguity: an ALPN segment that itself collides with a flavor name (channels/x/direct where the ALPN is alk/x/direct-shaped) is undecidable from the name alone — the explicit field is the disambiguator.

Verification gates:

• Review gates 1 and 3 as unit/round-trip tests: rebuild_spec_for("channels/tunnel/direct" summary) reconstructs WITH channel_open = alk/tunnel; channels/tty/sub (boolean) and channels/custom/proto/sub (multi-segment) round-trip unchanged; flavor-form spec serializes with the explicit string and rebuilds from it; op/register announced flavor-form spec round-trips with the marker.
• Review gate 2 (hub-relay e2e) lands with Unit 3c.

Unit 3 — the in-tree relay component (ADR-042 amendment, ADR-051) + gate 2 e2e

Adopted 2026-09-17, design pinned in ADR-051 (docs/architecture/ decisions/051-channel-relay-and-hub-leg.md): the relay is the wrapper composition (translate hop = the ADR-049 establisher shape; byte- forward hop = pump_bidi per ADR-050), the ADR-042 relay map dissolves (implicit per-channel mapping), channel/close needs no translation surface (the EOF cascade propagates with correct ledger accounting on both legs), and the registration seam is two-phase (discover/stash → per-connection fork-register — the ADR-047 §4 amendment's fork mechanism makes a one-call import impossible without adapter changes). The call-half support the hub/spoke family needs is the hub-leg install template (Unit 3b) — composition of existing pieces, not a new protocol surface. Three sub-units, three sessions' scope:

Unit 3a — ChannelRelay (the relay component)

New module src/channels/relay.rs:

1. ChannelRelay::register_relay_openable(consumer_registry, spec, producer_leg) — for a from_call-imported spec with channel_open set, registers on the hub's consumer-leg channel-0 fork registry via register_openable_with_establisher, where:
   • producer_leg is Arc<CallConnection> + producer-leg ChannelManager (ADR-051 §2 — not a ChannelClient: the hub's CallConnection is shared by import, relay establisher, and hub ops, so take_call_connection's detach is wrong; the establisher builds its forwarded payload directly — the hub as caller, the consumer as forwarded_for from the per-call identity, ADR-026 §3).
   • The establisher (the translate hop, ADR-042 layer 1): calls the producer leg's open op with the forwarded payload, adopts the returned spoke channel_id into the producer-leg manager, and returns Establishment::new(plan) whose plan carries the spoke-leg streams. The spoke reply's channel_id is stripped; the reply's other fields ride with_reply_fields (bound flows end-to-end, per-hop truthful). Failure mapping per ADR-051 §3 (the reason table; spoke CallError code + message preserved in the mapped variant).
   • The OpenHandler (the byte-forward hop, ADR-042 layer 2): pump_bidi(consumer_leg_bistream, spoke_recv, spoke_send) — the relay never parses chunk framing (per-leg mux/demux absorbs the 8-byte header; no ID rewrite exists — ids are per-connection). Awaited inline (R-02); on completion both legs tear down (the wrapper's handler-exit teardown covers the consumer leg; the pump's drops EOF the spoke leg).
   • The consumer leg keeps the full wrapper machinery: ACL, per-identity cap/ledger, establishment bound (bounds the spoke dial round-trip), teardown-on-failure — no parallel authorization path.
2. Rejection postures (ADR-051 §6): a Pub-typed marked spec is an assembly error at registration (channel:pub_open_not_implemented class, C-08 blocker) — never a silent stub; an unmarked spec passed to the relay registration is likewise an error (branch on the marker before calling).

Verification gates:

• The establisher populates forwarded_for from the consumer's per-call identity; the spoke reply's channel_id is stripped (the consumer reply carries the hub-allocated id).
• A spoke channel:open_failed maps to the consumer-leg channel:open_failed with the mapped reason and the spoke's message preserved; consumer leg tears down (ledger decremented); no spoke channel leaks (the adopt-then-fail window is covered).
• bound (Unit 1's projection) survives the relay to the consumer reply.
• Pub-typed and unmarked specs are rejected at registration.

Unit 3b — the hub-leg install template (the call-half support)

Landed (2026-09-17): src/channels/hub_leg.rs — HubLegImports (the Clone stash split by the marker) and HubLegTemplate (the §5 composition exported as the InstallChannelZero hook; serving identity via the CF-005 seam). One assembly rule the planning sketch missed, surfaced by the template's gate e2e: from_call discovers the spoke's bootstrap discovery ops like any plain op, so the template skips registry::discovery::BOOTSTRAP_DISCOVERY_OPS when re-registering plain bundles — the template's own install (closed over the fork, review 004 F-06) supersedes the imported copies. Unit 3c (the gate-2 e2e) remains.

The hub-side composition every test hand-rolls (make_install_channel_zero-shaped), exported in-tree (ADR-051 §5 — two-way-door API shape):

1. Fork the hub's base registry (ADR-047 §4 amendment mechanism).
2. Register the generic channel ops (ChannelOperations::register_on) and the bootstrap discovery ops (closed over the fork, so services/list sees the fork's ops — review 004 F-06).
3. Register the stashed plain bundles as-is (the existing forwarding handlers) and the marked specs via Unit 3a's relay registration. The stash is the Cloneable Vec<HandlerRegistration> from_call returned (registrations and HandlerKind are Clone; one discovered set serves any number of consumer legs).
4. Per-call identity resolution for the serving dispatch (ServingConfig-shaped precedence: payload token → explicit override → transport identity), then the single-stream dispatch loop.
5. Per-consumer op-subset filtering composes here (the fork is per consumer leg — ADR-051 §4's composition note). ACL layering is pinned in ADR-051 §5: the spoke's AccessControl sees only the hub identity; the end consumer's identity is forwarded_for metadata, never checked by any AccessControl::check.

Verification gates:

• The template composes fork + generic ops + bootstrap discovery + plain bundles + relay openables + serving identity and dispatches channel 0; services/list on the consumer leg shows the re-exposed ops; a plain imported op round-trips through the hub (the call-half forwarding path exercised for real).

Unit 3c — gate 2, the review's full e2e harness

The review's verification gate 2, now with the in-tree components:

1. Consumer → hub (Unit 3b's template on the consumer leg; ChannelsAdapter both legs) → producer, opening channels/tunnel/direct end to end: the open resolves on the consumer with the hub-allocated channel_id, the establisher-contributed bound field (Unit 1) survives the relay to the consumer reply, data flows both directions on the data channel (the hub never parses it), and hub-side disconnect / spoke-side close tears down both legs' channels.
2. The mid-establishment disconnect window (ADR-051 §6): consumer leg dies between spoke open and pump spawn — the spoke channel is reclaimed when the pumps run (EOF cascade), both ledgers decremented, no leak.
3. A companion channels/tty/sub relay run pins no regression on standard shapes.

Remaining bookkeeping for the final unit: ADR-042 amendment (landed with ADR-051 — the §Scope-note revision and the two mechanism supersessions), changelog + minor version bump (new pub API: ChannelRelay, register_relay_openable, the hub-leg template; open_channel_with_reply and Establishment::with_reply_field* landed with Units 1–2).

Sequencing and non-goals

Order: Unit 1 → Unit 2 → Unit 3a → Unit 3b → Unit 3c (Unit 3a is the component; 3b the assembly; 3c the e2e gate asserting bound through the relay — Units 1–2 are prerequisites of 3a; 3b and 3c of each other). One release (minor bump), matching the review-007 precedent. Not in scope (extends the review's non-asks): the data-plane wire format (untouched — the relay rides MpscSendStream/MpscRecvStream and pump_bidi; no header rewrite exists — ids are per-connection, ADR-051), any channel/control translation surface (close propagates through the EOF cascade; control is per-leg, OQ-39 — a hub wanting cross-leg control translation composes it downstream), the Pub-typed open path (rejected loudly; C-08 blocker), and the alktunnels-side work (the bind-first establisher, the listen-op spec's bound documentation, ADR-007/008 implementation) which sequences AFTER these land.

Review-008 status updates when units land: mark U-1/U-2 resolved with commit refs (the review-007 Status pattern), and correct the U-2 line citation (955 → 945) in an errata note rather than silently editing the as-filed text.