Skip to content

Commit 7d07123

Browse files
authored
fix: include subagent edits in the agent file-change report (spacering-net#632)
## What The `agentFileChangeReport` for a turn now includes files changed by the subagents that turn started. Fixes IJAI-3823. ## Why `CodexEventHandler` kept `turn/diff/updated` only from the root thread. Subagents report diffs under their own thread and turn ids, so a turn where only a subagent edited files produced `paths: []` and Air showed no "Since last turn" card. ## How - Child-thread diffs are recorded per child thread and turn, attributed to the root turn that was running (or the last one, if the subagent outlives it). Recording happens before subagent routing, so diffs of ended children or never-materialized spawns are kept. - `getTurnDiff` appends the attributed child diffs to the root diff; `isTurnDiffOversized` also accounts for oversized child diffs. Not covered: clients without native subagent sessions (child events never reach the handler), and subagent follow-up turns that run after the prompt ends.
1 parent aa02d43 commit 7d07123

3 files changed

Lines changed: 213 additions & 6 deletions

File tree

‎src/CodexEventHandler.ts‎

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import type {
2929
ThreadGoalUpdatedNotification,
3030
ThreadTokenUsageUpdatedNotification,
3131
Turn,
32+
TurnDiffUpdatedNotification,
3233
TurnPlanUpdatedNotification,
3334
WarningNotification
3435
} from "./app-server/v2";
@@ -232,6 +233,10 @@ export class CodexEventHandler {
232233
private readonly agentMessagePhases = new Map<string, string | null>();
233234
private readonly turnDiffs = new Map<string, string>();
234235
private readonly oversizedTurnDiffs = new Set<string>();
236+
/** Latest diff of each subagent turn, keyed by child thread and turn; `null` marks an oversized diff. */
237+
private readonly childTurnDiffs = new Map<string, {rootTurnId: string; diff: string | null}>();
238+
/** The last root turn that ended, for subagent turns that outlive it. */
239+
private latestRootTurnId: string | null = null;
235240
private readonly collectTurnDiffs: boolean;
236241
private readonly subagents: CodexSubagentEventRouter;
237242
/** Connection-level `authStatus` sink; the app-server account push feeds it. */
@@ -266,12 +271,20 @@ export class CodexEventHandler {
266271
return this.failure;
267272
}
268273

274+
/** The root turn's diff followed by the diffs of the subagent turns that ran during it. */
269275
getTurnDiff(turnId: string): string {
270-
return this.turnDiffs.get(turnId) ?? "";
276+
const diffs = [this.turnDiffs.get(turnId) ?? ""];
277+
for (const child of this.childTurnDiffs.values()) {
278+
if (child.rootTurnId === turnId && child.diff !== null) diffs.push(child.diff);
279+
}
280+
return diffs
281+
.filter(diff => diff.trim() !== "")
282+
.reduce((all, diff) => all === "" || all.endsWith("\n") ? all + diff : `${all}\n${diff}`, "");
271283
}
272284

273285
isTurnDiffOversized(turnId: string): boolean {
274-
return this.oversizedTurnDiffs.has(turnId);
286+
return this.oversizedTurnDiffs.has(turnId)
287+
|| [...this.childTurnDiffs.values()].some(child => child.rootTurnId === turnId && child.diff === null);
275288
}
276289

277290
getTerminalSessionFailureMeta(
@@ -399,6 +412,10 @@ export class CodexEventHandler {
399412
&& isModelOutputItem(notification.params.item)) {
400413
this.sessionState.promptTokenUsage?.observeModelOutput();
401414
}
415+
// Before subagent routing, which drops updates of ended children and of spawns that never materialized.
416+
if (notification.method === "turn/diff/updated" && notification.params.threadId !== this.sessionState.sessionId) {
417+
this.recordChildTurnDiff(notification.params);
418+
}
402419
await this.flushPendingErrors();
403420
await this.finishCompactionsForNotification(notification);
404421
const closingChildren = this.subagents.closingChildSessions(notification);
@@ -502,6 +519,20 @@ export class CodexEventHandler {
502519
this.disposed = true;
503520
this.turnDiffs.clear();
504521
this.oversizedTurnDiffs.clear();
522+
this.childTurnDiffs.clear();
523+
}
524+
525+
/** Subagent threads report their own turn diffs; the root turn's file-change report must include them. */
526+
private recordChildTurnDiff(params: TurnDiffUpdatedNotification): void {
527+
if (this.disposed || !this.collectTurnDiffs) return;
528+
const key = `${params.threadId}\n${params.turnId}`;
529+
// A child turn stays with the root turn it started under, even if its diff updates arrive later.
530+
const rootTurnId = this.childTurnDiffs.get(key)?.rootTurnId
531+
?? this.sessionState.currentTurnId
532+
?? this.latestRootTurnId;
533+
if (rootTurnId === null) return;
534+
const oversized = Buffer.byteLength(params.diff, "utf8") > AGENT_FILE_CHANGE_REPORT_MAX_DIFF_BYTES;
535+
this.childTurnDiffs.set(key, {rootTurnId, diff: oversized ? null : params.diff});
505536
}
506537

507538
private async createUpdateEvent(notification: ServerNotification): Promise<UpdateSessionEvent | null> {
@@ -560,6 +591,8 @@ export class CodexEventHandler {
560591
case "turn/completed":
561592
await this.plans.flush();
562593
this.plans.clearTurn();
594+
// Subagents can keep running after the root turn ends; their turns still belong to it.
595+
this.latestRootTurnId = this.sessionState.currentTurnId ?? this.latestRootTurnId;
563596
this.sessionState.currentTurnId = null;
564597
this.sessionState.interruptTurnId = null;
565598
this.sessionState.toolCallReports.releaseOpen(this.subagents.notificationSessionId(notification));

‎src/__tests__/CodexACPAgent/agent-file-change-report.test.ts‎

Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,16 @@ import {
66
type CodexMockTestFixture,
77
} from "../acp-test-utils";
88
import type {SessionState} from "../../CodexAcpServer";
9+
import {ACPSessionConnection} from "../../ACPSessionConnection";
10+
import {CodexSubagentEventRouter} from "../../subagents/CodexSubagentEventRouter";
11+
import type {ServerNotification} from "../../app-server";
912
import {CodexCommands} from "../../CodexCommands";
1013
import {logger} from "../../Logger";
1114
import type {Turn, TurnCompletedNotification} from "../../app-server/v2";
1215
import {AGENT_FILE_CHANGE_REPORT_MAX_DIFF_BYTES} from "../../AgentFileChangeReport";
1316

17+
const CHILD_THREAD_ID = "child-thread";
18+
1419
function createTurn(id: string, status: Turn["status"]): Turn {
1520
return {
1621
id,
@@ -61,6 +66,60 @@ const FILE_CHANGE_REPORT_CLIENT_CAPABILITIES: acp.ClientCapabilities = {
6166
},
6267
};
6368

69+
function modifiedFileDiff(pathname: string): string {
70+
return `diff --git a/${pathname} b/${pathname}\n--- a/${pathname}\n+++ b/${pathname}\n@@ -1 +1 @@\n-old\n+new\n`;
71+
}
72+
73+
function turnDiffUpdated(threadId: string, turnId: string, diff: string): ServerNotification {
74+
return {method: "turn/diff/updated", params: {threadId, turnId, diff}};
75+
}
76+
77+
/** Notifications of a root turn that spawns a native subagent, as Codex reports them. */
78+
function spawnChild(rootSessionId: string): ServerNotification[] {
79+
return [
80+
{
81+
method: "item/started",
82+
params: {
83+
threadId: rootSessionId,
84+
turnId: "main-turn",
85+
startedAtMs: 0,
86+
item: {
87+
type: "collabAgentToolCall",
88+
id: "spawn-child",
89+
tool: "spawnAgent",
90+
status: "inProgress",
91+
senderThreadId: rootSessionId,
92+
receiverThreadIds: [CHILD_THREAD_ID],
93+
prompt: "Edit the docs.",
94+
model: null,
95+
reasoningEffort: null,
96+
agentsStates: {[CHILD_THREAD_ID]: {status: "running", message: null}},
97+
},
98+
},
99+
},
100+
{
101+
method: "item/started",
102+
params: {
103+
threadId: rootSessionId,
104+
turnId: "main-turn",
105+
startedAtMs: 0,
106+
item: {
107+
type: "subAgentActivity",
108+
id: "activity-child",
109+
kind: "started",
110+
agentThreadId: CHILD_THREAD_ID,
111+
agentPath: "/root/docs_writer",
112+
},
113+
},
114+
},
115+
{method: "turn/started", params: {threadId: CHILD_THREAD_ID, turn: createTurn("child-turn", "inProgress")}},
116+
];
117+
}
118+
119+
function childTurnCompleted(): ServerNotification {
120+
return {method: "turn/completed", params: {threadId: CHILD_THREAD_ID, turn: createTurn("child-turn", "completed")}};
121+
}
122+
64123
async function setupMainPrompt(negotiateCapability = true): Promise<{
65124
fixture: CodexMockTestFixture;
66125
sessionState: SessionState;
@@ -87,6 +146,32 @@ async function setupMainPrompt(negotiateCapability = true): Promise<{
87146
return {fixture, sessionState, turnStart, awaitTurnCompleted};
88147
}
89148

149+
async function setupNativeSubagentPrompt(): Promise<{
150+
fixture: CodexMockTestFixture;
151+
sessionState: SessionState;
152+
awaitTurnCompleted: ReturnType<typeof vi.spyOn>;
153+
}> {
154+
const fixture = createCodexMockTestFixture();
155+
await fixture.getCodexAcpAgent().initialize({
156+
protocolVersion: acp.PROTOCOL_VERSION,
157+
clientCapabilities: {
158+
_meta: {jetbrains: {air: {version: 1, capabilities: ["agentFileChangeReport", "nativeSubagentSessions"]}}},
159+
},
160+
});
161+
const sessionState = createTestSessionState({cwd: "/workspace"});
162+
sessionState.subagents = new CodexSubagentEventRouter(
163+
sessionState.sessionId,
164+
true,
165+
new ACPSessionConnection(fixture.getAcpConnection(), sessionState.sessionId),
166+
() => {},
167+
);
168+
vi.spyOn(fixture.getCodexAcpAgent(), "getSessionState").mockReturnValue(sessionState);
169+
vi.spyOn(fixture.getCodexAppServerClient(), "turnStart")
170+
.mockResolvedValue({turn: createTurn("main-turn", "inProgress")});
171+
const awaitTurnCompleted = vi.spyOn(fixture.getCodexAppServerClient(), "awaitTurnCompleted");
172+
return {fixture, sessionState, awaitTurnCompleted};
173+
}
174+
90175
describe("agent file-change report lifecycle", () => {
91176
beforeEach(() => {
92177
vi.clearAllMocks();
@@ -321,4 +406,85 @@ describe("agent file-change report lifecycle", () => {
321406
}}}},
322407
});
323408
});
409+
410+
describe("with native subagents", () => {
411+
it("includes the files a subagent changed during the root turn", async () => {
412+
const {fixture, sessionState, awaitTurnCompleted} = await setupNativeSubagentPrompt();
413+
awaitTurnCompleted.mockImplementationOnce(async (): Promise<TurnCompletedNotification> => {
414+
for (const notification of [
415+
turnDiffUpdated(sessionState.sessionId, "main-turn", modifiedFileDiff("README.md")),
416+
...spawnChild(sessionState.sessionId),
417+
turnDiffUpdated(CHILD_THREAD_ID, "child-turn", modifiedFileDiff("README.md")
418+
+ modifiedFileDiff("community/README.md")),
419+
childTurnCompleted(),
420+
]) {
421+
fixture.sendServerNotification(notification);
422+
}
423+
return {threadId: sessionState.sessionId, turn: createTurn("main-turn", "completed")};
424+
});
425+
426+
await expect(fixture.getCodexAcpAgent().prompt(
427+
promptWithFileChangeReport(sessionState.sessionId, "request-subagent"),
428+
)).resolves.toMatchObject({stopReason: "end_turn"});
429+
430+
expect(reportedUpdates(fixture)[0]).toMatchObject({
431+
_meta: {jetbrains: {air: {agentFileChangeReport: {
432+
status: "reported",
433+
paths: ["/workspace/README.md", "/workspace/community/README.md"],
434+
}}}},
435+
});
436+
});
437+
438+
it("includes the files of a subagent that finishes after the root turn ends", async () => {
439+
const {fixture, sessionState, awaitTurnCompleted} = await setupNativeSubagentPrompt();
440+
awaitTurnCompleted.mockImplementationOnce(async (): Promise<TurnCompletedNotification> => {
441+
const completed = {threadId: sessionState.sessionId, turn: createTurn("main-turn", "completed")};
442+
for (const notification of [
443+
...spawnChild(sessionState.sessionId),
444+
{method: "turn/completed", params: completed} satisfies ServerNotification,
445+
turnDiffUpdated(CHILD_THREAD_ID, "child-turn", modifiedFileDiff("docs/guide.md")),
446+
childTurnCompleted(),
447+
]) {
448+
fixture.sendServerNotification(notification);
449+
}
450+
return completed;
451+
});
452+
453+
await fixture.getCodexAcpAgent().prompt(
454+
promptWithFileChangeReport(sessionState.sessionId, "request-late-subagent"),
455+
);
456+
457+
expect(reportedUpdates(fixture)[0]).toMatchObject({
458+
_meta: {jetbrains: {air: {agentFileChangeReport: {
459+
status: "reported",
460+
paths: ["/workspace/docs/guide.md"],
461+
}}}},
462+
});
463+
});
464+
465+
it("publishes invalidOutput when a subagent turn diff is oversized", async () => {
466+
const {fixture, sessionState, awaitTurnCompleted} = await setupNativeSubagentPrompt();
467+
awaitTurnCompleted.mockImplementationOnce(async (): Promise<TurnCompletedNotification> => {
468+
for (const notification of [
469+
...spawnChild(sessionState.sessionId),
470+
turnDiffUpdated(CHILD_THREAD_ID, "child-turn", "x".repeat(AGENT_FILE_CHANGE_REPORT_MAX_DIFF_BYTES + 1)),
471+
childTurnCompleted(),
472+
]) {
473+
fixture.sendServerNotification(notification);
474+
}
475+
return {threadId: sessionState.sessionId, turn: createTurn("main-turn", "completed")};
476+
});
477+
478+
await fixture.getCodexAcpAgent().prompt(
479+
promptWithFileChangeReport(sessionState.sessionId, "request-oversized-subagent"),
480+
);
481+
482+
expect(reportedUpdates(fixture)[0]).toMatchObject({
483+
_meta: {jetbrains: {air: {agentFileChangeReport: {
484+
status: "unavailable",
485+
reason: "invalidOutput",
486+
}}}},
487+
});
488+
});
489+
});
324490
});

‎src/__tests__/CodexACPAgent/turn-diff-events.test.ts‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,13 +34,22 @@ describe("CodexEventHandler - turn diff events", () => {
3434
expect(disabled.getTurnDiff("turn-1")).toBe("");
3535

3636
const enabled = createHandler(true).handler;
37-
await enabled.handleNotification(turnDiff("child-thread", "child diff"));
38-
expect(enabled.getTurnDiff("turn-1")).toBe("");
39-
4037
await enabled.handleNotification(turnDiff(sessionId, "root diff"));
4138
expect(enabled.getTurnDiff("turn-1")).toBe("root diff");
4239
});
4340

41+
it("appends subagent turn diffs to the root turn that was running", async () => {
42+
const {handler} = createHandler(true);
43+
await handler.handleNotification(turnDiff(sessionId, "root diff"));
44+
await handler.handleNotification({
45+
method: "turn/diff/updated",
46+
params: {threadId: "child-thread", turnId: "child-turn", diff: "child diff\n"},
47+
});
48+
49+
expect(handler.getTurnDiff("turn-1")).toBe("root diff\nchild diff\n");
50+
expect(handler.getTurnDiff("child-turn")).toBe("");
51+
});
52+
4453
it("clears a prior snapshot when Codex emits the final empty aggregate", async () => {
4554
const {handler} = createHandler(true);
4655
await handler.handleNotification(turnDiff(sessionId, "root diff"));
@@ -119,6 +128,5 @@ describe("CodexEventHandler - turn diff events", () => {
119128
await handler.handleNotification(turnDiff("child-thread", "child diff"));
120129

121130
expect(sessionState.sessionFailure).toMatchObject({severity: "warning"});
122-
expect(handler.getTurnDiff("turn-1")).toBe("");
123131
});
124132
});

0 commit comments

Comments
 (0)