fix(ai,agent,coding-agent): normalize null message content at ingestion boundaries (#6343)
The Message types require content to always be present, but untyped JS extension tools, hand-built histories, and old or hand-edited session files can violate that contract, crashing rendering, compaction, and provider request conversion with 'content is not iterable'. Normalize null/missing content to an empty array at the ingestion boundaries instead of guarding every consumer: - transformMessages (choke point before every provider request) - createToolResultMessage in the agent loop - session entry loading (message and custom_message entries) - extension custom messages (sendCustomMessage, before_agent_start) - message_end extension replacements fixes #6259, fixes #6276
This commit is contained in:
@@ -735,7 +735,9 @@ function createToolResultMessage(finalized: FinalizedToolCallOutcome): ToolResul
|
|||||||
role: "toolResult",
|
role: "toolResult",
|
||||||
toolCallId: finalized.toolCall.id,
|
toolCallId: finalized.toolCall.id,
|
||||||
toolName: finalized.toolCall.name,
|
toolName: finalized.toolCall.name,
|
||||||
content: finalized.result.content,
|
// Untyped tools (JS extensions) can return results without content; normalize
|
||||||
|
// so the null never enters session history or provider payloads.
|
||||||
|
content: finalized.result.content ?? [],
|
||||||
details: finalized.result.details,
|
details: finalized.result.details,
|
||||||
isError: finalized.isError,
|
isError: finalized.isError,
|
||||||
timestamp: Date.now(),
|
timestamp: Date.now(),
|
||||||
|
|||||||
@@ -68,7 +68,10 @@ export function transformMessages<TApi extends Api>(
|
|||||||
): Message[] {
|
): Message[] {
|
||||||
// Build a map of original tool call IDs to normalized IDs
|
// Build a map of original tool call IDs to normalized IDs
|
||||||
const toolCallIdMap = new Map<string, string>();
|
const toolCallIdMap = new Map<string, string>();
|
||||||
const imageAwareMessages = downgradeUnsupportedImages(messages, model);
|
// Normalize null/undefined content from untyped callers (custom tools, hand-built
|
||||||
|
// histories, old session files) so downstream code can rely on the type contract.
|
||||||
|
const normalizedMessages = messages.map((msg) => (msg.content == null ? { ...msg, content: [] } : msg));
|
||||||
|
const imageAwareMessages = downgradeUnsupportedImages(normalizedMessages, model);
|
||||||
|
|
||||||
// First pass: transform messages (unsupported image downgrade, thinking blocks, tool call ID normalization)
|
// First pass: transform messages (unsupported image downgrade, thinking blocks, tool call ID normalization)
|
||||||
const transformed = imageAwareMessages.map((msg) => {
|
const transformed = imageAwareMessages.map((msg) => {
|
||||||
|
|||||||
@@ -0,0 +1,67 @@
|
|||||||
|
/**
|
||||||
|
* The Message types require `content` to always be present, but untyped
|
||||||
|
* callers (custom tools, hand-built histories, old session files) can violate
|
||||||
|
* that contract. `transformMessages` is the choke point before every provider
|
||||||
|
* request and is intentionally lax: it normalizes null/missing content to an
|
||||||
|
* empty array (issues #6259, #6276).
|
||||||
|
*/
|
||||||
|
|
||||||
|
import { describe, expect, it } from "vitest";
|
||||||
|
import { transformMessages } from "../src/api/transform-messages.ts";
|
||||||
|
import type { Message, Model } from "../src/types.ts";
|
||||||
|
|
||||||
|
// Text-only model so the image downgrade path (replaceImagesWithPlaceholder) runs,
|
||||||
|
// which was the primary crash site for null tool result content.
|
||||||
|
function makeTextOnlyModel(): Model<"openai-completions"> {
|
||||||
|
return {
|
||||||
|
id: "test-model",
|
||||||
|
name: "Test Model",
|
||||||
|
api: "openai-completions",
|
||||||
|
provider: "openai",
|
||||||
|
baseUrl: "https://example.invalid/v1",
|
||||||
|
reasoning: false,
|
||||||
|
input: ["text"],
|
||||||
|
cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 },
|
||||||
|
contextWindow: 128000,
|
||||||
|
maxTokens: 16000,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
describe("lax message content handling", () => {
|
||||||
|
it("normalizes null/missing content to an empty array instead of crashing", () => {
|
||||||
|
const messages = [
|
||||||
|
{ role: "user", content: null, timestamp: Date.now() },
|
||||||
|
{
|
||||||
|
role: "assistant",
|
||||||
|
content: null,
|
||||||
|
api: "openai-completions",
|
||||||
|
provider: "openai",
|
||||||
|
model: "test-model",
|
||||||
|
usage: {
|
||||||
|
input: 0,
|
||||||
|
output: 0,
|
||||||
|
cacheRead: 0,
|
||||||
|
cacheWrite: 0,
|
||||||
|
totalTokens: 0,
|
||||||
|
cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 },
|
||||||
|
},
|
||||||
|
stopReason: "stop",
|
||||||
|
timestamp: Date.now(),
|
||||||
|
},
|
||||||
|
{
|
||||||
|
role: "toolResult",
|
||||||
|
toolCallId: "call_1",
|
||||||
|
toolName: "web_search",
|
||||||
|
isError: false,
|
||||||
|
timestamp: Date.now(),
|
||||||
|
},
|
||||||
|
] as unknown as Message[];
|
||||||
|
|
||||||
|
const result = transformMessages(messages, makeTextOnlyModel());
|
||||||
|
|
||||||
|
expect(result).toHaveLength(3);
|
||||||
|
for (const msg of result) {
|
||||||
|
expect(msg.content).toEqual([]);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -680,7 +680,17 @@ export class AgentSession {
|
|||||||
};
|
};
|
||||||
const replacement = await this._extensionRunner.emitMessageEnd(extensionEvent);
|
const replacement = await this._extensionRunner.emitMessageEnd(extensionEvent);
|
||||||
if (replacement) {
|
if (replacement) {
|
||||||
this._replaceMessageInPlace(event.message, replacement);
|
// Untyped extension handlers can return messages with null/missing content;
|
||||||
|
// normalize so it never enters agent state or session history.
|
||||||
|
const normalized =
|
||||||
|
(replacement.role === "user" ||
|
||||||
|
replacement.role === "assistant" ||
|
||||||
|
replacement.role === "toolResult" ||
|
||||||
|
replacement.role === "custom") &&
|
||||||
|
replacement.content == null
|
||||||
|
? ({ ...replacement, content: [] } as AgentMessage)
|
||||||
|
: replacement;
|
||||||
|
this._replaceMessageInPlace(event.message, normalized);
|
||||||
}
|
}
|
||||||
} else if (event.type === "tool_execution_start") {
|
} else if (event.type === "tool_execution_start") {
|
||||||
const extensionEvent: ToolExecutionStartEvent = {
|
const extensionEvent: ToolExecutionStartEvent = {
|
||||||
@@ -1143,7 +1153,8 @@ export class AgentSession {
|
|||||||
messages.push({
|
messages.push({
|
||||||
role: "custom",
|
role: "custom",
|
||||||
customType: msg.customType,
|
customType: msg.customType,
|
||||||
content: msg.content,
|
// Untyped extensions can pass null/missing content; normalize at ingestion.
|
||||||
|
content: msg.content ?? [],
|
||||||
display: msg.display,
|
display: msg.display,
|
||||||
details: msg.details,
|
details: msg.details,
|
||||||
timestamp: Date.now(),
|
timestamp: Date.now(),
|
||||||
@@ -1341,7 +1352,8 @@ export class AgentSession {
|
|||||||
const appMessage = {
|
const appMessage = {
|
||||||
role: "custom" as const,
|
role: "custom" as const,
|
||||||
customType: message.customType,
|
customType: message.customType,
|
||||||
content: message.content,
|
// Untyped extensions can pass null/missing content; normalize at ingestion.
|
||||||
|
content: message.content ?? [],
|
||||||
display: message.display,
|
display: message.display,
|
||||||
details: message.details,
|
details: message.details,
|
||||||
timestamp: Date.now(),
|
timestamp: Date.now(),
|
||||||
|
|||||||
@@ -378,10 +378,21 @@ function getSessionContextSettings(path: SessionEntry[]): Pick<SessionContext, "
|
|||||||
*/
|
*/
|
||||||
export function sessionEntryToContextMessages(entry: SessionEntry): AgentMessage[] {
|
export function sessionEntryToContextMessages(entry: SessionEntry): AgentMessage[] {
|
||||||
if (entry.type === "message") {
|
if (entry.type === "message") {
|
||||||
return [entry.message];
|
const message = entry.message;
|
||||||
|
// Session files are parsed without validation; old versions, forks, or
|
||||||
|
// hand-edited files can contain messages with null/missing content.
|
||||||
|
if (
|
||||||
|
(message.role === "user" || message.role === "assistant" || message.role === "toolResult") &&
|
||||||
|
message.content == null
|
||||||
|
) {
|
||||||
|
return [{ ...message, content: [] }];
|
||||||
|
}
|
||||||
|
return [message];
|
||||||
}
|
}
|
||||||
if (entry.type === "custom_message") {
|
if (entry.type === "custom_message") {
|
||||||
return [createCustomMessage(entry.customType, entry.content, entry.display, entry.details, entry.timestamp)];
|
return [
|
||||||
|
createCustomMessage(entry.customType, entry.content ?? [], entry.display, entry.details, entry.timestamp),
|
||||||
|
];
|
||||||
}
|
}
|
||||||
if (entry.type === "branch_summary" && entry.summary) {
|
if (entry.type === "branch_summary" && entry.summary) {
|
||||||
return [createBranchSummaryMessage(entry.summary, entry.fromId, entry.timestamp)];
|
return [createBranchSummaryMessage(entry.summary, entry.fromId, entry.timestamp)];
|
||||||
|
|||||||
@@ -0,0 +1,162 @@
|
|||||||
|
/**
|
||||||
|
* The Message types require `content` to always be present, but untyped JS
|
||||||
|
* extension tools, hand-built histories, and old or hand-edited session files
|
||||||
|
* can violate that contract. We are intentionally lax at the ingestion
|
||||||
|
* boundaries and normalize null/missing content to an empty array so it never
|
||||||
|
* reaches rendering, compaction, or provider request conversion
|
||||||
|
* (issues #6259, #6276).
|
||||||
|
*/
|
||||||
|
|
||||||
|
import type { AgentMessage, AgentToolResult } from "@earendil-works/pi-agent-core";
|
||||||
|
import { fauxAssistantMessage, fauxToolCall } from "@earendil-works/pi-ai";
|
||||||
|
import { Type } from "typebox";
|
||||||
|
import { describe, expect, it } from "vitest";
|
||||||
|
import { type SessionEntry, sessionEntryToContextMessages } from "../../src/core/session-manager.ts";
|
||||||
|
import type { ExtensionFactory } from "../../src/index.ts";
|
||||||
|
import { createHarness } from "./harness.ts";
|
||||||
|
|
||||||
|
function messageEntry(message: Record<string, unknown>): SessionEntry {
|
||||||
|
return {
|
||||||
|
type: "message",
|
||||||
|
id: "entry-1",
|
||||||
|
parentId: null,
|
||||||
|
timestamp: new Date().toISOString(),
|
||||||
|
message,
|
||||||
|
} as unknown as SessionEntry;
|
||||||
|
}
|
||||||
|
|
||||||
|
describe("lax message content handling", () => {
|
||||||
|
it("normalizes tool results from untyped tools that omit content", async () => {
|
||||||
|
const extensionFactories: ExtensionFactory[] = [
|
||||||
|
(pi) => {
|
||||||
|
pi.registerTool({
|
||||||
|
name: "web_search",
|
||||||
|
label: "Web Search",
|
||||||
|
description: "Custom tool that returns a result without content",
|
||||||
|
parameters: Type.Object({}),
|
||||||
|
// Simulate an untyped JS extension tool that omits content.
|
||||||
|
execute: async () => ({ details: {} }) as unknown as AgentToolResult<unknown>,
|
||||||
|
});
|
||||||
|
},
|
||||||
|
];
|
||||||
|
const harness = await createHarness({ extensionFactories });
|
||||||
|
|
||||||
|
try {
|
||||||
|
harness.setResponses([
|
||||||
|
fauxAssistantMessage(fauxToolCall("web_search", {}), { stopReason: "toolUse" }),
|
||||||
|
fauxAssistantMessage("done"),
|
||||||
|
]);
|
||||||
|
|
||||||
|
await harness.session.prompt("search something");
|
||||||
|
|
||||||
|
const toolResults = harness.session.messages.filter((message) => message.role === "toolResult");
|
||||||
|
expect(toolResults).toHaveLength(1);
|
||||||
|
expect(toolResults[0].content).toEqual([]);
|
||||||
|
// The follow-up turn consumed the normalized tool result without crashing.
|
||||||
|
expect(harness.getPendingResponseCount()).toBe(0);
|
||||||
|
} finally {
|
||||||
|
harness.cleanup();
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it("normalizes null content in message_end extension replacements", async () => {
|
||||||
|
const extensionFactories: ExtensionFactory[] = [
|
||||||
|
(pi) => {
|
||||||
|
pi.on("message_end", async (event) => {
|
||||||
|
if (event.message.role !== "assistant") return undefined;
|
||||||
|
// Simulate an untyped JS extension replacing a message without content.
|
||||||
|
return { message: { ...event.message, content: null } as unknown as AgentMessage };
|
||||||
|
});
|
||||||
|
},
|
||||||
|
];
|
||||||
|
const harness = await createHarness({ extensionFactories });
|
||||||
|
|
||||||
|
try {
|
||||||
|
harness.setResponses([fauxAssistantMessage("hello")]);
|
||||||
|
await harness.session.prompt("hi");
|
||||||
|
|
||||||
|
const assistantMessages = harness.session.messages.filter((message) => message.role === "assistant");
|
||||||
|
expect(assistantMessages).toHaveLength(1);
|
||||||
|
expect(assistantMessages[0].content).toEqual([]);
|
||||||
|
} finally {
|
||||||
|
harness.cleanup();
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it("normalizes null content in custom messages from extensions", async () => {
|
||||||
|
const harness = await createHarness();
|
||||||
|
|
||||||
|
try {
|
||||||
|
await harness.session.sendCustomMessage({
|
||||||
|
customType: "test",
|
||||||
|
content: null as unknown as string,
|
||||||
|
display: false,
|
||||||
|
details: undefined,
|
||||||
|
});
|
||||||
|
|
||||||
|
const customMessages = harness.session.messages.filter((message) => message.role === "custom");
|
||||||
|
expect(customMessages).toHaveLength(1);
|
||||||
|
expect(customMessages[0].content).toEqual([]);
|
||||||
|
} finally {
|
||||||
|
harness.cleanup();
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it("normalizes null or missing content when loading session message entries", () => {
|
||||||
|
const badMessages = [
|
||||||
|
{ role: "user", content: null, timestamp: Date.now() },
|
||||||
|
{
|
||||||
|
role: "assistant",
|
||||||
|
content: null,
|
||||||
|
api: "openai-completions",
|
||||||
|
provider: "openai",
|
||||||
|
model: "test-model",
|
||||||
|
usage: {
|
||||||
|
input: 0,
|
||||||
|
output: 0,
|
||||||
|
cacheRead: 0,
|
||||||
|
cacheWrite: 0,
|
||||||
|
totalTokens: 0,
|
||||||
|
cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 },
|
||||||
|
},
|
||||||
|
stopReason: "stop",
|
||||||
|
timestamp: Date.now(),
|
||||||
|
},
|
||||||
|
{
|
||||||
|
role: "toolResult",
|
||||||
|
toolCallId: "call_1",
|
||||||
|
toolName: "web_search",
|
||||||
|
isError: false,
|
||||||
|
timestamp: Date.now(),
|
||||||
|
},
|
||||||
|
];
|
||||||
|
|
||||||
|
for (const badMessage of badMessages) {
|
||||||
|
const [message] = sessionEntryToContextMessages(messageEntry(badMessage));
|
||||||
|
expect(message).toMatchObject({ role: badMessage.role, content: [] });
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it("normalizes null content when loading custom message entries", () => {
|
||||||
|
const entry = {
|
||||||
|
type: "custom_message",
|
||||||
|
id: "entry-1",
|
||||||
|
parentId: null,
|
||||||
|
timestamp: new Date().toISOString(),
|
||||||
|
customType: "test",
|
||||||
|
content: null,
|
||||||
|
display: false,
|
||||||
|
details: undefined,
|
||||||
|
} as unknown as SessionEntry;
|
||||||
|
|
||||||
|
const [message] = sessionEntryToContextMessages(entry);
|
||||||
|
expect(message).toMatchObject({ role: "custom", content: [] });
|
||||||
|
});
|
||||||
|
|
||||||
|
it("keeps valid message content untouched when loading session entries", () => {
|
||||||
|
const [message] = sessionEntryToContextMessages(
|
||||||
|
messageEntry({ role: "user", content: "hello", timestamp: Date.now() }),
|
||||||
|
);
|
||||||
|
expect(message).toMatchObject({ role: "user", content: "hello" });
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user