ref(toolkit-core): Share API path template matching between CLI and Code Mode

docs/architecture/code-mode.md

@@ -90,8 +90,10 @@ Before each `sentry.request()`, the broker (`broker.ts`) checks:
 
 Then its `ApiAuthorization` (`authorization.ts`, `access.ts`) checks:
 
-3. The path and method are a published operation. HEAD may use a published
-   GET. Traversal segments, unknown paths and ambiguous paths fail.
+3. The path and method are a published operation, matched with the template
+   matcher shared with the CLI. HEAD may use a published GET, and a missing
+   trailing slash is added to the path sent. Traversal segments, unknown paths
+   and ambiguous paths fail.
 4. On `/mcp/{org}`, the organization path parameter must be that organization.
    Other organizations, numeric aliases and global object-ID routes fail;
    query parameters cannot stand in for a scoped path.

packages/cli/src/lib/idempotent-operations.ts

@@ -7,6 +7,7 @@
  * so the CLI does not hard-code which assemble calls may retry.
  */
 
+import { matchApiPathTemplate } from "@sentry/toolkit-core/api-path-template";
 import generated from "../generated/idempotent-operations.json";
 
 /** A non-safe operation Sentry marks idempotent. */
@@ -17,24 +18,7 @@ export type IdempotentOperation = {
   path: string;
 };
 
-type Matcher = { method: string; pattern: RegExp };
-
-const TEMPLATE_SEGMENT = /^\{[^/]+\}$/;
-const REGEXP_SPECIAL = /[.*+?^${}()|[\]\\]/g;
-
-function toMatcher({ method, path }: IdempotentOperation): Matcher {
-  const source = path
-    .split("/")
-    .map((segment) =>
-      TEMPLATE_SEGMENT.test(segment)
-        ? "[^/]+"
-        : segment.replace(REGEXP_SPECIAL, "\\$&"),
-    )
-    .join("/");
-  return { method: method.toUpperCase(), pattern: new RegExp(`^${source}$`) };
-}
-
-let matchers: Matcher[] | undefined;
+const operations = generated as IdempotentOperation[];
 
 /**
  * Whether Sentry marks this request's operation idempotent.
@@ -57,9 +41,9 @@ export function isIdempotentOperation(method: string, url: string): boolean {
   const apiPath = pathname.slice(apiIndex);
   const normalized = apiPath.endsWith("/") ? apiPath : `${apiPath}/`;
   const upper = method.toUpperCase();
-
-  matchers ??= (generated as IdempotentOperation[]).map(toMatcher);
-  return matchers.some(
-    (matcher) => matcher.method === upper && matcher.pattern.test(normalized),
+  return operations.some(
+    (operation) =>
+      operation.method === upper &&
+      matchApiPathTemplate(operation.path, normalized) !== undefined,
   );
 }

packages/mcp-cloudflare/src/code-mode/access.test.ts

@@ -26,7 +26,11 @@ describe("authorizeApiRequest", () => {
   it("accepts published operations and HEAD on a published GET", () => {
     expect(
       request("/api/0/organizations/example/dashboards/?cursor=next", "GET"),
-    ).toEqual({ pathTemplate: dashboardPath, organizationSlug: "example" });
+    ).toEqual({
+      pathTemplate: dashboardPath,
+      pathname: "/api/0/organizations/example/dashboards/",
+      organizationSlug: "example",
+    });
     expect(
       request("/api/0/organizations/example/dashboards/", "HEAD").pathTemplate,
     ).toBe(dashboardPath);
@@ -51,6 +55,7 @@ describe("authorizeApiRequest", () => {
     }
     expect(request("/api/0/issues/123/", "GET")).toEqual({
       pathTemplate: "/api/0/issues/{issue_id}/",
+      pathname: "/api/0/issues/123/",
     });
   });
 
@@ -61,8 +66,14 @@ describe("authorizeApiRequest", () => {
     expect(() =>
       request("/api/0/organizations/example/dashboards/", "DELETE"),
     ).toThrow("HTTP method");
+  });
+
+  it("adds a missing trailing slash to match and send the canonical path", () => {
+    expect(
+      request("/api/0/organizations/example/dashboards", "GET").pathname,
+    ).toBe("/api/0/organizations/example/dashboards/");
     expect(() =>
-      request("/api/0/organizations/example/dashboards", "GET"),
+      request("/api/0/organizations/example/dashboards//", "GET"),
     ).toThrow("published API schema");
   });
 

packages/mcp-cloudflare/src/code-mode/access.ts

@@ -1,16 +1,16 @@
 import type { ApiOperation } from "@sentry/api/operations";
+import {
+  type ApiPathTemplateMatch,
+  matchApiPathTemplate,
+} from "@sentry/toolkit-core/api-path-template";
 
 export type CatalogOperation = Pick<ApiOperation, "method" | "path">;
 
 export interface CodeModeAccess {
   organizationSlug?: string | null;
 }
 
-interface MatchedPath {
-  pathTemplate: string;
-  parameters: Record<string, string>;
-  literalSegments: number;
-}
+type MatchedPath = ApiPathTemplateMatch & { pathTemplate: string };
 
 function decodePathSegment(segment: string): string {
   let decoded: string;
@@ -27,44 +27,23 @@ function decodePathSegment(segment: string): string {
   return decoded;
 }
 
-/** The most literal published path template matching the URL. */
-function findPath(url: URL, pathTemplates: Iterable<string>): MatchedPath {
-  const segments = url.pathname.split("/").map(decodePathSegment);
+/** The most literal published path template matching a decoded path. */
+function findPath(
+  path: string,
+  pathTemplates: Iterable<string>,
+): MatchedPath | undefined {
   const matches: MatchedPath[] = [];
   for (const pathTemplate of pathTemplates) {
-    const templateSegments = pathTemplate.split("/");
-    if (templateSegments.length !== segments.length) continue;
-    const parameters: Record<string, string> = {};
-    let literalSegments = 0;
-    const matchesPath = templateSegments.every((segment, index) => {
-      const parameter = /^\{([^{}]+)\}$/.exec(segment);
-      if (parameter) {
-        if (!segments[index] || Object.hasOwn(parameters, parameter[1])) {
-          return false;
-        }
-        parameters[parameter[1]] = segments[index];
-        return true;
-      }
-      literalSegments++;
-      return segment === segments[index];
-    });
-    if (matchesPath) {
-      matches.push({ pathTemplate, parameters, literalSegments });
-    }
+    const match = matchApiPathTemplate(pathTemplate, path);
+    if (match) matches.push({ pathTemplate, ...match });
   }
   matches.sort((a, b) => b.literalSegments - a.literalSegments);
-  const match = matches[0];
-  if (!match) {
-    throw new Error(
-      "This API path is not available in the published API schema.",
-    );
-  }
-  if (matches[1]?.literalSegments === match.literalSegments) {
+  if (matches[1] && matches[1].literalSegments === matches[0].literalSegments) {
     throw new Error(
       "This API path matches multiple operations in the API schema.",
     );
   }
-  return match;
+  return matches[0];
 }
 
 /**
@@ -77,11 +56,22 @@ export function authorizeApiRequest(
   method: string,
   catalog: readonly CatalogOperation[],
   access: CodeModeAccess,
-): { pathTemplate: string; organizationSlug?: string } {
-  const { pathTemplate, parameters } = findPath(
-    url,
-    new Set(catalog.map(({ path }) => path)),
-  );
+): { pathTemplate: string; pathname: string; organizationSlug?: string } {
+  const templates = new Set(catalog.map(({ path }) => path));
+  const decoded = url.pathname.split("/").map(decodePathSegment).join("/");
+  let pathname = url.pathname;
+  let match = findPath(decoded, templates);
+  // Most routes end with a slash. Send the canonical path rather than fail.
+  if (!match && !pathname.endsWith("/")) {
+    match = findPath(`${decoded}/`, templates);
+    pathname = `${pathname}/`;
+  }
+  if (!match) {
+    throw new Error(
+      "This API path is not available in the published API schema.",
+    );
+  }
+  const { pathTemplate, parameters } = match;
   const methods = new Set<string>(
     catalog.filter(({ path }) => path === pathTemplate).map((op) => op.method),
   );
@@ -103,5 +93,9 @@ export function authorizeApiRequest(
     );
   }
 
-  return { pathTemplate, ...(organizationSlug ? { organizationSlug } : {}) };
+  return {
+    pathTemplate,
+    pathname,
+    ...(organizationSlug ? { organizationSlug } : {}),
+  };
 }

packages/mcp-cloudflare/src/code-mode/authorization.ts

@@ -24,8 +24,9 @@ export class ApiAuthorization {
     const target = new URL(url);
     let organizationSlug: string | undefined;
     let pathTemplate: string;
+    let pathname: string;
     try {
-      ({ organizationSlug, pathTemplate } = authorizeApiRequest(
+      ({ organizationSlug, pathTemplate, pathname } = authorizeApiRequest(
         target,
         method,
         OPERATIONS,
@@ -40,7 +41,7 @@ export class ApiAuthorization {
       await apiServiceForOrganization(this.#context, organizationSlug)
     ).requestTarget();
     return {
-      url: new URL(`${target.pathname}${target.search}`, origin).toString(),
+      url: new URL(`${pathname}${target.search}`, origin).toString(),
       headers,
       pathTemplate,
     };

packages/mcp-cloudflare/src/code-mode/tools.integration.test.ts

@@ -417,15 +417,16 @@ describe("Code Mode on the authenticated MCP", () => {
     expect(apiRequests).toHaveLength(0);
   });
 
-  it("uses the API write grant, regional routing and attribution headers", async () => {
+  it("uses the API write grant, regional routing, the canonical path and attribution headers", async () => {
     apiResponse = (request) =>
       Response.json(
         { saved: JSON.parse(request.body) },
         { headers: { ETag: '"saved-monitor"', Link: "<next-page>; rel=next" } },
       );
+    // Without the trailing slash: the broker sends the published path.
     const result = await execute(
       `async () => sentry.request({
-      method: "PUT", path: "/projects/code-mode-org/web/monitors/billing/", body: {name: "Billing"}
+      method: "PUT", path: "/projects/code-mode-org/web/monitors/billing", body: {name: "Billing"}
     })`,
       { props: { grantedSkills: ["api-write"] } },
     );

packages/toolkit-core/package.json

@@ -15,6 +15,10 @@
       "types": "./src/api-path-segment.ts",
       "default": "./src/api-path-segment.ts"
     },
+    "./api-path-template": {
+      "types": "./src/api-path-template.ts",
+      "default": "./src/api-path-template.ts"
+    },
     "./auth-token": {
       "types": "./src/auth-token.ts",
       "default": "./src/auth-token.ts"

packages/toolkit-core/src/api-path-template.test.ts

@@ -0,0 +1,39 @@
+import { describe, expect, it } from "vitest";
+import { matchApiPathTemplate } from "./api-path-template.js";
+
+const issues = "/api/0/organizations/{organization_id_or_slug}/issues/";
+
+describe("matchApiPathTemplate", () => {
+  it("captures parameters and counts literal segments", () => {
+    expect(
+      matchApiPathTemplate(issues, "/api/0/organizations/acme/issues/"),
+    ).toEqual({
+      parameters: { organization_id_or_slug: "acme" },
+      // "", "api", "0", "organizations", "issues", ""
+      literalSegments: 6,
+    });
+  });
+
+  it.each([
+    ["a different literal", "/api/0/projects/acme/issues/"],
+    ["an empty parameter", "/api/0/organizations//issues/"],
+    ["a missing trailing slash", "/api/0/organizations/acme/issues"],
+    ["an extra segment", "/api/0/organizations/acme/issues/1/"],
+  ])("rejects %s", (_, path) => {
+    expect(matchApiPathTemplate(issues, path)).toBeUndefined();
+  });
+
+  it("ranks a literal route above a parameter at the same position", () => {
+    const path = "/api/0/organizations/settings/";
+    const generic = matchApiPathTemplate(
+      "/api/0/organizations/{organization_id_or_slug}/",
+      path,
+    );
+    const literal = matchApiPathTemplate(path, path);
+    expect(literal!.literalSegments).toBeGreaterThan(generic!.literalSegments);
+  });
+
+  it("rejects a template that repeats a parameter name", () => {
+    expect(matchApiPathTemplate("/{id}/{id}/", "/a/b/")).toBeUndefined();
+  });
+});

packages/toolkit-core/src/api-path-template.ts

@@ -0,0 +1,42 @@
+/** A concrete API path matched against an OpenAPI path template. */
+export type ApiPathTemplateMatch = {
+  /** Path parameter values by name, as they appear in the path. */
+  parameters: Record<string, string>;
+  /** Segments that matched the template literally, for ranking overlaps. */
+  literalSegments: number;
+};
+
+const TEMPLATE_SEGMENT = /^\{([^{}]+)\}$/;
+
+/**
+ * Matches a concrete API path against an OpenAPI path template such as
+ * `/api/0/organizations/{organization_id_or_slug}/issues/`.
+ *
+ * Each `{name}` matches one non-empty segment; every other segment must be
+ * equal, and the segment counts must match, trailing slash included. Callers
+ * own decoding and validating segments first and choosing among overlapping
+ * templates, for which `literalSegments` ranks the most specific.
+ */
+export function matchApiPathTemplate(
+  template: string,
+  path: string,
+): ApiPathTemplateMatch | undefined {
+  const templateSegments = template.split("/");
+  const segments = path.split("/");
+  if (templateSegments.length !== segments.length) return;
+
+  const parameters: Record<string, string> = {};
+  let literalSegments = 0;
+  for (const [index, templateSegment] of templateSegments.entries()) {
+    const segment = segments[index] ?? "";
+    const name = TEMPLATE_SEGMENT.exec(templateSegment)?.[1];
+    if (name === undefined) {
+      if (templateSegment !== segment) return;
+      literalSegments++;
+    } else {
+      if (!segment || Object.hasOwn(parameters, name)) return;
+      parameters[name] = segment;
+    }
+  }
+  return { parameters, literalSegments };
+}