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
5 changes: 5 additions & 0 deletions .changeset/fix-display-only-notices.md
Original file line number Diff line number Diff line change
@@ -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.
24 changes: 24 additions & 0 deletions source/acp/acp-agent.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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 => {
Expand Down Expand Up @@ -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 => {
Expand Down Expand Up @@ -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'));
Expand Down
15 changes: 12 additions & 3 deletions source/acp/acp-agent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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'};
}

Expand All @@ -355,6 +363,7 @@ export class AcpAgent implements Agent {
session.messages.push({
role: 'assistant',
content: formattedError,
displayOnly: true,
});

throw error;
Expand Down
151 changes: 151 additions & 0 deletions source/ai-sdk-client/converters/message-converter.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.'));
});
3 changes: 2 additions & 1 deletion source/ai-sdk-client/converters/message-converter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 }] }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
// ============================================================================
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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?.();
Expand Down
1 change: 1 addition & 0 deletions source/types/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export interface Message {
reasoning?: string;
structuredContent?: JSONValue;
images?: ImageAttachment[];
displayOnly?: boolean;
}

export interface ToolCall {
Expand Down
Loading