Add environment parameter to service_create and service_fork MCP tools - #228
Conversation
The CLI's `tiger service create` and `tiger service fork` both accept `--environment` (DEV/PROD), but the matching MCP tools took no such parameter and hardcoded `api.EnvironmentTagDEV` in their read-only gate. An oversight rather than a deliberate difference, raised in review of the read-only-modes PR (#195). Both tools now take an optional `environment` parameter, enumerated DEV/PROD and defaulting to DEV, so omitting it keeps today's behavior. The read-only gate checks the requested tag instead of a hardcoded one: under `read_only=prod`, creating a PROD service is refused the same way the CLI refuses it, since the mode would otherwise create a service it then refuses to stop or delete. Forking a PROD source into a DEV fork stays allowed. The prod variant of the MCP server instructions explains that new refusal too, so it doesn't read as a bug to an assistant. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
nathanjcochran
left a comment
nathanjcochran
left a comment
There was a problem hiding this comment.
Left two minor comments but LGTM!
| // Default to DEV when unspecified, matching `tiger service create`. | ||
| environmentTag := input.Environment | ||
| if environmentTag == "" { | ||
| environmentTag = api.EnvironmentTagDEV | ||
| } |
There was a problem hiding this comment.
I don't think you actually need to do this. Because you defined the default as api.EnvironmentTagDEV in the JSON schema properties above, the jsonschema library should populate the field automatically with the default if the caller omits it. So I believe this code is redundant (this is a common mistake that LLMs often make when building out these MCP tools - I've seen it many times now 😅). Same for the service_fork tool.
| schema.Properties["environment"].Enum = []any{api.EnvironmentTagDEV, api.EnvironmentTagPROD} | ||
| schema.Properties["environment"].Default = util.Must(json.Marshal(api.EnvironmentTagDEV)) | ||
| schema.Properties["environment"].Examples = []any{api.EnvironmentTagDEV, api.EnvironmentTagPROD} |
There was a problem hiding this comment.
I'm not convinced we need the Examples field if we also have Enum. The enum values already list all of the possible values, so the examples feel pretty redundant (and just take up context, if the MCP client makes them visible to the LLM).
|
Fwiw, I updated the CLAUDE.md in #229 to clarify both of the points I mentioned above (i.e. that we don't need the Examples field if Enum is set, and that the jsonschema library automatically sets default values), and cleaned up some of the existing MCP tools in the process. |
The MCP SDK applies a property's schema default before the handler sees the input (go-sdk mcp/tool.go calls ApplyDefaults on the input map prior to unmarshaling), so the handlers' manual "default to DEV when empty" blocks were dead code. Both handlers now use input.Environment directly, and the comment at each read-only gate records where the default comes from. Also drop the environment Examples from both schemas: the Enum already lists every legal value, so the examples only cost context in clients that surface both. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2 participants
Summary
tiger service createandtiger service forkboth accept--environment(DEV/PROD), but the matching MCP tools took no such parameter and hardcodedapi.EnvironmentTagDEVin their read-only gate. Raised in review of the read-only-modes PR (#195) as likely an oversight rather than a deliberate difference.Both tools now take an optional
environmentparameter:environmentto match both the CLI flag and theenvironmentfield already in these tools' output.api.EnvironmentTag, enumeratedDEV/PRODwithDEVas the schema default, so the SDK rejects anything else before the handler runs — the CLI validates againstvalidEnvironmentTagsthe same way.The read-only gate now checks the requested tag rather than a hardcoded one. Under
read_only=prod,service_createwithenvironment: PRODis refused, matching the CLI and the rule thatprodmode won't create a service it would then refuse to stop or delete. Forking a PROD source into a DEV fork stays allowed, since that reads production without changing it.The
prodvariant of the server instructions covers that new refusal too — it previously only described PROD services being protected, so a refusal with no existing service involved would have looked like a bug.