diff --git a/core/util/messageConversion.ts b/core/util/messageConversion.ts index 0712b4d90d5..97c2997137b 100644 --- a/core/util/messageConversion.ts +++ b/core/util/messageConversion.ts @@ -355,6 +355,17 @@ export function convertFromUnifiedHistory( /** * Convert ChatHistoryItem array to ChatCompletionMessageParam array with injected system message + * + * Many OpenAI-compatible providers require the system message to be the very + * first message in the array. The history can contain additional system + * messages (e.g. notifications added via `addSystemMessage`, compaction notices, + * or a system message already stored at `history[0]` in loaded/remote sessions). + * If those were emitted inline, the resulting array would have a system message + * at a non-zero index and the provider would reject the request with a + * "System message must be at the beginning" 400 error (notably surfaced after + * cancelling a tool call, which triggers a follow-up request). To avoid this we + * merge all system content into a single system message positioned at index 0. + * * @param historyItems - The chat history items * @param systemMessage - The system message to inject at the beginning */ @@ -362,17 +373,35 @@ export function convertFromUnifiedHistoryWithSystemMessage( historyItems: ChatHistoryItem[], systemMessage: string, ): ChatCompletionMessageParam[] { + const convertedMessages = convertFromUnifiedHistory(historyItems); + + // Collect system content (the injected message plus any system messages that + // ended up inside the history) and keep the remaining messages in order. + const systemContents: string[] = []; + if (systemMessage.trim()) { + systemContents.push(systemMessage); + } + + const nonSystemMessages: ChatCompletionMessageParam[] = []; + for (const message of convertedMessages) { + if (message.role === "system") { + if (typeof message.content === "string" && message.content.trim()) { + systemContents.push(message.content); + } + continue; + } + nonSystemMessages.push(message); + } + const messages: ChatCompletionMessageParam[] = []; - // Inject system message at the beginning + // Always inject a single system message at the beginning messages.push({ role: "system", - content: systemMessage, + content: systemContents.join("\n\n"), }); - // Convert the rest of the history - const convertedMessages = convertFromUnifiedHistory(historyItems); - messages.push(...convertedMessages); + messages.push(...nonSystemMessages); return messages; } diff --git a/extensions/cli/src/messageConversion.test.ts b/extensions/cli/src/messageConversion.test.ts index 45e7de2c263..b0638a41bd0 100644 --- a/extensions/cli/src/messageConversion.test.ts +++ b/extensions/cli/src/messageConversion.test.ts @@ -1,5 +1,8 @@ import type { ChatHistoryItem } from "core/index.js"; -import { convertFromUnifiedHistory } from "core/util/messageConversion.js"; +import { + convertFromUnifiedHistory, + convertFromUnifiedHistoryWithSystemMessage, +} from "core/util/messageConversion.js"; describe("convertFromUnifiedHistory", () => { it("should expand contextItems into user message content", () => { @@ -139,3 +142,124 @@ describe("convertFromUnifiedHistory", () => { }); }); }); + +describe("convertFromUnifiedHistoryWithSystemMessage", () => { + it("injects the system message at index 0", () => { + const historyItems: ChatHistoryItem[] = [ + { message: { role: "user", content: "Hello" }, contextItems: [] }, + ]; + + const result = convertFromUnifiedHistoryWithSystemMessage( + historyItems, + "You are a helpful assistant", + ); + + expect(result[0]).toEqual({ + role: "system", + content: "You are a helpful assistant", + }); + expect(result).toHaveLength(2); + expect(result[1].role).toBe("user"); + }); + + it("does not leave a duplicate system message when history already starts with one", () => { + const historyItems: ChatHistoryItem[] = [ + { + message: { role: "system", content: "Stored system" }, + contextItems: [], + }, + { message: { role: "user", content: "Hello" }, contextItems: [] }, + ]; + + const result = convertFromUnifiedHistoryWithSystemMessage( + historyItems, + "Injected system", + ); + + const systemMessages = result.filter((m) => m.role === "system"); + expect(systemMessages).toHaveLength(1); + expect(result[0]).toEqual({ + role: "system", + content: "Injected system\n\nStored system", + }); + // No system message should appear anywhere except index 0 + result.slice(1).forEach((m) => expect(m.role).not.toBe("system")); + }); + + it("hoists a mid-conversation system message so it is not sent at a non-zero index (tool cancellation follow-up)", () => { + // Reproduces the "System message must be at the beginning" 400 error: + // a tool call is cancelled and a system notification lives mid-history. + const historyItems: ChatHistoryItem[] = [ + { + message: { role: "user", content: "Run the command" }, + contextItems: [], + }, + { + message: { + role: "assistant", + content: "", + toolCalls: [ + { + id: "tc-1", + type: "function", + function: { name: "run", arguments: "{}" }, + }, + ], + }, + contextItems: [], + toolCallStates: [ + { + toolCallId: "tc-1", + toolCall: { + id: "tc-1", + type: "function", + function: { name: "run", arguments: "{}" }, + }, + status: "canceled", + parsedArgs: {}, + }, + ], + }, + { + message: { + role: "system", + content: "Chat history auto-compacted successfully.", + }, + contextItems: [], + }, + ]; + + const result = convertFromUnifiedHistoryWithSystemMessage( + historyItems, + "Base system", + ); + + // Only one system message, and it is first + expect(result[0].role).toBe("system"); + result.slice(1).forEach((m) => expect(m.role).not.toBe("system")); + expect(result[0]).toEqual({ + role: "system", + content: "Base system\n\nChat history auto-compacted successfully.", + }); + // The cancelled tool call still produces a tool result message + expect(result.some((m) => m.role === "tool")).toBe(true); + }); + + it("emits a leading system message even when base system message is empty but history has one", () => { + const historyItems: ChatHistoryItem[] = [ + { + message: { role: "system", content: "Only stored system" }, + contextItems: [], + }, + { message: { role: "user", content: "Hi" }, contextItems: [] }, + ]; + + const result = convertFromUnifiedHistoryWithSystemMessage(historyItems, ""); + + expect(result[0]).toEqual({ + role: "system", + content: "Only stored system", + }); + expect(result.filter((m) => m.role === "system")).toHaveLength(1); + }); +});