From cf84829d6af00b3bed9c9dc18e4e12a5575be18e Mon Sep 17 00:00:00 2001 From: Andrew Bierman Date: Thu, 30 Apr 2026 20:21:14 -0600 Subject: [PATCH 1/2] fix(trails): response.url crash + MCP tool deduplication - alltrails-preview: fall back to parsed.href when response.url is empty string (occurs in test environments and some CF Worker fetch contexts) - mcp/trails.ts: remove duplicate search_trails/get_trail/get_trail_geometry registrations left from merge; fix preview_alltrails_url endpoint path from /alltrails/preview to /trails/alltrails-preview --- packages/api/src/routes/trails/index.ts | 4 +- packages/mcp/src/tools/trails.ts | 98 +------------------------ 2 files changed, 4 insertions(+), 98 deletions(-) diff --git a/packages/api/src/routes/trails/index.ts b/packages/api/src/routes/trails/index.ts index f30b3bf6b4..b5fa671a67 100644 --- a/packages/api/src/routes/trails/index.ts +++ b/packages/api/src/routes/trails/index.ts @@ -353,7 +353,9 @@ export const trailsRoutes = new Elysia({ prefix: '/trails' }) return status(422, { error: 'Could not extract trail metadata from page' }); } - return { title, description, image, url: response.url }; + // response.url is empty string in test environments and some CF Worker contexts; + // fall back to the validated parsed.href rather than crashing on new URL(""). + return { title, description, image, url: response.url || parsed.href }; } catch (error) { if (error instanceof Error && error.name === 'TimeoutError') { return status(504, { error: 'AllTrails request timed out' }); diff --git a/packages/mcp/src/tools/trails.ts b/packages/mcp/src/tools/trails.ts index 67f8b8c1a1..7c60be6e77 100644 --- a/packages/mcp/src/tools/trails.ts +++ b/packages/mcp/src/tools/trails.ts @@ -113,102 +113,6 @@ export function registerTrailTools(agent: AgentContext): void { // ── AllTrails preview ───────────────────────────────────────────────────── - agent.server.registerTool( - 'search_trails', - { - description: - 'Search for hiking, cycling, skiing, or other outdoor trails via OpenStreetMap. ' + - 'Filter by name and/or location. Requires either q (text) or lat+lon (spatial search).', - inputSchema: { - q: z - .string() - .optional() - .describe('Trail or route name to search for (e.g. "John Muir Trail")'), - lat: z.number().optional().describe('Latitude of the center point for spatial search'), - lon: z.number().optional().describe('Longitude of the center point for spatial search'), - radius: z - .number() - .min(0) - .max(500) - .optional() - .describe('Search radius in kilometres (default 50, max 500)'), - sport: z - .enum(['hiking', 'cycling', 'skiing', 'running', 'horse_riding']) - .optional() - .describe('Filter by sport/activity type'), - limit: z - .number() - .int() - .min(1) - .max(200) - .default(50) - .describe('Maximum results to return (default 50)'), - offset: z - .number() - .int() - .min(0) - .default(0) - .describe('Offset for client-side pagination (default 0)'), - }, - }, - async ({ q, lat, lon, radius, sport, limit, offset }) => { - try { - const data = await agent.api.get('/trails/search', { - q, - lat, - lon, - radius, - sport, - limit, - offset, - }); - return ok(data); - } catch (e) { - return err(e); - } - }, - ); - - agent.server.registerTool( - 'get_trail', - { - description: - 'Get metadata for a specific trail by its OpenStreetMap relation ID. ' + - 'Returns name, sport, network, distance, difficulty, and bounding box.', - inputSchema: { - osmId: z.string().describe('OSM relation ID of the trail (e.g. "1243746")'), - }, - }, - async ({ osmId }) => { - try { - const data = await agent.api.get(`/trails/${osmId}`); - return ok(data); - } catch (e) { - return err(e); - } - }, - ); - - agent.server.registerTool( - 'get_trail_geometry', - { - description: - 'Get full GeoJSON geometry for a trail by its OpenStreetMap relation ID. ' + - 'Returns all trail metadata plus a GeoJSON LineString or MultiLineString.', - inputSchema: { - osmId: z.string().describe('OSM relation ID of the trail (e.g. "1243746")'), - }, - }, - async ({ osmId }) => { - try { - const data = await agent.api.get(`/trails/${osmId}/geometry`); - return ok(data); - } catch (e) { - return err(e); - } - }, - ); - agent.server.registerTool( 'preview_alltrails_url', { @@ -223,7 +127,7 @@ export function registerTrailTools(agent: AgentContext): void { }, async ({ url }) => { try { - const data = await agent.api.post('/alltrails/preview', { url }); + const data = await agent.api.post('/trails/alltrails-preview', { url }); return ok(data); } catch (e) { return err(e); From dfa98475ccd619a39aa139d36b0c1b0a6679a817 Mon Sep 17 00:00:00 2001 From: Andrew Bierman Date: Thu, 30 Apr 2026 20:32:40 -0600 Subject: [PATCH 2/2] fix(trails): hasMore pagination, remove duplicate alltrails handler, clean up tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - search_trails returns { trails, hasMore } instead of bare array (006) LIMIT n+1 trick — caller can page with offset until hasMore is false - remove POST /trails/alltrails-preview; /alltrails/preview (alltrails.ts) is the canonical endpoint (015); MCP tool updated to match - drop dead alltrails-preview test block from trails.test.ts — covered by alltrails.test.ts (012); remove unused afterEach/vi imports - fix misleading test name: stitching does not write back to DB (014) --- packages/api/src/routes/trails/index.ts | 132 +++-------------- packages/api/test/trails.test.ts | 181 +++++------------------- packages/mcp/src/tools/trails.ts | 4 +- 3 files changed, 53 insertions(+), 264 deletions(-) diff --git a/packages/api/src/routes/trails/index.ts b/packages/api/src/routes/trails/index.ts index b5fa671a67..f0fd9873b1 100644 --- a/packages/api/src/routes/trails/index.ts +++ b/packages/api/src/routes/trails/index.ts @@ -5,17 +5,6 @@ import { sql } from 'drizzle-orm'; import { Elysia, status } from 'elysia'; import { z } from 'zod'; -// ── OG meta extraction (AllTrails preview) ─────────────────────────────────── -// Two attribute orderings are valid per the HTML spec: property-then-content and -// content-then-property. Static top-level regexes avoid dynamic RegExp construction. - -const OG_TITLE_A = /]+property=["']og:title["'][^>]+content=["']([^"']+)["']/i; -const OG_TITLE_B = /]+content=["']([^"']+)["'][^>]+property=["']og:title["']/i; -const OG_DESC_A = /]+property=["']og:description["'][^>]+content=["']([^"']+)["']/i; -const OG_DESC_B = /]+content=["']([^"']+)["'][^>]+property=["']og:description["']/i; -const OG_IMAGE_A = /]+property=["']og:image["'][^>]+content=["']([^"']+)["']/i; -const OG_IMAGE_B = /]+content=["']([^"']+)["'][^>]+property=["']og:image["']/i; - // ── Zod schemas ───────────────────────────────────────────────────────────── const OsmMemberSchema = z.object({ @@ -102,21 +91,26 @@ export const trailsRoutes = new Elysia({ prefix: '/trails' }) ORDER BY CASE WHEN name IS NOT NULL THEN 0 ELSE 1 END, name - LIMIT ${limit} OFFSET ${offset} + LIMIT ${limit + 1} OFFSET ${offset} `); const rows = z.array(RouteSearchRowSchema).parse(result.rows); + const hasMore = rows.length > limit; + const page = rows.slice(0, limit); - return rows.map((row) => ({ - osmId: row.osm_id, - name: row.name, - sport: row.sport, - network: row.network, - distance: row.distance, - difficulty: row.difficulty, - description: row.description, - bbox: row.bbox ? JSON.parse(row.bbox) : null, - })); + return { + trails: page.map((row) => ({ + osmId: row.osm_id, + name: row.name, + sport: row.sport, + network: row.network, + distance: row.distance, + difficulty: row.difficulty, + description: row.description, + bbox: row.bbox ? JSON.parse(row.bbox) : null, + })), + hasMore, + }; } catch (error) { if (error instanceof Error && error.message.includes('not configured')) { return status(503, { error: 'Trail features are not enabled on this server' }); @@ -279,98 +273,4 @@ export const trailsRoutes = new Elysia({ prefix: '/trails' }) security: [{ bearerAuth: [] }], }, }, - ) - - /** - * POST /api/trails/alltrails-preview - * - * Fetches an AllTrails URL server-side and extracts OpenGraph metadata. - */ - .post( - '/alltrails-preview', - async ({ body }) => { - const { url } = body; - - let parsed: URL; - try { - parsed = new URL(url); - } catch { - return status(400, { error: 'Invalid URL' }); - } - - const { hostname, protocol } = parsed; - if ( - protocol !== 'https:' || - (hostname !== 'alltrails.com' && !hostname.endsWith('.alltrails.com')) - ) { - return status(400, { error: 'Only https://alltrails.com URLs are supported' }); - } - - const AT_UA = 'Mozilla/5.0 (compatible; PackRat/1.0; +https://packrat.world)'; - - try { - let response = await fetch(url, { - headers: { 'User-Agent': AT_UA, Accept: 'text/html' }, - redirect: 'manual', - signal: AbortSignal.timeout(8000), - }); - - if (response.status >= 300 && response.status < 400) { - const location = response.headers.get('location'); - if (!location) - return status(502, { error: 'AllTrails redirected without Location header' }); - let redirectUrl: URL; - try { - redirectUrl = new URL(location, url); - } catch { - return status(502, { error: 'Invalid redirect URL' }); - } - if ( - redirectUrl.protocol !== 'https:' || - (redirectUrl.hostname !== 'alltrails.com' && - !redirectUrl.hostname.endsWith('.alltrails.com')) - ) { - return status(400, { error: 'Redirect target is not alltrails.com' }); - } - response = await fetch(redirectUrl.toString(), { - headers: { 'User-Agent': AT_UA, Accept: 'text/html' }, - redirect: 'error', - signal: AbortSignal.timeout(8000), - }); - } - - if (!response.ok) { - return status(502, { error: `AllTrails returned ${response.status}` }); - } - - const html = await response.text(); - - const title = (html.match(OG_TITLE_A) ?? html.match(OG_TITLE_B))?.[1] ?? null; - const description = (html.match(OG_DESC_A) ?? html.match(OG_DESC_B))?.[1] ?? null; - const image = (html.match(OG_IMAGE_A) ?? html.match(OG_IMAGE_B))?.[1] ?? null; - - if (!title) { - return status(422, { error: 'Could not extract trail metadata from page' }); - } - - // response.url is empty string in test environments and some CF Worker contexts; - // fall back to the validated parsed.href rather than crashing on new URL(""). - return { title, description, image, url: response.url || parsed.href }; - } catch (error) { - if (error instanceof Error && error.name === 'TimeoutError') { - return status(504, { error: 'AllTrails request timed out' }); - } - console.error('AllTrails preview error:', error); - return status(502, { error: 'Failed to fetch AllTrails page' }); - } - }, - { - body: z.object({ url: z.string().url() }), - isAuthenticated: true, - detail: { - tags: ['Trails'], - summary: 'Fetch trail card metadata from an AllTrails URL via OG tags', - security: [{ bearerAuth: [] }], - }, - }, ); diff --git a/packages/api/test/trails.test.ts b/packages/api/test/trails.test.ts index d528a54f45..f3c86943bb 100644 --- a/packages/api/test/trails.test.ts +++ b/packages/api/test/trails.test.ts @@ -1,4 +1,4 @@ -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { beforeEach, describe, expect, it } from 'vitest'; import { TEST_GEOMETRY_LAT, TEST_GEOMETRY_LON } from './fixtures/trail-fixtures'; import { seedOsmRoute, seedOsmWay } from './utils/osm-db-helpers'; import { @@ -164,8 +164,10 @@ describe('Trails Routes', () => { const res = await apiWithAuth('/trails/search?q=John+Muir+Test'); const data = await expectJsonResponse(res); - expect(Array.isArray(data)).toBe(true); - const found = data.find((t: { osmId: string }) => t.osmId === String(RELATION_WITH_GEOM_ID)); + expect(Array.isArray(data.trails)).toBe(true); + const found = data.trails.find( + (t: { osmId: string }) => t.osmId === String(RELATION_WITH_GEOM_ID), + ); expect(found).toBeDefined(); expect(found.name).toBe('John Muir Test Trail'); expect(found.network).toBe('rwn'); @@ -176,15 +178,18 @@ describe('Trails Routes', () => { const res = await apiWithAuth('/trails/search?q=john+muir+test'); const data = await expectJsonResponse(res); - const found = data.find((t: { osmId: string }) => t.osmId === String(RELATION_WITH_GEOM_ID)); + const found = data.trails.find( + (t: { osmId: string }) => t.osmId === String(RELATION_WITH_GEOM_ID), + ); expect(found).toBeDefined(); }); it('returns empty array for a query that matches nothing', async () => { const res = await apiWithAuth('/trails/search?q=zzz_no_match_zzz'); const data = await expectJsonResponse(res); - expect(Array.isArray(data)).toBe(true); - expect(data).toHaveLength(0); + expect(Array.isArray(data.trails)).toBe(true); + expect(data.trails).toHaveLength(0); + expect(data.hasMore).toBe(false); }); it('does spatial search by lat/lon and returns nearby trails', async () => { @@ -194,9 +199,9 @@ describe('Trails Routes', () => { ); const data = await expectJsonResponse(res); - expect(Array.isArray(data)).toBe(true); + expect(Array.isArray(data.trails)).toBe(true); // At least the relation with pre-built geometry should be within 50 km - const osmIds = data.map((t: { osmId: string }) => t.osmId); + const osmIds = data.trails.map((t: { osmId: string }) => t.osmId); expect(osmIds).toContain(String(RELATION_WITH_GEOM_ID)); }); @@ -206,16 +211,16 @@ describe('Trails Routes', () => { `/trails/search?q=John+Muir+Test&lat=${TEST_GEOMETRY_LAT}&lon=${TEST_GEOMETRY_LON}&radius=50`, ); const hit = await expectJsonResponse(resHit); - expect(hit.some((t: { osmId: string }) => t.osmId === String(RELATION_WITH_GEOM_ID))).toBe( - true, - ); + expect( + hit.trails.some((t: { osmId: string }) => t.osmId === String(RELATION_WITH_GEOM_ID)), + ).toBe(true); // Correct name but very far location → no match const resMiss = await apiWithAuth('/trails/search?q=John+Muir+Test&lat=0&lon=0&radius=1'); const miss = await expectJsonResponse(resMiss); - expect(miss.some((t: { osmId: string }) => t.osmId === String(RELATION_WITH_GEOM_ID))).toBe( - false, - ); + expect( + miss.trails.some((t: { osmId: string }) => t.osmId === String(RELATION_WITH_GEOM_ID)), + ).toBe(false); }); it('returns 400 for out-of-range coordinates', async () => { @@ -236,7 +241,7 @@ describe('Trails Routes', () => { ); const data = await expectJsonResponse(res); - const osmIds = data.map((t: { osmId: string }) => t.osmId); + const osmIds = data.trails.map((t: { osmId: string }) => t.osmId); expect(osmIds).toContain(String(RELATION_HIKING_ID)); expect(osmIds).not.toContain(String(RELATION_CYCLING_ID)); }); @@ -244,7 +249,9 @@ describe('Trails Routes', () => { it('returns sport field in search results', async () => { const res = await apiWithAuth('/trails/search?q=Pacific+Crest+Hiking'); const data = await expectJsonResponse(res); - const found = data.find((t: { osmId: string }) => t.osmId === String(RELATION_HIKING_ID)); + const found = data.trails.find( + (t: { osmId: string }) => t.osmId === String(RELATION_HIKING_ID), + ); expect(found).toBeDefined(); expect(found.sport).toBe('hiking'); }); @@ -254,21 +261,24 @@ describe('Trails Routes', () => { `/trails/search?lat=${TEST_GEOMETRY_LAT}&lon=${TEST_GEOMETRY_LON}&radius=500&limit=1&offset=0`, ); const page1 = await expectJsonResponse(res1); - expect(page1).toHaveLength(1); + expect(page1.trails).toHaveLength(1); + expect(page1.hasMore).toBe(true); const res2 = await apiWithAuth( `/trails/search?lat=${TEST_GEOMETRY_LAT}&lon=${TEST_GEOMETRY_LON}&radius=500&limit=1&offset=1`, ); const page2 = await expectJsonResponse(res2); - expect(page2).toHaveLength(1); + expect(page2.trails).toHaveLength(1); - expect(page1[0].osmId).not.toBe(page2[0].osmId); + expect(page1.trails[0].osmId).not.toBe(page2.trails[0].osmId); }); it('returns bbox when geometry is present', async () => { const res = await apiWithAuth('/trails/search?q=John+Muir+Test'); const data = await expectJsonResponse(res); - const found = data.find((t: { osmId: string }) => t.osmId === String(RELATION_WITH_GEOM_ID)); + const found = data.trails.find( + (t: { osmId: string }) => t.osmId === String(RELATION_WITH_GEOM_ID), + ); expect(found.bbox).not.toBeNull(); expect(found.bbox.type).toBe('Polygon'); }); @@ -276,7 +286,9 @@ describe('Trails Routes', () => { it('returns null bbox when geometry is null', async () => { const res = await apiWithAuth('/trails/search?q=Unstored+Geometry'); const data = await expectJsonResponse(res); - const found = data.find((t: { osmId: string }) => t.osmId === String(RELATION_NO_GEOM_ID)); + const found = data.trails.find( + (t: { osmId: string }) => t.osmId === String(RELATION_NO_GEOM_ID), + ); expect(found).toBeDefined(); expect(found.bbox).toBeNull(); }); @@ -347,11 +359,10 @@ describe('Trails Routes', () => { expect(data.geometry.type).toMatch(/^(LineString|MultiLineString)$/); }); - it('returns stitched geometry on repeated calls when stored geometry is null', async () => { - // First call: triggers stitching and caching + it('stitches geometry on every call when stored geometry is null (no write-back)', async () => { + // Both calls stitch from member ways — the route does not write the result back. await apiWithAuth(`/trails/${RELATION_NO_GEOM_ID}/geometry`); - // Second call: should now hit the cached geometry branch const res2 = await apiWithAuth(`/trails/${RELATION_NO_GEOM_ID}/geometry`); const data2 = await expectJsonResponse(res2, ['osmId', 'geometry']); @@ -397,126 +408,4 @@ describe('Trails Routes', () => { expect(data.geometry.type).toBe('MultiLineString'); }); }); - - // ── POST /trails/alltrails-preview ──────────────────────────────────────── - - describe('POST /trails/alltrails-preview', () => { - afterEach(() => { - vi.unstubAllGlobals(); - }); - - it('returns 400 for a non-alltrails.com URL', async () => { - const res = await apiWithAuth('/trails/alltrails-preview', { - method: 'POST', - body: JSON.stringify({ url: 'https://example.com/trail' }), - headers: { 'Content-Type': 'application/json' }, - }); - expectBadRequest(res); - }); - - it('returns 400 for an invalid (non-URL) string', async () => { - const res = await apiWithAuth('/trails/alltrails-preview', { - method: 'POST', - body: JSON.stringify({ url: 'not-a-url' }), - headers: { 'Content-Type': 'application/json' }, - }); - expectBadRequest(res); - }); - - it('returns 400 for a URL on an alltrails subdomain that is not alltrails.com', async () => { - const res = await apiWithAuth('/trails/alltrails-preview', { - method: 'POST', - body: JSON.stringify({ url: 'https://evil.alltrails.com.attacker.com/trail' }), - headers: { 'Content-Type': 'application/json' }, - }); - expectBadRequest(res); - }); - - it('extracts OG metadata from a valid AllTrails page', async () => { - const mockHtml = ` - - - - - - - - - `; - - vi.stubGlobal( - 'fetch', - vi.fn().mockResolvedValue( - new Response(mockHtml, { - status: 200, - headers: { 'Content-Type': 'text/html' }, - }), - ), - ); - - const testUrl = 'https://www.alltrails.com/trail/us/utah/angels-landing-trail'; - const res = await apiWithAuth('/trails/alltrails-preview', { - method: 'POST', - body: JSON.stringify({ url: testUrl }), - headers: { 'Content-Type': 'application/json' }, - }); - - const data = await expectJsonResponse(res, ['title', 'url']); - expect(data.title).toBe('Angels Landing Trail'); - expect(data.description).toBe('One of the most popular hikes in Zion.'); - expect(data.image).toBe('https://images.alltrails.com/angels-landing.jpg'); - expect(data.url).toBe(testUrl); - }); - - it('extracts OG tags regardless of attribute order in the meta tag', async () => { - // Some pages write content before property in the attribute order - const mockHtml = ` - - - - - `; - - vi.stubGlobal('fetch', vi.fn().mockResolvedValue(new Response(mockHtml, { status: 200 }))); - - const res = await apiWithAuth('/trails/alltrails-preview', { - method: 'POST', - body: JSON.stringify({ url: 'https://www.alltrails.com/trail/us/co/summit-peak' }), - headers: { 'Content-Type': 'application/json' }, - }); - - const data = await expectJsonResponse(res, ['title']); - expect(data.title).toBe('Summit Peak Trail'); - expect(data.description).toBe('Challenging summit hike.'); - }); - - it('returns 422 when the page has no OG title', async () => { - vi.stubGlobal( - 'fetch', - vi - .fn() - .mockResolvedValue( - new Response('Page', { status: 200 }), - ), - ); - - const res = await apiWithAuth('/trails/alltrails-preview', { - method: 'POST', - body: JSON.stringify({ url: 'https://www.alltrails.com/trail/us/ut/no-og-trail' }), - headers: { 'Content-Type': 'application/json' }, - }); - expect(res.status).toBe(422); - }); - - it('returns 502 when AllTrails returns a non-OK status', async () => { - vi.stubGlobal('fetch', vi.fn().mockResolvedValue(new Response('Not Found', { status: 404 }))); - - const res = await apiWithAuth('/trails/alltrails-preview', { - method: 'POST', - body: JSON.stringify({ url: 'https://www.alltrails.com/trail/us/ut/missing' }), - headers: { 'Content-Type': 'application/json' }, - }); - expect(res.status).toBe(502); - }); - }); }); diff --git a/packages/mcp/src/tools/trails.ts b/packages/mcp/src/tools/trails.ts index 7c60be6e77..0720f9158d 100644 --- a/packages/mcp/src/tools/trails.ts +++ b/packages/mcp/src/tools/trails.ts @@ -9,7 +9,7 @@ export function registerTrailTools(agent: AgentContext): void { 'search_trails', { description: - 'Search outdoor trails and routes from OpenStreetMap. Filter by name, sport type, and/or proximity to a location. Returns lightweight metadata (no geometry) suitable for a search list.', + 'Search outdoor trails and routes from OpenStreetMap. Filter by name, sport type, and/or proximity to a location. Returns { trails: [...], hasMore: boolean } — use hasMore with offset to paginate.', inputSchema: { q: z .string() @@ -127,7 +127,7 @@ export function registerTrailTools(agent: AgentContext): void { }, async ({ url }) => { try { - const data = await agent.api.post('/trails/alltrails-preview', { url }); + const data = await agent.api.post('/alltrails/preview', { url }); return ok(data); } catch (e) { return err(e);