From a404118661b1d23ce3c56c68fc6f23a907119ac1 Mon Sep 17 00:00:00 2001 From: Leonardo Rojas Date: Wed, 10 Jun 2026 22:12:33 -0400 Subject: [PATCH] fix(completions): suppress filesystem fallback in Fish completions --- .../fish-completion-no-file-fallback.md | 5 + src/core/completions/command-registry.ts | 8 + .../completions/generators/fish-generator.ts | 148 ++++++++++------- .../completions/templates/fish-templates.ts | 25 +++ src/core/completions/types.ts | 5 + .../generators/fish-generator.test.ts | 149 ++++++++++++++++-- 6 files changed, 274 insertions(+), 66 deletions(-) create mode 100644 .changeset/fish-completion-no-file-fallback.md diff --git a/.changeset/fish-completion-no-file-fallback.md b/.changeset/fish-completion-no-file-fallback.md new file mode 100644 index 000000000..643ea5165 --- /dev/null +++ b/.changeset/fish-completion-no-file-fallback.md @@ -0,0 +1,5 @@ +--- +'@fission-ai/openspec': patch +--- + +Improve Fish completions so command, subcommand, flag, and indexed positional completions no longer fall back to filesystem suggestions unless the target is a real path. diff --git a/src/core/completions/command-registry.ts b/src/core/completions/command-registry.ts index 88ec88e05..496b3451c 100644 --- a/src/core/completions/command-registry.ts +++ b/src/core/completions/command-registry.ts @@ -245,6 +245,7 @@ export const COMMAND_REGISTRY: CommandDefinition[] = [ name: 'store-path', description: 'Existing local context store root for --initiative', takesValue: true, + completionType: 'path', }, { name: 'schema', @@ -282,6 +283,7 @@ export const COMMAND_REGISTRY: CommandDefinition[] = [ name: 'store-path', description: 'Existing local context store root for --initiative', takesValue: true, + completionType: 'path', }, COMMON_FLAGS.json, ], @@ -430,6 +432,7 @@ export const COMMAND_REGISTRY: CommandDefinition[] = [ name: 'store-path', description: 'Existing local context store root for --initiative', takesValue: true, + completionType: 'path', }, { name: 'agent', @@ -471,6 +474,7 @@ export const COMMAND_REGISTRY: CommandDefinition[] = [ name: 'path', description: 'Directory to use for the context store', takesValue: true, + completionType: 'path', }, { name: 'init-git', @@ -564,6 +568,7 @@ export const COMMAND_REGISTRY: CommandDefinition[] = [ name: 'store-path', description: 'Existing local context store root', takesValue: true, + completionType: 'path', }, { name: 'title', @@ -593,6 +598,7 @@ export const COMMAND_REGISTRY: CommandDefinition[] = [ name: 'store-path', description: 'Existing local context store root', takesValue: true, + completionType: 'path', }, COMMON_FLAGS.json, ], @@ -610,6 +616,7 @@ export const COMMAND_REGISTRY: CommandDefinition[] = [ name: 'store-path', description: 'Existing local context store root', takesValue: true, + completionType: 'path', }, COMMON_FLAGS.json, ], @@ -627,6 +634,7 @@ export const COMMAND_REGISTRY: CommandDefinition[] = [ name: 'store-path', description: 'Existing local context store root', takesValue: true, + completionType: 'path', }, COMMON_FLAGS.json, ], diff --git a/src/core/completions/generators/fish-generator.ts b/src/core/completions/generators/fish-generator.ts index fa1d21af9..50cbad0dc 100644 --- a/src/core/completions/generators/fish-generator.ts +++ b/src/core/completions/generators/fish-generator.ts @@ -15,17 +15,15 @@ export class FishGenerator implements CompletionGenerator { * @returns Fish completion script as a string */ generate(commands: CommandDefinition[]): string { - // Build top-level commands using push() for loop clarity const topLevelLines: string[] = []; for (const cmd of commands) { topLevelLines.push(`# ${cmd.name} command`); topLevelLines.push( - `complete -c openspec -n '__fish_openspec_no_subcommand' -a '${cmd.name}' -d '${this.escapeDescription(cmd.description)}'` + `complete -c openspec -n '__fish_openspec_no_subcommand' -f -a '${cmd.name}' -d '${this.escapeDescription(cmd.description)}'` ); } const topLevelCommands = topLevelLines.join('\n'); - // Build command-specific completions using push() for loop clarity const commandCompletionLines: string[] = []; for (const cmd of commands) { commandCompletionLines.push(...this.generateCommandCompletions(cmd)); @@ -33,13 +31,9 @@ export class FishGenerator implements CompletionGenerator { } const commandCompletions = commandCompletionLines.join('\n'); - // Static helper functions from template const helperFunctions = FISH_STATIC_HELPERS; - - // Dynamic completion helpers from template const dynamicHelpers = FISH_DYNAMIC_HELPERS; - // Assemble final script with template literal return `# Fish completion script for OpenSpec CLI # Auto-generated - do not edit manually @@ -56,42 +50,63 @@ ${commandCompletions}`; private generateCommandCompletions(cmd: CommandDefinition): string[] { const lines: string[] = []; - // If command has subcommands if (cmd.subcommands && cmd.subcommands.length > 0) { - // Add subcommand completions for (const subcmd of cmd.subcommands) { lines.push( - `complete -c openspec -n '__fish_openspec_using_subcommand ${cmd.name}; and not __fish_openspec_using_subcommand ${subcmd.name}' -a '${subcmd.name}' -d '${this.escapeDescription(subcmd.description)}'` + `complete -c openspec -n '__fish_openspec_using_subcommand ${cmd.name}; and not __fish_openspec_using_subcommand ${subcmd.name}' -f -a '${subcmd.name}' -d '${this.escapeDescription(subcmd.description)}'` ); } lines.push(''); - // Add flags for parent command for (const flag of cmd.flags) { lines.push(...this.generateFlagCompletion(flag, `__fish_openspec_using_subcommand ${cmd.name}`)); } - // Add completions for each subcommand for (const subcmd of cmd.subcommands) { lines.push(`# ${cmd.name} ${subcmd.name} flags`); for (const flag of subcmd.flags) { - lines.push(...this.generateFlagCompletion(flag, `__fish_openspec_using_subcommand ${cmd.name}; and __fish_openspec_using_subcommand ${subcmd.name}`)); + lines.push( + ...this.generateFlagCompletion( + flag, + `__fish_openspec_using_subcommand ${cmd.name}; and __fish_openspec_using_subcommand ${subcmd.name}` + ) + ); } - // Add positional completions for subcommand - if (subcmd.acceptsPositional) { - lines.push(...this.generatePositionalCompletion(subcmd.positionalType, `__fish_openspec_using_subcommand ${cmd.name}; and __fish_openspec_using_subcommand ${subcmd.name}`)); + if (subcmd.positionals?.length) { + lines.push( + ...this.generateIndexedPositionalCompletions( + subcmd.positionals, + `__fish_openspec_using_subcommand ${cmd.name}; and __fish_openspec_using_subcommand ${subcmd.name}`, + this.collectValueFlags(cmd.flags, subcmd.flags), + 2 + ) + ); + } else if (subcmd.acceptsPositional) { + lines.push( + ...this.generatePositionalCompletion( + subcmd.positionalType, + `__fish_openspec_using_subcommand ${cmd.name}; and __fish_openspec_using_subcommand ${subcmd.name}` + ) + ); } } } else { - // Command without subcommands lines.push(`# ${cmd.name} flags`); for (const flag of cmd.flags) { lines.push(...this.generateFlagCompletion(flag, `__fish_openspec_using_subcommand ${cmd.name}`)); } - // Add positional completions - if (cmd.acceptsPositional) { + if (cmd.positionals?.length) { + lines.push( + ...this.generateIndexedPositionalCompletions( + cmd.positionals, + `__fish_openspec_using_subcommand ${cmd.name}`, + this.collectValueFlags(cmd.flags), + 1 + ) + ); + } else if (cmd.acceptsPositional) { lines.push(...this.generatePositionalCompletion(cmd.positionalType, `__fish_openspec_using_subcommand ${cmd.name}`)); } } @@ -104,44 +119,23 @@ ${commandCompletions}`; */ private generateFlagCompletion(flag: FlagDefinition, condition: string): string[] { const lines: string[] = []; - const longFlag = `--${flag.name}`; - const shortFlag = flag.short ? `-${flag.short}` : undefined; + const description = this.escapeDescription(flag.description); + const shortFlag = flag.short ? `-s ${flag.short} ` : ''; + const flagOptions = `${shortFlag}-l ${flag.name}`; + const fileFallback = flag.completionType === 'path' ? '-r' : '-r -f'; if (flag.takesValue && flag.values) { - // Flag with enum values for (const value of flag.values) { - if (shortFlag) { - lines.push( - `complete -c openspec -n '${condition}' -s ${flag.short} -l ${flag.name} -a '${value}' -d '${this.escapeDescription(flag.description)}'` - ); - } else { - lines.push( - `complete -c openspec -n '${condition}' -l ${flag.name} -a '${value}' -d '${this.escapeDescription(flag.description)}'` - ); - } - } - } else if (flag.takesValue) { - // Flag that takes a value but no specific values defined - if (shortFlag) { - lines.push( - `complete -c openspec -n '${condition}' -s ${flag.short} -l ${flag.name} -r -d '${this.escapeDescription(flag.description)}'` - ); - } else { lines.push( - `complete -c openspec -n '${condition}' -l ${flag.name} -r -d '${this.escapeDescription(flag.description)}'` + `complete -c openspec -n '${condition}' ${flagOptions} -f -a '${value}' -d '${description}'` ); } + } else if (flag.takesValue) { + lines.push( + `complete -c openspec -n '${condition}' ${flagOptions} ${fileFallback} -d '${description}'` + ); } else { - // Boolean flag - if (shortFlag) { - lines.push( - `complete -c openspec -n '${condition}' -s ${flag.short} -l ${flag.name} -d '${this.escapeDescription(flag.description)}'` - ); - } else { - lines.push( - `complete -c openspec -n '${condition}' -l ${flag.name} -d '${this.escapeDescription(flag.description)}'` - ); - } + lines.push(`complete -c openspec -n '${condition}' ${flagOptions} -f -d '${description}'`); } return lines; @@ -170,22 +164,66 @@ ${commandCompletions}`; lines.push(`complete -c openspec -n '${condition}' -a 'zsh bash fish powershell' -f`); break; case 'path': - // Fish automatically completes files, no need to specify + // Emit the rule without -f so Fish can offer filesystem completions. + lines.push(`complete -c openspec -n '${condition}'`); + break; + default: + lines.push(`complete -c openspec -n '${condition}' -f`); break; } return lines; } + /** + * Generate indexed positional completions. + */ + private generateIndexedPositionalCompletions( + positionals: NonNullable, + condition: string, + valueFlags: string[], + depth: number + ): string[] { + const lines: string[] = []; + + for (const [index, positional] of positionals.entries()) { + const indexCondition = `${condition}; and __fish_openspec_positional_index ${index} ${depth}${valueFlags.length ? ` ${valueFlags.join(' ')}` : ''}`; + lines.push(...this.generatePositionalCompletion(positional.type, indexCondition)); + } + + return lines; + } + + /** + * Collect the long and short names for flags that consume the next token. + */ + private collectValueFlags(...flagGroups: FlagDefinition[][]): string[] { + const flags = new Set(); + + for (const group of flagGroups) { + for (const flag of group) { + if (!flag.takesValue) { + continue; + } + + flags.add(`--${flag.name}`); + if (flag.short) { + flags.add(`-${flag.short}`); + } + } + } + + return [...flags]; + } /** * Escape description text for Fish */ private escapeDescription(description: string): string { return description - .replace(/\\/g, '\\\\') // Backslashes first - .replace(/'/g, "\\'") // Single quotes - .replace(/\$/g, '\\$') // Dollar signs (prevents $()) - .replace(/`/g, '\\`'); // Backticks + .replace(/\\/g, '\\\\') // Backslashes first + .replace(/'/g, "\\'") // Single quotes + .replace(/\$/g, '\\$') // Dollar signs (prevents $()) + .replace(/`/g, '\\`'); // Backticks } } diff --git a/src/core/completions/templates/fish-templates.ts b/src/core/completions/templates/fish-templates.ts index f3349f77b..7abc43e5e 100644 --- a/src/core/completions/templates/fish-templates.ts +++ b/src/core/completions/templates/fish-templates.ts @@ -18,6 +18,31 @@ end function __fish_openspec_no_subcommand set -l cmd (commandline -opc) test (count $cmd) -eq 1 +end + +function __fish_openspec_positional_index + set -l target $argv[1] + set -l depth $argv[2] + set -l value_flags $argv[3..] + set -l tokens (commandline -opc) + set -e tokens[1] + set -l count 0 + set -l skip 0 + for token in $tokens + if test $skip -eq 1 + set skip 0 + continue + end + if contains -- $token $value_flags + set skip 1 + continue + end + if string match -q -- '-*' $token + continue + end + set count (math $count + 1) + end + test $count -eq (math $target + $depth) end`; export const FISH_DYNAMIC_HELPERS = `# Dynamic completion helpers diff --git a/src/core/completions/types.ts b/src/core/completions/types.ts index 90df710b2..5d88f0f3a 100644 --- a/src/core/completions/types.ts +++ b/src/core/completions/types.ts @@ -24,6 +24,11 @@ export interface FlagDefinition { */ takesValue?: boolean; + /** + * Completion type for the flag value. + */ + completionType?: PositionalType; + /** * Possible values for the flag (for completion suggestions) */ diff --git a/test/core/completions/generators/fish-generator.test.ts b/test/core/completions/generators/fish-generator.test.ts index 3ea1de1a4..df6b70926 100644 --- a/test/core/completions/generators/fish-generator.test.ts +++ b/test/core/completions/generators/fish-generator.test.ts @@ -9,6 +9,12 @@ describe('FishGenerator', () => { generator = new FishGenerator(); }); + function completionLine(script: string, needle: string): string | undefined { + return script + .split('\n') + .find((line) => line.includes('complete -c openspec') && line.includes(needle)); + } + describe('interface compliance', () => { it('should have shell property set to "fish"', () => { expect(generator.shell).toBe('fish'); @@ -48,6 +54,7 @@ describe('FishGenerator', () => { expect(script).toContain('function __fish_openspec_using_subcommand'); expect(script).toContain('function __fish_openspec_no_subcommand'); + expect(script).toContain('function __fish_openspec_positional_index'); expect(script).toContain('commandline -opc'); }); @@ -73,7 +80,7 @@ describe('FishGenerator', () => { const script = generator.generate(commands); expect(script).toContain("complete -c openspec"); - expect(script).toContain("-a 'init'"); + expect(script).toContain("-f -a 'init'"); expect(script).toContain("'Initialize OpenSpec'"); expect(script).toContain("-a 'validate'"); expect(script).toContain("'Validate specs'"); @@ -101,10 +108,13 @@ describe('FishGenerator', () => { const script = generator.generate(commands); - expect(script).toContain("-l strict"); - expect(script).toContain("'Enable strict mode'"); - expect(script).toContain("-l json"); - expect(script).toContain("'Output as JSON'"); + const strictLine = completionLine(script, '-l strict'); + const jsonLine = completionLine(script, '-l json'); + + expect(strictLine).toContain('-f'); + expect(strictLine).toContain("'Enable strict mode'"); + expect(jsonLine).toContain('-f'); + expect(jsonLine).toContain("'Output as JSON'"); }); it('should handle flags with short options', () => { @@ -128,7 +138,7 @@ describe('FishGenerator', () => { expect(script).toContain("-s r"); expect(script).toContain("-l requirement"); expect(script).toContain("'Show specific requirement'"); - expect(script).toContain("-r"); + expect(script).toContain("-r -f"); }); it('should use -r flag for flags that require values', () => { @@ -149,7 +159,37 @@ describe('FishGenerator', () => { const script = generator.generate(commands); expect(script).toContain("-l output"); - expect(script).toContain("-r"); + expect(script).toContain("-r -f"); + }); + + it('should allow file completion for path flags', () => { + const commands: CommandDefinition[] = [ + { + name: 'context-store', + description: 'Set up and inspect context stores', + flags: [], + subcommands: [ + { + name: 'setup', + description: 'Create or register a local context store', + flags: [ + { + name: 'path', + description: 'Directory to use for the context store', + takesValue: true, + completionType: 'path', + }, + ], + }, + ], + }, + ]; + + const script = generator.generate(commands); + const pathLine = completionLine(script, '-l path'); + + expect(pathLine).toContain('-r'); + expect(pathLine).not.toContain('-f'); }); it('should not use -r flag for boolean flags', () => { @@ -173,6 +213,7 @@ describe('FishGenerator', () => { expect(strictLine).toBeDefined(); expect(strictLine).not.toContain(' -r'); + expect(strictLine).toContain(' -f'); }); it('should handle flags with enum values', () => { @@ -194,8 +235,8 @@ describe('FishGenerator', () => { const script = generator.generate(commands); expect(script).toContain("-l type"); - expect(script).toContain("change"); - expect(script).toContain("spec"); + expect(script).toContain("-f -a 'change'"); + expect(script).toContain("-f -a 'spec'"); }); it('should handle commands with subcommands', () => { @@ -222,8 +263,8 @@ describe('FishGenerator', () => { const script = generator.generate(commands); expect(script).toContain("'change'"); - expect(script).toContain("'show'"); - expect(script).toContain("'list'"); + expect(script).toContain("-f -a 'show'"); + expect(script).toContain("-f -a 'list'"); expect(script).toContain("__fish_openspec_using_subcommand change"); }); @@ -239,7 +280,9 @@ describe('FishGenerator', () => { ]; const script = generator.generate(commands); + const line = completionLine(script, '__fish_openspec_changes'); + expect(line).toContain('-f'); expect(script).toContain('__fish_openspec_changes'); }); @@ -255,7 +298,9 @@ describe('FishGenerator', () => { ]; const script = generator.generate(commands); + const line = completionLine(script, '__fish_openspec_specs'); + expect(line).toContain('-f'); expect(script).toContain('__fish_openspec_specs'); }); @@ -271,7 +316,9 @@ describe('FishGenerator', () => { ]; const script = generator.generate(commands); + const line = completionLine(script, '__fish_openspec_items'); + expect(line).toContain('-f'); expect(script).toContain('__fish_openspec_items'); }); @@ -287,7 +334,9 @@ describe('FishGenerator', () => { ]; const script = generator.generate(commands); + const line = completionLine(script, "-a 'zsh bash fish powershell'"); + expect(line).toContain('-f'); expect(script).toContain('zsh'); expect(script).toContain('bash'); expect(script).toContain('fish'); @@ -306,11 +355,83 @@ describe('FishGenerator', () => { ]; const script = generator.generate(commands); + const line = completionLine(script, '__fish_openspec_schemas'); + expect(line).toContain('-f'); expect(script).toContain('__fish_openspec_schemas'); expect(script).toContain('openspec __complete schemas 2>/dev/null'); }); + it('should handle indexed positional arguments for schema fork', () => { + const commands: CommandDefinition[] = [ + { + name: 'schema', + description: 'Manage schemas', + flags: [], + subcommands: [ + { + name: 'fork', + description: 'Copy an existing schema to project for customization', + acceptsPositional: true, + positionals: [ + { name: 'source', type: 'schema-name' }, + { name: 'name', optional: true }, + ], + flags: [], + }, + ], + }, + ]; + + const script = generator.generate(commands); + const sourceLine = completionLine(script, '__fish_openspec_positional_index 0 2'); + const nameLine = completionLine(script, '__fish_openspec_positional_index 1 2'); + + expect(sourceLine).toContain('__fish_openspec_schemas'); + expect(sourceLine).toContain('-f'); + expect(nameLine).toContain('-f'); + expect(nameLine).not.toContain('__fish_openspec_schemas'); + }); + + it('should allow file completion for path-typed indexed positionals', () => { + const commands: CommandDefinition[] = [ + { + name: 'workspace', + description: 'Set up and inspect coordination workspaces', + flags: [], + subcommands: [ + { + name: 'relink', + description: 'Update the local path for an existing workspace link', + acceptsPositional: true, + positionals: [ + { name: 'name' }, + { name: 'path', type: 'path' }, + ], + flags: [ + { + name: 'workspace', + description: 'Workspace name from local workspace views', + takesValue: true, + }, + ], + }, + ], + }, + ]; + + const script = generator.generate(commands); + const firstLine = completionLine(script, '__fish_openspec_positional_index 0 2 --workspace'); + const secondLine = completionLine(script, '__fish_openspec_positional_index 1 2 --workspace'); + + expect(firstLine).toContain('-f'); + expect(secondLine).toContain('complete -c openspec -n'); + expect(secondLine).toContain('__fish_openspec_using_subcommand workspace'); + expect(secondLine).toContain('__fish_openspec_using_subcommand relink'); + expect(secondLine).toContain('__fish_openspec_positional_index 1 2 --workspace'); + expect(secondLine).not.toContain('-f'); + }); + it('should generate dynamic completion helper for changes', () => { const commands: CommandDefinition[] = [ { @@ -323,7 +444,9 @@ describe('FishGenerator', () => { ]; const script = generator.generate(commands); + const line = completionLine(script, '__fish_openspec_changes'); + expect(line).toContain('-f'); expect(script).toContain('function __fish_openspec_changes'); expect(script).toContain('openspec __complete changes 2>/dev/null'); expect(script).toContain('while read -l id desc'); @@ -342,7 +465,9 @@ describe('FishGenerator', () => { ]; const script = generator.generate(commands); + const line = completionLine(script, '__fish_openspec_specs'); + expect(line).toContain('-f'); expect(script).toContain('function __fish_openspec_specs'); expect(script).toContain('openspec __complete specs 2>/dev/null'); }); @@ -359,7 +484,9 @@ describe('FishGenerator', () => { ]; const script = generator.generate(commands); + const line = completionLine(script, '__fish_openspec_items'); + expect(line).toContain('-f'); expect(script).toContain('function __fish_openspec_items'); expect(script).toContain('__fish_openspec_changes'); expect(script).toContain('__fish_openspec_specs');