Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Fixed

- **Sync mode fires only for PRs merged into the default branch** (surfaced by QuantEcon/lecture-python-programming#629): the published sync workflow triggered on `pull_request: types: [closed]` with no `branches:` filter, and the job's `merged == true` guard never looked at the base branch — so a theme-migration PR merged into the long-lived `jb2` branch was forwarded to every target repo, opening translation PRs in lecture-python-programming.fr, .fa and .zh-cn for content the English edition does not publish. Two fixes, both needed. Every published copy of the sync trigger — README, `examples/`, quickstart, action-reference, the three tutorials, the scaffolder in `src/cli/commands/setup.ts`, and the E2E harness template — now carries `branches: [main]`, which stops the run before it starts. And the action itself skips a merged PR whose base branch is not the repository's default branch, on both the `pull_request` path and the `\translate-resync` path: an `issue_comment` trigger cannot carry a branch filter, and deployed workflows copied before this change carry none, so the in-action check is the backstop for both. The comparison is against `repository.default_branch`, not a hard-coded `main`, so a `master` edition is unaffected; an unreadable default branch or base ref warns and proceeds rather than silencing production on an unfamiliar payload. The workflow-template sweep in `workflow-templates.test.ts` now requires the filter on every sync job it finds, so a doc page added later is covered the day it lands. Deployed source repos still need the one-line workflow edit — this ships the template and the backstop.

## [0.28.0] - 2026-09-03

### Added
Expand Down
5 changes: 4 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ A GitHub Action and CLI tool for managing translations of MyST Markdown document
## What it does

**GitHub Action** — Runs in your CI pipeline with three modes:
- **Sync mode**: When a PR is merged in the English source repo, automatically translates changed sections and opens a PR in the target language repo.
- **Sync mode**: When a PR is merged into the default branch of the English source repo, automatically translates changed sections and opens a PR in the target language repo.
- **Review mode**: When a translation PR is opened, posts an AI quality review with scores and suggestions.
- **Rebase mode**: When a translation PR merges in the target repo, automatically rebases its open siblings, reusing cached translations for unchanged sections.
Comment thread
mmcky marked this conversation as resolved.

Expand All @@ -31,6 +31,7 @@ name: Sync Translations
on:
pull_request:
types: [closed]
branches: [main]
paths: ['lectures/**/*.md']
issue_comment:
types: [created]
Expand Down Expand Up @@ -64,6 +65,8 @@ jobs:

The `issue_comment` trigger enables the `\translate-resync` command — comment it on any merged PR to re-trigger sync (useful for recovering from failures). To retrigger only one language, add the code: `\translate-resync fa` or `\translate-resync zh-cn`. The run costs Anthropic credits and uses the target-repo PAT, so the `if:` admits only repo owners, org members and collaborators; comments from anyone else are ignored at the workflow level and never start a job.

`branches: [main]` limits the merge trigger to PRs whose base is `main`. Without it, `types: [closed]` fires for a PR merged into *any* branch, and a merge into a long-lived work branch would open translation PRs for content the published edition does not carry. The action also checks the base branch against the repository's default branch, so a `\translate-resync` on such a PR is a no-op.

### CLI

```bash
Expand Down
26 changes: 24 additions & 2 deletions dist-action/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -31850,6 +31850,21 @@ function getReviewInputs() {
};
}
var RESYNC_COMMAND = "\\translate-resync";
function mergedIntoDefaultBranch(baseRef, defaultBranch, prNumber) {
if (!defaultBranch) {
core.warning(`Could not determine the repository default branch; assuming PR #${prNumber} landed on it.`);
return true;
}
if (!baseRef) {
core.warning(`Could not determine the base branch of PR #${prNumber}; assuming it landed on '${defaultBranch}'.`);
return true;
}
if (baseRef !== defaultBranch) {
core.info(`PR #${prNumber} was merged into '${baseRef}', not the default branch '${defaultBranch}'. Skipping sync \u2014 only merges into the default branch are translated.`);
return false;
}
return true;
}
function validatePREvent(context3, testMode) {
const { eventName, payload } = context3;
if (eventName === "issue_comment") {
Expand All @@ -31871,11 +31886,15 @@ function validatePREvent(context3, testMode) {
}
const merged = payload.pull_request?.merged === true;
const prNumber = payload.pull_request?.number;
if (!prNumber) {
throw new Error("Could not determine PR number from event payload");
}
if (!merged) {
core.info("PR was closed but not merged. Skipping sync.");
return { merged: false, prNumber, isTestMode: false, isResync: false };
}
if (!prNumber) {
throw new Error("Could not determine PR number from event payload");
if (!mergedIntoDefaultBranch(payload.pull_request?.base?.ref, payload.repository?.default_branch, prNumber)) {
return { merged: false, prNumber, isTestMode: false, isResync: false };
}
core.info(`\u{1F680} Running in PRODUCTION mode for merged PR #${prNumber}`);
return { merged, prNumber, isTestMode: false, isResync: false };
Expand Down Expand Up @@ -39887,6 +39906,9 @@ async function runSync() {
core9.info(`PR #${prNumber} is not merged. Resync only works on merged PRs.`);
return;
}
if (!mergedIntoDefaultBranch(pr.base?.ref, pr.base?.repo?.default_branch, prNumber)) {
return;
}
if (pr.merge_commit_sha) {
effectiveSha = pr.merge_commit_sha;
core9.info(`\u{1F504} RESYNC: PR #${prNumber} is merged \u2014 using merge commit ${effectiveSha}`);
Expand Down
2 changes: 1 addition & 1 deletion dist-action/index.js.map

Large diffs are not rendered by default.

8 changes: 6 additions & 2 deletions docs/user/action-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ The action operates in three modes, specified by the `mode` input:

### Sync mode

Runs in the **source** (English) repository. When a PR is merged that changes Markdown files, the action:
Runs in the **source** (English) repository. When a PR that changes Markdown files is merged into the default branch, the action:

1. Detects which files and sections changed
2. Translates only the changed sections using Claude
Expand Down Expand Up @@ -119,6 +119,7 @@ name: Sync Translations
on:
pull_request:
types: [closed]
branches: [main]
paths:
- 'lectures/**/*.md'
- '_toc.yml'
Expand Down Expand Up @@ -162,6 +163,8 @@ The `issue_comment` trigger enables the `\translate-resync` command — comment

**The four conditions on that clause are load-bearing.** `issue_comment` workflows run in default-branch context with full access to secrets, and GitHub cannot filter the event by comment body at the trigger level — so the `if:` is the only gate. It checks that the comment is on a **pull request** (`github.event.issue.pull_request`; plain issues raise the same event), that it carries the command, and that its author is an `OWNER`, `MEMBER` or `COLLABORATOR`. Without the last one, any GitHub account can spend your Anthropic credits by commenting on a merged PR. `CONTRIBUTOR` — anyone with one merged PR — is deliberately excluded, and the action enforces the same set internally, so widening the workflow alone would only buy a run that no-ops. `permissions: contents: read` completes the picture: the action authenticates to the target repo with `TRANSLATION_PAT`, so the ambient `GITHUB_TOKEN` never needs write.

**`branches: [main]` is the other half of the trigger.** `pull_request: types: [closed]` fires for a PR closed against *any* base branch, and `github.event.pull_request.merged` says nothing about which branch it merged into — so without the filter a PR merged into a long-lived work branch (a theme migration, a lecture rewrite in progress) is forwarded to every target repo as if it had been published. The action checks too: a merged PR whose base is not the repository's default branch is skipped, on the `pull_request` path and on `\translate-resync` (an `issue_comment` trigger cannot carry a branch filter). If your default branch is not `main`, change the filter to match; the action's check follows the repository setting.

### Multi-language sync

Use separate jobs for each target language. They run in parallel and create independent PRs:
Expand All @@ -172,6 +175,7 @@ name: Sync Translations
on:
pull_request:
types: [closed]
branches: [main]
paths:
- 'lectures/**/*.md'
issue_comment:
Expand Down Expand Up @@ -286,7 +290,7 @@ jobs:

## How sync mode works

When a PR is merged, sync mode:
When a PR is merged into the default branch, sync mode:

1. **Identifies changed files** — Compares the PR's diff against the docs folder pattern
2. **Classifies each file** — New file (full translation) or existing file (section-level update)
Expand Down
6 changes: 5 additions & 1 deletion docs/user/faq.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ This can happen if:
- The changed files were not `.md` files
- The translation of the changed sections is identical to what was already in the target

### Does sync run for PRs merged into branches other than `main`?

No. The workflow's `branches: [main]` filter stops the run before it starts, and the action itself skips a merged PR whose base branch is not the repository's default branch — the `\translate-resync` comment path included. A long-lived work branch (a theme migration, a rewrite in progress) is not a publication, so merging into it must not open translation PRs. If your sync workflow predates the filter, add `branches: [main]` under `pull_request:`; the action's check covers the gap until you do.

### The action created a PR but some sections weren't translated

Check that:
Expand Down Expand Up @@ -122,7 +126,7 @@ This is expected for well-maintained translations. The backward analysis is desi

| Aspect | Sync Action | Forward CLI |
|--------|------------|-------------|
| **Trigger** | PR merge event | Manual command |
| **Trigger** | PR merged into the default branch | Manual command |
| **Change signal** | Git diff from PR | Whole-document comparison |
| **Translation mode** | UPDATE (section-level) | RESYNC (whole-file) |
| **Scope** | Files changed in that PR | Any drifted files |
Expand Down
3 changes: 2 additions & 1 deletion docs/user/quickstart.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ name: Sync Translations
on:
pull_request:
types: [closed]
branches: [main]
paths:
- 'lectures/**/*.md'
- '_toc.yml'
Expand Down Expand Up @@ -71,7 +72,7 @@ jobs:
github-token: ${{ secrets.TRANSLATION_PAT }}
```

This workflow triggers whenever a PR that touches Markdown files in `lectures/` is merged. It detects which sections changed and creates a translation PR in the target repository. The `issue_comment` trigger enables re-syncing by commenting `\translate-resync` on a merged PR. To retrigger only one language, add the language code (e.g., `\translate-resync zh-cn`).
This workflow triggers whenever a PR that touches Markdown files in `lectures/` is merged into `main` — `branches: [main]` is what keeps a merge into a work-in-progress branch from being translated as if it were published (the action itself checks against the repository's default branch, so edit the filter if yours is not `main`). It detects which sections changed and creates a translation PR in the target repository. The `issue_comment` trigger enables re-syncing by commenting `\translate-resync` on a merged PR. To retrigger only one language, add the language code (e.g., `\translate-resync zh-cn`).

The `author_association` check on that clause is a trust gate: an `issue_comment` workflow runs with full access to your secrets, so without it any GitHub account could spend Anthropic credits by commenting on a merged PR. Keep all four conditions — dropping any one of them re-opens that.

Expand Down
1 change: 1 addition & 0 deletions docs/user/tutorials/add-language.md
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,7 @@ name: Sync Translations
on:
pull_request:
types: [closed]
branches: [main]
paths:
- 'lectures/**/*.md'
- '_toc.yml'
Expand Down
1 change: 1 addition & 0 deletions docs/user/tutorials/connect-existing.md
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,7 @@ name: Sync Translations
on:
pull_request:
types: [closed]
branches: [main]
paths:
- 'lectures/**/*.md'
- 'lectures/_toc.yml'
Expand Down
1 change: 1 addition & 0 deletions docs/user/tutorials/fresh-setup.md
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,7 @@ name: Sync Translations
on:
pull_request:
types: [closed]
branches: [main]
paths:
- 'lectures/**/*.md'
- '_toc.yml'
Expand Down
12 changes: 12 additions & 0 deletions examples/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ name: Sync Translations to Chinese
on:
pull_request:
types: [closed]
branches: [main]
paths:
- 'lectures/**/*.md'
issue_comment:
Expand Down Expand Up @@ -66,6 +67,16 @@ and its author is an `OWNER`, `MEMBER` or `COLLABORATOR`. `CONTRIBUTOR` — anyo
merged PR — is deliberately excluded; the action applies the same three-way set internally,
so admitting it here would only produce a run that no-ops.

### Only merges into `main` sync

`branches: [main]` is part of the trigger, not a nicety. `types: [closed]` fires for a PR
closed against *any* base branch, and the job's `merged == true` check does not look at the
base either — so without the filter, a PR merged into a long-lived work branch opens
translation PRs in every target repo for content the published edition does not carry. The
action also refuses a merged PR whose base is not the repository's default branch, which is
the only check on the `\translate-resync` path (an `issue_comment` trigger has no branch
filter). Change the filter if your default branch has another name.

## Multi-Language Support

You can sync to multiple target repositories. Each target language needs a
Expand All @@ -78,6 +89,7 @@ name: Sync Translations
on:
pull_request:
types: [closed]
branches: [main]
paths:
- 'lectures/**/*.md'
issue_comment:
Expand Down
110 changes: 110 additions & 0 deletions src/__tests__/inputs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
getRebaseInputs,
validatePREvent,
validateReviewPREvent,
mergedIntoDefaultBranch,
} from '../inputs.js';

// Mock @actions/core
Expand Down Expand Up @@ -320,7 +321,9 @@ describe('validatePREvent (sync mode)', () => {
pull_request: {
merged: true,
number: 123,
base: { ref: 'main' },
},
repository: { default_branch: 'main' },
},
};

Expand All @@ -331,6 +334,88 @@ describe('validatePREvent (sync mode)', () => {
expect(result.isResync).toBe(false);
});

describe('base branch guard', () => {
// lecture-python-programming#629: a theme migration merged into the
// long-lived `jb2` branch fired every sync workflow and opened
// translation PRs in three target repos for unpublished content.
it('should skip a PR merged into a branch other than the default branch', () => {
const context = {
eventName: 'pull_request',
payload: {
action: 'closed',
pull_request: {
merged: true,
number: 629,
base: { ref: 'jb2' },
},
repository: { default_branch: 'main' },
},
};

const result = validatePREvent(context, false);
expect(result.merged).toBe(false);
expect(result.prNumber).toBe(629);
expect(result.isResync).toBe(false);
expect(mockedCore.info).toHaveBeenCalledWith(
expect.stringContaining("merged into 'jb2', not the default branch 'main'")
);
});

it('should follow the repository setting, not a hard-coded main', () => {
const context = {
eventName: 'pull_request',
payload: {
action: 'closed',
pull_request: {
merged: true,
number: 7,
base: { ref: 'master' },
},
repository: { default_branch: 'master' },
},
};

expect(validatePREvent(context, false).merged).toBe(true);
});

it('should proceed with a warning when the payload has no default branch', () => {
const context = {
eventName: 'pull_request',
payload: {
action: 'closed',
pull_request: {
merged: true,
number: 8,
base: { ref: 'jb2' },
},
},
};

expect(validatePREvent(context, false).merged).toBe(true);
expect(mockedCore.warning).toHaveBeenCalledWith(
expect.stringContaining('Could not determine the repository default branch')
);
});

it('should not consult the base branch for a closed-unmerged PR', () => {
const context = {
eventName: 'pull_request',
payload: {
action: 'closed',
pull_request: {
merged: false,
number: 9,
base: { ref: 'jb2' },
},
repository: { default_branch: 'main' },
},
};

expect(validatePREvent(context, false).merged).toBe(false);
expect(mockedCore.warning).not.toHaveBeenCalled();
});
});

it('should handle closed but not merged PR', () => {
const context = {
eventName: 'pull_request',
Expand Down Expand Up @@ -611,6 +696,31 @@ describe('validatePREvent (sync mode)', () => {
// PR EVENT VALIDATION TESTS (REVIEW MODE)
// =============================================================================

describe('mergedIntoDefaultBranch', () => {
// Shared by the pull_request path (validatePREvent) and the resync path in
// runSync, which reads the base from pulls.get because an issue_comment
// payload carries neither the base nor the merge state.
beforeEach(() => {
jest.clearAllMocks();
});

it('accepts a base that matches the default branch', () => {
expect(mergedIntoDefaultBranch('main', 'main', 1)).toBe(true);
expect(mockedCore.warning).not.toHaveBeenCalled();
});

it('rejects a base that differs from the default branch', () => {
expect(mergedIntoDefaultBranch('jb2', 'main', 629)).toBe(false);
expect(mockedCore.info).toHaveBeenCalledWith(expect.stringContaining('PR #629'));
});

it('passes with a warning when either side is unknown', () => {
expect(mergedIntoDefaultBranch(undefined, 'main', 2)).toBe(true);
expect(mergedIntoDefaultBranch('jb2', undefined, 3)).toBe(true);
expect(mockedCore.warning).toHaveBeenCalledTimes(2);
});
});

describe('validateReviewPREvent (review mode)', () => {
beforeEach(() => {
jest.clearAllMocks();
Expand Down
Loading
Loading