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

0 Fork 0

Files

T

glm-5.3-flash 0ceecf582b docs: remove em dashes from repo prose and forbid them going forward

- Rewrote the remaining em dashes outside docs/reviews/ (preserved as
  historical record, matching the alktty precedent): tests/ (313),
  fuzz/ (34 plus one shell comment), tasks/ (671 plus 12 kept in
  fences), docs/research + sdd_process + plans (525 plus 25 in
  fences), docs/architecture/overview.md fence labels (13), README,
  CHANGELOG, AGENTS.md, fuzz/README.md (176), and the
  .opencode/agents prompts (110)
- Rewrote fenced content too: ASCII tree and module-map label
  separators, Rust snippet comments, and template placeholders; wire
  data, JSON/BAST examples, and transcripts survive verbatim
- Rewrote 6 assert/expect message literals in tests/ and 2 format
  strings in fuzz/shared/src; no test data changed
- Added Project Convention 18: no em dashes in prose (doc comments,
  markdown including docs/ and tasks/, agent prompts, commit
  messages); fence content rule, en-dash clause rule, and the
  no-spaced-hyphen prohibition follow alktty's convention 15

Follows 6b20924 (src doc comments) and cd0afe0 (docs/architecture).

Verification: cargo test (141 pass); cargo clippy --all-targets -D
warnings; cargo fmt --check; cargo doc --no-deps; wasm check and
clippy for wasm32-unknown-unknown with -D warnings; em dash grep over
the repo returns matches only in docs/reviews/

2026-11-03 14:12:46 +00:00

4.5 KiB

Raw Permalink 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, while 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), so 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 and 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 Permalink Blame History Unescape Escape