feat(api): AllTrails OG preview endpoint and MCP tool#2310
Conversation
POST /api/alltrails/preview scrapes og:title, og:description, og:image from AllTrails URLs server-side. MCP tool preview_alltrails_url exposes this to agents for trip/pack enrichment.
13 tests covering request validation, OG metadata extraction, timeout (504), network failure (502), upstream 4xx (502), missing og:title (422), and redirect-outside-alltrails.com (400).
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds a new AllTrails preview endpoint ( Changes
Sequence DiagramsequenceDiagram
participant MCP as MCP Client
participant Tool as Trail Tools
participant API as API Endpoint
participant AllTrails as AllTrails Host
MCP->>Tool: preview_alltrails_url(url)
Tool->>Tool: Validate URL format
Tool->>API: POST /alltrails/preview {url}
API->>API: Validate domain (alltrails.com)
API->>API: Enforce HTTPS
API->>AllTrails: GET page (custom User-Agent, 8s timeout)
AllTrails-->>API: HTML response
API->>API: Extract OG metadata via regex
API->>API: Verify og:title exists
API->>API: Verify redirect URL on alltrails.com
API-->>Tool: {title, description, image, url}
Tool-->>MCP: ok({...metadata}) or err(exception)
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Coverage Report for API Unit Tests Coverage (./packages/api)
File CoverageNo changed files found. |
Coverage Report for Expo Unit Tests Coverage (./apps/expo)
File CoverageNo changed files found. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Pull request overview
Adds an API + MCP surface for generating OpenGraph previews from AllTrails trail pages, intended to enrich PackRat entities (trips/packs) when a user shares an AllTrails link.
Changes:
- Added
POST /api/alltrails/previewroute to fetch and parseog:title,og:description, andog:imagefrom AllTrails HTML. - Added MCP tool
preview_alltrails_urlthat calls the new API endpoint. - Added Vitest coverage for validation, success cases, and upstream error handling.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/api/src/routes/alltrails.ts | Implements the new AllTrails OG preview endpoint (validation, fetch, HTML parsing, error mapping). |
| packages/api/src/routes/index.ts | Registers alltrailsRoutes under the aggregated /api router. |
| packages/api/test/alltrails.test.ts | Adds route-level tests covering input validation, successful parsing, and failure modes. |
| packages/mcp/src/tools/trails.ts | Introduces preview_alltrails_url MCP tool to call the API preview route. |
| packages/mcp/src/index.ts | Registers the new trail tools during MCP agent initialization. |
| bun.lock | Updates lockfile/workspace versions and records dependency graph changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| response = await fetch(url, { | ||
| headers: { 'User-Agent': UA }, | ||
| signal: AbortSignal.timeout(8000), | ||
| }); |
There was a problem hiding this comment.
The redirect check happens after fetch() completes, but fetch() will follow redirects by default. That means a crafted alltrails.com URL could still cause the worker to make an outbound request to a non-AllTrails host before you return the 400, which defeats the intent of the redirect guard. Consider using redirect: 'manual' and explicitly validating/following 3xx Location targets only when they remain within *.alltrails.com (or disable redirects entirely).
| let parsed: URL; | ||
| try { | ||
| parsed = new URL(url); | ||
| } catch { | ||
| return status(400, { error: 'Invalid URL' }); | ||
| } |
There was a problem hiding this comment.
Because the route already declares body: z.object({ url: z.string().url() }), Elysia validation should reject invalid URLs before the handler runs (via the app-level onError handler). This try/catch + Invalid URL branch is likely dead code and the manual parse can be simplified to const parsed = new URL(url) to avoid redundant validation paths and inconsistent error messaging.
| let parsed: URL; | |
| try { | |
| parsed = new URL(url); | |
| } catch { | |
| return status(400, { error: 'Invalid URL' }); | |
| } | |
| const parsed = new URL(url); |
| export const alltrailsRoutes = new Elysia({ prefix: '/alltrails' }).post( | ||
| '/preview', | ||
| async ({ body }) => { | ||
| const { url } = body; | ||
|
|
There was a problem hiding this comment.
This endpoint proxies/scrapes a third-party site and currently appears to be callable without authentication (no isAuthenticated: true). That makes it easy to abuse for unauthenticated scraping/egress, even if the hostname is restricted. Consider gating it behind Bearer auth (e.g. add isAuthenticated: true + security: [{ bearerAuth: [] }], and ensure the auth macro is available for this router) to align with other external-fetch endpoints like /knowledge-base/reader/extract.
| import { Elysia, status } from 'elysia'; | ||
| import { z } from 'zod'; | ||
|
|
||
| const ALLTRAILS_HOSTNAME_RE = /^(?:[a-z0-9-]+\.)?alltrails\.com$/; |
There was a problem hiding this comment.
ALLTRAILS_HOSTNAME_RE only allows at most one subdomain level ((?:...\.)?). That rejects valid multi-level subdomains like foo.bar.alltrails.com even though the PR description says subdomains are supported. Consider changing this to allow any number of subdomain labels (e.g. (?:[a-z0-9-]+\.)*alltrails\.com).
| const ALLTRAILS_HOSTNAME_RE = /^(?:[a-z0-9-]+\.)?alltrails\.com$/; | |
| const ALLTRAILS_HOSTNAME_RE = /^(?:[a-z0-9-]+\.)*alltrails\.com$/; |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/mcp/src/tools/trails.ts (1)
11-13: Tighten the input description to reflect accepted subdomains.The route accepts
https://(*.)alltrails.com/...(e.g.,www.,es.), but the agent-facing hint reads "must behttps://alltrails.com/...", which may cause the model to reject otherwise valid links a user shares.- url: z.string().url().describe('Full AllTrails URL (must be https://alltrails.com/...)'), + url: z + .string() + .url() + .describe('Full AllTrails URL (https://alltrails.com/... or any *.alltrails.com)'),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/mcp/src/tools/trails.ts` around lines 11 - 13, The inputSchema's url description is too strict; update the describe text for the url field inside inputSchema to reflect accepted subdomains (e.g., allow https://*.alltrails.com/ like www., es.) instead of "must be https://alltrails.com/..."; adjust the string in the url.describe(...) for the url property so the hint clearly states "Full AllTrails URL (must be https://*.alltrails.com/...)" or similar phrasing that mentions allowed subdomains, keeping the validation using z.string().url() unchanged.packages/api/src/routes/alltrails.ts (1)
7-16: Decoded HTML entities andname="og:..."variants are missed by the regex extractor.OpenGraph values served by AllTrails (and most CMSs) frequently contain HTML entities (
&,',", named entities for accents, etc.).extractOgTagreturns the raw attribute substring, so callers — and ultimately UI components rendering the title/description — will receive literal&rather than&. The regex also won't match<meta name="og:title" ...>, which some pages emit. linkedom is already a project dependency (seepackages/api/src/routes/knowledgeBase/reader.ts); switching to it gives correct entity decoding and tolerates attribute ordering/casing.♻️ Suggested approach using linkedom
-import { Elysia, status } from 'elysia'; -import { z } from 'zod'; +import { parseHTML } from 'linkedom'; +import { Elysia, status } from 'elysia'; +import { z } from 'zod'; @@ -function extractOgTag(html: string, property: string): string | null { - const match = - html.match( - new RegExp(`<meta[^>]+property=["']${property}["'][^>]+content=["']([^"']+)["']`, 'i'), - ) ?? - html.match( - new RegExp(`<meta[^>]+content=["']([^"']+)["'][^>]+property=["']${property}["']`, 'i'), - ); - return match?.[1] ?? null; -} +function extractOgTag(document: Document, property: string): string | null { + const el = + document.querySelector(`meta[property="${property}"]`) ?? + document.querySelector(`meta[name="${property}"]`); + return el?.getAttribute('content') ?? null; +}And at the call site:
- const html = await response.text(); - - const title = extractOgTag(html, 'og:title'); + const html = await response.text(); + const { document } = parseHTML(html); + + const title = extractOgTag(document, 'og:title'); @@ - const description = extractOgTag(html, 'og:description'); - const image = extractOgTag(html, 'og:image'); + const description = extractOgTag(document, 'og:description'); + const image = extractOgTag(document, 'og:image');Note: the static-analysis ReDoS warning on these lines is a false positive —
propertyis only ever called with hardcodedog:title/og:description/og:image— but switching to a parser side-steps it entirely.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/api/src/routes/alltrails.ts` around lines 7 - 16, extractOgTag currently uses regex that misses name="og:..." and returns raw HTML entities; replace its implementation to parse the HTML with linkedom's DOMParser (imported from linkedom), query for a meta element whose property or name equals the provided property (case-insensitive and tolerant of attribute order), and return the meta's content attribute (linkedom will decode HTML entities) or null if not found; update references to extractOgTag to keep the same signature (string -> string|null).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/api/src/routes/alltrails.ts`:
- Around line 30-32: Update the 400 error message in the alltrails route to
accurately reflect allowed hosts: when checking parsed.protocol and
ALLTRAILS_HOSTNAME_RE, change the returned message from "URL must be an
https://alltrails.com URL" to something that mentions https and allows
alltrails.com and its single-level subdomains (e.g., "URL must be an
https://alltrails.com or subdomain URL"); locate the check that uses parsed and
ALLTRAILS_HOSTNAME_RE and update the status(400, { error: ... }) string
accordingly.
- Around line 73-83: This route currently uses Elysia-style route metadata and
must be rewritten to use OpenAPIHono's createRoute with the existing Zod body
schema; replace the current Elysia route definition in alltrails.ts with a
createRoute call that declares method (POST), path, and the Zod body schema
(z.object({ url: z.string().url() })) as the request schema, move the
tags/summary/description into the OpenAPI metadata for createRoute, and ensure
the handler function signature matches OpenAPIHono expectations and returns the
same OG preview response; import createRoute from OpenAPIHono and keep the Zod
schema identifier so type-safe validation and generated docs work.
---
Nitpick comments:
In `@packages/api/src/routes/alltrails.ts`:
- Around line 7-16: extractOgTag currently uses regex that misses name="og:..."
and returns raw HTML entities; replace its implementation to parse the HTML with
linkedom's DOMParser (imported from linkedom), query for a meta element whose
property or name equals the provided property (case-insensitive and tolerant of
attribute order), and return the meta's content attribute (linkedom will decode
HTML entities) or null if not found; update references to extractOgTag to keep
the same signature (string -> string|null).
In `@packages/mcp/src/tools/trails.ts`:
- Around line 11-13: The inputSchema's url description is too strict; update the
describe text for the url field inside inputSchema to reflect accepted
subdomains (e.g., allow https://*.alltrails.com/ like www., es.) instead of
"must be https://alltrails.com/..."; adjust the string in the url.describe(...)
for the url property so the hint clearly states "Full AllTrails URL (must be
https://*.alltrails.com/...)" or similar phrasing that mentions allowed
subdomains, keeping the validation using z.string().url() unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ec1d7943-13c8-4de9-b7e1-d9a6c6992d9e
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
packages/api/src/routes/alltrails.tspackages/api/src/routes/index.tspackages/api/test/alltrails.test.tspackages/mcp/src/index.tspackages/mcp/src/tools/trails.ts
| if (parsed.protocol !== 'https:' || !ALLTRAILS_HOSTNAME_RE.test(parsed.hostname)) { | ||
| return status(400, { error: 'URL must be an https://alltrails.com URL' }); | ||
| } |
There was a problem hiding this comment.
Error message understates what's accepted.
The host regex permits any single-level subdomain (www., es., …) but the message implies only the bare apex. A user posting https://es.alltrails.com/... that fails parsing will see a misleading error. Minor wording fix:
- return status(400, { error: 'URL must be an https://alltrails.com URL' });
+ return status(400, { error: 'URL must be an https://(*.)alltrails.com URL' });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (parsed.protocol !== 'https:' || !ALLTRAILS_HOSTNAME_RE.test(parsed.hostname)) { | |
| return status(400, { error: 'URL must be an https://alltrails.com URL' }); | |
| } | |
| if (parsed.protocol !== 'https:' || !ALLTRAILS_HOSTNAME_RE.test(parsed.hostname)) { | |
| return status(400, { error: 'URL must be an https://(*.)alltrails.com URL' }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/api/src/routes/alltrails.ts` around lines 30 - 32, Update the 400
error message in the alltrails route to accurately reflect allowed hosts: when
checking parsed.protocol and ALLTRAILS_HOSTNAME_RE, change the returned message
from "URL must be an https://alltrails.com URL" to something that mentions https
and allows alltrails.com and its single-level subdomains (e.g., "URL must be an
https://alltrails.com or subdomain URL"); locate the check that uses parsed and
ALLTRAILS_HOSTNAME_RE and update the status(400, { error: ... }) string
accordingly.
| { | ||
| body: z.object({ | ||
| url: z.string().url(), | ||
| }), | ||
| detail: { | ||
| tags: ['AllTrails'], | ||
| summary: 'Fetch AllTrails OG preview', | ||
| description: | ||
| 'Scrapes OpenGraph metadata (title, description, image) from an AllTrails trail page.', | ||
| }, | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does Elysia 1.4 support Zod schemas directly for the body validator via Standard Schema, or is a plugin/adapter required?
💡 Result:
Yes, Elysia 1.4 supports Zod schemas directly for the body validator via Standard Schema out of the box. No plugin or adapter is required.
Citations:
- 1: https://elysiajs.com/blog/elysia-14.html
- 2: https://elysiajs.com/blog/elysia-14
- 3: https://elysiajs.com/essential/validation
🏁 Script executed:
head -100 packages/api/src/routes/alltrails.tsRepository: PackRat-AI/PackRat
Length of output: 2588
Use OpenAPIHono with createRoute per API guidelines instead of Elysia. Elysia 1.4 validates Zod schemas directly via Standard Schema out of the box, so the original concern about unvalidated bodies is not applicable—the schema will be honored. However, this file violates the mandatory requirement that API routes in packages/api/src/**/*.ts must use OpenAPIHono with createRoute and Zod schemas for type-safe, documented endpoints.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/api/src/routes/alltrails.ts` around lines 73 - 83, This route
currently uses Elysia-style route metadata and must be rewritten to use
OpenAPIHono's createRoute with the existing Zod body schema; replace the current
Elysia route definition in alltrails.ts with a createRoute call that declares
method (POST), path, and the Zod body schema (z.object({ url: z.string().url()
})) as the request schema, move the tags/summary/description into the OpenAPI
metadata for createRoute, and ensure the handler function signature matches
OpenAPIHono expectations and returns the same OG preview response; import
createRoute from OpenAPIHono and keep the Zod schema identifier so type-safe
validation and generated docs work.
feat(api): AllTrails OG preview endpoint and MCP tool
Summary
POST /api/alltrails/preview— scrapesog:title,og:description,og:imagefrom an AllTrails trail pagealltrails.comhostname (regex, covers subdomains, blocks lookalikes)preview_alltrails_urlMCP toolTest plan
cd packages/api && bun run test -- test/alltrails.test.ts— 13 tests pass🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Tests