From 4cabef2745f7734f9d16b54e5cb1e49fd665a590 Mon Sep 17 00:00:00 2001 From: Akshay Dodeja Date: Thu, 25 Jun 2026 17:37:35 -0700 Subject: [PATCH 1/2] fix(mcp): annotate tools with readOnlyHint so reads aren't treated as edit tools MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tools were registered without MCP ToolAnnotations, so clients defaulted to treating every tool as mutating/destructive. This added friction for the 9 read-only tools and mis-categorized them as edit/write operations. Add an `annotations` field to all 10 tool registrations: Read-only (readOnlyHint: true, openWorldHint: true): search_container, get_container, get_container_route, get_container_transport_events, get_shipment_details, list_containers, list_shipments, list_tracking_requests Read-only, bounded catalog (readOnlyHint: true, openWorldHint: false): get_supported_shipping_lines Write (readOnlyHint: false, destructiveHint: false, idempotentHint: true, openWorldHint: true): track_container — a create, not destructive, and idempotent (re-tracking the same number returns the existing tracking request, no duplicate). Adds a regression test (annotations.test.ts) asserting each tool's hints, and a note in docs/mcp/home.mdx clarifying the read/write split. No tool behavior, schemas, or handlers changed — annotations only. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/mcp/home.mdx | 4 ++ packages/mcp/src/annotations.test.ts | 79 ++++++++++++++++++++++++++++ packages/mcp/src/server.ts | 10 ++++ 3 files changed, 93 insertions(+) create mode 100644 packages/mcp/src/annotations.test.ts diff --git a/docs/mcp/home.mdx b/docs/mcp/home.mdx index f13d5047..2893895b 100644 --- a/docs/mcp/home.mdx +++ b/docs/mcp/home.mdx @@ -109,6 +109,10 @@ Input and output recording is disabled by default. Leave `SENTRY_MCP_RECORD_INPU ## Tools reference + +Every tool is read-only except `track_container`, which creates a tracking request. `track_container` is idempotent—re-tracking a number that's already tracked returns the existing request instead of creating a duplicate. + + ### `search_container` Find containers by container number, BL, booking, or your own reference. This is the fastest way to locate containers. diff --git a/packages/mcp/src/annotations.test.ts b/packages/mcp/src/annotations.test.ts new file mode 100644 index 00000000..b367136f --- /dev/null +++ b/packages/mcp/src/annotations.test.ts @@ -0,0 +1,79 @@ +import { describe, expect, it, vi } from 'vitest'; +import { createTerminal49McpServer } from './server.js'; + +vi.mock('@sentry/node', () => ({ + captureException: vi.fn(), + flush: vi.fn().mockResolvedValue(true), + isInitialized: vi.fn(() => false), + wrapMcpServerWithSentry: vi.fn((server) => server), +})); + +type ToolAnnotations = { + readOnlyHint?: boolean; + destructiveHint?: boolean; + idempotentHint?: boolean; + openWorldHint?: boolean; +}; + +function getRegisteredTools(): Record< + string, + { annotations?: ToolAnnotations } +> { + const server = createTerminal49McpServer('token'); + return (server as any)._registeredTools as Record< + string, + { annotations?: ToolAnnotations } + >; +} + +describe('MCP tool annotations', () => { + const readOnlyTools = [ + 'search_container', + 'get_container', + 'get_container_route', + 'get_container_transport_events', + 'get_shipment_details', + 'get_supported_shipping_lines', + 'list_containers', + 'list_shipments', + 'list_tracking_requests', + ]; + + it('marks the nine read tools as read-only', () => { + const tools = getRegisteredTools(); + + for (const name of readOnlyTools) { + const annotations = tools[name]?.annotations; + expect(annotations, name).toBeDefined(); + expect(annotations?.readOnlyHint, name).toBe(true); + } + }); + + it('marks track_container as a non-destructive idempotent write', () => { + const tools = getRegisteredTools(); + const annotations = tools.track_container?.annotations; + + expect(annotations).toBeDefined(); + expect(annotations?.readOnlyHint).toBe(false); + expect(annotations?.destructiveHint).toBe(false); + expect(annotations?.idempotentHint).toBe(true); + expect(annotations?.openWorldHint).toBe(true); + }); + + it('marks get_supported_shipping_lines as a closed-world catalog', () => { + const tools = getRegisteredTools(); + const annotations = tools.get_supported_shipping_lines?.annotations; + + expect(annotations).toBeDefined(); + expect(annotations?.readOnlyHint).toBe(true); + expect(annotations?.openWorldHint).toBe(false); + }); + + it('annotates every registered tool', () => { + const tools = getRegisteredTools(); + + for (const [name, tool] of Object.entries(tools)) { + expect(tool.annotations, name).toBeDefined(); + } + }); +}); diff --git a/packages/mcp/src/server.ts b/packages/mcp/src/server.ts index 666ef3c5..a1a86ca5 100644 --- a/packages/mcp/src/server.ts +++ b/packages/mcp/src/server.ts @@ -633,6 +633,7 @@ export function createTerminal49McpServer( 'booking number, bill of lading, or reference number. ' + 'This is the fastest way to find container information. ' + 'Examples: CAIU2885402, MAEU123456789, or any reference number.', + annotations: { readOnlyHint: true, openWorldHint: true }, inputSchema: { query: z.string().min(1).describe('Search query - can be a container number, booking number, BL number, or reference number'), intent: toolIntentSchema, @@ -672,6 +673,7 @@ export function createTerminal49McpServer( 'Track a container, bill of lading, or booking number. ' + 'Uses inference to choose the carrier/type when possible, creates a tracking request, ' + 'and returns detailed container information.', + annotations: { readOnlyHint: false, destructiveHint: false, idempotentHint: true, openWorldHint: true }, inputSchema: { number: z.string().optional().describe('Container, bill of lading, or booking number to track'), numberType: z @@ -714,6 +716,7 @@ export function createTerminal49McpServer( 'Get container information with flexible data loading. Returns core container data (status, location, equipment, dates) ' + 'plus optional related data. Choose includes based on user question and container state. ' + 'Response includes metadata hints to guide follow-up queries.', + annotations: { readOnlyHint: true, openWorldHint: true }, inputSchema: { id: z.string().uuid().describe('The Terminal49 container ID (UUID format)'), include: z @@ -749,6 +752,7 @@ export function createTerminal49McpServer( 'Get detailed shipment information including routing, BOL, containers, and port details. ' + 'Use this when user asks about a shipment (vs a specific container). ' + 'Returns: Bill of Lading, shipping line, port details, vessel info, ETAs, container list.', + annotations: { readOnlyHint: true, openWorldHint: true }, inputSchema: { id: z.string().uuid().describe('The Terminal49 shipment ID (UUID format)'), include_containers: z.boolean().optional().default(true).describe('Include list of containers in this shipment. Default: true'), @@ -777,6 +781,7 @@ export function createTerminal49McpServer( '(vessel loaded, departed, arrived, discharged, rail movements, delivery). ' + 'Use this for questions about journey history, "what happened", timeline analysis, rail tracking. ' + 'More efficient than get_container with transport_events when you only need event data.', + annotations: { readOnlyHint: true, openWorldHint: true }, inputSchema: { id: z.string().uuid().describe('The Terminal49 container ID (UUID format)'), intent: toolIntentSchema, @@ -802,6 +807,7 @@ export function createTerminal49McpServer( 'Get list of shipping lines (carriers) supported by Terminal49 for container tracking. ' + 'Returns SCAC codes, full names, and common abbreviations. ' + 'Use this when user asks which carriers are supported or to validate a carrier name.', + annotations: { readOnlyHint: true, openWorldHint: false }, inputSchema: { search: z.string().optional().describe('Optional: Filter by carrier name or SCAC code'), intent: toolIntentSchema, @@ -841,6 +847,7 @@ export function createTerminal49McpServer( 'Shows complete multi-leg journey (origin → transshipment ports → destination). ' + 'NOTE: This is a paid feature and may not be available for all accounts. ' + 'Use for questions about routing, transshipments, or detailed vessel itinerary.', + annotations: { readOnlyHint: true, openWorldHint: true }, inputSchema: { id: z.string().uuid().describe('The Terminal49 container ID (UUID format)'), intent: toolIntentSchema, @@ -917,6 +924,7 @@ export function createTerminal49McpServer( description: 'List shipments with optional filters and pagination. ' + 'Use for queries like "show recent shipments" or "shipments for a carrier".', + annotations: { readOnlyHint: true, openWorldHint: true }, inputSchema: { status: z.string().optional().describe('Filter by shipment status'), port: z.string().optional().describe('Filter by POD port LOCODE'), @@ -951,6 +959,7 @@ export function createTerminal49McpServer( description: 'List containers with optional filters and pagination. ' + 'Use for queries like "containers at port" or "latest updates".', + annotations: { readOnlyHint: true, openWorldHint: true }, inputSchema: { status: z.string().optional().describe('Filter by container status'), port: z.string().optional().describe('Filter by POD port LOCODE'), @@ -985,6 +994,7 @@ export function createTerminal49McpServer( description: 'List tracking requests with optional filters and pagination. ' + 'Useful for monitoring recent tracking activity.', + annotations: { readOnlyHint: true, openWorldHint: true }, inputSchema: { filters: z .record(z.string(), z.string()) From 681eb04d31b1fcdd04f6a09aca2e1d5c623150a5 Mon Sep 17 00:00:00 2001 From: Akshay Dodeja Date: Fri, 26 Jun 2026 05:38:36 -0500 Subject: [PATCH 2/2] fix(mcp): drop inaccurate idempotentHint on track_container track_container is not idempotent: when no container is found via search it always creates a new tracking request, and a pending request (no linked container yet) won't be matched by search on a subsequent call, so repeated calls can create additional tracking requests. Set idempotentHint: false so clients don't treat the write as safe to silently retry/replay, and correct the docs note accordingly. Also harden annotations.test.ts: assert openWorldHint: true for the open-world read tools (excluding the closed-world catalog), and guard the "annotates every tool" check against _registeredTools silently resolving to an empty/undefined map if SDK internals change. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/mcp/home.mdx | 2 +- packages/mcp/src/annotations.test.ts | 24 ++++++++++++++++++++++-- packages/mcp/src/server.ts | 2 +- 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/docs/mcp/home.mdx b/docs/mcp/home.mdx index 2893895b..878c6e68 100644 --- a/docs/mcp/home.mdx +++ b/docs/mcp/home.mdx @@ -110,7 +110,7 @@ Input and output recording is disabled by default. Leave `SENTRY_MCP_RECORD_INPU ## Tools reference -Every tool is read-only except `track_container`, which creates a tracking request. `track_container` is idempotent—re-tracking a number that's already tracked returns the existing request instead of creating a duplicate. +Every tool is read-only except `track_container`, which creates a tracking request. If a container for the number is already in your account, `track_container` returns it instead of creating a new request; otherwise it creates a tracking request, so repeated calls before a container links can create additional requests. ### `search_container` diff --git a/packages/mcp/src/annotations.test.ts b/packages/mcp/src/annotations.test.ts index b367136f..598a9118 100644 --- a/packages/mcp/src/annotations.test.ts +++ b/packages/mcp/src/annotations.test.ts @@ -39,6 +39,10 @@ describe('MCP tool annotations', () => { 'list_tracking_requests', ]; + // get_supported_shipping_lines is a closed-world catalog; the other read + // tools query live shipment data and should be open-world. + const closedWorldReadTools = ['get_supported_shipping_lines']; + it('marks the nine read tools as read-only', () => { const tools = getRegisteredTools(); @@ -46,17 +50,21 @@ describe('MCP tool annotations', () => { const annotations = tools[name]?.annotations; expect(annotations, name).toBeDefined(); expect(annotations?.readOnlyHint, name).toBe(true); + + if (!closedWorldReadTools.includes(name)) { + expect(annotations?.openWorldHint, name).toBe(true); + } } }); - it('marks track_container as a non-destructive idempotent write', () => { + it('marks track_container as a non-destructive non-idempotent write', () => { const tools = getRegisteredTools(); const annotations = tools.track_container?.annotations; expect(annotations).toBeDefined(); expect(annotations?.readOnlyHint).toBe(false); expect(annotations?.destructiveHint).toBe(false); - expect(annotations?.idempotentHint).toBe(true); + expect(annotations?.idempotentHint).toBe(false); expect(annotations?.openWorldHint).toBe(true); }); @@ -72,6 +80,18 @@ describe('MCP tool annotations', () => { it('annotates every registered tool', () => { const tools = getRegisteredTools(); + // Guard against the SDK renaming/restructuring `_registeredTools`: if the + // map ever resolves to undefined or empty, the per-tool loop below would + // pass vacuously. Assert it actually contains the tools we expect first. + expect( + tools, + '_registeredTools is empty - SDK internals may have changed', + ).toBeTruthy(); + expect( + Object.keys(tools).length, + '_registeredTools is empty - SDK internals may have changed', + ).toBeGreaterThanOrEqual(readOnlyTools.length + 1); + for (const [name, tool] of Object.entries(tools)) { expect(tool.annotations, name).toBeDefined(); } diff --git a/packages/mcp/src/server.ts b/packages/mcp/src/server.ts index a1a86ca5..34ee5557 100644 --- a/packages/mcp/src/server.ts +++ b/packages/mcp/src/server.ts @@ -673,7 +673,7 @@ export function createTerminal49McpServer( 'Track a container, bill of lading, or booking number. ' + 'Uses inference to choose the carrier/type when possible, creates a tracking request, ' + 'and returns detailed container information.', - annotations: { readOnlyHint: false, destructiveHint: false, idempotentHint: true, openWorldHint: true }, + annotations: { readOnlyHint: false, destructiveHint: false, idempotentHint: false, openWorldHint: true }, inputSchema: { number: z.string().optional().describe('Container, bill of lading, or booking number to track'), numberType: z