refactor(retention): narrow RetentionSetting type and optimize deletion flow

- Replace generic 'number' with explicit RetentionDays union type to prevent accidental misconfiguration
- Skip aggressive fs cleanup when deleteTaskById callback successfully removes the directory
- This avoids redundant filesystem work and log noise on stubborn paths
This commit is contained in:
Hannes Rudolph 2025-11-26 17:07:57 -07:00
parent 8b30590665
commit dbf47c259e

View file

@ -5,12 +5,17 @@ import type { Dirent } from "fs"
import { getStorageBasePath } from "./storage" import { getStorageBasePath } from "./storage"
import { GlobalFileNames } from "../shared/globalFileNames" import { GlobalFileNames } from "../shared/globalFileNames"
/**
* Allowed retention day values (as numbers).
*/
export type RetentionDays = 90 | 60 | 30 | 7 | 3
/** /**
* Supported values for the retention setting. * Supported values for the retention setting.
* - "never" or 0 disables purging * - "never" or 0 disables purging
* - "90" | "60" | "30" | "7" | "3" (string) or 90 | 60 | 30 | 7 | 3 (number) specify days * - "90" | "60" | "30" | "7" | "3" (string) or 90 | 60 | 30 | 7 | 3 (number) specify days
*/ */
export type RetentionSetting = "never" | "90" | "60" | "30" | "7" | "3" | 90 | 60 | 30 | 7 | 3 | 0 | "0" | number export type RetentionSetting = "never" | "0" | `${RetentionDays}` | RetentionDays | 0
export type PurgeResult = { export type PurgeResult = {
purgedCount: number purgedCount: number
@ -213,20 +218,26 @@ export async function purgeOldTasks(
// Attempt deletion using provider callback (for full cleanup) or direct rm // Attempt deletion using provider callback (for full cleanup) or direct rm
let deletionError: unknown | null = null let deletionError: unknown | null = null
let deleted = false
try { try {
if (deleteTaskById) { if (deleteTaskById) {
logv(`[Retention] Deleting task ${d.name} via provider @ ${taskDir} (${reason})`) logv(`[Retention] Deleting task ${d.name} via provider @ ${taskDir} (${reason})`)
await deleteTaskById(d.name, taskDir) await deleteTaskById(d.name, taskDir)
// Provider callback handles full cleanup; check if directory is gone
deleted = !(await pathExists(taskDir))
} else { } else {
logv(`[Retention] Deleting task ${d.name} via fs.rm @ ${taskDir} (${reason})`) logv(`[Retention] Deleting task ${d.name} via fs.rm @ ${taskDir} (${reason})`)
await fs.rm(taskDir, { recursive: true, force: true }) await fs.rm(taskDir, { recursive: true, force: true })
deleted = !(await pathExists(taskDir))
} }
} catch (e) { } catch (e) {
deletionError = e deletionError = e
} }
// Verify deletion; if still exists, attempt aggressive cleanup with retries // If directory still exists after initial attempt, try aggressive cleanup with retries
let deleted = await removeDirAggressive(taskDir) if (!deleted) {
deleted = await removeDirAggressive(taskDir)
}
if (!deleted) { if (!deleted) {
// Did not actually remove; report the most relevant error // Did not actually remove; report the most relevant error