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

## Related Issue

No issue — reported directly on Windows. The problem is described below.

## Problem

A Windows user on v0.12.0 saw `Updating to v0.13.1 / Installing in the
background`, then
`↑ v0.13.1 restart to apply` under the prompt. Restarting the terminal
still gave v0.12.0, and the
update log recorded the same install as *succeeded* several times over.

`where.exe pythinker` put
`%LOCALAPPDATA%\Programs\Pythinker\pythinker.exe` first, and the
published
0.13.1 Windows binary is correct (its sha256 matches the channel
manifest and it reports 0.13.1), so
the executable that ran was the one the installer targets and the
advertised version was real. What
was wrong is that nothing ever checked. Investigating it found four
defects:

- **A success is recorded from an exit code alone.** The background
finalizer writes `lastSuccess`
when the installer exits 0. No step asks whether the binary that runs
next is the target version,
so an installer that exits 0 without replacing anything advertises
"restart to apply" forever, on
  every launch.
- **`doctor` crashes on every native install.** It reports the package
root, and a packaged binary
  ships no `package.json`, so the command died with
`Error: Could not locate package.json near …` — exactly when a user
needs it most. The same
  lookup sits on the launch path in install-source detection.
- **npm-family auto-update cannot start on Windows.** `npm.cmd`,
`pnpm.cmd` and `yarn.cmd` are
spawned directly, which Node ≥18.20/20.12 refuses (CVE-2024-27980) with
`EINVAL`. The same call
fails in the npm-prefix lookup, so those installs also classify as
`unsupported`.
- **`install.ps1` emits no progress.** `install.sh` writes
machine-readable `progress:` lines on
stderr and the parent renders them; the PowerShell installer wrote none,
so the footer's
downloading state was unreachable on Windows and an update in flight
looked identical to a wedged
  one — the defect #38 set out to close, still open on one platform.

## What changed

**A success now means the new version runs.** After an installer exits
0, the version is verified
against the artifact the installer replaced, and a mismatch is recorded
as a failure carrying the
reason (`… still reports 0.12.0 (expected 0.13.1)`) instead of a
success. Only `native` installs are
verified, by probing `process.execPath --version`: an npm global
reinstall rewrites the directory
this process was loaded from, so nothing readable there proves what the
next launch runs, and a
wrong answer would park a healthy version.

Verification fails **open** — a probe that times out (an antivirus scan
on a fresh unsigned exe is
the realistic case), cannot run, or prints no version records the
success anyway, with a note saying
why it is unproven. `doctor` prints that note next to the recorded
outcome, so the next report of
"it says updated but it did not" is answerable in one command.

**Windows package-manager shims run through the command interpreter.**
`cmd.exe /d /s /c npm.cmd …`,
spelled out as argv rather than `shell: true`, so the exact command line
is visible in the source and
asserted in tests instead of being assembled by Node's string joining.
Same fix in the npm-prefix
lookup that classifies the install source.

**`install.ps1` speaks the progress protocol**, mirroring `install.sh`:
`state=waiting` while release
assets are not up yet, `state=downloading` with percent and byte counts
(one line per second at
most), `state=done`, and a single `state=failed` after the last retry —
not between attempts, which
would drop the footer out of its downloading state and back into a
failure it is about to recover
from.

**`doctor` survives a native install**, reporting the package root only
when there is one, and the
launch-path source detection classifies an unresolvable layout as
`unsupported` rather than throwing.
It also now prints the last recorded update success, which is what would
have shown the original
problem immediately.

One scope decision worth flagging: the interactive `Updated … to X`
message still prints unchanged
when a native probe could not run. The mismatch case — the actual lie —
throws and is reported as a
failure on both foreground paths; the unproven case only loses a line in
a flow the user is watching,
and it is recorded in the install state either way.

## Checklist

- [x] I have read the
[CONTRIBUTING](https://github.com/Pythoughts-labs/pythinker-code/blob/main/CONTRIBUTING.md)
document.
- [x] I have linked a related issue, or explained the problem above.
- [x] I have added tests that prove my feature works.
- [x] Ran `gen-changesets` skill, or this PR needs no changeset.
- [x] Ran `gen-docs` skill, or this PR needs no doc update. The
user-facing docs describe the
commands, not `doctor`'s runtime lines, and the update behaviour is
unchanged when an install
      really works.


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **Bug Fixes**
  - Fixed crashes when running diagnostics from native installations.
- Improved automatic updates for npm, pnpm, and Yarn installations on
Windows.
- Updates are no longer reported as successful when the installed
version remains unchanged.

- **New Features**
- Added post-update version verification with clearer failure and
unverified status reporting.
- Diagnostics now show the most recent successful update and its status.
- Windows installer downloads now display progress, waiting, completion,
and failure states.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Author: elkaix

.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 {