From bbfdc716a55ec03d8de08729c2d0895929e745c2 Mon Sep 17 00:00:00 2001 From: jelveh Date: Sun, 2 Aug 2026 22:08:21 -0700 Subject: [PATCH] test: pin the v1 overwrite/collision wire contract for move and copy The collision tests asserted only statusCode 409, which is how the item_with_same_name_exists code regressed to 'conflict' unnoticed and broke every replace/skip prompt in the GUI. Assert the legacy code and entry_name explicitly, and add controller tests for the overwrite path: the replaced entry must ride along as 'overwritten' in the /copy and /move responses and be announced via outer.gui.item.removed so clients drop its row. --- .../controllers/fs/LegacyFSController.test.ts | 164 ++++++++++++++++++ src/backend/services/fs/FSService.test.ts | 25 +-- 2 files changed, 179 insertions(+), 10 deletions(-) diff --git a/src/backend/controllers/fs/LegacyFSController.test.ts b/src/backend/controllers/fs/LegacyFSController.test.ts index 4954c58f1..0b02dd530 100644 --- a/src/backend/controllers/fs/LegacyFSController.test.ts +++ b/src/backend/controllers/fs/LegacyFSController.test.ts @@ -928,6 +928,87 @@ describe('LegacyFSController.copy', () => { // The entry must still exist — the ghost handler must NOT have run. expect(await server.stores.fsEntry.getEntryByPath(src)).not.toBeNull(); }); + + it('surfaces a name collision, then reports and removes the replaced entry on overwrite', async () => { + const { actor } = await makeUser(); + const username = actor.user!.username!; + const src = `/${username}/Documents/dup.txt`; + const existing = `/${username}/Pictures/dup.txt`; + for ( const p of [src, existing] ) { + await withActor(actor, () => + controller.touch( + makeReq({ body: { path: p }, actor }), + makeRes().res, + ), + ); + } + + // Without overwrite: the v1 conflict contract the GUI's + // replace/skip prompts key on. + await expect( + withActor(actor, () => + controller.copy( + makeReq({ + body: { + source: src, + destination: `/${username}/Pictures`, + }, + actor, + }), + makeRes().res, + ), + ), + ).rejects.toMatchObject({ + statusCode: 409, + legacyCode: 'item_with_same_name_exists', + fields: { entry_name: 'dup.txt' }, + }); + + const replaced = (await server.stores.fsEntry.getEntryByPath( + existing, + ))!; + + // With overwrite: the replaced entry rides along in the response + // (so the caller can drop its row) and item.removed tells every + // other client to do the same — without it they keep a ghost row + // until the directory is re-listed. + const emitSpy = vi.spyOn(server.clients.event, 'emit'); + let body: Array<{ + copied: { path: string }; + overwritten?: { id: string }; + }>; + let removedCall: (typeof emitSpy.mock.calls)[number] | undefined; + try { + const { res, captured } = makeRes(); + await withActor(actor, () => + controller.copy( + makeReq({ + body: { + source: src, + destination: `/${username}/Pictures`, + overwrite: true, + }, + actor, + }), + res, + ), + ); + body = captured.body as typeof body; + removedCall = emitSpy.mock.calls.find( + ([eventName]) => eventName === 'outer.gui.item.removed', + ); + } finally { + emitSpy.mockRestore(); + } + + expect(body[0].copied.path).toBe(existing); + expect(body[0].overwritten?.id).toBe(replaced.uuid); + expect(removedCall).toBeTruthy(); + const removedPayload = removedCall?.[1] as { + response?: { uid?: string }; + }; + expect(removedPayload.response?.uid).toBe(replaced.uuid); + }); }); // ── move ──────────────────────────────────────────────────────────── @@ -1004,6 +1085,89 @@ describe('LegacyFSController.move', () => { const body = captured.body as { moved: { path: string } }; expect(body.moved.path).toBe(`/${username}/Pictures/bar`); }); + + it('surfaces a name collision, then reports and removes the replaced entry on overwrite', async () => { + const { actor } = await makeUser(); + const username = actor.user!.username!; + const src = `/${username}/Documents/clash.txt`; + const existing = `/${username}/Pictures/clash.txt`; + for ( const p of [src, existing] ) { + await withActor(actor, () => + controller.touch( + makeReq({ body: { path: p }, actor }), + makeRes().res, + ), + ); + } + + // Without overwrite: the v1 conflict contract the GUI's + // replace/skip prompts key on. + await expect( + withActor(actor, () => + controller.move( + makeReq({ + body: { + source: src, + destination: `/${username}/Pictures`, + }, + actor, + }), + makeRes().res, + ), + ), + ).rejects.toMatchObject({ + statusCode: 409, + legacyCode: 'item_with_same_name_exists', + fields: { entry_name: 'clash.txt' }, + }); + + const replaced = (await server.stores.fsEntry.getEntryByPath( + existing, + ))!; + + // With overwrite: the replaced entry rides along in the response + // (so the caller can drop its row) and item.removed tells every + // other client to do the same — without it they keep a ghost row + // until the directory is re-listed. + const emitSpy = vi.spyOn(server.clients.event, 'emit'); + let body: { + moved: { path: string }; + old_path: string; + overwritten?: { id: string }; + }; + let removedCall: (typeof emitSpy.mock.calls)[number] | undefined; + try { + const { res, captured } = makeRes(); + await withActor(actor, () => + controller.move( + makeReq({ + body: { + source: src, + destination: `/${username}/Pictures`, + overwrite: true, + }, + actor, + }), + res, + ), + ); + body = captured.body as typeof body; + removedCall = emitSpy.mock.calls.find( + ([eventName]) => eventName === 'outer.gui.item.removed', + ); + } finally { + emitSpy.mockRestore(); + } + + expect(body.old_path).toBe(src); + expect(body.moved.path).toBe(existing); + expect(body.overwritten?.id).toBe(replaced.uuid); + expect(removedCall).toBeTruthy(); + const removedPayload = removedCall?.[1] as { + response?: { uid?: string }; + }; + expect(removedPayload.response?.uid).toBe(replaced.uuid); + }); }); // ── search ────────────────────────────────────────────────────────── diff --git a/src/backend/services/fs/FSService.test.ts b/src/backend/services/fs/FSService.test.ts index 0ea28c5bd..ad39d2d57 100644 --- a/src/backend/services/fs/FSService.test.ts +++ b/src/backend/services/fs/FSService.test.ts @@ -2248,6 +2248,10 @@ describe('FSService move', () => { fs.move(user.userId, { source, destinationParent: destination }), ); expect(conflict.statusCode).toBe(409); + // v1 wire contract: the GUI's replace/skip prompts key on this + // code + entry_name; a generic 'conflict' makes them fail silently. + expect(conflict.legacyCode).toBe('item_with_same_name_exists'); + expect(conflict.fields).toMatchObject({ entry_name: 'coll.txt' }); const deduped = await fs.move(user.userId, { source, @@ -2461,16 +2465,17 @@ describe('FSService copy', () => { ); await writeFile(user, `${user.home}/Desktop/cp-coll.txt`, 'existing'); - expect( - ( - await caught(() => - fs.copy(user.userId, { - source, - destinationParent: destination, - }), - ) - ).statusCode, - ).toBe(409); + const conflict = await caught(() => + fs.copy(user.userId, { + source, + destinationParent: destination, + }), + ); + expect(conflict.statusCode).toBe(409); + // v1 wire contract: the GUI's replace/skip prompts key on this + // code + entry_name; a generic 'conflict' makes them fail silently. + expect(conflict.legacyCode).toBe('item_with_same_name_exists'); + expect(conflict.fields).toMatchObject({ entry_name: 'cp-coll.txt' }); const deduped = await fs.copy(user.userId, { source,