Merge pull request 'fix: error handling and --force flag for create commands' (#10) from fix/create-error-handling into main
This commit is contained in:
@@ -1,5 +1,5 @@
|
|||||||
import { Command } from 'commander';
|
import { Command } from 'commander';
|
||||||
import type { ApiClient } from '../api-client.js';
|
import { type ApiClient, ApiError } from '../api-client.js';
|
||||||
export interface CreateCommandDeps {
|
export interface CreateCommandDeps {
|
||||||
client: ApiClient;
|
client: ApiClient;
|
||||||
log: (...args: unknown[]) => void;
|
log: (...args: unknown[]) => void;
|
||||||
@@ -72,6 +72,7 @@ export function createCreateCommand(deps: CreateCommandDeps): Command {
|
|||||||
.option('--replicas <count>', 'Number of replicas')
|
.option('--replicas <count>', 'Number of replicas')
|
||||||
.option('--env <entry>', 'Env var: KEY=value (inline) or KEY=secretRef:SECRET:KEY (secret ref, repeat for multiple)', collect, [])
|
.option('--env <entry>', 'Env var: KEY=value (inline) or KEY=secretRef:SECRET:KEY (secret ref, repeat for multiple)', collect, [])
|
||||||
.option('--from-template <name>', 'Create from template (name or name:version)')
|
.option('--from-template <name>', 'Create from template (name or name:version)')
|
||||||
|
.option('--force', 'Update if already exists')
|
||||||
.action(async (name: string, opts) => {
|
.action(async (name: string, opts) => {
|
||||||
let base: Record<string, unknown> = {};
|
let base: Record<string, unknown> = {};
|
||||||
|
|
||||||
@@ -92,9 +93,12 @@ export function createCreateCommand(deps: CreateCommandDeps): Command {
|
|||||||
if (!template) throw new Error(`Template '${tplName}' not found`);
|
if (!template) throw new Error(`Template '${tplName}' not found`);
|
||||||
}
|
}
|
||||||
|
|
||||||
// Copy template fields as base (strip template-only fields)
|
// Copy template fields as base (strip template-only, internal, and null fields)
|
||||||
const { id: _id, createdAt: _c, updatedAt: _u, ...tplFields } = template;
|
const { id: _id, createdAt: _c, updatedAt: _u, version: _v, name: _n, ...tplFields } = template;
|
||||||
base = { ...tplFields };
|
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)
|
// 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;
|
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;
|
if (!body.replicas) body.replicas = 1;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
try {
|
||||||
const server = await client.post<{ id: string; name: string }>('/api/v1/servers', body);
|
const server = await client.post<{ id: string; name: string }>('/api/v1/servers', body);
|
||||||
log(`server '${server.name}' created (id: ${server.id})`);
|
log(`server '${server.name}' created (id: ${server.id})`);
|
||||||
|
} catch (err) {
|
||||||
|
if (err instanceof ApiError && err.status === 409 && opts.force) {
|
||||||
|
const existing = (await client.get<Array<{ id: string; name: string }>>('/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 ---
|
// --- create secret ---
|
||||||
@@ -153,13 +169,25 @@ export function createCreateCommand(deps: CreateCommandDeps): Command {
|
|||||||
.description('Create a secret')
|
.description('Create a secret')
|
||||||
.argument('<name>', 'Secret name (lowercase, hyphens allowed)')
|
.argument('<name>', 'Secret name (lowercase, hyphens allowed)')
|
||||||
.option('--data <entry>', 'Secret data KEY=value (repeat for multiple)', collect, [])
|
.option('--data <entry>', 'Secret data KEY=value (repeat for multiple)', collect, [])
|
||||||
|
.option('--force', 'Update if already exists')
|
||||||
.action(async (name: string, opts) => {
|
.action(async (name: string, opts) => {
|
||||||
const data = parseEnvEntries(opts.data);
|
const data = parseEnvEntries(opts.data);
|
||||||
|
try {
|
||||||
const secret = await client.post<{ id: string; name: string }>('/api/v1/secrets', {
|
const secret = await client.post<{ id: string; name: string }>('/api/v1/secrets', {
|
||||||
name,
|
name,
|
||||||
data,
|
data,
|
||||||
});
|
});
|
||||||
log(`secret '${secret.name}' created (id: ${secret.id})`);
|
log(`secret '${secret.name}' created (id: ${secret.id})`);
|
||||||
|
} catch (err) {
|
||||||
|
if (err instanceof ApiError && err.status === 409 && opts.force) {
|
||||||
|
const existing = (await client.get<Array<{ id: string; name: string }>>('/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 ---
|
// --- create project ---
|
||||||
@@ -167,12 +195,24 @@ export function createCreateCommand(deps: CreateCommandDeps): Command {
|
|||||||
.description('Create a project')
|
.description('Create a project')
|
||||||
.argument('<name>', 'Project name')
|
.argument('<name>', 'Project name')
|
||||||
.option('-d, --description <text>', 'Project description', '')
|
.option('-d, --description <text>', 'Project description', '')
|
||||||
|
.option('--force', 'Update if already exists')
|
||||||
.action(async (name: string, opts) => {
|
.action(async (name: string, opts) => {
|
||||||
|
try {
|
||||||
const project = await client.post<{ id: string; name: string }>('/api/v1/projects', {
|
const project = await client.post<{ id: string; name: string }>('/api/v1/projects', {
|
||||||
name,
|
name,
|
||||||
description: opts.description,
|
description: opts.description,
|
||||||
});
|
});
|
||||||
log(`project '${project.name}' created (id: ${project.id})`);
|
log(`project '${project.name}' created (id: ${project.id})`);
|
||||||
|
} catch (err) {
|
||||||
|
if (err instanceof ApiError && err.status === 409 && opts.force) {
|
||||||
|
const existing = (await client.get<Array<{ id: string; name: string }>>('/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;
|
return cmd;
|
||||||
|
|||||||
@@ -14,7 +14,7 @@ import { createClaudeCommand } from './commands/claude.js';
|
|||||||
import { createProjectCommand } from './commands/project.js';
|
import { createProjectCommand } from './commands/project.js';
|
||||||
import { createBackupCommand, createRestoreCommand } from './commands/backup.js';
|
import { createBackupCommand, createRestoreCommand } from './commands/backup.js';
|
||||||
import { createLoginCommand, createLogoutCommand } from './commands/auth.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 { loadConfig } from './config/index.js';
|
||||||
import { loadCredentials } from './auth/index.js';
|
import { loadCredentials } from './auth/index.js';
|
||||||
import { resolveNameOrId } from './commands/shared.js';
|
import { resolveNameOrId } from './commands/shared.js';
|
||||||
@@ -143,5 +143,21 @@ const isDirectRun =
|
|||||||
import.meta.url === `file://${process.argv[1]}`;
|
import.meta.url === `file://${process.argv[1]}`;
|
||||||
|
|
||||||
if (isDirectRun) {
|
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);
|
||||||
|
});
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||||
import { createCreateCommand } from '../../src/commands/create.js';
|
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 {
|
function mockClient(): ApiClient {
|
||||||
return {
|
return {
|
||||||
@@ -73,6 +73,59 @@ describe('create command', () => {
|
|||||||
transport: 'STDIO',
|
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<string, unknown>;
|
||||||
|
// 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', () => {
|
describe('create secret', () => {
|
||||||
@@ -98,6 +151,21 @@ describe('create command', () => {
|
|||||||
data: {},
|
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', () => {
|
describe('create project', () => {
|
||||||
@@ -119,5 +187,14 @@ describe('create command', () => {
|
|||||||
description: '',
|
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");
|
||||||
|
});
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user