fix(fault_manager): keep a black box for faults confirmed mid post-roll

[mfaferek93]

Closes #559.

A fault that confirmed while another fault's post-roll was running was dropped
("Already recording post-fault data, skipping confirmation"), so the correlated
faults - the ones a burst is actually about - ended up with no black box at all.
With the shipped duration_after_sec: 1.0 that is anything confirming within a
second of the first fault.

A burst is one root cause in one window, so it gets one recording with several
lookup keys: attach_to_active_recording() registers the additional fault
against the in-flight bag (capped at 32 codes) and one metadata row per code is
written when the bag closes. The rosbag_files schema already allowed several
codes to name the same file_path.

Deferring the flush instead does not work: the ring buffer is not filled during a
post-roll (messages go straight to the writer), so a queued confirmation would
flush an empty buffer.

Shared bags need refcounted cleanup, so delete_rosbag_file / store_rosbag_file
unlink a bag directory only when no other fault still references the path, and
get_total_rosbag_storage_bytes counts a shared bag once instead of once per
fault - otherwise the quota evicts bags that fit. Both storage backends.

The recording state (current_fault_code_, current_bag_path_,
recording_post_fault_) is now published under post_fault_timer_mutex_, which
also removes an unlocked read of current_bag_path_.

Tests: FaultConfirmedDuringPostRollGetsTheSameBag,
ClearingOneFaultOfABurstKeepsTheSharedBag,
SharedRosbagSurvivesUntilTheLastFaultIsDeleted,
SharedRosbagCountsOnceTowardsStorageTotal. Full fault_manager suite green;
clang-format clean.

Known edge: with duration_after_sec: 0 there is no post-roll, and a burst there
still loses bags because the first flush drains the ring buffer. Different
mechanism, left alone.

[Copilot]

Pull request overview

This PR fixes a fault_manager edge case where a fault confirming during another fault’s post-roll window was previously dropped (no rosbag “black box”). It implements “burst” semantics: multiple correlated fault codes can reference the same in-flight recording, with shared-bag lifetime management and corrected storage accounting so quotas don’t evict shared bags incorrectly.

Changes:

• Attach confirmations arriving during an active post-roll to the in-flight recording (cap 32 attached codes), and persist one rosbag_files row per fault code pointing at the same file_path.
• Make rosbag deletion and storage-total calculations aware of shared bags (unlink only when last reference is removed; count shared bag once).
• Add integration/unit tests covering shared-bag attachment, cleanup behavior, and storage-total accounting.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/ros2_medkit_fault_manager/src/rosbag_capture.cpp	Implements attach-to-active-recording and writes metadata rows for attached fault codes on bag finalization.
src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/rosbag_capture.hpp	Documents burst/shared recording behavior; adds attachment tracking and cap.
src/ros2_medkit_fault_manager/src/sqlite_fault_storage.cpp	Prevents deleting shared bag paths; counts total storage per unique file_path.
src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/sqlite_fault_storage.hpp	Adds helper to detect shared file_path references.
src/ros2_medkit_fault_manager/src/fault_storage.cpp	Mirrors shared-bag semantics in in-memory backend (delete safety + deduped totals).
src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/fault_storage.hpp	Updates API docs to clarify shared-bag deletion and accounting semantics.
src/ros2_medkit_fault_manager/test/test_rosbag_capture.cpp	Adds integration tests for post-roll attachment and shared-bag cleanup.
src/ros2_medkit_fault_manager/test/test_sqlite_storage.cpp	Adds tests for shared-bag refcounted deletion and deduped storage totals.
src/ros2_medkit_fault_manager/CMakeLists.txt	Adds std_msgs dependency for new test publisher helper.

Comments suppressed due to low confidence (1)

src/ros2_medkit_fault_manager/src/rosbag_capture.cpp:997

• post_fault_timer_callback() persists rosbag rows for every code in attached_fault_codes_ unconditionally. If an attached fault is cleared (and auto_cleanup runs) before the post-roll timer fires, the earlier delete_rosbag_file() call is a no-op (no row yet), but the callback will still store a new row for that cleared fault — potentially keeping a shared bag alive indefinitely.

  // One recording, one row per fault it covers: the correlated faults that
  // confirmed inside this window each need their own lookup key to serve it.
  for (const auto & code : attached) {
    info.fault_code = code;
    storage_->store_rosbag_file(info);

[bburda]

Additional findings outside the diff:

• Could you update the docs? docs/tutorials/snapshots.rst and docs/config/fault-manager.rst still say rosbag "records one fault at a time", as do the package README and config/fault_manager.yaml. The auto_cleanup row says bags are deleted when faults are cleared, the entity mode row says only the faulting node's topics are written, and "Bags are named fault_{code}_{timestamp}/" now uses another fault's code. It would also help to say that a shared bag counts once against max_total_storage_mb and that eviction removes a whole burst.
• Could you change test_05_multiple_faults_separate_bags to wait for the first bag instead of sleeping? It asserts the two faults get different bags, and it passes only because its 1.0 s sleep is longer than the 0.5 s post-roll. Under load the second fault attaches and the assert fails.
• Would you add a launch test for this? Reporting two faults with no sleep between them in test_rosbag_integration.test.py, and asserting they return the same file_path, would cover the fix. That test fails on main.
• Could you also put a shared recording id in the x-medkit block? GET /{entity}/bulk-data/rosbags now lists one descriptor per fault for a shared bag, each with the full size, and a client cannot tell they are one file.

[mfaferek93]

The outside-diff items:

• Docs updated everywhere the one-bag story lived (snapshots.rst, fault-manager.rst, package README, fault_manager.yaml): burst semantics, auto_cleanup on the last fault of the burst, entity-mode topic widening, attached faults under the first fault's directory name, once-per-bag accounting and whole-burst eviction - ae6b21d.
• test_05 no longer sleeps: it waits for the previous fault's bag to be finalized before the next report - 0aacfd2.
• New test_07_burst_faults_share_one_recording reports two faults back to back and asserts one shared file_path; passes on the branch, run twice.
• Descriptors carry x-medkit.recording_id (bag directory basename) so clients can group a burst - 3238f18, unit tests plus a live assert in test_bulk_data_api.test.py.