Fix MCP CLI bridge numeric arg coercion for schema-less tools#28005
Merged
Fix MCP CLI bridge numeric arg coercion for schema-less tools#28005
Conversation
Closed
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/55e39088-50c1-442b-b53b-40bb60d9981c Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Fix integer parameters in logs tool for CLI bridge
Fix MCP CLI bridge numeric arg coercion for schema-less tools
Apr 23, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the MCP CLI bridge so that tool arguments passed via the generated CLI wrappers are coerced to numeric types even when tool schema metadata is unavailable, preventing schema validation failures for common numeric flags.
Changes:
- Added a schema-less numeric coercion fallback in
coerceToolArgValue(integers and floats) when no schema properties are available. - Wired
parseToolArgsto pass schema-availability context into coercion logic. - Added targeted Vitest coverage for schema-less numeric coercion and preserving non-numeric strings.
Show a summary per file
| File | Description |
|---|---|
| actions/setup/js/mcp_cli_bridge.cjs | Enables numeric coercion fallback for schema-less tool invocations and plumbs schema-availability state through argument parsing. |
| actions/setup/js/mcp_cli_bridge.test.cjs | Adds tests verifying numeric coercion and non-numeric preservation in the schema-less path. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 1
Comment on lines
+625
to
+640
| // When schema metadata is unavailable (e.g. empty tools cache), apply | ||
| // conservative numeric coercion fallback for CLI ergonomics. | ||
| if (allowNumericFallback && types.length === 0) { | ||
| if (/^-?\d+$/.test(rawValue)) { | ||
| const parsedInt = Number.parseInt(rawValue, 10); | ||
| if (Number.isSafeInteger(parsedInt)) { | ||
| return parsedInt; | ||
| } | ||
| } | ||
|
|
||
| if (/^-?(?:\d+\.\d+|\d+\.|\.\d+)(?:[eE][+-]?\d+)?$/.test(rawValue)) { | ||
| const parsedFloat = Number.parseFloat(rawValue); | ||
| if (!Number.isNaN(parsedFloat) && Number.isFinite(parsedFloat)) { | ||
| return parsedFloat; | ||
| } | ||
| } |
There was a problem hiding this comment.
The schema-less numeric fallback coerces floats only when the value matches the decimal-point regex, so scientific notation like 1e3 / -2E-4 (which Number()/parseFloat() parse fine) will not be coerced and will still be sent as a string in schema-less mode. Consider extending the float pattern to allow exponent-only forms (e.g. ^-?\d+(?:\.\d+)?(?:[eE][+-]?\d+)?$) or using a single numeric-validation regex shared by both integer/float branches.
Contributor
🧪 Test Quality Sentinel ReportTest Quality Score: 85/100✅ Excellent test quality
Test Classification Details
Flagged Tests — Requires ReviewNone. All tests passed quality review. Language SupportTests analyzed:
Verdict
Both new tests directly call Minor observation: Neither test covers an error-shape path. Since the feature doesn't throw errors itself, this is acceptable — but future tests could add boundary cases such as unsafe integers ( 📖 Understanding Test ClassificationsDesign Tests (High Value) verify what the system does:
Implementation Tests (Low Value) verify how the system does it:
Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators. References: §24834194143
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
pelikhan
reviewed
Apr 23, 2026
| // When schema metadata is unavailable (e.g. empty tools cache), apply | ||
| // conservative numeric coercion fallback for CLI ergonomics. | ||
| if (allowNumericFallback && types.length === 0) { | ||
| if (/^-?\d+$/.test(rawValue)) { |
Contributor
Author
There was a problem hiding this comment.
Updated in 27d4879: schema-less numeric fallback now uses parseInt(..., 10) with an isNaN check (and safe-integer guard) before coercing integer values.
| } | ||
| } | ||
|
|
||
| if (/^-?(?:\d+\.\d+|\d+\.|\.\d+)(?:[eE][+-]?\d+)?$/.test(rawValue)) { |
Contributor
Author
There was a problem hiding this comment.
Updated in 27d4879: schema-less numeric fallback now uses parseFloat(...) with an isNaN check for numeric coercion, including scientific notation values.
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/0c365ea8-8206-4477-bdf5-17411e8520c7 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/0c365ea8-8206-4477-bdf5-17411e8520c7 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Contributor
Author
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
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.
Daily CLI exploratory testing found that
agenticworkflows logs --count/--max_tokensfailed schema validation because CLI args were always passed as strings. This blocked practical use of result/token limits from the CLI path when tool metadata is unavailable.Problem scope
mcp_cli_bridgecorrectly coerced types when JSON schema was present, but with empty tools metadata it passed raw strings through tomcpToolsCall.logstool parameters likecountandmax_tokenstherefore arrived as"3"/"3000"instead of integers.Bridge argument coercion update
coerceToolArgValuefor values that are clearly numeric.parseInt(..., 10)withisNaNchecks for integer coercionparseFloat(...)withisNaNchecks for float coercion1e3,-2E-4).Parser wiring change
parseToolArgsnow detects whether schema properties are available and passes that context into coercion so fallback behavior is constrained to schema-less flows.Targeted test coverage
--count 3 --max_tokens 3000are coerced to numbers when schema is unavailable.--start_date -1d) remain strings in the same schema-less path.