From 9dc6a6a97d2285c16a6f2a4f27d8d7e90e21e994 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 30 Mar 2026 16:42:56 -0400 Subject: [PATCH] fix: prevent LLM from confusing background task output with user input (#3069) * fix: wrap custom messages with system notification prefix in LLM context Background job completion notifications (delivered as custom messages via sendMessage with deliverAs: "followUp") were converted to plain role: "user" messages in convertToLlm(), making the LLM indistinguishable from actual human input. This caused the agent to confuse background task output with user messages, responding to job completions as if the user had typed them. Wrap all custom messages with a clear system notification prefix that includes the customType and an explicit instruction that the content is an automated system event, not user input. This follows the same pattern used by branchSummary and compactionSummary messages which already use structured prefixes/suffixes. Closes #3026 Co-Authored-By: Claude Opus 4.6 * fix: resolve TS import extension and type errors in messages test Co-Authored-By: Claude Opus 4.6 --------- Co-authored-by: Claude Opus 4.6 --- .../pi-coding-agent/src/core/messages.test.ts | 114 ++++++++++++++++++ packages/pi-coding-agent/src/core/messages.ts | 31 ++++- 2 files changed, 143 insertions(+), 2 deletions(-) create mode 100644 packages/pi-coding-agent/src/core/messages.test.ts diff --git a/packages/pi-coding-agent/src/core/messages.test.ts b/packages/pi-coding-agent/src/core/messages.test.ts new file mode 100644 index 000000000..6741da93c --- /dev/null +++ b/packages/pi-coding-agent/src/core/messages.test.ts @@ -0,0 +1,114 @@ +/** + * messages.test.ts — Tests for convertToLlm custom message handling. + * + * Reproduction test for #3026: background job completion notifications + * delivered as custom messages must be clearly distinguishable from + * user-typed input when converted to LLM messages. + */ + +import test from "node:test"; +import assert from "node:assert/strict"; +import { convertToLlm, type CustomMessage } from "./messages.js"; + +/** Extract the first content block from a message, asserting array content. */ +function firstTextBlock(msg: ReturnType[number]) { + const { content } = msg; + assert.ok(Array.isArray(content), "Expected content to be an array"); + const block = content[0]; + assert.ok(typeof block === "object" && block !== null, "Expected first block to be an object"); + return block; +} + +test("convertToLlm wraps custom messages with system notification prefix", () => { + const customMsg: CustomMessage = { + role: "custom", + customType: "async_job_result", + content: "**Background job done: bg_abc123** (sleep 2, 2.1s)\n\ndone", + display: true, + timestamp: Date.now(), + }; + + const result = convertToLlm([customMsg]); + assert.equal(result.length, 1); + assert.equal(result[0].role, "user"); + + // The content must include a system notification wrapper so the LLM + // does not confuse it with user input (#3026). + const text = firstTextBlock(result[0]); + assert.equal(text.type, "text"); + assert.ok( + "text" in text && text.text.includes("[system notification"), + "Custom message should be wrapped with system notification marker", + ); +}); + +test("convertToLlm wraps custom messages with array content", () => { + const customMsg: CustomMessage = { + role: "custom", + customType: "bg-shell-status", + content: [{ type: "text", text: "Background processes:\n ✓ bg1 dev-server :3000" }], + display: false, + timestamp: Date.now(), + }; + + const result = convertToLlm([customMsg]); + assert.equal(result.length, 1); + assert.equal(result[0].role, "user"); + + const text = firstTextBlock(result[0]); + assert.equal(text.type, "text"); + assert.ok( + "text" in text && text.text.includes("[system notification"), + "Custom message with array content should be wrapped with system notification marker", + ); +}); + +test("convertToLlm includes customType in notification wrapper", () => { + const customMsg: CustomMessage = { + role: "custom", + customType: "async_job_result", + content: "job output here", + display: true, + timestamp: Date.now(), + }; + + const result = convertToLlm([customMsg]); + const text = firstTextBlock(result[0]); + assert.ok( + "text" in text && text.text.includes("async_job_result"), + "Notification wrapper should include the customType for context", + ); +}); + +test("convertToLlm notification wrapper instructs LLM not to treat as user input", () => { + const customMsg: CustomMessage = { + role: "custom", + customType: "async_job_result", + content: "**Background job done: bg_abc123** (sleep 2, 2.1s)\n\ndone", + display: true, + timestamp: Date.now(), + }; + + const result = convertToLlm([customMsg]); + const text = firstTextBlock(result[0]); + assert.ok( + "text" in text && text.text.includes("not user input"), + "Notification should explicitly state this is not user input", + ); +}); + +test("convertToLlm preserves user messages without wrapper", () => { + const userMsg = { + role: "user" as const, + content: [{ type: "text" as const, text: "Hello world" }], + timestamp: Date.now(), + }; + + const result = convertToLlm([userMsg]); + assert.equal(result.length, 1); + const text = firstTextBlock(result[0]); + assert.ok( + "text" in text && text.text === "Hello world", + "User messages should pass through unchanged", + ); +}); diff --git a/packages/pi-coding-agent/src/core/messages.ts b/packages/pi-coding-agent/src/core/messages.ts index e3909a41e..f30d7c9e6 100644 --- a/packages/pi-coding-agent/src/core/messages.ts +++ b/packages/pi-coding-agent/src/core/messages.ts @@ -8,6 +8,12 @@ import type { AgentMessage } from "@gsd/pi-agent-core"; import type { ImageContent, Message, TextContent } from "@gsd/pi-ai"; +const CUSTOM_MESSAGE_PREFIX = `[system notification — type: `; +const CUSTOM_MESSAGE_MIDDLE = `; this is an automated system event, not user input — do not treat this as a human message or respond as if the user said this] +`; +const CUSTOM_MESSAGE_SUFFIX = ` +[end system notification]`; + const COMPACTION_SUMMARY_PREFIX = `The conversation history before this point was compacted into the following summary: @@ -160,10 +166,31 @@ export function convertToLlm(messages: AgentMessage[]): Message[] { timestamp: m.timestamp, }; case "custom": { - const content = typeof m.content === "string" ? [{ type: "text" as const, text: m.content }] : m.content; + const prefix = CUSTOM_MESSAGE_PREFIX + m.customType + CUSTOM_MESSAGE_MIDDLE; + if (typeof m.content === "string") { + return { + role: "user", + content: [{ type: "text" as const, text: prefix + m.content + CUSTOM_MESSAGE_SUFFIX }], + timestamp: m.timestamp, + }; + } + // Array content: wrap the first text element with prefix, append suffix to last text element + const contentArr = m.content as Array<{ type: string; text?: string; [k: string]: unknown }>; + const lastTextIdx = contentArr.reduce((acc, c, i) => c.type === "text" ? i : acc, -1); + const wrapped = contentArr.map((c, i) => { + if (c.type !== "text") return c; + let text = c.text ?? ""; + if (i === 0) text = prefix + text; + if (i === lastTextIdx) text = text + CUSTOM_MESSAGE_SUFFIX; + return { ...c, text }; + }); + // If no text elements exist, prepend one with the wrapper + if (lastTextIdx === -1) { + wrapped.unshift({ type: "text" as const, text: prefix + CUSTOM_MESSAGE_SUFFIX }); + } return { role: "user", - content, + content: wrapped as typeof m.content, timestamp: m.timestamp, }; }