diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b140351b5d..93483d148d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,7 @@ Docs: https://docs.openclaw.ai - ACP/runtime attachments: forward normalized inbound image attachments into ACP runtime turns so ACPX sessions can preserve image prompt content on the runtime path. (#41427) Thanks @mbelinky. - ACP/regressions: add gateway RPC coverage for ACP lineage patching, ACPX runtime coverage for image prompt serialization, and an operator smoke-test procedure for live ACP spawn verification. (#41456) Thanks @mbelinky. - Agents/billing recovery: probe single-provider billing cooldowns on the existing throttle so topping up credits can recover without a manual gateway restart. (#41422) thanks @altaywtf. +- ACP/follow-up hardening: make session restore and prompt completion degrade gracefully on transcript/update failures, enforce bounded tool-location traversal, and skip non-image ACPX turns the runtime cannot serialize. (#41464) Thanks @mbelinky. ## 2026.3.8 diff --git a/src/acp/event-mapper.test.ts b/src/acp/event-mapper.test.ts new file mode 100644 index 00000000000..2aca401d483 --- /dev/null +++ b/src/acp/event-mapper.test.ts @@ -0,0 +1,18 @@ +import { describe, expect, it } from "vitest"; +import { extractToolCallLocations } from "./event-mapper.js"; + +describe("extractToolCallLocations", () => { + it("enforces the global node visit cap across nested structures", () => { + const nested = Array.from({ length: 20 }, (_, outer) => + Array.from({ length: 20 }, (_, inner) => + inner === 19 ? { path: `/tmp/file-${outer}.txt` } : { note: `${outer}-${inner}` }, + ), + ); + + const locations = extractToolCallLocations(nested); + + expect(locations).toBeDefined(); + expect(locations?.length).toBeLessThan(20); + expect(locations).not.toContainEqual({ path: "/tmp/file-19.txt" }); + }); +}); diff --git a/src/acp/event-mapper.ts b/src/acp/event-mapper.ts index 2a74f5691cf..c164f356307 100644 --- a/src/acp/event-mapper.ts +++ b/src/acp/event-mapper.ts @@ -186,9 +186,10 @@ function collectLocationsFromTextMarkers( function collectToolLocations( value: unknown, locations: Map, - state: { visited: number; depth: number }, + state: { visited: number }, + depth: number, ): void { - if (state.visited >= TOOL_LOCATION_MAX_NODES || state.depth > TOOL_LOCATION_MAX_DEPTH) { + if (state.visited >= TOOL_LOCATION_MAX_NODES || depth > TOOL_LOCATION_MAX_DEPTH) { return; } state.visited += 1; @@ -202,8 +203,7 @@ function collectToolLocations( } if (Array.isArray(value)) { for (const item of value) { - collectToolLocations(item, locations, { visited: state.visited, depth: state.depth + 1 }); - state.visited += 1; + collectToolLocations(item, locations, state, depth + 1); if (state.visited >= TOOL_LOCATION_MAX_NODES) { return; } @@ -230,9 +230,11 @@ function collectToolLocations( } } - for (const nested of Object.values(record)) { - collectToolLocations(nested, locations, { visited: state.visited, depth: state.depth + 1 }); - state.visited += 1; + for (const [key, nested] of Object.entries(record)) { + if (key === "content") { + continue; + } + collectToolLocations(nested, locations, state, depth + 1); if (state.visited >= TOOL_LOCATION_MAX_NODES) { return; } @@ -402,7 +404,7 @@ export function extractToolCallContent(value: unknown): ToolCallContent[] | unde export function extractToolCallLocations(...values: unknown[]): ToolCallLocation[] | undefined { const locations = new Map(); for (const value of values) { - collectToolLocations(value, locations, { visited: 0, depth: 0 }); + collectToolLocations(value, locations, { visited: 0 }, 0); } return locations.size > 0 ? [...locations.values()] : undefined; } diff --git a/src/acp/translator.session-rate-limit.test.ts b/src/acp/translator.session-rate-limit.test.ts index a591d30e1ac..d08ae1a1567 100644 --- a/src/acp/translator.session-rate-limit.test.ts +++ b/src/acp/translator.session-rate-limit.test.ts @@ -365,6 +365,63 @@ describe("acp session UX bridge behavior", () => { sessionStore.clearAllSessionsForTest(); }); + + it("falls back to an empty transcript when sessions.get fails during loadSession", async () => { + const sessionStore = createInMemorySessionStore(); + const connection = createAcpConnection(); + const sessionUpdate = connection.__sessionUpdateMock; + const request = vi.fn(async (method: string) => { + if (method === "sessions.list") { + return { + ts: Date.now(), + path: "/tmp/sessions.json", + count: 1, + defaults: { + modelProvider: null, + model: null, + contextTokens: null, + }, + sessions: [ + { + key: "agent:main:recover", + label: "recover", + displayName: "Recover session", + kind: "direct", + updatedAt: 1_710_000_000_000, + thinkingLevel: "adaptive", + modelProvider: "openai", + model: "gpt-5.4", + }, + ], + }; + } + if (method === "sessions.get") { + throw new Error("sessions.get unavailable"); + } + return { ok: true }; + }) as GatewayClient["request"]; + const agent = new AcpGatewayAgent(connection, createAcpGateway(request), { + sessionStore, + }); + + const result = await agent.loadSession(createLoadSessionRequest("agent:main:recover")); + + expect(result.modes?.currentModeId).toBe("adaptive"); + expect(sessionUpdate).toHaveBeenCalledWith({ + sessionId: "agent:main:recover", + update: expect.objectContaining({ + sessionUpdate: "available_commands_update", + }), + }); + expect(sessionUpdate).not.toHaveBeenCalledWith({ + sessionId: "agent:main:recover", + update: expect.objectContaining({ + sessionUpdate: "user_message_chunk", + }), + }); + + sessionStore.clearAllSessionsForTest(); + }); }); describe("acp setSessionMode bridge behavior", () => { @@ -771,6 +828,61 @@ describe("acp session metadata and usage updates", () => { sessionStore.clearAllSessionsForTest(); }); + + it("still resolves prompts when snapshot updates fail after completion", async () => { + const sessionStore = createInMemorySessionStore(); + const connection = createAcpConnection(); + const sessionUpdate = connection.__sessionUpdateMock; + const request = vi.fn(async (method: string) => { + if (method === "sessions.list") { + return { + ts: Date.now(), + path: "/tmp/sessions.json", + count: 1, + defaults: { + modelProvider: null, + model: null, + contextTokens: null, + }, + sessions: [ + { + key: "usage-session", + displayName: "Usage session", + kind: "direct", + updatedAt: 1_710_000_123_000, + thinkingLevel: "adaptive", + modelProvider: "openai", + model: "gpt-5.4", + totalTokens: 1200, + totalTokensFresh: true, + contextTokens: 4000, + }, + ], + }; + } + if (method === "chat.send") { + return new Promise(() => {}); + } + return { ok: true }; + }) as GatewayClient["request"]; + const agent = new AcpGatewayAgent(connection, createAcpGateway(request), { + sessionStore, + }); + + await agent.loadSession(createLoadSessionRequest("usage-session")); + sessionUpdate.mockClear(); + sessionUpdate.mockRejectedValueOnce(new Error("session update transport failed")); + + const promptPromise = agent.prompt(createPromptRequest("usage-session", "hello")); + await agent.handleGatewayEvent(createChatFinalEvent("usage-session")); + + await expect(promptPromise).resolves.toEqual({ stopReason: "end_turn" }); + const session = sessionStore.getSession("usage-session"); + expect(session?.activeRunId).toBeNull(); + expect(session?.abortController).toBeNull(); + + sessionStore.clearAllSessionsForTest(); + }); }); describe("acp prompt size hardening", () => { diff --git a/src/acp/translator.ts b/src/acp/translator.ts index e7fa4a7382e..667c075e9c0 100644 --- a/src/acp/translator.ts +++ b/src/acp/translator.ts @@ -458,7 +458,10 @@ export class AcpGatewayAgent implements Agent { this.log(`loadSession: ${session.sessionId} -> ${session.sessionKey}`); const [sessionSnapshot, transcript] = await Promise.all([ this.getSessionSnapshot(session.sessionKey), - this.getSessionTranscript(session.sessionKey), + this.getSessionTranscript(session.sessionKey).catch((err) => { + this.log(`session transcript fallback for ${session.sessionKey}: ${String(err)}`); + return []; + }), ]); await this.replaySessionTranscript(session.sessionId, transcript); await this.sendSessionSnapshotUpdate(session.sessionId, sessionSnapshot, { @@ -630,7 +633,6 @@ export class AcpGatewayAgent implements Agent { if (!session) { return; } - this.sessionStore.cancelActiveRun(params.sessionId); try { await this.gateway.request("chat.abort", { sessionKey: session.sessionKey }); @@ -841,9 +843,13 @@ export class AcpGatewayAgent implements Agent { this.pendingPrompts.delete(sessionId); this.sessionStore.clearActiveRun(sessionId); const sessionSnapshot = await this.getSessionSnapshot(pending.sessionKey); - await this.sendSessionSnapshotUpdate(sessionId, sessionSnapshot, { - includeControls: false, - }); + try { + await this.sendSessionSnapshotUpdate(sessionId, sessionSnapshot, { + includeControls: false, + }); + } catch (err) { + this.log(`session snapshot update failed for ${sessionId}: ${String(err)}`); + } pending.resolve({ stopReason }); } diff --git a/src/auto-reply/reply/dispatch-acp.test.ts b/src/auto-reply/reply/dispatch-acp.test.ts index 290846a6075..b19f2edde09 100644 --- a/src/auto-reply/reply/dispatch-acp.test.ts +++ b/src/auto-reply/reply/dispatch-acp.test.ts @@ -362,28 +362,58 @@ describe("tryDispatchAcpReply", () => { setReadyAcpResolution(); const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "dispatch-acp-")); const imagePath = path.join(tempDir, "inbound.png"); - await fs.writeFile(imagePath, "image-bytes"); - managerMocks.runTurn.mockResolvedValue(undefined); + try { + await fs.writeFile(imagePath, "image-bytes"); + managerMocks.runTurn.mockResolvedValue(undefined); - await runDispatch({ - bodyForAgent: " ", - ctxOverrides: { - MediaPath: imagePath, - MediaType: "image/png", - }, - }); + await runDispatch({ + bodyForAgent: " ", + ctxOverrides: { + MediaPath: imagePath, + MediaType: "image/png", + }, + }); - expect(managerMocks.runTurn).toHaveBeenCalledWith( - expect.objectContaining({ - text: "", - attachments: [ - { - mediaType: "image/png", - data: Buffer.from("image-bytes").toString("base64"), - }, - ], - }), - ); + expect(managerMocks.runTurn).toHaveBeenCalledWith( + expect.objectContaining({ + text: "", + attachments: [ + { + mediaType: "image/png", + data: Buffer.from("image-bytes").toString("base64"), + }, + ], + }), + ); + } finally { + await fs.rm(tempDir, { recursive: true, force: true }); + } + }); + + it("skips ACP turns for non-image attachments when there is no text prompt", async () => { + setReadyAcpResolution(); + const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "dispatch-acp-")); + const docPath = path.join(tempDir, "inbound.pdf"); + const { dispatcher } = createDispatcher(); + const onReplyStart = vi.fn(); + try { + await fs.writeFile(docPath, "pdf-bytes"); + + await runDispatch({ + bodyForAgent: " ", + dispatcher, + onReplyStart, + ctxOverrides: { + MediaPath: docPath, + MediaType: "application/pdf", + }, + }); + + expect(managerMocks.runTurn).not.toHaveBeenCalled(); + expect(onReplyStart).not.toHaveBeenCalled(); + } finally { + await fs.rm(tempDir, { recursive: true, force: true }); + } }); it("surfaces ACP policy errors as final error replies", async () => { diff --git a/src/auto-reply/reply/dispatch-acp.ts b/src/auto-reply/reply/dispatch-acp.ts index 3b89feaae13..8fc7110fc4c 100644 --- a/src/auto-reply/reply/dispatch-acp.ts +++ b/src/auto-reply/reply/dispatch-acp.ts @@ -16,6 +16,7 @@ import { logVerbose } from "../../globals.js"; import { getSessionBindingService } from "../../infra/outbound/session-binding-service.js"; import { generateSecureUuid } from "../../infra/secure-random.js"; import { prefixSystemMessage } from "../../infra/system-message.js"; +import { applyMediaUnderstanding } from "../../media-understanding/apply.js"; import { normalizeAttachmentPath, normalizeAttachments, @@ -69,6 +70,10 @@ async function resolveAcpAttachments(ctx: FinalizedMsgContext): Promise