From 25631d971c1633031766bb0fbb5fe1544842acc0 Mon Sep 17 00:00:00 2001 From: SmallSpider <568442079@qq.com> Date: Wed, 20 May 2026 11:31:28 +0800 Subject: [PATCH] fix(codex): stabilize goal status updates (#651) --- cli/src/codex/codexRemoteLauncher.test.ts | 55 ++++++++++- cli/src/codex/codexRemoteLauncher.ts | 95 +++++++++++++++--- web/src/chat/reducer.test.ts | 112 ++++++++++++++++++++++ web/src/chat/reducer.ts | 13 ++- 4 files changed, 257 insertions(+), 18 deletions(-) diff --git a/cli/src/codex/codexRemoteLauncher.test.ts b/cli/src/codex/codexRemoteLauncher.test.ts index a3b834f4..c9db9ec5 100644 --- a/cli/src/codex/codexRemoteLauncher.test.ts +++ b/cli/src/codex/codexRemoteLauncher.test.ts @@ -23,6 +23,7 @@ const harness = vi.hoisted(() => ({ goalGetCalls: [] as unknown[], goalClearCalls: [] as unknown[], goal: null as Record | null, + suppressGoalNotifications: false, suppressTurnCompletion: false, remainingThreadSystemErrors: 0, startTurnMessages: [] as string[], @@ -140,8 +141,10 @@ vi.mock('./codexAppServerClient', () => { updatedAt: 2 }; const notification = { threadId, goal: harness.goal }; - harness.notifications.push({ method: 'thread/goal/updated', params: notification }); - this.notificationHandler?.('thread/goal/updated', notification); + if (!harness.suppressGoalNotifications) { + harness.notifications.push({ method: 'thread/goal/updated', params: notification }); + this.notificationHandler?.('thread/goal/updated', notification); + } return { goal: harness.goal }; } @@ -156,8 +159,10 @@ vi.mock('./codexAppServerClient', () => { harness.goal = null; if (cleared) { const notification = { threadId: params?.threadId ?? 'thread-unknown' }; - harness.notifications.push({ method: 'thread/goal/cleared', params: notification }); - this.notificationHandler?.('thread/goal/cleared', notification); + if (!harness.suppressGoalNotifications) { + harness.notifications.push({ method: 'thread/goal/cleared', params: notification }); + this.notificationHandler?.('thread/goal/cleared', notification); + } } return { cleared }; } @@ -946,6 +951,7 @@ describe('codexRemoteLauncher', () => { harness.goalGetCalls = []; harness.goalClearCalls = []; harness.goal = null; + harness.suppressGoalNotifications = false; harness.suppressTurnCompletion = false; harness.startTurnMessages = []; harness.failResumeThreadIds = []; @@ -1175,6 +1181,9 @@ describe('codexRemoteLauncher', () => { type: 'message', message: 'Goal active' }); + expect(sessionEvents).not.toContainEqual({ + type: 'ready' + }); expect(codexMessages).toEqual(expect.arrayContaining([ expect.objectContaining({ type: 'thread_goal_updated', @@ -1206,6 +1215,44 @@ describe('codexRemoteLauncher', () => { }); }); + it('forwards goal RPC responses when the app-server does not emit goal notifications', async () => { + harness.suppressGoalNotifications = true; + const { session, codexMessages } = createSessionStub(['/goal improve benchmark coverage']); + + const exitReason = await codexRemoteLauncher(session as never); + + expect(exitReason).toBe('exit'); + expect(codexMessages).toEqual(expect.arrayContaining([ + expect.objectContaining({ + type: 'thread_goal_updated', + thread_id: 'thread-1', + goal: expect.objectContaining({ + objective: 'improve benchmark coverage', + status: 'active' + }) + }) + ])); + }); + + it('does not emit ready when a goal command interrupts an active turn', async () => { + harness.suppressTurnCompletion = true; + harness.emitTurnAbortedOnInterrupt = true; + const { session, sessionEvents } = createSessionStub(['first message', '/goal improve benchmark coverage']); + + const exitReason = await codexRemoteLauncher(session as never); + + expect(exitReason).toBe('exit'); + expect(harness.interruptedTurns).toEqual([{ threadId: 'thread-1', turnId: 'turn-1' }]); + expect(harness.goalSetCalls).toEqual([{ + threadId: 'thread-1', + objective: 'improve benchmark coverage', + status: 'active' + }]); + expect(sessionEvents).not.toContainEqual({ + type: 'ready' + }); + }); + it('switches collaboration mode to default after approving exit_plan_mode', async () => { const { session, rpcHandlers, collaborationModes, getCollaborationMode } = createSessionStub(['plan this'], { permissionMode: 'default', diff --git a/cli/src/codex/codexRemoteLauncher.ts b/cli/src/codex/codexRemoteLauncher.ts index 82000a78..5eb64e51 100644 --- a/cli/src/codex/codexRemoteLauncher.ts +++ b/cli/src/codex/codexRemoteLauncher.ts @@ -1855,6 +1855,39 @@ class CodexRemoteLauncher extends RemoteLauncherBase { }; const forwardedGoalSignaturesByThreadId = new Map(); + const forwardedGoalClearsByThreadId = new Set(); + const adminInterruptedTurnIds = new Set(); + const adminInterruptedTurnTimers = new Map>(); + + const suppressReadyForInterruptedTurn = (turnId: string | null) => { + if (!turnId) { + return; + } + adminInterruptedTurnIds.add(turnId); + const previousTimer = adminInterruptedTurnTimers.get(turnId); + if (previousTimer) { + clearTimeout(previousTimer); + } + const timer = setTimeout(() => { + adminInterruptedTurnIds.delete(turnId); + adminInterruptedTurnTimers.delete(turnId); + }, 30_000); + timer.unref?.(); + adminInterruptedTurnTimers.set(turnId, timer); + }; + + const consumeInterruptedTurnReadySuppression = (turnId: string | null): boolean => { + if (!turnId || !adminInterruptedTurnIds.has(turnId)) { + return false; + } + adminInterruptedTurnIds.delete(turnId); + const timer = adminInterruptedTurnTimers.get(turnId); + if (timer) { + clearTimeout(timer); + adminInterruptedTurnTimers.delete(turnId); + } + return true; + }; const shouldForwardGoalUpdate = (msg: Record, threadId: string | null): boolean => { const goal = asRecord(msg.goal); @@ -1871,14 +1904,22 @@ class CodexRemoteLauncher extends RemoteLauncherBase { return false; } + forwardedGoalClearsByThreadId.delete(scopedThreadId); forwardedGoalSignaturesByThreadId.set(scopedThreadId, signature); return true; }; - const noteGoalCleared = (threadId: string | null) => { - if (threadId) { - forwardedGoalSignaturesByThreadId.delete(threadId); + const shouldForwardGoalClear = (threadId: string | null): boolean => { + if (!threadId) { + return true; } + if (forwardedGoalClearsByThreadId.has(threadId)) { + logger.debug(`[Codex] Suppressing duplicate thread goal clear; threadId=${threadId}`); + return false; + } + forwardedGoalClearsByThreadId.add(threadId); + forwardedGoalSignaturesByThreadId.delete(threadId); + return true; }; const handleCodexEvent = (msg: Record) => { @@ -1887,6 +1928,9 @@ class CodexRemoteLauncher extends RemoteLauncherBase { const eventTurnId = asString(msg.turn_id ?? msg.turnId); const eventThreadId = asString(msg.thread_id ?? msg.threadId); const isTerminalEvent = msgType === 'task_complete' || msgType === 'turn_aborted' || msgType === 'task_failed'; + const suppressReadyForThisTerminalEvent = isTerminalEvent + ? consumeInterruptedTurnReadySuppression(eventTurnId) + : false; if (msgType === 'thread_started') { const threadId = asString(msg.thread_id ?? msg.threadId); @@ -1947,7 +1991,9 @@ class CodexRemoteLauncher extends RemoteLauncherBase { } if (msgType === 'thread_goal_cleared') { - noteGoalCleared(eventThreadId ?? this.currentThreadId); + if (!shouldForwardGoalClear(eventThreadId ?? this.currentThreadId)) { + return; + } session.sendAgentMessage({ ...addCodexEventScope(msg, 'parent', eventThreadId ?? this.currentThreadId), id: randomUUID() @@ -2091,9 +2137,9 @@ class CodexRemoteLauncher extends RemoteLauncherBase { wakeLoop(); } - if (isTerminalEvent && !turnInFlight) { + if (isTerminalEvent && !turnInFlight && !suppressReadyForThisTerminalEvent) { scheduleReadyAfterTurn?.(); - } else if (readyAfterTurnTimer && msgType !== 'task_started') { + } else if (readyAfterTurnTimer && msgType !== 'task_started' && !suppressReadyForThisTerminalEvent) { scheduleReadyAfterTurn?.(); } @@ -2496,6 +2542,7 @@ class CodexRemoteLauncher extends RemoteLauncherBase { let hasThread = false; let pending: QueuedMessage | null = null; + let suppressReadyForAdminCommand = false; clearReadyAfterTurnTimer = () => { if (!readyAfterTurnTimer) { @@ -2507,8 +2554,14 @@ class CodexRemoteLauncher extends RemoteLauncherBase { scheduleReadyAfterTurn = () => { clearReadyAfterTurnTimer?.(); + if (suppressReadyForAdminCommand) { + return; + } readyAfterTurnTimer = setTimeout(() => { readyAfterTurnTimer = null; + if (suppressReadyForAdminCommand) { + return; + } emitReadyIfIdle({ pending: pending ?? (recoveryInFlight ? activeMessage : null), queueSize: () => session.queue.size(), @@ -2527,7 +2580,9 @@ class CodexRemoteLauncher extends RemoteLauncherBase { const sendGoalEvent = (event: Record) => { const threadId = asString(event.thread_id ?? event.threadId) ?? this.currentThreadId; if (event.type === 'thread_goal_cleared') { - noteGoalCleared(threadId); + if (!shouldForwardGoalClear(threadId)) { + return; + } } else if (event.type === 'thread_goal_updated' && !shouldForwardGoalUpdate(event, threadId)) { return; } @@ -2549,6 +2604,7 @@ class CodexRemoteLauncher extends RemoteLauncherBase { }; const interruptActiveTurn = async () => { + suppressReadyForInterruptedTurn(this.currentTurnId); await this.interruptActiveTurns('slash command'); }; @@ -2751,6 +2807,7 @@ class CodexRemoteLauncher extends RemoteLauncherBase { } else { sendVisibleStatus('No goal to clear'); } + sendGoalEvent({ type: 'thread_goal_cleared', thread_id: threadId }); return true; } @@ -2764,6 +2821,7 @@ class CodexRemoteLauncher extends RemoteLauncherBase { }); const goal = normalizeGoal(response.goal); sendVisibleStatus(formatGoalUsage(goal)); + sendGoalEvent({ type: 'thread_goal_updated', thread_id: threadId, goal }); } catch (error) { const detail = error instanceof Error ? error.message : String(error); if (/goals feature is disabled|unsupported remote app-server request|method not found/i.test(detail)) { @@ -2860,6 +2918,12 @@ class CodexRemoteLauncher extends RemoteLauncherBase { messageBuffer.addMessage(message.message, 'user'); } activeMessage = message; + const isGoalCommand = parseGoalCommand(message.message) !== null; + let suppressReadyAfterMessage = isGoalCommand; + if (isGoalCommand) { + suppressReadyForAdminCommand = true; + clearReadyAfterTurnTimer?.(); + } try { if (await handleGoalCommand(message)) { @@ -3021,12 +3085,17 @@ class CodexRemoteLauncher extends RemoteLauncherBase { childAgentRuntimeById.clear(); session.onThinkingChange(false); clearReadyAfterTurnTimer?.(); - emitReadyIfIdle({ - pending: pending ?? (recoveryInFlight ? activeMessage : null), - queueSize: () => session.queue.size(), - shouldExit: this.shouldExit, - sendReady - }); + if (!suppressReadyAfterMessage) { + emitReadyIfIdle({ + pending: pending ?? (recoveryInFlight ? activeMessage : null), + queueSize: () => session.queue.size(), + shouldExit: this.shouldExit, + sendReady + }); + } + } + if (suppressReadyAfterMessage) { + suppressReadyForAdminCommand = false; } logActiveHandles('after-turn'); } diff --git a/web/src/chat/reducer.test.ts b/web/src/chat/reducer.test.ts index edf3ea7f..f44fa4dd 100644 --- a/web/src/chat/reducer.test.ts +++ b/web/src/chat/reducer.test.ts @@ -1,6 +1,57 @@ import { describe, expect, it } from 'vitest' import { reduceChatBlocks } from './reducer' import type { NormalizedMessage } from './types' +import type { ThreadGoal, ThreadGoalStatus } from '@/types/api' + +function userMessage(id: string, text: string, createdAt: number): NormalizedMessage { + return { + id, + localId: null, + createdAt, + role: 'user', + content: { type: 'text', text }, + isSidechain: false + } +} + +function goalMessage(id: string, status: ThreadGoalStatus, createdAt: number): NormalizedMessage { + const goal: ThreadGoal = { + threadId: 'thread-1', + objective: 'ship goal support', + status, + tokenBudget: null, + tokensUsed: 0, + timeUsedSeconds: 0, + createdAt, + updatedAt: createdAt + } + return { + id, + localId: null, + createdAt, + role: 'event', + content: { + type: 'thread-goal-updated', + threadId: 'thread-1', + goal + }, + isSidechain: false + } +} + +function goalClearedMessage(id: string, createdAt: number): NormalizedMessage { + return { + id, + localId: null, + createdAt, + role: 'event', + content: { + type: 'thread-goal-cleared', + threadId: 'thread-1' + }, + isSidechain: false + } +} describe('reduceChatBlocks', () => { it('ignores child agent usage when calculating parent latest usage', () => { @@ -43,4 +94,65 @@ describe('reduceChatBlocks', () => { contextSize: 100 }) }) + + it('keeps active goals visible across later normal user messages', () => { + const reduced = reduceChatBlocks([ + goalMessage('goal-active', 'active', 1), + userMessage('user-later', 'continue working', 2) + ], null) + + expect(reduced.latestGoal).toMatchObject({ + status: 'active', + objective: 'ship goal support' + }) + }) + + it('keeps a completed goal visible when it is the latest relevant event', () => { + const reduced = reduceChatBlocks([ + goalMessage('goal-complete', 'complete', 1) + ], null) + + expect(reduced.latestGoal).toMatchObject({ + status: 'complete', + objective: 'ship goal support' + }) + }) + + it('hides a completed goal after a later non-goal user message', () => { + const reduced = reduceChatBlocks([ + goalMessage('goal-complete', 'complete', 1), + userMessage('user-later', 'start a new task', 2) + ], null) + + expect(reduced.latestGoal).toBeNull() + }) + + it('does not treat later goal slash commands as non-goal activity', () => { + const reduced = reduceChatBlocks([ + goalMessage('goal-complete', 'complete', 1), + userMessage('user-later', '/goal', 2) + ], null) + + expect(reduced.latestGoal).toMatchObject({ + status: 'complete' + }) + }) + + it('treats slash commands with a goal prefix as non-goal activity', () => { + const reduced = reduceChatBlocks([ + goalMessage('goal-complete', 'complete', 1), + userMessage('user-later', '/goal-foo', 2) + ], null) + + expect(reduced.latestGoal).toBeNull() + }) + + it('clears latest goal after an explicit goal clear event', () => { + const reduced = reduceChatBlocks([ + goalMessage('goal-active', 'active', 1), + goalClearedMessage('goal-cleared', 2) + ], null) + + expect(reduced.latestGoal).toBeNull() + }) }) diff --git a/web/src/chat/reducer.ts b/web/src/chat/reducer.ts index 47f31279..0e2fa8f0 100644 --- a/web/src/chat/reducer.ts +++ b/web/src/chat/reducer.ts @@ -29,13 +29,24 @@ export type LatestUsage = { } function getLatestThreadGoal(normalized: NormalizedMessage[]): ThreadGoal | null { + let sawNewerNonGoalUserMessage = false for (let i = normalized.length - 1; i >= 0; i--) { const msg = normalized[i] + if (msg.role === 'user') { + if (!/^\s*\/goal(?:\s|$)/i.test(msg.content.text)) { + sawNewerNonGoalUserMessage = true + } + continue + } if (msg.role !== 'event') continue const event = msg.content as AgentEvent if (event.type === 'thread-goal-cleared') return null if (event.type === 'thread-goal-updated') { - return (event as { goal?: ThreadGoal }).goal ?? null + const goal = (event as { goal?: ThreadGoal }).goal ?? null + if (goal?.status === 'complete' && sawNewerNonGoalUserMessage) { + return null + } + return goal } } return null