From c7e8c875fdcb8ad844375ecb3c2ee7b62c19a623 Mon Sep 17 00:00:00 2001 From: "glm-5.3-flash" Date: Tue, 3 Nov 2026 12:42:02 +0000 Subject: [PATCH] review(tunnels/corrmiss-review): the W1 gate surface approved before the consumer waves fork - API vs ADR-008 Amendment 2 ink: all pins hold (ForwardedGate shape, registration signature, error mapping via the existing From table, typed parse-failure posture, timeout: None, unchanged scope table); ForwardedPlan marker matches the API task's pinned payload choice - Two-shape posture consistent across docs/re-exports; one drift fixed: make_forwarded_op_handler's doc header retitled from "the no-establisher serving shape" to "the serving-shape wrapper both registrations share" - CHANGELOG: Added entry under [Unreleased] for the establisher variant (the implementation tasks left it unrecorded) - Five gate tests re-checked to pin the wire-observable contract at the CallError surface (channel:open_failed / unknown_resource / FORBIDDEN ordering / residual-race reap / malformed-params delta) - Verification (re-run at review): cargo test green (14 suites), clippy --all-targets -D warnings native clean, wasm clippy + check clean, fmt clean, --all-features green, cargo doc --no-deps clean - Task status -> completed; Notes/Summary record the outcome. The surface is ready for the W2 (alkssh) / W3 (alksocks) adoptions. --- CHANGELOG.md | 4 ++ src/producer.rs | 3 +- tasks/tunnels/corrmiss-review.md | 104 +++++++++++++++++++++++++++---- 3 files changed, 99 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 779a2f8..450074d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ this crate adheres to [Semantic Versioning](https://semver.org/). ## [Unreleased] +### Added + +- **The forwarded-op establisher variant (`register_tunnel_forwarded_openable_with_establisher` + `ForwardedGate` + `ForwardedPlan`, ADR-008 Amendment 2)** — the serving side of `channels/tunnel/forwarded` gains an establishment phase: the assembly injects a correlation gate (`ForwardedGate` — a sync closure over the parsed `TunnelForwardedParams`) and the crate wraps it into the alkcall `OpenEstablisher` dance, so a stale/missing `listen` token fails the open as a typed `channel:open_failed` with `details.reason == "unknown_resource"` BEFORE any channel is born (no phantom channel — ADR-049's posture extended to the forwarded op's serving side; the wire vocabulary is unextended). An establisher parse failure is likewise typed (`handler_error`) — an improvement over the base shape's success-then-silent-birth-teardown for malformed ingress. Gate order pinned: the registry runs the `tunnel:forwarded` ACL before the establisher (pinned by test with a gate-invocation witness). The base `register_tunnel_forwarded_openable` stays byte-for-byte compatible as the compatibility + residual-race-fallback posture (the narrow gate-pass→handler-run `correlation-miss` race keeps teardown-at-birth); the establisher variant is the recommended shape and new assemblies take it by default. The wire delta is empty. Five gate tests pin the typed miss/no-phantom/no-handler, the unchanged gate-pass serving flow (full byte round-trip), the residual-race reap, the scope-gate ordering, and the malformed-params delta. + ## [0.4.1] - 2026-10-22 The dependency-patch release: the alkcall substrate bumped 0.10.0 → diff --git a/src/producer.rs b/src/producer.rs index e7a3b93..ce93e29 100644 --- a/src/producer.rs +++ b/src/producer.rs @@ -983,7 +983,8 @@ pub type ForwardedHandler = Arc tokio::task::JoinHandle<()> + Send + Sync>; /// The forwarded op's opener-side pump handler (ADR-008 §3, the -/// no-establisher serving shape): parse `TunnelForwardedParams` +/// serving-shape wrapper both registrations share): parse +/// `TunnelForwardedParams` /// (`deny_unknown_fields` + the positive-`listen` serde pin — a /// malformed open fails the parse LOUD; the wrapper then returns early /// and the channel tears down at birth — no phantom channel, the diff --git a/tasks/tunnels/corrmiss-review.md b/tasks/tunnels/corrmiss-review.md index 7a95db7..a270dfa 100644 --- a/tasks/tunnels/corrmiss-review.md +++ b/tasks/tunnels/corrmiss-review.md @@ -1,7 +1,7 @@ --- id: tunnels/corrmiss-review name: "Review: the W1 gate surface before the consumer waves adopt it (the {W2, W3} fork's critical point)" -status: pending +status: completed depends_on: [tunnels/corrmiss-gate-tests] scope: narrow risk: low @@ -22,22 +22,22 @@ API surface is de-facto wire-stable for them. Check: -- [ ] The API matches the ADR ink (the gate type, the registration +- [x] 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; +- [x] 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 +- [x] 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 +- [x] Patterns are consistent with the existing establisher shapes (the `bindfirst_establisher` mirror; the `From` table reuse; no new error axes) -- [ ] `cargo test`, clippy native + wasm32 `-D warnings`, fmt — green +- [x] `cargo test`, clippy native + wasm32 `-D warnings`, fmt — green (re-confirm, they gate the review) -- [ ] CHANGELOG updated (Added: the establisher variant) if the +- [x] CHANGELOG updated (Added: the establisher variant) if the implementation tasks did not already Filing guidance: if the surface needs a shape change (naming, gate @@ -46,9 +46,9 @@ moment before the consumers fork. ## Acceptance Criteria -- [ ] All checks above pass; any found shape drift fixed and +- [x] 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 +- [x] The review outcome recorded in this task's Notes (what was checked, what moved if anything) ## References @@ -63,8 +63,90 @@ moment before the consumers fork. ## Notes -> To be filled by implementation agent +Review performed 2026-11-03 (review checkpoint gating the W1 → {W2, +W3} fork). Outcome: **surface approved — 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 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 for EstablishmentError` table + (`src/producer.rs:142`) — no new variants minted; 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 — + 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 — 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 — 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 — 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 -> To be filled on completion \ No newline at end of file +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` 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 needed — +the surface is ready for the consumer waves (W2 alkssh, W3 alksocks) +to adopt. \ No newline at end of file