From 65c340a03cd63aa2d9a412d52fa1dae2ffcfcef3 Mon Sep 17 00:00:00 2001 From: Michal Date: Mon, 23 Feb 2026 19:36:45 +0000 Subject: [PATCH] fix: prevent attach/detach-server from repeating server arg on tab Added __mcpctl_needs_server_arg guard in fish and position check in bash so completions stop after one server name is selected. Co-Authored-By: Claude Opus 4.6 --- completions/mcpctl.bash | 4 ++++ completions/mcpctl.fish | 28 ++++++++++++++++++++++++---- src/cli/tests/completions.test.ts | 13 ++++++++----- 3 files changed, 36 insertions(+), 9 deletions(-) diff --git a/completions/mcpctl.bash b/completions/mcpctl.bash index d676a7d..8037a9f 100644 --- a/completions/mcpctl.bash +++ b/completions/mcpctl.bash @@ -125,6 +125,8 @@ _mcpctl() { COMPREPLY=($(compgen -W "-i --input -p --password -c --conflict -h --help" -- "$cur")) return ;; attach-server) + # Only complete if no server arg given yet (first arg after subcmd) + if [[ $((cword - subcmd_pos)) -ne 1 ]]; then return; fi local proj names all_servers proj_servers proj=$(_mcpctl_get_project_value) if [[ -n "$proj" ]]; then @@ -137,6 +139,8 @@ _mcpctl() { COMPREPLY=($(compgen -W "$names" -- "$cur")) return ;; detach-server) + # Only complete if no server arg given yet (first arg after subcmd) + if [[ $((cword - subcmd_pos)) -ne 1 ]]; then return; fi local proj names proj=$(_mcpctl_get_project_value) if [[ -n "$proj" ]]; then diff --git a/completions/mcpctl.fish b/completions/mcpctl.fish index cf66f80..8bc628b 100644 --- a/completions/mcpctl.fish +++ b/completions/mcpctl.fish @@ -159,11 +159,31 @@ complete -c mcpctl -n "__fish_seen_subcommand_from edit; and __mcpctl_needs_reso # Resource names — after resource type is selected complete -c mcpctl -n "__fish_seen_subcommand_from get describe delete edit; and not __mcpctl_needs_resource_type" -a '(__mcpctl_resource_names)' -d 'Resource name' -# attach-server: show servers NOT in the project -complete -c mcpctl -n "__fish_seen_subcommand_from attach-server" -a '(__mcpctl_available_servers)' -d 'Server' +# Helper: check if attach-server/detach-server already has a server argument +function __mcpctl_needs_server_arg + set -l tokens (commandline -opc) + set -l found_cmd false + for tok in $tokens + if $found_cmd + if not string match -q -- '-*' $tok + return 1 # server arg already present + end + end + if contains -- $tok attach-server detach-server + set found_cmd true + end + end + if $found_cmd + return 0 # command found but no server arg yet + end + return 1 +end -# detach-server: show servers IN the project -complete -c mcpctl -n "__fish_seen_subcommand_from detach-server" -a '(__mcpctl_project_servers)' -d 'Server' +# attach-server: show servers NOT in the project (only if no server arg yet) +complete -c mcpctl -n "__fish_seen_subcommand_from attach-server; and __mcpctl_needs_server_arg" -a '(__mcpctl_available_servers)' -d 'Server' + +# detach-server: show servers IN the project (only if no server arg yet) +complete -c mcpctl -n "__fish_seen_subcommand_from detach-server; and __mcpctl_needs_server_arg" -a '(__mcpctl_project_servers)' -d 'Server' # get/describe options complete -c mcpctl -n "__fish_seen_subcommand_from get" -s o -l output -d 'Output format' -xa 'table json yaml' diff --git a/src/cli/tests/completions.test.ts b/src/cli/tests/completions.test.ts index 8e08556..cf4a349 100644 --- a/src/cli/tests/completions.test.ts +++ b/src/cli/tests/completions.test.ts @@ -94,19 +94,20 @@ describe('fish completions', () => { expect(resourceNamesFn, 'must handle instances via server.name').toContain('.server.name'); }); - it('attach-server completes with available (unattached) servers', () => { - // Find the line that provides argument completions AFTER attach-server is selected + it('attach-server completes with available (unattached) servers and guards against repeat', () => { const attachLine = fishFile.split('\n').find((l) => l.startsWith('complete') && l.includes('__fish_seen_subcommand_from attach-server')); expect(attachLine, 'attach-server argument completion must exist').toBeDefined(); expect(attachLine, 'attach-server must use __mcpctl_available_servers').toContain('__mcpctl_available_servers'); + expect(attachLine, 'attach-server must guard with __mcpctl_needs_server_arg').toContain('__mcpctl_needs_server_arg'); }); - it('detach-server completes with project servers', () => { + it('detach-server completes with project servers and guards against repeat', () => { const detachLine = fishFile.split('\n').find((l) => l.startsWith('complete') && l.includes('__fish_seen_subcommand_from detach-server')); expect(detachLine, 'detach-server argument completion must exist').toBeDefined(); expect(detachLine, 'detach-server must use __mcpctl_project_servers').toContain('__mcpctl_project_servers'); + expect(detachLine, 'detach-server must guard with __mcpctl_needs_server_arg').toContain('__mcpctl_needs_server_arg'); }); it('non-project commands do not show with --project', () => { @@ -142,16 +143,18 @@ describe('bash completions', () => { expect(bashFile).toMatch(/get\|describe\|delete\)[\s\S]*?_mcpctl_resource_names/); }); - it('attach-server filters out already-attached servers using project context', () => { + it('attach-server filters out already-attached servers and guards against repeat', () => { const attachBlock = bashFile.match(/attach-server\)[\s\S]*?return ;;/)?.[0] ?? ''; expect(attachBlock, 'attach-server must use _mcpctl_get_project_value').toContain('_mcpctl_get_project_value'); expect(attachBlock, 'attach-server must query project servers to exclude').toContain('--project'); + expect(attachBlock, 'attach-server must check position to prevent repeat').toContain('cword - subcmd_pos'); }); - it('detach-server shows only project servers', () => { + it('detach-server shows only project servers and guards against repeat', () => { const detachBlock = bashFile.match(/detach-server\)[\s\S]*?return ;;/)?.[0] ?? ''; expect(detachBlock, 'detach-server must use _mcpctl_get_project_value').toContain('_mcpctl_get_project_value'); expect(detachBlock, 'detach-server must query project servers').toContain('--project'); + expect(detachBlock, 'detach-server must check position to prevent repeat').toContain('cword - subcmd_pos'); }); it('instances use server.name instead of name', () => {