fix(gateway): address artifact RPC review blockers

Copilot · BunsDev · web-flow · commit 1ac5dec86719 · 2026-04-30T07:55:35.000Z
Merges remote task-scope fixes with additional tests and SDK hardening:

- Fix taskId resolution via task registry: getTaskById().requesterSessionKey
  with fallback to resolveSessionKeyForRun(task.runId)
- Add messageTaskId support in resolveMessageTaskId
- Fix broken taskId test: use getTaskById mock, messageTaskId field, title not label
- Add assertions: resolveSessionKeyForRun not called, loadSessionEntry with sessionKey
- Add test for non-base64 data: URL in content field (content-field regression guard)
- Make ArtifactsNamespace params required with requireArtifactQueryScope helper
  for fast local SDK feedback instead of runtime Gateway rejection

Co-authored-by: BunsDev &lt;68980965+BunsDev@users.noreply.github.com&gt;
diff --git a/docs/concepts/openclaw-sdk.md b/docs/concepts/openclaw-sdk.md
@@ -224,7 +224,8 @@ await oc.tools.effective({ sessionKey: "main" });
 ```
 
 Artifact helpers expose the Gateway artifact projection for session, run, or
-task context:
+task context. Each call requires one explicit `sessionKey`, `runId`, or
+`taskId` scope:
 
 ```typescript
 const { artifacts } = await oc.artifacts.list({ sessionKey: "main" });
diff --git a/docs/gateway/protocol.md b/docs/gateway/protocol.md
@@ -388,6 +388,7 @@ enumeration of `src/gateway/server-methods/*.ts`.
     - `agents.list` returns configured agent entries, including effective model and runtime metadata.
     - `agents.create`, `agents.update`, and `agents.delete` manage agent records and workspace wiring.
     - `agents.files.list`, `agents.files.get`, and `agents.files.set` manage the bootstrap workspace files exposed for an agent.
+    - `artifacts.list`, `artifacts.get`, and `artifacts.download` expose transcript-derived artifact summaries and downloads for an explicit `sessionKey`, `runId`, or `taskId` scope. Run and task queries resolve the owning session server-side and only return transcript media with matching provenance; unsafe or local URL sources return unsupported downloads instead of fetching server-side.
     - `agent.identity.get` returns the effective assistant identity for an agent or session.
     - `agent.wait` waits for a run to finish and returns the terminal snapshot when available.
 
diff --git a/packages/sdk/src/client.ts b/packages/sdk/src/client.ts
@@ -189,6 +189,20 @@ function asRecord(value: unknown): Record<string, unknown> {
   return typeof value === "object" && value !== null ? (value as Record<string, unknown>) : {};
 }
 
+function hasArtifactQueryScope(params: unknown): params is ArtifactQuery {
+  const record = asRecord(params);
+  return [record.sessionKey, record.runId, record.taskId].some(
+    (value) => typeof value === "string" && value.trim().length > 0,
+  );
+}
+
+function requireArtifactQueryScope(api: string, params: unknown): ArtifactQuery {
+  if (!hasArtifactQueryScope(params)) {
+    throw new Error(`${api} requires one of sessionKey, runId, or taskId`);
+  }
+  return params;
+}
+
 function readChatProjection(event: OpenClawEvent): ChatProjection | undefined {
   const raw = event.raw;
   if (event.type !== "raw" || raw?.event !== "chat") {
@@ -763,15 +777,21 @@ export class ArtifactsNamespace extends RpcNamespace {
   }
 
   async list(params: ArtifactQuery): Promise<ArtifactsListResult> {
-    return await this.call("list", params);
+    return await this.call("list", requireArtifactQueryScope("oc.artifacts.list", params));
   }
 
   async get(id: string, params: ArtifactQuery): Promise<ArtifactsGetResult> {
-    return await this.call("get", { ...asRecord(params), artifactId: id });
+    return await this.call("get", {
+      ...requireArtifactQueryScope("oc.artifacts.get", params),
+      artifactId: id,
+    });
   }
 
   async download(id: string, params: ArtifactQuery): Promise<ArtifactsDownloadResult> {
-    return await this.call("download", { ...asRecord(params), artifactId: id });
+    return await this.call("download", {
+      ...requireArtifactQueryScope("oc.artifacts.download", params),
+      artifactId: id,
+    });
   }
 }
 
diff --git a/packages/sdk/src/index.test.ts b/packages/sdk/src/index.test.ts
@@ -306,6 +306,22 @@ describe("OpenClaw SDK", () => {
     ]);
   });
 
+  it("requires artifact query scope before calling Gateway", async () => {
+    const transport = new FakeTransport({});
+    const oc = new OpenClaw({ transport });
+
+    await expect(oc.artifacts.list(undefined as never)).rejects.toThrow(
+      "oc.artifacts.list requires one of sessionKey, runId, or taskId",
+    );
+    await expect(oc.artifacts.get("artifact_123", undefined as never)).rejects.toThrow(
+      "oc.artifacts.get requires one of sessionKey, runId, or taskId",
+    );
+    await expect(oc.artifacts.download("artifact_123", undefined as never)).rejects.toThrow(
+      "oc.artifacts.download requires one of sessionKey, runId, or taskId",
+    );
+    expect(transport.calls).toEqual([]);
+  });
+
   it("throws explicit unsupported errors for SDK namespaces without Gateway RPCs", async () => {
     const transport = new FakeTransport({});
     const oc = new OpenClaw({ transport });
diff --git a/packages/sdk/src/types.ts b/packages/sdk/src/types.ts
@@ -88,11 +88,10 @@ export type ArtifactSummary = {
   expiresAt?: string;
 };
 
-export type ArtifactQuery = {
-  sessionKey?: string;
-  runId?: string;
-  taskId?: string;
-};
+export type ArtifactQuery =
+  | { sessionKey: string; runId?: string; taskId?: string }
+  | { runId: string; sessionKey?: string; taskId?: string }
+  | { taskId: string; sessionKey?: string; runId?: string };
 
 export type ArtifactsListResult = {
   artifacts: ArtifactSummary[];
diff --git a/src/gateway/server-methods/artifacts.test.ts b/src/gateway/server-methods/artifacts.test.ts
@@ -2,10 +2,14 @@ import { beforeEach, describe, expect, it, vi } from "vitest";
 import { artifactsHandlers, collectArtifactsFromMessages } from "./artifacts.js";
 
 const hoisted = vi.hoisted(() => ({
+  getTaskById: vi.fn(),
   loadSessionEntry: vi.fn(),
   readSessionMessages: vi.fn(),
   resolveSessionKeyForRun: vi.fn(),
-  getTaskById: vi.fn(),
+}));
+
+vi.mock("../../tasks/task-registry.js", () => ({
+  getTaskById: hoisted.getTaskById,
 }));
 
 vi.mock("../session-utils.js", async () => {
@@ -27,16 +31,6 @@ vi.mock("../server-session-key.js", async () => {
   };
 });
 
-vi.mock("../../tasks/task-registry.js", async () => {
-  const actual = await vi.importActual<typeof import("../../tasks/task-registry.js")>(
-    "../../tasks/task-registry.js",
-  );
-  return {
-    ...actual,
-    getTaskById: hoisted.getTaskById,
-  };
-});
-
 function createResponder() {
   const calls: Array<{ ok: boolean; payload?: unknown; error?: unknown }> = [];
   return {
@@ -50,6 +44,7 @@ function createResponder() {
 describe("artifacts RPC handlers", () => {
   beforeEach(() => {
     vi.clearAllMocks();
+    hoisted.getTaskById.mockReturnValue(undefined);
     hoisted.loadSessionEntry.mockReturnValue({
       storePath: "/tmp/sessions.json",
       entry: { sessionId: "sess-main", sessionFile: "/tmp/sess-main.jsonl" },
@@ -167,17 +162,21 @@ describe("artifacts RPC handlers", () => {
   });
 
   it("resolves taskId queries through the task registry and filters artifacts by messageTaskId", async () => {
-    hoisted.getTaskById.mockReturnValue({ requesterSessionKey: "agent:main:main" });
+    hoisted.getTaskById.mockReturnValue({
+      taskId: "task-1",
+      requesterSessionKey: "agent:main:main",
+      runId: "run-for-task-1",
+    });
     hoisted.readSessionMessages.mockReturnValue([
       {
         role: "assistant",
         content: [{ type: "image", data: "dGFyZ2V0", alt: "task-result.png" }],
-        __openclaw: { seq: 2, taskId: "task-1" },
+        __openclaw: { seq: 2, messageTaskId: "task-1" },
       },
       {
         role: "assistant",
         content: [{ type: "image", data: "b3RoZXI=", alt: "other-task.png" }],
-        __openclaw: { seq: 3, taskId: "task-2" },
+        __openclaw: { seq: 3, messageTaskId: "task-2" },
       },
       {
         role: "assistant",
@@ -198,6 +197,8 @@ describe("artifacts RPC handlers", () => {
 
     expect(list.calls[0]?.ok).toBe(true);
     expect(hoisted.getTaskById).toHaveBeenCalledWith("task-1");
+    expect(hoisted.resolveSessionKeyForRun).not.toHaveBeenCalled();
+    expect(hoisted.loadSessionEntry).toHaveBeenCalledWith("agent:main:main");
     const listPayload = list.calls[0]?.payload as { artifacts?: Array<Record<string, unknown>> };
     expect(listPayload.artifacts).toHaveLength(1);
     expect(listPayload.artifacts?.[0]).toMatchObject({
diff --git a/src/gateway/server-methods/artifacts.ts b/src/gateway/server-methods/artifacts.ts
@@ -1,4 +1,5 @@
 import { createHash } from "node:crypto";
+import { getTaskById } from "../../tasks/task-registry.js";
 import {
   ErrorCodes,
   errorShape,
@@ -8,7 +9,6 @@ import {
   validateArtifactsGetParams,
   validateArtifactsListParams,
 } from "../protocol/index.js";
-import { getTaskById } from "../../tasks/task-registry.js";
 import { resolveSessionKeyForRun } from "../server-session-key.js";
 import { loadSessionEntry, readSessionMessages } from "../session-utils.js";
 import type { GatewayRequestHandlers, RespondFn } from "./types.js";
@@ -134,7 +134,12 @@ function resolveMessageRunId(message: Record<string, unknown>): string | undefin
 
 function resolveMessageTaskId(message: Record<string, unknown>): string | undefined {
   const meta = asRecord(message.__openclaw);
-  return asNonEmptyString(meta?.taskId) ?? asNonEmptyString(message.taskId);
+  return (
+    asNonEmptyString(meta?.messageTaskId) ??
+    asNonEmptyString(meta?.taskId) ??
+    asNonEmptyString(message.messageTaskId) ??
+    asNonEmptyString(message.taskId)
+  );
 }
 
 function resolveBlockDownload(block: Record<string, unknown>): {
@@ -274,13 +279,12 @@ function resolveQuerySessionKey(query: ArtifactQuery): string | undefined {
   }
   if (query.taskId) {
     const task = getTaskById(query.taskId);
-    if (task?.requesterSessionKey) {
-      return task.requesterSessionKey;
+    const requesterSessionKey = asNonEmptyString(task?.requesterSessionKey);
+    if (requesterSessionKey) {
+      return requesterSessionKey;
     }
-    if (task?.runId) {
-      return resolveSessionKeyForRun(task.runId);
-    }
-    return undefined;
+    const runId = asNonEmptyString(task?.runId);
+    return runId ? resolveSessionKeyForRun(runId) : undefined;
   }
   return undefined;
 }
