fix: Unit 6 — convention + doc cleanup (C-09, C-20-rem, C-22-rem, C-24, P-10, P-11)
Conventions satisfied, `cargo doc` clean, no actively-wrong comments, producer/consumer naming consistent in the call/registry docs. P-10 — `pump_sink` matched the string literals "call.published", "call.completed", "call.aborted" (dispatch.rs) instead of the EVENT_PUBLISHED / EVENT_COMPLETED / EVENT_ABORTED constants the rest of the file imports. Replaced with the constants — pure refactor hazard, no behavior change. C-20 remainder — the wrong "SAFETY:" comment at from_call.rs:271 marked no `unsafe` block and was factually wrong (described a `'static` return that isn't what `derive_alpn_from_op_name` does — it returns `Option<String>`; the leak happens in `leak_alpn`). Reworded to a plain note about the `'static` lifetime requirement. Also reworded the abort-cancels claims in the `pump_sink` and `pump_stream` doc comments (dispatch.rs): both claimed `call.aborted` "cancels the task and drops the handler future" — the handler is actually `join!`-ed to completion and not yet cancelled (the abort-cancels-Pub mechanism is review 001 Unit 9). Trimmed step-numbered narration comments in adapter.rs / client.rs that restated what the code does, keeping the ordering-constraint and REQ-CH comments. The big reassembly.rs deliberation landed with Unit 4; this finishes the remainder. C-09 — fixed the 2 remaining `cargo doc` warnings (was 4; the register_openable links were fixed in Unit 3): - `unresolved link to default_policy` (operations.rs:50) — the [`default_policy`] intra-doc link resolves to super::policy::default_policy; used the full path. - `env is both a module and a macro` (channels/mod.rs:30) — the [`env`] link collided with the std `env!` macro; qualified as [`self::env`]. `cargo doc --no-deps` now emits 0 warnings. C-22 remainder — removed the filler `PhantomData` test at client.rs (`let _ = std::marker::PhantomData::<ChannelClient>;` — asserts nothing). The env.rs tautology was already removed in Unit 3. C-24 — replaced "client→server streaming" with "producer→consumer streaming" in call-protocol.md, operation-registry.md, README.md, and open-questions.md (4 occurrences). Per AGENTS.md §8 the convention is producer/consumer, not server/client. The remaining "client→server" references in channels ADRs 034/037 are in stream_type table contexts that Unit 10 (C-26) will handle as part of the spec-doc renumbering. P-11 — amended ADR-046 §3's SinkHandler type so the stream item type matches §6. §3 declared `Pin<Box<dyn Stream<Item = Value> + Send>>`; §6 declared `Pin<Box<dyn Stream<Item = Result<Value, CallError>> + Send>>`. The code uses §6's shape uniformly (registration.rs:32-40, aliased as PublishStream). §3's text and the Door-type section are amended to match §6; the Door-type section already marked the concrete stream item type a two-way-door detail, so this is a text correction, not a design change. Added an amendment note dated 2026-08-13. Verification: - cargo test --lib → 449 passed, 0 failed (was 450; -1 removed filler test) - cargo clippy --all-targets -- -D warnings → clean - cargo fmt --check → clean - cargo doc --no-deps → 0 warnings (was 2)
This commit is contained in:
1 parent
48564a8f49
commit
bfb265e31b
11 files changed
+65
-84
No files matched your search
+6
-20
@@ -197,21 +197,13 @@ impl ProtocolHandler for ChannelsAdapter {
|
||||
}
|
||||
|
||||
async fn handle(&self, connection: Connection, auth: &AuthContext) -> Result<(), HandlerError> {
|
||||
// 1. Get the bidi stream(s) from the transport. On an in-line
|
||||
// transport, `accept_bi()` yields once (the single stream
|
||||
// carries all channels via the header). On QUIC, it yields
|
||||
// repeatedly. We take the first stream for the demux loop
|
||||
// and use its write half for the mux.
|
||||
let bidi = connection.accept_bi().await.map_err(|e| match e {
|
||||
StreamError::ConnectionClosed => HandlerError::ConnectionClosed,
|
||||
other => HandlerError::StreamError(std::io::Error::other(format!("{other:?}"))),
|
||||
})?;
|
||||
|
||||
// Split the bidi stream into read and write halves. The read
|
||||
// half feeds the demux; the write half feeds the mux.
|
||||
let (reader, writer) = tokio::io::split(bidi);
|
||||
|
||||
// 2. Construct the mux (write side) and the manager.
|
||||
let (mux_handle, mux_runner) = MuxRunner::new(Box::new(writer));
|
||||
let manager = ChannelManager::new(
|
||||
mux_handle,
|
||||
@@ -230,14 +222,9 @@ impl ProtocolHandler for ChannelsAdapter {
|
||||
}
|
||||
});
|
||||
|
||||
// 3. Install channel 0 (pre-negotiated as `alknet/call`,
|
||||
// ADR-036). Channel 0's reassembly buffer + mux write half
|
||||
// are joined into a `BiStream` and wrapped in a
|
||||
// `Connection` (via `ChannelBidiStreamSource`). The
|
||||
// `install_channel_zero` hook (provided by `channels-call`)
|
||||
// runs the call dispatch loop on this `Connection` — the
|
||||
// single-stream call mode (ADR-036 amendment) multiplexes
|
||||
// all `EventEnvelope` frames on channel 0's one `BiStream`.
|
||||
// Channel 0 is pre-negotiated as `alknet/call` (ADR-036); the
|
||||
// `install_channel_zero` hook runs the single-stream call
|
||||
// dispatch loop on channel 0's `Connection`.
|
||||
let (channel0_send, channel0_recv) = manager
|
||||
.install_channel_zero(None)
|
||||
.await
|
||||
@@ -248,10 +235,9 @@ impl ProtocolHandler for ChannelsAdapter {
|
||||
let _handler_task =
|
||||
(self.install_channel_zero)(manager.clone(), channel0_conn, auth.clone());
|
||||
|
||||
// 4. Run the demux loop (read side). This blocks until
|
||||
// transport EOF, then clears the channel map (REQ-CH-02)
|
||||
// and decrements the per-identity policy for each drained
|
||||
// channel (ADR-047 §7 — connection-drop teardown path).
|
||||
// The demux loop blocks until transport EOF, then clears the
|
||||
// channel map (REQ-CH-02) and decrements the per-identity
|
||||
// policy for each drained channel (ADR-047 §7).
|
||||
Self::run_demux_loop(&manager, Box::new(reader), &self.policy).await;
|
||||
|
||||
Ok(())
|
||||
|
||||
+7
-26
@@ -68,9 +68,7 @@ impl ChannelClient {
|
||||
|
||||
// Spawn the mux runner BEFORE installing channel 0 —
|
||||
// `install_channel_zero` calls `mux.register(0).await` which
|
||||
// sends a `Registration` to the runner and waits for the
|
||||
// response. The runner must be draining `new_pumps` for the
|
||||
// registration to complete.
|
||||
// needs the runner to be draining `new_pumps`.
|
||||
let _mux_task = tokio::spawn(async move {
|
||||
if let Err(e) = mux_runner.run().await {
|
||||
tracing::warn!(error = %e, "channel client: mux runner ended with error");
|
||||
@@ -85,20 +83,14 @@ impl ChannelClient {
|
||||
ChannelSide::Connect,
|
||||
);
|
||||
|
||||
// Install channel 0 — the call adapter's read/write halves.
|
||||
let (channel0_send, channel0_recv) = manager
|
||||
.install_channel_zero(None)
|
||||
.await
|
||||
.map_err(|_| StreamError::StreamClosed)?;
|
||||
|
||||
// Join channel 0's reassembled read half + mux write half into
|
||||
// a `BiStream` (via `channel_source`), wrap as a `Connection`
|
||||
// (for ALPN/addr/identity), then yield the `BiStream` once via
|
||||
// `accept_bi` and split it into a `SharedFrameWriter` (the
|
||||
// framed write half for `call.requested` etc.) + a raw read
|
||||
// half (for the read pump). This is the single-stream call
|
||||
// mode (ADR-036 amendment): all `EventEnvelope` frames are
|
||||
// multiplexed on channel 0's one `BiStream`.
|
||||
// Single-stream call mode (ADR-036 amendment): channel 0's
|
||||
// read+write halves are split into a `SharedFrameWriter`
|
||||
// (for `call.requested`) and a read pump (for responses).
|
||||
let channel0_source =
|
||||
super::source::channel_source(channel0_recv, channel0_send, remote_addr);
|
||||
let channel0_conn = Connection::from_source(channel0_source, b"alknet/call".to_vec());
|
||||
@@ -109,12 +101,6 @@ impl ChannelClient {
|
||||
let call_connection =
|
||||
CallConnection::new_single_stream(channel0_conn, single_stream_writer);
|
||||
|
||||
// Spawn the read pump for channel 0's response frames. Reads
|
||||
// `EventEnvelope` frames off channel 0's reassembled read half
|
||||
// (fed by the demux) and routes them into the
|
||||
// `PendingRequestMap` via `dispatch_envelope`, resolving
|
||||
// pending calls. This is the single-stream analogue of
|
||||
// `read_stream_until_closed` in stream-per-request mode.
|
||||
let pending_map = Arc::clone(call_connection.pending());
|
||||
let _read_pump = tokio::spawn(async move {
|
||||
crate::protocol::connection::read_single_stream_until_closed(
|
||||
@@ -124,9 +110,9 @@ impl ChannelClient {
|
||||
.await;
|
||||
});
|
||||
|
||||
// Spawn the demux loop (read side). The loop reads 8-byte
|
||||
// chunk headers and routes payloads to the manager. It ends
|
||||
// on transport EOF, clearing the channel map (REQ-CH-02).
|
||||
// The demux loop ends on transport EOF, clearing the channel
|
||||
// map (REQ-CH-02). The connect side passes `policy: None` (it
|
||||
// does not enforce the per-identity cap).
|
||||
let demux_manager = manager.clone();
|
||||
let _demux_task = tokio::spawn(async move {
|
||||
super::adapter::ChannelsAdapter::run_demux_loop_for_client(
|
||||
@@ -288,11 +274,6 @@ mod tests {
|
||||
})
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn channel_client_is_not_send_safe_across_await_by_default() {
|
||||
let _ = std::marker::PhantomData::<ChannelClient>;
|
||||
}
|
||||
|
||||
/// C-25 #1 — the end-to-end acceptance gate for Unit 2 (channel 0
|
||||
/// single-stream call mode, ADR-036 amendment). Wires
|
||||
/// `ChannelClient` (connect side) ↔ `ChannelsAdapter` (accept
|
||||
|
||||
+1
-1
@@ -27,7 +27,7 @@
|
||||
//! (ADR-041, amended by ADR-047 §7 — opener ledger).
|
||||
//! - [`client`]: `ChannelClient` — transport-agnostic
|
||||
//! `from_connection` (ADR-043).
|
||||
//! - [`env`]: `ChannelOperationEnv` extension trait (ADR-047 §4 —
|
||||
//! - [`self::env`]: `ChannelOperationEnv` extension trait (ADR-047 §4 —
|
||||
//! keeps the call crate free of channels types).
|
||||
//!
|
||||
//! See `docs/architecture/` for the full specification.
|
||||
|
||||
@@ -47,7 +47,8 @@ pub struct ChannelOperations {
|
||||
impl ChannelOperations {
|
||||
/// Construct with a `ChannelManager` and a
|
||||
/// `ChannelLifecyclePolicy`. The default policy is
|
||||
/// `PerIdentityChannelPolicy::new(256)` (via [`default_policy`]).
|
||||
/// `PerIdentityChannelPolicy::new(256)` (via
|
||||
/// [`default_policy`][super::policy::default_policy]).
|
||||
pub fn new(manager: ChannelManager, policy: Arc<dyn ChannelLifecyclePolicy>) -> Self {
|
||||
Self { manager, policy }
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user