Refactor virtual drive sync engine#405
Open
egalvis27 wants to merge 6 commits into
Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR adds lazy virtual-drive hydration with SQLite-backed directory state, refactors temporal uploads into an async queue with staged files, updates FUSE operations to hydrate paths on demand, wraps SQLite batch upserts in transactions, and adjusts remote-sync fetching, startup wiring, and DI registration. ChangesVirtual drive hydration, upload queue, and sync flow
Estimated code review effort: 4 (Complex) | ~75 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/backend/features/virtual-drive/ipc/handlers.ts (1)
14-24: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winHandle the fire-and-forget failure path.
void Promise.all([...])drops rejections from either async call, so a failure inupdateVirtualDriveContainerorDirectoryStateSqliteRepository.clear()becomes an unhandled promise rejection.🤖 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/backend/features/virtual-drive/ipc/handlers.ts` around lines 14 - 24, The fire-and-forget logic in remoteChangesSyncedHandler currently ignores failures from both updateVirtualDriveContainer and DirectoryStateSqliteRepository.clear(), which can surface as unhandled rejections. Update this handler to explicitly catch and log errors from the Promise.all work, using the existing remoteChangesSyncedHandler, updateVirtualDriveContainer, and DirectoryStateSqliteRepository.clear symbols so the failure path is handled safely without dropping rejected promises.
🧹 Nitpick comments (5)
src/backend/features/virtual-drive/services/operations/opendir.service.test.ts (1)
57-59: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest only covers rejection with an already-well-formed
FuseError.This masks the cast issue in
opendir.service.ts(error: err as FuseError) — add a case wherehydrator.readDirectoryrejects with a plainErrorto verify the service still returns a validFuseErrorwith a correct.code.🤖 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/backend/features/virtual-drive/services/operations/opendir.service.test.ts` around lines 57 - 59, The existing opendir service test only exercises a rejection that is already a FuseError, so it does not catch the unsafe cast in opendir.service.ts. Extend the test in opendir.service.test.ts around hydrator.readDirectory to also reject with a plain Error and assert the service still resolves to a FuseError with the expected code. Use the opendir service path and the FuseError/FuseCodes assertions to verify the fallback behavior when err is not already a FuseError.src/backend/features/virtual-drive/services/operations/get-attributes.service.ts (1)
86-118: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate file/folder attribute-mapping logic.
The hydrated-entry file/folder attribute blocks (Lines 89-117) closely mirror the earlier local-entry blocks (Lines 54-82). Consider extracting
toFileAttributes(file)/toFolderAttributes(folder)helpers to avoid drift between the two near-identical mappings.🤖 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/backend/features/virtual-drive/services/operations/get-attributes.service.ts` around lines 86 - 118, The file and folder attribute mapping in GetAttributesService is duplicated between the local-entry and hydrated-entry branches, which risks drift over time. Refactor the shared logic into reusable helpers such as toFileAttributes and toFolderAttributes (or equivalent methods near findLocalEntry / ensurePathLoaded) and have both branches call them so the mode, size, timestamps, uid/gid, and nlink mapping stays consistent in one place.src/backend/features/virtual-drive/services/operations/get-attributes.service.test.ts (1)
75-84: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing coverage for hydrated-entry early-return branches.
The new logic in
getAttributes(implementation Lines 86-117) re-resolvesfindLocalEntryafter hydration and returns early if a file/folder is now found — this is the core new behavior for this layer. This test only exercises the path where hydration finds nothing and falls back todocument. Add cases wherefileSearcher.run/folderSearcher.runresolve to a value only afterhydrator.ensurePathLoadedis called, to verify the hydrated early-return paths.🤖 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/backend/features/virtual-drive/services/operations/get-attributes.service.test.ts` around lines 75 - 84, The current getAttributes test only covers the fallback path after hydration, but it misses the new early-return behavior after re-running findLocalEntry. Add test cases in get-attributes.service.test.ts where hydrator.ensurePathLoaded is invoked and then fileSearcher.run or folderSearcher.run resolves to a hydrated file/folder, and assert getAttributes returns those attributes immediately instead of continuing to document fallback. Use the existing getAttributes, hydrator.ensurePathLoaded, fileSearcher.run, and folderSearcher.run symbols to target the new hydrated-entry branches.src/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue.ts (1)
12-22: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
PropsandUploadTaskare structurally identical.Consider reusing one type to avoid duplication as the shape evolves.
🤖 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/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue.ts` around lines 12 - 22, Props and UploadTask are duplicated with the same shape, so reuse a single type definition in TemporalFileUploadQueue to avoid drift as fields change. Update the queue’s type declarations so one of the symbols, Props or UploadTask, becomes the shared source of truth and the other references it directly.src/apps/drive/dependency-injection/virtual-drive/registerFolderServices.ts (1)
28-30: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe
// TODO: can be private?comment on line 28 is now stale — this binding is intentionally made non-private so the lazy hydrator can resolveFolderRepository. Consider removing the TODO to avoid confusion.🤖 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/apps/drive/dependency-injection/virtual-drive/registerFolderServices.ts` around lines 28 - 30, The TODO comment in registerFolderServices is stale because the FolderRepository binding is intentionally public for lazy hydrator resolution. Remove the “can be private?” comment from the builder.register(FolderRepository).use(InMemoryFolderRepository).asSingleton() registration so the intent is clear and there’s no confusion for future readers.
🤖 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/apps/main/database/data-source.ts`:
- Around line 41-49: `initializeVirtualDriveSqlite()` is currently
all-or-nothing, so a single failing bootstrap statement can stop startup and
prevent the `APP_DATA_SOURCE_INITIALIZED` emit from happening. Update this
helper to make `SQLITE_BOOTSTRAP_STATEMENTS` best-effort by handling errors per
statement inside the loop, logging failures with context via
`AppDataSource.query`, and continuing instead of throwing so the virtual-drive
startup path can proceed.
In
`@src/backend/features/virtual-drive/services/lazy/LazyVirtualDriveHydrator.ts`:
- Around line 51-52: The `readLocalDirectory` call in `LazyVirtualDriveHydrator`
is passing an unused `statusScope` argument, which is misleading and flagged by
lint. Remove the `statusScope` parameter from the `readLocalDirectory` signature
and update the `LazyVirtualDriveHydrator` call site (and any other referenced
overloads around the same code path) so `folder` is the only needed argument,
keeping the behavior unchanged.
- Around line 79-96: The loop in LazyVirtualDriveHydrator is intentionally
sequential, so the no-await-in-loop lint warning should be handled explicitly
rather than left unresolved. Add a targeted lint disable comment on the await
inside the path-segment loop in LazyVirtualDriveHydrator, and include a brief
justification that each segment depends on the previous folder being hydrated
before continuing.
- Around line 127-196: The delete order in fetchAndStoreChildren is unsafe
because folderRepository.deleteMatchingPartial and
fileRepository.deleteMatchingPartial run before the new local children are fully
persisted. Update fetchAndStoreChildren to write the replacement folders/files
first using createOrUpdateFolderByBatch, createOrUpdateFileByBatch, and the
Promise.all add/upsert step, then remove stale rows afterward. Keep the existing
mapping logic and error handling, but make sure the durable insert/update
completes before any delete calls.
In
`@src/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue.ts`:
- Around line 40-56: The deduplication in TemporalFileUploadQueue.enqueue is
vulnerable to a race because queuedPaths is only updated after the await on
repository.stage, allowing duplicate uploads for the same path. Reserve the path
in queuedPaths before calling repository.stage in enqueue, then proceed to stage
and push the task using the existing enqueue, drain, and queuedPaths symbols so
concurrent calls can no longer pass the has check before the path is marked.
In `@src/context/virtual-drive/files/infrastructure/InMemoryFileRepository.ts`:
- Around line 107-118: deleteMatchingPartial in InMemoryFileRepository should
use the same contentsId normalization behavior as matchingPartial, since strict
equality can miss entries that should match. Update the comparison logic inside
deleteMatchingPartial to special-case contentsId the same way matchingPartial
does, while keeping the rest of the key checks unchanged, so the two methods
stay consistent.
---
Outside diff comments:
In `@src/backend/features/virtual-drive/ipc/handlers.ts`:
- Around line 14-24: The fire-and-forget logic in remoteChangesSyncedHandler
currently ignores failures from both updateVirtualDriveContainer and
DirectoryStateSqliteRepository.clear(), which can surface as unhandled
rejections. Update this handler to explicitly catch and log errors from the
Promise.all work, using the existing remoteChangesSyncedHandler,
updateVirtualDriveContainer, and DirectoryStateSqliteRepository.clear symbols so
the failure path is handled safely without dropping rejected promises.
---
Nitpick comments:
In `@src/apps/drive/dependency-injection/virtual-drive/registerFolderServices.ts`:
- Around line 28-30: The TODO comment in registerFolderServices is stale because
the FolderRepository binding is intentionally public for lazy hydrator
resolution. Remove the “can be private?” comment from the
builder.register(FolderRepository).use(InMemoryFolderRepository).asSingleton()
registration so the intent is clear and there’s no confusion for future readers.
In
`@src/backend/features/virtual-drive/services/operations/get-attributes.service.test.ts`:
- Around line 75-84: The current getAttributes test only covers the fallback
path after hydration, but it misses the new early-return behavior after
re-running findLocalEntry. Add test cases in get-attributes.service.test.ts
where hydrator.ensurePathLoaded is invoked and then fileSearcher.run or
folderSearcher.run resolves to a hydrated file/folder, and assert getAttributes
returns those attributes immediately instead of continuing to document fallback.
Use the existing getAttributes, hydrator.ensurePathLoaded, fileSearcher.run, and
folderSearcher.run symbols to target the new hydrated-entry branches.
In
`@src/backend/features/virtual-drive/services/operations/get-attributes.service.ts`:
- Around line 86-118: The file and folder attribute mapping in
GetAttributesService is duplicated between the local-entry and hydrated-entry
branches, which risks drift over time. Refactor the shared logic into reusable
helpers such as toFileAttributes and toFolderAttributes (or equivalent methods
near findLocalEntry / ensurePathLoaded) and have both branches call them so the
mode, size, timestamps, uid/gid, and nlink mapping stays consistent in one
place.
In
`@src/backend/features/virtual-drive/services/operations/opendir.service.test.ts`:
- Around line 57-59: The existing opendir service test only exercises a
rejection that is already a FuseError, so it does not catch the unsafe cast in
opendir.service.ts. Extend the test in opendir.service.test.ts around
hydrator.readDirectory to also reject with a plain Error and assert the service
still resolves to a FuseError with the expected code. Use the opendir service
path and the FuseError/FuseCodes assertions to verify the fallback behavior when
err is not already a FuseError.
In
`@src/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue.ts`:
- Around line 12-22: Props and UploadTask are duplicated with the same shape, so
reuse a single type definition in TemporalFileUploadQueue to avoid drift as
fields change. Update the queue’s type declarations so one of the symbols, Props
or UploadTask, becomes the shared source of truth and the other references it
directly.
🪄 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: 522a22bd-87c6-469c-a716-8b578d062dac
📒 Files selected for processing (37)
src/apps/drive/dependency-injection/offline-drive/registerTemporalFilesServices.tssrc/apps/drive/dependency-injection/virtual-drive/registerFilesServices.tssrc/apps/drive/dependency-injection/virtual-drive/registerFolderServices.tssrc/apps/drive/dependency-injection/virtual-drive/registerVirtualDriveSharedServices.tssrc/apps/main/database/data-source.tssrc/apps/main/remote-sync/RemoteSyncManager.test.tssrc/apps/main/remote-sync/RemoteSyncManager.tssrc/apps/main/remote-sync/service.tssrc/backend/features/virtual-drive/ipc/handlers.tssrc/backend/features/virtual-drive/services/drive-folder/virtual-drive.service.test.tssrc/backend/features/virtual-drive/services/drive-folder/virtual-drive.service.tssrc/backend/features/virtual-drive/services/lazy/DirectoryStateSqliteRepository.tssrc/backend/features/virtual-drive/services/lazy/LazyVirtualDriveHydrator.tssrc/backend/features/virtual-drive/services/operations/get-attributes.service.test.tssrc/backend/features/virtual-drive/services/operations/get-attributes.service.tssrc/backend/features/virtual-drive/services/operations/open.service.test.tssrc/backend/features/virtual-drive/services/operations/open.service.tssrc/backend/features/virtual-drive/services/operations/opendir.service.test.tssrc/backend/features/virtual-drive/services/operations/opendir.service.tssrc/backend/features/virtual-drive/services/operations/release.service.test.tssrc/backend/features/virtual-drive/services/operations/release.service.tssrc/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue.tssrc/context/storage/TemporalFiles/domain/TemporalFileRepository.tssrc/context/storage/TemporalFiles/infrastructure/NodeTemporalFileRepository.tssrc/context/virtual-drive/files/domain/FileRepository.tssrc/context/virtual-drive/files/infrastructure/InMemoryFileRepository.tssrc/context/virtual-drive/folders/__mocks__/FolderRepositoryMock.tssrc/context/virtual-drive/folders/application/create/SimpleFolderCreator.tssrc/context/virtual-drive/folders/domain/FolderRepository.tssrc/context/virtual-drive/folders/infrastructure/InMemoryFolderRepository.tssrc/context/virtual-drive/remoteTree/infrastructure/SQLiteRemoteItemsGenerator.tssrc/core/bootstrap/register-session-event-handlers.tssrc/core/electron/paths.tssrc/infra/local-file-system/safe-readdir.test.tssrc/infra/sqlite/services/file/create-or-update-file-by-batch.tssrc/infra/sqlite/services/folder/create-or-update-folder-by-batch.tsvitest.setup.main.ts
Comment on lines
+79
to
+96
| for (const segment of requestedPath.split('/').filter(Boolean)) { | ||
| currentPath = `${currentPath}/${segment}`; | ||
|
|
||
| const existing = folderRepository.matchingPartial({ path: currentPath })[0]; | ||
| if (existing) { | ||
| currentFolder = existing; | ||
| continue; | ||
| } | ||
|
|
||
| await refreshChildrenIfNeeded({ folder: currentFolder, statusScope }); | ||
|
|
||
| const hydrated = folderRepository.matchingPartial({ path: currentPath })[0]; | ||
| if (!hydrated) { | ||
| throw new FuseError(FuseCodes.ENOENT, `[FUSE - Lazy] Folder not found: ${currentPath}`); | ||
| } | ||
|
|
||
| currentFolder = hydrated; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Silence the no-await-in-loop warning with justification instead of leaving it unresolved.
Flagged by pipeline lint. The sequential await here is intentional — each path segment's materialization depends on the previous one being resolved — so this isn't a real parallelization opportunity; add an explicit disable comment to document that.
🧹 Proposed fix
const existing = folderRepository.matchingPartial({ path: currentPath })[0];
if (existing) {
currentFolder = existing;
continue;
}
+ // eslint-disable-next-line no-await-in-loop -- each segment must be resolved sequentially before descending
await refreshChildrenIfNeeded({ folder: currentFolder, statusScope });📝 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
| for (const segment of requestedPath.split('/').filter(Boolean)) { | |
| currentPath = `${currentPath}/${segment}`; | |
| const existing = folderRepository.matchingPartial({ path: currentPath })[0]; | |
| if (existing) { | |
| currentFolder = existing; | |
| continue; | |
| } | |
| await refreshChildrenIfNeeded({ folder: currentFolder, statusScope }); | |
| const hydrated = folderRepository.matchingPartial({ path: currentPath })[0]; | |
| if (!hydrated) { | |
| throw new FuseError(FuseCodes.ENOENT, `[FUSE - Lazy] Folder not found: ${currentPath}`); | |
| } | |
| currentFolder = hydrated; | |
| } | |
| for (const segment of requestedPath.split('/').filter(Boolean)) { | |
| currentPath = `${currentPath}/${segment}`; | |
| const existing = folderRepository.matchingPartial({ path: currentPath })[0]; | |
| if (existing) { | |
| currentFolder = existing; | |
| continue; | |
| } | |
| // eslint-disable-next-line no-await-in-loop -- each segment must be resolved sequentially before descending | |
| await refreshChildrenIfNeeded({ folder: currentFolder, statusScope }); | |
| const hydrated = folderRepository.matchingPartial({ path: currentPath })[0]; | |
| if (!hydrated) { | |
| throw new FuseError(FuseCodes.ENOENT, `[FUSE - Lazy] Folder not found: ${currentPath}`); | |
| } | |
| currentFolder = hydrated; | |
| } |
🧰 Tools
🪛 GitHub Actions: Lint / 0_🔍 Lint.txt
[warning] 88-88: ESLint warning: Unexpected await inside a loop. (no-await-in-loop)
🤖 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/backend/features/virtual-drive/services/lazy/LazyVirtualDriveHydrator.ts`
around lines 79 - 96, The loop in LazyVirtualDriveHydrator is intentionally
sequential, so the no-await-in-loop lint warning should be handled explicitly
rather than left unresolved. Add a targeted lint disable comment on the await
inside the path-segment loop in LazyVirtualDriveHydrator, and include a brief
justification that each segment depends on the previous folder being hydrated
before continuing.
Source: Pipeline failures
Comment on lines
+127
to
+196
| async function fetchAndStoreChildren({ folder, statusScope }: { folder: Folder; statusScope: DirectoryStatusScope }) { | ||
| try { | ||
| const { data, error } = await fetchFolder(folder.uuid); | ||
|
|
||
| if (error || !data) { | ||
| throw error ?? new FolderNotFoundError(folder.path); | ||
| } | ||
|
|
||
| const remoteFolders = data.children.filter(isExistingFolder).map((child) => toRemoteFolder(child)); | ||
| const remoteFiles = data.files.filter(isExistingFile).map((child) => toRemoteFile(child)); | ||
|
|
||
| // SQLite (better-sqlite3) uses a single writer connection. | ||
| // Running both batch transactions in parallel causes nested transaction errors. | ||
| await createOrUpdateFolderByBatch({ folders: remoteFolders }); | ||
| await createOrUpdateFileByBatch({ files: remoteFiles }); | ||
| await folderRepository.deleteMatchingPartial({ parentId: folder.id }); | ||
| await fileRepository.deleteMatchingPartial({ folderId: folder.id }); | ||
|
|
||
| const localFolders = data.children.filter(isExistingFolder).map((child) => { | ||
| return createFolderFromServerFolder( | ||
| { | ||
| bucket: child.bucket ?? null, | ||
| createdAt: child.createdAt, | ||
| id: child.id, | ||
| name: child.name, | ||
| parentId: child.parentId, | ||
| updatedAt: child.updatedAt, | ||
| plain_name: child.plainName, | ||
| status: ServerFolderStatus.EXISTS, | ||
| uuid: child.uuid, | ||
| }, | ||
| joinVirtualPath(folder.path, child.plainName), | ||
| ); | ||
| }); | ||
|
|
||
| const localFiles = data.files.filter(isExistingFile).map((child) => { | ||
| return createFileFromServerFile( | ||
| { | ||
| bucket: child.bucket, | ||
| createdAt: child.createdAt, | ||
| encrypt_version: child.encryptVersion, | ||
| fileId: child.fileId ?? '', | ||
| folderId: child.folderId, | ||
| id: child.id, | ||
| modificationTime: child.modificationTime ?? child.updatedAt, | ||
| name: child.name, | ||
| size: Number.parseInt(String(child.size), 10), | ||
| type: child.type, | ||
| updatedAt: child.updatedAt, | ||
| userId: child.userId, | ||
| status: ServerFileStatus.EXISTS, | ||
| plainName: child.plainName, | ||
| uuid: child.uuid, | ||
| }, | ||
| joinVirtualPath(folder.path, buildFileName(child)), | ||
| ); | ||
| }); | ||
|
|
||
| await Promise.all([ | ||
| ...localFolders.map((child) => folderRepository.add(child)), | ||
| ...localFiles.map((child) => fileRepository.upsert(child)), | ||
| ]); | ||
|
|
||
| await directoryStateRepository.markLoaded({ folderId: folder.id, statusScope }); | ||
| } catch (error) { | ||
| await directoryStateRepository.markError({ folderId: folder.id, statusScope }); | ||
| logger.error({ msg: '[FUSE - Lazy] Failed to fetch folder children', error, path: folder.path }); | ||
| throw new FuseError(FuseCodes.EIO, `[FUSE - Lazy] Unable to hydrate path: ${folder.path}`); | ||
| } | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: check FolderRepository's add/upsert contract to see if a non-destructive reconciliation is possible
ast-grep run --pattern 'add($_) { $$$ }' --lang typescript src/context/virtual-drive/folders
rg -n "add\(|upsert\(" src/context/virtual-drive/folders/domain/FolderRepository.ts src/context/virtual-drive/folders/infrastructure/InMemoryFolderRepository.tsRepository: internxt/drive-desktop-linux
Length of output: 2008
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the relevant files first.
git ls-files | rg '^src/(backend/features/virtual-drive/services/lazy/LazyVirtualDriveHydrator\.ts|context/virtual-drive/folders/.*)$' || true
# Show the relevant sections if the files exist.
for f in \
src/backend/features/virtual-drive/services/lazy/LazyVirtualDriveHydrator.ts \
src/context/virtual-drive/folders/domain/FolderRepository.ts \
src/context/virtual-drive/folders/infrastructure/InMemoryFolderRepository.ts
do
if [ -f "$f" ]; then
echo "===== $f ====="
wc -l "$f"
sed -n '1,260p' "$f"
fi
doneRepository: internxt/drive-desktop-linux
Length of output: 1979
Avoid deleting children before the replacement set is durable
If mapping or the final Promise.all fails after these deletes, the folder is left empty in the local store until a later hydration repopulates it. Write the new entries first, then remove stale ones.
🤖 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/backend/features/virtual-drive/services/lazy/LazyVirtualDriveHydrator.ts`
around lines 127 - 196, The delete order in fetchAndStoreChildren is unsafe
because folderRepository.deleteMatchingPartial and
fileRepository.deleteMatchingPartial run before the new local children are fully
persisted. Update fetchAndStoreChildren to write the replacement folders/files
first using createOrUpdateFolderByBatch, createOrUpdateFileByBatch, and the
Promise.all add/upsert step, then remove stale rows afterward. Keep the existing
mapping logic and error handling, but make sure the durable insert/update
completes before any delete calls.
Comment on lines
+40
to
+56
| async function enqueue({ temporalFile, path, processName }: Props) { | ||
| if (queuedPaths.has(path)) { | ||
| logger.debug({ msg: '[UploadQueue] Path already queued', path, processName }); | ||
| return; | ||
| } | ||
|
|
||
| const stagedTemporalFile = await repository.stage(temporalFile.path, PATHS.UPLOAD_QUEUE); | ||
|
|
||
| tasks.push({ | ||
| temporalFile: stagedTemporalFile, | ||
| path, | ||
| processName, | ||
| }); | ||
| queuedPaths.add(path); | ||
|
|
||
| void drain(); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Race condition: dedup check and queuedPaths.add are split by an await, allowing duplicate staged uploads for the same path.
queuedPaths.has(path) is checked synchronously, but queuedPaths.add(path) only happens after await repository.stage(...) resolves (Line 53). If enqueue is called twice for the same path in quick succession (e.g., rapid FUSE release events), both calls can pass the has() check before either adds to the set, resulting in two staged copies and two uploads for the same file.
🔒 Proposed fix: reserve the path before the await
async function enqueue({ temporalFile, path, processName }: Props) {
if (queuedPaths.has(path)) {
logger.debug({ msg: '[UploadQueue] Path already queued', path, processName });
return;
}
- const stagedTemporalFile = await repository.stage(temporalFile.path, PATHS.UPLOAD_QUEUE);
-
- tasks.push({
- temporalFile: stagedTemporalFile,
- path,
- processName,
- });
- queuedPaths.add(path);
+ queuedPaths.add(path);
+
+ try {
+ const stagedTemporalFile = await repository.stage(temporalFile.path, PATHS.UPLOAD_QUEUE);
+ tasks.push({ temporalFile: stagedTemporalFile, path, processName });
+ } catch (error) {
+ queuedPaths.delete(path);
+ throw error;
+ }
void drain();
}📝 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
| async function enqueue({ temporalFile, path, processName }: Props) { | |
| if (queuedPaths.has(path)) { | |
| logger.debug({ msg: '[UploadQueue] Path already queued', path, processName }); | |
| return; | |
| } | |
| const stagedTemporalFile = await repository.stage(temporalFile.path, PATHS.UPLOAD_QUEUE); | |
| tasks.push({ | |
| temporalFile: stagedTemporalFile, | |
| path, | |
| processName, | |
| }); | |
| queuedPaths.add(path); | |
| void drain(); | |
| } | |
| async function enqueue({ temporalFile, path, processName }: Props) { | |
| if (queuedPaths.has(path)) { | |
| logger.debug({ msg: '[UploadQueue] Path already queued', path, processName }); | |
| return; | |
| } | |
| queuedPaths.add(path); | |
| try { | |
| const stagedTemporalFile = await repository.stage(temporalFile.path, PATHS.UPLOAD_QUEUE); | |
| tasks.push({ temporalFile: stagedTemporalFile, path, processName }); | |
| } catch (error) { | |
| queuedPaths.delete(path); | |
| throw error; | |
| } | |
| void drain(); | |
| } |
🤖 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/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue.ts`
around lines 40 - 56, The deduplication in TemporalFileUploadQueue.enqueue is
vulnerable to a race because queuedPaths is only updated after the await on
repository.stage, allowing duplicate uploads for the same path. Reserve the path
in queuedPaths before calling repository.stage in enqueue, then proceed to stage
and push the task using the existing enqueue, drain, and queuedPaths symbols so
concurrent calls can no longer pass the has check before the path is marked.
… safe-readdir tests
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (3)
src/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue/preserve-rejected-upload.test.ts (1)
29-50: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd test for the preservation-error case.
There's no test covering when
preserveRejectedFileSizeTooBigreturns an error. In that case,deleter.runshould not be called and the function should return early.♻️ Suggested test
it('should not delete the file when preservation fails', async () => { const deleter = { run: vi.fn().mockResolvedValue(undefined) }; const task = { temporalFile: { contentFilePath: '/tmp/content.bin', size: { value: 123 }, } as unknown as TemporalFile, path: '/target/file.txt', processName: 'process', }; vi.mocked(preserveRejectedFileSizeTooBig).mockResolvedValue({ error: new Error('copy failed') } as never); await preserveRejectedUpload({ task, deleter } as never); expect(deleter.run).not.toHaveBeenCalled(); });🤖 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/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue/preserve-rejected-upload.test.ts` around lines 29 - 50, The preserveRejectedUpload test suite is missing coverage for the preservation-error path, so add a new test around preserveRejectedUpload and preserveRejectedFileSizeTooBig that mocks a returned error. In this case, assert that deleter.run is not called and that the function returns early without attempting deletion; use the existing task shape and the preserveRejectedFileSizeTooBig mock to locate the behavior.src/context/virtual-drive/files/infrastructure/InMemoryFileRepository.ts (1)
30-36: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer strict equality (
===) over loose equality (==).The
matchesPartialhelper uses==for both thecontentsIdnormalization branch and the generic comparison. The past review's proposed fix used===, and strict equality is the TypeScript best practice to avoid subtle type-coercion bugs.♻️ Proposed refactor
return keys.every((key: keyof FileAttributes) => { if (key === 'contentsId') { - return attributes[key].normalize() == (partial[key] as string).normalize(); + return attributes[key].normalize() === (partial[key] as string).normalize(); } - return attributes[key] == partial[key]; + return attributes[key] === partial[key]; });🤖 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/context/virtual-drive/files/infrastructure/InMemoryFileRepository.ts` around lines 30 - 36, The matchesPartial helper in InMemoryFileRepository currently uses loose equality for both the contentsId normalization branch and the generic attribute comparison. Update the comparisons in matchesPartial to use strict equality instead, including the normalized contentsId check, so the behavior is explicit and avoids type coercion; keep the rest of the key iteration logic unchanged.src/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue/drain-upload-queue.test.ts (1)
47-74: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd test coverage for the generic (non-UploadSizeLimitError) failure path.
The current tests cover the happy path and the
UploadSizeLimitErrorpath, but there's no test for whenuploadQueuedTaskthrows a generic error. This is the path with the orphaned-file issue, so a test would help verify the intended behavior and prevent regressions.♻️ Suggested test
it('should log and keep task in queue on generic error', async () => { const state = createTemporalFileUploadQueueState(); const task = { temporalFile: { path: '/staged/file.txt' }, path: '/target/file.txt', processName: 'process', }; state.tasks.push(task as never); state.queuedPaths.add(task.path); const deleter = { run: vi.fn().mockResolvedValue(undefined) }; vi.mocked(uploadQueuedTask).mockRejectedValue(new Error('network failure')); await drainUploadQueue({ uploader: {} as never, deleter, fileSearcher: {} as never, state, } as never); expect(deleter.run).not.toHaveBeenCalled(); // Assert based on the intended behavior after fixing the issue: // - If retrying: expect(state.tasks).toHaveLength(1); // - If cleaning up: expect(deleter.run).toHaveBeenCalledWith(task.path); });🤖 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/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue/drain-upload-queue.test.ts` around lines 47 - 74, Add a test in drainUploadQueue.test for the generic uploadQueuedTask failure path, not just UploadSizeLimitError. Use the existing drainUploadQueue, uploadQueuedTask, and createTemporalFileUploadQueueState setup to mock a non-UploadSizeLimitError rejection (for example, a generic Error) and assert the intended orphaned-file behavior in this branch. Verify the side effects differ from the oversized-upload case by checking whether deleter.run is called or the task remains queued, and keep the task/state assertions aligned with the actual behavior of drainUploadQueue.
🤖 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/backend/features/user/file-size-limit/upload-size-limit-blocked-paths.ts`:
- Around line 1-13: This file likely has Prettier formatting issues flagged by
the format check; run the repository formatting command to auto-fix the style in
the upload-size-limit-blocked-paths module. Verify the exported helpers
markUploadSizeLimitBlockedPath, isUploadSizeLimitBlockedPath, and
clearUploadSizeLimitBlockedPath remain unchanged functionally, then commit the
formatted output.
In
`@src/backend/features/virtual-drive/services/lazy/virtual-drive-hydrator/children.ts`:
- Around line 106-108: The isExistingFile helper currently only checks
FileDto.status and can still treat deleted or removed files as existing. Update
isExistingFile in children.ts to mirror isExistingFolder by also excluding
deleted and removed states, and ensure the hydration path that uses this helper
does not include those files in listings or persistence.
In
`@src/backend/features/virtual-drive/services/lazy/virtual-drive-hydrator/create-lazy-virtual-drive-hydrator-service.ts`:
- Around line 1-94: The file is failing Prettier check, so reformat the
`createLazyVirtualDriveHydratorService` module to match the project style
without changing behavior. Run the formatter on this file and ensure the
exported `LazyVirtualDriveHydrator` symbol, `readDirectory`, and
`ensurePathLoaded` remain intact with only formatting updates.
- Around line 69-88: Refresh the parent folder before retrying the file lookup
in ensurePathLoaded, since it currently only calls ensureFolderMaterialized and
may skip repopulating children when the parent is already fresh. Update the
logic in create-lazy-virtual-drive-hydrator-service.ts so ensurePathLoaded also
forces a refresh of the parent directory’s children before open retries the
lookup, using the existing refreshChildrenIfNeeded path along with
folderRepository, directoryStateRepository, inflight, and
getFetchAndStoreChildren() to keep new files visible.
In
`@src/backend/features/virtual-drive/services/lazy/virtual-drive-hydrator/fetch-and-store-children.ts`:
- Around line 43-52: The delete-then-add flow in fetchAndStoreChildren leaves
the in-memory repositories inconsistent if one of the Promise.all writes fails.
Update the logic in fetchAndStoreChildren to avoid clearing existing children
before the new ones are safely present, either by upserting/adding the new
folder and file children first and then removing stale entries, or by adding
rollback/compensation around the delete-and-replace sequence so a partial
failure does not leave the virtual drive missing entries.
- Around line 55-58: The catch block in fetchAndStoreChildren lets
directoryStateRepository.markError replace the original failure, which can
prevent logger.error and the intended FuseError from being raised. Update the
error-handling path in fetchAndStoreChildren so markError is best-effort and
wrapped separately, preserving the original caught error; then always log the
failure and throw the FuseError(EIO) for the folder path even if markError
fails.
In `@src/backend/features/virtual-drive/services/operations/release.service.ts`:
- Around line 73-75: In `release.service.ts`, the error handling after
`enqueueTemporalFile()` is deleting the temporal file for all failures, which
can destroy the only copy on transient enqueue errors. Update the `release` flow
to preserve `path` whenever `enqueueTemporalFile()` rejects for
non-`UploadSizeLimitError` cases, and only remove or clean up the file in the
explicit non-recoverable path; use the existing `enqueueTemporalFile`,
`UploadSizeLimitError`, and `deleteTemporalFile` logic to route failures
correctly.
In
`@src/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue/drain-upload-queue.ts`:
- Around line 15-34: The retry handling in processTask is dropping transient
failures from the in-memory queue while also leaving the staged file on disk,
which can orphan uploads. Update the UploadSizeLimitError /
non-UploadSizeLimitError flow in drain-upload-queue.ts so state.tasks.shift()
and state.queuedPaths.delete(...) only happen after a successful upload or a
permanent rejection handled by preserveRejectedUpload; for transient errors,
keep the task queued for retry (or explicitly delete the file if you choose not
to retry). Ensure the logger.error path and deleter.run behavior in processTask
reflect the chosen retry/delete policy consistently.
In
`@src/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue/enqueue-upload.ts`:
- Line 40: The fire-and-forget call to drainUploadQueue in enqueue-upload.ts can
still produce an unhandled rejection if preserveRejectedUpload fails, since the
returned promise is discarded. Update the enqueue-upload flow to attach a
top-level catch to the drainUploadQueue invocation, or handle the
preserveRejectedUpload failure inside processTask, so unexpected errors are
swallowed or logged instead of bubbling unhandled.
In
`@src/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue/state.ts`:
- Around line 1-11: Prettier formatting is out of sync in
createTemporalFileUploadQueueState, so update the affected file to match the
repo’s formatter output. Re-run formatting on the changed files with the
project’s Prettier command, then verify the formatting around
createTemporalFileUploadQueueState and the QueueState object literal is
consistent with CI expectations.
---
Nitpick comments:
In
`@src/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue/drain-upload-queue.test.ts`:
- Around line 47-74: Add a test in drainUploadQueue.test for the generic
uploadQueuedTask failure path, not just UploadSizeLimitError. Use the existing
drainUploadQueue, uploadQueuedTask, and createTemporalFileUploadQueueState setup
to mock a non-UploadSizeLimitError rejection (for example, a generic Error) and
assert the intended orphaned-file behavior in this branch. Verify the side
effects differ from the oversized-upload case by checking whether deleter.run is
called or the task remains queued, and keep the task/state assertions aligned
with the actual behavior of drainUploadQueue.
In
`@src/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue/preserve-rejected-upload.test.ts`:
- Around line 29-50: The preserveRejectedUpload test suite is missing coverage
for the preservation-error path, so add a new test around preserveRejectedUpload
and preserveRejectedFileSizeTooBig that mocks a returned error. In this case,
assert that deleter.run is not called and that the function returns early
without attempting deletion; use the existing task shape and the
preserveRejectedFileSizeTooBig mock to locate the behavior.
In `@src/context/virtual-drive/files/infrastructure/InMemoryFileRepository.ts`:
- Around line 30-36: The matchesPartial helper in InMemoryFileRepository
currently uses loose equality for both the contentsId normalization branch and
the generic attribute comparison. Update the comparisons in matchesPartial to
use strict equality instead, including the normalized contentsId check, so the
behavior is explicit and avoids type coercion; keep the rest of the key
iteration logic unchanged.
🪄 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: f294bc9c-486e-4dcf-a2c3-2d73baf7051e
📒 Files selected for processing (40)
src/apps/drive/dependency-injection/offline-drive/registerTemporalFilesServices.tssrc/apps/drive/dependency-injection/virtual-drive/registerVirtualDriveSharedServices.tssrc/backend/features/user/file-size-limit/add-max-file-size-rejection.tssrc/backend/features/user/file-size-limit/upload-size-limit-blocked-paths.tssrc/backend/features/virtual-drive/controllers/operations/release.controller.test.tssrc/backend/features/virtual-drive/controllers/operations/release.controller.tssrc/backend/features/virtual-drive/services/lazy/virtual-drive-hydrator/children.test.tssrc/backend/features/virtual-drive/services/lazy/virtual-drive-hydrator/children.tssrc/backend/features/virtual-drive/services/lazy/virtual-drive-hydrator/create-lazy-virtual-drive-hydrator-service.tssrc/backend/features/virtual-drive/services/lazy/virtual-drive-hydrator/ensure-folder-materialized.test.tssrc/backend/features/virtual-drive/services/lazy/virtual-drive-hydrator/ensure-folder-materialized.tssrc/backend/features/virtual-drive/services/lazy/virtual-drive-hydrator/fetch-and-store-children.test.tssrc/backend/features/virtual-drive/services/lazy/virtual-drive-hydrator/fetch-and-store-children.tssrc/backend/features/virtual-drive/services/lazy/virtual-drive-hydrator/file-name.tssrc/backend/features/virtual-drive/services/lazy/virtual-drive-hydrator/path.test.tssrc/backend/features/virtual-drive/services/lazy/virtual-drive-hydrator/path.tssrc/backend/features/virtual-drive/services/lazy/virtual-drive-hydrator/refresh-children-if-needed.tssrc/backend/features/virtual-drive/services/lazy/virtual-drive-hydrator/resolve-root-folder.tssrc/backend/features/virtual-drive/services/lazy/virtual-drive-hydrator/types.tssrc/backend/features/virtual-drive/services/operations/get-attributes.service.test.tssrc/backend/features/virtual-drive/services/operations/get-attributes.service.tssrc/backend/features/virtual-drive/services/operations/open.service.test.tssrc/backend/features/virtual-drive/services/operations/open.service.tssrc/backend/features/virtual-drive/services/operations/opendir.service.test.tssrc/backend/features/virtual-drive/services/operations/opendir.service.tssrc/backend/features/virtual-drive/services/operations/release.service.test.tssrc/backend/features/virtual-drive/services/operations/release.service.tssrc/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue/create-temporal-file-upload-queue-service.tssrc/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue/drain-upload-queue.test.tssrc/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue/drain-upload-queue.tssrc/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue/enqueue-upload.test.tssrc/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue/enqueue-upload.tssrc/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue/preserve-rejected-upload.test.tssrc/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue/preserve-rejected-upload.tssrc/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue/state.tssrc/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue/types.tssrc/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue/upload-queued-task.test.tssrc/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue/upload-queued-task.tssrc/context/virtual-drive/files/infrastructure/InMemoryFileRepository.test.tssrc/context/virtual-drive/files/infrastructure/InMemoryFileRepository.ts
✅ Files skipped from review due to trivial changes (3)
- src/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue/upload-queued-task.test.ts
- src/backend/features/virtual-drive/services/lazy/virtual-drive-hydrator/path.ts
- src/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue/types.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- src/backend/features/virtual-drive/services/operations/open.service.ts
- src/apps/drive/dependency-injection/virtual-drive/registerVirtualDriveSharedServices.ts
- src/backend/features/virtual-drive/services/operations/get-attributes.service.ts
- src/apps/drive/dependency-injection/offline-drive/registerTemporalFilesServices.ts
- src/backend/features/virtual-drive/services/operations/open.service.test.ts
- src/backend/features/virtual-drive/services/operations/opendir.service.ts
- src/backend/features/virtual-drive/services/operations/opendir.service.test.ts
- src/backend/features/virtual-drive/services/operations/get-attributes.service.test.ts
Comment on lines
+1
to
+13
| const uploadSizeLimitBlockedPaths = new Set<string>(); | ||
|
|
||
| export function markUploadSizeLimitBlockedPath(path: string): void { | ||
| uploadSizeLimitBlockedPaths.add(path); | ||
| } | ||
|
|
||
| export function isUploadSizeLimitBlockedPath(path: string): boolean { | ||
| return uploadSizeLimitBlockedPaths.has(path); | ||
| } | ||
|
|
||
| export function clearUploadSizeLimitBlockedPath(path: string): void { | ||
| uploadSizeLimitBlockedPaths.delete(path); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔴 Critical | ⚡ Quick win
Fix Prettier formatting failures before merge.
The pipeline's format check (npm run format) reported code style issues in 2 files. As a new file in this PR, this file may be one of them. Run npm run format to auto-fix.
#!/bin/bash
# Identify which files have Prettier formatting issues
npx prettier src --check 2>&1 | head -20🧰 Tools
🪛 GitHub Actions: Lint / 👨🎨 Format Check
[error] 1-1: Command failed: npm run format (prettier src --check). Prettier reported code style issues in 2 files, exiting with code 1.
🤖 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/backend/features/user/file-size-limit/upload-size-limit-blocked-paths.ts`
around lines 1 - 13, This file likely has Prettier formatting issues flagged by
the format check; run the repository formatting command to auto-fix the style in
the upload-size-limit-blocked-paths module. Verify the exported helpers
markUploadSizeLimitBlockedPath, isUploadSizeLimitBlockedPath, and
clearUploadSizeLimitBlockedPath remain unchanged functionally, then commit the
formatted output.
Source: Pipeline failures
Comment on lines
+106
to
+108
| function isExistingFile(file: FileDto) { | ||
| return file.status === 'EXISTS'; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify whether FileDto has 'deleted' or 'removed' fields
# Find files referencing FileDto
rg -l 'FileDto' src/infra/drive-server/ --type ts
# Check the generated dto file for FileDto properties
rg -n -C3 'FileDto' src/infra/drive-server/out/dto.ts | head -80
# Search for 'deleted' or 'removed' near FileDto in the generated types
rg -n 'deleted|removed' src/infra/drive-server/out/dto.ts | head -30
# Also look for OpenAPI spec files that define the schema
fd -i 'openapi\.(json|yaml|yml)' . --max-depth 5
fd -i 'swagger\.(json|yaml|yml)' . --max-depth 5Repository: internxt/drive-desktop-linux
Length of output: 714
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the generated schema definitions for FileDto and FolderDto
FILE=src/infra/drive-server/out/schemas.d
rg -n -C 8 '"FileDto"|"FolderDto"|deleted|removed|status' "$FILE" | head -200
# Show the exact FileDto and FolderDto schema sections if present
python3 - <<'PY'
from pathlib import Path
p = Path('src/infra/drive-server/out/schemas.d')
text = p.read_text()
for name in ['FileDto', 'FolderDto']:
idx = text.find(f'"{name}"')
print(f"\n===== {name} @ {idx} =====")
if idx != -1:
start = max(0, idx - 500)
end = min(len(text), idx + 3500)
print(text[start:end])
PYRepository: internxt/drive-desktop-linux
Length of output: 243
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the schema/type source that defines FileDto and FolderDto
fd -a 'schemas.d' src .
fd -a 'dto.ts' src/infra/drive-server/out src
# Search for the FileDto and FolderDto schema definitions across likely generated files
rg -n -C 8 '"FileDto"|"FolderDto"|deleted|removed|status' src/infra/drive-server -g '*.ts' -g '*.d.ts' -g '*.json' -g '*.yaml' -g '*.yml' | head -300
# If the schema file is elsewhere under src, print candidate sections around FileDto/FolderDto
for f in $(fd -a 'schemas.d' src . 2>/dev/null); do
echo "===== $f ====="
rg -n -C 8 '"FileDto"|"FolderDto"|deleted|removed|status' "$f" | head -200
doneRepository: internxt/drive-desktop-linux
Length of output: 43090
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=src/backend/features/virtual-drive/services/lazy/virtual-drive-hydrator/children.ts
# Show the relevant area around the filters and any nearby mapping logic
cat -n "$FILE" | sed -n '70,140p'
# Find references to isExistingFile / isExistingFolder in this file
rg -n 'isExistingFile|isExistingFolder|deleted|removed|status' "$FILE"Repository: internxt/drive-desktop-linux
Length of output: 3304
Filter out deleted/removed files here FileDto also carries deleted and removed, so isExistingFile should match isExistingFolder and exclude those states; otherwise deleted files can still be hydrated into listings and persistence.
🤖 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/backend/features/virtual-drive/services/lazy/virtual-drive-hydrator/children.ts`
around lines 106 - 108, The isExistingFile helper currently only checks
FileDto.status and can still treat deleted or removed files as existing. Update
isExistingFile in children.ts to mirror isExistingFolder by also excluding
deleted and removed states, and ensure the hydration path that uses this helper
does not include those files in listings or persistence.
Comment on lines
+69
to
+88
| async function ensurePathLoaded({ path: requestedPath }: EnsurePathLoadedProps) { | ||
| const normalizedPath = normalizePath(requestedPath); | ||
| const statusScope: HydratorStatusScope = 'EXISTS'; | ||
| const parentPath = isRootPath(normalizedPath) | ||
| ? normalizedPath | ||
| : normalizedPath.slice(0, normalizedPath.lastIndexOf('/')) || '/'; | ||
|
|
||
| await ensureFolderMaterialized({ | ||
| requestedPath: parentPath, | ||
| statusScope, | ||
| folderRepository, | ||
| refreshChildrenIfNeeded: (props) => | ||
| refreshChildrenIfNeeded({ | ||
| ...props, | ||
| directoryStateRepository, | ||
| inflight, | ||
| fetchAndStoreChildren: getFetchAndStoreChildren(), | ||
| }), | ||
| }); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if open/get-attributes services call refreshChildrenIfNeeded or similar
rg -n 'refreshChildrenIfNeeded\|ensurePathLoaded\|readDirectory' \
src/backend/features/virtual-drive/services/operations/open.service.ts \
src/backend/features/virtual-drive/services/operations/get-attributes.service.tsRepository: internxt/drive-desktop-linux
Length of output: 166
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the virtual-drive operations and hydrator files, then inspect the relevant call sites.
git ls-files | rg 'virtual-drive.*(open|get-attributes|hydrator|ensurePathLoaded|readDirectory)|open\.service|get-attributes\.service|create-lazy-virtual-drive-hydrator-service'
echo '---'
fd -a 'open.service.ts|get-attributes.service.ts|create-lazy-virtual-drive-hydrator-service.ts' src backend . 2>/dev/null || true
echo '--- AST OUTLINES ---'
for f in \
src/backend/features/virtual-drive/services/lazy/virtual-drive-hydrator/create-lazy-virtual-drive-hydrator-service.ts \
src/backend/features/virtual-drive/services/operations/open.service.ts \
src/backend/features/virtual-drive/services/operations/get-attributes.service.ts
do
if [ -f "$f" ]; then
echo "FILE: $f"
ast-grep outline "$f" --view expanded || true
echo '---'
fi
doneRepository: internxt/drive-desktop-linux
Length of output: 4249
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for f in \
src/backend/features/virtual-drive/services/lazy/virtual-drive-hydrator/ensure-folder-materialized.ts \
src/backend/features/virtual-drive/services/lazy/virtual-drive-hydrator/refresh-children-if-needed.ts \
src/backend/features/virtual-drive/services/operations/open.service.ts \
src/backend/features/virtual-drive/services/operations/get-attributes.service.ts \
src/backend/features/virtual-drive/services/lazy/virtual-drive-hydrator/create-lazy-virtual-drive-hydrator-service.ts
do
echo "===== $f ====="
wc -l "$f"
echo "---"
cat -n "$f" | sed -n '1,220p'
echo
doneRepository: internxt/drive-desktop-linux
Length of output: 15268
Refresh the parent folder before retrying the file lookup. ensurePathLoaded only materializes ancestor folders; if the parent is already fresh, it never repopulates that folder’s children. open retries the lookup after this call, and get-attributes still returns ENOENT when the file isn’t already tracked as temporal, so a new child can stay invisible until readDirectory runs or the freshness window expires.
🤖 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/backend/features/virtual-drive/services/lazy/virtual-drive-hydrator/create-lazy-virtual-drive-hydrator-service.ts`
around lines 69 - 88, Refresh the parent folder before retrying the file lookup
in ensurePathLoaded, since it currently only calls ensureFolderMaterialized and
may skip repopulating children when the parent is already fresh. Update the
logic in create-lazy-virtual-drive-hydrator-service.ts so ensurePathLoaded also
forces a refresh of the parent directory’s children before open retries the
lookup, using the existing refreshChildrenIfNeeded path along with
folderRepository, directoryStateRepository, inflight, and
getFetchAndStoreChildren() to keep new files visible.
Comment on lines
+43
to
+52
| await folderRepository.deleteMatchingPartial({ parentId: folder.id }); | ||
| await fileRepository.deleteMatchingPartial({ folderId: folder.id }); | ||
|
|
||
| const localFolders = createLocalFolders({ folderPath: folder.path, children: data.children }); | ||
| const localFiles = createLocalFiles({ folderPath: folder.path, files: data.files }); | ||
|
|
||
| await Promise.all([ | ||
| ...localFolders.map((child) => folderRepository.add(child)), | ||
| ...localFiles.map((child) => fileRepository.upsert(child)), | ||
| ]); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Non-atomic delete-then-add leaves in-memory state inconsistent on partial failure.
Lines 43–44 delete all existing children from the in-memory repositories before lines 49–52 add the new children via Promise.all. If any add/upsert rejects, the remaining promises may have already resolved, leaving the directory partially populated with no rollback. During this window, FUSE operations will return ENOENT for files that exist on the server. The SQLite state (lines 40–41) is correct, so this self-heals on restart, but the in-session experience is degraded.
Consider adding new children first (upsert) and then deleting stale entries, or wrapping the delete-add in a compensating rollback that restores old children if the add fails.
🤖 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/backend/features/virtual-drive/services/lazy/virtual-drive-hydrator/fetch-and-store-children.ts`
around lines 43 - 52, The delete-then-add flow in fetchAndStoreChildren leaves
the in-memory repositories inconsistent if one of the Promise.all writes fails.
Update the logic in fetchAndStoreChildren to avoid clearing existing children
before the new ones are safely present, either by upserting/adding the new
folder and file children first and then removing stale entries, or by adding
rollback/compensation around the delete-and-replace sequence so a partial
failure does not leave the virtual drive missing entries.
Comment on lines
+55
to
+58
| } catch (error) { | ||
| await directoryStateRepository.markError({ folderId: folder.id, statusScope }); | ||
| logger.error({ msg: '[FUSE - Lazy] Failed to fetch folder children', error, path: folder.path }); | ||
| throw new FuseError(FuseCodes.EIO, `[FUSE - Lazy] Unable to hydrate path: ${folder.path}`); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
markError in catch block can shadow the original error.
If directoryStateRepository.markError throws (e.g., SQLite is unavailable), the logger.error and throw new FuseError(...) on lines 57–58 will never execute. The FUSE driver would receive an unexpected error instead of the intended FuseError(EIO).
🛡️ Proposed fix
} catch (error) {
- await directoryStateRepository.markError({ folderId: folder.id, statusScope });
+ try {
+ await directoryStateRepository.markError({ folderId: folder.id, statusScope });
+ } catch {
+ // best-effort; original error takes priority
+ }
logger.error({ msg: '[FUSE - Lazy] Failed to fetch folder children', error, path: folder.path });
throw new FuseError(FuseCodes.EIO, `[FUSE - Lazy] Unable to hydrate path: ${folder.path}`);
}📝 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
| } catch (error) { | |
| await directoryStateRepository.markError({ folderId: folder.id, statusScope }); | |
| logger.error({ msg: '[FUSE - Lazy] Failed to fetch folder children', error, path: folder.path }); | |
| throw new FuseError(FuseCodes.EIO, `[FUSE - Lazy] Unable to hydrate path: ${folder.path}`); | |
| } catch (error) { | |
| try { | |
| await directoryStateRepository.markError({ folderId: folder.id, statusScope }); | |
| } catch { | |
| // best-effort; original error takes priority | |
| } | |
| logger.error({ msg: '[FUSE - Lazy] Failed to fetch folder children', error, path: folder.path }); | |
| throw new FuseError(FuseCodes.EIO, `[FUSE - Lazy] Unable to hydrate path: ${folder.path}`); |
🤖 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/backend/features/virtual-drive/services/lazy/virtual-drive-hydrator/fetch-and-store-children.ts`
around lines 55 - 58, The catch block in fetchAndStoreChildren lets
directoryStateRepository.markError replace the original failure, which can
prevent logger.error and the intended FuseError from being raised. Update the
error-handling path in fetchAndStoreChildren so markError is best-effort and
wrapped separately, preserving the original caught error; then always log the
failure and throw the FuseError(EIO) for the folder path even if markError
fails.
Comment on lines
73
to
75
| logger.error({ msg: '[Release] Upload failed, deleting temporal file', error: uploadError, path, processName }); | ||
| await container.get(TemporalFileDeleter).run(path); | ||
| await deleteTemporalFile(path); | ||
| return { error: new FuseIOError('Upload failed due to insufficient storage or network issues.') }; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether enqueue involves network/I/O calls or is purely in-memory
cat src/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue/enqueue-upload.tsRepository: internxt/drive-desktop-linux
Length of output: 1354
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== release.service.ts ==\n'
sed -n '1,180p' src/backend/features/virtual-drive/services/operations/release.service.ts
printf '\n== queue enqueue types/usages ==\n'
rg -n "enqueueTemporalFile|deleteTemporalFile|enqueueUpload\\(" src/backend src/context -g '!**/dist/**' -g '!**/build/**'Repository: internxt/drive-desktop-linux
Length of output: 7482
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== release.service.ts around the reported lines ==\n'
nl -ba src/backend/features/virtual-drive/services/operations/release.service.ts | sed -n '50,95p'
printf '\n== UploadSizeLimitError references ==\n'
rg -n "UploadSizeLimitError" src -g '!**/dist/**' -g '!**/build/**'
printf '\n== deleteTemporalFile definition ==\n'
rg -n "function deleteTemporalFile|const deleteTemporalFile|export .*deleteTemporalFile" src -g '!**/dist/**' -g '!**/build/**'Repository: internxt/drive-desktop-linux
Length of output: 258
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== enqueue-upload.test.ts ==\n'
sed -n '1,220p' src/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue/enqueue-upload.test.ts
printf '\n== drain-upload-queue.ts ==\n'
sed -n '1,220p' src/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue/drain-upload-queue.ts
printf '\n== queue types ==\n'
sed -n '1,220p' src/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue/types.tsRepository: internxt/drive-desktop-linux
Length of output: 6291
Preserve the temporal file when enqueue staging fails. enqueueTemporalFile() stages the file into the upload queue; if that step rejects for anything other than UploadSizeLimitError, deleting path here drops the only copy and makes the upload unrecoverable. Keep the temporal file on transient failures as well, or route those into the same preservation path.
🤖 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/backend/features/virtual-drive/services/operations/release.service.ts`
around lines 73 - 75, In `release.service.ts`, the error handling after
`enqueueTemporalFile()` is deleting the temporal file for all failures, which
can destroy the only copy on transient enqueue errors. Update the `release` flow
to preserve `path` whenever `enqueueTemporalFile()` rejects for
non-`UploadSizeLimitError` cases, and only remove or clean up the file in the
explicit non-recoverable path; use the existing `enqueueTemporalFile`,
`UploadSizeLimitError`, and `deleteTemporalFile` logic to route failures
correctly.
Comment on lines
+15
to
+34
| async function processTask({ task, uploader, deleter, fileSearcher, state }: Props) { | ||
| try { | ||
| await uploadQueuedTask({ task, uploader, fileSearcher }); | ||
| await deleter.run(task.path); | ||
| } catch (error) { | ||
| if (error instanceof UploadSizeLimitError) { | ||
| await preserveRejectedUpload({ task, deleter }); | ||
| return; | ||
| } | ||
|
|
||
| logger.error({ | ||
| msg: '[UploadQueue] Upload failed, keeping staged file in queue folder', | ||
| error, | ||
| path: task.path, | ||
| }); | ||
| } finally { | ||
| state.tasks.shift(); | ||
| state.queuedPaths.delete(task.path); | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Non-UploadSizeLimitError handling orphans staged files and loses uploads.
When a non-UploadSizeLimitError occurs, the finally block removes the task from state.tasks and state.queuedPaths, but deleter.run is never called — the staged file remains on disk untracked. The log message says "keeping staged file in queue folder," but the task is removed from the in-memory queue, so it will never be retried during this session. Transient errors (network, server 5xx) will cause permanent upload loss with orphaned files.
Either re-enqueue the task for retry (don't remove from state.tasks on transient errors) or delete the staged file to avoid orphaning it.
🐛 Proposed fix: only remove task from queue on success or permanent rejection
async function processTask({ task, uploader, deleter, fileSearcher, state }: Props) {
try {
await uploadQueuedTask({ task, uploader, fileSearcher });
await deleter.run(task.path);
+ state.tasks.shift();
+ state.queuedPaths.delete(task.path);
} catch (error) {
if (error instanceof UploadSizeLimitError) {
await preserveRejectedUpload({ task, deleter });
+ state.tasks.shift();
+ state.queuedPaths.delete(task.path);
return;
}
logger.error({
msg: '[UploadQueue] Upload failed, keeping staged file in queue folder',
error,
path: task.path,
});
- } finally {
- state.tasks.shift();
- state.queuedPaths.delete(task.path);
}
}This ensures the task stays in the queue for retry on transient errors, while still being removed on success or permanent rejection.
📝 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
| async function processTask({ task, uploader, deleter, fileSearcher, state }: Props) { | |
| try { | |
| await uploadQueuedTask({ task, uploader, fileSearcher }); | |
| await deleter.run(task.path); | |
| } catch (error) { | |
| if (error instanceof UploadSizeLimitError) { | |
| await preserveRejectedUpload({ task, deleter }); | |
| return; | |
| } | |
| logger.error({ | |
| msg: '[UploadQueue] Upload failed, keeping staged file in queue folder', | |
| error, | |
| path: task.path, | |
| }); | |
| } finally { | |
| state.tasks.shift(); | |
| state.queuedPaths.delete(task.path); | |
| } | |
| } | |
| async function processTask({ task, uploader, deleter, fileSearcher, state }: Props) { | |
| try { | |
| await uploadQueuedTask({ task, uploader, fileSearcher }); | |
| await deleter.run(task.path); | |
| state.tasks.shift(); | |
| state.queuedPaths.delete(task.path); | |
| } catch (error) { | |
| if (error instanceof UploadSizeLimitError) { | |
| await preserveRejectedUpload({ task, deleter }); | |
| state.tasks.shift(); | |
| state.queuedPaths.delete(task.path); | |
| return; | |
| } | |
| logger.error({ | |
| msg: '[UploadQueue] Upload failed, keeping staged file in queue folder', | |
| error, | |
| path: task.path, | |
| }); | |
| } | |
| } |
🤖 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/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue/drain-upload-queue.ts`
around lines 15 - 34, The retry handling in processTask is dropping transient
failures from the in-memory queue while also leaving the staged file on disk,
which can orphan uploads. Update the UploadSizeLimitError /
non-UploadSizeLimitError flow in drain-upload-queue.ts so state.tasks.shift()
and state.queuedPaths.delete(...) only happen after a successful upload or a
permanent rejection handled by preserveRejectedUpload; for transient errors,
keep the task queued for retry (or explicitly delete the file if you choose not
to retry). Ensure the logger.error path and deleter.run behavior in processTask
reflect the chosen retry/delete policy consistently.
| processName, | ||
| }); | ||
|
|
||
| void drainUploadQueue({ repository, uploader, deleter, fileSearcher, state }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if processTask/upload-queued-task handles errors internally
ast-grep outline src/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue/upload-queued-task.ts --items allRepository: internxt/drive-desktop-linux
Length of output: 804
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the task handler and queue drain implementation with line numbers.
for f in \
src/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue/upload-queued-task.ts \
src/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue/enqueue-upload.ts
do
echo "===== $f ====="
wc -l "$f"
sed -n '1,220p' "$f" | cat -n
echo
doneRepository: internxt/drive-desktop-linux
Length of output: 3230
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the drain helper and any surrounding error handling in the queue module.
rg -n "drainUploadQueue|processTask|catch\\(|finally|draining" \
src/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue \
-g '*.ts'Repository: internxt/drive-desktop-linux
Length of output: 3030
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read only the relevant helper implementation if it exists.
f=src/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue/drain-upload-queue.ts
if [ -f "$f" ]; then
echo "===== $f ====="
wc -l "$f"
sed -n '1,240p' "$f" | cat -n
else
echo "Missing: $f"
fiRepository: internxt/drive-desktop-linux
Length of output: 2313
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# If the drain helper lives elsewhere, find its definition.
ast-grep outline src/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue --items allRepository: internxt/drive-desktop-linux
Length of output: 3179
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the task handler body and any inline error handling nearby.
f=src/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue/upload-queued-task.ts
echo "===== $f ====="
wc -l "$f"
sed -n '1,220p' "$f" | cat -nRepository: internxt/drive-desktop-linux
Length of output: 1548
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the preserve-rejected-upload helper and its tests for its error behavior.
for f in \
src/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue/preserve-rejected-upload.ts \
src/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue/preserve-rejected-upload.test.ts \
src/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue/drain-upload-queue.test.ts
do
echo "===== $f ====="
wc -l "$f"
sed -n '1,240p' "$f" | cat -n
echo
doneRepository: internxt/drive-desktop-linux
Length of output: 7094
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find any explicit error handling or thrown failures around preserveRejectedUpload.
rg -n "preserveRejectedUpload|UploadSizeLimitError|logger\\.error|throw " \
src/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue \
-g '*.ts'Repository: internxt/drive-desktop-linux
Length of output: 3063
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read only the helper if it is short.
f=src/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue/preserve-rejected-upload.ts
echo "===== $f ====="
wc -l "$f"
sed -n '1,220p' "$f" | cat -nRepository: internxt/drive-desktop-linux
Length of output: 1611
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the helper tests to see whether failures are expected or swallowed.
f=src/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue/drain-upload-queue.test.ts
echo "===== $f ====="
wc -l "$f"
sed -n '1,240p' "$f" | cat -nRepository: internxt/drive-desktop-linux
Length of output: 3281
Handle unexpected rejections from the fire-and-forget drain (enqueue-upload.ts:40) processTask catches the common upload/delete errors, but a rejection from preserveRejectedUpload can still bubble out. Since the promise is discarded, that becomes an unhandled rejection. Add a top-level .catch() or handle that branch locally.
🤖 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/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue/enqueue-upload.ts`
at line 40, The fire-and-forget call to drainUploadQueue in enqueue-upload.ts
can still produce an unhandled rejection if preserveRejectedUpload fails, since
the returned promise is discarded. Update the enqueue-upload flow to attach a
top-level catch to the drainUploadQueue invocation, or handle the
preserveRejectedUpload failure inside processTask, so unexpected errors are
swallowed or logged instead of bubbling unhandled.
Comment on lines
+1
to
+11
| import type { QueueState } from './types'; | ||
|
|
||
| export function createTemporalFileUploadQueueState() { | ||
| const state: QueueState = { | ||
| queuedPaths: new Set<string>(), | ||
| tasks: [], | ||
| draining: false, | ||
| }; | ||
|
|
||
| return state; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix Prettier formatting issues flagged by CI.
The Lint / Format Check pipeline failed with Prettier reporting style issues in 2 files. Run npx prettier --write on the changed files and commit the result.
🧰 Tools
🪛 GitHub Actions: Lint / 👨🎨 Format Check
[error] 1-1: Command failed: npm run format (prettier src --check). Prettier reported code style issues in 2 files, exiting with code 1.
🤖 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/context/storage/TemporalFiles/application/upload/TemporalFileUploadQueue/state.ts`
around lines 1 - 11, Prettier formatting is out of sync in
createTemporalFileUploadQueueState, so update the affected file to match the
repo’s formatter output. Re-run formatting on the changed files with the
project’s Prettier command, then verify the formatting around
createTemporalFileUploadQueueState and the QueueState object literal is
consistent with CI expectations.
Source: Pipeline failures
|
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.
What is Changed / Added
opendir,getattr,open,release) to work with lazy hydration and the new sync behavior.Why
Summary by CodeRabbit
New Features
Bug Fixes
Infrastructure