Export workspace - #717
Conversation
|
Capy auto-review is paused for this organization because the usage-cycle auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews. |
📝 WalkthroughWalkthroughChangesWorkspace export now reconstructs workspace state, converts documents, creates sanitized ZIP archives, and serves downloads through a new API route and toolbar action. Local development supports proxied direct uploads, while preview failures use the uploaded source object as fallback. Workspace export
Local-development upload handling
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant WorkspaceTopBar
participant WorkspaceExportRoute
participant WorkspaceExport
participant WorkspaceDatabase
participant GotenbergContainer
WorkspaceTopBar->>WorkspaceExportRoute: GET workspace export
WorkspaceExportRoute->>WorkspaceDatabase: load membership and workspace page
WorkspaceExportRoute->>WorkspaceExport: build workspace ZIP
WorkspaceExport->>GotenbergContainer: convert document HTML to PDF
GotenbergContainer-->>WorkspaceExport: return PDF or conversion failure
WorkspaceExport-->>WorkspaceExportRoute: return ZIP bytes
WorkspaceExportRoute-->>WorkspaceTopBar: download response
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/features/workspaces/upload/workspace-file-upload-storage.ts (1)
104-129: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winFallback preview is stored with a Content-Type that does not match its actual bytes.
When
createWorkspaceFilePreviewfails, the.catchhandler returns the raw uploaded/converted object'sbodyandsizeBytesas the "preview" (lines 119-128). This value flows intoputFixedLengthR2Object(..., preview, { httpMetadata: { contentType: WORKSPACE_FILE_PREVIEW_CONTENT_TYPE } })at line 134, which unconditionally applies the preview-specific content type to whateverpreview.bodycontains. In the fallback case,preview.bodyis the original file (which can be a PDF, an Office document, a video, and so on), so the stored preview object is served with a Content-Type header that does not describe its actual bytes. Any consumer that renders the preview object based on its stored Content-Type will get broken or misleading output.Preserve the actual content type of the fallback object, or route the fallback path away from the generic preview content type.
#!/bin/bash # Description: Confirm the value and expected semantics of WORKSPACE_FILE_PREVIEW_CONTENT_TYPE. rg -n "WORKSPACE_FILE_PREVIEW_CONTENT_TYPE" -C3🐛 Proposed fix
}).catch(async (error) => { recordWorkspaceUploadPreviewFallback({ error, fileName: upload.fileName, }); const fallbackObject = await input.env.WORKSPACE_KERNEL_FILES.get(upload.objectKey); if (!fallbackObject) { throw error; } return { body: fallbackObject.body, sizeBytes: fallbackObject.size, + contentType: fallbackObject.httpMetadata?.contentType ?? upload.contentType, }; }); const stored = await putFixedLengthR2Object( input.env.WORKSPACE_KERNEL_FILES, input.previewObjectKey, preview, - { httpMetadata: { contentType: WORKSPACE_FILE_PREVIEW_CONTENT_TYPE } }, + { + httpMetadata: { + contentType: preview.contentType ?? WORKSPACE_FILE_PREVIEW_CONTENT_TYPE, + }, + }, );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/workspaces/upload/workspace-file-upload-storage.ts` around lines 104 - 129, Update storeWorkspaceFileUploadPreview so the fallback result from WORKSPACE_KERNEL_FILES.get preserves fallbackObject.httpMetadata.contentType (or otherwise bypasses the preview-specific metadata) when passed to putFixedLengthR2Object. Keep WORKSPACE_FILE_PREVIEW_CONTENT_TYPE for successfully generated previews, but ensure raw fallback bytes are stored and served with their actual content type.
🧹 Nitpick comments (7)
src/features/workspaces/export/workspace-zip.ts (1)
165-173: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueUse an index loop for the CRC32 hot path.
for (const byte of data)allocates iterator results for every byte. For multi-megabyte workspace files this is measurably slower than an index loop.♻️ Proposed change
function crc32(data: Uint8Array) { let crc = 0xffffffff; - for (const byte of data) { - crc = (crc >>> 8) ^ crcTable[(crc ^ byte) & 0xff]!; - } + for (let index = 0; index < data.length; index += 1) { + crc = (crc >>> 8) ^ crcTable[(crc ^ data[index]!) & 0xff]!; + } return (crc ^ 0xffffffff) >>> 0; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/workspaces/export/workspace-zip.ts` around lines 165 - 173, Update the crc32 function’s byte-processing loop to use an index-based loop over data instead of for...of iteration, while preserving the existing CRC calculation and unsigned return value.src/features/workspaces/export/workspace-export.test.ts (1)
121-128: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the archive bytes and for
The current ZIP test checks the local signature and a substring. It cannot detect a wrong CRC32, a wrong central-directory offset, or a wrong size field, and a corrupt archive would still pass.
Add two cases:
- Assert
crc32output against a known vector, for example CRC32 of"123456789"is0xcbf43926.- Assert that a document named
Reportand a file namedReport.pdfin the same folder produce two distinct entry paths, becausestripExportExtensionmaps both toReport.pdfbeforereserveUniqueZipPathruns.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/workspaces/export/workspace-export.test.ts` around lines 121 - 128, Extend the workspace export tests with a known-vector assertion for the crc32 utility, verifying that “123456789” produces 0xcbf43926. Add a collision case covering a document named Report and a file named Report.pdf in the same folder, and assert their exported ZIP entry paths remain distinct after stripExportExtension and reserveUniqueZipPath.src/routes/api/v1/workspaces.$workspaceId.export.ts (2)
141-145: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid the full archive copy.
toResponseBodyallocates a second buffer of the same size, so peak memory doubles for every export. AUint8Arrayis a validBodyInitin Workers, so passarchive.bodydirectly.♻️ Proposed change
- return new Response(toResponseBody(archive.body), { + return new Response(archive.body, {If the copy exists to detach a view over a pooled buffer, add a comment that states this reason.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/routes/api/v1/workspaces`.$workspaceId.export.ts around lines 141 - 145, Remove the full-buffer allocation and copying in toResponseBody, and pass archive.body directly as the response BodyInit. If the copy is required to detach a view from pooled storage, retain it and document that specific reason in a comment.
17-41: 🩺 Stability & Availability | 🔵 TrivialConsider rate limiting this endpoint.
Each request reconstructs workspace state, reads every stored file, and starts a Gotenberg container. A repeated GET is cheap for a caller and expensive for the platform. Add a per-user limit or a short in-flight guard on the server, since the client-side
exportingflag protects only one browser tab.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/routes/api/v1/workspaces`.$workspaceId.export.ts around lines 17 - 41, Add server-side protection to handleWorkspaceExport by applying a per-user rate limit or short-lived in-flight guard before getExportWorkspacePage and exportWorkspaceToZip run. Key the guard by the authenticated session.user.id, reject excess concurrent or rapid requests with the existing API error pattern, and release the guard reliably after export completion or failure.src/features/workspaces/export/workspace-export.ts (1)
54-123: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftThe export holds the full archive in memory and serializes every conversion.
Two scaling limits apply on Workers:
- Line 97 reads each stored file with
arrayBuffer(), andcreateZipArchivereturns oneUint8Array. Peak memory is roughly twice the total workspace size. Large workspaces will exceed the isolate memory limit.- The loop awaits each document conversion in sequence. Export duration grows linearly with the document count and can hit the request time limit.
Consider streaming ZIP output through a
TransformStreamand bounding document conversions with a small concurrency limit.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/workspaces/export/workspace-export.ts` around lines 54 - 123, Refactor buildWorkspaceExportEntries and createZipArchive to stream ZIP entries through a TransformStream instead of collecting full file buffers and returning one Uint8Array. Stream stored file bodies without calling object.arrayBuffer(), and process document conversions with a small bounded concurrency limit while preserving entry ordering, failure notices, and unique paths.src/features/workspaces/components/WorkspaceTopBar.tsx (1)
84-94: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winDefer revoking the download blob URL.
link.click()starts the download asynchronously; revokingobjectUrlimmediately afterlink.click()can invalidate the blob before the browser reads it. ScheduleURL.revokeObjectURL(objectUrl)on a later task.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/workspaces/components/WorkspaceTopBar.tsx` around lines 84 - 94, In the download flow around WorkspaceTopBar, defer URL.revokeObjectURL(objectUrl) until a later task after link.click() completes, rather than revoking it synchronously. Keep the temporary link creation, click, and removal behavior unchanged.src/features/workspaces/upload/workspace-file-direct-upload.ts (1)
31-58: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor: avoid signing extraneous fields in the claims payload.
claimsis built by spreadinginput, which includesallowLocalDevUploadProxyandlocalUploadOrigin. Neither field is part ofuploadClaimsSchema.signUploadClaimssigns the fullclaimsobject, so these two fields end up base64-encoded (not encrypted) inside the completion token returned to the client. Verification strips them safely becauseuploadClaimsSchema.parse()drops unknown keys, so there is no functional bug today. Buildclaimsfrom an explicit allow-list of schema fields instead of spreadinginput, so any future addition to the local-proxy options object cannot leak into the client-visible token by accident.♻️ Proposed refactor
const claims: WorkspaceDirectUploadClaims = { - ...input, + clientMutationId: input.clientMutationId, + contentType: input.contentType, + fileName: input.fileName, + fileSize: input.fileSize, + parentId: input.parentId, + target: input.target, + userId: input.userId, + workspaceId: input.workspaceId, expiresAt: Math.floor(Date.now() / 1_000) + uploadUrlLifetimeSeconds, itemId: crypto.randomUUID(), version: uploadTokenVersion, };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/workspaces/upload/workspace-file-direct-upload.ts` around lines 31 - 58, Update createWorkspaceDirectUploadSession so claims includes only fields defined by WorkspaceDirectUploadClaims/uploadClaimsSchema, explicitly selecting the allowed upload claim properties from input instead of spreading the full input object. Keep allowLocalDevUploadProxy and localUploadOrigin available for createUploadUrl, but exclude them from the object passed to signUploadClaims.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/features/workspaces/components/WorkspaceTopBar.tsx`:
- Around line 176-184: Update getDownloadFileName so decodeURIComponent is
applied only to the RFC 5987 filename*=UTF-8'' match; return quoted and unquoted
filename matches after trimming without decoding, and guard the RFC 5987 decode
so malformed percent-encoding falls back safely instead of throwing.
In `@src/features/workspaces/export/workspace-export.ts`:
- Around line 427-432: Update stripExportExtension so document titles containing
dots are preserved rather than truncating at the last dot; append .pdf to the
sanitized title without stripping arbitrary dot segments, or only remove
extensions from an explicit known document-extension list.
- Around line 368-399: Update createFallbackTextPdf to paginate wrapped lines
into chunks of the approximately 52 lines supported by the current page layout,
generating corresponding /Pages Kids entries, /Count, page objects, and content
streams so all lines are retained. Update escapePdfText to preserve supported
non-ASCII characters through WinAnsi mapping or replace unsupported characters
with a visible placeholder instead of silently dropping them.
- Around line 78-85: Guard the document branch in the workspace export loop
around renderWorkspaceDocumentPdf so a rendering failure does not abort the
archive. On failure, follow the existing file-branch fallback pattern by adding
a .missing.txt notice for the document and continue exporting remaining entries;
preserve the current PDF entry path for successful renders and update the error
handling in renderWorkspaceDocumentPdf as needed to support this behavior.
- Around line 290-312: Update the converter.fetch call in the workspace export
flow to include an AbortSignal with a bounded timeout, aligned with the existing
60-second converter startup limit. Ensure a stalled Gotenberg request aborts and
triggers the existing fallback-to-text-PDF handling.
In `@src/features/workspaces/export/workspace-zip.ts`:
- Around line 43-59: Add explicit ZIP32 limit validation in the workspace
archive writer before creating the central directory and EOCD: reject archives
with central-directory offsets or sizes exceeding the 32-bit maximum, and reject
more than 65,535 prepared entries. Throw clear errors identifying the exceeded
limit, while preserving normal processing for valid archives and ensuring checks
cover both the central-directory construction and final EOCD generation.
In `@src/routes/api/v1/workspaces`.$workspaceId.export.ts:
- Around line 43-51: Update the Content-Disposition header in the export
response to emit both an ASCII-safe filename fallback and an RFC 5987 UTF-8
filename* parameter. Preserve the sanitized workspace filename for filename*,
while deriving a valid ASCII fallback for filename so non-ASCII names do not
become mojibake; keep the existing attachment behavior and header formatting
intact.
In `@src/routes/api/v1/workspaces`.$workspaceId.file-upload.ts:
- Around line 426-431: Replace hostname-based isLocalDevRequest with the
server-controlled env.ALLOW_LOCAL_DEV_UPLOAD_PROXY flag, and use that same flag
to gate both storeLocalDevDirectUpload and getUploadTokenSecret’s shared
local-development secret fallback. In
src/routes/api/v1/workspaces.$workspaceId.file-upload.ts lines 426-431 update
the mode check; in lines 117-164 apply the flag to both upload paths; in
src/features/workspaces/upload/workspace-file-direct-upload.ts lines 158-170
move completionToken out of the URL query string into a less log-exposed
transport while preserving completion behavior.
---
Outside diff comments:
In `@src/features/workspaces/upload/workspace-file-upload-storage.ts`:
- Around line 104-129: Update storeWorkspaceFileUploadPreview so the fallback
result from WORKSPACE_KERNEL_FILES.get preserves
fallbackObject.httpMetadata.contentType (or otherwise bypasses the
preview-specific metadata) when passed to putFixedLengthR2Object. Keep
WORKSPACE_FILE_PREVIEW_CONTENT_TYPE for successfully generated previews, but
ensure raw fallback bytes are stored and served with their actual content type.
---
Nitpick comments:
In `@src/features/workspaces/components/WorkspaceTopBar.tsx`:
- Around line 84-94: In the download flow around WorkspaceTopBar, defer
URL.revokeObjectURL(objectUrl) until a later task after link.click() completes,
rather than revoking it synchronously. Keep the temporary link creation, click,
and removal behavior unchanged.
In `@src/features/workspaces/export/workspace-export.test.ts`:
- Around line 121-128: Extend the workspace export tests with a known-vector
assertion for the crc32 utility, verifying that “123456789” produces 0xcbf43926.
Add a collision case covering a document named Report and a file named
Report.pdf in the same folder, and assert their exported ZIP entry paths remain
distinct after stripExportExtension and reserveUniqueZipPath.
In `@src/features/workspaces/export/workspace-export.ts`:
- Around line 54-123: Refactor buildWorkspaceExportEntries and createZipArchive
to stream ZIP entries through a TransformStream instead of collecting full file
buffers and returning one Uint8Array. Stream stored file bodies without calling
object.arrayBuffer(), and process document conversions with a small bounded
concurrency limit while preserving entry ordering, failure notices, and unique
paths.
In `@src/features/workspaces/export/workspace-zip.ts`:
- Around line 165-173: Update the crc32 function’s byte-processing loop to use
an index-based loop over data instead of for...of iteration, while preserving
the existing CRC calculation and unsigned return value.
In `@src/features/workspaces/upload/workspace-file-direct-upload.ts`:
- Around line 31-58: Update createWorkspaceDirectUploadSession so claims
includes only fields defined by WorkspaceDirectUploadClaims/uploadClaimsSchema,
explicitly selecting the allowed upload claim properties from input instead of
spreading the full input object. Keep allowLocalDevUploadProxy and
localUploadOrigin available for createUploadUrl, but exclude them from the
object passed to signUploadClaims.
In `@src/routes/api/v1/workspaces`.$workspaceId.export.ts:
- Around line 141-145: Remove the full-buffer allocation and copying in
toResponseBody, and pass archive.body directly as the response BodyInit. If the
copy is required to detach a view from pooled storage, retain it and document
that specific reason in a comment.
- Around line 17-41: Add server-side protection to handleWorkspaceExport by
applying a per-user rate limit or short-lived in-flight guard before
getExportWorkspacePage and exportWorkspaceToZip run. Key the guard by the
authenticated session.user.id, reject excess concurrent or rapid requests with
the existing API error pattern, and release the guard reliably after export
completion or failure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ed8dfe4b-b0ca-4afc-917a-ccca834151a7
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
containers/gotenberg/Dockerfilepackage.jsonsrc/features/workspaces/components/WorkspaceTopBar.tsxsrc/features/workspaces/export/workspace-export.test.tssrc/features/workspaces/export/workspace-export.tssrc/features/workspaces/export/workspace-zip.tssrc/features/workspaces/upload/workspace-file-direct-upload.tssrc/features/workspaces/upload/workspace-file-upload-storage.tssrc/routeTree.gen.tssrc/routes/api/v1/workspaces.$workspaceId.export.tssrc/routes/api/v1/workspaces.$workspaceId.file-upload.ts
| function getDownloadFileName(headers: Headers) { | ||
| const contentDisposition = headers.get("content-disposition"); | ||
| const match = | ||
| contentDisposition?.match(/filename\*=UTF-8''([^;]+)/i) ?? | ||
| contentDisposition?.match(/filename="([^"]+)"/i) ?? | ||
| contentDisposition?.match(/filename=([^;]+)/i); | ||
|
|
||
| return match?.[1] ? decodeURIComponent(match[1].trim()) : null; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
A % in the workspace name breaks the download.
decodeURIComponent runs for all three branches. A plain quoted filename such as 100% notes.zip contains an unescaped %, so decodeURIComponent throws URIError. The error propagates out of getDownloadFileName, reaches the catch at Line 95, and the user sees an export failure although the response succeeded. Decode only the RFC 5987 form, and guard the decode.
🐛 Proposed fix
function getDownloadFileName(headers: Headers) {
const contentDisposition = headers.get("content-disposition");
- const match =
- contentDisposition?.match(/filename\*=UTF-8''([^;]+)/i) ??
- contentDisposition?.match(/filename="([^"]+)"/i) ??
- contentDisposition?.match(/filename=([^;]+)/i);
-
- return match?.[1] ? decodeURIComponent(match[1].trim()) : null;
+ const encoded = contentDisposition?.match(/filename\*=UTF-8''([^;]+)/i);
+
+ if (encoded?.[1]) {
+ try {
+ return decodeURIComponent(encoded[1].trim());
+ } catch {
+ // Fall through to the plain filename parameter.
+ }
+ }
+
+ const plain =
+ contentDisposition?.match(/filename="([^"]+)"/i) ??
+ contentDisposition?.match(/filename=([^;]+)/i);
+
+ return plain?.[1]?.trim() || null;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function getDownloadFileName(headers: Headers) { | |
| const contentDisposition = headers.get("content-disposition"); | |
| const match = | |
| contentDisposition?.match(/filename\*=UTF-8''([^;]+)/i) ?? | |
| contentDisposition?.match(/filename="([^"]+)"/i) ?? | |
| contentDisposition?.match(/filename=([^;]+)/i); | |
| return match?.[1] ? decodeURIComponent(match[1].trim()) : null; | |
| } | |
| function getDownloadFileName(headers: Headers) { | |
| const contentDisposition = headers.get("content-disposition"); | |
| const encoded = contentDisposition?.match(/filename\*=UTF-8''([^;]+)/i); | |
| if (encoded?.[1]) { | |
| try { | |
| return decodeURIComponent(encoded[1].trim()); | |
| } catch { | |
| // Fall through to the plain filename parameter. | |
| } | |
| } | |
| const plain = | |
| contentDisposition?.match(/filename="([^"]+)"/i) ?? | |
| contentDisposition?.match(/filename=([^;]+)/i); | |
| return plain?.[1]?.trim() || null; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/features/workspaces/components/WorkspaceTopBar.tsx` around lines 176 -
184, Update getDownloadFileName so decodeURIComponent is applied only to the RFC
5987 filename*=UTF-8'' match; return quoted and unquoted filename matches after
trimming without decoding, and guard the RFC 5987 decode so malformed
percent-encoding falls back safely instead of throwing.
| if (item.type === "document") { | ||
| entries.push({ | ||
| data: await renderWorkspaceDocumentPdf(env, kernel, item), | ||
| modifiedAt: new Date(item.updatedAt), | ||
| path: reserveUniqueZipPath(`${stripExportExtension(zipPath)}.pdf`, usedZipPaths), | ||
| }); | ||
| continue; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
One failing document aborts the whole export.
renderWorkspaceDocumentPdf rethrows at Line 219. Line 80 awaits it without a guard, so the route returns a 500 and the user receives no archive. The file branch at Lines 104-118 writes a .missing.txt notice instead. Apply the same graceful degradation to documents.
♻️ Proposed fallback for document render failures
if (item.type === "document") {
- entries.push({
- data: await renderWorkspaceDocumentPdf(env, kernel, item),
- modifiedAt: new Date(item.updatedAt),
- path: reserveUniqueZipPath(`${stripExportExtension(zipPath)}.pdf`, usedZipPaths),
- });
- continue;
+ const pdf = await renderWorkspaceDocumentPdf(env, kernel, item).catch(() => null);
+ entries.push(
+ pdf
+ ? {
+ data: pdf,
+ modifiedAt: new Date(item.updatedAt),
+ path: reserveUniqueZipPath(`${stripExportExtension(zipPath)}.pdf`, usedZipPaths),
+ }
+ : {
+ data: createMissingWorkspaceFileNotice(item.name),
+ modifiedAt: new Date(item.updatedAt),
+ path: reserveUniqueZipPath(`${zipPath}.missing.txt`, usedZipPaths),
+ },
+ );
+ continue;
}Also applies to: 199-221
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/features/workspaces/export/workspace-export.ts` around lines 78 - 85,
Guard the document branch in the workspace export loop around
renderWorkspaceDocumentPdf so a rendering failure does not abort the archive. On
failure, follow the existing file-branch fallback pattern by adding a
.missing.txt notice for the document and continue exporting remaining entries;
preserve the current PDF entry path for successful renders and update the error
handling in renderWorkspaceDocumentPdf as needed to support this behavior.
| await Promise.resolve( | ||
| converter.startAndWaitForPorts({ | ||
| cancellationOptions: { | ||
| portReadyTimeoutMS: 60_000, | ||
| }, | ||
| }), | ||
| ); | ||
|
|
||
| const [response] = await Promise.all([ | ||
| converter.fetch( | ||
| new Request(`http://office-pdf-converter${gotenbergChromiumHtmlPath}`, { | ||
| body: multipart.body, | ||
| duplex: "half", | ||
| headers: { "content-type": multipart.contentType }, | ||
| method: "POST", | ||
| } as RequestInit & { duplex: "half" }), | ||
| ), | ||
| multipart.done, | ||
| ]); | ||
|
|
||
| if (!response.ok) { | ||
| throw new Error(`HTML to PDF conversion failed with status ${response.status}.`); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Add a timeout to the converter request.
startAndWaitForPorts uses a 60 s port timeout, but converter.fetch at Line 299 has no deadline. If the Gotenberg container accepts the connection and then stalls, the export request hangs until the platform kills the worker. Pass an AbortSignal with a bounded timeout so the code falls back to the text PDF.
🛠️ Proposed timeout
const [response] = await Promise.all([
converter.fetch(
new Request(`http://office-pdf-converter${gotenbergChromiumHtmlPath}`, {
body: multipart.body,
duplex: "half",
headers: { "content-type": multipart.contentType },
method: "POST",
+ signal: AbortSignal.timeout(120_000),
} as RequestInit & { duplex: "half" }),
),
multipart.done,
]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await Promise.resolve( | |
| converter.startAndWaitForPorts({ | |
| cancellationOptions: { | |
| portReadyTimeoutMS: 60_000, | |
| }, | |
| }), | |
| ); | |
| const [response] = await Promise.all([ | |
| converter.fetch( | |
| new Request(`http://office-pdf-converter${gotenbergChromiumHtmlPath}`, { | |
| body: multipart.body, | |
| duplex: "half", | |
| headers: { "content-type": multipart.contentType }, | |
| method: "POST", | |
| } as RequestInit & { duplex: "half" }), | |
| ), | |
| multipart.done, | |
| ]); | |
| if (!response.ok) { | |
| throw new Error(`HTML to PDF conversion failed with status ${response.status}.`); | |
| } | |
| await Promise.resolve( | |
| converter.startAndWaitForPorts({ | |
| cancellationOptions: { | |
| portReadyTimeoutMS: 60_000, | |
| }, | |
| }), | |
| ); | |
| const [response] = await Promise.all([ | |
| converter.fetch( | |
| new Request(`http://office-pdf-converter${gotenbergChromiumHtmlPath}`, { | |
| body: multipart.body, | |
| duplex: "half", | |
| headers: { "content-type": multipart.contentType }, | |
| method: "POST", | |
| signal: AbortSignal.timeout(120_000), | |
| } as RequestInit & { duplex: "half" }), | |
| ), | |
| multipart.done, | |
| ]); | |
| if (!response.ok) { | |
| throw new Error(`HTML to PDF conversion failed with status ${response.status}.`); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/features/workspaces/export/workspace-export.ts` around lines 290 - 312,
Update the converter.fetch call in the workspace export flow to include an
AbortSignal with a bounded timeout, aligned with the existing 60-second
converter startup limit. Ensure a stalled Gotenberg request aborts and triggers
the existing fallback-to-text-PDF handling.
| function createFallbackTextPdf(text: string) { | ||
| const lines = text.split(/\r?\n/).flatMap((line) => wrapPdfLine(line, 88)); | ||
| const content = `BT | ||
| /F1 11 Tf | ||
| 50 742 Td | ||
| 14 TL | ||
| ${lines.map((line) => `(${escapePdfText(line)}) Tj T*`).join("\n")} | ||
| ET`; | ||
| const objects = [ | ||
| "<< /Type /Catalog /Pages 2 0 R >>", | ||
| "<< /Type /Pages /Kids [3 0 R] /Count 1 >>", | ||
| "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] /Resources << /Font << /F1 4 0 R >> >> /Contents 5 0 R >>", | ||
| "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica >>", | ||
| `<< /Length ${content.length} >>\nstream\n${content}\nendstream`, | ||
| ]; | ||
| let pdf = "%PDF-1.4\n"; | ||
| const offsets = [0]; | ||
|
|
||
| for (const [index, object] of objects.entries()) { | ||
| offsets.push(pdf.length); | ||
| pdf += `${index + 1} 0 obj\n${object}\nendobj\n`; | ||
| } | ||
|
|
||
| const xrefOffset = pdf.length; | ||
| pdf += `xref\n0 ${objects.length + 1}\n0000000000 65535 f \n`; | ||
| for (const offset of offsets.slice(1)) { | ||
| pdf += `${String(offset).padStart(10, "0")} 00000 n \n`; | ||
| } | ||
| pdf += `trailer << /Size ${objects.length + 1} /Root 1 0 R >>\nstartxref\n${xrefOffset}\n%%EOF\n`; | ||
|
|
||
| return textEncoder.encode(pdf); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
The fallback PDF drops content past the first page and drops non-ASCII text.
Two defects exist in createFallbackTextPdf:
- The generator emits exactly one
/MediaBox [0 0 612 792]page. Text starts at50 742with14 TLleading, so only about 52 lines are visible. All later lines are written outside the page and are lost. escapePdfTextat Lines 457-464 discards every character above code point 126. Documents with accented, CJK, or emoji content lose that text silently.
Split lines into pages and add one page object per chunk. For non-ASCII text, either map to WinAnsi where possible or replace unsupported characters with a visible placeholder, so the loss is not silent.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/features/workspaces/export/workspace-export.ts` around lines 368 - 399,
Update createFallbackTextPdf to paginate wrapped lines into chunks of the
approximately 52 lines supported by the current page layout, generating
corresponding /Pages Kids entries, /Count, page objects, and content streams so
all lines are retained. Update escapePdfText to preserve supported non-ASCII
characters through WinAnsi mapping or replace unsupported characters with a
visible placeholder instead of silently dropping them.
| function stripExportExtension(path: string) { | ||
| const slashIndex = path.lastIndexOf("/"); | ||
| const dotIndex = path.lastIndexOf("."); | ||
|
|
||
| return dotIndex > slashIndex + 1 ? path.slice(0, dotIndex) : path; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
stripExportExtension truncates document titles that contain a dot.
Documents have no file extension. A title such as Notes v1.2 produces Notes v1.pdf, and Q3 report. final produces Q3 report.pdf. Append .pdf to the sanitized name instead of stripping the last dot segment, or strip only a known document extension list.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/features/workspaces/export/workspace-export.ts` around lines 427 - 432,
Update stripExportExtension so document titles containing dots are preserved
rather than truncating at the last dot; append .pdf to the sanitized title
without stripping arbitrary dot segments, or only remove extensions from an
explicit known document-extension list.
| const centralDirectoryOffset = offset; | ||
| const centralDirectory = preparedEntries.map(createCentralDirectoryHeader); | ||
| for (const chunk of centralDirectory) { | ||
| chunks.push(chunk); | ||
| offset += chunk.byteLength; | ||
| } | ||
|
|
||
| const centralDirectorySize = offset - centralDirectoryOffset; | ||
| chunks.push( | ||
| createEndOfCentralDirectory({ | ||
| centralDirectoryOffset, | ||
| centralDirectorySize, | ||
| entryCount: preparedEntries.length, | ||
| }), | ||
| ); | ||
|
|
||
| return concatBytes(chunks); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Add explicit limit checks; the writer has no ZIP64 support.
The central directory and EOCD store sizes and offsets as 32-bit values, and the entry count as a 16-bit value. If the archive exceeds 4 GiB or contains more than 65535 entries, the values wrap and the archive is silently corrupt. Readers then report a damaged file with no server-side error.
Throw a clear error when a limit is exceeded, so the export route can return a explicit failure.
🛡️ Proposed guard
const centralDirectorySize = offset - centralDirectoryOffset;
+ if (preparedEntries.length > 0xffff || offset > 0xffffffff) {
+ throw new Error("Archive exceeds the ZIP format limits supported by this writer.");
+ }
chunks.push(Also applies to: 108-123
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/features/workspaces/export/workspace-zip.ts` around lines 43 - 59, Add
explicit ZIP32 limit validation in the workspace archive writer before creating
the central directory and EOCD: reject archives with central-directory offsets
or sizes exceeding the 32-bit maximum, and reject more than 65,535 prepared
entries. Throw clear errors identifying the exceeded limit, while preserving
normal processing for valid archives and ensuring checks cover both the
central-directory construction and final EOCD generation.
| return new Response(toResponseBody(archive.body), { | ||
| headers: { | ||
| "cache-control": "no-store", | ||
| "content-disposition": `attachment; filename="${sanitizeHeaderFileName(archive.fileName)}"`, | ||
| "content-length": String(archive.body.byteLength), | ||
| "content-type": "application/zip", | ||
| "x-request-id": requestId, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Non-ASCII workspace names corrupt the download filename.
sanitizeHeaderFileName keeps non-ASCII characters. Header values are interpreted as ISO-8859-1 by browsers, so a workspace named Biologie Résumé yields a mojibake filename. The client already parses filename*=UTF-8'' in WorkspaceTopBar.tsx Line 179. Emit both an ASCII filename and an RFC 5987 filename*.
🛠️ Proposed header change
+function buildContentDisposition(fileName: string) {
+ const ascii = sanitizeHeaderFileName(fileName).replace(/[^\x20-\x7e]/g, "_");
+ return `attachment; filename="${ascii}"; filename*=UTF-8''${encodeURIComponent(fileName)}`;
+} headers: {
"cache-control": "no-store",
- "content-disposition": `attachment; filename="${sanitizeHeaderFileName(archive.fileName)}"`,
+ "content-disposition": buildContentDisposition(archive.fileName),
"content-length": String(archive.body.byteLength),Also applies to: 137-139
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/routes/api/v1/workspaces`.$workspaceId.export.ts around lines 43 - 51,
Update the Content-Disposition header in the export response to emit both an
ASCII-safe filename fallback and an RFC 5987 UTF-8 filename* parameter. Preserve
the sanitized workspace filename for filename*, while deriving a valid ASCII
fallback for filename so non-ASCII names do not become mojibake; keep the
existing attachment behavior and header formatting intact.
| function isLocalDevRequest(request: Request) { | ||
| const hostname = new URL(request.url).hostname; | ||
|
|
||
| return hostname === "localhost" || hostname === "127.0.0.1" || hostname === "0.0.0.0"; | ||
| } | ||
|
|
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== tracked relevant files =="
git ls-files | grep -E '(^|/)(workspaces\.\$workspaceId\.file-upload\.ts|workspace-file-direct-upload\.ts)$' || true
echo
echo "== file sizes =="
for f in \
'src/routes/api/v1/workspaces.$workspaceId.file-upload.ts' \
'src/features/workspaces/upload/workspace-file-direct-upload.ts'
do
if [ -f "$f" ]; then
wc -l "$f"
fi
done
echo
echo "== outline file 1 =="
ast-grep outline 'src/routes/api/v1/workspaces.$workspaceId.file-upload.ts' || true
echo
echo "== lines 1-240 file 1 =="
sed -n '1,240p' 'src/routes/api/v1/workspaces.$workspaceId.file-upload.ts' | cat -n
echo
echo "== lines 380-450 file 1 =="
sed -n '380,450p' 'src/routes/api/v1/workspaces.$workspaceId.file-upload.ts' | cat -n
echo
echo "== relevant lines around workspace-file-direct-upload =="
sed -n '120,190p' 'src/features/workspaces/upload/workspace-file-direct-upload.ts' | cat -n
echo
echo "== searches for isLocalDevRequest/storeLocalDevDirectUpload/getUploadTokenSecret =="
rg -n "isLocalDevRequest|storeLocalDevDirectUpload|getUploadTokenSecret|allowLocalDevUploadProxy|WORKSPACE_UPLOAD_TOKEN_SECRET" src || trueRepository: ThinkEx-OSS/thinkex
Length of output: 19463
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== repository diff/stat summary =="
git diff --stat HEAD~1..HEAD 2>/dev/null || git diff --stat 2>/dev/null || true
echo
echo "== programmatic behavior probe for isLocalDevRequest semantics =="
python3 - <<'PY'
from urllib.parse import urlparse
def is_local_dev_request(url):
hostname = urlparse(url).hostname
return hostname in {"localhost", "127.0.0.1", "0.0.0.0"}
cases = [
"https://localhost:8787/path?data=x",
"https://127.0.0.1:8787/path?data=x",
"https://0.0.0.0:8787/path?data=x",
"https://example.com/path?hostname=localhost&data=x",
"https://attacker.example.com/path?data=x",
]
for url in cases:
print(f"{url}\t-> {is_local_dev_request(url)}")
PY
echo
echo "== structural call path probes using static text =="
python3 - <<'PY'
for path in [
'src/routes/api/v1/workspaces.$workspaceId.file-upload.ts',
'src/routes/api/v1/workspaces.$workspaceId.file-upload.ts',
'src/features/workspaces/upload/workspace-file-direct-upload.ts',
]:
try:
text = open(path, encoding='utf-8').read().splitlines()
except FileNotFoundError:
print(f"MISSING {path}")
continue
for i, line in enumerate(text, 1):
if any(tok in line for tok in ["function isLocalDevRequest", "async function storeLocalDevDirectUpload", "function getUploadTokenSecret"]):
start=max(1,i-3); end=min(len(text), i+25)
print(f"\n--- {path}:{i} ---")
for j in range(start,end+1):
print(f"{j:5d}: {text[j-1]}")
PYRepository: ThinkEx-OSS/thinkex
Length of output: 6919
Gate the local-dev upload bypass on deployment configuration, not the request hostname. isLocalDevRequest enables both storeLocalDevDirectUpload and getUploadTokenSecret’s fallback for upload tokens using new URL(request.url).hostname, which comes from the incoming request. Route this mode through a server-controlled env flag such as env.ALLOW_LOCAL_DEV_UPLOAD_PROXY, keep storeLocalDevDirectUpload behind it, and restrict the shared local-dev upload secret to the same flag. Also consider moving the completionToken used by storeLocalDevDirectUpload out of the URL query string to reduce exposure in request logs.
📍 Affects 2 files
src/routes/api/v1/workspaces.$workspaceId.file-upload.ts#L426-L431(this comment)src/routes/api/v1/workspaces.$workspaceId.file-upload.ts#L117-L164src/features/workspaces/upload/workspace-file-direct-upload.ts#L158-L170
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/routes/api/v1/workspaces`.$workspaceId.file-upload.ts around lines 426 -
431, Replace hostname-based isLocalDevRequest with the server-controlled
env.ALLOW_LOCAL_DEV_UPLOAD_PROXY flag, and use that same flag to gate both
storeLocalDevDirectUpload and getUploadTokenSecret’s shared local-development
secret fallback. In src/routes/api/v1/workspaces.$workspaceId.file-upload.ts
lines 426-431 update the mode check; in lines 117-164 apply the flag to both
upload paths; in src/features/workspaces/upload/workspace-file-direct-upload.ts
lines 158-170 move completionToken out of the URL query string into a less
log-exposed transport while preserving completion behavior.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
13 issues found across 12 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="package.json">
<violation number="1" location="package.json:130">
P3: This new direct dependency isn't imported anywhere in src/ (checked both `prosemirror-transform` and the `@tiptap/pm/transform` re-export path), and the lockfile already carries prosemirror-transform@1.12.0 transitively. If it isn't actually used by the export/PDF code, drop it to avoid an unused dependency and potential version drift; if it is used, add the import and note that it's pinned exactly (no caret) unlike the adjacent `^`/exact-mixed deps.</violation>
</file>
<file name="src/features/workspaces/components/WorkspaceTopBar.tsx">
<violation number="1" location="src/features/workspaces/components/WorkspaceTopBar.tsx:84">
P2: Large workspace exports must fully download and buffer in the tab before the download starts, which can cause substantial client memory pressure or failure. Stream/navigate to the attachment endpoint instead of materializing it as a `Blob`, while preserving an error-handling strategy.</violation>
<violation number="2" location="src/features/workspaces/components/WorkspaceTopBar.tsx:183">
P2: The download filename is decoded with decodeURIComponent, but the server never URL-encodes it. The export route sets `content-disposition: attachment; filename="${sanitizeHeaderFileName(archive.fileName)}"` with a plain (raw, non-RFC5987) value, and getDownloadFileName does not match the filename*=UTF-8'' branch because the server never emits it. sanitizeExportName keeps the % character, so for a workspace named e.g. "100% effort" the filename is `100% effort.zip` and decodeURIComponent("100% effort.zip") throws URIError: URI malformed (verified). That error is caught by the surrounding try/catch, so instead of downloading the ZIP the user gets an error toast even though the server successfully exported the workspace. Recommend not decoding the plain filename="..." value (only decode the RFC 5987 filename* branch), and/or having the server emit RFC 5987 encoding so both sides agree on the format.</violation>
</file>
<file name="src/routes/api/v1/workspaces.$workspaceId.export.ts">
<violation number="1" location="src/routes/api/v1/workspaces.$workspaceId.export.ts:43">
P2: Large exports allocate a second full ZIP-sized buffer immediately before sending the response, increasing worker memory pressure and making otherwise-exportable workspaces fail sooner. Pass `archive.body` to `Response` (or otherwise avoid this copy) and remove `toResponseBody`.</violation>
<violation number="2" location="src/routes/api/v1/workspaces.$workspaceId.export.ts:53">
P3: The 403 branch never runs: a signed-in nonmember reaches `WorkspaceNotFoundError` and receives the 404 response instead. Either throw/use `WorkspaceForbiddenError` for failed membership checks or remove the unreachable 403 handling so API behavior is intentional.</violation>
</file>
<file name="src/features/workspaces/export/workspace-zip.ts">
<violation number="1" location="src/features/workspaces/export/workspace-zip.ts:117">
P2: Exports with 65,536 entries encode an entry count of zero; totals or offsets above 4 GiB also wrap because ZIP32 fields are written without ZIP64 records. Add ZIP64 support or reject unsupported sizes before serialization.</violation>
<violation number="2" location="src/features/workspaces/export/workspace-zip.ts:149">
P2: An entry such as `..\\outside.txt` survives normalization; extractors that treat backslashes as separators can write outside the selected destination. Normalize backslashes before filtering traversal segments.</violation>
</file>
<file name="src/features/workspaces/export/workspace-export.ts">
<violation number="1" location="src/features/workspaces/export/workspace-export.ts:97">
P1: Large exports require several full in-memory copies of every uploaded file, so even a permitted 100 MiB file causes high memory pressure and work before download starts. Stream R2 bodies through a streaming ZIP writer and return that stream instead of collecting `ArrayBuffer` entries and a complete archive.</violation>
<violation number="2" location="src/features/workspaces/export/workspace-export.ts:100">
P2: A file can collide with a folder at extraction time: a sibling folder `foo` plus an uploaded file whose original name is `foo` yields both `foo/` and `foo` in the ZIP. Reserve basename conflicts across file/directory paths, or retain the collision-resolved workspace item name for exported files.</violation>
<violation number="3" location="src/features/workspaces/export/workspace-export.ts:219">
P1: A missing or malformed document checkpoint makes the entire export return 500 instead of producing the remaining assets. Return a fallback/notice PDF from this catch so document-local failures remain best-effort like file failures.</violation>
<violation number="4" location="src/features/workspaces/export/workspace-export.ts:379">
P2: Fallback PDFs lose most normal-length documents when conversion is unavailable because all lines are drawn on one page. Paginate the fallback content and emit a page/content object for each page.</violation>
<violation number="5" location="src/features/workspaces/export/workspace-export.ts:461">
P2: Fallback PDFs silently omit non-ASCII workspace content, including accented text and all non-Latin scripts. Generate the fallback with a Unicode-capable font/encoding rather than filtering these characters.</violation>
</file>
<file name="src/features/workspaces/upload/workspace-file-upload-storage.ts">
<violation number="1" location="src/features/workspaces/upload/workspace-file-upload-storage.ts:114">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**
The new preview-generation fallback behavior introduced in `storeWorkspaceFileUploadPreview` is not exercised by any test. When `createWorkspaceFilePreview` throws, the code now catches the error, records it, and attempts to fall back to the already-stored source object — a meaningful behavioral change that affects what bytes end up under `previewObjectKey`. None of the existing tests in `workspace-file-upload-storage.test.ts` cover the path where the fallback object is found and successfully used as the preview (e.g., for converted uploads where the source was stored in R2). The test `preserves a canonical PDF when validation infrastructure fails` only covers the rethrow path because the mock bucket lacks the object. A regression test that mocks `createWorkspaceFilePreview` to reject and asserts the fallback preview is stored would be practical and valuable.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| } | ||
|
|
||
| entries.push({ | ||
| data: await object.arrayBuffer(), |
There was a problem hiding this comment.
P1: Large exports require several full in-memory copies of every uploaded file, so even a permitted 100 MiB file causes high memory pressure and work before download starts. Stream R2 bodies through a streaming ZIP writer and return that stream instead of collecting ArrayBuffer entries and a complete archive.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/export/workspace-export.ts, line 97:
<comment>Large exports require several full in-memory copies of every uploaded file, so even a permitted 100 MiB file causes high memory pressure and work before download starts. Stream R2 bodies through a streaming ZIP writer and return that stream instead of collecting `ArrayBuffer` entries and a complete archive.</comment>
<file context>
@@ -0,0 +1,477 @@
+ }
+
+ entries.push({
+ data: await object.arrayBuffer(),
+ modifiedAt: new Date(item.updatedAt),
+ path: reserveUniqueZipPath(
</file context>
| workspace_id: item.workspaceId, | ||
| }, | ||
| }); | ||
| throw new Error("Unable to render a workspace document for export."); |
There was a problem hiding this comment.
P1: A missing or malformed document checkpoint makes the entire export return 500 instead of producing the remaining assets. Return a fallback/notice PDF from this catch so document-local failures remain best-effort like file failures.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/export/workspace-export.ts, line 219:
<comment>A missing or malformed document checkpoint makes the entire export return 500 instead of producing the remaining assets. Return a fallback/notice PDF from this catch so document-local failures remain best-effort like file failures.</comment>
<file context>
@@ -0,0 +1,477 @@
+ workspace_id: item.workspaceId,
+ },
+ });
+ throw new Error("Unable to render a workspace document for export.");
+ }
+}
</file context>
| ); | ||
| } | ||
|
|
||
| const blob = await response.blob(); |
There was a problem hiding this comment.
P2: Large workspace exports must fully download and buffer in the tab before the download starts, which can cause substantial client memory pressure or failure. Stream/navigate to the attachment endpoint instead of materializing it as a Blob, while preserving an error-handling strategy.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/WorkspaceTopBar.tsx, line 84:
<comment>Large workspace exports must fully download and buffer in the tab before the download starts, which can cause substantial client memory pressure or failure. Stream/navigate to the attachment endpoint instead of materializing it as a `Blob`, while preserving an error-handling strategy.</comment>
<file context>
@@ -61,7 +63,41 @@ export default function WorkspaceTopBar({
+ );
+ }
+
+ const blob = await response.blob();
+ const objectUrl = URL.createObjectURL(blob);
+ const link = document.createElement("a");
</file context>
| userId: session.user.id, | ||
| }); | ||
|
|
||
| return new Response(toResponseBody(archive.body), { |
There was a problem hiding this comment.
P2: Large exports allocate a second full ZIP-sized buffer immediately before sending the response, increasing worker memory pressure and making otherwise-exportable workspaces fail sooner. Pass archive.body to Response (or otherwise avoid this copy) and remove toResponseBody.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/routes/api/v1/workspaces.$workspaceId.export.ts, line 43:
<comment>Large exports allocate a second full ZIP-sized buffer immediately before sending the response, increasing worker memory pressure and making otherwise-exportable workspaces fail sooner. Pass `archive.body` to `Response` (or otherwise avoid this copy) and remove `toResponseBody`.</comment>
<file context>
@@ -0,0 +1,155 @@
+ userId: session.user.id,
+ });
+
+ return new Response(toResponseBody(archive.body), {
+ headers: {
+ "cache-control": "no-store",
</file context>
| const view = new DataView(output.buffer); | ||
|
|
||
| view.setUint32(0, 0x06054b50, true); | ||
| view.setUint16(8, input.entryCount, true); |
There was a problem hiding this comment.
P2: Exports with 65,536 entries encode an entry count of zero; totals or offsets above 4 GiB also wrap because ZIP32 fields are written without ZIP64 records. Add ZIP64 support or reject unsupported sizes before serialization.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/export/workspace-zip.ts, line 117:
<comment>Exports with 65,536 entries encode an entry count of zero; totals or offsets above 4 GiB also wrap because ZIP32 fields are written without ZIP64 records. Add ZIP64 support or reject unsupported sizes before serialization.</comment>
<file context>
@@ -0,0 +1,189 @@
+ const view = new DataView(output.buffer);
+
+ view.setUint32(0, 0x06054b50, true);
+ view.setUint16(8, input.entryCount, true);
+ view.setUint16(10, input.entryCount, true);
+ view.setUint32(12, input.centralDirectorySize, true);
</file context>
| const objects = [ | ||
| "<< /Type /Catalog /Pages 2 0 R >>", | ||
| "<< /Type /Pages /Kids [3 0 R] /Count 1 >>", | ||
| "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] /Resources << /Font << /F1 4 0 R >> >> /Contents 5 0 R >>", |
There was a problem hiding this comment.
P2: Fallback PDFs lose most normal-length documents when conversion is unavailable because all lines are drawn on one page. Paginate the fallback content and emit a page/content object for each page.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/export/workspace-export.ts, line 379:
<comment>Fallback PDFs lose most normal-length documents when conversion is unavailable because all lines are drawn on one page. Paginate the fallback content and emit a page/content object for each page.</comment>
<file context>
@@ -0,0 +1,477 @@
+ const objects = [
+ "<< /Type /Catalog /Pages 2 0 R >>",
+ "<< /Type /Pages /Kids [3 0 R] /Count 1 >>",
+ "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] /Resources << /Font << /F1 4 0 R >> >> /Contents 5 0 R >>",
+ "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica >>",
+ `<< /Length ${content.length} >>\nstream\n${content}\nendstream`,
</file context>
| contentDisposition?.match(/filename="([^"]+)"/i) ?? | ||
| contentDisposition?.match(/filename=([^;]+)/i); | ||
|
|
||
| return match?.[1] ? decodeURIComponent(match[1].trim()) : null; |
There was a problem hiding this comment.
P2: The download filename is decoded with decodeURIComponent, but the server never URL-encodes it. The export route sets content-disposition: attachment; filename="${sanitizeHeaderFileName(archive.fileName)}" with a plain (raw, non-RFC5987) value, and getDownloadFileName does not match the filename*=UTF-8'' branch because the server never emits it. sanitizeExportName keeps the % character, so for a workspace named e.g. "100% effort" the filename is 100% effort.zip and decodeURIComponent("100% effort.zip") throws URIError: URI malformed (verified). That error is caught by the surrounding try/catch, so instead of downloading the ZIP the user gets an error toast even though the server successfully exported the workspace. Recommend not decoding the plain filename="..." value (only decode the RFC 5987 filename* branch), and/or having the server emit RFC 5987 encoding so both sides agree on the format.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/WorkspaceTopBar.tsx, line 183:
<comment>The download filename is decoded with decodeURIComponent, but the server never URL-encodes it. The export route sets `content-disposition: attachment; filename="${sanitizeHeaderFileName(archive.fileName)}"` with a plain (raw, non-RFC5987) value, and getDownloadFileName does not match the filename*=UTF-8'' branch because the server never emits it. sanitizeExportName keeps the % character, so for a workspace named e.g. "100% effort" the filename is `100% effort.zip` and decodeURIComponent("100% effort.zip") throws URIError: URI malformed (verified). That error is caught by the surrounding try/catch, so instead of downloading the ZIP the user gets an error toast even though the server successfully exported the workspace. Recommend not decoding the plain filename="..." value (only decode the RFC 5987 filename* branch), and/or having the server emit RFC 5987 encoding so both sides agree on the format.</comment>
<file context>
@@ -126,3 +172,22 @@ export default function WorkspaceTopBar({
+ contentDisposition?.match(/filename="([^"]+)"/i) ??
+ contentDisposition?.match(/filename=([^;]+)/i);
+
+ return match?.[1] ? decodeURIComponent(match[1].trim()) : null;
+}
+
</file context>
| body: object.body, | ||
| contentType: upload.contentType, | ||
| sizeBytes: object.size, | ||
| }).catch(async (error) => { |
There was a problem hiding this comment.
P2: Custom agent: Flag AI Slop and Fabricated Changes
The new preview-generation fallback behavior introduced in storeWorkspaceFileUploadPreview is not exercised by any test. When createWorkspaceFilePreview throws, the code now catches the error, records it, and attempts to fall back to the already-stored source object — a meaningful behavioral change that affects what bytes end up under previewObjectKey. None of the existing tests in workspace-file-upload-storage.test.ts cover the path where the fallback object is found and successfully used as the preview (e.g., for converted uploads where the source was stored in R2). The test preserves a canonical PDF when validation infrastructure fails only covers the rethrow path because the mock bucket lacks the object. A regression test that mocks createWorkspaceFilePreview to reject and asserts the fallback preview is stored would be practical and valuable.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/upload/workspace-file-upload-storage.ts, line 114:
<comment>The new preview-generation fallback behavior introduced in `storeWorkspaceFileUploadPreview` is not exercised by any test. When `createWorkspaceFilePreview` throws, the code now catches the error, records it, and attempts to fall back to the already-stored source object — a meaningful behavioral change that affects what bytes end up under `previewObjectKey`. None of the existing tests in `workspace-file-upload-storage.test.ts` cover the path where the fallback object is found and successfully used as the preview (e.g., for converted uploads where the source was stored in R2). The test `preserves a canonical PDF when validation infrastructure fails` only covers the rethrow path because the mock bucket lacks the object. A regression test that mocks `createWorkspaceFilePreview` to reject and asserts the fallback preview is stored would be practical and valuable.</comment>
<file context>
@@ -110,6 +111,21 @@ async function storeWorkspaceFileUploadPreview(
body: object.body,
contentType: upload.contentType,
sizeBytes: object.size,
+ }).catch(async (error) => {
+ recordWorkspaceUploadPreviewFallback({
+ error,
</file context>
| "partyserver": "^0.5.8", | ||
| "posthog-js": "^1.396.6", | ||
| "posthog-node": "^5.39.4", | ||
| "prosemirror-transform": "1.12.0", |
There was a problem hiding this comment.
P3: This new direct dependency isn't imported anywhere in src/ (checked both prosemirror-transform and the @tiptap/pm/transform re-export path), and the lockfile already carries prosemirror-transform@1.12.0 transitively. If it isn't actually used by the export/PDF code, drop it to avoid an unused dependency and potential version drift; if it is used, add the import and note that it's pinned exactly (no caret) unlike the adjacent ^/exact-mixed deps.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At package.json, line 130:
<comment>This new direct dependency isn't imported anywhere in src/ (checked both `prosemirror-transform` and the `@tiptap/pm/transform` re-export path), and the lockfile already carries prosemirror-transform@1.12.0 transitively. If it isn't actually used by the export/PDF code, drop it to avoid an unused dependency and potential version drift; if it is used, add the import and note that it's pinned exactly (no caret) unlike the adjacent `^`/exact-mixed deps.</comment>
<file context>
@@ -127,6 +127,7 @@
"partyserver": "^0.5.8",
"posthog-js": "^1.396.6",
"posthog-node": "^5.39.4",
+ "prosemirror-transform": "1.12.0",
"react": "^19.2.7",
"react-dom": "^19.2.7",
</file context>
| }, | ||
| }); | ||
| } catch (error) { | ||
| if (error instanceof WorkspaceForbiddenError) { |
There was a problem hiding this comment.
P3: The 403 branch never runs: a signed-in nonmember reaches WorkspaceNotFoundError and receives the 404 response instead. Either throw/use WorkspaceForbiddenError for failed membership checks or remove the unreachable 403 handling so API behavior is intentional.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/routes/api/v1/workspaces.$workspaceId.export.ts, line 53:
<comment>The 403 branch never runs: a signed-in nonmember reaches `WorkspaceNotFoundError` and receives the 404 response instead. Either throw/use `WorkspaceForbiddenError` for failed membership checks or remove the unreachable 403 handling so API behavior is intentional.</comment>
<file context>
@@ -0,0 +1,155 @@
+ },
+ });
+ } catch (error) {
+ if (error instanceof WorkspaceForbiddenError) {
+ return apiError(
+ requestId,
</file context>
1 participant
Adds workspace export support with a new Export button that downloads the workspace as a ZIP. The export includes documents as PDFs, uploaded files, folders, safe ZIP paths, and graceful fallbacks for missing local file data or unavailable local PDF conversion.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by CodeRabbit