fix(overrides): describe OA008 range mismatches accurately (#1190)

[anbv29]

Summary

• distinguish concrete OA008 floors from semver range mismatches
• partition range mismatches into below, above, and outside accepted intervals
• avoid suggesting an exact-pinned parent for range mismatches
• preserve existing detection behavior and concrete-pin guidance
• add regression coverage for above-range, mixed-version, and union-gap cases

Testing

• npm run lint:tests
• npm run build
• npm test -- --runInBand tests/overrides/detectors/oa008.test.ts tests/utils/version-extensions.test.ts

Closes #1190

[anbv29]

Hi @sonukapoor , I’ve submitted the fix based on your guidance.

I kept OA008’s detection unchanged, used the existing in-process ctxOf and copy helpers, and added regression coverage for above-range, mixed below/above, concrete-pin, and union-gap cases. The exact-parent explanation is now limited to the concrete-pin case where it applies.

The targeted tests and build pass. Please review it when you have time, and let me know if you’d prefer any changes to the wording or partitioning. Thank you!

[prx-my]

Review

Verdict: Approve

I reviewed the implementation against the reported behavior in #1190 and verified both the existing detection semantics and the new range-specific explanation logic.

Verification

• Ran:
  • npm test -- tests/overrides/detectors/oa008.test.ts tests/utils/version-extensions.test.ts
  • 58 tests passed
• Ran:
  • npm run build
  • passed
• Manually verified the new positionAgainstRange() classification against the relevant semver edge cases.

things i think which are correct

The core distinction is correct:

• Concrete pin → retains the existing floor-specific explanation.
• Range pin → uses the new range-mismatch explanation instead of incorrectly describing the violation as a concrete floor problem.

This also correctly removes the speculative "likely a parent declares this dep as exact" explanation from range-based findings, keeping it specific to concrete pins.

I verified the classifier for the important cases:

• 2.0.0 vs ^1.2.0 → above
• 2.0.0 vs 1.x || 3.x → outside
• 1.7.0 vs >=1.0.0 <1.5.0 || >=2.0.0 <2.5.0 → outside
• 1.2.0-beta.1 vs ^1.2.0 → below

The prerelease behavior is also consistent with the existing satisfiesRange() behavior using includePrerelease: true, so detection and explanation should remain aligned.

The normalization/coercion path in positionAgainstRange() also matches the existing version handling, and the detector inputs are already validated before reaching this logic.

Regression coverage

The tests cover the important behavioral boundaries:

• below-range versions
• above-range versions
• mixed below/above copies
• union-gap/outside-range versions
• concrete pins retaining their previous behavior

This gives good coverage of the actual regression described in #1190 without changing the underlying OA008 detection semantics.

Optional cleanup

One maintainability consideration: positionAgainstRange() currently duplicates some of the version/range normalization performed by satisfiesRange().

The two paths are consistent in this PR, but extracting the shared normalization into a private helper could prevent them from drifting apart if version-handling semantics change in the future.

This is not blocking for this PR.

Overall, this is a focused change that fixes the misleading OA008 range explanation while preserving the existing detection behavior.

Approved.

[anbv29]

Hey @prx-my , thank you for the approval. Looking forward to the merge and other issues where I can contribute.

[prx-my]

Hey @prx-my , thank you for the approval. Looking forward to the merge and other issues where I can contribute.

let @sonukapoor go thru this and then he will be putting the final approval please rebase the branch so it merges cleanly

[sonukapoor]

Thanks @prx-my for getting to this before me - your read of the core is right.

Good first contribution. The partitioning holds on every boundary, union-gap, prerelease and 0.x case I threw at it, and detection really is unchanged: 667 combinations of pin and installed versions, identical findings on both sides. Your tests do real work too, which is rarer than it should be - I mutated the implementation twelve ways and eleven of them failed a test.

One change before it lands, on rangeMismatchDetails. It's inline.

Two other inline notes, both optional. The docs are out of step with your output now as well, but that one's mine to fix.

[anbv29]

Thanks @sonukapoor it's done as you explained. The explanation now follows the direction of the mismatch instead of the pin shape, so a range floor with a copy below it still gets it. Test added. Did the two optionals as well.

[prx-my]

@anbv29 please update the branch make sure the ancestor is the current head of the main for this pr
soon i will review it too

[anbv29]

does it look good now?

[sonukapoor]

Approved. You went past what I asked for, which is worth saying.

The direction fix is right - I checked each case rather than reading it. A range floor with a copy below it gets the cause hint back, above-only and outside-only correctly don't, and a mixed set gets the hint alongside a partition list that shows which copies it applies to. That last one reads better than what I'd suggested.

Detection is unchanged, which is the part that mattered most: across 280 combinations of pin value and installed-version set, the findings, severities, paths and version lists are identical to main. Only the details strings differ, and only on range pins. Concrete-pin messages are byte-identical.

The version.ts fix works too. Both cases I gave you now agree with satisfiesRange, and across a large fuzz of version and range pairs I found no disagreements and nothing that throws.

You also did the two optional notes. Thanks for that.

One small thing I'm filing rather than sending back, because the code is correct and I don't want to hold this up: the version.ts fix isn't pinned by a test. If I restore the original defect - comparing against validRange's normalized output instead of the raw range - all 60 tests still pass, while positionAgainstRange("3.0.0-rc.1", "3.x") goes from satisfies back to below. One assertion closes it, and I've opened it as #1255.

I'll also correct the PR description when I squash. It still describes the earlier approach, avoiding the exact-parent explanation on range mismatches, which your latest commit deliberately reversed. Since a squash takes its message from the description, I'd rather fix it than leave the wrong thing in the history.

[sonukapoor]

Merged - thank you @anbv29, and congratulations on your first contribution.

This closes #1190. OA008 no longer tells people a copy is "below the floor" when it is above it, and the cause hint now follows the direction of the mismatch rather than the shape of the pin.

Two notes on the merge. I corrected the description when squashing: it still described the earlier approach of dropping the exact-parent explanation from range mismatches, which your last commit deliberately reversed, and a squash takes its message from the description. And the two follow-ups are filed as #1255 and #1256 rather than sitting on you.

Thanks also to @prx-my for reviewing this ahead of me and for the rebase nudge.

[anbv29]

Thank you @sonukapoor and thanks for catching the stale description before the squash. Good lesson, I'll keep it in sync with the final approach next time. I'd like to keep contributing here. Happy to take #1255 and #1256 since they came out of this change, or anything else in the tracker you'd like picked up. Thanks again @prx-my for the early review and the rebase nudge.