feat(web): web support MVP — platform shims, Playwright e2e, OTP fix#2366
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds comprehensive web support and Playwright E2E infrastructure: CI workflow, authenticated global setup/fixtures, many web-specific shims/stubs, platform-aware image/layout fixes, testID instrumentation, polyfills, and a dev-only OTP endpoint plus small API/schema tweaks. Changes
Sequence Diagram(s)sequenceDiagram
participant CI as GitHub Actions (CI)
participant Builder as Expo Web Build
participant Server as Static Server (apps/expo/dist)
participant Browser as Playwright (Chromium)
participant API as Backend (API)
CI->>Builder: checkout + install deps + build web export
CI->>Server: start static server (PORT 8081)
Server-->>CI: ready
CI->>Browser: launch tests (BASE_URL -> http://localhost:8081)
Browser->>Server: request pages / static assets
Browser->>API: HTTP calls (POST/PUT/GET)
API-->>Browser: responses (used for assertions)
Browser-->>CI: upload reports/traces (artifact)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
dependencies
Contributor
Coverage Report for Expo Unit Tests Coverage (./apps/expo)
File Coverage
|
||||||||||||||||||||||||||||||||||||||
Contributor
Coverage Report for API Unit Tests Coverage (./packages/api)
File CoverageNo changed files found. |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a Web MVP for the Expo app by introducing platform-specific shims, web-only layouts/providers, and a Playwright E2E suite, plus a few backend/client fixes to support OTP and soft-deleting trips.
Changes:
- Introduce web/platform shims for native-only modules (updates/dev-client/apple-auth/picker/storage/AI/local model/image cache, etc.)
- Add a Playwright web E2E test suite + CI workflow and repo scripts for running it
- Fix/extend API + client behavior around OTP recency and trip soft-delete handling
Reviewed changes
Copilot reviewed 48 out of 49 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/env/scripts/no-raw-process-env.ts | Allows Playwright directory to read env vars in Node/CI. |
| packages/checks/src/check-type-casts.ts | Excludes Playwright tests from unsafe-cast checker. |
| packages/api/src/routes/trips/index.ts | Extends update schema to accept deleted and writes it to DB updates. |
| packages/api/src/routes/auth/index.ts | Adds dev-only endpoint to fetch latest OTP; imports desc for ordering. |
| package.json | Adds Playwright test dependency. |
| bun.lock | Locks Playwright-related packages. |
| apps/expo/vitest.config.ts | Excludes *.web.ts files from coverage runs. |
| apps/expo/providers/index.web.tsx | Web-only Providers composition (removes native-only providers). |
| apps/expo/playwright/tests/trips.spec.ts | Web E2E coverage for trips CRUD + validation. |
| apps/expo/playwright/tests/profile.spec.ts | Web E2E coverage for profile flows. |
| apps/expo/playwright/tests/packs.spec.ts | Web E2E coverage for packs/items CRUD + validation. |
| apps/expo/playwright/tests/globalSetup.ts | Global auth seeding for Playwright (token/credential/OTP fallback). |
| apps/expo/playwright/tests/fixtures.ts | Playwright fixtures to seed auth into localStorage. |
| apps/expo/playwright/tests/core.spec.ts | Broad “core app” E2E smoke coverage. |
| apps/expo/playwright/playwright.config.ts | Playwright runner configuration for the Expo web build. |
| apps/expo/package.json | Adds test:web scripts to run Playwright. |
| apps/expo/mocks/react-native-google-signin.ts | Web stub for Google Sign-In native SDK. |
| apps/expo/mocks/react-native-community-datetimepicker.tsx | Web stub rendering native date/time picker as HTML inputs. |
| apps/expo/mocks/expo-sqlite-kv-store.ts | Web/localStorage behavior changes for kv-store shim. |
| apps/expo/metro.config.js | Adds/updates module stubs for web (google-signin, datetimepicker, async-storage). |
| apps/expo/lib/utils/getRelativeTime.ts | Aligns translation function typing and makes units readonly. |
| apps/expo/lib/utils/ImageCacheManager.web.ts | Web no-op for image caching. |
| apps/expo/lib/updates.web.ts | Web updates shim (reload via window.location.reload). |
| apps/expo/lib/updates.ts | Native re-export wrapper for expo-updates. |
| apps/expo/lib/hooks/useColorScheme.web.tsx | Web color scheme hook without Android-only nav bar integration. |
| apps/expo/lib/devClient.web.ts | Web no-op shim for dev client. |
| apps/expo/lib/devClient.ts | Native import wrapper for expo-dev-client. |
| apps/expo/lib/constants.web.ts | Web constants shim (no filesystem image dir). |
| apps/expo/lib/appleAuthentication.web.ts | Web stub for Apple auth. |
| apps/expo/lib/appleAuthentication.ts | Native re-export wrapper for Apple auth. |
| apps/expo/lib/Picker.web.tsx | Web Picker shim backed by <select>. |
| apps/expo/lib/Picker.tsx | Native re-export wrapper for picker. |
| apps/expo/features/trips/screens/TripDetailScreen.tsx | Adds testIDs + edit/delete controls using NativeWindUI alert. |
| apps/expo/features/trips/components/TripForm.tsx | Uses Picker shim and adds testIDs for Playwright/Maestro alignment. |
| apps/expo/features/packs/utils/uploadImage.web.ts | Web image upload implementation via fetch PUT to presigned URL. |
| apps/expo/features/packs/hooks/usePackOwnershipCheck.ts | Makes ownership check reactive via use$. |
| apps/expo/features/auth/hooks/useAuthInit.ts | Web navigation timing workaround for expo-router readiness. |
| apps/expo/features/auth/hooks/useAuthActions.ts | Switches to platform shims for Apple auth + updates. |
| apps/expo/features/auth/atoms/authAtoms.ts | Enables getOnInit for token atoms (web hydration). |
| apps/expo/features/ai/lib/localModelManager.web.ts | Web no-op AI local model manager. |
| apps/expo/atoms/atomWithSecureStorage.web.ts | Web localStorage-based secure-storage atom shim. |
| apps/expo/app/auth/one-time-password.tsx | Guards setNativeProps usage for web. |
| apps/expo/app/_layout.web.tsx | Web root layout variant (no dev-client, no Sentry RN). |
| apps/expo/app/_layout.tsx | Switches dev-client import to platform wrapper. |
| apps/expo/app/(app)/_layout.tsx | Switches dev-client import to platform wrapper. |
| apps/expo/app/(app)/(tabs)/profile/index.tsx | Uses updates shim + adds testIDs to profile list/sign-out. |
| apps/expo/app/(app)/(tabs)/_layout.web.tsx | Web tabs layout using standard Expo Router Tabs. |
| apps/expo/.gitignore | Ignores Playwright token cache + reports/results. |
| .github/workflows/web-e2e-tests.yml | Adds CI job to export web build and run Playwright against it. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+26
to
+38
| function RootLayout() { | ||
| useInitialAndroidBarSync(); | ||
|
|
||
| appAlert = useRef<AlertMethods>(null); | ||
|
|
||
| const { colorScheme, isDarkColorScheme } = useColorScheme(); | ||
|
|
||
| return ( | ||
| <Providers> | ||
| <StatusBar | ||
| key={`root-status-bar-${isDarkColorScheme ? 'light' : 'dark'}`} | ||
| style={isDarkColorScheme ? 'light' : 'dark'} | ||
| /> |
Comment on lines
+19
to
+22
| const DB_URL = | ||
| process.env.NEON_DATABASE_URL ?? | ||
| 'postgresql://packrat-neon-db_owner:npg_6YTmFJhaSV8d@ep-billowing-rice-a4p2xhgf-pooler.us-east-1.aws.neon.tech/packrat-neon-db?sslmode=require'; | ||
|
|
Comment on lines
+8
to
+31
| type PickerProps = { | ||
| selectedValue?: string | number | null; | ||
| onValueChange?: (value: string) => void; | ||
| children?: React.ReactNode; | ||
| style?: React.CSSProperties; | ||
| }; | ||
|
|
||
| function PickerItem(_props: ItemProps) { | ||
| return null; | ||
| } | ||
|
|
||
| function Picker({ selectedValue, onValueChange, children }: PickerProps) { | ||
| const options: ItemProps[] = []; | ||
| React.Children.forEach(children, (child) => { | ||
| if (React.isValidElement<ItemProps>(child) && child.type === PickerItem) { | ||
| options.push(child.props); | ||
| } | ||
| }); | ||
|
|
||
| return ( | ||
| <select | ||
| value={selectedValue ?? ''} | ||
| onChange={(e) => onValueChange?.(e.target.value)} | ||
| style={{ |
Comment on lines
+80
to
+96
| const handleDeleteTrip = () => { | ||
| alertRef.current?.alert({ | ||
| title: t('trips.deleteTrip'), | ||
| message: t('trips.deleteTripConfirmation'), | ||
| buttons: [ | ||
| { text: t('common.cancel'), style: 'cancel' }, | ||
| { | ||
| text: t('common.delete'), | ||
| style: 'destructive', | ||
| onPress: async () => { | ||
| await deleteTrip(id as string); | ||
| router.back(); | ||
| }, | ||
| }, | ||
| ], | ||
| }); | ||
| }; |
![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 (1)
packages/api/src/routes/trips/index.ts (1)
199-220:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHard DELETE violates soft-delete requirement.
The coding guidelines state: "All user-generated content must use soft deletes (never hard DELETE)." Line 213 uses
db.delete(trips)which permanently removes the row.Since you've already added
deletedfield support in the PUT endpoint, the DELETE handler should setdeleted: trueinstead.🛠️ Proposed fix
.delete( '/:tripId', async ({ params, user }) => { const db = createDb(); const tripId = params.tripId; try { const trip = await db.query.trips.findFirst({ where: eq(trips.id, tripId), }); if (!trip) return status(404, { error: 'Trip not found' }); if (trip.userId !== user.userId) return status(403, { error: 'Forbidden' }); - await db.delete(trips).where(eq(trips.id, tripId)); + await db + .update(trips) + .set({ deleted: true, updatedAt: new Date() }) + .where(eq(trips.id, tripId)); return { success: true }; } catch (error) {Based on learnings: "Implement soft deletes for all user content in the database"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/api/src/routes/trips/index.ts` around lines 199 - 220, The delete handler currently performs a hard delete via db.delete(trips) which violates the soft-delete policy; change the logic in the .delete('/:tripId', async ({ params, user }) => { ... }) handler to perform an update that sets the trips.deleted flag to true instead of calling db.delete(trips), keeping the same ownership checks (trip.userId !== user.userId) and using the same filter eq(trips.id, tripId) to target the row; return the same success payload and preserve the existing error handling.
🤖 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/web-e2e-tests.yml:
- Line 58: Replace all mutable GitHub Action version tags with pinned commit
SHAs: update the occurrences of "actions/checkout@v6", "oven-sh/setup-bun@v2",
"actions/cache@v4", and both "actions/upload-artifact@v7" entries to use their
corresponding full commit SHAs; locate these strings in the workflow and replace
the `@v`* suffixes with the specific commit SHA for each action so the workflow
references immutable versions.
- Around line 105-110: The readiness loop using "for i in $(seq 1 30); do ...
curl -sf http://localhost:8081 ... done" can silently continue and allow the job
to proceed when the server never becomes ready; change it to fail fast by
detecting when the loop finishes without a successful curl (e.g., set a success
flag or check the final curl exit status) and immediately exit with a non-zero
code and a clear error log message so the workflow stops on timeout and surfaces
the root cause.
- Around line 99-103: Update the GitHub Actions step "Serve web app (SPA mode,
port 8081)" to pin the serve package to a known safe version instead of calling
an unpinned npx; change the run invocation that currently uses "npx serve -s
dist -l 8081 &" to use a fixed version (for example "npx serve@14.1.2 -s dist -l
8081 &") so CI fetches a deterministic release and reduces supply-chain risk.
In `@apps/expo/features/packs/utils/uploadImage.web.ts`:
- Around line 22-23: The code builds remoteFileName using userStore.id.peek()
which can be null/undefined and lead to shared/incorrect object keys; update the
upload flow (where remoteFileName is created) to check that userStore and
userStore.id.peek() are present and non-empty, and if not fail fast by throwing
or returning an error before generating the key; replace the current line that
sets remoteFileName (`const remoteFileName =
`${userStore.id.peek()}-${fileName}`;`) with a guarded path that validates
userStore and the id (or calls a helper like ensureUserId()) and only constructs
remoteFileName when the id exists.
- Around line 57-71: The urlToBlob function currently accepts malformed data:
URLs by silently decoding an empty string and does not validate fetch()
responses; update urlToBlob to (1) parse and validate the data URL properly by
ensuring url.startsWith('data:') then extracting and verifying the portion after
the comma exists and is non-empty (throw a clear error if arr[1] is missing or
not base64 when expected) before calling atob and creating the Blob, and (2)
after calling fetch(url) check res.ok and throw a descriptive error (including
status and url) if the response is not successful before calling res.blob();
keep the function name urlToBlob and ensure thrown errors include context so
callers can avoid uploading corrupted images.
In `@apps/expo/features/trips/screens/TripDetailScreen.tsx`:
- Around line 89-91: The delete handler currently only flips local state and
navigates away, so change the flow to perform a real persisted delete and wait
for its completion before calling router.back: update useDeleteTrip (the hook
used by the onPress) to call the backend mutation/API that marks a trip deleted
(or call trips.store.updateTrip with the { deleted: true } payload and ensure
trips.store.updateTrip sends the deleted field to the server), have the hook
return a promise that rejects on error, and modify the onPress to await that
promise and only call router.back() on success (and show/handle errors on
failure). Ensure you reference and update useDeleteTrip, updateTrip in
apps/expo/features/trips/hooks/useDeleteTrip.ts and
apps/expo/features/trips/store/trips.ts and add error handling/ui feedback so
navigation only occurs after a confirmed persisted delete.
In `@apps/expo/lib/Picker.web.tsx`:
- Around line 12-13: The declared prop "style?: React.CSSProperties" is not
applied in the component's render, creating a misleading API; either remove
"style" from the props interface or pass it through to the rendered element
(e.g., spread or set as style on the root JSX node) inside the component (look
for the function/component that consumes the props in Picker.web.tsx — e.g., the
component receiving props typed with "style?: React.CSSProperties") and ensure
any other places noted (lines referenced around 19 and 31-38) consistently use
or drop the prop from the props type/signature.
In `@apps/expo/lib/utils/ImageCacheManager.web.ts`:
- Around line 6-31: The web shim is missing a static singleton accessor used by
tests; add a static getInstance() method to the WebImageCacheManager class that
lazily constructs and returns a singleton (use a private/static instance
property on WebImageCacheManager) so calls to ImageCacheManager.getInstance()
work; update the export to continue exporting the class as ImageCacheManager and
you may keep the default export of a new WebImageCacheManager() or export the
singleton via getInstance(), but ensure the class contains the static
getInstance() and a static/private instance symbol to locate the singleton.
In `@apps/expo/playwright/tests/core.spec.ts`:
- Around line 25-45: The tests duplicate the "waitForResponse + goto + fill +
submit" pack-creation flow; extract or reuse the existing helper named
createPackViaForm (or implement it in fixtures.ts) and replace the repeated
inline logic in core.spec.ts (and the other ranges at 49-88 and 90-124) with
calls to that helper; ensure the helper accepts the Playwright page (authedPage)
and packName, performs the POST interception (returns the created pack
ID/response or verifies success), and update the tests to await
createPackViaForm(page, packName) instead of duplicating the
navigation/fill/submit/waitForResponse sequence.
- Line 191: The test uses brittle icon-codepoint text selectors getByText('')
and getByText(''); update the app components to add stable testID props (e.g.,
add testID="catalog:search-toggle" on the search toggle button and
testID="ai-chat:send-button" on the AI chat send button), then update the spec
to use page.getByTestId('catalog:search-toggle').first().click() and
page.getByTestId('ai-chat:send-button').click() (replace occurrences of
getByText('') and getByText('')) so tests no longer rely on icon font
codepoints.
- Line 63: The test incorrectly types the returned pack id as a number; update
the casts where packResponse.json() (and similar responses) are destructured to
use string ids instead of numbers — e.g., change the cast in core.spec.ts where
you do const { id: packId } = (await packResponse.json()) as { id: number } to
use as { id: string } for the occurrences around the current packResponse usage
(including the instances near the symbols packResponse and any subsequent
destructuring at the other two locations referenced).
In `@apps/expo/playwright/tests/globalSetup.ts`:
- Around line 19-21: The code currently hardcodes a full Neon connection string
in the DB_URL fallback; replace this with an explicit requirement for
NEON_DATABASE_URL to avoid checked-in credentials. Change the DB_URL
initialization (the DB_URL constant that currently reads
process.env.NEON_DATABASE_URL ?? '...') to throw or fail fast when
process.env.NEON_DATABASE_URL is missing (or alternatively switch the fallback
to the dev-only OTP API), so the code no longer contains any embedded DB
credentials and will error if the env var is not set.
In `@apps/expo/playwright/tests/packs.spec.ts`:
- Around line 238-240: Replace the flaky fixed delay usage of
page.waitForTimeout(1_000) with an explicit wait for the expected condition:
e.g., use await page.waitForNavigation() or await page.waitForURL(expectedUrl)
when expecting a route change, or await
page.locator('selector-for-element').waitFor({ state: 'visible' }) when waiting
for an element to appear/settle; update the instances of page.waitForTimeout in
packs.spec.ts (including the occurrence around the "empty pack name" test and
the later occurrence at lines referenced) to wait for the specific
navigation/element/network condition that the test asserts instead of using a
hardcoded timeout.
- Around line 136-143: The tests currently rely on a shared mutable variable
sharedPackId set in test.beforeEach (via createPackViaForm) which is brittle if
tests are run in parallel; instead remove sharedPackId and create the pack
inside each test (call createPackViaForm(page, packName) at the top of each test
that needs it) or, if you prefer keeping setup, attach the pack id to the
Playwright test context rather than a module-scoped variable (e.g., obtain
packId within each test from test.info() or pass it through the test fixture),
making the code changes around test.beforeEach, sharedPackId, and
createPackViaForm/authedPage.
- Around line 180-182: The test is silently swallowing failures by calling
editPromise.catch(() => null) after clicking the submit button; change this so
the test fails on API errors instead of masking them — either await editPromise
without a catch or replace the catch with one that logs the error and rethrows
(e.g., handle editPromise so it surfaces the error), keeping the submit call
(page.getByTestId('items:submit').click()) intact and referencing the existing
editPromise identifier.
In `@apps/expo/playwright/tests/profile.spec.ts`:
- Line 62: The test uses a non-deterministic value when creating newFirst
(`const newFirst = \`E2E-${Date.now()}\``); replace that with a deterministic
value (e.g., a fixed string or a value derived from a seeded RNG or mocked
clock) so reruns are reproducible. Update the setup in profile.spec.ts to stop
calling Date.now() (modify the `newFirst` assignment), and if other tests rely
on the current time, mock the clock or provide a seeded generator consistently
across the spec so the same value is produced on every run.
In `@apps/expo/playwright/tests/trips.spec.ts`:
- Around line 184-185: The dialog handler added with page.on('dialog', ...)
persists across tests; change it to a one-time handler or remove it after use:
replace page.on('dialog', (dialog) => dialog.accept()) with page.once('dialog',
(dialog) => dialog.accept()) (or register the listener and call
page.off('dialog', handler) after use) so the dialog listener does not leak
beyond the current test and only the intended Alert.alert is auto-accepted.
- Around line 171-172: The tripId variable is captured but never used; either
remove tripId and its void statement or use it to navigate directly to the trip
page instead of relying on tripName. Update the test in trips.spec.ts: if you
want to navigate by id, replace the current delete flow that searches by
tripName with a direct navigation to `/trip/${tripId}` (use the captured tripId
where the test locates the trip), or simply delete the tripId declaration and
the `void tripId;` line to avoid misleading dead code. Ensure references to
tripName or selectors are adjusted accordingly (look for tripId and tripName in
the surrounding test block).
- Around line 28-33: postPromise is currently registered before page.goto which
is confusing; instead remove the early page.waitForResponse and register the
waitForResponse at the moment you trigger the form submission (the same place
that calls the submit/click that results in the POST), then await both the
action and response together with Promise.all. Concretely: delete the
pre-navigation postPromise, and when you perform the form submit (the code that
clicks the submit button or calls the submit helper), create responseWait =
page.waitForResponse((r)=> r.url().includes('/api/trips') &&
r.request().method() === 'POST', { timeout: 20_000 }) and use await
Promise.all([responseWait, page.click(...)/submitAction]) so the POST wait is
tied directly to the submit event.
In `@packages/api/src/routes/auth/index.ts`:
- Around line 519-524: The query that selects the latest OTP should exclude
expired entries and use creation time for “latest”: modify the db.select from
oneTimePasswords (selecting oneTimePasswords.code) to add a where clause
filtering gt(oneTimePasswords.expiresAt, new Date()) in addition to
eq(oneTimePasswords.userId, user.id), and change
orderBy(desc(oneTimePasswords.expiresAt)) to
orderBy(desc(oneTimePasswords.createdAt)) (or desc(oneTimePasswords.id) if
createdAt is unavailable) while keeping limit(1) so the returned otp is the
newest non-expired code.
---
Outside diff comments:
In `@packages/api/src/routes/trips/index.ts`:
- Around line 199-220: The delete handler currently performs a hard delete via
db.delete(trips) which violates the soft-delete policy; change the logic in the
.delete('/:tripId', async ({ params, user }) => { ... }) handler to perform an
update that sets the trips.deleted flag to true instead of calling
db.delete(trips), keeping the same ownership checks (trip.userId !==
user.userId) and using the same filter eq(trips.id, tripId) to target the row;
return the same success payload and preserve the existing error handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e3cc1f6f-31d3-47c4-904a-09b28981511b
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock,!bun.lock
📒 Files selected for processing (44)
.github/workflows/web-e2e-tests.ymlapps/expo/.gitignoreapps/expo/app/(app)/(tabs)/profile/index.tsxapps/expo/app/(app)/_layout.tsxapps/expo/app/_layout.tsxapps/expo/app/auth/one-time-password.tsxapps/expo/features/ai/lib/localModelManager.web.tsapps/expo/features/auth/atoms/authAtoms.tsapps/expo/features/auth/hooks/useAuthActions.tsapps/expo/features/auth/hooks/useAuthInit.tsapps/expo/features/packs/hooks/usePackOwnershipCheck.tsapps/expo/features/packs/utils/uploadImage.web.tsapps/expo/features/trips/components/TripForm.tsxapps/expo/features/trips/screens/TripDetailScreen.tsxapps/expo/lib/Picker.tsxapps/expo/lib/Picker.web.tsxapps/expo/lib/appleAuthentication.tsapps/expo/lib/appleAuthentication.web.tsapps/expo/lib/constants.web.tsapps/expo/lib/devClient.tsapps/expo/lib/devClient.web.tsapps/expo/lib/hooks/useColorScheme.web.tsxapps/expo/lib/updates.tsapps/expo/lib/updates.web.tsapps/expo/lib/utils/ImageCacheManager.web.tsapps/expo/lib/utils/getRelativeTime.tsapps/expo/metro.config.jsapps/expo/mocks/expo-sqlite-kv-store.tsapps/expo/mocks/react-native-community-datetimepicker.tsxapps/expo/mocks/react-native-google-signin.tsapps/expo/package.jsonapps/expo/playwright/playwright.config.tsapps/expo/playwright/tests/core.spec.tsapps/expo/playwright/tests/fixtures.tsapps/expo/playwright/tests/globalSetup.tsapps/expo/playwright/tests/packs.spec.tsapps/expo/playwright/tests/profile.spec.tsapps/expo/playwright/tests/trips.spec.tsapps/expo/vitest.config.tspackage.jsonpackages/api/src/routes/auth/index.tspackages/api/src/routes/trips/index.tspackages/checks/src/check-type-casts.tspackages/env/scripts/no-raw-process-env.ts
| EXPO_PUBLIC_API_URL: ${{ secrets.EXPO_PUBLIC_API_URL }} | ||
|
|
||
| - name: Checkout repository | ||
| uses: actions/checkout@v6 |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify workflows still contain mutable action tags (should return no matches after fix)
fd -e yml -e yaml .github/workflows | xargs rg -nP '^\s*uses:\s*[^@]+@v[0-9]+(\b|$)'Repository: PackRat-AI/PackRat
Length of output: 438
🏁 Script executed:
# First, read the workflow file to see what's actually on the mentioned lines
head -n 135 .github/workflows/web-e2e-tests.yml | tail -n +50Repository: PackRat-AI/PackRat
Length of output: 3086
🏁 Script executed:
# Corrected verification script with proper fd syntax
fd . '.github/workflows' -e yml -e yaml | xargs rg -nP '^\s*uses:\s*[^@]+@v[0-9]+(\b|$)'Repository: PackRat-AI/PackRat
Length of output: 3285
Pin all third-party GitHub Actions to full commit SHAs.
Lines 58, 61, 67, 123, and 131 use mutable version tags (@v*) instead of immutable commit SHAs. Replace each with a full commit SHA to prevent supply-chain attacks:
- Line 58:
actions/checkout@v6→actions/checkout@<commit-sha> - Line 61:
oven-sh/setup-bun@v2→oven-sh/setup-bun@<commit-sha> - Line 67:
actions/cache@v4→actions/cache@<commit-sha> - Line 123:
actions/upload-artifact@v7→actions/upload-artifact@<commit-sha> - Line 131:
actions/upload-artifact@v7→actions/upload-artifact@<commit-sha>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/web-e2e-tests.yml at line 58, Replace all mutable GitHub
Action version tags with pinned commit SHAs: update the occurrences of
"actions/checkout@v6", "oven-sh/setup-bun@v2", "actions/cache@v4", and both
"actions/upload-artifact@v7" entries to use their corresponding full commit
SHAs; locate these strings in the workflow and replace the `@v`* suffixes with the
specific commit SHA for each action so the workflow references immutable
versions.
Comment on lines
+99
to
+103
| - name: Serve web app (SPA mode, port 8081) | ||
| working-directory: apps/expo | ||
| # -s routes all 404s to index.html for client-side routing | ||
| run: npx serve -s dist -l 8081 & | ||
|
|
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether 'serve' is already pinned in package manifests/lockfile
fd -a 'package.json' | xargs rg -n '"serve"\s*:'
rg -n '"name":\s*"serve"|serve@' bun.lockRepository: PackRat-AI/PackRat
Length of output: 44
Pin the serve package version in CI to prevent supply-chain attacks.
Line 102 runs npx serve without pinning, which fetches an unpredictable version from npm at runtime. This creates supply-chain risk and breaks reproducibility.
Suggested change
- run: npx serve -s dist -l 8081 &
+ run: bunx --bun serve@<pinned-version> -s dist -l 8081 &Determine an appropriate version for serve and update the command accordingly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/web-e2e-tests.yml around lines 99 - 103, Update the GitHub
Actions step "Serve web app (SPA mode, port 8081)" to pin the serve package to a
known safe version instead of calling an unpinned npx; change the run invocation
that currently uses "npx serve -s dist -l 8081 &" to use a fixed version (for
example "npx serve@14.1.2 -s dist -l 8081 &") so CI fetches a deterministic
release and reduces supply-chain risk.
Comment on lines
+105
to
+110
| run: | | ||
| for i in $(seq 1 30); do | ||
| curl -sf http://localhost:8081 && echo "Server ready" && break | ||
| echo "Waiting... ($i/30)" | ||
| sleep 2 | ||
| done |
Contributor
There was a problem hiding this comment.
Fail fast when server readiness check times out.
Line 105-110 keeps going even if the server never comes up, which hides the root cause and makes failures noisier.
Suggested change
- name: Wait for web server
run: |
+ ready=0
for i in $(seq 1 30); do
- curl -sf http://localhost:8081 && echo "Server ready" && break
+ if curl -sf http://localhost:8081 > /dev/null; then
+ echo "Server ready"
+ ready=1
+ break
+ fi
echo "Waiting... ($i/30)"
sleep 2
done
+ if [ "$ready" -ne 1 ]; then
+ echo "::error::Web server did not become ready on http://localhost:8081"
+ exit 1
+ fi📝 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
| run: | | |
| for i in $(seq 1 30); do | |
| curl -sf http://localhost:8081 && echo "Server ready" && break | |
| echo "Waiting... ($i/30)" | |
| sleep 2 | |
| done | |
| run: | | |
| ready=0 | |
| for i in $(seq 1 30); do | |
| if curl -sf http://localhost:8081 > /dev/null; then | |
| echo "Server ready" | |
| ready=1 | |
| break | |
| fi | |
| echo "Waiting... ($i/30)" | |
| sleep 2 | |
| done | |
| if [ "$ready" -ne 1 ]; then | |
| echo "::error::Web server did not become ready on http://localhost:8081" | |
| exit 1 | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/web-e2e-tests.yml around lines 105 - 110, The readiness
loop using "for i in $(seq 1 30); do ... curl -sf http://localhost:8081 ...
done" can silently continue and allow the job to proceed when the server never
becomes ready; change it to fail fast by detecting when the loop finishes
without a successful curl (e.g., set a success flag or check the final curl exit
status) and immediately exit with a non-zero code and a clear error log message
so the workflow stops on timeout and surfaces the root cause.
| authedPage: page, | ||
| }) => { | ||
| test.setTimeout(60_000); | ||
| const newFirst = `E2E-${Date.now()}`; |
Contributor
There was a problem hiding this comment.
Use deterministic test data instead of Date.now() in spec execution.
Line 62 depends on wall-clock time. Use deterministic value generation so reruns are reproducible.
Suggested change
- const newFirst = `E2E-${Date.now()}`;
+ // deterministic toggle avoids clock dependence across reruns
+ const currentFirst = await page.getByTestId(testIds.profile.firstNameInput).inputValue();
+ const newFirst = currentFirst === 'E2E-First-A' ? 'E2E-First-B' : 'E2E-First-A';As per coding guidelines: "Tests must be deterministic — mock all external services and clocks."
📝 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 newFirst = `E2E-${Date.now()}`; | |
| // deterministic toggle avoids clock dependence across reruns | |
| const currentFirst = await page.getByTestId(testIds.profile.firstNameInput).inputValue(); | |
| const newFirst = currentFirst === 'E2E-First-A' ? 'E2E-First-B' : 'E2E-First-A'; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/expo/playwright/tests/profile.spec.ts` at line 62, The test uses a
non-deterministic value when creating newFirst (`const newFirst =
\`E2E-${Date.now()}\``); replace that with a deterministic value (e.g., a fixed
string or a value derived from a seeded RNG or mocked clock) so reruns are
reproducible. Update the setup in profile.spec.ts to stop calling Date.now()
(modify the `newFirst` assignment), and if other tests rely on the current time,
mock the clock or provide a seeded generator consistently across the spec so the
same value is produced on every run.
Comment on lines
+519
to
+524
| const [otp] = await db | ||
| .select({ code: oneTimePasswords.code }) | ||
| .from(oneTimePasswords) | ||
| .where(eq(oneTimePasswords.userId, user.id)) | ||
| .orderBy(desc(oneTimePasswords.expiresAt)) | ||
| .limit(1); |
Contributor
There was a problem hiding this comment.
Use createdAt for “latest” and exclude expired OTPs.
Sorting by expiresAt does not reliably mean “newest” once the table mixes flows with different TTLs. A fresh 1-hour reset code can sort behind an older 24-hour verification code, and this query can also return an already-expired OTP. Filter with gt(oneTimePasswords.expiresAt, new Date()) and order by desc(oneTimePasswords.createdAt) (or id) instead.
Suggested fix
const [otp] = await db
.select({ code: oneTimePasswords.code })
.from(oneTimePasswords)
- .where(eq(oneTimePasswords.userId, user.id))
- .orderBy(desc(oneTimePasswords.expiresAt))
+ .where(
+ and(
+ eq(oneTimePasswords.userId, user.id),
+ gt(oneTimePasswords.expiresAt, new Date()),
+ ),
+ )
+ .orderBy(desc(oneTimePasswords.createdAt))
.limit(1);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/api/src/routes/auth/index.ts` around lines 519 - 524, The query that
selects the latest OTP should exclude expired entries and use creation time for
“latest”: modify the db.select from oneTimePasswords (selecting
oneTimePasswords.code) to add a where clause filtering
gt(oneTimePasswords.expiresAt, new Date()) in addition to
eq(oneTimePasswords.userId, user.id), and change
orderBy(desc(oneTimePasswords.expiresAt)) to
orderBy(desc(oneTimePasswords.createdAt)) (or desc(oneTimePasswords.id) if
createdAt is unavailable) while keeping limit(1) so the returned otp is the
newest non-expired code.
Contributor
Deploying packrat-guides with
|
| Latest commit: |
f873e9f
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://fc3749dc.packrat-guides-6gq.pages.dev |
| Branch Preview URL: | https://feat-web-support-mvp.packrat-guides-6gq.pages.dev |
Contributor
Deploying packrat-landing with
|
| Latest commit: |
f873e9f
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://fefdbb9f.packrat-landing.pages.dev |
| Branch Preview URL: | https://feat-web-support-mvp.packrat-landing.pages.dev |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/expo/app/_layout.tsx`:
- Around line 7-17: The console.error patch currently re-wraps on every Fast
Refresh run; make it idempotent by storing the original function once and
guarding re-application with a global flag (e.g.,
window.__CONSOLE_ERROR_PATCHED__ or globalThis.__CONSOLE_ERROR_PATCHED__). On
first run capture/bind the original into _origError, replace console.error with
the wrapper that filters the specific message, set the global flag to true, and
skip re-wrapping when the flag is present to avoid stacking wrappers in
subsequent HMR/dev runs.
In `@apps/expo/playwright/tests/globalSetup.ts`:
- Line 44: The test setup is printing the secret test account email (variable
`email`) to CI logs; update the globalSetup code that calls
console.log(`[globalSetup] Logged in as ${email}`) to avoid exposing identifying
info by either removing the log entirely or replacing it with a non-identifying
message (e.g., `[globalSetup] Logged in to test account`) or redacting the value
before logging; make this change in the globalSetup function in
apps/expo/playwright/tests/globalSetup.ts where `email` is referenced.
- Around line 26-47: The browser/process leak risk is that any exception after
chromium.launch() will skip browser.close(); fix by declaring let browser; let
context; let page; before launch, then wrap the async work (chromium.launch(),
browser.newContext(), context.newPage(), page.goto(), interactions, await
context.storageState(...)) inside a try block and add a finally that awaits
page?.close(), context?.close(), and browser?.close() (or at minimum
browser?.close()) so the Chromium process is always cleaned up; reference the
existing variables browser, context, page, chromium.launch(),
context.storageState({ path: AUTH_STATE_PATH }) and browser.close() when
implementing.
In `@packages/env/src/expo-client.ts`:
- Line 21: EXPO_PUBLIC_GOOGLE_IOS_CLIENT_ID is optional in the schema but not
checked before calling GoogleSignin.configure in useAuthInit.ts; add an explicit
runtime check when Platform.OS === 'ios' to validate
EXPO_PUBLIC_GOOGLE_IOS_CLIENT_ID (from the env schema) and throw or log a clear
error if it's undefined/empty so GoogleSignin.configure is never called with
undefined, or alternatively make EXPO_PUBLIC_GOOGLE_IOS_CLIENT_ID required in
the schema to enforce a fail-fast build-time error; update either the env schema
symbol EXPO_PUBLIC_GOOGLE_IOS_CLIENT_ID or the runtime check in useAuthInit.ts
(the Platform.OS === 'ios' branch) accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0f3e11d6-daf0-4577-bad1-50668a60016a
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock,!bun.lock
📒 Files selected for processing (6)
.github/workflows/web-e2e-tests.ymlapps/expo/.gitignoreapps/expo/app/_layout.tsxapps/expo/playwright/tests/fixtures.tsapps/expo/playwright/tests/globalSetup.tspackages/env/src/expo-client.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (11)
.github/workflows/web-e2e-tests.yml (4)
121-131:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPin the artifact upload actions too.
Both
actions/upload-artifact@v7entries are mutable tags. Replace them with commit SHAs like the other workflow actions.Suggested direction
- uses: actions/upload-artifact@v7 + uses: actions/upload-artifact@<commit-sha>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/web-e2e-tests.yml around lines 121 - 131, The workflow uses mutable tags for the artifact uploads: replace both uses entries referencing actions/upload-artifact@v7 (in the steps named "Upload Playwright report on failure" and "Upload Playwright traces on failure") with the corresponding commit SHAs (pin to a specific commit) like the other actions in the workflow; update the two uses lines to the exact commit SHAs for actions/upload-artifact to ensure immutability and consistency.
50-60:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPin the workflow toolchain instead of using mutable versions.
actions/checkout@v6,oven-sh/setup-bun@v2,actions/cache@v4, andbun-version: latestall let the build environment drift. Pin the actions to commit SHAs and Bun to a fixed release.Suggested direction
- uses: actions/checkout@v6 - uses: oven-sh/setup-bun@v2 - uses: actions/cache@v4 + uses: actions/checkout@<commit-sha> + uses: oven-sh/setup-bun@<commit-sha> + uses: actions/cache@<commit-sha> @@ - bun-version: latest + bun-version: <pinned-bun-release>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/web-e2e-tests.yml around lines 50 - 60, The workflow uses mutable action refs and a floating Bun version (actions/checkout@v6, oven-sh/setup-bun@v2, actions/cache@v4 and bun-version: latest); update each uses: ref to a specific commit SHA (replace actions/checkout@v6, oven-sh/setup-bun@v2, actions/cache@v4 with their verified commit SHAs) and replace bun-version: latest with an explicit released version string (e.g., "1.x.y") to pin the toolchain; ensure the chosen SHAs correspond to the intended releases and update any tests or cache keys if needed.
106-112:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFail the job when the readiness loop times out.
If the server never comes up, the loop falls through and the tests still run against a dead endpoint. Exit non-zero once the retries are exhausted.
Suggested fix
- name: Wait for web server run: | + ready=0 for i in $(seq 1 30); do - curl -sf http://localhost:8081 && echo "Server ready" && break + if curl -sf http://localhost:8081 > /dev/null; then + echo "Server ready" + ready=1 + break + fi echo "Waiting... ($i/30)" sleep 2 done + if [ "$ready" -ne 1 ]; then + echo "::error::Web server did not become ready on http://localhost:8081" + exit 1 + fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/web-e2e-tests.yml around lines 106 - 112, The "Wait for web server" readiness loop must fail the job when it times out; modify the shell step so that if the for-loop exhausts retries without a successful curl, the step exits non‑zero. Concretely, update the loop block in the "Wait for web server" step to detect success (e.g., set a flag or test curl's return) and after the loop, if the server was not reached, call exit 1 so the workflow fails instead of proceeding to run tests against a dead endpoint.
101-104:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPin
servebefore starting the SPA server.
npx servefetches whatever npm release is current at runtime. Use a fixed version so the CI web server is reproducible.Suggested fix
- run: npx serve -s dist -l 8081 & + run: npx serve@<pinned-version> -s dist -l 8081 &🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/web-e2e-tests.yml around lines 101 - 104, The CI step that starts the SPA uses a floating npx invocation ("npx serve -s dist -l 8081 &") which pulls the latest release at runtime; change it to pin the serve package version (e.g., use "npx serve@<fixed-version> -s dist -l 8081 &") so the workflow is reproducible, or alternatively install a fixed version via npm ci before running and then call "npx serve" — update the run line in the workflow where the SPA server is started to reference the fixed version.apps/expo/playwright/tests/trips.spec.ts (2)
192-193:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse a one-shot dialog handler.
page.on('dialog', ...)stays registered for the rest of the test.page.once(...)is enough here and avoids leaking the listener into later actions.Proposed fix
- page.on('dialog', (dialog) => dialog.accept()); + page.once('dialog', (dialog) => dialog.accept());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/expo/playwright/tests/trips.spec.ts` around lines 192 - 193, The dialog handler is registered with page.on('dialog', ...) and remains active for the rest of the test; replace it with a one-shot handler by using page.once('dialog', ...) so the Alert.alert confirmation is accepted only for this interaction and the listener won't leak into subsequent steps—modify the call where page.on('dialog', (dialog) => dialog.accept()) is used (in the test setup around the trip confirmation) to use page.once instead.
175-180:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDrop the unused
tripIdplaceholder.
tripIdis only assigned tovoid tripId, which makes the comment misleading. Either remove the variable or use it to navigate directly.Proposed fix
- const tripId = await createTrip(page, { + await createTrip(page, { name: tripName, startDate: '2026-10-01', endDate: '2026-10-05', }); - void tripId; // id captured for scoping the URL; used below🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/expo/playwright/tests/trips.spec.ts` around lines 175 - 180, The local variable tripId returned from createTrip is unused and only followed by a no-op "void tripId" comment; remove the unnecessary assignment and the void line (or instead use the returned id to navigate explicitly). Locate the call to createTrip in the test (the createTrip(...) invocation and the tripId variable) and either (a) delete "const tripId = ..." and the "void tripId" line and update any comment to reflect that navigation uses other means, or (b) replace the void with a real navigation using the id (e.g., navigate to the trip URL built from tripId) so the value is actually used.apps/expo/playwright/tests/packs.spec.ts (2)
176-186:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFail on the edit response instead of swallowing it.
.catch(() => null)hides API failures and makes the later assertion misleading. Await the response and checkok().Proposed fix
- await editPromise.catch(() => null); + const editResponse = await editPromise; + expect(editResponse.ok()).toBeTruthy();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/expo/playwright/tests/packs.spec.ts` around lines 176 - 186, The test is swallowing API failures by calling .catch(() => null) on editPromise; instead await the response returned from page.waitForResponse (editPromise) after triggering the submit (page.getByTestId('items:submit').click()) and assert the response is successful by checking response.ok() (or throwing/asserting when !response.ok()), so replace the .catch(...) with code that awaits editPromise into a Response object and validates response.ok() for the '/api/packs' '/items' PUT/PATCH request.
242-243:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReplace the fixed sleeps with condition-based waits.
Both
waitForTimeout(1_000)calls keep the test slow and flaky. Wait for the expected URL or element state instead.Proposed fix
- await page.waitForTimeout(1_000); - - expect(page.url()).toBe(formUrl); + await expect(page).toHaveURL(formUrl);Also applies to: 264-265
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/expo/playwright/tests/packs.spec.ts` around lines 242 - 243, The test uses fixed sleeps via page.waitForTimeout(1_000) (in packs.spec.ts) which makes it slow and flaky; replace those calls with condition-based waits such as page.waitForURL(...) for expected navigation or page.waitForSelector(..., { state: 'visible' | 'hidden' }) / Locator.waitFor for DOM/state changes so the test proceeds as soon as the app is ready; update both occurrences (around the two waitForTimeout calls) to assert and wait for the specific URL or element state that the test expects before continuing.apps/expo/playwright/tests/globalSetup.ts (1)
18-41:⚠️ Potential issue | 🟠 Major | ⚡ Quick winClose Chromium in
finallyand remove the email log.If anything after
chromium.launch()throws, the browser leaks. The success log also prints the test-account email. Wrap the setup intry/finallyand replace the log with a non-identifying message.Proposed fix
- const browser = await chromium.launch(); - const context = await browser.newContext(); - const page = await context.newPage(); + const browser = await chromium.launch(); + try { + const context = await browser.newContext(); + const page = await context.newPage(); @@ - console.log(`[globalSetup] Logged in as ${email}`); - - await browser.close(); + console.log('[globalSetup] Logged in'); + } finally { + await browser.close(); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/expo/playwright/tests/globalSetup.ts` around lines 18 - 41, The setup currently launches Chromium with chromium.launch() but doesn’t guarantee browser.close() on errors and logs the test account email; wrap the browser/context/page flow in a try/finally so the browser is always closed (call browser.close() in the finally block) and move context.storageState({ path: AUTH_STATE_PATH }) into the try before finally, and replace console.log(`[globalSetup] Logged in as ${email}`) with a non-identifying message like console.log('[globalSetup] Auth state saved') to avoid printing the email.apps/expo/playwright/tests/core.spec.ts (2)
66-66:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse
stringfor the pack id casts.
/api/packsreturns string ids, so thesenumbercasts are misleading. Update all three destructures to{ id: string }.Proposed fix
- const { id: packId } = (await packResponse.json()) as { id: number }; + const { id: packId } = (await packResponse.json()) as { id: string };Also applies to: 106-106, 245-245
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/expo/playwright/tests/core.spec.ts` at line 66, The tests cast JSON responses as { id: number } but the API returns string ids; update all three destructured casts from { id: number } to { id: string } (e.g., the line that creates packId: const { id: packId } = (await packResponse.json()) as { id: number } should cast to { id: string }, and do the same for the other two destructures in this file). Ensure the variable names (packId and the other id destructures) remain unchanged—only change the type in the as-cast to string.
257-257:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReplace the icon glyph selector with a stable test id.
getByText('')depends on the icon font codepoint, so this test will break if the font or glyph mapping changes. Add a dedicated test id for the send button and target that instead.Proposed fix
- await page.getByText('').click(); + await page.getByTestId('ai-chat:send-button').click();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/expo/playwright/tests/core.spec.ts` at line 257, Replace the brittle icon glyph selector page.getByText('') with a stable test id: update the test to target page.getByTestId('send-button') (or getByRole with accessible name) and add a corresponding data-testid="send-button" attribute to the send button element in the UI component that renders the send icon (the component/function that currently outputs the glyph). Ensure the test uses the new test id and the component markup includes the data-testid so the selector no longer depends on the icon font codepoint.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/expo/polyfills.ts`:
- Around line 27-43: The startup currently calls setupPolyfills() without
handling rejections which can cause unhandled promise rejections and partial
global patching; update setupPolyfills and its invocation so all dynamic imports
and polyfill registrations are wrapped in a try/catch (or call
setupPolyfills().catch(...)) to handle errors, log them, and avoid applying any
polyfills if imports fail; specifically ensure the imports of
'@stardazed/streams-text-encoding' and
'react-native/Libraries/Utilities/PolyfillFunctions' and the registrations via
polyfillGlobal('TextEncoderStream'...), polyfillGlobal('TextDecoderStream'...)
and polyfillGlobal('structuredClone'...) occur only after successful import and
on error emit a clear console.error (or app logger) including the caught error
and do not leave globals partially patched.
---
Duplicate comments:
In @.github/workflows/web-e2e-tests.yml:
- Around line 121-131: The workflow uses mutable tags for the artifact uploads:
replace both uses entries referencing actions/upload-artifact@v7 (in the steps
named "Upload Playwright report on failure" and "Upload Playwright traces on
failure") with the corresponding commit SHAs (pin to a specific commit) like the
other actions in the workflow; update the two uses lines to the exact commit
SHAs for actions/upload-artifact to ensure immutability and consistency.
- Around line 50-60: The workflow uses mutable action refs and a floating Bun
version (actions/checkout@v6, oven-sh/setup-bun@v2, actions/cache@v4 and
bun-version: latest); update each uses: ref to a specific commit SHA (replace
actions/checkout@v6, oven-sh/setup-bun@v2, actions/cache@v4 with their verified
commit SHAs) and replace bun-version: latest with an explicit released version
string (e.g., "1.x.y") to pin the toolchain; ensure the chosen SHAs correspond
to the intended releases and update any tests or cache keys if needed.
- Around line 106-112: The "Wait for web server" readiness loop must fail the
job when it times out; modify the shell step so that if the for-loop exhausts
retries without a successful curl, the step exits non‑zero. Concretely, update
the loop block in the "Wait for web server" step to detect success (e.g., set a
flag or test curl's return) and after the loop, if the server was not reached,
call exit 1 so the workflow fails instead of proceeding to run tests against a
dead endpoint.
- Around line 101-104: The CI step that starts the SPA uses a floating npx
invocation ("npx serve -s dist -l 8081 &") which pulls the latest release at
runtime; change it to pin the serve package version (e.g., use "npx
serve@<fixed-version> -s dist -l 8081 &") so the workflow is reproducible, or
alternatively install a fixed version via npm ci before running and then call
"npx serve" — update the run line in the workflow where the SPA server is
started to reference the fixed version.
In `@apps/expo/playwright/tests/core.spec.ts`:
- Line 66: The tests cast JSON responses as { id: number } but the API returns
string ids; update all three destructured casts from { id: number } to { id:
string } (e.g., the line that creates packId: const { id: packId } = (await
packResponse.json()) as { id: number } should cast to { id: string }, and do the
same for the other two destructures in this file). Ensure the variable names
(packId and the other id destructures) remain unchanged—only change the type in
the as-cast to string.
- Line 257: Replace the brittle icon glyph selector page.getByText('') with a
stable test id: update the test to target page.getByTestId('send-button') (or
getByRole with accessible name) and add a corresponding
data-testid="send-button" attribute to the send button element in the UI
component that renders the send icon (the component/function that currently
outputs the glyph). Ensure the test uses the new test id and the component
markup includes the data-testid so the selector no longer depends on the icon
font codepoint.
In `@apps/expo/playwright/tests/globalSetup.ts`:
- Around line 18-41: The setup currently launches Chromium with
chromium.launch() but doesn’t guarantee browser.close() on errors and logs the
test account email; wrap the browser/context/page flow in a try/finally so the
browser is always closed (call browser.close() in the finally block) and move
context.storageState({ path: AUTH_STATE_PATH }) into the try before finally, and
replace console.log(`[globalSetup] Logged in as ${email}`) with a
non-identifying message like console.log('[globalSetup] Auth state saved') to
avoid printing the email.
In `@apps/expo/playwright/tests/packs.spec.ts`:
- Around line 176-186: The test is swallowing API failures by calling .catch(()
=> null) on editPromise; instead await the response returned from
page.waitForResponse (editPromise) after triggering the submit
(page.getByTestId('items:submit').click()) and assert the response is successful
by checking response.ok() (or throwing/asserting when !response.ok()), so
replace the .catch(...) with code that awaits editPromise into a Response object
and validates response.ok() for the '/api/packs' '/items' PUT/PATCH request.
- Around line 242-243: The test uses fixed sleeps via page.waitForTimeout(1_000)
(in packs.spec.ts) which makes it slow and flaky; replace those calls with
condition-based waits such as page.waitForURL(...) for expected navigation or
page.waitForSelector(..., { state: 'visible' | 'hidden' }) / Locator.waitFor for
DOM/state changes so the test proceeds as soon as the app is ready; update both
occurrences (around the two waitForTimeout calls) to assert and wait for the
specific URL or element state that the test expects before continuing.
In `@apps/expo/playwright/tests/trips.spec.ts`:
- Around line 192-193: The dialog handler is registered with page.on('dialog',
...) and remains active for the rest of the test; replace it with a one-shot
handler by using page.once('dialog', ...) so the Alert.alert confirmation is
accepted only for this interaction and the listener won't leak into subsequent
steps—modify the call where page.on('dialog', (dialog) => dialog.accept()) is
used (in the test setup around the trip confirmation) to use page.once instead.
- Around line 175-180: The local variable tripId returned from createTrip is
unused and only followed by a no-op "void tripId" comment; remove the
unnecessary assignment and the void line (or instead use the returned id to
navigate explicitly). Locate the call to createTrip in the test (the
createTrip(...) invocation and the tripId variable) and either (a) delete "const
tripId = ..." and the "void tripId" line and update any comment to reflect that
navigation uses other means, or (b) replace the void with a real navigation
using the id (e.g., navigate to the trip URL built from tripId) so the value is
actually used.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7d5f833d-b23f-4f7e-afde-eca1aecf6e0d
📒 Files selected for processing (31)
.github/workflows/web-e2e-tests.ymlapps/expo/app/(app)/recent-packs.tsxapps/expo/app/_layout.tsxapps/expo/app/auth/(login)/index.tsxapps/expo/app/auth/index.tsxapps/expo/app/auth/one-time-password.tsxapps/expo/components/initial/UserAvatar.tsxapps/expo/features/catalog/components/ItemReviews.tsxapps/expo/features/catalog/screens/CatalogItemsScreen.tsxapps/expo/features/pack-templates/components/AppTemplateBadge.tsxapps/expo/features/pack-templates/components/FeaturedPacksSection.tsxapps/expo/features/pack-templates/components/PackTemplateCard.tsxapps/expo/features/pack-templates/components/PackTemplateItemCard.tsxapps/expo/features/pack-templates/screens/PackTemplateDetailScreen.tsxapps/expo/features/pack-templates/screens/PackTemplateItemDetailScreen.tsxapps/expo/features/packs/components/PackCard.tsxapps/expo/features/packs/components/PackItemCard.tsxapps/expo/features/packs/components/TemplateItemsSection.tsxapps/expo/features/packs/screens/PackDetailScreen.tsxapps/expo/features/packs/screens/PackItemDetailScreen.tsxapps/expo/features/trips/components/TripForm.tsxapps/expo/features/weather/components/WeatherAuthWall.tsxapps/expo/lib/testIds.tsapps/expo/mocks/react-native-community-datetimepicker.tsxapps/expo/playwright/tests/core.spec.tsapps/expo/playwright/tests/globalSetup.tsapps/expo/playwright/tests/packs.spec.tsapps/expo/playwright/tests/trips.spec.tsapps/expo/polyfills.jsapps/expo/polyfills.tspatches/@packrat-ai+nativewindui@1.1.0.patch
💤 Files with no reviewable changes (1)
- apps/expo/polyfills.js
Comment on lines
+27
to
+43
| const setupPolyfills = async () => { | ||
| const { polyfillGlobal } = await import('react-native/Libraries/Utilities/PolyfillFunctions'); | ||
|
|
||
| const { TextEncoderStream, TextDecoderStream } = await import( | ||
| '@stardazed/streams-text-encoding' | ||
| ); | ||
|
|
||
| if (!('structuredClone' in global)) { | ||
| polyfillGlobal('structuredClone', () => structuredClone); | ||
| } | ||
|
|
||
| polyfillGlobal('TextEncoderStream', () => TextEncoderStream); | ||
| polyfillGlobal('TextDecoderStream', () => TextDecoderStream); | ||
| }; | ||
|
|
||
| setupPolyfills(); | ||
| } |
Contributor
There was a problem hiding this comment.
Handle async polyfill setup failures explicitly (Line 42).
setupPolyfills() is called without handling rejection. If any dynamic import fails, startup can produce an unhandled
promise rejection and leave globals partially patched.
Suggested fix
- setupPolyfills();
+ void setupPolyfills().catch((error: unknown) => {
+ console.error('Failed to initialize native polyfills', error);
+ });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/expo/polyfills.ts` around lines 27 - 43, The startup currently calls
setupPolyfills() without handling rejections which can cause unhandled promise
rejections and partial global patching; update setupPolyfills and its invocation
so all dynamic imports and polyfill registrations are wrapped in a try/catch (or
call setupPolyfills().catch(...)) to handle errors, log them, and avoid applying
any polyfills if imports fail; specifically ensure the imports of
'@stardazed/streams-text-encoding' and
'react-native/Libraries/Utilities/PolyfillFunctions' and the registrations via
polyfillGlobal('TextEncoderStream'...), polyfillGlobal('TextDecoderStream'...)
and polyfillGlobal('structuredClone'...) occur only after successful import and
on error emit a clear console.error (or app logger) including the caught error
and do not leave globals partially patched.
Contributor
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
packrat-admin | f873e9f | Commit Preview URL Branch Preview URL |
Jun 01 2026, 03:39 AM |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
apps/expo/features/trips/screens/TripDetailScreen.tsx (1)
80-95:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftDelete still only mutates local state before navigating away.
Line 90 awaits
useDeleteTrip(), but that hook still only flips the localdeletedflag and returns immediately. This screen backs out as if the delete persisted, so the trip can come back after sync/reload. Please wire this button to the real soft-delete persistence path and only callrouter.back()after that succeeds.🤖 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 80 - 95, The delete button currently calls deleteTrip(id) which only flips a local deleted flag; update the onPress handler in handleDeleteTrip so it invokes the real persistence soft-delete path (e.g., call the repository/API method used elsewhere like tripsRepository.softDeleteTrip or the backend client function that actually persists soft deletes) and await its promise, handle errors (show an alert via alertRef or similar) and only call router.back() after the persistence call completes successfully; ensure deleteTrip (from useDeleteTrip) is wired to that persistence function or replace its usage with the persistence method so the deletion survives sync/reload.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/expo/features/auth/atoms/authAtoms.ts`:
- Around line 14-20: tokenAtom and refreshTokenAtom are inconsistent: tokenAtom
currently has getOnInit: true but you also manually hydrate it in useAuthInit
via store.set(tokenAtom, accessToken), making getOnInit unreachable; choose one
approach. Either remove getOnInit: true from tokenAtom in authAtoms.ts and keep
the manual hydration in useAuthInit (remove the redundant flag), or remove the
manual store.set(tokenAtom, ...) call in useAuthInit so tokenAtom relies on
atomWithStorage's getOnInit behavior; leave refreshTokenAtom's getOnInit: true
as-is since it has no manual init. Ensure you update only tokenAtom or the
manual hydration in useAuthInit accordingly so both atoms follow the same
initialization strategy.
In `@apps/expo/features/packs/components/PackItemCard.tsx`:
- Around line 99-104: The delete confirmation uses hard-coded English strings;
update PackItemCard to pull all strings from i18n via useTranslation() (or the
existing t function) instead of literals: replace 'Delete item?', 'Are you sure
you want to delete this item?', 'Cancel', 'OK' with translation keys (e.g.
t('pack.deleteTitle'), t('pack.deleteMessage'), t('common.cancel'),
t('common.confirm') or your preferred keys) when calling alertRef.current?.alert
and for the button labels and onPress handler referencing deleteItem(item.id);
ensure the component imports/uses useTranslation() if not already.
In `@apps/expo/playwright/tests/core.spec.ts`:
- Around line 58-63: Replace the hardcoded test id string 'packs:name-input'
with the canonical constant testIds.packs.nameInput in all pack-creation tests:
update the selector in core.spec.ts and in the createPackViaForm helper and edit
test in packs.spec.ts to use page.getByTestId(testIds.packs.nameInput). Add an
import for the testIds constant from the testIds module at the top of each test
file where it's used and remove the hardcoded string occurrences so the tests
target the actual rendered testID.
In `@apps/expo/playwright/tests/trips.spec.ts`:
- Around line 244-253: The test uses a hard wait page.waitForTimeout(1_500)
after submitButton.click(), which is flaky; remove the wait and replace the URL
check expect(page.url()).toBe(formUrl) with a Playwright web-first assertion
such as await expect(page).toHaveURL(formUrl) (or await
expect(page).toHaveURL(/\/trip\/new/') if you prefer a contains/regex check),
keeping the surrounding submitButton.click() and the final
expect(page.url()).toContain('/trip/new') assertion intact.
---
Duplicate comments:
In `@apps/expo/features/trips/screens/TripDetailScreen.tsx`:
- Around line 80-95: The delete button currently calls deleteTrip(id) which only
flips a local deleted flag; update the onPress handler in handleDeleteTrip so it
invokes the real persistence soft-delete path (e.g., call the repository/API
method used elsewhere like tripsRepository.softDeleteTrip or the backend client
function that actually persists soft deletes) and await its promise, handle
errors (show an alert via alertRef or similar) and only call router.back() after
the persistence call completes successfully; ensure deleteTrip (from
useDeleteTrip) is wired to that persistence function or replace its usage with
the persistence method so the deletion survives sync/reload.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: cdc88ac1-ec96-47bb-9791-2f7442cd7706
📒 Files selected for processing (15)
apps/expo/app/(app)/(tabs)/profile/index.tsxapps/expo/app/auth/index.tsxapps/expo/features/auth/atoms/authAtoms.tsapps/expo/features/auth/hooks/useAuthActions.tsapps/expo/features/catalog/screens/CatalogItemsScreen.tsxapps/expo/features/packs/components/HorizontalCatalogItemCard.tsxapps/expo/features/packs/components/PackItemCard.tsxapps/expo/features/trips/components/TripForm.tsxapps/expo/features/trips/screens/TripDetailScreen.tsxapps/expo/lib/testIds.tsapps/expo/lib/utils/getRelativeTime.tsapps/expo/mocks/react-native-community-datetimepicker.tsxapps/expo/playwright/tests/core.spec.tsapps/expo/playwright/tests/packs.spec.tsapps/expo/playwright/tests/trips.spec.ts
Comment on lines
+99
to
+104
| alertRef.current?.alert({ | ||
| title: 'Delete item?', | ||
| message: 'Are you sure you want to delete this item?', | ||
| buttons: [ | ||
| { text: 'Cancel', style: 'cancel' }, | ||
| { text: 'OK', style: 'destructive', onPress: () => deleteItem(item.id) }, |
Contributor
There was a problem hiding this comment.
Localize the new delete confirmation copy.
These strings are now hard-coded in English, while the surrounding screens use useTranslation(). That will regress the web modal for non-English locales; please pull the title/message/button labels from translation keys here too.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/expo/features/packs/components/PackItemCard.tsx` around lines 99 - 104,
The delete confirmation uses hard-coded English strings; update PackItemCard to
pull all strings from i18n via useTranslation() (or the existing t function)
instead of literals: replace 'Delete item?', 'Are you sure you want to delete
this item?', 'Cancel', 'OK' with translation keys (e.g. t('pack.deleteTitle'),
t('pack.deleteMessage'), t('common.cancel'), t('common.confirm') or your
preferred keys) when calling alertRef.current?.alert and for the button labels
and onPress handler referencing deleteItem(item.id); ensure the component
imports/uses useTranslation() if not already.
andrew-bierman
force-pushed
the
feat/web-support-mvp
branch
from
9c03f9c to
418817e
Compare
andrew-bierman
force-pushed
the
feat/web-support-mvp
branch
from
6816ac5 to
e7442e8
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
andrew-bierman
force-pushed
the
feat/web-support-mvp
branch
from
e7442e8 to
b5576c1
Compare
… landed) Resolved 3 conflicts: - UserAvatar.tsx: absorbed dev's MockUser/avatarUrl refactor while preserving HEAD's Platform.select web-image shim. - TemplateItemsSection.tsx: dropped HEAD's redundant WeightUnit re-import (dev already imports from @packrat/constants); kept Platform import. - bun.lock: regenerated from dev's lockfile via bun install. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
8 tasks
Matches packageManager (bun@1.3.10) but locks the actual version used in CI / CF Pages to 1.3.14 to mirror what's already running in worktrees and mcp-cli-eden. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Substantial rebase covering 225 dev commits — #2414 type unification, #2422 single-param refactor, #2433 MCP+CLI Eden Treaty rewrite, #2439 OG meta validation, #2441/#2442 OG URL fix, plus many smaller. Conflicts resolved: - apps/expo/features/packs/utils/uploadImage.ts: kept HEAD's userId cache, used dev's object-arg getPresignedUrl call (matches function signature). - apps/expo/features/trips/hooks/useDeleteTrip.ts: kept HEAD's async + optimistic-delete comment, used dev's object-arg obs() call (matches current obs signature in apps/expo/lib/store.ts). Post-merge cleanup of dev-introduced single-param violations: - apps/expo/lib/utils/__tests__/getRelativeTime.test.ts: rewrote 3 test call sites to object args matching the refactored getRelativeTime. - packages/api/src/utils/__tests__/embeddingHelper.test.ts: rewrote 7 test call sites to object args matching the refactored getEmbeddingText; updated Parameters<> type indexes. - packages/overpass/src/client.test.ts: converted makeResponse to single object param and updated all 11 call sites. - scripts/lint/no-owned-max-params.ts: added apps/trails/scripts/generate-og-images.ts to EXCLUDED_FILES (same globalThis.fetch shim pattern as the existing landing/guides entries). Verification: bun install ok; bun check-types 0 errors; biome check 0 errors (2 unrelated warnings); no-owned-max-params 0 violations. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The passwordResetService mock declared timingSafeEqual as (a, b) but the
real util (and the service call site) take a single { a, b } object, so the
mock returned false on matching OTPs and 5 verifyOtpAndResetPassword tests
threw 'Invalid or expired reset code' before reaching their assertions.
…rt-mvp # Conflicts: # packages/api/src/utils/__tests__/embeddingHelper.test.ts
_layout.web.tsx was missing the dark-mode class sync effect present in the native _layout.tsx, so darkMode: 'class' styling could get out of sync on web. Mirrors the native layout's effect.
globalSetup embedded a literal '***REDACTED_DB_URL***' fallback that would silently pass a bogus connection string to neon() when NEON_DATABASE_URL is unset. Require the env var and throw a clear error instead.
…harness The web support PR's Playwright globalSetup logged in via the old custom /api/auth/login endpoints, which the development merge replaced with Better Auth (/api/auth/sign-in/email) — so every web e2e run 404'd before any test. Several cross-origin gaps also blocked the web app from talking to the API. API: - Route /api/auth/** through Elysia instead of dispatching before it, so the existing cors plugin applies its credentialed-CORS policy + OPTIONS preflight to auth routes (previously they got no CORS headers at all). Removes the hand-rolled withAuthCors shim. - trustedOrigins trusts http://localhost:* in development so the e2e web app passes Better Auth's CSRF check. Client: - api-client sends credentials:'include' on the no-bearer (web) path. On web the Better Auth session is an HttpOnly cookie JS can't read, so cookie auth is the only path; native is unchanged (still bearer-only). E2E harness: - globalSetup signs in via Better Auth and saves browser storage state; fixtures load it. playwright.config honors PW_CHANNEL for system Chrome locally. Validated locally: sign-in 200 with CORS via the Elysia plugin and the auth/page-load e2e tests pass. Data-mutation tests still pending.
…ecureStore Web mutations silently no-op'd: the api client read the bearer token from expo-secure-store, whose web build is an empty stub that throws on call — so every authenticated request errored before it was sent, and Legend-State sync (gated on isAuthed) never POSTed. Root-caused via the e2e harness. - Add a lib/secureStore wrapper (+ .web variant backed by localStorage) and route the api client, auth client, and atomWithSecureStorage through it so secure storage works on both platforms without per-call platform branches. - auth client sends credentials:'include' so getSession() includes the Better Auth session cookie cross-origin (web has no JS-readable token). - Enforce the convention: scripts/lint/no-direct-wrapped-imports.ts fails if a wrapped module (expo-secure-store/-apple-authentication/-updates) is imported directly instead of from lib/. Wired into lint:custom. Also: - db: recycle the local pg connection per use (maxUses:1) + keepAlive so a wrangler-dev worker against raw Postgres doesn't reuse sockets miniflare drops (~50% query failures locally). Gated to localhost; prod uses Hyperdrive/Neon. - e2e: globalSetup signs in via Better Auth and saves the session-cookie storage state (the app rehydrates auth from it); fix wrong pack-name-input locators. Validated locally: create-a-pack e2e passes end-to-end; auth/page-load tests green.
…onnections Add a non-deployed `local` wrangler env that fronts the local Postgres with a Hyperdrive binding (localConnectionString), mirroring how production should front Neon. enrichEnv maps DB_HYPERDRIVE.connectionString → NEON_DATABASE_URL, the same pattern already used for OSM_HYPERDRIVE. Run with `wrangler dev -e local`. Also set the pg pool to maxUses:1 (open a fresh connection per use). This is the correct client pattern when Hyperdrive pools at the edge, and it avoids the workerd/miniflare behaviour where a pooled TCP socket is dropped between requests and the next acquire times out. Only the pg path (Hyperdrive/standard Postgres) is affected; Neon's serverless driver is untouched. Makes local auth + pack-CRUD e2e reliable. (Full local suite still needs real AI keys for embeddings and seeded catalog data — those are validated in CI.)
…perdrive
The previous local-e2e approach connected wrangler dev straight to a raw Postgres
via node-postgres, which workerd silently drops sockets on ("unexpected EOF",
~50% query timeouts under load). Earlier commits tried to paper over it with
maxUses:1 and a local Hyperdrive binding — neither reliable.
Adopt the pattern that already existed on feat/web-e2e-fix: run the official
local Neon HTTP proxy (ghcr.io/timowilhelm/local-neon-http-proxy via
docker-compose.test.yml) and route @neondatabase/serverless at it when
NEON_DATABASE_URL points to db.localtest.me. The app then uses the exact same
Neon HTTP/WS driver as prod — no raw TCP sockets, no socket drops.
- db/index.ts: db.localtest.me routes to the neon driver (not pg.Pool); drop maxUses.
- index.ts: maybeConfigureLocalNeon() points neonConfig at the local proxy.
- Remove the unused local-only Hyperdrive env + DB_HYPERDRIVE binding/type.
- Add docker-compose.test.yml + Dockerfile.test.
Local result: rock-solid (20/20 seq, 10/10 concurrent, 0 timeouts). Suite goes
from flaky to 9/14 passing; the remaining 5 are all real-OpenAI-key/catalog-data
dependent (item create, catalog, AI chat) — validated in CI.
…B learnings - integration-issues: web Better Auth cross-origin failure — CORS-via-Elysia, trustedOrigins, credentials:'include' on both clients, expo-secure-store web stub throws (fixed via lib/secureStore wrapper). - developer-experience: use the local Neon HTTP proxy (db.localtest.me) for Workers+Neon local/e2e instead of raw node-postgres in workerd. - CLAUDE.md: add a Documented Solutions pointer so agents discover docs/solutions/.
…rt-mvp # Conflicts: # packages/overpass/src/client.test.ts
github-actions
Bot
added
the
documentation
The web secure-store shim must mirror expo-secure-store's positional (key, value) API, so setItemAsync legitimately takes two params. Add it to the EXCLUDED_FILES allowlist (same rationale as the R2 positional-API shim).
andrew-bierman
added a commit
that referenced
this pull request
Jun 1, 2026
development advanced by 1010 commits since our last merge, largely landing the same web-support work via #2366 plus the turborepo migration #2200, @packrat/typescript-config #2527, per-package check-types #2532, bun pin via .tool-versions #2534, and the schemas refactor 0cdf8b2. Conflict resolution policy: - Take development's version everywhere by default (fly on the wall). - Take HEAD only for files that carry post-merge fixes unique to this branch, i.e. the three commits ab6328e / 006bcdd / 522c805: apps/expo/lib/api/packrat.ts (web token cache), apps/expo/lib/auth-client.ts (credentials:include), apps/expo/mocks/react-native-community-datetimepicker.tsx (forward testID), apps/expo/providers/index.web.tsx (BottomSheet + ActionSheet), apps/expo/features/trips/hooks/useDeleteTrip.ts (DELETE via apiClient), apps/expo/features/trips/screens/TripDetailScreen.tsx (window.confirm on web), apps/expo/playwright/tests/trips.spec.ts (date-input via forwarded testID + DELETE wait + explicit goto), apps/expo/app/(app)/ ai-chat.tsx (async headers + tokenRef), packages/api/src/auth/index.ts (rate limit disabled on localhost). - Root package.json: keep our kysely override + drop the now-unused patchedDependencies block + drop development's nativewindui 2.0.3-2 pin (we're on 2.0.6 via #2528). Regenerated bun.lock. - Removed the orphan patches/@PackRat-AI%2Fnativewindui@2.0.3.patch brought back by the modify/delete conflict (no longer referenced from package.json).
This was referenced Jun 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
lib/*.ts+lib/*.web.ts):Picker,appleAuthentication,updates,devClient— replaced 4 Metro-level stubs with proper platform abstractionsapp/_layout.web.tsxandapp/(app)/(tabs)/_layout.web.tsx(no NativeTabs on web)atomWithSecureStorage.web.ts,uploadImage.web.ts,localModelManager.web.ts,providers/index.web.tsxapps/expo/playwright/): auth, packs, trips, profile flows with CI workfloworderBy desc(expiresAt)to fetch newest code, not oldesttrips/index.ts: suppress deleted-trip 404sone-time-password.tsx: guardsetNativePropson webprofile/index.tsx:expo-updates→lib/updates;DataItemgets optionaltestIDfieldTest plan
bun check-typespassesbun test:webfromapps/expo🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores
Update — web auth now works cross-origin + reliable local e2e
This branch was extended to make the web app actually authenticate against the Workers/Elysia/Better Auth API and to make the web-e2e suite runnable.
Web auth (was silently broken on web):
/api/auth/*through Elysia so thecorsplugin + OPTIONS preflight apply (previously dispatched before Elysia → no CORS headers).trustedOriginstrustshttp://localhost:*in development (cross-origin CSRF).credentials:'include'on both the api client and the Better Auth client (web session is an HttpOnly cookie; no JS-readable token).lib/secureStorewrapper (+.web) —expo-secure-store's web build is an empty stub that throws, so the api client'sgetAccessTokenthrew before any request fired. Routed api-client/auth-client/atoms through the wrapper.scripts/lint/no-direct-wrapped-imports.ts(wired intolint:custom) enforces the lib/ wrapper convention.e2e harness:
globalSetupsigns in via Better Auth/api/auth/sign-in/emailand saves the session-cookie storage state; fixtures load it; fixed pack-name locators;PW_CHANNELfor system Chrome locally.Reliable local DB: use the local Neon HTTP proxy (
db.localtest.meviapackages/api/docker-compose.test.yml+maybeConfigureLocalNeon) so wrangler dev uses the same@neondatabase/serverlessdriver as prod — no raw node-postgres TCP sockets (which workerd drops). Local suite: rock-solid DB, 9/14 core e2e passing; the remaining 5 need real OpenAI keys + catalog data (validated in CI).Docs:
docs/solutions/entries for the web cross-origin auth fix and the local-neon-proxy setup;CLAUDE.mdpointer to the knowledge store.