Repository navigation
fix(gridfs): fence file-id ownership across uploads, finalize, abort and delete [WIP] - #12
Conversation
rockyshi1993
left a comment
There was a problem hiding this comment.
Independent merge gate — unresolved P1 crash recovery. Do not merge yet.
Code evidence on current head: src/capabilities/gridfs/resumable.ts returns GRIDFS_UPLOAD_BUSY for status:'finalizing' when no files document exists, and for status:'aborting' unconditionally; src/capabilities/gridfs/ownership.ts only implements claim/release/cleanup and has no supported recovery/inspection for stale direct, delete, activeAppend, finalizing or aborting records. Since these intentionally have no TTL, a process crash or ambiguous network failure can strand an upload ID indefinitely. The approved V0.1 explicitly requires safe, audited stale owner recovery after the old writer is stopped. Add a documented recovery path, deterministic fail-after-CAS/crash tests, and test explicit idempotent abort/cleanup under a retained reservation. Also get exact-head latest Node18/20/22, MongoDB7/8, Driver6/7 and preflight CI verdicts; current newest jobs are still queued. Green MongoDB 8 initial 32/32 from an older SHA is insufficient for merge. No production release authorized.
|
Merge gate update: P1 crash/ambiguous-write recovery is being addressed in draft PR #13 on isolated branch |
Merge gate — verified recovery follow-up is still pendingThe P1 crash-recovery follow-up remains draft PR #13, based on PR #12's feature branch (not on New independent-review fixes in #13:
Latest HEAD checks remain queued, not passed: child CI and MongoDB 7/8 safety matrix. Earlier canceled/green jobs belong to different SHAs and cannot serve as the merge gate. There is no verified Node18/20/22 + MongoDB7/8 + applicable Driver6/7 + release preflight verdict for Strict sequence: independent review and all child exact-HEAD checks green -> merge #13 into |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 32cfee1. Configure here.
| nextChunk: 0, | ||
| createdAt: now, | ||
| updatedAt: now, | ||
| expiresAt: new Date(now.getTime() + 604800 * 1000), |
There was a problem hiding this comment.
Begin wipes chunks after stale preflight
High Severity
beginResumableUpload treats the preflight files/uploads reads as proof that no file exists, then deletes every chunk for that id after inserting the session. A concurrent upload/uploadStream can finish and release its reservation during those awaits, so this deleteMany can strip chunks from a now-visible file and leave a corrupt files document.
Reviewed by Cursor Bugbot for commit 32cfee1. Configure here.
| if (!transitioned.matchedCount) throw uploadBusy('finalizeResumableUpload', ctx, uploadId); | ||
| // Only the winner of this CAS may inspect/commit files. Abort will now | ||
| // fail its pending -> aborting CAS instead of deleting validated chunks. | ||
| session = await requireSession(ctx, uploadId, 'finalizeResumableUpload'); |
There was a problem hiding this comment.
Finalize conflict leaves session frozen
Medium Severity
After the pending -> finalizing CAS, committedFile errors such as GRIDFS_FILE_EXISTS or GRIDFS_INTEGRITY_MISMATCH propagate without restoring pending. Chunk-validation failures do roll back. The leftover finalizing row then makes abortResumableUpload and delete return GRIDFS_UPLOAD_BUSY, so a conflicting visible file cannot be cleared through the public API.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 32cfee1. Configure here.
| } | ||
| } finally { | ||
| // Rename never removes chunks and is safe to unlock after driver settles. | ||
| await releaseOwner(ctx, owner, 'rename'); |
There was a problem hiding this comment.
Rename release overwrites the real result
Medium Severity
renameFile always awaits releaseOwner in finally. If that release throws, it replaces a successful rename or a GRIDFS_FILE_NOT_FOUND with GRIDFS_UPLOAD_BUSY. Upload already treats post-commit release failure as a warning and still returns success; rename does not, so callers see failure while the filename change may already have landed and the reservation stays stuck.
Reviewed by Cursor Bugbot for commit 32cfee1. Configure here.


GridFS V0.1 lifecycle repair (draft — not mergeable yet)
Base:
mainv3.5.3. Branch:fix/gridfs-id-ownership-v01.Problems: MZ-15, MZ-16, MZ-18 (deterministically reproduced on original main, 4/4), plus MZ-04 TTL and MZ-14 matrix.
Production changes in this PR
<bucket>.uploadsacross upload / uploadStream / resumable begin / delete / rename / known orphan cleanup.pending -> finalizing/pending -> aborting.createdAtTTL to seven-day idleexpiresAt; active operations have no TTL to avoid stale writers resuming after ownership expires.GRIDFS_UPLOAD_BUSYfor ID contention.Regression coverage
Verified evidence so far
createdAtTTL collision; after expiry migration this no longer errored and left a test stream waiting. The test has now been corrected to conflict with the newexpiresAtTTL.Release blockers — DO NOT MERGE
No PR merge and no npm publish are authorized by this draft.
Note
High Risk
Changes GridFS write/delete/consistency paths and requires coordinated writer drain for TTL migration; incorrect recovery or mixed-version deploys could corrupt or lose file data.
Overview
Introduces durable per-file-ID ownership in
<bucket>.uploads, serializing upload, uploadStream, resumable append/finalize/abort, delete, rename, and orphan cleanup. Conflicting operations now surfaceGRIDFS_UPLOAD_BUSYinstead of racing on chunks.Resumable lifecycle adds
activeAppendtokens, CAS transitions (pending→finalizing/aborting), and chunk re-verification on ambiguous finalize retries (metadata SHA alone is not trusted).ensureIndexesmigrates legacycreatedAtTTL to seven-day idleexpiresAtfor pending sessions only; active claims stay untimed.An operator-only
gridfs-recover.cjsplus EN/ZH runbooks support dry-run/apply recovery after verified writer drain. CI gains a GridFS MongoDB 7/8 matrix workflow, expanded integration suites, concurrency cancellation, and raised bundle budgets (~4–5 KiB) for the new code.Reviewed by Cursor Bugbot for commit 32cfee1. Bugbot is set up for automated code reviews on this repo. Configure here.