From 22de30f998216c0c480b8858f9185d9829d90bae Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 4 Aug 2026 19:57:48 -0700 Subject: [PATCH] fix(media): decouple playback cache retention (#119417) --- docs/nodes/media-playback.md | 5 +- src/gateway/server-maintenance.test.ts | 86 +++++++++++++++++++++++--- src/gateway/server-maintenance.ts | 40 ++++++------ src/media/store.playback-cache.test.ts | 25 +++++--- src/media/store.ts | 8 +-- 5 files changed, 122 insertions(+), 42 deletions(-) diff --git a/docs/nodes/media-playback.md b/docs/nodes/media-playback.md index 73dca0162bee..6f10f76ab0fa 100644 --- a/docs/nodes/media-playback.md +++ b/docs/nodes/media-playback.md @@ -59,8 +59,9 @@ Playback conversion is lazy: unplayable-media fallback and keep the download action available. Transcoding accepts sources up to 20 minutes and never raises the normal audio -or video byte cap. Cached playback renditions are pruned by normal media-store -maintenance. +or video byte cap. Cached playback renditions use a fixed seven-day retention +that Gateway maintenance enforces at startup and hourly, independently of +`attachments.ttlHours`. ## Managed attachments and access diff --git a/src/gateway/server-maintenance.test.ts b/src/gateway/server-maintenance.test.ts index 1898ca2aac4a..fa4fcc31f9d6 100644 --- a/src/gateway/server-maintenance.test.ts +++ b/src/gateway/server-maintenance.test.ts @@ -11,12 +11,14 @@ import { pendingChatSendDedupeKey } from "./server-shared.js"; import { createGatewayMaintenanceStateForTest } from "./test-helpers.maintenance-state.js"; const cleanOldMediaMock = vi.fn(async () => {}); +const prunePlaybackTranscodeCacheMock = vi.fn(async () => {}); vi.mock("../media/store.js", async () => { const actual = await vi.importActual("../media/store.js"); return { ...actual, cleanOldMedia: cleanOldMediaMock, + prunePlaybackTranscodeCache: prunePlaybackTranscodeCacheMock, }; }); @@ -137,18 +139,23 @@ describe("startGatewayMaintenanceTimers", () => { vi.useRealTimers(); vi.restoreAllMocks(); vi.clearAllMocks(); + cleanOldMediaMock.mockReset().mockResolvedValue(undefined); + prunePlaybackTranscodeCacheMock.mockReset().mockResolvedValue(undefined); }); - it("does not schedule recursive media cleanup unless ttl is configured", async () => { + it("runs playback cache cleanup at startup and hourly without an attachment ttl", async () => { vi.useFakeTimers(); const { startGatewayMaintenanceTimers } = await import("./server-maintenance.js"); - const timers = startGatewayMaintenanceTimers({ - ...createMaintenanceTimerDeps(), - }); + const timers = startGatewayMaintenanceTimers(createMaintenanceTimerDeps()); + await vi.advanceTimersByTimeAsync(0); + expect(prunePlaybackTranscodeCacheMock).toHaveBeenCalledTimes(1); + expect(cleanOldMediaMock).not.toHaveBeenCalled(); + + await vi.advanceTimersByTimeAsync(60 * 60_000); + expect(prunePlaybackTranscodeCacheMock).toHaveBeenCalledTimes(2); expect(cleanOldMediaMock).not.toHaveBeenCalled(); - expect(timers.mediaCleanup).toBeNull(); stopMaintenanceTimers(timers); }); @@ -236,7 +243,7 @@ describe("startGatewayMaintenanceTimers", () => { stopMaintenanceTimers(timers); }); - it("runs startup media cleanup and repeats it hourly", async () => { + it("adds configured attachment cleanup to playback maintenance", async () => { vi.useFakeTimers(); const { startGatewayMaintenanceTimers } = await import("./server-maintenance.js"); @@ -245,14 +252,17 @@ describe("startGatewayMaintenanceTimers", () => { mediaCleanupTtlMs: MEDIA_CLEANUP_TTL_MS, }); + await vi.advanceTimersByTimeAsync(0); + expect(prunePlaybackTranscodeCacheMock).toHaveBeenCalledTimes(1); expect(cleanOldMediaMock).toHaveBeenCalledWith(MEDIA_CLEANUP_TTL_MS, { recursive: true, pruneEmptyDirs: true, }); - cleanOldMediaMock.mockClear(); await vi.advanceTimersByTimeAsync(60 * 60_000); - expect(cleanOldMediaMock).toHaveBeenCalledWith(MEDIA_CLEANUP_TTL_MS, { + expect(prunePlaybackTranscodeCacheMock).toHaveBeenCalledTimes(2); + expect(cleanOldMediaMock).toHaveBeenCalledTimes(2); + expect(cleanOldMediaMock).toHaveBeenLastCalledWith(MEDIA_CLEANUP_TTL_MS, { recursive: true, pruneEmptyDirs: true, }); @@ -260,6 +270,34 @@ describe("startGatewayMaintenanceTimers", () => { stopMaintenanceTimers(timers); }); + it("keeps playback cleanup independent of attachment cleanup failures", async () => { + vi.useFakeTimers(); + cleanOldMediaMock.mockRejectedValueOnce(new Error("synthetic attachment cleanup failure")); + const { startGatewayMaintenanceTimers } = await import("./server-maintenance.js"); + const deps = { + ...createMaintenanceTimerDeps(), + logHealth: { error: vi.fn() }, + }; + + const timers = startGatewayMaintenanceTimers({ + ...deps, + mediaCleanupTtlMs: MEDIA_CLEANUP_TTL_MS, + }); + + await vi.waitFor(() => { + expect(deps.logHealth.error).toHaveBeenCalledWith( + expect.stringContaining("synthetic attachment cleanup failure"), + ); + }); + expect(prunePlaybackTranscodeCacheMock).toHaveBeenCalledTimes(1); + + await vi.advanceTimersByTimeAsync(60 * 60_000); + expect(prunePlaybackTranscodeCacheMock).toHaveBeenCalledTimes(2); + expect(cleanOldMediaMock).toHaveBeenCalledTimes(2); + + stopMaintenanceTimers(timers); + }); + it("broadcasts tick keepalives without dropIfSlow", async () => { vi.useFakeTimers(); vi.setSystemTime(new Date("2026-04-12T00:00:00Z")); @@ -297,7 +335,7 @@ describe("startGatewayMaintenanceTimers", () => { stopMaintenanceTimers(timers); }); - it("skips overlapping media cleanup runs", async () => { + it("skips overlapping configured attachment cleanup runs", async () => { vi.useFakeTimers(); let resolveCleanup = () => {}; let cleanupReady = false; @@ -328,6 +366,36 @@ describe("startGatewayMaintenanceTimers", () => { await vi.advanceTimersByTimeAsync(60 * 60_000); expect(cleanOldMediaMock).toHaveBeenCalledTimes(2); + resolveCleanup(); + await vi.advanceTimersByTimeAsync(0); + stopMaintenanceTimers(timers); + }); + + it("skips overlapping playback cache cleanup runs", async () => { + vi.useFakeTimers(); + let resolveCleanup = () => {}; + prunePlaybackTranscodeCacheMock.mockImplementation( + () => + new Promise((resolve) => { + resolveCleanup = resolve; + }), + ); + const { startGatewayMaintenanceTimers } = await import("./server-maintenance.js"); + + const timers = startGatewayMaintenanceTimers(createMaintenanceTimerDeps()); + + await vi.advanceTimersByTimeAsync(0); + expect(prunePlaybackTranscodeCacheMock).toHaveBeenCalledTimes(1); + await vi.advanceTimersByTimeAsync(60 * 60_000); + expect(prunePlaybackTranscodeCacheMock).toHaveBeenCalledTimes(1); + + resolveCleanup(); + await vi.advanceTimersByTimeAsync(0); + await vi.advanceTimersByTimeAsync(60 * 60_000); + expect(prunePlaybackTranscodeCacheMock).toHaveBeenCalledTimes(2); + + resolveCleanup(); + await vi.advanceTimersByTimeAsync(0); stopMaintenanceTimers(timers); }); diff --git a/src/gateway/server-maintenance.ts b/src/gateway/server-maintenance.ts index 865fe1ec5139..08d0bfa7489c 100644 --- a/src/gateway/server-maintenance.ts +++ b/src/gateway/server-maintenance.ts @@ -11,7 +11,7 @@ import type { OpenClawConfig } from "../config/types.openclaw.js"; import { sweepStaleRunContexts } from "../infra/agent-run-registry.js"; import { pruneMapToMaxSize } from "../infra/map-size.js"; import { pruneOrphanedDeliveryQueueMedia } from "../infra/outbound/delivery-queue-media-spool.js"; -import { cleanOldMedia } from "../media/store.js"; +import { cleanOldMedia, prunePlaybackTranscodeCache } from "../media/store.js"; import { createLazyPromiseLoader } from "../shared/lazy-promise.js"; import { startSkillCuratorMaintenance } from "../skills/workshop/curator.js"; import { @@ -309,23 +309,23 @@ export function startGatewayMaintenanceTimers(params: { sweepStaleRunContexts(); }, 60_000); - if (typeof params.mediaCleanupTtlMs !== "number") { - return { - tickInterval, - healthInterval, - dedupeCleanup, - mediaCleanup: null, - worktreeCleanup, - skillCuratorCleanup, - }; - } + const playbackTranscodeCacheCleanupLoader = createLazyPromiseLoader(async () => { + try { + await prunePlaybackTranscodeCache(); + } catch (err) { + params.logHealth.error(`playback transcode cache cleanup failed: ${formatError(err)}`); + } finally { + playbackTranscodeCacheCleanupLoader.clear(); + } + }); let mediaCleanupInFlight: Promise | null = null; - const runMediaCleanup = () => { - if (mediaCleanupInFlight) { + const runConfiguredMediaCleanup = () => { + const ttlMs = params.mediaCleanupTtlMs; + if (typeof ttlMs !== "number" || mediaCleanupInFlight) { return mediaCleanupInFlight; } - mediaCleanupInFlight = cleanOldMedia(params.mediaCleanupTtlMs, { + mediaCleanupInFlight = cleanOldMedia(ttlMs, { recursive: true, pruneEmptyDirs: true, }) @@ -338,11 +338,15 @@ export function startGatewayMaintenanceTimers(params: { return mediaCleanupInFlight; }; - const mediaCleanup = setInterval(() => { - void runMediaCleanup(); - }, 60 * 60_000); + const runMediaMaintenance = () => { + // Playback has a fixed cache lifecycle and must not depend on the optional + // attachment-retention sweep being enabled or completing successfully. + void playbackTranscodeCacheCleanupLoader.load(); + void runConfiguredMediaCleanup(); + }; + const mediaCleanup = setInterval(runMediaMaintenance, 60 * 60_000); - void runMediaCleanup(); + runMediaMaintenance(); return { tickInterval, diff --git a/src/media/store.playback-cache.test.ts b/src/media/store.playback-cache.test.ts index 531ade66aa54..df6cd7cbbd42 100644 --- a/src/media/store.playback-cache.test.ts +++ b/src/media/store.playback-cache.test.ts @@ -32,10 +32,7 @@ afterAll(async () => { }); afterEach(async () => { - await fs.rm(path.join(store.getMediaDir(), store.PLAYBACK_TRANSCODE_SUBDIR), { - recursive: true, - force: true, - }); + await fs.rm(store.getMediaDir(), { recursive: true, force: true }); }); it("evicts oldest playback transcodes when insertion enforcement exceeds its byte budget", async () => { @@ -58,20 +55,30 @@ it("evicts oldest playback transcodes when insertion enforcement exceeds its byt await expect(fs.stat(newPath)).resolves.toMatchObject({ size: sparseSize }); }); -it("uses the long playback TTL instead of the default transient-media TTL", async () => { +it("prunes only playback entries using the fixed seven-day retention", async () => { const testApi = getPlaybackCacheTestApi(); - const cacheDir = path.join(store.getMediaDir(), store.PLAYBACK_TRANSCODE_SUBDIR); + const mediaDir = await store.ensureMediaDir(); + const cacheDir = path.join(mediaDir, store.PLAYBACK_TRANSCODE_SUBDIR); await fs.mkdir(cacheDir, { recursive: true }); const freshPath = path.join(cacheDir, "v2-fresh.m4a"); const oldPath = path.join(cacheDir, "v2-expired.m4a"); - await Promise.all([fs.writeFile(freshPath, "fresh"), fs.writeFile(oldPath, "old")]); + const transientPath = path.join(mediaDir, "expired-transient.m4a"); + await Promise.all([ + fs.writeFile(freshPath, "fresh"), + fs.writeFile(oldPath, "old"), + fs.writeFile(transientPath, "transient"), + ]); const nowMs = Date.now(); await fs.utimes(freshPath, (nowMs - 5 * 60_000) / 1000, (nowMs - 5 * 60_000) / 1000); const expiredMs = nowMs - testApi.PLAYBACK_TRANSCODE_TTL_MS - 1_000; - await fs.utimes(oldPath, expiredMs / 1000, expiredMs / 1000); + await Promise.all([ + fs.utimes(oldPath, expiredMs / 1000, expiredMs / 1000), + fs.utimes(transientPath, expiredMs / 1000, expiredMs / 1000), + ]); - await store.cleanOldMedia(); + await store.prunePlaybackTranscodeCache(); await expect(fs.stat(freshPath)).resolves.toMatchObject({ size: 5 }); await expect(fs.stat(oldPath)).rejects.toMatchObject({ code: "ENOENT" }); + await expect(fs.stat(transientPath)).resolves.toMatchObject({ size: 9 }); }); diff --git a/src/media/store.ts b/src/media/store.ts index d2492e89d94f..e918e19cabd3 100644 --- a/src/media/store.ts +++ b/src/media/store.ts @@ -290,11 +290,12 @@ async function enforcePlaybackTranscodeCacheLimit(): Promise { await queuePlaybackCacheOperation(prunePlaybackTranscodeCacheToSize); } -async function prunePlaybackTranscodeCacheRetention(): Promise { +/** Prunes expired playback renditions and reapplies the fixed cache size budget. */ +export async function prunePlaybackTranscodeCache(): Promise { await queuePlaybackCacheOperation(async () => { const cacheDir = resolveMediaScopedDir( PLAYBACK_TRANSCODE_SUBDIR, - "prunePlaybackTranscodeCacheRetention", + "prunePlaybackTranscodeCache", ); await openMediaStore(MAX_BYTES, cacheDir).pruneExpired({ ttlMs: PLAYBACK_TRANSCODE_TTL_MS, @@ -305,10 +306,9 @@ async function prunePlaybackTranscodeCacheRetention(): Promise { }); } -/** Prunes expired media files, optionally recursing into scoped media subdirectories. */ +/** Prunes expired non-playback media, optionally recursing into scoped subdirectories. */ export async function cleanOldMedia(ttlMs = DEFAULT_TTL_MS, options: CleanOldMediaOptions = {}) { await pruneNonPlaybackMedia(ttlMs, options); - await prunePlaybackTranscodeCacheRetention(); // Trust metadata must not outlive the staged file that it authorizes. const { pruneStaleTrustedGeneratedHtmlMarkers } = await import("./web-media.js"); await pruneStaleTrustedGeneratedHtmlMarkers();