fix(mcp): require signalRequests/signalNames on signals and signalsLatest#292
Closed
zer0stars wants to merge 1 commit into
Closed
fix(mcp): require signalRequests/signalNames on signals and signalsLatest#292zer0stars wants to merge 1 commit into
zer0stars wants to merge 1 commit into
Conversation
…test
The MCP shortcut tools for `signals` and `signalsLatest` wrapped those
GraphQL fields with a static selection of `timestamp` / `lastSeen` only.
The underlying resolvers read signal names and aggregations from the
GraphQL selection set, so the MCP tool returned buckets with no data
and models frequently stuffed signal names into the `filter` argument
(the only shape that looked plausible).
Add required arguments that carry signal names through explicitly:
- `signals(signalRequests: [SignalAggregationRequest!]!)` where each
entry is `{name, agg}`, and a new `signals: [SignalAggregationValue!]!`
sub-field on `SignalAggregations` carrying one entry per supplied
request.
- `signalsLatest(signalNames: [String!]!)` and a new
`signals: [LatestSignal!]!` sub-field on `SignalCollection` carrying
the latest value for each requested name, filtered by caller
privileges.
The tool names (`get_signals_time_series`, `get_latest_signals`) and
selection-set-based aggregation paths are preserved; this is purely an
additive schema change that makes the shortcut tools usable.
Breaking change for GraphQL clients of `signals`/`signalsLatest`: the
new arguments are required (non-null), so clients must pass them (an
empty list `[]` is valid when using only selection-set aggregation).
zer0stars
force-pushed
the
fix/mcp-signal-requests
branch
from
a890ef4 to
27508a7
Compare
zer0stars
changed the title
Member
Author
|
Superseded by a fix in mcpgen — DIMO-Network/server-garage#34 adds templated |
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
1 participant
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.
Summary
signalRequests: [SignalAggregationRequest!]!tosignalsandsignalNames: [String!]!tosignalsLatest.signals: [SignalAggregationValue!]!sub-field onSignalAggregationsandsignals: [LatestSignal!]!sub-field onSignalCollection, populated from those arguments.telemetry_get_signals_time_series,telemetry_get_latest_signals) so callers pass signal names through arguments instead of a GraphQL selection set that the MCP wrapper cannot express.Why
The generated MCP tools used a static selection of
timestamp/lastSeenwith no way to carry signal names. Since the underlying resolvers build per-signalFloatArgsfrom the GraphQL selection set, the shortcut tools returned empty buckets, and models commonly stuffed signal names into thefilterargument (which only acceptssource) and got nothing back. ThesegmentsanddailyActivitytools already work this way viaSegmentSignalRequest; this mirrors that pattern on the signals tools themselves.Changes
schema/base.graphqls: requiredsignalRequests/signalNamesargs; newSignalAggregationRequestinput; newsignalssub-fields onSignalAggregationsandSignalCollection; updated@mcpToolselections totimestamp signals { name agg value }andlastSeen signals { name timestamp valueNumber valueString valueLocation { ... } }.internal/graph/signal_requests.go: helpers that append FloatArgs for each privileged request and populate the bucketsignalslist from the query result; snapshot-based filtering for the latest-by-name path.internal/graph/base.resolvers.go: resolver wiring.internal/graph/mcp_tools_gen.go,internal/graph/generated.go,internal/graph/model/models_gen.go,internal/graph/model/signal_aggs.go: regenerated / field added.Breaking change
GraphQL clients of
signalsandsignalsLatestmust now supply the new required arguments. An empty list (signalRequests: []/signalNames: []) is valid and preserves the existing selection-set aggregation behavior.Test plan
make lintcleanmake buildgo test ./internal/...telemetry_get_signals_time_seriesandtelemetry_get_latest_signalsvia/mcpagainst a deployed env with a real token; confirm non-emptysignalsin each bucket and in the latest response