alksocks/tasks/implementation/fix-hygiene-residue.md

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

id	name	status	depends_on	scope	risk	impact	level
implementation/fix-hygiene-residue	Fix: error-schema declaration + small hygiene residue (review-002 D-3, N-3, N-9)	completed		narrow	low	component	implementation

Description

The small mechanical residue that does not ride either code-fix task: one doc-vs-code drift with a real fix, plus two one-line postures (docs/reviews/002-protocol-core-implementation-review.md):

1. D-3 (minor): error_schemas is vec![] while the module doc claims the ADR-049 §4 + ADR-016 contract (src/params.rs:94-96 vs :117). The typed vocabulary works behaviorally (SocksOpenError::reason parses details.reason off the wire, tested), but a discovery consumer reading error_schemas from the registry sees no declared failure contract for channels/socks5/sub, and the doc describes a contract the spec does not carry.

   Fix per the review's lean: declare the ErrorDefinition, porting alktty's (the first protocol crate's precedent, /workspace/@alkdev/alktty/src/channels.rs:165-208: the channel:open_failed definition with the { reason, message } schema, pinned by a test asserting the declaration at :790). One ADR-cited block; the schema is already written there. Adapt the description text to this crate's establishment story (the establisher's refusal reasons per ADR-049 §3 + the registry's FORBIDDEN class). One test mirrors alktty's declaration pin. If the declare-side turns out to hit a substrate constraint (the registry rejects the definition, say), fall back to the honest reword ("empty, since the typed contract is behavioral via SocksOpenError::reason; the ErrorDefinition pass is crate-family-wide, matching alktunnels, pending") and record why.

2. N-3 (observation): DatagramReader::feed's let-else at src/wire.rs:229-231 is structurally unreachable (0 executions; the block above always sets current before the let-else when buf.len() >= 2, and the loop broke at :223 otherwise). Restructure per the review's sketch: match on the already-known-Some state, or re-order to drop the else. Zero behavior change; the full wire suite stays green unchanged.

3. N-9 (observation): is_mid_frame is public but consumed nowhere outside the codec's own tests (src/wire.rs:253-260). Keep (a batch-2 front-door bridge driving its own reader loop would use it), and add one doc line naming the anticipated consumer ("the doors") so the next dead-surface sweep does not flag it without this note.

Acceptance Criteria

• socks5_open_spec declares the channel:open_failed ErrorDefinition (the alktty-ported schema, adapted), or the doc is reworded to the honest empty posture with the reason recorded
• A test pins the declaration (mirroring alktty's), or the reworded doc matches the spec exactly
• The feed let-else is restructured; zero behavior change (the wire suite green unchanged)
• is_mid_frame's doc names the anticipated consumer
• cargo test green; clippy/fmt clean; both wasm checks clean; cargo doc --no-deps clean

References

• docs/reviews/002-protocol-core-implementation-review.md §D-3, §N-3, §N-9
• alkcall ADR-049 §4 + ADR-016 (the typed error contract: /workspace/@alkdev/alkcall/docs/architecture/decisions/)
• alktty precedent: /workspace/@alkdev/alktty/src/channels.rs:165-208 (the ErrorDefinition + schema) and :790 (the pinning test)
• alktunnels sibling: /workspace/@alkdev/alktunnels/src/params.rs (the empty-vec posture, the fallback's precedent)

Notes

• D-3: declared, not reworded. The review's lean (the declare side) worked with no substrate constraint: the registry accepted the definition on the first try. The schema is alktty's channel:open_failed block (/workspace/@alkdev/alktty/src/channels.rs:165-208), adapted: the reason enum carries all five ADR-049 §3 codes (dial_failed / unknown_resource / resource_shortage / handler_error / timeout); alktty's enum has four because its establisher has no resource_shortage path, while this crate's declared vocabulary is chosen to match SocksEstablishmentReason's parse table exactly (the typed view a consumer branches on), not the current default establisher's reachable subset (socks5_session_establisher is a session gate returning Establishment::default(); a composed/registry establisher could produce any of the five). The description text is rewritten to this crate's establishment story: the registry gate classes first (FORBIDDEN / INVALID_INPUT / channel:too_many_channels; the ACL runs before the establisher, ADR-006), then the establishment vocabulary, then the no-phantom-channel property. One judgment call worth noting for the review round: the declared enum is wider than the default producer's behavioral surface (which produces none of the five today, because the dial is in-stream, post-open). That is the correct direction to over-declare: error_schemas discloses the op's contract shape (any registration of this op, per ADR-049 §4), and the enum is the wire's parse table (SocksEstablishmentReason::parse), so a stricter enum would under-declare for custom-establisher registrations.
• The pinning test cross-checks schema vs typed vocabulary. Beyond mirroring alktty's declaration pin (:780-807), open_spec_declares_the_open_failed_error_definition asserts each enum value round-trips through SocksEstablishmentReason::parse/as_str, so the declared schema and the typed vocabulary cannot drift silently (the exact D-3 drift class, pinned shut).
• N-3: restructured to a live-else shape, not the review's sketch. Both of the review's alternates were considered: the let Some(&mut (declared, ref mut acc)) match has the same dead-arm problem (it must fire after the header read, which is exactly the unreachable region), and the re-order-to-drop-else sketch inverts the loop's exit order. The landed shape folds the header read into the let-else's else arm (let Some((declared, acc)) = self.current.as_mut() else { <read header; continue; }), where every branch is live, the "current set above" invariant is structural (the else arm is the only place current is set), and the body is untouched. Zero behavior change: the full wire suite passes unmodified.
• N-9: kept with the note. One doc paragraph naming the anticipated batch-2 consumer (a front-door bridge driving its own reader loop, i.e. the doors) and citing the review finding.

Summary

All three residue items closed in one pass (src/params.rs, src/wire.rs):

1. D-3: socks5_open_spec now declares the channel:open_failed ErrorDefinition (an alktty-ported schema ({ reason (enum of the five ADR-049 §3 codes), message }, both required), description adapted to this crate's gate classes
   • establishment story. Module doc updated from claim to fact. Pinned by open_spec_declares_the_open_failed_error_definition (declaration + enum round-trips through SocksEstablishmentReason, pinning schema and typed vocabulary drift-free).
2. N-3: DatagramReader::feed's structurally-unreachable let-else restructured into a live-else shape (header read in the else arm); zero behavior change: the wire suite green unchanged.
3. N-9: is_mid_frame kept; its doc names the anticipated consumer (the batch-2 front-door bridges) so future dead-surface sweeps don't flag it.

Verification: cargo test 357 passed / 0 failed (all suites); cargo clippy --all-targets -- -D warnings clean; cargo fmt --check clean; cargo doc --no-deps clean; cargo check --target wasm32-unknown-unknown clean; cargo clippy --target wasm32-unknown-unknown -- -D warnings clean. Doc-vs-code drift is gone either way (review-002 D-3 closed; the review-round gate item "the ErrorDefinition is declared" holds).