fix: sanitize session history to remove orphaned tool calls
When the process crashes mid-tool-execution (or from previous session collision bugs), the session history can end up with assistant messages containing tool calls that have no matching tool results. APIs like Codex strictly reject this with "No tool output found for function call". Added SanitizeHistory() that trims incomplete tool-call groups from the tail of session history on load. If orphaned messages are found, the cleaned history is persisted back to disk. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
93681f3cf2
commit
634647a25a
3 changed files with 137 additions and 0 deletions
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue