alktunnels/tasks/tunnels/review-core-crates.md at f315b0e0ca7829da96b09fef87366358ee25a56f

Files

T

glm-5.3-flash a0f939c6df review: phase-gate review 001 — spec conformance of the v1 implementation

Findings + review doc: docs/reviews/001-implementation-review.md (the
alkcall house pattern). Closes tunnels/review-core-crates and
tunnels/review-impl.

Findings:
- U-1 [major, NOT fixed] — the local UDP adapter is not the framed
  adapter ADR-003 mandates: codec-framed bytes reach real targets,
  empty datagrams are EOF-shaped at the adapter (tunnel teardown),
  >8 KiB datagrams split, unframed target-initiated datagrams are
  dropped. Executable-pinned by real-socket probes (temporary test,
  deleted after the run). Remediation task: tunnels/fix-udp-framed-adapter.
- C-1 [major, fixed] — AcceptQueue::pop lost wakeup in the
  check→register window (open op hangs to the establishment bound);
  fixed via the tokio notify_waiters contract (Notified created before
  the check each iteration) + multi-thread regression test.
- N-4/N-5/N-7/N-8/N-9/N-10/N-11 [minor, fixed] — serialize TunnelParams
  in open_reverse_channel; stale doc ref; broken doc link; root
  re-exports (TunnelParams/Substrate/tunnel_open_spec/OP_TUNNEL_OPEN);
  README (publish dry-run was failing on the missing readme); harness
  comment; pump_against UDP framing doc note.
- N-2 retracted by executable falsification (codec buffering is
  bounded by construction); N-6 non-finding with rationale.

Verified: 63 tests default / 70 with --features local, 3x repeat-run
clean both configs; clippy -D warnings (all-targets + wasm32); fmt;
doc warning-free; wasm32 check; cargo publish --dry-run passes.

2026-09-08 10:58:11 +00:00

4.5 KiB

Raw Blame History

id, name, status, depends_on, scope, risk, impact, level, tags

name

status

depends_on

scope

risk

impact

level

Description

Review injection point (SDD §Review Injection — "high-risk tasks: review before proceeding"): tunnels/producer-open-op and tunnels/consumer-session are the two risk: high, impact: phase tasks and the crate's wire/API surface. Review them BEFORE tunnels/local-socket-halves and tunnels/end-to-end-suite build against the shapes — a wire or API mistake found here costs one task's rework; found later it costs three.

Checklist (focused — the full gate is `tunnels/review-impl`)

Wire conformance of what landed (params/codec/open-op vs wire.md + ADR-001/003) — the one-way-door check.
The pump handler shape: pump_bidi inline, JoinHandle tracks the data plane (R-02) — the POC's hang-bug class must be structurally absent.
Teardown matrix of TunnelSession — close/join/Drop soundness, no leaks (the W3 class).
No substrate types outside src/local/-to-be; no hand-rolled two-pump loops; no side-channel handoff.
Integration tests green + repeat-run stable (3×).

Deliverable: findings inline here (Notes/Summary); criticals block the downstream tasks (Safe Exit); majors create remediation notes for tunnels/review-impl to re-check.

Acceptance Criteria

The 5 checklist items each with a verdict
Criticals (if any) resolved before proceeding; majors logged

References

docs/architecture/ (the spec set under review)
docs/sdd_process.md §Review Injection

Notes

Agent fills during implementation.

Mid-phase verdicts (2026-09-08, at tree `fb2389b`..`09d32d5` scope)

Executed with the mid-phase point in view (before local-socket-halves/end-to-end-suite landed): the five checklist items were run against producer.rs/consumer.rs/params.rs/ wire.rs/error.rs + the then-existing suites, cross-checked against alkcall 0.7.0's wrapper/establisher/pump source. Because both downstream tasks built on the shapes immediately after, the full re-verification (with the complete suite) landed in tunnels/review-impl — consolidated findings live in docs/reviews/001-implementation-review.md (the review-impl deliverable). Verdicts, summarized:

Wire conformance — PASS. Params shape exact ({resource, substrate}, deny_unknown_fields, enum ["tcp","udp","unix"] — schema + serde both pinned by tests); op id/ALPN/scope constants asserted against wire.md; codec ([len: u16 BE], len=0 legal, Oversize at frame time, no sentinel) correct.
Pump handler shape — PASS. pump_bidi awaited inline inside the spawned handler task; accept_bi → plan downcast → Arc::try_unwrap → inline await; no spawn-and-forget, no hand-rolled two-pump loop (grep-verified); R-02 pinned by an end-to-end test (pump_handle_tracks_the_data_plane_r02).
Teardown matrix — PASS. close (abort + reap), join (await + reap + counts, panic arm logged), Drop (abort + sync reap), pump-less join (0, 0, reaped), failed-adopt no-leak — all pinned with channel_ids() leak asserts on both sides. No Clone (compile-fail doctest).
No substrate types / no hand-rolled pumps / no side-channel handoff — PASS. The only plan flow is Establishment::new(plan) → wrapper → handler plan param (R-01); the only Mutex<HashMap> is the ResourceRegistry lookup table (a registry, not a plan handoff); tokio::net appears only under the local gate.
Tests green + 3× repeat-run stable — PASS (both configs).

Majors logged (carried to `tunnels/review-impl`, re-verified there)

C-1 (AcceptQueue::pop lost wakeup — found in the full sweep, fixed in-review): see review 001 §C-1.
U-1 (the local UDP adapter is not the framed adapter — found by the review-impl probes over real sockets): see review 001 §U-1; remediation task tunnels/fix-udp-framed-adapter.

No criticals: the wire surface (the one-way door) is conformant; no finding blocks the phase.

Summary

Agent fills this on completion.

Mid-phase review executed as part of the consolidated tunnels/ review-impl pass (2026-09-08): all five checklist items PASS; two majors logged (C-1, U-1) and carried to the phase-gate review, where C-1 was fixed and U-1 got the remediation task. Full findings + severity grading: docs/reviews/001-implementation-review.md.

4.5 KiB Raw Blame History Unescape Escape