📋 Daily Codebase Review - 2026-02-04

[claude]

Executive Summary

This codebase review of the Sobers recovery app (Expo 54/React Native 0.81) reveals a well-maintained, production-ready application with solid architecture, comprehensive testing (97 test files), and proper error tracking integration. The main areas requiring attention are: (1) a security concern with Math.random() for invite codes, (2) privacy-sensitive Sentry configuration, and (3) skipped tests that need resolution.

Health Score: B+

Justification: Strong fundamentals with excellent test coverage structure, proper logging patterns, and good separation of concerns. Deductions for the security issue with random invite code generation, sendDefaultPii: true in Sentry config, and skipped tests. The codebase shows active maintenance with recent improvements.

---

Top 5 Priority Items

Priority	Finding	Severity	Effort
1	Security: Math.random() used for invite code generation	HIGH	Low
2	Privacy: Sentry sendDefaultPii: true sends PII	MEDIUM	Low
3	Bug: useFrameworkReady hook missing dependency array	MEDIUM	Low
4	Testing: Skipped MeetingsScreen tests need fixing	MEDIUM	Medium
5	Code Smell: SettingsContent.tsx setTimeout without cleanup	LOW	Low

---

Detailed Findings

🔴 Security Issues

1. Insecure Random Number Generation for Invite Codes

• File: app/(app)/(tabs)/profile/index.tsx:169
• Issue: Uses Math.random().toString(36).substring(2, 10).toUpperCase() for invite code generation
• Risk: Math.random() is not cryptographically secure and produces predictable values
• Recommendation: Use crypto.getRandomValues() for secure random generation
• Note: Issue security: Replace Math.random() with crypto.getRandomValues() for invite codes #320 already tracks this - verify if implemented

2. Sentry PII Configuration

• File: lib/sentry.ts:78
• Issue: sendDefaultPii: true sends user IP addresses and other PII to Sentry
• Risk: Privacy compliance concerns (GDPR, CCPA)
• Recommendation: Set to false and rely on privacy hooks for selective data collection
• Note: Previous issue security: Disable sendDefaultPii in Sentry configuration #348 tracked this - verify current state

🟡 Code Health Issues

3. Missing Dependency Array in useFrameworkReady

• File: hooks/useFrameworkReady.ts:10-12
• Issue: useEffect called without dependency array causes re-run on every render
• Code:

  useEffect(() => {
    window.frameworkReady?.();
  }); // Missing []

• Recommendation: Add empty dependency array [] for mount-only behavior
• Note: Issue 🐛 useFrameworkReady hook missing dependency array causes effect to run on every render #308 already tracks this

4. Skipped Tests in MeetingsScreen

• File: __tests__/app/program/meetings.test.tsx:106
• Issue: Entire MeetingsScreen test suite is skipped with describe.skip
• Reason: "Fix mock setup - tests fail with Unable to find node on an unmounted component"
• Impact: Reduced test coverage for critical meeting tracking feature
• Note: Issues test: Fix skipped MeetingsScreen tests in meetings.test.tsx #354, 🧪 Fix skipped MeetingsScreen tests #360, test: Fix skipped MeetingsScreen tests in meetings.test.tsx #344 track this

5. setTimeout Without Cleanup

• File: components/settings/SettingsContent.tsx:607
• Issue: setTimeout(() => setCopiedField(null), 2000) without cleanup
• Risk: Potential memory leak if component unmounts before timeout
• Recommendation: Use useRef to track timeout and clear in cleanup

🟢 Positive Observations

1. No console.log in production code - Proper logger usage enforced
2. No hardcoded secrets - Environment variables properly used
3. No dangerouslySetInnerHTML or eval() - No XSS vectors found
4. Privacy hooks implemented - sentry-privacy.ts strips sensitive data
5. Comprehensive test structure - 97 test files covering all major areas
6. Strong TypeScript usage - Strict mode enabled
7. OAuth implementation - Robust handling with race condition guards

---

Test Coverage Analysis

• Total Test Files: 97
• Skipped Test Suites: 2 (MeetingsScreen, one navigation test)
• Coverage Goal: 80% (per CLAUDE.md)

Missing Test Coverage Areas:

• components/program/DayDetailSheet.tsx - No dedicated test file
• components/program/LogMeetingSheet.tsx - No dedicated test file
• components/program/MeetingListItem.tsx - No dedicated test file

🔄 Documentation Status

• CLAUDE.md: Current and comprehensive
• README.md: Accurate project structure, up-to-date commands
• Project structure in README: Matches actual codebase

---

Dependency Health

Key Dependencies (all recent versions):

• expo: ~54.0.32 ✅
• react: 19.1.0 ✅
• react-native: 0.81.5 ✅
• @supabase/supabase-js: ^2.93.2 ✅
• @sentry/react-native: 7.2.0 ✅
• typescript: ~5.9.3 ✅

No critical security vulnerabilities detected in primary dependencies.

---

Existing Issues Relevant to Findings

Several findings are already tracked:

• 🐛 useFrameworkReady hook missing dependency array causes effect to run on every render #308 - useFrameworkReady hook missing dependency array
• security: Replace Math.random() with crypto.getRandomValues() for invite codes #320 - Replace Math.random() with crypto.getRandomValues()
• security: Disable sendDefaultPii in Sentry configuration #348 - Disable sendDefaultPii in Sentry
• test: Fix skipped MeetingsScreen tests in meetings.test.tsx #354, 🧪 Fix skipped MeetingsScreen tests #360, test: Fix skipped MeetingsScreen tests in meetings.test.tsx #344 - Fix skipped MeetingsScreen tests
• refactor: Split SettingsContent.tsx (2,088 lines) into focused modules #346 - Split SettingsContent.tsx (2,088 lines)

---

Recommendations

Immediate Actions

1. Verify security: Replace Math.random() with crypto.getRandomValues() for invite codes #320 (Math.random replacement) is merged - HIGH priority security fix
2. Verify security: Disable sendDefaultPii in Sentry configuration #348 (sendDefaultPii: false) is merged - Privacy compliance
3. Resolve 🐛 useFrameworkReady hook missing dependency array causes effect to run on every render #308 (useFrameworkReady dependency array)

Short-term

1. Fix MeetingsScreen test mock setup to restore test coverage
2. Add missing tests for DayDetailSheet, LogMeetingSheet, MeetingListItem
3. Add setTimeout cleanup in SettingsContent.tsx

Long-term

1. Consider splitting AuthContext.tsx (647 lines) into smaller contexts
2. Implement global data fetching layer (React Query) for consistency
3. Continue addressing SettingsContent.tsx refactoring (refactor: Split SettingsContent.tsx (2,088 lines) into focused modules #346)

---

Generated by automated daily codebase review

[coderabbitai]

🔗 Related PRs

#165 - refactor: rename app from Sobriety Waypoint to Sobers [merged]
#187 - Release 1.1.0 [merged]
#192 - test: Add comprehensive unit tests for DevToolsContext, SettingsContent, and settings exports (Vibe Kanban) [merged]
#221 - test: increase test coverage across multiple components [merged]
#334 - Release: Analytics migration, 12-Step content toggle, and dependency updates [merged]

---

📝 Issue Planner

_{Check the box below or use the @coderabbitai plan command to generate an implementation plan and prompts that you can use with your favorite coding assistant.}

• Create Plan

---

🧪 Issue enrichment is currently in open beta.

You can configure auto-planning by selecting labels in the issue_enrichment configuration.

To disable automatic issue enrichment, add the following to your .coderabbit.yaml:

issue_enrichment:
  auto_enrich:
    enabled: false

💬 Have feedback or questions? Drop into our discord!