fix(completion): harden auto-refresh paths and add coverage

docs/api-reference.md

@@ -248,7 +248,7 @@ const extracted = extractFields(schema);
 
 ### `withCompletionCommand`
 
-Wraps a command with shell completion support. Adds both a `completion` subcommand and a hidden `__complete` command for dynamic completion.
+Wraps a command with shell completion support. Adds a `completion` subcommand, a hidden `__complete` command for dynamic completion, and a hidden `__refresh-completion` command used by the on-disk cache auto-refresh path. See [Shell Completion](./shell-completion.md#auto-refresh) for details on the refresh flow and the `POLITTY_NO_COMPLETION_REFRESH` opt-out.
 
 ```typescript
 function withCompletionCommand<T extends AnyCommand>(
@@ -266,9 +266,12 @@ function withCompletionCommand<T extends AnyCommand>(
 
 **WithCompletionOptions:**
 
-| Property      | Type      | Description                                      |
-| ------------- | --------- | ------------------------------------------------ |
-| `programName` | `string?` | Override program name (defaults to command.name) |
+| Property           | Type          | Description                                                                                                           |
+| ------------------ | ------------- | --------------------------------------------------------------------------------------------------------------------- |
+| `programName`      | `string?`     | Override program name (defaults to command.name)                                                                      |
+| `globalArgsSchema` | `ArgsSchema?` | Global args schema for deriving global options in completion                                                          |
+| `cacheDir`         | `string?`     | Hardcode the on-disk cache directory used by the rc loader and the runMain background refresh (overrides XDG default) |
+| `programVersion`   | `string?`     | Program version embedded in the script header                                                                         |
 
 #### Example
 
@@ -285,8 +288,9 @@ const mainCommand = withCompletionCommand(
 );
 
 // Now includes:
-// - mycli completion bash|zsh|fish
+// - mycli completion bash|zsh|fish [--install] [--loader] [--instructions]
 // - mycli __complete -- <args>
+// - mycli __refresh-completion <shell>  (hidden; spawned by the rc loader / runMain hook)
 
 runMain(mainCommand);
 ```
@@ -310,11 +314,16 @@ function generateCompletion(command: AnyCommand, options: CompletionOptions): Co
 
 **CompletionOptions:**
 
-| Property              | Type        | Description                            |
-| --------------------- | ----------- | -------------------------------------- |
-| `shell`               | `ShellType` | Target shell: "bash", "zsh", or "fish" |
-| `programName`         | `string`    | Program name as invoked                |
-| `includeDescriptions` | `boolean?`  | Include descriptions (default: true)   |
+| Property              | Type          | Description                                                                                            |
+| --------------------- | ------------- | ------------------------------------------------------------------------------------------------------ |
+| `shell`               | `ShellType`   | Target shell: "bash", "zsh", or "fish"                                                                 |
+| `programName`         | `string`      | Program name as invoked                                                                                |
+| `includeSubcommands`  | `boolean?`    | Include subcommand completions (default: true)                                                         |
+| `includeDescriptions` | `boolean?`    | Include descriptions (default: true)                                                                   |
+| `globalArgsSchema`    | `ArgsSchema?` | Global args schema for deriving global options in completion                                           |
+| `binPath`             | `string?`     | Path to the binary whose mtime is the freshness signature (defaults to `process.argv[1]`)              |
+| `programVersion`      | `string?`     | Program version embedded in the script header                                                          |
+| `cacheDir`            | `string?`     | Cache directory hardcoded into the generated rc loader (defaults to XDG cache dir resolved at runtime) |
 
 #### Return Value
 

src/completion/fish.ts

@@ -13,7 +13,7 @@ import {
   getVisibleSubs,
   sanitize,
 } from "./extractor.js";
-import { buildHeaderLines, computeBinSig } from "./header.js";
+import { buildHeaderLines, computeBinSig, resolveBinPath } from "./header.js";
 import type {
   CompletableOption,
   CompletablePositional,
@@ -271,24 +271,44 @@ export function generateFishCompletion(
   // from `$__fish_config_dir/completions/<prog>.fish` lazily, so the
   // refresh check has to live in the file itself. When the binary's
   // mtime no longer matches the embedded sig, we regenerate the file
-  // in place and let the next autoload pick up the new content.
+  // in place via the hidden __refresh-completion subcommand, then
+  // `source` the rewritten file so the *current* session picks up the
+  // new definitions, and `return` from this script so the rest of the
+  // *old* file (stale helper functions and `complete` registrations)
+  // doesn't run on top of the freshly sourced new definitions.
   // Failures are silent — a stale completion is preferable to a
   // shell-startup error.
-  const sig = computeBinSig(options.binPath ?? process.argv[1] ?? "");
+  //
+  // We invoke __refresh-completion (internal) instead of
+  // `<bin> completion fish`: the foreground completion command runs
+  // user setup/cleanup/prompt and validates required globalArgs, which
+  // can fail or block when triggered from autoload.
+  const sig = computeBinSig(resolveBinPath(programName, options.binPath));
   const refreshFn = `__${fn}_refresh_completion`;
   lines.push(`function ${refreshFn} --no-scope-shadowing`);
   lines.push(`    set -l _bin (command -v ${programName})`);
-  lines.push(`    test -z "$_bin"; and return`);
+  lines.push(`    test -z "$_bin"; and return 1`);
+  // `-L` follows symlinks so the shell-side mtime matches Node's
+  // `fs.statSync`, mirroring the bash/zsh loader. Probe order matches
+  // the bash/zsh loader: GNU (`-c`) first because `-f` is filesystem
+  // mode there and would otherwise dump filesystem info into `_sig`.
   lines.push(
-    `    set -l _sig (stat -f '%m' "$_bin" 2>/dev/null; or stat -c '%Y' "$_bin" 2>/dev/null)`,
+    `    set -l _sig (stat -L -c '%Y' "$_bin" 2>/dev/null; or stat -L -f '%m' "$_bin" 2>/dev/null)`,
   );
-  lines.push(`    test "$_sig" = "${sig}"; and return`);
+  lines.push(`    test "$_sig" = "${sig}"; and return 1`);
   lines.push(`    set -l _target "$__fish_config_dir/completions/${programName}.fish"`);
-  lines.push(`    "$_bin" completion fish > "$_target.tmp" 2>/dev/null`);
-  lines.push(`    and mv "$_target.tmp" "$_target"`);
+  lines.push(`    "$_bin" __refresh-completion fish 2>/dev/null`);
+  lines.push(`    and source "$_target" 2>/dev/null`);
+  lines.push(`    and return 0`);
+  lines.push(`    return 1`);
   lines.push(`end`);
   lines.push(`${refreshFn}`);
+  lines.push(`set -l _politty_refreshed $status`);
   lines.push(`functions -e ${refreshFn}`);
+  // `return` from a sourced fish script aborts the rest of the source
+  // call, so the stale `complete -c` lines below do not execute when
+  // the fresh script has already been sourced.
+  lines.push(`test $_politty_refreshed -eq 0; and return`);
   lines.push(``);
 
   // Helper: check if option is already used

src/completion/header.ts

@@ -8,6 +8,7 @@
  */
 
 import { statSync } from "node:fs";
+import { join } from "node:path";
 import type { ShellType } from "./types.js";
 
 /** Schema version of the header itself. Bump when the header layout changes. */
@@ -26,6 +27,43 @@ export function computeBinSig(binPath: string): string {
   }
 }
 
+/**
+ * Walk `$PATH` looking for an executable named `programName`. Returns
+ * the first match's full path, or `null` when not found. We mirror the
+ * shell's `command -v <prog>` here so the sig embedded in the header
+ * (computed by Node) lines up with what the rc loader stat-checks at
+ * runtime — including pnpm/npm bin shims that wrap the real entrypoint.
+ * Without this alignment, shimmed installs would never match the
+ * embedded sig and the cache would regenerate on every shell startup.
+ */
+function findOnPath(programName: string): string | null {
+  // eslint-disable-next-line no-control-regex -- intentional null-byte rejection to block path injection in `programName`
+  if (!programName || /[/\\\0]/.test(programName)) return null;
+  const path = process.env.PATH ?? "";
+  for (const dir of path.split(":")) {
+    if (!dir) continue;
+    const candidate = join(dir, programName);
+    try {
+      if (statSync(candidate).isFile()) return candidate;
+    } catch {
+      // skip
+    }
+  }
+  return null;
+}
+
+/**
+ * Resolve the binary path used for sig computation and stat checks.
+ *
+ * Order: explicit override → `$PATH` lookup of `programName` → `process.argv[1]`.
+ * The `$PATH` lookup keeps Node-side and shell-side stats pointed at the
+ * same shim file when the CLI is invoked through a package-manager bin shim.
+ */
+export function resolveBinPath(programName: string, override?: string | undefined): string {
+  if (override) return override;
+  return findOnPath(programName) ?? process.argv[1] ?? "";
+}
+
 export interface HeaderOptions {
   programName: string;
   shell: ShellType;
@@ -39,8 +77,7 @@ export interface HeaderOptions {
  * marker.
  */
 export function buildHeaderLines(opts: HeaderOptions): string[] {
-  const binPath = opts.binPath ?? process.argv[1] ?? "";
-  const sig = binPath ? computeBinSig(binPath) : "0";
+  const sig = computeBinSig(resolveBinPath(opts.programName, opts.binPath));
   const lines = [
     `# politty-completion-version: ${COMPLETION_VERSION}`,
     `# politty-bin-sig: ${sig}`,

src/completion/index.ts

@@ -32,7 +32,7 @@ import { generateBashCompletion } from "./bash.js";
 import { createDynamicCompleteCommand } from "./dynamic/index.js";
 import { generateFishCompletion } from "./fish.js";
 import {
-  detectShellEnv,
+  hasManagedCache,
   install as installCompletion,
   refreshIfStale,
   spawnBackgroundRefresh,
@@ -182,12 +182,49 @@ export function createCompletionCommand(
   const resolvedProgramName = programName ?? rootCommand.name;
   const { cacheDir, programVersion } = extra;
 
+  // Build the option fragments once. Under exactOptionalPropertyTypes
+  // we can't pass `undefined` values directly, so we omit absent keys.
+  const refreshExtra: {
+    cacheDir?: string;
+    programVersion?: string;
+    globalArgsSchema?: ArgsSchema;
+  } = {
+    ...(cacheDir !== undefined && { cacheDir }),
+    ...(programVersion !== undefined && { programVersion }),
+    ...(globalArgsSchema !== undefined && { globalArgsSchema }),
+  };
+  const installCtxBase: Omit<Parameters<typeof installCompletion>[0], "rootCommand"> = {
+    programName: resolvedProgramName,
+    ...refreshExtra,
+  };
+  const loaderOptsBase = {
+    programName: resolvedProgramName,
+    ...(cacheDir !== undefined && { cacheDir }),
+  };
+
   if (!rootCommand.subCommands?.__complete) {
     rootCommand.subCommands = {
       ...rootCommand.subCommands,
       __complete: createDynamicCompleteCommand(rootCommand, resolvedProgramName),
     };
   }
+  // Register `__refresh-completion` here too so callers using
+  // `createCompletionCommand` directly (rather than
+  // `withCompletionCommand`) still expose the subcommand the generated
+  // rc loaders / fish autoload expect to invoke after the binary's
+  // mtime changes. Without it, the loaders would call an unknown
+  // subcommand with stderr swallowed and silently keep sourcing the
+  // stale cache.
+  if (!rootCommand.subCommands?.["__refresh-completion"]) {
+    rootCommand.subCommands = {
+      ...rootCommand.subCommands,
+      "__refresh-completion": createRefreshCompletionCommand(
+        rootCommand,
+        resolvedProgramName,
+        refreshExtra,
+      ),
+    };
+  }
 
   return defineCommand({
     name: "completion",
@@ -204,54 +241,33 @@ export function createCompletionCommand(
       }
 
       if (args.install) {
+        let target: string;
         try {
-          const target = installCompletion(
-            {
-              rootCommand,
-              programName: resolvedProgramName,
-              ...(programVersion !== undefined && { programVersion }),
-              ...(cacheDir !== undefined && { cacheDir }),
-              ...(globalArgsSchema !== undefined && { globalArgsSchema }),
-            },
-            shellType,
-          );
-          console.error(`installed: ${target}`);
-          if (shellType !== "fish") {
-            console.error("");
-            console.error(`Add to your ~/.${shellType}rc:`);
-            console.error("");
-            console.error(
-              generateLoader({
-                programName: resolvedProgramName,
-                shell: shellType,
-                ...(cacheDir !== undefined && { cacheDir }),
-              })
-                .trim()
-                .replace(/^/gm, "    "),
-            );
-          }
+          target = installCompletion({ rootCommand, ...installCtxBase }, shellType);
         } catch (e) {
-          console.error(`install failed: ${e instanceof Error ? e.message : String(e)}`);
-          process.exitCode = 1;
+          throw new Error(`install failed: ${e instanceof Error ? e.message : String(e)}`);
+        }
+        console.error(`installed: ${target}`);
+        if (shellType !== "fish") {
+          console.error("");
+          console.error(`Add to your ~/.${shellType}rc:`);
+          console.error("");
+          console.error(
+            generateLoader({ ...loaderOptsBase, shell: shellType })
+              .trim()
+              .replace(/^/gm, "    "),
+          );
         }
         return;
       }
 
       if (args.loader) {
         if (shellType === "fish") {
-          console.error(
+          throw new Error(
             "fish does not use an rc loader. Run `<program> completion fish --install` to write the self-refreshing autoload file instead.",
           );
-          process.exitCode = 1;
-          return;
         }
-        process.stdout.write(
-          generateLoader({
-            programName: resolvedProgramName,
-            shell: shellType,
-            ...(cacheDir !== undefined && { cacheDir }),
-          }),
-        );
+        process.stdout.write(generateLoader({ ...loaderOptsBase, shell: shellType }));
         return;
       }
 
@@ -289,16 +305,7 @@ export function createRefreshCompletionCommand(
     description: "(internal) Refresh the on-disk completion cache if stale.",
     args: refreshArgsSchema,
     run(args) {
-      refreshIfStale(
-        {
-          rootCommand,
-          programName,
-          ...(extra.programVersion !== undefined && { programVersion: extra.programVersion }),
-          ...(extra.cacheDir !== undefined && { cacheDir: extra.cacheDir }),
-          ...(extra.globalArgsSchema !== undefined && { globalArgsSchema: extra.globalArgsSchema }),
-        },
-        args.shell,
-      );
+      refreshIfStale({ rootCommand, programName, ...extra }, args.shell);
     },
   });
 }
@@ -352,10 +359,11 @@ export function withCompletionCommand<T extends AnyCommand>(
 
   const { programName, globalArgsSchema, cacheDir, programVersion } = opts;
   const resolvedProgramName = programName ?? command.name;
-  const extra: { cacheDir?: string; programVersion?: string; globalArgsSchema?: ArgsSchema } = {};
-  if (cacheDir !== undefined) extra.cacheDir = cacheDir;
-  if (programVersion !== undefined) extra.programVersion = programVersion;
-  if (globalArgsSchema !== undefined) extra.globalArgsSchema = globalArgsSchema;
+  const extra: { cacheDir?: string; programVersion?: string; globalArgsSchema?: ArgsSchema } = {
+    ...(cacheDir !== undefined && { cacheDir }),
+    ...(programVersion !== undefined && { programVersion }),
+    ...(globalArgsSchema !== undefined && { globalArgsSchema }),
+  };
 
   const wrappedCommand = {
     ...command,
@@ -375,7 +383,10 @@ export function withCompletionCommand<T extends AnyCommand>(
   };
 
   wrappedCommand.runMainHook = (argv) => {
-    maybeSpawnRefresh(argv);
+    maybeSpawnRefresh(argv, {
+      programName: resolvedProgramName,
+      ...(cacheDir !== undefined && { cacheDir }),
+    });
   };
 
   return wrappedCommand;
@@ -390,8 +401,15 @@ export function withCompletionCommand<T extends AnyCommand>(
  *   - $SHELL doesn't resolve to a known shell
  *   - the user opted out via $POLITTY_NO_COMPLETION_REFRESH
  *   - process.argv[1] is missing (shouldn't happen for normal CLIs)
+ *   - no politty-managed cache exists yet — i.e. the user hasn't
+ *     installed completion. Without this gate the detached child would
+ *     create a fish autoload (or any cache file) on every CLI run,
+ *     even though the user never opted in via `--install` or the rc loader.
  */
-function maybeSpawnRefresh(argv: readonly string[]): void {
+function maybeSpawnRefresh(
+  argv: readonly string[],
+  ctx: { programName: string; cacheDir?: string | undefined },
+): void {
   if (process.env.POLITTY_NO_COMPLETION_REFRESH) return;
 
   const firstPositional = argv.find((a) => !a.startsWith("-"));
@@ -403,10 +421,11 @@ function maybeSpawnRefresh(argv: readonly string[]): void {
     return;
   }
 
-  const shell = detectShellEnv();
+  const shell = detectShell();
   if (!shell) return;
   const argv0 = process.argv[1];
   if (!argv0) return;
+  if (!hasManagedCache(ctx, shell)) return;
 
   spawnBackgroundRefresh(argv0, shell);
 }