From 6c2b69b0e930489f3a3d317d4644bf10a71559da 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:25:53 +0000 Subject: [PATCH 1/3] feat(mcp): Default organizationSlug to the user's only organization A tool call that omits organizationSlug now uses the organization when the login can access exactly one. With several, a tool that requires it fails with a ready-to-run retry and the list of organizations, the same recovery the CLI added in #1634. The server never picks between organizations. Tool registration makes a required organizationSlug optional, and the shared param-prep step fills it before credential routing, so no tool schema or handler changes. The organization list is fetched through the shared client and memoized per context for five minutes. Co-Authored-By: BYK <126780+BYK@users.noreply.github.com> --- docs/contributing/common-patterns.md | 7 + docs/specs/subpath-constraints.md | 1 + .../tool-helpers/credential-routing.ts | 2 +- .../tool-helpers/default-organization.test.ts | 274 +++++++++++++++++ .../tool-helpers/default-organization.ts | 276 ++++++++++++++++++ packages/mcp-core/src/schema.ts | 13 +- packages/mcp-core/src/server.test.ts | 33 ++- .../src/tools/catalog-runtime/availability.ts | 44 ++- .../src/tools/special/execute-tool.ts | 4 +- 9 files changed, 641 insertions(+), 13 deletions(-) create mode 100644 packages/mcp-core/src/internal/tool-helpers/default-organization.test.ts create mode 100644 packages/mcp-core/src/internal/tool-helpers/default-organization.ts diff --git a/docs/contributing/common-patterns.md b/docs/contributing/common-patterns.md index dda599ec0..19b2f4693 100644 --- a/docs/contributing/common-patterns.md +++ b/docs/contributing/common-patterns.md @@ -29,6 +29,13 @@ export const ParamRegionUrl = z See: `packages/mcp-core/src/schema.ts` +Keep `organizationSlug` required when the tool needs it. Tool registration +makes it optional in the published schema, and `applyDefaultOrganization` +(`packages/mcp-core/src/internal/tool-helpers/default-organization.ts`) fills +it in when the caller can access exactly one organization, or fails with the +list and a ready-to-run retry. Handlers never default the organization +themselves. + ### Flexible Schema Patterns ```typescript diff --git a/docs/specs/subpath-constraints.md b/docs/specs/subpath-constraints.md index 983209876..6dd0d273d 100644 --- a/docs/specs/subpath-constraints.md +++ b/docs/specs/subpath-constraints.md @@ -27,6 +27,7 @@ Examples: - Constrained parameters are removed from the tool schemas entirely and injected server-side, so a scoped session cannot override them or reach another organization - Resource identifiers you pass (event, attachment, replay, profile, and client key IDs) are format-validated and URL-encoded, so they cannot rewrite the scoped path either - If you don't provide a scope, tools work across your accessible organizations when supported +- Without a scope, a tool call that omits `organizationSlug` uses your organization when you can access exactly one. With several, the call fails with a ready-to-run retry and the list of organizations; the server never picks one for you - Some tools are filtered when not useful: `find_organizations` is hidden when scoped to an org, and `find_projects` is hidden when scoped to a project ## Access verification diff --git a/packages/mcp-core/src/internal/tool-helpers/credential-routing.ts b/packages/mcp-core/src/internal/tool-helpers/credential-routing.ts index a455f184c..c7f844e4d 100644 --- a/packages/mcp-core/src/internal/tool-helpers/credential-routing.ts +++ b/packages/mcp-core/src/internal/tool-helpers/credential-routing.ts @@ -25,7 +25,7 @@ const ORGANIZATION_URL_PARAMS = [ * constraint injection), else the organization of a Sentry URL parameter. * The tool handlers resolve the organization from the same values. */ -function getTargetOrganizationSlug( +export function getTargetOrganizationSlug( params: Record, ): string | null { const slug = params.organizationSlug; diff --git a/packages/mcp-core/src/internal/tool-helpers/default-organization.test.ts b/packages/mcp-core/src/internal/tool-helpers/default-organization.test.ts new file mode 100644 index 000000000..8120c05e6 --- /dev/null +++ b/packages/mcp-core/src/internal/tool-helpers/default-organization.test.ts @@ -0,0 +1,274 @@ +import { mswServer } from "@sentry/mcp-server-mocks"; +import { HttpResponse, http } from "msw"; +import { afterEach, describe, expect, it } from "vitest"; +import { z } from "zod"; +import { ParamOrganizationSlug, ParamRegionUrl } from "../../schema"; +import { getServerContext } from "../../test-setup"; +import { + getFilteredInputSchema, + prepareToolCallParams, +} from "../../tools/catalog-runtime/availability"; +import type { ToolConfig } from "../../tools/types"; +import type { CredentialRouter, ServerContext } from "../../types"; + +const requiredOrgTool: ToolConfig = { + name: "required_org_tool", + description: "required_org_tool description", + skills: ["inspect"], + requiredScopes: [], + annotations: { + readOnlyHint: true, + destructiveHint: false, + openWorldHint: true, + }, + inputSchema: { + organizationSlug: ParamOrganizationSlug, + regionUrl: ParamRegionUrl.nullable().default(null), + query: z.string().default("is:unresolved"), + }, + handler: async () => "ok", +}; + +const optionalOrgTool: ToolConfig = { + ...requiredOrgTool, + name: "optional_org_tool", + inputSchema: { + organizationSlug: ParamOrganizationSlug.optional(), + issueUrl: z.string().optional(), + issueId: z.string().optional(), + }, +}; + +function organization(slug: string, regionUrl = "https://us.sentry.io") { + return { + id: slug, + slug, + name: slug, + links: { organizationUrl: `https://${slug}.sentry.io`, regionUrl }, + }; +} + +/** Serve the organization list and count how often it is requested. */ +function serveOrganizations( + organizations: ReturnType[], + { hasMore = false }: { hasMore?: boolean } = {}, +) { + const requests: string[] = []; + mswServer.use( + http.get("https://sentry.io/api/0/organizations/", ({ request }) => { + requests.push(request.headers.get("authorization") ?? ""); + return HttpResponse.json(organizations, { + headers: hasMore + ? { + Link: '; rel="next"; results="true"; cursor="0:25:0"', + } + : {}, + }); + }), + ); + return requests; +} + +async function prepare( + tool: ToolConfig, + params: Record, + context: ServerContext = getServerContext({ sentryHost: "sentry.io" }), +) { + return await prepareToolCallParams({ tool, params, context }); +} + +describe("applyDefaultOrganization", () => { + afterEach(() => { + mswServer.resetHandlers(); + }); + + it("makes a required organizationSlug optional unless a constraint sets it", () => { + const unscoped = getFilteredInputSchema( + requiredOrgTool, + getServerContext(), + ); + expect(unscoped.organizationSlug?.safeParse(undefined).success).toBe(true); + expect(unscoped.organizationSlug?.safeParse("").success).toBe(false); + + const scoped = getFilteredInputSchema( + requiredOrgTool, + getServerContext({ constraints: { organizationSlug: "my-org" } }), + ); + expect(scoped).not.toHaveProperty("organizationSlug"); + }); + + it("uses the only organization and its region", async () => { + serveOrganizations([organization("solo-org", "https://de.sentry.io")]); + + await expect(prepare(requiredOrgTool, {})).resolves.toEqual({ + organizationSlug: "solo-org", + regionUrl: "https://de.sentry.io", + query: "is:unresolved", + }); + }); + + it("keeps a regionUrl the caller passed", async () => { + serveOrganizations([organization("solo-org", "https://de.sentry.io")]); + + await expect( + prepare(requiredOrgTool, { regionUrl: "https://us.sentry.io" }), + ).resolves.toMatchObject({ + organizationSlug: "solo-org", + regionUrl: "https://us.sentry.io", + }); + }); + + it("leaves regionUrl unset when the organization has none (self-hosted)", async () => { + const requests: string[] = []; + mswServer.use( + http.get("https://sentry.example.com/api/0/organizations/", () => { + requests.push("self-hosted"); + return HttpResponse.json([organization("acme", "")]); + }), + ); + + await expect( + prepare( + requiredOrgTool, + {}, + getServerContext({ sentryHost: "sentry.example.com" }), + ), + ).resolves.toEqual({ + organizationSlug: "acme", + regionUrl: null, + query: "is:unresolved", + }); + expect(requests).toEqual(["self-hosted"]); + }); + + it("lists the organizations once per context", async () => { + const requests = serveOrganizations([organization("solo-org")]); + const context = getServerContext({ sentryHost: "sentry.io" }); + + await prepare(requiredOrgTool, {}, context); + await prepare(requiredOrgTool, { query: "is:resolved" }, context); + + expect(requests).toHaveLength(1); + }); + + it("does not list organizations when the call names one", async () => { + const requests = serveOrganizations([organization("solo-org")]); + + await expect( + prepare(requiredOrgTool, { organizationSlug: "my-org" }), + ).resolves.toMatchObject({ organizationSlug: "my-org" }); + await expect( + prepare(optionalOrgTool, { + issueUrl: "https://other-org.sentry.io/issues/PROJ-1/", + }), + ).resolves.not.toHaveProperty("organizationSlug"); + expect(requests).toHaveLength(0); + }); + + it("fails with a ready-to-run retry when there are several organizations", async () => { + serveOrganizations([ + organization("org-a"), + organization("org-b"), + organization("org-c"), + ]); + + await expect(prepare(requiredOrgTool, { query: "is:resolved" })).rejects + .toThrowErrorMatchingInlineSnapshot(` + [UserInputError: \`organizationSlug\` is required: you can access more than one organization, so none was picked. + + Retry example: \`required_org_tool(organizationSlug='org-a', query='is:resolved')\` + Other available organizations: org-b, org-c] + `); + }); + + it("writes the retry as execute_sentry_tool for a catalog tool", async () => { + serveOrganizations( + Array.from({ length: 25 }, (_, index) => organization(`org-${index}`)), + { hasMore: true }, + ); + const context = getServerContext({ + sentryHost: "sentry.io", + directToolNames: new Set(["execute_sentry_tool"]), + }); + + const error = await prepare( + requiredOrgTool, + { query: "is:resolved" }, + context, + ).catch((caught: unknown) => caught); + + expect(String(error)).toContain( + `Retry example: \`execute_sentry_tool(name='required_org_tool', arguments={"organizationSlug":"org-0","query":"is:resolved"})\``, + ); + expect(String(error)).toContain( + "More organizations exist. Use `find_organizations(query='...')` to search them.", + ); + }); + + it("leaves default values out of the retry", async () => { + serveOrganizations([organization("org-a"), organization("org-b")]); + + await expect( + prepare(requiredOrgTool, { query: "is:unresolved", regionUrl: null }), + ).rejects.toThrow("`required_org_tool(organizationSlug='org-a')`"); + }); + + it("leaves an optional organizationSlug unset when there are several", async () => { + serveOrganizations([organization("org-a"), organization("org-b")]); + + await expect( + prepare(optionalOrgTool, { issueId: "PROJ-1" }), + ).resolves.toEqual({ issueId: "PROJ-1" }); + }); + + it("fills an optional organizationSlug when there is only one", async () => { + serveOrganizations([organization("solo-org")]); + + await expect( + prepare(optionalOrgTool, { issueId: "PROJ-1" }), + ).resolves.toEqual({ issueId: "PROJ-1", organizationSlug: "solo-org" }); + }); + + it("falls back to the find_organizations hint when listing fails", async () => { + mswServer.use( + http.get("https://sentry.io/api/0/organizations/", () => + HttpResponse.json({ detail: "Forbidden" }, { status: 403 }), + ), + ); + + await expect(prepare(requiredOrgTool, {})).rejects.toThrow( + "Invalid arguments for required_org_tool: organizationSlug: Required. Call `find_organizations()`", + ); + await expect( + prepare(optionalOrgTool, { issueId: "PROJ-1" }), + ).resolves.toEqual({ issueId: "PROJ-1" }); + }); + + it("counts every login's organizations once", async () => { + mswServer.use( + http.get("https://sentry.io/api/0/organizations/", ({ request }) => + HttpResponse.json( + request.headers.get("authorization") === "Bearer work-token" + ? [organization("shared-org")] + : [organization("shared-org"), organization("side-org")], + ), + ), + ); + const credentials = [ + { id: "work", accessToken: "work-token" }, + { id: "personal", accessToken: "personal-token" }, + ]; + const credentialRouter: CredentialRouter = { + resolve: async () => credentials[0]!, + list: async () => credentials, + }; + const context = getServerContext({ + sentryHost: "sentry.io", + credentialRouter, + }); + + await expect(prepare(requiredOrgTool, {}, context)).rejects.toThrow( + "Other available organizations: side-org", + ); + }); +}); diff --git a/packages/mcp-core/src/internal/tool-helpers/default-organization.ts b/packages/mcp-core/src/internal/tool-helpers/default-organization.ts new file mode 100644 index 000000000..2c83239da --- /dev/null +++ b/packages/mcp-core/src/internal/tool-helpers/default-organization.ts @@ -0,0 +1,276 @@ +/** + * Default `organizationSlug` for tool calls that omit it, the MCP side of + * the CLI's missing-context recovery (#1634). When the login reaches exactly + * one organization, the call uses it. A tool that requires an organization + * and reaches several fails with a ready-to-run retry and the list, so the + * caller never gets a guessed organization. + */ +import { inlineCode } from "@sentry/toolkit-core/markdown"; +import type { ZodType } from "zod"; +import { ApiError } from "../../api-client"; +import { UserInputError } from "../../errors"; +import { MISSING_ORGANIZATION_SLUG_HINT } from "../../schema"; +import type { ToolConfig } from "../../tools/types"; +import type { ServerContext } from "../../types"; +import { apiServiceFromContext } from "./api"; +import { + getTargetOrganizationSlug, + listCredentialContexts, + mapCredentialContexts, +} from "./credential-routing"; +import { formatToolCall } from "./tool-call-formatting"; + +/** Matches the `find_organizations` page size. */ +const ORGANIZATION_LIST_LIMIT = 25; +/** + * Matches the shared response cache's `stable` tier, so a new organization + * shows up at most this late in a long-lived stdio session. + */ +const ORGANIZATION_LIST_TTL_MS = 5 * 60 * 1000; + +type ReachableOrganizations = { + organizations: Array<{ slug: string; regionUrl: string | null }>; + hasMore: boolean; +}; + +type CachedOrganizations = { + accessToken: string; + sentryHost: ServerContext["sentryHost"]; + sentryProtocol: ServerContext["sentryProtocol"]; + expiresAt: number; + pending: Promise; +}; + +const organizationsCache = new WeakMap(); + +type ToolArguments = NonNullable< + Parameters[0]["arguments"] +>; + +async function fetchReachableOrganizations( + context: ServerContext, +): Promise { + const listPage = async (callContext: ServerContext) => + await apiServiceFromContext(callContext).listOrganizations({ + limit: ORGANIZATION_LIST_LIMIT, + }); + // With several logins, an organization any of them reaches counts, and + // the credential router later picks the login that knows it. + const accounts = await listCredentialContexts(context); + const pages = accounts + ? ( + await mapCredentialContexts(accounts, ({ context: callContext }) => + listPage(callContext), + ) + ).map(({ value }) => value) + : [await listPage(context)]; + + const organizations = new Map(); + for (const page of pages) { + for (const organization of page.organizations) { + if (!organizations.has(organization.slug)) { + organizations.set( + organization.slug, + organization.links?.regionUrl?.trim() || null, + ); + } + } + } + return { + organizations: [...organizations].map(([slug, regionUrl]) => ({ + slug, + regionUrl, + })), + hasMore: pages.some(({ nextCursor }) => nextCursor !== null), + }; +} + +/** + * The organizations the context's login reaches, listed once per context, + * credential, and host for {@link ORGANIZATION_LIST_TTL_MS}. A failed + * listing is not kept. + */ +function getReachableOrganizations( + context: ServerContext, +): Promise { + const cached = organizationsCache.get(context); + if ( + cached && + cached.expiresAt > Date.now() && + cached.accessToken === context.accessToken && + cached.sentryHost === context.sentryHost && + cached.sentryProtocol === context.sentryProtocol + ) { + return cached.pending; + } + const pending = fetchReachableOrganizations(context); + const entry: CachedOrganizations = { + accessToken: context.accessToken, + sentryHost: context.sentryHost, + sentryProtocol: context.sentryProtocol, + expiresAt: Date.now() + ORGANIZATION_LIST_TTL_MS, + pending, + }; + organizationsCache.set(context, entry); + pending.catch(() => { + if (organizationsCache.get(context) === entry) { + organizationsCache.delete(context); + } + }); + return pending; +} + +/** Whether a tool's `organizationSlug` schema rejects an omitted value. */ +export function isOrganizationSlugRequired(schema: ZodType): boolean { + return !schema.safeParse(undefined).success; +} + +function isMissing(value: unknown): boolean { + return value == null || (typeof value === "string" && !value.trim()); +} + +/** Whether `value` is what the tool's schema fills in for an omitted `key`. */ +function isDefaultValue( + tool: ToolConfig, + key: string, + value: unknown, +): boolean { + const omitted = tool.inputSchema[key]?.safeParse(undefined); + return ( + !!omitted?.success && JSON.stringify(omitted.data) === JSON.stringify(value) + ); +} + +/** + * The same call with `organizationSlug` set, in the form the caller used: + * a direct tool call, or `execute_sentry_tool` for a catalog tool. The MCP + * SDK hands direct tools their arguments with defaults applied, so values + * equal to a default are left out to keep the retry short. + */ +function formatRetry( + tool: ToolConfig, + rawParams: Record, + organizationSlug: string, + context: ServerContext, +): string { + const { organizationSlug: _omitted, ...rest } = rawParams; + const args = Object.fromEntries( + Object.entries({ organizationSlug, ...rest }).filter( + ([key, value]) => + value !== undefined && !isDefaultValue(tool, key, value), + ), + ) as ToolArguments; + const isDirect = context.directToolNames?.has(tool.name) ?? true; + return inlineCode( + isDirect + ? formatToolCall({ toolName: tool.name, arguments: args }) + : formatToolCall({ + toolName: "execute_sentry_tool", + arguments: { name: tool.name, arguments: args }, + }), + ); +} + +function formatMultipleOrganizationsError( + tool: ToolConfig, + rawParams: Record, + reachable: ReachableOrganizations, + context: ServerContext, +): string { + const [first = "", ...others] = reachable.organizations.map( + ({ slug }) => slug, + ); + const lines = [ + "`organizationSlug` is required: you can access more than one organization, so none was picked.", + "", + `Retry example: ${formatRetry(tool, rawParams, first, context)}`, + ]; + if (others.length > 0) { + lines.push(`Other available organizations: ${others.join(", ")}`); + } + if (reachable.hasMore) { + lines.push( + "More organizations exist. Use `find_organizations(query='...')` to search them.", + ); + } + return lines.join("\n"); +} + +/** + * Fill `organizationSlug` when a call omits it and names no Sentry URL that + * carries the organization. Runs after constraint injection, so an + * org-scoped session never reaches it. + * + * - One reachable organization: the call uses it, and its `regionUrl` when + * the tool takes one and the caller left it unset. + * - Several or none: a tool that requires `organizationSlug` fails with the + * list and a ready-to-run retry; a tool where it is optional runs as is + * and reports what it is missing. + */ +export async function applyDefaultOrganization({ + tool, + params, + rawParams, + context, +}: { + tool: ToolConfig; + params: Record; + rawParams: Record; + context: ServerContext; +}): Promise> { + const organizationSchema = tool.inputSchema.organizationSlug; + if ( + !organizationSchema || + !isMissing(params.organizationSlug) || + getTargetOrganizationSlug(params) !== null + ) { + return params; + } + const isRequired = isOrganizationSlugRequired(organizationSchema); + + let reachable: ReachableOrganizations; + try { + reachable = await getReachableOrganizations(context); + } catch (error) { + if (!(error instanceof ApiError)) { + throw error; + } + if (!isRequired) { + return params; + } + // The login cannot list organizations (such as a token without + // org:read), so fall back to the schema's own recovery hint. + throw new UserInputError( + `Invalid arguments for ${tool.name}: organizationSlug: ${MISSING_ORGANIZATION_SLUG_HINT}`, + { cause: error }, + ); + } + + const [only] = reachable.organizations; + if (only && reachable.organizations.length === 1) { + const defaulted: Record = { + ...params, + organizationSlug: only.slug, + }; + if ( + only.regionUrl && + "regionUrl" in tool.inputSchema && + isMissing(params.regionUrl) + ) { + defaulted.regionUrl = only.regionUrl; + } + return defaulted; + } + + if (!isRequired) { + return params; + } + if (!only) { + throw new UserInputError( + "`organizationSlug` is required, and this login cannot access any organization.", + ); + } + throw new UserInputError( + formatMultipleOrganizationsError(tool, rawParams, reachable, context), + ); +} diff --git a/packages/mcp-core/src/schema.ts b/packages/mcp-core/src/schema.ts index fb86c4381..64810f506 100644 --- a/packages/mcp-core/src/schema.ts +++ b/packages/mcp-core/src/schema.ts @@ -9,16 +9,19 @@ import { z } from "zod"; import { SENTRY_GUIDES } from "./constants"; import { validateResourceId, validateSlug } from "./utils/slug-validation"; +/** Recovery hint for a missing organization the server could not default. */ +export const MISSING_ORGANIZATION_SLUG_HINT = + "Required. Call `find_organizations()` to list your organizations, then retry with `organizationSlug` set to one of their slugs."; + // Sentry slug lookups can be exact and case-sensitive on legacy instances. // Preserve caller casing for resource slugs; only trim and validate shape. export const ParamOrganizationSlug = z .string({ - // The MCP SDK rejects a missing slug before the handler runs, so this - // message is the only recovery hint the caller gets. + // Tool registration makes a required slug optional and defaults it + // (see applyDefaultOrganization), but a null slug, or one inside a nested + // schema, still fails here, and this is the recovery hint it gets. error: (issue) => - issue.input == null - ? "Required. Call `find_organizations()` to list your organizations, then retry with `organizationSlug` set to one of their slugs." - : undefined, + issue.input == null ? MISSING_ORGANIZATION_SLUG_HINT : undefined, }) .trim() .superRefine(validateSlug) diff --git a/packages/mcp-core/src/server.test.ts b/packages/mcp-core/src/server.test.ts index 02743d998..5abca3fa2 100644 --- a/packages/mcp-core/src/server.test.ts +++ b/packages/mcp-core/src/server.test.ts @@ -502,7 +502,32 @@ describe("buildServer", () => { }); }); - it("explains how to recover a missing organizationSlug", async () => { + it("defaults a missing organizationSlug to the only organization", async () => { + const server = buildServer({ context: baseContext }); + + const result = await callRegisteredTool(server, "search_issues", { + query: "is:unresolved", + period: "1h", + }); + + expect(result.isError).toBeFalsy(); + expect(getTextContent(result)).toContain( + "https://sentry-mcp-evals.sentry.io/issues/", + ); + }); + + it("lists the organizations when a missing organizationSlug is ambiguous", async () => { + mswServer.use( + http.get( + "https://sentry.io/api/0/organizations/", + () => + HttpResponse.json([ + { id: "1", slug: "org-a", name: "A" }, + { id: "2", slug: "org-b", name: "B" }, + ]), + { once: true }, + ), + ); const server = buildServer({ context: baseContext }); const result = await callRegisteredTool(server, "search_issues", { @@ -512,8 +537,10 @@ describe("buildServer", () => { expect(result).toMatchObject({ isError: true }); const text = getTextContent(result); - expect(text).toContain("Required. Call `find_organizations()`"); - expect(text).not.toContain("period"); + expect(text).toContain( + "`search_issues(organizationSlug='org-a', period='1h')`", + ); + expect(text).toContain("Other available organizations: org-b"); }); it("caps tool input elements", async () => { diff --git a/packages/mcp-core/src/tools/catalog-runtime/availability.ts b/packages/mcp-core/src/tools/catalog-runtime/availability.ts index 629cd5e4d..6bd7658ea 100644 --- a/packages/mcp-core/src/tools/catalog-runtime/availability.ts +++ b/packages/mcp-core/src/tools/catalog-runtime/availability.ts @@ -6,6 +6,10 @@ import { getConstraintParametersToInject, } from "../../internal/constraint-helpers"; import { withRoutedCredential } from "../../internal/tool-helpers/credential-routing"; +import { + applyDefaultOrganization, + isOrganizationSlugRequired, +} from "../../internal/tool-helpers/default-organization"; import { type Skill, isEnabledBySkills } from "../../skills"; import type { ProjectCapabilities, ServerContext } from "../../types"; import { @@ -120,6 +124,11 @@ function isMissingFeedbackDsn(key: string): boolean { return key === "submit_feedback_to_sentry_team" && !getClient()?.getDsn(); } +/** + * The input schema a session exposes: constrained keys are removed, and a + * required `organizationSlug` becomes optional because + * {@link applyDefaultOrganization} fills it, or explains which one to pass. + */ export function getFilteredInputSchema( tool: ToolConfig, context: ServerContext, @@ -128,11 +137,16 @@ export function getFilteredInputSchema( getConstraintKeysToFilter(context.constraints, tool.inputSchema), ); - return Object.fromEntries( + const schema = Object.fromEntries( Object.entries(tool.inputSchema).filter( ([key]) => !constraintKeysToFilter.has(key), ), ) as Record; + const organizationSlug = schema.organizationSlug; + if (organizationSlug && isOrganizationSlugRequired(organizationSlug)) { + schema.organizationSlug = organizationSlug.optional(); + } + return schema; } export function injectConstraintParams( @@ -248,7 +262,11 @@ export async function executeToolHandler({ params: Record; context: ServerContext; }) { - const paramsWithConstraints = prepareToolParams({ tool, params, context }); + const paramsWithConstraints = await prepareToolCallParams({ + tool, + params, + context, + }); return await withRoutedCredential( context, @@ -280,6 +298,28 @@ export function prepareToolParams({ return injectConstraintParams(parsed.data, tool, context); } +/** + * {@link prepareToolParams}, then {@link applyDefaultOrganization}. Every + * tool call runs through this before its credential is routed, so routing + * sees the defaulted organization. + */ +export async function prepareToolCallParams({ + tool, + params, + context, +}: { + tool: ToolConfig; + params: Record; + context: ServerContext; +}): Promise> { + return await applyDefaultOrganization({ + tool, + params: prepareToolParams({ tool, params, context }), + rawParams: params, + context, + }); +} + export function resolveToolDescription( tool: ToolConfig, context: DescriptionContext, diff --git a/packages/mcp-core/src/tools/special/execute-tool.ts b/packages/mcp-core/src/tools/special/execute-tool.ts index 28a7189c2..9a248180e 100644 --- a/packages/mcp-core/src/tools/special/execute-tool.ts +++ b/packages/mcp-core/src/tools/special/execute-tool.ts @@ -11,7 +11,7 @@ import { ALL_SKILLS } from "../../skills"; import type { ServerContext } from "../../types"; import { getSearchableTools, - prepareToolParams, + prepareToolCallParams, type ToolRegistry, } from "../catalog-runtime/availability"; import type { ToolConfig } from "../types"; @@ -52,7 +52,7 @@ async function executeCatalogToolWithSpan({ }, async (span) => { try { - const paramsWithConstraints = prepareToolParams({ + const paramsWithConstraints = await prepareToolCallParams({ tool, params, context, From 956b757b8685fc61e8b1cc42071419cfe3eef61e 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:36:10 +0000 Subject: [PATCH 2/3] fix(mcp): Default the organization only from a complete list With several logins, a login whose organization listing fails is skipped, so one remaining organization could be defaulted while the failed login hid another. Default only when every login answered; otherwise the required-slug error says some logins could not list their organizations. A partial list is not memoized. --- .../tool-helpers/default-organization.test.ts | 38 +++++++++++++++++++ .../tool-helpers/default-organization.ts | 38 +++++++++++++++---- 2 files changed, 69 insertions(+), 7 deletions(-) diff --git a/packages/mcp-core/src/internal/tool-helpers/default-organization.test.ts b/packages/mcp-core/src/internal/tool-helpers/default-organization.test.ts index 8120c05e6..d566a8711 100644 --- a/packages/mcp-core/src/internal/tool-helpers/default-organization.test.ts +++ b/packages/mcp-core/src/internal/tool-helpers/default-organization.test.ts @@ -244,6 +244,44 @@ describe("applyDefaultOrganization", () => { ).resolves.toEqual({ issueId: "PROJ-1" }); }); + it("does not default when another login cannot list its organizations", async () => { + const requests: string[] = []; + mswServer.use( + http.get("https://sentry.io/api/0/organizations/", ({ request }) => { + const authorization = request.headers.get("authorization") ?? ""; + requests.push(authorization); + return authorization === "Bearer work-token" + ? HttpResponse.json([organization("work-org")]) + : HttpResponse.json({ detail: "Internal error" }, { status: 500 }); + }), + ); + const credentials = [ + { id: "work", accessToken: "work-token" }, + { id: "personal", accessToken: "personal-token" }, + ]; + const context = getServerContext({ + sentryHost: "sentry.io", + credentialRouter: { + resolve: async () => credentials[0]!, + list: async () => credentials, + }, + }); + + await expect(prepare(requiredOrgTool, {}, context)).rejects + .toThrowErrorMatchingInlineSnapshot(` + [UserInputError: \`organizationSlug\` is required: some logins could not list their organizations, so none was picked. + + Retry example: \`required_org_tool(organizationSlug='work-org')\` + Call \`find_organizations()\` to check which organizations each login can access.] + `); + await expect( + prepare(optionalOrgTool, { issueId: "PROJ-1" }, context), + ).resolves.toEqual({ issueId: "PROJ-1" }); + // A partial list is not kept, so the next call asks every login again. + const workRequests = requests.filter((r) => r === "Bearer work-token"); + expect(workRequests).toHaveLength(2); + }); + it("counts every login's organizations once", async () => { mswServer.use( http.get("https://sentry.io/api/0/organizations/", ({ request }) => diff --git a/packages/mcp-core/src/internal/tool-helpers/default-organization.ts b/packages/mcp-core/src/internal/tool-helpers/default-organization.ts index 2c83239da..a15b4649d 100644 --- a/packages/mcp-core/src/internal/tool-helpers/default-organization.ts +++ b/packages/mcp-core/src/internal/tool-helpers/default-organization.ts @@ -31,6 +31,8 @@ const ORGANIZATION_LIST_TTL_MS = 5 * 60 * 1000; type ReachableOrganizations = { organizations: Array<{ slug: string; regionUrl: string | null }>; hasMore: boolean; + /** A login's listing failed, so the list may miss organizations. */ + isPartial: boolean; }; type CachedOrganizations = { @@ -64,6 +66,8 @@ async function fetchReachableOrganizations( ) ).map(({ value }) => value) : [await listPage(context)]; + // mapCredentialContexts skips a login whose listing fails. + const isPartial = accounts !== null && pages.length < accounts.length; const organizations = new Map(); for (const page of pages) { @@ -82,13 +86,14 @@ async function fetchReachableOrganizations( regionUrl, })), hasMore: pages.some(({ nextCursor }) => nextCursor !== null), + isPartial, }; } /** * The organizations the context's login reaches, listed once per context, - * credential, and host for {@link ORGANIZATION_LIST_TTL_MS}. A failed - * listing is not kept. + * credential, and host for {@link ORGANIZATION_LIST_TTL_MS}. A failed or + * partial listing is not kept. */ function getReachableOrganizations( context: ServerContext, @@ -112,11 +117,12 @@ function getReachableOrganizations( pending, }; organizationsCache.set(context, entry); - pending.catch(() => { + const forget = () => { if (organizationsCache.get(context) === entry) { organizationsCache.delete(context); } - }); + }; + pending.then(({ isPartial }) => isPartial && forget(), forget); return pending; } @@ -181,14 +187,20 @@ function formatMultipleOrganizationsError( ({ slug }) => slug, ); const lines = [ - "`organizationSlug` is required: you can access more than one organization, so none was picked.", + reachable.isPartial + ? "`organizationSlug` is required: some logins could not list their organizations, so none was picked." + : "`organizationSlug` is required: you can access more than one organization, so none was picked.", "", `Retry example: ${formatRetry(tool, rawParams, first, context)}`, ]; if (others.length > 0) { lines.push(`Other available organizations: ${others.join(", ")}`); } - if (reachable.hasMore) { + if (reachable.isPartial) { + lines.push( + "Call `find_organizations()` to check which organizations each login can access.", + ); + } else if (reachable.hasMore) { lines.push( "More organizations exist. Use `find_organizations(query='...')` to search them.", ); @@ -247,7 +259,14 @@ export async function applyDefaultOrganization({ } const [only] = reachable.organizations; - if (only && reachable.organizations.length === 1) { + // Default only from a complete list, so a login that failed to answer + // cannot hide a second organization. + if ( + only && + reachable.organizations.length === 1 && + !reachable.hasMore && + !reachable.isPartial + ) { const defaulted: Record = { ...params, organizationSlug: only.slug, @@ -265,6 +284,11 @@ export async function applyDefaultOrganization({ if (!isRequired) { return params; } + if (!only && reachable.isPartial) { + throw new UserInputError( + `Invalid arguments for ${tool.name}: organizationSlug: ${MISSING_ORGANIZATION_SLUG_HINT}`, + ); + } if (!only) { throw new UserInputError( "`organizationSlug` is required, and this login cannot access any organization.", From 4ae48dfbf8fe6b987ce0e801de9858e14b1a013e 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:57:14 +0000 Subject: [PATCH 3/3] fix(mcp): Default a required slug even when the tool takes an external URL The URL check that lets a Sentry URL stand in for organizationSlug also ran for tools that require the slug. Uptime monitor tools take their target as `url`, so https://example.com/health was read as organization "example" and the default was skipped. Only optional slugs check URLs now; a tool that requires the slug never reads it from a URL. --- .../tool-helpers/default-organization.test.ts | 19 +++++++++++++++++++ .../tool-helpers/default-organization.ts | 16 +++++++++------- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/packages/mcp-core/src/internal/tool-helpers/default-organization.test.ts b/packages/mcp-core/src/internal/tool-helpers/default-organization.test.ts index d566a8711..1d4a299e5 100644 --- a/packages/mcp-core/src/internal/tool-helpers/default-organization.test.ts +++ b/packages/mcp-core/src/internal/tool-helpers/default-organization.test.ts @@ -165,6 +165,25 @@ describe("applyDefaultOrganization", () => { expect(requests).toHaveLength(0); }); + it("defaults a required slug even when the tool's url is an external one", async () => { + serveOrganizations([organization("solo-org")]); + const uptimeLikeTool: ToolConfig = { + ...requiredOrgTool, + name: "uptime_like_tool", + inputSchema: { + organizationSlug: ParamOrganizationSlug, + url: z.string().url(), + }, + }; + + await expect( + prepare(uptimeLikeTool, { url: "https://example.com/health" }), + ).resolves.toEqual({ + organizationSlug: "solo-org", + url: "https://example.com/health", + }); + }); + it("fails with a ready-to-run retry when there are several organizations", async () => { serveOrganizations([ organization("org-a"), diff --git a/packages/mcp-core/src/internal/tool-helpers/default-organization.ts b/packages/mcp-core/src/internal/tool-helpers/default-organization.ts index a15b4649d..391c0d039 100644 --- a/packages/mcp-core/src/internal/tool-helpers/default-organization.ts +++ b/packages/mcp-core/src/internal/tool-helpers/default-organization.ts @@ -209,8 +209,8 @@ function formatMultipleOrganizationsError( } /** - * Fill `organizationSlug` when a call omits it and names no Sentry URL that - * carries the organization. Runs after constraint injection, so an + * Fill `organizationSlug` when a call omits it and, for a tool where it is + * optional, names no Sentry URL that carries the organization. Runs after constraint injection, so an * org-scoped session never reaches it. * * - One reachable organization: the call uses it, and its `regionUrl` when @@ -231,14 +231,16 @@ export async function applyDefaultOrganization({ context: ServerContext; }): Promise> { const organizationSchema = tool.inputSchema.organizationSlug; - if ( - !organizationSchema || - !isMissing(params.organizationSlug) || - getTargetOrganizationSlug(params) !== null - ) { + if (!organizationSchema || !isMissing(params.organizationSlug)) { return params; } const isRequired = isOrganizationSlugRequired(organizationSchema); + // A Sentry URL can stand in for an optional slug. A tool that requires the + // slug never reads it from a URL, and its `url` may be an external one + // (such as an uptime monitor's target), so only optional slugs check it. + if (!isRequired && getTargetOrganizationSlug(params) !== null) { + return params; + } let reachable: ReachableOrganizations; try {