Fix Codex subagent final result preservation (#602)

This commit is contained in:
SmallSpider
2026-05-09 08:58:31 +08:00
committed by GitHub
parent 752a505973
commit 3eac3456b4
4 changed files with 352 additions and 33 deletions
+201 -2
View File
@@ -21,6 +21,12 @@ const harness = vi.hoisted(() => ({
emitChildThreadEvents: false,
emitChildUsageEvents: false,
emitChildReasoningBurst: false,
emitChildDoneStatusWithoutMessage: false,
emitChildWaitStructuredOutput: false,
emitChildTaskCompleteBeforeMessage: false,
suppressChildTaskCompleteEvent: false,
emitSecondChildMessage: false,
emitLateChildCommandAfterParentTool: false,
emitParentUsageEvents: false,
emitChildNestedAgentTool: false,
emitParentTitleChange: false,
@@ -228,6 +234,19 @@ vi.mock('./codexAppServerClient', () => {
const childThreadId = 'child-thread';
const childTurnId = 'child-turn';
const childMessage = 'child output should stay hidden';
const secondChildMessage = 'final child output should win';
const emitChildDone = () => {
const childDone = {
msg: {
type: 'task_complete',
thread_id: childThreadId,
turn_id: childTurnId
}
};
harness.notifications.push({ method: 'codex/event/task_complete', params: childDone });
this.notificationHandler?.('codex/event/task_complete', childDone);
};
if (harness.emitChildReasoningBurst) {
for (let i = 0; i < 20; i += 1) {
@@ -245,6 +264,10 @@ vi.mock('./codexAppServerClient', () => {
}
}
if (harness.emitChildDoneStatusWithoutMessage && harness.emitChildTaskCompleteBeforeMessage) {
emitChildDone();
}
const childMessageCompleted = {
item: {
id: 'child-msg-1',
@@ -257,6 +280,28 @@ vi.mock('./codexAppServerClient', () => {
harness.notifications.push({ method: 'item/completed', params: childMessageCompleted });
this.notificationHandler?.('item/completed', childMessageCompleted);
if (harness.emitSecondChildMessage) {
const secondChildMessageCompleted = {
item: {
id: 'child-msg-2',
type: 'agentMessage',
content: [{ type: 'text', text: secondChildMessage }]
},
threadId: childThreadId,
turnId: childTurnId
};
harness.notifications.push({ method: 'item/completed', params: secondChildMessageCompleted });
this.notificationHandler?.('item/completed', secondChildMessageCompleted);
}
if (
harness.emitChildDoneStatusWithoutMessage
&& !harness.emitChildTaskCompleteBeforeMessage
&& !harness.suppressChildTaskCompleteEvent
) {
emitChildDone();
}
if (harness.emitChildUsageEvents) {
const childUsage = {
tokenUsage: {
@@ -413,8 +458,15 @@ vi.mock('./codexAppServerClient', () => {
receiverThreadIds: [childThreadId],
agentsStates: {
[childThreadId]: {
status: 'completed',
message: childMessage
status: harness.emitChildDoneStatusWithoutMessage ? 'done' : 'completed',
message: harness.emitChildWaitStructuredOutput
? ''
: harness.emitChildDoneStatusWithoutMessage
? null
: harness.emitSecondChildMessage
? secondChildMessage
: childMessage,
...(harness.emitChildWaitStructuredOutput ? { output: { value: 42 } } : {})
}
}
},
@@ -489,6 +541,20 @@ vi.mock('./codexAppServerClient', () => {
harness.notifications.push({ method: 'item/completed', params: resumeCompleted });
this.notificationHandler?.('item/completed', resumeCompleted);
}
if (harness.emitLateChildCommandAfterParentTool) {
const lateChildCommandStart = {
item: {
id: 'late-child-cmd',
type: 'commandExecution',
command: 'echo late'
},
threadId: childThreadId,
turnId: childTurnId
};
harness.notifications.push({ method: 'item/started', params: lateChildCommandStart });
this.notificationHandler?.('item/started', lateChildCommandStart);
}
}
const completed = { status: 'Completed', turn: { id: turnId } };
@@ -658,6 +724,12 @@ describe('codexRemoteLauncher', () => {
harness.emitChildThreadEvents = false;
harness.emitChildUsageEvents = false;
harness.emitChildReasoningBurst = false;
harness.emitChildDoneStatusWithoutMessage = false;
harness.emitChildWaitStructuredOutput = false;
harness.emitChildTaskCompleteBeforeMessage = false;
harness.suppressChildTaskCompleteEvent = false;
harness.emitSecondChildMessage = false;
harness.emitLateChildCommandAfterParentTool = false;
harness.emitParentUsageEvents = false;
harness.emitChildNestedAgentTool = false;
harness.emitParentTitleChange = false;
@@ -954,6 +1026,108 @@ describe('codexRemoteLauncher', () => {
}));
});
it('keeps the child final message as result when wait_agent only reports done', async () => {
harness.emitChildThreadEvents = true;
harness.emitChildDoneStatusWithoutMessage = true;
const { session, codexMessages } = createSessionStub();
await codexRemoteLauncher(session as never);
const completedUpdates = codexMessages.filter((message) => {
const record = message as Record<string, unknown>;
return record.type === 'agent-run-update'
&& record.agentId === 'child-thread'
&& record.status === 'completed';
}) as Array<Record<string, unknown>>;
expect(completedUpdates).toContainEqual(expect.objectContaining({
result: 'child output should stay hidden',
activity: 'Completed: child output should stay hidden'
}));
expect(completedUpdates).not.toContainEqual(expect.objectContaining({
result: expect.objectContaining({
status: 'done'
})
}));
});
it('fills wait_agent done without message from the latest child message', async () => {
harness.emitChildThreadEvents = true;
harness.emitChildDoneStatusWithoutMessage = true;
harness.suppressChildTaskCompleteEvent = true;
harness.emitSecondChildMessage = true;
const { session, codexMessages } = createSessionStub();
await codexRemoteLauncher(session as never);
const completedUpdates = codexMessages.filter((message) => {
const record = message as Record<string, unknown>;
return record.type === 'agent-run-update'
&& record.agentId === 'child-thread'
&& record.status === 'completed';
}) as Array<Record<string, unknown>>;
const lastCompleted = completedUpdates.at(-1);
expect(lastCompleted).toEqual(expect.objectContaining({
result: 'final child output should win',
activity: 'Completed: final child output should win'
}));
});
it('preserves wait_agent structured output when status message is empty', async () => {
harness.emitChildThreadEvents = true;
harness.emitChildWaitStructuredOutput = true;
harness.suppressChildTaskCompleteEvent = true;
const { session, codexMessages } = createSessionStub();
await codexRemoteLauncher(session as never);
const completedUpdates = codexMessages.filter((message) => {
const record = message as Record<string, unknown>;
return record.type === 'agent-run-update'
&& record.agentId === 'child-thread'
&& record.status === 'completed';
}) as Array<Record<string, unknown>>;
const lastCompleted = completedUpdates.at(-1);
expect(lastCompleted).toEqual(expect.objectContaining({
result: { value: 42 },
activity: 'Completed: {"value":42}'
}));
});
it('does not regress a completed child agent to running when message arrives late', async () => {
harness.emitChildThreadEvents = true;
harness.emitChildDoneStatusWithoutMessage = true;
harness.emitChildTaskCompleteBeforeMessage = true;
const { session, codexMessages } = createSessionStub();
await codexRemoteLauncher(session as never);
const terminalIndex = codexMessages.findIndex((message) => {
const record = message as Record<string, unknown>;
return record.type === 'agent-run-update'
&& record.agentId === 'child-thread'
&& record.status === 'completed';
});
expect(terminalIndex).toBeGreaterThanOrEqual(0);
const laterUpdates = codexMessages.slice(terminalIndex + 1).filter((message) => {
const record = message as Record<string, unknown>;
return record.type === 'agent-run-update'
&& record.agentId === 'child-thread';
}) as Array<Record<string, unknown>>;
expect(laterUpdates).not.toContainEqual(expect.objectContaining({
status: 'running'
}));
expect(laterUpdates).toContainEqual(expect.objectContaining({
status: 'completed',
result: 'child output should stay hidden',
activity: 'Completed: child output should stay hidden'
}));
});
it('surfaces send_input failures on the target child agent card', async () => {
harness.emitChildThreadEvents = true;
harness.emitParentSendInputFailure = true;
@@ -1011,6 +1185,31 @@ describe('codexRemoteLauncher', () => {
}));
});
it('does not regress a terminal child after resume_agent when a late command starts', async () => {
harness.emitChildThreadEvents = true;
harness.emitParentResumeSuccess = true;
harness.emitLateChildCommandAfterParentTool = true;
const { session, codexMessages } = createSessionStub();
await codexRemoteLauncher(session as never);
expect(codexMessages).toContainEqual(expect.objectContaining({
type: 'agent-run-trace',
agentId: 'child-thread',
message: expect.objectContaining({
type: 'tool-call',
callId: 'late-child-cmd'
})
}));
expect(codexMessages).not.toContainEqual(expect.objectContaining({
type: 'agent-run-update',
agentId: 'child-thread',
activity: 'Running command: echo late',
activityKind: 'running-command',
status: 'running'
}));
});
it('throttles child agent reasoning activity updates instead of emitting one per delta', async () => {
harness.emitChildThreadEvents = true;
harness.emitChildReasoningBurst = true;
+128 -25
View File
@@ -36,6 +36,8 @@ type ChildAgentRuntime = {
}>;
pendingTitleByCallId: Map<string, string>;
reasoningPreview: string;
finalMessage: string | null;
terminal: boolean;
blockedNestedAgent: boolean;
};
@@ -696,7 +698,8 @@ class CodexRemoteLauncher extends RemoteLauncherBase {
if (
isTerminalAgentRunStatus(currentStatus)
&& !isTerminalAgentRunStatus(nextStatus)
&& (activityKind === 'wait_agent' || activityKind === 'close_agent')
&& activityKind !== 'send_input'
&& activityKind !== 'resume_agent'
) {
return;
}
@@ -822,14 +825,64 @@ class CodexRemoteLauncher extends RemoteLauncherBase {
activeToolsByCallId: new Map(),
pendingTitleByCallId: new Map(),
reasoningPreview: '',
finalMessage: null,
terminal: false,
blockedNestedAgent: false
};
childAgentRuntimeById.set(agentId, runtime);
return runtime;
};
const extractAgentStatusMessage = (record: Record<string, unknown>): unknown => {
const message = asString(record.message);
if (message) return message;
for (const key of ['output', 'result', 'finalMessage', 'final_message'] as const) {
const value = record[key];
if (value !== undefined && value !== null) {
return asString(value) ?? value;
}
}
return undefined;
};
const normalizeAgentStateValue = (value: unknown): string | null => {
return asString(value)?.trim().toLowerCase().replace(/[\s_-]/g, '') ?? null;
};
const hasOwn = (record: Record<string, unknown>, key: string): boolean => {
return Object.prototype.hasOwnProperty.call(record, key);
};
const fillCompletedAgentUpdateFromRuntime = (
agentId: string,
update: Record<string, unknown>
): Record<string, unknown> => {
if (asString(update.status) !== 'completed') return update;
if (hasOwn(update, 'result') || hasOwn(update, 'error')) return update;
const result = childAgentRuntimeById.get(agentId)?.finalMessage;
if (!result) return update;
return {
...update,
activity: formatActivity('Completed', result),
result
};
};
const normalizeAgentStatusUpdate = (value: unknown): Record<string, unknown> => {
if (typeof value === 'string') {
const normalized = normalizeAgentStateValue(value);
if (normalized === 'completed' || normalized === 'complete' || normalized === 'done') {
return {
status: 'completed',
statusText: 'Completed',
activity: 'Completed',
activityKind: 'completed'
};
}
const activity = formatActivity('Completed', previewText(value));
return {
status: 'completed',
@@ -862,6 +915,16 @@ class CodexRemoteLauncher extends RemoteLauncherBase {
result: completed
};
}
const done = asString(record.done);
if (done) {
return {
status: 'completed',
statusText: 'Completed',
activity: formatActivity('Completed', done),
activityKind: 'completed',
result: done
};
}
const failed = asString(record.failed ?? record.error);
if (failed) {
return {
@@ -884,7 +947,8 @@ class CodexRemoteLauncher extends RemoteLauncherBase {
}
const rawStatus = asString(record.status ?? record.state);
if (rawStatus === 'notFound' || rawStatus === 'not_found') {
const normalizedStatus = normalizeAgentStateValue(record.status ?? record.state);
if (normalizedStatus === 'notfound') {
const error = record.message ?? record.error ?? value;
return {
status: 'failed',
@@ -894,18 +958,24 @@ class CodexRemoteLauncher extends RemoteLauncherBase {
error
};
}
if (rawStatus === 'completed') {
const result = record.message ?? record.output ?? value;
if (
normalizedStatus === 'completed'
|| normalizedStatus === 'complete'
|| normalizedStatus === 'done'
|| record.completed === true
|| record.done === true
) {
const result = extractAgentStatusMessage(record);
return {
status: 'completed',
statusText: 'Completed',
activity: formatActivity('Completed', previewText(result)),
activityKind: 'completed',
result
...(result !== undefined && result !== null ? { result } : {})
};
}
if (rawStatus === 'failed' || rawStatus === 'error') {
const error = record.message ?? record.error ?? value;
if (normalizedStatus === 'failed' || normalizedStatus === 'error') {
const error = extractAgentStatusMessage(record) ?? record.error ?? value;
return {
status: 'failed',
statusText: 'Failed',
@@ -914,8 +984,8 @@ class CodexRemoteLauncher extends RemoteLauncherBase {
error
};
}
if (rawStatus === 'canceled' || rawStatus === 'cancelled') {
const error = record.message ?? record.error ?? value;
if (normalizedStatus === 'canceled' || normalizedStatus === 'cancelled') {
const error = extractAgentStatusMessage(record) ?? record.error ?? value;
return {
status: 'canceled',
statusText: 'Canceled',
@@ -928,7 +998,7 @@ class CodexRemoteLauncher extends RemoteLauncherBase {
return {
status: rawStatus ?? 'running',
statusText: rawStatus ?? 'Running',
activity: formatActivity(rawStatus ?? 'Running', previewText(record.message ?? record.output ?? value)),
activity: formatActivity(rawStatus ?? 'Running', previewText(extractAgentStatusMessage(record) ?? value)),
activityKind: rawStatus ?? 'running',
result: value
};
@@ -981,10 +1051,19 @@ class CodexRemoteLauncher extends RemoteLauncherBase {
const outputRecord = asRecord(output);
const statusMap = asRecord(outputRecord?.status) ?? {};
for (const [agentId, statusValue] of Object.entries(statusMap)) {
const update = normalizeAgentStatusUpdate(statusValue);
const update = fillCompletedAgentUpdateFromRuntime(
agentId,
normalizeAgentStatusUpdate(statusValue)
);
if (!agentCardByAgentId.has(agentId) && isAgentNotFoundStatusUpdate(update)) {
continue;
}
if (asString(update.status) === 'completed') {
const runtime = childAgentRuntimeById.get(agentId);
if (runtime) {
runtime.terminal = true;
}
}
emitAgentRunUpdate(agentId, update);
}
return;
@@ -1044,6 +1123,9 @@ class CodexRemoteLauncher extends RemoteLauncherBase {
activityKind: string,
extra?: Record<string, unknown>
): void => {
if (runtime.terminal) {
return;
}
emitAgentRunUpdate(agentId, {
status: 'running',
statusText: activity,
@@ -1068,6 +1150,9 @@ class CodexRemoteLauncher extends RemoteLauncherBase {
if (msgType === 'task_started') {
runtime.reasoningPreview = '';
runtime.finalMessage = null;
runtime.terminal = false;
agentStatusByAgentId.delete(agentId);
updateActivity('Starting task', 'starting');
return;
}
@@ -1098,12 +1183,25 @@ class CodexRemoteLauncher extends RemoteLauncherBase {
if (msgType === 'agent_message') {
const message = asString(msg.message);
if (message) {
runtime.finalMessage = message;
emitAgentRunTraceMessage(agentId, {
type: 'message',
message,
id: randomUUID()
});
}
if (runtime.terminal) {
if (message) {
emitAgentRunUpdate(agentId, {
status: 'completed',
statusText: 'Completed',
activity: formatActivity('Completed', message),
activityKind: 'completed',
result: message
});
}
return;
}
updateActivity(formatActivity('Writing', message), 'writing');
return;
}
@@ -1122,18 +1220,20 @@ class CodexRemoteLauncher extends RemoteLauncherBase {
id: randomUUID()
});
const command = normalizeCommand(inputs.command) ?? 'command';
runtime.activeToolsByCallId.set(callId, {
name: 'CodexBash',
label: command,
activity: formatActivity('Running command', command),
activityKind: 'running-command'
});
emitAgentRunUpdate(agentId, {
status: 'running',
statusText: formatActivity('Running command', command),
activity: formatActivity('Running command', command),
activityKind: 'running-command'
});
if (!runtime.terminal) {
runtime.activeToolsByCallId.set(callId, {
name: 'CodexBash',
label: command,
activity: formatActivity('Running command', command),
activityKind: 'running-command'
});
emitAgentRunUpdate(agentId, {
status: 'running',
statusText: formatActivity('Running command', command),
activity: formatActivity('Running command', command),
activityKind: 'running-command'
});
}
}
return;
}
@@ -1355,6 +1455,7 @@ class CodexRemoteLauncher extends RemoteLauncherBase {
return;
}
if (isChildTerminalEvent) {
runtime.terminal = true;
runtime.reasoningProcessor.reset();
runtime.diffProcessor.reset();
runtime.activeToolsByCallId.clear();
@@ -1377,11 +1478,13 @@ class CodexRemoteLauncher extends RemoteLauncherBase {
activityKind: 'canceled'
});
} else {
const result = runtime.finalMessage;
emitAgentRunUpdate(agentId, {
status: 'completed',
statusText: 'Completed',
activity: 'Completed',
activityKind: 'completed'
activity: formatActivity('Completed', result),
activityKind: 'completed',
...(result ? { result } : {})
});
}
}
@@ -279,7 +279,10 @@ describe('AppServerEventConverter', () => {
status: 'completed',
receiverThreadIds: ['agent-1'],
agentsStates: {
'agent-1': { status: 'completed', message: '42' }
'agent-1': { status: 'completed', message: '42' },
'agent-2': { status: 'done', message: null },
'agent-3': { status: 'done', result: { text: 'structured result' } },
'agent-4': { status: 'completed', message: '', output: { value: 42 } }
}
}
});
@@ -289,7 +292,10 @@ describe('AppServerEventConverter', () => {
name: 'wait_agent',
output: {
status: {
'agent-1': { completed: '42' }
'agent-1': { completed: '42' },
'agent-2': { status: 'completed', message: null },
'agent-3': { status: 'completed', result: { text: 'structured result' } },
'agent-4': { status: 'completed', message: '', output: { value: 42 } }
},
timed_out: false
},
+15 -4
View File
@@ -295,11 +295,22 @@ function statusObjectFromAgentState(value: unknown): unknown {
const record = asRecord(value);
if (!record) return value;
const message = asString(record.message);
const message = asString(record.message)
?? asString(record.output)
?? asString(record.result)
?? asString(record.finalMessage)
?? asString(record.final_message);
const status = asString(record.status ?? record.state);
if (status === 'completed' && message) return { completed: message };
if ((status === 'failed' || status === 'error') && message) return { failed: message };
if ((status === 'canceled' || status === 'cancelled') && message) return { canceled: message };
const normalizedStatus = status?.trim().toLowerCase().replace(/[\s_-]/g, '');
const completed = normalizedStatus === 'completed'
|| normalizedStatus === 'complete'
|| normalizedStatus === 'done'
|| record.completed === true
|| record.done === true;
if (completed && message) return { completed: message };
if (completed) return { ...record, status: 'completed' };
if ((normalizedStatus === 'failed' || normalizedStatus === 'error') && message) return { failed: message };
if ((normalizedStatus === 'canceled' || normalizedStatus === 'cancelled') && message) return { canceled: message };
return value;
}