🏆 ยอดนิยม: googleapis/mcp toolbox/issues/

he-yufeng · 2026-06-24T15:30:40Z

Description

ArrayParameter.Parse and MapParameter.Parse construct their ParseTypeError from the variable produced by the type assertion (arrVal / m). But that branch is only reached after the assertion has failed, so the variable is always the nil zero value. The resulting error drops the value the caller actually passed:

an array parameter given a string reports [] not type "array"
a map parameter given a string reports map[] not type "map"

Every other Parse implementation (StringParameter, IntParameter, FloatParameter, BooleanParameter, …) passes the original v to ParseTypeError; these two had just drifted out of line. Passing v in both restores the actual input in the message:

"not-an-array" not type "array"
"not-a-map" not type "map"

Added TestParseTypeErrorReportsValue, which asserts the array/map type errors name the offending value. It fails on main and passes with this change.

PR Checklist

Reviewed CONTRIBUTING.md
Tests and linter pass locally (go test ./internal/util/parameters/, go vet, gofmt -l)
Code coverage does not decrease (adds a test)
Docs — not applicable
Breaking change — no

Didn't file a separate issue first since this is a small, self-contained consistency fix; happy to open one if you'd prefer to track it.

gemini-code-assist

Code Review

This pull request fixes an issue in the array and map parameter parsers where type assertion failures reported the nil zero-value of the target type instead of the actual offending input value. A new unit test, TestParseTypeErrorReportsValue, has been added to verify that the correct offending value is reported in the error message. There are no review comments to address.

ArrayParameter.Parse and MapParameter.Parse passed their asserted variable (arrVal / m) to ParseTypeError, but that assertion had already failed, so the value was always the nil zero value. The errors read `[] not type "array"` and `map[] not type "map"` instead of naming the actual input. Every other Parse implementation passes the original value v; align these two with it.

… type field (#3516) ## Description `parseParamFromDelayedUnmarshaler` reads the `type` field from a parameter map and passes it straight to `ParseParameter` with an unchecked type assertion: ```go return ParseParameter(ctx, p, t.(string)) ``` If a tools file declares a parameter whose `type` is not a string (for example `type: 123` from a YAML typo, or `type:` left as a mapping), `t.(string)` panics with `interface conversion: interface {} is int, not string` instead of surfacing a config error. The panic propagates out of `UnmarshalYAML`, so a single malformed parameter takes down config loading with a stack trace rather than a readable message. This is inconsistent with the rest of the function and with `ParseParameter` itself: the missing-`type` case right above already returns a clean error, and `ParseParameter`'s switch has a default branch that reports unknown types as errors. Only the string assertion was unguarded. The fix uses the comma-ok form and returns the same style of error the surrounding code already uses: ```go typeStr, ok := t.(string) if !ok { return nil, fmt.Errorf("parameter 'type' field must be a string, got %T", t) } return ParseParameter(ctx, p, typeStr) ``` Added a `TestFailParametersUnmarshal` case ("common parameter with non-string type") that feeds an integer `type` and expects the error. I confirmed it panics on `main` before the change and passes after; the full `internal/util/parameters` package still passes and `gofmt`/`go vet` are clean. Note this is a separate concern from #3512, which also touches `parameters.go` but in a different function (the array/map element parsing paths). No overlap with that change. ## PR Checklist - [x] Make sure you reviewed [CONTRIBUTING.md](https://github.com/googleapis/mcp-toolbox/blob/main/CONTRIBUTING.md) - [x] Ensure the tests and linter pass - [x] Code coverage does not decrease (added a test covering the new branch) - [ ] Appropriate docs were updated (no behavior/doc change, internal robustness fix) Co-authored-by: Yuan Teoh <45984206+Yuan325@users.noreply.github.com>

Yuan325 · 2026-07-07T03:42:12Z

/gcbrun

… errors (googleapis#3512) ## Description `ArrayParameter.Parse` and `MapParameter.Parse` construct their `ParseTypeError` from the variable produced by the type assertion (`arrVal` / `m`). But that branch is only reached *after* the assertion has failed, so the variable is always the nil zero value. The resulting error drops the value the caller actually passed: - an array parameter given a string reports `[] not type "array"` - a map parameter given a string reports `map[] not type "map"` Every other `Parse` implementation (`StringParameter`, `IntParameter`, `FloatParameter`, `BooleanParameter`, …) passes the original `v` to `ParseTypeError`; these two had just drifted out of line. Passing `v` in both restores the actual input in the message: - `"not-an-array" not type "array"` - `"not-a-map" not type "map"` Added `TestParseTypeErrorReportsValue`, which asserts the array/map type errors name the offending value. It fails on `main` and passes with this change. ## PR Checklist - [x] Reviewed CONTRIBUTING.md - [x] Tests and linter pass locally (`go test ./internal/util/parameters/`, `go vet`, `gofmt -l`) - [x] Code coverage does not decrease (adds a test) - [ ] Docs — not applicable - [ ] Breaking change — no Didn't file a separate issue first since this is a small, self-contained consistency fix; happy to open one if you'd prefer to track it. Co-authored-by: Yuan Teoh <45984206+Yuan325@users.noreply.github.com> 4034d6f

🤖 I have created a release *beep* *boop* --- ## [1.7.0](v1.6.0...v1.7.0) (2026-07-16) #### Features * Add `quotaProject` support for BigQuery and Looker conversational analytics ([#2610](#2610)) ([f3e7ca9](f3e7ca9)) * **arcadedb:** Add arcadedb source and tools ([#2961](#2961)) ([351de00](351de00)) * **cmd/internal,docs:** Add warning log that prebuilt tools are for developer use ([#3451](#3451)) ([8cffcef](8cffcef)) * **source/postgres:** Add optional connectTimeout ([#3620](#3620)) ([b574b07](b574b07)) * **tool/clickhouse-sql:** Add native vector embedding support ([#3229](#3229)) ([6cbe1c2](6cbe1c2)) * **tool/dataplex-update-data-product,tool/dataplex-create-data-asset,tool/dataplex-update-data-asset:** Add update data product, create data asset, and update data asset tools for knowledge catalog source ([#3574](#3574)) ([721c204](721c204)) * **tools/dataplex-create-data-product:** Add dataplex-create-data-product tool ([#3504](#3504)) ([5cee0d2](5cee0d2)) * **tools/dataplex-get-data-asset:** Add dataplex-get-data-asset tool ([#3503](#3503)) ([1ddfbe9](1ddfbe9)) ### Bug Fixes * **parameters:** Report the offending value in array/map type errors ([#3512](#3512)) ([4034d6f](4034d6f)) * **parameters:** Return an error instead of panicking on a non-string type field ([#3516](#3516)) ([66a0d53](66a0d53)) * **source/looker:** Dynamically resolve public host URL ([#3603](#3603)) ([0428afd](0428afd)) * **tool/looker-run-dashboard:** Add support for SQL Runner query tiles ([#3594](#3594)) ([0975d0a](0975d0a)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> Co-authored-by: Yuan Teoh <45984206+Yuan325@users.noreply.github.com>

🤖 I have created a release *beep* *boop* --- ## [1.7.0](v1.6.0...v1.7.0) (2026-07-16) #### Features * Add `quotaProject` support for BigQuery and Looker conversational analytics ([#2610](#2610)) ([f3e7ca9](f3e7ca9)) * **arcadedb:** Add arcadedb source and tools ([#2961](#2961)) ([351de00](351de00)) * **cmd/internal,docs:** Add warning log that prebuilt tools are for developer use ([#3451](#3451)) ([8cffcef](8cffcef)) * **source/postgres:** Add optional connectTimeout ([#3620](#3620)) ([b574b07](b574b07)) * **tool/clickhouse-sql:** Add native vector embedding support ([#3229](#3229)) ([6cbe1c2](6cbe1c2)) * **tool/dataplex-update-data-product,tool/dataplex-create-data-asset,tool/dataplex-update-data-asset:** Add update data product, create data asset, and update data asset tools for knowledge catalog source ([#3574](#3574)) ([721c204](721c204)) * **tools/dataplex-create-data-product:** Add dataplex-create-data-product tool ([#3504](#3504)) ([5cee0d2](5cee0d2)) * **tools/dataplex-get-data-asset:** Add dataplex-get-data-asset tool ([#3503](#3503)) ([1ddfbe9](1ddfbe9)) ### Bug Fixes * **parameters:** Report the offending value in array/map type errors ([#3512](#3512)) ([4034d6f](4034d6f)) * **parameters:** Return an error instead of panicking on a non-string type field ([#3516](#3516)) ([66a0d53](66a0d53)) * **source/looker:** Dynamically resolve public host URL ([#3603](#3603)) ([0428afd](0428afd)) * **tool/looker-run-dashboard:** Add support for SQL Runner query tiles ([#3594](#3594)) ([0975d0a](0975d0a)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> Co-authored-by: Yuan Teoh <45984206+Yuan325@users.noreply.github.com> fbe2b21

🤖 I have created a release *beep* *boop* --- ## [1.7.0](googleapis/mcp-toolbox@v1.6.0...v1.7.0) (2026-07-16) #### Features * Add `quotaProject` support for BigQuery and Looker conversational analytics ([googleapis#2610](googleapis#2610)) ([f3e7ca9](googleapis@f3e7ca9)) * **arcadedb:** Add arcadedb source and tools ([googleapis#2961](googleapis#2961)) ([351de00](googleapis@351de00)) * **cmd/internal,docs:** Add warning log that prebuilt tools are for developer use ([googleapis#3451](googleapis#3451)) ([8cffcef](googleapis@8cffcef)) * **source/postgres:** Add optional connectTimeout ([googleapis#3620](googleapis#3620)) ([b574b07](googleapis@b574b07)) * **tool/clickhouse-sql:** Add native vector embedding support ([googleapis#3229](googleapis#3229)) ([6cbe1c2](googleapis@6cbe1c2)) * **tool/dataplex-update-data-product,tool/dataplex-create-data-asset,tool/dataplex-update-data-asset:** Add update data product, create data asset, and update data asset tools for knowledge catalog source ([googleapis#3574](googleapis#3574)) ([721c204](googleapis@721c204)) * **tools/dataplex-create-data-product:** Add dataplex-create-data-product tool ([googleapis#3504](googleapis#3504)) ([5cee0d2](googleapis@5cee0d2)) * **tools/dataplex-get-data-asset:** Add dataplex-get-data-asset tool ([googleapis#3503](googleapis#3503)) ([1ddfbe9](googleapis@1ddfbe9)) ### Bug Fixes * **parameters:** Report the offending value in array/map type errors ([googleapis#3512](googleapis#3512)) ([4034d6f](googleapis@4034d6f)) * **parameters:** Return an error instead of panicking on a non-string type field ([googleapis#3516](googleapis#3516)) ([66a0d53](googleapis@66a0d53)) * **source/looker:** Dynamically resolve public host URL ([googleapis#3603](googleapis#3603)) ([0428afd](googleapis@0428afd)) * **tool/looker-run-dashboard:** Add support for SQL Runner query tiles ([googleapis#3594](googleapis#3594)) ([0975d0a](googleapis@0975d0a)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> Co-authored-by: Yuan Teoh <45984206+Yuan325@users.noreply.github.com> fbe2b21

🤖 I have created a release *beep* *boop* --- ## [1.7.0](v1.6.0...v1.7.0) (2026-07-16) #### Features * Add `quotaProject` support for BigQuery and Looker conversational analytics ([#2610](#2610)) ([f3e7ca9](f3e7ca9)) * **arcadedb:** Add arcadedb source and tools ([#2961](#2961)) ([351de00](351de00)) * **cmd/internal,docs:** Add warning log that prebuilt tools are for developer use ([#3451](#3451)) ([8cffcef](8cffcef)) * **source/postgres:** Add optional connectTimeout ([#3620](#3620)) ([b574b07](b574b07)) * **tool/clickhouse-sql:** Add native vector embedding support ([#3229](#3229)) ([6cbe1c2](6cbe1c2)) * **tool/dataplex-update-data-product,tool/dataplex-create-data-asset,tool/dataplex-update-data-asset:** Add update data product, create data asset, and update data asset tools for knowledge catalog source ([#3574](#3574)) ([721c204](721c204)) * **tools/dataplex-create-data-product:** Add dataplex-create-data-product tool ([#3504](#3504)) ([5cee0d2](5cee0d2)) * **tools/dataplex-get-data-asset:** Add dataplex-get-data-asset tool ([#3503](#3503)) ([1ddfbe9](1ddfbe9)) ### Bug Fixes * **parameters:** Report the offending value in array/map type errors ([#3512](#3512)) ([4034d6f](4034d6f)) * **parameters:** Return an error instead of panicking on a non-string type field ([#3516](#3516)) ([66a0d53](66a0d53)) * **source/looker:** Dynamically resolve public host URL ([#3603](#3603)) ([0428afd](0428afd)) * **tool/looker-run-dashboard:** Add support for SQL Runner query tiles ([#3594](#3594)) ([0975d0a](0975d0a)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> Co-authored-by: Yuan Teoh <45984206+Yuan325@users.noreply.github.com>

…ce (#3554) `ApplyQueryLimits` decides whether to append a `LIMIT` to a SELECT by checking `strings.Contains(strings.ToUpper(sql), " LIMIT ")`. That substring is anchored on single spaces, so any query that separates `LIMIT` with other whitespace, for example a multiline or tab-formatted statement, is treated as having no limit and gets a SECOND one appended: ``` SELECT * FROM users LIMIT 50 -> SELECT *\nFROM users\nLIMIT 50 LIMIT 100 (invalid SQL) ``` The trailing append branch already trims arbitrary whitespace, so the mismatch is purely in this detection. The fix replaces the substring check with a precompiled word-boundary regex `(?i)\bLIMIT\b`, which matches `LIMIT` regardless of the surrounding whitespace or case. This mirrors the package's existing `regexp.MustCompile` style (see `RedactSQL` in the same file). The append/trim logic is unchanged. Three "already has LIMIT" cases (multiline, lowercase multiline, tab) were added to `TestApplyQueryLimits`. ### PR Checklist - [ ] Make sure to open an issue as a bug/issue before writing your code! - [x] Ensure the tests and linter pass - [x] Code coverage does not decrease (if any source code was changed) - [ ] Appropriate docs were updated (if necessary) - none needed (behavior fix) - [ ] Make sure to add `!` if this involves a breaking change - not breaking ### Issue Reference No tracking issue exists. This is a small, self-contained bug fix. Either open a short bug issue first and use `Fixes #<n> 🦕`, or note in the PR that it is a tiny self-contained fix (precedent: #3512 merged without a prior issue). --------- Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com> Co-authored-by: Wenxin Du <117315983+duwenxin99@users.noreply.github.com>

…ce (googleapis#3554) `ApplyQueryLimits` decides whether to append a `LIMIT` to a SELECT by checking `strings.Contains(strings.ToUpper(sql), " LIMIT ")`. That substring is anchored on single spaces, so any query that separates `LIMIT` with other whitespace, for example a multiline or tab-formatted statement, is treated as having no limit and gets a SECOND one appended: ``` SELECT * FROM users LIMIT 50 -> SELECT *\nFROM users\nLIMIT 50 LIMIT 100 (invalid SQL) ``` The trailing append branch already trims arbitrary whitespace, so the mismatch is purely in this detection. The fix replaces the substring check with a precompiled word-boundary regex `(?i)\bLIMIT\b`, which matches `LIMIT` regardless of the surrounding whitespace or case. This mirrors the package's existing `regexp.MustCompile` style (see `RedactSQL` in the same file). The append/trim logic is unchanged. Three "already has LIMIT" cases (multiline, lowercase multiline, tab) were added to `TestApplyQueryLimits`. ### PR Checklist - [ ] Make sure to open an issue as a bug/issue before writing your code! - [x] Ensure the tests and linter pass - [x] Code coverage does not decrease (if any source code was changed) - [ ] Appropriate docs were updated (if necessary) - none needed (behavior fix) - [ ] Make sure to add `!` if this involves a breaking change - not breaking ### Issue Reference No tracking issue exists. This is a small, self-contained bug fix. Either open a short bug issue first and use `Fixes #<n> 🦕`, or note in the PR that it is a tiny self-contained fix (precedent: googleapis#3512 merged without a prior issue). --------- Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com> Co-authored-by: Wenxin Du <117315983+duwenxin99@users.noreply.github.com>

he-yufeng requested a review from a team as a code owner June 24, 2026 15:30

gemini-code-assist Bot reviewed Jun 24, 2026

View reviewed changes

This was referenced Jun 24, 2026

fix: pass original value to ParseTypeError in ArrayParameter and MapParameter #3486

Closed

fix(parameters): return an error instead of panicking on a non-string type field #3516

Merged

he-yufeng force-pushed the fix/array-map-parse-error-value branch from 150b734 to 0982761 Compare June 25, 2026 04:51

averikitsch added the blunderbuss: assign Have blunderbuss assign this to someone new. label Jun 25, 2026

blunderbuss-gcf Bot removed the blunderbuss: assign Have blunderbuss assign this to someone new. label Jun 25, 2026

blunderbuss-gcf Bot assigned duwenxin99 Jun 25, 2026

he-yufeng added 2 commits June 26, 2026 02:30

Merge branch 'main' into fix/array-map-parse-error-value

3a71267

Merge branch 'main' into fix/array-map-parse-error-value

b781668

Yuan325 assigned Yuan325 and unassigned duwenxin99 Jul 7, 2026

Yuan325 added the priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. label Jul 7, 2026

Yuan325 approved these changes Jul 7, 2026

View reviewed changes

Merge branch 'main' into fix/array-map-parse-error-value

457b664

Yuan325 enabled auto-merge (squash) July 7, 2026 03:42

Yuan325 merged commit 4034d6f into googleapis:main Jul 7, 2026
20 checks passed

release-please Bot mentioned this pull request Jul 7, 2026

chore(main): release 1.7.0 #3568

Merged

BrewTestBot mentioned this pull request Jul 16, 2026

mcp-toolbox 1.7.0 Homebrew/homebrew-core#293607

Merged

anxkhn mentioned this pull request Jul 27, 2026

fix(source/cockroachdb): detect existing LIMIT regardless of whitespace #3554

Merged

5 tasks

Navigation Menu

Uh oh!

fix(parameters): report the offending value in array/map type errors - #3512

fix(parameters): report the offending value in array/map type errors#3512
Yuan325 merged 4 commits into
googleapis:mainfrom
he-yufeng:fix/array-map-parse-error-value

he-yufeng commented Jun 24, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

Yuan325 commented Jul 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

he-yufeng commented Jun 24, 2026

Description

PR Checklist

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Yuan325 commented Jul 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants