From 7b7e40cb0e0ebf792fe325bdcc025d7e33dee77a Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Fri, 19 Jun 2026 02:45:34 +0800 Subject: [PATCH] refactor(gateway): remove duplicate artifact collector --- src/gateway/server-methods/artifacts.test.ts | 194 +++++++++---------- src/gateway/server-methods/artifacts.ts | 17 -- 2 files changed, 96 insertions(+), 115 deletions(-) diff --git a/src/gateway/server-methods/artifacts.test.ts b/src/gateway/server-methods/artifacts.test.ts index 4783e8d2af7..51575a67875 100644 --- a/src/gateway/server-methods/artifacts.test.ts +++ b/src/gateway/server-methods/artifacts.test.ts @@ -2,7 +2,7 @@ // session lookup, list/get/download responses, and validation errors. import { beforeEach, describe, expect, it, vi } from "vitest"; import { expectRecordFields } from "../test-helpers.assertions.js"; -import { artifactsHandlers, collectArtifactsFromMessages } from "./artifacts.js"; +import { artifactsHandlers } from "./artifacts.js"; const hoisted = vi.hoisted(() => ({ getTaskSessionLookupByIdForStatus: vi.fn(), @@ -325,11 +325,9 @@ describe("artifacts RPC handlers", () => { }); it("gets and downloads an inline artifact", async () => { - const listed = collectArtifactsFromMessages({ - sessionKey: "agent:main:main", - messages: [resultImageMessage()], - }); - const artifactId = listed[0]?.id; + const listed = await listArtifacts({ sessionKey: "agent:main:main" }, { id: "list-inline" }); + const listedPayload = expectArtifactList(listed.calls); + const artifactId = listedPayload.artifacts?.[0]?.id; const artifactIdString = requireNonEmptyString(artifactId, "expected listed artifact id"); const get = await getArtifact( @@ -354,37 +352,35 @@ describe("artifacts RPC handlers", () => { expectFields(downloadPayload.artifact, { id: artifactId }); }); - it("can scan artifact summaries without retaining inline data", () => { - const artifacts = collectArtifactsFromMessages({ - sessionKey: "agent:main:main", - includeDownloadData: false, - messages: [ - { - role: "assistant", - content: [ - { - type: "image", - data: "aGVsbG8=", - mimeType: "image/png", - alt: "result.png", - }, - ], - __openclaw: { seq: 2 }, - }, - ], - }); + it("can scan artifact summaries without retaining inline data", async () => { + mockedMessages([ + { + role: "assistant", + content: [ + { + type: "image", + data: "aGVsbG8=", + mimeType: "image/png", + alt: "result.png", + }, + ], + __openclaw: { seq: 2 }, + }, + ]); + const { calls } = await listArtifacts({ sessionKey: "agent:main:main" }); + const artifacts = expectArtifactList(calls).artifacts; expect(artifacts).toHaveLength(1); - expectFields(artifacts[0], { + expectFields(artifacts?.[0], { title: "result.png", mimeType: "image/png", sizeBytes: 5, }); - expectFields(artifacts[0]?.download, { mode: "bytes" }); - expect(artifacts[0]).not.toHaveProperty("data"); + expectFields(artifacts?.[0]?.download, { mode: "bytes" }); + expect(artifacts?.[0]).not.toHaveProperty("data"); }); - it("hydrates inline data only for the requested download artifact", () => { + it("hydrates inline data only for the requested download artifact", async () => { const messages = [ { role: "assistant", @@ -405,23 +401,28 @@ describe("artifacts RPC handlers", () => { __openclaw: { seq: 2 }, }, ]; - const summaries = collectArtifactsFromMessages({ - sessionKey: "agent:main:main", - includeDownloadData: false, - messages, - }); - const secondArtifactId = requireNonEmptyString(summaries[1]?.id, "expected second artifact id"); + mockedMessages(messages); - const hydrated = collectArtifactsFromMessages({ - sessionKey: "agent:main:main", - downloadArtifactId: secondArtifactId, - messages, - }); + const summaries = await listArtifacts({ sessionKey: "agent:main:main" }); + const summaryArtifacts = expectArtifactList(summaries.calls).artifacts; + const secondArtifactId = requireNonEmptyString( + summaryArtifacts?.[1]?.id, + "expected second artifact id", + ); + expect(summaryArtifacts?.[0]).not.toHaveProperty("data"); + expect(summaryArtifacts?.[1]).not.toHaveProperty("data"); - expect(hydrated).toHaveLength(2); - expectFields(hydrated[0], { title: "first.png" }); - expect(hydrated[0]).not.toHaveProperty("data"); - expectFields(hydrated[1], { title: "second.png", data: "c2Vjb25k" }); + const download = await downloadArtifact({ + sessionKey: "agent:main:main", + artifactId: secondArtifactId, + }); + const downloadPayload = expectOkPayload(download.calls) as { + artifact?: Record; + data?: string; + }; + + expectFields(downloadPayload.artifact, { title: "second.png" }); + expectFields(downloadPayload, { data: "c2Vjb25k" }); }); it("resolves runId queries through the gateway run-to-session lookup", async () => { @@ -700,76 +701,73 @@ describe("artifacts RPC handlers", () => { expectFields(artifact?.download, { mode: "bytes" }); }); - it("treats transcript non-base64 data URLs as unsupported downloads", () => { - const artifacts = collectArtifactsFromMessages({ - sessionKey: "agent:main:main", - messages: [ - { - role: "user", - content: [ - { - type: "input_image", - image_url: "data:text/plain,hello", - alt: "uploaded.txt", - }, - ], - __openclaw: { seq: 4 }, - }, - ], - }); + it("treats transcript non-base64 data URLs as unsupported downloads", async () => { + mockedMessages([ + { + role: "user", + content: [ + { + type: "input_image", + image_url: "data:text/plain,hello", + alt: "uploaded.txt", + }, + ], + __openclaw: { seq: 4 }, + }, + ]); + const { calls } = await listArtifacts({ sessionKey: "agent:main:main" }); + const artifacts = expectArtifactList(calls).artifacts; expect(artifacts).toHaveLength(1); - expectFields(artifacts[0], { + expectFields(artifacts?.[0], { type: "image", title: "uploaded.txt", }); - expectFields(artifacts[0]?.download, { mode: "unsupported" }); - expect(artifacts[0]?.download).not.toHaveProperty("encoding", "base64"); + expectFields(artifacts?.[0]?.download, { mode: "unsupported" }); + expect(artifacts?.[0]?.download).not.toHaveProperty("encoding", "base64"); }); - it("treats non-base64 data URLs in the content field as unsupported downloads", () => { - const artifacts = collectArtifactsFromMessages({ - sessionKey: "agent:main:main", - messages: [ - { - role: "assistant", - content: [ - { - type: "file", - content: "data:text/plain,hello", - title: "plain.txt", - }, - ], - __openclaw: { seq: 5 }, - }, - ], - }); + it("treats non-base64 data URLs in the content field as unsupported downloads", async () => { + mockedMessages([ + { + role: "assistant", + content: [ + { + type: "file", + content: "data:text/plain,hello", + title: "plain.txt", + }, + ], + __openclaw: { seq: 5 }, + }, + ]); + const { calls } = await listArtifacts({ sessionKey: "agent:main:main" }); + const artifacts = expectArtifactList(calls).artifacts; expect(artifacts).toHaveLength(1); - expectFields(artifacts[0], { + expectFields(artifacts?.[0], { title: "plain.txt", }); - expectFields(artifacts[0]?.download, { mode: "unsupported" }); - expect(artifacts[0]).not.toHaveProperty("data"); + expectFields(artifacts?.[0]?.download, { mode: "unsupported" }); + expect(artifacts?.[0]).not.toHaveProperty("data"); }); - it("treats unsafe artifact URLs as unsupported downloads", () => { - const artifacts = collectArtifactsFromMessages({ - sessionKey: "agent:main:main", - messages: [ - { - role: "assistant", - content: [{ type: "file", title: "secret.txt", url: "file:///etc/passwd" }], - __openclaw: { seq: 4 }, - }, - ], - }); + it("treats unsafe artifact URLs as unsupported downloads", async () => { + mockedMessages([ + { + role: "assistant", + content: [{ type: "file", title: "secret.txt", url: "file:///etc/passwd" }], + __openclaw: { seq: 4 }, + }, + ]); - expectFields(artifacts[0], { + const { calls } = await listArtifacts({ sessionKey: "agent:main:main" }); + const artifacts = expectArtifactList(calls).artifacts; + expectFields(artifacts?.[0], { title: "secret.txt", }); - expectFields(artifacts[0]?.download, { mode: "unsupported" }); - expect(artifacts[0]).not.toHaveProperty("url"); + expectFields(artifacts?.[0]?.download, { mode: "unsupported" }); + expect(artifacts?.[0]).not.toHaveProperty("url"); }); it("returns typed errors for missing query scope and missing artifacts", async () => { diff --git a/src/gateway/server-methods/artifacts.ts b/src/gateway/server-methods/artifacts.ts index 73d748eaece..e67f8bbe475 100644 --- a/src/gateway/server-methods/artifacts.ts +++ b/src/gateway/server-methods/artifacts.ts @@ -320,23 +320,6 @@ function isArtifactBlock(block: Record): boolean { ); } -export function collectArtifactsFromMessages(params: { - messages: unknown[]; - sessionKey: string; - runId?: string; - taskId?: string; - includeDownloadData?: boolean; - downloadArtifactId?: string; -}): ArtifactRecord[] { - const artifacts: ArtifactRecord[] = []; - let messageFallbackSeq = 0; - for (const message of params.messages) { - messageFallbackSeq += 1; - collectArtifactsFromMessage({ ...params, message, messageFallbackSeq, artifacts }); - } - return artifacts; -} - function collectArtifactsFromMessage(params: { message: unknown; messageFallbackSeq: number;