Skip to content

Commit 67df34b

Browse files
zhangguiping-xydtzhangguiping-xydt
committed
fix(codex): wait for native subagent completion
Codex native subagent lifecycle status is only a progress signal; the task row should not report success until the transcript or native completion result is available.
1 parent 7bd533a commit 67df34b

4 files changed

Lines changed: 131 additions & 36 deletions

File tree

‎extensions/codex/src/app-server/native-subagent-monitor.test.ts‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,10 +203,97 @@ describe("CodexNativeSubagentMonitor", () => {
203203
task: "inspect the repo",
204204
}),
205205
);
206+
expect(runtime.recordTaskRunProgressByRunId).toHaveBeenCalledWith(
207+
expect.objectContaining({
208+
runId: "codex-thread:child-thread",
209+
progressSummary: "Codex native subagent is idle.",
210+
}),
211+
);
212+
expect(runtime.finalizeTaskRunByRunId).not.toHaveBeenCalled();
213+
});
214+
215+
it("does not complete mirrored task rows from idle status before native completion", async () => {
216+
const client = createClient();
217+
const runtime = createRuntime();
218+
const monitor = new CodexNativeSubagentMonitor(client, runtime);
219+
monitor.registerParent({
220+
parentThreadId: "parent-thread",
221+
requesterSessionKey: "agent:main:discord:channel:C123",
222+
taskRuntimeScope: createTaskScope(),
223+
agentId: "main",
224+
});
225+
226+
await notifyChildStarted(client);
227+
await client.notify({
228+
method: "thread/status/changed",
229+
params: {
230+
threadId: "child-thread",
231+
status: { type: "idle" },
232+
},
233+
});
234+
235+
expect(runtime.finalizeTaskRunByRunId).not.toHaveBeenCalled();
236+
expect(runtime.deliverAgentHarnessTaskCompletion).not.toHaveBeenCalled();
237+
238+
await client.notify(
239+
nativeCompletionNotification({
240+
agentPath: "child-thread",
241+
statusLabel: "completed",
242+
result: "child final result",
243+
}),
244+
);
245+
246+
expect(runtime.finalizeTaskRunByRunId).toHaveBeenCalledWith(
247+
expect.objectContaining({
248+
runId: "codex-thread:child-thread",
249+
status: "succeeded",
250+
terminalSummary: "child final result",
251+
}),
252+
);
253+
expect(runtime.deliverAgentHarnessTaskCompletion).toHaveBeenCalledWith(
254+
expect.objectContaining({
255+
childSessionId: "child-thread",
256+
result: "child final result",
257+
}),
258+
);
259+
});
260+
261+
it("keeps late idle lifecycle updates from overwriting native completion results", async () => {
262+
const client = createClient();
263+
const runtime = createRuntime();
264+
const monitor = new CodexNativeSubagentMonitor(client, runtime);
265+
monitor.registerParent({
266+
parentThreadId: "parent-thread",
267+
requesterSessionKey: "agent:main:discord:channel:C123",
268+
taskRuntimeScope: createTaskScope(),
269+
agentId: "main",
270+
});
271+
272+
await notifyChildStarted(client);
273+
await client.notify(
274+
nativeCompletionNotification({
275+
agentPath: "child-thread",
276+
statusLabel: "completed",
277+
result: "child final result",
278+
}),
279+
);
280+
runtime.recordTaskRunProgressByRunId.mockClear();
281+
282+
await client.notify({
283+
method: "thread/status/changed",
284+
params: {
285+
threadId: "child-thread",
286+
status: { type: "idle" },
287+
},
288+
});
289+
290+
expect(runtime.recordTaskRunProgressByRunId).not.toHaveBeenCalled();
291+
expect(runtime.finalizeTaskRunByRunId).toHaveBeenCalledTimes(1);
206292
expect(runtime.finalizeTaskRunByRunId).toHaveBeenCalledWith(
207293
expect.objectContaining({
208294
runId: "codex-thread:child-thread",
209295
status: "succeeded",
296+
terminalSummary: "child final result",
210297
}),
211298
);
212299
});

‎extensions/codex/src/app-server/native-subagent-monitor.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -493,6 +493,9 @@ export class CodexNativeSubagentMonitor {
493493
if (!taskRuntime) {
494494
return;
495495
}
496+
this.getMirrorForChild(completion.childThreadId)?.markAuthoritativeCompletion(
497+
completion.childThreadId,
498+
);
496499
taskRuntime.finalizeTaskRunByRunId({
497500
runId: codexNativeSubagentRunId(completion.childThreadId),
498501
status: completion.status,
@@ -510,6 +513,12 @@ export class CodexNativeSubagentMonitor {
510513
return state?.taskRuntime;
511514
}
512515

516+
private getMirrorForChild(childThreadId: string): CodexNativeSubagentTaskMirror | undefined {
517+
const childState = this.childStates.get(childThreadId.trim());
518+
const state = childState ? this.parentStates.get(childState.parentThreadId) : undefined;
519+
return state?.mirror;
520+
}
521+
513522
private registerChildThread(
514523
parentThreadId: string,
515524
childThreadId: string,

‎extensions/codex/src/app-server/native-subagent-task-mirror.test.ts‎

Lines changed: 18 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -163,15 +163,13 @@ describe("CodexNativeSubagentTaskMirror", () => {
163163
},
164164
});
165165

166-
expect(runtime.finalizeTaskRunByRunId).toHaveBeenNthCalledWith(1, {
166+
expect(runtime.recordTaskRunProgressByRunId).toHaveBeenCalledWith({
167167
runId: codexNativeSubagentRunId("child-thread"),
168-
status: "succeeded",
169-
endedAt: 30_000,
170168
lastEventAt: 30_000,
171169
progressSummary: "Codex native subagent is idle.",
172-
terminalSummary: "Codex native subagent finished.",
173170
});
174-
expect(runtime.finalizeTaskRunByRunId).toHaveBeenNthCalledWith(2, {
171+
expect(runtime.finalizeTaskRunByRunId).toHaveBeenCalledTimes(1);
172+
expect(runtime.finalizeTaskRunByRunId).toHaveBeenCalledWith({
175173
runId: codexNativeSubagentRunId("failed-child"),
176174
status: "failed",
177175
endedAt: 30_000,
@@ -249,14 +247,12 @@ describe("CodexNativeSubagentTaskMirror", () => {
249247
lastEventAt: 40_000,
250248
progressSummary: "Codex native subagent is initializing.",
251249
});
252-
expect(runtime.finalizeTaskRunByRunId).toHaveBeenCalledWith({
250+
expect(runtime.recordTaskRunProgressByRunId).toHaveBeenCalledWith({
253251
runId: "codex-thread:child-thread",
254-
status: "succeeded",
255-
endedAt: 40_000,
256252
lastEventAt: 40_000,
257253
progressSummary: "done",
258-
terminalSummary: "done",
259254
});
255+
expect(runtime.finalizeTaskRunByRunId).not.toHaveBeenCalled();
260256
});
261257

262258
it("uses the notification thread id when collab agent items omit sender thread id", () => {
@@ -326,13 +322,13 @@ describe("CodexNativeSubagentTaskMirror", () => {
326322
task: "inspect one thing",
327323
}),
328324
);
329-
expect(runtime.finalizeTaskRunByRunId).toHaveBeenCalledWith(
325+
expect(runtime.recordTaskRunProgressByRunId).toHaveBeenCalledWith(
330326
expect.objectContaining({
331327
runId: "codex-thread:child-thread",
332-
status: "succeeded",
333-
terminalSummary: "done",
328+
progressSummary: "done",
334329
}),
335330
);
331+
expect(runtime.finalizeTaskRunByRunId).not.toHaveBeenCalled();
336332
});
337333

338334
it("finalizes stale collab agent state from the blocked tool call status", () => {
@@ -459,7 +455,7 @@ describe("CodexNativeSubagentTaskMirror", () => {
459455
expect(runtime.finalizeTaskRunByRunId).not.toHaveBeenCalled();
460456
});
461457

462-
it("preserves a completed collab agent message when the thread later goes idle", () => {
458+
it("records completed collab agent and idle thread states as progress only", () => {
463459
const runtime = createRuntime();
464460
const mirror = new CodexNativeSubagentTaskMirror(
465461
{
@@ -496,18 +492,16 @@ describe("CodexNativeSubagentTaskMirror", () => {
496492
},
497493
});
498494

499-
expect(runtime.finalizeTaskRunByRunId).toHaveBeenCalledTimes(1);
500-
expect(runtime.finalizeTaskRunByRunId).toHaveBeenCalledWith({
495+
expect(runtime.recordTaskRunProgressByRunId).toHaveBeenCalledTimes(1);
496+
expect(runtime.recordTaskRunProgressByRunId).toHaveBeenCalledWith({
501497
runId: "codex-thread:child-thread",
502-
status: "succeeded",
503-
endedAt: 50_000,
504498
lastEventAt: 50_000,
505499
progressSummary: "No user task is specified.",
506-
terminalSummary: "No user task is specified.",
507500
});
501+
expect(runtime.finalizeTaskRunByRunId).not.toHaveBeenCalled();
508502
});
509503

510-
it("lets terminal collab agent state correct an earlier idle thread status", () => {
504+
it("lets terminal collab agent state finalize after an earlier idle thread status", () => {
511505
const runtime = createRuntime();
512506
const mirror = new CodexNativeSubagentTaskMirror(
513507
{
@@ -545,15 +539,13 @@ describe("CodexNativeSubagentTaskMirror", () => {
545539
},
546540
});
547541

548-
expect(runtime.finalizeTaskRunByRunId).toHaveBeenNthCalledWith(1, {
542+
expect(runtime.recordTaskRunProgressByRunId).toHaveBeenCalledWith({
549543
runId: "codex-thread:child-thread",
550-
status: "succeeded",
551-
endedAt: 55_000,
552544
lastEventAt: 55_000,
553545
progressSummary: "Codex native subagent is idle.",
554-
terminalSummary: "Codex native subagent finished.",
555546
});
556-
expect(runtime.finalizeTaskRunByRunId).toHaveBeenNthCalledWith(2, {
547+
expect(runtime.finalizeTaskRunByRunId).toHaveBeenCalledTimes(1);
548+
expect(runtime.finalizeTaskRunByRunId).toHaveBeenCalledWith({
557549
runId: "codex-thread:child-thread",
558550
status: "failed",
559551
endedAt: 55_000,
@@ -614,13 +606,11 @@ describe("CodexNativeSubagentTaskMirror", () => {
614606
lastEventAt: 60_000,
615607
progressSummary: "Codex native subagent is initializing.",
616608
});
617-
expect(runtime.finalizeTaskRunByRunId).toHaveBeenCalledWith({
609+
expect(runtime.recordTaskRunProgressByRunId).toHaveBeenCalledWith({
618610
runId: "codex-thread:child-thread",
619-
status: "succeeded",
620-
endedAt: 60_000,
621611
lastEventAt: 60_000,
622612
progressSummary: "done",
623-
terminalSummary: "done",
624613
});
614+
expect(runtime.finalizeTaskRunByRunId).not.toHaveBeenCalled();
625615
});
626616
});

‎extensions/codex/src/app-server/native-subagent-task-mirror.ts‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ export class CodexNativeSubagentTaskMirror {
3636
private readonly mirroredThreadIds = new Set<string>();
3737
private readonly failedMirrorThreadIds = new Set<string>();
3838
private readonly terminalRunIds = new Set<string>();
39+
private readonly authoritativeRunIds = new Set<string>();
3940
private readonly now: () => number;
4041

4142
constructor(
@@ -45,6 +46,12 @@ export class CodexNativeSubagentTaskMirror {
4546
this.now = params.now ?? Date.now;
4647
}
4748

49+
markAuthoritativeCompletion(childThreadId: string): void {
50+
const runId = codexNativeSubagentRunId(childThreadId);
51+
this.authoritativeRunIds.add(runId);
52+
this.terminalRunIds.add(runId);
53+
}
54+
4855
handleNotification(notification: CodexServerNotification): void {
4956
const params = isJsonObject(notification.params) ? notification.params : undefined;
5057
if (!params) {
@@ -109,6 +116,7 @@ export class CodexNativeSubagentTaskMirror {
109116
}
110117
this.failedMirrorThreadIds.delete(threadId);
111118
this.terminalRunIds.delete(runId);
119+
this.authoritativeRunIds.delete(runId);
112120
this.applyStatus(threadId, thread.status);
113121
}
114122

@@ -129,6 +137,9 @@ export class CodexNativeSubagentTaskMirror {
129137
return;
130138
}
131139
const runId = codexNativeSubagentRunId(threadId);
140+
if (this.authoritativeRunIds.has(runId)) {
141+
return;
142+
}
132143
if (this.terminalRunIds.has(runId) && statusType !== "systemError") {
133144
return;
134145
}
@@ -143,13 +154,10 @@ export class CodexNativeSubagentTaskMirror {
143154
}
144155
if (statusType === "idle") {
145156
this.terminalRunIds.add(runId);
146-
this.runtime.finalizeTaskRunByRunId({
157+
this.runtime.recordTaskRunProgressByRunId({
147158
runId,
148-
status: "succeeded",
149-
endedAt: eventAt,
150159
lastEventAt: eventAt,
151160
progressSummary: "Codex native subagent is idle.",
152-
terminalSummary: "Codex native subagent finished.",
153161
});
154162
return;
155163
}
@@ -257,6 +265,7 @@ export class CodexNativeSubagentTaskMirror {
257265
}
258266
this.failedMirrorThreadIds.delete(normalizedThreadId);
259267
this.terminalRunIds.delete(runId);
268+
this.authoritativeRunIds.delete(runId);
260269
}
261270

262271
private applyCollabAgentStatus(
@@ -272,6 +281,9 @@ export class CodexNativeSubagentTaskMirror {
272281
return;
273282
}
274283
const runId = codexNativeSubagentRunId(threadId);
284+
if (this.authoritativeRunIds.has(runId)) {
285+
return;
286+
}
275287
if (this.terminalRunIds.has(runId) && isNonTerminalAgentStateStatus(normalizedStatus)) {
276288
return;
277289
}
@@ -290,13 +302,10 @@ export class CodexNativeSubagentTaskMirror {
290302
}
291303
if (normalizedStatus === "completed") {
292304
this.terminalRunIds.add(runId);
293-
this.runtime.finalizeTaskRunByRunId({
305+
this.runtime.recordTaskRunProgressByRunId({
294306
runId,
295-
status: "succeeded",
296-
endedAt: eventAt,
297307
lastEventAt: eventAt,
298308
progressSummary: trimOptional(message) ?? "Codex native subagent completed.",
299-
terminalSummary: trimOptional(message) ?? "Codex native subagent finished.",
300309
});
301310
return;
302311
}

0 commit comments

Comments
 (0)