From 4e3d896ef62c98db1cd0f9348667eccf3f80f6e7 Mon Sep 17 00:00:00 2001 From: Michal Date: Sun, 22 Feb 2026 23:06:33 +0000 Subject: [PATCH] fix: proper error handling and --force flag for create commands - Add global error handler: clean messages instead of stack traces - Add --force flag to create server/secret/project: updates on 409 conflict - Strip null values and template-only fields from --from-template payload - Add tests: 409 handling, --force update, null-stripping from templates Co-Authored-By: Claude Opus 4.6 --- src/cli/src/commands/create.ts | 72 ++++++++++++++++++------ src/cli/src/index.ts | 20 ++++++- src/cli/tests/commands/create.test.ts | 79 ++++++++++++++++++++++++++- 3 files changed, 152 insertions(+), 19 deletions(-) diff --git a/src/cli/src/commands/create.ts b/src/cli/src/commands/create.ts index 4344c8e..d705abb 100644 --- a/src/cli/src/commands/create.ts +++ b/src/cli/src/commands/create.ts @@ -1,5 +1,5 @@ import { Command } from 'commander'; -import type { ApiClient } from '../api-client.js'; +import { type ApiClient, ApiError } from '../api-client.js'; export interface CreateCommandDeps { client: ApiClient; log: (...args: unknown[]) => void; @@ -72,6 +72,7 @@ export function createCreateCommand(deps: CreateCommandDeps): Command { .option('--replicas ', 'Number of replicas') .option('--env ', 'Env var: KEY=value (inline) or KEY=secretRef:SECRET:KEY (secret ref, repeat for multiple)', collect, []) .option('--from-template ', 'Create from template (name or name:version)') + .option('--force', 'Update if already exists') .action(async (name: string, opts) => { let base: Record = {}; @@ -92,9 +93,12 @@ export function createCreateCommand(deps: CreateCommandDeps): Command { if (!template) throw new Error(`Template '${tplName}' not found`); } - // Copy template fields as base (strip template-only fields) - const { id: _id, createdAt: _c, updatedAt: _u, ...tplFields } = template; - base = { ...tplFields }; + // Copy template fields as base (strip template-only, internal, and null fields) + const { id: _id, createdAt: _c, updatedAt: _u, version: _v, name: _n, ...tplFields } = template; + base = {}; + for (const [k, v] of Object.entries(tplFields)) { + if (v !== null && v !== undefined) base[k] = v; + } // Convert template env (description/required) to server env (name/value/valueFrom) const tplEnv = template.env as Array<{ name: string; description?: string; required?: boolean; defaultValue?: string }> | undefined; @@ -144,8 +148,20 @@ export function createCreateCommand(deps: CreateCommandDeps): Command { if (!body.replicas) body.replicas = 1; } - const server = await client.post<{ id: string; name: string }>('/api/v1/servers', body); - log(`server '${server.name}' created (id: ${server.id})`); + try { + const server = await client.post<{ id: string; name: string }>('/api/v1/servers', body); + log(`server '${server.name}' created (id: ${server.id})`); + } catch (err) { + if (err instanceof ApiError && err.status === 409 && opts.force) { + const existing = (await client.get>('/api/v1/servers')).find((s) => s.name === name); + if (!existing) throw err; + const { name: _n, ...updateBody } = body; + await client.put(`/api/v1/servers/${existing.id}`, updateBody); + log(`server '${name}' updated (id: ${existing.id})`); + } else { + throw err; + } + } }); // --- create secret --- @@ -153,13 +169,25 @@ export function createCreateCommand(deps: CreateCommandDeps): Command { .description('Create a secret') .argument('', 'Secret name (lowercase, hyphens allowed)') .option('--data ', 'Secret data KEY=value (repeat for multiple)', collect, []) + .option('--force', 'Update if already exists') .action(async (name: string, opts) => { const data = parseEnvEntries(opts.data); - const secret = await client.post<{ id: string; name: string }>('/api/v1/secrets', { - name, - data, - }); - log(`secret '${secret.name}' created (id: ${secret.id})`); + try { + const secret = await client.post<{ id: string; name: string }>('/api/v1/secrets', { + name, + data, + }); + log(`secret '${secret.name}' created (id: ${secret.id})`); + } catch (err) { + if (err instanceof ApiError && err.status === 409 && opts.force) { + const existing = (await client.get>('/api/v1/secrets')).find((s) => s.name === name); + if (!existing) throw err; + await client.put(`/api/v1/secrets/${existing.id}`, { data }); + log(`secret '${name}' updated (id: ${existing.id})`); + } else { + throw err; + } + } }); // --- create project --- @@ -167,12 +195,24 @@ export function createCreateCommand(deps: CreateCommandDeps): Command { .description('Create a project') .argument('', 'Project name') .option('-d, --description ', 'Project description', '') + .option('--force', 'Update if already exists') .action(async (name: string, opts) => { - const project = await client.post<{ id: string; name: string }>('/api/v1/projects', { - name, - description: opts.description, - }); - log(`project '${project.name}' created (id: ${project.id})`); + try { + const project = await client.post<{ id: string; name: string }>('/api/v1/projects', { + name, + description: opts.description, + }); + log(`project '${project.name}' created (id: ${project.id})`); + } catch (err) { + if (err instanceof ApiError && err.status === 409 && opts.force) { + const existing = (await client.get>('/api/v1/projects')).find((p) => p.name === name); + if (!existing) throw err; + await client.put(`/api/v1/projects/${existing.id}`, { description: opts.description }); + log(`project '${name}' updated (id: ${existing.id})`); + } else { + throw err; + } + } }); return cmd; diff --git a/src/cli/src/index.ts b/src/cli/src/index.ts index dd8afec..48fae67 100644 --- a/src/cli/src/index.ts +++ b/src/cli/src/index.ts @@ -14,7 +14,7 @@ import { createClaudeCommand } from './commands/claude.js'; import { createProjectCommand } from './commands/project.js'; import { createBackupCommand, createRestoreCommand } from './commands/backup.js'; import { createLoginCommand, createLogoutCommand } from './commands/auth.js'; -import { ApiClient } from './api-client.js'; +import { ApiClient, ApiError } from './api-client.js'; import { loadConfig } from './config/index.js'; import { loadCredentials } from './auth/index.js'; import { resolveNameOrId } from './commands/shared.js'; @@ -143,5 +143,21 @@ const isDirectRun = import.meta.url === `file://${process.argv[1]}`; if (isDirectRun) { - createProgram().parseAsync(process.argv); + createProgram().parseAsync(process.argv).catch((err: unknown) => { + if (err instanceof ApiError) { + let msg: string; + try { + const parsed = JSON.parse(err.body) as { error?: string; message?: string }; + msg = parsed.error ?? parsed.message ?? err.body; + } catch { + msg = err.body; + } + console.error(`Error: ${msg}`); + } else if (err instanceof Error) { + console.error(`Error: ${err.message}`); + } else { + console.error(`Error: ${String(err)}`); + } + process.exit(1); + }); } diff --git a/src/cli/tests/commands/create.test.ts b/src/cli/tests/commands/create.test.ts index ad214f2..f2b091f 100644 --- a/src/cli/tests/commands/create.test.ts +++ b/src/cli/tests/commands/create.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import { createCreateCommand } from '../../src/commands/create.js'; -import type { ApiClient } from '../../src/api-client.js'; +import { type ApiClient, ApiError } from '../../src/api-client.js'; function mockClient(): ApiClient { return { @@ -73,6 +73,59 @@ describe('create command', () => { transport: 'STDIO', })); }); + + it('strips null values from template when using --from-template', async () => { + vi.mocked(client.get).mockResolvedValueOnce([{ + id: 'tpl-1', + name: 'grafana', + version: '1.0.0', + description: 'Grafana MCP', + packageName: '@leval/mcp-grafana', + dockerImage: null, + transport: 'STDIO', + repositoryUrl: 'https://github.com/test', + externalUrl: null, + command: null, + containerPort: null, + replicas: 1, + env: [{ name: 'TOKEN', required: true, description: 'A token' }], + healthCheck: { tool: 'test', arguments: {} }, + createdAt: '2025-01-01', + updatedAt: '2025-01-01', + }] as never); + const cmd = createCreateCommand({ client, log }); + await cmd.parseAsync([ + 'server', 'my-grafana', '--from-template=grafana', + '--env', 'TOKEN=secretRef:creds:TOKEN', + ], { from: 'user' }); + const call = vi.mocked(client.post).mock.calls[0]![1] as Record; + // null fields from template should NOT be in the body + expect(call).not.toHaveProperty('dockerImage'); + expect(call).not.toHaveProperty('externalUrl'); + expect(call).not.toHaveProperty('command'); + expect(call).not.toHaveProperty('containerPort'); + // non-null fields should be present + expect(call.packageName).toBe('@leval/mcp-grafana'); + expect(call.healthCheck).toEqual({ tool: 'test', arguments: {} }); + expect(call.templateName).toBe('grafana'); + }); + + it('throws on 409 without --force', async () => { + vi.mocked(client.post).mockRejectedValueOnce(new ApiError(409, '{"error":"Server already exists: my-server"}')); + const cmd = createCreateCommand({ client, log }); + await expect(cmd.parseAsync(['server', 'my-server'], { from: 'user' })).rejects.toThrow('API error 409'); + }); + + it('updates existing server on 409 with --force', async () => { + vi.mocked(client.post).mockRejectedValueOnce(new ApiError(409, '{"error":"Server already exists"}')); + vi.mocked(client.get).mockResolvedValueOnce([{ id: 'srv-1', name: 'my-server' }] as never); + const cmd = createCreateCommand({ client, log }); + await cmd.parseAsync(['server', 'my-server', '--force'], { from: 'user' }); + expect(client.put).toHaveBeenCalledWith('/api/v1/servers/srv-1', expect.objectContaining({ + transport: 'STDIO', + })); + expect(output.join('\n')).toContain("server 'my-server' updated"); + }); }); describe('create secret', () => { @@ -98,6 +151,21 @@ describe('create command', () => { data: {}, }); }); + + it('throws on 409 without --force', async () => { + vi.mocked(client.post).mockRejectedValueOnce(new ApiError(409, '{"error":"Secret already exists: my-creds"}')); + const cmd = createCreateCommand({ client, log }); + await expect(cmd.parseAsync(['secret', 'my-creds', '--data', 'KEY=val'], { from: 'user' })).rejects.toThrow('API error 409'); + }); + + it('updates existing secret on 409 with --force', async () => { + vi.mocked(client.post).mockRejectedValueOnce(new ApiError(409, '{"error":"Secret already exists"}')); + vi.mocked(client.get).mockResolvedValueOnce([{ id: 'sec-1', name: 'my-creds' }] as never); + const cmd = createCreateCommand({ client, log }); + await cmd.parseAsync(['secret', 'my-creds', '--data', 'KEY=val', '--force'], { from: 'user' }); + expect(client.put).toHaveBeenCalledWith('/api/v1/secrets/sec-1', { data: { KEY: 'val' } }); + expect(output.join('\n')).toContain("secret 'my-creds' updated"); + }); }); describe('create project', () => { @@ -119,5 +187,14 @@ describe('create command', () => { description: '', }); }); + + it('updates existing project on 409 with --force', async () => { + vi.mocked(client.post).mockRejectedValueOnce(new ApiError(409, '{"error":"Project already exists"}')); + vi.mocked(client.get).mockResolvedValueOnce([{ id: 'proj-1', name: 'my-proj' }] as never); + const cmd = createCreateCommand({ client, log }); + await cmd.parseAsync(['project', 'my-proj', '-d', 'updated', '--force'], { from: 'user' }); + expect(client.put).toHaveBeenCalledWith('/api/v1/projects/proj-1', { description: 'updated' }); + expect(output.join('\n')).toContain("project 'my-proj' updated"); + }); }); });