Repository navigation
fix(organize): a lost temporary copy is not reported as a missing original - #126
Merged
Merged
Conversation
npm audit --omit=dev started failing on 2026-11-12, after the checks on #124 had passed: versions up to 4.13.15 carry three moderate advisories (GHSA-4m76-xfj5-7975, GHSA-h7mg-cq63-vhpq, GHSA-3gg2-335m-rxc6). The build of main at 82f912e stopped at that step on all three platforms. hono is a transitive dependency of @modelcontextprotocol/sdk. Only its lockfile entry and its row in the commercial-use inventory change. 4.13.16 (2026-11-01) is the first patched release. 4.13.17 (2026-11-07) is used; 4.13.18 was published a day before this commit and is left for a later update.
…ginal A move to another drive copies the file to a temporary name in the destination folder, checks it, and renames it into place. Every failure after the copy was labelled with the source path, so a temporary copy that another program removed or held read as "File not found: <source>" while the source was still there. - crossDeviceMove names the file the failure is about. Node reports the path a failed call was given as error.path; a failure on the staging path is a FileSystemError with the new code STAGED_COPY_FAILED. Its message names the destination folder, says whether the copy was removed (ENOENT) or could not be read or renamed (with the code), and says the original was not moved. - A destination name that cannot be probed is reported with the destination path, not the source path. - The batch handler counts STAGED_COPY_FAILED with the move check's findings: one is a per-file failure, three in a row with no confirmed move between them roll the batch back. A removed copy used to arrive as FILE_NOT_FOUND, which never counted. - The renderer maps the code and shows the handler's message. - constants.test.js checks that the two copies of FILE_SYSTEM_ERROR_CODES (src/shared/constants.js and src/main/errors/FileSystemError.js) match.
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
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.
Follow-up to #124, which listed this under "Not in this PR": when another program removes the temporary copy during a move to another drive, the row said "File not found:" although the source was still there.
What changes
The move between drives (
atomicFileOperations.js)crossDeviceMovecopies the file to a temporary name in the destination folder, checks it, and renames it into place. Its catch labelled every failure after the copy with the source path.error.path. A failure on the temporary copy is aFileSystemErrorwith the new codeSTAGED_COPY_FAILED. The message names the destination folder, says whether the copy was removed (ENOENT) or could not be read or renamed (with the code), and says the original was not moved:The temporary copy in D:\Sorted was removed before the move finished. The original was not moved: C:\Input\scan.pdfFILE_NOT_FOUNDwith the source path.Escalation (
batchOrganizeHandler.js) — a behaviour changeSTAGED_COPY_FAILEDjoins the codes that are a per-file failure once and systemic in a run. Three in a row with no confirmed move between them roll the batch back, as failed move checks do since fix(organize): a file that fails the check after its move fails alone #124.FILE_NOT_FOUND, which never counted toward that rule. A destination that removed every temporary copy failed every file one by one and the batch ran to the end. It now stops after three.Renderer (
errorMapping.js)Also in this PR
hono4.13.15 → 4.13.17 in the lockfile and the inventory (first commit).npm audit --omit=devstarted failing on a new advisory after fix(organize): a file that fails the check after its move fails alone #124 merged, which failedaudit:productionon all three platforms of themainbuild (run 43343195582).test/constants.test.jschecks that the two copies ofFILE_SYSTEM_ERROR_CODES(src/shared/constants.jsandsrc/main/errors/FileSystemError.js) are the same table.Not in this PR
rolledBackflag. Files that were mid-move get aninterruptedrow, files that had not started get no row, and the summary counts neither ("0 successful, 4 failed" for a run of 8). This is the same for permission denials onmainand predates this PR. It needs its own change.copyFileis still labelled with the source path. Itserror.pathis the source whichever side failed, so the two cannot be told apart from the error.mkdirof the destination folder is still labelled with the source path.Checks
Windows 11 x64, on this branch's tree:
npm run local:gate, every step: 957 test suites, 14158 tests passed, 11 skipped, coverage thresholds met, production build, native modules.New tests run on the real filesystem in a temp folder.
test/atomicFileOperations.crossDeviceCollision.test.js(7 added): the copy removed right after the copy, while its checksum is read, and between the check and the rename; the copy held by another program (EPERM); a destination name that cannot be probed; a source that really is missing;atomicMovereporting it once.test/batchOrganize.moveCheckFailsAlone.realfs.test.js(2 added): one file fails alone and four stay moved; a destination that keeps taking copies is rolled back and the confirmed file returns.With the
atomicFileOperations.jschange stashed, 5 of the 7 new move tests fail; with it restored they pass.With
STAGED_COPY_FAILEDtaken out of the escalating codes, the batch test "a destination that keeps taking them is rolled back" fails (no rollback); with it restored it passes.In the temp folder both folders are on one drive, so the tests refuse the first rename with
EXDEVto reach the cross-drive path. Everything after that is real.One-off, not in the repo:
crossDeviceMovefrom C: (NTFS) to an external FAT32 drive. An untouched move arrives; with the temporary copy removed right after it is written, the call rejects withSTAGED_COPY_FAILED, the message names the folder on the external drive, the original is intact and nothing is left in the destination folder.Not checked: macOS (pull requests run Ubuntu and Windows; the real-filesystem tests run on macOS when
mainnext builds), a packaged build, the Organize screen showing the new message, and a real sync client or antivirus product. In every check the copy is removed by a stand-in inside the test process.