From fb14afdb90a9adf2be7af9b4f5e71bd7c10364ff Mon Sep 17 00:00:00 2001 From: zer0stars <74260741+zer0stars@users.noreply.github.com> Date: Mon, 3 Aug 2026 21:03:57 -0400 Subject: [PATCH 1/2] fix(mcp): select Location subfields for coordinate signals telemetry_get_signals_time_series and telemetry_get_latest_signals rendered bare selections for location signals, so every call naming currentLocationCoordinates failed with "must have a selection of subfields". Location signal names now come from the parsed schema and render { latitude longitude hdop }; descriptions document the LocationAggregation set. --- e2e/mcp_test.go | 78 +++++++++++++++++++++++++++- internal/graph/mcp_overrides.go | 59 ++++++++++++++++++--- internal/graph/mcp_overrides_test.go | 51 ++++++++++++++++++ 3 files changed, 181 insertions(+), 7 deletions(-) diff --git a/e2e/mcp_test.go b/e2e/mcp_test.go index f17f795..b0cf5a5 100644 --- a/e2e/mcp_test.go +++ b/e2e/mcp_test.go @@ -171,11 +171,19 @@ func TestMCPSignalTools(t *testing.T) { CloudEventHeader: cloudevent.CloudEventHeader{Source: source, Subject: subject}, Data: vss.SignalData{Timestamp: baseTime.Add(90 * time.Minute), Name: vss.FieldPowertrainTractionBatteryStateOfChargeCurrent, ValueNumber: 42.5}, }, + { + CloudEventHeader: cloudevent.CloudEventHeader{Source: source, Subject: subject}, + Data: vss.SignalData{ + Timestamp: baseTime.Add(60 * time.Minute), + Name: vss.FieldCurrentLocationCoordinates, + ValueLocation: vss.Location{Latitude: 42.615208, Longitude: -83.029093, HDOP: 5}, + }, + }, } insertSignal(t, services.CH, signals) server := newMCPServer(t, services.Settings) - token := services.Auth.CreateVehicleToken(t, mcpTestTokenID, []string{tokenclaims.PermissionGetNonLocationHistory}) + token := services.Auth.CreateVehicleToken(t, mcpTestTokenID, []string{tokenclaims.PermissionGetNonLocationHistory, tokenclaims.PermissionGetLocationHistory}) t.Run("latest signals returns data", func(t *testing.T) { text, isError := callTool(t, server.URL, token, "telemetry_get_latest_signals", map[string]any{ @@ -251,6 +259,74 @@ func TestMCPSignalTools(t *testing.T) { assert.Equal(t, 42.5, *second.SoC, "LAST of SoC in second bucket") }) + t.Run("latest signals returns location values with subfields", func(t *testing.T) { + text, isError := callTool(t, server.URL, token, "telemetry_get_latest_signals", map[string]any{ + "tokenId": mcpTestTokenID, + "signalNames": []string{"speed", "currentLocationCoordinates"}, + }) + require.False(t, isError, "tool error: %s", text) + + var resp struct { + Data struct { + SignalsLatest struct { + Coordinates struct { + Timestamp string `json:"timestamp"` + Value struct { + Latitude float64 `json:"latitude"` + Longitude float64 `json:"longitude"` + HDOP float64 `json:"hdop"` + } `json:"value"` + } `json:"currentLocationCoordinates"` + } `json:"signalsLatest"` + } `json:"data"` + Errors []struct { + Message string `json:"message"` + } `json:"errors"` + } + require.NoError(t, json.Unmarshal([]byte(text), &resp)) + require.Empty(t, resp.Errors, "GraphQL errors: %s", text) + + coords := resp.Data.SignalsLatest.Coordinates + assert.Equal(t, baseTime.Add(60*time.Minute).Format(time.RFC3339), coords.Timestamp) + assert.Equal(t, 42.615208, coords.Value.Latitude) + assert.Equal(t, -83.029093, coords.Value.Longitude) + assert.Equal(t, 5.0, coords.Value.HDOP) + }) + + t.Run("time series aggregates location signals", func(t *testing.T) { + text, isError := callTool(t, server.URL, token, "telemetry_get_signals_time_series", map[string]any{ + "tokenId": mcpTestTokenID, + "interval": "1h", + "from": baseTime.Format(time.RFC3339), + "to": baseTime.Add(2 * time.Hour).Format(time.RFC3339), + "signalRequests": []map[string]any{ + {"name": "currentLocationCoordinates", "agg": "LAST"}, + }, + }) + require.False(t, isError, "tool error: %s", text) + + var resp struct { + Data struct { + Signals []struct { + Coordinates *struct { + Latitude float64 `json:"latitude"` + Longitude float64 `json:"longitude"` + } `json:"currentLocationCoordinates"` + } `json:"signals"` + } `json:"data"` + Errors []struct { + Message string `json:"message"` + } `json:"errors"` + } + require.NoError(t, json.Unmarshal([]byte(text), &resp)) + require.Empty(t, resp.Errors, "GraphQL errors: %s", text) + require.NotEmpty(t, resp.Data.Signals) + + require.NotNil(t, resp.Data.Signals[0].Coordinates) + assert.Equal(t, 42.615208, resp.Data.Signals[0].Coordinates.Latitude) + assert.Equal(t, -83.029093, resp.Data.Signals[0].Coordinates.Longitude) + }) + t.Run("missing agg key fails template render before execution", func(t *testing.T) { text, isError := callTool(t, server.URL, token, "telemetry_get_signals_time_series", map[string]any{ "tokenId": mcpTestTokenID, diff --git a/internal/graph/mcp_overrides.go b/internal/graph/mcp_overrides.go index 806a413..131f5d6 100644 --- a/internal/graph/mcp_overrides.go +++ b/internal/graph/mcp_overrides.go @@ -2,6 +2,8 @@ package graph import ( "fmt" + "sort" + "strings" "github.com/DIMO-Network/server-garage/pkg/mcpserver" ) @@ -52,29 +54,74 @@ func OverrideMCPTools(tools []mcpserver.ToolDefinition) ([]mcpserver.ToolDefinit } func overrideSignalsTimeSeries(t *mcpserver.ToolDefinition) { - t.Description = "Get aggregated time series for a named list of float signals. Pass signalRequests as [{name, agg}] (e.g. [{name:\"speed\",agg:\"AVG\"},{name:\"powertrainTractionBatteryStateOfChargeCurrent\",agg:\"LAST\"}]). Returns buckets of {timestamp, : , ...}. Signal names come from get_available_signals or get_data_summary. Aggregations: AVG, MED, MAX, MIN, RAND, FIRST, LAST." + t.Description = "Get aggregated time series for a named list of float or location signals. Pass signalRequests as [{name, agg}] (e.g. [{name:\"speed\",agg:\"AVG\"},{name:\"currentLocationCoordinates\",agg:\"LAST\"}]). Returns buckets of {timestamp, : , ...}; location signals yield {latitude, longitude, hdop} values. Signal names come from get_available_signals or get_data_summary. Aggregations for float signals: AVG, MED, MAX, MIN, RAND, FIRST, LAST; for location signals: AVG, RAND, FIRST, LAST." t.Query = `query($tokenId: Int!, $interval: String!, $from: Time!, $to: Time!, $filter: SignalFilter) { signals(tokenId: $tokenId, interval: $interval, from: $from, to: $to, filter: $filter) { __MCPGEN_SELECTION__ } }` - t.SelectionTemplate = "timestamp{{range .signalRequests}} {{.name}}(agg: {{.agg}}){{end}}" + t.SelectionTemplate = fmt.Sprintf( + "timestamp{{range .signalRequests}} {{if %s}}{{.name}}(agg: {{.agg}}) %s{{else}}{{.name}}(agg: {{.agg}}){{end}}{{end}}", + locationNameCondition(".name"), locationSelection) t.Args = append(t.Args, mcpserver.ArgDefinition{ Name: "signalRequests", Type: "array", ItemsType: "object", Required: true, ToolOnly: true, - Description: "List of {name, agg} pairs specifying which float signals to aggregate. Each `name` is a signal field name; each `agg` is one of AVG, MED, MAX, MIN, RAND, FIRST, LAST.", + Description: "List of {name, agg} pairs specifying which signals to aggregate. Each `name` is a signal field name. For float signals `agg` is one of AVG, MED, MAX, MIN, RAND, FIRST, LAST; for location signals one of AVG, RAND, FIRST, LAST.", }) } func overrideLatestSignals(t *mcpserver.ToolDefinition) { - t.Description = "Get the most recent value for a named list of float signals. Pass signalNames as an array of strings (e.g. [\"speed\",\"powertrainTractionBatteryStateOfChargeCurrent\"]). For non-float signals (strings, locations) use get_signals_snapshot. Signal names come from get_available_signals or get_data_summary." + t.Description = "Get the most recent value for a named list of signals. Pass signalNames as an array of strings (e.g. [\"speed\",\"currentLocationCoordinates\"]). Float signals return {timestamp, value}; location signals return {timestamp, value: {latitude, longitude, hdop}}. For string signals use get_signals_snapshot. Signal names come from get_available_signals or get_data_summary." t.Query = `query($tokenId: Int!, $filter: SignalFilter) { signalsLatest(tokenId: $tokenId, filter: $filter) { __MCPGEN_SELECTION__ } }` - t.SelectionTemplate = "lastSeen{{range .signalNames}} {{.}} {timestamp value}{{end}}" + t.SelectionTemplate = fmt.Sprintf( + "lastSeen{{range .signalNames}} {{if %s}}{{.}} {timestamp value %s}{{else}}{{.}} {timestamp value}{{end}}{{end}}", + locationNameCondition("."), locationSelection) t.Args = append(t.Args, mcpserver.ArgDefinition{ Name: "signalNames", Type: "array", ItemsType: "string", Required: true, ToolOnly: true, - Description: "List of float-signal field names to return the latest value for.", + Description: "List of float- or location-signal field names to return the latest value for.", }) } + +// locationSelection is the subfield selection required for location-valued +// signals; the Location type has exactly these fields. +const locationSelection = "{ latitude longitude hdop }" + +// locationSignalNames lists the signal fields whose value type is a location, +// read from the parsed schema so regenerated location signals are picked up +// without touching this file. SignalCollection wraps them in SignalLocation; +// the same names take LocationAggregation on SignalAggregations. +func locationSignalNames() []string { + def := parsedSchema.Types["SignalCollection"] + if def == nil { + return nil + } + var names []string + for _, f := range def.Fields { + if f.Type.Name() == "SignalLocation" { + names = append(names, f.Name) + } + } + sort.Strings(names) + return names +} + +// locationNameCondition renders a text/template boolean expression that is +// true when the signal name referenced by ref (e.g. ".name" or ".") is a +// location signal. +func locationNameCondition(ref string) string { + names := locationSignalNames() + if len(names) == 0 { + return "false" + } + terms := make([]string, len(names)) + for i, n := range names { + terms[i] = fmt.Sprintf("(eq %s %q)", ref, n) + } + if len(terms) == 1 { + return terms[0] + } + return "or " + strings.Join(terms, " ") +} diff --git a/internal/graph/mcp_overrides_test.go b/internal/graph/mcp_overrides_test.go index a7ec331..5f1d135 100644 --- a/internal/graph/mcp_overrides_test.go +++ b/internal/graph/mcp_overrides_test.go @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/require" "github.com/vektah/gqlparser/v2/ast" "github.com/vektah/gqlparser/v2/parser" + "github.com/vektah/gqlparser/v2/validator" ) func TestOverrideMCPTools_PatchesBothTools(t *testing.T) { @@ -103,6 +104,56 @@ func TestOverrideMCPTools_TemplatesRenderValidGraphQL(t *testing.T) { } } +// TestOverrideMCPTools_LocationSignalsValidateAgainstSchema reproduces the +// 2026-08-03 production failures: location signals passed to the time-series +// and latest tools rendered selections without subfields, and the executor +// rejected them with "must have a selection of subfields". Parsing alone +// can't catch that, so rendered queries must validate against the schema. +func TestOverrideMCPTools_LocationSignalsValidateAgainstSchema(t *testing.T) { + out, err := OverrideMCPTools(MCPTools) + require.NoError(t, err) + + cases := []struct { + toolName string + args map[string]any + }{ + { + toolName: "telemetry_get_signals_time_series", + args: map[string]any{ + "signalRequests": []any{ + map[string]any{"name": "speed", "agg": "AVG"}, + map[string]any{"name": "currentLocationCoordinates", "agg": "LAST"}, + map[string]any{"name": "currentLocationApproximateCoordinates", "agg": "FIRST"}, + }, + }, + }, + { + toolName: "telemetry_get_latest_signals", + args: map[string]any{ + "signalNames": []any{"speed", "currentLocationCoordinates", "currentLocationApproximateCoordinates"}, + }, + }, + } + + for _, tc := range cases { + t.Run(tc.toolName, func(t *testing.T) { + tool := findTool(t, out, tc.toolName) + + tmpl, err := template.New(tc.toolName).Option("missingkey=error").Parse(tool.SelectionTemplate) + require.NoError(t, err) + var buf strings.Builder + require.NoError(t, tmpl.Execute(&buf, tc.args)) + + query := strings.Replace(tool.Query, mcpserver.SelectionPlaceholder, buf.String(), 1) + doc, parseErr := parser.ParseQuery(&ast.Source{Input: query}) + require.Nil(t, parseErr, "rendered query must parse: %s", query) + + errs := validator.Validate(parsedSchema, doc) + require.Empty(t, errs, "rendered query must validate against the schema: %s", query) + }) + } +} + func findTool(t *testing.T, tools []mcpserver.ToolDefinition, name string) mcpserver.ToolDefinition { t.Helper() for _, tool := range tools { From 58cb0dd684595a8111d9ab698a68827b7d018466 Mon Sep 17 00:00:00 2001 From: zer0stars <74260741+zer0stars@users.noreply.github.com> Date: Mon, 3 Aug 2026 21:24:34 -0400 Subject: [PATCH 2/2] test(mcp): use ValidateWithRules over deprecated Validate --- internal/graph/mcp_overrides_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/graph/mcp_overrides_test.go b/internal/graph/mcp_overrides_test.go index 5f1d135..7cbf0e9 100644 --- a/internal/graph/mcp_overrides_test.go +++ b/internal/graph/mcp_overrides_test.go @@ -148,7 +148,7 @@ func TestOverrideMCPTools_LocationSignalsValidateAgainstSchema(t *testing.T) { doc, parseErr := parser.ParseQuery(&ast.Source{Input: query}) require.Nil(t, parseErr, "rendered query must parse: %s", query) - errs := validator.Validate(parsedSchema, doc) + errs := validator.ValidateWithRules(parsedSchema, doc, nil) require.Empty(t, errs, "rendered query must validate against the schema: %s", query) }) }