diff --git a/docs/reviews/011-pub-open-remediation-input-from-alkblobs.md b/docs/reviews/011-pub-open-remediation-input-from-alkblobs.md new file mode 100644 index 0000000..59736af --- /dev/null +++ b/docs/reviews/011-pub-open-remediation-input-from-alkblobs.md @@ -0,0 +1,230 @@ +# Review 011 — The marked-pub open contract: downstream input on review 010's S-01, and what the single-publish fix must pin + +## Status + +Open — filed 2026-10-13. From alkblobs (the downstream crate paused on +review 010 S-01; its ops-put task gates on this surface). Purpose: +correct review 010's S-01 remediation sketch before the amendment is +drafted, enumerate what the amendment must pin so one publish +(0.9.0) fixes all four stubs right, and record the downstream +requirements and non-requirements from the consumer seat. + +Input basis: alkblobs ADR-014 +(`/workspace/@alkdev/alkblobs/docs/architecture/decisions/ +014-marked-pub-put-contract-and-alkcall-gate.md`, committed 7ac0099) +— the reviewed downstream ruling this review summarizes the +requirements of. + +--- + +## 1. S-01: the remediation is "implement", but the review's option-1 sketch is the wrong shape — amend the review text + +Review 010's S-01 poses two remedies. Option 2 (reject at +registration) is honest but strands the downstream: alkblobs' put is +the real consumer of the marked-pub shape, and its settlement was +"implement, in the adoption shape". The one publish should take +option 1. But **option 1's sketch is wrong for marked ops** and +would, if implemented as sketched, ship a second incoherence this +ADR cycle exists to end: + +> "hand the `OpenHandler` the `PublishStream` already produced by the +> serving loop along with the channel's `Connection`" + +That reads ADR-046's **unmarked** Pub semantics (`call.published` +JSON chunks through `publish_schema` validation) into the **marked** +case. ADR-047 §2's marker sentence says the opposite: *"the +`channel_open` marker that tells the channels layer 'this op's +stream is binary, not JSON' — allocate a channel for it."* A marked +Pub op whose bulk bytes rode `call.published` would be the worst of +both stacks: bulk binary re-validated per-chunk as JSON against +`publish_schema`, MB-scale payloads through the JSON event grammar, +and no bounded-buffer backpressure (ADR-040's bounded buffer is +precisely the mechanism that exists for this). Section 010's +"tension" paragraph (stub comment vs ADR-047 §1's `call.published` +prose) resolves the same way: §1/AAD-046's `call.published` prose +describes *unmarked* Pub ops; the *marked* case's stream is the +allocated channel's BiStream. + +The settled shape (agreed with the downstream's architecture +session; recorded as alkblobs ADR-014 §1/§2/§3): + +1. **The initiator allocates and writes immediately.** + Connection-owner allocates the `channel_id` (ADR-047 §5; connect + side → odd per the implemented odd/even split, + `src/channels/manager.rs` `next_channel_id`), opens the write + half on it, and streams bytes before the open op resolves. The + existing early-arrival parking (`drain_early_arrivals`, + `src/channels/manager.rs`) is the established mechanism covering + the responder-adopts race — implemented, tested, no new + machinery. +2. **The responder adopts.** The channel ID crosses the open + invocation as a wrapper-reserved field. The reserved-field + placement question — reserved input key vs envelope field — is + decided by schema-validation ordering: validation runs in + `invoke_*` before any handler, so a reserved *input* key must be + stripped in the dispatch sink branch (the layer that reads the + `channel_open` marker) before `input_schema` validation; an + envelope field would extend the call event grammar for one op + family. Recommendation: **reserved input key + dispatch-layer + strip**, symmetric with amendment 3's reply-side + `RESERVED_REPLY_KEY` reservation — the input side of the same + pattern. Either way, the exact key name and the reject-not-strip + posture for a *marked* op are wire-visible from the first + consumer (alkblobs' put; not yet shipped — it pends behind this + amendment, so this is the cheapest moment to pin). +3. **The wrapper is the responder of record.** Flow: + `policy.check_open` → adopt → bounded establisher (offer + validation, establishment reply fields — ADR-049 amendment 3's + mechanism, unchanged) → spawn the `OpenHandler` on the channel's + `Connection` → **await the handler's `JoinHandle`** → respond + once: the `{channel_id}` output extended with the establisher's + reply fields and the handler-completion result merged (R-02's + lifetime contract is load-bearing here — the handle must track + the data plane). A failed establishment tears the channel down + and replies `channel:open_failed` (the only response); initiator + bytes racing the torn-down channel hit EOF and are discarded. +4. **Zero `call.published` data on the call plane for marked pub + ops** — a nonempty publish-chunk stream on a marked op is + rejected loudly, never consumed. Two competing data paths is the + incoherence that must never exist. +5. **Relay lifting:** `PubTypedOpen` rejection + (`src/channels/relay.rs`) is replaced with real relay + translation for pub openables (the hub's put-forwarding shape). + +The corollary this implies downstream (also pinned in alkblobs +ADR-014 §2): **the put's establishment ack is informative-in-result, +not a gating ack.** A sink op gets exactly one `call.responded` +(ADR-046 §5), produced after the handler completes — so a putter +cannot wait for the ack before streaming (a wait deadlocks: the +response cannot exist until the stream completes). The fetch/put +asymmetry is direction-inherent: fetch (`Sub`) acks at +establishment before any byte; put (`Pub`) acks in the result. The +amendment should state this sentence, not leave it for each +consumer to derive — it is exactly the kind of half-stated +implication that review 010 flagged as drift fuel. + +## 2. What amendment 4 must pin (the publish checklist) + +All of these are wire-visible or API-visible from the first +consumer; none is safely deferrable to a later amendment while +put-shaped consumers exist: + +1. **Marker semantics** — the marked Pub op's stream is the adopted + channel's `BiStream` (ADR-050 pump patterns, one direction); + zero `call.published` (§1.4 above; rejection is the wire + behavior). +2. **`publish_schema` on a marked op is refused at registration** — + publish chunks never exist, so the field is dead machinery on a + marked spec; the same class review 010 outlaws (registered but + incapable of mattering). Registration-time loudness, per the + house pattern. +3. **The reserved-field wire form** for the adopted channel id + (input-key + dispatch-strip recommended, §1.2) — key name and + strip-before-validation ordering pinned in the ADR; one-way from + the first consumer. +4. **The wrapper-as-responder-of-record flow** (§1.3) — response + merge order (`channel_id` reserved → establisher reply fields → + handler result), single-response semantics, teardown-on-fail. +5. **The ack-timing sentence** (§1 corollary) — put acks in-result; + consumers stream without waiting. +6. **Relay translation for pub openables** (§1.5) — no + `PubTypedOpen` remains. +7. **The client-side initiator-allocates put-open capability.** + Nothing in `ChannelClient` supports stream-then-open today: + `open_channel`/`open_channel_with_reply` await the open + *reply* before returning the halves — unusable when bytes flow + before the reply. The amendment supplies the putter-facing + shape (allocate odd id locally → write half streaming + immediately → send the open op carrying the reserved id → await + the single merged response). Without this the server-side + wrapper is untestable end-to-end in-crate. + +## 3. Verification gates for the S-01 implementation + +To be landed as tests in the same publish (deleting the pinned stub +tests per review 010's action 3 — +`make_open_handler_sink_returns_not_implemented`, +`relay_tests.rs:113`, and the C-08/C-09 blocker-comment assertions +they assert): + +- known-length put e2e: offer + adopt + bytes → one response, + `{channel_id, ack fields, handler result}` merge order verified; +- unknown-length put e2e: the handler's digest rides the merged + response (the downstream putter's wire contract, ADR-012 §2's + `{token, digest}`); +- publish-chunk rejection: a marked pub op carrying nonempty + publish chunks → loud rejection, no handler consumption; +- establishment failure: `channel:open_failed`, channel torn down, + ledger un-incremented, racing bytes EOF-discarded; +- `publish_schema` registration refusal on a marked op; +- relay translation: a hub translating a pub openable end-to-end + (the ADR-051-§6 gap closed). + +## 4. S-02 / S-03 / S-04 from the downstream seat + +No alkblobs requirement rides any of the three — the ops module +rides neither `channel/control` nor `channel/resources/subscribe` +nor `op/register`-announced streaming. Each remedy is downstream-safe: + +- **S-02/S-03**: implement-or-stop-advertising per the ruling; + implementing both is preferred (the publish is one 0.x bump + either way, and re-advertising later would be a second bump). + What the downstream needs: nothing. Do not couple the S-01 shape + to these designs (the OQ-39 control-handle and OQ-40 enumerator + designs are independent machinery). +- **S-04**: forwarding-through + (`make_streaming_forwarding_handler` / + `make_sink_forwarding_handler` already exist) over rejection — + the rejection leaves a discoverable-but-unusable service + announcement, which is the same consumer-visible failure mode + the ruling targets, one layer over. + +## 5. The four published crates' impact (the same-publish update) + +No published consumer registers a Pub-typed openable (alktunnels' +four openables are Sub-type; alkhttp has no openables in its +default surface; alkty rides Sub + control-JSON; alksocks rides +Sub) — **the S-01 implementation is capability-additive in +practice**. The semver-relevant risk concentrates in what the +publish *removes*: + +- S-02/S-03 implemented → additive, zero removals, the four crates + re-verify and bump their floors only. +- S-02/S-03 stop-advertised → two discoverable ops disappear; + acceptable per the ruling, but a 0.x removal worth a changelog + line for every consumer. +- S-01/§2's wire pins (reserved key, reject-chunks) carry no + shipped consumer; the first consumer (alkblobs' put) pends + behind the amendment — ADR-014 §3's gate — and floors up to the + amendment release at task start. + +## 6. Sequencing for the one publish + +1. The four amendment documents (ADR-047 amendment 4; the + OQ-39/OQ-40 promotions; the S-04 pin) — drafted for review + before code, per the working agreement. +2. Implementation + the §3 gates + the review-010 action-3 guard + (`not_implemented` source-tree grep wired into AGENTS.md's + release checklist; the four pinned stub tests deleted). +3. Docs: OQ-39/40 resolved, C-08 closed, channel-operations.md's + hedge-era sentences rewritten, review 010's S-01 sketch + corrected to this review's shape (so the published record stops + carrying the wrong-graph reading). +4. One publish, all downstream floors bump in it. + +## References + +- review 010 (the stub sweep this review amends the S-01 path of) +- alkblobs ADR-014 (`014-marked-pub-put-contract-and-alkcall-gate.md`) + — the downstream ruling; its §2 pins the ack-timing asymmetry +- alkblobs tasks/ops-put.md — the paused task; its Notes record the + gate and the two timing clarifications +- ADR-040 (bounded buffer — the backpressure the data plane rides), + ADR-046 (the pub lifecycle's single response), ADR-047 §2/§5 + (the marker; connection-owner allocates), ADR-049 + amendments + 2/3 (establishment, plan, reply fields; the RESERVED_REPLY_KEY + pattern the input-side reservation mirrors), ADR-050 (pumps) +- ADR-051 §6 (the relay-side documentation of the same gap) +- `src/channels/manager.rs` (`next_channel_id` odd/even; + `drain_early_arrivals`), `src/channels/operations.rs` (the stub, + the wrapper), `src/channels/relay.rs` (`PubTypedOpen`) \ No newline at end of file