From 802d94ec073ffde0eeca6d64183c8a37097ce868 Mon Sep 17 00:00:00 2001 From: "glm-5.3-flash" Date: Sat, 5 Sep 2026 05:45:31 +0000 Subject: [PATCH] docs(review 007 Unit 2): WS-31 discovery-shadowing note + record corrections MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - WS-31: websocket.md §"Data channels for browsers" + ADR-067's landed note record that WS-session discovery is the bootstrap set — the hook's bootstrap `services/*` registrations overwrite a base-registry `services/*` registration on the WS path by design (a deployment's custom `services/list` is shadowed on WS sessions only). - ADR-048's landed note: correction + completion — the WS-26 retention sentence was aspirational at the landed commit (WS-28) and is now real; the UP-02 posture's override half is now an explicit surface (`with_ws_op_register_acl` / `OpRegisterAcl`), with the note that `ChannelsPolicy` could not carry an op ACL. - ADR-067's landed note: review-007 notes (WS-28 fix + gate, WS-29 surface, WS-31 record). - OQ-05 resolution: the retention claim carries the WS-28 correction. - review-006 UP-02 log + WS-26 paragraph: corrections marking what the pre-fix tree did not have, with the landed remediation named. - review-002 WS-17: the "bounded at 64 sessions" claim corrected — the bare-registry semaphore was per-request and bounded nothing; `SessionSlots` is the shared-cap surface. - review-007 status: open for remediation → remediated, with the decisions taken (both "implement" options) and the gate names. Verification: cargo test 454 passed / 0 failed; cargo doc --no-deps clean (6 pre-existing warnings, identical at baseline). Review: docs/reviews/007-ws-data-channel-surface-review.md --- ...048-websocket-native-session-not-gateway.md | 16 ++++++++++++++++ .../067-websocket-carries-channels.md | 12 ++++++++++++ docs/architecture/open-questions.md | 7 ++++++- docs/architecture/websocket.md | 11 +++++++++++ docs/reviews/002-post-remediation-review.md | 9 +++++++++ .../006-alkcall-0.3.0-consequence-review.md | 18 ++++++++++++++++-- .../007-ws-data-channel-surface-review.md | 18 +++++++++++++++++- 7 files changed, 87 insertions(+), 4 deletions(-) diff --git a/docs/architecture/decisions/048-websocket-native-session-not-gateway.md b/docs/architecture/decisions/048-websocket-native-session-not-gateway.md index 8f49920..5f65dba 100644 --- a/docs/architecture/decisions/048-websocket-native-session-not-gateway.md +++ b/docs/architecture/decisions/048-websocket-native-session-not-gateway.md @@ -51,6 +51,22 @@ Accepted > `AccessControl::default()` (the SRV-10 permissive-crate-default > precedent); deployments gate via a stricter `ChannelsPolicy` > passed through the hook (review 006 UP-02 decision). +> +> **Review-007 correction + completion (2026-09-05):** the retention +> sentence above was, at the time of the landed note, aspirational — +> review 007 WS-28 found the `ConnectionGuard` dropping at the end of +> its `if let` block (the handle visible for microseconds) and +> remediation bound it in the channel-0 task's frame; the retention is +> now real, gated by +> `live_connections_visible_mid_session_and_drain_after_teardown`. +> The ACL posture's second half is likewise now real as an explicit +> surface (the `ChannelsPolicy` object could not carry an op ACL): +> `HttpAdapter::with_ws_op_register_acl` / the `OpRegisterAcl` +> request extension set the `op/register` op's `AccessControl` — +> default unchanged (`AccessControl::default()`, any authenticated +> peer may announce); a deployment restricting which authenticated +> peers may announce threads a stricter value. The UP-02 posture +> stands; only the override surface it named now exists. ## Status amendment (alkhttp port) diff --git a/docs/architecture/decisions/067-websocket-carries-channels.md b/docs/architecture/decisions/067-websocket-carries-channels.md index 58ad880..b7ae7cc 100644 --- a/docs/architecture/decisions/067-websocket-carries-channels.md +++ b/docs/architecture/decisions/067-websocket-carries-channels.md @@ -119,6 +119,18 @@ On upgrade, the handler: > (WS-26). Gates: the six e2e scenarios in > `tests/ws_upgrade_session.rs` + the `services/list-peers` announced-op > discovery gate (alkcall 0.3.1). +> +> **Review-007 notes (2026-09-05):** WS-28 fixed the WS-26 retention — +> the `ConnectionGuard` now lives in the channel-0 task's frame (its +> drop removes the handle on any teardown path), verified by +> `live_connections_visible_mid_session_and_drain_after_teardown`. +> WS-29 implemented the `op/register` override surface the review-006 +> UP-02 posture named: `HttpAdapter::with_ws_op_register_acl` on the +> built-in surface, the `OpRegisterAcl` request extension on +> bare-registry routes (default still `AccessControl::default()`). +> WS-31 recorded: WS-session discovery is the bootstrap set — the +> hook's bootstrap registrations overwrite a base-registry +> `services/*` registration on the WS path by design. Everything ADR-048 says about dispatch — `call.requested` → `Dispatcher::dispatch_requested` with `AccessControl::check` gating, diff --git a/docs/architecture/open-questions.md b/docs/architecture/open-questions.md index 16b6596..15defd0 100644 --- a/docs/architecture/open-questions.md +++ b/docs/architecture/open-questions.md @@ -104,7 +104,12 @@ with their resolutions; new alkhttp OQs start at OQ-01. `op/register` (alkcall ADR-022 amendment), then dispatches over the fork — alkcall ADR-047 §4 amendment #2's per-session-fork shape. The session retains its live `Arc` in `WsSessions` - (evicted with the session). One premise of the original deferral + (evicted with the session; review 007 WS-28 corrected the landed + guard's scope — the handle was retained for microseconds, not the + task's lifetime — and its remediation bound the guard in the + channel-0 task's frame, gated by + `live_connections_visible_mid_session_and_drain_after_teardown`). + One premise of the original deferral was corrected along the way (review-003 WS-23): "reopened when a browser consumer lands" underestimated the consumer set — WS is also the native-client fallback behind hostile NAT/firewall, and a diff --git a/docs/architecture/websocket.md b/docs/architecture/websocket.md index 0616e93..b22a54d 100644 --- a/docs/architecture/websocket.md +++ b/docs/architecture/websocket.md @@ -285,6 +285,17 @@ overlay; discovery: `services/list` (the session's own surface) and `services/list-peers` (peer-announced ops, alkcall 0.3.1). E2E gates for the whole flow live in `tests/ws_upgrade_session.rs`. +WS-session discovery is the **bootstrap set**: the hook registers +alkcall's `services/list` / `services/schema` / `services/list-peers` +closed over the session's fork (so the listing sees the session's own +openables — the F-06 shape), which **shadows by design** any +`services/*` operation the deployment registered on its base registry +(`register` inserts by name, so the per-session fork's bootstrap +registrations overwrite the deployment's on the WS path). A deployment +with a custom `services/list` keeps it on every other transport and +gets alkcall's bootstrap listing on WS sessions — recorded here so the +asymmetry is a decision, not a surprise (review 007 WS-31). + ### Bidirectionality The WS channels session inherits both protocols' native diff --git a/docs/reviews/002-post-remediation-review.md b/docs/reviews/002-post-remediation-review.md index cf35f19..578ecbd 100644 --- a/docs/reviews/002-post-remediation-review.md +++ b/docs/reviews/002-post-remediation-review.md @@ -191,6 +191,15 @@ long-lived silent subscription deployment needs `None` and cannot get it. **Fix:** a `WsIdleTimeout`/`WsSessions` request extension mirroring `ChannelsPolicy`, or document the fixed values. +*(Correction, review 007 WS-30: the "bounded at 64 sessions" half was +wrong — the bare-registry `SessionState` is built by `FromRef` per +request, so its 64-permit semaphore is fresh per request and bounds +nothing across requests. The idle-timeout half above was the real +finding and landed as the `WsTimeouts` extension (WS-17's acceptance +gate). The session-cap half landed later as the `SessionSlots` +extension — the shared-cap surface a custom route inserts for an +effective bound.)* + ## WS-18 [minor] — Write-side stall is unbounded: a peer that stops reading parks the mux indefinitely **Verified:** YES. `byte_adapter.rs:584-597` — the idle knob covers diff --git a/docs/reviews/006-alkcall-0.3.0-consequence-review.md b/docs/reviews/006-alkcall-0.3.0-consequence-review.md index 2395cb5..8ad0f41 100644 --- a/docs/reviews/006-alkcall-0.3.0-consequence-review.md +++ b/docs/reviews/006-alkcall-0.3.0-consequence-review.md @@ -355,7 +355,14 @@ precedent: permissive crate default), and an assembly layer overrides via a stricter policy passed through the hook — the posture mirrors `ChannelsPolicy`. The override surface rides the existing `policy` parameter; the assembly layer's `ChannelsPolicy` value is the same -object the open wrappers consult. +object the open wrappers consult. *(Review 007 WS-29 correction: the +`ChannelsPolicy` object is an `Arc` — the +channel-cap policy — and cannot carry an operation `AccessControl`; at +this commit the hook hardcoded the permissive default and no override +surface existed. Remediation implemented the surface as decided: +`HttpAdapter::with_ws_op_register_acl` + the `OpRegisterAcl` request +extension, review 007 Unit 2. The posture above stands as recorded; +only the "rides the existing `policy` parameter" mechanism was wrong.)* **Openable-ALPN surface (WS-22).** `OpenableAlpn { spec, open_handler }` + `HttpAdapter::with_ws_openable_alpns(Vec<..>)`, @@ -369,7 +376,14 @@ no openables (channel 0 only). the channel-0 `Arc` under a self-removing `ConnectionGuard` (dropped when the channel-0 task ends — any teardown path). Shared only when a shared `WsSessions` instance is in -play (the bare-registry/test-server path passes `None`). +play (the bare-registry/test-server path passes `None`). *(Review 007 +WS-28 correction: at this commit the guard was bound inside the `if +let` block and dropped microseconds after insertion — the retention +sentence above described the intended scope, not the landed one. +Remediation bound the guard as an `Option` in the +channel-0 task's frame; the gate +`live_connections_visible_mid_session_and_drain_after_teardown` pins +it.)* **Policy threading.** The hook takes the session's resolved `ChannelsPolicy` (the extension resolution, `NoCap` default) and diff --git a/docs/reviews/007-ws-data-channel-surface-review.md b/docs/reviews/007-ws-data-channel-surface-review.md index b2d2eea..1cc1edb 100644 --- a/docs/reviews/007-ws-data-channel-surface-review.md +++ b/docs/reviews/007-ws-data-channel-surface-review.md @@ -2,7 +2,23 @@ ## Status -Open for remediation. +Remediated (2026-09-05). WS-28: the `ConnectionGuard` now binds in the +channel-0 task's frame (`Option`, the `SessionGuard` +shape), with the re-added repro gate +(`live_connections_visible_mid_session_and_drain_after_teardown`) — +verified to fail against the pre-fix tree and pass with the fix. The +decisions taken during remediation, both the "implement" options: WS-29 +landed the recorded override surface +(`HttpAdapter::with_ws_op_register_acl` + the `OpRegisterAcl` request +extension; builder + extension gates); WS-30 landed the `SessionSlots` +extension (the shared-cap surface for bare-registry routes, with the +review-002 WS-17 correction). WS-31: the discovery-shadowing sentence +is in (websocket.md §"Data channels for browsers", ADR-067's landed +note). WS-32: the builder-path openables gate +(`builder_path_openables_serve_the_data_channel_surface`). The +review-006 UP-02 / WS-26 log entries and ADR-048's landed note carry +corrections where the pre-fix code did not match the recorded claim. +Verification: the standard gate set (below), extended to the new gates. ## Scope