From a552803a3f9736480321f29d5cd85ec27ff61152 Mon Sep 17 00:00:00 2001 From: Ammar Date: Mon, 6 Jul 2026 20:32:47 -0500 Subject: [PATCH] fix: hide synthetic queued messages from user queue UI --- .../agentSession.queueDispatch.test.ts | 79 +++++++++++++++++ src/node/services/agentSession.ts | 44 ++++++---- src/node/services/messageQueue.test.ts | 87 ++++++++++++++----- src/node/services/messageQueue.ts | 53 ++++++++--- 4 files changed, 210 insertions(+), 53 deletions(-) diff --git a/src/node/services/agentSession.queueDispatch.test.ts b/src/node/services/agentSession.queueDispatch.test.ts index 750483ccd6..5a89d85b2b 100644 --- a/src/node/services/agentSession.queueDispatch.test.ts +++ b/src/node/services/agentSession.queueDispatch.test.ts @@ -1,5 +1,6 @@ import { describe, expect, spyOn, test } from "bun:test"; +import { isQueuedMessageChanged, isRestoreToInput } from "@/common/orpc/types"; import { Ok } from "@/common/types/result"; import { createAgentSessionHarness } from "./agentSession.testHarness"; @@ -215,6 +216,84 @@ describe("AgentSession queued message tool-call dispatch", () => { } }); + test("hides synthetic queued messages from the queued-message-changed event", async () => { + const workspaceId = "queue-dispatch-synthetic-hidden"; + const { session, cleanup, events } = await createAgentSessionHarness({ + workspaceId, + captureEvents: true, + }); + + try { + session.queueMessage( + "A background bash monitor matched output.", + { model: TEST_MODEL, agentId: "exec" }, + { synthetic: true, agentInitiated: true } + ); + session.queueMessage("user follow up", { model: TEST_MODEL, agentId: "exec" }); + + const queueEvents = events.filter(isQueuedMessageChanged); + expect(queueEvents).toHaveLength(2); + // Synthetic-only queue renders as empty (no queue banner)... + expect(queueEvents[0].queuedMessages).toEqual([]); + expect(queueEvents[0].displayText).toBe(""); + // ...and a mixed batch shows only the user-authored text. + expect(queueEvents[1].queuedMessages).toEqual(["user follow up"]); + expect(queueEvents[1].displayText).toBe("user follow up"); + } finally { + session.dispose(); + await cleanup(); + } + }); + + test("does not restore synthetic queued messages to the input box", async () => { + const workspaceId = "queue-dispatch-synthetic-restore"; + const { session, cleanup, events } = await createAgentSessionHarness({ + workspaceId, + captureEvents: true, + }); + + try { + session.queueMessage( + "A background bash monitor matched output.", + { model: TEST_MODEL, agentId: "exec" }, + { synthetic: true, agentInitiated: true } + ); + session.restoreQueueToInput(); + + // Queue is cleared, but no system text leaks into the composer. + expect(session.hasQueuedMessages()).toBe(false); + expect(events.filter(isRestoreToInput)).toHaveLength(0); + } finally { + session.dispose(); + await cleanup(); + } + }); + + test("restores only user text when the queue mixes synthetic and user messages", async () => { + const workspaceId = "queue-dispatch-mixed-restore"; + const { session, cleanup, events } = await createAgentSessionHarness({ + workspaceId, + captureEvents: true, + }); + + try { + session.queueMessage( + "A background bash monitor matched output.", + { model: TEST_MODEL, agentId: "exec" }, + { synthetic: true, agentInitiated: true } + ); + session.queueMessage("user follow up", { model: TEST_MODEL, agentId: "exec" }); + session.restoreQueueToInput(); + + const restoreEvents = events.filter(isRestoreToInput); + expect(restoreEvents).toHaveLength(1); + expect(restoreEvents[0].text).toBe("user follow up"); + } finally { + session.dispose(); + await cleanup(); + } + }); + test("does not send the queued message after a user abort", async () => { const workspaceId = "queue-dispatch-user-abort"; const { session, cleanup, aiEmitter, aiService } = await createAgentSessionHarness({ diff --git a/src/node/services/agentSession.ts b/src/node/services/agentSession.ts index 0a3e63cc8f..f3bbaca7d7 100644 --- a/src/node/services/agentSession.ts +++ b/src/node/services/agentSession.ts @@ -2173,16 +2173,7 @@ export class AgentSession { // rebuild queue UI state even when history replay errored mid-flight. listener({ workspaceId: this.workspaceId, - message: { - type: "queued-message-changed", - workspaceId: this.workspaceId, - queuedMessages: this.messageQueue.getMessages(), - displayText: this.messageQueue.getDisplayText(), - fileParts: this.messageQueue.getFileParts(), - reviews: this.messageQueue.getReviews(), - queueDispatchMode: this.messageQueue.getQueueDispatchMode(), - hasCompactionRequest: this.messageQueue.hasCompactionRequest(), - }, + message: this.buildQueuedMessageChangedEvent(), }); // Rehydrate pending auto-retry countdown state on reconnect/reload so @@ -5178,11 +5169,20 @@ export class AgentSession { restoreQueueToInput(): void { this.assertNotDisposed("restoreQueueToInput"); if (!this.messageQueue.isEmpty()) { - const displayText = this.messageQueue.getDisplayText(); + // Restore only user-authored content. Synthetic (system-injected) entries such as + // bash monitor wake prompts are canceled via clearQueue's callbacks instead of + // leaking system text into the composer. + const displayText = this.messageQueue.getUserDisplayText(); const fileParts = this.messageQueue.getFileParts(); const reviews = this.messageQueue.getReviews(); this.clearQueue(); + const hasUserContent = + displayText.length > 0 || fileParts.length > 0 || (reviews?.length ?? 0) > 0; + if (!hasUserContent) { + return; + } + this.emitChatEvent({ type: "restore-to-input", workspaceId: this.workspaceId, @@ -5193,17 +5193,29 @@ export class AgentSession { } } - private emitQueuedMessageChanged(): void { - this.emitChatEvent({ + /** + * Snapshot of the queue for UI consumption. Only user-authored entries are included; + * synthetic (system-injected) messages like bash monitor wakes still dispatch with the + * queued batch but must never appear in the queued-message banner. + */ + private buildQueuedMessageChangedEvent(): Extract< + WorkspaceChatMessage, + { type: "queued-message-changed" } + > { + return { type: "queued-message-changed", workspaceId: this.workspaceId, - queuedMessages: this.messageQueue.getMessages(), - displayText: this.messageQueue.getDisplayText(), + queuedMessages: this.messageQueue.getUserMessages(), + displayText: this.messageQueue.getUserDisplayText(), fileParts: this.messageQueue.getFileParts(), reviews: this.messageQueue.getReviews(), queueDispatchMode: this.messageQueue.getQueueDispatchMode(), hasCompactionRequest: this.messageQueue.hasCompactionRequest(), - }); + }; + } + + private emitQueuedMessageChanged(): void { + this.emitChatEvent(this.buildQueuedMessageChangedEvent()); } /** diff --git a/src/node/services/messageQueue.test.ts b/src/node/services/messageQueue.test.ts index 9bab2c0f4d..c0c9de26ad 100644 --- a/src/node/services/messageQueue.test.ts +++ b/src/node/services/messageQueue.test.ts @@ -10,12 +10,12 @@ describe("MessageQueue", () => { queue = new MessageQueue(); }); - describe("getDisplayText", () => { + describe("getUserDisplayText", () => { it("should return joined messages for normal messages", () => { queue.add("First message"); queue.add("Second message"); - expect(queue.getDisplayText()).toBe("First message\nSecond message"); + expect(queue.getUserDisplayText()).toBe("First message\nSecond message"); }); it("should return rawCommand for compaction request", () => { @@ -33,7 +33,7 @@ describe("MessageQueue", () => { queue.add("Summarize this conversation into a compact form...", options); - expect(queue.getDisplayText()).toBe("/compact -t 3000"); + expect(queue.getUserDisplayText()).toBe("/compact -t 3000"); }); it("should throw when adding compaction after normal message", () => { @@ -71,11 +71,11 @@ describe("MessageQueue", () => { queue.add("Regular message", options); - expect(queue.getDisplayText()).toBe("Regular message"); + expect(queue.getUserDisplayText()).toBe("Regular message"); }); it("should return empty string for empty queue", () => { - expect(queue.getDisplayText()).toBe(""); + expect(queue.getUserDisplayText()).toBe(""); }); it("should return joined messages after clearing compaction metadata", () => { @@ -95,7 +95,7 @@ describe("MessageQueue", () => { queue.clear(); queue.add("New message"); - expect(queue.getDisplayText()).toBe("New message"); + expect(queue.getUserDisplayText()).toBe("New message"); }); }); @@ -116,9 +116,9 @@ describe("MessageQueue", () => { queue.add("Summarize this conversation...", options); // getMessages should return the actual message text for editing - expect(queue.getMessages()).toEqual(["Summarize this conversation..."]); - // getDisplayText should return the slash command - expect(queue.getDisplayText()).toBe("/compact"); + expect(queue.getUserMessages()).toEqual(["Summarize this conversation..."]); + // getUserDisplayText should return the slash command + expect(queue.getUserDisplayText()).toBe("/compact"); }); }); @@ -225,7 +225,7 @@ describe("MessageQueue", () => { ).toThrow(/Cannot queue compaction request/); expect(queue.getQueueDispatchMode()).toBe("turn-end"); - expect(queue.getMessages()).toHaveLength(1); + expect(queue.getUserMessages()).toHaveLength(1); }); it("should preserve turn-end mode when agent-skill enqueue is rejected", () => { @@ -256,7 +256,7 @@ describe("MessageQueue", () => { ).toThrow(/Cannot queue agent skill/); expect(queue.getQueueDispatchMode()).toBe("turn-end"); - expect(queue.getMessages()).toHaveLength(1); + expect(queue.getUserMessages()).toHaveLength(1); }); it("should reset mode to tool-end when cleared", () => { @@ -374,7 +374,7 @@ describe("MessageQueue", () => { expect(addedFirst).toBe(true); expect(addedSecond).toBe(false); - expect(queue.getMessages()).toEqual(["Follow up"]); + expect(queue.getUserMessages()).toEqual(["Follow up"]); expect(queue.getFileParts()).toEqual([image]); }); }); @@ -385,8 +385,8 @@ describe("MessageQueue", () => { queue.add("Second message"); queue.add("Third message"); - expect(queue.getMessages()).toEqual(["First message", "Second message", "Third message"]); - expect(queue.getDisplayText()).toBe("First message\nSecond message\nThird message"); + expect(queue.getUserMessages()).toEqual(["First message", "Second message", "Third message"]); + expect(queue.getUserDisplayText()).toBe("First message\nSecond message\nThird message"); }); it("should preserve compaction metadata when follow-up is added", () => { @@ -404,10 +404,10 @@ describe("MessageQueue", () => { queue.add("And then do this follow-up task"); // Display shows all messages (multiple messages = not just compaction) - expect(queue.getDisplayText()).toBe("Summarize...\nAnd then do this follow-up task"); + expect(queue.getUserDisplayText()).toBe("Summarize...\nAnd then do this follow-up task"); // getMessages includes both - expect(queue.getMessages()).toEqual(["Summarize...", "And then do this follow-up task"]); + expect(queue.getUserMessages()).toEqual(["Summarize...", "And then do this follow-up task"]); // produceMessage preserves compaction metadata from first message const { message, options } = queue.produceMessage(); @@ -454,7 +454,7 @@ describe("MessageQueue", () => { muxMetadata: metadata, }); - expect(queue.getDisplayText()).toBe("/init"); + expect(queue.getUserDisplayText()).toBe("/init"); expect(() => queue.add("Follow-up message")).toThrow( /agent skill invocation is already queued/ @@ -489,13 +489,13 @@ describe("MessageQueue", () => { fileParts: [image2], }); - expect(queue.getMessages()).toEqual([ + expect(queue.getUserMessages()).toEqual([ "Message with image", "Follow-up without image", "Another with image", ]); expect(queue.getFileParts()).toEqual([image1, image2]); - expect(queue.getDisplayText()).toBe( + expect(queue.getUserDisplayText()).toBe( "Message with image\nFollow-up without image\nAnother with image" ); }); @@ -531,6 +531,45 @@ describe("MessageQueue", () => { }); }); + describe("synthetic entry segregation", () => { + it("should hide synthetic entries from user-facing accessors while still dispatching them", () => { + queue.add( + "A background bash monitor matched output.", + { model: "gpt-4", agentId: "exec" }, + { synthetic: true } + ); + + // User-facing view is empty (nothing to show in the queue banner or restore to input)... + expect(queue.getUserMessages()).toEqual([]); + expect(queue.getUserDisplayText()).toBe(""); + // ...but the entry still counts as queued and dispatches with the batch. + expect(queue.isEmpty()).toBe(false); + expect(queue.produceMessage().message).toBe("A background bash monitor matched output."); + }); + + it("should show only user text for mixed synthetic + user batches", () => { + queue.add("Synthetic wake prompt", { model: "gpt-4", agentId: "exec" }, { synthetic: true }); + queue.add("User follow-up", { model: "gpt-4", agentId: "exec" }); + + expect(queue.getUserMessages()).toEqual(["User follow-up"]); + expect(queue.getUserDisplayText()).toBe("User follow-up"); + // Dispatch still includes both, in queue order. + expect(queue.produceMessage().message).toBe("Synthetic wake prompt\nUser follow-up"); + }); + + it("should not apply rawCommand shortcut when a synthetic entry shares the queue", () => { + queue.add("Summarize this conversation...", { + model: "gpt-4", + agentId: "exec", + muxMetadata: { type: "compaction-request", rawCommand: "/compact" }, + }); + queue.add("Synthetic follow-up", { model: "gpt-4", agentId: "exec" }, { synthetic: true }); + + // rawCommand would misrepresent the batch; fall back to the actual user text. + expect(queue.getUserDisplayText()).toBe("Summarize this conversation..."); + }); + }); + describe("getFileParts", () => { it("should return accumulated images from multiple messages", () => { const image1 = { @@ -600,7 +639,7 @@ describe("MessageQueue", () => { const image = { url: "data:image/png;base64,abc", mediaType: "image/png" }; queue.add("", { model: "gpt-4", agentId: "exec", fileParts: [image] }); - expect(queue.getMessages()).toEqual([]); + expect(queue.getUserMessages()).toEqual([]); expect(queue.getFileParts()).toEqual([image]); expect(queue.isEmpty()).toBe(false); }); @@ -609,7 +648,7 @@ describe("MessageQueue", () => { queue.add("", { model: "gpt-4", agentId: "exec" }); expect(queue.isEmpty()).toBe(true); - expect(queue.getMessages()).toEqual([]); + expect(queue.getUserMessages()).toEqual([]); expect(queue.getFileParts()).toEqual([]); }); @@ -620,7 +659,7 @@ describe("MessageQueue", () => { queue.add("Text message", { model: "gpt-4", agentId: "exec", fileParts: [image1] }); queue.add("", { model: "gpt-4", agentId: "exec", fileParts: [image2] }); // Image-only - expect(queue.getMessages()).toEqual(["Text message"]); + expect(queue.getUserMessages()).toEqual(["Text message"]); expect(queue.getFileParts()).toEqual([image1, image2]); expect(queue.isEmpty()).toBe(false); }); @@ -643,11 +682,11 @@ describe("MessageQueue", () => { expect(options?.model).toBe("gpt-4"); }); - it("should return empty string for getDisplayText with image-only", () => { + it("should return empty string for getUserDisplayText with image-only", () => { const image = { url: "data:image/png;base64,abc", mediaType: "image/png" }; queue.add("", { model: "gpt-4", agentId: "exec", fileParts: [image] }); - expect(queue.getDisplayText()).toBe(""); + expect(queue.getUserDisplayText()).toBe(""); }); }); }); diff --git a/src/node/services/messageQueue.ts b/src/node/services/messageQueue.ts index d9dfff4461..e4b6f24a1a 100644 --- a/src/node/services/messageQueue.ts +++ b/src/node/services/messageQueue.ts @@ -95,8 +95,19 @@ interface QueuedMessageInternalOptions { onCanceled?: (reason: string) => Promise | void; } +/** + * A single queued text entry. `synthetic` marks system-injected messages + * (bash monitor wakes, goal continuations, workspace-turn follow-ups, ...) + * so user-facing accessors can exclude them: system prompts must never + * surface in the queued-message banner or be restored to the composer. + */ +interface QueueEntry { + text: string; + synthetic: boolean; +} + export class MessageQueue { - private messages: string[] = []; + private entries: QueueEntry[] = []; private firstMuxMetadata?: unknown; private latestOptions?: SendMessageOptions; private accumulatedFileParts: FilePart[] = []; @@ -262,7 +273,7 @@ export class MessageQueue { // Add text message if non-empty if (trimmedMessage.length > 0) { - this.messages.push(trimmedMessage); + this.entries.push({ text: trimmedMessage, synthetic: internal?.synthetic === true }); } if (options) { @@ -300,31 +311,47 @@ export class MessageQueue { } /** - * Get all queued message texts (for editing/restoration). + * Get user-authored queued message texts (for the queue banner and editing/restoration). + * Synthetic (system-injected) entries are deliberately excluded: they dispatch with the + * batch but must never render as if the user had typed them. */ - getMessages(): string[] { - return [...this.messages]; + getUserMessages(): string[] { + return this.entries.filter((entry) => !entry.synthetic).map((entry) => entry.text); } /** - * Get display text for queued messages. + * Get display text for user-authored queued messages. * - Single compaction request shows rawCommand (/compact) * - Single agent-skill invocation shows rawCommand (/{skill}) * - Multiple messages show all actual message texts + * Synthetic (system-injected) entries are excluded, same as getUserMessages(). */ - getDisplayText(): string { + getUserDisplayText(): string { + const userMessages = this.getUserMessages(); + // rawCommand shortcuts only apply when the queue holds nothing but the user command + // (a synthetic entry alongside would make rawCommand misrepresent the batch). + const hasSyntheticEntries = this.entries.some((entry) => entry.synthetic); + // Only show rawCommand for single compaction request - if (this.messages.length === 1 && isCompactionMetadata(this.firstMuxMetadata)) { + if ( + !hasSyntheticEntries && + userMessages.length === 1 && + isCompactionMetadata(this.firstMuxMetadata) + ) { return this.firstMuxMetadata.rawCommand; } // Only show rawCommand for a single agent-skill invocation. // (Batching agent-skill with other messages is disallowed.) - if (this.messages.length <= 1 && isAgentSkillMetadata(this.firstMuxMetadata)) { + if ( + !hasSyntheticEntries && + userMessages.length <= 1 && + isAgentSkillMetadata(this.firstMuxMetadata) + ) { return this.firstMuxMetadata.rawCommand; } - return this.messages.join("\n"); + return userMessages.join("\n"); } /** @@ -364,7 +391,7 @@ export class MessageQueue { options?: SendMessageOptions & { fileParts?: FilePart[] }; internal?: QueuedMessageInternalOptions; } { - const joinedMessages = this.messages.join("\n"); + const joinedMessages = this.entries.map((entry) => entry.text).join("\n"); // First metadata takes precedence (preserves compaction + agent-skill invocations) const muxMetadata = this.firstMuxMetadata !== undefined @@ -414,7 +441,7 @@ export class MessageQueue { * Clear all queued messages, options, and images. */ clear(): void { - this.messages = []; + this.entries = []; this.firstMuxMetadata = undefined; this.latestOptions = undefined; this.accumulatedFileParts = []; @@ -433,6 +460,6 @@ export class MessageQueue { * Check if queue is empty (no messages AND no images). */ isEmpty(): boolean { - return this.messages.length === 0 && this.accumulatedFileParts.length === 0; + return this.entries.length === 0 && this.accumulatedFileParts.length === 0; } }