Skip to content

fix: preserve numeric prerelease representation when incrementing - #916

Draft
CHLIN0 wants to merge 1 commit into
npm:mainfrom
CHLIN0:codex/prerelease-counter-boundary
Draft

CHLIN0 wants to merge 1 commit into
npm:mainfrom
CHLIN0:codex/prerelease-counter-boundary

Conversation

@CHLIN0

@CHLIN0 CHLIN0 commented Oct 7, 2026

Copy link
Copy Markdown

What / Why

Repeatedly incrementing the same SemVer instance can stop advancing when a numeric prerelease identifier crosses the safe-integer storage boundary. The constructor keeps numeric prerelease identifiers at or above MAX_SAFE_INTEGER as strings, but the increment loop leaves them as numbers. After reaching 2^53, another increment can round back to the same number.

Minimal reproduction:

const { SemVer } = require('semver')
const v = new SemVer('1.2.3-beta.9007199254740990')
for (let i = 0; i < 4; i++) console.log(v.inc('prerelease').version)
Call Before After
1 1.2.3-beta.9007199254740991 1.2.3-beta.9007199254740991
2 1.2.3-beta.9007199254740992 1.2.3-beta.9007199254740991.0
3 1.2.3-beta.9007199254740992 1.2.3-beta.9007199254740991.1
4 1.2.3-beta.9007199254740992 1.2.3-beta.9007199254740991.2

Change and scope

The four added source lines convert incremented identifiers at or above MAX_SAFE_INTEGER (9007199254740991) to strings, matching the constructor. Subsequent calls use the existing appended-counter behavior and agree with incrementing reparsed versions.

This is a rare boundary case involving extremely large counters and repeated use of the same instance. It does not add arbitrary-precision arithmetic or change the version comparison algorithm, main-version calculations, dependencies, or tooling.

Regression coverage checks plain numeric identifiers, prefixed identifiers, and numeric identifiers followed by text, with four consecutive increments for each. Each step must increase precedence, match reparsed behavior, and preserve constructor representation.

Validation

On macOS / Node 22.23.0, npm 10.9.8, against base 6e05b7637396ac66522cff8731f07cfe0ef49a29:

  • New class regression before the fix: 27 failed / 881 passed assertions; exit 1.
  • Targeted class and public increment tests after the fix: 1,420 assertions pass; exit 0.
  • Full TAP suite: 51 files / 9,218 assertions pass; statement, branch, function, and line coverage all 100%.
  • ESLint and git diff --check pass.
  • The complete npm test -- --reporter=terse command still exits 1 at the final template-oss-check. It reports existing template mismatches in .github/dependabot.yml, .github/settings.yml, .github/workflows/ci.yml, .github/workflows/codeql-analysis.yml, and .github/workflows/release.yml.
  • npm run postlint in a clean local clone of the base commit, using the same dependencies, reproduces the same five mismatches with a clean worktree. Those automatically maintained files are excluded from this patch, in accordance with the contribution guidelines.

References and assistance

Related to #880's large numeric identifier topic, but that PR changes comparison precision in internal/identifiers.js; this change fixes SemVer.inc() and does not depend on it. No matching increment fix was found in the pre-submission searches.

Prepared with Codex assistance and submitted as a draft for review. Remote CI and maintainer review results are not included in the local validation claims.

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