Skip to content

Conversation

@transphorm
Copy link
Member

@transphorm transphorm commented Sep 28, 2025

update demo app with some polish. generating a mock document and adding to the document store works. registering the mock document is still a WiP

Screenshot 2025-09-30 at 10 20 16 PM

Summary

  • add react-native-safe-area-context and wrap the demo in a SafeAreaProvider
  • introduce a shared SafeAreaScrollView wrapper and use it across demo screens
  • gate register/prove menu actions until mock data exists and add subtitles for clarity

Testing

  • yarn workspace mobile-sdk-demo test --runTestsByPath tests/App.test.tsx

https://chatgpt.com/codex/tasks/task_b_68d9405738b0832db9e79a0a5df03d17

Summary by CodeRabbit

  • New Features

    • Demo: Home menu, My Documents, Proof History, Register, Generate Mock, Camera, NFC, and QR flows; generate, store, register, and view mock documents; consistent safe-area root and standardized header/scroll UI.
  • Refactor

    • Descriptor-driven navigation and shared screen context powering dynamic screen loading and unified screen wiring.
  • Bug Fixes

    • Improved iOS NFC/MRZ startup robustness and removed launcher branding label.
  • Chores

    • Scripts, dependency updates, Metro/workspace resolution and lint ignores, SVG/type declarations, font registration.
  • Tests

    • Migrate testing setup to Vitest with new test suites for crypto, storage, and MRZ parsing.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 28, 2025

Walkthrough

Refactors the demo into a descriptor-driven navigation, adds SafeArea/StandardHeader components and multiple new screens, provides an in-memory SelfClientProvider and document store, migrates tests/config from Jest to Vitest, adjusts Metro and native iOS/Android bits, adds secure-storage and crypto/ethers adjustments, and updates CI/build scripts and package resolutions.

Changes

Cohort / File(s) Summary
CI & build scripts / root package
app/scripts/mobile-ci-build-android.sh, package.json, app/package.json, packages/mobile-sdk-demo/package.json, packages/mobile-sdk-demo/.gitignore
Guarded restore of package.json/yarn.lock with failure handling and post-check for tarball refs; add @swc/core resolution; pin react-native-svg exact version; add demo scripts (install-app, reinstall, format, types, vitest); update RN deps; ignore build/.
App bootstrap
packages/mobile-sdk-demo/index.js
Wrap App with SafeAreaProvider (Root) and register Root with AppRegistry.
Descriptor-driven navigation & App shell
packages/mobile-sdk-demo/App.tsx, packages/mobile-sdk-demo/src/screens/index.ts, packages/mobile-sdk-demo/src/screens/HomeScreen.tsx, packages/mobile-sdk-demo/__tests__/App.test.tsx
Add typed screen descriptors/registry and runtime loader; App uses descriptor.load()/getProps(); HomeScreen renders sections; previous App.test removed.
Safe-area & header components
packages/mobile-sdk-demo/src/components/SafeAreaScrollView.tsx, packages/mobile-sdk-demo/src/components/StandardHeader.tsx
New SafeAreaScrollView and StandardHeader components (default exports).
Screen migrations to SafeArea + header
packages/mobile-sdk-demo/src/screens/DocumentCamera.tsx, .../DocumentNFCScan.tsx, .../QRCodeViewFinder.tsx, .../GenerateMock.tsx, .../RegisterDocument.tsx
Replace ScrollView with SafeAreaScrollView, add StandardHeader, adjust layouts; add GenerateMock and RegisterDocument screens (new props/flows).
New demo screens
packages/mobile-sdk-demo/src/screens/DocumentsList.tsx, packages/mobile-sdk-demo/src/screens/ProofHistory.tsx
Add DocumentsList (load/delete docs) and ProofHistory (mock activities).
Demo provider & in-memory documents
packages/mobile-sdk-demo/src/providers/SelfClientProvider.tsx, packages/mobile-sdk-demo/src/utils/documentStore.ts
Add SelfClientProvider wiring demo adapters and an in-memory DocumentsAdapter plus resetDocumentStore().
Tests & test infra (Jest → Vitest)
packages/mobile-sdk-demo/jest.setup.js (removed), packages/mobile-sdk-demo/jest.config.cjs (removed), packages/mobile-sdk-demo/vitest.config.ts, packages/mobile-sdk-demo/tests/setup.ts, packages/mobile-sdk-demo/__tests__/*, packages/mobile-sdk-demo/tests/*
Remove prior Jest config/setup; add Vitest config and setup; add/convert tests and mocks for secureStorage, documentStore, crypto polyfill, etc.
Metro / resolver / TS types / SVG
packages/mobile-sdk-demo/metro.config.cjs, packages/mobile-sdk-demo/types/svg.d.ts, packages/mobile-sdk-demo/tsconfig.json
Workspace-aware Metro resolver with extraNodeModules, node-polyfills and blockList; add SVG transformer and types; update tsconfig to include react-native and vitest types and test folders.
Native iOS/Android changes
packages/mobile-sdk-alpha/ios/SelfSDK/PassportReader.swift, .../SelfMRZScannerModule.swift, packages/mobile-sdk-demo/ios/SelfDemoApp/LaunchScreen.storyboard, packages/mobile-sdk-demo/ios/SelfDemoApp/Info.plist, packages/mobile-sdk-demo/android/app/build.gradle
Remove usePacePolling arg; change root VC lookup to use connected UIWindowScene; remove "Powered by React Native" label; add Ionicons.ttf; apply fonts.gradle import for vector icons.
Mock generator & MRZ parsing
packages/mobile-sdk-alpha/src/mock/generator.ts, packages/mobile-sdk-alpha/src/processing/mrz.ts, packages/mobile-sdk-alpha/src/mrz/index.ts, packages/mobile-sdk-alpha/src/index.ts, packages/mobile-sdk-alpha/src/browser.ts, packages/mobile-sdk-alpha/tests/processing/mrz.test.ts
Extend generateMockDocument to accept/propagate firstName/lastName and return `PassportData
Crypto / polyfills / ethers adjustments
packages/mobile-sdk-demo/src/utils/ethers.ts, common/src/polyfills/crypto.ts, packages/mobile-sdk-demo/crypto-polyfill.js (removed), packages/mobile-sdk-demo/tests/cryptoPolyfill.test.ts, packages/mobile-sdk-demo/__tests__/cryptoPolyfills.test.ts
Ensure sha256/sha512 return plain Uint8Array; coerce Buffer subclasses to Uint8Array before hashing; remove legacy crypto-polyfill module; update tests for Vitest and new environment.
Secure storage & keychain
packages/mobile-sdk-demo/src/utils/secureStorage.ts, related tests
New secureStorage API with native Keychain (react-native-keychain) and web/localStorage fallback: getOrCreateSecret, hasSecret, getSecretMetadata, clearSecret, isValidSecret, and generateSecret.
Housekeeping & exports
.eslintignore, .watchmanconfig, .cursorignore, common/index.ts, packages/mobile-sdk-alpha/src/stores/protocolStore.ts, app/package.json
Add broad lint/watchman ignores; remove RN config from .cursorignore; export Environment type from common/index.ts; change runtime import to type-only import in protocolStore; pin react-native-svg exact version in app/package.json.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant U as User
  participant App as DemoApp
  participant Registry as screenDescriptors
  participant Provider as SelfClientProvider
  participant Screen as ScreenComponent

  U->>App: Launch
  App->>Registry: build registry (screenDescriptors / screenMap)
  App->>Provider: mount SelfClientProvider (adapters)
  App->>App: render HomeScreen(screenContext)
  U->>App: tap menu item
  App->>Registry: resolve descriptor(id) and load component
  Registry-->>App: Component
  App->>Registry: descriptor.getProps(screenContext)
  App->>Screen: render component with props + self client
  Screen->>Provider: request docs / save / prove
  Provider-->>Screen: respond (catalog, ack, events)
  Screen-->>App: navigate('home') / onBack
  App->>App: render HomeScreen
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested reviewers

  • aaronmgdr
  • shazarre

Poem

A registry blooms where screens align,
Safe edges cradle every line.
Secrets tucked where keychains keep,
CI watches while builders sleep.
Demo sails steady — small, but brave.

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title Check ❓ Inconclusive The title uses the correct “chore:” prefix and indicates polish to the mobile demo app, but the term “polish” is fairly generic and does not clearly convey the key changes such as SafeAreaProvider integration, new wrappers, and gated workflows; a teammate scanning the history may not immediately understand the primary modifications. Please refine the title to more specifically summarize the core updates—for example, “chore: integrate SafeAreaProvider and gate register/prove actions in mobile demo app” or similar—to clearly highlight the main changes.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch codex/review-mobile-sdk-demo-app-for-polish-suggestions

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@transphorm transphorm changed the title Add safe area support to mobile SDK demo app chore: add polish to mobile demo app Sep 28, 2025
@transphorm transphorm marked this pull request as ready for review September 29, 2025 00:51
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1c2a59f and 417a428.

⛔ Files ignored due to path filters (2)
  • app/ios/Podfile.lock is excluded by !**/*.lock
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (16)
  • app/scripts/mobile-ci-build-android.sh (1 hunks)
  • package.json (1 hunks)
  • packages/mobile-sdk-demo/App.tsx (2 hunks)
  • packages/mobile-sdk-demo/__tests__/App.test.tsx (1 hunks)
  • packages/mobile-sdk-demo/index.js (2 hunks)
  • packages/mobile-sdk-demo/jest.setup.js (1 hunks)
  • packages/mobile-sdk-demo/package.json (1 hunks)
  • packages/mobile-sdk-demo/src/components/SafeAreaScrollView.tsx (1 hunks)
  • packages/mobile-sdk-demo/src/screens/DocumentCamera.tsx (2 hunks)
  • packages/mobile-sdk-demo/src/screens/DocumentNFCScan.tsx (2 hunks)
  • packages/mobile-sdk-demo/src/screens/DocumentOnboarding.tsx (2 hunks)
  • packages/mobile-sdk-demo/src/screens/GenerateMock.tsx (3 hunks)
  • packages/mobile-sdk-demo/src/screens/ProveQRCode.tsx (3 hunks)
  • packages/mobile-sdk-demo/src/screens/QRCodeViewFinder.tsx (2 hunks)
  • packages/mobile-sdk-demo/src/screens/RegisterDocument.tsx (3 hunks)
  • packages/mobile-sdk-demo/src/screens/index.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,ts,tsx,jsx,sol,nr}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{js,ts,tsx,jsx,sol,nr}: 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., ***-***-1234 for passport numbers, J*** D*** for names).

Files:

  • packages/mobile-sdk-demo/src/components/SafeAreaScrollView.tsx
  • packages/mobile-sdk-demo/__tests__/App.test.tsx
  • packages/mobile-sdk-demo/src/screens/index.ts
  • packages/mobile-sdk-demo/src/screens/DocumentOnboarding.tsx
  • packages/mobile-sdk-demo/src/screens/DocumentCamera.tsx
  • packages/mobile-sdk-demo/src/screens/QRCodeViewFinder.tsx
  • packages/mobile-sdk-demo/jest.setup.js
  • packages/mobile-sdk-demo/App.tsx
  • packages/mobile-sdk-demo/src/screens/DocumentNFCScan.tsx
  • packages/mobile-sdk-demo/index.js
  • packages/mobile-sdk-demo/src/screens/ProveQRCode.tsx
  • packages/mobile-sdk-demo/src/screens/GenerateMock.tsx
  • packages/mobile-sdk-demo/src/screens/RegisterDocument.tsx
**/*.{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:

  • packages/mobile-sdk-demo/__tests__/App.test.tsx
🧠 Learnings (32)
📓 Common learnings
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/demo/** : Provide an in-SDK lightweight React Native demo under packages/mobile-sdk-alpha/demo/
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/**/package.json : Ensure package exports are properly configured

Applied to files:

  • app/scripts/mobile-ci-build-android.sh
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/**/package.json : Verify package conditions are valid (e.g., exports conditions)

Applied to files:

  • app/scripts/mobile-ci-build-android.sh
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/package.json : Expose a 'test:build' script in the SDK's package.json that runs build, test, types, and lint

Applied to files:

  • app/scripts/mobile-ci-build-android.sh
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/package.json : Enable tree shaking for the SDK (e.g., ensure 'sideEffects' is correctly set in package.json and exports are ESM-friendly)

Applied to files:

  • app/scripts/mobile-ci-build-android.sh
  • packages/mobile-sdk-demo/package.json
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.{ts,tsx} : Avoid introducing circular dependencies

Applied to files:

  • app/scripts/mobile-ci-build-android.sh
  • packages/mobile-sdk-demo/package.json
📚 Learning: 2025-09-22T11:10:22.019Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Test, build, and deploy scripts (`yarn test`, `yarn ios`, `yarn test:e2e:ios`, Fastlane, etc.) must be used for automation.

Applied to files:

  • app/scripts/mobile-ci-build-android.sh
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/src/artifacts/** : Place artifact and manifest management in packages/mobile-sdk-alpha/src/artifacts/

Applied to files:

  • app/scripts/mobile-ci-build-android.sh
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/demo/** : Provide an in-SDK lightweight React Native demo under packages/mobile-sdk-alpha/demo/

Applied to files:

  • packages/mobile-sdk-demo/src/components/SafeAreaScrollView.tsx
  • packages/mobile-sdk-demo/package.json
  • packages/mobile-sdk-demo/App.tsx
  • packages/mobile-sdk-demo/index.js
  • packages/mobile-sdk-demo/src/screens/GenerateMock.tsx
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Use actual imports from selfxyz/mobile-sdk-alpha in tests

Applied to files:

  • packages/mobile-sdk-demo/__tests__/App.test.tsx
  • packages/mobile-sdk-demo/jest.setup.js
  • packages/mobile-sdk-demo/src/screens/GenerateMock.tsx
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Write integration tests that exercise the real validation logic (not mocks)

Applied to files:

  • packages/mobile-sdk-demo/__tests__/App.test.tsx
  • packages/mobile-sdk-demo/jest.setup.js
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Test isPassportDataValid() with realistic synthetic passport data (never real user data)

Applied to files:

  • packages/mobile-sdk-demo/__tests__/App.test.tsx
  • packages/mobile-sdk-demo/jest.setup.js
  • packages/mobile-sdk-demo/src/screens/GenerateMock.tsx
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Ensure parseNFCResponse() works with representative, synthetic NFC data

Applied to files:

  • packages/mobile-sdk-demo/__tests__/App.test.tsx
  • packages/mobile-sdk-demo/jest.setup.js
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Verify extractMRZInfo() using published sample MRZ strings (e.g., ICAO examples)

Applied to files:

  • packages/mobile-sdk-demo/__tests__/App.test.tsx
  • packages/mobile-sdk-demo/jest.setup.js
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Never use real user PII in tests; use only synthetic, anonymized, or approved test vectors

Applied to files:

  • packages/mobile-sdk-demo/__tests__/App.test.tsx
  • packages/mobile-sdk-demo/jest.setup.js
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Do NOT mock selfxyz/mobile-sdk-alpha in tests (avoid jest.mock('selfxyz/mobile-sdk-alpha') and replacing real functions with mocks)

Applied to files:

  • packages/mobile-sdk-demo/__tests__/App.test.tsx
  • packages/mobile-sdk-demo/jest.setup.js
  • packages/mobile-sdk-demo/src/screens/GenerateMock.tsx
📚 Learning: 2025-09-22T11:10:22.019Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Always use `@/` for src imports and `tests/` for test imports; prefer platform-specific imports with conditional rendering.

Applied to files:

  • packages/mobile-sdk-demo/__tests__/App.test.tsx
📚 Learning: 2025-09-22T11:10:22.019Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Applies to tests/**/*.test.{js,ts,tsx,jsx} : Use `renderHook` for testing custom React hooks.

Applied to files:

  • packages/mobile-sdk-demo/__tests__/App.test.tsx
📚 Learning: 2025-09-22T11:10:22.019Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Applies to tests/**/*.test.{js,ts,tsx,jsx} : Test error boundaries and recovery mechanisms.

Applied to files:

  • packages/mobile-sdk-demo/__tests__/App.test.tsx
📚 Learning: 2025-09-22T11:10:22.019Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Applies to src/screens/**/*.{js,ts,tsx,jsx} : Lazy load screens using `React.lazy()`, organize screens by feature modules.

Applied to files:

  • packages/mobile-sdk-demo/__tests__/App.test.tsx
  • packages/mobile-sdk-demo/src/screens/index.ts
  • packages/mobile-sdk-demo/App.tsx
📚 Learning: 2025-08-26T14:41:41.821Z
Learnt from: shazarre
PR: selfxyz/self#936
File: app/src/screens/aesop/PassportOnboardingScreen.tsx:0-0
Timestamp: 2025-08-26T14:41:41.821Z
Learning: When verifying provider hierarchies in React Native apps, always check the main App.tsx file at the app root, not just navigation/index.tsx and layout files, as providers are often configured at the top-level App component.

Applied to files:

  • packages/mobile-sdk-demo/__tests__/App.test.tsx
  • packages/mobile-sdk-demo/App.tsx
  • packages/mobile-sdk-demo/index.js
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/src/index.ts : Re-export new SDK modules via packages/mobile-sdk-alpha/src/index.ts

Applied to files:

  • packages/mobile-sdk-demo/src/screens/index.ts
📚 Learning: 2025-07-16T02:20:44.173Z
Learnt from: transphorm
PR: selfxyz/self#636
File: app/src/components/native/QRCodeScanner.tsx:135-142
Timestamp: 2025-07-16T02:20:44.173Z
Learning: In app/src/components/native/QRCodeScanner.tsx, the Android camera dimension multipliers (screenWidth * 3 and screenHeight * 2) are intentionally set to these values and should not be changed. These multipliers are correct and any visual issues with black areas in the camera preview are caused by other factors, not the dimension calculations.

Applied to files:

  • packages/mobile-sdk-demo/src/screens/DocumentCamera.tsx
  • packages/mobile-sdk-demo/src/screens/QRCodeViewFinder.tsx
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to app/jest.setup.js : Provide comprehensive Jest setup in app/jest.setup.js with required mocks

Applied to files:

  • packages/mobile-sdk-demo/jest.setup.js
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/README.md : Document new/updated SDK modules and usage in packages/mobile-sdk-alpha/README.md

Applied to files:

  • packages/mobile-sdk-demo/jest.setup.js
📚 Learning: 2025-09-22T11:10:22.019Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Applies to tests/**/*.test.{js,ts,tsx,jsx} : Mock console.error to avoid test output clutter.

Applied to files:

  • packages/mobile-sdk-demo/jest.setup.js
📚 Learning: 2025-08-26T14:49:11.190Z
Learnt from: shazarre
PR: selfxyz/self#936
File: app/src/screens/passport/PassportNFCScanScreen.tsx:28-31
Timestamp: 2025-08-26T14:49:11.190Z
Learning: The main App.tsx file is located at app/App.tsx (not in app/src), and it properly wraps the entire app with SelfClientProvider at the top of the provider hierarchy, enabling useSelfClient() hook usage throughout all navigation screens.

Applied to files:

  • packages/mobile-sdk-demo/App.tsx
  • packages/mobile-sdk-demo/index.js
📚 Learning: 2025-09-22T11:10:22.019Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Applies to src/navigation/**/*.{js,ts,tsx,jsx} : Platform-specific initial routes: web uses 'Home', mobile uses 'Splash'.

Applied to files:

  • packages/mobile-sdk-demo/App.tsx
📚 Learning: 2025-09-22T11:10:57.879Z
Learnt from: CR
PR: selfxyz/self#0
File: app/AGENTS.md:0-0
Timestamp: 2025-09-22T11:10:57.879Z
Learning: Applies to app/ios/**/*.{m,mm,swift} : Document complex native iOS module changes in the PR

Applied to files:

  • packages/mobile-sdk-demo/App.tsx
📚 Learning: 2025-08-26T14:49:15.210Z
Learnt from: shazarre
PR: selfxyz/self#936
File: app/src/screens/passport/PassportNFCScanScreen.web.tsx:8-11
Timestamp: 2025-08-26T14:49:15.210Z
Learning: The main App.tsx file is located at app/App.tsx (at the app root), not at app/src/App.tsx, and contains the SelfClientProvider wrapping the entire application.

Applied to files:

  • packages/mobile-sdk-demo/App.tsx
  • packages/mobile-sdk-demo/index.js
📚 Learning: 2025-08-26T14:49:11.190Z
Learnt from: shazarre
PR: selfxyz/self#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:

  • packages/mobile-sdk-demo/App.tsx
📚 Learning: 2025-08-23T02:02:02.556Z
Learnt from: transphorm
PR: selfxyz/self#942
File: app/vite.config.ts:170-0
Timestamp: 2025-08-23T02:02:02.556Z
Learning: In the selfxyz/self React Native app, devTools from '@/navigation/devTools' are intentionally shipped to production builds for testing purposes, not excluded as is typical in most applications.

Applied to files:

  • packages/mobile-sdk-demo/App.tsx
🧬 Code graph analysis (11)
packages/mobile-sdk-demo/src/components/SafeAreaScrollView.tsx (1)
app/src/mocks/react-native-safe-area-context.js (1)
  • SafeAreaView (21-23)
packages/mobile-sdk-demo/__tests__/App.test.tsx (1)
packages/mobile-sdk-demo/src/screens/index.ts (1)
  • screenDescriptors (41-120)
packages/mobile-sdk-demo/src/screens/DocumentOnboarding.tsx (1)
packages/mobile-sdk-demo/src/components/SafeAreaScrollView.tsx (1)
  • SafeAreaScrollView (13-27)
packages/mobile-sdk-demo/src/screens/DocumentCamera.tsx (1)
packages/mobile-sdk-demo/src/components/SafeAreaScrollView.tsx (1)
  • SafeAreaScrollView (13-27)
packages/mobile-sdk-demo/src/screens/QRCodeViewFinder.tsx (1)
packages/mobile-sdk-demo/src/components/SafeAreaScrollView.tsx (1)
  • SafeAreaScrollView (13-27)
packages/mobile-sdk-demo/App.tsx (2)
packages/mobile-sdk-demo/src/screens/index.ts (4)
  • ScreenRoute (39-39)
  • ScreenContext (18-23)
  • screenMap (122-128)
  • orderedSectionEntries (130-142)
packages/mobile-sdk-demo/src/components/SafeAreaScrollView.tsx (1)
  • SafeAreaScrollView (13-27)
packages/mobile-sdk-demo/src/screens/DocumentNFCScan.tsx (1)
packages/mobile-sdk-demo/src/components/SafeAreaScrollView.tsx (1)
  • SafeAreaScrollView (13-27)
packages/mobile-sdk-demo/index.js (1)
app/src/mocks/react-native-safe-area-context.js (1)
  • SafeAreaProvider (17-19)
packages/mobile-sdk-demo/src/screens/ProveQRCode.tsx (1)
packages/mobile-sdk-demo/src/components/SafeAreaScrollView.tsx (1)
  • SafeAreaScrollView (13-27)
packages/mobile-sdk-demo/src/screens/GenerateMock.tsx (1)
packages/mobile-sdk-demo/src/components/SafeAreaScrollView.tsx (1)
  • SafeAreaScrollView (13-27)
packages/mobile-sdk-demo/src/screens/RegisterDocument.tsx (1)
packages/mobile-sdk-demo/src/components/SafeAreaScrollView.tsx (1)
  • SafeAreaScrollView (13-27)
🪛 GitHub Check: workspace-lint
packages/mobile-sdk-demo/__tests__/App.test.tsx

[warning] 48-48:
Insert ,

packages/mobile-sdk-demo/src/screens/index.ts

[warning] 133-133:
Replace ··const·existingSection·=·sections.find((section) with ····const·existingSection·=·sections.find(section


[warning] 130-130:
Replace ⏎··Array<{·title:·string;·items:·ScreenDescriptor[]·}>⏎>( with Array<{·title:·string;·items:·ScreenDescriptor[]·}>>(⏎··


[warning] 77-77:
Replace ⏎········?·'Walk·through·the·proof·sharing·experience'⏎······· with ·?·'Walk·through·the·proof·sharing·experience'


[warning] 60-60:
Replace ⏎········?·'View·the·mock·document·registration·flow'⏎······· with ·?·'View·the·mock·document·registration·flow'


[warning] 9-9:
Replace ⏎··|·'generate'⏎··|·'register'⏎··|·'prove'⏎··|·'camera'⏎··|·'nfc'⏎··|·'onboarding'⏎· with ·'generate'·|·'register'·|·'prove'·|·'camera'·|·'nfc'·|·'onboarding'

packages/mobile-sdk-demo/App.tsx

[warning] 104-104:
Replace ⏎················?·descriptor.subtitle(screenContext)⏎··············· with ·?·descriptor.subtitle(screenContext)


[warning] 100-100:
Replace (descriptor) with descriptor


[warning] 79-79:
Replace ⏎············styles.menuButtonSubtitle,⏎············disabled·?·styles.disabledSubtitleText·:·styles.placeholderButtonSubtitle,⏎·········· with styles.menuButtonSubtitle,·disabled·?·styles.disabledSubtitleText·:·styles.placeholderButtonSubtitle


[warning] 11-11:
Replace ⏎··orderedSectionEntries,⏎··screenMap,⏎··type·ScreenContext,⏎··type·ScreenRoute,⏎ with ·orderedSectionEntries,·screenMap,·type·ScreenContext,·type·ScreenRoute·

⏰ 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). (4)
  • GitHub Check: build-deps
  • GitHub Check: e2e-ios
  • GitHub Check: analyze-android
  • GitHub Check: analyze-ios

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/mobile-sdk-demo/src/screens/RegisterDocument.tsx (1)

37-44: Redact PII in mock display and remove PII logging

  • In packages/mobile-sdk-demo/src/screens/RegisterDocument.tsx, don’t use JSON.stringify(document) to dump the full IDDocument (which may include names, birth dates, addresses). Instead explicitly render only non-sensitive mock fields or sanitize/redact PII before display.
  • In packages/mobile-sdk-alpha/src/mock/generator.ts, remove the two console.log calls under if ('qrData' in result) { … }—they output real-looking name and extracted fields and violate “no PII logging” rules.
♻️ Duplicate comments (1)
packages/mobile-sdk-demo/package.json (1)

42-42: Previous version concern appears resolved.

The dependency is now at ^5.6.1, which is compatible with React Native 0.76.9 as specified in line 40. This addresses the compatibility issue raised in the previous review.

🧹 Nitpick comments (8)
packages/mobile-sdk-alpha/ios/SelfSDK/SelfMRZScannerModule.swift (1)

28-29: Consider filtering for the active scene or key window.

Using connectedScenes.first may return an unexpected scene in multi-window scenarios (e.g., iPad apps). A more robust approach would filter for the foreground active scene or find the key window explicitly.

Apply this diff for a more reliable implementation:

-        guard let windowScene = UIApplication.shared.connectedScenes.first as? UIWindowScene,
-              let rootViewController = windowScene.windows.first?.rootViewController else {
+        guard let windowScene = UIApplication.shared.connectedScenes
+                .first(where: { $0.activationState == .foregroundActive }) as? UIWindowScene,
+              let rootViewController = windowScene.windows.first(where: { $0.isKeyWindow })?.rootViewController else {
            reject("error", "Unable to find root view controller", nil)
            return
        }

This ensures you're selecting the active foreground scene and its key window, which is more predictable in edge cases.

packages/mobile-sdk-demo/src/screens/DocumentNFCScan.tsx (1)

17-18: Consider removing duplicate backgroundColor.

Both SafeAreaScrollView (line 17) and container style (line 43) specify backgroundColor: '#fafbfc'. Since the container's background is already set, you can remove the prop from SafeAreaScrollView to avoid redundancy.

-    <SafeAreaScrollView contentContainerStyle={styles.container} backgroundColor="#fafbfc">
+    <SafeAreaScrollView contentContainerStyle={styles.container}>

Also applies to: 41-43

packages/mobile-sdk-demo/src/screens/DocumentCamera.tsx (1)

17-18: Consider removing duplicate backgroundColor.

Both SafeAreaScrollView (line 17) and container style (line 42) specify backgroundColor: '#fafbfc'. Since the container's background is already set, you can remove the prop from SafeAreaScrollView to avoid redundancy.

-    <SafeAreaScrollView contentContainerStyle={styles.container} backgroundColor="#fafbfc">
+    <SafeAreaScrollView contentContainerStyle={styles.container}>

Also applies to: 40-42

packages/mobile-sdk-demo/src/screens/RegisterDocument.tsx (1)

20-21: Consider removing duplicate backgroundColor.

Both SafeAreaScrollView (line 20) and container style (line 55) specify backgroundColor: '#fafbfc'. Since the container's background is already set, you can remove the prop from SafeAreaScrollView to avoid redundancy.

-    <SafeAreaScrollView contentContainerStyle={styles.container} backgroundColor="#fafbfc">
+    <SafeAreaScrollView contentContainerStyle={styles.container}>

Also applies to: 53-55

packages/mobile-sdk-demo/src/screens/HomeScreen.tsx (2)

50-63: Consider simplifying the nested ternary for subtitle styling.

The nested ternary operator for subtitle text style (lines 54-58) reduces readability. While functional, extracting the style computation could improve maintainability.

Apply this diff to simplify the subtitle style logic:

+  const getSubtitleStyle = () => {
+    if (disabled) return styles.disabledSubtitleText;
+    if (isWorking) return styles.workingButtonSubtitle;
+    return styles.placeholderButtonSubtitle;
+  };
+
   {subtitle ? (
     <Text
       style={[
         styles.menuButtonSubtitle,
-        disabled
-          ? styles.disabledSubtitleText
-          : isWorking
-            ? styles.workingButtonSubtitle
-            : styles.placeholderButtonSubtitle,
+        getSubtitleStyle(),
       ]}
     >
       {subtitle}
     </Text>
   ) : null}

119-132: Remove unused style definitions.

The subtitle and tagline styles are defined but not referenced in the component's render logic. Consider removing them to reduce clutter.

Apply this diff to clean up unused styles:

-  subtitle: {
-    fontSize: 17,
-    color: '#656d76',
-    textAlign: 'center',
-    marginBottom: 8,
-    fontWeight: '500',
-  },
-  tagline: {
-    fontSize: 15,
-    color: '#8b949e',
-    textAlign: 'center',
-    paddingHorizontal: 20,
-    lineHeight: 22,
-  },
packages/mobile-sdk-demo/src/components/StandardHeader.tsx (1)

14-24: Add accessibility props for better screen reader support.

The back button lacks accessibility annotations, which impacts users relying on assistive technologies. Consider adding accessibilityRole and accessibilityLabel props.

Apply this diff to improve accessibility:

-    <TouchableOpacity style={styles.backButton} onPress={onBack}>
+    <TouchableOpacity 
+      style={styles.backButton} 
+      onPress={onBack}
+      accessibilityRole="button"
+      accessibilityLabel="Go back to previous screen"
+    >
       <Text style={styles.backButtonText}>← Back</Text>
     </TouchableOpacity>
packages/mobile-sdk-demo/src/screens/index.ts (1)

110-116: Add duplicate ScreenId validation.

The screenMap reduce logic doesn't detect duplicate id values in screenDescriptors. If duplicates exist, the later descriptor silently overwrites the earlier one, which could lead to navigation bugs.

Add a validation check after the reduce:

export const screenMap = screenDescriptors.reduce<Record<ScreenId, ScreenDescriptor>>(
  (map, descriptor) => {
    map[descriptor.id] = descriptor;
    return map;
  },
  {} as Record<ScreenId, ScreenDescriptor>,
);

// Validate no duplicate IDs
if (Object.keys(screenMap).length !== screenDescriptors.length) {
  throw new Error('Duplicate ScreenId detected in screenDescriptors');
}
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 417a428 and fe02213.

⛔ Files ignored due to path filters (2)
  • packages/mobile-sdk-demo/ios/Podfile.lock is excluded by !**/*.lock
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (17)
  • packages/mobile-sdk-alpha/ios/SelfSDK/PassportReader.swift (0 hunks)
  • packages/mobile-sdk-alpha/ios/SelfSDK/SelfMRZScannerModule.swift (1 hunks)
  • packages/mobile-sdk-demo/.gitignore (1 hunks)
  • packages/mobile-sdk-demo/App.tsx (1 hunks)
  • packages/mobile-sdk-demo/__tests__/App.test.tsx (1 hunks)
  • packages/mobile-sdk-demo/ios/SelfDemoApp/LaunchScreen.storyboard (0 hunks)
  • packages/mobile-sdk-demo/package.json (2 hunks)
  • packages/mobile-sdk-demo/src/components/StandardHeader.tsx (1 hunks)
  • packages/mobile-sdk-demo/src/screens/DocumentCamera.tsx (2 hunks)
  • packages/mobile-sdk-demo/src/screens/DocumentNFCScan.tsx (2 hunks)
  • packages/mobile-sdk-demo/src/screens/DocumentsList.tsx (1 hunks)
  • packages/mobile-sdk-demo/src/screens/GenerateMock.tsx (3 hunks)
  • packages/mobile-sdk-demo/src/screens/HomeScreen.tsx (1 hunks)
  • packages/mobile-sdk-demo/src/screens/ProofHistory.tsx (1 hunks)
  • packages/mobile-sdk-demo/src/screens/QRCodeViewFinder.tsx (2 hunks)
  • packages/mobile-sdk-demo/src/screens/RegisterDocument.tsx (3 hunks)
  • packages/mobile-sdk-demo/src/screens/index.ts (1 hunks)
💤 Files with no reviewable changes (2)
  • packages/mobile-sdk-demo/ios/SelfDemoApp/LaunchScreen.storyboard
  • packages/mobile-sdk-alpha/ios/SelfSDK/PassportReader.swift
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/mobile-sdk-demo/tests/App.test.tsx
  • packages/mobile-sdk-demo/src/screens/GenerateMock.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,jsx,sol,nr}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{js,ts,tsx,jsx,sol,nr}: 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., ***-***-1234 for passport numbers, J*** D*** for names).

Files:

  • packages/mobile-sdk-demo/src/screens/ProofHistory.tsx
  • packages/mobile-sdk-demo/src/screens/DocumentsList.tsx
  • packages/mobile-sdk-demo/src/components/StandardHeader.tsx
  • packages/mobile-sdk-demo/src/screens/DocumentNFCScan.tsx
  • packages/mobile-sdk-demo/src/screens/QRCodeViewFinder.tsx
  • packages/mobile-sdk-demo/src/screens/HomeScreen.tsx
  • packages/mobile-sdk-demo/src/screens/DocumentCamera.tsx
  • packages/mobile-sdk-demo/src/screens/index.ts
  • packages/mobile-sdk-demo/src/screens/RegisterDocument.tsx
  • packages/mobile-sdk-demo/App.tsx
🧠 Learnings (6)
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/src/index.ts : Re-export new SDK modules via packages/mobile-sdk-alpha/src/index.ts

Applied to files:

  • packages/mobile-sdk-demo/src/screens/index.ts
📚 Learning: 2025-09-22T11:10:22.019Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Applies to src/screens/**/*.{js,ts,tsx,jsx} : Lazy load screens using `React.lazy()`, organize screens by feature modules.

Applied to files:

  • packages/mobile-sdk-demo/src/screens/index.ts
  • packages/mobile-sdk-demo/App.tsx
📚 Learning: 2025-08-26T14:49:11.190Z
Learnt from: shazarre
PR: selfxyz/self#936
File: app/src/screens/passport/PassportNFCScanScreen.tsx:28-31
Timestamp: 2025-08-26T14:49:11.190Z
Learning: The main App.tsx file is located at app/App.tsx (not in app/src), and it properly wraps the entire app with SelfClientProvider at the top of the provider hierarchy, enabling useSelfClient() hook usage throughout all navigation screens.

Applied to files:

  • packages/mobile-sdk-demo/App.tsx
📚 Learning: 2025-09-22T11:10:22.019Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Applies to src/navigation/**/*.{js,ts,tsx,jsx} : Platform-specific initial routes: web uses 'Home', mobile uses 'Splash'.

Applied to files:

  • packages/mobile-sdk-demo/App.tsx
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/demo/** : Provide an in-SDK lightweight React Native demo under packages/mobile-sdk-alpha/demo/

Applied to files:

  • packages/mobile-sdk-demo/package.json
📚 Learning: 2025-09-22T11:10:22.019Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Test, build, and deploy scripts (`yarn test`, `yarn ios`, `yarn test:e2e:ios`, Fastlane, etc.) must be used for automation.

Applied to files:

  • packages/mobile-sdk-demo/package.json
🧬 Code graph analysis (8)
packages/mobile-sdk-demo/src/screens/ProofHistory.tsx (2)
packages/mobile-sdk-demo/src/components/SafeAreaScrollView.tsx (1)
  • SafeAreaScrollView (13-27)
packages/mobile-sdk-demo/src/components/StandardHeader.tsx (1)
  • StandardHeader (14-24)
packages/mobile-sdk-demo/src/screens/DocumentsList.tsx (2)
packages/mobile-sdk-demo/src/components/SafeAreaScrollView.tsx (1)
  • SafeAreaScrollView (13-27)
packages/mobile-sdk-demo/src/components/StandardHeader.tsx (1)
  • StandardHeader (14-24)
packages/mobile-sdk-demo/src/screens/DocumentNFCScan.tsx (2)
packages/mobile-sdk-demo/src/components/SafeAreaScrollView.tsx (1)
  • SafeAreaScrollView (13-27)
packages/mobile-sdk-demo/src/components/StandardHeader.tsx (1)
  • StandardHeader (14-24)
packages/mobile-sdk-demo/src/screens/QRCodeViewFinder.tsx (2)
packages/mobile-sdk-demo/src/components/SafeAreaScrollView.tsx (1)
  • SafeAreaScrollView (13-27)
packages/mobile-sdk-demo/src/components/StandardHeader.tsx (1)
  • StandardHeader (14-24)
packages/mobile-sdk-demo/src/screens/HomeScreen.tsx (2)
packages/mobile-sdk-demo/src/screens/index.ts (2)
  • ScreenContext (11-16)
  • orderedSectionEntries (118-131)
packages/mobile-sdk-demo/src/components/SafeAreaScrollView.tsx (1)
  • SafeAreaScrollView (13-27)
packages/mobile-sdk-demo/src/screens/DocumentCamera.tsx (2)
packages/mobile-sdk-demo/src/components/SafeAreaScrollView.tsx (1)
  • SafeAreaScrollView (13-27)
packages/mobile-sdk-demo/src/components/StandardHeader.tsx (1)
  • StandardHeader (14-24)
packages/mobile-sdk-demo/src/screens/RegisterDocument.tsx (2)
packages/mobile-sdk-demo/src/components/SafeAreaScrollView.tsx (1)
  • SafeAreaScrollView (13-27)
packages/mobile-sdk-demo/src/components/StandardHeader.tsx (1)
  • StandardHeader (14-24)
packages/mobile-sdk-demo/App.tsx (2)
packages/mobile-sdk-demo/src/screens/index.ts (3)
  • ScreenRoute (32-32)
  • ScreenContext (11-16)
  • screenMap (110-116)
packages/mobile-sdk-demo/src/screens/HomeScreen.tsx (1)
  • HomeScreen (15-97)
⏰ 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). (5)
  • GitHub Check: build-deps
  • GitHub Check: e2e-ios
  • GitHub Check: android-build-test
  • GitHub Check: analyze-ios
  • GitHub Check: analyze-android
🔇 Additional comments (16)
packages/mobile-sdk-demo/.gitignore (1)

8-9: LGTM!

Adding build/ to .gitignore is appropriate for keeping generated artifacts out of version control.

packages/mobile-sdk-demo/package.json (1)

16-17: LGTM!

The new convenience scripts (ia, install-app, reinstall) appropriately automate setup steps including prebuild and pod install, following the project's automation patterns.

Also applies to: 23-23

packages/mobile-sdk-demo/src/screens/QRCodeViewFinder.tsx (1)

17-34: LGTM! Consistent SafeArea adoption.

The refactor to SafeAreaScrollView and StandardHeader aligns with the PR objectives and matches the pattern applied across other screens. The UI polish is consistent and the implementation is correct.

packages/mobile-sdk-demo/src/screens/DocumentsList.tsx (3)

16-29: LGTM! Mock data for demo purposes.

The hardcoded mock document contains PII fields (name, birthDate, documentNumber), but this is clearly demo data (marked mock: true) and is only rendered in the UI, not logged. This aligns with the PR's objective to showcase the document registry flow.


31-62: LGTM! DocumentCard implementation is clean.

The DocumentCard component properly displays document metadata with appropriate styling and status badges. The date formatting is correctly implemented using toLocaleDateString.


64-81: LGTM! Consistent SafeArea pattern.

The screen correctly uses SafeAreaScrollView and StandardHeader, maintaining consistency with other refactored screens. The empty state provides helpful context for demo users.

packages/mobile-sdk-demo/src/screens/ProofHistory.tsx (3)

16-41: LGTM! Mock proof history data is appropriate.

The mock activities demonstrate proof verification flow without exposing actual PII. The data includes disclosure categories (e.g., "Age over 18", "Name") rather than real values, which is appropriate for a demo.


43-66: LGTM! ActivityCard implementation is well-structured.

The status dot color logic correctly handles success/pending/error states. The disclosure rendering properly joins the array of shared attributes for display.


68-83: LGTM! Consistent SafeArea implementation.

The screen follows the established SafeAreaScrollView and StandardHeader pattern. The empty state provides helpful context about the demo data.

packages/mobile-sdk-demo/App.tsx (3)

25-29: LGTM! Infinite re-render issue resolved.

The useEffect correctly moves the screen validation logic out of the render path, preventing the infinite re-render issue flagged in the previous review. The effect watches screen and only updates state when an invalid route is detected, allowing React to complete the render cycle before the state change.


31-44: LGTM! Descriptor-driven navigation is clean.

The dynamic screen loading via descriptor.load() and prop injection via descriptor.getProps?.(screenContext) provide a flexible, data-driven navigation system. Returning null for missing descriptors (Line 38) correctly short-circuits the render without triggering state updates.


18-23: LGTM! ScreenContext provides clean navigation API.

The screenContext object properly exposes navigation functions and shared state (mockDocument), enabling screens to navigate and share data without prop drilling. This aligns with the descriptor-based architecture.

packages/mobile-sdk-demo/src/screens/HomeScreen.tsx (1)

67-97: LGTM!

The main render logic correctly handles dynamic descriptor properties with safe optional chaining and appropriate fallbacks. The navigation and state management patterns are sound.

packages/mobile-sdk-demo/src/components/StandardHeader.tsx (1)

26-53: LGTM!

The style definitions are well-structured and semantically named. No issues found.

packages/mobile-sdk-demo/src/screens/index.ts (2)

118-131: LGTM!

The section grouping logic correctly organizes descriptors by sectionTitle. The use of find() inside reduce() is acceptable given the small dataset size.


34-108: LGTM — verified no console or logger calls expose mockDocument or PII in screen files.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (2)
packages/mobile-sdk-demo/src/utils/documentStore.ts (1)

12-18: Consider limitations of JSON clone for edge-case fields.

JSON.parse(JSON.stringify(...)) works for plain objects but silently drops Date, Map, Set, undefined, functions, and symbols. If IDDocument or DocumentCatalog may contain these types (e.g., Date fields for expiry), this approach will serialize them to strings or omit them. For a demo, this is likely acceptable if the types are purely JSON-serializable, but be aware of this constraint if the types evolve.

packages/mobile-sdk-demo/src/screens/index.ts (1)

39-127: Consider React.lazy() for proper lazy loading.

The load functions use require('./ScreenName').default, which loads modules synchronously at call-time rather than implementing true lazy loading. According to learnings, screens should be lazy-loaded using React.lazy() for optimal performance and code-splitting.

Refactor to use React.lazy():

 {
   id: 'generate',
   title: 'Generate Mock Document',
   subtitle: 'Create sample passport data for testing',
   sectionTitle: '⭐ Core Features',
   status: 'working',
-  load: () => require('./GenerateMock').default,
+  load: () => React.lazy(() => import('./GenerateMock')),
   getProps: ({ refreshDocuments, navigate }) => ({
     onDocumentStored: refreshDocuments,
     onNavigate: navigate,
     onBack: () => navigate('home'),
   }),
 },

Apply similar changes to all screen descriptors. This enables React Native's code-splitting and improves initial bundle size.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fc5c4ec and 7560493.

📒 Files selected for processing (7)
  • packages/mobile-sdk-demo/App.tsx (1 hunks)
  • packages/mobile-sdk-demo/jest.setup.js (2 hunks)
  • packages/mobile-sdk-demo/src/providers/SelfClientProvider.tsx (1 hunks)
  • packages/mobile-sdk-demo/src/screens/DocumentsList.tsx (1 hunks)
  • packages/mobile-sdk-demo/src/screens/GenerateMock.tsx (5 hunks)
  • packages/mobile-sdk-demo/src/screens/index.ts (1 hunks)
  • packages/mobile-sdk-demo/src/utils/documentStore.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,jsx,sol,nr}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{js,ts,tsx,jsx,sol,nr}: 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., ***-***-1234 for passport numbers, J*** D*** for names).

Files:

  • packages/mobile-sdk-demo/src/screens/GenerateMock.tsx
  • packages/mobile-sdk-demo/src/providers/SelfClientProvider.tsx
  • packages/mobile-sdk-demo/src/utils/documentStore.ts
  • packages/mobile-sdk-demo/jest.setup.js
  • packages/mobile-sdk-demo/src/screens/index.ts
  • packages/mobile-sdk-demo/src/screens/DocumentsList.tsx
  • packages/mobile-sdk-demo/App.tsx
🧠 Learnings (15)
📚 Learning: 2025-08-26T14:49:11.190Z
Learnt from: shazarre
PR: selfxyz/self#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:

  • packages/mobile-sdk-demo/src/providers/SelfClientProvider.tsx
  • packages/mobile-sdk-demo/App.tsx
📚 Learning: 2025-08-26T14:49:11.190Z
Learnt from: shazarre
PR: selfxyz/self#936
File: app/src/screens/passport/PassportNFCScanScreen.tsx:28-31
Timestamp: 2025-08-26T14:49:11.190Z
Learning: The main App.tsx file is located at app/App.tsx (not in app/src), and it properly wraps the entire app with SelfClientProvider at the top of the provider hierarchy, enabling useSelfClient() hook usage throughout all navigation screens.

Applied to files:

  • packages/mobile-sdk-demo/src/providers/SelfClientProvider.tsx
  • packages/mobile-sdk-demo/App.tsx
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Do NOT mock selfxyz/mobile-sdk-alpha in tests (avoid jest.mock('selfxyz/mobile-sdk-alpha') and replacing real functions with mocks)

Applied to files:

  • packages/mobile-sdk-demo/jest.setup.js
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to app/jest.setup.js : Provide comprehensive Jest setup in app/jest.setup.js with required mocks

Applied to files:

  • packages/mobile-sdk-demo/jest.setup.js
📚 Learning: 2025-09-22T11:10:22.019Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Applies to jest.setup.js : Comprehensive mocks for all native modules must be set up in `jest.setup.js`.

Applied to files:

  • packages/mobile-sdk-demo/jest.setup.js
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Use actual imports from selfxyz/mobile-sdk-alpha in tests

Applied to files:

  • packages/mobile-sdk-demo/jest.setup.js
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Test isPassportDataValid() with realistic synthetic passport data (never real user data)

Applied to files:

  • packages/mobile-sdk-demo/jest.setup.js
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Write integration tests that exercise the real validation logic (not mocks)

Applied to files:

  • packages/mobile-sdk-demo/jest.setup.js
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Ensure parseNFCResponse() works with representative, synthetic NFC data

Applied to files:

  • packages/mobile-sdk-demo/jest.setup.js
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Never use real user PII in tests; use only synthetic, anonymized, or approved test vectors

Applied to files:

  • packages/mobile-sdk-demo/jest.setup.js
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Verify extractMRZInfo() using published sample MRZ strings (e.g., ICAO examples)

Applied to files:

  • packages/mobile-sdk-demo/jest.setup.js
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/src/index.ts : Re-export new SDK modules via packages/mobile-sdk-alpha/src/index.ts

Applied to files:

  • packages/mobile-sdk-demo/src/screens/index.ts
📚 Learning: 2025-09-22T11:10:22.019Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Applies to src/screens/**/*.{js,ts,tsx,jsx} : Lazy load screens using `React.lazy()`, organize screens by feature modules.

Applied to files:

  • packages/mobile-sdk-demo/src/screens/index.ts
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/demo/** : Provide an in-SDK lightweight React Native demo under packages/mobile-sdk-alpha/demo/

Applied to files:

  • packages/mobile-sdk-demo/App.tsx
📚 Learning: 2025-08-26T14:49:15.210Z
Learnt from: shazarre
PR: selfxyz/self#936
File: app/src/screens/passport/PassportNFCScanScreen.web.tsx:8-11
Timestamp: 2025-08-26T14:49:15.210Z
Learning: The main App.tsx file is located at app/App.tsx (at the app root), not at app/src/App.tsx, and contains the SelfClientProvider wrapping the entire application.

Applied to files:

  • packages/mobile-sdk-demo/App.tsx
🧬 Code graph analysis (4)
packages/mobile-sdk-demo/src/screens/GenerateMock.tsx (2)
packages/mobile-sdk-demo/src/components/SafeAreaScrollView.tsx (1)
  • SafeAreaScrollView (13-27)
packages/mobile-sdk-demo/src/components/StandardHeader.tsx (1)
  • StandardHeader (14-24)
packages/mobile-sdk-demo/src/providers/SelfClientProvider.tsx (1)
packages/mobile-sdk-demo/src/utils/documentStore.ts (1)
  • inMemoryDocumentsAdapter (20-37)
packages/mobile-sdk-demo/src/screens/DocumentsList.tsx (2)
packages/mobile-sdk-demo/src/components/SafeAreaScrollView.tsx (1)
  • SafeAreaScrollView (13-27)
packages/mobile-sdk-demo/src/components/StandardHeader.tsx (1)
  • StandardHeader (14-24)
packages/mobile-sdk-demo/App.tsx (3)
packages/mobile-sdk-demo/src/screens/index.ts (3)
  • ScreenRoute (37-37)
  • ScreenContext (15-21)
  • screenMap (129-135)
packages/mobile-sdk-demo/src/screens/HomeScreen.tsx (1)
  • HomeScreen (15-97)
packages/mobile-sdk-demo/src/providers/SelfClientProvider.tsx (1)
  • SelfClientProvider (47-92)
🪛 GitHub Actions: Mobile SDK Demo CI
packages/mobile-sdk-demo/src/providers/SelfClientProvider.tsx

[error] 50-50: TypeScript: Property 'notification' is missing in type 'Adapters' required in type 'Adapters'.

🪛 GitHub Check: workspace-lint
packages/mobile-sdk-demo/src/screens/index.ts

[warning] 59-59:
Replace ⏎········?·'View·the·most·recently·generated·mock·document'⏎······· with ·?·'View·the·most·recently·generated·mock·document'


[warning] 7-7:
Replace ⏎··DocumentCatalog,⏎··DocumentMetadata,⏎··IDDocument,⏎ with ·DocumentCatalog,·DocumentMetadata,·IDDocument·

packages/mobile-sdk-demo/src/screens/DocumentsList.tsx

[warning] 105-105:
Insert ⏎···········

packages/mobile-sdk-demo/App.tsx

[warning] 7-7:
Replace ⏎··DocumentCatalog,⏎··DocumentMetadata,⏎ with ·DocumentCatalog,·DocumentMetadata·

⏰ 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). (4)
  • GitHub Check: analyze-ios
  • GitHub Check: build-deps
  • GitHub Check: e2e-ios
  • GitHub Check: analyze-android
🔇 Additional comments (20)
packages/mobile-sdk-demo/src/utils/documentStore.ts (2)

20-37: LGTM! Clean in-memory adapter for demo purposes.

The adapter implementation is straightforward and provides defensive cloning to prevent external mutation of the store. For a demo app, the lack of persistence and error handling is acceptable.


39-42: LGTM! Reset utility supports test isolation.

The reset function is useful for clearing state between test runs or demo resets.

packages/mobile-sdk-demo/App.tsx (4)

30-41: LGTM! Robust error handling for document refresh.

The refreshDocuments callback correctly resets catalog and selection state on failure, preventing stale or inconsistent data from persisting. The warning log does not expose PII.


53-57: LGTM! Fixes the prior re-render loop issue.

Moving the screen correction into a useEffect prevents the "Too many re-renders" crash that was flagged in the previous review. The render path now short-circuits (line 70 returns null) when the descriptor is missing, and the effect asynchronously corrects the route.


73-76: LGTM! Descriptor-driven dynamic screen loading.

The component dynamically loads and renders screens via descriptor.load() and descriptor.getProps(), enabling a clean registry-based navigation architecture.


59-61: Verify that useSelfClient returns a stable reference across renders.
Since refreshDocuments is memoized on selfClient, an unstable hook implementation will retrigger the effect on every render. Confirm (by inspecting the hook’s code or its documentation) that useSelfClient returns the same client instance unless its actual dependencies change.

packages/mobile-sdk-demo/src/screens/GenerateMock.tsx (4)

6-6: Minor: ActivityIndicator import retained correctly.

The diff shows ActivityIndicator is still imported (line 6) and used (line 146), so no functional change here.


81-101: LGTM! Proper document catalog integration.

The code correctly:

  • Generates a content-based document ID
  • Loads the current catalog and checks for duplicates
  • Saves the document and appends metadata for new entries
  • Sets the selected document ID and persists the catalog
  • Invokes the onDocumentStored callback (async/sync compatible)

No PII is logged (mock data only), and error handling is delegated to the outer try/catch.


110-112: LGTM! Consistent SafeArea + StandardHeader usage.

Replacing ScrollView with SafeAreaScrollView and adding StandardHeader aligns with the PR's polish goals and provides consistent navigation UX.


164-169: LGTM! Container styles updated for new wrapper.

The flexGrow: 1 ensures content fills the available space in the scroll view, and the background color matches the SafeAreaScrollView prop.

packages/mobile-sdk-demo/src/providers/SelfClientProvider.tsx (2)

64-66: Ensure signing error message doesn't expose sensitive keyRef data.

The error message includes keyRef in the output. If keyRef could contain sensitive information (tokens, private key identifiers, PII), this violates the coding guideline against logging sensitive data.

As per coding guidelines, verify that keyRef values in your codebase don't contain sensitive information. If they do, redact or mask the value in the error message:

-        async sign(_data: Uint8Array, keyRef: string): Promise<Uint8Array> {
-          throw new Error(`Signing is not supported in the demo client (keyRef: ${keyRef}).`);
-        },
+        async sign(_data: Uint8Array, keyRef: string): Promise<Uint8Array> {
+          throw new Error(`Signing is not supported in the demo client (keyRef: ${keyRef.slice(0, 8)}***).`);
+        },

20-29: Incorrect suggestion – React Native includes a global fetch implementation by default. No manual polyfill import is required in a standard React Native environment.

Likely an incorrect or invalid review comment.

packages/mobile-sdk-demo/src/screens/DocumentsList.tsx (3)

111-134: Document ID truncation pattern is clear.

The truncated document ID display (${metadata.id.slice(0, 8)}…${metadata.id.slice(-6)}) provides a good balance between readability and brevity for UUID-like identifiers.


49-79: Proper async cleanup pattern.

The useEffect implements a cancellation flag (active) correctly, preventing state updates after unmount. This avoids React warning about setting state on unmounted components.


32-41: Potential PII exposure in data preview.

The formatDataPreview function displays raw metadata.data content, which could contain sensitive information like passport numbers, names, DOB, or other PII. According to coding guidelines, sensitive data must be redacted/masked in any displayed output.

If metadata.data can contain PII, add redaction logic:

 const formatDataPreview = (metadata: DocumentMetadata) => {
   if (!metadata.data) {
     return 'No preview available';
   }
 
   const lines = metadata.data.split(/\r?\n/).filter(Boolean);
-  const preview = lines.slice(0, 2).join('\n');
+  // Redact potential PII fields before displaying
+  const preview = lines.slice(0, 2).map(line => {
+    // Example: redact passport numbers, names, etc.
+    return line.replace(/\b\d{9}\b/g, '***-***-***').replace(/\b[A-Z]{2}\d{7}\b/g, '***-****');
+  }).join('\n');
 
   return preview.length > 120 ? `${preview.slice(0, 117)}…` : preview;
 };

Alternatively, if mock data is guaranteed not to contain real PII, add a comment clarifying this assumption.

Likely an incorrect or invalid review comment.

packages/mobile-sdk-demo/jest.setup.js (2)

208-225: Correct pattern for augmenting SDK exports.

The mock correctly preserves actual exports from @selfxyz/mobile-sdk-alpha using jest.requireActual() and spreads them before adding test-specific mocks. This aligns with the learning that states "Use actual imports from selfxyz/mobile-sdk-alpha in tests" and "Do NOT mock selfxyz/mobile-sdk-alpha in tests."

Based on learnings.


109-114: Shared tree mock promotes consistency.

The createTreeMock helper consolidates common mock structure for IMT, LeanIMT, and SMT, reducing duplication and ensuring consistent behavior across zk-kit tree mocks.

packages/mobile-sdk-demo/src/screens/index.ts (3)

58-63: Dynamic subtitle and gating logic is well-designed.

The register screen's descriptor elegantly handles conditional behavior: subtitle changes based on whether a document exists, status upgrades from 'placeholder' to 'working', and the screen is disabled when no document is available. This provides clear user feedback and prevents invalid navigation.


102-111: Document count subtitle provides helpful context.

The dynamic subtitle for the documents screen gives users immediate visibility into their vault contents (0, 1, or multiple documents). The pluralization logic is correct and user-friendly.


129-150: Efficient descriptor transformation logic.

The screenMap and orderedSectionEntries construction uses reduce efficiently to transform the flat descriptor array into lookup and grouped structures. No unnecessary iterations or memory allocations.

…' of github.com:selfxyz/self into codex/review-mobile-sdk-demo-app-for-polish-suggestions
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

🧹 Nitpick comments (1)
packages/mobile-sdk-alpha/src/mock/generator.ts (1)

83-84: Remove or gate console logging of document data.

Even though this generates synthetic mock data, logging qrData and extractedFields sets a risky precedent. If this code path is ever invoked with real data or the pattern is copied elsewhere, it could lead to PII exposure.

Apply this diff to remove the logs:

-    const result = genMockIdDoc(idDocInput);
-    if ('qrData' in result) {
-      console.log('Generated Aadhaar qrData:', result.qrData);
-      console.log('Generated Aadhaar extractedFields:', result.extractedFields);
-    }
-    return result;
+    return genMockIdDoc(idDocInput);

Alternatively, gate behind an explicit debug flag if debugging is critical:

     const result = genMockIdDoc(idDocInput);
     if ('qrData' in result) {
-      console.log('Generated Aadhaar qrData:', result.qrData);
-      console.log('Generated Aadhaar extractedFields:', result.extractedFields);
+      if (__DEV__) {
+        console.log('[DEBUG] Generated mock Aadhaar');
+      }
     }
     return result;

As per coding guidelines.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7560493 and 0580892.

⛔ Files ignored due to path filters (3)
  • app/Gemfile.lock is excluded by !**/*.lock
  • app/ios/Podfile.lock is excluded by !**/*.lock
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (6)
  • app/package.json (1 hunks)
  • packages/mobile-sdk-alpha/src/mock/generator.ts (2 hunks)
  • packages/mobile-sdk-demo/App.tsx (1 hunks)
  • packages/mobile-sdk-demo/src/screens/DocumentsList.tsx (1 hunks)
  • packages/mobile-sdk-demo/src/screens/GenerateMock.tsx (5 hunks)
  • packages/mobile-sdk-demo/src/screens/index.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/mobile-sdk-demo/App.tsx
🧰 Additional context used
📓 Path-based instructions (4)
app/package.json

📄 CodeRabbit inference engine (.cursor/rules/mobile-sdk-migration.mdc)

Expose a 'test:build' script in the app's package.json that builds deps, types, performs bundle analysis, and runs tests

Files:

  • app/package.json
packages/mobile-sdk-alpha/**/*.{ts,tsx}

📄 CodeRabbit inference engine (packages/mobile-sdk-alpha/AGENTS.md)

packages/mobile-sdk-alpha/**/*.{ts,tsx}: Use strict TypeScript type checking across the codebase
Follow ESLint TypeScript-specific rules
Avoid introducing circular dependencies

Files:

  • packages/mobile-sdk-alpha/src/mock/generator.ts
**/*.{js,ts,tsx,jsx,sol,nr}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{js,ts,tsx,jsx,sol,nr}: 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., ***-***-1234 for passport numbers, J*** D*** for names).

Files:

  • packages/mobile-sdk-alpha/src/mock/generator.ts
  • packages/mobile-sdk-demo/src/screens/DocumentsList.tsx
  • packages/mobile-sdk-demo/src/screens/index.ts
  • packages/mobile-sdk-demo/src/screens/GenerateMock.tsx
packages/mobile-sdk-alpha/**/*.{ts,tsx,js,jsx}

⚙️ CodeRabbit configuration file

packages/mobile-sdk-alpha/**/*.{ts,tsx,js,jsx}: Review alpha mobile SDK code for:

  • API consistency with core SDK
  • Platform-neutral abstractions
  • Performance considerations
  • Clear experimental notes or TODOs

Files:

  • packages/mobile-sdk-alpha/src/mock/generator.ts
🧠 Learnings (3)
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Test isPassportDataValid() with realistic synthetic passport data (never real user data)

Applied to files:

  • packages/mobile-sdk-alpha/src/mock/generator.ts
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/src/index.ts : Re-export new SDK modules via packages/mobile-sdk-alpha/src/index.ts

Applied to files:

  • packages/mobile-sdk-demo/src/screens/index.ts
📚 Learning: 2025-09-22T11:10:22.019Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Applies to src/screens/**/*.{js,ts,tsx,jsx} : Lazy load screens using `React.lazy()`, organize screens by feature modules.

Applied to files:

  • packages/mobile-sdk-demo/src/screens/index.ts
🧬 Code graph analysis (3)
packages/mobile-sdk-alpha/src/mock/generator.ts (1)
common/src/utils/types.ts (2)
  • PassportData (65-76)
  • AadhaarData (15-22)
packages/mobile-sdk-demo/src/screens/DocumentsList.tsx (2)
packages/mobile-sdk-demo/src/components/SafeAreaScrollView.tsx (1)
  • SafeAreaScrollView (13-27)
packages/mobile-sdk-demo/src/components/StandardHeader.tsx (1)
  • StandardHeader (14-24)
packages/mobile-sdk-demo/src/screens/GenerateMock.tsx (2)
packages/mobile-sdk-demo/src/components/SafeAreaScrollView.tsx (1)
  • SafeAreaScrollView (13-27)
packages/mobile-sdk-demo/src/components/StandardHeader.tsx (1)
  • StandardHeader (14-24)
🔇 Additional comments (1)
packages/mobile-sdk-alpha/src/mock/generator.ts (1)

5-5: LGTM! Type changes align with implementation.

The addition of AadhaarData to imports and the updated return type Promise<PassportData | AadhaarData> correctly reflect the function's ability to generate either document type based on selectedDocumentType.

Also applies to: 51-51

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/mobile-sdk-alpha/src/stores/protocolStore.ts (1)

5-27: Fix duplicate imports and sort order.

The type-only import of Environment at line 27 should be merged into the existing import block from @selfxyz/common (lines 8-26). This addresses both the duplicate import warning and the import sorting failure.

Apply this diff:

 import { create } from 'zustand';
 
-import type { DeployedCircuits, DocumentCategory, OfacTree } from '@selfxyz/common';
+import type { DeployedCircuits, DocumentCategory, Environment, OfacTree } from '@selfxyz/common';
 import {
   API_URL,
   API_URL_STAGING,
   CSCA_TREE_URL,
   CSCA_TREE_URL_ID_CARD,
   CSCA_TREE_URL_STAGING,
   CSCA_TREE_URL_STAGING_ID_CARD,
   DSC_TREE_URL,
   DSC_TREE_URL_ID_CARD,
   DSC_TREE_URL_STAGING,
   DSC_TREE_URL_STAGING_ID_CARD,
   fetchOfacTrees,
   IDENTITY_TREE_URL,
   IDENTITY_TREE_URL_ID_CARD,
   IDENTITY_TREE_URL_STAGING,
   IDENTITY_TREE_URL_STAGING_ID_CARD,
   TREE_URL,
   TREE_URL_STAGING,
 } from '@selfxyz/common';
-import type { Environment } from '@selfxyz/common';
 
 import type { SelfClient } from '../types/public';
♻️ Duplicate comments (2)
packages/mobile-sdk-demo/src/providers/SelfClientProvider.tsx (1)

78-82: LGTM! Notification adapter now satisfies type contract.

The missing notification adapter has been added with a no-op implementation, resolving the TypeScript error flagged in the previous review.

packages/mobile-sdk-demo/src/screens/DocumentsList.tsx (1)

126-126: LGTM! documentCategory guard now prevents crashes.

The past review comment flagged a missing guard for documentCategory. This has been correctly resolved with ?? 'unknown', ensuring the UI renders even when the field is absent.

🧹 Nitpick comments (1)
packages/mobile-sdk-demo/src/screens/HomeScreen.tsx (1)

19-66: Consider extracting MenuButton if reused elsewhere.

The MenuButton component is well-structured and encapsulates button styling logic. If it's used in other screens, consider extracting it to a shared component file for reusability and consistency.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0580892 and 6602f67.

⛔ Files ignored due to path filters (3)
  • packages/mobile-sdk-demo/ios/Podfile.lock is excluded by !**/*.lock
  • packages/mobile-sdk-demo/src/assets/images/logo.svg is excluded by !**/*.svg
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (14)
  • .eslintignore (1 hunks)
  • .watchmanconfig (1 hunks)
  • app/package.json (1 hunks)
  • common/index.ts (1 hunks)
  • packages/mobile-sdk-alpha/src/stores/protocolStore.ts (1 hunks)
  • packages/mobile-sdk-demo/__tests__/App.test.tsx (0 hunks)
  • packages/mobile-sdk-demo/jest.setup.js (1 hunks)
  • packages/mobile-sdk-demo/metro.config.cjs (4 hunks)
  • packages/mobile-sdk-demo/package.json (3 hunks)
  • packages/mobile-sdk-demo/src/providers/SelfClientProvider.tsx (1 hunks)
  • packages/mobile-sdk-demo/src/screens/DocumentsList.tsx (1 hunks)
  • packages/mobile-sdk-demo/src/screens/HomeScreen.tsx (1 hunks)
  • packages/mobile-sdk-demo/src/screens/index.ts (1 hunks)
  • packages/mobile-sdk-demo/types/svg.d.ts (1 hunks)
💤 Files with no reviewable changes (1)
  • packages/mobile-sdk-demo/tests/App.test.tsx
✅ Files skipped from review due to trivial changes (1)
  • .watchmanconfig
🚧 Files skipped from review as they are similar to previous changes (2)
  • app/package.json
  • packages/mobile-sdk-demo/package.json
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,ts,tsx,jsx,sol,nr}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{js,ts,tsx,jsx,sol,nr}: 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., ***-***-1234 for passport numbers, J*** D*** for names).

Files:

  • common/index.ts
  • packages/mobile-sdk-demo/src/screens/index.ts
  • packages/mobile-sdk-demo/src/screens/HomeScreen.tsx
  • packages/mobile-sdk-demo/src/screens/DocumentsList.tsx
  • packages/mobile-sdk-demo/src/providers/SelfClientProvider.tsx
  • packages/mobile-sdk-demo/jest.setup.js
  • packages/mobile-sdk-demo/types/svg.d.ts
  • packages/mobile-sdk-alpha/src/stores/protocolStore.ts
packages/mobile-sdk-alpha/**/*.{ts,tsx}

📄 CodeRabbit inference engine (packages/mobile-sdk-alpha/AGENTS.md)

packages/mobile-sdk-alpha/**/*.{ts,tsx}: Use strict TypeScript type checking across the codebase
Follow ESLint TypeScript-specific rules
Avoid introducing circular dependencies

Files:

  • packages/mobile-sdk-alpha/src/stores/protocolStore.ts
packages/mobile-sdk-alpha/**/*.{ts,tsx,js,jsx}

⚙️ CodeRabbit configuration file

packages/mobile-sdk-alpha/**/*.{ts,tsx,js,jsx}: Review alpha mobile SDK code for:

  • API consistency with core SDK
  • Platform-neutral abstractions
  • Performance considerations
  • Clear experimental notes or TODOs

Files:

  • packages/mobile-sdk-alpha/src/stores/protocolStore.ts
🧠 Learnings (12)
📚 Learning: 2025-08-25T14:25:57.586Z
Learnt from: aaronmgdr
PR: selfxyz/self#951
File: app/src/providers/authProvider.web.tsx:17-18
Timestamp: 2025-08-25T14:25:57.586Z
Learning: The selfxyz/mobile-sdk-alpha/constants/analytics import path is properly configured with SDK exports, Metro aliases, and TypeScript resolution. Import changes from @/consts/analytics to this path are part of valid analytics migration, not TypeScript resolution issues.

Applied to files:

  • packages/mobile-sdk-demo/metro.config.cjs
  • packages/mobile-sdk-alpha/src/stores/protocolStore.ts
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/src/index.ts : Re-export new SDK modules via packages/mobile-sdk-alpha/src/index.ts

Applied to files:

  • packages/mobile-sdk-demo/src/screens/index.ts
📚 Learning: 2025-09-22T11:10:22.019Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Applies to src/screens/**/*.{js,ts,tsx,jsx} : Lazy load screens using `React.lazy()`, organize screens by feature modules.

Applied to files:

  • packages/mobile-sdk-demo/src/screens/index.ts
📚 Learning: 2025-08-26T14:49:11.190Z
Learnt from: shazarre
PR: selfxyz/self#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:

  • packages/mobile-sdk-demo/src/providers/SelfClientProvider.tsx
📚 Learning: 2025-08-26T14:49:11.190Z
Learnt from: shazarre
PR: selfxyz/self#936
File: app/src/screens/passport/PassportNFCScanScreen.tsx:28-31
Timestamp: 2025-08-26T14:49:11.190Z
Learning: The main App.tsx file is located at app/App.tsx (not in app/src), and it properly wraps the entire app with SelfClientProvider at the top of the provider hierarchy, enabling useSelfClient() hook usage throughout all navigation screens.

Applied to files:

  • packages/mobile-sdk-demo/src/providers/SelfClientProvider.tsx
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to app/jest.setup.js : Provide comprehensive Jest setup in app/jest.setup.js with required mocks

Applied to files:

  • packages/mobile-sdk-demo/jest.setup.js
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Do NOT mock selfxyz/mobile-sdk-alpha in tests (avoid jest.mock('selfxyz/mobile-sdk-alpha') and replacing real functions with mocks)

Applied to files:

  • packages/mobile-sdk-demo/jest.setup.js
📚 Learning: 2025-09-22T11:10:22.019Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Applies to jest.setup.js : Comprehensive mocks for all native modules must be set up in `jest.setup.js`.

Applied to files:

  • packages/mobile-sdk-demo/jest.setup.js
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/tests/setup.ts : Provide Vitest setup file at packages/mobile-sdk-alpha/tests/setup.ts to suppress console noise

Applied to files:

  • packages/mobile-sdk-demo/jest.setup.js
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Use actual imports from selfxyz/mobile-sdk-alpha in tests

Applied to files:

  • packages/mobile-sdk-demo/jest.setup.js
  • packages/mobile-sdk-alpha/src/stores/protocolStore.ts
📚 Learning: 2025-09-22T11:10:22.019Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Applies to tests/**/*.test.{js,ts,tsx,jsx} : Mock console.error to avoid test output clutter.

Applied to files:

  • packages/mobile-sdk-demo/jest.setup.js
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Test isPassportDataValid() with realistic synthetic passport data (never real user data)

Applied to files:

  • packages/mobile-sdk-demo/jest.setup.js
🧬 Code graph analysis (3)
packages/mobile-sdk-demo/src/screens/HomeScreen.tsx (2)
packages/mobile-sdk-demo/src/screens/index.ts (2)
  • ScreenContext (11-17)
  • orderedSectionEntries (113-126)
packages/mobile-sdk-demo/src/components/SafeAreaScrollView.tsx (1)
  • SafeAreaScrollView (13-27)
packages/mobile-sdk-demo/src/screens/DocumentsList.tsx (2)
packages/mobile-sdk-demo/src/components/SafeAreaScrollView.tsx (1)
  • SafeAreaScrollView (13-27)
packages/mobile-sdk-demo/src/components/StandardHeader.tsx (1)
  • StandardHeader (14-24)
packages/mobile-sdk-demo/src/providers/SelfClientProvider.tsx (1)
packages/mobile-sdk-demo/src/utils/documentStore.ts (1)
  • inMemoryDocumentsAdapter (20-37)
🪛 GitHub Actions: Mobile SDK CI
packages/mobile-sdk-alpha/src/stores/protocolStore.ts

[error] 5-5: simple-import-sort/imports: Run autofix to sort these imports!

🪛 GitHub Actions: Workspace CI
packages/mobile-sdk-alpha/src/stores/protocolStore.ts

[error] 5-5: simple-import-sort/imports: Run autofix to sort these imports!

🪛 GitHub Check: lint
packages/mobile-sdk-alpha/src/stores/protocolStore.ts

[failure] 27-27:
'/home/runner/work/self/self/common/dist/esm/index.js' imported multiple times

🪛 GitHub Check: workspace-lint
packages/mobile-sdk-alpha/src/stores/protocolStore.ts

[failure] 27-27:
'/home/runner/work/self/self/common/dist/esm/index.js' imported multiple times

⏰ 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). (5)
  • GitHub Check: android-build-test
  • GitHub Check: e2e-ios
  • GitHub Check: build-deps
  • GitHub Check: analyze-android
  • GitHub Check: analyze-ios
🔇 Additional comments (14)
common/index.ts (1)

21-22: LGTM!

The type-only export of Environment correctly exposes the type from the common package's public API surface, enabling clean type-only imports in dependent packages.

packages/mobile-sdk-demo/src/providers/SelfClientProvider.tsx (3)

20-29: LGTM! Fetch polyfill approach is appropriate for demo.

The createFetch factory gracefully handles environments without fetch by returning a function that throws a descriptive error. This is acceptable for a demo app.


31-43: LGTM! WebSocket stub is appropriate for demo.

The createWsAdapter returns a stub implementation where send() throws and lifecycle methods are no-ops. This is acceptable for a demo environment.


45-45: LGTM! Using @noble/hashes for cryptographic hash.

The hash function correctly uses sha256 from @noble/hashes, which is a well-audited library recommended in the learnings.

packages/mobile-sdk-demo/src/screens/HomeScreen.tsx (2)

75-96: LGTM! Descriptor-driven rendering is clean and dynamic.

The implementation correctly iterates over orderedSectionEntries, dynamically computes status/disabled/subtitle from descriptors, and renders buttons with proper navigation. This is a solid pattern for data-driven UIs.


68-73: LGTM! Safe area and header layout are appropriate.

The use of SafeAreaScrollView and the header layout with logo and title provide a clean, consistent UI structure.

packages/mobile-sdk-demo/src/screens/DocumentsList.tsx (3)

49-79: LGTM! useEffect cleanup prevents state updates after unmount.

The active flag pattern correctly guards against state updates when the component unmounts during an async operation. This is a best practice for avoiding memory leaks and warnings.


24-41: LGTM! Helper functions provide good UX.

humanizeDocumentType handles mock prefixes cleanly, and formatDataPreview limits the preview to 2 lines and 120 characters, providing a clear and concise display for users.


112-135: LGTM! Document card rendering is clean and informative.

The card layout displays document type, registration status, metadata, preview, and truncated ID in a well-structured format. Status badge logic is correct and styles are appropriate.

packages/mobile-sdk-demo/src/screens/index.ts (4)

35-103: LGTM! Descriptor-based screen registry is well-designed.

The screenDescriptors array provides a clean, data-driven approach to screen management. Lazy loading via require() aligns with best practices for screen loading, and dynamic functions (getStatus, isDisabled, subtitle) enable context-aware UI behavior.


49-63: LGTM! Register screen gating logic is correct.

The isDisabled function correctly gates the "Register Document" action until a document is selected, and the dynamic subtitle provides clear feedback to users. This aligns with the PR objectives.


82-102: LGTM! Documents screen subtitle provides dynamic feedback.

The subtitle function dynamically displays the document count in the vault, providing clear and useful feedback to users. This is a good UX pattern.


105-126: LGTM! Derived data structures are efficient and clean.

screenMap and orderedSectionEntries are correctly derived from screenDescriptors using reduce(), providing efficient lookup and sectioned rendering for the UI.

packages/mobile-sdk-demo/jest.setup.js (1)

84-92: LGTM! Console mocks properly configured.

The console methods are correctly mocked to suppress test output clutter, following established best practices for Jest setup files.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/mobile-sdk-alpha/tests/processing/mrz.test.ts (1)

25-85: Remove duplicate test blocks.

Lines 25-33, 47-54, and 67-74 contain identical tests for valid TD1 MRZ parsing. This appears to be unintentional duplication.

Apply this diff to remove duplicates:

-  it('parses valid TD1 MRZ', () => {
-    const info = extractMRZInfo(sampleTD1);
-    expect(info.documentNumber).toBe('X4RTBPFW4');
-    expect(info.issuingCountry).toBe('FRA');
-    expect(info.dateOfBirth).toBe('900713');
-    expect(info.dateOfExpiry).toBe('300211');
-    expect(info.validation?.overall).toBe(true);
-  });
-
-  it('rejects invalid TD1 MRZ', () => {
-    const invalid = `FRAX4RTBPFW46`;
-    expect(() => extractMRZInfo(invalid)).toThrow();
-  });
-
-  it('Fails overall validation for invalid TD1 MRZ', () => {
-    const invalid = `IDFRAX4RTBPFW46`;
-    const info = extractMRZInfo(invalid);
-    expect(info.validation?.overall).toBe(false);
-  });
-
-  it('parses valid TD1 MRZ', () => {
-    const info = extractMRZInfo(sampleTD1);
-    expect(info.documentNumber).toBe('X4RTBPFW4');
-    expect(info.issuingCountry).toBe('FRA');
-    expect(info.dateOfBirth).toBe('900713');
-    expect(info.dateOfExpiry).toBe('300211');
-    expect(info.validation?.overall).toBe(true);
-  });
-
-  it('rejects invalid TD1 MRZ', () => {
-    const invalid = `FRAX4RTBPFW46`;
-    expect(() => extractMRZInfo(invalid)).toThrow();
-  });
-
-  it('Fails overall validation for invalid TD1 MRZ', () => {
-    const invalid = `IDFRAX4RTBPFW46`;
-    const info = extractMRZInfo(invalid);
-    expect(info.validation?.overall).toBe(false);
-  });
-
♻️ Duplicate comments (2)
packages/mobile-sdk-demo/src/providers/SelfClientProvider.tsx (2)

84-88: Notification adapter properly added.

The notification adapter with a no-op registerDeviceToken method has been added, resolving the previous TypeScript compilation error. This satisfies the Adapters type contract.


65-67: Signing error message no longer exposes sensitive keyRef.

The keyRef parameter is now properly prefixed with underscore (_keyRef) to indicate it's unused, and the error message no longer includes it. This resolves the previous security concern.

🧹 Nitpick comments (7)
common/src/polyfills/crypto.ts (1)

50-57: Consider extracting Buffer normalization into a helper function.

The Buffer-to-Uint8Array conversion logic is correct but duplicated between createHash.update and createHmac.update. The double-negative conditional (!(data instanceof Uint8Array && data.constructor === Uint8Array)) is also hard to parse at a glance.

Consider extracting this into a shared helper:

+function normalizeToUint8Array(data: Uint8Array): Uint8Array {
+  // Convert Buffer or other TypedArray subclasses to pure Uint8Array
+  // noble/hashes requires pure Uint8Array, not Buffer subclasses
+  const isPureUint8Array = data instanceof Uint8Array && data.constructor === Uint8Array;
+  if (isPureUint8Array) {
+    return data;
+  }
+  return new Uint8Array(data.buffer, data.byteOffset, data.byteLength);
+}
+
 function createHash(algorithm: string) {
   // ...
   return {
     update(data: string | Uint8Array) {
       if (finalized) {
         throw new Error('Cannot update after calling digest(). Hash instance has been finalized.');
       }
       if (typeof data === 'string') {
         hasher.update(new TextEncoder().encode(data));
       } else {
-        // Convert Buffer to pure Uint8Array if needed
-        // Buffer is a subclass of Uint8Array but noble/hashes expects pure Uint8Array
-        const bytes =
-          ArrayBuffer.isView(data) &&
-          !(data instanceof Uint8Array && data.constructor === Uint8Array)
-            ? new Uint8Array(data.buffer, data.byteOffset, data.byteLength)
-            : data;
-        hasher.update(bytes);
+        hasher.update(normalizeToUint8Array(data));
       }
       return this;
     },

Then apply the same helper in createHmac.update (lines 106-117).

packages/mobile-sdk-alpha/src/processing/mrz.ts (1)

265-273: Consider documenting TD3 character length assumptions.

The hardcoded 88/90 character lengths for single-line TD3 MRZ are correct per ICAO 9303, but a brief inline comment would improve maintainability.

   // Handle single-line MRZ strings (common for stored data)
-  // TD3 format: 88 or 90 characters total (2 lines of 44 or 45 chars each)
+  // TD3 format per ICAO 9303: 88 chars (2×44) or 90 chars (2×45)
+  // 88-char: standard passport format
+  // 90-char: variant with extended optional data
   if (lines.length === 1) {
packages/mobile-sdk-demo/src/utils/ethers.ts (1)

36-40: LGTM! Defensive Uint8Array coercion for sha256.

The runtime check ensures a pure Uint8Array is returned rather than a subclass like Buffer. This aligns with test expectations and provides cross-platform compatibility. The performance overhead is negligible.

Optional: If @noble/hashes guarantees Uint8Array return types, the constructor check could be simplified or removed. Consider verifying the library's type guarantees:

#!/bin/bash
# Check noble/hashes source for digest return types
rg -n -A3 -B3 "digest\(\)" node_modules/@noble/hashes/esm/sha256.js node_modules/@noble/hashes/esm/sha512.js
packages/mobile-sdk-demo/src/utils/documentStore.ts (2)

16-22: Consider deep cloning for nested structures with circular references.

The JSON.parse(JSON.stringify()) pattern works for simple objects but will fail if the catalog or document ever contains circular references, functions, or non-serializable types (other than BigInt, which is already handled separately). While this is likely fine for the current data structures, consider documenting this limitation or using a more robust cloning library if the data model becomes more complex.


51-61: Re-parsing logic may execute unnecessarily on every load.

The condition !passportDoc.dsc_parsed || !passportDoc.passportMetadata will trigger re-parsing even if only one field is missing. If dsc_parsed exists but passportMetadata doesn't (or vice versa), you'll re-parse unnecessarily. Consider checking if the data is truly corrupted (e.g., checking for specific corruption markers) rather than just missing fields.

However, since this is defensive code to handle BigInt corruption after JSON serialization, and re-parsing is safe, this is likely acceptable for the demo environment.

packages/mobile-sdk-alpha/src/mock/generator.ts (1)

75-75: Hardcoded sex value reduces mock data diversity.

The sex field is hardcoded to 'M' for all documents. While this may be intentional for the demo, consider making this configurable or randomizing it to create more diverse mock data. This would make the mock documents more representative of real-world data variations.

packages/mobile-sdk-demo/src/screens/GenerateMock.tsx (1)

104-104: Consider type safety for mrz/qrData extraction.

The line uses (doc as any) type assertions to access mrz and qrData. While this works, it bypasses TypeScript's type checking. Consider using type guards or narrowing the type based on documentCategory to maintain type safety.

Example improvement:

-          data: isMRZDocument(doc) ? (doc as any).mrz : 'qrData' in doc ? (doc as any).qrData : '',
+          data: isMRZDocument(doc) 
+            ? (doc as PassportData).mrz 
+            : doc.documentCategory === 'aadhaar' 
+              ? (doc as AadhaarData).qrData 
+              : '',
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6602f67 and 3ce39ff.

⛔ Files ignored due to path filters (2)
  • packages/mobile-sdk-demo/ios/Podfile.lock is excluded by !**/*.lock
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (32)
  • .cursorignore (0 hunks)
  • common/src/polyfills/crypto.ts (2 hunks)
  • packages/mobile-sdk-alpha/src/browser.ts (1 hunks)
  • packages/mobile-sdk-alpha/src/index.ts (1 hunks)
  • packages/mobile-sdk-alpha/src/mock/generator.ts (5 hunks)
  • packages/mobile-sdk-alpha/src/mrz/index.ts (1 hunks)
  • packages/mobile-sdk-alpha/src/processing/mrz.ts (1 hunks)
  • packages/mobile-sdk-alpha/src/stores/protocolStore.ts (1 hunks)
  • packages/mobile-sdk-alpha/tests/processing/mrz.test.ts (2 hunks)
  • packages/mobile-sdk-demo/__tests__/cryptoPolyfills.test.ts (2 hunks)
  • packages/mobile-sdk-demo/__tests__/documentStore.simple.test.ts (1 hunks)
  • packages/mobile-sdk-demo/__tests__/secureStorage.test.ts (1 hunks)
  • packages/mobile-sdk-demo/android/app/build.gradle (1 hunks)
  • packages/mobile-sdk-demo/crypto-polyfill.js (0 hunks)
  • packages/mobile-sdk-demo/ios/SelfDemoApp/Info.plist (1 hunks)
  • packages/mobile-sdk-demo/jest.config.cjs (0 hunks)
  • packages/mobile-sdk-demo/jest.setup.js (0 hunks)
  • packages/mobile-sdk-demo/package.json (2 hunks)
  • packages/mobile-sdk-demo/src/components/StandardHeader.tsx (1 hunks)
  • packages/mobile-sdk-demo/src/providers/SelfClientProvider.tsx (1 hunks)
  • packages/mobile-sdk-demo/src/screens/DocumentsList.tsx (1 hunks)
  • packages/mobile-sdk-demo/src/screens/GenerateMock.tsx (1 hunks)
  • packages/mobile-sdk-demo/src/screens/RegisterDocument.tsx (1 hunks)
  • packages/mobile-sdk-demo/src/screens/index.ts (1 hunks)
  • packages/mobile-sdk-demo/src/utils/documentStore.ts (1 hunks)
  • packages/mobile-sdk-demo/src/utils/ethers.ts (1 hunks)
  • packages/mobile-sdk-demo/src/utils/secureStorage.ts (1 hunks)
  • packages/mobile-sdk-demo/tests/cryptoPolyfill.test.ts (5 hunks)
  • packages/mobile-sdk-demo/tests/setup.ts (1 hunks)
  • packages/mobile-sdk-demo/tsconfig.json (1 hunks)
  • packages/mobile-sdk-demo/types/svg.d.ts (1 hunks)
  • packages/mobile-sdk-demo/vitest.config.ts (1 hunks)
💤 Files with no reviewable changes (4)
  • packages/mobile-sdk-demo/crypto-polyfill.js
  • .cursorignore
  • packages/mobile-sdk-demo/jest.setup.js
  • packages/mobile-sdk-demo/jest.config.cjs
🚧 Files skipped from review as they are similar to previous changes (3)
  • packages/mobile-sdk-demo/src/screens/index.ts
  • packages/mobile-sdk-demo/src/components/StandardHeader.tsx
  • packages/mobile-sdk-demo/src/screens/DocumentsList.tsx
🧰 Additional context used
📓 Path-based instructions (8)
packages/mobile-sdk-alpha/**/*.{ts,tsx}

📄 CodeRabbit inference engine (packages/mobile-sdk-alpha/AGENTS.md)

packages/mobile-sdk-alpha/**/*.{ts,tsx}: Use strict TypeScript type checking across the codebase
Follow ESLint TypeScript-specific rules
Avoid introducing circular dependencies

Files:

  • packages/mobile-sdk-alpha/src/stores/protocolStore.ts
  • packages/mobile-sdk-alpha/tests/processing/mrz.test.ts
  • packages/mobile-sdk-alpha/src/browser.ts
  • packages/mobile-sdk-alpha/src/processing/mrz.ts
  • packages/mobile-sdk-alpha/src/index.ts
  • packages/mobile-sdk-alpha/src/mock/generator.ts
  • packages/mobile-sdk-alpha/src/mrz/index.ts
**/*.{js,ts,tsx,jsx,sol,nr}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{js,ts,tsx,jsx,sol,nr}: 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., ***-***-1234 for passport numbers, J*** D*** for names).

Files:

  • packages/mobile-sdk-alpha/src/stores/protocolStore.ts
  • packages/mobile-sdk-alpha/tests/processing/mrz.test.ts
  • packages/mobile-sdk-demo/__tests__/documentStore.simple.test.ts
  • packages/mobile-sdk-alpha/src/browser.ts
  • packages/mobile-sdk-demo/src/utils/ethers.ts
  • packages/mobile-sdk-demo/src/providers/SelfClientProvider.tsx
  • packages/mobile-sdk-demo/src/utils/documentStore.ts
  • packages/mobile-sdk-demo/tests/setup.ts
  • packages/mobile-sdk-demo/__tests__/cryptoPolyfills.test.ts
  • packages/mobile-sdk-demo/types/svg.d.ts
  • packages/mobile-sdk-demo/src/screens/RegisterDocument.tsx
  • packages/mobile-sdk-alpha/src/processing/mrz.ts
  • packages/mobile-sdk-demo/tests/cryptoPolyfill.test.ts
  • packages/mobile-sdk-demo/src/utils/secureStorage.ts
  • packages/mobile-sdk-demo/__tests__/secureStorage.test.ts
  • packages/mobile-sdk-alpha/src/index.ts
  • packages/mobile-sdk-alpha/src/mock/generator.ts
  • packages/mobile-sdk-demo/vitest.config.ts
  • packages/mobile-sdk-alpha/src/mrz/index.ts
  • common/src/polyfills/crypto.ts
  • packages/mobile-sdk-demo/src/screens/GenerateMock.tsx
packages/mobile-sdk-alpha/**/*.{ts,tsx,js,jsx}

⚙️ CodeRabbit configuration file

packages/mobile-sdk-alpha/**/*.{ts,tsx,js,jsx}: Review alpha mobile SDK code for:

  • API consistency with core SDK
  • Platform-neutral abstractions
  • Performance considerations
  • Clear experimental notes or TODOs

Files:

  • packages/mobile-sdk-alpha/src/stores/protocolStore.ts
  • packages/mobile-sdk-alpha/tests/processing/mrz.test.ts
  • packages/mobile-sdk-alpha/src/browser.ts
  • packages/mobile-sdk-alpha/src/processing/mrz.ts
  • packages/mobile-sdk-alpha/src/index.ts
  • packages/mobile-sdk-alpha/src/mock/generator.ts
  • packages/mobile-sdk-alpha/src/mrz/index.ts
packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}}

📄 CodeRabbit inference engine (packages/mobile-sdk-alpha/AGENTS.md)

packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}}: Do NOT mock @selfxyz/mobile-sdk-alpha in tests (avoid jest.mock('@selfxyz/mobile-sdk-alpha') and replacing real functions with mocks)
Use actual imports from @selfxyz/mobile-sdk-alpha in tests
Write integration tests that exercise the real validation logic (not mocks)
Test isPassportDataValid() with realistic synthetic passport data (never real user data)
Verify extractMRZInfo() using published sample MRZ strings (e.g., ICAO examples)
Ensure parseNFCResponse() works with representative, synthetic NFC data
Never use real user PII in tests; use only synthetic, anonymized, or approved test vectors

Files:

  • packages/mobile-sdk-alpha/tests/processing/mrz.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:

  • packages/mobile-sdk-alpha/tests/processing/mrz.test.ts
  • packages/mobile-sdk-demo/__tests__/documentStore.simple.test.ts
  • packages/mobile-sdk-demo/__tests__/cryptoPolyfills.test.ts
  • packages/mobile-sdk-demo/tests/cryptoPolyfill.test.ts
  • packages/mobile-sdk-demo/__tests__/secureStorage.test.ts
packages/mobile-sdk-alpha/src/processing/**

📄 CodeRabbit inference engine (.cursor/rules/mobile-sdk-migration.mdc)

Place MRZ processing helpers in packages/mobile-sdk-alpha/src/processing/

Files:

  • packages/mobile-sdk-alpha/src/processing/mrz.ts
packages/mobile-sdk-alpha/src/index.ts

📄 CodeRabbit inference engine (.cursor/rules/mobile-sdk-migration.mdc)

Re-export new SDK modules via packages/mobile-sdk-alpha/src/index.ts

Files:

  • packages/mobile-sdk-alpha/src/index.ts
common/src/**/*.{ts,tsx,js,jsx}

⚙️ CodeRabbit configuration file

common/src/**/*.{ts,tsx,js,jsx}: Review shared utilities for:

  • Reusability and modular design
  • Type safety and error handling
  • Side-effect management
  • Documentation and naming clarity

Files:

  • common/src/polyfills/crypto.ts
🧠 Learnings (23)
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Use actual imports from selfxyz/mobile-sdk-alpha in tests

Applied to files:

  • packages/mobile-sdk-alpha/src/stores/protocolStore.ts
  • packages/mobile-sdk-demo/tsconfig.json
  • packages/mobile-sdk-demo/tests/setup.ts
  • packages/mobile-sdk-demo/__tests__/cryptoPolyfills.test.ts
  • packages/mobile-sdk-demo/tests/cryptoPolyfill.test.ts
  • packages/mobile-sdk-demo/package.json
  • packages/mobile-sdk-demo/vitest.config.ts
📚 Learning: 2025-08-25T14:25:57.586Z
Learnt from: aaronmgdr
PR: selfxyz/self#951
File: app/src/providers/authProvider.web.tsx:17-18
Timestamp: 2025-08-25T14:25:57.586Z
Learning: The selfxyz/mobile-sdk-alpha/constants/analytics import path is properly configured with SDK exports, Metro aliases, and TypeScript resolution. Import changes from @/consts/analytics to this path are part of valid analytics migration, not TypeScript resolution issues.

Applied to files:

  • packages/mobile-sdk-alpha/src/stores/protocolStore.ts
  • packages/mobile-sdk-demo/tsconfig.json
  • packages/mobile-sdk-demo/vitest.config.ts
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Verify extractMRZInfo() using published sample MRZ strings (e.g., ICAO examples)

Applied to files:

  • packages/mobile-sdk-alpha/tests/processing/mrz.test.ts
  • packages/mobile-sdk-alpha/src/browser.ts
  • packages/mobile-sdk-alpha/src/processing/mrz.ts
  • packages/mobile-sdk-alpha/src/index.ts
  • packages/mobile-sdk-alpha/src/mrz/index.ts
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/src/processing/** : Place MRZ processing helpers in packages/mobile-sdk-alpha/src/processing/

Applied to files:

  • packages/mobile-sdk-alpha/tests/processing/mrz.test.ts
  • packages/mobile-sdk-alpha/src/browser.ts
  • packages/mobile-sdk-alpha/src/processing/mrz.ts
  • packages/mobile-sdk-alpha/src/index.ts
  • packages/mobile-sdk-alpha/src/mrz/index.ts
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Test isPassportDataValid() with realistic synthetic passport data (never real user data)

Applied to files:

  • packages/mobile-sdk-alpha/tests/processing/mrz.test.ts
  • packages/mobile-sdk-demo/__tests__/documentStore.simple.test.ts
  • packages/mobile-sdk-alpha/src/mock/generator.ts
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/src/index.ts : Re-export new SDK modules via packages/mobile-sdk-alpha/src/index.ts

Applied to files:

  • packages/mobile-sdk-alpha/src/browser.ts
  • packages/mobile-sdk-alpha/src/index.ts
  • packages/mobile-sdk-demo/vitest.config.ts
  • packages/mobile-sdk-alpha/src/mrz/index.ts
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.{ts,tsx} : Use strict TypeScript type checking across the codebase

Applied to files:

  • packages/mobile-sdk-demo/tsconfig.json
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/tests/setup.ts : Provide Vitest setup file at packages/mobile-sdk-alpha/tests/setup.ts to suppress console noise

Applied to files:

  • packages/mobile-sdk-demo/tsconfig.json
  • packages/mobile-sdk-demo/tests/setup.ts
  • packages/mobile-sdk-demo/__tests__/cryptoPolyfills.test.ts
  • packages/mobile-sdk-demo/tests/cryptoPolyfill.test.ts
  • packages/mobile-sdk-demo/vitest.config.ts
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.{ts,tsx} : Avoid introducing circular dependencies

Applied to files:

  • packages/mobile-sdk-demo/tsconfig.json
  • packages/mobile-sdk-demo/package.json
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.{ts,tsx} : Follow ESLint TypeScript-specific rules

Applied to files:

  • packages/mobile-sdk-demo/tsconfig.json
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/vitest.config.ts : Use Vitest in the SDK with a Node environment configured in packages/mobile-sdk-alpha/vitest.config.ts

Applied to files:

  • packages/mobile-sdk-demo/tsconfig.json
  • packages/mobile-sdk-demo/tests/setup.ts
  • packages/mobile-sdk-demo/tests/cryptoPolyfill.test.ts
  • packages/mobile-sdk-demo/vitest.config.ts
📚 Learning: 2025-08-26T14:49:11.190Z
Learnt from: shazarre
PR: selfxyz/self#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:

  • packages/mobile-sdk-demo/src/providers/SelfClientProvider.tsx
📚 Learning: 2025-08-26T14:49:11.190Z
Learnt from: shazarre
PR: selfxyz/self#936
File: app/src/screens/passport/PassportNFCScanScreen.tsx:28-31
Timestamp: 2025-08-26T14:49:11.190Z
Learning: The main App.tsx file is located at app/App.tsx (not in app/src), and it properly wraps the entire app with SelfClientProvider at the top of the provider hierarchy, enabling useSelfClient() hook usage throughout all navigation screens.

Applied to files:

  • packages/mobile-sdk-demo/src/providers/SelfClientProvider.tsx
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to app/jest.setup.js : Provide comprehensive Jest setup in app/jest.setup.js with required mocks

Applied to files:

  • packages/mobile-sdk-demo/tests/setup.ts
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Do NOT mock selfxyz/mobile-sdk-alpha in tests (avoid jest.mock('selfxyz/mobile-sdk-alpha') and replacing real functions with mocks)

Applied to files:

  • packages/mobile-sdk-demo/tests/setup.ts
  • packages/mobile-sdk-demo/__tests__/cryptoPolyfills.test.ts
  • packages/mobile-sdk-demo/tests/cryptoPolyfill.test.ts
  • packages/mobile-sdk-demo/package.json
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Never use real user PII in tests; use only synthetic, anonymized, or approved test vectors

Applied to files:

  • packages/mobile-sdk-demo/tests/setup.ts
  • packages/mobile-sdk-demo/__tests__/cryptoPolyfills.test.ts
  • packages/mobile-sdk-demo/__tests__/secureStorage.test.ts
📚 Learning: 2025-09-22T11:10:22.019Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Applies to jest.setup.js : Comprehensive mocks for all native modules must be set up in `jest.setup.js`.

Applied to files:

  • packages/mobile-sdk-demo/tests/setup.ts
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Write integration tests that exercise the real validation logic (not mocks)

Applied to files:

  • packages/mobile-sdk-demo/tests/setup.ts
  • packages/mobile-sdk-demo/tests/cryptoPolyfill.test.ts
  • packages/mobile-sdk-demo/__tests__/secureStorage.test.ts
📚 Learning: 2025-09-22T11:10:22.019Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Applies to tests/**/*.test.{js,ts,tsx,jsx} : Mock console.error to avoid test output clutter.

Applied to files:

  • packages/mobile-sdk-demo/tests/setup.ts
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Ensure parseNFCResponse() works with representative, synthetic NFC data

Applied to files:

  • packages/mobile-sdk-demo/tests/setup.ts
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/package.json : Expose a 'test:build' script in the SDK's package.json that runs build, test, types, and lint

Applied to files:

  • packages/mobile-sdk-demo/package.json
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/demo/** : Provide an in-SDK lightweight React Native demo under packages/mobile-sdk-alpha/demo/

Applied to files:

  • packages/mobile-sdk-demo/package.json
📚 Learning: 2025-09-22T11:10:22.019Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Test, build, and deploy scripts (`yarn test`, `yarn ios`, `yarn test:e2e:ios`, Fastlane, etc.) must be used for automation.

Applied to files:

  • packages/mobile-sdk-demo/package.json
🧬 Code graph analysis (8)
packages/mobile-sdk-alpha/tests/processing/mrz.test.ts (1)
packages/mobile-sdk-alpha/src/processing/mrz.ts (1)
  • extractNameFromMRZ (254-320)
packages/mobile-sdk-demo/src/providers/SelfClientProvider.tsx (2)
packages/mobile-sdk-demo/src/utils/documentStore.ts (1)
  • persistentDocumentsAdapter (24-87)
packages/mobile-sdk-demo/src/utils/secureStorage.ts (1)
  • getOrCreateSecret (179-184)
packages/mobile-sdk-demo/tests/setup.ts (1)
app/src/screens/prove/ProofRequestStatusScreen.tsx (1)
  • styles (323-339)
packages/mobile-sdk-demo/src/screens/RegisterDocument.tsx (4)
packages/mobile-sdk-alpha/src/proving/provingMachine.ts (1)
  • useProvingStore (369-1436)
packages/mobile-sdk-demo/src/components/SafeAreaScrollView.tsx (1)
  • SafeAreaScrollView (13-27)
packages/mobile-sdk-demo/src/components/StandardHeader.tsx (1)
  • StandardHeader (14-24)
packages/mobile-sdk-alpha/src/processing/mrz.ts (1)
  • extractNameFromMRZ (254-320)
packages/mobile-sdk-alpha/src/processing/mrz.ts (3)
packages/mobile-sdk-alpha/src/browser.ts (1)
  • extractNameFromMRZ (58-58)
packages/mobile-sdk-alpha/src/index.ts (1)
  • extractNameFromMRZ (90-90)
packages/mobile-sdk-alpha/src/mrz/index.ts (1)
  • extractNameFromMRZ (15-15)
packages/mobile-sdk-demo/__tests__/secureStorage.test.ts (1)
packages/mobile-sdk-demo/src/utils/secureStorage.ts (6)
  • generateSecret (47-54)
  • isValidSecret (235-238)
  • hasSecret (192-198)
  • getOrCreateSecret (179-184)
  • getSecretMetadata (207-213)
  • clearSecret (222-228)
packages/mobile-sdk-alpha/src/mock/generator.ts (1)
common/src/utils/types.ts (2)
  • PassportData (67-78)
  • AadhaarData (15-22)
packages/mobile-sdk-demo/src/screens/GenerateMock.tsx (3)
packages/mobile-sdk-alpha/src/mock/generator.ts (2)
  • signatureAlgorithmToStrictSignatureAlgorithm (122-157)
  • generateMockDocument (46-120)
packages/mobile-sdk-demo/src/components/SafeAreaScrollView.tsx (1)
  • SafeAreaScrollView (13-27)
packages/mobile-sdk-demo/src/components/StandardHeader.tsx (1)
  • StandardHeader (14-24)
⏰ 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). (5)
  • GitHub Check: build-deps
  • GitHub Check: e2e-ios
  • GitHub Check: android-build-test
  • GitHub Check: analyze-android
  • GitHub Check: analyze-ios
🔇 Additional comments (37)
common/src/polyfills/crypto.ts (1)

106-117: Buffer normalization logic is correct and consistent.

The conversion logic here matches the pattern in createHash.update (lines 50-57), ensuring Buffer subclasses are normalized to pure Uint8Array for @noble/hashes compatibility. The intermediate dataBytes variable improves clarity over the inline ternary in createHash.

Note: The refactor suggestion from the previous comment (extracting a normalizeToUint8Array helper) would also eliminate this duplication and improve maintainability.

packages/mobile-sdk-alpha/src/stores/protocolStore.ts (1)

7-7: LGTM: Type-only import follows best practices.

Using import type for the Environment type (and other type-only imports) is correct and helps with tree-shaking while avoiding potential circular dependencies.

packages/mobile-sdk-alpha/src/index.ts (1)

90-90: LGTM: Export consolidation improves maintainability.

The consolidated export statement for MRZ functions aligns with the guideline to re-export new SDK modules via this index file and improves clarity.

packages/mobile-sdk-alpha/tests/processing/mrz.test.ts (3)

8-8: LGTM: Correct import for new function.


13-15: LGTM: TD1 sample data is appropriate.


115-290: Excellent test coverage for extractNameFromMRZ.

The test suite comprehensively covers:

  • TD3 and TD1 formats with synthetic ICAO-style data
  • Edge cases (empty, null, whitespace, line endings)
  • Special characters (apostrophes, hyphens)
  • Single-line MRZ strings (88/90 chars)
  • Error handling

This aligns with coding guidelines to use ICAO examples and synthetic data.

Based on learnings

packages/mobile-sdk-alpha/src/browser.ts (1)

58-58: LGTM: Browser export updated consistently.

The addition of extractNameFromMRZ to the browser-safe exports maintains API consistency across entry points.

packages/mobile-sdk-alpha/src/mrz/index.ts (1)

15-15: LGTM: MRZ module export updated correctly.

Re-exporting extractNameFromMRZ from the processing module follows the established pattern and coding guidelines.

Based on learnings

packages/mobile-sdk-alpha/src/processing/mrz.ts (5)

241-253: LGTM: Well-documented function signature.

The JSDoc clearly explains the function's purpose, parameters, return type, and includes a helpful example.


254-277: LGTM: Robust input validation and normalization.

The function properly:

  • Guards against null/non-string input
  • Handles both Unix and Windows line endings
  • Normalizes whitespace
  • Supports single-line MRZ strings (88/90 chars for stored TD3 data)

279-300: LGTM: TD3 name parsing logic is sound.

The implementation correctly:

  • Matches passport format with country code
  • Splits on the << delimiter per ICAO 9303
  • Converts < padding to spaces
  • Handles single-name edge case (lastName only)

302-317: LGTM: TD1 name parsing mirrors TD3 logic.

Correctly uses line 3 for TD1 format and applies consistent name extraction logic.


319-320: LGTM: Safe fallback for unrecognized formats.

Returning null for unparseable input is appropriate and consistent with the function signature.

packages/mobile-sdk-demo/tsconfig.json (2)

5-6: LGTM! TypeScript config aligns with Vitest migration.

The addition of "vitest/globals" to types enables global test functions (describe, it, expect) without explicit imports, and "react-native" types provide proper RN runtime typings. The allowJs: true flag adds flexibility for the demo app to include JavaScript files alongside TypeScript.


8-8: LGTM! Test directory coverage expanded appropriately.

Including __tests__/**/* and tests/**/* ensures TypeScript properly checks all test files for the Vitest-based test suite introduced in this PR.

packages/mobile-sdk-demo/android/app/build.gradle (1)

47-48: LGTM! Standard react-native-vector-icons integration.

The fonts.gradle script inclusion is the documented approach for bundling icon fonts on Android. This ensures vector icon assets are available at runtime.

packages/mobile-sdk-demo/__tests__/cryptoPolyfills.test.ts (3)

5-5: LGTM! Vitest imports added for test framework migration.

The explicit Vitest imports align with the PR's migration to Vitest and the updated tsconfig.json that includes vitest/globals types.


183-192: Track the skipped ethers.sha256 integration test.

The ethers.sha256 registration test is skipped without an explanation or tracking ticket. While the direct sha256 utility tests pass, skipping the ethers integration test suggests the polyfill registration may be incomplete or problematic.

Please clarify:

  1. Is this skip intentional technical debt or a known issue?
  2. Should this be tracked in an issue for follow-up?

If the registration is broken, consider adding a TODO comment with context or a tracking issue reference.


194-203: Track the skipped ethers.sha512 integration test.

Similar to sha256, the ethers.sha512 registration test is skipped. The direct sha512 utility tests pass, but the ethers integration is not verified.

Ensure both skipped ethers integration tests are tracked together. If the ethers registration is incomplete, document the reason and create a follow-up task.

packages/mobile-sdk-demo/src/utils/ethers.ts (1)

42-46: LGTM! Defensive Uint8Array coercion for sha512.

Consistent with the sha256 implementation, this ensures type purity for downstream consumers expecting plain Uint8Array instances.

packages/mobile-sdk-demo/types/svg.d.ts (1)

1-6: LGTM! SVG ambient module declaration is correct.

The type declaration properly imports FC from React (addressing the previous review concern) and SvgProps from react-native-svg. This enables TypeScript support for SVG imports as React components, aligning with the Metro transformer configuration added in this PR.

packages/mobile-sdk-demo/src/utils/documentStore.ts (3)

33-34: Error logging may expose document IDs or catalog structure.

While document IDs are hashes and not directly PII, consider whether logging them alongside error details could aid in correlation attacks or expose system internals. For production code, ensure error messages are sanitized and don't leak sensitive metadata.

For a demo app, this is acceptable, but be mindful when adapting this code for production use.

As per coding guidelines regarding PII/sensitive data logging.

Also applies to: 41-42, 67-68, 75-76, 83-84, 100-101


89-103: resetDocumentStore function works correctly.

The function properly loads the catalog, deletes all documents, then clears the catalog itself. Error handling is appropriate.


109-142: In-memory adapter implementation is consistent with persistent adapter.

The in-memory implementation mirrors the persistent adapter's structure and includes the same BigInt re-parsing logic. Good for testing and backwards compatibility.

packages/mobile-sdk-demo/__tests__/documentStore.simple.test.ts (2)

18-53: Excellent documentation of BigInt serialization behavior.

This test clearly demonstrates that number arrays survive JSON serialization and remain compatible with BigInt conversion. The test structure is clear and the assertions are appropriate.

Based on learnings - uses synthetic data appropriately.


104-147: Clear explanation of the root cause and solution.

This test effectively documents that the real issue is not with the number arrays (eContent, signedAttr, encryptedDigest) but with the complex parsed objects (dsc_parsed, passportMetadata, csca_parsed) that contain BigInt values and get corrupted during JSON serialization. The solution of re-parsing after loading is well-explained.

packages/mobile-sdk-alpha/src/mock/generator.ts (1)

82-88: Name handling logic is correct and follows OFAC branching properly.

The code correctly applies provided firstName/lastName values when available, falling back to defaults for OFAC-listed entries and using provided values otherwise. The logic is consistent across Aadhaar and non-Aadhaar paths.

Also applies to: 101-107

packages/mobile-sdk-demo/src/providers/SelfClientProvider.tsx (4)

21-30: Fetch polyfill error handling is appropriate.

The createFetch function properly checks for fetch availability and throws a clear error if it's not available. This is suitable for a demo environment.


32-44: WebSocket adapter stub is appropriate for demo.

The WebSocket adapter returns a connection object with lifecycle methods as no-ops and throws on send. This is appropriate for a demo environment where WebSocket functionality isn't needed. The error message clearly indicates the limitation.


46-46: Hash adapter implementation is correct.

The hash function properly wraps noble's sha256 and returns a Uint8Array as expected by the crypto adapter interface. The async wrapper in the adapter is appropriate for the interface contract.

Also applies to: 62-64


74-82: Error logging in getPrivateKey is safe as implemented
The caught errors originate from JSON parsing, localStorage, or Keychain calls and do not include the secret value or sensitive paths, so logging them here poses no leak risk. In this demo context, console.error is acceptable—just remove or sanitize these logs in a production build.

packages/mobile-sdk-demo/src/screens/GenerateMock.tsx (6)

42-43: Faker usage generates uppercase names correctly.

The helper functions properly generate random uppercase names using faker, which is appropriate for document generation (passport names are typically uppercase). Good use of faker for realistic mock data.


74-79: Input validation is thorough and appropriate.

The validation checks for:

  • Finite numbers
  • Age range: 0-120
  • Expiry range: 0-30

These ranges are sensible for document generation. Error messages are clear and user-friendly.


80-91: Name trimming and fallback logic is sound.

The code properly trims whitespace from firstName/lastName inputs and falls back to random names if empty. This prevents empty strings from being passed to generateMockDocument while still allowing user customization.


92-115: Document generation and catalog update flow is correct.

The sequence is well-structured:

  1. Generate document
  2. Calculate content hash for ID
  3. Load existing catalog
  4. Save document
  5. Create/update metadata
  6. Update selectedDocumentId
  7. Save catalog
  8. Trigger callback
  9. Navigate to register screen

Good separation of concerns and proper async/await usage.


164-205: Conditional rendering of algorithm and country pickers is appropriate.

The code correctly hides algorithm and country pickers for Aadhaar documents (where they don't apply). Platform-specific icon rendering for iOS enhances UX consistency.


236-237: Loading and error states are properly displayed.

The component shows an activity indicator during generation and displays error messages clearly. Good UX for async operations.

"react-native-screens": "4.15.3",
"react-native-sqlite-storage": "^6.0.1",
"react-native-svg": "^15.12.1",
"react-native-svg": "15.12.1",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lock to this specific version so the build doesn't break

skipCA: skipCABool,
skipPACE: skipPACEBool,
useExtendedMode: extendedModeBool,
usePacePolling: usePacePollingBool,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@seshanthS fyi the demo wouldn't build with this param

@transphorm transphorm changed the title chore: add polish to mobile demo app chore: add polish to the mobile demo app Oct 1, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

🧹 Nitpick comments (6)
packages/mobile-sdk-demo/tests/cryptoPolyfill.test.ts (6)

40-40: Consider typing crypto more specifically.

The any type loses type safety. If the polyfill exports a known interface, consider typing it explicitly or using typeof import('../src/polyfills/cryptoPolyfill.js') for better autocomplete and error detection.

- let crypto: any;
+ let crypto: typeof import('../src/polyfills/cryptoPolyfill.js') | undefined;

62-102: Consider adding edge case tests for method chaining.

The current tests cover the happy path well, but additional edge cases would improve coverage:

  • Empty string updates: update('').update('').digest()
  • No updates: createHash('sha256').digest() immediately
  • Very long chains (stress test)
  • Mixed data types in a chain (string → Buffer → Uint8Array)

Example test structure:

it('should handle empty string updates in chain', async () => {
  crypto = await import('../src/polyfills/cryptoPolyfill.js');
  const result = crypto.createHash('sha256')
    .update('')
    .update('data')
    .digest('hex');
  expect(result).toBe(crypto.createHash('sha256').update('data').digest('hex'));
});

126-144: Consider more specific error matching and additional edge cases.

The error test at line 141 uses a broad regex. Consider:

  1. Making the regex more specific to match the exact error message format
  2. Adding tests for partial crypto objects (crypto exists but no getRandomValues)
  3. Testing getRandomValues that throws during execution

Example additional test:

it('should handle partial crypto object gracefully', async () => {
  vi.resetModules();
  global.crypto = {} as any; // crypto exists but incomplete
  
  await expect(async () => {
    crypto = await import('../src/polyfills/cryptoPolyfill.js');
    crypto.randomBytes(16);
  }).rejects.toThrow(/getRandomValues is not available/);
});

148-166: Test name suggests unavailability scenario but always has Buffer.

The test name "should gracefully handle Buffer availability check" and comment (lines 149-151) indicate testing without Buffer, but the test always runs with Buffer available. Consider either:

  1. Renaming to "should work correctly when Buffer is available"
  2. Or adding a separate test that actually simulates Buffer unavailability (if feasible)

The current test is valuable for verifying typed array output, but the name is misleading.

- it('should gracefully handle Buffer availability check', async () => {
-   // This test verifies that the crypto polyfill checks for Buffer availability
-   // Note: We can't actually delete Buffer because Vitest's worker threads need it
-   // Instead, we verify that the polyfill works correctly with and without Buffer
+ it('should return valid Buffer when Buffer is available', async () => {
+   // This test verifies that the crypto polyfill works correctly with Buffer available
+   // Note: We can't test without Buffer because Vitest's worker threads require it

168-208: Consider extracting common setup pattern.

Lines 170-172 and 187-189 repeat the same setup sequence (vi.unmock('buffer'), vi.resetModules(), vi.restoreAllMocks()). This could be extracted into a helper function for better maintainability.

Example refactor:

function resetBufferEnvironment() {
  vi.unmock('buffer');
  vi.resetModules();
  vi.restoreAllMocks();
  global.Buffer = require('buffer').Buffer;
}

it('should work with Buffer polyfill imported', async () => {
  resetBufferEnvironment();
  crypto = await import('../src/polyfills/cryptoPolyfill.js');
  // ... rest of test
});

211-242: Consider expanding integration test scenarios.

The single integration test covers the happy path well, but additional scenarios would strengthen confidence:

  • Error handling integration (invalid algorithm + missing crypto)
  • Multiple hash algorithms in sequence
  • Large data processing (stress test)
  • Real-world usage patterns (e.g., signing flow)

Example additional test:

it('should handle multiple algorithms in integration', async () => {
  // ... setup ...
  crypto = await import('../src/polyfills/cryptoPolyfill.js');
  
  const sha256 = crypto.createHash('sha256').update('data').digest('hex');
  const sha512 = crypto.createHash('sha512').update('data').digest('hex');
  const random = crypto.randomBytes(32);
  
  expect(sha256.length).toBe(64);
  expect(sha512.length).toBe(128);
  expect(random.length).toBe(32);
});
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3ce39ff and c1a270a.

📒 Files selected for processing (6)
  • packages/mobile-sdk-demo/__tests__/cryptoPolyfills.test.ts (1 hunks)
  • packages/mobile-sdk-demo/__tests__/secureStorage.test.ts (1 hunks)
  • packages/mobile-sdk-demo/src/utils/secureStorage.ts (1 hunks)
  • packages/mobile-sdk-demo/tests/cryptoPolyfill.test.ts (5 hunks)
  • packages/mobile-sdk-demo/tests/setup.ts (1 hunks)
  • packages/mobile-sdk-demo/vitest.config.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/mobile-sdk-demo/vitest.config.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,ts,tsx,jsx,sol,nr}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{js,ts,tsx,jsx,sol,nr}: 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., ***-***-1234 for passport numbers, J*** D*** for names).

Files:

  • packages/mobile-sdk-demo/tests/cryptoPolyfill.test.ts
  • packages/mobile-sdk-demo/tests/setup.ts
  • packages/mobile-sdk-demo/__tests__/cryptoPolyfills.test.ts
  • packages/mobile-sdk-demo/__tests__/secureStorage.test.ts
  • packages/mobile-sdk-demo/src/utils/secureStorage.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:

  • packages/mobile-sdk-demo/tests/cryptoPolyfill.test.ts
  • packages/mobile-sdk-demo/__tests__/cryptoPolyfills.test.ts
  • packages/mobile-sdk-demo/__tests__/secureStorage.test.ts
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/demo/** : Provide an in-SDK lightweight React Native demo under packages/mobile-sdk-alpha/demo/
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/tests/setup.ts : Provide Vitest setup file at packages/mobile-sdk-alpha/tests/setup.ts to suppress console noise

Applied to files:

  • packages/mobile-sdk-demo/tests/cryptoPolyfill.test.ts
  • packages/mobile-sdk-demo/tests/setup.ts
  • packages/mobile-sdk-demo/__tests__/cryptoPolyfills.test.ts
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Use actual imports from selfxyz/mobile-sdk-alpha in tests

Applied to files:

  • packages/mobile-sdk-demo/tests/cryptoPolyfill.test.ts
  • packages/mobile-sdk-demo/tests/setup.ts
  • packages/mobile-sdk-demo/__tests__/cryptoPolyfills.test.ts
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Do NOT mock selfxyz/mobile-sdk-alpha in tests (avoid jest.mock('selfxyz/mobile-sdk-alpha') and replacing real functions with mocks)

Applied to files:

  • packages/mobile-sdk-demo/tests/cryptoPolyfill.test.ts
  • packages/mobile-sdk-demo/tests/setup.ts
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Write integration tests that exercise the real validation logic (not mocks)

Applied to files:

  • packages/mobile-sdk-demo/tests/cryptoPolyfill.test.ts
  • packages/mobile-sdk-demo/tests/setup.ts
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Never use real user PII in tests; use only synthetic, anonymized, or approved test vectors

Applied to files:

  • packages/mobile-sdk-demo/tests/cryptoPolyfill.test.ts
  • packages/mobile-sdk-demo/tests/setup.ts
  • packages/mobile-sdk-demo/__tests__/cryptoPolyfills.test.ts
  • packages/mobile-sdk-demo/__tests__/secureStorage.test.ts
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Ensure parseNFCResponse() works with representative, synthetic NFC data

Applied to files:

  • packages/mobile-sdk-demo/tests/cryptoPolyfill.test.ts
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to app/jest.setup.js : Provide comprehensive Jest setup in app/jest.setup.js with required mocks

Applied to files:

  • packages/mobile-sdk-demo/tests/setup.ts
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/vitest.config.ts : Use Vitest in the SDK with a Node environment configured in packages/mobile-sdk-alpha/vitest.config.ts

Applied to files:

  • packages/mobile-sdk-demo/tests/setup.ts
📚 Learning: 2025-09-22T11:10:22.019Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Applies to jest.setup.js : Comprehensive mocks for all native modules must be set up in `jest.setup.js`.

Applied to files:

  • packages/mobile-sdk-demo/tests/setup.ts
📚 Learning: 2025-09-22T11:10:22.019Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Applies to tests/**/*.test.{js,ts,tsx,jsx} : Mock console.error to avoid test output clutter.

Applied to files:

  • packages/mobile-sdk-demo/tests/setup.ts
📚 Learning: 2025-09-22T11:10:22.019Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Always check `Platform.OS` before running any platform-specific code; use different implementations and tests as required.

Applied to files:

  • packages/mobile-sdk-demo/tests/setup.ts
📚 Learning: 2025-09-22T11:10:22.019Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Applies to src/**/*.{js,ts,tsx,jsx} : Always handle platform differentiation with `Platform.OS` checks before platform-specific logic.

Applied to files:

  • packages/mobile-sdk-demo/tests/setup.ts
🧬 Code graph analysis (2)
packages/mobile-sdk-demo/tests/setup.ts (1)
app/src/screens/prove/ProofRequestStatusScreen.tsx (1)
  • styles (323-339)
packages/mobile-sdk-demo/__tests__/secureStorage.test.ts (1)
packages/mobile-sdk-demo/src/utils/secureStorage.ts (6)
  • clearSecret (223-229)
  • generateSecret (48-55)
  • isValidSecret (236-239)
  • hasSecret (193-199)
  • getOrCreateSecret (180-185)
  • getSecretMetadata (208-214)
⏰ 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). (5)
  • GitHub Check: build-deps
  • GitHub Check: android-build-test
  • GitHub Check: e2e-ios
  • GitHub Check: analyze-ios
  • GitHub Check: analyze-android
🔇 Additional comments (11)
packages/mobile-sdk-demo/tests/cryptoPolyfill.test.ts (2)

12-34: LGTM: Clean Vitest setup with proper global crypto mocking.

The imports, preservation of original crypto, and conditional mock setup follow best practices. The deterministic mock values (i % 256) ensure reproducible tests, and the Object.defineProperty approach enables proper cleanup.


42-60: LGTM: Thorough setup and teardown.

The beforeEach/afterEach hooks properly isolate tests by resetting modules and restoring global state. The Object.defineProperty approach for crypto restoration handles read-only properties correctly.

packages/mobile-sdk-demo/__tests__/secureStorage.test.ts (9)

33-43: LGTM! Excellent test isolation.

Proper cleanup in both beforeEach and afterEach ensures tests run independently without side effects.


45-80: Good coverage for generateSecret.

Tests validate format, byte length, and uniqueness. The mock implementation swap in the uniqueness test is properly managed by vi.clearAllMocks() in beforeEach.


82-109: LGTM! Comprehensive validation coverage.

Tests cover all critical validation paths including edge cases (length boundaries, character sets, empty input). The gitleaks:allow annotation is appropriate for test data.


111-128: LGTM! Core functionality tested.

Tests verify both creation and idempotency of secret retrieval.


130-147: LGTM! Complete lifecycle coverage.

Tests validate hasSecret across all states: empty, populated, and after clear.


154-160: Verify platform-specific testing approach.

The comment states this tests native platform behavior, but Platform.OS isn't mocked in this test suite. In the current test environment (likely web-based given the localStorage/crypto mocks), this test may not accurately validate native vs. web behavior.

Consider either:

  1. Mocking Platform.OS to explicitly test native behavior
  2. Updating the comment to reflect what's actually being tested (current environment behavior)
  3. Adding separate describe blocks for web vs. native platform behaviors

Example mock approach:

it('should return null on native platforms', async () => {
  vi.mock('react-native/Libraries/Utilities/Platform', () => ({
    OS: 'ios',
  }));
  
  await getOrCreateSecret();
  const metadata = await getSecretMetadata();
  expect(metadata).toBeNull();
});

163-175: LGTM! Robust deletion tests.

Tests cover both successful deletion and idempotent behavior when secret doesn't exist, ensuring the function is safe to call in any state.


177-185: LGTM! Critical security verification.

Explicitly testing the 256-bit (32-byte) secret length is essential for cryptographic strength. The type check for Uint8Array is also valuable.


187-222: LGTM! Excellent integration tests.

These tests verify the complete lifecycle and interaction between functions, ensuring they work together correctly. The mock modification in the lifecycle test to ensure a different secret is generated is well-reasoned.

Comment on lines +38 to +41
Platform: {
OS: 'ios',
select: (obj: Record<string, any>) => (Object.prototype.hasOwnProperty.call(obj, 'ios') ? obj.ios : obj.default),
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Fix Platform.select so tests hit the right platform branches.

The stub always returns the ios entry (or default) regardless of the current Platform.OS, so any test that flips Platform.OS to 'android' (or anything else) silently takes the wrong code path. That hides mobile regressions—the opposite of what this setup is meant to catch. Mirror React Native’s real behavior by reading the active OS (and falling back to native/default) before returning.

-  Platform: {
-    OS: 'ios',
-    select: (obj: Record<string, any>) => (Object.prototype.hasOwnProperty.call(obj, 'ios') ? obj.ios : obj.default),
-  },
+  Platform: (() => {
+    const platformMock: any = { OS: 'ios' };
+    platformMock.select = (obj: Record<string, any>) => {
+      const currentOS = platformMock.OS;
+      if (Object.prototype.hasOwnProperty.call(obj, currentOS)) {
+        return obj[currentOS];
+      }
+      if ((currentOS === 'ios' || currentOS === 'android') && Object.prototype.hasOwnProperty.call(obj, 'native')) {
+        return obj.native;
+      }
+      return obj.default;
+    };
+    return platformMock;
+  })(),
📝 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
Platform: {
OS: 'ios',
select: (obj: Record<string, any>) => (Object.prototype.hasOwnProperty.call(obj, 'ios') ? obj.ios : obj.default),
},
Platform: (() => {
const platformMock: any = { OS: 'ios' };
platformMock.select = (obj: Record<string, any>) => {
const currentOS = platformMock.OS;
if (Object.prototype.hasOwnProperty.call(obj, currentOS)) {
return obj[currentOS];
}
if (
(currentOS === 'ios' || currentOS === 'android') &&
Object.prototype.hasOwnProperty.call(obj, 'native')
) {
return obj.native;
}
return obj.default;
};
return platformMock;
})(),
🤖 Prompt for AI Agents
In packages/mobile-sdk-demo/tests/setup.ts around lines 38 to 41, the
Platform.select stub currently always returns the 'ios' entry (or default)
instead of honoring the runtime Platform.OS; change the select implementation to
read the active Platform.OS value, then: 1) if the provided object has a
property matching that OS, return that entry; 2) else if it has 'native' return
that; 3) otherwise return 'default'; ensure the function uses
Object.prototype.hasOwnProperty.call for the checks and preserves the existing
types/signature.

@transphorm transphorm merged commit c55112d into dev Oct 1, 2025
37 checks passed
@transphorm transphorm deleted the codex/review-mobile-sdk-demo-app-for-polish-suggestions branch October 1, 2025 07:16
@coderabbitai coderabbitai bot mentioned this pull request Oct 1, 2025
@transphorm transphorm changed the title chore: add polish to the mobile demo app [SELF-781] chore: add polish to the mobile demo app Oct 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants