fix(codex): stabilize goal status updates (#651)

This commit is contained in:
SmallSpider
2026-05-20 11:31:28 +08:00
committed by GitHub
parent 2aaae25d0a
commit 25631d971c
4 changed files with 257 additions and 18 deletions
+47
View File
@@ -23,6 +23,7 @@ const harness = vi.hoisted(() => ({
goalGetCalls: [] as unknown[],
goalClearCalls: [] as unknown[],
goal: null as Record<string, unknown> | 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 };
if (!harness.suppressGoalNotifications) {
harness.notifications.push({ method: 'thread/goal/updated', params: notification });
this.notificationHandler?.('thread/goal/updated', notification);
}
return { goal: harness.goal };
}
@@ -156,9 +159,11 @@ vi.mock('./codexAppServerClient', () => {
harness.goal = null;
if (cleared) {
const notification = { threadId: params?.threadId ?? 'thread-unknown' };
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',
+76 -7
View File
@@ -1855,6 +1855,39 @@ class CodexRemoteLauncher extends RemoteLauncherBase {
};
const forwardedGoalSignaturesByThreadId = new Map<string, string>();
const forwardedGoalClearsByThreadId = new Set<string>();
const adminInterruptedTurnIds = new Set<string>();
const adminInterruptedTurnTimers = new Map<string, ReturnType<typeof setTimeout>>();
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<string, unknown>, 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<string, unknown>) => {
@@ -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<string, unknown>) => {
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,6 +3085,7 @@ class CodexRemoteLauncher extends RemoteLauncherBase {
childAgentRuntimeById.clear();
session.onThinkingChange(false);
clearReadyAfterTurnTimer?.();
if (!suppressReadyAfterMessage) {
emitReadyIfIdle({
pending: pending ?? (recoveryInFlight ? activeMessage : null),
queueSize: () => session.queue.size(),
@@ -3028,6 +3093,10 @@ class CodexRemoteLauncher extends RemoteLauncherBase {
sendReady
});
}
}
if (suppressReadyAfterMessage) {
suppressReadyForAdminCommand = false;
}
logActiveHandles('after-turn');
}
}
+112
View File
@@ -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()
})
})
+12 -1
View File
@@ -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