Skip to content

Commit 4e7991e

Browse files
committed
Fix Goose2 ACP notification state handling
Signed-off-by: Andrew Harvard <aharvard@squareup.com>
1 parent 6ae3a31 commit 4e7991e

5 files changed

Lines changed: 131 additions & 68 deletions

File tree

ui/goose2/src/features/chat/ui/MessageBubble.tsx

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@ import type {
4040
MessageContent,
4141
TextContent,
4242
ImageContent,
43-
McpAppContent,
4443
ToolResponseContent,
4544
ThinkingContent,
4645
ReasoningContent as ReasoningContentType,
@@ -226,10 +225,8 @@ function renderContentBlock(
226225
case "toolResponse":
227226
// Handled by groupContentSections toolChain rendering
228227
return null;
229-
case "mcpApp": {
230-
const mcpApp = content as McpAppContent;
231-
return <McpAppView key={`mcp-app-${index}`} payload={mcpApp.payload} />;
232-
}
228+
case "mcpApp":
229+
return <McpAppView key={`mcp-app-${index}`} payload={content.payload} />;
233230
case "thinking": {
234231
const th = content as ThinkingContent;
235232
return (
Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
import { beforeEach, describe, expect, it, vi } from "vitest";
2+
import { render, screen } from "@testing-library/react";
3+
import { MessageBubble } from "../MessageBubble";
4+
import { useAgentStore } from "@/features/agents/stores/agentStore";
5+
import type { Message } from "@/shared/types/messages";
6+
7+
vi.mock("@tauri-apps/plugin-opener", () => ({
8+
openPath: vi.fn(),
9+
}));
10+
11+
function assistantMessage(
12+
content: Message["content"],
13+
overrides: Partial<Message> = {},
14+
): Message {
15+
return {
16+
id: "a1",
17+
role: "assistant",
18+
created: Date.now(),
19+
content,
20+
...overrides,
21+
};
22+
}
23+
24+
describe("MessageBubble MCP app rendering", () => {
25+
beforeEach(() => {
26+
useAgentStore.setState({ personas: [] });
27+
});
28+
29+
it("renders MCP App blocks", () => {
30+
const msg = assistantMessage([
31+
{
32+
type: "toolRequest",
33+
id: "tool-1",
34+
name: "weather: open app",
35+
arguments: {},
36+
status: "completed",
37+
},
38+
{
39+
type: "toolResponse",
40+
id: "tool-1",
41+
name: "weather: open app",
42+
result: "done",
43+
isError: false,
44+
},
45+
{
46+
type: "mcpApp",
47+
id: "tool-1",
48+
payload: {
49+
sessionId: "local-session",
50+
gooseSessionId: "goose-session",
51+
toolCallId: "tool-1",
52+
toolCallTitle: "weather: open app",
53+
source: "toolCallUpdateMeta",
54+
tool: {
55+
name: "weather__open_app",
56+
extensionName: "weather",
57+
resourceUri: "ui://weather/app",
58+
},
59+
resource: {
60+
result: {
61+
contents: [
62+
{
63+
uri: "ui://weather/app",
64+
mimeType: "text/html",
65+
text: "<div>Hello</div>",
66+
},
67+
],
68+
},
69+
},
70+
},
71+
},
72+
]);
73+
74+
render(<MessageBubble message={msg} />);
75+
76+
const mcpAppView = screen.getByTestId("mcp-app-view");
77+
expect(mcpAppView).toBeInTheDocument();
78+
expect(mcpAppView).toHaveTextContent("ui://weather/app");
79+
expect(mcpAppView).toHaveTextContent("<div>Hello</div>");
80+
});
81+
});

ui/goose2/src/features/chat/ui/__tests__/MessageBubble.test.tsx

Lines changed: 0 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -380,59 +380,6 @@ describe("MessageBubble", () => {
380380
expect(screen.queryByText("Tool result")).not.toBeInTheDocument();
381381
});
382382

383-
it("renders MCP App blocks", () => {
384-
const msg = assistantMessage([
385-
{
386-
type: "toolRequest",
387-
id: "tool-1",
388-
name: "weather: open app",
389-
arguments: {},
390-
status: "completed",
391-
},
392-
{
393-
type: "toolResponse",
394-
id: "tool-1",
395-
name: "weather: open app",
396-
result: "done",
397-
isError: false,
398-
},
399-
{
400-
type: "mcpApp",
401-
id: "tool-1",
402-
payload: {
403-
sessionId: "local-session",
404-
gooseSessionId: "goose-session",
405-
toolCallId: "tool-1",
406-
toolCallTitle: "weather: open app",
407-
source: "toolCallUpdateMeta",
408-
tool: {
409-
name: "weather__open_app",
410-
extensionName: "weather",
411-
resourceUri: "ui://weather/app",
412-
},
413-
resource: {
414-
result: {
415-
contents: [
416-
{
417-
uri: "ui://weather/app",
418-
mimeType: "text/html",
419-
text: "<div>Hello</div>",
420-
},
421-
],
422-
},
423-
},
424-
},
425-
},
426-
]);
427-
428-
render(<MessageBubble message={msg} />);
429-
430-
const mcpAppView = screen.getByTestId("mcp-app-view");
431-
expect(mcpAppView).toBeInTheDocument();
432-
expect(mcpAppView).toHaveTextContent("ui://weather/app");
433-
expect(mcpAppView).toHaveTextContent("<div>Hello</div>");
434-
});
435-
436383
it("renders thinking content as Reasoning block", () => {
437384
const msg = assistantMessage([{ type: "thinking", text: "deep thoughts" }]);
438385
render(<MessageBubble message={msg} />);

ui/goose2/src/shared/api/acpNotificationHandler.test.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,14 @@ import {
66
getAndDeleteReplayBuffer,
77
} from "@/features/chat/hooks/replayBuffer";
88
import { registerSession } from "./acpSessionTracker";
9-
import { handleSessionNotification } from "./acpNotificationHandler";
9+
import {
10+
clearMessageTracking,
11+
handleSessionNotification,
12+
} from "./acpNotificationHandler";
1013

1114
describe("acpNotificationHandler", () => {
1215
beforeEach(() => {
16+
clearMessageTracking();
1317
clearReplayBuffer("draft-session-1");
1418
clearReplayBuffer("draft-session-2");
1519
useChatStore.setState({

ui/goose2/src/shared/api/acpNotificationHandler.ts

Lines changed: 43 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,10 @@ import {
2525
ensureReplayAssistantMessage,
2626
getTrackedReplayAssistantMessageId,
2727
} from "./acpReplayAssistant";
28-
import { getLocalSessionId } from "./acpSessionTracker";
28+
import {
29+
getLocalSessionId,
30+
subscribeToSessionRegistration,
31+
} from "./acpSessionTracker";
2932
import { perfLog } from "@/shared/lib/perfLog";
3033

3134
// Pre-set message ID for the next live stream per goose session
@@ -44,6 +47,20 @@ interface LivePerf {
4447
chunkCount: number;
4548
}
4649
const livePerf = new Map<string, LivePerf>();
50+
const pendingUsageUpdates = new Map<
51+
string,
52+
{ accumulatedTotal: number; contextLimit: number }
53+
>();
54+
55+
subscribeToSessionRegistration((localSessionId, gooseSessionId) => {
56+
const pendingUsage = pendingUsageUpdates.get(gooseSessionId);
57+
if (!pendingUsage) {
58+
return;
59+
}
60+
61+
useChatStore.getState().updateTokenState(localSessionId, pendingUsage);
62+
pendingUsageUpdates.delete(gooseSessionId);
63+
});
4764

4865
export function setActiveMessageId(
4966
gooseSessionId: string,
@@ -78,7 +95,8 @@ export async function handleSessionNotification(
7895
notification: SessionNotification,
7996
): Promise<void> {
8097
const gooseSessionId = notification.sessionId;
81-
const sessionId = getLocalSessionId(gooseSessionId) ?? gooseSessionId;
98+
const localSessionId = getLocalSessionId(gooseSessionId);
99+
const sessionId = localSessionId ?? gooseSessionId;
82100
const { update } = notification;
83101
const isReplay = useChatStore.getState().loadingSessionIds.has(sessionId);
84102

@@ -93,7 +111,7 @@ export async function handleSessionNotification(
93111
}
94112
perf.lastAt = now;
95113
perf.count += 1;
96-
handleReplay(sessionId, gooseSessionId, update);
114+
handleReplay(sessionId, gooseSessionId, localSessionId, update);
97115
} else {
98116
const perf = livePerf.get(gooseSessionId);
99117
if (perf && update.sessionUpdate === "agent_message_chunk") {
@@ -106,7 +124,7 @@ export async function handleSessionNotification(
106124
);
107125
}
108126
}
109-
handleLive(sessionId, gooseSessionId, update);
127+
handleLive(sessionId, gooseSessionId, localSessionId, update);
110128
}
111129
}
112130

@@ -125,6 +143,7 @@ export function clearReplayPerf(sessionId: string): void {
125143
function handleReplay(
126144
sessionId: string,
127145
gooseSessionId: string,
146+
localSessionId: string | null,
128147
update: SessionUpdate,
129148
): void {
130149
switch (update.sessionUpdate) {
@@ -247,7 +266,7 @@ function handleReplay(
247266
case "session_info_update":
248267
case "config_option_update":
249268
case "usage_update":
250-
handleShared(sessionId, update);
269+
handleShared(sessionId, gooseSessionId, localSessionId, update);
251270
break;
252271

253272
default:
@@ -258,6 +277,7 @@ function handleReplay(
258277
function handleLive(
259278
sessionId: string,
260279
gooseSessionId: string,
280+
localSessionId: string | null,
261281
update: SessionUpdate,
262282
): void {
263283
const store = useChatStore.getState();
@@ -350,15 +370,20 @@ function handleLive(
350370
case "session_info_update":
351371
case "config_option_update":
352372
case "usage_update":
353-
handleShared(sessionId, update);
373+
handleShared(sessionId, gooseSessionId, localSessionId, update);
354374
break;
355375

356376
default:
357377
break;
358378
}
359379
}
360380

361-
function handleShared(sessionId: string, update: SessionUpdate): void {
381+
function handleShared(
382+
sessionId: string,
383+
gooseSessionId: string,
384+
localSessionId: string | null,
385+
update: SessionUpdate,
386+
): void {
362387
switch (update.sessionUpdate) {
363388
case "session_info_update": {
364389
const info = update as SessionUpdate & {
@@ -410,7 +435,6 @@ function handleShared(sessionId: string, update: SessionUpdate): void {
410435
currentModelId;
411436

412437
const sessionStore = useChatSessionStore.getState();
413-
sessionStore.setSessionModels(sessionId, availableModels);
414438
sessionStore.updateSession(
415439
sessionId,
416440
{ modelId: currentModelId, modelName: currentModelName },
@@ -423,7 +447,16 @@ function handleShared(sessionId: string, update: SessionUpdate): void {
423447

424448
case "usage_update": {
425449
const usage = update as SessionUpdate & { sessionUpdate: "usage_update" };
426-
useChatStore.getState().updateTokenState(sessionId, {
450+
451+
if (!localSessionId) {
452+
pendingUsageUpdates.set(gooseSessionId, {
453+
accumulatedTotal: usage.used,
454+
contextLimit: usage.size,
455+
});
456+
break;
457+
}
458+
459+
useChatStore.getState().updateTokenState(localSessionId, {
427460
accumulatedTotal: usage.used,
428461
contextLimit: usage.size,
429462
});
@@ -485,6 +518,7 @@ function ensureLiveAssistantMessage(
485518

486519
export function clearMessageTracking(): void {
487520
presetMessageIds.clear();
521+
pendingUsageUpdates.clear();
488522
clearReplayAssistantTracking();
489523
}
490524

0 commit comments

Comments
 (0)