Skip to content

Read npm min-release-age from .npmrc and apply as cooldown#15132

Open
yeikel wants to merge 3 commits into
dependabot:mainfrom
yeikel:npm-min-release-age
Open

Read npm min-release-age from .npmrc and apply as cooldown#15132
yeikel wants to merge 3 commits into
dependabot:mainfrom
yeikel:npm-min-release-age

Conversation

@yeikel
Copy link
Copy Markdown
Contributor

@yeikel yeikel commented May 25, 2026

Summary

min-release-age setting in .npmrc prevents installing any package version younger than N days. Dependabot was unaware of this setting, so it could try to open PRs for versions that would violate user's configuration

This pull request reads min-release-age from .npmrc and uses its value as a lower bound on every update_cooldown field. If no update_cooldown is configured in dependabot.yml, one is created from the npmrc value. If one is already configured, the npmrc value only raises individual fields that are below it

Fixes #14151

Merge semantics

Scenario Result
No update_cooldown in dependabot.yml default_days = .npmrc value
dependabot.yml default_days >= npmrc value all fields unchanged (debug log only)
dependabot.yml default_days < npmrc value default_days raised to npmrc floor; warn logged
semver_*_days field < npmrc value that field raised to npmrc floor; per-field warn logged
include/exclude patterns always dropped; npm applies min-release-age globally with no per-package filtering

The npmrc value acts as a floor: it can only raise cooldown values, never lower them.

Anything you want to highlight for special attention from reviewers?

.npmrc and dependabot.yml cooldown settings don't translate cleanly. This is, for the most part, because Dependabot support features that npm does not support yet

  • npm's min-release-age is a global, unconditional install-time constraint with no per-package filtering

  • dependabot.yml supports include/exclude patterns and per-semver thresholds that have no npm equivalent. When both are present, the choices are: honour dependabot.yml as-is and risk Dependabot proposing versions npm install immediately rejects; or use min-release-age as a floor and silently drop incompatible dependabot.yml features

This PR takes the second path on the grounds that a broken install is worse than a dropped preference, with a log warning to surface the trade-off. A third option such as failing the pipeline on conflict seems more damaging.

How will you know you've accomplished your goal?

  • Both existing an new tests pass
  • Tested via the Dependabot CLI

Reproducer: https://github.com/yeikel/dependabot-issue-14151

Without this change, dependabot happily suggests the latest version of my dependency at test(3.5.34) ignoring my min-release-age configuration :

image

However, after this change, it suggests the version the configuration expects(3.4.27):

https://github.com/yeikel/dependabot-issue-14151/blob/bfc9181bbe5b21e8c9f2a8b0853506164946cdab/.npmrc#L1

image

Checklist

  • I have run the complete test suite to ensure all tests and linters pass.
  • I have thoroughly tested my code changes to ensure they work as expected, including adding additional tests for new functionality.
  • I have written clear and descriptive commit messages.
  • I have provided a detailed description of the changes in the pull request, including the problem it addresses, how it fixes the problem, and any relevant details about the implementation.
  • I have ensured that the code is well-documented and easy to understand.

@yeikel yeikel force-pushed the npm-min-release-age branch 5 times, most recently from e5daee2 to b07b1cf Compare May 25, 2026 20:24
@yeikel yeikel marked this pull request as ready for review May 25, 2026 20:41
@yeikel yeikel requested a review from a team as a code owner May 25, 2026 20:41
@robaiken robaiken self-assigned this May 26, 2026
@robaiken
Copy link
Copy Markdown
Contributor

@yeikel This is great, thank you. I need to check with the team to make sure this is the direction we want to take, and I'll get back to you.

@robaiken
Copy link
Copy Markdown
Contributor

@yeikel Thanks for driving these changes, and we agree with your approach. The floor logic is exactly right: min-release-age raising lower configured values is the same as always taking the higher cooldown value, which is what we want. Quick confirmation on the team's position: security alerts and updates aren't eligible for cooldown (covered in your pr: #15139), and for version updates we always take the higher value. Your PR already does all of this, so no changes needed there.

One question: does this also work for pnpm and yarn, or is it npm-only right now? They have similar minimum-release-age settings. No pressure either way, if you'd like to extend the PR to cover them that's great, otherwise we're happy to merge this as the npm piece and pick up pnpm and yarn as a follow-up. Your call, just let me know.

@yeikel
Copy link
Copy Markdown
Contributor Author

yeikel commented May 27, 2026

@yeikel Thanks for driving these changes, and we agree with your approach. The floor logic is exactly right: min-release-age raising lower configured values is the same as always taking the higher cooldown value, which is what we want. Quick confirmation on the team's position: security alerts and updates aren't eligible for cooldown (covered in your pr: #15139), and for version updates we always take the higher value. Your PR already does all of this, so no changes needed there.

Awesome, I’m glad we’re aligned

Just for the record, differences such as inclusion/exclusion are features that are expected to land in npm in a future feature release, so we should be able to reach feature parity over time. npm/cli#8994

One question: does this also work for pnpm and yarn, or is it npm-only right now? They have similar minimum-release-age settings.

Yes, this is what I know so far but I still need to do some additional research for the TODO items below:

Ecosystem General Cooldown Support/Conversion Ignore configuration for security updates
Yarn #14134 #15137
NPM #14151 #15112
uv #13816 TODO
PNPM #13405 TODO

Maybe we can create a meta issue using the table above if it helps to track this easier.

No pressure either way, if you'd like to extend the PR to cover them that's great, otherwise we're happy to merge this as the npm piece and pick up pnpm and yarn as a follow-up. Your call, just let me know.

What do you think would work best?

The reason I started with npm first was mainly for two reasons:

  • To align on the overall design choices early on (and so far it seems we’re on the same page with the decisions I made!)
  • To keep the work split into smaller, atomic commits/PRs that are easier to review and, if needed, easier to revert. That’s the main reason I’d lean toward reviewing/sharing feedback and merging this and Pass --min-release-age=0 for npm security updates to bypass .npmrc #15139, and then handle the rest as separate pull requests. That way we can ship these two early and gain community feedback/issues if any while we work on the other ecosystems in parallel

As for the follow-up work, as you know I enjoy contributing to this project, so I’d be happy to pick up the remaining work. The hardest part of contributing to core is usually getting consistent reviews, which I completely understand given the limited capacity. If you’re able to help review, I’d be happy to handle the implementation and get your help to move them forward

Likewise, if you would prefer to develop some of it yourself, I’d also be happy to help test things out. Let me know what you think would work best 🙏

@robaiken
Copy link
Copy Markdown
Contributor

@yeikel This all makes sense to me, and I think your incremental approach is the right one. This all makes sense to me, and I think your incremental approach is the right one. Shipping this and #15139 first, then handling the other ecosystems as separate PRs, fits well with how we like to keep changes atomic and reviewable, and getting them out early for community feedback is a nice bonus.

A meta issue tracking the table above would be really helpful, please go ahead and create one. That'll make it much easier to see the state of each ecosystem at a glance. Thanks also for the npm/cli#8994 pointer, good to know inclusion/exclusion parity should come over time.

On splitting the work: I really appreciate all the work you put into driving Dependabot forward. I'd never want to assign work to you as a contributor or take your time for granted, and it's honestly not my place to hand out tasks. So I'm more than happy for you to drive the implementation yourself if that's what you'd like, but I'd never expect it of you. Whatever you pick up is genuinely appreciated. What I can do is make sure you get consistent reviews from our side to keep things moving, since that's usually the real bottleneck. We're putting more effort into reviewing community pull requests, it'll take time but we're making reviews more of a priority.

So my suggestion: you keep implementing whatever you're happy to take on, and I'll review and help shepherd them through. If there's a part you'd rather not work on, just tag me in it and I'll assign the ticket to myself. Also tag me in any prs related and I will make sure the pr is reviews quickly as this is an issue affected a lot of users. I'll leave that decision to you.

- `default_days` now uses `[existing.default_days, npmrc_days].max` so
  the npmrc value never lowers a higher value set in dependabot.yml
- Conflict warning is conditional: only fires when at least one cooldown
  field is actually below the npmrc threshold; otherwise logs at debug
- Removed `default_days` from the per-field override-warning loop;
  the top-level conflict message covers it; per-field messages are for
  the three `semver_*_days` fields only
- `.npmrc` filename match tightened to `File.basename(f.name) == ".npmrc"`
  to avoid false matches like `foo.npmrc`
- Extracted `log_npmrc_cooldown_conflicts` and `merge_cooldown_with_npmrc_floor`
  helpers (satisfies Rubocop Metrics cops)
- TODO comment added for Yarn 4.10+ `npmMinimalAgeGate` follow-up

Spec changes:
- "when explicit update_cooldown already exceeds the npmrc floor": asserts
  default_days stays unchanged and no warning is logged
- "when dependabot.yml default_days is below the npmrc floor": expects
  conflict warning once and per-field warnings for semver fields only
- Replace `instance_variable_get(:@update_cooldown)` with `checker.update_cooldown`
@yeikel
Copy link
Copy Markdown
Contributor Author

yeikel commented May 27, 2026

@yeikel This all makes sense to me, and I think your incremental approach is the right one. This all makes sense to me, and I think your incremental approach is the right one. Shipping this and #15139 first, then handling the other ecosystems as separate PRs, fits well with how we like to keep changes atomic and reviewable, and getting them out early for community feedback is a nice bonus.

Sounds good to me. Please let me know if you have any feedback/what is pending to ship these two for now

A meta issue tracking the table above would be really helpful, please go ahead and create one. That'll make it much easier to see the state of each ecosystem at a glance. Thanks also for the npm/cli#8994 pointer, good to know inclusion/exclusion parity should come over time.

I created #15157

On splitting the work: I really appreciate all the work you put into driving Dependabot forward. I'd never want to assign work to you as a contributor or take your time for granted, and it's honestly not my place to hand out tasks. So I'm more than happy for you to drive the implementation yourself if that's what you'd like, but I'd never expect it of you. Whatever you pick up is genuinely appreciated. What I can do is make sure you get consistent reviews from our side to keep things moving, since that's usually the real bottleneck. We're putting more effort into reviewing community pull requests, it'll take time but we're making reviews more of a priority.

That's fair and understandable; I know that it is not expected but I like helping out as I also benefit

So my suggestion: you keep implementing whatever you're happy to take on, and I'll review and help shepherd them through. If there's a part you'd rather not work on, just tag me in it and I'll assign the ticket to myself. Also tag me in any prs related and I will make sure the pr is reviews quickly as this is an issue affected a lot of users. I'll leave that decision to you.

Sounds good. As soon as this and the other are merged, I'll pick up the next. Based on refinement, that seems to be yarn but I'd be happy to pivot based on what makes more sense. I ignore how many users we have on each ecosystem, but I do know that we got a direct report from users for yarn specifically (but not pnpm yet)

@robaiken
Copy link
Copy Markdown
Contributor

robaiken commented May 27, 2026

@yeikel I think we should prioritise PNPM is affecting the most users at the moment, I am going to start reviewing these prs and get them deployed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Convert npm min-release-age to equivalent Cooldown options

2 participants