Fix comment attachment URL resolution (#4816)

Co-authored-by: multica-agent <github@multica.ai>

packages/core/types/attachment-url.test.ts

@@ -67,7 +67,12 @@ describe("attachmentIdFromDownloadURL", () => {
 });
 
 describe("contentReferencesAttachment", () => {
-  const att = { id: ID, url: "/uploads/workspaces/ws/legacy.png" };
+  const att = {
+    id: ID,
+    url: "/uploads/workspaces/ws/legacy.png",
+    download_url: "https://cdn.example.com/workspaces/ws/file.png?Signature=fresh",
+    markdown_url: `https://multica-api.copilothub.ai/api/attachments/${ID}/download`,
+  };
 
   it("matches when the markdown uses the stable download path", () => {
     const md = `body\n\n![file](${attachmentDownloadPath(ID)})\n`;
@@ -79,6 +84,22 @@ describe("contentReferencesAttachment", () => {
     expect(contentReferencesAttachment(md, att)).toBe(true);
   });
 
+  it("matches when the markdown uses the response download_url", () => {
+    const md = `body\n\n![file](${att.download_url})\n`;
+    expect(contentReferencesAttachment(md, att)).toBe(true);
+  });
+
+  it("matches when the markdown uses an older signed download_url for the same object", () => {
+    const stale = "https://cdn.example.com/workspaces/ws/file.png?Signature=stale";
+    const md = `body\n\n![file](${stale})\n`;
+    expect(contentReferencesAttachment(md, att)).toBe(true);
+  });
+
+  it("matches when the markdown uses the server-provided markdown_url", () => {
+    const md = `body\n\n![file](${att.markdown_url})\n`;
+    expect(contentReferencesAttachment(md, att)).toBe(true);
+  });
+
   it("matches when both shapes are present (rollout-window mixed content)", () => {
     const md = `before\n\n![a](${attachmentDownloadPath(ID)})\n\n![b](${att.url})\n`;
     expect(contentReferencesAttachment(md, att)).toBe(true);
@@ -111,8 +132,7 @@ describe("contentReferencesAttachment", () => {
   // absolute-host download URL via its stable-path substring, so the
   // attachment now binds the same way comments do.
   it("matches the absolute-host markdown_url the editor actually persists", () => {
-    const markdownUrl = `https://multica-api.copilothub.ai/api/attachments/${ID}/download`;
-    const md = `body\n\n![pasted](${markdownUrl})\n`;
+    const md = `body\n\n![pasted](${att.markdown_url})\n`;
     // The raw storage url is NOT present in the body — the old
     // `md.includes(a.url)` check would have returned false here.
     expect(md.includes(att.url)).toBe(false);

packages/core/types/attachment-url.ts

@@ -92,6 +92,17 @@ export function attachmentIdFromDownloadURL(rawURL: string): string | undefined
   return id;
 }
 
+function stripQueryAndFragment(url: string): string {
+  return url.split(/[?#]/, 1)[0] ?? "";
+}
+
+function contentReferencesURL(content: string, url?: string): boolean {
+  if (!url) return false;
+  if (content.includes(url)) return true;
+  const stable = stripQueryAndFragment(url);
+  return stable !== "" && content.includes(stable);
+}
+
 /**
  * True when `content` contains a markdown reference to `attachment` —
  * either the new stable `/api/attachments/<id>/download` shape OR the
@@ -109,10 +120,17 @@ export function attachmentIdFromDownloadURL(rawURL: string): string | undefined
  */
 export function contentReferencesAttachment(
   content: string,
-  attachment: { id: string; url: string },
+  attachment: {
+    id: string;
+    url: string;
+    download_url?: string;
+    markdown_url?: string;
+  },
 ): boolean {
   if (!content) return false;
   if (content.includes(attachmentDownloadPath(attachment.id))) return true;
-  if (attachment.url && content.includes(attachment.url)) return true;
+  if (contentReferencesURL(content, attachment.url)) return true;
+  if (contentReferencesURL(content, attachment.download_url)) return true;
+  if (contentReferencesURL(content, attachment.markdown_url)) return true;
   return false;
 }

packages/views/editor/attachment-download-context.tsx

@@ -28,6 +28,18 @@ interface ProviderProps {
   children: ReactNode;
 }
 
+function stripQueryAndFragment(url: string): string {
+  return url.split(/[?#]/, 1)[0] ?? "";
+}
+
+function matchesAttachmentURL(embeddedURL: string, attachmentURL?: string): boolean {
+  if (!embeddedURL || !attachmentURL) return false;
+  if (embeddedURL === attachmentURL) return true;
+  const embeddedStable = stripQueryAndFragment(embeddedURL);
+  const attachmentStable = stripQueryAndFragment(attachmentURL);
+  return embeddedStable !== "" && embeddedStable === attachmentStable;
+}
+
 /**
  * Provides a click-time download handler to Tiptap NodeViews mounted inside
  * `ContentEditor`. Without a provider the consumer falls back to opening the
@@ -60,7 +72,12 @@ export function AttachmentDownloadProvider({ attachments, children }: ProviderPr
         // straight at the CDN, and anything else where
         // `attachments[i].url` was the literal value embedded in
         // markdown.
-        return attachments.find((a) => a.url === url);
+        return attachments.find(
+          (a) =>
+            matchesAttachmentURL(url, a.url) ||
+            matchesAttachmentURL(url, a.download_url) ||
+            matchesAttachmentURL(url, a.markdown_url),
+        );
       };
       return {
         resolveAttachmentId: (url) => lookup(url)?.id,

packages/views/editor/readonly-content.test.tsx

@@ -535,6 +535,31 @@ describe("ReadonlyContent file-card → AttachmentBlock HTML routing", () => {
     expect(container.querySelector("iframe")).toBeNull();
     expect(container.querySelector("img")).toBeNull();
   });
+
+  it("resolves a markdown image whose src is the response download_url", () => {
+    const href = "https://cdn.example.test/shot.png?Signature=stale";
+    const fresh = "https://cdn.example.test/shot.png?Signature=fresh";
+    const attachment = {
+      id: "11111111-2222-3333-4444-555555555555",
+      url: "https://cdn.example.test/shot.png",
+      download_url: fresh,
+      markdown_url: "/api/attachments/11111111-2222-3333-4444-555555555555/download",
+      filename: "shot.png",
+      content_type: "image/png",
+      size_bytes: 1024,
+    } as any;
+
+    const { container } = renderWithQuery(
+      <ReadonlyContent
+        content={`![](${href})`}
+        attachments={[attachment]}
+      />,
+    );
+
+    const img = container.querySelector("img");
+    expect(img?.getAttribute("src")).toBe(fresh);
+    expect(img?.getAttribute("alt")).toBe("shot.png");
+  });
 });
 
 describe("ReadonlyContent slash command rendering", () => {

packages/views/issues/components/comment-card.test.tsx

@@ -109,4 +109,26 @@ describe("AttachmentList — inline attachment filtering", () => {
     expect(screen.queryByText("report.pdf")).toBeNull();
     expect(container.firstChild).toBeNull();
   });
+
+  it("does not render a bottom attachment row when the body already has the response download_url", () => {
+    const href = "https://cdn.example.test/report.pdf?Signature=stale";
+    const attachment = {
+      id: "11111111-2222-3333-4444-555555555555",
+      url: "/uploads/report.pdf",
+      download_url: "https://cdn.example.test/report.pdf?Signature=fresh",
+      filename: "report.pdf",
+      content_type: "application/pdf",
+      size_bytes: 1024,
+    } as any;
+
+    const { container } = renderWithQuery(
+      <AttachmentList
+        attachments={[attachment]}
+        content={`!file[report.pdf](${href})`}
+      />,
+    );
+
+    expect(screen.queryByText("report.pdf")).toBeNull();
+    expect(container.firstChild).toBeNull();
+  });
 });