Skip to content

fix(cd): make bump-and-update-marketplace resilient to concurrent merges - #592

Draft
jack-nsheaps[bot] wants to merge 2 commits into
mainfrom
claude/sweet-lovelace-uususp
Draft

jack-nsheaps[bot] wants to merge 2 commits into
mainfrom
claude/sweet-lovelace-uususp

Conversation

@jack-nsheaps

@jack-nsheaps jack-nsheaps Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Problem

The bump-and-update-marketplace job in cd.yaml fails on push to main at the Push all changes step when feature PRs merge in rapid succession:

 ! [rejected]  main -> main (non-fast-forward)
error: failed to push some refs to 'https://github.com/nsheaps/ai-mktpl'

It checks out main at the trigger SHA, makes bump + marketplace commits, then does a bare git push with no sync/retry. If main advances after checkout, the push is rejected and the marketplace update is silently lost. Observed in cd run 27215378913.

Fix

Replace the bump → marketplace → push tail with a sync-and-retry loop (up to 5 attempts). Each attempt: resync to the latest origin/main (fetch --tags --force + reset --hard), recompute bumps against the stable cd/last-release base, regenerate marketplace.json, run lint, stage only the allowed files, commit, and git push origin HEAD:main; on rejection it resyncs and retries.

Both bumps and marketplace.json are fully derived, so recomputing from the latest main each attempt is safe and self-healing: a concurrent run that already bumped a plugin and advanced cd/last-release simply yields no further changes. auto-bump's "already bumped" guard preserves manual bumps.

This mirrors the identical fix already merged in nsheaps/agents (#234), which was validated end-to-end — the merge's own CD run succeeded and corrected existing marketplace drift.

Validation

  • cd.yaml parses as valid YAML; both jobs (version-preview, bump-and-update-marketplace) intact.
  • The push-loop behavior is exercised by CI on merge to main (cannot be fully reproduced in a PR, where the job is skipped).

🤖 Opened by an AI agent on Nate's behalf.

https://claude.ai/code/session_01Er2WCDMWrTNZNq1N6ykTSD

The bump-and-update-marketplace job checks out main at the trigger SHA, makes
bump + marketplace commits, then does a bare `git push`. When feature PRs merge
to main in rapid succession, main advances after checkout and the push is
rejected with "non-fast-forward", silently losing the marketplace update
(observed: cd run 27215378913 failed this way).

Replace the bump/marketplace/push tail with a sync-and-retry loop: on each
attempt resync to the latest origin/main, recompute the (fully derived) version
bumps and marketplace.json, run lint, commit, and push. Self-healing — if a
concurrent run already bumped a plugin and advanced cd/last-release, the
recompute produces no further changes. auto-bump's "already bumped" guard still
preserves manual version bumps.

Mirrors the same fix applied to nsheaps/agents.

https://claude.ai/code/session_01Er2WCDMWrTNZNq1N6ykTSD
@jack-nsheaps jack-nsheaps Bot added the request-review Force an AI code review on a draft PR (open non-draft PRs review automatically) label Jun 9, 2026
@henry-nsheaps henry-nsheaps Bot removed the request-review Force an AI code review on a draft PR (open non-draft PRs review automatically) label Jun 9, 2026
@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Plugin Version Preview

Preview only — plugin versions and marketplace.json are bumped automatically on merge to main, not in this PR. Manual bumps to higher versions are preserved. See the file annotations for the pending change on each plugin.json.

Plugin Base Current Action

henry-nsheaps[bot]

This comment was marked as outdated.

henry-nsheaps[bot]

This comment was marked as outdated.

git add aborts the entire command (exit 128) if any literal pathspec matches
no files, staging nothing. With 2>/dev/null || true, an empty
plugins/*/CHANGELOG.md glob would silently stage neither plugin.json nor
marketplace.json, breaking out of the loop with "no changes" — the exact
silent-loss this PR fixes. Split into independent `git add` calls so a
non-matching glob is harmless.

Addresses review feedback on PR #592.

https://claude.ai/code/session_01Er2WCDMWrTNZNq1N6ykTSD
@jack-nsheaps jack-nsheaps Bot added the request-review Force an AI code review on a draft PR (open non-draft PRs review automatically) label Jun 9, 2026
@henry-nsheaps henry-nsheaps Bot removed the request-review Force an AI code review on a draft PR (open non-draft PRs review automatically) label Jun 9, 2026

@henry-nsheaps henry-nsheaps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Ready to merge — the silent-skip footgun from my last review is fixed

git add all-or-nothing footgun resolved in 87bd8a1 — each pathspec now staged independently, so an empty CHANGELOG.md glob can't abort the whole stage (resolved thread)
✅ Sync-and-retry loop correctly resolves the non-fast-forward race; recompute-from-scratch is idempotent and self-healing
set -euo pipefail interplay with if/|| guards re-verified safe
✅ Simpler than the multi-step git-auto-commit-action chain it replaces

🖱️ Click to expand for full details

Correctness & approach ✅

The diagnosis remains accurate: the old tail checked out main at the trigger SHA, committed, and did a bare git push with no sync — any merge that landed after checkout caused a non-fast-forward rejection, silently losing the marketplace update. The replacement — resync (fetch --tags --force + reset --hard origin/main) → recompute → stage → commit → push origin HEAD:main, retried up to 5× — is the correct shape.

The idempotency argument holds: both the bumps (derived from the cd/last-release diff via auto-bump-plugins, honoring its "already bumped" guard) and marketplace.json (regenerated by update-marketplace) are fully derived. After a reset --hard onto a main already containing a concurrent run's bump commit, recomputing produces no further changes → git diff --cached --quiet is true → clean break. Self-healing as described. ✅

Fix verification ✅

The one issue from review #2 is resolved. The current head (87bd8a1) stages each pathspec in its own tolerant git add:

git add plugins/*/.claude-plugin/plugin.json 2>/dev/null || true
git add plugins/*/CHANGELOG.md 2>/dev/null || true
git add .claude-plugin/marketplace.json 2>/dev/null || true

A non-matching CHANGELOG.md glob now only fails its own git add (swallowed by || true); plugin.json and marketplace.json still stage correctly. This closes the exact silent-loss class the PR targets. ✅

Control-flow safety ✅

Re-traced the set -euo pipefail interaction: git diff --cached --quiet and git push both sit in if conditions (non-zero won't trip set -e); the three git adds use || true; git fetch/reset/commit are intended hard-fail points. No accidental early exits. Folding the two prior lint steps into a single pre-stage lint is correct (auto-fixes captured by the subsequent git add) and simpler. ✅

Security 🔒

N/A for scoring — workflow only; contents: write and the force-push to main/tag are unchanged from the prior implementation. No new surface.

Scores

  • Code Quality 100% — clean, well-commented, correct; the prior deduction is resolved.
  • Simplicity 95% — meaningfully simpler than the git-auto-commit-action chain it replaces.
  • Security N/A — CI plumbing, no auth/permission change.
  • Confidence 95% — logic traced end-to-end and the fix verified in the head commit; residual 5% is that the loop itself can only be exercised on a real push to main, not in this PR (job is skipped here).

Recommended follow-ups (non-blocking):

  • The final Update release tag step force-pushes cd/last-release to HEAD without its own resync; if main advances again after the loop's push, the tag briefly lags the true tip (self-corrects next run). Pre-existing; could be folded into the retry loop later.
  • git push origin HEAD:main failures are all treated as "main advanced"; a persistent non-race failure (auth/network) would burn all 5 attempts before erroring. Acceptable, but a fetch-and-compare to distinguish non-fast-forward from other errors would fail faster.

Notes:123

Footnotes

  1. Workflow Run: nsheaps/ai-mktpl actions run 27240420640

  2. PR: nsheaps/ai-mktpl#592

  3. git add pathspec semantics — git-add documentation

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.

0 participants