Keep more than one rosbag per fault, and stop losing the evidence around it

docs/api/rest.rst

@@ -1077,7 +1077,7 @@ Query and manage faults.
             {
               "type": "rosbag",
               "name": "fault_recording",
-              "bulk_data_uri": "/apps/motor_controller/bulk-data/rosbags/550e8400-e29b-41d4-a716-446655440000",
+              "bulk_data_uri": "/apps/motor_controller/bulk-data/rosbags/fault_MOTOR_OVERHEAT_1738664999000",
               "size_bytes": 1234567,
               "duration_sec": 6.0,
               "format": "mcap"
@@ -1334,13 +1334,13 @@ List all bulk-data items in a category for the entity.
    {
      "items": [
        {
-         "id": "550e8400-e29b-41d4-a716-446655440000",
-         "name": "MOTOR_OVERHEAT recording 2026-02-04T10:30:00Z",
+         "id": "fault_MOTOR_OVERHEAT_1738664999000",
+         "name": "fault_MOTOR_OVERHEAT_1738664999000 recording 2026-02-04T10:30:00.000Z",
          "mimetype": "application/x-mcap",
          "size": 1234567,
          "creation_date": "2026-02-04T10:30:00.000Z",
          "x-medkit": {
-           "fault_code": "MOTOR_OVERHEAT",
+           "fault_codes": ["MOTOR_OVERHEAT", "MOTOR_STALL"],
            "duration_sec": 6.0,
            "format": "mcap",
            "recording_id": "fault_MOTOR_OVERHEAT_1738664999000"
@@ -1349,10 +1349,13 @@ List all bulk-data items in a category for the entity.
      ]
    }
 
-For ``rosbags``, faults confirmed in one burst share a single recording: each
-fault gets its own descriptor with the full bag size, and
-``x-medkit.recording_id`` (the bag directory name) is the same for every
-descriptor served from that recording, so clients can group them.
+For ``rosbags``, the descriptor ``id`` is the recording id - the bag directory
+name - and it is what the download URL takes. There is **one descriptor per
+recording**, not one per fault: faults confirmed in one burst share a single
+recording, and ``x-medkit.fault_codes`` lists every fault attached to it. A
+recording therefore reports its size once. One fault code can appear on several
+descriptors, one per occurrence it kept, told apart by ``creation_date``, which
+is the time that recording was made.
 
 Download Bulk Data
 ~~~~~~~~~~~~~~~~~~
@@ -1364,14 +1367,14 @@ Download a specific bulk-data file.
 **Response Headers:**
 
 - ``Content-Type``: ``application/x-mcap`` (MCAP format) or ``application/x-sqlite3`` (db3)
-- ``Content-Disposition``: ``attachment; filename="FAULT_CODE.mcap"``
+- ``Content-Disposition``: ``attachment; filename="<recording_id>.mcap"`` (named after the recording actually served, which for a pre-#620 fault-code URL is not the segment the client sent)
 - ``Access-Control-Expose-Headers``: ``Content-Disposition``
 
 **Example:**
 
 .. code-block:: bash
 
-   curl -O -J http://localhost:8080/api/v1/apps/motor_controller/bulk-data/rosbags/550e8400-e29b-41d4-a716-446655440000
+   curl -O -J http://localhost:8080/api/v1/apps/motor_controller/bulk-data/rosbags/fault_MOTOR_OVERHEAT_1738664999000
 
 **Response Codes:**
 
@@ -2814,7 +2817,8 @@ Other extensions beyond SOVD:
   optional ``x-medkit`` SOVD payload-extension object with ``entity_type`` and ``entity_id``
   fields when the gateway can resolve the fault's first reporting source back to an entity,
   so consumers can hit ``/{entity_type}/{entity_id}/bulk-data/rosbags/{fault_code}`` directly
-  without enumerating entities. Resolution is snapshotted at event arrival; the entire
+  without enumerating entities - that address serves the fault's newest recording. To reach an
+  older one, list ``/bulk-data/rosbags`` and use the descriptor ``id``. Resolution is snapshotted at event arrival; the entire
   ``x-medkit`` object is omitted when no entity can be resolved.
 - ``/health`` - Health check with discovery pipeline diagnostics
 - ``/version-info`` - Gateway version information

docs/config/fault-manager.rst

@@ -209,8 +209,18 @@ Basic Snapshot Settings
        Prevents snapshot storms when a fault is reported repeatedly. Set to 0 to disable.
    * - ``snapshots.max_per_fault``
      - ``10``
-     - Maximum number of snapshots stored per fault code. When the limit is reached,
-       new snapshots for that fault are rejected. Set to 0 for unlimited.
+     - Maximum number of snapshot rows stored per fault code. One confirmation
+       writes one row per configured topic, and those rows are evicted together:
+       past the limit the OLDEST capture set is dropped whole. A capture larger
+       than the cap is kept anyway rather than torn, since half a freeze frame is
+       indistinguishable from topics that were silent. Set to 0 for unlimited.
+   * - ``snapshots.retain_on_clear``
+     - ``false``
+     - Keep a fault's value snapshots when it is acknowledged. ``false`` is the
+       historical behaviour: clearing a fault deletes them. Turn it on together
+       with ``rosbag.max_bags_per_fault``, or acknowledging leaves the fault
+       holding recordings whose matching readings are gone. Independent of
+       ``max_per_fault``, which still bounds growth either way.
    * - ``snapshots.capture_pool_size``
      - ``2``
      - Max concurrent capture threads under a fault storm (>= 1). The capture pool is
@@ -250,6 +260,7 @@ Capture continuous rosbag recordings around fault events.
            max_buffer_mb: 256              # Ring-buffer RAM cap
            max_bag_size_mb: 50             # Max size per bag file
            max_total_storage_mb: 500       # Max total storage
+           max_bags_per_fault: 1           # Recordings kept per fault code
            auto_cleanup: true              # Auto-delete old bags
 
 .. list-table::
@@ -324,10 +335,41 @@ Capture continuous rosbag recordings around fault events.
      - Maximum total storage for all rosbags (MB). A recording shared by a
        burst of faults counts once towards the total, and eviction removes a
        whole burst's bag at a time (oldest first).
+   * - ``rosbag.max_bags_per_fault``
+     - ``1``
+     - How many recordings one fault code keeps. Past the cap the oldest is
+       unlinked, so the default reproduces the historical behaviour exactly: a
+       new recording replaces the previous one. ``0`` means unlimited, bounded
+       only by ``max_total_storage_mb``. ``3`` is a reasonable value for a fault
+       that flaps - see the note below before raising it.
    * - ``rosbag.auto_cleanup``
      - ``true``
-     - Delete a fault's bag when the fault is cleared. A recording shared by a
-       burst survives until the last fault referencing it clears.
+     - Delete a fault's bags when the fault is cleared. A recording shared by a
+       burst survives until the last fault referencing it clears. Has no effect
+       once ``max_bags_per_fault`` is anything other than ``1``: a history someone
+       configured must not be what an acknowledgement takes away, so the cap
+       governs retention there instead.
+
+.. note::
+
+   ``max_bags_per_fault`` is a **fairness** knob, not a depth knob.
+   ``max_total_storage_mb`` is the real disk bound and eviction across it is
+   global and oldest-first, so a fault that flaps often enough will consume the
+   budget and push out every other fault's black box. Raise the per-fault cap
+   when you need the history of a specific intermittent fault; raise the total
+   budget with it if other faults still need theirs.
+
+   The cap keeps the newest recordings and evicts the oldest, the same direction
+   as ``snapshots.max_per_fault``. Refusing a NEW recording instead would mean a
+   technician standing next to a machine faulting right now downloads a bag from
+   three days ago.
+
+   ``snapshots.recapture_cooldown_sec`` (default 60 s) gates the capture job as a
+   whole, rosbags included, so it puts a floor under how fast a history can grow:
+   a fault that returns sooner than the cooldown keeps ONE recording however high
+   this cap is. That is the fast-flapping fault the cap exists for, so lower the
+   cooldown when you raise the cap. The fault manager logs a warning at startup
+   when the two are configured against each other.
 
 .. _rosbag-recording-lifecycle:
 

docs/tutorials/snapshots.rst

@@ -31,8 +31,9 @@ configuration always overrides the zero-config fallback when present.
 
 .. note::
 
-   Snapshots are automatically deleted when a fault is cleared via the
+   By default, snapshots are deleted when a fault is cleared via the
    ``DELETE /api/v1/faults/{code}`` endpoint or ``~/clear_fault`` service.
+   Set ``snapshots.retain_on_clear: true`` to keep them across clears.
 
 Quick Start
 -----------
@@ -98,8 +99,19 @@ Configure snapshot capture via fault manager parameters:
        Prevents snapshot storms when a fault is reported repeatedly. Set to 0 to disable.
    * - ``snapshots.max_per_fault``
      - ``10``
-     - Maximum number of snapshots stored per fault code. When the limit is reached,
-       new snapshots for that fault are rejected. Set to 0 for unlimited.
+     - Maximum number of snapshot rows stored per fault code. One confirmation
+       writes one row per configured topic, and those rows are evicted together:
+       past the limit the OLDEST capture set is dropped whole. A capture larger
+       than the cap is kept anyway rather than torn, since half a freeze frame
+       is indistinguishable from topics that were silent. Set to 0 for
+       unlimited.
+   * - ``snapshots.retain_on_clear``
+     - ``false``
+     - Keep a fault's value snapshots when it is acknowledged. ``false`` is the
+       historical behaviour: clearing a fault deletes them. Turn it on together
+       with ``rosbag.max_bags_per_fault``, or acknowledging leaves the fault
+       holding recordings whose matching readings are gone. Independent of
+       ``max_per_fault``, which still bounds growth either way.
    * - ``snapshots.capture_pool_size``
      - ``2``
      - Max concurrent capture threads under a fault storm (>= 1). This parallelizes
@@ -266,7 +278,7 @@ Snapshots are included inline in the fault response as ``environment_data``:
          {
            "type": "rosbag",
            "name": "fault_recording",
-           "bulk_data_uri": "/apps/motor_controller/bulk-data/rosbags/550e8400-e29b-41d4-a716-446655440000",
+           "bulk_data_uri": "/apps/motor_controller/bulk-data/rosbags/fault_MOTOR_OVERHEAT_1738664999000",
            "size_bytes": 1234567,
            "duration_sec": 6.0,
            "format": "mcap"
@@ -286,7 +298,10 @@ Snapshots are included inline in the fault response as ``environment_data``:
   gateway start instead, marked ``x-medkit.capture_origin: startup``; a
   plugin entity that reports its link down contributes its last known values,
   marked ``connected: false`` in ``x-medkit``
-- ``rosbag``: Recording file available via bulk-data endpoint (binary format)
+- ``rosbag``: Recording file available via bulk-data endpoint (binary format).
+  One entry per recording the fault kept, newest first, each addressed by its own
+  ``bulk_data_uri``. With the default ``max_bags_per_fault`` of ``1`` there is at
+  most one.
 
 **Get snapshots from fault response using jq:**
 
@@ -345,7 +360,8 @@ Troubleshooting
 
 **Empty topics object in response**
 
-- The fault may have been cleared (snapshots are deleted on clear)
+- The fault may have been cleared (snapshots are deleted on clear unless
+  ``snapshots.retain_on_clear`` is enabled)
 - No topics were configured for this fault code
 - All configured topics timed out or exceeded size limit
 
@@ -495,7 +511,10 @@ Rosbag Configuration Options
      - ``true``
      - Automatically delete a fault's bag when it is cleared. A recording
        shared by a burst of faults is deleted when the last fault referencing
-       it clears. Set to ``false`` to retain bags for manual analysis.
+       it clears. Set to ``false`` to retain bags for manual analysis. Has no
+       effect once ``max_bags_per_fault`` is anything other than ``1``: a
+       history someone configured must not be what an acknowledgement takes
+       away, so the cap governs retention there instead.
    * - ``snapshots.rosbag.lazy_start``
      - ``false``
      - Controls when the ring buffer starts recording. See diagram below.
@@ -509,6 +528,17 @@ Rosbag Configuration Options
        deleted when this limit is exceeded. A recording shared by a burst of
        faults counts once towards the total, and eviction removes a whole
        burst's bag at a time.
+   * - ``snapshots.rosbag.max_bags_per_fault``
+     - ``1``
+     - Recordings kept per fault code; ``0`` means unlimited. Past the cap the
+       fault's oldest recording is dropped, and a bag is deleted only once no
+       fault still references it (a burst shares one recording). ``1`` is the
+       historical behaviour - each re-confirmation replaces the previous bag;
+       ``3`` is a good starting point for an intermittent fault you are
+       chasing. This is a fairness knob rather than a depth knob:
+       ``max_total_storage_mb`` is the real disk bound, and a high value lets
+       one flapping fault consume the budget and evict every other fault's
+       recording.
 
 Bursts and the Window Boundary
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -700,20 +730,27 @@ Rosbag files are downloaded via SOVD bulk-data endpoints.
 
    curl http://localhost:8080/api/v1/apps/motor_controller/bulk-data/rosbags
 
+One item per **recording**, not per fault. A burst of correlated faults shares a
+single recording and appears once, with every fault it covers listed in
+``x-medkit.fault_codes``. A fault that confirmed several times contributes one
+item per recording it kept (see ``max_bags_per_fault`` in the configuration
+table above).
+
 **Response:**
 
 .. code-block:: json
 
    {
      "items": [
        {
-         "id": "550e8400-e29b-41d4-a716-446655440000",
-         "name": "MOTOR_OVERHEAT recording",
+         "id": "fault_MOTOR_OVERHEAT_1738664999000",
+         "name": "fault_MOTOR_OVERHEAT_1738664999000 recording 2026-02-04T10:30:00.000Z",
          "mimetype": "application/x-mcap",
          "size": 1234567,
          "creation_date": "2026-02-04T10:30:00.000Z",
          "x-medkit": {
-           "fault_code": "MOTOR_OVERHEAT",
+           "fault_codes": ["MOTOR_OVERHEAT"],
+           "recording_id": "fault_MOTOR_OVERHEAT_1738664999000",
            "duration_sec": 6.0,
            "format": "mcap"
          }
@@ -723,12 +760,15 @@ Rosbag files are downloaded via SOVD bulk-data endpoints.
 
 **2. Download a specific rosbag:**
 
-Use the ``bulk_data_uri`` from the fault response, or construct from listing:
+Use the ``bulk_data_uri`` from the fault response, or the descriptor ``id`` from
+the listing. A URL carrying a bare fault code instead of a recording id still
+resolves and serves that fault's newest recording, so addresses built before
+recordings had their own identity keep working.
 
 .. code-block:: bash
 
    # Using bulk_data_uri from fault response
-   curl -O -J http://localhost:8080/api/v1/apps/motor_controller/bulk-data/rosbags/550e8400-e29b-41d4-a716-446655440000
+   curl -O -J http://localhost:8080/api/v1/apps/motor_controller/bulk-data/rosbags/fault_MOTOR_OVERHEAT_1738664999000
 
 The ``-J`` flag uses the server-provided filename from ``Content-Disposition`` header.
 

src/ros2_medkit_fault_manager/CHANGELOG.rst

@@ -4,6 +4,7 @@ Changelog for package ros2_medkit_fault_manager
 
 Forthcoming
 -----------
+* Rosbag black-box recordings are no longer limited to one per fault code. A fault that re-confirms keeps a bounded history of recordings instead of overwriting the previous one, controlled by the new ``snapshots.rosbag.max_bags_per_fault`` (default ``1``, which reproduces the previous behaviour exactly; ``0`` = unlimited). Retention is keep-newest and the bag is unlinked only when no fault still references it, so a burst that shares one recording behaves as before. Internally the ``rosbag_files`` grain changed from "one row per fault" to "one row per (fault, recording) link": ``recording_id`` is now a stored, indexed column, and the legacy column-level ``UNIQUE(fault_code)`` is replaced by a ``UNIQUE INDEX`` on ``(fault_code, file_path)`` through an automatic, idempotent table rebuild on first open. Four latent defects are fixed on the way: quota eviction deleted by fault code rather than by recording, ``get_rosbag_file`` had no ``ORDER BY`` and would have served an arbitrary recording, the stale-row self-heals deleted a fault's entire history because one bag had vanished from disk, and both ``delete_rosbag_file`` / ``delete_rosbag_files`` read only the first ``file_path`` of a fault, so deleting a fault with several recordings removed every row but left all but one bag on disk - unreachable and still charged against the quota (`#620 <https://github.com/selfpatch/ros2_medkit/issues/620>`_)
 * Optional append-only, hash-chained audit log of fault state transitions: each transition appends one immutable row (``record_hash = sha256(prev_hash + canonical(event))`` via OpenSSL EVP SHA-256) with a persisted chain head, a ``verify`` routine, a read API, and retention that seals a segment anchor before pruning. Time-based (PREFAILED->CONFIRMED) auto-confirmations are also audited. ``verify`` reads the chain head directly from the database, so deleting the newest row together with the head row is reported as tampering instead of silently recovering. ``BEFORE UPDATE`` / ``BEFORE DELETE`` triggers reject out-of-band edits as defense-in-depth. The chain is unkeyed and stored in a single writable file, so ``verify`` detects edits/deletions that did not recompute the chain (casual or accidental tampering); it is not a defence against an attacker who can rewrite the whole file. Off by default (`#483 <https://github.com/selfpatch/ros2_medkit/issues/483>`_)
 
 0.6.0 (2026-06-22)

src/ros2_medkit_fault_manager/CMakeLists.txt

@@ -136,6 +136,11 @@ if(BUILD_TESTING)
   target_link_libraries(test_sqlite_storage fault_manager_lib)
   medkit_target_dependencies(test_sqlite_storage rclcpp ros2_medkit_msgs)
 
+  # Rosbag retention parity: every assertion runs against both storage backends.
+  medkit_add_gtest(test_rosbag_storage_parity test/test_rosbag_storage_parity.cpp)
+  target_link_libraries(test_rosbag_storage_parity fault_manager_lib)
+  medkit_target_dependencies(test_rosbag_storage_parity rclcpp ros2_medkit_msgs)
+
   # Fault audit log tests (hash chain, verify, rotation, reopen)
   medkit_add_gtest(test_fault_audit_log test/test_fault_audit_log.cpp)
   target_link_libraries(test_fault_audit_log fault_manager_lib)
@@ -190,6 +195,12 @@ if(BUILD_TESTING)
   medkit_add_launch_test(test_rosbag_entity_scope test/test_rosbag_entity_scope.test.py TIMEOUT 120
     LABELS "integration")
 
+  # The only suite running above max_bags_per_fault=1: drives confirm / clear /
+  # confirm on one code and asserts both recordings survive and stay separately
+  # addressable. Six occurrences plus their post-roll windows, hence the timeout.
+  medkit_add_launch_test(test_rosbag_history test/test_rosbag_history.test.py TIMEOUT 240
+    LABELS "integration")
+
   # Parametrized over both storage formats (sqlite3 + mcap), so the launch
   # runs twice inside one ctest invocation.
   medkit_add_launch_test(test_rosbag_boundary test/test_rosbag_boundary.test.py TIMEOUT 240