Skip to content

ship.sh bumps the version on every run, so a failed ship inflates it (dead 'skipping bump' branch) #174

Description

@garretpremo

What happens

scripts/ship.sh bumps the version unconditionally, then guards the commit with a condition that can never be false:

CURRENT_VERSION=$(node -p "require('./package.json').version")   # :347
npm version "$BUMP_LEVEL" --no-git-tag-version --quiet >/dev/null # :348  always increments
NEW_VERSION=$(node -p "require('./package.json').version")        # :349

if [ "$CURRENT_VERSION" != "$NEW_VERSION" ]; then                 # :351  always true
    git add package.json
    git commit -m "chore(release): v$NEW_VERSION" --quiet
    ...
else
    warn "Version already at $CURRENT_VERSION, skipping bump"     # :357  unreachable
fi

npm version <level> always produces a different version, so CURRENT != NEW always holds and the else branch is dead code. The bump is then committed and pushed to dev before CI is waited on.

Why it matters

Every failed ship leaves dev bumped and pushed. Re-running bumps again from the already-bumped value, so the version climbs once per attempt while nothing is published.

Observed shipping v1.17.1: five attempts were needed (four died on a GitHub Actions outage — oven-sh/setup-bun returning 429/502/503 before any test ran). Without manual intervention dev would have reached 1.17.5 while npm was still on 1.17.0. Each retry required hand-editing package.json back to the last published version first, and dev still accumulated a reset/bump pair per attempt:

328a627 chore(release): v1.17.1
1961b17 chore: reset version to 1.17.0 for a clean ship attempt
070e867 chore(release): v1.17.1
7e99e74 chore: reset version to 1.17.0 so the next ship bumps to 1.17.1
aa38cec chore(release): v1.17.2      <- drift from a failed attempt

The failure mode is silent: nothing warns that the version being bumped is already ahead of the last published release. A less careful re-run publishes a version with a gap (1.17.0 → 1.17.2), or a maintainer reasonably assumes the bump is idempotent and ships the wrong number.

This is the same class as #167 — the failure and resume paths aren't exercised, so they rot. resume_untagged_release covers "merged to main but never tagged"; nothing covers "bumped and pushed but CI failed".

Proposed fix

Derive the target version from the last published release rather than from whatever package.json currently says, and make the bump idempotent:

  • Compute BASE_VERSION from origin/main's package.json (or the latest v* tag).
  • Compute TARGET = bump(BASE_VERSION, BUMP_LEVEL).
  • If package.json already equals TARGET, skip the bump and the commit — that is the genuinely-already-bumped case the dead else branch was reaching for, and it makes re-running a failed ship safe.
  • If package.json is ahead of TARGET, warn loudly and stop rather than compounding the drift.

That makes a re-run after a CI failure a no-op on the version, which is the behavior the current code appears to intend.

Acceptance criteria

  • Re-running ship.sh after a failed run does not bump the version a second time
  • The target version is derived from the last published release, not from the working package.json
  • A package.json already ahead of the computed target aborts with a clear message instead of bumping further
  • The "already at target, skipping bump" path is reachable and covered by a test
  • --bump <level> (ship.sh never pushes dev after the post-release rebase, leaving origin/dev diverged #167-era override) still composes with the above, and may still only raise the level

Related

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    needs triageMarks an issue that has not yet received acknowledgement

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions