From a8067b000bba92fd8d5d87f0c0fa813053037e2e Mon Sep 17 00:00:00 2001 From: "Liam Verdejo (ES)" Date: Fri, 4 Sep 2026 11:16:00 +0200 Subject: [PATCH] fix: atomic entity file writes to avoid corruption on process kill writeEntityRows wrote directly to .json. A process killed mid-write (e.g. Cloud Run SIGTERM/SIGKILL) could leave it truncated, and if the pending commit still reached the remote before the kill, the corrupted file would be permanently baked into git history and break every future clone. Now writes go to a sibling temp file and rename() over the target, which is atomic on the same filesystem. A .gitignore for *.tmp is created on init so a leftover temp file (killed between write and rename) never gets committed. --- src/infrastructure/file-manager.test.ts | 76 +++++++++++++++++++++++++ src/infrastructure/file-manager.ts | 27 ++++++++- src/infrastructure/git-repository.ts | 9 +++ 3 files changed, 109 insertions(+), 3 deletions(-) create mode 100644 src/infrastructure/file-manager.test.ts diff --git a/src/infrastructure/file-manager.test.ts b/src/infrastructure/file-manager.test.ts new file mode 100644 index 0000000..79f4a7c --- /dev/null +++ b/src/infrastructure/file-manager.test.ts @@ -0,0 +1,76 @@ +import { mkdtemp, readdir, readFile, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { FileManager } from './file-manager.ts'; + +describe('FileManager', () => { + let basePath: string; + let fileManager: FileManager; + + beforeEach(async () => { + basePath = await mkdtemp(path.join(tmpdir(), 'gitdb-file-manager-test-')); + fileManager = new FileManager(basePath); + }); + + afterEach(async () => { + await rm(basePath, { recursive: true, force: true }); + }); + + it('crea el fichero de la entidad como array vacío la primera vez que se lee', async () => { + const rows = await fileManager.readEntityRows('users'); + + expect(rows).toEqual([]); + }); + + it('persiste y relee filas de la entidad', async () => { + await fileManager.writeEntityRows('users', [{ id: '1', name: 'kettu' }]); + + const rows = await fileManager.readEntityRows<{ id: string; name: string }>('users'); + + expect(rows).toEqual([{ id: '1', name: 'kettu' }]); + }); + + it('no deja el fichero de la entidad truncado si la escritura falla a mitad', async () => { + await fileManager.writeEntityRows('users', [{ id: '1', name: 'kettu' }]); + + // simulates a process killed mid-`writeFile`: previous complete content must survive + const before = await readFile(path.join(basePath, 'users.json'), 'utf8'); + expect(() => JSON.parse(before)).not.toThrow(); + + await fileManager.writeEntityRows('users', [ + { id: '1', name: 'kettu' }, + { id: '2', name: 'gitops' }, + ]); + + const after = await readFile(path.join(basePath, 'users.json'), 'utf8'); + expect(JSON.parse(after)).toEqual([ + { id: '1', name: 'kettu' }, + { id: '2', name: 'gitops' }, + ]); + }); + + it('limpia el fichero temporal cuando la escritura falla', async () => { + // pre-create a directory where the temp file would go, so writeFile rejects (EISDIR) + // and the atomic-write path has to unlink a temp path that never got created + const rows = [{ id: '1', name: 'kettu' }]; + + await expect(fileManager.writeEntityRows('missing-dir/users', rows)).rejects.toThrow(); + + const entries = await readdir(basePath).catch(() => []); + const strayTempFiles = entries.filter((entry) => entry.endsWith('.tmp')); + expect(strayTempFiles).toEqual([]); + }); + + it('deja el fichero anterior intacto si el rename nunca llega a producirse', async () => { + await fileManager.writeEntityRows('users', [{ id: '1', name: 'kettu' }]); + + // a leftover temp file from a process killed between writeFile and rename must not + // be picked up as the entity's content on the next read + await writeFile(path.join(basePath, `users.json.stale-leftover.tmp`), '[corrupt', 'utf8'); + + const rows = await fileManager.readEntityRows<{ id: string; name: string }>('users'); + + expect(rows).toEqual([{ id: '1', name: 'kettu' }]); + }); +}); diff --git a/src/infrastructure/file-manager.ts b/src/infrastructure/file-manager.ts index 0dece6a..505edf8 100644 --- a/src/infrastructure/file-manager.ts +++ b/src/infrastructure/file-manager.ts @@ -1,5 +1,6 @@ +import { randomUUID } from 'node:crypto'; import { existsSync } from 'node:fs'; -import { readFile, writeFile } from 'node:fs/promises'; +import { readFile, rename, unlink, writeFile } from 'node:fs/promises'; import path from 'node:path'; export class FileManager { @@ -21,7 +22,7 @@ export class FileManager { async writeEntityRows(entityName: string, rows: T[]): Promise { const filePath = this.getEntityFilePath(entityName); - await writeFile(filePath, `${JSON.stringify(rows, null, 2)}\n`, 'utf8'); + await this.writeFileAtomic(filePath, `${JSON.stringify(rows, null, 2)}\n`); } private async ensureEntityFile(entityName: string): Promise { @@ -31,7 +32,27 @@ export class FileManager { return; } - await writeFile(filePath, '[]\n', 'utf8'); + await this.writeFileAtomic(filePath, '[]\n'); + } + + /** + * Writes to a sibling temp file and renames it over the target. `rename` is atomic on the + * same filesystem, so a process killed mid-write (e.g. Cloud Run SIGKILL) leaves either the + * previous complete file or a stray `*.tmp` — never a truncated entity file that would + * corrupt the next commit pushed to the remote. + */ + private async writeFileAtomic(filePath: string, content: string): Promise { + const tempPath = `${filePath}.${randomUUID()}.tmp`; + + try { + await writeFile(tempPath, content, 'utf8'); + await rename(tempPath, filePath); + } catch (error) { + await unlink(tempPath).catch(() => { + // best-effort cleanup, the write already failed + }); + throw error; + } } private getEntityFilePath(entityName: string): string { diff --git a/src/infrastructure/git-repository.ts b/src/infrastructure/git-repository.ts index 3d9639b..0c6eee1 100644 --- a/src/infrastructure/git-repository.ts +++ b/src/infrastructure/git-repository.ts @@ -27,6 +27,7 @@ export class GitRepository { private readonly gitUserName: string; private readonly gitUserEmail: string; private readonly manifestPath: string; + private readonly gitignorePath: string; private readonly authToken: string; private readonly authUsername: string; private readonly logger: any; @@ -43,6 +44,7 @@ export class GitRepository { this.repositoryUrl = options.repositoryUrl; this.repoPath = options.dataPath; this.manifestPath = path.join(this.repoPath, 'gitdb.manifest.json'); + this.gitignorePath = path.join(this.repoPath, '.gitignore'); this.autoCommitIntervalMs = options.autoCommitIntervalMs; this.immediateCommitDelayMs = options.immediateCommitDelayMs; this.syncPollMs = Math.max(0, options.syncPollSeconds) * 1000; @@ -92,6 +94,13 @@ export class GitRepository { this.logger.info('[gitdb] manifest written'); } + if (!existsSync(this.gitignorePath)) { + // FileManager writes entity files atomically (temp file + rename); if a process is + // killed between those two steps it leaves a stray `*.tmp` sibling. Ignoring it keeps + // `git add -A` from ever committing a half-written file. + await writeFile(this.gitignorePath, '*.tmp\n', 'utf8'); + } + this.intervalTimer = setInterval(() => { void this.commitNow('auto-interval').catch(() => { // Background auto-commits must not create unhandled rejections.