diff --git a/.dev/STATE.md b/.dev/STATE.md index dd81dc51..a67adced 100644 --- a/.dev/STATE.md +++ b/.dev/STATE.md @@ -10,8 +10,9 @@ Roadmap detail lives in [PLAN.md](PLAN.md), not here. - **Tech-debt Wave 1** (#158–#168, milestone 1) — A (#158, guardrails) and B (#159, init parity guard; corpus measured: 31/248 seeded pairs fail, 25 in intro.zh-cn) **merged**; C (#160, failure reporting — red-by-design: rebase can fail, bulk exits non-zero, - `--resume` retries errored) open. D–K follow in any order. Boundaries the wave must - respect: + `--resume` retries errored) **merged**; D (#161, canonical review-workflow template — + five of six copies could never fire) open. E–K follow in any order. Boundaries the wave + must respect: [`D-2026-07-24-tech-debt-audit-boundaries.md`](decisions/D-2026-07-24-tech-debt-audit-boundaries.md). - **PR #71** — Malayalam (`ml`) draft; awaiting native-reviewer calibration batch. Glossary PR **#69** (ja) open, awaiting native review + a `LANGUAGE_CONFIGS` entry. diff --git a/.dev/log/2026-07-24-pr-d-canonical-workflow.md b/.dev/log/2026-07-24-pr-d-canonical-workflow.md new file mode 100644 index 00000000..48bf3a9d --- /dev/null +++ b/.dev/log/2026-07-24-pr-d-canonical-workflow.md @@ -0,0 +1,26 @@ +# 2026-07-24 — #161 (PR D): one canonical review-workflow template + +The fleet-template PR. Six copies of the review workflow, five unfireable (no `labeled` +trigger — the label lands after `opened`, so the gate never passes; the v0.21.0 +six-PRs-merged-unreviewed shape). Fix is structural, not a six-way patch: +`examples/review-translations.yml` is canonical, `translate setup` renders it at runtime +(examples dir threaded from the entry point like the glossary dir — same Jest-CJS reason), +docs quote its shape, and `workflow-templates.test.ts` (branch-naming mold) fails when any +copy loses a structural line. Scaffolder also: `@v0` replaces the stale `v0.9.0` literals +(F27; and F137's release-checklist gap closes because there is nothing left to bump), +writes `rebase-translations.yml` verbatim from its canonical file, source workflow gains +`issue_comment` resync + composite guard (F91), `checkout@v7` (F135), and the +paths-filter normalizer gets its missing `g` flag (`./lectures/` emitted +`lectures//**/*.md`). `doctor` now warns on review workflows without `labeled` (F7's +second half). + +F127 landed as its docs half only — option (b), removing the dead review `target-language` +knob everywhere and documenting the suffix convention — because option (a) (honouring the +input) touches `inputs.ts`/`index.ts` and this PR is declared no-dist-action-rebuild. +If review mode ever honours the input, the docs come back with it; that is a deliberate +non-decision recorded here. + +Substitution contract worth knowing: `generateTargetWorkflowYaml` is the identity when +rendered with the template's own example values (tested), so the canonical file doubles as +a golden output; a template edit that breaks a substitution key throws at scaffold time +and fails the unit test. diff --git a/CHANGELOG.md b/CHANGELOG.md index 2688ebb5..877d9cc1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Changed +- **One canonical review-workflow template, and `translate setup` scaffolds repos that actually work** (#161, audit F7/F136/F27/F91/F135/F137): the review workflow existed in six divergent copies — the scaffolder plus five doc pages — and five of them could never fire on Action sync PRs, because the `action-translation` label is applied *after* the PR opens and `types: [opened, synchronize]` never sees it. That is the shape behind the v0.21.0 incident where six PRs of real translated content merged unreviewed. The scaffolder's copy was also pinned to `v0.9.0` (25 releases stale, the release checklist never knew to bump it) with no `permissions:` or `concurrency:` and `checkout@v4` against the docs' `@v7`. **Now**: `examples/review-translations.yml` is the single canonical template — `translate setup` renders it (substituting `source-repo`/`source-language`/`docs-folder`) and every doc page quotes its shape, locked by a drift test in the `branch-naming` mold that fails when any copy loses the `labeled`/`reopened` triggers, the label guard, `permissions`, or the per-PR `concurrency` group. The scaffolder emits the floating `@v0` (nothing left for a release to forget), also writes `rebase-translations.yml` from its canonical template, and its source workflow gains the `issue_comment` resync trigger and composite guard the docs teach — plus the `g`-flag fix in the paths-filter normalizer, which emitted a broken `lectures//**/*.md` for `./lectures/`. `translate doctor` now warns on a review workflow whose trigger list lacks `labeled` instead of passing the repo. +- **Review-mode docs stop teaching a dead knob** (#161, audit F127 remnant): review mode has never read a `target-language` input — it detects the language from the repository-name suffix and, since v0.23.0, warns when it can't. The input row, the quickstart line, and the scaffolded workflow key are gone; every review-workflow snippet now documents the suffix convention instead. (Honouring the input is a separate decision, deliberately not taken here.) Also corrected: the required-inputs table now admits `rebase` as a mode, the CLI reference no longer names a scaffold file (`translation-sync.yml`) that setup never wrote, and its `.translate/` samples show `` instead of a hard-coded `0.8.0`. - **Failure is no longer optional: rebase mode can fail, bulk runs exit non-zero, and `backward --resume` retries errored files** (#160, audit F37/F39/F66 — turns previously-green runs red *by design*; red here means failures that were always happening are now reported). Three paths reported success over failure. **Rebase mode could not fail**: `runRebase` counted per-PR errors and finished with an info line, so a run that failed every PR completed green; it now calls `core.setFailed` when any PR errored. Worse, `rebaseSinglePR` force-reset the PR branch *before* knowing translation succeeded — on partial failure it committed only the successful files onto the freshly-reset branch, silently dropping the errored files' previous translations from the PR, then commented "the translation content is preserved". It now throws **before** any branch reset, leaving the branch untouched, and returns a typed `rebased`/`skipped` outcome so skipped PRs are no longer counted — or commented on — as rebased. **Bulk CLI runs exited 0 however many files failed**: `backward` and `forward` bulk (including a `forward --github` wave where every push and PR creation failed) now exit 1 with a count when any file errored, as does single-file `forward`. The `backward` aggregate report gains optional `filesErrored` + `erroredFiles` fields (absent on pre-#160 reports, which still parse). **`backward --resume` treated errored files as done**, so a retry silently skipped exactly the files that needed retrying — a documented choice, deliberately reversed: resume now retries errored files and drops their stale error records, so repeated `--resume` converges on a clean run instead of a permanently partial one. - **Terminal failures now carry a stack trace, decoded through the committed sourcemap** (#160, audit F29): no handler logged `error.stack` anywhere — 116 catch sites discard it — so a crash in a 2.9 MB bundle reported one message with no pointer into the code. The Action's terminal handler now logs the stack, and a new `dist-action/run.cjs` entry shim (now `action.yml`'s `main:`) enables source-map support before the bundle is compiled, so frames decode to `src/` locations — measured: `src/inputs.ts:25` instead of `index.js:21610`. A shim is the only placement that works: a `node24` action's `runs:` block has no `env:` key for `NODE_OPTIONS`, and enabling inside the bundle is too late. The CLI's terminal catches print the stack (dimmed) below the error message. diff --git a/README.md b/README.md index c2693558..48744dea 100644 --- a/README.md +++ b/README.md @@ -42,7 +42,7 @@ jobs: (github.event_name == 'issue_comment' && contains(github.event.comment.body, '\translate-resync')) runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v7 with: fetch-depth: 2 - uses: quantecon/action-translation@v0 diff --git a/docs/user/action-reference.md b/docs/user/action-reference.md index efdb28fe..2fe9405c 100644 --- a/docs/user/action-reference.md +++ b/docs/user/action-reference.md @@ -44,7 +44,7 @@ Runs in the **target** (translated) repository, triggered when a translation PR | Input | Description | |-------|-------------| -| `mode` | Operation mode: `sync` or `review` | +| `mode` | Operation mode: `sync`, `review`, or `rebase` | | `anthropic-api-key` | Anthropic API key for Claude | | `github-token` | GitHub token for API access (cross-repo requires a PAT with `repo` scope) | @@ -70,12 +70,13 @@ Runs in the **target** (translated) repository, triggered when a translation PR |-------|---------|-------------| | `source-repo` | *(required)* | Source repository for English content (`owner/repo`) | | `source-language` | `en` | Source language code | -| `target-language` | *(required for review)* | Target language code | | `docs-folder` | `lectures/` | Documentation folder | | `max-suggestions` | `5` | Maximum findings in review comment | | `claude-model` | `claude-sonnet-5` | Claude model for review | | `auto-merge-mode` | `off` | `off` or `shadow`. Shadow records the would-auto-merge decision in the verdict block and outputs without acting on it. `active` is not implemented and fails loudly ([#103](https://github.com/QuantEcon/action-translation/issues/103)) | +There is no `target-language` input in review mode: the language is detected from the repository-name suffix (`lecture-python-intro.zh-cn` → `zh-cn`), which selects the glossary for terminology review. A repo whose name carries no language suffix logs a warning and reviews without a glossary. + ### Rebase mode inputs | Input | Default | Description | @@ -205,12 +206,26 @@ name: Review Translations on: pull_request: - types: [opened, synchronize] + types: [opened, synchronize, labeled, reopened] jobs: review: - if: contains(github.event.pull_request.labels.*.name, 'action-translation') + # `labeled` matters: the sync applies its labels after opening the PR. + # The second clause ignores `labeled` events for every other label. + if: > + contains(github.event.pull_request.labels.*.name, 'action-translation') && + (github.event.action != 'labeled' || github.event.label.name == 'action-translation') runs-on: ubuntu-latest + + permissions: + contents: read + pull-requests: write + + # One review per PR — supersede an in-flight review instead of running both + concurrency: + group: review-translations-${{ github.event.pull_request.number }} + cancel-in-progress: true + steps: - uses: actions/checkout@v7 with: @@ -221,7 +236,6 @@ jobs: mode: review source-repo: 'QuantEcon/lecture-python-intro' source-language: 'en' - target-language: 'zh-cn' docs-folder: 'lectures/' max-suggestions: 5 anthropic-api-key: ${{ secrets.ANTHROPIC_API_KEY }} diff --git a/docs/user/cli-reference.md b/docs/user/cli-reference.md index 90a222a3..3a5fdd1f 100644 --- a/docs/user/cli-reference.md +++ b/docs/user/cli-reference.md @@ -517,7 +517,7 @@ The `.translate/` folder in the target repo stores persistent sync metadata. It source-language: en target-language: zh-cn docs-folder: lectures -tool-version: 0.8.0 # Version that last wrote this config +tool-version: # Version that last wrote this config ``` Provides defaults so CLI flags don't need to be repeated every invocation. @@ -530,7 +530,7 @@ synced-at: "2026-03-06" # ISO date of last sync model: claude-sonnet-5 # Model used for translation mode: NEW # Translation mode: NEW / UPDATE / RESYNC section-count: 5 # Source section count at sync time -tool-version: 0.8.0 # Version that performed this sync +tool-version: # Version that performed this sync ``` ### How each command uses `.translate/` @@ -592,7 +592,7 @@ npx translate setup --source --target-language [options] 1. Derives target repo name: `{source-repo}.{lang}` (e.g., `lecture-python-intro.zh-cn`) 2. Creates GitHub repo via `gh repo create` and clones it locally -3. Writes `.translate/config.yml`, `.github/workflows/translation-sync.yml`, `.gitignore`, `README.md` +3. Writes `.translate/config.yml`, `.github/workflows/review-translations.yml`, `.github/workflows/rebase-translations.yml`, `.gitignore`, `README.md` 4. Makes initial commit and pushes **Requirements:** The `gh` CLI must be installed and authenticated (`gh auth login`). diff --git a/docs/user/quickstart.md b/docs/user/quickstart.md index d8ccd9d8..67a08d96 100644 --- a/docs/user/quickstart.md +++ b/docs/user/quickstart.md @@ -71,11 +71,15 @@ name: Review Translations on: pull_request: - types: [opened, synchronize] + types: [opened, synchronize, labeled, reopened] jobs: review: - if: contains(github.event.pull_request.labels.*.name, 'action-translation') + # `labeled` matters: the sync applies its labels after opening the PR. + # The second clause ignores `labeled` events for every other label. + if: > + contains(github.event.pull_request.labels.*.name, 'action-translation') && + (github.event.action != 'labeled' || github.event.label.name == 'action-translation') runs-on: ubuntu-latest permissions: @@ -97,13 +101,12 @@ jobs: mode: review source-repo: 'YourOrg/your-source-repo' source-language: 'en' - target-language: 'zh-cn' docs-folder: 'lectures/' anthropic-api-key: ${{ secrets.ANTHROPIC_API_KEY }} github-token: ${{ secrets.GITHUB_TOKEN }} ``` -This posts an AI-generated quality review comment on each translation PR, including a translation score, diff quality score, and improvement suggestions. +This posts an AI-generated quality review comment on each translation PR, including a translation score, diff quality score, and improvement suggestions. There is no `target-language` input in review mode — the language is detected from the repository-name suffix (`your-repo.zh-cn` → `zh-cn`). ## What happens next diff --git a/docs/user/tutorials/add-language.md b/docs/user/tutorials/add-language.md index ae2828e5..ee619a62 100644 --- a/docs/user/tutorials/add-language.md +++ b/docs/user/tutorials/add-language.md @@ -284,11 +284,15 @@ name: Review Translations on: pull_request: - types: [opened, synchronize] + types: [opened, synchronize, labeled, reopened] jobs: review: - if: contains(github.event.pull_request.labels.*.name, 'action-translation') + # `labeled` matters: the sync applies its labels after opening the PR. + # The second clause ignores `labeled` events for every other label. + if: > + contains(github.event.pull_request.labels.*.name, 'action-translation') && + (github.event.action != 'labeled' || github.event.label.name == 'action-translation') runs-on: ubuntu-latest permissions: contents: read @@ -306,12 +310,13 @@ jobs: mode: review source-repo: 'QuantEcon/lecture-python-intro' source-language: 'en' - target-language: 'ja' docs-folder: 'lectures' anthropic-api-key: ${{ secrets.ANTHROPIC_API_KEY }} github-token: ${{ secrets.GITHUB_TOKEN }} ``` +There is no `target-language` input in review mode — the language is detected from the repository-name suffix (`lecture-python-intro.ja` → `ja`). + ### Secrets Add these secrets to the relevant repos: diff --git a/docs/user/tutorials/connect-existing.md b/docs/user/tutorials/connect-existing.md index 2b0def39..08d4b4d7 100644 --- a/docs/user/tutorials/connect-existing.md +++ b/docs/user/tutorials/connect-existing.md @@ -434,12 +434,13 @@ jobs: mode: review source-repo: 'QuantEcon/lecture-python-intro' source-language: 'en' - target-language: 'zh-cn' docs-folder: 'lectures' anthropic-api-key: ${{ secrets.ANTHROPIC_API_KEY }} github-token: ${{ secrets.GITHUB_TOKEN }} ``` +There is no `target-language` input here: review mode detects the language from the repository-name suffix (`lecture-python-intro.zh-cn` → `zh-cn`). A repo without a language suffix logs a warning and reviews without a glossary. + :::{tip} The `labeled` event type is important — without it, the workflow won't trigger if the `action-translation` label is added after the PR is opened. The `github.event.label.name` check keeps it from firing again for each *other* label the sync applies, and the `concurrency` group collapses the `opened` and `labeled` events a single sync produces into one review. Without both, one sync bills several reviews of the same diff ([#96](https://github.com/QuantEcon/action-translation/issues/96)). ::: diff --git a/docs/user/tutorials/fresh-setup.md b/docs/user/tutorials/fresh-setup.md index 55ae30b2..e6e48e22 100644 --- a/docs/user/tutorials/fresh-setup.md +++ b/docs/user/tutorials/fresh-setup.md @@ -317,11 +317,15 @@ name: Review Translations on: pull_request: - types: [opened, synchronize] + types: [opened, synchronize, labeled, reopened] jobs: review: - if: contains(github.event.pull_request.labels.*.name, 'action-translation') + # `labeled` matters: the sync applies its labels after opening the PR. + # The second clause ignores `labeled` events for every other label. + if: > + contains(github.event.pull_request.labels.*.name, 'action-translation') && + (github.event.action != 'labeled' || github.event.label.name == 'action-translation') runs-on: ubuntu-latest permissions: @@ -343,13 +347,12 @@ jobs: mode: review source-repo: 'QuantEcon/lecture-python-intro' source-language: 'en' - target-language: 'zh-cn' docs-folder: 'lectures' anthropic-api-key: ${{ secrets.ANTHROPIC_API_KEY }} github-token: ${{ secrets.GITHUB_TOKEN }} ``` -Add the `ANTHROPIC_API_KEY` secret to the **target** repo as well (for the review workflow). +There is no `target-language` input in review mode — the language is detected from the repository-name suffix (`lecture-python-intro.zh-cn` → `zh-cn`). Add the `ANTHROPIC_API_KEY` secret to the **target** repo as well (for the review workflow). ### 4d: Target repo — Secret diff --git a/examples/review-translations.yml b/examples/review-translations.yml new file mode 100644 index 00000000..f9be895c --- /dev/null +++ b/examples/review-translations.yml @@ -0,0 +1,54 @@ +# Review Translation PRs +# +# Install this workflow in the TARGET (translated) repository at: +# .github/workflows/review-translations.yml +# +# It posts an AI quality review on every translation PR the sync opens. +# The review's target language is not an input: it is detected from the +# repository-name suffix (lecture-python-intro.zh-cn -> zh-cn). +# +# This file is the single canonical template: `translate setup` scaffolds +# new target repos from it (substituting source-repo, source-language and +# docs-folder), and the docs quote the same shape. A drift test +# (src/cli/__tests__/workflow-templates.test.ts) keeps every copy aligned. + +name: Review Translations + +on: + pull_request: + types: [opened, synchronize, labeled, reopened] + +jobs: + review: + # `labeled` in the trigger list matters: the sync applies its labels after + # opening the PR, so without it a PR labeled late is never reviewed. The + # second clause then ignores `labeled` events for every OTHER label — a + # sync applies labels one call at a time, and each would otherwise start + # a full (billed) review of the same diff (#96). + if: > + contains(github.event.pull_request.labels.*.name, 'action-translation') && + (github.event.action != 'labeled' || github.event.label.name == 'action-translation') + runs-on: ubuntu-latest + + permissions: + contents: read + pull-requests: write + + # One review per PR — supersede an in-flight review instead of running both + concurrency: + group: review-translations-${{ github.event.pull_request.number }} + cancel-in-progress: true + + steps: + - uses: actions/checkout@v7 + with: + fetch-depth: 2 + + - uses: QuantEcon/action-translation@v0 + with: + mode: review + source-repo: 'QuantEcon/lecture-python-intro' + source-language: 'en' + docs-folder: 'lectures' + anthropic-api-key: ${{ secrets.ANTHROPIC_API_KEY }} + github-token: ${{ secrets.GITHUB_TOKEN }} diff --git a/src/cli/__tests__/doctor.test.ts b/src/cli/__tests__/doctor.test.ts index 1dd3ba67..4a814ba2 100644 --- a/src/cli/__tests__/doctor.test.ts +++ b/src/cli/__tests__/doctor.test.ts @@ -331,6 +331,89 @@ describe('checkWorkflow', () => { expect(result.status).toBe('warn'); expect(result.message).toContain('none reference action-translation'); }); + + test('warns when the review workflow cannot fire on sync PRs (#161)', () => { + const workflowDir = path.join(tmpDir, '.github', 'workflows'); + fs.mkdirSync(workflowDir, { recursive: true }); + // The pre-#161 scaffold: label-gated but no `labeled` trigger, so the + // gate always sees an unlabeled PR and the review never runs. + fs.writeFileSync( + path.join(workflowDir, 'review-translations.yml'), + [ + 'on:', + ' pull_request:', + ' types: [opened, synchronize]', + 'jobs:', + ' review:', + " if: contains(github.event.pull_request.labels.*.name, 'action-translation')", + ' steps:', + ' - uses: QuantEcon/action-translation@v0', + ' with:', + ' mode: review', + ].join('\n'), + 'utf-8' + ); + + const result = checkWorkflow(tmpDir); + expect(result.status).toBe('warn'); + expect(result.message).toContain('cannot fire'); + expect(result.details?.join('\n')).toContain('review-translations.yml'); + }); + + test('is not fooled by an if-guard that mentions labeled without the trigger', () => { + const workflowDir = path.join(tmpDir, '.github', 'workflows'); + fs.mkdirSync(workflowDir, { recursive: true }); + // A half-fixed workflow: carries the label guard in `if:` (which mentions + // 'labeled') but never added `labeled` to the trigger types. + fs.writeFileSync( + path.join(workflowDir, 'review-translations.yml'), + [ + 'on:', + ' pull_request:', + ' types: [opened, synchronize]', + 'jobs:', + ' review:', + ' if: >', + " contains(github.event.pull_request.labels.*.name, 'action-translation') &&", + " (github.event.action != 'labeled' || github.event.label.name == 'action-translation')", + ' steps:', + ' - uses: QuantEcon/action-translation@v0', + ' with:', + ' mode: review', + ].join('\n'), + 'utf-8' + ); + + const result = checkWorkflow(tmpDir); + expect(result.status).toBe('warn'); + expect(result.message).toContain('cannot fire'); + }); + + test('accepts the dash-list trigger form', () => { + const workflowDir = path.join(tmpDir, '.github', 'workflows'); + fs.mkdirSync(workflowDir, { recursive: true }); + fs.writeFileSync( + path.join(workflowDir, 'review-translations.yml'), + 'on:\n pull_request:\n types:\n - opened\n - labeled\nuses: QuantEcon/action-translation@v0\nmode: review\n', + 'utf-8' + ); + + const result = checkWorkflow(tmpDir); + expect(result.status).toBe('pass'); + }); + + test('passes a review workflow with the labeled trigger', () => { + const workflowDir = path.join(tmpDir, '.github', 'workflows'); + fs.mkdirSync(workflowDir, { recursive: true }); + fs.writeFileSync( + path.join(workflowDir, 'review-translations.yml'), + 'on:\n pull_request:\n types: [opened, synchronize, labeled, reopened]\nuses: QuantEcon/action-translation@v0\nmode: review\n', + 'utf-8' + ); + + const result = checkWorkflow(tmpDir); + expect(result.status).toBe('pass'); + }); }); // ============================================================================ diff --git a/src/cli/__tests__/setup.test.ts b/src/cli/__tests__/setup.test.ts index f8c464fc..62ff9f6e 100644 --- a/src/cli/__tests__/setup.test.ts +++ b/src/cli/__tests__/setup.test.ts @@ -12,6 +12,7 @@ import { deriveTargetRepoName, generateSourceWorkflowYaml, generateTargetWorkflowYaml, + loadWorkflowTemplate, runSetup, SetupOptions, GhRunner, @@ -19,6 +20,10 @@ import { } from '../commands/setup.js'; import { readConfig } from '../translate-state.js'; +/** The packaged canonical templates — what the CLI entry threads in. */ +const EXAMPLES_DIR = path.join(__dirname, '..', '..', '..', 'examples'); +const REVIEW_TEMPLATE = loadWorkflowTemplate(EXAMPLES_DIR, 'review-translations.yml'); + // ============================================================================ // SETUP // ============================================================================ @@ -62,74 +67,96 @@ describe('deriveTargetRepoName', () => { // ============================================================================ describe('generateSourceWorkflowYaml', () => { - test('generates pull_request:closed trigger for source repo', () => { + test('generates a workflow that can fire on merges and resync comments', () => { const yaml = generateSourceWorkflowYaml( 'QuantEcon/lecture-python-intro.zh-cn', 'zh-cn', - 'lectures', - '0.8.0' + 'lectures' ); expect(yaml).toContain('name: Sync Translations'); expect(yaml).toContain('pull_request:'); expect(yaml).toContain('types: [closed]'); + expect(yaml).toContain('issue_comment:'); + expect(yaml).toContain('types: [created]'); + expect(yaml).toContain("contains(github.event.comment.body, '\\translate-resync')"); + expect(yaml).toContain('github.event.pull_request.merged == true'); expect(yaml).toContain('paths:'); expect(yaml).toContain("'lectures/**/*.md'"); expect(yaml).toContain('target-repo: QuantEcon/lecture-python-intro.zh-cn'); expect(yaml).toContain('target-language: zh-cn'); expect(yaml).toContain('docs-folder: lectures'); - expect(yaml).toContain('QuantEcon/action-translation@v0.8.0'); + expect(yaml).toContain('QuantEcon/action-translation@v0'); + // The floating tag, not a pinned @v0.x — the old scaffold shipped @v0.9.0. + expect(yaml).not.toMatch(/action-translation@v0\.\d/); + expect(yaml).toContain('actions/checkout@v7'); expect(yaml).toContain('mode: sync'); expect(yaml).toContain('${{ secrets.TRANSLATION_PAT }}'); expect(yaml).not.toContain('repository_dispatch'); }); test('uses custom docs-folder in paths filter', () => { - const yaml = generateSourceWorkflowYaml('Owner/repo.fa', 'fa', 'docs', '1.0.0'); + const yaml = generateSourceWorkflowYaml('Owner/repo.fa', 'fa', 'docs'); expect(yaml).toContain("'docs/**/*.md'"); expect(yaml).toContain('docs-folder: docs'); }); - test('normalizes root-level docs-folder in paths filter', () => { - const yaml1 = generateSourceWorkflowYaml('Owner/repo.fa', 'fa', '.', '1.0.0'); - expect(yaml1).toContain("'**/*.md'"); - expect(yaml1).not.toContain("'./**/*.md'"); - - const yaml2 = generateSourceWorkflowYaml('Owner/repo.fa', 'fa', '/', '1.0.0'); - expect(yaml2).toContain("'**/*.md'"); + test('normalizes root-level and decorated docs-folders in paths filter', () => { + for (const root of ['.', '/', '']) { + const yaml = generateSourceWorkflowYaml('Owner/repo.fa', 'fa', root); + expect(yaml).toContain("'**/*.md'"); + expect(yaml).not.toContain("'./**/*.md'"); + } + // `./lectures/` used to emit a broken `lectures//**/*.md` — the strip + // regex was missing its `g` flag, so only one decoration came off. + for (const decorated of ['lectures', 'lectures/', 'lectures//', './lectures/', '/lectures/']) { + const yaml = generateSourceWorkflowYaml('Owner/repo.fa', 'fa', decorated); + expect(yaml).toContain("'lectures/**/*.md'"); + expect(yaml).not.toContain('//'); + } }); }); describe('generateTargetWorkflowYaml', () => { - test('generates review workflow for target repo', () => { + test('renders the canonical template with substituted inputs', () => { const yaml = generateTargetWorkflowYaml( - 'QuantEcon/lecture-python-intro', - 'zh-cn', - 'lectures', - '0.8.0' + REVIEW_TEMPLATE, + 'QuantEcon/lecture-python-programming', + 'docs', + 'fr' ); expect(yaml).toContain('name: Review Translations'); - expect(yaml).toContain('pull_request:'); - expect(yaml).toContain('action-translation'); + expect(yaml).toContain('types: [opened, synchronize, labeled, reopened]'); + expect(yaml).toContain("github.event.label.name == 'action-translation'"); + expect(yaml).toContain('pull-requests: write'); + expect(yaml).toContain('group: review-translations-${{ github.event.pull_request.number }}'); expect(yaml).toContain('mode: review'); - expect(yaml).toContain('source-repo: QuantEcon/lecture-python-intro'); - expect(yaml).toContain('source-language: en'); - expect(yaml).toContain('target-language: zh-cn'); - expect(yaml).toContain('QuantEcon/action-translation@v0.8.0'); - expect(yaml).toContain('${{ secrets.ANTHROPIC_API_KEY }}'); - expect(yaml).toContain('${{ secrets.GITHUB_TOKEN }}'); + expect(yaml).toContain("source-repo: 'QuantEcon/lecture-python-programming'"); + expect(yaml).toContain("source-language: 'fr'"); + expect(yaml).toContain("docs-folder: 'docs'"); + expect(yaml).toContain('QuantEcon/action-translation@v0'); + expect(yaml).not.toMatch(/action-translation@v0\.\d/); + expect(yaml).toContain('actions/checkout@v7'); + // Review mode has no target-language input — it detects from the repo suffix. + expect(yaml).not.toContain('target-language'); }); - test('uses custom docs-folder', () => { - const yaml = generateTargetWorkflowYaml('Owner/repo', 'fa', 'docs', '1.0.0'); - expect(yaml).toContain('docs-folder: docs'); + test('rendering the template with its own example values is the identity', () => { + const yaml = generateTargetWorkflowYaml( + REVIEW_TEMPLATE, + 'QuantEcon/lecture-python-intro', + 'lectures', + 'en' + ); + expect(yaml).toBe(REVIEW_TEMPLATE); }); - test('uses provided source-language', () => { - const yaml = generateTargetWorkflowYaml('Owner/repo', 'zh-cn', 'lectures', '1.0.0', 'ja'); - expect(yaml).toContain('source-language: ja'); - expect(yaml).not.toContain('source-language: en'); + test('throws when the template loses a substitution key', () => { + const broken = REVIEW_TEMPLATE.replace(/^\s*source-repo:.*$/m, ''); + expect(() => generateTargetWorkflowYaml(broken, 'Owner/repo', 'lectures')).toThrow( + /no `source-repo:` line/ + ); }); }); @@ -146,6 +173,7 @@ describe('runSetup dry-run', () => { docsFolder: 'lectures', visibility: 'public', dryRun: true, + examplesDir: EXAMPLES_DIR, }; const result = await runSetup(options); @@ -155,6 +183,7 @@ describe('runSetup dry-run', () => { expect(result.repoFullName).toBe('QuantEcon/lecture-python-intro.zh-cn'); expect(result.filesCreated).toContain('.translate/config.yml'); expect(result.filesCreated).toContain('.github/workflows/review-translations.yml'); + expect(result.filesCreated).toContain('.github/workflows/rebase-translations.yml'); // No actual files created in dry run expect(fs.existsSync(path.join(result.localPath, '.translate'))).toBe(false); @@ -192,6 +221,7 @@ describe('runSetup with mock runners', () => { docsFolder: 'lectures', visibility: 'public', dryRun: false, + examplesDir: EXAMPLES_DIR, }; // Override cwd so path.resolve lands inside tmpDir @@ -214,8 +244,17 @@ describe('runSetup with mock runners', () => { const workflowPath = path.join(repoDir, '.github', 'workflows', 'review-translations.yml'); expect(fs.existsSync(workflowPath)).toBe(true); const workflowContent = fs.readFileSync(workflowPath, 'utf-8'); - expect(workflowContent).toContain('source-repo: QuantEcon/lecture-python-intro'); + expect(workflowContent).toContain("source-repo: 'QuantEcon/lecture-python-intro'"); expect(workflowContent).toContain('mode: review'); + expect(workflowContent).toContain('types: [opened, synchronize, labeled, reopened]'); + + // The rebase workflow is written verbatim from the canonical template + const rebasePath = path.join(repoDir, '.github', 'workflows', 'rebase-translations.yml'); + expect(fs.existsSync(rebasePath)).toBe(true); + expect(fs.readFileSync(rebasePath, 'utf-8')).toBe( + loadWorkflowTemplate(EXAMPLES_DIR, 'rebase-translations.yml') + ); + expect(result.filesCreated).toContain('.github/workflows/rebase-translations.yml'); // Check .gitignore expect(fs.existsSync(path.join(repoDir, '.gitignore'))).toBe(true); @@ -243,6 +282,7 @@ describe('runSetup with mock runners', () => { docsFolder: 'lectures', visibility: 'public', dryRun: false, + examplesDir: EXAMPLES_DIR, }; const origCwd = process.cwd(); @@ -268,6 +308,7 @@ describe('runSetup with mock runners', () => { docsFolder: 'lectures', visibility: 'public', dryRun: false, + examplesDir: EXAMPLES_DIR, }; const origCwd = process.cwd(); @@ -307,6 +348,7 @@ describe('runSetup with mock runners', () => { visibility: 'public', dryRun: false, sourceWorkflow: sourceWorkflowPath, + examplesDir: EXAMPLES_DIR, }; const origCwd = process.cwd(); diff --git a/src/cli/__tests__/workflow-templates.test.ts b/src/cli/__tests__/workflow-templates.test.ts new file mode 100644 index 00000000..9e20d3d6 --- /dev/null +++ b/src/cli/__tests__/workflow-templates.test.ts @@ -0,0 +1,81 @@ +/** + * Drift guard for the review-workflow template (#161 — audit F7/F136). + * + * The review workflow existed in six divergent copies, five of which could + * never fire on Action sync PRs: the `action-translation` label is applied + * after the PR opens, so `types: [opened, synchronize]` never sees it. That + * is the shape behind the v0.21.0 incident where six PRs of real translated + * content merged unreviewed. examples/review-translations.yml is now the one + * canonical template — the scaffolder renders it and the docs quote it — and + * this test fails loudly when any copy drifts. Same mold as + * branch-naming.test.ts's guard over examples/rebase-translations.yml. + */ + +import * as fs from 'fs'; +import * as path from 'path'; + +const ROOT = path.join(__dirname, '..', '..', '..'); +const CANONICAL = fs.readFileSync(path.join(ROOT, 'examples', 'review-translations.yml'), 'utf8'); + +/** + * The lines that make the workflow fireable and safe. A copy missing any of + * them regresses to the unfireable (or multi-billed, or unauthorized) shape. + */ +const STRUCTURAL_LINES = [ + 'types: [opened, synchronize, labeled, reopened]', + "contains(github.event.pull_request.labels.*.name, 'action-translation')", + "github.event.action != 'labeled' || github.event.label.name == 'action-translation'", + 'pull-requests: write', + 'group: review-translations-${{ github.event.pull_request.number }}', + 'cancel-in-progress: true', + 'actions/checkout@v7', + 'QuantEcon/action-translation@v0', +]; + +/** The floating tag, not a pinned @v0.x — the old scaffold shipped @v0.9.0. */ +const PINNED_TAG = /action-translation@v0\.\d/; + +/** Doc pages that quote the review workflow. */ +const DOC_PAGES = [ + 'docs/user/quickstart.md', + 'docs/user/action-reference.md', + 'docs/user/tutorials/fresh-setup.md', + 'docs/user/tutorials/add-language.md', + 'docs/user/tutorials/connect-existing.md', +]; + +describe('the canonical review workflow template', () => { + it('carries every structural line', () => { + for (const line of STRUCTURAL_LINES) { + expect(CANONICAL).toContain(line); + } + }); + + it('has no target-language input — review mode detects it from the repo suffix', () => { + expect(CANONICAL).not.toContain('target-language'); + }); + + it('uses the floating @v0 tag, not a pinned version', () => { + expect(CANONICAL).not.toMatch(PINNED_TAG); + }); +}); + +describe('every documented copy of the review workflow', () => { + it.each(DOC_PAGES)('%s carries every structural line', (page) => { + const content = fs.readFileSync(path.join(ROOT, page), 'utf8'); + const missing = STRUCTURAL_LINES.filter((line) => !content.includes(line)); + expect(missing).toEqual([]); + expect(content).not.toMatch(PINNED_TAG); + }); + + it.each(DOC_PAGES)('%s does not re-teach the dead review target-language knob', (page) => { + const content = fs.readFileSync(path.join(ROOT, page), 'utf8'); + // target-language legitimately appears in sync-mode blocks; the review + // block is identified by its mode line and scanned to the fence close. + for (const match of content.matchAll(/^\s*mode: review$/gm)) { + const rest = content.slice(match.index); + const block = rest.slice(0, rest.indexOf('```')); + expect(block).not.toContain('target-language'); + } + }); +}); diff --git a/src/cli/commands/doctor.ts b/src/cli/commands/doctor.ts index 714f7956..1011c0b4 100644 --- a/src/cli/commands/doctor.ts +++ b/src/cli/commands/doctor.ts @@ -314,6 +314,33 @@ export function checkWorkflow(targetPath: string): CheckResult { }; } + // A review workflow whose trigger list lacks `labeled` can never fire on + // Action sync PRs: the `action-translation` label is applied after the PR + // opens, so the label gate always sees an unlabeled PR (#161 — the shape + // behind six PRs of real translated content merging unreviewed in v0.21.0). + const unfireable = translationWorkflows.filter((f) => { + const content = fs.readFileSync(path.join(workflowDir, f), 'utf-8'); + if (!content.includes('mode: review')) return false; + // `labeled` must appear in a trigger `types:` list (inline or dash form) — + // an `if:` guard mentioning 'labeled' does not make the workflow fireable. + const triggerHasLabeled = + /types:\s*\[[^\]]*\blabeled\b[^\]]*\]/.test(content) || /^\s*-\s*labeled\b/m.test(content); + return !triggerHasLabeled; + }); + if (unfireable.length > 0) { + return { + name: 'Workflow', + status: 'warn', + message: `Review workflow cannot fire on sync PRs — trigger list lacks \`labeled\``, + details: [ + ...unfireable.map((f) => `Unfireable: ${f}`), + 'The action-translation label is applied after the PR opens, so', + '`types: [opened, synchronize]` never sees it. Update the workflow from', + 'examples/review-translations.yml in the action-translation repo.', + ], + }; + } + return { name: 'Workflow', status: 'pass', diff --git a/src/cli/commands/setup.ts b/src/cli/commands/setup.ts index ac217555..d699ac7d 100644 --- a/src/cli/commands/setup.ts +++ b/src/cli/commands/setup.ts @@ -30,6 +30,7 @@ export interface SetupOptions { visibility: 'public' | 'private'; // Repo visibility (default: "public") dryRun: boolean; // Preview without creating sourceWorkflow?: string; // Path to write source workflow file (optional) + examplesDir: string; // Packaged canonical workflow templates (/examples) } export interface SetupResult { @@ -122,20 +123,29 @@ function extractOwner(sourceRepo: string): string { * Triggers on merged PRs that touch docs files — creates translation PRs in TARGET. */ +// Strip leading `./`//`/` and trailing slashes from a docs-folder value. +// The `g` flag and `\/+` matter: without them only one decoration comes off, +// and `./lectures/` or `lectures//` leaves a broken double slash behind. +function trimDocsFolder(docsFolder: string): string { + return docsFolder.replace(/^\.?\/+|\/+$/g, ''); +} + // Normalize docsFolder for workflow path filters. // When docs are at repo root, emit '**/*.md' instead of broken patterns. function normalizePathsFilter(docsFolder: string): string { - const trimmed = docsFolder.replace(/^\.?\/+|\/$/, ''); + const trimmed = trimDocsFolder(docsFolder); return trimmed === '' || trimmed === '.' ? '**/*.md' : `${trimmed}/**/*.md`; } export function generateSourceWorkflowYaml( targetRepo: string, targetLanguage: string, - docsFolder: string, - actionVersion: string + docsFolder: string ): string { const pathsFilter = normalizePathsFilter(docsFolder); + // Echo the cleaned value into the input line too — a decorated folder like + // `lectures//` should not survive into the scaffolded workflow. + docsFolder = trimDocsFolder(docsFolder) || '.'; return `# Auto-generated by \`translate setup\` # Place this file in the SOURCE repository: .github/workflows/sync-translations.yml name: Sync Translations @@ -146,18 +156,24 @@ on: paths: - '${pathsFilter}' - '_toc.yml' + issue_comment: + types: [created] jobs: sync: - if: github.event.pull_request.merged == true + # Merged PRs sync; the issue_comment trigger lets \`\\translate-resync\` + # on a merged PR retry a failed sync. + if: > + (github.event_name == 'pull_request' && github.event.pull_request.merged == true) || + (github.event_name == 'issue_comment' && contains(github.event.comment.body, '\\translate-resync')) runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v7 with: fetch-depth: 2 - - uses: QuantEcon/action-translation@v${actionVersion} + - uses: QuantEcon/action-translation@v0 with: mode: sync target-repo: ${targetRepo} @@ -169,43 +185,44 @@ jobs: } /** - * Generate the TARGET repository review workflow YAML. - * Triggers on translation PRs (labeled action-translation) — posts quality review. + * Read a canonical workflow template from the packaged examples/ directory. + * The templates are the single source the docs quote and the drift test locks. + */ +export function loadWorkflowTemplate(examplesDir: string, name: string): string { + const templatePath = path.join(examplesDir, name); + try { + return fs.readFileSync(templatePath, 'utf-8'); + } catch { + throw new Error(`Cannot read canonical workflow template ${templatePath}`); + } +} + +/** + * Render the TARGET repository review workflow from the canonical template + * (examples/review-translations.yml), substituting the per-repo inputs. + * The target language is deliberately NOT an input: review mode detects it + * from the repository-name suffix. */ export function generateTargetWorkflowYaml( + template: string, sourceRepo: string, - targetLanguage: string, docsFolder: string, - actionVersion: string, sourceLanguage: string = 'en' ): string { - return `# Auto-generated by \`translate setup\` -name: Review Translations - -on: - pull_request: - types: [opened, synchronize] - -jobs: - review: - if: contains(github.event.pull_request.labels.*.name, 'action-translation') - runs-on: ubuntu-latest - - steps: - - uses: actions/checkout@v4 - with: - fetch-depth: 2 - - - uses: QuantEcon/action-translation@v${actionVersion} - with: - mode: review - source-repo: ${sourceRepo} - source-language: ${sourceLanguage} - target-language: ${targetLanguage} - docs-folder: ${docsFolder} - anthropic-api-key: \${{ secrets.ANTHROPIC_API_KEY }} - github-token: \${{ secrets.GITHUB_TOKEN }} -`; + const substitutions: Array<[string, string]> = [ + ['source-repo', sourceRepo], + ['source-language', sourceLanguage], + ['docs-folder', docsFolder], + ]; + let rendered = template; + for (const [key, value] of substitutions) { + const pattern = new RegExp(`^(\\s*${key}:).*$`, 'm'); + if (!pattern.test(rendered)) { + throw new Error(`Canonical review workflow template has no \`${key}:\` line to substitute`); + } + rendered = rendered.replace(pattern, `$1 '${value}'`); + } + return rendered; } // ============================================================================ @@ -254,6 +271,7 @@ export async function runSetup( console.log(` Target repo files:`); console.log(` .translate/config.yml`); console.log(` .github/workflows/review-translations.yml`); + console.log(` .github/workflows/rebase-translations.yml`); console.log(` .gitignore`); console.log(` README.md`); if (options.sourceWorkflow) { @@ -268,6 +286,7 @@ export async function runSetup( filesCreated: [ '.translate/config.yml', '.github/workflows/review-translations.yml', + '.github/workflows/rebase-translations.yml', '.gitignore', 'README.md', ], @@ -275,6 +294,12 @@ export async function runSetup( }; } + // The canonical workflow templates ship in /examples — resolved + // by the entry point (import.meta cannot load under the Jest CJS registry, so + // the directory is threaded in as an option, like the glossary dir). + const reviewTemplate = loadWorkflowTemplate(options.examplesDir, 'review-translations.yml'); + const rebaseTemplate = loadWorkflowTemplate(options.examplesDir, 'rebase-translations.yml'); + // Step 1: Create GitHub repo and clone console.log(`\n📦 Creating repository ${targetFullName}…`); const createResult = ghRunner([ @@ -314,10 +339,9 @@ export async function runSetup( const workflowDir = path.join(localPath, '.github', 'workflows'); fs.mkdirSync(workflowDir, { recursive: true }); const targetWorkflowContent = generateTargetWorkflowYaml( + reviewTemplate, options.source, - options.targetLanguage, - options.docsFolder, - '0.9.0', + trimDocsFolder(options.docsFolder) || '.', options.sourceLanguage ); fs.writeFileSync( @@ -327,6 +351,11 @@ export async function runSetup( ); filesCreated.push('.github/workflows/review-translations.yml'); + // .github/workflows/rebase-translations.yml (TARGET repo) — verbatim from + // the canonical template; it has no per-repo values to substitute. + fs.writeFileSync(path.join(workflowDir, 'rebase-translations.yml'), rebaseTemplate, 'utf-8'); + filesCreated.push('.github/workflows/rebase-translations.yml'); + // .gitignore fs.writeFileSync(path.join(localPath, '.gitignore'), generateGitignore(), 'utf-8'); filesCreated.push('.gitignore'); @@ -369,8 +398,7 @@ export async function runSetup( const sourceWorkflowContent = generateSourceWorkflowYaml( targetFullName, options.targetLanguage, - options.docsFolder, - '0.9.0' + options.docsFolder ); const sourceWorkflowPath = path.resolve(options.sourceWorkflow); fs.mkdirSync(path.dirname(sourceWorkflowPath), { recursive: true }); @@ -391,7 +419,7 @@ export async function runSetup( console.log(` gh secret set ANTHROPIC_API_KEY -R ${targetFullName}`); if (!options.sourceWorkflow) { console.log(` 4. Add sync workflow to SOURCE repo:`); - console.log(` Copy the workflow template from docs/user/cli-reference.md into`); + console.log(` Copy the workflow template from docs/user/quickstart.md into`); console.log(` ${options.source}/.github/workflows/sync-translations.yml`); console.log(` Or re-run setup with --source-workflow on the initial invocation.`); } diff --git a/src/cli/index.ts b/src/cli/index.ts index b0272cc5..48b6a381 100644 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -59,6 +59,9 @@ const { version } = require('../../package.json'); // commands as an option and the resolution logic lives in a testable module. const CLI_DIR = path.dirname(fileURLToPath(import.meta.url)); const BUILT_IN_GLOSSARY_DIR = path.resolve(CLI_DIR, '..', '..', 'glossary'); +// Canonical workflow templates (/examples) — threaded into +// `setup` the same way, and for the same Jest-CJS reason, as the glossary dir. +const EXAMPLES_DIR = path.resolve(CLI_DIR, '..', '..', 'examples'); const program = new Command(); @@ -605,6 +608,7 @@ program visibility: opts.visibility, dryRun: opts.dryRun, sourceWorkflow: opts.sourceWorkflow, + examplesDir: EXAMPLES_DIR, }; try {