Skip to content

Commit aee356a

Browse files
JohnMcLearclaude
andauthored
fix: use atomic git push in plugin npmpublish workflow (#7494)
The plugin publish workflow ran `git push --follow-tags` after `pnpm version patch`. `--follow-tags` is non-atomic per ref: if a concurrent publish run won the race, the branch fast-forward would be rejected but the tag push would still land — leaving a dangling `vN+1` tag with no matching version-bump commit on the branch. Every subsequent push would then fail forever with `npm error fatal: tag 'vN+1' already exists`, because `pnpm version patch` would re-derive the same tag name from the unchanged `package.json`. On 2026-04-08, a single churn day (badge fixes + Dependabot merges firing back-to-back) put ~46 plugins into this state simultaneously. Recovery required hand-bumping `package.json` past the dangling tag on every affected repo, twice (a second wave appeared after the first sweep finished, racing the next wave of publishes). Fix: use `git push --atomic origin <branch> <tag>` so the branch update and the tag update succeed or fail as a single server-side transaction. A rejected branch push now also rejects the tag push, the run aborts cleanly, and the next workflow tick can retry against the up-to-date refs without leaving any orphaned tags. Also derive the new tag name from `package.json` after the bump (rather than parsing pnpm version's stdout, which has historically varied) and pass it explicitly into the push. Adds a backend regression test that asserts the workflow file uses `--atomic`, does not contain a literal `git push --follow-tags` command (ignoring the historical comment), and includes both the branch ref and the freshly-bumped tag in the atomic push. The test gates against accidental reverts. This file is the source of truth that `bin/plugins/checkPlugin.ts` propagates into every `ether/ep_*` plugin's `.github/workflows/`, so the next `update-plugins` cron tick will roll the fix out across all plugins automatically. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent b57b25a commit aee356a

2 files changed

Lines changed: 97 additions & 2 deletions

File tree

bin/plugins/lib/npmpublish.yml

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ jobs:
1515
publish-npm:
1616
runs-on: ubuntu-latest
1717
permissions:
18-
contents: write # for `git push --follow-tags` of the version bump
18+
contents: write # for the atomic version-bump push (branch + tag)
1919
id-token: write # for npm OIDC trusted publishing
2020
steps:
2121
- uses: actions/setup-node@v6
@@ -60,8 +60,22 @@ jobs:
6060
git config user.name 'github-actions[bot]'
6161
git config user.email '41898282+github-actions[bot]@users.noreply.github.com'
6262
pnpm i
63+
# `pnpm version patch` bumps package.json, makes a commit, and creates
64+
# a `v<new-version>` tag. Capture the new tag name from package.json
65+
# rather than parsing pnpm's output, which has historically varied.
6366
pnpm version patch
64-
git push --follow-tags
67+
NEW_TAG="v$(node -p "require('./package.json').version")"
68+
# CRITICAL: use --atomic so the branch update and the tag update
69+
# succeed (or fail) as a single transaction on the server. The old
70+
# `git push --follow-tags` was non-atomic per ref: if a concurrent
71+
# publish run won the race, the branch fast-forward would be rejected
72+
# but the tag push would still land — leaving a dangling tag with no
73+
# matching commit on the branch. Subsequent runs would then forever
74+
# try to bump to the same already-existing tag and fail with
75+
# `tag 'vN+1' already exists`. With --atomic, a rejected branch push
76+
# rejects the tag push too, and the next workflow tick can retry
77+
# cleanly against the up-to-date refs.
78+
git push --atomic origin "${GITHUB_REF_NAME}" "${NEW_TAG}"
6579
# This is required if the package has a prepare script that uses something
6680
# in dependencies or devDependencies.
6781
-
Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
'use strict';
2+
3+
// Regression test for bin/plugins/lib/npmpublish.yml.
4+
//
5+
// This file is the source-of-truth template that `bin/plugins/checkPlugin.ts`
6+
// propagates into every `ether/ep_*` plugin's `.github/workflows/`. The
7+
// version-bump step in it MUST use `git push --atomic` rather than the older
8+
// `git push --follow-tags`, otherwise concurrent publish runs can leave
9+
// dangling `vN+1` tags on plugin repos with no matching version-bump commit —
10+
// at which point every subsequent push fails forever with
11+
// `npm error fatal: tag 'vN+1' already exists` until someone reconciles the
12+
// repo by hand.
13+
//
14+
// On 2026-04-08 a single churn day produced ~46 broken plugins this way; the
15+
// recovery was painful enough to be worth a regression test.
16+
17+
import {strict as assert} from 'assert';
18+
import * as fs from 'fs';
19+
import * as path from 'path';
20+
21+
const REPO_ROOT = path.resolve(__dirname, '..', '..', '..', '..');
22+
const NPMPUBLISH_YML = path.join(REPO_ROOT, 'bin', 'plugins', 'lib', 'npmpublish.yml');
23+
24+
describe(__filename, function () {
25+
let yml: string;
26+
27+
before(function () {
28+
yml = fs.readFileSync(NPMPUBLISH_YML, 'utf8');
29+
});
30+
31+
it('uses git push --atomic for the version bump', function () {
32+
assert.match(
33+
yml, /git push --atomic\b/,
34+
'npmpublish.yml must use `git push --atomic` so the branch update and ' +
35+
'the tag push happen as a single transaction. Without --atomic, a ' +
36+
'rejected branch fast-forward (e.g. lost race against a concurrent ' +
37+
'publish run) can still leave the tag pushed, producing a dangling ' +
38+
'vN+1 tag and breaking every future publish on the plugin.',
39+
);
40+
});
41+
42+
it('does not regress to `git push --follow-tags`', function () {
43+
// Strip YAML comments before checking — the historical bug is described
44+
// in a comment block above the new code, and that's an intentional
45+
// forensic note, not a regression. We only care if the actual command
46+
// line uses --follow-tags.
47+
const commandLines = yml
48+
.split('\n')
49+
.filter((l) => !/^\s*#/.test(l))
50+
.join('\n');
51+
assert.doesNotMatch(
52+
commandLines, /git push --follow-tags\b/,
53+
'`git push --follow-tags` is non-atomic per ref and is the exact ' +
54+
'failure mode this workflow used to have. Use `git push --atomic ' +
55+
'origin <branch> <tag>` instead.',
56+
);
57+
});
58+
59+
it('pushes both the branch ref and the version tag in the atomic command', function () {
60+
// Find the atomic push line and assert it carries at least two refspecs
61+
// (the branch + the tag). We don't pin the exact variable names — just
62+
// require that the line names something tag-shaped and something
63+
// branch-shaped — but we DO require the new tag to be derived from the
64+
// freshly-bumped package.json so it can't drift from what `pnpm version
65+
// patch` actually wrote.
66+
const lines = yml.split('\n');
67+
const pushLine = lines.find((l) => /git push --atomic\b/.test(l));
68+
assert.ok(pushLine, 'expected to find a `git push --atomic` line');
69+
// Branch ref — workflow_call inherits the caller's ref via GITHUB_REF_NAME.
70+
assert.match(
71+
pushLine!, /\$\{?GITHUB_REF_NAME\}?/,
72+
'atomic push must include the branch ref via $GITHUB_REF_NAME so it ' +
73+
'works for both `main`- and `master`-default plugins',
74+
);
75+
// Tag ref — must reference the variable holding the just-bumped tag.
76+
assert.match(
77+
pushLine!, /\$\{?NEW_TAG\}?|\$\{?TAG\}?/,
78+
'atomic push must include the version tag (NEW_TAG / TAG) it just created',
79+
);
80+
});
81+
});

0 commit comments

Comments
 (0)