mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-25 14:53:34 +00:00
Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
d0a23ab41c | ||
|
|
19dcaad6cf | ||
|
|
6ef8bc28eb |
@@ -10,7 +10,9 @@ use std::borrow::Cow;
|
||||
|
||||
use crate::agent::session::PendingApproval;
|
||||
use crate::error::Error;
|
||||
use crate::llm::{ChatMessage, FinishReason, Reasoning, ReasoningContext, RespondResult};
|
||||
use crate::llm::{
|
||||
ChatMessage, FinishReason, Reasoning, ReasoningContext, RespondResult, ResponseMetadata,
|
||||
};
|
||||
|
||||
/// Signal from the delegate indicating how the loop should proceed.
|
||||
pub enum LoopSignal {
|
||||
@@ -38,6 +40,8 @@ pub enum LoopOutcome {
|
||||
Stopped,
|
||||
/// Max iterations exceeded.
|
||||
MaxIterations,
|
||||
/// Loop terminated early with a clear failure reason.
|
||||
Failure(String),
|
||||
/// A tool requires user approval before continuing (chat delegate only).
|
||||
NeedApproval(Box<PendingApproval>),
|
||||
}
|
||||
@@ -103,6 +107,7 @@ pub trait LoopDelegate: Send + Sync {
|
||||
async fn handle_text_response(
|
||||
&self,
|
||||
text: &str,
|
||||
metadata: ResponseMetadata,
|
||||
reason_ctx: &mut ReasoningContext,
|
||||
) -> TextAction;
|
||||
|
||||
@@ -209,7 +214,10 @@ pub async fn run_agentic_loop(
|
||||
consecutive_tool_intent_nudges = 0;
|
||||
}
|
||||
|
||||
match delegate.handle_text_response(&text, reason_ctx).await {
|
||||
match delegate
|
||||
.handle_text_response(&text, output.metadata, reason_ctx)
|
||||
.await
|
||||
{
|
||||
TextAction::Return(outcome) => return Ok(outcome),
|
||||
TextAction::Continue => {}
|
||||
}
|
||||
@@ -279,7 +287,7 @@ pub fn truncate_for_preview(s: &str, max: usize) -> Cow<'_, str> {
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::llm::{RespondOutput, TokenUsage, ToolCall};
|
||||
use crate::llm::{RespondOutput, ResponseAnomaly, ResponseMetadata, TokenUsage, ToolCall};
|
||||
use crate::testing::StubLlm;
|
||||
use std::sync::Arc;
|
||||
use std::sync::atomic::{AtomicUsize, Ordering};
|
||||
@@ -303,6 +311,7 @@ mod tests {
|
||||
result: RespondResult::Text(text.to_string()),
|
||||
usage: zero_usage(),
|
||||
finish_reason: FinishReason::Stop,
|
||||
metadata: ResponseMetadata::default(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -314,6 +323,7 @@ mod tests {
|
||||
},
|
||||
usage: zero_usage(),
|
||||
finish_reason: FinishReason::ToolUse,
|
||||
metadata: ResponseMetadata::default(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -391,6 +401,7 @@ mod tests {
|
||||
async fn handle_text_response(
|
||||
&self,
|
||||
text: &str,
|
||||
_metadata: ResponseMetadata,
|
||||
_reason_ctx: &mut ReasoningContext,
|
||||
) -> TextAction {
|
||||
TextAction::Return(LoopOutcome::Response(text.to_string()))
|
||||
@@ -508,6 +519,79 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_text_response_metadata_can_fail_fast() {
|
||||
struct FailOnMalformedResponse;
|
||||
|
||||
#[async_trait]
|
||||
impl LoopDelegate for FailOnMalformedResponse {
|
||||
async fn check_signals(&self) -> LoopSignal {
|
||||
LoopSignal::Continue
|
||||
}
|
||||
|
||||
async fn before_llm_call(
|
||||
&self,
|
||||
_: &mut ReasoningContext,
|
||||
_: usize,
|
||||
) -> Option<LoopOutcome> {
|
||||
None
|
||||
}
|
||||
|
||||
async fn call_llm(
|
||||
&self,
|
||||
_: &Reasoning,
|
||||
_: &mut ReasoningContext,
|
||||
_: usize,
|
||||
) -> Result<crate::llm::RespondOutput, crate::error::Error> {
|
||||
Ok(RespondOutput {
|
||||
result: RespondResult::Text("fallback".to_string()),
|
||||
usage: zero_usage(),
|
||||
finish_reason: FinishReason::Stop,
|
||||
metadata: ResponseMetadata {
|
||||
anomaly: Some(ResponseAnomaly::EmptyToolCompletion),
|
||||
},
|
||||
})
|
||||
}
|
||||
|
||||
async fn handle_text_response(
|
||||
&self,
|
||||
_: &str,
|
||||
metadata: ResponseMetadata,
|
||||
_: &mut ReasoningContext,
|
||||
) -> TextAction {
|
||||
assert_eq!(metadata.anomaly, Some(ResponseAnomaly::EmptyToolCompletion));
|
||||
TextAction::Return(LoopOutcome::Failure(
|
||||
"malformed tool completion".to_string(),
|
||||
))
|
||||
}
|
||||
|
||||
async fn execute_tool_calls(
|
||||
&self,
|
||||
_: Vec<ToolCall>,
|
||||
_: Option<String>,
|
||||
_: &mut ReasoningContext,
|
||||
) -> Result<Option<LoopOutcome>, crate::error::Error> {
|
||||
Ok(None)
|
||||
}
|
||||
}
|
||||
|
||||
let delegate = FailOnMalformedResponse;
|
||||
let reasoning = stub_reasoning();
|
||||
let mut ctx = ReasoningContext::new();
|
||||
let outcome = run_agentic_loop(
|
||||
&delegate,
|
||||
&reasoning,
|
||||
&mut ctx,
|
||||
&AgenticLoopConfig::default(),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
assert!(
|
||||
matches!(outcome, LoopOutcome::Failure(ref reason) if reason == "malformed tool completion")
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_max_iterations_reached() {
|
||||
struct ContinueDelegate;
|
||||
@@ -535,6 +619,7 @@ mod tests {
|
||||
async fn handle_text_response(
|
||||
&self,
|
||||
_: &str,
|
||||
_: ResponseMetadata,
|
||||
ctx: &mut ReasoningContext,
|
||||
) -> TextAction {
|
||||
ctx.messages.push(ChatMessage::assistant("still working"));
|
||||
@@ -671,6 +756,7 @@ mod tests {
|
||||
},
|
||||
usage: zero_usage(),
|
||||
finish_reason: FinishReason::Length, // response was truncated
|
||||
metadata: ResponseMetadata::default(),
|
||||
};
|
||||
let delegate = MockDelegate::new(vec![truncated_output, text_output("Summarized it.")]);
|
||||
let reasoning = stub_reasoning();
|
||||
@@ -719,6 +805,7 @@ mod tests {
|
||||
},
|
||||
usage: zero_usage(),
|
||||
finish_reason: FinishReason::Length,
|
||||
metadata: ResponseMetadata::default(),
|
||||
};
|
||||
// Three truncated responses, then a text response
|
||||
let delegate = MockDelegate::new(vec![
|
||||
|
||||
@@ -219,6 +219,11 @@ impl Agent {
|
||||
reason: format!("Exceeded maximum tool iterations ({max_tool_iterations})"),
|
||||
}
|
||||
.into()),
|
||||
LoopOutcome::Failure(reason) => Err(crate::error::LlmError::InvalidResponse {
|
||||
provider: "agent".to_string(),
|
||||
reason,
|
||||
}
|
||||
.into()),
|
||||
LoopOutcome::NeedApproval(pending) => Ok(AgenticLoopResult::NeedApproval { pending }),
|
||||
}
|
||||
}
|
||||
@@ -439,6 +444,7 @@ impl<'a> LoopDelegate for ChatDelegate<'a> {
|
||||
async fn handle_text_response(
|
||||
&self,
|
||||
text: &str,
|
||||
_metadata: crate::llm::ResponseMetadata,
|
||||
_reason_ctx: &mut ReasoningContext,
|
||||
) -> TextAction {
|
||||
// Strip internal "[Called tool ...]" text that can leak when
|
||||
|
||||
@@ -14,7 +14,6 @@ use uuid::Uuid;
|
||||
use crate::channels::web::auth::AuthenticatedUser;
|
||||
use crate::channels::web::server::GatewayState;
|
||||
use crate::channels::web::types::*;
|
||||
use crate::channels::web::util::{sanitized_db_error, sanitized_internal_error_response};
|
||||
|
||||
pub async fn jobs_list_handler(
|
||||
State(state): State<Arc<GatewayState>>,
|
||||
@@ -214,7 +213,10 @@ pub async fn jobs_detail_handler(
|
||||
}
|
||||
Ok(None) => {}
|
||||
Err(e) => {
|
||||
return Err(sanitized_db_error(e, "get sandbox job detail"));
|
||||
return Err((
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
format!("Database error: {}", e),
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -255,7 +257,10 @@ pub async fn jobs_detail_handler(
|
||||
}))
|
||||
}
|
||||
Ok(None) => Err((StatusCode::NOT_FOUND, "Job not found".to_string())),
|
||||
Err(e) => Err(sanitized_db_error(e, "get agent job detail")),
|
||||
Err(e) => Err((
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
format!("Database error: {}", e),
|
||||
)),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -290,7 +295,7 @@ pub async fn jobs_cancel_handler(
|
||||
Some(chrono::Utc::now()),
|
||||
)
|
||||
.await
|
||||
.map_err(|e| sanitized_db_error(e, "persist sandbox job cancellation"))?;
|
||||
.map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?;
|
||||
}
|
||||
return Ok(Json(serde_json::json!({
|
||||
"status": "cancelled",
|
||||
@@ -299,7 +304,10 @@ pub async fn jobs_cancel_handler(
|
||||
}
|
||||
Ok(None) => {}
|
||||
Err(e) => {
|
||||
return Err(sanitized_db_error(e, "get sandbox job for cancellation"));
|
||||
return Err((
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
format!("Database error: {}", e),
|
||||
));
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -333,7 +341,7 @@ pub async fn jobs_cancel_handler(
|
||||
Some("Cancelled by user"),
|
||||
)
|
||||
.await
|
||||
.map_err(|e| sanitized_db_error(e, "persist agent job cancellation"))?;
|
||||
.map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?;
|
||||
}
|
||||
return Ok(Json(serde_json::json!({
|
||||
"status": "cancelled",
|
||||
@@ -342,7 +350,10 @@ pub async fn jobs_cancel_handler(
|
||||
}
|
||||
Ok(None) => {}
|
||||
Err(e) => {
|
||||
return Err(sanitized_db_error(e, "get agent job for cancellation"));
|
||||
return Err((
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
format!("Database error: {}", e),
|
||||
));
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -410,7 +421,7 @@ pub async fn jobs_restart_handler(
|
||||
store
|
||||
.save_sandbox_job(&record)
|
||||
.await
|
||||
.map_err(|e| sanitized_db_error(e, "persist restarted sandbox job"))?;
|
||||
.map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?;
|
||||
|
||||
let mode = match store.get_sandbox_job_mode(old_job_id).await {
|
||||
Ok(Some(m)) if m == "claude_code" => {
|
||||
@@ -441,13 +452,16 @@ pub async fn jobs_restart_handler(
|
||||
)
|
||||
.await
|
||||
.map_err(|e| {
|
||||
sanitized_internal_error_response(e, "create restarted sandbox container")
|
||||
(
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
format!("Failed to create container: {}", e),
|
||||
)
|
||||
})?;
|
||||
|
||||
store
|
||||
.update_sandbox_job_status(new_job_id, "running", None, None, Some(now), None)
|
||||
.await
|
||||
.map_err(|e| sanitized_db_error(e, "mark restarted sandbox job running"))?;
|
||||
.map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?;
|
||||
|
||||
return Ok(Json(serde_json::json!({
|
||||
"status": "restarted",
|
||||
@@ -457,7 +471,10 @@ pub async fn jobs_restart_handler(
|
||||
}
|
||||
Ok(None) => {}
|
||||
Err(e) => {
|
||||
return Err(sanitized_db_error(e, "get sandbox job for restart"));
|
||||
return Err((
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
format!("Database error: {}", e),
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -504,9 +521,7 @@ pub async fn jobs_restart_handler(
|
||||
let new_job_id = scheduler
|
||||
.dispatch_job(&old_job.user_id, &title, &old_job.description, None)
|
||||
.await
|
||||
.map_err(|e| {
|
||||
sanitized_internal_error_response(e, "dispatch restarted agent job")
|
||||
})?;
|
||||
.map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?;
|
||||
|
||||
Ok(Json(serde_json::json!({
|
||||
"status": "restarted",
|
||||
@@ -515,7 +530,10 @@ pub async fn jobs_restart_handler(
|
||||
})))
|
||||
}
|
||||
Ok(None) => Err((StatusCode::NOT_FOUND, "Job not found".to_string())),
|
||||
Err(e) => Err(sanitized_db_error(e, "get agent job for restart")),
|
||||
Err(e) => Err((
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
format!("Database error: {}", e),
|
||||
)),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -591,7 +609,10 @@ pub async fn jobs_prompt_handler(
|
||||
return Err((StatusCode::NOT_FOUND, "Job not found".to_string()));
|
||||
}
|
||||
Err(e) => {
|
||||
return Err(sanitized_db_error(e, "get agent job for prompt"));
|
||||
return Err((
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
format!("Database error: {}", e),
|
||||
));
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -604,9 +625,10 @@ pub async fn jobs_prompt_handler(
|
||||
if let Some(ref scheduler) = *scheduler_guard
|
||||
&& scheduler.is_running(job_id).await
|
||||
{
|
||||
scheduler.send_message(job_id, content).await.map_err(|e| {
|
||||
sanitized_internal_error_response(e, "send prompt to running agent job")
|
||||
})?;
|
||||
scheduler
|
||||
.send_message(job_id, content)
|
||||
.await
|
||||
.map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?;
|
||||
return Ok(Json(serde_json::json!({
|
||||
"status": "sent",
|
||||
"job_id": job_id.to_string(),
|
||||
@@ -645,14 +667,17 @@ pub async fn jobs_events_handler(
|
||||
return Err((StatusCode::NOT_FOUND, "Job not found".to_string()));
|
||||
}
|
||||
Err(e) => {
|
||||
return Err(sanitized_db_error(e, "get sandbox job events"));
|
||||
return Err((
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
format!("Database error: {}", e),
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
let events = store
|
||||
.list_job_events(job_id, None)
|
||||
.await
|
||||
.map_err(|e| sanitized_db_error(e, "list job events"))?;
|
||||
.map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?;
|
||||
|
||||
let events_json: Vec<serde_json::Value> = events
|
||||
.into_iter()
|
||||
@@ -696,7 +721,7 @@ pub async fn job_files_list_handler(
|
||||
let job = store
|
||||
.get_sandbox_job(job_id)
|
||||
.await
|
||||
.map_err(|e| sanitized_db_error(e, "get sandbox job file list"))?
|
||||
.map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?
|
||||
.ok_or((StatusCode::NOT_FOUND, "Job not found".to_string()))?;
|
||||
|
||||
if job.user_id != user.user_id {
|
||||
@@ -764,7 +789,7 @@ pub async fn job_files_read_handler(
|
||||
let job = store
|
||||
.get_sandbox_job(job_id)
|
||||
.await
|
||||
.map_err(|e| sanitized_db_error(e, "get sandbox job file read"))?
|
||||
.map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?
|
||||
.ok_or((StatusCode::NOT_FOUND, "Job not found".to_string()))?;
|
||||
|
||||
if job.user_id != user.user_id {
|
||||
|
||||
@@ -14,7 +14,7 @@ use crate::agent::routine::{Trigger, next_cron_fire};
|
||||
use crate::channels::web::auth::AuthenticatedUser;
|
||||
use crate::channels::web::server::GatewayState;
|
||||
use crate::channels::web::types::*;
|
||||
use crate::channels::web::util::{sanitized_db_error, sanitized_routine_error};
|
||||
use crate::error::RoutineError;
|
||||
|
||||
pub async fn routines_list_handler(
|
||||
State(state): State<Arc<GatewayState>>,
|
||||
@@ -28,7 +28,7 @@ pub async fn routines_list_handler(
|
||||
let routines = store
|
||||
.list_routines(&user.user_id)
|
||||
.await
|
||||
.map_err(|e| sanitized_db_error(e, "list routines"))?;
|
||||
.map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?;
|
||||
|
||||
let items: Vec<RoutineInfo> = routines.iter().map(RoutineInfo::from_routine).collect();
|
||||
|
||||
@@ -47,7 +47,7 @@ pub async fn routines_summary_handler(
|
||||
let routines = store
|
||||
.list_routines(&user.user_id)
|
||||
.await
|
||||
.map_err(|e| sanitized_db_error(e, "list routines summary"))?;
|
||||
.map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?;
|
||||
|
||||
let total = routines.len() as u64;
|
||||
let enabled = routines.iter().filter(|r| r.enabled).count() as u64;
|
||||
@@ -95,7 +95,7 @@ pub async fn routines_detail_handler(
|
||||
let routine = store
|
||||
.get_routine(routine_id)
|
||||
.await
|
||||
.map_err(|e| sanitized_db_error(e, "get routine detail"))?
|
||||
.map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?
|
||||
.ok_or((StatusCode::NOT_FOUND, "Routine not found".to_string()))?;
|
||||
|
||||
if routine.user_id != user.user_id {
|
||||
@@ -105,7 +105,7 @@ pub async fn routines_detail_handler(
|
||||
let runs = store
|
||||
.list_routine_runs(routine_id, 20)
|
||||
.await
|
||||
.map_err(|e| sanitized_db_error(e, "list routine detail runs"))?;
|
||||
.map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?;
|
||||
|
||||
let recent_runs: Vec<RoutineRunInfo> = runs
|
||||
.iter()
|
||||
@@ -163,7 +163,7 @@ pub async fn routines_trigger_handler(
|
||||
let run_id = engine
|
||||
.fire_manual(routine_id, Some(&user.user_id))
|
||||
.await
|
||||
.map_err(|e| sanitized_routine_error(e, "trigger routine manually"))?;
|
||||
.map_err(|e| (routine_error_status(&e), e.to_string()))?;
|
||||
|
||||
Ok(Json(serde_json::json!({
|
||||
"status": "triggered",
|
||||
@@ -194,7 +194,7 @@ pub async fn routines_toggle_handler(
|
||||
let mut routine = store
|
||||
.get_routine(routine_id)
|
||||
.await
|
||||
.map_err(|e| sanitized_db_error(e, "get routine for toggle"))?
|
||||
.map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?
|
||||
.ok_or((StatusCode::NOT_FOUND, "Routine not found".to_string()))?;
|
||||
|
||||
if routine.user_id != user.user_id {
|
||||
@@ -228,7 +228,7 @@ pub async fn routines_toggle_handler(
|
||||
store
|
||||
.update_routine(&routine)
|
||||
.await
|
||||
.map_err(|e| sanitized_db_error(e, "update routine toggle state"))?;
|
||||
.map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?;
|
||||
|
||||
// Refresh the in-memory event trigger cache so event/system_event
|
||||
// routines reflect the new enabled state immediately (issue #1076).
|
||||
@@ -259,7 +259,7 @@ pub async fn routines_delete_handler(
|
||||
let routine = store
|
||||
.get_routine(routine_id)
|
||||
.await
|
||||
.map_err(|e| sanitized_db_error(e, "get routine for delete"))?
|
||||
.map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?
|
||||
.ok_or((StatusCode::NOT_FOUND, "Routine not found".to_string()))?;
|
||||
|
||||
if routine.user_id != user.user_id {
|
||||
@@ -269,7 +269,7 @@ pub async fn routines_delete_handler(
|
||||
let deleted = store
|
||||
.delete_routine(routine_id)
|
||||
.await
|
||||
.map_err(|e| sanitized_db_error(e, "delete routine"))?;
|
||||
.map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?;
|
||||
|
||||
if deleted {
|
||||
// Refresh the in-memory event trigger cache so deleted event/system_event
|
||||
@@ -305,7 +305,7 @@ pub async fn routines_runs_handler(
|
||||
let routine = store
|
||||
.get_routine(routine_id)
|
||||
.await
|
||||
.map_err(|e| sanitized_db_error(e, "get routine runs"))?
|
||||
.map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?
|
||||
.ok_or((StatusCode::NOT_FOUND, "Routine not found".to_string()))?;
|
||||
|
||||
if routine.user_id != user.user_id {
|
||||
@@ -315,7 +315,7 @@ pub async fn routines_runs_handler(
|
||||
let runs = store
|
||||
.list_routine_runs(routine_id, 50)
|
||||
.await
|
||||
.map_err(|e| sanitized_db_error(e, "list routine runs"))?;
|
||||
.map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?;
|
||||
|
||||
let run_infos: Vec<RoutineRunInfo> = runs
|
||||
.iter()
|
||||
@@ -336,3 +336,15 @@ pub async fn routines_runs_handler(
|
||||
"runs": run_infos,
|
||||
})))
|
||||
}
|
||||
|
||||
/// Map `RoutineError` variants to appropriate HTTP status codes.
|
||||
fn routine_error_status(err: &RoutineError) -> StatusCode {
|
||||
match err {
|
||||
RoutineError::NotFound { .. } => StatusCode::NOT_FOUND,
|
||||
RoutineError::NotAuthorized { .. } => StatusCode::FORBIDDEN,
|
||||
RoutineError::Disabled { .. }
|
||||
| RoutineError::Cooldown { .. }
|
||||
| RoutineError::MaxConcurrent { .. } => StatusCode::CONFLICT,
|
||||
_ => StatusCode::INTERNAL_SERVER_ERROR,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -15,7 +15,6 @@ use subtle::ConstantTimeEq;
|
||||
|
||||
use crate::agent::routine::Trigger;
|
||||
use crate::channels::web::server::GatewayState;
|
||||
use crate::channels::web::util::{sanitized_db_error, sanitized_routine_error};
|
||||
|
||||
/// Validate the webhook secret for a routine.
|
||||
///
|
||||
@@ -104,7 +103,7 @@ async fn fire_webhook_inner(
|
||||
let routine = store
|
||||
.get_webhook_routine_by_path(path, user_id)
|
||||
.await
|
||||
.map_err(|e| sanitized_db_error(e, "get webhook routine by path"))?
|
||||
.map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?
|
||||
.ok_or((
|
||||
StatusCode::NOT_FOUND,
|
||||
"No routine matches this webhook path".to_string(),
|
||||
@@ -127,10 +126,16 @@ async fn fire_webhook_inner(
|
||||
))?
|
||||
};
|
||||
|
||||
let run_id = engine
|
||||
.fire_webhook(routine.id, path)
|
||||
.await
|
||||
.map_err(|e| sanitized_routine_error(e, "trigger routine from webhook"))?;
|
||||
let run_id = engine.fire_webhook(routine.id, path).await.map_err(|e| {
|
||||
let status = match &e {
|
||||
crate::error::RoutineError::NotFound { .. } => StatusCode::NOT_FOUND,
|
||||
crate::error::RoutineError::Disabled { .. }
|
||||
| crate::error::RoutineError::Cooldown { .. }
|
||||
| crate::error::RoutineError::MaxConcurrent { .. } => StatusCode::CONFLICT,
|
||||
_ => StatusCode::INTERNAL_SERVER_ERROR,
|
||||
};
|
||||
(status, e.to_string())
|
||||
})?;
|
||||
|
||||
Ok(Json(serde_json::json!({
|
||||
"status": "triggered",
|
||||
|
||||
+16
-127
@@ -1717,13 +1717,7 @@ async fn chat_history_handler(
|
||||
let (messages, has_more) = store
|
||||
.list_conversation_messages_paginated(thread_id, before_cursor, limit as i64)
|
||||
.await
|
||||
.map_err(|e| {
|
||||
tracing::error!(error = %e, "DB error listing paginated messages");
|
||||
(
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
"Database error".to_string(),
|
||||
)
|
||||
})?;
|
||||
.map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?;
|
||||
|
||||
let oldest_timestamp = messages.first().map(|m| m.created_at.to_rfc3339());
|
||||
let turns = build_turns_from_db_messages(&messages);
|
||||
@@ -1796,13 +1790,7 @@ async fn chat_history_handler(
|
||||
let (messages, has_more) = store
|
||||
.list_conversation_messages_paginated(thread_id, None, limit as i64)
|
||||
.await
|
||||
.map_err(|e| {
|
||||
tracing::error!(error = %e, "DB error listing messages");
|
||||
(
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
"Database error".to_string(),
|
||||
)
|
||||
})?;
|
||||
.map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?;
|
||||
|
||||
if !messages.is_empty() {
|
||||
let oldest_timestamp = messages.first().map(|m| m.created_at.to_rfc3339());
|
||||
@@ -1845,13 +1833,7 @@ async fn chat_threads_handler(
|
||||
let assistant_id = store
|
||||
.get_or_create_assistant_conversation(&user.user_id, "gateway")
|
||||
.await
|
||||
.map_err(|e| {
|
||||
tracing::error!(error = %e, "DB error getting assistant conversation");
|
||||
(
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
"Database error".to_string(),
|
||||
)
|
||||
})?;
|
||||
.map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?;
|
||||
|
||||
match store
|
||||
.list_conversations_all_channels(&user.user_id, 50)
|
||||
@@ -2073,13 +2055,7 @@ async fn extensions_list_handler(
|
||||
let installed = ext_mgr
|
||||
.list(None, false, &user.user_id)
|
||||
.await
|
||||
.map_err(|e| {
|
||||
tracing::error!(error = %e, "Error listing extensions");
|
||||
(
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
"Internal error".to_string(),
|
||||
)
|
||||
})?;
|
||||
.map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?;
|
||||
|
||||
let pairing_store = crate::pairing::PairingStore::new();
|
||||
let mut owner_bound_channels = std::collections::HashSet::new();
|
||||
@@ -2509,13 +2485,7 @@ async fn extensions_setup_handler(
|
||||
let setup = ext_mgr
|
||||
.get_setup_schema(&name, &user.user_id)
|
||||
.await
|
||||
.map_err(|e| {
|
||||
tracing::error!(error = %e, "Error getting extension setup schema");
|
||||
(
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
"Internal error".to_string(),
|
||||
)
|
||||
})?;
|
||||
.map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?;
|
||||
|
||||
let kind = ext_mgr
|
||||
.list(None, false, &user.user_id)
|
||||
@@ -2589,13 +2559,9 @@ async fn pairing_list_handler(
|
||||
Path(channel): Path<String>,
|
||||
) -> Result<Json<PairingListResponse>, (StatusCode, String)> {
|
||||
let store = crate::pairing::PairingStore::new();
|
||||
let requests = store.list_pending(&channel).map_err(|e| {
|
||||
tracing::error!(error = %e, "Error listing pairing requests");
|
||||
(
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
"Internal error".to_string(),
|
||||
)
|
||||
})?;
|
||||
let requests = store
|
||||
.list_pending(&channel)
|
||||
.map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?;
|
||||
|
||||
let infos = requests
|
||||
.into_iter()
|
||||
@@ -2651,26 +2617,17 @@ async fn routines_runs_handler(
|
||||
let routine = store
|
||||
.get_routine(routine_id)
|
||||
.await
|
||||
.map_err(|e| {
|
||||
tracing::error!(error = %e, "DB error getting routine");
|
||||
(
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
"Database error".to_string(),
|
||||
)
|
||||
})?
|
||||
.map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?
|
||||
.ok_or((StatusCode::NOT_FOUND, "Routine not found".to_string()))?;
|
||||
|
||||
if routine.user_id != user.user_id {
|
||||
return Err((StatusCode::NOT_FOUND, "Routine not found".to_string()));
|
||||
}
|
||||
|
||||
let runs = store.list_routine_runs(routine_id, 50).await.map_err(|e| {
|
||||
tracing::error!(error = %e, "DB error listing routine runs");
|
||||
(
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
"Database error".to_string(),
|
||||
)
|
||||
})?;
|
||||
let runs = store
|
||||
.list_routine_runs(routine_id, 50)
|
||||
.await
|
||||
.map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?;
|
||||
|
||||
let run_infos: Vec<RoutineRunInfo> = runs
|
||||
.iter()
|
||||
@@ -3068,10 +3025,8 @@ mod tests {
|
||||
|
||||
// --- OAuth callback handler tests ---
|
||||
|
||||
fn test_gateway_state_inner(
|
||||
ext_mgr: Option<Arc<ExtensionManager>>,
|
||||
store: Option<Arc<dyn crate::db::Database>>,
|
||||
) -> Arc<GatewayState> {
|
||||
/// Build a minimal `GatewayState` for testing the OAuth callback handler.
|
||||
fn test_gateway_state(ext_mgr: Option<Arc<ExtensionManager>>) -> Arc<GatewayState> {
|
||||
Arc::new(GatewayState {
|
||||
msg_tx: tokio::sync::RwLock::new(None),
|
||||
sse: Arc::new(SseManager::new()),
|
||||
@@ -3082,7 +3037,7 @@ mod tests {
|
||||
log_level_handle: None,
|
||||
extension_manager: ext_mgr,
|
||||
tool_registry: None,
|
||||
store,
|
||||
store: None,
|
||||
job_manager: None,
|
||||
prompt_queue: None,
|
||||
owner_id: "test".to_string(),
|
||||
@@ -3104,31 +3059,6 @@ mod tests {
|
||||
})
|
||||
}
|
||||
|
||||
/// Build a minimal `GatewayState` for testing the OAuth callback handler.
|
||||
fn test_gateway_state(ext_mgr: Option<Arc<ExtensionManager>>) -> Arc<GatewayState> {
|
||||
test_gateway_state_inner(ext_mgr, None)
|
||||
}
|
||||
|
||||
fn test_gateway_state_with_store(
|
||||
store: Arc<dyn crate::db::Database>,
|
||||
ext_mgr: Option<Arc<ExtensionManager>>,
|
||||
) -> Arc<GatewayState> {
|
||||
test_gateway_state_inner(ext_mgr, Some(store))
|
||||
}
|
||||
|
||||
#[cfg(feature = "libsql")]
|
||||
async fn create_unmigrated_test_db() -> (Arc<dyn crate::db::Database>, tempfile::TempDir) {
|
||||
use crate::db::libsql::LibSqlBackend;
|
||||
|
||||
let temp_dir = tempfile::tempdir().expect("tempdir");
|
||||
let db_path = temp_dir.path().join("test.db");
|
||||
let backend = LibSqlBackend::new_local(&db_path)
|
||||
.await
|
||||
.expect("LibSqlBackend");
|
||||
let db: Arc<dyn crate::db::Database> = Arc::new(backend);
|
||||
(db, temp_dir)
|
||||
}
|
||||
|
||||
/// Build a test router with just the OAuth callback route.
|
||||
fn test_oauth_router(state: Arc<GatewayState>) -> Router {
|
||||
Router::new()
|
||||
@@ -3370,47 +3300,6 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(feature = "libsql")]
|
||||
#[tokio::test]
|
||||
async fn test_routines_list_sanitizes_database_errors() {
|
||||
use axum::body::Body;
|
||||
use tower::ServiceExt;
|
||||
|
||||
let (db, _tmp) = create_unmigrated_test_db().await;
|
||||
let state = test_gateway_state_with_store(db, None);
|
||||
let app = Router::new()
|
||||
.route(
|
||||
"/api/routines",
|
||||
get(crate::channels::web::handlers::routines::routines_list_handler),
|
||||
)
|
||||
.with_state(state);
|
||||
|
||||
let mut req = axum::http::Request::builder()
|
||||
.method("GET")
|
||||
.uri("/api/routines")
|
||||
.body(Body::empty())
|
||||
.expect("request");
|
||||
req.extensions_mut().insert(UserIdentity {
|
||||
user_id: "test".to_string(),
|
||||
workspace_read_scopes: Vec::new(),
|
||||
});
|
||||
|
||||
let resp = ServiceExt::<axum::http::Request<Body>>::oneshot(app, req)
|
||||
.await
|
||||
.expect("response");
|
||||
assert_eq!(resp.status(), StatusCode::INTERNAL_SERVER_ERROR);
|
||||
|
||||
let body = axum::body::to_bytes(resp.into_body(), 1024 * 64)
|
||||
.await
|
||||
.expect("body");
|
||||
let text = String::from_utf8(body.to_vec()).expect("utf8 body");
|
||||
assert_eq!(text, "Database error");
|
||||
assert!(
|
||||
!text.contains("no such table"),
|
||||
"client response should not leak backend error details"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_extensions_setup_submit_telegram_verification_does_not_broadcast_auth_required() {
|
||||
use axum::body::Body;
|
||||
|
||||
@@ -1,9 +1,5 @@
|
||||
//! Shared utility functions for the web gateway.
|
||||
|
||||
use std::fmt::Display;
|
||||
|
||||
use axum::http::StatusCode;
|
||||
|
||||
use crate::channels::web::types::{ToolCallInfo, TurnInfo};
|
||||
|
||||
pub use ironclaw_common::truncate_preview;
|
||||
@@ -13,59 +9,6 @@ pub fn tool_error_for_display(error: &str) -> String {
|
||||
ironclaw_safety::SafetyLayer::unwrap_tool_output(error).unwrap_or_else(|| error.to_string())
|
||||
}
|
||||
|
||||
fn sanitized_internal_error<E: Display>(
|
||||
error: E,
|
||||
context: &str,
|
||||
client_message: &str,
|
||||
) -> (StatusCode, String) {
|
||||
tracing::error!(error = %error, context, "Web gateway request failed");
|
||||
(
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
client_message.to_string(),
|
||||
)
|
||||
}
|
||||
|
||||
/// Log a detailed backend error while returning a generic DB message to the client.
|
||||
pub fn sanitized_db_error<E: Display>(error: E, context: &str) -> (StatusCode, String) {
|
||||
sanitized_internal_error(error, context, "Database error")
|
||||
}
|
||||
|
||||
/// Log a detailed backend error while returning a generic internal message to the client.
|
||||
pub fn sanitized_internal_error_response<E: Display>(
|
||||
error: E,
|
||||
context: &str,
|
||||
) -> (StatusCode, String) {
|
||||
sanitized_internal_error(error, context, "Internal error")
|
||||
}
|
||||
|
||||
/// Return safe client responses for `RoutineError` while preserving user-actionable variants.
|
||||
pub fn sanitized_routine_error(
|
||||
error: crate::error::RoutineError,
|
||||
context: &str,
|
||||
) -> (StatusCode, String) {
|
||||
use crate::error::RoutineError;
|
||||
|
||||
match error {
|
||||
err @ RoutineError::NotFound { .. } => (StatusCode::NOT_FOUND, err.to_string()),
|
||||
err @ RoutineError::NotAuthorized { .. } => (StatusCode::FORBIDDEN, err.to_string()),
|
||||
err @ RoutineError::Disabled { .. }
|
||||
| err @ RoutineError::Cooldown { .. }
|
||||
| err @ RoutineError::MaxConcurrent { .. } => (StatusCode::CONFLICT, err.to_string()),
|
||||
err @ RoutineError::Database { .. } => sanitized_db_error(err, context),
|
||||
err @ RoutineError::LlmFailed { .. }
|
||||
| err @ RoutineError::JobDispatchFailed { .. }
|
||||
| err @ RoutineError::EmptyResponse
|
||||
| err @ RoutineError::TruncatedResponse
|
||||
| err @ RoutineError::UnknownTriggerType { .. }
|
||||
| err @ RoutineError::UnknownActionType { .. }
|
||||
| err @ RoutineError::MissingField { .. }
|
||||
| err @ RoutineError::InvalidCron { .. }
|
||||
| err @ RoutineError::UnknownRunStatus { .. } => {
|
||||
sanitized_internal_error_response(err, context)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Parse tool call summary JSON objects into `ToolCallInfo` structs.
|
||||
fn parse_tool_call_infos(calls: &[serde_json::Value]) -> Vec<ToolCallInfo> {
|
||||
calls
|
||||
@@ -185,30 +128,6 @@ mod tests {
|
||||
use super::*;
|
||||
use uuid::Uuid;
|
||||
|
||||
#[test]
|
||||
fn test_sanitized_db_error_hides_internal_details() {
|
||||
let (_, body) = sanitized_db_error("sqlite: no such table: routines", "list routines");
|
||||
assert_eq!(body, "Database error");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_sanitized_internal_error_hides_internal_details() {
|
||||
let (_, body) =
|
||||
sanitized_internal_error_response("container launch failed: timeout", "restart job");
|
||||
assert_eq!(body, "Internal error");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_sanitized_routine_error_hides_database_details() {
|
||||
let (_, body) = sanitized_routine_error(
|
||||
crate::error::RoutineError::Database {
|
||||
reason: "sqlite: no such table: routine_runs".to_string(),
|
||||
},
|
||||
"trigger routine",
|
||||
);
|
||||
assert_eq!(body, "Database error");
|
||||
}
|
||||
|
||||
// ---- build_turns_from_db_messages tests ----
|
||||
|
||||
fn make_msg(role: &str, content: &str, offset_ms: i64) -> crate::history::ConversationMessage {
|
||||
|
||||
+3
-3
@@ -62,9 +62,9 @@ pub use provider::{
|
||||
ToolDefinition, ToolResult, generate_tool_call_id,
|
||||
};
|
||||
pub use reasoning::{
|
||||
ActionPlan, Reasoning, ReasoningContext, RespondOutput, RespondResult, SILENT_REPLY_TOKEN,
|
||||
TOOL_INTENT_NUDGE, TRUNCATED_TOOL_CALL_NOTICE, TokenUsage, ToolSelection, is_silent_reply,
|
||||
llm_signals_tool_intent,
|
||||
ActionPlan, Reasoning, ReasoningContext, RespondOutput, RespondResult, ResponseAnomaly,
|
||||
ResponseMetadata, SILENT_REPLY_TOKEN, TOOL_INTENT_NUDGE, TRUNCATED_TOOL_CALL_NOTICE,
|
||||
TokenUsage, ToolSelection, is_silent_reply, llm_signals_tool_intent,
|
||||
};
|
||||
pub use recording::RecordingLlm;
|
||||
pub use registry::{ProviderDefinition, ProviderProtocol, ProviderRegistry};
|
||||
|
||||
+134
-5
@@ -337,6 +337,23 @@ impl TokenUsage {
|
||||
}
|
||||
}
|
||||
|
||||
/// Structured anomaly classification for LLM responses.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub enum ResponseAnomaly {
|
||||
/// Tool mode was requested, but the provider returned no usable tool calls
|
||||
/// and no recoverable text content.
|
||||
EmptyToolCompletion,
|
||||
/// Text mode returned no usable content after cleaning/truncation.
|
||||
EmptyTextResponse,
|
||||
}
|
||||
|
||||
/// Metadata attached to `RespondOutput` so callers can react to malformed
|
||||
/// provider behavior without inferring it from fallback strings.
|
||||
#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)]
|
||||
pub struct ResponseMetadata {
|
||||
pub anomaly: Option<ResponseAnomaly>,
|
||||
}
|
||||
|
||||
/// Result of a response with potential tool calls.
|
||||
///
|
||||
/// Used by the agent loop to handle tool execution before returning a final response.
|
||||
@@ -359,6 +376,7 @@ pub struct RespondOutput {
|
||||
pub result: RespondResult,
|
||||
pub usage: TokenUsage,
|
||||
pub finish_reason: FinishReason,
|
||||
pub metadata: ResponseMetadata,
|
||||
}
|
||||
|
||||
/// Reasoning engine for the agent.
|
||||
@@ -744,12 +762,11 @@ Respond in JSON format:
|
||||
},
|
||||
usage,
|
||||
finish_reason: response.finish_reason,
|
||||
metadata: ResponseMetadata::default(),
|
||||
});
|
||||
}
|
||||
|
||||
let content = response
|
||||
.content
|
||||
.unwrap_or_else(|| "I'm not sure how to respond to that.".to_string());
|
||||
let content = response.content.unwrap_or_default();
|
||||
|
||||
// Some models (e.g. GLM-4.7) emit tool calls as XML tags in content
|
||||
// instead of using the structured tool_calls field. Try to recover
|
||||
@@ -772,6 +789,7 @@ Respond in JSON format:
|
||||
},
|
||||
usage,
|
||||
finish_reason: response.finish_reason,
|
||||
metadata: ResponseMetadata::default(),
|
||||
});
|
||||
}
|
||||
|
||||
@@ -785,11 +803,18 @@ Respond in JSON format:
|
||||
// Pre-truncate at tool tags to preserve text before the tag.
|
||||
let pre_truncated = truncate_at_tool_tags(&content);
|
||||
let cleaned = clean_response(&pre_truncated);
|
||||
let final_text = if cleaned.trim().is_empty() {
|
||||
let metadata = if cleaned.trim().is_empty() {
|
||||
tracing::warn!(
|
||||
"LLM response was empty after cleaning (original len={}), using fallback",
|
||||
content.len()
|
||||
);
|
||||
ResponseMetadata {
|
||||
anomaly: Some(ResponseAnomaly::EmptyToolCompletion),
|
||||
}
|
||||
} else {
|
||||
ResponseMetadata::default()
|
||||
};
|
||||
let final_text = if metadata.anomaly.is_some() {
|
||||
"I'm not sure how to respond to that.".to_string()
|
||||
} else {
|
||||
cleaned
|
||||
@@ -798,6 +823,7 @@ Respond in JSON format:
|
||||
result: RespondResult::Text(final_text),
|
||||
usage,
|
||||
finish_reason: response.finish_reason,
|
||||
metadata,
|
||||
})
|
||||
} else {
|
||||
// No tools, use simple completion
|
||||
@@ -812,11 +838,18 @@ Respond in JSON format:
|
||||
let response = self.llm.complete(request).await?;
|
||||
let pre_truncated = truncate_at_tool_tags(&response.content);
|
||||
let cleaned = clean_response(&pre_truncated);
|
||||
let final_text = if cleaned.trim().is_empty() {
|
||||
let metadata = if cleaned.trim().is_empty() {
|
||||
tracing::warn!(
|
||||
"LLM response was empty after cleaning (original len={}), using fallback",
|
||||
response.content.len()
|
||||
);
|
||||
ResponseMetadata {
|
||||
anomaly: Some(ResponseAnomaly::EmptyTextResponse),
|
||||
}
|
||||
} else {
|
||||
ResponseMetadata::default()
|
||||
};
|
||||
let final_text = if metadata.anomaly.is_some() {
|
||||
"I'm not sure how to respond to that.".to_string()
|
||||
} else {
|
||||
cleaned
|
||||
@@ -830,6 +863,7 @@ Respond in JSON format:
|
||||
cache_creation_input_tokens: response.cache_creation_input_tokens,
|
||||
},
|
||||
finish_reason: response.finish_reason,
|
||||
metadata,
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -3101,9 +3135,104 @@ That's my plan."#;
|
||||
context.force_text = true;
|
||||
|
||||
let output = reasoning.respond_with_tools(&context).await.unwrap();
|
||||
let metadata = output.metadata;
|
||||
match output.result {
|
||||
RespondResult::Text(text) => {
|
||||
assert_eq!(text, "I'm not sure how to respond to that.");
|
||||
assert_eq!(metadata.anomaly, Some(ResponseAnomaly::EmptyTextResponse));
|
||||
}
|
||||
RespondResult::ToolCalls { .. } => {
|
||||
panic!("Expected fallback text, not tool calls");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_respond_with_tools_flags_empty_tool_completion() {
|
||||
use crate::testing::StubLlm;
|
||||
let llm = Arc::new(StubLlm::new(""));
|
||||
let reasoning = Reasoning::new(llm);
|
||||
|
||||
let context = ReasoningContext::new()
|
||||
.with_message(ChatMessage::user("list tools"))
|
||||
.with_tools(vec![ToolDefinition {
|
||||
name: "tool_list".to_string(),
|
||||
description: "Lists tools".to_string(),
|
||||
parameters: serde_json::json!({}),
|
||||
}]);
|
||||
|
||||
let output = reasoning.respond_with_tools(&context).await.unwrap();
|
||||
let metadata = output.metadata;
|
||||
match output.result {
|
||||
RespondResult::Text(text) => {
|
||||
assert_eq!(text, "I'm not sure how to respond to that.");
|
||||
assert_eq!(metadata.anomaly, Some(ResponseAnomaly::EmptyToolCompletion));
|
||||
}
|
||||
RespondResult::ToolCalls { .. } => {
|
||||
panic!("Expected fallback text, not tool calls");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_respond_with_tools_flags_empty_tool_completion_when_content_is_none() {
|
||||
use crate::llm::{
|
||||
FinishReason, LlmProvider, ToolCompletionRequest, ToolCompletionResponse,
|
||||
};
|
||||
use async_trait::async_trait;
|
||||
use rust_decimal::Decimal;
|
||||
|
||||
struct NoneContentToolLlm;
|
||||
|
||||
#[async_trait]
|
||||
impl LlmProvider for NoneContentToolLlm {
|
||||
fn model_name(&self) -> &str {
|
||||
"none-content-tool-llm"
|
||||
}
|
||||
|
||||
fn cost_per_token(&self) -> (Decimal, Decimal) {
|
||||
(Decimal::ZERO, Decimal::ZERO)
|
||||
}
|
||||
|
||||
async fn complete(
|
||||
&self,
|
||||
_request: crate::llm::CompletionRequest,
|
||||
) -> Result<crate::llm::CompletionResponse, crate::llm::LlmError> {
|
||||
unreachable!("tool-mode test should not call complete()")
|
||||
}
|
||||
|
||||
async fn complete_with_tools(
|
||||
&self,
|
||||
_request: ToolCompletionRequest,
|
||||
) -> Result<ToolCompletionResponse, crate::llm::LlmError> {
|
||||
Ok(ToolCompletionResponse {
|
||||
content: None,
|
||||
tool_calls: Vec::new(),
|
||||
input_tokens: 0,
|
||||
output_tokens: 0,
|
||||
finish_reason: FinishReason::Stop,
|
||||
cache_read_input_tokens: 0,
|
||||
cache_creation_input_tokens: 0,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
let reasoning = Reasoning::new(Arc::new(NoneContentToolLlm));
|
||||
|
||||
let context = ReasoningContext::new()
|
||||
.with_message(ChatMessage::user("list tools"))
|
||||
.with_tools(vec![ToolDefinition {
|
||||
name: "tool_list".to_string(),
|
||||
description: "Lists tools".to_string(),
|
||||
parameters: serde_json::json!({}),
|
||||
}]);
|
||||
|
||||
let output = reasoning.respond_with_tools(&context).await.unwrap();
|
||||
let metadata = output.metadata;
|
||||
match output.result {
|
||||
RespondResult::Text(text) => {
|
||||
assert_eq!(text, "I'm not sure how to respond to that.");
|
||||
assert_eq!(metadata.anomaly, Some(ResponseAnomaly::EmptyToolCompletion));
|
||||
}
|
||||
RespondResult::ToolCalls { .. } => {
|
||||
panic!("Expected fallback text, not tool calls");
|
||||
|
||||
@@ -0,0 +1,150 @@
|
||||
use crate::llm::{ResponseAnomaly, ResponseMetadata};
|
||||
|
||||
pub(crate) const EMPTY_TOOL_COMPLETION_NUDGE: &str = "\
|
||||
Your previous tool-enabled response was empty or malformed.\n\
|
||||
If you need to use a tool, call it now with valid arguments.\n\
|
||||
Otherwise, provide a real status update about work already completed.";
|
||||
|
||||
pub(crate) const FORCE_TEXT_RECOVERY_PROMPT: &str = "\
|
||||
Your previous tool-enabled responses were empty or malformed.\n\
|
||||
Do not call any more tools in the next reply.\n\
|
||||
Instead, provide a concise final status based only on work already completed.\n\
|
||||
If the job is complete, say so explicitly. If not, explain what blocked you.";
|
||||
|
||||
pub(crate) const EMPTY_TOOL_COMPLETION_FAILURE: &str = "the selected model repeatedly returned empty or malformed tool-completion responses and is not reliable for autonomous tool use.";
|
||||
|
||||
#[derive(Debug, Default, Clone, Copy)]
|
||||
pub(crate) struct AutonomousRecoveryState {
|
||||
consecutive_empty_tool_completions: usize,
|
||||
force_text_recovery_pending: bool,
|
||||
force_text_recovery_active: bool,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub(crate) enum AutonomousRecoveryAction {
|
||||
Continue,
|
||||
ToolModeNudge,
|
||||
ForceTextRecovery,
|
||||
Fail,
|
||||
}
|
||||
|
||||
impl AutonomousRecoveryState {
|
||||
pub(crate) fn begin_iteration(&mut self) -> bool {
|
||||
if self.force_text_recovery_pending {
|
||||
self.force_text_recovery_pending = false;
|
||||
self.force_text_recovery_active = true;
|
||||
true
|
||||
} else {
|
||||
self.force_text_recovery_active
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn on_text_response(
|
||||
&mut self,
|
||||
metadata: ResponseMetadata,
|
||||
text: &str,
|
||||
) -> AutonomousRecoveryAction {
|
||||
match metadata.anomaly {
|
||||
Some(ResponseAnomaly::EmptyToolCompletion) => {
|
||||
self.consecutive_empty_tool_completions =
|
||||
self.consecutive_empty_tool_completions.saturating_add(1);
|
||||
self.force_text_recovery_active = false;
|
||||
match self.consecutive_empty_tool_completions {
|
||||
1 => AutonomousRecoveryAction::ToolModeNudge,
|
||||
2 => {
|
||||
self.force_text_recovery_pending = true;
|
||||
AutonomousRecoveryAction::ForceTextRecovery
|
||||
}
|
||||
_ => AutonomousRecoveryAction::Fail,
|
||||
}
|
||||
}
|
||||
Some(ResponseAnomaly::EmptyTextResponse) if self.force_text_recovery_active => {
|
||||
self.force_text_recovery_active = false;
|
||||
AutonomousRecoveryAction::Fail
|
||||
}
|
||||
_ if !text.trim().is_empty() => {
|
||||
self.reset();
|
||||
AutonomousRecoveryAction::Continue
|
||||
}
|
||||
_ => AutonomousRecoveryAction::Continue,
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn on_valid_tool_call(&mut self) {
|
||||
self.reset();
|
||||
}
|
||||
|
||||
fn reset(&mut self) {
|
||||
self.consecutive_empty_tool_completions = 0;
|
||||
self.force_text_recovery_pending = false;
|
||||
self.force_text_recovery_active = false;
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
fn metadata(anomaly: ResponseAnomaly) -> ResponseMetadata {
|
||||
ResponseMetadata {
|
||||
anomaly: Some(anomaly),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn first_empty_tool_completion_issues_nudge() {
|
||||
let mut state = AutonomousRecoveryState::default();
|
||||
let action = state.on_text_response(
|
||||
metadata(ResponseAnomaly::EmptyToolCompletion),
|
||||
"I'm not sure how to respond to that.",
|
||||
);
|
||||
assert_eq!(action, AutonomousRecoveryAction::ToolModeNudge);
|
||||
assert!(!state.begin_iteration());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn second_empty_tool_completion_schedules_text_recovery() {
|
||||
let mut state = AutonomousRecoveryState::default();
|
||||
let _ = state.on_text_response(metadata(ResponseAnomaly::EmptyToolCompletion), "fallback");
|
||||
let action =
|
||||
state.on_text_response(metadata(ResponseAnomaly::EmptyToolCompletion), "fallback");
|
||||
assert_eq!(action, AutonomousRecoveryAction::ForceTextRecovery);
|
||||
assert!(state.begin_iteration());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn forced_text_recovery_fallback_fails() {
|
||||
let mut state = AutonomousRecoveryState::default();
|
||||
let _ = state.on_text_response(metadata(ResponseAnomaly::EmptyToolCompletion), "fallback");
|
||||
let _ = state.on_text_response(metadata(ResponseAnomaly::EmptyToolCompletion), "fallback");
|
||||
assert!(state.begin_iteration());
|
||||
let action =
|
||||
state.on_text_response(metadata(ResponseAnomaly::EmptyTextResponse), "fallback");
|
||||
assert_eq!(action, AutonomousRecoveryAction::Fail);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn valid_tool_call_resets_counter() {
|
||||
let mut state = AutonomousRecoveryState::default();
|
||||
let _ = state.on_text_response(metadata(ResponseAnomaly::EmptyToolCompletion), "fallback");
|
||||
state.on_valid_tool_call();
|
||||
let action =
|
||||
state.on_text_response(metadata(ResponseAnomaly::EmptyToolCompletion), "fallback");
|
||||
assert_eq!(action, AutonomousRecoveryAction::ToolModeNudge);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn meaningful_text_after_text_recovery_resets_state() {
|
||||
let mut state = AutonomousRecoveryState::default();
|
||||
let _ = state.on_text_response(metadata(ResponseAnomaly::EmptyToolCompletion), "fallback");
|
||||
let _ = state.on_text_response(metadata(ResponseAnomaly::EmptyToolCompletion), "fallback");
|
||||
assert!(state.begin_iteration());
|
||||
|
||||
let action = state.on_text_response(ResponseMetadata::default(), "Still working on step 2");
|
||||
assert_eq!(action, AutonomousRecoveryAction::Continue);
|
||||
|
||||
let next =
|
||||
state.on_text_response(metadata(ResponseAnomaly::EmptyToolCompletion), "fallback");
|
||||
assert_eq!(next, AutonomousRecoveryAction::ToolModeNudge);
|
||||
}
|
||||
}
|
||||
+86
-3
@@ -21,11 +21,15 @@ use crate::agent::agentic_loop::{
|
||||
use crate::config::SafetyConfig;
|
||||
use crate::context::JobContext;
|
||||
use crate::error::WorkerError;
|
||||
use crate::llm::{ChatMessage, LlmProvider, Reasoning, ReasoningContext};
|
||||
use crate::llm::{ChatMessage, LlmProvider, Reasoning, ReasoningContext, ResponseMetadata};
|
||||
use crate::safety::SafetyLayer;
|
||||
use crate::tools::ToolRegistry;
|
||||
use crate::tools::execute::{execute_tool_simple, process_tool_result};
|
||||
use crate::worker::api::{CompletionReport, JobEventPayload, StatusUpdate, WorkerHttpClient};
|
||||
use crate::worker::autonomous_recovery::{
|
||||
AutonomousRecoveryAction, AutonomousRecoveryState, EMPTY_TOOL_COMPLETION_FAILURE,
|
||||
EMPTY_TOOL_COMPLETION_NUDGE, FORCE_TEXT_RECOVERY_PROMPT,
|
||||
};
|
||||
use crate::worker::proxy_llm::ProxyLlmProvider;
|
||||
|
||||
/// Configuration for the worker runtime.
|
||||
@@ -170,6 +174,7 @@ Work independently to complete this job. When finished, your final message MUST
|
||||
extra_env: self.extra_env.clone(),
|
||||
last_output: Mutex::new(String::new()),
|
||||
iteration_tracker: iteration_tracker.clone(),
|
||||
recovery_state: Mutex::new(AutonomousRecoveryState::default()),
|
||||
};
|
||||
|
||||
let config = AgenticLoopConfig {
|
||||
@@ -228,6 +233,24 @@ Work independently to complete this job. When finished, your final message MUST
|
||||
})
|
||||
.await?;
|
||||
}
|
||||
Ok(Ok(LoopOutcome::Failure(reason))) => {
|
||||
tracing::warn!("Worker failed for job {}: {}", self.config.job_id, reason);
|
||||
self.post_event(
|
||||
"result",
|
||||
serde_json::json!({
|
||||
"success": false,
|
||||
"message": reason,
|
||||
}),
|
||||
)
|
||||
.await;
|
||||
self.client
|
||||
.report_complete(&CompletionReport {
|
||||
success: false,
|
||||
message: Some(reason),
|
||||
iterations,
|
||||
})
|
||||
.await?;
|
||||
}
|
||||
Ok(Ok(LoopOutcome::Stopped | LoopOutcome::NeedApproval(_))) => {
|
||||
tracing::info!("Worker for job {} stopped", self.config.job_id);
|
||||
self.client
|
||||
@@ -304,6 +327,7 @@ struct ContainerDelegate {
|
||||
/// Tracks the current iteration — shared with the outer `run` method so
|
||||
/// `CompletionReport` can include accurate iteration counts.
|
||||
iteration_tracker: Arc<Mutex<u32>>,
|
||||
recovery_state: Mutex<AutonomousRecoveryState>,
|
||||
}
|
||||
|
||||
impl ContainerDelegate {
|
||||
@@ -377,8 +401,17 @@ impl LoopDelegate for ContainerDelegate {
|
||||
// conversation. Ensure the last message is user-role before calling the LLM.
|
||||
crate::util::ensure_ends_with_user_message(&mut reason_ctx.messages);
|
||||
|
||||
// Refresh tools (in case WASM tools were built)
|
||||
reason_ctx.available_tools = self.tools.tool_definitions().await;
|
||||
let force_text_recovery = {
|
||||
let mut recovery = self.recovery_state.lock().await;
|
||||
recovery.begin_iteration()
|
||||
};
|
||||
if force_text_recovery {
|
||||
tracing::warn!("Switching to text-only recovery after malformed tool completions");
|
||||
reason_ctx.available_tools.clear();
|
||||
} else {
|
||||
// Refresh tools (in case WASM tools were built)
|
||||
reason_ctx.available_tools = self.tools.tool_definitions().await;
|
||||
}
|
||||
|
||||
None
|
||||
}
|
||||
@@ -399,8 +432,53 @@ impl LoopDelegate for ContainerDelegate {
|
||||
async fn handle_text_response(
|
||||
&self,
|
||||
text: &str,
|
||||
metadata: ResponseMetadata,
|
||||
reason_ctx: &mut ReasoningContext,
|
||||
) -> TextAction {
|
||||
let action = {
|
||||
let mut recovery = self.recovery_state.lock().await;
|
||||
recovery.on_text_response(metadata, text)
|
||||
};
|
||||
match action {
|
||||
AutonomousRecoveryAction::ToolModeNudge => {
|
||||
tracing::warn!("Malformed empty tool completion detected; retrying in tool mode");
|
||||
self.post_event(
|
||||
"status",
|
||||
serde_json::json!({
|
||||
"message": "Model returned an empty tool-completion response; retrying with a stronger tool-use nudge.",
|
||||
}),
|
||||
)
|
||||
.await;
|
||||
reason_ctx
|
||||
.messages
|
||||
.push(ChatMessage::user(EMPTY_TOOL_COMPLETION_NUDGE));
|
||||
return TextAction::Continue;
|
||||
}
|
||||
AutonomousRecoveryAction::ForceTextRecovery => {
|
||||
tracing::warn!(
|
||||
"Repeated malformed tool completions detected; switching to text-only recovery"
|
||||
);
|
||||
self.post_event(
|
||||
"status",
|
||||
serde_json::json!({
|
||||
"message": "Model returned repeated empty tool-completion responses; requesting a final status update without tools.",
|
||||
}),
|
||||
)
|
||||
.await;
|
||||
reason_ctx
|
||||
.messages
|
||||
.push(ChatMessage::user(FORCE_TEXT_RECOVERY_PROMPT));
|
||||
return TextAction::Continue;
|
||||
}
|
||||
AutonomousRecoveryAction::Fail => {
|
||||
tracing::warn!("Failing fast after repeated malformed autonomous responses");
|
||||
return TextAction::Return(LoopOutcome::Failure(
|
||||
EMPTY_TOOL_COMPLETION_FAILURE.to_string(),
|
||||
));
|
||||
}
|
||||
AutonomousRecoveryAction::Continue => {}
|
||||
}
|
||||
|
||||
self.post_event(
|
||||
"message",
|
||||
serde_json::json!({
|
||||
@@ -431,6 +509,11 @@ impl LoopDelegate for ContainerDelegate {
|
||||
content: Option<String>,
|
||||
reason_ctx: &mut ReasoningContext,
|
||||
) -> Result<Option<LoopOutcome>, crate::error::Error> {
|
||||
{
|
||||
let mut recovery = self.recovery_state.lock().await;
|
||||
recovery.on_valid_tool_call();
|
||||
}
|
||||
|
||||
if let Some(ref text) = content {
|
||||
self.post_event(
|
||||
"message",
|
||||
|
||||
+84
-4
@@ -23,8 +23,8 @@ use crate::context::{ContextManager, JobState};
|
||||
use crate::error::Error;
|
||||
use crate::hooks::HookRegistry;
|
||||
use crate::llm::{
|
||||
ActionPlan, ChatMessage, LlmProvider, Reasoning, ReasoningContext, RespondResult, ToolCall,
|
||||
ToolSelection,
|
||||
ActionPlan, ChatMessage, LlmProvider, Reasoning, ReasoningContext, RespondResult,
|
||||
ResponseMetadata, ToolCall, ToolSelection,
|
||||
};
|
||||
use crate::safety::SafetyLayer;
|
||||
use crate::tenant::AdminScope;
|
||||
@@ -33,6 +33,10 @@ use crate::tools::rate_limiter::RateLimitResult;
|
||||
use crate::tools::{
|
||||
ApprovalContext, ToolRegistry, autonomous_unavailable_error, prepare_tool_params, redact_params,
|
||||
};
|
||||
use crate::worker::autonomous_recovery::{
|
||||
AutonomousRecoveryAction, AutonomousRecoveryState, EMPTY_TOOL_COMPLETION_FAILURE,
|
||||
EMPTY_TOOL_COMPLETION_NUDGE, FORCE_TEXT_RECOVERY_PROMPT,
|
||||
};
|
||||
use ironclaw_common::AppEvent;
|
||||
|
||||
/// Shared dependencies for worker execution.
|
||||
@@ -391,6 +395,7 @@ Report when the job is complete or if you encounter issues you cannot resolve."#
|
||||
worker: self,
|
||||
rx: tokio::sync::Mutex::new(rx),
|
||||
consecutive_rate_limits: std::sync::atomic::AtomicUsize::new(0),
|
||||
recovery_state: tokio::sync::Mutex::new(AutonomousRecoveryState::default()),
|
||||
};
|
||||
|
||||
let config = AgenticLoopConfig {
|
||||
@@ -409,6 +414,9 @@ Report when the job is complete or if you encounter issues you cannot resolve."#
|
||||
self.mark_failed("Maximum iterations exceeded: job hit the iteration cap")
|
||||
.await?;
|
||||
}
|
||||
LoopOutcome::Failure(reason) => {
|
||||
self.mark_failed(&reason).await?;
|
||||
}
|
||||
LoopOutcome::Stopped => {
|
||||
// Stop signal handled — nothing more to do
|
||||
}
|
||||
@@ -1109,6 +1117,7 @@ struct JobDelegate<'a> {
|
||||
rx: tokio::sync::Mutex<&'a mut mpsc::Receiver<WorkerMessage>>,
|
||||
/// Tracks consecutive rate-limit errors to fail fast instead of burning iterations.
|
||||
consecutive_rate_limits: std::sync::atomic::AtomicUsize,
|
||||
recovery_state: tokio::sync::Mutex<AutonomousRecoveryState>,
|
||||
}
|
||||
|
||||
impl<'a> JobDelegate<'a> {
|
||||
@@ -1159,6 +1168,7 @@ impl<'a> JobDelegate<'a> {
|
||||
result: RespondResult::Text(String::new()),
|
||||
usage: crate::llm::TokenUsage::default(),
|
||||
finish_reason: crate::llm::FinishReason::Stop,
|
||||
metadata: ResponseMetadata::default(),
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -1250,8 +1260,21 @@ impl<'a> LoopDelegate for JobDelegate<'a> {
|
||||
reason_ctx: &mut ReasoningContext,
|
||||
_iteration: usize,
|
||||
) -> Option<LoopOutcome> {
|
||||
// Refresh tool definitions so newly built tools become visible
|
||||
reason_ctx.available_tools = self.worker.tools().tool_definitions().await;
|
||||
let force_text_recovery = {
|
||||
let mut recovery = self.recovery_state.lock().await;
|
||||
recovery.begin_iteration()
|
||||
};
|
||||
|
||||
if force_text_recovery {
|
||||
tracing::warn!(
|
||||
job_id = %self.worker.job_id,
|
||||
"Switching to text-only recovery after malformed tool completions"
|
||||
);
|
||||
reason_ctx.available_tools.clear();
|
||||
} else {
|
||||
// Refresh tool definitions so newly built tools become visible
|
||||
reason_ctx.available_tools = self.worker.tools().tool_definitions().await;
|
||||
}
|
||||
|
||||
// Claude 4.6 rejects assistant prefill; NEAR AI rejects any non-user-ending
|
||||
// conversation. Ensure the last message is user-role before calling the LLM.
|
||||
@@ -1285,6 +1308,7 @@ impl<'a> LoopDelegate for JobDelegate<'a> {
|
||||
},
|
||||
usage: crate::llm::TokenUsage::default(),
|
||||
finish_reason: crate::llm::FinishReason::ToolUse,
|
||||
metadata: ResponseMetadata::default(),
|
||||
});
|
||||
}
|
||||
Ok(_) => {} // empty selections, fall through
|
||||
@@ -1328,8 +1352,59 @@ impl<'a> LoopDelegate for JobDelegate<'a> {
|
||||
async fn handle_text_response(
|
||||
&self,
|
||||
text: &str,
|
||||
metadata: ResponseMetadata,
|
||||
reason_ctx: &mut ReasoningContext,
|
||||
) -> TextAction {
|
||||
let action = {
|
||||
let mut recovery = self.recovery_state.lock().await;
|
||||
recovery.on_text_response(metadata, text)
|
||||
};
|
||||
|
||||
match action {
|
||||
AutonomousRecoveryAction::ToolModeNudge => {
|
||||
tracing::warn!(
|
||||
job_id = %self.worker.job_id,
|
||||
"Malformed empty tool completion detected; retrying in tool mode"
|
||||
);
|
||||
self.worker.log_event(
|
||||
"status",
|
||||
serde_json::json!({
|
||||
"message": "Model returned an empty tool-completion response; retrying with a stronger tool-use nudge.",
|
||||
}),
|
||||
);
|
||||
reason_ctx
|
||||
.messages
|
||||
.push(ChatMessage::user(EMPTY_TOOL_COMPLETION_NUDGE));
|
||||
return TextAction::Continue;
|
||||
}
|
||||
AutonomousRecoveryAction::ForceTextRecovery => {
|
||||
tracing::warn!(
|
||||
job_id = %self.worker.job_id,
|
||||
"Repeated malformed tool completions detected; switching to text-only recovery"
|
||||
);
|
||||
self.worker.log_event(
|
||||
"status",
|
||||
serde_json::json!({
|
||||
"message": "Model returned repeated empty tool-completion responses; requesting a final status update without tools.",
|
||||
}),
|
||||
);
|
||||
reason_ctx
|
||||
.messages
|
||||
.push(ChatMessage::user(FORCE_TEXT_RECOVERY_PROMPT));
|
||||
return TextAction::Continue;
|
||||
}
|
||||
AutonomousRecoveryAction::Fail => {
|
||||
tracing::warn!(
|
||||
job_id = %self.worker.job_id,
|
||||
"Failing fast after repeated malformed autonomous responses"
|
||||
);
|
||||
return TextAction::Return(LoopOutcome::Failure(
|
||||
EMPTY_TOOL_COMPLETION_FAILURE.to_string(),
|
||||
));
|
||||
}
|
||||
AutonomousRecoveryAction::Continue => {}
|
||||
}
|
||||
|
||||
// Empty text from rate-limit backoff retry — skip processing and let the
|
||||
// loop proceed to the next iteration which will re-call the LLM.
|
||||
if text.is_empty() {
|
||||
@@ -1368,6 +1443,11 @@ impl<'a> LoopDelegate for JobDelegate<'a> {
|
||||
content: Option<String>,
|
||||
reason_ctx: &mut ReasoningContext,
|
||||
) -> Result<Option<LoopOutcome>, crate::error::Error> {
|
||||
{
|
||||
let mut recovery = self.recovery_state.lock().await;
|
||||
recovery.on_valid_tool_call();
|
||||
}
|
||||
|
||||
if let Some(ref text) = content {
|
||||
self.worker.log_event(
|
||||
"message",
|
||||
|
||||
@@ -25,6 +25,7 @@
|
||||
//! ```
|
||||
|
||||
pub mod api;
|
||||
mod autonomous_recovery;
|
||||
pub mod claude_bridge;
|
||||
pub mod container;
|
||||
pub mod job;
|
||||
|
||||
@@ -11,9 +11,76 @@ mod tests {
|
||||
use std::time::Duration;
|
||||
|
||||
use ironclaw::agent::routine::{RoutineAction, Trigger};
|
||||
use ironclaw::context::{JobContext, JobState};
|
||||
use uuid::Uuid;
|
||||
|
||||
use crate::support::test_rig::TestRigBuilder;
|
||||
use crate::support::trace_llm::LlmTrace;
|
||||
use crate::support::test_rig::{TestRig, TestRigBuilder};
|
||||
use crate::support::trace_llm::{LlmTrace, RequestHint, TraceResponse, TraceStep};
|
||||
|
||||
fn text_step(content: &str) -> TraceStep {
|
||||
TraceStep {
|
||||
request_hint: None,
|
||||
response: TraceResponse::Text {
|
||||
content: content.to_string(),
|
||||
input_tokens: 10,
|
||||
output_tokens: 5,
|
||||
},
|
||||
expected_tool_results: Vec::new(),
|
||||
}
|
||||
}
|
||||
|
||||
fn hinted_text_step(content: &str, last_user_message_contains: &str) -> TraceStep {
|
||||
TraceStep {
|
||||
request_hint: Some(RequestHint {
|
||||
last_user_message_contains: Some(last_user_message_contains.to_string()),
|
||||
min_message_count: None,
|
||||
}),
|
||||
response: TraceResponse::Text {
|
||||
content: content.to_string(),
|
||||
input_tokens: 10,
|
||||
output_tokens: 5,
|
||||
},
|
||||
expected_tool_results: Vec::new(),
|
||||
}
|
||||
}
|
||||
|
||||
fn extract_job_id(response: &str) -> Uuid {
|
||||
let id = response
|
||||
.lines()
|
||||
.find_map(|line| line.strip_prefix("ID: "))
|
||||
.expect("job creation response should include an ID line");
|
||||
Uuid::parse_str(id).expect("job ID should be a UUID")
|
||||
}
|
||||
|
||||
async fn wait_for_job_state(rig: &TestRig, job_id: Uuid, expected: JobState) -> JobContext {
|
||||
let deadline = tokio::time::Instant::now() + Duration::from_secs(10);
|
||||
|
||||
loop {
|
||||
if let Some(job) = rig
|
||||
.database()
|
||||
.get_job(job_id)
|
||||
.await
|
||||
.expect("get_job should succeed")
|
||||
&& job.state == expected
|
||||
{
|
||||
return job;
|
||||
}
|
||||
|
||||
assert!(
|
||||
tokio::time::Instant::now() < deadline,
|
||||
"job {job_id} did not reach state {expected:?} before timeout"
|
||||
);
|
||||
|
||||
tokio::time::sleep(Duration::from_millis(50)).await;
|
||||
}
|
||||
}
|
||||
|
||||
fn requests_contain(requests: &[Vec<ironclaw::llm::ChatMessage>], needle: &str) -> bool {
|
||||
requests
|
||||
.iter()
|
||||
.flatten()
|
||||
.any(|message| message.content.contains(needle))
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// Test 1: time_parse_and_diff
|
||||
@@ -685,6 +752,149 @@ mod tests {
|
||||
rig.shutdown();
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// Test 8a: command_job_fails_fast_on_repeated_empty_tool_completions
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
#[tokio::test]
|
||||
async fn command_job_fails_fast_on_repeated_empty_tool_completions() {
|
||||
let trace = LlmTrace::single_turn(
|
||||
"test-empty-tool-recovery-fail",
|
||||
"(worker only)",
|
||||
vec![
|
||||
text_step(""),
|
||||
text_step(""),
|
||||
hinted_text_step("", "valid arguments"),
|
||||
text_step(""),
|
||||
hinted_text_step("", "Do not call any more tools in the next reply."),
|
||||
],
|
||||
);
|
||||
|
||||
let rig = TestRigBuilder::new()
|
||||
.with_trace(trace)
|
||||
.with_auto_approve_tools(true)
|
||||
.build()
|
||||
.await;
|
||||
|
||||
rig.send_message("/job reproduce empty tool completion loop")
|
||||
.await;
|
||||
let create_responses = rig.wait_for_responses(1, Duration::from_secs(15)).await;
|
||||
let job_id = extract_job_id(&create_responses[0].content);
|
||||
|
||||
let job = wait_for_job_state(&rig, job_id, JobState::Failed).await;
|
||||
assert_eq!(job.title, "reproduce empty tool completion loop");
|
||||
|
||||
let failure_reason = rig
|
||||
.database()
|
||||
.get_agent_job_failure_reason(job_id)
|
||||
.await
|
||||
.expect("get_agent_job_failure_reason should succeed")
|
||||
.expect("failed job should persist a failure reason");
|
||||
assert!(
|
||||
failure_reason
|
||||
.contains("repeatedly returned empty or malformed tool-completion responses"),
|
||||
"unexpected failure reason: {failure_reason}"
|
||||
);
|
||||
assert!(
|
||||
!failure_reason.contains("max iterations"),
|
||||
"failure should not surface as iteration exhaustion: {failure_reason}"
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
rig.llm_call_count(),
|
||||
5,
|
||||
"worker should stop after the bounded recovery flow"
|
||||
);
|
||||
assert!(
|
||||
!rig.collect_metrics().await.hit_iteration_limit,
|
||||
"bounded recovery should stop before iteration-limit reporting"
|
||||
);
|
||||
|
||||
let requests = rig.captured_llm_requests();
|
||||
assert!(
|
||||
requests_contain(&requests, "call it now with valid arguments"),
|
||||
"expected targeted tool-mode recovery nudge in worker requests"
|
||||
);
|
||||
assert!(
|
||||
requests_contain(&requests, "Do not call any more tools in the next reply."),
|
||||
"expected forced text-only recovery prompt in worker requests"
|
||||
);
|
||||
|
||||
rig.clear().await;
|
||||
rig.send_message(&format!("/status {}", job_id)).await;
|
||||
let status_responses = rig.wait_for_responses(1, Duration::from_secs(5)).await;
|
||||
assert!(
|
||||
status_responses[0].content.contains("Status: Failed"),
|
||||
"unexpected status response: {:?}",
|
||||
status_responses[0].content
|
||||
);
|
||||
|
||||
rig.shutdown();
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// Test 8b: command_job_text_recovery_can_complete
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
#[tokio::test]
|
||||
async fn command_job_text_recovery_can_complete() {
|
||||
let trace = LlmTrace::single_turn(
|
||||
"test-empty-tool-recovery-success",
|
||||
"(worker only)",
|
||||
vec![
|
||||
text_step(""),
|
||||
text_step(""),
|
||||
hinted_text_step("", "valid arguments"),
|
||||
text_step(""),
|
||||
hinted_text_step(
|
||||
"The job is complete. I finished the requested work and there is nothing left to do.",
|
||||
"Do not call any more tools in the next reply.",
|
||||
),
|
||||
],
|
||||
);
|
||||
|
||||
let rig = TestRigBuilder::new()
|
||||
.with_trace(trace)
|
||||
.with_auto_approve_tools(true)
|
||||
.build()
|
||||
.await;
|
||||
|
||||
rig.send_message("/job recover after malformed tool completions")
|
||||
.await;
|
||||
let create_responses = rig.wait_for_responses(1, Duration::from_secs(15)).await;
|
||||
let job_id = extract_job_id(&create_responses[0].content);
|
||||
|
||||
let job = wait_for_job_state(&rig, job_id, JobState::Completed).await;
|
||||
assert_eq!(job.title, "recover after malformed tool completions");
|
||||
|
||||
assert_eq!(
|
||||
rig.llm_call_count(),
|
||||
5,
|
||||
"worker should complete within the bounded recovery flow"
|
||||
);
|
||||
|
||||
let requests = rig.captured_llm_requests();
|
||||
assert!(
|
||||
requests_contain(&requests, "call it now with valid arguments"),
|
||||
"expected targeted tool-mode recovery nudge in worker requests"
|
||||
);
|
||||
assert!(
|
||||
requests_contain(&requests, "Do not call any more tools in the next reply."),
|
||||
"expected forced text-only recovery prompt in worker requests"
|
||||
);
|
||||
|
||||
rig.clear().await;
|
||||
rig.send_message(&format!("/status {}", job_id)).await;
|
||||
let status_responses = rig.wait_for_responses(1, Duration::from_secs(5)).await;
|
||||
assert!(
|
||||
status_responses[0].content.contains("Status: Completed"),
|
||||
"unexpected status response: {:?}",
|
||||
status_responses[0].content
|
||||
);
|
||||
|
||||
rig.shutdown();
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// Test 9: job_list_cancel
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user