- Review 003 written (docs/reviews/003-assembly-layer-implementation-
review.md): the batch-2 assembly layer post-landing, the review-002
template — correctness/security/smells/coverage over the five
batch-2 modules + the batch-2 consumer/producer diff surface.
Baseline: 375 default / 620 all-features green, clippy/fmt/doc
clean, both wasm checks clean, publish dry-run completes,
llvm-cov 93.68% lines / 91.01% functions.
- F-1 (major, fixed): the cred-bearing door swallowed every in-band
refusal's mapped reply code (the CONNECT/BIND session-open errors
and the accept-stage Err were bare closes) — a vanilla client got
no reply where ADR-007 pins a reply-code-shaped refusal. Fixed with
a write_refusal_reply helper (the door's boundary codec, RFC
placeholder BND.ADDR) at all three sites, pinned by
cred_door_in_band_refusal_surfaces_the_reply_code.
- F-2 (major, fixed): UdpFrontDoor::serve_on returned Ok(()) on a
fatal relay-socket I/O failure against its own doc contract —
the Io exit now surfaces as Err; the idle/association/session
exits stay clean.
- Findings recorded, not fixed (the release-polish round's residue):
D-1 (associate_with_token 0% covered), D-2 (the composed-egress
failure matrix), D-3 (the io-kind refusal-table arms), S-1 (the
UDP door's learned client address), N-1..N-8 (doc lines, posture
notes, coverage residue).
Verification: cargo test (375) + --all-features (620) green,
clippy --all-targets --all-features -D warnings clean, fmt --check
clean, doc --no-deps clean, both wasm checks clean, publish
dry-run completes.
The closing checkpoint (tasks/implementation/review-fix-round-002.md):
each review-002 finding verified closed against the landed tree, the
full verification sweep green, conventions re-swept, and the gate
condition (ADR-014 accepted, no major finding) holds.
Per-finding verification (all nine checklist points):
- C-1: the take-guard verified — a second await_inbound returns the
spent shape Ok(None), the re-read structurally impossible (the plane
owns the stream); pinned by the double-read + reply1-twice tests.
D-1's one-reader decision recorded (option (a), BindReply1 cut).
- C-3 closed: the builder's explicit bind_policy is the substrate-free
gate; the presence reading is only the backward-compat default; the
pump handler's identity-derived gate byte-identical.
- D-2 closed: the full builder chain through .register(...) serves all
three commands end to end (RegisterMode::ProducerBuilder + the
integration test, identity witness asserted).
- N-1/N-4 coherent: the cut list gone (zero references), keep-notes
current, the Refused vocabulary honest at all five sites + the
harness's minimal producer, wire untouched, provenance extended.
- D-3 resolved (declare side): the channel:open_failed ErrorDefinition
declared, schema pinned drift-free against the typed parse table.
- N-8 re-run: 96.30% regions / 97.28% lines; every named line covered;
the residual attributed (accept_bi log arm, fork accepted-uncovered,
test panic-guards) — no new load-bearing gaps.
- N-5/N-6 verified: the shared module compiles into both crates via
the crate-root mount + extern-crate-self alias; open_and_negotiate
single-sources the teardown-at-birth prologue.
- C-2 decided: ADR-014 accepted (option 1 — teardown-governed phases),
the spec lines carry the posture.
- Cross-cutting sweep green: test 375 green (+ all-features, local,
composed), clippy -D warnings clean, fmt clean, doc clean, both wasm
checks clean, publish dry-run completes; no unwrap/expect/panic in
non-test code (mechanically attributed across src/); no local/
composed code exists (the batch boundary holds).
One gate-round finding, F-1 (minor), fixed in this change: the
review-numbering collision — "review 002 R-1/R-2/R-4" cited the
batch-1 gate review's findings (tasks/implementation/
review-protocol-core.md) under a name that now means the general
review; reworded to "the gate review's R-N" at all six sites
(producer.rs, consumer.rs, egress.rs, the review doc). Review 002
marked resolved; the next general review doc is 003-*.
Verification: cargo test (375 green), --all-features + --features
local/composed green, clippy --all-targets -D warnings clean, fmt
--check clean, doc clean, both wasm checks clean, publish dry-run
completes, llvm-cov 96.30% regions / 97.28% lines attributed.
The general review (docs/reviews/002-protocol-core-implementation-review.md)
decomposed into the fix-round group, grouped so no session carries
both a code-fix vertical and the test sweep:
- fix-bind-reply2-reentry: C-1 (the BIND reply#2 re-entry guard) +
D-1 (the reader decision) + N-6 (the constructor prologue
helper) + S-2 (the reap doc line) — one consumer session
- fix-producer-gate-policy: C-3 (the explicit BIND gate input) +
D-2 (the register/builder coverage) + S-1/N-2 (the stage doc
lines) — one producer session
- fix-rfc1928-posture: N-1 (the keep-or-cut pass) + N-4 (the honest
refusal vocabulary) — one fork posture session
- fix-hygiene-residue: D-3 (the ErrorDefinition declaration) +
N-3 (the unreachable let-else) + N-9 (the is_mid_frame note)
- fix-coverage-residue: N-8's attributed line list — tests only,
after the code fixes (final shapes)
- fix-test-helper-consolidation: N-5 — after the coverage round
- architecture/adr-014-idle-timeout-posture: C-2 — the ADR-sized
decision, independent, before the doors exist
- review-fix-round-002: the closing checkpoint; the batch-2 gate
The review doc's disposition table now names the landed tasks.
N-7 is recorded-only (the review's own recommendation).
General review of the landed batch-1 protocol core (docs/reviews/
002-protocol-core-implementation-review.md), complementing the
narrow fidelity review: correctness hazards, security posture,
dead surface, and cargo-llvm-cov attribution, cross-checked against
alkcall 0.8.0 and the sibling crates.
Two majors:
- C-1: BindSession::await_inbound (and BindReply1::await_inbound)
is re-entrant with no spent-marker — a second call re-reads the
live data plane as reply#2, silently corrupting the pipe. Fix
sketched: inner.take() (the session's own None-arm posture) +
tests.
- C-2: no timeout surface on the pre-command RFC handshake or the
association (bounded memory, unbounded time; the alkcall
establishment bound covers only the open op). A decision to make
before batch 2's doors route vanilla clients through the same
phases; sketched as an ADR-level choice, not a core defect.
Minors: C-3 (the substrate-free can_bind gate conflates
factory-wired with allowed), D-1 (BindReply1::await_inbound is
spec'd, 0% covered, never called — the two-reader ambiguity), D-2
(Socks5Producer::register + the builder's egress/accept setters
unexercised — the spec'd assembly shape has no test), D-3
(error_schemas declared vec![] while params.rs's doc claims the
ADR-049/ADR-016 contract; alktty declares it), S-1 (the stage's
Ok(None) defense is livelock-safe but rate-unbounded — posture
note), N-1 (review 001's keep-or-cut triggers fired:
StandardAuthentication/skip_auth_this_is_not_rfc_compliant/
CheckResult impls are cut candidates; ConnectError/transfer/
wait_on_tcp stay as hand-port-map companions; Bug is now consumed).
Coverage (94.52% regions / 95.55% lines): the load-bearing gaps
attributed in N-8 — the two untested split surfaces, ASSOCIATE
into_inner's Ok arm, the datagram stage's three failure exits
(ReplyFrameRejected/EgressEnded/Io), read_reply2's version arm,
plus single-arm cosmetics.
Verified: cargo test 348 green; --all-features green; clippy
--all-targets -D warnings clean; fmt --check clean; cargo doc
--no-deps clean; both wasm checks clean; cargo publish --dry-run
completes; cargo llvm-cov 94.52/94.67/95.55 (regions/functions/
lines). No code changed in this review — findings documented for
the fix decomposition.
Security + code-smell + coverage review of the targeted fork as owned
code (the extraction review was narrow; this is the full pass).
Findings: 1 major (F-1: reply_success/reply_error write replies with
.write() — a partial-write hazard invisible to clippy's
unused_io_amount under the err_when-chaining idiom, executable-probe
verified; rfc1928.md's write_all rationale is false for these sites),
7 minors (2 security-posture notes, 2 extraction leftovers, 3
coverage gaps), 4 observations. No critical findings.
Verification: 38+32 tests, clippy (all-targets + wasm32) -D warnings,
fmt, doc, wasm check, publish dry-run, MSRV 1.88 native+wasm — all
green. Coverage 92.63% regions / 95.40% lines (cargo-llvm-cov); gaps
attributed per finding. Differential suite's 15 upstream citations
spot-checked against the v1.0.0 reference checkout — all accurate.
glm-5.3-flash