From 86b8d3401acc63d559ba2a67aa4c05fd03408268 Mon Sep 17 00:00:00 2001 From: Matthew Podwysocki Date: Thu, 27 Aug 2026 13:49:59 -0400 Subject: [PATCH] fix: replace static_map_image_tool's mapbox://temp/ ref with a re-fetching inline-image ref Same hosted multi-task bug as directions_tool/isochrone_tool (#257): static_map_image_tool stashed large images in temporaryResourceManager (an in-process Map, 30-min TTL) behind a mapbox://temp/static-map-{id} ref, which a follow-up resources/read landing on a different ECS task couldn't see. That fix's JSON-in-ref approach doesn't transfer to images: base64- encoding raw bytes into a URI blows past the MCP SDK's own 1,000,000- character resource-URI cap almost immediately (a 750KB image alone encodes to 1,024,055 characters), and images needing this fallback can run into multiple megabytes. Instead, the new mapbox://inline-image/static-map?data=... ref encodes only the original request params (center, zoom, style, overlays - a few hundred bytes), and resources/read re-issues the Static Images API request at read time using those params and the reader's own resolved access token. The API is a deterministic renderer, so this reproduces the exact same image on demand instead of storing it - no size ceiling, works from any task. Extracted buildStaticMapRequestUrl.ts so the tool and the resource share the same URL-building logic. Verified with a real two-process integration test (fetch in one spawned server, kill it, re-fetch identical bytes from a second, independent process), and confirmed it fails against the old mapbox://temp/ code path before the fix and passes after. --- CHANGELOG.md | 4 + .../inline-image/InlineImageResource.ts | 102 +++++++++++++ src/resources/resourceRegistry.ts | 2 + .../StaticMapImageTool.ts | 127 +++------------- .../buildStaticMapRequestUrl.ts | 115 ++++++++++++++ src/utils/inlineImageRef.ts | 82 ++++++++++ .../hostedMultiTaskFollowupStaticMap.test.ts | 142 ++++++++++++++++++ .../inline-image/InlineImageResource.test.ts | 124 +++++++++++++++ .../StaticMapImageTool.test.ts | 37 +++-- test/utils/inlineImageRef.test.ts | 88 +++++++++++ 10 files changed, 706 insertions(+), 117 deletions(-) create mode 100644 src/resources/inline-image/InlineImageResource.ts create mode 100644 src/tools/static-map-image-tool/buildStaticMapRequestUrl.ts create mode 100644 src/utils/inlineImageRef.ts create mode 100644 test/integration/hostedMultiTaskFollowupStaticMap.test.ts create mode 100644 test/resources/inline-image/InlineImageResource.test.ts create mode 100644 test/utils/inlineImageRef.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index ca8c8125..876d8e99 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ ## Unreleased +### Fixes + +- **`static_map_image_tool`: large images no longer return a `mapbox://temp/...` ref that only exists on one hosted ECS task.** Same underlying bug as `directions_tool`/`isochrone_tool` (see the fast-follow entry above, or PR #257): images over the 700KB inline threshold were stashed in `temporaryResourceManager` (an in-process `Map`, 30-minute TTL) behind a `mapbox://temp/static-map-{id}` ref, which a follow-up `resources/read` landing on a different hosted ECS task couldn't see. Directly reusing the JSON-ref approach from that fix doesn't work here: base64-encoding raw image bytes into a URI blows past the MCP SDK's own hard 1,000,000-character resource-URI cap almost immediately (confirmed live — a 750KB image alone encodes to a 1,024,055-character URI), and images needing this fallback can run into multiple megabytes (e.g. 1280x1280 @2x). Instead, the new `mapbox://inline-image/static-map?data=...` ref encodes only the original request parameters (center, zoom, style, overlays, etc. — a few hundred bytes at most) and `resources/read` re-issues the Static Images API request at read time using those params and the reader's own resolved access token. The Static Images API is a deterministic renderer, so this reproduces the exact same image on demand rather than storing it, works from any task, and has no size ceiling — verified with a real two-process integration test that fetches the image in one spawned server, kills it, and re-fetches the identical bytes from a second, independent process. The one tradeoff: a follow-up read costs one extra Static Images API call instead of returning cached bytes. + ### New Features - **The server now identifies which MCP client connected to it.** `server.server.getClientVersion()` (populated from the `clientInfo` sent in the client's `initialize` request) is now logged on connect, e.g. `Client identified as: claude-ai v1.0.0` — useful for support/debugging when behavior differs across Claude Desktop, Cursor, VS Code, etc. Reading it required moving the read to a `server.server.oninitialized` callback rather than right after `server.connect()`, since `getClientVersion()`/`getClientCapabilities()` are only populated once the client's `initialize` request has actually been processed, which is not guaranteed by the time `connect()`'s promise resolves (it only waits for the transport to start). Confirmed live that reading capabilities immediately after `connect()` reliably returned `undefined` even for a client that declared them; moving both reads into `oninitialized` fixed the same-shaped bug in the existing capability-gated tool registration (currently dormant, since no tool is registered through that path yet, but was silently broken for whenever one is added). The client name/version is also recorded as `mcp.client.name`/`mcp.client.version` on every subsequent tool-execution trace span, so OTel-backed traces can be filtered or grouped by client. diff --git a/src/resources/inline-image/InlineImageResource.ts b/src/resources/inline-image/InlineImageResource.ts new file mode 100644 index 00000000..98e0fca0 --- /dev/null +++ b/src/resources/inline-image/InlineImageResource.ts @@ -0,0 +1,102 @@ +// Copyright (c) Mapbox, Inc. +// Licensed under the MIT License. + +import { BaseResource } from '../BaseResource.js'; +import type { RequestHandlerExtra } from '@modelcontextprotocol/sdk/shared/protocol.js'; +import type { + ReadResourceResult, + ServerNotification, + ServerRequest +} from '@modelcontextprotocol/sdk/types.js'; +import type { HttpRequest } from '../../utils/types.js'; +import { MapboxApiBasedTool } from '../../tools/MapboxApiBasedTool.js'; +import { resolveInlineImageRef } from '../../utils/inlineImageRef.js'; +import { StaticMapImageInputSchema } from '../../tools/static-map-image-tool/StaticMapImageTool.input.schema.js'; +import { + buildStaticMapRequestUrl, + getStaticMapMimeType +} from '../../tools/static-map-image-tool/buildStaticMapRequestUrl.js'; + +/** + * Resource for `mapbox://inline-image/{tool}?data=...` refs — see + * inlineImageRef.ts for why this exists and why it works this way. + * + * Unlike the other `inline-*` resources, `read()` doesn't just unwrap the + * ref: it re-issues the original Static Images API request with the ref's + * own params and the reader's own resolved access token, and returns the + * freshly fetched bytes. Nothing is kept in memory between the original + * tool call and the read, so this works identically no matter which + * process or hosted task serves the follow-up. + */ +export class InlineImageResource extends BaseResource { + readonly name = 'Inline Image Response'; + readonly uri = 'mapbox://inline-image/{spec}'; + readonly description = + "Stateless large static_map_image_tool response, re-fetched on demand from the ref's own request parameters."; + readonly mimeType = 'application/octet-stream'; + + private readonly httpRequest: HttpRequest; + + constructor(params: { httpRequest: HttpRequest }) { + super(); + this.httpRequest = params.httpRequest; + } + + async read( + uri: string, + extra?: RequestHandlerExtra + ): Promise { + const malformed: ReadResourceResult = { + contents: [ + { + uri, + mimeType: 'text/plain', + text: 'Inline image ref was malformed.' + } + ] + }; + + const resolved = resolveInlineImageRef(uri); + if (!resolved) return malformed; + + const accessToken = + (extra?.authInfo?.token as string | undefined) ?? + MapboxApiBasedTool.mapboxAccessToken; + if (!accessToken) { + return { + contents: [ + { + uri, + mimeType: 'text/plain', + text: 'No access token available to re-fetch this image. Provide a Bearer token or set MAPBOX_ACCESS_TOKEN.' + } + ] + }; + } + + const parsedInput = StaticMapImageInputSchema.safeParse(resolved.params); + if (!parsedInput.success) return malformed; + + const { url } = buildStaticMapRequestUrl(parsedInput.data, accessToken); + const response = await this.httpRequest(url); + if (!response.ok) { + return { + contents: [ + { + uri, + mimeType: 'text/plain', + text: `Failed to re-fetch image: ${response.status} ${response.statusText}` + } + ] + }; + } + + const buffer = await response.arrayBuffer(); + const mimeType = getStaticMapMimeType(parsedInput.data.style); + const base64Data = Buffer.from(buffer).toString('base64'); + + return { + contents: [{ uri, mimeType, blob: base64Data }] + }; + } +} diff --git a/src/resources/resourceRegistry.ts b/src/resources/resourceRegistry.ts index 9ac8dc77..652f78a7 100644 --- a/src/resources/resourceRegistry.ts +++ b/src/resources/resourceRegistry.ts @@ -6,6 +6,7 @@ import { CategoryListResource } from './category-list/CategoryListResource.js'; import { TemporaryDataResource } from './temporary/TemporaryDataResource.js'; import { ComputeResource } from './compute/ComputeResource.js'; import { InlinePayloadResource } from './inline-payload/InlinePayloadResource.js'; +import { InlineImageResource } from './inline-image/InlineImageResource.js'; import { MapAppUIResource } from './ui-apps/MapAppUIResource.js'; import { VersionResource } from './version/VersionResource.js'; import { httpRequest } from '../utils/httpPipeline.js'; @@ -17,6 +18,7 @@ export const ALL_RESOURCES = [ new TemporaryDataResource(), new ComputeResource(), new InlinePayloadResource(), + new InlineImageResource({ httpRequest }), // Single shared map renderer, targeted exclusively by render_map_tool. new MapAppUIResource({ httpRequest }), new VersionResource() diff --git a/src/tools/static-map-image-tool/StaticMapImageTool.ts b/src/tools/static-map-image-tool/StaticMapImageTool.ts index 60901af8..895bbb70 100644 --- a/src/tools/static-map-image-tool/StaticMapImageTool.ts +++ b/src/tools/static-map-image-tool/StaticMapImageTool.ts @@ -1,37 +1,24 @@ // Copyright (c) Mapbox, Inc. // Licensed under the MIT License. -import { randomUUID, randomBytes } from 'node:crypto'; +import { randomUUID } from 'node:crypto'; import type { z } from 'zod'; import { MapboxApiBasedTool } from '../MapboxApiBasedTool.js'; import type { HttpRequest } from '../../utils/types.js'; import { StaticMapImageInputSchema } from './StaticMapImageTool.input.schema.js'; -import type { OverlaySchema } from './StaticMapImageTool.input.schema.js'; import type { CallToolResult } from '@modelcontextprotocol/sdk/types.js'; -import { temporaryResourceManager } from '../../utils/temporaryResourceManager.js'; -import { getUserNameFromToken } from '../../utils/jwtUtils.js'; - -// Images larger than this threshold are stored as temporary resources instead -// of being inlined as base64, to avoid exceeding Claude Desktop's 1MB tool -// result limit. base64 adds ~33% overhead, so 700KB raw ≈ 933KB encoded. +import { buildInlineImageRef } from '../../utils/inlineImageRef.js'; +import { + buildStaticMapRequestUrl, + getStaticMapMimeType +} from './buildStaticMapRequestUrl.js'; + +// Images larger than this threshold fall back to a self-describing +// mapbox://inline-image/ ref instead of being inlined as base64, to avoid +// exceeding Claude Desktop's 1MB tool result limit. base64 adds ~33% +// overhead, so 700KB raw ≈ 933KB encoded. const IMAGE_INLINE_THRESHOLD = 700 * 1024; // 700KB -// encodeURIComponent leaves (, ), !, ', and * unescaped (they're valid in a -// URI component per RFC 3986's "unreserved" carve-out from the older -// escape() behaviour). Every overlay value below is embedded inside a -// path segment delimited by literal parentheses (e.g. `url-(lon,lat)`, -// `geojson()`), so a raw `)` in the value can terminate that segment -// early from the Static Images API's own overlay-syntax parser's point of -// view, even though this value already passed URL/JSON validation on our -// side. Escape those characters explicitly so both parsers agree on where -// the value actually ends. -function encodeOverlayComponent(value: string): string { - return encodeURIComponent(value).replace( - /[()!'*]/g, - (c) => '%' + c.charCodeAt(0).toString(16).toUpperCase() - ); -} - export class StaticMapImageTool extends MapboxApiBasedTool< typeof StaticMapImageInputSchema > { @@ -53,79 +40,11 @@ export class StaticMapImageTool extends MapboxApiBasedTool< }); } - private encodeOverlay(overlay: z.infer): string { - switch (overlay.type) { - case 'marker': { - const size = overlay.size === 'large' ? 'pin-l' : 'pin-s'; - let marker = size; - - if (overlay.label) { - marker += `-${overlay.label}`; - } - - if (overlay.color) { - marker += `+${overlay.color}`; - } - - return `${marker}(${overlay.longitude},${overlay.latitude})`; - } - - case 'custom-marker': { - const encodedUrl = encodeOverlayComponent(overlay.url); - return `url-${encodedUrl}(${overlay.longitude},${overlay.latitude})`; - } - - case 'path': { - let path = `path-${overlay.strokeWidth}`; - - if (overlay.strokeColor) { - path += `+${overlay.strokeColor}`; - if (overlay.strokeOpacity !== undefined) { - path += `-${overlay.strokeOpacity}`; - } - } - - if (overlay.fillColor) { - path += `+${overlay.fillColor}`; - if (overlay.fillOpacity !== undefined) { - path += `-${overlay.fillOpacity}`; - } - } - - // URL encode the polyline to handle special characters - return `${path}(${encodeOverlayComponent(overlay.encodedPolyline)})`; - } - - case 'geojson': { - const geojsonString = JSON.stringify(overlay.data); - return `geojson(${encodeOverlayComponent(geojsonString)})`; - } - } - } - protected async execute( input: z.infer, accessToken: string ): Promise { - const { longitude: lng, latitude: lat } = input.center; - const { width, height } = input.size; - - // Build overlay string - let overlayString = ''; - if (input.overlays && input.overlays.length > 0) { - const encodedOverlays = input.overlays.map((overlay) => { - return this.encodeOverlay(overlay); - }); - overlayString = encodedOverlays.join(',') + '/'; - } - - const density = input.highDensity ? '@2x' : ''; - const encodedStyle = input.style - .split('/') - .map(encodeURIComponent) - .join('/'); - const publicUrl = `${MapboxApiBasedTool.mapboxApiEndpoint}styles/v1/${encodedStyle}/static/${overlayString}${lng},${lat},${input.zoom}/${width}x${height}${density}`; - const url = `${publicUrl}?access_token=${accessToken}`; + const { publicUrl, url } = buildStaticMapRequestUrl(input, accessToken); // Fetch image const response = await this.httpRequest(url); @@ -137,8 +56,7 @@ export class StaticMapImageTool extends MapboxApiBasedTool< }; } const buffer = await response.arrayBuffer(); - const isRasterStyle = input.style.includes('satellite'); - const mimeType = isRasterStyle ? 'image/jpeg' : 'image/png'; + const mimeType = getStaticMapMimeType(input.style); // Use public URL (without credentials) to avoid leaking the access token const content: CallToolResult['content'] = [ @@ -146,21 +64,14 @@ export class StaticMapImageTool extends MapboxApiBasedTool< ]; if (buffer.byteLength > IMAGE_INLINE_THRESHOLD) { - // Image is too large to inline safely — store as temporary resource - const resourceId = randomBytes(16).toString('hex'); - const resourceUri = `mapbox://temp/static-map-${resourceId}`; - const base64Data = Buffer.from(buffer).toString('base64'); - temporaryResourceManager.create({ - id: resourceId, - uri: resourceUri, - data: base64Data, - metadata: { toolName: this.name, size: buffer.byteLength }, - mimeType, - owner: getUserNameFromToken(accessToken) - }); + // Image is too large to inline safely. Ref encodes the request params, + // not the image bytes — see inlineImageRef.ts for why (a raw-bytes ref + // for an image this size would itself exceed the MCP SDK's own + // resource-URI length cap). + const resourceUri = buildInlineImageRef('static-map', input); content.push({ type: 'text', - text: `⚠️ Image (${Math.round(buffer.byteLength / 1024)}KB) stored as temporary resource.\nResource URI: ${resourceUri}\nTTL: 30 minutes` + text: `⚠️ Image (${Math.round(buffer.byteLength / 1024)}KB) exceeds the inline size limit.\n\nFetch it via the MCP resources API — this re-renders the same deterministic image on demand, so it works from any server instance and there is nothing to wait for.\nResource URI: ${resourceUri}` }); } else { // Image is small enough to inline as base64 diff --git a/src/tools/static-map-image-tool/buildStaticMapRequestUrl.ts b/src/tools/static-map-image-tool/buildStaticMapRequestUrl.ts new file mode 100644 index 00000000..cdbc34ba --- /dev/null +++ b/src/tools/static-map-image-tool/buildStaticMapRequestUrl.ts @@ -0,0 +1,115 @@ +// Copyright (c) Mapbox, Inc. +// Licensed under the MIT License. + +import type { z } from 'zod'; +import { MapboxApiBasedTool } from '../MapboxApiBasedTool.js'; +import type { + StaticMapImageInputSchema, + OverlaySchema +} from './StaticMapImageTool.input.schema.js'; + +// encodeURIComponent leaves (, ), !, ', and * unescaped (they're valid in a +// URI component per RFC 3986's "unreserved" carve-out from the older +// escape() behaviour). Every overlay value below is embedded inside a +// path segment delimited by literal parentheses (e.g. `url-(lon,lat)`, +// `geojson()`), so a raw `)` in the value can terminate that segment +// early from the Static Images API's own overlay-syntax parser's point of +// view, even though this value already passed URL/JSON validation on our +// side. Escape those characters explicitly so both parsers agree on where +// the value actually ends. +export function encodeOverlayComponent(value: string): string { + return encodeURIComponent(value).replace( + /[()!'*]/g, + (c) => '%' + c.charCodeAt(0).toString(16).toUpperCase() + ); +} + +export function encodeOverlay(overlay: z.infer): string { + switch (overlay.type) { + case 'marker': { + const size = overlay.size === 'large' ? 'pin-l' : 'pin-s'; + let marker = size; + + if (overlay.label) { + marker += `-${overlay.label}`; + } + + if (overlay.color) { + marker += `+${overlay.color}`; + } + + return `${marker}(${overlay.longitude},${overlay.latitude})`; + } + + case 'custom-marker': { + const encodedUrl = encodeOverlayComponent(overlay.url); + return `url-${encodedUrl}(${overlay.longitude},${overlay.latitude})`; + } + + case 'path': { + let path = `path-${overlay.strokeWidth}`; + + if (overlay.strokeColor) { + path += `+${overlay.strokeColor}`; + if (overlay.strokeOpacity !== undefined) { + path += `-${overlay.strokeOpacity}`; + } + } + + if (overlay.fillColor) { + path += `+${overlay.fillColor}`; + if (overlay.fillOpacity !== undefined) { + path += `-${overlay.fillOpacity}`; + } + } + + // URL encode the polyline to handle special characters + return `${path}(${encodeOverlayComponent(overlay.encodedPolyline)})`; + } + + case 'geojson': { + const geojsonString = JSON.stringify(overlay.data); + return `geojson(${encodeOverlayComponent(geojsonString)})`; + } + } +} + +export function getStaticMapMimeType( + style: string +): 'image/jpeg' | 'image/png' { + return style.includes('satellite') ? 'image/jpeg' : 'image/png'; +} + +export interface StaticMapRequestUrls { + /** The request URL without the access token, safe to surface to a client. */ + publicUrl: string; + /** The full request URL including the access token, for the server's own fetch. */ + url: string; +} + +/** + * Pure function of (input, accessToken) — the Static Images API is a + * deterministic renderer, so the same inputs always produce the same image. + * Shared between `StaticMapImageTool` (the original request) and + * `InlineImageResource` (re-fetching a large image on a follow-up read; see + * inlineImageRef.ts for why re-fetching rather than storing the bytes). + */ +export function buildStaticMapRequestUrl( + input: z.infer, + accessToken: string +): StaticMapRequestUrls { + const { longitude: lng, latitude: lat } = input.center; + const { width, height } = input.size; + + let overlayString = ''; + if (input.overlays && input.overlays.length > 0) { + overlayString = input.overlays.map(encodeOverlay).join(',') + '/'; + } + + const density = input.highDensity ? '@2x' : ''; + const encodedStyle = input.style.split('/').map(encodeURIComponent).join('/'); + const publicUrl = `${MapboxApiBasedTool.mapboxApiEndpoint}styles/v1/${encodedStyle}/static/${overlayString}${lng},${lat},${input.zoom}/${width}x${height}${density}`; + const url = `${publicUrl}?access_token=${accessToken}`; + + return { publicUrl, url }; +} diff --git a/src/utils/inlineImageRef.ts b/src/utils/inlineImageRef.ts new file mode 100644 index 00000000..8b098b42 --- /dev/null +++ b/src/utils/inlineImageRef.ts @@ -0,0 +1,82 @@ +// Copyright (c) Mapbox, Inc. +// Licensed under the MIT License. + +const INLINE_IMAGE_URI_PREFIX = 'mapbox://inline-image/'; + +const INLINE_IMAGE_TOOLS = ['static-map'] as const; +export type InlineImageTool = (typeof INLINE_IMAGE_TOOLS)[number]; + +export interface InlineImageRef { + tool: InlineImageTool; + params: Record; +} + +/** + * Self-describing ref for `static_map_image_tool`'s large-image fallback. + * Same motivation as `inlineResponseRef.ts` (the old `mapbox://temp/` ref + * was backed by an in-process `Map`, which the hosted deployment's multiple + * stateless ECS tasks don't share), but a different mechanism: this embeds + * only the original *request params*, not the image bytes. + * + * The Static Images API is a deterministic renderer of + * (style, center, zoom, size, overlays) — re-issuing the same request always + * reproduces the same image byte-for-byte — so there's nothing to gain by + * storing or transmitting the output at all. That's also a hard requirement, + * not just an optimization: embedding raw bytes the way `inlineResponseRef` + * does for JSON works there because those responses are at most a few + * hundred KB, but this tool's fallback only triggers above 700KB, and a + * single-pass base64 encoding of an image that size alone already produces + * a URI over the MCP SDK's hard 1,000,000-character resource-URI cap — + * confirmed live (a 750KB image encoded to a 1,024,055-character URI and + * the SDK rejected the read). Images can run into multiple megabytes (e.g. + * 1280x1280 @2x), so there's no realistic threshold below which encoding + * the bytes directly would be safe. Re-fetching the (tiny) params instead + * of storing the (large) output sidesteps the size limit entirely, at the + * cost of one extra Static Images API call per follow-up read — see + * `InlineImageResource.read()`, which performs that re-fetch. + */ +export function buildInlineImageRef( + tool: InlineImageTool, + params: Record +): string { + const data = Buffer.from(JSON.stringify(params), 'utf8').toString( + 'base64url' + ); + return `${INLINE_IMAGE_URI_PREFIX}${tool}?data=${data}`; +} + +export function isInlineImageRef(uri: string): boolean { + return uri.startsWith(INLINE_IMAGE_URI_PREFIX); +} + +/** + * Unwrap a `mapbox://inline-image/...` ref back into its tool name and + * request params. Returns null if the ref isn't this scheme, names an + * unrecognized tool, or its data doesn't decode to a JSON object. + */ +export function resolveInlineImageRef(uri: string): InlineImageRef | null { + if (!isInlineImageRef(uri)) return null; + + let tool: string; + let encoded: string | null; + try { + const parsed = new URL(uri); + tool = parsed.pathname.replace(/^\//, ''); + encoded = parsed.searchParams.get('data'); + } catch { + return null; + } + + if (!INLINE_IMAGE_TOOLS.includes(tool as InlineImageTool)) return null; + if (!encoded) return null; + + try { + const params = JSON.parse( + Buffer.from(encoded, 'base64url').toString('utf8') + ); + if (!params || typeof params !== 'object') return null; + return { tool: tool as InlineImageTool, params }; + } catch { + return null; + } +} diff --git a/test/integration/hostedMultiTaskFollowupStaticMap.test.ts b/test/integration/hostedMultiTaskFollowupStaticMap.test.ts new file mode 100644 index 00000000..ccc00d8c --- /dev/null +++ b/test/integration/hostedMultiTaskFollowupStaticMap.test.ts @@ -0,0 +1,142 @@ +// Copyright (c) Mapbox, Inc. +// Licensed under the MIT License. + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { existsSync } from 'node:fs'; +import { join, dirname } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { createServer, type Server } from 'node:http'; +import { Client } from '@modelcontextprotocol/sdk/client/index.js'; +import { StdioClientTransport } from '@modelcontextprotocol/sdk/client/stdio.js'; + +/** + * Same failure mode as hostedMultiTaskFollowup.test.ts, for + * `static_map_image_tool`'s large-image fallback: a response over the 700KB + * inline threshold used to be stored via `temporaryResourceManager` (an + * in-process `Map`) and returned as a `mapbox://temp/static-map-{id}` ref. + * On the hosted deployment's stateless, multi-ECS-task setup, a follow-up + * `resources/read` landing on a different task than the one that fetched + * the image found nothing. + * + * This proves the fix (a self-describing `mapbox://inline-image/` ref — see + * inlineImageRef.ts) survives exactly that: the ref is produced by one real, + * spawned server process, that process is killed outright, and a second, + * completely independent process re-fetches the image from the ref's own + * request params via a real `resources/read` call — no bytes, store, or + * memory of any kind carried over from the original process. + */ + +const __dirname = dirname(fileURLToPath(import.meta.url)); +const SERVER_ENTRY = join(__dirname, '..', '..', 'dist', 'esm', 'index.js'); + +const DUMMY_TOKEN = 'eyJhbGciOiJIUzI1NiJ9.eyJzdWIiOiJ0ZXN0In0.signature'; + +// Larger than the tool's 700KB inline threshold, and filled with a +// non-trivial byte pattern so a truncation or off-by-one bug in the +// encode/decode path would be caught by an exact equality check. +const BIG_IMAGE = Buffer.alloc(750 * 1024); +for (let i = 0; i < BIG_IMAGE.length; i++) { + BIG_IMAGE[i] = i % 256; +} + +let mockStaticApi: Server; +let mockApiEndpoint: string; + +beforeAll(async () => { + mockStaticApi = createServer((_req, res) => { + res.writeHead(200, { 'content-type': 'image/png' }); + res.end(BIG_IMAGE); + }); + await new Promise((resolve) => { + mockStaticApi.listen(0, '127.0.0.1', resolve); + }); + const address = mockStaticApi.address(); + if (!address || typeof address === 'string') { + throw new Error('Failed to bind mock Static Images API server'); + } + mockApiEndpoint = `http://127.0.0.1:${address.port}/`; +}); + +afterAll(async () => { + await new Promise((resolve, reject) => { + mockStaticApi.close((err) => (err ? reject(err) : resolve())); + }); +}); + +async function spawnFreshServerClient(): Promise { + const transport = new StdioClientTransport({ + command: process.execPath, + args: [SERVER_ENTRY], + env: { + MAPBOX_ACCESS_TOKEN: DUMMY_TOKEN, + MAPBOX_API_ENDPOINT: mockApiEndpoint, + PATH: process.env.PATH ?? '' + } + }); + const client = new Client({ + name: 'hosted-multi-task-followup-static-map-test', + version: '1.0.0' + }); + await client.connect(transport); + return client; +} + +async function closeClient(client: Client): Promise { + await client.close().catch(() => { + // Already closed or the process exited on its own. + }); +} + +describe.skipIf(!existsSync(SERVER_ENTRY))( + 'hosted stateless follow-up for static_map_image_tool (real server process, real MCP protocol, simulated two-task deployment)', + () => { + it('reads a large static_map_image_tool inline-image ref from a completely independent process, with zero shared state', async () => { + const taskA = await spawnFreshServerClient(); + let resourceUri: string | undefined; + + try { + const result = await taskA.callTool({ + name: 'static_map_image_tool', + arguments: { + center: { longitude: -74.006, latitude: 40.7128 }, + zoom: 12, + size: { width: 1280, height: 900 }, + style: 'mapbox/streets-v12' + } + }); + expect(result.isError).toBeFalsy(); + + const textBlock = ( + result.content as Array<{ type: string; text?: string }> + ).find((c) => c.type === 'text' && c.text?.includes('inline-image')); + expect(textBlock?.text).toContain('exceeds the inline size limit'); + + resourceUri = textBlock!.text!.match( + /mapbox:\/\/inline-image\/\S+/ + )?.[0]; + expect(resourceUri).toBeTruthy(); + } finally { + // Kill task A entirely — no shared memory, no shared temp-file + // store, nothing carried over except the ref string itself. + await closeClient(taskA); + } + + // Process B: a completely independent process/task that never saw + // the original call, standing in for the ALB routing the follow-up + // resources/read to a different ECS task. + const taskB = await spawnFreshServerClient(); + try { + const readResult = await taskB.readResource({ uri: resourceUri! }); + const contentItem = ( + readResult.contents as Array<{ mimeType?: string; blob?: string }> + )[0]; + expect(contentItem?.mimeType).toBe('image/png'); + expect(Buffer.from(contentItem!.blob as string, 'base64')).toEqual( + BIG_IMAGE + ); + } finally { + await closeClient(taskB); + } + }); + } +); diff --git a/test/resources/inline-image/InlineImageResource.test.ts b/test/resources/inline-image/InlineImageResource.test.ts new file mode 100644 index 00000000..b4a9f86f --- /dev/null +++ b/test/resources/inline-image/InlineImageResource.test.ts @@ -0,0 +1,124 @@ +// Copyright (c) Mapbox, Inc. +// Licensed under the MIT License. + +import { describe, it, expect, afterEach, vi } from 'vitest'; +import { setupHttpRequest } from '../../utils/httpPipelineUtils.js'; +import { InlineImageResource } from '../../../src/resources/inline-image/InlineImageResource.js'; +import { buildInlineImageRef } from '../../../src/utils/inlineImageRef.js'; + +const TOKEN = 'eyJhbGciOiJIUzI1NiJ9.eyJzdWIiOiJ0ZXN0In0.signature'; + +describe('InlineImageResource', () => { + afterEach(() => { + vi.restoreAllMocks(); + vi.unstubAllEnvs(); + }); + + it("re-fetches the image from the ref's own params via the real resources/read path with zero prior server-side state — the actual fix for the hosted multi-task follow-up bug", async () => { + const imageBytes = new Uint8Array([137, 80, 78, 71, 1, 2, 3]); + const { httpRequest, mockHttpRequest } = setupHttpRequest({ + arrayBuffer: async () => imageBytes.buffer + }); + + const resource = new InlineImageResource({ httpRequest }); + const ref = buildInlineImageRef('static-map', { + center: { longitude: -74.006, latitude: 40.7128 }, + zoom: 12, + size: { width: 1280, height: 900 }, + style: 'mapbox/streets-v12' + }); + + // No store, no cache, no owner check — this re-issues the request using + // the ref's own params, so it succeeds identically whether it's the same + // process, a restarted one, or a different hosted ECS task than the one + // that handled the original tool call. + const result = await resource.read(ref, { + authInfo: { token: TOKEN } + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any); + + expect(mockHttpRequest).toHaveBeenCalledTimes(1); + const calledUrl = mockHttpRequest.mock.calls[0][0].toString(); + expect(calledUrl).toContain('styles/v1/mapbox/streets-v12/static/'); + expect(calledUrl).toContain(`access_token=${TOKEN}`); + + expect(result.contents[0].mimeType).toBe('image/png'); + expect(Buffer.from(result.contents[0].blob as string, 'base64')).toEqual( + Buffer.from(imageBytes) + ); + }); + + it("picks the jpeg mime type for satellite styles, matching the tool's own behavior", async () => { + const { httpRequest } = setupHttpRequest({ + arrayBuffer: async () => new Uint8Array([1, 2, 3]).buffer + }); + const resource = new InlineImageResource({ httpRequest }); + const ref = buildInlineImageRef('static-map', { + center: { longitude: 0, latitude: 0 }, + zoom: 5, + size: { width: 600, height: 400 }, + style: 'mapbox/satellite-v9' + }); + + const result = await resource.read(ref, { + authInfo: { token: TOKEN } + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any); + + expect(result.contents[0].mimeType).toBe('image/jpeg'); + }); + + it('returns a text/plain explanation (not a crash) for a malformed ref', async () => { + const { httpRequest } = setupHttpRequest(); + const resource = new InlineImageResource({ httpRequest }); + const result = await resource.read( + 'mapbox://inline-image/static-map?data=!!!' + ); + + expect(result.contents[0].mimeType).toBe('text/plain'); + expect(result.contents[0].text).toContain('malformed'); + }); + + it('returns a text/plain explanation when no access token is available', async () => { + // Other test files in this suite set process.env.MAPBOX_ACCESS_TOKEN at + // module load time; stub it away explicitly rather than relying on it + // being unset. + vi.stubEnv('MAPBOX_ACCESS_TOKEN', undefined); + const { httpRequest } = setupHttpRequest(); + const resource = new InlineImageResource({ httpRequest }); + const ref = buildInlineImageRef('static-map', { + center: { longitude: 0, latitude: 0 }, + zoom: 5, + size: { width: 600, height: 400 }, + style: 'mapbox/streets-v12' + }); + + const result = await resource.read(ref); + + expect(result.contents[0].mimeType).toBe('text/plain'); + expect(result.contents[0].text).toContain('access token'); + }); + + it('returns a text/plain explanation when the re-fetch fails upstream', async () => { + const { httpRequest } = setupHttpRequest({ + ok: false, + status: 401, + statusText: 'Unauthorized' + }); + const resource = new InlineImageResource({ httpRequest }); + const ref = buildInlineImageRef('static-map', { + center: { longitude: 0, latitude: 0 }, + zoom: 5, + size: { width: 600, height: 400 }, + style: 'mapbox/streets-v12' + }); + + const result = await resource.read(ref, { + authInfo: { token: TOKEN } + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any); + + expect(result.contents[0].mimeType).toBe('text/plain'); + expect(result.contents[0].text).toContain('401'); + }); +}); diff --git a/test/tools/static-map-image-tool/StaticMapImageTool.test.ts b/test/tools/static-map-image-tool/StaticMapImageTool.test.ts index 6e9a7e37..7a1230c9 100644 --- a/test/tools/static-map-image-tool/StaticMapImageTool.test.ts +++ b/test/tools/static-map-image-tool/StaticMapImageTool.test.ts @@ -8,7 +8,7 @@ import { describe, it, expect, afterEach, vi } from 'vitest'; import { setupHttpRequest } from '../../utils/httpPipelineUtils.js'; import { CustomMarkerOverlaySchema } from '../../../src/tools/static-map-image-tool/StaticMapImageTool.input.schema.js'; import { StaticMapImageTool } from '../../../src/tools/static-map-image-tool/StaticMapImageTool.js'; -import { temporaryResourceManager } from '../../../src/utils/temporaryResourceManager.js'; +import { resolveInlineImageRef } from '../../../src/utils/inlineImageRef.js'; describe('StaticMapImageTool', () => { afterEach(() => { @@ -245,15 +245,14 @@ describe('StaticMapImageTool', () => { }); }); - it('stores large images as temporary resources instead of inlining base64', async () => { + it('falls back to a self-describing inline-image ref (not mapbox://temp/) instead of inlining base64', async () => { // Simulate a response larger than the 700KB inline threshold const largeBuffer = new ArrayBuffer(750 * 1024); + new Uint8Array(largeBuffer).fill(7); const { httpRequest } = setupHttpRequest({ arrayBuffer: async () => largeBuffer } as Partial); - temporaryResourceManager.clear(); - const result = await new StaticMapImageTool({ httpRequest }).run({ center: { longitude: -74.006, latitude: 40.7128 }, zoom: 12, @@ -264,17 +263,37 @@ describe('StaticMapImageTool', () => { expect(result.isError).toBe(false); // Should not contain an inline image expect(result.content.some((c) => c.type === 'image')).toBe(false); - // Should contain a text item with the resource URI + expect( + result.content.every((c) => c.type !== 'text' || !c.text.includes('TTL')) + ).toBe(true); + const resourceText = result.content.find( (c) => c.type === 'text' && (c as { type: 'text'; text: string }).text.includes( - 'mapbox://temp/static-map-' + 'mapbox://inline-image/static-map' ) - ); + ) as { type: 'text'; text: string } | undefined; expect(resourceText).toBeDefined(); - // Temp resource should be registered - expect(temporaryResourceManager.count()).toBe(1); + expect(resourceText!.text).not.toContain('mapbox://temp/'); + + // No temporaryResourceManager, no owner, no store involved at all — the + // ref encodes the (tiny) request params rather than the image bytes, so + // resolving it is a pure function of the URI string regardless of how + // large the actual image was. + const uri = resourceText!.text.match( + /mapbox:\/\/inline-image\/static-map\?\S+/ + )?.[0]; + expect(uri).toBeTruthy(); + expect((uri as string).length).toBeLessThan(1000); + const resolved = resolveInlineImageRef(uri as string); + expect(resolved?.tool).toBe('static-map'); + expect(resolved?.params).toMatchObject({ + center: { longitude: -74.006, latitude: 40.7128 }, + zoom: 12, + size: { width: 1280, height: 900 }, + style: 'mapbox/streets-v12' + }); }); it('returns error when Mapbox API returns non-2xx response', async () => { diff --git a/test/utils/inlineImageRef.test.ts b/test/utils/inlineImageRef.test.ts new file mode 100644 index 00000000..e1f36036 --- /dev/null +++ b/test/utils/inlineImageRef.test.ts @@ -0,0 +1,88 @@ +// Copyright (c) Mapbox, Inc. +// Licensed under the MIT License. + +import { describe, it, expect } from 'vitest'; +import { + buildInlineImageRef, + resolveInlineImageRef, + isInlineImageRef +} from '../../src/utils/inlineImageRef.js'; + +describe('inlineImageRef', () => { + it('round-trips request params with no server-side state involved', () => { + const params = { + center: { longitude: -74.006, latitude: 40.7128 }, + zoom: 12, + size: { width: 1280, height: 900 }, + style: 'mapbox/streets-v12' + }; + const ref = buildInlineImageRef('static-map', params); + expect(ref).toMatch(/^mapbox:\/\/inline-image\/static-map\?data=/); + + const resolved = resolveInlineImageRef(ref); + expect(resolved?.tool).toBe('static-map'); + expect(resolved?.params).toEqual(params); + }); + + it('encodes only the (tiny) request params, never the image bytes — the ref stays far under the MCP SDK 1,000,000-char URI cap regardless of how large the eventual image is', () => { + const ref = buildInlineImageRef('static-map', { + center: { longitude: -74.006, latitude: 40.7128 }, + zoom: 12, + size: { width: 1280, height: 1280 }, + style: 'mapbox/streets-v12', + highDensity: true + }); + expect(ref.length).toBeLessThan(1000); + }); + + it('is resolvable independently of any prior process state (simulated restart, or a different hosted task)', () => { + const ref = buildInlineImageRef('static-map', { + center: { longitude: 0, latitude: 0 }, + zoom: 1, + size: { width: 100, height: 100 }, + style: 'mapbox/streets-v12' + }); + const first = resolveInlineImageRef(ref); + const second = resolveInlineImageRef(ref); + expect(first).toEqual(second); + }); + + it('returns null for a malformed data param', () => { + expect( + resolveInlineImageRef( + 'mapbox://inline-image/static-map?data=not-base64!!' + ) + ).toBeNull(); + }); + + it('returns null when the data param is missing', () => { + expect( + resolveInlineImageRef('mapbox://inline-image/static-map') + ).toBeNull(); + }); + + it('returns null for an unrecognized tool', () => { + const ref = buildInlineImageRef('static-map', {}).replace( + 'static-map', + 'bogus' + ); + expect(resolveInlineImageRef(ref)).toBeNull(); + }); + + it('returns null for a non-inline-image uri', () => { + expect(resolveInlineImageRef('mapbox://temp/static-map-abc')).toBeNull(); + expect( + resolveInlineImageRef('mapbox://inline-response/directions?data=abc') + ).toBeNull(); + }); + + it('isInlineImageRef distinguishes inline-image refs from other schemes', () => { + expect(isInlineImageRef('mapbox://inline-image/static-map?data=abc')).toBe( + true + ); + expect(isInlineImageRef('mapbox://temp/static-map-abc')).toBe(false); + expect( + isInlineImageRef('mapbox://inline-response/directions?data=abc') + ).toBe(false); + }); +});