Skip to content
92 changes: 75 additions & 17 deletions crates/goose/src/agents/agent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,9 @@ const DEFAULT_MAX_TURNS: u32 = 1000;
const DEFAULT_STOP_HOOK_BLOCK_CAP: u32 = 8;
const COMPACTION_PROGRESS_TEXT: &str = "goose is compacting the conversation...";
const MAX_TURNS_MESSAGE: &str = "I've reached the maximum number of actions I can do without user input. Would you like me to continue?";
const MAX_EMPTY_TURN_RETRIES: u32 = 3;
const EMPTY_TURN_MESSAGE: &str =
"The model returned an empty response. Please resend your message to continue.";
const DEFAULT_FRONTEND_INSTRUCTIONS: &str = "The following tools are provided directly by the frontend and will be executed by the frontend when called.";

#[derive(Debug, Clone, Copy, PartialEq, Eq)]
Expand Down Expand Up @@ -687,23 +690,15 @@ impl Agent {
messages: &mut Conversation,
session_config: &SessionConfig,
initial_messages: &[Message],
) -> Result<bool> {
let result = self
.retry_manager
) -> Result<RetryResult> {
self.retry_manager
.handle_retry_logic(
messages,
session_config,
initial_messages,
&self.final_output_tool,
)
.await?;

match result {
RetryResult::Retried => Ok(true),
RetryResult::Skipped
| RetryResult::MaxAttemptsReached
| RetryResult::SuccessChecksPassed => Ok(false),
}
.await
}
async fn load_project_instructions(&self, session: &Session) -> Option<String> {
let project_id = session.project_id.as_deref()?;
Expand Down Expand Up @@ -1908,6 +1903,8 @@ impl Agent {
.unwrap_or(DEFAULT_MAX_TURNS)
});
let mut compaction_attempts = 0;
let mut empty_turn_retries = 0u32;
let mut retrying_after_empty_turn = false;
let mut last_assistant_text = String::new();
let mut goal_check_pending = false;
let mut tool_pair_summarization_done = false;
Expand Down Expand Up @@ -1984,6 +1981,8 @@ impl Agent {

if retrying_after_stop_hook_denial {
retrying_after_stop_hook_denial = false;
} else if retrying_after_empty_turn {
retrying_after_empty_turn = false;
} else {
turns_taken += 1;
}
Expand Down Expand Up @@ -2036,6 +2035,7 @@ impl Agent {
let mut tools_updated = false;
let mut did_recovery_compact_this_iteration = false;
let mut exit_chat = false;
let mut provider_errored = false;
let mut pending_final_output: Option<String> = None;
let mut pending_turn_usage: Option<ProviderUsage> = None;

Expand Down Expand Up @@ -2411,6 +2411,7 @@ impl Agent {
}
#[allow(unused_variables)]
Err(ref provider_err @ ProviderError::ContextLengthExceeded(_)) => {
provider_errored = true;
#[cfg(feature = "telemetry")]
crate::posthog::emit_error(provider_err.telemetry_type(), &provider_err.to_string());
compaction_attempts += 1;
Expand Down Expand Up @@ -2470,6 +2471,7 @@ impl Agent {
}
}
Err(ref provider_err @ ProviderError::CreditsExhausted { details: _, ref top_up_url }) => {
provider_errored = true;
#[cfg(feature = "telemetry")]
crate::posthog::emit_error(provider_err.telemetry_type(), &provider_err.to_string());
error!("Error: {}", provider_err);
Expand All @@ -2494,6 +2496,7 @@ impl Agent {
break;
}
Err(ref provider_err @ ProviderError::Refusal { ref details, ref category }) => {
provider_errored = true;
#[cfg(feature = "telemetry")]
crate::posthog::emit_error(provider_err.telemetry_type(), &provider_err.to_string());
error!("Error: {}", provider_err);
Expand All @@ -2509,6 +2512,7 @@ impl Agent {
break;
}
Err(ref provider_err @ ProviderError::NetworkError(_)) => {
provider_errored = true;
#[cfg(feature = "telemetry")]
crate::posthog::emit_error(provider_err.telemetry_type(), &provider_err.to_string());
error!("Error: {}", provider_err);
Expand All @@ -2520,6 +2524,7 @@ impl Agent {
break;
}
Err(ref provider_err) => {
provider_errored = true;
#[cfg(feature = "telemetry")]
crate::posthog::emit_error(provider_err.telemetry_type(), &provider_err.to_string());
error!("Error: {}", provider_err);
Expand Down Expand Up @@ -2551,6 +2556,24 @@ impl Agent {
}
}

// An empty provider response — no tool calls, no text, and no error
// or recovery compaction that legitimately produces no assistant
// output — must never be persisted: strict providers reject a
// conversation that contains an empty assistant turn. Drop it here
// regardless of what the match below decides to do about the turn
// (final-output nudge, steer, goal/grind, retry, or fallback).
let empty_response = no_tools_called
&& !exit_chat
&& !provider_errored
&& !did_recovery_compact_this_iteration
&& last_assistant_text.is_empty();

if empty_response {
messages_to_add = Conversation::default();
} else {
empty_turn_retries = 0;
}

if no_tools_called && !exit_chat {
// Lock, extract state, drop guard before branching — handle_retry_logic
// also locks final_output_tool and tokio::sync::Mutex is not reentrant.
Expand Down Expand Up @@ -2614,17 +2637,52 @@ impl Agent {
None => {
self.set_goal(None).await;
self.set_grind(None).await;
// Recipe retry logic owns the turn whenever a
// retry_config is present: it runs success checks,
// on_failure, and max_retries. Only when no recipe
// retry is configured (Skipped) does the empty-turn
// fallback apply.
match self.handle_retry_logic(&mut conversation, &session_config, &initial_messages).await {
Ok(should_retry) => {
if should_retry {
info!("Retry logic triggered, restarting agent loop");
messages_to_add = Conversation::default();
session_manager.replace_conversation(&session_config.id, &conversation).await?;
yield AgentEvent::HistoryReplaced(conversation.clone());
Ok(RetryResult::Retried) => {
info!("Retry logic triggered, restarting agent loop");
messages_to_add = Conversation::default();
session_manager.replace_conversation(&session_config.id, &conversation).await?;
yield AgentEvent::HistoryReplaced(conversation.clone());
}
Ok(RetryResult::Skipped) if empty_response => {
// No recipe retry configured, and this empty
// turn would otherwise fall through to a
// silent exit. Retry a bounded number of
// times, then surface a visible message so
// the user is never left with no response.
if empty_turn_retries < MAX_EMPTY_TURN_RETRIES {
empty_turn_retries += 1;
retrying_after_empty_turn = true;
warn!(
"Provider returned an empty response; retrying ({}/{})",
empty_turn_retries, MAX_EMPTY_TURN_RETRIES
);
} else {
warn!("Provider returned an empty response after retries; ending turn");
last_assistant_text = EMPTY_TURN_MESSAGE.to_string();
let message = Message::assistant().with_text(EMPTY_TURN_MESSAGE);
messages_to_add.push(message.clone());
yield AgentEvent::Message(message);
exit_chat = true;
}
}
Ok(RetryResult::MaxAttemptsReached(message)) => {
// Surface and persist the failure message
// through the normal path so recipes don't
// exit silently when retries are exhausted.
last_assistant_text = message.as_concat_text();
messages_to_add.push(message.clone());
yield AgentEvent::Message(message);
exit_chat = true;
}
Ok(_) => {
exit_chat = true;
Comment thread
kojiromike marked this conversation as resolved.
}
Err(e) => {
error!("Retry logic failed: {}", e);
yield AgentEvent::Message(
Expand Down
20 changes: 9 additions & 11 deletions crates/goose/src/agents/retry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,9 @@ use crate::tool_monitor::RepetitionInspector;
pub enum RetryResult {
/// No retry configuration or session available, retry logic skipped
Skipped,
/// Maximum retry attempts reached, cannot retry further
MaxAttemptsReached,
/// Maximum retry attempts reached, cannot retry further. Carries the
/// user-facing failure message so the caller can yield and persist it.
MaxAttemptsReached(Message),
/// Success checks passed, no retry needed
SuccessChecksPassed,
/// Retry is needed and will be performed
Expand Down Expand Up @@ -135,7 +136,6 @@ impl RetryManager {
"Maximum retry attempts ({}) exceeded. Unable to complete the task successfully.",
retry_config.max_retries
));
messages.push(error_msg);
warn!(
"Maximum retry attempts ({}) exceeded",
retry_config.max_retries
Expand All @@ -145,7 +145,7 @@ impl RetryManager {
"retry_max_exceeded",
&format!("Max retries ({}) exceeded", retry_config.max_retries),
);
return Ok(RetryResult::MaxAttemptsReached);
return Ok(RetryResult::MaxAttemptsReached(error_msg));
}

if let Some(on_failure_cmd) = &retry_config.on_failure {
Expand Down Expand Up @@ -335,21 +335,19 @@ mod tests {

#[test]
fn test_retry_result_enum() {
assert_ne!(RetryResult::Skipped, RetryResult::MaxAttemptsReached);
let max_attempts = RetryResult::MaxAttemptsReached(Message::assistant().with_text("done"));
assert_ne!(RetryResult::Skipped, max_attempts);
assert_ne!(RetryResult::Skipped, RetryResult::SuccessChecksPassed);
assert_ne!(RetryResult::Skipped, RetryResult::Retried);
assert_ne!(
RetryResult::MaxAttemptsReached,
RetryResult::SuccessChecksPassed
);
assert_ne!(RetryResult::MaxAttemptsReached, RetryResult::Retried);
assert_ne!(max_attempts, RetryResult::SuccessChecksPassed);
assert_ne!(max_attempts, RetryResult::Retried);
assert_ne!(RetryResult::SuccessChecksPassed, RetryResult::Retried);

let result = RetryResult::Retried;
let cloned = result.clone();
assert_eq!(result, cloned);

let debug_str = format!("{:?}", RetryResult::MaxAttemptsReached);
let debug_str = format!("{:?}", max_attempts);
assert!(debug_str.contains("MaxAttemptsReached"));
}

Expand Down
Loading
Loading