diff --git a/docs/architecture/overview.md b/docs/architecture/overview.md index aba641c8f..e9882feb6 100644 --- a/docs/architecture/overview.md +++ b/docs/architecture/overview.md @@ -271,6 +271,18 @@ The same availability filters (skills, constraints, experimental mode, and required capabilities) apply before either direct registration or catalog execution. +`sentry mcp --commands` (a preview) serves CLI commands instead of the +catalog, through `packages/mcp-core/src/command-surface/`: every `exposed` +command in the generated `command-catalog.json` is its own tool, named from +its path (`sentry issue list` → `sentry_issue_list`), with the catalog's +brief, examples, and parameter schema, and hints from its permission class. +There is no search or execute meta-tool; harnesses search the tool list +themselves. A call is checked against the command's schema and the +session's constraints before a runner executes it. The stdio runner +(`packages/cli/src/lib/mcp/command-surface.ts`) runs calls through +`createSentrySDK().run()`; the hosted server will pass its own. +`--read-only` drops the write tools. + ### 6. Error Handling Philosophy Two-tier error system: diff --git a/docs/operations/stdio-auth.md b/docs/operations/stdio-auth.md index 3db6af964..27bae5869 100644 --- a/docs/operations/stdio-auth.md +++ b/docs/operations/stdio-auth.md @@ -131,7 +131,9 @@ until the server restarts. Routing is off when the credential is a static token (`SENTRY_AUTH_TOKEN`, `--access-token`, `SENTRY_ACCESS_TOKEN`): that token is the only account. `--organization-slug` (or an `/` target) picks the account that knows the -organization once, at startup, and every call uses it. To use accounts on +organization once, at startup, and every call uses it. `--commands` does not +route calls either: it uses the account that knows its `/` target, else +the startup account. To use accounts on different servers at once (say, sentry.io and a self-hosted server), configure one MCP server per host via `SENTRY_HOST`. diff --git a/docs/operations/token-cost-tracking.md b/docs/operations/token-cost-tracking.md index 3c043d4b9..eb5c4cab9 100644 --- a/docs/operations/token-cost-tracking.md +++ b/docs/operations/token-cost-tracking.md @@ -43,6 +43,13 @@ Per-Tool Breakdown: ... ``` +**Measure the command surface** (`sentry mcp --commands`: one tool per +exposed CLI command) instead of the tool catalog: + +```bash +pnpm run measure-tokens -- --commands +``` + **Write JSON to file:** ```bash diff --git a/packages/cli/plugins/sentry-cli/skills/sentry-cli/references/mcp.md b/packages/cli/plugins/sentry-cli/skills/sentry-cli/references/mcp.md index 8fd4bdb9b..bbc1f8f7b 100644 --- a/packages/cli/plugins/sentry-cli/skills/sentry-cli/references/mcp.md +++ b/packages/cli/plugins/sentry-cli/skills/sentry-cli/references/mcp.md @@ -29,6 +29,8 @@ Start a local Sentry MCP server - `--all-skills - Grant every active skill, including seer on self-hosted` - `--disable-skills - Skills to remove, comma-separated` - `--experimental - Enable forward-looking tool variants and experimental features` +- `--commands - Serve each CLI command as its own tool (sentry_issue_list, ...) instead of the tool catalog (preview)` +- `--read-only - With --commands: list no write tools and refuse write calls` - `--agent-provider - LLM provider for AI-powered search: openai, azure-openai, anthropic, or openrouter` - `--openai-base-url - OpenAI API base URL` - `--openai-model - OpenAI model (default: gpt-5)` @@ -54,6 +56,9 @@ sentry mcp --all-skills # Grant only some skills sentry mcp --skills inspect,triage +# Serve CLI commands instead of the tool catalog, read-only +sentry mcp --commands --read-only + # Pick the LLM provider for AI-powered search sentry mcp --agent-provider anthropic ``` diff --git a/packages/cli/src/commands/mcp.ts b/packages/cli/src/commands/mcp.ts index 4785819ab..166eba5f2 100644 --- a/packages/cli/src/commands/mcp.ts +++ b/packages/cli/src/commands/mcp.ts @@ -115,6 +115,11 @@ export const mcpCommand = buildCommand({ description: "Grant only some skills", command: "sentry mcp --skills inspect,triage", }, + { + description: + "Serve CLI commands instead of the tool catalog, read-only", + command: "sentry mcp --commands --read-only", + }, { description: "Pick the LLM provider for AI-powered search", command: "sentry mcp --agent-provider anthropic", @@ -165,6 +170,12 @@ export const mcpCommand = buildCommand({ experimental: booleanFlag( "Enable forward-looking tool variants and experimental features", ), + commands: booleanFlag( + "Serve each CLI command as its own tool (sentry_issue_list, ...) instead of the tool catalog (preview)", + ), + "read-only": booleanFlag( + "With --commands: list no write tools and refuse write calls", + ), "agent-provider": stringFlag( "LLM provider for AI-powered search: openai, azure-openai, anthropic, or openrouter", ), diff --git a/packages/cli/src/lib/command-access.ts b/packages/cli/src/lib/command-access.ts index edbd54213..f93254a1a 100644 --- a/packages/cli/src/lib/command-access.ts +++ b/packages/cli/src/lib/command-access.ts @@ -1,8 +1,8 @@ /** * What a command may do (permission class) and where agents may run it - * (exposure). `sentry mcp` lists and runs commands by these fields: - * `execute_sentry_read` runs only read commands, and hosts allowlist by - * permission class (#1254). `check:command-access` keeps them honest. + * (exposure). `sentry mcp` lists and runs commands by these fields: its + * command tools carry hints from the permission class, which hosts allowlist + * by (#1254). `check:command-access` keeps them honest. */ /** diff --git a/packages/cli/src/lib/mcp.ts b/packages/cli/src/lib/mcp.ts index bd2a6af9a..1f8796f37 100644 --- a/packages/cli/src/lib/mcp.ts +++ b/packages/cli/src/lib/mcp.ts @@ -440,6 +440,7 @@ export async function serveMcp( const { token, pin } = await withEnv(credentialEnv, () => resolveMcpCredentialWithLogin(config), ); + // A static token (`--access-token`, `SENTRY_AUTH_TOKEN`) is the only // credential; a stored login can route calls to the other stored logins. const router = @@ -447,6 +448,23 @@ export async function serveMcp( ? createRouter(config, credentialEnv, pin) : undefined; + if (config.commands) { + // One account for the session: the stored login that knows + // `--organization-slug`, else the startup account. Calls are not routed + // between logins, and the router refreshes a token near expiry. + const target = { organizationSlug: config.organizationSlug ?? null }; + const getToken = router + ? async () => (await router.resolve(target)).accessToken + : async () => token; + const { createCliCommandTools } = await import("./mcp/command-surface.js"); + await serveMcpStdio(config, { + accessToken: await getToken(), + env, + tools: createCliCommandTools({ config, cwd, getToken }), + }); + return; + } + if (router && !config.organizationSlug) { await serveMcpStdio(config, { accessToken: token, diff --git a/packages/cli/src/lib/mcp/args.ts b/packages/cli/src/lib/mcp/args.ts index 353b5691c..a70073fd0 100644 --- a/packages/cli/src/lib/mcp/args.ts +++ b/packages/cli/src/lib/mcp/args.ts @@ -31,6 +31,10 @@ export type McpFlags = { readonly "all-skills": boolean; readonly "disable-skills"?: string; readonly experimental: boolean; + /** Serve CLI commands instead of the tool catalog. */ + readonly commands: boolean; + /** With `--commands`: no write tool, and write calls are refused. */ + readonly "read-only": boolean; readonly org?: string; readonly project?: string; /** Deprecated alias of `--org`. */ @@ -193,6 +197,8 @@ export type McpMergedArgs = { allSkills: boolean; disableSkills?: string; experimental: boolean; + commands: boolean; + readOnly: boolean; organizationSlug?: string; projectSlug?: string; }; @@ -248,6 +254,8 @@ export function mergeMcpArgs( allSkills: flags["all-skills"], disableSkills: flags["disable-skills"] ?? env.disableSkills, experimental: flags.experimental, + commands: flags.commands, + readOnly: flags["read-only"], organizationSlug: scopeFlag(flags, "org", "organization-slug", scope?.org), projectSlug: scopeFlag(flags, "project", "project-slug", scope?.project), }; diff --git a/packages/cli/src/lib/mcp/command-surface.ts b/packages/cli/src/lib/mcp/command-surface.ts new file mode 100644 index 000000000..45e9472f7 --- /dev/null +++ b/packages/cli/src/lib/mcp/command-surface.ts @@ -0,0 +1,66 @@ +/** + * `sentry mcp --commands`: serve each exposed CLI command as its own MCP tool + * through mcp-core's command surface. + * + * The tools come from the generated command catalog, and each call runs + * through `createSentrySDK().run()`, which loads only the top-level route the + * call runs, never the whole command tree. + * + * Loaded only in commands mode. `@sentry/mcp-server` does not bundle command + * handlers, so its bundle replaces this module with a stub. + */ + +import { + type CommandCall, + CommandFailedError, + createCommandTools, + type ToolRegistry, + toCommandArgs, +} from "@sentry/mcp-core/command-surface"; +import { createSentrySDK, SentryError } from "../../index.js"; +import type { McpServerConfig } from "./config.js"; + +export type CommandSurfaceSetup = { + config: McpServerConfig; + /** Where commands run, for DSN detection and project roots. */ + cwd: string; + /** The session's current token, refreshed as needed. */ + getToken: () => Promise; +}; + +/** Run one call through the CLI SDK, with JSON output and no telemetry of its own. */ +async function runCall( + setup: CommandSurfaceSetup, + call: CommandCall, +): Promise { + const { config } = setup; + const sdk = createSentrySDK({ + token: await setup.getToken(), + url: `${config.sentryProtocol}://${config.sentryHost}`, + org: call.org, + project: call.project, + cwd: setup.cwd, + // The MCP server owns the process's Sentry client. + telemetry: false, + }); + try { + // `run()` is typed for streaming flags too; the command surface refuses them. + return await (sdk.run(...toCommandArgs(call)) as Promise); + } catch (error) { + if (error instanceof SentryError) { + throw new CommandFailedError(call.path.join(" "), error.message); + } + throw error; + } +} + +/** The command surface's tools, running calls through the CLI SDK. */ +export function createCliCommandTools( + setup: CommandSurfaceSetup, +): ToolRegistry { + return createCommandTools({ + runtime: "stdio", + readOnly: setup.config.readOnly, + run: (call) => runCall(setup, call), + }); +} diff --git a/packages/cli/src/lib/mcp/config.ts b/packages/cli/src/lib/mcp/config.ts index 5baa7b0b2..c2976b0ec 100644 --- a/packages/cli/src/lib/mcp/config.ts +++ b/packages/cli/src/lib/mcp/config.ts @@ -40,6 +40,10 @@ export type McpServerConfig = { agentProvider?: McpAgentProvider; agentProviderFromFlag: boolean; experimental: boolean; + /** Serve each CLI command as its own tool instead of the tool catalog. */ + commands: boolean; + /** With `commands`: list no write tools, and refuse write calls. */ + readOnly: boolean; /** Skills granted for this session. */ finalSkills: Set; organizationSlug?: string; @@ -173,6 +177,11 @@ export function finalizeMcpConfig(input: McpMergedArgs): McpServerConfig { ); } const finalSkills = resolveSkills(input, sentryHost); + if (input.readOnly && !input.commands) { + throw new ValidationError( + "--read-only works only with --commands. To limit the tool catalog, pass --skills.", + ); + } return { sentryHost, @@ -192,6 +201,8 @@ export function finalizeMcpConfig(input: McpMergedArgs): McpServerConfig { agentProvider: resolveAgentProvider(input.agentProvider), agentProviderFromFlag: input.agentProviderFromFlag, experimental: input.experimental, + commands: input.commands, + readOnly: input.readOnly, finalSkills, organizationSlug: input.organizationSlug, projectSlug: input.projectSlug, diff --git a/packages/cli/src/lib/mcp/server.ts b/packages/cli/src/lib/mcp/server.ts index 92077830b..968aabd52 100644 --- a/packages/cli/src/lib/mcp/server.ts +++ b/packages/cli/src/lib/mcp/server.ts @@ -18,6 +18,7 @@ import { import { buildServer } from "@sentry/mcp-core/server"; import { sentryBeforeSend } from "@sentry/mcp-core/telem/sentry"; import { startStdio } from "@sentry/mcp-core/transports/stdio"; +import type { ToolRegistry } from "@sentry/mcp-core/command-surface"; import type { CredentialRouter, ServerContext } from "@sentry/mcp-core/types"; import { setServerVersion } from "@sentry/mcp-core/version"; import { @@ -59,6 +60,8 @@ export type McpServeOptions = { */ credentialRouter?: CredentialRouter; env: NodeJS.ProcessEnv; + /** Tools to serve instead of the tool catalog (`--commands`). */ + tools?: ToolRegistry; }; const SENTRY_FLUSH_TIMEOUT_MS = 5000; @@ -426,6 +429,7 @@ export async function serveMcpStdio( const server = buildServer({ context, experimentalMode: config.experimental, + tools: options.tools, }); const connection = await startStdio(server, context).catch( (error: unknown) => { diff --git a/packages/cli/src/lib/sdk-invoke.ts b/packages/cli/src/lib/sdk-invoke.ts index 4f88c9982..7783065ba 100644 --- a/packages/cli/src/lib/sdk-invoke.ts +++ b/packages/cli/src/lib/sdk-invoke.ts @@ -201,8 +201,29 @@ export function applyFlagDefaults( // oxlint-disable-next-line no-control-regex -- ANSI escape sequences use ESC (0x1b) const ANSI_RE = /\x1b\[[0-9;]*m/g; +/** + * Run `executor` in a telemetry span, or without one when the caller turned + * telemetry off (`SentryOptions.telemetry`). + */ +async function withSdkTelemetry( + options: SentryOptions | undefined, + executor: (span: Span | undefined) => Promise, +): Promise { + if (options?.telemetry === false) { + await executor(undefined); + return; + } + const { withTelemetry } = await import("./telemetry.js"); + await withTelemetry(executor, { libraryMode: true }); +} + /** Flush Sentry telemetry (no beforeExit handler in library mode). */ -async function flushTelemetry(): Promise { +async function flushTelemetry( + options: SentryOptions | undefined, +): Promise { + if (options?.telemetry === false) { + return; + } // oxlint-disable-next-line sentry-cli/no-silent-catch -- grandfathered silent catch — see #1531; drain by adding log.debug()/log.warn() or re-throwing. try { const Sentry = await import("@sentry/node"); @@ -299,7 +320,8 @@ type CaptureContext = { stdin: NodeJS.ReadStream & { fd: 0 }; env: NodeJS.ProcessEnv; cwd: () => string; - exitCode: number; + /** Unset until something fails, as on Node's own `process`. */ + exitCode: number | undefined; }; stdout: Writer; stderr: Writer; @@ -361,7 +383,9 @@ async function buildCaptureContext( stdin: process.stdin, env, cwd: () => cwd, - exitCode: 0, + // Unset, as on Node's `process`: Stricli's `run()` records a failure + // with `exitCode ??= code`, which a 0 here would swallow. + exitCode: undefined, }; if (opts?.abortSignal) { @@ -414,14 +438,14 @@ async function executeWithCapture( const { withCustomHeadersOverride } = await import("./custom-headers.js"); return await withCustomHeadersOverride(options?.headers, async () => { const captureCtx = await buildCaptureContext(env, cwd); - const { withTelemetry } = await import("./telemetry.js"); try { - await withTelemetry(async (span) => await executor(captureCtx, span), { - libraryMode: true, - }); + await withSdkTelemetry( + options, + async (span) => await executor(captureCtx, span), + ); } catch (thrown) { - await flushTelemetry(); + await flushTelemetry(options); // OutputError: data was already rendered (captured) before the throw. // Return it despite the non-zero exit code — this is the "HTTP 404 body" @@ -436,14 +460,12 @@ async function executeWithCapture( throw buildSdkError(captureCtx.stderrChunks, exitCode, thrown); } - await flushTelemetry(); + await flushTelemetry(options); // Check exit code (Stricli sets it without throwing for some errors) - if (captureCtx.context.process.exitCode !== 0) { - throw buildSdkError( - captureCtx.stderrChunks, - captureCtx.context.process.exitCode, - ); + const exitCode = captureCtx.context.process.exitCode ?? 0; + if (exitCode !== 0) { + throw buildSdkError(captureCtx.stderrChunks, exitCode); } return parseOutput( @@ -495,26 +517,21 @@ function executeWithStream( try { const { withCustomHeadersOverride } = await import("./custom-headers.js"); await withCustomHeadersOverride(options?.headers, async () => { - captureCtx = await buildCaptureContext(env, cwd, { + const ctx = await buildCaptureContext(env, cwd, { channel: channel as AsyncChannel, abortSignal: controller.signal, }); + captureCtx = ctx; - const { withTelemetry } = await import("./telemetry.js"); - - // oxlint-disable-next-line typescript/no-non-null-assertion -- captureCtx is assigned on the line above - await withTelemetry(async (span) => await executor(captureCtx!, span), { - libraryMode: true, - }); + await withSdkTelemetry( + options, + async (span) => await executor(ctx, span), + ); // Check exit code — Stricli sets it without throwing for some errors - if (captureCtx.context.process.exitCode !== 0) { - channel.error( - buildSdkError( - captureCtx.stderrChunks, - captureCtx.context.process.exitCode, - ), - ); + const exitCode = captureCtx.context.process.exitCode ?? 0; + if (exitCode !== 0) { + channel.error(buildSdkError(captureCtx.stderrChunks, exitCode)); } else { // Drain any raw stdout the command wrote directly (via stdout.write) // instead of yielding via captureObject — e.g. a binary Uint8Array @@ -539,7 +556,7 @@ function executeWithStream( : buildSdkError(stderrChunks, exitCode, thrown); channel.error(err); } finally { - await flushTelemetry(); + await flushTelemetry(options); } }); invocation.catch((error: unknown) => { diff --git a/packages/cli/src/lib/sdk-types.ts b/packages/cli/src/lib/sdk-types.ts index 562aa2c9d..1b9169df0 100644 --- a/packages/cli/src/lib/sdk-types.ts +++ b/packages/cli/src/lib/sdk-types.ts @@ -58,6 +58,12 @@ export type SentryOptions = { */ headers?: Record; + /** + * Set `false` to run commands without the CLI's telemetry. The CLI's + * telemetry initializes `@sentry/node` for each call, which replaces a + * Sentry client the host application set up (the MCP server's). + */ + telemetry?: boolean; /** * Abort signal for cancelling streaming commands. * When aborted, streaming iterables stop on the next poll cycle. diff --git a/packages/cli/test/lib/mcp-start.test.ts b/packages/cli/test/lib/mcp-start.test.ts index cf3c9240f..39f105b37 100644 --- a/packages/cli/test/lib/mcp-start.test.ts +++ b/packages/cli/test/lib/mcp-start.test.ts @@ -521,6 +521,15 @@ describe("serveMcp account routing", () => { ); expect(refreshedWith).toEqual(["b-token-refresh"]); }); + + test("--commands serves the account that knows the session's org", async () => { + loginTwoAccounts(); + const { options } = await start(["--commands", "b-org/"]); + + expect(options.credentialRouter).toBeUndefined(); + expect(options.accessToken).toBe("b-token"); + expect(options.tools).toBeDefined(); + }); }); describe("serveMcp deprecated --access-token", () => { diff --git a/packages/cli/test/lib/mcp/args.test.ts b/packages/cli/test/lib/mcp/args.test.ts index 68104d16b..959045a66 100644 --- a/packages/cli/test/lib/mcp/args.test.ts +++ b/packages/cli/test/lib/mcp/args.test.ts @@ -209,6 +209,16 @@ describe("@sentry/mcp-server flag parity", () => { allSkills: true, disableSkills: "seer", experimental: true, + commands: false, + readOnly: false, + }); + }); + + it("reads --commands and --read-only", async () => { + const flags = await parseMcpFlags(["--commands", "--read-only"]); + expect(mergeMcpArgs(flags, {})).toMatchObject({ + commands: true, + readOnly: true, }); }); diff --git a/packages/cli/test/lib/mcp/command-surface.test.ts b/packages/cli/test/lib/mcp/command-surface.test.ts new file mode 100644 index 000000000..cafbf6d05 --- /dev/null +++ b/packages/cli/test/lib/mcp/command-surface.test.ts @@ -0,0 +1,78 @@ +/** + * `sentry mcp --commands` runs command tool calls through the CLI SDK. These run + * real commands (`sentry api --dry-run` and argument errors), so no request + * leaves the process. + */ + +import type { ServerContext } from "@sentry/mcp-core/types"; +import { describe, expect, test, vi } from "vitest"; +import { createCliCommandTools } from "../../../src/lib/mcp/command-surface.js"; +import type { McpServerConfig } from "../../../src/lib/mcp/config.js"; +import { useTestConfigDir } from "../../helpers.js"; + +// The SDK loads only the route a call runs (`sdk-app.ts`); the full command +// tree must never load for a command tool call. +vi.mock("../../../src/app.js", () => { + throw new Error("a command tool call loaded the full command tree (app.ts)"); +}); + +const config = { + sentryHost: "sentry.io", + sentryProtocol: "https", + readOnly: false, +} as McpServerConfig; + +const context: ServerContext = { accessToken: "unused", constraints: {} }; + +function tools() { + return createCliCommandTools({ + config, + cwd: process.cwd(), + getToken: async () => `sntryu_${"a".repeat(64)}`, + }); +} + +/** Call the `sentry api` tool. */ +async function api(params: Record): Promise { + const tool = tools().sentry_api; + return JSON.parse((await tool?.handler(params as never, context)) as string); +} + +describe("createCliCommandTools", () => { + useTestConfigDir("mcp-command-surface-"); + + test("runs a write call through the SDK with parsed flags", async () => { + expect( + await api({ + positional: ["organizations/acme/releases/"], + flags: { method: "POST", "dry-run": true, field: ["version=1.0.0"] }, + }), + ).toEqual({ + command: "api", + result: { + method: "POST", + url: "https://sentry.io/api/0/organizations/acme/releases/", + headers: { "content-type": "application/json" }, + body: { version: "1.0.0" }, + }, + }); + }); + + test("runs a read call, with a boolean flag turned off", async () => { + expect( + await api({ + positional: ["organizations/"], + flags: { "dry-run": true, verbose: false }, + }), + ).toMatchObject({ result: { method: "GET", body: null } }); + }); + + test("reports a failing command with its own message", async () => { + await expect( + api({ + positional: ["organizations/"], + flags: { method: "BREW", "dry-run": true }, + }), + ).rejects.toThrow(/`api` failed: .*BREW/); + }); +}); diff --git a/packages/cli/test/lib/mcp/config.test.ts b/packages/cli/test/lib/mcp/config.test.ts index 8a7615c54..a4c862f1c 100644 --- a/packages/cli/test/lib/mcp/config.test.ts +++ b/packages/cli/test/lib/mcp/config.test.ts @@ -333,4 +333,16 @@ describe("finalizeMcpConfig", () => { projectSlug: "web", }); }); + it("keeps --commands and --read-only", () => { + expect(finalize({ commands: true, readOnly: true })).toMatchObject({ + commands: true, + readOnly: true, + }); + }); + + it("rejects --read-only without --commands", () => { + expect(() => finalize({ readOnly: true })).toThrow( + "--read-only works only with --commands", + ); + }); }); diff --git a/packages/cli/test/lib/route-registry.test.ts b/packages/cli/test/lib/route-registry.test.ts index 41d4a1ad7..ef32941fb 100644 --- a/packages/cli/test/lib/route-registry.test.ts +++ b/packages/cli/test/lib/route-registry.test.ts @@ -18,6 +18,12 @@ const SRC = join(import.meta.dirname, "../../src"); const APP = join(SRC, "app.ts"); /** Library-mode runner: executes the full application, reached only from `src/index.ts`. */ const SDK_RUNNER = join(SRC, "lib/sdk-invoke.ts"); +/** + * The SDK entry. `sentry mcp --commands` runs catalog commands through it; + * the runner loads `app.ts` only for `help` and unknown names, so the check + * stops at the SDK as it does at the runner. + */ +const SDK_ENTRY = join(SRC, "index.ts"); const ANY_IMPORT = /.*/; const RELATIVE_IMPORT = /^\.\.?\//; @@ -122,6 +128,9 @@ describe("route registry", () => { JS_EXTENSION, ".ts", ); + if (target === SDK_ENTRY) { + return { path: args.path, external: true }; + } if (target === APP) { return { errors: [ diff --git a/packages/cli/test/lib/sdk-invoke.test.ts b/packages/cli/test/lib/sdk-invoke.test.ts index d8fcfc52f..179e27e81 100644 --- a/packages/cli/test/lib/sdk-invoke.test.ts +++ b/packages/cli/test/lib/sdk-invoke.test.ts @@ -9,6 +9,7 @@ import { describe, expect, test } from "vitest"; import { applyFlagDefaults, buildInvoker, + buildRunner, type FlagDef, parseOutput, } from "../../src/lib/sdk-invoke.js"; @@ -280,3 +281,13 @@ describe("buildInvoker: lazily parsed positionals", () => { ).rejects.toThrow("sentry project delete /"); }); }); + +describe("buildRunner: failures", () => { + test("rejects with the command's error when Stricli fails without throwing", async () => { + // Stricli's run() records an argument error as `exitCode ??= code`. + const run = buildRunner({ token: "sntrys_test", telemetry: false }); + await expect( + run("org", "list", "--limit=abc") as Promise, + ).rejects.toThrow('Failed to parse "abc" for limit'); + }); +}); diff --git a/packages/mcp-core/package.json b/packages/mcp-core/package.json index d64b1bdea..6f03ca4aa 100644 --- a/packages/mcp-core/package.json +++ b/packages/mcp-core/package.json @@ -33,6 +33,10 @@ "types": "./dist/command-catalog.d.ts", "default": "./dist/command-catalog.js" }, + "./command-surface": { + "types": "./dist/command-surface/index.d.ts", + "default": "./dist/command-surface/index.js" + }, "./constants": { "types": "./dist/constants.d.ts", "default": "./dist/constants.js" diff --git a/packages/mcp-core/scripts/measure-token-cost.ts b/packages/mcp-core/scripts/measure-token-cost.ts index 2807e2f67..97f0b56a3 100644 --- a/packages/mcp-core/scripts/measure-token-cost.ts +++ b/packages/mcp-core/scripts/measure-token-cost.ts @@ -25,10 +25,13 @@ const toolTypesModule = await import("../src/tools/types.ts"); function parseArgs() { const args = process.argv.slice(2); let outputFile: string | null = null; + let commands = false; for (let i = 0; i < args.length; i++) { const arg = args[i]; - if (arg === "--output" || arg === "-o") { + if (arg === "--commands") { + commands = true; + } else if (arg === "--output" || arg === "-o") { outputFile = args[i + 1]; if (!outputFile) { throw new Error("--output requires a file path"); @@ -39,6 +42,7 @@ function parseArgs() { Usage: tsx measure-token-cost.ts [options] Options: + --commands Measure the command surface (sentry mcp --commands) -o, --output Write JSON output to file -h, --help Show this help message @@ -50,7 +54,7 @@ Examples: } } - return { outputFile }; + return { outputFile, commands }; } type ToolDefinition = { @@ -152,7 +156,7 @@ async function main() { let encoder: Tiktoken | null = null; try { - const { outputFile } = parseArgs(); + const { outputFile, commands } = parseArgs(); // Load tools const toolsDefault = toolsModule.default as @@ -162,13 +166,18 @@ async function main() { throw new Error("Failed to import tools from src/tools/index.ts"); } - const toolsToMeasure = Object.fromEntries( - Object.entries(toolsDefault).filter( - ([, tool]) => - surfacesModule.isTopLevelTool(tool, false) && - toolTypesModule.isToolVisibleInMode(tool, false), - ), - ); + const toolsToMeasure = commands + ? ((await import("../src/command-surface/index.ts")).createCommandTools({ + runtime: "stdio", + run: async () => undefined, + }) as Record) + : Object.fromEntries( + Object.entries(toolsDefault).filter( + ([, tool]) => + surfacesModule.isTopLevelTool(tool, false) && + toolTypesModule.isToolVisibleInMode(tool, false), + ), + ); // Format as MCP would send them (as a complete tools array) const mcpTools = formatToolsForMCP(toolsToMeasure); diff --git a/packages/mcp-core/src/command-surface/args.ts b/packages/mcp-core/src/command-surface/args.ts new file mode 100644 index 000000000..2baef76e5 --- /dev/null +++ b/packages/mcp-core/src/command-surface/args.ts @@ -0,0 +1,43 @@ +/** + * Turn a validated call into CLI arguments for the SDK's `run()`. + * + * `run()` parses them with the command's own Stricli parsers, the same as a + * terminal invocation, so flag values reach the handler parsed (`--period 7d` + * becomes a time range) and positionals go through their parsers too. + */ + +import type { CommandCall } from "./types"; + +type FlagValue = CommandCall["flags"][string]; + +/** + * `--name=value` keeps a value that starts with `-` (`--sort=-age`) from + * reading as a flag. Stricli does not read the joined form for an empty value + * or one with a line break, so those are passed as two arguments. + */ +function flagArgs(name: string, value: FlagValue): string[] { + if (Array.isArray(value)) { + return value.flatMap((item) => flagArgs(name, item)); + } + const text = String(value); + return text === "" || /[\r\n]/.test(text) + ? [`--${name}`, text] + : [`--${name}=${text}`]; +} + +/** + * The arguments for `call`: the command path, its flags, `--json`, and the + * positionals after `--`, so a positional that starts with `-` is not read + * as a flag. + */ +export function toCommandArgs(call: CommandCall): string[] { + const flags = Object.entries(call.flags).flatMap(([name, value]) => { + if (typeof value === "boolean") { + return [`--${name}=${value}`]; + } + return flagArgs(name, value); + }); + const positional = + call.positional.length > 0 ? ["--", ...call.positional] : []; + return [...call.path, ...flags, "--json", ...positional]; +} diff --git a/packages/mcp-core/src/command-surface/commands.ts b/packages/mcp-core/src/command-surface/commands.ts new file mode 100644 index 000000000..1b4fb3517 --- /dev/null +++ b/packages/mcp-core/src/command-surface/commands.ts @@ -0,0 +1,155 @@ +/** + * Command names and the per-call rules that do not depend on the session: + * which flags agents may pass, the permission class of a call, and the + * skills that grant a command. + */ + +import type { CommandCatalogEntry, JsonSchema } from "../command-catalog"; +import { ALL_SKILLS, type Skill } from "../skills"; + +/** Where the server runs: the hosted one keeps no CLI state between calls. */ +export type CommandRuntime = "stdio" | "hosted"; + +/** What one call may do to Sentry. */ +type CallPermission = "read" | "write" | "destructive"; + +/** + * Flags the executor sets itself: it always asks for JSON. Callers that pass + * it are ignored rather than refused. + */ +const INJECTED_FLAGS = new Set(["json"]); + +/** Flags that turn a command into a stream with no end. */ +const STREAMING_FLAGS = new Set(["follow", "refresh", "watch"]); + +/** Flags that open a browser on the machine that runs the command. */ +const BROWSER_FLAGS = new Set(["web"]); + +/** + * Flags that make the command fetch a host the caller picks, by command + * (`sentry status show --url`). Agents could probe internal hosts with them. + */ +const HOST_FLAGS: Record = { + "sentry status show": ["url"], +}; + +/** + * The skills that grant a write or destructive command, by command name + * prefix; other writes need `project-management`. + */ +const WRITE_SKILLS: [prefix: string, skill: Skill][] = [ + ["issue explain", "seer"], + ["issue plan", "seer"], + ["issue ", "triage"], + ["feedback ", "triage"], +]; + +/** Skills that grant writes through `sentry api`. */ +export const API_WRITE_SKILLS: readonly Skill[] = [ + "triage", + "project-management", +]; + +/** HTTP methods that only read. */ +const READ_METHODS = new Set(["GET", "HEAD", "OPTIONS"]); + +/** The command name without `sentry ` (`issue list`). */ +export function commandName(entry: CommandCatalogEntry): string { + return entry.path.replace(/^sentry /, ""); +} + +/** Path segments for the SDK (`["issue", "list"]`). */ +export function commandSegments(entry: CommandCatalogEntry): string[] { + return commandName(entry).split(" "); +} + +/** Why a flag cannot be passed through the MCP, or `undefined` when it can. */ +export function refusedFlagReason( + entry: CommandCatalogEntry, + flag: string, +): string | undefined { + if (entry.localPathFlags?.includes(flag)) { + return "it reads or writes a path on the local machine"; + } + if (STREAMING_FLAGS.has(flag)) { + return "it streams output with no end"; + } + if (BROWSER_FLAGS.has(flag)) { + return "it opens a browser"; + } + if (HOST_FLAGS[entry.path]?.includes(flag)) { + return "it fetches a host the caller picks"; + } + return; +} + +/** + * The skills that grant `entry`: every skill for reads and `sentry api` + * (whose writes need {@link API_WRITE_SKILLS}), one write skill otherwise. + */ +export function commandSkills(entry: CommandCatalogEntry): Skill[] { + if (entry.permission === "read" || entry.permission === "method") { + return ALL_SKILLS; + } + const name = `${commandName(entry)} `; + const match = WRITE_SKILLS.find(([prefix]) => name.startsWith(prefix)); + return [match ? match[1] : "project-management"]; +} + +/** Whether the executor drops `flag` because it sets the flag itself. */ +export function isInjectedFlag(flag: string): boolean { + return INJECTED_FLAGS.has(flag); +} + +/** `entry`'s flag schemas, by flag name. */ +function flagSchemas(entry: CommandCatalogEntry): Record { + return entry.parameters.properties?.flags?.properties ?? {}; +} + +/** + * The parameters schema agents see and calls are checked against: without + * the injected flags and the flags the MCP refuses. + */ +export function agentParameters(entry: CommandCatalogEntry): JsonSchema { + const { parameters } = entry; + const flags = parameters.properties?.flags; + if (!flags?.properties) { + return parameters; + } + const kept = Object.fromEntries( + Object.entries(flagSchemas(entry)).filter( + ([name]) => !(isInjectedFlag(name) || refusedFlagReason(entry, name)), + ), + ); + return { + ...parameters, + properties: { + ...parameters.properties, + flags: { ...flags, properties: kept }, + }, + }; +} + +/** + * The permission class of a call. A `method` command (`sentry api`) takes + * the class of the HTTP method it sends: reads for GET, HEAD, and OPTIONS, + * destructive for DELETE, and writes otherwise. + */ +export function callPermission( + entry: CommandCatalogEntry, + flags: Record, +): CallPermission { + if (entry.permission !== "method") { + return entry.permission; + } + const declared = flagSchemas(entry).method?.default; + const raw = flags.method ?? declared ?? "GET"; + if (typeof raw !== "string") { + return "write"; + } + const method = raw.trim().toUpperCase(); + if (READ_METHODS.has(method)) { + return "read"; + } + return method === "DELETE" ? "destructive" : "write"; +} diff --git a/packages/mcp-core/src/command-surface/constraints.ts b/packages/mcp-core/src/command-surface/constraints.ts new file mode 100644 index 000000000..ccab8d97e --- /dev/null +++ b/packages/mcp-core/src/command-surface/constraints.ts @@ -0,0 +1,371 @@ +/** + * Session constraints (`sentry mcp /[]`, hosted URL + * constraints) applied to command calls. + * + * The runner passes the constrained organization and project to the SDK as + * the default target (`SENTRY_ORG`/`SENTRY_PROJECT`), which every command + * uses when a call names no target. This module checks the targets a call + * does name and refuses one outside the session, or rewrites a bare project + * into the session's organization. Opaque identifiers (numeric issue IDs, + * short IDs) carry no organization to check here. + */ + +import { UserInputError } from "../errors"; +import type { CommandCatalogEntry, JsonSchema } from "../command-catalog"; +import type { Constraints } from "../types"; +import { commandName } from "./commands"; + +/** Completion kinds whose positional names a target. */ +const TARGET_KINDS = new Set(["org", "org/project", "dashboard", "issue"]); + +export const URL_PATTERN = /^https?:\/\//i; + +type Session = { org: string; project: string | null }; + +function outside( + entry: CommandCatalogEntry, + value: string, + session: Session, +): UserInputError { + const scope = session.project + ? `${session.org}/${session.project}` + : `${session.org}/`; + return new UserInputError( + `\`${commandName(entry)}\` was called with ${JSON.stringify(value)}, which is outside this session's scope (${scope}). Name a target in ${scope}, or leave the target out to use it.`, + ); +} + +/** + * The organization and project a Sentry web URL names, read the way the + * CLI's URL parser reads them: the org from `/organizations//`, + * `/settings//`, or an `.sentry.io` subdomain; the project from + * `.../projects//` or a feedback link's `feedbackSlug=:`. + */ +function urlTarget(value: string): { org?: string; project?: string } { + let url: URL; + try { + url = new URL(value); + } catch { + return {}; + } + const segments = url.pathname.split("/").filter(Boolean); + const [subdomain, ...rest] = url.hostname.split("."); + // `acme.sentry.io`, but not `sentry.io`, a region host (`us.sentry.io`), + // or a self-hosted host. + const subdomainOrg = + rest.join(".") === "sentry.io" && subdomain && subdomain.length > 2 + ? subdomain + : undefined; + const pathOrg = + (segments[0] === "organizations" || + (segments[0] === "settings" && segments[1] !== "projects")) && + segments[1] + ? segments[1] + : undefined; + const projectsAt = segments.indexOf("projects"); + const project = + (projectsAt >= 0 ? segments[projectsAt + 1] : undefined) ?? + url.searchParams.get("feedbackSlug")?.split(":")[0] ?? + undefined; + return { + org: pathOrg ?? subdomainOrg, + ...(project ? { project } : {}), + }; +} + +/** + * Refuse a Sentry URL for another organization, or for another project in a + * project session. + * + * @returns Whether `value` is a URL + */ +function checkUrl( + entry: CommandCatalogEntry, + value: string, + session: Session, +): boolean { + if (!URL_PATTERN.test(value)) { + return false; + } + const target = urlTarget(value); + if ( + (target.org && target.org !== session.org) || + (session.project && target.project && target.project !== session.project) + ) { + throw outside(entry, value, session); + } + return true; +} + +/** The title of positional `index`, such as `org/project/trace-id`. */ +function positionalTitle( + positional: JsonSchema | undefined, + index: number, +): string | undefined { + const prefix = positional?.prefixItems ?? []; + const item = index < prefix.length ? prefix[index] : positional?.items; + return item ? item.title : undefined; +} + +/** + * The completion kind of positional `index`. A variadic positional has one + * kind; only its first word names a target (`[] `), + * except for issues and `[<org>/]<id>` lists, where every word can. + */ +function positionalKind( + entry: CommandCatalogEntry, + index: number, +): string | undefined { + const kinds = entry.completions?.positional ?? []; + if (entry.parameters.properties?.positional?.prefixItems) { + return kinds[index] ?? undefined; + } + const kind = kinds[0] ?? undefined; + return index === 0 || kind === "issue" || kind === "org" ? kind : undefined; +} + +/** + * The `<org>/<project>/<id>` form the CLI reads a GitHub-style + * (`[<org>/]<project>#<id>`) or `<project>:<id>` issue reference as, with + * the session's organization when it names none; else `value`. + */ +function issueSlashForm(value: string, org: string): string { + const hash = value.indexOf("#"); + if (hash > 0) { + const prefix = value.slice(0, hash); + const id = value.slice(hash + 1); + return prefix.includes("/") ? `${prefix}/${id}` : `${org}/${prefix}/${id}`; + } + const colon = /^([^:/]+):([^/]+)$/.exec(value); + return colon ? `${org}/${colon[1]}/${colon[2]}` : value; +} + +/** + * Check one positional that names a target, and rewrite a bare project + * (`web`, a project search across organizations) to `<org>/web`. + * + * @param leading - The first of several words, which the CLI reads as the + * organization in `<org> <id>` + */ +function pinTarget( + entry: CommandCatalogEntry, + value: string, + kind: string, + catalogTitle: string, + leading: boolean, + session: Session, +): string { + if (URL_PATTERN.test(value)) { + return value; + } + if (kind === "issue") { + const slashForm = issueSlashForm(value, session.org); + if (slashForm !== value) { + try { + pinTarget(entry, slashForm, kind, catalogTitle, leading, session); + } catch { + throw outside(entry, value, session); + } + return value; + } + } + // `target` is `<org>/`, `<org>/<project>`, or `<project>`. + const title = catalogTitle === "target" ? "org/project" : catalogTitle; + const segments = value.split("/"); + const titleSegments = title.split("/"); + if (segments.length === 1) { + const orgWord = title === "org" || (leading && titleSegments[0] === "org"); + // `<org> <id>`: the leading word of `agent-conversation view` (`org`) + // or `dashboard view` (`dashboard`) is the organization. + if ( + (kind === "org" || kind === "dashboard") && + orgWord && + value !== session.org + ) { + throw outside(entry, value, session); + } + if (kind === "org/project" && title === "org/project") { + if (session.project && value !== session.project) { + throw outside(entry, value, session); + } + return `${session.org}/${value}`; + } + return value; + } + // `<org>/...`: a target whose first segment is the organization. A + // one-word title (`feedback`, `replay-id-or-url`) takes `<org>/...` too; + // a slashed title not led by `org` (`trace-id/span-id`) doesn't. + if ( + titleSegments[0] === "org" || + titleSegments.length === 1 || + kind === "issue" || + kind === "dashboard" + ) { + if (segments[0] !== session.org) { + throw outside(entry, value, session); + } + // `<org>/` (every project) becomes the session's project. + if ( + session.project && + kind === "org/project" && + title === "org/project" && + value === `${session.org}/` + ) { + return `${session.org}/${session.project}`; + } + // `<org>/<project>/...`, or `<org>/<project>/<suffix>` under a + // one-word title (`feedback`, `issue`). An `org/project` target can stop + // after the project, as `<org>/<project> <trace-id>` or `span list + // <org>/<project>` do under `org/project/trace-id`. + const namesProject = + titleSegments.length === 1 + ? segments.length >= 3 + : (kind === "org/project" && + titleSegments[1]?.startsWith("project") === true) || + (titleSegments[1] === "project" && + segments.length >= titleSegments.length); + const project = segments[1]; + if ( + session.project && + namesProject && + project && + project !== session.project + ) { + throw outside(entry, value, session); + } + } + return value; +} + +/** + * The organization and project a `sentry api` endpoint names, or `undefined` + * for an endpoint not scoped to one (`issues/1/`, `organizations/`). + */ +function apiEndpointTarget( + endpoint: string, +): { org: string; project?: string } | undefined { + // Resolve the path as the request will (`..`, `%2e%2e`), so a dot segment + // cannot leave the organization it names first. + let segments: string[]; + try { + const base = "https://sentry.invalid/api/0/"; + // Absolute URLs are refused before this (`prepareCommandCall`). + const url = new URL(endpoint.replace(/^\/*(?:api\/0(?:\/|$))?/, ""), base); + segments = url.pathname.split("/").filter(Boolean); + } catch { + return; + } + if (segments[0] === "api" && segments[1] === "0") { + segments.splice(0, 2); + } + // The prefixes the CLI reads a target from (`parseOrgProjectFromApiPath`). + const [prefix, org, ...tail] = segments; + if (!org) { + return; + } + if (prefix === "organizations") { + return tail[0] === "projects" && tail[1] + ? { org, project: tail[1] } + : { org }; + } + if (prefix === "projects") { + return tail[0] ? { org, project: tail[0] } : { org }; + } + if (prefix === "teams") { + return { org }; + } + return; +} + +function pinApiEndpoint( + entry: CommandCatalogEntry, + endpoint: string, + session: Session, +): void { + const target = apiEndpointTarget(endpoint); + // A project session takes only endpoints that name its project: an + // organization-wide one (`organizations/<org>/issues/`) spans every project. + if (!target || (session.project && !target.project)) { + const scope = session.project + ? `projects/${session.org}/${session.project}/ or organizations/${session.org}/projects/${session.project}/` + : `organizations/${session.org}/, projects/${session.org}/, or teams/${session.org}/`; + throw new UserInputError( + `\`${commandName(entry)}\` was called with ${JSON.stringify(endpoint)}, which is not scoped to ${session.project ? "a project" : "an organization"}. In this session, use an endpoint under ${scope}.`, + ); + } + if ( + target.org !== session.org || + (session.project && target.project !== session.project) + ) { + throw outside(entry, endpoint, session); + } +} + +/** + * Apply the session's constraints to a validated call. Returns the + * positional arguments to run, rewritten where a bare project needed the + * session's organization. + * + * @throws {UserInputError} When the call names a target outside the session + */ +export function pinConstraints( + entry: CommandCatalogEntry, + positional: string[], + flags: Record<string, unknown>, + constraints: Constraints, +): string[] { + const org = constraints.organizationSlug; + if (!org) { + return positional; + } + const session: Session = { org, project: constraints.projectSlug ?? null }; + + // `--project` (`release create`, `alert metrics create`): one slug, or + // several, repeated or comma-separated. + if (session.project) { + for (const item of [flags.project].flat()) { + if (typeof item !== "string") { + continue; + } + for (const slug of item.split(",")) { + if (slug.trim() && slug.trim() !== session.project) { + throw outside(entry, item, session); + } + } + } + } + for (const value of Object.values(flags)) { + for (const item of [value].flat()) { + if (typeof item === "string") { + checkUrl(entry, item, session); + } + } + } + + if (entry.permission === "method") { + const [endpoint] = positional; + if (endpoint !== undefined) { + pinApiEndpoint(entry, endpoint, session); + } + return positional; + } + + const schema = entry.parameters.properties?.positional; + return positional.map((value, index) => { + if (checkUrl(entry, value, session)) { + return value; + } + const kind = positionalKind(entry, index); + const title = positionalTitle(schema, index); + return kind && title && TARGET_KINDS.has(kind) + ? pinTarget( + entry, + value, + kind, + title, + index === 0 && positional.length > 1, + session, + ) + : value; + }); +} diff --git a/packages/mcp-core/src/command-surface/errors.ts b/packages/mcp-core/src/command-surface/errors.ts new file mode 100644 index 000000000..87b85c497 --- /dev/null +++ b/packages/mcp-core/src/command-surface/errors.ts @@ -0,0 +1,13 @@ +import { UserInputError } from "../errors"; + +/** + * A command ran and failed: a missing resource, a rejected value, an API + * error. Runners throw it with the command's own message, which the agent + * reads to fix the call. Any other error a runner throws is a bug. + */ +export class CommandFailedError extends UserInputError { + constructor(command: string, message: string) { + super(`\`${command}\` failed: ${message}`); + this.name = "CommandFailedError"; + } +} diff --git a/packages/mcp-core/src/command-surface/index.ts b/packages/mcp-core/src/command-surface/index.ts new file mode 100644 index 000000000..64ae5657b --- /dev/null +++ b/packages/mcp-core/src/command-surface/index.ts @@ -0,0 +1,10 @@ +/** + * The command surface: one MCP tool per exposed Sentry CLI command. + * See `tools.ts`. + */ + +export { toCommandArgs } from "./args"; +export { CommandFailedError } from "./errors"; +export { type CommandSurfaceOptions, createCommandTools } from "./tools"; +export type { ToolRegistry } from "../tools/catalog-runtime/availability"; +export type { CommandCall, CommandRunner } from "./types"; diff --git a/packages/mcp-core/src/command-surface/prepare.ts b/packages/mcp-core/src/command-surface/prepare.ts new file mode 100644 index 000000000..de2daf252 --- /dev/null +++ b/packages/mcp-core/src/command-surface/prepare.ts @@ -0,0 +1,204 @@ +/** + * Everything a command tool call is checked against before it runs: + * exposure, refused flags, the parameter schema, read-only sessions, and the + * session's constraints. + */ + +import { UserInputError } from "../errors"; +import type { CommandCatalogEntry } from "../command-catalog"; +import type { ServerContext } from "../types"; +import { + API_WRITE_SKILLS, + agentParameters, + type CommandRuntime, + callPermission, + commandName, + commandSegments, + isInjectedFlag, + refusedFlagReason, +} from "./commands"; +import { pinConstraints, URL_PATTERN } from "./constraints"; +import type { CommandCall } from "./types"; +import { validateJsonSchema } from "./validate"; + +/** + * Pagination keywords that need the CLI's stored cursor stack, with their + * aliases (`last`, `previous`). + */ +const CURSOR_KEYWORDS = new Set(["next", "prev", "last", "previous"]); + +export type SessionPolicy = { + runtime: CommandRuntime; + /** Refuse write and destructive calls (`--read-only`, hosted API Read). */ + readOnly: boolean; +}; + +/** + * Why the session cannot run `entry` at all, or `undefined` when it can. + * Per-call reasons (a refused flag, the method of `sentry api`) are checked + * by {@link prepareCommandCall}. + */ +export function unavailableReason( + entry: CommandCatalogEntry, + policy: SessionPolicy, +): string | undefined { + if (entry.exposure !== "exposed") { + return "it needs the local machine"; + } + if (!entry.parameters.properties?.flags?.properties?.json) { + return "it has no JSON output"; + } + if ( + policy.readOnly && + entry.permission !== "read" && + entry.permission !== "method" + ) { + return "this session is read-only"; + } + return; +} + +function refuse(entry: CommandCatalogEntry, reason: string): UserInputError { + return new UserInputError( + `\`${commandName(entry)}\` cannot run here: ${reason}.`, + ); +} + +/** The arguments of a command tool call, as its input schema accepts them. */ +type CommandParams = { + positional?: string[] | null; + flags?: unknown; +}; + +/** Drop the injected `json` flag, and refuse flags the MCP does not pass through. */ +function readFlags( + entry: CommandCatalogEntry, + rawFlags: unknown, +): Record<string, unknown> { + if (rawFlags === undefined || rawFlags === null) { + return {}; + } + if (typeof rawFlags !== "object" || Array.isArray(rawFlags)) { + throw new UserInputError( + `Invalid arguments for \`${commandName(entry)}\`:\n- flags: must be an object`, + ); + } + const flags: Record<string, unknown> = {}; + for (const [name, value] of Object.entries(rawFlags)) { + if (isInjectedFlag(name) || value === undefined || value === null) { + continue; + } + const reason = refusedFlagReason(entry, name); + if (reason) { + // Turning a refused boolean flag off asks for nothing. + if (value === false) { + continue; + } + throw refuse( + entry, + `\`--${name}\` is not available to agents, because ${reason}`, + ); + } + flags[name] = value; + } + return flags; +} + +/** + * Check a command tool call and build the call to run. + * + * @throws {UserInputError} When the command is unavailable, the params fail + * the schema, a read-only session gets a write call, or a target is + * outside the session's constraints + */ +export function prepareCommandCall({ + entry, + params, + policy, + context, +}: { + entry: CommandCatalogEntry; + params: CommandParams | undefined; + policy: SessionPolicy; + context: ServerContext; +}): CommandCall { + const name = commandName(entry); + const unavailable = unavailableReason(entry, policy); + if (unavailable) { + throw refuse(entry, unavailable); + } + + // `null` (some clients send it for an unset field) means none. + const positional = params?.positional ?? undefined; + const flags = readFlags(entry, params?.flags); + const errors = validateJsonSchema( + agentParameters(entry), + positional === undefined ? { flags } : { positional, flags }, + ); + if (errors.length > 0) { + throw new UserInputError( + `Invalid arguments for \`${name}\`:\n${errors.map((error) => `- ${error}`).join("\n")}`, + ); + } + + const permission = callPermission(entry, flags); + if (policy.readOnly && permission !== "read") { + throw refuse( + entry, + `this session is read-only, and this call is a ${permission} call`, + ); + } + // Write tools are listed only for their skills; `sentry api` is listed for + // every skill, so its writes are checked here. + const granted = context.grantedSkills; + if ( + entry.permission === "method" && + permission !== "read" && + granted && + !API_WRITE_SKILLS.some((skill) => granted.has(skill)) + ) { + throw refuse( + entry, + `this ${permission} call needs the ${API_WRITE_SKILLS.join(" or ")} skill`, + ); + } + + // `sentry api` sends an absolute URL to that URL's own host. + const [endpoint] = positional ?? []; + if ( + entry.permission === "method" && + endpoint !== undefined && + // The CLI drops whitespace and line breaks around an endpoint first. + URL_PATTERN.test(endpoint.replace(/\s+/g, "")) + ) { + throw refuse( + entry, + "an absolute URL fetches a host the caller picks. Pass the path relative to /api/0/ instead, such as `organizations/<org>/issues/`", + ); + } + + const cursor = flags.cursor; + if ( + policy.runtime === "hosted" && + typeof cursor === "string" && + CURSOR_KEYWORDS.has(cursor) + ) { + throw new UserInputError( + `The cursor keyword "${cursor}" needs stored pagination state, which this server does not keep. Pass the nextCursor value from the previous result instead.`, + ); + } + + const { organizationSlug, projectSlug } = context.constraints; + return { + path: commandSegments(entry), + positional: pinConstraints( + entry, + positional ?? [], + flags, + context.constraints, + ), + flags: flags as CommandCall["flags"], + ...(organizationSlug ? { org: organizationSlug } : {}), + ...(organizationSlug && projectSlug ? { project: projectSlug } : {}), + }; +} diff --git a/packages/mcp-core/src/command-surface/result.ts b/packages/mcp-core/src/command-surface/result.ts new file mode 100644 index 000000000..5afd339ad --- /dev/null +++ b/packages/mcp-core/src/command-surface/result.ts @@ -0,0 +1,154 @@ +/** + * Shapes a command's output into an execute tool result: the next cursor + * lifted out of list envelopes, and the whole result capped in size. + */ + +/** + * Largest result text, in characters (about 12,500 tokens). Claude Code's + * default MCP output limit is 25,000 tokens; staying well under it leaves + * room for several results in one turn. + */ +const MAX_RESULT_CHARS = 50_000; + +type Envelope = Record<string, unknown> & { + data?: unknown; + nextCursor?: unknown; +}; + +type Payload = { + command: string; + result: unknown; + nextCursor?: string; +}; + +function isEnvelope(value: unknown): value is Envelope { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +/** Lift a non-empty `nextCursor` out of a list envelope (`{ data, hasMore, nextCursor }`). */ +function withCursor(command: string, output: unknown): Payload { + if ( + isEnvelope(output) && + typeof output.nextCursor === "string" && + output.nextCursor + ) { + const { nextCursor, ...rest } = output; + return { command, nextCursor, result: rest }; + } + return { command, result: output ?? null }; +} + +/** The list a result can be cut down by, and how to put a shorter one back. */ +function listOf( + result: unknown, +): { items: unknown[]; rebuild: (items: unknown[]) => unknown } | undefined { + if (Array.isArray(result)) { + return { items: result, rebuild: (items) => items }; + } + if (isEnvelope(result) && Array.isArray(result.data)) { + return { + items: result.data, + rebuild: (items) => ({ ...result, data: items }), + }; + } + return; +} + +/** + * The longest prefix of a list result that fits in `maxChars`, found by + * binary search over the serialized size. `nextCursor` is dropped: it points + * past the whole page, so following it would skip the dropped items. + */ +function truncateList(payload: Payload, maxChars: number): string | undefined { + const list = listOf(payload.result); + if (!list) { + return; + } + const { nextCursor, ...rest } = payload; + const total = list.items.length; + const render = (count: number) => + JSON.stringify({ + ...rest, + result: list.rebuild(list.items.slice(0, count)), + truncated: { + returned: count, + total, + hint: nextCursor + ? "The result was too large, so the items after these were dropped, and no nextCursor is returned because it would skip them. Ask again with a lower limit, or fewer fields (fields), to page through them." + : "The result was too large, so the items after these were dropped. Ask for fewer items (limit) or fewer fields (fields).", + }, + }); + let low = 0; + let high = total - 1; + while (low < high) { + const middle = Math.ceil((low + high) / 2); + if (render(middle).length <= maxChars) { + low = middle; + } else { + high = middle - 1; + } + } + const text = render(low); + return text.length <= maxChars ? text : undefined; +} + +/** + * A result that is not a list, cut to fit in `maxChars`: `result` holds the + * start of its JSON text as a string, so the tool still returns valid JSON. + */ +function truncateText( + payload: Payload, + text: string, + maxChars: number, +): string { + // The cursor would skip what was cut, as in `truncateList`. + const { nextCursor: _skipped, ...rest } = payload; + const render = (length: number) => + JSON.stringify({ + ...rest, + result: text.slice(0, length), + truncated: { + characters: text.length, + hint: "The result was too large, so result holds only the start of its JSON text. Select fewer fields with the fields flag.", + }, + }); + let low = 0; + let high = text.length; + while (low < high) { + const middle = Math.ceil((low + high) / 2); + if (render(middle).length <= maxChars) { + low = middle; + } else { + high = middle - 1; + } + } + return render(low); +} + +/** + * The text an execute tool returns for `output`: JSON with the command, the + * result, and `nextCursor` for paginated commands. A result over `maxChars` + * keeps as many list items as fit, or keeps the start of its JSON text. + */ +export function formatCommandResult( + command: string, + output: unknown, + maxChars = MAX_RESULT_CHARS, +): string { + if (output instanceof Uint8Array) { + return JSON.stringify({ + command, + result: null, + error: `The command returned ${output.byteLength} bytes of binary output, which this tool cannot return.`, + }); + } + const payload = withCursor(command, output); + const text = JSON.stringify(payload); + if (text.length <= maxChars) { + return text; + } + return ( + truncateList(payload, maxChars) ?? + truncateText(payload, JSON.stringify(payload.result), maxChars) + ); +} diff --git a/packages/mcp-core/src/command-surface/tools.test.ts b/packages/mcp-core/src/command-surface/tools.test.ts new file mode 100644 index 000000000..c59c670d3 --- /dev/null +++ b/packages/mcp-core/src/command-surface/tools.test.ts @@ -0,0 +1,672 @@ +import { Client } from "@modelcontextprotocol/sdk/client/index.js"; +import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js"; +import { describe, expect, it, vi } from "vitest"; +import { type CommandCatalogEntry, commandCatalog } from "../command-catalog"; +import { buildServer } from "../server"; +import { createTestContext } from "../test-utils/context"; +import type { Constraints, ServerContext } from "../types"; +import { toCommandArgs } from "./args"; +import { type CommandSurfaceOptions, createCommandTools } from "./tools"; +import type { CommandCall } from "./types"; + +vi.mock("@sentry/core/server", () => ({ + wrapMcpServerWithSentry: vi.fn((server) => server), +})); + +/** Tools over the generated catalog, with a runner that records each call. */ +function setup( + options: Partial<CommandSurfaceOptions> = {}, + output: unknown = { data: [] }, +) { + const calls: CommandCall[] = []; + const tools = createCommandTools({ + runtime: "stdio", + run: async (call) => { + calls.push(call); + return output; + }, + ...options, + }); + // The registry's handlers are typed per tool; tests call them loosely. + const call = async ( + command: string, + params: Record<string, unknown> = {}, + context: ServerContext = createTestContext(), + ): Promise<Record<string, unknown>> => { + const tool = tools[toolName(command)]; + if (!tool) { + throw new Error(`no tool for ${command}`); + } + return JSON.parse((await tool.handler(params as never, context)) as string); + }; + return { tools, calls, call }; +} + +/** `issue list` → `sentry_issue_list`. */ +function toolName(command: string): string { + return `sentry ${command}`.replace(/[ -]/g, "_"); +} + +/** The catalog's exposed commands with JSON output, as tool names. */ +function exposedToolNames(include: (entry: CommandCatalogEntry) => boolean) { + return commandCatalog.commands + .filter( + (entry) => + entry.exposure === "exposed" && + entry.parameters.properties?.flags?.properties?.json && + include(entry), + ) + .map((entry) => entry.path.replace(/[ -]/g, "_")) + .sort(); +} + +function constrained(constraints: Constraints): ServerContext { + return createTestContext({ constraints }); +} + +describe("command tools: listing", () => { + it("lists one tool per exposed command", () => { + expect(Object.keys(setup().tools).sort()).toEqual( + exposedToolNames(() => true), + ); + }); + + it("lists only read tools and sentry api when read-only", () => { + expect(Object.keys(setup({ readOnly: true }).tools).sort()).toEqual( + exposedToolNames( + (entry) => entry.permission === "read" || entry.permission === "method", + ), + ); + }); + + it("grants writes only to the skill that covers them", () => { + const { tools } = setup(); + expect(tools.sentry_issue_list?.skills).toContain("inspect"); + expect(tools.sentry_issue_resolve?.skills).toEqual(["triage"]); + expect(tools.sentry_issue_plan?.skills).toEqual(["seer"]); + expect(tools.sentry_project_delete?.skills).toEqual(["project-management"]); + expect(tools.sentry_api?.skills).toContain("inspect"); + }); + + it("refuses sentry api writes without a write skill", async () => { + const { call, calls } = setup(); + const inspectOnly = createTestContext({ + grantedSkills: new Set(["inspect"]), + }); + await call("api", { positional: ["organizations/"] }, inspectOnly); + await expect( + call( + "api", + { positional: ["projects/acme/web/"], flags: { method: "DELETE" } }, + inspectOnly, + ), + ).rejects.toThrow("needs the triage or project-management skill"); + expect(calls).toHaveLength(1); + }); + + it("names tools from the command path", () => { + const { tools } = setup(); + expect(tools.sentry_issue_list).toBeDefined(); + expect(tools.sentry_agent_conversation_list).toBeDefined(); + // Hidden and stdio-only commands are not tools. + expect(tools.sentry_auth_login).toBeUndefined(); + expect(tools.sentry_sourcemap_upload).toBeUndefined(); + }); + + it("titles each tool from its command path", () => { + const { tools } = setup(); + expect(tools.sentry_issue_list?.title).toBe("Issue List"); + expect(tools.sentry_agent_conversation_list?.title).toBe( + "Agent Conversation List", + ); + const titles = Object.values(tools).map((tool) => tool.title); + expect(new Set(titles).size).toBe(titles.length); + }); + + it("describes a command with its brief and usable examples", () => { + const description = setup().tools.sentry_issue_view?.description ?? ""; + expect(description).toMatch(/^View details of/); + expect(description).toContain("sentry issue view FRONT-ABC"); + // Examples that pipe output elsewhere are dropped. + expect(description).not.toContain("| jq"); + }); + + it("gives sentry api the hints of its session", () => { + expect(setup().tools.sentry_api?.annotations).toMatchObject({ + readOnlyHint: false, + destructiveHint: true, + }); + expect( + setup({ readOnly: true }).tools.sentry_api?.annotations, + ).toMatchObject({ readOnlyHint: true, destructiveHint: false }); + }); +}); + +describe("command tools: permission classes", () => { + it("marks read, write, and destructive commands", () => { + const { tools } = setup(); + expect( + [ + tools.sentry_issue_list, + tools.sentry_issue_resolve, + tools.sentry_project_delete, + ].map((tool) => tool?.annotations), + ).toEqual([ + { readOnlyHint: true, destructiveHint: false, openWorldHint: true }, + { readOnlyHint: false, destructiveHint: true, openWorldHint: true }, + { readOnlyHint: false, destructiveHint: true, openWorldHint: true }, + ]); + }); + + it("runs a read command", async () => { + const { call, calls } = setup(); + await call("issue list", { + positional: ["acme/web"], + flags: { limit: 5, query: "is:unresolved" }, + }); + expect(calls).toEqual([ + { + path: ["issue", "list"], + positional: ["acme/web"], + flags: { limit: 5, query: "is:unresolved" }, + }, + ]); + }); + + it("runs write and destructive commands", async () => { + const { call, calls } = setup(); + await call("issue resolve", { positional: ["WEB-1"] }); + await call("project delete", { + positional: ["acme/web"], + flags: { yes: true }, + }); + expect(calls.map((c) => c.path.join(" "))).toEqual([ + "issue resolve", + "project delete", + ]); + }); + + it("runs sentry api with any method", async () => { + const { call, calls } = setup(); + await call("api", { positional: ["organizations/"] }); + await call("api", { + positional: ["projects/acme/web/"], + flags: { method: "delete" }, + }); + expect(calls.map((c) => c.flags.method)).toEqual([undefined, "delete"]); + }); + + it("refuses sentry api writes when read-only", async () => { + const { call, calls } = setup({ readOnly: true }); + await call("api", { positional: ["organizations/"] }); + await expect( + call("api", { + positional: ["organizations/acme/releases/"], + flags: { method: "PUT" }, + }), + ).rejects.toThrow( + "this session is read-only, and this call is a write call", + ); + expect(calls).toHaveLength(1); + }); +}); + +describe("command tools: arguments", () => { + it("rejects invalid arguments with what to pass instead", async () => { + const { call, calls } = setup(); + const error = await call("agent-conversation list", { + flags: { limit: "ten", sort: "oldest", colour: "red" }, + }).catch((e: Error) => e); + + expect(error).toBeInstanceOf(Error); + const message = (error as Error).message; + expect(message).toContain("flags.limit: expected number, got string"); + expect(message).toMatch( + /flags\.sort: "oldest" is not allowed \(expected one of: /, + ); + expect(message).toMatch( + /flags\.colour: unknown \(expected one of: .*query/, + ); + expect(calls).toEqual([]); + }); + + it("requires positionals the command needs", async () => { + const { call } = setup(); + await expect(call("api")).rejects.toThrow("positional: required"); + }); + + it("ignores json, which is always on", async () => { + const { call, calls } = setup(); + await call("org list", { flags: { json: true } }); + expect(calls[0]?.flags).toEqual({}); + }); + + it.each([ + ["issue view", "web", true, "it opens a browser"], + ["log list", "follow", "5", "it streams output with no end"], + ["dashboard view", "refresh", 10, "it streams output with no end"], + [ + "api", + "input", + "body.json", + "it reads or writes a path on the local machine", + ], + [ + "status show", + "url", + "http://169.254.169.254", + "it fetches a host the caller picks", + ], + ])("refuses %s --%s", async (command, flag, value, reason) => { + const { call } = setup(); + await expect( + call(command, { positional: ["x"], flags: { [flag]: value } }), + ).rejects.toThrow( + `\`--${flag}\` is not available to agents, because ${reason}`, + ); + }); + + it.each([ + "https://evil.example/api/0/organizations/acme/issues/", + " HTTP://169.254.169.254/latest/", + "ht\ntps://evil.example/organizations/acme/", + ])("refuses sentry api with the absolute URL %j", async (endpoint) => { + const { call, calls } = setup(); + await expect(call("api", { positional: [endpoint] })).rejects.toThrow( + "an absolute URL fetches a host the caller picks", + ); + expect(calls).toEqual([]); + }); + + it.each(["next", "prev", "last", "previous"])( + "refuses the stored-cursor keyword %s when hosted", + async (cursor) => { + const { call } = setup({ runtime: "hosted" }); + await expect(call("issue list", { flags: { cursor } })).rejects.toThrow( + "Pass the nextCursor value", + ); + }, + ); + + it("treats null flags and positionals as none", async () => { + const { call, calls } = setup(); + await call("org list", { positional: null, flags: null }); + expect(calls[0]).toMatchObject({ positional: [], flags: {} }); + await expect(call("org list", { flags: "limit=5" })).rejects.toThrow( + "flags: must be an object", + ); + }); +}); + +describe("command tools: session constraints", () => { + const org = constrained({ organizationSlug: "acme", projectSlug: null }); + const project = constrained({ organizationSlug: "acme", projectSlug: "web" }); + + it("passes the session's target as the default", async () => { + const { call, calls } = setup(); + await call("issue list", {}, project); + expect(calls[0]).toMatchObject({ + org: "acme", + project: "web", + positional: [], + }); + }); + + it("pins a bare project to the session's organization", async () => { + const { call, calls } = setup(); + await call("issue list", { positional: ["web"] }, org); + await call("attribute list", { positional: ["web"] }, org); + expect(calls.map((c) => c.positional)).toEqual([ + ["acme/web"], + ["acme/web"], + ]); + }); + + it("pins every project of an organization to the session's project", async () => { + const { call, calls } = setup(); + await call("issue list", { positional: ["acme/"] }, project); + await call("explore", { positional: ["acme/"] }, project); + await call("issue list", { positional: ["acme/"] }, org); + expect(calls.map((c) => c.positional)).toEqual([ + ["acme/web"], + ["acme/web"], + ["acme/"], + ]); + }); + + it("keeps GitHub-style and colon issue references in the session", async () => { + const { call, calls } = setup(); + await call("issue view", { positional: ["acme/web#G"] }, project); + await call("issue view", { positional: ["web:G"] }, project); + await call("issue view", { positional: ["#G"] }, project); + expect(calls.map((c) => c.positional)).toEqual([ + ["acme/web#G"], + ["web:G"], + ["#G"], + ]); + }); + + it("keeps dashboard and trace targets in the session", async () => { + const { call, calls } = setup(); + await call("dashboard view", { positional: ["acme", "123"] }, org); + await call("span list", { positional: ["acme/web"] }, project); + await call("trace view", { positional: ["acme/web", "abc123"] }, project); + expect(calls.map((c) => c.positional)).toEqual([ + ["acme", "123"], + ["acme/web"], + ["acme/web", "abc123"], + ]); + }); + + it("keeps ids that name no target", async () => { + const { call, calls } = setup(); + await call("agent-conversation view", { positional: ["conv-1"] }, org); + await call( + "agent-conversation view", + { positional: ["acme", "conv-1"] }, + org, + ); + expect(calls.map((c) => c.positional)).toEqual([ + ["conv-1"], + ["acme", "conv-1"], + ]); + }); + + it.each([ + ["issue list", ["other/web"], org], + ["issue list", ["acme/api"], project], + ["org view", ["other"], org], + ["issue view", ["other/WEB-1"], org], + ["issue view", ["https://other.sentry.io/issues/1/"], org], + ["project view", ["https://sentry.io/settings/other/projects/web/"], org], + [ + "project view", + ["https://sentry.io/settings/acme/projects/api/"], + project, + ], + [ + "project view", + ["https://acme.sentry.io/settings/projects/api/"], + project, + ], + [ + "feedback view", + ["https://acme.sentry.io/feedback/?feedbackSlug=api:1"], + project, + ], + ["release view", ["other/1.0.0"], org], + ["attribute list", ["other/web"], org], + ["attribute list", ["api"], project], + ["explore", ["other/"], org], + ["feedback view", ["other/WEB-1"], org], + ["feedback view", ["acme/api/1"], project], + ["issue view", ["acme/api/G"], project], + ["issue view", ["acme/api#G"], project], + ["issue view", ["api#G"], project], + ["issue view", ["api:G"], project], + ["issue view", ["other/web#G"], org], + ["agent-conversation view", ["conv-1", "other/conv-2"], org], + ["agent-conversation view", ["other", "conv-1"], org], + ["dashboard view", ["other", "123"], org], + ["dashboard view", ["other", "Errors"], org], + ["span list", ["acme/api"], project], + ["trace view", ["acme/api", "abc123"], project], + ["log list", ["acme/api"], project], + ])( + "refuses %s %j outside the session", + async (command, positional, context) => { + const { call, calls } = setup(); + await expect(call(command, { positional }, context)).rejects.toThrow( + "outside this session's scope", + ); + expect(calls).toEqual([]); + }, + ); + + it.each([ + ["release create", ["acme/1.0.0"], "api"], + ["release create", ["acme/1.0.0"], "web,api"], + ["alert metrics edit", ["acme/rule"], ["web", "api"]], + ["alert metrics edit", ["acme/rule"], ["web,api"]], + ])( + "refuses %s with --project %j for another project", + async (command, positional, projectFlag) => { + const { call, calls } = setup(); + await expect( + call(command, { positional, flags: { project: projectFlag } }, project), + ).rejects.toThrow("outside this session's scope"); + expect(calls).toEqual([]); + }, + ); + + it("checks sentry api endpoints against the session", async () => { + const { call, calls } = setup(); + await call( + "api", + { positional: ["/api/0/organizations/acme/issues/"] }, + org, + ); + await call("api", { positional: ["api/0/organizations/acme/"] }, org); + expect(calls).toHaveLength(2); + await expect( + call("api", { positional: ["api/0/organizations/other/"] }, org), + ).rejects.toThrow("outside this session's scope"); + await expect( + call("api", { positional: ["projects/other/web/"] }, org), + ).rejects.toThrow("outside this session's scope"); + await expect( + call( + "api", + { positional: ["organizations/acme/projects/api/keys/"] }, + project, + ), + ).rejects.toThrow("outside this session's scope"); + await call( + "api", + { positional: ["organizations/acme/projects/web/keys/"] }, + project, + ); + expect(calls).toHaveLength(3); + await call( + "project view", + { positional: ["https://sentry.io/settings/acme/projects/web/"] }, + project, + ); + expect(calls).toHaveLength(4); + await expect( + call("api", { positional: ["issues/1/"] }, org), + ).rejects.toThrow("not scoped to an organization"); + await expect( + call( + "api", + { positional: ["organizations/acme/../../projects/other/web/"] }, + org, + ), + ).rejects.toThrow("outside this session's scope"); + await expect( + call("api", { positional: ["organizations/acme/%2e%2e/other/"] }, org), + ).rejects.toThrow("outside this session's scope"); + await call("api", { positional: ["teams/acme/backend/projects/"] }, org); + await expect( + call("api", { positional: ["teams/other/backend/"] }, org), + ).rejects.toThrow("outside this session's scope"); + for (const endpoint of [ + "organizations/acme/issues/", + "teams/acme/backend/projects/", + "projects/acme/", + ]) { + await expect( + call("api", { positional: [endpoint] }, project), + ).rejects.toThrow("not scoped to a project"); + } + }); +}); + +describe("command tools: results", () => { + it("lifts nextCursor out of a list envelope", async () => { + const { call } = setup( + {}, + { data: [{ id: "1" }], hasMore: true, nextCursor: "0:25:0" }, + ); + expect(await call("issue list", {})).toEqual({ + command: "issue list", + nextCursor: "0:25:0", + result: { data: [{ id: "1" }], hasMore: true }, + }); + }); + + it("returns several yields as one result", async () => { + const { call } = setup({}, [{ a: 1 }, { b: 2 }]); + expect(await call("org list", {})).toEqual({ + command: "org list", + result: [{ a: 1 }, { b: 2 }], + }); + }); + + it("caps a large list at the items that fit", async () => { + const data = Array.from({ length: 100 }, (_, id) => ({ + id, + title: "x".repeat(100), + })); + const { tools } = setup( + { maxResultChars: 2_000 }, + { data, hasMore: false }, + ); + const text = (await tools.sentry_issue_list?.handler( + {} as never, + createTestContext(), + )) as string; + const result = JSON.parse(text); + + expect(text.length).toBeLessThanOrEqual(2_000); + expect(result.truncated).toMatchObject({ total: 100 }); + expect(result.result.data).toHaveLength(result.truncated.returned); + }); + + it("drops nextCursor when a page is cut, since it would skip items", async () => { + const data = Array.from({ length: 100 }, (_, id) => ({ + id, + title: "x".repeat(100), + })); + const { tools } = setup( + { maxResultChars: 2_000 }, + { data, hasMore: true, nextCursor: "0:100:0" }, + ); + const result = JSON.parse( + (await tools.sentry_issue_list?.handler( + {} as never, + createTestContext(), + )) as string, + ); + + expect(result.truncated.returned).toBeLessThan(100); + expect(result).not.toHaveProperty("nextCursor"); + expect(result.truncated.hint).toContain("lower limit"); + }); + + it("cuts a large single value to valid JSON within the cap", async () => { + const { tools } = setup( + { maxResultChars: 500 }, + { body: "x".repeat(2_000) }, + ); + const text = (await tools.sentry_org_list?.handler( + {} as never, + createTestContext(), + )) as string; + const parsed = JSON.parse(text); + + expect(text.length).toBeLessThanOrEqual(500); + expect(parsed.truncated.characters).toBe(2_011); + expect(parsed.result).toMatch(/^\{"body":"x+$/); + }); +}); + +describe("toCommandArgs", () => { + it("passes flags joined, --json, and positionals after --", () => { + expect( + toCommandArgs({ + path: ["issue", "list"], + positional: ["-acme/"], + flags: { + sort: "-date", + limit: 5, + fresh: true, + field: ["a=1", "b=2"], + query: "", + }, + }), + ).toEqual([ + "issue", + "list", + "--sort=-date", + "--limit=5", + "--fresh=true", + "--field=a=1", + "--field=b=2", + "--query", + "", + "--json", + "--", + "-acme/", + ]); + }); +}); + +describe("buildServer with the command tools", () => { + async function listTools(options: Partial<CommandSurfaceOptions>) { + const server = buildServer({ + context: createTestContext(), + tools: setup(options).tools, + }); + const [clientTransport, serverTransport] = + InMemoryTransport.createLinkedPair(); + const client = new Client({ + name: "command-surface-test", + version: "1.0.0", + }); + await server.connect(serverTransport); + await client.connect(clientTransport); + try { + return (await client.listTools()).tools; + } finally { + await client.close(); + await server.close(); + } + } + + it("lists every command tool with the catalog's schema and hints", async () => { + const tools = await listTools({}); + expect(tools.map((tool) => tool.name).sort()).toEqual( + exposedToolNames(() => true), + ); + + const issueList = tools.find((tool) => tool.name === "sentry_issue_list"); + if (!issueList) { + throw new Error("sentry_issue_list is not listed"); + } + expect(issueList.annotations).toMatchObject({ readOnlyHint: true }); + const flags = ( + issueList.inputSchema.properties as { + flags: { properties: Record<string, { default?: unknown }> }; + } + ).flags.properties; + expect(flags.query).toBeDefined(); + expect(flags.limit?.default).toBe(25); + expect(flags.json).toBeUndefined(); + + const api = tools.find((tool) => tool.name === "sentry_api"); + expect(api?.annotations?.destructiveHint).toBe(true); + expect(api?.inputSchema.required).toEqual(["positional"]); + }); + + it("lists no write tools when read-only", async () => { + const names = (await listTools({ readOnly: true })).map( + (tool) => tool.name, + ); + expect(names.sort()).toEqual( + exposedToolNames( + (entry) => entry.permission === "read" || entry.permission === "method", + ), + ); + }); +}); diff --git a/packages/mcp-core/src/command-surface/tools.ts b/packages/mcp-core/src/command-surface/tools.ts new file mode 100644 index 000000000..796813bdb --- /dev/null +++ b/packages/mcp-core/src/command-surface/tools.ts @@ -0,0 +1,185 @@ +/** + * The command surface: one MCP tool per exposed CLI command in the generated + * command catalog (`sentry issue list` → `sentry_issue_list`), instead of + * hand-written tools. Harnesses search large tool lists themselves, so there + * is no search or execute meta-tool. + * + * The tools only check and shape calls; a {@link CommandRunner} runs them. + * The stdio server (`sentry mcp --commands`) runs them through the CLI SDK; + * the hosted server will pass its own runner. + */ + +import { z } from "zod"; +import { + type CommandCatalog, + type CommandCatalogEntry, + commandCatalog, + type JsonSchema, +} from "../command-catalog"; +import { defineTool } from "../internal/tool-helpers/define"; +import type { ToolRegistry } from "../tools/catalog-runtime/availability"; +import type { ToolConfig } from "../tools/types"; +import { + agentParameters, + type CommandRuntime, + commandName, + commandSkills, + refusedFlagReason, +} from "./commands"; +import { + prepareCommandCall, + type SessionPolicy, + unavailableReason, +} from "./prepare"; +import { formatCommandResult } from "./result"; +import type { CommandRunner } from "./types"; + +export type CommandSurfaceOptions = { + run: CommandRunner; + runtime: CommandRuntime; + /** List only read tools, and refuse write calls of `sentry api`. */ + readOnly?: boolean; + /** Defaults to the generated catalog. */ + catalog?: CommandCatalog; + maxResultChars?: number; +}; + +/** The MCP tool name of a command: its path, with `_` for spaces and dashes. */ +function commandToolName(entry: CommandCatalogEntry): string { + return entry.path.replace(/[ -]/g, "_"); +} + +/** The display title of a command's tool: `sentry issue list` → `Issue List`. */ +function commandToolTitle(entry: CommandCatalogEntry): string { + return commandName(entry) + .split(/[ -]/) + .map((word) => word.charAt(0).toUpperCase() + word.slice(1)) + .join(" "); +} + +/** + * Hints from the command's permission class. `sentry api` takes the class of + * each call's method, so it is marked destructive unless the session is + * read-only, where it refuses everything but reads. + */ +function annotationsFor( + entry: CommandCatalogEntry, + policy: SessionPolicy, +): ToolConfig["annotations"] { + const readOnly = + entry.permission === "read" || + (entry.permission === "method" && policy.readOnly); + return readOnly + ? { readOnlyHint: true, destructiveHint: false, openWorldHint: true } + : { readOnlyHint: false, destructiveHint: true, openWorldHint: true }; +} + +const PIPE = / \| /; + +/** + * The command's examples an agent can follow: none that pipe output to + * another program or pass a flag the MCP refuses. + */ +function usableExamples(entry: CommandCatalogEntry): string[] { + const flagNames = Object.keys( + entry.parameters.properties?.flags?.properties ?? {}, + ); + const refused = flagNames.filter((name) => refusedFlagReason(entry, name)); + return entry.examples.filter( + (example) => + !PIPE.test(example) && + !refused.some((name) => example.includes(`--${name}`)), + ); +} + +function describeCommand( + entry: CommandCatalogEntry, + policy: SessionPolicy, +): string { + const flags = agentParameters(entry).properties?.flags?.properties ?? {}; + const examples = usableExamples(entry); + return [ + entry.brief, + "", + "Pass positional arguments in `positional` and flags in `flags`, by CLI flag name. Returns JSON.", + ...(flags.cursor + ? ["Paginated: pass the result's nextCursor as flags.cursor."] + : []), + ...(entry.permission === "method" + ? [ + policy.readOnly + ? "This session is read-only: only GET, HEAD, and OPTIONS run." + : "GET, HEAD, and OPTIONS read; other methods change data.", + ] + : []), + ...(examples.length > 0 ? ["", "Examples, in CLI form:", ...examples] : []), + ].join("\n"); +} + +/** + * The tool's input shape, listed as the catalog's JSON Schema. The schema + * rides along as metadata on permissive Zod types, so `tools/list` shows it + * unchanged while {@link prepareCommandCall} checks calls against it (with + * errors that name the allowed values) and no defaults are filled in. + */ +function inputShape(parameters: JsonSchema): Record<string, z.ZodType> { + const required = new Set(parameters.required ?? []); + return Object.fromEntries( + Object.entries(parameters.properties ?? {}).map(([name, schema]) => { + const field = z.unknown().meta(schema); + return [name, required.has(name) ? field : field.optional()]; + }), + ); +} + +function createCommandTool( + entry: CommandCatalogEntry, + policy: SessionPolicy, + options: CommandSurfaceOptions, +) { + return defineTool({ + name: commandToolName(entry), + title: commandToolTitle(entry), + skills: commandSkills(entry), + requiredScopes: [], + description: describeCommand(entry, policy), + inputSchema: inputShape(agentParameters(entry)), + annotations: annotationsFor(entry, policy), + async handler(params, context) { + const call = prepareCommandCall({ entry, params, policy, context }); + const output = await options.run(call, context); + return formatCommandResult( + commandName(entry), + output, + options.maxResultChars, + ); + }, + }); +} + +/** + * One tool per command this session can run, for `buildServer({ tools })`: + * the catalog's exposed commands with JSON output, without write commands + * when the session is read-only. + */ +export function createCommandTools( + options: CommandSurfaceOptions, +): ToolRegistry { + const catalog = options.catalog ?? commandCatalog; + const policy: SessionPolicy = { + runtime: options.runtime, + readOnly: options.readOnly ?? false, + }; + const tools: ToolRegistry = {}; + for (const entry of catalog.commands) { + if (unavailableReason(entry, policy)) { + continue; + } + const name = commandToolName(entry); + if (tools[name]) { + throw new Error(`Two commands map to the MCP tool name ${name}`); + } + tools[name] = createCommandTool(entry, policy, options); + } + return tools; +} diff --git a/packages/mcp-core/src/command-surface/types.ts b/packages/mcp-core/src/command-surface/types.ts new file mode 100644 index 000000000..d3b43730d --- /dev/null +++ b/packages/mcp-core/src/command-surface/types.ts @@ -0,0 +1,26 @@ +import type { ServerContext } from "../types"; + +/** A validated command call, ready for the CLI SDK. */ +export type CommandCall = { + /** Command path without `sentry`, such as `["issue", "list"]`. */ + path: string[]; + positional: string[]; + /** By CLI flag name (`dry-run`), without the injected `json`. */ + flags: Record<string, string | number | boolean | (string | number)[]>; + /** + * The session's organization and project, for the SDK to use when the call + * names no target. + */ + org?: string; + project?: string; +}; + +/** + * Runs a call and returns the command's JSON output: one value, or an array + * when the command yielded more than once. Throws `CommandFailedError` when + * the command fails. + */ +export type CommandRunner = ( + call: CommandCall, + context: ServerContext, +) => Promise<unknown>; diff --git a/packages/mcp-core/src/command-surface/validate.ts b/packages/mcp-core/src/command-surface/validate.ts new file mode 100644 index 000000000..e716f291e --- /dev/null +++ b/packages/mcp-core/src/command-surface/validate.ts @@ -0,0 +1,143 @@ +/** + * Checks a value against the JSON Schema subset `command-catalog.json` uses. + * + * Hand-written rather than a general validator: the subset is small, the + * messages can name what to pass instead, and the hosted MCP runs on Workers, + * where validators that compile schemas with `new Function` do not run. + */ + +import type { JsonSchema } from "../command-catalog"; + +/** Show at most this many allowed values or flag names in one message. */ +const MAX_LISTED = 25; + +function list(values: readonly string[]): string { + const shown = values.slice(0, MAX_LISTED).join(", "); + return values.length > MAX_LISTED + ? `${shown}, and ${values.length - MAX_LISTED} more` + : shown; +} + +/** The path of a property: `flags.limit`, or `flags` at the top level. */ +function child(path: string, name: string): string { + return path ? `${path}.${name}` : name; +} + +function typeOf(value: unknown): string { + if (value === null) { + return "null"; + } + return Array.isArray(value) ? "array" : typeof value; +} + +function matchesType(type: JsonSchema["type"], value: unknown): boolean { + switch (type) { + case undefined: + return true; + case "number": + return typeof value === "number" && Number.isFinite(value); + case "array": + return Array.isArray(value); + case "object": + return ( + typeof value === "object" && value !== null && !Array.isArray(value) + ); + default: + return typeof value === type; + } +} + +function validateArray( + schema: JsonSchema, + value: unknown[], + path: string, + errors: string[], +): void { + const prefix = schema.prefixItems ?? []; + if (schema.minItems !== undefined && value.length < schema.minItems) { + const names = prefix + .slice(0, schema.minItems) + .map((item, index) => item.title ?? `#${index}`); + errors.push( + `${path}: expected at least ${schema.minItems} value(s)${names.length ? ` (${names.join(", ")})` : ""}, got ${value.length}`, + ); + } + value.forEach((item, index) => { + const itemSchema = index < prefix.length ? prefix[index] : schema.items; + if (itemSchema === false) { + errors.push( + `${path}: expected at most ${prefix.length} value(s), got ${value.length}`, + ); + return; + } + if (itemSchema) { + validateValue(itemSchema, item, `${path}[${index}]`, errors); + } + }); +} + +function validateObject( + schema: JsonSchema, + value: Record<string, unknown>, + path: string, + errors: string[], +): void { + const properties = schema.properties ?? {}; + for (const name of schema.required ?? []) { + if (value[name] === undefined) { + errors.push(`${child(path, name)}: required`); + } + } + for (const [name, item] of Object.entries(value)) { + const itemPath = child(path, name); + const itemSchema = properties[name]; + if (!itemSchema) { + if (schema.additionalProperties === false) { + const known = Object.keys(properties); + errors.push( + `${itemPath}: unknown${known.length ? ` (expected one of: ${list(known)})` : ""}`, + ); + } + continue; + } + if (item !== undefined) { + validateValue(itemSchema, item, itemPath, errors); + } + } +} + +function validateValue( + schema: JsonSchema, + value: unknown, + path: string, + errors: string[], +): void { + if (!matchesType(schema.type, value)) { + errors.push(`${path}: expected ${schema.type}, got ${typeOf(value)}`); + return; + } + if (schema.enum && !schema.enum.includes(value as string)) { + errors.push( + `${path}: ${JSON.stringify(value)} is not allowed (expected one of: ${list(schema.enum)})`, + ); + return; + } + if (Array.isArray(value)) { + validateArray(schema, value, path, errors); + } else if (schema.type === "object") { + validateObject(schema, value as Record<string, unknown>, path, errors); + } +} + +/** + * Every way an object `value` fails `schema`, as `path: problem` lines + * (`flags.limit: expected number, got string`). + */ +export function validateJsonSchema( + schema: JsonSchema, + value: Record<string, unknown>, +): string[] { + const errors: string[] = []; + validateObject(schema, value, "", errors); + return errors; +} diff --git a/packages/mcp-server/script/bundle.ts b/packages/mcp-server/script/bundle.ts index 4fbc39664..62d6d55af 100644 --- a/packages/mcp-server/script/bundle.ts +++ b/packages/mcp-server/script/bundle.ts @@ -43,6 +43,7 @@ const MAX_BUNDLE_BYTES = 3.75 * 1024 * 1024; const FORBIDDEN_INPUTS: ReadonlyArray<RegExp> = [ /[\\/]packages[\\/]cli[\\/]src[\\/](?:app\.ts$|commands[\\/](?!auth[\\/]|help\.ts$|mcp\.ts$|local[\\/]loopback\.ts$))/, /[\\/]packages[\\/]cli[\\/]src[\\/]lib[\\/](?:agent-skills|complete|completions)\.ts$/, + /[\\/]packages[\\/]cli[\\/]src[\\/]lib[\\/]mcp[\\/]command-surface\.ts$/, /[\\/]packages[\\/]cli[\\/]src[\\/]lib[\\/](?:sixel|sixel-image|kitty-image|formatters[\\/]sixel-dashboard)\.ts$/, /[\\/]packages[\\/]cli[\\/]src[\\/]generated[\\/]banner-image\.ts$/, /[\\/]packages[\\/]cli[\\/]src[\\/]lib[\\/](?:bash-hook|build|dif|react-native|select-ui|snapshots|sourcemap|wasm)[\\/]/, @@ -78,6 +79,32 @@ const stubModulesPlugin: Plugin = { }, }; +/** + * `sentry mcp --commands` runs CLI commands, which this package does not + * bundle yet (#1909), so its module (`lib/mcp/command-surface.ts`) becomes + * one that refuses the flag. + */ +const commandSurfaceStubPlugin: Plugin = { + name: "stub-command-surface", + setup(b) { + b.onResolve({ filter: /^\.\/mcp\/command-surface\.js$/ }, (args) => ({ + path: args.path, + namespace: "command-surface-stub", + pluginData: { resolveDir: args.resolveDir }, + })); + b.onLoad({ filter: /.*/, namespace: "command-surface-stub" }, (args) => ({ + contents: [ + 'import { ValidationError } from "./errors.js";', + "export function createCliCommandTools() {", + ' throw new ValidationError("--commands is not available in @sentry/mcp-server yet. Run `sentry mcp --commands` from the Sentry CLI instead.");', + "}", + ].join("\n"), + loader: "ts", + resolveDir: (args.pluginData as { resolveDir: string }).resolveDir, + })); + }, +}; + /** Inputs on {@link FORBIDDEN_INPUTS} that contribute code to the output. */ function findForbiddenInputs(metafile: Metafile): string[] { return Object.values(metafile.outputs) @@ -139,6 +166,7 @@ const result = await build({ external: ["node:*"], metafile: true, plugins: [ + commandSurfaceStubPlugin, stubModulesPlugin, textImportPlugin, requireAliasPlugin,