From 07e6e30ee3e6dd1ecbdbf46a65e08e50d16e82fe Mon Sep 17 00:00:00 2001
From: Nitanshu Lokhande <56120084+nlok5923@users.noreply.github.com>
Date: Thu, 19 Mar 2026 00:04:11 +0530
Subject: [PATCH] fix: add debug_assert invariant guards to critical code paths
 (#1312)

* fix: add debug_assert invariant guards to critical code paths (closes #1215)

Add three debug_assert! calls to catch impossible-in-correct-code states
early in debug builds without affecting release performance:

- execute_tool_with_safety: assert tool_name is non-empty at entry
- JobContext::transition_to: assert state machine transition is valid
- CircuitBreakerProvider::record_success: assert circuit is not Open
  (check_allowed() must gate all calls before record_success())

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* test: add regression test for empty tool name invariant guard

Covers the debug_assert!(!tool_name.is_empty()) added in execute_tool_with_safety.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
---
 src/context/state.rs       |  7 +++++++
 src/llm/circuit_breaker.rs |  6 ++++++
 src/tools/execute.rs       | 23 +++++++++++++++++++++++
 3 files changed, 36 insertions(+)

diff --git a/src/context/state.rs b/src/context/state.rs
index f5307947..bae5bdf1 100644
--- a/src/context/state.rs
+++ b/src/context/state.rs
@@ -258,6 +258,13 @@ impl JobContext {
         new_state: JobState,
         reason: Option<String>,
     ) -> Result<(), String> {
+        debug_assert!(
+            self.state.can_transition_to(new_state),
+            "BUG: invalid job state transition {} -> {} for job {}",
+            self.state,
+            new_state,
+            self.job_id
+        );
         if !self.state.can_transition_to(new_state) {
             return Err(format!(
                 "Cannot transition from {} to {}",
diff --git a/src/llm/circuit_breaker.rs b/src/llm/circuit_breaker.rs
index db47647e..46f29ded 100644
--- a/src/llm/circuit_breaker.rs
+++ b/src/llm/circuit_breaker.rs
@@ -167,6 +167,12 @@ impl CircuitBreakerProvider {
                 }
             }
             CircuitState::Open => {
+                debug_assert!(
+                    false,
+                    "BUG: record_success() called while circuit breaker is Open — \
+                     check_allowed() was bypassed for provider {}",
+                    self.inner.model_name()
+                );
                 // Shouldn't get here (check_allowed blocks Open), but recover
                 state.state = CircuitState::Closed;
                 state.consecutive_failures = 0;
diff --git a/src/tools/execute.rs b/src/tools/execute.rs
index c6c20dc1..fa52c59c 100644
--- a/src/tools/execute.rs
+++ b/src/tools/execute.rs
@@ -22,6 +22,10 @@ pub async fn execute_tool_with_safety(
     params: &serde_json::Value,
     job_ctx: &JobContext,
 ) -> Result<String, Error> {
+    debug_assert!(
+        !tool_name.is_empty(),
+        "BUG: execute_tool_with_safety called with empty tool_name"
+    );
     let tool = tools
         .get(tool_name)
         .await
@@ -291,6 +295,25 @@ mod tests {
         registry
     }
 
+    #[tokio::test]
+    async fn test_execute_empty_tool_name_returns_not_found() {
+        // Regression: execute_tool_with_safety must reject empty tool names before
+        // even attempting a registry lookup (the debug_assert guards this invariant).
+        let registry = registry_with(vec![]).await;
+        let safety = test_safety();
+
+        let result = execute_tool_with_safety(
+            &registry,
+            &safety,
+            "",
+            &serde_json::json!({}),
+            &test_job_ctx(),
+        )
+        .await;
+
+        assert!(result.is_err(), "Empty tool name should return an error"); // safety: test-only assertion
+    }
+
     #[tokio::test]
     async fn test_execute_success() {
         let registry = registry_with(vec![Arc::new(EchoTool)]).await;