From f23dd9966220f856a7d45dd0e6fdb65c1f52a9c8 Mon Sep 17 00:00:00 2001 From: Michal Date: Mon, 23 Feb 2026 19:16:36 +0000 Subject: [PATCH] feat: erase stale fish completions and add completion tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fish completions are additive — sourcing a new file doesn't remove old rules. Add `complete -c mcpctl -e` at the top to clear stale entries. Also add 12 structural tests to prevent completion regressions. Co-Authored-By: Claude Opus 4.6 --- completions/mcpctl.fish | 3 + src/cli/tests/completions.test.ts | 115 ++++++++++++++++++++++++++++++ 2 files changed, 118 insertions(+) create mode 100644 src/cli/tests/completions.test.ts diff --git a/completions/mcpctl.fish b/completions/mcpctl.fish index 9c2efae..24c8b15 100644 --- a/completions/mcpctl.fish +++ b/completions/mcpctl.fish @@ -1,5 +1,8 @@ # mcpctl fish completions +# Erase any stale completions from previous versions +complete -c mcpctl -e + set -l commands status login logout config get describe delete logs create edit apply backup restore help set -l project_commands attach-server detach-server get describe delete logs create edit help diff --git a/src/cli/tests/completions.test.ts b/src/cli/tests/completions.test.ts new file mode 100644 index 0000000..b2a513c --- /dev/null +++ b/src/cli/tests/completions.test.ts @@ -0,0 +1,115 @@ +import { describe, it, expect } from 'vitest'; +import { readFileSync } from 'node:fs'; +import { join, dirname } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const root = join(dirname(fileURLToPath(import.meta.url)), '..', '..', '..'); +const fishFile = readFileSync(join(root, 'completions', 'mcpctl.fish'), 'utf-8'); +const bashFile = readFileSync(join(root, 'completions', 'mcpctl.bash'), 'utf-8'); + +describe('fish completions', () => { + it('erases stale completions at the top', () => { + const lines = fishFile.split('\n'); + const firstComplete = lines.findIndex((l) => l.startsWith('complete ')); + expect(lines[firstComplete]).toContain('-e'); + }); + + it('does not offer resource types without __mcpctl_needs_resource_type guard', () => { + const resourceTypes = ['servers', 'instances', 'secrets', 'templates', 'projects', 'users', 'groups', 'rbac']; + const lines = fishFile.split('\n').filter((l) => l.startsWith('complete ')); + + for (const line of lines) { + // Find lines that offer resource types as positional args + const offersResourceType = resourceTypes.some((r) => { + // Match `-a "...servers..."` or `-a 'servers projects'` + const aMatch = line.match(/-a\s+['"]([^'"]+)['"]/); + if (!aMatch) return false; + return aMatch[1].split(/\s+/).includes(r); + }); + + if (!offersResourceType) continue; + + // Skip the help completions line and the -e line + if (line.includes('__fish_seen_subcommand_from help')) continue; + // Skip project-scoped command offerings (those offer commands, not resource types) + if (line.includes('attach-server') || line.includes('detach-server')) continue; + // Skip lines that offer commands (not resource types) + if (line.includes("-d 'Show") || line.includes("-d 'Manage") || line.includes("-d 'Authenticate") || + line.includes("-d 'Log out'") || line.includes("-d 'Get instance") || line.includes("-d 'Create a resource'") || + line.includes("-d 'Edit a resource'") || line.includes("-d 'Apply") || line.includes("-d 'Backup") || + line.includes("-d 'Restore") || line.includes("-d 'List resources") || line.includes("-d 'Delete a resource'")) continue; + + // Lines offering resource types MUST have __mcpctl_needs_resource_type in their condition + expect(line, `Resource type completion missing guard: ${line}`).toContain('__mcpctl_needs_resource_type'); + } + }); + + it('resource name completions require resource type to be selected', () => { + const lines = fishFile.split('\n').filter((l) => l.startsWith('complete') && l.includes('__mcpctl_resource_names')); + expect(lines.length).toBeGreaterThan(0); + for (const line of lines) { + expect(line).toContain('not __mcpctl_needs_resource_type'); + } + }); + + it('defines --project option', () => { + expect(fishFile).toContain("complete -c mcpctl -l project"); + }); + + it('attach-server only shows with --project', () => { + const lines = fishFile.split('\n').filter((l) => l.includes('attach-server') && l.startsWith('complete')); + expect(lines.length).toBeGreaterThan(0); + for (const line of lines) { + expect(line).toContain('__mcpctl_has_project'); + } + }); + + it('detach-server only shows with --project', () => { + const lines = fishFile.split('\n').filter((l) => l.includes('detach-server') && l.startsWith('complete')); + expect(lines.length).toBeGreaterThan(0); + for (const line of lines) { + expect(line).toContain('__mcpctl_has_project'); + } + }); + + it('non-project commands do not show with --project', () => { + const nonProjectCmds = ['status', 'login', 'logout', 'config', 'apply', 'backup', 'restore']; + const lines = fishFile.split('\n').filter((l) => l.startsWith('complete') && l.includes('-a ')); + + for (const cmd of nonProjectCmds) { + const cmdLines = lines.filter((l) => { + const aMatch = l.match(/-a\s+(\S+)/); + return aMatch && aMatch[1].replace(/['"]/g, '') === cmd; + }); + for (const line of cmdLines) { + expect(line, `${cmd} should require 'not __mcpctl_has_project'`).toContain('not __mcpctl_has_project'); + } + } + }); +}); + +describe('bash completions', () => { + it('separates project commands from regular commands', () => { + expect(bashFile).toContain('project_commands='); + expect(bashFile).toContain('attach-server detach-server'); + }); + + it('checks has_project before offering project commands', () => { + expect(bashFile).toContain('if $has_project'); + expect(bashFile).toContain('$project_commands'); + }); + + it('fetches resource names dynamically after resource type', () => { + expect(bashFile).toContain('_mcpctl_resource_names'); + // get/describe/delete should use resource_names when resource_type is set + expect(bashFile).toMatch(/get\|describe\|delete\)[\s\S]*?_mcpctl_resource_names/); + }); + + it('offers server names for attach-server/detach-server', () => { + expect(bashFile).toMatch(/attach-server\|detach-server\)[\s\S]*?_mcpctl_resource_names.*servers/); + }); + + it('defines --project option', () => { + expect(bashFile).toContain('--project'); + }); +}); -- 2.49.1