diff --git a/src/gateway/chat-attachments.test.ts b/src/gateway/chat-attachments.test.ts index 67d90cab15e3..fe3041009378 100644 --- a/src/gateway/chat-attachments.test.ts +++ b/src/gateway/chat-attachments.test.ts @@ -156,7 +156,7 @@ afterEach(() => { }); describe("persistInboundImagesForTranscript", () => { - it("preserves mixed image order and appends non-image offloads", async () => { + it("preserves original mixed-media order in claim-only transcript facts", async () => { saveMediaBufferMock.mockResolvedValueOnce({ id: "inline", path: "/media/inbound/inline.jpg", @@ -164,39 +164,103 @@ describe("persistInboundImagesForTranscript", () => { contentType: "image/jpeg", }); - const saved = await persistInboundImagesForTranscript({ - images: [{ type: "image", data: "aGVsbG8=", mimeType: "image/jpeg" }], - imageOrder: ["offloaded", "inline"], + const result = await persistInboundImagesForTranscript({ + images: [ + { + type: "image", + data: "aGVsbG8=", + mimeType: "image/jpeg", + sourceIndex: 1, + }, + ], offloadedRefs: [ { - mediaRef: "media://inbound/offloaded", - id: "offloaded", - path: "/media/inbound/offloaded.png", - kind: "image", - mimeType: "image/png", - label: "offloaded.png", - sizeBytes: 2_100_000, - }, - { - mediaRef: "media://inbound/report", - id: "report", - path: "/media/inbound/report.pdf", - kind: "document", - mimeType: "application/pdf", - label: "report.pdf", + mediaRef: "https://signed.example/private-video", + id: "video", + path: "/media/inbound/video.mp4", + kind: "video", + mimeType: "video/mp4", + label: "video.mp4", sizeBytes: 100, + durationMs: 2_000, + sourceIndex: 0, }, ], log: { warn: vi.fn() }, logContext: "test", }); - expect(saved.map((entry) => entry.path)).toEqual([ - "/media/inbound/offloaded.png", - "/media/inbound/inline.jpg", - "/media/inbound/report.pdf", + expect(result.entries.map((entry) => entry.sourceIndex)).toEqual([0, 1]); + expect(result.entries.map((entry) => entry.fact)).toEqual([ + { + url: "media://inbound/video", + contentType: "video/mp4", + kind: "video", + fileName: "video.mp4", + sizeBytes: 100, + durationMs: 2_000, + hydrationSuppressed: true, + }, + { + url: "media://inbound/inline", + contentType: "image/jpeg", + kind: "image", + sizeBytes: 5, + }, ]); + expect(result.omission).toBe("none"); + const durable = JSON.stringify(result.entries.map((entry) => entry.fact)); + expect(durable).not.toContain("/media/"); + expect(durable).not.toContain("signed.example"); + expect(durable).not.toContain("sourceIndex"); }); + + it("reports an inline image whose durable managed save fails", async () => { + saveMediaBufferMock.mockRejectedValueOnce(new Error("disk unavailable")); + const warn = vi.fn(); + + const result = await persistInboundImagesForTranscript({ + images: [ + { + type: "image", + data: "aGVsbG8=", + mimeType: "image/jpeg", + sourceIndex: 0, + }, + ], + offloadedRefs: [], + log: { warn }, + logContext: "test", + }); + + expect(result).toEqual({ entries: [], omission: "inline-image-save-failed" }); + expect(warn).toHaveBeenCalledWith(expect.stringContaining("disk unavailable")); + }); + + it.each(["nested/id", String.raw`nested\id`, "bad\0id", ".", "..", "claim?sig", "claim#part"])( + "rejects an unsafe saved media id %j before projecting a durable claim", + async (id) => { + await expect( + persistInboundImagesForTranscript({ + images: [], + offloadedRefs: [ + { + mediaRef: "https://signed.example/private", + id, + path: "/media/inbound/private", + kind: "video", + mimeType: "video/mp4", + label: "private.mp4", + sizeBytes: 10, + sourceIndex: 0, + }, + ], + log: { warn: vi.fn() }, + logContext: "test", + }), + ).rejects.toThrow(); + }, + ); }); describe("parseMessageWithAttachments", () => { @@ -289,8 +353,10 @@ describe("parseMessageWithAttachments", () => { it("keeps image inline and offloads non-image side by side", async () => { const { parsed } = await parseWithWarnings("x", [pngAttachment(), pdfAttachment()]); expectSingleInlinePng(parsed); + expect(parsed.images[0]?.sourceIndex).toBe(0); expect(parsed.offloadedRefs).toHaveLength(1); expect(parsed.offloadedRefs[0]?.mimeType).toBe("application/pdf"); + expect(parsed.offloadedRefs[0]?.sourceIndex).toBe(1); expect(parsed.imageOrder).toEqual(["inline"]); }); diff --git a/src/gateway/chat-attachments.ts b/src/gateway/chat-attachments.ts index 64c51454cf13..2e8feca42cde 100644 --- a/src/gateway/chat-attachments.ts +++ b/src/gateway/chat-attachments.ts @@ -9,9 +9,10 @@ import { formatErrorMessage, formatUncaughtError } from "../infra/errors.js"; import type { SubsystemLogger } from "../logging/subsystem.js"; import type { MediaFact } from "../media/media-facts.js"; import { probeMediaFilesWithinBudget } from "../media/media-probe.js"; +import { parseInboundMediaUri } from "../media/media-reference.js"; import type { PromptImageOrderEntry } from "../media/prompt-image-order.js"; import { sniffMimeFromBase64 } from "../media/sniff-mime-from-base64.js"; -import { deleteMediaBuffer, saveMediaBuffer, type SavedMedia } from "../media/store.js"; +import { deleteMediaBuffer, saveMediaBuffer } from "../media/store.js"; import { DEFAULT_CHAT_ATTACHMENT_MAX_BYTES } from "./chat-attachment-policy.js"; import { formatForLog } from "./ws-log.js"; @@ -30,6 +31,7 @@ export type ChatImageContent = { type: "image"; data: string; mimeType: string; + sourceIndex: number; }; export type OffloadedRef = { @@ -40,6 +42,7 @@ export type OffloadedRef = { mimeType: string; label: string; sizeBytes: number; + sourceIndex: number; durationMs?: number; width?: number; height?: number; @@ -64,12 +67,18 @@ type NormalizedAttachment = { base64: string; }; -type SavedMediaRef = { - id: string; - path: string; - durationMs?: number; - width?: number; - height?: number; +export const INLINE_IMAGE_DURABLE_OMISSION_MARKER = + "[image attachment omitted: durable managed media claim unavailable]"; + +type PersistInboundImagesResult = { + entries: Array<{ + id: string; + path: string; + sourceIndex: number; + imageKind?: PromptImageOrderEntry; + fact: MediaFact; + }>; + omission: "none" | "inline-image-save-failed"; }; const OFFLOAD_THRESHOLD_BYTES = 2_000_000; @@ -122,54 +131,62 @@ export function stripImageMediaMarkers(message: string, refs: readonly Offloaded export async function persistInboundImagesForTranscript(params: { images: ChatImageContent[]; - imageOrder: PromptImageOrderEntry[]; offloadedRefs: OffloadedRef[]; log: Pick; logContext: string; -}): Promise { - const inline: SavedMedia[] = []; +}): Promise { + const entries: PersistInboundImagesResult["entries"] = []; + let omission: PersistInboundImagesResult["omission"] = "none"; for (const image of params.images) { try { - inline.push( - await saveMediaBuffer(Buffer.from(image.data, "base64"), image.mimeType, "inbound"), + const saved = await saveMediaBuffer( + Buffer.from(image.data, "base64"), + image.mimeType, + "inbound", ); + const trusted = assertSavedMedia(saved, `inline image ${image.sourceIndex + 1}`); + entries.push({ + id: trusted.id, + path: trusted.path, + sourceIndex: image.sourceIndex, + imageKind: "inline", + fact: { + url: trusted.mediaRef, + contentType: saved.contentType ?? image.mimeType, + kind: "image", + sizeBytes: saved.size, + }, + }); } catch (err) { + omission = "inline-image-save-failed"; params.log.warn( `${params.logContext}: failed to persist inbound image (${image.mimeType}): ${formatErrorMessage(err)}`, ); } } - const imageOffloaded: SavedMedia[] = []; - const nonImageOffloaded: SavedMedia[] = []; for (const ref of params.offloadedRefs) { - const saved = { + const fact: MediaFact = { + url: buildManagedInboundMediaRef(ref.id), + contentType: ref.mimeType, + kind: ref.kind, + fileName: ref.label, + sizeBytes: ref.sizeBytes, + ...(ref.durationMs !== undefined ? { durationMs: ref.durationMs } : {}), + ...(ref.width !== undefined ? { width: ref.width } : {}), + ...(ref.height !== undefined ? { height: ref.height } : {}), + ...(ref.mimeType.startsWith("image/") ? {} : { hydrationSuppressed: true }), + }; + entries.push({ id: ref.id, path: ref.path, - size: ref.sizeBytes, - contentType: ref.mimeType, - }; - (ref.mimeType.startsWith("image/") ? imageOffloaded : nonImageOffloaded).push(saved); + sourceIndex: ref.sourceIndex, + ...(ref.mimeType.startsWith("image/") ? { imageKind: "offloaded" as const } : {}), + fact, + }); } - if (params.imageOrder.length === 0) { - return [...inline, ...imageOffloaded, ...nonImageOffloaded]; - } - - const ordered: SavedMedia[] = []; - let inlineIndex = 0; - let offloadedIndex = 0; - for (const entry of params.imageOrder) { - const media = entry === "inline" ? inline[inlineIndex++] : imageOffloaded[offloadedIndex++]; - if (media) { - ordered.push(media); - } - } - ordered.push( - ...inline.slice(inlineIndex), - ...imageOffloaded.slice(offloadedIndex), - ...nonImageOffloaded, - ); - return ordered; + entries.sort((left, right) => left.sourceIndex - right.sourceIndex); + return { entries, omission }; } type UnsupportedAttachmentReason = @@ -299,7 +316,19 @@ function ensureExtension(label: string, mime: string): string { return ext ? `${label}${ext}` : label; } -function assertSavedMedia(value: unknown, label: string): SavedMediaRef { +function buildManagedInboundMediaRef(id: string): string { + const candidate = `media://inbound/${id}`; + const parsed = parseInboundMediaUri(candidate); + if (!parsed || parsed.id !== id) { + throw new Error("Saved media ID failed canonical validation"); + } + return parsed.normalizedSource; +} + +function assertSavedMedia( + value: unknown, + label: string, +): { id: string; mediaRef: string; path: string } { if ( value === null || typeof value !== "object" || @@ -309,20 +338,11 @@ function assertSavedMedia(value: unknown, label: string): SavedMediaRef { throw new Error(`attachment ${label}: saveMediaBuffer returned an unexpected shape`); } const id = (value as Record).id as string; - if (id.length === 0) { - throw new Error(`attachment ${label}: saveMediaBuffer returned an empty media ID`); - } - if (id.includes("/") || id.includes("\\") || id.includes("\0")) { - throw new Error( - `attachment ${label}: saveMediaBuffer returned an unsafe media ID ` + - `(contains path separator or null byte)`, - ); - } const path = (value as Record).path; if (typeof path !== "string" || path.length === 0) { throw new Error(`attachment ${label}: saveMediaBuffer returned no on-disk path`); } - return { id, path }; + return { id, mediaRef: buildManagedInboundMediaRef(id), path }; } function normalizeAttachment( @@ -482,7 +502,7 @@ export async function parseMessageWithAttachments( shouldForceImageOffload || !isImage || sizeBytes > OFFLOAD_THRESHOLD_BYTES; if (!shouldOffload) { - images.push({ type: "image", data: b64, mimeType: finalMime }); + images.push({ type: "image", data: b64, mimeType: finalMime, sourceIndex: idx }); imageOrder.push("inline"); continue; } @@ -490,7 +510,7 @@ export async function parseMessageWithAttachments( const buffer = Buffer.from(b64, "base64"); verifyDecodedSize(buffer, sizeBytes, label); - let savedMedia: SavedMediaRef; + let savedMedia: ReturnType; try { const labelWithExt = ensureExtension(label, finalMime); const rawResult = await saveMediaBuffer( @@ -510,7 +530,7 @@ export async function parseMessageWithAttachments( savedMediaIds.push(savedMedia.id); - const mediaRef = `media://inbound/${savedMedia.id}`; + const mediaRef = savedMedia.mediaRef; updatedMessage += `\n[media attached: ${mediaRef}]`; log?.info?.( shouldForceImageOffload && isImage @@ -526,6 +546,7 @@ export async function parseMessageWithAttachments( mimeType: finalMime, label, sizeBytes, + sourceIndex: idx, ...(typeof att.durationMs === "number" && Number.isFinite(att.durationMs) && att.durationMs >= 0 diff --git a/src/gateway/server-methods/chat-send-user-turn.test.ts b/src/gateway/server-methods/chat-send-user-turn.test.ts index 4dd63945823d..872458965fcd 100644 --- a/src/gateway/server-methods/chat-send-user-turn.test.ts +++ b/src/gateway/server-methods/chat-send-user-turn.test.ts @@ -16,6 +16,7 @@ import { buildPersistedUserTurnMessage, type UserTurnInput, } from "../../sessions/user-turn-transcript.js"; +import * as chatAttachments from "../chat-attachments.js"; import { applyChatSendManagedMedia, prepareChatSendUserTurn } from "./chat-send-user-turn.js"; function createUserTurnInputController() { @@ -53,10 +54,17 @@ function createAttachments( mediaPathOffloadTypes: string[]; mediaPathOffloadWorkspaceDir: string | undefined; imageOrder: Array<"inline" | "offloaded">; + parsedImages: Array<{ + type: "image"; + data: string; + mimeType: string; + sourceIndex: number; + }>; offloadedRefs: Array<{ mediaRef: string; id: string; path: string; + sourceIndex: number; kind: "image" | "audio" | "video" | "document" | "sticker" | "unknown"; mimeType: string; label: string; @@ -253,6 +261,7 @@ describe("prepareChatSendUserTurn", () => { mimeType: "image/png", label: "image.png", sizeBytes: 10, + sourceIndex: 0, }, ], parsedMessage: `inspect\n[media attached: ${mediaRef}]`, @@ -275,6 +284,118 @@ describe("prepareChatSendUserTurn", () => { }); }); + it("persists video then image as claim-only facts with the image at fact index one", async () => { + const { controller, readInput } = createUserTurnInputController(); + prepareChatSendUserTurn({ + request: { + clientInfo: createClientInfo(), + normalizedAttachments: [{}, {}], + suppressCommandInterpretation: false, + systemInputProvenance: undefined, + systemProvenanceReceipt: undefined, + }, + session: { + agentId: "main", + clientRunId: "run-mixed", + sessionKey: "agent:main:main", + }, + admission: { + originatingRoute: { originatingChannel: "webchat", explicitDeliverRoute: false }, + }, + attachments: createAttachments({ + imageOrder: ["offloaded"], + offloadedRefs: [ + { + mediaRef: "https://signed.example/video", + id: "video.mp4", + path: "/private/media/video.mp4", + sourceIndex: 0, + kind: "video", + mimeType: "video/mp4", + label: "video.mp4", + sizeBytes: 20, + }, + { + mediaRef: "file:///private/image.png", + id: "image.png", + path: "/private/media/image.png", + sourceIndex: 1, + kind: "image", + mimeType: "image/png", + label: "image.png", + sizeBytes: 10, + }, + ], + }), + client: null, + logGateway: { warn: vi.fn() } as never, + userTurn: controller, + }); + + const input = await readInput(); + expect(input.media?.map((fact) => fact.kind)).toEqual(["video", "image"]); + expect(input.mediaImageLayout).toEqual({ + slots: [{ kind: "offloaded", factIndex: 1 }], + }); + const serialized = JSON.stringify(buildPersistedUserTurnMessage(input)); + expect(serialized).toContain("media://inbound/video.mp4"); + expect(serialized).toContain("media://inbound/image.png"); + for (const privateValue of [ + "/private/media", + "signed.example", + "file://", + "workspaceDir", + '"data"', + "base64", + ]) { + expect(serialized).not.toContain(privateValue); + } + }); + + it("records a visible durable omission without failing the live inline-image turn", async () => { + const persist = vi + .spyOn(chatAttachments, "persistInboundImagesForTranscript") + .mockResolvedValueOnce({ entries: [], omission: "inline-image-save-failed" }); + try { + const { controller, readInput } = createUserTurnInputController(); + const prepared = prepareChatSendUserTurn({ + request: { + clientInfo: createClientInfo(), + normalizedAttachments: [{}], + suppressCommandInterpretation: false, + systemInputProvenance: undefined, + systemProvenanceReceipt: undefined, + }, + session: { + agentId: "main", + clientRunId: "run-omission", + sessionKey: "agent:main:main", + }, + admission: { + originatingRoute: { originatingChannel: "webchat", explicitDeliverRoute: false }, + }, + attachments: createAttachments({ + imageOrder: ["inline"], + parsedImages: [ + { type: "image", data: "aGVsbG8=", mimeType: "image/jpeg", sourceIndex: 0 }, + ], + }), + client: null, + logGateway: { warn: vi.fn() } as never, + userTurn: controller, + }); + + expect(prepared.replyOptionImages).toEqual([ + { type: "image", data: "aGVsbG8=", mimeType: "image/jpeg", sourceIndex: 0 }, + ]); + await expect(readInput()).resolves.toMatchObject({ + text: "raw message\n[image attachment omitted: durable managed media claim unavailable]", + }); + } finally { + persist.mockRestore(); + } + }); + it.each([ { kind: "audio" as const, mimeType: "audio/mpeg", fileName: "voice.mp3" }, { kind: "video" as const, mimeType: "video/mp4", fileName: "clip.mp4" }, @@ -307,6 +428,7 @@ describe("prepareChatSendUserTurn", () => { mimeType, label: fileName, sizeBytes: 12, + sourceIndex: 0, }, ], parsedMessage: `play this\n[media attached: ${mediaRef}]`, @@ -319,7 +441,6 @@ describe("prepareChatSendUserTurn", () => { const input = await readInput(); expect(input.media).toEqual([ { - path: `/media/inbound/${fileName}`, url: mediaRef, contentType: mimeType, kind, @@ -335,7 +456,7 @@ describe("prepareChatSendUserTurn", () => { ).toEqual(input.media); }); - it("persists and prunes the staged PDF claim-check alias as structured ownership", async () => { + it("persists and prunes the managed PDF claim as structured ownership", async () => { const { controller, readInput } = createUserTurnInputController(); const mediaRef = "media://inbound/report.pdf"; prepareChatSendUserTurn({ @@ -364,6 +485,7 @@ describe("prepareChatSendUserTurn", () => { mimeType: "application/pdf", label: "report.pdf", sizeBytes: 10, + sourceIndex: 0, }, ], parsedMessage: `read this\n[media attached: ${mediaRef}]`, @@ -376,7 +498,6 @@ describe("prepareChatSendUserTurn", () => { const input = await readInput(); expect(input.media).toEqual([ { - path: "/media/inbound/report.pdf", url: mediaRef, contentType: "application/pdf", kind: "document", @@ -445,6 +566,7 @@ describe("prepareChatSendUserTurn", () => { mimeType: "image/png", label: "image.png", sizeBytes: 10, + sourceIndex: 0, }, ], parsedMessage: text, @@ -457,7 +579,6 @@ describe("prepareChatSendUserTurn", () => { const input = await readInput(); expect(input.media).toEqual([ { - path: imagePath, url: mediaRef, contentType: "image/png", kind: "image", @@ -475,7 +596,6 @@ describe("prepareChatSendUserTurn", () => { ).media, ).toEqual([ { - path: imagePath, url: mediaRef, contentType: "image/png", kind: "image", diff --git a/src/gateway/server-methods/chat-send-user-turn.ts b/src/gateway/server-methods/chat-send-user-turn.ts index b1a6996d5f79..077114f322df 100644 --- a/src/gateway/server-methods/chat-send-user-turn.ts +++ b/src/gateway/server-methods/chat-send-user-turn.ts @@ -2,14 +2,13 @@ import path from "node:path"; import type { GatewayClientInfo } from "../../../packages/gateway-protocol/src/client-info.js"; import type { RuntimeMsgContext as MsgContext } from "../../auto-reply/templating.js"; import type { MediaFact } from "../../media/media-facts.js"; -import type { PromptImageOrderEntry } from "../../media/prompt-image-order.js"; -import type { SavedMedia } from "../../media/store.js"; import type { InputProvenance } from "../../sessions/input-provenance.js"; import type { UserTurnInput } from "../../sessions/user-turn-transcript.js"; import { INTERNAL_MESSAGE_CHANNEL, isOperatorUiClient } from "../../utils/message-channel.js"; import { type ChatImageContent, type OffloadedRef, + INLINE_IMAGE_DURABLE_OMISSION_MARKER, persistInboundImagesForTranscript, } from "../chat-attachments.js"; import { isAcpBridgeClient } from "./chat-origin-routing.js"; @@ -31,32 +30,34 @@ type ChatSendUserTurnInputController = { setInputPromise: (input: Promise) => void; }; +type PersistedChatSendMedia = Awaited< + ReturnType +>["entries"]; + async function persistChatSendImages(params: { images: ChatImageContent[]; - imageOrder: PromptImageOrderEntry[]; offloadedRefs: OffloadedRef[]; client: GatewayRequestHandlerOptions["client"]; logGateway: GatewayRequestContext["logGateway"]; -}): Promise { +}): Promise>> { if ( (params.images.length === 0 && params.offloadedRefs.length === 0) || isAcpBridgeClient(params.client) ) { - return []; + return { entries: [], omission: "none" }; } return await persistInboundImagesForTranscript({ images: params.images, - imageOrder: params.imageOrder, offloadedRefs: params.offloadedRefs, log: params.logGateway, logContext: "chat.send", }); } -function resolveChatSendManagedMedia(savedImages: SavedMedia[]): MediaFact[] { - return savedImages.map((entry) => ({ +function resolveChatSendManagedMedia(entries: PersistedChatSendMedia): MediaFact[] { + return entries.map((entry) => ({ path: entry.path, - contentType: entry.contentType ?? "application/octet-stream", + contentType: entry.fact.contentType ?? "application/octet-stream", })); } @@ -66,39 +67,6 @@ export function applyChatSendManagedMedia(ctx: MsgContext, media: MediaFact[]): } } -function buildChatSendUserTurnMedia( - savedMedia: SavedMedia[], - offloadedRefs: OffloadedRef[], -): MediaFact[] { - const offloadedRefsById = new Map(offloadedRefs.map((ref) => [ref.id, ref] as const)); - return savedMedia.map((entry) => { - const offloadedRef = offloadedRefsById.get(entry.id); - return { - path: entry.path, - ...(offloadedRef - ? { - // Every offload keeps its claim-check alias so persisted marker - // ownership survives; only non-images skip native image hydration. - url: offloadedRef.mediaRef, - kind: offloadedRef.kind, - fileName: offloadedRef.label, - sizeBytes: offloadedRef.sizeBytes, - ...(offloadedRef.durationMs !== undefined - ? { durationMs: offloadedRef.durationMs } - : {}), - ...(offloadedRef.width !== undefined ? { width: offloadedRef.width } : {}), - ...(offloadedRef.height !== undefined ? { height: offloadedRef.height } : {}), - ...(offloadedRef.mimeType.startsWith("image/") ? {} : { hydrationSuppressed: true }), - } - : {}), - contentType: entry.contentType, - ...(offloadedRef?.durationMs ? { durationMs: offloadedRef.durationMs } : {}), - ...(offloadedRef?.width ? { width: offloadedRef.width } : {}), - ...(offloadedRef?.height ? { height: offloadedRef.height } : {}), - }; - }); -} - function buildChatSendPromptMedia( attachments: PreparedChatSendAttachments, ): MediaFact[] | undefined { @@ -231,35 +199,35 @@ export function prepareChatSendUserTurn(params: { const { request, session, admission, attachments, client, logGateway, userTurn } = params; const persistedMediaForTranscriptPromise = persistChatSendImages({ images: attachments.parsedImages, - imageOrder: attachments.imageOrder, offloadedRefs: attachments.offloadedRefs, client, logGateway, }); - const preparedUserTurnMediaPromise: Promise = - request.normalizedAttachments.length > 0 - ? persistedMediaForTranscriptPromise.then((media) => - buildChatSendUserTurnMedia(media, attachments.offloadedRefs), - ) - : Promise.resolve([]); userTurn.setInputPromise( - preparedUserTurnMediaPromise.then((media) => ({ - ...userTurn.baseInput, - ...(media.length > 0 ? { media } : {}), - ...(media.length > 0 && attachments.imageOrder.length > 0 - ? { - mediaImageLayout: { - // persistInboundImagesForTranscript emits image facts in this exact order, - // then appends non-images, so image slot ordinals are fact ordinals. - slots: attachments.imageOrder.map((kind, factIndex) => ({ kind, factIndex })), - }, - } - : {}), - })), + persistedMediaForTranscriptPromise.then((result) => { + const media = result.entries.map((entry) => entry.fact); + const slots = result.entries.flatMap((entry, factIndex) => + entry.imageKind ? [{ kind: entry.imageKind, factIndex }] : [], + ); + return { + ...userTurn.baseInput, + ...(result.omission === "inline-image-save-failed" + ? { + text: [userTurn.baseInput.text, INLINE_IMAGE_DURABLE_OMISSION_MARKER] + .filter(Boolean) + .join("\n"), + } + : {}), + ...(media.length > 0 ? { media } : {}), + ...(slots.length > 0 ? { mediaImageLayout: { slots } } : {}), + }; + }), ); const pluginBoundMediaPromise = attachments.explicitOriginTargetsPlugin && attachments.parsedImages.length > 0 - ? persistedMediaForTranscriptPromise.then(resolveChatSendManagedMedia) + ? persistedMediaForTranscriptPromise.then((result) => + resolveChatSendManagedMedia(result.entries), + ) : Promise.resolve([]); void pluginBoundMediaPromise.catch(() => undefined); const messageContext = buildChatSendMessageContext({ diff --git a/src/gateway/server-methods/chat.directive-tags.test.ts b/src/gateway/server-methods/chat.directive-tags.test.ts index 7b1627602897..dd6ac7f83b7e 100644 --- a/src/gateway/server-methods/chat.directive-tags.test.ts +++ b/src/gateway/server-methods/chat.directive-tags.test.ts @@ -137,7 +137,7 @@ const mockState = vi.hoisted(() => ({ message?: unknown; messageId?: string; }>, - savedMediaResults: [] as Array<{ path: string; contentType?: string }>, + savedMediaResults: [] as Array<{ id?: string; path: string; contentType?: string }>, saveMediaError: null as Error | null, savedMediaCalls: [] as Array<{ contentType?: string; subdir?: string; size: number }>, saveMediaWait: null as Promise | null, @@ -621,7 +621,7 @@ vi.mock("../../media/store.js", async () => { const next = mockState.savedMediaResults.shift(); try { return { - id: "saved-media", + id: next?.id ?? "saved-media", path: next?.path ?? `/tmp/${mockState.savedMediaCalls.length}.png`, size: buffer.byteLength, contentType: next?.contentType ?? contentType, @@ -640,6 +640,28 @@ async function waitForAssertion(assertion: () => void, timeoutMs = 5_000, stepMs await vi.waitFor(assertion, { interval: stepMs, timeout: timeoutMs }); } +function expectClaimOnlyTranscriptMedia( + message: unknown, + expectedMedia: unknown[], + forbiddenValues: string[], +) { + const media = ( + message as { __openclaw?: { media?: Array> } } | undefined + )?.["__openclaw"]?.media; + expect(media).toEqual(expectedMedia); + for (const fact of media ?? []) { + expect(fact.url).toMatch(/^media:\/\/inbound\/[^?#]+$/u); + expect(fact).not.toHaveProperty("path"); + expect(fact).not.toHaveProperty("workspaceDir"); + expect(fact).not.toHaveProperty("data"); + } + const serialized = JSON.stringify(message); + expect(serialized).not.toContain("base64"); + for (const value of forbiddenValues) { + expect(serialized).not.toContain(value); + } +} + function createFixturePaths(prefix: string): { dir: string; transcriptPath: string } { const dir = fs.mkdtempSync(path.join(suiteFixtureRoot, `${suiteFixtureSeq++}-${prefix}`)); const transcriptPath = path.join(dir, "sess.jsonl"); @@ -5650,12 +5672,20 @@ describe("chat directive tag stripping for non-streaming final payloads", () => }); }); - it("prepares persisted media paths for Pi user-turn persistence", async () => { + it("prepares managed image claims for Pi user-turn persistence", async () => { await createReadyChatTranscript("openclaw-chat-send-user-transcript-images-"); mockState.triggerAgentRunStart = true; mockState.savedMediaResults = [ - { path: "/tmp/chat-send-image-a.png", contentType: "image/png" }, - { path: "/tmp/chat-send-image-b.jpg", contentType: "image/jpeg" }, + { + id: "chat-send-image-a.png", + path: "/tmp/chat-send-image-a.png", + contentType: "image/png", + }, + { + id: "chat-send-image-b.jpg", + path: "/tmp/chat-send-image-b.jpg", + contentType: "image/jpeg", + }, ]; const { send } = createChatRequestFixture(); @@ -5696,30 +5726,35 @@ describe("chat directive tag stripping for non-streaming final payloads", () => expect(typeof mockState.savedMediaCalls[0]?.size).toBe("number"); expect(typeof mockState.savedMediaCalls[1]?.size).toBe("number"); const userTurnInput = mockState.lastDispatchUserTurnInput as - | { - __openclaw?: { media?: Array<{ contentType?: string; path?: string }> }; - content?: unknown; - } + | { content?: unknown } | undefined; if (!userTurnInput) { throw new Error("expected user turn input with media metadata"); } expect(findUserUpdate()).toBeUndefined(); expect(userTurnInput.content).toBe("edit these"); - expect(userTurnInput["__openclaw"]?.media?.map((fact) => fact.path)).toEqual([ - "/tmp/chat-send-image-a.png", - "/tmp/chat-send-image-b.jpg", - ]); - expect(userTurnInput["__openclaw"]?.media?.map((fact) => fact.contentType)).toEqual([ - "image/png", - "image/jpeg", - ]); + expectClaimOnlyTranscriptMedia( + userTurnInput, + [ + expect.objectContaining({ + url: "media://inbound/chat-send-image-a.png", + contentType: "image/png", + kind: "image", + }), + expect.objectContaining({ + url: "media://inbound/chat-send-image-b.jpg", + contentType: "image/jpeg", + kind: "image", + }), + ], + ["/tmp/chat-send-image-a.png", "/tmp/chat-send-image-b.jpg"], + ); expect(mockState.lastDispatchCtx?.media).toBeUndefined(); expect(mockState.lastDispatchImages).toHaveLength(2); }); }); - it("prepares non-image chat.send attachments as media refs without dispatch images", async () => { + it("prepares non-image chat.send attachments as claim-only media refs without dispatch images", async () => { await createReadyChatTranscript("openclaw-chat-send-user-transcript-file-"); mockState.triggerAgentRunStart = true; mockState.savedMediaResults = [ @@ -5746,10 +5781,7 @@ describe("chat directive tag stripping for non-streaming final payloads", () => await waitForAssertion(() => { const userTurnInput = mockState.lastDispatchUserTurnInput as - | { - __openclaw?: { media?: Array<{ contentType?: string; path?: string }> }; - content?: unknown; - } + | { content?: unknown } | undefined; expect(mockState.lastDispatchImages).toBeUndefined(); expect(mockState.lastDispatchImageOrder).toBeUndefined(); @@ -5761,16 +5793,24 @@ describe("chat directive tag stripping for non-streaming final payloads", () => expect(typeof mockState.savedMediaCalls[0]?.size).toBe("number"); expect(findUserUpdate()).toBeUndefined(); expect(userTurnInput?.content).toBe("summarize this"); - expect(userTurnInput?.["__openclaw"]?.media).toEqual([ - expect.objectContaining({ - path: "/tmp/chat-send-brief.pdf", - contentType: "application/pdf", - }), - ]); + expectClaimOnlyTranscriptMedia( + userTurnInput, + [ + { + url: "media://inbound/saved-media", + contentType: "application/pdf", + kind: "document", + fileName: "brief.pdf", + sizeBytes: 9, + hydrationSuppressed: true, + }, + ], + ["/tmp/chat-send-brief.pdf", "%PDF-1.4"], + ); }); }); - it("preserves offloaded attachment media paths in transcript order", async () => { + it("preserves managed attachment claims in transcript order", async () => { await createReadyChatTranscript("openclaw-chat-send-user-transcript-offloaded-"); mockState.triggerAgentRunStart = true; mockState.sessionEntry = { @@ -5788,8 +5828,16 @@ describe("chat directive tag stripping for non-streaming final payloads", () => }, ]; mockState.savedMediaResults = [ - { path: "/tmp/offloaded-big.png", contentType: "image/png" }, - { path: "/tmp/chat-send-inline.png", contentType: "image/png" }, + { + id: "offloaded-big.png", + path: "/tmp/offloaded-big.png", + contentType: "image/png", + }, + { + id: "chat-send-inline.png", + path: "/tmp/chat-send-inline.png", + contentType: "image/png", + }, ]; const { send } = createChatRequestFixture(); const bigPng = Buffer.alloc(2_100_000); @@ -5817,17 +5865,26 @@ describe("chat directive tag stripping for non-streaming final payloads", () => await waitForAssertion(() => { const userTurnInput = mockState.lastDispatchUserTurnInput as - | { - __openclaw?: { media?: Array<{ path?: string }> }; - content?: unknown; - } + | { content?: unknown } | undefined; expect(findUserUpdate()).toBeUndefined(); expect(userTurnInput?.content).toBe("edit both"); - expect(userTurnInput?.["__openclaw"]?.media?.map((fact) => fact.path)).toEqual([ - "/tmp/chat-send-inline.png", - "/tmp/offloaded-big.png", - ]); + expectClaimOnlyTranscriptMedia( + userTurnInput, + [ + expect.objectContaining({ + url: "media://inbound/chat-send-inline.png", + contentType: "image/png", + kind: "image", + }), + expect.objectContaining({ + url: "media://inbound/offloaded-big.png", + contentType: "image/png", + kind: "image", + }), + ], + ["/tmp/chat-send-inline.png", "/tmp/offloaded-big.png"], + ); expect(userTurnInput?.content).not.toContain("media://"); }); }); diff --git a/src/gateway/server-node-events.runtime.ts b/src/gateway/server-node-events.runtime.ts index c39b49fc69ba..d31cf40c5fcf 100644 --- a/src/gateway/server-node-events.runtime.ts +++ b/src/gateway/server-node-events.runtime.ts @@ -22,6 +22,7 @@ export { normalizeMainKey } from "../routing/session-key.js"; export { defaultRuntime } from "../runtime.js"; export { resolveChatAttachmentMaxBytes } from "./chat-attachment-policy.js"; export { + INLINE_IMAGE_DURABLE_OMISSION_MARKER, parseMessageWithAttachments, persistInboundImagesForTranscript, } from "./chat-attachments.js"; diff --git a/src/gateway/server-node-events.test.ts b/src/gateway/server-node-events.test.ts index eb8fb6fb072d..4ac38a5b4b42 100644 --- a/src/gateway/server-node-events.test.ts +++ b/src/gateway/server-node-events.test.ts @@ -98,6 +98,8 @@ const runtimeMocks = vi.hoisted(() => ({ enqueueSystemEvent: vi.fn(), formatForLog: vi.fn((err: unknown) => (err instanceof Error ? err.message : String(err))), getRuntimeConfig: vi.fn(() => ({ session: { mainKey: "agent:main:main" } })), + INLINE_IMAGE_DURABLE_OMISSION_MARKER: + "[image attachment omitted: durable managed media claim unavailable]", loadOrCreateProcessDeviceIdentity: loadOrCreateProcessDeviceIdentityMock, loadSessionEntry: vi.fn((sessionKey: string) => buildSessionLookup(sessionKey)), upsertSessionEntry: vi.fn(), @@ -342,7 +344,7 @@ describe("node exec events", () => { loadOrCreateProcessDeviceIdentityMock.mockClear(); normalizeChannelIdVi.mockClear(); persistInboundImagesForTranscriptMock.mockReset(); - persistInboundImagesForTranscriptMock.mockResolvedValue([]); + persistInboundImagesForTranscriptMock.mockResolvedValue({ entries: [], omission: "none" }); normalizeChannelIdVi.mockImplementation((channel?: string | null) => channel ?? null); updatePairedDevicePresenceMock.mockClear(); updatePairedDevicePresenceMock.mockResolvedValue(true); @@ -1636,7 +1638,7 @@ describe("agent request events", () => { runtimeMocks.resolveSessionModelRef.mockClear(); runtimeMocks.resolveGatewayModelSupportsImages.mockClear(); persistInboundImagesForTranscriptMock.mockReset(); - persistInboundImagesForTranscriptMock.mockResolvedValue([]); + persistInboundImagesForTranscriptMock.mockResolvedValue({ entries: [], omission: "none" }); runtimeMocks.deleteMediaBuffer.mockClear(); upsertSessionEntryMock.mockClear(); loadSessionEntryMock.mockClear(); @@ -1776,14 +1778,18 @@ describe("agent request events", () => { }); it("cleans persisted transcript media when detached agent admission is revoked", async () => { - persistInboundImagesForTranscriptMock.mockResolvedValueOnce([ - { - id: "saved-after-admission", - path: "/media/inbound/saved-after-admission.png", - size: 5, - contentType: "image/png", - }, - ]); + persistInboundImagesForTranscriptMock.mockResolvedValueOnce({ + entries: [ + { + id: "saved-after-admission", + path: "/media/inbound/saved-after-admission.png", + sourceIndex: 0, + imageKind: "inline", + fact: { url: "media://inbound/saved-after-admission.png", contentType: "image/png" }, + }, + ], + omission: "none", + }); let currentnessChecks = 0; const isConnectionCurrent = vi.fn(async () => { currentnessChecks += 1; @@ -1960,7 +1966,7 @@ describe("agent request events", () => { it("passes ordered durable media metadata to the agent transcript recorder", async () => { parseMessageWithAttachmentsMock.mockResolvedValueOnce({ message: "describe\n[media attached: media://inbound/offloaded]", - images: [{ type: "image", data: "aGVsbG8=", mimeType: "image/jpeg" }], + images: [{ type: "image", data: "aGVsbG8=", mimeType: "image/jpeg", sourceIndex: 1 }], imageOrder: ["offloaded", "inline"], offloadedRefs: [ { @@ -1971,23 +1977,29 @@ describe("agent request events", () => { mimeType: "image/png", label: "offloaded.png", sizeBytes: 2_100_000, + sourceIndex: 0, }, ], }); - persistInboundImagesForTranscriptMock.mockResolvedValueOnce([ - { - id: "offloaded", - path: "/media/inbound/offloaded.png", - size: 2_100_000, - contentType: "image/png", - }, - { - id: "saved-inline", - path: "/media/inbound/saved-inline.jpg", - size: 5, - contentType: "image/jpeg", - }, - ]); + persistInboundImagesForTranscriptMock.mockResolvedValueOnce({ + entries: [ + { + id: "offloaded", + path: "/media/inbound/offloaded.png", + sourceIndex: 0, + imageKind: "offloaded", + fact: { url: "media://inbound/offloaded", contentType: "image/png" }, + }, + { + id: "saved-inline", + path: "/media/inbound/saved-inline.jpg", + sourceIndex: 1, + imageKind: "inline", + fact: { url: "media://inbound/saved-inline", contentType: "image/jpeg" }, + }, + ], + omission: "none", + }); await handleNodeEvent(buildCtx(), "node-media", { event: "agent.request", @@ -1998,20 +2010,46 @@ describe("agent request events", () => { }), }); - expect(persistInboundImagesForTranscriptMock).toHaveBeenCalledWith( - expect.objectContaining({ imageOrder: ["offloaded", "inline"] }), - ); + expect(persistInboundImagesForTranscriptMock).toHaveBeenCalledOnce(); expect(agentCommandMock).toHaveBeenCalledTimes(1); expectFields(mockCallArg(agentCommandMock), { message: "describe\n[media attached: media://inbound/offloaded]", transcriptMessage: "describe", transcriptMedia: [ - { path: "/media/inbound/offloaded.png", contentType: "image/png" }, - { path: "/media/inbound/saved-inline.jpg", contentType: "image/jpeg" }, + { url: "media://inbound/offloaded", contentType: "image/png" }, + { url: "media://inbound/saved-inline", contentType: "image/jpeg" }, ], }); }); + it("records a visible durable omission when inline image persistence fails", async () => { + parseMessageWithAttachmentsMock.mockResolvedValueOnce({ + message: "describe", + images: [{ type: "image", data: "aGVsbG8=", mimeType: "image/jpeg", sourceIndex: 0 }], + imageOrder: ["inline"], + offloadedRefs: [], + }); + persistInboundImagesForTranscriptMock.mockResolvedValueOnce({ + entries: [], + omission: "inline-image-save-failed", + }); + + await handleNodeEvent(buildCtx(), "node-media-omission", { + event: "agent.request", + payloadJSON: JSON.stringify({ + message: "describe", + sessionKey: "agent:main:main", + attachments: [{ type: "image", mimeType: "image/jpeg", content: "AAAA" }], + }), + }); + + expectFields(mockCallArg(agentCommandMock), { + message: "describe", + transcriptMessage: + "describe\n[image attachment omitted: durable managed media claim unavailable]", + }); + }); + it("declines non-image attachments cleanly when parse throws UnsupportedAttachmentError", async () => { const warn = vi.fn(); const ctx = buildCtx(); diff --git a/src/gateway/server-node-events.ts b/src/gateway/server-node-events.ts index b433f1cd1f06..73e04b46a867 100644 --- a/src/gateway/server-node-events.ts +++ b/src/gateway/server-node-events.ts @@ -36,6 +36,7 @@ import { enqueueSystemEvent, formatForLog, getRuntimeConfig, + INLINE_IMAGE_DURABLE_OMISSION_MARKER, loadOrCreateProcessDeviceIdentity, loadSessionEntry, normalizeChannelId, @@ -656,11 +657,11 @@ export const handleNodeEvent = async ( } let message = (link?.message ?? "").trim(); - const transcriptMessage = message; + let transcriptMessage = message; const normalizedAttachments = normalizeRpcAttachmentsToChatAttachments( link?.attachments ?? undefined, ); - let images: Array<{ type: "image"; data: string; mimeType: string }> = []; + let images: Awaited>["images"] = []; let imageOrder: PromptImageOrderEntry[] = []; let offloadedRefs: Awaited>["offloadedRefs"] = []; @@ -790,22 +791,23 @@ export const handleNodeEvent = async ( } const persistedTranscriptMedia = await persistInboundImagesForTranscript({ images, - imageOrder, offloadedRefs, log: ctx.logGateway, logContext: "agent.request", }); if (!(await isNodeEventConnectionCurrent(opts))) { await cleanupNodeEventMedia( - persistedTranscriptMedia.map((media) => media.id), + persistedTranscriptMedia.entries.map((media) => media.id), ctx, ); return pairingChangedResult(evt.event); } - const transcriptMedia = persistedTranscriptMedia.map((media) => ({ - path: media.path, - contentType: media.contentType, - })); + if (persistedTranscriptMedia.omission === "inline-image-save-failed") { + transcriptMessage = [transcriptMessage, INLINE_IMAGE_DURABLE_OMISSION_MARKER] + .filter(Boolean) + .join("\n"); + } + const transcriptMedia = persistedTranscriptMedia.entries.map((media) => media.fact); if (wantsReceipt && deliveryChannel && deliveryTo) { // Delivery stays detached from agent startup, but remains part of the @@ -839,7 +841,10 @@ export const handleNodeEvent = async ( message, images, imageOrder, - ...(transcriptMedia.length > 0 ? { transcriptMessage, transcriptMedia } : {}), + ...(transcriptMedia.length > 0 || + persistedTranscriptMedia.omission === "inline-image-save-failed" + ? { transcriptMessage, ...(transcriptMedia.length > 0 ? { transcriptMedia } : {}) } + : {}), sessionId, sessionKey: canonicalKey, thinking: link?.thinking ?? undefined, @@ -854,7 +859,7 @@ export const handleNodeEvent = async ( opts?.isConnectionCurrent, () => cleanupNodeEventMedia( - persistedTranscriptMedia.map((media) => media.id), + persistedTranscriptMedia.entries.map((media) => media.id), ctx, ), ); diff --git a/src/gateway/server.chat.gateway-server-chat-b.test.ts b/src/gateway/server.chat.gateway-server-chat-b.test.ts index 098d6cabe744..5ed70c1e612b 100644 --- a/src/gateway/server.chat.gateway-server-chat-b.test.ts +++ b/src/gateway/server.chat.gateway-server-chat-b.test.ts @@ -5091,6 +5091,21 @@ describe("gateway server chat", () => { text: "inspect mixed attachments", timestamp: Date.now(), media: [ + { + kind: "video", + url: "media://inbound/video-claim", + contentType: "video/mp4", + fileName: "managed-video.mp4", + durationMs: 5678, + }, + { + kind: "image", + url: "media://inbound/image-claim", + contentType: "image/png", + fileName: "managed-image.png", + width: 640, + height: 480, + }, { kind: "image", path: "/private/media/local-image.png", @@ -5109,13 +5124,6 @@ describe("gateway server chat", () => { fileName: "remote-audio.wav", durationMs: 1234, }, - { - kind: "video", - path: "media://inbound/video-claim", - contentType: "video/mp4", - fileName: "managed-video.mp4", - durationMs: 5678, - }, { kind: "document", url: "not a media reference", @@ -5129,10 +5137,11 @@ describe("gateway server chat", () => { fileName: `invalid-claim-${index}.png`, })), ], + mediaImageLayout: { slots: [{ kind: "offloaded", factIndex: 1 }] }, }) as unknown as Record; const metadata = persisted["__openclaw"] as Record; const facts = metadata.media as Array>; - Object.assign(expectDefined(facts[0], "local media fact"), { + Object.assign(expectDefined(facts[2], "local media fact"), { data: "private-inline-data", blob: "private-inline-blob", filePath: "/private/media/alternate-image.png", @@ -5171,7 +5180,23 @@ describe("gateway server chat", () => { content: "inspect mixed attachments", __openclaw: { keepMe: { durable: true }, + mediaImageLayout: { slots: [{ kind: "offloaded", factIndex: 1 }] }, media: [ + { + kind: "video", + url: "media://inbound/video-claim", + contentType: "video/mp4", + fileName: "managed-video.mp4", + durationMs: 5678, + }, + { + kind: "image", + url: "media://inbound/image-claim", + contentType: "image/png", + fileName: "managed-image.png", + width: 640, + height: 480, + }, { kind: "image", contentType: "image/png", @@ -5189,13 +5214,6 @@ describe("gateway server chat", () => { fileName: "remote-audio.wav", durationMs: 1234, }, - { - kind: "video", - path: "media://inbound/video-claim", - contentType: "video/mp4", - fileName: "managed-video.mp4", - durationMs: 5678, - }, { kind: "document", contentType: "application/pdf", @@ -5214,9 +5232,10 @@ describe("gateway server chat", () => { ?.media ?? [] ).map((fact) => fact.path ?? fact.url ?? null); expect(projectedMedia, boundary).toEqual([ + "media://inbound/video-claim", + "media://inbound/image-claim", null, "https://media.example/audio.wav", - "media://inbound/video-claim", ...Array.from({ length: invalidClaims.length + 1 }, () => null), ]); const serialized = JSON.stringify(messages);