Skip to content

Commit 03c07c9

Browse files
factorydroidFactory Bot
authored andcommitted
fix(cortex-exec): disable automatic retries by default to prevent duplicate side effects
Fixes bounty issue #1519 The ExecRunner's send_request function had automatic retry logic (MAX_RETRIES=3) for transient LLM errors. This could cause non-idempotent operations like tool calls with side effects to be executed multiple times during retries. Changes: - Add max_retries field to ExecOptions, defaulting to 0 (no retries) - Update send_request to use configurable max_retries instead of hardcoded value - Only retry when max_retries > 0 and error is retriable - Add test coverage for max_retries configuration Users who want retry behavior for idempotent prompts can explicitly set max_retries to a positive value.
1 parent ee89f3c commit 03c07c9

File tree

1 file changed

+44
-7
lines changed

1 file changed

+44
-7
lines changed

cortex-exec/src/runner.rs

Lines changed: 44 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,11 @@ pub struct ExecOptions {
6464
pub enabled_tools: Option<Vec<String>>,
6565
/// Tools to disable.
6666
pub disabled_tools: Vec<String>,
67+
/// Maximum number of retries for transient LLM request errors.
68+
/// Defaults to 0 (no retries) to prevent duplicate execution of
69+
/// non-idempotent operations like tool calls with side effects.
70+
/// Set to a positive value (e.g., 3) only for read-only prompts.
71+
pub max_retries: usize,
6772
}
6873

6974
impl Default for ExecOptions {
@@ -82,6 +87,8 @@ impl Default for ExecOptions {
8287
streaming: true,
8388
enabled_tools: None,
8489
disabled_tools: Vec::new(),
90+
// Default to 0 retries to prevent duplicate execution of non-idempotent operations
91+
max_retries: 0,
8592
}
8693
}
8794
}
@@ -465,7 +472,11 @@ impl ExecRunner {
465472
})
466473
}
467474

468-
/// Send a request to the LLM with retry logic.
475+
/// Send a request to the LLM with optional retry logic.
476+
///
477+
/// The number of retries is controlled by `self.options.max_retries`.
478+
/// When set to 0 (the default), no retries are attempted to prevent
479+
/// duplicate execution of non-idempotent operations like tool calls.
469480
async fn send_request(
470481
&self,
471482
conversation: &ConversationManager,
@@ -491,14 +502,19 @@ impl ExecRunner {
491502
.unwrap_or(DEFAULT_REQUEST_TIMEOUT_SECS),
492503
);
493504

494-
// Retry loop for transient errors
505+
// Total attempts = 1 (initial) + max_retries
506+
// When max_retries is 0, we make exactly 1 attempt with no retries
507+
let max_attempts = self.options.max_retries + 1;
508+
509+
// Retry loop for transient errors (disabled by default to prevent
510+
// duplicate execution of non-idempotent operations)
495511
let mut last_error = None;
496-
for attempt in 0..MAX_RETRIES {
512+
for attempt in 0..max_attempts {
497513
if attempt > 0 {
498514
self.output.write_info(&format!(
499515
"Retrying request (attempt {}/{})",
500516
attempt + 1,
501-
MAX_RETRIES
517+
max_attempts
502518
));
503519
// Exponential backoff
504520
tokio::time::sleep(Duration::from_millis(500 * 2u64.pow(attempt as u32))).await;
@@ -517,7 +533,8 @@ impl ExecRunner {
517533
match result {
518534
Ok(Ok(response)) => return Ok(response),
519535
Ok(Err(e)) => {
520-
if e.is_retriable() {
536+
// Only retry if retries are enabled and error is retriable
537+
if self.options.max_retries > 0 && e.is_retriable() {
521538
last_error = Some(e);
522539
continue;
523540
}
@@ -528,8 +545,12 @@ impl ExecRunner {
528545
"Request timed out after {} seconds",
529546
request_timeout.as_secs()
530547
));
531-
last_error = Some(CortexError::Timeout);
532-
continue;
548+
// Only retry on timeout if retries are enabled
549+
if self.options.max_retries > 0 {
550+
last_error = Some(CortexError::Timeout);
551+
continue;
552+
}
553+
return Err(CortexError::Timeout);
533554
}
534555
}
535556
}
@@ -768,6 +789,22 @@ mod tests {
768789
assert_eq!(opts.timeout_secs, Some(DEFAULT_TIMEOUT_SECS));
769790
assert!(!opts.full_auto);
770791
assert!(opts.streaming);
792+
// max_retries defaults to 0 to prevent duplicate execution of non-idempotent operations
793+
assert_eq!(opts.max_retries, 0);
794+
}
795+
796+
#[test]
797+
fn test_max_retries_configuration() {
798+
// Default should be 0 (no retries) to protect against duplicate side effects
799+
let default_opts = ExecOptions::default();
800+
assert_eq!(default_opts.max_retries, 0);
801+
802+
// Users can explicitly enable retries if they know their prompt is idempotent
803+
let opts_with_retries = ExecOptions {
804+
max_retries: 3,
805+
..Default::default()
806+
};
807+
assert_eq!(opts_with_retries.max_retries, 3);
771808
}
772809

773810
#[test]

0 commit comments

Comments
 (0)