Repository navigation
fix(mcp): refuse a non-number for a numeric parameter instead of reading it as absent (BUG-4065) - #2306
Merged
Conversation
…ing it as absent (BUG-4065) numericInput read any value outside float64, int, int64 and json.Number as absent, so a string, a bool or an object for limit, offset or days was silently replaced by the default. itemExpectedSeqParam had its own switch with the same fall-through, so a string expected_seq sent the content write out UNGUARDED. Now only null is absent: - A string is admitted only as a plain decimal integer (an optional '-' and digits), parsed exactly and bounded like a JSON number (|x| < 2^53), so "10" is 10 and "123" is a guarded write. That matches what stdio's CLI flags and the buildQuery doors already accept (lead ruling af4bff40, Q1). Every other string, and every other type, is refused with validation_failed naming the parameter. - Every Go integer type is admitted exactly; an unsigned value above MaxInt64 is refused. - itemExpectedSeqParam, pr_number and chatSequence share that one rule (pr_number's bound becomes the exclusive 2^53; chatSequence's float64 gets the 2^53 bound and an exact json.Number). Their messages and hints are unchanged. - item patch validates expected_seq and edits[].line_hint locally, and the collection and role update doors validate sort_order, so each answers validation_failed naming the field rather than the server decode's 400. - buildQuery routes the native integers through the same check; strings are still forwarded verbatim. - The item-list resource's limit flag is refused when malformed, not defaulted. ToolSurfaceVersion 0.103 -> 0.104, with the changelog in version.go and docs/mcp.md; the README, instructions.md heading and wire goldens follow. Claude-Session: https://claude.ai/code/session_01Hk4vxsaKDm3E1LvsoHtHeV
…t defaulted or re-read (BUG-4065 r1) r1 found two Mediums (lead ruling 5a06782e): - The buildQuery doors forwarded a bool, an object or a non-integer string for a numeric key (item backlinks, item search, item context's budget, project activity, project standup and changelog). The server Atoi'd it and silently kept its default. Each route now names its numeric query keys (routeSpec.numericQuery, and the explicit lists at the direct buildQuery callers), and those keys go through numericInput. TestBUG4065_NoRouteForwards ANonNumberForANumberParam walks every remote route against every parameter its tool declares "number", so a new numeric key cannot be left off. - "010" was 10 here and 8 on stdio (pflag parses base 0). A plain decimal string is now ^-?(0|[1-9][0-9]*)$, so a leading zero is refused. The stdio side is BUG-4070. Also: an end-to-end row through the pad_item tool handler, value rows at the chat doors, an array value at every door, absent-means-default, the resource flag's negative row, stale expected_seq comments, and the v0.104 changelog corrected in version.go and docs/mcp.md. Claude-Session: https://claude.ai/code/session_01Hk4vxsaKDm3E1LvsoHtHeV
…intptr admitted (BUG-4065 r2) r2 found the r1 population walk could pass on broken code (lead ruling b9025d7c): it looked for the raw bad value under the catalog name only, so a rename, a path segment or a value dropped as absent was invisible, and its "x"-filled inputs tripped unrelated validation, so most routes refused before reading the number. With a route's numericQuery dropped, the walk alone stayed green. The walk now uses a differential oracle from a curated valid base: each (route, number param) is dispatched with the param absent, as "7", and as a bad value. If "7" changes what is forwarded, the route reads the param and a bad value must be refused naming it with no write; otherwise a bad value must forward exactly what the absent case does. An inconclusive case fails. The derived read-set is pinned in bug4065ReadSet, so a route that stops reading a param altogether cannot be reclassified and go green. Also: uintptr is admitted like the other unsigned types; an empty string and false are named as refused in the v0.104 changelog; rows for a missing key, a null expected_seq (still unguarded), "" and a 20-digit string at every door. Claude-Session: https://claude.ai/code/session_01Hk4vxsaKDm3E1LvsoHtHeV
…UG-4065 r3) r3 rulings (9bd7fa2d), all Low: - false is fed to the unit table, the door table and the population walk; the changelog says it is refused, and nothing pinned it. - pad_item.list and pad_item.history read limit through intParam before the dispatcher; TestBUG4065_CatalogLimitNonNumbers refuses "abc", true, false, an object, "010" and "" there naming limit, and uses "7". - The v0.104 changelog says pad_chat's numeric params still refuse a present null, as before, with the same message; "only null is absent" read as if they did not. - A misnamed row; the refused pr_number rows assert no request went out. Claude-Session: https://claude.ai/code/session_01Hk4vxsaKDm3E1LvsoHtHeV
… (BUG-4065 r4) Comment-only (lead ruling 86de95f1): my r1 edit joined two comment lines in dispatch_http_advanced.go into one; and the walk's doc comment now says conditional reads are outside it, with edits[].line_hint covered by the door table. Claude-Session: https://claude.ai/code/session_01Hk4vxsaKDm3E1LvsoHtHeV
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 BUG-4065.
The remote MCP dispatcher's
numericInputread any value outsidefloat64,int,int64andjson.Numberas absent. So a string, a bool or an object forlimit,offsetordayswas silently replaced by the default.itemExpectedSeqParamhad its own switch with the same fall-through, so a stringexpected_seqsent a content write out unguarded. The red-on-main test reproduces this on 3e7f9e1:expected_seq: "123"reaches the server as{content: x}.Change
nullis absent.^-?[0-9]+$), parsed exactly and bounded like a JSON number (|x| < 2^53)."10"is 10, and"123"is a guarded write. This matches what stdio's CLI flags and thebuildQuerydoors already accept (lead ruling on Q1). Every other string ("1.0","1e3"," 10","+10","") and every other type is refused withvalidation_failednaming the parameter.itemExpectedSeqParam,pr_numberandchatSequenceshare that one rule.pr_number's bound becomes the exclusive 2^53;chatSequence's float64 gets the 2^53 bound and an exactjson.Number. Their messages and hints are unchanged.item patchvalidatesexpected_seqandedits[].line_hintlocally, and collection and role update validatesort_order. Each now answersvalidation_failednaming the field, not the server decode's 400.buildQueryroutes native integers through the same check. Strings are still forwarded verbatim.limitflag is refused when malformed, not defaulted. Today that flag is always written internally.ToolSurfaceVersion0.103 → 0.104, with the changelog inversion.goanddocs/mcp.md. The README, theinstructions.mdheading and the wire goldens follow; the golden diff is only the version.Tests (
internal/mcp/bug4065_non_numeric_inputs_test.go)numericInput: every admitted and refused type, and the ruled string rows ("10","-0","010"read as decimal 10;"1.0","1e3"," 10","+10","","abc","9007199254740992"refused)."7"is the value forwarded, not the default.chatSequence,pr_number(2^53-1, 2^53,json.Number),buildQuerynative integers, and the resource flag.Draft until review and gates finish.
https://claude.ai/code/session_01Hk4vxsaKDm3E1LvsoHtHeV