From de0c9c6e2259369c3c79fff443af213be546fb8a Mon Sep 17 00:00:00 2001
From: Niklas Salarp <niklasgss@gmail.com>
Date: Fri, 28 Aug 2026 08:36:52 +0200
Subject: [PATCH] fix(git): stop leaking temp files when the cache write fails

writePersistentCache wrote a uniquely named .tmp file and renamed it over
the cache path inside a swallow-all catch. When the write or rename fails
(notably EPERM on Windows while a virus scanner or sync client holds a
handle), the temp file was never unlinked, leaking one file per cache
write - observed at ~400k orphaned files in ~/.cache/ccstatusline/git-cache.

Clean up the temp file on failure, mirroring config.ts:writeSettingsJson
minus the rethrow since this cache is best-effort. Because the same held
handle can make the cleanup unlink fail too, the temp name is now stable
(cachePath + '.tmp') instead of unique: the worst case becomes a single
orphan per repo that the next write truncates. A torn concurrent write
only produces malformed JSON, which readPersistentCache already treats
as a cache miss.

Deliberately no sweep for pre-existing orphans: a readdir over a directory
that large on the render path costs more than it saves. Deleting the
git-cache directory once by hand is enough - it regenerates itself.
---
 src/utils/__tests__/git.test.ts | 25 +++++++++++++++++++++++++
 src/utils/git.ts                | 19 +++++++++++++++----
 2 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/src/utils/__tests__/git.test.ts b/src/utils/__tests__/git.test.ts
index befdc9c5..4d5687bb 100644
--- a/src/utils/__tests__/git.test.ts
+++ b/src/utils/__tests__/git.test.ts
@@ -301,6 +301,31 @@ describe('git utils', () => {
             expect(runGit('symbolic-ref --short HEAD', context)).toBe('new-value');
             expect(mockExecFileSync.mock.calls).toHaveLength(2);
         });
+
+        it('leaves no temp file behind when the cache write cannot be renamed into place', () => {
+            vi.spyOn(Date, 'now').mockReturnValue(1000);
+            const home = useTempHome();
+            const { root } = createGitRepo();
+            const context: RenderContext = { data: { cwd: root }, gitCacheTtlSeconds: 5 };
+            mockExecFileSync.mockReturnValueOnce('feature/leak\n');
+
+            expect(runGit('symbolic-ref --short HEAD', context)).toBe('feature/leak');
+
+            // Make the cache path a directory so the final rename always fails,
+            // exercising the cleanup-on-error path in writePersistentCache.
+            const cachePath = getOnlyGitCachePath(home);
+            fs.rmSync(cachePath);
+            fs.mkdirSync(cachePath);
+
+            clearGitCache();
+            mockExecFileSync.mockReturnValueOnce('feature/leak\n');
+
+            expect(runGit('symbolic-ref --short HEAD', context)).toBe('feature/leak');
+            // The blocking directory is still in place and no temp file leaked.
+            expect(fs.statSync(cachePath).isDirectory()).toBe(true);
+            const cacheDir = path.dirname(cachePath);
+            expect(fs.readdirSync(cacheDir).filter(name => name.endsWith('.tmp'))).toEqual([]);
+        });
     });
 
     describe('isInsideGitWorkTree', () => {
diff --git a/src/utils/git.ts b/src/utils/git.ts
index e5714050..019440d1 100644
--- a/src/utils/git.ts
+++ b/src/utils/git.ts
@@ -209,14 +209,25 @@ function readPersistentCache(cachePath: string): PersistentGitCache | null {
 }
 
 function writePersistentCache(cachePath: string, cache: PersistentGitCache): void {
+    // Stable temp name: a held handle (e.g. a Windows virus scanner or sync
+    // client) can make the rename and the cleanup unlink both fail with
+    // EPERM, so a unique name would leak one file per write. Reusing one
+    // name bounds that to a single orphan per repo that the next write
+    // truncates. Costs: while the handle is held, writes for that repo fail
+    // (just extra cache misses), and a concurrent writer can tear the
+    // renamed-over cache file - readPersistentCache treats malformed JSON
+    // as a miss either way.
+    const tempPath = `${cachePath}.tmp`;
     try {
-        const cacheDir = path.dirname(cachePath);
-        fs.mkdirSync(cacheDir, { recursive: true });
-        const tempPath = `${cachePath}.${process.pid}.${Date.now()}.tmp`;
+        fs.mkdirSync(path.dirname(cachePath), { recursive: true });
         fs.writeFileSync(tempPath, JSON.stringify(cache), 'utf-8');
         fs.renameSync(tempPath, cachePath);
     } catch {
-        // Best-effort cache; statusline rendering should never fail because of it.
+        // Best-effort cache; statusline rendering should never fail because
+        // of it, so unlike config.ts:writeSettingsJson we do not rethrow.
+        try {
+            fs.unlinkSync(tempPath);
+        } catch { /* best-effort cleanup; ignore */ }
     }
 }