feat(trails): trail search micro frontend acquisition surface#2380
Conversation
github-actions
Bot
added
documentation
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. |
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (96)
WalkthroughAdds a Trails Next.js app with client auth (register/login/verify), token persistence, Overpass and PackRat API search, Leaflet-based map UI, a Cloudflare Worker proxy for /api, and app/build configuration for deployment. ChangesTrails Web App: Auth, Search, Mapping, and Worker Deployment
Sequence Diagram(s)sequenceDiagram
participant Browser
participant Worker
participant PackRatAPI
participant Overpass
participant MapUI
Browser->>Worker: GET /api/trails/search (with Authorization)
Worker->>PackRatAPI: forward request (preserve headers, apply rate limit/CORS)
PackRatAPI-->>Worker: JSON { data: { trails }, hasMore }
Worker-->>Browser: proxied response + CORS
Browser->>MapUI: render displayedTrails (markers)
Note right of PackRatAPI: Overpass used server-side for public trails
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 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 |
There was a problem hiding this comment.
Pull request overview
Adds a new “Trails” acquisition surface (static Next.js site + Cloudflare Worker proxy) backed by a new dedicated OSM/PostGIS data pipeline and API endpoints for authenticated trail search + geometry retrieval.
Changes:
- Introduces
apps/trails(static Next.js export) +apps/trails/workerproxy (CORS + rate limiting + edge caching) to powertrails.packratai.com. - Adds OSM import + schema tooling (
packages/osm-import,packages/osm-db) and new API/trails/*routes backed by a dedicated PostGIS database (optionally via Hyperdrive). - Updates admin UI with a Trail Viewer, expands API tests to include PostGIS, and bumps various dependencies (Expo 55 / RN 0.83 / React 19.2).
Reviewed changes
Copilot reviewed 99 out of 102 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Adds path aliases for trails/overpass + excludes new OSM packages from root TS project |
| packages/web-ui/package.json | Bumps React types |
| packages/ui/package.json | Bumps nativewindui dependency |
| packages/osm-import/sync.ts | Adds Bun script to pg_dump/pg_restore OSM tables to production |
| packages/osm-import/routes.lua | Adds osm2pgsql flex config to build osm_ways/osm_routes tables |
| packages/osm-import/package.json | Adds osm-import workspace package + scripts |
| packages/osm-import/import.ts | Adds Bun import pipeline (download PBF, run osm2pgsql, re-run migrations, optional sync) |
| packages/osm-db/tsconfig.json | Adds TS config for OSM DB package |
| packages/osm-db/src/schema.ts | Defines Drizzle schema + indexes for OSM tables (PostGIS + trgm) |
| packages/osm-db/src/index.ts | Re-exports schema |
| packages/osm-db/package.json | Adds OSM DB workspace package (drizzle + neon/pg) |
| packages/osm-db/migrate.ts | Adds migration runner (Neon vs standard Postgres) |
| packages/osm-db/drizzle/meta/0001_snapshot.json | Drizzle snapshot for initial OSM schema |
| packages/osm-db/drizzle/meta/_journal.json | Drizzle migration journal for OSM schema |
| packages/osm-db/drizzle/0001_osm_schema.sql | Creates OSM tables + indexes |
| packages/osm-db/drizzle/0000_extensions.sql | Enables required PostGIS + pg_trgm extensions |
| packages/osm-db/drizzle.config.ts | Drizzle-kit config for OSM DB |
| packages/mcp/src/tools/trails.ts | Updates MCP tools for new /trails/* API + snake_case inputs |
| packages/env/src/node.ts | Adds OSM-related env vars (URLs, cache size, import mode) |
| packages/env/scripts/no-raw-process-env.ts | Allows raw process.env pass-through for OSM import script spawning |
| packages/config/src/config.ts | Adds EnableTrails feature flag |
| packages/api/wrangler.jsonc | Documents Hyperdrive binding; adds dev Hyperdrive stub |
| packages/api/tsconfig.json | Adjusts TS path mapping style |
| packages/api/test/utils/osm-db-helpers.ts | Adds raw SQL seed helpers for OSM tables (PostGIS WKT) |
| packages/api/test/trails.test.ts | Adds integration tests for /trails/search, /trails/:id, /trails/:id/geometry |
| packages/api/test/setup.ts | Ensures PostGIS extensions + OSM tables exist in test DB; cleans OSM tables between tests |
| packages/api/test/fixtures/trail-fixtures.ts | Adds OSM fixture builders + default geometries |
| packages/api/src/utils/env-validation.ts | Adds OSM_DATABASE_URL + OSM_HYPERDRIVE to env schema/types |
| packages/api/src/utils/tests/env-validation.test.ts | Extends env validation tests with OSM env var |
| packages/api/src/services/trails.ts | Adds runtime geometry stitching via PostGIS (ST_LineMerge over member ways) |
| packages/api/src/schemas/catalog.ts | Adjusts catalog schema nullability + date typing |
| packages/api/src/routes/trails/index.ts | Implements new trails routes backed by OSM DB (search, metadata, geometry) |
| packages/api/src/routes/admin/index.ts | Updates CF Access verification callsite to new options object |
| packages/api/src/middleware/cfAccess.ts | Refactors CF Access verifier signature to { teamDomain, aud } options |
| packages/api/src/middleware/tests/cfAccess.test.ts | Updates CF Access tests for new signature |
| packages/api/src/index.ts | Enriches worker env to derive OSM_DATABASE_URL from Hyperdrive binding |
| packages/api/src/db/schema.ts | Adds trips.trailOsmId column |
| packages/api/src/db/index.ts | Adds createOsmDb() to connect to dedicated OSM database |
| packages/api/package.json | Switches drizzle-kit generate script to use drizzle.config.ts |
| packages/api/drizzle/meta/0037_snapshot.json | Snapshot including trips.trail_osm_id |
| packages/api/drizzle/meta/_journal.json | Adds migration journal entry for 0037_trips_trail_osm_id |
| packages/api/drizzle/0037_trips_trail_osm_id.sql | Adds trail_osm_id column + index on trips |
| packages/api/drizzle.config.ts | Adds drizzle-kit config; filters out osm tables from main DB schema |
| packages/api/Dockerfile.test | Builds a test Postgres image with PostGIS added |
| packages/api/docker-compose.test.yml | Uses custom PostGIS-enabled test DB image |
| package.json | Adds trails dev script; bumps React to 19.2; bumps expo-sqlite override |
| CLAUDE.md | Updates Expo/RN/Router versions documentation |
| biome.json | Excludes coverage directories from Biome |
| apps/trails/wrangler.jsonc | Adds Trails worker config (assets + rate limiting + vars) |
| apps/trails/worker/index.ts | Adds proxy worker (rate limiting, CORS, edge caching for detail) |
| apps/trails/tsconfig.json | Adds Trails app TS config + path aliases |
| apps/trails/tailwind.config.ts | Adds Tailwind preset + content globs |
| apps/trails/postcss.config.mjs | Adds PostCSS config |
| apps/trails/package.json | Adds Trails app dependencies/scripts |
| apps/trails/next.config.mjs | Configures static export + transpilePackages |
| apps/trails/lib/useAuth.tsx | Adds auth context (register/verify/login/logout + auth gate state) |
| apps/trails/lib/trailSearch.ts | Adds authenticated search client + bbox centroid extraction |
| apps/trails/lib/overpass.ts | Adds public nearby-trails loader via Overpass |
| apps/trails/lib/geolocation.ts | Adds geolocation helper + default centers/zooms |
| apps/trails/lib/auth.ts | Adds resilient localStorage token/user helpers + auth API helpers |
| apps/trails/lib/apiFetch.ts | Adds authed fetch with refresh-on-401 retry |
| apps/trails/components/VerifyEmail.tsx | Adds OTP email verification UI |
| apps/trails/components/ui/sonner.tsx | Adds Toaster wrapper |
| apps/trails/components/TrailsPage.tsx | Main trails page (map + list + gated search + pagination + CTA) |
| apps/trails/components/TrailMap.tsx | Leaflet map + marker rendering (client-only) |
| apps/trails/components/TrailCard.tsx | Trail result card UI |
| apps/trails/components/SearchBar.tsx | Gated search input (opens auth gate when unauth) |
| apps/trails/components/DownloadCTA.tsx | Mobile download CTA banner |
| apps/trails/components/AuthGate.tsx | Radix dialog with register/login/forgot + verify flow |
| apps/trails/app/page.tsx | Renders TrailsPage |
| apps/trails/app/layout.tsx | Adds app shell, metadata, AuthProvider, Toaster |
| apps/trails/app/globals.css | Imports shared globals + Leaflet CSS + z-index tweaks |
| apps/landing/package.json | Bumps React types |
| apps/guides/package.json | Bumps React types |
| apps/expo/package.json | Upgrades Expo/RN deps; swaps nativewindui import path usage |
| apps/expo/features/packs/utils/getPackItemDetailOptions.tsx | Updates nativewindui import path |
| apps/expo/features/packs/utils/getPackDetailOptions.tsx | Updates nativewindui import path |
| apps/expo/features/pack-templates/utils/getPackTemplateItemDetailOptions.tsx | Updates nativewindui import path |
| apps/expo/features/pack-templates/utils/getPackTemplateDetailOptions.tsx | Updates nativewindui import path + formatting |
| apps/expo/features/catalog/types.ts | Updates catalog item date fields to Date |
| apps/expo/features/ai/components/WeatherGenerativeUI.tsx | Updates nativewindui import path + formatting |
| apps/expo/features/ai/components/ToolCard.tsx | Updates nativewindui import path |
| apps/expo/components/Markdown.tsx | Updates nativewindui import path |
| apps/expo/components/initial/ExpandableText.tsx | Updates nativewindui import path |
| apps/expo/components/ErrorState.tsx | Updates nativewindui import path |
| apps/expo/app/(app)/recent-packs.tsx | Adds t as any casts for getRelativeTime calls |
| apps/expo/app/(app)/current-pack/[id].tsx | Adds t as any cast for getRelativeTime call |
| apps/expo/app/(app)/(tabs)/_layout.tsx | Updates expo-router native tabs API usage |
| apps/expo/app/_layout.tsx | Updates nativewindui import path |
| apps/expo/app.config.ts | Adds react-native-maps config plugin |
| apps/admin/package.json | Adds Leaflet deps + bumps React types |
| apps/admin/next.config.mjs | Adds Leaflet webpack externals configuration |
| apps/admin/next-env.d.ts | Adds Next TS env typings file |
| apps/admin/lib/queryKeys.ts | Adds query keys for OSM trail viewer |
| apps/admin/lib/api.ts | Adds admin fetcher for trail geometry endpoint |
| apps/admin/config/nav.ts | Adds “Trail Viewer” nav item |
| apps/admin/components/trail-map.tsx | Adds Leaflet map renderer for trail GeoJSON |
| apps/admin/app/layout.tsx | Loads Leaflet CSS/JS from CDN |
| apps/admin/app/dashboard/trails/page.tsx | Adds Trail Viewer page (load OSM relation ID + render geometry) |
| .env.example | Documents OSM env vars for import + optional sync |
| .coderabbit.yaml | Minor instruction wording tweak |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| reviews: z | ||
| .array( | ||
| z.object({ | ||
| user_name: z.string(), | ||
| user_name: z.string().nullable().optional(), | ||
| user_avatar: z.string().nullable().optional(), | ||
| context: z.record(z.string(), z.string()).nullable().optional(), | ||
| recommends: z.boolean().nullable().optional(), | ||
| rating: z.number(), | ||
| title: z.string(), | ||
| text: z.string(), | ||
| date: z.string(), | ||
| date: z.date(), | ||
| images: z.array(z.string()).nullable().optional(), |
| ) | ||
| .nullable() | ||
| .optional(), | ||
| createdAt: z.string().datetime(), | ||
| updatedAt: z.string().datetime(), | ||
| createdAt: z.date(), | ||
| updatedAt: z.date(), | ||
| }); |
| {children} | ||
| </ThemeProvider> | ||
| </QueryProvider> | ||
| <Script src="https://unpkg.com/leaflet@1.9.4/dist/leaflet.js" strategy="afterInteractive" /> |
| webpack: (config, { isServer }) => { | ||
| // Leaflet is loaded from CDN at runtime (see layout.tsx). Tell webpack to | ||
| // skip bundling leaflet in both the server and client passes so the build | ||
| // succeeds even when `leaflet` is absent from node_modules. The trail-map | ||
| // component uses `{ ssr: false }` so leaflet is never executed server-side. | ||
| if (isServer) { | ||
| const prev = Array.isArray(config.externals) | ||
| ? config.externals | ||
| : config.externals | ||
| ? [config.externals] | ||
| : []; | ||
| config.externals = [...prev, 'leaflet']; | ||
| } else { | ||
| config.externals = { leaflet: 'L' }; | ||
| } | ||
| return config; |
| if (!res.ok) { | ||
| const body = (await res.json().catch(() => ({}))) as { message?: string }; | ||
| throw new Error(body.message ?? `Search failed: ${res.status}`); | ||
| } |
| // Local dev: points directly at the Docker PostGIS container. | ||
| // Replace localConnectionString with your local OSM DB URL. | ||
| "hyperdrive": [ | ||
| { | ||
| "binding": "OSM_HYPERDRIVE", | ||
| "id": "TODO_replace_with_hyperdrive_config_id", | ||
| "localConnectionString": "postgresql://packrat:packrat@localhost:5433/osm" | ||
| } | ||
| ], |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/expo/app/(app)/recent-packs.tsx (1)
37-48:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSame
t as anyviolation as incurrent-pack/[id].tsx— fix thegetRelativeTimesignature instead.Both
getRelativeTimecall sites here castttoanyto paper over the same type mismatch. Fixing the parameter type ingetRelativeTime(as described in the sibling comment oncurrent-pack/[id].tsx) will resolve both occurrences here without any changes at these call sites beyond dropping the cast.As per coding guidelines,
**/*.{ts,tsx}: Noanytype — use proper TypeScript types orunknowninstead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/expo/app/`(app)/recent-packs.tsx around lines 37 - 48, getRelativeTime's parameter type is too loose causing call sites to cast t to any; update the getRelativeTime function signature to accept the correct translation function type (e.g., the i18n TFunction or the app's useTranslations return type) instead of leaving it untyped/using any, import the proper type where getRelativeTime is defined, and adjust its implementation signature so callers in recent-packs.tsx and current-pack/[id].tsx no longer need to use "t as any" (do not introduce any new any usages; prefer the explicit i18n type or unknown if you must).packages/api/src/index.ts (1)
100-117:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse enriched env consistently in queue handlers.
setWorkerEnv(enrichEnv(env))is set, butprocessQueueBatchandCatalogServicestill receive the originalenv. That can bypassOSM_DATABASE_URLenrichment for codepaths that read env directly.Suggested fix
async queue(batch: MessageBatch<unknown>, env: Env): Promise<void> { - setWorkerEnv(enrichEnv(env) as unknown as Record<string, unknown>); + const e = enrichEnv(env); + setWorkerEnv(e as unknown as Record<string, unknown>); if (batch.queue === 'packrat-etl-queue' || batch.queue === 'packrat-etl-queue-dev') { - if (!env.ETL_QUEUE) { + if (!e.ETL_QUEUE) { throw new Error('ETL_QUEUE is not configured'); } @@ - await processQueueBatch({ batch: batch as MessageBatch<CatalogETLMessage>, env }); + await processQueueBatch({ batch: batch as MessageBatch<CatalogETLMessage>, env: e }); @@ - if (!env.EMBEDDINGS_QUEUE) { + if (!e.EMBEDDINGS_QUEUE) { throw new Error('EMBEDDINGS_QUEUE is not configured'); } - await new CatalogService(env, true).handleEmbeddingsBatch(batch); + await new CatalogService(e, true).handleEmbeddingsBatch(batch);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/api/src/index.ts` around lines 100 - 117, The queue handler calls setWorkerEnv(enrichEnv(env)) but still passes the original env into downstream handlers, which can bypass enrichment; change to compute a single enrichedEnv = enrichEnv(env) (and setWorkerEnv(enrichedEnv)), then pass enrichedEnv (appropriately cast) into processQueueBatch and into new CatalogService(...).handleEmbeddingsBatch so both processQueueBatch and CatalogService.receive the enriched environment; update references to processQueueBatch and CatalogService.handleEmbeddingsBatch to use enrichedEnv instead of the original env.
🟠 Major comments (20)
apps/expo/app/(app)/current-pack/[id].tsx-158-158 (1)
158-158:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRemove
t as any— violates the no-anyguidelineThe
as anycast masks a type incompatibility caused by dynamic key construction.getRelativeTimebuilds translation keys at runtime (e.g.,'common.timeAgo.${key}'), but react-i18next'stfunction is strictly typed to accept only literal keys fromen.json.Fix by widening the parameter type in
getRelativeTimeto explicitly accept a generic translation function:♻️ Suggested fix in
apps/expo/lib/utils/getRelativeTime.ts-type TFunction = (key: string, options?: Record<string, unknown>) => string; +type TFunction = (key: string, options?: Record<string, unknown>) => string; // Already sufficientIf the current type is insufficient, ensure it accepts the actual signature of i18next's
tfunction with dynamic keys allowed. Then remove the cast at call sites.Alternatively, change all dynamic key calls to use
twith a guard that passes a fallback:-t(`common.timeAgo.${key}`, { count: val }) +t('common.timeAgo.months' as const, { count: val }) // or handle dynamically🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/expo/app/`(app)/current-pack/[id].tsx at line 158, The call site is using "t as any" to bypass type errors when calling getRelativeTime; instead update the getRelativeTime signature to accept the actual i18next translation function type (e.g., import and use i18next's TFunction or declare t: (key: string, options?: any) => string) so dynamic keys are allowed, then remove the cast at the call site where time: getRelativeTime(pack.localUpdatedAt ?? pack.updatedAt, t). Ensure getRelativeTime uses that widened t type for its internal dynamic key construction and return type so all callers remain type-safe without any-casts.packages/api/src/schemas/catalog.ts-61-61 (1)
61-61:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse
z.coerce.date()orz.string().datetime()for API-facing date fields, not rawz.date()Lines 61, 97, and 98 define fields that will be serialized as ISO strings in JSON responses. Using
z.date()creates a schema mismatch: the validator expects JavaScript Date objects, but JSON transport delivers ISO strings. If response validation is enabled, parsing will fail. TheimageDetectionServicealready exposes this risk by returningDetectedItemWithMatches(which includesCatalogItemSchema) with database-sourced Date objects that serialize to strings.Align with
packages/api/src/types/index.ts, which correctly usesz.string().datetime()for the same fields, or usez.coerce.date()to accept and parse ISO strings.Proposed fix
- date: z.date(), + date: z.coerce.date(), ... - createdAt: z.date(), - updatedAt: z.date(), + createdAt: z.coerce.date(), + updatedAt: z.coerce.date(),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/api/src/schemas/catalog.ts` at line 61, The schema uses raw z.date() for API-facing date fields (e.g., the date property in CatalogItemSchema and the other two date fields) which fails when JSON sends ISO strings; update those fields to either z.coerce.date() to accept and parse ISO string dates or z.string().datetime() to validate ISO strings, and ensure the change is applied in the CatalogItemSchema (and any related schema declarations that currently use z.date()) so runtime JSON responses validate correctly against the schema.packages/api/src/db/schema.ts-338-338 (1)
338-338:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
mode: 'bigint'will crash JSON serialization for any trip with a non-nulltrailOsmId.
bigint('trail_osm_id', { mode: 'bigint' })maps the column to a JavaScriptBigIntprimitive.BigIntis not serializable byJSON.stringify— it throwsTypeError: Do not know how to serialize a BigInt. TheGET /tripsendpoint (and others returningTripobjects) directly serialize responses viaJSON.stringify, so any request returning a row wheretrailOsmIdis populated will result in a 500 error.
mode: 'number'is safe here: OSM node IDs (~12 billion), way IDs (~1 billion), and relation IDs are all well belowNumber.MAX_SAFE_INTEGER(~9 × 10¹⁵). The mode is a TypeScript/JS-mapping-only setting in Drizzle — no SQL migration is needed to change it.Proposed fix
- trailOsmId: bigint('trail_osm_id', { mode: 'bigint' }), + trailOsmId: bigint('trail_osm_id', { mode: 'number' }),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/api/src/db/schema.ts` at line 338, The column mapping for trailOsmId uses BigInt mode which causes JSON.stringify to throw on Trip responses; update the Drizzle column definition trailOsmId: bigint('trail_osm_id', { mode: 'bigint' }) to use mode: 'number' instead (trailOsmId: bigint('trail_osm_id', { mode: 'number' })) so the value is mapped to a JS Number (no DB migration needed) and trips with a populated trailOsmId can be JSON serialized without error.packages/api/wrangler.jsonc-190-195 (1)
190-195:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReplace placeholder Hyperdrive ID before merge
At line 193,
idis still a TODO placeholder. Keeping this in tracked config breaks localwranglerworkflows for anyone usingenv.dev.Replace
"TODO_replace_with_hyperdrive_config_id"with the actual Hyperdrive ID obtained from runningwrangler hyperdrive create.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/api/wrangler.jsonc` around lines 190 - 195, The hyperdrive entry in the "hyperdrive" array still uses the placeholder id "TODO_replace_with_hyperdrive_config_id"; update the id field for the binding "OSM_HYPERDRIVE" to the real Hyperdrive config ID returned by running `wrangler hyperdrive create` so local wrangler workflows using env.dev work correctly — replace the placeholder string in the id property with that actual ID.apps/trails/next.config.mjs-4-9 (1)
4-9:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDisable type and lint gates across all Next.js apps creates deployment risk
The same build-time check disabling pattern exists in all four Next.js apps (
apps/admin,apps/guides,apps/landing, andapps/trails). While root-levelcheck-typesandlint:strictscripts exist, they don't substitute for app-level validation during individual app builds. Disabling both gates at the app level means developers building locally or in CI won't catch regressions in dependencies like@packrat/web-uior@packrat/overpass.Set both
eslint.ignoreDuringBuildsandtypescript.ignoreBuildErrorstofalseacross all Next.js app configs to restore safety gates during builds.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/trails/next.config.mjs` around lines 4 - 9, Change the Next.js app configs to re-enable build-time linting and type checks: set eslint.ignoreDuringBuilds = false and typescript.ignoreBuildErrors = false in each Next.js config where they were disabled (e.g., the configs for apps/admin, apps/guides, apps/landing, apps/trails), so the eslint.ignoreDuringBuilds and typescript.ignoreBuildErrors flags no longer bypass build gates and CI/local builds will fail on lint/type errors.apps/expo/app.config.ts-69-69 (1)
69-69:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard the maps plugin on missing API key
At line 69, the
react-native-mapsplugin is always added to the plugins array even whenEXPO_PUBLIC_GOOGLE_MAPS_API_KEYis undefined. This can cause configuration fragility, especially in development environments without the key set.Conditionally include the plugin only when the API key is defined:
Suggested fix
+const mapsApiKey = process.env.EXPO_PUBLIC_GOOGLE_MAPS_API_KEY; + export default (): ExpoConfig => withSentry( { plugins: [ 'expo-router', - ['react-native-maps', { iosGoogleMapsApiKey: process.env.EXPO_PUBLIC_GOOGLE_MAPS_API_KEY }], + ...(mapsApiKey + ? [['react-native-maps', { iosGoogleMapsApiKey: mapsApiKey }] as const] + : []), '@react-native-community/datetimepicker',🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/expo/app.config.ts` at line 69, Conditionally include the react-native-maps plugin only when process.env.EXPO_PUBLIC_GOOGLE_MAPS_API_KEY is defined: update the plugins array construction so that the entry ['react-native-maps', { iosGoogleMapsApiKey: process.env.EXPO_PUBLIC_GOOGLE_MAPS_API_KEY }] is added only if that env var is truthy (e.g., via a conditional push/filter), leaving the plugins array unchanged when the key is missing; target the plugins array creation in app.config.ts and reference the exact plugin tuple and process.env.EXPO_PUBLIC_GOOGLE_MAPS_API_KEY when making the change.packages/api/drizzle.config.ts-12-16 (1)
12-16:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse a placeholder database URL to avoid blocking schema generation
At lines 12-16, the IIFE throw executes during config module load, which blocks
db:generatewhenNEON_DATABASE_URLis unset. Sincedrizzle-kit generateonly needs the dialect and schema path (not a live database connection), replace the throw with a placeholder URL.Suggested fix
export default defineConfig({ schema: './src/db/schema.ts', out: './drizzle', dialect: 'postgresql', tablesFilter: ['!osm_ways', '!osm_routes'], dbCredentials: { - url: - nodeEnv.NEON_DATABASE_URL ?? - (() => { - throw new Error('NEON_DATABASE_URL is not set'); - })(), + url: nodeEnv.NEON_DATABASE_URL ?? 'postgresql://placeholder/placeholder', }, });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/api/drizzle.config.ts` around lines 12 - 16, The config currently throws an error during module load when nodeEnv.NEON_DATABASE_URL is undefined (the IIFE used for the url field), which blocks db:generate; instead modify the url expression so that if nodeEnv.NEON_DATABASE_URL is missing it returns a harmless placeholder database URL (e.g., a localhost/postgres placeholder) rather than throwing. Locate the url property in drizzle.config.ts (the url: nodeEnv.NEON_DATABASE_URL ?? (() => { ... })() expression) and replace the throwing IIFE with one that returns a placeholder string so schema generation can run without a live connection.apps/admin/app/layout.tsx-34-34 (2)
34-34:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMissing Subresource Integrity (SRI) on CDN-loaded Leaflet assets in the admin panel.
Leaflet's own documentation explicitly states that "integrity hashes are included for security when using Leaflet from CDN" — yet both resources here omit the
integrityattribute. Loading JavaScript or CSS from third-party CDNs is convenient but introduces a security risk; if the CDN is compromised, attackers could inject malicious code into your website. This risk is especially acute for an admin dashboard where session tokens are present.🛡️ Proposed fix — add SRI and crossorigin to both tags
- <link rel="stylesheet" href="https://unpkg.com/leaflet@1.9.4/dist/leaflet.css" /> + <link + rel="stylesheet" + href="https://unpkg.com/leaflet@1.9.4/dist/leaflet.css" + integrity="<sha256/384 hash from leafletjs.com/download>" + crossOrigin="anonymous" + />- <Script src="https://unpkg.com/leaflet@1.9.4/dist/leaflet.js" strategy="afterInteractive" /> + <Script + src="https://unpkg.com/leaflet@1.9.4/dist/leaflet.js" + strategy="afterInteractive" + integrity="<sha256/384 hash from leafletjs.com/download>" + crossOrigin="anonymous" + />The canonical hashes for 1.9.4 are published on https://leafletjs.com/download.html. Alternatively, vendor Leaflet through the already-installed npm package and drop the CDN entirely.
Also applies to: 47-47
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/admin/app/layout.tsx` at line 34, The Leaflet CDN <link> and corresponding <script> includes in apps/admin/app/layout.tsx are missing Subresource Integrity and crossorigin attributes; update the Leaflet <link rel="stylesheet" href="https://unpkg.com/leaflet@1.9.4/dist/leaflet.css" /> and the Leaflet <script src=".../leaflet.js"> tag to include the canonical integrity hashes for version 1.9.4 and add crossorigin="anonymous", or alternatively replace the CDN references by importing Leaflet from the installed npm package and serving the assets locally.
34-34:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMissing Subresource Integrity (SRI) on CDN-loaded Leaflet assets in the admin panel.
Both CDN resources (
leaflet.cssandleaflet.js) are loaded from unpkg withoutintegrityattributes. In an admin dashboard, a compromised or substituted CDN response can exfiltrate session tokens or perform privileged actions. Add SRI hashes for both resources.🛡️ Proposed fix with SRI
- <link rel="stylesheet" href="https://unpkg.com/leaflet@1.9.4/dist/leaflet.css" /> + <link + rel="stylesheet" + href="https://unpkg.com/leaflet@1.9.4/dist/leaflet.css" + integrity="sha256-p4NxAoJBhIIN+hmNHrzRCf9tD/miZyoHS5obTRR9BMY=" + crossOrigin="anonymous" + />- <Script src="https://unpkg.com/leaflet@1.9.4/dist/leaflet.js" strategy="afterInteractive" /> + <Script + src="https://unpkg.com/leaflet@1.9.4/dist/leaflet.js" + strategy="afterInteractive" + integrity="sha256-20nQCchB9co0qIjJZRGuk2/Z9VM+kNiyxNV1lvTlZBo=" + crossOrigin="anonymous" + />Alternatively, vendor Leaflet via the npm package already in
package.jsonand skip CDN loading entirely.Also applies to: 47-47
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/admin/app/layout.tsx` at line 34, The link/script tags loading Leaflet from unpkg in layout.tsx lack Subresource Integrity; update the <link rel="stylesheet" href="https://unpkg.com/leaflet@1.9.4/dist/leaflet.css" /> and the corresponding <script src=".../leaflet.js"> tag to include proper integrity="sha384-..." and crossorigin="anonymous" attributes using the official SRI hashes for Leaflet v1.9.4, or alternatively remove the CDN tags and import Leaflet from the installed npm package (e.g., import 'leaflet/dist/leaflet.css' and import L from 'leaflet') to vendor the assets locally.apps/trails/app/globals.css-19-31 (1)
19-31:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFlattening all Leaflet pane z-indices to 10 breaks the library's internal layering.
Map panes use the z-index CSS property to always show some layers on top of others — specifically tiles at 200, overlays at 400, markers at 600, popups at 700. Because
.leaflet-tile-pane,.leaflet-marker-pane, etc. all also carry the.leaflet-paneclass, and both.leaflet-tile-pane { z-index: 200 }(from the@imported Leaflet CSS) and.leaflet-pane { z-index: 10 }(from this file) have the same CSS specificity (0,1,0), the later declaration wins — your rule overrides every specific pane's z-index. The resulting layering then relies entirely on DOM sibling order, which is fragile across Leaflet versions.The same problem applies to
.leaflet-top/.leaflet-bottom(controls) and.leaflet-control: dropping them from 1000/800 to 10 puts them at the same level as tile panes in the stacking context.The idiomatic fix is to create an isolated stacking context on the map container and leave Leaflet's internals untouched:
🛠 Proposed fix — use CSS isolation instead
-/* Ensure Leaflet z-index plays nicely with modals */ -.leaflet-pane { - z-index: 10; -} - -.leaflet-top, -.leaflet-bottom { - z-index: 10; -} - -.leaflet-control { - z-index: 10; -} /* Leaflet map container */ .trail-map { - `@apply` h-full w-full rounded-lg; + `@apply` h-full w-full rounded-lg; + /* Isolate the map's stacking context so it sits below modals + without touching Leaflet's internal pane z-index hierarchy. */ + isolation: isolate; + z-index: 0; }Then ensure modals in the page have
z-indexgreater than 0 (standard dialog/sheet implementations already do this).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/trails/app/globals.css` around lines 19 - 31, The CSS is overriding Leaflet's pane-specific z-indexes by setting .leaflet-pane, .leaflet-top, .leaflet-bottom and .leaflet-control to 10; remove those overrides and instead create an isolated stacking context on your map container (e.g., the element that holds Leaflet) so Leaflet's internal pane z-indexes (tile/overlay/marker/popup) remain intact while allowing modals to sit above the map; in practice, delete or stop emitting the rules for .leaflet-pane/.leaflet-top/.leaflet-bottom/.leaflet-control and add a stacking-context rule on the map container (so modals with higher z-index still appear above the map).apps/trails/lib/apiFetch.ts-12-40 (1)
12-40:⚠️ Potential issue | 🟠 Major | ⚡ Quick winConcurrent 401s race the refresh-token rotation.
If two requests are in flight when the access token expires, both will hit 401, both will read the same
refreshTokenfrom storage, and both will callapiRefreshTokenindependently. If the backend rotates refresh tokens (a single-use refresh token is the standard pattern), the second call will be invalidated by the first and will throwAuthExpiredError, kicking the user out even though the session is healthy. Consider de-duplicating in-flight refreshes by caching the refresh promise on the module scope and awaiting the same promise from every caller.Sketch
let inflightRefresh: Promise<{ accessToken: string; refreshToken: string }> | null = null; async function refreshOnce(refreshToken: string) { if (!inflightRefresh) { inflightRefresh = apiRefreshToken(refreshToken).finally(() => { inflightRefresh = null; }); } return inflightRefresh; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/trails/lib/apiFetch.ts` around lines 12 - 40, The authedFetch flow can race concurrent 401 responses and call apiRefreshToken multiple times; add a module-scoped inflightRefresh Promise (e.g., let inflightRefresh: Promise<{accessToken:string;refreshToken:string}>|null) and implement a helper refreshOnce(refreshToken:string) that, if inflightRefresh is null, sets inflightRefresh = apiRefreshToken(refreshToken).finally(()=> inflightRefresh = null) and returns inflightRefresh; update authedFetch to call refreshOnce(refreshToken) instead of apiRefreshToken so multiple callers awaiting a refresh share the same promise, then use the resolved {accessToken, refreshToken: newRefresh} to setTokens and retry the original request.packages/osm-import/sync.ts-45-92 (1)
45-92:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid passing connection URLs (with passwords) on the command line.
Both
LOCAL_URLandPRODUCTION_URLare spawned as positional arguments topg_dump/pg_restore, which means the embedded password is visible inps/process listings to any local user, and may also leak into shell history or audit logs of CI runners. Prefer splitting into discrete--host/--port/--user/--dbnameflags and exporting the password via thePGPASSWORDenv on the spawn (or use a.pgpass/service file).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/osm-import/sync.ts` around lines 45 - 92, The pg_dump/pg_restore invocations pass LOCAL_URL and PRODUCTION_URL as positional args (in the Bun.spawn calls), leaking passwords to process listings; instead parse each connection URL (LOCAL_URL and PRODUCTION_URL) into host, port, user, dbname and pass those as explicit flags (e.g. --host, --port, --username, --table/--dbname or -d) to pg_dump/pg_restore, and supply the password via the spawn environment (PGPASSWORD) in the Bun.spawn options (or alternatively rely on .pgpass/service file); update the dump spawn and restore spawn calls (where Bun.spawn is invoked, and variables dumpPath, LOCAL_URL, PRODUCTION_URL are referenced) to remove the raw URL positional argument and add the parsed flags plus env: { ...process.env, PGPASSWORD: password }.apps/trails/worker/index.ts-65-68 (1)
65-68:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid public edge-caching authenticated trail detail responses.
Cache-Control: publicis set even when anAuthorizationheader is present. If/api/trails/:id
ever includes user-scoped fields, this can leak cached data across users.Suggested fix
- if (TRAIL_DETAIL_RE.test(url.pathname) && request.method === 'GET') { - headers.set('Cache-Control', 'public, max-age=3600, stale-while-revalidate=600'); + if (TRAIL_DETAIL_RE.test(url.pathname) && request.method === 'GET') { + const isAuthed = Boolean(request.headers.get('Authorization')); + if (!isAuthed) { + headers.set('Cache-Control', 'public, max-age=3600, stale-while-revalidate=600'); + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/trails/worker/index.ts` around lines 65 - 68, The current conditional sets Cache-Control to "public" for GET trail detail responses even when requests include Authorization, risking cross-user leaks; update the logic in the handler that checks TRAIL_DETAIL_RE (the block using TRAIL_DETAIL_RE.test(url.pathname), request.method, and headers.set) to detect an Authorization header on the incoming request and, if present, avoid public edge caching—either set a more restrictive header like "private, max-age=0, must-revalidate" or skip setting Cache-Control entirely for authenticated requests, while leaving the existing public cache behavior for unauthenticated GETs.apps/trails/lib/trailSearch.ts-27-44 (1)
27-44:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
bbox.coordinatestyping is too deep and makes center extraction fragile.The parser assumes a polygon ring, but
number[][][][]models an extra nesting level. This can
silently producenullcenters depending on payload shape.Suggested fix
interface ApiTrail { @@ - bbox: { coordinates?: number[][][][] } | null; + bbox: { coordinates?: number[][][] } | null; } @@ - const ring = bbox.coordinates[0]; + const ring = bbox.coordinates[0]; if (!ring) return null;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/trails/lib/trailSearch.ts` around lines 27 - 44, The bbox.coordinates type is over-nested (number[][][][]) and makes bboxCenter brittle; update the ApiTrail['bbox'] coordinates typing to a more realistic shape (e.g., number[][][] or a flexible union like Array<Array<number[]>> | number[][][]) and modify bboxCenter to robustly locate the polygon ring by descending one nesting level if needed (inspect bbox.coordinates[0] and bbox.coordinates[0][0] until you find an array of [lon, lat] pairs), then compute centroid using that ring; reference the bboxCenter function and the ApiTrail['bbox'] coordinates property when making the change.apps/trails/components/TrailMap.tsx-21-66 (1)
21-66:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard async Leaflet init against unmount to avoid detached-map initialization.
If the component unmounts before
import('leaflet')resolves,init()can still create a map on a stale element. Add a cancellation flag check before map creation.Suggested fix
useEffect(() => { const container = containerRef.current; if (!container) return; + let cancelled = false; let L: typeof import('leaflet'); const initialCenter = center; async function init(el: HTMLDivElement) { L = (await import('leaflet')).default; + if (cancelled) return; // Fix default icon paths broken by webpack/bun bundlers @@ if (mapRef.current) return; // already initialized const map = L.map(el, { @@ } - init(container); + void init(container); return () => { + cancelled = true; mapRef.current?.remove(); mapRef.current = null; markersRef.current = null; }; }, []); // eslint-disable-line react-hooks/exhaustive-deps🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/trails/components/TrailMap.tsx` around lines 21 - 66, The async init function can create a Leaflet map after the component unmounts; to fix, add a cancellation flag (e.g., let cancelled = false) in the useEffect scope, set cancelled = true in the cleanup, and inside init check the flag immediately after awaiting import('leaflet') and again before creating the map and assigning mapRef.current/markersRef.current; only proceed with L.map, L.tileLayer, and ref assignments when cancelled is false to avoid initializing on a stale element (references: init, L, containerRef, mapRef, markersRef in the shown useEffect).apps/trails/components/TrailsPage.tsx-50-69 (1)
50-69:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFallback-center flow skips public trail loading and geolocation failures can stall the page.
loadNearbyTrails()only runs whencoordsexists. If location is denied (orgetUserLocation()throws), users get the fallback map but no public trails (or stay stuck in loading on throw).Suggested fix
async function loadMap() { - const coords = await getUserLocation(); - if (cancelled) return; - - const center = coords ?? DEFAULT_CENTER; - setMapCenter(center); - setMapState({ status: 'idle', center }); - - if (!coords) { - setMapState({ status: 'idle', center: DEFAULT_CENTER }); - return; - } - try { + const coords = await getUserLocation(); + if (cancelled) return; + const center = coords ?? DEFAULT_CENTER; + setMapCenter(center); + setMapState({ status: 'idle', center }); + const trails = await loadNearbyTrails(center[0], center[1]); if (!cancelled) setPublicTrails(trails); } catch { - // Overpass failure is non-fatal; map still shows with no trails - if (!cancelled) setPublicTrails([]); + if (cancelled) return; + setMapCenter(DEFAULT_CENTER); + setMapState({ status: 'idle', center: DEFAULT_CENTER }); + setPublicTrails([]); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/trails/components/TrailsPage.tsx` around lines 50 - 69, The loadMap function currently only calls loadNearbyTrails when getUserLocation returns coords and will also stall if getUserLocation throws; update loadMap so it catches errors from getUserLocation (treat as no coords) and always calls loadNearbyTrails using center = coords ?? DEFAULT_CENTER, then setPublicTrails with the result (or empty array on Overpass failure), and ensure setMapState is updated to 'idle' with the chosen center regardless of geolocation success; refer to loadMap, getUserLocation, loadNearbyTrails, setMapState, setPublicTrails, DEFAULT_CENTER and cancelled to locate and implement these changes.apps/trails/lib/auth.ts-96-103 (1)
96-103:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon’t treat a missing
userIdas a successful register response.Returning
''here masks a malformed success payload and makes downstream auth flow bugs much harder to diagnose. The other auth helpers already fail fast on missing required fields; register should do the same.Proposed fix
export async function apiRegister(opts: { email: string; password: string; username: string; }): Promise<{ userId: string }> { const data = await authFetch('/auth/register', opts); - return { userId: data.userId ?? '' }; + if (!data.userId) { + throw new Error('Registration failed: missing userId'); + } + return { userId: data.userId }; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/trails/lib/auth.ts` around lines 96 - 103, The apiRegister function currently returns an empty string when data.userId is missing, which masks malformed responses; update apiRegister to validate that authFetch('/auth/register', opts) returns a non-empty data.userId and, if not, throw a descriptive error (e.g., "Missing userId in register response") including the raw response data for debugging instead of returning ''. Keep the same return type Promise<{ userId: string }> and fail fast on missing/invalid userId so downstream callers don't receive an empty id.packages/api/src/routes/trails/index.ts-91-94 (1)
91-94:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd a stable tiebreaker for offset pagination.
Sorting only by
nameis nondeterministic for duplicate names, solimit/offsetpaging can skip or repeat rows between requests. Use a unique final sort key such asosm_id.Proposed fix
ORDER BY CASE WHEN name IS NOT NULL THEN 0 ELSE 1 END, - name + name, + osm_id LIMIT ${limit + 1} OFFSET ${offset}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/api/src/routes/trails/index.ts` around lines 91 - 94, The ORDER BY clause currently sorts by the NULLs-first expression and then by name, which is nondeterministic for duplicate names and breaks offset pagination; update the ORDER BY in the SQL (the clause using "CASE WHEN name IS NOT NULL THEN 0 ELSE 1 END, name") to append a unique tiebreaker such as osm_id so the final sort keys are stable (e.g., add ", osm_id" after name) while keeping the existing NULL-handling and respecting the existing limit and offset variables (limit, offset).apps/trails/lib/auth.ts-83-93 (1)
83-93:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle non-JSON auth failures before parsing.
If the worker, proxy, or upstream auth route returns HTML or an empty body on failure,
res.json()throws aSyntaxErrorbefore you reach the!res.okbranch. That hides the real HTTP failure from login/register flows.Proposed fix
async function authFetch(path: string, body: Record<string, string>): Promise<AuthResponse> { const res = await fetch(`${API_BASE}${path}`, { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(body), }); - const data = fromZod(AuthResponseSchema)(await res.json()) ?? {}; + let data: AuthResponse = {}; + const contentType = res.headers.get('content-type') ?? ''; + + if (contentType.includes('application/json')) { + data = fromZod(AuthResponseSchema)(await res.json()) ?? {}; + } + if (!res.ok) { throw new Error(data.message ?? `Request failed: ${res.status}`); } return data; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/trails/lib/auth.ts` around lines 83 - 93, authFetch currently calls res.json() unconditionally so non-JSON error responses (HTML/empty) throw a SyntaxError before the res.ok check; update authFetch to check res.ok first and only parse JSON when appropriate: if res.ok, parse JSON and validate with AuthResponseSchema; if !res.ok, attempt to read text (res.text()) and include that text or status in the thrown Error (and fall back to content-type check or an empty-body message if text is empty) so the real HTTP failure is surfaced instead of a parse exception.packages/api/src/routes/trails/index.ts-50-72 (1)
50-72:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReject incomplete spatial filters instead of silently dropping them.
A request like
q=foo&lat=37.5currently passes validation and falls back to text-only search because the spatial predicate is only added when both coordinates are present. That returns broader results than the caller asked for.Proposed fix
const { q, lat, lon, radius = 50, sport, limit = 50, offset = 0 } = query; + const hasLat = lat !== undefined; + const hasLon = lon !== undefined; - if (!q && (lat === undefined || lon === undefined)) { + if (hasLat !== hasLon) { + return status(400, { error: 'lat and lon must be provided together' }); + } + + if (query.radius !== undefined && (!hasLat || !hasLon)) { + return status(400, { error: 'radius requires both lat and lon' }); + } + + if (!q && (!hasLat || !hasLon)) { return status(400, { error: 'Provide q (text) and/or lat+lon for spatial search' }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/api/src/routes/trails/index.ts` around lines 50 - 72, The handler currently allows requests with only one spatial coordinate (lat or lon) and silently ignores spatial filtering; update the validation after destructuring query ({ q, lat, lon, radius, sport, limit, offset }) to reject requests that supply only one of lat or lon by returning status(400, { error: 'Provide both lat and lon for spatial search' }) so callers don’t get broader text-only results; keep the rest of the logic (createOsmDb, conditions array, sql predicates) intact and only proceed to build the ST_DWithin predicate when both lat and lon are present.
🧹 Nitpick comments (13)
apps/expo/app/(app)/(tabs)/_layout.tsx (2)
23-26: 💤 Low valueFeed tab is using the home icon asset instead of a feed-specific icon.
src={require('../../../assets/tab-icons/home.png')}is passed for the feed tab. This was likely pre-existing, but worth confirming it's intentional or if a dedicated feed icon should be used.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/expo/app/`(app)/(tabs)/_layout.tsx around lines 23 - 26, The Feed tab's NativeTabs.Trigger.Icon is using the home icon asset; update the src passed to NativeTabs.Trigger.Icon for the feed tab to the correct feed-specific asset (e.g., require('../../../assets/tab-icons/feed.png') or the actual feed icon filename in assets) so the feed tab displays the proper icon instead of home.
3-3: 💤 Low valueNative tabs remains a beta API in Expo Router 55.x with no stable alternative available.
unstable-native-tabsis the officially documented approach for SDK 55 (per https://docs.expo.dev/router/advanced/native-tabs/), and its API is subject to change. The~55.0.13version constraint allows patch updates but permits minor-version changes (e.g., 55.1.0) that could introduce breaking changes. Consider adding a comment in the code or a tracking item in your issue tracker to review API changes when upgrading within the 55.x series.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/expo/app/`(app)/(tabs)/_layout.tsx at line 3, The import of NativeTabs from 'expo-router/unstable-native-tabs' uses a beta API whose surface may change within the 55.x series; add a clear inline comment next to the import (referencing NativeTabs) that notes this is an unstable/beta API for Expo Router 55.x and include a reminder to review the API when upgrading patch/minor versions (or create a tracking issue in the project tracker) so future upgrades within 55.x are verified for breaking changes.packages/osm-db/tsconfig.json (1)
1-11: 💤 Low valueConsider adding
declaration: trueifpackages/osm-db/srcexports types consumed by other workspace packages.Without
declaration: true,tscwon't emit.d.tsfiles intodist/, meaning downstream packages that import frompackages/osm-dbvia the compiled output won't receive proper type information. If this package is only used as a script runner (migrate + drizzle config) and all consumers import source directly, this is a non-issue.💡 Proposed addition
"compilerOptions": { "target": "ES2022", "module": "ESNext", "moduleResolution": "bundler", "strict": true, "skipLibCheck": true, - "outDir": "dist" + "outDir": "dist", + "declaration": true, + "declarationDir": "dist" },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/osm-db/tsconfig.json` around lines 1 - 11, Add declaration generation to the TypeScript build by setting compilerOptions.declaration to true in the tsconfig.json for the package (update the "compilerOptions" object in packages/osm-db/tsconfig.json to include "declaration": true); optionally also add "declarationMap": true if you want sourcemaps for declarations and ensure "outDir": "dist" remains correct so emitted .d.ts files land in dist for consumers of the package.packages/api/src/db/index.ts (1)
60-76: 🏗️ Heavy lift
createOsmDbpasses the main-app schema to Drizzle, making all OSM table queries untyped.
createConnectionis hardcoded to{ schema }(the PackRat main-app schema imported at the top of the file). The OSM database has its own tables (osm_ways,osm_routesfrompackages/osm-db/src/schema.ts), which are absent from that schema. As a result, the returned Drizzle instance has no typed table definitions for OSM entities —db.query.osm_waysdoesn't exist, and all OSM queries are forced to use rawdb.execute(sql...). This silently loses the type-safety that Drizzle provides.The proper fix is to make
createConnectionaccept a generic schema (or add a separatecreateOsmConnection):♻️ Sketch of a schema-generic createConnection
-import * as schema from '@packrat/api/db/schema'; +import * as mainSchema from '@packrat/api/db/schema'; +import * as osmSchema from '@packrat/osm-db/schema'; // add `@packrat/osm-db` workspace dep -const createConnection = (url: string, useNeonHttp?: boolean) => { +const createConnection = <S extends Record<string, unknown>>( + url: string, + schema: S, + useNeonHttp?: boolean, +) => { if (isStandardPostgresUrl(url)) { // ... - return drizzlePg(pool, { schema }); + return drizzlePg(pool, { schema }); } // ... }; export const createDb = () => { const { NEON_DATABASE_URL } = getEnv(); - return createConnection(NEON_DATABASE_URL); + return createConnection(NEON_DATABASE_URL, mainSchema); }; export const createOsmDb = () => { const { OSM_DATABASE_URL } = getEnv(); if (!OSM_DATABASE_URL) { throw ...; } - return createConnection(OSM_DATABASE_URL); + return createConnection(OSM_DATABASE_URL, osmSchema); };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/api/src/db/index.ts` around lines 60 - 76, createOsmDb currently calls createConnection which is hardcoded to the PackRat main-app schema, so the returned Drizzle instance lacks OSM table typings; change the implementation so createConnection can accept a schema parameter (or add a new createOsmConnection) and pass the OSM DB schema (imported from packages/osm-db/src/schema.ts, e.g. osm schema object) instead of the main-app schema when creating the connection in createOsmDb, ensuring the returned Drizzle client exposes typed tables like osm_ways/osm_routes for type-safe queries.apps/admin/lib/api.ts (1)
261-270: 💤 Low value
geometry: object | nullshould use a more precise type.
objectaccepts any non-primitive. Since this field carries a GeoJSON geometry, usingGeoJSON.Geometry | null(from@types/geojson, which is a transitive dep in most mapping setups) gives callers type-safe access to.typeand.coordinateswithout downstream casts.♻️ Proposed improvement
+import type { Geometry } from 'geojson'; + export interface TrailGeometry { osmId: string; name: string | null; sport: string | null; network: string | null; distance: string | null; difficulty: string | null; description: string | null; - geometry: object | null; + geometry: Geometry | null; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/admin/lib/api.ts` around lines 261 - 270, The TrailGeometry interface's geometry field is typed too loosely as object | null; update it to use the GeoJSON geometry type (e.g., GeoJSON.Geometry | null or import { Geometry } from 'geojson' and use Geometry | null) so callers get typed .type and .coordinates; adjust the file imports to bring in the GeoJSON type (or reference the global GeoJSON namespace) and replace the geometry: object | null declaration in the TrailGeometry interface accordingly.packages/osm-db/src/schema.ts (1)
51-51: ⚡ Quick win
ref: numberin members JSONB type may lose precision for very large OSM IDs.OSM member ref IDs are
bigintin the DB column but typed asnumberin the JSONB payload. Current OSM IDs (~10⁹–10¹⁰) are within JS safe integer range, butnumberprovides no compile-time guard if serialized values exceed 2⁵³ − 1. Usingstring(orbigint) forrefmakes the intent explicit and is safe for deserialization.♻️ Proposed change
- members: jsonb('members').$type<Array<{ type: string; ref: number; role: string }>>(), + members: jsonb('members').$type<Array<{ type: string; ref: string; role: string }>>(),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/osm-db/src/schema.ts` at line 51, The members JSONB type in schema.ts currently declares ref as number which can lose precision for very large OSM IDs; update the type used in jsonb('members').$type for the members array (the line with jsonb('members').$type<Array<{ type: string; ref: number; role: string }>>()) to use string (or bigint-as-string) for ref instead of number so deserialized IDs are preserved and type-safe; ensure downstream code that consumes this field treats ref as a string (or converts to BigInt explicitly) to avoid runtime precision loss.apps/admin/components/trail-map.tsx (1)
6-9: 💤 Low value
geometry: objectis overly broad — prefer a GeoJSON type.
objectaccepts any non-primitive, giving no IDE assistance or type safety on the prop. Since@types/leafletalready depends on@types/geojson, theGeoJSON.GeoJsonObjecttype is available at no extra cost.♻️ Proposed refactor
+import type { GeoJsonObject } from 'geojson'; interface TrailMapProps { - geometry: object; + geometry: GeoJsonObject; name: string | null; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/admin/components/trail-map.tsx` around lines 6 - 9, Replace the overly broad geometry: object in the TrailMapProps interface with a GeoJSON type for better type safety — e.g., change TrailMapProps to use geometry: GeoJSON.GeoJsonObject (or import type { GeoJsonObject } from 'geojson' and use geometry: GeoJsonObject) and update any usages in the TrailMap component to match; ensure you add the type import if your environment doesn't expose the global GeoJSON namespace.apps/trails/tsconfig.json (1)
2-28: ⚡ Quick winMissing
noUncheckedIndexedAccess— required by coding guidelines.
strict: truedoes not includenoUncheckedIndexedAccess. The project guideline explicitly requires it ("no unchecked indexed access in TypeScript"), which means array/record reads likearr[i]will not be typed asT | undefined.🛠️ Proposed fix
"strict": true, + "noUncheckedIndexedAccess": true,As per coding guidelines: "Strict null checks enabled; no unchecked indexed access in TypeScript."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/trails/tsconfig.json` around lines 2 - 28, The tsconfig's compilerOptions are missing the required noUncheckedIndexedAccess flag; update the "compilerOptions" in apps/trails/tsconfig.json to add "noUncheckedIndexedAccess": true so array/object indexed reads are typed as possibly undefined—modify the existing compilerOptions block (where "lib", "strict", "noEmit", etc. are defined) to include this setting to comply with the project's "no unchecked indexed access" guideline.packages/osm-import/import.ts (1)
67-67: 💤 Low valueConsole log will display
undefinedwhenIMPORT_MODEis not set.
nodeEnv.IMPORT_MODEis optional, soMode: undefinedis printed in the default case. Consider falling back to a label that matches the actual flags chosen on line 73.Proposed fix
-console.log(`Mode: ${IMPORT_MODE}`); +console.log(`Mode: ${IMPORT_MODE === 'append' ? 'append' : 'create'}`);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/osm-import/import.ts` at line 67, The console.log currently prints IMPORT_MODE which may be undefined (nodeEnv.IMPORT_MODE); instead compute a fallback human-readable label and log that—create a derived importModeLabel (e.g., importModeLabel = nodeEnv.IMPORT_MODE ?? <label computed from the runtime flags used for import>) and use that in the console.log; reference IMPORT_MODE/nodeEnv.IMPORT_MODE and the runtime flags checked later in this file to decide the fallback label so the printed "Mode:" always reflects the actual chosen flags.apps/trails/lib/apiFetch.ts (1)
17-34: 💤 Low valueRetried request reuses the original
init.body, which may already be consumed.When
init.bodyis aReadableStream,Blob, orFormDatainstance that the underlyingfetchhas already drained, the secondfetch(input, { ...init, headers })call will fail (locked/consumed body) instead of succeeding after refresh. For the current trails usage (mostly GET) this is latent, but the helper is exported and easy to misuse. At minimum, document the constraint or buffer the body to a string/ArrayBufferbefore the first call when retry is needed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/trails/lib/apiFetch.ts` around lines 17 - 34, The retry path in apiFetch reuses the original init.body which may be a consumed ReadableStream/Blob/FormData; update apiFetch to detect a body in the provided init (or Request) and, before the first fetch, buffer it into a reusable representation (e.g., ArrayBuffer or string) and replace init.body with that buffer so the same body can be reused on retry after apiRefreshToken succeeds; ensure buffering occurs only when needed, preserve headers like Content-Type, and continue to call setTokens, clearTokens, clearUser and throw AuthExpiredError as before if no refresh token.packages/osm-db/migrate.ts (1)
3-12: 💤 Low value
wsimport +neonConfig.webSocketConstructorare unused on the neon-http path.The Neon branch uses
neon()+drizzle-orm/neon-http, which performs migrations over HTTP and does not open a WebSocket. Theimport WebSocket from 'ws'and theneonConfig.webSocketConstructor = WebSocketassignment can be removed (andwsfrom this package's deps), reducing bundle/runtime surface.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/osm-db/migrate.ts` around lines 3 - 12, Remove the unused WebSocket wiring: delete the import "WebSocket from 'ws'" and the assignment "neonConfig.webSocketConstructor = WebSocket" because the neon() + drizzle from 'drizzle-orm/neon-http' path uses HTTP migrations and does not need a WebSocket constructor; also remove 'ws' from this package's dependencies. Locate these symbols (the WebSocket import and neonConfig.webSocketConstructor usage) in the file alongside the neon() / drizzle ('drizzle-orm/neon-http') imports and remove them so only the HTTP-based neon/drizzle path remains.packages/api/src/services/trails.ts (2)
31-48: 💤 Low valueArray literal can be simplified with Drizzle's array binding.
sql.join(wayRefs.map((ref) => sql\${ref}`), sql`, `)then wrapping inARRAY[...]::bigint[]` works, but Drizzle binds JS arrays directly when typed, so you can pass the array as a single parameter. Less SQL-string fuss and one parameter regardless of input size.Proposed simplification
- const arrayLiteral = sql.join( - wayRefs.map((ref) => sql`${ref}`), - sql`, `, - ); - - const result = await db.execute(sql` + const result = await db.execute(sql` SELECT ST_AsGeoJSON( ST_LineMerge( ST_Collect(geometry ORDER BY ordinality) ) ) AS geojson FROM osm_ways JOIN unnest( - ARRAY[${arrayLiteral}]::bigint[] + ${wayRefs}::bigint[] ) WITH ORDINALITY AS t(osm_id, ordinality) USING (osm_id) WHERE geometry IS NOT NULL `);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/api/src/services/trails.ts` around lines 31 - 48, The current code builds arrayLiteral via sql.join(wayRefs.map(...)) and inlines it into ARRAY[...]::bigint[], which is unnecessary; instead bind the entire wayRefs JS array as a single typed parameter when calling db.execute (remove sql.join and the ARRAY[...] wrapper) by passing wayRefs as a Drizzle array binding (typed as bigint[]) into the SQL used in db.execute (update the JOIN unnest(...) WITH ORDINALITY AS t(osm_id, ordinality) to use that bound parameter). Keep references to the existing variables: replace usage of arrayLiteral and wayRefs in the db.execute call and remove the sql.join creation code.
5-29: ⚡ Quick winTighten the
OsmMemberSchematype to an enum to surface parsing errors at validation time.The filter
m.type === 'w'is correct —routes.luaserializes member types as single-character codes ('w','n','r') from the osm2pgsql Lua API. However,OsmMemberSchemausestype: z.string()without constraints. Consider changing it totype: z.enum(['w', 'n', 'r'])to catch any serialization drift or upstream schema changes at parse time rather than silently filtering out mismatched members.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/api/src/services/trails.ts` around lines 5 - 29, OsmMemberSchema currently allows any string for the member "type", which hides malformed/changed data; tighten it by changing OsmMemberSchema's type from z.string() to z.enum(['w','n','r']) so validation fails fast and OsmMember (the inferred type) reflects only the three valid codes; update any callers that construct/parse members to handle/propagate validation errors (e.g., where stitchRouteGeometry receives members) and run tests to ensure no downstream type mismatches.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/trails/components/AuthGate.tsx`:
- Around line 199-205: The forgot-password success flag forgotSent isn't cleared
when changing tabs, so add logic in the tab-switch handlers inside the AuthGate
component to reset forgotSent to false whenever switching away from or into the
'forgot' tab; specifically update the onClick that calls setTab('forgot') (and
any other tab-switch calls in the same component, e.g., the links around the
224-265 region) to also call setForgotSent(false) (or call a small helper
resetForgotState) so the success panel is cleared and users can resubmit a new
email.
- Around line 76-82: The handleForgot function currently sets
setForgotSent(true) without validating the API response; change the call to
apiClient.auth['forgot-password'].post inside handleForgot to capture the
returned response (e.g., { data, error, status }) and only call
setForgotSent(true) when there is no error and data is present (if error ||
!data then handle it in the catch/else branch and do not set forgotSent). Ensure
setLoading(false) is executed in both success and failure paths and keep using
setForgotEmail/setForgotSent/update UI accordingly.
In `@apps/trails/components/TrailsPage.tsx`:
- Around line 102-110: When clearing an empty search you currently only call
setSearchTrailResults(null) and leave pagination state stale; update
handleSearch so that when query.trim() is empty you also reset pagination and
last-search state by calling setOffset(0), setHasMore(false), and clearing
lastSearchRef (e.g., lastSearchRef.current = null or equivalent) before
returning; apply the same reset logic to the other search-clear path that
mirrors lines 135-145 (the other handler that calls setSearchTrailResults) so
both runSearch triggers and “Load more” visibility are prevented for cleared
searches.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f6c88bd4-474a-4750-8889-d40ef6b41719
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock,!bun.lock
📒 Files selected for processing (8)
CLAUDE.mdapps/trails/components/AuthGate.tsxapps/trails/components/TrailsPage.tsxapps/trails/lib/apiClient.tsapps/trails/lib/auth.tsapps/trails/lib/trailSearch.tsapps/trails/lib/useAuth.tsxapps/trails/package.json
| async function handleForgot(e: React.FormEvent) { | ||
| e.preventDefault(); | ||
| setLoading(true); | ||
| try { | ||
| await apiClient.auth['forgot-password'].post({ email: forgotEmail }); | ||
| setForgotSent(true); | ||
| } catch { |
There was a problem hiding this comment.
Validate forgot-password API result before showing success.
Line 80 currently ignores { error, status }, so a 4xx/5xx API response can still set forgotSent to true on Line 81.
Proposed fix
async function handleForgot(e: React.FormEvent) {
e.preventDefault();
setLoading(true);
try {
- await apiClient.auth['forgot-password'].post({ email: forgotEmail });
+ const { error, status } = await apiClient.auth['forgot-password'].post({
+ email: forgotEmail,
+ });
+ if (error) throw new Error(`Could not send reset email (${status}).`);
setForgotSent(true);
- } catch {
- toast.error('Could not send reset email. Try again.');
+ } catch (err) {
+ toast.error(err instanceof Error ? err.message : 'Could not send reset email. Try again.');
} finally {
setLoading(false);
}
}As per coding guidelines: apps/**/*.{ts,tsx}: Handle API client responses by checking if (error || !data) before using data property.
📝 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.
| async function handleForgot(e: React.FormEvent) { | |
| e.preventDefault(); | |
| setLoading(true); | |
| try { | |
| await apiClient.auth['forgot-password'].post({ email: forgotEmail }); | |
| setForgotSent(true); | |
| } catch { | |
| async function handleForgot(e: React.FormEvent) { | |
| e.preventDefault(); | |
| setLoading(true); | |
| try { | |
| const { error, status } = await apiClient.auth['forgot-password'].post({ | |
| email: forgotEmail, | |
| }); | |
| if (error) throw new Error(`Could not send reset email (${status}).`); | |
| setForgotSent(true); | |
| } catch (err) { | |
| toast.error(err instanceof Error ? err.message : 'Could not send reset email. Try again.'); | |
| } finally { | |
| setLoading(false); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/trails/components/AuthGate.tsx` around lines 76 - 82, The handleForgot
function currently sets setForgotSent(true) without validating the API response;
change the call to apiClient.auth['forgot-password'].post inside handleForgot to
capture the returned response (e.g., { data, error, status }) and only call
setForgotSent(true) when there is no error and data is present (if error ||
!data then handle it in the catch/else branch and do not set forgotSent). Ensure
setLoading(false) is executed in both success and failure paths and keep using
setForgotEmail/setForgotSent/update UI accordingly.
| function handleSearch(query: string) { | ||
| if (!query.trim()) { | ||
| setSearchTrailResults(null); | ||
| return; | ||
| } | ||
| setOffset(0); | ||
| setHasMore(false); | ||
| runSearch({ q: query.trim(), limit: 20, offset: 0 }); | ||
| } |
There was a problem hiding this comment.
Reset pagination cursor when clearing/emptying search.
hasMore, offset, and lastSearchRef are not reset when search is cleared. That can leave “Load more” visible in public mode and trigger stale-query fetches.
Proposed fix
+ const resetSearchState = useCallback(() => {
+ setSearchTrailResults(null);
+ setHasMore(false);
+ setOffset(0);
+ lastSearchRef.current = null;
+ }, []);
+
function handleSearch(query: string) {
if (!query.trim()) {
- setSearchTrailResults(null);
+ resetSearchState();
return;
}
setOffset(0);
setHasMore(false);
runSearch({ q: query.trim(), limit: 20, offset: 0 });
}
...
{isAuthed && isSearchMode && (
<button
type="button"
onClick={() => {
- setSearchTrailResults(null);
+ resetSearchState();
setSearchQuery('');
}}
className="shrink-0 text-xs text-muted-foreground underline-offset-4 hover:underline"
>
Clear
</button>
)}Also applies to: 135-145
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/trails/components/TrailsPage.tsx` around lines 102 - 110, When clearing
an empty search you currently only call setSearchTrailResults(null) and leave
pagination state stale; update handleSearch so that when query.trim() is empty
you also reset pagination and last-search state by calling setOffset(0),
setHasMore(false), and clearing lastSearchRef (e.g., lastSearchRef.current =
null or equivalent) before returning; apply the same reset logic to the other
search-clear path that mirrors lines 135-145 (the other handler that calls
setSearchTrailResults) so both runSearch triggers and “Load more” visibility are
prevented for cleared searches.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/expo/features/catalog/components/ItemReviews.tsx (1)
29-36:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThe
Date | stringparameter is a workaround for a response serialization gap, not a fix—response validation should happen in the route handler.
CatalogItem['reviews'][number]['date']is typed asDate(fromz.date()), but the route handler at line 87 returnsresult.itemswithout validating throughCatalogItemsResponseSchema. When Elysia serializes to JSON,Dateobjects become ISO strings. The client receives strings, notDateobjects, forcing the component to acceptDate | string.The correct fix is to validate the response before returning:
const result = await catalogService.getCatalogItems({...}); const validated = CatalogItemsResponseSchema.parse({ items: result.items, totalCount: result.total, page, limit, totalPages, }); return validated;Using
z.coerce.date()in the schema won't solve this because the schema validation isn't being applied to the response. This is a systemic issue: responses should be validated before serialization to ensure types match reality on the client.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/expo/features/catalog/components/ItemReviews.tsx` around lines 29 - 36, The route handler currently returns result.items (from catalogService.getCatalogItems) without validating with CatalogItemsResponseSchema, so Dates were serialized to strings and forced the client helper formatDate to accept Date | string; fix it by validating the service response before returning: build the response object { items: result.items, totalCount: result.total, page, limit, totalPages } and call CatalogItemsResponseSchema.parse(...) (or safeParse and handle errors) and return the validated value; after this change you can restore formatDate's parameter to expect Date only and rely on schema validation to ensure types match the client.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/expo/features/auth/hooks/useAuthActions.ts`:
- Line 72: Replace raw Zod throws by switching UserSchema.parse(...) to
UserSchema.safeParse(...) at each call-site (the lines using
userStore.set(UserSchema.parse(data.user))) and, if safeParse.success is false,
throw a clear, named error (e.g., new Error('UserSchemaValidationError: failed
to parse authenticated user') or a domain-specific AuthError) with a
human-readable message rather than rethrowing the ZodError; on success call
userStore.set(result.data). Apply the same change to all similar usages in this
file (the other three userStore.set(UserSchema.parse(...)) occurrences).
In `@apps/expo/features/catalog/hooks/useCatalogItemDetails.ts`:
- Around line 8-9: The hook useCatalogItemDetails currently throws only when
error is truthy but then calls CatalogItemSchema.parse(data) without guarding
against a missing data value; update the early-return check in the hook (the
block that currently reads "if (error) ...") to check "if (error || !data)" and
throw a clear Error like "Failed to fetch catalog item" when either condition is
true so that CatalogItemSchema.parse(data) never receives undefined.
In `@apps/expo/features/catalog/hooks/useVectorSearch.ts`:
- Around line 16-17: The code currently only checks `error` before parsing, so
`VectorSearchResponseSchema.parse(data)` will raise a confusing ZodError when
`data` is null/undefined; update the guard to check `if (error || !data)` and
throw a clear API error (including `error?.value` or a descriptive message)
before calling `VectorSearchResponseSchema.parse(data)` in useVectorSearch.ts to
ensure you never parse an empty response.
In `@packages/api/src/schemas/catalog.ts`:
- Around line 97-99: Replace non-coercing z.date() usages with z.coerce.date()
so JSON ISO date strings validate correctly: update the catalog schema fields
(usageCount remains as-is) replacing createdAt: z.date() and updatedAt: z.date()
with createdAt: z.coerce.date() and updatedAt: z.coerce.date(), and change the
reviews sub-schema's date field (reviews[].date) to z.coerce.date() as well;
this ensures the .parse() calls from useCatalogItemDetails.ts and
useVectorSearch.ts accept ISO string dates instead of requiring Date instances.
---
Outside diff comments:
In `@apps/expo/features/catalog/components/ItemReviews.tsx`:
- Around line 29-36: The route handler currently returns result.items (from
catalogService.getCatalogItems) without validating with
CatalogItemsResponseSchema, so Dates were serialized to strings and forced the
client helper formatDate to accept Date | string; fix it by validating the
service response before returning: build the response object { items:
result.items, totalCount: result.total, page, limit, totalPages } and call
CatalogItemsResponseSchema.parse(...) (or safeParse and handle errors) and
return the validated value; after this change you can restore formatDate's
parameter to expect Date only and rely on schema validation to ensure types
match the client.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f44d1ec2-53ac-41ce-acb0-65d41b6ef4d1
📒 Files selected for processing (13)
apps/expo/features/auth/hooks/useAuthActions.tsapps/expo/features/catalog/components/CatalogBrowserModal.tsxapps/expo/features/catalog/components/ItemReviews.tsxapps/expo/features/catalog/hooks/useCatalogItemDetails.tsapps/expo/features/catalog/hooks/useVectorSearch.tsapps/expo/features/catalog/screens/CatalogItemsScreen.tsxapps/expo/features/catalog/types.tsapps/expo/features/pack-templates/components/AddPackTemplateItemActions.tsxapps/expo/features/packs/components/AddPackItemActions.tsxapps/expo/features/packs/hooks/useCreatePackItem.tsapps/expo/features/packs/input.tsapps/expo/features/profile/types.tspackages/api/src/schemas/catalog.ts
| await setRefreshToken(data.refreshToken); | ||
| // safe-cast: Treaty response type differs from local User type; Zod-validated at API boundary | ||
| userStore.set(data.user as unknown as User); | ||
| userStore.set(UserSchema.parse(data.user)); |
There was a problem hiding this comment.
UserSchema.parse() at auth boundaries will lock out users on schema mismatch
UserSchema.parse(data.user) throws a raw ZodError if the server response deviates from the schema — even after the user has successfully authenticated server-side. The thrown error bypasses the human-readable t('auth.failedToSignIn') message and surfaces cryptically to the caller. Worse, any schema/API drift (a field added as required in UserSchema before the server ships it, or a nullable field the server now omits) silently locks the user out of a session they legitimately obtained.
Use safeParse and throw a clear, named error on mismatch:
🛡️ Proposed fix (same pattern applies to all four call-sites)
- userStore.set(UserSchema.parse(data.user));
+ const parsed = UserSchema.safeParse(data.user);
+ if (!parsed.success) {
+ console.error('UserSchema validation failed:', parsed.error);
+ throw new Error(t('auth.failedToSignIn'));
+ }
+ userStore.set(parsed.data);Also applies to: 103-103, 150-150, 257-257
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/expo/features/auth/hooks/useAuthActions.ts` at line 72, Replace raw Zod
throws by switching UserSchema.parse(...) to UserSchema.safeParse(...) at each
call-site (the lines using userStore.set(UserSchema.parse(data.user))) and, if
safeParse.success is false, throw a clear, named error (e.g., new
Error('UserSchemaValidationError: failed to parse authenticated user') or a
domain-specific AuthError) with a human-readable message rather than rethrowing
the ZodError; on success call userStore.set(result.data). Apply the same change
to all similar usages in this file (the other three
userStore.set(UserSchema.parse(...)) occurrences).
| if (error) throw new Error(`Failed to fetch catalog item: ${error.value}`); | ||
| return data; | ||
| return CatalogItemSchema.parse(data); |
There was a problem hiding this comment.
Same missing !data guard as in useVectorSearch.ts.
🐛 Proposed fix
- if (error) throw new Error(`Failed to fetch catalog item: ${error.value}`);
- return CatalogItemSchema.parse(data);
+ if (error || !data) throw new Error(`Failed to fetch catalog item: ${error?.value ?? 'empty response'}`);
+ return CatalogItemSchema.parse(data);As per coding guidelines: "Check if (error || !data) before using data from API client responses which return { data, error, status }."
📝 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 (error) throw new Error(`Failed to fetch catalog item: ${error.value}`); | |
| return data; | |
| return CatalogItemSchema.parse(data); | |
| if (error || !data) throw new Error(`Failed to fetch catalog item: ${error?.value ?? 'empty response'}`); | |
| return CatalogItemSchema.parse(data); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/expo/features/catalog/hooks/useCatalogItemDetails.ts` around lines 8 -
9, The hook useCatalogItemDetails currently throws only when error is truthy but
then calls CatalogItemSchema.parse(data) without guarding against a missing data
value; update the early-return check in the hook (the block that currently reads
"if (error) ...") to check "if (error || !data)" and throw a clear Error like
"Failed to fetch catalog item" when either condition is true so that
CatalogItemSchema.parse(data) never receives undefined.
…files Replace end-of-stream single flush with per-BATCH_SIZE flushes during the CSV parse loop. Valid and invalid item batches are written to DB every 100 rows, arrays are cleared, and totalProcessed is updated incrementally so progress is visible on long-running jobs. Remaining rows are flushed after the loop as before. Fixes evo (174K rows) and gearx (166K rows) jobs stuck in 'running' forever.
Clothing and footwear brands (kuhl, obozfootwear, prana) routinely omit weight data, causing 0% ingest rates. Weight is important for comparisons but it is better to ingest items without it than to reject them entirely. Items missing weight are stored as-is and simply excluded from weight views. Weight and weightUnit remain nullable at the DB layer (no schema change).
The ON CONFLICT DO UPDATE SET clause was using COALESCE(table.field, excluded.field) which means existing non-null values could never be corrected by a fresh scrape. Changed to excluded.field directly so re-scraping always wins, allowing price, weight, availability, and other fields to be corrected when the source data changes. Exception: created_at retains COALESCE so the original creation timestamp is preserved across upserts.
…state Manually runnable script that fails any ETL job that has been in 'running' status for more than 30 minutes. Addresses jobs left stuck when the Worker crashes mid-stream (e.g. OOM) without reaching the error handler.
Protect against scrapers returning weight=0 or weight<0 overwriting valid existing weight data. Weight and weight_unit now only update when the incoming value is a positive number; otherwise the stored value is preserved. All other fields continue to accept the fresh scraped value so stale data can still be corrected by re-scraping.
- Create packages/units with NIST-exact avoirdupois constants (1 oz = 28.349523125 g, 1 lb = 453.59237 g — single source of truth) - Expose normalize, fromGrams, convert, displayWeight, isWeightUnit, parseWeightUnit; 174 tests including cross-validation vs convert-units v3 - Migrate all call sites: apps/expo weight utils, computePackWeights, computePackTemplateWeights, computeCategories, lib/utils/compute-pack, packages/api weight utils and compute-pack - Fix convertToGrams/convertFromGrams which were NOT inverses of each other (used 28.3495 vs 28.35 — silent drift of ~0.016 oz per 100 oz) - Fix computeCategories percentage bug: was mixing pack.totalWeight (in preferred unit) with convertToGrams, giving wrong % for non-gram users - Update all test suites to NIST values; 730 tests passing (0 failures) - Add convert-units 3.0.0-beta.8 as devDependency for cross-validation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- ErrorBoundary: add resetKeys={[pathname]} so boundary auto-resets
on route changes instead of keeping fallback across navigation
- error-fallback.tsx + global-error.tsx: stop surfacing raw error.message
to users; log to console instead
- RawObjectDialog: add visually-hidden DialogDescription to silence
Radix UI aria-describedby warning
- Note: .returning({ id }) on reset-stuck blocked by Drizzle TS types
in this version; full .returning() retained
feat(units): weight math audit — new @packrat/units package with NIST constants and 174 tests
…lidation-v2 feat(admin): pagination, search debounce, reset-stuck UI, error boundaries
Introduces apps/trails — a standalone Next.js + Cloudflare Worker app deployed to trails.packratai.com. Visitors can explore nearby trails on a public Leaflet/Overpass map without auth. Search is gated behind account creation, converting anonymous visitors into PackRat users before nudging them toward the native app. Key pieces: - CF Worker hybrid: serves Next.js static export + proxies /api/* to PackRat API with Bearer token pass-through, CORS, rate limiting, and edge caching on trail detail responses - Public map: Overpass API queries via @packrat/overpass, no auth required, geolocation with US center fallback - Auth gate: tabbed register/login/forgot-password dialog; full email OTP verification flow using existing /api/auth/* endpoints - Web token storage: localStorage with resilientTokenStorage pattern (JSON-parse guard for atomWithStorage compat) - Authenticated search: authedFetch with automatic 401 refresh + retry - Download CTA: dismissable sticky banner with mobile UA detection for App Store / Google Play deep links 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
workers-types ^4.20250620.0→^4.20250405.0, @types/leaflet ^1.9.17→^1.9.21, wrangler ^4.21.1→^4.21.2
UserInfo/AuthResponse now use zod schemas; Tab narrowing uses makeEnumGuard.
- Add lib/apiClient.ts: Treaty client wired to same-origin proxy so CF Worker rate limiting applies - Strip lib/auth.ts to storage helpers only (UserInfo now matches API shape: id:number, firstName/lastName) - Delete lib/apiFetch.ts (AuthExpiredError moved to apiClient.ts) - Rewrite lib/trailSearch.ts and lib/useAuth.tsx using typed Treaty endpoints - Update AuthGate.tsx: username→firstName field, forgot-password via apiClient - Update CLAUDE.md with @packrat/api-client usage pattern for all web apps
Direct `as CatalogItem[]` rejected because Treaty's inferred type doesn't sufficiently overlap the hand-written interface; `as unknown as` is the standard escape hatch when shapes are known to match at runtime.
Replace the hand-written CatalogItem interface with z.infer<typeof CatalogItemSchema> so the type flows directly from the API schema — no more divergence, no more casts. - Add usageCount to CatalogItemSchema (computed from packItems in detail route) - Parse useVectorSearch and useCatalogItemDetails responses through their schemas - Remove all as CatalogItem[] casts from CatalogBrowserModal and CatalogItemsScreen - Widen PackItemInput.description to string | null to satisfy intersection type - Convert null→undefined at the assignment boundary in useCreatePackItem
…undaries
Replace hand-written User interface (which diverged from the API schema on
firstName/lastName/role nullability and had a preferredWeightUnit field never
returned by any endpoint) with z.infer<typeof UserSchema> & { preferredWeightUnit? }.
Replace the four `data.user as unknown as User` casts in useAuthActions with
UserSchema.parse(), giving runtime validation of the API response shape at
every sign-in and email-verify path.
- Fix API response shape: search returns plain array, not {trails,hasMore}
- Fix bbox destructuring order: [minlon,minlat,maxlon,maxlat] = west/south/east/north
- Fix bboxCenter: accept bbox tuple instead of GeoJSON object
- Fix apiError: read 'error' field before 'message' to match PackRat API shape
- Restrict CORS to allowed origins; exempt OPTIONS from rate limiting
- Tighten TRAIL_DETAIL_RE to numeric OSM IDs only (/api/trails/\d+)
- Add NEXT_PUBLIC_PACKRAT_API_ORIGIN dev override for apiClient
- Add usageCount .min(0) constraint to CatalogItemSchema
- Keep import type for CatalogItemSchema/UserSchema (z.infer<typeof> is type-level)
Adds apps/trails/lib/env.ts following the same pattern as apps/admin/lib/env.ts so NEXT_PUBLIC_PACKRAT_API_ORIGIN is parsed through Zod once at module load. Unblocks no-raw-process-env pre-push check.
Defines ApiTrailSchema using Zod and parses the raw API response through fromZod() instead of a double as-cast, satisfying check:casts:strict.
- sport: string → OsmSport in TrailSearchParams (matches API enum) - offset: pass ?? 0 fallback so undefined doesn't fail required number type - Pin @packrat-ai/nativewindui to 2.0.3 via root overrides; 2.0.6 has a type regression against react-native 0.81 autocapitalize types
andrew-bierman
force-pushed
the
feat/trail-search-micro-frontend
branch
from
79ee919 to
17ad75c
Compare
…tsconfig - worker: drop X-Forwarded-For fallback (client-spoofable); use CF-Connecting-IP only - worker: include CORS headers on 429 rate-limit response so browsers see the actual error - auth: add typeof-window guards to setTokens/clearTokens/setUser/clearUser mutators - apiClient: clear session when counterpart token is absent on refresh callbacks - TrailMap: add cancellation flag to async marker-update effect to prevent stale layers - TrailsPage: remove redundant setMapState inside if(!coords) block (dead code) - tsconfig: add noUncheckedIndexedAccess + ESNext target + @packrat/api bare alias - next.config: remove ignoreBuildErrors/ignoreDuringBuilds; rely on bun check-types gate 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Creates packages/app with safeLocalStorage and safeSessionStorage helpers that include SSR guards, eliminating the repeated typeof-window checks scattered across apps. Removes 9 guard duplicates from trails auth.ts and 2 from admin auth.ts. apps/trails/lib/auth.ts and apps/admin/lib/auth.ts now delegate all storage access through safeLocalStorage/safeSessionStorage. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
packrat-admin | b0d4b22 | Commit Preview URL Branch Preview URL |
May 08 2026, 03:13 AM |
…PackRat API The worker was a same-origin proxy adding rate limiting and CORS, but the PackRat API already handles both. Pointing @packrat/api-client straight at the API via NEXT_PUBLIC_PACKRAT_API_ORIGIN (defaults to api.packratai.com) eliminates the extra deployment layer. Removes: worker/index.ts, wrangler.jsonc, @cloudflare/workers-types, wrangler dep 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Summary
apps/trails— a standalone Next.js + Cloudflare Worker app fortrails.packratai.comWhy: Tiny, hyper-focused web experiences give people a reason to create a PackRat account before they ever download the app.
Architecture
worker/index.ts— CF Worker: rate limiting → CORS → Bearer pass-through proxy, asset servinglib/auth.ts— resilientTokenStorage + Zod-validated UserInfo/AuthResponselib/useAuth.tsx— React context with register/OTP/login/logout + authGateOpen gatecomponents/TrailMap.tsx— Leaflet with next/dynamic ssr:falsecomponents/AuthGate.tsx— Radix Dialog with register/login/forgot tabs + makeEnumGuardlib/trailSearch.ts— authenticated search with 401 refresh-retry + bbox centroid extractionTesting
All pre-push hooks pass: typeof check, raw regex check, check:casts:strict (no unsafe as-casts), circular deps, package ordering, version catalog. Root
bun check-typespasses.Post-Deploy Monitoring & Validation
Setup before first deploy:
wrangler rate-limit create --simple --limit 60 --period 60→ replace__REPLACE_WITH_NAMESPACE_ID__inapps/trails/wrangler.jsoncPACKRAT_API_BASE_URL=https://api.packratai.comin CF dashboardtrails.packratai.comCNAME → CF Workers routeLogs:
wrangler tail packrat-trails— watch for 429s, 5xx proxy errors, 401 auth failuresMetrics: CF dashboard → Workers → packrat-trails → Requests/Errors/CPU
Healthy: <1% error rate; map loads <3s; search returns <2s
Failure trigger: >5% errors or sustained 5xx → check API_BASE_URL var and API health
Validation window: First 24h; owner: @andrew-bierman
Summary by CodeRabbit
New Features
Infrastructure / Chores
Documentation