Skip to content

Commit b5e6afb

Browse files
committed
Block session reopen during stale cleanup
1 parent 9c7bdc6 commit b5e6afb

2 files changed

Lines changed: 51 additions & 0 deletions

File tree

src/CodexAcpServer.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,10 +222,16 @@ export class CodexAcpServer implements acp.Agent {
222222

223223
private async closeStaleSessionOpen(sessionId: string, generation: number): Promise<void> {
224224
if (this.sessionOpenGenerations.get(sessionId) === generation) {
225+
const staleCloseGeneration = this.bumpSessionGeneration(sessionId);
226+
this.closingSessions.add(sessionId);
225227
try {
226228
await this.runWithProcessCheck(() => this.codexAcpClient.closeSession(sessionId));
227229
} catch (err) {
228230
logger.error(`Failed to close stale session open for ${sessionId}`, err);
231+
} finally {
232+
if (this.getSessionGeneration(sessionId) === staleCloseGeneration) {
233+
this.closingSessions.delete(sessionId);
234+
}
229235
}
230236
}
231237
throw RequestError.invalidRequest(`Session ${sessionId} is closing`);

src/__tests__/CodexACPAgent/session-close.test.ts

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,51 @@ describe("ACP session close", () => {
244244
expect(closeSessionSpy).toHaveBeenCalledTimes(1);
245245
expect(codexAcpAgent.getSessionState(sessionId).sessionId).toBe(sessionId);
246246
});
247+
248+
it("blocks reopen while stale resume cleanup is unsubscribing", async () => {
249+
const {codexAcpAgent, codexAcpClient} = await createSession();
250+
const staleResume = deferred<SessionMetadata>();
251+
const staleUnsubscribe = deferred<void>();
252+
const resumeSpy = vi.spyOn(codexAcpClient, "resumeSession")
253+
.mockReturnValueOnce(staleResume.promise)
254+
.mockResolvedValueOnce(createSessionMetadata());
255+
const closeSessionSpy = vi.spyOn(codexAcpClient, "closeSession")
256+
.mockResolvedValueOnce()
257+
.mockReturnValueOnce(staleUnsubscribe.promise);
258+
259+
const staleResumePromise = codexAcpAgent.resumeSession({
260+
sessionId,
261+
cwd: "/test/cwd",
262+
mcpServers: [],
263+
});
264+
265+
await codexAcpAgent.closeSession({sessionId});
266+
staleResume.resolve(createSessionMetadata());
267+
268+
await vi.waitFor(() => {
269+
expect(closeSessionSpy).toHaveBeenCalledTimes(2);
270+
});
271+
272+
await expect(codexAcpAgent.resumeSession({
273+
sessionId,
274+
cwd: "/test/cwd",
275+
mcpServers: [],
276+
})).rejects.toThrow("Invalid request");
277+
expect(resumeSpy).toHaveBeenCalledTimes(1);
278+
279+
staleUnsubscribe.resolve(undefined);
280+
await expect(staleResumePromise).rejects.toThrow("Invalid request");
281+
282+
await expect(codexAcpAgent.resumeSession({
283+
sessionId,
284+
cwd: "/test/cwd",
285+
mcpServers: [],
286+
})).resolves.toEqual(expect.objectContaining({
287+
models: expect.objectContaining({
288+
currentModelId: "model-id[medium]",
289+
}),
290+
}));
291+
});
247292
});
248293

249294
async function createSession(options: {

0 commit comments

Comments
 (0)