Conversation
Removes the anonymous visitor waitlist email capture from the public share screen and deletes the unused app-level waitlist API helper. Updates the direct waitlist UI/API tests to guard that the removed surfaces stay gone, while leaving the Supabase waitlist migration untouched.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR removes public-share waitlist capture and its API, updates related tests, configures Supabase mobile sessions to use Expo SecureStore, removes Sentry feedback integration, documents guest account-linking behavior, trims public exports and declarations, and updates CI to Node.js 22. ChangesWaitlist removal
Secure session storage
App and package surface cleanup
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant SupabaseAuth
participant secureStoreAdapter
participant SecureStore
SupabaseAuth->>secureStoreAdapter: Persist or retrieve session
secureStoreAdapter->>SecureStore: Migrate or persist session data
SecureStore-->>secureStoreAdapter: Return stored value
secureStoreAdapter-->>SupabaseAuth: Return session value
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Checkov (3.3.8).github/workflows/verify.ymlTraceback (most recent call last): Comment |
![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: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/supabase.ts`:
- Line 75: Update the Supabase storage setup around the secureStoreAdapter so
existing mobile sessions are migrated from the legacy AsyncStorage key before
SecureStore-only reads begin: read the legacy entry once, write it to
SecureStore, then remove the legacy entry, while preserving current web behavior
and avoiding repeated migration.
- Around line 17-24: Update the setItem adapter to avoid calling
SecureStore.setItemAsync for values exceeding the SecureStore limit; fail fast
or route oversized values through a storage strategy that preserves the full
payload. Replace the Android-specific warning text with platform-neutral
messaging while retaining normal persistence for values within the limit.
In `@lib/waitlist.test.js`:
- Around line 7-11: Replace formatting-dependent source regex assertions with
semantic checks across all three cleanup tests: in lib/waitlist.test.js, inspect
actual API exports or parse the API AST so re-exports and equivalent literal
syntax are covered; in __tests__/share-waitlist.test.js lines 10-15, verify the
rendered CTA/navigation is absent or inspect JSX semantically; and in lines
17-28, verify the rendered email input is absent or inspect JSX identifiers and
props semantically.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 47533293-c6b4-418f-9a8e-163cae216eb1
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (12)
__tests__/share-waitlist.test.jsapp.jsonapp/_layout.tsxapp/share/[id].tsxcomponents/home-screen.tsxlib/api.tslib/index.tslib/supabase.tslib/waitlist.test.jspackage.jsontypes/index.tstypes/react-native-vector-icons.d.ts
💤 Files with no reviewable changes (5)
- types/react-native-vector-icons.d.ts
- lib/index.ts
- types/index.ts
- lib/api.ts
- app/share/[id].tsx
📜 Review details
🔇 Additional comments (4)
app/_layout.tsx (1)
29-29: LGTM!components/home-screen.tsx (1)
56-58: LGTM!package.json (1)
41-41: LGTM!app.json (1)
50-50: LGTM!
Comment on lines
+7
to
+11
| test("does not expose an app-level public waitlist API", () => { | ||
| assert.doesNotMatch(apiSource, /type WaitlistJoinResult/); | ||
| assert.doesNotMatch(apiSource, /export async function joinWaitlist/); | ||
| assert.doesNotMatch(apiSource, /\.from\("waitlist_signups"\)/); | ||
| assert.doesNotMatch(apiSource, /source: "public_share"/); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use semantic absence checks across both cleanup tests.
Both tests rely on formatting-dependent source regexes, allowing equivalent API exports or UI implementations to evade regression coverage.
lib/waitlist.test.js#L7-L11: verify actual exports or parse the API AST, including re-exports and alternate literal syntax.__tests__/share-waitlist.test.js#L10-L15: verify rendered CTA/navigation absence or inspect JSX semantically.__tests__/share-waitlist.test.js#L17-L28: verify rendered email-input absence or inspect JSX identifiers and props semantically.
📍 Affects 2 files
lib/waitlist.test.js#L7-L11(this comment)__tests__/share-waitlist.test.js#L10-L15__tests__/share-waitlist.test.js#L17-L28
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/waitlist.test.js` around lines 7 - 11, Replace formatting-dependent
source regex assertions with semantic checks across all three cleanup tests: in
lib/waitlist.test.js, inspect actual API exports or parse the API AST so
re-exports and equivalent literal syntax are covered; in
__tests__/share-waitlist.test.js lines 10-15, verify the rendered CTA/navigation
is absent or inspect JSX semantically; and in lines 17-28, verify the rendered
email input is absent or inspect JSX identifiers and props semantically.
Use a runtime API export check for joinWaitlist removal, widen waitlist source-text checks for alternate literal syntax, and document the remaining share-screen regex limitations without adding new test dependencies.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/waitlist.test.js`:
- Around line 3-4: Update the hook import in the waitlist test to use the Node
20-compatible API instead of registerHooks, while preserving the existing test
runner import and hook behavior.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 801b48a8-f7da-4b88-8619-7a35aa52585b
📒 Files selected for processing (2)
__tests__/share-waitlist.test.jslib/waitlist.test.js
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/share-waitlist.test.js
📜 Review details
⚠️ CI failures not shown inline (2)
GitHub Actions: Verify / 0_verify.txt: Pre-resubmission cleanup: privacy fix, auth hardening, dead code removal
Conclusion: failure
##[group]Run npm run verify
�[36;1mnpm run verify�[0m
shell: /usr/bin/bash -e {0}
##[endgroup]
> best-list-app@1.0.0 verify
> npm run lint && npm run typecheck && npm test
> best-list-app@1.0.0 lint
> expo lint
> best-list-app@1.0.0 typecheck
> tsc --noEmit
> best-list-app@1.0.0 test
> node --import tsx --test lib/*.test.js components/*.test.js __tests__/*.test.js
TAP version 13
# Subtest: Expo config skips the Google iOS client ID outside EAS Build
ok 1 - Expo config skips the Google iOS client ID outside EAS Build
---
duration_ms: 1.544222
...
# Subtest: Expo config fails loudly without the Google iOS client ID during EAS Build
ok 2 - Expo config fails loudly without the Google iOS client ID during EAS Build
---
duration_ms: 0.662495
...
# Subtest: Expo config includes the Google Sign-In plugin when configured
ok 3 - Expo config includes the Google Sign-In plugin when configured
---
duration_ms: 1.134722
...
# Subtest: entry photos offer camera and photo library choices
ok 4 - entry photos offer camera and photo library choices
---
duration_ms: 1.428664
...
# Subtest: profile avatars offer camera and photo library choices
ok 5 - profile avatars offer camera and photo library choices
---
duration_ms: 0.427945
...
# Subtest: setup avatars offer camera and photo library choices
ok 6 - setup avatars offer camera and photo library choices
---
duration_ms: 0.400961
...
# Subtest: iOS and expo-image-picker declare matching photo permissions
ok 7 - iOS and expo-image-picker declare matching photo permissions
---
duration_ms: 1.070591
...
# Subtest: shows feedback when the contact form submission fails
ok 8 - shows feedback when the contact form submission fails
---
duration_ms: 2.537889
...
# Subtest: shows the same feedback if the contact form submission rejects
ok 9 - shows the same feedback if the contact form submission rejects
---
duration_ms: 0.339357
.....
GitHub Actions: Verify / verify: Pre-resubmission cleanup: privacy fix, auth hardening, dead code removal
Conclusion: failure
##[group]Run npm run verify
�[36;1mnpm run verify�[0m
shell: /usr/bin/bash -e {0}
##[endgroup]
> best-list-app@1.0.0 verify
> npm run lint && npm run typecheck && npm test
> best-list-app@1.0.0 lint
> expo lint
> best-list-app@1.0.0 typecheck
> tsc --noEmit
> best-list-app@1.0.0 test
> node --import tsx --test lib/*.test.js components/*.test.js __tests__/*.test.js
TAP version 13
# Subtest: Expo config skips the Google iOS client ID outside EAS Build
ok 1 - Expo config skips the Google iOS client ID outside EAS Build
---
duration_ms: 1.544222
...
# Subtest: Expo config fails loudly without the Google iOS client ID during EAS Build
ok 2 - Expo config fails loudly without the Google iOS client ID during EAS Build
---
duration_ms: 0.662495
...
# Subtest: Expo config includes the Google Sign-In plugin when configured
ok 3 - Expo config includes the Google Sign-In plugin when configured
---
duration_ms: 1.134722
...
# Subtest: entry photos offer camera and photo library choices
ok 4 - entry photos offer camera and photo library choices
---
duration_ms: 1.428664
...
# Subtest: profile avatars offer camera and photo library choices
ok 5 - profile avatars offer camera and photo library choices
---
duration_ms: 0.427945
...
# Subtest: setup avatars offer camera and photo library choices
ok 6 - setup avatars offer camera and photo library choices
---
duration_ms: 0.400961
...
# Subtest: iOS and expo-image-picker declare matching photo permissions
ok 7 - iOS and expo-image-picker declare matching photo permissions
---
duration_ms: 1.070591
...
# Subtest: shows feedback when the contact form submission fails
ok 8 - shows feedback when the contact form submission fails
---
duration_ms: 2.537889
...
# Subtest: shows the same feedback if the contact form submission rejects
ok 9 - shows the same feedback if the contact form submission rejects
---
duration_ms: 0.339357
.....
🧰 Additional context used
🪛 GitHub Actions: Verify / 0_verify.txt
lib/waitlist.test.js
[error] 3-3: Test run failed with SyntaxError: The requested module 'node:module' does not provide an export named 'registerHooks'. Import at lib/waitlist.test.js:3.
🪛 GitHub Actions: Verify / verify
lib/waitlist.test.js
[error] 3-3: Node.js test failure: SyntaxError — The requested module 'node:module' does not provide an export named 'registerHooks' (import in lib/waitlist.test.js).
🔇 Additional comments (1)
lib/waitlist.test.js (1)
8-47: LGTM!Also applies to: 51-55
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/verify.yml (1)
23-23: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUpdate the verification design document to Node.js 22.
docs/superpowers/specs/2026-07-01-pull-request-verification-ci-design.mdstill documents Node.js 20, so the workflow and its written CI contract now disagree. Update the document or explicitly record why it intentionally remains on 20.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/verify.yml at line 23, Update the Node.js version reference in the verification design document to 22 so it matches the workflow’s node-version setting. If the document must remain on 20, explicitly document the reason for the intentional mismatch.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/workflows/verify.yml:
- Line 23: Update the Node.js version reference in the verification design
document to 22 so it matches the workflow’s node-version setting. If the
document must remain on 20, explicitly document the reason for the intentional
mismatch.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1956ad41-b321-4466-9869-260c3d45d908
📒 Files selected for processing (1)
.github/workflows/verify.yml
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
1 participant
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.
Remove public share waitlist capture — the "Join the App Store waitlist" email capture on the public share screen collected emails from anonymous visitors without being disclosed in the privacy policy. Same category of issue as the original 7/8 withdrawal, so removing it outright rather than adding a disclosure. Will be replaced with a real App Store download link once approved.
Remove unused Sentry feedback integration — Sentry.feedbackIntegration() was registered but never wired to any UI; the actual bug-report flow (app/report-a-bug.tsx) uses Sentry.captureFeedback() directly. Dead code since 7/9.
Use SecureStore for Supabase auth sessions — login tokens were stored in plain AsyncStorage; now encrypted via the iOS Keychain / Android Keystore (expo-secure-store). Hardening only, not an Apple-flagged issue.
Update guest sign-up comment — corrected a stale code comment claiming guest→account sign-up loses data. It doesn't; linkGuestEmail already preserves the guest's list via email linking. Comment predated that fix.
chore: remove unused barrel and icon type files — deleted lib/index.ts, types/index.ts, and types/react-native-vector-icons.d.ts, confirmed unused via a full dead-code audit (zero imports anywhere in the repo).