From 67d2f4affb0c16fa64ee6dd2239dd7529e6a4d7e Mon Sep 17 00:00:00 2001 From: deepseek-v4-pro Date: Fri, 17 Jul 2026 10:33:42 +0000 Subject: [PATCH] feat(endpoint): decompose Phase 2 into 9 tasks for alknet-endpoint crate extraction - endpoint/crate-init: initialize crate, Cargo.toml, module skeleton - endpoint/registry: extract HandlerRegistry (~50 lines) - endpoint/endpoint-core: AlknetEndpoint fresh build against ADR-083 shape - endpoint/dispatch: public dispatch, build_auth_context, ACME guard - endpoint/accept-quinn: quinn accept loop + extractors (extracted) - endpoint/accept-iroh: iroh accept loop + extractors (extracted) - endpoint/accept-tcp-tls: TCP+TLS accept loop (new code) - endpoint/tests: move + adapt 17 tests - endpoint/review-endpoint: review checkpoint 7 generations, 3 parallel tasks (accept loops), no cycles. Depends on tls/review-tls (Phase 1 complete). --- tasks/endpoint/accept-iroh.md | 163 +++++++++++++++++++++++ tasks/endpoint/accept-quinn.md | 207 +++++++++++++++++++++++++++++ tasks/endpoint/accept-tcp-tls.md | 169 ++++++++++++++++++++++++ tasks/endpoint/crate-init.md | 128 ++++++++++++++++++ tasks/endpoint/dispatch.md | 162 +++++++++++++++++++++++ tasks/endpoint/endpoint-core.md | 211 ++++++++++++++++++++++++++++++ tasks/endpoint/registry.md | 99 ++++++++++++++ tasks/endpoint/review-endpoint.md | 144 ++++++++++++++++++++ tasks/endpoint/tests.md | 156 ++++++++++++++++++++++ 9 files changed, 1439 insertions(+) create mode 100644 tasks/endpoint/accept-iroh.md create mode 100644 tasks/endpoint/accept-quinn.md create mode 100644 tasks/endpoint/accept-tcp-tls.md create mode 100644 tasks/endpoint/crate-init.md create mode 100644 tasks/endpoint/dispatch.md create mode 100644 tasks/endpoint/endpoint-core.md create mode 100644 tasks/endpoint/registry.md create mode 100644 tasks/endpoint/review-endpoint.md create mode 100644 tasks/endpoint/tests.md diff --git a/tasks/endpoint/accept-iroh.md b/tasks/endpoint/accept-iroh.md new file mode 100644 index 0000000..5b4f89b --- /dev/null +++ b/tasks/endpoint/accept-iroh.md @@ -0,0 +1,163 @@ +--- +id: endpoint/accept-iroh +name: Implement iroh accept loop and extractors in alknet-endpoint +status: pending +depends_on: [endpoint/dispatch] +scope: narrow +risk: low +impact: component +level: implementation +--- + +## Description + +Phase 2, Task 6 of the crate extraction. Implement the iroh accept loop and +transport-specific extractors in `crates/alknet-endpoint/src/accept/iroh.rs`. + +Extract from `crates/alknet-core/src/endpoint.rs` lines 390-472: +`run_iroh_accept_loop`, `extract_iroh_client_fingerprint`. + +The old code **stays** in `endpoint.rs` (duplicated) — no breakage. + +### Types to extract + +From `endpoint.rs` lines 390-472: + +| Type/Function | Lines | Destination | +|---------------|-------|-------------| +| `run_iroh_accept_loop()` | 391-437 | `accept/iroh.rs` | +| `extract_iroh_client_fingerprint()` | 469-472 | `accept/iroh.rs` | + +### Adaptations + +1. **`run_iroh_accept_loop` → `run_accept_loop`**: Rename since it's already in the + `iroh` module. Takes `iroh::Endpoint`, `Arc`, + `Arc`, `&mut watch::Receiver`. + +2. **Connection conversion**: After the handshake completes, convert the + `iroh::endpoint::Connection` to `alknet_core::Connection` via + `Connection::from_iroh(connection)` — same as the old code. + +3. **Call `dispatch` instead of `dispatch_iroh`**: The accept loop extracts ALPN and + fingerprint, then calls `crate::dispatch::dispatch_connection()` (the free function). + The old code called `dispatch_iroh(connection, alpn, &handlers, &identity_provider)`. + +4. **Imports**: Update `crate::` imports to `alknet_core::` and `crate::`. + +5. **Feature gates**: All code in this module is gated on `#[cfg(feature = "iroh")]`. + +### Implementation sketch + +```rust +// accept/iroh.rs +#[cfg(feature = "iroh")] +use std::sync::Arc; +#[cfg(feature = "iroh")] +use tokio::sync::watch; +#[cfg(feature = "iroh")] +use tracing::{debug, warn}; + +#[cfg(feature = "iroh")] +use alknet_core::auth::IdentityProvider; +#[cfg(feature = "iroh")] +use alknet_core::types::Connection; + +#[cfg(feature = "iroh")] +use crate::registry::HandlerRegistry; + +#[cfg(feature = "iroh")] +pub(crate) async fn run_accept_loop( + iroh: iroh::Endpoint, + handlers: Arc, + identity_provider: Arc, + shutdown_rx: &mut watch::Receiver, +) { + loop { + tokio::select! { + _ = shutdown_rx.changed() => { + debug!("iroh accept loop: shutdown signaled"); + break; + } + incoming = iroh.accept() => { + let Some(incoming) = incoming else { + debug!("iroh accept loop: endpoint closed"); + break; + }; + let handlers = handlers.clone(); + let identity_provider = identity_provider.clone(); + tokio::spawn(async move { + let mut connecting = match incoming.accept() { + Ok(c) => c, + Err(e) => { + warn!("iroh accept failed: {e}"); + return; + } + }; + let alpn = match connecting.alpn().await { + Ok(alpn) => alpn, + Err(e) => { + warn!("iroh ALPN negotiation failed: {e}"); + return; + } + }; + let connection = match connecting.await { + Ok(conn) => conn, + Err(e) => { + warn!("iroh handshake completion failed: {e}"); + return; + } + }; + let fingerprint = extract_client_fingerprint(&connection); + let conn = Connection::from_iroh(connection); + crate::dispatch::dispatch_connection( + conn, alpn, fingerprint, None, // iroh has no SocketAddr + &handlers, &identity_provider, + ); + }); + } + } + } +} + +#[cfg(feature = "iroh")] +pub(crate) fn extract_client_fingerprint(connection: &iroh::endpoint::Connection) -> Option { + let node_id = connection.remote_id(); + Some(format!("ed25519:{}", node_id)) +} +``` + +### What stays in core + +The old `run_iroh_accept_loop` and `extract_iroh_client_fingerprint` in `endpoint.rs` are +**not deleted** — they stay as duplicates. The prune happens in Phase 4. + +## Acceptance Criteria + +- [ ] `accept/iroh.rs` contains `run_accept_loop`, `extract_client_fingerprint` +- [ ] `run_accept_loop` spawns a task per accepted connection, handles shutdown signal +- [ ] `run_accept_loop` negotiates ALPN via `connecting.alpn().await` +- [ ] `extract_client_fingerprint` extracts the iroh `NodeId` as `ed25519:` +- [ ] Accept loop calls `crate::dispatch::dispatch_connection()` with extracted values +- [ ] `remote_addr` is `None` for iroh (no `SocketAddr`) +- [ ] All code gated on `#[cfg(feature = "iroh")]` +- [ ] All imports use `alknet_core::` (not `crate::` from core) +- [ ] `cargo check -p alknet-endpoint --features iroh` succeeds +- [ ] `cargo clippy -p alknet-endpoint --features iroh` succeeds with no warnings +- [ ] `cargo test -p alknet-core` still passes (old code untouched) + +## References + +- docs/research/alknet-crate-extraction/findings.md — Phase 2, accept/iroh module +- docs/architecture/crates/endpoint/README.md — Accept loops (lines 177-193) +- crates/alknet-core/src/endpoint.rs — lines 390-472 (source code to extract) + +## Notes + +> This is a straightforward extraction — the iroh accept loop logic doesn't change. +> The main adaptation is calling `crate::dispatch::dispatch_connection()` instead of +> the old `dispatch_iroh()`. Iroh connections don't have a `SocketAddr`, so +> `remote_addr` is always `None`. The old code in `endpoint.rs` is NOT deleted. + +## Summary + +> To be filled on completion diff --git a/tasks/endpoint/accept-quinn.md b/tasks/endpoint/accept-quinn.md new file mode 100644 index 0000000..ff989f3 --- /dev/null +++ b/tasks/endpoint/accept-quinn.md @@ -0,0 +1,207 @@ +--- +id: endpoint/accept-quinn +name: Implement quinn accept loop and extractors in alknet-endpoint +status: pending +depends_on: [endpoint/dispatch] +scope: narrow +risk: low +impact: component +level: implementation +--- + +## Description + +Phase 2, Task 5 of the crate extraction. Implement the quinn accept loop and +transport-specific extractors in `crates/alknet-endpoint/src/accept/quinn.rs`. + +Extract from `crates/alknet-core/src/endpoint.rs` lines 287-388: +`run_quinn_accept_loop`, `extract_quinn_alpn`, `extract_quinn_client_fingerprint`. + +The old code **stays** in `endpoint.rs` (duplicated) — no breakage. + +### Types to extract + +From `endpoint.rs` lines 287-388: + +| Type/Function | Lines | Destination | +|---------------|-------|-------------| +| `run_quinn_accept_loop()` | 288-327 | `accept/quinn.rs` | +| `extract_quinn_alpn()` | 368-378 | `accept/quinn.rs` | +| `extract_quinn_client_fingerprint()` | 381-388 | `accept/quinn.rs` | + +### Adaptations + +1. **`run_quinn_accept_loop` → `run_accept_loop`**: Rename to `run_accept_loop` since + it's already in the `quinn` module. Takes `quinn::Endpoint`, `Arc`, + `Arc`, `&mut watch::Receiver`. + +2. **Connection conversion**: After the TLS handshake completes, convert the + `quinn::Connection` to `alknet_core::Connection` via + `Connection::from_quinn_with_alpn(connection, alpn.clone())` — same as the old code. + +3. **Call `dispatch` instead of `dispatch_quinn`**: The accept loop extracts ALPN and + fingerprint, then calls `AlknetEndpoint::dispatch(connection, alpn, fingerprint, remote_addr)`. + The old code called `dispatch_quinn(connection, &handlers, &identity_provider)` which + did the extraction internally. The new code extracts first, then dispatches. + +4. **Imports**: Update `crate::` imports to `alknet_core::` and `crate::` (for + `crate::dispatch::build_auth_context` — but `build_auth_context` is called inside + `dispatch` now, not in the accept loop). + +5. **Feature gates**: All code in this module is gated on `#[cfg(feature = "quinn")]`. + +### Implementation sketch + +```rust +// accept/quinn.rs +#[cfg(feature = "quinn")] +use std::net::SocketAddr; +#[cfg(feature = "quinn")] +use std::sync::Arc; +#[cfg(feature = "quinn")] +use tokio::sync::watch; +#[cfg(feature = "quinn")] +use tracing::{debug, warn}; + +#[cfg(feature = "quinn")] +use alknet_core::auth::IdentityProvider; +#[cfg(feature = "quinn")] +use alknet_core::types::Connection; + +#[cfg(feature = "quinn")] +use crate::registry::HandlerRegistry; + +#[cfg(feature = "quinn")] +pub(crate) async fn run_accept_loop( + quinn: quinn::Endpoint, + handlers: Arc, + identity_provider: Arc, + shutdown_rx: &mut watch::Receiver, +) { + loop { + tokio::select! { + _ = shutdown_rx.changed() => { + debug!("quinn accept loop: shutdown signaled"); + break; + } + incoming = quinn.accept() => { + let Some(incoming) = incoming else { + debug!("quinn accept loop: endpoint closed"); + break; + }; + let connecting = match incoming.accept() { + Ok(c) => c, + Err(e) => { + warn!("quinn accept failed: {e}"); + continue; + } + }; + let handlers = handlers.clone(); + let identity_provider = identity_provider.clone(); + tokio::spawn(async move { + let connection = match connecting.await { + Ok(conn) => conn, + Err(e) => { + warn!("quinn TLS handshake failure: {e}"); + return; + } + }; + let alpn = extract_alpn(&connection); + let remote_addr = Some(connection.remote_address()); + let fingerprint = extract_client_fingerprint(&connection); + let conn = Connection::from_quinn_with_alpn(connection, alpn.clone()); + // dispatch is called by the endpoint — but the accept loop + // doesn't have a reference to the endpoint. Instead, the + // endpoint passes a dispatch function or the accept loop + // calls a free function. + // + // Design choice: the accept loop calls + // crate::dispatch::dispatch_connection(...) which takes + // the extracted values + handlers + identity_provider. + // This avoids coupling the accept loop to AlknetEndpoint. + crate::dispatch::dispatch_connection( + conn, alpn, fingerprint, remote_addr, + &handlers, &identity_provider, + ); + }); + } + } + } +} + +#[cfg(feature = "quinn")] +pub(crate) fn extract_alpn(connection: &quinn::Connection) -> Vec { + use quinn::crypto::rustls::HandshakeData; + if let Some(data) = connection.handshake_data() { + if let Ok(hs) = data.downcast::() { + if let Some(protocol) = hs.protocol { + return protocol; + } + } + } + Vec::new() +} + +#[cfg(feature = "quinn")] +pub(crate) fn extract_client_fingerprint(connection: &quinn::Connection) -> Option { + let identity = connection.peer_identity()?; + let certs = identity + .downcast::>() + .ok()?; + let leaf = certs.first()?; + alknet_core::fingerprint::fingerprint_from_cert_der(leaf.as_ref()) +} +``` + +### Design note: accept loop → dispatch coupling + +The accept loop needs to call `dispatch` but doesn't have a reference to `AlknetEndpoint`. +Two approaches: + +**Option A (recommended):** The accept loop calls a free function in `crate::dispatch` +that takes `(Connection, alpn, fingerprint, remote_addr, &HandlerRegistry, &IdentityProvider)`. +This is the same pattern the old code used — `dispatch_quinn` was a free function that +took `&HandlerRegistry` and `&IdentityProvider`. The `AlknetEndpoint::dispatch` method +delegates to this same free function. + +**Option B:** The accept loop receives a closure or `Arc`. This couples +the accept loop to the endpoint struct, which is unnecessary — the accept loop only needs +the handler registry and identity provider. + +Use **Option A** — it's the simplest, matches the old code's pattern, and keeps the +accept loop decoupled from the endpoint struct. + +### What stays in core + +The old `run_quinn_accept_loop`, `extract_quinn_alpn`, `extract_quinn_client_fingerprint` +in `endpoint.rs` are **not deleted** — they stay as duplicates. The prune happens in Phase 4. + +## Acceptance Criteria + +- [ ] `accept/quinn.rs` contains `run_accept_loop`, `extract_alpn`, `extract_client_fingerprint` +- [ ] `run_accept_loop` spawns a task per accepted connection, handles shutdown signal +- [ ] `extract_alpn` extracts the negotiated ALPN from quinn handshake data +- [ ] `extract_client_fingerprint` extracts the client cert fingerprint via `alknet_core::fingerprint` +- [ ] Accept loop calls `crate::dispatch::dispatch_connection()` (free function) with extracted values +- [ ] All code gated on `#[cfg(feature = "quinn")]` +- [ ] All imports use `alknet_core::` (not `crate::` from core) +- [ ] `cargo check -p alknet-endpoint --features quinn` succeeds +- [ ] `cargo clippy -p alknet-endpoint --features quinn` succeeds with no warnings +- [ ] `cargo test -p alknet-core` still passes (old code untouched) + +## References + +- docs/research/alknet-crate-extraction/findings.md — Phase 2, accept/quinn module +- docs/architecture/crates/endpoint/README.md — Accept loops (lines 177-193) +- crates/alknet-core/src/endpoint.rs — lines 287-388 (source code to extract) + +## Notes + +> This is a straightforward extraction — the quinn accept loop logic doesn't change. +> The main adaptation is calling `crate::dispatch::dispatch_connection()` instead of +> the old `dispatch_quinn()`. The accept loop is a free function, not a method on +> `AlknetEndpoint`, to keep it decoupled. The old code in `endpoint.rs` is NOT deleted. + +## Summary + +> To be filled on completion diff --git a/tasks/endpoint/accept-tcp-tls.md b/tasks/endpoint/accept-tcp-tls.md new file mode 100644 index 0000000..c16d6f4 --- /dev/null +++ b/tasks/endpoint/accept-tcp-tls.md @@ -0,0 +1,169 @@ +--- +id: endpoint/accept-tcp-tls +name: Implement TCP+TLS accept loop and extractors in alknet-endpoint (new code) +status: pending +depends_on: [endpoint/dispatch] +scope: narrow +risk: medium +impact: component +level: implementation +--- + +## Description + +Phase 2, Task 7 of the crate extraction. Implement the TCP+TLS accept loop and +transport-specific extractors in `crates/alknet-endpoint/src/accept/tcp_tls.rs`. + +**This is new code** — the current `endpoint.rs` does not have a TCP+TLS accept loop. +It must be written fresh, following the same pattern as the quinn and iroh accept loops +but adapted for TCP+TLS transport. + +### Design + +The TCP+TLS accept loop follows the same pattern as quinn and iroh: + +1. `tcp_listener.accept()` → get a `TcpStream` +2. `tls_acceptor.accept(tcp_stream)` → TLS handshake → get a `TlsStream` +3. Extract ALPN from the TLS session +4. Extract client fingerprint from the peer certificate chain +5. Convert to `Connection::from_bidi(tls_stream)` (the `TlsStream` is + `AsyncRead + AsyncWrite`) +6. Call `crate::dispatch::dispatch_connection()` + +### Implementation sketch + +```rust +// accept/tcp_tls.rs +#[cfg(feature = "tcp")] +use std::net::SocketAddr; +#[cfg(feature = "tcp")] +use std::sync::Arc; +#[cfg(feature = "tcp")] +use tokio::sync::watch; +#[cfg(feature = "tcp")] +use tracing::{debug, warn}; + +#[cfg(feature = "tcp")] +use alknet_core::auth::IdentityProvider; +#[cfg(feature = "tcp")] +use alknet_core::types::Connection; + +#[cfg(feature = "tcp")] +use crate::registry::HandlerRegistry; + +#[cfg(feature = "tcp")] +pub(crate) async fn run_accept_loop( + listener: tokio::net::TcpListener, + acceptor: tokio_rustls::TlsAcceptor, + handlers: Arc, + identity_provider: Arc, + shutdown_rx: &mut watch::Receiver, +) { + loop { + tokio::select! { + _ = shutdown_rx.changed() => { + debug!("tcp+tls accept loop: shutdown signaled"); + break; + } + result = listener.accept() => { + let (tcp_stream, remote_addr) = match result { + Ok(r) => r, + Err(e) => { + warn!("tcp+tls accept failed: {e}"); + continue; + } + }; + let acceptor = acceptor.clone(); + let handlers = handlers.clone(); + let identity_provider = identity_provider.clone(); + tokio::spawn(async move { + let tls_stream = match acceptor.accept(tcp_stream).await { + Ok(s) => s, + Err(e) => { + warn!("tcp+tls TLS handshake failure: {e}"); + return; + } + }; + let (alpn, fingerprint) = extract_tls_session_info(&tls_stream); + let conn = Connection::from_bidi(tls_stream); + crate::dispatch::dispatch_connection( + conn, alpn, fingerprint, Some(remote_addr), + &handlers, &identity_provider, + ); + }); + } + } + } +} + +#[cfg(feature = "tcp")] +fn extract_tls_session_info( + tls_stream: &tokio_rustls::server::TlsStream, +) -> (Vec, Option) { + let (_, session) = tls_stream.get_ref(); + let alpn = session.alpn_protocol().map(|a| a.to_vec()).unwrap_or_default(); + let fingerprint = session + .peer_certificates() + .and_then(|certs| certs.first()) + .and_then(|cert| alknet_core::fingerprint::fingerprint_from_cert_der(cert.as_ref())); + (alpn, fingerprint) +} +``` + +### Key design decisions + +1. **`Connection::from_bidi`**: The `TlsStream` implements `AsyncRead + AsyncWrite`, + so it can be passed directly to `Connection::from_bidi`. No `QuicStream` wrapper needed + (that's the Phase 6 fix for `alknet-http`). + +2. **ALPN extraction**: `session.alpn_protocol()` returns the negotiated ALPN from the TLS + session. This is the standard rustls API. + +3. **Fingerprint extraction**: `session.peer_certificates()` returns the peer's certificate + chain. The leaf cert's fingerprint is extracted via `alknet_core::fingerprint::fingerprint_from_cert_der`. + +4. **`remote_addr`**: Available from `TcpListener::accept()` — passed to dispatch. + +5. **Feature gate**: All code gated on `#[cfg(feature = "tcp")]`. + +### What stays in core + +There is no existing TCP+TLS accept loop in `endpoint.rs` — this is entirely new code. +No duplication, no prune needed. + +## Acceptance Criteria + +- [ ] `accept/tcp_tls.rs` contains `run_accept_loop`, `extract_tls_session_info` +- [ ] `run_accept_loop` accepts TCP connections, performs TLS handshake, spawns handler task +- [ ] `run_accept_loop` handles shutdown signal via `watch::Receiver` +- [ ] `extract_tls_session_info` extracts ALPN from TLS session +- [ ] `extract_tls_session_info` extracts client cert fingerprint via `alknet_core::fingerprint` +- [ ] Accept loop calls `crate::dispatch::dispatch_connection()` with extracted values +- [ ] `Connection::from_bidi(tls_stream)` used (no hand-rolled wrapper) +- [ ] `remote_addr` passed from `TcpListener::accept()` +- [ ] All code gated on `#[cfg(feature = "tcp")]` +- [ ] All imports use `alknet_core::` (not `crate::` from core) +- [ ] `cargo check -p alknet-endpoint --features tcp` succeeds +- [ ] `cargo clippy -p alknet-endpoint --features tcp` succeeds with no warnings +- [ ] `cargo test -p alknet-core` still passes (old code untouched) + +## References + +- docs/research/alknet-crate-extraction/findings.md — Phase 2, accept/tcp_tls module +- docs/architecture/crates/endpoint/README.md — Accept loops (lines 177-193), TcpTlsListener (lines 164-175) +- docs/architecture/decisions/083-endpoint-as-accept-loop-runner.md — ADR-083 +- docs/architecture/decisions/065-connection-from-stream-generic-single-stream.md — ADR-065 (Connection::from_bidi) +- crates/alknet-core/src/endpoint.rs — lines 287-388 (quinn accept loop, reference pattern) + +## Notes + +> This is the only genuinely new code in Phase 2 — the current `endpoint.rs` has no +> TCP+TLS accept loop. It follows the same pattern as the quinn and iroh accept loops +> but uses `TcpListener::accept()` + `TlsAcceptor::accept()` + `Connection::from_bidi`. +> The `TlsStream` is already `AsyncRead + AsyncWrite` — no wrapper needed. +> Risk is medium because it's new code, but the pattern is well-established by the +> quinn and iroh loops. + +## Summary + +> To be filled on completion diff --git a/tasks/endpoint/crate-init.md b/tasks/endpoint/crate-init.md new file mode 100644 index 0000000..20e8280 --- /dev/null +++ b/tasks/endpoint/crate-init.md @@ -0,0 +1,128 @@ +--- +id: endpoint/crate-init +name: Initialize alknet-endpoint crate with Cargo.toml, dependencies, and module skeleton +status: pending +depends_on: [tls/review-tls] +scope: moderate +risk: low +impact: project +level: implementation +--- + +## Description + +Phase 2, Task 1 of the crate extraction (per `docs/research/alknet-crate-extraction/findings.md`). +Initialize the `alknet-endpoint` crate from scratch. This crate provides the server-side +multi-transport accept-loop runner — `AlknetEndpoint`, `HandlerRegistry`, and the +transport-specific accept loops (quinn, iroh, TCP+TLS). + +The endpoint takes **pre-built transports** via builder methods (`with_quinn`, `with_iroh`, +`with_tcp_tls`) — it does not build transports and does not depend on `alknet-tls`. The +assembly layer builds transports from `alknet-tls`'s `TlsServerConfig` and hands them to +the endpoint. See `docs/architecture/crates/endpoint/README.md` for the full spec. + +### Crate setup + +Create `crates/alknet-endpoint/` with: + +- `Cargo.toml` — package metadata, dependencies, feature flags +- `src/lib.rs` — crate root with module declarations and re-exports +- Module skeleton files for: + - `src/registry.rs` — `HandlerRegistry` (extracted from `endpoint.rs` lines 66-116) + - `src/endpoint.rs` — `AlknetEndpoint` struct, `new`, builder methods, `run`, `shutdown` (built fresh against ADR-083 shape) + - `src/dispatch.rs` — `dispatch` (public), `build_auth_context`, ACME guard (extracted from `endpoint.rs` lines 330-490) + - `src/accept/mod.rs` — accept module declarations + - `src/accept/quinn.rs` — `dispatch_quinn`, `run_quinn_accept_loop`, `extract_quinn_alpn`, `extract_quinn_client_fingerprint` (extracted from `endpoint.rs` lines 287-388) + - `src/accept/iroh.rs` — `dispatch_iroh`, `run_iroh_accept_loop`, `extract_iroh_client_fingerprint` (extracted from `endpoint.rs` lines 390-472) + - `src/accept/tcp_tls.rs` — `dispatch_tcp_tls`, `run_tcp_tls_accept_loop`, `extract_tcp_tls_alpn`, `extract_tcp_tls_client_fingerprint` (new code, not in current `endpoint.rs`) + +### Dependencies + +Per the findings (Phase 2) and the architecture spec: + +| Crate | Purpose | +|-------|---------| +| `alknet-core` | `Connection`, `ProtocolHandler`, `AuthContext`, `IdentityProvider`, `DynamicConfig` (workspace path) | +| `quinn` 0.11 | QUIC transport (optional, feature-gated) | +| `iroh` 0.28 | Iroh transport (optional, feature-gated) | +| `tokio-rustls` 0.26 | TCP+TLS transport (optional, feature-gated) | +| `tokio` 1 (full) | Async runtime, spawn, watch, TcpListener | +| `arc-swap` 1 | `DynamicConfig` | +| `tracing` 0.1 | Structured logging | + +The endpoint does **not** depend on `alknet-tls` — it takes pre-built transports. TLS config +construction stays at the assembly layer. + +### Feature flags + +```toml +[features] +default = [] +quinn = ["dep:quinn", "alknet-core/quinn"] # with_quinn — quinn accept loop +iroh = ["dep:iroh", "alknet-core/iroh"] # with_iroh — iroh accept loop +tcp = ["dep:tokio-rustls"] # with_tcp_tls — TCP+TLS accept loop +``` + +The `quinn`/`iroh` features pull the corresponding features on `alknet-core` (for +`Connection::from_quinn` / `from_iroh` — the constructors stay in core). A deployment +enables the features for the transports it runs. + +### Workspace Cargo.toml + +Add `crates/alknet-endpoint` to the workspace `members` list in the root `Cargo.toml`. + +### Module skeleton + +```rust +// src/lib.rs +//! alknet-endpoint: Server-side multi-transport accept-loop runner. +//! +//! `AlknetEndpoint` takes pre-built transports (quinn, iroh, TCP+TLS) via +//! builder methods, runs their accept loops inside `run()`, and dispatches +//! each accepted connection to the registered `ProtocolHandler` by ALPN. +//! +//! The endpoint does not build transports and does not depend on +//! `alknet-tls` — transport construction is the assembly layer's concern. + +pub mod accept; +pub mod dispatch; +pub mod endpoint; +pub mod registry; + +// Re-exports (filled in by subsequent tasks) +``` + +Each module file gets a doc comment and `// TODO: implement` marker. + +## Acceptance Criteria + +- [ ] `crates/alknet-endpoint/Cargo.toml` exists with all dependencies and feature flags +- [ ] `crates/alknet-endpoint/src/lib.rs` exists with module declarations +- [ ] Module skeleton files exist: `registry.rs`, `endpoint.rs`, `dispatch.rs`, `accept/mod.rs`, `accept/quinn.rs`, `accept/iroh.rs`, `accept/tcp_tls.rs` +- [ ] Root `Cargo.toml` `members` list includes `crates/alknet-endpoint` +- [ ] `cargo check -p alknet-endpoint` succeeds +- [ ] `cargo clippy -p alknet-endpoint` succeeds with no warnings +- [ ] Dual licensing: `MIT OR Apache-2.0` (workspace-inherited) +- [ ] `alknet-core` dependency uses workspace path (`path = "../alknet-core"`) +- [ ] No dependency on `alknet-tls` (endpoint takes pre-built transports) +- [ ] Feature flags: `quinn`, `iroh`, `tcp` (all optional, default off) + +## References + +- docs/research/alknet-crate-extraction/findings.md — Phase 2 +- docs/architecture/crates/endpoint/README.md — full architecture spec +- docs/architecture/decisions/083-endpoint-as-accept-loop-runner.md — ADR-083 +- crates/alknet-core/Cargo.toml — reference for dep versions +- crates/alknet-tls/Cargo.toml — reference for feature flag pattern + +## Notes + +> This is the foundational setup task for alknet-endpoint. All subsequent endpoint/* +> tasks depend on this one. The crate has no alknet dependencies beyond core. +> The endpoint does NOT depend on alknet-tls — it takes pre-built transports via +> builder methods. The `quinn`/`iroh` features pull the corresponding features on +> `alknet-core` for `Connection::from_quinn` / `from_iroh` constructors. + +## Summary + +> To be filled on completion diff --git a/tasks/endpoint/dispatch.md b/tasks/endpoint/dispatch.md new file mode 100644 index 0000000..128af22 --- /dev/null +++ b/tasks/endpoint/dispatch.md @@ -0,0 +1,162 @@ +--- +id: endpoint/dispatch +name: Implement public dispatch, build_auth_context, and ACME guard +status: pending +depends_on: [endpoint/endpoint-core] +scope: narrow +risk: low +impact: component +level: implementation +--- + +## Description + +Phase 2, Task 4 of the crate extraction. Implement the shared `dispatch` method, +`build_auth_context`, and the ACME `acme-tls/1` guard in +`crates/alknet-endpoint/src/dispatch.rs`. + +`dispatch` is the shared dispatch path for every transport — the endpoint's own accept +loops call it after transport-specific extraction (ALPN, fingerprint, remote address), +and external dispatch callers (SSH channels, future WebTransport streams) call it after +their own extraction. It is **public** and **synchronous** (non-async): performs the +ACME guard, handler lookup, `build_auth_context`, and `tokio::spawn`s the handler. + +### Types to extract / build + +From `endpoint.rs` lines 330-490: + +| Type/Function | Lines | Destination | +|---------------|-------|-------------| +| `dispatch_quinn()` | 330-365 | Adapted into `dispatch()` (public, transport-agnostic) | +| `extract_quinn_alpn()` | 368-378 | Stays in `accept/quinn.rs` (Task 5) | +| `extract_quinn_client_fingerprint()` | 381-388 | Stays in `accept/quinn.rs` (Task 5) | +| `dispatch_iroh()` | 440-466 | Adapted into `dispatch()` (public, transport-agnostic) | +| `extract_iroh_client_fingerprint()` | 469-472 | Stays in `accept/iroh.rs` (Task 6) | +| `build_auth_context()` | 475-490 | `dispatch.rs` | + +### `dispatch` (public) + +The new `dispatch` is transport-agnostic — it receives already-extracted values instead +of extracting them from a transport-specific connection: + +```rust +/// Dispatch an accepted connection to its `ProtocolHandler` by ALPN. +/// +/// Synchronous (non-async): performs the ACME guard, handler lookup, +/// `build_auth_context`, and `tokio::spawn`s the handler. Returns +/// immediately after spawning. +/// +/// Public for connection-internal multiplexing shapes (SSH channels, +/// future WebTransport streams) that the endpoint can't own. +pub fn dispatch( + &self, + connection: Connection, + alpn: Vec, + fingerprint: Option, + remote_addr: Option, +) { + // ACME guard + #[cfg(feature = "acme")] + if alpn == b"acme-tls/1" { + debug!("acme-tls/1 challenge connection; closing"); + connection.close(0u32.into(), b"acme done"); + return; + } + + let handler = match self.handlers.get(&alpn) { + Some(h) => h.clone(), + None => { + connection.close(0u32.into(), b"no handler"); + warn!("dispatch: no handler for ALPN {:?}", String::from_utf8_lossy(&alpn)); + return; + } + }; + + let auth = build_auth_context(&alpn, remote_addr, fingerprint, &self.identity_provider); + tokio::spawn(async move { + if let Err(e) = handler.handle(connection, &auth).await { + error!("handler returned error: {e}"); + } + }); +} +``` + +Key differences from the old `dispatch_quinn` / `dispatch_iroh`: + +1. **Takes `Connection` not `quinn::Connection` / `iroh::Connection`**: The accept loop + converts the transport-specific connection to `alknet_core::Connection` before calling + `dispatch`. This is the same pattern the old code used (`Connection::from_quinn_with_alpn`, + `Connection::from_iroh`). + +2. **Takes pre-extracted values**: `alpn`, `fingerprint`, `remote_addr` are passed in + rather than extracted inside `dispatch`. The extraction is transport-specific and + lives in the accept loop modules. + +3. **No `EndpointError`**: Handler-not-found is swallowed (close + log). No error return. + +4. **ACME guard is `#[cfg(feature = "acme")]`**: The `acme` feature is not on + `alknet-endpoint` itself (the endpoint doesn't build ACME configs), but the guard + is kept for forward-compatibility. If the assembly layer registers an ACME handler, + the guard prevents it from being dispatched as a normal protocol handler. + +### `build_auth_context` + +Extracted from `endpoint.rs` lines 475-490, with imports updated: + +```rust +pub(crate) fn build_auth_context( + alpn: &[u8], + remote_addr: Option, + tls_client_fingerprint: Option, + identity_provider: &Arc, +) -> AuthContext { + let identity = tls_client_fingerprint + .as_ref() + .and_then(|fp| identity_provider.resolve_from_fingerprint(fp)); + AuthContext { + identity, + alpn: alpn.to_vec(), + remote_addr, + tls_client_fingerprint, + } +} +``` + +### What stays in core + +The old `dispatch_quinn`, `dispatch_iroh`, and `build_auth_context` in `endpoint.rs` are +**not deleted** — they stay as duplicates. The prune happens in Phase 4. + +## Acceptance Criteria + +- [ ] `dispatch()` is public, synchronous, takes `&self`, `Connection`, `alpn`, `fingerprint`, `remote_addr` +- [ ] `dispatch()` performs ACME guard (`acme-tls/1` → close + return) when `acme` feature enabled +- [ ] `dispatch()` looks up handler by ALPN, closes connection + logs warning on miss +- [ ] `dispatch()` calls `build_auth_context` and `tokio::spawn`s the handler +- [ ] `build_auth_context()` resolves identity from fingerprint via `IdentityProvider` +- [ ] `build_auth_context()` returns `AuthContext` with all fields populated +- [ ] No `EndpointError` — handler-not-found is swallowed (close + log) +- [ ] All imports use `alknet_core::` (not `crate::`) +- [ ] Feature gates: `acme` guard is `#[cfg(feature = "acme")]`; rest is always available +- [ ] `cargo check -p alknet-endpoint` succeeds (all feature combos) +- [ ] `cargo clippy -p alknet-endpoint` succeeds with no warnings +- [ ] `cargo test -p alknet-core` still passes (old code untouched) + +## References + +- docs/research/alknet-crate-extraction/findings.md — Phase 2, dispatch module +- docs/architecture/crates/endpoint/README.md — dispatch spec (lines 195-211) +- docs/architecture/decisions/083-endpoint-as-accept-loop-runner.md — ADR-083 +- crates/alknet-core/src/endpoint.rs — lines 330-490 (source code to extract/adapt) + +## Notes + +> `dispatch` is the shared dispatch path for all transports. It's public because +> connection-internal multiplexing shapes (SSH channels, future WT streams) need to +> call it after their own extraction. The accept loops (quinn, iroh, TCP+TLS) call +> it internally. The old `dispatch_quinn` and `dispatch_iroh` are merged into one +> transport-agnostic `dispatch`. The old code in `endpoint.rs` is NOT deleted. + +## Summary + +> To be filled on completion diff --git a/tasks/endpoint/endpoint-core.md b/tasks/endpoint/endpoint-core.md new file mode 100644 index 0000000..71a9a21 --- /dev/null +++ b/tasks/endpoint/endpoint-core.md @@ -0,0 +1,211 @@ +--- +id: endpoint/endpoint-core +name: Implement AlknetEndpoint struct, new, builder methods, run, and shutdown +status: pending +depends_on: [endpoint/registry] +scope: moderate +risk: medium +impact: component +level: implementation +--- + +## Description + +Phase 2, Task 3 of the crate extraction. Implement the `AlknetEndpoint` struct and its +core methods (`new`, `with_quinn`, `with_iroh`, `with_tcp_tls`, `run`, `shutdown`, +`shutdown_sender`) in `crates/alknet-endpoint/src/endpoint.rs`. + +This is a **fresh build against the ADR-083 shape**, not a direct copy of the old +`endpoint.rs`. The old `AlknetEndpoint::new()` took a `StaticConfig` and built transports +internally. The new `new()` takes no `StaticConfig` and no TLS config — the assembly layer +builds transports and hands them to the endpoint via builder methods. + +### Target shape (per ADR-083 / architecture spec) + +```rust +pub struct AlknetEndpoint { + #[cfg(feature = "quinn")] + quinn: Option, + #[cfg(feature = "iroh")] + iroh: Option, + #[cfg(feature = "tcp")] + tcp_tls: Option, // (TcpListener, TlsAcceptor) + handlers: Arc, + dynamic: Arc>, + identity_provider: Arc, + shutdown_tx: watch::Sender, + shutdown_rx: watch::Receiver, + drain_timeout: Duration, +} + +impl AlknetEndpoint { + pub fn new( + handlers: HandlerRegistry, + dynamic: Arc>, + identity_provider: Arc, + drain_timeout: Duration, + ) -> Self; + + #[cfg(feature = "quinn")] + pub fn with_quinn(mut self, endpoint: quinn::Endpoint) -> Self; + + #[cfg(feature = "iroh")] + pub fn with_iroh(mut self, endpoint: iroh::Endpoint) -> Self; + + #[cfg(feature = "tcp")] + pub fn with_tcp_tls( + mut self, + listener: tokio::net::TcpListener, + acceptor: tokio_rustls::TlsAcceptor, + ) -> Self; + + pub fn shutdown_sender(&self) -> watch::Sender; + + pub async fn run(self: Arc); + + /// Infallible — signals all owned accept loops to stop, waits for + /// in-flight handlers with drain_timeout, then forcefully closes. + pub async fn shutdown(&self); +} +``` + +### Key differences from the old `endpoint.rs` + +1. **`new()` takes no `StaticConfig`**: The old `new()` read `listen_addr`, `tls_identity`, + `iroh_relay` from `StaticConfig` and built transports internally. The new `new()` takes + only `HandlerRegistry`, `DynamicConfig`, `IdentityProvider`, and `drain_timeout` — no + transport construction. The assembly layer reads `StaticConfig` and builds transports. + +2. **Builder methods instead of internal construction**: `with_quinn(endpoint)`, + `with_iroh(endpoint)`, `with_tcp_tls(listener, acceptor)` replace the internal + `TlsSetup::new()` + `build_quinn_server_config_from_rustls()` + `build_iroh_endpoint()` + chain. The endpoint receives pre-built, pre-bound transports. + +3. **`TcpTlsListener` type**: A new type alias for the TCP+TLS transport pair: + ```rust + #[cfg(feature = "tcp")] + pub(crate) type TcpTlsListener = (tokio::net::TcpListener, tokio_rustls::TlsAcceptor); + ``` + +4. **`shutdown()` is infallible**: Returns `()` not `Result<(), EndpointError>`. The old + `shutdown()` returned `Result` but could never actually fail (the `?` was on + `iroh.close().await` which is infallible). The new `shutdown()` is `async fn shutdown(&self)` + with no `Result`. + +5. **No `EndpointError`**: The error type is removed entirely. `BindFailed` is vestigial + (the endpoint doesn't bind). `HandlerNotFound` is swallowed by `dispatch` (close + log). + `TlsConfig` was already removed by ADR-083. + +6. **No `acme_state_handle` field**: ACME state lives on `TlsServerConfig` in `alknet-tls` + now. The endpoint doesn't see it. + +7. **`run()` spawns accept loops for each active transport**: Quinn, iroh, and TCP+TLS + each get their own `tokio::spawn`'d accept loop. The old `run()` only handled quinn + and iroh; the new one adds TCP+TLS. + +### `run()` implementation + +```rust +pub async fn run(self: Arc) { + let mut tasks: Vec> = Vec::new(); + + #[cfg(feature = "quinn")] + if let Some(quinn) = &self.quinn { + let quinn = quinn.clone(); + let handlers = self.handlers.clone(); + let identity_provider = self.identity_provider.clone(); + let mut shutdown_rx = self.shutdown_rx.clone(); + tasks.push(tokio::spawn(async move { + crate::accept::quinn::run_accept_loop(quinn, handlers, identity_provider, &mut shutdown_rx).await; + })); + } + + #[cfg(feature = "iroh")] + if let Some(iroh) = &self.iroh { + // ... same pattern for iroh + } + + #[cfg(feature = "tcp")] + if let Some((listener, acceptor)) = self.tcp_tls.take() { + // ... same pattern for TCP+TLS + } + + for task in tasks { + let _ = task.await; + } +} +``` + +### `shutdown()` implementation + +```rust +pub async fn shutdown(&self) { + let _ = self.shutdown_tx.send(true); + + #[cfg(feature = "quinn")] + if let Some(quinn) = &self.quinn { + quinn.close(0u32.into(), b"shutdown"); + } + + #[cfg(feature = "iroh")] + if let Some(iroh) = &self.iroh { + iroh.close().await; + } + + #[cfg(feature = "tcp")] + // TCP+TLS: the accept loop watches shutdown_rx; no explicit close needed + // (the listener is dropped when the endpoint is dropped) + + tokio::time::sleep(self.drain_timeout).await; + + #[cfg(feature = "quinn")] + if let Some(quinn) = &self.quinn { + quinn.wait_idle().await; + } +} +``` + +### What stays in core + +The old `AlknetEndpoint` in `endpoint.rs` lines 118-277 is **not deleted** — it stays as a +duplicate. The prune happens in Phase 4. This task only adds code to `alknet-endpoint`. + +## Acceptance Criteria + +- [ ] `AlknetEndpoint` struct defined with all fields (quinn, iroh, tcp_tls, handlers, dynamic, identity_provider, shutdown_tx/rx, drain_timeout) +- [ ] `AlknetEndpoint::new()` takes `HandlerRegistry`, `Arc>`, `Arc`, `Duration` — no `StaticConfig`, no TLS config +- [ ] `with_quinn(endpoint)` builder method (feature-gated on `quinn`) +- [ ] `with_iroh(endpoint)` builder method (feature-gated on `iroh`) +- [ ] `with_tcp_tls(listener, acceptor)` builder method (feature-gated on `tcp`) +- [ ] `TcpTlsListener` type alias defined (feature-gated on `tcp`) +- [ ] `shutdown_sender()` returns a clone of the shutdown watch sender +- [ ] `run()` spawns accept loops for each active transport +- [ ] `shutdown()` is infallible (`async fn shutdown(&self)`, no `Result`) +- [ ] `Debug` impl for `AlknetEndpoint` (lists handlers, drain_timeout; no transport internals) +- [ ] No `EndpointError` type (removed) +- [ ] No `acme_state_handle` field (ACME lives in `alknet-tls`) +- [ ] No `has_iroh_identity` function (transport-building decision moved to assembly layer) +- [ ] Feature gates correct: `quinn`, `iroh`, `tcp` each gate their respective fields/methods +- [ ] `cargo check -p alknet-endpoint` succeeds (all feature combos) +- [ ] `cargo clippy -p alknet-endpoint` succeeds with no warnings +- [ ] `cargo test -p alknet-core` still passes (old code untouched) + +## References + +- docs/research/alknet-crate-extraction/findings.md — Phase 2, endpoint module +- docs/architecture/crates/endpoint/README.md — AlknetEndpoint spec (lines 48-103) +- docs/architecture/decisions/083-endpoint-as-accept-loop-runner.md — ADR-083 +- crates/alknet-core/src/endpoint.rs — lines 118-277 (old code, reference only) + +## Notes + +> This is the core structural task of Phase 2. The `AlknetEndpoint` is built fresh +> against the ADR-083 shape — it's not a copy-paste of the old code. The key +> difference: the old `new()` built transports internally from `StaticConfig`; the +> new `new()` takes no transport config and receives pre-built transports via +> builder methods. `EndpointError` is removed entirely. `shutdown()` is infallible. +> The old code in `endpoint.rs` is NOT deleted — that's Phase 4. + +## Summary + +> To be filled on completion diff --git a/tasks/endpoint/registry.md b/tasks/endpoint/registry.md new file mode 100644 index 0000000..0489d8d --- /dev/null +++ b/tasks/endpoint/registry.md @@ -0,0 +1,99 @@ +--- +id: endpoint/registry +name: Extract HandlerRegistry from alknet-core/endpoint.rs into alknet-endpoint +status: pending +depends_on: [endpoint/crate-init] +scope: narrow +risk: low +impact: component +level: implementation +--- + +## Description + +Phase 2, Task 2 of the crate extraction. Extract the `HandlerRegistry` type from +`crates/alknet-core/src/endpoint.rs` (lines 66-116) into `crates/alknet-endpoint/src/registry.rs`. + +The old code **stays** in `endpoint.rs` (duplicated) — no breakage. The new crate is +self-contained and builds standalone. + +### Types to extract + +From `endpoint.rs` lines 66-116: + +| Type/Function | Lines | Destination | +|---------------|-------|-------------| +| `HandlerRegistry` struct | 66-68 | `registry.rs` | +| `HandlerRegistry::new()` | 71-75 | `registry.rs` | +| `HandlerRegistry::register()` | 77-86 | `registry.rs` | +| `HandlerRegistry::get()` | 88-90 | `registry.rs` | +| `HandlerRegistry::alpn_strings()` | 92-94 | `registry.rs` | +| `impl Default for HandlerRegistry` | 97-101 | `registry.rs` | +| `impl Debug for HandlerRegistry` | 103-116 | `registry.rs` | + +### Adaptations + +1. **Imports**: Update `crate::types::ProtocolHandler` to `alknet_core::types::ProtocolHandler`. +2. **No other changes**: The `HandlerRegistry` is a pure data structure with no transport deps. + It maps ALPN byte strings to `ProtocolHandler` instances. No feature gates needed — it's + always available regardless of which transports are enabled. + +### Public API + +```rust +// registry.rs + +/// Maps ALPN byte strings to `ProtocolHandler` instances. +/// Registered statically at startup by the assembly layer; the endpoint +/// dispatches by looking up the negotiated ALPN. +pub struct HandlerRegistry { + handlers: HashMap<&'static [u8], Arc>, +} + +impl HandlerRegistry { + pub fn new() -> Self; + /// Insert a handler. Panics if the ALPN is already registered. + pub fn register(&mut self, handler: Arc); + /// Look up a handler by ALPN string. + pub fn get(&self, alpn: &[u8]) -> Option<&Arc>; + /// Return all registered ALPN strings. + pub fn alpn_strings(&self) -> Vec>; +} +``` + +### What stays in core + +The old `HandlerRegistry` in `endpoint.rs` lines 66-116 is **not deleted** — it stays as a +duplicate. The prune happens in Phase 4. This task only adds code to `alknet-endpoint`. + +## Acceptance Criteria + +- [ ] `crates/alknet-endpoint/src/registry.rs` contains `HandlerRegistry` with all methods +- [ ] `HandlerRegistry::new()` creates an empty registry +- [ ] `HandlerRegistry::register()` inserts a handler, panics on duplicate ALPN +- [ ] `HandlerRegistry::get()` looks up by ALPN, returns `None` for unknown +- [ ] `HandlerRegistry::alpn_strings()` returns all registered ALPNs +- [ ] `Default` impl delegates to `new()` +- [ ] `Debug` impl lists ALPNs without exposing handler internals +- [ ] All imports use `alknet_core::` (not `crate::`) +- [ ] No feature gates (always available) +- [ ] `cargo check -p alknet-endpoint` succeeds +- [ ] `cargo clippy -p alknet-endpoint` succeeds with no warnings +- [ ] `cargo test -p alknet-core` still passes (old code untouched) + +## References + +- docs/research/alknet-crate-extraction/findings.md — Phase 2, registry module +- docs/architecture/crates/endpoint/README.md — HandlerRegistry spec +- crates/alknet-core/src/endpoint.rs — lines 66-116 (source code to extract) + +## Notes + +> This is the simplest extraction in Phase 2 — ~50 lines of pure data structure. +> `HandlerRegistry` has no transport deps and no feature gates. It's extracted +> first because `AlknetEndpoint::new()` takes it as a parameter. The old code in +> `endpoint.rs` is NOT deleted — that's Phase 4. + +## Summary + +> To be filled on completion diff --git a/tasks/endpoint/review-endpoint.md b/tasks/endpoint/review-endpoint.md new file mode 100644 index 0000000..8fe7696 --- /dev/null +++ b/tasks/endpoint/review-endpoint.md @@ -0,0 +1,144 @@ +--- +id: endpoint/review-endpoint +name: Review alknet-endpoint implementation for spec conformance, API shape, and test coverage +status: pending +depends_on: [endpoint/tests] +scope: moderate +risk: low +impact: phase +level: review +--- + +## Description + +Phase 2 review checkpoint. Verify the `alknet-endpoint` crate is spec-conformant, +self-contained, and ready for downstream consumption by the assembly layer (Phase 4+ +and beyond). The crate must match the ADR-083 shape: `new()` takes no `StaticConfig`, +transports are injected via builder methods, `dispatch` is public, `shutdown()` is +infallible, and `EndpointError` is removed. + +### Review Checklist + +1. **Crate structure**: + - Module layout matches spec: `registry.rs`, `endpoint.rs`, `dispatch.rs`, `accept/{quinn,iroh,tcp_tls}.rs` + - Public API types: `AlknetEndpoint`, `HandlerRegistry` + - Re-exports in `lib.rs` are correct and minimal + - No dependency on `alknet-tls` (endpoint takes pre-built transports) + +2. **`AlknetEndpoint` API shape (ADR-083)**: + - `new(handlers, dynamic, identity_provider, drain_timeout)` — no `StaticConfig`, no TLS config + - `with_quinn(endpoint: quinn::Endpoint)` builder (feature-gated on `quinn`) + - `with_iroh(endpoint: iroh::Endpoint)` builder (feature-gated on `iroh`) + - `with_tcp_tls(listener, acceptor)` builder (feature-gated on `tcp`) + - `TcpTlsListener` type alias (feature-gated on `tcp`) + - `shutdown_sender()` returns `watch::Sender` + - `run(self: Arc)` spawns accept loops for each active transport + - `shutdown(&self)` is infallible (`async fn shutdown(&self)`, no `Result`) + - `Debug` impl lists handlers and drain_timeout (no transport internals) + +3. **`HandlerRegistry`**: + - `new()`, `register()`, `get()`, `alpn_strings()` methods + - `register()` panics on duplicate ALPN + - `Default` impl delegates to `new()` + - `Debug` impl lists ALPNs without exposing handler internals + - No feature gates (always available) + +4. **`dispatch` (public)**: + - Takes `&self`, `Connection`, `alpn`, `fingerprint`, `remote_addr` + - Synchronous (non-async) — spawns handler and returns immediately + - ACME guard (`acme-tls/1` → close + return) when `acme` feature enabled + - Handler-not-found is swallowed (close + log, no error) + - Calls `build_auth_context` and `tokio::spawn`s the handler + +5. **`build_auth_context`**: + - Resolves identity from fingerprint via `IdentityProvider` + - Returns `AuthContext` with all fields populated + - Feature-gated on `#[cfg(any(feature = "quinn", feature = "iroh"))]` + +6. **Accept loops**: + - Quinn: `run_accept_loop` spawns per-connection tasks, extracts ALPN + fingerprint, calls dispatch + - Iroh: `run_accept_loop` negotiates ALPN, extracts fingerprint, calls dispatch + - TCP+TLS: `run_accept_loop` accepts TCP, performs TLS handshake, extracts ALPN + fingerprint, calls dispatch + - All three feed the same `dispatch_connection` free function + - All three handle shutdown signal via `watch::Receiver` + +7. **What's NOT present (correctly absent)**: + - No `EndpointError` type (removed — `BindFailed` vestigial, `HandlerNotFound` swallowed) + - No `StaticConfig` parameter on `new()` (assembly layer reads it) + - No `TlsSetup`, `build_rustls_server_config`, `build_quinn_server_config_from_rustls` (in `alknet-tls`) + - No `has_iroh_identity` function (transport-building decision moved to assembly layer) + - No `acme_state_handle` field (ACME state lives in `alknet-tls`) + - No dependency on `alknet-tls` + +8. **Dependency hygiene**: + - `alknet-core` is the only alknet dependency + - `quinn` is optional, gated behind `quinn` feature (pulls `alknet-core/quinn`) + - `iroh` is optional, gated behind `iroh` feature (pulls `alknet-core/iroh`) + - `tokio-rustls` is optional, gated behind `tcp` feature + - No unexpected heavy deps + +9. **Test coverage**: + - All 7 registry tests pass + - All 3 `build_auth_context` tests pass + - `dispatch_decision_logic_lookup_and_auth` passes + - 3 `has_iroh_identity` replacement tests pass (builder pattern) + - `endpoint_constructs_with_iroh_raw_key_identity` (adapted) passes + - `iroh_endpoint_runs_accept_loop_and_shutdown` (adapted) passes + - `debug_for_alknet_endpoint_is_implemented_without_panicking` (adapted) passes + - Tests exercise error paths (unknown ALPN, missing fingerprint, etc.) + - Feature-gated tests are correctly annotated + +10. **Cross-cutting checks**: + - `cargo build -p alknet-endpoint` succeeds (all feature combos) + - `cargo test -p alknet-endpoint` succeeds (all feature combos) + - `cargo clippy -p alknet-endpoint --all-targets` succeeds with no warnings + - `cargo fmt --check -p alknet-endpoint` passes + - `cargo build --workspace` still succeeds (old code untouched) + - `cargo test --workspace` still succeeds (old tests untouched) + +## Acceptance Criteria + +- [ ] Crate structure matches spec (7 source files, correct module layout) +- [ ] `AlknetEndpoint` API matches ADR-083 shape (no `StaticConfig`, builder methods, infallible shutdown) +- [ ] `HandlerRegistry` API correct and complete +- [ ] `dispatch` is public, synchronous, transport-agnostic +- [ ] `build_auth_context` resolves identity correctly +- [ ] Quinn accept loop extracts ALPN + fingerprint, calls dispatch +- [ ] Iroh accept loop negotiates ALPN, extracts fingerprint, calls dispatch +- [ ] TCP+TLS accept loop performs TLS handshake, extracts ALPN + fingerprint, calls dispatch +- [ ] No `EndpointError` type present +- [ ] No `StaticConfig` in `new()` signature +- [ ] No `has_iroh_identity` function +- [ ] No dependency on `alknet-tls` +- [ ] All 17 tests pass +- [ ] `cargo build -p alknet-endpoint` succeeds (all feature combos) +- [ ] `cargo test -p alknet-endpoint` succeeds (all feature combos) +- [ ] `cargo clippy -p alknet-endpoint --all-targets` succeeds with no warnings +- [ ] `cargo fmt --check -p alknet-endpoint` passes +- [ ] Workspace still green: `cargo build --workspace` + `cargo test --workspace` pass + +## References + +- docs/research/alknet-crate-extraction/findings.md — Phase 2 +- docs/architecture/crates/endpoint/README.md — full architecture spec +- docs/architecture/decisions/083-endpoint-as-accept-loop-runner.md — ADR-083 +- tasks/endpoint/crate-init.md +- tasks/endpoint/registry.md +- tasks/endpoint/endpoint-core.md +- tasks/endpoint/dispatch.md +- tasks/endpoint/accept-quinn.md +- tasks/endpoint/accept-iroh.md +- tasks/endpoint/accept-tcp-tls.md +- tasks/endpoint/tests.md + +## Notes + +> This review gates Phase 2 completion. The crate must be self-contained and +> spec-conformant before Phase 3 (`alknet-client`) begins, since the assembly layer +> (which consumes both) will wire them together. The old code in core's `endpoint.rs` +> is intentionally still present (duplicated) — the prune happens in Phase 4. +> If deviations are found, document and fix before proceeding to Phase 3. + +## Summary + +> To be filled on completion diff --git a/tasks/endpoint/tests.md b/tasks/endpoint/tests.md new file mode 100644 index 0000000..a6f49f0 --- /dev/null +++ b/tasks/endpoint/tests.md @@ -0,0 +1,156 @@ +--- +id: endpoint/tests +name: Move and adapt endpoint tests from alknet-core/endpoint.rs into alknet-endpoint +status: pending +depends_on: [endpoint/accept-quinn, endpoint/accept-iroh, endpoint/accept-tcp-tls] +scope: moderate +risk: medium +impact: component +level: implementation +--- + +## Description + +Phase 2, Task 8 of the crate extraction. Move the endpoint-related tests from +`crates/alknet-core/src/endpoint.rs` into `crates/alknet-endpoint/`. Adapt them to +test the new ADR-083 API shape (`new(handlers, dynamic, identity_provider, drain_timeout)` ++ `with_quinn`/`with_iroh`/`with_tcp_tls` + infallible `shutdown()`). + +The old tests **stay** in `endpoint.rs` (duplicated) — no breakage. The new crate's +tests are self-contained and pass standalone. + +### Tests to move and adapt + +#### Category A — Registry tests (5 tests, minimal adaptation) + +Move to `crates/alknet-endpoint/src/registry.rs` `#[cfg(test)] mod tests`: + +| Test | Line | Adaptation | +|------|------|------------| +| `handler_registry_new_is_empty` | 967 | Import `HandlerRegistry` from `crate::registry`; no other changes | +| `handler_registry_register_then_get` | 974 | Same | +| `handler_registry_multiple_alpns` | 983 | Same | +| `handler_registry_register_panics_on_duplicate` | 1000 | Same | +| `handler_registry_debug_lists_alpns` | 1007 | Same | +| `handler_registry_default_is_empty` | 1311 | Same | +| `handler_registry_debug_lists_alpns_via_default` | 1319 | Same | + +These tests use `DummyHandler` + `make_handler()` helpers — move those helpers too. + +#### Category B — `build_auth_context` tests (3 tests, minimal adaptation) + +Move to `crates/alknet-endpoint/src/dispatch.rs` `#[cfg(test)] mod tests`: + +| Test | Line | Adaptation | +|------|------|------------| +| `build_auth_context_resolves_identity_from_fingerprint` | 1026 | Import `build_auth_context` from `crate::dispatch`; feature-gate on `quinn` or `iroh` | +| `build_auth_context_no_fingerprint_no_identity` | 1058 | Same | +| `build_auth_context_fingerprint_unknown_identity_none` | 1076 | Same | + +These tests use `IdentityProvider` + `AuthToken` + `Identity` from `alknet_core::auth`. +Feature-gate on `#[cfg(any(feature = "quinn", feature = "iroh"))]` (same as old code). + +#### Category C — `dispatch_decision_logic_lookup_and_auth` (1 test, moderate adaptation) + +Move to `crates/alknet-endpoint/src/dispatch.rs` `#[cfg(test)] mod tests`: + +| Test | Line | Adaptation | +|------|------|------------| +| `dispatch_decision_logic_lookup_and_auth` | 1212 | Tests handler lookup + `build_auth_context` together. Adapt to use `crate::registry::HandlerRegistry` + `crate::dispatch::build_auth_context`. Feature-gate on `#[cfg(any(feature = "quinn", feature = "iroh"))]`. | + +#### Category D — `has_iroh_identity` tests (3 tests, significant adaptation) + +| Test | Line | Adaptation | +|------|------|------------| +| `has_iroh_identity_true_for_raw_key` | 1327 | **`has_iroh_identity` does not exist in the new crate** — the transport-building decision moves to the assembly layer. Replace with a test that verifies `AlknetEndpoint::new(...).with_iroh(endpoint)` sets the iroh field correctly. | +| `has_iroh_identity_false_for_x509` | 1341 | Same — replace with a test that verifies `with_iroh` is not called (iroh field stays `None`). | +| `has_iroh_identity_false_when_no_identity` | 1356 | Same — replace with a test that verifies the endpoint works without iroh. | + +These tests need to be rewritten for the new API shape. The old tests verified a +transport-building decision (`has_iroh_identity`); the new tests verify the builder +pattern (`with_iroh` sets the field, omitting it leaves it `None`). + +#### Category E — Endpoint construction + run + shutdown tests (3 tests, significant adaptation) + +| Test | Line | Adaptation | +|------|------|------------| +| `endpoint_constructs_with_iroh_raw_key_identity` | 1108 | Old: `AlknetEndpoint::new(&static_config, ...)`. New: `AlknetEndpoint::new(registry, dynamic, provider, timeout).with_iroh(endpoint)`. Need to build an iroh endpoint first. Feature-gate on `iroh`. | +| `iroh_endpoint_runs_accept_loop_and_shutdown` | 1139 | Old: uses `StaticConfig` to build iroh internally. New: build iroh endpoint externally, pass via `with_iroh`. The `CountingHandler` pattern stays. Feature-gate on `iroh`. | +| `debug_for_alknet_endpoint_is_implemented_without_panicking` | 1578 | Old: `AlknetEndpoint::new(&static_config, ...)`. New: `AlknetEndpoint::new(registry, dynamic, provider, timeout)`. No transport needed for Debug test. Feature-gate on `quinn` (or no feature gate — Debug is always available). | + +### Test helpers to move + +From `endpoint.rs` lines 944-965: + +```rust +struct DummyHandler { alpn: &'static [u8] } +impl ProtocolHandler for DummyHandler { ... } +fn make_handler(alpn: &'static [u8]) -> Arc { ... } +``` + +Move to a shared `#[cfg(test)]` module or duplicate in each test module that needs them. +The `CountingHandler` (lines 1162-1179) is only used by the iroh accept-loop test — move +it there. + +### Test adaptations summary + +1. **Imports**: Update `crate::config::*` → `alknet_core::config::*`, `crate::auth::*` → + `alknet_core::auth::*`, `crate::types::*` → `alknet_core::types::*`. Use + `crate::registry::HandlerRegistry`, `crate::dispatch::build_auth_context`, + `crate::endpoint::AlknetEndpoint`. + +2. **API shape**: All endpoint construction tests must use the new API: + `AlknetEndpoint::new(registry, dynamic, provider, drain_timeout)` instead of + `AlknetEndpoint::new(&static_config, registry, dynamic, provider)`. + +3. **`shutdown()` is infallible**: Old tests call `.expect("shutdown ok")` on + `shutdown().await`. New tests call `shutdown().await` directly (no `Result`). + +4. **`has_iroh_identity` tests**: Rewritten to test the builder pattern instead. + +5. **Feature gates**: Match the old feature gates where possible. Registry tests need no + feature gates. `build_auth_context` tests need `#[cfg(any(feature = "quinn", feature = "iroh"))]`. + Iroh tests need `#[cfg(feature = "iroh")]`. Debug test needs `#[cfg(feature = "quinn")]` + (or no gate — Debug is always available; the old code gated it on `quinn` because + `AlknetEndpoint` itself was gated). + +### What stays in the original file + +The old tests in `endpoint.rs` are **not deleted** — they stay as duplicates. The prune +happens in Phase 4. This task only adds tests to `alknet-endpoint`. + +## Acceptance Criteria + +- [ ] All 7 registry tests moved to `registry.rs` and pass +- [ ] All 3 `build_auth_context` tests moved to `dispatch.rs` and pass +- [ ] `dispatch_decision_logic_lookup_and_auth` moved to `dispatch.rs` and passes +- [ ] 3 `has_iroh_identity` tests rewritten for builder pattern and pass +- [ ] `endpoint_constructs_with_iroh_raw_key_identity` adapted to new API and passes +- [ ] `iroh_endpoint_runs_accept_loop_and_shutdown` adapted to new API and passes +- [ ] `debug_for_alknet_endpoint_is_implemented_without_panicking` adapted to new API and passes +- [ ] Test helpers (`DummyHandler`, `make_handler`, `CountingHandler`) moved with their tests +- [ ] `shutdown()` calls are infallible (no `.expect()` on shutdown) +- [ ] Feature gates correct on all moved tests +- [ ] `cargo test -p alknet-endpoint` passes (all feature combos) +- [ ] `cargo test -p alknet-core` still passes (old tests untouched) +- [ ] `cargo clippy -p alknet-endpoint --all-targets` succeeds with no warnings + +## References + +- docs/research/alknet-crate-extraction/findings.md — Phase 2, test list +- docs/architecture/crates/endpoint/README.md — ADR-083 API shape +- crates/alknet-core/src/endpoint.rs — lines 935-1606 (source tests) + +## Notes + +> This is the test migration task — 17 tests total (7 registry + 3 auth_context + 1 +> dispatch + 3 has_iroh_identity + 3 endpoint construction). The `has_iroh_identity` +> tests need the most adaptation because `has_iroh_identity` doesn't exist in the new +> crate (the transport-building decision moves to the assembly layer). They're rewritten +> to test the builder pattern instead. The endpoint construction tests need adaptation +> for the new `new()` API (no `StaticConfig`). The old tests stay in `endpoint.rs` — +> the prune happens in Phase 4. Risk is medium because of the test rewrites. + +## Summary + +> To be filled on completion