From ff7977ab8a44f5ec4d6175ca69487b6a708fd7e2 Mon Sep 17 00:00:00 2001 From: Charlie Luo Date: Tue, 27 Oct 2026 10:25:42 -0700 Subject: [PATCH] fix(tools): Preserve issue activity source attribution Show the acting user or Sentry App alongside the MCP client, API, or CLI source in issue activity and comment results. Preserve explicit comment references and accept older responses without attribution metadata. Cover the schemas, formatted responses, and MCP catalog dispatch. Co-authored-by: Codex --- docs/contributing/api-patterns.md | 7 + .../mcp-core/src/api-client/schema.test.ts | 23 ++++ packages/mcp-core/src/api-client/schema.ts | 2 + packages/mcp-core/src/server.test.ts | 44 ++++++ .../src/tools/catalog/add-issue-note.test.ts | 49 ++++++- .../src/tools/catalog/add-issue-note.ts | 12 +- .../tools/catalog/get-issue-activity.test.ts | 128 ++++++++++++++---- .../src/tools/catalog/get-issue-activity.ts | 11 +- .../catalog/support/activity-formatting.ts | 44 ++++++ .../src/fixtures/issue-activity.json | 4 + .../src/fixtures/issue-comments.json | 2 + 11 files changed, 285 insertions(+), 41 deletions(-) create mode 100644 packages/mcp-core/src/tools/catalog/support/activity-formatting.ts diff --git a/docs/contributing/api-patterns.md b/docs/contributing/api-patterns.md index 82f86e5dc..106b6a53e 100644 --- a/docs/contributing/api-patterns.md +++ b/docs/contributing/api-patterns.md @@ -21,6 +21,13 @@ rg -n "class OrganizationAIConversations|ai-conversations" ~/src/sentry/src ~/sr Use the verified source contract to define Zod schemas, query parameters, pagination behavior, feature-gate handling, and test fixtures. +### Issue Activity Attribution + +`source` identifies the channel or client, such as `mcp:claude`; `user` and +`sentry_app` identify the actor. `commentId` is the reference accepted by the +notes endpoints and can differ from the activity `id`. For comment creation +responses, use `commentId ?? id`. Both `source` and `commentId` may be absent or null. + ### Client Creation ```typescript diff --git a/packages/mcp-core/src/api-client/schema.test.ts b/packages/mcp-core/src/api-client/schema.test.ts index af8279144..cab4ccf98 100644 --- a/packages/mcp-core/src/api-client/schema.test.ts +++ b/packages/mcp-core/src/api-client/schema.test.ts @@ -14,6 +14,7 @@ import { EventSchema, FlamegraphSchema, IssueSchema, + IssueActivitySchema, IssueTagValuesSchema, ProfileChunkResponseSchema, ProfileChunkSampleSchema, @@ -28,6 +29,28 @@ import { TransactionProfileSchema, } from "./schema"; +describe("IssueActivitySchema", () => { + it("types source metadata and the separate comment reference", () => { + const activity = { + id: "activity-1", + commentId: "comment-1", + source: "mcp:claude", + }; + expect(IssueActivitySchema.parse(activity)).toEqual(activity); + expect( + IssueActivitySchema.safeParse({ ...activity, source: 123 }).success, + ).toBe(false); + }); + + it("accepts missing or null metadata from older and system activity", () => { + expect(IssueActivitySchema.parse({ id: "activity-1" })).toEqual({ + id: "activity-1", + }); + const activity = { id: "activity-1", commentId: null, source: null }; + expect(IssueActivitySchema.parse(activity)).toEqual(activity); + }); +}); + describe("IssueSchema", () => { it("should parse a standard error issue", () => { const errorIssue = { diff --git a/packages/mcp-core/src/api-client/schema.ts b/packages/mcp-core/src/api-client/schema.ts index 3ffea37c8..191e3df1f 100644 --- a/packages/mcp-core/src/api-client/schema.ts +++ b/packages/mcp-core/src/api-client/schema.ts @@ -830,6 +830,8 @@ export const CommittersResponseSchema = z.object({ export const IssueActivitySchema = z .object({ id: ApiResourceIdSchema, + commentId: ApiResourceIdSchema.nullable().optional(), + source: z.string().nullable().optional(), type: z.string().nullable().optional(), dateCreated: z.string().datetime().nullable().optional(), user: ApiActorSchema.nullable().optional(), diff --git a/packages/mcp-core/src/server.test.ts b/packages/mcp-core/src/server.test.ts index 1256b4f09..3b685213a 100644 --- a/packages/mcp-core/src/server.test.ts +++ b/packages/mcp-core/src/server.test.ts @@ -1513,6 +1513,50 @@ describe("buildServer", () => { ); }); + it.each([ + ["get_issue_activity", {}], + ["add_issue_note", { text: "Investigating after the latest deploy." }], + ] as const)( + "execute_sentry_tool preserves attribution for %s", + async (name, args) => { + mswServer.use( + http.post( + "*/api/0/organizations/:org/issues/:issueId/notes/", + ({ params }) => { + expect(params.org).toBe("sentry-mcp-evals"); + return HttpResponse.json({ + id: "activity-1", + commentId: "comment-1", + type: "note", + source: "mcp:claude", + user: { name: "Jane Developer" }, + data: { text: "Investigating after the latest deploy." }, + }); + }, + ), + ); + const server = buildServer({ + context: { + ...baseContext, + constraints: { + organizationSlug: "sentry-mcp-evals", + projectSlug: null, + }, + }, + }); + + const result = await callRegisteredTool(server, "execute_sentry_tool", { + name, + arguments: { issueId: "CLOUDFLARE-MCP-41", ...args }, + }); + + expect(result.isError).not.toBe(true); + expect(getTextContent(result)).toContain( + "Jane Developer (Claude via MCP)", + ); + }, + ); + it("execute_sentry_tool dispatches to catalog-only event stacktrace", async () => { const server = buildServer({ context: baseContext, diff --git a/packages/mcp-core/src/tools/catalog/add-issue-note.test.ts b/packages/mcp-core/src/tools/catalog/add-issue-note.test.ts index f9862f3e5..b4124c22e 100644 --- a/packages/mcp-core/src/tools/catalog/add-issue-note.test.ts +++ b/packages/mcp-core/src/tools/catalog/add-issue-note.test.ts @@ -1,4 +1,6 @@ -import { describe, expect, it } from "vitest"; +import { afterEach, describe, expect, it } from "vitest"; +import { mswServer } from "@sentry/mcp-server-mocks"; +import { http, HttpResponse } from "msw"; import addIssueNote from "./add-issue-note.js"; const context = { @@ -9,7 +11,52 @@ const context = { userId: "1", }; +afterEach(() => { + mswServer.resetHandlers(); +}); + describe("add_issue_note", () => { + it("includes MCP attribution and the comment reference returned by Sentry", async () => { + mswServer.use( + http.post("*/api/0/organizations/:org/issues/:issueId/notes/", () => + HttpResponse.json({ + id: "activity-1", + commentId: "comment-1", + type: "note", + source: "mcp:claude", + user: { name: "Jane Developer" }, + dateCreated: "2025-04-14T12:34:56.000Z", + data: { text: "Investigating after the latest deploy." }, + }), + ), + ); + + const result = await addIssueNote.handler( + { + organizationSlug: "sentry-mcp-evals", + regionUrl: null, + issueId: "CLOUDFLARE-MCP-41", + text: "Investigating after the latest deploy.", + }, + context, + ); + + expect(result).toMatchInlineSnapshot(` + "# Added Note to Issue CLOUDFLARE-MCP-41 in **sentry-mcp-evals** + + **Comment ID**: comment-1 + **Type**: note + **Author**: Jane Developer (Claude via MCP) + **Created**: 2025-04-14T12:34:56.000Z + **Text**: Investigating after the latest deploy. + + ## Response Notes + + - The note is visible in the issue activity feed." + `); + expect(result).not.toContain("activity-1"); + }); + it("serializes the created note", async () => { const result = await addIssueNote.handler( { diff --git a/packages/mcp-core/src/tools/catalog/add-issue-note.ts b/packages/mcp-core/src/tools/catalog/add-issue-note.ts index 1a0403c69..fd99eef4c 100644 --- a/packages/mcp-core/src/tools/catalog/add-issue-note.ts +++ b/packages/mcp-core/src/tools/catalog/add-issue-note.ts @@ -14,19 +14,15 @@ import { } from "../../schema"; import { setTargetTagsAndAttributes } from "../../telem/scope"; import type { ServerContext } from "../../types"; -import { - formatActor, - formatDate, - formatId, - readString, -} from "./support/api-formatting"; +import { formatDate, formatId, readString } from "./support/api-formatting"; +import { formatActivityAuthor } from "./support/activity-formatting"; function formatComment(comment: IssueComment): string { - const actor = comment.user ? formatActor(comment.user) : "current user"; + const actor = formatActivityAuthor(comment, "current user"); const text = comment.data ? readString(comment.data, "text") : null; return [ - `**Comment ID**: ${formatId(comment.id)}`, + `**Comment ID**: ${formatId(comment.commentId ?? comment.id)}`, `**Type**: ${comment.type ?? "note"}`, `**Author**: ${actor}`, formatDate(comment.dateCreated) diff --git a/packages/mcp-core/src/tools/catalog/get-issue-activity.test.ts b/packages/mcp-core/src/tools/catalog/get-issue-activity.test.ts index 35cebdeec..ec8e36232 100644 --- a/packages/mcp-core/src/tools/catalog/get-issue-activity.test.ts +++ b/packages/mcp-core/src/tools/catalog/get-issue-activity.test.ts @@ -1,5 +1,7 @@ import { mswServer } from "@sentry/mcp-server-mocks"; import { afterEach, describe, expect, it } from "vitest"; +import { http, HttpResponse } from "msw"; +import type { IssueActivity } from "../../api-client/types"; import getIssueActivity from "./get-issue-activity.js"; const context = { @@ -10,22 +12,32 @@ const context = { userId: "1", }; +const params = { + organizationSlug: "sentry-mcp-evals", + regionUrl: null, + issueId: "CLOUDFLARE-MCP-41", + includeComments: true, + limit: 25, +}; + +function mockActivity(activity: IssueActivity) { + mswServer.use( + http.get("*/api/0/organizations/:org/issues/:issueId/activities/", () => + HttpResponse.json({ activity: [activity] }), + ), + http.get("*/api/0/organizations/:org/issues/:issueId/notes/", () => + HttpResponse.json([activity]), + ), + ); +} + afterEach(() => { mswServer.resetHandlers(); }); describe("get_issue_activity", () => { it("serializes issue activity and comments", async () => { - const result = await getIssueActivity.handler( - { - organizationSlug: "sentry-mcp-evals", - regionUrl: null, - issueId: "CLOUDFLARE-MCP-41", - includeComments: true, - limit: 25, - }, - context, - ); + const result = await getIssueActivity.handler(params, context); expect(result).toMatchInlineSnapshot(` "# Activity for Issue CLOUDFLARE-MCP-41 in **sentry-mcp-evals** @@ -34,12 +46,77 @@ describe("get_issue_activity", () => { - 2025-04-10T22:55:22.411Z: auto_set_ongoing by system (4633815464) - Data: {"after_days":7} - - 2025-04-14T12:00:00.000Z: note by Jane Developer (4633816000) + - 2025-04-14T12:00:00.000Z: note by Jane Developer (Claude via MCP) (4633816000) + - Investigating after the latest deploy. + + ## Comments + + - 2025-04-14T12:00:00.000Z: note by Jane Developer (Claude via MCP) (4633816000) + - Investigating after the latest deploy. + + ## Response Notes + + - Use the Sentry tool \`add_issue_note\` to add a new human-visible issue comment. + " + `); + }); + + it.each([ + ["mcp:claude-code", "Claude via MCP"], + ["mcp:cursor", "Cursor via MCP"], + ["mcp", "via MCP"], + ["mcp:new-client", "via MCP"], + ["api", "via API"], + ["sentry-cli", "via Sentry CLI"], + ["web", null], + [null, null], + [undefined, null], + ])( + "formats source %s in both activity and comments", + async (source, label) => { + mockActivity({ + id: "activity-1", + type: "note", + source, + user: { name: "Jane Developer" }, + }); + const result = await getIssueActivity.handler(params, context); + const attribution = label + ? `Jane Developer (${label})` + : "Jane Developer"; + const [activity, comments] = result.split("## Comments"); + expect(activity).toContain(`by ${attribution} (activity-1)`); + expect(comments).toContain(`by ${attribution} (activity-1)`); + expect(result).not.toContain("Comment ID:"); + }, + ); + + it("attributes Sentry Apps by name and separates comment IDs from activity IDs", async () => { + mockActivity({ + id: "activity-1", + commentId: "comment-1", + type: "note", + dateCreated: "2025-04-14T12:00:00.000Z", + source: "api", + user: null, + sentry_app: { name: "Example App", slug: "example-app" }, + data: { text: "Investigating after the latest deploy." }, + }); + + const result = await getIssueActivity.handler(params, context); + expect(result).toMatchInlineSnapshot(` + "# Activity for Issue CLOUDFLARE-MCP-41 in **sentry-mcp-evals** + + ## Activity + + - 2025-04-14T12:00:00.000Z: note by Example App (activity-1) + - Comment ID: comment-1 - Investigating after the latest deploy. ## Comments - - 2025-04-14T12:00:00.000Z: note by Jane Developer (4633816000) + - 2025-04-14T12:00:00.000Z: note by Example App (activity-1) + - Comment ID: comment-1 - Investigating after the latest deploy. ## Response Notes @@ -47,26 +124,19 @@ describe("get_issue_activity", () => { - Use the Sentry tool \`add_issue_note\` to add a new human-visible issue comment. " `); + expect(result).not.toContain("by system"); + expect(result).not.toContain("via API"); }); it("omits add_issue_note guidance when the tool is unavailable", async () => { - const result = await getIssueActivity.handler( - { - organizationSlug: "sentry-mcp-evals", - regionUrl: null, - issueId: "CLOUDFLARE-MCP-41", - includeComments: true, - limit: 25, - }, - { - ...context, - availableToolNames: new Set([ - "get_issue_activity", - "execute_sentry_tool", - ]), - directToolNames: new Set(["execute_sentry_tool"]), - }, - ); + const result = await getIssueActivity.handler(params, { + ...context, + availableToolNames: new Set([ + "get_issue_activity", + "execute_sentry_tool", + ]), + directToolNames: new Set(["execute_sentry_tool"]), + }); expect(result).not.toContain("Response Notes"); expect(result).not.toContain("add_issue_note"); diff --git a/packages/mcp-core/src/tools/catalog/get-issue-activity.ts b/packages/mcp-core/src/tools/catalog/get-issue-activity.ts index 5bf5fa503..3cffc5d8c 100644 --- a/packages/mcp-core/src/tools/catalog/get-issue-activity.ts +++ b/packages/mcp-core/src/tools/catalog/get-issue-activity.ts @@ -17,12 +17,12 @@ import { import { setTargetTagsAndAttributes } from "../../telem/scope"; import type { ServerContext } from "../../types"; import { - formatActor, formatDate, formatId, formatUnknown, readString, } from "./support/api-formatting"; +import { formatActivityAuthor } from "./support/activity-formatting"; function getActivityText( activity: IssueActivity | IssueComment, @@ -41,7 +41,7 @@ function getActivityText( } function formatActivity(activity: IssueActivity | IssueComment): string { - const actor = activity.user ? formatActor(activity.user) : "system"; + const actor = formatActivityAuthor(activity); const date = formatDate(activity.dateCreated) ?? "unknown time"; const text = getActivityText(activity); const details = @@ -50,7 +50,12 @@ function formatActivity(activity: IssueActivity | IssueComment): string { Object.keys(activity.data).length > 0 ? `\n - Data: ${formatUnknown(activity.data)}` : ""; - return `- ${date}: ${activity.type ?? "activity"} by ${actor} (${formatId(activity.id)})${text ? `\n - ${text}` : ""}${details}`; + const commentId = activity.commentId; + const commentDetails = + commentId != null && String(commentId) !== String(activity.id) + ? `\n - Comment ID: ${formatId(commentId)}` + : ""; + return `- ${date}: ${activity.type ?? "activity"} by ${actor} (${formatId(activity.id)})${commentDetails}${text ? `\n - ${text}` : ""}${details}`; } export default defineTool({ diff --git a/packages/mcp-core/src/tools/catalog/support/activity-formatting.ts b/packages/mcp-core/src/tools/catalog/support/activity-formatting.ts new file mode 100644 index 000000000..a59fa0177 --- /dev/null +++ b/packages/mcp-core/src/tools/catalog/support/activity-formatting.ts @@ -0,0 +1,44 @@ +import type { IssueActivity } from "../../../api-client/types"; +import { formatActor, readString } from "./api-formatting"; + +const MCP_CLIENT_NAMES = new Map([ + ["claude", "Claude"], + ["claude-code", "Claude"], + ["codex", "Codex"], + ["copilot", "GitHub Copilot"], + ["cursor", "Cursor"], + ["opencode", "OpenCode"], +]); + +function formatActivitySource( + source: string | null | undefined, +): string | null { + const [channel, client] = source?.split(":") ?? []; + switch (channel) { + case "api": + return "via API"; + case "sentry-cli": + return "via Sentry CLI"; + case "mcp": { + const name = client ? MCP_CLIENT_NAMES.get(client) : undefined; + return name ? `${name} via MCP` : "via MCP"; + } + default: + return null; + } +} + +export function formatActivityAuthor( + activity: IssueActivity, + fallback = "system", +): string { + const actor = + readString(activity.sentry_app, "name") ?? + readString(activity.sentry_app, "slug") ?? + (activity.user ? formatActor(activity.user) : fallback); + // Sentry Apps are attributed by name rather than their API channel. + const source = activity.sentry_app + ? null + : formatActivitySource(activity.source); + return source ? `${actor} (${source})` : actor; +} diff --git a/packages/mcp-server-mocks/src/fixtures/issue-activity.json b/packages/mcp-server-mocks/src/fixtures/issue-activity.json index ec52d1c48..229a5e159 100644 --- a/packages/mcp-server-mocks/src/fixtures/issue-activity.json +++ b/packages/mcp-server-mocks/src/fixtures/issue-activity.json @@ -2,6 +2,8 @@ "activity": [ { "id": "4633815464", + "commentId": null, + "source": null, "user": null, "sentry_app": null, "type": "auto_set_ongoing", @@ -12,6 +14,8 @@ }, { "id": "4633816000", + "commentId": "4633816000", + "source": "mcp:claude", "user": { "id": "12345", "name": "Jane Developer", diff --git a/packages/mcp-server-mocks/src/fixtures/issue-comments.json b/packages/mcp-server-mocks/src/fixtures/issue-comments.json index 56c36bc2d..9bacd0c78 100644 --- a/packages/mcp-server-mocks/src/fixtures/issue-comments.json +++ b/packages/mcp-server-mocks/src/fixtures/issue-comments.json @@ -1,6 +1,8 @@ [ { "id": "4633816000", + "commentId": "4633816000", + "source": "mcp:claude", "user": { "id": "12345", "name": "Jane Developer",