From 2d512db031638c211edee45acac79f64e608bb46 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Tue, 3 Nov 2026 00:22:19 +0000 Subject: [PATCH 1/2] feat(mcp-cloudflare): Cache Sentry API responses in memory behind MCP_RESPONSE_CACHE Wire the shared byte-budgeted memory store into the hosted MCP. The cache is off unless MCP_RESPONSE_CACHE is `memory` or `tiered` (memory only until the R2 tier lands). Entries are keyed by a SHA-256 of the MCP grant's user and grant IDs from the wrapper token, so one grant never reads another's responses; requests without a grant, such as Sentry-Bearer direct tokens, are not cached. SentryApiService takes an optional responseCache and sends through a caching transport for its token. apiServiceFromContext drops it when a credential router can swap tokens. Writes run in waitUntil. Adds hit/miss/stale lookup, write (with encoding), and eviction metrics, memory gauges, and a one-time log when an isolate reaches its budget. Co-Authored-By: BYK <126780+BYK@users.noreply.github.com> --- docs/cloudflare/architecture.md | 38 +++ docs/operations/monitoring.md | 30 ++ .../src/server/lib/mcp-handler.test.ts | 126 ++++++++ .../src/server/lib/mcp-handler.ts | 17 ++ .../src/server/lib/response-cache.test.ts | 274 ++++++++++++++++++ .../src/server/lib/response-cache.ts | 238 +++++++++++++++ packages/mcp-cloudflare/src/server/types.ts | 5 + .../api-client/client-response-cache.test.ts | 90 ++++++ packages/mcp-core/src/api-client/client.ts | 52 +++- .../src/internal/tool-helpers/api.test.ts | 62 +++- .../mcp-core/src/internal/tool-helpers/api.ts | 2 + packages/mcp-core/src/types.ts | 19 ++ .../response-cache/memory-store.test.ts | 17 ++ .../api-client/response-cache/memory-store.ts | 12 +- 14 files changed, 966 insertions(+), 16 deletions(-) create mode 100644 packages/mcp-cloudflare/src/server/lib/response-cache.test.ts create mode 100644 packages/mcp-cloudflare/src/server/lib/response-cache.ts create mode 100644 packages/mcp-core/src/api-client/client-response-cache.test.ts diff --git a/docs/cloudflare/architecture.md b/docs/cloudflare/architecture.md index b5efaa81f..158ff7792 100644 --- a/docs/cloudflare/architecture.md +++ b/docs/cloudflare/architecture.md @@ -166,6 +166,44 @@ SENTRY_CLIENT_SECRET = "..." # OAuth app secret - `/api/metadata` - MCP metadata endpoint - `/sse` - Server-sent events for MCP +### Sentry API Response Cache + +The MCP handler can cache Sentry API GET responses through the shared +response cache in `@sentry/toolkit-core/api-client/response-cache`. It is +behind `MCP_RESPONSE_CACHE`: + +| Value | Behavior | +| ------------------- | -------------------------------------------------------------------------- | +| unset, `off`, other | No caching. The default in every deployed environment. | +| `memory` | Caches every tier in the isolate's memory. | +| `tiered` | Memory only for now; the R2 tier for allowlisted detail endpoints follows. | + +How it works (`src/server/lib/response-cache.ts`): + +- **Identity.** Each entry is keyed by the SHA-256 of the MCP grant's user ID + and grant ID, both from the wrapper bearer token (`userId:grantId:secret`). + A grant never reads another grant's entries, even for the same user and + URL, and the key stays the same when the wrapper token refreshes. Requests + without a grant ID, including Sentry-Bearer direct tokens, are not cached. +- **Storage.** One memory store per isolate, shared by every request it + serves: a 24 MiB byte budget counted over compressed bodies, a 1 MiB + per-entry cap, and least recently used eviction. +- **Freshness.** Server cache headers win; otherwise the URL's tier applies + (60 s volatile, 5 minutes stable, never for autofix polling). A successful + write invalidates the same grant's entries under the written resource. +- **Writes** run in `ctx.waitUntil()`, so they never delay a response. +- **Bodies** are compressed with zstd level 3 when `node:zlib` has it, and + brotli quality 4 otherwise. Local workerd (miniflare `5.20261019.1-beta`) + has zstd; a deployed Worker is not confirmed yet. The + `app.response_cache.write` metric records the encoding each write used. + +Turning it on in production is a separate decision: revoked access or deleted +data can still be served from the cache until the entry expires (up to 5 +minutes for stable entries). Leave `MCP_RESPONSE_CACHE` unset in production +until that window has privacy and security sign-off. Setting it back to `off` +restores uncached behavior. Metrics are listed in +[Monitoring](../operations/monitoring.md#response-cache-metrics). + ## Security Considerations 1. **Authentication**: OAuth tokens stored encrypted in KV diff --git a/docs/operations/monitoring.md b/docs/operations/monitoring.md index 547e16633..c8aa899c3 100644 --- a/docs/operations/monitoring.md +++ b/docs/operations/monitoring.md @@ -336,6 +336,36 @@ Interpretation: - Upstream/provider 429s increment `app.server.response` with status `429`, but do not include `app.response.reason` +### Response Cache Metrics + +The hosted MCP's Sentry API response cache (`MCP_RESPONSE_CACHE`, see +[Cloudflare Architecture](../cloudflare/architecture.md#sentry-api-response-cache)) +sends these through the same metrics API. Nothing is sent while the cache is +off. + +- `app.response_cache.lookup` - Count of cache lookups, with + `app.response_cache.result`: `hit` (served from cache), `miss`, or `stale` + (an expired entry with an `ETag`, revalidated with `If-None-Match`) +- `app.response_cache.write` - Count of responses written to the memory + store, with `app.response_cache.encoding`: `zstd`, `br`, or `identity` + (bodies under 256 bytes). `br` on every write means the Worker has no zstd. +- `app.response_cache.eviction` - Count of entries evicted to stay within the + memory budget +- `app.response_cache.memory.bytes` - Gauge of bytes the isolate's store + charges against its budget, after each write +- `app.response_cache.memory.entries` - Gauge of entries in the isolate's + store, after each write + +The first eviction in an isolate also logs `Response cache reached its memory +budget` at info level, with the budget, bytes, and entry count. + +Interpretation: + +- Hit rate: `sum(app.response_cache.lookup)` filtered to `result=hit`, over + the unfiltered sum +- A steady eviction rate with `memory.bytes` at the budget means the budget is + too small for the traffic an isolate serves + ### Traces Configuration ```typescript 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 809449976..3404253ed 100644 --- a/packages/mcp-cloudflare/src/server/lib/mcp-handler.test.ts +++ b/packages/mcp-cloudflare/src/server/lib/mcp-handler.test.ts @@ -17,6 +17,7 @@ vi.mock("@sentry/cloudflare", async (importOriginal) => ({ getActiveSpan: vi.fn(() => undefined), metrics: { count: sentryMetricsCount, + gauge: vi.fn(), }, setUser: sentrySetUser, })); @@ -925,6 +926,131 @@ describe("MCP Handler", () => { }); }); + describe("response cache", () => { + const whoamiCall = { + name: "execute_sentry_tool", + arguments: { name: "whoami", arguments: {} }, + }; + let grantSequence = 0; + + /** A wrapper token for a grant no other test has used. */ + function newGrantToken(userId = DEFAULT_OAUTH_PROPS.id): string { + grantSequence += 1; + return `${userId}:cache-grant-${grantSequence}:secret`; + } + + function spyOnAuthRequests() { + const spy = vi.spyOn(globalThis, "fetch"); + return { + count: () => + spy.mock.calls.filter(([input]) => + String(input instanceof Request ? input.url : input).endsWith( + "/api/0/auth/", + ), + ).length, + restore: () => spy.mockRestore(), + }; + } + + async function callWhoami( + env: Env, + bearerToken: string, + props: Partial = {}, + ): Promise { + const ctx = Object.assign(createExecutionContext(), { + props: { ...DEFAULT_OAUTH_PROPS, ...props }, + }); + const response = await mcpHandler.fetch!( + createMcpRequest("tools/call", whoamiCall, { bearerToken }), + env, + ctx, + ); + expect(response.status).toBe(200); + const body = await parseSSEResponse<{ result: { isError?: boolean } }>( + response, + ); + expect(body.result.isError).toBeFalsy(); + await waitOnExecutionContext(ctx); + } + + it("is off by default", async () => { + const auth = spyOnAuthRequests(); + try { + const token = newGrantToken(); + await callWhoami(createTestEnv(), token); + await callWhoami(createTestEnv(), token); + expect(auth.count()).toBe(2); + } finally { + auth.restore(); + } + }); + + it("serves a grant's repeated reads from memory, but never another grant's", async () => { + const env = { ...createTestEnv(), MCP_RESPONSE_CACHE: "memory" }; + const auth = spyOnAuthRequests(); + try { + const grantA = newGrantToken(); + await callWhoami(env, grantA); + await callWhoami(env, grantA); + expect(auth.count()).toBe(1); + + // Same user and URL, another grant: a separate identity. + const grantB = newGrantToken(); + await callWhoami(env, grantB); + expect(auth.count()).toBe(2); + + // Another user on the same isolate. + await callWhoami(env, newGrantToken("other-user"), { + id: "other-user", + accessToken: "other-access-token", + }); + expect(auth.count()).toBe(3); + } finally { + auth.restore(); + } + }); + + it("does not cache requests without a wrapper grant", async () => { + const env = { ...createTestEnv(), MCP_RESPONSE_CACHE: "memory" }; + const auth = spyOnAuthRequests(); + try { + await callWhoami(env, "not-a-wrapper-token"); + await callWhoami(env, "not-a-wrapper-token"); + expect(auth.count()).toBe(2); + } finally { + auth.restore(); + } + }); + + it("does not cache Sentry-Bearer direct tokens", async () => { + const env = { + ...createTestEnv(), + MCP_RESPONSE_CACHE: "memory", + SENTRY_HOST: "direct-token.test", + }; + const auth = spyOnAuthRequests(); + try { + for (let i = 0; i < 2; i++) { + const ctx = createExecutionContext(); + const response = await handleSentryBearerMcpRequest( + createMcpRequest("tools/call", whoamiCall, { + headers: { "X-Sentry-Utm-Source": "plugin" }, + }), + env, + ctx, + DIRECT_AUTH_ASSERTION_TOKEN, + ); + expect(response.status).toBe(200); + await response.text(); + await waitOnExecutionContext(ctx); + } + expect(auth.count()).toBe(2); + } finally { + auth.restore(); + } + }); + }); + describe("request limits", () => { function createDirectContext(): ExecutionContext { return { diff --git a/packages/mcp-cloudflare/src/server/lib/mcp-handler.ts b/packages/mcp-cloudflare/src/server/lib/mcp-handler.ts index 906861fbf..e6580f88a 100644 --- a/packages/mcp-cloudflare/src/server/lib/mcp-handler.ts +++ b/packages/mcp-cloudflare/src/server/lib/mcp-handler.ts @@ -47,6 +47,7 @@ import { } from "./attribution"; import { resolveClientFamily } from "./client-family"; import { verifyConstraintsAccess } from "./constraint-utils"; +import { getResponseCache, resolveResponseCacheMode } from "./response-cache"; /** * ExecutionContext with OAuth props injected by the OAuth provider. @@ -60,6 +61,8 @@ type OAuthMcpContext = { accessToken: string; grantedSkills: Set; userId: string; + /** From the wrapper token; scopes the response cache. */ + grantId: string | null; clientId: string; clientName?: string; tokenConstraintOrganizationSlug?: string | null; @@ -443,6 +446,18 @@ async function handleAuthenticatedMcpRequest( return constraints; } + // Only an MCP grant gives a cache identity; direct tokens are not cached. + const responseCache = + auth.kind === "oauth" && auth.grantId + ? await getResponseCache({ + mode: resolveResponseCacheMode(env.MCP_RESPONSE_CACHE), + userId: auth.userId, + grantId: auth.grantId, + sentryHost, + waitUntil: (promise) => ctx.waitUntil(promise), + }) + : undefined; + const serverContext: ServerContext = { userId: auth.kind === "oauth" ? auth.userId : undefined, userIpAddress, @@ -459,6 +474,7 @@ async function handleAuthenticatedMcpRequest( transport: "http", onUpstreamUnauthorized: auth.kind === "oauth" ? auth.onUpstreamUnauthorized : undefined, + responseCache, }; // `createMcpHandler` from agents only forwards `maxRequestBodySize` to the @@ -610,6 +626,7 @@ const mcpHandler: ExportedHandler = { return await handleAuthenticatedMcpRequest(request, env, ctx, { kind: "oauth", userId, + grantId: requestGrantId, clientId, clientName, accessToken, diff --git a/packages/mcp-cloudflare/src/server/lib/response-cache.test.ts b/packages/mcp-cloudflare/src/server/lib/response-cache.test.ts new file mode 100644 index 000000000..0e53e68e4 --- /dev/null +++ b/packages/mcp-cloudflare/src/server/lib/response-cache.test.ts @@ -0,0 +1,274 @@ +import { + type CacheEntry, + createMemoryStore, +} from "@sentry/toolkit-core/api-client/response-cache"; +import { ENTRY_OVERHEAD_BYTES } from "@sentry/toolkit-core/api-client/response-cache/memory-store"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +const { logInfo, sentryMetricsCount, sentryMetricsGauge } = vi.hoisted(() => ({ + logInfo: vi.fn(), + sentryMetricsCount: vi.fn(), + sentryMetricsGauge: vi.fn(), +})); + +vi.mock("@sentry/mcp-core/telem/logging", async (importOriginal) => ({ + ...(await importOriginal()), + logInfo, +})); + +vi.mock("@sentry/cloudflare", async (importOriginal) => ({ + ...(await importOriginal()), + metrics: { count: sentryMetricsCount, gauge: sentryMetricsGauge }, +})); + +import { SentryApiService } from "@sentry/mcp-core/api-client"; +import { issueFixture } from "@sentry/mcp-server-mocks/payloads"; +import { + createMeteredMemoryStore, + createResponseCache, + getGrantCacheIdentity, + getResponseCache, + resolveResponseCacheMode, +} from "./response-cache"; + +const USERS: Record = { + "Bearer token-a": { id: "1", name: "User A", email: "a@example.com" }, + "Bearer token-b": { id: "2", name: "User B", email: "b@example.com" }, +}; + +/** + * Answers `/api/0/auth/` with the user the Authorization header belongs to, + * and issue reads and writes with an issue fixture. + */ +function mockSentryApi() { + return vi + .spyOn(globalThis, "fetch") + .mockImplementation(async (input, init) => { + const request = new Request(input, init); + const user = USERS[request.headers.get("Authorization") ?? ""]; + if (!user) { + return new Response("{}", { status: 401 }); + } + if (new URL(request.url).pathname.includes("/issues/")) { + return Response.json(issueFixture); + } + return Response.json(user); + }); +} + +function lookups(result: string): number { + return sentryMetricsCount.mock.calls.filter( + ([name, , options]) => + name === "app.response_cache.lookup" && + options?.attributes?.["app.response_cache.result"] === result, + ).length; +} + +describe("resolveResponseCacheMode", () => { + it.each([ + [undefined, "off"], + ["", "off"], + ["off", "off"], + ["on", "off"], + ["true", "off"], + ["memory", "memory"], + [" Memory ", "memory"], + ["tiered", "tiered"], + ])("maps %j to %s", (value, mode) => { + expect(resolveResponseCacheMode(value)).toBe(mode); + }); +}); + +describe("getGrantCacheIdentity", () => { + it("is stable for a grant and differs across users and grants", async () => { + const identity = await getGrantCacheIdentity("user-1", "grant-1"); + expect(identity).toMatch(/^[0-9a-f]{64}$/); + expect(await getGrantCacheIdentity("user-1", "grant-1")).toBe(identity); + expect(await getGrantCacheIdentity("user-1", "grant-2")).not.toBe(identity); + expect(await getGrantCacheIdentity("user-2", "grant-1")).not.toBe(identity); + // A separator in one ID cannot make two pairs collide. + expect(await getGrantCacheIdentity("a:b", "c")).not.toBe( + await getGrantCacheIdentity("a", "b:c"), + ); + }); +}); + +describe("createResponseCache", () => { + let fetchSpy: ReturnType; + const pending: Promise[] = []; + const waitUntil = (promise: Promise) => { + pending.push(promise); + }; + + beforeEach(() => { + sentryMetricsCount.mockReset(); + sentryMetricsGauge.mockReset(); + fetchSpy = mockSentryApi(); + }); + + afterEach(() => { + fetchSpy.mockRestore(); + pending.length = 0; + }); + + async function apiFor( + token: string, + identity: string, + store: ReturnType, + ) { + return new SentryApiService({ + accessToken: token, + responseCache: createResponseCache({ + store, + identity, + apiBaseUrl: "https://sentry.io", + waitUntil, + }), + }); + } + + async function whoami(api: SentryApiService) { + const user = await api.getAuthenticatedUser(); + // Writes finish in waitUntil, as on a Worker. + await Promise.all(pending); + return user; + } + + it("never serves one identity's entry to another for the same URL", async () => { + const store = createMemoryStore(); + const identityA = await getGrantCacheIdentity("user-1", "grant-a"); + const identityB = await getGrantCacheIdentity("user-1", "grant-b"); + const apiA = await apiFor("token-a", identityA, store); + const apiB = await apiFor("token-b", identityB, store); + + expect(await whoami(apiA)).toMatchObject({ email: "a@example.com" }); + expect(fetchSpy).toHaveBeenCalledTimes(1); + expect(await whoami(apiA)).toMatchObject({ email: "a@example.com" }); + expect(fetchSpy).toHaveBeenCalledTimes(1); + + // Same URL, same isolate store, other identity: a miss, not A's body. + expect(await whoami(apiB)).toMatchObject({ email: "b@example.com" }); + expect(fetchSpy).toHaveBeenCalledTimes(2); + expect(await whoami(apiB)).toMatchObject({ email: "b@example.com" }); + expect(await whoami(apiA)).toMatchObject({ email: "a@example.com" }); + expect(fetchSpy).toHaveBeenCalledTimes(2); + + expect(store.stats().entries).toBe(2); + expect(lookups("miss")).toBe(2); + expect(lookups("hit")).toBe(3); + }); + + it("invalidates only the writing identity's entries", async () => { + const store = createMemoryStore(); + const apiA = await apiFor("token-a", "identity-a", store); + const apiB = await apiFor("token-b", "identity-b", store); + const issue = { organizationSlug: "org", issueId: "CLOUDFLARE-MCP-41" }; + await apiA.getIssue(issue); + await apiB.getIssue(issue); + await Promise.all(pending); + expect(fetchSpy).toHaveBeenCalledTimes(2); + + await apiA.updateIssue({ ...issue, status: "resolved" }); + fetchSpy.mockClear(); + + await apiB.getIssue(issue); + expect(fetchSpy).not.toHaveBeenCalled(); + await apiA.getIssue(issue); + expect(fetchSpy).toHaveBeenCalledTimes(1); + }); + + it("does not read or keep a non-JSON body", async () => { + const store = createMemoryStore(); + const { cache } = createResponseCache({ + store, + identity: "identity-a", + apiBaseUrl: "https://sentry.io", + waitUntil, + }); + await cache.store( + { url: "https://sentry.io/api/0/x/", requestHeaders: {}, identity: "" }, + new Response("plain text", { headers: { "Content-Type": "text/plain" } }), + ); + expect(store.stats().entries).toBe(0); + }); +}); + +describe("createMeteredMemoryStore", () => { + beforeEach(() => { + sentryMetricsCount.mockReset(); + sentryMetricsGauge.mockReset(); + logInfo.mockReset(); + }); + + function entry(size: number): CacheEntry { + return { + url: "https://sentry.io/api/0/x/", + identity: "identity-a", + tier: "stable", + status: 200, + headers: {}, + body: { + bytes: new Uint8Array(size), + encoding: "identity", + rawSize: size, + }, + policy: {}, + createdAt: Date.now(), + }; + } + + it("reports each write, its usage, and every eviction", async () => { + const entrySize = 100 + ENTRY_OVERHEAD_BYTES; + const store = createMeteredMemoryStore(entrySize * 2); + for (const key of ["a", "b", "c", "d"]) { + await store.set(key, entry(100), 60_000); + } + + expect(sentryMetricsGauge).toHaveBeenLastCalledWith( + "app.response_cache.memory.entries", + 2, + ); + expect(sentryMetricsGauge).toHaveBeenCalledWith( + "app.response_cache.memory.bytes", + entrySize * 2, + ); + expect( + sentryMetricsCount.mock.calls.filter( + ([name]) => name === "app.response_cache.eviction", + ), + ).toHaveLength(2); + expect(sentryMetricsCount).toHaveBeenCalledWith( + "app.response_cache.write", + 1, + { attributes: { "app.response_cache.encoding": "identity" } }, + ); + // The budget is logged once per store, not on every eviction. + expect(logInfo).toHaveBeenCalledTimes(1); + expect(logInfo).toHaveBeenCalledWith( + "Response cache reached its memory budget", + expect.objectContaining({ + extra: expect.objectContaining({ budgetBytes: entrySize * 2 }), + }), + ); + }); +}); + +describe("getResponseCache", () => { + const options = { + userId: "user-1", + grantId: "grant-1", + sentryHost: "sentry.io", + waitUntil: () => {}, + }; + + it("is off unless a mode enables it", async () => { + expect(await getResponseCache({ ...options, mode: "off" })).toBeUndefined(); + }); + + it("scopes the cache to the grant", async () => { + const cache = await getResponseCache({ ...options, mode: "memory" }); + expect(cache?.identity).toBe( + await getGrantCacheIdentity("user-1", "grant-1"), + ); + }); +}); diff --git a/packages/mcp-cloudflare/src/server/lib/response-cache.ts b/packages/mcp-cloudflare/src/server/lib/response-cache.ts new file mode 100644 index 000000000..678bfc508 --- /dev/null +++ b/packages/mcp-cloudflare/src/server/lib/response-cache.ts @@ -0,0 +1,238 @@ +/** + * The hosted MCP's Sentry API response cache, behind `MCP_RESPONSE_CACHE`: + * `off` (the default, and any unknown value), `memory`, or `tiered`. + * + * Entries live in one byte-budgeted memory store per isolate, shared by every + * request it serves. Each entry is keyed by an identity hashed from the MCP + * grant (the wrapper token's user and grant IDs), so one grant never reads + * another's responses. Requests without a grant, such as Sentry-Bearer + * direct tokens, are not cached. + */ + +import * as Sentry from "@sentry/cloudflare"; +import { logInfo, logWarn } from "@sentry/mcp-core/telem/logging"; +import type { ResponseCache } from "@sentry/mcp-core/types"; +import { + buildCacheKey, + type CacheStore, + classifyUrl, + computeInvalidationPrefixes, + createMemoryStore, + lookupResponse, + type MemoryStore, + storeResponse, +} from "@sentry/toolkit-core/api-client/response-cache"; +import { createBodyCodec } from "@sentry/toolkit-core/response-cache/codec"; + +export type ResponseCacheMode = "off" | "memory" | "tiered"; + +/** Per-isolate budget over all entries; a Worker isolate has 128 MB. */ +const MEMORY_BUDGET_BYTES = 24 * 1024 * 1024; + +const LOGGER_SCOPE = ["cloudflare", "response-cache"]; +const LOOKUP_METRIC = "app.response_cache.lookup"; +const WRITE_METRIC = "app.response_cache.write"; +const EVICTION_METRIC = "app.response_cache.eviction"; +const MEMORY_BYTES_METRIC = "app.response_cache.memory.bytes"; +const MEMORY_ENTRIES_METRIC = "app.response_cache.memory.entries"; + +const encoder = new TextEncoder(); +const codec = createBodyCodec(); + +/** The isolate's store, shared by every request it serves. */ +let memoryStore: MemoryStore | undefined; + +/** + * The cache mode for `MCP_RESPONSE_CACHE`. Anything other than `memory` or + * `tiered` is `off`. `tiered` caches in memory only until the R2 tier lands. + */ +export function resolveResponseCacheMode( + value: string | undefined, +): ResponseCacheMode { + const mode = value?.trim().toLowerCase(); + return mode === "memory" || mode === "tiered" ? mode : "off"; +} + +/** + * The cache identity for an MCP grant: a SHA-256 of the user and grant IDs, + * so entries are never keyed by a raw ID. The grant ID stays the same when + * the wrapper token refreshes, and the grant has one Sentry user behind it. + */ +export async function getGrantCacheIdentity( + userId: string, + grantId: string, +): Promise { + const digest = await crypto.subtle.digest( + "SHA-256", + encoder.encode(JSON.stringify(["mcp-grant", userId, grantId])), + ); + return Array.from(new Uint8Array(digest), (byte) => + byte.toString(16).padStart(2, "0"), + ).join(""); +} + +function recordLookup(result: "hit" | "miss" | "stale"): void { + Sentry.metrics.count(LOOKUP_METRIC, 1, { + attributes: { "app.response_cache.result": result }, + }); +} + +/** + * A memory store that reports each write with its body encoding, evictions, + * and its usage after each write, and logs the first time it reaches + * `maxBytes`. + */ +export function createMeteredMemoryStore( + maxBytes = MEMORY_BUDGET_BYTES, +): MemoryStore { + let budgetReached = false; + const store: MemoryStore = createMemoryStore({ + maxBytes, + onEvict: () => { + Sentry.metrics.count(EVICTION_METRIC, 1); + if (!budgetReached) { + budgetReached = true; + logInfo("Response cache reached its memory budget", { + loggerScope: LOGGER_SCOPE, + extra: { budgetBytes: maxBytes, ...store.stats() }, + }); + } + }, + }); + return { + ...store, + async set(key, entry, ttlMs) { + await store.set(key, entry, ttlMs); + Sentry.metrics.count(WRITE_METRIC, 1, { + attributes: { "app.response_cache.encoding": entry.body.encoding }, + }); + const { bytes, entries } = store.stats(); + Sentry.metrics.gauge(MEMORY_BYTES_METRIC, bytes); + Sentry.metrics.gauge(MEMORY_ENTRIES_METRIC, entries); + }, + }; +} + +function isJson(response: Response): boolean { + return /\bjson\b/i.test(response.headers.get("content-type") ?? ""); +} + +export type ResponseCacheOptions = { + store: CacheStore; + identity: string; + /** The Sentry host's base URL, for cross-endpoint invalidation rules. */ + apiBaseUrl: string; + /** Keeps cache writes alive after the response is returned. */ + waitUntil: (promise: Promise) => void; +}; + +/** + * A response cache over `store` for one identity, in the shape + * `ServerContext.responseCache` takes. + */ +export function createResponseCache({ + store, + identity, + apiBaseUrl, + waitUntil, +}: ResponseCacheOptions): ResponseCache { + async function write( + url: string, + requestHeaders: Record, + response: Response, + ): Promise { + // Skip non-JSON bodies, such as attachments, before reading them. + if (classifyUrl(url) === "no-cache" || !isJson(response)) { + response.body?.cancel().catch(() => {}); + return; + } + const key = await buildCacheKey("GET", url, identity); + await storeResponse({ + store, + codec, + key, + identity, + url, + requestHeaders, + response, + }); + } + + return { + identity, + cache: { + async lookup({ url, requestHeaders }) { + if (classifyUrl(url) === "no-cache") { + return; + } + try { + const key = await buildCacheKey("GET", url, identity); + const lookup = await lookupResponse({ + store, + codec, + key, + url, + requestHeaders, + }); + recordLookup(lookup?.kind ?? "miss"); + return lookup; + } catch (error) { + // A corrupt entry is a miss; the next response replaces it. + logWarn("Response cache lookup failed", { + loggerScope: LOGGER_SCOPE, + extra: { error: String(error) }, + }); + recordLookup("miss"); + return; + } + }, + + store({ url, requestHeaders }, response) { + const pending = write(url, requestHeaders, response).catch( + (error: unknown) => { + logWarn("Response cache write failed", { + loggerScope: LOGGER_SCOPE, + extra: { error: String(error) }, + }); + }, + ); + waitUntil(pending); + return pending; + }, + + async invalidate(url) { + for (const prefix of computeInvalidationPrefixes(url, apiBaseUrl)) { + await store.deleteMatching(identity, prefix); + } + }, + }, + }; +} + +/** + * The response cache for one MCP request under `mode`, or `undefined` when + * caching is off. + */ +export async function getResponseCache({ + mode, + userId, + grantId, + sentryHost, + waitUntil, +}: { + mode: ResponseCacheMode; + userId: string; + grantId: string; + sentryHost: string; + waitUntil: (promise: Promise) => void; +}): Promise { + if (mode === "off") { + return; + } + return createResponseCache({ + store: (memoryStore ??= createMeteredMemoryStore()), + identity: await getGrantCacheIdentity(userId, grantId), + apiBaseUrl: `https://${sentryHost}`, + waitUntil, + }); +} diff --git a/packages/mcp-cloudflare/src/server/types.ts b/packages/mcp-cloudflare/src/server/types.ts index 5d493089e..0b4fe1446 100644 --- a/packages/mcp-cloudflare/src/server/types.ts +++ b/packages/mcp-cloudflare/src/server/types.ts @@ -85,4 +85,9 @@ export interface Env { // Backward-compatible fallback while deployments roll out dedicated MCP limiters. MCP_RATE_LIMITER?: RateLimit; AUTORAG_INDEX_NAME?: string; + /** + * Sentry API response cache: `off` (default), `memory`, or `tiered`. See + * docs/cloudflare/architecture.md before enabling it in production. + */ + MCP_RESPONSE_CACHE?: string; } diff --git a/packages/mcp-core/src/api-client/client-response-cache.test.ts b/packages/mcp-core/src/api-client/client-response-cache.test.ts new file mode 100644 index 000000000..378984ec0 --- /dev/null +++ b/packages/mcp-core/src/api-client/client-response-cache.test.ts @@ -0,0 +1,90 @@ +import { mswServer } from "@sentry/mcp-server-mocks"; +import { HttpResponse, http } from "msw"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { SentryApiService } from "./client"; + +describe("responseCache", () => { + afterEach(() => mswServer.resetHandlers()); + + function fakeCache(user: Record | undefined) { + return { + lookup: vi.fn(async () => + user + ? { + kind: "hit" as const, + response: Response.json(user), + createdAt: Date.now(), + size: 0, + } + : undefined, + ), + store: vi.fn(async () => {}), + invalidate: vi.fn(async () => {}), + }; + } + + it("serves a hit without calling the API, under the cache's identity", async () => { + const apiCalled = vi.fn(); + mswServer.use( + http.get("https://sentry.io/api/0/auth/", () => { + apiCalled(); + return HttpResponse.json({ id: "1", name: "API", email: "a@x.io" }); + }), + ); + const cache = fakeCache({ id: "2", name: "Cached", email: "c@x.io" }); + const api = new SentryApiService({ + accessToken: "token", + responseCache: { identity: "identity-a", cache }, + }); + + expect(await api.getAuthenticatedUser()).toMatchObject({ name: "Cached" }); + expect(apiCalled).not.toHaveBeenCalled(); + expect(cache.lookup).toHaveBeenCalledWith( + expect.objectContaining({ + url: "https://sentry.io/api/0/auth/", + identity: "identity-a", + }), + ); + }); + + it("stores a miss under the cache's identity", async () => { + mswServer.use( + http.get("https://sentry.io/api/0/auth/", ({ request }) => + HttpResponse.json({ + id: "1", + name: request.headers.get("Authorization"), + email: "a@x.io", + }), + ), + ); + const cache = fakeCache(undefined); + const api = new SentryApiService({ + accessToken: "token", + responseCache: { identity: "identity-a", cache }, + }); + + // Authorization is unchanged by the caching transport. + expect(await api.getAuthenticatedUser()).toMatchObject({ + name: "Bearer token", + }); + expect(cache.store).toHaveBeenCalledWith( + expect.objectContaining({ identity: "identity-a" }), + expect.any(Response), + ); + }); + + it("is not used without an access token", async () => { + mswServer.use( + http.get("https://sentry.io/api/0/auth/", () => + HttpResponse.json({ id: "1", name: "API", email: "a@x.io" }), + ), + ); + const cache = fakeCache({ id: "2", name: "Cached", email: "c@x.io" }); + const api = new SentryApiService({ + responseCache: { identity: "identity-a", cache }, + }); + + expect(await api.getAuthenticatedUser()).toMatchObject({ name: "API" }); + expect(cache.lookup).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/mcp-core/src/api-client/client.ts b/packages/mcp-core/src/api-client/client.ts index 988733871..12d7eb43c 100644 --- a/packages/mcp-core/src/api-client/client.ts +++ b/packages/mcp-core/src/api-client/client.ts @@ -25,6 +25,7 @@ import { } from "@sentry/toolkit-core/api-client/idempotent-operations"; import type { RetryPolicy } from "@sentry/toolkit-core/api-client/retry"; import { + type ApiClientCore, type ApiTarget, createApiClientCore, listPage, @@ -39,7 +40,7 @@ import { z } from "zod"; import { DEFAULT_SEARCH_ISSUES_PERIOD } from "../constants"; import { ConfigurationError } from "../errors"; import { logIssue, logWarn } from "../telem/logging"; -import type { SentryProtocol } from "../types"; +import type { ResponseCache, SentryProtocol } from "../types"; import { type EventsDataset, isMetricsDataset, @@ -330,6 +331,10 @@ const REQUEST_RETRY_POLICY: RetryPolicy = { maxTotalDelayMs: 3000, }; +const isIdempotent = createIdempotentOperationMatcher(() => + selectIdempotentWrites(operations), +); + /** * Every request goes through the shared transport. `SentryApiService` sets its * own headers, including Authorization, so the transport only retries; as @@ -337,11 +342,27 @@ const REQUEST_RETRY_POLICY: RetryPolicy = { */ const transport = createApiTransport({ retry: REQUEST_RETRY_POLICY, - isIdempotent: createIdempotentOperationMatcher(() => - selectIdempotentWrites(operations), - ), + isIdempotent, }); +/** + * A transport that also serves and invalidates GETs through `responseCache`. + * The transport caches only for a credential, so it gets `accessToken` under + * the cache's identity; the Authorization it sets matches the one + * `SentryApiService` already sends. + */ +function createCachingTransport( + accessToken: string, + { identity, cache }: ResponseCache, +) { + return createApiTransport({ + retry: REQUEST_RETRY_POLICY, + isIdempotent, + credentials: { current: () => ({ token: accessToken, identity }) }, + cache, + }); +} + /** * Default cap for returning attachment bytes inline over the MCP transport. * A 30 MB attachment takes ~80 MB in memory once base64-encoded and serialized, @@ -623,14 +644,7 @@ export class SentryApiService { protected host: string; protected protocol: SentryProtocol; protected apiPrefix: string; - private readonly core = createApiClientCore({ - transport, - headers: () => this.requestHeaders(), - logger: { - warn: (message, extra) => - logWarn(message, { loggerScope: ["api", "client"], extra }), - }, - }); + private readonly core: ApiClientCore; /** * Creates a new Sentry API service instance. @@ -644,6 +658,7 @@ export class SentryApiService { * @param config.clientName DCR-registered OAuth client name * @param config.clientFamily Bucketed client family (e.g. "claude-code", "cursor") * @param config.utmSource Sanitized MCP request attribution source + * @param config.responseCache Caches GETs for `accessToken`'s identity; needs an `accessToken` */ constructor({ accessToken = null, @@ -653,6 +668,7 @@ export class SentryApiService { clientName = null, clientFamily = null, utmSource = null, + responseCache = null, }: { accessToken?: string | null; host?: string; @@ -661,6 +677,7 @@ export class SentryApiService { clientName?: string | null; clientFamily?: string | null; utmSource?: string | null; + responseCache?: ResponseCache | null; }) { this.accessToken = accessToken; this.clientId = clientId; @@ -670,6 +687,17 @@ export class SentryApiService { this.host = host; this.protocol = protocol; this.apiPrefix = `${protocol}://${host}/api/0`; + this.core = createApiClientCore({ + transport: + accessToken !== null && responseCache + ? createCachingTransport(accessToken, responseCache) + : transport, + headers: () => this.requestHeaders(), + logger: { + warn: (message, extra) => + logWarn(message, { loggerScope: ["api", "client"], extra }), + }, + }); } /** diff --git a/packages/mcp-core/src/internal/tool-helpers/api.test.ts b/packages/mcp-core/src/internal/tool-helpers/api.test.ts index 20ba6216b..1cf0c9195 100644 --- a/packages/mcp-core/src/internal/tool-helpers/api.test.ts +++ b/packages/mcp-core/src/internal/tool-helpers/api.test.ts @@ -1,11 +1,69 @@ -import { describe, it, expect } from "vitest"; +import { mswServer } from "@sentry/mcp-server-mocks"; +import { HttpResponse, http } from "msw"; +import { afterEach, describe, it, expect, vi } from "vitest"; import { ApiAuthenticationError, ApiNotFoundError, createApiError, } from "../../api-client"; import { UserInputError } from "../../errors"; -import { handleApiError, withApiErrorHandling } from "./api"; +import type { ServerContext } from "../../types"; +import { + apiServiceFromContext, + handleApiError, + withApiErrorHandling, +} from "./api"; + +describe("apiServiceFromContext", () => { + afterEach(() => mswServer.resetHandlers()); + + function contextWithCache( + overrides: Partial = {}, + ): ServerContext & { + responseCache: NonNullable; + } { + return { + accessToken: "token", + constraints: {}, + responseCache: { + identity: "identity-a", + cache: { + lookup: vi.fn(async () => undefined), + store: vi.fn(async () => {}), + invalidate: vi.fn(async () => {}), + }, + }, + ...overrides, + }; + } + + function mockAuth() { + mswServer.use( + http.get("https://sentry.io/api/0/auth/", () => + HttpResponse.json({ id: "1", name: "User", email: "a@x.io" }), + ), + ); + } + + it("uses the context's response cache", async () => { + mockAuth(); + const context = contextWithCache(); + await apiServiceFromContext(context).getAuthenticatedUser(); + expect(context.responseCache.cache.lookup).toHaveBeenCalled(); + }); + + it("drops the response cache when a credential router swaps tokens", async () => { + mockAuth(); + const context = contextWithCache({ + credentialRouter: { + resolve: vi.fn(), + list: vi.fn(), + }, + }); + await apiServiceFromContext(context).getAuthenticatedUser(); + expect(context.responseCache.cache.lookup).not.toHaveBeenCalled(); + }); +}); describe("handleApiError", () => { it("converts 404 errors with params to list all parameters", () => { diff --git a/packages/mcp-core/src/internal/tool-helpers/api.ts b/packages/mcp-core/src/internal/tool-helpers/api.ts index 8b475a5a3..49a697b24 100644 --- a/packages/mcp-core/src/internal/tool-helpers/api.ts +++ b/packages/mcp-core/src/internal/tool-helpers/api.ts @@ -37,6 +37,8 @@ export function apiServiceFromContext( clientName: context.clientName, clientFamily: context.clientFamily, utmSource: context.utmSource, + // The cache identity belongs to `accessToken`; a router swaps tokens. + responseCache: context.credentialRouter ? null : context.responseCache, }); } diff --git a/packages/mcp-core/src/types.ts b/packages/mcp-core/src/types.ts index 9af4d7573..04c15a0cb 100644 --- a/packages/mcp-core/src/types.ts +++ b/packages/mcp-core/src/types.ts @@ -5,6 +5,7 @@ * and server context. Uses advanced TypeScript patterns for type-safe parameter * extraction and handler registration. */ +import type { TransportCache } from "@sentry/toolkit-core/api-client/transport"; import type { Skill } from "./skills"; /** @@ -77,6 +78,19 @@ export type CredentialRouter = { list(): Promise; }; +/** + * A host's cache for Sentry API GET responses, bound to one identity. + */ +export type ResponseCache = { + /** + * Stable hash of who is asking, which scopes every entry. Never a raw + * token, and it must differ whenever `ServerContext.accessToken` can + * belong to someone else. + */ + identity: string; + cache: TransportCache; +}; + export type ServerContext = { sentryHost?: string; sentryProtocol?: SentryProtocol; @@ -119,4 +133,9 @@ export type ServerContext = { * default credential's token. */ credentialRouter?: CredentialRouter; + /** + * Caches GET responses for this context's single credential. Ignored when + * `credentialRouter` is set, since routed calls run with other tokens. + */ + responseCache?: ResponseCache; }; diff --git a/packages/toolkit-core/src/api-client/response-cache/memory-store.test.ts b/packages/toolkit-core/src/api-client/response-cache/memory-store.test.ts index 71fe9acba..c2bfc3c54 100644 --- a/packages/toolkit-core/src/api-client/response-cache/memory-store.test.ts +++ b/packages/toolkit-core/src/api-client/response-cache/memory-store.test.ts @@ -114,6 +114,23 @@ describe("createMemoryStore", () => { expect(store.stats()).toEqual({ entries: 3, bytes: size * 3 }); }); + it("reports each budget eviction, but not replacements or expiry", async () => { + const size = 100 + ENTRY_OVERHEAD_BYTES; + const onEvict = vi.fn(); + const store = createMemoryStore({ maxBytes: size * 2, onEvict }); + await store.set("a", entry(`${API}/a/`, bodyOfSize(100)), MINUTE); + await store.set("a", entry(`${API}/a/`, bodyOfSize(100)), MINUTE); + await store.set("b", entry(`${API}/b/`, bodyOfSize(100)), 1); + vi.setSystemTime(Date.now() + 2); + await store.get("b"); + expect(onEvict).not.toHaveBeenCalled(); + + await store.set("c", entry(`${API}/c/`, bodyOfSize(100)), MINUTE); + await store.set("d", entry(`${API}/d/`, bodyOfSize(100)), MINUTE); + + expect(onEvict).toHaveBeenCalledTimes(1); + }); + it("evicts as many entries as a large one needs", async () => { const store = createMemoryStore({ maxBytes: 4 * ENTRY_OVERHEAD_BYTES }); for (const key of ["a", "b", "c", "d"]) { diff --git a/packages/toolkit-core/src/api-client/response-cache/memory-store.ts b/packages/toolkit-core/src/api-client/response-cache/memory-store.ts index 5a489c3d9..24f284742 100644 --- a/packages/toolkit-core/src/api-client/response-cache/memory-store.ts +++ b/packages/toolkit-core/src/api-client/response-cache/memory-store.ts @@ -25,6 +25,11 @@ export type MemoryStoreOptions = { * Defaults to 32 MiB. */ maxRawBytes?: number; + /** + * Called for each entry evicted to stay within `maxBytes`. Not called for + * expired, replaced, or deleted entries. + */ + onEvict?: () => void; }; export type MemoryStore = CacheStore & { @@ -72,12 +77,13 @@ export function createMemoryStore( const slots = new Map(); let bytes = 0; - function remove(key: string): void { + function remove(key: string): boolean { const slot = slots.get(key); if (slot) { slots.delete(key); bytes -= slot.size; } + return slot !== undefined; } return { @@ -119,7 +125,9 @@ export function createMemoryStore( if (bytes <= maxBytes) { break; } - remove(oldest); + if (remove(oldest)) { + options.onEvict?.(); + } } }, From ddbee73fafff37ef433f0cfcffd5f40fd0ae19c2 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Tue, 3 Nov 2026 00:46:55 +0000 Subject: [PATCH 2/2] fix(mcp-cloudflare): Release unkept cache bodies and fix CI lint Cancel the transport's body copy whenever the cache does not read it, including JSON responses the policy refuses to store. Let getResponseCache infer its return type: CI lints before building mcp-core, so the ResponseCache | undefined union tripped no-redundant-type-constituents. Co-Authored-By: BYK <126780+BYK@users.noreply.github.com> --- .../src/server/lib/response-cache.test.ts | 12 +++++-- .../src/server/lib/response-cache.ts | 36 +++++++++++-------- 2 files changed, 31 insertions(+), 17 deletions(-) diff --git a/packages/mcp-cloudflare/src/server/lib/response-cache.test.ts b/packages/mcp-cloudflare/src/server/lib/response-cache.test.ts index 0e53e68e4..3bf6aedcf 100644 --- a/packages/mcp-cloudflare/src/server/lib/response-cache.test.ts +++ b/packages/mcp-cloudflare/src/server/lib/response-cache.test.ts @@ -177,7 +177,10 @@ describe("createResponseCache", () => { expect(fetchSpy).toHaveBeenCalledTimes(1); }); - it("does not read or keep a non-JSON body", async () => { + it.each([ + ["a non-JSON body", "text/plain", "max-age=60"], + ["a non-storable JSON body", "application/json", "no-store"], + ])("releases %s it does not keep", async (_, contentType, cacheControl) => { const store = createMemoryStore(); const { cache } = createResponseCache({ store, @@ -185,10 +188,15 @@ describe("createResponseCache", () => { apiBaseUrl: "https://sentry.io", waitUntil, }); + const cancel = vi.fn(); + const body = new ReadableStream({ cancel }); await cache.store( { url: "https://sentry.io/api/0/x/", requestHeaders: {}, identity: "" }, - new Response("plain text", { headers: { "Content-Type": "text/plain" } }), + new Response(body, { + headers: { "Content-Type": contentType, "Cache-Control": cacheControl }, + }), ); + expect(cancel).toHaveBeenCalled(); expect(store.stats().entries).toBe(0); }); }); diff --git a/packages/mcp-cloudflare/src/server/lib/response-cache.ts b/packages/mcp-cloudflare/src/server/lib/response-cache.ts index 678bfc508..8037b3268 100644 --- a/packages/mcp-cloudflare/src/server/lib/response-cache.ts +++ b/packages/mcp-cloudflare/src/server/lib/response-cache.ts @@ -141,21 +141,27 @@ export function createResponseCache({ requestHeaders: Record, response: Response, ): Promise { - // Skip non-JSON bodies, such as attachments, before reading them. - if (classifyUrl(url) === "no-cache" || !isJson(response)) { - response.body?.cancel().catch(() => {}); - return; + try { + // Skip non-JSON bodies, such as attachments, before reading them. + if (classifyUrl(url) === "no-cache" || !isJson(response)) { + return; + } + const key = await buildCacheKey("GET", url, identity); + await storeResponse({ + store, + codec, + key, + identity, + url, + requestHeaders, + response, + }); + } finally { + // Release the transport's copy of the body when nothing read it. + if (!response.bodyUsed) { + response.body?.cancel().catch(() => {}); + } } - const key = await buildCacheKey("GET", url, identity); - await storeResponse({ - store, - codec, - key, - identity, - url, - requestHeaders, - response, - }); } return { @@ -225,7 +231,7 @@ export async function getResponseCache({ grantId: string; sentryHost: string; waitUntil: (promise: Promise) => void; -}): Promise { +}) { if (mode === "off") { return; }