[prx-my]

Closes #1180

Problem

scanProjectForPackageUsage() stops after 5000 source files and gives the
caller no signal that the walk was truncated. "Not found because we scanned
everything" and "not found because we stopped scanning" had the same
representation ([]), so on a large repo a package whose only import sat past
the cutoff was recorded as imported: false, and --only-used filtered the
finding out. A vulnerable repo could scan green.

The cap itself is fine; the ambiguity is the bug.

Fix

• src/usage/scanner.ts now returns a typed result
  (PackageUsageScanResult / AllImportsScanResult) carrying
  truncated + scannedFiles. truncated is set only when an eligible file
  beyond the cap was skipped, so "didn't scan it" is now distinguishable from
  "genuinely absent".
• single-scan.ts / multi-folder-scan.ts: on a truncated scan a not-found
  package is left with usage unset (unknown) instead of imported: false,
  the --only-used filter is skipped for that walk, and a one-line warning is
  printed. Findings therefore survive into --fail-on, so the gate can't be
  silently narrowed.
• context-builder.ts: scanAllImports() (PD001/PD002/OA009) exposes the same
  cap and warns via the override logger.
• Below the cap, behavior is unchanged: genuinely unused packages are still
  filtered by --only-used.

Security invariant

A package absent from the files inspected during a truncated usage scan must
never be treated as definitively unused merely because the scanner did not
reach the rest of the project.

Covered by tests/single-scan-usage.test.ts (finding retained, usage
unknown, reachesFailOn still true on a >5000-file project) and the
handleMultiFolderScan - truncated usage gate cases (truncated → EXIT_FINDINGS,
complete → EXIT_OK).

Tests

• tests/usage.test.ts: boundary tests at 4999 / 5000 / 5001 files for both
  scanners (deterministic, order-independent fixture).
• tests/single-scan-usage.test.ts (new): complete vs truncated --only-used
  semantics at the scanProject level.
• tests/multi-folder-scan.test.ts: mocks updated; truncated gate keeps
  findings, complete scan still filters.
• tests/overrides/context-builder.test.ts: warns only when the cap is hit.

Verification

• npx tsc -p tsconfig.json --noEmit — clean
• npm run lint:tests — passed
• npm run build — OK
• npm test — 146 suites / 1948 tests passing

[Copilot]

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

[prx-my]

@sonukapoor hey, this is gtg for the review, lmk if anything i missed

[sonukapoor]

Approving. This is the most consequential of the open PRs and I want to record the verification, because the bug it fixes is the worst output this tool can produce.

I reproduced it on main first: a project with 5,201 source files where the only require('tmp') sits past the cap, and tmp@0.2.5 is genuinely vulnerable. --usage --only-used on main reports "Scan complete. No known vulnerabilities found." A vulnerable repository scanning green, because "we stopped looking" and "it is not there" were the same value.

On your branch, same project, same flags: 1 vulnerable package, 1 high, and the finding survives into the exit code. It also says why, which is the part I would have asked for if it were missing:

Usage scan stopped at 5000 files; packages imported only in unscanned files may appear unused.

The regression I went looking for is the one that would make this a bad fix: does --only-used still filter a package that genuinely is not imported? Built a second fixture below the cap, ten files, no import of tmp anywhere, and it correctly reports no vulnerabilities. So the filter still does its job and only steps aside when the walk was actually truncated.

146 suites, 1948 tests green.

Your framing in the description is the right one and worth keeping in the commit history: the cap itself is fine, the ambiguity was the bug. Leaving usage unset rather than imported: false is what makes the two states distinguishable, and it is the sort of distinction that is obvious only after someone draws it.

This is behind main and will want a rebase before merge.

[prx-my]

Thanks for the thorough verification and for testing both the truncated and genuinely-unused cases. Glad the distinction between false and unknown holds as intended. I’ll rebase onto main, rerun the verification, and update the PR.

I will rebase it in next 30 mins im making my dinner

[sonukapoor]

This is a real bug and a serious one, so I want to lead with that. I reproduced it before reviewing the code.

I built a project with 5,201 source files over the direct-fixable lockfile, with one file importing axios@0.21.1, which carries 24 CVEs and rates high. On current main:

✔ Scan complete. No known vulnerabilities found.
exit=0

That is a false all-clear with a green CI gate on an imported, high-severity direct dependency. Your branch reports 1 vulnerable package, 1 urgent (1 high) on the identical input. Same class as #1159, which we shipped in v1.35.0 and had to fix.

One refinement worth putting in the issue, because it makes this sneakier than "projects over 5,000 files are affected". It depends on scan order. My first attempt put the importing file at src/app.js and main found the CVE fine, because that file is walked before the filler and the cap never touches it. Only when I moved it to src/zzz/app.js, past the cap, did the all-clear appear. So the trigger is not the file count alone, it is whether the importing file happens to fall beyond the cap, which means it can appear and disappear as a repo is reorganised.

The boundary test with it.each([4999, 5000, 5001]) is better than most work I see here, and establishing that exactly 5,000 is not truncation is the kind of thing that saves an argument later. Asserting real exit codes in both directions in the multi-folder tests is the strongest form available.

Four things before it lands.

1. The warning does not say the thing the user needs, and describes behaviour this PR removes. It says packages "may appear unused". After your change they do not appear unused, the filter is skipped entirely. What it never says is the actionable part: that --only-used filtering did not happen, which is why the finding count jumped. There is also no remedy offered, because there is no flag to raise the cap, which leads to item 4.

2. Three near-duplicate strings, with two different endings. context-builder.ts:91 ends "may not be detected", single-scan.ts:723 and multi-folder-scan.ts:110 end "may appear unused". This is the repeated-string-literal case we flag in review; one exported helper beside MAX_FILES_TO_SCAN covers all three and stops them drifting further apart than they already have.

3. The disclosure is terminal-only, and --json is what CI and the GitHub App actually read. logWarn returns early under --json. I checked the emitted file on the truncated project: there is a top-level warnings array and it comes back []. So a machine consumer gets a finding with "usage": null and no way to tell truncation from "--usage was never run". The scan result already carries warnings, and that array already flows to both the JSON and the HTML notices panel, so pushing the message there fixes every surface at once.

4. It collides with #1175, which merged this morning. contextual-signals.ts documents its usage signal as "Null when --usage was not run", and computeUsageSignal returns null on !finding.usage. Your change makes truncation produce that same absent value, so the prioritization block will report "not run" when it did run and was cut short. That needs a decision rather than being left to land quietly: either a tri-state that can say "unknown", or an explicit note that we accept the conflation. I would rather have the tri-state.

And one missing test. Hoisting the let usageTruncated declaration in multi-folder-scan.ts out of the per-folder loop leaves the whole suite green. That mutation should fail something: with it hoisted, one truncated folder disables --only-used for every folder after it. Your code is correct today, nothing pins it. It is the same cross-folder over-block we caught on #937.

Separately, and not something I am asking you to solve here: on a project past the cap, --only-used silently becomes a no-op with no escape hatch, so an adopter gating CI on --only-used --fail-on high will start seeing failures with no way back. Making MAX_FILES_TO_SCAN injectable with a 5,000 default would give them an opt-out, make the warning actionable, and make your tests cheaper than materialising 30,000 files. Worth considering, but I am happy for it to be a follow-up.

[prx-my]

Rebased onto the latest upstream/main (331940f) and force-pushed.

During the rebase, upstream #1114 had changed the usage scanner APIs to return raw Record/Map values. Since this PR wraps those results with { usage/imports, truncated, scannedFiles }, I updated the four affected upstream tests to consume the wrapped API. No new behavior or scope was introduced.

Post-rebase verification:

• npm test: 154 suites / 2122 tests passed
• npx tsc --noEmit: passed
• npm run build: passed
• test hygiene: passed
• working tree clean

[prx-my]

@sonukapoor

[sonukapoor]

This is good now. All four asks are in, and the one that convinced me is the SARIF side rather than the terminal.

On a 5,001 file project where the importing file sorts past the cap, main writes a SARIF file with zero results and exits 0. Your branch writes 24 and exits 1. We have adopters gating merges on that file, so they were being handed an empty report on a project with a high finding.

The warning says the consequence now instead of describing the behaviour you removed, the three strings are one helper, the disclosure reaches --json and both HTML reporters, and the usage: null collision with #1175 is properly resolved into three distinct states. The cap boundary is exact at 4999, 5000 and 5001 for both scanners.

Two things I am taking as follow-ups rather than holding this up for.

The warning still does not say what to do about it. There is no flag for the cap, so the only remedy is scanning a narrower path and the message never mentions it.

And three rendering paths are unpinned: the table Usage cell, the compact usage line and the HTML risk summary. Delete the "we did not look" branch in any of them and the suite stays green while the output flips to a green unused on the same scan whose warning says filtering was skipped. The code is right in your PR, it just has nothing holding it there, and those are the three places a person actually reads it.

I have filed both. Merging this.

[prx-my]

@sonukapoor i saw it wasnt up to date do i updated it

[sonukapoor]

Merged, thank you @prx-my.

The thing that sold it in the end was the SARIF side. On that 5,001 file case main was writing a file with zero results for a project with a high finding, and people gate merges on that file. Good catch all round, and thanks for turning the review round twice.

Both follow-ups I mentioned are filed together as #1259, so they do not get lost.

[prx-my]

@sonukapoor would like to work on the more issues as i have 8 merged pr and i want to follow up with proper contribution to the community as i understood the code base, if you can assign me some in house issue to check my approches then it will be great.
Thanks

[sonukapoor]

Thanks, and the work has been consistently good. Eight merges, and you reviewed two other people's PRs this morning unprompted, which is more than I asked for.

I am not going to assign you anything, and that is not a brush-off. We dropped assignment entirely in September after three collisions in three days: an issue was assigned to one person and somebody else opened a PR for it within hours, because nobody checks the assignee. So an assignment from me would read as a reservation while the issue still looks open to everyone, and I would end up breaking that in public. First working PR wins, for everyone including you.

What your question did do is make me re-read the in-house labels, and two of them were wrong. #1243 and #1244 are both well scoped with a clear success criterion, which is exactly the kind of thing that should be out in the open rather than sitting with me. I have taken the label off both.

#1244 in particular looks like your sort of change. It is the behaviour-neutral refactor shape you have done three times now, and the SARIF entry points have grown to eight positional parameters.

Two others worth knowing about, no claim implied on any of these. #1265 is a real crash in positionAgainstRange that is live on main right now, in the same file as your #1183. #1263 is a field that went missing from report.json when it was unified with --json.

The two that genuinely stay with me are #1120 and #1232. #1120 is scan-loop performance, where the measurement matters as much as the fix, so I keep that in-house. #1232 is architectural and the pnpm half of it does not have an answer yet.