Compare commits
2 Commits
9e17cd35da
...
ed45ec54ed
| Author | SHA1 | Date | |
|---|---|---|---|
| ed45ec54ed | |||
| fc8a0aa6ae |
@ -125,6 +125,27 @@ fn parse_pending_tool_output(output: &str) -> Option<String> {
|
|||||||
output.strip_prefix(PENDING_USER_ACTION_MARKER).map(|rest| rest.trim().to_string())
|
output.strip_prefix(PENDING_USER_ACTION_MARKER).map(|rest| rest.trim().to_string())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn normalize_tool_arguments(arguments: &serde_json::Value) -> serde_json::Value {
|
||||||
|
match arguments {
|
||||||
|
serde_json::Value::String(raw) => {
|
||||||
|
serde_json::from_str(raw).unwrap_or_else(|_| arguments.clone())
|
||||||
|
}
|
||||||
|
_ => arguments.clone(),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fn format_error_chain(error: &(dyn std::error::Error + 'static)) -> String {
|
||||||
|
let mut details = vec![error.to_string()];
|
||||||
|
let mut current = error.source();
|
||||||
|
|
||||||
|
while let Some(source) = current {
|
||||||
|
details.push(source.to_string());
|
||||||
|
current = source.source();
|
||||||
|
}
|
||||||
|
|
||||||
|
details.join("\ncaused by: ")
|
||||||
|
}
|
||||||
|
|
||||||
fn is_recoverable_llm_error(error: &str) -> bool {
|
fn is_recoverable_llm_error(error: &str) -> bool {
|
||||||
let normalized = error.to_ascii_lowercase();
|
let normalized = error.to_ascii_lowercase();
|
||||||
normalized.contains("504")
|
normalized.contains("504")
|
||||||
@ -433,7 +454,13 @@ impl AgentLoop {
|
|||||||
let response = match (*self.provider).chat(request).await {
|
let response = match (*self.provider).chat(request).await {
|
||||||
Ok(response) => response,
|
Ok(response) => response,
|
||||||
Err(e) => {
|
Err(e) => {
|
||||||
tracing::error!(error = %e, "LLM request failed");
|
tracing::error!(
|
||||||
|
provider = %self.provider.name(),
|
||||||
|
model = %self.provider.model_id(),
|
||||||
|
error = %e,
|
||||||
|
error_details = %format_error_chain(e.as_ref()),
|
||||||
|
"LLM request failed"
|
||||||
|
);
|
||||||
let assistant_message = ChatMessage::assistant(recoverable_llm_message(&e.to_string()));
|
let assistant_message = ChatMessage::assistant(recoverable_llm_message(&e.to_string()));
|
||||||
emitted_messages.push(assistant_message.clone());
|
emitted_messages.push(assistant_message.clone());
|
||||||
return Ok(AgentProcessResult {
|
return Ok(AgentProcessResult {
|
||||||
@ -609,7 +636,13 @@ impl AgentLoop {
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
Err(e) => {
|
Err(e) => {
|
||||||
tracing::error!(error = %e, "Failed to get summary from LLM");
|
tracing::error!(
|
||||||
|
provider = %self.provider.name(),
|
||||||
|
model = %self.provider.model_id(),
|
||||||
|
error = %e,
|
||||||
|
error_details = %format_error_chain(e.as_ref()),
|
||||||
|
"Failed to get summary from LLM"
|
||||||
|
);
|
||||||
let final_message = ChatMessage::assistant(recoverable_llm_message(&e.to_string()));
|
let final_message = ChatMessage::assistant(recoverable_llm_message(&e.to_string()));
|
||||||
emitted_messages.push(final_message.clone());
|
emitted_messages.push(final_message.clone());
|
||||||
Ok(AgentProcessResult {
|
Ok(AgentProcessResult {
|
||||||
@ -720,8 +753,10 @@ impl AgentLoop {
|
|||||||
|
|
||||||
/// Internal tool execution without event tracking.
|
/// Internal tool execution without event tracking.
|
||||||
async fn execute_tool_internal(&self, tool_call: &ToolCall) -> ToolExecutionOutcome {
|
async fn execute_tool_internal(&self, tool_call: &ToolCall) -> ToolExecutionOutcome {
|
||||||
|
let normalized_arguments = normalize_tool_arguments(&tool_call.arguments);
|
||||||
|
|
||||||
if tool_call.name == "skill_activate" {
|
if tool_call.name == "skill_activate" {
|
||||||
let skill_name = match tool_call.arguments.get("name").and_then(|v| v.as_str()) {
|
let skill_name = match normalized_arguments.get("name").and_then(|v| v.as_str()) {
|
||||||
Some(name) if !name.trim().is_empty() => name,
|
Some(name) if !name.trim().is_empty() => name,
|
||||||
_ => {
|
_ => {
|
||||||
self.record_skill_event(
|
self.record_skill_event(
|
||||||
@ -729,7 +764,7 @@ impl AgentLoop {
|
|||||||
None,
|
None,
|
||||||
serde_json::json!({
|
serde_json::json!({
|
||||||
"reason": "missing_name",
|
"reason": "missing_name",
|
||||||
"arguments": tool_call.arguments,
|
"arguments": normalized_arguments,
|
||||||
}),
|
}),
|
||||||
);
|
);
|
||||||
return ToolExecutionOutcome::failure(
|
return ToolExecutionOutcome::failure(
|
||||||
@ -752,7 +787,7 @@ impl AgentLoop {
|
|||||||
Some(skill_name),
|
Some(skill_name),
|
||||||
serde_json::json!({
|
serde_json::json!({
|
||||||
"reason": err,
|
"reason": err,
|
||||||
"arguments": tool_call.arguments,
|
"arguments": normalized_arguments,
|
||||||
}),
|
}),
|
||||||
);
|
);
|
||||||
ToolExecutionOutcome::failure(
|
ToolExecutionOutcome::failure(
|
||||||
@ -774,7 +809,7 @@ impl AgentLoop {
|
|||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
match tool.execute_with_context(&self.tool_context, tool_call.arguments.clone()).await {
|
match tool.execute_with_context(&self.tool_context, normalized_arguments.clone()).await {
|
||||||
Ok(result) => {
|
Ok(result) => {
|
||||||
if result.success {
|
if result.success {
|
||||||
if let Some(pending_output) = parse_pending_tool_output(&result.output) {
|
if let Some(pending_output) = parse_pending_tool_output(&result.output) {
|
||||||
@ -784,6 +819,14 @@ impl AgentLoop {
|
|||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
let error = result.error.unwrap_or_default();
|
let error = result.error.unwrap_or_default();
|
||||||
|
tracing::error!(
|
||||||
|
tool = %tool_call.name,
|
||||||
|
args = %truncate_args(&tool_call.arguments, 2_000),
|
||||||
|
normalized_args = %truncate_args(&normalized_arguments, 2_000),
|
||||||
|
error = %error,
|
||||||
|
output = %result.output,
|
||||||
|
"Tool returned an error result"
|
||||||
|
);
|
||||||
ToolExecutionOutcome::failure(
|
ToolExecutionOutcome::failure(
|
||||||
format!("Error: {}", error),
|
format!("Error: {}", error),
|
||||||
Some(error),
|
Some(error),
|
||||||
@ -791,7 +834,14 @@ impl AgentLoop {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
Err(e) => {
|
Err(e) => {
|
||||||
tracing::error!(tool = %tool_call.name, error = %e, "Tool execution failed");
|
tracing::error!(
|
||||||
|
tool = %tool_call.name,
|
||||||
|
args = %truncate_args(&tool_call.arguments, 2_000),
|
||||||
|
normalized_args = %truncate_args(&normalized_arguments, 2_000),
|
||||||
|
error = %e,
|
||||||
|
error_details = %format!("{:#}", e),
|
||||||
|
"Tool execution failed"
|
||||||
|
);
|
||||||
ToolExecutionOutcome::failure(
|
ToolExecutionOutcome::failure(
|
||||||
format!("Error: {}", e),
|
format!("Error: {}", e),
|
||||||
Some(e.to_string()),
|
Some(e.to_string()),
|
||||||
@ -940,6 +990,22 @@ mod tests {
|
|||||||
assert!(parse_pending_tool_output("normal output").is_none());
|
assert!(parse_pending_tool_output("normal output").is_none());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_normalize_tool_arguments_parses_stringified_json() {
|
||||||
|
let normalized = normalize_tool_arguments(&serde_json::Value::String(
|
||||||
|
"{\"command\":\"ls -la\"}".to_string(),
|
||||||
|
));
|
||||||
|
|
||||||
|
assert_eq!(normalized, serde_json::json!({ "command": "ls -la" }));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_normalize_tool_arguments_keeps_plain_string() {
|
||||||
|
let normalized = normalize_tool_arguments(&serde_json::Value::String("plain text".to_string()));
|
||||||
|
|
||||||
|
assert_eq!(normalized, serde_json::Value::String("plain text".to_string()));
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn test_build_content_blocks_skips_non_image_media_refs() {
|
fn test_build_content_blocks_skips_non_image_media_refs() {
|
||||||
let temp_dir = tempdir().unwrap();
|
let temp_dir = tempdir().unwrap();
|
||||||
|
|||||||
@ -8,6 +8,18 @@ use crate::bus::message::ContentBlock;
|
|||||||
use super::{ChatCompletionRequest, ChatCompletionResponse, LLMProvider, Tool, ToolCall};
|
use super::{ChatCompletionRequest, ChatCompletionResponse, LLMProvider, Tool, ToolCall};
|
||||||
use super::traits::Usage;
|
use super::traits::Usage;
|
||||||
|
|
||||||
|
fn format_error_chain(error: &(dyn std::error::Error + 'static)) -> String {
|
||||||
|
let mut details = vec![error.to_string()];
|
||||||
|
let mut current = error.source();
|
||||||
|
|
||||||
|
while let Some(source) = current {
|
||||||
|
details.push(source.to_string());
|
||||||
|
current = source.source();
|
||||||
|
}
|
||||||
|
|
||||||
|
details.join("\ncaused by: ")
|
||||||
|
}
|
||||||
|
|
||||||
fn serialize_content_blocks<S>(blocks: &[serde_json::Value], serializer: S) -> Result<S::Ok, S::Error>
|
fn serialize_content_blocks<S>(blocks: &[serde_json::Value], serializer: S) -> Result<S::Ok, S::Error>
|
||||||
where
|
where
|
||||||
S: serde::Serializer,
|
S: serde::Serializer,
|
||||||
@ -204,6 +216,15 @@ impl LLMProvider for AnthropicProvider {
|
|||||||
let text = resp.text().await?;
|
let text = resp.text().await?;
|
||||||
|
|
||||||
if !status.is_success() {
|
if !status.is_success() {
|
||||||
|
tracing::error!(
|
||||||
|
provider = %self.name,
|
||||||
|
model = %self.model_id,
|
||||||
|
url = %url,
|
||||||
|
status = %status,
|
||||||
|
response_len = text.len(),
|
||||||
|
response_body = %text,
|
||||||
|
"Anthropic API request failed"
|
||||||
|
);
|
||||||
return Err(format!("API error {}: {}", status, text).into());
|
return Err(format!("API error {}: {}", status, text).into());
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -213,8 +234,18 @@ impl LLMProvider for AnthropicProvider {
|
|||||||
tracing::debug!(status = %status, response_preview = %resp_preview, response_len = %text.len(), timeout_secs = self.llm_timeout_secs, "Anthropic response (first 100 chars shown)");
|
tracing::debug!(status = %status, response_preview = %resp_preview, response_len = %text.len(), timeout_secs = self.llm_timeout_secs, "Anthropic response (first 100 chars shown)");
|
||||||
}
|
}
|
||||||
|
|
||||||
let anthropic_resp: AnthropicResponse = serde_json::from_str(&text)
|
let anthropic_resp: AnthropicResponse = serde_json::from_str(&text).map_err(|e| {
|
||||||
.map_err(|e| format!("decode error: {} | body: {}", e, &text))?;
|
tracing::error!(
|
||||||
|
provider = %self.name,
|
||||||
|
model = %self.model_id,
|
||||||
|
url = %url,
|
||||||
|
error = %format_error_chain(&e),
|
||||||
|
response_len = text.len(),
|
||||||
|
response_body = %text,
|
||||||
|
"Failed to decode Anthropic response"
|
||||||
|
);
|
||||||
|
format!("decode error: {} | body: {}", e, &text)
|
||||||
|
})?;
|
||||||
|
|
||||||
let mut content = String::new();
|
let mut content = String::new();
|
||||||
let mut tool_calls = Vec::new();
|
let mut tool_calls = Vec::new();
|
||||||
|
|||||||
@ -14,6 +14,18 @@ const INTERNAL_MODEL_EXTRA_KEYS: &[&str] = &[
|
|||||||
"mock_response_content",
|
"mock_response_content",
|
||||||
];
|
];
|
||||||
|
|
||||||
|
fn format_error_chain(error: &(dyn std::error::Error + 'static)) -> String {
|
||||||
|
let mut details = vec![error.to_string()];
|
||||||
|
let mut current = error.source();
|
||||||
|
|
||||||
|
while let Some(source) = current {
|
||||||
|
details.push(source.to_string());
|
||||||
|
current = source.source();
|
||||||
|
}
|
||||||
|
|
||||||
|
details.join("\ncaused by: ")
|
||||||
|
}
|
||||||
|
|
||||||
fn convert_content_blocks(blocks: &[ContentBlock]) -> Value {
|
fn convert_content_blocks(blocks: &[ContentBlock]) -> Value {
|
||||||
if blocks.len() == 1 {
|
if blocks.len() == 1 {
|
||||||
if let ContentBlock::Text { text } = &blocks[0] {
|
if let ContentBlock::Text { text } = &blocks[0] {
|
||||||
@ -280,18 +292,37 @@ impl LLMProvider for OpenAIProvider {
|
|||||||
let text = resp.text().await?;
|
let text = resp.text().await?;
|
||||||
|
|
||||||
// Debug: Log LLM response (only in debug builds)
|
// Debug: Log LLM response (only in debug builds)
|
||||||
|
if !status.is_success() {
|
||||||
|
tracing::error!(
|
||||||
|
provider = %self.name,
|
||||||
|
model = %self.model_id,
|
||||||
|
url = %url,
|
||||||
|
status = %status,
|
||||||
|
response_len = text.len(),
|
||||||
|
response_body = %text,
|
||||||
|
"OpenAI-compatible API request failed"
|
||||||
|
);
|
||||||
|
return Err(format!("API error {}: {}", status, text).into());
|
||||||
|
}
|
||||||
|
|
||||||
#[cfg(debug_assertions)]
|
#[cfg(debug_assertions)]
|
||||||
{
|
{
|
||||||
let resp_preview: String = text.chars().take(100).collect();
|
let resp_preview: String = text.chars().take(100).collect();
|
||||||
tracing::debug!(status = %status, response_preview = %resp_preview, response_len = %text.len(), timeout_secs = self.llm_timeout_secs, "LLM response (first 100 chars shown)");
|
tracing::debug!(status = %status, response_preview = %resp_preview, response_len = %text.len(), timeout_secs = self.llm_timeout_secs, "LLM response (first 100 chars shown)");
|
||||||
}
|
}
|
||||||
|
|
||||||
if !status.is_success() {
|
let openai_resp: OpenAIResponse = serde_json::from_str(&text).map_err(|e| {
|
||||||
return Err(format!("API error {}: {}", status, text).into());
|
tracing::error!(
|
||||||
}
|
provider = %self.name,
|
||||||
|
model = %self.model_id,
|
||||||
let openai_resp: OpenAIResponse = serde_json::from_str(&text)
|
url = %url,
|
||||||
.map_err(|e| format!("decode error: {} | body: {}", e, &text))?;
|
error = %format_error_chain(&e),
|
||||||
|
response_len = text.len(),
|
||||||
|
response_body = %text,
|
||||||
|
"Failed to decode OpenAI-compatible API response"
|
||||||
|
);
|
||||||
|
format!("decode error: {} | body: {}", e, &text)
|
||||||
|
})?;
|
||||||
|
|
||||||
let content = openai_resp.choices[0]
|
let content = openai_resp.choices[0]
|
||||||
.message
|
.message
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user