Commit Graph

2 Commits

Author SHA1 Message Date
Multica Eve
57d1a0a00f fix(quick-create): track concurrent uploads with in-flight counter (MUL-3339) (#4562)
`useFileUpload` exposed a single `uploading: boolean` shared across all
concurrent upload calls. When the user drag-dropped N images into the
Quick Create modal, the first upload's `finally` flipped the flag back
to false while N-1 uploads were still in flight. The submit gate (which
only checked `uploading`) re-enabled, and on submit:

  - `getMarkdown()` ran `stripBlobUrls()` and erased every still-pending
    `![alt](blob:...)` placeholder from the prompt.
  - `pendingAttachments` only contained the first-completed upload, so
    `activeAttachmentIds` shipped a single ID.
  - The remaining attachment rows never got linked to the new issue —
    their `issue_id` stayed NULL and the UI showed "Attachment doesn't
    exist".

Fix:

1. Replace the boolean with an in-flight counter in `useFileUpload`.
   `uploading = inFlight > 0` so the flag stays true until ALL concurrent
   uploads resolve (or reject — the `finally` decrements either way).
   The public return shape is unchanged; every existing call site keeps
   working.

2. Belt-and-suspenders: add `editorRef.current?.hasActiveUploads()` to
   the quick-create submit gate. The editor already tracks per-node
   `uploading` attrs (the source of truth for "is there a blob preview
   still resolving"); checking it on submit guarantees the strip step
   can never run against an in-flight image even if some future code
   path mis-clears `uploading`.

3. Regression coverage in `use-file-upload.test.ts`:
   - Fire two concurrent uploads, resolve them out of order, assert
     `uploading` stays true until BOTH resolve. Confirmed to fail
     against the pre-fix code.
   - Reject one of the concurrent uploads, assert the counter still
     decrements correctly (the `finally` runs on rejection).

The mock ContentEditor in `quick-create-issue.test.tsx` gains a
`hasActiveUploads: () => false` stub so the new defense call doesn't
explode in existing tests.

Verified: `pnpm test` on @multica/core (663 tests) and @multica/views
(1471 tests) both green; typecheck + lint clean on both packages.

Co-authored-by: Eve <eve@multica-ai.local>
Co-authored-by: multica-agent <github@multica.ai>
2026-06-25 14:11:59 +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