Skip to content

[Bug] OA008 calls every flagged copy "below the floor", including copies above the range #1190

Closed

Description

@sonukapoor

OA008 describes every copy it flags as "below the floor", but for a range pin it flags anything that does not satisfy the range, which includes copies above it.

Reproduction

Override ^1.2.0, one installed copy at 2.0.0:

Override demands pkg ^1.2.0, but 1 installed copy (2.0.0) is below the floor.
The override is not effective everywhere - likely a parent declares this dep as
exact and wins resolution.

2.0.0 is above the range, not below the floor.

Cause

src/overrides/detectors/oa008-materialized.ts:33-43 selects copies two different ways:

if (isValidRange(pinValue) && !isConcrete(pinValue)) {
  return !satisfiesRange(c.version, pinValue);   // anything OUTSIDE the range
}
const floor = coerceVersion(pinValue);
return floor ? compareVersions(c.version, floor) < 0 : false;   // strictly below

The concrete-pin branch really does mean "below the floor". The range branch means "outside the range", which is a superset. The message at :57 only describes the first.

Why it matters

The finding is correct; the explanation is not. A maintainer reading it goes looking for an old copy that does not exist, and the closing sentence compounds it by asserting a specific cause ("a parent declares this dep as exact") that does not fit an above-range copy either.

Not new, but about to get louder

main mislabels this today. #1183 tightens the caret rule so that ^0.x ranges stop admitting versions npm would never resolve, which means more copies fall outside the range and this message fires more often. Found independently while reviewing #1181 and #1183, and promised to both contributors.

Fix

Describe what was actually detected. Split the message by branch: "below the floor" for a concrete pin, and something like "does not satisfy" naming the range for a range pin, with the copies partitioned into below and above where both are present. The speculative closing sentence should probably only appear in the case it actually explains.

Related: #1181 fixes the copyies typo and the lexicographic ordering in this same string, and does not touch the wording.

Activity

  1. added
    bugSomething isn't working
    in-houseMaintainer-handled internal work - not open for contribution
    on Sep 20, 2026
  2. anbv29 commented on Sep 24, 2026

    @anbv29
    Contributor

    Hi! I’d like to create a small mixed-version fixture for OA008, correct the classification/message, and add regression tests. Is there a preferred fixture style?

  3. added
    help wantedExtra attention is needed
    and removed
    in-houseMaintainer-handled internal work - not open for contribution
    on Sep 26, 2026
  4. sonukapoor commented on Sep 26, 2026

    @sonukapoor
    CollaboratorAuthor

    @anbv29 - yours, go ahead. I have taken the in-house label off this one.

    On fixture style: OA008's tests build the context in-process rather than writing files to disk. tests/overrides/detectors/oa008.test.ts has a ctxOf(entries, copies) helper and a copy(name, version, path) helper, and each test just hands the detector the entries and installed copies it needs. That is the style to follow here, and a mixed-version case is only a couple of lines:

    const ctx = ctxOf(
      [e('pkg', '^1.2.0')],
      { pkg: [copy('pkg', '1.0.0', '/x/node_modules/a/node_modules/pkg'),
              copy('pkg', '2.0.0', '/x/node_modules/b/node_modules/pkg')] },
    );

    No on-disk fixture needed. examples/ is for end-to-end scans, and this rule reads ctx.installedCopies directly, so the unit-level context is both faster and more precise about what you are testing.

    Three things worth knowing before you start.

    The detection is correct, only the wording is wrong. Do not change which copies get flagged. The range branch deliberately flags anything outside the range, and that is right; it is the sentence that says "below the floor" which is not.

    #1181 already landed and touched this same string for the pluralisation and ordering. So work from current main rather than the message quoted in the issue body, which is now slightly stale.

    The closing sentence is part of the bug, not just the first half. "Likely a parent declares this dep as exact and wins resolution" explains a below-floor copy and does not explain an above-range one, so it should probably only appear in the case it actually fits.

    A test that fails before your change and passes after is the thing I will look for most. If the partitioning gets awkward when both below-range and above-range copies are present in one finding, say so on here and we can talk about the shape rather than you guessing.

  5. anbv29 commented on Sep 30, 2026

    @anbv29
    Contributor

    @sonukapoor @prx-my branch is now updated with the current head of main, and all checks are green. Ready for the re-review whenever you have a moment. Thank you!

  6. added a commit that references this issue on Sep 30, 2026
    0003d6f
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

    bugSomething isn't workinghelp wantedExtra attention is needed

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions