From 5c2d3d5d26f5839e3234836110897915eeabb2f8 Mon Sep 17 00:00:00 2001 From: rt Date: Tue, 25 Aug 2026 16:54:56 +0200 Subject: [PATCH 1/2] i18n: surface uncurated English pages instead of burying them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six English pages sat untranslated in all 18 locales for up to three weeks and nothing said so. Two of them were noticed by a human browsing the live site. The detector was not silent — that was my first diagnosis and it was wrong. It already rendered an "Uncurated source pages" section. The problem is where: line 122 of a 133-line dashboard issue, below a Details list that runs one row per stale pair (126 of them that day), and absent from the summary table at the top. Technically visible, reliably unseen. The console line had the same shape: `Uncurated source pages: 6 — e.g. `, which reads as a footnote rather than a work item. Worse, when the corpus is otherwise clean the dashboard said: ✅ All 3654 curated (page × locale) pairs are fresh. Nothing to sync. True and misleading at once. A page that was never curated cannot be stale or missing, so it is counted in none of the columns — the all-clear is measured over exactly the set that excludes the problem. ## Changes * `uncurated` is now a column in the summary table, with a callout under it explaining that those pages are counted in none of the other columns and why. * the all-fresh banner no longer implies there is no work when uncurated pages exist; it states both facts. * the section itself now says what the state costs (untranslated in every locale, English served) and how to change it (add to curated-pages.txt → shows up as `missing` × 18 → next sync fills it). * the console summary prints the FULL list, not `e.g.` plus three. ## New: advisory PR comment (.github/workflows/uncurated-page-advisory.yml) Comments on a PR that adds English pages missing from the curated set, so the debt is visible while someone is still looking at the PR. It is explicitly ADVISORY: it never fails a check, and the comment opens by telling the contributor nothing is wrong with their PR. There is deliberately no rule that every new page must be curated. Curation is a lasting editorial commitment — the wiki then maintains that page in 18 languages — and it is not a drive-by contributor's call to make. A blocking gate would also be worse than useless: the fix that makes CI green is to add the line, which creates 18 items of real work with nobody having decided the page deserves them. That is precisely how a backlog grows silently. `curated ⊆ site` is the invariant; `site ⊆ curated` is not, by design. Security: it uses pull_request_target, because most PRs here come from forks and a plain `pull_request` token cannot comment on those. That is only safe if PR code never runs, so the job has ONE step, no checkout of anything, and no `run:` step at all. The PR is read purely as data through the API — the file list, and the text of curated-pages.txt at the head sha. The header comment says not to add a PR checkout; please keep it that way. It reads curated-pages.txt at the HEAD sha, not base, so a PR that adds a page AND curates it in the same change stays silent. It upserts a single comment, so repeated pushes edit rather than pile up, and it withdraws its own warning if the pages get curated later in the PR's life. Verified against the six currently-open PRs: five stay silent (they only edit existing pages) and exactly one fires — #1930, which adds Zcash_Tech/Project_Tachyon.md. That is a real 7th uncurated page about to land with nothing today that would report it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01YEgDv6GG6Eybs1gu2mhcgY --- .github/workflows/uncurated-page-advisory.yml | 143 ++++++++++++++++++ translation/detect-staleness.mjs | 39 ++++- 2 files changed, 176 insertions(+), 6 deletions(-) create mode 100644 .github/workflows/uncurated-page-advisory.yml diff --git a/.github/workflows/uncurated-page-advisory.yml b/.github/workflows/uncurated-page-advisory.yml new file mode 100644 index 000000000..4d5fe1f1f --- /dev/null +++ b/.github/workflows/uncurated-page-advisory.yml @@ -0,0 +1,143 @@ +name: Uncurated page advisory + +# Advisory ONLY. Comments on a PR that adds English pages which are not in the +# translation curated set, so the translation backlog it creates is visible while +# someone is still looking at the PR — instead of surfacing weeks later on the +# dashboard, or not at all. +# +# This deliberately does NOT fail the PR, and there is deliberately no rule that +# every new page must be curated. Curation is an editorial commitment (ZecHub +# then maintains that page in 18 languages, indefinitely) and it is not the +# contributor's call. A blocking check would also be worse than useless: faced +# with a red gate, the fix that makes CI pass is to add the line, which creates +# 18 items of real work with nobody having decided the page deserves them. That +# is how a backlog grows silently, which is the exact problem this is meant to +# expose. `curated ⊆ site` is the invariant; `site ⊆ curated` is not, by design +# (see translation/check-invariants.mjs). +# +# SECURITY — why pull_request_target, and why it is safe here: +# Most PRs to this repo come from forks, and a plain `pull_request` trigger gets +# a read-only token for those, so it cannot comment. pull_request_target runs +# with the base repo's token, which is only safe if PR code never executes. This +# workflow therefore: +# * checks out nothing from the PR (no `ref:` override, no PR checkout at all), +# * runs no build, no install, no script from the PR, +# * reads the PR only as DATA through the API — the file list, and the text of +# translation/curated-pages.txt at the head sha. +# Do not add a checkout of the PR head or any step that runs PR-authored code. + +on: + pull_request_target: + types: [opened, synchronize, reopened] + branches: [main] + paths: + - "site/**/*.md" + +permissions: + contents: read + pull-requests: write + +concurrency: + group: uncurated-advisory-${{ github.event.pull_request.number }} + cancel-in-progress: true + +jobs: + advise: + runs-on: ubuntu-latest + steps: + - uses: actions/github-script@v7 + with: + script: | + const marker = ''; + const pr = context.payload.pull_request; + const { owner, repo } = context.repo; + + // 1. Which English pages does this PR ADD? Renames and edits do not + // create translation debt; only new pages do. + const files = await github.paginate(github.rest.pulls.listFiles, { + owner, repo, pull_number: pr.number, per_page: 100, + }); + const added = files + .filter((f) => f.status === 'added') + .map((f) => f.filename) + .filter((f) => f.startsWith('site/') && f.endsWith('.md')) + // zechubglobal/** is hand-maintained per-language content, not part + // of the curated English corpus — the detector excludes it too. + .filter((f) => !f.startsWith('site/zechubglobal/')) + .map((f) => f.slice('site/'.length)); + + if (added.length === 0) { + core.info('no newly added English pages — nothing to advise'); + return; + } + + // 2. Read the curated list AT THE PR HEAD, not at base: if the PR + // already curates the pages it adds, there is nothing to say. This + // is a data read, not code execution. + let curated = new Set(); + try { + const res = await github.rest.repos.getContent({ + owner: pr.head.repo.owner.login, + repo: pr.head.repo.name, + path: 'translation/curated-pages.txt', + ref: pr.head.sha, + }); + const text = Buffer.from(res.data.content, 'base64').toString('utf8'); + curated = new Set(text.split('\n').map((l) => l.trim()).filter(Boolean)); + } catch (e) { + // Fall back to the base copy. Worst case we advise about a page the + // PR already curated, which is noise — never a failure. + core.info(`could not read curated-pages.txt at head (${e.status || e.message}); using base`); + const res = await github.rest.repos.getContent({ + owner, repo, path: 'translation/curated-pages.txt', ref: pr.base.sha, + }); + const text = Buffer.from(res.data.content, 'base64').toString('utf8'); + curated = new Set(text.split('\n').map((l) => l.trim()).filter(Boolean)); + } + + const uncurated = added.filter((p) => !curated.has(p)).sort(); + + // 3. Upsert one comment, so a synchronize event edits rather than piles on. + const comments = await github.paginate(github.rest.issues.listComments, { + owner, repo, issue_number: pr.number, per_page: 100, + }); + const existing = comments.find((c) => c.body && c.body.includes(marker)); + + if (uncurated.length === 0) { + // Everything added here is curated. If we advised earlier in this + // PR's life, say so rather than leaving a stale warning standing. + if (existing) { + await github.rest.issues.updateComment({ + owner, repo, comment_id: existing.id, + body: `${marker}\n✅ All English pages added by this PR are in \`translation/curated-pages.txt\`. Nothing outstanding for translation.`, + }); + } + core.info('all added pages are curated'); + return; + } + + const list = uncurated.map((p) => `- \`${p}\``).join('\n'); + const body = [ + marker, + '### 🌐 Translation: no action required from you', + '', + `This PR adds ${uncurated.length} English page(s) that are **not** in the translation curated set:`, + '', + list, + '', + 'That is a perfectly valid state — the wiki serves English for uncurated pages, and not every page is meant to exist in 18 languages. **This comment does not block merging and nothing is wrong with your PR.**', + '', + '
For maintainers: how to translate these', + '', + 'Add the path(s) to `translation/curated-pages.txt`. Each page then appears as `missing` × 18 locales on the [staleness dashboard](../issues/1889), and the next translation sync fills it.', + '', + 'Curating is a lasting commitment — ZecHub maintains that page in every locale from then on — so it is a deliberate editorial choice, which is why it is not automated and not enforced.', + '
', + ].join('\n'); + + if (existing) { + await github.rest.issues.updateComment({ owner, repo, comment_id: existing.id, body }); + } else { + await github.rest.issues.createComment({ owner, repo, issue_number: pr.number, body }); + } + core.info(`advised on ${uncurated.length} uncurated page(s)`); diff --git a/translation/detect-staleness.mjs b/translation/detect-staleness.mjs index 0acdf993d..4bf19348d 100644 --- a/translation/detect-staleness.mjs +++ b/translation/detect-staleness.mjs @@ -275,7 +275,12 @@ else { } } } -if (uncurated.length) console.log(`Uncurated source pages (not in any locale): ${uncurated.length} — e.g. ${uncurated.slice(0, 3).join(", ")}`); +if (uncurated.length) { + // Print the FULL list, not `e.g.` + three. The truncated form is how six pages + // hid for three weeks: the line looked like a note rather than a work item. + console.log(`Uncurated source pages (not curated, untranslated in all locales): ${uncurated.length}`); + for (const p of uncurated) console.log(` - ${p}`); +} // Reporting tool: always succeed. process.exit(0); @@ -287,13 +292,30 @@ function renderMarkdown(r) { let md = `${marker}\n# 🌐 Translation staleness dashboard\n\n`; md += `_Auto-generated by \`translation/detect-staleness.mjs\`. Do not edit by hand — edits are overwritten._\n\n`; if (r.generatedAgainstCommit) md += `Against \`${r.generatedAgainstCommit.slice(0, 12)}\` · ${t.curatedPages} curated pages × ${t.locales} locales.\n\n`; + const nUncurated = r.uncuratedSourcePages.length; if (clean) { - md += `✅ **All ${t.pairs} curated (page × locale) pairs are fresh.** Nothing to sync.\n`; - if (r.uncuratedSourcePages.length) md += renderUncurated(r); + // "Nothing to sync" was true and misleading at the same time: a page that was + // never curated cannot be stale or missing, so the dashboard read all-clear + // while six English pages sat untranslated for three weeks. State both facts. + md += `✅ **All ${t.pairs} curated (page × locale) pairs are fresh.** Nothing to sync.\n\n`; + if (nUncurated) { + md += `⚠️ **${nUncurated} English page(s) are not curated**, so they are invisible to `; + md += `every count above — untranslated, and not reported as missing. See below.\n`; + } + if (nUncurated) md += renderUncurated(r); return md; } - md += `| | stale | missing | orphan | decurated | high-severity |\n|---|---:|---:|---:|---:|---:|\n`; - md += `| **total** | ${t.stale} | ${t.missing} | ${t.orphan} | ${t.decurated ?? 0} | ${t.highSeverity} |\n\n`; + // `uncurated` belongs in the summary table, not only in a section at the bottom. + // It was already rendered — as the LAST section, under a details list that can + // run to hundreds of rows — so it was technically visible and never seen. A + // number in the first screenful is the whole fix. + md += `| | stale | missing | orphan | decurated | uncurated | high-severity |\n|---|---:|---:|---:|---:|---:|---:|\n`; + md += `| **total** | ${t.stale} | ${t.missing} | ${t.orphan} | ${t.decurated ?? 0} | ${nUncurated} | ${t.highSeverity} |\n\n`; + if (nUncurated) { + md += `> ⚠️ **${nUncurated} uncurated English page(s)** — untranslated in all ${t.locales} locales and `; + md += `counted in none of the columns above, because a page that was never curated cannot be `; + md += `stale or missing. [Jump to the list](#uncurated-source-pages).\n\n`; + } if (t.highSeverity) { md += `## 🚨 High-severity (safety-critical — out-of-band sync)\n\n`; for (const f of r.findings.filter((x) => x.highSeverity)) { @@ -324,7 +346,12 @@ function renderMarkdown(r) { function renderUncurated(r) { if (!r.uncuratedSourcePages.length) return ""; - let md = `## Uncurated source pages\n\nIn \`site/\` but not in \`curated-pages.txt\` (candidates to add, or intentionally out of scope — the frontend serves English for these):\n\n`; + let md = `## Uncurated source pages\n\n`; + md += `In \`site/\` but not in \`curated-pages.txt\`. These are **untranslated in every locale** and the `; + md += `frontend serves English for them. That is legal and sometimes intended — but nothing else `; + md += `reports them, so each one is either a deliberate scope decision or a silent gap.\n\n`; + md += `To translate one: add its path to \`translation/curated-pages.txt\`. It then shows up as `; + md += `\`missing\` × ${r.totals.locales} and the next sync fills it.\n\n`; for (const p of r.uncuratedSourcePages) md += `- \`${p}\`\n`; return md + `\n`; } From cab79332cf3e38d7f6d250fc5a041456336a1c76 Mon Sep 17 00:00:00 2001 From: rt Date: Tue, 25 Aug 2026 18:50:48 +0200 Subject: [PATCH 2/2] i18n: fix four advisory-workflow defects found by adversarial review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three reviewers (Fable, gpt-5.5, Zabaniya) all cleared the part I most wanted challenged: the `pull_request_target` surface is safe as written — one github-script step, no checkout, no `run:`, and the only `${{ }}` interpolation anywhere is `pull_request.number` (an integer). PR content is read purely as API data. Two of them verified independently that the getContent fallback cannot post a wrong comment, that the >1MB `content`-omission case is ~150x away for a 7KB file and degrades safely regardless, and that the concurrency group cannot produce duplicate or misdirected comments. Four real defects, each fixed and re-verified by execution: 1. Renames were invisible. The filter only accepted `status === 'added'`, but a PR that MOVES drafts/X.md to site/X.md creates exactly the same translation debt and GitHub reports it as `renamed`. Now accepts added/renamed/copied. Confirmed against real PR data that this adds no noise: the same 5 of 6 open PRs stay silent, the same one fires. 2. The comment lookup matched on the marker alone, so anyone could pre-post a comment containing `` and the job would try to edit it — failing instead of advising, or rewriting someone else's comment. Now requires the marker AND a github-actions bot author. 3. A stale warning could stand forever. If a later push dropped the added pages while still touching site/**, `if (added.length === 0) return` fired BEFORE the branch that rewrites the comment to "nothing outstanding". The PR then permanently carried an advisory listing pages it no longer adds. The early return is gone; the flow falls through to the withdraw branch. 4. Filenames are attacker-controlled and were interpolated straight into a markdown code span. A backtick in a path closes the span and lets the rest render as markdown — an @mention in a filename would ping people. Verified: "x`@everyone`.md" escapes the span raw and does not after stripping. Also noted by review and deliberately left: the advisory hardcodes a link to issue 1889, which goes stale if that dashboard issue is ever closed and recreated. Not worth a config surface for an advisory comment. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01YEgDv6GG6Eybs1gu2mhcgY --- .github/workflows/uncurated-page-advisory.yml | 31 ++++++++++++++----- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/.github/workflows/uncurated-page-advisory.yml b/.github/workflows/uncurated-page-advisory.yml index 4d5fe1f1f..16910fc28 100644 --- a/.github/workflows/uncurated-page-advisory.yml +++ b/.github/workflows/uncurated-page-advisory.yml @@ -57,8 +57,12 @@ jobs: const files = await github.paginate(github.rest.pulls.listFiles, { owner, repo, pull_number: pr.number, per_page: 100, }); + // 'renamed' and 'copied' introduce a page at a NEW path just as much as + // 'added' does — a PR moving drafts/X.md to site/X.md creates exactly the + // same translation debt, and GitHub reports it as renamed, not added. + // Missing those meant the advisory stayed silent for the move case. const added = files - .filter((f) => f.status === 'added') + .filter((f) => ['added', 'renamed', 'copied'].includes(f.status)) .map((f) => f.filename) .filter((f) => f.startsWith('site/') && f.endsWith('.md')) // zechubglobal/** is hand-maintained per-language content, not part @@ -66,16 +70,17 @@ jobs: .filter((f) => !f.startsWith('site/zechubglobal/')) .map((f) => f.slice('site/'.length)); - if (added.length === 0) { - core.info('no newly added English pages — nothing to advise'); - return; - } + // NOTE: no early return here. A later push can drop the added pages while + // still touching site/**, and returning at this point left the earlier + // warning standing on a PR that no longer adds anything. Fall through so + // the withdraw-the-warning branch below runs. + if (added.length === 0) core.info('no newly added English pages'); // 2. Read the curated list AT THE PR HEAD, not at base: if the PR // already curates the pages it adds, there is nothing to say. This // is a data read, not code execution. let curated = new Set(); - try { + if (added.length) try { const res = await github.rest.repos.getContent({ owner: pr.head.repo.owner.login, repo: pr.head.repo.name, @@ -101,7 +106,13 @@ jobs: const comments = await github.paginate(github.rest.issues.listComments, { owner, repo, issue_number: pr.number, per_page: 100, }); - const existing = comments.find((c) => c.body && c.body.includes(marker)); + // Match on the marker AND on authorship. Marker-only matching let anyone + // pre-post a comment containing the marker and have this job try to edit + // it — at best the job fails instead of advising, at worst it rewrites + // someone else's comment. + const existing = comments.find((c) => + c.body && c.body.includes(marker) && + c.user && c.user.type === 'Bot' && c.user.login.startsWith('github-actions')); if (uncurated.length === 0) { // Everything added here is curated. If we advised earlier in this @@ -116,7 +127,11 @@ jobs: return; } - const list = uncurated.map((p) => `- \`${p}\``).join('\n'); + // Backticks in a filename would close the code span and let the rest render + // as markdown (an @mention in a path would ping people). Strip them; these + // are attacker-controlled strings rendered into a comment. + const safe = (t) => String(t).replace(/`/g, ''); + const list = uncurated.map((p) => `- \`${safe(p)}\``).join('\n'); const body = [ marker, '### 🌐 Translation: no action required from you',