Skip to content

Commit f6aa545

Browse files
committed
Suppress stale turn notifications after routing them
1 parent 5f8491a commit f6aa545

2 files changed

Lines changed: 66 additions & 8 deletions

File tree

src/CodexAppServerClient.ts

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -144,17 +144,13 @@ export class CodexAppServerClient {
144144
this.recordCompactionCompleted(serverNotification);
145145
}
146146
const routing = extractTurnRouting(serverNotification);
147-
const staleTurnNotification = this.isStaleTurn(routing.threadId, routing.turnId);
148-
if (staleTurnNotification) {
149-
if (isTurnCompletedNotification(serverNotification) && routing.threadId !== null && routing.turnId !== null) {
150-
this.clearStaleTurn(routing.threadId, routing.turnId);
151-
}
152-
for (const callback of this.codexEventHandlers) {
153-
callback({ eventType: "notification", ...serverNotification });
154-
}
147+
if (this.handleStaleTurnNotification(serverNotification, routing)) {
155148
return;
156149
}
157150
this.recordTurnRouting(routing);
151+
if (this.handleStaleTurnNotification(serverNotification, routing)) {
152+
return;
153+
}
158154
this.notify(serverNotification);
159155
for (const callback of this.codexEventHandlers) {
160156
callback({ eventType: "notification", ...serverNotification });
@@ -554,6 +550,22 @@ export class CodexAppServerClient {
554550
}
555551
}
556552

553+
private handleStaleTurnNotification(
554+
notification: ServerNotification,
555+
routing: { threadId: string | null, turnId: string | null },
556+
): boolean {
557+
if (!this.isStaleTurn(routing.threadId, routing.turnId)) {
558+
return false;
559+
}
560+
if (isTurnCompletedNotification(notification) && routing.threadId !== null && routing.turnId !== null) {
561+
this.clearStaleTurn(routing.threadId, routing.turnId);
562+
}
563+
for (const callback of this.codexEventHandlers) {
564+
callback({ eventType: "notification", ...notification });
565+
}
566+
return true;
567+
}
568+
557569
private isStaleTurn(threadId: string | null, turnId: string | null): boolean {
558570
if (threadId === null || turnId === null) {
559571
return false;

src/__tests__/CodexACPAgent/CodexAcpClient.test.ts

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1536,6 +1536,52 @@ describe('ACP server test', { timeout: 40_000 }, () => {
15361536
});
15371537
});
15381538

1539+
it('suppresses the first routed goal notification after cancellation marks the turn stale', async () => {
1540+
const { mockFixture } = setupPromptFixture();
1541+
const codexAppServerClient = mockFixture.getCodexAppServerClient();
1542+
const threadGoalSetSpy = vi.spyOn(codexAppServerClient, "threadGoalSet")
1543+
.mockResolvedValue({ goal: createThreadGoal() });
1544+
const turnInterruptSpy = vi.spyOn(mockFixture.getCodexAcpClient(), "turnInterrupt")
1545+
.mockResolvedValue(undefined);
1546+
const controller = new AbortController();
1547+
1548+
const promptPromise = mockFixture.getCodexAcpAgent().prompt({
1549+
sessionId: "session-id",
1550+
prompt: [{ type: "text", text: "/goal Ship the migration and keep tests green" }],
1551+
}, controller.signal);
1552+
1553+
await vi.waitFor(() => {
1554+
expect(threadGoalSetSpy).toHaveBeenCalledWith({
1555+
threadId: "session-id",
1556+
objective: "Ship the migration and keep tests green",
1557+
status: "active",
1558+
});
1559+
});
1560+
1561+
controller.abort();
1562+
await expect(promptPromise).resolves.toMatchObject({stopReason: "cancelled"});
1563+
mockFixture.clearAcpConnectionDump();
1564+
1565+
mockFixture.sendServerNotification({
1566+
method: "item/agentMessage/delta",
1567+
params: {
1568+
threadId: "session-id",
1569+
turnId: "goal-turn-id",
1570+
itemId: "goal-message-id",
1571+
delta: "leaked goal output",
1572+
},
1573+
});
1574+
1575+
await vi.waitFor(() => {
1576+
expect(turnInterruptSpy).toHaveBeenCalledWith({
1577+
threadId: "session-id",
1578+
turnId: "goal-turn-id",
1579+
});
1580+
});
1581+
await flushAsyncWork();
1582+
expect(mockFixture.getAcpConnectionDump([])).not.toContain("leaked goal output");
1583+
});
1584+
15391585
it('does not hang when goal set starts no continuation turn', async () => {
15401586
const mockFixture = createCodexMockTestFixture();
15411587
const codexAppServerClient = mockFixture.getCodexAppServerClient();

0 commit comments

Comments
 (0)