refactor: consolidate restore under backup command

mcpctl backup restore list/diff/to instead of separate mcpctl restore.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Michal
2026-03-08 01:17:03 +00:00
parent 7818cb2194
commit 98f3a3eda0
7 changed files with 32 additions and 85 deletions

View File

@@ -5,7 +5,7 @@ _mcpctl() {
local cur prev words cword local cur prev words cword
_init_completion || return _init_completion || return
local commands="status login logout config get describe delete logs create edit apply patch backup restore approve console cache" local commands="status login logout config get describe delete logs create edit apply patch backup approve console cache"
local project_commands="get describe delete logs create edit attach-server detach-server" local project_commands="get describe delete logs create edit attach-server detach-server"
local global_opts="-v --version --daemon-url --direct -p --project -h --help" local global_opts="-v --version --daemon-url --direct -p --project -h --help"
local resources="servers instances secrets templates projects users groups rbac prompts promptrequests serverattachments proxymodels all" local resources="servers instances secrets templates projects users groups rbac prompts promptrequests serverattachments proxymodels all"
@@ -235,7 +235,7 @@ _mcpctl() {
backup) backup)
local backup_sub=$(_mcpctl_get_subcmd $subcmd_pos) local backup_sub=$(_mcpctl_get_subcmd $subcmd_pos)
if [[ -z "$backup_sub" ]]; then if [[ -z "$backup_sub" ]]; then
COMPREPLY=($(compgen -W "log key help" -- "$cur")) COMPREPLY=($(compgen -W "log key restore help" -- "$cur"))
else else
case "$backup_sub" in case "$backup_sub" in
log) log)
@@ -244,27 +244,9 @@ _mcpctl() {
key) key)
COMPREPLY=($(compgen -W "-h --help" -- "$cur")) COMPREPLY=($(compgen -W "-h --help" -- "$cur"))
;; ;;
*) restore)
COMPREPLY=($(compgen -W "-h --help" -- "$cur")) COMPREPLY=($(compgen -W "-h --help" -- "$cur"))
;; ;;
esac
fi
return ;;
restore)
local restore_sub=$(_mcpctl_get_subcmd $subcmd_pos)
if [[ -z "$restore_sub" ]]; then
COMPREPLY=($(compgen -W "list diff to help" -- "$cur"))
else
case "$restore_sub" in
list)
COMPREPLY=($(compgen -W "-n --limit -h --help" -- "$cur"))
;;
diff)
COMPREPLY=($(compgen -W "-h --help" -- "$cur"))
;;
to)
COMPREPLY=($(compgen -W "--force -h --help" -- "$cur"))
;;
*) *)
COMPREPLY=($(compgen -W "-h --help" -- "$cur")) COMPREPLY=($(compgen -W "-h --help" -- "$cur"))
;; ;;

View File

@@ -4,7 +4,7 @@
# Erase any stale completions from previous versions # Erase any stale completions from previous versions
complete -c mcpctl -e complete -c mcpctl -e
set -l commands status login logout config get describe delete logs create edit apply patch backup restore approve console cache set -l commands status login logout config get describe delete logs create edit apply patch backup approve console cache
set -l project_commands get describe delete logs create edit attach-server detach-server set -l project_commands get describe delete logs create edit attach-server detach-server
# Disable file completions by default # Disable file completions by default
@@ -228,7 +228,6 @@ complete -c mcpctl -n "not __mcpctl_has_project; and not __fish_seen_subcommand_
complete -c mcpctl -n "not __mcpctl_has_project; and not __fish_seen_subcommand_from $commands" -a apply -d 'Apply declarative configuration from a YAML or JSON file' complete -c mcpctl -n "not __mcpctl_has_project; and not __fish_seen_subcommand_from $commands" -a apply -d 'Apply declarative configuration from a YAML or JSON file'
complete -c mcpctl -n "not __mcpctl_has_project; and not __fish_seen_subcommand_from $commands" -a patch -d 'Patch a resource field (e.g. mcpctl patch project myproj llmProvider=none)' complete -c mcpctl -n "not __mcpctl_has_project; and not __fish_seen_subcommand_from $commands" -a patch -d 'Patch a resource field (e.g. mcpctl patch project myproj llmProvider=none)'
complete -c mcpctl -n "not __mcpctl_has_project; and not __fish_seen_subcommand_from $commands" -a backup -d 'Git-based backup status and management' complete -c mcpctl -n "not __mcpctl_has_project; and not __fish_seen_subcommand_from $commands" -a backup -d 'Git-based backup status and management'
complete -c mcpctl -n "not __mcpctl_has_project; and not __fish_seen_subcommand_from $commands" -a restore -d 'Restore mcpctl state from backup history'
complete -c mcpctl -n "not __mcpctl_has_project; and not __fish_seen_subcommand_from $commands" -a approve -d 'Approve a pending prompt request (atomic: delete request, create prompt)' complete -c mcpctl -n "not __mcpctl_has_project; and not __fish_seen_subcommand_from $commands" -a approve -d 'Approve a pending prompt request (atomic: delete request, create prompt)'
complete -c mcpctl -n "not __mcpctl_has_project; and not __fish_seen_subcommand_from $commands" -a console -d 'Interactive MCP console — unified timeline with tools, provenance, and lab replay' complete -c mcpctl -n "not __mcpctl_has_project; and not __fish_seen_subcommand_from $commands" -a console -d 'Interactive MCP console — unified timeline with tools, provenance, and lab replay'
complete -c mcpctl -n "not __mcpctl_has_project; and not __fish_seen_subcommand_from $commands" -a cache -d 'Manage ProxyModel pipeline cache' complete -c mcpctl -n "not __mcpctl_has_project; and not __fish_seen_subcommand_from $commands" -a cache -d 'Manage ProxyModel pipeline cache'
@@ -354,25 +353,14 @@ complete -c mcpctl -n "__mcpctl_subcmd_active create promptrequest" -l content-f
complete -c mcpctl -n "__mcpctl_subcmd_active create promptrequest" -l priority -d 'Priority 1-10 (default: 5, higher = more important)' -x complete -c mcpctl -n "__mcpctl_subcmd_active create promptrequest" -l priority -d 'Priority 1-10 (default: 5, higher = more important)' -x
# backup subcommands # backup subcommands
set -l backup_cmds log key set -l backup_cmds log key restore
complete -c mcpctl -n "__fish_seen_subcommand_from backup; and not __fish_seen_subcommand_from $backup_cmds" -a log -d 'Show backup commit history' complete -c mcpctl -n "__fish_seen_subcommand_from backup; and not __fish_seen_subcommand_from $backup_cmds" -a log -d 'Show backup commit history'
complete -c mcpctl -n "__fish_seen_subcommand_from backup; and not __fish_seen_subcommand_from $backup_cmds" -a key -d 'Show SSH public key for deploy key setup' complete -c mcpctl -n "__fish_seen_subcommand_from backup; and not __fish_seen_subcommand_from $backup_cmds" -a key -d 'Show SSH public key for deploy key setup'
complete -c mcpctl -n "__fish_seen_subcommand_from backup; and not __fish_seen_subcommand_from $backup_cmds" -a restore -d 'Restore mcpctl state from backup history'
# backup log options # backup log options
complete -c mcpctl -n "__mcpctl_subcmd_active backup log" -s n -l limit -d 'number of commits to show' -x complete -c mcpctl -n "__mcpctl_subcmd_active backup log" -s n -l limit -d 'number of commits to show' -x
# restore subcommands
set -l restore_cmds list diff to
complete -c mcpctl -n "__fish_seen_subcommand_from restore; and not __fish_seen_subcommand_from $restore_cmds" -a list -d 'List available restore points'
complete -c mcpctl -n "__fish_seen_subcommand_from restore; and not __fish_seen_subcommand_from $restore_cmds" -a diff -d 'Preview what restoring to a commit would change'
complete -c mcpctl -n "__fish_seen_subcommand_from restore; and not __fish_seen_subcommand_from $restore_cmds" -a to -d 'Restore to a specific commit'
# restore list options
complete -c mcpctl -n "__mcpctl_subcmd_active restore list" -s n -l limit -d 'number of entries' -x
# restore to options
complete -c mcpctl -n "__mcpctl_subcmd_active restore to" -l force -d 'skip confirmation'
# cache subcommands # cache subcommands
set -l cache_cmds stats clear set -l cache_cmds stats clear
complete -c mcpctl -n "__fish_seen_subcommand_from cache; and not __fish_seen_subcommand_from $cache_cmds" -a stats -d 'Show cache statistics' complete -c mcpctl -n "__fish_seen_subcommand_from cache; and not __fish_seen_subcommand_from $cache_cmds" -a stats -d 'Show cache statistics'

View File

@@ -112,14 +112,11 @@ export function createBackupCommand(deps: BackupDeps): Command {
deps.log('Add this key as a deploy key (with write access) in your Git hosting provider.'); deps.log('Add this key as a deploy key (with write access) in your Git hosting provider.');
}); });
return cmd; // ── Restore subcommand group ──
} const restore = new Command('restore')
export function createRestoreCommand(deps: BackupDeps): Command {
const cmd = new Command('restore')
.description('Restore mcpctl state from backup history'); .description('Restore mcpctl state from backup history');
cmd restore
.command('list') .command('list')
.description('List available restore points') .description('List available restore points')
.option('-n, --limit <count>', 'number of entries', '30') .option('-n, --limit <count>', 'number of entries', '30')
@@ -156,7 +153,7 @@ export function createRestoreCommand(deps: BackupDeps): Command {
} }
}); });
cmd restore
.command('diff <commit>') .command('diff <commit>')
.description('Preview what restoring to a commit would change') .description('Preview what restoring to a commit would change')
.action(async (commit: string) => { .action(async (commit: string) => {
@@ -186,7 +183,7 @@ export function createRestoreCommand(deps: BackupDeps): Command {
deps.log(`Total: ${preview.added.length} added, ${preview.modified.length} modified, ${preview.removed.length} removed`); deps.log(`Total: ${preview.added.length} added, ${preview.modified.length} modified, ${preview.removed.length} removed`);
}); });
cmd restore
.command('to <commit>') .command('to <commit>')
.description('Restore to a specific commit') .description('Restore to a specific commit')
.option('--force', 'skip confirmation', false) .option('--force', 'skip confirmation', false)
@@ -236,6 +233,8 @@ export function createRestoreCommand(deps: BackupDeps): Command {
} }
}); });
cmd.addCommand(restore);
return cmd; return cmd;
} }

View File

@@ -10,7 +10,7 @@ import { createLogsCommand } from './commands/logs.js';
import { createApplyCommand } from './commands/apply.js'; import { createApplyCommand } from './commands/apply.js';
import { createCreateCommand } from './commands/create.js'; import { createCreateCommand } from './commands/create.js';
import { createEditCommand } from './commands/edit.js'; import { createEditCommand } from './commands/edit.js';
import { createBackupCommand, createRestoreCommand } from './commands/backup.js'; import { createBackupCommand } from './commands/backup.js';
import { createLoginCommand, createLogoutCommand } from './commands/auth.js'; import { createLoginCommand, createLogoutCommand } from './commands/auth.js';
import { createAttachServerCommand, createDetachServerCommand, createApproveCommand } from './commands/project-ops.js'; import { createAttachServerCommand, createDetachServerCommand, createApproveCommand } from './commands/project-ops.js';
import { createMcpCommand } from './commands/mcp.js'; import { createMcpCommand } from './commands/mcp.js';
@@ -191,11 +191,6 @@ export function createProgram(): Command {
log: (...args) => console.log(...args), log: (...args) => console.log(...args),
})); }));
program.addCommand(createRestoreCommand({
client,
log: (...args) => console.log(...args),
}));
const projectOpsDeps = { const projectOpsDeps = {
client, client,
log: (...args: string[]) => console.log(...args), log: (...args: string[]) => console.log(...args),

View File

@@ -1,5 +1,5 @@
import { describe, it, expect, vi, beforeEach } from 'vitest'; import { describe, it, expect, vi, beforeEach } from 'vitest';
import { createBackupCommand, createRestoreCommand } from '../../src/commands/backup.js'; import { createBackupCommand } from '../../src/commands/backup.js';
const mockClient = { const mockClient = {
get: vi.fn(), get: vi.fn(),
@@ -10,14 +10,17 @@ const mockClient = {
const log = vi.fn(); const log = vi.fn();
function makeCmd() {
return createBackupCommand({ client: mockClient as never, log });
}
describe('backup command', () => { describe('backup command', () => {
beforeEach(() => { beforeEach(() => {
vi.resetAllMocks(); vi.resetAllMocks();
}); });
it('creates backup command', () => { it('creates backup command', () => {
const cmd = createBackupCommand({ client: mockClient as never, log }); expect(makeCmd().name()).toBe('backup');
expect(cmd.name()).toBe('backup');
}); });
it('shows status when enabled', async () => { it('shows status when enabled', async () => {
@@ -31,8 +34,7 @@ describe('backup command', () => {
pendingCount: 0, pendingCount: 0,
}); });
const cmd = createBackupCommand({ client: mockClient as never, log }); await makeCmd().parseAsync([], { from: 'user' });
await cmd.parseAsync([], { from: 'user' });
expect(mockClient.get).toHaveBeenCalledWith('/api/v1/backup/status'); expect(mockClient.get).toHaveBeenCalledWith('/api/v1/backup/status');
expect(log).toHaveBeenCalledWith(expect.stringContaining('ssh://git@10.0.0.194:2222/michal/mcp-backup.git')); expect(log).toHaveBeenCalledWith(expect.stringContaining('ssh://git@10.0.0.194:2222/michal/mcp-backup.git'));
@@ -50,8 +52,7 @@ describe('backup command', () => {
pendingCount: 0, pendingCount: 0,
}); });
const cmd = createBackupCommand({ client: mockClient as never, log }); await makeCmd().parseAsync([], { from: 'user' });
await cmd.parseAsync([], { from: 'user' });
expect(log).toHaveBeenCalledWith(expect.stringContaining('disabled')); expect(log).toHaveBeenCalledWith(expect.stringContaining('disabled'));
}); });
@@ -67,8 +68,7 @@ describe('backup command', () => {
pendingCount: 5, pendingCount: 5,
}); });
const cmd = createBackupCommand({ client: mockClient as never, log }); await makeCmd().parseAsync([], { from: 'user' });
await cmd.parseAsync([], { from: 'user' });
expect(log).toHaveBeenCalledWith(expect.stringContaining('5 changes pending')); expect(log).toHaveBeenCalledWith(expect.stringContaining('5 changes pending'));
}); });
@@ -76,8 +76,7 @@ describe('backup command', () => {
it('shows SSH public key', async () => { it('shows SSH public key', async () => {
mockClient.get.mockResolvedValue({ publicKey: 'ssh-ed25519 AAAA... mcpd@mcpctl.local' }); mockClient.get.mockResolvedValue({ publicKey: 'ssh-ed25519 AAAA... mcpd@mcpctl.local' });
const cmd = createBackupCommand({ client: mockClient as never, log }); await makeCmd().parseAsync(['key'], { from: 'user' });
await cmd.parseAsync(['key'], { from: 'user' });
expect(mockClient.get).toHaveBeenCalledWith('/api/v1/backup/key'); expect(mockClient.get).toHaveBeenCalledWith('/api/v1/backup/key');
expect(log).toHaveBeenCalledWith('ssh-ed25519 AAAA... mcpd@mcpctl.local'); expect(log).toHaveBeenCalledWith('ssh-ed25519 AAAA... mcpd@mcpctl.local');
@@ -91,28 +90,20 @@ describe('backup command', () => {
], ],
}); });
const cmd = createBackupCommand({ client: mockClient as never, log }); await makeCmd().parseAsync(['log'], { from: 'user' });
await cmd.parseAsync(['log'], { from: 'user' });
expect(mockClient.get).toHaveBeenCalledWith('/api/v1/backup/log?limit=20'); expect(mockClient.get).toHaveBeenCalledWith('/api/v1/backup/log?limit=20');
// Header
expect(log).toHaveBeenCalledWith(expect.stringContaining('COMMIT')); expect(log).toHaveBeenCalledWith(expect.stringContaining('COMMIT'));
// Entries
expect(log).toHaveBeenCalledWith(expect.stringContaining('abc1234')); expect(log).toHaveBeenCalledWith(expect.stringContaining('abc1234'));
expect(log).toHaveBeenCalledWith(expect.stringContaining('[manual]')); expect(log).toHaveBeenCalledWith(expect.stringContaining('[manual]'));
}); });
}); });
describe('restore command', () => { describe('backup restore subcommands', () => {
beforeEach(() => { beforeEach(() => {
vi.resetAllMocks(); vi.resetAllMocks();
}); });
it('creates restore command', () => {
const cmd = createRestoreCommand({ client: mockClient as never, log });
expect(cmd.name()).toBe('restore');
});
it('lists restore points', async () => { it('lists restore points', async () => {
mockClient.get.mockResolvedValue({ mockClient.get.mockResolvedValue({
entries: [ entries: [
@@ -120,8 +111,7 @@ describe('restore command', () => {
], ],
}); });
const cmd = createRestoreCommand({ client: mockClient as never, log }); await makeCmd().parseAsync(['restore', 'list'], { from: 'user' });
await cmd.parseAsync(['list'], { from: 'user' });
expect(mockClient.get).toHaveBeenCalledWith('/api/v1/backup/log?limit=30'); expect(mockClient.get).toHaveBeenCalledWith('/api/v1/backup/log?limit=30');
expect(log).toHaveBeenCalledWith(expect.stringContaining('abc1234')); expect(log).toHaveBeenCalledWith(expect.stringContaining('abc1234'));
@@ -137,8 +127,7 @@ describe('restore command', () => {
modified: ['projects/default.yaml'], modified: ['projects/default.yaml'],
}); });
const cmd = createRestoreCommand({ client: mockClient as never, log }); await makeCmd().parseAsync(['restore', 'diff', 'abc1234'], { from: 'user' });
await cmd.parseAsync(['diff', 'abc1234'], { from: 'user' });
expect(mockClient.post).toHaveBeenCalledWith('/api/v1/backup/restore/preview', { commit: 'abc1234' }); expect(mockClient.post).toHaveBeenCalledWith('/api/v1/backup/restore/preview', { commit: 'abc1234' });
expect(log).toHaveBeenCalledWith(expect.stringContaining('+ servers/new.yaml')); expect(log).toHaveBeenCalledWith(expect.stringContaining('+ servers/new.yaml'));
@@ -156,17 +145,14 @@ describe('restore command', () => {
modified: [], modified: [],
}); });
const cmd = createRestoreCommand({ client: mockClient as never, log }); await makeCmd().parseAsync(['restore', 'to', 'abc1234'], { from: 'user' });
await cmd.parseAsync(['to', 'abc1234'], { from: 'user' });
// Should show preview but NOT call restore endpoint
expect(mockClient.post).toHaveBeenCalledWith('/api/v1/backup/restore/preview', { commit: 'abc1234' }); expect(mockClient.post).toHaveBeenCalledWith('/api/v1/backup/restore/preview', { commit: 'abc1234' });
expect(mockClient.post).not.toHaveBeenCalledWith('/api/v1/backup/restore', expect.anything()); expect(mockClient.post).not.toHaveBeenCalledWith('/api/v1/backup/restore', expect.anything());
expect(log).toHaveBeenCalledWith(expect.stringContaining('--force')); expect(log).toHaveBeenCalledWith(expect.stringContaining('--force'));
}); });
it('executes restore with --force', async () => { it('executes restore with --force', async () => {
// First call: preview, second call: restore
mockClient.post mockClient.post
.mockResolvedValueOnce({ .mockResolvedValueOnce({
targetCommit: 'abc1234567890', targetCommit: 'abc1234567890',
@@ -183,8 +169,7 @@ describe('restore command', () => {
errors: [], errors: [],
}); });
const cmd = createRestoreCommand({ client: mockClient as never, log }); await makeCmd().parseAsync(['restore', 'to', 'abc1234', '--force'], { from: 'user' });
await cmd.parseAsync(['to', 'abc1234', '--force'], { from: 'user' });
expect(mockClient.post).toHaveBeenCalledWith('/api/v1/backup/restore', { commit: 'abc1234' }); expect(mockClient.post).toHaveBeenCalledWith('/api/v1/backup/restore', { commit: 'abc1234' });
expect(log).toHaveBeenCalledWith(expect.stringContaining('1 applied')); expect(log).toHaveBeenCalledWith(expect.stringContaining('1 applied'));
@@ -208,8 +193,7 @@ describe('restore command', () => {
errors: ['Failed to apply servers/broken.yaml: invalid YAML'], errors: ['Failed to apply servers/broken.yaml: invalid YAML'],
}); });
const cmd = createRestoreCommand({ client: mockClient as never, log }); await makeCmd().parseAsync(['restore', 'to', 'abc1234', '--force'], { from: 'user' });
await cmd.parseAsync(['to', 'abc1234', '--force'], { from: 'user' });
expect(log).toHaveBeenCalledWith('Errors:'); expect(log).toHaveBeenCalledWith('Errors:');
expect(log).toHaveBeenCalledWith(expect.stringContaining('invalid YAML')); expect(log).toHaveBeenCalledWith(expect.stringContaining('invalid YAML'));

View File

@@ -119,7 +119,7 @@ describe('fish completions', () => {
}); });
it('non-project commands do not show with --project', () => { it('non-project commands do not show with --project', () => {
const nonProjectCmds = ['status', 'login', 'logout', 'config', 'apply', 'backup', 'restore']; const nonProjectCmds = ['status', 'login', 'logout', 'config', 'apply', 'backup'];
const lines = fishFile.split('\n').filter((l) => l.startsWith('complete') && l.includes('-a ')); const lines = fishFile.split('\n').filter((l) => l.startsWith('complete') && l.includes('-a '));
for (const cmd of nonProjectCmds) { for (const cmd of nonProjectCmds) {

View File

@@ -22,7 +22,6 @@ describe('CLI command registration (e2e)', () => {
expect(commandNames).toContain('create'); expect(commandNames).toContain('create');
expect(commandNames).toContain('edit'); expect(commandNames).toContain('edit');
expect(commandNames).toContain('backup'); expect(commandNames).toContain('backup');
expect(commandNames).toContain('restore');
}); });
it('old project and claude top-level commands are removed', () => { it('old project and claude top-level commands are removed', () => {