feat(overrides): detect unused / misconfigured pnpm dependency overrides#362
Merged
Conversation
BartWaardenburg
added a commit
that referenced
this pull request
May 12, 2026
Two CONCERN-class fixes flagged by the pre-ship review of #362. 1. Hint inversion: the unused-override `hint` field was emitted only for parent-chain shapes (`react>react-dom`), the inverse of the panel's intent. The panel's verdict #3 was "bare-target unused override stays at warn, ship a hint field" precisely to cover the bare-target transitive-CVE pattern (`debug: 'npm:obug@^1.0.2'`, react-is canary aliases, etc.). Phase 6 smoke on benchmark fixtures surfaced four real-world examples on vite + next.js, all flagged without the hint. Fix: emit the hint on every unused-override finding (both bare-target and parent-chain). The constant renames from HINT_PARENT_CHAIN_TRANSITIVE to HINT_MAY_BE_TRANSITIVE, the integration test renames from parent_chain_override_carries_transitive_hint to unused_overrides_carry_transitive_hint_on_every_shape and now exercises BOTH shapes in the same tempdir. 2. Cross-surface source naming inconsistency: JSON output emitted `source: "pnpm_workspace_yaml" / "pnpm_package_json"` (snake_case enum names) while `ignoreDependencyOverrides[].source` expected `"pnpm-workspace.yaml" / "package.json"` (filename labels). An agent reading a finding's source and copying it into a suppression rule would silently not match. Fix: serialize DependencyOverrideSource as filename labels via explicit `#[serde(rename = "...")]` per variant. Both surfaces now agree. Refs #336
BartWaardenburg
added a commit
that referenced
this pull request
May 12, 2026
Three load-bearing wiring gaps caught by codex's parallel /fallow-review of #362 plus one stale-doc concern. BLOCK 1: misconfigured-dependency-overrides did not fail CI despite the default error severity. has_error_severity_issues and promote_warns_to_errors in crates/cli/src/check/rules.rs both skipped the two new arrays, so a malformed pnpm override produced exit 0 even when the rule's default severity was error. Both functions now check the new arrays alongside unused_catalog_entries. BLOCK 2: scoped analysis + audit did not handle the new path-anchored issue types. crates/core/src/changed_files.rs::filter_results_by_changed_files filtered through unresolved_catalog_references but not the two override arrays, so dead-code --changed-since would still return unrelated override findings. Audit (crates/cli/src/audit.rs) was missing the new arrays in dead_code_keys, retain_introduced_dead_code, and annotate_dead_code_json, so audit --gate new-only would mis-attribute every override finding as 'not introduced' and the introduced flag would never land in the JSON. All five sites now mirror the unresolved_catalog_references pattern. BLOCK 3: JSON machine output was internally inconsistent. The summary block in crates/cli/src/report/json.rs omitted both new arrays, so total_issues counted findings the summary block did not surface; the action_spec_for_kind switch had no entry for either array, so finding JSON had no actions array. Added summary keys + two new ActionSpecs (remove-dependency-override fix-type for unused entries, fix-dependency-override for misconfigured entries) plus an AddToConfigIgnoreDependencyOverrides SuppressKind that emits a paste- ready { package, source } pair for ignoreDependencyOverrides. CONCERN 4: stale hint docs in unused_overrides.rs and results.rs still claimed the hint was emitted only on parent-chain shapes. Updated both doc comments to match the post-fix behavior (hint emitted on every unused finding). Snapshot tests regenerated to absorb the two new summary keys. Refs #336
BartWaardenburg
added a commit
that referenced
this pull request
May 12, 2026
… rules + suppress value) Three load-bearing wiring gaps caught by codex's parallel /fallow-review of Phase B (PR #362), missed by claude's local team review. BLOCK 1: --changed-since filtered out the new override findings. UnusedDependencyOverride.path / MisconfiguredDependencyOverride.path stored project-root-relative strings ("pnpm-workspace.yaml", "package.json"), but try_get_changed_files returns absolute paths, so the changed_files.contains check in filter_results_by_changed_files never matched. Storage flipped to absolute internally (config.root.join(...) at detection time), matching the UnresolvedCatalogReference convention shipped in #334. serde_path::serialize keeps the JSON output relative. BLOCK 2: per-file overrides.rules did not apply because crates/cli/src/check/ rules.rs::apply_rules' has_overrides branch handled file-scoped issue types but not the new override findings; setting both rules off for pnpm-workspace.yaml / package.json via config overrides still surfaced findings. Added per-file resolve in both apply_rules and has_error_severity_issues for unused_dependency_overrides and misconfigured_dependency_overrides, matching the unresolved-catalog-references fix from 176698f. BLOCK 3: JSON add-to-config suppress action could emit an unsuppressable value for misconfigured findings with the EmptyValue reason. raw_key "react@<18" was used as fallback when target_package was absent, but the suppression matcher keys on the parsed package name "react"; the suggested { package: "react@<18" } did not actually suppress the finding. Added target_package: Option<String> to MisconfiguredDependencyOverride, populated from the parsed key when it parses (EmptyValue path; remains None for UnparsableKey). JSON now reads the parsed name first and skips the suppress action entirely when no parseable package name exists, mirroring the empty- raw_key handling shipped earlier. LSP follow-up: dropped root.join in push_dependency_override_diagnostics now that finding.path is absolute; regression tests updated to construct absolute paths and assert under them. Snapshot sample data also flipped to absolute. Codex repros against the rebuilt binary now produce: - BLOCK 1: editing pnpm-workspace.yaml under --changed-since HEAD~1 retains the override findings (was 0 before, now 3) - BLOCK 2: per-file overrides.rules.{unused,misconfigured}-dependency- overrides: "off" clears the findings (was 2+2, now 0) - BLOCK 3: misconfigured react@<18 EmptyValue suggests {package: "react"} (was "react@<18"), unparsable-key emits no add-to-config action Refs #336
…des (Phase A) Two new issue types in one drop, mirroring the unused-catalog-entries + unresolved-catalog-references shape from #329/#334. - types/results: UnusedDependencyOverride + MisconfiguredDependencyOverride structs with raw_key, target_package, parent_package, version_constraint, version_range, source, path, line, and an optional transitive-CVE hint on the unused variant. - types/suppress: IssueKind discriminants 23 (UnusedDependencyOverride) and 24 (MisconfiguredDependencyOverride), parse aliases for the kebab singular / plural forms. - config: 2 new rule fields (default warn + error), IgnoreDependencyOverrideRule + CompiledIgnoreDependencyOverrideRule, ignoreDependencyOverrides on FallowConfig. - config/workspace/pnpm_overrides: new parser for the overrides: section of pnpm-workspace.yaml AND the pnpm.overrides section of root package.json. Handles bare names, scoped packages, version selectors on the target (@types/react@<18), parent matchers (react>react-dom, including scoped parents and chains with version selectors on the parent). Allowlists pnpm idioms (-, $ref, npm:alias) so they are never flagged as misconfigured. - core/analyze/unused_overrides: detector module with find_unused_dependency_overrides + find_misconfigured_dependency_overrides. Conservative static algorithm walks every workspace package.json dep section to compute the declared-packages set. Parent-chain semantics per the panel: USED when EITHER parent OR target is declared, covering the CVE-fix pattern where the override forces a transitive version under a declared parent. Bare-target unused findings carry a hint flagging the transitive-dependency possibility. Phase A scope: detector + types + config + JSON output + integration tests. Subsequent phases add CLI report formats (human, SARIF, compact, markdown, CodeClimate), LSP / MCP / NAPI / VS Code surfaces, and CI wrappers (action/jq, ci/jq, fixtures). 8 integration tests at tests/fixtures/issue-336-unused-overrides/ cover: both sources, parent-chain rule, version selectors, misconfigured detection, ignoreDependencyOverrides suppression with source scoping, severity-off short-circuit, transitive-CVE hint. Refs #336
Two CONCERN-class fixes flagged by the pre-ship review of #362. 1. Hint inversion: the unused-override `hint` field was emitted only for parent-chain shapes (`react>react-dom`), the inverse of the panel's intent. The panel's verdict #3 was "bare-target unused override stays at warn, ship a hint field" precisely to cover the bare-target transitive-CVE pattern (`debug: 'npm:obug@^1.0.2'`, react-is canary aliases, etc.). Phase 6 smoke on benchmark fixtures surfaced four real-world examples on vite + next.js, all flagged without the hint. Fix: emit the hint on every unused-override finding (both bare-target and parent-chain). The constant renames from HINT_PARENT_CHAIN_TRANSITIVE to HINT_MAY_BE_TRANSITIVE, the integration test renames from parent_chain_override_carries_transitive_hint to unused_overrides_carry_transitive_hint_on_every_shape and now exercises BOTH shapes in the same tempdir. 2. Cross-surface source naming inconsistency: JSON output emitted `source: "pnpm_workspace_yaml" / "pnpm_package_json"` (snake_case enum names) while `ignoreDependencyOverrides[].source` expected `"pnpm-workspace.yaml" / "package.json"` (filename labels). An agent reading a finding's source and copying it into a suppression rule would silently not match. Fix: serialize DependencyOverrideSource as filename labels via explicit `#[serde(rename = "...")]` per variant. Both surfaces now agree. Refs #336
Three load-bearing wiring gaps caught by codex's parallel /fallow-review of #362 plus one stale-doc concern. BLOCK 1: misconfigured-dependency-overrides did not fail CI despite the default error severity. has_error_severity_issues and promote_warns_to_errors in crates/cli/src/check/rules.rs both skipped the two new arrays, so a malformed pnpm override produced exit 0 even when the rule's default severity was error. Both functions now check the new arrays alongside unused_catalog_entries. BLOCK 2: scoped analysis + audit did not handle the new path-anchored issue types. crates/core/src/changed_files.rs::filter_results_by_changed_files filtered through unresolved_catalog_references but not the two override arrays, so dead-code --changed-since would still return unrelated override findings. Audit (crates/cli/src/audit.rs) was missing the new arrays in dead_code_keys, retain_introduced_dead_code, and annotate_dead_code_json, so audit --gate new-only would mis-attribute every override finding as 'not introduced' and the introduced flag would never land in the JSON. All five sites now mirror the unresolved_catalog_references pattern. BLOCK 3: JSON machine output was internally inconsistent. The summary block in crates/cli/src/report/json.rs omitted both new arrays, so total_issues counted findings the summary block did not surface; the action_spec_for_kind switch had no entry for either array, so finding JSON had no actions array. Added summary keys + two new ActionSpecs (remove-dependency-override fix-type for unused entries, fix-dependency-override for misconfigured entries) plus an AddToConfigIgnoreDependencyOverrides SuppressKind that emits a paste- ready { package, source } pair for ignoreDependencyOverrides. CONCERN 4: stale hint docs in unused_overrides.rs and results.rs still claimed the hint was emitted only on parent-chain shapes. Updated both doc comments to match the post-fix behavior (hint emitted on every unused finding). Snapshot tests regenerated to absorb the two new summary keys. Refs #336
…Code, Action, CI (Phase B)
CLI plumbing for the two new detectors: --unused-dependency-overrides /
--misconfigured-dependency-overrides flags, IssueFilters / apply_rules /
filter-to-workspaces / programmatic / baseline / explain registry with
cross-references between the override pair.
All six report formats render the findings (human two-tier with raw_key
headline + forces-line / reason-line, JSON with discriminated
remove-dependency-override / fix-dependency-override primary actions +
ignoreDependencyOverrides add-to-config suppress, SARIF rules
fallow/unused-dependency-override / fallow/misconfigured-dependency-override,
compact, markdown, codeclimate).
LSP emits WARNING for unused-dependency-override and ERROR for
misconfigured-dependency-override, anchored at the source file (root-relative
path joined with the analyzer root, mirroring the unused-catalog-entry fix).
DIAGNOSTIC_ISSUE_TYPES + AnalysisCompleteParams + merge_results + dedup_results
gain matching entries. Two new LSP regression tests cover the path-anchor
behavior + the severity mapping.
MCP accepts issue_types: ["unused-dependency-overrides",
"misconfigured-dependency-overrides"] and ISSUE_TYPE_FLAGS gains the two CLI
flags. NAPI bindings expose both. The analyze tool description is updated
to reference both new detectors.
VS Code: types, treeView icons + category branch, config default,
analysis-utils total count, diagnosticFilter entries, statusBar count + label,
commands filter mapping, package.json configuration default + property entries.
output-contract.d.ts (vscode + npm) regenerated. dist bundle rebuilt.
GitHub Action + GitLab CI: filter-changed.jq total_issues + retain-by-path
rule, summary-check.jq table row + section block, summary-combined.jq count row,
annotations-check.jq emits ::warning for unused with hint and ::error for
misconfigured with reason, summary-audit.jq audit rows. Fixtures + run.sh
inline JSON updated; both action (196) and ci (203) test suites green.
CLI snapshot tests cover both new types via sample_results additions.
schema.json regenerated. docs/output-schema.json gains UnusedDependencyOverride
and MisconfiguredDependencyOverride definitions, FixAction enum entries
(remove-dependency-override, fix-dependency-override), and broadened
AddToConfigAction.value to accept the { package, source } object shape.
Refs #336
…-action)
Two non-blocking concerns from the parallel team review, addressed in one
follow-up.
json-output-reviewer (CONCERN):
1. `ignoreDependencyOverrides` add-to-config action lacked `value_schema`
while the sibling `ignoreCatalogReferences` action has it. Added
IGNORE_DEPENDENCY_OVERRIDES_VALUE_SCHEMA constant and threaded it through
so CI agents and IDE integrations can validate the suppression snippet
shape before writing to fallow config.
2. Misconfigured findings with `unparsable-key` reason carry an empty
`raw_key` and no `target_package`. The previous fallback emitted
`{"package":"","source":"package.json"}`, which the config parser would
silently ignore. The suppress action is now skipped entirely when
neither `target_package` nor `raw_key` yields a non-empty name; the
primary `fix-dependency-override` action still fires.
github-action-reviewer (CONCERN):
3. action/jq/annotations-check.jq and action/jq/summary-check.jq (plus the
ci/ mirror) called `.reason | san` and `.raw_key` / `.raw_value` directly
on the misconfigured payload. If a future schema refactor were to drop
one of those required fields, jq would crash with exit 5 or render
literal `null`. Added `// ""` null-safe fallbacks throughout so the
action layer degrades gracefully.
Internal docs (README "What it finds", CHANGELOG `[Unreleased]`, and
.claude/rules/detection.md) updated to surface both new rules in the
dependencies family.
Snapshot tests regenerated to absorb the new value_schema field.
Action (196) and ci (203) test suites green.
Refs #336
BartWaardenburg
force-pushed
the
feat/issue-336-pnpm-overrides
branch
from
2e68813 to
0f51492
Compare
BartWaardenburg
added a commit
that referenced
this pull request
May 12, 2026
… rules + suppress value) Three load-bearing wiring gaps caught by codex's parallel /fallow-review of Phase B (PR #362), missed by claude's local team review. BLOCK 1: --changed-since filtered out the new override findings. UnusedDependencyOverride.path / MisconfiguredDependencyOverride.path stored project-root-relative strings ("pnpm-workspace.yaml", "package.json"), but try_get_changed_files returns absolute paths, so the changed_files.contains check in filter_results_by_changed_files never matched. Storage flipped to absolute internally (config.root.join(...) at detection time), matching the UnresolvedCatalogReference convention shipped in #334. serde_path::serialize keeps the JSON output relative. BLOCK 2: per-file overrides.rules did not apply because crates/cli/src/check/ rules.rs::apply_rules' has_overrides branch handled file-scoped issue types but not the new override findings; setting both rules off for pnpm-workspace.yaml / package.json via config overrides still surfaced findings. Added per-file resolve in both apply_rules and has_error_severity_issues for unused_dependency_overrides and misconfigured_dependency_overrides, matching the unresolved-catalog-references fix from 176698f. BLOCK 3: JSON add-to-config suppress action could emit an unsuppressable value for misconfigured findings with the EmptyValue reason. raw_key "react@<18" was used as fallback when target_package was absent, but the suppression matcher keys on the parsed package name "react"; the suggested { package: "react@<18" } did not actually suppress the finding. Added target_package: Option<String> to MisconfiguredDependencyOverride, populated from the parsed key when it parses (EmptyValue path; remains None for UnparsableKey). JSON now reads the parsed name first and skips the suppress action entirely when no parseable package name exists, mirroring the empty- raw_key handling shipped earlier. LSP follow-up: dropped root.join in push_dependency_override_diagnostics now that finding.path is absolute; regression tests updated to construct absolute paths and assert under them. Snapshot sample data also flipped to absolute. Codex repros against the rebuilt binary now produce: - BLOCK 1: editing pnpm-workspace.yaml under --changed-since HEAD~1 retains the override findings (was 0 before, now 3) - BLOCK 2: per-file overrides.rules.{unused,misconfigured}-dependency- overrides: "off" clears the findings (was 2+2, now 0) - BLOCK 3: misconfigured react@<18 EmptyValue suggests {package: "react"} (was "react@<18"), unparsable-key emits no add-to-config action Refs #336
BartWaardenburg
changed the title
… rules + suppress value) Three load-bearing wiring gaps caught by codex's parallel /fallow-review of Phase B (PR #362), missed by claude's local team review. BLOCK 1: --changed-since filtered out the new override findings. UnusedDependencyOverride.path / MisconfiguredDependencyOverride.path stored project-root-relative strings ("pnpm-workspace.yaml", "package.json"), but try_get_changed_files returns absolute paths, so the changed_files.contains check in filter_results_by_changed_files never matched. Storage flipped to absolute internally (config.root.join(...) at detection time), matching the UnresolvedCatalogReference convention shipped in #334. serde_path::serialize keeps the JSON output relative. BLOCK 2: per-file overrides.rules did not apply because crates/cli/src/check/ rules.rs::apply_rules' has_overrides branch handled file-scoped issue types but not the new override findings; setting both rules off for pnpm-workspace.yaml / package.json via config overrides still surfaced findings. Added per-file resolve in both apply_rules and has_error_severity_issues for unused_dependency_overrides and misconfigured_dependency_overrides, matching the unresolved-catalog-references fix from 176698f. BLOCK 3: JSON add-to-config suppress action could emit an unsuppressable value for misconfigured findings with the EmptyValue reason. raw_key "react@<18" was used as fallback when target_package was absent, but the suppression matcher keys on the parsed package name "react"; the suggested { package: "react@<18" } did not actually suppress the finding. Added target_package: Option<String> to MisconfiguredDependencyOverride, populated from the parsed key when it parses (EmptyValue path; remains None for UnparsableKey). JSON now reads the parsed name first and skips the suppress action entirely when no parseable package name exists, mirroring the empty- raw_key handling shipped earlier. LSP follow-up: dropped root.join in push_dependency_override_diagnostics now that finding.path is absolute; regression tests updated to construct absolute paths and assert under them. Snapshot sample data also flipped to absolute. Codex repros against the rebuilt binary now produce: - BLOCK 1: editing pnpm-workspace.yaml under --changed-since HEAD~1 retains the override findings (was 0 before, now 3) - BLOCK 2: per-file overrides.rules.{unused,misconfigured}-dependency- overrides: "off" clears the findings (was 2+2, now 0) - BLOCK 3: misconfigured react@<18 EmptyValue suggests {package: "react"} (was "react@<18"), unparsable-key emits no add-to-config action Refs #336
BartWaardenburg
force-pushed
the
feat/issue-336-pnpm-overrides
branch
from
0f51492 to
d7100f8
Compare
Collaborator
Author
|
Released in v2.73.0. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two new issue types in one drop, mirroring the unused-catalog-entries + unresolved-catalog-references shape from #329/#334.
unused-dependency-override(defaultwarn): override target not declared in any workspacepackage.json. Conservative static algorithm with the panel-approved parent-chain rule (USED when EITHER parent OR target is declared, covering the CVE-fix pattern where the override forces a transitive version under a declared parent). Bare-target unused findings carry ahintflagging the could-be-transitive case.misconfigured-dependency-override(defaulterror): override key cannot be parsed (empty key, dangling separators) or value is missing. pnpm refuses to honor these. Valid pnpm idioms (-,$ref,npm:alias) are explicitly allowlisted.Scope: Phase A
This PR is "Phase A" mirroring how #340 (the unresolved-catalog-references fix) was originally structured as a multi-phase commit train.
Included:
crates/config/src/workspace/pnpm_overrides.rs) for bothpnpm-workspace.yamloverrides:AND rootpackage.jsonpnpm.overrides. Handles bare names, scoped packages, version selectors (@types/react@<18), parent matchers (react>react-dom, including scoped parents and parent version selectorsreact@1>zoo). Allowlists pnpm special values.UnusedDependencyOverride,MisconfiguredDependencyOverride,DependencyOverrideSource,DependencyOverrideMisconfigReason) onAnalysisResults.IssueKinddiscriminants 23, 24).RulesConfig+PartialRulesConfigfields,IgnoreDependencyOverrideRule+CompiledIgnoreDependencyOverrideRule).crates/core/src/analyze/unused_overrides.rs) withfind_unused_dependency_overrides+find_misconfigured_dependency_overrideswired intoanalyze/mod.rsorchestration.tests/fixtures/issue-336-unused-overrides/.[Unreleased]entry with upgrade note for theerror-severity default.Deferred to subsequent phases:
issue_types+ NAPI binding param + VS Code extension surface.summary-check,summary-combined,annotations-check,review-comments-check,filter-changed).schema.json+docs/output-schema.jsonupdates.fallow-docs,fallow-skills).Naming
Multi-surface knob naming drafted simultaneously per the implement skill:
UnusedDependencyOverride(23),MisconfiguredDependencyOverride(24)unused_dependency_overrides,misconfigured_dependency_overridesunused-dependency-override/unused-dependency-overrides(andmisconfigured-*parallel)unused-dependency-overrides,misconfigured-dependency-overrideswarn(unused),error(misconfigured)ignoreDependencyOverrides: [{ package, source? }]Name uses
dependency-override, notpnpm-override, so a future PR can extend the same detector to npmoverridesand yarnresolutionswithout renaming. Panel-validated.Verification
cargo test --workspace --all-targets: 37 suites greencargo clippy --workspace --all-targets -- -D warnings: cleancargo fmt --all -- --check: cleancargo doc --workspace --no-deps --document-private-items: cleantypos: cleanTest plan
@types/react@<18) resolve correctly-,$ref,npm:alias) NOT flaggedignoreDependencyOverridessuppresses with optional source scopeSeverity::Offshort-circuits both detectorsRefs #336