From 5d6a4407a9d289c4b17e2af032d31761e803da6d Mon Sep 17 00:00:00 2001 From: Levy Tate <78818969+iLevyTate@users.noreply.github.com> Date: Thu, 12 Nov 2026 16:52:54 -0500 Subject: [PATCH 1/3] fix(organize): a file that fails the check after its move fails alone After each move the batch checks that the file is at its destination and gone from its source. A file that failed the check was a critical error: the batch stopped and every confirmed move was rolled back. The file that failed never joins the completed moves, so the rollback did not bring it back; it only undid the rest. - A failed check is a per-file failure. The row is listed as not moved and the other files stay moved and undoable. - "Destination missing" now always carries MOVE_VERIFICATION_DESTINATION_FAILED (it carried the raw ENOENT) and its message ends with whether the file is still at its original location. A destination that cannot be read for another reason keeps the filesystem's code, so EIO still stops the batch. - Failed checks share the escalation rule that permission denials already had: three in a row with no verified move in between roll the batch back. One counter, one predicate, one threshold. - Strict mode still stops on the first failure. - The renderer maps both codes and shows the handler's message. Without an entry the destination code fell through to the generic save-failed copy. --- CHANGELOG.md | 5 + src/main/ipc/files/batchOrganizeHandler.js | 122 ++++++++++------ src/renderer/utils/errorMapping.js | 13 ++ ...tchOrganizeHandler.failureHandling.test.js | 135 ++++++++++++++++++ test/batchOrganizeHandler.test.js | 24 ++++ test/errorMapping.fileOperations.test.js | 23 +++ 6 files changed, 280 insertions(+), 42 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a5411f1..bab90f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,11 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). undone could move on to the Complete screen as if it had worked. An identical copy that was removed instead of moved is listed as a duplicate, with the copy that was kept, and is no longer counted as a moved file. +- **One file that cannot be confirmed after its move no longer undoes the whole run.** After each + move the app checks that the file is at its destination and gone from its source. A file that + failed this check stopped the run and sent every file already moved back. It is now listed as not + moved, with whether it is still at its original location, and the other files stay moved. Three + such failures in a row, with no confirmed move between them, still undo the run. - **Files that are already in the folder chosen for them are pointed out before the run.** When a folder you sort from is also one of your Smart Folders, files matched to it stay where they are (or are renamed there). A notice on the Organize screen and a mark in Preview now say which, so a diff --git a/src/main/ipc/files/batchOrganizeHandler.js b/src/main/ipc/files/batchOrganizeHandler.js index 1d7d77d..c7243d9 100644 --- a/src/main/ipc/files/batchOrganizeHandler.js +++ b/src/main/ipc/files/batchOrganizeHandler.js @@ -267,6 +267,31 @@ const pLimit = (concurrency) => { // Use shared delay() from promiseUtils (supports .unref() for clean shutdown) +/** + * The two findings that only the move check reports: the file is not at its + * destination, or it is still at its source. See isEscalatingFileError for + * how the batch treats them. + */ +const MOVE_CHECK_DESTINATION_MISSING = 'MOVE_VERIFICATION_DESTINATION_FAILED'; +const MOVE_CHECK_SOURCE_REMAINS = 'MOVE_VERIFICATION_SOURCE_EXISTS'; + +/** + * Where the file is when the move check does not find it at its destination, + * as a sentence that ends the failure message. + * @param {string} source + * @returns {Promise} + */ +async function describeSourceAfterMissingDestination(source) { + try { + await fs.access(source); + return 'The file is still at its original location.'; + } catch (error) { + return error.code === 'ENOENT' + ? 'The file is no longer at its original location either.' + : 'Its original location could not be checked.'; + } +} + /** * Verify move operation completed correctly. * Checks that destination exists and source is gone. @@ -286,12 +311,15 @@ async function verifyMoveCompletion(source, destination, log) { const isRetryable = error.code === 'ENOENT' || (error.code && FILE_LOCK_ERROR_CODES.has(error.code)); if (!isRetryable || attempt === VERIFY_MAX_ATTEMPTS) { - const message = - error.code === 'ENOENT' - ? `Move verification failed: destination does not exist after move: ${destination}` - : `Move verification failed: destination not accessible after move: ${destination}`; + const missing = error.code === 'ENOENT'; + const message = missing + ? `Move verification failed: destination does not exist after move: ${destination}. ${await describeSourceAfterMissingDestination(source)}` + : `Move verification failed: destination not accessible after move: ${destination}`; const verificationError = /** @type {NodeJS.ErrnoException} */ (new Error(message)); - verificationError.code = error.code || 'MOVE_VERIFICATION_DESTINATION_FAILED'; + // A destination that cannot be read keeps the filesystem's code, so + // EIO still stops the batch and EACCES counts as a permission failure. + verificationError.code = + missing || !error.code ? MOVE_CHECK_DESTINATION_MISSING : error.code; throw verificationError; } log.debug('[FILE-OPS] Destination not yet visible after move, retrying', { @@ -321,7 +349,7 @@ async function verifyMoveCompletion(source, destination, log) { `Move verification failed: source file still exists at original location: ${source}` ) ); - verificationError.code = 'MOVE_VERIFICATION_SOURCE_EXISTS'; + verificationError.code = MOVE_CHECK_SOURCE_REMAINS; throw verificationError; } log.debug('[FILE-OPS] Source still exists after move, retrying', { @@ -512,9 +540,9 @@ async function handleBatchOrganize(params) { // batchId already defined above for lock acquisition let shouldRollback = false; let rollbackReason = null; - // Permission failures since the last successful move; a run of them - // means permissions are gone for the batch, not for one file. - let consecutivePermissionFailures = 0; + // Permission failures and failed move checks since the last verified + // move; a run of them means the destination is at fault, not one file. + let consecutiveEscalatingFailures = 0; const processedKeys = new Set(); // For idempotency const mkdirPromises = new Map(); // Deduplicate mkdir across concurrent pLimit workers let postBatchPersistence = { undoRecorded: true, pathIndexUpdated: true }; @@ -931,17 +959,16 @@ async function handleBatchOrganize(params) { await verifyMoveCompletion(op.source, resolvedDestination, log); } catch (verifyError) { // The source was removed as a duplicate and the copy it - // duplicated cannot be confirmed. Rollback has nothing to - // copy back, so the message is where the user learns that the - // file is in the trash. + // duplicated cannot be confirmed. Nothing in the batch puts it + // back, so the message is where the user learns that the file + // is in the trash. if (moveResult.sourceRemoval === 'trashed' && verifyError instanceof Error) { - verifyError.message += - '. The original was moved to the system Trash / Recycle Bin as a duplicate and can be restored from there.'; + verifyError.message = `${verifyError.message.replace(/\.?$/, '.')} The original was moved to the system Trash / Recycle Bin as a duplicate and can be restored from there.`; } throw verifyError; } - consecutivePermissionFailures = 0; + consecutiveEscalatingFailures = 0; // A duplicate skip still mutates disk state by deleting the source file. // Track it so a later critical failure can restore the original path. completedOperations.push({ @@ -1027,7 +1054,7 @@ async function handleBatchOrganize(params) { // Post-move verification: ensure destination exists and source is gone await verifyMoveCompletion(op.source, op.destination, log); - consecutivePermissionFailures = 0; + consecutiveEscalatingFailures = 0; // Push to completedOperations IMMEDIATELY after verify so rollback // can restore this file if another concurrent task triggers abort completedOperations.push({ @@ -1094,11 +1121,12 @@ async function handleBatchOrganize(params) { // Determine if critical error (always triggers rollback) or if // strict mode makes any failure terminal. One permission denial - // is a per-file failure; a run of them is systemic. + // or failed move check is a per-file failure; a run of them is + // systemic. let isCriticalError = isCriticalFileError(error); - if (isPermissionFileError(error)) { - consecutivePermissionFailures++; - if (consecutivePermissionFailures >= PERMISSION_FAILURE_ESCALATION_THRESHOLD) { + if (isEscalatingFileError(error)) { + consecutiveEscalatingFailures++; + if (consecutiveEscalatingFailures >= FAILURE_ESCALATION_THRESHOLD) { isCriticalError = true; } } @@ -1479,10 +1507,8 @@ async function handleBatchOrganize(params) { * Grouped by root cause: * - Read-only FS: EROFS * - Storage/quota: ENOSPC, EDQUOT, EFBIG - * - IO integrity: EIO, checksum mismatch, verification failed - * - Path integrity: ENAMETOOLONG, EUUID_EXHAUSTED, - * MOVE_VERIFICATION_DESTINATION_FAILED, - * MOVE_VERIFICATION_SOURCE_EXISTS + * - IO integrity: EIO, checksum mismatch + * - Path integrity: ENAMETOOLONG, EUUID_EXHAUSTED * - Resource pressure:EMFILE, ENFILE * - Process lifecycle:ECANCELED * @@ -1502,11 +1528,18 @@ async function handleBatchOrganize(params) { * are FILE_LOCK_ERROR_CODES above: a Windows AV/indexer hold, a * delete-pending handle, or one source folder the user cannot write * surfaces as EPERM/EACCES for that one file, and rolling back every other - * completed move for it discarded the whole organize. A run of - * PERMISSION_FAILURE_ESCALATION_THRESHOLD permission failures with no - * successful move in between is still treated as systemic (permissions - * revoked on the destination) and rolls the batch back; see - * isPermissionFileError and its use in handleBatchOrganize. + * completed move for it discarded the whole organize. + * + * NOTE: a failed move check (MOVE_VERIFICATION_DESTINATION_FAILED, + * MOVE_VERIFICATION_SOURCE_EXISTS) is NOT critical on its own either. The + * file that failed it never joins the completed moves, so a rollback would + * not bring it back; it only undid every move that had been confirmed. The + * failed row says where the file is. + * + * A run of FAILURE_ESCALATION_THRESHOLD of these two kinds of failure with no + * verified move in between is still treated as systemic (permissions revoked + * on the destination, or a destination that is losing files) and rolls the + * batch back; see isEscalatingFileError and its use in handleBatchOrganize. */ const CRITICAL_ERROR_CODES = new Set([ 'EROFS', @@ -1516,8 +1549,6 @@ const CRITICAL_ERROR_CODES = new Set([ 'EIO', 'ENAMETOOLONG', 'EUUID_EXHAUSTED', - 'MOVE_VERIFICATION_DESTINATION_FAILED', - 'MOVE_VERIFICATION_SOURCE_EXISTS', 'EMFILE', 'ENFILE', 'ECANCELED' @@ -1534,32 +1565,39 @@ const CRITICAL_MAPPED_ERROR_CODES = new Set([ 'TOO_MANY_OPEN_FILES' ]); -const CRITICAL_MESSAGE_FRAGMENTS = ['checksum mismatch', 'verification failed']; +const CRITICAL_MESSAGE_FRAGMENTS = ['checksum mismatch']; -const PERMISSION_ERROR_CODES = new Set([ +/** + * Codes that are a per-file failure once and systemic in a run: a permission + * or access denial, and the move check's two findings. + */ +const ESCALATING_ERROR_CODES = new Set([ 'EACCES', 'EPERM', 'FILE_ACCESS_DENIED', - 'PERMISSION_DENIED' + 'PERMISSION_DENIED', + MOVE_CHECK_DESTINATION_MISSING, + MOVE_CHECK_SOURCE_REMAINS ]); /** - * Permission failures in a row (no successful move in between) after which - * the batch treats them as systemic and rolls back, as it does for ENOSPC. + * Escalating failures in a row (no verified move in between) after which the + * batch treats them as systemic and rolls back, as it does for ENOSPC. */ -const PERMISSION_FAILURE_ESCALATION_THRESHOLD = 3; +const FAILURE_ESCALATION_THRESHOLD = 3; /** - * Whether a failed op's error is a permission/access denial. On its own that - * is a per-file failure (see CRITICAL_ERROR_CODES); repeated, it escalates. + * Whether a failed op's error is one of ESCALATING_ERROR_CODES. On its own + * that is a per-file failure (see CRITICAL_ERROR_CODES); repeated, it + * escalates. * @param {any} error * @returns {boolean} */ -function isPermissionFileError(error) { +function isEscalatingFileError(error) { if (!error || typeof error !== 'object') return false; - if (typeof error.code === 'string' && PERMISSION_ERROR_CODES.has(error.code)) return true; + if (typeof error.code === 'string' && ESCALATING_ERROR_CODES.has(error.code)) return true; const originalCode = error.metadata?.originalCode; - return typeof originalCode === 'string' && PERMISSION_ERROR_CODES.has(originalCode); + return typeof originalCode === 'string' && ESCALATING_ERROR_CODES.has(originalCode); } /** diff --git a/src/renderer/utils/errorMapping.js b/src/renderer/utils/errorMapping.js index 7a06765..b63b33c 100644 --- a/src/renderer/utils/errorMapping.js +++ b/src/renderer/utils/errorMapping.js @@ -103,6 +103,19 @@ const FILE_OPERATION_CODE_MAP = { message: 'That folder leads into a system folder through a link or short name. Choose a different folder.', useHandlerMessage: true + }, + // Organize (batchOrganizeHandler.js): the move ran but the check after it + // did not find the file where it should be. The handler's message names the + // path and says where the file is, so it is shown as is. + MOVE_VERIFICATION_DESTINATION_FAILED: { + severity: 'error', + message: 'The move could not be confirmed: the file was not found at its destination.', + useHandlerMessage: true + }, + MOVE_VERIFICATION_SOURCE_EXISTS: { + severity: 'error', + message: 'The move could not be confirmed: the file is still at its original location.', + useHandlerMessage: true } }; diff --git a/test/batchOrganizeHandler.failureHandling.test.js b/test/batchOrganizeHandler.failureHandling.test.js index 011f966..a9a7b2b 100644 --- a/test/batchOrganizeHandler.failureHandling.test.js +++ b/test/batchOrganizeHandler.failureHandling.test.js @@ -8,6 +8,8 @@ * destination's parent directory is missing. Covers: * - one file's permission error (EPERM/EACCES) is a per-file failure, not a * batch-wide rollback; repeated permission errors still roll back + * - one file that fails the check after its move is a per-file failure too; + * a run of them still rolls back * - a destination folder removed mid-batch is recreated, and is never * reported as a missing source * - a cross-device move that loses the publish race walks to the next @@ -25,6 +27,8 @@ const mockState = { dirs: new Set(), /** @type {((src: string, dest: string) => any) | null} */ renameHook: null, + /** Runs once the rename has been applied. @type {((src: string, dest: string) => void) | null} */ + afterRenameHook: null, /** @type {((src: string, dest: string) => void) | null} */ copyHook: null, /** @type {Array<[string, string]>} */ @@ -78,6 +82,7 @@ jest.mock('fs', () => { // Real rename semantics: an occupied destination is REPLACED. mockState.files.set(d, mockState.files.get(s)); mockState.files.delete(s); + if (mockState.afterRenameHook) mockState.afterRenameHook(s, d); }), copyFile: jest.fn(async (src, dest, mode) => { const s = mockNorm(src); @@ -233,6 +238,7 @@ beforeEach(() => { mockState.files.clear(); mockState.dirs.clear(); mockState.renameHook = null; + mockState.afterRenameHook = null; mockState.copyHook = null; mockState.renames = []; mockState.mkdirs = []; @@ -294,6 +300,135 @@ describe('one file with a permission error', () => { }); }); +describe('one file that fails the check after its move', () => { + // Something else (a sync client, an antivirus quarantine) takes the file + // right after it lands. + const vanishAfterMove = (shouldVanish) => { + mockState.afterRenameHook = (s, d) => { + if (s.startsWith(R('/src')) && shouldVanish(s)) mockState.files.delete(d); + }; + }; + + test('a file missing from its destination fails alone; the rest stay moved and undoable', async () => { + const ops = makeOps(5); + vanishAfterMove((s) => s === R('/src/doc3.pdf')); + + const { result, recordAction } = await run(ops); + + expect(executeRollback).not.toHaveBeenCalled(); + expect(result.rolledBack).toBeUndefined(); + expect(result.successCount).toBe(4); + expect(result.failCount).toBe(1); + expect(resultFor(result, 3)).toMatchObject({ + success: false, + errorCode: 'MOVE_VERIFICATION_DESTINATION_FAILED', + critical: false + }); + expect(resultFor(result, 3).error).toBe( + `Move verification failed: destination does not exist after move: ${R('/dest/doc3.pdf')}. The file is no longer at its original location either.` + ); + for (const k of [1, 2, 4, 5]) { + expect(mockState.files.get(R(`/dest/doc${k}.pdf`))).toBe(`DOC-${k}`); + } + // Undo covers the four confirmed moves and not the one that failed. + expect(recordAction).toHaveBeenCalledTimes(1); + expect(recordAction.mock.calls[0][1].operations.map((o) => o.originalPath).sort()).toEqual( + [1, 2, 4, 5].map((k) => R(`/src/doc${k}.pdf`)) + ); + }); + + test('a file still at its source after the move fails alone and says so', async () => { + const ops = makeOps(3); + mockState.afterRenameHook = (s) => { + if (s === R('/src/doc2.pdf')) mockState.files.set(s, 'DOC-2'); + }; + + const { result } = await run(ops); + + expect(executeRollback).not.toHaveBeenCalled(); + expect(result.successCount).toBe(2); + expect(resultFor(result, 2)).toMatchObject({ + success: false, + errorCode: 'MOVE_VERIFICATION_SOURCE_EXISTS', + critical: false + }); + expect(resultFor(result, 2).error).toContain(R('/src/doc2.pdf')); + }); + + test('a destination that is missing while the source is back says the file is still there', async () => { + const ops = makeOps(1); + mockState.afterRenameHook = (s, d) => { + mockState.files.set(s, mockState.files.get(d)); + mockState.files.delete(d); + }; + + const { result } = await run(ops); + + expect(resultFor(result, 1).error).toMatch(/The file is still at its original location\.$/); + expect(result.successCount).toBe(0); + expect(result.failCount).toBe(1); + }); + + test('files vanishing from the destination several times in a row still roll the batch back', async () => { + const ops = makeOps(8); + vanishAfterMove((s) => s !== R('/src/doc1.pdf')); + + const { result } = await run(ops); + + expect(executeRollback).toHaveBeenCalledTimes(1); + expect(result.rolledBack).toBe(true); + // doc1 was confirmed and is returned by the rollback. + expect(mockState.files.get(R('/src/doc1.pdf'))).toBe('DOC-1'); + expect(mockState.files.has(R('/dest/doc1.pdf'))).toBe(false); + }); + + test('a confirmed move between two failed checks keeps them per-file', async () => { + // A move whose source is another move's destination makes the batch run + // one file at a time, so the order below is the order on disk. + seedFile('/src/chain.pdf', 'CHAIN'); + const chain = [ + { source: '/src/chain.pdf', destination: '/stage/chain.pdf', type: 'move' }, + { source: '/stage/chain.pdf', destination: '/dest/chain.pdf', type: 'move' } + ]; + // doc1, doc3 and doc5 fail; doc2, doc4 and doc6 are confirmed in between. + vanishAfterMove((s) => /doc[135]\.pdf$/.test(s)); + + const { result } = await run(chain.concat(makeOps(6))); + + expect(executeRollback).not.toHaveBeenCalled(); + expect(result.successCount).toBe(5); + expect(result.failCount).toBe(3); + }); + + test('strict mode still rolls back on the first failed check', async () => { + const ops = makeOps(3); + vanishAfterMove((s) => s === R('/src/doc3.pdf')); + + const { result } = await run(ops, { strict: true }); + + expect(result.rolledBack).toBe(true); + expect(resultFor(result, 3)).toMatchObject({ critical: false, strictModeTriggered: true }); + }); + + test('a destination that cannot be read for another reason keeps its own code', async () => { + const fsPromises = require('fs').promises; + const ops = makeOps(3); + const realAccess = fsPromises.access.getMockImplementation(); + fsPromises.access.mockImplementation(async (fp) => { + if (mockNorm(fp) === R('/dest/doc3.pdf')) throw mockErr('EIO', 'i/o error'); + return realAccess(fp); + }); + + try { + const { result } = await run(ops); + expect(result.rolledBack).toBe(true); + expect(result.results.find((r) => r.errorCode === 'EIO')).toMatchObject({ critical: true }); + } finally { + fsPromises.access.mockImplementation(realAccess); + } + }); +}); + describe('destination folder removed mid-batch', () => { test('later moves recreate the folder instead of reporting the source missing', async () => { const ops = makeOps(4, '/dest/D'); diff --git a/test/batchOrganizeHandler.test.js b/test/batchOrganizeHandler.test.js index b354067..9a15dfb 100644 --- a/test/batchOrganizeHandler.test.js +++ b/test/batchOrganizeHandler.test.js @@ -1208,6 +1208,30 @@ describe('Batch Organize Handler', () => { }) ).toBe(false); }); + + test.each([ + [ + 'MOVE_VERIFICATION_DESTINATION_FAILED', + 'Move verification failed: destination does not exist after move: /dest/a.pdf. The file is still at its original location.' + ], + [ + 'MOVE_VERIFICATION_SOURCE_EXISTS', + 'Move verification failed: source file still exists at original location: /src/a.pdf' + ] + ])('treats one failed move check (%s) as a per-file failure', (code, message) => { + // The file never joins the completed moves, so a rollback would undo + // every other file without bringing this one back. + expect(isCriticalFileError({ code, message })).toBe(false); + }); + + test('still treats a checksum mismatch as critical', () => { + expect( + isCriticalFileError({ + code: 'INTEGRITY_ERROR', + message: 'File checksum mismatch - possible corruption: /dest/a.pdf' + }) + ).toBe(true); + }); }); describe('recordUndoAndUpdateDatabase', () => { diff --git a/test/errorMapping.fileOperations.test.js b/test/errorMapping.fileOperations.test.js index 9f0af43..c7875d2 100644 --- a/test/errorMapping.fileOperations.test.js +++ b/test/errorMapping.fileOperations.test.js @@ -104,6 +104,29 @@ describe('errorMapping: file operation codes', () => { }); }); + describe('a move the check after it could not confirm', () => { + const HANDLER_MESSAGE = + 'Move verification failed: destination does not exist after move: /dest/a.pdf. The file is still at its original location.'; + + test('shows where the file is, not the generic save failure', () => { + const mapped = mapErrorToNotification({ + error: HANDLER_MESSAGE, + errorCode: 'MOVE_VERIFICATION_DESTINATION_FAILED' + }); + expect(mapped).toMatchObject({ severity: 'error', message: HANDLER_MESSAGE }); + }); + + test.each([ + ['MOVE_VERIFICATION_DESTINATION_FAILED', /not found at its destination/], + ['MOVE_VERIFICATION_SOURCE_EXISTS', /still at its original location/] + ])('%s without a handler message still says what happened', (errorCode, expected) => { + expect(mapErrorToNotification({ errorCode })).toMatchObject({ + severity: 'error', + message: expect.stringMatching(expected) + }); + }); + }); + test('an unknown code falls back to the handler message', () => { const mapped = mapErrorToNotification({ error: 'Invalid destination path: something odd', From ff6be043a038f749fb0cdd4edbc280dedcdde62c Mon Sep 17 00:00:00 2001 From: Levy Tate <78818969+iLevyTate@users.noreply.github.com> Date: Thu, 12 Nov 2026 18:45:07 -0500 Subject: [PATCH 2/3] test(organize): check the move-check failure on the real filesystem The existing tests for a file that fails the check after its move run on a mocked filesystem. This one runs the real batch handler, the real move, the real check and the real rollback in a temp folder. The only stand-in is the other program: a real filesystem call made right after a file lands at its destination. - A file removed from its destination fails alone; the others stay moved. - A file put back at its source fails alone and is reported as still there. - A file present at both places fails alone and neither copy is touched. - A destination that keeps losing files is rolled back. With the handler from main the first three fail: the whole run is rolled back. --- ...rganize.moveCheckFailsAlone.realfs.test.js | 249 ++++++++++++++++++ 1 file changed, 249 insertions(+) create mode 100644 test/batchOrganize.moveCheckFailsAlone.realfs.test.js diff --git a/test/batchOrganize.moveCheckFailsAlone.realfs.test.js b/test/batchOrganize.moveCheckFailsAlone.realfs.test.js new file mode 100644 index 0000000..95723b1 --- /dev/null +++ b/test/batchOrganize.moveCheckFailsAlone.realfs.test.js @@ -0,0 +1,249 @@ +/** + * @jest-environment node + */ +/** + * A file that is not where it should be right after its move (a sync client + * or an antivirus took it, or put it back) is one failed row; the rest of the + * run stays moved. Runs the real batch handler, the real move, the real check + * after it and the real rollback against a temp dir. + * + * The only stand-in is the other program: `afterEachMove` runs a real + * filesystem call of its own right after a file lands, which is the one + * moment the check cannot see past. + */ + +jest.unmock('fs'); +jest.unmock('fs/promises'); + +jest.mock('../src/main/ipc/files/embeddingSync', () => ({ + removeEmbeddingsForPathBestEffort: jest.fn(), + syncEmbeddingForMove: jest.fn(), + verifyEmbeddingAfterMove: jest.fn().mockResolvedValue({ valid: true, issues: [] }) +})); + +jest.mock('../src/main/ipc/files/batchLockManager', () => ({ + acquireBatchLock: jest.fn().mockResolvedValue(true), + renewBatchLock: jest.fn().mockReturnValue(true), + releaseBatchLock: jest.fn() +})); + +jest.mock('../src/main/ipc/files/batchProgressReporter', () => ({ + sendFileOperationBatchComplete: jest.fn(), + sendOperationProgress: jest.fn(), + sendChunkedResults: jest.fn().mockResolvedValue({ sent: true }) +})); + +jest.mock('../src/main/services/ServiceContainer', () => ({ + container: { has: jest.fn(() => false), resolve: jest.fn(), tryResolve: jest.fn(() => null) }, + ServiceIds: { FILE_PATH_COORDINATOR: 'filePathCoordinator', LEARNING_FEEDBACK: 'lf' } +})); + +jest.mock('../src/main/ipc/files/destinationPolicy', () => ({ + authorizeDestinations: jest.fn().mockResolvedValue(new Map()), + authorizeDestination: jest.fn().mockResolvedValue({ allowed: true }), + decisionForDestination: jest.fn(() => ({ allowed: true })), + buildStrictFileOpsAllowlist: jest.fn(() => ({ enabled: false, allowlist: [] })) +})); + +jest.mock('../src/shared/pathSanitization', () => ({ + validateFileOperationPath: jest.fn(async (p) => ({ + valid: true, + normalizedPath: require('path').resolve(p) + })), + normalizePathForIndex: jest.fn((p) => String(p).replace(/\\/g, '/')) +})); + +jest.mock('../src/main/services/FolderMatchingService', () => ({ + matchCategoryToFolder: jest.fn(() => null) +})); + +jest.mock('../src/main/ipc/semantic', () => ({ + getSearchServiceInstance: jest.fn(() => null) +})); + +const fs = require('fs'); +const fsp = fs.promises; +const path = require('path'); +const realOs = jest.requireActual('os'); + +const { handleBatchOrganize } = require('../src/main/ipc/files/batchOrganizeHandler'); + +const silentLog = { info() {}, warn() {}, error() {}, debug() {} }; + +/** @type {string[]} */ +const tempRoots = []; + +async function makeRoot() { + const root = await fsp.mkdtemp(path.join(realOs.tmpdir(), 'ss-move-check-')); + tempRoots.push(root); + return root; +} + +afterEach(() => { + jest.restoreAllMocks(); +}); + +afterAll(async () => { + for (const root of tempRoots) { + await fsp.rm(root, { recursive: true, force: true }); + } +}); + +/** + * Every file under `dir`, as sorted forward-slash paths relative to it. + * @param {string} dir + * @param {string} [base] + * @returns {Promise} + */ +async function tree(dir, base = dir) { + const out = []; + for (const entry of await fsp.readdir(dir, { withFileTypes: true })) { + const full = path.join(dir, entry.name); + if (entry.isDirectory()) out.push(...(await tree(full, base))); + else out.push(path.relative(base, full).replace(/\\/g, '/')); + } + return out.sort(); +} + +/** + * `count` files in Input, each to be moved to Sorted under the same name. + * @param {string} root + * @param {number} count + */ +async function seedMoves(root, count) { + const moves = []; + await fsp.mkdir(path.join(root, 'Input'), { recursive: true }); + for (let k = 1; k <= count; k++) { + const source = path.join(root, 'Input', `doc${k}.txt`); + await fsp.writeFile(source, `DOC-${k}`); + moves.push({ source, destination: path.join(root, 'Sorted', `doc${k}.txt`) }); + } + return moves; +} + +/** @param {{ source: string, destination: string }[]} moves */ +function organize(moves) { + return handleBatchOrganize({ + operation: { operations: moves.map((move) => ({ type: 'move', ...move })) }, + logger: silentLog, + getServiceIntegration: () => null, + getMainWindow: () => null + }); +} + +/** + * The other program with a hand on the folder: `act` runs right after a file + * has really landed in Sorted, before the handler looks at it. + * @param {(source: string, destination: string) => Promise} act + */ +function afterEachMove(act) { + const realRename = fsp.rename.bind(fsp); + jest.spyOn(fsp, 'rename').mockImplementation(async (from, to) => { + await realRename(from, to); + if (path.basename(path.dirname(String(to))) === 'Sorted') { + await act(String(from), String(to)); + } + }); +} + +/** + * @param {any} result + * @param {string} name + */ +const rowFor = (result, name) => + result.results.find((/** @type {any} */ row) => path.basename(row.source) === name); + +describe('a file that is not where it should be after its move (real filesystem)', () => { + test('a file taken from its destination fails alone; the others stay moved', async () => { + const root = await makeRoot(); + const moves = await seedMoves(root, 5); + afterEachMove(async (_source, destination) => { + if (path.basename(destination) === 'doc3.txt') await fsp.unlink(destination); + }); + + const result = await organize(moves); + + expect(result.rolledBack).toBeFalsy(); + expect(result.successCount).toBe(4); + expect(result.failCount).toBe(1); + expect(rowFor(result, 'doc3.txt')).toMatchObject({ + success: false, + errorCode: 'MOVE_VERIFICATION_DESTINATION_FAILED', + critical: false + }); + expect(rowFor(result, 'doc3.txt').error).toBe( + `Move verification failed: destination does not exist after move: ${moves[2].destination}. The file is no longer at its original location either.` + ); + expect(await tree(root)).toEqual([ + 'Sorted/doc1.txt', + 'Sorted/doc2.txt', + 'Sorted/doc4.txt', + 'Sorted/doc5.txt' + ]); + }); + + test('a file put back where it came from fails alone and is reported as still there', async () => { + const root = await makeRoot(); + const moves = await seedMoves(root, 3); + afterEachMove(async (source, destination) => { + if (path.basename(destination) === 'doc2.txt') await fsp.rename(destination, source); + }); + + const result = await organize(moves); + + expect(result.rolledBack).toBeFalsy(); + expect(result.successCount).toBe(2); + expect(rowFor(result, 'doc2.txt')).toMatchObject({ + success: false, + errorCode: 'MOVE_VERIFICATION_DESTINATION_FAILED' + }); + expect(rowFor(result, 'doc2.txt').error).toMatch( + /The file is still at its original location\.$/ + ); + expect(await tree(root)).toEqual(['Input/doc2.txt', 'Sorted/doc1.txt', 'Sorted/doc3.txt']); + expect(await fsp.readFile(moves[1].source, 'utf8')).toBe('DOC-2'); + }); + + test('a file that is at both places after its move fails alone, and neither copy is touched', async () => { + const root = await makeRoot(); + const moves = await seedMoves(root, 3); + afterEachMove(async (source, destination) => { + if (path.basename(destination) === 'doc2.txt') await fsp.copyFile(destination, source); + }); + + const result = await organize(moves); + + expect(result.rolledBack).toBeFalsy(); + expect(result.successCount).toBe(2); + expect(rowFor(result, 'doc2.txt')).toMatchObject({ + success: false, + errorCode: 'MOVE_VERIFICATION_SOURCE_EXISTS', + critical: false + }); + expect(rowFor(result, 'doc2.txt').error).toContain(moves[1].source); + expect(await tree(root)).toEqual([ + 'Input/doc2.txt', + 'Sorted/doc1.txt', + 'Sorted/doc2.txt', + 'Sorted/doc3.txt' + ]); + }); + + test('a destination that keeps losing files is rolled back, and the confirmed file returns', async () => { + const root = await makeRoot(); + const moves = await seedMoves(root, 8); + // One confirmed move can split seven failures into two runs at most, so + // one run reaches the three in a row that make it systemic. + afterEachMove(async (_source, destination) => { + if (path.basename(destination) !== 'doc1.txt') await fsp.unlink(destination); + }); + + const result = await organize(moves); + + expect(result.rolledBack).toBe(true); + const after = await tree(root); + expect(after).toContain('Input/doc1.txt'); + expect(after.filter((file) => file.startsWith('Sorted/'))).toEqual([]); + expect(await fsp.readFile(moves[0].source, 'utf8')).toBe('DOC-1'); + }); +}); From cb5d973738c40461a46817217f6d37bbe8ea9158 Mon Sep 17 00:00:00 2001 From: Levy Tate <78818969+iLevyTate@users.noreply.github.com> Date: Thu, 12 Nov 2026 18:45:16 -0500 Subject: [PATCH 3/3] build(deps): bump @hono/node-server to 2.1.6 npm audit --omit=dev started failing on 2026-11-12: versions 2.0.0 to 2.1.5 carry two moderate advisories (GHSA-qv5m-9pm6-hqp3, GHSA-jpvv-r467-5p4w). It is a transitive dependency of @modelcontextprotocol/sdk. Only its lockfile entry and its row in the commercial-use inventory change. 2.1.6 (2026-11-02) is the first patched release; 2.1.8 was published hours before this commit and is left for a later update. --- docs/commercial-use-inventory.json | 4 ++-- package-lock.json | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/commercial-use-inventory.json b/docs/commercial-use-inventory.json index 93cd7ff..73f2659 100644 --- a/docs/commercial-use-inventory.json +++ b/docs/commercial-use-inventory.json @@ -1,5 +1,5 @@ { - "generatedAt": "2026-11-12T01:40:35.862Z", + "generatedAt": "2026-11-12T23:23:23.866Z", "projectLicense": { "riskLevel": "allowed", "reason": "Project license is PolyForm Noncommercial 1.0.0 for personal/noncommercial use. Commercial rights are sold separately; third-party dependency obligations remain governed by their own licenses.", @@ -6091,7 +6091,7 @@ }, { "packageName": "@hono/node-server", - "version": "2.1.5", + "version": "2.1.6", "license": "MIT", "riskLevel": "allowed", "directRuntime": false, diff --git a/package-lock.json b/package-lock.json index 6488199..6012765 100644 --- a/package-lock.json +++ b/package-lock.json @@ -8678,9 +8678,9 @@ } }, "node_modules/@hono/node-server": { - "version": "2.1.5", - "resolved": "https://registry.npmjs.org/@hono/node-server/-/node-server-2.1.5.tgz", - "integrity": "sha512-MgP/I06GdvsilDspMk/qmRzpk8LOUStP6jFAbSRi40W09DhoyUn+Mj7eyKICPqeYKybOjVFLVIAQzvu1bfIa7Q==", + "version": "2.1.6", + "resolved": "https://registry.npmjs.org/@hono/node-server/-/node-server-2.1.6.tgz", + "integrity": "sha512-p/KH9MnfNcsm3sR6x4lNgm5j+QN4Et8srKredl9DZfXiyZ1UQ3aQ2YVpFLbqktYN5JI+DB+VZFKM2PqE5hLKjQ==", "license": "MIT", "engines": { "node": ">=20"