AIT-395: fix both Windows bugs — command shims and the teardown abort (0.14.16)

.github/workflows/ci.yml

@@ -13,6 +13,8 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v4
+        with:
+          persist-credentials: false
       - uses: actions/setup-node@v4
         with:
           node-version-file: '.node-version'
@@ -28,8 +30,47 @@ jobs:
       - run: npm test
       - run: node bin/hookmyapp.js --version
 
+  # AIT-395: both Windows bugs a customer reported (bare `spawnSync` never
+  # resolving `.cmd` shims, and the libuv abort at teardown) shipped because
+  # every job here was ubuntu-only. Mocked unit tests with an injected platform
+  # cannot catch either — only actually running the CLI on Windows can.
+  test-windows:
+    runs-on: windows-latest
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          persist-credentials: false
+      - uses: actions/setup-node@v4
+        with:
+          node-version-file: '.node-version'
+          cache: 'npm'
+      - run: npm ci
+      - run: node build.mjs
+      - run: npm test
+      # Exercises the exit path itself: a clean 0, not the exit-9 abort.
+      - run: node bin/hookmyapp.js --version
+      - name: doctor sees the npm/npx shims and exits cleanly
+        shell: pwsh
+        run: |
+          $out = node bin/hookmyapp.js doctor --json 2>&1 | Out-String
+          $code = $LASTEXITCODE
+          Write-Host $out
+          # Parse first: a teardown abort produces no JSON at all, so this is
+          # what actually distinguishes "crashed" from "ran and reported".
+          try { $report = $out | ConvertFrom-Json }
+          catch { throw "doctor produced no JSON report (exit $code) — likely the libuv teardown abort" }
+          foreach ($id in @('npm', 'npx')) {
+            $check = $report.checks | Where-Object { $_.id -eq $id }
+            if (-not $check.ok) { throw "doctor could not see $id on Windows: $($check.detail)" }
+          }
+          # An unreachable API is a runner network blip, not a regression: only
+          # fail on a non-zero exit that the network check does not explain.
+          $network = $report.checks | Where-Object { $_.id -eq 'network' }
+          if ($code -ne 0 -and $network.ok) { throw "doctor exited $code with every hard check passing (9 = the libuv teardown abort)" }
+          if (-not $network.ok) { Write-Host "note: API unreachable from this runner; exit-code assertion skipped" }
+
   notify-failure:
-    needs: [test]
+    needs: [test, test-windows]
     if: always() && github.event_name == 'push' && github.ref == 'refs/heads/main' && (contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled'))
     runs-on: ubuntu-latest
     timeout-minutes: 2

CHANGELOG.md

@@ -2,6 +2,13 @@
 
 All notable changes to `@gethookmyapp/cli` are documented here.
 
+## 0.14.16 — 2026-08-14
+
+### Fixed
+
+- Commands no longer crash on Windows after finishing their work. Every command that made a network call printed the right output and then died with an assertion from Node's event loop and exit code 9, which made successful runs look like failures and made exit codes unusable in scripts and CI (AIT-395).
+- `hookmyapp doctor` no longer reports `npm` and `npx` as missing on Windows when both are installed and working, and no longer exits 1 on a healthy install. `hookmyapp mcp` had the same problem locating Claude Code on Windows and is fixed with it. npm and npx are now reported for information only: the CLI runs on Node alone and never calls them, so they cannot block setup (AIT-395).
+
 ## 0.14.15 — 2026-08-13
 
 ### Changed

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@gethookmyapp/cli",
-  "version": "0.14.15",
+  "version": "0.14.16",
   "description": "HookMyApp CLI, No BS. Just go live.",
   "type": "module",
   "bin": {

src/__tests__/flush-exit.test.ts

@@ -50,11 +50,10 @@ describe('flushAndExit awaits PostHog AND Sentry in parallel', () => {
   it('calls posthog.shutdown(2000) when PostHog client is initialised', async () => {
     process.env.HOOKMYAPP_POSTHOG_TOKEN = 'phc_test';
     await initPostHogLazy(); // initialise
-    const exitSpy = vi.spyOn(process, 'exit').mockImplementation(((code?: number) => {
-      throw new Error(`__test_exit_${code ?? 0}__`);
-    }) as never);
+    const exitSpy = vi.spyOn(process, 'exit').mockImplementation(() => undefined as never);
     try {
-      await expect(flushAndExit(0)).rejects.toThrow('__test_exit_0__');
+      await flushAndExit(0);
+      expect(process.exitCode).toBe(0);
       expect(fakeShutdown).toHaveBeenCalledTimes(1);
       expect(fakeShutdown).toHaveBeenCalledWith(2000);
     } finally {
@@ -66,12 +65,12 @@ describe('flushAndExit awaits PostHog AND Sentry in parallel', () => {
     process.env.HOOKMYAPP_POSTHOG_TOKEN = 'phc_test';
     fakeShutdown.mockRejectedValueOnce(new Error('posthog drop'));
     await initPostHogLazy();
-    const exitSpy = vi.spyOn(process, 'exit').mockImplementation(((code?: number) => {
-      throw new Error(`__test_exit_${code ?? 0}__`);
-    }) as never);
+    const exitSpy = vi.spyOn(process, 'exit').mockImplementation(() => undefined as never);
     try {
-      await expect(flushAndExit(2)).rejects.toThrow('__test_exit_2__');
-      expect(exitSpy).toHaveBeenCalledWith(2);
+      await flushAndExit(2);
+      // AIT-395: status set, process left to drain — never a hard exit.
+      expect(process.exitCode).toBe(2);
+      expect(exitSpy).not.toHaveBeenCalled();
     } finally {
       exitSpy.mockRestore();
     }
@@ -81,11 +80,9 @@ describe('flushAndExit awaits PostHog AND Sentry in parallel', () => {
     process.env.HOOKMYAPP_TELEMETRY = 'off';
     process.env.HOOKMYAPP_POSTHOG_TOKEN = 'phc_test';
     await initPostHogLazy(); // returns null — no client
-    const exitSpy = vi.spyOn(process, 'exit').mockImplementation(((code?: number) => {
-      throw new Error(`__test_exit_${code ?? 0}__`);
-    }) as never);
+    const exitSpy = vi.spyOn(process, 'exit').mockImplementation(() => undefined as never);
     try {
-      await expect(flushAndExit(0)).rejects.toThrow('__test_exit_0__');
+      await flushAndExit(0);
       expect(fakeShutdown).not.toHaveBeenCalled();
     } finally {
       exitSpy.mockRestore();

src/__tests__/notifications-nudge.test.ts

@@ -1,7 +1,7 @@
 import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
 import { mkdtempSync, readFileSync, readdirSync, rmSync } from 'node:fs';
 import { tmpdir } from 'node:os';
-import { join } from 'node:path';
+import { basename, join } from 'node:path';
 
 vi.mock('node:child_process', () => ({
   spawn: vi.fn(),
@@ -256,7 +256,7 @@ describe('maybeNudge banner', () => {
 describe('cache namespacing', () => {
   it('readCache falls back to the pre-rename notices-nudge filename after an upgrade', () => {
     const file = cacheFilePath(getEffectiveApiUrl(), 'agc_legacy');
-    const legacy = join(dir, file.split('/').pop()!.replace('notifications-nudge-', 'notices-nudge-'));
+    const legacy = join(dir, basename(file).replace('notifications-nudge-', 'notices-nudge-'));
     writeCacheAtomic(legacy, unreadCache);
     expect(readCache(file)?.hasUnread).toBe(true); // unread state survives the rename
   });

src/__tests__/sentry-init.test.ts

@@ -190,27 +190,30 @@ describe('shouldCaptureToSentry filter — capture every non-null error', () =>
 });
 
 describe('flushAndExit', () => {
-  it('calls process.exit with the provided code when Sentry is not initialized (fast path)', async () => {
-    const exitSpy = vi.spyOn(process, 'exit').mockImplementation(((code?: number) => {
-      throw new Error(`__test_exit_${code ?? 0}__`);
-    }) as never);
+  // AIT-395: the contract changed from "hard exit" to "set the status and let
+  // the loop drain" — process.exit() on top of a closing libuv handle aborts
+  // the process on Windows (exit 9) after the command already succeeded.
+  it('sets the provided exit code when Sentry is not initialized (fast path)', async () => {
+    const exitSpy = vi.spyOn(process, 'exit').mockImplementation(() => undefined as never);
     try {
-      await expect(flushAndExit(2)).rejects.toThrow('__test_exit_2__');
-      expect(exitSpy).toHaveBeenCalledWith(2);
+      await flushAndExit(2);
+      expect(process.exitCode).toBe(2);
+      expect(exitSpy).not.toHaveBeenCalled();
     } finally {
       exitSpy.mockRestore();
+      process.exitCode = undefined;
     }
   });
 
-  it('exits with 0 when passed 0', async () => {
-    const exitSpy = vi.spyOn(process, 'exit').mockImplementation(((code?: number) => {
-      throw new Error(`__test_exit_${code ?? 0}__`);
-    }) as never);
+  it('sets 0 when passed 0', async () => {
+    const exitSpy = vi.spyOn(process, 'exit').mockImplementation(() => undefined as never);
     try {
-      await expect(flushAndExit(0)).rejects.toThrow('__test_exit_0__');
-      expect(exitSpy).toHaveBeenCalledWith(0);
+      await flushAndExit(0);
+      expect(process.exitCode).toBe(0);
+      expect(exitSpy).not.toHaveBeenCalled();
     } finally {
       exitSpy.mockRestore();
+      process.exitCode = undefined;
     }
   });
 });

src/commands/__tests__/billing.test.ts

@@ -476,8 +476,9 @@ describe('billingUpgrade — free tier plan prompt (GET /plans)', () => {
     await assertion;
   });
   // advanceUntilSettled drives up to 500 real event-loop turns per poll test.
-  // On a loaded CI runner that can exceed the 5s default and time out even
-  // though nothing is wrong — it already timed out once on this branch. The
-  // block-level budget keeps these tests honest without making each one
-  // declare its own.
-}, 30_000);
+  // On a loaded CI runner that can exceed the default and time out even
+  // though nothing is wrong. The block-level budget keeps these tests honest
+  // without making each one declare its own — and it OVERRIDES the config's
+  // testTimeout, so raising the budget means raising it here (AIT-395: 30s
+  // still wasn't enough on either runner).
+}, 60_000);

src/commands/__tests__/mcp.test.ts

@@ -1,10 +1,16 @@
 import { beforeEach, describe, expect, test, vi } from 'vitest';
-import { spawnSync } from 'node:child_process';
+import { runTool } from '../../lib/spawn-tool.js';
 import { Command } from 'commander';
 import { resolve } from 'node:path';
 import { getValidAccessToken } from '../../api/client.js';
 
-vi.mock('node:child_process', () => ({ spawnSync: vi.fn() }));
+// Mock the tool-spawn seam, not node:child_process: `runTool` adds the
+// cmd.exe /c prefix on Windows, so asserting raw runTool argv here is a
+// platform-dependent test, not a contract test (AIT-395).
+vi.mock('../../lib/spawn-tool.js', async (importOriginal) => ({
+  ...(await importOriginal<typeof import('../../lib/spawn-tool.js')>()),
+  runTool: vi.fn(),
+}));
 vi.mock('../../api/client.js', () => ({ getValidAccessToken: vi.fn() }));
 vi.mock('../../config/env-profiles.js', () => ({
   getEffectiveApiUrl: () => 'https://api.hookmyapp.com',
@@ -33,11 +39,11 @@ describe('MCP setup', () => {
   });
 
   test('installs a user-scoped Claude headersHelper without storing a token', () => {
-    vi.mocked(spawnSync).mockReturnValue({ status: 0 } as never);
+    vi.mocked(runTool).mockReturnValue({ status: 0 } as never);
 
     installClaudeMcp();
 
-    const [, args] = vi.mocked(spawnSync).mock.calls[0];
+    const [, args] = vi.mocked(runTool).mock.calls[0];
     expect(args).toEqual([
       'mcp',
       'add-json',
@@ -54,14 +60,14 @@ describe('MCP setup', () => {
   });
 
   test('skips automatic setup when Claude Code is absent', () => {
-    vi.mocked(spawnSync).mockReturnValueOnce({
+    vi.mocked(runTool).mockReturnValueOnce({
       status: null,
       error: Object.assign(new Error('ENOENT'), { code: 'ENOENT' }),
     } as never);
 
     maybeInstallClaudeMcp(true);
 
-    expect(spawnSync).toHaveBeenCalledOnce();
+    expect(runTool).toHaveBeenCalledOnce();
   });
 
   test('shell-quotes helper paths without expanding metacharacters', () => {
@@ -72,30 +78,30 @@ describe('MCP setup', () => {
   });
 
   test('replaces an existing Claude entry', () => {
-    vi.mocked(spawnSync)
+    vi.mocked(runTool)
       .mockReturnValueOnce({ status: 1, stderr: 'already exists' } as never)
       .mockReturnValueOnce({ status: 0 } as never)
       .mockReturnValueOnce({ status: 0 } as never);
 
     installClaudeMcp();
 
-    expect(spawnSync).toHaveBeenCalledTimes(3);
-    expect(vi.mocked(spawnSync).mock.calls[1][1]).toEqual(['mcp', 'remove', '--scope', 'user', 'hookmyapp']);
+    expect(runTool).toHaveBeenCalledTimes(3);
+    expect(vi.mocked(runTool).mock.calls[1][1]).toEqual(['mcp', 'remove', '--scope', 'user', 'hookmyapp']);
   });
 
   test('removes only the user-scoped HookMyApp entry', () => {
-    vi.mocked(spawnSync).mockReturnValue({ status: 0 } as never);
+    vi.mocked(runTool).mockReturnValue({ status: 0 } as never);
 
     removeClaudeMcp(true);
 
-    expect(spawnSync).toHaveBeenCalledWith('claude', ['mcp', 'remove', '--scope', 'user', 'hookmyapp'], {
+    expect(runTool).toHaveBeenCalledWith('claude', ['mcp', 'remove', '--scope', 'user', 'hookmyapp'], {
       encoding: 'utf8',
       timeout: 10_000,
     });
   });
 
   test('treats missing Claude as successful cleanup', () => {
-    vi.mocked(spawnSync).mockReturnValue({
+    vi.mocked(runTool).mockReturnValue({
       status: null,
       error: Object.assign(new Error('ENOENT'), { code: 'ENOENT' }),
     } as never);
@@ -104,24 +110,24 @@ describe('MCP setup', () => {
   });
 
   test('treats an absent MCP entry as successful cleanup', () => {
-    vi.mocked(spawnSync).mockReturnValue({ status: 1, stderr: 'MCP server hookmyapp not found' } as never);
+    vi.mocked(runTool).mockReturnValue({ status: 1, stderr: 'MCP server hookmyapp not found' } as never);
 
     expect(removeClaudeMcp(true)).toEqual({ ok: true });
   });
 
   test('reports a bounded Claude status timeout', async () => {
-    vi.mocked(spawnSync).mockReturnValue({
+    vi.mocked(runTool).mockReturnValue({
       status: null,
       error: Object.assign(new Error('timed out'), { code: 'ETIMEDOUT' }),
     } as never);
     const { getClaudeMcpStatus } = await import('../mcp.js');
 
     expect(getClaudeMcpStatus()).toEqual({ ok: false, detail: 'Claude MCP check timed out' });
-    expect(vi.mocked(spawnSync).mock.calls[0][2]).toMatchObject({ timeout: 10_000 });
+    expect(vi.mocked(runTool).mock.calls[0][2]).toMatchObject({ timeout: 10_000 });
   });
 
   test('emits JSON for mcp install in global JSON mode', async () => {
-    vi.mocked(spawnSync).mockReturnValue({ status: 0 } as never);
+    vi.mocked(runTool).mockReturnValue({ status: 0 } as never);
     const write = vi.spyOn(process.stdout, 'write').mockImplementation(() => true);
     const program = new Command().option('--json');
     registerMcpCommand(program);

src/commands/doctor.ts

@@ -1,5 +1,5 @@
 import type { Command } from 'commander';
-import { spawnSync } from 'node:child_process';
+import { runTool } from '../lib/spawn-tool.js';
 import { readCredentials } from '../auth/store.js';
 import { apiClient } from '../api/client.js';
 import { AuthError, ForbiddenError, PermissionError } from '../output/error.js';
@@ -18,7 +18,7 @@ function parseMajor(v: string): number { return Number(v.replace(/^v/, '').split
 
 function toolVersion(cmd: string): string | null {
   try {
-    const r = spawnSync(cmd, ['-v'], { encoding: 'utf8' });
+    const r = runTool(cmd, ['-v'], { encoding: 'utf8' });
     return r.status === 0 ? (r.stdout || '').trim() : null;
   } catch { return null; }
 }
@@ -33,11 +33,13 @@ export async function collectDoctorReport(
   checks.push({ id: 'node', label: 'Node.js >= 20', ok: nodeOk, hard: true, detail: nodeOk ? nodeV : `${nodeV} — upgrade at https://nodejs.org` });
 
   // npm/npx presence (best-effort; gated so unit tests stay hermetic).
+  // Reported, never a gate: the CLI never shells out to npm, so a probe that
+  // can't see it says nothing about whether the CLI works (AIT-395).
   if (opts.checkTools !== false) {
     const npm = toolVersion('npm');
-    checks.push({ id: 'npm', label: 'npm', ok: npm !== null, hard: true, detail: npm ?? 'not found on PATH' });
+    checks.push({ id: 'npm', label: 'npm', ok: npm !== null, hard: false, detail: npm ?? 'not found on PATH' });
     const npx = toolVersion('npx');
-    checks.push({ id: 'npx', label: 'npx', ok: npx !== null, hard: true, detail: npx ?? 'not found on PATH' });
+    checks.push({ id: 'npx', label: 'npx', ok: npx !== null, hard: false, detail: npx ?? 'not found on PATH' });
     const mcp = getClaudeMcpStatus();
     checks.push({ id: 'mcp', label: 'HookMyApp MCP (Claude)', ok: mcp.ok, hard: false, detail: mcp.detail });
   }