Skip to content

Commit 4d9e8d1

Browse files
committed
🤖 Fix init display bug - restore defensive checks
The cleanup removed necessary defensive checks, causing crashes when init-output or init-end arrive without init-start (can happen during replay or out-of-order events). Restored graceful handling. Added TDD test to prevent regression. Generated with `cmux`
1 parent 8c8a92a commit 4d9e8d1

File tree

2 files changed

+81
-3
lines changed

2 files changed

+81
-3
lines changed
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
import { StreamingMessageAggregator } from "./StreamingMessageAggregator";
2+
3+
interface InitDisplayedMessage {
4+
type: "workspace-init";
5+
status: "running" | "success" | "error";
6+
lines: string[];
7+
exitCode: number | null;
8+
}
9+
10+
describe("Init display after cleanup changes", () => {
11+
it("should display init messages correctly", () => {
12+
const aggregator = new StreamingMessageAggregator();
13+
14+
// Simulate init start
15+
aggregator.handleMessage({
16+
type: "init-start",
17+
hookPath: "/test/.cmux/init",
18+
timestamp: Date.now(),
19+
});
20+
21+
let messages = aggregator.getDisplayedMessages();
22+
expect(messages).toHaveLength(1);
23+
expect(messages[0].type).toBe("workspace-init");
24+
expect((messages[0] as InitDisplayedMessage).status).toBe("running");
25+
26+
// Simulate init output
27+
aggregator.handleMessage({
28+
type: "init-output",
29+
line: "Installing dependencies...",
30+
timestamp: Date.now(),
31+
isError: false,
32+
});
33+
34+
messages = aggregator.getDisplayedMessages();
35+
expect(messages).toHaveLength(1);
36+
expect((messages[0] as InitDisplayedMessage).lines).toContain("Installing dependencies...");
37+
38+
// Simulate init end
39+
aggregator.handleMessage({
40+
type: "init-end",
41+
exitCode: 0,
42+
timestamp: Date.now(),
43+
});
44+
45+
messages = aggregator.getDisplayedMessages();
46+
expect(messages).toHaveLength(1);
47+
expect((messages[0] as InitDisplayedMessage).status).toBe("success");
48+
expect((messages[0] as InitDisplayedMessage).exitCode).toBe(0);
49+
});
50+
51+
it("should handle init-output without init-start (defensive)", () => {
52+
const aggregator = new StreamingMessageAggregator();
53+
54+
// This might crash with non-null assertion if initState is null
55+
expect(() => {
56+
aggregator.handleMessage({
57+
type: "init-output",
58+
line: "Some output",
59+
timestamp: Date.now(),
60+
isError: false,
61+
});
62+
}).not.toThrow();
63+
});
64+
65+
it("should handle init-end without init-start (defensive)", () => {
66+
const aggregator = new StreamingMessageAggregator();
67+
68+
expect(() => {
69+
aggregator.handleMessage({
70+
type: "init-end",
71+
exitCode: 0,
72+
timestamp: Date.now(),
73+
});
74+
}).not.toThrow();
75+
});
76+
});

src/utils/messages/StreamingMessageAggregator.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -511,15 +511,17 @@ export class StreamingMessageAggregator {
511511
}
512512

513513
if (isInitOutput(data)) {
514+
if (!this.initState) return; // Defensive: shouldn't happen but handle gracefully
514515
const line = data.isError ? `ERROR: ${data.line}` : data.line;
515-
this.initState!.lines.push(line.trimEnd());
516+
this.initState.lines.push(line.trimEnd());
516517
this.invalidateCache();
517518
return;
518519
}
519520

520521
if (isInitEnd(data)) {
521-
this.initState!.exitCode = data.exitCode;
522-
this.initState!.status = data.exitCode === 0 ? "success" : "error";
522+
if (!this.initState) return; // Defensive: shouldn't happen but handle gracefully
523+
this.initState.exitCode = data.exitCode;
524+
this.initState.status = data.exitCode === 0 ? "success" : "error";
523525
this.invalidateCache();
524526
return;
525527
}

0 commit comments

Comments
 (0)