feat(cli): introduce events commands - #97
Conversation
| const result = await mcpCallTool(serverOpts(token), 'listEventDefinitions', { | ||
| pageToken: opts?.pageToken, | ||
| }); | ||
| return extractText(result); |
There was a problem hiding this comment.
mcpCallTool returns isError: true results without throwing, and extractText just returns the error text. A server-side failure (e.g. get missing-event) is printed as success and exits 0. Check result.isError and throw (applies to all six wrappers).
| if (format === 'json') { | ||
| message(formatJson(text)); |
There was a problem hiding this comment.
formatJson(text) is given a string, so --json likely prints an escaped string literal instead of a JSON object (breaks \| jq). Parse first, e.g. parseToolJson. Same in get/create/update/usage.
| const confirmed = await confirm({ | ||
| message: `Delete event definition "${name}"? This cannot be undone.`, | ||
| default: false, | ||
| }); |
There was a problem hiding this comment.
confirm() runs unconditionally: it throws or hangs without a TTY (CI/pipes), and --dry-run is ignored. Add --yes, fail when non-interactive without it, and respect --dry-run.
| const schemaKey = `${type}Schema`; | ||
| return [name, { [schemaKey]: {} }]; |
There was a problem hiding this comment.
Field type isn't validated: amount:dobule sends { dobuleSchema: {} }. Check against an allow-list of supported types.
| const fieldSpecs = (argv.field as string[] | undefined) ?? []; | ||
| schema = Object.fromEntries(fieldSpecs.map(parseFieldArg)); |
There was a problem hiding this comment.
No --field and no --from-file creates an event with an empty schema. update rejects this case, so create should too.
| "@features/*": ["./src/features/*"], | ||
| "@output/*": ["./src/output/*"], | ||
| "@api/*": ["./src/api/*"] | ||
| "@network/*": ["./src/network/*"] |
There was a problem hiding this comment.
@api/* was renamed to @network/*. CLAUDE.md and the cli skill still list @api/*.
| ): (argv: Record<string, unknown>) => Promise<void> { | ||
| return async (argv) => { | ||
| try { | ||
| await fn(argv as Record<string, unknown>); |
There was a problem hiding this comment.
Redundant cast. Also, every handler already catches its own errors, so safely only covers requireAuth throwing.
There was a problem hiding this comment.
Yes, but safely also formats the error message as fail call. I'd say the unification is worth it, especially since an additional try-catch doesn't really affect the performance that much
There was a problem hiding this comment.
Fair point. As a catch-all it guarantees consistent fail output and exit code for anything that slips past the handler's own try/catch, so it's worth keeping. The only remaining nit is the redundant argv as Record<string, unknown> cast; feel free to ignore.
| import { formatJson } from '@output/json.js'; | ||
| import { resolveFormat } from '@output/detect.js'; | ||
| import { requireAuth } from './require-auth.js'; | ||
| import { parseFieldArg } from './create.js'; |
There was a problem hiding this comment.
Importing parseFieldArg from create.ts. Move it to its own field-spec.ts.
| const format = resolveFormat(extractFlags(argv)); | ||
| if (format === 'json') { | ||
| message(formatJson(text)); | ||
| } else { | ||
| message(text); | ||
| } |
There was a problem hiding this comment.
This format/print block is copy-pasted five times. Extract a helper like printToolText(argv, text).
| ) | ||
| .demandCommand( | ||
| 1, | ||
| 'Available actions: setup, list, get, create, update, delete, usage. Run "confidence events --help" for details.', |
There was a problem hiding this comment.
The action list is hand-maintained and will drift as actions are added.
extractText ignored CallToolResult.isError, silently returning error text as a successful response. Commands like `events get missing-event` would print the error message to stdout and exit 0. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Network wrappers now return raw CallToolResult, and a shared printMcpResult helper handles format-aware output. For --json, the MCP text is parsed so the envelope contains a real object instead of an escaped string literal. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add --force/-f to skip confirmation, fail with a clear message when stdin is not a TTY and --force is absent, and respect --dry-run by printing the request body without calling the server. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Reject unknown types like "dobule" with a clear error listing the supported types (string, int, double, bool, struct). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Without --field or --from-file the command silently sent an empty schema. Now it fails early, matching the update command's behavior. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Reject --days outside 1-7 in events usage. Extract isDefined type guard into shared-kernel to replace loose/truthiness undefined checks. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The MCP client sent a hard-coded '1.0.0' version. Add clientVersion to McpClientOptions and pass the CLI package version so server-side telemetry reflects the actual release. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Moves the field parsing logic out of create.ts into its own module so both create and update import from a shared location instead of update reaching into create. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Yargs already prints available commands on missing action, so the hard-coded lists in events and mcp were redundant and prone to drift. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Thanks for the review, @filipmyllari! Updated the code according to most of the comments, left responses to some 🙌 |
confidence events list|get|create|update|delete|usagepackages/core/src/mcp/using@modelcontextprotocol/sdk, reusable for future command groupspackages/core/src/api/(unused by events, ready for future commands)confidence-flagsMCP server tools (listEventDefinitions,getEventDefinition,createEventDefinition,updateEventDefinition,deleteEventDefinition,queryEventsUsage)deleteprompts for confirmation via@inquirer/confirmpackages/testing/src/msw/handlers/extractRegionfrom core's auth barrelNoticed that REST for events isn't supported yet, so had to introduce the MCP client in
core, but I decided to keep the REST client in this PR as well (as it will be needed for other features anyways).Commands
JSON output
--jsonwraps the MCP server's text response in a{ "data": "..." }envelope. The output is human-readable text, not structured JSON, because the MCP server returns text formatted for AI agents. Structured JSON output might become available if a REST API is added.