docs(review 001): decompose Unit 6 into Units 6-10
The original Unit 6 was the largest unit by item count and mixed
trivial doc fixes with real protocol-semantics work (P-03/P-04/P-06/
P-07) in one commit. Decomposed into five focused units, each
independently shippable, ordered by risk and dependency:
- Unit 6 — convention + doc cleanup (C-09, C-20-rem, C-22-rem, C-24,
P-10, P-11): mechanical; `cargo doc` clean, producer/consumer
naming, ADR-046 §3 text amendment, EVENT_* constants in pump_sink.
- Unit 7 — small correctness fixes (C-19-misc, C-21, C-23, P-13):
write_header returns Result not panic, honest error envelopes,
mux pump map leak, derive_alpn_from_op_name dead branch, Box::leak
Arc<str> refactor, REQ-CH-04 counter, opaque-wrap chunk_tx.
- Unit 8 — Pub protocol semantics (P-03, P-04, P-07): publish_schema
per-chunk validation via alktype, initiator call.error terminates
the stream, early handler return short-circuits the feed.
- Unit 9 — abort-cancels-Pub (P-06): drop the handler future on
abort (not just join! to completion); needs a cancellation token
per in-flight sink; depends on Unit 8's pump_sink changes.
- Unit 10 — stubs + substrate + doc renumber (C-10, C-11, C-14,
C-15, C-26): honestly stub the deferred behaviors with OQs, remove
stale doc claims, renumber alknet ADR refs to alkcall ADR-001..047.
Also records which original Unit 6 items were already absorbed by
Units 1-5 (P-09, C-20's reassembly deliberation, C-22's env tautology,
C-19's next_id/demux_sender/mux-error-mapping) so the units below
cover only what remains.
Verified 2026-08-13 against tree f25d0a6 (post-Units 1-5): every
remaining item re-checked in source; the `cargo doc` warning count
(2, down from 4 — the register_openable links were fixed in Unit 3)
and file:line references updated to the current tree.
This commit is contained in:
glm-5.2 committed
1 parent
f25d0a6920
commit
48564a8f49
1 file changed
+302
-79
@@ -953,91 +953,314 @@ promises.
|
|||||||
- Test: C-25 #4 #5 — open-at-cap-then-drop-transport-then-reopen, and
|
- Test: C-25 #4 #5 — open-at-cap-then-drop-transport-then-reopen, and
|
||||||
the concurrent-open race test.
|
the concurrent-open race test.
|
||||||
|
|
||||||
## Unit 6 — Cleanup: comments, doc links, stale spec docs, stubs (C-09, C-10, C-11, C-14, C-15, C-19, C-20, C-21, C-22, C-23, C-24, C-26, P-03, P-04, P-06, P-07, P-09, P-10, P-11, P-13)
|
## Units 6-10 — the post-integration long tail
|
||||||
|
|
||||||
**Goal:** conventions satisfied, docs consistent, the remaining
|
The original Unit 6 was the largest unit by item count and mixed
|
||||||
decided-but-unimplemented behaviors either implemented or explicitly
|
trivial doc fixes with real protocol-semantics work (P-03/P-04/P-06/
|
||||||
deferred with an OQ.
|
P-07). It has been decomposed into five focused units, ordered by risk
|
||||||
**Scope:** This is the largest unit by item count but the lowest risk;
|
and dependency. Several original Unit 6 items were already absorbed by
|
||||||
do it last, after the integration is end-to-end functional. Items:
|
Units 1-5 during their implementation:
|
||||||
- **C-20:** remove the ~50 lines of abandoned deliberation in
|
|
||||||
`reassembly.rs:205-255` and the pervasive inline `//` comments
|
|
||||||
across the channels module that aren't carrying a non-obvious
|
|
||||||
correctness constraint. Fix the actively-wrong comments (mux EOF
|
|
||||||
claim, abort-cancels claim, wrong-side peer-EOF claim, "SAFETY:"
|
|
||||||
comment marking no `unsafe`).
|
|
||||||
- **C-09:** fix the remaining `cargo doc` warnings (`default_policy`
|
|
||||||
link, `env is both a module and a macro`).
|
|
||||||
- **C-26:** update `channel-client.md`, `channels-wire.md`,
|
|
||||||
`channels-adapter.md` to the post-047 model (remove
|
|
||||||
`open_channel(..., direction)`, `ChannelDirection`,
|
|
||||||
`ResourceEntry.access`, the generic `channel/open` handler,
|
|
||||||
`channel:unknown_alpn`; renumber stale alknet ADR refs to alkcall
|
|
||||||
ADR-001..047).
|
|
||||||
- **C-21:** make `write_header` return `Result<_, ChunkError>` instead
|
|
||||||
of panicking.
|
|
||||||
- **C-22:** remove the filler tests (`client.rs:131-136`, `env.rs:155`)
|
|
||||||
or replace them with real assertions.
|
|
||||||
- **C-23:** give `dispatch_requested`'s Sink/Stream error envelopes a
|
|
||||||
real request id (or document why empty is correct).
|
|
||||||
- **C-24:** replace "client→server streaming" in
|
|
||||||
`call-protocol.md:335` with producer/consumer phrasing; amend
|
|
||||||
ADR-046 §3 to match §6 (P-11).
|
|
||||||
- **C-19 misc:** fix `next_id` wrap, the unbounded resources-snapshot
|
|
||||||
loop, the mux pump map leak / duplicate registration, the wrong
|
|
||||||
`ManagerError` mapping on mux register failure, the misleading
|
|
||||||
`drop(state.demux_sender.clone())`, the `derive_alpn_from_op_name`
|
|
||||||
multi-segment bug + dead branch, the `Box::leak` `Arc<str>` refactor,
|
|
||||||
the `policy.rs:127-140` check-and-reserve rollback trap (document or
|
|
||||||
add a rollback API), the `Demux::stats()` error counter (REQ-CH-04).
|
|
||||||
- **P-03:** implement `publish_schema` per-chunk validation using
|
|
||||||
`alktype` (the dependency is already declared and unused), and wire
|
|
||||||
it through `spec_to_json` / `operation_spec_schema()` /
|
|
||||||
`rebuild_spec_for` so imported Pub ops keep the schema. Or, if
|
|
||||||
deferred, file an OQ and remove the misleading doc comment at
|
|
||||||
`spec.rs:181-186`.
|
|
||||||
- **P-04:** make `pump_sink` match `call.error` and inject the
|
|
||||||
initiator's `CallError` as an `Err` item that terminates the stream
|
|
||||||
(per ADR-046 §6).
|
|
||||||
- **P-06:** implement abort-cancels-Pub. This likely needs the
|
|
||||||
single-stream call mode from Unit 2 (so an abort on the same stream
|
|
||||||
can reach the running pump) plus a mechanism to drop the handler
|
|
||||||
future on abort (the doc comments at `dispatch.rs:494-498` describe
|
|
||||||
the target shape). Decide whether to fix the cross-stream path
|
|
||||||
(`abort()` opening a new stream) or deprecate it in favor of
|
|
||||||
same-stream abort.
|
|
||||||
- **P-07:** fix the handler-returns-early stall — when the handler
|
|
||||||
returns, the feed should be short-circuited and the response written
|
|
||||||
immediately (don't `join!` to completion). Use `tokio::select!` or
|
|
||||||
feed the handler's completion back into the feed loop.
|
|
||||||
- **P-09:** fix `make_sink_forwarding_handler` to stream (not
|
|
||||||
`collect`), and to terminate on `Err` (not `filter_map` it away).
|
|
||||||
Depends on P-01/P-02.
|
|
||||||
- **P-10:** use the `EVENT_*` wire constants in `pump_sink` instead of
|
|
||||||
string literals.
|
|
||||||
- **P-13:** opaque-wrap `SinkDispatch::chunk_tx` before crates.io.
|
|
||||||
- **C-10 / C-11:** either implement `channel/control` routing and live
|
|
||||||
`resources/subscribe`, or file OQs and make the stubs honest (return
|
|
||||||
`unimplemented`-style errors, not fake success).
|
|
||||||
- **C-14 / C-15:** either implement the QUIC-native substrate and the
|
|
||||||
full `ChannelClient` API (`open_channel`, `Channel`,
|
|
||||||
`subscribe_resources`), or file OQs and remove the misleading doc
|
|
||||||
comments / nonexistent method references.
|
|
||||||
|
|
||||||
## Suggested sequencing
|
- **P-09** fell out of Unit 1's P-01/P-02 streaming rework
|
||||||
|
(`make_sink_forwarding_handler` now streams + terminates on `Err`).
|
||||||
|
- **C-20's** biggest item — the ~50 lines of abandoned deliberation in
|
||||||
|
`reassembly.rs` — landed with Unit 4's backpressure rewrite
|
||||||
|
(reassembly.rs was rewritten from scratch).
|
||||||
|
- **C-22's** `env.rs:155` tautology test was removed in Unit 3.
|
||||||
|
- **C-19's** `next_id` wrap-to-0, the misleading
|
||||||
|
`drop(state.demux_sender.clone())`, and the `manager.rs` mux-register
|
||||||
|
error mapping were fixed in Units 4/5.
|
||||||
|
- **C-13** (drain-before-close) and **C-06** (all 4 teardown paths)
|
||||||
|
landed in Unit 5.
|
||||||
|
|
||||||
|
The units below cover what remains. Each is independently shippable.
|
||||||
|
|
||||||
|
## Unit 6 — Convention + doc cleanup (C-09, C-20-rem, C-22-rem, C-24, P-10, P-11)
|
||||||
|
|
||||||
|
**Goal:** conventions satisfied, `cargo doc` clean, no actively-wrong
|
||||||
|
comments, producer/consumer naming consistent in docs.
|
||||||
|
**Files:** `src/protocol/dispatch.rs`, `src/client/from_call.rs`,
|
||||||
|
`src/channels/operations.rs`, `src/channels/mod.rs`,
|
||||||
|
`docs/architecture/call-protocol.md`,
|
||||||
|
`docs/architecture/operation-registry.md`,
|
||||||
|
`docs/architecture/decisions/046-publish-operation-type-and-handler-kind-sink.md`.
|
||||||
|
**Scope:**
|
||||||
|
- **P-10:** `pump_sink` matches the string literals `"call.published"`,
|
||||||
|
`"call.completed"`, `"call.aborted"` (`dispatch.rs:475,485,486`)
|
||||||
|
instead of the `EVENT_PUBLISHED`/`EVENT_COMPLETED`/`EVENT_ABORTED`
|
||||||
|
constants the rest of the file imports (`wire.rs:12-17`). Pure
|
||||||
|
refactor hazard; no behavior change.
|
||||||
|
- **C-20 remainder:** the wrong "SAFETY:" comment at
|
||||||
|
`from_call.rs:271` (marks no `unsafe` block — `derive_alpn_from_op_name`
|
||||||
|
returns `Option<String>`, the leak happens in `leak_alpn`). Reword to
|
||||||
|
a plain note about the `'static` lifetime requirement. The mux EOF
|
||||||
|
claim, abort-cancels claim, and wrong-side peer-EOF claim were fixed
|
||||||
|
with Unit 4's reassembly rewrite — verify they're gone and remove
|
||||||
|
any remaining stray inline `//` comments in the channels module that
|
||||||
|
aren't carrying a non-obvious correctness constraint.
|
||||||
|
- **C-09:** fix the 2 remaining `cargo doc` warnings (verified 2026-08-13):
|
||||||
|
- `unresolved link to default_policy` (`operations.rs:50` — the
|
||||||
|
`[`default_policy`]` intra-doc link resolves to `super::policy::default_policy`;
|
||||||
|
use the full path `[`super::policy::default_policy`]` or re-export).
|
||||||
|
- `env is both a module and a macro` (`channels/mod.rs:30` — the
|
||||||
|
`[`env`]` link in the module doc is ambiguous; qualify it as
|
||||||
|
`[`env`][self::env]` or use the full path in the prose).
|
||||||
|
- **C-22 remainder:** remove the filler `PhantomData` test at
|
||||||
|
`client.rs:291-294` (`let _ = std::marker::PhantomData::<ChannelClient>;`
|
||||||
|
— asserts nothing). The `env.rs:155` tautology is already gone.
|
||||||
|
- **C-24:** replace "client→server streaming" with producer/consumer
|
||||||
|
phrasing in `call-protocol.md` (the Pub reference-table line) and
|
||||||
|
`operation-registry.md` (the ADR-046 reference-table line). No public
|
||||||
|
API names offend; this is doc-only.
|
||||||
|
- **P-11:** amend ADR-046 §3's `SinkHandler` type so the stream item
|
||||||
|
type matches §6. §3 (`decisions/046-...md:146`) declares
|
||||||
|
`Pin<Box<dyn Stream<Item = Value> + Send>>`; §6 (`:268`) declares
|
||||||
|
`Pin<Box<dyn Stream<Item = Result<Value, CallError>> + Send>>`. The
|
||||||
|
code uses §6's shape uniformly (`registration.rs:32-40`, aliased as
|
||||||
|
`PublishStream`). §3 is the one to amend (§6's `Result`-carrying shape
|
||||||
|
is correct — the handler must see initiator errors). The ADR's
|
||||||
|
Door-type section explicitly marks the concrete stream item type a
|
||||||
|
two-way-door detail, so this is a text amendment, not a design change.
|
||||||
|
**Acceptance gate:** `cargo doc --no-deps` emits 0 warnings;
|
||||||
|
`cargo clippy --all-targets -- -D warnings` clean; `cargo fmt --check`
|
||||||
|
clean; `cargo test` green.
|
||||||
|
|
||||||
|
## Unit 7 — Small correctness fixes (C-19-misc, C-21, C-23, P-13)
|
||||||
|
|
||||||
|
**Goal:** no panic paths in library code, no misleading public fields,
|
||||||
|
honest error envelopes, latent bugs in `derive_alpn_from_op_name` and
|
||||||
|
the mux pump map fixed.
|
||||||
|
**Files:** `src/channels/wire.rs`, `src/channels/mux.rs`,
|
||||||
|
`src/channels/operations.rs`, `src/client/from_call.rs`,
|
||||||
|
`src/protocol/dispatch.rs`, `src/registry/spec.rs`.
|
||||||
|
**Scope:**
|
||||||
|
- **C-21:** `write_header` (`wire.rs:110-114`) slices `out[..CHUNK_HEADER_LEN]`
|
||||||
|
and panics on short buffers. The doc comment at `:106-107` declares
|
||||||
|
the panic. Return `Result<_, ChunkError::HeaderTooShort>` instead
|
||||||
|
(the variant already exists for the parse side, `wire.rs:67-68`).
|
||||||
|
Update `write_chunk` (`:138-150`) and all callers (they pass
|
||||||
|
`[0u8; 8]` today, so the `Result` will always be `Ok` — but the API
|
||||||
|
is `&mut [u8]` and the contract should be typed, not panic-documented).
|
||||||
|
- **C-23:** `dispatch_requested`'s Sink and Stream error arms use
|
||||||
|
`String::new()` as the request id (`dispatch.rs:236,242`), producing
|
||||||
|
envelopes with an empty id. Pass the real `request_id` (the `Once`
|
||||||
|
arm at `:298` already does this correctly via `invoke`).
|
||||||
|
- **P-13:** `SinkDispatch::chunk_tx` is `pub` (`dispatch.rs:76-79`),
|
||||||
|
exposing the `futures::mpsc::Sender` type and the 64-slot buffer size
|
||||||
|
as effective public API. Make the field private (or `pub(crate)`)
|
||||||
|
before crates.io; `handle_stream` and `pump_sink` are the only
|
||||||
|
readers and they're in the same module.
|
||||||
|
- **C-19 (mux pump map leak + duplicate registration):** `mux.rs:159`
|
||||||
|
inserts each pump's `JoinHandle` into `self.pumps` and never removes
|
||||||
|
finished ones — they accumulate for the connection's lifetime.
|
||||||
|
Duplicate `register(channel_id)` silently overwrites the old handle
|
||||||
|
while the old pump keeps running (two pumps, one channel id). Fix:
|
||||||
|
detect a finished pump before insert (poll the `JoinHandle` or use
|
||||||
|
`is_finished()`), and reject duplicate registration (return an error
|
||||||
|
the caller maps to `ChannelExists`).
|
||||||
|
- **C-19 (`derive_alpn_from_op_name`):** `from_call.rs:291-303` takes
|
||||||
|
only the first path segment (`rest.split('/').next()`), so
|
||||||
|
`channels/custom/proto/sub` derives `alknet/custom` instead of the
|
||||||
|
full ALPN `custom/proto`. The `segment.starts_with("alknet/")` branch
|
||||||
|
at `:297` is dead (a single segment can't contain `/`), and
|
||||||
|
`segment == "alknet"` yields the nonsense ALPN `"alknet"`. Fix: strip
|
||||||
|
the known `/sub`|`/pub` suffix from `rest` instead of taking the
|
||||||
|
first segment, so multi-segment ALPNs survive. The `channels/tty/sub`
|
||||||
|
common case already works; this fixes the latent non-`alknet/*`
|
||||||
|
multi-segment case (no live callers yet, but the `Arc<str>` refactor
|
||||||
|
in Unit 8 will make this path reachable).
|
||||||
|
- **C-19 (`Box::leak` per discovery):** `from_call.rs:314-316` leaks
|
||||||
|
a `String` to `'static` on every `leak_alpn` call. Bounded per unique
|
||||||
|
ALPN in theory, but leaks on every *rediscovery* of every marked op.
|
||||||
|
Refactor `ChannelOpenSpec::alpn` from `&'static str` to
|
||||||
|
`Cow<'static, str>` (or `Arc<str>`) so `leak_alpn` can return an
|
||||||
|
owned value. This is a small public-API change to `ChannelOpenSpec`
|
||||||
|
(`spec.rs:38-46`); no deployments exist, and the ADR-047 §2 shape
|
||||||
|
said `&'static str` was "for now" (the `Box::leak` was the workaround
|
||||||
|
for that constraint). The `Cow`/`Arc<str>` is the deferred refactor.
|
||||||
|
(If this grows beyond a quick edit, split it into its own unit.)
|
||||||
|
- **C-19 (REQ-CH-04 error counter):** the demux's lenient
|
||||||
|
unknown-channel drop (`manager.rs:389-394`) only `debug!`-logs; there
|
||||||
|
is no counter or stats surface. Add a simple `AtomicU64` dropped-chunks
|
||||||
|
counter to `ChannelManager` with a `dropped_unknown_chunks()` accessor
|
||||||
|
for observability. (Low priority — file an OQ if the counter surface
|
||||||
|
isn't worth the API surface today.)
|
||||||
|
- **C-19 (resources-snapshot unbounded loop):** `operations.rs:278-285`
|
||||||
|
iterates `0..u32::MAX` with a per-iteration mutex lock, breaking when
|
||||||
|
`resources.len() >= manager.open_count()`. With sparse ids (monotonic
|
||||||
|
after churn) or a channel closing mid-iteration, this can iterate
|
||||||
|
millions of times. Replace with an iterator over the channel map's
|
||||||
|
keys (add a `ChannelManager::channel_ids() -> Vec<u32>` accessor) so
|
||||||
|
the snapshot is O(open channels), not O(max id). (This is also
|
||||||
|
touched by Unit 9's `resources/subscribe` rewrite — coordinate or
|
||||||
|
fold the loop fix into Unit 9.)
|
||||||
|
**Acceptance gate:** `cargo test` green; `cargo clippy --all-targets
|
||||||
|
-- -D warnings` clean; no panics reachable from `write_header`; the
|
||||||
|
`derive_alpn_from_op_name` test covers a multi-segment non-`alknet/*`
|
||||||
|
ALPN.
|
||||||
|
|
||||||
|
## Unit 8 — Pub protocol semantics (P-03, P-04, P-07)
|
||||||
|
|
||||||
|
**Goal:** the publish path's decided-but-unimplemented behaviors land:
|
||||||
|
per-chunk schema validation, initiator `call.error` terminates the
|
||||||
|
stream, and an early handler return short-circuits the feed.
|
||||||
|
**Files:** `src/registry/spec.rs`, `src/registry/discovery.rs`,
|
||||||
|
`src/client/from_call.rs`, `src/protocol/dispatch.rs`.
|
||||||
|
**Scope:**
|
||||||
|
- **P-03:** `OperationSpec::publish_schema` (`spec.rs:181-186`) is
|
||||||
|
declared, has a builder (`with_publish_schema`, `:236-239`), defaults
|
||||||
|
to `None`, and is **never read**. `pump_sink` forwards chunks as-is
|
||||||
|
(`dispatch.rs:475-484`). It is also missing from `spec_to_json`
|
||||||
|
(`discovery.rs:197-217`), from `operation_spec_schema()`
|
||||||
|
(`discovery.rs:102-161`), and from `from_call`'s `rebuild_spec_for`
|
||||||
|
(`from_call.rs:205-282`) — so an imported Pub op loses the schema.
|
||||||
|
Implement: (a) add `publish_schema` to `spec_to_json` (emit as
|
||||||
|
`"publish_schema"` when `Some`), `operation_spec_schema()` (add the
|
||||||
|
property to the schema), and `rebuild_spec_for` (parse it back); (b)
|
||||||
|
in `pump_sink`, validate each `call.published` chunk's `input`
|
||||||
|
against `publish_schema` before yielding it — on validation failure,
|
||||||
|
inject an `Err(CallError::invalid_input(...))` and terminate the
|
||||||
|
stream (matching the `SinkHandler` contract). Use `alktype`'s
|
||||||
|
`validation::build_validator` (the dependency is declared and unused;
|
||||||
|
`jsonschema` is transitively available). When `publish_schema` is
|
||||||
|
`None`, chunks are yielded as-is (current behavior).
|
||||||
|
- **P-04:** `pump_sink` matches only `"call.published"`, `"call.completed"`,
|
||||||
|
`"call.aborted"` and drops everything else into a debug-log ignore
|
||||||
|
branch (`dispatch.rs:492-497`). A `call.error` frame from the
|
||||||
|
initiator is silently ignored. Per ADR-046 §6, an initiator-side
|
||||||
|
`call.error` should inject the initiator's `CallError` as an `Err`
|
||||||
|
item that terminates the stream. Match `"call.error"` in the
|
||||||
|
`pump_sink` event match, parse the `CallError` from the payload, send
|
||||||
|
`Err(call_error)` into `chunk_tx`, and break the feed.
|
||||||
|
- **P-07:** `pump_sink` uses `tokio::join!(feed_fut, handler)`
|
||||||
|
(`dispatch.rs:510`) and only writes the response after **both**
|
||||||
|
complete. If the `SinkHandler` returns early (e.g. rejects after chunk
|
||||||
|
1), the feed loop notices only when its next `chunk_tx.send` fails —
|
||||||
|
a paused or long initiator never learns of the early error. Fix:
|
||||||
|
use `tokio::select!` (or race the handler against the feed) so that
|
||||||
|
when the handler completes, the feed is short-circuited and the
|
||||||
|
response is written immediately. Dropping `chunk_tx` on the handler
|
||||||
|
side signals the feed to stop. Add a test: a sink handler that
|
||||||
|
returns after 1 chunk; an initiator that publishes 3; assert the
|
||||||
|
response arrives without waiting for all 3 chunks.
|
||||||
|
**Acceptance gate:** a test that registers a Pub op with a
|
||||||
|
`publish_schema`, publishes a chunk that violates it, and asserts the
|
||||||
|
handler receives an `Err` and the stream terminates; a test that an
|
||||||
|
initiator `call.error` terminates the publish stream; a test that an
|
||||||
|
early-returning handler's response is not deferred behind a slow feed.
|
||||||
|
|
||||||
|
## Unit 9 — Abort-cancels-Pub (P-06)
|
||||||
|
|
||||||
|
**Goal:** an initiator can actually cancel an in-flight Pub; the
|
||||||
|
handler future is dropped on abort, not just `join!`-ed to completion.
|
||||||
|
**Files:** `src/protocol/dispatch.rs`, `src/protocol/connection.rs`,
|
||||||
|
possibly `src/protocol/pending.rs`.
|
||||||
|
**Scope:** This is the one Unit 6 item with a design decision, so it
|
||||||
|
gets its own unit. The single-stream call mode from Unit 2 already
|
||||||
|
routes `call.aborted` for an in-flight sink's `request_id` to the
|
||||||
|
matching `chunk_tx` (`dispatch.rs:684-693`), injecting an `Err` — but
|
||||||
|
the handler future is spawned separately (`dispatch.rs:670-680`) and
|
||||||
|
is **not** dropped on abort; it runs to completion and its response is
|
||||||
|
written to the wire after the abort. The stream-per-request
|
||||||
|
`pump_sink` path (`dispatch.rs:447-517`) `join!`s the handler to
|
||||||
|
completion regardless of abort. Decisions:
|
||||||
|
- **Single-stream path:** wire a cancellation token (or `oneshot`) per
|
||||||
|
in-flight sink so `call.aborted` both injects the `Err` **and**
|
||||||
|
aborts the spawned handler task. The handler's `JoinHandle` is
|
||||||
|
already stored implicitly via `tokio::spawn` — keep it in the
|
||||||
|
`in_flight_sinks` map alongside `chunk_tx` so abort can `.abort()` it.
|
||||||
|
- **Stream-per-request path:** `pump_sink` needs the same: on
|
||||||
|
`call.aborted` for this `request_id`, drop `chunk_tx` (feed gets EOF)
|
||||||
|
**and** drop the handler future (use `select!` with an abort signal
|
||||||
|
instead of `join!`). The cross-stream `abort()` path
|
||||||
|
(`connection.rs:398-415`) opens a new stream and only mutates
|
||||||
|
`PendingRequestMap` — on the responder, a different `handle_stream`
|
||||||
|
task receives it and calls `handle_abort`, which doesn't reach the
|
||||||
|
running `pump_sink`. The single-stream mode makes the cross-stream
|
||||||
|
abort path less relevant for channel 0; document whether
|
||||||
|
stream-per-request abort is worth fixing or should be deprecated in
|
||||||
|
favor of single-stream.
|
||||||
|
- **Also (from P-06):** during a sink pump, `call.aborted` frames for
|
||||||
|
**other** request IDs on the same stream are dropped by the
|
||||||
|
id-mismatch guard before the type match (`dispatch.rs:466-473`). On
|
||||||
|
the single-stream path this is handled (the outer loop routes by
|
||||||
|
`request_id` first); on the stream-per-request path it's a latent
|
||||||
|
bug. Verify the single-stream path is correct and decide whether the
|
||||||
|
stream-per-request path needs the multiplexing fix or just
|
||||||
|
deprecation.
|
||||||
|
**Acceptance gate:** a test that aborts an in-flight publish and
|
||||||
|
asserts the handler future is cancelled (the handler observes a
|
||||||
|
drop/cancellation, not just an `Err` injected into the stream it's no
|
||||||
|
longer reading); a test that the aborting initiator receives
|
||||||
|
confirmation and the responder's resources are released.
|
||||||
|
|
||||||
|
## Unit 10 — Stubs, substrate modes, and spec-doc renumbering (C-10, C-11, C-14, C-15, C-26)
|
||||||
|
|
||||||
|
**Goal:** the remaining decided-but-deferred behaviors are honestly
|
||||||
|
stubbed (no fake success) with OQs filed; the spec docs are consistent
|
||||||
|
with the post-047 model and the alkcall ADR numbering.
|
||||||
|
**Files:** `src/channels/operations.rs`, `src/channels/adapter.rs`,
|
||||||
|
`src/channels/client.rs`, `docs/architecture/channel-client.md`,
|
||||||
|
`docs/architecture/channels-adapter.md`,
|
||||||
|
`docs/architecture/channels-wire.md`, `docs/architecture/open-questions.md`.
|
||||||
|
**Scope:** This is the lowest-risk unit and can land last. It is mostly
|
||||||
|
docs + OQ filing.
|
||||||
|
- **C-10 / C-11:** `channel/control` (`operations.rs:234-262`) returns
|
||||||
|
`{"ok": true}` for a silently-discarded `message`; there is no
|
||||||
|
control-handle concept. `channel/resources/subscribe`
|
||||||
|
(`operations.rs:268-289`) is `futures::stream::once(...)` (one-shot,
|
||||||
|
not live), emits currently-open channels' ALPNs (not the set of
|
||||||
|
openable ALPNs aggregated from ALPN-crate resource enumerators), and
|
||||||
|
has the unbounded `0..u32::MAX` loop (see Unit 7). Either implement
|
||||||
|
(control-handle routing; live subscription aggregated from
|
||||||
|
enumerators) or file OQs and make the stubs honest: return an
|
||||||
|
explicit `unimplemented`-style `CallError` (code
|
||||||
|
`channel:control_not_implemented` / `channel:resources_not_implemented`)
|
||||||
|
so callers fail loudly, not with fake success. Recommend: file OQs
|
||||||
|
(the control-handle and resource-enumerator surfaces are real design
|
||||||
|
work, not cleanup) and make the stubs honest.
|
||||||
|
- **C-14 / C-15:** only the in-line substrate mode is implemented
|
||||||
|
(`adapter.rs:199-259` accepts one bidi stream once). The QUIC-native
|
||||||
|
multi-stream substrate ("accept remaining bidi streams, read headers
|
||||||
|
off each") is not implemented. `ChannelClient` has `open_channel`
|
||||||
|
(Unit 5) but not `subscribe_resources` or the `Channel { channel_id,
|
||||||
|
source }` struct from ADR-043. Either implement or file OQs and
|
||||||
|
remove the misleading doc comments / nonexistent method references.
|
||||||
|
Recommend: file OQs (QUIC-native substrate + the full `ChannelClient`
|
||||||
|
API are features, not fixes) and remove the stale doc claims
|
||||||
|
(`client.rs` doc comment referencing a nonexistent
|
||||||
|
`manager.open_channel_stream`).
|
||||||
|
- **C-26:** update `channel-client.md`, `channels-adapter.md`,
|
||||||
|
`channels-wire.md` to the post-047 model: remove `open_channel(...,
|
||||||
|
direction)`, `ChannelDirection`, `ResourceEntry.access`, the generic
|
||||||
|
`channel/open` handler, `channel:unknown_alpn` (replaced by
|
||||||
|
per-ALPN ops in ADR-047 §3). Renumber stale alknet ADR refs
|
||||||
|
(071/075/076/093/094/079/080 and others >047) to their alkcall
|
||||||
|
renumbered equivalents (ADR-001..047), or mark them as
|
||||||
|
"alknet source ADR, not yet ported" where no alkcall equivalent
|
||||||
|
exists. This is a doc-only pass; verify against
|
||||||
|
`docs/architecture/decisions/` (which only goes to 047).
|
||||||
|
**Acceptance gate:** stubs return explicit errors (not `{"ok": true}`);
|
||||||
|
`cargo doc --no-deps` has no broken links to nonexistent methods; the
|
||||||
|
channels spec docs no longer describe the pre-047 `channel/open`
|
||||||
|
handler or `ChannelDirection`; OQs filed for C-10/C-11/C-14/C-15.
|
||||||
|
|
||||||
|
## Suggested sequencing (post-Units 1-5)
|
||||||
|
|
||||||
```
|
```
|
||||||
Unit 1 (Pub end-to-end) → unblocks the Pub flagship use cases
|
Unit 6 (convention + doc cleanup) → no deps; mechanical; do first
|
||||||
Unit 2 (channel 0 single-stream) → unblocks all channel-0 integration tests
|
Unit 7 (small correctness fixes) → no deps; independent
|
||||||
Unit 3 (register_openable) → needs a §4 spec decision first; unblocks opening channels
|
Unit 8 (Pub protocol semantics) → no deps beyond Units 1-5; P-03 uses alktype
|
||||||
Unit 4 (backpressure) → independent; can run in parallel with 2/3
|
Unit 9 (abort-cancels-Pub) → depends on Unit 8's pump_sink changes (shared file)
|
||||||
Unit 5 (ledger + adoption) → needs a §5 spec decision first; depends on 3
|
Unit 10 (stubs + substrate + doc renumber) → no deps; can land any time
|
||||||
Unit 6 (cleanup) → last; after the integration is end-to-end functional
|
|
||||||
```
|
```
|
||||||
|
|
||||||
Units 1, 2, 4 can proceed immediately with no spec decisions. Units 3
|
Units 6, 7, 8, 10 are independent and can proceed in any order (or in
|
||||||
and 5 each need one written decision (ADR-047 §4 affirm/amend; ADR-047
|
parallel across separate branches). Unit 9 touches the same `pump_sink`
|
||||||
§5 odd/even-or-adoption). Unit 6 is the long tail.
|
code as Unit 8, so land 8 before 9 to avoid merge conflicts. None of
|
||||||
|
Units 6-10 require a spec decision (the ADR-047 §4 and §5 decisions
|
||||||
|
were made in Units 3 and 5). Units 1-5 are complete (commits
|
||||||
|
`502488c` through `f25d0a6`); the integration is end-to-end functional.
|
||||||
|
|
||||||
## On the baseline objective
|
## On the baseline objective
|
||||||
|
|
||||||
|
|||||||
Reference in new issue
Block a user