From cfec98a0fdc4b8029c4e507fc588ebf25969762b Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Sat, 10 Oct 2026 02:44:57 +0000 Subject: [PATCH] feat(mcp): Use SDK calls for organization lists Co-Authored-By: GPT-6 Sol --- docs/contributing/api-patterns.md | 11 +- .../mcp-core/src/api-client/client.test.ts | 182 ++++++++++++++++- packages/mcp-core/src/api-client/client.ts | 191 ++++++++++++------ 3 files changed, 311 insertions(+), 73 deletions(-) diff --git a/docs/contributing/api-patterns.md b/docs/contributing/api-patterns.md index 9611d5da2..3510c77fd 100644 --- a/docs/contributing/api-patterns.md +++ b/docs/contributing/api-patterns.md @@ -70,10 +70,13 @@ fail to resolve for legacy mixed-case project slugs; keep them for display. ### Multi-Region Support -Organization discovery uses a single `/api/0/organizations/` request. Public -SaaS hosts use `sentry.io` to list organizations across regions. Single-tenant -hosts under `*.my.sentry.io` and self-hosted instances keep their configured -host for this request. +Organization discovery uses `/api/0/organizations/`; a limit over 100 follows +bounded pages through Sentry's Link cursors. Organization, team, and project +lists use `@sentry/api` operations to build GET paths and queries. The MCP +client still owns the request transport, token handling, GET retries, error +types, and Zod response validation. Public SaaS hosts use `sentry.io` to list +organizations across regions. Single-tenant hosts under `*.my.sentry.io` and +self-hosted instances keep their configured host for this request. User identity (`/api/0/auth/`, used by `whoami`) follows the same control-host routing. Organization-scoped requests continue to use the configured host or diff --git a/packages/mcp-core/src/api-client/client.test.ts b/packages/mcp-core/src/api-client/client.test.ts index 9a6457001..62d3c27b7 100644 --- a/packages/mcp-core/src/api-client/client.test.ts +++ b/packages/mcp-core/src/api-client/client.test.ts @@ -8,7 +8,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { ConfigurationError } from "../errors"; import { parseSentryUrl } from "../internal/url-helpers"; import { SentryApiService } from "./client"; -import { ApiNotFoundError, ApiServerError } from "./errors"; +import { ApiNotFoundError, ApiPermissionError, ApiServerError } from "./errors"; describe("API bearer token validation", () => { it("removes edge padding before sending a request", async () => { @@ -1373,6 +1373,47 @@ describe("listOrganizations", () => { globalThis.fetch = originalFetch; }); + it("collects a requested page larger than the API maximum without skipping its next cursor", async () => { + const pageSizes: number[] = []; + const cursors: (string | null)[] = []; + mswServer.use( + http.get("https://sentry.io/api/0/organizations/", ({ request }) => { + const url = new URL(request.url); + const perPage = Number(url.searchParams.get("per_page")); + const cursor = url.searchParams.get("cursor"); + pageSizes.push(perPage); + cursors.push(cursor); + if (perPage > 100) { + return HttpResponse.json( + { detail: "Invalid per_page" }, + { status: 400 }, + ); + } + const start = cursor === null ? 0 : Number(cursor); + return HttpResponse.json( + Array.from({ length: perPage }, (_, offset) => ({ + id: String(start + offset), + slug: `org-${start + offset}`, + name: `Org ${start + offset}`, + })), + { + headers: { + Link: `; rel="next"; results="true"; cursor="${start + perPage}"`, + }, + }, + ); + }), + ); + + const api = new SentryApiService({ host: "sentry.io" }); + const result = await api.listOrganizations({ limit: 205 }); + expect(result.organizations).toHaveLength(205); + expect(result.organizations[204]?.slug).toBe("org-204"); + expect(result.nextCursor).toBe("205"); + expect(pageSizes).toEqual([100, 100, 5]); + expect(cursors).toEqual([null, "100", "200"]); + }); + it("should fetch from the organizations endpoint on the root host for SaaS", async () => { const mockOrgs = [ { id: "1", slug: "org-us", name: "Org US" }, @@ -1383,11 +1424,7 @@ describe("listOrganizations", () => { globalThis.fetch = vi.fn().mockImplementation((url: string) => { callCount++; if (url.includes("/organizations/")) { - return Promise.resolve({ - ok: true, - headers: new Headers({ "content-type": "application/json" }), - json: () => Promise.resolve(mockOrgs), - }); + return Promise.resolve(HttpResponse.json(mockOrgs)); } return Promise.reject(new Error("Unexpected URL")); }); @@ -1425,11 +1462,7 @@ describe("listOrganizations", () => { globalThis.fetch = vi.fn().mockImplementation((url: string) => { callCount++; if (url.includes("/organizations/")) { - return Promise.resolve({ - ok: true, - headers: new Headers({ "content-type": "application/json" }), - json: () => Promise.resolve(mockOrgs), - }); + return Promise.resolve(HttpResponse.json(mockOrgs)); } return Promise.reject(new Error("Unexpected URL")); }); @@ -1453,6 +1486,133 @@ describe("listOrganizations", () => { }); }); +describe("organization SDK list pages", () => { + it.each([ + { + resource: "teams", + fixture: teamFixture, + list: (api: SentryApiService) => + api.listTeams( + "my-org", + { limit: 201, query: "api" }, + { host: "de.sentry.io" }, + ), + }, + { + resource: "projects", + fixture: projectFixture, + list: (api: SentryApiService) => + api.listProjects( + "my-org", + { limit: 201, query: "api" }, + { host: "de.sentry.io" }, + ), + }, + ])( + "caps each $resource page and retains the regional host", + async ({ resource, fixture, list }) => { + const pageSizes: number[] = []; + mswServer.use( + http.get( + `https://de.sentry.io/api/0/organizations/my-org/${resource}/`, + ({ request }) => { + const url = new URL(request.url); + const perPage = Number(url.searchParams.get("per_page")); + pageSizes.push(perPage); + expect(url.searchParams.get("query")).toBe("api"); + expect(request.headers.get("authorization")).toBe( + "Bearer test-token", + ); + if (perPage > 100) { + return HttpResponse.json( + { detail: "Invalid per_page" }, + { status: 400 }, + ); + } + const start = Number(url.searchParams.get("cursor") ?? "0"); + return HttpResponse.json( + Array.from({ length: perPage }, (_, offset) => ({ + ...fixture, + id: String(start + offset), + })), + { + headers: { + Link: `; rel="next"; results="true"; cursor="${start + perPage}"`, + }, + }, + ); + }, + ), + ); + const api = new SentryApiService({ accessToken: "test-token" }); + const result = await list(api); + const rows = "teams" in result ? result.teams : result.projects; + expect(rows).toHaveLength(201); + expect(result.nextCursor).toBe("201"); + expect(pageSizes).toEqual([100, 100, 1]); + }, + ); + + it("rejects dot-segment organization IDs before an SDK request", async () => { + const api = new SentryApiService({ host: "sentry.example.com" }); + await expect(api.listProjects("..", { limit: 1 })).rejects.toThrow(); + await expect(api.listTeams(".", { limit: 1 })).rejects.toThrow(); + }); + + it("confines an organization slug with a slash to one path segment", async () => { + const paths: string[] = []; + mswServer.use( + http.get("https://sentry.io/api/0/*", ({ request }) => { + paths.push(new URL(request.url).pathname); + return HttpResponse.json([teamFixture]); + }), + ); + + const api = new SentryApiService({ host: "sentry.io" }); + const result = await api.listTeams("my/org"); + expect(result.teams).toHaveLength(1); + expect(paths).toEqual(["/api/0/organizations/my%2Forg/teams/"]); + }); + + it("keeps MCP's HTTP error class and details when an SDK list fails", async () => { + mswServer.use( + http.get("https://sentry.io/api/0/organizations/my-org/teams/", () => + HttpResponse.json({ detail: "Team access denied" }, { status: 403 }), + ), + ); + const api = new SentryApiService({ host: "sentry.io" }); + const error = await api + .listTeams("my-org") + .catch((cause: unknown) => cause); + expect(error).toBeInstanceOf(ApiPermissionError); + expect(error).toMatchObject({ status: 403, detail: "Team access denied" }); + }); + + it("rejects repeated cursors and oversized limits without looping", async () => { + let requests = 0; + mswServer.use( + http.get("https://sentry.io/api/0/organizations/my-org/teams/", () => { + requests += 1; + return HttpResponse.json([teamFixture], { + headers: { + Link: '; rel="next"; results="true"; cursor="again"', + }, + }); + }), + ); + + const api = new SentryApiService({ host: "sentry.io" }); + await expect(api.listTeams("my-org", { limit: 5001 })).rejects.toThrow( + "limit must be an integer", + ); + expect(requests).toBe(0); + await expect(api.listTeams("my-org", { limit: 3 })).rejects.toThrow( + "repeated pagination cursor", + ); + expect(requests).toBe(2); + }); +}); + describe("host configuration", () => { it("should handle hostname without protocol", () => { const apiService = new SentryApiService({ host: "sentry.io" }); diff --git a/packages/mcp-core/src/api-client/client.ts b/packages/mcp-core/src/api-client/client.ts index 1f63c6e26..96f964cdd 100644 --- a/packages/mcp-core/src/api-client/client.ts +++ b/packages/mcp-core/src/api-client/client.ts @@ -1,4 +1,10 @@ -import { parseSentryLinkHeader } from "@sentry/api"; +import { + createSentryClient, + listOrganizationProjects, + listOrganizationTeams, + listOrganizations, + parseSentryLinkHeader, +} from "@sentry/api"; import { buildSentryApiUrl } from "@sentry/toolkit-core/api-request"; import { sentryBearerHeader } from "@sentry/toolkit-core/auth-token"; import { z } from "zod"; @@ -299,6 +305,8 @@ type RequestOptions = { */ const RETRYABLE_REQUEST_MAX_RETRIES = 2; const RETRYABLE_REQUEST_INITIAL_DELAY_MS = 250; +const API_MAX_PER_PAGE = 100; +const MAX_LIST_PAGES = 50; /** * Default cap for returning attachment bytes inline over the MCP transport. @@ -1010,6 +1018,82 @@ export class SentryApiService { return this.parseJsonResponse(response); } + /** Let SDK operations build paths and queries while MCP owns transport and errors. */ + private sdkClient(expectedPath: string, host: string = this.host) { + const origin = new URL(`${this.protocol}://${host}`).origin; + return createSentryClient({ + baseUrl: origin, + fetch: async (sdkRequest) => { + if (!(sdkRequest instanceof Request)) { + throw new ConfigurationError("Unexpected SDK request input"); + } + const url = new URL(sdkRequest.url); + if ( + url.origin !== origin || + url.pathname !== "/api/0".concat(expectedPath) || + sdkRequest.method !== "GET" + ) { + throw new ConfigurationError( + "Unexpected SDK request target or method", + ); + } + const path = `${url.pathname.slice("/api/0".length)}${url.search}`; + const response = await this.request( + path, + { method: "GET", signal: sdkRequest.signal }, + { host }, + ); + const contentType = response.headers.get("content-type"); + if (!contentType?.includes("application/json")) { + await this.parseJsonResponse(response); + } + return response; + }, + }).client; + } + + private async sdkListPage( + fetchPage: ( + perPage: number, + cursor: string | undefined, + ) => Promise<{ data: unknown; response: Response }>, + schema: z.ZodType, + limit: number, + initialCursor?: string | null, + ): Promise<{ items: T[]; nextCursor: string | null }> { + if ( + !Number.isSafeInteger(limit) || + limit < 1 || + limit > API_MAX_PER_PAGE * MAX_LIST_PAGES + ) { + throw new ApiValidationError( + `limit must be an integer between 1 and ${API_MAX_PER_PAGE * MAX_LIST_PAGES}`, + ); + } + const items: T[] = []; + const seenCursors = new Set(); + let cursor = initialCursor ?? undefined; + for (let page = 0; page < MAX_LIST_PAGES; page += 1) { + const perPage = Math.min(API_MAX_PER_PAGE, limit - items.length); + const { data, response } = await fetchPage(perPage, cursor); + const rows = schema.parse(data); + if (rows.length > perPage) { + throw new Error("API returned more items than requested"); + } + items.push(...rows); + const nextCursor = getNextCursor(response.headers.get("link")); + if (items.length >= limit || !nextCursor) { + return { items, nextCursor }; + } + if (seenCursors.has(nextCursor) || nextCursor === cursor) { + throw new Error("API returned a repeated pagination cursor"); + } + seenCursors.add(nextCursor); + cursor = nextCursor; + } + throw new Error("API pagination exceeded its page limit"); + } + /** * Generates a Sentry issue URL for browser navigation. * @@ -1640,33 +1724,26 @@ export class SentryApiService { limit?: number; cursor?: string | null; }): Promise<{ organizations: OrganizationList; nextCursor: string | null }> { - const limit = params?.limit ?? 25; - - // Build query parameters - const queryParams = new URLSearchParams(); - queryParams.set("per_page", String(limit)); - if (params?.query) { - queryParams.set("query", params.query); - } - if (params?.cursor) { - queryParams.set("cursor", params.cursor); - } - const queryString = queryParams.toString(); - const path = `/organizations/?${queryString}`; - - let host = undefined; // Public SaaS lists across regions on sentry.io; single-tenant instances - // must keep organization discovery on their configured host. - if (this.isPublicSaas()) { - host = "sentry.io"; - } - - const response = await this.request(path, undefined, { host }); - const body = await this.parseJsonResponse(response); - + // keep organization discovery on their configured host. + const client = this.sdkClient( + "/organizations/", + this.isPublicSaas() ? "sentry.io" : this.host, + ); + const { items, nextCursor } = await this.sdkListPage( + (perPage, cursor) => + listOrganizations({ + client, + query: { per_page: perPage, query: params?.query, cursor }, + throwOnError: true, + }), + OrganizationListSchema, + params?.limit ?? 25, + params?.cursor, + ); return { - organizations: OrganizationListSchema.parse(body), - nextCursor: getNextCursor(response.headers.get("link")), + organizations: items, + nextCursor, }; } @@ -1718,24 +1795,23 @@ export class SentryApiService { params?: { query?: string; limit?: number; cursor?: string | null }, opts?: RequestOptions, ): Promise<{ teams: TeamList; nextCursor: string | null }> { - const queryParams = new URLSearchParams(); - queryParams.set("per_page", String(params?.limit ?? 25)); - if (params?.query) { - queryParams.set("query", params.query); - } - if (params?.cursor) { - queryParams.set("cursor", params.cursor); - } - const queryString = queryParams.toString(); const teamsPath = apiPath`/organizations/${organizationSlug}/teams/`; - const path = `${teamsPath}?${queryString}`; - - const response = await this.request(path, undefined, opts); - const body = await this.parseJsonResponse(response); - + const client = this.sdkClient(teamsPath, opts?.host); + const { items, nextCursor } = await this.sdkListPage( + (perPage, cursor) => + listOrganizationTeams({ + client, + path: { organization_id_or_slug: organizationSlug }, + query: { per_page: perPage, query: params?.query, cursor }, + throwOnError: true, + }), + TeamListSchema, + params?.limit ?? 25, + params?.cursor, + ); return { - teams: TeamListSchema.parse(body), - nextCursor: getNextCursor(response.headers.get("link")), + teams: items, + nextCursor, }; } @@ -1786,24 +1862,23 @@ export class SentryApiService { params?: { query?: string; limit?: number; cursor?: string | null }, opts?: RequestOptions, ): Promise<{ projects: ProjectList; nextCursor: string | null }> { - const queryParams = new URLSearchParams(); - queryParams.set("per_page", String(params?.limit ?? 25)); - if (params?.query) { - queryParams.set("query", params.query); - } - if (params?.cursor) { - queryParams.set("cursor", params.cursor); - } - const queryString = queryParams.toString(); const projectsPath = apiPath`/organizations/${organizationSlug}/projects/`; - const path = `${projectsPath}?${queryString}`; - - const response = await this.request(path, undefined, opts); - const body = await this.parseJsonResponse(response); - + const client = this.sdkClient(projectsPath, opts?.host); + const { items, nextCursor } = await this.sdkListPage( + (perPage, cursor) => + listOrganizationProjects({ + client, + path: { organization_id_or_slug: organizationSlug }, + query: { per_page: perPage, query: params?.query, cursor }, + throwOnError: true, + }), + ProjectListSchema, + params?.limit ?? 25, + params?.cursor, + ); return { - projects: ProjectListSchema.parse(body), - nextCursor: getNextCursor(response.headers.get("link")), + projects: items, + nextCursor, }; }