From 9a59485527daa54e01f557eab8282199c7de7441 Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Sat, 27 Jun 2026 08:01:35 +0200 Subject: [PATCH] fix(acp): treat session/prompt ack timeout as delivered MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit POST /v1/sessions/:id/send to an idle session returned a false-negative 422 PROMPT_DELIVERY_FAILED even though the prompt was delivered and the agent replied — the dashboard surfaced an error toast for a message that went through. Root cause: sendPrompt treated an AcpJsonRpcTimeoutError as "not delivered" (#4705). Empirically the request is written to a live JSON-RPC pipe and claude-agent-acp (single-threaded, in-order) processes the queued prompt; the ack merely lags past the 5s window on idle/cold-resumed sessions. A dead pipe rejects with a transport error, never a timeout — so a timeout is a slow ack, not a non-delivery. Reverses the #4705 assumption: timeout now returns delivered:true; the transcript remains the source of truth. Actual JSON-RPC errors (e.g. -32601) are still thrown. Updated the 3 test sites that asserted the old delivered:false / prompt_ack_timeout behavior. Verified live: /send on an idle session returns {"ok":true,"delivered":true,"attempts":1} and the reply appears in the transcript. Generated by Hephaestus (Aegis dev agent) --- src/__tests__/acp-backend.test.ts | 7 +-- .../acp-sendprompt-timeout-4705.test.ts | 48 +++++++++++++++---- src/services/acp/backend/prompts.ts | 16 +++++-- 3 files changed, 53 insertions(+), 18 deletions(-) diff --git a/src/__tests__/acp-backend.test.ts b/src/__tests__/acp-backend.test.ts index 0f17b3ff..bb1c4d92 100644 --- a/src/__tests__/acp-backend.test.ts +++ b/src/__tests__/acp-backend.test.ts @@ -661,10 +661,11 @@ describe('AcpBackend session lifecycle', () => { await backend.createSession({ ...scope, cwd }); const result = await backend.sendPrompt('session-1', 'hello', scope); - // Issue #4705: timeout means CC did not ack → delivered:false - expect(result.delivered).toBe(false); + // Timeout = slow ack on a live pipe, not non-delivery (request was written; + // claude-agent-acp processes it in order). Transcript is source of truth. + expect(result.delivered).toBe(true); expect(result.attempts).toBe(1); - expect(result.error).toBe('prompt_ack_timeout'); + expect(result.error).toBeUndefined(); }); it('sendPrompt surfaces -32601 Method not found errors (#3479, #4705)', async () => { diff --git a/src/__tests__/acp-sendprompt-timeout-4705.test.ts b/src/__tests__/acp-sendprompt-timeout-4705.test.ts index a741d199..a3e3ffd4 100644 --- a/src/__tests__/acp-sendprompt-timeout-4705.test.ts +++ b/src/__tests__/acp-sendprompt-timeout-4705.test.ts @@ -3,12 +3,15 @@ * * Issue #4705: API /v1/sessions/:id/send accepts message but never forwards to CC runtime. * - * Root cause: sendPrompt treats AcpJsonRpcTimeoutError as "delivered: true" when the - * JSON-RPC request to session/prompt times out. The timeout means CC did not acknowledge - * the prompt within 5s — the message may not have been delivered. + * Original fix (#4705): a session/prompt timeout → delivered:false (CC did not ack). + * Revised 2026-06-26: empirically the request IS written to a live JSON-RPC pipe and + * claude-agent-acp (single-threaded, in-order) processes it; the ack merely lags past + * the window on idle/cold-resumed sessions, and the reply always appears in the + * transcript. A dead pipe rejects with a transport error, never a timeout. So a + * timeout is now treated as a slow ack, not a non-delivery. * * Acceptance criteria: - * - Timeout on session/prompt → delivered: false with error 'prompt_ack_timeout' + * - Timeout on session/prompt → delivered: true (slow ack; transcript is source of truth) * - Actual error (e.g. -32601 Method not found) → thrown as AcpBackendLifecycleError * - Successful ack → delivered: true */ @@ -41,12 +44,12 @@ const scope: AcpSessionScope = { const cwd = '/tmp/test-workspace'; describe('Issue #4705: sendPrompt timeout handling', () => { - it('returns delivered:false when session/prompt times out', async () => { + it('returns delivered:true when session/prompt times out (slow ack, not non-delivery)', async () => { const service = new FakeSessionService(); const client = new FakeBackendClient(); client.setResult('initialize', {}); client.setResult('session/new', { sessionId: 'acp-agent-session-1' }); - // session/prompt will timeout + // session/prompt will timeout — request was still written to a live pipe. client.setTimeout('session/prompt', 50); const backend = new AcpBackend({ @@ -59,11 +62,9 @@ describe('Issue #4705: sendPrompt timeout handling', () => { const result = await backend.sendPrompt('session-1', 'hello', scope, cwd); - // BUG: currently returns delivered: true on timeout - // FIX: should return delivered: false with timeout error - expect(result.delivered).toBe(false); + expect(result.delivered).toBe(true); expect(result.attempts).toBe(1); - expect(result.error).toBe('prompt_ack_timeout'); + expect(result.error).toBeUndefined(); }, 10000); it('returns delivered:true when session/prompt acks within timeout', async () => { @@ -107,6 +108,33 @@ describe('Issue #4705: sendPrompt timeout handling', () => { }); }); +/** + * Direct sendPrompt: a session/prompt timeout on a live runtime surfaces as + * delivered:true (the request was written to a live JSON-RPC pipe; the ack + * merely lagged). This covers the continue-conversation case where an idle + * session acks slowly — the dashboard must not show a false error toast. + */ +describe('sendPrompt timeout on a live runtime', () => { + it('returns delivered:true when session/prompt times out', async () => { + const client = new FakeBackendClient(); + client.setTimeout('session/prompt', 50); + const runtime = { client } as unknown as import('../services/acp/backend/types.js').AcpBackendRuntime; + const deps = { + sessionService: { getSession: async () => ({ acpAgentSessionId: 'acp-agent-session-1' }) }, + inFlightPrompts: new Map(), + pendingHandshakes: new Map(), + } as unknown as import('../services/acp/backend/prompts.js').PromptDeps; + + const { sendPrompt } = await import('../services/acp/backend/prompts.js'); + + const result = await sendPrompt(deps, runtime, 'session-1', 'hello', scope); + + expect(result.delivered).toBe(true); + expect(result.attempts).toBe(1); + expect(result.error).toBeUndefined(); + }, 10000); +}); + // Fake client that supports timeout and error simulation class FakeBackendClient implements AcpBackendClient { readonly requests: { method: string; params: AcpJsonValue | undefined }[] = []; diff --git a/src/services/acp/backend/prompts.ts b/src/services/acp/backend/prompts.ts index 7f4a0a8e..f9a1cdd0 100644 --- a/src/services/acp/backend/prompts.ts +++ b/src/services/acp/backend/prompts.ts @@ -106,9 +106,15 @@ export async function sendPrompt( return { delivered: false, attempts: 0, error: 'no_agent_session' }; } - // Issue #4705: Fix timeout handling — timeout means CC did not ack, - // so the prompt was NOT delivered. Return delivered:false with timeout error. - // Actual JSON-RPC errors (e.g. -32601 Method not found) are thrown to caller. + // Issue #4705: a session/prompt timeout originally meant "CC did not ack, + // treat as not delivered". In practice the request is written to a live + // JSON-RPC pipe and claude-agent-acp (single-threaded, in-order) DOES + // process it — the ack simply lags past the window on idle/cold-resumed + // sessions (verified: the reply appears in the transcript on every + // observed timeout). A dead pipe rejects with a transport error, never a + // timeout. So a timeout is a slow ack, not a non-delivery: surface + // delivered:true and let the transcript be the source of truth. Actual + // JSON-RPC errors (e.g. -32601 Method not found) are still thrown. try { await runtime.client.request('session/prompt', { sessionId: acpSessionId, @@ -116,8 +122,8 @@ export async function sendPrompt( }, { timeoutMs: ACP_PROMPT_ACK_TIMEOUT_MS }); } catch (err) { if (err instanceof Error && err.name === 'AcpJsonRpcTimeoutError') { - log.warn({ component: 'acp-backend', operation: 'promptAckTimeout', attributes: { sessionId } }); - return { delivered: false, attempts: 1, error: 'prompt_ack_timeout' }; + log.warn({ component: 'acp-backend', operation: 'promptAckTimeoutAccepted', attributes: { sessionId } }); + return { delivered: true, attempts: 1 }; } else { throw err; }