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
21 changes: 16 additions & 5 deletions extensions/cli/src/services/ChatHistoryService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import type { ChatHistoryItem, ToolStatus } from "core/index.js";
import { createHistoryItem } from "core/util/messageConversion.js";

import { loadSessionById, updateSessionHistory } from "../session.js";
import type { ChatHistoryItemWithType } from "../uiNotices.js";
import { logger } from "../util/logger.js";

import { BaseService } from "./BaseService.js";
Expand Down Expand Up @@ -205,13 +206,23 @@ export class ChatHistoryService extends BaseService<ChatHistoryState> {
}

/**
* Add a system message to the history
* Add a system message to the history.
*
* Everything that reaches this method is a notice addressed to the user —
* "Remote environment is shutting down…", `WARNING: Tool … requires
* permission`, command output, diffs — so the item is marked as a UI notice
* and is dropped before the history is converted to wire format. Callers that
* ever need to add a real system *instruction* should build the item
* themselves rather than reach for this.
*/
addSystemMessage(content: string): ChatHistoryItem {
const newMessage = createHistoryItem({
role: "system",
content,
});
const newMessage: ChatHistoryItemWithType = {
...createHistoryItem({
role: "system",
content,
}),
messageType: "system",
};

const newHistory = [...this.currentState.history, newMessage];
this.setHistoryInternal(newHistory);
Expand Down
17 changes: 3 additions & 14 deletions extensions/cli/src/stream/streamChatResponse.autoCompaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
shouldAutoCompact,
} from "../compaction.js";
import { updateSessionHistory } from "../session.js";
import { uiNotice } from "../uiNotices.js";
import { formatError } from "../util/formatError.js";
import { logger } from "../util/logger.js";

Expand Down Expand Up @@ -74,13 +75,7 @@ function handleCompactionSuccess(
callbacks.setCompactionIndex(result.compactionIndex);
callbacks.setMessages((prev: ChatHistoryItem[]) => [
...prev,
{
message: {
role: "system",
content: successMessage,
},
contextItems: [],
},
uiNotice(successMessage),
]);
}
}
Expand All @@ -105,13 +100,7 @@ function handleCompactionError(
} else if (callbacks?.setMessages) {
callbacks.setMessages((prev: ChatHistoryItem[]) => [
...prev,
{
message: {
role: "system",
content: warningMessage,
},
contextItems: [],
},
uiNotice(warningMessage),
]);
}
}
Expand Down
107 changes: 107 additions & 0 deletions extensions/cli/src/stream/streamChatResponse.systemMessage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import type { ChatHistoryItem } from "core/index.js";
import { beforeEach, describe, expect, it, vi } from "vitest";

import { services } from "../services/index.js";
import { uiNotice } from "../uiNotices.js";

import { processStreamingResponse } from "./streamChatResponse.js";

Expand Down Expand Up @@ -160,6 +161,112 @@ describe("streamChatResponse system message validation", () => {
expect(services.systemMessage.getSystemMessage).not.toHaveBeenCalled();
});

it("should not send UI notices to the provider as extra system messages", async () => {
const model = {
model: "test-model",
defaultCompletionOptions: {
contextLength: 1000,
maxTokens: 100,
},
} as ModelConfig;

const systemMessage = "System instructions";

// A `/model` notice recorded in the history. It is role:"system" for
// rendering, and carries the messageType discriminator from
// spec/wire-format.md marking it as display-only.
const chatHistory: ChatHistoryItem[] = [
{ message: { role: "user", content: "hello" }, contextItems: [] },
{
message: { role: "system", content: "Switched to model: gpt-4o" },
contextItems: [],
messageType: "system",
} as ChatHistoryItem,
{ message: { role: "user", content: "and now?" }, contextItems: [] },
];

const mockStream = {
[Symbol.asyncIterator]: async function* () {
yield { choices: [{ delta: { content: "ok" } }] };
},
};
mockLlmApi.chatCompletionStream = vi.fn().mockResolvedValue(mockStream);

await processStreamingResponse({
chatHistory,
model,
llmApi: mockLlmApi,
abortController,
systemMessage,
});

const sent = vi.mocked(mockLlmApi.chatCompletionStream).mock.calls[0]?.[0];
const systemMessages = sent!.messages.filter((m) => m.role === "system");

// Exactly one system message, and it is first — otherwise providers reject
// the request with "System message must be at the beginning".
expect(systemMessages).toHaveLength(1);
expect(sent!.messages[0]?.role).toBe("system");
expect(sent!.messages[0]?.content).toBe(systemMessage);
expect(
sent!.messages.some((m) =>
typeof m.content === "string"
? m.content.includes("Switched to model")
: false,
),
).toBe(false);
});

it("should not send notices from the non-selector sites either", async () => {
const model = {
model: "test-model",
defaultCompletionOptions: { contextLength: 1000, maxTokens: 100 },
} as ModelConfig;

const systemMessage = "System instructions";

// The shape reported in #13026: an error banner sitting next to a /model
// notice. The banner comes from useChat.ts, a site that does not go through
// the selector hooks, so it is only filtered because every notice site now
// builds its item with uiNotice().
const chatHistory: ChatHistoryItem[] = [
{ message: { role: "user", content: "hello" }, contextItems: [] },
uiNotice("Switched to model: qwen/qwen3.6-35b-a3b") as ChatHistoryItem,
uiNotice("Error: Connection error.") as ChatHistoryItem,
uiNotice(
"[Tool canceled - please tell Continue what to do differently]",
) as ChatHistoryItem,
uiNotice("Chat history compacted successfully.") as ChatHistoryItem,
uiNotice("Session exported to /tmp/x.json") as ChatHistoryItem,
{ message: { role: "user", content: "and now?" }, contextItems: [] },
];

const mockStream = {
[Symbol.asyncIterator]: async function* () {
yield { choices: [{ delta: { content: "ok" } }] };
},
};
mockLlmApi.chatCompletionStream = vi.fn().mockResolvedValue(mockStream);

await processStreamingResponse({
chatHistory,
model,
llmApi: mockLlmApi,
abortController,
systemMessage,
});

const sent = vi.mocked(mockLlmApi.chatCompletionStream).mock.calls[0]?.[0];

expect(sent!.messages.filter((m) => m.role === "system")).toHaveLength(1);
expect(sent!.messages[0]?.content).toBe(systemMessage);
expect(sent!.messages.map((m) => m.content)).toEqual([
systemMessage,
"hello",
"and now?",
]);
});

it("should use safety buffer in validation", async () => {
const model = {
model: "test-model",
Expand Down
8 changes: 6 additions & 2 deletions extensions/cli/src/stream/streamChatResponse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { services } from "../services/index.js";
import { telemetryService } from "../telemetry/telemetryService.js";
import { applyChatCompletionToolOverrides } from "../tools/applyToolOverrides.js";
import { ToolCall } from "../tools/index.js";
import { withoutUiNotices } from "../uiNotices.js";
import {
chatCompletionStreamWithBackoff,
isContextLengthError,
Expand Down Expand Up @@ -249,9 +250,12 @@ export async function processStreamingResponse(
throw new Error(`Context length validation failed: ${validation.error}`);
}

// Create OpenAI format history with validated system message
// Create OpenAI format history with validated system message. UI notices are
// dropped first: they are `role: "system"` items the TUI added for display
// only, and sending them would put a second system message at a non-zero
// index, which providers reject.
const openaiChatHistory = convertFromUnifiedHistoryWithSystemMessage(
chatHistory,
withoutUiNotices(chatHistory),
systemMessage,
) as ChatCompletionMessageParam[];
const requestStartTime = Date.now();
Expand Down
25 changes: 4 additions & 21 deletions extensions/cli/src/ui/TUIChat.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
UpdateServiceState,
} from "../services/types.js";
import { getTotalSessionCost } from "../session.js";
import { uiNotice } from "../uiNotices.js";
import { bashToolEvents } from "../util/cli.js";
import { logger } from "../util/logger.js";

Expand Down Expand Up @@ -302,13 +303,7 @@ const TUIChat: React.FC<TUIChatProps> = ({
if (!session) {
setChatHistory((prev) => [
...prev,
{
message: {
role: "system",
content: `Failed to export: Session ${sessionId} not found`,
},
contextItems: [],
},
uiNotice(`Failed to export: Session ${sessionId} not found`),
]);
closeCurrentScreen();
return;
Expand All @@ -329,25 +324,13 @@ const TUIChat: React.FC<TUIChatProps> = ({

setChatHistory((prev) => [
...prev,
{
message: {
role: "system",
content: `Session exported to ${defaultPath}`,
},
contextItems: [],
},
uiNotice(`Session exported to ${defaultPath}`),
]);
closeCurrentScreen();
} catch (error: any) {
setChatHistory((prev) => [
...prev,
{
message: {
role: "system",
content: `Failed to export session: ${error.message}`,
},
contextItems: [],
},
uiNotice(`Failed to export session: ${error.message}`),
]);
closeCurrentScreen();
}
Expand Down
4 changes: 3 additions & 1 deletion extensions/cli/src/ui/hooks/useChat.clear.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,16 @@ describe("useChat clear command", () => {
chatHistory[0], // The system message ChatHistoryItem
]);

// Second call should be with the "Chat history cleared" message
// Second call should be with the "Chat history cleared" message, marked as
// a UI notice so it is shown to the user but not sent to the model.
expect(mockSetChatHistory).toHaveBeenLastCalledWith([
{
message: {
role: "system",
content: "Chat history cleared",
},
contextItems: [],
messageType: "system",
},
]);
});
Expand Down
28 changes: 4 additions & 24 deletions extensions/cli/src/ui/hooks/useChat.compaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import type { ChatHistoryItem, Session } from "core/index.js";
import { compactChatHistory } from "../../compaction.js";
import { updateSessionHistory } from "../../session.js";
import { handleAutoCompaction as coreHandleAutoCompaction } from "../../stream/streamChatResponse.autoCompaction.js";
import { uiNotice } from "../../uiNotices.js";
import { formatError } from "../../util/formatError.js";
import { logger } from "../../util/logger.js";

Expand Down Expand Up @@ -68,39 +69,18 @@ export async function handleCompactCommand({
// Add success message to chat
setChatHistory((prev) => [
...prev,
{
message: {
role: "system",
content: "Chat history compacted successfully.",
},
contextItems: [],
},
uiNotice("Chat history compacted successfully."),
]);
} catch (error) {
// Check if the error was due to abortion
if (compactionController.signal.aborted) {
logger.info("Manual compaction was cancelled by user");
setChatHistory((prev) => [
...prev,
{
message: {
role: "system",
content: "Compaction cancelled.",
},
contextItems: [],
},
]);
setChatHistory((prev) => [...prev, uiNotice("Compaction cancelled.")]);
} else {
logger.error("Compaction failed:", error);
setChatHistory((prev) => [
...prev,
{
message: {
role: "system",
content: `Compaction failed: ${formatError(error)}`,
},
contextItems: [],
},
uiNotice(`Compaction failed: ${formatError(error)}`),
]);
}
} finally {
Expand Down
30 changes: 4 additions & 26 deletions extensions/cli/src/ui/hooks/useChat.helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { logger } from "src/util/logger.js";
import { DEFAULT_SESSION_TITLE } from "../../constants/session.js";
import { loadSession, startNewSession } from "../../session.js";
import { telemetryService } from "../../telemetry/telemetryService.js";
import { uiNotice } from "../../uiNotices.js";

import { processImagePlaceholder } from "./useChat.imageProcessing.js";
import { SlashCommandResult } from "./useChat.types.js";
Expand Down Expand Up @@ -124,42 +125,19 @@ export function processSlashCommandResult({
}

if (result.output) {
setChatHistory([
{
message: {
role: "system",
content: result.output,
},
contextItems: [],
},
]);
setChatHistory([uiNotice(result.output)]);
}
return null;
}

if (result.diffContent) {
setChatHistory((prev) => [
...prev,
{
message: {
role: "system",
content: `Diff:\n${result.diffContent}`,
},
contextItems: [],
},
uiNotice(`Diff:\n${result.diffContent}`),
]);
return null;
} else if (result.output) {
setChatHistory((prev) => [
...prev,
{
message: {
role: "system",
content: result.output || "",
},
contextItems: [],
},
]);
setChatHistory((prev) => [...prev, uiNotice(result.output || "")]);
return null;
}

Expand Down
Loading
Loading