From 158d3db85a470f78bbc5cbc52ea6d7504d88eb2f Mon Sep 17 00:00:00 2001 From: Harjoth Khara Date: Mon, 15 Jun 2026 09:39:17 -0700 Subject: [PATCH] fix(gateway): pass managed inbound PDFs through chat.send (#90115) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(gateway): pass managed inbound PDFs through when sandbox staging fails chat.send force-stages offloaded non-image media into the sandbox workspace when one exists. If that optional staging was unavailable or incomplete, prestageMediaPathOffloads deleted the media buffers and failed the whole send with a 5xx — even for already-managed inbound PDFs that are safe to read host-side. A Control-UI-uploaded PDF could fail to send. When staging throws or is incomplete, fall back to the absolute managed paths iff every non-image offloaded ref is a managed-inbound application/pdf (reusing the existing resolveInboundMediaReference allow-check + the PDF mime type). This mirrors the existing no-sandbox passthrough: with MediaWorkspaceDir unset the managed media dir is a default media-understanding local root, so the absolute path resolves host-side. Gated all-or-nothing so a single non-managed or non-PDF ref keeps the previous delete + 5xx behavior. Success path and oversized 4xx are unchanged; managed buffers are not deleted on the fallback. Fixes #90097 Co-Authored-By: Claude Opus 4.8 * fix(gateway): exempt managed PDFs from staging cap --------- Co-authored-by: Claude Opus 4.8 --- .../chat.directive-tags.test.ts | 228 +++++++++++++++++- src/gateway/server-methods/chat.ts | 65 ++++- 2 files changed, 281 insertions(+), 12 deletions(-) diff --git a/src/gateway/server-methods/chat.directive-tags.test.ts b/src/gateway/server-methods/chat.directive-tags.test.ts index 5e4e0755a79a..3c49ca54de09 100644 --- a/src/gateway/server-methods/chat.directive-tags.test.ts +++ b/src/gateway/server-methods/chat.directive-tags.test.ts @@ -107,6 +107,9 @@ const mockState = vi.hoisted(() => ({ // stageSandboxMedia silently skips over-cap files. unstagedSources: null as string[] | null, deleteMediaBufferCalls: [] as Array<{ id: string; subdir?: string }>, + // Paths the mocked resolveInboundMediaReference treats as managed inbound + // media-store entries (drives the #90097 managed-PDF staging-fallback tests). + managedInboundPaths: null as string[] | null, })); function readTranscriptJsonLines(transcriptPath: string): Array> { @@ -428,6 +431,22 @@ vi.mock("../../media/store.js", async () => { }; }); +vi.mock("../../media/media-reference.js", async () => { + const original = await vi.importActual( + "../../media/media-reference.js", + ); + return { + ...original, + // Treat only test-declared paths as managed inbound media, so the #90097 + // managed-PDF fallback can be exercised without a real on-disk media store. + resolveInboundMediaReference: vi.fn(async (source: string) => + mockState.managedInboundPaths?.includes(source) + ? { id: source, normalizedSource: source, physicalPath: source, sourceType: "path" } + : null, + ), + }; +}); + const { chatHandlers } = await import("./chat.js"); async function waitForAssertion(assertion: () => void, timeoutMs = 1000, stepMs = 2) { @@ -842,6 +861,7 @@ describe("chat directive tag stripping for non-streaming final payloads", () => mockState.stagedRelativePaths = null; mockState.unstagedSources = null; mockState.deleteMediaBufferCalls = []; + mockState.managedInboundPaths = null; mockState.hasBeforeAgentRunHooks = false; mockState.beforeMessageWriteBlock = false; mockState.beforeMessageWriteContent = null; @@ -5573,6 +5593,198 @@ describe("chat directive tag stripping for non-streaming final payloads", () => ]); }); + it("falls back to the managed inbound PDF path when sandbox staging is incomplete (#90097)", async () => { + createTranscriptFixture("openclaw-chat-send-managed-pdf-incomplete-"); + mockState.finalText = "ok"; + mockState.sessionEntry = { modelProvider: "test-provider", model: "vision-model" }; + mockState.modelCatalog = [ + { + provider: "test-provider", + id: "vision-model", + name: "Vision model", + input: ["text", "image"], + }, + ]; + const managedPath = "/home/user/.openclaw/media/inbound/report.pdf"; + mockState.savedMediaResults = [{ path: managedPath, contentType: "application/pdf" }]; + mockState.sandboxWorkspace = { workspaceDir: "/sandbox/workspace" }; + // Staging returns an incomplete map (the file silently fell out). + mockState.stagedRelativePaths = ["media/inbound/report.pdf"]; + mockState.unstagedSources = [managedPath]; + mockState.managedInboundPaths = [managedPath]; + const respond = vi.fn(); + const context = createChatContext(); + const pdf = Buffer.from("%PDF-1.4\n").toString("base64"); + + await runNonStreamingChatSend({ + context, + respond, + idempotencyKey: "idem-managed-pdf-incomplete", + message: "read this", + requestParams: { + attachments: [ + { type: "file", mimeType: "application/pdf", fileName: "report.pdf", content: pdf }, + ], + }, + expectBroadcast: false, + }); + + // Send proceeds with the absolute managed path; buffers are NOT deleted. + expect(mockState.deleteMediaBufferCalls).toEqual([]); + expect(mockState.lastDispatchCtx?.MediaPaths).toEqual([managedPath]); + expect(mockState.lastDispatchCtx?.MediaPath).toBe(managedPath); + expect(mockState.lastDispatchCtx?.MediaTypes).toEqual(["application/pdf"]); + // Host-side readable: no sandbox workspace dir is carried for the fallback. + expect(mockState.lastDispatchCtx?.MediaWorkspaceDir).toBeUndefined(); + // Marker still blocks the dispatch pipeline from re-running staging. + expect(mockState.lastDispatchCtx?.MediaStaged).toBe(true); + }); + + it("falls back to the managed inbound PDF path when sandbox staging throws (#90097)", async () => { + createTranscriptFixture("openclaw-chat-send-managed-pdf-throw-"); + mockState.finalText = "ok"; + mockState.sessionEntry = { modelProvider: "test-provider", model: "vision-model" }; + mockState.modelCatalog = [ + { + provider: "test-provider", + id: "vision-model", + name: "Vision model", + input: ["text", "image"], + }, + ]; + const managedPath = "/home/user/.openclaw/media/inbound/report.pdf"; + mockState.savedMediaResults = [{ path: managedPath, contentType: "application/pdf" }]; + mockState.sandboxWorkspace = { workspaceDir: "/sandbox/workspace" }; + mockState.stageSandboxMediaError = new Error("ENOSPC: no space left on device"); + mockState.managedInboundPaths = [managedPath]; + const respond = vi.fn(); + const context = createChatContext(); + const pdf = Buffer.from("%PDF-1.4\n").toString("base64"); + + await runNonStreamingChatSend({ + context, + respond, + idempotencyKey: "idem-managed-pdf-throw", + message: "read this", + requestParams: { + attachments: [ + { type: "file", mimeType: "application/pdf", fileName: "report.pdf", content: pdf }, + ], + }, + expectBroadcast: false, + }); + + expect(mockState.deleteMediaBufferCalls).toEqual([]); + expect(mockState.lastDispatchCtx?.MediaPaths).toEqual([managedPath]); + expect(mockState.lastDispatchCtx?.MediaWorkspaceDir).toBeUndefined(); + expect(mockState.lastDispatchCtx?.MediaStaged).toBe(true); + }); + + it("does not fall back when not every offloaded ref is a managed PDF (#90097)", async () => { + createTranscriptFixture("openclaw-chat-send-managed-pdf-mixed-"); + mockState.finalText = "ok"; + mockState.sessionEntry = { modelProvider: "test-provider", model: "vision-model" }; + mockState.modelCatalog = [ + { + provider: "test-provider", + id: "vision-model", + name: "Vision model", + input: ["text", "image"], + }, + ]; + const managedPdf = "/home/user/.openclaw/media/inbound/report.pdf"; + const managedZip = "/home/user/.openclaw/media/inbound/data.zip"; + mockState.savedMediaResults = [ + { path: managedPdf, contentType: "application/pdf" }, + { path: managedZip, contentType: "application/zip" }, + ]; + mockState.sandboxWorkspace = { workspaceDir: "/sandbox/workspace" }; + mockState.stagedRelativePaths = ["media/inbound/report.pdf", "media/inbound/data.zip"]; + mockState.unstagedSources = [managedPdf]; + // Both managed, but the non-PDF ref must block the all-or-nothing fallback. + mockState.managedInboundPaths = [managedPdf, managedZip]; + const respond = vi.fn(); + const context = createChatContext(); + const pdf = Buffer.from("%PDF-1.4\n").toString("base64"); + const zip = Buffer.from("PKdata").toString("base64"); + + await runNonStreamingChatSend({ + context, + respond, + idempotencyKey: "idem-managed-pdf-mixed", + message: "read these", + requestParams: { + attachments: [ + { type: "file", mimeType: "application/pdf", fileName: "report.pdf", content: pdf }, + { type: "file", mimeType: "application/zip", fileName: "data.zip", content: zip }, + ], + }, + expectBroadcast: false, + waitFor: "none", + }); + + // Mixed batch keeps the existing delete + 5xx behavior (no silent passthrough). + expect(mockState.lastDispatchCtx).toBeUndefined(); + const [ok, , error] = lastRespondCall(respond) ?? []; + expect(ok).toBe(false); + expect(error?.code).toBe(ErrorCodes.UNAVAILABLE); + expect(mockState.deleteMediaBufferCalls.map((c) => c.id).toSorted()).toEqual([ + "saved-media", + "saved-media", + ]); + }); + + it("passes sandbox-oversized managed inbound PDFs through instead of rejecting them (#90097)", async () => { + createTranscriptFixture("openclaw-chat-send-managed-pdf-oversize-"); + mockState.finalText = "ok"; + mockState.sessionEntry = { + modelProvider: "test-provider", + model: "vision-model", + }; + mockState.modelCatalog = [ + { + provider: "test-provider", + id: "vision-model", + name: "Vision model", + input: ["text", "image"], + }, + ]; + const managedPath = "/home/user/.openclaw/media/inbound/huge.pdf"; + mockState.savedMediaResults = [{ path: managedPath, contentType: "application/pdf" }]; + mockState.sandboxWorkspace = { workspaceDir: "/sandbox/workspace" }; + mockState.managedInboundPaths = [managedPath]; + const respond = vi.fn(); + const context = createChatContext(); + const oversized = Buffer.alloc(6 * 1024 * 1024); + oversized.set(Buffer.from("%PDF-1.4\n"), 0); + + await runNonStreamingChatSend({ + context, + respond, + idempotencyKey: "idem-managed-pdf-oversize", + message: "read this", + requestParams: { + attachments: [ + { + type: "file", + mimeType: "application/pdf", + fileName: "huge.pdf", + content: oversized.toString("base64"), + }, + ], + }, + expectBroadcast: false, + }); + + expect(mockState.deleteMediaBufferCalls).toEqual([]); + expect(mockState.lastDispatchCtx?.MediaPath).toBe(managedPath); + expect(mockState.lastDispatchCtx?.MediaPaths).toEqual([managedPath]); + expect(mockState.lastDispatchCtx?.MediaType).toBe("application/pdf"); + expect(mockState.lastDispatchCtx?.MediaTypes).toEqual(["application/pdf"]); + expect(mockState.lastDispatchCtx?.MediaWorkspaceDir).toBeUndefined(); + expect(mockState.lastDispatchCtx?.MediaStaged).toBe(true); + }); + it("rejects sandbox-oversized non-image attachments as 4xx before staging", async () => { // Regression: resolveChatAttachmentMaxBytes defaults to 20MB, but // stageSandboxMedia caps each file at STAGED_MEDIA_MAX_BYTES (5MB) and @@ -5595,15 +5807,18 @@ describe("chat directive tag stripping for non-streaming final payloads", () => }, ]; mockState.savedMediaResults = [ - { path: "/home/user/.openclaw/media/inbound/huge.pdf", contentType: "application/pdf" }, + { + path: "/home/user/.openclaw/media/inbound/huge.bin", + contentType: "application/octet-stream", + }, ]; mockState.sandboxWorkspace = { workspaceDir: "/sandbox/workspace" }; const respond = vi.fn(); const context = createChatContext(); // 6MB buffer — above STAGED_MEDIA_MAX_BYTES (5MB) but below the 20MB parse cap. const oversized = Buffer.alloc(6 * 1024 * 1024); - oversized.set(Buffer.from("%PDF-1.4\n"), 0); - const pdf = oversized.toString("base64"); + oversized.set(Buffer.from("OPENCLAW-BINARY\n"), 0); + const oversizedPayload = oversized.toString("base64"); await runNonStreamingChatSend({ context, @@ -5612,7 +5827,12 @@ describe("chat directive tag stripping for non-streaming final payloads", () => message: "read this", requestParams: { attachments: [ - { type: "file", mimeType: "application/pdf", fileName: "huge.pdf", content: pdf }, + { + type: "file", + mimeType: "application/octet-stream", + fileName: "huge.bin", + content: oversizedPayload, + }, ], }, expectBroadcast: false, diff --git a/src/gateway/server-methods/chat.ts b/src/gateway/server-methods/chat.ts index 48b2a419d99d..a70c99d43664 100644 --- a/src/gateway/server-methods/chat.ts +++ b/src/gateway/server-methods/chat.ts @@ -78,6 +78,7 @@ import { appendLocalMediaParentRoots, getAgentScopedMediaLocalRoots, } from "../../media/local-roots.js"; +import { resolveInboundMediaReference } from "../../media/media-reference.js"; import type { PromptImageOrderEntry } from "../../media/prompt-image-order.js"; import { deleteMediaBuffer, @@ -1344,6 +1345,32 @@ function stripTrailingOffloadedMediaMarkers(message: string, refs: OffloadedRef[ // Returned paths are absolute media-store paths when no sandbox is active, or // sandbox-relative paths plus `workspaceDir` when sandboxing is active. Host-side // media-understanding uses MediaWorkspaceDir to resolve those relative paths. +// Optional sandbox staging is a convenience copy into the container; a managed +// inbound media-store PDF is already host-readable (the managed media dir is a +// default media-understanding local root, resolved via the absolute path when +// MediaWorkspaceDir is unset). When staging is unavailable/incomplete we can +// therefore pass those absolute managed paths through instead of deleting the +// buffers and failing the send (#90097). Gated all-or-nothing to managed-inbound +// PDFs so a single non-managed or non-PDF ref cannot bypass staging; reuses the +// same resolveInboundMediaReference allow-check stageSandboxMedia applies. +async function resolveManagedInboundPdfFallback( + refs: readonly OffloadedRef[], +): Promise<{ paths: string[]; types: string[] } | null> { + for (const ref of refs) { + if (ref.mimeType !== "application/pdf") { + return null; + } + const managed = await resolveInboundMediaReference(ref.path).catch(() => null); + if (!managed) { + return null; + } + } + return { + paths: refs.map((ref) => ref.path), + types: refs.map((ref) => ref.mimeType), + }; +} + async function prestageMediaPathOffloads(params: { offloadedRefs: OffloadedRef[]; includeImageRefs?: boolean; @@ -1377,9 +1404,14 @@ async function prestageMediaPathOffloads(params: { // (resolveChatAttachmentMaxBytes, default 20MB) is higher, so a sandboxed // session receiving a file between the two caps would otherwise // pass parse, fail staging, and surface as a retryable 5xx even though - // retry cannot succeed. Reject here as a client-side 4xx instead. + // retry cannot succeed. Managed inbound PDFs are already host-readable, so + // let the managed-PDF fallback handle them before rejecting everything else. const oversizedForSandbox = mediaPathRefs.filter((ref) => ref.sizeBytes > MEDIA_MAX_BYTES); if (oversizedForSandbox.length > 0) { + const managedPdfFallback = await resolveManagedInboundPdfFallback(mediaPathRefs); + if (managedPdfFallback) { + return managedPdfFallback; + } const details = oversizedForSandbox .map((ref) => `${ref.label} (${ref.sizeBytes} bytes)`) .join(", "); @@ -1395,13 +1427,24 @@ async function prestageMediaPathOffloads(params: { MediaType: mediaPathRefs[0].mimeType, MediaTypes: mediaPathRefs.map((ref) => ref.mimeType), }; - const stageResult = await stageSandboxMedia({ - ctx: stagingCtx, - sessionCtx: stagingCtx as TemplateContext, - cfg: params.cfg, - sessionKey: params.sessionKey, - workspaceDir, - }); + let stageResult: Awaited>; + try { + stageResult = await stageSandboxMedia({ + ctx: stagingCtx, + sessionCtx: stagingCtx as TemplateContext, + cfg: params.cfg, + sessionKey: params.sessionKey, + workspaceDir, + }); + } catch (stageErr) { + // Staging is optional; pass managed-inbound PDFs through rather than + // deleting buffers + failing the send. Rethrow otherwise (#90097). + const managedPdfFallback = await resolveManagedInboundPdfFallback(mediaPathRefs); + if (managedPdfFallback) { + return managedPdfFallback; + } + throw stageErr; + } // stageSandboxMedia silently keeps unstaged entries as their original // absolute path, so length parity with `nonImage` does not prove every @@ -1412,6 +1455,12 @@ async function prestageMediaPathOffloads(params: { const stagedSources = stageResult.staged; const missing = mediaPathRefs.filter((ref) => !stagedSources.has(ref.path)); if (missing.length > 0) { + // Incomplete staging: pass managed-inbound PDFs through (host-readable) + // rather than failing the send. Fail otherwise as before (#90097). + const managedPdfFallback = await resolveManagedInboundPdfFallback(mediaPathRefs); + if (managedPdfFallback) { + return managedPdfFallback; + } throw new Error( `attachment staging incomplete: ${stagedSources.size}/${mediaPathRefs.length} paths staged into sandbox workspace (missing: ${missing.map((ref) => ref.path).join(", ")})`, );