From 70b029fbd73604d92ae21b4d8634f22f5c58b792 Mon Sep 17 00:00:00 2001 From: Ammar Date: Wed, 3 Dec 2025 12:23:39 -0600 Subject: [PATCH 1/2] =?UTF-8?q?=F0=9F=A4=96=20fix:=20prevent=20compaction?= =?UTF-8?q?=20infinite=20loop=20and=20stale=20usage=20display?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two related bugs were causing compaction to loop forever: 1. **Message queue overwrote compaction metadata**: When follow-up messages were queued after a compaction request, the queue's latestOptions got overwritten, losing the muxMetadata that marks the message as a compaction-request. The compaction would then be sent as a normal message, never triggering history replacement. Fix: Preserve the first muxMetadata and use it in produceMessage(). This ensures compaction metadata survives even when follow-ups are queued afterward. 2. **Usage display showed pre-compaction context**: After compaction, the summary message's usage field contained the pre-compaction context size (since that's what was sent to generate the summary). This inflated the context window display, making it appear usage was still high and triggering auto-compaction again. Fix: Skip compacted messages when computing lastContextUsage. The summary's usage reflects historical cost, not current context. _Generated with `mux`_ --- src/browser/stores/WorkspaceStore.ts | 6 +++ src/node/services/messageQueue.test.ts | 71 ++++++++++++++++++++++---- src/node/services/messageQueue.ts | 65 +++++++++++++++++++---- 3 files changed, 122 insertions(+), 20 deletions(-) diff --git a/src/browser/stores/WorkspaceStore.ts b/src/browser/stores/WorkspaceStore.ts index 396d3ae1ce..17d286fba9 100644 --- a/src/browser/stores/WorkspaceStore.ts +++ b/src/browser/stores/WorkspaceStore.ts @@ -476,10 +476,16 @@ export class WorkspaceStore { // Get last message's context usage for context window display // Uses contextUsage (last step) if available, falls back to usage for old messages + // Skips compacted messages - their usage reflects pre-compaction context, not current const lastContextUsage = (() => { for (let i = messages.length - 1; i >= 0; i--) { const msg = messages[i]; if (msg.role === "assistant") { + // Skip compacted messages - their usage is from pre-compaction context + // and doesn't reflect current context window size + if (msg.metadata?.compacted) { + continue; + } const rawUsage = msg.metadata?.contextUsage ?? msg.metadata?.usage; const providerMeta = msg.metadata?.contextProviderMetadata ?? msg.metadata?.providerMetadata; diff --git a/src/node/services/messageQueue.test.ts b/src/node/services/messageQueue.test.ts index bcb63b081b..b351ad91a8 100644 --- a/src/node/services/messageQueue.test.ts +++ b/src/node/services/messageQueue.test.ts @@ -35,7 +35,7 @@ describe("MessageQueue", () => { expect(queue.getDisplayText()).toBe("/compact -t 3000"); }); - it("should show actual messages when compaction is added after normal message", () => { + it("should throw when adding compaction after normal message", () => { queue.add("First message"); const metadata: MuxFrontendMetadata = { @@ -49,11 +49,11 @@ describe("MessageQueue", () => { muxMetadata: metadata, }; - queue.add("Summarize this conversation...", options); - - // When multiple messages are queued, compaction metadata is lost when sent, - // so display shows actual messages (not rawCommand) to match what will be sent - expect(queue.getDisplayText()).toBe("First message\nSummarize this conversation..."); + // Compaction requests cannot be mixed with other messages to prevent + // silent failures where compaction metadata would be lost + expect(() => queue.add("Summarize this conversation...", options)).toThrow( + /Cannot queue compaction request/ + ); }); it("should return joined messages when metadata type is not compaction-request", () => { @@ -117,6 +117,48 @@ describe("MessageQueue", () => { }); }); + describe("hasCompactionRequest", () => { + it("should return false for empty queue", () => { + expect(queue.hasCompactionRequest()).toBe(false); + }); + + it("should return false for normal messages", () => { + queue.add("Regular message", { model: "gpt-4" }); + expect(queue.hasCompactionRequest()).toBe(false); + }); + + it("should return true when compaction request is queued", () => { + const metadata: MuxFrontendMetadata = { + type: "compaction-request", + rawCommand: "/compact", + parsed: {}, + }; + + queue.add("Summarize...", { + model: "claude-3-5-sonnet-20241022", + muxMetadata: metadata, + }); + + expect(queue.hasCompactionRequest()).toBe(true); + }); + + it("should return false after clearing", () => { + const metadata: MuxFrontendMetadata = { + type: "compaction-request", + rawCommand: "/compact", + parsed: {}, + }; + + queue.add("Summarize...", { + model: "claude-3-5-sonnet-20241022", + muxMetadata: metadata, + }); + queue.clear(); + + expect(queue.hasCompactionRequest()).toBe(false); + }); + }); + describe("multi-message batching", () => { it("should batch multiple follow-up messages", () => { queue.add("First message"); @@ -127,7 +169,7 @@ describe("MessageQueue", () => { expect(queue.getDisplayText()).toBe("First message\nSecond message\nThird message"); }); - it("should batch follow-up message after compaction", () => { + it("should preserve compaction metadata when follow-up is added", () => { const metadata: MuxFrontendMetadata = { type: "compaction-request", rawCommand: "/compact", @@ -140,11 +182,20 @@ describe("MessageQueue", () => { }); queue.add("And then do this follow-up task"); - // When a follow-up is added, compaction metadata is lost (latestOptions overwritten), - // so display shows actual messages to match what will be sent + // Display shows all messages (multiple messages = not just compaction) expect(queue.getDisplayText()).toBe("Summarize...\nAnd then do this follow-up task"); - // Raw messages have the actual prompt + + // getMessages includes both expect(queue.getMessages()).toEqual(["Summarize...", "And then do this follow-up task"]); + + // produceMessage preserves compaction metadata from first message + const { message, options } = queue.produceMessage(); + expect(message).toBe("Summarize...\nAnd then do this follow-up task"); + const muxMeta = options?.muxMetadata as MuxFrontendMetadata; + expect(muxMeta.type).toBe("compaction-request"); + if (muxMeta.type === "compaction-request") { + expect(muxMeta.rawCommand).toBe("/compact"); + } }); it("should produce combined message for API call", () => { diff --git a/src/node/services/messageQueue.ts b/src/node/services/messageQueue.ts index 21c2f6810a..902625ceb3 100644 --- a/src/node/services/messageQueue.ts +++ b/src/node/services/messageQueue.ts @@ -17,22 +17,39 @@ function isCompactionMetadata(meta: unknown): meta is CompactionMetadata { * * Stores: * - Message texts (accumulated) - * - Latest options (model, thinking level, etc. - overwrites on each add) + * - First muxMetadata (preserved - never overwritten by subsequent adds) + * - Latest options (model, etc. - updated on each add) * - Image parts (accumulated across all messages) * + * IMPORTANT: muxMetadata from the first message is preserved even when + * subsequent messages are added. This prevents compaction requests from + * losing their metadata when follow-up messages are queued. + * * Display logic: * - Single compaction request → shows rawCommand (/compact) - * - Multiple messages → shows all actual message texts (since compaction metadata is lost anyway) + * - Multiple messages → shows all actual message texts */ export class MessageQueue { private messages: string[] = []; + // muxMetadata from first message (preserved, never overwritten) + private firstMuxMetadata?: unknown; + // Latest options (for model, etc.) private latestOptions?: SendMessageOptions; private accumulatedImages: ImagePart[] = []; + /** + * Check if the queue currently contains a compaction request. + */ + hasCompactionRequest(): boolean { + return isCompactionMetadata(this.firstMuxMetadata); + } + /** * Add a message to the queue. - * Updates to latest options, accumulates image parts. - * Allows image-only messages (empty text with images). + * Preserves muxMetadata from first message, updates other options. + * Accumulates image parts. + * + * @throws Error if trying to add a compaction request when queue already has messages */ add(message: string, options?: SendMessageOptions & { imageParts?: ImagePart[] }): void { const trimmedMessage = message.trim(); @@ -43,6 +60,18 @@ export class MessageQueue { return; } + const incomingIsCompaction = isCompactionMetadata(options?.muxMetadata); + const queueHasMessages = !this.isEmpty(); + + // Cannot add compaction to a queue that already has messages + // (user should wait for those messages to send first) + if (incomingIsCompaction && queueHasMessages) { + throw new Error( + "Cannot queue compaction request: queue already has messages. " + + "Wait for current stream to complete before compacting." + ); + } + // Add text message if non-empty if (trimmedMessage.length > 0) { this.messages.push(trimmedMessage); @@ -50,6 +79,14 @@ export class MessageQueue { if (options) { const { imageParts, ...restOptions } = options; + + // Preserve first muxMetadata (critical for compaction) + // This ensures compaction metadata isn't lost when follow-ups are added + if (options.muxMetadata !== undefined && this.firstMuxMetadata === undefined) { + this.firstMuxMetadata = options.muxMetadata; + } + + // Always update latest options (for model changes, etc.) this.latestOptions = restOptions; if (imageParts && imageParts.length > 0) { @@ -68,14 +105,12 @@ export class MessageQueue { /** * Get display text for queued messages. * - Single compaction request shows rawCommand (/compact) - * - Multiple messages or non-compaction show actual message texts + * - Multiple messages show all actual message texts */ getDisplayText(): string { // Only show rawCommand for single compaction request - // (compaction metadata is only preserved when no follow-up messages are added) - const muxMetadata = this.latestOptions?.muxMetadata as unknown; - if (this.messages.length === 1 && isCompactionMetadata(muxMetadata)) { - return muxMetadata.rawCommand; + if (this.messages.length === 1 && isCompactionMetadata(this.firstMuxMetadata)) { + return this.firstMuxMetadata.rawCommand; } return this.messages.join("\n"); @@ -90,7 +125,7 @@ export class MessageQueue { /** * Get combined message and options for sending. - * Returns joined messages with latest options + accumulated images. + * Returns joined messages with options (using preserved muxMetadata). */ produceMessage(): { message: string; @@ -98,9 +133,18 @@ export class MessageQueue { } { const joinedMessages = this.messages.join("\n"); + // Merge latest options with preserved muxMetadata + // Use preserved first muxMetadata if available (critical for compaction) + // Falls back to latest options' muxMetadata if no first was stored + const effectiveMuxMetadata = + this.firstMuxMetadata !== undefined + ? this.firstMuxMetadata + : (this.latestOptions?.muxMetadata as unknown); + const options = this.latestOptions ? { ...this.latestOptions, + muxMetadata: effectiveMuxMetadata, imageParts: this.accumulatedImages.length > 0 ? this.accumulatedImages : undefined, } : undefined; @@ -113,6 +157,7 @@ export class MessageQueue { */ clear(): void { this.messages = []; + this.firstMuxMetadata = undefined; this.latestOptions = undefined; this.accumulatedImages = []; } From 88a6ad3eb0e85313168dd60b0485fb4d0bd07ddd Mon Sep 17 00:00:00 2001 From: Ammar Date: Wed, 3 Dec 2025 12:47:16 -0600 Subject: [PATCH 2/2] refactor: consolidate comments and simplify produceMessage() --- src/node/services/messageQueue.ts | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/src/node/services/messageQueue.ts b/src/node/services/messageQueue.ts index 902625ceb3..916de4cee7 100644 --- a/src/node/services/messageQueue.ts +++ b/src/node/services/messageQueue.ts @@ -31,9 +31,7 @@ function isCompactionMetadata(meta: unknown): meta is CompactionMetadata { */ export class MessageQueue { private messages: string[] = []; - // muxMetadata from first message (preserved, never overwritten) private firstMuxMetadata?: unknown; - // Latest options (for model, etc.) private latestOptions?: SendMessageOptions; private accumulatedImages: ImagePart[] = []; @@ -80,13 +78,10 @@ export class MessageQueue { if (options) { const { imageParts, ...restOptions } = options; - // Preserve first muxMetadata (critical for compaction) - // This ensures compaction metadata isn't lost when follow-ups are added + // Preserve first muxMetadata (see class docblock for rationale) if (options.muxMetadata !== undefined && this.firstMuxMetadata === undefined) { this.firstMuxMetadata = options.muxMetadata; } - - // Always update latest options (for model changes, etc.) this.latestOptions = restOptions; if (imageParts && imageParts.length > 0) { @@ -125,26 +120,21 @@ export class MessageQueue { /** * Get combined message and options for sending. - * Returns joined messages with options (using preserved muxMetadata). */ produceMessage(): { message: string; options?: SendMessageOptions & { imageParts?: ImagePart[] }; } { const joinedMessages = this.messages.join("\n"); - - // Merge latest options with preserved muxMetadata - // Use preserved first muxMetadata if available (critical for compaction) - // Falls back to latest options' muxMetadata if no first was stored - const effectiveMuxMetadata = + // First metadata takes precedence (preserves compaction requests) + const muxMetadata = this.firstMuxMetadata !== undefined ? this.firstMuxMetadata : (this.latestOptions?.muxMetadata as unknown); - const options = this.latestOptions ? { ...this.latestOptions, - muxMetadata: effectiveMuxMetadata, + muxMetadata, imageParts: this.accumulatedImages.length > 0 ? this.accumulatedImages : undefined, } : undefined;