diff --git a/docs/api-reference.md b/docs/api-reference.md index 03111b07..31c3afc6 100644 --- a/docs/api-reference.md +++ b/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( @@ -266,9 +266,12 @@ function withCompletionCommand( **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 -- +// - mycli __refresh-completion (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 diff --git a/src/completion/fish.ts b/src/completion/fish.ts index 990bf991..53f21fe4 100644 --- a/src/completion/fish.ts +++ b/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/.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 + // ` 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 diff --git a/src/completion/header.ts b/src/completion/header.ts index ce2cdaf0..37053b86 100644 --- a/src/completion/header.ts +++ b/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 ` 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}`, diff --git a/src/completion/index.ts b/src/completion/index.ts index 69c028af..9b5b82e3 100644 --- a/src/completion/index.ts +++ b/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[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 ` 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( 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( }; wrappedCommand.runMainHook = (argv) => { - maybeSpawnRefresh(argv); + maybeSpawnRefresh(argv, { + programName: resolvedProgramName, + ...(cacheDir !== undefined && { cacheDir }), + }); }; return wrappedCommand; @@ -390,8 +401,15 @@ export function withCompletionCommand( * - $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); } diff --git a/src/completion/install.ts b/src/completion/install.ts index 461e75f5..ff34ed9f 100644 --- a/src/completion/install.ts +++ b/src/completion/install.ts @@ -15,7 +15,7 @@ import { spawn } from "node:child_process"; import { existsSync, mkdirSync, readFileSync, renameSync, statSync, writeFileSync } from "node:fs"; import { dirname, join } from "node:path"; import type { AnyCommand, ArgsSchema } from "../types.js"; -import { computeBinSig } from "./header.js"; +import { resolveBinPath } from "./header.js"; import { generateCompletion } from "./index.js"; import { defaultCacheDir } from "./loader.js"; import type { ShellType } from "./types.js"; @@ -91,16 +91,21 @@ function readCachedSig(path: string): string | null { /** * Rewrite the cache only when stale. Used by: - * - ` __refresh-completion ` (the hidden subcommand) - * - the background spawn from runMain + * - ` __refresh-completion ` (the hidden subcommand + * spawned both by the rc loader and by the runMain background hook) * - * Both call paths must never throw — a stale completion is fine, a - * crash isn't. + * Caller is responsible for gating: the runMain hook (`maybeSpawnRefresh`) + * checks `hasManagedCache` before spawning so we don't silently create + * a fish autoload the user never opted into. The rc loader / fish + * autoload only run after the user has installed completion in the + * first place, so they're allowed to refresh unconditionally. + * + * Must never throw — a stale completion is fine, a crash isn't. */ export function refreshIfStale(ctx: InstallContext, shell: ShellType): void { try { const target = installPath(ctx.programName, shell, ctx.cacheDir); - const binPath = ctx.binPath ?? process.argv[1] ?? ""; + const binPath = resolveBinPath(ctx.programName, ctx.binPath); if (!binPath) return; let currentSig: string; try { @@ -116,15 +121,19 @@ export function refreshIfStale(ctx: InstallContext, shell: ShellType): void { } /** - * Detect the user's shell from $SHELL. Returns null if it isn't one of - * the supported shells; callers should treat that as "skip refresh." + * Returns true when a politty-managed cache file already exists on disk + * for the given shell — i.e. the user has installed completion via + * ` completion --install` or the rc loader has already + * sourced one. Used by the runMain background hook to avoid spawning + * the refresher (and thereby silently creating files) on plain CLI runs + * the user never opted into. */ -export function detectShellEnv(): ShellType | null { - const shell = (process.env.SHELL ?? "").split("/").pop()?.toLowerCase() ?? ""; - if (shell.includes("bash")) return "bash"; - if (shell.includes("zsh")) return "zsh"; - if (shell.includes("fish")) return "fish"; - return null; +export function hasManagedCache( + ctx: { programName: string; cacheDir?: string | undefined }, + shell: ShellType, +): boolean { + const target = installPath(ctx.programName, shell, ctx.cacheDir); + return readCachedSig(target) !== null; } /** @@ -135,7 +144,7 @@ export function detectShellEnv(): ShellType | null { * Caller is expected to gate this on the right conditions (interactive * shell, not running inside `__complete` itself, etc.). * - * Re-exports `void` and never throws — even spawn failures are absorbed. + * Returns `void` and never throws — even spawn failures are absorbed. */ export function spawnBackgroundRefresh(programArgv0: string, shell: ShellType): void { try { @@ -149,6 +158,3 @@ export function spawnBackgroundRefresh(programArgv0: string, shell: ShellType): // Best-effort. } } - -/** computeBinSig is re-exported so install.ts is self-contained for callers. */ -export { computeBinSig }; diff --git a/src/completion/loader.ts b/src/completion/loader.ts index 26bcafad..78dab8c4 100644 --- a/src/completion/loader.ts +++ b/src/completion/loader.ts @@ -29,14 +29,24 @@ export interface LoaderOptions { cacheDir?: string; } +/** + * Single-quote escape: `'` -> `'\''`. Inside single quotes the shell + * performs no expansion at all, so `$`, backticks, and `$(...)` are + * inert. Used for hardcoded paths because callers may sources them + * from env / config — we must not let metachars in the path execute as + * commands when the rc snippet is sourced. + */ +function shSingleQuote(s: string): string { + return `'${s.replace(/'/g, "'\\''")}'`; +} + function bashCachePathExpr( programName: string, cacheDir: string | undefined, shell: "bash" | "zsh", ): string { if (cacheDir) { - // Hardcoded — quote-once so user-supplied paths don't word-split. - return `"${cacheDir.replace(/"/g, '\\"')}/completion.${shell}"`; + return shSingleQuote(`${cacheDir}/completion.${shell}`); } return `"\${XDG_CACHE_HOME:-$HOME/.cache}/${programName}/completion.${shell}"`; } @@ -44,18 +54,41 @@ function bashCachePathExpr( function generateBashLoader(opts: LoaderOptions): string { const fn = sanitize(opts.programName); const cache = bashCachePathExpr(opts.programName, opts.cacheDir, "bash"); + // `type -P` is path-only — it skips aliases, functions, and builtins. + // `command -v` would surface the alias text or function name when the + // user has shadowed the CLI in their rc, and the subsequent `stat` + // would fail, leaving completions unsourced for that shell. + // + // `-L` follows symlinks so the shell-side mtime matches Node's + // `fs.statSync`, which also follows symlinks. Without `-L`, BSD `stat` + // (macOS) reports the symlink's own mtime, so package-manager bin + // shims (npm/pnpm/etc.) would never match the embedded sig and the + // cache would regenerate on every shell startup. + // + // Try GNU (`-c '%Y'`) before BSD (`-f '%m'`): GNU treats `-f` as + // file-system mode, so `stat -L -f '%m' file` would emit filesystem + // info on stdout *and* exit non-zero, causing the `||` fallback to + // append a second line and break the header comparison. return `__${fn}_load_completion() { local _bin _cache _sig _hdr - _bin=$(command -v ${opts.programName}) || return 0 + _bin=$(type -P ${opts.programName} 2>/dev/null) + [[ -n "$_bin" ]] || return 0 _cache=${cache} - _sig=$(stat -f '%m' "$_bin" 2>/dev/null || stat -c '%Y' "$_bin" 2>/dev/null) || return 0 + _sig=$(stat -L -c '%Y' "$_bin" 2>/dev/null || stat -L -f '%m' "$_bin" 2>/dev/null) || return 0 _hdr="# politty-bin-sig: $_sig" if [[ ! -f "$_cache" ]] || ! head -5 "$_cache" 2>/dev/null | grep -qF "$_hdr"; then - mkdir -p "$(dirname "$_cache")" 2>/dev/null || return 0 - "$_bin" completion bash > "$_cache.tmp.$$" 2>/dev/null \\ - && mv "$_cache.tmp.$$" "$_cache" \\ - || { rm -f "$_cache.tmp.$$" 2>/dev/null; return 0; } + # Use the hidden __refresh-completion subcommand instead of + # \`$_bin completion bash\`: the foreground completion command + # is subject to user setup/cleanup/prompt and required + # globalArgs validation, which can silently fail or block when + # invoked from rc; runMain bypasses those for __-prefixed + # internal subcommands. + "$_bin" __refresh-completion bash 2>/dev/null fi + # If regen failed but a stale cache survived from a previous run, + # source it anyway — a stale completion is preferable to no + # completion at all. + [[ -f "$_cache" ]] || return 0 # shellcheck disable=SC1090 source "$_cache" } @@ -67,20 +100,25 @@ unset -f __${fn}_load_completion function generateZshLoader(opts: LoaderOptions): string { const fn = sanitize(opts.programName); const cache = bashCachePathExpr(opts.programName, opts.cacheDir, "zsh"); + // `whence -p` is the zsh equivalent of bash's `type -P` — path-only, + // ignoring aliases / functions / builtins. See bash loader for the + // rationale and for `-L` / stat probe-order. return `__${fn}_load_completion() { emulate -L zsh setopt local_options no_aliases local _bin _cache _sig _hdr - _bin=$(command -v ${opts.programName}) || return 0 + _bin=$(whence -p ${opts.programName} 2>/dev/null) + [[ -n "$_bin" ]] || return 0 _cache=${cache} - _sig=$(stat -f '%m' "$_bin" 2>/dev/null || stat -c '%Y' "$_bin" 2>/dev/null) || return 0 + _sig=$(stat -L -c '%Y' "$_bin" 2>/dev/null || stat -L -f '%m' "$_bin" 2>/dev/null) || return 0 _hdr="# politty-bin-sig: $_sig" if [[ ! -f "$_cache" ]] || ! head -5 "$_cache" 2>/dev/null | grep -qF "$_hdr"; then - mkdir -p "$_cache:h" 2>/dev/null || return 0 - "$_bin" completion zsh > "$_cache.tmp.$$" 2>/dev/null \\ - && mv "$_cache.tmp.$$" "$_cache" \\ - || { rm -f "$_cache.tmp.$$" 2>/dev/null; return 0; } + # See bash loader for why we use __refresh-completion instead + # of \`$_bin completion zsh\`. + "$_bin" __refresh-completion zsh 2>/dev/null fi + # See bash loader: keep stale completion over no completion. + [[ -f "$_cache" ]] || return 0 source "$_cache" } __${fn}_load_completion @@ -90,7 +128,7 @@ unfunction __${fn}_load_completion /** * Build the rc-loader snippet for bash or zsh. Fish doesn't have an - * rc-loader; instead, ` completion install fish` writes a + * rc-loader; instead, ` completion fish --install` writes a * self-rewriting autoload file. */ export function generateLoader(opts: LoaderOptions): string { @@ -101,16 +139,16 @@ export function generateLoader(opts: LoaderOptions): string { return generateZshLoader(opts); case "fish": throw new Error( - "fish does not use an rc loader. Run ` completion install fish` to write the self-refreshing autoload file instead.", + "fish does not use an rc loader. Run ` completion fish --install` to write the self-refreshing autoload file instead.", ); } } /** - * Default cache file path (used by `completion install bash|zsh` and - * the `__refresh-completion` subcommand). For fish, the install path - * is `$__fish_config_dir/completions/.fish` and is computed - * inside `installPath()` instead. + * Default cache file path (used by `completion --install` + * and the `__refresh-completion` subcommand). For fish, the install + * path is `$__fish_config_dir/completions/.fish` and is + * computed inside `installPath()` instead. */ export function defaultCacheDir(programName: string): string { const xdg = process.env.XDG_CACHE_HOME ?? `${process.env.HOME ?? ""}/.cache`; diff --git a/src/core/run-main.test.ts b/src/core/run-main.test.ts index d11444f6..5f294bad 100644 --- a/src/core/run-main.test.ts +++ b/src/core/run-main.test.ts @@ -558,3 +558,152 @@ describe("runMain displayErrors", () => { process.argv = originalArgv; }); }); + +describe("runMain internal subcommand bypass", () => { + const originalArgv = process.argv; + + it("skips user setup/cleanup/prompt for `__`-prefixed registered subcommands", async () => { + process.argv = ["node", "test", "__internal"]; + const exitSpy = vi.spyOn(process, "exit").mockImplementation(() => undefined as never); + + const setup = vi.fn(); + const cleanup = vi.fn(); + const prompt = vi.fn(); + let internalRan = false; + + const internal = defineCommand({ + name: "__internal", + run: () => { + internalRan = true; + }, + }); + + const cmd = defineCommand({ + name: "test", + run: () => {}, + subCommands: { __internal: internal }, + }); + + await runMain(cmd, { setup, cleanup, prompt }); + + expect(internalRan).toBe(true); + expect(setup).not.toHaveBeenCalled(); + expect(cleanup).not.toHaveBeenCalled(); + expect(prompt).not.toHaveBeenCalled(); + expect(exitSpy).toHaveBeenCalledWith(0); + + exitSpy.mockRestore(); + process.argv = originalArgv; + }); + + it("still runs user setup/cleanup for ordinary subcommands", async () => { + process.argv = ["node", "test", "regular"]; + const exitSpy = vi.spyOn(process, "exit").mockImplementation(() => undefined as never); + + const setup = vi.fn(); + const cleanup = vi.fn(); + + const regular = defineCommand({ name: "regular", run: () => {} }); + const cmd = defineCommand({ + name: "test", + run: () => {}, + subCommands: { regular }, + }); + + await runMain(cmd, { setup, cleanup }); + + expect(setup).toHaveBeenCalled(); + expect(cleanup).toHaveBeenCalled(); + expect(exitSpy).toHaveBeenCalledWith(0); + + exitSpy.mockRestore(); + process.argv = originalArgv; + }); + + it("does not bypass when an unregistered `__`-prefixed positional is passed", async () => { + // Defense in depth: we only bypass for subcommands that are actually + // registered, so a stray `__foo` argument doesn't accidentally skip + // user lifecycle. + process.argv = ["node", "test", "__not-registered"]; + const exitSpy = vi.spyOn(process, "exit").mockImplementation(() => undefined as never); + + const setup = vi.fn(); + const cmd = defineCommand({ name: "test", run: () => {} }); + + await runMain(cmd, { setup }); + + expect(setup).toHaveBeenCalled(); + + exitSpy.mockRestore(); + process.argv = originalArgv; + }); + + it("does not bypass when `__name` appears as a global option *value*", async () => { + // `--name __internal` is a value for --name, not the subcommand + // token. Without schema-aware scanning, the naive + // "first non-flag token" check would mistakenly bypass lifecycle + // for ordinary invocations whose option values happen to start + // with `__`. + process.argv = ["node", "test", "--name", "__internal"]; + const exitSpy = vi.spyOn(process, "exit").mockImplementation(() => undefined as never); + + const setup = vi.fn(); + const internal = defineCommand({ name: "__internal", run: () => {} }); + const cmd = defineCommand({ + name: "test", + run: () => {}, + subCommands: { __internal: internal }, + }); + + await runMain(cmd, { + setup, + globalArgs: z.object({ name: arg(z.string().optional(), {}) }), + }); + + expect(setup).toHaveBeenCalled(); + + exitSpy.mockRestore(); + process.argv = originalArgv; + }); +}); + +describe("runMain runMainHook", () => { + const originalArgv = process.argv; + + it("invokes the hook once with the parsed argv before any command execution", async () => { + process.argv = ["node", "test", "--flag", "value"]; + const exitSpy = vi.spyOn(process, "exit").mockImplementation(() => undefined as never); + const hook = vi.fn(); + + const cmd = defineCommand({ name: "test", run: () => {} }); + cmd.runMainHook = hook; + + await runMain(cmd); + + expect(hook).toHaveBeenCalledTimes(1); + expect(hook).toHaveBeenCalledWith(["--flag", "value"]); + + exitSpy.mockRestore(); + process.argv = originalArgv; + }); + + it("swallows hook errors so a misbehaving hook never blocks the CLI", async () => { + process.argv = ["node", "test"]; + const exitSpy = vi.spyOn(process, "exit").mockImplementation(() => undefined as never); + const runFn = vi.fn(); + + const cmd = defineCommand({ name: "test", run: runFn }); + cmd.runMainHook = () => { + throw new Error("hook blew up"); + }; + + await runMain(cmd); + + // The user command must still run despite the hook throwing. + expect(runFn).toHaveBeenCalled(); + expect(exitSpy).toHaveBeenCalledWith(0); + + exitSpy.mockRestore(); + process.argv = originalArgv; + }); +}); diff --git a/src/core/runner.ts b/src/core/runner.ts index 25360861..57e51673 100644 --- a/src/core/runner.ts +++ b/src/core/runner.ts @@ -155,6 +155,30 @@ export async function runCommand( return result; } +/** + * Hidden internal subcommands (e.g. `__refresh-completion`) are spawned + * by background hooks and must not run user-provided + * `setup`/`cleanup`/`prompt` or required `globalArgs`. Those exist for + * the foreground CLI run; replaying them in a detached child causes + * duplicate side effects, stuck prompts, and validation failures the + * user never opted into. + * + * We treat any registered subcommand whose name starts with `__` as + * internal. We use `findFirstPositional` (schema-aware) instead of the + * naive "first non-flag token" so an option *value* like + * `--name __refresh-completion` doesn't trip the bypass — that would + * silently skip lifecycle hooks for ordinary invocations. + */ +function isInternalSubcommandInvocation( + command: AnyCommand, + argv: string[], + globalExtracted?: ExtractedFields, +): boolean { + const firstPositional = findFirstPositional(argv, globalExtracted); + if (!firstPositional || !firstPositional.startsWith("__")) return false; + return Boolean(command.subCommands?.[firstPositional]); +} + /** * Run a CLI command as the main entry point * @@ -162,6 +186,11 @@ export async function runCommand( * - Uses process.argv for arguments * - Handles SIGINT/SIGTERM signals * - Calls process.exit with the appropriate exit code + * - Invokes `command.runMainHook` once before parsing if set, so plug-ins + * like `withCompletionCommand` can fire detached background work + * - Bypasses user `setup`/`cleanup`/`prompt` and required `globalArgs` + * for registered hidden subcommands whose name starts with `__` + * (e.g. `__refresh-completion`) * * @param command - The command to run * @param options - Main options (version, debug) @@ -190,17 +219,45 @@ export async function runMain(command: AnyCommand, options: MainOptions = {}): P } } - const globalExtracted = extractAndValidateGlobal(options); + const argv = process.argv.slice(2); + // Extract the global schema once *before* the bypass check so + // `findFirstPositional` can correctly skip option values. We re-use + // the same `globalExtracted` for the actual run when the call is + // foreground. + let globalExtractedForBypass: ExtractedFields | undefined; + if (options.globalArgs) { + try { + globalExtractedForBypass = extractFields(options.globalArgs); + } catch { + // If the schema is malformed we'll error later; for the bypass + // check fall back to the no-schema scan (conservative — option + // values may be misclassified, but that only over-bypasses the + // detection, never under-bypasses it for ordinary invocations). + } + } + // For internal subcommands, drop user lifecycle hooks and the + // globalArgs schema. The internal command implements its own + // best-effort behavior and should not be subject to user policies. + // Note: under exactOptionalPropertyTypes we must omit the keys (not + // assign undefined), since `globalArgs?: ArgsSchema` does not accept + // `undefined` as a value. + let effectiveOptions: MainOptions = options; + if (isInternalSubcommandInvocation(command, argv, globalExtractedForBypass)) { + const { setup: _s, cleanup: _c, prompt: _p, globalArgs: _g, ...rest } = options; + effectiveOptions = rest; + } + + const globalExtracted = extractAndValidateGlobal(effectiveOptions); // Global setup - if (options.setup) { + if (effectiveOptions.setup) { try { - await options.setup({}); + await effectiveOptions.setup({}); } catch (e) { const error = e instanceof Error ? e : new Error(String(e)); - if (options.cleanup) { + if (effectiveOptions.cleanup) { try { - await options.cleanup({ error }); + await effectiveOptions.cleanup({ error }); } catch { // Swallow cleanup error when setup already failed } @@ -209,37 +266,37 @@ export async function runMain(command: AnyCommand, options: MainOptions = {}): P } } - const result = await runCommandInternal(command, process.argv.slice(2), { - debug: options.debug, - captureLogs: options.captureLogs, - skipValidation: options.skipValidation, + const result = await runCommandInternal(command, argv, { + debug: effectiveOptions.debug, + captureLogs: effectiveOptions.captureLogs, + skipValidation: effectiveOptions.skipValidation, handleSignals: true, - logger: options.logger, - globalArgs: options.globalArgs, - prompt: options.prompt, + logger: effectiveOptions.logger, + globalArgs: effectiveOptions.globalArgs, + prompt: effectiveOptions.prompt, _globalExtracted: globalExtracted, - _globalCleanup: options.cleanup, + _globalCleanup: effectiveOptions.cleanup, _context: { commandPath: [], rootName: command.name, - rootVersion: options.version, + rootVersion: effectiveOptions.version, globalExtracted, }, }); // Display errors (controlled by displayErrors option, default: true) - if ((options.displayErrors ?? true) && !result.success && result.error) { - const errorLogger = options.logger ?? defaultLogger; - errorLogger.error(formatRuntimeError(result.error, options.debug ?? false)); + if ((effectiveOptions.displayErrors ?? true) && !result.success && result.error) { + const errorLogger = effectiveOptions.logger ?? defaultLogger; + errorLogger.error(formatRuntimeError(result.error, effectiveOptions.debug ?? false)); } // Global cleanup (always) - if (options.cleanup) { + if (effectiveOptions.cleanup) { const cleanupCtx: GlobalCleanupContext = { error: !result.success ? result.error : undefined, }; try { - await options.cleanup(cleanupCtx); + await effectiveOptions.cleanup(cleanupCtx); } catch { // Swallow - we're about to exit anyway } diff --git a/tests/completion.test.ts b/tests/completion.test.ts index 160e5e8f..4f809c95 100644 --- a/tests/completion.test.ts +++ b/tests/completion.test.ts @@ -1,6 +1,7 @@ -import { mkdtempSync, readFileSync, statSync, utimesSync, writeFileSync } from "node:fs"; +import type * as childProcess from "node:child_process"; +import { mkdirSync, mkdtempSync, readFileSync, statSync, utimesSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; -import { join } from "node:path"; +import { dirname, join } from "node:path"; import { afterEach, beforeEach, describe, expect, expectTypeOf, it, vi } from "vitest"; import { z } from "zod"; import { @@ -15,7 +16,12 @@ import { parseCompletionContext, withCompletionCommand, } from "../src/completion/index.js"; -import { install, installPath, refreshIfStale } from "../src/completion/install.js"; +import { + hasManagedCache, + install, + installPath, + refreshIfStale, +} from "../src/completion/install.js"; import { defaultCacheDir, generateLoader } from "../src/completion/loader.js"; import { arg, @@ -26,6 +32,19 @@ import { type CompletionMeta, } from "../src/index.js"; +// Spy on `spawn` so the runMainHook tests below can assert gating without +// actually spawning a child process. We must mock at module level — the +// hook calls the destructured `spawn` import inside src/completion/install.ts, +// so a `vi.spyOn` after the fact would not intercept it. Use `importOriginal` +// to keep every other child_process export intact (e.g. `execSync` which +// dynamic completion candidate generation depends on). +vi.mock("node:child_process", async (importOriginal) => ({ + ...(await importOriginal()), + spawn: vi.fn(() => ({ unref: () => {} })), +})); +const childProcessMock = await import("node:child_process"); +const spawnSpy = vi.mocked(childProcessMock.spawn); + describe("Completion", () => { describe("extractCompletionData", () => { it("should extract options from a simple command", () => { @@ -1588,6 +1607,22 @@ describe("Completion", () => { ), ).not.toThrow(); }); + + it("hasManagedCache returns false when the cache file does not exist", () => { + expect(hasManagedCache({ programName: "mycli", cacheDir }, "bash")).toBe(false); + }); + + it("hasManagedCache returns false for a non-politty cache file", () => { + const target = installPath("mycli", "bash", cacheDir); + mkdirSync(dirname(target), { recursive: true }); + writeFileSync(target, "# user-managed\ncomplete -c mycli\n"); + expect(hasManagedCache({ programName: "mycli", cacheDir }, "bash")).toBe(false); + }); + + it("hasManagedCache returns true once install has run", () => { + install({ rootCommand: cmd, programName: "mycli", cacheDir, binPath: fakeBin }, "bash"); + expect(hasManagedCache({ programName: "mycli", cacheDir }, "bash")).toBe(true); + }); }); describe("generateLoader", () => { @@ -1612,10 +1647,22 @@ describe("Completion", () => { shell: "bash", cacheDir: "/opt/cache", }); - expect(snippet).toContain('"/opt/cache/completion.bash"'); + expect(snippet).toContain("'/opt/cache/completion.bash'"); expect(snippet).not.toContain("XDG_CACHE_HOME"); }); + it("single-quote escapes hardcoded cache paths so shell metachars stay inert", () => { + const snippet = generateLoader({ + programName: "mycli", + shell: "bash", + cacheDir: "/opt/$(rm -rf)/cache's", + }); + // Path appears once, fully single-quoted, with `'` escaped via `'\''`. + expect(snippet).toContain("'/opt/$(rm -rf)/cache'\\''s/completion.bash'"); + // No naked `$(...)` that would run on source. + expect(snippet).not.toContain('"/opt/$(rm -rf)/cache'); + }); + it("throws for fish — fish uses an autoload file instead", () => { expect(() => generateLoader({ programName: "mycli", shell: "fish" })).toThrow( /fish does not use an rc loader/, @@ -1650,4 +1697,229 @@ describe("Completion", () => { expect(defaultCacheDir("mycli")).toBe("/home/alice/.cache/mycli"); }); }); + + describe("fish self-rewriting autoload", () => { + const cmd = defineCommand({ name: "mycli", run: () => {} }); + + it("invokes __refresh-completion (not `completion fish`) and bails out of the stale body on success", () => { + const fakeBin = join(mkdtempSync(join(tmpdir(), "politty-bin-")), "mycli"); + writeFileSync(fakeBin, "#!/bin/sh\nexit 0\n"); + const { script } = generateCompletion(cmd, { + shell: "fish", + programName: "mycli", + binPath: fakeBin, + }); + // Refresh body uses the hidden subcommand so user setup/cleanup/prompt is skipped. + expect(script).toContain('"$_bin" __refresh-completion fish 2>/dev/null'); + // The stale body must be skipped after a successful refresh. + expect(script).toContain("set -l _politty_refreshed $status"); + expect(script).toContain("test $_politty_refreshed -eq 0; and return"); + // GNU stat probed before BSD stat (otherwise BSD stat -f reports filesystem mode). + expect(script).toContain("stat -L -c '%Y'"); + expect(script).toContain("stat -L -f '%m'"); + }); + + it("embeds the resolved bin-sig so the refresh function can early-exit when fresh", () => { + const fakeBin = join(mkdtempSync(join(tmpdir(), "politty-bin-")), "mycli"); + writeFileSync(fakeBin, "#!/bin/sh\n"); + const sig = Math.floor(statSync(fakeBin).mtimeMs / 1000).toString(); + const { script } = generateCompletion(cmd, { + shell: "fish", + programName: "mycli", + binPath: fakeBin, + }); + expect(script).toContain(`test "$_sig" = "${sig}"; and return 1`); + }); + }); + + describe("completion subcommand --install / --loader flags", () => { + let cacheDir: string; + let fakeBin: string; + const cmd = defineCommand({ name: "mycli", run: () => {} }); + + beforeEach(() => { + const root = mkdtempSync(join(tmpdir(), "politty-installflag-")); + cacheDir = join(root, "cache"); + fakeBin = join(root, "mycli"); + writeFileSync(fakeBin, "#!/bin/sh\nexit 0\n"); + }); + + it("--install writes the script to the cache and prints the loader snippet to stderr (bash)", () => { + const subcommand = createCompletionCommand(cmd, "mycli", undefined, { cacheDir }); + const captured: string[] = []; + const errSpy = vi.spyOn(console, "error").mockImplementation((...args: unknown[]) => { + captured.push(args.map(String).join(" ")); + }); + try { + subcommand.run?.({ shell: "bash", instructions: false, install: true, loader: false }); + + const target = join(cacheDir, "completion.bash"); + const written = readFileSync(target, "utf8"); + expect(written).toContain("# politty-bin-sig:"); + expect(written).toContain("_mycli_completions"); + + const stderr = captured.join("\n"); + expect(stderr).toContain(`installed: ${target}`); + expect(stderr).toContain("Add to your ~/.bashrc:"); + expect(stderr).toContain("__mycli_load_completion()"); + } finally { + errSpy.mockRestore(); + } + }); + + it("--install for fish writes the autoload file and does NOT print a loader snippet", () => { + const cfgRoot = mkdtempSync(join(tmpdir(), "politty-fishcfg-")); + const prevXdg = process.env.XDG_CONFIG_HOME; + process.env.XDG_CONFIG_HOME = cfgRoot; + + const subcommand = createCompletionCommand(cmd, "mycli", undefined, { cacheDir }); + const captured: string[] = []; + const errSpy = vi.spyOn(console, "error").mockImplementation((...args: unknown[]) => { + captured.push(args.map(String).join(" ")); + }); + try { + subcommand.run?.({ shell: "fish", instructions: false, install: true, loader: false }); + + const target = join(cfgRoot, "fish", "completions", "mycli.fish"); + expect(readFileSync(target, "utf8")).toContain("# shell: fish"); + const stderr = captured.join("\n"); + expect(stderr).toContain(`installed: ${target}`); + // Fish has no rc-loader story; we must not tell the user to paste anything. + expect(stderr).not.toContain("Add to your ~/"); + } finally { + errSpy.mockRestore(); + if (prevXdg === undefined) delete process.env.XDG_CONFIG_HOME; + else process.env.XDG_CONFIG_HOME = prevXdg; + } + }); + + it("--loader prints just the rc loader to stdout (no script body)", () => { + const subcommand = createCompletionCommand(cmd, "mycli", undefined, { cacheDir }); + const captured: string[] = []; + const writeSpy = vi + .spyOn(process.stdout, "write") + .mockImplementation((chunk: string | Uint8Array): boolean => { + captured.push(typeof chunk === "string" ? chunk : Buffer.from(chunk).toString()); + return true; + }); + try { + subcommand.run?.({ shell: "zsh", instructions: false, install: false, loader: true }); + + const out = captured.join(""); + expect(out).toContain("__mycli_load_completion()"); + expect(out).toContain("emulate -L zsh"); + // Loader must NOT contain the full completion script body. + expect(out).not.toContain("_mycli_completions"); + expect(out).not.toContain("#compdef mycli"); + } finally { + writeSpy.mockRestore(); + } + }); + + it("--loader for fish throws because fish uses an autoload file instead", () => { + const subcommand = createCompletionCommand(cmd, "mycli"); + expect(() => + subcommand.run?.({ shell: "fish", instructions: false, install: false, loader: true }), + ).toThrow(/fish does not use an rc loader/); + }); + }); + + describe("__refresh-completion subcommand registration", () => { + it("withCompletionCommand registers __refresh-completion so the loader can call it", () => { + const wrapped = withCompletionCommand(defineCommand({ name: "mycli", run: () => {} })); + const refresh = wrapped.subCommands?.["__refresh-completion"]; + expect(refresh).toBeDefined(); + if (!refresh || typeof refresh === "function" || isLazyCommand(refresh)) { + throw new Error("expected __refresh-completion to be a registered command object"); + } + expect(refresh.name).toBe("__refresh-completion"); + }); + + it("createCompletionCommand also auto-registers __refresh-completion on the root", () => { + // Without this, host CLIs that wire `completion: createCompletionCommand(...)` directly + // would generate loaders that shell out to a subcommand the CLI never exposed. + const root = defineCommand({ name: "mycli", run: () => {} }); + createCompletionCommand(root, "mycli"); + expect(root.subCommands?.["__refresh-completion"]).toBeDefined(); + }); + }); + + describe("withCompletionCommand runMainHook (background refresh gates)", () => { + let cacheDir: string; + let prevShell: string | undefined; + let prevOptOut: string | undefined; + let wrapped: ReturnType; + + beforeEach(() => { + cacheDir = mkdtempSync(join(tmpdir(), "politty-hook-")); + prevShell = process.env.SHELL; + prevOptOut = process.env.POLITTY_NO_COMPLETION_REFRESH; + process.env.SHELL = "/usr/local/bin/bash"; + delete process.env.POLITTY_NO_COMPLETION_REFRESH; + // Pre-populate a politty-managed cache so `hasManagedCache` returns true, + // letting us isolate the *other* gates one at a time. + const fakeBin = join(cacheDir, "mycli"); + writeFileSync(fakeBin, "#!/bin/sh\n"); + install( + { + rootCommand: defineCommand({ name: "mycli", run: () => {} }), + programName: "mycli", + cacheDir, + binPath: fakeBin, + }, + "bash", + ); + wrapped = withCompletionCommand(defineCommand({ name: "mycli", run: () => {} }), { + programName: "mycli", + cacheDir, + }); + spawnSpy.mockClear(); + }); + + afterEach(() => { + if (prevShell === undefined) delete process.env.SHELL; + else process.env.SHELL = prevShell; + if (prevOptOut === undefined) delete process.env.POLITTY_NO_COMPLETION_REFRESH; + else process.env.POLITTY_NO_COMPLETION_REFRESH = prevOptOut; + }); + + it("spawns a detached refresh child for ordinary CLI invocations", () => { + wrapped.runMainHook?.(["build", "--watch"]); + expect(spawnSpy).toHaveBeenCalledTimes(1); + const [, spawnArgs, opts] = spawnSpy.mock.calls[0]!; + expect(spawnArgs).toContain("__refresh-completion"); + expect(spawnArgs).toContain("bash"); + expect(opts).toMatchObject({ detached: true, stdio: "ignore" }); + }); + + it("opt-out via POLITTY_NO_COMPLETION_REFRESH skips the spawn", () => { + process.env.POLITTY_NO_COMPLETION_REFRESH = "1"; + wrapped.runMainHook?.(["build"]); + expect(spawnSpy).not.toHaveBeenCalled(); + }); + + it("invocations of completion / __complete / __refresh-completion never re-spawn", () => { + wrapped.runMainHook?.(["completion", "bash"]); + wrapped.runMainHook?.(["__complete", "anything"]); + wrapped.runMainHook?.(["__refresh-completion", "bash"]); + expect(spawnSpy).not.toHaveBeenCalled(); + }); + + it("skips the spawn when no managed cache exists yet (avoids creating files the user never opted into)", () => { + // Point at an empty cacheDir so `hasManagedCache` returns false. + const emptyDir = mkdtempSync(join(tmpdir(), "politty-hook-empty-")); + const w = withCompletionCommand(defineCommand({ name: "mycli", run: () => {} }), { + programName: "mycli", + cacheDir: emptyDir, + }); + w.runMainHook?.(["build"]); + expect(spawnSpy).not.toHaveBeenCalled(); + }); + + it("skips the spawn when $SHELL is unrecognized", () => { + process.env.SHELL = "/bin/dash"; + wrapped.runMainHook?.(["build"]); + expect(spawnSpy).not.toHaveBeenCalled(); + }); + }); });