feat(i18n): internationalization across the API and both React apps - #1344
feat(i18n): internationalization across the API and both React apps#1344marcelo-maciel wants to merge 76 commits into
Conversation
Add SupportedCultures constant (Default/Tags/RequestMatch) in BuildingBlocks/Core/Localization and reject unsupported locale tags in UpdateUserCommandValidator; null/empty locale still passes.
Inject IStringLocalizer<SharedResources> and swap hardcoded ProblemDetails titles (Validation/Unauthorized/NotFound/BadRequest/Unexpected + the 500 Detail) for resx keys. Exception-supplied Detail messages stay raw. Docker-free handler-level tests exercise the localized 404 and 500 branches under pt-BR and en-US.
Inject IStringLocalizer<SharedResources> into UpdateUserCommandValidator and resolve the three custom WithMessage literals lazily (Func overload, so the lookup runs per validation under the request culture, not at construction). Add the keys to both resx catalogs. Built-in FluentValidation messages localize automatically via CurrentUICulture (FV ships a pt catalog) — no LanguageManager wiring needed. Adds a resx key-parity test (neutral vs .pt) and updates the Task 2 validator test to supply a localizer.
Add a Language section to the profile dropdown, mirroring the Theme section: one item per SUPPORTED locale with an active-locale check. Selecting a locale calls i18n.changeLanguage and persists it via a new updateMyProfile mutation (PUT /identity/profile). The locale travels through the mutate argument (frontend rule fullstackhero#9). Current name/phone are echoed to avoid the backend wiping FirstName/LastName on a locale-only save. onSuccess triggers a best-effort token refresh so the new locale JWT claim is minted.
Mirror the admin Task 10 switcher on the tenant dashboard: a Language
section in the profile dropdown that switches the UI locale in place,
persists it, and re-mints the JWT locale claim.
- topbar: LanguageMenuItem + Language section (preventDefault keeps the
menu open so the section label re-localizes visibly); onSelectLanguage
calls i18n.changeLanguage, persists via updateMyProfile (locale by
mutate arg), and refreshes the token best-effort onSuccess. Profile
query is not invalidated so a refetch cannot revert the switch.
- api/identity: UpdateProfileInput gains locale; the PUT body echoes it
alongside the profile-read name/phone so a locale-only save cannot wipe
FirstName/LastName (backend sets them unconditionally).
- test: switcher spec asserts PUT {locale: pt-BR} with names preserved,
in-place localization, and the token refresh firing.
…e, settings, search)
…/combobox primitives
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Both Playwright configs raise actionTimeout to 10s and navigationTimeout to 15s but leave `expect` at its 5s default, so the assertion every test ends on had the tightest deadline in the suite. Under CPU contention the first paint of a lazy route lands past 5s while staying well inside the budgets the config already chose, and the run fails on toBeVisible. Measured on the dashboard suite at 16 workers, same machine, three runs: no expect timeout 13 failures, mostly `Timeout: 5000ms` on toBeVisible expect 10s 5 failures, none of them an assertion reverted 17 failures, 14 of them assertions CI runs 2 workers with 2 retries, which is why this stayed hidden there: the retry absorbs it and the job still reports green. What remains under deliberate oversubscription is action and navigation timeouts, i.e. a saturated machine rather than a budget mismatch. Not chased further — inflating those would hide real regressions.
iammukeshm
left a comment
There was a problem hiding this comment.
The engineering here is strong and I want to be explicit about that before I ask for anything, because what I'm asking for is not a criticism of the work.
Several decisions in this are better than what I'd have specified:
MessageKey/MessageArgs/ResourceSourceonCustomException, withMessagestaying English. Localising the response body while keeping logs culture-independent is the correct split, and getting it right at the exception type means call sites don't each have to remember it.- Surfacing
MessageKeyas acodeextension on ProblemDetails. This is the detail that makes the whole thing usable. Localiseddetailis prose that changes withAccept-Language; without a stable key, every client that needs to branch on a specific error ends up matching translated text. Addingcodeturns a localisation change into an API improvement. The tests pinning "unknown key still travels as the code" and "no key → nocodeproperty at all" show the contract was thought through rather than stumbled into. ILocalizableMessagesubclasses that preserveUnauthorizedAccessException/KeyNotFoundExceptionas base types because audit severity classification keys off the runtime type. That's a non-obvious coupling and most contributors would have swapped the types and quietly broken audit classification.- Scoping
LogContext.PushPropertyinusingblocks. That fixes a pre-existing AsyncLocal leak that was contaminating every subsequent log entry in the request. It's unrelated to i18n and it's a genuine bug fix. - Dropping
CookieRequestCultureProviderby type rather than by index, with the comment explaining that a framework reshuffle of the defaults would otherwise remove the wrong provider.
What I'm asking for: split this into three PRs
519 files and ~20k lines, touching BuildingBlocks/Core, Web, Jobs and Storage, every module, both React apps, and an Identity migration — landing weeks after the 10.0.0 GA.
I can't review this to the standard the change deserves, and neither can anyone else. The parts I've examined closely are good; that's precisely the problem, because it means the risk sits in the parts nobody will read carefully. A 22-resx, 519-file diff gets skimmed, and skimming is how a wrong translation on an authorization message or a subtly changed exception type gets in.
Please split:
- Framework —
BuildingBlockslocalization,SharedResources, the exception plumbing,GlobalExceptionHandler, the culture-provider chain, theUser.Localecolumn and migration. This is the one that needs real scrutiny and it's small enough to get it. clients/admin— react-i18next wiring, switcher, catalogs, parity tests.clients/dashboard— same.
(1) is the hard review and unblocks the other two. (2) and (3) are then mostly mechanical and can be reviewed quickly and in parallel. Same total work for you, dramatically better review for the repo. I'll prioritise them as they arrive so this doesn't stall.
Issues to carry into the split
Golden Rule #4 — undeclared BuildingBlocks changes. This touches Core (exceptions, localization, csproj), Web (Extensions.cs, GlobalExceptionHandler, validation), Jobs and Storage. The description doesn't mention it. Protected code needs explicit sign-off and the description has to say so plainly — see #1323 and #1334 for the form.
UseRequestLocalization sets CurrentCulture, not just CurrentUICulture. Every ToString(), Parse and interpolation in the request pipeline shifts to the negotiated culture. In a codebase not written culture-aware, that's how you get 1,5 in a JSON number or a date that round-trips wrong.
I checked before raising it: AnalysisMode=AllEnabledByDefault with CodeAnalysisTreatWarningsAsErrors=true means CA1305 is enforced, so first-party code already passes explicit format providers — the exposure is much smaller than it first looks. Residual risk is third-party libraries and any [SuppressMessage]. Please state this explicitly in the framework PR rather than leaving it implicit, and say whether you considered setting only CurrentUICulture and leaving CurrentCulture invariant. For an API whose output is JSON, UI-culture-only is arguably the safer default, with formatting handled at the presentation layer where both React apps are already doing it.
The Locale column doesn't match its description. The summary says "a new nullable User.Locale column (default en-US)" but the migration is type: "text", nullable: true with no default — the en-US is a code-level fallback. That's a fine design, but say what it is. Also constrain the column: a BCP-47 tag is varchar(10), not unbounded text. Validation should reject anything outside SupportedCultures.Tags at the write boundary (I see Validation.UnsupportedLocale exists — confirm it's wired to the profile update).
SharedResources.pt.resx is neutral pt; the front-end catalogs are pt-BR. Resource fallback makes this work today (pt-BR → parent pt), and using the neutral is defensible. But the asymmetry with the front-end will confuse the next contributor, and adding pt-PT later silently serves Brazilian strings. Pick one convention and document the fallback intent either way.
RequestMatch includes bare pt and en. Combined with CurrentCulture being set, a request can resolve to a neutral culture, whose formatting comes from a representative culture rather than a specified one. Harmless in practice, but if you go UI-culture-only above, it stops mattering at all.
JWT-claim-carried locale. A language switch doesn't reach the API until the next token issue. The front-end persists to the profile so it converges, but there's a window where the UI is in one language and API errors come back in another. That's an acceptable trade for not adding a per-request DB read — it just needs to be documented as known behaviour rather than discovered.
Docs (Golden Rule #10). Noted that these are prepared as a companion docs PR; they should land with the framework PR, since code on ProblemDetails and the new config section are both public contract.
On #1301
Branching this off v10 GA as discussed was the right call. The foundation is one I'm happy to build on — I'd just like to merge it in pieces I can actually stand behind. Please open the framework PR first and I'll turn it around quickly.
Brings in the 24 commits since the branch point (outbox redesign, wallet currency match, SQLite/crypto package pins). Conflict: src/Directory.Packages.props. main added the same System.Security.Cryptography.Xml 10.0.10 pin this branch carried, with a more accurate comment (five advisories, not four, and the real transitive source). Took main's block verbatim, so the branch no longer changes the central package file at all.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 304e8f777b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| HttpStatusCode.Unauthorized => "Error.Unauthorized", | ||
| HttpStatusCode.Forbidden => "Error.Forbidden", | ||
| HttpStatusCode.BadRequest => "Error.BadRequest", | ||
| _ => "Error.Unexpected", |
There was a problem hiding this comment.
Map non-standard custom statuses before falling back
When a CustomException uses statuses that are already common in this repo, such as HttpStatusCode.Conflict for duplicate brands/categories/products or InsufficientStorage for quota failures, this default makes the response title Error.Unexpected even though the HTTP status and detail describe an expected business error. That regresses these localized ProblemDetails from status-specific/type-specific errors into an “unexpected error” banner; please add mappings for the business statuses in use or fall back to the exception type/title instead of Error.Unexpected. .agents/rules/localization.mdL36-L36
Useful? React with 👍 / 👎.
…rals
The backend catalogs were neutral `*.pt.resx` holding Brazilian Portuguese while
the front-end catalogs are `pt-BR`. Resource fallback made that work (pt-BR ->
parent pt), but it left two problems: the asymmetry with the front-end is a trap
for the next contributor, and a future pt-PT would have been served Brazilian
strings silently by the same parent fallback.
Rename all eleven catalogs to `*.pt-BR.resx` so every catalog is named for the
specific culture it actually contains. A bare `pt`, or an unsupported variant
like pt-PT, now walks its parent chain, finds no catalog and lands on the neutral
English one instead of being handed Portuguese it was never translated into.
SharedResourcesLocalizationTests pins that consequence directly.
Adding a language is now: add its specific tag to SupportedCultures.Tags plus a
`*.{tag}.resx` per catalog. Pure rename, no string changed (all eleven are R100).
UseRequestLocalization was configured with both AddSupportedCultures and
AddSupportedUICultures, so every negotiated request also moved
CultureInfo.CurrentCulture. In a codebase that is not written culture-aware that
is how a JSON number arrives as "1,5" or a date round-trips wrong. Localizing the
response body is the goal; shifting formatting for the whole request is not.
Note that main has no request localization at all, so pinning the formatting
culture is LESS change than negotiating it: CurrentCulture now behaves exactly as
it does today on main, and only resource lookup follows the request.
Getting there is not a single switch. RequestLocalizationMiddleware's
SetCurrentThreadCulture assigns both CurrentCulture and CurrentUICulture
unconditionally, so the culture half has to be pinned rather than left alone:
- DefaultRequestCulture now carries (InvariantCulture, configured default). The
middleware resolves the culture half as `cultureInfo ??=
DefaultRequestCulture.Culture`, which makes invariant the only reachable
value.
- SupportedCultures is null so the middleware skips culture filtering. A
one-element [InvariantCulture] list behaves the same but logs
UnsupportedCultures on every request: the middleware's parent-culture walk
bails at the empty culture name, so invariant is unmatchable by design.
With formatting out of the negotiation, the neutral `pt`/`en` entries that existed
only to widen Accept-Language matching no longer buy anything, and they made a
request resolvable to a neutral culture whose formatting comes from a
representative culture rather than a specified one. SupportedCultures.RequestMatch
is therefore gone; Tags is the single list, specific tags only, matching the
renamed catalogs.
Behaviour change worth stating plainly: a request asking only for a bare `pt` or
an unsupported variant now resolves to the configured default rather than
Portuguese. Both React apps canonicalize variants onto supported tags before
calling the API, so app traffic is unaffected; a hand-rolled client sending bare
`pt` is.
The new test drives the real middleware and pre-sets CurrentCulture to pt-BR
before invoking it, so it proves the middleware actively resets the formatting
culture rather than merely leaving an already-invariant ambient value alone.
The column shipped as unbounded `text`. A BCP-47 tag is short and bounded, so there is no reason to accept arbitrary input at the storage layer: 10 characters covers language-script-region (zh-Hant-TW), which is the longest form the platform could ever offer. Writes were already constrained to SupportedCultures.Tags by UpdateUserCommandValidator on PUT /identity/profile, so this is the storage-level backstop, not the validation. AddUserLocale is edited in place rather than stacked with an ALTER: it has never shipped in a release, it only exists on this branch. Verified with `dotnet ef migrations has-pending-model-changes` (red before the snapshot edit, green after) and by reading the generated DDL: `ALTER TABLE identity."Users" ADD "Locale" character varying(10);`
Closes the third Codex P2, the one still open on fullstackhero#1344. StartImpersonation strips the target's `locale` claim on purpose so the operator keeps reading in their own language. But the dashboard normally runs on a separate origin from admin, so it cannot read admin's persisted `i18nextLng`, and the handoff URL carried only token/tenant/expiresAt. With no claim to negotiate from and nothing in the handoff, the API culture fell through to the dashboard's own browser detection: the operator picked Português in admin and then read English error details in the impersonated session. The handoff now carries `locale`, and the dashboard adopts it before createRoot. That fixes both halves at once: the shell renders in the operator's language, and apiFetch derives Accept-Language from i18n.language so the API localizes to it too. Unsupported or absent tags are ignored, leaving normal detection in place. Chosen over adding an actor-locale claim to the token, which Codex offered as the alternative: this stays inside the two React apps and leaves the token contract and the framework culture provider untouched. `installImpersonationFromHash` becomes async for the changeLanguage await. The token install stays synchronous at the top, and main.tsx awaits the whole thing before createRoot, so the "installed before AuthProvider's first render" guarantee is unchanged. Known gap, deliberately not addressed here and named in the spec: the SignalR client builds its own requests instead of going through apiFetch, so the hub negotiate carries the browser's Accept-Language rather than the app's locale. That applies to every session, not just impersonation. The spec asserts every OTHER path carries the operator's locale, so a new channel that stops carrying it fails the test instead of slipping through.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
The varchar(10) constraint had no test pinning it — reverting HasMaxLength(10) left the whole suite green, so the fix could regress silently. Asserts the model-level max length directly, following the EventingDbContextModelTests pattern for building a module DbContext without a database.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…tive The two window validators interpolated `MaxWindow.TotalDays` — a double — into a localized message. `ResourceManagerStringLocalizer`'s indexer formats arguments with `string.Format` under `CurrentCulture`, which is now pinned invariant, so a non-integral value would render with an invariant decimal separator regardless of the reader's language. Today the window is exactly 90 days, so both cultures render "90" and no output changes. The point is that the type made it culture-sensitive by construction, and this is the only place in any catalog where a placeholder argument was not an int, long, string or enum — verified by enumerating every `MessageArgs` and every `localizer["…", …]` call site. Fixed at the source rather than at the call site: `MaxWindowDays` is the int the message wants and `MaxWindow` derives from it, so the two can never disagree and no cast can truncate.
Ten modules have a hand-written parity test. Notifications does not — it has no test project at all, so the catalog this branch added to it shipped with no parity guard. Parity is exactly the invariant that fails silently: a key missing from a translated catalog falls back to the neutral English string and ships looking translated. Discovers catalogs by reflection (a type whose full name matches an embedded `.resources` manifest, i.e. the co-located `ResourcesPath = ""` convention) across every `FSH.Modules.*.dll` in the test output plus Core, then compares each culture's OWN key set with `tryParents: false` — with parent fallback on, a missing key would be answered by the neutral catalog and parity would always look perfect. Covers new modules with no new test. Asserts a floor on the number of catalogs discovered so a reflection regression fails instead of silently guarding nothing. Verified by mutation: dropping `Notifications.NotificationNotFound` from NotificationsResources.pt-BR.resx turns it red naming both the catalog and the key.
The rule still described the design this branch replaced: a negotiated CurrentCulture, neutral `.pt` catalogs, and neutrals in the supported-tag list. A contributor reading it would have re-introduced exactly what the review asked to remove. Records the UI-culture-only split and why the culture half has to be pinned rather than left alone, the specific-tags-only convention and what adding a language now involves, the requirement that placeholder arguments be culture-insensitive, and the generic parity guard. Adds a "Known behaviour" section for the three things that are deliberate and were previously only discoverable by reading code: the one-token lag of the `locale` claim, impersonation carrying the operator's language, and SignalR not carrying the app locale at all.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
The dashboard side of the handoff was pinned; the producer side was not. Dropping
`params.set("locale", i18n.language)` from the dialog left every suite green, so
half the fix had no gate.
Drives Re-open (which pre-fills the user and skips the picker step) through to
Start, with `window.open` stubbed — the URL is the thing under test and the real
dashboard origin is not served in this suite. Asserts the operator's language in
both directions (pt-BR and en-US, selected via the `?culture=` detection hook) and
re-asserts token/tenant/expiresAt so the added parameter cannot quietly displace
the pre-existing contract.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Matching keys are not enough. The caller passes ONE argument list for every culture,
so a translation that consumes a different set of `{n}` placeholders than the
neutral string is broken in that culture only — never on the reviewer's machine.
`{1}` present in Portuguese but not English is the dangerous direction:
string.Format throws FormatException when the index is out of range, turning a
localized 404 into a 500 for Portuguese readers. The opposite direction silently
drops an argument the message was supposed to show.
Compares the placeholder index SETS per key, tolerating reordering (which
translation legitimately needs) and format specifiers, and stripping escaped braces
so a literal brace is not read as a placeholder.
All eleven catalog pairs currently match. Verified by mutation: adding a `{1}` to
Catalog.BrandNotFound in pt-BR fails with
"neutral uses {0} but pt-BR uses {0,1}".
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
index.html ships a static `lang="en"` in both apps and nothing ever updated it, so a Portuguese UI kept declaring itself English to screen readers, browser translation offers and hyphenation. The whole point of the feature is that the page is in the reader's language; the attribute that tells assistive tech so was left behind. An i18next `languageChanged` listener registered before init, so it covers the initial detected language as well as every switch. Also the only language signal in these apps that a stale render cannot satisfy, which makes it the right assertion target for locale specs.
… language Audit finding (concurrency lens), verified at the source. `updateMyProfile` is a GET-then-PUT with no concurrency token, and Settings > Profile invalidates the SAME ["identity","me"] key the topbar reads. So a Settings save whose read preceded the language PUT echoes the pre-switch locale back and wins if it lands second. The topbar's hydration effect then saw `persistedLocale` change and called `changeLanguage` on it — the user watched the UI revert with no error and nothing to act on. Admin has the same shape via two rapid switches landing out of network order. Hydration now stops once the user picks a language in this session. A locale chosen on another device still carries over, because that is a fresh mount with no in-session choice — pinned by hydration-guard.spec.ts. This does NOT fix the lost update itself; the server can still end up holding the old locale, which is what the `ponytail:` notes in both topbars record. The damage is bounded from "the app changed language while I was using it" to "my language did not stick across a reload". The real fix is an ETag / RowVersion with If-Match on PUT /identity/profile — a contract change to an existing endpoint, its own PR. Not pinned by a regression test: reproducing it needs an in-mount profile refetch driven through the Settings form, and the click races the language-change re-render (element detached from the DOM). Three distinct attempts, then stopped rather than paper over it with retries. Recorded as unpinned rather than presented as verified.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Both verified at the source before changing anything.
UpdateUserCommandValidatorTests: the built-in-message test asserted only
ShouldNotContain("is not a valid email address"). A blank message, a raw resource key
leaking through, or any wrong-but-non-English string all satisfied that — the exact
failures it existed to catch. Now pins the actual Portuguese text, matching the
sibling test one method up. The literal came from the runtime, not from guessing.
ExceptionSeverityClassifierTests: LocalizedUnauthorizedAccessException subclasses the
BCL type precisely so the audit severity classifier keeps mapping unauthorized access
to Warning, and that intent existed only as a code comment. Changing the base type
would have silently reclassified every unauthorized access as Error with the whole
suite green. Added for both Localized subclasses.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Audit finding (data-contract lens), verified at the source. TitleKeyFor mapped four statuses and sent everything else to "Error.Unexpected". The type-name fallback next to it looked like it covered the rest, but it only fires on ResourceNotFound — and Error.Unexpected resolves, so it never fired. Every status outside those four reported a title contradicting its own status code and its own detail. The 41 Conflict throw sites across Billing and Catalog answered Status 409 with "An unexpected error occurred" and a detail describing an ordinary business-rule conflict. Before this branch, Title was `e.GetType().Name` — imperfect, but at least status-consistent. This regressed that. TitleKeyFor now returns null for a status with no title of its own, which routes back to the exception type name, and Conflict gets a real localized title (Error.Conflict, added to both shared catalogs). Any RFC7807-aware client branching on `title` for a conflict sees a coherent value again. Pinned in both directions: Conflict renders "Conflict"/"Conflito", and an untranslated status (Locked) falls back to the type name rather than claiming the error was unexpected.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Both parity specs iterate a hand-maintained namespace array. A namespace file added without a matching entry escaped every parity assertion in the file — it could ship half-translated with the suite green, which is precisely the failure these specs exist to prevent. The backend closed the equivalent gap generically via reflection; these two did not. Verified by mutation: dropping a new JSON catalog into src/locales/en-US fails the check naming the uncovered namespace.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Thanks — and the split is the right call. I'm not arguing with it, but I've closed the review on this branch first, and I want to explain the order rather than just do it. Three of these don't respect the split boundary. The impersonation locale fix spans both apps and only makes sense as one change. The Everything below was verified at the source, and every fix has a mutation gate: I revert the fix, require the pinning test to go red, and restore the file byte-exact. Where something isn't pinned I say so. Golden Rule #4. You're right and the omission was mine. The description now lists all eighteen
It wasn't a single switch, and the reason is worth recording:
Argument formatting. Since the localizer formats with The
JWT-carried locale. Documented as known behaviour, in the description and in the rules file, together with the impersonation and SignalR caveats. Docs. Will land with the framework PR. Changed after your review, and not reviewed by anyone.
Not pinned, stated plainly: the hydration guard has no regression test. Reproducing it needs an in-mount profile refetch through the Settings form and the click races the language-change re-render. Three distinct approaches, then I stopped rather than reach for retries or a longer timeout. CI is red on |
Implements internationalization (i18n) with multi-language support across the backend API and both React front-ends, following up on the discussion in #1301 (branched off v10 GA as suggested).
src/BuildingBlocks(Golden Rule #4)The previous description omitted this, which was wrong. Eighteen files, needing maintainer sign-off:
Core—Core.csproj;Exceptions/(CustomException,ForbiddenException,UnauthorizedException, and the newILocalizableMessage,LocalizedKeyNotFoundException,LocalizedUnauthorizedAccessException);Localization/(newSharedResourcesmarker +SupportedCultures+ the two shared catalogs).Web—Extensions.cs(registers and orders the localization middleware, +6 lines);Exceptions/GlobalExceptionHandler.cs;Validation/PagedQueryValidator.cs; newLocalization/(LocalizationExtensions,UserLocaleRequestCultureProvider).Jobs—Extensions.cs, one exception message.Storage—QuotaMeteredStorageService.cs, one exception message.No existing behaviour of other building blocks is altered.
src/Directory.Packages.propsis not touched: the pin this branch carried is already onmain, so after the merge the file has a zero-line diff.UseRequestLocalizationsets the UI culture onlyYou asked whether UI-culture-only was considered. It is now what ships.
mainhas no request localization at all, so pinning the formatting culture is less change than negotiating it:CultureInfo.CurrentCulturebehaves exactly as it does onmaintoday, and only resource lookup follows the request. For an API whose output is JSON that is the safer default, and it makes the CA1305 question moot rather than merely bounded.It is not one switch.
RequestLocalizationMiddleware.SetCurrentThreadCultureassigns both cultures unconditionally, so the culture half has to be pinned:DefaultRequestCulturecarries(InvariantCulture, configured default). The middleware resolves the culture half ascultureInfo ??= DefaultRequestCulture.Culture, making invariant the only reachable value.SupportedCulturesisnull, so the middleware skips culture filtering entirely. A one-element[InvariantCulture]list behaves identically but logsUnsupportedCultureson every request — the middleware's parent-culture walk bails at the empty culture name, so invariant is unmatchable by design.With formatting out of the negotiation, the neutral
pt/enentries bought nothing and are gone;SupportedCultures.Tagsis the single, specific-only list. A request asking for a bareptor an unsupported variant now resolves to the configured default. Both React apps canonicalise variants onto supported tags before calling the API, so app traffic is unaffected; a hand-rolled client sending bareptgets the default.Message arguments are culture-insensitive too. The localizer formats with
string.FormatunderCurrentCulture, so adoubleorDateTimein a message would render with an invariant separator. EveryMessageArgssite and everylocalizer["…", …]call site was enumerated: allint,long,stringor enum, exceptMaxWindow.TotalDaysin the two audit-window validators, which is now anintat the source.Catalogs are named for specific cultures
SharedResources.pt.resxand the ten module catalogs are now*.pt-BR.resx, matching the front-end. Pure renames, no string changed.The asymmetry you flagged is gone, and so is the trap behind it: a future
pt-PTis no longer served Brazilian strings by parent fallback. The documented consequence is that a bareptor an unsupported variant lands on the neutral English catalog rather than on Portuguese. Adding a language is now: add the specific tag toSupportedCultures.Tags, add a*.{tag}.resxper catalog, add the JSON catalogs to both apps, and drop it from the front-endCANONmap if it was being folded into another tag..agents/rules/localization.mdrecords all of this.The
LocalecolumnThe old summary was wrong: there is no DB default. The column is nullable with
en-USas a code-level fallback, and it is nowcharacter varying(10)rather than unboundedtext— 10 covers language-script-region (zh-Hant-TW).AddUserLocalewas edited in place rather than stacked with anALTER, since it has never shipped in a release.Confirmed as you asked:
Validation.UnsupportedLocaleis wired at the write boundary.UpdateUserCommandValidatorrestrictsLocaletoSupportedCultures.Tags, onPUT /identity/profile, via the MediatorValidationBehavior. The column constraint is the storage-level backstop, not the validation.Known behaviour (documented, not bugs)
localeclaim lags a language switch by one token. The provider reads the JWT claim, so a switch reaches the API at the next token issue. The front-end persists to the profile and re-mints, so it converges; in between, the shell can be in the new language while an API error is still in the old one. The alternative is a per-request DB read on every authenticated call.apiFetch, soAccept-Languageon the negotiate is the browser's. Applies to every session, not just impersonation. Named explicitly inhandoff-locale.spec.tsso any other channel that stops carrying the locale fails the test.Fixed since the last review
Three reviewer bots' P2s, plus what a five-lens adversarial pass turned up. Each was verified at the source before being treated as real, and each fix has a mutation gate — the fix is reverted, the pinning test must go red, and the file is restored byte-exact.
StartImpersonationstrips the target'slocaleclaim on purpose, and the two apps normally sit on different origins, so the handoff URL had no way to convey it and the API fell through to the dashboard's own browser detection. The handoff now carrieslocale; the dashboard adopts it beforecreateRoot, which fixes the shell andAccept-Languageat once. Chosen over an actor-locale token claim: it leaves the token contract and the framework culture provider untouched.TitleKeyForsent everything outside four statuses toError.Unexpected; the type-name fallback beside it only fires onResourceNotFound, and that key resolves, so it never fired. Every one of the 41Conflictthrow sites across Billing and Catalog answeredStatus: 409with a title contradicting its own detail. This branch regressed that — before it,Titlewas the exception type name. Unmapped statuses now fall back to the type name again, andConflictgets a real localized title.updateMyProfileis a read-modify-write with no concurrency token, and Settings › Profile invalidates the same["identity","me"]key the topbar reads, so a save whose read preceded the language PUT could echo the old locale back and win. The topbar's hydration effect then changed the UI language to it. Hydration now stops once the user chooses a language in-session; a locale set on another device still carries over on a fresh mount.lang="en"that nothing updated, so a Portuguese UI announced itself as English to screen readers and browser translation.ExceptionSeverityClassifierwas never exercised with theLocalized*subclasses, even though those subclass the BCL types precisely to keep audit severity classification working — changing a base type would have silently reclassified every unauthorized access with the suite green.CatalogParityTestsdiscovers every catalog by reflection and compares each culture's own key set withtryParents: false, plus the placeholder-index set per key —{1}present in one culture and not the other throwsFormatExceptionat render time, in that culture only. New module catalogs are covered without a new test.Known: CI is red on a pre-existing dependency advisory
NU1903/GHSA-q939-rpr3-3284onSSH.NET2025.1.0, pulled transitively by Testcontainers. It failsrestorefor the whole solution underTreatWarningsAsErrors, onmaintoo —dotnet restore src/FSH.Starter.slnxat3f2959e6fails identically. Not introduced here and not fixed here; the pin lives in #1333. Verified locally with the audit disabled on the command line only, never committed.Testing
Not pinned, and stated rather than glossed: the hydration guard has no regression test. Reproducing it needs an in-mount profile refetch driven through the Settings form, and the click races the language-change re-render (element detached from the DOM). Three distinct approaches, then stopped rather than paper over it with retries or a longer timeout.
Notes
en-USandpt-BRare held at strict key and placeholder parity, enforced by tests, so a missing or mis-arged translation fails the build instead of shipping English.codeon ProblemDetails and the new config section are public contract.