fix: fall back to older versions when pnpm trust downgrade blocks latest#14213
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request enhances Dependabot's npm_and_yarn ecosystem to handle pnpm trust downgrade errors more gracefully. When pnpm detects ERR_PNPM_TRUST_DOWNGRADE (indicating a provenance attestation disappeared from a newer version), instead of skipping the update entirely, Dependabot now iterates through older versions in descending order to find the highest version that passes pnpm's trust check.
Changes:
- Modified
latest_resolvable_versionmethod to call fallback logic when trust downgrade is detected instead of returning nil immediately - Added
find_version_without_trust_downgrademethod that iterates through candidate versions below the latest_allowable_version - Added
version_has_trust_downgrade?method that checks if a specific version triggers the trust downgrade error - Enhanced test coverage with three scenarios: all versions fail, one older version passes, multiple older versions fail
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| npm_and_yarn/lib/dependabot/npm_and_yarn/update_checker/version_resolver.rb | Implements fallback logic to try older versions when pnpm trust downgrade is detected, including helper methods to iterate candidates and check individual versions |
| npm_and_yarn/spec/dependabot/npm_and_yarn/update_checker/version_resolver_spec.rb | Adds comprehensive test coverage for trust downgrade scenarios with proper mocking of pnpm commands |
Comments suppressed due to low confidence (1)
npm_and_yarn/lib/dependabot/npm_and_yarn/update_checker/version_resolver.rb:303
- The method resets instance variables
@trust_downgrade_detectedand@peer_dependency_errorsbefore each check. This is necessary because these are set as side effects byfetch_peer_dependency_errors. However, this means the final state of these variables after the method completes may not accurately reflect the state for the version that was ultimately selected. If other code depends on these instance variables afterfind_version_without_trust_downgradeis called, it may see stale or incorrect values. Consider documenting this behavior or refactoring to avoid side-effect-based state management.
def version_has_trust_downgrade?(version)
@trust_downgrade_detected = false
@peer_dependency_errors = nil
fetch_peer_dependency_errors(version: version)
npm_and_yarn/spec/dependabot/npm_and_yarn/update_checker/version_resolver_spec.rb
Show resolved
Hide resolved
npm_and_yarn/lib/dependabot/npm_and_yarn/update_checker/version_resolver.rb
Show resolved
Hide resolved
|
Can you go over the following AI suggestions? Issue 1: Performance Concern with Unbounded Version IterationLocation: Problem: The current implementation iterates through all candidate versions without a limit. Each iteration calls
Suggestion: Consider adding a maximum iteration limit (e.g., 5-10 versions) to cap the number of trust checks performed. If no trusted version is found within the limit, return Issue 2: Side Effect Reliance and Type SafetyLocation: Problem: The method relies on
Suggestion: Consider either:
Issue 3: Missing Test Coverage for Edge CasesLocation: Problem: The current tests cover the happy paths well, but some edge cases are not covered:
Suggestion: Add test cases covering:
Issue 4: User-Facing Documentation GapLocation: PR description and user documentation Problem: When this feature activates, users will receive PRs that update to a version that is not the latest. This could cause confusion because:
Suggestion: Consider:
Summary Table
|
When pnpm blocks a version due to ERR_PNPM_TRUST_DOWNGRADE (provenance attestation disappeared), Dependabot previously skipped the update entirely. Now it iterates through older versions in descending order and selects the highest one that passes pnpm's trust check.
022cf50 to
88f4dff
Compare
What are you trying to accomplish?
When pnpm blocks a version due to
ERR_PNPM_TRUST_DOWNGRADE(provenance attestation disappeared), Dependabot previously skipped the update entirely. Now it iterates through older versions in descending order and selects the highest one that passes pnpm's trust check.Problem:
chokidar@4.0.0.4.0.3, fails the pnpm trust policy, so pnpm blocks installation.4.0.1does comply with the trust policy and would be the expected fallback version.Anything you want to highlight for special attention from reviewers?
How will you know you've accomplished your goal?
After this changes Local dependabot cli run against the workflow ensures:
Checklist