fix(producer): resolve residual font and probe failures

[jrusso1020]

What

Fix two producer failures observed in Claude MCP production renders:

• Resolve URL-style Google Font family names such as DM+Mono, IBM+Plex+Mono, and Spline+Sans+Mono without changing the authored CSS alias.
• Ensure every browser probe uses the replacement screenshot session after a BeginFrame liveness fallback closes the original session.

Why

In the July 22 production sample, these two signatures accounted for 9 of 12 Claude MCP render_failed outcomes:

• 6 FONT_FETCH_FAILED failures for valid Google families authored with + separators.
• 3 Puppeteer detached-frame failures.

The font resolver encoded the authored + as a literal plus in the Google Fonts request, so valid families were treated as unresolved. Separately, probeStage captured a local session before its liveness fallback; after closing that browser and launching a healthy replacement, duration and media probes continued using the stale closed session.

How

• Convert + separators to spaces only when constructing the outbound Google Fonts family query. The emitted @font-face retains the authored family name so existing CSS continues to match.
• Bind the downstream probe session after the complete BeginFrame fallback block.
• Add table-driven coverage for all three production font families.
• Add a regression that forces the liveness fallback and verifies the original session is closed, the replacement uses screenshot mode, duration discovery receives the replacement, and the replacement is returned downstream.

Test plan

• Unit tests added/updated
• Producer focused tests: 48 passed
• Producer unit lane: 31 Vitest files / 379 tests passed, plus all Bun unit files
• Producer TypeScript typecheck
• oxlint and oxfmt checks
• Manual testing performed
• Documentation updated (not applicable)

[james-russo-rames-d-jusso]

Clean prod-hardening pair. Both fixes land at the right layer, and the diagnostic framing in the PR body (July 22 sample, 9 of 12 render_failed, signature breakdown) makes the scope and rationale unambiguous.

Font path — familyName.replace(/\+/g, " ") is the minimal correct change: the outbound family= Google Fonts query uses the canonical spelling, while familyName still flows into fontSlug (line 745) and every buildFontFaceRule/emitted @font-face (line 483/499), so authored CSS font-family: "DM+Mono" still matches the injected alias. fontSlug collapses + and space to the same - (dm-mono), so "DM+Mono" and "DM Mono" share a cache dir with no collision risk — the subset filenames are keyed by subsetToken(woff2Url). Table-driven tests cover the three concrete production families and lock in both "outbound uses the space-separated spelling" and "authored spelling survives in the emitted rule."

Probe path — moving const session = probeSession to after the BeginFrame liveness fallback is the fix; the pre-PR binding captured the closed session, and each of the five downstream call sites (getCompositionDuration, resolveCompositionDurations, discoverMediaFromBrowser, discoverAudioVolumeAutomationFromTimeline, discoverVideoVisibilityFromTimeline) then dereferenced the closed browser. Between the old and new binding lines every reference is to probeSession (not session), so the shift is safe. The new invariant check (if (!probeSession) throw) replaces a ! assertion with an explicit throw at the same guarantee — small resilience win. The regression test locks in session count = 2, the original is closed, the replacement is forceScreenshot, duration probe receives the replacement, and the returned probeSession is the replacement.

No blockers, no concerns worth surfacing. LGTM from my side.

— Review by Rames D Jusso

[vanceingalls]

Pray, allow me to take up the lantern where Rames left it — the diagnosis he lays out, I have retraced independently, and I find myself in complete concurrence with the mechanism he identifies at both fault lines.

On the matter of the Google Fonts family, the culprit was encodeURIComponent(familyName) applied to an authored spelling of DM+Mono, which the encoder faithfully rendered as family=DM%2BMono and dispatched to a catalogue that knows no such creature. The remedy — familyName.replace(/\+/g, " ") narrowed to the outbound query — is minimal, correct, and, I venture to observe, admirably restrained. The authored familyName continues to flow into fontSlug at line 745, the @font-face alias emitted at line 448, and every downstream match against the CSS the author actually wrote, so the browser's identifier lookup remains uninterrupted. I confirm independently what Rames noted about the cache directory: fontSlug("DM+Mono") and fontSlug("DM Mono") alike collapse to dm-mono under the [^a-z0-9]+ → - normalisation at line 604, and the subset filenames are keyed by subsetToken(woff2Url), so no two spellings can collide catastrophically. The table-driven regression, covering all three families the July 22 sample surfaced, exercises the outbound spelling and the preserved alias in the same assertion — it would have failed pre-fix, and is not the aspirational sort.

On the matter of the probe session, the pre-PR arrangement bound session before the BeginFrame liveness fallback might close and replace the browser — a lifetimes bug of the classical variety: five downstream operations (getCompositionDuration, resolveCompositionDurations, discoverMediaFromBrowser, discoverAudioVolumeAutomationFromTimeline, discoverVideoVisibilityFromTimeline) all dereferenced a corpse. The remedy — sliding the const session = probeSession binding to after the fallback — is the surgically correct one. I traced each reference between the old binding and the new: every intermediate access is against probeSession (the outer let), not against the stale local, so the shift preserves behaviour exactly for the healthy path and repairs it for the fallback path. The regression test locks in the four independent facts that together characterise the fix: two sessions created, the first closed, the second launched with forceScreenshot: true, the duration probe received the replacement, and the returned probeSession is likewise the replacement. Pre-fix this test would fail on the fourth assertion.

A small consolation, the probeSession! non-null assertion has been replaced by an explicit invariant throw — a modest resilience win, and one that clarifies to future readers that the retry loop's post-condition is being upheld deliberately rather than assumed.

One quiet observation, offered as commentary and not as a request. Should assertNotAborted() fire between the fallback's initializeSession(probeSession) and the downstream code, the freshly created replacement session would be leaked — but I hasten to note this is an existing pattern shared with the outer retry loop, not a regression introduced here. If a future pass ever unifies the session-cleanup story around a try/finally disposer, the fallback branch would benefit alongside the retry branch. Not for this PR.

CI: green across all thirty-odd required and shard jobs at 856fc50. Format, Lint, Typecheck, Producer unit + integration, Test, Windows render verification, all eight regression shards, and CodeQL all completed SUCCESS. Nothing on the merge is blocking beyond the required-reviewers gate itself.

No blockers. Indeed, no non-blocking concerns worth surfacing beyond the abort-race note above. This is a clean prod-hardening pair.

— Review by Via

[miguel-heygen]

Exact-head review at 856fc50. Additive to Rames and Via; I independently traced both contracts and found no further gaps.

• packages/producer/src/services/deterministicFonts.ts:745-754 normalizes plus separators only for the Google request while retaining the authored family for the emitted alias and cache key.
• packages/producer/src/services/render/stages/probeStage.ts:324-392 binds session after the BeginFrame fallback, so duration, unresolved-composition, media, audio, and video probes all use the replacement.
• The exact-head focused suite passes: 48 tests, 105 expectations. All required GitHub checks are green.

Verdict: APPROVE
Reasoning: Both fixes are narrowly scoped to the production failure mechanisms, preserve existing external behavior, and have regression coverage for the exact failing paths.

— Magi