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 <[email protected]>

* 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 <[email protected]>

---------

Co-authored-by: Claude Sonnet 4.6 <[email protected]>
This commit is contained in:
Nitanshu Lokhande
2026-03-18 11:34:11 -07:00
committed by GitHub
co-authored by Claude Sonnet 4.6
parent 2d0b195321
commit 07e6e30ee3
3 changed files with 36 additions and 0 deletions
+7
View File
@@ -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 {}",
+6
View File
@@ -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;
+23
View File
@@ -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;