-
Notifications
You must be signed in to change notification settings - Fork 210
Release to Staging - 2026-01-10 #1576
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…add unit tests (#1572) * Add staleness hook test * format
* Add keychain error tests * format
Update build numbers and deployment timestamps after successful deployment. Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
📝 WalkthroughWalkthroughRenames the staleness-check hook to Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/stores/database.ts (1)
45-48: Remove sessionId from error logs to comply with security guidelines.The coding guidelines explicitly state: "NEVER log sensitive data including PII (names, DOB, passport numbers, addresses), credentials, tokens, API keys, private keys, or session identifiers." Logging the sessionId creates a compliance and privacy risk if logs are exposed.
🔒 Proposed fix to redact sessionId
} catch (error) { console.error( - `Failed to update proof status for session ${sessionId}:`, + 'Failed to update proof status for session:', error, ); // Continue with the next iteration instead of stopping the entire loopAs per coding guidelines: sensitive data logging prohibition.
🤖 Fix all issues with AI agents
In @app/src/providers/passportDataProvider.tsx:
- Around line 93-122: In handleKeychainReadError, avoid double-logging by making
the final generic console.log conditional: check if error was handled by
isUserCancellation or isKeychainCryptoError (or simply return after handling
those cases) so the generic log only runs for unrecognized errors; update the
function around the isUserCancellation and isKeychainCryptoError branches (and
any throwOnUserCancel behavior) to either return after handling or use an
else/else-if before the final console.log, and keep notifyKeychainFailure calls
unchanged.
🧹 Nitpick comments (2)
app/tests/src/utils/keychainErrors.test.ts (1)
11-26: Consider adding edge case tests fornull,undefined, and primitive inputs.The current tests cover the expected object shapes well, but the utility functions accept
unknown. Adding tests fornull,undefined, and primitive values (e.g., strings, numbers) would ensure robustness when unexpected error types propagate.💡 Suggested additional test cases
it('handles null, undefined, and primitives gracefully', () => { expect(isUserCancellation(null)).toBe(false); expect(isUserCancellation(undefined)).toBe(false); expect(isUserCancellation('some string')).toBe(false); expect(isUserCancellation(42)).toBe(false); expect(isKeychainCryptoError(null)).toBe(false); expect(isKeychainCryptoError(undefined)).toBe(false); expect(getKeychainErrorIdentity(null)).toEqual({ code: undefined, name: undefined }); expect(getKeychainErrorIdentity(undefined)).toEqual({ code: undefined, name: undefined }); });app/src/providers/passportDataProvider.tsx (1)
77-79: Consider using the exportedKeychainErrorTypefor callback signature consistency.The callback type at line 78 uses an inline union
'user_cancelled' | 'crypto_failed'instead of the newly exportedKeychainErrorType. Using the exported type would ensure consistency and reduce maintenance burden if the type changes.♻️ Suggested refactor
-let keychainCryptoFailureCallback: - | ((errorType: 'user_cancelled' | 'crypto_failed') => void) - | null = null; +let keychainCryptoFailureCallback: + | ((errorType: KeychainErrorType) => void) + | null = null; export function setPassportKeychainErrorCallback( - callback: ((errorType: 'user_cancelled' | 'crypto_failed') => void) | null, + callback: ((errorType: KeychainErrorType) => void) | null, ) { keychainCryptoFailureCallback = callback; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
app/src/hooks/useProofDisclosureStalenessCheck.tsapp/src/providers/authProvider.tsxapp/src/providers/passportDataProvider.tsxapp/src/screens/verification/DocumentSelectorForProvingScreen.tsxapp/src/screens/verification/ProveScreen.tsxapp/src/stores/database.tsapp/src/utils/keychainErrors.tsapp/tests/src/hooks/useProofDisclosureStalenessCheck.test.tsapp/tests/src/utils/keychainErrors.test.tsapp/version.json
💤 Files with no reviewable changes (2)
- app/src/screens/verification/DocumentSelectorForProvingScreen.tsx
- app/src/screens/verification/ProveScreen.tsx
🧰 Additional context used
📓 Path-based instructions (15)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,jsx,ts,tsx}: NEVER log sensitive data including PII (names, DOB, passport numbers, addresses), credentials, tokens, API keys, private keys, or session identifiers.
ALWAYS redact/mask sensitive fields in logs using consistent patterns (e.g.,***-***-1234for passport numbers,J*** D***for names).
Files:
app/src/utils/keychainErrors.tsapp/tests/src/utils/keychainErrors.test.tsapp/src/providers/authProvider.tsxapp/src/stores/database.tsapp/src/hooks/useProofDisclosureStalenessCheck.tsapp/src/providers/passportDataProvider.tsxapp/tests/src/hooks/useProofDisclosureStalenessCheck.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx,js,jsx}: Use React Navigation withcreateStaticNavigationfor type-safe navigation in React Native applications.
Implement platform-specific handling withPlatform.OS === 'ios' ? 'iOS' : 'Android'checks before platform-specific code in React Native.
Initialize native modules withinitializeNativeModules()before any native operations in React Native.
Implement lazy loading for screens usingReact.lazy()in React Native applications.
Implement custom modal system withuseModalhook and callback registry in React Native.
Integrate haptic feedback usinguseHapticNavigationhook in React Native navigation.
Use platform-specific initial routes: web uses 'Home', mobile uses 'Splash' in React Navigation.
Use Zustand for global state management in React Native applications.
Use custom hooks for complex state (useModal,useHapticNavigation) instead of inline logic.
Use AsyncStorage for simple data, SQLite for complex data, and Keychain for sensitive data in React Native.
Use@/alias for src imports and@tests/alias for test imports in TypeScript/JavaScript files.
Use conditional rendering with Platform.OS for platform-specific code in React Native.
Use Tamagui for UI components in React Native applications.
Do not log sensitive data in production, including identity verification and passport information.
Use Keychain for secure storage of sensitive data in React Native.
Implement proper cleanup of sensitive data after use.
Implement certificate validation for passport data verification.
Always use try-catch for async operations in React Native and TypeScript code.
Implement graceful degradation when native modules fail in React Native.
Provide user-friendly error messages in UI and error handlers.
Lazy load screens and components to optimize bundle size in React Native.
Prevent memory leaks in native modules in React Native.
Files:
app/src/utils/keychainErrors.tsapp/tests/src/utils/keychainErrors.test.tsapp/src/providers/authProvider.tsxapp/src/stores/database.tsapp/src/hooks/useProofDisclosureStalenessCheck.tsapp/src/providers/passportDataProvider.tsxapp/tests/src/hooks/useProofDisclosureStalenessCheck.test.ts
**/*.{tsx,jsx,ts,js}
📄 CodeRabbit inference engine (.cursorrules)
Implement proper cleanup in useEffect and component unmount hooks in React.
Files:
app/src/utils/keychainErrors.tsapp/tests/src/utils/keychainErrors.test.tsapp/src/providers/authProvider.tsxapp/src/stores/database.tsapp/src/hooks/useProofDisclosureStalenessCheck.tsapp/src/providers/passportDataProvider.tsxapp/tests/src/hooks/useProofDisclosureStalenessCheck.test.ts
**/{compliance,ofac,verification,identity,utils}/**/*.{ts,tsx,js,py}
📄 CodeRabbit inference engine (.cursor/rules/compliance-verification.mdc)
Normalize names using case-folding, Unicode NFKC normalization, and diacritics removal for OFAC matching
Files:
app/src/utils/keychainErrors.tsapp/tests/src/utils/keychainErrors.test.ts
**/{mobile,client,app,time,verification}/**/*.{ts,tsx,js,swift,kt}
📄 CodeRabbit inference engine (.cursor/rules/compliance-verification.mdc)
Use server-signed time tokens or chain block timestamps for trusted time in mobile clients, do not trust device wall-clock alone
Files:
app/src/utils/keychainErrors.tsapp/tests/src/utils/keychainErrors.test.tsapp/src/providers/authProvider.tsxapp/src/stores/database.tsapp/src/hooks/useProofDisclosureStalenessCheck.tsapp/src/providers/passportDataProvider.tsxapp/tests/src/hooks/useProofDisclosureStalenessCheck.test.ts
**/{mobile,client,app,proof,zk}/**/*.{ts,tsx,js,swift,kt}
📄 CodeRabbit inference engine (.cursor/rules/compliance-verification.mdc)
**/{mobile,client,app,proof,zk}/**/*.{ts,tsx,js,swift,kt}: Include trusted time anchor in proof generation and verify time anchor authenticity before proof generation in mobile implementations
Achieve proof generation in <60 seconds on mid-tier mobile devices
Files:
app/src/utils/keychainErrors.tsapp/tests/src/utils/keychainErrors.test.tsapp/src/providers/authProvider.tsxapp/src/stores/database.tsapp/src/hooks/useProofDisclosureStalenessCheck.tsapp/src/providers/passportDataProvider.tsxapp/tests/src/hooks/useProofDisclosureStalenessCheck.test.ts
app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (app/AGENTS.md)
Ensure
yarn typespasses (TypeScript validation) before creating a PR
Files:
app/src/utils/keychainErrors.tsapp/tests/src/utils/keychainErrors.test.tsapp/src/providers/authProvider.tsxapp/src/stores/database.tsapp/src/hooks/useProofDisclosureStalenessCheck.tsapp/src/providers/passportDataProvider.tsxapp/tests/src/hooks/useProofDisclosureStalenessCheck.test.ts
app/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (app/AGENTS.md)
app/**/*.{ts,tsx,js,jsx}: Ensure web build succeeds withyarn webbefore creating a PR
Do not include sensitive data in logs - avoid logging PII, credentials, and tokens
Usereact-native-dotenvfor environment configuration via@envimport
Confirm no sensitive data exposed before PR merge
Files:
app/src/utils/keychainErrors.tsapp/tests/src/utils/keychainErrors.test.tsapp/src/providers/authProvider.tsxapp/src/stores/database.tsapp/src/hooks/useProofDisclosureStalenessCheck.tsapp/src/providers/passportDataProvider.tsxapp/tests/src/hooks/useProofDisclosureStalenessCheck.test.ts
app/src/**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
app/src/**/*.{ts,tsx,js,jsx}: Review React Native TypeScript code for:
- Component architecture and reusability
- State management patterns
- Performance optimizations
- TypeScript type safety
- React hooks usage and dependencies
- Navigation patterns
Files:
app/src/utils/keychainErrors.tsapp/src/providers/authProvider.tsxapp/src/stores/database.tsapp/src/hooks/useProofDisclosureStalenessCheck.tsapp/src/providers/passportDataProvider.tsx
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.test.{ts,tsx,js,jsx}: UserenderHookfor testing custom React hooks instead of rendering components.
Mock console.error in tests to avoid test output clutter while testing error scenarios.
Test error boundaries and recovery mechanisms in React components.
Mock SQLite operations withexecuteSqlmethod in database tests using utilities fromtests/__setup__/databaseMocks.ts.
**/*.test.{ts,tsx,js,jsx}: Never userequire('react-native')in test files; use ES6importstatements instead to avoid nested require() calls that cause out-of-memory errors in CI/CD pipelines
Never userequire('react')in test files; use ES6import React from 'react'instead to avoid nested require() calls that cause out-of-memory errors
Files:
app/tests/src/utils/keychainErrors.test.tsapp/tests/src/hooks/useProofDisclosureStalenessCheck.test.ts
{app,packages/mobile-sdk-alpha}/**/*.{test,spec}.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never create nested require('react-native') calls in tests as this causes out-of-memory errors in CI/CD pipelines; use ES6 import statements instead and avoid dynamic require() calls in beforeEach/afterEach hooks
Files:
app/tests/src/utils/keychainErrors.test.tsapp/tests/src/hooks/useProofDisclosureStalenessCheck.test.ts
app/**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (app/AGENTS.md)
app/**/*.test.{ts,tsx,js,jsx}: Ensureyarn testpasses (unit tests) before creating a PR
Avoid nestedrequire('react')orrequire('react-native')calls in test files - use ES6 import statements instead
Put all imports at the top of test files - avoid dynamic imports in hooks
Never userequire()calls inbeforeEach/afterEachhooks in test files
Verify no memory leaks introduced, including test memory patterns
Use ES6importstatements exclusively - never userequire('react')orrequire('react-native')in test files
Files:
app/tests/src/utils/keychainErrors.test.tsapp/tests/src/hooks/useProofDisclosureStalenessCheck.test.ts
app/tests/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (app/AGENTS.md)
Do not create nested
require('react-native')calls in tests - causes OOM in CI
Files:
app/tests/src/utils/keychainErrors.test.tsapp/tests/src/hooks/useProofDisclosureStalenessCheck.test.ts
**/*.{test,spec}.{ts,js,tsx,jsx}
⚙️ CodeRabbit configuration file
**/*.{test,spec}.{ts,js,tsx,jsx}: Review test files for:
- Test coverage completeness
- Test case quality and edge cases
- Mock usage appropriateness
- Test readability and maintainability
Files:
app/tests/src/utils/keychainErrors.test.tsapp/tests/src/hooks/useProofDisclosureStalenessCheck.test.ts
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursorrules)
Implement comprehensive error boundaries in React components.
Files:
app/src/providers/authProvider.tsxapp/src/providers/passportDataProvider.tsx
🧠 Learnings (35)
📓 Common learnings
Learnt from: CR
Repo: selfxyz/self PR: 0
File: app/AGENTS.md:0-0
Timestamp: 2025-12-25T19:19:04.954Z
Learning: Address CodeRabbitAI feedback during PR review process
📚 Learning: 2025-12-25T19:19:35.354Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-12-25T19:19:35.354Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.{ts,tsx} : Ensure no breaking changes to public API or document them properly
Applied to files:
app/version.jsonapp/tests/src/utils/keychainErrors.test.tsapp/src/providers/authProvider.tsxapp/src/hooks/useProofDisclosureStalenessCheck.tsapp/src/providers/passportDataProvider.tsx
📚 Learning: 2025-12-25T19:19:04.954Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: app/AGENTS.md:0-0
Timestamp: 2025-12-25T19:19:04.954Z
Learning: Applies to app/{package.json,app/app.json} : Bump version in `package.json` and `app.json` before deployment
Applied to files:
app/version.json
📚 Learning: 2025-12-25T19:19:04.954Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: app/AGENTS.md:0-0
Timestamp: 2025-12-25T19:19:04.954Z
Learning: Applies to app/**/CHANGELOG* : Update changelog before deployment
Applied to files:
app/version.json
📚 Learning: 2025-12-25T19:19:35.354Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-12-25T19:19:35.354Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.{ts,tsx} : Before committing changes, ensure the build succeeds by running `yarn build`
Applied to files:
app/version.json
📚 Learning: 2025-11-22T23:16:26.719Z
Learnt from: transphorm
Repo: selfxyz/self PR: 1446
File: .github/workflows/mobile-bundle-analysis.yml:117-117
Timestamp: 2025-11-22T23:16:26.719Z
Learning: In the selfxyz/self repository, for mobile workflows (bundle analysis, deployment, CI/CD):
- Both iOS and Android builds now cache Ruby gems at the unified path `app/vendor/bundle`
- The previous separate paths (app/ios/vendor/bundle for iOS) have been deprecated in favor of this unified approach
Applied to files:
app/version.json
📚 Learning: 2025-09-10T14:47:40.945Z
Learnt from: shazarre
Repo: selfxyz/self PR: 1041
File: app/src/providers/passportDataProvider.tsx:297-301
Timestamp: 2025-09-10T14:47:40.945Z
Learning: In app/src/providers/passportDataProvider.tsx: The deleteDocumentDirectlyFromKeychain function is a low-level utility used by the DocumentsAdapter and should not include error handling since callers like deleteDocument() already implement appropriate try/catch with logging for Keychain operations.
Applied to files:
app/src/utils/keychainErrors.tsapp/tests/src/utils/keychainErrors.test.tsapp/src/providers/authProvider.tsxapp/src/providers/passportDataProvider.tsx
📚 Learning: 2025-11-25T14:06:55.970Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-25T14:06:55.970Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use Keychain for secure storage of sensitive data in React Native.
Applied to files:
app/src/utils/keychainErrors.tsapp/tests/src/utils/keychainErrors.test.tsapp/src/providers/authProvider.tsxapp/src/providers/passportDataProvider.tsx
📚 Learning: 2025-11-25T14:07:28.188Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: .cursor/rules/compliance-verification.mdc:0-0
Timestamp: 2025-11-25T14:07:28.188Z
Learning: Applies to **/{compliance,crypto,key,security,auth}/**/*.{ts,tsx,js,py} : Implement proper key rotation and secure storage for compliance verification keys
Applied to files:
app/src/utils/keychainErrors.tsapp/tests/src/utils/keychainErrors.test.ts
📚 Learning: 2025-11-25T14:06:55.970Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-25T14:06:55.970Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Implement certificate validation for passport data verification.
Applied to files:
app/src/utils/keychainErrors.tsapp/tests/src/utils/keychainErrors.test.tsapp/src/providers/authProvider.tsxapp/src/providers/passportDataProvider.tsx
📚 Learning: 2025-11-25T14:07:28.188Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: .cursor/rules/compliance-verification.mdc:0-0
Timestamp: 2025-11-25T14:07:28.188Z
Learning: Applies to **/{compliance,crypto,security,auth}/**/*.{ts,tsx,js,py} : Use only cryptographically approved algorithms and key sizes for compliance verification security
Applied to files:
app/src/utils/keychainErrors.ts
📚 Learning: 2025-12-25T19:19:35.354Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-12-25T19:19:35.354Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.test.{ts,tsx} : Test `isPassportDataValid()` with realistic, synthetic passport data (never use real user data)
Applied to files:
app/tests/src/utils/keychainErrors.test.tsapp/src/providers/passportDataProvider.tsxapp/tests/src/hooks/useProofDisclosureStalenessCheck.test.ts
📚 Learning: 2025-12-25T19:19:04.954Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: app/AGENTS.md:0-0
Timestamp: 2025-12-25T19:19:04.954Z
Learning: Applies to app/**/*.test.{ts,tsx,js,jsx} : Verify no memory leaks introduced, including test memory patterns
Applied to files:
app/tests/src/utils/keychainErrors.test.tsapp/tests/src/hooks/useProofDisclosureStalenessCheck.test.ts
📚 Learning: 2025-12-25T19:19:35.354Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-12-25T19:19:35.354Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.test.{ts,tsx} : Never use real user PII (personally identifiable information) in tests - use only synthetic, anonymized, or approved test vectors
Applied to files:
app/tests/src/utils/keychainErrors.test.tsapp/tests/src/hooks/useProofDisclosureStalenessCheck.test.ts
📚 Learning: 2025-12-25T19:19:35.354Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-12-25T19:19:35.354Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.test.{ts,tsx} : Write integration tests that exercise real validation logic from the SDK
Applied to files:
app/tests/src/utils/keychainErrors.test.ts
📚 Learning: 2025-12-25T19:19:35.354Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-12-25T19:19:35.354Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.test.{ts,tsx} : Verify `extractMRZInfo()` using published sample MRZ strings (e.g., ICAO examples)
Applied to files:
app/tests/src/utils/keychainErrors.test.ts
📚 Learning: 2025-11-25T14:06:55.970Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-25T14:06:55.970Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock SQLite operations with `executeSql` method in database tests using utilities from `tests/__setup__/databaseMocks.ts`.
Applied to files:
app/src/stores/database.ts
📚 Learning: 2025-11-25T14:07:28.188Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: .cursor/rules/compliance-verification.mdc:0-0
Timestamp: 2025-11-25T14:07:28.188Z
Learning: Applies to **/{mobile,client,app,proof,zk}/**/*.{ts,tsx,js,swift,kt} : Include trusted time anchor in proof generation and verify time anchor authenticity before proof generation in mobile implementations
Applied to files:
app/src/hooks/useProofDisclosureStalenessCheck.ts
📚 Learning: 2025-11-25T14:06:55.970Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-25T14:06:55.970Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use custom hooks for complex state (`useModal`, `useHapticNavigation`) instead of inline logic.
Applied to files:
app/src/hooks/useProofDisclosureStalenessCheck.tsapp/tests/src/hooks/useProofDisclosureStalenessCheck.test.ts
📚 Learning: 2025-08-26T14:49:11.190Z
Learnt from: shazarre
Repo: selfxyz/self PR: 936
File: app/src/screens/passport/PassportNFCScanScreen.tsx:28-31
Timestamp: 2025-08-26T14:49:11.190Z
Learning: SelfClientProvider is wrapped in app/App.tsx, providing context for useSelfClient() hook usage throughout the React Native app navigation stacks.
Applied to files:
app/src/hooks/useProofDisclosureStalenessCheck.ts
📚 Learning: 2025-12-13T18:00:46.963Z
Learnt from: seshanthS
Repo: selfxyz/self PR: 1497
File: app/src/screens/verification/ProveScreen.tsx:125-161
Timestamp: 2025-12-13T18:00:46.963Z
Learning: In app/src/screens/verification/ProveScreen.tsx: The document expiration check using checkDocumentExpiration() is UX-only to prevent wasted gas and provide better user experience. The authoritative expiration validation is enforced in the circuits and smart contracts using trusted time sources (block timestamps), not device clock.
Applied to files:
app/src/hooks/useProofDisclosureStalenessCheck.ts
📚 Learning: 2025-12-25T19:19:04.954Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: app/AGENTS.md:0-0
Timestamp: 2025-12-25T19:19:04.954Z
Learning: Document complex native module changes in PRs
Applied to files:
app/src/providers/passportDataProvider.tsx
📚 Learning: 2025-08-26T14:42:45.297Z
Learnt from: aaronmgdr
Repo: selfxyz/self PR: 936
File: app/src/utils/proving/validateDocument.ts:53-54
Timestamp: 2025-08-26T14:42:45.297Z
Learning: In the Self app, native modules (including Keychain) are initialized early in the app startup sequence, before functions like checkAndUpdateRegistrationStates() are called, so additional native module readiness guards in document-related functions are not needed.
Applied to files:
app/src/providers/passportDataProvider.tsx
📚 Learning: 2025-11-25T14:06:55.970Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-25T14:06:55.970Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use `renderHook` for testing custom React hooks instead of rendering components.
Applied to files:
app/tests/src/hooks/useProofDisclosureStalenessCheck.test.ts
📚 Learning: 2025-12-25T19:19:04.954Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: app/AGENTS.md:0-0
Timestamp: 2025-12-25T19:19:04.954Z
Learning: Applies to app/**/*.test.{ts,tsx,js,jsx} : Never use `require()` calls in `beforeEach`/`afterEach` hooks in test files
Applied to files:
app/tests/src/hooks/useProofDisclosureStalenessCheck.test.ts
📚 Learning: 2025-12-25T19:19:35.354Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-12-25T19:19:35.354Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.test.{ts,tsx} : Use real imports from `selfxyz/mobile-sdk-alpha` in tests instead of mocking the entire package
Applied to files:
app/tests/src/hooks/useProofDisclosureStalenessCheck.test.ts
📚 Learning: 2025-12-25T19:19:04.954Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: app/AGENTS.md:0-0
Timestamp: 2025-12-25T19:19:04.954Z
Learning: Applies to app/**/*.test.{ts,tsx,js,jsx} : Put all imports at the top of test files - avoid dynamic imports in hooks
Applied to files:
app/tests/src/hooks/useProofDisclosureStalenessCheck.test.ts
📚 Learning: 2025-12-25T19:19:35.354Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-12-25T19:19:35.354Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.test.{ts,tsx} : Avoid dynamic `require()` calls in `beforeEach`/`afterEach` hooks to prevent out-of-memory errors
Applied to files:
app/tests/src/hooks/useProofDisclosureStalenessCheck.test.ts
📚 Learning: 2025-12-25T19:18:22.033Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: .cursor/rules/test-memory-optimization.mdc:0-0
Timestamp: 2025-12-25T19:18:22.033Z
Learning: Applies to **/tests/setup.ts : Use `vi.mock()` instead of `require()` calls in Vitest setup files; ensure React Native mocks are established once at setup time, not during test execution
Applied to files:
app/tests/src/hooks/useProofDisclosureStalenessCheck.test.ts
📚 Learning: 2025-12-25T19:19:04.954Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: app/AGENTS.md:0-0
Timestamp: 2025-12-25T19:19:04.954Z
Learning: Applies to app/**/*.test.{ts,tsx,js,jsx} : Avoid nested `require('react')` or `require('react-native')` calls in test files - use ES6 import statements instead
Applied to files:
app/tests/src/hooks/useProofDisclosureStalenessCheck.test.ts
📚 Learning: 2025-12-25T19:18:43.224Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-25T19:18:43.224Z
Learning: Applies to {app,packages/mobile-sdk-alpha}/**/*.{test,spec}.{js,ts,jsx,tsx} : Never create nested require('react-native') calls in tests as this causes out-of-memory errors in CI/CD pipelines; use ES6 import statements instead and avoid dynamic require() calls in beforeEach/afterEach hooks
Applied to files:
app/tests/src/hooks/useProofDisclosureStalenessCheck.test.ts
📚 Learning: 2025-12-25T19:19:04.954Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: app/AGENTS.md:0-0
Timestamp: 2025-12-25T19:19:04.954Z
Learning: Applies to app/tests/**/*.{ts,tsx,js,jsx} : Do not create nested `require('react-native')` calls in tests - causes OOM in CI
Applied to files:
app/tests/src/hooks/useProofDisclosureStalenessCheck.test.ts
📚 Learning: 2025-12-25T19:18:22.033Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: .cursor/rules/test-memory-optimization.mdc:0-0
Timestamp: 2025-12-25T19:18:22.033Z
Learning: Applies to **/__tests__/**/*.{ts,tsx,js,jsx} : Never use `require('react-native')` in test files; use ES6 `import` statements instead to avoid nested require() calls that cause out-of-memory errors in CI/CD pipelines
Applied to files:
app/tests/src/hooks/useProofDisclosureStalenessCheck.test.ts
📚 Learning: 2025-12-25T19:19:35.354Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-12-25T19:19:35.354Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.test.{ts,tsx} : React Native is mocked in `tests/setup.ts` using `vi.mock()` - use imports in test files instead of require()
Applied to files:
app/tests/src/hooks/useProofDisclosureStalenessCheck.test.ts
📚 Learning: 2025-12-25T19:19:35.354Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-12-25T19:19:35.354Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.test.{ts,tsx} : Avoid mocking the entire package in Jest setup or replacing real functions with mock implementations
Applied to files:
app/tests/src/hooks/useProofDisclosureStalenessCheck.test.ts
🧬 Code graph analysis (5)
app/tests/src/utils/keychainErrors.test.ts (1)
app/src/utils/keychainErrors.ts (3)
isUserCancellation(37-46)isKeychainCryptoError(25-35)getKeychainErrorIdentity(18-23)
app/src/providers/authProvider.tsx (1)
app/src/utils/keychainErrors.ts (1)
getKeychainErrorIdentity(18-23)
app/src/stores/database.ts (1)
app/tests/__setup__/databaseMocks.ts (1)
SQLite(8-11)
app/src/providers/passportDataProvider.tsx (1)
app/src/utils/keychainErrors.ts (4)
KeychainErrorType(16-16)isUserCancellation(37-46)isKeychainCryptoError(25-35)getKeychainErrorIdentity(18-23)
app/tests/src/hooks/useProofDisclosureStalenessCheck.test.ts (1)
app/src/hooks/useProofDisclosureStalenessCheck.ts (1)
useProofDisclosureStalenessCheck(20-40)
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: android-build-test
- GitHub Check: e2e-ios
🔇 Additional comments (12)
app/version.json (1)
1-10: LGTM - Standard version bump for staging release.The iOS and Android build numbers and deployment timestamps have been updated appropriately for this automated staging release.
app/src/stores/database.ts (2)
17-18: Good defensive coding with the toInsertId helper.This normalizes the insertId handling for cases where
INSERT OR IGNOREskips insertion due to duplicates, providing a consistent string return value.
40-51: Excellent error handling improvement.Wrapping each
setProofStatuscall in a try-catch prevents one failure from aborting the entire batch operation. This ensures all stale proofs are processed even if individual updates fail.app/src/hooks/useProofDisclosureStalenessCheck.ts (1)
20-40: LGTM - Clear and appropriate hook rename.The rename from
useSelfAppStalenessChecktouseProofDisclosureStalenessCheckbetter reflects the hook's purpose of validating proof disclosure data. The implementation and behavior remain unchanged.app/tests/src/hooks/useProofDisclosureStalenessCheck.test.ts (1)
1-85: Excellent test coverage following best practices.The test suite comprehensively covers the three scenarios (missing selfApp, empty disclosures, valid data) and follows all testing guidelines:
- Uses
renderHookfrom @testing-library/react-native- ES6 imports throughout (no
require()calls)- Proper fake timer setup/teardown
- Appropriate use of
act()for async operationsapp/src/providers/authProvider.tsx (1)
28-32: Good refactor to centralize keychain error handling.Moving the error utilities (
getKeychainErrorIdentity,isKeychainCryptoError,isUserCancellation) to a shared module eliminates code duplication and improves maintainability. The control flow and behavior remain unchanged while enabling consistent error handling across the codebase.Based on learnings: centralized error handling pattern.
Also applies to: 187-214
app/tests/src/utils/keychainErrors.test.ts (1)
1-64: Good test coverage for the new centralized keychain error utilities.The test file properly uses ES6 imports, covers the main scenarios for each exported function, and validates both positive and negative cases. The structure is clean and maintainable.
app/src/utils/keychainErrors.ts (3)
5-16: Well-structured type definitions for keychain error handling.The types are appropriately scoped -
KeychainErroris internal whileKeychainErrorIdentityandKeychainErrorTypeare exported for consumer use. This is a good separation of concerns.
25-35: Solid implementation of crypto error detection with correct exclusion logic.The function properly checks multiple error representations (code, name, message) and correctly excludes user cancellations from being classified as crypto errors. The
&&condition with!isUserCancellation(error)ensures mutual exclusivity.
37-46: User cancellation detection covers known patterns effectively.The function handles multiple cancellation indicators from the Keychain library. The variety of message patterns (
User canceled,Authentication canceled,cancelled by user) suggests this was derived from real-world error observations.app/src/providers/passportDataProvider.tsx (2)
70-75: Good refactor: Centralizing keychain error utilities.The imports from
@/utils/keychainErrorsconsolidate error handling logic that was previously duplicated. This improves maintainability and consistency across providers.
503-506: Clean integration of the new error handling utility.The replacement of inline error handling with
handleKeychainReadErrorin bothloadDocumentByIdDirectlyFromKeychainandloadDocumentCatalogDirectlyFromKeychainis a good refactor. ThethrowOnUserCancel: trueoption for the catalog load makes sense since that's a more critical operation.Also applies to: 550-554
| function handleKeychainReadError({ | ||
| contextLabel, | ||
| error, | ||
| throwOnUserCancel = false, | ||
| }: { | ||
| contextLabel: string; | ||
| error: unknown; | ||
| throwOnUserCancel?: boolean; | ||
| }) { | ||
| if (isUserCancellation(error)) { | ||
| console.log(`User cancelled authentication for ${contextLabel}`); | ||
| notifyKeychainFailure('user_cancelled'); | ||
|
|
||
| if (throwOnUserCancel) { | ||
| throw error; | ||
| } | ||
| } | ||
|
|
||
| if (isKeychainCryptoError(error)) { | ||
| const err = getKeychainErrorIdentity(error); | ||
| console.error(`Keychain crypto error loading ${contextLabel}:`, { | ||
| code: err?.code, | ||
| name: err?.name, | ||
| }); | ||
|
|
||
| notifyKeychainFailure('crypto_failed'); | ||
| } | ||
|
|
||
| console.log(`Error loading ${contextLabel}:`, error); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential double logging of errors.
Line 121 logs every error unconditionally, but user cancellations are already logged at line 103 and crypto errors at lines 113-116. This results in duplicate log entries for recognized error types.
Consider moving the generic log to an else branch or removing it for already-handled cases.
🔧 Suggested fix
if (isKeychainCryptoError(error)) {
const err = getKeychainErrorIdentity(error);
console.error(`Keychain crypto error loading ${contextLabel}:`, {
code: err?.code,
name: err?.name,
});
notifyKeychainFailure('crypto_failed');
+ return;
}
- console.log(`Error loading ${contextLabel}:`, error);
+ // Log unrecognized errors only
+ console.warn(`Unhandled error loading ${contextLabel}:`, error);
}🤖 Prompt for AI Agents
In @app/src/providers/passportDataProvider.tsx around lines 93 - 122, In
handleKeychainReadError, avoid double-logging by making the final generic
console.log conditional: check if error was handled by isUserCancellation or
isKeychainCryptoError (or simply return after handling those cases) so the
generic log only runs for unrecognized errors; update the function around the
isUserCancellation and isKeychainCryptoError branches (and any throwOnUserCancel
behavior) to either return after handling or use an else/else-if before the
final console.log, and keep notifyKeychainFailure calls unchanged.
🚀 Weekly Release to Staging
Release Date: January 10, 2026
Release Branch:
release/staging-2026-01-10This automated PR promotes a snapshot of
devtostagingfor testing.What's Included
All commits merged to
devup to the branch creation time.Note: This PR uses a dedicated release branch, so new commits to
devwill NOT automatically appear here.Review Checklist
Next Steps
After merging, the staging environment will be updated. A production release PR will be created on Sunday.
This PR was automatically created by the Release Calendar workflow on January 10, 2026
Summary by CodeRabbit
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.