fix(tools/looker): strip wrapping quotes from filter values for unquoted parameters (#3273)

## Description

LookML `parameter` fields declared `type: unquoted` reject queries
through the Looker tools because filter values arrive with a literal
layer of wrapping quote characters and are substituted into SQL bare via
`{% parameter %}` — producing invalid SQL like `... = "first_touch"`.

`ProcessQueryArgs` in
`internal/tools/looker/lookercommon/lookercommon.go` already strips a
wrapping layer of quotes from filter *keys*; this extends the same
handling to string *values*, stripping a single layer of wrapping `"` or
`'` only when the first and last characters match. Bare values are
correct for all parameter types over the `WriteQuery` wire format, so
this is applied unconditionally — no need to branch on the field's
declared LookML type. The `filters` parameter description is also
tightened so the model is told explicitly not to wrap values in extra
quotes, addressing the upstream cause while the normalization stays a
backstop.

One fix covers every tool built on the shared helper: `looker-query`,
`looker-query-sql`, `looker-query-url`, `looker-make-look`, and
`looker-add-dashboard-element`.

## Tests

- **Unit:** added `TestProcessQueryArgsStripsWrappingQuotes` exercising
`ProcessQueryArgs` directly — bare values (unchanged), double-quoted
values, single-quoted values, quoted keys (regression),
quoted-key+quoted-value, non-string values (untouched), single-character
strings (no length-check footgun), and mismatched wrapping characters
(left alone). `go test -race ./internal/tools/looker/lookercommon/...`
passes.
- **Integration:** updated the `filters` manifest assertions in
`tests/looker/looker_integration_test.go` to match the tightened
parameter description, keeping the live
`looker-query`/`looker-query-sql`/`looker-query-url` integration suite
green.

## PR Checklist

- [x] Make sure you reviewed
[CONTRIBUTING.md](https://github.com/googleapis/mcp-toolbox/blob/main/CONTRIBUTING.md)
- [x] 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)
- [ ] Make sure to add `!` if this involve a breaking change

🛠️ Fixes #3272

Co-authored-by: Dr. Strangelove <drstrangelove@google.com>

Author: anandnalya

internal/tools/looker/lookercommon/lookercommon.go

@@ -118,7 +118,11 @@ func GetQueryParameters() parameters.Parameters {
 	)
 	filtersParameter := parameters.NewMapParameterWithDefault("filters",
 		map[string]any{},
-		"The filters for the query",
+		"The filters for the query. Keys are fully-qualified field names "+
+			"(e.g. \"view.field\") and values are filter expressions or "+
+			"parameter values. Pass values bare — do not wrap them in extra "+
+			"quote characters. For LookML `parameter` fields, use the raw "+
+			"allowed_value (e.g. `first_touch`), not `\"first_touch\"`.",
 		"",
 	)
 	pivotsParameter := parameters.NewArrayParameterWithDefault("pivots",
@@ -174,13 +178,25 @@ func ProcessQueryArgs(ctx context.Context, params parameters.ParamValues) (*v4.W
 	}
 	fields := f.([]string)
 	filters := paramsMap["filters"].(map[string]any)
-	// Sometimes filters come as "'field.id'": "expression" so strip extra ''
+	// Strip a single layer of wrapping quotes from keys and string values.
+	// Values matter for LookML `type: unquoted` parameters, where Looker
+	// substitutes the value bare into SQL via {% parameter %}. Build a new map
+	// rather than mutating during iteration, and avoid comparing `any` values
+	// directly (non-comparable dynamic types like slices would panic).
+	processedFilters := make(map[string]any, len(filters))
 	for k, v := range filters {
-		if len(k) > 0 && k[0] == '\'' && k[len(k)-1] == '\'' {
-			delete(filters, k)
-			filters[k[1:len(k)-1]] = v
+		newKey := k
+		if len(k) >= 2 && (k[0] == '\'' || k[0] == '"') && k[0] == k[len(k)-1] {
+			newKey = k[1 : len(k)-1]
 		}
+		newVal := v
+		if s, ok := v.(string); ok && len(s) >= 2 &&
+			(s[0] == '\'' || s[0] == '"') && s[0] == s[len(s)-1] {
+			newVal = s[1 : len(s)-1]
+		}
+		processedFilters[newKey] = newVal
 	}
+	filters = processedFilters
 	p, err := parameters.ConvertAnySliceToTyped(paramsMap["pivots"].([]any), "string")
 	if err != nil {
 		return nil, fmt.Errorf("can't convert pivots to array of strings: %s", err)

internal/tools/looker/lookercommon/lookercommon_test.go

@@ -21,6 +21,7 @@ import (
 	"github.com/google/go-cmp/cmp"
 	"github.com/googleapis/mcp-toolbox/internal/testutils"
 	"github.com/googleapis/mcp-toolbox/internal/tools/looker/lookercommon"
+	"github.com/googleapis/mcp-toolbox/internal/util/parameters"
 	v4 "github.com/looker-open-source/sdk-codegen/go/sdk/v4"
 )
 
@@ -171,6 +172,90 @@ func TestExtractLookerFieldPropertiesWithNilFields(t *testing.T) {
 	}
 }
 
+func TestProcessQueryArgsStripsWrappingQuotes(t *testing.T) {
+	ctx, err := testutils.ContextWithNewLogger()
+	if err != nil {
+		t.Fatalf("unexpected error: %s", err)
+	}
+
+	tcs := []struct {
+		desc       string
+		filtersIn  map[string]any
+		filtersOut map[string]any
+	}{
+		{
+			desc:       "bare string value passed through unchanged",
+			filtersIn:  map[string]any{"view.attribution_model": "first_touch"},
+			filtersOut: map[string]any{"view.attribution_model": "first_touch"},
+		},
+		{
+			desc:       "double-quoted value has wrapping quotes stripped",
+			filtersIn:  map[string]any{"view.attribution_model": `"first_touch"`},
+			filtersOut: map[string]any{"view.attribution_model": "first_touch"},
+		},
+		{
+			desc:       "single-quoted value has wrapping quotes stripped",
+			filtersIn:  map[string]any{"view.attribution_model": "'first_touch'"},
+			filtersOut: map[string]any{"view.attribution_model": "first_touch"},
+		},
+		{
+			desc:       "single-quoted key has wrapping quotes stripped",
+			filtersIn:  map[string]any{"'view.field'": "value"},
+			filtersOut: map[string]any{"view.field": "value"},
+		},
+		{
+			desc:       "quoted key and quoted value are both stripped",
+			filtersIn:  map[string]any{`"view.field"`: `"value"`},
+			filtersOut: map[string]any{"view.field": "value"},
+		},
+		{
+			desc:       "non-string values are not touched",
+			filtersIn:  map[string]any{"view.threshold": 42, "view.enabled": true},
+			filtersOut: map[string]any{"view.threshold": 42, "view.enabled": true},
+		},
+		{
+			desc:       "non-comparable values are passed through without panic",
+			filtersIn:  map[string]any{"view.ids": []any{"a", "b"}, "view.meta": map[string]any{"k": "v"}},
+			filtersOut: map[string]any{"view.ids": []any{"a", "b"}, "view.meta": map[string]any{"k": "v"}},
+		},
+		{
+			desc:       "single-character string is not mangled by the length check",
+			filtersIn:  map[string]any{"view.code": "x"},
+			filtersOut: map[string]any{"view.code": "x"},
+		},
+		{
+			desc:       "mismatched wrapping characters are left alone",
+			filtersIn:  map[string]any{"view.f": `"value'`},
+			filtersOut: map[string]any{"view.f": `"value'`},
+		},
+	}
+
+	for _, tc := range tcs {
+		t.Run(tc.desc, func(t *testing.T) {
+			params := parameters.ParamValues{
+				{Name: "model", Value: "marketing"},
+				{Name: "explore", Value: "cohort_marketing_performance"},
+				{Name: "fields", Value: []any{"view.channel"}},
+				{Name: "filters", Value: tc.filtersIn},
+				{Name: "pivots", Value: []any{}},
+				{Name: "sorts", Value: []any{}},
+				{Name: "limit", Value: 10},
+				{Name: "tz", Value: "Etc/UTC"},
+			}
+			wq, err := lookercommon.ProcessQueryArgs(ctx, params)
+			if err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+			if wq.Filters == nil {
+				t.Fatalf("expected non-nil Filters")
+			}
+			if diff := cmp.Diff(tc.filtersOut, *wq.Filters); diff != "" {
+				t.Fatalf("incorrect filters: diff %v", diff)
+			}
+		})
+	}
+}
+
 func TestRequestRunInlineQuery2(t *testing.T) {
 	fields := make([]string, 1)
 	fields[0] = "foo.bar"

tests/looker/looker_integration_test.go

@@ -514,7 +514,7 @@ func TestLooker(t *testing.T) {
 					map[string]any{
 						"additionalProperties": true,
 						"authServices":         []any{},
-						"description":          "The filters for the query",
+						"description":          "The filters for the query. Keys are fully-qualified field names (e.g. \"view.field\") and values are filter expressions or parameter values. Pass values bare — do not wrap them in extra quote characters. For LookML `parameter` fields, use the raw allowed_value (e.g. `first_touch`), not `\"first_touch\"`.",
 						"name":                 "filters",
 						"required":             false,
 						"default":              map[string]any{},
@@ -606,7 +606,7 @@ func TestLooker(t *testing.T) {
 					map[string]any{
 						"additionalProperties": true,
 						"authServices":         []any{},
-						"description":          "The filters for the query",
+						"description":          "The filters for the query. Keys are fully-qualified field names (e.g. \"view.field\") and values are filter expressions or parameter values. Pass values bare — do not wrap them in extra quote characters. For LookML `parameter` fields, use the raw allowed_value (e.g. `first_touch`), not `\"first_touch\"`.",
 						"name":                 "filters",
 						"required":             false,
 						"default":              map[string]any{},
@@ -698,7 +698,7 @@ func TestLooker(t *testing.T) {
 					map[string]any{
 						"additionalProperties": true,
 						"authServices":         []any{},
-						"description":          "The filters for the query",
+						"description":          "The filters for the query. Keys are fully-qualified field names (e.g. \"view.field\") and values are filter expressions or parameter values. Pass values bare — do not wrap them in extra quote characters. For LookML `parameter` fields, use the raw allowed_value (e.g. `first_touch`), not `\"first_touch\"`.",
 						"name":                 "filters",
 						"required":             false,
 						"default":              map[string]any{},
@@ -950,7 +950,7 @@ func TestLooker(t *testing.T) {
 					map[string]any{
 						"additionalProperties": true,
 						"authServices":         []any{},
-						"description":          "The filters for the query",
+						"description":          "The filters for the query. Keys are fully-qualified field names (e.g. \"view.field\") and values are filter expressions or parameter values. Pass values bare — do not wrap them in extra quote characters. For LookML `parameter` fields, use the raw allowed_value (e.g. `first_touch`), not `\"first_touch\"`.",
 						"name":                 "filters",
 						"required":             false,
 						"default":              map[string]any{},
@@ -1232,7 +1232,7 @@ func TestLooker(t *testing.T) {
 					map[string]any{
 						"additionalProperties": true,
 						"authServices":         []any{},
-						"description":          "The filters for the query",
+						"description":          "The filters for the query. Keys are fully-qualified field names (e.g. \"view.field\") and values are filter expressions or parameter values. Pass values bare — do not wrap them in extra quote characters. For LookML `parameter` fields, use the raw allowed_value (e.g. `first_touch`), not `\"first_touch\"`.",
 						"name":                 "filters",
 						"required":             false,
 						"default":              map[string]any{},