diff --git a/.github/workflows/uncurated-page-advisory.yml b/.github/workflows/uncurated-page-advisory.yml new file mode 100644 index 000000000..16910fc28 --- /dev/null +++ b/.github/workflows/uncurated-page-advisory.yml @@ -0,0 +1,158 @@ +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, + }); + // '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) => ['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 + // of the curated English corpus — the detector excludes it too. + .filter((f) => !f.startsWith('site/zechubglobal/')) + .map((f) => f.slice('site/'.length)); + + // 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(); + if (added.length) 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, + }); + // 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 + // 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; + } + + // 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', + '', + `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`; }