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
|
||||
|
||||
+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");
|
||||
|
||||
@@ -117,13 +117,6 @@ impl Tool for ReadFileTool {
|
||||
|
||||
let path = validate_path(path_str, self.base_dir.as_deref())?;
|
||||
|
||||
if super::path_utils::is_sensitive_path(&path) {
|
||||
return Err(ToolError::NotAuthorized(format!(
|
||||
"Access denied: '{}' is a sensitive path. Use the appropriate secrets management tool instead.",
|
||||
path_str
|
||||
)));
|
||||
}
|
||||
|
||||
// Check file size
|
||||
let metadata = fs::metadata(&path)
|
||||
.await
|
||||
@@ -263,13 +256,6 @@ impl Tool for WriteFileTool {
|
||||
|
||||
let path = validate_path(path_str, self.base_dir.as_deref())?;
|
||||
|
||||
if super::path_utils::is_sensitive_path(&path) {
|
||||
return Err(ToolError::NotAuthorized(format!(
|
||||
"Access denied: '{}' is a sensitive path",
|
||||
path_str
|
||||
)));
|
||||
}
|
||||
|
||||
// Create parent directories
|
||||
if let Some(parent) = path.parent() {
|
||||
fs::create_dir_all(parent).await.map_err(|e| {
|
||||
@@ -378,13 +364,6 @@ impl Tool for ListDirTool {
|
||||
|
||||
let path = validate_path(path_str, self.base_dir.as_deref())?;
|
||||
|
||||
if super::path_utils::is_sensitive_path(&path) {
|
||||
return Err(ToolError::NotAuthorized(format!(
|
||||
"Access denied: '{}' is a sensitive directory",
|
||||
path_str
|
||||
)));
|
||||
}
|
||||
|
||||
let mut entries = Vec::new();
|
||||
list_dir_inner(&path, &path, recursive, max_depth, 0, &mut entries).await?;
|
||||
|
||||
@@ -468,10 +447,6 @@ async fn list_dir_inner(
|
||||
entries.push(display);
|
||||
|
||||
if recursive && is_dir && current_depth < max_depth {
|
||||
// Skip sensitive directories during recursive traversal
|
||||
if super::path_utils::is_sensitive_path(&entry_path) {
|
||||
continue;
|
||||
}
|
||||
// Skip common non-essential directories
|
||||
let name = entry.file_name();
|
||||
let name_str = name.to_string_lossy();
|
||||
@@ -586,13 +561,6 @@ impl Tool for ApplyPatchTool {
|
||||
|
||||
let path = validate_path(path_str, self.base_dir.as_deref())?;
|
||||
|
||||
if super::path_utils::is_sensitive_path(&path) {
|
||||
return Err(ToolError::NotAuthorized(format!(
|
||||
"Access denied: '{}' is a sensitive path",
|
||||
path_str
|
||||
)));
|
||||
}
|
||||
|
||||
// Read current content
|
||||
let content = fs::read_to_string(&path)
|
||||
.await
|
||||
|
||||
@@ -4,119 +4,9 @@
|
||||
//! attacks and ensure paths stay within allowed sandboxes.
|
||||
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::sync::LazyLock;
|
||||
|
||||
use crate::tools::tool::ToolError;
|
||||
|
||||
/// Paths that contain credentials, secrets, or private keys.
|
||||
/// Used by both file tools (exact path check) and shell tool (substring scan).
|
||||
/// Keep sorted by category for readability.
|
||||
static SENSITIVE_PATH_PATTERNS: LazyLock<Vec<&'static str>> = LazyLock::new(|| {
|
||||
vec![
|
||||
// SSH
|
||||
"/.ssh/",
|
||||
"/id_rsa",
|
||||
"/id_ed25519",
|
||||
"/id_ecdsa",
|
||||
"/id_dsa",
|
||||
"/authorized_keys",
|
||||
"/known_hosts",
|
||||
// GPG
|
||||
"/.gnupg/",
|
||||
// AWS
|
||||
"/.aws/credentials",
|
||||
"/.aws/config",
|
||||
// Kubernetes
|
||||
"/.kube/config",
|
||||
// Cloud providers
|
||||
"/.azure/",
|
||||
"/.gcloud/",
|
||||
"/.config/gcloud/",
|
||||
// Terraform
|
||||
"/.terraform.d/credentials.tfrc.json",
|
||||
// GitHub CLI
|
||||
"/.config/gh/hosts.yml",
|
||||
// Docker
|
||||
"/.docker/config.json",
|
||||
// Vault
|
||||
"/.vault-token",
|
||||
// Shell history
|
||||
"/.bash_history",
|
||||
"/.zsh_history",
|
||||
"/.histfile",
|
||||
// Env files (may contain secrets)
|
||||
"/.env",
|
||||
// Git credentials
|
||||
"/.git-credentials",
|
||||
"/.netrc",
|
||||
"/.pgpass",
|
||||
// IronClaw's own secrets
|
||||
"/.ironclaw/secrets/",
|
||||
// System
|
||||
"/etc/shadow",
|
||||
"/etc/gshadow",
|
||||
]
|
||||
});
|
||||
|
||||
/// File extensions that are always sensitive regardless of location.
|
||||
static SENSITIVE_EXTENSIONS: LazyLock<Vec<&'static str>> =
|
||||
LazyLock::new(|| vec![".pem", ".key", ".p12", ".pfx", ".jks", ".keystore"]);
|
||||
|
||||
/// Suffixes that indicate a file is safe despite matching a sensitive pattern
|
||||
/// (e.g., `.env.example`, `.env.sample`).
|
||||
static SAFE_SUFFIXES: LazyLock<Vec<&'static str>> =
|
||||
LazyLock::new(|| vec![".example", ".sample", ".template", ".dist", ".bak.example"]);
|
||||
|
||||
/// Check if a resolved file path points to a sensitive location.
|
||||
/// Used by file tools (read, write, list_dir, apply_patch).
|
||||
pub fn is_sensitive_path(path: &Path) -> bool {
|
||||
let path_str = match path.canonicalize() {
|
||||
Ok(p) => p.to_string_lossy().to_string(),
|
||||
Err(_) => path.to_string_lossy().to_string(),
|
||||
};
|
||||
|
||||
// Safe suffixes override sensitive patterns
|
||||
let lower = path_str.to_lowercase();
|
||||
if SAFE_SUFFIXES.iter().any(|s| lower.ends_with(s)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Check sensitive path patterns
|
||||
if SENSITIVE_PATH_PATTERNS.iter().any(|p| path_str.contains(p)) {
|
||||
return true;
|
||||
}
|
||||
|
||||
// Check sensitive file extensions
|
||||
if let Some(ext) = path.extension().and_then(|e| e.to_str()) {
|
||||
let dot_ext = format!(".{}", ext.to_lowercase());
|
||||
if SENSITIVE_EXTENSIONS.iter().any(|e| *e == dot_ext) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
false
|
||||
}
|
||||
|
||||
/// Scan a shell command string for references to sensitive paths.
|
||||
/// Returns the first matched pattern, or None if the command is clean.
|
||||
/// Used by the shell tool to block `cat ~/.ssh/id_rsa` etc.
|
||||
pub fn command_references_sensitive_path(command: &str) -> Option<&'static str> {
|
||||
let normalized = command.to_lowercase();
|
||||
|
||||
for pattern in SENSITIVE_PATH_PATTERNS.iter() {
|
||||
// For path patterns, check case-insensitively
|
||||
if normalized.contains(&pattern.to_lowercase()) {
|
||||
return Some(pattern);
|
||||
}
|
||||
}
|
||||
|
||||
// Check for sensitive extensions in file arguments
|
||||
SENSITIVE_EXTENSIONS
|
||||
.iter()
|
||||
.find(|ext| normalized.contains(*ext))
|
||||
.copied()
|
||||
}
|
||||
|
||||
/// Normalize a path by resolving `.` and `..` components lexically (no filesystem access).
|
||||
///
|
||||
/// This is critical for security: `std::fs::canonicalize` only works on paths that exist,
|
||||
@@ -346,95 +236,4 @@ mod tests {
|
||||
let result = validate_path("a/b/../c.txt", Some(dir.path()));
|
||||
assert!(result.is_ok());
|
||||
}
|
||||
|
||||
// ── sensitive path tests ──
|
||||
|
||||
#[test]
|
||||
fn test_is_sensitive_path_blocks_ssh() {
|
||||
assert!(is_sensitive_path(Path::new("/home/user/.ssh/id_rsa")));
|
||||
assert!(is_sensitive_path(Path::new(
|
||||
"/home/user/.ssh/authorized_keys"
|
||||
)));
|
||||
assert!(is_sensitive_path(Path::new("/root/.ssh/config")));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_is_sensitive_path_blocks_cloud_credentials() {
|
||||
assert!(is_sensitive_path(Path::new("/home/user/.aws/credentials")));
|
||||
assert!(is_sensitive_path(Path::new("/home/user/.kube/config")));
|
||||
assert!(is_sensitive_path(Path::new("/home/user/.azure/some_token")));
|
||||
assert!(is_sensitive_path(Path::new(
|
||||
"/home/user/.config/gh/hosts.yml"
|
||||
)));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_is_sensitive_path_blocks_system_secrets() {
|
||||
assert!(is_sensitive_path(Path::new("/etc/shadow")));
|
||||
assert!(is_sensitive_path(Path::new("/etc/gshadow")));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_is_sensitive_path_blocks_key_files_by_extension() {
|
||||
assert!(is_sensitive_path(Path::new("/tmp/server.pem")));
|
||||
assert!(is_sensitive_path(Path::new("/app/certs/private.key")));
|
||||
assert!(is_sensitive_path(Path::new("/home/user/keystore.p12")));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_is_sensitive_path_allows_safe_suffixes() {
|
||||
assert!(!is_sensitive_path(Path::new("/app/.env.example")));
|
||||
assert!(!is_sensitive_path(Path::new("/app/.env.sample")));
|
||||
assert!(!is_sensitive_path(Path::new("/app/.env.template")));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_is_sensitive_path_allows_normal_files() {
|
||||
assert!(!is_sensitive_path(Path::new("/app/src/main.rs")));
|
||||
assert!(!is_sensitive_path(Path::new("/home/user/README.md")));
|
||||
assert!(!is_sensitive_path(Path::new("/tmp/output.json")));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_is_sensitive_path_blocks_env_files() {
|
||||
assert!(is_sensitive_path(Path::new("/app/.env")));
|
||||
assert!(is_sensitive_path(Path::new("/app/.env.local")));
|
||||
assert!(is_sensitive_path(Path::new("/app/.env.production")));
|
||||
}
|
||||
|
||||
// ── command scanning tests ──
|
||||
|
||||
#[test]
|
||||
fn test_command_references_sensitive_path_catches_cat_ssh() {
|
||||
assert!(command_references_sensitive_path("cat ~/.ssh/id_rsa").is_some());
|
||||
assert!(
|
||||
command_references_sensitive_path("head -n 5 /home/user/.ssh/authorized_keys")
|
||||
.is_some()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_command_references_sensitive_path_catches_aws() {
|
||||
assert!(command_references_sensitive_path("cat ~/.aws/credentials").is_some());
|
||||
assert!(command_references_sensitive_path("grep key ~/.aws/config").is_some());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_command_references_sensitive_path_catches_etc_shadow() {
|
||||
assert!(command_references_sensitive_path("cat /etc/shadow").is_some());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_command_references_sensitive_path_catches_key_extensions() {
|
||||
assert!(command_references_sensitive_path("cp server.pem /tmp/").is_some());
|
||||
assert!(command_references_sensitive_path("cat private.key").is_some());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_command_references_sensitive_path_allows_safe_commands() {
|
||||
assert!(command_references_sensitive_path("ls -la").is_none());
|
||||
assert!(command_references_sensitive_path("cargo build").is_none());
|
||||
assert!(command_references_sensitive_path("git status").is_none());
|
||||
assert!(command_references_sensitive_path("cat README.md").is_none());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -83,12 +83,21 @@ static BLOCKED_COMMANDS: LazyLock<HashSet<&'static str>> = LazyLock::new(|| {
|
||||
});
|
||||
|
||||
/// Patterns that indicate potentially dangerous commands.
|
||||
/// Note: sensitive file paths (/.ssh/, /etc/shadow, etc.) are now handled by
|
||||
/// `command_references_sensitive_path` in path_utils.rs for consistency with
|
||||
/// file tool protections. This list covers command-level dangers only.
|
||||
static DANGEROUS_PATTERNS: LazyLock<Vec<&'static str>> = LazyLock::new(|| {
|
||||
vec![
|
||||
"sudo ", "doas ", " | sh", " | bash", " | zsh", "eval ", "$(curl", "$(wget",
|
||||
"sudo ",
|
||||
"doas ",
|
||||
" | sh",
|
||||
" | bash",
|
||||
" | zsh",
|
||||
"eval ",
|
||||
"$(curl",
|
||||
"$(wget",
|
||||
"/etc/passwd",
|
||||
"/etc/shadow",
|
||||
"~/.ssh",
|
||||
".bash_history",
|
||||
"id_rsa",
|
||||
]
|
||||
});
|
||||
|
||||
@@ -613,11 +622,6 @@ impl ShellTool {
|
||||
}
|
||||
}
|
||||
|
||||
// Block commands that reference sensitive file paths (shared with file tools)
|
||||
if super::path_utils::command_references_sensitive_path(cmd).is_some() {
|
||||
return Some("Command references sensitive file path");
|
||||
}
|
||||
|
||||
None
|
||||
}
|
||||
|
||||
|
||||
@@ -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