From 43e51038f56e631b5856aa0d2f9a20237cdd8d8c Mon Sep 17 00:00:00 2001 From: Ammar Date: Fri, 24 Oct 2025 12:26:22 -0500 Subject: [PATCH 1/3] =?UTF-8?q?=F0=9F=A4=96=20Add=20defensive=20checks=20f?= =?UTF-8?q?or=20init-output=20and=20message=20processing?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes two issues found after PR #228: 1. **Init-output undefined line crash**: Added null check for data.line before calling trimEnd(). Prevents crash when init-output events arrive with missing line data during replay or out-of-order scenarios. 2. **Message processing condition tightened**: Added 'role' field check to ensure only CmuxMessages are processed in the caught-up branch. This makes the condition symmetric with the buffering branch and ensures we don't accidentally process malformed messages. Both changes follow the defensive programming pattern established in PR #228 - gracefully handle edge cases during replay rather than crash. _Generated with `cmux`_ --- src/stores/WorkspaceStore.ts | 5 ++++- src/utils/messages/StreamingMessageAggregator.ts | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/stores/WorkspaceStore.ts b/src/stores/WorkspaceStore.ts index ec649630b..cdfc8ab4c 100644 --- a/src/stores/WorkspaceStore.ts +++ b/src/stores/WorkspaceStore.ts @@ -964,12 +964,15 @@ export class WorkspaceStore { const historicalMsgs = this.historicalMessages.get(workspaceId) ?? []; historicalMsgs.push(data); this.historicalMessages.set(workspaceId, historicalMsgs); - } else if (isCaughtUp) { + } else if (isCaughtUp && "role" in data) { // Process live events immediately (after history loaded) + // Check for role field to ensure this is a CmuxMessage aggregator.handleMessage(data); this.states.bump(workspaceId); this.checkAndBumpRecencyIfChanged(); } + // Note: Messages not matching above conditions are silently ignored + // This is expected for malformed messages or events handled elsewhere // Note: Init events and stream events are handled by isStreamEvent() buffering above } } diff --git a/src/utils/messages/StreamingMessageAggregator.ts b/src/utils/messages/StreamingMessageAggregator.ts index 67af75874..c0b2e411b 100644 --- a/src/utils/messages/StreamingMessageAggregator.ts +++ b/src/utils/messages/StreamingMessageAggregator.ts @@ -512,6 +512,7 @@ export class StreamingMessageAggregator { if (isInitOutput(data)) { if (!this.initState) return; // Defensive: shouldn't happen but handle gracefully + if (!data.line) return; // Defensive: skip events with missing line data const line = data.isError ? `ERROR: ${data.line}` : data.line; this.initState.lines.push(line.trimEnd()); this.invalidateCache(); From bf50520736c691caae83143cdbd31fd911b18758 Mon Sep 17 00:00:00 2001 From: Ammar Date: Fri, 24 Oct 2025 12:32:43 -0500 Subject: [PATCH 2/3] Add error logging for unexpected states Log errors when: - init-output arrives without init-start - init-output has missing line field - init-end arrives without init-start - message in WorkspaceStore doesn't match any processing condition This helps debug edge cases and prevents silent failures. --- src/stores/WorkspaceStore.ts | 14 +++++++++++--- src/utils/messages/StreamingMessageAggregator.ts | 15 ++++++++++++--- 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/src/stores/WorkspaceStore.ts b/src/stores/WorkspaceStore.ts index cdfc8ab4c..99a368928 100644 --- a/src/stores/WorkspaceStore.ts +++ b/src/stores/WorkspaceStore.ts @@ -970,10 +970,18 @@ export class WorkspaceStore { aggregator.handleMessage(data); this.states.bump(workspaceId); this.checkAndBumpRecencyIfChanged(); + } else if ("role" in data || "type" in data) { + // Unexpected: message with role/type field didn't match any condition + console.error("[WorkspaceStore] Message not processed - unexpected state", { + workspaceId, + isCaughtUp, + hasRole: "role" in data, + hasType: "type" in data, + type: "type" in data ? (data as { type: string }).type : undefined, + role: "role" in data ? (data as { role: string }).role : undefined, + }); } - // Note: Messages not matching above conditions are silently ignored - // This is expected for malformed messages or events handled elsewhere - // Note: Init events and stream events are handled by isStreamEvent() buffering above + // Note: Messages without role/type are silently ignored (expected for some IPC events) } } diff --git a/src/utils/messages/StreamingMessageAggregator.ts b/src/utils/messages/StreamingMessageAggregator.ts index c0b2e411b..489b98a37 100644 --- a/src/utils/messages/StreamingMessageAggregator.ts +++ b/src/utils/messages/StreamingMessageAggregator.ts @@ -511,8 +511,14 @@ export class StreamingMessageAggregator { } if (isInitOutput(data)) { - if (!this.initState) return; // Defensive: shouldn't happen but handle gracefully - if (!data.line) return; // Defensive: skip events with missing line data + if (!this.initState) { + console.error("Received init-output without init-start", { data }); + return; + } + if (!data.line) { + console.error("Received init-output with missing line field", { data }); + return; + } const line = data.isError ? `ERROR: ${data.line}` : data.line; this.initState.lines.push(line.trimEnd()); this.invalidateCache(); @@ -520,7 +526,10 @@ export class StreamingMessageAggregator { } if (isInitEnd(data)) { - if (!this.initState) return; // Defensive: shouldn't happen but handle gracefully + if (!this.initState) { + console.error("Received init-end without init-start", { data }); + return; + } this.initState.exitCode = data.exitCode; this.initState.status = data.exitCode === 0 ? "success" : "error"; this.invalidateCache(); From 3cc69dc6327619538d21f6ba181bd7b543edf717 Mon Sep 17 00:00:00 2001 From: Ammar Date: Fri, 24 Oct 2025 12:37:45 -0500 Subject: [PATCH 3/3] Add extra defensive check for init-output line type Add typeof check after line assignment to handle any edge case where data.line might become undefined after the initial null check. This provides defense-in-depth against malformed events from any source (disk corruption, IPC errors, etc.). --- src/utils/messages/StreamingMessageAggregator.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/utils/messages/StreamingMessageAggregator.ts b/src/utils/messages/StreamingMessageAggregator.ts index 489b98a37..d8ab95b05 100644 --- a/src/utils/messages/StreamingMessageAggregator.ts +++ b/src/utils/messages/StreamingMessageAggregator.ts @@ -520,6 +520,11 @@ export class StreamingMessageAggregator { return; } const line = data.isError ? `ERROR: ${data.line}` : data.line; + // Extra defensive check (should never hit due to check above, but prevents crash if data changes) + if (typeof line !== "string") { + console.error("Init-output line is not a string", { line, data }); + return; + } this.initState.lines.push(line.trimEnd()); this.invalidateCache(); return;