feat: atomic file upload with server-side quota tracking#56
Merged
Conversation
Phase 7.1: Atomic File Upload - Traced full upload flow across 13+ frontend and backend files - Identified per-file IPNS publish as biggest latency bottleneck - Recommended: new POST /ipfs/upload endpoint (pin + DB record) - Recommended: addFilesToFolder batch function (single IPNS publish) - No new dependencies needed - pure refactoring of existing code - Documented 7 pitfalls including quota double-counting and stale state Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Phase 7.1: Atomic File Upload - 2 plans in 2 waves - Plan 01 (Wave 1): Backend atomic upload endpoint with quota check + pin recording - Plan 02 (Wave 2): Frontend batch upload flow with single IPNS publish - Ready for execution Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Create UploadResponseDto with cid, size, recorded fields - Add POST /ipfs/upload endpoint: checks quota, pins to IPFS, records pin - Import VaultModule into IpfsModule for VaultService access - Throw PayloadTooLargeException when storage quota exceeded - Update controller tests with VaultService mock and 3 upload tests - Regenerate API client with new upload endpoint types Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Tasks completed: 2/2 - Task 1: Create UploadResponseDto and POST /ipfs/upload endpoint - Task 2: Regenerate API client with new upload types SUMMARY: .planning/phases/07.1-atomic-file-upload/07.1-01-SUMMARY.md Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…registration - addToIpfs now posts to /ipfs/upload (atomic pin + quota record) - addFilesToFolder batch function publishes IPNS once for N files - Upload store adds 'registering' status for metadata registration phase - Quota refreshed from server after upload (fetchQuota replaces per-file addUsage) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- useFolder exposes addFiles (batch) alongside existing addFile (single) - UploadZone uses batch addFiles with single IPNS publish per upload - EmptyState uses batch addFiles with single IPNS publish per upload - useFileUpload isUploading includes registering status - setRegistering called before batch folder metadata registration Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Tasks completed: 2/2 - Switch upload API to atomic endpoint, add batch folder registration, update quota flow - Wire batch upload flow into hooks and components SUMMARY: .planning/phases/07.1-atomic-file-upload/07.1-02-SUMMARY.md Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add Playwright MCP automation-first rule to verify-work workflow. Tests 1-5 pass via automated Playwright verification. Test 6 (folder ops) blocked by upload modal overlay - pending. 4 pre-existing glitches documented (not 7.1 regressions). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Final UAT test (folder metadata operations) passed via Playwright. Phase completion doc captures 3 pre-existing bugs discovered during testing: upload modal stuck, auth refresh race, IPNS resolve 502. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Upload lifecycle gaps, modal dismiss patterns, async dedup, and IPNS reliability lessons from investigating 4 pre-existing bugs surfaced during automated UAT testing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (6)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the
WalkthroughAdds a quota-aware POST /ipfs/upload endpoint on the backend and front-end support for batch file registration: new UploadResponseDto, VaultService integration for quota checks/recording, and frontend changes to upload/register multiple files with a new 'registering' state. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant WebClient as Web Client
participant VaultSvc as Vault Service
participant IPFSSvc as IPFS Provider
participant Storage as Storage/DB
User->>WebClient: POST /ipfs/upload (multipart file)
WebClient->>VaultSvc: checkQuota(userId, size?)
alt quota available
VaultSvc-->>WebClient: allowed
WebClient->>IPFSSvc: pinFile(file)
IPFSSvc-->>WebClient: { cid, size }
WebClient->>VaultSvc: recordPin(userId, cid, size)
VaultSvc->>Storage: persist pin/quota
Storage-->>VaultSvc: persisted
VaultSvc-->>WebClient: recorded=true
WebClient-->>User: 201 UploadResponseDto(cid, size, recorded=true)
else quota exceeded
VaultSvc-->>WebClient: allowed=false
WebClient-->>User: 413 Payload Too Large
else record fails
VaultSvc-->>WebClient: allowed
WebClient->>IPFSSvc: pinFile(file)
IPFSSvc-->>WebClient: { cid, size }
WebClient->>VaultSvc: recordPin(...) fails
WebClient->>IPFSSvc: unpinFile(cid)
WebClient-->>User: error (record failure)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #56 +/- ##
==========================================
- Coverage 88.94% 88.88% -0.06%
==========================================
Files 34 34
Lines 1058 1071 +13
Branches 199 203 +4
==========================================
+ Hits 941 952 +11
Misses 72 72
- Partials 45 47 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/web/src/services/upload.service.ts (1)
123-134:⚠️ Potential issue | 🟡 MinorRefresh quota even when a batch fails mid-way.
Right now the refresh only happens after full success; if any file fails after some uploads, UI quota can drift. Consider moving the refresh tofinallyso it runs after partial success too.💡 Suggested change (refresh in finally)
- // Refresh quota from server (authoritative source after atomic upload) - await useQuotaStore.getState().fetchQuota(); - - uploadStore.setSuccess(); - return results; - } catch (error) { + uploadStore.setSuccess(); + return results; + } catch (error) { const message = (error as Error).message; if (message !== 'Upload cancelled by user') { uploadStore.setError(message); console.error('Upload failed:', error); } throw error; + } finally { + // Refresh quota from server (authoritative source after atomic upload) + await useQuotaStore.getState().fetchQuota(); }apps/web/src/lib/api/ipfs.ts (1)
14-25:⚠️ Potential issue | 🟡 MinorUse
UploadResponseDtoto capture therecordedfield from/ipfs/uploadresponse.The endpoint returns a
recordedboolean flag indicating quota tracking status, but theAddResponsetype omits it, preventing callers from accessing this information. Replace the manualAddResponsetype with the generatedUploadResponseDtoto align with the actual API contract.🔧 Suggested fix
-import axios, { AxiosProgressEvent, CancelToken } from 'axios'; +import axios, { AxiosProgressEvent, CancelToken } from 'axios'; +import type { UploadResponseDto } from '../../api/models'; -export type AddResponse = { cid: string; size: number }; +export type AddResponse = UploadResponseDto; -export async function addToIpfs( +export async function addToIpfs( encryptedFile: Blob, onProgress?: (percent: number) => void, cancelToken?: CancelToken -): Promise<AddResponse> { +): Promise<UploadResponseDto> { const { accessToken } = useAuthStore.getState(); const formData = new FormData(); formData.append('file', encryptedFile); - const response = await axios.post<AddResponse>(`${BASE_URL}/ipfs/upload`, formData, { + const response = await axios.post<UploadResponseDto>(`${BASE_URL}/ipfs/upload`, formData, {apps/web/src/components/file-browser/UploadZone.tsx (1)
70-95:⚠️ Potential issue | 🟠 MajorClear the registering state after batch metadata registration.
setRegistering()is invoked, but there’s no guaranteed terminal state on success or failure; ifaddFilesthrows, the upload UI can remain stuck in “registering.” Add a success/error (or finally) transition to clear the state.
🤖 Fix all issues with AI agents
In `@apps/api/src/ipfs/ipfs.controller.spec.ts`:
- Around line 137-170: Replace the loose "as any" assertion on mockReq with a
proper Request-typed mock: create mockReq as a typed object matching
Express.Request (or Partial<Request>) that includes the user property (e.g.,
type it as Partial<Request> & { user: { id: string } } and cast to Request if
needed) so the spec uses an explicit type instead of any; update the mock
declaration referenced as mockReq used in the controller.upload tests to use
that explicit type.
In `@apps/api/src/ipfs/ipfs.controller.ts`:
- Around line 145-149: The current flow checks quota with
vaultService.checkQuota(req.user.id, file.size), then calls
ipfsProvider.pinFile(...) and vaultService.recordPin(...), which can race and
use mismatched sizes; instead, pin first to obtain result.size, then perform a
single atomic quota validation+record inside VaultService (add a method like
recordPinWithQuotaCheck(userId, cid, size) that validates using result.size and
records within a DB transaction), and if that method throws, ensure the
controller calls ipfsProvider.unpin(result.cid) to rollback the pin; update the
controller to use the new VaultService transactional method and remove the
separate checkQuota call so quota logic and record are atomic and consistent.
Comment on lines
+145
to
+149
| const hasQuota = await this.vaultService.checkQuota(req.user.id, file.size); | ||
| if (!hasQuota) throw new PayloadTooLargeException('Storage quota exceeded'); | ||
| const result = await this.ipfsProvider.pinFile(file.buffer); | ||
| await this.vaultService.recordPin(req.user.id, result.cid, result.size); | ||
| return { cid: result.cid, size: result.size, recorded: true }; |
There was a problem hiding this comment.
Make quota enforcement truly atomic and rollback on failure.
Line 145 performs a pre-check, then pinFile/recordPin proceed independently. Concurrent uploads can pass the check and exceed quota, and if recordPin fails the file stays pinned but unrecorded. Also, the check uses file.size while the recorded size uses result.size, which can diverge. Consider moving quota validation + record into a single VaultService transaction (using result.size) and unpin on failure.
🛠️ Possible rollback guard in the controller
- const result = await this.ipfsProvider.pinFile(file.buffer);
- await this.vaultService.recordPin(req.user.id, result.cid, result.size);
+ const result = await this.ipfsProvider.pinFile(file.buffer);
+ try {
+ await this.vaultService.recordPin(req.user.id, result.cid, result.size);
+ } catch (err) {
+ await this.ipfsProvider.unpinFile(result.cid).catch(() => undefined);
+ throw err;
+ }📝 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.
Suggested change
| const hasQuota = await this.vaultService.checkQuota(req.user.id, file.size); | |
| if (!hasQuota) throw new PayloadTooLargeException('Storage quota exceeded'); | |
| const result = await this.ipfsProvider.pinFile(file.buffer); | |
| await this.vaultService.recordPin(req.user.id, result.cid, result.size); | |
| return { cid: result.cid, size: result.size, recorded: true }; | |
| const hasQuota = await this.vaultService.checkQuota(req.user.id, file.size); | |
| if (!hasQuota) throw new PayloadTooLargeException('Storage quota exceeded'); | |
| const result = await this.ipfsProvider.pinFile(file.buffer); | |
| try { | |
| await this.vaultService.recordPin(req.user.id, result.cid, result.size); | |
| } catch (err) { | |
| await this.ipfsProvider.unpinFile(result.cid).catch(() => undefined); | |
| throw err; | |
| } | |
| return { cid: result.cid, size: result.size, recorded: true }; |
🤖 Prompt for AI Agents
In `@apps/api/src/ipfs/ipfs.controller.ts` around lines 145 - 149, The current
flow checks quota with vaultService.checkQuota(req.user.id, file.size), then
calls ipfsProvider.pinFile(...) and vaultService.recordPin(...), which can race
and use mismatched sizes; instead, pin first to obtain result.size, then perform
a single atomic quota validation+record inside VaultService (add a method like
recordPinWithQuotaCheck(userId, cid, size) that validates using result.size and
records within a DB transaction), and if that method throws, ensure the
controller calls ipfsProvider.unpin(result.cid) to rollback the pin; update the
controller to use the new VaultService transactional method and remove the
separate checkQuota call so quota logic and record are atomic and consistent.
…fresh - Add rollback (unpin) if recordPin fails after successful IPFS pin - Call setSuccess() after addFiles in UploadZone/EmptyState to dismiss modal - Move quota refresh to finally block for partial failure resilience - Add recorded field to AddResponse type to match UploadResponseDto - Remove `as any` on mockReq in controller spec (lint fix) - Add test for recordPin failure rollback behavior Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/components/file-browser/UploadZone.tsx (1)
77-96:⚠️ Potential issue | 🟠 MajorMissing store error state when
addFilesfails leaves UI stuck in "registering".If
addFilesthrows after successful upload, the catch block sets local error but never updates the upload store. The store remains in'registering'status indefinitely, which can cause the upload modal to get stuck (as noted in PR known issues).🐛 Proposed fix to reset store on addFiles failure
await addFiles( folderId, uploadedFiles.map((uploaded) => ({ cid: uploaded.cid, wrappedKey: uploaded.wrappedKey, iv: uploaded.iv, originalName: uploaded.originalName, originalSize: uploaded.originalSize, })) ); useUploadStore.getState().setSuccess(); onUploadComplete?.(); } catch (err) { // Error is already set in upload store // Only set local error if not a cancellation if ((err as Error).message !== 'Upload cancelled by user') { + useUploadStore.getState().setError((err as Error).message); setError((err as Error).message); } }
- Upload modal no dismiss button (ui) - Upload button text stuck on "uploading..." (ui) - Auth refresh race condition with parallel 401s (api) - IPNS resolve 502 needs DB-cached CID fallback (api) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Upload store stays in 'registering' if batch folder registration throws, leaving the modal stuck. Needs setError() in catch block. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This was referenced Feb 10, 2026
This was referenced Feb 18, 2026
8 tasks
Closed
This was referenced Mar 25, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
POST /ipfs/uploadendpoint that atomically pins files to IPFS and records quota in a single server-side transaction, replacing the previous two-step client flow (pin → record) that was prone to quota driftuseFileUploadhook and upload store state managementPayloadTooLargeExceptionwhen upload would exceed vault storage limitChanges
API (
apps/api)POST /ipfs/upload— multipart file upload with atomic pin + quota recordUploadResponseDtowith CID, size, and recording statusVaultServiceinjected intoIpfsModulefor quota checksWeb (
apps/web)/ipfs/uploadendpointuseFolderhook after successful uploadsUploadZoneandEmptyStatecomponents updated for new flowGenerated
Known Issues (documented in UAT)
setSuccess()isn't called (fix identified, not yet applied)Test Plan
ipfs.controller.spec.ts)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Enhancements