From 82f08360d7d3a5f86087cb74543ff7326a4bbbdb Mon Sep 17 00:00:00 2001 From: jubnl Date: Sun, 5 Apr 2026 15:36:14 +0200 Subject: [PATCH] fix(mcp): route search_place through mapsService to support Google Maps The search_place MCP tool was hardcoding a direct Nominatim call, ignoring any configured Google Maps API key and never returning google_place_id despite the tool description advertising it. Replace the inline fetch with the existing searchPlaces() service which already switches between Google and Nominatim. Update unit tests to mock mapsService instead of global fetch, and add a dedicated test case for the Google path returning google_place_id. Closes #424 --- server/src/mcp/tools.ts | 24 +++------- server/tests/unit/mcp/tools-places.test.ts | 53 ++++++++++++++-------- 2 files changed, 39 insertions(+), 38 deletions(-) diff --git a/server/src/mcp/tools.ts b/server/src/mcp/tools.ts index ab542be..b2dbd8a 100644 --- a/server/src/mcp/tools.ts +++ b/server/src/mcp/tools.ts @@ -22,6 +22,7 @@ import { createNote as createCollabNote, updateNote as updateCollabNote, deleteN import { markCountryVisited, unmarkCountryVisited, createBucketItem, deleteBucketItem, } from '../services/atlasService'; +import { searchPlaces } from '../services/mapsService'; const MAX_MCP_TRIP_DAYS = 90; @@ -235,25 +236,12 @@ export function registerTools(server: McpServer, userId: number): void { }, }, async ({ query }) => { - // Use Nominatim (no API key needed, always available) - const params = new URLSearchParams({ - q: query, format: 'json', addressdetails: '1', limit: '5', 'accept-language': 'en', - }); - const response = await fetch(`https://nominatim.openstreetmap.org/search?${params}`, { - headers: { 'User-Agent': 'TREK Travel Planner' }, - }); - if (!response.ok) { - return { content: [{ type: 'text' as const, text: 'Search failed — Nominatim API error.' }], isError: true }; + try { + const result = await searchPlaces(userId, query); + return ok(result); + } catch { + return { content: [{ type: 'text' as const, text: 'Place search failed.' }], isError: true }; } - const data = await response.json() as { osm_type: string; osm_id: number; name: string; display_name: string; lat: string; lon: string }[]; - const places = data.map(item => ({ - osm_id: `${item.osm_type}:${item.osm_id}`, - name: item.name || item.display_name?.split(',')[0] || '', - address: item.display_name || '', - lat: parseFloat(item.lat) || null, - lng: parseFloat(item.lon) || null, - })); - return ok({ places }); } ); diff --git a/server/tests/unit/mcp/tools-places.test.ts b/server/tests/unit/mcp/tools-places.test.ts index aa05833..60a594b 100644 --- a/server/tests/unit/mcp/tools-places.test.ts +++ b/server/tests/unit/mcp/tools-places.test.ts @@ -37,6 +37,9 @@ vi.mock('../../../src/config', () => ({ const { broadcastMock } = vi.hoisted(() => ({ broadcastMock: vi.fn() })); vi.mock('../../../src/websocket', () => ({ broadcast: broadcastMock })); +const { searchPlacesMock } = vi.hoisted(() => ({ searchPlacesMock: vi.fn() })); +vi.mock('../../../src/services/mapsService', () => ({ searchPlaces: searchPlacesMock })); + import { createTables } from '../../../src/db/schema'; import { runMigrations } from '../../../src/db/migrations'; import { resetTestDb } from '../../helpers/test-db'; @@ -51,6 +54,7 @@ beforeAll(() => { beforeEach(() => { resetTestDb(testDb); broadcastMock.mockClear(); + searchPlacesMock.mockClear(); delete process.env.DEMO_MODE; }); @@ -267,44 +271,53 @@ describe('Tool: list_categories', () => { // --------------------------------------------------------------------------- describe('Tool: search_place', () => { - it('returns formatted results from Nominatim', async () => { + it('returns OSM results when no Google key is configured', async () => { const { user } = createUser(testDb); - const mockFetch = vi.fn().mockResolvedValue({ - ok: true, - json: async () => [ - { - osm_type: 'node', - osm_id: 12345, - name: 'Eiffel Tower', - display_name: 'Eiffel Tower, Paris, France', - lat: '48.8584', - lon: '2.2945', - }, + searchPlacesMock.mockResolvedValue({ + source: 'openstreetmap', + places: [ + { osm_id: 'node:12345', name: 'Eiffel Tower', address: 'Eiffel Tower, Paris, France', lat: 48.8584, lng: 2.2945 }, ], }); - vi.stubGlobal('fetch', mockFetch); await withHarness(user.id, async (h) => { const result = await h.client.callTool({ name: 'search_place', arguments: { query: 'Eiffel Tower' } }); const data = parseToolResult(result) as any; + expect(searchPlacesMock).toHaveBeenCalledWith(user.id, 'Eiffel Tower'); expect(data.places).toHaveLength(1); - expect(data.places[0].name).toBe('Eiffel Tower'); expect(data.places[0].osm_id).toBe('node:12345'); + expect(data.places[0].name).toBe('Eiffel Tower'); expect(data.places[0].lat).toBeCloseTo(48.8584); }); - - vi.unstubAllGlobals(); }); - it('returns error when Nominatim API fails', async () => { + it('returns google_place_id when Google Maps is configured', async () => { const { user } = createUser(testDb); - vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: false })); + searchPlacesMock.mockResolvedValue({ + source: 'google', + places: [ + { google_place_id: 'ChIJD3uTd9hx5kcR1IQvGfr8dbk', name: 'Eiffel Tower', address: 'Champ de Mars, Paris', lat: 48.8584, lng: 2.2945, rating: 4.7, website: 'https://toureiffel.paris', phone: null }, + ], + }); + + await withHarness(user.id, async (h) => { + const result = await h.client.callTool({ name: 'search_place', arguments: { query: 'Eiffel Tower' } }); + const data = parseToolResult(result) as any; + expect(searchPlacesMock).toHaveBeenCalledWith(user.id, 'Eiffel Tower'); + expect(data.places).toHaveLength(1); + expect(data.places[0].google_place_id).toBe('ChIJD3uTd9hx5kcR1IQvGfr8dbk'); + expect(data.places[0].name).toBe('Eiffel Tower'); + expect(data.places[0].rating).toBe(4.7); + }); + }); + + it('returns error when place search fails', async () => { + const { user } = createUser(testDb); + searchPlacesMock.mockRejectedValue(new Error('Search failed')); await withHarness(user.id, async (h) => { const result = await h.client.callTool({ name: 'search_place', arguments: { query: 'something' } }); expect(result.isError).toBe(true); }); - - vi.unstubAllGlobals(); }); });