ZettaScaleLabs
diff --git a/‎crates/hiroz/src/config.rs‎
Lines changed: 9 additions & 9 deletions b/‎crates/hiroz/src/config.rs‎
Lines changed: 9 additions & 9 deletions
diff --git a/‎crates/hiroz/tests/two_session_default_config.rs‎
Lines changed: 237 additions & 0 deletions b/‎crates/hiroz/tests/two_session_default_config.rs‎
Lines changed: 237 additions & 0 deletions
@@ -6,7 +6,7 @@
 //!
 //! # Architecture
 //! - Common overrides: 10 settings shared between router and session
-//! - Router-specific: 5 settings unique to router mode
+//! - Router-specific: 4 settings unique to router mode
 //! - Session-specific: 6 settings unique to peer mode
 //!
 //! # Example
@@ -195,7 +195,7 @@ fn common_overrides() -> &'static [ConfigOverride] {
     &COMMON
 }
 
-/// Router-specific overrides (5 settings)
+/// Router-specific overrides (4 settings)
 fn router_specific_overrides() -> &'static [ConfigOverride] {
     static ROUTER_SPECIFIC: LazyLock<Vec<ConfigOverride>> = LazyLock::new(|| {
         vec![
@@ -214,11 +214,6 @@ fn router_specific_overrides() -> &'static [ConfigOverride] {
                 value: serde_json::json!([]),
                 reason: "Router does not connect to other endpoints (empty list)",
             },
-            ConfigOverride {
-                key: "routing/router/peers_failover_brokering",
-                value: serde_json::json!(false),
-                reason: "Changed from true to false - unnecessary when peers connect directly, reduces overhead",
-            },
             ConfigOverride {
                 key: "transport/link/tx/queue/congestion_control/block/wait_before_close",
                 value: serde_json::json!(5000000),
@@ -246,8 +241,13 @@ fn session_specific_overrides() -> &'static [ConfigOverride] {
             },
             ConfigOverride {
                 key: "listen/endpoints",
-                value: serde_json::json!(["tcp/localhost:0"]),
-                reason: "Accept connections only from localhost - external traffic routed via router",
+                value: serde_json::json!(["tcp/[::]:0"]),
+                reason: "Listen on every interface, as rmw_zenoh_cpp's session config does. \
+                         This was \"tcp/localhost:0\", which made two ROS nodes on two hosts \
+                         unable to exchange anything: a peer that advertises only a loopback \
+                         locator cannot be dialled from another host, and the router will not \
+                         relay between two peers on their behalf. \
+                         See tests/two_session_default_config.rs",
             },
             ConfigOverride {
                 key: "scouting/gossip/autoconnect_strategy",
 
@@ -0,0 +1,237 @@
+//! Two independent sessions in the **shipped default** configuration.
+//!
+//! # The failure this pins
+//!
+//! Two `hiroz` processes on two hosts, with a router both of them reached,
+//! delivered nothing to each other and reported nothing:
+//!
+//! | subscriber | publisher | published | received |
+//! |---|---|---|---|
+//! | shipped default | shipped default | 20 | **0** |
+//! | `with_mode("client")` | `with_mode("client")` | 20 | 20 |
+//! | `with_mode("client")` | shipped default | 20 | 20 |
+//! | shipped default | `with_mode("client")` | 20 | 20 |
+//!
+//! Both sessions opened, the publisher published all 20, the router stayed up
+//! throughout, and no error surfaced anywhere. Only the configuration every
+//! real user gets — the one with no `with_mode` call — failed.
+//!
+//! The shipped session config asked for three things that cannot hold
+//! together: `mode: "peer"`, `scouting/multicast/enabled: false`, and
+//! `listen/endpoints: ["tcp/localhost:0"]`. A zenoh router does **not** route
+//! between two peers connected to it. The setting that used to make it do so,
+//! `routing.router.peers_failover_brokering`, is deprecated in zenoh 1.9 and
+//! has no effect — the router logs exactly that on every start, and hiroz's
+//! own router config was still setting it. So the two peers had to link
+//! directly to each other; and a peer that advertises only `tcp/localhost:0`
+//! is not dialable from another host, so they never did.
+//!
+//! Changing the listen endpoint to `tcp/[::]:0` — which is what
+//! rmw_zenoh_cpp's own session config leaves it at — makes the same 2×2 read
+//! 20 received in every cell.
+//!
+//! # Why this was never caught
+//!
+//! Every pub/sub test in this crate puts the publisher and the subscriber on
+//! **one** `ZContext`, so the message never crosses a session boundary and the
+//! network configuration is not exercised at all. The integration suite that
+//! does use two endpoints builds its contexts with
+//! `create_hiroz_context_with_endpoint`, which calls `.with_mode("client")` —
+//! it opts out of the shipped default, and client↔anything is the row of the
+//! table that works.
+//!
+//! # What each test here can and cannot prove
+//!
+//! [`two_default_sessions_exchange_a_message`] is the two-session delivery
+//! test whose absence hid this. On a single host it passes both before and
+//! after the fix, because two peers on one machine *can* dial each other's
+//! loopback locator — that is precisely why running the suite on one machine
+//! showed nothing wrong. It is kept as the guard for the general path.
+//!
+//! [`shipped_default_advertises_a_reachable_listen_endpoint`] is the test that
+//! fails at the broken revision. It asserts the property the cross-host
+//! measurement above turned on: a session in the shipped default configuration
+//! must advertise a locator that something other than its own machine can
+//! connect to. Reproducing the delivery failure itself needs two hosts, which
+//! this suite has no way to arrange.
+
+use std::time::Duration;
+
+use hiroz::{
+    Builder, TypeHash,
+    config::{RouterConfigBuilder, session_config},
+    context::{ZContext, ZContextBuilder},
+    ros_msg::MessageTypeInfo,
+};
+use serde::{Deserialize, Serialize};
+use zenoh::Wait;
+
+#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Default)]
+struct Ping {
+    count: u64,
+}
+
+impl MessageTypeInfo for Ping {
+    fn type_name() -> &'static str {
+        "test_msgs::msg::dds_::Ping_"
+    }
+
+    fn type_hash() -> TypeHash {
+        TypeHash::zero()
+    }
+}
+
+impl hiroz::ros_msg::WithTypeInfo for Ping {}
+
+impl hiroz::msg::ZMessage for Ping {
+    type Serdes = hiroz::msg::SerdeCdrSerdes<Ping>;
+}
+
+/// The host part of a zenoh TCP endpoint string, e.g. `tcp/[::]:0` -> `[::]`.
+fn host_of(endpoint: &str) -> &str {
+    let after_proto = endpoint.split_once('/').map_or(endpoint, |(_, rest)| rest);
+    match after_proto.rsplit_once(':') {
+        Some((host, _port)) => host,
+        None => after_proto,
+    }
+}
+
+/// Whether a listen endpoint binds somewhere only this machine can reach.
+///
+/// `localhost` is included deliberately: it is what the broken revision used,
+/// and it resolves to a loopback address on every host this runs on.
+fn is_loopback_only(endpoint: &str) -> bool {
+    let host = host_of(endpoint);
+    host == "localhost" || host == "[::1]" || host == "::1" || host.starts_with("127.")
+}
+
+/// A session built the way a user gets one: `ZContextBuilder::default()`, with
+/// no `with_mode` call, and only the router endpoint repointed at this test's
+/// router instead of the fixed `tcp/localhost:7447`.
+fn shipped_default_context(port: u16) -> ZContext {
+    ZContextBuilder::default()
+        .with_router_endpoint(format!("tcp/127.0.0.1:{port}"))
+        .expect("router endpoint")
+        .build()
+        .expect("a session in the shipped default configuration failed to open")
+}
+
+/// A router on a free port, already accepting connections when this returns.
+fn start_router() -> (zenoh::Session, u16) {
+    let port = {
+        let listener = std::net::TcpListener::bind("127.0.0.1:0").expect("bind port 0");
+        listener.local_addr().expect("local_addr").port()
+    };
+
+    let config = RouterConfigBuilder::new()
+        .with_listen_port(port)
+        .build_config()
+        .expect("router config");
+    let session = zenoh::open(config).wait().expect("router failed to open");
+
+    // Wait for the listener rather than sleeping blind: a session that dials
+    // before the router accepts would report a connect failure as a delivery
+    // failure, which is a different defect wearing this one's clothes.
+    let addr = std::net::SocketAddr::from(([127, 0, 0, 1], port));
+    let mut accepted = false;
+    for _ in 0..40 {
+        if std::net::TcpStream::connect_timeout(&addr, Duration::from_millis(50)).is_ok() {
+            accepted = true;
+            break;
+        }
+        std::thread::sleep(Duration::from_millis(50));
+    }
+    assert!(
+        accepted,
+        "router never accepted a connection on port {port}"
+    );
+
+    (session, port)
+}
+
+/// Two *independent* sessions in the shipped default configuration exchange a
+/// message. Two contexts, not two nodes on one context: a single context would
+/// carry the message inside one zenoh session and prove nothing about the
+/// network configuration.
+#[test]
+fn two_default_sessions_exchange_a_message() {
+    let (_router, port) = start_router();
+
+    let publisher_ctx = shipped_default_context(port);
+    let subscriber_ctx = shipped_default_context(port);
+
+    let sub_node = subscriber_ctx
+        .create_node("two_session_listener")
+        .build()
+        .expect("subscriber node");
+    let subscriber = sub_node
+        .create_sub::<Ping>("/two_session_default")
+        .build()
+        .expect("subscriber");
+
+    let pub_node = publisher_ctx
+        .create_node("two_session_talker")
+        .build()
+        .expect("publisher node");
+    let publisher = pub_node
+        .create_pub::<Ping>("/two_session_default")
+        .build()
+        .expect("publisher");
+
+    // Publish repeatedly rather than once: discovery is asynchronous, so a
+    // single publish may legitimately precede the subscriber being known. The
+    // assertion is on delivery happening within the budget, not on the first
+    // message arriving.
+    let mut received = 0u64;
+    let mut published = 0u64;
+    let deadline = std::time::Instant::now() + Duration::from_secs(20);
+    while std::time::Instant::now() < deadline && received == 0 {
+        publisher
+            .publish(&Ping { count: published })
+            .expect("publish");
+        published += 1;
+        if subscriber.recv_timeout(Duration::from_millis(200)).is_ok() {
+            received += 1;
+        }
+    }
+
+    assert!(
+        received > 0,
+        "two sessions in the shipped default configuration exchanged nothing: \
+         {published} published, 0 received, router up on port {port}"
+    );
+}
+
+/// The shipped default session config must advertise a listen endpoint that
+/// another host can dial.
+///
+/// This is the assertion that fails at the revision this test was written
+/// against, where the value was `["tcp/localhost:0"]`. Its comment there read
+/// "accept connections only from localhost - external traffic routed via
+/// router", which is a policy a **client** can hold. A peer cannot: no router
+/// relays between two peers, so a peer that nothing can dial has no path to
+/// any other peer, and it reports no error while having none.
+#[test]
+fn shipped_default_advertises_a_reachable_listen_endpoint() {
+    let config = session_config().expect("session config");
+    let raw = config
+        .get_json("listen/endpoints")
+        .expect("listen/endpoints must be set in the shipped session config");
+    let endpoints: Vec<String> =
+        serde_json::from_str(&raw).expect("listen/endpoints must be a list of strings");
+
+    assert!(
+        !endpoints.is_empty(),
+        "the shipped session config advertises no listen endpoint at all, so no \
+         other host can reach a hiroz node: {raw}"
+    );
+    assert!(
+        endpoints.iter().any(|e| !is_loopback_only(e)),
+        "every listen endpoint in the shipped session config is loopback-only ({raw}), \
+         so a hiroz node cannot be dialled from another host. Measured with two hosts \
+         and a router both reached: 20 published, 0 received, no error reported. \
+         A zenoh router does not relay between two peers — \
+         routing.router.peers_failover_brokering is deprecated in zenoh 1.9 and has \
+         no effect — so a peer nothing can dial can talk to nobody."
+    );
+}