diff --git a/src/services/chat/cache.ts b/src/services/chat/cache.ts index d1902f1..bf735d7 100644 --- a/src/services/chat/cache.ts +++ b/src/services/chat/cache.ts @@ -25,6 +25,15 @@ export class ChatCache { private cooldown: InvitationCooldown | null = null; constructor(private logger: Logger, filePath?: string) { + if (!filePath && process.env.NODE_ENV === 'test') { + // Without this guard a test that constructs a ChatCache (or a + // ChatService that builds one) would silently write to the prod + // ./data/chat-cache.json — concurrent test runs have already torn + // that file once. Force every test to opt into a tmp path. + throw new Error( + 'ChatCache: refusing to fall back to ./data/chat-cache.json under NODE_ENV=test — pass an explicit filePath', + ); + } this.filePath = filePath ?? path.join(process.cwd(), 'data', 'chat-cache.json'); } @@ -127,8 +136,7 @@ export class ChatCache { cooldown: this.cooldown, }; try { - await fs.mkdir(path.dirname(this.filePath), { recursive: true }); - await fs.writeFile(this.filePath, JSON.stringify(payload, null, 2), 'utf-8'); + await this.atomicWrite(payload); } catch (error) { this.logger.warn('Failed to save chat cache cooldown:', error); } @@ -145,10 +153,30 @@ export class ChatCache { cooldown: this.cooldown, }; try { - await fs.mkdir(path.dirname(this.filePath), { recursive: true }); - await fs.writeFile(this.filePath, JSON.stringify(payload, null, 2), 'utf-8'); + await this.atomicWrite(payload); } catch (error) { this.logger.warn('Failed to save chat cache:', error); } } + + /** + * Write the cache file via a temp+rename so a crashed/interrupted/concurrent + * writer can never leave the on-disk file half-written. rename(2) is atomic + * on the same filesystem, so any reader sees either the prior complete file + * or the new complete file — never NULs or truncated JSON. + * + * Serialises against itself so two concurrent saves don't fight over the + * same temp path or trample each other's rename. + */ + private writeQueue: Promise = Promise.resolve(); + private async atomicWrite(payload: ChatCacheFile): Promise { + const task = this.writeQueue.then(async () => { + await fs.mkdir(path.dirname(this.filePath), { recursive: true }); + const tmp = `${this.filePath}.${process.pid}.${Date.now()}.${Math.random().toString(36).slice(2, 8)}.tmp`; + await fs.writeFile(tmp, JSON.stringify(payload, null, 2), 'utf-8'); + await fs.rename(tmp, this.filePath); + }); + this.writeQueue = task.catch(() => undefined); + return task; + } } diff --git a/tests/chat-service-cooldown.test.ts b/tests/chat-service-cooldown.test.ts index 3b3b84e..26d6c2b 100644 --- a/tests/chat-service-cooldown.test.ts +++ b/tests/chat-service-cooldown.test.ts @@ -23,7 +23,13 @@ function buildService() { new Response('{}', { status: 200, headers: { 'content-type': 'application/json' } }), )) as typeof fetch; const matrix = new MatrixClient(creds, logger, { minRequestIntervalMs: 0, fetchImpl }); - const svc = new ChatService(matrix, logger); + // Point the cache at a tmp path; without this, the default falls back to + // ./data/chat-cache.json and the test would clobber the production cache. + const cache = new ChatCache( + logger, + path.join(os.tmpdir(), `chat-cache-${Date.now()}-${Math.random()}.json`), + ); + const svc = new ChatService(matrix, logger, undefined, undefined, undefined, undefined, cache); return { svc, logger }; } @@ -168,7 +174,11 @@ describe('ChatService invitation cooldown', () => { matrix.sendTextMessage = async () => { throw new MatrixRateLimitError('429', 90_000, 'M_LIMIT_EXCEEDED', null); }; - const svc = new ChatService(matrix, logger); + const cache = new ChatCache( + logger, + path.join(os.tmpdir(), `chat-cache-${Date.now()}-${Math.random()}.json`), + ); + const svc = new ChatService(matrix, logger, undefined, undefined, undefined, undefined, cache); const result = await svc.sendChatMessage('!room:example.test', 'hi'); expect(result).toBe(false); diff --git a/tests/chat-service-delta-media.test.ts b/tests/chat-service-delta-media.test.ts index 255890b..00d2506 100644 --- a/tests/chat-service-delta-media.test.ts +++ b/tests/chat-service-delta-media.test.ts @@ -172,7 +172,11 @@ describe('ChatService delta sync — media attachment', () => { }, })) as typeof matrix.sync; - const svc = new ChatService(matrix, logger); + const cache = new ChatCache( + logger, + path.join(os.tmpdir(), `chat-cache-${Date.now()}-${Math.random()}.json`), + ); + const svc = new ChatService(matrix, logger, undefined, undefined, undefined, undefined, cache); await expect(svc.getActiveChats()).resolves.toBeDefined(); }); }); diff --git a/tests/chat-service-username-persist.test.ts b/tests/chat-service-username-persist.test.ts index 2bca8b8..a504b5a 100644 --- a/tests/chat-service-username-persist.test.ts +++ b/tests/chat-service-username-persist.test.ts @@ -26,6 +26,13 @@ function buildMatrix() { } describe('ChatService username persistence', () => { + function tmpCache(logger: Parameters[0]) { + return new ChatCache( + logger, + path.join(os.tmpdir(), `chat-cache-${Date.now()}-${Math.random()}.json`), + ); + } + it('seeds the in-memory cache from RedditUsersRepository on construction', () => { const db = new Database(':memory:'); runMigrations(db); @@ -36,7 +43,9 @@ describe('ChatService username persistence', () => { const svc = new ChatService( buildMatrix(), logger, - undefined, undefined, undefined, undefined, undefined, undefined, + undefined, undefined, undefined, undefined, + tmpCache(logger), + undefined, repo, ); @@ -47,7 +56,12 @@ describe('ChatService username persistence', () => { it('falls back to t2 form when no mapping is known', () => { const logger = createMockLogger(); - const svc = new ChatService(buildMatrix(), logger); + const svc = new ChatService( + buildMatrix(), + logger, + undefined, undefined, undefined, undefined, + tmpCache(logger), + ); expect(svc.resolveSenderDisplay('@t2_unknown:reddit.com')).toBe('t2_unknown'); });