alktunnels/tasks/tunnels/corrmiss-review.md

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

id	name	status	depends_on	scope	risk	impact	level	tags
tunnels/corrmiss-review	Review: the W1 gate surface before the consumer waves adopt it (the {W2, W3} fork's critical point)	completed	tunnels/corrmiss-gate-tests	narrow	low	phase	review	review checkpoint establisher review-003

Description

Review checkpoint before the parallel consumer waves (W2, alkssh, W3, alksocks) adopt the landed establisher variant: the surface these sessions build on must be right before two sibling repos burn sessions on it. Wire-stability context: the REPLY-side semantics (a typed channel:open_failed with the pre-pinned unknown_resource reason) are what the consumers ride: once alkssh ships the gate shape, the API surface is de-facto wire-stable for them.

Check:

• The API matches the ADR ink (the gate type, the registration signature, the error-mapping pin, the timeout, the no-scoped change)
• The doc comments' two-shape posture (establisher = recommended; base = compatibility + residual-race fallback) is consistent across make_forwarded_op_handler, the base/variant registrations, and lib.rs's re-export surface
• The tests actually pin the wire-observable contract (channel:open_failed / unknown_resource, gate ordering, residual-race reaping), not just internal shapes
• Patterns are consistent with the existing establisher shapes (the bindfirst_establisher mirror; the From<TunnelEstablishError> table reuse; no new error axes)
• cargo test, clippy native + wasm32 -D warnings, and fmt are green (re-confirm, they gate the review)
• CHANGELOG updated (Added: the establisher variant) if the implementation tasks did not already

Filing guidance: if the surface needs a shape change (naming, gate arity, plan payload), fix it in this checkpoint; it is the last cheap moment before the consumers fork.

Acceptance Criteria

• All checks above pass; any found shape drift fixed and re-verified before the consumer-wave tasks leave pending
• The review outcome recorded in this task's Notes (what was checked, what moved if anything)

References

• docs/plans/003-correlation-miss-resolve.md §Ordering (the W1 → {W2, W3} fork this review gates)
• docs/architecture/decisions/008-listen-metadata-forwarded-op.md (the amended ADR, the shape source)
• tasks/tunnels/corrmiss-adr.md, corrmiss-forwarded-gate.md, corrmiss-gate-tests.md (the W1 trio being reviewed)
• src/producer.rs (the landed surface)

Notes

Review performed 2026-11-03 (review checkpoint gating the W1 → {W2, W3} fork). Outcome: surface approved with no shape change needed; two doc-level fixes applied and re-verified.

• API vs the ADR ink (check 1): all pins hold.
  • The gate type ForwardedGate = Arc<dyn Fn(&TunnelForwardedParams) -> Result<(), TunnelEstablishError> + Send + Sync> matches the plan's candidate shape verbatim; the registration signature (core, on_registry, auth, gate, handler) -> Result<(), String> matches.
  • Error-mapping pin: gate Err passes through the existing From<TunnelEstablishError> for EstablishmentError table (src/producer.rs:142); no new variants are minted, and the gate doc pins UnknownResource as the canonical stale-token reason.
  • Establisher parse failure → HandlerError → typed channel:open_failed, deliberately pinned in the establisher doc (an improvement over the base shape's silent birth-teardown), exactly as corrmiss-forwarded-gate's decision 1 recorded.
  • Timeout: timeout: None (the 10s default) in the registration.
  • No scope-table change: the same tunnel_forwarded_spec() / tunnel:forwarded gate runs; the wrapper order pin (schema → ACL → allocate → establisher → handler) is in the establisher doc.
  • The plan payload is the pinned ForwardedPlan marker struct (the ()-alternative was rejected in the API task's Notes, as self-documenting, downcast-able; consistent with the typed-opaque posture of TargetHandle/BindFirstPlan).
• Two-shape posture consistency (check 2): present and consistent in make_forwarded_op_handler's correlation-miss block, the base registration's doc, the variant registration's doc, lib.rs's re-export surface (base + variant + ForwardedGate/ForwardedPlan all re-exported), plus the downstream syncs (wire.md §The Forwarded Op's two-shape prose, tunnel_forwarded_spec()'s serving registration block). ONE drift found and fixed: the handler doc's HEADER still read "the no-establisher serving shape" while it now serves both registrations, so it was retitled "the serving-shape wrapper both registrations share" (the two-shape block below it already carried the posture; this aligns the header).
• Tests vs the wire contract (check 3): the five tests in tests/producer_forwarded_gate.rs pin wire-observable behavior, not internal shapes: the miss asserts err.code == "channel:open_failed" and details["reason"] == "unknown_resource" plus no-phantom (no opener-side channels) and handler-not-run; the ordering test asserts FORBIDDEN naming the missing scope with the gate witness unmoved; the residual race asserts the call-plane RESOLVES then the channel reaps; the malformed-params delta asserts the typed handler_error reason and parse-before-gate. Each contract assertion is at the CallError/reply surface, and a wire change would break them.
• Pattern consistency (check 4): make_forwarded_gate_establisher mirrors the bindfirst_establisher closure shape (the Arc::clone-into-Box::pin pattern, the same Establishment::new(Arc::new(..) as ChannelPlan) construction); the error table is reused 1:1; no new error axes; poisoned-lock test helpers follow convention 2 (unwrap_or_else(|e| e.into_inner())); the private establisher + public registration split matches the crate-owned-surface rule (assemblies cannot hand-roll the alkcall plumbing).
• CHANGELOG (check 6): the implementation tasks did NOT update it (Unreleased was empty); added the "[Unreleased] / Added" entry for the establisher variant per this task's checklist.
• Verification (check 5), run at this review: cargo test green (14 suites, 0 failures, which includes the five gate tests), clippy --all-targets -- -D warnings clean (native), wasm clippy cargo clippy --target wasm32-unknown-unknown -- -D warnings clean, cargo check --target wasm32-unknown-unknown clean (default crate stays wasm-clean), cargo fmt --check clean, cargo test --all-features green (14 suites), cargo doc --no-deps clean. The wasm-clippy --all-targets deviation recorded in corrmiss-gate-tests' Notes is pre-existing and unchanged (verified there; not re-triangulated here).

Summary

Review checkpoint passed: the W1 gate surface (ForwardedGate, ForwardedPlan, register_tunnel_forwarded_openable_with_establisher) matches ADR-008 Amendment 2's ink on every pin (gate shape, registration signature, error mapping through the existing From<TunnelEstablishError> table, typed parse-failure posture, the 10s default timeout, the unchanged scope table); the two-shape posture is consistent across all docs and re-exports (one header drift retitled); the five gate tests assert the wire-observable contract at the CallError surface; patterns mirror bindfirst_establisher with no new error axes. Found and fixed: the stale "no-establisher serving shape" handler-doc header, and the missing CHANGELOG entry (Added under [Unreleased]). Re-verification all green (test, clippy native + wasm, fmt, wasm check, --all-features, doc). No shape change is needed, and the surface is ready for the consumer waves (W2 alkssh, W3 alksocks) to adopt.