fix(update): stop reporting an update that never installed

.changeset/doctor-native-install.md

@@ -0,0 +1,5 @@
+---
+"@pythoughts/pythinker-code": patch
+---
+
+Fix `pythinker doctor` crashing on native installs, and report the last recorded update outcome.

.changeset/update-verify-before-reporting-success.md

@@ -0,0 +1,5 @@
+---
+"@pythoughts/pythinker-code": patch
+---
+
+Stop reporting an update as installed when the executable did not change; the version is checked after the installer finishes and a mismatch is recorded as a failure with the reason.

.changeset/windows-installer-progress.md

@@ -0,0 +1,5 @@
+---
+"@pythoughts/pythinker-code": patch
+---
+
+Show download progress under the prompt while a Windows update installs, instead of nothing until it finishes.

.changeset/windows-package-manager-updates.md

@@ -0,0 +1,5 @@
+---
+"@pythoughts/pythinker-code": patch
+---
+
+Fix automatic updates on Windows for npm, pnpm, and yarn installs, which failed to start at all.

apps/pythinker-code/src/cli/sub/doctor.ts

@@ -24,7 +24,7 @@ import {
 } from '#/cli/update/preflight';
 import { detectInstallSource } from '#/cli/update/source';
 import type { UpdateInstallFailure } from '#/cli/update/types';
-import { getHostPackageRoot, getVersion } from '#/cli/version';
+import { findHostPackageRoot, getVersion } from '#/cli/version';
 import { getUpdateInstallLogFile } from '#/utils/paths';
 
 interface WritableLike {
@@ -50,7 +50,8 @@ export interface DoctorDeps {
 export interface DoctorRuntimeInfo {
   readonly version: string;
   readonly installSource: string;
-  readonly packageRoot: string;
+  /** Absent on a native binary: a packaged install has no `package.json`. */
+  readonly packageRoot?: string;
   readonly executable: string;
   readonly installations?: readonly string[];
   readonly ripgrep?: RgResolution;
@@ -62,6 +63,7 @@ export interface DoctorRuntimeInfo {
     readonly pendingVersion?: string;
     readonly pendingRequestedBy?: 'automatic' | 'manual';
     readonly activeOperation?: string;
+    readonly lastSuccess?: string;
     readonly lastFailure?: string;
     readonly logPath?: string;
   };
@@ -191,7 +193,7 @@ function resolveDeps(deps: Partial<DoctorDeps> | DoctorDeps | undefined): Resolv
         return {
           version: getVersion(),
           installSource,
-          packageRoot: getHostPackageRoot(),
+          packageRoot: findHostPackageRoot() ?? undefined,
           executable: process.execPath,
           installations,
           ripgrep,
@@ -206,6 +208,14 @@ function resolveDeps(deps: Partial<DoctorDeps> | DoctorDeps | undefined): Resolv
               installState.active === null
                 ? undefined
                 : `${installState.active.operation ?? 'install'} ${installState.active.version}`,
+            lastSuccess:
+              installState.lastSuccess === null
+                ? undefined
+                : `${installState.lastSuccess.version} (installed ` +
+                  `${installState.lastSuccess.installedAt})` +
+                  (installState.lastSuccess.unverified === undefined
+                    ? ''
+                    : ` — unverified: ${installState.lastSuccess.unverified}`),
             lastFailure:
               installState.lastFailure === null
                 ? undefined
@@ -387,7 +397,7 @@ function formatRuntimeInfo(info: DoctorRuntimeInfo | undefined): string[] {
     'Runtime',
     `  Version: ${info.version}`,
     `  Install source: ${info.installSource}`,
-    `  Package root: ${info.packageRoot}`,
+    ...(info.packageRoot === undefined ? [] : [`  Package root: ${info.packageRoot}`]),
     `  Executable: ${info.executable}`,
     ...(installations.length > 1
       ? [
@@ -414,6 +424,9 @@ function formatRuntimeInfo(info: DoctorRuntimeInfo | undefined): string[] {
           ...(info.update.activeOperation === undefined
             ? []
             : [`  Update operation: ${info.update.activeOperation}`]),
+          ...(info.update.lastSuccess === undefined
+            ? []
+            : [`  Last update success: ${info.update.lastSuccess}`]),
           ...(info.update.lastFailure === undefined
             ? []
             : [`  Last update failure: ${info.update.lastFailure}`]),

apps/pythinker-code/src/cli/sub/upgrade.ts

@@ -13,6 +13,7 @@ import {
 } from '#/cli/update/install-state';
 import { isTargetInstallable, selectUpdateTarget } from '#/cli/update/select';
 import { detectInstallSource } from '#/cli/update/source';
+import type { InstallOutcome } from '#/cli/update/verify-install';
 import {
   canAutoInstall,
   installCommandFor,
@@ -47,7 +48,7 @@ export interface UpgradeDeps {
     source: InstallSource,
     version: string,
     platform: NodeJS.Platform,
-  ) => Promise<void>;
+  ) => Promise<InstallOutcome>;
   readonly promptForInstallChoice: (
     options: InstallPromptOptions,
   ) => Promise<InstallPromptChoiceValue>;
@@ -180,7 +181,7 @@ export async function handleUpgrade(
       target_version: target.version,
       source,
     });
-    await deps.installUpdate(source, target.version, deps.platform);
+    const outcome = await deps.installUpdate(source, target.version, deps.platform);
     await deps.writeUpdateInstallState({
       ...installState,
       active: null,
@@ -189,6 +190,7 @@ export async function handleUpgrade(
         version: target.version,
         installedAt: nowIso(),
         notifiedAt: null,
+        unverified: outcome.unverified,
       },
     }).catch(() => {});
     trackUpgradeEvent(deps.track, 'upgrade_command_succeeded', {

apps/pythinker-code/src/cli/update/install-state.ts

@@ -153,6 +153,7 @@ const UpdateInstallStateSchema: z.ZodType<UpdateInstallState> = z
         version: z.string().min(1),
         installedAt: z.string().min(1),
         notifiedAt: z.string().min(1).nullable(),
+        unverified: z.string().min(1).optional(),
       })
       .strict()
       .nullable(),

apps/pythinker-code/src/cli/update/preflight.ts

@@ -56,9 +56,17 @@ import {
   type UpdateRequestOrigin,
   type UpdateTarget,
 } from './types';
+import {
+  verifyInstalledVersion,
+  type InstallOutcome,
+  type InstallVerification,
+} from './verify-install';
 
 export type { UpdatePreflightResult } from './types';
 
+/** Reused for the paths that never reach verification (a failed install). */
+const OK_VERIFICATION: InstallVerification = { ok: true };
+
 export interface RunUpdatePreflightOptions {
   readonly stdout?: { write(chunk: string): boolean };
   readonly stderr?: { write(chunk: string): boolean };
@@ -81,6 +89,33 @@ function bunCommand(platform: NodeJS.Platform): string {
   return platform === 'win32' ? 'bun.exe' : 'bun';
 }
 
+/**
+ * Node ≥18.20/20.12 refuses to spawn a `.cmd`/`.bat` file directly
+ * (CVE-2024-27980) and fails with `EINVAL` — which is every npm-family update
+ * on Windows: `npm.cmd`, `pnpm.cmd`, `yarn.cmd`. The command interpreter runs
+ * them instead. It is spelled out as argv rather than `shell: true` so the
+ * exact command line is visible here (and asserted in tests) instead of being
+ * assembled by Node's string joining.
+ */
+function viaCommandInterpreter(command: SpawnCommand): SpawnCommand {
+  return {
+    ...command,
+    cmd: process.env['ComSpec'] ?? 'cmd.exe',
+    args: ['/d', '/s', '/c', command.cmd, ...command.args],
+  };
+}
+
+/** True for the Windows package-manager shims that cannot be spawned directly. */
+export function isWindowsShim(cmd: string, platform: NodeJS.Platform): boolean {
+  if (platform !== 'win32') return false;
+  const lower = cmd.toLowerCase();
+  return lower.endsWith('.cmd') || lower.endsWith('.bat');
+}
+
+function spawnable(command: SpawnCommand, platform: NodeJS.Platform): SpawnCommand {
+  return isWindowsShim(command.cmd, platform) ? viaCommandInterpreter(command) : command;
+}
+
 export function installCommandFor(
   source: InstallSource,
   version: string,
@@ -145,11 +180,20 @@ export function spawnForSource(
 ): SpawnCommand {
   switch (source) {
     case 'npm-global':
-      return { cmd: withCmdSuffix('npm', platform), args: ['install', '-g', `${NPM_PACKAGE_NAME}@${version}`] };
+      return spawnable(
+        { cmd: withCmdSuffix('npm', platform), args: ['install', '-g', `${NPM_PACKAGE_NAME}@${version}`] },
+        platform,
+      );
     case 'pnpm-global':
-      return { cmd: withCmdSuffix('pnpm', platform), args: ['add', '-g', `${NPM_PACKAGE_NAME}@${version}`] };
+      return spawnable(
+        { cmd: withCmdSuffix('pnpm', platform), args: ['add', '-g', `${NPM_PACKAGE_NAME}@${version}`] },
+        platform,
+      );
     case 'yarn-global':
-      return { cmd: withCmdSuffix('yarn', platform), args: ['global', 'add', `${NPM_PACKAGE_NAME}@${version}`] };
+      return spawnable(
+        { cmd: withCmdSuffix('yarn', platform), args: ['global', 'add', `${NPM_PACKAGE_NAME}@${version}`] },
+        platform,
+      );
     case 'bun-global':
       return { cmd: bunCommand(platform), args: ['add', '-g', `${NPM_PACKAGE_NAME}@${version}`] };
     case 'homebrew':
@@ -543,7 +587,7 @@ export async function installUpdate(
   source: InstallSource,
   version: string,
   platform: NodeJS.Platform,
-): Promise<void> {
+): Promise<InstallOutcome> {
   const { cmd, args, env } = spawnForSource(source, version, platform);
   await new Promise<void>((resolve, reject) => {
     const child = spawn(cmd, [...args], {
@@ -560,6 +604,14 @@ export async function installUpdate(
       reject(new Error(`${cmd} exited with ${detail}`));
     });
   });
+  // Exit code 0 is the installer's opinion; this is the fact. Rejecting here
+  // routes a silent no-op install into the same failure reporting a crashed
+  // installer gets, instead of printing "Updated …" over an unchanged binary.
+  const verification = await verifyInstalledVersion(source, version);
+  if (!verification.ok) throw new Error(verification.reason);
+  // Returned so the caller can record *why* a success is unproven; see
+  // verify-install.ts for the fail-open rule.
+  return { unverified: verification.unverified };
 }
 
 /** Keep the tail only: installers can be chatty, and the state file is small. */
@@ -861,11 +913,19 @@ async function startBackgroundInstall(
       // `settled` already stops new progress writes; drain the ones in flight so
       // none of them renames over the outcome below.
       await progressWrites;
+      // An installer that exits 0 without replacing the binary must not be
+      // recorded as a success: the footer would advertise "restart to apply"
+      // for a version that never runs, on every launch, forever.
+      const verification = succeeded
+        ? await verifyInstalledVersion(source, target.version)
+        : OK_VERIFICATION;
+      const installed = succeeded && verification.ok;
+      const outcomeReason = verification.ok ? reason : verification.reason;
       const attempts = failureAttemptsFor(startedState, target, 'install') + 1;
       const stderrTail = readStderrTail();
-      const message = stderrTail === undefined ? reason : `${reason}: ${stderrTail}`;
+      const message = stderrTail === undefined ? outcomeReason : `${outcomeReason}: ${stderrTail}`;
 
-      const nextState: UpdateInstallState = succeeded
+      const nextState: UpdateInstallState = installed
         ? {
           ...startedState,
           active: null,
@@ -874,6 +934,7 @@ async function startBackgroundInstall(
             version: target.version,
             installedAt: nowIso(),
             notifiedAt: null,
+            unverified: verification.ok ? verification.unverified : undefined,
           },
         }
         : {
@@ -889,14 +950,17 @@ async function startBackgroundInstall(
         };
       try {
         await writeUpdateInstallState(nextState).catch(() => {});
-        if (succeeded) {
+        if (installed) {
           trackUpdateEvent(track, 'update_background_install_succeeded', {
             target_version: target.version,
             source,
           });
           logUpdateInfo(logger, 'background update install succeeded', {
             targetVersion: target.version,
             source,
+            // Present when the install was recorded without proof, so a report
+            // of "it says updated but it did not" is answerable from the log.
+            unverified: verification.ok ? verification.unverified : undefined,
           });
           return;
         }
@@ -1392,7 +1456,7 @@ export async function runUpdatePreflight(
     if (lock === null) return 'continue';
 
     try {
-      await installUpdate(source, userVisibleTarget.version, platform);
+      const outcome = await installUpdate(source, userVisibleTarget.version, platform);
       await writeUpdateInstallState({
         ...installState,
         active: null,
@@ -1401,6 +1465,7 @@ export async function runUpdatePreflight(
           version: userVisibleTarget.version,
           installedAt: nowIso(),
           notifiedAt: null,
+          unverified: outcome.unverified,
         },
       }).catch(() => {});
       stdout.write(renderInstallSuccessMessage(userVisibleTarget));

apps/pythinker-code/src/cli/update/source.ts

@@ -76,15 +76,30 @@ function npmCommand(platform: NodeJS.Platform): string {
   return platform === 'win32' ? 'npm.cmd' : 'npm';
 }
 
-function execFileText(command: string, args: readonly string[]): Promise<string> {
+function execFileText(
+  command: string,
+  args: readonly string[],
+  platform: NodeJS.Platform = process.platform,
+): Promise<string> {
+  // `npm.cmd` cannot be spawned directly on Node ≥18.20/20.12
+  // (CVE-2024-27980): it fails with EINVAL, and every npm-family Windows
+  // install then classifies as `unsupported` and never auto-updates.
+  const viaInterpreter = platform === 'win32' && command.toLowerCase().endsWith('.cmd');
+  const spawnCommand = viaInterpreter ? process.env['ComSpec'] ?? 'cmd.exe' : command;
+  const spawnArgs = viaInterpreter ? ['/d', '/s', '/c', command, ...args] : [...args];
   return new Promise((resolveOutput, reject) => {
-    execFile(command, [...args], { encoding: 'utf-8' }, (error, stdout) => {
-      if (error) {
-        reject(error);
-        return;
-      }
-      resolveOutput(stdout);
-    });
+    execFile(
+      spawnCommand,
+      spawnArgs,
+      { encoding: 'utf-8', windowsHide: true },
+      (error, stdout) => {
+        if (error) {
+          reject(error);
+          return;
+        }
+        resolveOutput(stdout);
+      },
+    );
   });
 }
 
@@ -140,14 +155,22 @@ export async function detectInstallSource(
     getPackageRoot: deps.getPackageRoot ?? getHostPackageRoot,
     getGlobalPrefix:
       deps.getGlobalPrefix ??
-      (() => execFileText(npmCommand(platform), ['prefix', '-g']).then((text) => text.trim())),
+      (() =>
+        execFileText(npmCommand(platform), ['prefix', '-g'], platform).then((text) => text.trim())),
     detectNative: deps.detectNative ?? detectNativeInstall,
     platform,
   };
 
   if (resolved.detectNative()) return 'native';
 
-  const packageRoot = resolved.getPackageRoot();
+  // A layout with no reachable `package.json` cannot be classified, and this
+  // runs on every launch — it reports "unsupported" rather than throwing.
+  let packageRoot: string;
+  try {
+    packageRoot = resolved.getPackageRoot();
+  } catch {
+    return 'unsupported';
+  }
   const heuristic = classifyByPathHeuristic(packageRoot);
   if (heuristic !== null) return heuristic;
 

apps/pythinker-code/src/cli/update/types.ts

@@ -112,6 +112,12 @@ export interface UpdateInstallSuccess {
   readonly version: string;
   readonly installedAt: string;
   readonly notifiedAt: string | null;
+  /**
+   * Why this success was recorded without proof that the new version runs.
+   * Absent when the installed binary was probed and matched. `doctor` prints
+   * it, so "it says updated but it did not" is answerable in one command.
+   */
+  readonly unverified?: string;
 }
 
 export interface UpdateInstallState {