Skip to content
Merged
36 changes: 36 additions & 0 deletions .github/workflows/clients.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,52 @@ name: Clients

# CI for the foreign-language client packages under clients/. The .NET solution CI does not cover
# these, so typecheck + test them here on any PR that touches them.
#
# 🚨 The trigger must list every INPUT these jobs read, not just the tree they live in.
# Several jobs deliberately reach ACROSS into src/ — the server is the single place a string, a
# control mapping or a wire contract is authored, and the client copies are checked against it:
#
# src/MeshWeaver.Messaging.Hub/Localization/** → clients/react/src/i18n/localize.test.ts
# (byte-identical catalog drift guard)
# src/MeshWeaver.Messaging.Contract/Localization/ → the same test scrapes Locales.Supported
# Locales.cs
# src/MeshWeaver.Blazor/BlazorViewRegistry.cs → clients/react/src/render/parity.test.ts
# + clients/react-native/src/parity.test.ts
# (control/skin vocabulary is DERIVED from it)
# src/MeshWeaver.Hosting.Grpc/Protos/** → the canonical mesh.proto the python,
# grpc-web and typescript clients generate from
#
# Filtering on "clients/**" alone made every one of those guards blind to the side that actually
# changes: a server-only PR adds a key (or a control, or a proto field), CI stays green because
# nothing under clients/ moved, and the gap sits LATENT until some unrelated PR happens to touch the
# client tree and inherits a red it did not cause. That is not hypothetical — it is issue #964:
# c34bd01e6 and 18b3e2ee3 each added localization keys with ZERO files under clients/, both merged
# green, and the 5 resulting orphan keys survived on main until this change. Keeping the trigger
# aligned with the jobs' real inputs puts the cost back on the PR that created it.
#
# clients/react/src/ciTrigger.test.ts fails if a client test grows a NEW cross-tree read this list
# does not cover, so the trigger and the tests' real inputs cannot drift apart again.
#
# The two lists are spelled out rather than shared through a YAML anchor — the Actions workflow
# parser does not expand anchors/aliases. That same test asserts they stay identical.
on:
pull_request:
paths:
- "clients/**"
- ".github/workflows/clients.yml"
- "src/MeshWeaver.Messaging.Hub/Localization/**"
- "src/MeshWeaver.Messaging.Contract/Localization/Locales.cs"
- "src/MeshWeaver.Blazor/BlazorViewRegistry.cs"
- "src/MeshWeaver.Hosting.Grpc/Protos/**"
push:
branches: [main]
paths:
- "clients/**"
- ".github/workflows/clients.yml"
- "src/MeshWeaver.Messaging.Hub/Localization/**"
- "src/MeshWeaver.Messaging.Contract/Localization/Locales.cs"
- "src/MeshWeaver.Blazor/BlazorViewRegistry.cs"
- "src/MeshWeaver.Hosting.Grpc/Protos/**"

jobs:
react:
Expand Down
138 changes: 138 additions & 0 deletions clients/react/src/ciTrigger.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
import { lstatSync, readFileSync, readdirSync, statSync } from "node:fs";
import { resolve, sep } from "node:path";
import { describe, expect, it } from "vitest";

// GUARD THE GUARD (issue #964).
//
// Several client tests are deliberately CROSS-TREE: they read a file under src/ and assert the
// client copy matches it. That only works as a guard if the workflow that runs them is TRIGGERED by
// a change to the file they read. .github/workflows/clients.yml used to filter on "clients/**"
// alone, so every cross-tree guard was blind to the side that actually changes — a server-only PR
// added a key, CI stayed green, and the gap sat latent until an unrelated PR touched the client tree
// and inherited a red it did not cause.
//
// Widening the filter once fixes today's holes; this test is what stops the next one. Adding a new
// `../../src/...` read to a client package is itself a change under clients/**, so this job runs and
// fails until the path filter covers the new input.

// Two forms of the same directory: `repoDir` is native (the base for fs calls and resolve()),
// `repoRoot` is POSIX-normalized for the string comparisons — workflow `paths:` globs are always
// "/"-separated, so comparing them against a Windows "\" path would match nothing.
const repoDir = resolve(process.cwd(), "../..");
const repoRoot = repoDir.split(sep).join("/");
const workflow = readFileSync(resolve(repoDir, ".github/workflows/clients.yml"), "utf8");

/**
* The `paths:` list under each trigger. Parsed without a YAML dependency: find a `paths:` key, then
* take the `- "…"` items that follow at a deeper indent.
*/
function pathFilters(): string[][] {
const lines = workflow.split("\n");
const lists: string[][] = [];
for (let i = 0; i < lines.length; i++) {
const head = /^(\s*)paths:\s*$/.exec(lines[i]);
if (!head) continue;
const items: string[] = [];
for (let j = i + 1; j < lines.length; j++) {
const item = /^\s+-\s+"([^"]+)"\s*$/.exec(lines[j]);
if (!item) break;
items.push(item[1]);
}
lists.push(items);
}
return lists;
}

/** Repo-relative paths outside clients/ that a client source or test file reads at runtime. */
function crossTreeReads(): string[] {
const found = new Set<string>();
// A `../`-relative reference (import specifier, resolve() argument, buf/protoc CLI arg), and the
// `$repo_root/...` shape the shell codegen scripts use. Both are RESOLVED against the referencing
// file below — a literal "../../src/x" means clients/<pkg>/src/x in some packages and the repo's
// src/ in others, so matching the text alone would report the wrong thing either way.
const relRef = /(?:\.\.\/)+[A-Za-z0-9_.\-/]+/g;
const rootRef = /\$(?:repo_root|repoRoot)\/([A-Za-z0-9_.\-/]+)/g;

const walk = (dir: string, pkgRoot: string) => {
for (const entry of readdirSync(dir)) {
if (entry === "node_modules" || entry === "dist" || entry === ".next" || entry === "gen") continue;
const full = resolve(dir, entry);
// lstat, not stat: some client packages commit a node_modules SYMLINK, and descending through
// a link can walk straight out of the repo (or into a cycle). Links are skipped outright.
const st = lstatSync(full, { throwIfNoEntry: false });
if (!st || st.isSymbolicLink()) continue;
if (st.isDirectory()) {
walk(full, pkgRoot);
continue;
}
if (!/\.(ts|tsx|js|mjs|cjs|json|sh|yaml|yml)$/.test(entry)) continue;
const text = readFileSync(full, "utf8");
const keep = (abs: string) => {
// Compare in POSIX form — resolve() yields "\" on Windows, where a "/"-based startsWith
// would silently match nothing and report every input as uncovered.
const posix = abs.split(sep).join("/");
if (!posix.startsWith(`${repoRoot}/src/`)) return; // only reads that leave clients/
if (!statSync(abs, { throwIfNoEntry: false })) return; // and only ones that really resolve
found.add(posix.slice(repoRoot.length + 1));
};
// A `../` reference is written relative to the FILE (import specifier) or to the PACKAGE ROOT
// (`resolve(process.cwd(), …)`, an npm-script CLI arg). Try both bases and keep whichever
// actually lands on a file in src/ — that existence check is also what filters out the noise
// ("../..", "../../.." in prose) a purely textual match would report.
for (const m of text.matchAll(relRef)) {
keep(resolve(dir, m[0]));
keep(resolve(pkgRoot, m[0]));
}
for (const m of text.matchAll(rootRef)) keep(resolve(repoDir, m[1]));
}
};
// Each immediate child of clients/ is a package; its root is the base for cwd-relative reads.
const clientsDir = resolve(repoDir, "clients");
for (const pkg of readdirSync(clientsDir)) {
const pkgRoot = resolve(clientsDir, pkg);
if (lstatSync(pkgRoot, { throwIfNoEntry: false })?.isDirectory()) walk(pkgRoot, pkgRoot);
}
return [...found].sort();
}

/** Does a workflow `paths:` entry match this repo-relative file (or directory) path? */
function covers(filter: string, path: string): boolean {
if (filter.endsWith("/**")) {
const prefix = filter.slice(0, -3);
// A reference to the directory ITSELF (e.g. buf generate <dir>) is covered too.
return path === prefix || path.startsWith(`${prefix}/`);
}
return filter === path;
}

describe("clients.yml trigger covers every input the jobs read", () => {
it("declares a paths filter on both the pull_request and push triggers", () => {
const lists = pathFilters();
expect(lists).toHaveLength(2);
for (const list of lists) expect(list.length).toBeGreaterThan(2);
});

it("keeps the pull_request and push filters identical", () => {
// A filter that fires on the PR but not the push (or vice versa) is the same blind spot in half
// the pipeline — and the Actions parser cannot share them through a YAML anchor.
const [pr, push] = pathFilters();
expect(push).toEqual(pr);
});

it("scrapes a plausible set of cross-tree reads", () => {
// If the scrape breaks, the coverage assertion below would vacuously pass on an empty list.
const reads = crossTreeReads();
expect(reads.length).toBeGreaterThan(2);
expect(reads).toContain("src/MeshWeaver.Blazor/BlazorViewRegistry.cs");
});

it("every server file a client reads is in the trigger", () => {
const [filters] = pathFilters();
const uncovered = crossTreeReads().filter((p) => !filters.some((f) => covers(f, p)));
expect(
uncovered,
`clients.yml is not triggered by: ${uncovered.join(", ")} — a change to these would leave the ` +
`client guard latent. Add them to BOTH paths: lists in .github/workflows/clients.yml.`,
).toEqual([]);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
---
Name: Client text no longer lags behind the server
Category: What's New
Description: The check that keeps the JavaScript clients' copy of the portal's text in step with the server now runs whenever either side changes, instead of only when someone edited the client.
Icon: Globe
---

# Client text no longer lags behind the server

Every user-visible string is written once, on the server. The JavaScript clients carry a copy of
that text so they can put words on screen without waiting for a round trip. A check compares the
two and fails if they ever disagree.

The check only ran when someone edited the client. A string added on the server alone changed
nothing under the client folder, so the check never ran, and the copy silently fell behind — by
several dozen strings, including a sentence that had been reworded on the server and never
updated in the copy. Nothing warned about it, because the one thing that would have warned was
the thing that never ran. It went unnoticed long enough to happen three separate times.

The check now also runs when the server's text changes, so the two are compared whenever either
side moves. The change that introduces a gap is the one that has to close it, instead of the gap
being discovered later by whoever happens to touch the client next — which, until now, is how
every one of these was found.

The same blind spot applied to the checks that keep the clients' control set and wire format in
step with the server. Those run on both sides now too, and a further check fails if a client ever
starts reading a server file that this arrangement does not yet cover.
Loading