---
id: tunnels/review-impl
name: Review alktunnels v1 implementation for spec conformance (pre-release gate)
status: pending
depends_on: [tunnels/end-to-end-suite, tunnels/review-core-crates]
scope: moderate
risk: low
impact: project
level: review
tags: [review, phase-gate]
---

## Description

Review the v1 implementation against the architecture spec before any
publish consideration. This is the phase-gate review (the SDD process's
review-injection point at the end of the critical path): the wire
surface (ADR-001/002/003) is a one-way door, and the API surface
(ADR-005/006) becomes ABI-stable once a consumer exists.

### Review checklist

1. **Wire conformance** (wire.md + ADR-001/002/003):
   - params shape exact (`{resource, substrate}`, `deny_unknown_fields`,
     schema enum without `unix` until OQ-TN-14)
   - op id `channels/tunnel/sub`, ALPN marker `alk/tunnel`, scope
     `tunnel:open`
   - codec: `[len: u16 BE]`, `len=0` = legal empty datagram, `Oversize`
     at frame time, no sentinel in the codec
   - typed errors: the five establishment reasons + `FORBIDDEN` +
     `channel:too_many_channels` surfaces
2. **Producer conformance** (producer.md):
   - establisher = the awaited bounded phase; plan flow (R-01) — no
     side-channel handoff anywhere (grep for `Mutex<HashMap>` +
     poll-loop take patterns — the POC's dead shape)
   - pump handler: `pump_bidi` inline, no substrate types, JoinHandle
     tracks the data plane (R-02)
   - registration: post-hoc supported (W2); `ChannelOperations::
     register_on` guidance correct
   - listen variant: same op, no new wire surface, typed error mapping
3. **Consumer conformance** (consumer.md + ADR-005):
   - `TunnelSession`: open/adopt/stream_halves/take_halves/
     send_datagram/recv_datagram/pump_against/close/join/Drop
   - teardown matrix sound (no leaks; pump-less join `(0, 0, reaped)`)
   - no `Clone`
4. **Conventions sweep** (AGENTS.md):
   - no comments in code (doc comments fine); no unwrap/expect outside
     tests; thiserror everywhere; poisoned-lock `unwrap_or_else(into_inner)`
   - substrate types confined to `src/local/` (grep-verify: no
     `tokio::net` outside the feature gate + tests)
   - wasm-clean default crate (the structural guard passed at every
     step, but re-verify)
   - `pump_bidi` consumed, never hand-rolled (grep for hand-rolled
     two-pump loops)
5. **Docs ↔ implementation sync**: lib.rs doc comments match
   overview.md's module map; public API = lib.rs re-exports (convention
   16); ADR statuses updated from Draft → Accepted where the
   implementation confirms them (or findings filed back to the OQ
   tracker).

### Deliverables

Findings as a review doc (`docs/reviews/001-implementation-review.md`,
the alkhttp/alkcall house pattern) with severity levels + concrete
remediation tasks. Criticals block the phase; majors get remediation
tasks; minors get a follow-up batch.

## Acceptance Criteria

- [ ] Review doc filed with severity-graded findings
- [ ] Remediation tasks created for anything above trivial
- [ ] The 5 checklist sections each covered with a verdict

## References

- docs/architecture/ (the full spec set)
- House pattern: `/workspace/@alkdev/alkcall/docs/reviews/` (the review
  numbering + severity legend)

## Notes

> Agent fills during implementation.

## Summary

> Agent fills this on completion.