fix(record): replay past a crash-truncated trailing record

[phil-opp]

Issue

RecordingReader::next_entry is designed for crash survivability: a torn length prefix is already treated as clean end-of-stream (UnexpectedEof → Ok(None)), which is exactly what the truncated_file_no_footer test exercises. But a torn record body was mapped to a hard error via .wrap_err("truncated record")?.

A crash (SIGKILL / Ctrl-C) can flush a record's 4-byte length prefix but only part of the body that follows, because RecordingWriter buffers through a BufWriter. In that case, replay of an otherwise fully recoverable recording failed outright — discarding every complete record before the torn tail. The boundary was inconsistent:

• 1–3 stray prefix bytes → Ok(None) (graceful)
• complete prefix + partial body → Err (whole replay lost)

Fix

Map UnexpectedEof on the body read to Ok(None) as well, so every fully-written record still replays and iteration stops gracefully at the torn tail — matching the length-prefix handling directly above it.

Corruption within a complete record is unaffected: it is still rejected by the existing bounds checks in read_array / read_slice, covered by corrupt_node_id_len_returns_err_not_panic and corrupt_event_bytes_len_returns_err_not_panic.

Validation

• New regression test truncated_record_body_stops_gracefully (writes a complete record, then a length prefix followed by a partial body; asserts the complete record replays and the torn tail yields Ok(None)).
• cargo test -p dora-recording --lib → 15 passed.
• cargo clippy -p dora-recording --all-targets -- -D warnings clean.

---

⚠️ This PR is machine-generated by Claude (an AI assistant) as part of an automated code-review run, and was verified against current origin/main. Please review carefully before merging.

🤖 Generated with Claude Code

https://claude.ai/code/session_01TMv79fayzGF9mDV7vPMQN7

---

Generated by Claude Code

[trunk-io]

😎 Merged successfully - details.

[phil-opp]

Automated code-review run — summary

This is the umbrella summary for a machine-generated review run (branched off current origin/main @ a166752). Five PRs were opened; findings that didn't clear the bar are listed with reasons.

PRs opened, by category

Correctness

• fix(record): replay past a crash-truncated trailing record #2836 — fix(record): replay past a crash-truncated trailing record (torn record body now stops gracefully like a torn length prefix, instead of failing the whole replay).

Optimization

• perf(daemon): skip the metadata deep-clone for pure-remote outputs #2837 — perf(daemon): skip the Metadata deep-clone for pure-remote outputs (build the Arc lazily; empty local_receivers no longer pays the clone).
• perf(node): avoid per-message DataType clone in first-message type check #2838 — perf(node): avoid the per-message DataType clone in the first-message type check on both receive paths (contains_key-guard first, then take on consume), and unify the two drift-prone copies into one helper.

Documentation

• docs(tracing): correct with_span_capture crate-filter description #2839 — docs(tracing): correct with_span_capture doc — the filter enables only dora_coordinator/dora_core, not all dora_* crates.

Simplification

• refactor(memory-pool): drain the table in cleanup_all instead of cloning keys #2840 — refactor(memory-pool): drain the table with std::mem::take in cleanup_all instead of cloning every key into a throwaway Vec.

Each PR was verified with fmt + clippy -D warnings + tests for its affected crate; /review + /simplify gates were run on the combined diff (the /simplify efficiency pass caught and corrected a hot-path regression in the node change before it shipped).

Findings considered but rejected (one-line reasons)

• message InputMapping::Logs drops node_filter on serialize — the wire grammar dora/logs/{level}/{node} can't represent {level: None, node: Some}; unreachable in-tree and the fix is awkward.
• message Metadata::open_telemetry_context returns owned String — a public-API signature break for a marginal per-call saving; deferred.
• arrow-convert single-element validation duplicated 4× — pure maintainability churn on a cold path; low value.
• core check_module_file nested-module containment stricter than expand_module_node — real inconsistency, but the fix needs threading a project-root concept into a standalone validator; too much regression risk for an automated run.
• core check_module_file doc claims a cycle check it never performs — trivial doc-only fix; skipped to keep the run focused.
• core merge_env Some(empty) asymmetry vs its doc — cosmetic (empty BTreeMap).
• daemon circuit-breaker "recovery" block duplicated across two functions — genuine ~45-line duplication, but extracting it touches fault-tolerance recovery semantics; deferred to avoid regression risk.
• daemon inline headroom-drop vs send_with_timestamp — judgment call; the inline version deliberately avoids building the event before the capacity check.
• node Stream::poll_next bypasses scheduler queue_size/flush/backpressure/drop-counts — a real behavioral gap, but routing the Stream path through the scheduler is a semantics change too large for this run and a doc-only warning is low value; deferred.
• node scheduler.rs::is_correlated re-implements carries_pattern_correlation — pre-existing and outside the reviewed diff; natural follow-up to perf(node): avoid per-message DataType clone in first-message type check #2838.
• recording payload double-allocation — the "read fields directly from the stream" rewrite would regress the record_len framing corruption-guard (fix(hardening): soundness audit report + quick-win fixes #2027); no safe zero-copy without a larger redesign.

---

Generated by Claude Code

[phil-opp]

🤖 Automated review by Claude — fully automated, no human in the loop.

Reviewed the diff in libraries/recording/src/lib.rs. The change is correct and strictly improves crash recovery.

Mapping UnexpectedEof on the record-body read_exact to Ok(None) only triggers when the stream truly ends before record_len bytes are available — i.e. a torn trailing record — making the body case consistent with the already-handled torn length-prefix case. A complete but internally corrupt record is unaffected: its record_len bytes are present, so read_exact succeeds and the existing bounds checks in read_array/read_slice still reject it. Genuine (non-UnexpectedEof) read errors are still propagated with wrap_err("failed to read record").

The regression test truncated_record_body_stops_gracefully is genuine: it writes one complete record, appends a length prefix (64) with only 10 body bytes, then asserts the complete record replays and the torn tail returns None without error. No issues found.

---

Generated by Claude Code