fix(message-tool): hydrate structured reply attachments

Fix outbound message actions so structured attachments[] media participates in existing sandbox, local-root, and hydration checks. Single-attachment actions select structured attachments only when no top-level or plugin media source wins, while send collects all structured attachments. Proof: git diff --check; pnpm tsgo:core && pnpm tsgo:test:src; direct selector/hydration probe; autoreview clean.
2026-08-08 02:52:15 +00:00 · 2026-05-26 14:50:32 +01:00
parent ce61d224d8
commit 45feb37b13
4 changed files with 373 additions and 4 deletions
--- a/src/infra/outbound/message-action-params.test.ts
+++ b/src/infra/outbound/message-action-params.test.ts
@@ -199,6 +199,32 @@ describe("message action media helpers", () => {
    }
  });

+  maybeIt("normalizes the selected structured attachment sandbox source", async () => {
+    const sandboxRoot = await fs.mkdtemp(path.join(os.tmpdir(), "msg-params-attachment-"));
+    try {
+      const attachment: Record<string, unknown> = {
+        path: "/workspace/replies/photo.png",
+        mimeType: "image/png",
+        name: "photo.png",
+      };
+      const args: Record<string, unknown> = {
+        attachments: [attachment],
+      };
+
+      await normalizeSandboxMediaParams({
+        args,
+        mediaPolicy: {
+          mode: "sandbox",
+          sandboxRoot,
+        },
+      });
+
+      expect(attachment.path).toBe(path.join(sandboxRoot, "replies", "photo.png"));
+    } finally {
+      await fs.rm(sandboxRoot, { recursive: true, force: true });
+    }
+  });
+
  it("collects host media source hints from the shared media-source key set", () => {
    expect(
      collectActionMediaSourceHints(
@@ -220,6 +246,73 @@ describe("message action media helpers", () => {
    ]);
  });

+  it("collects the selected structured attachment source for host media access", () => {
+    expect(
+      collectActionMediaSourceHints({
+        attachments: [
+          {
+            path: " /workspace/uploads/photo.png ",
+            mimeType: "image/png",
+            name: "photo.png",
+          },
+        ],
+      }),
+    ).toEqual([" /workspace/uploads/photo.png "]);
+  });
+
+  it("does not collect ignored structured attachments when top-level media wins", () => {
+    expect(
+      collectActionMediaSourceHints({
+        media: "https://example.com/top-level.png",
+        attachments: [
+          {
+            path: "/workspace/uploads/ignored.png",
+            mimeType: "image/png",
+            name: "ignored.png",
+          },
+        ],
+      }),
+    ).toEqual(["https://example.com/top-level.png"]);
+  });
+
+  it("does not collect ignored structured attachments when plugin media params win", () => {
+    expect(
+      collectActionMediaSourceHints(
+        {
+          avatarPath: "/workspace/avatars/profile.png",
+          attachments: [
+            {
+              path: "/workspace/uploads/ignored.png",
+              mimeType: "image/png",
+              name: "ignored.png",
+            },
+          ],
+        },
+        matrixMediaSourceParamKeys,
+      ),
+    ).toEqual(["/workspace/avatars/profile.png"]);
+  });
+
+  it("collects every structured attachment source when the send path uses all attachments", () => {
+    expect(
+      collectActionMediaSourceHints(
+        {
+          media: "https://example.com/top-level.png",
+          attachments: [
+            { path: "/workspace/uploads/one.png" },
+            { fileUrl: "/workspace/uploads/two.png" },
+          ],
+        },
+        undefined,
+        { structuredAttachments: "all" },
+      ),
+    ).toEqual([
+      "https://example.com/top-level.png",
+      "/workspace/uploads/one.png",
+      "/workspace/uploads/two.png",
+    ]);
+  });
+
  maybeIt("normalizes extension snake_case avatar_path and avatar_url aliases", async () => {
    const sandboxRoot = await fs.mkdtemp(path.join(os.tmpdir(), "msg-params-avatar-snake-"));
    try {
@@ -430,6 +523,56 @@ describe("message action media helpers", () => {
    expect(args.filename).toBe("cute.png");
  });

+  it("hydrates reply attachments from the first structured attachment source", async () => {
+    const args: Record<string, unknown> = {
+      attachments: [
+        {
+          url: "https://example.com/cute.png",
+          mimeType: "image/png",
+          name: "cute.png",
+        },
+      ],
+    };
+
+    await hydrateAttachmentParamsForAction({
+      cfg,
+      channel: "imessage",
+      args,
+      action: "reply",
+      dryRun: true,
+      mediaPolicy: { mode: "host" },
+    });
+
+    expect(args.filename).toBe("cute.png");
+    expect(args.contentType).toBe("image/png");
+  });
+
+  it("does not hydrate ignored structured attachments when plugin media params win", async () => {
+    const args: Record<string, unknown> = {
+      avatarPath: "/workspace/avatars/profile.png",
+      attachments: [
+        {
+          url: "https://example.com/ignored.png",
+          mimeType: "image/png",
+          name: "ignored.png",
+        },
+      ],
+    };
+
+    await hydrateAttachmentParamsForAction({
+      cfg,
+      channel: "imessage",
+      args,
+      action: "reply",
+      dryRun: true,
+      mediaPolicy: { mode: "host" },
+      extraParamKeys: matrixMediaSourceParamKeys,
+    });
+
+    expect(args.filename).toBe("attachment");
+    expect(args.contentType).toBeUndefined();
+  });
+
  it("does not fall back caption->message on reply (reply has its own text field)", async () => {
    // sendAttachment uses caption as the body text and falls back from
    // message -> caption when the agent only supplied `message`. Reply has
--- a/src/infra/outbound/message-action-params.ts
+++ b/src/infra/outbound/message-action-params.ts
@@ -32,10 +32,35 @@ const BASE_ACTION_MEDIA_SOURCE_PARAM_KEYS = [
  "image",
 ] as const;

+const STRUCTURED_ATTACHMENT_MEDIA_SOURCE_PARAM_KEYS = [
+  "media",
+  "mediaUrl",
+  "path",
+  "filePath",
+  "fileUrl",
+  "url",
+] as const;
+const STRUCTURED_ATTACHMENT_FILE_SOURCE_PARAM_KEYS = new Set(["path", "filePath", "fileUrl"]);
+
+type StructuredAttachmentSource = {
+  attachment: Record<string, unknown>;
+  key: string;
+  value: string;
+  kind: "media" | "file";
+  contentType?: string;
+  filename?: string;
+};
+
+type StructuredAttachmentMode = "selected" | "all";
+
 function readMediaParam(args: Record<string, unknown>, key: string): string | undefined {
  return readStringParam(args, key, { trim: false });
 }

+function isRecord(value: unknown): value is Record<string, unknown> {
+  return Boolean(value && typeof value === "object" && !Array.isArray(value));
+}
+
 function resolveMediaParamEntry(
  args: Record<string, unknown>,
  key: string,
@@ -54,6 +79,61 @@ function resolveMediaParamEntry(
  };
 }

+function hasExplicitAttachmentPayload(
+  args: Record<string, unknown>,
+  extraParamKeys?: readonly string[],
+): boolean {
+  if (readStringParam(args, "buffer", { trim: false })) {
+    return true;
+  }
+  return buildActionMediaSourceParamKeys(extraParamKeys).some((key) => {
+    const entry = resolveMediaParamEntry(args, key);
+    return Boolean(entry && normalizeOptionalString(entry.value));
+  });
+}
+
+function collectStructuredAttachmentSources(
+  args: Record<string, unknown>,
+): StructuredAttachmentSource[] {
+  const attachments = args.attachments;
+  if (!Array.isArray(attachments)) {
+    return [];
+  }
+  const sources: StructuredAttachmentSource[] = [];
+  for (const attachment of attachments) {
+    if (!isRecord(attachment)) {
+      continue;
+    }
+    for (const key of STRUCTURED_ATTACHMENT_MEDIA_SOURCE_PARAM_KEYS) {
+      const entry = resolveMediaParamEntry(attachment, key);
+      if (!entry || !normalizeOptionalString(entry.value)) {
+        continue;
+      }
+      sources.push({
+        attachment,
+        key: entry.key,
+        value: entry.value,
+        kind: STRUCTURED_ATTACHMENT_FILE_SOURCE_PARAM_KEYS.has(key) ? "file" : "media",
+        contentType:
+          readStringParam(attachment, "contentType") ?? readStringParam(attachment, "mimeType"),
+        filename: readStringParam(attachment, "filename") ?? readStringParam(attachment, "name"),
+      });
+      break;
+    }
+  }
+  return sources;
+}
+
+function resolveStructuredAttachmentSource(
+  args: Record<string, unknown>,
+  extraParamKeys?: readonly string[],
+): StructuredAttachmentSource | undefined {
+  if (hasExplicitAttachmentPayload(args, extraParamKeys)) {
+    return undefined;
+  }
+  return collectStructuredAttachmentSources(args)[0];
+}
+
 function buildActionMediaSourceParamKeys(extraParamKeys?: readonly string[]): string[] {
  const keys = new Set<string>(BASE_ACTION_MEDIA_SOURCE_PARAM_KEYS);
  extraParamKeys?.forEach((key) => keys.add(key));
@@ -91,6 +171,7 @@ export function resolveExtraActionMediaSourceParamKeys(params: {
 export function collectActionMediaSourceHints(
  args: Record<string, unknown>,
  extraParamKeys?: readonly string[],
+  options?: { structuredAttachments?: StructuredAttachmentMode },
 ): string[] {
  const sources: string[] = [];
  for (const key of buildActionMediaSourceParamKeys(extraParamKeys)) {
@@ -99,6 +180,14 @@ export function collectActionMediaSourceHints(
      sources.push(entry.value);
    }
  }
+  if (options?.structuredAttachments === "all") {
+    sources.push(...collectStructuredAttachmentSources(args).map((source) => source.value));
+  } else {
+    const attachmentSource = resolveStructuredAttachmentSource(args, extraParamKeys);
+    if (attachmentSource) {
+      sources.push(attachmentSource.value);
+    }
+  }
  return sources;
 }

@@ -306,6 +395,7 @@ export async function normalizeSandboxMediaParams(params: {
  args: Record<string, unknown>;
  mediaPolicy: AttachmentMediaPolicy;
  extraParamKeys?: readonly string[];
+  structuredAttachments?: StructuredAttachmentMode;
 }): Promise<void> {
  const sandboxRoot =
    params.mediaPolicy.mode === "sandbox" ? params.mediaPolicy.sandboxRoot.trim() : undefined;
@@ -323,6 +413,28 @@ export async function normalizeSandboxMediaParams(params: {
      params.args[entry.key] = normalized;
    }
  }
+  const attachmentSources =
+    params.structuredAttachments === "all"
+      ? collectStructuredAttachmentSources(params.args)
+      : [resolveStructuredAttachmentSource(params.args, params.extraParamKeys)].filter(
+          (source): source is StructuredAttachmentSource => Boolean(source),
+        );
+  if (attachmentSources.length === 0) {
+    return;
+  }
+  for (const attachmentSource of attachmentSources) {
+    assertMediaNotDataUrl(attachmentSource.value);
+    if (!sandboxRoot) {
+      continue;
+    }
+    const normalized = await resolveSandboxedMediaSource({
+      media: attachmentSource.value,
+      sandboxRoot,
+    });
+    if (normalized !== attachmentSource.value) {
+      attachmentSource.attachment[attachmentSource.key] = normalized;
+    }
+  }
 }

 export async function normalizeSandboxMediaList(params: {
@@ -360,11 +472,21 @@ async function hydrateAttachmentActionPayload(params: {
  allowMessageCaptionFallback?: boolean;
  mediaPolicy: AttachmentMediaPolicy;
  optimizeImages?: boolean;
+  extraParamKeys?: readonly string[];
 }): Promise<void> {
+  const attachmentSource = resolveStructuredAttachmentSource(params.args, params.extraParamKeys);
  const mediaHint = readAttachmentMediaHint(params.args);
  const fileHint = readAttachmentFileHint(params.args);
  const contentTypeParam =
-    readStringParam(params.args, "contentType") ?? readStringParam(params.args, "mimeType");
+    readStringParam(params.args, "contentType") ??
+    readStringParam(params.args, "mimeType") ??
+    attachmentSource?.contentType;
+  if (attachmentSource?.filename && !readStringParam(params.args, "filename")) {
+    params.args.filename = attachmentSource.filename;
+  }
+  if (attachmentSource?.contentType && !readStringParam(params.args, "contentType")) {
+    params.args.contentType = attachmentSource.contentType;
+  }

  if (params.allowMessageCaptionFallback) {
    const caption = readStringParam(params.args, "caption", { allowEmpty: true })?.trim();
@@ -381,8 +503,9 @@ async function hydrateAttachmentActionPayload(params: {
    args: params.args,
    dryRun: params.dryRun,
    contentTypeParam,
-    mediaHint,
-    fileHint,
+    mediaHint:
+      mediaHint ?? (attachmentSource?.kind === "media" ? attachmentSource.value : undefined),
+    fileHint: fileHint ?? (attachmentSource?.kind === "file" ? attachmentSource.value : undefined),
    mediaPolicy: params.mediaPolicy,
    optimizeImages: params.optimizeImages,
  });
@@ -396,6 +519,7 @@ export async function hydrateAttachmentParamsForAction(params: {
  action: ChannelMessageActionName;
  dryRun?: boolean;
  mediaPolicy: AttachmentMediaPolicy;
+  extraParamKeys?: readonly string[];
 }): Promise<void> {
  const shouldHydrateUploadFile = params.action === "upload-file";
  // Reply gets the same hydration as sendAttachment so threaded sends with
@@ -421,6 +545,7 @@ export async function hydrateAttachmentParamsForAction(params: {
    args: params.args,
    dryRun: params.dryRun,
    mediaPolicy: params.mediaPolicy,
+    extraParamKeys: params.extraParamKeys,
    optimizeImages: shouldHydrateUploadFile && forceDocument ? false : undefined,
    allowMessageCaptionFallback: params.action === "sendAttachment" || shouldHydrateUploadFile,
  });
--- a/src/infra/outbound/message-action-runner.media.test.ts
+++ b/src/infra/outbound/message-action-runner.media.test.ts
@@ -719,6 +719,102 @@ describe("runMessageAction media behavior", () => {
      expect(handlerParams.contentType).toBe("image/png");
    });

+    it("hydrates buffer and metadata from attachments[] before the reply handler runs", async () => {
+      const result = await runMessageAction({
+        cfg,
+        action: "reply",
+        params: {
+          channel: "replychat",
+          target: "+15551234567",
+          messageId: "parent-id",
+          text: "look at this",
+          attachments: [
+            {
+              url: "https://example.com/pic.png",
+              name: "reply.png",
+              mimeType: "image/png",
+            },
+          ],
+        },
+      });
+
+      expect(result.kind).toBe("action");
+      expect(loadWebMedia).toHaveBeenCalledWith("https://example.com/pic.png", expect.any(Object));
+      expect(handleActionMock).toHaveBeenCalledTimes(1);
+      const handlerParams = firstMockArg(handleActionMock, "handleAction");
+      expect(handlerParams.buffer).toBe(Buffer.from("hello").toString("base64"));
+      expect(handlerParams.filename).toBe("reply.png");
+      expect(handlerParams.contentType).toBe("image/png");
+    });
+
+    it("does not copy metadata from attachments[] when top-level media wins", async () => {
+      await runMessageAction({
+        cfg,
+        action: "reply",
+        params: {
+          channel: "replychat",
+          target: "+15551234567",
+          messageId: "parent-id",
+          text: "look at this",
+          media: "https://example.com/pic.png",
+          attachments: [
+            {
+              url: "https://example.com/ignored.pdf",
+              name: "ignored.pdf",
+              mimeType: "application/pdf",
+            },
+          ],
+        },
+      });
+
+      expect(loadWebMedia).toHaveBeenCalledWith("https://example.com/pic.png", expect.any(Object));
+      const handlerParams = firstMockArg(handleActionMock, "handleAction");
+      expect(handlerParams.filename).toBe("pic.png");
+      expect(handlerParams.contentType).toBe("image/png");
+    });
+
+    it("routes attachments[] host paths into local-root expansion", async () => {
+      const actual = await vi.importActual<typeof import("../../media/web-media.js")>(
+        "../../media/web-media.js",
+      );
+      vi.mocked(loadWebMedia).mockImplementation(actual.loadWebMedia);
+
+      const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "msg-reply-attachment-path-"));
+      try {
+        const attachmentPath = path.join(tempDir, "photo.png");
+        await fs.writeFile(attachmentPath, onePixelPng);
+
+        const result = await runMessageAction({
+          cfg: {
+            ...cfg,
+            tools: { fs: { workspaceOnly: false } },
+          },
+          action: "reply",
+          params: {
+            channel: "replychat",
+            target: "+15551234567",
+            messageId: "parent-id",
+            text: "look at this",
+            attachments: [
+              {
+                path: attachmentPath,
+                name: "photo.png",
+                mimeType: "image/png",
+              },
+            ],
+          },
+        });
+
+        expect(result.kind).toBe("action");
+        const handlerParams = firstMockArg(handleActionMock, "handleAction");
+        expect(handlerParams.filename).toBe("photo.png");
+        expect(handlerParams.contentType).toBe("image/png");
+        expect(typeof handlerParams.buffer).toBe("string");
+      } finally {
+        await fs.rm(tempDir, { recursive: true, force: true });
+      }
+    });
+
    it("rejects host paths outside mediaLocalRoots before invoking the reply handler", async () => {
      // Use the real loader so its localRoots/workspaceOnly enforcement runs.
      const actual = await vi.importActual<typeof import("../../media/web-media.js")>(
--- a/src/infra/outbound/message-action-runner.ts
+++ b/src/infra/outbound/message-action-runner.ts
@@ -1355,17 +1355,21 @@ export async function runMessageAction(
    requesterSenderId: input.requesterSenderId,
    senderIsOwner: input.senderIsOwner,
  });
+  const structuredAttachmentMode = action === "send" ? "all" : "selected";

  await normalizeSandboxMediaParams({
    args: params,
    mediaPolicy: normalizationPolicy,
    extraParamKeys: extraActionMediaSourceParamKeys,
+    structuredAttachments: structuredAttachmentMode,
  });

  const mediaAccess = resolveAgentScopedOutboundMediaAccess({
    cfg,
    agentId: resolvedAgentId,
-    mediaSources: collectActionMediaSourceHints(params, extraActionMediaSourceParamKeys),
+    mediaSources: collectActionMediaSourceHints(params, extraActionMediaSourceParamKeys, {
+      structuredAttachments: structuredAttachmentMode,
+    }),
    sessionKey: input.sessionKey,
    messageProvider: input.sessionKey ? undefined : channel,
    accountId: input.sessionKey ? (input.requesterAccountId ?? accountId) : accountId,
@@ -1387,6 +1391,7 @@ export async function runMessageAction(
    action,
    dryRun,
    mediaPolicy,
+    extraParamKeys: extraActionMediaSourceParamKeys,
  });

  const resolvedTarget = await resolveActionTarget({