fix: prune stale download and tarball spill directories (#16620)

Clean up spill directories left after an interrupted download or archive extraction during store prune. Reuse the existing store-directory scan and removal helpers without adding install-time work.

Only remove matching directories when their own mtime and the mtimes of their immediate files are more than 24 hours old. Checking files preserves an old directory whose download is still writing. Stream file entries and check one directory and file at a time to bound pending filesystem work. Stop scanning a directory after its first recent file. Skip symlinks and tolerate ENOENT when another process finishes cleanup first.

Age is a conservative heuristic, not a process-liveness check: an install suspended for over 24 hours without filesystem activity can lose its spill directory.

Add regression coverage for stale, recent, empty, and unrelated entries, symlinks, missing stores, recent writes in old directories, and expected versus unexpected filesystem errors. Close SQLite fixture handles before removing test directories for Windows compatibility.

Closes pnpm/tasks#109.

pnpm11/docs/cli/store.md

@@ -45,6 +45,10 @@ again. This could occur when switching branches and installing older
 dependencies, in which case pnpm would need to re-download all removed packages,
 briefly slowing down the installation process.
 
+`pnpm store prune` also removes temporary download and extraction directories
+left by interrupted installs when the directories and their files have not been
+modified for more than 24 hours. More recent directories are kept.
+
 After pruning, pnpm displays the total size of removed files.
 
 When the [global virtual store] is enabled, `pnpm store prune` also performs mark-and-sweep garbage collection on the global virtual store's `links/` directory. Projects using the store are registered via symlinks in `{storeDir}/v11/projects/`, allowing pnpm to track active usage and safely remove unused packages from the global virtual store.

pnpm11/store/controller/src/storeController/prune.ts

@@ -26,6 +26,7 @@ export async function prune ({ cacheDir, storeDir, storeIndex }: PruneOptions, r
 
   // 2. Clean up metadata cache
   await removeCachedMetadata(cacheDir, storeDir)
+  await removeStaleSpillDirectories(storeDir)
 
   // 3. Prune the content-addressable store (CAS)
   const removedHashes = await pruneContentAddressableStore(storeDir, removeAlienFiles)
@@ -45,6 +46,35 @@ async function removeCachedMetadata (cacheDir: string, storeDir: string): Promis
   globalInfo('Removed all cached metadata files')
 }
 
+async function removeStaleSpillDirectories (storeDir: string): Promise<void> {
+  const cutoff = Date.now() - 24 * 60 * 60_000
+  const directories = await getSubdirsSafely(storeDir)
+  for (const directory of directories) {
+    if (!directory.startsWith('download-') && !directory.startsWith('tarball-')) continue
+    // eslint-disable-next-line no-await-in-loop -- bound filesystem work across abandoned spill directories
+    await removeSpillDirectoryIfStale(path.join(storeDir, directory), cutoff)
+  }
+}
+
+async function removeSpillDirectoryIfStale (directoryPath: string, cutoff: number): Promise<void> {
+  try {
+    const stats = await fs.lstat(directoryPath)
+    if (stats.isDirectory() && stats.mtimeMs < cutoff && await spillFilesAreStale(directoryPath, cutoff)) {
+      await rimrafIgnoringMissing(directoryPath)
+    }
+  } catch (err: unknown) {
+    if (!(isError(err) && 'code' in err && err.code === 'ENOENT')) throw err
+  }
+}
+
+async function spillFilesAreStale (directoryPath: string, cutoff: number): Promise<boolean> {
+  for await (const file of await fs.opendir(directoryPath)) {
+    const stats = await fs.lstat(path.join(directoryPath, file.name))
+    if (stats.mtimeMs >= cutoff) return false
+  }
+  return true
+}
+
 async function rimrafIgnoringMissing (dir: string): Promise<void> {
   try {
     await rimraf(dir)

pnpm11/store/controller/test/prune.ts

@@ -0,0 +1,134 @@
+import { promises as fs } from 'node:fs'
+import path from 'node:path'
+
+import { afterEach, expect, jest, test } from '@jest/globals'
+import { StoreIndex } from '@pnpm/store.index'
+import { temporaryDirectory } from 'tempy'
+
+import { prune } from '../src/storeController/prune.js'
+
+const temporaryDirectories: string[] = []
+const storeIndexes: StoreIndex[] = []
+
+afterEach(async () => {
+  jest.restoreAllMocks()
+  for (const storeIndex of storeIndexes.splice(0)) storeIndex.close()
+  await Promise.all(temporaryDirectories.splice(0).map(directory => fs.rm(directory, { recursive: true, force: true })))
+})
+
+test('prune removes stale spill directories and keeps recent directories and unrelated entries', async () => {
+  const options = createPruneOptions()
+  const oldDate = new Date(Date.now() - 48 * 60 * 60_000)
+  await Promise.all(['download-', 'tarball-'].map(async (prefix) => {
+    const staleDirectory = await fs.mkdtemp(path.join(options.storeDir, prefix))
+    await fs.writeFile(path.join(staleDirectory, 'archive'), 'leftover data')
+    await fs.utimes(path.join(staleDirectory, 'archive'), oldDate, oldDate)
+    await fs.utimes(staleDirectory, oldDate, oldDate)
+    const recentDirectory = await fs.mkdtemp(path.join(options.storeDir, prefix))
+    await fs.writeFile(path.join(recentDirectory, 'archive'), 'active data')
+  }))
+  const unrelatedDirectory = path.join(options.storeDir, 'unrelated')
+  await fs.mkdir(unrelatedDirectory)
+  await fs.utimes(unrelatedDirectory, oldDate, oldDate)
+  const unrelatedFile = path.join(options.storeDir, 'download-file')
+  await fs.writeFile(unrelatedFile, 'keep')
+  await fs.utimes(unrelatedFile, oldDate, oldDate)
+
+  await prune(options)
+
+  const entries = await fs.readdir(options.storeDir)
+  expect(entries.filter(entry => entry.startsWith('download-') && entry !== 'download-file')).toHaveLength(1)
+  expect(entries.filter(entry => entry.startsWith('tarball-'))).toHaveLength(1)
+  expect(await fs.readFile(unrelatedFile, 'utf8')).toBe('keep')
+  expect((await fs.stat(unrelatedDirectory)).isDirectory()).toBe(true)
+  await Promise.all(entries.filter(entry => entry.startsWith('tarball-') || (entry.startsWith('download-') && entry !== 'download-file')).map(async (entry) => {
+    expect(await fs.readFile(path.join(options.storeDir, entry, 'archive'), 'utf8')).toBe('active data')
+  }))
+})
+
+test('prune keeps old spill directories whose files were written recently', async () => {
+  const options = createPruneOptions()
+  const oldDate = new Date(Date.now() - 48 * 60 * 60_000)
+  const directories = await Promise.all(['download-', 'tarball-'].map(async (prefix) => {
+    const directory = await fs.mkdtemp(path.join(options.storeDir, prefix))
+    await fs.writeFile(path.join(directory, 'archive'), 'active download')
+    await fs.utimes(directory, oldDate, oldDate)
+    return directory
+  }))
+  await prune(options)
+  await Promise.all(directories.map(async (directory) => {
+    expect(await fs.readFile(path.join(directory, 'archive'), 'utf8')).toBe('active download')
+  }))
+})
+
+test('prune stops scanning a spill directory after finding a recent file', async () => {
+  const options = createPruneOptions()
+  const directory = await fs.mkdtemp(path.join(options.storeDir, 'download-'))
+  await Promise.all(['first', 'second'].map(file => fs.writeFile(path.join(directory, file), 'keep')))
+  const stat = jest.spyOn(fs, 'lstat')
+  const oldDate = new Date(Date.now() - 48 * 60 * 60_000)
+  await fs.utimes(directory, oldDate, oldDate)
+
+  await prune(options)
+
+  expect(stat).toHaveBeenCalledTimes(2)
+  expect(stat).toHaveBeenNthCalledWith(1, directory)
+  expect(await fs.readdir(directory)).toHaveLength(2)
+})
+
+test('prune removes empty stale spill directories', async () => {
+  const options = createPruneOptions()
+  const directory = await fs.mkdtemp(path.join(options.storeDir, 'tarball-'))
+  const oldDate = new Date(Date.now() - 48 * 60 * 60_000)
+  await fs.utimes(directory, oldDate, oldDate)
+  await prune(options)
+  await expect(fs.lstat(directory)).rejects.toMatchObject({ code: 'ENOENT' })
+})
+
+test('prune tolerates a missing store directory', async () => {
+  const options = createPruneOptions()
+  options.storeDir = path.join(options.storeDir, 'missing')
+  await expect(prune(options)).resolves.toBeUndefined()
+})
+
+test('prune does not follow spill directory symlinks', async () => {
+  const options = createPruneOptions()
+  const target = path.join(options.cacheDir, 'target')
+  await fs.mkdir(target, { recursive: true })
+  await fs.writeFile(path.join(target, 'archive'), 'keep')
+  const oldDate = new Date(Date.now() - 48 * 60 * 60_000)
+  await fs.utimes(target, oldDate, oldDate)
+  await Promise.all(['download-', 'tarball-'].map(async (prefix) => {
+    await fs.symlink(target, path.join(options.storeDir, `${prefix}link`), 'junction')
+  }))
+
+  await prune(options)
+
+  expect(await fs.readFile(path.join(target, 'archive'), 'utf8')).toBe('keep')
+  expect((await fs.lstat(path.join(options.storeDir, 'download-link'))).isSymbolicLink()).toBe(true)
+  expect((await fs.lstat(path.join(options.storeDir, 'tarball-link'))).isSymbolicLink()).toBe(true)
+})
+
+test('prune tolerates a spill directory removed by its owner during cleanup', async () => {
+  const options = createPruneOptions()
+  await fs.mkdir(path.join(options.storeDir, 'download-disappeared'))
+  jest.spyOn(fs, 'lstat').mockRejectedValueOnce(Object.assign(new Error('already removed'), { code: 'ENOENT' }))
+  await expect(prune(options)).resolves.toBeUndefined()
+})
+
+test('prune propagates unexpected spill directory errors', async () => {
+  const options = createPruneOptions()
+  await fs.mkdir(path.join(options.storeDir, 'tarball-denied'))
+  const error = Object.assign(new Error('access denied'), { code: 'EACCES' })
+  jest.spyOn(fs, 'lstat').mockRejectedValueOnce(error)
+  await expect(prune(options)).rejects.toBe(error)
+})
+
+function createPruneOptions () {
+  const directory = temporaryDirectory()
+  temporaryDirectories.push(directory)
+  const storeDir = path.join(directory, 'store')
+  const storeIndex = new StoreIndex(storeDir)
+  storeIndexes.push(storeIndex)
+  return { storeDir, cacheDir: path.join(directory, 'cache'), storeIndex }
+}