mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-02 06:51:35 +00:00
fix(gateway): pass managed inbound PDFs through chat.send (#90115)
* 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 <noreply@anthropic.com> * fix(gateway): exempt managed PDFs from staging cap --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -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<Record<string, unknown>> {
|
||||
@@ -428,6 +431,22 @@ vi.mock("../../media/store.js", async () => {
|
||||
};
|
||||
});
|
||||
|
||||
vi.mock("../../media/media-reference.js", async () => {
|
||||
const original = await vi.importActual<typeof import("../../media/media-reference.js")>(
|
||||
"../../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,
|
||||
|
||||
@@ -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<ReturnType<typeof stageSandboxMedia>>;
|
||||
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(", ")})`,
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user