From 9895248bbf5254997099ec536e99bb1b442280eb Mon Sep 17 00:00:00 2001 From: Michael Mikovsky <77305074+Astatin3@users.noreply.github.com> Date: Sat, 25 Apr 2026 13:34:18 -0600 Subject: [PATCH] Clarify tree protocol runtime and routing Document the hook lifecycle, ingress rules, and longest-prefix routing behavior so the tree endpoint code is easier to follow. Keep the pass behavior-neutral while tightening local names and comments around non-obvious protocol paths. --- src/protocol/tree/endpoint/builders.rs | 22 +++++++--- src/protocol/tree/endpoint/core.rs | 18 ++++++++ src/protocol/tree/endpoint/hooks.rs | 5 +++ src/protocol/tree/endpoint/introspection.rs | 5 ++- src/protocol/tree/endpoint/mod.rs | 6 ++- src/protocol/tree/endpoint/receive.rs | 34 +++++++++------ src/protocol/tree/hook.rs | 46 +++++++++++++++++++++ src/protocol/tree/mod.rs | 5 +++ src/protocol/tree/routing.rs | 46 ++++++++++++++------- 9 files changed, 152 insertions(+), 35 deletions(-) diff --git a/src/protocol/tree/endpoint/builders.rs b/src/protocol/tree/endpoint/builders.rs index eca1ffc..0201040 100644 --- a/src/protocol/tree/endpoint/builders.rs +++ b/src/protocol/tree/endpoint/builders.rs @@ -79,6 +79,9 @@ impl ProtocolEndpoint { header: &PacketHeader, call: &CallMessage, ) -> Result<(), EndpointError> { + // Outbound calls reserve their response hook before the frame is emitted so + // the endpoint can accept a synchronous local response path as well as a + // remote one. if let Some(hook) = &call.response_hook && self .hooks @@ -99,20 +102,21 @@ impl ProtocolEndpoint { } #[must_use] + /// Creates an endpoint with compiled routing tables for its current topology. pub fn new( path: Vec, parent_path: Option>, children: Vec, leaves: Vec, ) -> Self { - let registered_children = children + let registered_child_paths = children .iter() .filter(|child| child.state == super::core::ConnectionState::Registered) .map(|child| child.path.clone()) .collect::>(); Self { - routing: CompiledRoutes::new(&path, ®istered_children, parent_path.is_some()), + routing: CompiledRoutes::new(&path, ®istered_child_paths, parent_path.is_some()), path, children, leaves: leaves @@ -124,6 +128,7 @@ impl ProtocolEndpoint { } } + /// Registers a procedure that is handled directly by the endpoint. pub fn add_endpoint_procedure( &mut self, procedure_id: impl Into, @@ -135,10 +140,12 @@ impl ProtocolEndpoint { } #[must_use] + /// Allocates a hook id scoped to this endpoint path. pub fn allocate_hook_id(&mut self) -> u64 { self.hooks.allocate_hook_id(&self.path) } + /// Encodes a call frame without routing it through the local endpoint. pub fn make_call( &mut self, dst_path: Vec, @@ -153,6 +160,7 @@ impl ProtocolEndpoint { Ok(encode_packet(&header, &call)?) } + /// Builds and immediately routes a call, producing either a forward or a local event. pub fn send_call( &mut self, dst_path: Vec, @@ -174,6 +182,7 @@ impl ProtocolEndpoint { } } + /// Encodes a data frame without routing it through the local endpoint. pub fn make_data( &self, dst_path: Vec, @@ -187,6 +196,7 @@ impl ProtocolEndpoint { Ok(encode_packet(&header, &message)?) } + /// Builds and immediately routes a data packet, updating local hook state for end-of-stream. pub fn send_data( &mut self, dst_path: Vec, @@ -199,12 +209,14 @@ impl ProtocolEndpoint { self.prepare_data(dst_path, hook_id, procedure_id, data, end_hook)?; if end_hook { - let sender_key = self + // Locally-originated streams may not have been resolved against a peer yet, + // so fall back to the endpoint's own hook key shape when closing them. + let local_hook_key = self .hooks .resolve_active_key(&self.path, hook_id, &self.path) .unwrap_or_else(|| HookKey::new(self.path.clone(), hook_id)); - if self.hooks.mark_local_end(&sender_key) { - self.hooks.remove_active(&sender_key); + if self.hooks.mark_local_end(&local_hook_key) { + self.hooks.remove_active(&local_hook_key); } } diff --git a/src/protocol/tree/endpoint/core.rs b/src/protocol/tree/endpoint/core.rs index 4de7cd3..3622e91 100644 --- a/src/protocol/tree/endpoint/core.rs +++ b/src/protocol/tree/endpoint/core.rs @@ -13,15 +13,19 @@ use crate::protocol::{ use super::super::{CompiledRoutes, HookTable, RouteDecision}; +/// Registration state for a direct child endpoint. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum ConnectionState { Unregistered, Registered, } +/// Routing metadata for one direct child endpoint. #[derive(Debug, Clone, PartialEq, Eq)] pub struct ChildRoute { + /// Absolute path for the child endpoint inside the protocol tree. pub path: Vec, + /// Whether this child currently participates in routing decisions. pub state: ConnectionState, } @@ -35,12 +39,16 @@ impl ChildRoute { } } +/// Procedures exposed by a named leaf attached to this endpoint. #[derive(Debug, Clone, PartialEq, Eq)] pub struct LeafSpec { + /// Leaf identifier used in packet headers. pub name: String, + /// Procedures this leaf accepts. pub procedures: Vec, } +/// Where an inbound frame entered this endpoint. #[derive(Debug, Clone, PartialEq, Eq)] pub enum Ingress { Parent, @@ -48,6 +56,7 @@ pub enum Ingress { Local, } +/// Event produced when the endpoint handles a packet locally. #[derive(Debug, Clone, PartialEq, Eq)] pub enum LocalEvent { Call { @@ -64,10 +73,14 @@ pub enum LocalEvent { }, } +/// Result of processing a frame or building a locally-sent packet. #[derive(Debug, Default)] pub struct EndpointOutcome { + /// Frame to forward, together with the next routing decision. pub forward: Option<(RouteDecision, FrameBytes)>, + /// Locally-delivered protocol event. pub event: Option, + /// Whether the packet was intentionally discarded. pub dropped: bool, } @@ -100,6 +113,7 @@ impl EndpointOutcome { } } +/// Error surfaced while validating or encoding protocol frames. #[derive(Debug)] pub enum EndpointError { Frame(FrameError), @@ -129,9 +143,12 @@ impl From for EndpointError { } } +/// Minimal interface implemented by protocol-tree endpoints. pub trait Endpoint { + /// Returns this endpoint's absolute path. fn path(&self) -> &[String]; + /// Processes one inbound frame from the given ingress. fn receive( &mut self, ingress: &Ingress, @@ -139,6 +156,7 @@ pub trait Endpoint { ) -> Result; } +/// Runtime state for one endpoint in the protocol tree. #[derive(Debug, Default)] pub struct ProtocolEndpoint { pub(crate) path: Vec, diff --git a/src/protocol/tree/endpoint/hooks.rs b/src/protocol/tree/endpoint/hooks.rs index 711a635..b666a32 100644 --- a/src/protocol/tree/endpoint/hooks.rs +++ b/src/protocol/tree/endpoint/hooks.rs @@ -61,6 +61,8 @@ impl ProtocolEndpoint { }; if active.peer_path != header.src_path { + // A reused hook id from the wrong peer is treated as terminal for this hook, + // because the endpoint can no longer trust future traffic on it. self.hooks.remove_active(&key); return Ok(EndpointOutcome::event(LocalEvent::Fault { header: PacketHeader { @@ -77,6 +79,7 @@ impl ProtocolEndpoint { } if active.procedure_id != message.procedure_id { + // Data frames stay bound to the procedure chosen by the original call. return Ok(EndpointOutcome::dropped()); } @@ -127,6 +130,8 @@ impl ProtocolEndpoint { pub(crate) fn valid_source_for_ingress(&self, ingress: &Ingress, src_path: &[String]) -> bool { match ingress { Ingress::Parent => { + // Parent ingress may carry packets from ancestors, siblings, or the endpoint + // itself, but not from descendants pretending to be upstream. if src_path.len() < self.path.len() { return true; } diff --git a/src/protocol/tree/endpoint/introspection.rs b/src/protocol/tree/endpoint/introspection.rs index 496d013..c8db41f 100644 --- a/src/protocol/tree/endpoint/introspection.rs +++ b/src/protocol/tree/endpoint/introspection.rs @@ -21,7 +21,7 @@ impl ProtocolEndpoint { return Ok(EndpointOutcome::dropped()); }; - let payload = if let Some(leaf_name) = &header.dst_leaf { + let response_payload = if let Some(leaf_name) = &header.dst_leaf { let Some(leaf) = self.leaves.get(leaf_name) else { return self.emit_fault_if_possible(Some(key), ProtocolFault::UNKNOWN_LEAF); }; @@ -61,10 +61,11 @@ impl ProtocolEndpoint { }; let response = DataMessage { procedure_id: String::new(), - data: payload, + data: response_payload, end_hook: true, }; + // Introspection always completes in a single response frame. if self.hooks.mark_local_end(&key) { self.hooks.remove_active(&key); } diff --git a/src/protocol/tree/endpoint/mod.rs b/src/protocol/tree/endpoint/mod.rs index 105d489..e75f143 100644 --- a/src/protocol/tree/endpoint/mod.rs +++ b/src/protocol/tree/endpoint/mod.rs @@ -1,4 +1,8 @@ -//! Endpoint runtime and traits. +//! Protocol-tree endpoint runtime. +//! +//! This module holds the state machine that validates ingress, decides whether a +//! packet should be handled locally or forwarded, and manages hook lifetimes for +//! call/data/fault exchanges. mod builders; mod core; diff --git a/src/protocol/tree/endpoint/receive.rs b/src/protocol/tree/endpoint/receive.rs index 3c6c9f8..3f07153 100644 --- a/src/protocol/tree/endpoint/receive.rs +++ b/src/protocol/tree/endpoint/receive.rs @@ -12,6 +12,21 @@ use super::core::{ }; impl ProtocolEndpoint { + fn supports_local_procedure(&self, dst_leaf: Option<&str>, procedure_id: &str) -> bool { + match dst_leaf { + Some(leaf_name) => self + .leaves + .get(leaf_name) + .map(|leaf| { + leaf.procedures + .iter() + .any(|procedure| procedure == procedure_id) + }) + .unwrap_or(false), + None => self.endpoint_procedures.contains(procedure_id), + } + } + pub(crate) fn handle_local_call( &mut self, header: crate::protocol::PacketHeader, @@ -26,20 +41,10 @@ impl ProtocolEndpoint { return self.handle_introspection(&header, key); } - let supported = match &header.dst_leaf { - Some(leaf_name) => self - .leaves - .get(leaf_name) - .map(|leaf| { - leaf.procedures - .iter() - .any(|procedure| procedure == &message.procedure_id) - }) - .unwrap_or(false), - None => self.endpoint_procedures.contains(&message.procedure_id), - }; + let procedure_is_supported = + self.supports_local_procedure(header.dst_leaf.as_deref(), &message.procedure_id); - if !supported { + if !procedure_is_supported { let fault = if header .dst_leaf .as_ref() @@ -94,6 +99,9 @@ impl Endpoint for ProtocolEndpoint { match header.packet_type { PacketType::Call => { + // Calls only enter from the parent side of the tree or from the endpoint + // itself. Children can return data/faults, but they do not initiate new + // calls through this node. if !matches!(ingress, Ingress::Parent | Ingress::Local) { return Ok(EndpointOutcome::dropped()); } diff --git a/src/protocol/tree/hook.rs b/src/protocol/tree/hook.rs index b83a010..e8514d7 100644 --- a/src/protocol/tree/hook.rs +++ b/src/protocol/tree/hook.rs @@ -1,15 +1,25 @@ //! Hook state for pending and active protocol flows. +//! +//! Hooks move through two phases: +//! - `PendingHook` tracks enough context to attribute faults before the callee accepts. +//! - `ActiveHook` tracks the live bidirectional flow after activation. +//! +//! The table indexes active hooks both by their host-side return path and by the remote +//! peer path so routing code can resolve whichever side of the relationship it currently has. use alloc::{collections::BTreeMap, string::String, vec::Vec}; /// Hook table key scoped to the hook host path. #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)] pub struct HookKey { + /// Path of the endpoint hosting the hook state. pub return_path: Vec, + /// Per-host hook identifier. pub hook_id: u64, } impl HookKey { + /// Builds the canonical key for a hook hosted at `return_path`. #[must_use] pub fn new(return_path: Vec, hook_id: u64) -> Self { Self { @@ -22,22 +32,34 @@ impl HookKey { /// Pending hook context used only for fault attribution before activation. #[derive(Debug, Clone, PartialEq, Eq)] pub struct PendingHook { + /// Path of the endpoint hosting the pending hook. pub return_path: Vec, + /// Per-host hook identifier. pub hook_id: u64, + /// Caller path to promote into `peer_path` once the hook becomes active. pub caller_src_path: Vec, + /// Procedure that created the hook. pub procedure_id: String, + /// Optional destination leaf inside the peer endpoint. pub dst_leaf: Option, } /// Active hook context used for ordinary data traffic. #[derive(Debug, Clone, PartialEq, Eq)] pub struct ActiveHook { + /// Path of the endpoint hosting the active hook. pub return_path: Vec, + /// Per-host hook identifier. pub hook_id: u64, + /// Remote endpoint path currently paired with this hook. pub peer_path: Vec, + /// Procedure that owns the hook conversation. pub procedure_id: String, + /// Optional destination leaf inside the peer endpoint. pub dst_leaf: Option, + /// Set once the local side has emitted its terminal message. pub local_ended: bool, + /// Set once the peer side has emitted its terminal message. pub peer_ended: bool, } @@ -55,6 +77,10 @@ pub struct HookTable { } impl HookTable { + /// Allocates a non-zero hook id for a hook hosted at `return_path`. + /// + /// Hook ids are scoped by host path, so this only needs to guarantee uniqueness within the + /// local table. The wrapped increment keeps allocation infallible for long-lived runtimes. #[must_use] pub fn allocate_hook_id(&mut self, _return_path: &[String]) -> u64 { let id = self.next_id.max(1); @@ -62,6 +88,7 @@ impl HookTable { id } + /// Inserts a hook that has been announced but not yet accepted by the callee. pub fn insert_pending(&mut self, pending: PendingHook) -> Result<(), HookConflict> { let key = HookKey::new(pending.return_path.clone(), pending.hook_id); if self.pending.contains_key(&key) || self.active.contains_key(&key) { @@ -71,6 +98,10 @@ impl HookTable { Ok(()) } + /// Promotes a pending hook into the active table. + /// + /// Activation intentionally reuses the original hook id and host path, but swaps the + /// pending caller attribution into the active peer path used for data routing. pub fn activate_pending(&mut self, key: &HookKey) -> Option<()> { let pending = self.pending.remove(key)?; self.insert_active(ActiveHook { @@ -86,6 +117,7 @@ impl HookTable { Some(()) } + /// Inserts a live hook and its peer-path lookup entry. pub fn insert_active(&mut self, active: ActiveHook) -> Result<(), HookConflict> { let key = HookKey::new(active.return_path.clone(), active.hook_id); if self.pending.contains_key(&key) @@ -105,10 +137,12 @@ impl HookTable { Ok(()) } + /// Removes a pending hook without affecting active state. pub fn remove_pending(&mut self, key: &HookKey) -> Option { self.pending.remove(key) } + /// Removes an active hook and its secondary peer-path index entry. pub fn remove_active(&mut self, key: &HookKey) -> Option { let active = self.active.remove(key)?; if let Some(peer_paths) = self.active_by_peer.get_mut(&active.hook_id) { @@ -120,20 +154,28 @@ impl HookTable { Some(active) } + /// Returns the pending hook for `key`, if present. #[must_use] pub fn pending(&self, key: &HookKey) -> Option<&PendingHook> { self.pending.get(key) } + /// Returns the active hook for `key`, if present. #[must_use] pub fn active(&self, key: &HookKey) -> Option<&ActiveHook> { self.active.get(key) } + /// Returns the mutable active hook for `key`, if present. pub fn active_mut(&mut self, key: &HookKey) -> Option<&mut ActiveHook> { self.active.get_mut(key) } + /// Resolves an active hook from either side of the conversation. + /// + /// The host side addresses hooks directly by `(return_path, hook_id)`. Peer-originated + /// traffic only has `(hook_id, peer_path)`, so the secondary index maps that back to the + /// canonical host-scoped key. #[must_use] pub fn resolve_active_key( &self, @@ -148,6 +190,7 @@ impl HookTable { self.active_by_peer.get(&hook_id)?.get(peer_path).cloned() } + /// Marks the local side finished and returns `true` once both sides are finished. pub fn mark_local_end(&mut self, key: &HookKey) -> bool { let Some(active) = self.active_mut(key) else { return false; @@ -156,6 +199,7 @@ impl HookTable { active.peer_ended } + /// Marks the peer side finished and returns `true` once both sides are finished. pub fn mark_peer_end(&mut self, key: &HookKey) -> bool { let Some(active) = self.active_mut(key) else { return false; @@ -164,11 +208,13 @@ impl HookTable { active.local_ended } + /// Returns the number of active hooks. #[must_use] pub fn active_len(&self) -> usize { self.active.len() } + /// Returns the number of pending hooks. #[must_use] pub fn pending_len(&self) -> usize { self.pending.len() diff --git a/src/protocol/tree/mod.rs b/src/protocol/tree/mod.rs index 8cb9984..6208d91 100644 --- a/src/protocol/tree/mod.rs +++ b/src/protocol/tree/mod.rs @@ -1,4 +1,9 @@ //! Explicit tree declaration, routing, and a small endpoint runtime. +//! +//! This module keeps the protocol tree machinery split by concern: +//! - `routing` contains static path declarations and longest-prefix routing helpers. +//! - `hook` contains the pending/active hook lifecycle tables used by endpoint runtime code. +//! - `endpoint` ties those pieces together into the runtime-facing protocol endpoint API. mod endpoint; mod hook; diff --git a/src/protocol/tree/routing.rs b/src/protocol/tree/routing.rs index fc15ee6..53cafc3 100644 --- a/src/protocol/tree/routing.rs +++ b/src/protocol/tree/routing.rs @@ -1,13 +1,16 @@ //! Path routing helpers and explicit enum tree declarations. +//! +//! Routing follows a longest-prefix rule over endpoint paths. Each endpoint boundary can compile +//! its children into a small trie so repeated route decisions do not need to scan every child. use alloc::{collections::BTreeMap, string::String, vec, vec::Vec}; /// Explicit test tree declaration used for configuration. #[derive(Debug, Clone, PartialEq, Eq)] pub enum TreeNode { - Root { - children: Vec, - }, + /// The protocol root. Its path is always empty. + Root { children: Vec }, + /// An addressable endpoint segment in the tree. Endpoint { segment: String, leaves: Vec, @@ -18,23 +21,27 @@ pub enum TreeNode { /// Leaf declaration used inside the explicit tree enum. #[derive(Debug, Clone, PartialEq, Eq)] pub struct LeafNode { + /// Leaf name local to an endpoint path. pub name: String, + /// Procedures served by this leaf. pub procedures: Vec, } impl TreeNode { + /// Flattens the explicit tree into the set of endpoint paths it declares. pub fn paths(&self) -> Vec> { - let mut output = Vec::new(); - self.collect_paths(&[], &mut output); - output + let mut paths = Vec::new(); + self.collect_paths(&[], &mut paths); + paths } - fn collect_paths(&self, prefix: &[String], output: &mut Vec>) { + fn collect_paths(&self, prefix: &[String], paths: &mut Vec>) { match self { Self::Root { children } => { - output.push(Vec::new()); + paths.push(Vec::new()); for child in children { - child.collect_paths(&[], output); + // Root always restarts collection from the empty path. + child.collect_paths(&[], paths); } } Self::Endpoint { @@ -42,9 +49,9 @@ impl TreeNode { } => { let mut next = prefix.to_vec(); next.push(segment.clone()); - output.push(next.clone()); + paths.push(next.clone()); for child in children { - child.collect_paths(&next, output); + child.collect_paths(&next, paths); } } } @@ -54,9 +61,13 @@ impl TreeNode { /// Longest-prefix route decision. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum RouteDecision { + /// Forward to the child at the given local child index. Child(usize), + /// Deliver locally at this endpoint. Local, + /// Forward upward because the destination is outside the local subtree. Parent, + /// Drop because no local, child, or parent route applies. Drop, } @@ -70,24 +81,26 @@ pub struct CompiledRoutes { #[derive(Debug, Clone, Default)] struct RouteTrieNode { + /// Child selected when traversal stops exactly at this trie node. best_child: Option, edges: BTreeMap, } impl CompiledRoutes { + /// Compiles child endpoint paths into a trie rooted at `local_path`. #[must_use] pub fn new(local_path: &[String], child_paths: &[Vec], has_parent: bool) -> Self { - let mut table = Self { + let mut routes = Self { local_path: local_path.to_vec(), has_parent, nodes: vec![RouteTrieNode::default()], }; for (index, child_path) in child_paths.iter().enumerate() { - table.insert_child(index, child_path); + routes.insert_child(index, child_path); } - table + routes } fn insert_child(&mut self, index: usize, child_path: &[String]) { @@ -113,6 +126,7 @@ impl CompiledRoutes { self.nodes[node_index].best_child = Some(index); } + /// Resolves `dst_path` using the compiled longest-prefix trie. #[must_use] pub fn route(&self, dst_path: &[String]) -> RouteDecision { if !is_prefix(&self.local_path, dst_path) { @@ -131,6 +145,8 @@ impl CompiledRoutes { }; node_index = *next_index; if let Some(index) = self.nodes[node_index].best_child { + // Keep the deepest matching child seen so far; if traversal breaks later, the + // protocol still routes to the longest matching descendant boundary. best_child = Some(index); } } @@ -156,6 +172,7 @@ pub fn is_prefix(prefix: &[String], path: &[String]) -> bool { /// Trait for resolving a destination path to a routing decision. pub trait RouteProvider { + /// Returns the route decision for `dst_path` from the perspective of `local_path`. fn route_destination( &self, local_path: &[String], @@ -191,6 +208,7 @@ impl RouteProvider for DefaultRouteProvider { } } +/// Resolves `dst_path` with the default longest-prefix route provider. pub fn route_destination( local_path: &[String], child_paths: I,