From 5aafabb0aff01127ffd064e44266754e581e5195 Mon Sep 17 00:00:00 2001
From: yushen <ldnvnbl@gmail.com>
Date: Thu, 4 Jun 2026 13:57:37 +0800
Subject: [PATCH] MUL-2976: resolve server-relative attachment download_url on
 Desktop and Mobile

When the backend has no CloudFront signer configured, attachment metadata
returns a server-relative `download_url` like `/api/attachments/{id}/download`.
The CLI already handles relative URLs but the UI clients did not, leaving
the unified-endpoint path failing on Desktop downloads + Mobile Linking +
RN image previews.

Desktop renderer (web + Electron):
- packages/views/editor/use-download-attachment.ts: resolve via
  `resolvePublicFileUrl` from @multica/core/workspace/avatar-url before
  handing to Electron's `downloadURLSafely` bridge or to the web
  placeholder/last-resort window.location redirect.
- packages/views/editor/attachment-preview-modal.tsx: resolve at
  `normalize()` so `<img>`/`<iframe>`/`<video>` srcs are absolute when an
  apiBaseUrl is configured; web with empty base keeps same-origin behaviour.

Mobile (RN):
- apps/mobile/lib/attachment-url.ts: new mirror helper using
  `EXPO_PUBLIC_API_URL` (mobile sharing rule: mirror, don't import core).
- apps/mobile/components/issue/comment-attachment-list.tsx: resolve before
  `Linking.openURL` (RN rejects relative URLs with 'Cannot open URL').
- apps/mobile/lib/markdown/markdown-image.tsx: extend `resolvedUri` memo to
  feed the result through the helper for both `mc://` lookups and pass-through
  paths so the RN image loader receives an absolute URL.

Tests:
- Three new desktop cases in use-download-attachment.test.tsx (relative URL
  resolved against base, base trailing slash trimmed, absolute pass-through).
- New describe block in attachment-preview-modal.test.tsx covering image src,
  iframe src, web same-origin keep-as-is, trailing slash trim, absolute
  pass-through.

Verified: pnpm -C packages/views run typecheck, pnpm -C apps/desktop run
typecheck (node + web), pnpm -C apps/mobile run typecheck, vitest for the
two affected files (40 tests) and the full views suite (1158 tests).

Co-authored-by: multica-agent <github@multica.ai>
---
 .../issue/comment-attachment-list.tsx         |  14 ++-
 apps/mobile/lib/attachment-url.ts             |  44 +++++++
 apps/mobile/lib/markdown/markdown-image.tsx   |  19 ++-
 .../editor/attachment-preview-modal.test.tsx  | 119 +++++++++++++++++-
 .../views/editor/attachment-preview-modal.tsx |  16 ++-
 .../editor/use-download-attachment.test.tsx   |  84 ++++++++++++-
 .../views/editor/use-download-attachment.ts   |  29 ++++-
 7 files changed, 310 insertions(+), 15 deletions(-)
 create mode 100644 apps/mobile/lib/attachment-url.ts

diff --git a/apps/mobile/components/issue/comment-attachment-list.tsx b/apps/mobile/components/issue/comment-attachment-list.tsx
index c6348375d..f48b4b301 100644
--- a/apps/mobile/components/issue/comment-attachment-list.tsx
+++ b/apps/mobile/components/issue/comment-attachment-list.tsx
@@ -26,6 +26,7 @@ import { Linking, Pressable, View } from "react-native";
 import { Ionicons } from "@expo/vector-icons";
 import type { Attachment } from "@multica/core/types";
 import { MarkdownImage } from "@/lib/markdown/markdown-image";
+import { resolveAttachmentUrl } from "@/lib/attachment-url";
 import { useColorScheme } from "@/lib/use-color-scheme";
 import { THEME } from "@/lib/theme";
 import { Text } from "@/components/ui/text";
@@ -108,12 +109,19 @@ function FileCard({
   return (
     <Pressable
       onPress={() => {
-        // download_url is the signed HTTPS link; opening it hands off to
+        // download_url is the canonical link — opening it hands off to
         // Safari which handles auth-token-free download + previewing for
         // common types (PDF, txt). Mirrors what the markdown link renderer
         // does for `[name](url)`.
-        if (attachment.download_url) {
-          void Linking.openURL(attachment.download_url);
+        //
+        // The backend may return a server-relative URL like
+        // `/api/attachments/{id}/download` when no CloudFront signer is
+        // configured (MUL-2976). RN's `Linking.openURL` requires an
+        // absolute http(s) URL — it returns "Cannot open URL" otherwise —
+        // so resolve against `EXPO_PUBLIC_API_URL` first.
+        const target = resolveAttachmentUrl(attachment.download_url);
+        if (target) {
+          void Linking.openURL(target);
         }
       }}
       accessibilityRole="button"
diff --git a/apps/mobile/lib/attachment-url.ts b/apps/mobile/lib/attachment-url.ts
new file mode 100644
index 000000000..c22f33fad
--- /dev/null
+++ b/apps/mobile/lib/attachment-url.ts
@@ -0,0 +1,44 @@
+/**
+ * Resolve a server-relative attachment URL against the configured API base.
+ *
+ * Background: when the backend has no CloudFront signer configured (e.g.
+ * the self-hosted RustFS / private-S3 case in MUL-2976), `attachment.url`
+ * and `attachment.download_url` come back as server-relative paths like
+ * `/api/attachments/{id}/download`. Web is happy with that — same-origin
+ * `<img src="/api/...">` resolves against the document base — but RN
+ * needs an absolute http(s) URL for both `Linking.openURL` (`Cannot open
+ * URL` otherwise) and `<Image source={{ uri }}>` (no document origin to
+ * resolve against; the request is silently dropped).
+ *
+ * Mirrors `packages/core/workspace/avatar-url.ts:resolvePublicFileUrl`
+ * exactly. We don't import the core helper because its `getBaseUrl()`
+ * pulls from a singleton ApiClient that lives in `@multica/core/api` —
+ * not on the mobile sharing whitelist (apps/mobile/CLAUDE.md "mirror,
+ * don't import"). Mobile reads its own `EXPO_PUBLIC_API_URL` from the
+ * Expo env, the same value the rest of `data/api.ts` uses.
+ *
+ * Contract:
+ *   - null / undefined / "" → null (caller should treat as "no URL").
+ *   - already-absolute URL  → returned unchanged.
+ *   - server-relative path  → API base + path, with a single boundary
+ *                             slash (we trim trailing slashes from the
+ *                             base before joining).
+ */
+
+const API_URL = process.env.EXPO_PUBLIC_API_URL ?? "";
+
+export function resolveAttachmentUrlWithBase(
+  rawUrl: string | null | undefined,
+  baseUrl: string,
+): string | null {
+  if (!rawUrl) return null;
+  if (!rawUrl.startsWith("/")) return rawUrl;
+  const trimmedBaseUrl = baseUrl.replace(/\/+$/, "");
+  return `${trimmedBaseUrl}${rawUrl}`;
+}
+
+export function resolveAttachmentUrl(
+  rawUrl: string | null | undefined,
+): string | null {
+  return resolveAttachmentUrlWithBase(rawUrl, API_URL);
+}
diff --git a/apps/mobile/lib/markdown/markdown-image.tsx b/apps/mobile/lib/markdown/markdown-image.tsx
index abdf3ab07..c13746781 100644
--- a/apps/mobile/lib/markdown/markdown-image.tsx
+++ b/apps/mobile/lib/markdown/markdown-image.tsx
@@ -28,6 +28,7 @@ import { useEffect, useMemo, useState } from "react";
 import { Image as RNImage, Pressable, View } from "react-native";
 import { Image as ExpoImage } from "expo-image";
 import type { Attachment } from "@multica/core/types";
+import { resolveAttachmentUrl } from "@/lib/attachment-url";
 import { useLightbox } from "./lightbox-provider";
 
 interface Props {
@@ -41,9 +42,21 @@ export function MarkdownImage({ uri, attachments }: Props) {
   const [aspect, setAspect] = useState<number | null>(null);
 
   const resolvedUri = useMemo(() => {
-    if (!attachments || attachments.length === 0) return uri;
-    const match = attachments.find((a) => a.url === uri);
-    return match?.download_url || uri;
+    // mc://file/<id> → look up the matching attachment's download_url.
+    // No match (external link, html https URL, or unresolved mc://) falls
+    // through to the original uri.
+    let candidate: string | null | undefined = uri;
+    if (attachments && attachments.length > 0) {
+      const match = attachments.find((a) => a.url === uri);
+      if (match?.download_url) candidate = match.download_url;
+    }
+    // The backend may return a server-relative `download_url` (e.g.
+    // `/api/attachments/{id}/download`) when no CloudFront signer is
+    // configured — see MUL-2976. RN's image loader has no document
+    // origin to resolve against, so prepend `EXPO_PUBLIC_API_URL` for
+    // server-relative paths and let absolute URLs / external links pass
+    // through unchanged.
+    return resolveAttachmentUrl(candidate) ?? uri;
   }, [uri, attachments]);
 
   useEffect(() => {
diff --git a/packages/views/editor/attachment-preview-modal.test.tsx b/packages/views/editor/attachment-preview-modal.test.tsx
index 43ffd9917..4a87ec692 100644
--- a/packages/views/editor/attachment-preview-modal.test.tsx
+++ b/packages/views/editor/attachment-preview-modal.test.tsx
@@ -17,6 +17,7 @@ vi.mock("../platform", () => ({
 const {
   getAttachmentTextContentMock,
   downloadMock,
+  getBaseUrlMock,
   FakePreviewTooLargeError,
   FakePreviewUnsupportedError,
 } = vi.hoisted(() => {
@@ -35,13 +36,19 @@ const {
   return {
     getAttachmentTextContentMock: vi.fn(),
     downloadMock: vi.fn(),
+    // Default to the web shape (empty base, same-origin). Tests covering
+    // the desktop-renderer / standalone-shell case override per-test.
+    getBaseUrlMock: vi.fn(() => ""),
     FakePreviewTooLargeError,
     FakePreviewUnsupportedError,
   };
 });
 
 vi.mock("@multica/core/api", () => ({
-  api: { getAttachmentTextContent: getAttachmentTextContentMock },
+  api: {
+    getAttachmentTextContent: getAttachmentTextContentMock,
+    getBaseUrl: getBaseUrlMock,
+  },
   PreviewTooLargeError: FakePreviewTooLargeError,
   PreviewUnsupportedError: FakePreviewUnsupportedError,
 }));
@@ -147,6 +154,9 @@ beforeEach(() => {
   vi.clearAllMocks();
   navState.hasOpenInNewTab = true;
   slugState.value = "acme";
+  // Default to web's same-origin empty base so existing absolute-URL tests
+  // remain unaffected by the relative-URL resolution added in normalize().
+  getBaseUrlMock.mockReturnValue("");
 });
 
 afterEach(() => {
@@ -260,6 +270,113 @@ describe("AttachmentPreviewModal — dispatch", () => {
   });
 });
 
+describe("AttachmentPreviewModal — server-relative download_url resolution (MUL-2976)", () => {
+  // The unified `/api/attachments/{id}/download` endpoint returns a
+  // server-relative path on non-CloudFront deployments. The web app keeps
+  // working same-origin because `apiBaseUrl=""`, but the desktop renderer
+  // is loaded from `app://` / file: / dev-server origin and needs the
+  // absolute URL — otherwise `<img src>`, `<iframe src>`, `<video src>`
+  // hit the shell origin and fail.
+  it("prefixes the configured API base for image previews when download_url is server-relative", () => {
+    getBaseUrlMock.mockReturnValue("https://api.example.test");
+    const att = makeAttachment({
+      filename: "shot.png",
+      content_type: "image/png",
+      download_url: "/api/attachments/att-1/download",
+    });
+    render(
+      <AttachmentPreviewModal
+        source={{ kind: "full", attachment: att }}
+        open
+        onClose={() => {}}
+      />,
+    );
+    const img = document.querySelector("img");
+    expect(img?.getAttribute("src")).toBe(
+      "https://api.example.test/api/attachments/att-1/download",
+    );
+  });
+
+  it("prefixes the configured API base for PDF previews when download_url is server-relative", () => {
+    getBaseUrlMock.mockReturnValue("https://api.example.test");
+    const att = makeAttachment({
+      filename: "manual.pdf",
+      content_type: "application/pdf",
+      download_url: "/api/attachments/att-1/download",
+    });
+    render(
+      <AttachmentPreviewModal
+        source={{ kind: "full", attachment: att }}
+        open
+        onClose={() => {}}
+      />,
+    );
+    const iframe = document.querySelector("iframe");
+    expect(iframe?.getAttribute("src")).toBe(
+      "https://api.example.test/api/attachments/att-1/download",
+    );
+  });
+
+  it("keeps a same-origin relative URL untouched when the configured base is empty (web)", () => {
+    // Default web shape — empty base. Browser resolves the relative path
+    // against the document origin, no prefix needed.
+    const att = makeAttachment({
+      filename: "shot.png",
+      content_type: "image/png",
+      download_url: "/api/attachments/att-1/download",
+    });
+    render(
+      <AttachmentPreviewModal
+        source={{ kind: "full", attachment: att }}
+        open
+        onClose={() => {}}
+      />,
+    );
+    const img = document.querySelector("img");
+    expect(img?.getAttribute("src")).toBe("/api/attachments/att-1/download");
+  });
+
+  it("trims a trailing slash on the configured base when joining a relative URL", () => {
+    getBaseUrlMock.mockReturnValue("https://api.example.test/");
+    const att = makeAttachment({
+      filename: "shot.png",
+      content_type: "image/png",
+      download_url: "/api/attachments/att-1/download",
+    });
+    render(
+      <AttachmentPreviewModal
+        source={{ kind: "full", attachment: att }}
+        open
+        onClose={() => {}}
+      />,
+    );
+    const img = document.querySelector("img");
+    expect(img?.getAttribute("src")).toBe(
+      "https://api.example.test/api/attachments/att-1/download",
+    );
+  });
+
+  it("passes an already-absolute CloudFront/presigned download_url through unchanged", () => {
+    getBaseUrlMock.mockReturnValue("https://api.example.test");
+    const att = makeAttachment({
+      filename: "shot.png",
+      content_type: "image/png",
+      download_url: "https://cdn.example.test/att-1.png?Signature=s",
+    });
+    render(
+      <AttachmentPreviewModal
+        source={{ kind: "full", attachment: att }}
+        open
+        onClose={() => {}}
+      />,
+    );
+    const img = document.querySelector("img");
+    expect(img?.getAttribute("src")).toBe(
+      "https://cdn.example.test/att-1.png?Signature=s",
+    );
+  });
+});
+
 describe("AttachmentPreviewModal — error states", () => {
   it("shows the too-large fallback on PreviewTooLargeError", async () => {
     getAttachmentTextContentMock.mockRejectedValueOnce(new FakePreviewTooLargeError());
diff --git a/packages/views/editor/attachment-preview-modal.tsx b/packages/views/editor/attachment-preview-modal.tsx
index f1a18e7cc..f304e4bbf 100644
--- a/packages/views/editor/attachment-preview-modal.tsx
+++ b/packages/views/editor/attachment-preview-modal.tsx
@@ -45,6 +45,7 @@ import {
 import { Download, ExternalLink, FileText, Loader2, X } from "lucide-react";
 import type { Attachment } from "@multica/core/types";
 import { paths, useWorkspaceSlug } from "@multica/core/paths";
+import { resolvePublicFileUrl } from "@multica/core/workspace/avatar-url";
 import { useT } from "../i18n";
 import { useNavigation } from "../navigation";
 import { openExternal } from "../platform";
@@ -92,18 +93,29 @@ interface PreviewState {
 }
 
 function normalize(source: PreviewSource): PreviewState {
+  // Resolve any server-relative URL (e.g. `/api/attachments/{id}/download`
+  // returned by the unified-endpoint metadata path when no CloudFront
+  // signer is configured) against the configured API base. Web with the
+  // default empty base keeps the relative path and resolves it against
+  // the page origin — same behaviour as before this PR. Desktop renderer
+  // (loaded from `app://` / file: / dev-server origin) needs the absolute
+  // form so `<img src>` / `<iframe src>` / `<video src>` actually point at
+  // the API server instead of the shell origin.
   if (source.kind === "full") {
+    const mediaUrl =
+      resolvePublicFileUrl(source.attachment.download_url) ??
+      source.attachment.download_url;
     return {
       filename: source.attachment.filename,
       contentType: source.attachment.content_type,
-      mediaUrl: source.attachment.download_url,
+      mediaUrl,
       attachmentId: source.attachment.id,
     };
   }
   return {
     filename: source.filename,
     contentType: "",
-    mediaUrl: source.url,
+    mediaUrl: resolvePublicFileUrl(source.url) ?? source.url,
     attachmentId: null,
   };
 }
diff --git a/packages/views/editor/use-download-attachment.test.tsx b/packages/views/editor/use-download-attachment.test.tsx
index 4ac5e828f..649ecb781 100644
--- a/packages/views/editor/use-download-attachment.test.tsx
+++ b/packages/views/editor/use-download-attachment.test.tsx
@@ -4,9 +4,10 @@ import { act, renderHook, waitFor } from "@testing-library/react";
 // Hoisted mock for the API singleton: vi.mock factories cannot reference
 // outside-of-scope vars, but vi.hoisted runs before the import graph.
 const getAttachmentMock = vi.hoisted(() => vi.fn());
+const getBaseUrlMock = vi.hoisted(() => vi.fn(() => ""));
 
 vi.mock("@multica/core/api", () => ({
-  api: { getAttachment: getAttachmentMock },
+  api: { getAttachment: getAttachmentMock, getBaseUrl: getBaseUrlMock },
 }));
 
 vi.mock("sonner", () => ({
@@ -25,6 +26,10 @@ const SIGNED_URL =
 
 beforeEach(() => {
   vi.clearAllMocks();
+  // Default: web with same-origin API (empty base). Each test that needs
+  // a non-empty base (desktop standalone, server-relative download URL)
+  // overrides via getBaseUrlMock.mockReturnValue(...).
+  getBaseUrlMock.mockReturnValue("");
 });
 
 afterEach(() => {
@@ -118,4 +123,81 @@ describe("useDownloadAttachment (desktop)", () => {
     expect(downloadURL).not.toHaveBeenCalled();
     await waitFor(() => expect(toast.error).toHaveBeenCalled());
   });
+
+  // MUL-2976: when the backend has no CloudFront signer, `getAttachment`
+  // returns a server-relative `download_url` like `/api/attachments/.../download`.
+  // The Electron main-process `downloadURLSafely` requires a parsable
+  // http(s) URL or it drops the request — so the renderer must resolve
+  // the path against the configured API base before crossing the bridge.
+  it("resolves a server-relative download_url against the API base before handing it to the desktop bridge", async () => {
+    const downloadURL = vi.fn();
+    (window as unknown as { desktopAPI: { downloadURL: typeof downloadURL } }).desktopAPI = {
+      downloadURL,
+    };
+    getBaseUrlMock.mockReturnValue("https://api.example.test");
+    getAttachmentMock.mockResolvedValueOnce({
+      id: "att-1",
+      url: "https://static.example.test/file.md",
+      download_url: "/api/attachments/att-1/download",
+      filename: "file.md",
+    });
+
+    const { result } = renderHook(() => useDownloadAttachment());
+
+    await act(async () => {
+      await result.current("att-1");
+    });
+
+    expect(downloadURL).toHaveBeenCalledWith(
+      "https://api.example.test/api/attachments/att-1/download",
+    );
+  });
+
+  it("trims a trailing slash on the API base when resolving a relative download_url", async () => {
+    const downloadURL = vi.fn();
+    (window as unknown as { desktopAPI: { downloadURL: typeof downloadURL } }).desktopAPI = {
+      downloadURL,
+    };
+    getBaseUrlMock.mockReturnValue("https://api.example.test/");
+    getAttachmentMock.mockResolvedValueOnce({
+      id: "att-1",
+      url: "/api/attachments/att-1/content",
+      download_url: "/api/attachments/att-1/download",
+      filename: "file.md",
+    });
+
+    const { result } = renderHook(() => useDownloadAttachment());
+
+    await act(async () => {
+      await result.current("att-1");
+    });
+
+    expect(downloadURL).toHaveBeenCalledWith(
+      "https://api.example.test/api/attachments/att-1/download",
+    );
+  });
+
+  it("passes an already-absolute download_url through unchanged when the bridge is present", async () => {
+    const downloadURL = vi.fn();
+    (window as unknown as { desktopAPI: { downloadURL: typeof downloadURL } }).desktopAPI = {
+      downloadURL,
+    };
+    // Even with a non-empty base configured, a CloudFront signed URL
+    // must not be re-prefixed.
+    getBaseUrlMock.mockReturnValue("https://api.example.test");
+    getAttachmentMock.mockResolvedValueOnce({
+      id: "att-1",
+      url: "https://cdn.example.test/att-1.bin",
+      download_url: SIGNED_URL,
+      filename: "file.md",
+    });
+
+    const { result } = renderHook(() => useDownloadAttachment());
+
+    await act(async () => {
+      await result.current("att-1");
+    });
+
+    expect(downloadURL).toHaveBeenCalledWith(SIGNED_URL);
+  });
 });
diff --git a/packages/views/editor/use-download-attachment.ts b/packages/views/editor/use-download-attachment.ts
index c2dea633f..b82f34877 100644
--- a/packages/views/editor/use-download-attachment.ts
+++ b/packages/views/editor/use-download-attachment.ts
@@ -3,6 +3,7 @@
 import { useCallback } from "react";
 import { toast } from "sonner";
 import { api } from "@multica/core/api";
+import { resolvePublicFileUrl } from "@multica/core/workspace/avatar-url";
 import { useT } from "../i18n";
 
 interface DesktopBridge {
@@ -49,14 +50,23 @@ export function useDownloadAttachment(): (attachmentId: string) => Promise<void>
       if (hasDesktopDownloadBridge()) {
         try {
           const fresh = await api.getAttachment(attachmentId);
-          if (!fresh.download_url) {
+          // Server may return a server-relative `download_url`
+          // (`/api/attachments/{id}/download`) when no CloudFront
+          // signer is configured — the unified download endpoint chooses
+          // CloudFront/presign/proxy at request time. Electron's main-side
+          // `downloadURLSafely` requires `new URL()` to parse to http/https,
+          // so resolve against the configured API base before we cross the
+          // bridge. Absolute URLs (legacy CloudFront / S3 presigned) pass
+          // through unchanged.
+          const downloadUrl = resolvePublicFileUrl(fresh.download_url);
+          if (!downloadUrl) {
             failed();
             return;
           }
           const bridge = (
             window as unknown as { desktopAPI?: DesktopBridge }
           ).desktopAPI;
-          await bridge!.downloadURL!(fresh.download_url);
+          await bridge!.downloadURL!(downloadUrl);
         } catch {
           failed();
         }
@@ -72,17 +82,26 @@ export function useDownloadAttachment(): (attachmentId: string) => Promise<void>
         : null;
       try {
         const fresh = await api.getAttachment(attachmentId);
-        if (!fresh.download_url) {
+        // Same relative-URL handling as desktop above. For web the
+        // `apiBaseUrl` is typically empty (same-origin), so `resolvePublicFileUrl`
+        // returns the path unchanged and the browser resolves it against
+        // the page's origin — the existing same-origin behaviour. When a
+        // non-empty base is configured (e.g. tauri/standalone shells that
+        // bundle the SPA but talk to a remote API), the resolver yields an
+        // absolute URL, which works for both `placeholder.location.href`
+        // and the last-resort `window.location.href` redirect.
+        const downloadUrl = resolvePublicFileUrl(fresh.download_url);
+        if (!downloadUrl) {
           placeholder?.close();
           failed();
           return;
         }
         if (placeholder) {
           placeholder.opener = null;
-          placeholder.location.href = fresh.download_url;
+          placeholder.location.href = downloadUrl;
         } else if (typeof window !== "undefined") {
           // Popup blocked outright — last-resort navigate the current tab.
-          window.location.href = fresh.download_url;
+          window.location.href = downloadUrl;
         }
       } catch {
         placeholder?.close();