fix(config/reader): don't warn when packageManager and devEngines.packageManager match (#12287)

When `package.json` sets both `packageManager` and `devEngines.packageManager` to the same pnpm version with the same integrity hash pnpm prints a spurious warning on every command. For example, a `package.json` file that looks like: ```json { "packageManager": "pnpm@11.5.1+sha512.93f7b57422ea7068257235b4c16eb60762eb68e1dc23723199cc739043ea9be2c4143274a399d8c6defa2b1176226d9ca1c4b63482d6200c1a8fbaa78c1d1485", "devEngines": { "packageManager": { "name": "pnpm", "version": "11.5.1+sha512.93f7b57422ea7068257235b4c16eb60762eb68e1dc23723199cc739043ea9be2c4143274a399d8c6defa2b1176226d9ca1c4b63482d6200c1a8fbaa78c1d1485", "onFail": "ignore" }, "runtime": [ { "name": "node", "version": "26.3.0", "onFail": "ignore" } ] } } ``` Issues a warning on every `pnpm` command: > Cannot use both "packageManager" and "devEngines.packageManager" in package.json. "packageManager" will be ignored. ## Root cause `getWantedPackageManager` compares the two fields to decide whether to warn, but the two sides were normalized differently: - `parsePackageManager` **strips the integrity hash** from the legacy `packageManager` field → `11.5.1` - the `devEngines.packageManager` version was compared **with its hash intact** → `11.5.1+sha512.93f7b57…` So, `"11.5.1" !== "11.5.1+sha512…"` was always true and the warning fired, even for identical specs. An earlier fix in #11307 only suppressed the warning when *neither* side carried a hash. ## Fix `parsePackageManager` now also returns the hash (via a shared `splitPackageManagerVersion`), and `getPackageManagerConflictWarning` compares the fields structurally. The warning is suppressed **only when the two specifiers are identical** (name + version + hash, both-absent counts as equal): | name | version | hash | result | |------|---------|------|--------| | same | same | both absent, or both present & equal | ✅ no warning | | same | same | present on **one side only** | ⚠️ generic "Cannot use both…" | | same | same | both present & **differ** | ⚠️ "…contradictory integrity hashes" | | same | **differ** | — | ⚠️ "…different versions of pnpm" | | **differ** | — | — | ⚠️ "…different package managers" | A hash on only one side is still a divergence — dropping the ignored `packageManager` field would lose that hash — so it warns with the original generic message. Two contradictory hashes for one version (a likely wrong-hash mistake) get a dedicated message. The generic single message is otherwise replaced by one tailored to each conflict, each ending with `"packageManager" will be ignored`. Closes #12028. --------- Signed-off-by: C. Spencer Beggs <spencer@beggs.codes> Signed-off-by: Zoltan Kochan <z@kochan.io> Co-authored-by: Zoltan Kochan <z@kochan.io>
2026-06-28 09:55:39 -04:00 · 2026-06-16 09:49:00 -04:00
parent 817f99dbe5
commit 302a2f7d2c
4 changed files with 261 additions and 15 deletions
--- a/.changeset/devengines-package-manager-same-hash.md
+++ b/.changeset/devengines-package-manager-same-hash.md
@@ -0,0 +1,8 @@
+---
+"@pnpm/config.reader": patch
+"pnpm": patch
+---
+
+No longer warn about using both `packageManager` and `devEngines.packageManager` when the two fields pin the same package manager at the same version with the same integrity hash (e.g. both `pnpm@11.5.1+sha512.…`). Previously the hash was stripped from the legacy `packageManager` field but not from `devEngines.packageManager`, so even identical specifications looked like a mismatch [#12028](https://github.com/pnpm/pnpm/issues/12028).
+
+The warning still fires on any genuine divergence, and several cases now state the specific reason instead of a single generic message: a different package manager, a different version, or contradictory integrity hashes for the same version.
--- a/config/reader/src/index.ts
+++ b/config/reader/src/index.ts
@@ -1,6 +1,7 @@
 import fs from 'node:fs'
 import os from 'node:os'
 import path from 'node:path'
+import { stripVTControlCharacters } from 'node:util'

 import { getCatalogsFromWorkspaceManifest } from '@pnpm/catalogs.config'
 import { createMatcher } from '@pnpm/config.matcher'
@@ -793,8 +794,12 @@ function getWantedPackageManager (manifest: ProjectManifest): { pm?: WantedPacka
    }
    if (manifest.packageManager) {
      const legacyPm = parsePackageManager(manifest.packageManager)
-      if (legacyPm.name !== pmFromDevEngines.name || legacyPm.version !== pmFromDevEngines.version) {
-        warnings.push('Cannot use both "packageManager" and "devEngines.packageManager" in package.json. "packageManager" will be ignored')
+      const conflictWarning = getPackageManagerConflictWarning(legacyPm, {
+        name: pmFromDevEngines.name,
+        ...splitPackageManagerVersion(pmFromDevEngines.version),
+      })
+      if (conflictWarning) {
+        warnings.push(conflictWarning)
      }
    }
    return { pm: { ...pmFromDevEngines, fromDevEngines: true }, warnings }
@@ -848,17 +853,90 @@ function getIgnoredPnpmFieldKeys (manifest: ProjectManifest): string[] {
  return Object.keys(legacyField as Record<string, unknown>).filter(k => MIGRATED_PNPM_FIELD_KEYS.has(k))
 }

-export function parsePackageManager (packageManager: string): { name: string, version: string | undefined } {
-  if (!packageManager.includes('@')) return { name: packageManager, version: undefined }
-  const [name, pmReference] = packageManager.split('@')
+export interface ParsedPackageManager {
+  name: string
+  version: string | undefined
+  hash: string | undefined
+}
+
+export function parsePackageManager (packageManager: string): ParsedPackageManager {
+  // Split on the `@` that separates the name from the reference. A leading `@`
+  // belongs to a scoped name (e.g. `@scope/pm@1.2.3`), so skip it; otherwise
+  // the first `@` is the separator. The first `@` (not the last) is used so a
+  // reference that is a URL containing `@` (e.g. credentials) stays intact.
+  const separatorIndex = packageManager.startsWith('@')
+    ? packageManager.indexOf('@', 1)
+    : packageManager.indexOf('@')
+  if (separatorIndex === -1) return { name: packageManager, version: undefined, hash: undefined }
+  const name = packageManager.slice(0, separatorIndex)
+  const pmReference = packageManager.slice(separatorIndex + 1)
  // pmReference is semantic versioning, not URL
-  if (pmReference.includes(':')) return { name, version: undefined }
-  // Remove the integrity hash. Ex: "pnpm@9.5.0+sha512.140036830124618d624a2187b50d04289d5a087f326c9edfc0ccd733d76c4f52c3a313d4fc148794a2a9d81553016004e6742e8cf850670268a7387fc220c903"
-  const [version] = pmReference.split('+')
-  return {
-    name,
-    version,
+  if (pmReference.includes(':')) return { name, version: undefined, hash: undefined }
+  return { name, ...splitPackageManagerVersion(pmReference) }
+}
+
+/**
+ * Splits a package manager version reference into its semver part and the
+ * integrity hash carried as semver build metadata, e.g.
+ * "9.5.0+sha512.140036830124618d624a2187b50d04289d5a087f326c9edfc0ccd733d76c4f52c3a313d4fc148794a2a9d81553016004e6742e8cf850670268a7387fc220c903"
+ * becomes `{ version: "9.5.0", hash: "sha512.14003…" }`. A reference without a
+ * hash yields an undefined hash; an undefined reference yields both undefined.
+ */
+function splitPackageManagerVersion (reference: string | undefined): { version: string | undefined, hash: string | undefined } {
+  if (reference == null) return { version: undefined, hash: undefined }
+  // Split on the first `+` only. The integrity hash is semver build metadata —
+  // everything after that `+` — and must be preserved whole, so a reference is
+  // never truncated at a later `+`.
+  const hashIndex = reference.indexOf('+')
+  if (hashIndex === -1) return { version: reference, hash: undefined }
+  return { version: reference.slice(0, hashIndex), hash: reference.slice(hashIndex + 1) }
+}
+
+/**
+ * Describes how the legacy `packageManager` field disagrees with
+ * `devEngines.packageManager`, or returns undefined when the two specifiers are
+ * identical (so keeping both fields in sync produces no warning). Any
+ * divergence warns — including an integrity hash (semver build metadata) on
+ * only one side, since dropping the ignored `packageManager` field would lose
+ * it. In every conflict `devEngines.packageManager` wins and `packageManager`
+ * is ignored.
+ */
+function getPackageManagerConflictWarning (legacy: ParsedPackageManager, devEngines: ParsedPackageManager): string | undefined {
+  const ignoredSuffix = '. "packageManager" will be ignored'
+  const genericWarning = `Cannot use both "packageManager" and "devEngines.packageManager" in package.json${ignoredSuffix}`
+  if (legacy.name !== devEngines.name) {
+    return `"packageManager" (${sanitizeManifestValue(legacy.name)}) and "devEngines.packageManager" (${sanitizeManifestValue(devEngines.name)}) specify different package managers in package.json${ignoredSuffix}`
  }
+  if (legacy.version !== devEngines.version) {
+    // "different versions" only makes sense when both sides are concrete
+    // versions. If one side has no semver version — e.g. the legacy field is a
+    // URL or a bare name — fall back to the generic notice rather than claiming
+    // a version mismatch.
+    if (legacy.version == null || devEngines.version == null) return genericWarning
+    return `"packageManager" and "devEngines.packageManager" specify different versions of ${sanitizeManifestValue(legacy.name)} in package.json${ignoredSuffix}`
+  }
+  if (legacy.hash !== devEngines.hash) {
+    // Same name and version, but the integrity hashes differ. Two distinct
+    // hashes for one version is a likely wrong-hash mistake, so call it out
+    // specifically; a hash on only one side is a softer mismatch (the version
+    // still agrees) and gets the generic notice.
+    if (legacy.hash != null && devEngines.hash != null && legacy.version != null) {
+      return `"packageManager" and "devEngines.packageManager" specify ${sanitizeManifestValue(legacy.name)}@${sanitizeManifestValue(legacy.version)} with different integrity hashes in package.json${ignoredSuffix}`
+    }
+    return genericWarning
+  }
+  return undefined
+}
+
+/**
+ * Renders a package.json-controlled value safe to embed in a warning printed to
+ * the terminal. Strips ANSI escape sequences and replaces remaining control
+ * characters (including newlines) with spaces so a malicious manifest cannot
+ * forge or rewrite terminal/CI log output.
+ */
+function sanitizeManifestValue (value: string): string {
+  // eslint-disable-next-line no-control-regex
+  return stripVTControlCharacters(value).replace(/[\u0000-\u001f\u007f]/g, ' ')
 }

 /**
--- a/config/reader/test/index.ts
+++ b/config/reader/test/index.ts
@@ -13,7 +13,7 @@ import { writeYamlFileSync } from 'write-yaml-file'

 jest.unstable_mockModule('@pnpm/network.git-utils', () => ({ getCurrentBranch: jest.fn() }))

-const { getConfig } = await import('@pnpm/config.reader')
+const { getConfig, parsePackageManager } = await import('@pnpm/config.reader')
 const { getCurrentBranch } = await import('@pnpm/network.git-utils')

 // To override any local settings,
@@ -207,6 +207,109 @@ test('devEngines.packageManager with explicit onFail is respected (regression gu
  expect(context.wantedPackageManager?.onFail).toBe('error')
 })

+describe('"packageManager" / "devEngines.packageManager" conflict warning', () => {
+  const HASH_A = 'sha512.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'
+  const HASH_B = 'sha512.bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'
+  const IGNORED = '. "packageManager" will be ignored'
+
+  async function warningsFor (packageManager: string, devEnginesVersion: string): Promise<string[]> {
+    prepare({
+      packageManager,
+      devEngines: {
+        packageManager: { name: 'pnpm', version: devEnginesVersion, onFail: 'ignore' },
+      },
+    })
+    const { warnings } = await getConfig({
+      cliOptions: {},
+      packageManager: { name: 'pnpm', version: '1.2.3' },
+    })
+    return warnings
+  }
+
+  test.each([
+    { caseName: 'same version without a hash', packageManager: 'pnpm@1.2.3', devEnginesVersion: '1.2.3' },
+    { caseName: 'same version with the same hash', packageManager: `pnpm@1.2.3+${HASH_A}`, devEnginesVersion: `1.2.3+${HASH_A}` },
+  ])('does not warn when the specifiers are identical: $caseName', async ({ packageManager, devEnginesVersion }) => {
+    const warnings = await warningsFor(packageManager, devEnginesVersion)
+    expect(warnings.some(w => w.includes(IGNORED))).toBe(false)
+  })
+
+  test.each([
+    { caseName: 'hash on the legacy field only', packageManager: `pnpm@1.2.3+${HASH_A}`, devEnginesVersion: '1.2.3' },
+    { caseName: 'hash on the devEngines field only', packageManager: 'pnpm@1.2.3', devEnginesVersion: `1.2.3+${HASH_A}` },
+  ])('warns generically when the integrity hash is present on only one field: $caseName', async ({ packageManager, devEnginesVersion }) => {
+    const warnings = await warningsFor(packageManager, devEnginesVersion)
+    expect(warnings).toContain(`Cannot use both "packageManager" and "devEngines.packageManager" in package.json${IGNORED}`)
+  })
+
+  test('warns about contradictory integrity hashes for the same version', async () => {
+    const warnings = await warningsFor(`pnpm@1.2.3+${HASH_A}`, `1.2.3+${HASH_B}`)
+    expect(warnings).toContain(`"packageManager" and "devEngines.packageManager" specify pnpm@1.2.3 with different integrity hashes in package.json${IGNORED}`)
+  })
+
+  test.each([
+    { caseName: 'the legacy field is a URL reference', packageManager: 'pnpm@https://github.com/pnpm/pnpm' },
+    { caseName: 'the legacy field is a bare name with no version', packageManager: 'pnpm' },
+  ])('warns generically rather than claiming a version mismatch when one side is not a concrete version: $caseName', async ({ packageManager }) => {
+    const warnings = await warningsFor(packageManager, '1.2.3')
+    expect(warnings).toContain(`Cannot use both "packageManager" and "devEngines.packageManager" in package.json${IGNORED}`)
+    expect(warnings.some(w => w.includes('different versions'))).toBe(false)
+  })
+
+  test.each([
+    { caseName: 'different exact versions', devEnginesVersion: '1.2.4' },
+    { caseName: 'exact version versus a range', devEnginesVersion: '>=1.0.0' },
+  ])('warns about a version mismatch: $caseName', async ({ devEnginesVersion }) => {
+    const warnings = await warningsFor('pnpm@1.2.3', devEnginesVersion)
+    expect(warnings).toContain(`"packageManager" and "devEngines.packageManager" specify different versions of pnpm in package.json${IGNORED}`)
+  })
+
+  test('warns when the fields name different package managers', async () => {
+    prepare({
+      packageManager: 'yarn@1.2.3',
+      devEngines: {
+        packageManager: { name: 'pnpm', version: '1.2.3', onFail: 'ignore' },
+      },
+    })
+    const { warnings } = await getConfig({
+      cliOptions: {},
+      packageManager: { name: 'pnpm', version: '1.2.3' },
+    })
+    expect(warnings).toContain(`"packageManager" (yarn) and "devEngines.packageManager" (pnpm) specify different package managers in package.json${IGNORED}`)
+  })
+
+  test('strips control characters from package.json values embedded in the warning', async () => {
+    prepare({
+      packageManager: 'ev\u001b[31mi\nl@1.2.3',
+      devEngines: {
+        packageManager: { name: 'pnpm', version: '1.2.3', onFail: 'ignore' },
+      },
+    })
+    const { warnings } = await getConfig({
+      cliOptions: {},
+      packageManager: { name: 'pnpm', version: '1.2.3' },
+    })
+    const warning = warnings.find(w => w.includes('different package managers'))
+    expect(warning).toBeDefined()
+    // eslint-disable-next-line no-control-regex
+    expect(warning).not.toMatch(/[\u0000-\u001f\u007f]/)
+    expect(warning).toContain('"packageManager" (evi l)')
+  })
+})
+
+describe('parsePackageManager', () => {
+  test.each([
+    { input: 'pnpm@9.5.0', expected: { name: 'pnpm', version: '9.5.0', hash: undefined } },
+    { input: 'pnpm@9.5.0+sha512.abc123', expected: { name: 'pnpm', version: '9.5.0', hash: 'sha512.abc123' } },
+    { input: 'pnpm@9.5.0+a+b', expected: { name: 'pnpm', version: '9.5.0', hash: 'a+b' } },
+    { input: 'pnpm', expected: { name: 'pnpm', version: undefined, hash: undefined } },
+    { input: '@scope/pm@1.2.3', expected: { name: '@scope/pm', version: '1.2.3', hash: undefined } },
+    { input: 'pnpm@https://github.com/pnpm/pnpm', expected: { name: 'pnpm', version: undefined, hash: undefined } },
+  ])('parses $input', ({ input, expected }) => {
+    expect(parsePackageManager(input)).toEqual(expected)
+  })
+})
+
 test('throw error if --link-workspace-packages is used with --global', async () => {
  await expect(getConfig({
    cliOptions: {
--- a/pnpm/test/packageManagerCheck.test.ts
+++ b/pnpm/test/packageManagerCheck.test.ts
@@ -506,7 +506,62 @@ test('no warning when packageManager and devEngines.packageManager specify the s

  const { stderr } = execPnpmSync(['install'])

-  expect(stderr.toString()).not.toContain('Cannot use both')
+  expect(stderr.toString()).not.toContain('"packageManager" will be ignored')
+})
+
+test('no warning when packageManager and devEngines.packageManager specify the same version with the same integrity hash', async () => {
+  const hash = 'sha512.93f7b57422ea7068257235b4c16eb60762eb68e1dc23723199cc739043ea9be2c4143274a399d8c6defa2b1176226d9ca1c4b63482d6200c1a8fbaa78c1d1485'
+  prepare({
+    packageManager: `pnpm@1.2.3+${hash}`,
+    devEngines: {
+      packageManager: {
+        name: 'pnpm',
+        version: `1.2.3+${hash}`,
+        onFail: 'ignore',
+      },
+    },
+  })
+
+  const { stderr } = execPnpmSync(['install'])
+
+  expect(stderr.toString()).not.toContain('"packageManager" will be ignored')
+})
+
+test('warns when packageManager and devEngines.packageManager pin the same version but different integrity hashes', async () => {
+  prepare({
+    packageManager: 'pnpm@1.2.3+sha512.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa',
+    devEngines: {
+      packageManager: {
+        name: 'pnpm',
+        version: '1.2.3+sha512.bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb',
+        onFail: 'ignore',
+      },
+    },
+  })
+
+  const { stderr } = execPnpmSync(['install'])
+
+  expect(stderr.toString()).toContain('different integrity hashes')
+  expect(stderr.toString()).toContain('"packageManager" will be ignored')
+})
+
+test('warns when the integrity hash is present on packageManager but not devEngines.packageManager', async () => {
+  const hash = 'sha512.93f7b57422ea7068257235b4c16eb60762eb68e1dc23723199cc739043ea9be2c4143274a399d8c6defa2b1176226d9ca1c4b63482d6200c1a8fbaa78c1d1485'
+  prepare({
+    packageManager: `pnpm@1.2.3+${hash}`,
+    devEngines: {
+      packageManager: {
+        name: 'pnpm',
+        version: '1.2.3',
+        onFail: 'ignore',
+      },
+    },
+  })
+
+  const { stderr } = execPnpmSync(['install'])
+
+  expect(stderr.toString()).toContain('Cannot use both "packageManager" and "devEngines.packageManager"')
+  expect(stderr.toString()).toContain('"packageManager" will be ignored')
 })

 test('warns when packageManager specifies a different package manager from devEngines.packageManager', async () => {
@@ -523,7 +578,8 @@ test('warns when packageManager specifies a different package manager from devEn

  const { stderr } = execPnpmSync(['install'])

-  expect(stderr.toString()).toContain('Cannot use both "packageManager" and "devEngines.packageManager"')
+  expect(stderr.toString()).toContain('specify different package managers')
+  expect(stderr.toString()).toContain('"packageManager" will be ignored')
 })

 test('warns when packageManager version does not match the devEngines.packageManager version string exactly', async () => {
@@ -540,7 +596,8 @@ test('warns when packageManager version does not match the devEngines.packageMan

  const { stderr } = execPnpmSync(['install'])

-  expect(stderr.toString()).toContain('Cannot use both "packageManager" and "devEngines.packageManager"')
+  expect(stderr.toString()).toContain('specify different versions of pnpm')
+  expect(stderr.toString()).toContain('"packageManager" will be ignored')
 })

 test('pmOnFail=ignore via env var bypasses the devEngines.packageManager check', async () => {