[Fix] manage_environments tool receives empty arguments on some models - #8
Merged
Merged
Conversation
Contributor
|
Thank you for your contribution! Before we can merge this pull request, we need you to sign our Contributor License Agreement. You can sign it by posting the comment below. I have read the CLA Document and I hereby sign the CLA You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot. |
Author
|
No code issues found. Reviewed the wire-schema change and its regression test. See task |
mrubens
marked this pull request as ready for review
July 8, 2026 20:03
mrubens
requested review from
brunobergher,
daniel-lxs and
mrubens
as code owners
July 8, 2026 20:03
daniel-lxs
added a commit
that referenced
this pull request
Jul 10, 2026
Automation scans (e.g. ci-failure-triage on staging) were getting stuck in long retry loops because some models cannot reliably emit this tool's nested arguments: the workItems array was sent as a JSON string, fields were dropped, reasoning text leaked into optional fields, and the act-item requirements only surfaced as an opaque platform-API 400 after the round trip. Same failure class as the manage_environments fix (#8): the cure is a simpler schema, not stricter validation. - The tool now takes one flat work item per call (scalar params only) and is called once per work item; the worker wraps it into the platform API's existing workItems array - executionPrompt, targetRepositoryFullName, and targetEnvironmentId are required in the advertised schema, so models see the real contract instead of a post-hoc 400 - disposition is retained as an optional enum(['act']) for prompt compatibility and hardcoded to act in the handler; workspaceReadiness and readinessMessage are removed entirely -- the platform derives readiness from targetEnvironmentId and rejects bare_repo for act items, so those fields could never be legitimately set (readinessMessage was the field that wedged the staging loop) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
daniel-lxs
added a commit
that referenced
this pull request
Jul 10, 2026
…116) Automation scans (e.g. ci-failure-triage on staging) were getting stuck in long retry loops because some models cannot reliably emit this tool's nested arguments: the workItems array was sent as a JSON string, fields were dropped, reasoning text leaked into optional fields, and the act-item requirements only surfaced as an opaque platform-API 400 after the round trip. Same failure class as the manage_environments fix (#8): the cure is a simpler schema, not stricter validation. - The tool now takes one flat work item per call (scalar params only) and is called once per work item; the worker wraps it into the platform API's existing workItems array - executionPrompt, targetRepositoryFullName, and targetEnvironmentId are required in the advertised schema, so models see the real contract instead of a post-hoc 400 - disposition is retained as an optional enum(['act']) for prompt compatibility and hardcoded to act in the handler; workspaceReadiness and readinessMessage are removed entirely -- the platform derives readiness from targetEnvironmentId and rejects bare_repo for act items, so those fields could never be legitimately set (readinessMessage was the field that wedged the staging loop) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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.
What changed
The
manage_environmentsMCP tool'sdefinitionfield usedz.union([z.string(), z.record(z.unknown())])— the only such union in the Roomote MCP server. That schema serializes to a JSON SchemaanyOfplus an emptyadditionalProperties: {}object, which some model/provider tool-call paths drop entirely, sending an empty{}arguments object to the server. The MCP SDK then reportsactionanddefinitionasundefinedand the handler never runs, blocking the environment-setup skill from persisting any environment.This changes
definitionto a plainz.string(). The handler increate-environment.tsalready accepts YAML/JSON strings and auto-detects format, so no logic change was needed — only the wire schema. The field description now says "YAML or JSON string" instead of "YAML/JSON string or object." A regression test assertsdefinitionis az.ZodString, not az.ZodUnion, so the union cannot sneak back in.Why this change was made
manage_environmentswas the only Roomote MCP tool that broke on certain model/provider paths, while simple-schema tools (manage_tasks, etc.) worked in the same session. The union schema'sanyOfserialization was the differentiator. Removing the union from the wire schema makes the tool arguments survive the tool-call layer reliably regardless of provider.Impact
Agents calling
manage_environmentsnow consistently deliver theiractionanddefinitionarguments to the server across all models/providers. Object definitions are no longer accepted over the wire (agents pass a YAML or JSON string, which the environment-setup skill already does), while the handler's internal object-handling path remains as a harmless fallback for direct/test callers. No user-visible behavior change beyond the tool working reliably.