[kkroo]

Shaka default Fetch plugin captures window.fetch when Shaka loads. If an app loads Shaka before creating the ROUTE proxy, installFetch only patches page fetch and Shaka bypasses ROUTE cache. This updates the existing Shaka fetch snapshot only when it still equals the original page fetch, restores it safely, and leaves custom networking plugins untouched. Checks: npm run typecheck; npm test (34 passing); npm run test:e2e (real route.wasm + RaptorQ + Shaka DASH/HLS, native/AMT).

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 3404005

Critical Issues (0)

Important Issues (1)

• [native-codex] packages/mabr-route-proxy/src/proxy.ts:199 + packages/mabr-route-proxy/README.md:105 — the bridge does nothing with the production Shaka build, but the README now says it does.
  • Shaka's source marks shaka.net.HttpFetchPlugin.fetch_ as "Overridden in unit tests, but compiled out in production" (lib/net/http_fetch_plugin.js:341-346, @const @private). I checked the latest npm dist/: shaka-player.compiled.debug.js has 15 fetch_ references (shaka.net.HttpFetchPlugin.fetch_=window.fetch). shaka-player.compiled.js has zero. There, the snapshot is a module-local var xJ=window.fetch, which nothing outside the module can reach. So with the production bundle, shakaFetchPlugin()?.fetch_ is undefined, the guard skips, and Shaka keeps going around the ROUTE cache. That's the exact failure this PR describes, with no warning.
  • The new README sentence ("updates Shaka's default Fetch plugin if Shaka was loaded first, so the normal Shaka networking path works without a player change") is therefore true only for debug/uncompiled builds. Either scope it to those, or say that with the compiled build installFetch() must run before Shaka's script is evaluated, or that the page must register a networking plugin that calls proxy.fetch. A console.warn when globalThis.shaka exists but HttpFetchPlugin.fetch_ doesn't would also turn this silent bypass into a visible one.

Suggestions (1)

• [pr-review-toolkit:tests] packages/mabr-route-proxy/tests/route-wasm-shaka-e2e.mjs:131 — the e2e calls session.installFetch() (line 131) before it injects Shaka (line 261), and it loads compiled.debug.js. So the "Shaka loaded first" path is only covered by the unit stub, and the production-build case is never run. One e2e variant that loads Shaka first, against shaka-player.compiled.js, would have caught the finding above.

Strengths

• Only touches the snapshot when it still equals the original page fetch, so custom networking plugins are left alone.
• Restore is guarded with plugin.fetch_ === patched, so it won't overwrite a later replacement.
• The unit test checks both the patched response and the full restore.

Recommended Action

1. Address Important issues this cycle.
2. Consider Suggestions opportunistically.

[kkroo]

@allyblockcast Please re-review the current head 47495da. Addressed the compiled-Shaka finding: production bundles are documented as requiring installFetch before Shaka evaluation, a warning is emitted when the snapshot is unreachable, and the E2E now exercises the available production build across native and AMT DASH/HLS playback.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 47495da

Looks good. The new commit fixes the one blocker from the last review.

Prior Findings Dispositioned (1)

• prior:3404005 important 1 — fixed — packages/mabr-route-proxy/src/proxy.ts:203 + packages/mabr-route-proxy/README.md:105 — The README is now limited to debug/uncompiled builds. It tells production users to call installFetch() before Shaka evaluates, or to route a custom networking plugin through proxy.fetch. proxy.ts:203-204 adds a console.warn for the case where shaka.net.HttpFetchPlugin exists but has no fetch_ snapshot, which is what the compiled build looks like. The silent bypass is now visible, and index.test.ts covers the warning.

Critical Issues (0)

Important Issues (0)

Suggestions (1)

• [pr-review-toolkit:tests] packages/mabr-route-proxy/tests/route-wasm-shaka-e2e.mjs:42 — The production Shaka variant is skipped without a message when shaka-player.experimental.js is missing. A CI run without that artifact still passes and only prints configured/* lines. A console.log saying the variant was skipped would make the gap visible. Small nit: the inner for (const ingress ...) body was not re-indented under the new outer loop.

Strengths

• The Shaka snapshot is patched only when it still equals the original page fetch, so custom fetch plugins are left alone.
• Restore only puts the snapshot back if it is still patched, so a later replacement is not overwritten.
• The e2e now runs a production-build variant through the supported install-before-Shaka path, alongside the debug build.

Recommended Action

1. No blocking changes requested.
2. Merge once the remaining required CI checks finish green.