Skip to content

Commit 7c761c7

Browse files
EugeneTheDevEugeneTheDev
authored
fix: show a Codex subagent's follow-up turn in a new subagent session (spacering-net#634)
Fixes [IJAI-3527](https://youtrack.jetbrains.com/issue/IJAI-3527). Codex reports a follow-up to a finished subagent only as a `subAgentActivity` of kind `interacted` plus a new child turn, not as a `collabAgentToolCall`. The finished child stayed closed, so its follow-up turn was dropped live and skipped on session load. - Live: reopen a finished child as the next generation (`<thread>:generation:N`) when it starts a new turn. - Replay: open the next generation on `interacted` when the child thread has that turn. - Tests for v1 and v2; verified with real Codex 0.160.1. - AGENTS.md: changes must work on both ACP v1 and v2.
1 parent 7d07123 commit 7c761c7

7 files changed

Lines changed: 405 additions & 39 deletions

‎AGENTS.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
- Keep edits consistent with existing formatting.
1515
- When adding env/config knobs, document them in `readme-dev.md`.
16+
- The adapter serves ACP v1 and v2. A change that is not meant for one protocol version only must work correctly in both.
1617
- When updating discriminated-union/event `switch` statements, do not add a trailing fallback like `return null` only to satisfy TypeScript.
1718
- Handle each variant with an explicit `case`; if intentionally ignored, use an explicit no-op case.
1819
- Code comments should be concise and in plain language; avoid verbosity and overly specific detail.
@@ -24,6 +25,7 @@
2425
- Prefer snapshot-based tests using `toMatchFileSnapshot()` over inline assertions.
2526
- When snapshot response data drifts, prefer replacing that response payload with a stable placeholder over asserting fragile fields (except for 'model/list').
2627
- Focus on behavior and outputs rather than implementation details.
28+
- Cover both ACP v1 and v2 when a change affects what a client receives (updates, responses, replay). A change to internal logic only does not need its tests duplicated per version.
2729
- Use `/run-codex` skill (`.claude/skills/run-codex/`) to test with real Codex and observe actual events.
2830

2931
## Pull Requests

‎src/CodexAcpServer.ts‎

Lines changed: 37 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -3258,13 +3258,29 @@ export class CodexAcpServer {
32583258
if (!isOpen()) throw new SessionClosedDuringLoadError();
32593259
if (item.type === "subAgentActivity") {
32603260
const activityKind = item.kind as string;
3261-
if (activityKind === "started") {
3261+
if (activityKind === "started" || activityKind === "interacted") {
32623262
const previous = announced.get(item.agentThreadId);
32633263
if (previous && !previous.terminal) continue;
3264+
// A follow-up to a finished child is an `interacted` activity.
3265+
if (activityKind === "interacted" && !previous) continue;
32643266
const generation = (previous?.generation ?? 0) + 1;
32653267
const childSessionId = generation === 1
32663268
? item.agentThreadId
32673269
: `${item.agentThreadId}:generation:${generation}`;
3270+
// Each generation of a child is one turn of the child thread. The
3271+
// adapter reads only the items of that turn, one page at a time.
3272+
let childItems: AsyncIterable<ThreadItem[]> | null = null;
3273+
if (!ancestry.has(item.agentThreadId) && !unreadableChildren.has(item.agentThreadId)) {
3274+
try {
3275+
childItems = await this.codexAcpClient.readSessionTurnItems(item.agentThreadId, generation - 1);
3276+
}
3277+
catch (error) {
3278+
unreadableChildren.add(item.agentThreadId);
3279+
logger.error(`Failed to read subagent history ${item.agentThreadId}`, error);
3280+
}
3281+
}
3282+
// An interaction that started no child turn is not a new generation.
3283+
if (activityKind === "interacted" && !childItems) continue;
32683284
const name = nameFromAgentPath(item.agentPath, `Agent ${item.agentThreadId.slice(-8)}`);
32693285
await session.update({
32703286
sessionUpdate: "subagent_spawned",
@@ -3274,44 +3290,32 @@ export class CodexAcpServer {
32743290
capabilities: {},
32753291
});
32763292
announced.set(item.agentThreadId, {generation, sessionId: childSessionId, terminal: false});
3277-
if (!ancestry.has(item.agentThreadId) && !unreadableChildren.has(item.agentThreadId)) {
3278-
// Each generation of a child is one turn of the child thread. The
3279-
// adapter reads only the items of that turn, one page at a time.
3280-
let childItems: AsyncIterable<ThreadItem[]> | null = null;
3293+
if (childItems) {
3294+
const commandIds = new Set<string>();
32813295
try {
3282-
childItems = await this.codexAcpClient.readSessionTurnItems(item.agentThreadId, generation - 1);
3296+
await this.streamNativeThreadHistory(
3297+
childSessionId,
3298+
withCommandIds(untilSessionClose(childItems, isOpen), commandIds),
3299+
sessionState,
3300+
new Set([...ancestry, item.agentThreadId]),
3301+
unreadableChildren,
3302+
isOpen,
3303+
);
32833304
}
32843305
catch (error) {
3306+
if (error instanceof SessionClosedDuringLoadError) throw error;
3307+
// The child pages are read lazily. A child that fails midway keeps what it sent.
32853308
unreadableChildren.add(item.agentThreadId);
32863309
logger.error(`Failed to read subagent history ${item.agentThreadId}`, error);
32873310
}
3288-
if (childItems) {
3289-
const commandIds = new Set<string>();
3290-
try {
3291-
await this.streamNativeThreadHistory(
3292-
childSessionId,
3293-
withCommandIds(untilSessionClose(childItems, isOpen), commandIds),
3294-
sessionState,
3295-
new Set([...ancestry, item.agentThreadId]),
3296-
unreadableChildren,
3297-
isOpen,
3298-
);
3299-
}
3300-
catch (error) {
3301-
if (error instanceof SessionClosedDuringLoadError) throw error;
3302-
// The child pages are read lazily. A child that fails midway keeps what it sent.
3303-
unreadableChildren.add(item.agentThreadId);
3304-
logger.error(`Failed to read subagent history ${item.agentThreadId}`, error);
3305-
}
3306-
try {
3307-
await sessionState.asyncTasks.recover(
3308-
item.agentThreadId,
3309-
childSessionId,
3310-
commandIds,
3311-
);
3312-
} catch (error) {
3313-
logger.error(`Failed to restore background terminals for ${item.agentThreadId}`, error);
3314-
}
3311+
try {
3312+
await sessionState.asyncTasks.recover(
3313+
item.agentThreadId,
3314+
childSessionId,
3315+
commandIds,
3316+
);
3317+
} catch (error) {
3318+
logger.error(`Failed to restore background terminals for ${item.agentThreadId}`, error);
33153319
}
33163320
}
33173321
}

‎src/__tests__/CodexACPAgent/collab-agent-events.test.ts‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1469,6 +1469,94 @@ describe("CodexEventHandler - collab agent tool call events", () => {
14691469
}));
14701470
});
14711471

1472+
it("uses a new ACP child generation when a finished child starts a follow-up turn", async () => {
1473+
await initializeNativeSubagents();
1474+
const childTurn = (id: string, status: "completed" | "inProgress"): ServerNotification => ({
1475+
method: status === "completed" ? "turn/completed" : "turn/started",
1476+
params: {
1477+
threadId: "followup-child",
1478+
turn: {
1479+
id,
1480+
items: [],
1481+
itemsView: "notLoaded",
1482+
status,
1483+
error: null,
1484+
startedAt: null,
1485+
completedAt: null,
1486+
durationMs: null,
1487+
},
1488+
},
1489+
});
1490+
const activity = (id: string, kind: "started" | "interacted"): ServerNotification => ({
1491+
method: "item/completed",
1492+
params: {
1493+
threadId: sessionId,
1494+
turnId: "root-turn",
1495+
completedAtMs: 0,
1496+
item: {
1497+
type: "subAgentActivity",
1498+
id,
1499+
kind,
1500+
agentThreadId: "followup-child",
1501+
agentPath: "/root/reviewer",
1502+
},
1503+
},
1504+
});
1505+
// Codex reports a follow-up only as an `interacted` activity, then starts a new child turn.
1506+
await setupPromptAndSendNotifications(mockFixture, sessionId, sessionState, [
1507+
activity("spawn-activity", "started"),
1508+
childTurn("first-turn", "inProgress"),
1509+
childTurn("first-turn", "completed"),
1510+
activity("followup-activity", "interacted"),
1511+
childTurn("followup-turn", "inProgress"),
1512+
{
1513+
method: "item/agentMessage/delta",
1514+
params: {
1515+
threadId: "followup-child",
1516+
turnId: "followup-turn",
1517+
itemId: "followup-message",
1518+
delta: "Follow-up answer",
1519+
},
1520+
},
1521+
childTurn("followup-turn", "completed"),
1522+
]);
1523+
const subagentUpdates = mockFixture.getAcpConnectionEvents([])
1524+
.filter(event => event.method === "sessionUpdate")
1525+
.map(event => event.args[0])
1526+
.filter(params => params.update.sessionUpdate.startsWith("subagent_")
1527+
|| params.update.messageId === "followup-message");
1528+
expect(subagentUpdates).toEqual([
1529+
expect.objectContaining({
1530+
sessionId,
1531+
update: expect.objectContaining({sessionUpdate: "subagent_spawned", subagentSessionId: "followup-child"}),
1532+
}),
1533+
expect.objectContaining({
1534+
sessionId,
1535+
update: {sessionUpdate: "subagent_state_update", subagentSessionId: "followup-child", state: "completed"},
1536+
}),
1537+
expect.objectContaining({
1538+
sessionId,
1539+
update: expect.objectContaining({
1540+
sessionUpdate: "subagent_spawned",
1541+
subagentSessionId: "followup-child:generation:2",
1542+
name: "Reviewer",
1543+
}),
1544+
}),
1545+
expect.objectContaining({
1546+
sessionId: "followup-child:generation:2",
1547+
update: expect.objectContaining({messageId: "followup-message"}),
1548+
}),
1549+
expect.objectContaining({
1550+
sessionId,
1551+
update: {
1552+
sessionUpdate: "subagent_state_update",
1553+
subagentSessionId: "followup-child:generation:2",
1554+
state: "completed",
1555+
},
1556+
}),
1557+
]);
1558+
});
1559+
14721560
it("attaches a resumed nested child to the current parent generation", async () => {
14731561
const router = new CodexSubagentEventRouter(
14741562
sessionId,

‎src/__tests__/CodexACPAgent/load-session.test.ts‎

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,150 @@ describe("CodexACPAgent - loadSession", () => {
296296
expect(updates[secondOutputIndex]?.sessionId).toBe("child-history:generation:2");
297297
});
298298

299+
it("replays a follow-up turn of a finished child as a new generation", async () => {
300+
const fixture = createCodexMockTestFixture();
301+
const agent = fixture.getCodexAcpAgent();
302+
const client = fixture.getCodexAcpClient();
303+
const appServer = fixture.getCodexAppServerClient();
304+
client.readAuthRequirement = vi.fn().mockResolvedValue({required: false, account: null});
305+
client.getAccount = vi.fn().mockResolvedValue({account: null, requiresOpenaiAuth: false});
306+
client.listSkills = vi.fn().mockResolvedValue({data: []});
307+
const model = createTestModel();
308+
appServer.listModels = vi.fn().mockResolvedValue({data: [model], nextCursor: null});
309+
const turn = (id: string, items: Thread["turns"][number]["items"]): Thread["turns"][number] => ({
310+
id,
311+
itemsView: "full",
312+
status: "completed",
313+
error: null,
314+
startedAt: null,
315+
completedAt: null,
316+
durationMs: null,
317+
items,
318+
});
319+
const makeThread = (id: string, turns: Thread["turns"]): Thread => ({
320+
id,
321+
sessionId: id,
322+
parentThreadId: id === "root-history" ? null : "root-history",
323+
threadSource: null,
324+
originator: null,
325+
forkedFromId: null,
326+
preview: id,
327+
ephemeral: false,
328+
modelProvider: "openai",
329+
model: null,
330+
reasoningEffort: null,
331+
createdAt: 1,
332+
updatedAt: 2,
333+
recencyAt: null,
334+
status: {type: "idle"},
335+
path: null,
336+
cwd: "/workspace",
337+
cliVersion: "0",
338+
section: null,
339+
sectionEnteredAt: null,
340+
projectId: null,
341+
historyMode: "legacy",
342+
source: "cli",
343+
agentNickname: null,
344+
agentRole: null,
345+
gitInfo: null,
346+
name: null,
347+
turns,
348+
});
349+
const activity = (id: string, kind: "started" | "interacted" | "completed") => ({
350+
type: "subAgentActivity" as const,
351+
id,
352+
kind,
353+
agentThreadId: "child-history",
354+
agentPath: "/root/reviewer",
355+
});
356+
const message = (id: string) => ({
357+
type: "agentMessage" as const,
358+
id,
359+
text: id,
360+
phase: null,
361+
memoryCitation: null,
362+
delivery: null,
363+
questions: null,
364+
});
365+
// Codex records a follow-up to a finished child as an `interacted` activity of the parent.
366+
const root = makeThread("root-history", [
367+
turn("root-turn-1", [activity("spawn", "started"), activity("first-done", "completed")]),
368+
turn("root-turn-2", [activity("followup", "interacted"), activity("followup-done", "completed")]),
369+
// This interaction started no child turn.
370+
turn("root-turn-3", [activity("message-only", "interacted")]),
371+
]);
372+
const child = makeThread("child-history", [
373+
turn("child-turn-1", [message("first-answer")]),
374+
turn("child-turn-2", [message("followup-answer")]),
375+
]);
376+
appServer.threadResume = vi.fn().mockResolvedValue({
377+
thread: root,
378+
model: model.id,
379+
modelProvider: "openai",
380+
cwd: "/workspace",
381+
approvalPolicy: "never",
382+
sandbox: {type: "dangerFullAccess"},
383+
reasoningEffort: model.defaultReasoningEffort,
384+
});
385+
appServer.threadReadWithHistory = vi.fn().mockResolvedValue({thread: root});
386+
appServer.threadRead = vi.fn().mockResolvedValue({thread: child});
387+
388+
await agent.initialize({
389+
protocolVersion: 1,
390+
clientCapabilities: {
391+
_meta: {jetbrains: {air: {version: 1, capabilities: ["nativeSubagentSessions"]}}},
392+
},
393+
});
394+
await agent.loadSession({sessionId: root.id, cwd: "/workspace", mcpServers: []});
395+
396+
const updates = fixture.getAcpConnectionEvents([])
397+
.filter(event => event.method === "sessionUpdate")
398+
.map(event => event.args[0])
399+
.filter(({update}) => update.sessionUpdate.startsWith("subagent_")
400+
|| update.sessionUpdate === "agent_message_chunk")
401+
.map(({sessionId, update}) => ({
402+
sessionId,
403+
update: update.sessionUpdate === "agent_message_chunk" ? update.messageId : update,
404+
}));
405+
expect(updates).toEqual([
406+
{
407+
sessionId: "root-history",
408+
update: {
409+
sessionUpdate: "subagent_spawned",
410+
subagentSessionId: "child-history",
411+
name: "Reviewer",
412+
task: "Delegated task for Reviewer",
413+
capabilities: {},
414+
},
415+
},
416+
{sessionId: "child-history", update: "first-answer"},
417+
{
418+
sessionId: "root-history",
419+
update: {sessionUpdate: "subagent_state_update", subagentSessionId: "child-history", state: "completed"},
420+
},
421+
{
422+
sessionId: "root-history",
423+
update: {
424+
sessionUpdate: "subagent_spawned",
425+
subagentSessionId: "child-history:generation:2",
426+
name: "Reviewer",
427+
task: "Delegated task for Reviewer",
428+
capabilities: {},
429+
},
430+
},
431+
{sessionId: "child-history:generation:2", update: "followup-answer"},
432+
{
433+
sessionId: "root-history",
434+
update: {
435+
sessionUpdate: "subagent_state_update",
436+
subagentSessionId: "child-history:generation:2",
437+
state: "completed",
438+
},
439+
},
440+
]);
441+
});
442+
299443
it("should replay history during loadSession", async () => {
300444
const fixture = createCodexMockTestFixture();
301445
const codexAcpAgent = fixture.getCodexAcpAgent();

0 commit comments

Comments
 (0)