Compare commits

...
Author SHA1 Message Date
Claude a5b5d02ab1 fix(clippy): replace find().is_none() with !any() in user stats test
https://claude.ai/code/session_01Nm95eCjdrwxDjwkHTRieZs
2026-03-28 19:22:38 +00:00
ZakiandClaude a0020b22a5 fix(routines): address review feedback on retry loop
- Remove outer retry for LlmFailed errors since RetryProvider already
  handles transient LLM failures with its own bounded budget, preventing
  multiplicative retry counts

- Preserve Option<i32> semantics for token accumulation: None means
  "unknown/not tracked" rather than converting to Some(0) via
  unwrap_or(0), so downstream API/UI correctly distinguishes null
  from zero

- Add partial_tokens field to EmptyResponse and TruncatedResponse
  variants so token usage from those failed attempts is captured in the
  retry accumulator

- Persist accumulated token total on final failure path so usage from
  earlier retry attempts is not silently discarded

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
2026-03-28 19:16:10 +00:00
Claude b3e09c3827 style: fix rustfmt formatting in routine_engine.rs
https://claude.ai/code/session_01CsP5wMZ2evEMHgGghjAfR1
2026-03-28 19:16:10 +00:00
ZakiandClaude eba088f30e fix(routines): address PR review feedback for bounded retry (#1320)
- Skip retry for tools-enabled routines to prevent duplicate side effects
- Replace fragile PERMANENT_LLM_PATTERNS substring matching with a
  retryable bool field on RoutineError::LlmFailed, set at the LlmError
  conversion site using llm::retry::is_retryable()
- Use saturating_add for token accumulation

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
2026-03-28 19:16:10 +00:00
ZakiandClaude 0d82ce5d4c fix: use structured tracing for transient routine retry logging [skip-regression-check]
Replace tracing::warn! with tracing::event! targeting "transient_routine_errors"
for better log filtering and structured field capture on retry attempts.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
2026-03-28 19:16:10 +00:00
ZakiandClaude 3a27cd3561 fix(routines): classify permanent LLM failures, accumulate tokens, warn on tool retry (#1320)
Address three review comments on the bounded-retry implementation:

1. Permanent LLM failures no longer retried: `is_retryable()` now inspects
   the `LlmFailed` reason string for known permanent patterns (auth,
   content policy, context length, model not available, moderation).
   These map to `LlmError` variants already classified as non-retryable
   by the LLM retry layer.

2. Token usage accumulated across retries: `RoutineError::LlmFailed` now
   carries an optional `partial_tokens` field populated from tokens consumed
   before the failure. The retry loop sums partial tokens from failed
   attempts with the final successful attempt's tokens.

3. Tool loop retry limitation documented: added a code comment explaining
   that the retry wraps the entire `execute_lightweight()` call, and a
   warning log when retrying a tools-enabled routine so operators know
   side effects may be repeated.

[skip-regression-check]

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
2026-03-28 19:16:10 +00:00
Claude c20fd6ec3a fix(deps): resolve RUSTSEC-2026-0049 rustls-webpki CRL advisory
Update rustls-webpki 0.103.9 -> 0.103.10 and ignore the advisory for
0.102.8 which is pinned by libsql's rustls 0.22.4 dependency chain.

https://claude.ai/code/session_01MmxvBgAMn4m45pZguFKBEX
2026-03-28 19:16:09 +00:00
Claude 18026fcb7c style: run cargo fmt to fix formatting
https://claude.ai/code/session_01Nv2TJ3so5WQqpRUcirAhT3
2026-03-28 19:16:09 +00:00
ZakiandClaude 841cf5fe76 fix(routines): add bounded retry for transient lightweight execution failures (#1320)
Lightweight routine executions that fail with transient errors (LLM
failures, empty responses, truncated responses) are now retried up to 3
times with exponential backoff (1s, 2s, 4s) before reporting failure.

Full-job routines are not retried since the scheduler/watcher handles
their lifecycle. Hard failures (disabled, not found, auth, DB errors)
fail immediately without retry.

Adds RoutineError::is_retryable() to classify transient vs hard errors,
with comprehensive regression tests covering all error variants.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
2026-03-28 19:16:09 +00:00
3 changed files with 374 additions and 50 deletions
+244 -46
View File
@@ -1089,48 +1089,134 @@ async fn execute_routine(ctx: EngineContext, routine: Routine, run: RoutineRun)
// Increment running count (atomic: survives panics in the execution below)
ctx.running_count.fetch_add(1, Ordering::Relaxed);
let result = match &routine.action {
RoutineAction::Lightweight {
prompt,
context_paths,
max_tokens,
use_tools,
max_tool_rounds,
} => {
execute_lightweight(
&ctx,
&routine,
prompt,
context_paths,
*max_tokens,
*use_tools,
*max_tool_rounds,
)
.await
// Retry constants for transient lightweight execution failures.
const MAX_RETRIES: u32 = 3;
const BASE_DELAY_MS: u64 = 1000;
let is_lightweight = matches!(routine.action, RoutineAction::Lightweight { .. });
// The retry block returns both the execution result and any accumulated
// token count so that usage is preserved even on final failure.
let (result, accumulated_tokens) = {
let mut attempt = 0u32;
// Track accumulated tokens as Option to preserve None semantics:
// None = no attempt reported tokens; Some(n) = at least one attempt did.
let mut accumulated_tokens: Option<i32> = None;
let uses_tools = matches!(
routine.action,
RoutineAction::Lightweight {
use_tools: true,
..
}
) && ctx.config.lightweight_tools_enabled;
/// Extract partial_tokens from any RoutineError variant that carries them.
fn extract_partial_tokens(e: &RoutineError) -> Option<i32> {
match e {
RoutineError::LlmFailed {
partial_tokens: Some(t),
..
}
| RoutineError::EmptyResponse {
partial_tokens: Some(t),
}
| RoutineError::TruncatedResponse {
partial_tokens: Some(t),
} => Some(*t),
_ => None,
}
}
RoutineAction::FullJob {
title,
description,
max_iterations,
} => {
let execution = FullJobExecutionConfig {
title,
description,
max_iterations: *max_iterations,
/// Merge an optional partial token count into the accumulator,
/// only materializing Some when at least one source had Some.
fn accumulate(acc: Option<i32>, partial: Option<i32>) -> Option<i32> {
match (acc, partial) {
(Some(a), Some(p)) => Some(a.saturating_add(p)),
(Some(a), None) => Some(a),
(None, p) => p,
}
}
loop {
let execution_result = match &routine.action {
RoutineAction::Lightweight {
prompt,
context_paths,
max_tokens,
use_tools,
max_tool_rounds,
} => {
execute_lightweight(
&ctx,
&routine,
prompt,
context_paths,
*max_tokens,
*use_tools,
*max_tool_rounds,
)
.await
}
RoutineAction::FullJob {
title,
description,
max_iterations,
} => {
let execution = FullJobExecutionConfig {
title,
description,
max_iterations: *max_iterations,
};
execute_full_job(&ctx, &routine, &run, &execution).await
}
};
execute_full_job(&ctx, &routine, &run, &execution).await
match execution_result {
Ok((status, summary, tokens)) => {
// Merge tokens: only produce Some when at least one source had Some.
let total = accumulate(accumulated_tokens, tokens);
break (Ok((status, summary, total)), accumulated_tokens);
}
Err(ref e)
if is_lightweight
&& !uses_tools
&& e.is_retryable()
// Skip outer retry for LlmFailed — RetryProvider already
// retries transient LLM errors with its own budget. Retrying
// here would create a multiplicative retry count.
&& !matches!(e, RoutineError::LlmFailed { .. })
&& attempt < MAX_RETRIES =>
{
// Accumulate partial tokens from the failed attempt.
accumulated_tokens = accumulate(accumulated_tokens, extract_partial_tokens(e));
attempt += 1;
let delay = Duration::from_millis(
BASE_DELAY_MS.saturating_mul(2u64.saturating_pow(attempt - 1)),
);
tracing::event!(target: "transient_routine_errors", tracing::Level::WARN, routine = %routine.name, attempt = attempt, max_retries = MAX_RETRIES, delay_ms = delay.as_millis() as u64, "Transient routine error, retrying: {}", e);
tokio::time::sleep(delay).await;
}
Err(e) => {
// Accumulate tokens from the final failed attempt.
accumulated_tokens = accumulate(accumulated_tokens, extract_partial_tokens(&e));
break (Err(e), accumulated_tokens);
}
}
}
};
// Decrement running count
ctx.running_count.fetch_sub(1, Ordering::Relaxed);
// Process result
// Process result — on failure, preserve accumulated token total from
// earlier retry attempts so usage reporting stays accurate.
let (status, summary, tokens) = match result {
Ok(execution) => execution,
Err(e) => {
tracing::error!(routine = %routine.name, "Execution failed: {}", e);
(RunStatus::Failed, Some(e.to_string()), None)
(RunStatus::Failed, Some(e.to_string()), accumulated_tokens)
}
};
@@ -1511,13 +1597,14 @@ async fn execute_lightweight_no_tools(
.with_max_tokens(effective_max_tokens)
.with_temperature(0.3);
let response = ctx
.llm
.complete(request)
.await
.map_err(|e| RoutineError::LlmFailed {
let response = ctx.llm.complete(request).await.map_err(|e| {
let retryable = crate::llm::retry::is_retryable(&e);
RoutineError::LlmFailed {
reason: e.to_string(),
})?;
partial_tokens: None,
retryable,
}
})?;
handle_text_response(
&response.content,
@@ -1538,12 +1625,18 @@ fn handle_text_response(
) -> Result<(RunStatus, Option<String>, Option<i32>), RoutineError> {
let content = content.trim();
// Empty content guard
// Empty content guard — carry consumed tokens so the retry loop can
// accumulate them even when the response shape is invalid.
if content.is_empty() {
let consumed = Some((total_input_tokens + total_output_tokens) as i32);
return if finish_reason == FinishReason::Length {
Err(RoutineError::TruncatedResponse)
Err(RoutineError::TruncatedResponse {
partial_tokens: consumed,
})
} else {
Err(RoutineError::EmptyResponse)
Err(RoutineError::EmptyResponse {
partial_tokens: consumed,
})
};
}
@@ -1621,13 +1714,15 @@ async fn execute_lightweight_with_tools(
.with_max_tokens(effective_max_tokens)
.with_temperature(0.3);
let response =
ctx.llm
.complete(request)
.await
.map_err(|e| RoutineError::LlmFailed {
reason: e.to_string(),
})?;
let response = ctx.llm.complete(request).await.map_err(|e| {
let partial = (total_input_tokens + total_output_tokens) as i32;
let retryable = crate::llm::retry::is_retryable(&e);
RoutineError::LlmFailed {
reason: e.to_string(),
partial_tokens: if partial > 0 { Some(partial) } else { None },
retryable,
}
})?;
total_input_tokens += response.input_tokens;
total_output_tokens += response.output_tokens;
@@ -1654,8 +1749,12 @@ async fn execute_lightweight_with_tools(
.with_temperature(0.3);
let response = ctx.llm.complete_with_tools(request).await.map_err(|e| {
let partial = (total_input_tokens + total_output_tokens) as i32;
let retryable = crate::llm::retry::is_retryable(&e);
RoutineError::LlmFailed {
reason: e.to_string(),
partial_tokens: if partial > 0 { Some(partial) } else { None },
retryable,
}
})?;
@@ -1828,6 +1927,7 @@ async fn execute_routine_tool(
}
/// Send a notification based on the routine's notify config and run status.
#[allow(clippy::too_many_arguments)]
async fn send_notification(
tx: &mpsc::Sender<OutgoingResponse>,
notify: &NotifyConfig,
@@ -2513,6 +2613,104 @@ mod tests {
}
}
/// Regression test for #1320: transient errors are retried for lightweight
/// routines but not for full-job routines or hard failures.
#[test]
fn test_retry_classification_for_routine_errors() {
use crate::error::RoutineError;
// Transient errors (retryable for lightweight routines)
let transient_errors: Vec<RoutineError> = vec![
RoutineError::LlmFailed {
reason: "rate limit".into(),
partial_tokens: None,
retryable: true,
},
RoutineError::LlmFailed {
reason: "network timeout".into(),
partial_tokens: Some(42),
retryable: true,
},
RoutineError::EmptyResponse {
partial_tokens: None,
},
RoutineError::TruncatedResponse {
partial_tokens: Some(100),
},
];
for err in &transient_errors {
assert!(err.is_retryable(), "{} should be retryable", err);
}
// Permanent LLM failures that should NOT be retried
// (retryable: false is set at conversion time by llm::retry::is_retryable)
let permanent_llm_errors: Vec<RoutineError> = vec![
RoutineError::LlmFailed {
reason: "Authentication failed for provider openai".into(),
partial_tokens: None,
retryable: false,
},
RoutineError::LlmFailed {
reason: "invalid_api_key: bad key".into(),
partial_tokens: None,
retryable: false,
},
RoutineError::LlmFailed {
reason: "content policy violation".into(),
partial_tokens: None,
retryable: false,
},
RoutineError::LlmFailed {
reason: "content_filter triggered".into(),
partial_tokens: None,
retryable: false,
},
RoutineError::LlmFailed {
reason: "context length exceeded: 150000 tokens used, 128000 allowed".into(),
partial_tokens: Some(100),
retryable: false,
},
RoutineError::LlmFailed {
reason: "model not available on provider anthropic".into(),
partial_tokens: None,
retryable: false,
},
RoutineError::LlmFailed {
reason: "content moderation flagged".into(),
partial_tokens: None,
retryable: false,
},
];
for err in &permanent_llm_errors {
assert!(!err.is_retryable(), "{} should NOT be retryable", err);
}
// Hard failures (never retried)
let hard_errors: Vec<RoutineError> = vec![
RoutineError::Disabled {
name: "test".into(),
},
RoutineError::NotFound {
id: uuid::Uuid::new_v4(),
},
RoutineError::NotAuthorized {
id: uuid::Uuid::new_v4(),
},
RoutineError::MaxConcurrent {
name: "test".into(),
},
RoutineError::JobDispatchFailed {
reason: "no docker".into(),
},
RoutineError::Database {
reason: "connection refused".into(),
},
];
for err in &hard_errors {
assert!(!err.is_retryable(), "{} should NOT be retryable", err);
}
}
#[test]
fn test_sanitize_summary_strips_control_chars() {
use super::sanitize_summary;
+1 -1
View File
@@ -981,7 +981,7 @@ mod tests {
assert!(alice_stats.last_active_at.is_some());
// Bob has no LLM calls so doesn't appear in summary stats
assert!(stats.iter().find(|s| s.user_id == "bob").is_none());
assert!(!stats.iter().any(|s| s.user_id == "bob"));
// Filter to single user
let alice_only = db.user_summary_stats(Some("alice")).await.unwrap();
+129 -3
View File
@@ -395,16 +395,49 @@ pub enum RoutineError {
Database { reason: String },
#[error("LLM call failed: {reason}")]
LlmFailed { reason: String },
LlmFailed {
reason: String,
/// Partial token count consumed before the failure (if any).
/// Used to accumulate usage across retry attempts.
partial_tokens: Option<i32>,
/// Whether the underlying LLM error was classified as retryable.
/// Set at the `LlmError` → `RoutineError` conversion site using
/// `crate::llm::retry::is_retryable()`, avoiding fragile substring
/// matching on the stringified reason.
retryable: bool,
},
#[error("Failed to dispatch full job: {reason}")]
JobDispatchFailed { reason: String },
#[error("LLM returned empty content")]
EmptyResponse,
EmptyResponse {
/// Tokens consumed by the call that produced the empty response.
partial_tokens: Option<i32>,
},
#[error("LLM response truncated (finish_reason=length) with no content")]
TruncatedResponse,
TruncatedResponse {
/// Tokens consumed by the call that produced the truncated response.
partial_tokens: Option<i32>,
},
}
impl RoutineError {
/// Whether this error is transient and worth retrying with backoff.
///
/// Retryable: LLM failures where the underlying `LlmError` was classified
/// as retryable by `crate::llm::retry::is_retryable()`, empty responses,
/// and truncated responses.
/// Non-retryable: configuration errors, authorization, resource limits,
/// DB errors, and LLM failures caused by auth/content-policy/context-length.
pub fn is_retryable(&self) -> bool {
match self {
RoutineError::LlmFailed { retryable, .. } => *retryable,
RoutineError::EmptyResponse { .. } | RoutineError::TruncatedResponse { .. } => true,
_ => false,
}
}
}
/// Result type alias for the agent.
@@ -514,6 +547,99 @@ mod tests {
assert!(msg.contains("bad format"), "Should mention reason: {msg}");
}
#[test]
fn routine_error_retryable_classification() {
// Transient errors should be retryable
assert!(
RoutineError::LlmFailed {
reason: "timeout".into(),
partial_tokens: None,
retryable: true,
}
.is_retryable()
);
// Non-retryable LLM error
assert!(
!RoutineError::LlmFailed {
reason: "timeout".into(),
partial_tokens: None,
retryable: false,
}
.is_retryable()
);
assert!(
RoutineError::EmptyResponse {
partial_tokens: None
}
.is_retryable()
);
assert!(
RoutineError::TruncatedResponse {
partial_tokens: None
}
.is_retryable()
);
// Hard failures should NOT be retryable
assert!(
!RoutineError::Disabled {
name: "test".into()
}
.is_retryable()
);
assert!(
!RoutineError::JobDispatchFailed {
reason: "no docker".into()
}
.is_retryable()
);
assert!(
!RoutineError::Database {
reason: "conn refused".into()
}
.is_retryable()
);
assert!(!RoutineError::NotFound { id: Uuid::new_v4() }.is_retryable());
assert!(!RoutineError::NotAuthorized { id: Uuid::new_v4() }.is_retryable());
assert!(
!RoutineError::MaxConcurrent {
name: "test".into()
}
.is_retryable()
);
assert!(
!RoutineError::UnknownTriggerType {
trigger_type: "x".into()
}
.is_retryable()
);
assert!(
!RoutineError::UnknownActionType {
action_type: "x".into()
}
.is_retryable()
);
assert!(
!RoutineError::MissingField {
context: "c".into(),
field: "f".into()
}
.is_retryable()
);
assert!(
!RoutineError::InvalidCron {
reason: "bad".into()
}
.is_retryable()
);
assert!(
!RoutineError::UnknownRunStatus {
status: "bad".into()
}
.is_retryable()
);
}
#[test]
fn top_level_error_from_conversions() {
let config_err = ConfigError::MissingEnvVar("TEST".to_string());