fix(up-03): PeerCompositeEnv::peer_operations override — list-peers sees peer-announced ops
Surfaced by alkhttp review 006 (UP-03): services/list-peers showed every peer with an empty operations array. PeerCompositeEnv overrode peer_ids only, so peer_operations fell to the trait default (Vec::new()) and the ADR-022 amendment's "announced op is discoverable via services/list-peers" promise never resolved on the wire. ADR-030 prescribed the fix but it had never been ported into alkcall. The existing list-peers unit tests passed because they mock peer_operations with hand-rolled envs. Implements ADR-030 as specified: - OperationEnv gains list_operation_names (default Vec::new(), back-compat for all existing implementors) - OverlayOperationEnv overrides it with its overlay's registered names - PeerCompositeEnv::peer_operations delegates to the peer overlay's list_operation_names; PeerCompositeEnv::list_operation_names aggregates session + connections + base (mirrors its contains()) - LocalOperationEnv enumerates its registry; ChannelsSessionEnv delegates to base Gate: announced_op_is_discoverable_via_services_list_peers in src/registry/op_register.rs — announces an op through op/register, then asserts both the direct peer_operations probe and the services/list-peers wire shape attribute the announced op to the peer, over the exact compose_root_env shape (PeerCompositeEnv + attached connection overlay). Verified load-bearing: reverting the peer_operations override fails the gate. ADR-030 status Proposed -> Accepted with the UP-03 provenance note. Verification: 590 default / 607 all-features tests, clippy (all-targets, all-features, wasm32) clean, fmt clean, doc clean, semver-checks 196 pass against v0.3.0 (defaulted trait method is non-breaking).
This commit is contained in:
1 parent
1e20bb77d0
commit
fd212307e2
5 files changed
+168
-1
No files matched your search
@@ -2,7 +2,15 @@
|
|||||||
|
|
||||||
## Status
|
## Status
|
||||||
|
|
||||||
Proposed
|
Accepted (implemented 2026-09-04 — surfaced as UP-03 in alkhttp's
|
||||||
|
review 006 `docs/…/006-alkcall-0.3.0-consequence-review.md`: the
|
||||||
|
`op/register` amendment's "announced op is discoverable via
|
||||||
|
`services/list-peers`" promise did not resolve on the wire because
|
||||||
|
this override had never been ported into alkcall; the gate is
|
||||||
|
`announced_op_is_discoverable_via_services_list_peers` in
|
||||||
|
`src/registry/op_register.rs`. The `services/list-peers` unit tests
|
||||||
|
did not catch it because they mock `peer_operations` with hand-rolled
|
||||||
|
envs.)
|
||||||
|
|
||||||
## Context
|
## Context
|
||||||
|
|
||||||
|
|||||||
@@ -83,6 +83,10 @@ impl OperationEnv for ChannelsSessionEnv {
|
|||||||
self.base.peer_operations(peer)
|
self.base.peer_operations(peer)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn list_operation_names(&self) -> Vec<String> {
|
||||||
|
self.base.list_operation_names()
|
||||||
|
}
|
||||||
|
|
||||||
async fn invoke_peer(
|
async fn invoke_peer(
|
||||||
&self,
|
&self,
|
||||||
peer: &crate::registry::env::PeerRef,
|
peer: &crate::registry::env::PeerRef,
|
||||||
|
|||||||
@@ -838,6 +838,10 @@ impl OperationEnv for OverlayOperationEnv {
|
|||||||
fn contains(&self, name: &str) -> bool {
|
fn contains(&self, name: &str) -> bool {
|
||||||
self.overlay.read().contains_key(name)
|
self.overlay.read().contains_key(name)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn list_operation_names(&self) -> Vec<String> {
|
||||||
|
self.overlay.read().keys().cloned().collect()
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
pub struct SubscriptionStream {
|
pub struct SubscriptionStream {
|
||||||
|
|||||||
@@ -64,6 +64,16 @@ pub trait OperationEnv: Send + Sync {
|
|||||||
Vec::new()
|
Vec::new()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// The operation names this env layer serves. The default returns
|
||||||
|
/// empty — single-layer envs don't need it. `OverlayOperationEnv`
|
||||||
|
/// overrides it with its overlay's registered names, and
|
||||||
|
/// `PeerCompositeEnv::peer_operations` delegates to it per peer
|
||||||
|
/// (ADR-030 — without the override `services/list-peers` shows
|
||||||
|
/// every peer with an empty operation list).
|
||||||
|
fn list_operation_names(&self) -> Vec<String> {
|
||||||
|
Vec::new()
|
||||||
|
}
|
||||||
|
|
||||||
/// Peer-routing composition (ADR-029 §2). Routes to a specific peer
|
/// Peer-routing composition (ADR-029 §2). Routes to a specific peer
|
||||||
/// (`PeerRef::Specific`) or to the first peer that serves the op
|
/// (`PeerRef::Specific`) or to the first peer that serves the op
|
||||||
/// (`PeerRef::Any`). The default impl ignores the peer selector and
|
/// (`PeerRef::Any`). The default impl ignores the peer selector and
|
||||||
@@ -144,6 +154,14 @@ impl OperationEnv for LocalOperationEnv {
|
|||||||
|
|
||||||
self.registry.invoke(&name, input, context).await
|
self.registry.invoke(&name, input, context).await
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn list_operation_names(&self) -> Vec<String> {
|
||||||
|
self.registry
|
||||||
|
.list_operations()
|
||||||
|
.into_iter()
|
||||||
|
.map(|s| s.name)
|
||||||
|
.collect()
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Per-call composite env (ADR-024 + ADR-029 §1). Built by the `Dispatcher`
|
/// Per-call composite env (ADR-024 + ADR-029 §1). Built by the `Dispatcher`
|
||||||
@@ -298,6 +316,28 @@ impl OperationEnv for PeerCompositeEnv {
|
|||||||
fn peer_ids(&self) -> Vec<PeerId> {
|
fn peer_ids(&self) -> Vec<PeerId> {
|
||||||
self.connection_order.clone()
|
self.connection_order.clone()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn peer_operations(&self, peer: &PeerId) -> Vec<String> {
|
||||||
|
self.connections
|
||||||
|
.get(peer)
|
||||||
|
.map(|overlay| overlay.list_operation_names())
|
||||||
|
.unwrap_or_default()
|
||||||
|
}
|
||||||
|
|
||||||
|
fn list_operation_names(&self) -> Vec<String> {
|
||||||
|
let mut names: Vec<String> = self
|
||||||
|
.session
|
||||||
|
.as_ref()
|
||||||
|
.map(|s| s.list_operation_names())
|
||||||
|
.unwrap_or_default();
|
||||||
|
names.extend(
|
||||||
|
self.connections
|
||||||
|
.values()
|
||||||
|
.flat_map(|c| c.list_operation_names()),
|
||||||
|
);
|
||||||
|
names.extend(self.base.list_operation_names());
|
||||||
|
names
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
|
|||||||
@@ -258,6 +258,7 @@ mod tests {
|
|||||||
use super::*;
|
use super::*;
|
||||||
use crate::protocol::connection::CallConnection;
|
use crate::protocol::connection::CallConnection;
|
||||||
use crate::registry::context::OperationContext;
|
use crate::registry::context::OperationContext;
|
||||||
|
use crate::registry::discovery::install_bootstrap_discovery;
|
||||||
use crate::registry::registration::OperationRegistry;
|
use crate::registry::registration::OperationRegistry;
|
||||||
use crate::registry::spec::Visibility;
|
use crate::registry::spec::Visibility;
|
||||||
use std::collections::HashMap;
|
use std::collections::HashMap;
|
||||||
@@ -617,4 +618,114 @@ mod tests {
|
|||||||
"the serving side's own op resolves through composition, not a peer stub"
|
"the serving side's own op resolves through composition, not a peer stub"
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// UP-03 gate (alkhttp review 006): after a peer announces an op,
|
||||||
|
/// `services/list-peers` over the real `compose_root_env` shape
|
||||||
|
/// (`PeerCompositeEnv` + the connection overlay attached under the
|
||||||
|
/// peer's id) must list the announced op under that peer's
|
||||||
|
/// entry — not an empty operations array. The pre-fix failure:
|
||||||
|
/// `PeerCompositeEnv` overrode `peer_ids` only, so
|
||||||
|
/// `peer_operations` fell to the trait default (`Vec::new()`) and
|
||||||
|
/// every peer listed with `operations: []`. ADR-030's
|
||||||
|
/// `list_operation_names` override is what this exercises.
|
||||||
|
#[tokio::test]
|
||||||
|
async fn announced_op_is_discoverable_via_services_list_peers() {
|
||||||
|
use crate::registry::env::PeerCompositeEnv;
|
||||||
|
use crate::registry::{
|
||||||
|
context::ScopedPeerEnv, discovery::services_list_peers_handler, env::LocalOperationEnv,
|
||||||
|
};
|
||||||
|
|
||||||
|
let serving = Arc::new(OperationRegistry::new());
|
||||||
|
install_bootstrap_discovery(&serving).expect("bootstrap discovery install");
|
||||||
|
|
||||||
|
let peer_identity = crate::core::auth::Identity {
|
||||||
|
id: "consumer-peer".to_string(),
|
||||||
|
scopes: vec![],
|
||||||
|
resources: HashMap::new(),
|
||||||
|
};
|
||||||
|
let conn = Arc::new(CallConnection::new_overlay_only(peer_identity));
|
||||||
|
let handler = op_register_handler(Arc::clone(&conn), Arc::clone(&serving));
|
||||||
|
|
||||||
|
let input = OpRegisterRequest {
|
||||||
|
spec: announced_spec("worker/exec"),
|
||||||
|
replace: false,
|
||||||
|
}
|
||||||
|
.to_json();
|
||||||
|
assert!(
|
||||||
|
handler(input, test_context("req-or-9a"))
|
||||||
|
.await
|
||||||
|
.result
|
||||||
|
.is_ok(),
|
||||||
|
"announce lands in the connection overlay"
|
||||||
|
);
|
||||||
|
assert!(conn.overlay_contains("worker/exec"));
|
||||||
|
|
||||||
|
// The exact env shape `compose_root_env` produces for calls
|
||||||
|
// arriving on this connection: LocalOperationEnv base +
|
||||||
|
// the connection's overlay attached under the peer's id.
|
||||||
|
let base = Arc::new(LocalOperationEnv::new(Arc::clone(&serving)));
|
||||||
|
let mut composite = PeerCompositeEnv::new(base);
|
||||||
|
composite.attach_peer("consumer-peer".to_string(), conn.overlay_env());
|
||||||
|
let env: Arc<dyn crate::registry::env::OperationEnv + Send + Sync> = Arc::new(composite);
|
||||||
|
|
||||||
|
// Direct probe: the composite resolves the announced name from
|
||||||
|
// the attached overlay, and peer_operations lists it.
|
||||||
|
assert!(env.contains("worker/exec"));
|
||||||
|
let ops = env.peer_operations(&"consumer-peer".to_string());
|
||||||
|
assert_eq!(
|
||||||
|
ops,
|
||||||
|
vec!["worker/exec".to_string()],
|
||||||
|
"PeerCompositeEnv::peer_operations must surface the peer overlay's announced ops"
|
||||||
|
);
|
||||||
|
assert!(
|
||||||
|
env.peer_operations(&"unknown-peer".to_string()).is_empty(),
|
||||||
|
"an unattached peer has no operations"
|
||||||
|
);
|
||||||
|
|
||||||
|
// Wire-level probe: services/list-peers over the same env
|
||||||
|
// attributes the announced op to the peer.
|
||||||
|
let peers_registry = Arc::new(OperationRegistry::new());
|
||||||
|
install_bootstrap_discovery(&peers_registry).expect("bootstrap install");
|
||||||
|
let list_handler = services_list_peers_handler(Arc::clone(&peers_registry));
|
||||||
|
|
||||||
|
let mut ctx = test_context("req-or-9b");
|
||||||
|
ctx.env = env;
|
||||||
|
ctx.scoped_env = ScopedPeerEnv::empty();
|
||||||
|
let response = list_handler(json!({}), ctx).await;
|
||||||
|
let out = response.result.expect("list-peers ok");
|
||||||
|
let peers_arr = out
|
||||||
|
.get("peers")
|
||||||
|
.and_then(|v| v.as_array())
|
||||||
|
.expect("peers array");
|
||||||
|
let consumer = peers_arr
|
||||||
|
.iter()
|
||||||
|
.find(|p| p.get("peer_id").and_then(|v| v.as_str()) == Some("consumer-peer"))
|
||||||
|
.expect("consumer-peer present in list-peers output");
|
||||||
|
let names: Vec<&str> = consumer
|
||||||
|
.get("operations")
|
||||||
|
.and_then(|v| v.as_array())
|
||||||
|
.expect("consumer operations array")
|
||||||
|
.iter()
|
||||||
|
.filter_map(|o| o.get("name").and_then(|n| n.as_str()))
|
||||||
|
.collect();
|
||||||
|
assert!(
|
||||||
|
names.contains(&"worker/exec"),
|
||||||
|
"the announced op must be discoverable via services/list-peers (UP-03)"
|
||||||
|
);
|
||||||
|
|
||||||
|
// The local entry still lists the bootstrap ops (the serving
|
||||||
|
// registry's own surface is unaffected).
|
||||||
|
let local = peers_arr
|
||||||
|
.iter()
|
||||||
|
.find(|p| p.get("peer_id").and_then(|v| v.as_str()) == Some("local"))
|
||||||
|
.expect("local peer present");
|
||||||
|
let local_names: Vec<&str> = local
|
||||||
|
.get("operations")
|
||||||
|
.and_then(|v| v.as_array())
|
||||||
|
.expect("local operations array")
|
||||||
|
.iter()
|
||||||
|
.filter_map(|o| o.get("name").and_then(|n| n.as_str()))
|
||||||
|
.collect();
|
||||||
|
assert!(local_names.contains(&"services/list-peers"));
|
||||||
|
}
|
||||||
}
|
}
|
||||||
Reference in new issue
Block a user