fix(windows): resolve dev-install's claude through where.exe, not a fixed suffix

__tests__/claude-executable.test.js

@@ -0,0 +1,124 @@
+/**
+ * Tests for Claude executable resolution in lib/utils/claude-executable.js
+ */
+
+const childProcess = require('child_process');
+
+const { pickClaudeExecutable, claudeExecutable } = require('../lib/utils/claude-executable');
+
+/**
+ * Load the module fresh with a faked platform and a stubbed execFileSync.
+ *
+ * The module destructures execFileSync at require time, so the spy has to exist
+ * before the require - and the cache has to be a fresh one per test, which a
+ * reset module gives for free.
+ */
+function loadWithPlatform(platform, whereImpl) {
+  const originalPlatform = process.platform;
+  Object.defineProperty(process, 'platform', { value: platform, configurable: true });
+  jest.resetModules();
+  const execFileSync = jest.spyOn(childProcess, 'execFileSync').mockImplementation(whereImpl);
+  const loaded = require('../lib/utils/claude-executable');
+  return {
+    ...loaded,
+    execFileSync,
+    restore: () => Object.defineProperty(process, 'platform', { value: originalPlatform, configurable: true })
+  };
+}
+
+describe('pickClaudeExecutable', () => {
+  test('uses plain claude on posix and ignores any where output', () => {
+    expect(pickClaudeExecutable('linux', '')).toBe('claude');
+    expect(pickClaudeExecutable('darwin', 'C:\\npm\\claude.cmd')).toBe('claude');
+  });
+
+  test('uses the npm global shim path that where.exe resolved', () => {
+    const shim = 'C:\\Users\\u\\AppData\\Roaming\\npm\\claude.cmd';
+    expect(pickClaudeExecutable('win32', `${shim}\r\n`)).toBe(shim);
+  });
+
+  test('uses claude.exe from the native installer instead of assuming .cmd', () => {
+    const native = 'C:\\Users\\u\\.local\\bin\\claude.exe';
+    expect(pickClaudeExecutable('win32', `${native}\r\n`)).toBe(native);
+  });
+
+  test('prefers claude.exe over a batch shim in either PATH order', () => {
+    const exe = 'C:\\Users\\u\\.local\\bin\\claude.exe';
+    const cmd = 'C:\\npm\\claude.cmd';
+    expect(pickClaudeExecutable('win32', `${exe}\r\n${cmd}\r\n`)).toBe(exe);
+    expect(pickClaudeExecutable('win32', `${cmd}\r\n${exe}\r\n`)).toBe(exe);
+  });
+
+  test('skips entries CreateProcess cannot launch', () => {
+    // npm ships an extensionless shell script and a .ps1 alongside the .cmd
+    const out = 'C:\\npm\\claude\r\nC:\\npm\\claude.ps1\r\nC:\\npm\\claude.cmd\r\n';
+    expect(pickClaudeExecutable('win32', out)).toBe('C:\\npm\\claude.cmd');
+  });
+
+  test('falls back to the cmd shim when nothing spawnable was resolved', () => {
+    expect(pickClaudeExecutable('win32', '')).toBe('claude.cmd');
+    expect(pickClaudeExecutable('win32', '  \r\n \r\n')).toBe('claude.cmd');
+    expect(pickClaudeExecutable('win32', undefined)).toBe('claude.cmd');
+    expect(pickClaudeExecutable('win32', 'C:\\npm\\claude.ps1\r\n')).toBe('claude.cmd');
+  });
+});
+
+describe('claudeExecutable', () => {
+  test('resolves and caches without a shell on this platform', () => {
+    const first = claudeExecutable();
+    expect(typeof first).toBe('string');
+    expect(first.length).toBeGreaterThan(0);
+    expect(claudeExecutable()).toBe(first);
+    if (process.platform !== 'win32') {
+      expect(first).toBe('claude');
+    }
+  });
+
+  test('does not ask where.exe anything off Windows', () => {
+    const mod = loadWithPlatform('linux', () => 'C:\\npm\\claude.cmd\r\n');
+    try {
+      expect(mod.claudeExecutable()).toBe('claude');
+      expect(mod.execFileSync).not.toHaveBeenCalled();
+    } finally {
+      mod.restore();
+    }
+  });
+
+  test('takes the win32 answer from where.exe and asks only once', () => {
+    const native = 'C:\\Users\\u\\.local\\bin\\claude.exe';
+    const mod = loadWithPlatform('win32', () => `${native}\r\n`);
+    try {
+      expect(mod.claudeExecutable()).toBe(native);
+      expect(mod.claudeExecutable()).toBe(native);
+      expect(mod.execFileSync).toHaveBeenCalledTimes(1);
+      expect(mod.execFileSync).toHaveBeenCalledWith('where.exe', ['claude'], expect.objectContaining({ encoding: 'utf8' }));
+    } finally {
+      mod.restore();
+    }
+  });
+
+  test('falls back to the shim mapping when where.exe itself fails', () => {
+    const mod = loadWithPlatform('win32', () => {
+      throw Object.assign(new Error('spawnSync where.exe ENOENT'), { code: 'ENOENT' });
+    });
+    try {
+      expect(mod.claudeExecutable()).toBe('claude.cmd');
+    } finally {
+      mod.restore();
+    }
+  });
+
+  test('resetClaudeExecutableCache forces a second resolution', () => {
+    let answer = 'C:\\npm\\claude.cmd\r\n';
+    const mod = loadWithPlatform('win32', () => answer);
+    try {
+      expect(mod.claudeExecutable()).toBe('C:\\npm\\claude.cmd');
+      answer = 'C:\\Users\\u\\.local\\bin\\claude.exe\r\n';
+      expect(mod.claudeExecutable()).toBe('C:\\npm\\claude.cmd');
+      mod.resetClaudeExecutableCache();
+      expect(mod.claudeExecutable()).toBe('C:\\Users\\u\\.local\\bin\\claude.exe');
+    } finally {
+      mod.restore();
+    }
+  });
+});

__tests__/cli-args.test.js

@@ -6,7 +6,9 @@ const path = require('path');
 const fs = require('fs');
 
 // Import parseArgs directly from cli.js (now exported for testing)
-const { parseArgs, VALID_TOOLS, pickClaudeExecutable, claudeSpawnPlan, claudeExecutable } = require('../bin/cli.js');
+// Executable resolution moved to lib/utils/claude-executable.js - see
+// __tests__/claude-executable.test.js
+const { parseArgs, VALID_TOOLS, claudeSpawnPlan } = require('../bin/cli.js');
 
 describe('CLI argument parsing', () => {
   // Save original process.exit and restore after each test
@@ -188,43 +190,6 @@ describe('CLI argument parsing', () => {
   });
 });
 
-describe('pickClaudeExecutable', () => {
-  test('uses plain claude on posix and ignores any where output', () => {
-    expect(pickClaudeExecutable('linux', '')).toBe('claude');
-    expect(pickClaudeExecutable('darwin', 'C:\\npm\\claude.cmd')).toBe('claude');
-  });
-
-  test('uses the npm global shim path that where.exe resolved', () => {
-    const shim = 'C:\\Users\\u\\AppData\\Roaming\\npm\\claude.cmd';
-    expect(pickClaudeExecutable('win32', `${shim}\r\n`)).toBe(shim);
-  });
-
-  test('uses claude.exe from the native installer instead of assuming .cmd', () => {
-    const native = 'C:\\Users\\u\\.local\\bin\\claude.exe';
-    expect(pickClaudeExecutable('win32', `${native}\r\n`)).toBe(native);
-  });
-
-  test('prefers claude.exe over a batch shim in either PATH order', () => {
-    const exe = 'C:\\Users\\u\\.local\\bin\\claude.exe';
-    const cmd = 'C:\\npm\\claude.cmd';
-    expect(pickClaudeExecutable('win32', `${exe}\r\n${cmd}\r\n`)).toBe(exe);
-    expect(pickClaudeExecutable('win32', `${cmd}\r\n${exe}\r\n`)).toBe(exe);
-  });
-
-  test('skips entries CreateProcess cannot launch', () => {
-    // npm ships an extensionless shell script and a .ps1 alongside the .cmd
-    const out = 'C:\\npm\\claude\r\nC:\\npm\\claude.ps1\r\nC:\\npm\\claude.cmd\r\n';
-    expect(pickClaudeExecutable('win32', out)).toBe('C:\\npm\\claude.cmd');
-  });
-
-  test('falls back to the cmd shim when nothing spawnable was resolved', () => {
-    expect(pickClaudeExecutable('win32', '')).toBe('claude.cmd');
-    expect(pickClaudeExecutable('win32', '  \r\n \r\n')).toBe('claude.cmd');
-    expect(pickClaudeExecutable('win32', undefined)).toBe('claude.cmd');
-    expect(pickClaudeExecutable('win32', 'C:\\npm\\claude.ps1\r\n')).toBe('claude.cmd');
-  });
-});
-
 describe('claudeSpawnPlan', () => {
   const args = ['plugin', 'install', 'agentsys-core@agentsys'];
 
@@ -277,16 +242,6 @@ describe('claudeSpawnPlan', () => {
     expect(claudeSpawnPlan('claude', ['plugin', 'install', 'x&calc']).args)
       .toEqual(['plugin', 'install', 'x&calc']);
   });
-
-  test('claudeExecutable resolves and caches without a shell on this platform', () => {
-    const first = claudeExecutable();
-    expect(typeof first).toBe('string');
-    expect(first.length).toBeGreaterThan(0);
-    expect(claudeExecutable()).toBe(first);
-    if (process.platform !== 'win32') {
-      expect(first).toBe('claude');
-    }
-  });
 });
 
 describe('VALID_TOOLS constant', () => {

__tests__/dev-install.test.js

@@ -3,6 +3,7 @@
  */
 
 const fs = require('fs');
+const os = require('os');
 const path = require('path');
 
 const devInstallPath = path.join(__dirname, '..', 'scripts', 'dev-install.js');
@@ -283,6 +284,120 @@ describe('dev-install script', () => {
     });
   });
 
+  describe('installClaude external commands', () => {
+    const realPlatform = process.platform;
+    const realComspec = process.env.comspec;
+    const realHome = process.env.HOME;
+    const realUserProfile = process.env.USERPROFILE;
+    let home;
+
+    beforeEach(() => {
+      // installClaude writes under HOME - point it at a scratch dir so the run
+      // cannot touch the developer's own ~/.claude.
+      home = fs.mkdtempSync(path.join(os.tmpdir(), 'dev-install-claude-'));
+      process.env.HOME = home;
+      process.env.USERPROFILE = home;
+      process.env.comspec = 'cmd.exe';
+    });
+
+    afterEach(() => {
+      Object.defineProperty(process, 'platform', { value: realPlatform, configurable: true });
+      restoreEnv('HOME', realHome);
+      restoreEnv('USERPROFILE', realUserProfile);
+      restoreEnv('comspec', realComspec);
+      fs.rmSync(home, { recursive: true, force: true });
+      jest.resetModules();
+      jest.clearAllMocks();
+    });
+
+    function restoreEnv(name, value) {
+      if (value === undefined) {
+        delete process.env[name];
+      } else {
+        process.env[name] = value;
+      }
+    }
+
+    /**
+     * Run installClaude with a faked platform and a stubbed child_process.
+     *
+     * whereOutput is what `where.exe claude` answers, which is what decides the
+     * executable on win32; spawn is what every other command does, so a test can
+     * make the claude call fail the way a real spawn failure does.
+     */
+    function runInstallClaude(platform, { whereOutput = '', spawn = () => '' } = {}) {
+      Object.defineProperty(process, 'platform', { value: platform, configurable: true });
+      jest.resetModules();
+      const childProcess = require('child_process');
+      const execFileSync = jest.spyOn(childProcess, 'execFileSync').mockImplementation((file, args, options) => {
+        if (/where\.exe$/i.test(file) && args[0] === 'claude') {
+          return whereOutput;
+        }
+        return spawn(file, args, options);
+      });
+      const logs = [];
+      const consoleLog = jest.spyOn(console, 'log').mockImplementation(msg => logs.push(String(msg)));
+      require(devInstallPath).installClaude();
+      consoleLog.mockRestore();
+      const claudeCalls = execFileSync.mock.calls.filter(([, args]) => args.includes('marketplace') || args.join(' ').includes('"marketplace"'));
+      return { execFileSync, logs, claudeCalls };
+    }
+
+    test('spawns the claude.exe where.exe resolved, with no cmd.exe hop', () => {
+      const native = 'C:\\Users\\u\\.local\\bin\\claude.exe';
+      const { claudeCalls, execFileSync } = runInstallClaude('win32', { whereOutput: `${native}\r\n` });
+
+      expect(claudeCalls).toEqual([[native, ['plugin', 'marketplace', 'remove', 'agent-sh/agentsys'], { stdio: 'pipe' }]]);
+      expect(execFileSync.mock.calls.some(([file]) => file === 'cmd.exe')).toBe(false);
+    });
+
+    test('routes the claude.cmd where.exe resolved through cmd.exe', () => {
+      const shim = 'C:\\npm\\claude.cmd';
+      const { claudeCalls } = runInstallClaude('win32', { whereOutput: `${shim}\r\n` });
+
+      expect(claudeCalls).toEqual([[
+        'cmd.exe',
+        ['/d', '/s', '/c', `""${shim}" "plugin" "marketplace" "remove" "agent-sh/agentsys""`],
+        { stdio: 'pipe', windowsVerbatimArguments: true }
+      ]]);
+    });
+
+    test('spawns plain claude off Windows', () => {
+      const { claudeCalls } = runInstallClaude('linux');
+
+      expect(claudeCalls).toEqual([['claude', ['plugin', 'marketplace', 'remove', 'agent-sh/agentsys'], { stdio: 'pipe' }]]);
+    });
+
+    test('reports a claude that could not be started at all', () => {
+      // A wrong executable used to be indistinguishable from nothing to remove:
+      // the catch swallowed it and the run claimed success having done nothing.
+      const { logs } = runInstallClaude('linux', {
+        spawn: (file, args) => {
+          if (args.includes('marketplace')) {
+            throw Object.assign(new Error('spawnSync claude ENOENT'), { code: 'ENOENT', status: null });
+          }
+          return '';
+        }
+      });
+
+      expect(logs.some(line => line.includes('[WARN]') && line.includes('Could not run claude') && line.includes('ENOENT'))).toBe(true);
+    });
+
+    test('stays quiet when claude ran and exited non-zero', () => {
+      // Nothing to remove is the normal case, not a failure worth a warning.
+      const { logs } = runInstallClaude('linux', {
+        spawn: (file, args) => {
+          if (args.includes('marketplace')) {
+            throw Object.assign(new Error('Command failed'), { status: 1 });
+          }
+          return '';
+        }
+      });
+
+      expect(logs.some(line => line.includes('[WARN]'))).toBe(false);
+    });
+  });
+
   describe('output', () => {
     test('logs with [dev-install] prefix', () => {
       expect(devInstallSource.includes('[dev-install]')).toBe(true);