Repository navigation
fix(cli): a wedged render trips its 60 s watchdog, one local render at a time, and fonts the cloud can't resolve are flagged - #7168
Conversation
…the 60 s stall watchdog The sequential streaming watchdog raced only the capture call. Session init (where the fast-capture effect scan runs) and opening the screenshot session for effect frames waited on CDP's protocol timeout instead, which scales with output area up to 30 minutes, so a starved page hung a render for ~25 minutes before the watchdog saw the next frame. The effect scan now gives up when a chunk outlasts its 120 s budget, opening the effect-frame session gets its own no-progress window, and session init gets a 300 s one; each trip retries on a fresh session like any other stall. Co-Authored-By: Claude Opus 5.5 <[email protected]>
… says for whom Renders started side by side from different sessions starve each other until frames stop landing. `hyperframes render` now takes a machine-wide slot (a lock file in the HyperFrames cache directory, held by pid) after its preflight and holds it for the render or the whole batch. A second render waits and prints the pid and project it is waiting behind; `--no-wait` fails at once instead. A holder whose process is gone is taken over, and HYPERFRAMES_RENDER_LOCK=off restores side-by-side renders. `--workers` is unchanged. Co-Authored-By: Claude Opus 5.5 <[email protected]>
…nt shorthand Lint read only `font-family`, so `font: 500 16px "PingFang SC", sans-serif` passed while the renderer, which reads the shorthand too, drew the installed Mac font locally and the cloud refused it. The shorthand now counts for font_family_without_font_face (its parser moves to @hyperframes/parsers so core and lint share one), and a new font_face_local_only warning names an @font-face whose only sources are local(): it renders here and falls back in the cloud. The rule's hint and the format reference stop recommending local() for OS fonts. Co-Authored-By: Claude Opus 5.5 <[email protected]>
… use it The cloud's "fonts that could not be resolved: PingFang SC" named no composition. For a render submitted from a local project, the failure now lists each refused family with the project files that name it, and says how to ship the font so the cloud can use it. Co-Authored-By: Claude Opus 5.5 <[email protected]>
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Automations to automatically generate PRs for you. |
jrusso1020
left a comment
jrusso1020
left a comment
There was a problem hiding this comment.
Reviewed at 7da300c2f8c8a4a7c8de42e8fd073d42d6eb19ae.
Important — valid shorthand now fails font lint (packages/lint/src/rules/fonts.ts:117; packages/parsers/src/fontShorthand.ts:17–22). The new shorthand caller exposes an inherited tokenizer limitation as a new error: font: 16px / 1.5 Inter is diagnosed as missing family / 1.5 inter, even when Inter has an explicit @font-face. font: 16px /1.5 Inter similarly invents /1.5 inter. A pre-size var(--weight) is also mistaken for the size (--weight:700; font:var(--weight) 16px Inter reports 16px inter). These valid projects previously passed this rule and now fail lint / render --strict. Consume the optional slash and line-height independently of whitespace, and resolve or correctly distinguish pre-size variable tokens. Add positive tests for these forms plus the compact spelling; retain a negative test for a genuinely missing family.
Evidence: built the workspace and ran renderLock.test.ts + fontFailure.test.ts (13 passed), riskFrames.test.ts (18), fonts.test.ts (60), and bun test …/captureStreamingStage.test.ts (45): 136 passed. Additional direct lintHyperframeHtml probes produced no finding for 16px/1.5 Inter but an error for both spaced-slash forms above; separate probes with an explicit Inter face also reproduced the spaced slash and var(--weight) errors. No cloud render or original hang reproduction was run.
Audited: lock/acquisition/release helpers and their CLI wiring; risk scan budget; sequential streaming init, risk-session open, retry/resume and cleanup; shared shorthand extraction, font rules/tests and cloud font-error helper. Trusting: unchanged bulk render-plan/command and deterministic-font machinery beyond the touched integration paths; generated manifest was source-read rather than independently regenerated.
The lock reuses the existing atomic project-lock helper, defaults under the user's ~/.cache/hyperframes/locks/ (overridable by HYPERFRAMES_CACHE_DIR), and releases around the batch/single render. Nit / follow-up (packages/cli/src/utils/projectLock.ts:31): PID-only liveness cannot distinguish PID reuse; a stale lock can wait behind an unrelated live process (follow-up: record/check process birth identity). The new local()-only finding is a warning; missing shorthand families are errors. Default render does not block on these lint findings; --strict blocks errors and --strict-all also blocks warnings. Cleanup's motion-blur diagnostic and browser/page close are bounded; I found no second verified watchdog/lock blocker.
Need: yes · Size: ok, watchdog/lock/font fixes have focused tests · Reuse: existing atomic project-lock and shared font parser · Simpler: already localized · Structure: fine
Risk: medium — default lock/retry behavior and new lint errors affect existing projects.
Verdict: REQUEST CHANGES
Reasoning: Fix the newly introduced shorthand false errors before enabling the broader lint rule. The existing focused tests pass but omit these valid CSS forms.
— hf-review
| /** The family list a `font-family` or `font` declaration names; the renderer reads both. */ | ||
| function usedFamilyList(decl: postcss.Declaration): string { | ||
| if (isFontFaceDeclaration(decl)) return ""; | ||
| return decl.prop.toLowerCase() === "font" ? fontShorthandFamilies(decl.value) : decl.value; |
There was a problem hiding this comment.
Important: This new shorthand lint path errors on valid font: 16px / 1.5 Inter (missing family / 1.5 inter) and font: var(--weight) 16px Inter with --weight:700 (missing family 16px inter), even with an explicit Inter @font-face. Compact 16px/1.5 Inter passes. I reproduced these through lintHyperframeHtml; they newly fail lint and render --strict. Please fix the shared extractor to handle whitespace around slash/line-height and pre-size variables, and add regressions covering compact/spaced forms and a genuine missing family.
There was a problem hiding this comment.
Fixed in 72e3d14 (head dc2a72e4). The shorthand parser now follows the CSS grammar: a spaced / line-height is skipped, var() no longer counts as the size, and angles like oblique 10deg aren't taken for sizes. font: 16px / 1.5 Inter and font: var(--weight) 16px Inter both read as Inter. If the size can't be told apart from a var() (font: var(--size) Inter), lint skips the declaration instead of reporting a missing font. The renderer had the same bug: for the spaced slash it looked up a family called / 1.5 Inter. Two new core tests cover it. New parsers/src/fontShorthand.test.ts, plus cases in lint fonts.test.ts and core deterministicFonts-shorthand.test.ts; they fail on the old parser.
…it the single-render notes out of executeHeldRender Co-Authored-By: Claude Opus 5.5 <[email protected]>
jerrai-bot-heygen
left a comment
jerrai-bot-heygen
left a comment
There was a problem hiding this comment.
At 500dedf7, I independently verified three remaining gaps in the new watchdog/render-lock contract (inline): the default slot is per-user rather than machine-wide; a malformed lock left by a crash in the no-hard-link fallback cannot be reclaimed; and primary createCaptureSession remains outside the new init watchdog. The successor commit fixed the prior Comments/Fallow CI issues without changing these runtime paths. Rames has separately requested changes for valid spaced/variable font shorthand becoming a false error in lint; I verified the parser and do not duplicate that finding inline. local()-only face findings themselves are warnings. Hosted checks on this successor were still running at review time; no cloud render, local suite, or live hung-browser reproduction was run. No merge, queue or rollout action. — Jerrai
|
|
||
| const POLL_MS = 1_000; | ||
|
|
||
| const renderLockPath = () => hyperframesCacheDir("locks", "render.lock"); |
There was a problem hiding this comment.
This default path is per OS user, not per machine: hyperframesCacheDir() uses homedir()/.cache/hyperframes unless HYPERFRAMES_CACHE_DIR is explicitly shared (packages/core/src/cacheDir.ts:4–6). On a shared Mac/CI host, user A and user B acquire different render.lock files and render concurrently, despite the promised one render per machine. If the contract is machine-wide, use a machine-shared slot with a defined permissions/ownership policy (and a cross-user regression); otherwise narrow the claim/UI to per-user.
There was a problem hiding this comment.
Addressed in 84394f1 by correcting the claim, not the mechanism. The default stays per OS user: the bug-bash failure was many sessions of one user, and a lock shared across users under /tmp brings its own ownership and cleanup problems (sticky-bit deletes, stale files owned by another user). The waiting message, --no-wait error, --wait help, comments and docs/packages/cli.mdx now say "one local render at a time per user account", and say that setting HYPERFRAMES_CACHE_DIR to a shared folder shares the slot across accounts. renderLock.test.ts asserts the wording and the HYPERFRAMES_CACHE_DIR location.
| let waitingBehind: RenderLockHolder | null | undefined; | ||
| while (!tryAcquire(path, holder)) { | ||
| const current = readHolder<RenderLockHolder>(path); | ||
| if (!current && !existsSync(path)) continue; |
There was a problem hiding this comment.
A lock file that exists but cannot be parsed is never reclaimed: tryAcquire() returns false, readHolder() is null, existsSync(path) is true, and clearAbandonedBreak() also returns false when no valid .break holder exists. Then wait loops forever (or --no-wait refuses forever). This is reachable if the process dies mid-write in createFileAtomically()'s no-hard-link exclusive-write fallback (packages/core/src/atomicFile.ts:63–68,79–87), or if a .break write is interrupted. Please make malformed/dead records recoverable without deleting a genuinely live lock, with an injected crash-mid-write regression. The current PID-only liveness check also cannot distinguish a recycled PID; consider that in the ownership proof.
There was a problem hiding this comment.
Fixed in 0ed7521. An unreadable lock or .break file older than 5 s by mtime counts as abandoned, and a lock goes through the same .break takeover as a dead holder's lock (projectLock's takeover generalised to takeOver(path, holder, stillStale)). A fresh unreadable file is still left alone. Four tests, fake timers: an old unreadable lock is reclaimed; a fresh one is refused under --no-wait; a waiting render proceeds once the file passes 5 s; an old unreadable .break next to a dead holder is cleared.
| await initializeSession(opened); | ||
| return opened; | ||
| }; | ||
| const initStallMs = Math.max(SESSION_INIT_STALL_MS, resolveCaptureStallTimeoutMs()); |
There was a problem hiding this comment.
The new init watchdog starts after the primary session is opened: when probeSession is null, lines 1018–1026 await createCaptureSession(...) before this withinInitBudget wrapper and before the try/cleanup at line 1068. That call acquires Chrome and awaits browser.newPage()/CDP setup; a wedged launch can still sit on its browser/protocol timeout rather than the no-progress window, reproducing the reported long-hang class. Please guard primary session creation too and define how a session that arrives after the timeout is closed (as openRiskSession does), with a stalled-open test.
There was a problem hiding this comment.
Fixed in 7385ea8. Opening the primary capture session now runs under the same 300 s setup window. A trip says "while opening the capture session" and goes to the existing stall retry, and a session that finishes opening after its attempt was abandoned is closed instead of leaking a browser. The window uses the per-frame watchdog's clock, which forgives a >15 s gap between checks as possible sleep, so the worst case is two 300 s windows. captureStreamingStage.test.ts › "trips the watchdog when the capture session never finishes opening, and closes it if it opens late" fails on the old code.
…ze in the font shorthand `font: 16px / 1.5 Inter` was read as the family `/ 1.5 Inter`, and `font: var(--weight) 16px Inter` as `16px Inter`, because a var() counted as the size. The parser now follows the shorthand grammar: optional style, variant, weight and width keywords (or var()), then a length, percentage, keyword or calc() size, then a `/ line-height` in any spacing. When no size can be told apart from a var(), it says so: lint skips that declaration instead of reporting a missing font, and the renderer keeps its best guess. Co-Authored-By: Claude Opus 5.5 <[email protected]>
The lock lives in the account's HyperFrames cache folder, so renders by two user accounts never queued behind each other while the messages, flag help and docs said "this machine". They now say "per user account" and that HYPERFRAMES_CACHE_DIR set to a shared folder shares the slot across accounts. Co-Authored-By: Claude Opus 5.5 <[email protected]>
A lock or `.break` file that exists but cannot be parsed (a crash mid-write where hard links are unavailable) was never reclaimed: a waiting render waited forever and --no-wait refused forever. Once such a file is older than any real write (5 s by mtime) it counts as abandoned: the lock goes through the same `.break` takeover as a dead holder's, and a takeover file is removed. Co-Authored-By: Claude Opus 5.5 <[email protected]>
…tall budget The sequential streaming attempt opened its own capture session (Chrome launch and new page) before the setup watchdog and outside the attempt's cleanup, so a wedged launch still waited for the protocol timeout. Opening now shares the 300 s setup window with session initialisation, a trip fails the attempt like any other sequential stall, and a session that finishes opening after its attempt gave up is closed instead of leaking a browser. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]>
jerrai-bot-heygen
left a comment
jerrai-bot-heygen
left a comment
There was a problem hiding this comment.
At dc2a72e4, the shorthand parser, per-user lock wording and primary-session-open watchdog fixes look repaired, but the new malformed-lock recovery trades a permanent block for a double-acquire race: a live writer suspended >5 s after the no-hard-link fallback's exclusive open is misclassified as abandoned and unlinked, allowing a second render to take a replacement lock before the first resumes and returns success (inline). The recovery needs evidence of owner death or a publication protocol that cannot mistake a live partial write for abandonment. Rames's older shorthand finding appears fixed at this source head but their separate change request still needs re-review. Hosted CI at this successor was queued/running, not green, at review time; no live stall, cloud render, local tests or merge action from me. — Jerrai
| function abandonedWrite(path: string): boolean { | ||
| if (readHolder(path)) return false; | ||
| try { | ||
| return Date.now() - statSync(path).mtimeMs > ABANDONED_WRITE_MS; |
There was a problem hiding this comment.
The 5-second mtime grace cannot establish that the lock writer is dead. On a no-hard-link volume, createFileAtomically falls back to openSync(path, 'wx') followed by writeFileSync(fd, content) (packages/core/src/atomicFile.ts:63–69,79–89). If process A is paused after opening for >5 s, the file exists but is unreadable; process B's abandonedWrite() returns true, takeOver() unlinks it and acquires a replacement. When A resumes, its write to the still-open descriptor completes and tryAcquire() returns true as well. Both renders now hold what each believes is the one slot. Please make recovery require positive proof of a dead writer (or a publication protocol that cannot leave an ownerless-looking live lock), and add an injected pause-after-open/two-contenders regression. A time threshold alone trades the original wedged-lock failure for concurrent renders.
There was a problem hiding this comment.
Fixed in 8cfd411 (head 40262cc). Time alone no longer decides anything: the 5 s grace is gone. Before creating the lock, each contender writes its own uniquely named owner marker (render.lock.owner-<pid>-<random>, holding its pid and process start time). It keeps the marker while it holds the slot and removes it otherwise. An unreadable lock or .break is reclaimed only when no other contender's marker belongs to a live process (pid alive, start time matching where the platform reports one), through the .break takeover, which re-checks. Regressions: (a) A opens and pauses before writing, B waits through 60 s of fake time without taking over, C under --no-wait is refused, then B gets the slot only after A releases; this fails with the owner check removed. (b) A dies after open, B reclaims, and A's marker is cleaned up.
…rovably gone Where hard links are unavailable the lock is created by an exclusive open and then written, so a contender paused between the two leaves an unreadable lock while it is still alive. The 5 s mtime grace took such a lock over, and both processes then held the slot. Each contender now writes an owner marker under a name only it can take (pid plus random suffix, holding its process identity) before it creates the lock, keeps it while it holds the slot, and removes it otherwise. An unreadable lock or `.break` is reclaimed only when no other contender's marker belongs to a live process; markers of dead processes, or of a reused pid, are deleted. No reclaim is decided by time alone. Co-Authored-By: Claude Opus 5.5 <[email protected]>
…b under the complexity gate acquireRenderLock hands contender setup and a single marked claim to their own functions, and the test stub for createCaptureSession splits its held-open cases from the fake session it builds, so neither exceeds Fallow's cognitive or CRAP thresholds. Behaviour is unchanged. Co-Authored-By: Claude Opus 5.5 <[email protected]>
…ck and font-lint additions Main's CLI unpacked size was already ~0.99% over its budget; the render lock and owner markers push it past the 1% tolerance. The CLI unpacked budget takes the same value as #7169 and #7170 (25,159,107) so the PRs merge cleanly. The engine's packed size grows by about 4.7 KB for the effect-scan and session-open watchdogs; its packed budget moves to 5% above the measured 467,097, as the budget file's headroom rule sets it. Co-Authored-By: Claude Opus 5.5 <[email protected]>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
jrusso1020
left a comment
jrusso1020
left a comment
There was a problem hiding this comment.
Reviewed at 73d648a5c7e37990efc90ce097b142fee1759a9f, including the complete delta from 7da300c2f8c8a4a7c8de42e8fd073d42d6eb19ae.
My font-shorthand finding is fixed: compact/spaced slash, pre-size weight variables and oblique-angle forms now pass, while a genuine missing family still produces an error. The per-user wording, owner markers for unreadable writes and primary-session-open watchdog/late-close coverage address the previously reported cases. local()-only faces remain warnings.
Important — stale-break recovery can still grant two render slots (missed in round 1). packages/cli/src/utils/renderLock.ts:75–78 reads a dead .break holder, then unconditionally unlinks that pathname. Two reapers can race: B pauses after reading the old dead breaker; A clears it and creates a live replacement breaker; B resumes and deletes A's replacement, then acquires the main lock. A's already-started takeover (projectLock.ts:44–51) can then unlink B's main lock using its stale read and acquire too.
I reproduced this with two real child processes running the actual acquisition functions; filesystem hooks only paused completed reads to make the interleaving deterministic. Both acquireRenderLock() calls returned success while both processes remained alive and held their slots; the main lock changed from B to A. The new owner-marker guard is bypassed in this parsed-dead-breaker branch.
Do not auto-delete a breaker based solely on a prior read. A safe minimal fix is to refuse an abandoned .break with an actionable recovery error rather than reclaim it unsafely. If automatic recovery is retained, it needs ownership fencing/serialization that prevents removal of a replacement live breaker and protects the main-lock takeover. Add the two-contender replacement-breaker regression; a second unchecked read immediately before unlink is still a race.
Validation at this head: workspace build passed; lock (12), shorthand parser (19), font lint (64) and streaming stage (46) tests passed, 141 total. I independently replayed the two-process race and the original font cases with a missing-font negative control. No cloud render or original 25-minute hang was reproduced. The race is a remaining Important issue in the original PR's lock path, not claimed as introduced by this latest delta.
Audited: all 18 delta files, lock owner/publication/recovery helpers and consumers, shorthand parser/lint integration, primary/risk-session setup and cleanup. Unchanged bulk render machinery outside those integration paths retains the prior review scope. CI state is not the basis for this verdict.
Need: yes · Size: ok · Reuse: existing process-identity and project-lock helpers · Simpler: fail safely on uncertain breaker ownership · Structure: fine
Risk: medium — the default slot can fail to serialize concurrent renders.
Verdict: REQUEST CHANGES
Reasoning: The earlier specific repairs are present, but the demonstrated replacement-breaker race still violates exclusive ownership.
— hf-review
| if (existsSync(breakPath)) { | ||
| const breaker = readHolder(breakPath); | ||
| if (breaker ? alive(breaker.pid) : liveOtherOwners(path, marker) > 0) return false; | ||
| removeIfPresent(breakPath); |
There was a problem hiding this comment.
Important — replacement-breaker race, missed in round 1. B can pause after reading a dead .break; A clears it and creates a live replacement; B then unlinks A's replacement here and acquires the main lock. A's already-started takeover can overwrite B's new lock using its stale main-holder read. I reproduced two real processes both returning acquireRenderLock success while both remained alive; hooks only delayed reads. Do not reclaim a .break on a prior snapshot: fail safely with an actionable stale-break error, or add serialized ownership fencing so replacement breakers/main holders cannot be removed. Add the two-reaper interleaving regression; another unchecked read before unlink does not remove the race.
There was a problem hiding this comment.
Fixed in 9e78b03. A .break is now only ever removed by the render that wrote it, so there is no "delete the breaker I read earlier" step left to race. The takeover re-reads the main lock record (pid, start time, start token) under its own .break right before replacing it. New interleaving test pauses contender one while it holds .break (beforeReplace); contender two is refused under --no-wait and, when waiting, never touches the .break over 30 s of fake time.
![heygengenesis[bot]](https://avatars.githubusercontent.com/in/2913576?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: REQUEST CHANGES · 2 blocker · 3 important · 0 nits · head 73d648a5
The earlier findings are fixed. That covers the spaced slash, per-user wording, primary-session watchdog, unreadable-lock reclaim and the time-only race. Two new blockers ↓ inline:
- Windows lane red on the new lock test.
Windows render verification › Tests on windows-latest: engine-cli-2fails.renderLock.test.ts:125times out at 20 s, with an unhandledENOENTfromwriteOwnerMarker. That is the regression test for the owner-marker fix. - Font regression on unspaced
var()/line-height.font: 700 var(--size)/1.1 "Inter"now parses to no family. Main parsed it.
Scope
Read end-to-end: renderLock/renderLockOwners/projectLock, fontShorthand, fonts lint, fontFailure, and the captureStreamingStage setup/risk paths. The lock interleavings and shorthand forms were checked with scripts against the real modules; I ran no live stall or cloud render. The CodeQL alert at renderLock.test.ts:121 is in the test's deliberate pause-after-open helper and is not a product issue.
…s size `font: 700 var(--size)/1.1 "Inter"` read as no family at all: the token was tested whole against var(), and the slash after it hid that it was the size, so the renderer injected no face for Inter. A var() followed by `/ line-height` (glued or spaced) can only be the size, and is now read as one. Co-Authored-By: Claude Opus 5.5 <[email protected]>
…orthand Keeps parseFontShorthand under Fallow's cognitive-complexity gate; behaviour is unchanged. Co-Authored-By: Claude Opus 5.5 <[email protected]>
…a reused pid, works in a shared folder and on Windows - A `.break` takeover file is removed only by the render that wrote it. One whose writer is gone stops the render with the file to delete, instead of being removed on an earlier reading, which could remove a live `.break` written since and let two renders hold the slot. The takeover re-reads the lock under its own `.break` right before replacing it. - The lock record keeps its holder's process start time, so a pid that now belongs to another process (an OOM-killed render in a container with a persisted cache) no longer holds the slot forever. - The `locks` folder is created writable for every account, so accounts sharing HYPERFRAMES_CACHE_DIR take turns instead of the second failing with EACCES; a folder this account cannot write in says how to fix it, and another account's file that cannot be removed is left, not fatal. - Windows: no PowerShell start-time lookup per check (pid liveness only), a file another handle holds (EPERM/EBUSY/EACCES) reads as held, a lock that vanished is retried once instead of in a tight loop, and a wait stops when its signal aborts. The tests end every acquire they start before removing their folder, and the paused contender writes through its open descriptor. Co-Authored-By: Claude Opus 5.5 <[email protected]>
With main's audio-attribute rule (#7169) merged in, the font-shorthand and local()-only font findings take @hyperframes/lint's packed tarball to 181925 bytes, over the 179338 budget. The new budget is that measurement plus the usual 5% headroom. Co-Authored-By: Claude Opus 5.5 <[email protected]>
jrusso1020
left a comment
jrusso1020
left a comment
There was a problem hiding this comment.
Reviewed at 1140aa7a46b8967e5f6d3d093d1d0bc0df2f8eca, including the repairs at 9e78b03 and its subsequent main merge/budget update.
The replacement-breaker race from my 73d648a5 review is fixed. In packages/cli/src/utils/renderLockClaim.ts:73–87, only the writer removes its .break; an abandoned breaker now produces an actionable error and remains untouched. Takeover holds its own breaker through the main-record recheck/replacement. The live-breaker interleaving probe no longer permits a second owner, and dead-breaker refusal preserves the file. No new Important finding.
The other reported repairs are present (renderLockOwners.ts:20–26,53–58, renderLockClaim.ts:37–40, renderLock.ts:87–102, fontShorthand.ts:85–98): lock records use process start identity on POSIX; shared-cache directory/error handling supports cooperative cross-account use; wait cancellation/test teardown no longer leaves acquisitions running; Windows polling avoids repeated PowerShell identity calls and treats in-use file errors conservatively. Compact and spaced variable-size/line-height forms again identify Inter, including the renderer's actual font injection.
Validation:
- Full workspace build passed at
9e78b03; lock tests 19/19, parser tests 21/21, and deterministic-font injection 11/11 passed there. These lock/font/watchdog source paths are byte-identical at this reviewed head. - Independent actual-module probes cover live/stale breakers, Linux reused PID identity, marker cleanup and variable-font forms. A real ubuntu/nobody shared-cache probe serializes the live holder and reclaims dead unreadable owner files. I replayed the lock/font probe at this head successfully.
- After the main merge, rebuilt lint, producer and studio-server. Integrated suites at this head: audio/font lint 75 passed, core parity/font injection 28 passed, frame staging 15 passed/1 skipped, Studio split helper 59 passed — 177 passed, 1 skipped on Linux.
The incoming audio lint/Windows staging/Studio helper files match merged main cd7b7383c; the reviewed lock/font/watchdog source did not change. A fresh merge-integration pass found no regression in the reviewed integration paths and listed Linux suites. The lint packed budget 191022 equals ceil(181925 × 1.05) and preserves main's unpacked budget; I checked the policy, recorded reason and arithmetic, not an independent package-size measurement.
Platform limitation: Windows intentionally retains PID-only liveness, so start-token PID-reuse detection is not universal. Native Windows behavior was not executed here. The primary-session open remains guarded by the setup watchdog and closes late arrivals (captureStreamingStage.ts:1030–1047), source-unchanged from the preceding review. No original live hang or cloud render was reproduced. No blanket hosted-CI-green claim.
Need: yes · Size: ok · Reuse: shared process identity and existing compatibility/parser primitives · Simpler: safely refuse uncertain abandoned breakers · Structure: fine
Risk: medium — default lock/recovery behavior has platform-specific paths.
Verdict: APPROVE
Reasoning: The demonstrated exclusivity failure is repaired, and the reviewed integration and controlled probes found no new Important defect.
— hf-review
jerrai-bot-heygen
left a comment
jerrai-bot-heygen
left a comment
There was a problem hiding this comment.
Approved code at 1140aa7a, superseding my previous render-lock change requests. The paused-writer case now holds a live owner marker before exclusive create; the time-only reclaim is gone. The successor closes the stale .break double-acquire race by leaving abandoned breakers for explicit cleanup and only removing the breaker its own contender wrote, and the primary session open runs under the setup budget with late-session cleanup. The per-user lock scope, font shorthand repair and main-merged audio-lint composition were checked; Rames independently approved this exact head. CI is still queued/running on this head, so required checks remain an independent merge gate. The conservative Windows PID fallback and absence of a live cloud render are disclosed. No merge, queue or rollout action from me. — Jerrai
Ports: heygen-com#7165 (font preflight and system TTC names), heygen-com#7166 (font shorthand var(), weight-suffixed names, linked font sheets), heygen-com#7168 (render lock and --wait, effect-scan/session watchdog without the heygen-com#7104 retry, font_face_local_only lint), heygen-com#6972, heygen-com#7183 (adapted), heygen-com#7184; docs heygen-com#7132, heygen-com#7196; prerequisites heygen-com#6665, heygen-com#6697, heygen-com#6716 (cache-dir helper only). Deferred heygen-com#7182, heygen-com#7176. Package size budgets follow upstream's heygen-com#7168 increments. Watermarks: commit 15b1de4, PR 7189; monitored 92. Co-authored-by: Miguel Ángel <[email protected]> Co-authored-by: WaterrrForever <[email protected]> Co-authored-by: Ular Kimsanov <[email protected]> Co-authored-by: James Russo <[email protected]> Co-Authored-By: Claude Opus 5.5 <[email protected]>
What
Three render-side bugs from the style-sampling bug bash (Jake Moran's five-template run, CLI 0.9.9):
Why
Two 3D renders in the bug bash hung for ~25 min while other renders ran (one stuck at frame 56/173, one on frame 1 with "effect scan failed … Runtime.evaluate timed out", then FFmpeg exit 255 after 1,528 s). Finance's cloud render failed on
PingFang SC, named in a component through thefontshorthand, with no file named.Related work
Refs #6842 (style templates). Companion to the style-sampling fixes in #7164.
How
9497c72): in single-worker streaming capture the 60 s watchdog only wrapped the capture call. Session setup (where the fast-capture effect scan runs) and opening the effect-frame screenshot session waited on the browser's per-call protocol timeout, which scales with output size up to 30 min. The effect scan now gives up when a chunk outlasts what is left of its 120 s budget and falls back to plain screenshots. The screenshot-session open gets a 60 s no-progress window and session setup a 300 s one; each trip retries on a fresh session. A late-opening abandoned session closes itself.9e289b5):hyperframes render(local and--docker) takes one machine-wide slot after preflight: a pid-held lock under~/.cache/hyperframes/locks/(orHYPERFRAMES_CACHE_DIR). A second render waits and says for whom;--no-waitrefuses at once; a dead holder's lock is taken over;HYPERFRAMES_RENDER_LOCK=offdisables it. One lock covers a--batch;--workersis unchanged.26c0158,7da300c):font_family_without_font_facenow reads thefontshorthand too (the shorthand parser moved to@hyperframes/parsersso renderer and lint share it), findings carry file:line, and a newfont_face_local_onlywarning flags an@font-facewhose only source islocal(). The old hint recommendinglocal()is gone from lint,docs/reference/lint-rules.mdxandskills/hyperframes/references/format.md. When a cloud render from a local project fails on fonts, the CLI lists each refused family with the project files that name it.Behaviour change: shorthand-only projects that passed lint now get the font finding.
Test plan
riskFrames.test.ts› gives up when a chunk never answers (fake timers); fails without the fixcaptureStreamingStage.test.ts› a wedge outside the capture call trips at 60 s / 300 s (fake timers); fails without the fixsrc/utils/renderLock.test.ts(7 tests:--no-wait, wait then start, stale takeover, abandoned takeover, opt-out, messages); smoke: two concurrent--workers 1renders show the wait and the--no-waitrefusalfonts.test.tsshorthand andfont_face_local_onlycases (3 fail without the change);src/cloud/fontFailure.test.ts🤖 Generated with Claude Code