Repository navigation
fix(studio-server): a render that ran over 5 minutes can still be read after it finishes - #7087
Merged
Merged
Conversation
Contributor
Contributor
There was a problem hiding this comment.
Approved at fb588727. No blockers found.
- The fix is right: the TTL now counts from the first sweep that sees a job finished, not from
createdAt. The marker lives in aWeakMapkeyed by the job object, which suits an adapter that mutates jobs in place and doesn't leak. A job is kept at least the TTL after it finishes and at most one sweep longer. Both stores share the helper.createdAtis gone from the job maps, and the renders listing still reports its own mtimecreatedAt. - Fails without the fix: confirmed locally. With
render.tsandmedia.tsatmain(47ddded), bothfinishedJobs.test.tscases fail withexpected 404 to be 200. At this head, thefinishedJobs,render,media,render.streamingandrenderDeletiontests give 5 files and 122 tests passed.tsc --noEmitin studio-server exits 0. - Change since
9daa47d4: a rename only (forgetJobsFinishedLongerThan,firstSeenFinishedAt,ttlMsmoved first). Both call sites use the new argument order. Behaviour is the same. renderDeletion.test.ts: the move from 6 to 7 minutes follows from the new rule, since the job is first seen finished at the 1-minute sweep.- Side effect (fine): on-disk renders registered by the listing used to be dropped at the next sweep because their mtime was old. Now they stay for about 5 minutes, which is in keeping with the comment that they "remain downloadable".
CI: both CI runs at this head were cancelled because the Build job hit its 10-minute limit inside bun run verify:packed-manifests. That was not a test failure. The same timeout hit the earlier head and an unrelated branch (miao/templates-unpack-stdin), so it looks like runner slowness, not this change. I re-ran the failed jobs on run 41635493264. Merge should wait for that rerun to go green.
miguel-heygen
enabled auto-merge
November 3, 2026 13:15
Merged
1 task done
SanHsien
added a commit
to SanHsien/hyperframes
that referenced
this pull request
Nov 4, 2026
…r long runs Ports upstream heygen-com#7087 (fc006f7): the finished-job TTL now counts from when a sweep first sees the job finished, not from createdAt, so a render that ran longer than the TTL is still readable after it completes. Adds the shared helper and its regression tests, which failed on the old code with 404. Also records the fourth 2026-11-04 upstream review pass: watermark advanced to 3f42dc1 / PR heygen-com#7077 with file:line evidence; the remaining range stays in ROADMAP. Co-authored-by: Miguel Ángel <[email protected]> Co-Authored-By: Claude Opus 5.5 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
A finished render or background-removal job now stays readable for 5 minutes after it finishes. Before, the sweep counted those 5 minutes from when the job started, so any render that ran longer than 5 minutes was forgotten within a minute of finishing:
GET /render/:jobId/progressanswered 404 although the video was on disk, and a client waiting on it reported the export as missing.Why
The job store's TTL read
now - job.createdAt, andcreatedAtis stamped when the job is created. A long render (a 6-minute export is ordinary) is past the TTL the moment it ends, and the next 60 s sweep deletes it. The same pattern was in the media-processing job store.Related work
None.
How
helpers/finishedJobs.tsforgetJobsFinishedLongerThan, owns the rule for both stores. The adapter finishes a job by changing it in place (no event reaches the server), so the sweep notes, per job object, the first time it sees it finished, and forgets it once that is older than the TTL. A job is therefore kept at least the TTL after it finishes, and at most one sweep interval longer.routes/render.tsandroutes/media.tsuse it;createdAtis removed from both job maps (nothing else read it; the renders listing keeps its owncreatedAtfield, from the file's mtime).Test plan
routes/finishedJobs.test.ts(new): a render that runs 6 minutes, finishes, and is read 61 s later answers 200, then 404 once it has been finished for longer than the TTL; the same for a background-removal job.Fails without the fix: with
render.tsandmedia.tsat main and the new test kept, both tests fail (expected 404 to be 200).routes/renderDeletion.test.ts: its render is complete from the start, so it is first seen finished at the 1-minute sweep and now forgotten at 7 minutes instead of 6; the test advances 7 minutes.vitest runoffinishedJobs,render,media,render.streamingandrenderDeletiontests: 5 files, 122 tests passed, 3 runs in a row.tsc --noEmitin studio-server exit 0.oxlintandoxfmt --checkon the changed files: clean.Unit tests added/updated
Manual testing performed
Documentation updated (if applicable)
Comments follow CONTRIBUTING.md "Comments": they say why, not what, and a bug fix says what the code must do and how to reproduce the bug