Repository navigation
feat(mabr): export DS SLS discovery helper - #22
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Ally — Consolidated PR Review
Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex (nested CLI skipped in k8s runtime; applied directly).
Reviewed head: 4763404
Looks good. Small, surgical change: discoverSlsEndpoint is promoted to a public export with a narrowed Pick<RouteMabrOptions, 'sltUrl' | 'sltGroup' | 'serviceId'> parameter, and re-exported from the package index. The internal caller (route.ts:767, passing the full this.#options) remains structurally compatible, and RouteEndpoint is already exported as a type, so consumers can type the result.
Critical Issues (0)
Important Issues (0)
Suggestions (1)
- [pr-review-toolkit/tests]
packages/mabr-route-proxy/src/index.ts:11— the function is now public API, but nothing exercises it through the package entry point. A one-line test that importsdiscoverSlsEndpointfrom./index.tsand resolves a stubbed SLT would pin the export against an accidental removal.
Strengths
- The narrowed
Pick<>signature means player origins don't have to construct a fakegateway/wasmjust to resolve discovery. - The existing
credentials: 'omit'policy for DS fetches carries over unchanged to external callers, which matters more now that it's callable from arbitrary origins. - The doc comment correctly scopes the return value (SLS bootstrap/DRIAD source only; rendition groups still come from S-TSID).
Recommended Action
- No blocking changes requested.
- Merge once the remaining required CI checks finish green.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Export the existing DS-hostname HTTP LLS/SLS resolver so player origins can delegate discovery to @blockcast/mabr-route-proxy without copying XML parsing. The helper returns the TO-selected service SLS (S,G,port,source); rendition groups remain learned from S-TSID. Tests: mabr-route-proxy npm test and typecheck.