Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/adding-tools.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ Not every tool is exposed to every consumer. We rely on several mechanisms to ke
- **`requiredCapabilities`** — Tools declare which project capabilities they need (e.g. `profiles`, `replays`, `traces`). If the upstream project doesn't have a capability enabled, the tool is automatically hidden.
- **`experimental` / `hideInExperimentalMode`** — Feature flags for tools that are being tested or replaced.
- **Skills & constraints** — The server filters tools based on granted skills and org/project constraints.
- **Experimental skill merging** — A skill can set `mergedIntoSkillInExperimentalMode` to make its tools available through another skill in experimental mode only. For example, `preprod` currently merges into `inspect` for `?experimental=1` sessions.

We also expect upstream consumers (Claude Code plugins, Cursor, etc.) to use **tool selection** or **progressive disclosure** on their end. The catalog can contain more tools than the direct MCP surface, but the registered top-level tool count must still stay within the limits below.

Expand Down
35 changes: 35 additions & 0 deletions packages/mcp-cloudflare/src/server/lib/approval-dialog.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,41 @@ describe("approval-dialog", () => {
expect(html).toContain(">javascript</strong> project");
expect(html).toContain(">sentry</strong> organization");
});

it("should render merged skills separately outside experimental mode", async () => {
const response = await renderApprovalDialog(
new Request("https://example.com/oauth/authorize"),
{
client: mockClient,
server: { name: "Sentry MCP" },
state: { oauthReqInfo: { clientId: "test-client" } },
cookieSecret: TEST_SECRET,
},
);

const html = await response.text();

expect(html).toContain("Inspect Issues &amp; Events");
expect(html).toContain("Preprod Snapshots");
});

it("should hide merged skills in experimental mode", async () => {
const response = await renderApprovalDialog(
new Request("https://example.com/oauth/authorize"),
{
client: mockClient,
server: { name: "Sentry MCP" },
experimentalMode: true,
state: { oauthReqInfo: { clientId: "test-client" } },
cookieSecret: TEST_SECRET,
},
);

const html = await response.text();

expect(html).toContain("Inspect Issues &amp; Events");
expect(html).not.toContain("Preprod Snapshots");
});
});

describe("CSRF protection with HMAC-signed state", () => {
Expand Down
69 changes: 67 additions & 2 deletions packages/mcp-cloudflare/src/server/lib/approval-dialog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,10 @@ export interface ApprovalDialogOptions {
organizationSlug: string;
projectSlug: string | null;
} | null;
/**
* Whether the OAuth grant is for an experimental MCP endpoint.
*/
experimentalMode?: boolean;
/**
* The specific redirect URI selected for this authorization request.
*/
Expand All @@ -214,6 +218,56 @@ export interface ApprovalDialogOptions {
cookieSecret: string;
}

function getSkillsForApproval(
skills: SkillDefinition[],
experimentalMode: boolean,
): SkillDefinition[] {
if (!experimentalMode) {
return skills;
}

const visibleSkills = skills
.filter((skill) => !skill.mergedIntoSkillInExperimentalMode)
.map((skill) => ({ ...skill }));

for (const mergedSkill of skills) {
const targetSkillId = mergedSkill.mergedIntoSkillInExperimentalMode;
if (!targetSkillId) {
continue;
}

const targetSkill = visibleSkills.find(
(skill) => skill.id === targetSkillId,
);
if (!targetSkill) {
continue;
}

if (targetSkill.tools || mergedSkill.tools) {
const toolsByName = new Map(
(targetSkill.tools ?? []).map((tool) => [tool.name, tool]),
);
for (const tool of mergedSkill.tools ?? []) {
toolsByName.set(tool.name, tool);
}
targetSkill.tools = Array.from(toolsByName.values()).sort((a, b) =>
a.name.localeCompare(b.name),
);
targetSkill.toolCount = targetSkill.tools.length;
continue;
}

if (mergedSkill.toolCount === undefined) {
continue;
}

targetSkill.toolCount =
(targetSkill.toolCount ?? 0) + mergedSkill.toolCount;
}

return visibleSkills;
}

/**
* Encodes and signs arbitrary data to a HMAC-signed compact string.
* @param data - The data to encode (will be stringified).
Expand Down Expand Up @@ -279,10 +333,21 @@ export async function renderApprovalDialog(
request: Request,
options: ApprovalDialogOptions,
): Promise<Response> {
const { client, server, scope, redirectUri, state, cookieSecret } = options;
const {
client,
server,
scope,
experimentalMode = false,
redirectUri,
state,
cookieSecret,
} = options;

// Use static skill definitions bundled at build time
const skills: SkillDefinition[] = skillDefinitions as SkillDefinition[];
const skills = getSkillsForApproval(
skillDefinitions as SkillDefinition[],
experimentalMode,
);

// Generate HTML for all skills (checked if defaultEnabled)
const skillsHtml = skills
Expand Down
35 changes: 35 additions & 0 deletions packages/mcp-cloudflare/src/server/oauth/resource-scope.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { describe, expect, it } from "vitest";
import {
parseResourceExperimentalMode,
parseResourceMcpConstraints,
} from "./resource-scope";

describe("resource-scope", () => {
describe("parseResourceMcpConstraints", () => {
it("parses organization and project constraints from MCP resource URLs", () => {
expect(
parseResourceMcpConstraints("https://example.com/mcp/sentry/frontend"),
).toEqual({
organizationSlug: "sentry",
projectSlug: "frontend",
});
});
});

describe("parseResourceExperimentalMode", () => {
it("detects experimental MCP resource URLs", () => {
expect(
parseResourceExperimentalMode(
"https://example.com/mcp/sentry/frontend?experimental=1",
),
).toBe(true);
});

it("returns false for stable or invalid resource URLs", () => {
expect(
parseResourceExperimentalMode("https://example.com/mcp/sentry"),
).toBe(false);
expect(parseResourceExperimentalMode("not a url")).toBe(false);
});
});
});
14 changes: 14 additions & 0 deletions packages/mcp-cloudflare/src/server/oauth/resource-scope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,17 @@ export function parseResourceMcpConstraints(
return null;
}
}

export function parseResourceExperimentalMode(
resource: string | null | undefined,
): boolean {
if (!resource) {
return false;
}

try {
return new URL(resource).searchParams.get("experimental") === "1";
} catch {
return false;
}
}
7 changes: 6 additions & 1 deletion packages/mcp-cloudflare/src/server/oauth/routes/authorize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@ import {
import { SCOPES } from "../../../constants";
import { signState, type OAuthState } from "../state";
import { logWarn } from "@sentry/mcp-core/telem/logging";
import { parseResourceMcpConstraints } from "../resource-scope";
import {
parseResourceExperimentalMode,
parseResourceMcpConstraints,
} from "../resource-scope";

/**
* Extended AuthRequest that includes skills and resource parameter
Expand Down Expand Up @@ -143,6 +146,7 @@ export default new Hono<{ Bindings: Env }>()
...(resourceParam ? { resource: resourceParam } : {}),
};
const approvalScope = parseResourceMcpConstraints(resourceParam);
const experimentalMode = parseResourceExperimentalMode(resourceParam);

// XXX(dcramer): we want to confirm permissions on each time
// so you can always choose new ones
Expand All @@ -168,6 +172,7 @@ export default new Hono<{ Bindings: Env }>()
name: "Sentry MCP",
},
scope: approvalScope,
experimentalMode,
redirectUri: oauthReqInfoWithResource.redirectUri,
state: { oauthReqInfo: oauthReqInfoWithResource },
cookieSecret: c.env.COOKIE_SECRET,
Expand Down
15 changes: 12 additions & 3 deletions packages/mcp-cloudflare/src/server/oauth/routes/callback.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@ import {
getOAuthCallbackFailureDetails,
validateResourceParameter,
} from "../helpers";
import { parseResourceMcpConstraints } from "../resource-scope";
import {
parseResourceExperimentalMode,
parseResourceMcpConstraints,
} from "../resource-scope";
import { type OAuthState, verifyAndParseState } from "../state";

/**
Expand Down Expand Up @@ -264,13 +267,19 @@ export default new Hono<{ Bindings: Env }>().get("/", async (c) => {
);
}

const resource = (oauthReqInfo as AuthRequestWithSkills).resource;
const experimentalMode = parseResourceExperimentalMode(
typeof resource === "string" ? resource : undefined,
);

// Calculate Sentry API scopes from validated skills
const grantedScopes = await getScopesForSkills(validSkills);
const grantedScopes = await getScopesForSkills(validSkills, {
experimentalMode,
});

// Convert valid skills Set to array for OAuth props
const grantedSkills = Array.from(validSkills);

const resource = (oauthReqInfo as AuthRequestWithSkills).resource;
const resourceScope = parseResourceMcpConstraints(
typeof resource === "string" ? resource : undefined,
);
Expand Down
109 changes: 63 additions & 46 deletions packages/mcp-core/src/server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js";
import { setUser } from "@sentry/core";
import { beforeEach, describe, expect, it, vi } from "vitest";
import { buildServer } from "./server";
import type { Skill } from "./skills";
import tools from "./tools";
import {
CATALOG_INFRASTRUCTURE_TOOL_NAMES,
Expand Down Expand Up @@ -440,6 +441,59 @@ describe("buildServer", () => {
});
});

describe("experimental skill collapsing", () => {
it("does not expose preprod tools through inspect in stable mode", () => {
const server = buildServer({
context: {
...baseContext,
grantedSkills: new Set<Skill>(["inspect"]),
},
experimentalMode: false,
tools: {
preprod_tool: createMockTool("preprod_tool", {
skills: ["preprod"],
}),
},
});

expect(getRegisteredToolNames(server)).not.toContain("preprod_tool");
});

it("exposes preprod tools through inspect in experimental mode", () => {
const server = buildServer({
context: {
...baseContext,
grantedSkills: new Set<Skill>(["inspect"]),
},
experimentalMode: true,
tools: {
preprod_tool: createMockTool("preprod_tool", {
skills: ["preprod"],
}),
},
});

expect(getRegisteredToolNames(server)).toContain("preprod_tool");
});

it("continues to expose preprod tools to explicit preprod grants", () => {
const server = buildServer({
context: {
...baseContext,
grantedSkills: new Set<Skill>(["preprod"]),
},
experimentalMode: false,
tools: {
preprod_tool: createMockTool("preprod_tool", {
skills: ["preprod"],
}),
},
});

expect(getRegisteredToolNames(server)).toContain("preprod_tool");
});
});

describe("hideInExperimentalMode filtering", () => {
it("hides tools with hideInExperimentalMode when experimentalMode is true", () => {
const server = buildServer({
Expand Down Expand Up @@ -684,57 +738,20 @@ describe("buildServer", () => {
expect(registeredTools.map((tool) => tool.name)).toEqual(["use_sentry"]);
});

it("keeps preprod tools catalog-only while enforcing their skill gate", async () => {
const withoutPreprod = buildServer({
it("keeps preprod tools catalog-only while exposing them through inspect in experimental mode", async () => {
const server = buildServer({
context: baseContext,
experimentalMode: true,
});
const withoutPreprodToolNames = getRegisteredToolNames(withoutPreprod);
expect(withoutPreprodToolNames).not.toContain("get_snapshot");
expect(withoutPreprodToolNames).not.toContain("get_snapshot_image");
expect(withoutPreprodToolNames).not.toContain("get_latest_base_snapshot");

const hiddenResult = await callRegisteredTool(
withoutPreprod,
"search_tools",
{
query: "snapshot",
limit: 10,
},
);
const hiddenPayload = getStructuredContent<{
results: Array<{ name: string }>;
}>(hiddenResult);
expect(hiddenPayload.results.map((tool) => tool.name)).not.toContain(
"get_snapshot",
);
const topLevelToolNames = getRegisteredToolNames(server);
expect(topLevelToolNames).not.toContain("get_snapshot");
expect(topLevelToolNames).not.toContain("get_snapshot_image");
expect(topLevelToolNames).not.toContain("get_latest_base_snapshot");

const withPreprod = buildServer({
context: {
...baseContext,
grantedSkills: new Set([
"inspect",
"triage",
"project-management",
"seer",
"preprod",
]),
},
experimentalMode: true,
const visibleResult = await callRegisteredTool(server, "search_tools", {
query: "snapshot",
limit: 10,
});
const withPreprodToolNames = getRegisteredToolNames(withPreprod);
expect(withPreprodToolNames).not.toContain("get_snapshot");
expect(withPreprodToolNames).not.toContain("get_snapshot_image");
expect(withPreprodToolNames).not.toContain("get_latest_base_snapshot");

const visibleResult = await callRegisteredTool(
withPreprod,
"search_tools",
{
query: "snapshot",
limit: 10,
},
);
const visiblePayload = getStructuredContent<{
results: Array<{ name: string }>;
}>(visibleResult);
Expand Down
Loading
Loading