Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 34 additions & 5 deletions core/util/messageConversion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -355,24 +355,53 @@ 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
*/
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;
}
Expand Down
126 changes: 125 additions & 1 deletion extensions/cli/src/messageConversion.test.ts
Original file line number Diff line number Diff line change
@@ -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", () => {
Expand Down Expand Up @@ -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);
});
});
Loading