refactor(pubsub)!: one publish name, with ownership choosing the route

[YuanYuYuan]

Closes the publish-API design issue. Part of the intra-process meta issue.

Note

Stacked on #341, which is stacked on #340. The base is feat/payload-pool, so the diff here is the publish API alone.

Rebased onto the hardened base. Three things the rebase removed: Publishable for Arc<T> had been reconstructing a Published from a count, takes_the_bus() was a third copy of the routing condition, and #[instrument(name = "publish")] — which records the wire sequence number — had been left on the generic forwarder, where a bus publish would have emitted a span whose sn never advanced. It now sits on publish_ref.

publisher.publish(&msg)?;        // zenoh          -> Published::Wire
publisher.publish(arc)?;         // shared bus     -> Published::Bus(Delivery::Sent(n))
publisher.publish_moved(msg)?;   // gives it away, by name
publisher.publish_ref(&msg)?;    // explicit wire, keeps Result<()>

This is rclcpp's shape — publish(const&) against publish(std::move(msg)) — carried by a trait, since Rust has no overloading. Routing rules are unchanged: a bus route still needs the caller to assert the audience.

Three findings came out of building it, and two of them cost the design something:

finding	outcome
the impls cannot overlap, because the trait carries the message type	confirmed by the compiler — no E0119
a generic parameter does not deref-coerce, so publish(&arc) stops compiling where publish(&self, msg: &T) accepted it	a loud compile error, never a silent reroute; publish_ref is the fix
clippy::needless_borrows_for_generic_args suggested dropping the &, which selected a different impl and moved a message from the wire to the bus	no impl for T. Measured at one call site: with the impl the lint fires twice and clippy fails; without it the call compiles and clippy passes with zero errors

The third is why publish covers two routes rather than three. The moved route keeps its own name, which says what it does better than publish(msg) did.

Published { Wire, Bus(Delivery), BusAndWire(Delivery) } composes rather than widening Delivery: a Wire variant inside Delivery would have put a value the bus never returns into the type whose contract is that NoTaker and DepthExceeded stay distinct.

Note

Published is defined in #340, not here. It moved down once the base needed to report the same three routes; this PR consumes it. The base also resolves the route in one place, so publish and publish_moved cannot disagree about where a message goes.

Breaking changes

change	who is affected	before → after
publish returns Result<Published>, not Result<()>	callers who bind or match the result. publish(..)?; in statement position is unaffected	let () = pub.publish(&m)?; → pub.publish(&m)?;
publish is generic, so its argument no longer deref-coerces	callers passing &Arc<T>, &Box<T> or a newtype reference	publish(&arc) → publish_ref(&arc)
no Publishable impl for T	callers passing an owned value	publish(msg) → publish_moved(msg)
publish_owned renamed	callers of it	publish_owned(msg) → publish_moved(msg)

Each of the first three is a compile error, never a silent behaviour change.

publish_owned named the wrong party: read literally it describes the publisher's relationship to the message, which is equally true of publish_shared, so it distinguished nothing. The set now names the argument's fate — borrowed, shared, moved.

Not affected

	why
hiroz-py	reaches publishing through RawPublisher → publish_serialized; never touches ZPub::publish
rmw-zenoh-rs (the C ABI)	one call site pinned to publish_ref, because it is the tail expression of a Result<()> function. Builds clean — verified by compiling it, in the default devshell that carries AMENT_PREFIX_PATH
routing	unchanged; a bus route still needs the caller to assert the audience
publish(&T) behaviour	unchanged

Three tail-position call sites needed pinning. A grep found one; the compiler found the other two (action::server::publish_feedback, lifecycle::publisher::publish) and a third later. For a signature change the compiler is the detector and a grep is a hypothesis.

Evidence

Full gate at dd3aec9f8, in a devshell with ROS headers:

gate	result
clippy -D warnings	0
cargo fmt --all	0
cargo doc unresolved links	0
payload_pool	7 passed
intra_process	18 passed
payload_pool_compat	5 passed
rmw-zenoh-rs build	0 errors

The clippy result is the one that matters, and it is measured rather than argued: with impl Publishable for T present, the lint fires twice and clippy fails; with it removed, the same call site compiles and clippy passes at zero errors. The positive control is what makes the second half meaningful.

What fails without this

Nothing. No failing baseline: this is ergonomics. The justification is that hiroz's publish surface should read like the ROS API its users come from, and that Rust's type system already carries the distinction three names spelled out.

Evidence

At 32b3934f, on the worker: cargo fmt --all --check clean, clippy -p hiroz --all-targets -- -D warnings at 0 errors, cargo doc with 0 unresolved links, payload_pool 7/7, intra_process 23/23 with the executed-test count printed (ran=23) so an empty binary cannot read as a pass. rmw-zenoh-rs — the C ABI consumer — builds clean; that was measured at 28f343e6, and the only change since is a comment.

[Copilot]

🟡 Changes recommended

The new shared dispatch lacks direct coverage, and several public-facing descriptions contradict its routing behavior.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Unifies publishing under ownership-based routing while retaining explicit wire and moved APIs.

Changes:

• Adds Publishable dispatch for borrowed and shared messages.
• Renames owned publishing to publish_moved.
• Updates internal callers, documentation, exports, and tests.

File summaries

File	Description
crates/hiroz/src/pubsub.rs	Implements ownership-based publishing.
crates/hiroz/src/local_bus.rs	Renames moved-message bus publishing.
crates/hiroz/src/prelude.rs	Re-exports Publishable.
crates/hiroz/src/payload_pool.rs	Updates API references.
crates/hiroz/src/action/server.rs	Pins feedback to wire publishing.
crates/hiroz/src/lifecycle/publisher.rs	Preserves Result<()> publishing.
crates/hiroz/src/lifecycle/node.rs	Uses explicit wire publishing.
crates/rmw-zenoh-rs/src/pubsub.rs	Preserves the C ABI publish contract.
crates/hiroz-tests/tests/intra_process.rs	Updates moved-route tests and callers.

Review details

Suppressed comments (2)

crates/hiroz/src/pubsub.rs:651

• The Arc<T> row says this always takes “the bus,” but publish_shared can return Wire for a plain publisher and BusAndWire for Locality::Remote. Describe this as the configured shared route so the table agrees with the routing rules immediately below.

    /// | `Arc<T>` | the bus, every subscriber sharing one allocation | [`ZPub::publish_shared`] |

crates/hiroz/src/pubsub.rs:666

• Making publish(Arc<T>) an alias for publish_shared also makes the publish_shared section “Ordering against publish” incorrect: it says a message sent with publish always travels the wire and arrives later, although this new form performs the same synchronous bus delivery. Qualify that comparison as borrowed publish(&msg)/publish_ref.

    pub fn publish(&self, msg: impl Publishable<T, S>) -> Result<Published> {
        msg.publish_to(self)

• Files reviewed: 9/9 changed files
• Comments generated: 4
• Review effort level: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.