diff --git a/docs/reference/transcript-hygiene.md b/docs/reference/transcript-hygiene.md index 686a381be7..364c8db07f 100644 --- a/docs/reference/transcript-hygiene.md +++ b/docs/reference/transcript-hygiene.md @@ -11,11 +11,15 @@ title: "Transcript Hygiene" This document describes **provider-specific fixes** applied to transcripts before a run (building model context). These are **in-memory** adjustments used to satisfy strict -provider requirements. They do **not** rewrite the stored JSONL transcript on disk. +provider requirements. These hygiene steps do **not** rewrite the stored JSONL transcript +on disk; however, a separate session-file repair pass may rewrite malformed JSONL files +by dropping invalid lines before the session is loaded. When a repair occurs, the original +file is backed up alongside the session file. Scope includes: - Tool call id sanitization +- Tool call input validation (drop malformed tool_use/tool_call blocks missing input or arguments) - Tool result pairing repair - Turn validation / ordering - Thought signature cleanup @@ -36,6 +40,11 @@ All transcript hygiene is centralized in the embedded runner: The policy uses `provider`, `modelApi`, and `modelId` to decide what to apply. +Separate from transcript hygiene, session files are repaired (if needed) before load: + +- `repairSessionFileIfNeeded` in `src/agents/session-file-repair.ts` +- Called from `run/attempt.ts` and `compact.ts` (embedded runner) + --- ## Global rule: image sanitization @@ -50,6 +59,19 @@ Implementation: --- +## Global rule: malformed tool calls + +Assistant tool-call blocks that are missing both `input` and `arguments` are dropped +before model context is built. This prevents provider rejections from partially +persisted tool calls (for example, after a rate limit failure). + +Implementation: + +- `sanitizeToolCallInputs` in `src/agents/session-transcript-repair.ts` +- Applied in `sanitizeSessionHistory` in `src/agents/pi-embedded-runner/google.ts` + +--- + ## Provider matrix (current behavior) **OpenAI / OpenAI Codex** diff --git a/src/agents/pi-embedded-helpers.formatassistanterrortext.test.ts b/src/agents/pi-embedded-helpers.formatassistanterrortext.test.ts index 874cb68b4f..55ca283882 100644 --- a/src/agents/pi-embedded-helpers.formatassistanterrortext.test.ts +++ b/src/agents/pi-embedded-helpers.formatassistanterrortext.test.ts @@ -35,6 +35,12 @@ describe("formatAssistantErrorText", () => { "The AI service is temporarily overloaded. Please try again in a moment.", ); }); + it("returns a recovery hint when tool call input is missing", () => { + const msg = makeAssistantError("tool_use.input: Field required"); + const result = formatAssistantErrorText(msg); + expect(result).toContain("Session history looks corrupted"); + expect(result).toContain("/new"); + }); it("handles JSON-wrapped role errors", () => { const msg = makeAssistantError('{"error":{"message":"400 Incorrect role information"}}'); const result = formatAssistantErrorText(msg); diff --git a/src/agents/pi-embedded-helpers/errors.ts b/src/agents/pi-embedded-helpers/errors.ts index 0df9ac02a2..c230f0fd7c 100644 --- a/src/agents/pi-embedded-helpers/errors.ts +++ b/src/agents/pi-embedded-helpers/errors.ts @@ -351,6 +351,14 @@ export function formatAssistantErrorText( ); } + if (isMissingToolCallInputError(raw)) { + return ( + "Session history looks corrupted (tool call input missing). " + + "Use /new to start a fresh session. " + + "If this keeps happening, reset the session or delete the corrupted session transcript." + ); + } + const invalidRequest = raw.match(/"type":"invalid_request_error".*?"message":"([^"]+)"/); if (invalidRequest?.[1]) { return `LLM request rejected: ${invalidRequest[1]}`; @@ -465,6 +473,11 @@ const ERROR_PATTERNS = { ], } as const; +const TOOL_CALL_INPUT_MISSING_RE = + /tool_(?:use|call)\.(?:input|arguments).*?(?:field required|required)/i; +const TOOL_CALL_INPUT_PATH_RE = + /messages\.\d+\.content\.\d+\.tool_(?:use|call)\.(?:input|arguments)/i; + const IMAGE_DIMENSION_ERROR_RE = /image dimensions exceed max allowed size for many-image requests:\s*(\d+)\s*pixels/i; const IMAGE_DIMENSION_PATH_RE = /messages\.(\d+)\.content\.(\d+)\.image/i; @@ -505,6 +518,13 @@ export function isBillingErrorMessage(raw: string): boolean { ); } +export function isMissingToolCallInputError(raw: string): boolean { + if (!raw) { + return false; + } + return TOOL_CALL_INPUT_MISSING_RE.test(raw) || TOOL_CALL_INPUT_PATH_RE.test(raw); +} + export function isBillingAssistantError(msg: AssistantMessage | undefined): boolean { if (!msg || msg.stopReason !== "error") { return false; diff --git a/src/agents/pi-embedded-runner.sanitize-session-history.test.ts b/src/agents/pi-embedded-runner.sanitize-session-history.test.ts index b428c3328a..6ee05837bf 100644 --- a/src/agents/pi-embedded-runner.sanitize-session-history.test.ts +++ b/src/agents/pi-embedded-runner.sanitize-session-history.test.ts @@ -162,6 +162,26 @@ describe("sanitizeSessionHistory", () => { expect(result[0]?.role).toBe("assistant"); }); + it("drops malformed tool calls missing input or arguments", async () => { + const messages: AgentMessage[] = [ + { + role: "assistant", + content: [{ type: "toolCall", id: "call_1", name: "read" }], + }, + { role: "user", content: "hello" }, + ]; + + const result = await sanitizeSessionHistory({ + messages, + modelApi: "openai-responses", + provider: "openai", + sessionManager: mockSessionManager, + sessionId: "test-session", + }); + + expect(result.map((msg) => msg.role)).toEqual(["user"]); + }); + it("does not downgrade openai reasoning when the model has not changed", async () => { const sessionEntries: Array<{ type: string; customType: string; data: unknown }> = [ { diff --git a/src/agents/pi-embedded-runner/compact.ts b/src/agents/pi-embedded-runner/compact.ts index 57a30fbdff..1e75ce2f47 100644 --- a/src/agents/pi-embedded-runner/compact.ts +++ b/src/agents/pi-embedded-runner/compact.ts @@ -42,6 +42,7 @@ import { } from "../pi-settings.js"; import { createOpenClawCodingTools } from "../pi-tools.js"; import { resolveSandboxContext } from "../sandbox.js"; +import { repairSessionFileIfNeeded } from "../session-file-repair.js"; import { guardSessionManager } from "../session-tool-result-guard-wrapper.js"; import { acquireSessionWriteLock } from "../session-write-lock.js"; import { @@ -357,6 +358,10 @@ export async function compactEmbeddedPiSessionDirect( sessionFile: params.sessionFile, }); try { + await repairSessionFileIfNeeded({ + sessionFile: params.sessionFile, + warn: (message) => log.warn(message), + }); await prewarmSessionFile(params.sessionFile); const transcriptPolicy = resolveTranscriptPolicy({ modelApi: model.api, diff --git a/src/agents/pi-embedded-runner/google.ts b/src/agents/pi-embedded-runner/google.ts index 5bfdaf8662..03383622bd 100644 --- a/src/agents/pi-embedded-runner/google.ts +++ b/src/agents/pi-embedded-runner/google.ts @@ -12,7 +12,10 @@ import { sanitizeSessionMessagesImages, } from "../pi-embedded-helpers.js"; import { cleanToolSchemaForGemini } from "../pi-tools.schema.js"; -import { sanitizeToolUseResultPairing } from "../session-transcript-repair.js"; +import { + sanitizeToolCallInputs, + sanitizeToolUseResultPairing, +} from "../session-transcript-repair.js"; import { resolveTranscriptPolicy } from "../transcript-policy.js"; import { log } from "./logger.js"; import { describeUnknownError } from "./utils.js"; @@ -346,9 +349,10 @@ export async function sanitizeSessionHistory(params: { const sanitizedThinking = policy.normalizeAntigravityThinkingBlocks ? sanitizeAntigravityThinkingBlocks(sanitizedImages) : sanitizedImages; + const sanitizedToolCalls = sanitizeToolCallInputs(sanitizedThinking); const repairedTools = policy.repairToolUseResultPairing - ? sanitizeToolUseResultPairing(sanitizedThinking) - : sanitizedThinking; + ? sanitizeToolUseResultPairing(sanitizedToolCalls) + : sanitizedToolCalls; const isOpenAIResponsesApi = params.modelApi === "openai-responses" || params.modelApi === "openai-codex-responses"; diff --git a/src/agents/pi-embedded-runner/run/attempt.ts b/src/agents/pi-embedded-runner/run/attempt.ts index 03dfd11d2c..692f501405 100644 --- a/src/agents/pi-embedded-runner/run/attempt.ts +++ b/src/agents/pi-embedded-runner/run/attempt.ts @@ -46,6 +46,7 @@ import { toClientToolDefinitions } from "../../pi-tool-definition-adapter.js"; import { createOpenClawCodingTools } from "../../pi-tools.js"; import { resolveSandboxContext } from "../../sandbox.js"; import { resolveSandboxRuntimeStatus } from "../../sandbox/runtime-status.js"; +import { repairSessionFileIfNeeded } from "../../session-file-repair.js"; import { guardSessionManager } from "../../session-tool-result-guard-wrapper.js"; import { acquireSessionWriteLock } from "../../session-write-lock.js"; import { @@ -399,6 +400,10 @@ export async function runEmbeddedAttempt( let sessionManager: ReturnType | undefined; let session: Awaited>["session"] | undefined; try { + await repairSessionFileIfNeeded({ + sessionFile: params.sessionFile, + warn: (message) => log.warn(message), + }); const hadSessionFile = await fs .stat(params.sessionFile) .then(() => true) diff --git a/src/agents/session-file-repair.test.ts b/src/agents/session-file-repair.test.ts new file mode 100644 index 0000000000..9606031199 --- /dev/null +++ b/src/agents/session-file-repair.test.ts @@ -0,0 +1,42 @@ +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { describe, expect, it } from "vitest"; +import { repairSessionFileIfNeeded } from "./session-file-repair.js"; + +describe("repairSessionFileIfNeeded", () => { + it("rewrites session files that contain malformed lines", async () => { + const dir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-session-repair-")); + const file = path.join(dir, "session.jsonl"); + const header = { + type: "session", + version: 7, + id: "session-1", + timestamp: new Date().toISOString(), + cwd: "/tmp", + }; + const message = { + type: "message", + id: "msg-1", + parentId: null, + timestamp: new Date().toISOString(), + message: { role: "user", content: "hello" }, + }; + + const content = `${JSON.stringify(header)}\n${JSON.stringify(message)}\n{"type":"message"`; + await fs.writeFile(file, content, "utf-8"); + + const result = await repairSessionFileIfNeeded({ sessionFile: file }); + expect(result.repaired).toBe(true); + expect(result.droppedLines).toBe(1); + expect(result.backupPath).toBeTruthy(); + + const repaired = await fs.readFile(file, "utf-8"); + expect(repaired.trim().split("\n")).toHaveLength(2); + + if (result.backupPath) { + const backup = await fs.readFile(result.backupPath, "utf-8"); + expect(backup).toBe(content); + } + }); +}); diff --git a/src/agents/session-file-repair.ts b/src/agents/session-file-repair.ts new file mode 100644 index 0000000000..568858c516 --- /dev/null +++ b/src/agents/session-file-repair.ts @@ -0,0 +1,96 @@ +import fs from "node:fs/promises"; +import path from "node:path"; + +type RepairReport = { + repaired: boolean; + droppedLines: number; + backupPath?: string; + reason?: string; +}; + +function isSessionHeader(entry: unknown): entry is { type: string; id: string } { + if (!entry || typeof entry !== "object") { + return false; + } + const record = entry as { type?: unknown; id?: unknown }; + return record.type === "session" && typeof record.id === "string" && record.id.length > 0; +} + +export async function repairSessionFileIfNeeded(params: { + sessionFile: string; + warn?: (message: string) => void; +}): Promise { + const sessionFile = params.sessionFile.trim(); + if (!sessionFile) { + return { repaired: false, droppedLines: 0, reason: "missing session file" }; + } + + let content: string; + try { + content = await fs.readFile(sessionFile, "utf-8"); + } catch { + return { repaired: false, droppedLines: 0, reason: "missing session file" }; + } + + const lines = content.split("\n"); + const entries: unknown[] = []; + let droppedLines = 0; + + for (const line of lines) { + if (!line.trim()) { + continue; + } + try { + const entry = JSON.parse(line); + entries.push(entry); + } catch { + droppedLines += 1; + } + } + + if (entries.length === 0) { + return { repaired: false, droppedLines, reason: "empty session file" }; + } + + if (!isSessionHeader(entries[0])) { + return { repaired: false, droppedLines, reason: "invalid session header" }; + } + + if (droppedLines === 0) { + return { repaired: false, droppedLines: 0 }; + } + + const cleaned = `${entries.map((entry) => JSON.stringify(entry)).join("\n")}\n`; + const backupPath = `${sessionFile}.bak-${process.pid}-${Date.now()}`; + const tmpPath = `${sessionFile}.repair-${process.pid}-${Date.now()}.tmp`; + try { + const stat = await fs.stat(sessionFile).catch(() => null); + await fs.writeFile(backupPath, content, "utf-8"); + if (stat) { + await fs.chmod(backupPath, stat.mode); + } + await fs.writeFile(tmpPath, cleaned, "utf-8"); + if (stat) { + await fs.chmod(tmpPath, stat.mode); + } + await fs.rename(tmpPath, sessionFile); + } catch (err) { + try { + await fs.unlink(tmpPath); + } catch { + // ignore cleanup failures + } + return { + repaired: false, + droppedLines, + reason: `repair failed: ${err instanceof Error ? err.message : "unknown error"}`, + }; + } + + params.warn?.( + `session file repaired: dropped ${droppedLines} malformed line(s) (${path.basename( + sessionFile, + )})`, + ); + return { repaired: true, droppedLines, backupPath }; +} diff --git a/src/agents/session-tool-result-guard.test.ts b/src/agents/session-tool-result-guard.test.ts index 65a51cfb40..b5780b2d05 100644 --- a/src/agents/session-tool-result-guard.test.ts +++ b/src/agents/session-tool-result-guard.test.ts @@ -141,4 +141,21 @@ describe("installSessionToolResultGuard", () => { .map((e) => (e as { message: AgentMessage }).message); expect(messages.map((m) => m.role)).toEqual(["assistant", "toolResult"]); }); + + it("drops malformed tool calls missing input before persistence", () => { + const sm = SessionManager.inMemory(); + installSessionToolResultGuard(sm); + + sm.appendMessage({ + role: "assistant", + content: [{ type: "toolCall", id: "call_1", name: "read" }], + } as AgentMessage); + + const messages = sm + .getEntries() + .filter((e) => e.type === "message") + .map((e) => (e as { message: AgentMessage }).message); + + expect(messages).toHaveLength(0); + }); }); diff --git a/src/agents/session-tool-result-guard.ts b/src/agents/session-tool-result-guard.ts index 44d4cf13c3..86ae9b0402 100644 --- a/src/agents/session-tool-result-guard.ts +++ b/src/agents/session-tool-result-guard.ts @@ -1,7 +1,7 @@ import type { AgentMessage } from "@mariozechner/pi-agent-core"; import type { SessionManager } from "@mariozechner/pi-coding-agent"; import { emitSessionTranscriptUpdate } from "../sessions/transcript-events.js"; -import { makeMissingToolResult } from "./session-transcript-repair.js"; +import { makeMissingToolResult, sanitizeToolCallInputs } from "./session-transcript-repair.js"; type ToolCall = { id: string; name?: string }; @@ -96,16 +96,25 @@ export function installSessionToolResultGuard( }; const guardedAppend = (message: AgentMessage) => { + let nextMessage = message; const role = (message as { role?: unknown }).role; + if (role === "assistant") { + const sanitized = sanitizeToolCallInputs([message]); + if (sanitized.length === 0) { + return undefined; + } + nextMessage = sanitized[0] as AgentMessage; + } + const nextRole = (nextMessage as { role?: unknown }).role; - if (role === "toolResult") { - const id = extractToolResultId(message as Extract); + if (nextRole === "toolResult") { + const id = extractToolResultId(nextMessage as Extract); const toolName = id ? pending.get(id) : undefined; if (id) { pending.delete(id); } return originalAppend( - persistToolResult(message, { + persistToolResult(nextMessage, { toolCallId: id ?? undefined, toolName, isSynthetic: false, @@ -114,13 +123,13 @@ export function installSessionToolResultGuard( } const toolCalls = - role === "assistant" - ? extractAssistantToolCalls(message as Extract) + nextRole === "assistant" + ? extractAssistantToolCalls(nextMessage as Extract) : []; if (allowSyntheticToolResults) { // If previous tool calls are still pending, flush before non-tool results. - if (pending.size > 0 && (toolCalls.length === 0 || role !== "assistant")) { + if (pending.size > 0 && (toolCalls.length === 0 || nextRole !== "assistant")) { flushPendingToolResults(); } // If new tool calls arrive while older ones are pending, flush the old ones first. @@ -129,7 +138,7 @@ export function installSessionToolResultGuard( } } - const result = originalAppend(message as never); + const result = originalAppend(nextMessage as never); const sessionFile = ( sessionManager as { getSessionFile?: () => string | null } diff --git a/src/agents/session-transcript-repair.test.ts b/src/agents/session-transcript-repair.test.ts index ccc63ec7f1..7607f86f1f 100644 --- a/src/agents/session-transcript-repair.test.ts +++ b/src/agents/session-transcript-repair.test.ts @@ -1,6 +1,9 @@ import type { AgentMessage } from "@mariozechner/pi-agent-core"; import { describe, expect, it } from "vitest"; -import { sanitizeToolUseResultPairing } from "./session-transcript-repair.js"; +import { + sanitizeToolCallInputs, + sanitizeToolUseResultPairing, +} from "./session-transcript-repair.js"; describe("sanitizeToolUseResultPairing", () => { it("moves tool results directly after tool calls and inserts missing results", () => { @@ -110,3 +113,38 @@ describe("sanitizeToolUseResultPairing", () => { expect(out.map((m) => m.role)).toEqual(["user", "assistant"]); }); }); + +describe("sanitizeToolCallInputs", () => { + it("drops tool calls missing input or arguments", () => { + const input: AgentMessage[] = [ + { + role: "assistant", + content: [{ type: "toolCall", id: "call_1", name: "read" }], + }, + { role: "user", content: "hello" }, + ]; + + const out = sanitizeToolCallInputs(input); + expect(out.map((m) => m.role)).toEqual(["user"]); + }); + + it("keeps valid tool calls and preserves text blocks", () => { + const input: AgentMessage[] = [ + { + role: "assistant", + content: [ + { type: "text", text: "before" }, + { type: "toolUse", id: "call_ok", name: "read", input: { path: "a" } }, + { type: "toolCall", id: "call_drop", name: "read" }, + ], + }, + ]; + + const out = sanitizeToolCallInputs(input); + const assistant = out[0] as Extract; + const types = Array.isArray(assistant.content) + ? assistant.content.map((block) => (block as { type?: unknown }).type) + : []; + expect(types).toEqual(["text", "toolUse"]); + }); +}); diff --git a/src/agents/session-transcript-repair.ts b/src/agents/session-transcript-repair.ts index af93b1e5ce..1a0339d208 100644 --- a/src/agents/session-transcript-repair.ts +++ b/src/agents/session-transcript-repair.ts @@ -5,6 +5,16 @@ type ToolCallLike = { name?: string; }; +const TOOL_CALL_TYPES = new Set(["toolCall", "toolUse", "functionCall"]); + +type ToolCallBlock = { + type?: unknown; + id?: unknown; + name?: unknown; + input?: unknown; + arguments?: unknown; +}; + function extractToolCallsFromAssistant( msg: Extract, ): ToolCallLike[] { @@ -33,6 +43,21 @@ function extractToolCallsFromAssistant( return toolCalls; } +function isToolCallBlock(block: unknown): block is ToolCallBlock { + if (!block || typeof block !== "object") { + return false; + } + const type = (block as { type?: unknown }).type; + return typeof type === "string" && TOOL_CALL_TYPES.has(type); +} + +function hasToolCallInput(block: ToolCallBlock): boolean { + const hasInput = "input" in block ? block.input !== undefined && block.input !== null : false; + const hasArguments = + "arguments" in block ? block.arguments !== undefined && block.arguments !== null : false; + return hasInput || hasArguments; +} + function extractToolResultId(msg: Extract): string | null { const toolCallId = (msg as { toolCallId?: unknown }).toolCallId; if (typeof toolCallId === "string" && toolCallId) { @@ -66,6 +91,67 @@ function makeMissingToolResult(params: { export { makeMissingToolResult }; +export type ToolCallInputRepairReport = { + messages: AgentMessage[]; + droppedToolCalls: number; + droppedAssistantMessages: number; +}; + +export function repairToolCallInputs(messages: AgentMessage[]): ToolCallInputRepairReport { + let droppedToolCalls = 0; + let droppedAssistantMessages = 0; + let changed = false; + const out: AgentMessage[] = []; + + for (const msg of messages) { + if (!msg || typeof msg !== "object") { + out.push(msg); + continue; + } + + if (msg.role !== "assistant" || !Array.isArray(msg.content)) { + out.push(msg); + continue; + } + + const assistant = msg as Extract; + const nextContent = []; + let droppedInMessage = 0; + + for (const block of assistant.content) { + if (isToolCallBlock(block) && !hasToolCallInput(block)) { + droppedToolCalls += 1; + droppedInMessage += 1; + changed = true; + continue; + } + nextContent.push(block); + } + + if (droppedInMessage > 0) { + if (nextContent.length === 0) { + droppedAssistantMessages += 1; + changed = true; + continue; + } + out.push({ ...assistant, content: nextContent }); + continue; + } + + out.push(msg); + } + + return { + messages: changed ? out : messages, + droppedToolCalls, + droppedAssistantMessages, + }; +} + +export function sanitizeToolCallInputs(messages: AgentMessage[]): AgentMessage[] { + return repairToolCallInputs(messages).messages; +} + export function sanitizeToolUseResultPairing(messages: AgentMessage[]): AgentMessage[] { return repairToolUseResultPairing(messages).messages; }