mirror of
https://github.com/multica-ai/multica.git
synced 2026-07-26 04:25:46 +02:00
fix(attachments): server-driven markdown_url + legacy compat (MUL-3192)
Comment / issue / chat images uploaded inside the Desktop app rendered
as the broken-image fallback. The editor was persisting a site-relative
`/api/attachments/<id>/download` URL into markdown — that path only
resolves when the document origin proxies /api to the API host (apps/web
via Next.js rewrite). On Electron's file:// origin it never resolved.
Per GPT-Boy's plan, move the durable-URL choice from the client to the
server so the persisted shape is correct regardless of which client
performed the upload.
Server:
- AttachmentResponse gains a markdown_url field, computed by
buildMarkdownURL from the deployment policy:
• storage URL is already absolute + unsigned (public CDN, S3 public
bucket, LocalStorage with MULTICA_LOCAL_UPLOAD_BASE_URL on https) →
use it verbatim;
• CloudFront-signed mode → never expose the raw S3 URL (private
bucket); return cfg.PublicURL + /api/attachments/<id>/download so
the server can re-sign on every request;
• LocalStorage relative + cfg.PublicURL set → same prefixed API
endpoint;
• cfg.PublicURL unset → fall back to site-relative path so web's
Next.js rewrite still works.
- isDurablePublicURL helper rejects URLs carrying CloudFront / S3
signature query params, so a freshly-signed download_url can never
leak into persistence — the original MUL-3130 bug stays closed.
Frontend:
- Attachment type + AttachmentResponseSchema (and apps/mobile mirror)
carry markdown_url. Schema lenient-defaults to '' so a backend old
enough to predate this field doesn't break clients.
- useFileUpload picks markdownLink with three-layer fallback:
(1) att.markdown_url (modern server),
(2) attachmentDownloadPath(att.id) — legacy site-relative shape,
retained for backends old enough to omit markdown_url,
(3) att.url — no-workspace avatar branch with no attachment-row id.
- attachment.tsx keeps the relative→absolute absolutize pass, but
reframed as the legacy-compat fallback for already-persisted
/api/attachments/<id>/download or /uploads/<key> URLs in old
bodies. New content writes absolute URLs and skips this path.
- ContentEditor still tracks freshly-uploaded records into
AttachmentDownloadProvider so Quick Create's editor can swap the URL
via the resolver during the same session even before the server-side
binding lands.
Tests:
- server/internal/handler/file_test.go: 5 new buildMarkdownURL matrix
tests (public CDN passthrough, CloudFront-signed swap, relative
prefixing, PublicURL unset fallback, trailing-slash strip) + 15
table-driven isDurablePublicURL cases.
- packages/core/hooks/use-file-upload.test.ts: new file, 4 cases
covering modern server / legacy server / no-id avatar / oversize.
- packages/views/editor/attachment.test.tsx + content-editor.test.tsx:
10 cases for the absolutize matrix and in-session attachment merge.
- 6 existing test fixtures updated to include markdown_url.
Verification: 1236 @multica/views tests pass; 514 @multica/core tests
pass (4 new); server handler package tests pass for the new matrix
plus all pre-existing TestAttachmentToResponse* and TestDownload*
cases. Typecheck green for views/core/web/desktop. Lint clean on
touched files.
Quick Create attachment_ids binding (orphaned attachment relationship
on the resulting issue) is a follow-up — it requires a new --attachment-id
CLI flag and daemon prompt-template work and is intentionally scoped
out of this PR.
Refs: MUL-3192
Co-authored-by: multica-agent <github@multica.ai>
This commit is contained in:
@@ -10,6 +10,7 @@ const {
|
||||
downloadMock,
|
||||
openExternalMock,
|
||||
openByUrlMock,
|
||||
copyTextMock,
|
||||
} = vi.hoisted(() => ({
|
||||
getAttachmentTextContentMock: vi.fn(),
|
||||
// Default: empty base URL so existing tests render site-relative URLs
|
||||
@@ -20,6 +21,7 @@ const {
|
||||
downloadMock: vi.fn(),
|
||||
openExternalMock: vi.fn(),
|
||||
openByUrlMock: vi.fn(),
|
||||
copyTextMock: vi.fn().mockResolvedValue(true),
|
||||
}));
|
||||
|
||||
vi.mock("@multica/core/api", () => ({
|
||||
@@ -31,6 +33,10 @@ vi.mock("@multica/core/api", () => ({
|
||||
PreviewUnsupportedError: class extends Error {},
|
||||
}));
|
||||
|
||||
vi.mock("@multica/ui/lib/clipboard", () => ({
|
||||
copyText: copyTextMock,
|
||||
}));
|
||||
|
||||
vi.mock("./use-download-attachment", () => ({
|
||||
useDownloadAttachment: () => downloadMock,
|
||||
}));
|
||||
@@ -474,3 +480,85 @@ describe("Attachment — absolutize site-relative URLs (MUL-3192)", () => {
|
||||
expect(img?.getAttribute("src")).toBe("/api/attachments/abc-3/download");
|
||||
});
|
||||
});
|
||||
|
||||
// MUL-3192 — "Copy link" copies the rendered <img src>. When the
|
||||
// surrounding AttachmentDownloadProvider has the attachment record in
|
||||
// scope, pickInlineMediaURL swaps to the freshly-signed CloudFront URL
|
||||
// for inline rendering, and copy reflects that. The user's request was
|
||||
// to keep the description and comment surfaces consistent — they now
|
||||
// both copy whatever pickInlineMediaURL chose. The earlier inconsistency
|
||||
// (description copying the API endpoint while comment copied the signed
|
||||
// CDN URL) was caused by `issueAttachments` not being available to the
|
||||
// description's resolver in time; with the resolver populated on both
|
||||
// surfaces (descEditorAttachments + comment.attachments), both surfaces
|
||||
// copy the same shape.
|
||||
describe("Attachment — Copy link copies the rendered src so it stays consistent across surfaces (MUL-3192)", () => {
|
||||
beforeEach(() => {
|
||||
copyTextMock.mockClear();
|
||||
copyTextMock.mockResolvedValue(true);
|
||||
});
|
||||
|
||||
it("copies the freshly-signed CloudFront download_url when the resolver returned a record (comment / description surfaces with attachments resolved)", async () => {
|
||||
const att = makeRecord({
|
||||
url: "https://prod.s3.amazonaws.com/private.png",
|
||||
markdown_url: "https://api.multica.test/api/attachments/att-1/download",
|
||||
download_url:
|
||||
"https://cdn.multica.test/private.png?Signature=fresh&Expires=999",
|
||||
});
|
||||
renderWithQuery(<Attachment attachment={{ kind: "record", attachment: att }} />);
|
||||
|
||||
// Sanity: <img src> is the signed URL — same value the user sees in DOM.
|
||||
const renderedSrc = document.querySelector("img")?.getAttribute("src");
|
||||
expect(renderedSrc).toBe(att.download_url);
|
||||
|
||||
fireEvent.click(screen.getByTitle("Copy link"));
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
expect(copyTextMock).toHaveBeenCalledWith(renderedSrc);
|
||||
});
|
||||
|
||||
it("copies the persisted markdown URL when no resolver context is available (URL-only attachment)", async () => {
|
||||
// No surrounding AttachmentDownloadProvider — the kind=url path
|
||||
// with no resolved record. Without a swap, `<img src>` stays on the
|
||||
// input URL and copy mirrors it. This is the description-surface
|
||||
// behavior before the issueAttachments query has resolved.
|
||||
renderWithQuery(
|
||||
<Attachment
|
||||
attachment={{
|
||||
kind: "url",
|
||||
url: "https://api.multica.test/api/attachments/att-1/download",
|
||||
filename: "shot.png",
|
||||
forceKind: "image",
|
||||
}}
|
||||
/>,
|
||||
);
|
||||
|
||||
const renderedSrc = document.querySelector("img")?.getAttribute("src");
|
||||
fireEvent.click(screen.getByTitle("Copy link"));
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
expect(copyTextMock).toHaveBeenCalledWith(renderedSrc);
|
||||
});
|
||||
|
||||
it("copies the public CDN URL when the record's storage URL is itself a durable public URL (CloudFront public mode)", async () => {
|
||||
// markdown_url == record.url == public CDN URL, download_url is the
|
||||
// bare API endpoint (not signed). pickInlineMediaURL falls through
|
||||
// to record.markdown_url; copy mirrors the rendered src.
|
||||
const att = makeRecord({
|
||||
url: "https://cdn.multica.test/uploads/abc.png",
|
||||
markdown_url: "https://cdn.multica.test/uploads/abc.png",
|
||||
download_url: "/api/attachments/att-1/download",
|
||||
});
|
||||
renderWithQuery(<Attachment attachment={{ kind: "record", attachment: att }} />);
|
||||
|
||||
const renderedSrc = document.querySelector("img")?.getAttribute("src");
|
||||
expect(renderedSrc).toBe("https://cdn.multica.test/uploads/abc.png");
|
||||
|
||||
fireEvent.click(screen.getByTitle("Copy link"));
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
expect(copyTextMock).toHaveBeenCalledWith(
|
||||
"https://cdn.multica.test/uploads/abc.png",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user