added `devEngines` support breaks updating dependencies

[susnux]

The recent devEngines support seems to cause errors with dependabot (all updates broken), this is an example but there are more packages like this:

Error running package manager command: corepack npm install [email protected] --force --dry-run false --ignore-scripts --package-lock-only Error: Invalid package manager specification in package.json (npm@^10); expected a semver version NPM : Invalid package manager specification in package.json (npm@^10); expected a semver version

Context: Within our project we have this in the package.json:

"engines": {
    "node": "^20.11.0 || ^22 || ^24"
  },
  "devEngines": {
    "packageManager": {
      "name": "npm",
      "version": "^10",
      "onFail": "error"
    },
    "runtime": {
      "name": "node",
      "version": "^22",
      "onFail": "error"
    }
  }

(We support a wide range of engines - but for development devs should only use Node 22 and NPM 10 to have consistent compiled assets (and test results)).

I guess this is caused here: https://github.com/nodejs/corepack/pull/643/files#r2234021913

---

I see two ways to fix corepack:

1. Only enforce strict version if a full version was passed, see patch:

diff --git a/sources/specUtils.ts b/sources/specUtils.ts
index edd5c7e..82c1435 100644
--- a/sources/specUtils.ts
+++ b/sources/specUtils.ts
@@ -77,47 +77,51 @@ function warnOrThrow(errorMessage: string, onFail?: DevEngineDependency[`onFail`
       console.warn(`! Corepack validation warning: ${errorMessage}`);
   }
 }
-function parsePackageJSON(packageJSONContent: CorepackPackageJSON) {
-  const {packageManager: pm} = packageJSONContent;
-  if (packageJSONContent.devEngines?.packageManager != null) {
-    const {packageManager} = packageJSONContent.devEngines;
-
-    if (typeof packageManager !== `object`) {
-      console.warn(`! Corepack only supports objects as valid value for devEngines.packageManager. The current value (${JSON.stringify(packageManager)}) will be ignored.`);
-      return pm;
+function parsePackageJSON({devEngines, packageManager}: CorepackPackageJSON) {
+  const spec = {
+    packageManager,
+    enforceExactVersion: true,
+  };
+
+  if (devEngines?.packageManager != null) {
+    const {packageManager: pm} = devEngines;
+
+    if (typeof pm !== `object`) {
+      console.warn(`! Corepack only supports objects as valid value for devEngines.packageManager. The current value (${JSON.stringify(pm)}) will be ignored.`);
+      return spec;
     }
-    if (Array.isArray(packageManager)) {
+    if (Array.isArray(pm)) {
       console.warn(`! Corepack does not currently support array values for devEngines.packageManager`);
-      return pm;
+      return spec;
     }
 
-    const {name, version, onFail} = packageManager;
+    const {name, version, onFail} = pm;
     if (typeof name !== `string` || name.includes(`@`)) {
       warnOrThrow(`The value of devEngines.packageManager.name ${JSON.stringify(name)} is not a supported string value`, onFail);
-      return pm;
+      return spec;
     }
     if (version != null && (typeof version !== `string` || !semverValidRange(version))) {
       warnOrThrow(`The value of devEngines.packageManager.version ${JSON.stringify(version)} is not a valid semver range`, onFail);
-      return pm;
+      return spec;
     }
 
     debugUtils.log(`devEngines.packageManager defines that ${name}@${version} is the local package manager`);
 
-    if (pm) {
-      if (!pm.startsWith?.(`${name}@`))
-        warnOrThrow(`"packageManager" field is set to ${JSON.stringify(pm)} which does not match the "devEngines.packageManager" field set to ${JSON.stringify(name)}`, onFail);
-
-      else if (version != null && !semverSatisfies(pm.slice(packageManager.name.length + 1), version))
-        warnOrThrow(`"packageManager" field is set to ${JSON.stringify(pm)} which does not match the value defined in "devEngines.packageManager" for ${JSON.stringify(name)} of ${JSON.stringify(version)}`, onFail);
-
-      return pm;
+    if (packageManager) {
+      if (!packageManager.startsWith?.(`${name}@`))
+        warnOrThrow(`"packageManager" field is set to ${JSON.stringify(packageManager)} which does not match the "devEngines.packageManager" field set to ${JSON.stringify(name)}`, onFail);
+      else if (version != null && !semverSatisfies(packageManager.slice(pm.name.length + 1), version))
+        warnOrThrow(`"packageManager" field is set to ${JSON.stringify(packageManager)} which does not match the value defined in "devEngines.packageManager" for ${JSON.stringify(name)} of ${JSON.stringify(version)}`, onFail);
+      return spec;
     }
 
-
-    return `${name}@${version ?? `*`}`;
+    return {
+      enforceExactVersion: semverValid(version),
+      packageManager: `${name}@${version ?? `*`}`,
+    };
   }
 
-  return pm;
+  return spec;
 }
 
 export async function setLocalPackageManager(cwd: string, info: PreparedPackageManagerInfo) {
@@ -233,11 +237,11 @@ export async function loadSpec(initialCwd: string): Promise<LoadSpecResult> {
     process.env = selection.localEnv;
   }
 
-  const rawPmSpec = parsePackageJSON(selection.data);
-  if (typeof rawPmSpec === `undefined`)
+  const {enforceExactVersion, packageManager} = parsePackageJSON(selection.data);
+  if (typeof packageManager === `undefined`)
     return {type: `NoSpec`, target: selection.manifestPath};
 
-  debugUtils.log(`${selection.manifestPath} defines ${rawPmSpec} as local package manager`);
+  debugUtils.log(`${selection.manifestPath} defines ${packageManager} as local package manager`);
 
   return {
     type: `Found`,
@@ -249,6 +253,6 @@ export async function loadSpec(initialCwd: string): Promise<LoadSpecResult> {
       onFail: selection.data.devEngines.packageManager.onFail,
     },
     // Lazy-loading it so we do not throw errors on commands that do not need valid spec.
-    getSpec: () => parseSpec(rawPmSpec, path.relative(initialCwd, selection.manifestPath)),
+    getSpec: () => parseSpec(packageManager, path.relative(initialCwd, selection.manifestPath), {enforceExactVersion}),
   };
 }

2. Ignore devEngines if it is a version range, see patch:

diff --git a/sources/specUtils.ts b/sources/specUtils.ts
index edd5c7e..183a62e 100644
--- a/sources/specUtils.ts
+++ b/sources/specUtils.ts
@@ -113,8 +113,9 @@ function parsePackageJSON(packageJSONContent: CorepackPackageJSON) {
       return pm;
     }
 
-
-    return `${name}@${version ?? `*`}`;
+    if (semverValid(version)) {
+      return `${name}@${version ?? `*`}`;
+    }
   }
 
   return pm;

[aduh95]

To clarify, Corepack does support ranges in devEngines, but you must pin a specific version in packageManager field

[susnux]

@aduh95 yes I know - but I personally do not want to use Corepack. it is used by services like Dependabot or Renovate which are used by many projects.

So as soon as we use devEngines with a valid value (semantic version range) those services break because Corepack throws an exception.

From my point of view Corepack should handle this more gracefully and just ignore it. Like it is already doing if you provide an array for the packageManager key of devEngines (which is our current workaround for this regressions).

[aduh95]

I get that, I wonder why those services do not define COREPACK_ENABLE_STRICT=0 in their env as I agree it doesn't make much sense for them to fail in this case. Regarding whether Corepack by default should not complain, I don't know, not pinning the version in dev is going against best practices, it sorta makes sense to nudge the users towards following best practices.

[skjnldsv]

Hey @aduh95 👋 Any way we can help this move forward in #730 ?

Bonne journée!