fix(producer): decode percent-encoded video src in HDR pre-extract

[vanceingalls]

Fixes symptom (c) of PRINFRA-349 (non-ASCII / CJK media path breaks HDR render).

Root cause

HDR pre-extraction resolved a <video> source to a filesystem path by naively joining the raw src attribute:

let srcPath = v.src;
if (!srcPath.startsWith("/")) {
  srcPath = existsSync(join(compiledDir, srcPath)) ? join(compiledDir, srcPath) : join(projectDir, srcPath);
}

But v.src is the compiled DOM's URL attribute, so a source named 视频1.mp4 arrives percent-encoded as %E8%A7%86%E9%A2%911.mp4. That encoded string was passed straight to ffmpeg as a path → Error opening input file <project>/%E8%A7%86%E9%A2%911.mp4: No such file or directory. The SDR extractor never hit this because it resolves through resolveProjectRelativeSrc, which tries decodeUrlPathVariants against the filesystem; the HDR planner had its own hand-rolled join that skipped the decode.

Fix

planHdrResources now resolves via the shared resolveProjectRelativeSrc — the exact resolver the SDR path uses — so the encoded src decodes back to the real on-disk filename (and query strings, origin-root URLs, and .. traversal now resolve identically across the two paths). Removed the redundant injected existsSync param.

Test

• New planHdrResources tests: a percent-encoded CJK src resolves to the decoded on-disk path; an ASCII src is unchanged. (captureHdrResources.test.ts, 5/5)
• tsc / oxlint / oxfmt clean.

Scope note

The ticket aggregates three symptoms under "one root cause," but (c) is independent of (a)/(b): it's a cross-platform URL-vs-filesystem decode bug in the producer (reproduces on macOS, per the report), not Windows-native path handling. (a) Windows init access-violation and (b) Windows init EIO are a separate Windows-native theme — see the ticket comment for triage; they need a Windows repro to fix safely and aren't touched here.

Related: PRINFRA-349

🤖 Generated with Claude Code

[miga-heygen]

SSOT — duplicated path resolution eliminated

Textbook SSOT fix. The HDR planner had a hand-rolled path join that duplicated the decision resolveProjectRelativeSrc already owns — same structure (compiledDir → projectDir fallback) but missing the percent-decode pass. The fix deletes the duplicate and delegates to the canonical resolver.

Verified

• resolveProjectRelativeSrc signature: (src, baseDir, compiledDir?) — matches the call site (v.src, projectDir, compiledDir). ✓
• Absolute path handling: the shared resolver preserves absolute paths that exist on disk, treats non-existent absolute paths as origin-root URLs. The old code's startsWith("/") passthrough was less robust (passed through even for non-existent absolute paths). Behavioral improvement, not regression. ✓
• Fallback on missing file: join(baseDir, cleanSrc) — same as old behavior minus query strings (which the old code also didn't strip, so this is strictly better). ✓
• existsSync removal: still used at line 328 in captureHdrStage.ts — no dead import. ✓
• Tests use real filesystem (mkdtempSync + writeFileSync), not mocks. ✓

CI

Producer: integration tests was cancelled (not failed) — may need a re-run before merge. Unit tests passed.

Clean fix, good tests, SSOT improvement. Ship it.

— Miga

[james-russo-rames-d-jusso]

Reviewed at 0fcf792.

Correctly shaped fix — replaces the divergent hand-rolled join with the shared resolveProjectRelativeSrc, which is exactly the SDR-path resolver whose absence caused the HDR-only regression. Test uses a real CJK literal ("视频1.mp4") with a real tmpdir round-trip, not a %E4%B8%AD proxy, so the encode→decode→existsSync chain is exercised end-to-end. existsSync param removed cleanly from both planHdrResources and the runCaptureHdrStage call site. Root-cause explanation in the PR body is accurate — I traced decodeUrlPathVariants (returns [decoded, original]), resolveProjectRelativeSrc's candidate loop (compiledDir → baseDir, plus browser-style .. clamp), and confirmed the shape matches the video-probe callsite at extractVideosStage.ts:148-150 that this fix now brings HDR into parity with.

One concern — peer image-probe divergence in the same file:

packages/producer/src/services/render/stages/extractVideosStage.ts:173-179 still has the exact pre-fix hand-rolled pattern for the HDR image probe:

let imgPath = img.src;
if (!imgPath.startsWith("/")) {
  const fromCompiled = existsSync(join(compiledDir, imgPath))
    ? join(compiledDir, imgPath)
    : join(projectDir, imgPath);
  imgPath = fromCompiled;
}

Same bug reproduces for HDR image sources — a 16-bit PNG tagged BT.2020 PQ/HLG named 图1.png compiles to %E5%9B%BE1.png in the DOM URL attribute, join(projectDir, encoded) misses, existsSync(imgPath) at line 180 returns false → return null → the image never enters nativeHdrImageIds → HDR pipeline sees no HDR sources → composition renders through the SDR fallback with silently wrong color (the customer sees a rendered output, no error, wrong appearance). That's arguably worse than the video symptom, which at least errored out audibly at ffmpeg.

The video probe two blocks above (lines 148-150) already uses isAbsolute(v.src) ? v.src : resolveProjectRelativeSrc(v.src, projectDir, compiledDir) — copying that exact shape to the image loop is ~4 lines. Would strongly recommend either bundling it into this PR (natural cohesive fix, matches the "one root cause" framing) or filing a same-day follow-up before PRINFRA-349 symptom (c) is marked closed — otherwise the ticket claims a fix that doesn't cover HDR image compositions.

Nits:

• NFC/NFD coverage: test uses the NFC-normalized literal 视频1.mp4 on Linux CI where filesystem stores raw UTF-8 bytes — round-trip works because JS strings are already NFC. Won't catch NFC-vs-NFD divergence (macOS HFS+ normalizes to NFD on write, APFS doesn't). Not a production concern for the Linux-based render pipeline, but if user-machine renders on macOS ever land the same code path, the shared resolver's decodeURIComponent produces NFC while a stored NFD filename would miss the existsSync check. Worth documenting as an assumption; a normalize("NFC") / normalize("NFD") variant in decodeUrlPathVariants would harden it if that scope ever expands (would live in packages/parsers/src/utils/urlPath.ts, not this PR).

• CANCELLED integration test: the "Producer: integration tests" job in CI came back cancelled after 20m — worth confirming that's a matrix-cascade cancel from another failing check, not this fix hitting a real timeout in an integration path. All other CI (Producer unit, Windows render, all 8 regression shards including hdr-regression and hdr-hlg-regression, Windows tests, CodeQL, typecheck) is green.

Scope note is well-drawn: honestly separating symptom (c) — this cross-platform URL-decode bug — from symptoms (a)/(b) — Windows-native init issues needing Windows repro — is the right shape. The PR fixes what it claims and doesn't overreach.

What I didn't verify:

• Whether img.src in production carries file:// prefix (would break both old and new code) vs relative path (works with the shared resolver). Same question applies to v.src for HDR — the test assumes relative path, which matches Vance's reproduction report, but the composition-metadata build path could be a hidden divergence.
• Behavior when resolveProjectRelativeSrc hits the total-miss fallback (line 63 in videoFrameExtractor.ts) — it returns join(baseDir, cleanSrc) with the encoded string, so the ffmpeg error path still shows the encoded name. Not a regression; inherited from the shared resolver.
• Cross-check with the audio path — packages/producer/src/services/audioExtractor.ts:276 has a similarly-shaped startsWith("/") && startsWith("http") guard that I didn't fully trace; if it also hand-rolls, CJK audio-track paths would be a peer symptom.

Stamp routing per convention. Leaving as COMMENTED.

— Review by Rames D Jusso

[miguel-heygen]

Exact head 0fcf7928fe9096413b85a417a87a3562fbd2c844. The video-path SSOT change itself is correct: planHdrResources now delegates to the same resolver as SDR, the CJK test uses a real on-disk filename, and the current-main resolver still preserves the intended compiled-dir/base-dir/decode ordering. The branch also merges conflict-free with current main.

Blocker — the same HDR media-path contract is still violated for images. As Rames D Jusso noted, packages/producer/src/services/render/stages/extractVideosStage.ts:342-348 still resolves img.src with the old hand-rolled startsWith("/") + existsSync(join(...)) pattern. A compiled HDR image named 图1.png arrives percent-encoded, misses both filesystem joins, and silently fails HDR detection/materialization. This is the same precondition and failure class as the video bug, inside the same HDR pipeline; per the shared-boundary audit it should be fixed here, not left as a follow-up. Route that image path through resolveProjectRelativeSrc(img.src, projectDir, compiledDir) and add the real-filesystem percent-encoded HDR-image regression alongside the video case.

CI also is not currently stampable: required Producer: integration tests is cancelled. The log shows it hit the job’s ~20-minute limit while apt-get install ffmpeg was still downloading (the tests never started), so this is infrastructure rather than a code failure, but the required lane needs a successful rerun before merge.

Verdict: REQUEST CHANGES
Reasoning: The canonical resolver fix is right but leaves a sibling HDR source type with the identical CJK failure, and one required lane has no test result.

— Magi

[vanceingalls]

Good catch on the image probe — fixed in c8f7542, and chasing it turned up a deeper case worth flagging.

Your finding: the HDR image probe still hand-rolled the join. Fixed — it now goes through the shared resolver, so 图1.png → %E5%9B%BE1.png resolves and the image reaches nativeHdrImageIds instead of silently rendering SDR. Agreed that the silent-wrong-color symptom is worse than the video path's audible ffmpeg error.

The deeper case: my first pass extracted a resolveProbeSrc(src, …) = isAbsolute(src) ? src : resolveProjectRelativeSrc(…) helper, copying the video probe's existing shape. A review pass caught that this does not actually fix root-relative srcs: isAbsolute("/assets/%E5%9B%BE1.png") is true on POSIX, so the short-circuit returns it undecoded, existsSync fails, and we are back to the same silent SDR render. resolveProjectRelativeSrc already handles absolute paths correctly on its own (absolute-and-exists → return; otherwise treat a leading slash as a browser origin-root URL, which it has a dedicated test for). So the isAbsolute pre-check was both redundant and harmful.

Final shape: helper deleted, both probes call resolveProjectRelativeSrc directly — byte-identical in approach to planHdrResources in the merged half, so the two can no longer drift.

One more thing the widening exposed: resolving more srcs means files that previously never resolved are now reachable, including truncated / 0-byte assets where ffprobe exits non-zero. Both probes run inside a bare Promise.all, so an unguarded extractMediaMetadata throw would abort the entire render over one unreadable image. Added probeColorSpaceSafely() — logs a warning and treats the source as SDR.

Tests: four cases (percent-encoded CJK, origin-root percent-encoded CJK, compiledDir-over-projectDir precedence, existing-absolute passthrough), with distinct projectDir/compiledDir so the precedence is genuinely pinned. Fault-injection verified — reintroducing the isAbsolute short-circuit fails the origin-root test.

Known gap, not fixed: the image-probe call site still has no regression coverage — runExtractVideosStage is not imported by any test in the repo, so reverting the call site would not fail anything. Covering it needs a full job config + compiled composition + ffprobe fixtures. Flagging rather than faking it; happy to file a follow-up if you want that harness.

[james-russo-rames-d-jusso]

R1 concerns from both reviews resolved. The image probe now delegates to resolveProjectRelativeSrc (same shared resolver as the video probe), and the video probe drops its isAbsolute pre-check so root-relative CJK URLs like /assets/%E5%9B%BE1.png reach the decode step instead of being handed back verbatim.

Verified

• extractVideosStage.ts:205 — image probe now uses resolveProjectRelativeSrc(img.src, projectDir, compiledDir), mirroring :178. Both HDR probes are on the same code path; PRINFRA-349 symptom (c) closes with the same shape as (a)/(b).
• extractVideosStage.ts:168-178 — the isAbsolute pre-check drop on the video probe is intentional and load-bearing (comment spells out why: root-relative CJK URLs are POSIX-isAbsolute:true and would otherwise round-trip percent-encoded).
• Tests at extractVideosStage.test.ts:96-152 pin four contract shapes (percent-encoded relative, root-relative browser origin URL, compiledDir precedence, absolute unchanged). Real-filesystem, no mocks — matches the R1 test style.
• New probeColorSpaceSafely wrapper (extractVideosStage.ts:124-135) is a legitimate defensive move: the widened resolver now finds files that previously silently failed to resolve — including truncated / 0-byte assets on which extractMediaMetadata throws — and both probes run inside a bare Promise.all where an unguarded throw would abort the whole render. Log-and-treat-as-SDR is the right behavior for one unreadable file.

Same family, out of scope for this PR

• audioExtractor.ts:275-277 has the same hand-rolled shape for <audio> srcs: if (!srcPath.startsWith("/") && !srcPath.startsWith("http")) srcPath = join(projectDir, srcPath);. No percent-decode, no compiledDir, no Windows-isAbsolute. A CJK audio filename (音频1.mp3) served with a percent-encoded attribute reproduces the same silent-fail. Engine-side audioMixer.ts:552 already uses resolveProjectRelativeSrc — the producer-side extractor is the asymmetric one. Worth a same-day follow-up under the PRINFRA-349 umbrella; not a blocker for this PR since it's a distinct pipeline.

CI

• Required checks at c8f7542 show only WIP/Mintlify at HEAD. The producer integration / Windows render / regression suites don't appear to have re-run on the fix commit — either the fix push landed after the workflow trigger window or producer/* path filters didn't hit. Worth a manual re-run before merge; without it, the previous R1 CANCELLED producer-integration state carries forward untouched.

Fix itself LGTM from my side.

— Review by Rames D Jusso

[miguel-heygen]

Re-review at c8f7542b35f12397b95512ded445abd47fe1c6a1.

The R1 blocker is resolved at the mechanism. extractVideosStage.ts:178-180 and :205-207 now route both video and image sources through resolveProjectRelativeSrc without the harmful isAbsolute() short-circuit, so percent-encoded CJK and origin-root sources share the canonical resolver. probeColorSpaceSafely at :123-137 also prevents a newly-resolvable but unreadable asset from aborting the whole probe batch.

Important — the regression test still pins the helper, not the integration seam. extractVideosStage.test.ts:89-155 imports and calls resolveProjectRelativeSrc directly. It never invokes runExtractVideosStage, so reverting either production call site at extractVideosStage.ts:178 or :205 leaves all four new tests green. The author already called this out honestly; it is not a code blocker for this focused fix, but a follow-up integration test should pin that CJK image reaches nativeHdrImageIds / hdrImageSrcPaths.

Mechanical gate: GitHub reports this head as DIRTY / conflicting with current main, and the only head checks are WIP success plus Mintlify skipped. No Producer, typecheck, or regression result exists for c8f7542b. I cannot clear the prior change request until the branch is rebased/restacked and the normal CI suite runs green.

Verdict: COMMENT (Ready: No)
Reasoning: The HDR image-path blocker is fixed correctly, but the current head conflicts with main and has no code CI evidence; rebase and green CI are still required.

— Magi

[miguel-heygen]

Re-review at exact head 459dcdba9ee0c04b5997e269795e61b1574379db.

The prior blockers are cleared:

• Both HDR media probes now use the canonical resolveProjectRelativeSrc path. The video probe at extractVideosStage.ts:325 no longer short-circuits origin-root URLs through isAbsolute, and the image probe at :378 uses the same decode/query/traversal contract.
• The distributed HDR plan path is aligned at captureHdrResources.ts:86, so percent-encoded non-ASCII video sources resolve identically before ffmpeg extraction.
• probeImageColorSpaceSafely at extractVideosStage.ts:135 preserves the best-effort image-probe behavior after widening the resolver; one bad image no longer rejects the whole probe batch.
• The branch is mergeable and the current head is fully green, including Typecheck, Producer unit + integration, Windows render/tests, and all regression shards.

Non-blocking follow-ups remain as previously noted: the resolver tests exercise the real filesystem contract but do not drive the full runExtractVideosStage image seam, and audioExtractor.ts has the adjacent hand-rolled source-path shape. Neither reopens this PR's fixed video/image HDR path.

Verdict: APPROVE

Reasoning: The shared resolver is now the single source of truth at every modified HDR path, the previous merge/CI blocker is gone, and the exact head has comprehensive green validation.

— Magi