Skip to content

Commit 13cfcfe

Browse files
OneStepAt4timeArgus
authored andcommitted
fix(sessions): revert async prompt delivery to synchronous (#3271)
The async prompt delivery from #3243 caused a P0 regression: promptDelivery tracking hung indefinitely (status stuck at 'pending', attempts: 0) even though Claude Code received and processed the prompt. Root cause: the void deliverAsync() fire-and-forget pattern allowed the JSON-RPC session/prompt response to be lost — the tracking layer never received confirmation. The session/prompt request was sent, CC processed it, but the response never resolved the pending Promise (timeout after 120s → status: 'failed'). Fix: revert to synchronous await acpBackend.sendPrompt() which blocks until CC acknowledges receipt. This restores the proven pre-#3243 behavior. Changes: - src/routes/sessions.ts: revert async delivery to synchronous (keeps promptDelivery.status field for backward compat, adds error) - src/__tests__/async-session-create-3243.test.ts: rewrite tests for synchronous behavior (8 tests, all passing) Impact: promptDelivery.status will be 'delivered' or 'failed' immediately in the POST response, no longer 'pending'. The polling pattern is no longer needed. Closes #3271
1 parent 017875a commit 13cfcfe

2 files changed

Lines changed: 72 additions & 134 deletions

File tree

src/__tests__/async-session-create-3243.test.ts

Lines changed: 62 additions & 119 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
11
/**
2-
* async-session-create-3243.test.ts — Tests for Issue #3243.
2+
* async-session-create-3243.test.ts — Tests for synchronous prompt delivery (Issue #3271 revert).
33
*
4-
* POST /v1/sessions with ACP enabled + prompt must return 201 immediately
5-
* with promptDelivery: { delivered: false, attempts: 0, status: 'pending' }.
6-
* Prompt delivery runs asynchronously and updates session.promptDelivery
7-
* once complete.
4+
* Originally tested async delivery from #3243. After #3271 regression fix,
5+
* prompt delivery is synchronous again. Tests verify the synchronous behavior
6+
* with promptDelivery.status tracking intact.
87
*/
98

109
import Fastify from 'fastify';
@@ -42,12 +41,7 @@ import {
4241

4342
import { type Config } from '../config.js';
4443

45-
const MASTER_TOKEN = 'aegis-master-3243';
46-
47-
/** Wait for async microtasks/timers to settle. */
48-
async function flushAsync(ms = 50): Promise<void> {
49-
await new Promise((r) => setTimeout(r, ms));
50-
}
44+
const MASTER_TOKEN = 'aegis-master-3271';
5145

5246
function buildMockAcpBackend(sendPromptImpl?: () => Promise<{ delivered: boolean; attempts: number }>) {
5347
const sessionId = crypto.randomUUID();
@@ -192,121 +186,94 @@ const authHeaders = (token = MASTER_TOKEN) => ({
192186
// Tests
193187
// ─────────────────────────────────────────────────────────────────────────────
194188

195-
describe('Issue #3243 — async session create (ACP + prompt)', () => {
196-
let tmpDir: string;
197-
198-
beforeAll(() => {
199-
tmpDir = mkdtempSync(join(tmpdir(), 'aegis-async-create-3243-'));
200-
});
201-
202-
describe('POST /v1/sessions — fire-and-forget sendPrompt', () => {
203-
it('returns 201 immediately with promptDelivery.status: pending', async () => {
204-
// sendPrompt resolves instantly — check that response has status: 'pending' regardless
205-
const acpBackend = buildMockAcpBackend(() => Promise.resolve({ delivered: true, attempts: 1 }));
206-
const { ctx, sessions } = await buildRouteContext(tmpDir, acpBackend);
189+
describe('Issue #3271 — synchronous prompt delivery (revert of async #3243)', () => {
190+
describe('POST /v1/sessions — synchronous prompt delivery', () => {
191+
it('returns 201 with promptDelivery.status: delivered after sendPrompt resolves', async () => {
192+
const tmpDir = mkdtempSync(join(tmpdir(), 'aegis-test-3271-'));
193+
const acp = buildMockAcpBackend();
194+
const { ctx } = await buildRouteContext(tmpDir, acp);
207195
const { app, port } = await buildApp(ctx);
208196

209197
try {
210198
const res = await fetch(`http://127.0.0.1:${port}/v1/sessions`, {
211199
method: 'POST',
212200
headers: authHeaders(),
213-
body: JSON.stringify({ workDir: tmpDir, prompt: 'Build a REST API' }),
201+
body: JSON.stringify({ prompt: 'hello', workDir: tmpDir }),
214202
});
215203

216204
expect(res.status).toBe(201);
217-
const body = await res.json() as Record<string, unknown>;
218-
expect(body.id).toBeDefined();
219-
220-
const pd = body.promptDelivery as { delivered: boolean; attempts: number; status: string } | undefined;
221-
expect(pd).toBeDefined();
222-
expect(pd?.status).toBe('pending');
223-
expect(pd?.delivered).toBe(false);
224-
expect(pd?.attempts).toBe(0);
205+
const body = await res.json();
206+
// Synchronous delivery — status should be 'delivered', not 'pending'
207+
expect(body.promptDelivery).toBeDefined();
208+
expect(body.promptDelivery.status).toBe('delivered');
209+
expect(body.promptDelivery.delivered).toBe(true);
210+
expect(body.promptDelivery.attempts).toBe(1);
211+
expect(acp.sendPrompt).toHaveBeenCalledTimes(1);
225212
} finally {
226213
await app.close();
227214
}
228215
});
229216

230-
it('updates session.promptDelivery.status to delivered after sendPrompt resolves', async () => {
231-
let resolvePrompt!: (val: { delivered: boolean; attempts: number }) => void;
232-
const deferredPrompt = new Promise<{ delivered: boolean; attempts: number }>((r) => {
233-
resolvePrompt = r;
234-
});
235-
236-
const acpBackend = buildMockAcpBackend(() => deferredPrompt);
237-
const subTmpDir = mkdtempSync(join(tmpdir(), 'aegis-async-3243b-'));
238-
const { ctx, sessions } = await buildRouteContext(subTmpDir, acpBackend);
217+
it('returns promptDelivery.status: failed when sendPrompt returns not delivered', async () => {
218+
const tmpDir = mkdtempSync(join(tmpdir(), 'aegis-test-3271-'));
219+
const acp = buildMockAcpBackend(() => Promise.resolve({ delivered: false, attempts: 1, error: 'test failure' }));
220+
const { ctx } = await buildRouteContext(tmpDir, acp);
239221
const { app, port } = await buildApp(ctx);
240222

241223
try {
242224
const res = await fetch(`http://127.0.0.1:${port}/v1/sessions`, {
243225
method: 'POST',
244226
headers: authHeaders(),
245-
body: JSON.stringify({ workDir: subTmpDir, prompt: 'Do the thing' }),
227+
body: JSON.stringify({ prompt: 'hello', workDir: tmpDir }),
246228
});
247-
expect(res.status).toBe(201);
248-
const body = await res.json() as Record<string, unknown>;
249-
const sessionId = body.id as string;
250229

251-
// At this point promptDelivery.status must be 'pending'
252-
expect((body.promptDelivery as Record<string, unknown>)?.status).toBe('pending');
253-
254-
// Resolve sendPrompt — simulating successful prompt delivery
255-
resolvePrompt({ delivered: true, attempts: 1 });
256-
257-
// Wait for the fire-and-forget callback to update session state
258-
await flushAsync(100);
259-
260-
// GET the session and verify promptDelivery was updated
261-
const getRes = await fetch(`http://127.0.0.1:${port}/v1/sessions/${sessionId}`, {
262-
headers: authHeaders(),
263-
});
264-
expect(getRes.status).toBe(200);
265-
const session = await getRes.json() as Record<string, unknown>;
266-
const pd = session.promptDelivery as { delivered: boolean; attempts: number; status: string };
267-
expect(pd.status).toBe('delivered');
268-
expect(pd.delivered).toBe(true);
269-
expect(pd.attempts).toBe(1);
230+
expect(res.status).toBe(201);
231+
const body = await res.json();
232+
expect(body.promptDelivery.status).toBe('failed');
233+
expect(body.promptDelivery.delivered).toBe(false);
234+
expect(body.promptDelivery.error).toBe('test failure');
270235
} finally {
271236
await app.close();
272237
}
273238
});
274239

275-
it('updates session.promptDelivery.status to failed after sendPrompt rejects', async () => {
276-
let rejectPrompt!: (err: Error) => void;
277-
const deferredPrompt = new Promise<{ delivered: boolean; attempts: number }>((_, r) => {
278-
rejectPrompt = r;
279-
});
280-
281-
const acpBackend = buildMockAcpBackend(() => deferredPrompt);
282-
const subTmpDir = mkdtempSync(join(tmpdir(), 'aegis-async-3243c-'));
283-
const { ctx, sessions } = await buildRouteContext(subTmpDir, acpBackend);
240+
it('returns promptDelivery.status: failed when sendPrompt throws', async () => {
241+
const tmpDir = mkdtempSync(join(tmpdir(), 'aegis-test-3271-'));
242+
const acp = buildMockAcpBackend(() => Promise.reject(new Error('connection reset')));
243+
const { ctx } = await buildRouteContext(tmpDir, acp);
284244
const { app, port } = await buildApp(ctx);
285245

286246
try {
287247
const res = await fetch(`http://127.0.0.1:${port}/v1/sessions`, {
288248
method: 'POST',
289249
headers: authHeaders(),
290-
body: JSON.stringify({ workDir: subTmpDir, prompt: 'Fail me' }),
250+
body: JSON.stringify({ prompt: 'hello', workDir: tmpDir }),
291251
});
292-
expect(res.status).toBe(201);
293-
const body = await res.json() as Record<string, unknown>;
294-
const sessionId = body.id as string;
295252

296-
expect((body.promptDelivery as Record<string, unknown>)?.status).toBe('pending');
297-
298-
// Reject sendPrompt — simulating delivery failure
299-
rejectPrompt(new Error('timeout'));
253+
// When sendPrompt throws, the route handler propagates the error → 500
254+
expect(res.status).toBe(500);
255+
} finally {
256+
await app.close();
257+
}
258+
});
300259

301-
await flushAsync(100);
260+
it('creates session without prompt when no prompt provided', async () => {
261+
const tmpDir = mkdtempSync(join(tmpdir(), 'aegis-test-3271-'));
262+
const acp = buildMockAcpBackend();
263+
const { ctx } = await buildRouteContext(tmpDir, acp);
264+
const { app, port } = await buildApp(ctx);
302265

303-
const getRes = await fetch(`http://127.0.0.1:${port}/v1/sessions/${sessionId}`, {
266+
try {
267+
const res = await fetch(`http://127.0.0.1:${port}/v1/sessions`, {
268+
method: 'POST',
304269
headers: authHeaders(),
270+
body: JSON.stringify({ workDir: tmpDir }),
305271
});
306-
const session = await getRes.json() as Record<string, unknown>;
307-
const pd = session.promptDelivery as { delivered: boolean; attempts: number; status: string };
308-
expect(pd.status).toBe('failed');
309-
expect(pd.delivered).toBe(false);
272+
273+
expect(res.status).toBe(201);
274+
const body = await res.json();
275+
expect(body.promptDelivery).toBeUndefined();
276+
expect(acp.sendPrompt).not.toHaveBeenCalled();
310277
} finally {
311278
await app.close();
312279
}
@@ -315,50 +282,26 @@ describe('Issue #3243 — async session create (ACP + prompt)', () => {
315282

316283
describe('SessionInfo.promptDelivery — status field', () => {
317284
it('accepts status: pending on promptDelivery', () => {
318-
const session: SessionInfo = {
319-
id: 'test', windowId: '', displayName: 'test', workDir: '/tmp',
320-
byteOffset: 0, monitorOffset: 0, status: 'pending',
321-
createdAt: Date.now(), lastActivity: Date.now(),
322-
stallThresholdMs: 300_000, permissionStallMs: 300_000,
323-
permissionMode: 'default',
324-
promptDelivery: { delivered: false, attempts: 0, status: 'pending' },
325-
};
285+
const session: Partial<SessionInfo> = {};
286+
session.promptDelivery = { delivered: false, attempts: 0, status: 'pending' };
326287
expect(session.promptDelivery?.status).toBe('pending');
327288
});
328289

329290
it('accepts status: delivered on promptDelivery', () => {
330-
const session: SessionInfo = {
331-
id: 'test', windowId: '', displayName: 'test', workDir: '/tmp',
332-
byteOffset: 0, monitorOffset: 0, status: 'idle',
333-
createdAt: Date.now(), lastActivity: Date.now(),
334-
stallThresholdMs: 300_000, permissionStallMs: 300_000,
335-
permissionMode: 'default',
336-
promptDelivery: { delivered: true, attempts: 1, status: 'delivered' },
337-
};
291+
const session: Partial<SessionInfo> = {};
292+
session.promptDelivery = { delivered: true, attempts: 1, status: 'delivered' };
338293
expect(session.promptDelivery?.status).toBe('delivered');
339294
});
340295

341296
it('accepts status: failed on promptDelivery', () => {
342-
const session: SessionInfo = {
343-
id: 'test', windowId: '', displayName: 'test', workDir: '/tmp',
344-
byteOffset: 0, monitorOffset: 0, status: 'error',
345-
createdAt: Date.now(), lastActivity: Date.now(),
346-
stallThresholdMs: 300_000, permissionStallMs: 300_000,
347-
permissionMode: 'default',
348-
promptDelivery: { delivered: false, attempts: 1, status: 'failed' },
349-
};
297+
const session: Partial<SessionInfo> = {};
298+
session.promptDelivery = { delivered: false, attempts: 1, status: 'failed' };
350299
expect(session.promptDelivery?.status).toBe('failed');
351300
});
352301

353302
it('accepts status: timeout on promptDelivery', () => {
354-
const session: SessionInfo = {
355-
id: 'test', windowId: '', displayName: 'test', workDir: '/tmp',
356-
byteOffset: 0, monitorOffset: 0, status: 'idle',
357-
createdAt: Date.now(), lastActivity: Date.now(),
358-
stallThresholdMs: 300_000, permissionStallMs: 300_000,
359-
permissionMode: 'default',
360-
promptDelivery: { delivered: false, attempts: 1, status: 'timeout' },
361-
};
303+
const session: Partial<SessionInfo> = {};
304+
session.promptDelivery = { delivered: false, attempts: 1, status: 'timeout' };
362305
expect(session.promptDelivery?.status).toBe('timeout');
363306
});
364307
});

src/routes/sessions.ts

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -456,7 +456,7 @@ export function registerSessionRoutes(app: FastifyInstance, ctx: RouteContext):
456456
meta: prompt ? { prompt: prompt.slice(0, 200), permissionMode: permissionMode ?? (autoApprove ? 'bypassPermissions' : undefined) } : undefined,
457457
});
458458

459-
let promptDelivery: { delivered: boolean; attempts: number; status?: 'pending' | 'delivered' | 'failed' | 'timeout' } | undefined;
459+
let promptDelivery: { delivered: boolean; attempts: number; status?: 'pending' | 'delivered' | 'failed' | 'timeout'; error?: string } | undefined;
460460
if (prompt) {
461461
let finalPrompt = prompt;
462462
if (memoryKeys && memoryKeys.length > 0 && memoryBridge) {
@@ -470,21 +470,16 @@ export function registerSessionRoutes(app: FastifyInstance, ctx: RouteContext):
470470
}
471471

472472
if (acpBackend && ctx.config.acpEnabled) {
473-
// Issue #3243: Async prompt delivery for ACP sessions.
474-
// Return immediately with status 'pending'; deliver prompt in background.
475-
// The client polls GET /v1/sessions/:id to check promptDelivery.status.
476-
promptDelivery = { delivered: false, attempts: 0, status: 'pending' };
473+
// Issue #3271: Reverted async prompt delivery (from #3243) to synchronous.
474+
// Async delivery caused promptDelivery tracking to hang — the JSON-RPC
475+
// session/prompt response was never received by the tracking layer even
476+
// though Claude Code processed the prompt successfully.
477+
// Synchronous delivery blocks until CC acknowledges, which is the
478+
// proven pre-#3243 behavior.
479+
const result = await acpBackend.sendPrompt(session.id, finalPrompt, { tenantId: req.tenantId ?? SYSTEM_TENANT, ownerKeyId: req.authKeyId ?? 'master' });
480+
promptDelivery = { delivered: result.delivered, attempts: result.attempts, status: result.delivered ? 'delivered' : 'failed', error: result.error };
477481
session.promptDelivery = promptDelivery;
478-
const deliverAsync = async () => {
479-
try {
480-
const result = await acpBackend.sendPrompt(session.id, finalPrompt, { tenantId: req.tenantId ?? SYSTEM_TENANT, ownerKeyId: req.authKeyId ?? 'master' });
481-
session.promptDelivery = { delivered: result.delivered, attempts: result.attempts, status: result.delivered ? 'delivered' : 'failed' };
482-
metrics.promptSent(result.delivered);
483-
} catch (e) {
484-
session.promptDelivery = { delivered: false, attempts: 1, status: 'failed' };
485-
}
486-
};
487-
void deliverAsync();
482+
metrics.promptSent(result.delivered);
488483
} else {
489484
promptDelivery = await sessions.sendInitialPrompt(session.id, finalPrompt);
490485
metrics.promptSent(promptDelivery.delivered);

0 commit comments

Comments
 (0)