fix: Add schema-aware numeric coercion for MCP tool arguments #5478
Conversation
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
DOsinga
approved these changes
Oct 30, 2025
Collaborator
DOsinga
left a comment
DOsinga
left a comment
There was a problem hiding this comment.
this is definitely an improvement, but I think it should live somewhere else
crates/goose/src/agents/agent.rs
Outdated
| return; | ||
| }; | ||
|
|
||
| let tools = self.list_tools(None).await; |
Collaborator
There was a problem hiding this comment.
we're looking only for one tool, list_tools will actually go and call all mcp servers to ask for their tools - that could be rather expensive! now that I say that, I worry that we are doing this willy-nilly anyway
still though, I think we should move this code to the place where we actually get the toolrequests, in categorize_tools - which I see already has a list of tools (unused!)
crates/goose/src/agents/agent.rs
Outdated
|
|
||
| if should_be_number { | ||
| if let Ok(n) = s.parse::<f64>() { | ||
| // Preserve integer types when possible |
Collaborator
There was a problem hiding this comment.
json doesn't have a distinction between integers and floats, so this is all a bit of a no-op
we should consider to do the same for booleans maybe?
crates/goose/src/agents/agent.rs
Outdated
| cancellation_token: Option<CancellationToken>, | ||
| session: Option<SessionConfig>, | ||
| ) -> (String, Result<ToolCallResult, ErrorData>) { | ||
| self.coerce_numeric_arguments(&mut tool_call).await; |
Collaborator
There was a problem hiding this comment.
I don't think you want to do this in a mutable way - just make it functional
katzdave
added a commit
that referenced
this pull request
Nov 5, 2025
…est-and-fix * 'main' of github.com:block/goose: Document missing recipe param types (#5584) docs: description required for "Add Extension" in cli (#5573) fix: Add schema-aware numeric coercion for MCP tool arguments (#5478) Add uv for uvx in Justfile (#5581) Keep llm logs in place (#5577) bump to 1.12.0 (#5580) automate more of the release process (#5409) add clippy warning for string_slice (#5422)
michaelneale
added a commit
that referenced
this pull request
Nov 6, 2025
* main: (41 commits) Add pending extension indicator to extension panel (#5493) Add environment subsition for auth blocks (#5439) acp: ToolCallLocations and working cancellation (#5588) feat(providers): add Mistral AI provider (#5009) Listen for ctrl-c during provider request (#5585) Also accept null as description, not just missing (#5589) Document missing recipe param types (#5584) docs: description required for "Add Extension" in cli (#5573) fix: Add schema-aware numeric coercion for MCP tool arguments (#5478) Add uv for uvx in Justfile (#5581) Keep llm logs in place (#5577) bump to 1.12.0 (#5580) automate more of the release process (#5409) add clippy warning for string_slice (#5422) improve linux tray icon support (#5425) feat: log rotation (#5561) use app.isPackaged instead of checking for node env development (#5465) disable RPM build-ID generation to prevent package conflicts (#5563) Add Diagnostics Info to Q&A and Bug Report Templates (#5565) fix: improve server error messages to include HTTP status code (#5532) ...
michaelneale
added a commit
that referenced
this pull request
Nov 6, 2025
* main: (53 commits) acp: ToolCallLocations and working cancellation (#5588) feat(providers): add Mistral AI provider (#5009) Listen for ctrl-c during provider request (#5585) Also accept null as description, not just missing (#5589) Document missing recipe param types (#5584) docs: description required for "Add Extension" in cli (#5573) fix: Add schema-aware numeric coercion for MCP tool arguments (#5478) Add uv for uvx in Justfile (#5581) Keep llm logs in place (#5577) bump to 1.12.0 (#5580) automate more of the release process (#5409) add clippy warning for string_slice (#5422) improve linux tray icon support (#5425) feat: log rotation (#5561) use app.isPackaged instead of checking for node env development (#5465) disable RPM build-ID generation to prevent package conflicts (#5563) Add Diagnostics Info to Q&A and Bug Report Templates (#5565) fix: improve server error messages to include HTTP status code (#5532) improvement: add useful error message when attempting to use unauthenticated cursor-agent (#5300) fix: unblock acp via databricks (#5562) ...
lifeizhou-ap
added a commit
that referenced
this pull request
Nov 6, 2025
* main: fix: customised recipe to yaml string to avoid minininjia parsing error (#5494) Add pending extension indicator to extension panel (#5493) Add environment subsition for auth blocks (#5439) acp: ToolCallLocations and working cancellation (#5588) feat(providers): add Mistral AI provider (#5009) Listen for ctrl-c during provider request (#5585) Also accept null as description, not just missing (#5589) Document missing recipe param types (#5584) docs: description required for "Add Extension" in cli (#5573) fix: Add schema-aware numeric coercion for MCP tool arguments (#5478) Add uv for uvx in Justfile (#5581) Keep llm logs in place (#5577) bump to 1.12.0 (#5580) automate more of the release process (#5409)
aharvard
added a commit
that referenced
this pull request
Nov 6, 2025
* origin/main: (75 commits) fix: customised recipe to yaml string to avoid minininjia parsing error (#5494) Add pending extension indicator to extension panel (#5493) Add environment subsition for auth blocks (#5439) acp: ToolCallLocations and working cancellation (#5588) feat(providers): add Mistral AI provider (#5009) Listen for ctrl-c during provider request (#5585) Also accept null as description, not just missing (#5589) Document missing recipe param types (#5584) docs: description required for "Add Extension" in cli (#5573) fix: Add schema-aware numeric coercion for MCP tool arguments (#5478) Add uv for uvx in Justfile (#5581) Keep llm logs in place (#5577) bump to 1.12.0 (#5580) automate more of the release process (#5409) add clippy warning for string_slice (#5422) improve linux tray icon support (#5425) feat: log rotation (#5561) use app.isPackaged instead of checking for node env development (#5465) disable RPM build-ID generation to prevent package conflicts (#5563) Add Diagnostics Info to Q&A and Bug Report Templates (#5565) ...
fbalicchia
pushed a commit
to fbalicchia/goose
that referenced
this pull request
Nov 7, 2025
…5478) Signed-off-by: fbalicchia <[email protected]>
BlairAllan
pushed a commit
to BlairAllan/goose
that referenced
this pull request
Nov 29, 2025
…5478) Signed-off-by: Blair Allan <[email protected]>
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.
Some LLMs quote numeric arguments when calling MCP tools (e.g., sending
"42"instead of42), which can cause tool execution failures. This change adds schema-aware coercion that automatically converts string representations of numbers to their proper numeric types, but only when the tool's schema explicitly indicates a parameter should be numeric. The implementation is conservative - if no schema is available, no coercion is performed to avoid accidentally modifying strings that should remain as strings.