Commit Graph

13 Commits

Author SHA1 Message Date
Bohan Jiang
658e63d9be fix: prefer local upload attachment URLs (#4686)
Co-authored-by: J <j@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>
2026-06-29 14:49:04 +08:00
Multica Eve
9a7eebb194 fix: re-sign unresolved attachment media urls (#4132)
Co-authored-by: Eve <eve@multica-ai.local>
Co-authored-by: multica-agent <github@multica.ai>
2026-06-15 14:09:25 +08:00
Bohan Jiang
6c17771cce fix: re-sign inline attachment media for token-mode clients (#4085)
The two prior MUL-3254 fixes preserved draft/description state across a
modal close, but Desktop still could not RENDER the reopened image: in
CloudFront signed-URL mode every URL the renderer holds after reopen is
unloadable. The persisted record strips the expired signed download_url,
the raw CDN url is unsigned (403 on a signed distribution), and the
durable /api/attachments/<id>/download endpoint needs credentials that a
cross-site file:// <img> fetch cannot carry (web works via the same-site
session cookie, which is why the bug was desktop-only).

Two changes close the last mile:

- /api/config now reports cdn_signed when CloudFront signing is enabled,
  and pickInlineMediaURL stops picking the raw (unsigned) CDN url in
  that mode — it is a guaranteed 403.
- The Attachment renderer upgrades an auth-gated media URL to a freshly
  signed one via authenticated GET /api/attachments/<id> (the same
  re-sign the click-time download path already does), but only on
  clients without a same-origin /api proxy (api.getBaseUrl() non-empty:
  Desktop, mobile webview). Cached via TanStack Query with a 20-minute
  staleTime, inside the server's 30-minute signed-URL TTL.

Old servers omit cdn_signed; the schema defaults it to false so behavior
is unchanged there. Non-CloudFront deployments return the API path again
from the metadata fetch and the renderer keeps the original URL.

Co-authored-by: J <j@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>
2026-06-15 13:54:36 +08:00
Bohan Jiang
fa15041864 MUL-3254: fix pasted image draft rendering in desktop (#4066)
* fix: keep issue draft attachment records

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

* fix: avoid persisting signed draft attachment urls

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

* fix: reuse resolved media url for draft previews

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

* fix: address draft attachment review nits

- Backfill an empty caller download_url from the in-session upload on id
  collision so a just-pasted image first-paints from the signed URL
  instead of detouring through markdown_url.
- Prune draft attachments no longer referenced by the persisted
  description when the create dialog reopens.
- Backfill EMPTY_DRAFT defaults on draft-store rehydrate so drafts
  persisted before the attachments field existed get a stable shape.

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

---------

Co-authored-by: J <j@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>
2026-06-13 01:25:08 +08:00
LinYushen
2754b7d7d8 fix(attachments): render description images with CDN URL (#4005) 2026-06-10 17:26:13 +08:00
Multica Eve
abf99eb700 fix(attachments): server-driven markdown_url + legacy compat (MUL-3192) (#3991)
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: Eve <eve@multica-ai.local>
Co-authored-by: multica-agent <github@multica.ai>
2026-06-10 16:00:40 +08:00
Multica Eve
13e9485a3b MUL-3130: persist stable /api/attachments/<id>/download URL in comment markdown (#3937)
* MUL-3130: persist a stable attachment download URL in comment markdown

Comment image attachments rendered as broken placeholders ~30 minutes
after upload because the editor was persisting a short-lived
HMAC-signed URL into the comment body. After PR #3903 (MUL-3132)
hardened /uploads/* with auth, `attachmentToResponse` started signing
`attachment.url` as `/uploads/<key>?exp=<unix>&sig=<HMAC>` for
LocalStorage so token-auth clients could keep loading inline images.
The signature has a 30-min TTL by design — but `useFileUpload` was
returning that signed value as `link` and the editor was writing
`![file](signed-url)` straight into the markdown, so the comment
permanently captured a URL that stopped working as soon as the
signature expired.

The fix is to persist a stable per-attachment URL that the server can
re-sign on every request:

* `useFileUpload` now returns `link = /api/attachments/<id>/download`
  (avatar uploads without an id still fall back to `att.url` so the
  pre-attachment-row code paths keep working).
* `DownloadAttachment` self-resolves the workspace from the attachment
  row instead of reading X-Workspace-Slug / X-Workspace-ID headers,
  and the route is registered under the auth-only group so a native
  browser <img>/<video> resource load (which cannot attach those
  headers) succeeds. Membership is checked inside the handler with
  a 404 deny shape so the route does not act as an IDOR oracle.
* A new `GetAttachmentByIDOnly` SQL query supports the workspace-
  derivation step.
* `AttachmentDownloadProvider` now extracts the attachment id from
  the stable URL when matching markdown refs to attachment records,
  with a fallback to the existing url-equality check for legacy
  comments (and S3/CloudFront markdown that points straight at the
  CDN).
* `contentReferencesAttachment` covers both URL shapes for the
  composer / standalone-list dedup paths so an attachment uploaded
  before the fix and one uploaded after both deduplicate cleanly.

Tests:
- New unit tests for the URL helpers (16 tests, packages/core).
- Backend regression test: bare `<img src>`-style request without
  workspace headers now succeeds for a member (200) and 404s for a
  non-member, replacing the previous "400 without workspace context"
  contract.
- Existing TestDownload*, TestServeLocalUpload*, TestAttachmentTo
  Response* and the 1220 frontend views tests all pass.

Refs: MUL-3130, GitHub issue #3891
Co-authored-by: multica-agent <github@multica.ai>

* MUL-3130: address PR review — split markdown link from upload link, swap render src

Two follow-ups from GPT-Boy's review on PR #3937.

(1) Don't reroute every upload consumer through the workspace-gated
    download endpoint.

    The previous change made `useFileUpload`'s `link` field unconditionally
    return `/api/attachments/<id>/download` whenever the upload had an id.
    But `useFileUpload` is also used by avatar / logo pickers
    (account-tab, workspace-tab, agents/avatar-picker, squads/squad-detail-page)
    that persist `result.link` directly into `avatar_url`. Avatars are
    referenced cross-workspace (mention chips, member lists, inbox
    items), so binding their URL to a workspace-membership-gated
    endpoint would silently break cross-workspace avatar visibility.

    The fix splits the URL into two semantically distinct fields:

      - `link`         — same as `att.url` (legacy contract). Avatar /
                          logo callers continue to use this and remain
                          on whatever URL semantics the storage backend
                          dictates.
      - `markdownLink` — the stable per-attachment URL
                          `/api/attachments/<id>/download`. Only the
                          editor's markdown-persisting flow consumes
                          this. Falls back to `link` for the
                          no-workspace upload branch (where there is
                          no attachment-row id to address).

    `editor/extensions/file-upload.ts` switches `image.src` and
    `fileCard.href` to `markdownLink ?? link` so comment markdown gets
    the stable shape while avatar callers stay on `link` unchanged.

(2) Make the render-time img src loadable for token-mode clients.

    Persisting the stable `/api/attachments/<id>/download` URL fixes the
    expiry problem but the path itself sits behind `middleware.Auth`,
    which expects either a `multica_auth` cookie or a Bearer token in
    `Authorization`. Native `<img>`/`<video>` resource loads from
    token-mode clients (Electron's default mode, the mobile app,
    legacy-token web sessions) cannot attach the Authorization header,
    so the bare URL would 401 immediately rather than 30 minutes later.

    `Attachment.normalize` now runs the resolved record through a new
    `pickInlineMediaURL` helper that returns:

      - `record.download_url` when it's an absolute URL with a
         recognised CDN signature query (CloudFront-signed
         `Signature` / `Expires` / `Key-Pair-Id`, or
         `X-Amz-Signature` for raw S3 presigns) — these load as
         native resource src in any client.
      - else `record.url`, which on the LocalStorage backend carries
         a freshly-minted `/uploads/<key>?exp&sig` query whose
         signature IS the auth (token-mode-loadable). On non-CF S3
         backends this is the raw stored URL — same behaviour as
         today.
      - else the original input URL (legacy / unresolved markdown
         keeps its existing path).

    This gives the same effect for both `kind: "record"` and
    `kind: "url"` attachment inputs: once a record is in hand, the
    rendered media src is whichever URL the current backend exposes
    a working signature on.

Tests:

  - New `file-upload.test.ts` regression pinning that `markdownLink`
    is what lands in the markdown body when the upload result returns
    both a short-lived storage URL and a stable download path.
  - Updated `attachment.test.tsx` to reflect the new render-time
    swap (the rendered img src now follows the freshly signed URL,
    not the raw storage URL) and added a record-mode regression
    pinning the LocalStorage default — when `download_url` is the
    bare /api/attachments/<id>/download path, the renderer must fall
    through to the signed `record.url`.
  - Updated `chat-input.test.tsx` makeUpload helper for the new
    `markdownLink` UploadResult field.
  - 1222 frontend views tests + 507 core tests + typecheck across
    @multica/{core,ui,views} all pass.

Refs: MUL-3130, GitHub issue #3891. Builds on a740f7a35.
Co-authored-by: multica-agent <github@multica.ai>

* MUL-3130: chat upload map keys on persisted markdownLink, not the short-lived link

GPT-Boy's second-round review on PR #3937 caught a chat-only blocker
left over from the previous fix.

After the previous commit split `UploadResult.link` into `link`
(legacy avatar/logo URL) and `markdownLink` (stable per-attachment
URL persisted into markdown), the comment editor's image src + file
card href correctly switched to `markdownLink ?? link`. But chat
input still kept the upload-map key on the old `link`:

  uploadMapRef.current.set(result.link, result.id)
  …
  if (content.includes(url)) activeIds.push(id)

In the LocalStorage backend `link` is the short-lived
`/uploads/<key>?exp=&sig=` URL. The editor persists the stable
`/api/attachments/<id>/download` URL into the message body, so
`content.includes(url)` never matches and the send call drops
`attachment_ids`. The attachment ends up bound only to the chat
session, not to the message — agents reading message-level metadata
see no attachments.

Fix: key the upload map on the same value the editor actually wrote
into the markdown body (`markdownLink || link`). The
`content.includes(url)` check then matches and the attachment id is
correctly forwarded on send.

Tests:

- Updated the chat-input mock editor to insert `markdownLink || link`
  into its value, mirroring the real editor's persisted-URL choice
  (uploadAndInsertFile in editor/extensions/file-upload.ts). Without
  this the mock would silently paper over the bug.
- Added a regression test where the upload result returns a
  short-lived `link = /uploads/...?exp&sig` and a stable
  `markdownLink = /api/attachments/<id>/download`. Asserts (a) the
  message body carries the stable URL and never the signed query,
  and (b) the bound `attachment_ids` includes the attachment id.

All 1223 frontend views tests pass (was 1222, +1 new regression).
Typecheck and 507 core tests still green.

Refs: MUL-3130, PR #3937 review by GPT-Boy. Builds on f66a522d0.
Co-authored-by: multica-agent <github@multica.ai>

---------

Co-authored-by: Eve <eve@multica-ai.local>
Co-authored-by: multica-agent <github@multica.ai>
2026-06-09 14:26:36 +08:00
Bohan Jiang
d6540a1869 fix(clipboard): support copy over http:// via execCommand fallback (#3810)
navigator.clipboard is only exposed in a secure context (https or
localhost). On self-hosted instances served over plain http:// it is
undefined, so every copy / "copy all" / export button silently failed and
left the clipboard empty (GitHub #3781).

Add a shared copyText(text): Promise<boolean> helper in
@multica/ui/lib/clipboard that prefers the async Clipboard API and falls
back to a hidden <textarea> + document.execCommand('copy') for non-secure
contexts. Migrate all direct navigator.clipboard.writeText call sites
(code blocks, agent transcript copy-all, token / webhook / issue-link
copy, etc.) to it, gating success side-effects on the returned boolean,
and remove the now-redundant copyMarkdown wrapper. Secure-context users
keep the native path unchanged.

MUL-3068

Co-authored-by: J <j@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>
2026-06-05 14:55:23 +08:00
Naiyuan Qing
0dbe9f0a8f Move caret after inserted image uploads (#3796)
* fix editor image upload caret placement

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

* feat(editor): reserve image box via intrinsic dimensions to kill paste layout shift (#3803)

Capture an image's intrinsic width/height on upload and render them as
<img width height> so the browser reserves the box before the image
decodes. Removes the layout shift that pushed the caret out of view after
a pasted-image insert, making the post-insert scrollIntoView correct.

- Add width/height node attrs to ImageExtension (render-only; not
  serialized to markdown, so round-trips stay clean).
- Measure dimensions off-thread via createImageBitmap and patch the node
  after insert. Fire-and-forget so the synchronous-insert contract
  (instant preview) is preserved; degrades to no-box when the API is
  unavailable (jsdom). The src swap keeps width/height via attr spread.
- Thread width/height through ImageView -> Attachment -> <img>.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: multica-agent <github@multica.ai>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-05 13:48:36 +08:00
YOMXXX
7d24a8594a fix(comments): support edit-time attachment removal (#2965) 2026-05-27 09:48:59 +08:00
Naiyuan Qing
5c1fad4508 refactor(editor): split rich text styles (#3211) 2026-05-25 16:02:10 +08:00
Naiyuan Qing
d46e90ee0a refactor(editor): keep <Attachment> image rendering as a pure port of the original ImageView (#2857)
Earlier the unification commit dragged in a Tailwind override stack
(ring, rounded-md, transition-shadow, bg-background/95, button hover
classes) "to make standalone surfaces work without .rich-text-editor
scope". Because the legacy CSS rules were not removed, both layers
applied in the editor, producing a visible double-stroke selection
ring and a light-theme hover on top of the dark-glass toolbar.

This commit reverts the styling churn:

- ImageAttachmentView now emits the same span-only DOM as the original
  ReadonlyImage: <span.image-node> > <span.image-figure> > <img.image-content>
  + <span.image-toolbar> with naked <button> children. No Tailwind tax.
- The `.image-*` rules in content-editor.css are de-scoped from
  `.rich-text-editor` so the single set of styles also drives chat /
  AttachmentList renders. Editor-only behavior (640px cap, NodeView
  centering) stays under the `.rich-text-editor` scope.
- A `data-clickable` attribute carries the "this image is clickable
  to preview" hint that the readonly cursor rule used to key off the
  `.rich-text-editor.readonly` scope.
- ImageView NodeViewWrapper no longer adds its own `image-node` class
  because `<Attachment>` already emits one; the duplicate was harmless
  but redundant.

Visual: editor + readonly comments render identical to before. Chat /
AttachmentList previously rendered a gray file card for images (the
P0 fix in the parent commit) and now match the editor visual without
the heavy-handed Tailwind detour.

Tests: 98 attachment-related tests pass; full `pnpm typecheck` + `pnpm
test` (652 tests) green.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-19 16:58:45 +08:00
Naiyuan Qing
39f43a9a98 refactor(editor): unify attachment rendering into a single <Attachment> component (#2850)
Collapse the five separate attachment render paths (file-card NodeView,
image NodeView, readonly markdown img/fileCard renderers, AttachmentList
standalone fallback, and the parallel packages/ui/markdown renderer) into
one <Attachment attachment={a} /> dispatcher.

Fixes a P0 visual regression: a PNG attached to a message but not inlined
in the markdown body used to render as a gray "file card" because
getPreviewKind() lacked an "image" branch and image rendering bypassed
the dispatcher entirely. Now every surface routes through <Attachment>,
so the same PNG renders as a real <img> with hover toolbar and
preview-modal everywhere.

Key changes:
- PreviewKind gains "image"; getPreviewKind() detects image/* + common
  extensions before the html/text branches (so svg stays image, not text).
- AttachmentPreviewModal gains case "image" (replaces the standalone
  ImageLightbox, which is deleted).
- New packages/views/editor/attachment.tsx owns all kind-aware routing
  (image | html | file) and dispatches preview modal + download via the
  existing useAttachmentPreview / useDownloadAttachment hooks. Subsumes
  the deleted AttachmentBlock.
- AttachmentInput.url accepts a forceKind hint so callers that *know*
  the structural kind (markdown ![](url), Tiptap image node) skip the
  filename-based autodetect — fixes a regression where empty or
  descriptive alt text would route an image to the file-card chrome.
- Tiptap NodeViews (file-card.tsx, image-view.tsx) shrink to thin
  wrappers that forward editor hints (selected, deleteNode, uploading)
  to <Attachment>.
- ReadonlyContent and AttachmentList each mount their own
  AttachmentDownloadProvider so url → record resolution works outside
  ContentEditor's provider.
- packages/ui/markdown gains optional renderImage / renderFileCard slot
  props; packages/views/common/markdown.tsx injects <Attachment> into
  those slots and threads message attachments through to chat /
  skill-file viewers.
- chat-message-list passes message.attachments to every <Markdown> call
  site and renders a standalone AttachmentList under each bubble for
  attachments not referenced in the body.

Tests: attachment.test.tsx covers 9 scenarios (record image / pdf / html;
url-only image with resolver hit and miss; uploading state; editable
delete; forceKind regression). attachment-preview-modal.test.tsx gains
image-dispatch cases. 652/652 unit tests pass.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-19 16:23:19 +08:00