fix(files): download a markdown file as a zip only when it really has assets - #7009
Conversation
… assets A document that merely mentions an embed URL in prose or an inline code span counted as having attachments, so any document about the files API downloaded as a zip whose assets/ folder was empty. - Detect embeds with the markdown lexer instead of scanning raw text, so only real image embeds count: prose, code spans, fenced samples, and links no longer do - Choose the export format after resolving assets rather than from the candidate count, so a missing, unreadable, or oversized embed falls back to the plain document instead of an empty zip - Move the document scan out of the copilot tool tree into lib/uploads/server, where both file routes already live, and drop two pass-through wrappers - Share one <img> src reader between the clipboard handlers and the scan - Walk tokens explicitly: marked's walkTokens concatenates per token and costs O(n^2), measuring 5.4s on a 254KB document against 14ms here, on a path anonymous public-share traffic reaches
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview Embed detection now lexes markdown and counts only real images ( The scan moves to Reviewed by Cursor Bugbot for commit f346f5c. Configure here. |
Greptile SummaryThe PR makes Markdown export format depend on successfully bundled image assets and limits image-reference detection to actual Markdown or HTML image embeds.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| apps/sim/lib/uploads/server/embedded-image-refs.ts | Introduces bounded, lexer-based extraction of actual Markdown and HTML image references while preserving separate document and storage ID representations. |
| apps/sim/app/api/files/export/[id]/route.ts | Resolves document-spelled IDs through their stored form and returns plain Markdown when no image asset is successfully bundled. |
| apps/sim/app/api/files/public/[token]/inline/route.ts | Reuses the shared image-aware extraction result for the public share’s referenced-by-document authorization gate. |
| apps/sim/lib/uploads/utils/embedded-image-ref.ts | Keeps single-source reference parsing isomorphic and centralizes raw HTML image-source extraction for client and server consumers. |
| apps/sim/app/api/files/export/[id]/route.test.ts | Adds regression coverage for encoded IDs, unresolved or unreadable assets, and Markdown-versus-ZIP response selection. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Markdown document] --> B[Lex image tokens]
B --> C[Extract source-spelled IDs and decoded keys]
C --> D[Resolve stored file metadata]
D --> E{Any assets bundled?}
E -->|No| F[Return Markdown]
E -->|Yes| G[Rewrite embedded URLs]
G --> H[Return ZIP with assets]
C --> I[Public-share referenced-image gate]
I --> J[Workspace and content checks]
J --> K[Serve inline image]
Reviews (6): Last reviewed commit: "fix(files): resolve an embed by its stor..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Decoding the id let a percent-encoded embed resolve and bundle its asset while the rewrite, which searches the document for that id, found nothing — the zip kept an API URL that renders as a broken image offline. Keys stay decoded; they are matched against stored keys, not against document text.
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 10bb0e7. Configure here.
…spelling An embed carries two representations and they are not interchangeable: metadata resolves by the stored id, while the rewrite finds the embed by searching the document for the spelling it used. Using one for both either drops a percent-encoded asset or bundles it behind a link still pointing at the API.
|
Addressed the round-2 summary finding in f346f5c. The observation was correct: preserving the spelling fixed the rewrite but moved the problem to the lookup, so a percent-encoded embed resolved to nothing and was dropped. That was fail-safe — the asset simply isn't bundled and the markdown keeps its original working URL — but it was still a trade rather than a fix. An embed carries two representations and they aren't interchangeable:
Collapsing them in either direction breaks one side. The export now decodes for the lookup and keeps the spelling for the rewrite, so a percent-encoded embed both resolves and gets rewritten. Covered by a regression test that bundles the asset and asserts the markdown ends up with Worth noting for scope: no producer in the system emits encoded ids — the file agent and the editor both write them plainly, and ids are |
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit f346f5c. Configure here.
|
@cursor review |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit f346f5c. Configure here.
…om a document The export bundler decoded an embed's spelling before looking it up, but the file-agent's embeddability warning did not, so a percent-encoded embed the export resolves and bundles could still be reported as one that will not survive an export. Both now share one helper. Request-supplied ids are untouched: their route contracts already constrain them to the plain id charset, so there is no spelling to decode.
1 participant
Summary
assets/folder whenever the document merely mentioned an embed URL. Any document about the files API tripped it — an inline`/api/files/view/{id}`in prose was enough., reference images, and<img>tags), so a mention can't masquerade as an attachment. Links are excluded — a link is navigated to, not displayed.lib/uploads/server/, where both consuming file routes already live, and dropped two pass-through wrappers. The<img>src reader is now shared between the clipboard handlers and the scan instead of being defined twice.walkTokens, which concatenates its callback's return value once per token and so costs O(n²) — a 254KB document measured 5.4s of blocked event loop versus 14ms. That path is reachable by anonymous public-share traffic, so this also removes an unauthenticated stall vector.Type of Change
Testing
<img>, and that fenced/<pre>content is ignored.bun run type-check,bun run lint,bun run check:api-validation, and the fullbun run check:audits(33 audits) pass.Checklist