Conversation
miguel-heygen
left a comment
There was a problem hiding this comment.
Reviewed head 220b9a40fae3b4eb28c7126dfe0a67b45f98b2b0 against c777f36d47d7fb914c7b3be4a9536ff343c4075f (2 files, +187/-18). No prior reviews or comments; no code blocker found.
packages/studio-server/src/routes/render.ts:255-260preserves the cached output directory and discovers disk-only artifacts through the adapter after expiry/restart; both official adapter layouts are covered.packages/studio-server/src/routes/render.ts:21-29validates every candidate before unlinking any artifact, including the metadata sidecar. The route rejects path separators/NUL at :266.
Validation: 57/57 focused tests passed (38 existing route tests, 5 added regression tests, 14 independent temporary-filesystem witnesses). Covered all three formats, cache expiry/reload, disk-only and cached files, metadata, shared/multiple directories, preservation of unrelated files, idempotency, escaping/dangling/contained symlinks, adapter errors, real unlink failure and retry. The expiry test fails against the base because DELETE returns 200 while the MP4 remains. Changed-file lint/format, comment citations and diff whitespace checks passed.
CI is not verified: Graphite/WIP pass, but all eight exact-head Actions workflows—including CI, CodeQL and Windows verification—are action_required. CI run has zero jobs; no CI logs exist yet. A maintainer must approve the fork workflows before their results can support a merge verdict.
Scope: audited the full changed route/tests plus path guards, cache/history, Studio delete caller and both official adapter output/metadata paths. Local Hono/filesystem fixtures; no browser, live renderer, Windows execution, broad build or typecheck.
— Magi
Verdict: COMMENT
Reasoning: Local evidence supports the deletion fix and I found no source defect. The required verification workflows have not run, so a merge-ready approval remains pending CI.
What
fixes render deletion after the job cache expires.
Why
delete was returning success when the cached job was missing, but the file stayed on disk. reloading Studio brought the render back.
Related work
#3710 touches the same route and cache for shutdown handling, but doesn't fix deletion after expiry.
How
uses the cached output directory when available, otherwise checks the adapter's render directories. validates the paths before deleting the files and metadata.
Test plan
tested in Studio local preview with cleanup enabled. the expired render came back after reload before the fix and stayed deleted afterward. ordinary deletion was also checked.
added a regression test that fails without the fix.