From eab39c721b48045c96a80fe4d80955cd9ed3fb0e Mon Sep 17 00:00:00 2001 From: Pejman Pour-Moezzi Date: Mon, 9 Mar 2026 09:37:33 -0700 Subject: [PATCH] fix(acp): map error states to end_turn instead of unconditional refusal (#41187) * fix(acp): map error states to end_turn instead of unconditional refusal * fix: map ACP error stop reason to end_turn (#41187) (thanks @pejmanjohn) --------- Co-authored-by: Pejman Pour-Moezzi <481729+pejmanjohn@users.noreply.github.com> Co-authored-by: Onur --- CHANGELOG.md | 1 + src/acp/translator.stop-reason.test.ts | 111 +++++++++++++++++++++++++ src/acp/translator.ts | 6 +- 3 files changed, 117 insertions(+), 1 deletion(-) create mode 100644 src/acp/translator.stop-reason.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 25398c660f4..375d906bd34 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ Docs: https://docs.openclaw.ai - Context engine/tests: add bundled-registry regression coverage for cross-chunk resolution, plugin-sdk re-exports, and concurrent chunk registration. (#40460) thanks @dsantoreis. - Agents/embedded runner: bound compaction retry waiting and drain embedded runs during SIGUSR1 restart so session lanes recover instead of staying blocked behind compaction. (#40324) thanks @cgdusek. - ACP/sessions.patch: allow `spawnedBy` and `spawnDepth` lineage fields on ACP session keys so `sessions_spawn` with `runtime: "acp"` no longer fails during child-session setup. Fixes #40971. (#40995) thanks @xaeon2026. +- ACP/stop reason mapping: resolve gateway chat `state: "error"` completions as ACP `end_turn` instead of `refusal` so transient backend failures are not surfaced as deliberate refusals. (#41187) thanks @pejmanjohn. ## 2026.3.8 diff --git a/src/acp/translator.stop-reason.test.ts b/src/acp/translator.stop-reason.test.ts new file mode 100644 index 00000000000..6e4a2f135af --- /dev/null +++ b/src/acp/translator.stop-reason.test.ts @@ -0,0 +1,111 @@ +import type { PromptRequest } from "@agentclientprotocol/sdk"; +import { describe, expect, it, vi } from "vitest"; +import type { GatewayClient } from "../gateway/client.js"; +import type { EventFrame } from "../gateway/protocol/index.js"; +import { createInMemorySessionStore } from "./session.js"; +import { AcpGatewayAgent } from "./translator.js"; +import { createAcpConnection, createAcpGateway } from "./translator.test-helpers.js"; + +type PendingPromptHarness = { + agent: AcpGatewayAgent; + promptPromise: ReturnType; + runId: string; +}; + +async function createPendingPromptHarness(): Promise { + const sessionId = "session-1"; + const sessionKey = "agent:main:main"; + + let runId: string | undefined; + const request = vi.fn(async (method: string, params?: Record) => { + if (method === "chat.send") { + runId = params?.idempotencyKey as string | undefined; + return new Promise(() => {}); + } + return {}; + }) as GatewayClient["request"]; + + const sessionStore = createInMemorySessionStore(); + sessionStore.createSession({ + sessionId, + sessionKey, + cwd: "/tmp", + }); + + const agent = new AcpGatewayAgent( + createAcpConnection(), + createAcpGateway(request as unknown as GatewayClient["request"]), + { sessionStore }, + ); + const promptPromise = agent.prompt({ + sessionId, + prompt: [{ type: "text", text: "hello" }], + _meta: {}, + } as unknown as PromptRequest); + + await vi.waitFor(() => { + expect(runId).toBeDefined(); + }); + + return { + agent, + promptPromise, + runId: runId!, + }; +} + +function createChatEvent(payload: Record): EventFrame { + return { + type: "event", + event: "chat", + payload, + } as EventFrame; +} + +describe("acp translator stop reason mapping", () => { + it("error state resolves as end_turn, not refusal", async () => { + const { agent, promptPromise, runId } = await createPendingPromptHarness(); + + await agent.handleGatewayEvent( + createChatEvent({ + runId, + sessionKey: "agent:main:main", + seq: 1, + state: "error", + errorMessage: "gateway timeout", + }), + ); + + await expect(promptPromise).resolves.toEqual({ stopReason: "end_turn" }); + }); + + it("error state with no errorMessage resolves as end_turn", async () => { + const { agent, promptPromise, runId } = await createPendingPromptHarness(); + + await agent.handleGatewayEvent( + createChatEvent({ + runId, + sessionKey: "agent:main:main", + seq: 1, + state: "error", + }), + ); + + await expect(promptPromise).resolves.toEqual({ stopReason: "end_turn" }); + }); + + it("aborted state resolves as cancelled", async () => { + const { agent, promptPromise, runId } = await createPendingPromptHarness(); + + await agent.handleGatewayEvent( + createChatEvent({ + runId, + sessionKey: "agent:main:main", + seq: 1, + state: "aborted", + }), + ); + + await expect(promptPromise).resolves.toEqual({ stopReason: "cancelled" }); + }); +}); diff --git a/src/acp/translator.ts b/src/acp/translator.ts index 0dbf3099c3d..7cf556f3e75 100644 --- a/src/acp/translator.ts +++ b/src/acp/translator.ts @@ -473,7 +473,11 @@ export class AcpGatewayAgent implements Agent { return; } if (state === "error") { - this.finishPrompt(pending.sessionId, pending, "refusal"); + // ACP has no explicit "server_error" stop reason. Use "end_turn" so clients + // do not treat transient backend errors (timeouts, rate-limits) as deliberate + // refusals. TODO: when ChatEventSchema gains a structured errorKind field + // (e.g. "refusal" | "timeout" | "rate_limit"), use it to distinguish here. + this.finishPrompt(pending.sessionId, pending, "end_turn"); } }