fix: apply the review items for #719 at merge

- src/web/settings-writer.ts:110: export assertSettingsWritable(), the writer's own read-for-write rule run on its own
- src/web/routes/case-routes.ts:1860: an admin's rename checks settings.json before it ends any session or moves the folder, and answers 409 CONFLICT with the writer's message (placed before the session-starting check, so nothing is awaited between it and the lock)
- test/routes/case-rename-routes.test.ts:314: the torn-file pin now expects 409 CONFLICT (no stoppedSessions, like the other pre-checks)
- test/routes/case-rename-routes.test.ts:331: new test, a live session with stopSessions and a torn file: 409 CONFLICT, cleanupSession not called
- docs/api-reference.md: the Settings paragraph says an admin's rename answers this refusal as 409 CONFLICT before ending sessions
- src/hooks-config.ts:1161, src/prompt-cache-keepwarm.ts:341: comments describe the atomic writer, not an in-place rewrite

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Codeman maintainer committed 2026-10-27 01:24:15 +01:00
1 parent 8fa5b340c1
commit 8574e3c880
6 files changed
+57 -10

No files matched your search

+17 -1
View File
@@ -78,7 +78,12 @@ import {
parseBody,
readJsonConfig,
} from '../route-helpers.js';
import { mutateSettingsFile, settingsWriteErrorResponse } from '../settings-writer.js';
import {
assertSettingsWritable,
mutateSettingsFile,
SettingsWriteRefusedError,
settingsWriteErrorResponse,
} from '../settings-writer.js';
import { isMultiUserMode } from '../../config/multiuser.js';
import type { AuthUser } from '../../types.js';
import { SseEvent } from '../sse-events.js';
@@ -1850,6 +1855,17 @@ export function registerCaseRoutes(
if (open.some((s) => !canAccessOwned(user, s.owner))) {
return fail(403, ApiErrorCode.FORBIDDEN, "Another user's session has this case open");
}
// The admin's rename writes settings.json only after the sessions are ended and the folder moved, and
// the writer refuses a file that is not a JSON object: check it by the writer's own rule now, so no
// session is ended for a rename that cannot complete. Not between the starting check and the lock below.
if (isAdmin(req)) {
try {
await assertSettingsWritable();
} catch (err) {
if (err instanceof SettingsWriteRefusedError) return fail(409, ApiErrorCode.CONFLICT, err.message);
throw err;
}
}
// A session being created in the folder is not in ctx.sessions yet, so it could be neither listed nor
// stopped: it would start in the old path after the move. Checked in the same tick that takes the lock
// below (nothing is awaited in between), and the Session constructor refuses the folder from then on.
+11
View File
@@ -100,6 +100,17 @@ export function mutateSettingsFile(
return run;
}
/**
* Rejects with `SettingsWriteRefusedError` when `mutateSettingsFile()` would refuse a write right
* now: the writer's own read-for-write rule, run on its own. For a route that does something it
* cannot take back (case rename ends sessions) before its settings write. A check only, outside
* the chain (writes are atomic, so it never reads a half-written file); the file can still change
* before the write.
*/
export async function assertSettingsWritable(): Promise<void> {
await readSettingsForWrite(SETTINGS_PATH);
}
/** A refused write answers 409 CONFLICT with the fix; any other failure stays OPERATION_FAILED. */
export function settingsWriteErrorResponse(err: unknown): ApiResponse<never> {
if (err instanceof SettingsWriteRefusedError) return createErrorResponse(ApiErrorCode.CONFLICT, err.message);