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 @@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file.

## [Unreleased]

### Fixed
- When another push lands on the pull request branch while a run commits its translations, the run now puts its commit on top of the newer one and pushes again, as signed commits already did. This is the common case in a monorepo where each app's job commits to the same branch: before, the job that pushed second failed after three identical retries and its translations were lost. If the newer commits changed one of the same translation files, or the branch was rewritten, the run skips its commit with a notice instead of overwriting anything.
- A failed push only suggests checking `permissions: contents: write` when GitHub actually refused the push for permissions. Other failures, such as a network error, show git's output without that hint.

## [0.0.79] - 2026-10-08

### Changed
Expand Down
17 changes: 12 additions & 5 deletions src/commands/translate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ import {
BatchResult,
MissingLocaleEntry
} from '../utils/translation-utils.js';
import { autoCommitChanges, buildMakemessagesCommand, MISSING_BRANCH_ERROR, MISSING_TOKEN_ERROR, type CommitResult } from '../utils/github.js';
import { autoCommitChanges, buildMakemessagesCommand, MISSING_BRANCH_ERROR, MISSING_TOKEN_ERROR, PushPermissionError, type CommitResult } from '../utils/github.js';
import { GitHubGraphQLError } from '../utils/github-graphql.js';
import { escapeAnnotationData } from '../utils/ci-context.js';
import { detectTargetChanges, type TargetChangeFile } from '../utils/target-changes.js';
import { resetUnreadableFiles, getUnreadableFiles } from '../utils/unreadable-files.js';
Expand Down Expand Up @@ -160,9 +161,15 @@ const COMMIT_FAILURE_FIXES: Record<string, string> = {
const NOT_A_PULL_REQUEST_WARNING = '::warning::Translations were not committed: this is not a pull request run. Only pull_request runs commit translations.';
const PUSH_PERMISSION_FIX = 'Check that the workflow grants `permissions: contents: write`. On a pull request from a fork, GITHUB_TOKEN is read-only.';

function commitFailedAnnotation(reason: string): string {
const fix = COMMIT_FAILURE_FIXES[reason] ?? PUSH_PERMISSION_FIX;
return `::error::${escapeAnnotationData(`Translations were not committed: ${reason}. ${fix}`)}`;
function isPermissionFailure(error: Error): boolean {
if (error instanceof PushPermissionError) return true;
return error instanceof GitHubGraphQLError && (error.type === 'FORBIDDEN' || /\b403\b/.test(error.message));
}

function commitFailedAnnotation(error: Error): string {
const fix = isPermissionFailure(error) ? PUSH_PERMISSION_FIX : COMMIT_FAILURE_FIXES[error.message];
const text = [`Translations were not committed: ${error.message}.`, fix].filter(Boolean).join(' ');
return `::error::${escapeAnnotationData(text)}`;
}

/**
Expand Down Expand Up @@ -456,7 +463,7 @@ export async function translate(options: TranslationOptions = {}, deps: Translat
} else if (err.message === MISSING_BRANCH_ERROR) {
console.log(NOT_A_PULL_REQUEST_WARNING);
} else {
console.log(commitFailedAnnotation(err.message));
console.log(commitFailedAnnotation(err));
process.exitCode = 1;
}
}
Expand Down
110 changes: 98 additions & 12 deletions src/utils/github.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,15 @@ const SYNC_SKIP_NOTICES: Record<SkippedCommit, string> = {
// A tip that keeps moving is a busy branch; stop chasing it and let a later run commit.
const MAX_COMMITS_ON_NEWER_TIP = 2;

// A failed push is classified by what git printed, not by the branch's shape:
// a read-only token can still fetch a branch that moved.
const PERMISSION_DENIED_OUTPUT = /Permission to \S+ denied|returned error: 403|Authentication failed|could not read Username/;
const BRANCH_MOVED_OUTPUT = /\((fetch first|non-fast-forward|stale info)\)|cannot lock ref/;
const PUSH_ATTEMPTS = 3;

export class PushPermissionError extends Error {}
class BranchMovedError extends Error {}

/**
* Dependencies for the GitHub service
*/
Expand Down Expand Up @@ -367,26 +376,82 @@ ${buildExtractStep(options)} - name: Translate
return new Promise(resolve => setTimeout(resolve, ms));
},

/**
* A moved branch is replayed onto (never for an amend, which would carry the
* trigger commit along) or skipped; a permission failure fails at once; any
* other failure, such as the network, is retried.
*/
async pushWithRetry(
branchName: string,
token: string,
forceWithLease: boolean = false,
maxRetries: number = 3
): Promise<void> {
forceWithLease: boolean = false
): Promise<'pushed' | SkippedCommit> {
const { console: log } = this.deps;
let failedAttempts = 0;
let replays = 0;

for (let attempt = 1; attempt <= maxRetries; attempt++) {
for (;;) {
try {
this.pushToGitHub(branchName, token, forceWithLease);
return;
return 'pushed';
} catch (error) {
if (attempt === maxRetries) throw error;
log.log(`Push failed, retrying (${attempt}/${maxRetries})...`);
if (error instanceof PushPermissionError) throw error;
if (error instanceof BranchMovedError) {
if (forceWithLease || replays === MAX_COMMITS_ON_NEWER_TIP) return 'skipped-uncertain';
replays++;
const outcome = await this.rebaseOntoMovedBranch(branchName);
if (outcome !== 'rebased') return outcome;
continue;
}
failedAttempts++;
if (failedAttempts === PUSH_ATTEMPTS) throw error;
log.log(`Push failed, retrying (${failedAttempts}/${PUSH_ATTEMPTS})...`);
await this.sleep(2000);
}
}
},

/**
* After a push rejected because the branch moved: replay our translation
* commit onto the new tip when that is safe, the plain-git counterpart of
* findTipSafeToCommitOn. Safe means the branch only moved forward from our
* commit's parent (so nothing someone removed comes back) and the newer
* commits left our files alone. Anything else is a skip.
*/
async rebaseOntoMovedBranch(branchName: string): Promise<'rebased' | SkippedCommit> {
const { console: log } = this.deps;
const git = (args: string) => this.deps.exec(`git ${args}`, { stdio: 'pipe' }).toString().trim();
// --no-renames lists a rename under both paths, so a renamed file of ours counts as touched.
const changedFiles = (from: string, to: string) =>
git(`diff --name-only --no-renames ${from} ${to}`).split('\n').filter(Boolean);

try {
git(`fetch --no-tags origin ${branchName}`);
git('merge-base --is-ancestor HEAD~1 FETCH_HEAD');
} catch {
log.log('Could not confirm the branch only moved forward since the checkout.');
return 'skipped-uncertain';
}

const theirs = new Set(changedFiles('HEAD~1', 'FETCH_HEAD'));
const overlap = changedFiles('HEAD~1', 'HEAD').find(file => theirs.has(file));
if (overlap) {
log.log(`Newer commits on the branch changed ${overlap}.`);
return 'skipped-overlap';
}

// --autostash: translate leaves localhero.json (lastSyncedAt) modified and unstaged.
try {
git('rebase --autostash --onto FETCH_HEAD HEAD~1');
} catch {
try { git('rebase --abort'); } catch { /* nothing to abort */ }
log.log('Could not replay the translation commit onto the newer commits.');
return 'skipped-uncertain';
}
log.log(`Branch moved during the run; replayed the translation commit onto ${git('rev-parse --short FETCH_HEAD')}.`);
return 'rebased';
},

/**
* Push changes to GitHub using the provided token
* @param branchName Branch to push to
Expand Down Expand Up @@ -418,9 +483,22 @@ ${buildExtractStep(options)} - name: Translate
}

const pushCmd = forceWithLease
? `git push --force-with-lease origin HEAD:${branchName}`
: `git push origin HEAD:${branchName}`;
exec(pushCmd, { stdio: 'inherit' });
? `git push --force-with-lease origin HEAD:${branchName} 2>&1`
: `git push origin HEAD:${branchName} 2>&1`;
const mask = (text: string) => text.split(token).join('***TOKEN***').trim();

try {
const output = mask(exec(pushCmd, { stdio: 'pipe' }).toString());
if (output) log.log(output);
} catch (error: unknown) {
const err = error as Error & { stdout?: Buffer | string };
const output = mask(err.stdout?.toString() ?? '');
if (output) log.log(output);
const message = [mask(err.message), output].filter(Boolean).join('\n');
if (PERMISSION_DENIED_OUTPUT.test(output)) throw new PushPermissionError(message);
if (BRANCH_MOVED_OUTPUT.test(output)) throw new BranchMovedError(message);
throw new Error(message);
}
},

/**
Expand Down Expand Up @@ -565,7 +643,11 @@ ${buildExtractStep(options)} - name: Translate
this.commit(commitMessage, canAmend);

const token = await this.getTokenForPush();
await this.pushWithRetry(branchName, token, canAmend);
const outcome = await this.pushWithRetry(branchName, token, canAmend);
if (outcome !== 'pushed') {
log.log(`::warning::${SYNC_SKIP_NOTICES[outcome]}`);
return 'skipped';
}

if (canAmend) {
log.log('✓ Commit amended and pushed to GitHub\n');
Expand Down Expand Up @@ -681,7 +763,11 @@ ${buildExtractStep(options)} - name: Translate
this.commit(commitMessage);

const token = await this.getTokenForPush();
await this.pushWithRetry(branchName, token);
const outcome = await this.pushWithRetry(branchName, token);
if (outcome !== 'pushed') {
log.log(`::warning::${TRANSLATE_SKIP_NOTICES[outcome]}`);
return 'skipped';
}

log.log('Changes committed and pushed successfully.');
return 'new';
Expand Down
28 changes: 26 additions & 2 deletions tests/commands/translate.test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { jest } from '@jest/globals';
import { translate } from '../../src/commands/translate.js';
import { ApiResponseError } from '../../src/types/index.js';
import { PushPermissionError } from '../../src/utils/github.js';
import { GitHubGraphQLError } from '../../src/utils/github-graphql.js';

describe('translate command', () => {
let mockConsole;
Expand Down Expand Up @@ -1440,8 +1442,8 @@ describe('translate command', () => {
expect(process.exitCode).not.toBe(1);
});

it('fails the run and points at the push permission when the push is rejected', async () => {
gitUtils.autoCommitChanges.mockRejectedValue(new Error('Command failed: git push origin HEAD:feature\nremote: Permission denied'));
it('fails the run and points at the push permission when the push is denied', async () => {
gitUtils.autoCommitChanges.mockRejectedValue(new PushPermissionError('Command failed: git push origin HEAD:feature\nremote: Permission denied'));

await translate({}, createTranslateDeps());

Expand All @@ -1452,6 +1454,28 @@ describe('translate command', () => {
expect(process.exitCode).toBe(1);
});

it('points at the push permission when a signed commit is forbidden', async () => {
gitUtils.autoCommitChanges.mockRejectedValue(new GitHubGraphQLError('Resource not accessible by integration', 'FORBIDDEN'));

await translate({}, createTranslateDeps());

expect(errorAnnotations()).toEqual([
'::error::Translations were not committed: Resource not accessible by integration. ' +
'Check that the workflow grants `permissions: contents: write`. On a pull request from a fork, GITHUB_TOKEN is read-only.'
]);
});

it('fails the run without blaming permissions when the push failed for another reason', async () => {
gitUtils.autoCommitChanges.mockRejectedValue(new Error("Command failed: git push origin HEAD:feature\nfatal: unable to access: Could not resolve host: github.com"));

await translate({}, createTranslateDeps());

expect(errorAnnotations()).toEqual([
'::error::Translations were not committed: Command failed: git push origin HEAD:feature%0Afatal: unable to access: Could not resolve host: github.com.'
]);
expect(process.exitCode).toBe(1);
});

it.each([['skipped'], ['no-changes']])('passes when the commit result is %s', async (commitResult) => {
gitUtils.autoCommitChanges.mockResolvedValue(commitResult);

Expand Down
141 changes: 141 additions & 0 deletions tests/utils/git-push-branch-moved.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
import { describe, it, expect, jest, beforeEach, afterEach } from '@jest/globals';
import { execSync } from 'child_process';
import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'fs';
import os from 'os';
import path from 'path';
import { githubService } from '../../src/utils/github.js';

const BRANCH = 'feature';
const GIT = '-c user.email=t@t -c user.name=t -c commit.gpgsign=false';

function git(cwd: string, args: string): string {
return execSync(`git ${GIT} ${args}`, { cwd, stdio: 'pipe' }).toString().trim();
}

function commitFile(cwd: string, file: string, content: string, message: string): void {
mkdirSync(path.dirname(path.join(cwd, file)), { recursive: true });
writeFileSync(path.join(cwd, file), content);
git(cwd, `add ${file}`);
git(cwd, `commit -qm "${message}"`);
}

function remoteLog(remote: string): string[] {
return git(remote, `log --format=%s ${BRANCH}`).split('\n');
}

describe('rebaseOntoMovedBranch against a real remote', () => {
let root: string;
let remote: string;
let sibling: string;
let ours: string;
let originalCwd: string;

beforeEach(() => {
originalCwd = process.cwd();
root = mkdtempSync(path.join(os.tmpdir(), 'push-moved-'));
remote = path.join(root, 'remote.git');
sibling = path.join(root, 'sibling');
ours = path.join(root, 'ours');

execSync(`git init -q --bare -b ${BRANCH} ${remote}`);
execSync(`git clone -q ${remote} ${sibling}`, { stdio: 'pipe' });
commitFile(sibling, 'apps/editor/public/locale/sv.json', '{}\n', 'base');
git(sibling, `push -q origin HEAD:${BRANCH}`);
execSync(`git clone -q ${remote} ${ours}`, { stdio: 'pipe' });

// In CI configureGitUser sets the identity before committing; here the rebase needs one too.
const gitEnv = {
...process.env,
GIT_COMMITTER_NAME: 't', GIT_COMMITTER_EMAIL: 't@t',
GIT_CONFIG_COUNT: '1', GIT_CONFIG_KEY_0: 'commit.gpgsign', GIT_CONFIG_VALUE_0: 'false'
};
githubService.setDependencies({
exec: (cmd, options) => execSync(cmd, { ...options, env: gitEnv }),
env: {},
console: { log: jest.fn(), warn: jest.fn(), error: jest.fn() }
});
process.chdir(ours);
});

afterEach(() => {
process.chdir(originalCwd);
rmSync(root, { recursive: true, force: true });
});

it('replays our commit onto a tip that moved forward without touching our files', async () => {
commitFile(sibling, 'apps/editor/public/locale/de.json', '{}\n', 'editor translations');
git(sibling, `push -q origin HEAD:${BRANCH}`);
commitFile(ours, 'apps/portal/public/locales/sv.json', '{}\n', 'portal translations');

const outcome = await githubService.rebaseOntoMovedBranch(BRANCH);
git(ours, `push -q origin HEAD:${BRANCH}`);

expect(outcome).toBe('rebased');
expect(remoteLog(remote)).toEqual(['portal translations', 'editor translations', 'base']);
});

it('replays with the unstaged localhero.json change translate leaves behind, and keeps it', async () => {
commitFile(sibling, 'localhero.json', '{"lastSyncedAt": "old"}\n', 'config');
git(sibling, `push -q origin HEAD:${BRANCH}`);
git(ours, `pull -q origin ${BRANCH}`);
commitFile(sibling, 'apps/editor/public/locale/de.json', '{}\n', 'editor translations');
git(sibling, `push -q origin HEAD:${BRANCH}`);
commitFile(ours, 'apps/portal/public/locales/sv.json', '{}\n', 'portal translations');
writeFileSync(path.join(ours, 'localhero.json'), '{"lastSyncedAt": "new"}\n');

const outcome = await githubService.rebaseOntoMovedBranch(BRANCH);
git(ours, `push -q origin HEAD:${BRANCH}`);

expect(outcome).toBe('rebased');
expect(remoteLog(remote)).toEqual(['portal translations', 'editor translations', 'config', 'base']);
expect(git(ours, 'status --porcelain')).toBe('M localhero.json');
});

it('skips when the newer commits touched one of our files', async () => {
commitFile(sibling, 'apps/editor/public/locale/sv.json', '{"a": 1}\n', 'someone else edits sv');
git(sibling, `push -q origin HEAD:${BRANCH}`);
commitFile(ours, 'apps/editor/public/locale/sv.json', '{"a": 2}\n', 'our sv');

const outcome = await githubService.rebaseOntoMovedBranch(BRANCH);

expect(outcome).toBe('skipped-overlap');
expect(git(ours, 'log -1 --format=%s')).toBe('our sv');
expect(git(ours, 'status --porcelain')).toBe('');
});

it('treats a renamed file as touched, so our edits are not carried to the new name', async () => {
git(sibling, 'mv apps/editor/public/locale/sv.json apps/editor/public/locale/sv-SE.json');
git(sibling, 'commit -qm "rename sv"');
git(sibling, `push -q origin HEAD:${BRANCH}`);
commitFile(ours, 'apps/editor/public/locale/sv.json', '{"a": 2}\n', 'our sv');

const outcome = await githubService.rebaseOntoMovedBranch(BRANCH);

expect(outcome).toBe('skipped-overlap');
});

it('skips rather than replay a commit someone removed with a force-push', async () => {
commitFile(sibling, 'app/code.js', 'removed later\n', 'code change');
git(sibling, `push -q origin HEAD:${BRANCH}`);
git(ours, `pull -q origin ${BRANCH}`);
commitFile(ours, 'apps/portal/public/locales/sv.json', '{}\n', 'portal translations');
git(sibling, 'reset -q --hard HEAD~1');
commitFile(sibling, 'README.md', 'docs\n', 'replacement');
git(sibling, `push -q --force origin HEAD:${BRANCH}`);

const outcome = await githubService.rebaseOntoMovedBranch(BRANCH);

expect(outcome).toBe('skipped-uncertain');
expect(remoteLog(remote)).toEqual(['replacement', 'base']);
});

it('skips when the branch cannot be fetched', async () => {
commitFile(ours, 'apps/portal/public/locales/sv.json', '{}\n', 'portal translations');
git(ours, 'remote set-url origin /nonexistent/remote.git');

const outcome = await githubService.rebaseOntoMovedBranch(BRANCH);

expect(outcome).toBe('skipped-uncertain');
expect(git(ours, 'log -1 --format=%s')).toBe('portal translations');
});
});
Loading
Loading