fix(admin): CF Access identity schema — remove jwt field, drop manual JWT forwarding#2343
Merged
Merged
Conversation
github-actions
Bot
added
dependencies
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR introduces a comprehensive refactoring and feature expansion: it adds the Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Expo/Web Client
participant AdminApp as Admin App
participant AdminAPI as Admin API
participant CFAccess as Cloudflare Access
rect rgba(100, 150, 200, 0.5)
Note over Client,CFAccess: Before: Client-side JWT extraction
Client->>AdminAPI: Request with JWT assertion header
AdminAPI->>AdminAPI: Validate JWT
AdminAPI-->>Client: Response
end
rect rgba(150, 200, 100, 0.5)
Note over AdminApp,CFAccess: After: Server-side handling only
AdminApp->>AdminAPI: Request with Bearer token<br/>(from adminEnv.NEXT_PUBLIC_API_URL)
AdminAPI->>CFAccess: Auto-injected assertion header
CFAccess-->>AdminAPI: Auth validation
AdminAPI-->>AdminApp: Response
end
sequenceDiagram
participant Client as MCP/External Client
participant TrailsAPI as GET /trails/search API
participant QueryBuilder as TrailQueryBuilder
participant Overpass as Overpass Service
participant Transform as Transform Utils
Client->>TrailsAPI: Search request (q, lat/lon, sport, limit)
TrailsAPI->>QueryBuilder: Build query
QueryBuilder->>QueryBuilder: Apply filters (sport, name, bbox/around)
QueryBuilder-->>TrailsAPI: QL string
TrailsAPI->>Overpass: POST query
Overpass-->>TrailsAPI: JSON response (elements with geometry)
TrailsAPI->>Transform: Convert to TrailSummary/Detail
Transform->>Transform: Extract tags, bounds, geometry
Transform-->>TrailsAPI: TrailDetail[]
TrailsAPI-->>Client: Paginated results
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Coverage Report for API Unit Tests Coverage (./packages/api)
File CoverageNo changed files found. |
Contributor
Coverage Report for Expo Unit Tests Coverage (./apps/expo)
File CoverageNo changed files found. |
Contributor
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
packrat-admin | bc5112b | Commit Preview URL Branch Preview URL |
Apr 27 2026, 01:57 AM |
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes an admin authentication redirect loop by aligning the Cloudflare Access identity schema with the actual /cdn-cgi/access/get-identity response and removing manual CF Access JWT forwarding in the admin API client. In addition, this PR broadens scope substantially by introducing new repo-wide lint/check tooling, consolidating guard helpers, and adding a new Overpass-powered Trails feature (new package + API routes + MCP tools).
Changes:
- Admin: remove nonexistent
jwtfield from CF Access identity schema; stop attempting to forward CF Access JWT manually; introduce a typed env shim for the admin app. - Tooling: add new custom lint/check scripts (unauthenticated route detection, duplicate guard detection, stricter “top-level regex” enforcement) and wire them into hooks/CI.
- Trails: add
@packrat/overpasspackage plus new authenticated/trails/*API routes and MCP tool endpoints.
Reviewed changes
Copilot reviewed 145 out of 146 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/lint/no-unauth-routes.ts | New lint to detect Elysia routes missing auth macros / public-route annotation |
| scripts/lint/no-raw-typeof.ts | Exempt global typeof checks (SSR/env guards) and improve capture logic |
| scripts/lint/no-raw-regex.ts | Expand raw-regex allowlist (exclude additional file) |
| scripts/lint/no-duplicate-guards.ts | New lint to prevent re-implementing shared guards outside @packrat/guards |
| scripts/check-all.ts | Add new checks (no-raw-process-env, no-duplicate-guards, no-unauth-routes, strict casts) |
| packages/web-ui/src/lib/utils.ts | Replace local assertDefined with re-export from @packrat/guards |
| packages/web-ui/src/components/sidebar.tsx | Replace raw typeof checks with guard helpers |
| packages/web-ui/src/components/form.tsx | Add // safe-cast: annotations for context sentinels |
| packages/web-ui/src/components/chart.tsx | Hoist regex; replace raw typeof with guard helpers |
| packages/overpass/vitest.config.ts | Add Vitest config for new @packrat/overpass package |
| packages/overpass/tsconfig.json | Add TS config for new @packrat/overpass package |
| packages/overpass/src/types.ts | Add Overpass response TypeScript types |
| packages/overpass/src/transform.ts | Add Overpass → trail summary/detail/geometry transformers |
| packages/overpass/src/transform.test.ts | Unit tests for Overpass transformers |
| packages/overpass/src/schemas.ts | Add Zod schemas for Overpass response parsing |
| packages/overpass/src/index.ts | Export Overpass client + builder + transformers/types |
| packages/overpass/src/client.ts | Implement Overpass API client with schema validation |
| packages/overpass/src/builder.ts | Implement Overpass QL builder for trails queries |
| packages/overpass/src/builder.test.ts | Unit tests for Overpass query builder |
| packages/overpass/package.json | New workspace package definition |
| packages/mcp/src/tools/trips.ts | Hoist regex; minor ID formatting change |
| packages/mcp/src/tools/trails.ts | Add MCP tools for trail search/detail/geometry + AllTrails preview tool usage |
| packages/mcp/src/tools/trail-conditions.ts | Hoist regex; minor ID formatting change |
| packages/mcp/src/tools/packs.ts | Hoist regex; minor ID formatting change |
| packages/guards/src/narrow.ts | Replace comment with // safe-cast: annotation |
| packages/env/scripts/no-raw-process-env.ts | Expand allowlist for intentional process.env usage |
| packages/config/src/config.ts | Use isObject guard and add // safe-cast: rationale |
| packages/config/package.json | Add @packrat/guards dependency |
| packages/cli/src/shared.ts | Use isNumber; add // safe-cast: rationale on record casts |
| packages/cli/src/args.ts | Use isString guard in preprocessors |
| packages/cli/package.json | Add @packrat/guards dependency |
| packages/checks/src/check-type-casts.ts | Expand exclusions; add // safe-cast: annotation support |
| packages/checks/src/check-magic-strings.ts | Rewrite to cross-file detection + more ignore heuristics |
| packages/api/test/setup.ts | Hoist regex constant in test mock |
| packages/api/src/utils/format-ai-response.ts | Hoist regex constants |
| packages/api/src/utils/env-validation.ts | Use isObject; add // safe-cast: rationales for Worker bindings |
| packages/api/src/utils/csv-utils.ts | Hoist regex constants; use isString; add // safe-cast: notes |
| packages/api/src/utils/compute-pack.ts | Remove WeightUnit cast by using typed field directly |
| packages/api/src/utils/auth.ts | Use isNumber; add // safe-cast: rationale for payload cast |
| packages/api/src/services/refreshTokenService.ts | Add // safe-cast: rationale for Drizzle SQL casts |
| packages/api/src/services/r2-bucket.ts | Use guard helpers; hoist regex; add // safe-cast: rationales |
| packages/api/src/services/executeSqlAiTool.ts | Hoist regex constant |
| packages/api/src/services/etl/processValidItemsBatch.ts | Add // safe-cast: rationale for validated items cast |
| packages/api/src/services/etl/CatalogItemValidator.ts | Use isString/isNumber guards |
| packages/api/src/services/embeddingService.ts | Hoist newline regex constant |
| packages/api/src/services/aiService.ts | Use isFunction guard for runtime binding method check |
| packages/api/src/routes/wildlife/index.ts | Hoist slugify regex patterns |
| packages/api/src/routes/weather.ts | Use isString; add // safe-cast: rationales for API responses |
| packages/api/src/routes/trails/index.ts | Add new authenticated /trails routes backed by Overpass |
| packages/api/src/routes/trailConditions/reports.ts | Hoist LIKE-escape regex patterns |
| packages/api/src/routes/packs/index.ts | Add // safe-cast: rationale on NewPackItem cast |
| packages/api/src/routes/packTemplates/index.ts | Hoist UUID hyphen regex; minor ID generation change |
| packages/api/src/routes/knowledgeBase/reader.ts | Hoist HTML→Markdown regex patterns |
| packages/api/src/routes/index.ts | Register new trailsRoutes |
| packages/api/src/routes/catalog/index.ts | Use isString guard for query param handling |
| packages/api/src/routes/auth/index.ts | Add // public-route: annotations for intentionally public auth endpoints |
| packages/api/src/routes/alltrails.ts | Add redirect SSRF guard + mark endpoint authenticated |
| packages/api/src/routes/ai/index.ts | Use isString guard when validating schema info response |
| packages/api/src/middleware/auth.ts | Add // safe-cast: rationale for AuthUser shaping |
| packages/api/src/index.ts | Add // safe-cast: rationales for Worker entrypoint casts |
| packages/api/src/containers/AppContainer.ts | Add // safe-cast: rationale for module-level Workers env cast |
| packages/api/package.json | Add dependency on new @packrat/overpass workspace package |
| packages/api-client/src/index.ts | Use guard helpers; add // safe-cast: rationale on error parsing + generic boundary |
| packages/api-client/package.json | Add @packrat/guards dependency |
| packages/analytics/src/core/type-assertions.ts | Replace local assertDefined with re-export |
| packages/analytics/src/core/spec-parser.ts | Use isString; add // safe-cast: rationale for DuckDB row casts |
| packages/analytics/src/core/local-cache.ts | Add // safe-cast: rationale for generic boundary cast |
| packages/analytics/src/core/entity-resolver.ts | Hoist regex patterns; use isString; add cast rationales |
| packages/analytics/src/core/enrichment.ts | Add // safe-cast: rationale for DuckDB row casts |
| packages/analytics/src/core/data-export.ts | Hoist timestamp regex constant |
| packages/analytics/package.json | Add @packrat/guards dependency |
| package.json | Add check:all; expand lint:custom to run new scripts |
| lefthook.yml | Add pre-push “clean-checks” pipeline including new scripts |
| bun.lock | Update lockfile for new workspace packages/deps and bumps |
| biome.json | Upgrade useTopLevelRegex from warn → error |
| apps/landing/package.json | Add @packrat/guards dependency |
| apps/landing/lib/typeAssertions.ts | Replace local assertDefined with re-export |
| apps/landing/lib/icons.tsx | Use isFunction; add // safe-cast: notes for dynamic icon lookup |
| apps/guides/scripts/sync-to-r2.ts | Add // safe-cast: rationale for env shape cast |
| apps/guides/scripts/enhance-content.ts | Hoist timestamp regex; replace RegExp filter with includes() |
| apps/guides/package.json | Add @packrat/guards dependency |
| apps/guides/lib/assertDefined.ts | Replace local assertDefined with re-export |
| apps/guides/components/search.tsx | Add // safe-cast: note for DOM target cast |
| apps/guides/app/dev/generate/page.tsx | Add assertEnum; improve typing of category entries |
| apps/expo/utils/format-ai-response.ts | Hoist regex constants |
| apps/expo/lib/utils/domain-specific-extensions.ts | Use isFunction guard |
| apps/expo/lib/utils/dateUtils.ts | Use isString guard |
| apps/expo/lib/store.ts | Add // safe-cast: note for Legend-State proxy typing |
| apps/expo/features/wildlife/hooks/useWildlifeIdentification.ts | Use isObject; add // safe-cast: note |
| apps/expo/features/weather/screens/LocationPreviewScreen.tsx | Add // safe-cast: note for formatted data cast |
| apps/expo/features/weather/hooks/useWeatherAlert.ts | Add // safe-cast: note for subset cast |
| apps/expo/features/weather/hooks/useLocationSearch.ts | Add // safe-cast: note for formatted data cast |
| apps/expo/features/weather/hooks/useLocationRefresh.ts | Add // safe-cast: notes for partial update casts |
| apps/expo/features/trips/screens/TripDetailScreen.tsx | Add // safe-cast: note for store hydration timing |
| apps/expo/features/trips/components/TripForm.tsx | Use isString; add cast rationale for defaultValues |
| apps/expo/features/trail-conditions/hooks/useTrailConditionReports.ts | Add // safe-cast: notes on JSON/treaty/store casts |
| apps/expo/features/trail-conditions/components/TrailConditionReportCard.tsx | Remove unnecessary enum casts for label lookups |
| apps/expo/features/packs/utils/computePackWeights.ts | Remove WeightUnit cast by using typed field directly |
| apps/expo/features/packs/store/packs.ts | Add // safe-cast: notes for Zod→local type divergence |
| apps/expo/features/packs/store/packItems.ts | Add // safe-cast: notes for Zod→local type divergence |
| apps/expo/features/packs/screens/PackDetailScreen.tsx | Improve cast comment clarity |
| apps/expo/features/packs/screens/ItemsScanScreen.tsx | Add // safe-cast: note for router params typing |
| apps/expo/features/packs/hooks/useSeasonSuggestions.ts | Add // safe-cast: note for treaty response |
| apps/expo/features/packs/hooks/usePackItemDetailsFromApi.ts | Add // safe-cast: note for Zod/type divergence |
| apps/expo/features/packs/hooks/usePackGapAnalysis.ts | Add // safe-cast: note for treaty response |
| apps/expo/features/packs/hooks/useImageDetection.ts | Add // safe-cast: note for treaty response |
| apps/expo/features/packs/hooks/useDuplicatePack.ts | Add // safe-cast: note for treaty response |
| apps/expo/features/packs/components/SimilarItemsForPackItem.tsx | Add // safe-cast: note for treaty items typing |
| apps/expo/features/packs/components/PackCard.tsx | Clarify cast rationale for owned/non-owned pack shapes |
| apps/expo/features/packs/components/AddPackItemActions.tsx | Use isFunction guard for forwarded ref checks |
| apps/expo/features/pack-templates/store/packTemplates.ts | Add // safe-cast: notes for Zod→local type divergence |
| apps/expo/features/pack-templates/store/packTemplateItems.ts | Add // safe-cast: notes for Zod→local type divergence |
| apps/expo/features/pack-templates/screens/ItemsScanScreen.tsx | Add // safe-cast: note for router params typing |
| apps/expo/features/pack-templates/hooks/usePackTemplateSummary.ts | Remove WeightUnit casts by using typed field directly |
| apps/expo/features/pack-templates/hooks/useGenerateTemplateFromOnlineContent.ts | Use isObject; add // safe-cast: notes for treaty error/response |
| apps/expo/features/pack-templates/components/AddPackTemplateItemActions.tsx | Use isFunction guard for forwarded ref checks |
| apps/expo/features/catalog/screens/CatalogItemsScreen.tsx | Add // safe-cast: notes for treaty response typing |
| apps/expo/features/catalog/hooks/useSimilarItems.ts | Add // safe-cast: note for treaty response |
| apps/expo/features/catalog/components/SimilarItems.tsx | Remove redundant cast on data.items |
| apps/expo/features/catalog/components/CatalogBrowserModal.tsx | Add // safe-cast: notes for treaty response typing |
| apps/expo/features/auth/hooks/useAuthActions.ts | Use isObject; add // safe-cast: notes for router + user casts |
| apps/expo/features/ai/lib/localModelManager.ts | Use isString for platform version parsing |
| apps/expo/features/ai/lib/CustomChatTransport.ts | Use isString guard for error normalization |
| apps/expo/features/ai/hooks/useReportedContent.ts | Add // safe-cast: note for treaty response |
| apps/expo/features/ai/hooks/useReportContent.ts | Add // safe-cast: note for treaty response |
| apps/expo/features/ai/components/ToolInvocationRenderer.tsx | Add // safe-cast: notes for discriminant-based casts |
| apps/expo/features/ai/components/ChatBubble.tsx | Add // safe-cast: note for ToolUIPart prefix narrowing |
| apps/expo/features/ai/components/AIModeSheet.tsx | Use isFunction guard for forwarded ref checks |
| apps/expo/features/ai/atoms/chatStorageAtoms.ts | Use isObject/isString in runtime message validation |
| apps/expo/features/ai-packs/hooks/useGeneratedPacks.ts | Add // safe-cast: note for treaty response |
| apps/expo/components/initial/WeightBadge.tsx | Use isString guard for unit normalization |
| apps/expo/components/Icon/get-icon-names.ts | Add // safe-cast: notes for icon-name union limitations |
| apps/expo/components/Icon/Icon.tsx | Add // safe-cast: notes for icon-name union limitations |
| apps/expo/components/Icon/Icon.ios.tsx | Add // safe-cast: notes for icon-name union limitations |
| apps/expo/atoms/atomWithSecureStorage.ts | Use isFunction guard for functional updates |
| apps/expo/atoms/atomWithKvStorage.ts | Use isFunction guard for functional updates |
| apps/expo/atoms/atomWithAsyncStorage.ts | Use isFunction guard for functional updates |
| apps/expo/app/(app)/messages/chat.tsx | Use isString guard for mixed list rendering |
| apps/expo/app/(app)/messages/chat.android.tsx | Use isString guard for mixed list rendering |
| apps/expo/app/(app)/current-pack/[id].tsx | Add // safe-cast: note for PackItem shape divergence |
| apps/expo/app/(app)/(tabs)/profile/index.tsx | Use isString guard for section/item rendering |
| apps/admin/lib/env.ts | New admin env shim using Zod parsing at module load |
| apps/admin/lib/cfAccess.ts | Fix CF Access identity schema; remove jwt field + JWT helper |
| apps/admin/lib/api.ts | Remove CF Access JWT forwarding; use typed admin env shim |
| apps/admin/app/login/page.tsx | Use typed env shim instead of direct process.env access |
| .github/workflows/checks.yml | Run custom lint + unsafe cast checks in CI |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const ROOT = join(import.meta.dir, '..', '..'); | ||
| const ROUTES_ROOT = join(ROOT, 'packages', 'api', 'src', 'routes'); | ||
|
|
||
| const EXCLUDED_DIRS = new Set(['node_modules', 'dist', 'build', '__tests__', 'admin']); |
There was a problem hiding this comment.
EXCLUDED_DIRS currently excludes admin, but packages/api/src/routes/admin/ exists. This means the no-unauth-routes check will skip admin API routes entirely, which can hide accidentally-public admin endpoints. Remove admin from the excluded set (or replace it with a more targeted exclusion) so admin routes are scanned like the rest of the API.
Comment on lines
+54
to
+55
| - name: Check for unauthenticated routes | ||
| run: bun scripts/lint/no-unauth-routes.ts |
There was a problem hiding this comment.
CI runs bun lint:custom and then separately runs bun scripts/lint/no-unauth-routes.ts, but lint:custom already includes no-unauth-routes. This duplicates work on every PR; consider removing the standalone step (or removing it from lint:custom) to keep the workflow lean.
Suggested change
| - name: Check for unauthenticated routes | |
| run: bun scripts/lint/no-unauth-routes.ts |
Comment on lines
123
to
135
| function shouldSkipLine(line: string): boolean { | ||
| const trimmed = line.trimStart(); | ||
| return ( | ||
| trimmed.startsWith('import ') || | ||
| trimmed.startsWith('export ') || | ||
| trimmed.startsWith('//') || | ||
| trimmed.startsWith('*') || | ||
| trimmed.startsWith('export {') || | ||
| trimmed.startsWith('export type {') || | ||
| trimmed.startsWith('// ') || | ||
| trimmed.startsWith('//\t') || | ||
| trimmed === '//' || | ||
| trimmed.startsWith('* ') || | ||
| trimmed === '*' || | ||
| trimmed.startsWith('/*') | ||
| ); |
There was a problem hiding this comment.
The header comment says literals on "import/export/comment lines" are excluded, but shouldSkipLine() only skips export { / export type { (not export const, export function, etc.). This makes the behavior diverge from the script’s documented intent and can introduce noisy false positives from exported constants. Either expand the export detection (e.g. any export ) or update the comment to match the narrower rule.
Comment on lines
+53
to
+63
| build(): string { | ||
| const preamble = `[out:json][timeout:${this._timeout}];`; | ||
|
|
||
| if (this._byId !== null) { | ||
| return `${preamble}\nrelation(${this._byId});\nout geom;`; | ||
| } | ||
|
|
||
| const typeFilter = '["type"="route"]'; | ||
| const filters = [typeFilter, ...this._filters].join(''); | ||
| return `${preamble}\nrelation${filters}${this._spatial};\nout geom;`; | ||
| } |
There was a problem hiding this comment.
TrailQueryBuilder.build() always emits out geom;, even for endpoints that only return summary metadata. This guarantees larger Overpass responses than necessary (geometry can be very large), increasing latency and Overpass load. Consider supporting a lighter output mode (e.g. out tags bb; for summary/search) and reserving out geom; for the geometry endpoint.
Comment on lines
+1
to
+5
| import { authPlugin } from '@packrat/api/middleware/auth'; | ||
| import { queryOverpass, TrailQueryBuilder, toTrailDetail, toTrailSummary } from '@packrat/overpass'; | ||
| import { Elysia, status } from 'elysia'; | ||
| import { z } from 'zod'; | ||
|
|
There was a problem hiding this comment.
This PR is titled/described as a CF Access admin fix, but it also introduces a new Overpass client package and new /trails/* API routes (plus multiple lint/check changes). This makes the change set much broader than the stated scope; please either split the trails/overpass + lint work into separate PRs or update the PR description to explicitly cover these additions and their rollout plan.
Comment on lines
+13
to
+48
| export const trailsRoutes = new Elysia({ prefix: '/trails' }) | ||
| .use(authPlugin) | ||
| .get( | ||
| '/search', | ||
| async ({ query }) => { | ||
| const { q, lat, lon, radius, sport, limit, offset } = query; | ||
|
|
||
| if (!q && (lat === undefined || lon === undefined)) { | ||
| return status(400, { | ||
| error: 'Provide either q (text search) or lat + lon (spatial search)', | ||
| }); | ||
| } | ||
|
|
||
| const radiusM = Math.min((radius ?? 50) * 1000, 500_000); | ||
|
|
||
| const builder = new TrailQueryBuilder().timeout(25); | ||
|
|
||
| if (sport) builder.sport(sport); | ||
| if (q) builder.name(q); | ||
| if (lat !== undefined && lon !== undefined) builder.around(lat, lon, radiusM); | ||
|
|
||
| const ql = builder.build(); | ||
|
|
||
| try { | ||
| const response = await queryOverpass(ql); | ||
| const trails = response.elements.map(toTrailSummary); | ||
| const off = offset ?? 0; | ||
| const lim = limit ?? 50; | ||
| return trails.slice(off, off + lim); | ||
| } catch (err) { | ||
| if (isOverpassTimeout(err)) { | ||
| return status(504, { error: 'Overpass query timed out' }); | ||
| } | ||
| console.error('Overpass search error:', err); | ||
| return status(502, { error: 'Overpass API error' }); | ||
| } |
There was a problem hiding this comment.
New /trails routes add external-network behavior (Overpass querying, timeout handling, pagination) but there are no API tests added alongside them. Given the existing route coverage in packages/api/test/ (e.g. weather/alltrails), please add integration tests for /trails/search, /:osmId, and /:osmId/geometry (including validation + timeout/error cases).
Comment on lines
93
to
95
| { | ||
| isAuthenticated: true, | ||
| body: z.object({ |
There was a problem hiding this comment.
This route is now marked isAuthenticated: true, but the existing packages/api/test/alltrails.test.ts uses the unauthenticated api() helper to call /alltrails/preview. As-is, those tests will start failing with 401. Either update the tests to use apiWithAuth() (or apiWithApiKey() if that’s the intended auth mode for this endpoint) or keep this route explicitly public with a // public-route: annotation + rationale.
![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: 20
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
apps/guides/scripts/enhance-content.ts (1)
405-423:⚠️ Potential issue | 🟡 MinorHelp text is stale after switching
--patternfrom regex to substring.Line 407 still advertises
<regex>and Line 420 shows--pattern "hiking|backpacking", which used to match either term via regex alternation. With the newincludes()logic on Lines 113-114, that example matches no file (no basename contains the literal|) and fails silently. Update the docs to reflect the new substring semantics so users don't get empty results.📝 Proposed help-text fix
- -p, --pattern <regex> Only process files matching pattern (legacy mode) + -p, --pattern <substring> Only process files whose basename contains the (case-insensitive) substring @@ - # Pattern-based processing - bun run enhance-content --pattern "hiking|backpacking" --max-files 5 + # Pattern-based processing (case-insensitive substring match on basename) + bun run enhance-content --pattern "hiking" --max-files 5🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/guides/scripts/enhance-content.ts` around lines 405 - 423, Update the help text for the --pattern flag to reflect substring matching (not regex): change the option sigil from "<regex>" to "<substring>" and update the description to something like "Only process files whose basename includes the given substring". Replace the regex example (--pattern "hiking|backpacking") with substring examples (e.g. --pattern "hiking" and --pattern "backpacking" or a combined instruction showing sequential runs), and ensure any mention of regex alternation is removed; this corresponds to the CLI help block that references --pattern and the includes() logic in the processing code (lines around the includes() check).apps/expo/features/ai/lib/localModelManager.ts (1)
83-95:⚠️ Potential issue | 🔴 Critical
isAppleIntelligenceAvailable()tries to access properties on a Promise without awaiting.
getAppleModule()returns an un-awaitedimport(...)(a Promise), somod.apple.isAvailable()at line 94 executes on a Promise object and throws a TypeError, caught at line 95, making the check always returnfalse. Line 245 shows the async pattern is understood (await getAppleModule()), but line 90 omits it.However, making
isAppleIntelligenceAvailable()async is architecturally complex: it's called from React components (settings/index.tsx, AIModeSheet.tsx) without hooks, from the syncgetLocalModel()function (line 102), and from async functions without await (lines 122, 134). A proper fix requires either:
- Converting to a sync cache check (populate on startup via side effect)
- Restructuring the React components to use async checks via hooks/suspend
- Splitting logic (sync availability from cache, async initialization)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/expo/features/ai/lib/localModelManager.ts` around lines 83 - 95, isAppleIntelligenceAvailable() is synchronously calling mod.apple.isAvailable() on the Promise returned by getAppleModule(), causing it to always fail; to fix, introduce a module-scoped cache and an async initializer: create a cached variable (e.g., cachedAppleModule / cachedAppleAvailable) and an async function (e.g., initializeAppleModuleAsync) that awaits getAppleModule(), calls mod.apple.isAvailable(), and stores the boolean; change isAppleIntelligenceAvailable() to return the cached boolean synchronously, ensure initializeAppleModuleAsync is invoked on app startup (or before any usage) so callers like getLocalModel(), settings/index.tsx and AIModeSheet.tsx can keep synchronous checks while the real async detection runs in the background.packages/api/src/routes/packs/index.ts (2)
619-638:⚠️ Potential issue | 🟠 MajorAdd validation guard for
packIdparameter to prevent NULL constraint violation.The
packIdparameter usesz.string()validation, which allows empty strings. WhenpackIdis empty, the database insert fails with a NOT NULL constraint violation onpack_id. Add eitherz.string().min(1)to the schema or an early guardif (!packId) return status(400, { error: 'Pack ID is required' })matching the pattern in the POST/packsroute.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/api/src/routes/packs/index.ts` around lines 619 - 638, The packId coming into the route can be an empty string causing a NOT NULL DB error; add a validation guard by either tightening the input schema to z.string().min(1) for the packId param or adding an early check before the insert (e.g., if (!packId) return status(400, { error: 'Pack ID is required' })) consistent with the POST /packs route pattern; update the validation or add the guard near where packId is read and before the db.insert of NewPackItem so the insert into packItems (the block creating newItem) never runs with an empty packId.
175-177:⚠️ Potential issue | 🟡 MinorAdd defensive guard and increase test coverage for the
/analyze-imageendpoint.The
imagevariable is accessed with.startsWith()without a safety check. WhileAnalyzeImageRequestSchemadeclaresimageas required, there is no test coverage for this endpoint to verify validation actually runs before the handler executes. Add a defensive guard to prevent crashes if validation is bypassed or if the schema is later made optional:Proposed defensive guard
- if (!image.startsWith(`${user.userId}-`)) { + if (typeof image !== 'string' || !image.startsWith(`${user.userId}-`)) { return status(403, { error: 'Unauthorized' }); }Additionally, add a test case for the
/analyze-imageendpoint to verify it properly validates theimagefield and returns 400 when the field is missing or invalid.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/api/src/routes/packs/index.ts` around lines 175 - 177, Add a defensive guard in the /analyze-image handler so you never call image.startsWith on a non-string: before the existing check that uses image.startsWith(`${user.userId}-`), validate that typeof image === 'string' (or otherwise treat missing/invalid as a bad request) and return status(400, { error: 'Invalid image' }) if it fails; keep the existing 403 check for mismatched prefixes. Also add test coverage for the /analyze-image endpoint (using the same test suite that covers packs routes) that sends a request with image missing and with image as a non-string to assert the handler returns 400, and a positive test asserting the prefix check still returns 403 when the prefix does not match; reference AnalyzeImageRequestSchema, the POST '/analyze-image' route handler, and the image variable when locating the code to change.packages/api/src/routes/catalog/index.ts (1)
313-313:⚠️ Potential issue | 🟠 MajorAdd defensive validation for
params.idconversions to prevent NaN forwarding to database queries.The
Number(params.id)conversions at lines 313, 349, 407, and 468 lack validation and will produceNaNwhenparams.idis non-numeric (e.g., "abc") or empty. This causes Postgres errors likeinvalid input syntax for type integer: "NaN"when the NaN value reaches Drizzle queries.The admin routes in the same codebase already implement the correct pattern:
const id = Number(params.id); if (!Number.isFinite(id) || id <= 0) return status(400, { error: 'Invalid catalog item id' });Apply this same defensive check to all four routes (GET, GET similar, PUT, DELETE) in the catalog handler.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/api/src/routes/catalog/index.ts` at line 313, Replace direct Number(params.id) conversions in the catalog route handlers (the GET handler where itemId is set, the "similar" GET, the PUT and the DELETE) with a validated parse: const itemId = Number(params.id); if (!Number.isFinite(itemId) || itemId <= 0) return status(400, { error: 'Invalid catalog item id' }); This prevents NaN from being forwarded to Drizzle/Postgres; apply the same pattern wherever Number(params.id) is used in this file.packages/api/src/middleware/auth.ts (2)
28-34:⚠️ Potential issue | 🟡 Minor"Validated by auth middleware" comment is misleading — this is the middleware.
payload.userIdandpayload.roleare typedunknown(via the index signature onJWTPayload), andverifyJWTonly validates the JWT signature/exp, not the payload shape (see related comment inutils/auth.ts). TheNumber(payload.userId)coercion will silently yieldNaNifuserIdis missing or non-numeric, and the resulting object is then asserted asAuthUserwithuserId: number. ForadminAuthPlugin, thepayload.role !== 'ADMIN'check on line 52 does protect the role assertion, butuserIdstill has no structural guarantee.Consider validating the payload shape once (e.g. with a small Zod schema) inside
verifyJWTor here, and dropping the misleading comment.Also applies to: 55-61
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/api/src/middleware/auth.ts` around lines 28 - 34, The JWT payload shape is not guaranteed here so remove the misleading "validated by auth middleware" comment and add explicit runtime validation of payload.userId and payload.role before casting to AuthUser: either validate inside verifyJWT or immediately in this middleware (e.g., use a small Zod/schema check or manual checks) to ensure payload.userId is numeric and payload.role is 'USER'|'ADMIN'; if validation fails, return/throw an unauthorized error instead of coercing to Number (which can produce NaN) and then asserting the object as AuthUser; also apply the same validation pattern to the adminAuthPlugin branch where role and userId are used.
1-78:⚠️ Potential issue | 🔴 CriticalAdd
.use(authPlugin)to routes/knowledgeBase/reader.ts and routes/alltrails.ts.Two routes have
isAuthenticated: truein their handler options but are missing.use(authPlugin)at the Elysia group level, causing authentication enforcement to fail. Without the plugin registered, the macro resolver never runs, resulting in failed requests instead of proper 401 responses.packages/api/src/routes/knowledgeBase/reader.ts:
export const readerRoutes = new Elysia({ prefix: '/reader' }).use(authPlugin).post(packages/api/src/routes/alltrails.ts:
export const alltrailsRoutes = new Elysia({ prefix: '/alltrails' }).use(authPlugin).post(Both files must import
authPluginfrom@packrat/api/middleware/authand call.use(authPlugin)before their route definitions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/api/src/middleware/auth.ts` around lines 1 - 78, The readerRoutes and alltrailsRoutes Elysia instances are created without registering the auth macro, so routes that set isAuthenticated: true never run the macro; import authPlugin from '@packrat/api/middleware/auth' and call .use(authPlugin) on the Elysia builders that create readerRoutes and alltrailsRoutes (i.e., change new Elysia({ prefix: '/reader' }) and new Elysia({ prefix: '/alltrails' }) to chain .use(authPlugin) before defining .post) so the isAuthenticated macro resolver runs and returns proper 401/403 responses.scripts/lint/no-raw-typeof.ts (1)
28-102:⚠️ Potential issue | 🟡 MinorLint rule misses second
typeofon the same line and over-flags property accesses on exempt globals.Two small gaps worth tightening:
TYPEOF_PATTERNhas nogflag and the walker only callsexeconce per line (Line 96). On a line liketypeof window !== 'undefined' && typeof someValue === 'string', the first match captureswindow, the exemption checkcontinues to the next line, and the second (legitimate) violation onsomeValueis silently dropped.- The identifier capture
[A-Za-z_][A-Za-z0-9_.]*allows dotted member access, sotypeof window.location !== 'undefined'produces identifierwindow.location, which is not inGLOBAL_IDENTIFIERSand is flagged — even though the SSR-availability rationale ("accessing an undeclared global throws a ReferenceError") applies just as much towindow.locationas towindow.🛠️ Suggested fix
Make the pattern global, scan all matches on each line, and treat an identifier as exempt when its root (before the first
.) is in the exemption set:const TYPEOF_PATTERN = - /typeof\s+([A-Za-z_][A-Za-z0-9_.]*)\s*(===|!==)\s*(['"])(string|number|boolean|object|function|undefined|symbol|bigint)\3/; + /typeof\s+([A-Za-z_][A-Za-z0-9_.]*)\s*(===|!==)\s*(['"])(string|number|boolean|object|function|undefined|symbol|bigint)\3/g; @@ - for (let i = 0; i < lines.length; i++) { - const line = lines[i] ?? ''; - const match = TYPEOF_PATTERN.exec(line); - if (match) { - const identifier = match[1] ?? ''; - if (GLOBAL_IDENTIFIERS.has(identifier)) continue; - violations.push({ file: entryRel, line: i + 1, content: line.trimEnd() }); - } - } + for (let i = 0; i < lines.length; i++) { + const line = lines[i] ?? ''; + TYPEOF_PATTERN.lastIndex = 0; + let match: RegExpExecArray | null; + while ((match = TYPEOF_PATTERN.exec(line)) !== null) { + const identifier = match[1] ?? ''; + const root = identifier.split('.')[0] ?? identifier; + if (GLOBAL_IDENTIFIERS.has(root)) continue; + violations.push({ file: entryRel, line: i + 1, content: line.trimEnd() }); + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/lint/no-raw-typeof.ts` around lines 28 - 102, The regexp TYPEOF_PATTERN and the scan in walkDir miss multiple typeof occurrences per line and mis-handle dotted identifiers; update TYPEOF_PATTERN to be global (add the g flag) and in walkDir replace the single exec call with a loop that iterates all matches on the line (while (TYPEOF_PATTERN.exec(...)) style), and when checking exemptions use the root identifier (take match[1] up to the first '.' before consulting GLOBAL_IDENTIFIERS) so expressions like "typeof window.location" are treated as exempt while other dotted identifiers are still flagged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/checks.yml:
- Around line 52-57: Remove the duplicate "Check for unauthenticated routes"
workflow step that runs "bun scripts/lint/no-unauth-routes.ts" because that
check is already included in the "Custom lint rules (typeof guards, raw regex,
process.env)" step which runs "bun lint:custom"; delete the step containing the
name "Check for unauthenticated routes" and its run command while leaving the
"Custom lint rules" and "Check unsafe type casts" ("bun check:casts:strict")
steps intact.
In `@apps/expo/features/ai/components/ToolInvocationRenderer.tsx`:
- Line 25: The long inline single-line comment starting with "// safe-cast: case
guard narrows type to discriminant; local tool types extend ToolUIPart with that
exact `type` field" in ToolInvocationRenderer.tsx exceeds the 100-char Biome
limit—wrap it into multiple shorter comment lines (each under 100 chars)
preserving the same content and comment prefix, align the wrapped lines with the
existing indentation, and keep the same wording (e.g., split after
"discriminant;" and "ToolUIPart") so the meaning and `safe-cast` marker remain
intact.
In `@apps/expo/features/packs/screens/PackDetailScreen.tsx`:
- Around line 72-74: The cast at "const pack = (isOwnedByUser ? packFromStore :
packFromApi) as Pack" is unsafe because it happens before the guard that ensures
pack is defined; remove this premature assertion and keep pack typed as possibly
undefined (use the original union type from packFromStore/packFromApi), then
perform an explicit guard (e.g., if (!isDefined(pack) || ...) return loading UI)
before any closures that assume pack is non-undefined; if you need narrower
typing inside closures after that guard, perform the assertion or local const
there (e.g., const confirmedPack = pack as Pack) or rely on TypeScript narrowing
so optional chaining like pack?.items is still valid until the guard runs.
In `@apps/expo/features/trips/screens/TripDetailScreen.tsx`:
- Around line 25-26: Replace the hardcoded line reference in the comment about
'trip' in TripDetailScreen.tsx with a location-agnostic note and re-wrap it to
<=100 characters; e.g., state that 'trip' may be undefined until the store
hydrates and that an early-return guard in this component ensures a non-null
'trip' before render. Ensure the edited comment uses single quotes and keeps
2-space indentation to satisfy the project's Biome formatting rules.
In `@apps/expo/features/weather/hooks/useLocationSearch.ts`:
- Around line 68-69: Add an explicit return type to the formatWeatherData
function so TypeScript guarantees it returns a WeatherLocation and callers can
stop using unsafe casts; update the signature of formatWeatherData to export
function formatWeatherData(data: WeatherAPIForecastResponse): WeatherLocation,
then remove the "as unknown as WeatherLocation" casts at call sites (e.g.,
usages in useLocationSearch, useLocationRefresh, and LocationPreviewScreen) so
the compiler enforces the shape instead of relying on runtime casts.
In `@apps/guides/scripts/enhance-content.ts`:
- Around line 112-115: The current substring match using lowerPattern and
.includes() breaks patterns that are intended as regex (e.g.,
"hiking|backpacking") because it searches literally for the pipe; change the
logic to treat pattern as a regex: create a RegExp from pattern with the
case‑insensitive flag (e.g., const matcher = new RegExp(pattern, 'i')) and then
use files.filter(file => matcher.test(path.basename(file))) instead of
lowerPattern and .includes(); this preserves alternation and other regex
metacharacters while keeping the existing basename matching behavior.
In `@apps/landing/lib/icons.tsx`:
- Around line 1-12: The isFunction guard in the LucideIcon function causes every
lookup to fall back to FileQuestion because lucide-react icons are forwardRef
components (objects), not functions; update the guard in LucideIcon (which looks
up name in LucideIcons and currently uses isFunction(icon)) to detect forwardRef
components instead—e.g., check typeof icon === 'object' || (icon &&
icon.$$typeof === Symbol.for('react.forward_ref')) or test for a component
displayName—then return the cast icon as LucideIconType when that check passes,
otherwise fall back to LucideIcons.FileQuestion.
In `@packages/analytics/src/core/local-cache.ts`:
- Line 165: Shorten or wrap the trailing comment on the return statement so the
line stays within the 100-character limit; locate the return obj as T; statement
in local-cache.ts and either move the explanatory comment to the previous line
(e.g. // safe-cast: caller-provided generic boundary — obj is built from DuckDB
column names...) or shorten it to a brief one-liner after the return (e.g. //
safe-cast: validated by caller's return type), ensuring you keep the note about
the safe cast for obj and the generic T.
In `@packages/api/src/routes/trails/index.ts`:
- Around line 36-49: The current pagination slices the full result after
queryOverpass(ql) which re-runs the expensive Overpass query for every page;
change the callsite to limit Overpass work by updating queryOverpass to accept a
maxResults parameter (e.g., max = (offset ?? 0) + (limit ?? 50)) and modify
queryOverpass implementation to attach an Overpass `out geom <N>;` (or
equivalent) so Overpass returns at most maxResults elements; keep the existing
trails.slice(off, off+lim) logic but pass maxResults into queryOverpass instead
of fetching the entire dataset; alternatively, if you prefer caching, wrap the
queryOverpass call with a short-TTL cache keyed on a canonicalized ql string
before mapping toTrailSummary so repeated paging uses cached results.
In `@packages/api/src/services/embeddingService.ts`:
- Around line 54-57: generateManyEmbeddings currently removes empty inputs via
cleanValues which shifts indices and breaks callers that index embeddings by
original position; modify generateManyEmbeddings to preserve input order by
mapping inputs to cleaned strings but keeping placeholders for empty values
(e.g., produce an array of the same length where empty/whitespace inputs become
null), call the embedding provider only for the non-null cleaned entries and
then rebuild a result array matching the original indices (embedding vectors or
nulls), update the function signature/return type accordingly (e.g.,
Array<number[] | null>), and ensure callers that expect positional mapping
(catalogService usages: itemsToUpdate.map/itemsToEmbed loop and
processValidItemsBatch mergedItems.map) will receive embeddings aligned with
their original input positions.
In `@packages/api/src/services/etl/processValidItemsBatch.ts`:
- Line 20: The inline comment attached to the mergeItemsBySku call is too long
for the 100-char line width; move that explanatory text into a separate
single-line comment block immediately above the statement (e.g. add a // comment
above the line that references mergeItemsBySku and
NewCatalogItem/CatalogItemValidator), rewrap it so no line exceeds 100 chars,
keep 2-space indentation and single-quote style, then leave the call as: const
mergedItems = mergeItemsBySku(items as NewCatalogItem[]);
In `@packages/api/src/utils/auth.ts`:
- Line 67: The current safe-cast of "return payload as JWTPayload" only ensures
JWT signature/expiry, not the payload shape; update the verifyJWT flow in
auth.ts to perform runtime shape validation before returning: import or define a
JWTPayloadSchema (Zod or equivalent) and call JWTPayloadSchema.parse(payload)
(or perform explicit type-narrowing checks for userId/role) and return the
parsed object, or if you prefer a minimal change, replace the comment to state
that jwtVerify only validates signature/expiry (not shape) and add explicit
guard checks that throw when required fields are missing/invalid; refer to
JWTPayload, JWTPayloadSchema (or parse/guard logic), and the function where
"return payload as JWTPayload" appears.
In `@packages/config/src/config.ts`:
- Around line 51-57: The deepFreeze routine currently skips arrays because it
uses radash.isObject (which returns false for arrays); update the guard in
deepFreeze to treat any non-null object (including arrays) as eligible for
freezing—e.g., replace the isObject check with a non-null object test or a
helper like isNonNullObject that accepts arrays—then ensure the recursive loop
that calls deepFreeze on Object.values(value) still runs for arrays so
APP_CONFIG_SOURCE arrays such as layout.base, layout.weatherSection,
layout.gearSection, and layout.footerSection are frozen as well.
In `@packages/overpass/src/builder.ts`:
- Around line 43-46: The id(osmId: number | string | bigint) method currently
does this._byId = String(osmId) which allows arbitrary strings to be injected
into Overpass QL; change it to first stringify the value then validate it
against a digits-only pattern (e.g. /^\-?\d+$/ or /^\d+$/ depending on whether
negative IDs are allowed) and throw a TypeError/RangeError when the check fails,
otherwise set this._byId to the validated string; update the id method in
builder.ts (the id(...) function and the _byId assignment) to perform this
validation before assigning.
- Around line 6-12: Update the incorrect OSM tag value for equestrian routes:
change the mapping and expectations that currently use "horse_riding" to use
"horse" instead. Specifically, in packages/overpass/src/builder.ts update the
OSM_SPORT_MAP entry for key horse_riding from 'horse_riding' to 'horse' (symbol:
OSM_SPORT_MAP), in packages/overpass/src/transform.ts replace any occurrences
where horse_riding is mapped/translated to the tag 'horse_riding' to 'horse'
(look for the transform function handling OsmSport), and in
packages/overpass/src/builder.test.ts change the test expectation that asserts
["route"="horse_riding"] to expect ["route"="horse"] so the test matches real
OSM tags.
In `@packages/overpass/src/client.ts`:
- Around line 17-28: The fetch to Overpass (the call using fetch(endpoint, {
method: 'POST', ... body: `data=${encodeURIComponent(ql)}` })) can hang; wrap it
with an AbortSignal (e.g. add signal: AbortSignal.timeout(timeoutMs)) and use a
default timeout (8000ms) or read timeoutMs from OverpassClientConfig passed into
the client/constructor; ensure the fetch options include signal and that you
throw a distinct error on AbortError so callers can differentiate timeouts from
HTTP failures (keep the existing response.ok check and error throw for non-2xx
responses).
In `@packages/overpass/src/transform.ts`:
- Around line 3-9: The OSM_ROUTE_TO_SPORT mapping uses the wrong OSM key for
equestrian routes; update the key name in the OSM_ROUTE_TO_SPORT constant from
"horse_riding" to "horse" so it matches actual OSM route=* tags (leave the value
"horse_riding" or adjust the mapped sport value if desired) and ensure any
lookup logic referencing OSM_ROUTE_TO_SPORT continues to work with the corrected
key.
In `@packages/overpass/vitest.config.ts`:
- Around line 1-11: The config is being emitted as CJS because Vitest expects an
ESM config; change the config to ESM by renaming vitest.config.ts to
vitest.config.mts OR replace the use of __dirname with the ESM pattern using
import.meta.url (as in other packages) and ensure you export via defineConfig;
also simplify the include entry to a relative glob string ("src/**/*.test.ts")
instead of resolve(__dirname, 'src/**/*.test.ts') so the include uses a relative
path.
In `@packages/web-ui/src/components/form.tsx`:
- Line 26: The line creating the React context for FormFieldContext exceeds the
100-char limit; move the inline comment to the previous line (above the const)
or wrap it so the const declaration remains <=100 chars; specifically update the
line containing FormFieldContext, React.createContext<FormFieldContextValue> and
the trailing comment so the declaration is short and the explanatory comment is
on its own line (keep existing wording and single-quote/2-space style rules).
- Line 68: The line creating FormItemContext is over the 100-char limit because
it has an inline comment; move the trailing comment to its own preceding line
(use two spaces indentation) so the declaration reads: const FormItemContext =
React.createContext<FormItemContextValue>({} as FormItemContextValue); and place
the explanatory comment on the previous line (// safe-cast: React context
sentinel — FormItem always provides a real id via Provider before consumers run)
using single quotes where applicable and ensuring the line length is under 100
characters; keep references to FormItemContext, React.createContext, and
FormItemContextValue unchanged.
---
Outside diff comments:
In `@apps/expo/features/ai/lib/localModelManager.ts`:
- Around line 83-95: isAppleIntelligenceAvailable() is synchronously calling
mod.apple.isAvailable() on the Promise returned by getAppleModule(), causing it
to always fail; to fix, introduce a module-scoped cache and an async
initializer: create a cached variable (e.g., cachedAppleModule /
cachedAppleAvailable) and an async function (e.g., initializeAppleModuleAsync)
that awaits getAppleModule(), calls mod.apple.isAvailable(), and stores the
boolean; change isAppleIntelligenceAvailable() to return the cached boolean
synchronously, ensure initializeAppleModuleAsync is invoked on app startup (or
before any usage) so callers like getLocalModel(), settings/index.tsx and
AIModeSheet.tsx can keep synchronous checks while the real async detection runs
in the background.
In `@apps/guides/scripts/enhance-content.ts`:
- Around line 405-423: Update the help text for the --pattern flag to reflect
substring matching (not regex): change the option sigil from "<regex>" to
"<substring>" and update the description to something like "Only process files
whose basename includes the given substring". Replace the regex example
(--pattern "hiking|backpacking") with substring examples (e.g. --pattern
"hiking" and --pattern "backpacking" or a combined instruction showing
sequential runs), and ensure any mention of regex alternation is removed; this
corresponds to the CLI help block that references --pattern and the includes()
logic in the processing code (lines around the includes() check).
In `@packages/api/src/middleware/auth.ts`:
- Around line 28-34: The JWT payload shape is not guaranteed here so remove the
misleading "validated by auth middleware" comment and add explicit runtime
validation of payload.userId and payload.role before casting to AuthUser: either
validate inside verifyJWT or immediately in this middleware (e.g., use a small
Zod/schema check or manual checks) to ensure payload.userId is numeric and
payload.role is 'USER'|'ADMIN'; if validation fails, return/throw an
unauthorized error instead of coercing to Number (which can produce NaN) and
then asserting the object as AuthUser; also apply the same validation pattern to
the adminAuthPlugin branch where role and userId are used.
- Around line 1-78: The readerRoutes and alltrailsRoutes Elysia instances are
created without registering the auth macro, so routes that set isAuthenticated:
true never run the macro; import authPlugin from '@packrat/api/middleware/auth'
and call .use(authPlugin) on the Elysia builders that create readerRoutes and
alltrailsRoutes (i.e., change new Elysia({ prefix: '/reader' }) and new Elysia({
prefix: '/alltrails' }) to chain .use(authPlugin) before defining .post) so the
isAuthenticated macro resolver runs and returns proper 401/403 responses.
In `@packages/api/src/routes/catalog/index.ts`:
- Line 313: Replace direct Number(params.id) conversions in the catalog route
handlers (the GET handler where itemId is set, the "similar" GET, the PUT and
the DELETE) with a validated parse: const itemId = Number(params.id); if
(!Number.isFinite(itemId) || itemId <= 0) return status(400, { error: 'Invalid
catalog item id' }); This prevents NaN from being forwarded to Drizzle/Postgres;
apply the same pattern wherever Number(params.id) is used in this file.
In `@packages/api/src/routes/packs/index.ts`:
- Around line 619-638: The packId coming into the route can be an empty string
causing a NOT NULL DB error; add a validation guard by either tightening the
input schema to z.string().min(1) for the packId param or adding an early check
before the insert (e.g., if (!packId) return status(400, { error: 'Pack ID is
required' })) consistent with the POST /packs route pattern; update the
validation or add the guard near where packId is read and before the db.insert
of NewPackItem so the insert into packItems (the block creating newItem) never
runs with an empty packId.
- Around line 175-177: Add a defensive guard in the /analyze-image handler so
you never call image.startsWith on a non-string: before the existing check that
uses image.startsWith(`${user.userId}-`), validate that typeof image ===
'string' (or otherwise treat missing/invalid as a bad request) and return
status(400, { error: 'Invalid image' }) if it fails; keep the existing 403 check
for mismatched prefixes. Also add test coverage for the /analyze-image endpoint
(using the same test suite that covers packs routes) that sends a request with
image missing and with image as a non-string to assert the handler returns 400,
and a positive test asserting the prefix check still returns 403 when the prefix
does not match; reference AnalyzeImageRequestSchema, the POST '/analyze-image'
route handler, and the image variable when locating the code to change.
In `@scripts/lint/no-raw-typeof.ts`:
- Around line 28-102: The regexp TYPEOF_PATTERN and the scan in walkDir miss
multiple typeof occurrences per line and mis-handle dotted identifiers; update
TYPEOF_PATTERN to be global (add the g flag) and in walkDir replace the single
exec call with a loop that iterates all matches on the line (while
(TYPEOF_PATTERN.exec(...)) style), and when checking exemptions use the root
identifier (take match[1] up to the first '.' before consulting
GLOBAL_IDENTIFIERS) so expressions like "typeof window.location" are treated as
exempt while other dotted identifiers are still flagged.
---
Nitpick comments:
In `@apps/admin/lib/cfAccess.ts`:
- Around line 13-16: Tighten the CFAccessIdentitySchema by validating the email
shape: change the email field in CFAccessIdentitySchema from z.string() to
z.string().email() so isBehindCFAccess and useCFAccessIdentity only treat
properly-formed emails as valid CF Access identities; update any places that
construct or parse the identity to handle validation errors (e.g., where the
schema is parsed) so malformed or empty email values are rejected/handled
gracefully.
In `@apps/admin/lib/env.ts`:
- Around line 11-21: Replace the direct Zod parse call with a safeParse flow:
call adminEnvSchema.safeParse(process.env), check the result.success flag, and
if false build a clearer message (e.g., include "Invalid admin env:" plus
formatted zod errors from result.error) and throw a new Error with that message;
update the export of adminEnv to use the parsed data when successful. Reference:
adminEnvSchema, adminEnv, AdminEnv, and process.env.
In `@apps/expo/app/`(app)/current-pack/[id].tsx:
- Around line 199-201: The inline double-cast (item as unknown as PackItem)
hides a schema drift between Treaty response (createdAt?: string) and the
PackItem type (createdAt required); fix at the source by either making
PackItem.createdAt optional/nullable to match Treaty, or update
usePackDetailsFromStore to normalize pack.items into PackItem[] (e.g.,
map/validate and supply a default or convert createdAt to the required format)
so callers like ItemRow can consume a true PackItem without casting; remove the
cast in the JSX once the type is corrected.
In `@apps/expo/features/ai/components/ToolInvocationRenderer.tsx`:
- Around line 28-42: Remove the repeated "// safe-cast: case guard narrows type
to discriminant literal" comments in each case arm and keep a single top-level
comment explaining the safety of the casts (already present at Lines 20–22);
specifically, delete the per-case comments that precede the JSX returns for
WeatherGenerativeUI, CatalogItemsGenerativeUI, GuidesRAGGenerativeUI,
PackDetailsGenerativeUI, and PackItemDetailsGenerativeUI while leaving the
existing type assertions (e.g., "toolInvocation as WeatherTool", "as
CatalogItemsTool", etc.) and the switch/return logic unchanged.
In `@apps/expo/features/ai/hooks/useReportContent.ts`:
- Around line 27-28: The return currently unsafely casts API responses to
ReportContentResponse; add runtime validation using a Zod schema (e.g.,
ReportContentResponseSchema with success: boolean and reportId: string) and
validate the `data` before casting in the hook (useReportContent). Replace the
direct cast "return data as unknown as ReportContentResponse" with schema.parse
or schema.safeParse and handle failures (throw or return a rejected Promise /
error result) so only validated objects are treated as ReportContentResponse.
In `@apps/expo/features/ai/hooks/useReportedContent.ts`:
- Around line 29-30: The hook useReportedContent currently returns the treaty
response via an unsafe cast to ReportedContentResponse; add runtime validation
using a Zod schema (e.g., ReportedItemSchema and ReportedContentResponseSchema)
that matches the expected shape, parse/validate the fetched `data` before
returning, and handle validation failures by logging/throwing or returning a
safe fallback; update the return path in useReportedContent to return the
validated/parsed result (or error handling) instead of `data as unknown as
ReportedContentResponse`.
In `@apps/expo/features/auth/hooks/useAuthActions.ts`:
- Around line 72-73: Create a small adapter function to centralize the unsafe
cast: add a toUser(user: unknown): User helper that contains the existing
"safe-cast" comment and returns user as User, then replace all occurrences of
userStore.set(data.user as unknown as User) with
userStore.set(toUser(data.user)) (this includes the four call sites in
useAuthActions.ts where the pattern appears); keep the original comment in the
toUser implementation so the justification and validation note live in one
place.
- Around line 26-35: Remove the unnecessary JSON.parse branch in the redirect
function: simplify function redirect to directly call router.dismissTo(route as
Href) because redirectToAtom only stores plain path strings; delete the
try-catch and the parsedRoute logic and keep a single direct call to
router.dismissTo(route as Href) (referencing the redirect function and
router.dismissTo).
In `@apps/expo/features/catalog/components/CatalogBrowserModal.tsx`:
- Around line 191-196: Remove the duplicate "safe-cast: treaty response shape
matches CatalogItem[]" comment around the items declaration in
CatalogBrowserModal.tsx so only one instance remains (matching how popularItems
is documented); edit the block that defines const items = (...) as CatalogItem[]
and keep either the leading or trailing safe-cast comment (prefer the one
consistent with the popularItems block), deleting the other.
In `@apps/expo/features/catalog/hooks/useSimilarItems.ts`:
- Around line 32-33: The current safe-cast in useSimilarItems.ts returns "data
as unknown as SimilarItemsResponse", which relies on a double-cast; replace this
with a Zod-validated return using the API schema so the function's return type
is inferred and runtime-validated instead of asserting types. Locate the return
inside useSimilarItems (the line returning data) and parse/validate the raw
response with the existing Zod schema (the schema that represents
SimilarItemsResponse) and return the parsed result so the function's signature
can use the Zod-inferred type rather than the "as unknown as
SimilarItemsResponse" cast. Ensure errors from validation are handled or
propagated consistently with existing error handling.
In `@apps/expo/features/pack-templates/store/packTemplateItems.ts`:
- Around line 17-23: The current safe-cast uses "as unknown as
PackTemplateItem[]" after parsing with PackTemplateWithItemsSchema, which can
hide mismatches if PackTemplateItem diverges from PackTemplateItemSchema; fix by
aligning the TypeScript types with the Zod schema: replace the post-parse cast
with the inferred type from PackTemplateItemSchema (i.e. use z.infer<typeof
PackTemplateItemSchema>[] or update the PackTemplateItem type to equal
z.infer<typeof PackTemplateItemSchema>), and apply the same change to the other
occurrences that use "as unknown as PackTemplateItem" (the similar casts at the
other locations referenced in the review) so the compiler will enforce
schema/type consistency instead of relying on unsafe casts.
In `@apps/expo/features/packs/components/SimilarItemsForPackItem.tsx`:
- Around line 126-127: The current explicit cast in SimilarItemsForPackItem.tsx
(data.items as unknown as SimilarItem[]) should be removed by updating the hook
return type: change useSimilarPackItems to return a typed payload whose items is
SimilarItem[] (matching the cleanup already done in SimilarItems.tsx), so
consumers can pass data.items without casting; update the hook's TypeScript
signature and any treaty response typing to ensure the hook returns { items:
SimilarItem[] } (or SimilarItem[] directly) and then remove the double-cast in
SimilarItemsForPackItem.tsx.
In `@apps/expo/features/packs/hooks/useDuplicatePack.ts`:
- Around line 24-25: The code in useDuplicatePack.ts currently force-casts the
API response with "return data as unknown as Pack"; replace that unsafe cast
with runtime validation using the existing Zod schema (import PackSchema from
"@packrat/api/schemas/packs" or create PackSchema if missing), run
PackSchema.parse or safeParse on the response before returning, and handle
validation failures by logging/throwing or returning a rejected/nullable result
so invalid shapes are not silently accepted (update the function that returns
the Pack to validate and return the parsed value from PackSchema).
In `@apps/expo/features/packs/hooks/usePackGapAnalysis.ts`:
- Around line 31-32: The API response in usePackGapAnalysis (returned as data as
unknown as GapAnalysisResponse) is being unsafe-cast; add runtime validation by
defining Zod schemas (e.g., GapAnalysisItemSchema and GapAnalysisResponseSchema)
that mirror GapAnalysisResponse, then validate the fetched data with
GapAnalysisResponseSchema.parse or safeParse inside usePackGapAnalysis before
returning; on validation failure, handle appropriately (throw or log and return
a safe fallback) so only validated data flows out as GapAnalysisResponse.
In `@apps/guides/components/search.tsx`:
- Line 31: Update the inline "safe-cast" comment to be more precise: instead of
claiming MouseEvent.target is "always a DOM Node", state that MouseEvent.target
is typed as EventTarget | null and that you're making a local assumption (e.g.,
that during this mousedown handler the target will be a DOM Node/HTMLElement)
before performing the cast; reference the MouseEvent.target usage and the
safe-cast comment in apps/guides/components/search.tsx so reviewers know this is
an explicit assumption rather than a guaranteed type.
In `@apps/guides/scripts/sync-to-r2.ts`:
- Around line 41-44: Remove the duplicated "safe-cast" comment: keep a single
explanatory comment for the cast between SyncEnv and ValidatedEnv (either the
existing multi-line note before the const or the inline comment on the env as
unknown as ValidatedEnv expression) and delete the other; ensure the remaining
comment references the same rationale and mentions R2BucketService so readers
see why the cast in the R2BucketService constructor call is safe.
In `@lefthook.yml`:
- Around line 10-27: No change required for correctness—the pre-push block
(pre-push, skip entries CI and GITHUB_ACTIONS, and the clean-checks command) is
correct and can stay as-is; if you want the optional performance improvement,
replace the single long &&-chained command under clean-checks with multiple
separate commands entries (each calling one of the scripts like bun
scripts/lint/no-raw-typeof.ts, bun scripts/lint/no-raw-regex.ts, bun
packages/env/scripts/no-raw-process-env.ts, etc.) so lefthook can run them in
parallel and surface failures individually.
In `@packages/analytics/src/core/data-export.ts`:
- Line 14: The TIMESTAMP_UNSAFE_CHARS regex constant is duplicated elsewhere;
refactor by extracting TIMESTAMP_UNSAFE_CHARS into a shared helper module (e.g.,
export from a new util in packages/*), replace local declarations in
packages/analytics/src/core/data-export.ts and
apps/guides/scripts/enhance-content.ts with an import of that shared constant,
and update any usages to reference the imported TIMESTAMP_UNSAFE_CHARS to ensure
consistent sanitization across callers.
In `@packages/analytics/src/core/entity-resolver.ts`:
- Around line 380-385: The ternary branch inside the local helper v
unnecessarily calls String(x) after isString(x) has narrowed x to string; update
the branch in the helper v so that when isString(x) is true you pass x directly
to SQLFragments.escapeSql and build the quoted result (i.e., replace String(x)
with x), keeping the NULL and non-string branches unchanged; this touches the
local constant v and its usage of SQLFragments.escapeSql in entity-resolver.ts.
In `@packages/analytics/src/core/spec-parser.ts`:
- Around line 260-265: The inline value formatter function v contains a
redundant String(x) call inside the isString(x) branch; replace String(x) with
just x (or remove the extra conversion) when calling SQLFragments.escapeSql so
the branch uses the already-narrowed string directly (i.e., update the arrow
function v and its isString(x) branch to call SQLFragments.escapeSql(x) rather
than SQLFragments.escapeSql(String(x))).
In `@packages/api/src/routes/alltrails.ts`:
- Around line 34-79: Extract the duplicated fetch+error-handling logic into a
small helper (e.g., fetchAllTrails(url, fetchOptions)) used in
packages/api/src/routes/alltrails.ts so both the initial fetch (with redirect:
'manual') and the validated-redirect fetch (with redirect: 'error') call the
same routine; the helper should set the UA header and AbortSignal.timeout(8000)
if not provided, perform fetch, and translate a DOMException named
'TimeoutError' into a 504-style response indicator and other exceptions into a
502-style indicator so the existing callers (the code around response = await
fetch(...) and the second response = await fetch(...)) can return the same
status(...) results without duplicating try/catch logic.
In `@packages/api/src/routes/trailConditions/reports.ts`:
- Around line 9-12: Hoist the three module-scope regexes (LIKE_ESCAPE_BACKSLASH,
LIKE_ESCAPE_PERCENT, LIKE_ESCAPE_UNDERSCORE) are fine, but replace the
sequential .replace(...) calls that apply them (the three replaces at the
current escape site in reports.ts) with a single .replace using one regex that
matches backslash, percent, or underscore and a replacer function that returns
the correct escaped string for each match; update the code that constructs the
LIKE-escaped value to use this single-pass replacement while still honoring the
backslash → percent → underscore escape semantics by handling backslash matches
first within the replacer.
In `@packages/api/src/services/etl/processValidItemsBatch.ts`:
- Around line 9-20: The function processValidItemsBatch currently accepts items:
Partial<NewCatalogItem>[] but immediately casts to NewCatalogItem[] for
mergeItemsBySku, defeating static type checks; change the signature to accept
items: NewCatalogItem[] and remove the unsafe cast, and update all callers to
run CatalogItemValidator (or otherwise narrow/validate) before calling
processValidItemsBatch so callers pass fully validated NewCatalogItem[]; ensure
any call sites that currently pass Partial items perform the validation/type
narrowing first (e.g., using CatalogItemValidator) to satisfy the new type
contract.
In `@packages/api/src/utils/csv-utils.ts`:
- Around line 296-297: The runtime guard isString(input) inside parseFaqs(input:
string) is unreachable given the current signature; either widen the parameter
to accept unknown (change parseFaqs(input: unknown):
Array<{question:string;answer:string}>) so the defensive check is meaningful, or
remove the isString check and rely on the string type and callers; update any
internal uses of input accordingly and keep the function name parseFaqs and the
guard isString referenced so reviewers can locate the change.
In `@packages/checks/src/check-magic-strings.ts`:
- Around line 123-136: The current logic in shouldSkipLine and scanFile misses
trailing inline comments and some export forms, causing quoted text in inline
comments or inside /*…*/ blocks to be treated as code literals; add a helper
stripTrailingLineComment(line: string): string that scans the line once and
removes the first // that is not inside a single-quote, double-quote, or
backtick literal, then call stripTrailingLineComment at the start of scanFile's
per-line processing before shouldSkipLine/literal extraction; also broaden
shouldSkipLine to recognize export * from and export { ... } from (allowing
whitespace before the '{') so re-exports are skipped. Ensure helpers are named
stripTrailingLineComment, shouldSkipLine, and changes are applied in scanFile to
keep behavior consistent.
In `@packages/env/scripts/no-raw-process-env.ts`:
- Around line 9-17: Update the header "Allowed files (intentionally exempt — see
...)" docblock to include the six missing entries so the comment matches the
runtime allowlist: add packages/env/scripts/no-raw-process-env.ts,
packages/api/scripts/validate-cloudflare-api-env.ts,
packages/analytics/src/core/env.ts, apps/guides/scripts/sync-to-r2.ts,
packages/api/src/utils/__tests__/, and apps/admin/lib/env.ts into that
enumerated list; also make the same addition in the duplicate docblock later in
the file (the block at lines indicated in the review) so both header comments
remain in sync.
In `@packages/overpass/src/builder.ts`:
- Around line 53-63: The build() method can emit an unbounded planet-wide query
when neither id() nor a spatial constraint (around() or bbox(), i.e. this._byId
is null and this._spatial is empty); add a guard at the start of build() that
checks if this._byId === null && (!this._spatial || this._spatial.length === 0)
and throw a descriptive Error (e.g., require id(), around(), or bbox() before
build) to prevent accidental unbounded queries; update any callers/tests to
expect the thrown error or document the precondition on the class if you prefer
a non-throwing approach.
In `@packages/overpass/src/client.ts`:
- Around line 30-35: The thrown error after
OverpassResponseSchema.safeParse(data) loses validation details; update the
error thrown in the block that checks parsed.success to include the validation
failure information (e.g., append parsed.error.issues to the message or pass
parsed.error as the Error cause) so callers and logs get actionable schema-issue
details; locate the logic around OverpassResponseSchema.safeParse(data) and the
current throw new Error('Overpass response did not match expected schema') and
replace it with an error that includes parsed.error.issues (or uses the cause
option).
In `@packages/overpass/tsconfig.json`:
- Around line 2-8: Add the TypeScript compiler option noUncheckedIndexedAccess:
true to the package's tsconfig compilerOptions to match the repo convention;
update the existing "compilerOptions" block (which currently contains "target",
"module", "moduleResolution", "strict", and "skipLibCheck") by adding
"noUncheckedIndexedAccess": true so the overpass package disallows unchecked
indexed access like other packages.
In `@packages/web-ui/src/components/chart.tsx`:
- Around line 343-350: Replace the remaining raw typeof checks with the package
guard isString from `@packrat/guards`: in the conditional branches that currently
check "typeof payload[key as keyof typeof payload] === 'string'" and "typeof
payloadPayload[key as keyof typeof payloadPayload] === 'string'", import
isString and use it to assert the value (e.g., isString(payload[key as keyof
typeof payload]) and isString(payloadPayload[key as keyof typeof
payloadPayload])) before assigning to configLabelKey; remove the typeof checks
and keep the existing string casts only where still needed after the guard.
In `@scripts/lint/no-unauth-routes.ts`:
- Around line 96-117: The current loop that uses ROUTE_START, parenOpen, depth
and callEnd to find the end of a route call is brittle because it counts
parentheses inside strings/comments/template literals; replace the byte-wise
paren-depth walker with a proper TypeScript/JS AST scan (e.g.,
ts.createSourceFile + forEachChild or Bun.Transpiler scan) to locate the
CallExpression for the route method (the node whose callee matches ROUTE_START's
method capture) and read its full argument range; update the code that currently
computes parenOpen/depth/callEnd to instead derive start/end from the AST node
so AUTH_MACRO checks operate on the correct span.
🪄 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: 95dfcf60-1402-4215-9b12-7c7ae91bdbf9
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (145)
.github/workflows/checks.ymlapps/admin/app/login/page.tsxapps/admin/lib/api.tsapps/admin/lib/cfAccess.tsapps/admin/lib/env.tsapps/expo/app/(app)/(tabs)/profile/index.tsxapps/expo/app/(app)/current-pack/[id].tsxapps/expo/app/(app)/messages/chat.android.tsxapps/expo/app/(app)/messages/chat.tsxapps/expo/atoms/atomWithAsyncStorage.tsapps/expo/atoms/atomWithKvStorage.tsapps/expo/atoms/atomWithSecureStorage.tsapps/expo/components/Icon/Icon.ios.tsxapps/expo/components/Icon/Icon.tsxapps/expo/components/Icon/get-icon-names.tsapps/expo/components/initial/WeightBadge.tsxapps/expo/features/ai-packs/hooks/useGeneratedPacks.tsapps/expo/features/ai/atoms/chatStorageAtoms.tsapps/expo/features/ai/components/AIModeSheet.tsxapps/expo/features/ai/components/ChatBubble.tsxapps/expo/features/ai/components/ToolInvocationRenderer.tsxapps/expo/features/ai/hooks/useReportContent.tsapps/expo/features/ai/hooks/useReportedContent.tsapps/expo/features/ai/lib/CustomChatTransport.tsapps/expo/features/ai/lib/localModelManager.tsapps/expo/features/auth/hooks/useAuthActions.tsapps/expo/features/catalog/components/CatalogBrowserModal.tsxapps/expo/features/catalog/components/SimilarItems.tsxapps/expo/features/catalog/hooks/useSimilarItems.tsapps/expo/features/catalog/screens/CatalogItemsScreen.tsxapps/expo/features/pack-templates/components/AddPackTemplateItemActions.tsxapps/expo/features/pack-templates/hooks/useGenerateTemplateFromOnlineContent.tsapps/expo/features/pack-templates/hooks/usePackTemplateSummary.tsapps/expo/features/pack-templates/screens/ItemsScanScreen.tsxapps/expo/features/pack-templates/store/packTemplateItems.tsapps/expo/features/pack-templates/store/packTemplates.tsapps/expo/features/packs/components/AddPackItemActions.tsxapps/expo/features/packs/components/PackCard.tsxapps/expo/features/packs/components/SimilarItemsForPackItem.tsxapps/expo/features/packs/hooks/useDuplicatePack.tsapps/expo/features/packs/hooks/useImageDetection.tsapps/expo/features/packs/hooks/usePackGapAnalysis.tsapps/expo/features/packs/hooks/usePackItemDetailsFromApi.tsapps/expo/features/packs/hooks/useSeasonSuggestions.tsapps/expo/features/packs/screens/ItemsScanScreen.tsxapps/expo/features/packs/screens/PackDetailScreen.tsxapps/expo/features/packs/store/packItems.tsapps/expo/features/packs/store/packs.tsapps/expo/features/packs/utils/computePackWeights.tsapps/expo/features/trail-conditions/components/TrailConditionReportCard.tsxapps/expo/features/trail-conditions/hooks/useTrailConditionReports.tsapps/expo/features/trips/components/TripForm.tsxapps/expo/features/trips/screens/TripDetailScreen.tsxapps/expo/features/weather/hooks/useLocationRefresh.tsapps/expo/features/weather/hooks/useLocationSearch.tsapps/expo/features/weather/hooks/useWeatherAlert.tsapps/expo/features/weather/screens/LocationPreviewScreen.tsxapps/expo/features/wildlife/hooks/useWildlifeIdentification.tsapps/expo/lib/store.tsapps/expo/lib/utils/dateUtils.tsapps/expo/lib/utils/domain-specific-extensions.tsapps/expo/utils/format-ai-response.tsapps/guides/app/dev/generate/page.tsxapps/guides/components/search.tsxapps/guides/lib/assertDefined.tsapps/guides/package.jsonapps/guides/scripts/enhance-content.tsapps/guides/scripts/sync-to-r2.tsapps/landing/lib/icons.tsxapps/landing/lib/typeAssertions.tsapps/landing/package.jsonbiome.jsonlefthook.ymlpackage.jsonpackages/analytics/package.jsonpackages/analytics/src/core/data-export.tspackages/analytics/src/core/enrichment.tspackages/analytics/src/core/entity-resolver.tspackages/analytics/src/core/local-cache.tspackages/analytics/src/core/spec-parser.tspackages/analytics/src/core/type-assertions.tspackages/api-client/package.jsonpackages/api-client/src/index.tspackages/api/package.jsonpackages/api/src/containers/AppContainer.tspackages/api/src/index.tspackages/api/src/middleware/auth.tspackages/api/src/routes/ai/index.tspackages/api/src/routes/alltrails.tspackages/api/src/routes/auth/index.tspackages/api/src/routes/catalog/index.tspackages/api/src/routes/index.tspackages/api/src/routes/knowledgeBase/reader.tspackages/api/src/routes/packTemplates/index.tspackages/api/src/routes/packs/index.tspackages/api/src/routes/trailConditions/reports.tspackages/api/src/routes/trails/index.tspackages/api/src/routes/weather.tspackages/api/src/routes/wildlife/index.tspackages/api/src/services/aiService.tspackages/api/src/services/embeddingService.tspackages/api/src/services/etl/CatalogItemValidator.tspackages/api/src/services/etl/processValidItemsBatch.tspackages/api/src/services/executeSqlAiTool.tspackages/api/src/services/r2-bucket.tspackages/api/src/services/refreshTokenService.tspackages/api/src/utils/auth.tspackages/api/src/utils/compute-pack.tspackages/api/src/utils/csv-utils.tspackages/api/src/utils/env-validation.tspackages/api/src/utils/format-ai-response.tspackages/api/test/setup.tspackages/checks/src/check-magic-strings.tspackages/checks/src/check-type-casts.tspackages/cli/package.jsonpackages/cli/src/args.tspackages/cli/src/shared.tspackages/config/package.jsonpackages/config/src/config.tspackages/env/scripts/no-raw-process-env.tspackages/guards/src/narrow.tspackages/mcp/src/tools/packs.tspackages/mcp/src/tools/trail-conditions.tspackages/mcp/src/tools/trails.tspackages/mcp/src/tools/trips.tspackages/overpass/package.jsonpackages/overpass/src/builder.test.tspackages/overpass/src/builder.tspackages/overpass/src/client.tspackages/overpass/src/index.tspackages/overpass/src/schemas.tspackages/overpass/src/transform.test.tspackages/overpass/src/transform.tspackages/overpass/src/types.tspackages/overpass/tsconfig.jsonpackages/overpass/vitest.config.tspackages/web-ui/src/components/chart.tsxpackages/web-ui/src/components/form.tsxpackages/web-ui/src/components/sidebar.tsxpackages/web-ui/src/lib/utils.tsscripts/check-all.tsscripts/lint/no-duplicate-guards.tsscripts/lint/no-raw-regex.tsscripts/lint/no-raw-typeof.tsscripts/lint/no-unauth-routes.ts
Comment on lines
+52
to
+57
| - name: Custom lint rules (typeof guards, raw regex, process.env) | ||
| run: bun lint:custom | ||
| - name: Check for unauthenticated routes | ||
| run: bun scripts/lint/no-unauth-routes.ts | ||
| - name: Check unsafe type casts | ||
| run: bun check:casts:strict |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect what `lint:custom`, `check:casts:strict`, and `check:all` actually run.
fd -t f 'package.json' --max-depth 2 --exec sh -c 'echo "=== {} ==="; jq -r ".scripts | to_entries[] | select(.key|test(\"^(lint:custom|check:(all|casts(:strict)?|react-doctor)|check-types)$\")) | \"\(.key): \(.value)\"" {} 2>/dev/null'Repository: PackRat-AI/PackRat
Length of output: 566
🏁 Script executed:
cat -n scripts/check-all.ts | head -60Repository: PackRat-AI/PackRat
Length of output: 2392
🏁 Script executed:
sed -n '56,120p' scripts/check-all.tsRepository: PackRat-AI/PackRat
Length of output: 1817
Remove the explicit no-unauth-routes step at line 54–55; it already runs as part of lint:custom.
lint:custom includes no-unauth-routes in its chain, so the dedicated step at lines 54–55 runs that check a second time. The check:casts:strict step (line 56–57) is not part of lint:custom, so that one is not duplicated.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/checks.yml around lines 52 - 57, Remove the duplicate
"Check for unauthenticated routes" workflow step that runs "bun
scripts/lint/no-unauth-routes.ts" because that check is already included in the
"Custom lint rules (typeof guards, raw regex, process.env)" step which runs "bun
lint:custom"; delete the step containing the name "Check for unauthenticated
routes" and its run command while leaving the "Custom lint rules" and "Check
unsafe type casts" ("bun check:casts:strict") steps intact.
| // the cast is verified by the switch guard above each arm. | ||
| switch (toolInvocation.type) { | ||
| case 'tool-webSearchTool': | ||
| // safe-cast: case guard narrows type to discriminant; local tool types extend ToolUIPart with that exact `type` field |
Contributor
There was a problem hiding this comment.
Wrap the long inline comment to satisfy Biome line-width rules.
Line 25 appears to exceed the 100-character limit enforced for TSX files.
✂️ Suggested edit
- // safe-cast: case guard narrows type to discriminant; local tool types extend ToolUIPart with that exact `type` field
+ // safe-cast: case guard narrows type to discriminant; local tool types
+ // extend ToolUIPart with that exact `type` fieldAs per coding guidelines, **/*.{js,jsx,ts,tsx,json}: Enforce 2 spaces indentation, 100 character line width, and single quotes via Biome 2.0.
📝 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
| // safe-cast: case guard narrows type to discriminant; local tool types extend ToolUIPart with that exact `type` field | |
| // safe-cast: case guard narrows type to discriminant; local tool types | |
| // extend ToolUIPart with that exact `type` field |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/expo/features/ai/components/ToolInvocationRenderer.tsx` at line 25, The
long inline single-line comment starting with "// safe-cast: case guard narrows
type to discriminant; local tool types extend ToolUIPart with that exact `type`
field" in ToolInvocationRenderer.tsx exceeds the 100-char Biome limit—wrap it
into multiple shorter comment lines (each under 100 chars) preserving the same
content and comment prefix, align the wrapped lines with the existing
indentation, and keep the same wording (e.g., split after "discriminant;" and
"ToolUIPart") so the meaning and `safe-cast` marker remain intact.
Comment on lines
+72
to
74
| // safe-cast: pack is guaranteed non-undefined by the early-return guard below; | ||
| // TypeScript cannot track narrowing across the closure boundary. | ||
| const pack = (isOwnedByUser ? packFromStore : packFromApi) as Pack; |
Contributor
There was a problem hiding this comment.
Comment is misleading about cast safety.
The comment states "pack is guaranteed non-undefined by the early-return guard below," but the cast on line 74 occurs before the guard check at line 445. At the point of the cast, pack could still be undefined if both packFromStore and packFromApi are undefined.
The cast removes type safety between lines 74-445, masking potential undefined access. For example, line 162 (pack?.items?.length) uses optional chaining, suggesting the code already handles potential undefined values.
🔧 Suggested fix: Move the type assertion after the guard
- // safe-cast: pack is guaranteed non-undefined by the early-return guard below;
- // TypeScript cannot track narrowing across the closure boundary.
- const pack = (isOwnedByUser ? packFromStore : packFromApi) as Pack;
+ const pack = isOwnedByUser ? packFromStore : packFromApi;Then after the guard at line 445, if you need to narrow the type in closures:
if (!isDefined(pack)) {
return (/* loading UI */);
}
// At this point, TypeScript knows pack is defined
// Use pack directly in closures belowAlternatively, if the cast is truly necessary for closure narrowing, move the comment to explain that optional chaining is defensive until after the guard validates the value.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/expo/features/packs/screens/PackDetailScreen.tsx` around lines 72 - 74,
The cast at "const pack = (isOwnedByUser ? packFromStore : packFromApi) as Pack"
is unsafe because it happens before the guard that ensures pack is defined;
remove this premature assertion and keep pack typed as possibly undefined (use
the original union type from packFromStore/packFromApi), then perform an
explicit guard (e.g., if (!isDefined(pack) || ...) return loading UI) before any
closures that assume pack is non-undefined; if you need narrower typing inside
closures after that guard, perform the assertion or local const there (e.g.,
const confirmedPack = pack as Pack) or rely on TypeScript narrowing so optional
chaining like pack?.items is still valid until the guard runs.
Comment on lines
+25
to
+26
| // safe-cast: trip may be undefined before the store is hydrated; the guard at line ~38 handles | ||
| // the undefined case and returns early, ensuring trip is non-null at render time below. |
Contributor
There was a problem hiding this comment.
Avoid hardcoded line references in comments and wrap to max line length.
Line 25 hardcodes a line reference (~38) that is already stale (the guard is at Line 40), and the comment exceeds the 100-character limit. Please make it location-agnostic and wrap it.
Suggested edit
- // safe-cast: trip may be undefined before the store is hydrated; the guard at line ~38 handles
- // the undefined case and returns early, ensuring trip is non-null at render time below.
+ // safe-cast: trip may be undefined before the store hydrates.
+ // The early return below handles undefined and guarantees non-null access afterward.As per coding guidelines, "**/*.{js,jsx,ts,tsx,json}: Enforce 2 spaces indentation, 100 character line width, and single quotes via Biome 2.0".
📝 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
| // safe-cast: trip may be undefined before the store is hydrated; the guard at line ~38 handles | |
| // the undefined case and returns early, ensuring trip is non-null at render time below. | |
| // safe-cast: trip may be undefined before the store hydrates. | |
| // The early return below handles undefined and guarantees non-null access afterward. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/expo/features/trips/screens/TripDetailScreen.tsx` around lines 25 - 26,
Replace the hardcoded line reference in the comment about 'trip' in
TripDetailScreen.tsx with a location-agnostic note and re-wrap it to <=100
characters; e.g., state that 'trip' may be undefined until the store hydrates
and that an early-return guard in this component ensures a non-null 'trip'
before render. Ensure the edited comment uses single quotes and keeps 2-space
indentation to satisfy the project's Biome formatting rules.
Comment on lines
+68
to
69
| // safe-cast: formattedData is shaped by weatherService which guarantees WeatherLocation structure | ||
| const newLocation = formattedData as unknown as WeatherLocation; |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether formatWeatherData is strongly typed and locate all unsafe casts.
set -euo pipefail
echo "== formatWeatherData declarations/implementations =="
rg -n -C3 'formatWeatherData\s*[:(]' --type ts --type tsx
echo
echo "== WeatherLocation type declarations =="
rg -n -C3 'export\s+(type|interface)\s+WeatherLocation\b' --type ts --type tsx
echo
echo "== Double-cast call sites to WeatherLocation =="
rg -n -C2 'as unknown as (Partial<)?WeatherLocation' --type ts --type tsxRepository: PackRat-AI/PackRat
Length of output: 161
🏁 Script executed:
#!/bin/bash
# Corrected script - use glob patterns for TypeScript files instead of --type tsx
set -euo pipefail
echo "== formatWeatherData declarations/implementations =="
rg -n -C3 'formatWeatherData\s*[:(]' -g '*.ts' -g '*.tsx'
echo ""
echo "== WeatherLocation type declarations =="
rg -n -C3 'export\s+(type|interface)\s+WeatherLocation\b' -g '*.ts' -g '*.tsx'
echo ""
echo "== Double-cast call sites to WeatherLocation =="
rg -n -C2 'as unknown as (Partial<)?WeatherLocation' -g '*.ts' -g '*.tsx'
echo ""
echo "== Check formatWeatherData return type and any usage =="
rg -n 'function formatWeatherData|const formatWeatherData|export.*formatWeatherData' -g '*.ts' -g '*.tsx' -A5Repository: PackRat-AI/PackRat
Length of output: 7012
🏁 Script executed:
#!/bin/bash
# Get the complete formatWeatherData function implementation
rg -n 'export function formatWeatherData' -g '*.ts' -g '*.tsx' -A30Repository: PackRat-AI/PackRat
Length of output: 2606
🏁 Script executed:
#!/bin/bash
# Get the complete formatWeatherData function with return statement
sed -n '51,150p' apps/expo/features/weather/lib/weatherService.tsRepository: PackRat-AI/PackRat
Length of output: 3444
🏁 Script executed:
#!/bin/bash
# Get the complete WeatherLocation interface definition
sed -n '176,220p' apps/expo/features/weather/types.tsRepository: PackRat-AI/PackRat
Length of output: 881
Add explicit return type to formatWeatherData to eliminate unsafe casts across all call sites.
The function at apps/expo/features/weather/lib/weatherService.ts:51 lacks an explicit return type annotation, forcing callers to use as unknown as WeatherLocation at 4 separate locations. Since formatWeatherData objectively returns a WeatherLocation-shaped object, adding the return type export function formatWeatherData(data: WeatherAPIForecastResponse): WeatherLocation will allow TypeScript to enforce the contract and eliminate the need for all unsafe casts in useLocationRefresh.ts, useLocationSearch.ts, and LocationPreviewScreen.tsx.
This aligns with the coding guideline: **/*.{ts,tsx} must use proper TypeScript types.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/expo/features/weather/hooks/useLocationSearch.ts` around lines 68 - 69,
Add an explicit return type to the formatWeatherData function so TypeScript
guarantees it returns a WeatherLocation and callers can stop using unsafe casts;
update the signature of formatWeatherData to export function
formatWeatherData(data: WeatherAPIForecastResponse): WeatherLocation, then
remove the "as unknown as WeatherLocation" casts at call sites (e.g., usages in
useLocationSearch, useLocationRefresh, and LocationPreviewScreen) so the
compiler enforces the shape instead of relying on runtime casts.
Comment on lines
+17
to
+28
| const response = await fetch(endpoint, { | ||
| method: 'POST', | ||
| headers: { | ||
| 'Content-Type': 'application/x-www-form-urlencoded', | ||
| 'User-Agent': USER_AGENT, | ||
| }, | ||
| body: `data=${encodeURIComponent(ql)}`, | ||
| }); | ||
|
|
||
| if (!response.ok) { | ||
| throw new Error(`Overpass request failed: ${response.status} ${response.statusText}`); | ||
| } |
Contributor
There was a problem hiding this comment.
Add a timeout to the Overpass fetch to avoid hanging request handlers.
The Overpass public API (overpass-api.de) is known to be slow or unresponsive under load — requests can hang for minutes. Without an AbortSignal.timeout, this fetch can block the calling request handler indefinitely (notably on Cloudflare Workers where wall-clock is bounded but other requests pay the cost). The sibling alltrails.ts route already uses AbortSignal.timeout(8000); consider applying the same pattern here, or making the timeout configurable via OverpassClientConfig.
🛡️ Suggested fix
export interface OverpassClientConfig {
endpoint?: string;
+ timeoutMs?: number;
}
export async function queryOverpass(
ql: string,
config?: OverpassClientConfig,
): Promise<OverpassResponse> {
const endpoint = config?.endpoint ?? DEFAULT_ENDPOINT;
+ const timeoutMs = config?.timeoutMs ?? 25_000;
const response = await fetch(endpoint, {
method: 'POST',
headers: {
'Content-Type': 'application/x-www-form-urlencoded',
'User-Agent': USER_AGENT,
},
body: `data=${encodeURIComponent(ql)}`,
+ signal: AbortSignal.timeout(timeoutMs),
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/overpass/src/client.ts` around lines 17 - 28, The fetch to Overpass
(the call using fetch(endpoint, { method: 'POST', ... body:
`data=${encodeURIComponent(ql)}` })) can hang; wrap it with an AbortSignal (e.g.
add signal: AbortSignal.timeout(timeoutMs)) and use a default timeout (8000ms)
or read timeoutMs from OverpassClientConfig passed into the client/constructor;
ensure the fetch options include signal and that you throw a distinct error on
AbortError so callers can differentiate timeouts from HTTP failures (keep the
existing response.ok check and error throw for non-2xx responses).
Comment on lines
+3
to
+9
| const OSM_ROUTE_TO_SPORT: Record<string, string> = { | ||
| hiking: 'hiking', | ||
| bicycle: 'cycling', | ||
| ski: 'skiing', | ||
| running: 'running', | ||
| horse_riding: 'horse_riding', | ||
| }; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What are the standard OSM route=* tag values for hiking, cycling, skiing, running, and horse riding routes per the OpenStreetMap wiki?
💡 Result:
The standard OSM route=* tag values per the OpenStreetMap wiki are: - Hiking: route=hiking - Cycling: route=bicycle - Skiing: route=ski (or route=piste for pistes) - Running: route=running - Horse riding: route=horse
Citations:
- 1: http://wiki.openstreetmap.org/wiki/tag:route=bicycle
- 2: https://wiki.openstreetmap.org/wiki/Tag:route%3Dhiking
- 3: https://wiki.openstreetmap.org/wiki/Tag:route%3Drunning
- 4: https://wiki.openstreetmap.org/wiki/Tag:route%3Dhorse
- 5: http://wiki.openstreetmap.org/wiki/tag:route=ski
- 6: https://wiki.openstreetmap.org/wiki/Proposal:Tag:route%3Dpiste
- 7: https://wiki.osm.org/wiki/Riding
Update horse_riding key to horse to match standard OSM route tags.
The code's OSM_ROUTE_TO_SPORT map keys must match actual OSM route=* values. Per the OpenStreetMap wiki, the standard tag for equestrian routes is route=horse, not route=horse_riding. The current mapping will not match real OSM data. The other keys (hiking, bicycle, ski, running) are correct per OSM standards.
Required fix
const OSM_ROUTE_TO_SPORT: Record<string, string> = {
hiking: 'hiking',
bicycle: 'cycling',
ski: 'skiing',
running: 'running',
- horse_riding: 'horse_riding',
+ horse: 'horse_riding',
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/overpass/src/transform.ts` around lines 3 - 9, The
OSM_ROUTE_TO_SPORT mapping uses the wrong OSM key for equestrian routes; update
the key name in the OSM_ROUTE_TO_SPORT constant from "horse_riding" to "horse"
so it matches actual OSM route=* tags (leave the value "horse_riding" or adjust
the mapped sport value if desired) and ensure any lookup logic referencing
OSM_ROUTE_TO_SPORT continues to work with the corrected key.
Comment on lines
+1
to
+11
| import { resolve } from 'node:path'; | ||
| import { defineConfig } from 'vitest/config'; | ||
|
|
||
| export default defineConfig({ | ||
| test: { | ||
| name: 'overpass-unit', | ||
| environment: 'node', | ||
| globals: true, | ||
| include: [resolve(__dirname, 'src/**/*.test.ts')], | ||
| }, | ||
| }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Vitest 3.1+ with a "type": "module" package, is __dirname available inside a vitest.config.ts loaded by Vite's config loader?
💡 Result:
No, dirname is not natively available in vitest.config.ts when the package has "type": "module", as this makes it an ES module where dirname is undefined by Node.js. Vite's config loader (used by Vitest) bundles and preprocesses the config file, injecting or replacing dirname to make it available even in ESM configs, similar to how it handles CJS globals in other contexts. Evidence from sources: - Vite docs and issues confirm Vite preprocesses config files to support dirname/filename in ESM syntax by bundling/replacing them (13,19,29). - Vitest shares Vite's config loading mechanism (4), and issues show dirname works in vitest.config.ts (9 mentions it's correct there). - In tests (via vite-node), Vitest intentionally injects dirname for bundler compatibility, even in ESM (1). - Errors like "dirname is not defined" occur in plain Node ESM or certain loaders (runner mode), but not in standard Vite/Vitest config loading (13,17). - Common practice in configs uses resolve(dirname, ...) successfully (6,11,22,27). For Vitest 3.1+, use the standard polyfill if needed: import { dirname } from 'node:path' import { fileURLToPath } from 'node:url' const dirname = dirname(fileURLToPath(import.meta.url)) But direct dirname should work due to Vite's processing.
Citations:
- 1: Vitest incorrectly injects __dirname when type is module vitest-dev/vitest#2841
- 2: https://main.vitest.dev/config
- 3: __dirname and import.meta.url is wrong vitest-dev/vitest#1534
- 4: "__dirname is not defined in ES module scope" in vite.config.js with module type in package.json vitejs/vite#6946
- 5: feat: use module runner to import the config vitejs/vite#18637
- 6: https://v6.vite.dev/config
- 7: loadConfigFromFile API can not loadConfig have loadEnv func in vite.config.ts? vitejs/vite#15253
- 8: fix: load esm vite config without a temporary file vitejs/vite#17894
- 9: feat: better config
__dirnamesupport vitejs/vite#8442
🏁 Script executed:
# Check if the package.json file exists and contains "type": "module"
cat packages/overpass/package.json | head -20Repository: PackRat-AI/PackRat
Length of output: 433
🏁 Script executed:
# Also verify the vitest.config.ts file matches the review
cat packages/overpass/vitest.config.tsRepository: PackRat-AI/PackRat
Length of output: 321
🏁 Script executed:
# Search for other vitest.config files in the repo to see common patterns
fd -e vitest.config.ts -o -e vitest.config.jsRepository: PackRat-AI/PackRat
Length of output: 179
🏁 Script executed:
# Find all vitest.config files
find . -name "vitest.config.*" -type fRepository: PackRat-AI/PackRat
Length of output: 230
🏁 Script executed:
# Check Vitest documentation via web search for include pattern best practices
```web_search
Vitest test.include glob pattern relative path vs absolute path best practices
```Repository: PackRat-AI/PackRat
Length of output: 158
🏁 Script executed:
# Check if there are any issues or errors related to __dirname in this repo
git log --all --oneline --grep="__dirname\|vitest.config" 2>/dev/null | head -20Repository: PackRat-AI/PackRat
Length of output: 1554
🏁 Script executed:
# Check all other vitest.config files
for file in ./apps/expo/vitest.config.ts ./packages/analytics/vitest.config.ts ./packages/api/vitest.config.ts ./packages/mcp/vitest.config.ts; do
echo "=== $file ==="
cat "$file"
echo ""
doneRepository: PackRat-AI/PackRat
Length of output: 4201
🏁 Script executed:
# Check the git history commit that mentions renaming to .mts for ESM
git show aa1c845e --stat 2>/dev/null | head -30Repository: PackRat-AI/PackRat
Length of output: 566
🏁 Script executed:
# Check if there are any .mts vitest config files
find . -name "vitest.config.mts" -type fRepository: PackRat-AI/PackRat
Length of output: 44
Use .mts extension or import.meta.url pattern, and simplify include to use relative glob.
The __dirname issue is genuine in Vitest 4: recent commits show packages/api/vitest.config.ts was renamed to .mts because "vitest 4 + rolldown bundles vitest.config.ts as CJS by default" but the config must load as ESM. To fix this:
- Rename to
vitest.config.mts, or - Replace
__dirnamewith the proper ESM pattern (as used inpackages/analytics):
-import { resolve } from 'node:path';
import { defineConfig } from 'vitest/config';
export default defineConfig({
test: {
name: 'overpass-unit',
environment: 'node',
globals: true,
- include: [resolve(__dirname, 'src/**/*.test.ts')],
+ include: ['src/**/*.test.ts'],
},
});The include pattern should use a relative glob string instead of an absolute path—this is the standard Vitest convention and matches patterns in other configs like packages/analytics.
📝 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 { resolve } from 'node:path'; | |
| import { defineConfig } from 'vitest/config'; | |
| export default defineConfig({ | |
| test: { | |
| name: 'overpass-unit', | |
| environment: 'node', | |
| globals: true, | |
| include: [resolve(__dirname, 'src/**/*.test.ts')], | |
| }, | |
| }); | |
| import { defineConfig } from 'vitest/config'; | |
| export default defineConfig({ | |
| test: { | |
| name: 'overpass-unit', | |
| environment: 'node', | |
| globals: true, | |
| include: ['src/**/*.test.ts'], | |
| }, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/overpass/vitest.config.ts` around lines 1 - 11, The config is being
emitted as CJS because Vitest expects an ESM config; change the config to ESM by
renaming vitest.config.ts to vitest.config.mts OR replace the use of __dirname
with the ESM pattern using import.meta.url (as in other packages) and ensure you
export via defineConfig; also simplify the include entry to a relative glob
string ("src/**/*.test.ts") instead of resolve(__dirname, 'src/**/*.test.ts') so
the include uses a relative path.
| // Standard React context sentinel: {} is a valid placeholder because FormField | ||
| // always wraps its children in a Provider before useFormField is called. | ||
| const FormFieldContext = React.createContext<FormFieldContextValue>({} as FormFieldContextValue); | ||
| const FormFieldContext = React.createContext<FormFieldContextValue>({} as FormFieldContextValue); // safe-cast: React context sentinel — FormField always provides a real value via Provider before consumers run |
Contributor
There was a problem hiding this comment.
Line exceeds 100-character limit.
This line is approximately 217 characters, exceeding the 100-character line width enforced by Biome 2.0. Moving the inline comment to a preceding line would resolve this violation.
As per coding guidelines: "Enforce 2 spaces indentation, 100 character line width, and single quotes via Biome 2.0"
📏 Proposed fix to move comment to preceding line
+// safe-cast: React context sentinel — FormField always provides a real value via
+// Provider before consumers run
-const FormFieldContext = React.createContext<FormFieldContextValue>({} as FormFieldContextValue); // safe-cast: React context sentinel — FormField always provides a real value via Provider before consumers run
+const FormFieldContext = React.createContext<FormFieldContextValue>(
+ {} as FormFieldContextValue,
+);📝 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 FormFieldContext = React.createContext<FormFieldContextValue>({} as FormFieldContextValue); // safe-cast: React context sentinel — FormField always provides a real value via Provider before consumers run | |
| // safe-cast: React context sentinel — FormField always provides a real value via | |
| // Provider before consumers run | |
| const FormFieldContext = React.createContext<FormFieldContextValue>( | |
| {} as FormFieldContextValue, | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/web-ui/src/components/form.tsx` at line 26, The line creating the
React context for FormFieldContext exceeds the 100-char limit; move the inline
comment to the previous line (above the const) or wrap it so the const
declaration remains <=100 chars; specifically update the line containing
FormFieldContext, React.createContext<FormFieldContextValue> and the trailing
comment so the declaration is short and the explanatory comment is on its own
line (keep existing wording and single-quote/2-space style rules).
|
|
||
| // Standard React context sentinel: FormItem always provides a real id before consumers run. | ||
| const FormItemContext = React.createContext<FormItemContextValue>({} as FormItemContextValue); | ||
| const FormItemContext = React.createContext<FormItemContextValue>({} as FormItemContextValue); // safe-cast: React context sentinel — FormItem always provides a real id via Provider before consumers run |
Contributor
There was a problem hiding this comment.
Line exceeds 100-character limit.
This line is approximately 210 characters, exceeding the 100-character line width enforced by Biome 2.0. The inline comment should be moved to a preceding line.
As per coding guidelines: "Enforce 2 spaces indentation, 100 character line width, and single quotes via Biome 2.0"
📏 Proposed fix to move comment to preceding line
+// safe-cast: React context sentinel — FormItem always provides a real id via
+// Provider before consumers run
-const FormItemContext = React.createContext<FormItemContextValue>({} as FormItemContextValue); // safe-cast: React context sentinel — FormItem always provides a real id via Provider before consumers run
+const FormItemContext = React.createContext<FormItemContextValue>(
+ {} as FormItemContextValue,
+);📝 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 FormItemContext = React.createContext<FormItemContextValue>({} as FormItemContextValue); // safe-cast: React context sentinel — FormItem always provides a real id via Provider before consumers run | |
| // safe-cast: React context sentinel — FormItem always provides a real id via | |
| // Provider before consumers run | |
| const FormItemContext = React.createContext<FormItemContextValue>( | |
| {} as FormItemContextValue, | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/web-ui/src/components/form.tsx` at line 68, The line creating
FormItemContext is over the 100-char limit because it has an inline comment;
move the trailing comment to its own preceding line (use two spaces indentation)
so the declaration reads: const FormItemContext =
React.createContext<FormItemContextValue>({} as FormItemContextValue); and place
the explanatory comment on the previous line (// safe-cast: React context
sentinel — FormItem always provides a real id via Provider before consumers run)
using single quotes where applicable and ensuring the line length is under 100
characters; keep references to FormItemContext, React.createContext, and
FormItemContextValue unchanged.
andrew-bierman
force-pushed
the
fix/cf-identity-schema
branch
from
45457e9 to
95ee5aa
Compare
github-actions
Bot
removed
dependencies
Contributor
Deploying packrat-guides with
|
| Latest commit: |
29cf5b9
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://b774281f.packrat-guides-6gq.pages.dev |
| Branch Preview URL: | https://fix-cf-identity-schema.packrat-guides-6gq.pages.dev |
…ssJWT The /cdn-cgi/access/get-identity endpoint returns user identity (email, etc.) but not the JWT assertion in the response body. The Cf-Access-Jwt-Assertion header is added automatically by CF Access on every request to a CF-protected service — no manual forwarding required. Removing the required jwt field from CFAccessIdentitySchema means useCFAccessIdentity() correctly detects an active CF session, preventing the auth redirect loop where the SPA showed the login page even after successful CF authentication.
andrew-bierman
force-pushed
the
fix/cf-identity-schema
branch
from
29cf5b9 to
236bda9
Compare
…om fetcher Treaty's headers() function is called per-request (dynamic token reads work), and onResponse() handles 401 → redirect cleanly. Removes the Object.assign typeof-fetch workaround entirely.
andrew-bierman
added a commit
that referenced
this pull request
May 14, 2026
fix(admin): CF Access identity schema — remove jwt field, drop manual JWT forwarding
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.
Root cause
CFAccessIdentitySchemarequired ajwt: z.string()field, but the/cdn-cgi/access/get-identityendpoint does not include the JWT in its response body. This causeduseCFAccessIdentity()to always return null, soAuthGuardredirected to/logineven after a valid CF Access authentication — an infinite redirect loop.What changes
cfAccess.ts: Removejwtfrom schema (keepemailas the CF session indicator), dropgetCFAccessJWTexportapi.ts: RemovebuildAuthHeaders/getCFAccessJWT;adminFetchsends the session Bearer token for local dev. In prod, theCf-Access-Jwt-Assertionheader is added automatically by CF Access on every request to a protected service — no manual forwarding needed.Test plan
/login🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Security
Infrastructure