fix(fish): pass multi-segment CLI paths as separate arguments

.changeset/fish-multi-segment-delegation.md

@@ -0,0 +1,7 @@
+---
+'@bomb.sh/tab': patch
+---
+
+fix(fish): pass multi-segment CLI paths as separate arguments
+
+In fish, completing a package-manager-delegated CLI with more than one path segment (e.g. `pnpm <cli> <subcommand> --<TAB>`) returned nothing and fell back to the package manager's own flags. The generated fish script built the request with `string join ' '` + `eval`, and because fish does not expand bare `(...)` command substitution inside double quotes, the whole path collapsed into a single token on re-parse. The template now invokes the backend directly with fish list expansion, so every segment reaches the completion backend as its own argument. zsh and bash already quoted each argument individually; PowerShell was audited and quotes each token before `Invoke-Expression`, so it is unaffected.

src/fish.ts

@@ -31,17 +31,15 @@ function __${nameForVar}_perform_completion
 
     # Extract all args except the last one
     set -l args (commandline -opc)
-    # Extract the last arg and escape it in case it is a space or wildcard
-    set -l lastArg (string escape -- (commandline -ct))
+    # Extract the token currently being completed (may be empty on a trailing space)
+    set -l lastArg (commandline -ct)
 
     __${nameForVar}_debug "args: $args"
     __${nameForVar}_debug "last arg: $lastArg"
 
-    # Build the completion request command
-    set -l requestComp "${exec} complete -- (string join ' ' -- (string escape -- $args[2..-1])) $lastArg"
-
-    __${nameForVar}_debug "Calling $requestComp"
-    set -l results (eval $requestComp 2> /dev/null)
+    # Pass each arg as its own token (no string join/eval, which would collapse multi-segment paths); quote "$lastArg" so an empty trailing token is still sent.
+    __${nameForVar}_debug "Calling ${exec} complete -- $args[2..-1] $lastArg"
+    set -l results (${exec} complete -- $args[2..-1] "$lastArg" 2> /dev/null)
 
     # Some programs may output extra empty lines after the directive.
     # Let's ignore them or else it will break completion.

tests/__snapshots__/shell.test.ts.snap

@@ -15,17 +15,15 @@ function __testcli_perform_completion
 
     # Extract all args except the last one
     set -l args (commandline -opc)
-    # Extract the last arg and escape it in case it is a space or wildcard
-    set -l lastArg (string escape -- (commandline -ct))
+    # Extract the token currently being completed (may be empty on a trailing space)
+    set -l lastArg (commandline -ct)
 
     __testcli_debug "args: $args"
     __testcli_debug "last arg: $lastArg"
 
-    # Build the completion request command
-    set -l requestComp "/usr/bin/node /path/to/testcli complete -- (string join ' ' -- (string escape -- $args[2..-1])) $lastArg"
-
-    __testcli_debug "Calling $requestComp"
-    set -l results (eval $requestComp 2> /dev/null)
+    # Pass each arg as its own token (no string join/eval, which would collapse multi-segment paths); quote "$lastArg" so an empty trailing token is still sent.
+    __testcli_debug "Calling /usr/bin/node /path/to/testcli complete -- $args[2..-1] $lastArg"
+    set -l results (/usr/bin/node /path/to/testcli complete -- $args[2..-1] "$lastArg" 2> /dev/null)
 
     # Some programs may output extra empty lines after the directive.
     # Let's ignore them or else it will break completion.
@@ -253,17 +251,15 @@ function __test_cli_app_perform_completion
 
     # Extract all args except the last one
     set -l args (commandline -opc)
-    # Extract the last arg and escape it in case it is a space or wildcard
-    set -l lastArg (string escape -- (commandline -ct))
+    # Extract the token currently being completed (may be empty on a trailing space)
+    set -l lastArg (commandline -ct)
 
     __test_cli_app_debug "args: $args"
     __test_cli_app_debug "last arg: $lastArg"
 
-    # Build the completion request command
-    set -l requestComp "/usr/bin/node /path/to/testcli complete -- (string join ' ' -- (string escape -- $args[2..-1])) $lastArg"
-
-    __test_cli_app_debug "Calling $requestComp"
-    set -l results (eval $requestComp 2> /dev/null)
+    # Pass each arg as its own token (no string join/eval, which would collapse multi-segment paths); quote "$lastArg" so an empty trailing token is still sent.
+    __test_cli_app_debug "Calling /usr/bin/node /path/to/testcli complete -- $args[2..-1] $lastArg"
+    set -l results (/usr/bin/node /path/to/testcli complete -- $args[2..-1] "$lastArg" 2> /dev/null)
 
     # Some programs may output extra empty lines after the directive.
     # Let's ignore them or else it will break completion.

tests/completion-exec.test.ts

@@ -28,7 +28,9 @@ function extractExecCommand(shell: string, script: string): string | null {
       match = script.match(/requestComp="(.+?) complete --/);
       break;
     case 'fish':
-      match = script.match(/set -l requestComp "(.+?) complete --/);
+      // The fish template invokes the backend directly (no requestComp/eval),
+      // so match the direct call: `set -l results (<exec> complete -- ...)`.
+      match = script.match(/set -l results \((.+?) complete --/);
       break;
     case 'powershell':
       match = script.match(/\$RequestComp = "& (.+?) complete '--'/);

tests/fish-integration.test.ts

@@ -1,8 +1,11 @@
-import { describe, it, expect, beforeAll } from 'vitest';
-import { execSync, spawnSync } from 'child_process';
-import * as path from 'path';
+import { describe, it, expect, beforeAll, afterAll } from 'vitest';
+import { execSync, spawnSync } from 'node:child_process';
+import { fileURLToPath } from 'node:url';
+import { dirname, join, resolve, delimiter } from 'node:path';
+import { existsSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs';
+import { tmpdir } from 'node:os';
+import * as fish from '../src/fish';
 
-// Check if fish is available
 function isFishAvailable(): boolean {
   try {
     execSync('fish --version', { stdio: 'pipe' });
@@ -12,118 +15,100 @@ function isFishAvailable(): boolean {
   }
 }
 
-// Run fish command and return output
-function runFish(script: string): string {
-  const result = spawnSync('fish', ['-c', script], {
-    encoding: 'utf-8',
-    stdio: ['pipe', 'pipe', 'pipe'],
-  });
-  if (result.error) {
-    throw result.error;
-  }
-  return result.stdout + result.stderr;
-}
+const repoRoot = resolve(dirname(fileURLToPath(import.meta.url)), '..');
+const fixtureDir = join(repoRoot, 'tests', 'fixtures', 'mycli');
+const tabCli = join(repoRoot, 'dist', 'bin', 'cli.mjs');
 
-// Simulate TAB completion in fish
-function simulateTab(completionScript: string, commandLine: string): string[] {
-  const script = `
-    source (echo '${completionScript.replace(/'/g, "\\'")}' | psub)
-    complete --do-complete "${commandLine}"
-  `;
-  const output = runFish(script);
-  return output
-    .split('\n')
-    .filter((line) => line.trim() !== '')
-    .map((line) => line.trim());
+function shQuote(value: string): string {
+  return `'${value.replace(/'/g, `'\\''`)}'`;
 }
 
-describe.skipIf(!isFishAvailable())(
-  'fish shell completion integration tests',
-  () => {
-    const demoCliPath = path.join(
-      __dirname,
-      '../examples/demo-cli-cac/demo-cli-cac.js'
-    );
-    let completionScript: string;
-
-    beforeAll(() => {
-      // Generate the completion script from demo-cli-cac
-      const result = spawnSync('node', [demoCliPath, 'complete', 'fish'], {
-        encoding: 'utf-8',
-        cwd: path.dirname(demoCliPath),
-      });
-      completionScript = result.stdout;
-    });
-
-    it('should complete subcommands when pressing TAB after command', () => {
-      const completions = simulateTab(completionScript, 'demo-cli-cac ');
+describe.skipIf(!isFishAvailable())('fish delegation completion', () => {
+  let cacheDir: string;
+  let scriptPath: string;
 
-      // Should contain subcommands
-      expect(completions.some((c) => c.startsWith('start'))).toBe(true);
-      expect(completions.some((c) => c.startsWith('build'))).toBe(true);
-    });
+  beforeAll(() => {
+    if (!existsSync(tabCli)) {
+      execSync('pnpm build', { cwd: repoRoot, stdio: 'ignore' });
+    }
 
-    it('should complete flags when pressing TAB after --', () => {
-      const completions = simulateTab(completionScript, 'demo-cli-cac --');
+    cacheDir = mkdtempSync(join(tmpdir(), 'tab-fish-integration-'));
 
-      // Should contain global flags
-      expect(completions.some((c) => c.includes('--config'))).toBe(true);
-      expect(completions.some((c) => c.includes('--debug'))).toBe(true);
-      expect(completions.some((c) => c.includes('--help'))).toBe(true);
-      expect(completions.some((c) => c.includes('--version'))).toBe(true);
-    });
+    // Generate the pnpm completer wired to this repo's backend.
+    const exec = `${process.execPath} ${tabCli} pnpm`;
+    scriptPath = join(cacheDir, 'pnpm.fish');
+    writeFileSync(scriptPath, fish.generate('pnpm', exec));
+  });
 
-    it('should complete subcommand-specific flags', () => {
-      const completions = simulateTab(
-        completionScript,
-        'demo-cli-cac start --'
-      );
+  afterAll(() => {
+    if (cacheDir) rmSync(cacheDir, { recursive: true, force: true });
+  });
 
-      // Should contain start-specific flag
-      expect(completions.some((c) => c.includes('--port'))).toBe(true);
-      // Should also contain global flags
-      expect(completions.some((c) => c.includes('--config'))).toBe(true);
+  // Run the generated completer for a command line and return the completion
+  // values (the part before the tab-separated description).
+  function complete(commandLine: string): string[] {
+    const script = `source ${shQuote(scriptPath)}\ncomplete --do-complete ${shQuote(
+      commandLine
+    )}`;
+
+    const result = spawnSync('fish', ['-c', script], {
+      encoding: 'utf8',
+      env: {
+        ...process.env,
+        PATH: `${fixtureDir}${delimiter}${process.env.PATH ?? ''}`,
+        TAB_COMPLETION_CACHE_DIR: cacheDir,
+      },
     });
 
-    it('should complete build command flags', () => {
-      const completions = simulateTab(
-        completionScript,
-        'demo-cli-cac build --'
-      );
-
-      // Should contain build-specific flag
-      expect(completions.some((c) => c.includes('--mode'))).toBe(true);
-      // Should also contain global flags
-      expect(completions.some((c) => c.includes('--config'))).toBe(true);
-    });
+    return (result.stdout + result.stderr)
+      .split('\n')
+      .map((line) => line.trim())
+      .filter((line) => line !== '');
+  }
 
-    it('should show descriptions with completions', () => {
-      const completions = simulateTab(completionScript, 'demo-cli-cac ');
+  it('completes a delegated CLI\u2019s subcommands (single segment)', () => {
+    const completions = complete('pnpm mycli ');
+    expect(completions.some((c) => c.startsWith('start'))).toBe(true);
+    expect(completions.some((c) => c.startsWith('build'))).toBe(true);
+    expect(completions.some((c) => c.startsWith('db'))).toBe(true);
+  });
 
-      // Check that descriptions are included (tab-separated)
-      const startCompletion = completions.find((c) => c.startsWith('start'));
-      expect(startCompletion).toContain('Start the application');
+  it('includes descriptions from the delegated CLI', () => {
+    const completions = complete('pnpm mycli ');
+    const start = completions.find((c) => c.startsWith('start'));
+    expect(start).toContain('Start the application');
+  });
 
-      const buildCompletion = completions.find((c) => c.startsWith('build'));
-      expect(buildCompletion).toContain('Build the application');
-    });
+  it('filters delegated subcommands by partial input', () => {
+    const completions = complete('pnpm mycli st');
+    expect(completions.some((c) => c.startsWith('start'))).toBe(true);
+    expect(completions.some((c) => c.startsWith('build'))).toBe(false);
+  });
 
-    it('should filter completions based on partial input', () => {
-      const completions = simulateTab(completionScript, 'demo-cli-cac st');
+  it('completes the delegated CLI\u2019s flags', () => {
+    const completions = complete('pnpm mycli --');
+    expect(completions.some((c) => c.includes('--config'))).toBe(true);
+    expect(completions.some((c) => c.includes('--debug'))).toBe(true);
+  });
 
-      // Should only show completions starting with 'st'
-      expect(completions.some((c) => c.startsWith('start'))).toBe(true);
-      // 'build' should not appear
-      expect(completions.some((c) => c.startsWith('build'))).toBe(false);
-    });
+  // The following cases have a subcommand *after* the CLI name, so they are the
+  // multi-segment paths that regressed. Each must reach the delegated CLI with
+  // every segment intact.
+  it('completes a nested subcommand (two-segment path)', () => {
+    const completions = complete('pnpm mycli db ');
+    expect(completions.some((c) => c.startsWith('migrate'))).toBe(true);
+    expect(completions.some((c) => c.startsWith('seed'))).toBe(true);
+  });
 
-    it('should filter flag completions based on partial input', () => {
-      const completions = simulateTab(completionScript, 'demo-cli-cac --c');
+  it('completes flags after a subcommand (two-segment path)', () => {
+    const completions = complete('pnpm mycli start --');
+    expect(completions.some((c) => c.includes('--port'))).toBe(true);
+    expect(completions.some((c) => c.includes('--config'))).toBe(true);
+  });
 
-      // Should show --config
-      expect(completions.some((c) => c.includes('--config'))).toBe(true);
-      // Should not show --debug (doesn't start with --c)
-      expect(completions.some((c) => c.includes('--debug'))).toBe(false);
-    });
-  }
-);
+  it('completes flags after a nested subcommand (three-segment path)', () => {
+    const completions = complete('pnpm mycli db migrate --');
+    expect(completions.some((c) => c.includes('--force'))).toBe(true);
+    expect(completions.some((c) => c.includes('--name'))).toBe(true);
+  });
+});

tests/fixtures/mycli/mycli

@@ -0,0 +1,60 @@
+#!/usr/bin/env node
+// Minimal tab-protocol CLI fixture with a nested command tree, used to drive the
+// fish delegation integration tests (`pnpm mycli <sub> <TAB>`). It emits
+// `<value>\t<description>` lines plus a trailing `:<directive>`.
+
+const argv = process.argv.slice(2);
+if (argv[0] !== 'complete') {
+  console.log('mycli demo');
+  process.exit(0);
+}
+
+const dashIndex = argv.indexOf('--');
+const rest = dashIndex === -1 ? [] : argv.slice(dashIndex + 1);
+const toComplete = rest.length ? rest[rest.length - 1] : '';
+const prevWords = rest.slice(0, -1).filter((w) => !w.startsWith('-'));
+const commandPath = prevWords.join(' ');
+
+const globalFlags = [
+  ['--config', 'Specify config file'],
+  ['--debug', 'Enable debugging'],
+  ['--help', 'Display help'],
+  ['--version', 'Display version'],
+];
+
+const tree = {
+  '': {
+    subs: [
+      ['start', 'Start the application'],
+      ['build', 'Build the application'],
+      ['db', 'Database commands'],
+    ],
+    flags: [],
+  },
+  start: { subs: [], flags: [['--port', 'Port to use']] },
+  build: { subs: [], flags: [['--mode', 'Build mode']] },
+  db: {
+    subs: [
+      ['migrate', 'Run migrations'],
+      ['seed', 'Seed the database'],
+    ],
+    flags: [],
+  },
+  'db migrate': {
+    subs: [],
+    flags: [
+      ['--force', 'Force the migration'],
+      ['--name', 'Migration name'],
+    ],
+  },
+};
+
+const node = tree[commandPath] || { subs: [], flags: [] };
+const candidates = toComplete.startsWith('-')
+  ? [...node.flags, ...globalFlags]
+  : node.subs;
+
+for (const [value, description] of candidates) {
+  if (value.startsWith(toComplete)) console.log(`${value}\t${description}`);
+}
+console.log(':4');