Skip to content

fix(version): read the version from a package.json, and accept 3-digit semver - #2501

Open
YiftahR wants to merge 1 commit into
garrytan:mainfrom
YiftahR:fix/version-source-package-json-and-3-digit
Open

fix(version): read the version from a package.json, and accept 3-digit semver#2501
YiftahR wants to merge 1 commit into
garrytan:mainfrom
YiftahR:fix/version-source-package-json-and-3-digit

Conversation

@YiftahR

@YiftahR YiftahR commented Aug 10, 2026

Copy link
Copy Markdown

The problem

--version-path / .gstack/version-path already let a repo point the version tooling anywhere, but two shapes still failed — and both failed closed, so instead of erroring they silently disabled /ship's queue-collision check.

In a repo whose version lives in frontend/package.json as 3-digit semver:

$ bun run bin/gstack-version-bump classify --base main
fatal: path 'VERSION' does not exist in 'origin/main'
{"state":"FRESH","baseVersion":"0.0.0.0","currentVersion":"0.0.0.0","pkgVersion":null,"pkgExists":false}

$ bun run bin/gstack-next-version --base main --bump patch --current-version 0.99.2
Error: could not parse base version '0.99.2'

Two independent causes:

  1. A package.json version source was read as raw text. The pinned path was whitespace-stripped, so a JSON file became {"name":"frontend","version":"0.99.2",... — which parseVersion rejected, and every read fell back to the 0.0.0.0 default. This also silently discarded rival PRs' claims: the GitHub Contents API path base64-decodes the file and then parses it, so every competing claim was dropped as "malformed".
  2. parseVersion required exactly four components, so gstack-next-version exited 2 on every invocation in a 3-digit repo.

The second one is the damaging half, because gstack-next-version is the queue-collision check. With it erroring, /ship takes its documented offline path — naive local patch arithmetic. Two branches cut from the same base then pick the same version, and git merges that without a conflict, because both sides set one line to identical text. The second PR's bump silently evaporates: two PRs ship as one version, and only one gets a CHANGELOG entry.

We hit that six times in one repo before working out why — the tests pass, the diff looks right in isolation, and git merge reports success, so there's nothing to notice.

The fix

lib/version-source.ts holds the semantics so both CLIs agree by construction. Detection is by shape, not new configuration — no new flags or config keys:

  • a version-path ending in .json is read and written as JSON via .version
  • a version with three components stays three components through bumping and formatting

A repo with a root VERSION and 4-digit versions sees no behaviour change.

After:

$ bun run bin/gstack-version-bump classify --base main
{"state":"FRESH","baseVersion":"0.99.2","currentVersion":"0.99.2","pkgVersion":"0.99.2","pkgExists":true}

$ bun run bin/gstack-next-version --base main --bump micro --current-version 0.99.2
{"version": "0.99.3", "base_version": "0.99.2", "version_path": "frontend/package.json",
 "host": "github", "offline": false,
 "warnings": ["--bump micro has no component to move in a 3-digit version; treated as patch"]}

Decisions I'd most like a second opinion on

  • MICRO on a 3-digit version is carried out as a PATCH, with a warning in the output. /ship auto-picks MICRO by default, so erroring would make it unusable in every 3-digit repo — and a silent no-op would be worse, since the caller would write back the version it started with and claim a slot already taken. A hard error with a "pin a 4-digit VERSION file instead" message is the other defensible option.
  • When the version-path IS a package.json, it's treated as the single source of truth: the only file written, and the DRIFT_* states can't arise (no second file to drift from), so classify returns only FRESH/ALREADY_BUMPED and repair is a no-op. Also syncing a root package.json there would be guessing which of two JSON files the repo publishes from.

Also fixes a pre-existing bug

gstack-version-bump derived versionRel from the CLI flag alone, ignoring the .gstack/version-path pin — so a pinned repo compared its local (pinned) version against the base's root VERSION: two different files. On a repo with no root VERSION the base then always read 0.0.0.0, making every branch look FRESH.

Tests

test/gstack-next-version.test.ts and test/gstack-version-bump.test.ts, in a clean clone of this branch:

$ bun test test/gstack-version-bump.test.ts test/gstack-next-version.test.ts
 56 pass
 0 fail
Ran 56 tests across 2 files.

New coverage: 3-digit parsing/width, formatting narrowing, the MICRO→PATCH carry, slot-picking within a width, extractVersion for JSON vs plain-text paths, and an end-to-end temp-repo suite driving classifywriteclassifyrepair against a pinned frontend/package.json — asserting the values that used to be wrong (baseVersion was 0.0.0.0, pkgExists was false).

I also ran the wider static tier. It has failures I could not attribute to this change, and I checked rather than assumed — stashing the change and re-running gives the same result: swift build invariants > XCTest suite for StateServer fails identically on unmodified main here (toolchain), and the 5 gstack-gbrain-detect failures pass in isolation and reference none of the changed modules (they look like cross-file interference over HOME/GSTACK_HOME in a concurrent full run). Happy to dig into either if they're not already known.

Two existing assertions encoded the old contract and are updated with the reasoning inline — parseVersion('1.2.3') was asserted null, and VERSION_RE was asserted to reject 3-digit. The garbage-rejection cases are kept and extended (1.2, 1.2.3.4.5, v1.2.3.4, 1.2.3.x).

Deliberately not included

  • ship/SKILL.md prose. Documenting the new shapes there breaks the byte-for-byte golden test, and fixing it properly means regenerating three host-variant copies and three golden fixtures — that reads like a release chore, and refreshing the goldens myself would make that test tautological. Happy to add the prose in a follow-up if you point me at the right regeneration path.
  • CHANGELOG.md / VERSION. Those look maintainer-owned per the wave process in CONTRIBUTING.md. Say the word and I'll add an entry.

…t semver

The version-path pin (--version-path / .gstack/version-path) already let a repo point
the version tooling anywhere, but two real-world shapes still failed — and both failed
CLOSED, which silently disabled /ship's queue-collision check rather than erroring:

1. A package.json as the version source. The readers treated the pinned path as raw
   text, so a JSON file was whitespace-stripped into
   '{"name":"frontend","version":"0.99.2",...' — which parseVersion rejected, so every
   read fell through to the 0.0.0.0 default. This bites any repo whose version lives in
   a package.json rather than a plain VERSION file, at the root or not.

2. 3-digit semver. parseVersion required exactly four components, so gstack-next-version
   exited 2 on every invocation in such a repo. That CLI *is* the queue-collision check,
   so /ship took its documented offline path — naive local patch arithmetic. Two branches
   cut from the same base then pick the same version, and git merges that WITHOUT a
   conflict because both sides set one line to identical text. The duplicate slot ships
   silently: two PRs land as one version, and only one gets a CHANGELOG entry. We hit
   this six times in one repo before working out why.

lib/version-source.ts now holds the semantics so both CLIs agree by construction.
Detection is by shape rather than new configuration: a version-path ending in .json is
read (and written) as JSON via .version; a version string with three components stays
three components through bumping and formatting. A repo with a root VERSION file and
4-digit versions sees no behaviour change.

Details worth reviewing:

- MICRO on a 3-digit version is carried out as a PATCH, with a warning in the output.
  /ship auto-picks MICRO by default, so erroring would make it unusable in every 3-digit
  repo; a silent no-op would be worse, since the caller would write back the version it
  started with and claim a slot already taken.
- When the version-path IS a package.json, that file is the single source of truth: it is
  the only file written, and the DRIFT_* states cannot arise (there is no second file to
  drift from), so classify returns only FRESH / ALREADY_BUMPED and repair is a no-op.
  Also syncing a root package.json there would be a guess about which of two JSON files
  the repo publishes from.
- Fixes a pre-existing bug in gstack-version-bump: versionRel was derived from the CLI
  flag alone, ignoring the .gstack/version-path pin, so a pinned repo compared its local
  version against the BASE's root VERSION — two different files. On a repo with no root
  VERSION the base then always read as 0.0.0.0 and every branch looked FRESH.

Two existing assertions encoded the old 4-digit-only contract (parseVersion('1.2.3')
is null; VERSION_RE rejects 3-digit). Both are updated with the reasoning inline, and
the garbage-rejection cases are kept and extended.

ship/SKILL.md is deliberately untouched: documenting the new shapes there also requires
regenerating the three host-variant copies and three golden fixtures, which looks like a
release chore rather than something to guess at from outside. Happy to add the prose in a
follow-up if you tell me the right way to regenerate those.
@trunk-io

trunk-io Bot commented Aug 10, 2026

Copy link
Copy Markdown

Merging to main in this repository is managed by Trunk.

  • To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

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