From 2ec739fd3a18715dd0bfc09bc01515e4dcf9c6de Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Fri, 6 Nov 2026 02:27:39 +0100 Subject: [PATCH] test: pin the rename re-checks and the refused-registry answers (release review) The rename's checks inside mutateLinkedCases and the refused-registry branches of rename and clone had no test: removing them left every case route test green. A hook right after the folder move lands a link, an unlink, a re-pointed entry or a torn file between the route's first check and its registry write. Each new test fails when its check is removed. Co-Authored-By: Claude Opus 5.5 --- test/routes/case-clone-custom-folder.test.ts | 16 +++- test/routes/case-rename-routes.test.ts | 80 ++++++++++++++++++++ 2 files changed, 95 insertions(+), 1 deletion(-) diff --git a/test/routes/case-clone-custom-folder.test.ts b/test/routes/case-clone-custom-folder.test.ts index 39e1ac0d..3645414b 100644 --- a/test/routes/case-clone-custom-folder.test.ts +++ b/test/routes/case-clone-custom-folder.test.ts @@ -20,7 +20,7 @@ import { writeFileSync, } from 'node:fs'; import { homedir, tmpdir } from 'node:os'; -import { join } from 'node:path'; +import { dirname, join } from 'node:path'; import { createRouteTestHarness } from './_route-test-utils.js'; import { registerCaseRoutes } from '../../src/web/routes/case-routes.js'; import { dataPath } from '../../src/config/instance.js'; @@ -178,6 +178,20 @@ describe.skipIf(!isGitAvailable())('POST /api/cases/clone with a custom folder', expect(Object.keys(linked())).toEqual(['twin']); }); + it('a registry that is not a JSON object is a 409 that keeps the folder, names it and leaves the file alone', async () => { + const { app } = await createRouteTestHarness(registerCaseRoutes); + const target = join(workDir, 'refused'); + mkdirSync(dirname(LINKED()), { recursive: true }); + writeFileSync(LINKED(), '[]'); + const res = await clone(app, { name: 'refused', repository: origin, path: target }); + expect(res.statusCode).toBe(409); + expect(res.json().errorCode).toBe('CONFLICT'); + expect(res.json().error).toContain(target); + expect(res.json().error).toContain('Use Link Existing'); + expect(existsSync(join(target, 'README.md'))).toBe(true); + expect(readFileSync(LINKED(), 'utf8')).toBe('[]'); + }); + it('a direct clone whose git-slot wait times out leaves what was written into the folder meanwhile', async () => { const target = join(workDir, 'queued'); mkdirSync(target); diff --git a/test/routes/case-rename-routes.test.ts b/test/routes/case-rename-routes.test.ts index 0d85d9e0..b8605955 100644 --- a/test/routes/case-rename-routes.test.ts +++ b/test/routes/case-rename-routes.test.ts @@ -25,6 +25,7 @@ import { SETTINGS_PATH, resolveCasesDir } from '../../src/web/route-helpers.js'; import { caseSessionSettled, claudeProjectDirName } from '../../src/case-rename.js'; import { Session } from '../../src/session.js'; import { safeRmHomeTree } from '../mocks/index.js'; +import { mutateLinkedCases } from '../../src/web/linked-cases-writer.js'; import { mutateSettingsFile } from '../../src/web/settings-writer.js'; const LINKED = () => dataPath('linked-cases.json'); @@ -50,6 +51,17 @@ const failRegistryWrite = (before?: () => Promise, alsoFail?: (from: st return realRename(from, to); }); }; +/** + * Runs `during` right after `folder` moved: past every check the route makes before it writes the + * registry, and before that write starts. Every other rename goes through untouched. + */ +const afterFolderMove = (folder: string, during: () => Promise) => { + const realRename = fs.rename; + return vi.spyOn(fs, 'rename').mockImplementation(async (from, to) => { + await realRename(from, to); + if (from === folder) await during(); + }); +}; const settings = (): Record => JSON.parse(readFileSync(SETTINGS_PATH, 'utf8')); const rename = (app: Awaited>['app'], name: string, payload: object) => app.inject({ method: 'POST', url: `/api/cases/${name}/rename`, payload }); @@ -325,6 +337,74 @@ describe('POST /api/cases/:name/rename (a linked case)', () => { expect(settings()).toEqual({ caseOrder: ['a', 'proj', 'b'], lastUsedCase: 'proj', agentSkillEnabled: true }); }); + describe('when the registry changes after the first check', () => { + let real = ''; + type App = Awaited>['app']; + const renameRacing = async (app: App, during: () => Promise) => { + const spy = afterFolderMove(real, during); + try { + return await rename(app, 'proj', { newName: 'renamed', renameFolder: true }); + } finally { + spy.mockRestore(); + } + }; + const expectFolderAndSettingsBack = () => { + expect(existsSync(real)).toBe(true); + expect(existsSync(join(work, 'renamed'))).toBe(false); + expect(settings()).toEqual({ caseOrder: ['a', 'proj', 'b'], lastUsedCase: 'proj' }); + }; + beforeEach(() => { + real = join(work, 'my-project'); + mkdirSync(real); + writeFileSync(LINKED(), JSON.stringify({ proj: real })); + writeFileSync(SETTINGS_PATH, JSON.stringify({ caseOrder: ['a', 'proj', 'b'], lastUsedCase: 'proj' })); + }); + + it('a name linked meanwhile is 409 EXISTS: the link stands and the rename is undone', async () => { + const { app } = await createRouteTestHarness(registerCaseRoutes); + const other = join(work, 'other'); + mkdirSync(other); + const res = await renameRacing(app, () => + app.inject({ method: 'POST', url: '/api/cases/link', payload: { name: 'renamed', path: other } }) + ); + expect(res.statusCode).toBe(409); + expect(res.json()).toMatchObject({ errorCode: 'ALREADY_EXISTS', renameCode: 'EXISTS', stoppedSessions: 0 }); + expect(linked()).toEqual({ proj: real, renamed: other }); + expectFolderAndSettingsBack(); + }); + + it('an old entry unlinked meanwhile is 409 BUSY: the rename does not bring it back', async () => { + const { app } = await createRouteTestHarness(registerCaseRoutes); + const res = await renameRacing(app, () => app.inject({ method: 'DELETE', url: '/api/cases/proj' })); + expect(res.statusCode).toBe(409); + expect(res.json()).toMatchObject({ errorCode: 'CONFLICT', renameCode: 'BUSY', stoppedSessions: 0 }); + expect(linked()).toEqual({}); + expectFolderAndSettingsBack(); + }); + + it('an old entry pointed at another folder meanwhile is 409 BUSY: that entry is kept', async () => { + const { app } = await createRouteTestHarness(registerCaseRoutes); + const other = join(work, 'other'); + const res = await renameRacing(app, () => mutateLinkedCases((current) => ({ ...current, proj: other }))); + expect(res.statusCode).toBe(409); + expect(res.json()).toMatchObject({ errorCode: 'CONFLICT', renameCode: 'BUSY' }); + expect(linked()).toEqual({ proj: other }); + expectFolderAndSettingsBack(); + }); + + it('a registry that stopped parsing is refused 409 CONFLICT and left byte for byte', async () => { + const { app } = await createRouteTestHarness(registerCaseRoutes); + const torn = `{ "proj": "${real}" }tail"`; + const res = await renameRacing(app, async () => writeFileSync(LINKED(), torn)); + expect(res.statusCode).toBe(409); + expect(res.json()).toMatchObject({ errorCode: 'CONFLICT', stoppedSessions: 0 }); + expect(res.json().error).toContain('is not a valid JSON object'); + expect(res.json()).not.toHaveProperty('renameCode'); + expect(readFileSync(LINKED(), 'utf8')).toBe(torn); + expectFolderAndSettingsBack(); + }); + }); + it('refuses to rename while settings.json is not a JSON object, leaving the folder and the file alone', async () => { const { app } = await createRouteTestHarness(registerCaseRoutes); const oldDir = localCase('alpha');