fix(fault_manager): allow healing_threshold == 0 (single-event heal)

Threshold validation was too strict: it rewrote healing_threshold=0 to 3,
which broke action_status_bridge's single-event healing (heal on one
PASSED). Require confirmation_threshold < 0 <= healing_threshold instead.
Verified both bridge integration tests pass 5/5.

Author: mfaferek93

src/ros2_medkit_fault_manager/README.md

@@ -150,8 +150,8 @@ The fault manager uses an AUTOSAR DEM-style debounce model:
 
 The counter is always clamped to `[confirmation_threshold, healing_threshold]`, so a long run of
 one-sided events cannot push it out to the integer limits and delay the opposite transition.
-`confirmation_threshold < 0 < healing_threshold` is required; invalid thresholds fall back to safe
-defaults with a warning.
+`confirmation_threshold < 0 <= healing_threshold` is required (`healing_threshold = 0` heals on a
+single PASSED event); invalid thresholds fall back to safe defaults with a warning.
 
 `CONFIRMED` and `HEALED` are **latched** (hysteresis): once reached, the status holds until the
 counter reaches the opposite threshold, so a single opposite-direction event cannot flip it. As a

src/ros2_medkit_fault_manager/design/index.rst

@@ -239,7 +239,8 @@ becomes active again is not back in the default (CONFIRMED-only) list until the
 up to ``healing_threshold - confirmation_threshold`` events. During that window ``occurrence_count``
 and ``last_occurred`` still reflect the activity.
 
-Thresholds must satisfy ``confirmation_threshold < 0 < healing_threshold``; the node validates the
+Thresholds must satisfy ``confirmation_threshold < 0 <= healing_threshold`` (``healing_threshold == 0``
+means heal on a single PASSED event); the node validates the
 merged per-entity config at startup, logs a warning, and falls back to safe defaults if not. When
 healing is disabled, any HEALED row left by a previous (healing-enabled) run is reclassified to
 CLEARED once at startup so it does not behave inconsistently under the latch.

src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/fault_storage.hpp

@@ -49,8 +49,9 @@ struct DebounceConfig {
   /// Whether healing is enabled. When true, faults can transition to HEALED status.
   bool healing_enabled{false};
 
-  /// Healing threshold (positive). When healing is enabled, the fault is HEALED once the counter
-  /// reaches this value. The counter is always clamped to this upper bound, even when healing is
+  /// Healing threshold (non-negative; 0 means heal on a single PASSED event). When healing is enabled,
+  /// the fault is HEALED once the counter reaches this value. The counter is always clamped to this
+  /// upper bound, even when healing is
   /// disabled, so a heal heartbeat cannot drive it off to INT32_MAX; healing_enabled only controls
   /// whether reaching the bound produces a HEALED status.
   /// Default: 3 (3 more PASSED than FAILED events to heal).
@@ -72,8 +73,9 @@ int32_t clamp_debounce_counter(int32_t counter, const DebounceConfig & config);
 /// callers handle that. This is the single source of truth shared by both storage backends.
 std::string compute_debounce_status(int32_t counter, const std::string & current_status, const DebounceConfig & config);
 
-/// Validate a (merged) debounce config in place, enforcing confirmation_threshold < 0 < healing_threshold.
-/// Offending fields are reset to safe defaults (-1 / 3). Returns true if the config was already valid.
+/// Validate a (merged) debounce config in place, enforcing confirmation_threshold < 0 <= healing_threshold
+/// (healing_threshold == 0 means heal on a single PASSED event). Offending fields are reset to safe
+/// defaults (-1 / 3). Returns true if the config was already valid.
 bool sanitize_debounce_config(DebounceConfig & config);
 
 /// Internal fault state stored in memory

src/ros2_medkit_fault_manager/src/fault_manager_node.cpp

@@ -166,7 +166,7 @@ FaultManagerNode::FaultManagerNode(const rclcpp::NodeOptions & options) : Node("
   global_config_.auto_confirm_after_sec = auto_confirm_after_sec_;
   if (!sanitize_debounce_config(global_config_)) {
     RCLCPP_WARN(get_logger(),
-                "Invalid debounce thresholds (need confirmation_threshold < 0 < healing_threshold); using safe "
+                "Invalid debounce thresholds (need confirmation_threshold < 0 <= healing_threshold); using safe "
                 "defaults: confirmation=%d healing=%d",
                 global_config_.confirmation_threshold, global_config_.healing_threshold);
   }
@@ -186,7 +186,7 @@ FaultManagerNode::FaultManagerNode(const rclcpp::NodeOptions & options) : Node("
   if (!entity_thresholds_file.empty()) {
     auto entries = EntityThresholdResolver::load_from_yaml(entity_thresholds_file);
     // Validate each override against the global config: the merge is field by field, so an override
-    // can break confirmation_threshold < 0 < healing_threshold even when the global config is valid.
+    // can break confirmation_threshold < 0 <= healing_threshold even when the global config is valid.
     for (auto & entry : entries) {
       DebounceConfig merged = global_config_;
       if (entry.confirmation_threshold) {
@@ -197,7 +197,7 @@ FaultManagerNode::FaultManagerNode(const rclcpp::NodeOptions & options) : Node("
       }
       if (!sanitize_debounce_config(merged)) {
         RCLCPP_WARN(get_logger(),
-                    "Entity '%s' debounce thresholds invalid (need confirmation_threshold < 0 < healing_threshold); "
+                    "Entity '%s' debounce thresholds invalid (need confirmation_threshold < 0 <= healing_threshold); "
                     "using safe defaults",
                     entry.prefix.c_str());
         if (entry.confirmation_threshold) {
@@ -1226,7 +1226,7 @@ DebounceConfig FaultManagerNode::resolve_config(const std::string & source_id) c
     config = threshold_resolver_->resolve(source_id, global_config_);
   }
   // Defensive: the merged config is validated at load time, but never hand the storage backend a
-  // config that violates confirmation_threshold < 0 < healing_threshold (the counter would stick).
+  // config that violates confirmation_threshold < 0 <= healing_threshold (the counter would stick).
   sanitize_debounce_config(config);
   return config;
 }

src/ros2_medkit_fault_manager/src/fault_storage.cpp

@@ -60,7 +60,9 @@ bool sanitize_debounce_config(DebounceConfig & config) {
     config.confirmation_threshold = -1;
     valid = false;
   }
-  if (config.healing_threshold <= 0) {
+  // healing_threshold == 0 is valid: it means "heal on a single PASSED event" (the counter reaches
+  // the threshold at 0). Only a negative threshold is rejected.
+  if (config.healing_threshold < 0) {
     config.healing_threshold = 3;
     valid = false;
   }

src/ros2_medkit_fault_manager/test/test_fault_manager.cpp

@@ -729,12 +729,34 @@ TEST(DebounceHelpers, SanitizeDebounceConfigFixesBadThresholds) {
   DebounceConfig good;
   EXPECT_TRUE(sanitize_debounce_config(good));  // defaults are valid
 
+  // healing_threshold == 0 is valid (heal on a single PASSED event); leave it untouched.
+  DebounceConfig single_event_heal;
+  single_event_heal.confirmation_threshold = -1;
+  single_event_heal.healing_threshold = 0;
+  EXPECT_TRUE(sanitize_debounce_config(single_event_heal));
+  EXPECT_EQ(single_event_heal.healing_threshold, 0);
+
   DebounceConfig bad;
-  bad.confirmation_threshold = 0;
-  bad.healing_threshold = 0;
+  bad.confirmation_threshold = 0;  // must be < 0
+  bad.healing_threshold = -1;      // must be >= 0
   EXPECT_FALSE(sanitize_debounce_config(bad));
   EXPECT_LT(bad.confirmation_threshold, 0);
-  EXPECT_GT(bad.healing_threshold, 0);
+  EXPECT_GE(bad.healing_threshold, 0);
+}
+
+// healing_threshold == 0 heals on a single PASSED event (used by action_status_bridge).
+TEST_F(FaultStorageTest, HealingThresholdZeroHealsOnSinglePassed) {
+  rclcpp::Clock clock;
+  DebounceConfig config;
+  config.confirmation_threshold = -1;
+  config.healing_enabled = true;
+  config.healing_threshold = 0;
+
+  storage_.report_fault_event("F", ReportFault::Request::EVENT_FAILED, Fault::SEVERITY_ERROR, "e", "/n", clock.now(),
+                              config);
+  ASSERT_EQ(storage_.get_fault("F")->status, Fault::STATUS_CONFIRMED);
+  storage_.report_fault_event("F", ReportFault::Request::EVENT_PASSED, 0, "", "/n", clock.now(), config);
+  EXPECT_EQ(storage_.get_fault("F")->status, Fault::STATUS_HEALED);
 }
 
 TEST(DebounceHelpers, ComputeDebounceStatusLatches) {