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 */ } } }