From d8d9462fb4cbffba4aee31e5614e5411aa50d911 Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Wed, 1 Jul 2026 10:56:36 +0200 Subject: [PATCH] Emit goal changes as session metadata These were quite noisy and not always helpful. We can revisit with a new status api in ACP --- src/CodexAcpServer.ts | 3 +- src/CodexEventHandler.ts | 42 +++------------- .../CodexACPAgent/CodexAcpClient.test.ts | 17 ++++++- .../data/thread-goal-cleared.json | 9 ++-- .../data/thread-goal-updated-multiline.json | 13 +++-- .../data/thread-goal-updated.json | 13 +++-- .../CodexACPAgent/thread-goal-events.test.ts | 50 ++++++++++++++++--- 7 files changed, 90 insertions(+), 57 deletions(-) diff --git a/src/CodexAcpServer.ts b/src/CodexAcpServer.ts index 80486fe6..e953f604 100644 --- a/src/CodexAcpServer.ts +++ b/src/CodexAcpServer.ts @@ -13,6 +13,7 @@ import type { Model, ReasoningEffortOption, Thread, + ThreadGoalStatus, ThreadItem, TurnCompletedNotification, UserInput @@ -69,7 +70,7 @@ import {resolveTerminalOutputMode, type TerminalOutputMode} from "./TerminalOutp export interface ThreadGoalSnapshot { objective: string; - status: string; + status: ThreadGoalStatus; tokenBudget: number | null; } diff --git a/src/CodexEventHandler.ts b/src/CodexEventHandler.ts index 4ed0cdbe..e72c5b90 100644 --- a/src/CodexEventHandler.ts +++ b/src/CodexEventHandler.ts @@ -273,35 +273,9 @@ export class CodexEventHandler { } this.sessionState.currentGoal = goalSnapshot; - const status = this.formatThreadGoalStatus(event.goal.status); - const objective = goalSnapshot.objective; - const text = objective.includes("\n") - ? `Goal updated (${status}):\n${objective}` - : `Goal updated (${status}): ${objective}`; - return { - sessionUpdate: "agent_message_chunk", - content: { - type: "text", - text: `\n\n${text}\n\n`, - }, - }; - } - - private formatThreadGoalStatus(status: ThreadGoalUpdatedNotification["goal"]["status"]): string { - switch (status) { - case "active": - return "active"; - case "paused": - return "paused"; - case "budgetLimited": - return "budget limited"; - case "blocked": - return "blocked"; - case "usageLimited": - return "usage limited"; - case "complete": - return "complete"; - } + return this.createCodexSessionInfoUpdate({ + goal: goalSnapshot, + }); } private createThreadGoalClearedEvent(_event: ThreadGoalClearedNotification): UpdateSessionEvent | null { @@ -310,13 +284,9 @@ export class CodexEventHandler { } this.sessionState.currentGoal = null; - return { - sessionUpdate: "agent_message_chunk", - content: { - type: "text", - text: "\n\nGoal cleared.\n\n", - }, - }; + return this.createCodexSessionInfoUpdate({ + goal: null, + }); } private createThreadGoalSnapshot(event: ThreadGoalUpdatedNotification): ThreadGoalSnapshot { diff --git a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts index 77077e5e..3f98b757 100644 --- a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts +++ b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts @@ -1660,7 +1660,22 @@ describe('ACP server test', { timeout: 40_000 }, () => { await expect(promptPromise).resolves.toEqual(expect.objectContaining({ stopReason: "end_turn", })); - expect(mockFixture.getAcpConnectionDump([])).toContain("Goal updated (active): Ship the migration and keep tests green"); + expect(mockFixture.getAcpConnectionEvents([])).toContainEqual(expect.objectContaining({ + args: [expect.objectContaining({ + update: { + sessionUpdate: "session_info_update", + _meta: { + codex: { + goal: { + objective: "Ship the migration and keep tests green", + status: "active", + tokenBudget: null, + }, + }, + }, + }, + })], + })); }); it('completes goal slash command when a turn routes after the goal update', async () => { diff --git a/src/__tests__/CodexACPAgent/data/thread-goal-cleared.json b/src/__tests__/CodexACPAgent/data/thread-goal-cleared.json index c6fc87f1..acf61a68 100644 --- a/src/__tests__/CodexACPAgent/data/thread-goal-cleared.json +++ b/src/__tests__/CodexACPAgent/data/thread-goal-cleared.json @@ -4,10 +4,11 @@ { "sessionId": "test-session-id", "update": { - "sessionUpdate": "agent_message_chunk", - "content": { - "type": "text", - "text": "\n\nGoal cleared.\n\n" + "sessionUpdate": "session_info_update", + "_meta": { + "codex": { + "goal": null + } } } } diff --git a/src/__tests__/CodexACPAgent/data/thread-goal-updated-multiline.json b/src/__tests__/CodexACPAgent/data/thread-goal-updated-multiline.json index b9336936..c524584c 100644 --- a/src/__tests__/CodexACPAgent/data/thread-goal-updated-multiline.json +++ b/src/__tests__/CodexACPAgent/data/thread-goal-updated-multiline.json @@ -4,10 +4,15 @@ { "sessionId": "test-session-id", "update": { - "sessionUpdate": "agent_message_chunk", - "content": { - "type": "text", - "text": "\n\nGoal updated (budget limited):\nFirst task\nSecond task\n\n" + "sessionUpdate": "session_info_update", + "_meta": { + "codex": { + "goal": { + "objective": "First task\nSecond task", + "status": "budgetLimited", + "tokenBudget": 1000 + } + } } } } diff --git a/src/__tests__/CodexACPAgent/data/thread-goal-updated.json b/src/__tests__/CodexACPAgent/data/thread-goal-updated.json index 264c1e34..bc14e9cb 100644 --- a/src/__tests__/CodexACPAgent/data/thread-goal-updated.json +++ b/src/__tests__/CodexACPAgent/data/thread-goal-updated.json @@ -4,10 +4,15 @@ { "sessionId": "test-session-id", "update": { - "sessionUpdate": "agent_message_chunk", - "content": { - "type": "text", - "text": "\n\nGoal updated (active): Ship the goal update\n\n" + "sessionUpdate": "session_info_update", + "_meta": { + "codex": { + "goal": { + "objective": "Ship the goal update", + "status": "active", + "tokenBudget": null + } + } } } } diff --git a/src/__tests__/CodexACPAgent/thread-goal-events.test.ts b/src/__tests__/CodexACPAgent/thread-goal-events.test.ts index 2db982de..659b0892 100644 --- a/src/__tests__/CodexACPAgent/thread-goal-events.test.ts +++ b/src/__tests__/CodexACPAgent/thread-goal-events.test.ts @@ -18,7 +18,7 @@ describe("CodexEventHandler - thread goal events", () => { vi.clearAllMocks(); }); - it("should send thread goal updates as agent messages", async () => { + it("should send thread goal updates as session metadata", async () => { const goalUpdatedNotification: ServerNotification = { method: "thread/goal/updated", params: { @@ -44,7 +44,7 @@ describe("CodexEventHandler - thread goal events", () => { ); }); - it("should format multiline thread goal updates", async () => { + it("should trim multiline thread goal objectives in session metadata", async () => { const goalUpdatedNotification: ServerNotification = { method: "thread/goal/updated", params: { @@ -70,7 +70,7 @@ describe("CodexEventHandler - thread goal events", () => { ); }); - it("should send thread goal cleared as an agent message", async () => { + it("should send thread goal cleared as session metadata", async () => { const goalClearedNotification: ServerNotification = { method: "thread/goal/cleared", params: { @@ -123,10 +123,21 @@ describe("CodexEventHandler - thread goal events", () => { const events = mockFixture.getAcpConnectionEvents([]); expect(events).toHaveLength(1); - expect(events[0]!.args[0].update.content.text).toBe("\n\nGoal updated (active): Ship the goal update\n\n"); + expect(events[0]!.args[0].update).toEqual({ + sessionUpdate: "session_info_update", + _meta: { + codex: { + goal: { + objective: "Ship the goal update", + status: "active", + tokenBudget: null, + }, + }, + }, + }); }); - it("should separate completed goal updates from preceding agent text", async () => { + it("should not append completed goal updates to preceding agent text", async () => { const goalCompletedNotification: ServerNotification = { method: "thread/goal/updated", params: { @@ -160,7 +171,25 @@ describe("CodexEventHandler - thread goal events", () => { const events = mockFixture.getAcpConnectionEvents([]); expect(events).toHaveLength(2); - expect(events[1]!.args[0].update.content.text).toBe("\n\nGoal updated (complete): tell me a joke\n\n"); + expect(events[0]!.args[0].update).toEqual({ + sessionUpdate: "agent_message_chunk", + content: { + type: "text", + text: "Because they kept losing interest in `any`.", + }, + }); + expect(events[1]!.args[0].update).toEqual({ + sessionUpdate: "session_info_update", + _meta: { + codex: { + goal: { + objective: "tell me a joke", + status: "complete", + tokenBudget: null, + }, + }, + }, + }); }); it("should suppress duplicate thread goal cleared notifications", async () => { @@ -178,7 +207,14 @@ describe("CodexEventHandler - thread goal events", () => { const events = mockFixture.getAcpConnectionEvents([]); expect(events).toHaveLength(1); - expect(events[0]!.args[0].update.content.text).toBe("\n\nGoal cleared.\n\n"); + expect(events[0]!.args[0].update).toEqual({ + sessionUpdate: "session_info_update", + _meta: { + codex: { + goal: null, + }, + }, + }); }); function createSessionState(): SessionState {