test(version): cover positionAgainstRange's MAX_SAFE_INTEGER boundary (#1265)

[prx-my]

What changed and why

Follow-up to #1257, which fixed positionAgainstRange throwing above Number.MAX_SAFE_INTEGER as a side effect of tidying up OA008 range handling, and pinned it with a regression test.

This PR adds the two pieces that were not already covered:

• A comment on the existing guard in positionAgainstRange explaining the asymmetry that makes it necessary: satisfies guards the SemVer constructor inside Range.test(), while ltr/gtr route through outside() which does not. Without that note the guard reads as removable.
• A boundary test for positionAgainstRange("9007199254740991.0.0", "*"), which returns "satisfies". The wildcard takes a different path than the existing ^1.0.0 case, so it was uncovered.

No src behaviour changes.

Related

#1265, already closed by #1257

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[prx-my]

Update: this branch has been refreshed with the latest main. It is no longer behind and now only awaits the required review/status checks before it can merge.

[sonukapoor]

Your analysis was right, and then the ground moved under you. Worth spelling out because the diff no longer matches the title.

When you opened this on the 2nd it genuinely threw. I checked out the commit before #1257 and ran it: positionAgainstRange("9007199254740992.0.0", "*") gives Invalid major version, and so does ^1.2.0, while the boundary at 9007199254740991.0.0 returns satisfies. Exactly the boundary you described, and your root cause is right too: satisfies routes through Range.test() which swallows the constructor error, ltr and gtr go through outside() which does not.

#1257 then landed on the 5th, tidying up OA008 range handling, and fixed it as a side effect. It also shipped a regression test for it, returns null instead of throwing above semver's safe integer limit, in the file you are adding to, a few lines above your additions. So on current main nothing throws and the behaviour is already pinned.

That leaves three things in your PR and they are not equal.

The comment is worth keeping. It records the asymmetry between satisfies and ltr/gtr, which is the thing that would let someone "simplify" the guard away later. The existing test pins the behaviour; nothing explains the mechanism.

positionAgainstRange("9007199254740991.0.0", "*") is worth keeping too, and it is the one assertion here that is genuinely new. At the boundary * returns satisfies while the existing test's ^1.0.0 returns above, so the wildcard takes a different path and nothing currently covers it.

The other two, oversized against * and against ^1.2.0, both return null, which the existing test already asserts with ^1.0.0. Three ranges proving the same branch is the kind of thing I would normally ask you to trim.

So: keep the comment, keep the boundary-with-* assertion, drop the two oversized duplicates, and retitle from fix(version) to test(version) since nothing in src changes behaviour any more. Then it is a clean addition and I will take it.

One thing on us rather than you: #1265 is still open even though #1257 fixed and pinned it on the 5th, which is why this looked like live work when you picked it up. I am closing it with a note pointing at #1257.

[prx-my]

Done. Trimmed to what you asked for.

• Kept the comment, and expanded it so it records the asymmetry rather than just the symptom: satisfies guards the constructor inside Range.test(), while ltr/gtr route through outside() which does not, so only those two throw and only above MAX_SAFE_INTEGER. It now says outright that the guard is load-bearing, which is the bit someone would delete while "simplifying".
• Kept the boundary test. positionAgainstRange("9007199254740991.0.0", "*") returns "satisfies". You were right that * takes a different path than the existing ^1.0.0 assertion.
• Dropped the two oversized duplicates (* and ^1.2.0), since the existing ^1.0.0 case already proves that branch.
• Retitled to test(version).

Branch is on current main (the rebase was a no-op, it was already there) and green locally: version-extensions 50/50, lint:tests, and build.

Thanks for catching that #1257 had already fixed and pinned this, and for the breakdown of which parts were still worth keeping. That made the trim straightforward. The note about #1265 having been left open is appreciated too.