fix(check): honor package-lock.json when installing from package_file by marschattha · Pull Request #2829 · qltysh/qlty

marschattha · 2026-07-22T22:14:44Z

Summary

Fixes a user-reported bug where package-lock.json was not honored when installing node-based linters with package_file, contradicting the documented behavior ("When using package_file, Qlty respects the locked versions for reliability").

Problem

Since #1658, update_package_json copies the repo's package-lock.json into the tool staging directory (when package_file is set and no package_filters are configured). However, package_file_install then runs npm install --force --no-package-lock — and npm's --no-package-lock flag makes npm ignore lockfiles during install. The flag predates the lockfile-copy feature and was never removed, so the copied lockfile had no effect and npm resolved the latest versions matching the package.json semver ranges.

Repro: a repo with "eslint": "^8.0.0" in package.json, a lockfile pinning eslint to 8.0.0, and package_file = "package.json" in qlty.toml installed eslint 8.57.1 instead of the locked 8.0.0.

Fix

Drop --no-package-lock from the install command when a lockfile was staged. The flag is kept when no lockfile is present, and in the package_filters case, where no lockfile is copied since the filtered package.json would conflict with the full lockfile (also matches the documented behavior: "When using package_file with package_filters, the lock files are ignored").

Testing

New unit tests: lockfile staged → flag dropped; package_filters with lockfile → lockfile not staged, flag kept
Verified end-to-end with the repro above: eslint now installs at the locked 8.0.0
Full workspace test suite passes

🤖 Generated with Claude Code

The lockfile copied into the tool staging directory was being ignored because npm install ran with --no-package-lock, which tells npm to skip reading lockfiles entirely. Linters were installed at the latest versions matching the package.json semver ranges instead of the locked versions, contradicting the documented behavior. Drop --no-package-lock when a lockfile was staged (package_file set with no package_filters), and keep it otherwise since a filtered package.json would conflict with the full lockfile. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

claude

Claude Code Review

Claude Code Review is paused for this repository. To reconnect it, an admin of this repository's GitHub organization (or the account owner, for personal repositories) who can also manage your Claude organization's Code Review settings needs to re-link GitHub in Code Review settings. This is a one-time step.

_{Tip: disable this comment in your organization's Code Review settings.}

Copilot

Pull request overview

Fixes qlty check Node linter installs to actually respect a staged package-lock.json when package_file is used (and package_filters are not), aligning behavior with the documented lockfile semantics.

Changes:

Conditionally drops npm install --no-package-lock when a lockfile is staged so npm will honor locked versions.
Adds unit tests covering the “lockfile staged” case and the “package_filters ignores lockfiles” case.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

qltysh · 2026-07-22T22:19:39Z

Coverage Impact - ubuntu-latest

This PR will not change total coverage.

Modified Files with Diff Coverage (2)

File	% Diff	Uncovered Line #s
qlty-check/src/tool/node.rs	95.3%	416, 449, 473
qlty-check/src/tool/node/package_json.rs	100.0%
Total	96.3%

🤖 Increase coverage with AI coding...

In the `ma/fix-node-package-lock-honored` branch, add test coverage for this new code:

- `qlty-check/src/tool/node.rs` -- Lines 416, 449, and 473

🚦 See full report on Qlty Cloud »

🛟 Help

Diff Coverage: Coverage for added or modified lines of code (excludes deleted files). Learn more.
Total Coverage: Coverage for the whole repository, calculated as the sum of all File Coverage. Learn more.
File Coverage: Covered Lines divided by Covered Lines plus Missed Lines. (Excludes non-executable lines including blank lines and comments.)
- Indirect Changes: Changes to File Coverage for files that were not modified in this PR. Learn more.

qltysh · 2026-07-22T22:20:25Z

Coverage Impact - macos-15

⬆️ Merging this pull request will increase total coverage on main by 0.01%.

Modified Files with Diff Coverage (2)

File	% Diff	Uncovered Line #s
qlty-check/src/tool/node.rs	95.3%	416, 449, 473
qlty-check/src/tool/node/package_json.rs	100.0%
Total	96.3%

🤖 Increase coverage with AI coding...

In the `ma/fix-node-package-lock-honored` branch, add test coverage for this new code:

- `qlty-check/src/tool/node.rs` -- Lines 416, 449, and 473

🚦 See full report on Qlty Cloud »

🛟 Help

Diff Coverage: Coverage for added or modified lines of code (excludes deleted files). Learn more.
Total Coverage: Coverage for the whole repository, calculated as the sum of all File Coverage. Learn more.
File Coverage: Covered Lines divided by Covered Lines plus Missed Lines. (Excludes non-executable lines including blank lines and comments.)
- Indirect Changes: Changes to File Coverage for files that were not modified in this PR. Learn more.

The initial tool installation (npm install <tool>@<version>) generates a package-lock.json in the staging directory, so checking for lock file existence wrongly dropped --no-package-lock for repositories without a lock file. This surfaced in the stylelint plugin test, where the leftover lock pinned stylelint to the exact plugin version instead of re-resolving the merged package.json. Track whether update_package_json actually copied the user's lock file and key the install command off that instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

marschattha · 2026-07-22T22:44:25Z

The Plugin Tests failure (stylelint fixture=extends version=17.3.0) was caused by the first version of this fix: the initial tool install step (npm install --force <tool>@<version>) generates a package-lock.json in the staging directory, and the existence check treated that leftover as a user lock file, dropping --no-package-lock for repositories that have no lock file at all. npm then honored the leftover lock (pinning stylelint to exactly 17.3.0) instead of re-resolving the merged package.json (17.14.x), which changed the block-no-empty message wording.

Fixed in 60938c8 by tracking whether update_package_json actually copied the user's lock file and keying the install command off that instead of filesystem state. Verified locally: the stylelint and eslint plugin test suites pass, and the original repro still installs the locked version.

🤖 Generated with Claude Code

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

noahd1

Approving - but please evaluate the recently added GitHub Co-Pilot comment. May be worth addressing, I can't tell.

marschattha · 2026-07-22T23:55:34Z

Approving - but please evaluate the recently added GitHub Co-Pilot comment. May be worth addressing, I can't tell.

Its an pre-existing and unrelated code cleanup, but might as well.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

) ## Summary PHP counterpart to #2829: `composer.lock` was not honored when installing PHP linters with `package_file`, contradicting the [documented behavior](https://docs.qlty.sh/cli/linter-extensions#lock-files) ("When using `package_file`, Qlty respects the locked versions for reliability"). ## Problem Since #1658, `update_composer_json` copies the repo's `composer.lock` into the tool staging directory (when no `package_filters` are configured), but the package file was then installed with `composer update --no-interaction --ignore-platform-reqs` — and `composer update` re-resolves dependencies and rewrites the lock, ignoring the locked versions entirely. **Repro:** a repo with `"squizlabs/php_codesniffer": "^3.8"` in `composer.json`, a lockfile pinning `3.8.0`, and `package_file = "composer.json"` in `qlty.toml` installed phpcs **3.13.5** instead of the locked **3.8.0**. ## Fix When the repository's `composer.lock` was staged, attempt `composer install` (which installs exactly what the lock records). If it fails — a stale lock, a missing package, a constraint mismatch, anything composer's lock validation rejects — fall back to `composer update` with a warning, restoring the previous behavior at the cost of one fast-failing composer run. One wrinkle: qlty collapses `require-dev` into `require` in the staged composer.json, but `composer install` validates required prod packages against the lock's prod section — so a tool locked under `packages-dev` (the common case for linters) would hard-error with "Required package ... is not present in the lock file". The staged lock therefore gets the same treatment: `packages-dev` entries are collapsed into `packages`. ## Testing - New unit tests: lock staged → `composer install`; install failure → falls back to `composer update`; `package_filters` set → lock not staged, `composer update` only; dev packages collapsed in the staged lock - Verified end-to-end: locked phpcs 3.8.0 installs (including when locked under `require-dev`); a stale lock (constraint `^3.9` vs locked `3.8.0`) falls back to update and succeeds; without a lockfile, behavior is unchanged - phpstan, php-codesniffer, and php-cs-fixer plugin test suites pass locally - Full qlty-check test suite passes (249 tests) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

Automated PR for release 0.639.0. Review (and edit, if needed) the changelog entry below, then merge this pull request to publish the release. ## Draft release notes ### Fixed - Honor `package-lock.json` when installing node-based linters from `package_file`, so the locked versions are respected instead of npm resolving the latest versions matching the `package.json` semver ranges (#2829) - Honor `composer.lock` when installing PHP linters from `package_file`, so the locked versions are respected instead of `composer update` re-resolving to the latest matching versions (#2830) Co-authored-by: qlty-releases[bot] <181762136+qlty-releases[bot]@users.noreply.github.com>

Copilot AI review requested due to automatic review settings July 22, 2026 22:14

Copilot started reviewing on behalf of marschattha July 22, 2026 22:15 View session

marschattha marked this pull request as ready for review July 22, 2026 22:17

claude Bot reviewed Jul 22, 2026

View reviewed changes

marschattha requested a review from noahd1 July 22, 2026 22:17

Copilot AI reviewed Jul 22, 2026

View reviewed changes

Comment thread qlty-check/src/tool/node.rs Outdated

Copilot AI review requested due to automatic review settings July 22, 2026 22:44

Copilot started reviewing on behalf of marschattha July 22, 2026 22:44 View session

Copilot AI reviewed Jul 22, 2026

View reviewed changes

Comment thread qlty-check/src/tool/node/package_json.rs Outdated

docs(check): clarify lock file staging comment on update_package_json

7aa11d3

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copilot AI review requested due to automatic review settings July 22, 2026 22:51

Copilot started reviewing on behalf of marschattha July 22, 2026 22:52 View session

Copilot AI reviewed Jul 22, 2026

View reviewed changes

Comment thread qlty-check/src/tool/node/package_json.rs Outdated

noahd1 approved these changes Jul 22, 2026

View reviewed changes

refactor(check): use configured node package file

66eb6e4

Copilot AI review requested due to automatic review settings July 22, 2026 23:58

Copilot started reviewing on behalf of marschattha July 22, 2026 23:58 View session

Copilot AI reviewed Jul 23, 2026

View reviewed changes

test(check): normalize node file dependency path

8a8d52a

Copilot AI review requested due to automatic review settings July 23, 2026 00:08

Copilot started reviewing on behalf of marschattha July 23, 2026 00:08 View session

Copilot AI reviewed Jul 23, 2026

View reviewed changes

marschattha merged commit 5acfb65 into main Jul 23, 2026
18 checks passed

marschattha deleted the ma/fix-node-package-lock-honored branch July 23, 2026 14:46

marschattha mentioned this pull request Jul 23, 2026

fix(check): honor composer.lock when installing from package_file #2830

Merged

qlty-releases Bot mentioned this pull request Jul 23, 2026

Release 0.639.0 #2831

Merged

coderabbitai Bot mentioned this pull request Aug 12, 2026

ci(#345): mutate what a PR changes, and census the whole tree nightly VilnaCRM-Org/website#429

Merged

Navigation Menu

Uh oh!

fix(check): honor package-lock.json when installing from package_file - #2829

fix(check): honor package-lock.json when installing from package_file#2829
marschattha merged 5 commits into
mainfrom
ma/fix-node-package-lock-honored

marschattha commented Jul 22, 2026

Uh oh!

claude Bot left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

qltysh Bot commented Jul 22, 2026 •

edited

Loading

Uh oh!

qltysh Bot commented Jul 22, 2026 •

edited

Loading

Uh oh!

marschattha commented Jul 22, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

noahd1 left a comment

Uh oh!

marschattha commented Jul 22, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

marschattha commented Jul 22, 2026

Summary

Problem

Fix

Testing

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Claude Code Review

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

qltysh Bot commented Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qltysh Bot commented Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

marschattha commented Jul 22, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

noahd1 left a comment

Choose a reason for hiding this comment

Uh oh!

marschattha commented Jul 22, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

qltysh Bot commented Jul 22, 2026 •

edited

Loading

qltysh Bot commented Jul 22, 2026 •

edited

Loading