Skip to content

fix: use atomic git push in plugin npmpublish workflow#7494

Merged
JohnMcLear merged 1 commit intodevelopfrom
fix/npmpublish-atomic-push
Apr 8, 2026
Merged

fix: use atomic git push in plugin npmpublish workflow#7494
JohnMcLear merged 1 commit intodevelopfrom
fix/npmpublish-atomic-push

Conversation

@JohnMcLear
Copy link
Copy Markdown
Member

Summary

  • Replaces git push --follow-tags with git push --atomic origin <branch> <tag> in bin/plugins/lib/npmpublish.yml, the source-of-truth template that checkPlugin.ts propagates into every ether/ep_* plugin's .github/workflows/.
  • Derives the new tag name explicitly from package.json after pnpm version patch, instead of relying on parsing pnpm's stdout.
  • Adds src/tests/backend/specs/npmpublish-workflow.ts — a regression test that asserts the workflow uses --atomic, has no literal git push --follow-tags command (ignoring forensic comments), and includes both the branch ref and the freshly-bumped tag in the atomic push.

Why

git push --follow-tags updates the branch and tag refs in two separate server-side transactions. If a concurrent publish run wins the race, the branch fast-forward gets rejected — but the tag push still lands. That leaves a dangling vN+1 tag with no matching version-bump commit on the branch, and package.json still at version N.

Every subsequent push then runs pnpm version patch, derives the same vN+1 tag name from the unchanged package.json, and dies with npm error fatal: tag 'vN+1' already exists. The plugin can never publish again until someone hand-reconciles the repo.

On 2026-04-08, a single churn day (mass badge fixes + Dependabot auto-merge from #7493 firing back-to-back across ~80 plugins) put ~46 plugins into this state at once. The 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 publish runs that the sweep itself had triggered.

git push --atomic makes branch + tag a single server-side transaction. A rejected branch fast-forward also rejects the tag push, the run aborts cleanly, and the next workflow tick retries against up-to-date refs with no orphans left behind.

Propagation

This file is the canonical template; the daily update-plugins.yml cron in this repo runs checkPlugin.ts against every plugin, which copies this file into each plugin's .github/workflows/npmpublish.yml. So one commit here heals every plugin on the next cron tick (or workflow_dispatch).

Test plan

  • pnpm run ts-check — clean
  • New regression test runs green: npx mocha --import=tsx tests/backend/specs/npmpublish-workflow.ts (3 passing)
  • After merge, trigger update-plugins.yml manually and confirm the new file is propagated to plugin repos
  • On the next plugin Dependabot churn, confirm no new tag-drift incidents appear

Notes

The end-to-end behaviour of git push --atomic against the actual Git server isn't unit-testable (atomicity is a server feature, not local). The regression test is a static-analysis check that the workflow keeps using --atomic, which catches the most likely regression: someone reverting the change in a refactor without realizing why it matters. The forensic comment in the workflow file explains the historical bug for anyone reading the diff later.

🤖 Generated with Claude Code

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>
@JohnMcLear JohnMcLear merged commit aee356a into develop Apr 8, 2026
19 of 22 checks passed
@JohnMcLear JohnMcLear deleted the fix/npmpublish-atomic-push branch April 8, 2026 11:38
@qodo-free-for-open-source-projects
Copy link
Copy Markdown

Review Summary by Qodo

Use atomic git push in plugin npmpublish workflow

🐞 Bug fix 🧪 Tests

Grey Divider

Walkthroughs

Description
• Replace non-atomic git push --follow-tags with git push --atomic to prevent dangling version
  tags
• Derive tag name explicitly from package.json after version bump instead of parsing stdout
• Add regression test to prevent accidental reversion to non-atomic push behavior
• Fix addresses critical issue affecting ~46 plugins simultaneously on 2026-04-08
Diagram
flowchart LR
  A["pnpm version patch"] -->|bumps version| B["package.json"]
  B -->|extract tag| C["NEW_TAG variable"]
  C -->|atomic push| D["git push --atomic<br/>branch + tag"]
  D -->|single transaction| E["Success or<br/>complete failure"]
  F["Regression test"] -->|validates| D
  F -->|prevents revert| G["No dangling tags"]
Loading

Grey Divider

File Changes

1. bin/plugins/lib/npmpublish.yml 🐞 Bug fix +16/-2

Implement atomic git push for version bumps

• Replace git push --follow-tags with git push --atomic origin "${GITHUB_REF_NAME}" "${NEW_TAG}"
 for atomic transactions
• Extract new tag name from package.json after pnpm version patch instead of parsing stdout
• Add detailed forensic comments explaining the historical bug and atomic push requirement
• Update permissions comment to reflect atomic push instead of follow-tags

bin/plugins/lib/npmpublish.yml


2. src/tests/backend/specs/npmpublish-workflow.ts 🧪 Tests +81/-0

Add regression tests for atomic push workflow

• Add regression test file with three test cases for the npmpublish workflow
• Assert workflow uses git push --atomic command
• Verify workflow does not contain git push --follow-tags (ignoring comments)
• Validate atomic push includes both branch ref (GITHUB_REF_NAME) and version tag
 (NEW_TAG/TAG)

src/tests/backend/specs/npmpublish-workflow.ts


Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects
Copy link
Copy Markdown

qodo-free-for-open-source-projects Bot commented Apr 8, 2026

Code Review by Qodo

Grey Divider

New Review Started

This review has been superseded by a new analysis

Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown

Review Summary by Qodo

Use atomic git push in plugin npmpublish workflow

🐞 Bug fix 🧪 Tests

Grey Divider

Walkthroughs

Description
• Replace non-atomic git push --follow-tags with git push --atomic to prevent dangling tags
  - Fixes race condition where concurrent publishes leave orphaned version tags
  - Derives tag name explicitly from package.json instead of parsing pnpm stdout
• Add regression test to prevent accidental reversion of atomic push behavior
• Update workflow permissions comment to reflect atomic push semantics
Diagram
flowchart LR
  A["pnpm version patch"] --> B["Extract NEW_TAG from package.json"]
  B --> C["git push --atomic origin branch tag"]
  C --> D["Single server transaction"]
  D --> E["No dangling tags on race loss"]
  F["Regression test"] --> G["Assert --atomic usage"]
  G --> H["Prevent future reverts"]
Loading

Grey Divider

File Changes

1. bin/plugins/lib/npmpublish.yml 🐞 Bug fix +16/-2

Implement atomic git push for version bumps

• Replace git push --follow-tags with git push --atomic origin "${GITHUB_REF_NAME}" "${NEW_TAG}"
 for atomic branch and tag updates
• Extract new tag name from package.json after pnpm version patch instead of parsing stdout
• Add detailed forensic comments explaining the historical race condition and why atomicity is
 critical
• Update permissions comment to reflect atomic push semantics

bin/plugins/lib/npmpublish.yml


2. src/tests/backend/specs/npmpublish-workflow.ts 🧪 Tests +81/-0

Add regression tests for atomic push workflow

• Add regression test file with three test cases for the npmpublish workflow
• Assert workflow uses git push --atomic command
• Verify workflow does not contain literal git push --follow-tags (ignoring comments)
• Validate atomic push includes both branch ref and version tag in command

src/tests/backend/specs/npmpublish-workflow.ts


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown

qodo-code-review Bot commented Apr 8, 2026

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant