Skip to content

Pre-validate before mutation; atomic push prevents orphan tags#49

Merged
silverwind merged 5 commits into
masterfrom
validate-before-execute
May 28, 2026
Merged

Pre-validate before mutation; atomic push prevents orphan tags#49
silverwind merged 5 commits into
masterfrom
validate-before-execute

Conversation

@silverwind
Copy link
Copy Markdown
Owner

@silverwind silverwind commented May 28, 2026

The bug

git push <remote> <branch> <tag> is not atomic. If the branch push is rejected (non-FF) but the tag push succeeds, the rollback hooks — registered after await exec returns — never run, so the orphan tag stays on the remote.

Architecture

main() is now gather → validate → execute:

  • Gather — pure reads. Resolve versions, read files, probe remote refs and the forge endpoint.
  • Validate — read-only assertions; all must pass before any mutation:
    • at least one specified file would change
    • git author identity is available
    • the planned tag is absent on the remote
    • local HEAD is a descendant of the remote tip (FF possible)
    • if --release: forge reachable, token accepted, permissions.push is true
  • Execute — mutations only. Files → -c → commit → tag → git push --atomic → forge release. The local rollback chain runs only on failure before the atomic push lands; once it lands, the commit/tag are shared and we leave them.

Pre-validation removes the predictable post-push failure modes. --atomic removes the partial-push class. Post-push forge failure prints a recovery hint instead of force-pushing remote history.

Behavior changes

  • File no-op is fatal — passing files that contain no version pattern (or where --base is wrong) now errors instead of silently exiting 0.
  • ls-remote failure aborts — previously a warning + best-effort push; now refuses to mutate if the remote state can't be read.
  • Remote tag conflict aborts before any commit/tag — previously failed at push time after the local mutations. Resuming a partial-failure run now requires deleting the remote tag first.
  • Post-push release failure leaves the tag — the previous design force-pushed the branch back and deleted the remote tag. The new code prints `gh release create ` / `tea release create --tag ` and the user finishes the release manually.

Performance

All independent I/O probes fire in parallel at the top of main() and validate awaits them once. Critical path drops from ~450ms (sequential) to ~210–300ms (slowest single chain).

Tests

All 105 existing tests pass; added validate - aborts when remote branch has advanced beyond local regressing the exact orphan-tag trigger.


This PR was written with the help of Claude Opus 4.7

Splits main() into gather, validate, and execute. Validate runs
read-only checks (file would change, identity available, remote
tag absent, local HEAD is FF-able, forge reachable with push
permission) and prints + exits before any mutation if any fail.
Execute uses `git push --atomic` so a partial push (branch
rejected, tag accepted) can no longer leave an orphan tag on
the remote.

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR restructures the versions CLI workflow to pre-validate all read-only constraints before performing any mutations, and to eliminate partial remote state (notably orphan remote tags) by switching to an atomic git push.

Changes:

  • Split execution into Gather → Validate → Execute phases, ensuring validation failures exit cleanly before any file/git mutations.
  • Replace non-atomic remote updates with git push --atomic <remote> <branch> <tag> to prevent orphan tags on rejected branch pushes.
  • Add a regression test ensuring validation aborts when the remote branch has advanced beyond the local HEAD.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
index.ts Adds gather/validate/execute phasing, remote probing + forge ping validation, and atomic push to prevent partial remote updates.
index.test.ts Adds a regression test for the “remote advanced → abort before mutation” scenario and updates expectations for --remote + --release.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread index.ts
silverwind and others added 2 commits May 28, 2026 09:06
Fires every independent probe up front (resolveBaseVersion,
pushBranch, identity check, getRepoInfo + tokens + pingForge,
later probeRemote + merge-base ancestor check). validate
collects everything in one Promise.all instead of awaiting
each git/HTTPS call sequentially. The critical path drops
from ~450ms to whichever single network call is slowest.

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
Reviewer feedback: throwing a plain Error from the push-permission
check short-circuits withTokens, which only retries on AuthRetryable.
A user with two tokens where the first lacks push would have validate
fail even though the second token would succeed. Switch to
AuthRetryable so withTokens falls through.

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comment thread index.ts
Comment thread index.ts
silverwind and others added 2 commits May 28, 2026 09:28
* Use raw `files` count for the no-op check so a run that only
  specified unhandled lockfiles aborts instead of failing later at
  "nothing to commit". Gated on !args.gitless to preserve the
  existing "lockfile silently skipped" behavior in gitless mode.
* pingForge treats 404 as auth-retryable: GitHub returns 404
  (not 403) for private repos when the token lacks read access,
  so withTokens needs to fall through to the next token.

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
@silverwind silverwind merged commit 785269c into master May 28, 2026
20 checks passed
@silverwind silverwind deleted the validate-before-execute branch May 28, 2026 07:34
silverwind added a commit that referenced this pull request May 28, 2026
* update deps (silverwind)
* Pre-validate before mutation; atomic push prevents orphan tags (#49) (silverwind)
* simplify: share TOML walker, extract commit/tag/push, derive push gates (silverwind)
* make update a combination target, split out update-js (silverwind)
* add update-actions make target (silverwind)
* remove authorship attribution rule from AGENTS.md (silverwind)
* simplify: dedup forge helpers, pyproject section loop, changelog parser (silverwind)
* fix: tighten lock-file regex, preserve CRLF in changelog updates (silverwind)
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.

2 participants