diff --git a/docs/specs/subpath-constraints.md b/docs/specs/subpath-constraints.md index f5cdd9eab..983209876 100644 --- a/docs/specs/subpath-constraints.md +++ b/docs/specs/subpath-constraints.md @@ -44,6 +44,18 @@ If there’s a problem, you’ll receive a clear HTTP error when connecting: - 403: You don’t have access to the specified org/project - 404: Organization or project not found +For OAuth connections, project verification requests `collapse=organization` +to avoid fetching organization metadata that is not needed for project +capabilities. Successful verification is cached for 15 minutes. If project +verification takes longer than five seconds, the request continues with unknown +project capabilities. This result is cached for 60 seconds to reduce repeated +delays; after it expires, the next request retries verification. Timeout results +use separate cache keys, and verified results take precedence, so a concurrent +timeout cannot overwrite verified capabilities or shorten their cache lifetime. +Cache propagation and concurrent requests can still cause repeated verification +or temporarily return unknown capabilities. Each tool still requires permission +to access the requested data. + ## Region awareness For Sentry Cloud, your organization may be hosted in a regional cluster. When you scope by organization, we automatically determine the region (if available) and use it for API calls. You don’t need to take any action—this happens behind the scenes. For self-hosted Sentry, the region concept doesn’t apply. diff --git a/packages/mcp-cloudflare/src/server/lib/constraint-utils.test.ts b/packages/mcp-cloudflare/src/server/lib/constraint-utils.test.ts index c2a26b0bd..b560227fe 100644 --- a/packages/mcp-cloudflare/src/server/lib/constraint-utils.test.ts +++ b/packages/mcp-cloudflare/src/server/lib/constraint-utils.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, vi } from "vitest"; +import { afterEach, describe, it, expect, vi } from "vitest"; import "urlpattern-polyfill"; import { verifyConstraintsAccess, @@ -14,24 +14,68 @@ function createMockKV(options?: { getError?: Error; putError?: Error; }): KVNamespace { + const entries = new Map< + string, + { value: CachedConstraints; expiresAt: number } + >(); return { - get: vi.fn().mockImplementation(async () => { + get: vi.fn().mockImplementation(async (key: string) => { if (options?.getError) throw options.getError; + const entry = entries.get(key); + if (entry) { + return Date.now() < entry.expiresAt ? entry.value : null; + } return options?.getResult ?? null; }), - put: vi.fn().mockImplementation(async () => { - if (options?.putError) throw options.putError; - }), + put: vi + .fn() + .mockImplementation( + async ( + key: string, + value: string, + { expirationTtl }: { expirationTtl: number }, + ) => { + if (options?.putError) throw options.putError; + entries.set(key, { + value: JSON.parse(value), + expiresAt: Date.now() + expirationTtl * 1000, + }); + }, + ), delete: vi.fn(), list: vi.fn(), getWithMetadata: vi.fn(), } as unknown as KVNamespace; } +/** + * Override the project HTTP response while keeping the shared org interceptor. + * The Node-side fetchMock cannot follow workerd's fake timeout clock. + */ +function mockProjectResponse( + respond: (url: URL) => Response | Promise, +) { + const originalFetch = globalThis.fetch; + return vi.spyOn(globalThis, "fetch").mockImplementation((input, init) => { + const url = input instanceof Request ? input.url : String(input); + const projectUrl = + "https://us.sentry.io/api/0/projects/sentry-mcp-evals/cloudflare-mcp/"; + if (url === projectUrl || url === `${projectUrl}?collapse=organization`) { + return Promise.resolve(respond(new URL(url))); + } + return originalFetch(input, init); + }); +} + describe("verifyConstraintsAccess", () => { const token = "test-token"; const host = "sentry.io"; + afterEach(() => { + vi.restoreAllMocks(); + vi.useRealTimers(); + }); + it("returns ok with empty constraints when no org constraint provided", async () => { const result = await verifyConstraintsAccess( { organizationSlug: null, projectSlug: null }, @@ -106,6 +150,44 @@ describe("verifyConstraintsAccess", () => { } }); + it("collapses organization metadata while preserving project capabilities", async () => { + let projectUrl: URL | undefined; + mockProjectResponse((url) => { + projectUrl = url; + return Response.json({ + id: "123", + slug: "cloudflare-mcp", + name: "Cloudflare MCP", + organization: { id: "456", slug: "sentry-mcp-evals" }, + hasProfiles: true, + hasReplays: false, + hasLogs: true, + firstTransactionEvent: false, + }); + }); + + const result = await verifyConstraintsAccess( + { organizationSlug: "sentry-mcp-evals", projectSlug: "cloudflare-mcp" }, + { accessToken: token, sentryHost: host }, + ); + + expect(projectUrl?.searchParams.get("collapse")).toBe("organization"); + expect(result).toEqual({ + ok: true, + constraints: { + organizationSlug: "sentry-mcp-evals", + projectSlug: "cloudflare-mcp", + regionUrl: "https://us.sentry.io", + projectCapabilities: { + profiles: true, + replays: false, + logs: true, + traces: false, + }, + }, + }); + }); + it("fails when org does not exist", async () => { const result = await verifyConstraintsAccess( { organizationSlug: "nonexistent-org", projectSlug: null }, @@ -136,6 +218,169 @@ describe("verifyConstraintsAccess", () => { }); describe("caching", () => { + it("preserves verified capabilities when a concurrent request times out later", async () => { + vi.useFakeTimers(); + let notifyProjectStarted!: () => void; + const projectStarted = new Promise((resolve) => { + notifyProjectStarted = resolve; + }); + const slowProjectResponse = mockProjectResponse(() => { + notifyProjectStarted(); + return new Promise(() => {}); + }); + const kv = createMockKV(); + const cache: CacheOptions = { + kv, + userId: "concurrent-user", + waitUntil: vi.fn(), + }; + const constraints = { + organizationSlug: "sentry-mcp-evals", + projectSlug: "cloudflare-mcp", + }; + const options = { accessToken: token, sentryHost: host, cache }; + + const slowRequest = verifyConstraintsAccess(constraints, options); + await projectStarted; + slowProjectResponse.mockRestore(); + + const verified = await verifyConstraintsAccess(constraints, options); + expect(verified).toMatchObject({ + ok: true, + constraints: { + projectCapabilities: { + profiles: false, + replays: false, + logs: false, + traces: false, + }, + }, + }); + await vi.mocked(cache.waitUntil).mock.calls[0][0]; + + await vi.advanceTimersByTimeAsync(5000); + expect(await slowRequest).toMatchObject({ + ok: true, + constraints: { projectCapabilities: null }, + }); + await vi.mocked(cache.waitUntil).mock.calls[1][0]; + + const apiFetch = vi.spyOn(globalThis, "fetch"); + expect(await verifyConstraintsAccess(constraints, options)).toEqual( + verified, + ); + // The late timeout must not shorten the verified entry's lifetime either. + await vi.advanceTimersByTimeAsync(60_000); + expect(await verifyConstraintsAccess(constraints, options)).toEqual( + verified, + ); + expect(apiFetch).not.toHaveBeenCalled(); + }); + + it("reuses a project timeout for 60 seconds, then refreshes capabilities", async () => { + vi.useFakeTimers(); + let notifyProjectStarted!: () => void; + const projectStarted = new Promise((resolve) => { + notifyProjectStarted = resolve; + }); + const projectResponse = mockProjectResponse(() => { + notifyProjectStarted(); + return new Promise(() => {}); + }); + const kv = createMockKV(); + const cache: CacheOptions = { + kv, + userId: "timeout-user", + waitUntil: vi.fn(), + }; + const constraints = { + organizationSlug: "sentry-mcp-evals", + projectSlug: "cloudflare-mcp", + }; + const options = { accessToken: token, sentryHost: host, cache }; + const firstRequest = verifyConstraintsAccess(constraints, options); + await projectStarted; + await vi.advanceTimersByTimeAsync(5000); + + const timedOut = await firstRequest; + projectResponse.mockRestore(); + expect(timedOut).toEqual({ + ok: true, + constraints: { + ...constraints, + regionUrl: "https://us.sentry.io", + projectCapabilities: null, + }, + }); + expect(cache.waitUntil).toHaveBeenCalledOnce(); + await vi.mocked(cache.waitUntil).mock.calls[0][0]; + expect(kv.put).toHaveBeenCalledWith( + expect.any(String), + expect.any(String), + { expirationTtl: 60 }, + ); + + const apiFetch = vi.spyOn(globalThis, "fetch"); + await vi.advanceTimersByTimeAsync(59_999); + expect(await verifyConstraintsAccess(constraints, options)).toEqual( + timedOut, + ); + expect(apiFetch).not.toHaveBeenCalled(); + + await vi.advanceTimersByTimeAsync(1); + expect(await verifyConstraintsAccess(constraints, options)).toMatchObject( + { + ok: true, + constraints: { + projectCapabilities: { + profiles: false, + replays: false, + logs: false, + traces: false, + }, + }, + }, + ); + expect(apiFetch).toHaveBeenCalled(); + await vi.mocked(cache.waitUntil).mock.calls[1][0]; + expect(kv.put).toHaveBeenLastCalledWith( + expect.any(String), + expect.any(String), + { expirationTtl: 900 }, + ); + }); + + it.each([401, 403, 404])( + "does not cache a project verification failure with HTTP %i", + async (status) => { + mockProjectResponse( + () => + new Response(JSON.stringify({ detail: "Project access denied" }), { + status, + headers: { "Content-Type": "application/json" }, + }), + ); + const kv = createMockKV(); + const cache: CacheOptions = { + kv, + userId: "denied-user", + waitUntil: vi.fn(), + }; + + const result = await verifyConstraintsAccess( + { + organizationSlug: "sentry-mcp-evals", + projectSlug: "cloudflare-mcp", + }, + { accessToken: token, sentryHost: host, cache }, + ); + + expect(result).toMatchObject({ ok: false, status }); + expect(kv.put).not.toHaveBeenCalled(); + expect(cache.waitUntil).not.toHaveBeenCalled(); + }, + ); + const cachedData: CachedConstraints = { regionUrl: "https://us.sentry.io", projectCapabilities: { @@ -152,6 +397,7 @@ describe("verifyConstraintsAccess", () => { const cache: CacheOptions = { kv: mockKV, userId: "user-123", + waitUntil: vi.fn(), }; const result = await verifyConstraintsAccess( @@ -190,6 +436,7 @@ describe("verifyConstraintsAccess", () => { const cache: CacheOptions = { kv: mockKV, userId: "user-456", + waitUntil: vi.fn(), }; const result = await verifyConstraintsAccess( @@ -213,11 +460,7 @@ describe("verifyConstraintsAccess", () => { }); } - // Verify cache was checked - expect(mockKV.get).toHaveBeenCalledOnce(); - - // Wait for fire-and-forget cache write - await new Promise((resolve) => setTimeout(resolve, 10)); + await vi.mocked(cache.waitUntil).mock.calls[0][0]; // Verify cache was written with correct key and TTL expect(mockKV.put).toHaveBeenCalledOnce(); @@ -247,6 +490,7 @@ describe("verifyConstraintsAccess", () => { const cache: CacheOptions = { kv: mockKV, userId: "user-789", + waitUntil: vi.fn(), }; const result = await verifyConstraintsAccess( @@ -260,9 +504,6 @@ describe("verifyConstraintsAccess", () => { expect(result.constraints.organizationSlug).toBe("sentry-mcp-evals"); expect(result.constraints.projectSlug).toBe("cloudflare-mcp"); } - - // Verify cache read was attempted - expect(mockKV.get).toHaveBeenCalledOnce(); }); it("succeeds even when cache write fails", async () => { @@ -273,6 +514,7 @@ describe("verifyConstraintsAccess", () => { const cache: CacheOptions = { kv: mockKV, userId: "user-write-fail", + waitUntil: vi.fn(), }; const result = await verifyConstraintsAccess( @@ -287,8 +529,9 @@ describe("verifyConstraintsAccess", () => { expect(result.constraints.projectSlug).toBe("cloudflare-mcp"); } - // Wait for fire-and-forget cache write attempt - await new Promise((resolve) => setTimeout(resolve, 10)); + await expect( + vi.mocked(cache.waitUntil).mock.calls[0][0], + ).resolves.toBeUndefined(); // Verify cache write was attempted expect(mockKV.put).toHaveBeenCalledOnce(); @@ -304,6 +547,7 @@ describe("verifyConstraintsAccess", () => { const cache: CacheOptions = { kv: mockKV, userId: "user-org-only", + waitUntil: vi.fn(), }; const result = await verifyConstraintsAccess( @@ -334,6 +578,7 @@ describe("verifyConstraintsAccess", () => { const cache: CacheOptions = { kv: mockKV, userId: "user-sentinel", + waitUntil: vi.fn(), }; const orgOnlyResult = await verifyConstraintsAccess( @@ -364,6 +609,7 @@ describe("verifyConstraintsAccess", () => { const cache: CacheOptions = { kv: mockKV, userId: "user-org-cache-write", + waitUntil: vi.fn(), }; const result = await verifyConstraintsAccess( @@ -373,7 +619,7 @@ describe("verifyConstraintsAccess", () => { expect(result.ok).toBe(true); expect(mockKV.get).toHaveBeenCalledOnce(); - await new Promise((resolve) => setTimeout(resolve, 10)); + await vi.mocked(cache.waitUntil).mock.calls[0][0]; expect(mockKV.put).toHaveBeenCalledOnce(); expect(mockKV.put).toHaveBeenCalledWith( "caps:v1:user-org-cache-write:sentry.io:sentry-mcp-evals:org", diff --git a/packages/mcp-cloudflare/src/server/lib/constraint-utils.ts b/packages/mcp-cloudflare/src/server/lib/constraint-utils.ts index f45eedf17..c3a8c6200 100644 --- a/packages/mcp-cloudflare/src/server/lib/constraint-utils.ts +++ b/packages/mcp-cloudflare/src/server/lib/constraint-utils.ts @@ -7,6 +7,7 @@ const CAPABILITIES_TIMEOUT_MS = 5000; // Cache configuration const CACHE_TTL_SECONDS = 900; // 15 minutes +const PROJECT_TIMEOUT_CACHE_TTL_SECONDS = 60; const CACHE_KEY_VERSION = "v1"; /** @@ -14,7 +15,7 @@ const CACHE_KEY_VERSION = "v1"; */ export type CachedConstraints = { regionUrl: string | null; - /** Populated when a project constraint was verified; null for org-only cache entries. */ + /** Null for org-only entries or a project verification timeout. */ projectCapabilities: ProjectCapabilities | null; cachedAt: number; }; @@ -25,12 +26,14 @@ export type CachedConstraints = { export type CacheOptions = { kv: KVNamespace; userId: string; + waitUntil: (promise: Promise) => void; }; /** * Build a cache key for constraints verification. * Format: caps:v1:{userId}:{sentryHost}:{organizationSlug}:{scopeKey} * scopeKey is "org" for org-only entries or "project:{slug}" for project-scoped entries. + * Timeouts use a separate timeout:{cacheKey} so late writes cannot overwrite verified data. */ function buildCacheKey( userId: string, @@ -54,8 +57,7 @@ async function getCachedConstraints( key: string, ): Promise { try { - const cached = await kv.get(key, "json"); - return cached as CachedConstraints | null; + return await kv.get(key, "json"); } catch (error) { logWarn("Failed to read constraints cache", { loggerScope: ["cloudflare", "constraint-utils"], @@ -67,16 +69,17 @@ async function getCachedConstraints( /** * Store constraints in KV cache. - * Fire-and-forget - errors are logged but don't block the response. + * Registered with waitUntil by the caller; write errors do not fail requests. */ async function setCachedConstraints( kv: KVNamespace, key: string, data: CachedConstraints, + ttlSeconds: number, ): Promise { try { await kv.put(key, JSON.stringify(data), { - expirationTtl: CACHE_TTL_SECONDS, + expirationTtl: ttlSeconds, }); } catch (error) { logWarn("Failed to write constraints cache", { @@ -114,8 +117,8 @@ async function withTimeout( * Verify that provided org/project constraints exist and the user has access * by querying Sentry's API using the provided OAuth access token. * - * If cache options are provided with a projectSlug, results will be cached - * in KV to avoid repeated API calls during MCP sessions. + * Cache verified constraints for 15 minutes and project timeouts for 60 seconds + * to reduce repeated request delays while periodically retrying verification. */ export async function verifyConstraintsAccess( { organizationSlug, projectSlug }: Constraints, @@ -165,7 +168,11 @@ export async function verifyConstraintsAccess( organizationSlug, projectSlug, ); - const cached = await getCachedConstraints(cache.kv, cacheKey); + const cached = + (await getCachedConstraints(cache.kv, cacheKey)) ?? + (projectSlug + ? await getCachedConstraints(cache.kv, `timeout:${cacheKey}`) + : null); if (cached) { return { ok: true, @@ -214,6 +221,7 @@ export async function verifyConstraintsAccess( { organizationSlug, projectSlugOrId: projectSlug, + collapse: "organization", }, regionUrl ? { host: new URL(regionUrl).host } : undefined, ), @@ -259,14 +267,23 @@ export async function verifyConstraintsAccess( } } - // Cache: org-only after org fetch; org+project only when project verification - // succeeded (skip caching on project timeout so the next request can retry). - if (cacheKey && (!projectSlug || projectCapabilities !== null)) { - void setCachedConstraints(cache!.kv, cacheKey, { - regionUrl: regionUrl || null, - projectCapabilities: projectSlug ? projectCapabilities : null, - cachedAt: Date.now(), - }); + if (cache && cacheKey) { + const isProjectTimeout = + Boolean(projectSlug) && projectCapabilities === null; + cache.waitUntil( + setCachedConstraints( + cache.kv, + isProjectTimeout ? `timeout:${cacheKey}` : cacheKey, + { + regionUrl: regionUrl || null, + projectCapabilities, + cachedAt: Date.now(), + }, + isProjectTimeout + ? PROJECT_TIMEOUT_CACHE_TTL_SECONDS + : CACHE_TTL_SECONDS, + ), + ); } return { diff --git a/packages/mcp-cloudflare/src/server/lib/mcp-handler.test.ts b/packages/mcp-cloudflare/src/server/lib/mcp-handler.test.ts index f0535a2d5..237e12bd4 100644 --- a/packages/mcp-cloudflare/src/server/lib/mcp-handler.test.ts +++ b/packages/mcp-cloudflare/src/server/lib/mcp-handler.test.ts @@ -1,4 +1,8 @@ import type { ExecutionContext, RateLimit } from "@cloudflare/workers-types"; +import { + createExecutionContext, + waitOnExecutionContext, +} from "cloudflare:test"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { DIRECT_AUTH_ASSERTION_TOKEN } from "../../test-utils/fetch-mock-setup"; import type { Env } from "../types"; @@ -8,7 +12,8 @@ const { sentryMetricsCount, sentrySetUser } = vi.hoisted(() => ({ sentrySetUser: vi.fn(), })); -vi.mock("@sentry/cloudflare", () => ({ +vi.mock("@sentry/cloudflare", async (importOriginal) => ({ + ...(await importOriginal()), getActiveSpan: vi.fn(() => undefined), metrics: { count: sentryMetricsCount, @@ -560,6 +565,141 @@ describe("MCP Handler", () => { }); describe("URL constraints", () => { + it("rejects upstream permission failures with cached unknown capabilities", async () => { + const env = createTestEnv(); + const timeoutKey = + "timeout:caps:v1:test-user-123:sentry.io:test-org:project:test-project"; + env.MCP_CACHE = { + get: vi.fn().mockImplementation(async (key: string) => + key === timeoutKey + ? { + regionUrl: "https://us.sentry.io", + projectCapabilities: null, + cachedAt: Date.now(), + } + : null, + ), + put: vi.fn(), + } as unknown as KVNamespace; + const ctx = createMcpContext({ + accessToken: "test-current-user-token", + grantedSkills: ["inspect"], + constraintOrganizationSlug: "test-org", + constraintProjectSlug: "test-project", + }); + const profileId = "00000000000040008000000000000001"; + const profileUrl = `https://us.sentry.io/api/0/projects/test-org/test-project/profiling/profiles/${profileId}/`; + const request = createMcpRequest( + "tools/call", + { + name: "execute_sentry_tool", + arguments: { + name: "get_profile_details", + arguments: { profileId }, + }, + }, + { path: "/mcp/test-org/test-project" }, + ); + const originalFetch = globalThis.fetch; + const apiFetch = vi + .spyOn(globalThis, "fetch") + .mockImplementation((input, init) => { + const url = input instanceof Request ? input.url : String(input); + if (url === profileUrl) { + return Promise.resolve( + Response.json( + { + detail: "You do not have permission to perform this action.", + }, + { status: 403 }, + ), + ); + } + return originalFetch(input, init); + }); + + try { + const response = await mcpHandler.fetch!(request, env, ctx); + + expect(response.status).toBe(200); + const body = await parseSSEResponse<{ + result: { + isError?: boolean; + content: Array<{ type: string; text: string }>; + }; + }>(response); + expect(body.result.isError).toBe(true); + expect(body.result.content).toEqual([ + { type: "text", text: expect.stringContaining("HTTP 403") }, + ]); + expect(env.MCP_CACHE.put).not.toHaveBeenCalled(); + expect(apiFetch).toHaveBeenCalledOnce(); + expect(apiFetch).toHaveBeenCalledWith( + profileUrl, + expect.objectContaining({ + headers: expect.objectContaining({ + Authorization: `Bearer ${ctx.props.accessToken}`, + }), + }), + ); + } finally { + apiFetch.mockRestore(); + } + }); + + it("keeps the constraint cache write alive after returning the response", async () => { + let finishWrite!: () => void; + const writeReady = new Promise((resolve) => { + finishWrite = resolve; + }); + let stored: unknown = null; + const env = createTestEnv(); + env.MCP_CACHE = { + get: vi.fn().mockResolvedValue(null), + put: vi.fn().mockImplementation(async (_key: string, value: string) => { + await writeReady; + stored = JSON.parse(value); + }), + } as unknown as KVNamespace; + const ctx = Object.assign(createExecutionContext(), { + props: { ...DEFAULT_OAUTH_PROPS }, + }); + const request = createMcpRequest( + "initialize", + { + protocolVersion: "2024-11-05", + capabilities: {}, + clientInfo: { name: "test-client", version: "1.0.0" }, + }, + { path: "/mcp/sentry-mcp-evals/cloudflare-mcp" }, + ); + + try { + const response = await mcpHandler.fetch!(request, env, ctx); + expect(response.status).toBe(200); + const body = await parseSSEResponse<{ + result: { protocolVersion: string }; + }>(response); + expect(body.result.protocolVersion).toBeDefined(); + expect(stored).toBeNull(); + + let backgroundCompleted = false; + const background = waitOnExecutionContext(ctx).then(() => { + backgroundCompleted = true; + }); + // Let completed tasks drain; only the blocked KV write should remain. + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(backgroundCompleted).toBe(false); + + finishWrite(); + await background; + expect(stored).not.toBeNull(); + } finally { + finishWrite(); + await waitOnExecutionContext(ctx); + } + }); + it("should handle /mcp without constraints", async () => { const request = createMcpRequest( "initialize", diff --git a/packages/mcp-cloudflare/src/server/lib/mcp-handler.ts b/packages/mcp-cloudflare/src/server/lib/mcp-handler.ts index c884567f0..9959652e4 100644 --- a/packages/mcp-cloudflare/src/server/lib/mcp-handler.ts +++ b/packages/mcp-cloudflare/src/server/lib/mcp-handler.ts @@ -412,6 +412,7 @@ async function handleAuthenticatedMcpRequest( cache: { kv: env.MCP_CACHE, userId: auth.userId, + waitUntil: (promise) => ctx.waitUntil(promise), }, }, ); diff --git a/packages/mcp-cloudflare/src/test-utils/fetch-mock-setup.ts b/packages/mcp-cloudflare/src/test-utils/fetch-mock-setup.ts index 7b1d5c7dc..7081f0835 100644 --- a/packages/mcp-cloudflare/src/test-utils/fetch-mock-setup.ts +++ b/packages/mcp-cloudflare/src/test-utils/fetch-mock-setup.ts @@ -239,6 +239,24 @@ export function registerFetchMockInterceptors(fetchMock: FetchMockLike) { .reply(200, projectFixture, { headers: JSON_HEADERS }) .persist(); + pool + .intercept({ + path: "/api/0/projects/sentry-mcp-evals/cloudflare-mcp/?collapse=organization", + method: "GET", + }) + .reply( + 200, + { + ...projectFixture, + organization: { + id: organizationFixture.id, + slug: organizationFixture.slug, + }, + }, + { headers: JSON_HEADERS }, + ) + .persist(); + pool .intercept({ path: "/api/0/projects/sentry-mcp-evals/cloudflare-mcp/", @@ -258,6 +276,18 @@ export function registerFetchMockInterceptors(fetchMock: FetchMockLike) { ) .persist(); + pool + .intercept({ + path: "/api/0/projects/sentry-mcp-evals/nonexistent-project/?collapse=organization", + method: "GET", + }) + .reply( + 404, + { detail: "The requested resource does not exist" }, + { headers: JSON_HEADERS }, + ) + .persist(); + // ===== Client Keys ===== pool .intercept({ diff --git a/packages/mcp-core/src/api-client/client.test.ts b/packages/mcp-core/src/api-client/client.test.ts index bbc599a0b..981acad19 100644 --- a/packages/mcp-core/src/api-client/client.test.ts +++ b/packages/mcp-core/src/api-client/client.test.ts @@ -997,6 +997,85 @@ describe("monitor time parameters", () => { }); }); +describe("getProject", () => { + afterEach(() => mswServer.resetHandlers()); + + const project = { + ...projectFixture, + id: "123", + slug: "backend", + name: "Backend", + organization: { + id: "456", + slug: "test-org", + name: "Test Organization", + features: ["organizations:issue-basic"], + }, + hasProfiles: true, + hasReplays: false, + hasLogs: true, + firstTransactionEvent: false, + }; + const api = new SentryApiService({ + host: "sentry.io", + accessToken: "test-token", + }); + + it("keeps the default request and full organization response", async () => { + const requests: string[] = []; + mswServer.use( + http.get( + "https://sentry.io/api/0/projects/test-org/backend/", + ({ request }) => { + requests.push(request.url); + return HttpResponse.json(project); + }, + ), + ); + + const result = await api.getProject({ + organizationSlug: "test-org", + projectSlugOrId: "backend", + }); + + expect(requests).toEqual([ + "https://sentry.io/api/0/projects/test-org/backend/", + ]); + expect(result).toEqual(project); + }); + + it("collapses organization on the regional host and preserves project capabilities", async () => { + const collapsedProject = { + ...project, + organization: { id: "456", slug: "test-org" }, + }; + const requests: string[] = []; + mswServer.use( + http.get( + "https://eu.sentry.io/api/0/projects/test-org/backend/", + ({ request }) => { + requests.push(request.url); + return HttpResponse.json(collapsedProject); + }, + ), + ); + + const result = await api.getProject( + { + organizationSlug: "test-org", + projectSlugOrId: "backend", + collapse: "organization", + }, + { host: "eu.sentry.io" }, + ); + + expect(requests).toEqual([ + "https://eu.sentry.io/api/0/projects/test-org/backend/?collapse=organization", + ]); + expect(result).toEqual(collapsedProject); + }); +}); + describe("getIssue", () => { it("uses the numeric issue ID when Sentry returns a null shortId", async () => { mswServer.use( diff --git a/packages/mcp-core/src/api-client/client.ts b/packages/mcp-core/src/api-client/client.ts index 24352c1da..6ef0a6955 100644 --- a/packages/mcp-core/src/api-client/client.ts +++ b/packages/mcp-core/src/api-client/client.ts @@ -1981,6 +1981,7 @@ export class SentryApiService { * @param params Project fetch parameters * @param params.organizationSlug Organization identifier * @param params.projectSlugOrId Project slug or numeric ID + * @param params.collapse Return only organization ID and slug instead of full organization data * @param opts Request options * @returns Project data */ @@ -1988,14 +1989,22 @@ export class SentryApiService { { organizationSlug, projectSlugOrId, + collapse, }: { organizationSlug: string; projectSlugOrId: string; + collapse?: "organization"; }, opts?: RequestOptions, ): Promise { + const queryParams = new URLSearchParams(); + if (collapse) { + queryParams.set("collapse", collapse); + } + const queryString = queryParams.toString(); + const projectPath = apiPath`/projects/${organizationSlug}/${projectSlugOrId}/`; const body = await this.requestJSON( - apiPath`/projects/${organizationSlug}/${projectSlugOrId}/`, + `${projectPath}${queryString ? `?${queryString}` : ""}`, undefined, opts, ); diff --git a/packages/mcp-core/src/server.test.ts b/packages/mcp-core/src/server.test.ts index 061ac44b9..c37f79f9a 100644 --- a/packages/mcp-core/src/server.test.ts +++ b/packages/mcp-core/src/server.test.ts @@ -94,7 +94,7 @@ async function listRegisteredTools(server: ReturnType) { } async function callRegisteredTool( - server: ReturnType, + server: ReturnType | ModernMcpServer, name: string, args: Record, ) { @@ -1358,6 +1358,49 @@ describe("buildServer", () => { ); }); + it.each(["v1", "v2"] as const)( + "execute_sentry_tool rejects missing skills with unknown capabilities on SDK %s", + async (sdkVersion) => { + const upstreamRequest = vi.fn(() => + HttpResponse.json( + { detail: "You do not have permission to perform this action." }, + { status: 403 }, + ), + ); + mswServer.use( + http.get( + "https://us.sentry.io/api/0/projects/test-org/test-project/profiling/profiles/:profileId/", + upstreamRequest, + ), + ); + const context: ServerContext = { + ...baseContext, + grantedSkills: new Set(["docs"]), + constraints: { + organizationSlug: "test-org", + projectSlug: "test-project", + regionUrl: "https://us.sentry.io", + projectCapabilities: null, + }, + }; + const server = + sdkVersion === "v2" + ? buildServer({ context, sdkVersion }) + : buildServer({ context }); + + const result = await callRegisteredTool(server, "execute_sentry_tool", { + name: "get_profile_details", + arguments: { profileId: "00000000000040008000000000000001" }, + }); + + expect(result).toMatchObject({ isError: true }); + expect(getTextContent(result)).toContain( + 'Tool "get_profile_details" is not available in this session', + ); + expect(upstreamRequest).not.toHaveBeenCalled(); + }, + ); + it("search_sentry_tools and execute_sentry_tool enforce project capabilities by default", async () => { const server = buildServer({ context: {