Seven potential discovery / QoS defects

[0x53A]

From my agent, to the attention of your agent

After the other two issues, I asked for a wider review of similar patterns in Zenoh/hiroz, and got the attached report.

---

All references are against main @ 0787b2a, verified 2026-09-02.

Overview

#	Issue	Impact	Fix effort
1	Raw/FFI subscriber declares no liveliness token	Subscriber invisible to the ROS graph while data flows	trivial (+ struct field)
2a	Node's own liveliness token ignores custom KeyExprFormat	Node invisible under a custom prefix; its endpoints are not	trivial (one line)
2b	Graph's entity→key encoding likewise ignores it	Same, for graph indexing	decision — no encoder exists to thread
3	add_local_entity breaks the cached/parsed invariant	Duplicate appearance events, inflated matched counts	trivial (two lines)
4	deadline / lifespan / liveliness unrepresentable on the wire	Introspection and QoS events misreport peers	structural
5	Enclave is parsed but never encoded	rmw_get_node_names_with_enclaves cannot work	small, but a wire-format change
6	node_exists() returns true forever, disagrees with get_node_names()	Four rmw_* entry points gated on a stale answer	trivial
7	rmw-zenoh-rs emits QoS events its own public API says are impossible	False incompatibility events, and only for same-session peers	decision — amends #293
B1	Graph::new_with_pattern ignores its domain_id	Signature promises isolation it doesn't provide	trivial, once the intent is settled
B2	hiroz-protocol's std re-enables all of zenoh's defaults	Overrides the workspace's transport selection; blocks WASM builds	trivial, with a user-visible consequence

Relation to existing open issues

I checked the 37 open issues before writing this. None of the ten below is a duplicate,
but four are adjacent enough to matter:

• SUBSCRIPTION_INCOMPATIBLE_TYPE and PUBLISHER_INCOMPATIBLE_TYPE are declared supported and never raised #293 already records REQUESTED_QOS_INCOMPATIBLE / OFFERED_QOS_INCOMPATIBLE as
  raised by hiroz and not by rmw_zenoh_cpp, characterised as "hiroz is ahead". Issue 7
  below argues that characterisation is too generous — the events fire for the wrong
  peers and on partly fabricated data. That is a disagreement with a documented
  conclusion, not a new discovery, so it is worth reading as an amendment to SUBSCRIPTION_INCOMPATIBLE_TYPE and PUBLISHER_INCOMPATIBLE_TYPE are declared supported and never raised #293's
  audit table rather than a separate finding.
• FFI module is never compiled, linted or tested; the "Go FFI tests" CI step silently skips #270 and The ffi feature is built by no crate on the PR gate, and its re-entrancy detector was deleted #291 establish that the ffi module is never linted or tested and
  predict that defects will ship through that gap. Issue 1 below is a concrete instance
  — found the same way FFI module is never compiled, linted or tested; the "Go FFI tests" CI step silently skips #270 says the last one was, by manual review.
• test(qos): add QoS conformance matrix + rmw_zenoh_cpp interop tests #184 proposes a QoS conformance matrix over durability × reliability × history.
  Issue 4 below is why that matrix, as currently scoped, cannot catch the problem: those
  three are exactly the policies hiroz can represent on the wire, and the other four are
  structurally absent.
• Webassembly Port for Zenoh and Hiroz #222 (the WASM port) is the context for B2.

No open or closed issue covers issues 1, 2, 3, 5 or 6 — I searched for liveliness token, node_exists, enclave, keyexpr format, raw subscriber and
add_local_entity across both states.

---

1. Raw/FFI subscribers are never announced to the ROS graph — trivial

create_raw_subscriber_with_qos builds the entity, derives the topic key expression,
declares the Zenoh subscriber, and returns without ever declaring a liveliness token
(crates/hiroz/src/node.rs:694).

Every sibling on the raw path does declare one: create_raw_publisher_with_qos
(node.rs:616), create_raw_service_client (node.rs:739), create_raw_service_server
(node.rs:805).

Consequence: data flows fine — the data key expression does not depend on liveliness —
but ros2 topic info --verbose reports no subscriber, rmw_zenoh_cpp publishers never
see a match, and raw action clients silently lose their feedback subscription from the
graph. From an operator's seat this reads as "the subscriber works but intermittently
isn't there", which is an unpleasant thing to debug.

Fix shape. RawSubscriber (crates/hiroz/src/ffi/subscriber.rs:11) has one field
and exactly one construction site (node.rs:694). Add an _lv_token field mirroring
RawPublisher (crates/hiroz/src/ffi/publisher.rs:17) and reuse the publisher's
four-line declare block — keyexpr_format.liveliness_key_expr already derives the
correct MS kind from entity.kind, so nothing new is needed on the encoding side.

Relation to #270 / #291. Those issues argue that the ffi module is compiled but
never linted or tested, and that "building is not testing" has already let a
behavioural defect through — the missing LocalPublishGuard on
RawPublisher::publish_bytes, found by manual review. This is the same class, in the
same directory, found the same way: nothing in the suite fails today if the publisher's
liveliness token is deleted either, because no test asserts that a raw endpoint appears
in the graph. Worth adding to #270's list of "what turning the feature on would have
caught".

Second, unrelated bug in the same function. The raw publisher hardcodes
.congestion_control(CongestionControl::Block) (node.rs:612) regardless of the QoS it
was handed. The typed publisher maps reliability properly — Reliable → Block,
BestEffort → Drop (crates/hiroz/src/pubsub.rs:326-334). A raw publisher advertising
best-effort on the wire therefore still blocks on congestion. The correct match already
exists verbatim at that pubsub location.

2. Node liveliness tokens ignore a custom KeyExprFormat — one trivial half, one that needs a decision

Endpoints and — since #336 — the built-in services all honour the configured format.
Node tokens do not, in two separate places.

2a, the node's own token — trivial. ZNodeBuilder builds it via
crate::entity::node_lv_token_key_expr (node.rs:232), which calls
node_to_liveliness_ke (entity.rs:43,68), which constructs KeyExprFormat::default()
internally.

self.keyexpr_format is already in scope at that call site — it is used two lines later
for the type-description service — so this is a one-line swap to
self.keyexpr_format.node_liveliness_key_expr(&node). node_lv_token_key_expr then has
no internal callers left.

With a custom prefix the current behaviour is that a graph observer sees a node's
endpoints under the custom admin space and the node itself under @ros2_lv/: an
orphaned set of endpoints and an invisible node.

2b, the graph's entity→key encoding — needs a decision, not a patch.
Graph::add_local_entity and remove_local_entity key the graph through
entity_to_liveliness_ke (graph.rs:763,867,881,895,906,916), which has the same
hardcoded default.

This one cannot simply be threaded through, because Graph does not hold a format.
Graph::new (graph.rs:453) consumes its format argument into a liveliness pattern
plus a parser closure and hands both to new_with_pattern (graph.rs:575) — and
new_with_pattern accepts a parser with no corresponding encoder. So making the
graph's encoding format-aware requires either storing a KeyExprFormat on Graph
(impossible for pattern-constructed graphs) or giving new_with_pattern an encoder
parameter. That is an API decision.

Test gap. #336's new built_in_services_inherit_custom_keyexpr_format checks
endpoint indexes; adding a get_node_names() assertion to it would have caught 2a.

3. add_local_entity can violate the cached/parsed disjointness invariant — trivial

GraphData keeps two collections that are meant to be disjoint. remove() says so
explicitly and prints "Warning: LivelinessKE was in both cached and parsed" to stderr
if it ever finds otherwise (crates/hiroz/src/graph.rs:158-166).

The liveliness subscriber's PUT handler respects the invariant: it checks both
parsed and cached before inserting (graph.rs:637-638). add_local_entity checks
only parsed (graph.rs:766).

add_local_entity is reached from rmw-zenoh-rs (node.rs:155,
rmw.rs:350,731,1086,1305), so this is an rmw-layer issue rather than one on the
pure-Rust API. With a history-enabled liveliness subscriber, this ordering is reachable
there for a session's own tokens:

1. the history callback delivers our own token first — not in parsed, not in
   cached → inserted into cached, trigger_graph_change(appeared) fires;
2. add_local_entity then runs — already_exists is false, because the key is in
   cached and not parsed → inserted into parsed,
   trigger_graph_change(appeared) fires again.

Result: the key sits in both collections until the next parse() drains cached, two
appearance events fire for one entity (inflating publication/subscription matched
counts), and a removal landing inside that window trips the stderr warning above.

Fix shape. Two lines at graph.rs:766 — widen the check to
parsed.contains_key(&ke) || cached.contains(&ke), and cached.remove(&ke) as you
insert into parsed.

Related, same callback, slightly larger. The DELETE branch fires
trigger_graph_change(&entity, false, ...) before c_graph_data.lock().remove(&ke)
and without checking that the entity was ever known (graph.rs:674-677). remove()
itself tolerates the unknown case, but the event has already gone out, so a duplicate or
unmatched delete produces a spurious disappearance. Fixing it means having remove()
report whether anything was actually removed — it currently returns () — and gating
the event on that.

4. Three QoS policies are structurally unrepresentable on the wire — structural

hiroz::qos::QosProfile carries seven policies — reliability, durability, history,
deadline, lifespan, liveliness, liveliness lease duration
(crates/hiroz/src/qos.rs:160-168).

hiroz_protocol::qos::QosProfile carries three. The encoder consequently hardcodes the
rest as empty (crates/hiroz-protocol/src/qos.rs:58-60):

// Deadline, lifespan, liveliness - use defaults (empty/infinite)
let deadline = ",";
let lifespan = ",";
let liveliness = ",,";

and the decoder never reads those fields back. So the four dropped policies survive
neither direction: a hiroz node advertising a 100 ms deadline publishes
SYSTEM_DEFAULT, and an rmw_zenoh_cpp peer that advertises one is reported as having
none.

This does not break data flow — rmw_zenoh deliberately does not gate matching on QoS —
but it makes graph introspection and QoS-event reporting wrong, and it is what makes
issue 7's deadline and liveliness comparisons meaningless.

Not trivial: four new fields on the protocol QosProfile, encoder, decoder, and every
conversion in between.

Relation to #184. That issue proposes a conformance matrix over QosDurability ×
QosReliability × QosHistory × join ordering. Those three policies are precisely the
ones hiroz can represent, so the matrix as scoped would pass at full green while
deadline, lifespan, liveliness and lease duration remain unrepresentable. If #184 is
meant to "serve as a spec" — its words — the spec has a hole exactly where this issue
is. Its cross-implementation half would catch it, since an rmw_zenoh_cpp peer
advertising a deadline is observably reported as having none.

5. The enclave is parsed but never encoded — small diff, wire-format change

parse_liveliness reads the enclave segment and populates NodeEntity::enclave
(crates/hiroz-protocol/src/format/rmw_zenoh.rs:210,227). Both encoders throw it away:
the endpoint path destructures enclave: _ (rmw_zenoh.rs:104), the node path likewise
(rmw_zenoh.rs:153), and the node token emits the % placeholder unconditionally
(rmw_zenoh.rs:165).

rmw_get_node_names_with_enclaves therefore cannot report a real enclave for any
hiroz-originated node, and hiroz's tokens differ from rmw_zenoh_cpp's on the wire for
any node in a non-default enclave.

This interacts directly with #336: that PR added enclave to ParameterServiceConfig
and threads the node's real enclave into the built-in services' NodeEntity — and the
encoder then discards it. The comment on the new
built_in_services_inherit_context_domain test already notes the gap; this is the
underlying cause.

The code change is small — mirror the existing namespace handling, placeholder when
empty, mangled otherwise — but it changes what hiroz puts on the wire, so it needs the
mangling to match rmw_zenoh_cpp exactly and will churn the key-expression tests.

6. node_exists() returns true forever, and disagrees with get_node_names() — trivial

node_exists() is data.by_node.contains_key(&node_key) (graph.rs:1183).

remove_local_entity retains-out the dead weak pointers inside the slab but never
removes the now-empty by_node entry (graph.rs:878-885), and remove() deliberately
does not touch the index maps at all (graph.rs:153-157 — lazy cleanup, by design).

Two consequences:

• once a node has existed, node_exists() reports it forever, even after every one of
  its entities is gone;
• by_node also gets an entry for a node key when only an endpoint is indexed under
  it, so node_exists() returns true for a node whose own token was never observed.

get_node_names() gets this right — it walks the slabs and filters on Entity::Node
(graph.rs:1200-1217) — so the two APIs actively disagree. node_exists() gates four
rmw_* entry points (crates/rmw-zenoh-rs/src/rmw.rs:2115,2993,3155,3337) and
hiroz_graph_node_exists in the C FFI (crates/hiroz/src/ffi/graph.rs:302).

Fix shape. Make the predicate match get_node_names: the key exists and its slab
contains at least one live Entity::Node. That fixes both consequences at once and
needs no index-cleanup work.

Separately and not trivially, churning topic or node names accumulates empty
by_node / by_topic / by_service keys that nothing ever reclaims. That is a real
leak, but it is not what breaks node_exists().

7. rmw-zenoh-rs emits QoS-incompatibility events its own public API says are impossible — decision, amends #293

This is a disagreement with #293, not a new finding. That issue's audit table has:

rmw event	declared supported	raised	rmw_zenoh_cpp raises
REQUESTED_QOS_INCOMPATIBLE / OFFERED_QOS_INCOMPATIBLE	✅	✅	❌ — hiroz is ahead

"Ahead" is the right verdict only if the events are raised for the right peers on real
data. Today they are raised for neither. The three paragraphs below are the argument;
if it holds, that row should read something closer to "raised, but inverted and on
partly fabricated input".

rmw_qos_profile_check_compatible documents and implements the lenient rmw_zenoh_cpp
position — "Zenoh handles QoS internally, so we're very lenient"
(crates/rmw-zenoh-rs/src/qos.rs:288).

check_qos_compatibility_with_policy (crates/rmw-zenoh-rs/src/qos.rs:74) implements
full DDS matching rules instead: reliability, durability, deadline and liveliness. It is
called on publisher creation, but only for endpoints that pass
if node.z_id != local_zid { continue; } (crates/rmw-zenoh-rs/src/rmw.rs:284, and
again at :670).

The polarity is therefore backwards: two endpoints in the same Zenoh session can raise
REQUESTED_INCOMPATIBLE_QOS / OFFERED_INCOMPATIBLE_QOS, while genuinely remote peers —
the case DDS-style checking would be for — never raise one. Data flows either way, so
the events are pure noise.

The deadline and liveliness checks additionally cannot be right today: the remote QoS
comes from endpoint.qos, a three-field hiroz_protocol::QosProfile, and
protocol_qos_to_hiroz_qos fills the remaining four policies with
..Default::default() (crates/rmw-zenoh-rs/src/pubsub.rs:29). The comparison is
between a real local deadline and a fabricated remote one — issue 4's consequence.

The diff is small in either direction, but the direction is a product decision: drop the
DDS-style checks to match the documented lenient position, or keep them and apply them
to all peers — which then makes issue 4 a prerequisite, since three of the four rules
compare against fields that do not exist on the wire. Not ours to pick.

Note the sequencing this implies for #293: if you take the second option and keep being
"ahead", issue 4 has to land first, or the deadline and liveliness rules stay
meaningless no matter which peers they run against.

Bonus: two smaller things — one trivial, one trivial with a consequence

B1 — Graph::new_with_pattern ignores its domain_id (graph.rs:577, parameter
named _domain_id). The domain is only ever enforced by whatever the caller baked into
liveliness_pattern, so a deliberately broad custom pattern silently ingests entities
from every domain. The standard exact RMW pattern is unaffected; the signature is just
promising something it doesn't do. Trivial either way — enforce it in the parse path, or
drop the parameter — but which one depends on what it was meant to mean.

B2 — hiroz-protocol's std feature re-enables zenoh's entire default feature set.
crates/hiroz-protocol/Cargo.toml:15 has std = ["zenoh/default"]. Feature unification
means any build enabling hiroz-protocol's defaults — i.e. essentially every build —
switches zenoh's defaults back on, overriding the workspace manifest's deliberate
default-features = false, features = ["transport_tcp", "transport_serial"].

cargo tree -p zenoh -f '{f}' on main today:

auth_pubkey, auth_usrpwd, default, internal, shared-memory, transport_compression,
transport_multilink, transport_quic, transport_quic_datagram, transport_serial,
transport_tcp, transport_tls, transport_udp, transport_unixsock-stream, transport_ws,
unstable, zenoh-shm

and with std = []:

internal, shared-memory, transport_serial, transport_tcp, unstable, zenoh-shm

Nothing in the crate needs a zenoh feature — std only gates
#![cfg_attr(not(feature = "std"), no_std)] and one #[cfg(feature = "std")] impl in
entity.rs — so the link looks like a copy-paste rather than a decision. zenoh-ext
pulls the defaults back in the same way (Cargo.toml:60).

The code change is two lines, but it has a user-visible consequence worth deciding on
deliberately: TLS, QUIC, UDP, WebSocket, unixsock, compression, multilink and both auth
backends all disappear from native builds, and Cargo.lock loses ~890 lines (rustls,
quinn, ring, rsa, x509-parser, the windows-* and security-framework trees, and the
zenoh-link-{tls,quic,udp,ws,unixsock_stream} crates). You may well want to widen the
workspace zenoh feature list explicitly in the same commit rather than accept the
two-transport set the manifest currently claims to want.

Multicast scouting is not affected — net/runtime/orchestrator.rs binds its own
UdpSocket and does not go through transport_udp.

This one matters disproportionately outside native builds: it is what makes hiroz
impossible to compile for wasm32-unknown-unknown without patching the manifest, since
the defaults drag in transports that have no WASM implementation. We carry std = []
plus zenoh-ext = { default-features = false } on a fork branch for exactly that reason
— it is the single smallest change that moves the WASM port of #222 closer to not
needing a manifest patch at all.

---

Provenance

Read against main @ 0787b2a, verified 2026-09-02. Every line reference above was
confirmed against that commit, not inferred. All 37 open issues were read for overlap
before filing, plus keyword searches across open and closed for each finding's central
identifier.

The existing suites run green as-is — cargo test -p hiroz-protocol,
cargo test -p hiroz --test graph --test domain_id — and none of the above is caught by
them.

Context, in case it is useful for prioritising: this came out of getting hiroz running
as a browser-WASM ROS 2 client against an rmw_zenoh_cpp system on a non-default
domain. The two defects you already fixed in #336 and #337 were the ones actually
biting us; #337 in particular, since an rmw_zenoh_cpp endpoint publishing with
SYSTEM_DEFAULT QoS emitted a token the old parser rejected, so the endpoint's traffic
flowed while it never appeared in the graph at all.