[kkroo]

Make browser ROUTE bootstrap use the public delivery-service authority instead of deployment-owned presentation metadata. The library discovers /slt/ and the selected SLS service, reads its TO-generated SvcInetUrl for DASH or HLS, and keeps AMT/native DIMT and per-rendition S-TSID discovery in the browser.

The parser rejects cross-origin manifest URLs and does not expose fixed multicast groups or DS-specific paths. Tests cover generic DASH, HLS, DS-origin discovery, malformed/cross-origin URLs, AMT, native DIMT, and browser RaptorQ recovery.

Validation: npm test -- --test-reporter=spec; 38 tests passed.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex (applied directly; nested CLI not launched in the k8s runtime).
Reviewed head: d8cb72e

Looks good. SvcInetUrl discovery is origin-pinned and https-only, /slt/ group parsing no longer trusts arbitrary hrefs (same-origin, numeric, no query/hash), and the Shaka Fetch-plugin patch only replaces a snapshot that is identical to the page's fetch and restores it only if still ours.

Critical Issues (0)

Important Issues (0)

Suggestions (3)

• [native-codex] packages/mabr-route-proxy/src/route.ts (discoverSlsEndpoint, sltUrl/dsUrl origin check) — new URL(sltRaw) / new URL(dsRaw) are parsed without the location.href base used two lines later, so a relative sltUrl (still accepted when given alone) throws a bare TypeError: Invalid URL once dsUrl is also set.
  • Resolve both against the same base before comparing origins.
• [pr-review-toolkit:code] packages/mabr-route-proxy/src/route.ts (unique dedupe) — the dedupe key is source/group/port only, so if two SLT groups advertise the same SLS leg with different SvcInetUrls, the returned manifestUrl is whichever document resolved last, silently.
  • Either include manifestUrl in the key (so the ambiguity surfaces as the existing "select exactly one" error) or prefer the entry that carries a manifest.
• [gstack/review] packages/mabr-route-proxy/src/route.ts (advertisedManifestUrl) — the https: requirement means an http DS origin (dev/test, e.g. the 127.0.0.1 e2e server) always yields manifestUrl: undefined with no hint why; the README example then throws "did not advertise". Worth one sentence in the README or a debug warning.

Strengths

• Cross-origin SvcInetUrl rejection is tested, and a malformed optional URL cannot hide a valid SLS leg.
• Test fixtures drop the production DS hostname and real source IP in favour of ds.example.test / 192.0.2.10.
• e2e now exercises both the debug and the production compiled Shaka builds when present.

Recommended Action

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

[kkroo]

@ally please review exact head a6a882a4f9bad37ee4d0eaf7d9d899df3f5d13d2. It is d8cb72e rebased onto main 50eaccd: #26 squash-merged the two stacked commits, so only this PR's own commit remains, and its tree is identical to d8cb72e's (c57143e), which you approved at C0/I0.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex. Nested CLIs not launched in the k8s Job pod; lens prompts applied directly to the diff.
Reviewed head: a6a882a

The prior Ally review (d8cb72e) carried no Critical/Important findings, so there is no prior-finding ledger for this head. This head is a rebase over #26 plus the DS-discovery change.

Critical Issues (0)

Important Issues (1)

• [pr-review-toolkit:code / native-codex] packages/mabr-route-proxy/README.md:35 (and :69) — the documented integration loads a manifest URL that is outside the proxy's cache scope, so no ROUTE object can ever be served.
  • The example sets baseUrl = 'https://player.example/mabr/' (line 14) and then calls shakaPlayer.load(toSls.manifestUrl). advertisedManifestUrl (src/route.ts:224) only ever returns a URL on the DS origin (https://live.example/live/manifest.mpd). RouteProxy keys every cached object as new URL(contentLocation, baseUrl) and refuses anything outside new URL('.', baseUrl) (src/proxy.ts:90-101, lookup at :147). So the manifest and every segment it references resolve under live.example, never under player.example/mabr/ — every request misses and goes to unicast. Playback works, which is what makes it dangerous: the multicast path silently does nothing and nothing reports it.
  • Nothing in the code reconciles the two either: manifestUrl is returned on RouteEndpoint but RouteMabrSession never compares it with baseUrl.
  • Fix one of: (a) document/derive the relationship — the ROUTE Content-Location must resolve, against baseUrl, to the same URL as SvcInetUrl, and the example's baseUrl must be chosen accordingly (note baseUrl may not be the origin root per the scope rule, which constrains where the DS can publish SvcInetUrl); or (b) have the session reject/warn when an slsEndpoint.manifestUrl is not under the cache scope, so the misconfiguration is loud. A test that loads the discovered manifestUrl through proxy.fetch and asserts a cache hit would pin it.

Suggestions (3)

• [gstack/review] packages/mabr-route-proxy/src/route.ts:284 — the same-origin check uses new URL(sltRaw) / new URL(dsRaw) without a base, while the next line resolves against globalThis.location. A relative sltUrl (otherwise supported) plus dsUrl throws a TypeError: Invalid URL instead of the intended RangeError. Resolve both against the same base before comparing.
• [pr-review-toolkit:errors] packages/mabr-route-proxy/src/route.ts:216,224 — SvcInetUrl is accepted only with an .mpd/.m3u8 suffix and only over https:. Extensionless manifest endpoints and http: DS origins (local/staging) silently yield manifestUrl: undefined, which callers will read as "DS advertised nothing". Consider a short comment documenting both filters, or surfacing why a candidate was dropped.
• [pr-review-toolkit:code] packages/mabr-route-proxy/README.md:67 — the second example passes sltUrl to the session and calls discoverSlsEndpoint separately, so the SLT listing and group documents are fetched twice. Call discoverSlsEndpoint once and pass slsEndpoint: sls, as the first example does.

Strengths

• sltGroups replaces the bare-digit href regex with origin- and path-scoped URL resolution, so a listing can no longer steer discovery to another origin, and trailing-slash group links (7/) now work.
• SvcInetUrl is constrained to the DS's own HTTPS origin, with a test (does not return a cross-origin SvcInetUrl) covering the redirect case; a malformed URL does not hide a valid SLS leg.
• Test fixtures dropped real production hostnames/IPs for example.test / RFC 5737 addresses.

Recommended Action

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

[kkroo]

@ally please review exact head cf5677bffc58aa2c9fda8a73cb8397143d507cd9. This follow-up adds DS-relative URL resolution, manifest-aware SLS deduplication, and cache-scope validation; package tests, typecheck, and real route.wasm Shaka DASH/HLS E2E with RaptorQ recovery pass.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex. Nested CLIs not launched in the k8s Job pod; lens prompts applied directly to the diff.
Reviewed head: cf5677b

Looks good. The one new commit (fix(mabr): validate DS manifest cache scope) closes the prior Important finding and also picks up all three prior suggestions.

Prior Findings Dispositioned (1)

• prior:a6a882a important 1 — fixed — packages/mabr-route-proxy/src/route.ts:827 and :916-926 — start() now runs #assertManifestScope on every endpoint (discovered or passed as slsEndpoint) before any subscribe. It uses the same scope rule as RouteProxy (new URL('.', baseUrl) + startsWith, src/proxy.ts:91,101), so an out-of-scope manifestUrl now throws a RangeError that names both URLs. The README derives baseUrl from new URL('.', manifestUrl) (README.md:19, :63). The test fails loudly when the discovered manifest is outside the cache scope pins it, and its gateway throws on subscribe to prove the check runs first.

Critical Issues (0)

Important Issues (0)

Suggestions (1)

• [pr-review-toolkit:code] packages/mabr-route-proxy/src/route.ts:317-320 — the dedupe key now includes manifestUrl. Two SLT groups advertising the same SLS leg, one with a SvcInetUrl and one without, now count as 2 endpoints and throw "select exactly one". That is stricter than needed in the benign case. If that topology can happen, prefer the entry that carries a manifest before counting. Otherwise ignore this.

Strengths

• Relative sltUrl/dsUrl are resolved against one page base before the origin comparison, with a test (resolves relative discovery URLs before comparing their origins).
• Conflicting manifests for one leg now come up as an error, not last-write-wins, and there is a test for it.
• PresentationManifest (Blockcast SLT extension) shares the existing https/same-origin guard, so the new element cannot steer the page off the DS origin.
• The README's second example now fetches discovery once (slsEndpoint: sls) and documents that HTTP discovery yields no manifestUrl.

Recommended Action

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