diff --git a/packages/mcp-core/src/tools/catalog/update-issue.test.ts b/packages/mcp-core/src/tools/catalog/update-issue.test.ts index 1acebbd5b..cce729ab2 100644 --- a/packages/mcp-core/src/tools/catalog/update-issue.test.ts +++ b/packages/mcp-core/src/tools/catalog/update-issue.test.ts @@ -1,9 +1,14 @@ -import { afterEach, describe, expect, it } from "vitest"; -import { http, HttpResponse } from "msw"; import { issueFixture, mswServer } from "@sentry/mcp-server-mocks"; +import { HttpResponse, http } from "msw"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { logIssue } from "../../telem/logging"; import { prepareToolParams } from "../catalog-runtime/availability.js"; import updateIssue from "./update-issue.js"; +vi.mock("../../telem/logging", () => ({ + logIssue: vi.fn(), +})); + type MockIssue = typeof issueFixture; const serverContext = { @@ -23,6 +28,7 @@ function createIssue(overrides: Partial = {}): MockIssue { afterEach(() => { mswServer.resetHandlers(); + vi.clearAllMocks(); }); describe("update_issue", () => { @@ -1025,6 +1031,56 @@ describe("update_issue", () => { expect(result).toContain("**Comment not posted**"); }); + it("does not call logIssue when comment posting fails with a 403 permission error", async () => { + const currentIssue = createIssue({ + status: "unresolved", + statusDetails: {}, + }); + const updatedIssue = createIssue({ + status: "resolved", + statusDetails: {}, + }); + + mswServer.use( + http.get( + "https://sentry.io/api/0/organizations/sentry-mcp-evals/issues/CLOUDFLARE-MCP-41/", + () => HttpResponse.json(currentIssue), + ), + http.put( + "https://sentry.io/api/0/organizations/sentry-mcp-evals/issues/CLOUDFLARE-MCP-41/", + () => HttpResponse.json(updatedIssue), + ), + http.post( + "https://sentry.io/api/0/organizations/sentry-mcp-evals/issues/CLOUDFLARE-MCP-41/notes/", + () => + HttpResponse.json( + { detail: "You do not have permission to perform this action." }, + { status: 403 }, + ), + ), + ); + + const result = await updateIssue.handler( + { + organizationSlug: "sentry-mcp-evals", + issueId: "CLOUDFLARE-MCP-41", + status: "resolved", + assignedTo: undefined, + issueUrl: undefined, + regionUrl: null, + reason: "Resolving because fix deployed", + }, + serverContext, + ); + + // Update succeeded — output should show it + expect(result).toContain("**Status**: unresolved → **resolved**"); + // Comment failure should be reported gracefully, not thrown + expect(result).toContain("**Comment not posted**"); + // Expected 403 permission errors must NOT be logged as Sentry issues + expect(logIssue).not.toHaveBeenCalled(); + }); + it("strips null bytes from reason before posting as a comment", () => { // prepareToolParams runs the Zod schema (including transforms) the same // way the MCP server does at runtime, so this exercises the full parse path. diff --git a/packages/mcp-core/src/tools/catalog/update-issue.ts b/packages/mcp-core/src/tools/catalog/update-issue.ts index 3298e2e79..2b541421d 100644 --- a/packages/mcp-core/src/tools/catalog/update-issue.ts +++ b/packages/mcp-core/src/tools/catalog/update-issue.ts @@ -1,3 +1,4 @@ +import { ApiClientError } from "../../api-client"; import type { Issue } from "../../api-client/types"; import { UserInputError } from "../../errors"; import { apiServiceFromContext } from "../../internal/tool-helpers/api"; @@ -599,7 +600,9 @@ async function tryPostReasonComment( }); return { posted: true }; } catch (error) { - logIssue(error); + if (!(error instanceof ApiClientError)) { + logIssue(error); + } return { posted: false, error: error instanceof Error ? error.message : String(error),