fix(cli,replay-node,message): keep the debug-only _wire_size stamp out of recordings and replay

[phil-opp]

🤖 Machine-generated PR. This change was found and written by Claude Code during an automated codebase review, and was verified locally as described below. Please review it like any other contribution.

Problem

The daemon stamps _wire_size (dora_message::metadata::WIRE_SIZE) on every dora topic debug frame (binaries/daemon/src/dataflow_lifecycle.rs:488) so that dora topic info can count the real on-wire size. Its docs say it is "Debug/inspection path only — never set on real node→node outputs". strip_internal_parameters does not remove it, and nothing else does either, so it leaks in two places:

1. dora record --proxy → dora replay. run_record_proxy re-serializes each debug frame with its metadata unchanged, so every entry in the .drec carries _wire_size. The replay node then calls node.send_output(output_id, metadata.parameters, array), which puts a stale _wire_size into the input metadata of every downstream node in the replayed dataflow.
2. dora topic echo prints it as if it were the node's own metadata. Every frame shows metadata={"_wire_size":N}, and with --format json the metadata field is never null, even when the output has no metadata.

Fix

WIRE_SIZE becomes one of the internal keys removed by dora_message::metadata::strip_internal_parameters, next to _schema_hash and _framing. The daemon stamps the key after it calls that function on the debug path, so dora topic info still sees it; topic info is the key's only legitimate consumer.

That helper is now called at the three consumers that store, print or re-send debug frames:

• record --proxy: the existing wire→public output-id rewrite moves into a small prepare_proxy_event helper, which now also strips the key. The rewrite logic itself is unchanged.
• replay node: strips before send_output (replay_parameters), so proxy recordings made before this fix also replay clean.
• topic echo: strips before rendering.

The node API already calls strip_internal_parameters on receive, so a node can no longer observe the stamp through any path.

Validation

Class: C (record/replay semantics)

• New tests that pin the stripping at both storage/re-send points (the helpers they exercise are introduced here, so they can't be run against main as-is):
  • dora-cli: command::record::tests::proxy_event_drops_the_debug_wire_size_stamp
  • dora-replay-node: tests::replay_drops_the_debug_wire_size_stamp
• cargo test -p dora-cli --lib record (48 passed), cargo test -p dora-replay-node (13 passed), cargo test -p dora-message --lib metadata (23 passed), cargo test -p dora-node-api --lib (235 passed), cargo test -p dora-daemon --lib debug_topic (8 passed; the daemon stamp-after-strip tests).
• cargo clippy -p dora-message -p dora-cli -p dora-replay-node -p dora-daemon --all-targets -- -D warnings, cargo fmt --check: clean.
• make qa-breaking ARGS="--fast" against v1.1.0: no breaking changes.
• Workspace suite, make qa-fast, the fault-tolerance E2E and the contract tests were run once on the combined diff of this review batch; results are in the summary comment.

🤖 Generated with Claude Code

https://claude.ai/code/session_01C38big8SF3jEa21cdm4q6B

---

Generated by Claude Code

[trunk-io]

😎 Merged successfully - details.

[phil-opp]

Automated review run: summary

🤖 Machine-generated by Claude Code during a scheduled codebase review. Every finding below was checked against origin/main @ 8a8289c and against the open PRs before acting.

PRs opened (3)

Correctness

• fix(cli,replay-node,message): keep the debug-only _wire_size stamp out of recordings and replay #3756: dora record --proxy / replay / topic echo leak the debug-only _wire_size stamp into recordings and into downstream nodes' input metadata.
• fix(daemon): keep a crashed dora/logs subscriber subscribed across its restart #3757: daemon: a dora/logs subscriber that crashes and is restarted silently stops receiving logs, because its subscription entry was pruned along with its dead channel.
• fix(core): make a reused git clone wait for its sibling's clone under dora build --parallel #3758: dora build --parallel: a node reusing a sibling's git clone ran ahead of the clone (missing subdir, or an empty dir that poisons later builds).

Optimization / Documentation / Simplification: no PR this run. See the deferred and rejected items below.

Combined-diff validation (run once on all branches together)

• cargo fmt --check, cargo clippy --all --all-targets -D warnings (Python crates excluded): clean.

• Workspace tests, run in crate groups because of the sandbox's disk limit: all pass except

  • 7 dora-daemon spawn::endpoint_exchange tests and 2 dora-node-api zenoh_schema_cache tests. These need IPv6/zenoh networking that this sandbox lacks, and they fail the same way on main.

• fault-tolerance-e2e: 16/16 passed.

• example-smoke contract_: 5 passed. The 4 that failed are all Python-node examples, and this sandbox has no built dora Python package; CI's contract job builds one.

• make qa-fast: fmt, clippy, unwrap-budget, secret-files, publish-graph, package-includes and ci-reporting pass; typos (installed separately) passes; qa-breaking --fast vs v1.1.0 reports no breaking changes for these three PRs. cargo-audit was not available here; no dependencies change.

• /code-review and /simplify were run on the combined diff. Their findings were applied before pushing:

  • fix(core): make a reused git clone wait for its sibling's clone under dora build --parallel #3758: one per-dir record that waits for every writer, not just the latest.
  • fix(cli,replay-node,message): keep the debug-only _wire_size stamp out of recordings and replay #3756: WIRE_SIZE goes through strip_internal_parameters.
  • fix(daemon): keep a crashed dora/logs subscriber subscribed across its restart #3757: an early return when no log subscriber has a live channel.

  The affected crates were re-tested after those changes.

Deferred (real, but not opened this run)

• perf(message): bulk-encode ExtensionStore.value / ExtensionValue.value. Same wire bytes, about 10x faster decode of a 16 MiB value. However, qa-breaking counts any added serde(with = …) as a wire-format break. This needs either the accepted-findings baseline from ci(qa): let the breaking-change gate accept justified findings per baseline #3751 or a gate rule for bulk_bytes.
• record-node writes a random uuid::new_v4() as the .drec header dataflow_id (binaries/record-node/src/main.rs:342). The real id is available from node.dataflow_id(), and proxy mode already stores it. A meaningful test would need main refactored first.

Rejected findings

• Root-level types/std.yml overriding built-in types: real, but already fixed in open PR fix: six independent correctness fixes (C/C++ APIs, record, ROS2 bridge, tensor-pool, type registry) #3660.

• Operator runtime bails on any Event::Error (binaries/runtime-api/src/lib.rs:432): Event::Error also carries fatal stream errors, so forwarding them needs a recoverable/fatal split first. That is a design call.

• Per-input option typos (queue_szie, input-timeout) silently ignored (InputDef has no deny_unknown_fields): tightening the frozen 1.x YAML schema needs a maintainer decision.

• Outer module params not substituted into a nested module's params:: not a documented feature; it is a feature extension.

• Bare path: command inside a module subdirectory rewritten to modules/<cmd>: the fix would need an expand-time file-existence heuristic. Worth an issue rather than a drive-by fix.

• dora node restart counts against max_restarts / backoff: arguably intended (the crash budget covers all restarts). Behavior change for maintainers to decide.

• Networked dora logs ignores rotated log files: real but medium, and a larger change in the daemon Logs handler. Left for a follow-up.

• Daemon-routed outputs allocate shared memory then copy: hot-path perf, but it touches the send path's allocation policy, so it needs a benchmark-backed PR of its own.

• dora topic info fully decodes every frame to read its type: a CLI-only diagnostic cost.

• RedbStore::delete_dataflow unbounded retain_in scan: low impact (it only runs on dora clean and deletes).

• Smaller items:

  • InputMapping Display round-trip for Logs { None, Some(node) };
  • Parameter::Float(NaN) in JSON recordings;
  • the misplaced node_output_ids doc comment;
  • dead PendingNodes::handle_dataflow_stop;
  • the mavlink2-bridge connect doc;
  • topic commands on a dataflow with no outputs;
  • the ipc_encode schema-once doctest.

  All are low-impact; good candidates for a later docs/cleanup batch.

---

Generated by Claude Code

[phil-opp]

🤖 Automated review (fully automated Claude Code review; no human has checked this, advisory only).

No issues found.

What I checked:

• The leak is real on main. The daemon stamps _wire_size after strip_internal_parameters on the debug path (binaries/daemon/src/dataflow_lifecycle.rs). record --proxy stored the stamp, and replay re-sent it in send_output.
• dora topic info is unaffected. No CLI path strips a frame before debug_frame_wire_size reads it.
• No 1.x break. No signatures, wire format or recording format change. strip_internal_parameters now also removes an internal key, which is a bug fix. Recordings made by older versions are cleaned at replay time.
• Tests. With main's libraries/message/src/metadata.rs, replay_drops_the_debug_wire_size_stamp fails (!replayed.contains_key(WIRE_SIZE)), and it passes with the fix. I did not build the dora-cli record test; its helper goes through the same strip_internal_parameters.
• Overlap (fine as is). With the metadata.rs change, the node API strips _wire_size from every input it delivers, so downstream nodes built with this version are already protected. The replay-node strip additionally covers nodes linked against older node APIs.

---

Generated by Claude Code