fix(lint): don't flag unresolved templating-token asset srcs as missing - #2893

Clean, well-scoped fix. Central shape-based predicate in @hyperframes/parsers/asset-resolution layered next to the existing __UPPER__ skip at each site — matches the convention that's already there, avoids HF-specific coupling in the linter, and the regexes are anchored on delimiter-negation so they don't backtrack. The main lift is a sibling-callsite audit gap where the same skip pattern lives in other files that this PR doesn't extend.

Concerns

• Sibling-callsite miss — the __UPPER__ skip that this PR is layering next to is copy-pasted at three other sites the PR does not touch, and one of them is another pre-substitution lint with the exact same intent:

  • packages/lint/src/hevcPreviewLint.ts:91 (HEVC preview lint, if (/^__[A-Z_]+__$/.test(src)) continue;) — pre-substitution and video-src-shaped, so a <video src="<<tts_x>>"> slips past the __UPPER__ skip, gets treated as a candidate local video, and runs through the codec/existence path. Same class of false positive as audio_src_not_found. Suggest extending the hasUnresolvedTemplatingToken skip here too — it's the highest-value miss.
  • packages/studio-server/src/helpers/mediaCodecMap.ts:233 — codec detection, same skip shape. Lower priority (runtime is typically post-substitution) but will drift over time as the two skip lists diverge.
  • packages/producer/src/services/htmlCompiler.ts:171 — the comment there literally says "matches lint's skip". That's now a lie by omission — the lint skip is a superset. Even if the producer path is always post-substitution in practice, the drift is worth closing.

  Layered opportunity: since the two-line prelude (__UPPER__ + templating) now shows up at all four patched sites in project.ts and the same first line at all three unpatched sites, a shared isNonResolvableAssetSrc(src) in assetResolution.ts that folds both checks (plus optionally the remote/inline check) would let every caller stay in lockstep by construction. Fine to defer, but the drift risk is real once there's a third shape to skip.

• Test coverage only exercises one of the four patched sites. The new test file (project.test.ts:427-455) drives audio_src_not_found for the three token shapes, which correctly proves the shared predicate + one wire-through works. But the other three lint sites patched by this diff — lintMissingLocalAsset (video/img/source), lintTextureMaskAssetNotFound (CSS url()), and lintMissingOrEmptySubComposition (data-composition-src) — each have their own inline if (hasUnresolvedTemplatingToken(src)) continue; and no dedicated regression test. If someone reorders or removes one of those skips in a future refactor, the helper's own tests wouldn't catch it. Cheap to close: reuse hasAudioSrcNotFound's shape with <video> / CSS-url() / <div data-composition-src> variants.

Nits

• hasUnresolvedTemplatingToken docstring at packages/parsers/src/assetResolution.ts:16-24: "The legacy __UPPER__ placeholder shape is matched separately at each call site." — that's still true, but the new templating check is also inlined at each call site, so the sentence reads like it's justifying a different design than what actually landed. Either drop the aside or clarify it's talking about the historical shape only.
• Regex intent-lock tests: worth pinning empty / whitespace-only tokens (<<>>, {{ }}, ${}) and mismatched delimiters (<<x unclosed, {{x)}, ${x} inside a longer URL like audio/${name}.mp3). Today's regex behavior on those is defensible either way; a couple of assertions lock in what the team wants.
• Predicate name could be tighter — containsTemplatingToken(url) reads as a shape check without the "unresolved" narrative baked in. Minor; feel free to skip.

Questions

• Is <<...>> the only shape observed in prod today, with {{...}} and ${...} added preemptively for future consumers, or are there existing pipelines (astral or otherwise) already emitting the other two? Purely-curiosity-level — the shape-agnostic predicate is the right call either way; the answer would just clarify how load-bearing the extra branches are.
• Any plan (this PR or follow-up) to extend the skip to the three sibling sites above, or is the deliberate scope to fix only what the observed false positive touched?

What I didn't verify

• I did not run the @hyperframes/lint suite locally against the astral session that Somansh referenced (32422624-...).
• I did not check whether any downstream consumer relies on the current false-positive behavior (e.g. a test that expects audio_src_not_found on <<foo>>). Extremely unlikely given the framing, but worth a grep before merge.

— Review by Rames D Jusso

R1 review @ 7e58f4d

Verdict: APPROVE — clean shared-helper skip covers all four sibling sites; regex is safe; test proves under- and over-match. Ship.
Grade: A Rubric: CORRECT

Approach. Introduces a shared hasUnresolvedTemplatingToken(url) predicate in packages/parsers/src/assetResolution.ts matching <<...>> / {{...}} / ${...} shapes, then inserts one skip line alongside the existing ^__[A-Z_]+__$ guards at each project-lint call site (lintAudioSrcNotFound, lintMissingLocalAsset, lintTextureMaskAssetNotFound, lintMissingOrEmptySubComposition). Adds an audio integration test that asserts the three token shapes are skipped while a genuinely missing file still fires.

Findings.

• P1: none.
• P2: none.
• Non-blocker: packages/lint/src/hevcPreviewLint.ts:91 — collectLocalVideoCandidates carries the same /^__[A-Z_]+__$/ skip pattern but was NOT patched. In practice this is effect-equivalent because line ~93 (resolveExistingLocalAsset(...) → null → continue) already swallows unresolved tokens, so no false HEVC finding leaks. But it breaks the "every __UPPER__ skip pairs with a hasUnresolvedTemplatingToken skip" invariant Somansh established — worth a follow-up line for readability parity.
• Non-blocker: No direct unit test for hasUnresolvedTemplatingToken in packages/parsers/src/assetResolution.test.ts (that file currently only exercises maskNonScannableRanges). Coverage is via the audio integration test, which is fine, but a direct predicate test would cheaply lock in the shape (e.g. rejects <<>>, ${}, and bare ${a without closing brace; accepts embedded tokens; interior < doesn't create a nested match).

Test coverage. Positive (3 shapes skipped) and negative (audio/missing.mp3 still flags) are both present at packages/lint/src/project.test.ts:427-454 — the correct minimum. Regex /<<[^<>]+>>|\{\{[^{}]+\}\}|\$\{[^{}]+\}/ uses negated character classes (linear-time, no ReDoS risk) and correctly matches without anchoring so an embedded token in an otherwise-file-shaped URL (foo.mp3?v=<<x>>) also skips — intended behavior (a URL with any unresolved template is not statically resolvable). Full lint suite green (507) per PR body; CI Test/Typecheck in-progress but Producer unit + integration + Windows tests + SDK smoke all SUCCESS.

Verdict rationale. Diff faithfully matches the PR body's "same shape as the recent __UPPER__ prune" claim — all four project-lint sites patched in the same one-line-add form. The alternative astral-side fix would just relocate the problem: HF's static linter needs to model "not resolvable yet" cleanly, which is exactly what this predicate does. Non-blockers are quality nits, not correctness gaps.

— Via

One correctness blocker remains in two of the four newly guarded paths.

At packages/lint/src/project.ts:329-333 and :381-384, cleanAssetUrl() runs before hasUnresolvedTemplatingToken(). cleanAssetUrl() splits on ? and #, including delimiters inside a ${...} expression. For example:

• ${asset?.url} becomes ${asset
• ${assetUrl ?? fallback} becomes ${assetUrl

The new predicate returns true on each raw value and false after cleaning, so <img>/<video>/<source> and CSS mask-image: url(...) still emit the missing-asset false positive for valid unresolved ${...} templates. The audio and data-composition-src paths do not pre-clean, which is why the current audio-only regression test cannot catch this.

Please check the raw src / URL for templating tokens before stripping query and fragment suffixes (or make cleaning placeholder-aware), and add regression coverage through both affected lint paths. This is additive to Rames's broader call-site/test audit.

Verdict: REQUEST CHANGES

Reasoning: The advertised generic ${...} skip is incomplete in two production lint paths because preprocessing destroys common optional-chaining/nullish-coalescing expressions before the new predicate sees them.

— Magi

Re-review at exact head 8a9b5749.

The prior blocker is closed: packages/lint/src/project.ts:329 and :381 now test the raw value before cleanAssetUrl(), preserving valid ${asset?.url} and ${a ?? b} expressions. packages/lint/src/project.test.ts:456-508 pins both affected paths plus a real missing-file negative control. The same raw-before-cleaning order is now present in the HEVC candidate collector at packages/lint/src/hevcPreviewLint.ts:89-92, addressing the highest-value sibling from Rames review.

All 43 exact-head checks are terminal: 33 success, 10 intentionally skipped, no failures or pending jobs. The broader cross-package consolidation is correctly isolated in stacked follow-up #2894.

Verdict: APPROVE
Reasoning: The advertised templating-token skip now survives URL preprocessing across every changed lint path, with regression coverage for the exact optional-chaining/nullish-coalescing failure mode.

— Magi

R2 review @ 8a9b5749

Delta from R1 (7e58f4de → 8a9b5749, +64/-1) is on-target:

• Miguel's blocker (cleanAssetUrl splits inside ${asset?.url}) fully addressed. The raw-value check now runs before cleanAssetUrl at every visual-src site that goes through it: project.ts:326-328 (video/img/source), project.ts:379-381 (CSS url()), and hevcPreviewLint.ts:88-90. lintAudioSrcNotFound and lintMissingOrEmptySubComposition already used the raw match[1] directly so no reorder needed there — verified. Comment on each pre-clean site is explicit about why the reorder matters.
• R1 P1 (sibling-callsite miss) — the pre-substitution one is closed. hevcPreviewLint.ts now imports and uses hasUnresolvedTemplatingToken — the highest-value site I flagged. The remaining two (mediaCodecMap.ts, htmlCompiler.ts) are deferred to the stacked #2894, which is the correct place for them since they're post-substitution paths and the follow-up is what folds the whole family through one helper.
• R1 P2 (test coverage) — substantively closed. The new describe("templating tokens are checked on the raw src, before cleanAssetUrl", ...) block exercises <video>, <img>, and CSS url() with the exact tricky cases (${asset?.url}, ${a ?? b}, ${u}?v=1, <<video_x>>, {{ videoUrl }}), plus a genuinely-missing-video regression. The tricky-case coverage is stronger than what I asked for in R1.
• R1 docstring nit closed. The hasUnresolvedTemplatingToken docstring in assetResolution.ts:16-24 now correctly explains the raw-value requirement and drops the awkward "matched separately at each call site" aside.

Vance's earlier APPROVE (at 7e58f4de) predates Miguel's ?-split concern, so that stamp doesn't cover R2. Miguel's CHANGES_REQUESTED is what's blocking merge — that's for him to clear after re-reviewing.

Nit

• lintMissingOrEmptySubComposition at project.ts:531 still has no dedicated regression test for a <div data-composition-src="{{ token }}"> shape. Post-#2894 it routes through the well-tested isUnresolvedAssetPlaceholder, so this is minor — flagging only because the other three lint sites got new coverage in this round.

What I didn't verify

• Full lint suite run locally (trusting the 507 → 511 test count in the PR body).
• Behavior against the specific astral session token shapes in prod — R1 covered that.

LGTM from where I sit; deferring to Miguel to re-stamp on the ?-split fix.

— Review by Rames D Jusso

R2 verify @ 8a9b574

Verdict: APPROVE — correctness bar cleared; Magi's cleanAssetUrl-truncation P1 is fixed at all 4 lint sites with explicit regression coverage for ${asset?.url}.

Prior findings status:

• Magi P1 (cleanAssetUrl truncation): FIXED — predicate now runs on the RAW src before any ?/# split at every lint site: packages/lint/src/hevcPreviewLint.ts:91 (before cleanAssetUrl(rawSrc) at :93), packages/lint/src/project.ts:287 (lintAudioSrcNotFound, raw match[1]), packages/lint/src/project.ts:329 (lintMissingLocalAsset, before cleanAssetUrl(rawSrc) at :331), packages/lint/src/project.ts:382 (lintTextureMaskAssetNotFound, before cleanAssetUrl(rawUrl) at :384), packages/lint/src/project.ts:535 (lintMissingOrEmptySubComposition, trimmed raw — no cleanAssetUrl in this path).
• Magi regression tests (${asset?.url} case): COVERED — packages/lint/src/project.test.ts:472 defines TRICKY = ["${asset?.url}", "${a ?? b}", "${u}?v=1", "<<video_x>>", "{{ videoUrl }}"] and runs it against <video> (:474), <img> (:485), and CSS mask-image: url(...) (:494). The ?-inside-${…} truncation is directly exercised.
• Rames P1 (3 sibling sites): PARTIAL — 1 of 3 (hevcPreviewLint.ts:91) is fixed on this PR. The 2 outside packages/lint (mediaCodecMap.ts:233, htmlCompiler.ts:171) remain unpatched here BUT are addressed by the stacked #2894 which folds a shared isUnresolvedAssetPlaceholder helper and hits all three. Neither of the unpatched sites emits a spurious lint finding today (mediaCodecMap drops non-resolvable via !resolved; htmlCompiler runs post-substitution) — so consistency gap only, not a live FP path. Closed by the #2893 + #2894 stack.
• Rames P2 (test coverage 3 other patched sites): PARTIAL — 3 of 4 patched sites have dedicated coverage (audio + lintMissingLocalAsset + lintTextureMaskAssetNotFound via the two new describe blocks). lintMissingOrEmptySubComposition at project.ts:535 gained the skip but no new test asserting a <div data-composition-src="${sceneSrc}"> doesn't fire missing_or_empty_sub_composition. Follow-up nit.

New findings (adversarial pass): none. The hasUnresolvedTemplatingToken regex /<<[^<>]+>>|\{\{[^{}]+\}\}|\$\{[^{}]+\}/ is tight (negated char classes prevent ReDoS and cross-token bleed) and only skips statically-unresolvable values. No legitimate asset filename would start with <<, {{, or ${ in practice. Ordering is safe at every patched site.

CI: all required checks green. mergeStateStatus: BLOCKED is stale — Magi's earlier CHANGES_REQUESTED was resolved by his re-approval at this SHA.

— Via