Repository navigation
Fix inconsistencies across the CLI commands and MCP tools - #282
Draft
nathanjcochran wants to merge 26 commits into
Draft
nathanjcochran wants to merge 26 commits into
nathanjcochran wants to merge 26 commits into
Conversation
- service_rename trims the new name and refuses an empty one, as the CLI's service rename does. Its message names the service and its new name instead of saying it was renamed successfully. - service_list treats a 200 without a body as an error, as the CLI's service list does, instead of returning an empty list. - The warnings in service create, service fork, and the service output's connection details start lowercase, like every other warning.
Only service create rejected a zero or negative --wait-timeout. The other commands that wait (service fork, start, stop, resize, and branch create, start, stop, resize) now share the same check, validateWaitTimeout.
A 200 with no body indicates a bug, and most commands and tools already report it as "empty response from API". The rest now do too: - The service_backup_list, service_metrics_available, and service_metrics_series tools returned an empty list instead. - tiger project use and the project selection in tiger auth login reported it as an "unknown error", since they checked the body rather than the status code. - tiger db psql treated it as a service with no read replicas. - The wait loop and the log fetcher used their own wording for it. A JSON null list is still an empty list, and tiger service metrics available and series now print it as [] rather than null, as the other list commands do.
CLAUDE.md reserves fmt.Errorf for errors that need format verbs, but about a hundred static error strings still went through it.
--output rejects formats a command doesn't support, but TIGER_OUTPUT and the config file aren't validated against the command. Most commands then fell through to the table, but service list, project list, service backup list, and service backup region list/add refused env with an error. They now fall through to the table like the rest.
The same flags were described several different ways across commands. Every --output now lists its formats as json, yaml, any extra, then table, and every --no-wait and --wait-timeout shares one description. tiger service logs listed a text format that --output rejects; it now lists table, which it accepts.
The service_logs tool's schema requires a tail of at least 1 and a non-negative node, but the CLI accepted anything: a negative --tail panicked slicing the fetched entries, --tail 0 printed nothing, and a negative --node went to the API. They're now rejected up front.
The output schema said each line starts with an RFC3339 timestamp, but the tool writes "2006-01-02 15:04:05 UTC", and only for entries that have a timestamp.
The tools' messages follow the same wording rules as the CLI. service_fork now ends with "Service is ready.", as service_create and both CLI commands do, and service_resize's accepted message starts "Service resize request accepted", like service_start's and service_stop's.
With --no-wait, tiger branch start, stop, and resize suggested 'tiger branch get', which can't work: a branch command has no default to fall back on. The service commands suggested 'tiger service get' or 'tiger service list', which only find the service if it's the default or in a short list. Every hint now names the ID, as tiger branch create's already did, in the same wording.
Every tool logged a line at Info level for each call, and a few more on success, though a default tiger mcp start is meant to be silent. Failures that would otherwise be swallowed are still logged.
It only printed help, which cobra already does for a group command with no Run, and it was the one command using Run rather than RunE.
Most output switches matched on strings.ToLower(cfg.Output), but version, db query, db extensions list, config list, and mcp list/get matched it as given, so TIGER_OUTPUT=JSON printed JSON from some commands and a table from others.
tiger project list rendered an empty table when there were no projects, where the other list commands print a message to stderr.
Each prompt that needs a terminal refused without one in its own words. They now all say what couldn't be prompted for and then how to avoid the prompt. tiger auth login also mentions --project-id when it can't prompt for a project, and tiger db psql names tiger db save-password when it can't prompt for a password.
Most commands checked their flags first, but service resize, branch create, branch resize, the update-password commands, db query, and db ping loaded the config and client (and some resolved the service) before checking flag values that need neither, and db create role only checked its --from tsdbadmin restrictions after connecting to the database. They now all check flags first.
#241 settled on single quotes for service names and IDs, keeping %q for opaque values, but the MCP tool messages and elicitation prompts and a few shared errors and warnings still used %q. They now single-quote too.
…rsion
Renovate's Dockerfile manager already reads golang:${GO_VERSION} from the
ARG's default, so the dockerfileVersions custom manager and its # renovate:
comment extracted the same dependency a second time. A local dry run finds
golang 1.27 in the Dockerfile with or without them.
The guide is for any coding agent, not one in particular. References to it in the docs and code comments now point at AGENTS.md, and the development guide's pointer to a "Unit Test Pattern" section that no longer exists now points at the Testing section.
nathanjcochran
changed the title
Cobra only reports an unknown subcommand on the root command. A group command with no Run printed its help instead and exited 0, so a typo like 'tiger service lsit' went unnoticed by scripts, while tiger mcp errored because it alone had a Run. Every group command now sets Run: groupRun, which prints its help, and Args: groupArgs, which reports an unknown subcommand with cobra's suggestions. Run rather than RunE keeps them out of the per-invocation lifecycle, so printing help still loads nothing.
Each group command gets the same three cases TestServiceCmd has: help with no subcommand, without loading anything, and an unknown subcommand with and without suggestions.
It set SilenceErrors on its command struct, since the server logs its own failures, but that also hid every error before the server started: an unknown flag or argument, or a config that failed to load, exited 1 with no output. It now sets SilenceErrors in RunE, just before starting the server, as the other commands that report their own errors do.
tiger mcp start runs on its own and has subcommands, but used cobra.NoArgs, so 'tiger mcp start stdoi' didn't suggest stdio as a group command would. It now uses groupArgs too.
Every API call repeats the same checks after calling the generated client: the request error, the status code, and a nil body. JSON200, JSON201, and JSON202 do all three for the status their endpoint returns and hand back the body; Status202 and Status204 do the first two for calls whose body isn't needed.
Every command and tool that calls the Tiger Cloud API now checks the response with common.JSON2xx or common.Status2xx instead of by hand, and the wrappers that only existed to do those checks (common.GetService, common.ListBranches, common.CountBranches, and fetchReplicaSets in db psql) are inlined at their callers. API errors and missing bodies are reported as before; a failed request now reads "failed to call API: ..." rather than naming the operation, which the client's error already does with the method and URL. The resolve helpers keep their own checks, since they turn a 404 into a "not found" error and add a hint to a 400, as do the wait loop, which keeps polling through errors, and API key validation.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes inconsistencies across the commands and MCP tools, where some followed the conventions in the agent guide and others didn't, and tidies up a few related things.
Behavior
service_renametrims the new name and refuses an empty one, as the CLI'stiger service renamedoes.common.JSON200/JSON201/JSON202andcommon.Status202/Status204take both return values of a client call and check the request error, the expected status, and the body, replacing the hand-written checks at every call site and the wrappers that only did those checks (common.GetService,common.ListBranches,common.CountBranches). A failed request now reads "failed to call API: …" (the client's error names the method and URL) rather than naming the operation; API errors read as before. The resolve helpers, the wait loop, and API key validation keep their own checks, since they act on particular statuses.service_list,service_backup_list, and the metrics tools returned an empty list;tiger project useand the project selection intiger auth loginreported an "unknown error";tiger db psqltreated it as no read replicas; and the wait loop and log fetcher used their own wording. A JSONnulllist is still an empty list, andtiger service metrics available/seriesnow print it as[]rather thannull.--wait-timeoutmust be positive on every command that waits, not justtiger service create.tiger service logsrejects a--tailbelow 1 (a negative one panicked) and a negative--node, as theservice_logstool does.tiger db create rolechecks its--from tsdbadminrestrictions up front rather than after connecting to the database.TIGER_OUTPUTor the config file, such asenvon a list command) falls through to the table rather than erroring on a few commands.tiger project listprints "No projects found." for an empty list, like the other list commands.tiger service lsit→ "Did you mean this? list"), rather than printing help and exiting 0. Cobra only does this on its own for the root command; onlytiger mcpdid it before.tiger mcp start, which runs on its own but also has subcommands, gives the same suggestions. With no subcommand they still print their help, without loading the config. Every group command now has tests for this.tiger mcp start httpprints errors that come before the server starts (an unknown flag or argument, a config that fails to load), which it silently exited 1 on before. Only the server's own failures, which it logs, are left unprinted.Wording
Warning: failed to …), and every TTY-not-detected error readsTTY not detected - cannot prompt for <what>. <The alternative>.--output,--no-wait, and--wait-timeouthave the same help text on every command (tiger service logslisted atextformat it doesn't accept).--no-waithint names the resource's ID (the branch hints suggestedtiger branch getwith no ID, which can't work).service_forkandservice_resizemessages match their siblings',service_rename's message no longer says "successfully", and theservice_logsoutput schema describes its timestamp format correctly.errors.Newrather thanfmt.Errorf.Repo
dockerfileVersionscustom manager and the Dockerfile's# renovate:comment are gone: the built-in Dockerfile manager already reads the Go version from theARGdefault, so they extracted it a second time (confirmed with a local dry run).CLAUDE.mdis nowAGENTS.md. It also now covers the API response helpers, group commands, where to setSilenceErrors, flag validation order, empty list bodies, lowercasing and falling through on output formats, the TTY error wording, and quoting.