blocker: resolveRouteLocale() always does normalizeLocale(route.queryParamMap.get("lang")) and immediately setPreferredLocale(locale). When ?lang= is absent, normalizeLocale(null) resolves to default (en), so every host/player navigation resets persisted locale to en.
Regression risk: overrides user/chosen locale from wpp.locale, shell dataset locale, or browser locale.
Scope mismatch vs #241: requirement is to wire route-level i18n (not force-reset locale on routes without lang).
Concrete fix: only call setPreferredLocale(...) when lang query param is present and non-empty; otherwise keep existing preferred locale (e.g. via resolvePreferredLocale()) and return that locale in resolver output.
should-fix: add a regression test proving absence of ?lang does not overwrite existing preferred locale (e.g. preseed wpp.locale=da, run resolver without lang, expect locale da and no forced reset to en).
I did run the targeted spec (session-route-context.spec.ts) and it passes, but it currently misses the no-lang persistence case above.
Review summary (severity-tagged):
- **blocker**: `resolveRouteLocale()` always does `normalizeLocale(route.queryParamMap.get("lang"))` and immediately `setPreferredLocale(locale)`. When `?lang=` is absent, `normalizeLocale(null)` resolves to default (`en`), so every host/player navigation resets persisted locale to `en`.
- Regression risk: overrides user/chosen locale from `wpp.locale`, shell dataset locale, or browser locale.
- Scope mismatch vs #241: requirement is to wire route-level i18n (not force-reset locale on routes without `lang`).
- Concrete fix: only call `setPreferredLocale(...)` when `lang` query param is present and non-empty; otherwise keep existing preferred locale (e.g. via `resolvePreferredLocale()`) and return that locale in resolver output.
- **should-fix**: add a regression test proving absence of `?lang` does **not** overwrite existing preferred locale (e.g. preseed `wpp.locale=da`, run resolver without `lang`, expect locale `da` and no forced reset to `en`).
I did run the targeted spec (`session-route-context.spec.ts`) and it passes, but it currently misses the no-`lang` persistence case above.
Per guardrail, no new review/integrator trigger is sent until credential path is fixed and a new remote head SHA artifact is visible.
Scheduler blocker update (artifact-gate):
- Baseline head SHA before dev-fix run: `5538a918007e154951a16bf4b61717610ec79f0d`
- Dev-runner produced local fix commit: `4b2b21fe577f44f4229d243082fc8b117bfa340e`
- Relevant test passed: `frontend/angular npm test -- --run src/app/session-route-context.spec.ts` (6/6)
- Push failed: `Permission denied (publickey)` using DEV role
Status: **AUTH_PATH_MISMATCH** (API preflight green, git push path broken).
Per guardrail, no new review/integrator trigger is sent until credential path is fixed and a new remote head SHA artifact is visible.
Fixed review feedback ✅
- Guarded route-locale write: setPreferredLocale(...) now only runs when ?lang= is present and non-empty.
- Added regression test for no-lang path preserving existing preferred locale (wpp.locale=da).
new head SHA: 4b2b21fe577f44f4229d243082fc8b117bfa340e
commit SHA: 4b2b21fe577f44f4229d243082fc8b117bfa340e
Re-review complete for latest head SHA 4b2b21fe57.
Tidligere REQUEST_CHANGES er adresseret:
locale skrives nu kun når ?lang er til stede
no-lang regression test tilføjet for bevarelse af persisted locale
Valideret via targeted test:
frontend/angular: npm test -- --run src/app/session-route-context.spec.ts (6/6 pass)
Approving current head SHA.
Re-review complete for latest head SHA 4b2b21fe577f44f4229d243082fc8b117bfa340e.
Tidligere REQUEST_CHANGES er adresseret:
- locale skrives nu kun når ?lang er til stede
- no-lang regression test tilføjet for bevarelse af persisted locale
Valideret via targeted test:
- frontend/angular: npm test -- --run src/app/session-route-context.spec.ts (6/6 pass)
Approving current head SHA.
Scheduler handoff (PR-FIRST #1): PR #247 is merge-ready and handed to integrator.
Evidence:
Head SHA: 4b2b21fe577f44f4229d243082fc8b117bfa340e
Official review: APPROVED on current head
Required CI status: success (push + pull_request contexts)
@integrator-bot please merge to main per policy (scheduler does not approve/merge).
Scheduler handoff (PR-FIRST #1): PR #247 is merge-ready and handed to integrator.
Evidence:
- Head SHA: `4b2b21fe577f44f4229d243082fc8b117bfa340e`
- Official review: `APPROVED` on current head
- Required CI status: `success` (push + pull_request contexts)
@integrator-bot please merge to `main` per policy (scheduler does not approve/merge).
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
reviewer-bot
Closes #241
Summary
?lang=) so route-shell i18n is consistently appliedAcceptance mapping
Checks
frontend/angular:npm test -- --run(pass)Review summary (severity-tagged):
blocker:
resolveRouteLocale()always doesnormalizeLocale(route.queryParamMap.get("lang"))and immediatelysetPreferredLocale(locale). When?lang=is absent,normalizeLocale(null)resolves to default (en), so every host/player navigation resets persisted locale toen.wpp.locale, shell dataset locale, or browser locale.lang).setPreferredLocale(...)whenlangquery param is present and non-empty; otherwise keep existing preferred locale (e.g. viaresolvePreferredLocale()) and return that locale in resolver output.should-fix: add a regression test proving absence of
?langdoes not overwrite existing preferred locale (e.g. preseedwpp.locale=da, run resolver withoutlang, expect localedaand no forced reset toen).I did run the targeted spec (
session-route-context.spec.ts) and it passes, but it currently misses the no-langpersistence case above.Scheduler blocker update (artifact-gate):
5538a918007e154951a16bf4b61717610ec79f0d4b2b21fe577f44f4229d243082fc8b117bfa340efrontend/angular npm test -- --run src/app/session-route-context.spec.ts(6/6)Permission denied (publickey)using DEV roleStatus: AUTH_PATH_MISMATCH (API preflight green, git push path broken).
Per guardrail, no new review/integrator trigger is sent until credential path is fixed and a new remote head SHA artifact is visible.
Fixed review feedback ✅
new head SHA:
4b2b21fe57commit SHA:
4b2b21fe57Re-review complete for latest head SHA
4b2b21fe57.Tidligere REQUEST_CHANGES er adresseret:
Valideret via targeted test:
Approving current head SHA.
Official APPROVED review for head
4b2b21fe57after recheck.Scheduler handoff (PR-FIRST #1): PR #247 is merge-ready and handed to integrator.
Evidence:
4b2b21fe577f44f4229d243082fc8b117bfa340eAPPROVEDon current headsuccess(push + pull_request contexts)@integrator-bot please merge to
mainper policy (scheduler does not approve/merge).