fix(engine.pm.commands): honor minimumReleaseAgeExclude in self-update#11664
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
📝 WalkthroughWalkthroughpnpm self-update now applies minimumReleaseAge and minimumReleaseAgeExclude when resolving the target version: resolver options compute age-derived publishedBy, exclusion policies, and strict/ignore controls; tests and docs updated accordingly. ChangesSelf-update minimumReleaseAge support
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
There was a problem hiding this comment.
Pull request overview
This PR updates pnpm self-update so pnpm version resolution honors minimumReleaseAge and minimumReleaseAgeExclude, aligning self-update behavior with dependency resolution safeguards.
Changes:
- Passes release-age policy options into the self-update resolver.
- Adds version-policy dependency wiring for
engine.pm.commands. - Adds tests for age filtering and exclude patterns during implicit latest resolution.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
.changeset/fix-11655-self-update-minimum-release-age.md |
Documents the patch release behavior change. |
engine/pm/commands/src/self-updater/selfUpdate.ts |
Applies minimum release age policy when resolving target pnpm versions. |
engine/pm/commands/test/self-updater/selfUpdate.test.ts |
Adds tests for minimum release age and exclude handling. |
engine/pm/commands/package.json |
Adds @pnpm/config.version-policy dependency. |
engine/pm/commands/tsconfig.json |
Adds the version-policy project reference. |
pnpm-lock.yaml |
Records the new workspace dependency link. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
engine/pm/commands/test/self-updater/selfUpdate.test.ts (1)
72-77: ⚡ Quick winUse an options object for
createMetadataparameters.Line 72 expands this helper to 4 parameters, which breaks the local function-argument guideline and hurts readability at call sites.
♻️ Suggested refactor
function createMetadata ( latest: string, registry: string, - otherVersions: string[] = [], - time: Record<string, string> = {} + opts: { + otherVersions?: string[] + time?: Record<string, string> + } = {} ) { + const { otherVersions = [], time = {} } = opts const versions = [...otherVersions, latest] return { name: 'pnpm',// Example call-site updates outside this range: createMetadata('9.2.0', opts.registries.default, { otherVersions: ['9.1.0'] }) createMetadata('9.1.0', opts.registries.default, { otherVersions: ['9.0.0'], time: { /* ... */ }, })As per coding guidelines, "Functions should have no more than two or three arguments. If a function needs more parameters, use a single options object instead."
Also applies to: 95-95
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@engine/pm/commands/test/self-updater/selfUpdate.test.ts` around lines 72 - 77, Change createMetadata to accept a single options object instead of four positional args: function createMetadata({ latest, registry, otherVersions = [], time = {} }: { latest: string; registry: string; otherVersions?: string[]; time?: Record<string,string> }). Update every call site in this test (and any other uses) to pass an object like createMetadata({ latest: '9.2.0', registry: opts.registries.default, otherVersions: ['9.1.0'] }) and preserve the existing default values for otherVersions and time inside the function.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@engine/pm/commands/test/self-updater/selfUpdate.test.ts`:
- Around line 72-77: Change createMetadata to accept a single options object
instead of four positional args: function createMetadata({ latest, registry,
otherVersions = [], time = {} }: { latest: string; registry: string;
otherVersions?: string[]; time?: Record<string,string> }). Update every call
site in this test (and any other uses) to pass an object like createMetadata({
latest: '9.2.0', registry: opts.registries.default, otherVersions: ['9.1.0'] })
and preserve the existing default values for otherVersions and time inside the
function.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1d5c0d8f-0cc4-4e79-bd24-c12c4c87624d
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
.changeset/fix-11655-self-update-minimum-release-age.mdengine/pm/commands/package.jsonengine/pm/commands/src/self-updater/selfUpdate.tsengine/pm/commands/test/self-updater/selfUpdate.test.tsengine/pm/commands/tsconfig.json
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Agent
- GitHub Check: Run benchmark on ubuntu-latest
- GitHub Check: Compile & Lint
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use Standard Style with modifications: trailing commas are used, functions are preferred over classes, functions are declared after they are used (hoisting is relied upon).
Functions should have no more than two or three arguments. If a function needs more parameters, use a single options object instead.
Maintain import order: (1) Standard libraries, (2) External dependencies (sorted alphabetically), (3) Relative imports.
Files:
engine/pm/commands/src/self-updater/selfUpdate.tsengine/pm/commands/test/self-updater/selfUpdate.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use
instanceof Errorfor error type checking in Jest tests. Useutil.types.isNativeError()instead, as Jest runs tests in a VM context whereinstanceofchecks can fail across realms.
Files:
engine/pm/commands/test/self-updater/selfUpdate.test.ts
🧠 Learnings (2)
📚 Learning: 2026-05-05T23:03:04.286Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11479
File: __utils__/scripts/package.json:6-9
Timestamp: 2026-05-05T23:03:04.286Z
Learning: The pattern cross-env NODE_OPTIONS="$NODE_OPTIONS ..." in package.json scripts is an established convention in the pnpm/pnpm repository and is used across many packages (e.g., fs/hard-link-dir, worker, __utils__/scripts). Do not flag this as a cross-platform issue in individual files; if a change is needed, apply it as a repo-wide change in a separate PR. Scope this guidance to all package.json files in the repo; use the minimatch pattern '**/package.json' to identify relevant files and review changes at the repository level rather than per-file.
Applied to files:
engine/pm/commands/package.json
📚 Learning: 2026-05-14T09:04:00.133Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11622
File: resolving/npm-resolver/test/publishedBy.test.ts:350-354
Timestamp: 2026-05-14T09:04:00.133Z
Learning: In the pnpm/pnpm repository, ESLint is the authoritative style linter. Do not raise review findings for missing trailing commas in multiline function calls (e.g., `fs.writeFileSync(...)`) when this repo’s ESLint configuration does not report them and lint passes. Prefer deferring to the ESLint results for this specific trailing-comma rule rather than enforcing it manually in code review.
Applied to files:
engine/pm/commands/src/self-updater/selfUpdate.tsengine/pm/commands/test/self-updater/selfUpdate.test.ts
🔇 Additional comments (5)
engine/pm/commands/src/self-updater/selfUpdate.ts (1)
8-8: LGTM!Also applies to: 64-67, 83-95, 107-108, 319-326
engine/pm/commands/package.json (1)
39-39: LGTM!engine/pm/commands/tsconfig.json (1)
31-33: LGTM!engine/pm/commands/test/self-updater/selfUpdate.test.ts (1)
267-402: LGTM!.changeset/fix-11655-self-update-minimum-release-age.md (1)
1-8: LGTM!
Integrated-Benchmark Report (Linux)Scenario: Frozen Lockfile
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 2.43970781672,
"stddev": 0.0975692047225864,
"median": 2.43739245722,
"user": 2.66458024,
"system": 3.7051549600000007,
"min": 2.29994432172,
"max": 2.56672961872,
"times": [
2.36831815172,
2.40594555972,
2.55588699972,
2.44552749772,
2.42925741672,
2.55092145372,
2.56672961872,
2.46558431872,
2.30896282872,
2.29994432172
]
},
{
"command": "pacquet@main",
"mean": 2.3700433264200003,
"stddev": 0.06666765048208291,
"median": 2.35365177222,
"user": 2.66887044,
"system": 3.67647736,
"min": 2.29491806972,
"max": 2.5173590317200003,
"times": [
2.45402797672,
2.37694665772,
2.35608536472,
2.3518229607200003,
2.5173590317200003,
2.35548058372,
2.33351847172,
2.31796294172,
2.29491806972,
2.34231120572
]
},
{
"command": "pnpm",
"mean": 4.52984758032,
"stddev": 0.062019283386802404,
"median": 4.52682462872,
"user": 7.657588739999999,
"system": 4.03730476,
"min": 4.4281942997199994,
"max": 4.61243077472,
"times": [
4.51224904472,
4.58335789472,
4.4281942997199994,
4.501594486719999,
4.61107974972,
4.54564751572,
4.541400212719999,
4.61243077472,
4.51117541872,
4.45134640572
]
}
]
}Scenario: Frozen Lockfile (Hot Cache)
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 0.73117093734,
"stddev": 0.06539416470803319,
"median": 0.71013248744,
"user": 0.36735407999999997,
"system": 1.6280540399999999,
"min": 0.69230115894,
"max": 0.90730485094,
"times": [
0.90730485094,
0.69230115894,
0.72237280894,
0.72520151494,
0.69288672994,
0.76104099594,
0.71168251094,
0.69546592494,
0.69487041394,
0.70858246394
]
},
{
"command": "pacquet@main",
"mean": 0.75715879324,
"stddev": 0.04955506046200396,
"median": 0.7422811199399999,
"user": 0.36934607999999997,
"system": 1.65338084,
"min": 0.69452090694,
"max": 0.83984058694,
"times": [
0.83984058694,
0.74169454994,
0.82939446994,
0.74286768994,
0.72397052094,
0.70808270494,
0.76515309294,
0.73111489094,
0.79494851894,
0.69452090694
]
},
{
"command": "pnpm",
"mean": 2.41699257344,
"stddev": 0.08244039048242535,
"median": 2.42755374144,
"user": 2.91216338,
"system": 2.1841876399999998,
"min": 2.28267541794,
"max": 2.5533307089400004,
"times": [
2.47923887394,
2.47813572894,
2.3739423579400003,
2.44984995494,
2.39512192094,
2.30252328794,
2.28267541794,
2.4178598999400003,
2.43724758294,
2.5533307089400004
]
}
]
} |
5680df6 to
505c8e0
Compare
…tion Extract the publishedBy / publishedByExclude derivation duplicated across selfUpdate, dlx, outdated, and deps-resolver into a new `getPublishedByPolicy()` helper, and the version-policy error rewrap into `createPackageVersionPolicyOrThrow()`. Also adds the global self-update test branch (no wantedPackageManager) requested in PR review, and harmonizes the dlx/outdated error code for invalid minimumReleaseAgeExclude patterns with install/self-update.
pnpm#11664) * fix(engine.pm.commands): honor minimumReleaseAgeExclude in self-update * refactor(config.version-policy): centralize publishedBy policy derivation Extract the publishedBy / publishedByExclude derivation duplicated across selfUpdate, dlx, outdated, and deps-resolver into a new `getPublishedByPolicy()` helper, and the version-policy error rewrap into `createPackageVersionPolicyOrThrow()`. Also adds the global self-update test branch (no wantedPackageManager) requested in PR review, and harmonizes the dlx/outdated error code for invalid minimumReleaseAgeExclude patterns with install/self-update. * style(config.version-policy): rename 'callsite' to 'call site' to satisfy cspell --------- Co-authored-by: Zoltan Kochan <z@kochan.io>
close #11655
Summary by CodeRabbit
New Features
Tests
Documentation
Chores