Atomic chat-cache writes + stop tests clobbering prod cache
Three intertwined fixes for the torn-write that corrupted the prod
data/chat-cache.json mid-session:
- ChatCache.save / persistCooldown now write via temp file + rename.
rename(2) is atomic on the same filesystem, so a crashed or
concurrent writer can never leave NULs in the middle of the file.
A small serialising write queue stops two concurrent saves from
racing to the same temp name.
- ChatCache throws if constructed without a filePath under NODE_ENV=test.
The old default (./data/chat-cache.json) silently routed test
writes into the production cache — exactly how the corruption
happened. Future test that forgets to pass a tmp path fails loudly
instead of stomping data.
- Cooldown / delta-media / username-persist tests now pass an
explicit tmp ChatCache so the new guard is satisfied.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
1 parent
71548ec2ad
commit
b9df8459e3
4 files changed
+65
-9
No files matched your search
@@ -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<unknown> = Promise.resolve();
|
||||
private async atomicWrite(payload: ChatCacheFile): Promise<void> {
|
||||
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;
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user