feat(trails): trail search micro frontend acquisition surface#2389
Merged
Conversation
Contributor
|
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: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
Warning
|
| Layer / File(s) | Summary |
|---|---|
Type Definitions & Schema Imports apps/expo/features/catalog/types.ts, apps/expo/features/profile/types.ts |
Catalog and user types are derived from Zod schemas using z.infer<typeof Schema>; manual interface definitions removed. |
Catalog Type Restructuring apps/expo/features/catalog/types.ts |
CatalogItem, CatalogItemWithQuantity, and CatalogItemWithPackItemFields now use schema-inferred types; CatalogItemLink and CatalogItemReview interfaces removed and inlined. |
Schema Extensions packages/api/src/schemas/catalog.ts |
CatalogItemSchema extended with optional usageCount field for tracking item popularity. |
Pack Item Input & Description Handling apps/expo/features/packs/input.ts, apps/expo/features/packs/hooks/useCreatePackItem.ts |
PackItemInput description field widened to accept null; creation logic normalizes null to undefined. |
Runtime Validation in Auth Actions apps/expo/features/auth/hooks/useAuthActions.ts |
User data validated using UserSchema.parse() in signIn, signInWithGoogle, signInWithApple, and verifyEmail; type imports removed. |
Catalog Hook Schema Validation apps/expo/features/catalog/hooks/useCatalogItemDetails.ts, apps/expo/features/catalog/hooks/useVectorSearch.ts |
API responses parsed through schema validation before returning to React Query. |
Component Cast Removal apps/expo/features/catalog/components/CatalogBrowserModal.tsx, apps/expo/features/catalog/screens/CatalogItemsScreen.tsx, apps/expo/features/catalog/components/ItemReviews.tsx |
Unsafe type assertions removed; data flows naturally from validated sources; formatDate helper broadened to accept Date | string. |
Pack Integration Refactoring apps/expo/features/pack-templates/components/AddPackTemplateItemActions.tsx, apps/expo/features/packs/components/AddPackItemActions.tsx |
nullToUndefined coercion eliminated; catalog items passed directly to API without per-item description normalization. |
Trails Web Application
| Layer / File(s) | Summary |
|---|---|
Project Configuration & Build Setup apps/trails/tsconfig.json, apps/trails/package.json, apps/trails/next.config.mjs, apps/trails/postcss.config.mjs, apps/trails/tailwind.config.ts |
Next.js app configuration with strict TypeScript, static export, Tailwind UI preset, and path aliases for @packrat packages. |
Environment & Browser Storage Utilities apps/trails/lib/env.ts, packages/app/package.json, packages/app/src/browser.ts, packages/app/src/index.ts |
New @packrat/app package provides browser-safe localStorage/sessionStorage wrappers and environment variable validation with Zod. |
Auth Token & User Persistence apps/trails/lib/auth.ts |
Token getter/setter and user info schema-based persistence using safe storage; parseToken handles both JSON and raw JWT formats. |
API Client Configuration apps/trails/lib/apiClient.ts |
Configured API client with token refresh lifecycle, onNeedsReauth callback, and AuthExpiredError class for session timeouts. |
Geolocation & Trail Discovery apps/trails/lib/geolocation.ts, apps/trails/lib/overpass.ts, apps/trails/lib/trailSearch.ts |
Geolocation utility, Overpass trail loader with coordinate enrichment, and paginated trail search with Zod schema validation. |
Authentication Provider & Context apps/trails/lib/useAuth.tsx |
React context providing auth state and async actions (register, login, verify, resend, logout) with token persistence. |
Auth UI Components apps/trails/components/AuthGate.tsx, apps/trails/components/VerifyEmail.tsx, apps/trails/components/SearchBar.tsx, apps/trails/components/DownloadCTA.tsx, apps/trails/components/ui/sonner.tsx |
AuthGate dialog with tabs, OTP verification, auth-gated search bar, platform-aware download banner, and sonner Toaster wrapper. |
Trail Display Components apps/trails/components/TrailCard.tsx, apps/trails/components/TrailMap.tsx |
TrailCard list items with sport/difficulty/distance rendering; TrailMap Leaflet component with dynamic marker creation and selection. |
Page Layout & Main Content apps/trails/app/layout.tsx, apps/trails/app/page.tsx, apps/trails/components/TrailsPage.tsx |
Root layout wrapping app with AuthProvider and Toaster; TrailsPage orchestrating geolocation, trail loading, search pagination, and map/list UI. |
Global Styling & UI Components apps/trails/app/globals.css |
Global CSS with Leaflet integration and Tailwind base layer styles for smooth scrolling and theme colors. |
Root Configuration & Supporting Changes
| Layer / File(s) | Summary |
|---|---|
New @packrat/app Package packages/app/package.json, packages/app/src/browser.ts, packages/app/src/index.ts |
Shared package providing isBrowser helper and safeLocalStorage/safeSessionStorage wrappers for server-safe browser APIs. |
Admin App Safe Storage Migration apps/admin/lib/auth.ts, apps/admin/package.json, apps/admin/tsconfig.json |
Admin app refactored to use safeSessionStorage from @packrat/app instead of direct sessionStorage; TypeScript paths added. |
Monorepo Configuration & Aliases package.json, tsconfig.json |
Root scripts added for trails app; TypeScript path aliases for trails-app and @packrat/overpass packages. |
Documentation & Linting Configuration CLAUDE.md, packages/env/scripts/no-raw-process-env.ts |
CLAUDE.md updated with API client usage conventions; environment variable linting exemption added for trails env shim. |
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
- PackRat-AI/PackRat#2306: Adds backend trails API routes, DB migrations, and schema that this PR's client-side trails app consumes.
- PackRat-AI/PackRat#2268: Introduces createApiClient factory and Treaty RPC client that this PR uses for trails search and auth endpoints.
- PackRat-AI/PackRat#2274: Modifies the same useAuthActions hook with schema validation and user type changes.
Suggested labels
web, authentication, trails, schema-validation, new-feature
Suggested reviewers
- Isthisanmol
- mikib0
Poem
🐰 A trails app hops into view,
Maps and markers, shiny and new,
Schema validation makes types tight,
Auth flows dance in the browser's light,
Leaflet leaves mark mountains to climb!
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 8.89% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title 'feat(trails): trail search micro frontend acquisition surface' accurately describes the main change—adding a new Trails micro-frontend app with trail search functionality and a public map for user acquisition. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
feat/trail-search-micro-frontend
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 @coderabbitai help to get the list of available commands and usage tips.
github-actions
Bot
added
documentation
Contributor
Coverage Report for Expo Unit Tests Coverage (./apps/expo)
File CoverageNo changed files found. |
Contributor
Coverage Report for API Unit Tests Coverage (./packages/api)
File CoverageNo changed files found. |
andrew-bierman
force-pushed
the
feat/trail-search-micro-frontend
branch
from
b1316a7 to
9d3cca9
Compare
andrew-bierman
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
Introduces a new apps/trails Next.js (static export) + Cloudflare Worker acquisition surface for trail discovery/search, and performs schema-first type cleanups in the Expo app by deriving types from API Zod schemas.
Changes:
- Add
apps/trailswith Leaflet map, auth-gated trail search, and a Cloudflare Worker/api/*proxy (CORS + rate limiting). - Extend API
CatalogItemSchemawith optionalusageCount. - Replace hand-written Expo
User/CatalogIteminterfaces withz.infer<...>-derived types and add Zod parsing at key API boundaries.
Reviewed changes
Copilot reviewed 39 out of 41 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Adds path aliases for the new trails app and @packrat/overpass. |
| packages/api/src/schemas/catalog.ts | Adds usageCount to CatalogItemSchema. |
| package.json | Adds a root trails dev script. |
| CLAUDE.md | Documents a standard pattern for using @packrat/api-client in web apps. |
| bun.lock | Adds apps/trails workspace entry and dependency lock info. |
| apps/trails/wrangler.jsonc | Cloudflare Worker/Assets config for trails deployment, with rate limiting binding. |
| apps/trails/worker/index.ts | Worker proxy implementation for /api/* and static asset serving. |
| apps/trails/tsconfig.json | Trails app TS config + path aliases. |
| apps/trails/tailwind.config.ts | Tailwind preset + content globs for trails app. |
| apps/trails/postcss.config.mjs | PostCSS config (import + tailwind). |
| apps/trails/package.json | Trails app dependencies and scripts. |
| apps/trails/next.config.mjs | Static export config + package transpilation list. |
| apps/trails/lib/useAuth.tsx | Auth provider/context with register/login/verify/resend flows. |
| apps/trails/lib/trailSearch.ts | Authenticated trail search client helper. |
| apps/trails/lib/overpass.ts | Public (unauth) nearby trail loading via Overpass. |
| apps/trails/lib/geolocation.ts | Browser geolocation helper + default map center/zoom. |
| apps/trails/lib/auth.ts | localStorage token/user helpers. |
| apps/trails/lib/apiClient.ts | @packrat/api-client setup for same-origin /api proxy. |
| apps/trails/components/VerifyEmail.tsx | OTP verification UI + resend cooldown. |
| apps/trails/components/ui/sonner.tsx | Toaster wrapper for notifications. |
| apps/trails/components/TrailsPage.tsx | Main page orchestration (map + sidebar + search gating). |
| apps/trails/components/TrailMap.tsx | Leaflet map rendering + marker updates. |
| apps/trails/components/TrailCard.tsx | Sidebar trail list item UI. |
| apps/trails/components/SearchBar.tsx | Auth-gated search input. |
| apps/trails/components/DownloadCTA.tsx | App download CTA banner with dismissal. |
| apps/trails/components/AuthGate.tsx | Register/login/forgot-password + verify-email dialog. |
| apps/trails/app/page.tsx | Routes to the trails page component. |
| apps/trails/app/layout.tsx | App shell: metadata, auth provider, toaster, global styles. |
| apps/trails/app/globals.css | Imports shared web-ui globals + leaflet styles and z-index tweaks. |
| apps/expo/features/profile/types.ts | Switches User type to be derived from UserSchema. |
| apps/expo/features/packs/input.ts | Allows pack item description to be null. |
| apps/expo/features/packs/hooks/useCreatePackItem.ts | Normalizes description nullish handling on create. |
| apps/expo/features/packs/components/AddPackItemActions.tsx | Removes nullToUndefined mapping when bulk adding catalog items. |
| apps/expo/features/pack-templates/components/AddPackTemplateItemActions.tsx | Removes nullToUndefined mapping when adding template items. |
| apps/expo/features/catalog/types.ts | Switches CatalogItem type to be derived from CatalogItemSchema. |
| apps/expo/features/catalog/screens/CatalogItemsScreen.tsx | Removes unsafe casts now that results are schema-parsed. |
| apps/expo/features/catalog/hooks/useVectorSearch.ts | Parses vector search response with VectorSearchResponseSchema. |
| apps/expo/features/catalog/hooks/useCatalogItemDetails.ts | Parses catalog item details with CatalogItemSchema. |
| apps/expo/features/catalog/components/ItemReviews.tsx | Broadens date input type for formatting. |
| apps/expo/features/catalog/components/CatalogBrowserModal.tsx | Removes unsafe casts for popular/search items. |
| apps/expo/features/auth/hooks/useAuthActions.ts | Parses data.user with UserSchema before storing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+7
to
+8
| const TRAIL_DETAIL_RE = /^\/api\/trails\/[^/]+$/; | ||
|
|
Comment on lines
+25
to
+43
| // Rate limit by IP | ||
| if (env.RATE_LIMITER) { | ||
| const ip = | ||
| request.headers.get('CF-Connecting-IP') ?? | ||
| request.headers.get('X-Forwarded-For') ?? | ||
| 'unknown'; | ||
| const { success } = await env.RATE_LIMITER.limit({ key: ip }); | ||
| if (!success) { | ||
| return corsResponse( | ||
| 429, | ||
| JSON.stringify({ error: 'Too many requests. Please try again in a moment.' }), | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| // Handle CORS preflight | ||
| if (request.method === 'OPTIONS') { | ||
| return new Response(null, { status: 204, headers: CORS_HEADERS }); | ||
| } |
Comment on lines
+39
to
+40
| const msg = asStringRecord(error).message; | ||
| return new Error(msg ?? fallback); |
Comment on lines
+51
to
+60
| export async function searchTrails(params: TrailSearchParams): Promise<TrailSearchResult> { | ||
| const { data, error, status } = await apiClient.trails.search.get({ | ||
| query: { | ||
| q: params.q, | ||
| lat: params.lat, | ||
| lon: params.lon, | ||
| radius: params.radius, | ||
| sport: params.sport, | ||
| limit: params.limit ?? 20, | ||
| offset: params.offset, |
| const summary = toTrailSummary(el); | ||
| let center: [number, number] | null = null; | ||
| if (summary.bbox) { | ||
| const [south, west, north, east] = summary.bbox; |
Comment on lines
94
to
98
| ) | ||
| .nullable() | ||
| .optional(), | ||
| usageCount: z.number().int().optional(), | ||
| createdAt: z.string().datetime(), |
Comment on lines
+1
to
+7
| import type { UserSchema } from '@packrat/api/schemas/users'; | ||
| import type { z } from 'zod'; | ||
| import type { WeightUnit } from '../packs/types'; | ||
|
|
||
| export interface User { | ||
| id: number; | ||
| email: string; | ||
| firstName: string; | ||
| lastName: string; | ||
| avatarUrl?: string | null; | ||
| role: 'USER' | 'ADMIN'; | ||
| preferredWeightUnit: WeightUnit; | ||
| } | ||
| export type User = z.infer<typeof UserSchema> & { | ||
| preferredWeightUnit?: WeightUnit; | ||
| }; |
Comment on lines
+1
to
6
| import type { CatalogItemSchema } from '@packrat/api/schemas/catalog'; | ||
| import type { z } from 'zod'; | ||
| import type { PackItemInput } from '../packs/input'; | ||
|
|
||
| export interface CatalogItemLink { | ||
| id: string; | ||
| title: string; | ||
| url: string; | ||
| type: 'official' | 'review' | 'guide' | 'purchase' | 'other'; | ||
| } | ||
|
|
||
| export interface CatalogItemReview { | ||
| id: string; | ||
| userId: string; | ||
| userName: string; | ||
| userAvatar: string; | ||
| rating: number; | ||
| text: string; | ||
| date: string; | ||
| helpful: number; | ||
| verified: boolean; | ||
| } | ||
|
|
||
| export interface CatalogItem { | ||
| id: number; | ||
| name: string; | ||
| productUrl: string; | ||
| sku: string; | ||
| weight: number; | ||
| weightUnit: WeightUnit; | ||
| description?: string | null; | ||
| categories?: string[] | null; | ||
| images?: string[] | null; | ||
| brand?: string | null; | ||
| model?: string | null; | ||
| ratingValue?: number | null; | ||
| color?: string | null; | ||
| size?: string | null; | ||
| price?: number | null; | ||
| availability?: 'in_stock' | 'out_of_stock' | 'preorder' | null; | ||
| seller?: string | null; | ||
| productSku?: string | null; | ||
| material?: string | null; | ||
| currency?: string | null; | ||
| condition?: string | null; | ||
| reviewCount?: number | null; | ||
| usageCount?: number | null; | ||
|
|
||
| variants?: Array<{ | ||
| attribute: string; | ||
| values: string[]; | ||
| }> | null; | ||
|
|
||
| techs?: Record<string, string> | null; | ||
|
|
||
| links?: Array<{ | ||
| title: string; | ||
| url: string; | ||
| }> | null; | ||
|
|
||
| reviews?: Array<{ | ||
| user_name: string; | ||
| user_avatar?: string | null; | ||
| context?: Record<string, string> | null; | ||
| recommends?: boolean | null; | ||
| rating: number; | ||
| title: string; | ||
| text: string; | ||
| date: string; | ||
| images?: string[] | null; | ||
| upvotes?: number | null; | ||
| downvotes?: number | null; | ||
| verified?: boolean | null; | ||
| }> | null; | ||
|
|
||
| qas?: Array<{ | ||
| question: string; | ||
| user?: string | null; | ||
| date: string; | ||
| answers: Array<{ | ||
| a: string; | ||
| date: string; | ||
| user?: string | null; | ||
| upvotes?: number | null; | ||
| }>; | ||
| }> | null; | ||
|
|
||
| faqs?: Array<{ | ||
| question: string; | ||
| answer: string; | ||
| }> | null; | ||
|
|
||
| embedding?: number[] | null; // vector(1536) | ||
|
|
||
| createdAt: string; | ||
| updatedAt: string; | ||
| } | ||
| export type CatalogItem = z.infer<typeof CatalogItemSchema>; | ||
|
|
Comment on lines
+12
to
+14
| // Routes through the same-origin CF Worker proxy (/api/*) so rate limiting applies. | ||
| export const apiClient = createApiClient({ | ||
| baseUrl: typeof window !== 'undefined' ? window.location.origin : '', |
Comment on lines
+11
to
+22
| // Rate limiting: 60 requests per IP per 60 seconds | ||
| // Create namespace: wrangler rate-limit create --simple --limit 60 --period 60 | ||
| // Then replace namespace_id below with the returned ID | ||
| "rate_limiting": [ | ||
| { | ||
| "binding": "RATE_LIMITER", | ||
| "namespace_id": "__REPLACE_WITH_NAMESPACE_ID__", | ||
| "simple": { | ||
| "limit": 60, | ||
| "period": 60 | ||
| } | ||
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
package.json (1)
51-56:⚠️ Potential issue | 🟠 Major | ⚡ Quick winVersion conflict between
overridesandpatchedDependenciesfor@packrat-ai/nativewindui.
overridesforces@packrat-ai/nativewinduito2.0.3, butpatchedDependenciesstill references@packrat-ai/nativewindui@1.1.0. Bun applies patches after resolution; since the override resolves2.0.3, the1.1.0patch entry won't match and the patch will silently be skipped (or error), leaving2.0.3unpatched.Either remove the
patchedDependenciesentry if the patch is no longer needed at 2.0.3, or update it to@packrat-ai/nativewindui@2.0.3with an appropriate patch for that version.Also applies to: 121-123
🤖 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 `@package.json` around lines 51 - 56, The package.json has a version mismatch: overrides pins "@packrat-ai/nativewindui" to "2.0.3" while patchedDependencies references "@packrat-ai/nativewindui@1.1.0", causing Bun to skip/apply the wrong patch; update package.json so both refer to the same version (either remove the patchedDependencies entry if the patch is no longer needed for 2.0.3, or change the patchedDependencies key to "@packrat-ai/nativewindui@2.0.3" and supply a compatible patch), ensuring the entries that reference "@packrat-ai/nativewindui" are consistent across overrides and patchedDependencies.CLAUDE.md (1)
182-190:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
trails-app/*path alias is missing from the Path Aliases section.All
apps/trailssource files usetrails-app/*imports (e.g.,trails-app/lib/useAuth,trails-app/lib/auth), but this alias is absent from the documented list and — critically — likely from the roottsconfig.json. Without it,tsc --noEmitand IDE go-to-definition will break for anyone who clones the repo.📝 Proposed addition to the Path Aliases section
- `nativewindui/*` → `apps/expo/components/ui/*` +- `trails-app/*` → `apps/trails/*`Also verify the alias is present in the root
tsconfig.jsonpathsblock.🤖 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 `@CLAUDE.md` around lines 182 - 190, The Path Aliases section and the root tsconfig.json are missing the trails-app/* alias used by apps/trails; add an entry mapping "trails-app/*" to "apps/trails/*" in the documented list and ensure the root tsconfig.json "compilerOptions.paths" includes "trails-app/*": ["apps/trails/*"] so imports like trails-app/lib/useAuth and trails-app/lib/auth resolve for tsc and IDEs.
🧹 Nitpick comments (8)
apps/expo/features/catalog/components/CatalogBrowserModal.tsx (1)
190-192: 💤 Low valueMinor: prefer
??over||for array fallbacks.
searchResult?.itemsandpaginatedData?.pages.flatMap(...)are already typed as arrays;||happens to work but??better expresses "fall back only when nullish" and is consistent with line 167 (popularData?.items ?? []).♻️ Suggested tweak
- const items = isSearching - ? searchResult?.items || [] - : paginatedData?.pages.flatMap((page) => page.items) || []; + const items = isSearching + ? (searchResult?.items ?? []) + : (paginatedData?.pages.flatMap((page) => page.items) ?? []);🤖 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/CatalogBrowserModal.tsx` around lines 190 - 192, The fallback for computing items should use the nullish coalescing operator instead of logical OR: when building items (the const items using isSearching ? searchResult?.items || [] : paginatedData?.pages.flatMap((page) => page.items) || []), replace the two uses of || with ?? so you only fall back to [] for null/undefined values from searchResult?.items and paginatedData?.pages.flatMap(...); keep the existing isSearching, searchResult, paginatedData and pages references and preserve the flatMap call.apps/expo/features/catalog/hooks/useCatalogItemDetails.ts (1)
7-9: ⚡ Quick winSame
error || !dataguard recommendation asuseVectorSearch.
getCatalogItemonly narrows onerrorbefore parsing. Adding the!datacheck keeps these helpers consistent and yields a clearer message than a Zod parse failure onundefined.♻️ Proposed guard
- if (error) throw new Error(`Failed to fetch catalog item: ${error.value}`); + if (error || !data) { + throw new Error(`Failed to fetch catalog item: ${error?.value ?? 'no data returned'}`); + } return CatalogItemSchema.parse(data);As per coding guidelines: "API client responses follow
{ data, error, status }format — always checkif (error || !data)before usingdata".🤖 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 7 - 9, The current fetch in useCatalogItemDetails only checks for error before parsing, so add the same guard used in useVectorSearch: after calling apiClient.catalog({ id }).get() check if (error || !data) and throw a descriptive Error (e.g., `Failed to fetch catalog item: ${error?.value ?? 'no data'}`) before calling CatalogItemSchema.parse(data) to avoid Zod parsing undefined and keep API response handling consistent.apps/expo/features/catalog/hooks/useVectorSearch.ts (1)
16-17: ⚡ Quick winGuard against
databeing absent before parsing.Per the project convention, API client responses should be checked with
if (error || !data)before consumption. Today we only short-circuit onerror; if Treaty ever returns{ data: null, error: null }(e.g., 204 or unexpected upstream behavior),VectorSearchResponseSchema.parse(undefined)produces a less helpful error than an explicit guard.♻️ Proposed guard
- if (error) throw new Error(`Vector search API error: ${error.value}`); + if (error || !data) { + throw new Error(`Vector search API error: ${error?.value ?? 'no data returned'}`); + } return VectorSearchResponseSchema.parse(data);As per coding guidelines: "API client responses follow
{ data, error, status }format — always checkif (error || !data)before usingdata".🤖 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/useVectorSearch.ts` around lines 16 - 17, The code currently only checks `error` before parsing the response, so if `data` is null/undefined `VectorSearchResponseSchema.parse(data)` can throw an unclear error; update the guard to check `if (error || !data)` and throw a descriptive Error (e.g., "Vector search API returned no data" plus `error?.value` if present) before calling `VectorSearchResponseSchema.parse(data)`, modifying the block around the existing variables `error`, `data`, and the call to `VectorSearchResponseSchema.parse` in `useVectorSearch`.apps/trails/lib/useAuth.tsx (1)
53-58: ⚡ Quick winHydration sets
isAuthed: truewithout checking token expiry
getAccessToken()only verifies that a token exists in storage, not whether it's still valid. A user with an expired JWT will be hydrated as authenticated, briefly showing gated UI before the first 401 triggersAuthExpiredErrorand forces re-login.Adding a lightweight JWT
expclaim check avoids the transient incorrect state:♻️ Lightweight expiry check on hydration
useEffect(() => { const token = getAccessToken(); const user = getUser(); - if (token && user) { + if (token && user && !isTokenExpired(token)) { setState({ isAuthed: true, user, pendingEmail: null }); } }, []);Add to
auth.ts:export function isTokenExpired(token: string): boolean { try { const payload = JSON.parse(atob(token.split('.')[1])); return typeof payload.exp === 'number' && payload.exp * 1000 < Date.now(); } catch { return true; // treat malformed token as expired } }🤖 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/useAuth.tsx` around lines 53 - 58, Hydration currently marks the user authed if a token exists, but getAccessToken() doesn't check expiry; update the useEffect in useAuth (the block that calls getAccessToken() and getUser()) to call a new isTokenExpired(token) and only setState({ isAuthed: true, user, pendingEmail: null }) when token exists and isTokenExpired(token) returns false; add the helper isTokenExpired in auth.ts (parse JWT payload, treat malformed tokens as expired) and use it here to prevent briefly showing gated UI for expired/malformed tokens.apps/trails/lib/trailSearch.ts (2)
68-68: ⚡ Quick winSilent empty-result fallback masks API schema regressions
parseApiTrails(data) ?? []returns an empty array whendatadoesn't matchz.array(ApiTrailSchema). A schema mismatch (e.g., the API renames a field) is completely invisible to the caller — it looks like a search with zero results, not a contract error.♻️ Proposed fix — throw on parse failure
- const rawTrails = parseApiTrails(data) ?? []; + const rawTrails = parseApiTrails(data); + if (!rawTrails) { + throw new Error('Search failed: unexpected response shape from API'); + }🤖 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` at line 68, The call currently masks schema failures by assigning rawTrails = parseApiTrails(data) ?? []; change this to surface parsing errors: call parseApiTrails(data) and if it returns null/undefined (or a failed parse), throw a descriptive error (including the raw data or parse error details) instead of falling back to an empty array; update the code paths using rawTrails accordingly so callers receive a thrown error on schema mismatch rather than silently getting [] (refer to parseApiTrails and rawTrails).
81-81: ⚡ Quick win
hasMoreproduces a false positive when results end exactly on a page boundary
trails.length >= limitistruewhenever a full page is returned, even if it's the final page. This causesTrailsPageto render a "Load more" button that, when clicked, returns an empty result set.The standard fix is an
n+1probe: fetchlimit + 1items, sethasMore = results.length > limit, then trim the extra item before returning.♻️ Proposed fix
const limit = params.limit ?? 20; const { data, error, status } = await apiClient.trails.search.get({ query: { ... - limit, + limit: limit + 1, offset: params.offset ?? 0, }, }); // ... const rawTrails = parseApiTrails(data); if (!rawTrails) { throw new Error('Search failed: unexpected response shape from API'); } + const hasMore = rawTrails.length > limit; + const trimmed = hasMore ? rawTrails.slice(0, limit) : rawTrails; - const trails: TrailSummaryWithCoords[] = rawTrails.map((t) => ({ ... })); + const trails: TrailSummaryWithCoords[] = trimmed.map((t) => ({ ... })); - return { trails, hasMore: trails.length >= limit }; + return { trails, hasMore };🤖 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` at line 81, The hasMore calculation currently uses "trails.length >= limit" which gives a false positive on exact page boundaries; change the query that populates "trails" to request limit+1 items, compute hasMore = trails.length > limit, then trim the extra item (e.g., slice to limit) before returning so the returned trails array never exceeds the requested page size and hasMore accurately reflects additional pages; update the logic around the variable names "trails", "limit", and "hasMore" in trailSearch.ts (the same scope that currently returns { trails, hasMore }) to implement this n+1 probe.apps/trails/components/AuthGate.tsx (1)
51-54: ⚡ Quick winFragile string-match for duplicate-account detection
Matching
'already'or'exists'against the API error message couples the UI to backend error wording. A future API change to the error format (e.g.,'Email taken'or an error code) will silently fall through to the generic toast without the "Log in" action.Consider using an error code or HTTP status (e.g., 409 Conflict) instead, if the API surfaces one.
🤖 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 51 - 54, The current duplicate-account check in AuthGate.tsx relies on fragile substring matching of the API error message; update the error handling where the toast/error is shown to detect a definitive indicator (preferably HTTP 409 or a structured error code) instead of msg.includes('already'|'exists'). Modify the condition that currently triggers toast.error(..., { action: { label: 'Log in', onClick: () => setTab('login') } }) to first check error.response?.status === 409 or error?.data?.code === 'ACCOUNT_EXISTS' (or other structured code your API provides), falling back to the generic message only if those are absent; keep the existing toast and setTab('login') behavior when the conflict indicator is found.apps/trails/components/TrailsPage.tsx (1)
90-90: ⚡ Quick winFragile
err.message.includes('429')check for rate-limit errorsMatching on the error message string couples this to whatever text happens to be in the
Errorthrown bysearchTrails. A dedicatedRateLimitErrorclass (alongsideAuthExpiredError) would make this check type-safe and resilient to message changes.🤖 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` at line 90, Replace the fragile string check by introducing a dedicated RateLimitError class and using it where searchTrails (and any other network/error-producing helpers) currently detect a 429; modify searchTrails to throw new RateLimitError(...) instead of a generic Error on 429 responses, then change the catch in TrailsPage.tsx from "err instanceof Error && err.message.includes('429')" to "err instanceof RateLimitError" (alongside existing AuthExpiredError checks) so the rate-limit branch is type-safe and resilient to message changes.
🤖 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`:
- Around line 70-72: The current flows write tokens to storage before calling
UserSchema.parse, which can throw and leave app state inconsistent; update each
flow in useAuthActions.ts (the main signIn flow and the functions
signInWithGoogle, signInWithApple, and verifyEmail) to validate the server user
payload with UserSchema.safeParse (or call UserSchema.parse) before any await
setToken(...) / setRefreshToken(...); on safeParse failure, handle the error
explicitly (reject/throw or return a controlled error) and do not persist tokens
or call userStore.set/redirect until the user object is validated and
userStore.set(UserSchema.parse(...)) succeeds.
In `@apps/trails/components/DownloadCTA.tsx`:
- Around line 24-35: The code reads and writes sessionStorage directly in the
useEffect and dismiss functions which can throw SecurityError in restricted
environments; wrap sessionStorage.getItem(DISMISSED_KEY) in a try/catch inside
useEffect and treat any catch as "not dismissed" (i.e., do not return early so
the CTA shows), and wrap sessionStorage.setItem(DISMISSED_KEY, '1') in a
try/catch inside dismiss so that any write error is swallowed but you still call
setVisible(false) to hide locally; keep existing logic that sets platform via
getStoreLinks() and updates state via setPlatform/setVisible.
In `@apps/trails/components/TrailCard.tsx`:
- Around line 30-36: The button in TrailCard currently toggles selection via the
selected prop but does not expose that state to assistive tech; update the
rendered <button> in the TrailCard component to include an ARIA state attribute
(e.g., aria-pressed={Boolean(selected)} or aria-current when representing
current navigation) so screen readers announce selection changes, ensuring you
reference the existing props/identifiers (selected, onClick, TrailCard) when
making the change.
- Line 5: The import of TrailSummaryWithCoords in TrailCard.tsx uses the
disallowed path 'trails-app/lib/overpass'; update the import to use the
project's approved path alias (e.g., the configured alias under tsconfig/paths
such as `@packrat/api/`... or the app-specific alias) so the line importing
TrailSummaryWithCoords from 'trails-app/lib/overpass' is replaced with the
correct aliased module; verify the alias resolves by checking tsconfig paths and
leaving the referenced symbol TrailSummaryWithCoords and the TrailCard component
unchanged.
In `@apps/trails/components/TrailMap.tsx`:
- Around line 75-99: The effect that imports Leaflet asynchronously can add
stale markers when multiple runs overlap; update the useEffect in TrailMap to
use a cancellation flag (e.g., let cancelled = false) before calling
import('leaflet'), call group.clearLayers() as you do, and after the import
resolves check if cancelled is true before creating/binding markers and adding
them to markersRef.current; also set cancelled = true in the effect cleanup so
any in-flight promise resolution is ignored (make sure to reference markersRef,
group.clearLayers, import('leaflet'), and the marker creation/marker.on('click')
logic).
In `@apps/trails/components/TrailsPage.tsx`:
- Around line 53-59: Redundant setMapState: when coords is null the initial
const center = coords ?? DEFAULT_CENTER followed by setMapState({ status:
'idle', center }) already covers the same state, so remove the duplicate
setMapState call inside the if (!coords) block and keep the early return; keep
setMapCenter(center) and the first setMapState call (and the early return) as-is
to preserve behavior (see center, coords, setMapCenter, setMapState).
In `@apps/trails/components/VerifyEmail.tsx`:
- Around line 60-83: The OTPInput lacks an accessible name; add an aria label so
screen readers get context. Update the OTPInput invocation (the component with
props value/onChange/onComplete/disabled/render) to supply an accessible name
either by adding aria-label="One-time passcode" (or a localized string) directly
if supported, or by passing inputProps={{ 'aria-label': 'One-time passcode' }}
to target the underlying input; alternatively create a visually-hidden <span
id="otp-label">One-time passcode</span> and pass aria-labelledby="otp-label" to
OTPInput so the invisible input has an accessible name.
In `@apps/trails/lib/apiClient.ts`:
- Around line 22-29: The onAccessTokenRefreshed and onRefreshTokenRefreshed
callbacks currently drop the refreshed token when the counterpart token is
missing (getRefreshToken/getAccessToken returns null); update both callbacks so
that if the counterpart token is absent you call clearTokens() and clearUser()
(and optionally trigger onNeedsReauth) instead of silently doing nothing, and
otherwise call setTokens(access, refresh) as before—modify the logic in
onAccessTokenRefreshed and onRefreshTokenRefreshed to explicitly handle the else
branch by clearing stored state to surface the failure.
In `@apps/trails/lib/auth.ts`:
- Around line 32-40: The mutator functions setTokens and clearTokens (and
likewise setUser and clearUser) perform localStorage operations without the
browser guard used by the getters, causing errors during server-side execution;
update each mutator to first check for the browser environment (e.g., typeof
window !== 'undefined' or typeof localStorage !== 'undefined') before calling
localStorage.setItem or removeItem, and return early when not in a browser so
they mirror the existing guard used in the getters (apply the same pattern in
functions setTokens, clearTokens, setUser, and clearUser).
In `@apps/trails/next.config.mjs`:
- Around line 7-9: Remove the silent build-time type/error suppression by
deleting or setting typescript.ignoreBuildErrors to false in next.config.mjs
(and remove or set eslint.ignoreDuringBuilds to false if present), so Next.js no
longer bypasses type/eslint checks; rely on the monorepo’s established
CI/pre-push step that runs tsc --noEmit (and ESLint) to catch regressions
instead. Ensure you update any referencing code or comments that justified the
previous ignore flags.
In `@apps/trails/tsconfig.json`:
- Around line 1-32: The tsconfig's compilerOptions is missing
noUncheckedIndexedAccess and uses target "ES6"; update compilerOptions to set
"noUncheckedIndexedAccess": true and change "target" to "ESNext" so this project
honors the repo strict-null policy and aligns with root config; ensure these
keys are added alongside the existing compilerOptions entries (e.g., next plugin
and paths) so indexing lookups are correctly checked for undefined.
- Around line 21-28: The tsconfig paths in apps/trails currently include
"@packrat/api/*" but lack the bare-import alias "@packrat/api", which can break
imports from the barrel; add a mapping for the bare alias so the compiler
resolves imports from "@packrat/api" to the package entry (i.e., point
"@packrat/api" to "../../packages/api/src/index.ts") alongside the existing
"@packrat/api/*" entry to match the root tsconfig behavior.
In `@apps/trails/worker/index.ts`:
- Around line 44-47: The rate-limit key currently falls back to the
client-controlled header 'X-Forwarded-For' which is spoofable; change the ip
selection to use only the Cloudflare-injected header by retrieving
request.headers.get('CF-Connecting-IP') and falling back to a constant like
'unknown' (remove any use of request.headers.get('X-Forwarded-For')). Update the
variable named ip in apps/trails/worker/index.ts (where
request.headers.get('CF-Connecting-IP') is read) so the rate limiter and key
generation rely solely on CF-Connecting-IP.
- Around line 49-54: The 429 rate-limit response emitted when success is false
uses jsonError(429, ...) which doesn't include CORS headers, causing browsers to
surface a CORS error; change the call site to return a response that includes
the appropriate CORS headers (e.g., Access-Control-Allow-Origin,
Access-Control-Allow-Methods and Access-Control-Allow-Headers or at minimum
Access-Control-Allow-Origin) alongside the JSON payload, either by updating
jsonError to accept an optional headers param or by wrapping its result and
injecting headers before returning; refer to the jsonError call in index.ts
where success is checked and ensure the 429 response includes the CORS headers
so the browser receives the actual status and body.
In `@apps/trails/wrangler.jsonc`:
- Around line 15-24: The wrangler.jsonc rate_limiting entry contains an
unresolved placeholder "__REPLACE_WITH_NAMESPACE_ID__" for the binding
"RATE_LIMITER" which will cause wrangler deploy to fail; replace the placeholder
with the actual namespace id (or a CI-safe lookup) and add a pre-deploy guard:
update your deploy script to validate wrangler.jsonc for the literal
"__REPLACE_WITH_NAMESPACE_ID__" (e.g., grep/fail) or automate provisioning and
commit the real namespace id for the "RATE_LIMITER" binding so deployments
cannot proceed with the placeholder.
---
Outside diff comments:
In `@CLAUDE.md`:
- Around line 182-190: The Path Aliases section and the root tsconfig.json are
missing the trails-app/* alias used by apps/trails; add an entry mapping
"trails-app/*" to "apps/trails/*" in the documented list and ensure the root
tsconfig.json "compilerOptions.paths" includes "trails-app/*": ["apps/trails/*"]
so imports like trails-app/lib/useAuth and trails-app/lib/auth resolve for tsc
and IDEs.
In `@package.json`:
- Around line 51-56: The package.json has a version mismatch: overrides pins
"@packrat-ai/nativewindui" to "2.0.3" while patchedDependencies references
"@packrat-ai/nativewindui@1.1.0", causing Bun to skip/apply the wrong patch;
update package.json so both refer to the same version (either remove the
patchedDependencies entry if the patch is no longer needed for 2.0.3, or change
the patchedDependencies key to "@packrat-ai/nativewindui@2.0.3" and supply a
compatible patch), ensuring the entries that reference
"@packrat-ai/nativewindui" are consistent across overrides and
patchedDependencies.
---
Nitpick comments:
In `@apps/expo/features/catalog/components/CatalogBrowserModal.tsx`:
- Around line 190-192: The fallback for computing items should use the nullish
coalescing operator instead of logical OR: when building items (the const items
using isSearching ? searchResult?.items || [] :
paginatedData?.pages.flatMap((page) => page.items) || []), replace the two uses
of || with ?? so you only fall back to [] for null/undefined values from
searchResult?.items and paginatedData?.pages.flatMap(...); keep the existing
isSearching, searchResult, paginatedData and pages references and preserve the
flatMap call.
In `@apps/expo/features/catalog/hooks/useCatalogItemDetails.ts`:
- Around line 7-9: The current fetch in useCatalogItemDetails only checks for
error before parsing, so add the same guard used in useVectorSearch: after
calling apiClient.catalog({ id }).get() check if (error || !data) and throw a
descriptive Error (e.g., `Failed to fetch catalog item: ${error?.value ?? 'no
data'}`) before calling CatalogItemSchema.parse(data) to avoid Zod parsing
undefined and keep API response handling consistent.
In `@apps/expo/features/catalog/hooks/useVectorSearch.ts`:
- Around line 16-17: The code currently only checks `error` before parsing the
response, so if `data` is null/undefined
`VectorSearchResponseSchema.parse(data)` can throw an unclear error; update the
guard to check `if (error || !data)` and throw a descriptive Error (e.g.,
"Vector search API returned no data" plus `error?.value` if present) before
calling `VectorSearchResponseSchema.parse(data)`, modifying the block around the
existing variables `error`, `data`, and the call to
`VectorSearchResponseSchema.parse` in `useVectorSearch`.
In `@apps/trails/components/AuthGate.tsx`:
- Around line 51-54: The current duplicate-account check in AuthGate.tsx relies
on fragile substring matching of the API error message; update the error
handling where the toast/error is shown to detect a definitive indicator
(preferably HTTP 409 or a structured error code) instead of
msg.includes('already'|'exists'). Modify the condition that currently triggers
toast.error(..., { action: { label: 'Log in', onClick: () => setTab('login') }
}) to first check error.response?.status === 409 or error?.data?.code ===
'ACCOUNT_EXISTS' (or other structured code your API provides), falling back to
the generic message only if those are absent; keep the existing toast and
setTab('login') behavior when the conflict indicator is found.
In `@apps/trails/components/TrailsPage.tsx`:
- Line 90: Replace the fragile string check by introducing a dedicated
RateLimitError class and using it where searchTrails (and any other
network/error-producing helpers) currently detect a 429; modify searchTrails to
throw new RateLimitError(...) instead of a generic Error on 429 responses, then
change the catch in TrailsPage.tsx from "err instanceof Error &&
err.message.includes('429')" to "err instanceof RateLimitError" (alongside
existing AuthExpiredError checks) so the rate-limit branch is type-safe and
resilient to message changes.
In `@apps/trails/lib/trailSearch.ts`:
- Line 68: The call currently masks schema failures by assigning rawTrails =
parseApiTrails(data) ?? []; change this to surface parsing errors: call
parseApiTrails(data) and if it returns null/undefined (or a failed parse), throw
a descriptive error (including the raw data or parse error details) instead of
falling back to an empty array; update the code paths using rawTrails
accordingly so callers receive a thrown error on schema mismatch rather than
silently getting [] (refer to parseApiTrails and rawTrails).
- Line 81: The hasMore calculation currently uses "trails.length >= limit" which
gives a false positive on exact page boundaries; change the query that populates
"trails" to request limit+1 items, compute hasMore = trails.length > limit, then
trim the extra item (e.g., slice to limit) before returning so the returned
trails array never exceeds the requested page size and hasMore accurately
reflects additional pages; update the logic around the variable names "trails",
"limit", and "hasMore" in trailSearch.ts (the same scope that currently returns
{ trails, hasMore }) to implement this n+1 probe.
In `@apps/trails/lib/useAuth.tsx`:
- Around line 53-58: Hydration currently marks the user authed if a token
exists, but getAccessToken() doesn't check expiry; update the useEffect in
useAuth (the block that calls getAccessToken() and getUser()) to call a new
isTokenExpired(token) and only setState({ isAuthed: true, user, pendingEmail:
null }) when token exists and isTokenExpired(token) returns false; add the
helper isTokenExpired in auth.ts (parse JWT payload, treat malformed tokens as
expired) and use it here to prevent briefly showing gated UI for
expired/malformed tokens.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0eccfd20-64d5-49c0-8b82-9ca471001c2d
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (42)
CLAUDE.mdapps/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.tsapps/trails/app/globals.cssapps/trails/app/layout.tsxapps/trails/app/page.tsxapps/trails/components/AuthGate.tsxapps/trails/components/DownloadCTA.tsxapps/trails/components/SearchBar.tsxapps/trails/components/TrailCard.tsxapps/trails/components/TrailMap.tsxapps/trails/components/TrailsPage.tsxapps/trails/components/VerifyEmail.tsxapps/trails/components/ui/sonner.tsxapps/trails/lib/apiClient.tsapps/trails/lib/auth.tsapps/trails/lib/env.tsapps/trails/lib/geolocation.tsapps/trails/lib/overpass.tsapps/trails/lib/trailSearch.tsapps/trails/lib/useAuth.tsxapps/trails/next.config.mjsapps/trails/package.jsonapps/trails/postcss.config.mjsapps/trails/tailwind.config.tsapps/trails/tsconfig.jsonapps/trails/worker/index.tsapps/trails/wrangler.jsoncpackage.jsonpackages/api/src/schemas/catalog.tspackages/env/scripts/no-raw-process-env.tstsconfig.json
Comment on lines
70
to
+72
| await setToken(data.accessToken); | ||
| 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)); |
Contributor
There was a problem hiding this comment.
UserSchema.parse after token writes leaves session in an inconsistent state on schema mismatch.
The execution order in all four flows is:
await setToken(data.accessToken); // tokens written to storage ✓
await setRefreshToken(data.refreshToken);
userStore.set(UserSchema.parse(data.user)); // ZodError thrown here ✗
redirect(redirectTo); // never reached
If data.user doesn't satisfy UserSchema (missing required field, renamed key, unexpected null, etc.), parse throws a ZodError that is caught and re-thrown. The result is:
- Access and refresh tokens are already persisted in storage.
userStoreis not updated — no user in the reactive store.redirectnever executes — the user stays on the login screen.
On the next cold start the app has valid tokens but no user record — an inconsistent session that's hard to recover from without an explicit logout.
Use safeParse and handle failure explicitly before any storage writes, or move the parse before the token writes:
🛡️ Proposed fix — validate before committing tokens
- await setToken(data.accessToken);
- await setRefreshToken(data.refreshToken);
- userStore.set(UserSchema.parse(data.user));
+ const parsedUser = UserSchema.safeParse(data.user);
+ if (!parsedUser.success) {
+ throw new Error(t('auth.failedToSignIn')); // or a dedicated i18n key
+ }
+ await setToken(data.accessToken);
+ await setRefreshToken(data.refreshToken);
+ userStore.set(parsedUser.data);Apply the same pattern at lines 101–103 (signInWithGoogle), 148–150 (signInWithApple), and 255–257 (verifyEmail).
Also applies to: 101-103, 148-150, 255-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` around lines 70 - 72, The
current flows write tokens to storage before calling UserSchema.parse, which can
throw and leave app state inconsistent; update each flow in useAuthActions.ts
(the main signIn flow and the functions signInWithGoogle, signInWithApple, and
verifyEmail) to validate the server user payload with UserSchema.safeParse (or
call UserSchema.parse) before any await setToken(...) / setRefreshToken(...); on
safeParse failure, handle the error explicitly (reject/throw or return a
controlled error) and do not persist tokens or call userStore.set/redirect until
the user object is validated and userStore.set(UserSchema.parse(...)) succeeds.
Comment on lines
+24
to
+35
| useEffect(() => { | ||
| if (sessionStorage.getItem(DISMISSED_KEY)) return; | ||
| const { ios, android } = getStoreLinks(); | ||
| if (ios) setPlatform('ios'); | ||
| else if (android) setPlatform('android'); | ||
| setVisible(true); | ||
| }, []); | ||
|
|
||
| function dismiss() { | ||
| sessionStorage.setItem(DISMISSED_KEY, '1'); | ||
| setVisible(false); | ||
| } |
Contributor
There was a problem hiding this comment.
Harden sessionStorage access against runtime SecurityError
sessionStorage.getItem/setItem can throw in privacy-restricted contexts; this currently can break CTA init/dismiss flows. Wrap both calls in try/catch and fail open (show CTA if read fails, still hide locally if write fails).
🤖 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/DownloadCTA.tsx` around lines 24 - 35, The code reads
and writes sessionStorage directly in the useEffect and dismiss functions which
can throw SecurityError in restricted environments; wrap
sessionStorage.getItem(DISMISSED_KEY) in a try/catch inside useEffect and treat
any catch as "not dismissed" (i.e., do not return early so the CTA shows), and
wrap sessionStorage.setItem(DISMISSED_KEY, '1') in a try/catch inside dismiss so
that any write error is swallowed but you still call setVisible(false) to hide
locally; keep existing logic that sets platform via getStoreLinks() and updates
state via setPlatform/setVisible.
|
|
||
| import { cn } from '@packrat/web-ui/lib/utils'; | ||
| import { MapPin, Mountain } from 'lucide-react'; | ||
| import type { TrailSummaryWithCoords } from 'trails-app/lib/overpass'; |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use one of the approved import path aliases
trails-app/lib/overpass does not match the allowed alias set in project rules. Please switch this import to an approved alias mapping for this app.
As per coding guidelines "Path aliases must be used for imports: @packrat/api/, @packrat/ui/, expo-app/, guides-app/, landing-app/, nativewindui/".
🤖 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/TrailCard.tsx` at line 5, The import of
TrailSummaryWithCoords in TrailCard.tsx uses the disallowed path
'trails-app/lib/overpass'; update the import to use the project's approved path
alias (e.g., the configured alias under tsconfig/paths such as `@packrat/api/`...
or the app-specific alias) so the line importing TrailSummaryWithCoords from
'trails-app/lib/overpass' is replaced with the correct aliased module; verify
the alias resolves by checking tsconfig paths and leaving the referenced symbol
TrailSummaryWithCoords and the TrailCard component unchanged.
Comment on lines
+30
to
+36
| <button | ||
| type="button" | ||
| onClick={onClick} | ||
| className={cn( | ||
| 'w-full rounded-lg border p-4 text-left transition-colors hover:bg-accent', | ||
| selected && 'border-primary bg-accent', | ||
| )} |
Contributor
There was a problem hiding this comment.
Expose selection state to assistive technologies
This control has a selectable state but does not communicate it. Add aria-pressed={Boolean(selected)} (or aria-current if this represents current item navigation) so screen readers announce state changes.
🤖 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/TrailCard.tsx` around lines 30 - 36, The button in
TrailCard currently toggles selection via the selected prop but does not expose
that state to assistive tech; update the rendered <button> in the TrailCard
component to include an ARIA state attribute (e.g.,
aria-pressed={Boolean(selected)} or aria-current when representing current
navigation) so screen readers announce selection changes, ensuring you reference
the existing props/identifiers (selected, onClick, TrailCard) when making the
change.
Comment on lines
+75
to
+99
| useEffect(() => { | ||
| if (!markersRef.current) return; | ||
| const group = markersRef.current; | ||
| group.clearLayers(); | ||
|
|
||
| import('leaflet').then(({ default: L }) => { | ||
| for (const trail of trails) { | ||
| if (!trail.center) continue; | ||
| const isSelected = trail.osmId === selectedOsmId; | ||
| const marker = L.circleMarker(trail.center, { | ||
| radius: isSelected ? 10 : 7, | ||
| fillColor: isSelected ? '#6366f1' : '#3b82f6', | ||
| color: '#fff', | ||
| weight: 2, | ||
| opacity: 1, | ||
| fillOpacity: 0.9, | ||
| }); | ||
| marker.bindTooltip(trail.name ?? 'Unnamed Trail', { permanent: false, direction: 'top' }); | ||
| if (onTrailClick) { | ||
| marker.on('click', () => onTrailClick(trail.osmId)); | ||
| } | ||
| group.addLayer(marker); | ||
| } | ||
| }); | ||
| }, [trails, selectedOsmId, onTrailClick]); |
Contributor
There was a problem hiding this comment.
Stale markers from concurrent effect runs due to unguarded async import.
When trails, selectedOsmId, and onTrailClick change close together (e.g., initial data load), React can fire this effect multiple times before the first import('leaflet') promise resolves. clearLayers() is synchronous, so a later effect clears the group — but then the earlier promise resolves and adds its now-stale markers on top of the correct set.
🐛 Proposed fix — cancellation flag inside the effect
useEffect(() => {
if (!markersRef.current) return;
const group = markersRef.current;
group.clearLayers();
+ let cancelled = false;
import('leaflet').then(({ default: L }) => {
+ if (cancelled) return;
for (const trail of trails) {
if (!trail.center) continue;
const isSelected = trail.osmId === selectedOsmId;
const marker = L.circleMarker(trail.center, {
radius: isSelected ? 10 : 7,
fillColor: isSelected ? '#6366f1' : '#3b82f6',
color: '#fff',
weight: 2,
opacity: 1,
fillOpacity: 0.9,
});
marker.bindTooltip(trail.name ?? 'Unnamed Trail', { permanent: false, direction: 'top' });
if (onTrailClick) {
marker.on('click', () => onTrailClick(trail.osmId));
}
group.addLayer(marker);
}
});
+
+ return () => { cancelled = true; };
}, [trails, selectedOsmId, onTrailClick]);🤖 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 75 - 99, The effect that
imports Leaflet asynchronously can add stale markers when multiple runs overlap;
update the useEffect in TrailMap to use a cancellation flag (e.g., let cancelled
= false) before calling import('leaflet'), call group.clearLayers() as you do,
and after the import resolves check if cancelled is true before creating/binding
markers and adding them to markersRef.current; also set cancelled = true in the
effect cleanup so any in-flight promise resolution is ignored (make sure to
reference markersRef, group.clearLayers, import('leaflet'), and the marker
creation/marker.on('click') logic).
Comment on lines
+1
to
+32
| { | ||
| "compilerOptions": { | ||
| "lib": ["dom", "dom.iterable", "esnext"], | ||
| "allowJs": true, | ||
| "target": "ES6", | ||
| "skipLibCheck": true, | ||
| "strict": true, | ||
| "noEmit": true, | ||
| "esModuleInterop": true, | ||
| "module": "esnext", | ||
| "moduleResolution": "bundler", | ||
| "resolveJsonModule": true, | ||
| "isolatedModules": true, | ||
| "jsx": "preserve", | ||
| "incremental": true, | ||
| "plugins": [ | ||
| { | ||
| "name": "next" | ||
| } | ||
| ], | ||
| "paths": { | ||
| "trails-app/*": ["./*"], | ||
| "@packrat/api/*": ["../../packages/api/src/*"], | ||
| "@packrat/overpass": ["../../packages/overpass/src/index.ts"], | ||
| "@packrat/overpass/*": ["../../packages/overpass/src/*"], | ||
| "@packrat/web-ui": ["../../packages/web-ui/src"], | ||
| "@packrat/web-ui/*": ["../../packages/web-ui/src/*"] | ||
| } | ||
| }, | ||
| "include": ["next-env.d.ts", "**/*.ts", "**/*.tsx", ".next/types/**/*.ts"], | ||
| "exclude": ["node_modules"] | ||
| } |
Contributor
There was a problem hiding this comment.
noUncheckedIndexedAccess is missing — violates the project's strict-null policy.
The root tsconfig.json enables noUncheckedIndexedAccess: true, and the coding guidelines require it. Because apps/trails/tsconfig.json doesn't extend the root config, this safety net is absent here, so array/object indexed lookups will not be flagged as potentially undefined.
📐 Proposed fix
"compilerOptions": {
"lib": ["dom", "dom.iterable", "esnext"],
"allowJs": true,
- "target": "ES6",
+ "target": "ESNext",
"skipLibCheck": true,
"strict": true,
+ "noUncheckedIndexedAccess": true,
"noEmit": true,(target: "ESNext" also aligns with the root config and the rest of the monorepo.)
As per coding guidelines: "Enable strict null checks with no unchecked indexed access."
📝 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.
Suggested change
| { | |
| "compilerOptions": { | |
| "lib": ["dom", "dom.iterable", "esnext"], | |
| "allowJs": true, | |
| "target": "ES6", | |
| "skipLibCheck": true, | |
| "strict": true, | |
| "noEmit": true, | |
| "esModuleInterop": true, | |
| "module": "esnext", | |
| "moduleResolution": "bundler", | |
| "resolveJsonModule": true, | |
| "isolatedModules": true, | |
| "jsx": "preserve", | |
| "incremental": true, | |
| "plugins": [ | |
| { | |
| "name": "next" | |
| } | |
| ], | |
| "paths": { | |
| "trails-app/*": ["./*"], | |
| "@packrat/api/*": ["../../packages/api/src/*"], | |
| "@packrat/overpass": ["../../packages/overpass/src/index.ts"], | |
| "@packrat/overpass/*": ["../../packages/overpass/src/*"], | |
| "@packrat/web-ui": ["../../packages/web-ui/src"], | |
| "@packrat/web-ui/*": ["../../packages/web-ui/src/*"] | |
| } | |
| }, | |
| "include": ["next-env.d.ts", "**/*.ts", "**/*.tsx", ".next/types/**/*.ts"], | |
| "exclude": ["node_modules"] | |
| } | |
| { | |
| "compilerOptions": { | |
| "lib": ["dom", "dom.iterable", "esnext"], | |
| "allowJs": true, | |
| "target": "ESNext", | |
| "skipLibCheck": true, | |
| "strict": true, | |
| "noUncheckedIndexedAccess": true, | |
| "noEmit": true, | |
| "esModuleInterop": true, | |
| "module": "esnext", | |
| "moduleResolution": "bundler", | |
| "resolveJsonModule": true, | |
| "isolatedModules": true, | |
| "jsx": "preserve", | |
| "incremental": true, | |
| "plugins": [ | |
| { | |
| "name": "next" | |
| } | |
| ], | |
| "paths": { | |
| "trails-app/*": ["./*"], | |
| "@packrat/api/*": ["../../packages/api/src/*"], | |
| "@packrat/overpass": ["../../packages/overpass/src/index.ts"], | |
| "@packrat/overpass/*": ["../../packages/overpass/src/*"], | |
| "@packrat/web-ui": ["../../packages/web-ui/src"], | |
| "@packrat/web-ui/*": ["../../packages/web-ui/src/*"] | |
| } | |
| }, | |
| "include": ["next-env.d.ts", "**/*.ts", "**/*.tsx", ".next/types/**/*.ts"], | |
| "exclude": ["node_modules"] | |
| } |
🤖 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 1 - 32, The tsconfig's
compilerOptions is missing noUncheckedIndexedAccess and uses target "ES6";
update compilerOptions to set "noUncheckedIndexedAccess": true and change
"target" to "ESNext" so this project honors the repo strict-null policy and
aligns with root config; ensure these keys are added alongside the existing
compilerOptions entries (e.g., next plugin and paths) so indexing lookups are
correctly checked for undefined.
Comment on lines
+21
to
+28
| "paths": { | ||
| "trails-app/*": ["./*"], | ||
| "@packrat/api/*": ["../../packages/api/src/*"], | ||
| "@packrat/overpass": ["../../packages/overpass/src/index.ts"], | ||
| "@packrat/overpass/*": ["../../packages/overpass/src/*"], | ||
| "@packrat/web-ui": ["../../packages/web-ui/src"], | ||
| "@packrat/web-ui/*": ["../../packages/web-ui/src/*"] | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether any trails source file imports from '@packrat/api' (bare, no subpath)
rg --type=ts --type=tsx -n "from '@packrat/api'" apps/trails 2>/dev/null || \
rg -n "from '@packrat/api'" apps/trailsRepository: PackRat-AI/PackRat
Length of output: 44
🏁 Script executed:
cat -n tsconfig.json | head -100Repository: PackRat-AI/PackRat
Length of output: 2687
🏁 Script executed:
rg -n "from '@packrat/api'" --type=ts --type=tsx 2>/dev/null || echo "No matches found"Repository: PackRat-AI/PackRat
Length of output: 79
Add missing @packrat/api bare-import alias to maintain parity with root config.
The root tsconfig.json defines both "@packrat/api" (barrel) and "@packrat/api/*" (sub-paths), but this file only includes the wildcard form. To prevent type-checking failures if developers import from the barrel entry point, add:
"@packrat/api": ["../../packages/api/src/index.ts"]
🤖 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 21 - 28, The tsconfig paths in
apps/trails currently include "@packrat/api/*" but lack the bare-import alias
"@packrat/api", which can break imports from the barrel; add a mapping for the
bare alias so the compiler resolves imports from "@packrat/api" to the package
entry (i.e., point "@packrat/api" to "../../packages/api/src/index.ts")
alongside the existing "@packrat/api/*" entry to match the root tsconfig
behavior.
Comment on lines
+44
to
+47
| const ip = | ||
| request.headers.get('CF-Connecting-IP') ?? | ||
| request.headers.get('X-Forwarded-For') ?? | ||
| 'unknown'; |
Contributor
There was a problem hiding this comment.
X-Forwarded-For fallback for rate-limit key is client-spoofable
In a Cloudflare Worker CF-Connecting-IP is always injected by the Cloudflare network and cannot be forged by a client. X-Forwarded-For, by contrast, is a regular request header that any client can set arbitrarily (e.g., X-Forwarded-For: 1.2.3.4), allowing a bad actor to rotate through fake IPs and bypass the rate limiter entirely.
🛡️ Proposed fix — drop the spoofable fallback
- const ip =
- request.headers.get('CF-Connecting-IP') ??
- request.headers.get('X-Forwarded-For') ??
- 'unknown';
+ // CF-Connecting-IP is always set by Cloudflare; no client-controllable fallback
+ const ip = request.headers.get('CF-Connecting-IP') ?? 'unknown';📝 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.
Suggested change
| const ip = | |
| request.headers.get('CF-Connecting-IP') ?? | |
| request.headers.get('X-Forwarded-For') ?? | |
| 'unknown'; | |
| // CF-Connecting-IP is always set by Cloudflare; no client-controllable fallback | |
| const ip = request.headers.get('CF-Connecting-IP') ?? 'unknown'; |
🤖 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 44 - 47, The rate-limit key
currently falls back to the client-controlled header 'X-Forwarded-For' which is
spoofable; change the ip selection to use only the Cloudflare-injected header by
retrieving request.headers.get('CF-Connecting-IP') and falling back to a
constant like 'unknown' (remove any use of
request.headers.get('X-Forwarded-For')). Update the variable named ip in
apps/trails/worker/index.ts (where request.headers.get('CF-Connecting-IP') is
read) so the rate limiter and key generation rely solely on CF-Connecting-IP.
Comment on lines
+49
to
+54
| if (!success) { | ||
| return jsonError( | ||
| 429, | ||
| JSON.stringify({ error: 'Too many requests. Please try again in a moment.' }), | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
Rate-limited 429 responses are missing CORS headers — browsers surface a CORS error instead
jsonError(429, ...) returns no Access-Control-Allow-Origin header. A cross-origin browser client will see a CORS failure (not a 429), making the rate-limit condition completely opaque to the client's error-handling code.
🛡️ Proposed fix — inject CORS headers into the 429 response
- if (!success) {
- return jsonError(
- 429,
- JSON.stringify({ error: 'Too many requests. Please try again in a moment.' }),
- );
- }
+ if (!success) {
+ return new Response(
+ JSON.stringify({ error: 'Too many requests. Please try again in a moment.' }),
+ {
+ status: 429,
+ headers: { 'Content-Type': 'application/json', ...corsHeaders(origin) },
+ },
+ );
+ }🤖 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 49 - 54, The 429 rate-limit
response emitted when success is false uses jsonError(429, ...) which doesn't
include CORS headers, causing browsers to surface a CORS error; change the call
site to return a response that includes the appropriate CORS headers (e.g.,
Access-Control-Allow-Origin, Access-Control-Allow-Methods and
Access-Control-Allow-Headers or at minimum Access-Control-Allow-Origin)
alongside the JSON payload, either by updating jsonError to accept an optional
headers param or by wrapping its result and injecting headers before returning;
refer to the jsonError call in index.ts where success is checked and ensure the
429 response includes the CORS headers so the browser receives the actual status
and body.
Comment on lines
+15
to
+24
| "rate_limiting": [ | ||
| { | ||
| "binding": "RATE_LIMITER", | ||
| "namespace_id": "__REPLACE_WITH_NAMESPACE_ID__", | ||
| "simple": { | ||
| "limit": 60, | ||
| "period": 60 | ||
| } | ||
| } | ||
| ], |
Contributor
There was a problem hiding this comment.
Unresolved placeholder __REPLACE_WITH_NAMESPACE_ID__ will break wrangler deploy.
The comment documents the fix, but there is no CI guard preventing a deploy with this literal string. If the namespace isn't created before the first deploy, Wrangler will reject the config. Consider adding a pre-deploy check (e.g., a grep in the deploy script) or provisioning the namespace as part of the repo setup and committing the real ID.
🤖 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/wrangler.jsonc` around lines 15 - 24, The wrangler.jsonc
rate_limiting entry contains an unresolved placeholder
"__REPLACE_WITH_NAMESPACE_ID__" for the binding "RATE_LIMITER" which will cause
wrangler deploy to fail; replace the placeholder with the actual namespace id
(or a CI-safe lookup) and add a pre-deploy guard: update your deploy script to
validate wrangler.jsonc for the literal "__REPLACE_WITH_NAMESPACE_ID__" (e.g.,
grep/fail) or automate provisioning and commit the real namespace id for the
"RATE_LIMITER" binding so deployments cannot proceed with the placeholder.
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>
Contributor
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>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
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/pack-templates/components/AddPackTemplateItemActions.tsx (1)
121-129:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle
addItemsToPackTemplatefailures explicitly.This external write can reject, but the flow has no local catch/UX fallback in this handler.
Proposed fix
const handleCatalogItemsSelected = async (catalogItems: CatalogItem[]) => { trackRecentlyUsed(catalogItems); - await addItemsToPackTemplate(packTemplateId, catalogItems); + try { + await addItemsToPackTemplate(packTemplateId, catalogItems); + } catch (error) { + console.error('Error adding catalog items to pack template:', error); + appAlert.current?.alert({ + title: t('packTemplates.error'), + message: t('packTemplates.failedToProcessImage'), + buttons: [{ text: t('common.ok'), style: 'default' }], + }); + return; + } const itemWord = catalogItems.length === 1 ? t('packTemplates.item') : t('packTemplates.items'); Burnt.toast({🤖 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/pack-templates/components/AddPackTemplateItemActions.tsx` around lines 121 - 129, The handler handleCatalogItemsSelected currently awaits addItemsToPackTemplate without error handling; wrap the call to addItemsToPackTemplate(packTemplateId, catalogItems) in a try/catch so the success Burnt.toast only runs on success, and in the catch show an error toast (e.g., Burnt.toast with preset 'error' and a message from the caught error or a translation key), log the error (console.error or existing logger), and avoid reporting success; keep trackRecentlyUsed(catalogItems) placement as desired (either before or after the save) and adjust if you want to roll it back on failure.
♻️ Duplicate comments (2)
apps/expo/features/auth/hooks/useAuthActions.ts (1)
70-72:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate
data.userbefore persisting tokens/storage in all auth flows
UserSchema.parse(data.user)still runs after token/storage writes (Line 70-72, 101-103, 148-150, 253-257).
If parse throws, session becomes inconsistent (persisted tokens + missing user store + interrupted redirect).Suggested fix pattern (apply to all four flows)
- await setToken(data.accessToken); - await setRefreshToken(data.refreshToken); - userStore.set(UserSchema.parse(data.user)); + const parsedUser = UserSchema.safeParse(data.user); + if (!parsedUser.success) { + throw new Error(t('auth.failedToSignIn')); + } + await setToken(data.accessToken); + await setRefreshToken(data.refreshToken); + userStore.set(parsedUser.data);Also applies to: 101-103, 148-150, 253-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` around lines 70 - 72, Validate and parse the returned user before persisting anything: call UserSchema.parse(data.user) first and only if it succeeds proceed to await setToken(data.accessToken); await setRefreshToken(data.refreshToken); and userStore.set(parsedUser). Apply this pattern in all auth flows that currently call setToken/setRefreshToken then UserSchema.parse (the blocks around UserSchema.parse in useAuthActions.ts, specifically the sites referenced at lines ~70-72, ~101-103, ~148-150, and ~253-257), and on parse failure avoid writing tokens or mutating userStore and surface/handle the error instead.apps/trails/components/DownloadCTA.tsx (1)
24-35:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard
sessionStoragecalls to prevent runtimeSecurityErrorcrashes.Line 25 and Line 33 can throw in privacy-restricted contexts, which can break CTA init/dismiss behavior. Please wrap both storage calls in
try/catchand fail open.Proposed fix
useEffect(() => { - if (sessionStorage.getItem(DISMISSED_KEY)) return; + try { + if (sessionStorage.getItem(DISMISSED_KEY)) return; + } catch { + // Ignore storage access failures; continue showing CTA. + } const { ios, android } = getStoreLinks(); if (ios) setPlatform('ios'); else if (android) setPlatform('android'); setVisible(true); }, []); function dismiss() { - sessionStorage.setItem(DISMISSED_KEY, '1'); + try { + sessionStorage.setItem(DISMISSED_KEY, '1'); + } catch { + // Ignore storage write failures; still hide CTA locally. + } setVisible(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/DownloadCTA.tsx` around lines 24 - 35, The sessionStorage accesses in useEffect and dismiss (sessionStorage.getItem(DISMISSED_KEY) and sessionStorage.setItem(DISMISSED_KEY, '1')) can throw SecurityError; wrap both calls in try/catch and "fail open" (i.e., on error behave as if nothing was stored: proceed to compute platform and show CTA in useEffect, and in dismiss simply hide the CTA without rethrowing). Update the useEffect and dismiss functions in DownloadCTA.tsx to catch errors around sessionStorage, log/debug if desired, and continue to use setPlatform/setVisible/getStoreLinks unchanged.
🧹 Nitpick comments (7)
apps/trails/components/SearchBar.tsx (1)
19-24: 💤 Low valueSilently blurring an unauthenticated user's focus is an accessibility gap.
Keyboard and screen-reader users who tab to the input get focus instantly removed without any explanation. Consider adding
aria-labeloraria-describedbytext and/or anonClickfallback so AT users can still discover and trigger the auth gate.🤖 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/SearchBar.tsx` around lines 19 - 24, handleFocus currently blurs unauthenticated users which removes focus without explanation; instead stop silently blurring in handleFocus, call openAuthGate but keep focus on the input (or move focus to the opened auth modal), and add accessible affordances: give the inputRef-controlled element an aria-describedby pointing to a short helper message element (create e.g. <span id="searchAuthHint"> that explains "Sign in to search"), ensure the input has an aria-label or descriptive placeholder, and add a pointer fallback by wiring an onClick handler that calls openAuthGate (and an onKeyDown handler for Enter/Space to call openAuthGate) so keyboard and screen reader users can discover and trigger the auth flow; reference the existing handleFocus, inputRef, isAuthed, and openAuthGate when making these changes.apps/trails/components/AuthGate.tsx (1)
49-57: ⚖️ Poor tradeoffBrittle error-message string sniffing for "account already exists" detection.
Matching
.includes('already') || .includes('exists')on the error message is fragile — a server-side message change or i18n silently breaks the "switch to login" UX. Prefer a typed error code from the API (e.g.,error.code === 'EMAIL_EXISTS').🤖 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 49 - 57, Replace brittle string-sniffing in the catch block with a check for a typed error code on the thrown error (e.g., examine err.code or err.error?.code and/or narrow err to your API error type), and treat the specific code (for example 'EMAIL_EXISTS' or whatever your API uses) as the trigger to show the "Account already exists" toast and call setTab('login'); otherwise fall back to the existing msg logic. Update the condition around the err variable (in the catch where toast.error and setTab are used) to prefer err.code comparison, handle both string and nested error shapes safely, and keep the existing toast action behavior for the specific error code.apps/trails/components/TrailMap.tsx (2)
21-66: ⚡ Quick winUnhandled
init()promise — errors from Leaflet initialization are silently swallowed.
init(container)is called at Line 59 withoutawaitand without a.catch(). Ifimport('leaflet')rejects (network error, bundler issue) or Leaflet throws during map construction (e.g., container already initialized in React Strict Mode), the error is lost with no user feedback.♻️ Proposed fix
- init(container); + init(container).catch((err) => { + console.error('[TrailMap] Failed to initialize map:', err); + });🤖 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 init(container) call can reject and its errors are currently swallowed; make the useEffect start the Leaflet initialization inside an async IIFE or attach a .catch so the promise is awaited/handled, and wrap the body of init (including the dynamic import('leaflet') and map creation) in try/catch to log errors and perform cleanup; specifically update the useEffect to call an async function that awaits init(container) (or awaits the import and map setup inline), catch any errors and call console.error or the app logger, and ensure mapRef.current and markersRef.current are left in a consistent state on failure so subsequent mounts won't assume initialization succeeded.
36-38: 💤 Low valueLeaflet marker icons depend on
unpkg.comCDN availability.A CDN outage or CSP restriction will silently break map marker icons. Serving these assets locally (e.g., copying them to
/public/leaflet/) removes the external dependency.🤖 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 36 - 38, The marker icon URLs (iconRetinaUrl, iconUrl, shadowUrl) in TrailMap.tsx rely on unpkg.com and should be served locally to avoid CDN/CSP failures — copy the three Leaflet images into your app's public folder (e.g., /public/leaflet/) and update the icon paths used when configuring the default icon (where iconRetinaUrl, iconUrl, shadowUrl are set or where L.Icon.Default.mergeOptions is called) to point to the local URLs (e.g., /leaflet/marker-icon-2x.png, /leaflet/marker-icon.png, /leaflet/marker-shadow.png) or import the images and reference those imports instead so marker icons load reliably without external CDN dependency.apps/trails/lib/trailSearch.ts (2)
67-68: ⚡ Quick winSilent Zod parse failure produces an empty result set with no diagnostic signal.
If
datadoesn't matchApiTrailSchema[](e.g., due to an API contract change),parseApiTrailsreturnsnull/undefined, and the?? []fallback silently delivers zero results to the user. Aconsole.warnwould make this immediately visible in production logs.♻️ Proposed fix
- const rawTrails = parseApiTrails(data) ?? []; + const rawTrails = parseApiTrails(data); + if (!rawTrails) { + console.warn('[searchTrails] API response did not match expected schema', data); + } + const validTrails = rawTrails ?? [];🤖 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 67 - 68, The current silent fallback on parseApiTrails(data) can hide schema mismatches; update the logic around parseApiTrails in trailSearch.ts so that when parseApiTrails(data) returns null/undefined you emit a console.warn (or processLogger.warn) including a short diagnostic message, the fact parsing failed for ApiTrailSchema[], and relevant context (e.g., the raw data or a JSON snippet) before falling back to an empty array; keep the existing rawTrails = parseApiTrails(data) ?? [] pattern but add the warning branch so parsing failures are visible in logs.
81-81: ⚡ Quick win
hasMoreproduces a false positive when the total count equalslimit.
trails.length >= limitsignals "more results" when the API returns exactlylimititems, even if the page is the last one. This triggers an unnecessary empty "load more" request. The idiomatic fix is to requestlimit + 1items and checktrails.length > limit, slicing the extra before returning.♻️ Proposed fix
const limit = params.limit ?? 20; const { data, error, status } = await apiClient.trails.search.get({ query: { ... - limit, + limit: limit + 1, offset: params.offset ?? 0, }, }); ... + const hasMore = rawTrails.length > limit; + const trails: TrailSummaryWithCoords[] = rawTrails.slice(0, limit).map((t) => ({ - const trails: TrailSummaryWithCoords[] = rawTrails.map((t) => ({🤖 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` at line 81, The current hasMore calculation uses "trails.length >= limit" which yields false positives when the API returns exactly limit items; change the fetching logic to request limit + 1 items, then set hasMore to "trails.length > limit" and slice the returned trails down to the original limit before returning; update the code paths that call/define the limit/fetch to request limit + 1 and adjust the return in the function that returns { trails, hasMore } (referencing the variables/functions named "limit" and "trails" in trailSearch.ts).apps/trails/components/TrailsPage.tsx (1)
24-27: 💤 Low value
errorvariant inMapStateis dead code
setMapState({ status: 'error', ... })is never called anywhere in the component, and the render path on lines 152–163 only branches on'loading'vs everything-else — so anerrorstate would silently render the map. Either wire up the error state (e.g., catch agetUserLocationfailure and set it) or remove the unused variant to keep the type honest.♻️ Option A — remove the unused variant
type MapState = | { status: 'loading' } - | { status: 'idle'; center: [number, number] } - | { status: 'error'; message: string }; + | { status: 'idle'; center: [number, number] };♻️ Option B — wire it up in loadMap and render it
const coords = await getUserLocation(); if (cancelled) return; + if (!coords && /* some failure condition */) { + setMapState({ status: 'error', message: 'Could not determine location.' }); + return; + } const center = coords ?? DEFAULT_CENTER;And in the render:
- ) : ( + ) : mapState.status === 'error' ? ( + <div className="flex h-full items-center justify-center bg-muted"> + <p className="text-sm text-destructive">{mapState.message}</p> + </div> + ) : (🤖 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 24 - 27, MapState's 'error' variant is declared but never used; update loadMap (the function calling getUserLocation) to catch failures and call setMapState({ status: 'error', message: err.message || String(err) }) and update the render branch that currently distinguishes only 'loading' vs else (the JSX that renders the map) to explicitly handle status === 'error' (render an error message UI using MapState.message) and keep the existing map render for status === 'idle'; this ensures MapState (and setMapState) are wired correctly and the error case is shown instead of silently rendering the map.
🤖 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/catalog/screens/CatalogItemsScreen.tsx`:
- Around line 72-74: The filter currently drops falsy IDs (e.g., 0) by using
Boolean(item?.id); update the predicate used when building paginatedItems so it
explicitly checks for null/undefined instead: keep the flatMap over
paginatedData?.pages to collect page.items, then filter with item?.id != null
(or a type predicate) so valid zero IDs are retained; locate the paginatedItems
definition and replace the Boolean(item?.id) predicate accordingly.
In `@apps/expo/features/profile/types.ts`:
- Around line 1-2: The imports for UserSchema and z are declared as type-only
but are used at runtime with typeof, so change both "import type { UserSchema }"
and "import type { z }" to regular runtime imports (e.g., import { UserSchema }
and import { z }) so the symbols are present for typeof UserSchema and any z
usage; update the import statement at the top of types.ts to remove "type"
qualifiers while keeping the same named imports.
In `@apps/trails/components/AuthGate.tsx`:
- Around line 76-87: handleForgot currently ignores the Treaty client response
and always calls setForgotSent even when the API returned an error object;
update handleForgot to await and destructure the response from
apiClient.auth['forgot-password'].post into { data, error, status } (or the
client’s response shape), then only call setForgotSent(true) when error is falsy
and status indicates success; if error exists, call toast.error with the error
message (or a fallback) instead of setting forgotSent. Ensure you keep the
existing try/catch/finally behavior for thrown exceptions but add the
response-level error check before setting state.
In `@apps/trails/components/TrailsPage.tsx`:
- Around line 85-93: When runSearch throws and append === false you never clear
the previous results, so call setSearchTrailResults([]) (or otherwise reset the
results state used by TrailsPage) inside the catch branch for non-auth/429
errors (and/or for all errors when append is false) so the UI doesn't show stale
results; update the catch block that currently checks err instanceof
AuthExpiredError / err.message.includes('429') to also clear results via
setSearchTrailResults([]) when append === false and ensure any loading flags
(e.g., isSearching) are set consistently.
- Around line 76-99: runSearch can return stale results because concurrent
searchTrails requests are not cancellable; add an AbortController per runSearch
invocation, store it in a ref (e.g., lastControllerRef), and before starting a
new search abort any existing controller; pass the controller.signal into
searchTrails via its second param fetch option (searchTrails(params, { fetch: {
signal } })), then set lastControllerRef.current = controller and ensure you
abort/cleanup on component unmount; update runSearch to create/abort the
controller, and keep existing logic (setSearchLoading, setSearchTrailResults,
setOffset, lastSearchRef) but ignore results if the request was aborted (let the
thrown DOMException propagate to the catch where you can no-op for aborts).
In `@apps/trails/package.json`:
- Around line 5-11: Add the missing doctor:react npm script to the "scripts"
section of apps/trails's package.json so CI's bun check:react-doctor succeeds;
inspect other React apps (e.g., apps/guides or apps/landing) to copy their
doctor:react command and add it alongside "build", "clean", "dev", "lint", and
"start" in the scripts object so the project exposes the same doctor:react
entrypoint.
In `@apps/trails/tailwind.config.ts`:
- Line 11: The glob pattern string '*.{js,ts,jsx,tsx,mdx}' in the Tailwind
config only matches root-level files; update the pattern used in the content
array (the string literal currently shown) to '**/*.{js,ts,jsx,tsx,mdx}' so it
recursively scans subdirectories like the other patterns (or explicitly document
why the root-only pattern is intentional if you want to keep it).
In `@package.json`:
- Line 49: The trails app's package.json is missing the required doctor:react
script; add a "doctor:react" script entry to that package.json (alongside
existing scripts like "trails") with the command "bunx react-doctor" so the
check:react-doctor validation can run; ensure the scripts object contains
"doctor:react": "bunx react-doctor".
In `@packages/app/src/browser.ts`:
- Around line 3-31: safeLocalStorage and safeSessionStorage currently check
isBrowser() but still call localStorage/sessionStorage directly, which can throw
DOMExceptions in private modes; wrap each access inside getItem, setItem, and
removeItem for both objects (methods: safeLocalStorage.getItem, setItem,
removeItem and safeSessionStorage.getItem, setItem, removeItem) in a try-catch
that swallows or handles Storage SecurityError: on getItem return null on error,
on setItem/removeItem just return/no-op (optionally log the error). Ensure the
try-catch only surrounds the direct storage call so isBrowser() stays as the
early guard.
---
Outside diff comments:
In `@apps/expo/features/pack-templates/components/AddPackTemplateItemActions.tsx`:
- Around line 121-129: The handler handleCatalogItemsSelected currently awaits
addItemsToPackTemplate without error handling; wrap the call to
addItemsToPackTemplate(packTemplateId, catalogItems) in a try/catch so the
success Burnt.toast only runs on success, and in the catch show an error toast
(e.g., Burnt.toast with preset 'error' and a message from the caught error or a
translation key), log the error (console.error or existing logger), and avoid
reporting success; keep trackRecentlyUsed(catalogItems) placement as desired
(either before or after the save) and adjust if you want to roll it back on
failure.
---
Duplicate comments:
In `@apps/expo/features/auth/hooks/useAuthActions.ts`:
- Around line 70-72: Validate and parse the returned user before persisting
anything: call UserSchema.parse(data.user) first and only if it succeeds proceed
to await setToken(data.accessToken); await setRefreshToken(data.refreshToken);
and userStore.set(parsedUser). Apply this pattern in all auth flows that
currently call setToken/setRefreshToken then UserSchema.parse (the blocks around
UserSchema.parse in useAuthActions.ts, specifically the sites referenced at
lines ~70-72, ~101-103, ~148-150, and ~253-257), and on parse failure avoid
writing tokens or mutating userStore and surface/handle the error instead.
In `@apps/trails/components/DownloadCTA.tsx`:
- Around line 24-35: The sessionStorage accesses in useEffect and dismiss
(sessionStorage.getItem(DISMISSED_KEY) and sessionStorage.setItem(DISMISSED_KEY,
'1')) can throw SecurityError; wrap both calls in try/catch and "fail open"
(i.e., on error behave as if nothing was stored: proceed to compute platform and
show CTA in useEffect, and in dismiss simply hide the CTA without rethrowing).
Update the useEffect and dismiss functions in DownloadCTA.tsx to catch errors
around sessionStorage, log/debug if desired, and continue to use
setPlatform/setVisible/getStoreLinks unchanged.
---
Nitpick comments:
In `@apps/trails/components/AuthGate.tsx`:
- Around line 49-57: Replace brittle string-sniffing in the catch block with a
check for a typed error code on the thrown error (e.g., examine err.code or
err.error?.code and/or narrow err to your API error type), and treat the
specific code (for example 'EMAIL_EXISTS' or whatever your API uses) as the
trigger to show the "Account already exists" toast and call setTab('login');
otherwise fall back to the existing msg logic. Update the condition around the
err variable (in the catch where toast.error and setTab are used) to prefer
err.code comparison, handle both string and nested error shapes safely, and keep
the existing toast action behavior for the specific error code.
In `@apps/trails/components/SearchBar.tsx`:
- Around line 19-24: handleFocus currently blurs unauthenticated users which
removes focus without explanation; instead stop silently blurring in
handleFocus, call openAuthGate but keep focus on the input (or move focus to the
opened auth modal), and add accessible affordances: give the inputRef-controlled
element an aria-describedby pointing to a short helper message element (create
e.g. <span id="searchAuthHint"> that explains "Sign in to search"), ensure the
input has an aria-label or descriptive placeholder, and add a pointer fallback
by wiring an onClick handler that calls openAuthGate (and an onKeyDown handler
for Enter/Space to call openAuthGate) so keyboard and screen reader users can
discover and trigger the auth flow; reference the existing handleFocus,
inputRef, isAuthed, and openAuthGate when making these changes.
In `@apps/trails/components/TrailMap.tsx`:
- Around line 21-66: The init(container) call can reject and its errors are
currently swallowed; make the useEffect start the Leaflet initialization inside
an async IIFE or attach a .catch so the promise is awaited/handled, and wrap the
body of init (including the dynamic import('leaflet') and map creation) in
try/catch to log errors and perform cleanup; specifically update the useEffect
to call an async function that awaits init(container) (or awaits the import and
map setup inline), catch any errors and call console.error or the app logger,
and ensure mapRef.current and markersRef.current are left in a consistent state
on failure so subsequent mounts won't assume initialization succeeded.
- Around line 36-38: The marker icon URLs (iconRetinaUrl, iconUrl, shadowUrl) in
TrailMap.tsx rely on unpkg.com and should be served locally to avoid CDN/CSP
failures — copy the three Leaflet images into your app's public folder (e.g.,
/public/leaflet/) and update the icon paths used when configuring the default
icon (where iconRetinaUrl, iconUrl, shadowUrl are set or where
L.Icon.Default.mergeOptions is called) to point to the local URLs (e.g.,
/leaflet/marker-icon-2x.png, /leaflet/marker-icon.png,
/leaflet/marker-shadow.png) or import the images and reference those imports
instead so marker icons load reliably without external CDN dependency.
In `@apps/trails/components/TrailsPage.tsx`:
- Around line 24-27: MapState's 'error' variant is declared but never used;
update loadMap (the function calling getUserLocation) to catch failures and call
setMapState({ status: 'error', message: err.message || String(err) }) and update
the render branch that currently distinguishes only 'loading' vs else (the JSX
that renders the map) to explicitly handle status === 'error' (render an error
message UI using MapState.message) and keep the existing map render for status
=== 'idle'; this ensures MapState (and setMapState) are wired correctly and the
error case is shown instead of silently rendering the map.
In `@apps/trails/lib/trailSearch.ts`:
- Around line 67-68: The current silent fallback on parseApiTrails(data) can
hide schema mismatches; update the logic around parseApiTrails in trailSearch.ts
so that when parseApiTrails(data) returns null/undefined you emit a console.warn
(or processLogger.warn) including a short diagnostic message, the fact parsing
failed for ApiTrailSchema[], and relevant context (e.g., the raw data or a JSON
snippet) before falling back to an empty array; keep the existing rawTrails =
parseApiTrails(data) ?? [] pattern but add the warning branch so parsing
failures are visible in logs.
- Line 81: The current hasMore calculation uses "trails.length >= limit" which
yields false positives when the API returns exactly limit items; change the
fetching logic to request limit + 1 items, then set hasMore to "trails.length >
limit" and slice the returned trails down to the original limit before
returning; update the code paths that call/define the limit/fetch to request
limit + 1 and adjust the return in the function that returns { trails, hasMore }
(referencing the variables/functions named "limit" and "trails" in
trailSearch.ts).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 68460ba6-9c26-4328-9b79-e94584c90f1c
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (46)
CLAUDE.mdapps/admin/lib/auth.tsapps/admin/package.jsonapps/admin/tsconfig.jsonapps/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.tsapps/trails/app/globals.cssapps/trails/app/layout.tsxapps/trails/app/page.tsxapps/trails/components/AuthGate.tsxapps/trails/components/DownloadCTA.tsxapps/trails/components/SearchBar.tsxapps/trails/components/TrailCard.tsxapps/trails/components/TrailMap.tsxapps/trails/components/TrailsPage.tsxapps/trails/components/VerifyEmail.tsxapps/trails/components/ui/sonner.tsxapps/trails/lib/apiClient.tsapps/trails/lib/auth.tsapps/trails/lib/env.tsapps/trails/lib/geolocation.tsapps/trails/lib/overpass.tsapps/trails/lib/trailSearch.tsapps/trails/lib/useAuth.tsxapps/trails/next.config.mjsapps/trails/package.jsonapps/trails/postcss.config.mjsapps/trails/tailwind.config.tsapps/trails/tsconfig.jsonpackage.jsonpackages/api/src/schemas/catalog.tspackages/app/package.jsonpackages/app/src/browser.tspackages/app/src/index.tspackages/env/scripts/no-raw-process-env.tstsconfig.json
Comment on lines
+72
to
+74
| const paginatedItems = (paginatedData?.pages.flatMap((page) => page.items) ?? []).filter((item) => | ||
| Boolean(item?.id), | ||
| ); |
Contributor
There was a problem hiding this comment.
Use explicit null checks for id instead of truthiness.
Boolean(item?.id) drops items with id = 0. Use item?.id != null (or a type predicate) so valid zero IDs are retained.
Suggested fix
- const paginatedItems = (paginatedData?.pages.flatMap((page) => page.items) ?? []).filter((item) =>
- Boolean(item?.id),
- );
+ const paginatedItems = (paginatedData?.pages.flatMap((page) => page.items) ?? []).filter(
+ (item): item is CatalogItem => item?.id != null,
+ );📝 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.
Suggested change
| const paginatedItems = (paginatedData?.pages.flatMap((page) => page.items) ?? []).filter((item) => | |
| Boolean(item?.id), | |
| ); | |
| const paginatedItems = (paginatedData?.pages.flatMap((page) => page.items) ?? []).filter( | |
| (item): item is CatalogItem => item?.id != 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/expo/features/catalog/screens/CatalogItemsScreen.tsx` around lines 72 -
74, The filter currently drops falsy IDs (e.g., 0) by using Boolean(item?.id);
update the predicate used when building paginatedItems so it explicitly checks
for null/undefined instead: keep the flatMap over paginatedData?.pages to
collect page.items, then filter with item?.id != null (or a type predicate) so
valid zero IDs are retained; locate the paginatedItems definition and replace
the Boolean(item?.id) predicate accordingly.
Comment on lines
+1
to
+2
| import type { UserSchema } from '@packrat/api/schemas/users'; | ||
| import type { z } from 'zod'; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify how UserSchema is exported and whether type-only import is safe.
set -euo pipefail
echo "== Locate UserSchema declaration =="
rg -n "export\\s+(const|let|var|type|interface)\\s+UserSchema\\b|export\\s*\\{[^}]*UserSchema" -S
echo
echo "== Locate all type-only imports of UserSchema =="
rg -n "import\\s+type\\s*\\{\\s*UserSchema\\s*\\}\\s+from\\s+['\"][^'\"]+['\"]" -S
echo
echo "== Locate z.infer<typeof UserSchema> usage =="
rg -n "z\\.infer\\s*<\\s*typeof\\s+UserSchema\\s*>" -SRepository: PackRat-AI/PackRat
Length of output: 748
Change UserSchema and z to regular imports instead of type-only imports
Line 1 imports UserSchema as import type, but line 5 uses typeof UserSchema to infer the type. Since UserSchema is a const export (a runtime value), the typeof operator requires the value symbol to be available at compile-time. Type-only imports strip the symbol from compilation, breaking this reference. Similarly, z is also a runtime value and should not use import type.
Suggested fix
-import type { UserSchema } from '@packrat/api/schemas/users';
-import type { z } from 'zod';
+import { UserSchema } from '@packrat/api/schemas/users';
+import { z } from 'zod';📝 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.
Suggested change
| import type { UserSchema } from '@packrat/api/schemas/users'; | |
| import type { z } from 'zod'; | |
| import { UserSchema } from '@packrat/api/schemas/users'; | |
| import { z } from 'zod'; |
🤖 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/profile/types.ts` around lines 1 - 2, The imports for
UserSchema and z are declared as type-only but are used at runtime with typeof,
so change both "import type { UserSchema }" and "import type { z }" to regular
runtime imports (e.g., import { UserSchema } and import { z }) so the symbols
are present for typeof UserSchema and any z usage; update the import statement
at the top of types.ts to remove "type" qualifiers while keeping the same named
imports.
Comment on lines
+76
to
+87
| async function handleForgot(e: React.FormEvent) { | ||
| e.preventDefault(); | ||
| setLoading(true); | ||
| try { | ||
| await apiClient.auth['forgot-password'].post({ email: forgotEmail }); | ||
| setForgotSent(true); | ||
| } catch { | ||
| toast.error('Could not send reset email. Try again.'); | ||
| } finally { | ||
| setLoading(false); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
handleForgot ignores API error response — forgotSent set on any non-thrown response.
Every other apiClient call in this file and in useAuth.tsx destructures { data, error, status } and checks if (error). Here the result is not checked at all, so if the Treaty client returns an error in the result object rather than throwing, setForgotSent(true) fires even on a failed request, showing a false "check your inbox" message.
🐛 Proposed fix
async function handleForgot(e: React.FormEvent) {
e.preventDefault();
setLoading(true);
try {
- await apiClient.auth['forgot-password'].post({ email: forgotEmail });
- setForgotSent(true);
+ const { error, status } = await apiClient.auth['forgot-password'].post({ email: forgotEmail });
+ if (error) {
+ toast.error('Could not send reset email. Try again.');
+ } else {
+ setForgotSent(true);
+ }
} catch {
toast.error('Could not send reset email. Try again.');
} finally {
setLoading(false);
}
}📝 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.
Suggested change
| async function handleForgot(e: React.FormEvent) { | |
| e.preventDefault(); | |
| setLoading(true); | |
| try { | |
| await apiClient.auth['forgot-password'].post({ email: forgotEmail }); | |
| setForgotSent(true); | |
| } catch { | |
| toast.error('Could not send reset email. Try again.'); | |
| } finally { | |
| setLoading(false); | |
| } | |
| } | |
| async function handleForgot(e: React.FormEvent) { | |
| e.preventDefault(); | |
| setLoading(true); | |
| try { | |
| const { error, status } = await apiClient.auth['forgot-password'].post({ email: forgotEmail }); | |
| if (error) { | |
| toast.error('Could not send reset email. Try again.'); | |
| } else { | |
| setForgotSent(true); | |
| } | |
| } catch { | |
| toast.error('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 - 87, handleForgot
currently ignores the Treaty client response and always calls setForgotSent even
when the API returned an error object; update handleForgot to await and
destructure the response from apiClient.auth['forgot-password'].post into {
data, error, status } (or the client’s response shape), then only call
setForgotSent(true) when error is falsy and status indicates success; if error
exists, call toast.error with the error message (or a fallback) instead of
setting forgotSent. Ensure you keep the existing try/catch/finally behavior for
thrown exceptions but add the response-level error check before setting state.
Comment on lines
+76
to
+99
| const runSearch = useCallback( | ||
| async (params: TrailSearchParams, append = false) => { | ||
| setSearchLoading(true); | ||
| try { | ||
| const { trails, hasMore: more } = await searchTrails(params); | ||
| setSearchTrailResults((prev) => (append && prev ? [...prev, ...trails] : trails)); | ||
| setHasMore(more); | ||
| setOffset((params.offset ?? 0) + trails.length); | ||
| lastSearchRef.current = params; | ||
| } catch (err) { | ||
| if (err instanceof AuthExpiredError) { | ||
| toast.error('Session expired. Please log in again.'); | ||
| openAuthGate(); | ||
| } else if (err instanceof Error && err.message.includes('429')) { | ||
| toast.error('Too many requests. Please wait a moment.'); | ||
| } else { | ||
| toast.error('Search failed. Please try again.'); | ||
| } | ||
| } finally { | ||
| setSearchLoading(false); | ||
| } | ||
| }, | ||
| [openAuthGate], | ||
| ); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "trailSearch.ts" 2>/dev/nullRepository: PackRat-AI/PackRat
Length of output: 95
🏁 Script executed:
cat -n ./apps/trails/lib/trailSearch.tsRepository: PackRat-AI/PackRat
Length of output: 3086
🏁 Script executed:
find . -type f -name "apiClient.ts" | head -5Repository: PackRat-AI/PackRat
Length of output: 93
🏁 Script executed:
cat -n ./apps/trails/lib/apiClient.tsRepository: PackRat-AI/PackRat
Length of output: 1406
🏁 Script executed:
find . -type f -name "*.ts" -o -name "*.tsx" | xargs grep -l "createApiClient" | head -10Repository: PackRat-AI/PackRat
Length of output: 159
🏁 Script executed:
cat -n ./packages/api-client/src/index.tsRepository: PackRat-AI/PackRat
Length of output: 11786
🏁 Script executed:
grep -r "AbortSignal\|AbortController\|signal" ./packages/api-client --include="*.ts" --include="*.tsx"Repository: PackRat-AI/PackRat
Length of output: 167
🏁 Script executed:
find . -type f -name "package.json" -path "*/api-client/*" -exec cat {} \;Repository: PackRat-AI/PackRat
Length of output: 579
🏁 Script executed:
rg "treaty" ./apps/trails --type ts --type tsx -A 3 -B 1 | head -50Repository: PackRat-AI/PackRat
Length of output: 90
🏁 Script executed:
rg "treaty" ./apps/trails -A 3 -B 1 | head -50Repository: PackRat-AI/PackRat
Length of output: 44
🏁 Script executed:
web_search
What is `@elysiajs/eden` treaty client and how to use AbortSignal with itRepository: PackRat-AI/PackRat
Length of output: 156
🌐 Web query:
@elysiajs/eden treaty client AbortSignal RequestInit support
💡 Result:
Yes, the @elysiajs/eden treaty client supports AbortSignal via RequestInit. Eden Treaty is a fetch wrapper that accepts fetch options matching Fetch API's RequestInit, including signal for AbortController [1][2][3]. Example from official docs [1][3]: const controller = new AbortController() const cancelRequest = setTimeout(() => { controller.abort() }, 5000) await api.hello.get({ fetch: { signal: controller.signal } }) This aborts the request after 5 seconds. It works for regular requests and SSE/streams [4]. Config also supports default fetch: RequestInit options [2].
Citations:
- 1: https://elysiajs.com/eden/treaty/parameters.html
- 2: https://elysiajs.com/eden/treaty/config
- 3: https://elysiajs.com/eden/treaty/parameters
- 4: Eden: ability to cancel a SSE request elysiajs/elysia#1519
🏁 Script executed:
cat -n ./apps/trails/components/TrailsPage.tsx | sed -n '76,99p'Repository: PackRat-AI/PackRat
Length of output: 1125
Race condition: concurrent searchTrails calls can resolve out of order
runSearch has no cancellation mechanism. If the user submits a new search while a previous one is still in-flight, both calls remain active. Whichever resolves last wins and overwrites searchTrailResults, regardless of which query was submitted most recently. This can silently display results for a stale query.
A ref-based abort approach fixes this. Note that @elysiajs/eden treaty client supports AbortSignal via the second parameter's fetch option:
🐛 Proposed fix
+ const searchAbortRef = useRef<AbortController | null>(null);
+
const runSearch = useCallback(
async (params: TrailSearchParams, append = false) => {
+ if (!append) {
+ searchAbortRef.current?.abort();
+ searchAbortRef.current = new AbortController();
+ }
setSearchLoading(true);
try {
- const { trails, hasMore: more } = await searchTrails(params);
+ const { trails, hasMore: more } = await searchTrails(params, searchAbortRef.current?.signal);
+ if (searchAbortRef.current?.signal.aborted) return;
setSearchTrailResults((prev) => (append && prev ? [...prev, ...trails] : trails));
setHasMore(more);
setOffset((params.offset ?? 0) + trails.length);
lastSearchRef.current = params;
} catch (err) {
+ if (err instanceof Error && err.name === 'AbortError') return;
if (err instanceof AuthExpiredError) {Update searchTrails to accept and forward the signal:
-export async function searchTrails(params: TrailSearchParams): Promise<TrailSearchResult> {
+export async function searchTrails(params: TrailSearchParams, signal?: AbortSignal): Promise<TrailSearchResult> {
const limit = params.limit ?? 20;
const { data, error, status } = await apiClient.trails.search.get(
{
q: params.q,
lat: params.lat,
lon: params.lon,
radius: params.radius,
sport: params.sport,
limit,
offset: params.offset ?? 0,
},
+ signal ? { fetch: { signal } } : undefined
);🤖 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 76 - 99, runSearch can
return stale results because concurrent searchTrails requests are not
cancellable; add an AbortController per runSearch invocation, store it in a ref
(e.g., lastControllerRef), and before starting a new search abort any existing
controller; pass the controller.signal into searchTrails via its second param
fetch option (searchTrails(params, { fetch: { signal } })), then set
lastControllerRef.current = controller and ensure you abort/cleanup on component
unmount; update runSearch to create/abort the controller, and keep existing
logic (setSearchLoading, setSearchTrailResults, setOffset, lastSearchRef) but
ignore results if the request was aborted (let the thrown DOMException propagate
to the catch where you can no-op for aborts).
Comment on lines
+85
to
+93
| } catch (err) { | ||
| if (err instanceof AuthExpiredError) { | ||
| toast.error('Session expired. Please log in again.'); | ||
| openAuthGate(); | ||
| } else if (err instanceof Error && err.message.includes('429')) { | ||
| toast.error('Too many requests. Please wait a moment.'); | ||
| } else { | ||
| toast.error('Search failed. Please try again.'); | ||
| } |
Contributor
There was a problem hiding this comment.
Stale results remain visible when a new search fails
When append = false and runSearch throws, setSearchTrailResults is never called, so the previous search results stay on screen with no indication they belong to a different (now-abandoned) query. The toast appears, but the list is misleading.
🐛 Proposed fix
const runSearch = useCallback(
async (params: TrailSearchParams, append = false) => {
+ if (!append) setSearchTrailResults(null);
setSearchLoading(true);
try {🤖 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 85 - 93, When runSearch
throws and append === false you never clear the previous results, so call
setSearchTrailResults([]) (or otherwise reset the results state used by
TrailsPage) inside the catch branch for non-auth/429 errors (and/or for all
errors when append is false) so the UI doesn't show stale results; update the
catch block that currently checks err instanceof AuthExpiredError /
err.message.includes('429') to also clear results via setSearchTrailResults([])
when append === false and ensure any loading flags (e.g., isSearching) are set
consistently.
Comment on lines
+5
to
+11
| "scripts": { | ||
| "build": "next build", | ||
| "clean": "bunx rimraf node_modules .next out", | ||
| "dev": "next dev", | ||
| "lint": "next lint", | ||
| "start": "next start" | ||
| }, |
Contributor
There was a problem hiding this comment.
Add the required doctor:react script to fix the CI pipeline failure.
The pipeline requires a doctor:react script in every React app's package.json. Without it, bun check:react-doctor fails.
🛠️ Proposed fix
"scripts": {
"build": "next build",
"clean": "bunx rimraf node_modules .next out",
"dev": "next dev",
+ "doctor:react": "<react-doctor-command>",
"lint": "next lint",
"start": "next start"
},Check what command the other React apps use for doctor:react (e.g., apps/guides or apps/landing) and mirror it here.
🤖 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/package.json` around lines 5 - 11, Add the missing doctor:react
npm script to the "scripts" section of apps/trails's package.json so CI's bun
check:react-doctor succeeds; inspect other React apps (e.g., apps/guides or
apps/landing) to copy their doctor:react command and add it alongside "build",
"clean", "dev", "lint", and "start" in the scripts object so the project exposes
the same doctor:react entrypoint.
| './components/**/*.{ts,tsx}', | ||
| './app/**/*.{ts,tsx}', | ||
| './src/**/*.{ts,tsx}', | ||
| '*.{js,ts,jsx,tsx,mdx}', |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if there are any .mdx or .js files in subdirectories
# that should be scanned by Tailwind
fd -e mdx -e js -e jsx . apps/trails --exclude node_modules --exclude .next --exclude distRepository: PackRat-AI/PackRat
Length of output: 44
🏁 Script executed:
cat -n apps/trails/tailwind.config.tsRepository: PackRat-AI/PackRat
Length of output: 565
Pattern on line 11 matches only root-level files, not subdirectories.
The glob pattern *.{js,ts,jsx,tsx,mdx} matches only files in the root directory of the apps/trails app, whereas the other patterns in the content array (e.g., ./pages/**/*.{ts,tsx}) use recursive matching. If the intent is to scan all MDX and JavaScript files recursively across subdirectories, this should be **/*.{js,ts,jsx,tsx,mdx}. However, since no .js, .jsx, or .mdx files currently exist in the directory tree, this pattern has no practical effect. If it is intentionally limited to root-level config files, the current pattern is acceptable but may conflict with expectations set by the other patterns.
🤖 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/tailwind.config.ts` at line 11, The glob pattern string
'*.{js,ts,jsx,tsx,mdx}' in the Tailwind config only matches root-level files;
update the pattern used in the content array (the string literal currently
shown) to '**/*.{js,ts,jsx,tsx,mdx}' so it recursively scans subdirectories like
the other patterns (or explicitly document why the root-only pattern is
intentional if you want to keep it).
| "test:expo:rpc-types": "vitest run --config apps/expo/vitest.types.config.ts", | ||
| "test:mcp": "bun run --cwd packages/mcp test" | ||
| "test:mcp": "bun run --cwd packages/mcp test", | ||
| "trails": "bun run --cwd apps/trails dev" |
Contributor
There was a problem hiding this comment.
Add missing doctor:react script to apps/trails/package.json.
The pipeline failure indicates that the trails app is missing the required doctor:react script in its package.json. All React apps in the monorepo must include this script for the check:react-doctor validation to pass.
📋 Required addition to apps/trails/package.json
Add the following script to apps/trails/package.json:
"scripts": {
"doctor:react": "bunx react-doctor"
}🤖 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 `@package.json` at line 49, The trails app's package.json is missing the
required doctor:react script; add a "doctor:react" script entry to that
package.json (alongside existing scripts like "trails") with the command "bunx
react-doctor" so the check:react-doctor validation can run; ensure the scripts
object contains "doctor:react": "bunx react-doctor".
Comment on lines
+3
to
+31
| export const safeLocalStorage = { | ||
| getItem(key: string): string | null { | ||
| if (!isBrowser()) return null; | ||
| return localStorage.getItem(key); | ||
| }, | ||
| setItem(key: string, value: string): void { | ||
| if (!isBrowser()) return; | ||
| localStorage.setItem(key, value); | ||
| }, | ||
| removeItem(key: string): void { | ||
| if (!isBrowser()) return; | ||
| localStorage.removeItem(key); | ||
| }, | ||
| }; | ||
|
|
||
| export const safeSessionStorage = { | ||
| getItem(key: string): string | null { | ||
| if (!isBrowser()) return null; | ||
| return sessionStorage.getItem(key); | ||
| }, | ||
| setItem(key: string, value: string): void { | ||
| if (!isBrowser()) return; | ||
| sessionStorage.setItem(key, value); | ||
| }, | ||
| removeItem(key: string): void { | ||
| if (!isBrowser()) return; | ||
| sessionStorage.removeItem(key); | ||
| }, | ||
| }; |
Contributor
There was a problem hiding this comment.
Storage access can throw a SecurityError even when window is defined.
In Firefox private mode, restricted iframe sandboxes, and some privacy-hardened browser configurations, localStorage/sessionStorage access throws a DOMException despite isBrowser() being true. Without a try-catch, that exception propagates to every call site (auth flows, token persistence), potentially crashing the app for privacy-conscious users.
🛡️ Proposed fix — wrap all storage access in try-catch
export const safeLocalStorage = {
getItem(key: string): string | null {
if (!isBrowser()) return null;
- return localStorage.getItem(key);
+ try { return localStorage.getItem(key); } catch { return null; }
},
setItem(key: string, value: string): void {
if (!isBrowser()) return;
- localStorage.setItem(key, value);
+ try { localStorage.setItem(key, value); } catch { /* silent */ }
},
removeItem(key: string): void {
if (!isBrowser()) return;
- localStorage.removeItem(key);
+ try { localStorage.removeItem(key); } catch { /* silent */ }
},
};
export const safeSessionStorage = {
getItem(key: string): string | null {
if (!isBrowser()) return null;
- return sessionStorage.getItem(key);
+ try { return sessionStorage.getItem(key); } catch { return null; }
},
setItem(key: string, value: string): void {
if (!isBrowser()) return;
- sessionStorage.setItem(key, value);
+ try { sessionStorage.setItem(key, value); } catch { /* silent */ }
},
removeItem(key: string): void {
if (!isBrowser()) return;
- sessionStorage.removeItem(key);
+ try { sessionStorage.removeItem(key); } catch { /* silent */ }
},
};🤖 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/app/src/browser.ts` around lines 3 - 31, safeLocalStorage and
safeSessionStorage currently check isBrowser() but still call
localStorage/sessionStorage directly, which can throw DOMExceptions in private
modes; wrap each access inside getItem, setItem, and removeItem for both objects
(methods: safeLocalStorage.getItem, setItem, removeItem and
safeSessionStorage.getItem, setItem, removeItem) in a try-catch that swallows or
handles Storage SecurityError: on getItem return null on error, on
setItem/removeItem just return/no-op (optionally log the error). Ensure the
try-catch only surrounds the direct storage call so isBrowser() stays as the
early guard.
Merged
andrew-bierman
added a commit
that referenced
this pull request
May 14, 2026
…tend feat(trails): trail search micro frontend acquisition surface
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
apps/trails— standalone Next.js + Cloudflare Worker app attrails.packratai.com, a hyper-focused acquisition surface for PackRatTesting
Post-Deploy Monitoring & Validation
Summary by CodeRabbit
New Features
Refactoring
Documentation