From a9ba6256f641dbe198bf5edac7e370560666dd2f Mon Sep 17 00:00:00 2001 From: Shayna Chambless Date: Mon, 5 Oct 2026 13:54:26 -0700 Subject: [PATCH 1/9] undo shared formatter and keep structuredcontent --- packages/mcp-core/src/api-client/client.ts | 6 +- packages/mcp-core/src/api-client/schema.ts | 4 - packages/mcp-core/src/internal/formatting.ts | 226 ++++---- .../src/internal/tool-helpers/seer.ts | 14 - .../catalog/analyze-issue-with-seer.test.ts | 38 +- .../tools/catalog/analyze-issue-with-seer.ts | 22 +- .../tools/catalog/get-issue-details.test.ts | 543 +++++------------- .../src/tools/catalog/get-issue-details.ts | 124 ++-- 8 files changed, 358 insertions(+), 619 deletions(-) diff --git a/packages/mcp-core/src/api-client/client.ts b/packages/mcp-core/src/api-client/client.ts index 115a3687c..7e25485f2 100644 --- a/packages/mcp-core/src/api-client/client.ts +++ b/packages/mcp-core/src/api-client/client.ts @@ -4282,8 +4282,7 @@ export class SentryApiService { opts?: RequestOptions, ): Promise { const body = await this.requestJSON( - apiPath`/organizations/${organizationSlug}/issues/${issueId}/events/${eventId}/` + - `?llmFormat=json`, + apiPath`/organizations/${organizationSlug}/issues/${issueId}/events/${eventId}/`, undefined, opts, ); @@ -5248,8 +5247,7 @@ export class SentryApiService { opts?: RequestOptions, ): Promise { const body = await this.requestJSON( - apiPath`/organizations/${organizationSlug}/issues/${issueId}/autofix/` + - `?llmFormat=markdown`, + apiPath`/organizations/${organizationSlug}/issues/${issueId}/autofix/`, undefined, opts, ); diff --git a/packages/mcp-core/src/api-client/schema.ts b/packages/mcp-core/src/api-client/schema.ts index cccc58224..c39cdb68a 100644 --- a/packages/mcp-core/src/api-client/schema.ts +++ b/packages/mcp-core/src/api-client/schema.ts @@ -1178,8 +1178,6 @@ const BaseEventSchema = z.object({ _meta: z.unknown().optional(), // dateReceived is when the server received the event (may not be present in all contexts) dateReceived: z.string().datetime().nullish(), - // shared-formatter output, present when the event endpoint is called with ?llmFormat - formatted: z.object({ format: z.string(), content: z.string() }).optional(), }); export const ErrorEventSchema = BaseEventSchema.omit({ @@ -1414,8 +1412,6 @@ export const AutofixRunStateSchema = z.object({ }) .passthrough() .nullable(), - // shared-formatter output, present when the autofix endpoint is called with ?llmFormat - formatted: z.object({ format: z.string(), content: z.string() }).optional(), }); /** diff --git a/packages/mcp-core/src/internal/formatting.ts b/packages/mcp-core/src/internal/formatting.ts index 4f2c12fde..6bbe2dc10 100644 --- a/packages/mcp-core/src/internal/formatting.ts +++ b/packages/mcp-core/src/internal/formatting.ts @@ -43,7 +43,6 @@ import { getAutofixArtifactSummaries, getStatusDisplayName, isTerminalStatus, - wrapSeerContent, } from "./tool-helpers/seer"; import { formatToolCallInstruction } from "./tool-helpers/tool-call-formatting"; import { isPlainObject } from "./type-guards"; @@ -181,15 +180,6 @@ export function formatFrameHeader( } } -/** - * Whether the shared formatter covers this event type, and so whether its body should be used - * instead of the local rendering. - * - * "default" is an error event without exception data, "generic" a performance regression or - * metric issue, "csp" a Content Security Policy violation. Anything else (a transaction, most - * notably) keeps the local path, which renders things the shared body does not carry such as - * the fetched performance trace. - */ /** * Whether to read the issue's metadata instead of its top level fields. Performance issues can * have various categories such as 'db_query', but the issueType starts with 'performance_'. @@ -208,10 +198,15 @@ export function isPerformanceIssueType(issue: { ); } -export function usesSharedFormatterBody(event: { type?: unknown }): boolean { +/** + * Whether this server renders the event type. The Event union is ErrorEvent | DefaultEvent | + * TransactionEvent | GenericEvent | CspEvent, but in practice other types arrive as UnknownEvent. + */ +export function isSupportedEventType(event: { type?: unknown }): boolean { return ( event.type === "error" || event.type === "default" || + event.type === "transaction" || event.type === "generic" || event.type === "csp" ); @@ -237,6 +232,9 @@ export function formatEventOutput( availableToolNames?: ReadonlySet; directToolNames?: ReadonlySet; }; + // strip the replay ids without rendering the note, for callers that report replays + // themselves and would otherwise repeat them in tags and contexts + stripReplayIds?: boolean; }, ) { let output = ""; @@ -245,10 +243,10 @@ export function formatEventOutput( user?: z.infer["user"]; } ).user; - const eventWithReplayMetadataStripped = options?.replaySummary - ? stripReplayMetadata(event) - : event; - const eventToRender = eventWithReplayMetadataStripped; + const eventToRender = + options?.replaySummary || options?.stripReplayIds + ? stripReplayMetadata(event) + : event; if (options?.replaySummary) { output += formatIssueReplayOutput({ @@ -1954,24 +1952,17 @@ function formatSeerSummary(autofixState: AutofixRunState | undefined): string { parts.push(""); } - // Prefer the shared formatter's analysis for the body when the endpoint provides it, but - // keep the status handling around it: a run that failed or needs input must say so either - // way. Seer content is LLM-generated, so wrap it in the untrusted-data boundary. - if (autofixState.formatted?.content) { - parts.push(wrapSeerContent(autofixState.formatted.content, autofix.run_id)); - } else { - // Summarize from the run's artifacts: the solution if available, otherwise - // the root cause if it has been identified. - const { rootCause, solution } = getAutofixArtifactSummaries(autofix); - if (solution) { - parts.push("**Summary:**"); - parts.push(solution); - } else if (rootCause) { - parts.push("**Root Cause Identified:**"); - parts.push(rootCause); - } else if (!isTerminalStatus(autofix.status)) { - parts.push("Analysis has started but no results yet."); - } + // Summarize from the run's artifacts: the solution if available, otherwise + // the root cause if it has been identified. + const { rootCause, solution } = getAutofixArtifactSummaries(autofix); + if (solution) { + parts.push("**Summary:**"); + parts.push(solution); + } else if (rootCause) { + parts.push("**Root Cause Identified:**"); + parts.push(rootCause); + } else if (!isTerminalStatus(autofix.status)) { + parts.push("Analysis has started but no results yet."); } if (autofix.status === "error") { @@ -2128,18 +2119,8 @@ export function formatIssueOutput({ output += "## Event Details\n\n"; - // Check if this is an unsupported event type - // Event type union is: ErrorEvent | DefaultEvent | TransactionEvent | GenericEvent | CspEvent - // But in practice we may have other types returned as UnknownEvent const eventType = event.type; - const isUnsupportedType = - eventType !== "error" && - eventType !== "default" && - eventType !== "transaction" && - eventType !== "generic" && - eventType !== "csp"; - - if (isUnsupportedType) { + if (!isSupportedEventType(event)) { // Log to Sentry for tracking new/unknown event types const sentryEventId = logIssue( `Unsupported event type encountered: ${String(eventType)}`, @@ -2185,8 +2166,15 @@ export function formatIssueOutput({ output += `**Event ID**: ${event.id}\n`; output += `**Type**: ${event.type}\n`; - const isSharedFormatterType = usesSharedFormatterBody(event); - if (isSharedFormatterType) { + // "default" type represents error events without exception data + // "generic" type represents performance regressions and metric-based issues + // "csp" type represents Content Security Policy violations + if ( + event.type === "error" || + event.type === "default" || + event.type === "generic" || + event.type === "csp" + ) { const typedEvent = event as | z.infer | z.infer @@ -2200,40 +2188,17 @@ export function formatIssueOutput({ output += `**Message**:\n${event.message}\n`; } output += "\n"; - // only a markdown body belongs in this output; a json body is for structuredContent, and - // pasting it here would put a serialized object in the middle of the prose - if ( - isSharedFormatterType && - event.formatted?.format === "markdown" && - event.formatted.content - ) { - // the shared formatter body doesn't include the replay note — add it here to match formatEventOutput - output += formatIssueReplayOutput({ + output += formatEventOutput(event, { + performanceTrace, + replaySummary: { apiService, organizationSlug, - event, relatedReplayIds, experimentalMode: experimentalMode ?? false, availableToolNames, directToolNames, - }); - const formattedContent = event.formatted.content; - output += formattedContent.endsWith("\n") - ? formattedContent - : `${formattedContent}\n`; - } else { - output += formatEventOutput(event, { - performanceTrace, - replaySummary: { - apiService, - organizationSlug, - relatedReplayIds, - experimentalMode: experimentalMode ?? false, - availableToolNames, - directToolNames, - }, - }); - } + }, + }); // Add Seer context if available if (autofixState) { @@ -2249,27 +2214,74 @@ export function formatIssueOutput({ output += "\n"; } + output += "## Response Notes\n\n"; + for (const note of buildIssueResponseNotes({ + organizationSlug, + issue, + event, + apiService, + aiConversations, + experimentalMode, + availableToolNames, + directToolNames, + })) { + output += `- ${note}\n`; + } + return output; +} + +/** + * The response notes as individual lines, without bullets. + * + * Both the markdown output and the structured payload need these, and they are mostly + * tool-call instructions whose availability depends on the session, so they are built once + * here rather than restated per output shape. + */ +export function buildIssueResponseNotes({ + organizationSlug, + issue, + event, + apiService, + aiConversations, + experimentalMode, + availableToolNames, + directToolNames, +}: { + organizationSlug: string; + issue: Issue; + event: Event; + apiService: SentryApiService; + aiConversations?: AIConversationReference[]; + experimentalMode?: boolean; + availableToolNames?: ReadonlySet; + directToolNames?: ReadonlySet; +}): string[] { + const notes: string[] = []; const traceId = typeof event.contexts?.trace?.trace_id === "string" && event.contexts.trace.trace_id.length > 0 ? event.contexts.trace.trace_id : undefined; - output += "## Response Notes\n\n"; const commitIssueReference = /^\d+$/.test(issue.shortId) ? apiService.getIssueUrl(organizationSlug, issue.shortId) : issue.shortId; - output += `- Commit message issue reference: \`Fixes ${commitIssueReference}\` automatically closes the issue when the commit is merged.\n`; - output += - "- The stacktrace includes first-party application code and third-party code. First-party frames are usually the best starting point for triage.\n"; + notes.push( + `Commit message issue reference: \`Fixes ${commitIssueReference}\` automatically closes the issue when the commit is merged.`, + ); + notes.push( + "The stacktrace includes first-party application code and third-party code. First-party frames are usually the best starting point for triage.", + ); if (aiConversations && aiConversations.length > 0) { - output += formatAIConversationResponseNote({ - aiConversations, - organizationSlug, - experimentalMode: experimentalMode ?? false, - availableToolNames, - directToolNames, - }); + notes.push( + ...buildAIConversationResponseNotes({ + aiConversations, + organizationSlug, + experimentalMode: experimentalMode ?? false, + availableToolNames, + directToolNames, + }), + ); } const issueEventSearchInstruction = formatToolCallInstruction({ toolName: "search_issue_events", @@ -2283,7 +2295,7 @@ export function formatIssueOutput({ directToolNames, fallbackInstruction: "Issue event search is not available in this session", }); - output += `- Issue event search: ${issueEventSearchInstruction}\n`; + notes.push(`Issue event search: ${issueEventSearchInstruction}`); const hasMultipleThreads = event.entries?.some((entry) => { if (entry.type !== "threads") { return false; @@ -2308,7 +2320,7 @@ export function formatIssueOutput({ "to fetch a full thread stacktrace by numeric Thread ID or exact thread Name. Omit `thread` to use Sentry's default selected thread", }); if (stacktraceInstruction) { - output += `- Thread stacktrace lookup: ${stacktraceInstruction}\n`; + notes.push(`Thread stacktrace lookup: ${stacktraceInstruction}`); } } if (traceId) { @@ -2349,9 +2361,11 @@ export function formatIssueOutput({ fallbackInstruction: "Related log search is not available in this session", }); - output += `- Full distributed trace and span tree: ${traceDetailsInstruction}\n`; - output += `- Related span search: ${spanSearchInstruction}\n`; - output += `- Related log search: ${logSearchInstruction}\n`; + notes.push( + `Full distributed trace and span tree: ${traceDetailsInstruction}`, + ); + notes.push(`Related span search: ${spanSearchInstruction}`); + notes.push(`Related log search: ${logSearchInstruction}`); } if (experimentalMode) { const breadcrumbsInstruction = formatToolCallInstruction({ @@ -2365,12 +2379,14 @@ export function formatIssueOutput({ fallbackInstruction: "Issue breadcrumbs are not available in this session", }); - output += `- Breadcrumb trail leading up to this error: ${breadcrumbsInstruction}\n`; + notes.push( + `Breadcrumb trail leading up to this error: ${breadcrumbsInstruction}`, + ); } - return output; + return notes; } -function formatAIConversationResponseNote({ +function buildAIConversationResponseNotes({ aiConversations, organizationSlug, experimentalMode, @@ -2382,7 +2398,7 @@ function formatAIConversationResponseNote({ experimentalMode: boolean; availableToolNames?: ReadonlySet; directToolNames?: ReadonlySet; -}): string { +}): string[] { const instructions = formatAIConversationActionInstructions({ organizationSlug, aiConversations, @@ -2396,13 +2412,31 @@ function formatAIConversationResponseNote({ const spanSuffix = conversation.spanId ? ` Matching span: \`${conversation.spanId}\`.` : ""; - return `- Agent conversation found in this trace: \`${conversation.conversationId}\`.${spanSuffix}\n${instructions.map((instruction) => `- ${instruction}`).join("\n")}\n`; + return [ + `Agent conversation found in this trace: \`${conversation.conversationId}\`.${spanSuffix}`, + ...instructions, + ]; } const conversationIds = aiConversations .map((conversation) => `\`${conversation.conversationId}\``) .join(", "); - return `- Multiple agent conversations were found in this trace: ${conversationIds}.\n${instructions.map((instruction) => `- ${instruction}`).join("\n")}\n`; + return [ + `Multiple agent conversations were found in this trace: ${conversationIds}.`, + ...instructions, + ]; +} + +function formatAIConversationResponseNote(args: { + aiConversations: AIConversationReference[]; + organizationSlug: string; + experimentalMode: boolean; + availableToolNames?: ReadonlySet; + directToolNames?: ReadonlySet; +}): string { + return `${buildAIConversationResponseNotes(args) + .map((note) => `- ${note}`) + .join("\n")}\n`; } const MAX_DISPLAY_REPLAYS = 5; diff --git a/packages/mcp-core/src/internal/tool-helpers/seer.ts b/packages/mcp-core/src/internal/tool-helpers/seer.ts index 96293e31d..0a98ca7fd 100644 --- a/packages/mcp-core/src/internal/tool-helpers/seer.ts +++ b/packages/mcp-core/src/internal/tool-helpers/seer.ts @@ -116,20 +116,6 @@ function wrapSeerAnalysisOutput({ return `\n${output.trimEnd()}\n\n`; } -/** - * Wraps shared-formatter Seer analysis content in the provenance boundary, - * mirroring the tags getOutputForAutofixRun applies to MCP-rendered output. - * Seer content is LLM-generated, so it must be marked as untrusted data. - */ -export function wrapSeerContent(content: string, runId?: number): string { - return wrapSeerAnalysisOutput({ - output: content, - runId, - step: "analysis", - includeProvenanceTags: true, - }); -} - // Artifact data shapes from getsentry/sentry's // `src/sentry/seer/autofix/artifact_schemas.py`. Fields are LLM-generated, so // everything is treated as optional. diff --git a/packages/mcp-core/src/tools/catalog/analyze-issue-with-seer.test.ts b/packages/mcp-core/src/tools/catalog/analyze-issue-with-seer.test.ts index 3803f0ef3..80742fe13 100644 --- a/packages/mcp-core/src/tools/catalog/analyze-issue-with-seer.test.ts +++ b/packages/mcp-core/src/tools/catalog/analyze-issue-with-seer.test.ts @@ -4,7 +4,7 @@ import { createUnsupportedIssue, mswServer, } from "@sentry/mcp-server-mocks"; -import { http, HttpResponse } from "msw"; +import { HttpResponse, http } from "msw"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import analyzeIssueWithSeer from "./analyze-issue-with-seer.js"; @@ -57,42 +57,6 @@ describe("analyze_issue_with_seer", () => { expect(result).toContain("The analysis has completed successfully."); }); - it("uses formatted.content from the autofix endpoint when present", async () => { - mswServer.use( - http.get( - "https://sentry.io/api/0/organizations/sentry-mcp-evals/issues/CLOUDFLARE-MCP-FMT/autofix/", - () => - HttpResponse.json({ - autofix: { run_id: 42, status: "completed", blocks: [] }, - formatted: { - format: "markdown", - content: "## Root Cause\n\nSHARED-AUTOFIX-MARKER", - }, - }), - ), - ); - - const result = await analyzeIssueWithSeer.handler( - { - organizationSlug: "sentry-mcp-evals", - regionUrl: null, - instruction: undefined, - issueId: "CLOUDFLARE-MCP-FMT", - issueUrl: undefined, - }, - { - constraints: { organizationSlug: undefined }, - accessToken: "access-token", - userId: "1", - }, - ); - - expect(result).toContain("SHARED-AUTOFIX-MARKER"); // body from the shared /autofix/ formatter - // LLM-generated content is still wrapped in the untrusted-data boundary - expect(result).toContain(''); - expect(result).toContain(""); - }); - it("wraps completed Seer-authored sections with provenance tags", async () => { mswServer.use( http.get( diff --git a/packages/mcp-core/src/tools/catalog/analyze-issue-with-seer.ts b/packages/mcp-core/src/tools/catalog/analyze-issue-with-seer.ts index 20da93674..954576341 100644 --- a/packages/mcp-core/src/tools/catalog/analyze-issue-with-seer.ts +++ b/packages/mcp-core/src/tools/catalog/analyze-issue-with-seer.ts @@ -19,7 +19,6 @@ import { SEER_MAX_RETRIES, SEER_POLLING_INTERVAL, SEER_TIMEOUT, - wrapSeerContent, } from "../../internal/tool-helpers/seer"; import { ParamIssueShortId, @@ -178,12 +177,7 @@ export default defineTool({ if (isTerminalStatus(existingStatus)) { // Return results immediately, no polling needed output += `## Analysis ${getStatusDisplayName(existingStatus)}\n\n`; - output += autofixState.formatted?.content - ? wrapSeerContent( - autofixState.formatted.content, - autofixState.autofix.run_id, - ) - : getOutputForAutofixRun(autofixState.autofix); + output += getOutputForAutofixRun(autofixState.autofix); if (existingStatus !== "completed") { output += `\n**Status**: ${existingStatus}\n`; @@ -216,12 +210,7 @@ export default defineTool({ // Check if completed (terminal state) if (isTerminalStatus(status)) { output += `## Analysis ${getStatusDisplayName(status)}\n\n`; - output += autofixState.formatted?.content - ? wrapSeerContent( - autofixState.formatted.content, - autofixState.autofix.run_id, - ) - : getOutputForAutofixRun(autofixState.autofix); + output += getOutputForAutofixRun(autofixState.autofix); if (status !== "completed") { output += `\n**Status**: ${status}\n`; @@ -290,12 +279,7 @@ export default defineTool({ // Show current progress if (autofixState.autofix) { output += `**Current Status**: ${getStatusDisplayName(autofixState.autofix.status)}\n\n`; - output += autofixState.formatted?.content - ? wrapSeerContent( - autofixState.formatted.content, - autofixState.autofix.run_id, - ) - : getOutputForAutofixRun(autofixState.autofix); + output += getOutputForAutofixRun(autofixState.autofix); } // Timeout reached diff --git a/packages/mcp-core/src/tools/catalog/get-issue-details.test.ts b/packages/mcp-core/src/tools/catalog/get-issue-details.test.ts index 7f282caba..29c0dee64 100644 --- a/packages/mcp-core/src/tools/catalog/get-issue-details.test.ts +++ b/packages/mcp-core/src/tools/catalog/get-issue-details.test.ts @@ -337,240 +337,6 @@ describe("get_issue_details", () => { expect(result).not.toContain("**Culprit**: null"); }); - it.each([ - { - type: "error/default", - issueId: "CLOUDFLARE-MCP-41", - issue: undefined, - event: createDefaultEvent, - marker: "SHARED-FORMATTER-MARKER", - replacedRenderer: undefined, - }, - { - type: "generic", - issueId: "MCP-SERVER-EQE", - issue: createRegressedIssue, - event: createGenericEvent, - marker: "GENERIC-FORMATTER-MARKER", - replacedRenderer: "### Performance Regression Details", - }, - { - type: "csp", - issueId: "BLOG-CSP-4XC", - issue: createCspIssue, - event: createCspEvent, - marker: "CSP-FORMATTER-MARKER", - replacedRenderer: "### CSP Violation", - }, - ])( - "uses formatted.content for $type events", - async ({ issueId, issue, event, marker, replacedRenderer }) => { - const base = `https://sentry.io/api/0/organizations/sentry-mcp-evals/issues/${issueId}`; - if (issue) { - mswServer.use( - http.get(`${base}/`, () => HttpResponse.json(issue()), { - once: true, - }), - ); - } - mswServer.use( - http.get( - `https://sentry.io/api/0/organizations/sentry-mcp-evals/issues/${issue ? issue().id : "6507376925"}/events/latest/`, - () => - HttpResponse.json({ - ...event(), - formatted: { - format: "markdown", - content: `## Body\n\n${marker}`, - }, - }), - { once: true }, - ), - ); - - const result = await getIssueDetails.handler( - { - organizationSlug: "sentry-mcp-evals", - issueId, - eventId: undefined, - issueUrl: undefined, - regionUrl: null, - }, - baseContext, - ); - - // the body is rendered from the shared formatter's content - expect(result).toContain(marker); - // ...replacing MCP's type-specific renderer - if (replacedRenderer) { - expect(result).not.toContain(replacedRenderer); - } - }, - ); - - it("ignores formatted.content for non-error events (transaction)", async () => { - mswServer.use( - http.get( - "https://sentry.io/api/0/organizations/sentry-mcp-evals/issues/PERF-N1-001/", - () => HttpResponse.json(createPerformanceIssue()), - { once: true }, - ), - http.get( - "https://sentry.io/api/0/organizations/sentry-mcp-evals/issues/7890123456/events/latest/", - () => - HttpResponse.json({ - ...createPerformanceEvent(), - formatted: { - format: "markdown", - content: "TRANSACTION-SHOULD-IGNORE-THIS", - }, - }), - { once: true }, - ), - http.get( - "https://sentry.io/api/0/organizations/sentry-mcp-evals/trace/abcdef1234567890abcdef1234567890/", - () => HttpResponse.json(createTraceResponseFixture()), - { once: true }, - ), - ); - - const result = await getIssueDetails.handler( - { - organizationSlug: "sentry-mcp-evals", - issueId: "PERF-N1-001", - eventId: undefined, - issueUrl: undefined, - regionUrl: null, - }, - baseContext, - ); - - // transaction events still route through formatEventOutput, so formatted is unused - expect(result).toContain("Issue PERF-N1-001"); // sanity: real output was produced - expect(result).not.toContain("TRANSACTION-SHOULD-IGNORE-THIS"); - }); - - it("keeps the replay note when error events use formatted.content", async () => { - mswServer.use( - http.get( - "https://sentry.io/api/0/organizations/sentry-mcp-evals/issues/6507376925/events/latest/", - () => - HttpResponse.json({ - ...createDefaultEvent(), - contexts: { - replay: { - type: "default", - replay_id: "1234567890abcdef1234567890abcdef", - }, - }, - formatted: { - format: "markdown", - content: "## Title\n\nBODY-FROM-FORMATTER", - }, - }), - { once: true }, - ), - ); - - const result = await getIssueDetails.handler( - { - organizationSlug: "sentry-mcp-evals", - issueId: "CLOUDFLARE-MCP-41", - eventId: undefined, - issueUrl: undefined, - regionUrl: null, - }, - baseContext, - ); - - expect(result).toContain("BODY-FROM-FORMATTER"); // body from the shared formatter - expect(result).toContain("## Session Replay"); // replay note preserved (was inside formatEventOutput) - }); - - it("embeds the shared formatter's analysis in the Seer section when present", async () => { - mswServer.use( - http.get( - "https://sentry.io/api/0/organizations/sentry-mcp-evals/issues/6507376925/autofix/", - () => - HttpResponse.json({ - autofix: { run_id: 7, status: "completed", blocks: [] }, - formatted: { - format: "markdown", - content: "## Root Cause\n\nEMBEDDED-SEER-MARKER", - }, - }), - { once: true }, - ), - ); - - const result = await getIssueDetails.handler( - { - organizationSlug: "sentry-mcp-evals", - issueId: "CLOUDFLARE-MCP-41", - eventId: undefined, - issueUrl: undefined, - regionUrl: null, - }, - baseContext, - ); - - expect(result).toContain("## Seer Analysis"); - expect(result).toContain("EMBEDDED-SEER-MARKER"); - // LLM-generated content is wrapped in the untrusted-data boundary - expect(result).toContain(''); - }); - - it.each([ - { - label: "in progress", - status: "processing", - expected: "**Status:** Processing", - }, - { - label: "failed", - status: "error", - expected: "**Status:** Analysis failed.", - }, - { - label: "awaiting input", - status: "awaiting_user_input", - expected: "**Status:** Analysis paused - additional information needed.", - }, - ])( - "still reports Seer run status ($label) alongside formatted content", - async ({ status, expected }) => { - mswServer.use( - http.get( - "https://sentry.io/api/0/organizations/sentry-mcp-evals/issues/6507376925/autofix/", - () => - HttpResponse.json({ - autofix: { run_id: 7, status, blocks: [] }, - formatted: { - format: "markdown", - content: "## Root Cause\n\nEMBEDDED-SEER-MARKER", - }, - }), - { once: true }, - ), - ); - - const result = await getIssueDetails.handler( - { - organizationSlug: "sentry-mcp-evals", - issueId: "CLOUDFLARE-MCP-41", - eventId: undefined, - issueUrl: undefined, - regionUrl: null, - }, - baseContext, - ); - - // the formatted body must not hide that the run needs attention - expect(result).toContain("EMBEDDED-SEER-MARKER"); - expect(result).toContain(expected); - }, - ); - it("surfaces agent conversation IDs found by bounded span lookup", async () => { const traceId = "11112222333344445555666677778888"; const event = createDefaultEvent({ @@ -2369,20 +2135,9 @@ describe("get_issue_details", () => { }); describe("structuredContent", () => { - const FORMATTER_JSON = JSON.stringify({ - title: { text: "Error: Tried to cancel a non-cancellable request" }, - exception: { handled: "No", code: "at Object.fetch (index.js:1)" }, - tags: { environment: "production" }, - }); - - function mockLatestEventWithFormatted(formatted: unknown) { - mswServer.use( - http.get( - "https://sentry.io/api/0/organizations/sentry-mcp-evals/issues/6507376925/events/latest/", - () => HttpResponse.json({ ...createDefaultEvent(), formatted }), - ), - ); - } + // structuredResult returns no markdown, so the structured payload is gated on experimental + // mode until it is the only thing the tool returns + const experimentalContext = { ...baseContext, experimentalMode: true }; const params = { organizationSlug: "sentry-mcp-evals", @@ -2392,119 +2147,174 @@ describe("structuredContent", () => { regionUrl: null, }; - it("returns a structured payload when the formatter sends json", async () => { - mockLatestEventWithFormatted({ format: "json", content: FORMATTER_JSON }); - - const result = await getIssueDetails.handler(params, baseContext); + function mockLatestEvent(overrides: Record = {}) { + mswServer.use( + http.get( + "https://sentry.io/api/0/organizations/sentry-mcp-evals/issues/6507376925/events/latest/", + () => HttpResponse.json({ ...createDefaultEvent(), ...overrides }), + ), + ); + } + function payloadOf(result: unknown): Record { expect(result).toHaveProperty("structuredContent"); - const payload = (result as { structuredContent: Record }) + return (result as { structuredContent: Record }) .structuredContent; + } + + it("returns a structured payload in experimental mode", async () => { + mockLatestEvent(); + + const payload = payloadOf( + await getIssueDetails.handler(params, experimentalContext), + ); // the issue level fields the markdown used to assemble expect(payload.issue.shortId).toBe("CLOUDFLARE-MCP-41"); expect(payload.issue.url).toContain("CLOUDFLARE-MCP-41"); expect(typeof payload.issue.occurrences).toBe("number"); expect(typeof payload.issue.usersImpacted).toBe("number"); + }); + + it("returns markdown outside experimental mode", async () => { + mockLatestEvent(); + + const result = await getIssueDetails.handler(params, baseContext); + + expect(result).not.toHaveProperty("structuredContent"); + expect(result).toContain("CLOUDFLARE-MCP-41"); + }); - // the event body is the formatter's json, embedded as an object rather than a string - expect(payload.event.body).toEqual(JSON.parse(FORMATTER_JSON)); - expect(typeof payload.event.body).toBe("object"); + it("renders the event body itself rather than asking the api for one", async () => { + mockLatestEvent(); + + const payload = payloadOf( + await getIssueDetails.handler(params, experimentalContext), + ); + + // the headings are this server's own, so their presence is what shows the body was + // rendered here rather than handed over by the api + expect(typeof payload.event.body).toBe("string"); + expect(payload.event.body).toContain("### Error"); + expect(payload.event.body).toContain("Something went wrong"); + expect(payload.event.body).toContain("### Tags"); }); it("produces a payload that satisfies the schema", async () => { - mockLatestEventWithFormatted({ format: "json", content: FORMATTER_JSON }); + mockLatestEvent(); - const result = await getIssueDetails.handler(params, baseContext); - const payload = (result as { structuredContent: unknown }) - .structuredContent; + const payload = payloadOf( + await getIssueDetails.handler(params, experimentalContext), + ); // a tool that advertises a schema has to return something that satisfies it expect(() => getIssueDetailsOutputSchema.parse(payload)).not.toThrow(); }); - it("falls back to markdown when the org is not on the rollout", async () => { - mockLatestEventWithFormatted(undefined); + it("carries the response notes, which say which tool to call next", async () => { + mockLatestEvent(); - const result = await getIssueDetails.handler(params, baseContext); + const payload = payloadOf( + await getIssueDetails.handler(params, experimentalContext), + ); - // a structured result has to carry the whole answer; without the body it would not - expect(result).not.toHaveProperty("structuredContent"); - expect(result).toContain("CLOUDFLARE-MCP-41"); + // dropping these would leave the structured path strictly worse than the markdown + expect(payload.responseNotes.length).toBeGreaterThan(0); + const notes = payload.responseNotes.join("\n"); + expect(notes).toContain("Fixes CLOUDFLARE-MCP-41"); + expect(notes).toContain("search_issue_events"); }); - it("falls back to markdown when the body is not parseable json", async () => { - mockLatestEventWithFormatted({ format: "json", content: "## not json" }); + it("carries the top level message, which the body does not render", async () => { + // the markdown output prints event.message above the body; formatEventOutput only + // renders a message entry, so the payload has to carry it separately + mockLatestEvent({ message: "TOP-LEVEL-MESSAGE", entries: [] }); - const result = await getIssueDetails.handler(params, baseContext); + const payload = payloadOf( + await getIssueDetails.handler(params, experimentalContext), + ); - expect(result).not.toHaveProperty("structuredContent"); - expect(result).toContain("CLOUDFLARE-MCP-41"); + expect(payload.event.message).toBe("TOP-LEVEL-MESSAGE"); }); - it("keeps transactions on the local path so the performance trace survives", async () => { - // the shared body carries no performance trace; that is fetched separately and only - // rendered for transactions, so a transaction must not take the structured path - const event = createDefaultEvent(); + it("keeps an unsupported event type on the markdown path", async () => { + // the markdown path reports the type and warns; a structured body would render nothing mswServer.use( http.get( - "https://sentry.io/api/0/organizations/sentry-mcp-evals/issues/6507376925/events/latest/", - () => - HttpResponse.json({ - ...event, - type: "transaction", - formatted: { format: "json", content: FORMATTER_JSON }, - }), + "https://sentry.io/api/0/organizations/*/issues/7777777777/events/latest/", + () => HttpResponse.json(createUnknownEvent()), ), http.get( - `https://sentry.io/api/0/projects/sentry-mcp-evals/CLOUDFLARE-MCP/events/${event.id}/committers/`, - () => HttpResponse.json({ detail: "Issue not found" }, { status: 404 }), + "https://sentry.io/api/0/organizations/*/issues/FUTURE-TYPE-001", + () => HttpResponse.json(createUnsupportedIssue()), ), ); - const result = await getIssueDetails.handler(params, baseContext); + const result = await getIssueDetails.handler( + { ...params, issueId: "FUTURE-TYPE-001" }, + experimentalContext, + ); expect(result).not.toHaveProperty("structuredContent"); - expect(result).toContain("CLOUDFLARE-MCP-41"); - expect(result).not.toContain("## Suspect Commit"); + expect(result).toContain('Unsupported event type "future_ai_agent_trace"'); }); - it("keeps the attached replay, which lives on the event not the related list", async () => { - // an issue whose only replay is attached would otherwise report no replays at all + it("renders a transaction, whose body carries the performance trace", async () => { + // the local renderer covers every event type, so a transaction takes the same path mswServer.use( http.get( - "https://sentry.io/api/0/organizations/sentry-mcp-evals/issues/6507376925/events/latest/", - () => - HttpResponse.json({ - ...createDefaultEvent(), - contexts: { - replay: { - type: "default", - replay_id: "1234567890abcdef1234567890abcdef", - }, - }, - formatted: { format: "json", content: FORMATTER_JSON }, - }), + "https://sentry.io/api/0/organizations/sentry-mcp-evals/issues/PERF-N1-001/", + () => HttpResponse.json(createPerformanceIssue()), + ), + http.get( + "https://sentry.io/api/0/organizations/sentry-mcp-evals/issues/7890123456/events/latest/", + () => HttpResponse.json(createPerformanceEvent()), ), ); - const result = await getIssueDetails.handler(params, baseContext); - const payload = (result as { structuredContent: Record }) - .structuredContent; + const payload = payloadOf( + await getIssueDetails.handler( + { ...params, issueId: "PERF-N1-001" }, + experimentalContext, + ), + ); + + expect(payload.event.type).toBe("transaction"); + expect(payload.event.body).toContain("Span"); + }); + + it("keeps the attached replay, which lives on the event not the related list", async () => { + // an issue whose only replay is attached would otherwise report no replays at all + mockLatestEvent({ + contexts: { + replay: { + type: "default", + replay_id: "1234567890abcdef1234567890abcdef", + }, + }, + }); + + const payload = payloadOf( + await getIssueDetails.handler(params, experimentalContext), + ); expect(payload.replays?.attached).toBe("1234567890abcdef1234567890abcdef"); // and the attached id is not repeated in the related list expect(payload.replays?.related).not.toContain( "1234567890abcdef1234567890abcdef", ); + // nor left in the tags and contexts the body renders, which would be a third copy + expect(payload.event.body).not.toContain( + "1234567890abcdef1234567890abcdef", + ); }); it("reports no replays when there are none", async () => { - mockLatestEventWithFormatted({ format: "json", content: FORMATTER_JSON }); + mockLatestEvent(); - const result = await getIssueDetails.handler(params, baseContext); - const payload = (result as { structuredContent: Record }) - .structuredContent; + const payload = payloadOf( + await getIssueDetails.handler(params, experimentalContext), + ); expect(payload.replays).toBeNull(); }); @@ -2512,15 +2322,8 @@ describe("structuredContent", () => { it("maps external issues field by field so upstream extras cannot leak", async () => { // structuredContent is a product contract, not a view of the api response: several // upstream schemas are passthrough, so anything not mapped must not appear + mockLatestEvent(); mswServer.use( - http.get( - "https://sentry.io/api/0/organizations/sentry-mcp-evals/issues/6507376925/events/latest/", - () => - HttpResponse.json({ - ...createDefaultEvent(), - formatted: { format: "json", content: FORMATTER_JSON }, - }), - ), http.get( "https://sentry.io/api/0/organizations/sentry-mcp-evals/issues/6507376925/external-issues/", () => @@ -2537,9 +2340,9 @@ describe("structuredContent", () => { ), ); - const result = await getIssueDetails.handler(params, baseContext); - const payload = (result as { structuredContent: Record }) - .structuredContent; + const payload = payloadOf( + await getIssueDetails.handler(params, experimentalContext), + ); expect(JSON.stringify(payload)).not.toContain("internalOnlyToken"); expect(JSON.stringify(payload)).not.toContain("must-not-leak"); @@ -2548,21 +2351,11 @@ describe("structuredContent", () => { it("carries every field the markdown output surfaces", async () => { // greg's bar for this migration is "roughly the same content": anything the markdown // renders and the payload drops is a regression for every MCP user - mswServer.use( - http.get( - "https://sentry.io/api/0/organizations/sentry-mcp-evals/issues/6507376925/events/latest/", - () => - HttpResponse.json({ - ...createDefaultEvent(), - dateCreated: "2026-09-03T12:00:00.000Z", - formatted: { format: "json", content: FORMATTER_JSON }, - }), - ), - ); + mockLatestEvent({ dateCreated: "2026-09-03T12:00:00.000Z" }); - const result = await getIssueDetails.handler(params, baseContext); - const payload = (result as { structuredContent: Record }) - .structuredContent; + const payload = payloadOf( + await getIssueDetails.handler(params, experimentalContext), + ); // the issue header markdown builds before the event body for (const field of [ @@ -2607,19 +2400,12 @@ describe("structuredContent", () => { issueCategory: "error", }), ), - http.get( - "https://sentry.io/api/0/organizations/sentry-mcp-evals/issues/7890123456/events/latest/", - () => - HttpResponse.json({ - ...createDefaultEvent(), - formatted: { format: "json", content: FORMATTER_JSON }, - }), - ), ); + mockLatestEvent(); - const result = await getIssueDetails.handler(params, baseContext); - const payload = (result as { structuredContent: Record }) - .structuredContent; + const payload = payloadOf( + await getIssueDetails.handler(params, experimentalContext), + ); expect(payload.issue.queryPattern).toBeNull(); expect(payload.issue.location).toBeNull(); @@ -2632,8 +2418,8 @@ describe("structuredContent", () => { http.get( "https://sentry.io/api/0/organizations/sentry-mcp-evals/issues/CLOUDFLARE-MCP-41/", () => - HttpResponse.json({ - ...createPerformanceIssue({ + HttpResponse.json( + createPerformanceIssue({ shortId: "CLOUDFLARE-MCP-41", issueType: "performance_n_plus_one_db_queries", issueCategory: "performance", @@ -2643,21 +2429,14 @@ describe("structuredContent", () => { location: "/api/checkout", }, }), - }), - ), - http.get( - "https://sentry.io/api/0/organizations/sentry-mcp-evals/issues/7890123456/events/latest/", - () => - HttpResponse.json({ - ...createDefaultEvent(), - formatted: { format: "json", content: FORMATTER_JSON }, - }), + ), ), ); + mockLatestEvent(); - const result = await getIssueDetails.handler(params, baseContext); - const payload = (result as { structuredContent: Record }) - .structuredContent; + const payload = payloadOf( + await getIssueDetails.handler(params, experimentalContext), + ); expect(payload.issue.title).toBe("N+1 Query"); expect(payload.issue.queryPattern).toBe("SELECT * FROM users WHERE id = ?"); @@ -2669,15 +2448,8 @@ describe("structuredContent", () => { const many = Array.from({ length: 51 }, (_, i) => i.toString(16).padStart(32, "0"), ); + mockLatestEvent(); mswServer.use( - http.get( - "https://sentry.io/api/0/organizations/sentry-mcp-evals/issues/6507376925/events/latest/", - () => - HttpResponse.json({ - ...createDefaultEvent(), - formatted: { format: "json", content: FORMATTER_JSON }, - }), - ), // related ids come from replay-count, keyed by numeric issue id. Echo back whichever // id was asked for: a preceding test can leave a different issue fixture registered. http.get( @@ -2690,9 +2462,9 @@ describe("structuredContent", () => { ), ); - const result = await getIssueDetails.handler(params, baseContext); - const payload = (result as { structuredContent: Record }) - .structuredContent; + const payload = payloadOf( + await getIssueDetails.handler(params, experimentalContext), + ); expect(payload.replays).not.toBeNull(); expect(payload.replays.relatedCount).toBe(51); @@ -2710,16 +2482,11 @@ describe("suspect commits", () => { issueUrl: undefined, regionUrl: null, }; - const formatted = { - format: "json", - content: JSON.stringify({ title: { text: "Example error" } }), - }; + // the structured payload is gated on experimental mode + const experimentalContext = { ...baseContext, experimentalMode: true }; const committersUrl = `https://sentry.io/api/0/projects/sentry-mcp-evals/CLOUDFLARE-MCP/events/${fixtureEventId}/committers/`; - function mockEvent( - options: { type?: string; formatted?: unknown } = {}, - eventSelector = "latest", - ) { + function mockEvent(eventSelector = "latest") { mswServer.use( http.get( `https://sentry.io/api/0/organizations/sentry-mcp-evals/issues/6507376925/events/${eventSelector}/`, @@ -2729,7 +2496,6 @@ describe("suspect commits", () => { id: fixtureEventId, eventID: fixtureEventId, }), - ...options, }), ), ); @@ -2742,14 +2508,9 @@ describe("suspect commits", () => { }); describe.each([ - { mode: "structured JSON", type: "error", formatted, structured: true }, - { - mode: "Markdown without the formatter rollout", - type: "error", - formatted: undefined, - structured: false, - }, - ])("$mode", ({ type, formatted, structured }) => { + { mode: "structured JSON", context: experimentalContext, structured: true }, + { mode: "Markdown", context: baseContext, structured: false }, + ])("$mode", ({ context, structured }) => { it.each([ { selection: "latest event", eventId: undefined }, { @@ -2759,7 +2520,7 @@ describe("suspect commits", () => { ])( "includes the suspect commit for the $selection", async ({ eventId }) => { - mockEvent({ type, formatted }, eventId); + mockEvent(eventId); mswServer.use( http.get(committersUrl, () => HttpResponse.json({ @@ -2781,7 +2542,7 @@ describe("suspect commits", () => { const result = await getIssueDetails.handler( { ...params, eventId }, - baseContext, + context, ); if (structured) { @@ -2809,7 +2570,7 @@ describe("suspect commits", () => { }); it("uses the author's email and omits a null commit message from Markdown", async () => { - mockEvent({ formatted: undefined }); + mockEvent(); mswServer.use( http.get(committersUrl, () => HttpResponse.json({ @@ -2868,13 +2629,13 @@ describe("suspect commits", () => { ])( "preserves issue details for $failure (reported: $reported)", async ({ status, body, reported }) => { - mockEvent({ formatted }); + mockEvent(); const logIssue = vi.spyOn(logging, "logIssue").mockReturnValue(undefined); mswServer.use( http.get(committersUrl, () => HttpResponse.json(body, { status })), ); - const result = await getIssueDetails.handler(params, baseContext); + const result = await getIssueDetails.handler(params, experimentalContext); const payload = getIssueDetailsOutputSchema.parse( getStructuredContent(result), ); diff --git a/packages/mcp-core/src/tools/catalog/get-issue-details.ts b/packages/mcp-core/src/tools/catalog/get-issue-details.ts index 76674722a..f5118606b 100644 --- a/packages/mcp-core/src/tools/catalog/get-issue-details.ts +++ b/packages/mcp-core/src/tools/catalog/get-issue-details.ts @@ -15,12 +15,14 @@ import type { import { ConfigurationError, UserInputError } from "../../errors"; import type { CodeLocation } from "../../internal/code-location"; import { + buildIssueResponseNotes, dedupeReplayIds, + formatEventOutput, getReplayIdFromEvent, getSeerActionabilityLabel, getSuspectCommit, isPerformanceIssueType, - usesSharedFormatterBody, + isSupportedEventType, } from "../../internal/formatting"; import type { AIConversationReference } from "../../internal/tool-helpers/ai-conversation-actions"; import { apiServiceFromContext } from "../../internal/tool-helpers/api"; @@ -60,10 +62,9 @@ const TRACE_ID_PATTERN = /^[0-9a-fA-F]{32}$/; * Every field is mapped explicitly rather than spread from an api response, so a passthrough * upstream schema cannot leak backend-only fields into the public interface. * - * `event.body` is the one open record: it is whatever Sentry's shared formatter emits - * for `?llmFormat=json`, and its sections are decided there. Enumerating them here would make - * this schema a second declaration of that contract, needing a bump every time a section is - * added on the Sentry side. + * `event.body` stays a markdown string. It is this server's own rendering of the event, and a + * stacktrace, a request body and a span tree are prose that an agent reads rather than fields + * it indexes -- everything worth addressing by name is already a sibling field here. */ export const getIssueDetailsOutputSchema = z.object({ issue: z.object({ @@ -91,7 +92,8 @@ export const getIssueDetailsOutputSchema = z.object({ id: z.string(), type: z.string().nullish(), occurredAt: z.string().nullish(), - body: z.record(z.string(), z.unknown()), + message: z.string().nullish(), + body: z.string(), }), seer: z .object({ @@ -143,6 +145,9 @@ export const getIssueDetailsOutputSchema = z.object({ }), ) .nullish(), + // the markdown output's Response Notes: mostly which tool to call next, and session + // dependent, so they are as much a part of the answer here as they are there + responseNotes: z.array(z.string()), }); export type GetIssueDetailsPayload = z.infer< @@ -150,38 +155,9 @@ export type GetIssueDetailsPayload = z.infer< >; /** - * Parses the shared formatter's json body. Returns undefined when the caller's org is not on - * the rollout yet, which is the signal to keep returning markdown: a structured result has to - * carry the whole answer, and without the body it would not. - */ -function parseFormattedBody(event: Event): Record | undefined { - // the same event-type gate the markdown path applies: a transaction still needs the local - // rendering, which carries the fetched performance trace that the shared body does not - if (!usesSharedFormatterBody(event)) { - return undefined; - } - const content = event.formatted?.content; - if (!content) { - return undefined; - } - try { - const parsed = JSON.parse(content); - return parsed && typeof parsed === "object" && !Array.isArray(parsed) - ? (parsed as Record) - : undefined; - } catch { - return undefined; - } -} - -/** - * The attached replay plus the related ones, derived the way the markdown output derives them. - * The attached id lives on the event rather than in the related list, so passing that list - * through alone loses the replay for an issue whose only one is attached. - */ -/** - * ``dateCreated`` sits on the shared-formatter event types rather than the base union, and - * the markdown path normalizes it to ISO. Read it the same way and tolerate a bad value. + * ``dateCreated`` sits on the error, default, generic and csp event types rather than the base + * union, and the markdown path normalizes it to ISO. Read it the same way and tolerate a bad + * value. */ function eventOccurredAt(event: Event): string | null { const raw = "dateCreated" in event ? event.dateCreated : null; @@ -192,6 +168,11 @@ function eventOccurredAt(event: Event): string | null { return Number.isNaN(parsed.getTime()) ? null : parsed.toISOString(); } +/** + * The attached replay plus the related ones, derived the way the markdown output derives them. + * The attached id lives on the event rather than in the related list, so passing that list + * through alone loses the replay for an issue whose only one is attached. + */ function buildReplays( event: Event, relatedReplayIds?: string[], @@ -215,26 +196,32 @@ function buildIssueDetailsPayload({ organizationSlug, issue, event, - body, apiService, autofixState, + performanceTrace, externalIssues, relatedReplayIds, aiConversations, codeLocation, committers, + experimentalMode, + availableToolNames, + directToolNames, }: { organizationSlug: string; issue: Issue; event: Event; - body: Record; apiService: SentryApiService; autofixState?: AutofixRunState; + performanceTrace?: Trace; externalIssues?: ExternalIssueList; relatedReplayIds?: string[]; aiConversations?: AIConversationReference[]; codeLocation?: CodeLocation; committers?: CommitterList; + experimentalMode?: boolean; + availableToolNames?: ReadonlySet; + directToolNames?: ReadonlySet; }): GetIssueDetailsPayload { const autofix = autofixState?.autofix; // the run's own artifacts, not the whole state: an AutofixRunState carries every step and @@ -276,7 +263,18 @@ function buildIssueDetailsPayload({ id: event.id, type: typeof event.type === "string" ? event.type : null, occurredAt: eventOccurredAt(event), - body, + // the markdown output prints this above the body; formatEventOutput only renders a + // message entry, so an event with a top level message alone would otherwise lose it + message: + typeof event.message === "string" && event.message.length > 0 + ? event.message + : null, + // no replaySummary: replays are a field of their own below, and rendering the note + // here too would answer the same question twice + body: formatEventOutput(event, { + performanceTrace, + stripReplayIds: true, + }), }, seer: autofix ? { @@ -312,6 +310,16 @@ function buildIssueDetailsPayload({ spanId: conversation.spanId, })) : null, + responseNotes: buildIssueResponseNotes({ + organizationSlug, + issue, + event, + apiService, + aiConversations, + experimentalMode, + availableToolNames, + directToolNames, + }), }; } @@ -368,9 +376,9 @@ export default defineTool({ issueUrl: ParamIssueUrl.optional(), }, // outputSchema is deliberately not declared yet. tools/list would export it immediately, - // while an org that is not on sentry's formatter rollout still gets a markdown result with - // no structuredContent -- advertising a schema that some success paths cannot satisfy. Wire - // it up once the rollout guarantees a json body on every event. + // while a session outside experimental mode still gets a markdown result with no + // structuredContent -- advertising a schema that some success paths cannot satisfy. Wire it + // up once the structured payload is the only thing this tool returns. annotations: { readOnlyHint: true, destructiveHint: false, @@ -451,27 +459,31 @@ export default defineTool({ }), ]); - const body = parseFormattedBody(event); - if (body) { + // an unsupported event type stays on the markdown path, which reports it and warns + // rather than rendering a body it cannot + if (context.experimentalMode && isSupportedEventType(event)) { return structuredResult( buildIssueDetailsPayload({ organizationSlug: orgSlug, issue, event, - body, apiService, autofixState, + performanceTrace, externalIssues, relatedReplayIds, aiConversations, codeLocation, committers, + experimentalMode: context.experimentalMode, + availableToolNames: context.availableToolNames, + directToolNames: context.directToolNames, }), ); } - // no shared-formatter body for this org yet: keep returning markdown rather than a - // structured result that is missing the event itself + // structuredResult returns no markdown at all, so outside experimental mode keep the + // handwritten output that clients not reading structuredContent still depend on return formatIssueOutput({ organizationSlug: orgSlug, issue, @@ -559,27 +571,31 @@ export default defineTool({ }), ]); - const body = parseFormattedBody(event); - if (body) { + // an unsupported event type stays on the markdown path, which reports it and warns + // rather than rendering a body it cannot + if (context.experimentalMode && isSupportedEventType(event)) { return structuredResult( buildIssueDetailsPayload({ organizationSlug: orgSlug, issue, event, - body, apiService, autofixState, + performanceTrace, externalIssues, relatedReplayIds, aiConversations, codeLocation, committers, + experimentalMode: context.experimentalMode, + availableToolNames: context.availableToolNames, + directToolNames: context.directToolNames, }), ); } - // no shared-formatter body for this org yet: keep returning markdown rather than a - // structured result that is missing the event itself + // structuredResult returns no markdown at all, so outside experimental mode keep the + // handwritten output that clients not reading structuredContent still depend on return formatIssueOutput({ organizationSlug: orgSlug, issue, From 8b3dfed235a8ace198dc9f4fddf5b672d7b2ed7b Mon Sep 17 00:00:00 2001 From: Shayna Chambless Date: Mon, 5 Oct 2026 13:58:08 -0700 Subject: [PATCH 2/9] comments --- packages/mcp-core/src/internal/formatting.ts | 13 ----- .../tools/catalog/get-issue-details.test.ts | 21 +------- .../src/tools/catalog/get-issue-details.ts | 48 ++----------------- 3 files changed, 6 insertions(+), 76 deletions(-) diff --git a/packages/mcp-core/src/internal/formatting.ts b/packages/mcp-core/src/internal/formatting.ts index 6bbe2dc10..24e4faa99 100644 --- a/packages/mcp-core/src/internal/formatting.ts +++ b/packages/mcp-core/src/internal/formatting.ts @@ -198,10 +198,6 @@ export function isPerformanceIssueType(issue: { ); } -/** - * Whether this server renders the event type. The Event union is ErrorEvent | DefaultEvent | - * TransactionEvent | GenericEvent | CspEvent, but in practice other types arrive as UnknownEvent. - */ export function isSupportedEventType(event: { type?: unknown }): boolean { return ( event.type === "error" || @@ -232,8 +228,6 @@ export function formatEventOutput( availableToolNames?: ReadonlySet; directToolNames?: ReadonlySet; }; - // strip the replay ids without rendering the note, for callers that report replays - // themselves and would otherwise repeat them in tags and contexts stripReplayIds?: boolean; }, ) { @@ -2230,13 +2224,6 @@ export function formatIssueOutput({ return output; } -/** - * The response notes as individual lines, without bullets. - * - * Both the markdown output and the structured payload need these, and they are mostly - * tool-call instructions whose availability depends on the session, so they are built once - * here rather than restated per output shape. - */ export function buildIssueResponseNotes({ organizationSlug, issue, diff --git a/packages/mcp-core/src/tools/catalog/get-issue-details.test.ts b/packages/mcp-core/src/tools/catalog/get-issue-details.test.ts index 29c0dee64..a6ebea00c 100644 --- a/packages/mcp-core/src/tools/catalog/get-issue-details.test.ts +++ b/packages/mcp-core/src/tools/catalog/get-issue-details.test.ts @@ -2135,8 +2135,6 @@ describe("get_issue_details", () => { }); describe("structuredContent", () => { - // structuredResult returns no markdown, so the structured payload is gated on experimental - // mode until it is the only thing the tool returns const experimentalContext = { ...baseContext, experimentalMode: true }; const params = { @@ -2192,8 +2190,6 @@ describe("structuredContent", () => { await getIssueDetails.handler(params, experimentalContext), ); - // the headings are this server's own, so their presence is what shows the body was - // rendered here rather than handed over by the api expect(typeof payload.event.body).toBe("string"); expect(payload.event.body).toContain("### Error"); expect(payload.event.body).toContain("Something went wrong"); @@ -2218,7 +2214,6 @@ describe("structuredContent", () => { await getIssueDetails.handler(params, experimentalContext), ); - // dropping these would leave the structured path strictly worse than the markdown expect(payload.responseNotes.length).toBeGreaterThan(0); const notes = payload.responseNotes.join("\n"); expect(notes).toContain("Fixes CLOUDFLARE-MCP-41"); @@ -2226,8 +2221,6 @@ describe("structuredContent", () => { }); it("carries the top level message, which the body does not render", async () => { - // the markdown output prints event.message above the body; formatEventOutput only - // renders a message entry, so the payload has to carry it separately mockLatestEvent({ message: "TOP-LEVEL-MESSAGE", entries: [] }); const payload = payloadOf( @@ -2238,7 +2231,6 @@ describe("structuredContent", () => { }); it("keeps an unsupported event type on the markdown path", async () => { - // the markdown path reports the type and warns; a structured body would render nothing mswServer.use( http.get( "https://sentry.io/api/0/organizations/*/issues/7777777777/events/latest/", @@ -2260,7 +2252,6 @@ describe("structuredContent", () => { }); it("renders a transaction, whose body carries the performance trace", async () => { - // the local renderer covers every event type, so a transaction takes the same path mswServer.use( http.get( "https://sentry.io/api/0/organizations/sentry-mcp-evals/issues/PERF-N1-001/", @@ -2284,7 +2275,6 @@ describe("structuredContent", () => { }); it("keeps the attached replay, which lives on the event not the related list", async () => { - // an issue whose only replay is attached would otherwise report no replays at all mockLatestEvent({ contexts: { replay: { @@ -2303,7 +2293,6 @@ describe("structuredContent", () => { expect(payload.replays?.related).not.toContain( "1234567890abcdef1234567890abcdef", ); - // nor left in the tags and contexts the body renders, which would be a third copy expect(payload.event.body).not.toContain( "1234567890abcdef1234567890abcdef", ); @@ -2320,8 +2309,6 @@ describe("structuredContent", () => { }); it("maps external issues field by field so upstream extras cannot leak", async () => { - // structuredContent is a product contract, not a view of the api response: several - // upstream schemas are passthrough, so anything not mapped must not appear mockLatestEvent(); mswServer.use( http.get( @@ -2349,8 +2336,6 @@ describe("structuredContent", () => { }); it("carries every field the markdown output surfaces", async () => { - // greg's bar for this migration is "roughly the same content": anything the markdown - // renders and the payload drops is a regression for every MCP user mockLatestEvent({ dateCreated: "2026-09-03T12:00:00.000Z" }); const payload = payloadOf( @@ -2380,8 +2365,6 @@ describe("structuredContent", () => { }); it("does not label an error's exception message as a query pattern", async () => { - // metadata.value is a query pattern for a performance issue and the exception message for - // an error, so reading it unconditionally puts error text under the wrong name mswServer.use( http.get( "https://sentry.io/api/0/organizations/sentry-mcp-evals/issues/CLOUDFLARE-MCP-41/", @@ -2450,8 +2433,7 @@ describe("structuredContent", () => { ); mockLatestEvent(); mswServer.use( - // related ids come from replay-count, keyed by numeric issue id. Echo back whichever - // id was asked for: a preceding test can leave a different issue fixture registered. + // echo back whichever issue id was asked for http.get( "https://sentry.io/api/0/organizations/sentry-mcp-evals/replay-count/", ({ request }) => { @@ -2482,7 +2464,6 @@ describe("suspect commits", () => { issueUrl: undefined, regionUrl: null, }; - // the structured payload is gated on experimental mode const experimentalContext = { ...baseContext, experimentalMode: true }; const committersUrl = `https://sentry.io/api/0/projects/sentry-mcp-evals/CLOUDFLARE-MCP/events/${fixtureEventId}/committers/`; diff --git a/packages/mcp-core/src/tools/catalog/get-issue-details.ts b/packages/mcp-core/src/tools/catalog/get-issue-details.ts index f5118606b..f44934dfe 100644 --- a/packages/mcp-core/src/tools/catalog/get-issue-details.ts +++ b/packages/mcp-core/src/tools/catalog/get-issue-details.ts @@ -57,14 +57,8 @@ const AI_CONVERSATION_LOOKUP_WINDOW_MS = 24 * 60 * 60 * 1000; const TRACE_ID_PATTERN = /^[0-9a-fA-F]{32}$/; /** - * The issue payload as `structuredContent`. - * - * Every field is mapped explicitly rather than spread from an api response, so a passthrough - * upstream schema cannot leak backend-only fields into the public interface. - * - * `event.body` stays a markdown string. It is this server's own rendering of the event, and a - * stacktrace, a request body and a span tree are prose that an agent reads rather than fields - * it indexes -- everything worth addressing by name is already a sibling field here. + * The issue payload as `structuredContent`. Fields are mapped explicitly so passthrough api + * schemas can't leak backend-only fields. */ export const getIssueDetailsOutputSchema = z.object({ issue: z.object({ @@ -145,8 +139,6 @@ export const getIssueDetailsOutputSchema = z.object({ }), ) .nullish(), - // the markdown output's Response Notes: mostly which tool to call next, and session - // dependent, so they are as much a part of the answer here as they are there responseNotes: z.array(z.string()), }); @@ -154,11 +146,7 @@ export type GetIssueDetailsPayload = z.infer< typeof getIssueDetailsOutputSchema >; -/** - * ``dateCreated`` sits on the error, default, generic and csp event types rather than the base - * union, and the markdown path normalizes it to ISO. Read it the same way and tolerate a bad - * value. - */ +// dateCreated isn't on every event type function eventOccurredAt(event: Event): string | null { const raw = "dateCreated" in event ? event.dateCreated : null; if (typeof raw !== "string") { @@ -168,11 +156,6 @@ function eventOccurredAt(event: Event): string | null { return Number.isNaN(parsed.getTime()) ? null : parsed.toISOString(); } -/** - * The attached replay plus the related ones, derived the way the markdown output derives them. - * The attached id lives on the event rather than in the related list, so passing that list - * through alone loses the replay for an issue whose only one is attached. - */ function buildReplays( event: Event, relatedReplayIds?: string[], @@ -224,15 +207,12 @@ function buildIssueDetailsPayload({ directToolNames?: ReadonlySet; }): GetIssueDetailsPayload { const autofix = autofixState?.autofix; - // the run's own artifacts, not the whole state: an AutofixRunState carries every step and - // would dwarf the rest of the payload const summaries = autofix ? getAutofixArtifactSummaries(autofix) : undefined; const isPerf = isPerformanceIssueType(issue) && !!issue.metadata; return { issue: { shortId: issue.shortId, - // a performance issue's metadata carries the better title, as the markdown path prefers title: (isPerf ? issue.metadata?.title : null) || issue.title, culprit: issue.culprit, firstSeen: issue.firstSeen, @@ -254,8 +234,6 @@ function buildIssueDetailsPayload({ platform: issue.platform, project: issue.project?.name, url: apiService.getIssueUrl(organizationSlug, issue.shortId), - // metadata.value is a query pattern only for a performance issue; on an error it is the - // exception message, so reading it unconditionally would misname the error text location: isPerf ? issue.metadata?.location : null, queryPattern: isPerf ? issue.metadata?.value : null, }, @@ -263,14 +241,11 @@ function buildIssueDetailsPayload({ id: event.id, type: typeof event.type === "string" ? event.type : null, occurredAt: eventOccurredAt(event), - // the markdown output prints this above the body; formatEventOutput only renders a - // message entry, so an event with a top level message alone would otherwise lose it message: typeof event.message === "string" && event.message.length > 0 ? event.message : null, - // no replaySummary: replays are a field of their own below, and rendering the note - // here too would answer the same question twice + // replays are their own field below body: formatEventOutput(event, { performanceTrace, stripReplayIds: true, @@ -293,8 +268,6 @@ function buildIssueDetailsPayload({ : null, replays: buildReplays(event, relatedReplayIds), suspectCommit: getSuspectCommit(committers), - // mapped field by field, not handed through: several upstream schemas are passthrough, and - // structuredContent is a product contract rather than a view of the api response externalIssues: externalIssues?.length ? externalIssues.map((issue) => ({ id: String(issue.id), @@ -375,10 +348,7 @@ export default defineTool({ eventId: ParamEventId.optional(), issueUrl: ParamIssueUrl.optional(), }, - // outputSchema is deliberately not declared yet. tools/list would export it immediately, - // while a session outside experimental mode still gets a markdown result with no - // structuredContent -- advertising a schema that some success paths cannot satisfy. Wire it - // up once the structured payload is the only thing this tool returns. + // no outputSchema until every success path returns structuredContent annotations: { readOnlyHint: true, destructiveHint: false, @@ -459,8 +429,6 @@ export default defineTool({ }), ]); - // an unsupported event type stays on the markdown path, which reports it and warns - // rather than rendering a body it cannot if (context.experimentalMode && isSupportedEventType(event)) { return structuredResult( buildIssueDetailsPayload({ @@ -482,8 +450,6 @@ export default defineTool({ ); } - // structuredResult returns no markdown at all, so outside experimental mode keep the - // handwritten output that clients not reading structuredContent still depend on return formatIssueOutput({ organizationSlug: orgSlug, issue, @@ -571,8 +537,6 @@ export default defineTool({ }), ]); - // an unsupported event type stays on the markdown path, which reports it and warns - // rather than rendering a body it cannot if (context.experimentalMode && isSupportedEventType(event)) { return structuredResult( buildIssueDetailsPayload({ @@ -594,8 +558,6 @@ export default defineTool({ ); } - // structuredResult returns no markdown at all, so outside experimental mode keep the - // handwritten output that clients not reading structuredContent still depend on return formatIssueOutput({ organizationSlug: orgSlug, issue, From 732021cb17220e7b4cc76975d2f5fe10841aba44 Mon Sep 17 00:00:00 2001 From: Shayna Chambless Date: Mon, 5 Oct 2026 14:29:15 -0700 Subject: [PATCH 3/9] comments --- packages/mcp-core/src/internal/formatting.ts | 9 ++++----- packages/mcp-core/src/tools/catalog/get-issue-details.ts | 5 +---- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/packages/mcp-core/src/internal/formatting.ts b/packages/mcp-core/src/internal/formatting.ts index 24e4faa99..6ea7613e5 100644 --- a/packages/mcp-core/src/internal/formatting.ts +++ b/packages/mcp-core/src/internal/formatting.ts @@ -198,6 +198,7 @@ export function isPerformanceIssueType(issue: { ); } +/** Whether this server can render the event type; anything else arrives as an UnknownEvent. */ export function isSupportedEventType(event: { type?: unknown }): boolean { return ( event.type === "error" || @@ -228,6 +229,7 @@ export function formatEventOutput( availableToolNames?: ReadonlySet; directToolNames?: ReadonlySet; }; + // strip replay ids without rendering the replay note, for callers that report replays separately stripReplayIds?: boolean; }, ) { @@ -1946,8 +1948,7 @@ function formatSeerSummary(autofixState: AutofixRunState | undefined): string { parts.push(""); } - // Summarize from the run's artifacts: the solution if available, otherwise - // the root cause if it has been identified. + // Summarize the solution if available, otherwise the root cause. const { rootCause, solution } = getAutofixArtifactSummaries(autofix); if (solution) { parts.push("**Summary:**"); @@ -2160,9 +2161,7 @@ export function formatIssueOutput({ output += `**Event ID**: ${event.id}\n`; output += `**Type**: ${event.type}\n`; - // "default" type represents error events without exception data - // "generic" type represents performance regressions and metric-based issues - // "csp" type represents Content Security Policy violations + // default: error without exception data, generic: performance/metric issue, csp: CSP violation if ( event.type === "error" || event.type === "default" || diff --git a/packages/mcp-core/src/tools/catalog/get-issue-details.ts b/packages/mcp-core/src/tools/catalog/get-issue-details.ts index f44934dfe..7b4d17ac9 100644 --- a/packages/mcp-core/src/tools/catalog/get-issue-details.ts +++ b/packages/mcp-core/src/tools/catalog/get-issue-details.ts @@ -56,10 +56,7 @@ const MAX_AI_CONVERSATION_MATCHES = 3; const AI_CONVERSATION_LOOKUP_WINDOW_MS = 24 * 60 * 60 * 1000; const TRACE_ID_PATTERN = /^[0-9a-fA-F]{32}$/; -/** - * The issue payload as `structuredContent`. Fields are mapped explicitly so passthrough api - * schemas can't leak backend-only fields. - */ +/** The issue payload as `structuredContent`, mapped field by field so api passthrough fields can't leak. */ export const getIssueDetailsOutputSchema = z.object({ issue: z.object({ shortId: z.string(), From f189588e388b6ecb0be2c636b24cd28da808a8c7 Mon Sep 17 00:00:00 2001 From: Shayna Chambless Date: Mon, 5 Oct 2026 14:32:10 -0700 Subject: [PATCH 4/9] clean up --- packages/mcp-core/src/internal/formatting.ts | 29 ++----- .../src/tools/catalog/get-issue-details.ts | 78 ++++--------------- 2 files changed, 22 insertions(+), 85 deletions(-) diff --git a/packages/mcp-core/src/internal/formatting.ts b/packages/mcp-core/src/internal/formatting.ts index 6ea7613e5..fb2aa8d13 100644 --- a/packages/mcp-core/src/internal/formatting.ts +++ b/packages/mcp-core/src/internal/formatting.ts @@ -229,8 +229,6 @@ export function formatEventOutput( availableToolNames?: ReadonlySet; directToolNames?: ReadonlySet; }; - // strip replay ids without rendering the replay note, for callers that report replays separately - stripReplayIds?: boolean; }, ) { let output = ""; @@ -239,10 +237,9 @@ export function formatEventOutput( user?: z.infer["user"]; } ).user; - const eventToRender = - options?.replaySummary || options?.stripReplayIds - ? stripReplayMetadata(event) - : event; + const eventToRender = options?.replaySummary + ? stripReplayMetadata(event) + : event; if (options?.replaySummary) { output += formatIssueReplayOutput({ @@ -2146,13 +2143,15 @@ export function formatIssueOutput({ if (aiConversations && aiConversations.length > 0) { output += "\n## Response Notes\n\n"; - output += formatAIConversationResponseNote({ + for (const note of buildAIConversationResponseNotes({ aiConversations, organizationSlug, experimentalMode: experimentalMode ?? false, availableToolNames, directToolNames, - }); + })) { + output += `- ${note}\n`; + } } // For unsupported event types, return early without trying to render event details @@ -2413,18 +2412,6 @@ function buildAIConversationResponseNotes({ ]; } -function formatAIConversationResponseNote(args: { - aiConversations: AIConversationReference[]; - organizationSlug: string; - experimentalMode: boolean; - availableToolNames?: ReadonlySet; - directToolNames?: ReadonlySet; -}): string { - return `${buildAIConversationResponseNotes(args) - .map((note) => `- ${note}`) - .join("\n")}\n`; -} - const MAX_DISPLAY_REPLAYS = 5; function formatIssueReplayOutput({ @@ -2560,7 +2547,7 @@ function normalizeReplayId(replayId: string | null | undefined): string | null { return trimmedReplayId.replace(/-/g, ""); } -function stripReplayMetadata(event: Event): Event { +export function stripReplayMetadata(event: Event): Event { const tags = event.tags?.filter( (tag) => tag.key !== "replay.id" && tag.key !== "replayId", ); diff --git a/packages/mcp-core/src/tools/catalog/get-issue-details.ts b/packages/mcp-core/src/tools/catalog/get-issue-details.ts index 7b4d17ac9..16aff4c0e 100644 --- a/packages/mcp-core/src/tools/catalog/get-issue-details.ts +++ b/packages/mcp-core/src/tools/catalog/get-issue-details.ts @@ -23,6 +23,7 @@ import { getSuspectCommit, isPerformanceIssueType, isSupportedEventType, + stripReplayMetadata, } from "../../internal/formatting"; import type { AIConversationReference } from "../../internal/tool-helpers/ai-conversation-actions"; import { apiServiceFromContext } from "../../internal/tool-helpers/api"; @@ -172,6 +173,15 @@ function buildReplays( }; } +type IssueDetailsArgs = Parameters[0]; + +function issueDetailsResult(args: IssueDetailsArgs) { + if (args.experimentalMode && isSupportedEventType(args.event)) { + return structuredResult(buildIssueDetailsPayload(args)); + } + return formatIssueOutput(args); +} + function buildIssueDetailsPayload({ organizationSlug, issue, @@ -187,22 +197,7 @@ function buildIssueDetailsPayload({ experimentalMode, availableToolNames, directToolNames, -}: { - organizationSlug: string; - issue: Issue; - event: Event; - apiService: SentryApiService; - autofixState?: AutofixRunState; - performanceTrace?: Trace; - externalIssues?: ExternalIssueList; - relatedReplayIds?: string[]; - aiConversations?: AIConversationReference[]; - codeLocation?: CodeLocation; - committers?: CommitterList; - experimentalMode?: boolean; - availableToolNames?: ReadonlySet; - directToolNames?: ReadonlySet; -}): GetIssueDetailsPayload { +}: IssueDetailsArgs): GetIssueDetailsPayload { const autofix = autofixState?.autofix; const summaries = autofix ? getAutofixArtifactSummaries(autofix) : undefined; const isPerf = isPerformanceIssueType(issue) && !!issue.metadata; @@ -243,10 +238,7 @@ function buildIssueDetailsPayload({ ? event.message : null, // replays are their own field below - body: formatEventOutput(event, { - performanceTrace, - stripReplayIds: true, - }), + body: formatEventOutput(stripReplayMetadata(event), { performanceTrace }), }, seer: autofix ? { @@ -426,28 +418,7 @@ export default defineTool({ }), ]); - if (context.experimentalMode && isSupportedEventType(event)) { - return structuredResult( - buildIssueDetailsPayload({ - organizationSlug: orgSlug, - issue, - event, - apiService, - autofixState, - performanceTrace, - externalIssues, - relatedReplayIds, - aiConversations, - codeLocation, - committers, - experimentalMode: context.experimentalMode, - availableToolNames: context.availableToolNames, - directToolNames: context.directToolNames, - }), - ); - } - - return formatIssueOutput({ + return issueDetailsResult({ organizationSlug: orgSlug, issue, event, @@ -534,28 +505,7 @@ export default defineTool({ }), ]); - if (context.experimentalMode && isSupportedEventType(event)) { - return structuredResult( - buildIssueDetailsPayload({ - organizationSlug: orgSlug, - issue, - event, - apiService, - autofixState, - performanceTrace, - externalIssues, - relatedReplayIds, - aiConversations, - codeLocation, - committers, - experimentalMode: context.experimentalMode, - availableToolNames: context.availableToolNames, - directToolNames: context.directToolNames, - }), - ); - } - - return formatIssueOutput({ + return issueDetailsResult({ organizationSlug: orgSlug, issue, event, From 7a0ea8898c97bfeefe62fc74bc2a39030a3af454 Mon Sep 17 00:00:00 2001 From: Shayna Chambless Date: Mon, 5 Oct 2026 14:36:44 -0700 Subject: [PATCH 5/9] import --- .../mcp-core/src/tools/catalog/analyze-issue-with-seer.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/mcp-core/src/tools/catalog/analyze-issue-with-seer.test.ts b/packages/mcp-core/src/tools/catalog/analyze-issue-with-seer.test.ts index 80742fe13..e5b68895d 100644 --- a/packages/mcp-core/src/tools/catalog/analyze-issue-with-seer.test.ts +++ b/packages/mcp-core/src/tools/catalog/analyze-issue-with-seer.test.ts @@ -4,7 +4,7 @@ import { createUnsupportedIssue, mswServer, } from "@sentry/mcp-server-mocks"; -import { HttpResponse, http } from "msw"; +import { http, HttpResponse } from "msw"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import analyzeIssueWithSeer from "./analyze-issue-with-seer.js"; From 5cf522dd85a3f02b01a0dbcf0187e522603bc2cf Mon Sep 17 00:00:00 2001 From: Shayna Chambless Date: Mon, 5 Oct 2026 14:50:19 -0700 Subject: [PATCH 6/9] restore --- packages/mcp-core/src/internal/formatting.ts | 9 +- .../tools/catalog/get-issue-details.test.ts | 116 ++++++++++-------- .../src/tools/catalog/get-issue-details.ts | 31 ++++- 3 files changed, 98 insertions(+), 58 deletions(-) diff --git a/packages/mcp-core/src/internal/formatting.ts b/packages/mcp-core/src/internal/formatting.ts index fb2aa8d13..6ce581b92 100644 --- a/packages/mcp-core/src/internal/formatting.ts +++ b/packages/mcp-core/src/internal/formatting.ts @@ -237,9 +237,10 @@ export function formatEventOutput( user?: z.infer["user"]; } ).user; - const eventToRender = options?.replaySummary + const eventWithReplayMetadataStripped = options?.replaySummary ? stripReplayMetadata(event) : event; + const eventToRender = eventWithReplayMetadataStripped; if (options?.replaySummary) { output += formatIssueReplayOutput({ @@ -1945,7 +1946,8 @@ function formatSeerSummary(autofixState: AutofixRunState | undefined): string { parts.push(""); } - // Summarize the solution if available, otherwise the root cause. + // Summarize from the run's artifacts: the solution if available, otherwise + // the root cause if it has been identified. const { rootCause, solution } = getAutofixArtifactSummaries(autofix); if (solution) { parts.push("**Summary:**"); @@ -2111,6 +2113,9 @@ export function formatIssueOutput({ output += "## Event Details\n\n"; + // Check if this is an unsupported event type + // Event type union is: ErrorEvent | DefaultEvent | TransactionEvent | GenericEvent | CspEvent + // But in practice we may have other types returned as UnknownEvent const eventType = event.type; if (!isSupportedEventType(event)) { // Log to Sentry for tracking new/unknown event types diff --git a/packages/mcp-core/src/tools/catalog/get-issue-details.test.ts b/packages/mcp-core/src/tools/catalog/get-issue-details.test.ts index a6ebea00c..8bc2e613b 100644 --- a/packages/mcp-core/src/tools/catalog/get-issue-details.test.ts +++ b/packages/mcp-core/src/tools/catalog/get-issue-details.test.ts @@ -2154,18 +2154,14 @@ describe("structuredContent", () => { ); } - function payloadOf(result: unknown): Record { - expect(result).toHaveProperty("structuredContent"); - return (result as { structuredContent: Record }) - .structuredContent; - } - it("returns a structured payload in experimental mode", async () => { mockLatestEvent(); - const payload = payloadOf( - await getIssueDetails.handler(params, experimentalContext), - ); + const result = await getIssueDetails.handler(params, experimentalContext); + + expect(result).toHaveProperty("structuredContent"); + const payload = (result as { structuredContent: Record }) + .structuredContent; // the issue level fields the markdown used to assemble expect(payload.issue.shortId).toBe("CLOUDFLARE-MCP-41"); @@ -2186,9 +2182,9 @@ describe("structuredContent", () => { it("renders the event body itself rather than asking the api for one", async () => { mockLatestEvent(); - const payload = payloadOf( - await getIssueDetails.handler(params, experimentalContext), - ); + const result = await getIssueDetails.handler(params, experimentalContext); + const payload = (result as { structuredContent: Record }) + .structuredContent; expect(typeof payload.event.body).toBe("string"); expect(payload.event.body).toContain("### Error"); @@ -2199,9 +2195,9 @@ describe("structuredContent", () => { it("produces a payload that satisfies the schema", async () => { mockLatestEvent(); - const payload = payloadOf( - await getIssueDetails.handler(params, experimentalContext), - ); + const result = await getIssueDetails.handler(params, experimentalContext); + const payload = (result as { structuredContent: Record }) + .structuredContent; // a tool that advertises a schema has to return something that satisfies it expect(() => getIssueDetailsOutputSchema.parse(payload)).not.toThrow(); @@ -2210,9 +2206,9 @@ describe("structuredContent", () => { it("carries the response notes, which say which tool to call next", async () => { mockLatestEvent(); - const payload = payloadOf( - await getIssueDetails.handler(params, experimentalContext), - ); + const result = await getIssueDetails.handler(params, experimentalContext); + const payload = (result as { structuredContent: Record }) + .structuredContent; expect(payload.responseNotes.length).toBeGreaterThan(0); const notes = payload.responseNotes.join("\n"); @@ -2223,9 +2219,9 @@ describe("structuredContent", () => { it("carries the top level message, which the body does not render", async () => { mockLatestEvent({ message: "TOP-LEVEL-MESSAGE", entries: [] }); - const payload = payloadOf( - await getIssueDetails.handler(params, experimentalContext), - ); + const result = await getIssueDetails.handler(params, experimentalContext); + const payload = (result as { structuredContent: Record }) + .structuredContent; expect(payload.event.message).toBe("TOP-LEVEL-MESSAGE"); }); @@ -2263,18 +2259,19 @@ describe("structuredContent", () => { ), ); - const payload = payloadOf( - await getIssueDetails.handler( - { ...params, issueId: "PERF-N1-001" }, - experimentalContext, - ), + const result = await getIssueDetails.handler( + { ...params, issueId: "PERF-N1-001" }, + experimentalContext, ); + const payload = (result as { structuredContent: Record }) + .structuredContent; expect(payload.event.type).toBe("transaction"); expect(payload.event.body).toContain("Span"); }); it("keeps the attached replay, which lives on the event not the related list", async () => { + // an issue whose only replay is attached would otherwise report no replays at all mockLatestEvent({ contexts: { replay: { @@ -2284,9 +2281,9 @@ describe("structuredContent", () => { }, }); - const payload = payloadOf( - await getIssueDetails.handler(params, experimentalContext), - ); + const result = await getIssueDetails.handler(params, experimentalContext); + const payload = (result as { structuredContent: Record }) + .structuredContent; expect(payload.replays?.attached).toBe("1234567890abcdef1234567890abcdef"); // and the attached id is not repeated in the related list @@ -2301,14 +2298,16 @@ describe("structuredContent", () => { it("reports no replays when there are none", async () => { mockLatestEvent(); - const payload = payloadOf( - await getIssueDetails.handler(params, experimentalContext), - ); + const result = await getIssueDetails.handler(params, experimentalContext); + const payload = (result as { structuredContent: Record }) + .structuredContent; expect(payload.replays).toBeNull(); }); it("maps external issues field by field so upstream extras cannot leak", async () => { + // structuredContent is a product contract, not a view of the api response: several + // upstream schemas are passthrough, so anything not mapped must not appear mockLatestEvent(); mswServer.use( http.get( @@ -2327,20 +2326,22 @@ describe("structuredContent", () => { ), ); - const payload = payloadOf( - await getIssueDetails.handler(params, experimentalContext), - ); + const result = await getIssueDetails.handler(params, experimentalContext); + const payload = (result as { structuredContent: Record }) + .structuredContent; expect(JSON.stringify(payload)).not.toContain("internalOnlyToken"); expect(JSON.stringify(payload)).not.toContain("must-not-leak"); }); it("carries every field the markdown output surfaces", async () => { + // greg's bar for this migration is "roughly the same content": anything the markdown + // renders and the payload drops is a regression for every MCP user mockLatestEvent({ dateCreated: "2026-09-03T12:00:00.000Z" }); - const payload = payloadOf( - await getIssueDetails.handler(params, experimentalContext), - ); + const result = await getIssueDetails.handler(params, experimentalContext); + const payload = (result as { structuredContent: Record }) + .structuredContent; // the issue header markdown builds before the event body for (const field of [ @@ -2365,6 +2366,8 @@ describe("structuredContent", () => { }); it("does not label an error's exception message as a query pattern", async () => { + // metadata.value is a query pattern for a performance issue and the exception message for + // an error, so reading it unconditionally puts error text under the wrong name mswServer.use( http.get( "https://sentry.io/api/0/organizations/sentry-mcp-evals/issues/CLOUDFLARE-MCP-41/", @@ -2383,12 +2386,15 @@ describe("structuredContent", () => { issueCategory: "error", }), ), + http.get( + "https://sentry.io/api/0/organizations/sentry-mcp-evals/issues/7890123456/events/latest/", + () => HttpResponse.json(createDefaultEvent()), + ), ); - mockLatestEvent(); - const payload = payloadOf( - await getIssueDetails.handler(params, experimentalContext), - ); + const result = await getIssueDetails.handler(params, experimentalContext); + const payload = (result as { structuredContent: Record }) + .structuredContent; expect(payload.issue.queryPattern).toBeNull(); expect(payload.issue.location).toBeNull(); @@ -2401,8 +2407,8 @@ describe("structuredContent", () => { http.get( "https://sentry.io/api/0/organizations/sentry-mcp-evals/issues/CLOUDFLARE-MCP-41/", () => - HttpResponse.json( - createPerformanceIssue({ + HttpResponse.json({ + ...createPerformanceIssue({ shortId: "CLOUDFLARE-MCP-41", issueType: "performance_n_plus_one_db_queries", issueCategory: "performance", @@ -2412,14 +2418,17 @@ describe("structuredContent", () => { location: "/api/checkout", }, }), - ), + }), + ), + http.get( + "https://sentry.io/api/0/organizations/sentry-mcp-evals/issues/7890123456/events/latest/", + () => HttpResponse.json(createDefaultEvent()), ), ); - mockLatestEvent(); - const payload = payloadOf( - await getIssueDetails.handler(params, experimentalContext), - ); + const result = await getIssueDetails.handler(params, experimentalContext); + const payload = (result as { structuredContent: Record }) + .structuredContent; expect(payload.issue.title).toBe("N+1 Query"); expect(payload.issue.queryPattern).toBe("SELECT * FROM users WHERE id = ?"); @@ -2433,7 +2442,8 @@ describe("structuredContent", () => { ); mockLatestEvent(); mswServer.use( - // echo back whichever issue id was asked for + // related ids come from replay-count, keyed by numeric issue id. Echo back whichever + // id was asked for: a preceding test can leave a different issue fixture registered. http.get( "https://sentry.io/api/0/organizations/sentry-mcp-evals/replay-count/", ({ request }) => { @@ -2444,9 +2454,9 @@ describe("structuredContent", () => { ), ); - const payload = payloadOf( - await getIssueDetails.handler(params, experimentalContext), - ); + const result = await getIssueDetails.handler(params, experimentalContext); + const payload = (result as { structuredContent: Record }) + .structuredContent; expect(payload.replays).not.toBeNull(); expect(payload.replays.relatedCount).toBe(51); diff --git a/packages/mcp-core/src/tools/catalog/get-issue-details.ts b/packages/mcp-core/src/tools/catalog/get-issue-details.ts index 16aff4c0e..6476c88cf 100644 --- a/packages/mcp-core/src/tools/catalog/get-issue-details.ts +++ b/packages/mcp-core/src/tools/catalog/get-issue-details.ts @@ -57,7 +57,14 @@ const MAX_AI_CONVERSATION_MATCHES = 3; const AI_CONVERSATION_LOOKUP_WINDOW_MS = 24 * 60 * 60 * 1000; const TRACE_ID_PATTERN = /^[0-9a-fA-F]{32}$/; -/** The issue payload as `structuredContent`, mapped field by field so api passthrough fields can't leak. */ +/** + * The issue payload as `structuredContent`. + * + * Every field is mapped explicitly rather than spread from an api response, so a passthrough + * upstream schema cannot leak backend-only fields into the public interface. + * + * `event.body` is this server's own markdown rendering of the event. + */ export const getIssueDetailsOutputSchema = z.object({ issue: z.object({ shortId: z.string(), @@ -144,7 +151,15 @@ export type GetIssueDetailsPayload = z.infer< typeof getIssueDetailsOutputSchema >; -// dateCreated isn't on every event type +/** + * The attached replay plus the related ones, derived the way the markdown output derives them. + * The attached id lives on the event rather than in the related list, so passing that list + * through alone loses the replay for an issue whose only one is attached. + */ +/** + * ``dateCreated`` sits on the error, default, generic and csp event types rather than the base + * union, and the markdown path normalizes it to ISO. Read it the same way and tolerate a bad value. + */ function eventOccurredAt(event: Event): string | null { const raw = "dateCreated" in event ? event.dateCreated : null; if (typeof raw !== "string") { @@ -199,12 +214,15 @@ function buildIssueDetailsPayload({ directToolNames, }: IssueDetailsArgs): GetIssueDetailsPayload { const autofix = autofixState?.autofix; + // the run's own artifacts, not the whole state: an AutofixRunState carries every step and + // would dwarf the rest of the payload const summaries = autofix ? getAutofixArtifactSummaries(autofix) : undefined; const isPerf = isPerformanceIssueType(issue) && !!issue.metadata; return { issue: { shortId: issue.shortId, + // a performance issue's metadata carries the better title, as the markdown path prefers title: (isPerf ? issue.metadata?.title : null) || issue.title, culprit: issue.culprit, firstSeen: issue.firstSeen, @@ -226,6 +244,8 @@ function buildIssueDetailsPayload({ platform: issue.platform, project: issue.project?.name, url: apiService.getIssueUrl(organizationSlug, issue.shortId), + // metadata.value is a query pattern only for a performance issue; on an error it is the + // exception message, so reading it unconditionally would misname the error text location: isPerf ? issue.metadata?.location : null, queryPattern: isPerf ? issue.metadata?.value : null, }, @@ -257,6 +277,8 @@ function buildIssueDetailsPayload({ : null, replays: buildReplays(event, relatedReplayIds), suspectCommit: getSuspectCommit(committers), + // mapped field by field, not handed through: several upstream schemas are passthrough, and + // structuredContent is a product contract rather than a view of the api response externalIssues: externalIssues?.length ? externalIssues.map((issue) => ({ id: String(issue.id), @@ -337,7 +359,10 @@ export default defineTool({ eventId: ParamEventId.optional(), issueUrl: ParamIssueUrl.optional(), }, - // no outputSchema until every success path returns structuredContent + // outputSchema is deliberately not declared yet. tools/list would export it immediately, + // while a session outside experimental mode still gets a markdown result with + // no structuredContent -- advertising a schema that some success paths cannot satisfy. Wire + // it up once every success path returns structuredContent. annotations: { readOnlyHint: true, destructiveHint: false, From 78dd35ae040fa0024eb03fb8c4dcae4e475087b5 Mon Sep 17 00:00:00 2001 From: Shayna Chambless Date: Mon, 5 Oct 2026 16:15:55 -0700 Subject: [PATCH 7/9] fix --- packages/mcp-core/src/internal/formatting.ts | 109 +++++------------ .../tools/catalog/get-issue-details.test.ts | 111 +++++++++++------- .../src/tools/catalog/get-issue-details.ts | 29 ++--- 3 files changed, 111 insertions(+), 138 deletions(-) diff --git a/packages/mcp-core/src/internal/formatting.ts b/packages/mcp-core/src/internal/formatting.ts index 6ce581b92..d84e5b150 100644 --- a/packages/mcp-core/src/internal/formatting.ts +++ b/packages/mcp-core/src/internal/formatting.ts @@ -2117,7 +2117,14 @@ export function formatIssueOutput({ // Event type union is: ErrorEvent | DefaultEvent | TransactionEvent | GenericEvent | CspEvent // But in practice we may have other types returned as UnknownEvent const eventType = event.type; - if (!isSupportedEventType(event)) { + const isUnsupportedType = + eventType !== "error" && + eventType !== "default" && + eventType !== "transaction" && + eventType !== "generic" && + eventType !== "csp"; + + if (isUnsupportedType) { // Log to Sentry for tracking new/unknown event types const sentryEventId = logIssue( `Unsupported event type encountered: ${String(eventType)}`, @@ -2148,15 +2155,13 @@ export function formatIssueOutput({ if (aiConversations && aiConversations.length > 0) { output += "\n## Response Notes\n\n"; - for (const note of buildAIConversationResponseNotes({ + output += formatAIConversationResponseNote({ aiConversations, organizationSlug, experimentalMode: experimentalMode ?? false, availableToolNames, directToolNames, - })) { - output += `- ${note}\n`; - } + }); } // For unsupported event types, return early without trying to render event details @@ -2211,67 +2216,27 @@ export function formatIssueOutput({ output += "\n"; } - output += "## Response Notes\n\n"; - for (const note of buildIssueResponseNotes({ - organizationSlug, - issue, - event, - apiService, - aiConversations, - experimentalMode, - availableToolNames, - directToolNames, - })) { - output += `- ${note}\n`; - } - return output; -} - -export function buildIssueResponseNotes({ - organizationSlug, - issue, - event, - apiService, - aiConversations, - experimentalMode, - availableToolNames, - directToolNames, -}: { - organizationSlug: string; - issue: Issue; - event: Event; - apiService: SentryApiService; - aiConversations?: AIConversationReference[]; - experimentalMode?: boolean; - availableToolNames?: ReadonlySet; - directToolNames?: ReadonlySet; -}): string[] { - const notes: string[] = []; const traceId = typeof event.contexts?.trace?.trace_id === "string" && event.contexts.trace.trace_id.length > 0 ? event.contexts.trace.trace_id : undefined; + output += "## Response Notes\n\n"; const commitIssueReference = /^\d+$/.test(issue.shortId) ? apiService.getIssueUrl(organizationSlug, issue.shortId) : issue.shortId; - notes.push( - `Commit message issue reference: \`Fixes ${commitIssueReference}\` automatically closes the issue when the commit is merged.`, - ); - notes.push( - "The stacktrace includes first-party application code and third-party code. First-party frames are usually the best starting point for triage.", - ); + output += `- Commit message issue reference: \`Fixes ${commitIssueReference}\` automatically closes the issue when the commit is merged.\n`; + output += + "- The stacktrace includes first-party application code and third-party code. First-party frames are usually the best starting point for triage.\n"; if (aiConversations && aiConversations.length > 0) { - notes.push( - ...buildAIConversationResponseNotes({ - aiConversations, - organizationSlug, - experimentalMode: experimentalMode ?? false, - availableToolNames, - directToolNames, - }), - ); + output += formatAIConversationResponseNote({ + aiConversations, + organizationSlug, + experimentalMode: experimentalMode ?? false, + availableToolNames, + directToolNames, + }); } const issueEventSearchInstruction = formatToolCallInstruction({ toolName: "search_issue_events", @@ -2285,7 +2250,7 @@ export function buildIssueResponseNotes({ directToolNames, fallbackInstruction: "Issue event search is not available in this session", }); - notes.push(`Issue event search: ${issueEventSearchInstruction}`); + output += `- Issue event search: ${issueEventSearchInstruction}\n`; const hasMultipleThreads = event.entries?.some((entry) => { if (entry.type !== "threads") { return false; @@ -2310,7 +2275,7 @@ export function buildIssueResponseNotes({ "to fetch a full thread stacktrace by numeric Thread ID or exact thread Name. Omit `thread` to use Sentry's default selected thread", }); if (stacktraceInstruction) { - notes.push(`Thread stacktrace lookup: ${stacktraceInstruction}`); + output += `- Thread stacktrace lookup: ${stacktraceInstruction}\n`; } } if (traceId) { @@ -2351,11 +2316,9 @@ export function buildIssueResponseNotes({ fallbackInstruction: "Related log search is not available in this session", }); - notes.push( - `Full distributed trace and span tree: ${traceDetailsInstruction}`, - ); - notes.push(`Related span search: ${spanSearchInstruction}`); - notes.push(`Related log search: ${logSearchInstruction}`); + output += `- Full distributed trace and span tree: ${traceDetailsInstruction}\n`; + output += `- Related span search: ${spanSearchInstruction}\n`; + output += `- Related log search: ${logSearchInstruction}\n`; } if (experimentalMode) { const breadcrumbsInstruction = formatToolCallInstruction({ @@ -2369,14 +2332,12 @@ export function buildIssueResponseNotes({ fallbackInstruction: "Issue breadcrumbs are not available in this session", }); - notes.push( - `Breadcrumb trail leading up to this error: ${breadcrumbsInstruction}`, - ); + output += `- Breadcrumb trail leading up to this error: ${breadcrumbsInstruction}\n`; } - return notes; + return output; } -function buildAIConversationResponseNotes({ +function formatAIConversationResponseNote({ aiConversations, organizationSlug, experimentalMode, @@ -2388,7 +2349,7 @@ function buildAIConversationResponseNotes({ experimentalMode: boolean; availableToolNames?: ReadonlySet; directToolNames?: ReadonlySet; -}): string[] { +}): string { const instructions = formatAIConversationActionInstructions({ organizationSlug, aiConversations, @@ -2402,19 +2363,13 @@ function buildAIConversationResponseNotes({ const spanSuffix = conversation.spanId ? ` Matching span: \`${conversation.spanId}\`.` : ""; - return [ - `Agent conversation found in this trace: \`${conversation.conversationId}\`.${spanSuffix}`, - ...instructions, - ]; + return `- Agent conversation found in this trace: \`${conversation.conversationId}\`.${spanSuffix}\n${instructions.map((instruction) => `- ${instruction}`).join("\n")}\n`; } const conversationIds = aiConversations .map((conversation) => `\`${conversation.conversationId}\``) .join(", "); - return [ - `Multiple agent conversations were found in this trace: ${conversationIds}.`, - ...instructions, - ]; + return `- Multiple agent conversations were found in this trace: ${conversationIds}.\n${instructions.map((instruction) => `- ${instruction}`).join("\n")}\n`; } const MAX_DISPLAY_REPLAYS = 5; diff --git a/packages/mcp-core/src/tools/catalog/get-issue-details.test.ts b/packages/mcp-core/src/tools/catalog/get-issue-details.test.ts index 8bc2e613b..95c9dd974 100644 --- a/packages/mcp-core/src/tools/catalog/get-issue-details.test.ts +++ b/packages/mcp-core/src/tools/catalog/get-issue-details.test.ts @@ -2137,14 +2137,6 @@ describe("get_issue_details", () => { describe("structuredContent", () => { const experimentalContext = { ...baseContext, experimentalMode: true }; - const params = { - organizationSlug: "sentry-mcp-evals", - issueId: "CLOUDFLARE-MCP-41", - eventId: undefined, - issueUrl: undefined, - regionUrl: null, - }; - function mockLatestEvent(overrides: Record = {}) { mswServer.use( http.get( @@ -2154,6 +2146,14 @@ describe("structuredContent", () => { ); } + const params = { + organizationSlug: "sentry-mcp-evals", + issueId: "CLOUDFLARE-MCP-41", + eventId: undefined, + issueUrl: undefined, + regionUrl: null, + }; + it("returns a structured payload in experimental mode", async () => { mockLatestEvent(); @@ -2170,6 +2170,17 @@ describe("structuredContent", () => { expect(typeof payload.issue.usersImpacted).toBe("number"); }); + it("produces a payload that satisfies the schema", async () => { + mockLatestEvent(); + + const result = await getIssueDetails.handler(params, experimentalContext); + const payload = (result as { structuredContent: unknown }) + .structuredContent; + + // a tool that advertises a schema has to return something that satisfies it + expect(() => getIssueDetailsOutputSchema.parse(payload)).not.toThrow(); + }); + it("returns markdown outside experimental mode", async () => { mockLatestEvent(); @@ -2192,30 +2203,6 @@ describe("structuredContent", () => { expect(payload.event.body).toContain("### Tags"); }); - it("produces a payload that satisfies the schema", async () => { - mockLatestEvent(); - - const result = await getIssueDetails.handler(params, experimentalContext); - const payload = (result as { structuredContent: Record }) - .structuredContent; - - // a tool that advertises a schema has to return something that satisfies it - expect(() => getIssueDetailsOutputSchema.parse(payload)).not.toThrow(); - }); - - it("carries the response notes, which say which tool to call next", async () => { - mockLatestEvent(); - - const result = await getIssueDetails.handler(params, experimentalContext); - const payload = (result as { structuredContent: Record }) - .structuredContent; - - expect(payload.responseNotes.length).toBeGreaterThan(0); - const notes = payload.responseNotes.join("\n"); - expect(notes).toContain("Fixes CLOUDFLARE-MCP-41"); - expect(notes).toContain("search_issue_events"); - }); - it("carries the top level message, which the body does not render", async () => { mockLatestEvent({ message: "TOP-LEVEL-MESSAGE", entries: [] }); @@ -2272,14 +2259,21 @@ describe("structuredContent", () => { it("keeps the attached replay, which lives on the event not the related list", async () => { // an issue whose only replay is attached would otherwise report no replays at all - mockLatestEvent({ - contexts: { - replay: { - type: "default", - replay_id: "1234567890abcdef1234567890abcdef", - }, - }, - }); + mswServer.use( + http.get( + "https://sentry.io/api/0/organizations/sentry-mcp-evals/issues/6507376925/events/latest/", + () => + HttpResponse.json({ + ...createDefaultEvent(), + contexts: { + replay: { + type: "default", + replay_id: "1234567890abcdef1234567890abcdef", + }, + }, + }), + ), + ); const result = await getIssueDetails.handler(params, experimentalContext); const payload = (result as { structuredContent: Record }) @@ -2308,8 +2302,14 @@ describe("structuredContent", () => { it("maps external issues field by field so upstream extras cannot leak", async () => { // structuredContent is a product contract, not a view of the api response: several // upstream schemas are passthrough, so anything not mapped must not appear - mockLatestEvent(); mswServer.use( + http.get( + "https://sentry.io/api/0/organizations/sentry-mcp-evals/issues/6507376925/events/latest/", + () => + HttpResponse.json({ + ...createDefaultEvent(), + }), + ), http.get( "https://sentry.io/api/0/organizations/sentry-mcp-evals/issues/6507376925/external-issues/", () => @@ -2337,7 +2337,16 @@ describe("structuredContent", () => { it("carries every field the markdown output surfaces", async () => { // greg's bar for this migration is "roughly the same content": anything the markdown // renders and the payload drops is a regression for every MCP user - mockLatestEvent({ dateCreated: "2026-09-03T12:00:00.000Z" }); + mswServer.use( + http.get( + "https://sentry.io/api/0/organizations/sentry-mcp-evals/issues/6507376925/events/latest/", + () => + HttpResponse.json({ + ...createDefaultEvent(), + dateCreated: "2026-09-03T12:00:00.000Z", + }), + ), + ); const result = await getIssueDetails.handler(params, experimentalContext); const payload = (result as { structuredContent: Record }) @@ -2388,7 +2397,10 @@ describe("structuredContent", () => { ), http.get( "https://sentry.io/api/0/organizations/sentry-mcp-evals/issues/7890123456/events/latest/", - () => HttpResponse.json(createDefaultEvent()), + () => + HttpResponse.json({ + ...createDefaultEvent(), + }), ), ); @@ -2422,7 +2434,10 @@ describe("structuredContent", () => { ), http.get( "https://sentry.io/api/0/organizations/sentry-mcp-evals/issues/7890123456/events/latest/", - () => HttpResponse.json(createDefaultEvent()), + () => + HttpResponse.json({ + ...createDefaultEvent(), + }), ), ); @@ -2440,8 +2455,14 @@ describe("structuredContent", () => { const many = Array.from({ length: 51 }, (_, i) => i.toString(16).padStart(32, "0"), ); - mockLatestEvent(); mswServer.use( + http.get( + "https://sentry.io/api/0/organizations/sentry-mcp-evals/issues/6507376925/events/latest/", + () => + HttpResponse.json({ + ...createDefaultEvent(), + }), + ), // related ids come from replay-count, keyed by numeric issue id. Echo back whichever // id was asked for: a preceding test can leave a different issue fixture registered. http.get( diff --git a/packages/mcp-core/src/tools/catalog/get-issue-details.ts b/packages/mcp-core/src/tools/catalog/get-issue-details.ts index 6476c88cf..ffd2290c1 100644 --- a/packages/mcp-core/src/tools/catalog/get-issue-details.ts +++ b/packages/mcp-core/src/tools/catalog/get-issue-details.ts @@ -15,7 +15,6 @@ import type { import { ConfigurationError, UserInputError } from "../../errors"; import type { CodeLocation } from "../../internal/code-location"; import { - buildIssueResponseNotes, dedupeReplayIds, formatEventOutput, getReplayIdFromEvent, @@ -144,7 +143,6 @@ export const getIssueDetailsOutputSchema = z.object({ }), ) .nullish(), - responseNotes: z.array(z.string()), }); export type GetIssueDetailsPayload = z.infer< @@ -209,10 +207,19 @@ function buildIssueDetailsPayload({ aiConversations, codeLocation, committers, - experimentalMode, - availableToolNames, - directToolNames, -}: IssueDetailsArgs): GetIssueDetailsPayload { +}: { + organizationSlug: string; + issue: Issue; + event: Event; + apiService: SentryApiService; + autofixState?: AutofixRunState; + performanceTrace?: Trace; + externalIssues?: ExternalIssueList; + relatedReplayIds?: string[]; + aiConversations?: AIConversationReference[]; + codeLocation?: CodeLocation; + committers?: CommitterList; +}): GetIssueDetailsPayload { const autofix = autofixState?.autofix; // the run's own artifacts, not the whole state: an AutofixRunState carries every step and // would dwarf the rest of the payload @@ -294,16 +301,6 @@ function buildIssueDetailsPayload({ spanId: conversation.spanId, })) : null, - responseNotes: buildIssueResponseNotes({ - organizationSlug, - issue, - event, - apiService, - aiConversations, - experimentalMode, - availableToolNames, - directToolNames, - }), }; } From 5ea49db1d81069dbf5ad27faf0c3bbff93b914c4 Mon Sep 17 00:00:00 2001 From: Shayna Chambless Date: Mon, 5 Oct 2026 16:30:23 -0700 Subject: [PATCH 8/9] restore --- packages/mcp-core/src/internal/formatting.ts | 111 +++++++++++++----- .../tools/catalog/get-issue-details.test.ts | 13 ++ .../src/tools/catalog/get-issue-details.ts | 18 +++ 3 files changed, 112 insertions(+), 30 deletions(-) diff --git a/packages/mcp-core/src/internal/formatting.ts b/packages/mcp-core/src/internal/formatting.ts index d84e5b150..96ba4f4d3 100644 --- a/packages/mcp-core/src/internal/formatting.ts +++ b/packages/mcp-core/src/internal/formatting.ts @@ -2117,14 +2117,7 @@ export function formatIssueOutput({ // Event type union is: ErrorEvent | DefaultEvent | TransactionEvent | GenericEvent | CspEvent // But in practice we may have other types returned as UnknownEvent const eventType = event.type; - const isUnsupportedType = - eventType !== "error" && - eventType !== "default" && - eventType !== "transaction" && - eventType !== "generic" && - eventType !== "csp"; - - if (isUnsupportedType) { + if (!isSupportedEventType(event)) { // Log to Sentry for tracking new/unknown event types const sentryEventId = logIssue( `Unsupported event type encountered: ${String(eventType)}`, @@ -2216,27 +2209,67 @@ export function formatIssueOutput({ output += "\n"; } + output += "## Response Notes\n\n"; + for (const note of buildIssueResponseNotes({ + organizationSlug, + issue, + event, + apiService, + aiConversations, + experimentalMode, + availableToolNames, + directToolNames, + })) { + output += `- ${note}\n`; + } + return output; +} + +export function buildIssueResponseNotes({ + organizationSlug, + issue, + event, + apiService, + aiConversations, + experimentalMode, + availableToolNames, + directToolNames, +}: { + organizationSlug: string; + issue: Issue; + event: Event; + apiService: SentryApiService; + aiConversations?: AIConversationReference[]; + experimentalMode?: boolean; + availableToolNames?: ReadonlySet; + directToolNames?: ReadonlySet; +}): string[] { + const notes: string[] = []; const traceId = typeof event.contexts?.trace?.trace_id === "string" && event.contexts.trace.trace_id.length > 0 ? event.contexts.trace.trace_id : undefined; - output += "## Response Notes\n\n"; const commitIssueReference = /^\d+$/.test(issue.shortId) ? apiService.getIssueUrl(organizationSlug, issue.shortId) : issue.shortId; - output += `- Commit message issue reference: \`Fixes ${commitIssueReference}\` automatically closes the issue when the commit is merged.\n`; - output += - "- The stacktrace includes first-party application code and third-party code. First-party frames are usually the best starting point for triage.\n"; + notes.push( + `Commit message issue reference: \`Fixes ${commitIssueReference}\` automatically closes the issue when the commit is merged.`, + ); + notes.push( + "The stacktrace includes first-party application code and third-party code. First-party frames are usually the best starting point for triage.", + ); if (aiConversations && aiConversations.length > 0) { - output += formatAIConversationResponseNote({ - aiConversations, - organizationSlug, - experimentalMode: experimentalMode ?? false, - availableToolNames, - directToolNames, - }); + notes.push( + ...buildAIConversationResponseNotes({ + aiConversations, + organizationSlug, + experimentalMode: experimentalMode ?? false, + availableToolNames, + directToolNames, + }), + ); } const issueEventSearchInstruction = formatToolCallInstruction({ toolName: "search_issue_events", @@ -2250,7 +2283,7 @@ export function formatIssueOutput({ directToolNames, fallbackInstruction: "Issue event search is not available in this session", }); - output += `- Issue event search: ${issueEventSearchInstruction}\n`; + notes.push(`Issue event search: ${issueEventSearchInstruction}`); const hasMultipleThreads = event.entries?.some((entry) => { if (entry.type !== "threads") { return false; @@ -2275,7 +2308,7 @@ export function formatIssueOutput({ "to fetch a full thread stacktrace by numeric Thread ID or exact thread Name. Omit `thread` to use Sentry's default selected thread", }); if (stacktraceInstruction) { - output += `- Thread stacktrace lookup: ${stacktraceInstruction}\n`; + notes.push(`Thread stacktrace lookup: ${stacktraceInstruction}`); } } if (traceId) { @@ -2316,9 +2349,11 @@ export function formatIssueOutput({ fallbackInstruction: "Related log search is not available in this session", }); - output += `- Full distributed trace and span tree: ${traceDetailsInstruction}\n`; - output += `- Related span search: ${spanSearchInstruction}\n`; - output += `- Related log search: ${logSearchInstruction}\n`; + notes.push( + `Full distributed trace and span tree: ${traceDetailsInstruction}`, + ); + notes.push(`Related span search: ${spanSearchInstruction}`); + notes.push(`Related log search: ${logSearchInstruction}`); } if (experimentalMode) { const breadcrumbsInstruction = formatToolCallInstruction({ @@ -2332,12 +2367,14 @@ export function formatIssueOutput({ fallbackInstruction: "Issue breadcrumbs are not available in this session", }); - output += `- Breadcrumb trail leading up to this error: ${breadcrumbsInstruction}\n`; + notes.push( + `Breadcrumb trail leading up to this error: ${breadcrumbsInstruction}`, + ); } - return output; + return notes; } -function formatAIConversationResponseNote({ +function buildAIConversationResponseNotes({ aiConversations, organizationSlug, experimentalMode, @@ -2349,7 +2386,7 @@ function formatAIConversationResponseNote({ experimentalMode: boolean; availableToolNames?: ReadonlySet; directToolNames?: ReadonlySet; -}): string { +}): string[] { const instructions = formatAIConversationActionInstructions({ organizationSlug, aiConversations, @@ -2363,13 +2400,27 @@ function formatAIConversationResponseNote({ const spanSuffix = conversation.spanId ? ` Matching span: \`${conversation.spanId}\`.` : ""; - return `- Agent conversation found in this trace: \`${conversation.conversationId}\`.${spanSuffix}\n${instructions.map((instruction) => `- ${instruction}`).join("\n")}\n`; + return [ + `Agent conversation found in this trace: \`${conversation.conversationId}\`.${spanSuffix}`, + ...instructions, + ]; } const conversationIds = aiConversations .map((conversation) => `\`${conversation.conversationId}\``) .join(", "); - return `- Multiple agent conversations were found in this trace: ${conversationIds}.\n${instructions.map((instruction) => `- ${instruction}`).join("\n")}\n`; + return [ + `Multiple agent conversations were found in this trace: ${conversationIds}.`, + ...instructions, + ]; +} + +function formatAIConversationResponseNote( + args: Parameters[0], +): string { + return `${buildAIConversationResponseNotes(args) + .map((note) => `- ${note}`) + .join("\n")}\n`; } const MAX_DISPLAY_REPLAYS = 5; diff --git a/packages/mcp-core/src/tools/catalog/get-issue-details.test.ts b/packages/mcp-core/src/tools/catalog/get-issue-details.test.ts index 95c9dd974..84d75e0f4 100644 --- a/packages/mcp-core/src/tools/catalog/get-issue-details.test.ts +++ b/packages/mcp-core/src/tools/catalog/get-issue-details.test.ts @@ -2203,6 +2203,19 @@ describe("structuredContent", () => { expect(payload.event.body).toContain("### Tags"); }); + it("carries the response notes, which say which tool to call next", async () => { + mockLatestEvent(); + + const result = await getIssueDetails.handler(params, experimentalContext); + const payload = (result as { structuredContent: Record }) + .structuredContent; + + expect(payload.responseNotes.length).toBeGreaterThan(0); + const notes = payload.responseNotes.join("\n"); + expect(notes).toContain("Fixes CLOUDFLARE-MCP-41"); + expect(notes).toContain("search_issue_events"); + }); + it("carries the top level message, which the body does not render", async () => { mockLatestEvent({ message: "TOP-LEVEL-MESSAGE", entries: [] }); diff --git a/packages/mcp-core/src/tools/catalog/get-issue-details.ts b/packages/mcp-core/src/tools/catalog/get-issue-details.ts index ffd2290c1..7c0170065 100644 --- a/packages/mcp-core/src/tools/catalog/get-issue-details.ts +++ b/packages/mcp-core/src/tools/catalog/get-issue-details.ts @@ -15,6 +15,7 @@ import type { import { ConfigurationError, UserInputError } from "../../errors"; import type { CodeLocation } from "../../internal/code-location"; import { + buildIssueResponseNotes, dedupeReplayIds, formatEventOutput, getReplayIdFromEvent, @@ -143,6 +144,7 @@ export const getIssueDetailsOutputSchema = z.object({ }), ) .nullish(), + responseNotes: z.array(z.string()), }); export type GetIssueDetailsPayload = z.infer< @@ -207,6 +209,9 @@ function buildIssueDetailsPayload({ aiConversations, codeLocation, committers, + experimentalMode, + availableToolNames, + directToolNames, }: { organizationSlug: string; issue: Issue; @@ -219,6 +224,9 @@ function buildIssueDetailsPayload({ aiConversations?: AIConversationReference[]; codeLocation?: CodeLocation; committers?: CommitterList; + experimentalMode?: boolean; + availableToolNames?: ReadonlySet; + directToolNames?: ReadonlySet; }): GetIssueDetailsPayload { const autofix = autofixState?.autofix; // the run's own artifacts, not the whole state: an AutofixRunState carries every step and @@ -301,6 +309,16 @@ function buildIssueDetailsPayload({ spanId: conversation.spanId, })) : null, + responseNotes: buildIssueResponseNotes({ + organizationSlug, + issue, + event, + apiService, + aiConversations, + experimentalMode, + availableToolNames, + directToolNames, + }), }; } From a3e42e88863279421b4c5c45d7290e2840549e68 Mon Sep 17 00:00:00 2001 From: Shayna Chambless Date: Mon, 5 Oct 2026 16:40:15 -0700 Subject: [PATCH 9/9] function --- packages/mcp-core/src/internal/formatting.ts | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/packages/mcp-core/src/internal/formatting.ts b/packages/mcp-core/src/internal/formatting.ts index 96ba4f4d3..92e0e0dce 100644 --- a/packages/mcp-core/src/internal/formatting.ts +++ b/packages/mcp-core/src/internal/formatting.ts @@ -2148,13 +2148,15 @@ export function formatIssueOutput({ if (aiConversations && aiConversations.length > 0) { output += "\n## Response Notes\n\n"; - output += formatAIConversationResponseNote({ + output += buildAIConversationResponseNotes({ aiConversations, organizationSlug, experimentalMode: experimentalMode ?? false, availableToolNames, directToolNames, - }); + }) + .map((note) => `- ${note}\n`) + .join(""); } // For unsupported event types, return early without trying to render event details @@ -2415,14 +2417,6 @@ function buildAIConversationResponseNotes({ ]; } -function formatAIConversationResponseNote( - args: Parameters[0], -): string { - return `${buildAIConversationResponseNotes(args) - .map((note) => `- ${note}`) - .join("\n")}\n`; -} - const MAX_DISPLAY_REPLAYS = 5; function formatIssueReplayOutput({