diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index 226d274b1..879ba0832 100644 --- a/pkg/agent/loop.go +++ b/pkg/agent/loop.go @@ -25,6 +25,7 @@ import ( "github.com/sipeed/picoclaw/pkg/logger" "github.com/sipeed/picoclaw/pkg/providers" "github.com/sipeed/picoclaw/pkg/routing" + "github.com/sipeed/picoclaw/pkg/session" "github.com/sipeed/picoclaw/pkg/skills" "github.com/sipeed/picoclaw/pkg/state" "github.com/sipeed/picoclaw/pkg/stats" @@ -712,6 +713,20 @@ func (al *AgentLoop) runAgentLoop(ctx context.Context, agent *AgentInstance, opt if !opts.NoHistory { history = agent.Sessions.GetHistory(opts.SessionKey) summary = agent.Sessions.GetSummary(opts.SessionKey) + + // Sanitize history to remove orphaned tool calls (from crashes/session collisions) + var removedCount int + history, removedCount = session.SanitizeHistory(history) + if removedCount > 0 { + logger.WarnCF("agent", "Sanitized session history: removed orphaned messages", + map[string]any{ + "session_key": opts.SessionKey, + "removed_count": removedCount, + }) + // Persist the sanitized history + agent.Sessions.SetHistory(opts.SessionKey, history) + _ = agent.Sessions.Save(opts.SessionKey) + } } messages := agent.ContextBuilder.BuildMessages( history, diff --git a/pkg/session/manager.go b/pkg/session/manager.go index 08f0b0ad2..f625b69ed 100644 --- a/pkg/session/manager.go +++ b/pkg/session/manager.go @@ -265,6 +265,77 @@ func (sm *SessionManager) loadSessions() error { return nil } +// SanitizeHistory removes orphaned tool calls from session history. +// An orphaned tool call is an assistant message containing ToolCalls where +// one or more call IDs have no matching tool-result message (role="tool") +// following it. This can happen if the process crashed mid-execution. +// The function trims incomplete assistant+tool-result groups from the tail. +// Returns the sanitized history and the number of messages removed. +func SanitizeHistory(history []providers.Message) ([]providers.Message, int) { + if len(history) == 0 { + return history, 0 + } + + original := len(history) + + // Walk backwards from the tail, trimming incomplete tool-call groups. + for len(history) > 0 { + last := history[len(history)-1] + + // If tail is a tool result, find its parent assistant message and check completeness + if last.Role == "tool" { + // Find the nearest preceding assistant message with tool calls + assistantIdx := -1 + for i := len(history) - 2; i >= 0; i-- { + if history[i].Role == "assistant" && len(history[i].ToolCalls) > 0 { + assistantIdx = i + break + } + } + + if assistantIdx < 0 { + // Orphaned tool result with no assistant — remove it + history = history[:len(history)-1] + continue + } + + // Collect all expected tool call IDs from the assistant message + expected := make(map[string]bool) + for _, tc := range history[assistantIdx].ToolCalls { + expected[tc.ID] = true + } + + // Check how many results exist between assistant and end of history + for i := assistantIdx + 1; i < len(history); i++ { + if history[i].Role == "tool" && expected[history[i].ToolCallID] { + delete(expected, history[i].ToolCallID) + } + } + + if len(expected) > 0 { + // Incomplete group — remove everything from assistantIdx onward + history = history[:assistantIdx] + continue + } + + // Group is complete, we're done + break + } + + // If tail is an assistant with tool calls, check if ALL results follow + if last.Role == "assistant" && len(last.ToolCalls) > 0 { + // No tool results follow at all — orphaned + history = history[:len(history)-1] + continue + } + + // Tail is a normal message (user, assistant without tools) — we're done + break + } + + return history, original - len(history) +} + // SetHistory updates the messages of a session. func (sm *SessionManager) SetHistory(key string, history []providers.Message) { sm.mu.Lock() diff --git a/pkg/session/manager_test.go b/pkg/session/manager_test.go index 5ef5f4349..9c5543003 100644 --- a/pkg/session/manager_test.go +++ b/pkg/session/manager_test.go @@ -4,6 +4,8 @@ import ( "os" "path/filepath" "testing" + + "github.com/sipeed/picoclaw/pkg/providers" ) func TestSanitizeFilename(t *testing.T) { @@ -60,6 +62,55 @@ func TestSave_WithColonInKey(t *testing.T) { } } +func TestSanitizeHistory_OrphanedToolCall(t *testing.T) { + history := []providers.Message{ + {Role: "user", Content: "hello"}, + {Role: "assistant", Content: "sure", ToolCalls: []providers.ToolCall{ + {ID: "call_1", Name: "exec"}, + {ID: "call_2", Name: "list_dir"}, + }}, + {Role: "tool", Content: "ok", ToolCallID: "call_1"}, + // Missing tool result for call_2 → orphaned + } + + sanitized, removed := SanitizeHistory(history) + // The orphaned assistant msg (with call_2 missing) and the trailing tool result + // should both be removed, leaving just the user message + if removed == 0 { + t.Fatal("expected orphaned messages to be removed") + } + // After sanitization, only the user message should remain + if len(sanitized) != 1 || sanitized[0].Role != "user" { + t.Errorf("expected [user], got %d messages: %v", len(sanitized), sanitized) + } +} + +func TestSanitizeHistory_CleanHistory(t *testing.T) { + history := []providers.Message{ + {Role: "user", Content: "hello"}, + {Role: "assistant", Content: "sure", ToolCalls: []providers.ToolCall{ + {ID: "call_1", Name: "exec"}, + }}, + {Role: "tool", Content: "ok", ToolCallID: "call_1"}, + {Role: "assistant", Content: "done"}, + } + + sanitized, removed := SanitizeHistory(history) + if removed != 0 { + t.Errorf("expected 0 removed, got %d", removed) + } + if len(sanitized) != 4 { + t.Errorf("expected 4 messages, got %d", len(sanitized)) + } +} + +func TestSanitizeHistory_Empty(t *testing.T) { + sanitized, removed := SanitizeHistory(nil) + if removed != 0 || sanitized != nil { + t.Errorf("expected nil/0, got %v/%d", sanitized, removed) + } +} + func TestSave_RejectsPathTraversal(t *testing.T) { tmpDir := t.TempDir() sm := NewSessionManager(tmpDir)