diff --git a/docs/specs/search-events.md b/docs/specs/search-events.md index 8e88738f6..4d760c3d5 100644 --- a/docs/specs/search-events.md +++ b/docs/specs/search-events.md @@ -95,6 +95,21 @@ The AI produces different query patterns based on the selected dataset: - **Logs dataset**: Focus on `message`, `severity`, `severity_number`, **NO timestamp filters** (uses statsPeriod instead) - **Tracemetrics dataset**: Focus on `metric.name`, `metric.type`, `metric.unit`, `value`, and metric-aware aggregates like `p95(value,http.request.duration,distribution,millisecond)` +### Environment Filters + +The embedded agent receives known visible environment names as context. It must +only add an environment filter when requested; a single available environment or +grouping by environment does not imply a filter. These instructions also apply +when discovery fails or the list is too large to include in the prompt. + +For non-replay datasets, environment filters belong in `query`; the agent leaves +its separate `environment` output null. The internal `validateSearch` tool accepts +the candidate query without a separate environment argument. Replays retain the +separate environment parameter and do not use this validation tool. + +The discovered list is not an exhaustive allowlist: hidden environments can be +absent. Existing final validation and unknown-environment notices remain in place. + ### Time Series Requests for a metric over time ("per hour", "per day", "trend", "over time") return a bucketed series via the `events-stats` endpoint instead of failing. diff --git a/packages/mcp-core/src/tools/catalog/search-events.test.ts b/packages/mcp-core/src/tools/catalog/search-events.test.ts index efbd921d1..578d9b9ec 100644 --- a/packages/mcp-core/src/tools/catalog/search-events.test.ts +++ b/packages/mcp-core/src/tools/catalog/search-events.test.ts @@ -3133,6 +3133,7 @@ describe("search_events", () => { it("merges agent environment into the events search query for non-replay datasets", async () => { let eventsRequestUrl: URL | undefined; + let validationRequestUrl: URL | undefined; mockGenerateText.mockResolvedValueOnce( mockAIResponse( @@ -3149,7 +3150,10 @@ describe("search_events", () => { mswServer.use( http.get( "https://sentry.io/api/0/organizations/test-org/events/validate/", - () => HttpResponse.json(validEventsValidationResponse), + ({ request }) => { + validationRequestUrl = new URL(request.url); + return HttpResponse.json(validEventsValidationResponse); + }, ), http.get( "https://sentry.io/api/0/organizations/test-org/events/", @@ -3190,6 +3194,9 @@ describe("search_events", () => { expect(eventsRequestUrl!.searchParams.get("query")).toContain( "environment:production", ); + expect(validationRequestUrl!.searchParams.get("environment")).toBe( + "production", + ); }); it("does not call events when final validation fails", async () => { diff --git a/packages/mcp-core/src/tools/support/search-events/agent.ts b/packages/mcp-core/src/tools/support/search-events/agent.ts index 1d058160e..8879cf6eb 100644 --- a/packages/mcp-core/src/tools/support/search-events/agent.ts +++ b/packages/mcp-core/src/tools/support/search-events/agent.ts @@ -34,7 +34,7 @@ export const searchEventsAgentOutputSchema = z .nullable() .default(null) .describe( - "Separate environment filter for datasets like replays that do not support environment in the query string. Set only to a real environment the user named (see the 'Available environments' list); omit otherwise. Never use wildcards, placeholders, or example values.", + "Environment filter for replays only, when requested. Otherwise null; non-replay filters belong in query.", ), timeSeries: z .object({ @@ -120,9 +120,8 @@ export interface SearchEventsAgentOptions { environmentNames?: string[]; } -// Above this many environments we stop inlining the full list into the prompt -// (token cost) and rely on the guidance text alone; validation still checks the -// value against the real list. +// Above this many environments we omit the names to bound prompt size. The +// environment filtering instructions still apply without the list. const MAX_INLINE_ENVIRONMENTS = 100; /** @@ -137,16 +136,18 @@ export function buildSystemPromptWithEnvironments( base: string, environmentNames: string[], ): string { + const rule = + "Filter by environment only when requested. Use query filters for non-replays; reserve `environment` for replays and set it to null otherwise. Never invent values; grouping or availability alone does not request a filter."; if (environmentNames.length === 0) { - return base; + return `${base}\n\n## Environment filters\n${rule}`; } - const rule = - 'When the user names an environment, set the `environment` field to a matching value from this list EXACTLY; otherwise OMIT the field. Never use wildcards, placeholders, "null", "*", empty strings, or example values.'; if (environmentNames.length <= MAX_INLINE_ENVIRONMENTS) { - const list = environmentNames.map((name) => `"${name}"`).join(", "); - return `${base}\n\n## Available environments\nThe only valid \`environment\` values for this organization are: ${list}.\n${rule}`; + const list = environmentNames + .map((name) => JSON.stringify(name)) + .join(", "); + return `${base}\n\n## Available environments\nVisible environments: ${list}. Hidden environments may be absent.\n${rule}`; } - return `${base}\n\n## Environments\nThis organization has ${environmentNames.length} environments. ${rule}`; + return `${base}\n\n## Environments\n${environmentNames.length} visible environments (list omitted). ${rule}`; } /** diff --git a/packages/mcp-core/src/tools/support/search-events/environments.test.ts b/packages/mcp-core/src/tools/support/search-events/environments.test.ts index f220bd090..9bd6b8bee 100644 --- a/packages/mcp-core/src/tools/support/search-events/environments.test.ts +++ b/packages/mcp-core/src/tools/support/search-events/environments.test.ts @@ -3,28 +3,19 @@ import { SentryApiService } from "../../../api-client"; import { buildSystemPromptWithEnvironments } from "./agent"; describe("buildSystemPromptWithEnvironments", () => { - it("returns the base prompt unchanged when there are no environments", () => { - expect(buildSystemPromptWithEnvironments("BASE", [])).toBe("BASE"); - }); - - it("inlines a small list of real environment names with a guardrail", () => { + it("inlines a small list of real environment names", () => { const out = buildSystemPromptWithEnvironments("BASE", [ "production", "dev", ]); expect(out).toContain("BASE"); - expect(out).toContain("Available environments"); expect(out).toContain('"production"'); expect(out).toContain('"dev"'); - // Guardrail against the hallucinated placeholders that caused the failures. - expect(out).toContain("OMIT the field"); - expect(out).toContain("Never use wildcards"); }); it("does not dump the full list for very large orgs", () => { const many = Array.from({ length: 250 }, (_, i) => `env-${i}`); const out = buildSystemPromptWithEnvironments("BASE", many); - expect(out).toContain("250 environments"); expect(out).not.toContain('"env-0"'); }); }); diff --git a/packages/mcp-core/src/tools/support/search-events/utils.test.ts b/packages/mcp-core/src/tools/support/search-events/utils.test.ts index d2bca4fa1..4dbb0c158 100644 --- a/packages/mcp-core/src/tools/support/search-events/utils.test.ts +++ b/packages/mcp-core/src/tools/support/search-events/utils.test.ts @@ -1,9 +1,11 @@ import { mswServer } from "@sentry/mcp-server-mocks"; import { HttpResponse, http } from "msw"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { z } from "zod"; import { SentryApiService } from "../../../api-client"; import * as logging from "../../../telem/logging"; import { + createValidateEventsSearchTool, fetchCustomAttributes, formatEventsValidationResults, formatEventValue, @@ -12,6 +14,78 @@ import { looksLikeSentrySearchSyntax, } from "./utils"; +describe("validateSearch tool contract", () => { + it("does not expose a separate environment argument", () => { + const tool = createValidateEventsSearchTool({ + apiService: new SentryApiService({ accessToken: "test-token" }), + organizationSlug: "test-org", + }); + + const schema = tool.inputSchema as z.ZodObject; + expect(Object.keys(schema.shape)).not.toContain("environment"); + }); + + it.each([ + { + name: "does not add an environment filter when none is requested", + query: "span.duration:>100", + }, + { + name: "preserves environment filters in the candidate query", + query: "span.duration:>100 environment:production", + }, + { + name: "does not forward a hallucinated separate environment argument", + query: "span.duration:>100", + environment: ":/", + }, + ])("$name", async ({ query, environment }) => { + const requests: URLSearchParams[] = []; + mswServer.use( + http.get( + "https://sentry.io/api/0/organizations/test-org/events/validate/", + ({ request }) => { + requests.push(new URL(request.url).searchParams); + return HttpResponse.json({ + valid: true, + projects: [], + dataset: [], + environment: [], + field: [], + query: { valid: true, error: null, fields: [] }, + orderby: [], + }); + }, + ), + ); + const tool = createValidateEventsSearchTool({ + apiService: new SentryApiService({ accessToken: "test-token" }), + organizationSlug: "test-org", + }); + type ToolInput = Parameters>[0]; + const schema = tool.inputSchema as z.ZodType; + const input = schema.parse({ + dataset: "spans", + query, + fields: ["span.duration"], + sort: "-span.duration", + environment, + }); + + const result = await tool.execute!(input, { + toolCallId: "validate-search-test", + messages: [], + }); + + expect(requests).toHaveLength(1); + expect(requests[0]!.getAll("environment")).toEqual([]); + expect(requests[0]!.get("query")).toBe(query); + expect(result).toEqual({ + result: { valid: true, message: "Search validation passed." }, + }); + }); +}); + describe("formatEventValue", () => { describe("primitives", () => { it("should return 'null' for null", () => { @@ -313,12 +387,10 @@ describe("search query helpers", () => { // Bare substring matches are not enough — short values must not false-hit // inside unrelated full-text (e.g. "1" inside "401"). - expect( - isSemanticFilterDowngrade("id:1", 'message:"error 401"'), - ).toBe(false); - expect( - isSemanticFilterDowngrade("id:1", 'message:"error 1"'), - ).toBe(true); + expect(isSemanticFilterDowngrade("id:1", 'message:"error 401"')).toBe( + false, + ); + expect(isSemanticFilterDowngrade("id:1", 'message:"error 1"')).toBe(true); }); }); diff --git a/packages/mcp-core/src/tools/support/search-events/utils.ts b/packages/mcp-core/src/tools/support/search-events/utils.ts index 79028959a..74ffd2b68 100644 --- a/packages/mcp-core/src/tools/support/search-events/utils.ts +++ b/packages/mcp-core/src/tools/support/search-events/utils.ts @@ -177,7 +177,10 @@ function normalizeFilterValue(rawValue: string): string { * Read one filter value starting at `valueStart` in the original query. * Quoted values keep interior whitespace; unquoted values stop at whitespace. */ -function readRawFilterValue(query: string, valueStart: number): string | undefined { +function readRawFilterValue( + query: string, + valueStart: number, +): string | undefined { if (valueStart >= query.length) { return undefined; } @@ -241,9 +244,7 @@ function searchFilterOccurrences(query: string): SearchFilterOccurrence[] { return occurrences; } -function structuredFilterOccurrences( - query: string, -): SearchFilterOccurrence[] { +function structuredFilterOccurrences(query: string): SearchFilterOccurrence[] { return searchFilterOccurrences(query).filter( (occurrence) => !FULL_TEXT_SEARCH_KEYS.has(occurrence.key), ); @@ -284,9 +285,7 @@ function containsAsWholeToken(haystack: string, needle: string): boolean { } function isRelatedFilterValue(left: string, right: string): boolean { - return ( - containsAsWholeToken(left, right) || containsAsWholeToken(right, left) - ); + return containsAsWholeToken(left, right) || containsAsWholeToken(right, left); } /** @@ -1097,12 +1096,6 @@ export function createValidateEventsSearchTool(options: { .describe("Optional relative time period like 1h, 24h, 7d"), start: z.string().optional().describe("Optional ISO 8601 start time"), end: z.string().optional().describe("Optional ISO 8601 end time"), - environment: z - .union([z.string().min(1), z.array(z.string().min(1)).min(1)]) - .optional() - .describe( - "Optional environment filter. Prefer query filters for non-replay datasets.", - ), }), execute: async ({ dataset, @@ -1112,7 +1105,6 @@ export function createValidateEventsSearchTool(options: { statsPeriod, start, end, - environment, }) => { const validation = await validateEventsSearch(apiService, { organizationSlug, @@ -1121,7 +1113,6 @@ export function createValidateEventsSearchTool(options: { query, sort, projectId, - environment, statsPeriod, start, end, diff --git a/packages/mcp-server-evals/src/evals/search-events-environments.eval.ts b/packages/mcp-server-evals/src/evals/search-events-environments.eval.ts new file mode 100644 index 000000000..8b6541fd8 --- /dev/null +++ b/packages/mcp-server-evals/src/evals/search-events-environments.eval.ts @@ -0,0 +1,94 @@ +import { SentryApiService } from "@sentry/mcp-core/api-client"; +import { searchEventsAgent } from "@sentry/mcp-core/tools/search-events/agent"; +import { expect } from "vitest"; +import { describeEval } from "vitest-evals"; +import "../setup-env"; +import { StructuredOutputScorer } from "./utils/structuredOutputScorer"; + +describeEval("search-events-environment-grounding", { + data: async () => + ["message:*decoder*", "message:*decoder* environment:production"].map( + (query) => ({ + // Sanitized request shape from a real failure: a structured errors + // aggregate exhausted all five steps on invented environment values, + // despite the prompt already listing the only real environment. + // Keep the handler's request wrapper, including environment: null. + input: [ + "Translate this Sentry event search request.", + "The query may be natural language or already-valid Sentry search syntax.", + "Preserve valid explicit parameters, but correct dataset, query syntax, fields, sort, and time range when they conflict or would fail.", + "If the user query already uses Sentry search syntax, treat its filters as authoritative unless validateSearch proves a field is invalid.", + "Never replace a structured field filter with message/log.body/full-text matching. If no valid attribute exists for an explicit field:value filter, keep the field and let validation fail.", + "For spans, logs, and metrics, use datasetAttributes to discover likely fields with substringMatch, query, and attributeTypes before dropping or renaming explicit fields.", + "A broad datasetAttributes result may be truncated, so absence from that preview does not prove an explicit field is invalid.", + "For non-replay datasets, call validateSearch on the candidate request and fix failures in this same pass before returning.", + "For non-replay datasets, convert environment parameters into query filters. For replays, keep environment in the separate environment parameter.", + "", + `User query: ${query}`, + "Current parameters:", + JSON.stringify( + { + dataset: "errors", + fields: ["count()"], + sort: "-count()", + statsPeriod: "90d", + environment: null, + }, + null, + 2, + ), + ].join("\n"), + expected: { + dataset: "errors", + fields: ["count()"], + sort: "-count()", + environment: null, + timeRange: { statsPeriod: "90d" }, + }, + }), + ), + task: async (input) => { + const agentResult = await searchEventsAgent({ + query: input, + organizationSlug: "sentry-mcp-evals", + apiService: new SentryApiService({ accessToken: "test-token" }), + environmentNames: ["production"], + }); + + const validationCalls = agentResult.toolCalls.filter( + (call) => call.toolName === "validateSearch", + ); + expect(validationCalls.length).toBeGreaterThan(0); + + const queries = [agentResult.result.query]; + for (const call of validationCalls) { + expect(call.args).toBeTypeOf("object"); + expect(call.args).not.toBeNull(); + expect(call.args).not.toHaveProperty("environment"); + const args = call.args as Record; + expect(args.query).toBeTypeOf("string"); + queries.push(args.query as string); + } + + // Check every attempted validation, not just the final successful query: + // repeated invalid tool calls can consume the budget before any output. + const requestedEnvironment = input.includes("environment:production"); + for (const query of queries) { + expect(query).toContain("message:*decoder*"); + if (requestedEnvironment) { + expect(query).toMatch( + /(?:^|\s)environment:(?:production|"production")(?=\s|$)/, + ); + expect(query.match(/\benvironment\s*:/g)).toHaveLength(1); + } else { + expect(query).not.toMatch(/\benvironment\s*:/); + } + } + expect(agentResult.result.environment).toBeNull(); + + return { result: JSON.stringify(agentResult.result) }; + }, + scorers: [StructuredOutputScorer()], + threshold: 1, + timeout: 60000, +});