feat(seismograph): introduce runtime recording

[Ralf Biedert (ralfbiedert)]

Summary

• introduce Seismograph as the shared high-performance event recorder, thread-local ring-buffer, snapshot, and extensible source-data system
• integrate allocator, runtime, I/O, synchronization, and event-only Cachet telemetry with independently gated recording policies
• add monitor protocol and CLI controls while preserving legacy fixed-size message compatibility
• consolidate allocation hints and allocator telemetry, add performables instrumentation, and migrate the rebased implementation to the current thread-aware APIs

Validation

• just format
• cargo clippy --workspace --no-deps --all-targets -- -D warnings
• cargo test
• cargo build
• cargo build --release
• just readme
• just spellcheck

[Copilot]

🟡 Changes recommended

There are a couple of concrete correctness/maintainability issues in changed tests (e.g., a Linux HAL test still references a removed stack-capture helper, and a parse-error test now asserts an overly brittle full error string).

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

Pull request overview

This PR introduces Seismograph as a shared runtime telemetry/event recording system (thread-local ring buffers + snapshots + extensible per-source payloads), and migrates allocator/runtime/I/O/synchronization telemetry to plug into that snapshot/monitoring model, including a new unified CLI.

Changes:

• Add new Seismograph crates (core recorder, protocol, runtime + rallocator + I/O sources) and a new seismograph CLI for snapshot HTML + monitoring.
• Migrate rallocator and related tooling from the old rallocator_* telemetry/wire/CLI crates to seismograph_* equivalents and update tests/benches/examples accordingly.
• Introduce Performables telemetry integration (channels/sync/Arc events) and wire Cachet telemetry to Seismograph, plus update formatting/coverage scripts.

File summaries

File	Description
README.md	Updates top-level crate listing to Seismograph family and revised rallocator description.
justfiles/format.just	Avoids cargo fmt --all Windows CLI limit by formatting per-package.
justfiles/coverage.just	Narrows llvm-cov nextest target selection (--lib --bins --tests --examples).
crates/uniflight/README.md	Regenerated README content.
crates/tick/src/fmt/iso_8601.rs	Tightens parse error assertion in tests.
crates/tick/README.md	Regenerated README content.
crates/thread_aware/README.md	Regenerated README content.
crates/thread_aware_core/README.md	Regenerated README content.
crates/testing_aids/src/lib.rs	Switches test watchdog channels to Performables channels.
crates/testing_aids/Cargo.toml	Adds Performables dependency for test utilities.
crates/templated_uri/README.md	Regenerated README content and updated http link version.
crates/seismograph/src/recorder/thread.rs	Adds thread identity + thread log model types for recorder snapshots.
crates/seismograph/src/recorder/runtime.rs	Adds runtime identity types and RuntimeEvent fixed context payload.
crates/seismograph/src/recorder/alloc.rs	Adds allocation event model types and heap classification.
crates/seismograph/README.md	New crate README describing recorder + snapshot + monitor usage.
crates/seismograph/Cargo.toml	New seismograph crate manifest with monitor feature.
crates/seismograph_runtime/src/task.rs	Adds runtime task lifecycle instrumentation (wake/poll timing counters + events).
crates/seismograph_runtime/README.md	New crate README for runtime snapshot source and compatibility contract.
crates/seismograph_runtime/Cargo.toml	New seismograph_runtime crate manifest.
crates/seismograph_rallocator/src/wire/mod.rs	Makes wire primitives private to crate and adds container tests module.
crates/seismograph_rallocator/src/wire/format.rs	Moves/privatizes container framing types; uses snapshot Version.
crates/seismograph_rallocator/src/wire/container_tests.rs	Updates tests to target private wire module types.
crates/seismograph_rallocator/src/topology.rs	Updates module docs to reflect rallocator-specific topology model.
crates/seismograph_rallocator/src/snapshot.rs	Introduces local Version type and adds optional runtime events to snapshot.
crates/seismograph_rallocator/README.md	Renames/reframes crate as Seismograph rallocator source; updates links.
crates/seismograph_rallocator/Cargo.toml	Renames crate/package and switches dependencies to seismograph + backtrace.
crates/seismograph_protocol/src/codec.rs	Adds protocol codec helpers (little-endian primitives, hex, reader).
crates/seismograph_protocol/README.md	New/updated README for seismograph protocol crate.
crates/seismograph_protocol/Cargo.toml	Renames package to seismograph_protocol and adds unix libc dependency.
crates/seismograph_io/README.md	New crate README for low-overhead I/O event instrumentation.
crates/seismograph_io/Cargo.toml	New seismograph_io crate manifest.
crates/seismograph_cli/tests/support/mod.rs	Migrates CLI test support from rallocator telemetry to seismograph rallocator.
crates/seismograph_cli/tests/render_migration.rs	Updates render migration fixtures to new crate names.
crates/seismograph_cli/tests/render_html.rs	Updates HTML rendering tests to new crate names.
crates/seismograph_cli/tests/cli.rs	Renames CLI binary and adds a native seismograph capture read test.
crates/seismograph_cli/src/main.rs	New seismograph CLI entrypoint with monitor + snapshot subcommands.
crates/seismograph_cli/src/commands/snapshot/mod.rs	Adds snapshot command module scaffold.
crates/seismograph_cli/src/commands/snapshot/html.rs	Adds decoding that supports both native seismograph captures and legacy allocator-only snapshots.
crates/seismograph_cli/src/commands/mod.rs	Adds monitor command module and wires snapshot module.
crates/seismograph_cli/README.md	New crate README for seismograph CLI.
crates/seismograph_cli/Cargo.toml	Renames package/bin, adds monitor UI deps and seismograph-family deps.
crates/seatbelt/README.md	Regenerated README content.
crates/seatbelt_http/README.md	Regenerated README content.
crates/routerama/README.md	Regenerated README content.
crates/routerama_build/README.md	Regenerated README content.
crates/recoverable/README.md	Regenerated README content.
crates/rallocator/tests/tls_teardown.rs	Removes explicit allocator initialization call.
crates/rallocator/tests/support/mod.rs	Adds snapshot reader to extract rallocator stats via seismograph snapshot source.
crates/rallocator/tests/seismograph.rs	Adds integration test asserting seismograph snapshot contains rallocator source.
crates/rallocator/tests/runtime_snapshot.rs	Adds regression test for runtime snapshot symbol-cache lifetime behavior.
crates/rallocator/tests/performables_telemetry.rs	Adds test ensuring performables telemetry emits seismograph events.
crates/rallocator/tests/no_global_allocator.rs	Adds test for passive allocation hints without rallocator as global allocator.
crates/rallocator/tests/multithreaded.rs	Refactors to use new stats helper and unified allocator macro; removes explicit initialize.
crates/rallocator/tests/macro_configuration.rs	Adds test for unified macro configuration and tunables.
crates/rallocator/tests/initialization.rs	Updates tests to reflect hints working without explicit initialization.
crates/rallocator/src/tuning_telemetry.rs	Updates import paths and switches to shared test lock.
crates/rallocator/src/telemetry/stats.rs	Removes public re-export module (internalized).
crates/rallocator/src/telemetry/snapshot.rs	Removes public re-export module (internalized).
crates/rallocator/src/telemetry/mod.rs	Reframes telemetry module as internal snapshot-source support.
crates/rallocator/src/heap/general.rs	Adds general-purpose heap options and invariants tests.
crates/rallocator/src/heap/bump/mod.rs	Introduces bump heap API module and re-exports options.
crates/rallocator/src/heap/bump/api.rs	Makes bump heap options internal and adds default/options tests.
crates/rallocator/src/hal/win64.rs	Removes stack capture implementation and Debug API feature usage.
crates/rallocator/src/hal/native.rs	Removes peek_free_requested shim.
crates/rallocator/src/hal/mod.rs	Updates platform exports to remove capture_stack/peek_free_requested for non-miri.
crates/rallocator/src/hal/miri.rs	Removes capture_stack and peek_free_requested for miri.
crates/rallocator/src/hal/linux.rs	Removes stack capture implementation but retains a test referencing it (see comments).
crates/rallocator/src/domain/mod.rs	Refactors domain handling; introduces test-only Domain wrapper and retains default domain state.
crates/rallocator/examples/scoped_bump_heap.rs	Updates example to passive allocation_hints heap APIs and removes explicit initialize/usage print.
crates/rallocator/examples/allocation_tracking.rs	Switches tracking from old telemetry to Seismograph recording + new snapshot filenames.
crates/rallocator/Cargo.toml	Updates deps to seismograph family, adds dev deps, and renames benches.
crates/rallocator/benches/tracking_none.rs	Removes old bench.
crates/rallocator/benches/tracking_callers_capable.rs	Removes old bench.
crates/rallocator/benches/tracking_all.rs	Removes old bench.
crates/rallocator/benches/tracking_all_callers.rs	Updates bench to use seismograph recording for allocation backtraces.
crates/rallocator/benches/tracking_aggregates.rs	Updates bench to new macro config + revised description.
crates/rallocator/benches/rallocator.rs	Removes explicit initialize call.
crates/rallocator/benches/rallocator_tracking_events.rs	Adds bench for event recording without backtraces.
crates/rallocator/benches/rallocator_tracking_events_1_in_20.rs	Adds sampled event bench (1/20).
crates/rallocator/benches/rallocator_tracking_events_1_in_100.rs	Adds sampled event bench (1/100).
crates/rallocator/benches/rallocator_threaded.rs	Removes explicit initialize call.
crates/rallocator/benches/bump_allocation_hints.rs	Updates bump workloads bench to passive hint heaps + renamed identifier.
crates/rallocator_wire/README.md	Removes README for old wire crate (superseded by seismograph_protocol/private wire).
crates/rallocator_cli/src/main.rs	Removes old CLI entrypoint (superseded by seismograph CLI).
crates/plurality/README.md	Regenerated README content.
crates/performables/tests/support/waker.rs	Adds RawWaker-based helper for tests.
crates/performables/tests/support/serializer.rs	Adds minimal serializer for tests.
crates/performables/src/telemetry.rs	Adds seismograph-backed telemetry recording mapping for sync/Arc events.
crates/performables/src/sync/mod.rs	Adds sync module surface and poison error type.
crates/performables/src/lib.rs	New crate root docs and module exports.
crates/performables/README.md	New crate README.
crates/performables/Cargo.toml	New performables crate manifest and benches.
crates/performables/benches/performables_uncontended.rs	Adds uncontended ownership/lock benchmarks vs std.
crates/performables/benches/performables_telemetry_sampling.rs	Adds telemetry sampling overhead benchmark for Arc deref.
crates/ohno/README.md	Regenerated README content and updated macro link versions.
crates/observed/README.md	Regenerated README content.
crates/observed_utils/README.md	Regenerated README content.
crates/multitude/src/arena/mod.rs	Adds let _ = self; to avoid unused self in cfg-gated stats block.
crates/msvc_spectre_libs_build/README.md	Regenerated README content.
crates/layered/README.md	Regenerated README content.
crates/internity/README.md	Regenerated README content and updated serde link.
crates/http_path_template/README.md	Regenerated README content.
crates/http_extensions/README.md	Regenerated README content and updated http/bytes links.
crates/fetch/README.md	Regenerated README content and updated bytes links.
crates/fetch_tls/README.md	Regenerated README content.
crates/fetch_hyper/README.md	Regenerated README content.
crates/fetch_azure/README.md	Regenerated README content and updated azure_core links.
crates/data_privacy/README.md	Regenerated README content.
crates/data_privacy_core/README.md	Regenerated README content.
crates/cachet/src/telemetry/seismograph.rs	Adds Seismograph-backed Cachet telemetry events + gating test.
crates/cachet/src/telemetry/mod.rs	Wires seismograph telemetry module behind feature/test cfg.
crates/cachet/src/refresh.rs	Records suppressed refresh events and distinguishes fallback errors.
crates/cachet/src/fallback.rs	Adds promotion outcome telemetry on fallback-to-primary insert.
crates/cachet/src/cache.rs	Adds compute success/failure/none telemetry in cache miss paths.
crates/cachet/README.md	Regenerated README content.
crates/cachet/Cargo.toml	Adds optional seismograph feature + dependency and test dep path.
crates/cachet_tier/README.md	Regenerated README content.
crates/cachet_service/README.md	Regenerated README content.
crates/cachet_memory/README.md	Regenerated README content.
crates/bytesbuf/src/view.rs	Adds lazy buffer identity tracking + seismograph I/O buffer trait implementation.
crates/bytesbuf/src/telemetry.rs	Adds buffer identity implementation with seismograph/no-seismograph variants.
crates/bytesbuf/src/lib.rs	Documents new seismograph feature and adds telemetry module.
crates/bytesbuf/src/buf.rs	Adds lazy buffer identity tracking + seismograph I/O buffer trait implementation.
crates/bytesbuf/README.md	Documents new seismograph feature.
crates/bytesbuf/Cargo.toml	Adds optional seismograph_io dependency and seismograph feature.
crates/bytesbuf_io/README.md	Regenerated README content.
crates/automation/src/process.rs	Switches timeout wait channel from std mpsc to Performables channel with richer error checks.
crates/automation/Cargo.toml	Adds Performables dependency.
crates/arty_executor/README.md	Regenerated README content.
crates/anyspawn/src/handle.rs	Switches join handle oneshot receiver to Performables oneshot receiver.
crates/anyspawn/src/custom.rs	Switches custom spawner oneshot channel to Performables channel.
crates/anyspawn/README.md	Regenerated README content.
crates/anyspawn/Cargo.toml	Replaces futures-channel dependency with Performables.
crates/anyspawn_azure/README.md	Regenerated README content and updated azure_core links.
crates/allocation_hints/src/domain.rs	Removes domain backend API (passive hinting model no longer owns a backend).
crates/allocation_hints/README.md	Updates docs to passive hint model and new heaps APIs/examples.
crates/allocation_hints/Cargo.toml	Updates description/links and adds dependencies for passive TLS model.
crates/allocation_hints/build.rs	Updates build script docs to passive hint TLS resolution.
Cargo.toml	Adds workspace deps for new seismograph/performables crates and TUI/monitor deps; removes old rallocator_* crates.
.spelling	Adds new words for spellcheck allowlist.

Review details

• Files reviewed: 134/198 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Martin Taillefer (geeknoid)]

Static review of the seismograph recording change

Five passes — security, correctness, testing, performance, conformance — over the 198-file diff, scoped by subsystem. Nothing was executed: no build, tests, clippy, Miri, loom, coverage or benchmarks, so every claim below is from reading the code, and no number is cited that is not already in the tree.

Headline. Two findings stand out. capture_stack() on the allocation record path (recorder.rs:495) runs without a SuppressionGuard, while the sibling capture_backtrace() deliberately takes one — and nothing else on the alloc -> begin_allocation -> commit -> record chain establishes suppression, so glibc's allocating backtrace() can re-enter the recording allocator. Separately, unsafe impl<T: Sync, F: Send> Sync for LazyLock (once.rs:157) drops the T: Send that its OnceLock<T> field requires, which safe external code can reach.

On the description's claims. "Preserving legacy fixed-size message compatibility" holds at the size level — the 34/50/82-byte blocks are stable and asserted — but it is preserved by not encoding RecordingConfiguration::cache, which decode_recording then silently reconstructs as the default. So the size is compatible and the data is lost. The validation list also reports cargo test and cargo build passing, which cannot hold on Linux: hal/linux.rs:127 calls capture_stack, which linux.rs does not define and hal/mod.rs:20 re-exports only under cfg(miri). That is the point Copilot already raised on that line, and it appears to be a genuine build break rather than a stale comment.

What I attacked and could not break. The custom Arc does not reimplement reference counting — it delegates to std::sync::Arc (from_raw/increment_strong_count/decrement_strong_count), so those orderings are std's; there are no manual Send/Sync impls in arc/. The recorder's retain-snapshot path is mutually excluded by the per-slot spinlock, and the SeqCst writer_active/ACTIVE_SESSION handshake correctly prevents a destructive snapshot freeing a ring under a live writer. Mutex/RwLock acquire/release orderings are correct and their unsafe impl bounds match std. Seismograph's own recording path genuinely avoids the global allocator (System.alloc, hal::map, fixed stack arrays) — the reentrancy above comes from libc, not from this code. The report.rs move out of rallocator_cli changed no reporting behavior: the numeric machinery is byte-identical and only import paths and labels differ. The monitor's token comparison is constant-time, and I/O recording captures buffer metadata only, never contents.

Below the budget, not raised inline. The per-runtime Mutex<Vec<Arc<TaskControl>>> is taken on every task spawn and retire and is also held across snapshot cloning (seismograph_runtime/src/lib.rs:274, :631, :561) — worth a look, but I could not bound the hold time statically. crates/rallocator/tests/seismograph.rs:22 asserts only allocations != 0, which a badly miscounting allocator would survive. crates/seismograph/Cargo.toml declares getrandom and seismograph_protocol optional with no [dev-dependencies], against the rule in AGENTS.md; it is currently latent because the consumers sit behind cfg(feature = "monitor") alone, but recorder.rs already uses cfg(any(test, feature = "monitor")) nearby.

Open question. record_allocation and record_deallocation_stats both early-return under telemetry_suppressed() (telemetry/core.rs:1077, :1084). If a non-arena allocation made before a capture is freed by the capturing thread inside the suppressed region, live_bytes drifts upward permanently. I could not construct a trigger — every free I traced inside with_telemetry_suppressed is an arena allocation short-circuited earlier in dealloc. Auditing the Drops reachable from try_snapshot_with_runtime_events would settle it.

[Martin Taillefer (geeknoid)]

Backtrace capture on the allocation record path runs without suppression, so the recording allocator can re-enter itself — Correctness · High · Confidence Medium

record_enabled_with_recorder calls capture_stack() here with no suppression established. Its sibling capture_backtrace() wraps the identical call in let _suppression = SuppressionGuard::enter(); (recorder.rs:577-578), which is what makes the asymmetry look unintended rather than deliberate.

Nothing on the allocation path establishes suppression either. tracking::begin_allocation only reads the flag (rallocator/src/telemetry/core.rs:1384), and PendingTracking::commit calls seismograph::record_in_session_classified without entering a guard (core.rs:1399). So with allocation recording and capture_backtraces enabled under the rallocator:

1. alloc -> tracking.commit(..) -> record_in_session_classified -> record_enabled_with_recorder -> capture_stack().
2. On Linux capture_stack bottoms out in libc::backtrace (recorder.rs:1148). glibc's backtrace resolves the unwinder on first use via dlopen, which allocates through the global allocator.
3. That allocation re-enters alloc. telemetry_suppressed() is still false and recording is still enabled, so begin_allocation returns Some again and another backtrace is captured.

Note this is specifically about glibc's internals, not about seismograph's own allocations — those correctly use System.alloc/hal::map and I verified they do not re-enter. Confidence is Medium rather than High because whether this manifests as bounded first-call recursion or a hard deadlock depends on glibc's once-init locking, which I did not execute.

Direction: Wrap the capture_stack() call here in a SuppressionGuard::enter() scope, matching capture_backtrace(). The ring write itself does not allocate, so only the backtrace step needs guarding.

Done when: recording an allocation-class event with capture_backtraces = true under the rallocator cannot re-enter begin_allocation on the same thread, or the author shows this path is unreachable from an allocating context and documents why the sibling still suppresses.

[Martin Taillefer (geeknoid)]

unsafe impl Sync for LazyLock omits the T: Send bound its own field requires — Correctness · High · Confidence High

This asserts LazyLock<T, F>: Sync with only T: Sync, but the struct's field is value: OnceLock<T> (once.rs:150), wrapping std::sync::OnceLock<T>. StdOnceLock<T>: Sync holds only when T: Send + Sync — std requires Send because a value can be produced by one thread's get_or_init and later dropped by whichever thread owns the cell.

LazyLock::new is a pub const fn with no T: Send bound, so code outside this crate can instantiate LazyLock<G, F> with a G: Sync + !Send and share it across threads. The initializer then runs on one thread while the value is dropped on another — exactly what !Send exists to forbid. Safe external code is sufficient to reach it, so this is a soundness hole rather than a latent style issue.

The neighbouring Send impl on line 155 is correctly bounded T: Send, which is what makes this look like an omission. std's corresponding bound is unsafe impl<T: Sync + Send, F: Send> Sync for LazyLock<T, F>.

Direction: Add T: Send to the Sync impl so it matches both the OnceLock<T> field's own requirement and std.

Done when: LazyLock<T, F>: Sync requires T: Send + Sync, or the author demonstrates why a T: Sync + !Send initialized on one thread and dropped on another is sound here.

[Martin Taillefer (geeknoid)]

RecordingConfiguration::cache is never encoded and is hard-coded back to the default on decode, so it is silently lost in transit — Correctness · Medium · Confidence High

RecordingConfiguration carries a cache: RecordingPolicy field (message.rs:28-29), but encode_recording writes only five policies — allocations, general_events, arc_dereferences, runtime_tasks, io (message.rs:259-267) — and decode_recording reconstructs cache as RecordingPolicy::default() on this line rather than reading it.

Every RecordingConfiguration that crosses the wire is affected: Request::SetRecording, Response::Hello.recording, and Response::RecorderStatistics.recording. A monitor reading RecorderStatistics therefore always reports cache recording as disabled even while it is active, and a SetRecording carrying a cache policy is silently a no-op for that field.

The cache policy does have a working transport in the separate SetCacheRecording/ReadCacheRecording messages, which is presumably how this went unnoticed — but that makes cache's presence on the shared struct a field that cannot be trusted by any reader. This is also the mechanism by which the fixed 34-byte block size was preserved, so it is coupled to the compatibility claim in the description.

Direction: Either remove cache from RecordingConfiguration so it cannot flow through SetRecording/Hello/statistics, or encode it and grow the block deliberately with a version bump. Pick which of the two cache transport paths is authoritative and make the type reflect it.

Done when: cache cannot be set on a value that encode_recording then discards, or the field is documented and named such that its non-transmission is impossible to misuse.

[Martin Taillefer (geeknoid)]

CondvarWait::drop consumes a notify_one without forwarding it, unlike every sibling primitive — Correctness · Medium · Confidence High

• Op A — Condvar::notify_one (condition.rs:90-92): generation.fetch_add(1, Release), then waiters.wake_one(), which pops the front active waiter W from the queue (wait_queue.rs:128) and calls its waker.
• Op B — W's wait is cancelled after being woken: a wait_timeout_sync deadline (condition.rs:82) or a dropped select! branch runs this Drop, which calls only cancel(waiter).
• Shared state: the WaitQueue deque and the single logical notification; a second waiter W2 remains parked.
• Interleaving: wake_one pops W and unparks it; concurrently W's timeout expires, so W is dropped. cancel(W) finds W already removed, returns false, and does nothing. W2 was never popped, so its waker is never called.
• Consequence: the notification is lost and W2 stays parked although the predicate is satisfied — a hang until some later notification arrives.

The generation counter does not prevent this: it only helps a waiter that is actually polled, and a parked W2 is not re-polled without an explicit wake. The sibling primitives handle exactly this case — MutexLockResult::drop re-issues the wake when cancel reports the waiter was already removed (mutex.rs:313-315), and QueueWait::drop does the same (channel.rs:585-587). Only this Drop omits the forward.

Direction: Mirror the sibling pattern — when cancel reports the waiter was already removed, re-issue wake_one on the condvar's queue.

Done when: a cancelled or timed-out condvar waiter that was the target of a notify_one re-delivers that wakeup to another waiter, or the author shows the generation scheme already unparks W2 without an explicit wake.

[Martin Taillefer (geeknoid)]

The new hand-written synchronization primitives and the recorder's cross-thread handshake ship with no loom or Miri race model — Testing · High · Confidence High

This PR introduces a Mutex, RwLock, Condvar, WaitQueue, Once/LazyLock and channels in performables, plus a per-slot spinlock and a SeqCst Dekker-style writer_active/ACTIVE_SESSION handshake in seismograph/src/recorder.rs:481 that a destructive snapshot relies on to avoid freeing a ring under a live writer. That is the most ordering-sensitive code in the change.

The workspace already has this tooling: cfg(loom) and cfg(miri_race_coverage) are declared in the root Cargo.toml check-cfg, and peer crates use it (crates/plurality/tests/loom_pool.rs, crates/multitude/tests/loom_arena.rs, crates/internity/tests/loom_algorithm_sketches.rs). A grep across all of crates/performables/ and crates/seismograph/ returns zero occurrences of either cfg.

What ships instead are ordinary thread::spawn races such as this test and rw_lock_coordinates_... (sync.rs:530), which assert only that operations eventually complete. Downgrading unlock from Release to Relaxed would still pass these on x86, whose TSO hides precisely the acquire/release defects loom exists to find. The cancel-vs-notify interleaving I raised on condition.rs:172 is a concrete example of a bug a loom model would surface deterministically and these tests cannot.

Direction: Add a cfg(loom) leg modelling the enqueue-vs-wake and cancel-vs-notify races for WaitQueue/Mutex/RwLock/Condvar, and one modelling a writer against a destructive snapshot for the recorder ring, following the peer crates' structure.

Done when: each new primitive has at least one loom model that fails if its orderings are downgraded, or the author records why loom coverage is deferred and tracks it.

[Martin Taillefer (geeknoid)]

Decoders size allocations directly from untrusted 32-bit counts, so a few dozen bytes can force a multi-gigabyte allocation — Security (DoS) · Medium · Confidence High

decode() reads event_count from a u32 header field (snapshot.rs:293) and reaches this Vec::with_capacity(event_count) before consuming a single event byte. Nothing checks the declared count against the bytes actually remaining in the reader, so a ~30-byte snapshot declaring event_count = u32::MAX requests roughly 4.29e9 * size_of::<Event>(); the allocation fails and handle_alloc_error aborts the process. source_count (snapshot.rs:323) and thread_count (snapshot.rs:590) follow the same pattern.

The same root cause appears in the frame reader: read_frame validates the length only against maximum and then does let mut payload = vec![0; len]; before reading the payload (seismograph_protocol/src/lib.rs:141). On the response path maximum is MAX_SNAPSHOT_BYTES = u32::MAX, so a declared length of 0xFFFFFFFF forces an immediate ~4 GiB zeroing allocation that read_exact then fails.

Actor and impact: decode() is public and documented to accept external and older-version snapshots, so the source is any snapshot file or peer output a decoder is pointed at. For the frame reader the peer is a same-user local process, which bounds the severity — that half is hardening rather than a privilege boundary crossing.

Direction: Bound each with_capacity by the reader's remaining length divided by that record's minimum on-wire size, or grow the vectors incrementally instead of preallocating from the header value.

Done when: a short input advertising a huge event/thread/source count or frame length returns Error::invalid_format() instead of attempting an allocation larger than the bytes actually available.

[Martin Taillefer (geeknoid)]

The wire-format tests encode and decode with the same code, so nothing actually pins the layout the PR promises to preserve — Testing · Medium · Confidence High

Every round-trip test here feeds encode_* straight into decode_*. Such a test passes no matter what the layout is, because both sides move together: swapping the enabled and capture_backtraces bytes, renumbering request kinds 1-6 or response kinds 101-105/255, changing WorkerRole::wire_value, or flipping push_u32 from little- to big-endian (codec.rs:10) would all still round-trip cleanly.

legacy_recording_message_sizes_remain_stable (message.rs:432) pins only the three block lengths 34/50/82. That catches a size change but no reordering or endianness change within a block.

The description's claim is "preserving legacy fixed-size message compatibility", and the sizes genuinely do hold — but a round-trip suite is structurally incapable of detecting the regressions that claim is about, so the guarantee is currently untested.

Direction: Add a fixture test asserting encode_* output equals a hand-written &[u8] literal — and decodes back — for at least one request, one response, encode_recording, and a descriptor, pinning magic, version, discriminants, field order and endianness.

Done when: a byte-level fixture exists whose expected bytes were written by hand rather than produced by the encoder under test, so reordering a field or flipping endianness fails a test.

[Martin Taillefer (geeknoid)]

The public rallocator::telemetry API is removed rather than deprecated, as a side effect of the consolidation — Conformance · Medium · Confidence High

At the merge base, crates/rallocator/src/telemetry/mod.rs re-exported pub use core::{SnapshotError, SnapshotErrorKind, snapshot, stats, track_callers, try_snapshot} along with pub mod snapshot; and pub mod stats;. After this PR mod.rs is just mod core; pub(crate) use core::*;, so the entire surface is crate-private, and stats() on this line is additionally gated #[cfg(test)] — meaning there is no live in-process statistics accessor outside test builds at all.

Any downstream user of rallocator::telemetry::stats/snapshot/try_snapshot/track_callers now fails to compile, and the equivalent data is reachable only by driving the seismograph snapshot pipeline and decoding through seismograph_rallocator::decode. record_small_allocation/record_small_deallocation are likewise reduced to #[cfg(test)] stubs that ignore class_index/block_bytes (core.rs:1091-1105).

That may well be intended, but the description presents this as consolidating telemetry rather than removing a published API, and no migration path is stated. Flagging it so the break is a decision rather than a side effect.

Direction: Either keep thin pub shims forwarding to the seismograph path, or make the removal explicit — a documented breaking change naming the replacement for each removed entry point.

Done when: the removed public items are restored as forwarding shims, or the break is confirmed as intended and the migration from telemetry::stats()/track_callers() to the seismograph API is recorded.

[Martin Taillefer (geeknoid)]

Applying a recording configuration blocks the whole TUI on synchronous network I/O for up to ~30 s — Correctness · Medium · Confidence High

Every other client round-trip in this monitor is dispatched to a named background thread and read back through a channel: start_snapshot_capture (app.rs:828), start_discovery (app.rs:846), and start_recorder_statistics (app.rs:858). apply_recording_configuration is the single exception — it calls set_recording(descriptor, configuration) directly on the UI thread.

set_recording performs up to three separate blocking round-trips — cache_recording, then SetRecording, then SetCacheRecording (client.rs:54-69). Each connection sets a 300 ms connect timeout but a 10 s read/write timeout (IO_TIMEOUT, client.rs:18). Against a peer that accepts the TCP connection but is then slow or wedged, the frame loop never reaches terminal.draw or event::poll, so the screen freezes and even q/Ctrl-C go unprocessed for the duration — worst case roughly 30 s across the three calls.

The connect timeout does not bound this, because the stall happens after a successful connect.

Direction: Move set_recording onto the same background-thread-plus-receiver pattern already used for statistics and capture, surfacing pending and error state through self.status; or at minimum use a shorter write/response timeout on this interactive path.

Done when: applying a recording configuration against a connected-but-unresponsive monitor keeps the frame loop drawing and the keymap live, or the author argues a multi-second freeze is acceptable here.

[Martin Taillefer (geeknoid)]

Every allocation and free pays for three thread-local snapshot-arena probes that are inactive except during a capture — Performance · Medium · Evidence: Structural

GlobalRallocator::alloc now runs three thread-local probes before any allocation work: snapshot_arena_allocation_suspended() on this line, seismograph::snapshot::snapshot_arena_allocate(layout) (1896), and tracking::snapshot_arena_allocate(layout) (1899). dealloc adds two more (1908, 1911). Each is a thread_local! access with try_with, i.e. a Cell read plus an initialization/ok() branch.

Hot-path argument by position and frequency: this is the process global allocator's entry point, so the cost is paid on every allocation in any program using this allocator — and it is paid unconditionally, including when recording is disabled and no capture is in progress, which is the overwhelmingly common state. The arena is only ever populated inside with_snapshot_arena, i.e. during an active capture. The pre-existing begin_allocation gate is not affected; these probes are new in this PR.

Evidence is Structural rather than measured — I ran no benchmark, and the repository has no committed allocator benchmark numbers covering this path.

Direction: Collapse the alloc-side probes behind a single relaxed global AtomicBool set only while some thread holds an active snapshot arena, and take the TLS walk only when it is true. Same for the dealloc side.

Done when: the disabled-recording, no-capture-in-progress path performs at most one cheap gate before dispatching to the inner allocator, or a measurement shows the TLS probes are not material at this position and the item is closed as not worth doing.

[Ralf Biedert (ralfbiedert)]

🤖 Addressed the initial review and CI findings in 510c97c. Please re-review the updated changes.

[Copilot]

🔵 Needs a closer look

There is a confirmed buffer-identity handling bug in BytesView::append that can break Seismograph I/O correlation (and a small CLI docs mismatch).

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

crates/bytesbuf/src/view.rs:817

• BytesView::append clears the telemetry identity unconditionally. This changes the buffer identity even when other is empty, and it also discards other's identity when appending into an empty self (even though the resulting view is exactly other). This can break seismograph_io correlation for logically unchanged / moved data.

• Files reviewed: 139/202 changed files
• Comments generated: 0 new
• Review effort level: Lite