From a88339b1691d5f392ba879a1e23842ca53ffbc89 Mon Sep 17 00:00:00 2001 From: addyCooks Date: Tue, 18 Aug 2026 12:46:31 +0530 Subject: [PATCH] fix(messages): keep UI-only notices out of the model payload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cancellation notices, inline error banners, the non-interactive tool-approval notice, and the VS Code replies to built-in slash commands (/help, /copy, /model, unrecognized commands) were pushed into conversation history as assistant messages. They sit in the same Message[] that becomes the provider payload, so the next request handed the model harness-authored markdown as its own past output — and because sessions persist Message[] verbatim, a resumed session replayed it into context too. The built-in command replies never involved the model at all, so both halves of that exchange are marked, not just the reply. Mark those notices displayOnly and filter them in convertToModelMessages, the single point where history becomes the provider payload. They still render in chat and still replay with session history. Closes #893. --- .changeset/fix-display-only-notices.md | 5 + source/acp/acp-agent.spec.ts | 24 +++ source/acp/acp-agent.ts | 15 +- .../converters/message-converter.spec.ts | 151 ++++++++++++++++++ .../converters/message-converter.ts | 3 +- .../conversation/conversation-loop.spec.ts | 24 +++ .../conversation/conversation-loop.tsx | 6 +- source/types/core.ts | 1 + 8 files changed, 224 insertions(+), 5 deletions(-) create mode 100644 .changeset/fix-display-only-notices.md diff --git a/.changeset/fix-display-only-notices.md b/.changeset/fix-display-only-notices.md new file mode 100644 index 000000000..d6c18cb40 --- /dev/null +++ b/.changeset/fix-display-only-notices.md @@ -0,0 +1,5 @@ +--- +"@nanocollective/nanocoder": patch +--- + +Stopped nanocoder's own UI text from being sent to the model as its past output. Cancellation notices (`_Cancelled by user._`), inline error banners (`**Error:** ...`), the non-interactive "Tool approval required" notice, and the VS Code replies to built-in slash commands (`/help`, `/copy`, `/model`, unrecognized commands) were all pushed into conversation history as `assistant` messages, so on the next turn the provider received harness-authored markdown as if the model had written it — teaching it to imitate the chrome and, on a resumed session, to believe it had errored. These are now marked display-only: they still render in the chat and replay with session history, but they are filtered out before messages are converted to the provider payload. Closes #893. diff --git a/source/acp/acp-agent.spec.ts b/source/acp/acp-agent.spec.ts index 9ee80143a..7b28feac8 100644 --- a/source/acp/acp-agent.spec.ts +++ b/source/acp/acp-agent.spec.ts @@ -9,6 +9,7 @@ import { setToolRegistryGetter, setToolManagerGetter, } from '@/message-handler'; +import {convertToModelMessages} from '@/ai-sdk-client/converters/message-converter'; import {sessionManager} from '@/session/session-manager'; console.log('\nacp-agent.spec.ts'); @@ -339,6 +340,10 @@ test('AcpAgent.prompt - propagates API errors cleanly', async t => { // Ensure turnActive is reset even on error t.false(session.turnActive); + + const notice = session.messages[session.messages.length - 1]; + t.is(notice.role, 'assistant'); + t.true(notice.displayOnly, 'the error notice must never reach the model'); }); test('AcpAgent.prompt - resolves cleanly on user cancellation instead of throwing', async t => { @@ -371,6 +376,11 @@ test('AcpAgent.prompt - resolves cleanly on user cancellation instead of throwin u.update?.content?.text?.includes('Cancelled by user'), ), ); + + const persisted = agent['sessions'].get(session.sessionId)!.messages; + const notice = persisted[persisted.length - 1]; + t.is(notice.role, 'assistant'); + t.true(notice.displayOnly, 'the cancel notice must never reach the model'); }); test('AcpAgent.prompt - returns response for valid session', async t => { @@ -441,6 +451,20 @@ test('AcpAgent.prompt - /copy code is not treated as unrecognized', async t => { t.false(reply.includes('Unrecognized slash command')); }); +test('AcpAgent.prompt - a built-in command exchange stays out of model context', async t => { + const {agent} = createAgent(); + const session = await agent.newSession({cwd: '/tmp'}); + await agent.prompt({ + sessionId: session.sessionId, + prompt: [{type: 'text', text: '/help'}], + }); + + const messages = agent['sessions'].get(session.sessionId)!.messages; + t.is(messages.length, 2); + t.true(messages.every(m => m.displayOnly)); + t.deepEqual(convertToModelMessages(messages), []); +}); + test('AcpAgent.prompt - a genuinely unknown command still reports unrecognized', async t => { const reply = await promptForBuiltinReply('/definitelynotacommand'); t.true(reply.includes('Unrecognized slash command')); diff --git a/source/acp/acp-agent.ts b/source/acp/acp-agent.ts index 39d27ada3..ff69a68b4 100644 --- a/source/acp/acp-agent.ts +++ b/source/acp/acp-agent.ts @@ -202,8 +202,12 @@ export class AcpAgent implements Agent { const sendBuiltinReply = (msg: string) => { session.messages = [ ...session.messages, - {role: 'user', content: contextualUserText}, - {role: 'assistant', content: msg}, + { + role: 'user', + content: contextualUserText, + displayOnly: true, + }, + {role: 'assistant', content: msg, displayOnly: true}, ]; this.conn.sessionUpdate({ sessionId: params.sessionId, @@ -335,7 +339,11 @@ export class AcpAgent implements Agent { content: {type: 'text', text: cancelNotice}, }, }); - session.messages.push({role: 'assistant', content: cancelNotice}); + session.messages.push({ + role: 'assistant', + content: cancelNotice, + displayOnly: true, + }); return {stopReason: 'cancelled'}; } @@ -355,6 +363,7 @@ export class AcpAgent implements Agent { session.messages.push({ role: 'assistant', content: formattedError, + displayOnly: true, }); throw error; diff --git a/source/ai-sdk-client/converters/message-converter.spec.ts b/source/ai-sdk-client/converters/message-converter.spec.ts index 6e2a10d5c..eb1b99b07 100644 --- a/source/ai-sdk-client/converters/message-converter.spec.ts +++ b/source/ai-sdk-client/converters/message-converter.spec.ts @@ -398,3 +398,154 @@ test('dropOrphanedToolResults drops a tool result lacking a tool_call_id', t => t.is(result.length, 1); t.is(result[0].role, 'user'); }); + +test('convertToModelMessages drops display-only assistant notices', t => { + const messages: Message[] = [ + {role: 'user', content: 'do it'}, + { + role: 'assistant', + content: '\n\n_Cancelled by user._\n', + displayOnly: true, + }, + {role: 'assistant', content: 'Real reply'}, + ]; + + const result = convertToModelMessages(messages); + t.is(result.length, 2); + t.is(result[0].role, 'user'); + const content = result[1].content as Array<{type: string; text?: string}>; + t.is(content[0].text, 'Real reply'); +}); + +test('convertToModelMessages keeps messages with displayOnly false or absent', t => { + const messages: Message[] = [ + {role: 'user', content: 'hi', displayOnly: false}, + {role: 'assistant', content: 'hello'}, + ]; + + t.is(convertToModelMessages(messages).length, 2); +}); + +test('convertToModelMessages drops display-only messages of every role', t => { + const messages: Message[] = [ + {role: 'system', content: 'sys', displayOnly: true}, + {role: 'user', content: 'usr', displayOnly: true}, + { + role: 'tool', + content: 'res', + tool_call_id: 'call_1', + name: 'edit', + displayOnly: true, + }, + ]; + + t.deepEqual(convertToModelMessages(messages), []); +}); + +test('convertToModelMessages keeps tool results paired across a display-only notice', t => { + const messages: Message[] = [ + { + role: 'assistant', + content: '', + tool_calls: [{id: 'call_1', function: {name: 'edit', arguments: {}}}], + }, + { + role: 'assistant', + content: '\n\n**Error:** boom\n', + displayOnly: true, + }, + {role: 'tool', content: 'edited', tool_call_id: 'call_1', name: 'edit'}, + ]; + + const result = convertToModelMessages(messages); + t.is(result.length, 2); + t.is(result[0].role, 'assistant'); + t.is(result[1].role, 'tool'); +}); + +test('convertToModelMessages orphans results whose tool call is display-only', t => { + const messages: Message[] = [ + { + role: 'assistant', + content: '', + tool_calls: [{id: 'call_1', function: {name: 'edit', arguments: {}}}], + displayOnly: true, + }, + {role: 'tool', content: 'edited', tool_call_id: 'call_1', name: 'edit'}, + ]; + + t.deepEqual(convertToModelMessages(messages), []); +}); + +test('convertToModelMessages emits a tool-call round trip with no synthetic assistant text', t => { + const history: Message[] = [ + {role: 'user', content: 'Read config.json'}, + { + role: 'assistant', + content: 'Reading it now.', + tool_calls: [ + { + id: 'call_1', + function: {name: 'read_file', arguments: {path: 'config.json'}}, + }, + ], + }, + { + role: 'tool', + content: '{"port":3000}', + tool_call_id: 'call_1', + name: 'read_file', + }, + {role: 'assistant', content: '\n\n**Error:** boom\n', displayOnly: true}, + {role: 'user', content: 'and the port?'}, + ]; + + const payload = convertToModelMessages(history); + + t.deepEqual( + payload.map(m => m.role), + ['user', 'assistant', 'tool', 'user'], + ); + + const assistantText = payload + .filter(m => m.role === 'assistant') + .flatMap(m => m.content as Array<{type: string; text?: string}>) + .filter(part => part.type === 'text') + .map(part => part.text); + t.deepEqual(assistantText, ['Reading it now.']); + + t.deepEqual(payload[2].content, [ + { + type: 'tool-result', + toolCallId: 'call_1', + toolName: 'read_file', + output: {type: 'text', value: '{"port":3000}'}, + }, + ]); +}); + +test('convertToModelMessages never leaks a harness notice into the payload', t => { + const notices = [ + '_Cancelled by user._', + '**Error:** stream closed', + 'Tool approval required for: execute_bash. Exiting non-interactive mode', + 'Unrecognized slash command: `/nope`. Type `/help` to see available commands.', + 'Use the model selector in the chat header to switch models.', + ]; + + const history: Message[] = [ + {role: 'user', content: 'go'}, + ...notices.map(content => ({ + role: 'assistant' as const, + content, + displayOnly: true, + })), + {role: 'assistant', content: 'Done.'}, + ]; + + const serialized = JSON.stringify(convertToModelMessages(history)); + for (const notice of notices) { + t.false(serialized.includes(notice), `leaked into payload: ${notice}`); + } + t.true(serialized.includes('Done.')); +}); diff --git a/source/ai-sdk-client/converters/message-converter.ts b/source/ai-sdk-client/converters/message-converter.ts index 809605a96..e1530ff22 100644 --- a/source/ai-sdk-client/converters/message-converter.ts +++ b/source/ai-sdk-client/converters/message-converter.ts @@ -71,7 +71,8 @@ export function dropOrphanedToolResults(messages: Message[]): Message[] { * Orphaned tool results are dropped first (see dropOrphanedToolResults). */ export function convertToModelMessages(messages: Message[]): ModelMessage[] { - return dropOrphanedToolResults(messages).map((msg): ModelMessage => { + const modelFacing = messages.filter(msg => !msg.displayOnly); + return dropOrphanedToolResults(modelFacing).map((msg): ModelMessage => { if (msg.role === 'tool') { // Convert to AI SDK tool-result format // AI SDK expects: { role: 'tool', content: [{ type: 'tool-result', toolCallId, toolName, output }] } diff --git a/source/hooks/chat-handler/conversation/conversation-loop.spec.ts b/source/hooks/chat-handler/conversation/conversation-loop.spec.ts index ab7f75c9c..ef0146c27 100644 --- a/source/hooks/chat-handler/conversation/conversation-loop.spec.ts +++ b/source/hooks/chat-handler/conversation/conversation-loop.spec.ts @@ -237,6 +237,30 @@ test.serial('processAssistantResponse - exits in non-interactive mode when appro t.pass('Non-interactive exit requires proper mock setup'); }); +test.serial('processAssistantResponse - marks the non-interactive approval notice display-only', async t => { + const captured: Message[][] = []; + + const params = createDefaultParams({ + client: createMockClient({ + toolCalls: [{id: 'call_1', function: {name: 'some_tool', arguments: {}}}], + }), + toolManager: createMockToolManager({ + tools: ['some_tool'], + needsApproval: true, + }), + nonInteractiveMode: true, + setMessages: (msgs: Message[]) => captured.push(msgs), + }); + + await processAssistantResponse(params); + + const latest = captured[captured.length - 1]; + const notice = latest[latest.length - 1]; + t.is(notice.role, 'assistant'); + t.regex(notice.content, /Tool approval required for: some_tool/); + t.true(notice.displayOnly, 'the approval notice must never reach the model'); +}); + // ============================================================================ // Auto-Nudge Tests (lines 469-506) // ============================================================================ diff --git a/source/hooks/chat-handler/conversation/conversation-loop.tsx b/source/hooks/chat-handler/conversation/conversation-loop.tsx index 3d477b4e8..a8986a21a 100644 --- a/source/hooks/chat-handler/conversation/conversation-loop.tsx +++ b/source/hooks/chat-handler/conversation/conversation-loop.tsx @@ -804,7 +804,11 @@ export const processAssistantResponse = async ( ); const builder = new MessageBuilder(updatedMessages); builder.addToolResults(turnResults); - builder.addMessage({role: 'assistant', content: errorMsg}); + builder.addMessage({ + role: 'assistant', + content: errorMsg, + displayOnly: true, + }); setMessages(builder.build()); setIsGenerating(false); onConversationComplete?.(); diff --git a/source/types/core.ts b/source/types/core.ts index ab038d3f4..15c11629e 100644 --- a/source/types/core.ts +++ b/source/types/core.ts @@ -36,6 +36,7 @@ export interface Message { reasoning?: string; structuredContent?: JSONValue; images?: ImageAttachment[]; + displayOnly?: boolean; } export interface ToolCall {