diff --git a/src/features/chat/chat-notification-plan.ts b/src/features/chat/chat-notification-plan.ts index 00d6b3f9..1d636259 100644 --- a/src/features/chat/chat-notification-plan.ts +++ b/src/features/chat/chat-notification-plan.ts @@ -24,6 +24,7 @@ import { import type { DisplayItem, DisplayKind, MessageDisplayItem } from "./display/types"; import { planProgressDisplayItem } from "./display/plan"; import { createSystemItem } from "./display/system"; +import { goalChangeMessage } from "./goal-messages"; import { attachHookRunsToTurn, hookRunDisplayItem } from "./hook-display"; import { routeServerNotification } from "./inbound-routing"; @@ -69,7 +70,7 @@ export function planChatNotification( case "turnLifecycle": return planTurnLifecycle(state, route.notification); case "threadLifecycle": - return planThreadLifecycle(state, route.notification); + return planThreadLifecycle(state, route.notification, localItemId); case "requestResolved": return { actions: [{ type: "request/resolved", requestId: route.notification.params.requestId }], @@ -191,7 +192,7 @@ function planTurnLifecycle(state: ChatState, notification: ServerNotification): return EMPTY_PLAN; } -function planThreadLifecycle(state: ChatState, notification: ServerNotification): ChatNotificationPlan { +function planThreadLifecycle(state: ChatState, notification: ServerNotification, localItemId: LocalItemIdFactory): ChatNotificationPlan { const { method, params } = notification; if (method === "thread/started") { if (!state.activeThreadId || state.activeThreadId === params.thread.id) { @@ -239,11 +240,21 @@ function planThreadLifecycle(state: ChatState, notification: ServerNotification) } if (method === "thread/goal/updated") { if (state.activeThreadId !== params.threadId) return EMPTY_PLAN; - return actionPlan({ type: "thread/goal-set", goal: params.goal }); + const actions: ChatAction[] = [{ type: "thread/goal-set", goal: params.goal }]; + const message = goalChangeMessage(state.activeGoal, params.goal); + if (message) { + actions.push({ type: "system/message-added", item: createSystemItem(localItemId("system"), message) }); + } + return { actions, effects: [] }; } if (method === "thread/goal/cleared") { if (state.activeThreadId !== params.threadId) return EMPTY_PLAN; - return actionPlan({ type: "thread/goal-set", goal: null }); + const actions: ChatAction[] = [{ type: "thread/goal-set", goal: null }]; + const message = goalChangeMessage(state.activeGoal, null); + if (message) { + actions.push({ type: "system/message-added", item: createSystemItem(localItemId("system"), message) }); + } + return { actions, effects: [] }; } return EMPTY_PLAN; } diff --git a/src/features/chat/goal-controller.ts b/src/features/chat/goal-controller.ts index fe4a18a6..c6d69e7e 100644 --- a/src/features/chat/goal-controller.ts +++ b/src/features/chat/goal-controller.ts @@ -2,6 +2,7 @@ import type { AppServerClient } from "../../app-server/client"; import type { ThreadGoal } from "../../generated/app-server/v2/ThreadGoal"; import type { ThreadGoalStatus } from "../../generated/app-server/v2/ThreadGoalStatus"; import type { ChatStateStore } from "./chat-state"; +import { goalChangeMessage } from "./goal-messages"; export interface ChatGoalControllerHost { stateStore: ChatStateStore; @@ -24,7 +25,7 @@ export class ChatGoalController { if (!client) return; try { const response = await client.getThreadGoal(threadId); - this.applyGoalIfActive(threadId, response.goal); + this.applyGoalIfActive(threadId, response.goal, { reportChange: false }); } catch (error) { if (this.host.stateStore.getState().activeThreadId !== threadId) return; this.host.addSystemMessage(`Could not load thread goal: ${errorMessage(error)}`); @@ -43,14 +44,11 @@ export class ChatGoalController { status: current?.status ?? "active", tokenBudget, }); - if (applied) this.addSystemMessageIfActive(threadId, current ? "Goal updated." : "Goal set."); return applied; } async setStatus(threadId: string, status: ThreadGoalStatus): Promise { - const applied = await this.setGoal(threadId, { status }); - if (applied) this.addSystemMessageIfActive(threadId, goalStatusMessage(status)); - return applied; + return this.setGoal(threadId, { status }); } async clear(threadId: string): Promise { @@ -59,8 +57,7 @@ export class ChatGoalController { if (!client) return false; try { await client.clearThreadGoal(threadId); - this.applyGoalIfActive(threadId, null); - this.addSystemMessageIfActive(threadId, "Goal cleared."); + this.applyGoalIfActive(threadId, null, { reportChange: true }); return true; } catch (error) { this.host.addSystemMessage(errorMessage(error)); @@ -77,7 +74,7 @@ export class ChatGoalController { if (!client) return false; try { const response = await client.setThreadGoal(threadId, params); - this.applyGoalIfActive(threadId, response.goal); + this.applyGoalIfActive(threadId, response.goal, { reportChange: true }); return true; } catch (error) { this.host.addSystemMessage(errorMessage(error)); @@ -85,25 +82,17 @@ export class ChatGoalController { } } - private applyGoalIfActive(threadId: string, goal: ThreadGoal | null): void { - if (this.host.stateStore.getState().activeThreadId !== threadId) return; + private applyGoalIfActive(threadId: string, goal: ThreadGoal | null, options: { reportChange: boolean }): void { + const state = this.host.stateStore.getState(); + if (state.activeThreadId !== threadId) return; + const message = options.reportChange ? goalChangeMessage(state.activeGoal, goal) : null; this.host.stateStore.dispatch({ type: "thread/goal-set", goal }); + if (message) this.host.addSystemMessage(message); this.host.refreshLiveState(); this.host.render(); } - - private addSystemMessageIfActive(threadId: string, text: string): void { - if (this.host.stateStore.getState().activeThreadId !== threadId) return; - this.host.addSystemMessage(text); - } } function errorMessage(error: unknown): string { return error instanceof Error ? error.message : String(error); } - -function goalStatusMessage(status: ThreadGoalStatus): string { - if (status === "paused") return "Goal paused."; - if (status === "active") return "Goal resumed."; - return `Goal status set to ${status}.`; -} diff --git a/src/features/chat/goal-messages.ts b/src/features/chat/goal-messages.ts new file mode 100644 index 00000000..96b02b36 --- /dev/null +++ b/src/features/chat/goal-messages.ts @@ -0,0 +1,28 @@ +import type { ThreadGoal } from "../../generated/app-server/v2/ThreadGoal"; +import type { ThreadGoalStatus } from "../../generated/app-server/v2/ThreadGoalStatus"; + +export function goalChangeMessage(previous: ThreadGoal | null, next: ThreadGoal | null): string | null { + if (!previous && next) return "Goal set."; + if (previous && !next) return "Goal cleared."; + if (!previous || !next) return null; + if (previous.status !== next.status) return goalStatusMessage(next.status); + if (previous.objective !== next.objective || previous.tokenBudget !== next.tokenBudget) return "Goal updated."; + return null; +} + +function goalStatusMessage(status: ThreadGoalStatus): string { + switch (status) { + case "active": + return "Goal resumed."; + case "paused": + return "Goal paused."; + case "complete": + return "Goal completed."; + case "blocked": + return "Goal blocked."; + case "usageLimited": + return "Goal usage limited."; + case "budgetLimited": + return "Goal budget limited."; + } +} diff --git a/tests/features/chat/chat-controller.test.ts b/tests/features/chat/chat-controller.test.ts index dcf6df7d..2c402310 100644 --- a/tests/features/chat/chat-controller.test.ts +++ b/tests/features/chat/chat-controller.test.ts @@ -1375,7 +1375,7 @@ describe("ChatController", () => { expect(state.activeServiceTier).toBeNull(); }); - it("applies goal notifications to thread state without adding system messages", () => { + it("adds system messages for goal state changes from notifications", () => { const state = createChatState(); state.activeThreadId = "thread-active"; const controller = controllerForState(state); @@ -1396,7 +1396,48 @@ describe("ChatController", () => { } satisfies Extract); expect(state.activeGoal).toEqual(goal); - expect(state.displayItems).toEqual([]); + expect(state.displayItems.at(-1)).toMatchObject({ kind: "system", text: "Goal set." }); + + const afterSetMessageCount = state.displayItems.length; + controller.handleNotification({ + method: "thread/goal/updated", + params: { threadId: "thread-active", turnId: null, goal }, + } satisfies Extract); + expect(state.displayItems).toHaveLength(afterSetMessageCount); + + const updatedGoal = { ...goal, objective: "Finish well", updatedAt: 2 }; + controller.handleNotification({ + method: "thread/goal/updated", + params: { threadId: "thread-active", turnId: null, goal: updatedGoal }, + } satisfies Extract); + expect(state.displayItems.at(-1)).toMatchObject({ kind: "system", text: "Goal updated." }); + + const pausedGoal = { ...updatedGoal, status: "paused", updatedAt: 3 } satisfies Extract< + ServerNotification, + { method: "thread/goal/updated" } + >["params"]["goal"]; + controller.handleNotification({ + method: "thread/goal/updated", + params: { threadId: "thread-active", turnId: null, goal: pausedGoal }, + } satisfies Extract); + expect(state.displayItems.at(-1)).toMatchObject({ kind: "system", text: "Goal paused." }); + + const resumedGoal = { ...pausedGoal, status: "active", updatedAt: 4 } satisfies Extract< + ServerNotification, + { method: "thread/goal/updated" } + >["params"]["goal"]; + controller.handleNotification({ + method: "thread/goal/updated", + params: { threadId: "thread-active", turnId: null, goal: resumedGoal }, + } satisfies Extract); + expect(state.displayItems.at(-1)).toMatchObject({ kind: "system", text: "Goal resumed." }); + + const messageCount = state.displayItems.length; + controller.handleNotification({ + method: "thread/goal/updated", + params: { threadId: "thread-active", turnId: null, goal: { ...resumedGoal, tokensUsed: 10, timeUsedSeconds: 20 } }, + } satisfies Extract); + expect(state.displayItems).toHaveLength(messageCount); controller.handleNotification({ method: "thread/goal/cleared", @@ -1404,7 +1445,46 @@ describe("ChatController", () => { } satisfies Extract); expect(state.activeGoal).toBeNull(); - expect(state.displayItems).toEqual([]); + expect(state.displayItems.at(-1)).toMatchObject({ kind: "system", text: "Goal cleared." }); + }); + + it("adds a system message when a goal completes", () => { + const state = createChatState(); + state.activeThreadId = "thread-active"; + state.activeGoal = { + threadId: "thread-active", + objective: "Finish", + status: "active", + tokenBudget: null, + tokensUsed: 12, + timeUsedSeconds: 60, + createdAt: 1, + updatedAt: 1, + }; + const controller = controllerForState(state); + const completedGoal = { + ...state.activeGoal, + status: "complete", + tokensUsed: 42, + timeUsedSeconds: 120, + updatedAt: 2, + } satisfies Extract["params"]["goal"]; + + controller.handleNotification({ + method: "thread/goal/updated", + params: { threadId: "thread-active", turnId: "turn-1", goal: completedGoal }, + } satisfies Extract); + + expect(state.activeGoal).toEqual(completedGoal); + expect(state.displayItems).toHaveLength(1); + expect(state.displayItems[0]).toMatchObject({ kind: "system", text: "Goal completed." }); + + controller.handleNotification({ + method: "thread/goal/updated", + params: { threadId: "thread-active", turnId: "turn-1", goal: { ...completedGoal, tokensUsed: 43 } }, + } satisfies Extract); + + expect(state.displayItems).toHaveLength(1); }); it("ignores goal notifications that do not match the active thread", () => { diff --git a/tests/features/chat/goal-controller.test.ts b/tests/features/chat/goal-controller.test.ts index ef06d58d..922277ea 100644 --- a/tests/features/chat/goal-controller.test.ts +++ b/tests/features/chat/goal-controller.test.ts @@ -58,7 +58,8 @@ describe("ChatGoalController", () => { state.activeGoal = goal({ tokenBudget: 500 }); const stateStore = createChatStateStore(state); const updated = goal({ objective: "Updated", tokenBudget: 250 }); - const setThreadGoal = vi.fn().mockResolvedValue({ goal: updated }); + const paused = goal({ objective: "Updated", status: "paused", tokenBudget: 250 }); + const setThreadGoal = vi.fn().mockResolvedValueOnce({ goal: updated }).mockResolvedValueOnce({ goal: paused }); const clearThreadGoal = vi.fn().mockResolvedValue({ cleared: true }); const client = { setThreadGoal, @@ -87,11 +88,11 @@ describe("ChatGoalController", () => { expect(stateStore.getState().activeGoal).toBeNull(); }); - it("reports goal creation and resume as user-visible system messages", async () => { + it("reports goal creation as a user-visible state change", async () => { const state = createChatState(); state.activeThreadId = "thread"; const stateStore = createChatStateStore(state); - const setThreadGoal = vi.fn().mockResolvedValue({ goal: goal() }); + const setThreadGoal = vi.fn().mockResolvedValueOnce({ goal: goal() }); const client = { setThreadGoal } as unknown as AppServerClient; const addSystemMessage = vi.fn(); const controller = new ChatGoalController({ @@ -104,11 +105,53 @@ describe("ChatGoalController", () => { }); await controller.setObjective("thread", "Finish", null); - await controller.setStatus("thread", "active"); expect(addSystemMessage).toHaveBeenCalledWith("Goal set."); + }); + + it("reports goal resume as a user-visible state change", async () => { + const state = createChatState(); + state.activeThreadId = "thread"; + state.activeGoal = goal({ status: "paused" }); + const stateStore = createChatStateStore(state); + const setThreadGoal = vi.fn().mockResolvedValueOnce({ goal: goal() }); + const client = { setThreadGoal } as unknown as AppServerClient; + const addSystemMessage = vi.fn(); + const controller = new ChatGoalController({ + stateStore, + currentClient: () => client, + ensureConnected: vi.fn().mockResolvedValue(undefined), + addSystemMessage, + render: vi.fn(), + refreshLiveState: vi.fn(), + }); + + await controller.setStatus("thread", "active"); + expect(addSystemMessage).toHaveBeenCalledWith("Goal resumed."); }); + + it("does not report initial goal sync as a user-visible state change", async () => { + const state = createChatState(); + state.activeThreadId = "thread"; + const stateStore = createChatStateStore(state); + const currentGoal = goal(); + const client = { getThreadGoal: vi.fn().mockResolvedValue({ goal: currentGoal }) } as unknown as AppServerClient; + const addSystemMessage = vi.fn(); + const controller = new ChatGoalController({ + stateStore, + currentClient: () => client, + ensureConnected: vi.fn().mockResolvedValue(undefined), + addSystemMessage, + render: vi.fn(), + refreshLiveState: vi.fn(), + }); + + await controller.syncThreadGoal("thread"); + + expect(stateStore.getState().activeGoal).toEqual(currentGoal); + expect(addSystemMessage).not.toHaveBeenCalled(); + }); }); function goal(overrides: Partial = {}): ThreadGoal {