The localization guard only watched the side that never changes by rbuergi · Pull Request #984 · Systemorph/MeshWeaver

rbuergi · 2026-08-09T06:42:37Z

Fixes #964.

The defect

.github/workflows/clients.yml fired only on clients/**. But several of the jobs it runs are
deliberately cross-tree — they read a file under src/ and assert the client copy matches it:

Server input	Guard that reads it
`src/MeshWeaver.Messaging.Hub/Localization/**`	`clients/react/src/i18n/localize.test.ts` — byte-identical catalog drift guard
`src/MeshWeaver.Messaging.Contract/Localization/Locales.cs`	same test, scrapes `Locales.Supported`
`src/MeshWeaver.Blazor/BlazorViewRegistry.cs`	`clients/react/src/render/parity.test.ts` + `clients/react-native/src/parity.test.ts`
`src/MeshWeaver.Hosting.Grpc/Protos/**`	the canonical `mesh.proto` the python / grpc-web / typescript clients generate from

So every one of those guards was blind to the side that actually moves. A server-only PR adds a key
(or a control, or a proto field), the job never runs, CI is green, and the gap sits latent until an
unrelated PR happens to touch the client tree and inherits a red it did not cause.

The mechanism, proven rather than inferred

The two commits that introduced the currently-unmatched keys touched ZERO files under clients/:

c34bd01e6  A refused round explained itself with an HTTP header dump      clients/ files: 0
18b3e2ee3  A storage stall told a signed-in user their account did not exist   clients/ files: 0

Both merged green. This is the second occurrence within hours of PR #959 resyncing the previous 12
plugins.discovery.* keys — the guard gap regenerated the same defect immediately, which is what
distinguishes it from a one-off sync miss.

Remaining gaps the widened guard catches on current `main` — 5 keys

Verified by running the real test, not by construction. npx vitest run src/i18n/localize.test.ts
on unmodified main fails 2 of its tests (en and de), naming 5 keys present server-side with no
client counterpart:

chat.modelProviderError
chat.modelRateLimited
chat.modelSubstitutionNote
error.identityUnavailable
error.identityUnavailableHint

Fixed here — all five copied verbatim into clients/react/src/i18n/strings.{en,de}.json, in the
server's key order, values byte-identical.

Correction (see "Post-merge re-verification" below). An earlier revision of this description
claimed these were user-visible as raw codes — "a React-client user hitting a rate-limited model
saw the literal string chat.modelRateLimited". That is wrong, and the corrected claim is
narrower. chat.modelRateLimited and chat.modelProviderError are resolved server-side, by
LocalizationCatalog.Get(key, userAccessContext?.Locale, …) in ThreadExecution.cs (~line 2618),
off the server catalog — which always had them. What reaches the client is finished prose, not
a key. No client resolves any of these five: a repo-wide search finds no reference to them outside
the catalog JSON, no dynamic key construction (`chat.${…}`) in any client, and no
server-sends-a-key mechanism. A stale client copy therefore could not have produced a raw code for
them.

The real defect is duller and still worth fixing: the clients' bundled copy of the server's text
had silently fallen out of date, and the check that exists to catch exactly that was gated on the
one tree that does not change when the fault is introduced. The What's New entry was rewritten to
match this narrower, accurate claim.

Do the other ratchets share the blind spot?

Checked each rather than assuming:

clients/react/src/render/parity.test.ts and clients/react-native/src/parity.test.ts —
yes, same blind spot. Both scrape src/MeshWeaver.Blazor/BlazorViewRegistry.cs yet were
reachable only through a clients/**-filtered job. Both happen to pass on main today
(react: green; RN: 9/9 green), so there is no latent control gap right now — but that is luck, not
guarding: 993282677 modified BlazorViewRegistry.cs with 0 files under clients/. Fixed by the
same trigger widening.
test/MeshWeaver.Maui.Abstractions.Test/MauiControlCoverageTest.cs — the third parity ratchet.
Does NOT share the flaw, and is deliberately left alone. It is a .NET test in dotnet-test.yml,
which has no paths: filter at all, and it reflects over UiControl subclasses rather than
scraping a file. It already runs unconditionally on every PR.

How this relates to the server-side `LocalizationTest`

They guard different invariants and cannot disagree:

LocalizationTest (test/MeshWeaver.Messaging.Hub.Test/) is intra-catalogue: every English key
has a de translation, no orphan de keys, plural pairs complete. It never looks at the clients.
It runs on every PR via dotnet-test.yml.
localize.test.ts is cross-tree: the client copy is byte-identical to the server catalogue.

So the server test says "the server catalogue is internally complete" and the client test says "the
client copy equals the server catalogue". Neither constrains the other's subject; widening the
trigger adds no second opinion about what "in sync" means. Notably LocalizationTest was green
throughout — it is not the mechanism that could have caught these 5 keys, which is precisely why
the client-side guard needed to run.

Preventing recurrence

Widening a hand-maintained list fixes today's holes and nothing more — the list duplicates knowledge
that lives in the test files, so it will drift again. clients/react/src/ciTrigger.test.ts closes
that loop: it walks every client package for cross-tree reads (resolving each reference against both
the file's directory and the package root, keeping only those that land on a real file under src/)
and fails if the workflow's paths: filter does not cover one. It also asserts the pull_request
and push lists stay identical, since the Actions parser cannot share them through a YAML anchor.

Adding a new cross-tree read is itself a change under clients/**, so the job that catches it is
reached by definition — the guard-the-guard has no blind spot of its own.

Verification

Scrape output matches the hand analysis exactly — 5 server inputs found, all now covered.
Negative test: removing src/MeshWeaver.Blazor/BlazorViewRegistry.cs from the pull_request
list makes ciTrigger.test.ts fail with both assertions, naming the exact missing path. Restored
and re-verified green.
clients/react: npm run typecheck clean, npm test 34 files / 253 tests passed (after the
merge with origin/main).
clients/react-native: npx vitest run src/parity.test.ts 9/9 passed.
Catalog re-checked after merging origin/main: server 797 / client 797, no missing, no extra, no
value drift, both languages. (Superseded — main moved again; see "Post-merge re-verification".)
Workflow YAML parses; both paths: lists identical; all 8 jobs intact.
No .cs/.csproj touched, so no dotnet build -c Release -warnaserror was applicable.

Note on the committed `node_modules` symlink — RESOLVED by #983

clients/react/node_modules and clients/react-native/node_modules were committed symlinks to
absolute paths (/Users/roland/code/MeshWeaver/..., added in 69edd029d). They dangled anywhere
but that one machine — and on that machine they were self-referential: the target path is the
same committed symlink, so resolving it gave Too many levels of symbolic links. On CI the target
simply did not exist and npm install replaced the dead link, which is why it never went red.

#983 has since merged and deletes both (delete mode 120000 clients/react/node_modules) — the
right home for it, since that PR is the /Users/roland/... scrub. Nothing left to do here.

The catalogs themselves are NOT symlinks, and that is the whole point of the guard. Worth
stating explicitly, because "copy" vs "link" changes what drift even means here:

$ git ls-files -s clients/react/src/i18n/ clients/react/node_modules
100644 … clients/react/src/i18n/strings.de.json     ← regular file (real copy)
100644 … clients/react/src/i18n/strings.en.json     ← regular file (real copy)
120000 … clients/react/node_modules                 ← symlink (mode 120000)

strings.{en,de}.json are mode 100644 regular files — genuine byte copies of the server catalog,
duplicated so the clients can resolve text synchronously without a round trip. Nothing keeps them in
step automatically. If they were links, drift would be impossible and localize.test.ts would be
pinning nothing; because they are copies, drift is not only possible but is what happened, twice.
The only symlink in the area is node_modules, which is dependency-install state and unrelated to
the catalogs.

Post-merge re-verification (`20b9f1981`)

The parity figures above were measured before origin/main moved. Re-checking rather than
assuming was the right call: merging main re-opened the very gap this PR exists to close.

#985 and #992 landed 21 further server-side keys with no file under clients/, plus a reworded
about.intro the copy never received — i.e. the defect regenerated a third time, while this PR
sat open, which is the strongest possible argument for the trigger change:

	measured at branch-cut	after merging `origin/main`	now (`20b9f1981`)
keys missing from client	5	21	0
value drift	0	1 (`about.intro`)	0
server / client key count	797 / 797	818 / 797	818 / 818

Re-synced in 622e72e92; the client catalogs are byte-identical to the server's again (diff on
both files is empty, both languages). The 21: about.updateStatus / upToDate / updateAvailable,
menu.modelTiers + menu.modelTiersTooltip, and the 16-key aiCatalog.tab.* / aiCatalog.search.*
family. Both languages complete — no key ships English-only.

Guards re-proven by mutation at this HEAD, not assumed:

Mutation	Result
drop `src/MeshWeaver.Messaging.Hub/Localization/` from both** `paths:` lists	`ciTrigger.test.ts` ✗ — names the exact uncovered path
drop `src/MeshWeaver.Blazor/BlazorViewRegistry.cs` from both lists	`ciTrigger.test.ts` ✗ — names the exact uncovered path
drop the localization path from the push list only	`ciTrigger.test.ts` ✗ — "keeps the pull_request and push filters identical"
delete `menu.modelTiers` from the client `en` catalog	`localize.test.ts` ✗ — 817 vs 818 keys

All restored; both files green afterwards (19/19). The meta-guard is pinning something real.

Trigger ownership vs #983 — #983 has dropped its incidental clients.yml edit (its file list no
longer contains the workflow), so #984 owns it uniquely. Verified the result is coherent, not
duplicated: exactly two paths: blocks, each with the identical 6 entries, no repeats. #983 still
edits the same two catalog files, so whichever lands second will conflict there — the resolution is
mechanical, since the server catalog is the single source of truth and the correct client content is
a verbatim copy of it.

Full local run at this HEAD: clients/react npm test 34 files / 253 tests green, npm run typecheck clean, npm run build clean; LocalizationTest 33/33; DocumentationLinkIntegrityTest
1/1; MeshWeaver.Documentation builds clean under -c Release -warnaserror (0 warnings, 0 errors).

After merging #983 — the catalog change is gone, and that is the correct outcome

#983 merged mid-review and re-synced the client catalogs itself (54c0ffe75, "The client string
catalogs had fallen 8 keys behind the server"). origin/main therefore already has them
byte-identical to the server, so the merge dropped the catalog files out of this PR's diff:

.github/workflows/clients.yml            | 36 ++++++
clients/react/src/ciTrigger.test.ts      | 138 +++++++++++++++
…client-strings-no-longer-lag-the-server.md | 27 ++++
3 files changed, 201 insertions(+)

Both PRs converged on identical content without a conflict, which is what you would expect: the
correct client catalog is definitionally a verbatim copy of the server's, so there is exactly one
right answer to converge on. Re-verified post-merge: 818 / 818 keys, both languages, zero missing,
zero extra, zero value drift, and diff against the server catalog is empty for both files (so no
duplicated keys or reordering slipped in — JSON.parse would have silently hidden either). React
suite re-run on the merged tree: 34 files / 253 tests green.

What remains is exactly this PR's own substance — the trigger widening, the meta-guard, and the
note. The What's New was updated to stop claiming the sync.

Trigger ownership confirmed after the merge: #983's incidental workflow edit (77f2955ec) is
NOT in origin/main's clients.yml — it was dropped before merge, as coordinated. #984 owns the
trigger uniquely, and the merged result is coherent: exactly two paths: blocks, six identical
entries, no duplication.

🤖 Generated with Claude Code

Counterfactual: replaying the three commits that slipped through

Applying both filters to each commit's actual file list — the direct answer to "would the widened
guard have caught this?":

Commit	What it added	Old filter fires	New filter fires
`c34bd01e6`	`chat.model*` keys (8 files)	❌ no	✅ yes — `.../Localization/strings.{en,de}.json`
`18b3e2ee3`	`error.identityUnavailable*` keys (8 files)	❌ no	✅ yes — `.../Localization/strings.{en,de}.json`
`993282677`	changed `BlazorViewRegistry.cs` (11 files)	❌ no	✅ yes — `src/MeshWeaver.Blazor/BlazorViewRegistry.cs`

All three are server-only diffs with zero files under clients/. Under the old filter none of them
ran the guard; under the new one all three do — on the PR that created the change.

The client-side catalog drift guard and both Blazor-parity ratchets read files under src/, but .github/workflows/clients.yml only fired on "clients/**". So the guards were blind to the exact side that moves: a server-only PR adds a key, the job never runs, CI is green, and the gap sits latent until an unrelated PR happens to touch the client tree and inherits a red it did not cause. Not hypothetical. c34bd01 and 18b3e2e each added localization keys and touched ZERO files under clients/. Both merged green, and the 5 orphan keys they left survived on main until this commit — the second recurrence within hours of PR #959 resyncing the previous 12. The trigger now lists every input these jobs actually read: the localization catalog, Locales.cs, BlazorViewRegistry.cs, and the canonical mesh.proto. That puts the cost back on the PR that creates it. Widening a hand-maintained list once fixes today's holes, so ciTrigger.test.ts scrapes the client tree for cross-tree reads and fails if the filter misses one (or if the pull_request and push lists drift — Actions cannot share them via a YAML anchor). Adding such a read is itself a clients/** change, so the guard that catches it is reached by definition. Fixes #964

…parity-trigger

Copilot

Pull request overview

This PR closes a CI blind spot where the “Clients” workflow only triggered on clients/** even though multiple client-side parity/drift tests intentionally read canonical inputs from src/. It also resyncs missing localization keys so the React client no longer renders raw localization keys for several user-visible error states.

Changes:

Widen .github/workflows/clients.yml paths: filters (for both pull_request and push) to include server-side inputs that client parity/drift jobs read.
Add a “guard-the-guard” Vitest test (clients/react/src/ciTrigger.test.ts) that scrapes cross-tree reads and asserts the workflow trigger covers them.
Sync 5 missing localization keys into clients/react/src/i18n/strings.{en,de}.json and add a matching What’s New entry.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
`src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-09-client-strings-no-longer-lag-the-server.md`	Documents the localization parity/trigger fix and the user-visible string corrections.
`clients/react/src/i18n/strings.en.json`	Adds the 5 missing English localization strings used by the React client.
`clients/react/src/i18n/strings.de.json`	Adds the 5 missing German localization strings used by the React client.
`clients/react/src/ciTrigger.test.ts`	Adds a test that enforces the workflow trigger covers any `src/` files the client packages read at runtime.
`.github/workflows/clients.yml`	Expands trigger paths to include server-side canonical inputs used by client parity/drift checks.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

github-actions · 2026-08-09T06:59:27Z