Compare commits

...

3 Commits

11 changed files with 298 additions and 69 deletions

View File

@ -933,6 +933,21 @@ impl AgentLoop {
Some(tool_defs)
};
// Defense-in-depth: sanitize incomplete tool call sequences
// before EVERY LLM request, not just once at process() entry.
// This catches edge cases where compression, persistence races,
// or delta message merging may have introduced orphaned sequences
// that survived the initial sanitization.
let mid_loop_removed =
crate::bus::message::sanitize_incomplete_tool_call_sequences(&mut messages);
if mid_loop_removed > 0 {
tracing::warn!(
iteration = iteration,
removed_count = mid_loop_removed,
"Mid-loop sanitize removed incomplete tool call sequences before LLM request"
);
}
// 过滤超出轮次和数量限制的图片
let filtered_messages = filter_images_by_age_and_count(
&messages,
@ -2244,6 +2259,69 @@ mod tests {
assert_eq!(messages[4].content, "task 2");
assert_eq!(messages[5].content, "task 3");
}
#[test]
fn test_sanitize_removes_orphaned_tool_results_without_any_assistant_tool_calls() {
// After heavy compaction, all assistant tool_calls messages may be
// summarized into plain text, leaving only orphaned tool results.
// Phase 2 must still clean them even when no assistant has tool_calls.
let mut messages = vec![
ChatMessage::user("do something"),
ChatMessage::assistant("I used a tool to help you"), // plain text, no tool_calls
ChatMessage::tool("orphan_call", "search", "some result"), // orphaned!
ChatMessage::user("thanks"),
ChatMessage::assistant("you're welcome"),
];
let removed = crate::bus::message::sanitize_incomplete_tool_call_sequences(&mut messages);
assert_eq!(removed, 1, "should remove the orphaned tool result");
assert_eq!(messages.len(), 4);
assert_eq!(messages[0].role, "user");
assert_eq!(messages[1].role, "assistant");
assert_eq!(messages[2].role, "user");
assert_eq!(messages[2].content, "thanks");
assert_eq!(messages[3].role, "assistant");
}
#[test]
fn test_sanitize_handles_empty_tool_calls_vector() {
// An assistant message with tool_calls: Some(vec![]) — edge case
// that shouldn't crash or cause API errors.
let mut msg = ChatMessage::assistant("thinking...");
msg.tool_calls = Some(vec![]); // Some but empty
let mut messages = vec![
ChatMessage::user("hello"),
msg,
ChatMessage::assistant("here is my answer"),
];
let removed = crate::bus::message::sanitize_incomplete_tool_call_sequences(&mut messages);
assert_eq!(removed, 0, "empty tool_calls vec should not be removed");
assert_eq!(messages.len(), 3);
}
#[test]
fn test_sanitize_removes_multiple_orphaned_tool_results_after_compaction() {
// Simulates a scenario where compaction summarized all tool call
// sequences, leaving multiple orphaned tool results scattered.
let mut messages = vec![
ChatMessage::user("task 1"),
ChatMessage::assistant("completed task 1"), // summary text, no tool_calls
ChatMessage::tool("old_call_1", "read", "file contents"), // orphaned
ChatMessage::user("task 2"),
ChatMessage::assistant("completed task 2"), // summary text
ChatMessage::tool("old_call_2", "bash", "command output"), // orphaned
ChatMessage::user("task 3"),
ChatMessage::assistant("final answer"),
];
let removed = crate::bus::message::sanitize_incomplete_tool_call_sequences(&mut messages);
assert_eq!(removed, 2, "should remove both orphaned tool results");
assert_eq!(messages.len(), 6);
// Verify no tool messages remain
assert!(messages.iter().all(|m| m.role != "tool"));
}
}
#[derive(Debug)]

View File

@ -325,7 +325,12 @@ pub(crate) fn sanitize_incomplete_tool_call_sequences(messages: &mut Vec<ChatMes
// Phase 2: Forward pass to remove ALL orphaned tool messages (not just
// trailing ones). A tool message is orphaned if its tool_call_id has no
// matching parent assistant remaining in the history.
if !with_parent.is_empty() || !resolved_ids.is_empty() {
//
// Always execute this pass unconditionally — even when Phase 1 found no
// assistant messages with tool_calls (e.g., after heavy compaction that
// summarized all tool_call sequences into text), there may still be
// orphaned tool result messages in the history that must be cleaned up.
{
let mut i = 0;
while i < messages.len() {
let msg = &messages[i];

View File

@ -219,7 +219,6 @@ impl Default for InChatCommandRouter {
#[cfg(test)]
mod tests {
use super::*;
use uuid::Uuid;
struct TestHandler;

View File

@ -112,12 +112,6 @@ impl SaveSessionCommandHandler {
system_prompt_provider,
}
}
/// 从会话记录获取存储(用于测试)
#[cfg(test)]
fn store(&self) -> &Arc<SessionStore> {
&self.store
}
}
#[async_trait]

View File

@ -103,7 +103,6 @@ impl SystemPromptProvider for SimpleAgentPromptProvider {
#[cfg(test)]
mod tests {
use super::*;
use crate::storage::SessionStore;
use std::collections::HashMap;
fn test_config() -> LLMProviderConfig {

View File

@ -13,6 +13,7 @@ impl CliSessionService {
Self { store }
}
#[allow(dead_code)]
pub(crate) fn create(&self, title: Option<&str>) -> Result<SessionRecord, AgentError> {
self.store
.create_cli_session(title)

View File

@ -270,7 +270,7 @@ impl MemoryMaintenanceService {
)))
}
#[cfg_attr(not(test), allow(dead_code))]
#[allow(dead_code)]
pub(crate) async fn run_for_scope(
&self,
scope_key: &str,

View File

@ -93,6 +93,7 @@ impl ToolRegistryFactory {
}
/// Get a reference to the shell session manager for lifecycle control.
#[allow(dead_code)]
pub(crate) fn shell_session_manager(&self) -> Arc<ShellSessionManager> {
self.shell_session_manager.clone()
}

View File

@ -631,22 +631,89 @@ impl OpenAIProvider {
fn build_request_body(&self, request: &ChatCompletionRequest) -> Value {
let supports_images = self.supports_images();
// --- Final defense: validate tool_call / tool result pairing ---
// Collect all tool_call_ids that have a corresponding tool result message.
// Any assistant tool_call NOT in this set is orphaned and must be stripped
// to avoid API 400 errors ("insufficient tool messages following tool_calls").
let mut resolved_tool_ids: std::collections::HashSet<&str> = std::collections::HashSet::new();
for m in &request.messages {
if m.role == "tool" {
if let Some(ref tc_id) = m.tool_call_id {
resolved_tool_ids.insert(tc_id.as_str());
}
}
}
// Build the set of assistant tool_call_ids that are fully valid
// (ALL tool_calls in the message have corresponding results).
// If an assistant has partial or no valid tool_calls, we strip the
// tool_calls field and serialize it as a plain assistant message.
let mut valid_tool_call_parent_ids: std::collections::HashSet<&str> = std::collections::HashSet::new();
for m in &request.messages {
if m.role == "assistant" {
if let Some(ref calls) = m.tool_calls {
if !calls.is_empty()
&& calls.iter().all(|tc| resolved_tool_ids.contains(tc.id.as_str()))
{
for tc in calls {
valid_tool_call_parent_ids.insert(tc.id.as_str());
}
}
}
}
}
let mut body = json!({
"model": self.model_id,
"messages": request.messages.iter().enumerate().map(|(i, m)| {
"messages": request.messages.iter().enumerate().filter_map(|(i, m)| {
if m.role == "tool" {
json!({
// Skip orphaned tool results (no matching assistant tool_call)
let is_orphaned = match &m.tool_call_id {
Some(tc_id) => !valid_tool_call_parent_ids.contains(tc_id.as_str()),
None => true,
};
if is_orphaned {
tracing::warn!(
tool_call_id = ?m.tool_call_id,
message_index = i,
"build_request_body: skipping orphaned tool result message"
);
return None;
}
Some(json!({
"role": m.role,
"content": convert_content_blocks(supports_images, &self.name, &self.model_id, &m.content, i),
"tool_call_id": m.tool_call_id,
"name": m.name,
})
}))
} else if m.role == "assistant" && m.tool_calls.is_some() {
let mut message = json!({
"role": m.role,
"content": convert_content_blocks(supports_images, &self.name, &self.model_id, &m.content, i),
"tool_calls": m.tool_calls.as_ref().map(|calls| {
calls.iter().map(|call| json!({
let calls = m.tool_calls.as_ref().unwrap();
// Filter to only valid tool_calls (all have results)
let valid_calls: Vec<&ToolCall> = calls.iter()
.filter(|tc| valid_tool_call_parent_ids.contains(tc.id.as_str()))
.collect();
if valid_calls.is_empty() {
// All tool_calls are orphaned — serialize as plain assistant message
tracing::warn!(
orphaned_tool_call_count = calls.len(),
message_index = i,
"build_request_body: stripping all orphaned tool_calls from assistant message"
);
let mut message = json!({
"role": m.role,
"content": convert_content_blocks(supports_images, &self.name, &self.model_id, &m.content, i)
});
if let Some(reasoning_content) = &m.reasoning_content {
message["reasoning_content"] = Value::String(reasoning_content.clone());
}
Some(message)
} else {
let mut message = json!({
"role": m.role,
"content": convert_content_blocks(supports_images, &self.name, &self.model_id, &m.content, i),
"tool_calls": valid_calls.iter().map(|call| json!({
"id": call.id,
"type": "function",
"function": {
@ -654,14 +721,14 @@ impl OpenAIProvider {
"arguments": self.serialize_tool_arguments(&call.arguments)
}
})).collect::<Vec<_>>()
})
});
});
if let Some(reasoning_content) = &m.reasoning_content {
message["reasoning_content"] = Value::String(reasoning_content.clone());
if let Some(reasoning_content) = &m.reasoning_content {
message["reasoning_content"] = Value::String(reasoning_content.clone());
}
Some(message)
}
message
} else {
let mut message = json!({
"role": m.role,
@ -674,7 +741,7 @@ impl OpenAIProvider {
}
}
message
Some(message)
}
}).collect::<Vec<_>>(),
});
@ -970,18 +1037,28 @@ mod tests {
);
let request = ChatCompletionRequest {
messages: vec![Message {
role: "assistant".to_string(),
content: vec![ContentBlock::text("calling tool")],
reasoning_content: None,
tool_call_id: None,
name: None,
tool_calls: Some(vec![ToolCall {
id: "call_1".to_string(),
name: "calculator".to_string(),
arguments: json!({"expression": "1+1"}),
}]),
}],
messages: vec![
Message {
role: "assistant".to_string(),
content: vec![ContentBlock::text("calling tool")],
reasoning_content: None,
tool_call_id: None,
name: None,
tool_calls: Some(vec![ToolCall {
id: "call_1".to_string(),
name: "calculator".to_string(),
arguments: json!({"expression": "1+1"}),
}]),
},
Message {
role: "tool".to_string(),
content: vec![ContentBlock::text("2")],
reasoning_content: None,
tool_call_id: Some("call_1".to_string()),
name: Some("calculator".to_string()),
tool_calls: None,
},
],
temperature: None,
max_tokens: None,
tools: None,
@ -1016,18 +1093,28 @@ mod tests {
);
let request = ChatCompletionRequest {
messages: vec![Message {
role: "assistant".to_string(),
content: vec![ContentBlock::text("calling tool")],
reasoning_content: None,
tool_call_id: None,
name: None,
tool_calls: Some(vec![ToolCall {
id: "call_1".to_string(),
name: "calculator".to_string(),
arguments: json!({"expression": "1+1"}),
}]),
}],
messages: vec![
Message {
role: "assistant".to_string(),
content: vec![ContentBlock::text("calling tool")],
reasoning_content: None,
tool_call_id: None,
name: None,
tool_calls: Some(vec![ToolCall {
id: "call_1".to_string(),
name: "calculator".to_string(),
arguments: json!({"expression": "1+1"}),
}]),
},
Message {
role: "tool".to_string(),
content: vec![ContentBlock::text("2")],
reasoning_content: None,
tool_call_id: Some("call_1".to_string()),
name: Some("calculator".to_string()),
tool_calls: None,
},
],
temperature: None,
max_tokens: None,
tools: None,
@ -1059,18 +1146,28 @@ mod tests {
);
let request = ChatCompletionRequest {
messages: vec![Message {
role: "assistant".to_string(),
content: vec![ContentBlock::text("calling tool")],
reasoning_content: None,
tool_call_id: None,
name: None,
tool_calls: Some(vec![ToolCall {
id: "call_1".to_string(),
name: "calculator".to_string(),
arguments: Value::String("{\"expression\":\"1+1\"}".to_string()),
}]),
}],
messages: vec![
Message {
role: "assistant".to_string(),
content: vec![ContentBlock::text("calling tool")],
reasoning_content: None,
tool_call_id: None,
name: None,
tool_calls: Some(vec![ToolCall {
id: "call_1".to_string(),
name: "calculator".to_string(),
arguments: Value::String("{\"expression\":\"1+1\"}".to_string()),
}]),
},
Message {
role: "tool".to_string(),
content: vec![ContentBlock::text("2")],
reasoning_content: None,
tool_call_id: Some("call_1".to_string()),
name: Some("calculator".to_string()),
tool_calls: None,
},
],
temperature: None,
max_tokens: None,
tools: None,

View File

@ -1493,7 +1493,7 @@ impl SessionStore {
// Insert new todos
for item in items {
conn.execute(
"INSERT INTO todos (id, scope_key, session_id, topic_id, content, status, priority, created_at, updated_at)
"INSERT OR REPLACE INTO todos (id, scope_key, session_id, topic_id, content, status, priority, created_at, updated_at)
VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9)",
params![
item.id,
@ -1904,7 +1904,7 @@ fn ensure_todos_schema(conn: &Connection) -> Result<(), StorageError> {
conn.execute_batch(
"
CREATE TABLE IF NOT EXISTS todos (
id TEXT PRIMARY KEY,
id TEXT NOT NULL,
scope_key TEXT NOT NULL,
session_id TEXT NOT NULL,
topic_id TEXT,
@ -1912,7 +1912,8 @@ fn ensure_todos_schema(conn: &Connection) -> Result<(), StorageError> {
status TEXT NOT NULL DEFAULT 'pending',
priority TEXT NOT NULL DEFAULT 'medium',
created_at INTEGER NOT NULL,
updated_at INTEGER NOT NULL
updated_at INTEGER NOT NULL,
PRIMARY KEY (id, scope_key)
);
CREATE INDEX IF NOT EXISTS idx_todos_scope
@ -1922,6 +1923,55 @@ fn ensure_todos_schema(conn: &Connection) -> Result<(), StorageError> {
ON todos(session_id);
",
)?;
return Ok(());
}
// Migration: check if old schema has single-column PRIMARY KEY on `id`
// If so, migrate to composite PRIMARY KEY (id, scope_key)
let sql: String = conn
.query_row(
"SELECT sql FROM sqlite_master WHERE type='table' AND name='todos'",
[],
|row| row.get::<_, String>(0),
)
.unwrap_or_default();
let needs_migration = sql.contains("id TEXT PRIMARY KEY")
|| (sql.contains("PRIMARY KEY") && !sql.contains("PRIMARY KEY (id, scope_key)"));
if needs_migration {
tracing::info!("Migrating todos table to composite PRIMARY KEY (id, scope_key)");
conn.execute_batch(
"
CREATE TABLE todos_new (
id TEXT NOT NULL,
scope_key TEXT NOT NULL,
session_id TEXT NOT NULL,
topic_id TEXT,
content TEXT NOT NULL,
status TEXT NOT NULL DEFAULT 'pending',
priority TEXT NOT NULL DEFAULT 'medium',
created_at INTEGER NOT NULL,
updated_at INTEGER NOT NULL,
PRIMARY KEY (id, scope_key)
);
INSERT OR IGNORE INTO todos_new
SELECT id, scope_key, session_id, topic_id, content, status, priority, created_at, updated_at
FROM todos;
DROP TABLE todos;
ALTER TABLE todos_new RENAME TO todos;
CREATE INDEX IF NOT EXISTS idx_todos_scope
ON todos(scope_key, created_at ASC);
CREATE INDEX IF NOT EXISTS idx_todos_session
ON todos(session_id);
",
)?;
tracing::info!("Todos table migration complete");
}
Ok(())

View File

@ -165,6 +165,11 @@ export function ConfigPage({ onClose, onSaveConnection }: ConfigPageProps) {
const [toast, setToast] = useState('')
const [dirty, setDirty] = useState(false)
const handleClose = useCallback(() => {
if (dirty && !confirm('有未保存的更改,确定要关闭吗?')) return
onClose()
}, [dirty, onClose])
// Load config
useEffect(() => {
fetch('/api/config').then(r => r.json()).then(data => {
@ -175,10 +180,10 @@ export function ConfigPage({ onClose, onSaveConnection }: ConfigPageProps) {
// ESC to close
useEffect(() => {
const h = (e: KeyboardEvent) => { if (e.key === 'Escape') onClose() }
const h = (e: KeyboardEvent) => { if (e.key === 'Escape') handleClose() }
document.addEventListener('keydown', h)
return () => document.removeEventListener('keydown', h)
}, [onClose])
}, [handleClose])
const update = useCallback(<K extends keyof AppConfig>(key: K, value: AppConfig[K]) => {
setConfig(prev => prev ? { ...prev, [key]: value } : prev)
@ -589,7 +594,7 @@ export function ConfigPage({ onClose, onSaveConnection }: ConfigPageProps) {
}
return (
<div className="fixed inset-0 z-50 flex items-center justify-center bg-black/80 backdrop-blur-sm animate-[fadeIn_0.15s_ease-out]" onClick={onClose}>
<div className="fixed inset-0 z-50 flex items-center justify-center bg-black/80 backdrop-blur-sm animate-[fadeIn_0.15s_ease-out]" onClick={handleClose}>
<div
className="relative flex flex-col w-[92vw] max-w-4xl h-[85vh] rounded-2xl border border-[var(--border-color)] bg-[var(--bg-primary)] shadow-2xl overflow-hidden animate-[scaleIn_0.2s_ease-out]"
onClick={e => e.stopPropagation()}
@ -600,7 +605,7 @@ export function ConfigPage({ onClose, onSaveConnection }: ConfigPageProps) {
<span className="text-lg font-semibold text-[var(--text-primary)]"></span>
{dirty && <span className="text-xs text-amber-400 bg-amber-500/10 px-2 py-0.5 rounded-full"></span>}
<div className="flex-1" />
<button onClick={onClose} className="p-2 rounded-lg text-[var(--text-muted)] hover:text-[var(--text-primary)] hover:bg-[var(--overlay-hover)] transition-colors" title="关闭 (Esc)">
<button onClick={handleClose} className="p-2 rounded-lg text-[var(--text-muted)] hover:text-[var(--text-primary)] hover:bg-[var(--overlay-hover)] transition-colors" title="关闭 (Esc)">
<X className="h-5 w-5" />
</button>
</div>
@ -640,7 +645,7 @@ export function ConfigPage({ onClose, onSaveConnection }: ConfigPageProps) {
<div className="shrink-0 px-6 py-3 border-t border-[var(--border-color)] bg-[var(--bg-secondary)]/80 backdrop-blur-md flex items-center gap-3">
{error && <span className="text-sm text-red-400 truncate max-w-xs">{error}</span>}
<div className="flex-1" />
<button onClick={onClose} className="px-4 py-2 rounded-lg text-sm text-[var(--text-secondary)] hover:text-[var(--text-primary)] hover:bg-[var(--overlay-hover)] transition-colors">
<button onClick={handleClose} className="px-4 py-2 rounded-lg text-sm text-[var(--text-secondary)] hover:text-[var(--text-primary)] hover:bg-[var(--overlay-hover)] transition-colors">
</button>
<button