diff --git a/.github/workflows/translation-validation.yml b/.github/workflows/translation-validation.yml index 558ca08e8..010a76343 100644 --- a/.github/workflows/translation-validation.yml +++ b/.github/workflows/translation-validation.yml @@ -19,12 +19,41 @@ jobs: # TODO: pin actions to full commit SHAs (supply-chain hardening). - uses: actions/checkout@v4 with: + # Full history so the gate can read source and translations at the + # base and tell an introduced violation from inherited staleness. + fetch-depth: 0 persist-credentials: false - uses: actions/setup-node@v4 with: node-version: 20 + - name: Determine base ref + id: base + # Pass the (attacker-influenceable) ref names through env, never inline + # into the shell, to avoid command injection via a crafted branch name. + env: + EVENT_NAME: ${{ github.event_name }} + BASE_REF: ${{ github.base_ref }} + EVENT_BEFORE: ${{ github.event.before }} + run: | + if [ "$EVENT_NAME" = "pull_request" ]; then + git fetch --no-tags --quiet origin "$BASE_REF" + echo "ref=origin/$BASE_REF" >> "$GITHUB_OUTPUT" + elif git rev-parse --verify --quiet "$EVENT_BEFORE^{commit}" >/dev/null; then + echo "ref=$EVENT_BEFORE" >> "$GITHUB_OUTPUT" + else + # A branch-creation push sends the all-zero SHA, and a force-push can + # leave the previous tip unreachable. Compare against the preceding + # commit rather than failing the job for a history reason. + echo "ref=HEAD^" >> "$GITHUB_OUTPUT" + fi - name: Validate protected terminology - run: node scripts/check-protected-terms.mjs + # Scoped to what this change did to translations. Inherited staleness + # (English edited, translations not yet re-synced) is reported as a + # notice and tracked on the translation staleness dashboard instead of + # reddening every unrelated PR. + env: + BASE_REF_OUT: ${{ steps.base.outputs.ref }} + run: node scripts/check-protected-terms.mjs --base "$BASE_REF_OUT" hash-lib-tests: runs-on: ubuntu-latest diff --git a/scripts/check-protected-terms.mjs b/scripts/check-protected-terms.mjs index 8dff8ab28..0d840d1f3 100644 --- a/scripts/check-protected-terms.mjs +++ b/scripts/check-protected-terms.mjs @@ -1,7 +1,8 @@ // Validate that protected Zcash ecosystem terms are preserved verbatim in // translated wiki pages. // -// Usage: node scripts/check-protected-terms.mjs (run from the content repo root) +// Usage: node scripts/check-protected-terms.mjs [--base ] +// (run from the content repo root) // // This script is locale-generic. It walks the translations/ tree for whatever // locale folders exist (translations//site/...), maps each translated @@ -10,6 +11,32 @@ // // It passes cleanly (exit 0) when there are no translations yet, so the // tooling is self-consistent before any translated corpus lands. +// +// ---- Scope (--base) ------------------------------------------------------- +// +// Without --base every violation anywhere in the tree is fatal. That is right +// for a local audit but wrong for a PR gate: the wiki's English pages are +// edited continuously by many contributors, and each new protected term lands +// in `site/` long before the 18 translations catch up. A whole-tree gate turns +// that unavoidable lag into a red X on every open PR, including PRs that touch +// no translation at all — so the signal stops meaning anything and maintainers +// learn to merge through it. +// +// With --base, this gate polices what a change actually did to TRANSLATIONS, +// and leaves English-source drift to its owner: the staleness detector +// (translation/detect-staleness.mjs) and its dashboard issue. A violation is +// fatal only when the change introduced it into a translated file: +// +// introduced translation changed in this range and did not violate at the +// base (includes a newly added translation) -> FAIL +// stale-source translation untouched; the English source moved under it +// -> warn (this is staleness, tracked on the dashboard) +// pre-existing already violating at the base -> warn +// +// Mirrors the base-ref handling of translation/check-invariants.mjs, including +// its fail-closed behaviour: a base that was REQUESTED but cannot be resolved +// is an error, never a silent skip. +import { execFileSync } from "node:child_process"; import { existsSync, readdirSync, readFileSync, statSync } from "node:fs"; import { join, relative } from "node:path"; @@ -20,6 +47,21 @@ const config = JSON.parse( const translationsDir = join(root, "translations"); +function resolveBase() { + const i = process.argv.indexOf("--base"); + if (i >= 0) { + const explicit = process.argv[i + 1]; + // An explicitly passed but empty/absent value must not quietly degrade to a + // whole-tree audit: that changes what the gate means without saying so. + if (!explicit || explicit.startsWith("--")) { + die('--base requires a ref. Omit --base entirely for a whole-tree audit.'); + } + return explicit; + } + if (process.env.GITHUB_BASE_REF) return `origin/${process.env.GITHUB_BASE_REF}`; + return null; +} + // Collect every translated Markdown file under translations//site/... function walk(dir) { let out = []; @@ -45,8 +87,19 @@ function sourceForTranslation(absTranslated) { return sourceRel; } +function termRegExp(term) { + // Word-boundary match so short terms don't false-positive inside other + // words (e.g. "mining" must not match "deter*mining*"; "chain" is still + // satisfied by "block*chain*" only via the standalone token's boundaries). + const escaped = term.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + return new RegExp(`(? path the content lived at in the base, for pages moved in this +// range. Without this a moved translation looks brand-new, so drift it merely +// carried across the move would be blamed on the move. +const renamedFrom = new Map(); +if (mergeBase) { + let changed; + try { + changed = execFileSync("git", ["diff", "--name-status", "-M", "-z", `${mergeBase}..HEAD`], { cwd: root, encoding: "utf8", maxBuffer: 64 * 1024 * 1024 }); + } catch (e) { + die(`git diff against base ${mergeBase.slice(0, 12)} failed: ${e.message}`); + } + // -z --name-status is a NUL stream: STATUS \0 PATH \0 for ordinary changes, + // and R### \0 OLD \0 NEW \0 for renames/copies. + const fields = changed.split("\0"); + changedTranslations = new Set(); + for (let i = 0; i < fields.length; i += 1) { + const status = fields[i]; + if (!status) continue; + if (status[0] === "R" || status[0] === "C") { + const from = fields[i + 1]; + const to = fields[i + 2]; + i += 2; + if (!to) continue; + renamedFrom.set(to, from); + if (to.startsWith("translations/") && to.endsWith(".md")) changedTranslations.add(to); + } else { + const path = fields[i + 1]; + i += 1; + if (path && path.startsWith("translations/") && path.endsWith(".md")) changedTranslations.add(path); + } + } + // Pass 1 reads the WORKING TREE, so on a local run an uncommitted or untracked + // translation is invisible to the range diff and would read as inherited + // staleness. Fold both in. No effect in CI, where the checkout is clean. + for (const args of [ + ["diff", "--name-only", "-z", "HEAD", "--", "translations/"], + ["ls-files", "--others", "--exclude-standard", "-z", "--", "translations/"], + ]) { + try { + const out = execFileSync("git", args, { cwd: root, encoding: "utf8", maxBuffer: 64 * 1024 * 1024 }); + for (const p of out.split("\0")) if (p.endsWith(".md")) changedTranslations.add(p); + } catch { /* non-fatal: scope stays range-only */ } + } +} + +// translations//site/ -> site/, on repo-relative paths. +// String-only counterpart of sourceForTranslation(), for paths that exist at +// the base rather than on disk. +function sourceForTranslationRel(rel) { + const parts = rel.split("/"); + if (parts.length < 4 || parts[0] !== "translations" || parts[2] !== "site") return null; + return ["site", ...parts.slice(3)].join("/"); +} + +// Base file contents, cached: one `git show` per distinct path, not per term. +// Takes the path AS IT WAS at the base — callers resolve renames first. +const baseFileCache = new Map(); +function baseContentAt(basePath) { + if (baseFileCache.has(basePath)) return baseFileCache.get(basePath); + let content = null; // null = absent at the base + try { + content = execFileSync("git", ["show", `${mergeBase}:${basePath}`], { cwd: root, encoding: "utf8", maxBuffer: 64 * 1024 * 1024 }); + } catch { + content = null; + } + baseFileCache.set(basePath, content); + return content; +} + +function classify(v) { + if (!mergeBase) return "introduced"; // no base requested → whole-tree audit + // Judge pre-existence against the pairing that ACTUALLY existed at the base: + // the translated file's base path, and the English source that that path + // derives from. Pairing the old content with the CURRENT path's source + // invents a comparison that never existed — moving a translation into a + // different page's slot would then read every freshly created violation as + // pre-existing, because the new slot's English source legitimately carries + // terms the moved content never had to. + const baseTranslatedPath = renamedFrom.get(v.translatedRel) ?? v.translatedRel; + const baseSourcePath = sourceForTranslationRel(baseTranslatedPath); + const baseTranslated = baseContentAt(baseTranslatedPath); + const baseSource = baseSourcePath === null ? null : baseContentAt(baseSourcePath); + // A translation (or its source) absent at the base is new here, so it cannot + // be pre-existing. + const violatedAtBase = + baseTranslated !== null && + baseSource !== null && + termRegExp(v.term).test(baseSource) && + !termRegExp(v.term).test(baseTranslated); + if (violatedAtBase) return "pre-existing"; + return changedTranslations.has(v.translatedRel) ? "introduced" : "stale-source"; +} + +const buckets = { introduced: [], "stale-source": [], "pre-existing": [] }; +for (const v of violations) buckets[classify(v)].push(v); + +for (const v of buckets.introduced) { + console.error(`${v.translatedRel}: missing protected term "${v.term}"`); +} + +if (mergeBase) { + const carried = buckets["pre-existing"].length + buckets["stale-source"].length; + if (carried) { + console.log( + `\nnotice: ${carried} pre-existing violation(s) not caused by this change ` + + `(${buckets["pre-existing"].length} already failing at the base, ` + + `${buckets["stale-source"].length} from English pages edited since a translation was last synced). ` + + `These are translation staleness, tracked on the staleness dashboard — not a defect in this change.`, + ); + const sample = [...buckets["stale-source"], ...buckets["pre-existing"]].slice(0, 10); + for (const v of sample) console.log(` - ${v.translatedRel}: "${v.term}"`); + if (carried > sample.length) console.log(` … and ${carried - sample.length} more`); + + // Violations a change carried forward in a file it edited anyway. Deliberately + // not fatal: the miss came from English drift, and failing here would put the + // 18-locale sync burden on whoever touches the file next — the exact blocking + // this scoping removes. Surfaced separately so it is a visible, easy fix for + // an author already in that file rather than something the notice buries. + const touchedCarried = buckets["pre-existing"].filter((v) => changedTranslations.has(v.translatedRel)); + if (touchedCarried.length) { + console.log( + `\nnotice: ${touchedCarried.length} of those are in translated file(s) this change edits — ` + + `not required, but cheap to fix while you are in there:`, + ); + for (const v of touchedCarried.slice(0, 10)) console.log(` - ${v.translatedRel}: "${v.term}"`); + if (touchedCarried.length > 10) console.log(` … and ${touchedCarried.length - 10} more`); } } } -if (failures > 0) { +if (buckets.introduced.length > 0) { console.error( - `Protected terminology validation failed with ${failures} missing term(s).`, + `Protected terminology validation failed with ${buckets.introduced.length} missing term(s)` + + (mergeBase ? " introduced by this change." : "."), ); process.exit(1); } diff --git a/translation/check-invariants.mjs b/translation/check-invariants.mjs index 763d51154..5d3751123 100644 --- a/translation/check-invariants.mjs +++ b/translation/check-invariants.mjs @@ -21,9 +21,15 @@ // // Change-tracking (only when a base ref is available) // - If a translations//…md file changed vs the base ref, its -// manifest entry must have changed too (src/mode/tool), or the change -// must declare edited: true. You cannot silently mutate a translation -// without recording why. +// manifest entry must have changed too. You cannot silently mutate a +// translation without recording why. +// - Normally you record the pass in `tool`, which is free-form +// (e.g. "gpt-5.4+linkrepair"). Reach for `edited: true` ONLY for a +// human-authored translation fix you want protected from machine +// re-translation: it takes the page out of automated sync permanently +// (sync.mjs holds edited:true pages forever), so using it for a +// mechanical pass silently freezes that page against every future +// re-sync. A bulk mechanical fix should never flip it. // // Freshness declaration (always) // - If an entry claims freshness (src == current normalized source hash), @@ -249,7 +255,7 @@ if (!base) { const editFlipped = now.edited === true && was?.edited !== true; const provenanceChanged = !was || now.src !== was.src || now.mode !== was.mode || now.tool !== was.tool; if (!provenanceChanged && !editFlipped) { - fail(`${loc}/${page}: translation changed but manifest provenance did not (record the new src/mode/tool, or flip edited:true this PR for a deliberate hand-edit)`); + fail(`${loc}/${page}: translation changed but manifest provenance did not — record the pass in \`tool\` (free-form, e.g. "${now.tool || "gpt-5.4"}+linkrepair"). Only flip edited:true for a human-authored fix you want protected from machine re-translation: it removes the page from automated sync permanently.`); } }