Skip to content

Commit eedf2d2

Browse files
dcramerclaude
andauthored
fix: enforce required fields in agent output schemas (#638)
Remove .optional() and .default() from search_events and search_issues agent output schemas. OpenAI's structured outputs require all properties in the 'required' array, but Zod's .optional() and .default() break this. Changes: - Use z.nullable() instead of .optional()/.nullish() for optional fields - Add runtime validation in callEmbeddedAgent to catch missing fields - Update tests to expect UserInputError on schema validation failures - Add agent-output-schema.test.ts to verify all fields are required Co-authored-by: Claude <[email protected]>
1 parent ff49998 commit eedf2d2

File tree

7 files changed

+122
-50
lines changed

7 files changed

+122
-50
lines changed

packages/mcp-server/src/internal/agents/callEmbeddedAgent.ts

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { generateText, Output, type Tool } from "ai";
22
import { getOpenAIModel } from "./openai-provider";
3+
import { UserInputError } from "../../errors";
34
import type { z } from "zod";
45

56
export type ToolCall = {
@@ -20,7 +21,10 @@ interface EmbeddedAgentResult<T> {
2021
* - Errors are re-thrown for the calling agent to handle
2122
* - Each agent can implement its own error handling strategy
2223
*/
23-
export async function callEmbeddedAgent<T>({
24+
export async function callEmbeddedAgent<
25+
TOutput,
26+
TSchema extends z.ZodType<TOutput, z.ZodTypeDef, unknown>,
27+
>({
2428
system,
2529
prompt,
2630
tools,
@@ -29,8 +33,8 @@ export async function callEmbeddedAgent<T>({
2933
system: string;
3034
prompt: string;
3135
tools: Record<string, Tool>;
32-
schema: z.ZodSchema<T>;
33-
}): Promise<EmbeddedAgentResult<T>> {
36+
schema: TSchema;
37+
}): Promise<EmbeddedAgentResult<TOutput>> {
3438
const capturedToolCalls: ToolCall[] = [];
3539

3640
const result = await generateText({
@@ -57,8 +61,27 @@ export async function callEmbeddedAgent<T>({
5761
throw new Error("Failed to generate output");
5862
}
5963

64+
const rawOutput = result.experimental_output;
65+
66+
if (
67+
typeof rawOutput === "object" &&
68+
rawOutput !== null &&
69+
"error" in rawOutput &&
70+
typeof (rawOutput as { error?: unknown }).error === "string"
71+
) {
72+
throw new UserInputError((rawOutput as { error: string }).error);
73+
}
74+
75+
const parsedResult = schema.safeParse(rawOutput);
76+
77+
if (!parsedResult.success) {
78+
throw new UserInputError(
79+
`Invalid agent response: ${parsedResult.error.message}`,
80+
);
81+
}
82+
6083
return {
61-
result: result.experimental_output as T,
84+
result: parsedResult.data,
6285
toolCalls: capturedToolCalls,
6386
};
6487
}
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
import { describe, it } from "vitest";
2+
import { zodToJsonSchema } from "zod-to-json-schema";
3+
import type { ZodTypeAny } from "zod";
4+
import { searchEventsAgentOutputSchema } from "./search-events/agent";
5+
import { searchIssuesAgentOutputSchema } from "./search-issues/agent";
6+
7+
describe("agent output schemas", () => {
8+
function expectRequiredFields(
9+
name: string,
10+
schema: ZodTypeAny,
11+
expected: string[],
12+
) {
13+
const jsonSchema = zodToJsonSchema(schema, {
14+
$refStrategy: "none",
15+
}) as { required?: string[] };
16+
const required = jsonSchema.required ?? [];
17+
const missing = expected.filter((field) => !required.includes(field));
18+
const unexpected = required.filter((field) => !expected.includes(field));
19+
20+
if (missing.length || unexpected.length) {
21+
throw new Error(
22+
`${name} schema mismatch. Missing: ${missing.join(", ") || "none"}. Unexpected: ${unexpected.join(", ") || "none"}. Required fields: ${required.join(", ")}`,
23+
);
24+
}
25+
}
26+
27+
it("marks all search_events fields as required", () => {
28+
expectRequiredFields("search_events", searchEventsAgentOutputSchema, [
29+
"dataset",
30+
"query",
31+
"fields",
32+
"sort",
33+
"timeRange",
34+
"explanation",
35+
]);
36+
});
37+
38+
it("marks all search_issues fields as required", () => {
39+
expectRequiredFields("search_issues", searchIssuesAgentOutputSchema, [
40+
"query",
41+
"sort",
42+
"explanation",
43+
]);
44+
});
45+
});

packages/mcp-server/src/tools/search-events.test.ts

Lines changed: 23 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,8 @@ describe("search_events", () => {
5858
query,
5959
fields: fields || defaultFields[dataset],
6060
sort: sort || defaultSorts[dataset],
61-
...(timeRange && { timeRange }),
61+
timeRange: timeRange ?? null,
62+
explanation: "Test query translation",
6263
};
6364

6465
return {
@@ -240,23 +241,24 @@ describe("search_events", () => {
240241
mockAIResponse("errors", "", [], "Cannot parse this query"),
241242
);
242243

243-
await expect(
244-
searchEvents.handler(
245-
{
246-
organizationSlug: "test-org",
247-
naturalLanguageQuery: "some impossible query !@#$%",
248-
limit: 10,
249-
includeExplanation: false,
250-
},
251-
{
252-
constraints: {
253-
organizationSlug: null,
254-
},
255-
accessToken: "test-token",
256-
userId: "1",
244+
const promise = searchEvents.handler(
245+
{
246+
organizationSlug: "test-org",
247+
naturalLanguageQuery: "some impossible query !@#$%",
248+
limit: 10,
249+
includeExplanation: false,
250+
},
251+
{
252+
constraints: {
253+
organizationSlug: null,
257254
},
258-
),
259-
).rejects.toThrow(UserInputError);
255+
accessToken: "test-token",
256+
userId: "1",
257+
},
258+
);
259+
260+
await expect(promise).rejects.toThrow(UserInputError);
261+
await expect(promise).rejects.toThrow("Cannot parse this query");
260262
});
261263

262264
it("should return UserInputError for time series queries", async () => {
@@ -331,7 +333,7 @@ describe("search_events", () => {
331333
});
332334

333335
it("should handle missing sort parameter", async () => {
334-
// Mock AI response missing sort parameter
336+
// Mock AI response missing sort parameter - schema.parse() will catch this
335337
mockGenerateText.mockResolvedValueOnce({
336338
text: JSON.stringify({
337339
dataset: "errors",
@@ -342,6 +344,7 @@ describe("search_events", () => {
342344
dataset: "errors",
343345
query: "test",
344346
fields: ["title"],
347+
timeRange: null,
345348
},
346349
} as any);
347350

@@ -361,7 +364,7 @@ describe("search_events", () => {
361364
userId: "1",
362365
},
363366
),
364-
).rejects.toThrow("missing required 'sort' parameter");
367+
).rejects.toThrow(UserInputError);
365368
});
366369

367370
it("should handle agent self-correction when sort field not in fields array", async () => {
@@ -379,6 +382,7 @@ describe("search_events", () => {
379382
query: "test",
380383
fields: ["title", "timestamp"],
381384
sort: "-timestamp",
385+
timeRange: null,
382386
explanation: "Self-corrected to include sort field in fields array",
383387
},
384388
} as any);

packages/mcp-server/src/tools/search-events/agent.ts

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,15 @@ import { createWhoamiTool } from "../../internal/agents/tools/whoami";
77
import { createDatasetAttributesTool } from "./utils";
88
import { systemPrompt } from "./config";
99

10-
const outputSchema = z
10+
// OpenAI structured outputs (used by GPT-5) require all properties to be in the 'required' array.
11+
// Avoid .optional()/.default() so the generated JSON Schema keeps every field required.
12+
// Tracking: https://github.com/getsentry/sentry-mcp/issues/623
13+
export const searchEventsAgentOutputSchema = z
1114
.object({
1215
dataset: z
1316
.enum(["spans", "errors", "logs"])
1417
.describe("Which dataset to use for the query"),
15-
query: z
16-
.string()
17-
.default("")
18-
.nullish()
19-
.describe("The Sentry query string for filtering results"),
18+
query: z.string().describe("The Sentry query string for filtering results"),
2019
fields: z
2120
.array(z.string())
2221
.describe("Array of field names to return in results."),
@@ -32,8 +31,8 @@ const outputSchema = z
3231
start: z.string().describe("ISO 8601 start time"),
3332
end: z.string().describe("ISO 8601 end time"),
3433
}),
34+
z.null(),
3535
])
36-
.nullish()
3736
.describe(
3837
"Time range for filtering events. Use either statsPeriod for relative time or start/end for absolute time.",
3938
),
@@ -76,7 +75,7 @@ export interface SearchEventsAgentOptions {
7675
export async function searchEventsAgent(
7776
options: SearchEventsAgentOptions,
7877
): Promise<{
79-
result: z.infer<typeof outputSchema>;
78+
result: z.output<typeof searchEventsAgentOutputSchema>;
8079
toolCalls: any[];
8180
}> {
8281
if (!process.env.OPENAI_API_KEY) {
@@ -99,14 +98,17 @@ export async function searchEventsAgent(
9998
const whoamiTool = createWhoamiTool({ apiService: options.apiService });
10099

101100
// Use callEmbeddedAgent to translate the query with tool call capture
102-
return await callEmbeddedAgent({
101+
return await callEmbeddedAgent<
102+
z.output<typeof searchEventsAgentOutputSchema>,
103+
typeof searchEventsAgentOutputSchema
104+
>({
103105
system: systemPrompt,
104106
prompt: options.query,
105107
tools: {
106108
datasetAttributes: datasetAttributesTool,
107109
otelSemantics: otelLookupTool,
108110
whoami: whoamiTool,
109111
},
110-
schema: outputSchema,
112+
schema: searchEventsAgentOutputSchema,
111113
});
112114
}

packages/mcp-server/src/tools/search-events/handler.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,8 +115,8 @@ export default defineTool({
115115

116116
const parsed = agentResult.result;
117117

118-
// Get the dataset chosen by the agent (should be defined when no error)
119-
const dataset = parsed.dataset!;
118+
// Get the dataset chosen by the agent
119+
const dataset = parsed.dataset;
120120

121121
// Get recommended fields for this dataset (for fallback when no fields are provided)
122122
const recommendedFields = RECOMMENDED_FIELDS[dataset];

packages/mcp-server/src/tools/search-issues/agent.ts

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,16 +6,14 @@ import { createDatasetFieldsTool } from "../../internal/agents/tools/dataset-fie
66
import { createWhoamiTool } from "../../internal/agents/tools/whoami";
77
import { systemPrompt } from "./config";
88

9-
const outputSchema = z.object({
10-
query: z
11-
.string()
12-
.default("")
13-
.nullish()
14-
.describe("The Sentry issue search query"),
9+
// OpenAI structured outputs (used by GPT-5) require all properties to be in the 'required' array.
10+
// Avoid .optional()/.default() so the generated JSON Schema keeps every field required.
11+
// Tracking: https://github.com/getsentry/sentry-mcp/issues/623
12+
export const searchIssuesAgentOutputSchema = z.object({
13+
query: z.string().describe("The Sentry issue search query"),
1514
sort: z
1615
.enum(["date", "freq", "new", "user"])
17-
.default("date")
18-
.nullish()
16+
.nullable()
1917
.describe("How to sort the results"),
2018
explanation: z
2119
.string()
@@ -36,7 +34,7 @@ export interface SearchIssuesAgentOptions {
3634
export async function searchIssuesAgent(
3735
options: SearchIssuesAgentOptions,
3836
): Promise<{
39-
result: z.infer<typeof outputSchema>;
37+
result: z.output<typeof searchIssuesAgentOutputSchema>;
4038
toolCalls: any[];
4139
}> {
4240
if (!process.env.OPENAI_API_KEY) {
@@ -46,7 +44,10 @@ export async function searchIssuesAgent(
4644
}
4745

4846
// Create tools pre-bound with the provided API service and organization
49-
return await callEmbeddedAgent({
47+
return await callEmbeddedAgent<
48+
z.output<typeof searchIssuesAgentOutputSchema>,
49+
typeof searchIssuesAgentOutputSchema
50+
>({
5051
system: systemPrompt,
5152
prompt: options.query,
5253
tools: {
@@ -58,6 +59,6 @@ export async function searchIssuesAgent(
5859
}),
5960
whoami: createWhoamiTool({ apiService: options.apiService }),
6061
},
61-
schema: outputSchema,
62+
schema: searchIssuesAgentOutputSchema,
6263
});
6364
}

packages/mcp-test-client/src/mcp-test-client.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,7 @@ export async function connectToMCPServer(
2525
},
2626
async (span) => {
2727
try {
28-
const args = [
29-
`--access-token=${config.accessToken}`,
30-
"--all-scopes", // Ensure all tools are available in local stdio runs
31-
];
28+
const args = [`--access-token=${config.accessToken}`];
3229
if (config.host) {
3330
args.push(`--host=${config.host}`);
3431
}

0 commit comments

Comments
 (0)