fix(windows): repair legacy PNPM_HOME registry type

os/env/path-extender-windows/path-extender-windows.spec.ts

@@ -306,7 +306,7 @@ test('PNPM_HOME is already set, but Path is updated', async () => {
     failed: false,
     stdout: `
 HKEY_CURRENT_USER\\Environment
-    PNPM_HOME    REG_EXPAND_SZ    ${pnpmHomeDirNormalized}
+    PNPM_HOME    REG_SZ    ${pnpmHomeDirNormalized}
     Path    REG_EXPAND_SZ    ${currentPathInRegistry}
 `,
   }).mockResolvedValueOnce({

os/env/path-extender-windows/path-extender-windows.ts

@@ -90,21 +90,26 @@ async function updateEnvVariable (
     overwrite: boolean
   }
 ): Promise<EnvVariableChange> {
-  const currentValue = await getEnvValueFromRegistry(registryOutput, name)
+  const current = await getEnvVariableFromRegistry(registryOutput, name)
+  const currentValue = current?.data
   if (currentValue && !opts.overwrite) {
     if (currentValue !== value) {
       throw new BadEnvVariableError({ envName: name, currentValue, wantedValue: value })
     }
-    return { variable: name, action: 'skipped', oldValue: currentValue, newValue: value }
-  } else {
-    await setEnvVarInRegistry(name, value, { expandableString: opts.expandableString })
-    return { variable: name, action: 'updated', oldValue: currentValue as string, newValue: value }
+    const wantedType = opts.expandableString ? 'REG_EXPAND_SZ' : 'REG_SZ'
+    // Older pnpm versions stored PNPM_HOME as REG_EXPAND_SZ. Repair its type
+    // even when the path is unchanged, without requiring an overwrite.
+    if (current?.type === wantedType) {
+      return { variable: name, action: 'skipped', oldValue: currentValue, newValue: value }
+    }
   }
+  await setEnvVarInRegistry(name, value, { expandableString: opts.expandableString })
+  return { variable: name, action: 'updated', oldValue: currentValue, newValue: value }
 }
 
 async function addToPath (registryOutput: string, addedDir: string, position: AddingPosition = 'start'): Promise<EnvVariableChange> {
   const variable = 'Path'
-  const pathData = await getEnvValueFromRegistry(registryOutput, variable)
+  const pathData = (await getEnvVariableFromRegistry(registryOutput, variable))?.data
   if (pathData === undefined || pathData == null || pathData.trim() === '') {
     throw new PnpmError('NO_PATH', '"Path" environment variable is not found in the registry')
   } else if (pathData.split(path.delimiter).includes(addedDir)) {
@@ -135,10 +140,10 @@ async function getRegistryOutput (): Promise<string> {
   }
 }
 
-async function getEnvValueFromRegistry (registryOutput: string, envVarName: string): Promise<string | undefined> {
+async function getEnvVariableFromRegistry (registryOutput: string, envVarName: string): Promise<IEnvironmentValueMatch['groups'] | undefined> {
   const regexp = new RegExp(`^ {4}(?<name>${envVarName}) {4}(?<type>\\w+) {4}(?<data>.*)$`, 'gim')
   const match = Array.from(matchAll(registryOutput, regexp))[0] as IEnvironmentValueMatch
-  return match?.groups.data
+  return match?.groups
 }
 
 async function setEnvVarInRegistry (

os/env/path-extender-windows/registry-type.spec.ts

@@ -0,0 +1,67 @@
+import execa from 'safe-execa'
+import { addDirToWindowsEnvPath } from './path-extender-windows'
+
+jest.mock('safe-execa')
+
+const regKey = 'HKEY_CURRENT_USER\\Environment'
+const home = 'C:\\Users\\Test User\\AppData\\Local\\pnpm'
+const userPath = '%PNPM_HOME%\\bin;%USERPROFILE%\\WindowsApps'
+const opts = { proxyVarName: 'PNPM_HOME', proxyVarSubDir: 'bin' }
+
+function mockRegistry (type: string, value = home) {
+  execa['mockReset']()
+  execa['mockResolvedValueOnce']({ failed: false, stdout: 'Active code page: 437' })
+    .mockResolvedValueOnce({ failed: false, stdout: '' })
+    .mockResolvedValueOnce({
+      failed: false,
+      stdout: `\r\n${regKey}\r\n    PNPM_HOME    ${type}    ${value}\r\n    Path    REG_EXPAND_SZ    ${userPath}\r\n`,
+    })
+    .mockResolvedValue({ failed: false, stdout: '' })
+}
+
+function registryWrites () {
+  return execa['mock'].calls.filter(([command, args]: [string, string[]]) => command === 'reg' && args[0] === 'add')
+}
+
+test('repairs legacy PNPM_HOME without duplicating Path, then skips a repeat setup', async () => {
+  mockRegistry('REG_EXPAND_SZ')
+  const report = await addDirToWindowsEnvPath(home, opts)
+  expect(report).toStrictEqual([
+    { variable: 'PNPM_HOME', action: 'updated', oldValue: home, newValue: home },
+    { variable: 'Path', action: 'skipped', oldValue: userPath, newValue: userPath },
+  ])
+  expect(registryWrites()).toStrictEqual([
+    ['reg', ['add', regKey, '/v', 'PNPM_HOME', '/t', 'REG_SZ', '/d', home, '/f'], { windowsHide: false }],
+  ])
+  expect(execa).toHaveBeenLastCalledWith('chcp', ['437'])
+
+  mockRegistry('REG_SZ')
+  const repeatReport = await addDirToWindowsEnvPath(home, opts)
+  expect(repeatReport.every(change => change.action === 'skipped')).toBe(true)
+  expect(registryWrites()).toStrictEqual([])
+})
+
+test.each(['REG_SZ', 'REG_EXPAND_SZ'])('preserves a different PNPM_HOME stored as %s', async (type) => {
+  mockRegistry(type, 'C:\\other')
+  await expect(addDirToWindowsEnvPath(home, opts)).rejects.toMatchObject({ code: 'ERR_PNPM_BAD_ENV_FOUND' })
+  expect(registryWrites()).toStrictEqual([])
+  expect(execa).toHaveBeenLastCalledWith('chcp', ['437'])
+})
+
+test.each(['REG_SZ', 'REG_EXPAND_SZ'])('force replaces a different PNPM_HOME stored as %s', async (type) => {
+  mockRegistry(type, 'C:\\other')
+  const report = await addDirToWindowsEnvPath(home, { ...opts, overwriteProxyVar: true })
+  expect(report[0]).toStrictEqual({ variable: 'PNPM_HOME', action: 'updated', oldValue: 'C:\\other', newValue: home })
+  expect(registryWrites()).toStrictEqual([
+    ['reg', ['add', regKey, '/v', 'PNPM_HOME', '/t', 'REG_SZ', '/d', home, '/f'], { windowsHide: false }],
+  ])
+})
+
+test('propagates a failed type repair and restores the code page', async () => {
+  mockRegistry('REG_EXPAND_SZ')
+  execa['mockRejectedValueOnce'](Object.assign(new Error('Access denied'), { stderr: 'Access denied' }))
+  await expect(addDirToWindowsEnvPath(home, opts)).rejects.toMatchObject({ code: 'ERR_PNPM_FAILED_SET_ENV' })
+  expect(registryWrites()).toHaveLength(1)
+  expect(execa['mock'].calls.some(([command]: [string]) => command === 'setx')).toBe(false)
+  expect(execa).toHaveBeenLastCalledWith('chcp', ['437'])
+})