Skip to content

Instantly share code, notes, and snippets.

@wodin
Created November 30, 2025 17:11
Show Gist options
  • Select an option

  • Save wodin/a09ff82fc938f4764a42283ffdbeb0ea to your computer and use it in GitHub Desktop.

Select an option

Save wodin/a09ff82fc938f4764a42283ffdbeb0ea to your computer and use it in GitHub Desktop.
PR #54 Multi-Agent Code Review - watermelondb-expo-plugin

PR #54 Final Review - Multi-Agent Consolidated Assessment

Verdict: BLOCK MERGE

PR: #54 (v2.4.0) Branch: v2.4.0 → beta Reviewers: Security, Performance, Quality, Gemini, Codex


Executive Summary

This PR removes legacy Expo SDK <50 code and adds a new example app with Supabase sync. While the simplification is positive, critical build-breaking bugs on both Android and iOS prevent the plugin from working at all. The example app has severe security flaws and cannot be installed due to dependency conflicts.

Must fix before merge:

  1. Android builds fail (2 bugs)
  2. iOS builds fail (undocumented breaking change)
  3. Example app cannot install (wrong React/RN versions)

Critical Issues (Block Merge)

1. Android JSI Registration Broken

File: src/withWatermelon.ts:64-72 Found by: Codex, confirmed by all

// WRONG - WatermelonDBJSIPackage implements JSIModulePackage, not ReactPackage
add(WatermelonDBJSIPackage())

Impact: Kotlin compilation fails. Android builds completely broken.

Fix: Override getJSIModulePackage() instead:

override fun getJSIModulePackage(): JSIModulePackage {
    return WatermelonDBJSIPackage()
}

2. Missing Gradle Packaging Rule

File: src/withWatermelon.ts:40-52 Found by: Codex, Gemini

Removed packagingOptions { pickFirst '**/libc++_shared.so' } with no replacement or documentation.

Impact: Gradle fails with "More than one file was found with OS independent path 'lib/**/libc++_shared.so'"

Fix: Restore rule or document manual setup in README.

3. iOS simdjson Breaking Change Undocumented

File: src/withWatermelon.ts:94-123, README.md Found by: Gemini

withCocoaPods function (simdjson patching) is commented out. iOS builds fail without manual pod addition.

Impact: iOS builds fail for all users with no guidance.

Fix: Add to README:

### iOS Setup (Required)
Add to your `ios/Podfile`:
pod 'simdjson', path: File.join(...)

4. Example App Dependency Mismatch

File: example/package.json:22-24 Found by: Gemini, Codex

"react": "19.1.0",
"react-native": "0.81.4"

Expo SDK 54 requires React 18.2.0 / RN 0.75.x.

Impact: npm install fails. Example cannot be run.

Fix: npx expo install react react-native react-dom


High Severity Issues

5. No Real Authentication (Security)

File: example/src/Components/Auth.tsx, example/src/model/sync.ts Found by: Security, confirmed by all

Login accepts any string as username. This username is passed directly to Supabase RPC calls for authorization (p_record_owner).

Impact: Any user can impersonate any record owner. SQL injection risk if server uses raw concatenation.

Fix: Use Supabase Auth with JWT-based user ID, not client-provided username.

6. No Query Pagination (Performance/DoS)

File: example/app/(app)/index.tsx:10 Found by: Performance, Security

const gamesQuery = getDb().get('board_games').query(); // Loads ALL records

Impact: Memory exhaustion. Potential DoS vector.

Fix: Add .take(100) or use FlatList with observables.

7. Missing Input Validation (Security/Quality)

File: example/src/Components/BoardGameForm.tsx:44 Found by: Security, Quality

Number("abc") returns NaN, corrupting database.

Fix:

const num = parseInt(text, 10);
if (!isNaN(num) && num >= 1) setMinPlayers(num);

8. Error Message Copy-Paste Bug

File: example/src/supabase/supabase-client.ts:11 Found by: Security, Quality

Says "Missing env var: REACT_APP_SUPABASE_URL" for both URL and key.

Fix: Correct to EXPO_PUBLIC_SUPA_ANON_KEY.

9. Auth Race Condition

File: example/src/Components/Auth.tsx:26-36 Found by: Quality, Security

useEffect(() => {
  load(); // Not awaited, no error handling, no cleanup
}, []);

Impact: setState on unmounted component, inconsistent auth state.

Fix: Add cancellation token and error handling.

10. Unsafe Database Reset

File: example/src/Components/Auth.tsx:49-51 Found by: Quality, Security

unsafeResetDatabase() called without confirmation or sync check.

Impact: Data loss.

Fix: Add confirmation dialog and sync before reset.


Medium Severity Issues

11. Wrong React Key

File: example/src/Components/GameList.tsx:11 Found by: Performance

key={game.title} - non-unique, causes reconciliation bugs.

Fix: key={game.id}

12. No Sync Retry Logic

File: example/src/model/sync.ts:29-31 Found by: Quality, Performance, Security

Single failure causes permanent sync failure. No exponential backoff.

13. No Rate Limiting

File: example/app/(app)/index.tsx:23 Found by: Security, Performance

Manual sync button with no cooldown enables server overload.

14. Unused Imports

File: src/withWatermelon.ts:11-13 Found by: Quality, Performance

path, resolveFrom, insertLinesHelper imported but unused.

15. Commented Code Bloat

File: src/withWatermelon.ts:94-175 Found by: Quality, Performance

80+ lines commented instead of deleted. Unclear restoration plan.


Positive Aspects

  • Clean removal of legacy SDK <50 conditionals
  • Well-documented breaking changes in CHANGELOG
  • JSI enabled for native performance
  • Modern example app with expo-router
  • Clear separation of concerns in example
  • SQL migrations provided for Supabase setup
  • UUID v4 generation for Supabase compatibility

Reviewer Consensus

Issue Security Performance Quality Gemini Codex
Android JSI broken - - Missed Found Found
Gradle rule missing - - Missed Found Found
iOS simdjson - - Missed Found -
Deps mismatch - Missed Found Found Found
No auth Found - Found Found Found
No pagination Found Found Missed Found Found
Input validation Found Found Found Found Found
React key - Found Missed Found Found

Universal agreement: Example app is not production-ready. Build system has critical failures.


Prioritized Recommendations

P0 - Block Merge (Must Fix)

  • Fix Android JSI registration (getJSIModulePackage() override)
  • Restore Gradle packaging rule OR document manual setup
  • Document iOS simdjson requirement in README
  • Fix example dependencies (React 18.2.0, RN 0.75.x)

P1 - High Priority (Before Stable)

  • Implement real Supabase Auth OR clearly mark "DEMO ONLY"
  • Add query pagination (.take(100))
  • Add input validation
  • Fix error message copy-paste bug
  • Fix auth race condition
  • Add unsafe reset confirmation

P2 - Medium Priority (Nice to Have)

  • Fix React key (game.id)
  • Add sync retry logic with exponential backoff
  • Add rate limiting to sync button
  • Remove unused imports
  • Delete commented code

P3 - Low Priority

  • Add TypeScript types to model
  • Add tests for plugin configuration
  • Create migration guide for SDK <50 users

Alternative Path

If blocking is not acceptable, merge to beta only with:

  1. Prominent README warning: "BETA - Android/iOS builds may require manual configuration"
  2. Example app marked: "DEMO ONLY - DO NOT USE IN PRODUCTION"
  3. Issue created tracking all critical fixes

Review Metadata

Date: 2025-11-30 Method: Multi-agent parallel review with cross-validation Agents: Security, Performance, Quality, Gemini (general), Codex (general) Rounds: 2 (independent review + cross-review)

Key Insight: Cross-review was essential. Round 1 missed critical Android build failures that Codex caught. No single reviewer would have found all issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment