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..1d4a299e5 --- /dev/null +++ b/packages/mcp-core/src/internal/tool-helpers/default-organization.test.ts @@ -0,0 +1,331 @@ +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("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"), + 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("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 }) => + 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..391c0d039 --- /dev/null +++ b/packages/mcp-core/src/internal/tool-helpers/default-organization.ts @@ -0,0 +1,302 @@ +/** + * 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; + /** A login's listing failed, so the list may miss organizations. */ + isPartial: 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)]; + // mapCredentialContexts skips a login whose listing fails. + const isPartial = accounts !== null && pages.length < accounts.length; + + 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), + isPartial, + }; +} + +/** + * The organizations the context's login reaches, listed once per context, + * credential, and host for {@link ORGANIZATION_LIST_TTL_MS}. A failed or + * partial 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); + const forget = () => { + if (organizationsCache.get(context) === entry) { + organizationsCache.delete(context); + } + }; + pending.then(({ isPartial }) => isPartial && forget(), forget); + 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 = [ + 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.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.", + ); + } + return lines.join("\n"); +} + +/** + * 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 + * 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)) { + 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 { + 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; + // 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, + }; + if ( + only.regionUrl && + "regionUrl" in tool.inputSchema && + isMissing(params.regionUrl) + ) { + defaulted.regionUrl = only.regionUrl; + } + return defaulted; + } + + 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.", + ); + } + 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,