chore(toolchain): node 24.19.0 and exempt @kno2 from the release-age guard - #147
dapperdandev wants to merge 2 commits into
Conversation
min-release-age=1 is a supply-chain guard for third-party packages, but it also applies to our own scoped packages. Without an exemption npm silently resolves an older @Kno2 version when a fresh one is inside the window, at exit 0, so a dependency bump looks successful while pinning a stale build. Scope the exemption to @kno2/* so third-party packages keep the guard. Matches what kno2fy-web already does. Needs npm 11.17.0, which this repo's .nvmrc (24.19.0) already provides. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
The repository pins Node 24.18.0, which does not support this npm configuration; update the pinned runtime and related configuration.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Updates npm configuration to exempt @kno2/* packages from the release-age guard.
Changes:
- Adds
min-release-age-exclude[]=@kno2/*to.npmrc.
File summaries
| File | Summary |
|---|---|
.npmrc |
Adds the scoped package exclusion; runtime support must be updated to ensure it is effective. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| @@ -1,2 +1,3 @@ | |||
| save-exact=true | |||
| min-release-age=1 | |||
| min-release-age-exclude[]=@kno2/* | |||
There was a problem hiding this comment.
Correct, and thanks - this was a real defect in the PR.
I had read .nvmrc as 24.19.0 from the feature-cda-explorer branch that happened to be checked out, not from master, which is 24.18.0 (npm 11.16.0). On that npm min-release-age-exclude does not exist, so the line I added would have been parsed and silently ignored, leaving exactly the stale-resolution behaviour the change is meant to prevent.
Fixed in d79b997: .nvmrc 24.18.0 -> 24.19.0. The PR workflow uses node-version-file: '.nvmrc', so CI moves with it.
Verified on 24.19.0 / npm 11.17.0 in this repo:
min-release-age = 1
min-release-age-exclude = ["@kno2/*"]
A fresh npm install under npm 11.17.0 is still a no-op, so no lockfile churn comes with the runtime bump.
Addresses Copilot review comment 4031256698. The exclude added in the previous commit needs npm 11.17.0, which first ships with node 24.19.0. master pinned 24.18.0 (npm 11.16.0), where the config does not exist and is silently ignored, so the stale-version behavior would have persisted. CI reads node-version-file: .nvmrc, so this moves CI too. Lockfile verified unchanged under npm 11.17.0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
| @@ -1 +1 @@ | |||
| 24.18.0 | |||
| 24.19.0 | |||
There was a problem hiding this comment.
Correct, the description had gone stale. Updated.
It described the first revision, where the change was .npmrc-only on the mistaken belief that .nvmrc was already 24.19.0 (I had read that value off the feature-cda-explorer branch rather than master). Your earlier comment is what prompted d79b997, and I did not re-sync the body afterwards.
The runtime change is intentional and is now stated as such: min-release-age-exclude needs npm >= 11.17.0, which first ships with node 24.19.0, so on the old 24.18.0 pin the added line would have been dead config. pull-request.yml:20 consuming .nvmrc means CI moves too, which is the point, since CI is where a silently stale @kno2 resolution would actually land.
Title and body now cover both files. A fresh npm install under npm 11.17.0 remains a no-op, so no lockfile churn rides along.
dapperdandev
changed the title
|
Folded into #146 rather than kept as a separate PR - two PRs against the same repo for changes this small was needless overhead, and the toolchain bump is what makes the dependency work correct in the first place. Both commits (b8bffb3, d79b997) were cherry-picked onto Re-verified on the combined branch: lockfile is still a fixed point on node 24.19.0 / npm 11.17.0, lint and build pass. |
Two changes that go together.
.npmrc: addsmin-release-age-exclude[]=@kno2/*.nvmrc: 24.18.0 -> 24.19.0Why
min-release-age=1is a supply-chain guard aimed at third-party packages, but it applies to our own scoped packages too. When a fresh@kno2version is inside the window, npm does not error. It silently resolves an older version at exit 0, so a dependency bump looks successful while pinning a stale build.Measured against
@kno2/sdk@1.184.3(published inside the window) on node 24.19.0 / npm 11.17.0:.npmrcmin-release-age=1min-release-age=1+min-release-age-exclude[]=@kno2/*This also replaces the blunter workaround of passing
--min-release-age=0, which disables the guard for every package in that install. The exclude exempts only matching names, and per npm's docs an exempted package's own dependencies still follow the policy.Why the runtime bump is part of this change
min-release-age-excluderequires npm >= 11.17.0, which first ships with node 24.19.0.masterpinned 24.18.0 (npm 11.16.0), where the config simply does not exist and is parsed and ignored.The first revision of this PR changed
.npmrcalone, on the mistaken belief that.nvmrcwas already 24.19.0 (that value was read from thefeature-cda-explorerbranch, notmaster). Copilot caught it. Without the runtime bump the added line would have been dead config and the stale-resolution behaviour would have persisted, so the two belong in one change..github/workflows/pull-request.yml:20usesnode-version-file: '.nvmrc', so this intentionally moves the CI runtime too - that is the point, since CI is where a silently stale@kno2resolution would land.Node 24 is the active LTS line, so this stays on LTS. kno2fy-web already runs this configuration.
Verification
npm config ls -lin this repo on 24.19.0 now reports:No dependency or lockfile changes. A fresh
npm installunder npm 11.17.0 is a no-op, so the runtime bump brings no lockfile churn. The diff is.npmrc+.nvmrconly.🤖 Generated with Claude Code