Hotfix/android camera msdk #1037
Conversation
WalkthroughAdds cross-platform QR scanning and hardens MRZ scanning: new Android/iOS QR modules and view managers, a ZXing-based detector and fragment, MRZ module UI lifecycle and cleanup fixes, JS components/adapters and exports, Gradle dependency/packaging updates, and removal of several Android styles. Changes
Sequence Diagram(s)sequenceDiagram
participant JS as JavaScript
participant RN as React Native Bridge
participant Module as Native Module (SelfQRScanner / SelfMRZScanner)
participant Activity as FragmentActivity / UIViewController
participant UI as Overlay (FrameLayout / UIView)
participant Fragment as QrCodeScannerFragment / CameraMLKitFragment
JS->>RN: NativeModules.startScanning()
RN->>Module: startScanning(promise)
Module->>Activity: locate FragmentActivity / root VC
alt no activity
Module-->>RN: reject(E_NO_ACTIVITY)
else activity present
Module->>Activity: mainThread { generateUnusedId -> create UI overlay -> attach to decor/root view }
Module->>Activity: FragmentManager/VC commitNow or commitAllowingStateLoss { replace(UI.id, Fragment) }
Fragment->>Module: onQRData / onPassportRead
Module->>RN: resolve(promise, data)
Module->>Activity: cleanup() removes fragment and UI
end
alt error during setup or scanning
Module-->>RN: reject(E_SCANNING_ERROR / E_QR_SCAN_ERROR)
Module->>Activity: cleanup()
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@seshanthS merged with |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/ui/CameraFragment.kt (1)
149-156: Configure zoom after runtime permission grant to avoid missing pinch-to-zoomWhen permission is granted via the dialog, you start the camera in onRequestPermissionsResult but never call configureZoom(), so zoom remains uninitialized in that flow.
Apply in onRequestPermissionsResult after starting the camera:
Manifest.permission.CAMERA -> { hasCameraPermission = true fotoapparat?.start() + configureZoom() }packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfOCRViewManager.kt (2)
151-159: Choreographer callback leaks and continuous relayout can degrade performancePosting a self-rescheduling FrameCallback every frame without removal causes perpetual work and a leak after the view is dropped.
Add a tracked callback and remove it in onDropViewInstance:
class SelfOCRViewManager( open val reactContext: ReactApplicationContext ) : ViewGroupManager<FrameLayout>(), CameraMLKitFragment.CameraMLKitCallback { + private var frameCallback: Choreographer.FrameCallback? = null private fun setupLayout(view: View) { - Choreographer.getInstance().postFrameCallback(object: Choreographer.FrameCallback { - override fun doFrame(frameTimeNanos: Long) { - manuallyLayoutChildren(view) - view.viewTreeObserver.dispatchOnGlobalLayout() - Choreographer.getInstance().postFrameCallback(this) - } - }) + frameCallback = object : Choreographer.FrameCallback { + override fun doFrame(frameTimeNanos: Long) { + manuallyLayoutChildren(view) + view.viewTreeObserver.dispatchOnGlobalLayout() + Choreographer.getInstance().postFrameCallback(this) + } + } + Choreographer.getInstance().postFrameCallback(frameCallback!!) } + + override fun onDropViewInstance(view: FrameLayout) { + frameCallback?.let { Choreographer.getInstance().removeFrameCallback(it) } + frameCallback = null + super.onDropViewInstance(view) + }
137-149: Null-safe destroy path to avoid ClassCastException/IllegalStateExceptiondestroyFragment assumes a non-null currentActivity and FragmentActivity cast. Mirror the create path’s null/state checks to avoid crashes during teardown.
- val activity = reactContext.currentActivity as FragmentActivity + val activity = reactContext.currentActivity as? FragmentActivity + if (activity == null || activity.isFinishing || activity.isDestroyed) { + android.util.Log.w("SelfOCRViewManager", "Activity unavailable during destroy") + return + }packages/mobile-sdk-alpha/src/components/MRZScannerView.tsx (2)
125-136: Pixel units vs. dp: oversized container risk on high‑DPI devicesPixelRatio.getPixelSizeForLayoutSize(800) returns physical pixels, but RN style expects dp. This can inflate the container massively (e.g., 2400px on 3x). Prefer dp (plain 800) or compute from Dimensions/PX↔DP conversion consistently with native fallbacks.
- style={{ - height: PixelRatio.getPixelSizeForLayoutSize(800), - width: PixelRatio.getPixelSizeForLayoutSize(800), - }} + style={{ + height: 800, + width: 800, + }}
64-91: Eliminate any in event handlers to comply with strict TS across the SDKThe alpha SDK mandates strict typing. Type the event payloads to match platform shapes and remove any.
- const handleMRZDetected = useCallback( - (event: any) => { + type IOSPassportEvent = NativeSyntheticEvent<{ + data: { documentNumber: string; expiryDate: string; birthDate: string; documentType: string; countryCode: string }; + }>; + type AndroidPassportEvent = NativeSyntheticEvent<{ data: string }>; + + const handleMRZDetected = useCallback( + (event: IOSPassportEvent | AndroidPassportEvent) => { const data = event.nativeEvent.data;- const handleError = useCallback( - (event: any) => { + const handleError = useCallback( + (event: NativeSyntheticEvent<{ error: string; errorMessage: string; stackTrace: string }>) => { const { error } = event.nativeEvent;packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfMRZScannerModule.kt (1)
20-23: Add RN lifecycle hooks to SelfMRZScannerModule to prevent leaks on reload/navigation.Verified: CameraMLKitFragment implements camera permission request + denial flow (requestCameraPermission, ConfirmationDialog -> parent.requestPermissions, onRequestPermissionsResult shows denial dialog).
Action (mandatory): Update packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfMRZScannerModule.kt to implement LifecycleEventListener — addLifecycleEventListener in init; override onHostDestroy to call cleanup() and reject/clear scanPromise (e.g. reject with "E_ACTIVITY_DESTROYED"); override onCatalystInstanceDestroy to removeLifecycleEventListener, cleanup(), and clear scanPromise; onHostPause/onHostResume can be no-ops.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfMRZScannerModule.kt(1 hunks)packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfOCRViewManager.kt(4 hunks)packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/ui/CameraFragment.kt(3 hunks)packages/mobile-sdk-alpha/android/src/main/res/values/styles.xml(0 hunks)packages/mobile-sdk-alpha/src/components/MRZScannerView.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- packages/mobile-sdk-alpha/android/src/main/res/values/styles.xml
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/technical-specification.mdc)
**/*.{ts,tsx}: Define IdentityCommitment with fields: commitment (Poseidon hash), nullifier (domain-separated), timestamp (UTC number), version (circuit version), documentType ('passport' | 'eu_id_card')
Define DSCKeyCommitment with fields: publicKeyHash (Poseidon hash), certificateChain (hashes), revocationStatus (boolean), issuer (country code)
Define VerificationConfig with fields: circuitVersion (semver), complianceRules array, timeWindow (seconds, 24h), clockDrift (±5 min), trustAnchors, revocationRoots, timeSource (NTP), nullifierScope (domain separation)
Files:
packages/mobile-sdk-alpha/src/components/MRZScannerView.tsx
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/components/MRZScannerView.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/components/MRZScannerView.tsx
🧠 Learnings (8)
📚 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/android/src/main/java/com/selfxyz/selfSDK/SelfMRZScannerModule.ktpackages/mobile-sdk-alpha/src/components/MRZScannerView.tsx
📚 Learning: 2025-08-24T18:52:25.796Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-08-24T18:52:25.796Z
Learning: Applies to native/android/**/*.{kt,kts} : Android NFC: implement RNPassportReaderModule in Kotlin
Applied to files:
packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfMRZScannerModule.kt
📚 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/src/components/MRZScannerView.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/src/index.ts : Re-export new SDK modules via packages/mobile-sdk-alpha/src/index.ts
Applied to files:
packages/mobile-sdk-alpha/src/components/MRZScannerView.tsx
📚 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-alpha/src/components/MRZScannerView.tsx
📚 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/components/MRZScannerView.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-alpha/src/components/MRZScannerView.tsx
📚 Learning: 2025-07-12T22:00:02.041Z
Learnt from: transphorm
PR: selfxyz/self#636
File: app/src/utils/cameraPermission.ts:19-21
Timestamp: 2025-07-12T22:00:02.041Z
Learning: The Android permission handling in app/src/utils/cameraPermission.ts is robust and properly implemented using PermissionsAndroid.request() and PermissionsAndroid.check(). Previous issues with Android permission assumptions have been resolved in the current implementation.
Applied to files:
packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/ui/CameraFragment.kt
🔇 Additional comments (2)
packages/mobile-sdk-alpha/src/components/MRZScannerView.tsx (1)
128-129: Android mapping string matches native class name — goodfragmentComponentName now aligns with SelfOCRViewManager.RECT_CLASS. This unblocks command routing and event bubbling.
packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfMRZScannerModule.kt (1)
38-69: Harden against Activity state issues; avoid commitNow crashes/jank.File: packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfMRZScannerModule.kt (lines 38–69)
activity.runOnUiThread { try { + if (activity.isFinishing || activity.isDestroyed) { + scanPromise = null + promise.reject("E_NO_ACTIVITY", "Activity is finishing/destroyed") + return@runOnUiThread + } + val fm = activity.supportFragmentManager + if (fm.isStateSaved) { + scanPromise = null + promise.reject("E_STATE_SAVED", "Cannot start scanning after onSaveInstanceState") + return@runOnUiThread + } val container = FrameLayout(activity) // just using view.generateViewId() doesn't work. val containerId = generateUnusedId(activity.window.decorView as ViewGroup) container.id = containerId @@ - activity.supportFragmentManager + fm .beginTransaction() .replace(containerId, fragment) - .commitNow() + .commitNowAllowingStateLoss()Verified: packages/mobile-sdk-alpha/android/build.gradle shows minSdkVersion = 23. Grep did not find an explicit "androidx.fragment" dependency in Gradle files — confirm the project’s Fragment artifact supplies commitNowAllowingStateLoss or add a fallback (e.g., commitAllowingStateLoss) if unavailable.
packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfMRZScannerModule.kt
Outdated
Show resolved
Hide resolved
packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfMRZScannerModule.kt
Show resolved
Hide resolved
| override fun onPassportRead(mrzInfo: MRZInfo) { | ||
| scanPromise?.resolve(mrzInfo.toString()) | ||
| scanPromise = null | ||
| cleanup() | ||
| } |
There was a problem hiding this comment.
Don’t return raw MRZ string; minimize/structure PII.
MRZInfo.toString() can expose full passport details to JS/logs. Return a minimal, structured payload with masking.
- scanPromise?.resolve(mrzInfo.toString())
+ val payload = com.facebook.react.bridge.Arguments.createMap().apply {
+ putString("documentType", mrzInfo.documentCode)
+ putString("issuingState", mrzInfo.issuingState)
+ putString("nationality", mrzInfo.nationality)
+ putString("documentNumberMasked", mrzInfo.documentNumber?.let { maskDoc(it) })
+ // Add more fields only if strictly required; avoid full MRZ dump.
+ }
+ scanPromise?.resolve(payload)Add alongside the class (helper):
private fun maskDoc(v: String): String =
if (v.length <= 3) "***" else "*".repeat(v.length - 3) + v.takeLast(3)If JS currently expects a string, consider a temporary dual-path (behind a debug flag) or bumping the API contract with a clear migration note.
packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfMRZScannerModule.kt
Show resolved
Hide resolved
packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfOCRViewManager.kt
Show resolved
Hide resolved
packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfOCRViewManager.kt
Show resolved
Hide resolved
| val activity = reactContext.currentActivity as? FragmentActivity | ||
| if (activity == null) { | ||
| android.util.Log.e("SelfOCRViewManager", "No FragmentActivity found") | ||
| return | ||
| } | ||
|
|
||
| // Check if activity is in a valid state | ||
| if (activity.isFinishing || activity.isDestroyed) { | ||
| android.util.Log.e("SelfOCRViewManager", "Activity is finishing or destroyed") | ||
| return | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Null-check container before layout; bail early if missing
root.findViewById may return null; setupLayout(parentView) will then NPE and the subsequent fragment transaction will still run, risking IllegalArgumentException on replace().
- val parentView = root.findViewById<ViewGroup>(reactNativeViewId)
- setupLayout(parentView)
+ val parentView = root.findViewById<ViewGroup?>(reactNativeViewId)
+ if (parentView == null) {
+ android.util.Log.e("SelfOCRViewManager", "Container view $reactNativeViewId not found in root")
+ return
+ }
+ setupLayout(parentView)📝 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.
| val activity = reactContext.currentActivity as? FragmentActivity | |
| if (activity == null) { | |
| android.util.Log.e("SelfOCRViewManager", "No FragmentActivity found") | |
| return | |
| } | |
| // Check if activity is in a valid state | |
| if (activity.isFinishing || activity.isDestroyed) { | |
| android.util.Log.e("SelfOCRViewManager", "Activity is finishing or destroyed") | |
| return | |
| } | |
| val activity = reactContext.currentActivity as? FragmentActivity | |
| if (activity == null) { | |
| android.util.Log.e("SelfOCRViewManager", "No FragmentActivity found") | |
| return | |
| } | |
| // Check if activity is in a valid state | |
| if (activity.isFinishing || activity.isDestroyed) { | |
| android.util.Log.e("SelfOCRViewManager", "Activity is finishing or destroyed") | |
| return | |
| } | |
| val parentView = root.findViewById<ViewGroup?>(reactNativeViewId) | |
| if (parentView == null) { | |
| android.util.Log.e("SelfOCRViewManager", "Container view $reactNativeViewId not found in root") | |
| return | |
| } | |
| setupLayout(parentView) |
🤖 Prompt for AI Agents
In
packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfOCRViewManager.kt
around lines 105 to 116, root.findViewById can return null so
setupLayout(parentView) may NPE and the fragment transaction could still run;
check the parentView returned by findViewById and if it's null log an error and
return early before calling setupLayout or performing any fragment transaction
so you don't attempt to replace a fragment into a null container.
| val width = propWidth ?: 800 // Default fallback | ||
| val height = propHeight ?: 800 // Default fallback | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Default 800x800 sizing may fight RN layout and cause over/under-sizing
Hard defaults in native (px) can conflict with RN dp sizing, especially since RCTFragment passes PixelRatio.getPixelSizeForLayoutSize(800) on the JS side. Consider deriving from view.measuredWidth/Height when available, falling back only if zero.
- val width = propWidth ?: 800 // Default fallback
- val height = propHeight ?: 800 // Default fallback
+ val measuredW = if (view.measuredWidth > 0) view.measuredWidth else null
+ val measuredH = if (view.measuredHeight > 0) view.measuredHeight else null
+ val width = propWidth ?: measuredW ?: 800
+ val height = propHeight ?: measuredH ?: 800📝 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.
| val width = propWidth ?: 800 // Default fallback | |
| val height = propHeight ?: 800 // Default fallback | |
| val measuredW = if (view.measuredWidth > 0) view.measuredWidth else null | |
| val measuredH = if (view.measuredHeight > 0) view.measuredHeight else null | |
| val width = propWidth ?: measuredW ?: 800 | |
| val height = propHeight ?: measuredH ?: 800 |
🤖 Prompt for AI Agents
In
packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfOCRViewManager.kt
around lines 163-165, the code currently hard-sets width/height to 800 which can
conflict with React Native layout; change the logic to use propWidth/propHeight
if provided, otherwise use the view's measuredWidth/measuredHeight when > 0, and
only fall back to the hard default (e.g., 800) if both prop and measured values
are zero; ensure the values are treated as pixels and, if necessary, convert
from DP to pixels consistently with RN before using them.
| protected fun hasCameraPermission(): Boolean { | ||
| return ContextCompat.checkSelfPermission(context!!, Manifest.permission.CAMERA) != PackageManager.PERMISSION_GRANTED | ||
| val hasPermission = ContextCompat.checkSelfPermission(context!!, Manifest.permission.CAMERA) == PackageManager.PERMISSION_GRANTED | ||
| return hasPermission | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid context!! in permission check (potential NPE)
Using context!! can crash if the Fragment is detached. Prefer requireContext() only when you truly want to throw, or gracefully handle null.
- protected fun hasCameraPermission(): Boolean {
- val hasPermission = ContextCompat.checkSelfPermission(context!!, Manifest.permission.CAMERA) == PackageManager.PERMISSION_GRANTED
- return hasPermission
- }
+ protected fun hasCameraPermission(): Boolean {
+ val ctx = context ?: return false
+ return ContextCompat.checkSelfPermission(ctx, Manifest.permission.CAMERA) ==
+ PackageManager.PERMISSION_GRANTED
+ }📝 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.
| protected fun hasCameraPermission(): Boolean { | |
| return ContextCompat.checkSelfPermission(context!!, Manifest.permission.CAMERA) != PackageManager.PERMISSION_GRANTED | |
| val hasPermission = ContextCompat.checkSelfPermission(context!!, Manifest.permission.CAMERA) == PackageManager.PERMISSION_GRANTED | |
| return hasPermission | |
| } | |
| protected fun hasCameraPermission(): Boolean { | |
| val ctx = context ?: return false | |
| return ContextCompat.checkSelfPermission(ctx, Manifest.permission.CAMERA) == | |
| PackageManager.PERMISSION_GRANTED | |
| } |
🤖 Prompt for AI Agents
In
packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/ui/CameraFragment.kt
around lines 248–251, the permission check uses context!! which can throw an NPE
if the Fragment is detached; replace the forced unwrap with a safe check: either
use requireContext() if you intend to throw when context is missing, or
preferably use a nullable-safe form (e.g., context?.let {
ContextCompat.checkSelfPermission(it, Manifest.permission.CAMERA) ==
PackageManager.PERMISSION_GRANTED } ?: false) so the method returns false when
context is null.
* add qrcode module * fix test * fix test: fetchOfacTrees
seshanthS
temporarily deployed
to
development
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 24
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/mobile-sdk-alpha/android/build.gradle (4)
109-109: Remove React Native wildcard dependency and duplicates; pin to the app’s RN and avoid ABI drift.Two additions of react-native:+ will pull arbitrary versions and can mismatch the host app. Keep a single RN artifact and align versions via the app.
- implementation 'com.facebook.react:react-native:+' @@ - implementation 'com.facebook.react:react-native:+'Option: keep react-android only (as you already have) and ensure the app controls the exact RN version.
Also applies to: 140-142
3-7: Upgrade AGP & Gradle to support compileSdkVersion 35 (Android 15).AGP 8.6.0+ is required (Android recommends 8.6.1+). Match Gradle wrapper versions: AGP 8.6 → Gradle 8.7; 8.7 → 8.9; 8.8 → 8.10.2; 8.9/8.10 → 8.11.1.
File: packages/mobile-sdk-alpha/android/build.gradle lines 3–7 (also 18–20) set compileSdkVersion = 35 — update the project-level com.android.tools.build:gradle to 8.6.1 (or later) and gradle/wrapper/gradle-wrapper.properties distributionUrl to the corresponding Gradle (e.g., Gradle 8.7 for AGP 8.6) to avoid Android 15 build/tooling failures.
130-146: Remove duplicate Play Services ML Kit; keep standalone com.google.mlkitCode uses com.google.mlkit.* (TextRecognition.getClient). Remove the Play Services ML Kit artifacts to avoid class conflicts and bloat — prefer com.google.mlkit:text-recognition.
- packages/mobile-sdk-alpha/android/build.gradle — remove both occurrences of 'com.google.android.gms:play-services-mlkit-text-recognition:18.0.2' (around lines 130 and 144–145); keep 'com.google.mlkit:text-recognition:16.0.0'.
- app/android/android-passport-reader/app/build.gradle:56 — remove/replace 'com.google.android.gms:play-services-mlkit-text-recognition:18.0.2'.
- app/android/app/build.gradle:211 — review/align 'com.google.android.gms:play-services-mlkit-text-recognition-common:19.1.0' with the chosen artifact.
131-131: Do not remove Fotoapparat without a coordinated migration — it’s used throughout the repo.Multiple files import and use io.fotoapparat types (Frame, CameraView, Fotoapparat, FrameProcessor). packages/mobile-sdk-alpha/android/build.gradle declares Fotoapparat (line ~131) and CameraX (lines ~148–151), and Fotoapparat is referenced in (examples):
- packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/ui/CameraFragment.kt
- packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/ui/CameraMLKitFragment.kt
- packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/ui/QrCodeScannerFragment.kt
- packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/mlkit/VisionImageProcessor.kt and VisionProcessorBase.kt
- packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/utils/QrCodeDetectorProcessor.kt
App modules also include Fotoapparat (app/android/app/build.gradle and app/android/android-passport-reader/app/build.gradle) with duplicate usages.
Action (choose one):
- Migrate: replace all io.fotoapparat usages above with CameraX equivalents in a single, coordinated refactor, then remove the Fotoapparat dependency.
- Or keep Fotoapparat: remove or isolate CameraX dependencies (scope CameraX to a separate module) to avoid dual camera stacks, transitive bloat, and camera resource contention.
This is a mandatory refactor affecting the listed files and build.gradle entries.
🧹 Nitpick comments (5)
common/index.ts (1)
58-60: Harden identifier hashing and expose mandated types
- Security: Please confirm calculateUserIdentifierHash uses explicit domain separation, a canonical byte encoding, and a fixed version tag. Collisions or scope bleed here would be high‑impact.
- Compliance: Per coding guidelines, this package must define and export IdentityCommitment, DSCKeyCommitment, and VerificationConfig. I don’t see them exported here.
Apply outside-of-hunk additions:
// src/types/commitments.ts import type { Country3LetterCode } from '../constants/index.js'; export type DocumentType = 'passport' | 'eu_id_card'; export interface IdentityCommitment { commitment: string; // Poseidon hash (hex) nullifier: string; // domain-separated nullifier (hex) timestamp: number; // UTC epoch seconds version: string; // circuit version (semver) documentType: DocumentType; } export interface DSCKeyCommitment { publicKeyHash: string; // Poseidon hash (hex) certificateChain: string[]; // parent->leaf hashes (hex) revocationStatus: boolean; issuer: Country3LetterCode; } export interface VerificationConfig { circuitVersion: string; // semver complianceRules: string[]; // rule ids timeWindow: number; // seconds (e.g., 86400) clockDrift: number; // seconds (±300) trustAnchors: string[]; // CA/DSC anchors (hashes/ids) revocationRoots: string[]; // Merkle roots timeSource: 'NTP'; nullifierScope: string; // domain separation label/URN }Then re-export from this entrypoint:
export type { IdentityCommitment, DSCKeyCommitment, VerificationConfig } from './src/types/commitments.js';packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/utils/QrCodeDetectorProcessor.kt (1)
22-22: Remove deprecated AsyncTask importAsyncTask has been deprecated since API level 30 and is not used in this code.
Remove the unused import:
-import android.os.AsyncTaskpackages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfQRScannerModule.kt (1)
75-85: Resolve/reject on UI thread for consistency with RN and to avoid races.Callbacks can arrive off the UI thread; standardize dispatch.
override fun onQRData(data: String) { - scanPromise?.resolve(data) - scanPromise = null - cleanup() + (reactApplicationContext.currentActivity as? FragmentActivity)?.runOnUiThread { + scanPromise?.resolve(data) + scanPromise = null + cleanup() + } ?: run { + scanPromise?.resolve(data) + scanPromise = null + cleanup() + } } override fun onError(e: Exception) { - scanPromise?.reject("E_QR_SCAN_ERROR", e.message, e) - scanPromise = null - cleanup() + (reactApplicationContext.currentActivity as? FragmentActivity)?.runOnUiThread { + scanPromise?.reject("E_QR_SCAN_ERROR", e.message, e) + scanPromise = null + cleanup() + } ?: run { + scanPromise?.reject("E_QR_SCAN_ERROR", e.message, e) + scanPromise = null + cleanup() + } }packages/mobile-sdk-alpha/ios/SelfSDK/SelfQRScannerViewManager.swift (1)
63-65: Don’t run metadata callbacks on the main queue.Heavy QR processing and per‑frame callbacks on main risk frame drops. Use a dedicated serial queue.
- metadataOutput.setMetadataObjectsDelegate(self, queue: DispatchQueue.main) + metadataOutput.setMetadataObjectsDelegate(self, queue: DispatchQueue(label: "selfsdk.qr.metadata"))packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/ui/QrCodeScannerFragment.kt (1)
246-248: Fragment dialog creation without proper fragment lifecycle checkCreating and showing an error dialog without checking if the fragment is added could crash if the fragment is detached.
Add fragment state check:
if (grantResults.size != 1 || grantResults[0] != PackageManager.PERMISSION_GRANTED) { + if (isAdded) { ErrorDialog.newInstance("Camera permission is required to scan QR codes") .show(childFragmentManager, FRAGMENT_DIALOG) + } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
common/index.ts(3 hunks)packages/mobile-sdk-alpha/android/build.gradle(2 hunks)packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/RNSelfPassportReaderPackage.kt(1 hunks)packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfQRScannerModule.kt(1 hunks)packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfQRScannerViewManager.kt(1 hunks)packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/ui/QrCodeScannerFragment.kt(1 hunks)packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/utils/QrCodeDetectorProcessor.kt(1 hunks)packages/mobile-sdk-alpha/ios/SelfSDK/SelfQRScannerModule.m(1 hunks)packages/mobile-sdk-alpha/ios/SelfSDK/SelfQRScannerModule.swift(1 hunks)packages/mobile-sdk-alpha/ios/SelfSDK/SelfQRScannerViewManager.m(1 hunks)packages/mobile-sdk-alpha/ios/SelfSDK/SelfQRScannerViewManager.swift(1 hunks)packages/mobile-sdk-alpha/src/adapters/react-native/scanner.ts(3 hunks)packages/mobile-sdk-alpha/src/components/QRCodeScannerView.tsx(1 hunks)packages/mobile-sdk-alpha/src/index.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
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
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/technical-specification.mdc)
**/*.{ts,tsx}: Define IdentityCommitment with fields: commitment (Poseidon hash), nullifier (domain-separated), timestamp (UTC number), version (circuit version), documentType ('passport' | 'eu_id_card')
Define DSCKeyCommitment with fields: publicKeyHash (Poseidon hash), certificateChain (hashes), revocationStatus (boolean), issuer (country code)
Define VerificationConfig with fields: circuitVersion (semver), complianceRules array, timeWindow (seconds, 24h), clockDrift (±5 min), trustAnchors, revocationRoots, timeSource (NTP), nullifierScope (domain separation)
Files:
packages/mobile-sdk-alpha/src/index.tspackages/mobile-sdk-alpha/src/adapters/react-native/scanner.tspackages/mobile-sdk-alpha/src/components/QRCodeScannerView.tsxcommon/index.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/index.tspackages/mobile-sdk-alpha/src/adapters/react-native/scanner.tspackages/mobile-sdk-alpha/src/components/QRCodeScannerView.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/index.tspackages/mobile-sdk-alpha/src/adapters/react-native/scanner.tspackages/mobile-sdk-alpha/src/components/QRCodeScannerView.tsx
🧠 Learnings (15)
📚 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:
packages/mobile-sdk-alpha/android/build.gradle
📚 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/android/build.gradlepackages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/RNSelfPassportReaderPackage.kt
📚 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: Update the app to consume the mobile-sdk-alpha package and replace local modules with SDK imports
Applied to files:
packages/mobile-sdk-alpha/android/build.gradle
📚 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-alpha/android/build.gradlepackages/mobile-sdk-alpha/src/adapters/react-native/scanner.ts
📚 Learning: 2025-08-24T18:52:25.796Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-08-24T18:52:25.796Z
Learning: Applies to native/android/**/*.{kt,kts} : Android NFC: implement RNPassportReaderModule in Kotlin
Applied to files:
packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/RNSelfPassportReaderPackage.ktpackages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfQRScannerModule.kt
📚 Learning: 2025-08-24T18:52:25.796Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-08-24T18:52:25.796Z
Learning: Applies to native/ios/**/*.{swift} : iOS NFC: implement custom PassportReader as a Swift module
Applied to files:
packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/RNSelfPassportReaderPackage.ktpackages/mobile-sdk-alpha/ios/SelfSDK/SelfQRScannerModule.mpackages/mobile-sdk-alpha/ios/SelfSDK/SelfQRScannerModule.swiftpackages/mobile-sdk-alpha/src/adapters/react-native/scanner.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/index.tspackages/mobile-sdk-alpha/src/adapters/react-native/scanner.tspackages/mobile-sdk-alpha/src/components/QRCodeScannerView.tsxcommon/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} : Avoid introducing circular dependencies
Applied to files:
packages/mobile-sdk-alpha/src/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}} : Use actual imports from selfxyz/mobile-sdk-alpha in tests
Applied to files:
packages/mobile-sdk-alpha/src/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}} : Verify extractMRZInfo() using published sample MRZ strings (e.g., ICAO examples)
Applied to files:
packages/mobile-sdk-alpha/src/index.tspackages/mobile-sdk-alpha/src/adapters/react-native/scanner.ts
📚 Learning: 2025-08-24T18:52:25.796Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-08-24T18:52:25.796Z
Learning: Organize screens by feature modules (passport, home, settings, etc.)
Applied to files:
packages/mobile-sdk-alpha/src/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}} : Ensure parseNFCResponse() works with representative, synthetic NFC data
Applied to files:
packages/mobile-sdk-alpha/src/adapters/react-native/scanner.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-alpha/src/components/QRCodeScannerView.tsx
📚 Learning: 2025-08-29T15:30:12.210Z
Learnt from: CR
PR: selfxyz/self#0
File: app/AGENTS.md:0-0
Timestamp: 2025-08-29T15:30:12.210Z
Learning: Document complex native module changes for AI review
Applied to files:
common/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:
common/index.ts
🧬 Code graph analysis (5)
packages/mobile-sdk-alpha/ios/SelfSDK/SelfQRScannerModule.m (1)
app/ios/PassportReader.swift (3)
iOS(32-364)scanPassport(78-239)requiresMainQueueSetup(360-363)
packages/mobile-sdk-alpha/ios/SelfSDK/SelfQRScannerViewManager.swift (3)
packages/mobile-sdk-alpha/ios/SelfSDK/SelfQRScannerModule.swift (2)
requiresMainQueueSetup(21-23)moduleName(17-19)packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfQRScannerModule.kt (2)
onQRData(75-79)onError(81-85)packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfQRScannerViewManager.kt (2)
onQRData(146-152)onError(154-162)
packages/mobile-sdk-alpha/ios/SelfSDK/SelfQRScannerModule.swift (4)
packages/mobile-sdk-alpha/ios/SelfSDK/SelfQRScannerViewManager.swift (3)
moduleName(26-28)requiresMainQueueSetup(18-20)view(22-24)packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfQRScannerModule.kt (3)
startScanning(27-73)onQRData(75-79)onError(81-85)packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfQRScannerViewManager.kt (2)
onQRData(146-152)onError(154-162)packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/ui/QrCodeScannerFragment.kt (3)
onQRData(329-332)onQRData(330-330)onError(331-331)
packages/mobile-sdk-alpha/src/adapters/react-native/scanner.ts (1)
packages/mobile-sdk-alpha/src/types/public.ts (2)
ScanOpts(141-154)ScanResult(171-171)
packages/mobile-sdk-alpha/src/components/QRCodeScannerView.tsx (3)
packages/mobile-sdk-alpha/src/index.ts (1)
QRCodeScannerView(70-70)packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfQRScannerViewManager.kt (1)
onQRData(146-152)packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/ui/QrCodeScannerFragment.kt (2)
onQRData(329-332)onQRData(330-330)
🪛 SwiftLint (0.57.0)
packages/mobile-sdk-alpha/ios/SelfSDK/SelfQRScannerModule.swift
[Warning] 60-60: TODOs should be resolved (- check)
(todo)
🪛 detekt (1.23.8)
packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/ui/QrCodeScannerFragment.kt
[warning] 143-144: This empty block of code can be removed.
(detekt.empty-blocks.EmptyFunctionBlock)
⏰ 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-android
- GitHub Check: build-ios
- GitHub Check: test
- GitHub Check: analyze-android
- GitHub Check: analyze-ios
🔇 Additional comments (7)
common/index.ts (2)
76-76: Lock ABI packing; add golden testsgetSolidityPackedUserContextData must remain byte‑for‑byte stable with the on‑chain ABI. Please add a golden test comparing the packed bytes and keccak256 against a known contract fixture to prevent silent breakage.
95-102: Remove Aadhaar test utilities from the public index — move to a testing-only entrypointExporting generateTestData, testCustomData, and prepareAadhaar* from the top-level index (common/index.ts) invites accidental production use, increases bundle size, and may leak test vectors. Location: common/index.ts (≈ lines 95–102).
-export { generateTestData, testCustomData } from './src/utils/aadhaar/utils.js'; - -export { - prepareAadhaarDiscloseTestData, - prepareAadhaarRegisterData, - prepareAadhaarRegisterTestData, -} from './src/utils/aadhaar/mockData.js';Action: Move these exports into a dedicated testing entrypoint (e.g., ./testing or ./test-utils) and remove them from the package's production exports/exports map.
Run locally to confirm there are no non-test usages:
rg -n -C1 -S '(generateTestData|testCustomData|prepareAadhaar(?:Disclose|Register(?:Test)?Data)|genMockIdDoc(?:AndInitDataParsing)?)' \ --glob '!**/__tests__/**' --glob '!**/*.test.*' --glob '!node_modules/**' .packages/mobile-sdk-alpha/src/index.ts (1)
70-70: LGTM! Proper SDK module re-exportThe QRCodeScannerView export follows the established pattern and aligns with the retrieved learning about re-exporting new SDK modules via the index.
packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/RNSelfPassportReaderPackage.kt (1)
26-37: LGTM! Consistent module registrationThe QR scanner module and view manager are properly registered alongside the existing MRZ scanner components, maintaining architectural consistency.
packages/mobile-sdk-alpha/src/adapters/react-native/scanner.ts (1)
121-133: LGTM! Proper QR scanning implementationThe QR scanning implementation follows the established pattern for other scan modes with appropriate error handling and native module validation.
Also applies to: 222-234
packages/mobile-sdk-alpha/ios/SelfSDK/SelfQRScannerViewManager.m (1)
1-12: LGTM! Standard React Native view manager bridgeThe iOS view manager properly exports the required event properties for QR scanning, following React Native conventions.
packages/mobile-sdk-alpha/ios/SelfSDK/SelfQRScannerModule.m (1)
7-12: Bridge signature matches Swift export — good.Obj‑C externs align with the Swift selector and promise types; nothing blocking here.
| genMockIdDoc, | ||
| genMockIdDocAndInitDataParsing, |
There was a problem hiding this comment.
Don’t export mock ID generators from the public SDK entrypoint
Shipping genMockIdDoc and genMockIdDocAndInitDataParsing on the main index increases misuse risk in production apps and widens the attack surface. Move these to a dedicated testing entrypoint.
- genMockIdDoc,
- genMockIdDocAndInitDataParsing,Outside-of-hunk: create a test-only entrypoint and re-export there:
// common/testing.ts
export {
genAndInitMockPassportData,
genMockIdDoc,
genMockIdDocAndInitDataParsing,
} from './src/utils/index.js';
export { generateTestData, testCustomData } from './src/utils/aadhaar/utils.js';
export {
prepareAadhaarDiscloseTestData,
prepareAadhaarRegisterData,
prepareAadhaarRegisterTestData,
} from './src/utils/aadhaar/mockData.js';Consider also moving genAndInitMockPassportData (Line 63) for consistency.
🤖 Prompt for AI Agents
In common/index.ts around lines 64-65, the public SDK incorrectly exports
test-only functions genMockIdDoc and genMockIdDocAndInitDataParsing; remove
these exports from the main index and instead create a new test-only entrypoint
(e.g., common/testing.ts) that re-exports genAndInitMockPassportData,
genMockIdDoc, genMockIdDocAndInitDataParsing from ./src/utils/index.js and any
other test helpers (generateTestData, testCustomData,
prepareAadhaarDiscloseTestData, etc.); update package exports (or build config)
to expose the new testing entrypoint only for tests and adjust any imports in
the repo to point to common/testing where needed, optionally moving
genAndInitMockPassportData (line 63) into the test entrypoint for consistency.
| exclude 'META-INF/androidx.exifinterface_exifinterface.version' | ||
| pickFirst '**/libc++_shared.so' | ||
| pickFirst '**/libjsc.so' | ||
| pickFirst 'META-INF/versions/9/OSGI-INF/MANIFEST.MF' |
There was a problem hiding this comment.
Avoid pickFirst on OSGI manifest; exclude instead for deterministic builds.
pickFirst masks conflicts and depends on dependency order. Prefer excluding the OSGI metadata so builds are stable.
- pickFirst 'META-INF/versions/9/OSGI-INF/MANIFEST.MF'
+ exclude 'META-INF/versions/9/OSGI-INF/MANIFEST.MF'📝 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.
| pickFirst 'META-INF/versions/9/OSGI-INF/MANIFEST.MF' | |
| exclude 'META-INF/versions/9/OSGI-INF/MANIFEST.MF' |
🤖 Prompt for AI Agents
In packages/mobile-sdk-alpha/android/build.gradle around line 99, the build
currently uses pickFirst 'META-INF/versions/9/OSGI-INF/MANIFEST.MF' which masks
conflicts and yields non-deterministic builds; replace this with an explicit
exclusion in the packagingOptions/resources (or
android.packagingOptions.resources.excludes) to remove that OSGI manifest
instead of picking first so the file is excluded deterministically from the
APK/AAR.
| // QR Code scanning dependencies | ||
| implementation 'com.google.zxing:core:3.5.2' | ||
| implementation 'com.google.zxing:android-core:3.3.0' |
There was a problem hiding this comment.
Drop zxing:android-core (deprecated, pre‑AndroidX); it can clash with AndroidX and add legacy support libs.
If you’re decoding from CameraX frames, ZXing core is sufficient. If you need camera UI/helpers, use journeyapps’ AndroidX artifact instead.
Minimal change:
- implementation 'com.google.zxing:core:3.5.2'
- implementation 'com.google.zxing:android-core:3.3.0'
+ implementation 'com.google.zxing:core:3.5.2'Alternative (if you want the embedded helpers):
- implementation 'com.google.zxing:core:3.5.2'
- implementation 'com.google.zxing:android-core:3.3.0'
+ implementation 'com.journeyapps:zxing-android-embedded:4.3.0'📝 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.
| // QR Code scanning dependencies | |
| implementation 'com.google.zxing:core:3.5.2' | |
| implementation 'com.google.zxing:android-core:3.3.0' | |
| // QR Code scanning dependencies | |
| implementation 'com.google.zxing:core:3.5.2' |
🤖 Prompt for AI Agents
In packages/mobile-sdk-alpha/android/build.gradle around lines 160 to 162,
remove the deprecated pre-AndroidX dependency
"com.google.zxing:android-core:3.3.0" to avoid AndroidX clashes and legacy
support libs; keep "com.google.zxing:core:3.5.2" for decoding from CameraX
frames, or if you need embedded camera UI/helpers replace the removed line with
the AndroidX-compatible JourneyApps artifact (android-integration) and update
its version accordingly.
| @ReactMethod | ||
| fun startScanning(promise: Promise) { | ||
| scanPromise = promise | ||
| val activity = reactApplicationContext.currentActivity as? FragmentActivity | ||
| if (activity == null) { | ||
| promise.reject("E_NO_ACTIVITY", "No FragmentActivity found") | ||
| return | ||
| } | ||
|
|
There was a problem hiding this comment.
Guard against concurrent scans to avoid promise overwrite and UI leaks.
Second calls will overwrite scanPromise and stack overlays. Reject if a scan is already active.
Apply this diff:
@ReactMethod
fun startScanning(promise: Promise) {
- scanPromise = promise
+ if (scanPromise != null || currentFragment != null || currentContainer != null) {
+ promise.reject("E_ALREADY_SCANNING", "A scan is already in progress")
+ return
+ }
+ scanPromise = promise📝 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.
| @ReactMethod | |
| fun startScanning(promise: Promise) { | |
| scanPromise = promise | |
| val activity = reactApplicationContext.currentActivity as? FragmentActivity | |
| if (activity == null) { | |
| promise.reject("E_NO_ACTIVITY", "No FragmentActivity found") | |
| return | |
| } | |
| @ReactMethod | |
| fun startScanning(promise: Promise) { | |
| if (scanPromise != null || currentFragment != null || currentContainer != null) { | |
| promise.reject("E_ALREADY_SCANNING", "A scan is already in progress") | |
| return | |
| } | |
| scanPromise = promise | |
| val activity = reactApplicationContext.currentActivity as? FragmentActivity | |
| if (activity == null) { | |
| promise.reject("E_NO_ACTIVITY", "No FragmentActivity found") | |
| return | |
| } |
🤖 Prompt for AI Agents
In
packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfQRScannerModule.kt
around lines 27 to 35, the startScanning method currently overwrites scanPromise
on concurrent calls and may create duplicate UI overlays; guard against
concurrent scans by checking if scanPromise is already non-null (or a scan
fragment/popup is already present) and immediately reject the incoming promise
with a clear error code/message (e.g. "E_SCAN_IN_PROGRESS") instead of replacing
the existing promise; ensure you do not proceed to create or show a new
fragment/overlay when a scan is active, and keep the original promise/cleanup
flow intact so the existing scan can finish or be cancelled safely.
packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfQRScannerModule.kt
Show resolved
Hide resolved
| @objc func startScanning(_ resolve: @escaping RCTPromiseResolveBlock, rejecter reject: @escaping RCTPromiseRejectBlock) { | ||
| DispatchQueue.main.async { | ||
| guard let rootViewController = UIApplication.shared.keyWindow?.rootViewController else { | ||
| reject("error", "Unable to find root view controller", nil) | ||
| return | ||
| } | ||
|
|
||
| let scannerView = SelfQRScannerView(frame: rootViewController.view.bounds) | ||
|
|
||
| scannerView.onQRData = { resultDict in | ||
| if let dict = resultDict, let data = dict["data"] as? String { | ||
| resolve(data) | ||
| } else { | ||
| reject("error", "Invalid QR code data", nil) | ||
| } | ||
| } | ||
|
|
||
| scannerView.onError = { errorDict in | ||
| if let dict = errorDict { | ||
| let errorMessage = dict["errorMessage"] as? String ?? "QR scanning failed" | ||
| reject("error", errorMessage, nil) | ||
| } else { | ||
| reject("error", "QR scanning failed", nil) | ||
| } | ||
| } | ||
|
|
||
| let modalViewController = UIViewController() | ||
| modalViewController.view = scannerView | ||
| modalViewController.modalPresentationStyle = .fullScreen | ||
| rootViewController.present(modalViewController, animated: true, completion: nil) | ||
| } |
There was a problem hiding this comment.
Fix root VC lookup and add proper modal dismissal/one‑shot guarding.
UIApplication.shared.keyWindowis deprecated and often nil under multi‑scene; useRCTPresentedViewController()(React) or scene‑based lookup.- The modal is never dismissed on resolve/reject, leaving users stuck with a full‑screen overlay and the camera active.
- Guard against multiple settlements.
Apply this refactor:
@objc func startScanning(_ resolve: @escaping RCTPromiseResolveBlock, rejecter reject: @escaping RCTPromiseRejectBlock) {
DispatchQueue.main.async {
- guard let rootViewController = UIApplication.shared.keyWindow?.rootViewController else {
- reject("error", "Unable to find root view controller", nil)
- return
- }
-
- let scannerView = SelfQRScannerView(frame: rootViewController.view.bounds)
-
- scannerView.onQRData = { resultDict in
- if let dict = resultDict, let data = dict["data"] as? String {
- resolve(data)
- } else {
- reject("error", "Invalid QR code data", nil)
- }
- }
-
- scannerView.onError = { errorDict in
- if let dict = errorDict {
- let errorMessage = dict["errorMessage"] as? String ?? "QR scanning failed"
- reject("error", errorMessage, nil)
- } else {
- reject("error", "QR scanning failed", nil)
- }
- }
-
- let modalViewController = UIViewController()
- modalViewController.view = scannerView
- modalViewController.modalPresentationStyle = .fullScreen
- rootViewController.present(modalViewController, animated: true, completion: nil)
+ guard let presenter = RCTPresentedViewController() ?? UIApplication.shared.connectedScenes
+ .compactMap { ($0 as? UIWindowScene)?.keyWindow?.rootViewController }
+ .first else {
+ reject("error", "Unable to find a presenter view controller", nil)
+ return
+ }
+
+ var handled = false
+ let modalVC = UIViewController()
+ let scannerView = SelfQRScannerView(frame: presenter.view.bounds)
+ modalVC.view = scannerView
+ modalVC.modalPresentationStyle = .fullScreen
+
+ let finish: (Result<String, String>) -> Void = { result in
+ guard !handled else { return }
+ handled = true
+ modalVC.dismiss(animated: true) {
+ switch result {
+ case .success(let data):
+ resolve(data)
+ case .failure(let message):
+ reject("error", message, nil)
+ }
+ }
+ }
+
+ scannerView.onQRData = { resultDict in
+ if let dict = resultDict, let data = dict["data"] as? String {
+ finish(.success(data))
+ } else {
+ finish(.failure("Invalid QR code data"))
+ }
+ }
+
+ scannerView.onError = { errorDict in
+ let errorMessage = (errorDict?["errorMessage"] as? String) ?? "QR scanning failed"
+ finish(.failure(errorMessage))
+ }
+
+ presenter.present(modalVC, animated: true, completion: nil)
}
}Committable suggestion skipped: line range outside the PR's diff.
| override func layoutSubviews() { | ||
| super.layoutSubviews() | ||
| previewLayer?.frame = self.bounds | ||
| } | ||
| } |
There was a problem hiding this comment.
Stop session when the view is detached to avoid camera leaks.
If unmounted without a scan, the camera continues running.
Add (outside this hunk):
override func willMove(toWindow newWindow: UIWindow?) {
super.willMove(toWindow: newWindow)
if newWindow == nil, let session = captureSession, session.isRunning {
DispatchQueue.global(qos: .background).async { session.stopRunning() }
}
}🤖 Prompt for AI Agents
In packages/mobile-sdk-alpha/ios/SelfSDK/SelfQRScannerViewManager.swift around
lines 96-100, the view does not stop the AVCaptureSession when it is detached,
causing the camera to keep running; add an override of willMove(toWindow:)
(placed outside the shown layoutSubviews hunk) that checks if newWindow is nil
and captureSession?.isRunning is true, and if so calls session.stopRunning() on
a background DispatchQueue to stop the camera and avoid leaks.
| style={{ | ||
| height: PixelRatio.getPixelSizeForLayoutSize(800), | ||
| width: PixelRatio.getPixelSizeForLayoutSize(400), | ||
| }} |
There was a problem hiding this comment.
Avoid PixelRatio.getPixelSizeForLayoutSize for view sizing — it explodes dimensions and hurts performance.
getPixelSizeForLayoutSize(800) returns physical pixels; React Native styles expect DIP. On xxhdpi devices this yields multi‑thousand DIP sizes, causing oversized renders and jank.
Use DIP or fill styles instead:
- style={{
- height: PixelRatio.getPixelSizeForLayoutSize(800),
- width: PixelRatio.getPixelSizeForLayoutSize(400),
- }}
+ style={{
+ width: '100%',
+ height: '100%',
+ }}If you need overscan to avoid letterboxing, prefer slight percentages (e.g., 110%) similar to iOS, or compute from Dimensions.get('window') in DIP.
📝 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.
| style={{ | |
| height: PixelRatio.getPixelSizeForLayoutSize(800), | |
| width: PixelRatio.getPixelSizeForLayoutSize(400), | |
| }} | |
| style={{ | |
| width: '100%', | |
| height: '100%', | |
| }} |
🤖 Prompt for AI Agents
In packages/mobile-sdk-alpha/src/components/QRCodeScannerView.tsx around lines
87 to 90, the view is using PixelRatio.getPixelSizeForLayoutSize which returns
physical pixels and inflates size on high‑density screens; replace those calls
with DIP-based sizing (e.g., use percentage-based styles like width: '100%' /
height: '100%' or a small overscan like width: '110%' / height: '110%') or
compute DIP dimensions via Dimensions.get('window') and use those values,
ensuring styles use DIP units rather than PixelRatio.getPixelSizeForLayoutSize
to avoid oversized renders and performance issues.
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfMRZScannerModule.kt (1)
20-22: Clean up on RN lifecycle events to avoid leaks when host is destroyed.Tie overlay/fragment cleanup to the RN lifecycle; prevents stuck overlays when the app backgrounds or activity is destroyed.
-class SelfMRZScannerModule(reactContext: ReactApplicationContext) : -ReactContextBaseJavaModule(reactContext), CameraMLKitFragment.CameraMLKitCallback { +class SelfMRZScannerModule(reactContext: ReactApplicationContext) : + ReactContextBaseJavaModule(reactContext), + CameraMLKitFragment.CameraMLKitCallback, + com.facebook.react.bridge.LifecycleEventListener { + + init { reactContext.addLifecycleEventListener(this) }Add near the end (new code):
override fun onCatalystInstanceDestroy() { reactApplicationContext.removeLifecycleEventListener(this) cleanup() } override fun onHostResume() {} override fun onHostPause() {} override fun onHostDestroy() { cleanup() }Also applies to: 124-125
packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfOCRViewManager.kt (3)
151-163: Destroy path can NPE and re-schedules layout; fix and make state-safe
- setupLayout() in destroy re-adds a perpetual frame callback.
- Unsafe cast to FragmentActivity and no state checks can crash.
- private fun destroyFragment(root: FrameLayout, reactNativeViewId: Int) { - val parentView = root.findViewById<ViewGroup>(reactNativeViewId) - setupLayout(parentView) - - val activity = reactContext.currentActivity as FragmentActivity - val cameraFragment = activity.supportFragmentManager.findFragmentByTag(reactNativeViewId.toString()) - cameraFragment?.let { - activity.supportFragmentManager - .beginTransaction() - .remove(it) - .commit() - } - } + private fun destroyFragment(root: FrameLayout, reactNativeViewId: Int) { + val parentView = root.findViewById<ViewGroup?>(reactNativeViewId) + if (parentView == null) { + android.util.Log.w("SelfOCRViewManager", "destroyFragment: container $reactNativeViewId not found") + return + } + val activity = reactContext.currentActivity as? FragmentActivity + if (activity == null || activity.isFinishing || activity.isDestroyed) { + android.util.Log.w("SelfOCRViewManager", "destroyFragment: invalid activity; dropping remove for $reactNativeViewId") + return + } + val fm = activity.supportFragmentManager + if (fm.isStateSaved) { + android.util.Log.w("SelfOCRViewManager", "destroyFragment: state saved; skipping remove for $reactNativeViewId") + return + } + fm.findFragmentByTag(reactNativeViewId.toString())?.let { + fm.beginTransaction() + .setReorderingAllowed(true) + .remove(it) + .commit() + } + }
165-173: Per-frame callback never stops; gate on attachment to prevent leaksThe Choreographer loop will continue after the view is detached, wasting CPU/battery. Stop re-posting when not attached.
Choreographer.getInstance().postFrameCallback(object: Choreographer.FrameCallback { override fun doFrame(frameTimeNanos: Long) { - manuallyLayoutChildren(view) - view.viewTreeObserver.dispatchOnGlobalLayout() - Choreographer.getInstance().postFrameCallback(this) + if (!view.isAttachedToWindow) return + manuallyLayoutChildren(view) + view.viewTreeObserver.dispatchOnGlobalLayout() + if (view.isAttachedToWindow) { + Choreographer.getInstance().postFrameCallback(this) + } } })
187-203: Event emission can NPE and is unsafe for multiple instancesUsing this.reactNativeViewId!! can crash and a single field is incorrect if multiple SelfOCR views exist; events may go to the wrong target.
Immediate guard:
- .receiveEvent(this.reactNativeViewId!!, SUCCESS_EVENT, event) + val targetId = this.reactNativeViewId + if (targetId == null) { + android.util.Log.w("SelfOCRViewManager", "onPassportRead dropped: viewId is null") + return + } + reactContext + .getJSModule(RCTEventEmitter::class.java) + .receiveEvent(targetId, SUCCESS_EVENT, event)Apply analogous change in onError.
Recommended: pass the viewId into CameraMLKitFragment (e.g., newInstance(viewId)) and have the callback include viewId so events are always emitted to the correct React tag when multiple instances are present.
🧹 Nitpick comments (2)
packages/mobile-sdk-alpha/ios/SelfSDK/SelfQRScannerViewManager.swift (2)
60-60: Consider standardizing error structure with Android implementation.The iOS error format uses different field names than the Android counterpart. For consistency across platforms, consider aligning the error structure.
Looking at the Android implementation in
/packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfQRScannerViewManager.kt, the error format useserrorMessage,error, andstackTracefields, which matches your current implementation. This consistency is good for cross-platform usage.Also applies to: 71-71
97-97: Remove commented code.The commented line should be cleaned up since the functionality is properly implemented below.
- // captureSession?.stopRunning()
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfMRZScannerModule.kt(1 hunks)packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfOCRViewManager.kt(4 hunks)packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfQRScannerModule.kt(1 hunks)packages/mobile-sdk-alpha/ios/SelfSDK/SelfQRScannerViewManager.swift(1 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
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)
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/
📚 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/android/src/main/java/com/selfxyz/selfSDK/SelfMRZScannerModule.kt
📚 Learning: 2025-08-24T18:52:25.796Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-08-24T18:52:25.796Z
Learning: Applies to native/android/**/*.{kt,kts} : Android NFC: implement RNPassportReaderModule in Kotlin
Applied to files:
packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfMRZScannerModule.ktpackages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfQRScannerModule.kt
📚 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/android/src/main/java/com/selfxyz/selfSDK/SelfMRZScannerModule.kt
📚 Learning: 2025-08-25T14:07:52.997Z
Learnt from: aaronmgdr
PR: selfxyz/self#889
File: app/src/utils/utils.ts:20-29
Timestamp: 2025-08-25T14:07:52.997Z
Learning: In the SELF passport/identity verification app, the primary PII concerns are MRZ data from passports and document numbers. SSNs and credit cards are not relevant to this domain, so PII redaction should focus on passport-specific data patterns rather than general financial/personal identifiers.
Applied to files:
packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfMRZScannerModule.kt
📚 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-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfOCRViewManager.kt
📚 Learning: 2025-09-11T19:41:16.601Z
Learnt from: aaronmgdr
PR: selfxyz/self#1055
File: app/src/components/homeScreen/idCard.tsx:6-6
Timestamp: 2025-09-11T19:41:16.601Z
Learning: The React Native app in selfxyz/self does not implement responsive layouts for orientation changes. The codebase consistently uses Dimensions.get('window') rather than useWindowDimensions(), indicating that orientation handling is not a priority or architectural pattern in this project.
Applied to files:
packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfOCRViewManager.kt
🧬 Code graph analysis (3)
packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfMRZScannerModule.kt (1)
packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfQRScannerModule.kt (2)
generateUnusedId(114-120)cleanup(97-112)
packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfQRScannerModule.kt (1)
packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfMRZScannerModule.kt (2)
generateUnusedId(94-98)cleanup(100-124)
packages/mobile-sdk-alpha/ios/SelfSDK/SelfQRScannerViewManager.swift (3)
packages/mobile-sdk-alpha/ios/SelfSDK/SelfQRScannerModule.swift (2)
requiresMainQueueSetup(21-23)moduleName(17-19)packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfQRScannerModule.kt (2)
onQRData(85-89)onError(91-95)packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfQRScannerViewManager.kt (2)
onQRData(146-152)onError(154-162)
🔇 Additional comments (8)
packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfMRZScannerModule.kt (1)
82-86: Do not return raw MRZ; return minimal, structured, masked data.Returning
mrzInfo.toString()exposes full PII and risks accidental logging/retention. Return only necessary fields and mask identifiers (per learnings on MRZ PII).Apply:
- override fun onPassportRead(mrzInfo: MRZInfo) { - scanPromise?.resolve(mrzInfo.toString()) - scanPromise = null - cleanup() - } + override fun onPassportRead(mrzInfo: MRZInfo) { + val payload = com.facebook.react.bridge.Arguments.createMap().apply { + putString("documentType", mrzInfo.documentCode) + putString("issuingState", mrzInfo.issuingState) + putString("nationality", mrzInfo.nationality) + putString("documentNumberMasked", mrzInfo.documentNumber?.let { maskDoc(it) }) + // Add more fields only if strictly required by the JS API. + } + scanPromise?.resolve(payload) + scanPromise = null + cleanup() + }Add helper (outside the ranges, near other privates):
private fun maskDoc(v: String): String = if (v.length <= 3) "***" else "*".repeat(v.length - 3) + v.takeLast(3)packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfQRScannerModule.kt (2)
27-35: Guard re-entrancy and assign promise only after validation.Concurrent calls overwrite the promise and can stack overlays; also leaves a stale promise when activity is null.
@ReactMethod fun startScanning(promise: Promise) { - scanPromise = promise - val activity = reactApplicationContext.currentActivity as? FragmentActivity - if (activity == null) { - promise.reject("E_NO_ACTIVITY", "No FragmentActivity found") - return - } + if (scanPromise != null || currentFragment != null || currentContainer != null) { + promise.reject("E_SCAN_IN_PROGRESS", "A scan is already in progress") + return + } + val activity = reactApplicationContext.currentActivity as? FragmentActivity + if (activity == null) { + promise.reject("E_NO_ACTIVITY", "No FragmentActivity found") + return + } + scanPromise = promise
78-81: Cleanup on error and clear promise to avoid dangling state/UI.Catch path leaves overlays and a non-null promise.
} catch (e: Exception) { android.util.Log.e("SelfQRScannerModule", "Error in startScanning", e) - promise.reject("E_SCANNING_ERROR", e.message, e) + try { cleanup() } catch (_: Exception) {} + scanPromise = null + promise.reject("E_SCANNING_ERROR", e.message, e) }packages/mobile-sdk-alpha/ios/SelfSDK/SelfQRScannerViewManager.swift (3)
47-52: LGTM! Camera initialization lifecycle properly addressed.The implementation correctly moves scanner initialization to
didMoveToWindow()and guards against double initialization, ensuring React Native event handlers are properly bound before any callbacks are emitted.
98-102: LGTM! Proper background thread handling for session stopRunning.The capture session is correctly stopped on a background thread to prevent UI blocking, addressing the threading concern from previous reviews.
31-111: Add session cleanup when view is detached to prevent camera resource leaks.The scanner starts when attached to a window but doesn't stop when detached, potentially leaving the camera running after the view is unmounted.
Add this method to properly clean up camera resources:
+ override func willMove(toWindow newWindow: UIWindow?) { + super.willMove(toWindow: newWindow) + if newWindow == nil, let session = captureSession, session.isRunning { + DispatchQueue.global(qos: .background).async { + session.stopRunning() + } + } + } + override func layoutSubviews() { super.layoutSubviews() previewLayer?.frame = self.bounds }packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfOCRViewManager.kt (2)
175-178: Hardcoded 800x800 leads to wrong sizing/cropping; derive from measured size firstUse measured width/height (or current layout) before falling back to a constant.
- val width = propWidth ?: 800 // Default fallback - val height = propHeight ?: 800 // Default fallback + val measuredW = if (view.measuredWidth > 0) view.measuredWidth else null + val measuredH = if (view.measuredHeight > 0) view.measuredHeight else null + val width = propWidth ?: measuredW ?: view.width.takeIf { it > 0 } ?: 800 + val height = propHeight ?: measuredH ?: view.height.takeIf { it > 0 } ?: 800
75-80: Mirror robust arg validation for int-command overloadSame crash vector here; validate size and type before getInt(0).
- val reactNativeViewId = args?.getInt(0) - if (reactNativeViewId == null) { - android.util.Log.w("SelfOCRViewManager", "receiveCommand called with null or empty args for command: $commandId") - return - } + val reactNativeViewId = args?.let { arr -> + if (arr.size() > 0 && arr.getType(0) == ReadableType.Number) arr.getInt(0) else null + } + if (reactNativeViewId == null) { + android.util.Log.w("SelfOCRViewManager", "receiveCommand: missing/invalid viewId for commandId=$commandId; args=$args") + return + }
| val container = FrameLayout(activity) | ||
| // just using view.generateViewId() doesn't work. | ||
| val containerId = generateUnusedId(activity.window.decorView as ViewGroup) | ||
| container.id = containerId | ||
|
|
||
| container.layoutParams = ViewGroup.LayoutParams( |
There was a problem hiding this comment.
Prevent orphaned overlays on exceptions by storing container early.
If an exception occurs before currentContainer is set, the added view can stick. Set the reference immediately after assigning the ID.
val container = FrameLayout(activity)
// just using view.generateViewId() doesn't work.
val containerId = generateUnusedId(activity.window.decorView as ViewGroup)
container.id = containerId
+ // Ensure we can clean up even if later steps fail
+ currentContainer = container
@@
- // Store references for cleanup
- currentContainer = container
- currentFragment = fragment
+ // Store fragment reference for cleanup
+ currentFragment = fragmentAlso applies to: 66-68
🤖 Prompt for AI Agents
In
packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfMRZScannerModule.kt
around lines 44-49 (and also apply the same change at lines ~66-68), the newly
created FrameLayout container is assigned an ID and added to the view hierarchy
but currentContainer isn’t set until later, which can leave an orphaned overlay
if an exception occurs; fix this by assigning currentContainer = container
immediately after setting container.id (before any operations that may throw) so
the reference exists for cleanup, and make the identical change at the other
location mentioned (lines 66-68).
| activity.supportFragmentManager | ||
| .beginTransaction() | ||
| .replace(containerId, fragment) | ||
| .commitNow() | ||
|
|
There was a problem hiding this comment.
Avoid commitNow crashes when state is saved; guard and fall back.
commitNow() can throw if FragmentManager state is saved. Use isStateSaved and fall back to commitAllowingStateLoss().
- activity.supportFragmentManager
- .beginTransaction()
- .replace(containerId, fragment)
- .commitNow()
+ val fm = activity.supportFragmentManager
+ val tx = fm.beginTransaction().replace(containerId, fragment)
+ if (fm.isStateSaved) {
+ tx.commitAllowingStateLoss()
+ android.util.Log.w("SelfMRZScannerModule", "Fragment committed with state loss due to saved state")
+ } else {
+ tx.commitNow()
+ }📝 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.
| activity.supportFragmentManager | |
| .beginTransaction() | |
| .replace(containerId, fragment) | |
| .commitNow() | |
| val fm = activity.supportFragmentManager | |
| val tx = fm.beginTransaction().replace(containerId, fragment) | |
| if (fm.isStateSaved) { | |
| tx.commitAllowingStateLoss() | |
| android.util.Log.w("SelfMRZScannerModule", "Fragment committed with state loss due to saved state") | |
| } else { | |
| tx.commitNow() | |
| } |
🤖 Prompt for AI Agents
In
packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfMRZScannerModule.kt
around lines 69 to 73, the code calls fragment transactions with commitNow()
which can throw if the FragmentManager state is already saved; change the logic
to check activity.supportFragmentManager.isStateSaved() before committing and,
if true, use commitAllowingStateLoss() (or commitNowAllowingStateLoss() if
immediate behavior is required) as a safe fallback so the app doesn't crash when
state is saved.
| override fun onError(e: Exception) { | ||
| scanPromise?.reject(e) | ||
| scanPromise = null | ||
| cleanup() | ||
| } |
There was a problem hiding this comment.
Use explicit error code for JS contract consistency.
Promise.reject(e) yields unspecified codes; standardize to a named code to allow JS to handle reliably.
- override fun onError(e: Exception) {
- scanPromise?.reject(e)
+ override fun onError(e: Exception) {
+ scanPromise?.reject("E_MRZ_SCAN_ERROR", e.message, e)
scanPromise = null
cleanup()
}📝 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.
| override fun onError(e: Exception) { | |
| scanPromise?.reject(e) | |
| scanPromise = null | |
| cleanup() | |
| } | |
| override fun onError(e: Exception) { | |
| scanPromise?.reject("E_MRZ_SCAN_ERROR", e.message, e) | |
| scanPromise = null | |
| cleanup() | |
| } |
🤖 Prompt for AI Agents
In
packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfMRZScannerModule.kt
around lines 88 to 92, the onError currently calls scanPromise?.reject(e) which
yields unspecified error codes to JS; change it to reject with an explicit
string error code (e.g. "SCAN_ERROR"), pass the exception message as the second
parameter, and include the original exception as the throwable/third parameter
so JS receives a stable code, readable message, and the underlying exception;
keep the subsequent scanPromise = null and cleanup() calls unchanged.
| val reactNativeViewId = args?.getInt(0) | ||
| if (reactNativeViewId == null) { | ||
| android.util.Log.w("SelfOCRViewManager", "receiveCommand called with null or empty args for command: $commandId") | ||
| return | ||
| } |
There was a problem hiding this comment.
Guard ReadableArray bounds/types; current access can throw
args?.getInt(0) will still crash if args exists but is empty or type ≠ number. Validate size and type before reading.
Apply:
- val reactNativeViewId = args?.getInt(0)
- if (reactNativeViewId == null) {
- android.util.Log.w("SelfOCRViewManager", "receiveCommand called with null or empty args for command: $commandId")
- return
- }
+ val reactNativeViewId = args?.let { arr ->
+ if (arr.size() > 0 && arr.getType(0) == ReadableType.Number) arr.getInt(0) else null
+ }
+ if (reactNativeViewId == null) {
+ android.util.Log.w("SelfOCRViewManager", "receiveCommand: missing/invalid viewId for command=$commandId; args=$args")
+ return
+ }Add import:
import com.facebook.react.bridge.ReadableType🤖 Prompt for AI Agents
In
packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfOCRViewManager.kt
around lines 48 to 52, the code accesses args?.getInt(0) without checking bounds
or type which can crash if args exists but is empty or the element is not a
number; update the code to first import com.facebook.react.bridge.ReadableType,
then guard by verifying args != null && args.size() > 0 && args.getType(0) ==
ReadableType.Number before calling getInt(0), and if the checks fail log a
warning with context and return to avoid the crash.
| android.util.Log.d("SelfOCRViewManager", "Starting fragment transaction") | ||
|
|
||
| // Post to ensure activity is fully ready | ||
| activity.window.decorView.post { | ||
| try { | ||
| if (!activity.isFinishing && !activity.isDestroyed) { | ||
| activity.supportFragmentManager | ||
| .beginTransaction() | ||
| .replace(reactNativeViewId, cameraFragment, reactNativeViewId.toString()) | ||
| .commitNow() | ||
| } else { | ||
| android.util.Log.e("SelfOCRViewManager", "Activity no longer valid for fragment transaction") | ||
| } | ||
| } catch (e: Exception) { | ||
| android.util.Log.e("SelfOCRViewManager", "Fragment transaction failed", e) | ||
| } | ||
| } |
There was a problem hiding this comment.
Avoid commitNow after state save; check isStateSaved and use reordering
commitNow() can throw IllegalStateException if state is saved. Check FragmentManager.isStateSaved and prefer commit() with setReorderingAllowed(true).
- activity.window.decorView.post {
- try {
- if (!activity.isFinishing && !activity.isDestroyed) {
- activity.supportFragmentManager
- .beginTransaction()
- .replace(reactNativeViewId, cameraFragment, reactNativeViewId.toString())
- .commitNow()
- } else {
- android.util.Log.e("SelfOCRViewManager", "Activity no longer valid for fragment transaction")
- }
- } catch (e: Exception) {
- android.util.Log.e("SelfOCRViewManager", "Fragment transaction failed", e)
- }
- }
+ activity.window.decorView.post {
+ try {
+ if (!activity.isFinishing && !activity.isDestroyed) {
+ val fm = activity.supportFragmentManager
+ if (!fm.isStateSaved) {
+ fm.beginTransaction()
+ .setReorderingAllowed(true)
+ .replace(reactNativeViewId, cameraFragment, reactNativeViewId.toString())
+ .commit()
+ } else {
+ android.util.Log.w("SelfOCRViewManager", "FragmentManager state saved; aborting create for $reactNativeViewId")
+ }
+ } else {
+ android.util.Log.e("SelfOCRViewManager", "Activity no longer valid for fragment transaction")
+ }
+ } catch (e: Exception) {
+ android.util.Log.e("SelfOCRViewManager", "Fragment transaction failed", e)
+ }
+ }🤖 Prompt for AI Agents
In
packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfOCRViewManager.kt
around lines 132 to 148, the code uses supportFragmentManager.commitNow which
can throw if the FragmentManager state is saved; instead check
activity.supportFragmentManager.isStateSaved before committing and if state is
saved skip or defer the transaction, otherwise perform a normal asynchronous
transaction using
beginTransaction().setReorderingAllowed(true).replace(...).commit() (not
commitNow) so the transaction is safe across saved state and respects
reordering.
| class SelfQRScannerModule(reactContext: ReactApplicationContext) : | ||
| ReactContextBaseJavaModule(reactContext), QrCodeScannerFragment.QRCodeScannerCallback { | ||
| override fun getName() = "SelfQRScannerModule" |
There was a problem hiding this comment.
Hook into RN lifecycle to auto-clean overlays on background/destroy.
Prevents stuck overlays when the host activity is destroyed or app backgrounds mid-scan.
-class SelfQRScannerModule(reactContext: ReactApplicationContext) :
-ReactContextBaseJavaModule(reactContext), QrCodeScannerFragment.QRCodeScannerCallback {
+class SelfQRScannerModule(reactContext: ReactApplicationContext) :
+ ReactContextBaseJavaModule(reactContext),
+ QrCodeScannerFragment.QRCodeScannerCallback,
+ com.facebook.react.bridge.LifecycleEventListener {
+ init { reactContext.addLifecycleEventListener(this) }Add near the end (new code):
override fun onCatalystInstanceDestroy() {
reactApplicationContext.removeLifecycleEventListener(this)
cleanup()
}
override fun onHostResume() {}
override fun onHostPause() { /* optionally pause camera */ }
override fun onHostDestroy() { cleanup() }Also applies to: 121-121
🤖 Prompt for AI Agents
In
packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfQRScannerModule.kt
around lines 19-21 (and also apply the change near line 121), the module doesn't
hook into React Native lifecycle so overlays can remain when the host activity
is destroyed or app backgrounds; register the module as a LifecycleEventListener
in the constructor (or onCreate equivalent) and implement onHostResume,
onHostPause, and onHostDestroy to manage camera/scan state, and override
onCatalystInstanceDestroy to remove the lifecycle listener and call cleanup();
ensure onHostDestroy also calls cleanup() and onHostPause optionally pauses the
camera to avoid stuck overlays.
packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfQRScannerModule.kt
Show resolved
Hide resolved
packages/mobile-sdk-alpha/android/src/main/java/com/selfxyz/selfSDK/SelfQRScannerModule.kt
Show resolved
Hide resolved
seshanthS
temporarily deployed
to
development
GitHub Actions
Inactive
seshanthS
temporarily deployed
to
development
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
app/ios/Self.xcodeproj/project.pbxproj (1)
770-770: Update NSCameraUsageDescription to include QR scanning (App Store compliance)Both occurrences still mention only passport MRZ — update to reference QR codes as well to avoid App Store review rejection. Locations: app/ios/Self.xcodeproj/project.pbxproj — lines 770, 864.
- INFOPLIST_KEY_NSCameraUsageDescription = "Needed to scan your passport MRZ, you can however enter it manually."; + INFOPLIST_KEY_NSCameraUsageDescription = "Needed to scan QR codes and your passport MRZ. You can also enter details manually.";common/src/utils/aadhaar/mockData.ts (1)
64-75: Normalize and bound field lengths before packing to avoid commitment drift.State/name aren’t truncated and pincode/phone aren’t validated. Overlong or non‑canonical values will change the commitment shape and can break inclusion proofs across platforms/environments.
Apply this diff to enforce canonical sizes and numeric constraints:
export function computePackedCommitment( extractedFields: ReturnType<typeof extractQRDataFields> ): bigint { - const packedCommitmentArgs = [ - 3, - ...stringToAsciiArray(extractedFields.pincode), - ...stringToAsciiArray(extractedFields.state.padEnd(31, '\0')), - ...stringToAsciiArray(extractedFields.phoneNoLast4Digits), - ...stringToAsciiArray(extractedFields.name.padEnd(62, '\0')), - ]; + const stateNormalized = extractedFields.state.trim().slice(0, 31).padEnd(31, '\0'); + const nameNormalized = extractedFields.name.trim().slice(0, 62).padEnd(62, '\0'); + const pincodeNormalized = extractedFields.pincode.replace(/\D/g, '').slice(0, 6).padStart(6, '0'); + const phoneNormalized = extractedFields.phoneNoLast4Digits + .replace(/\D/g, '') + .slice(-4) + .padStart(4, '0'); + const packedCommitmentArgs = [ + 3, + ...stringToAsciiArray(pincodeNormalized), + ...stringToAsciiArray(stateNormalized), + ...stringToAsciiArray(phoneNormalized), + ...stringToAsciiArray(nameNormalized), + ]; return BigInt(packBytesAndPoseidon(packedCommitmentArgs)); }common/src/utils/circuits/registerInputs.ts (2)
93-101: Remove sensitive logging (secrets and PII).Logs include secret and Aadhaar payload; this violates our logging policy and risks leakage.
Apply this diff to drop the logs:
- console.log( - 'publicKeys-aadhaar', - publicKeys, - 'secret-aadhaar', - secret, - 'aadhaarData-aadhaar', - aadhaarData - );Optionally replace with a redacted/debug-safe line:
+ // logger.debug('generateTEEInputsAadhaarRegister:start', { publicKeysCount: publicKeys?.length ?? 0 });
252-256: Remove sensitive console logs in production code (common/src/utils/circuits/registerInputs.ts:252-256).inputs may contain MRZ/PII — do not print; replace with a non-PII debug marker.
- console.log('inputs-aadhaar', inputs); - console.log('circuitName-aadhaar', circuitName); - console.log('endpointType-aadhaar', endpointType); - console.log('endpoint-aadhaar', endpoint); + // logger.debug('generateTEEInputsRegister:aadhaar', { hasInputs: Boolean(inputs), circuitName, endpointType });common/src/utils/circuits/generateInputs.ts (2)
218-234: Incorrect keys used for padding limits; can cause mis-padding and circuit failures.
- eContent length check indexed by signatureAlgorithmFullName (likely not a hash algo key).
- eContent max length keyed by dg1HashFunction instead of eContentHashFunction.
- signedAttr padding max keyed by eContentHashFunction instead of signedAttrHashFunction.
Use the hash function actually applied to each blob.
Apply this diff:
- if (eContent.length > MAX_PADDED_ECONTENT_LEN[signatureAlgorithmFullName]) { + if (eContent.length > MAX_PADDED_ECONTENT_LEN[passportMetadata.eContentHashFunction]) { @@ - const [eContentPadded, eContentLen] = pad(passportMetadata.eContentHashFunction)( - eContent, - MAX_PADDED_ECONTENT_LEN[passportMetadata.dg1HashFunction] - ); + const [eContentPadded, eContentLen] = pad(passportMetadata.eContentHashFunction)( + eContent, + MAX_PADDED_ECONTENT_LEN[passportMetadata.eContentHashFunction] + ); @@ - const [signedAttrPadded, signedAttrPaddedLen] = pad(passportMetadata.signedAttrHashFunction)( - signedAttr, - MAX_PADDED_SIGNED_ATTR_LEN[passportMetadata.eContentHashFunction] - ); + const [signedAttrPadded, signedAttrPaddedLen] = pad(passportMetadata.signedAttrHashFunction)( + signedAttr, + MAX_PADDED_SIGNED_ATTR_LEN[passportMetadata.signedAttrHashFunction] + );
299-306: Test variant uses mismatched constants too. Mirror the same fixes.Apply this diff:
- if (eContent.length > MAX_PADDED_ECONTENT_LEN[signatureAlgorithmFullName]) { + if (eContent.length > MAX_PADDED_ECONTENT_LEN[passportMetadata.eContentHashFunction]) { @@ - const [eContentPadded, eContentLen] = pad(passportMetadata.eContentHashFunction)( - eContent, - MAX_PADDED_ECONTENT_LEN[passportMetadata.dg1HashFunction] - ); + const [eContentPadded, eContentLen] = pad(passportMetadata.eContentHashFunction)( + eContent, + MAX_PADDED_ECONTENT_LEN[passportMetadata.eContentHashFunction] + ); @@ - const [signedAttrPadded, signedAttrPaddedLen] = pad(passportMetadata.signedAttrHashFunction)( - signedAttr, - MAX_PADDED_SIGNED_ATTR_LEN_FOR_TESTS[passportMetadata.eContentHashFunction] - ); + const [signedAttrPadded, signedAttrPaddedLen] = pad(passportMetadata.signedAttrHashFunction)( + signedAttr, + MAX_PADDED_SIGNED_ATTR_LEN_FOR_TESTS[passportMetadata.signedAttrHashFunction] + );
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
app/ios/Podfile.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
app/ios/Self.xcodeproj/project.pbxproj(2 hunks)common/index.ts(0 hunks)common/src/constants/constants.ts(1 hunks)common/src/utils/aadhaar/mockData.ts(3 hunks)common/src/utils/circuits/generateInputs.ts(4 hunks)common/src/utils/circuits/registerInputs.ts(1 hunks)common/src/utils/index.ts(1 hunks)common/src/utils/passports/genMockIdDoc.ts(1 hunks)
💤 Files with no reviewable changes (1)
- common/index.ts
✅ Files skipped from review due to trivial changes (2)
- common/src/utils/index.ts
- common/src/utils/passports/genMockIdDoc.ts
🧰 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.,***-***-1234for passport numbers,J*** D***for names).
Files:
common/src/constants/constants.tscommon/src/utils/circuits/registerInputs.tscommon/src/utils/aadhaar/mockData.tscommon/src/utils/circuits/generateInputs.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/constants/constants.tscommon/src/utils/circuits/registerInputs.tscommon/src/utils/aadhaar/mockData.tscommon/src/utils/circuits/generateInputs.ts
app/ios/**/*
⚙️ CodeRabbit configuration file
app/ios/**/*: Review iOS-specific code for:
- Platform-specific implementations
- Performance considerations
- Security best practices for mobile
Files:
app/ios/Self.xcodeproj/project.pbxproj
🧠 Learnings (3)
📓 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/src/processing/** : Place MRZ processing helpers in packages/mobile-sdk-alpha/src/processing/
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)
📚 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:
common/src/utils/circuits/registerInputs.tscommon/src/utils/circuits/generateInputs.ts
📚 Learning: 2025-09-22T11:10:57.852Z
Learnt from: CR
PR: selfxyz/self#0
File: app/AGENTS.md:0-0
Timestamp: 2025-09-22T11:10:57.852Z
Learning: Applies to app/ios/**/*.{m,mm,swift} : Document complex native iOS module changes in the PR
Applied to files:
app/ios/Self.xcodeproj/project.pbxproj
🧬 Code graph analysis (2)
common/src/utils/aadhaar/mockData.ts (3)
common/src/utils/shaPad.ts (1)
shaPad(86-105)common/src/utils/aadhaar/utils.ts (1)
extractQRDataFields(245-326)common/src/utils/hash.ts (1)
packBytesAndPoseidon(185-188)
common/src/utils/circuits/generateInputs.ts (4)
common/src/utils/circuits/registerInputs.ts (1)
generateCircuitInputsRegister(27-27)common/src/utils/index.ts (2)
generateCircuitInputsRegister(57-57)generateCircuitInputsRegisterForTests(58-58)common/index.ts (2)
generateCircuitInputsRegister(68-68)generateCircuitInputsRegisterForTests(69-69)common/src/constants/constants.ts (2)
MAX_PADDED_SIGNED_ATTR_LEN(139-145)MAX_PADDED_SIGNED_ATTR_LEN_FOR_TESTS(147-153)
⏰ 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). (7)
- GitHub Check: test
- GitHub Check: build-android
- GitHub Check: build-ios
- GitHub Check: analyze-ios
- GitHub Check: analyze-android
- GitHub Check: e2e-ios
- GitHub Check: android-build-test
🔇 Additional comments (7)
app/ios/Self.xcodeproj/project.pbxproj (1)
792-795: No stray blank linker flag in OTHER_LDFLAGS — no action required.Checked app/ios/Self.xcodeproj/project.pbxproj with the supplied script; output: OK: no blank OTHER_LDFLAGS entries found.
Likely an incorrect or invalid review comment.
common/src/constants/constants.ts (2)
139-146: Rename + dual-constant exposure looks good.No behavioral risk detected here.
147-154: Test mirror constant OK.Keeps tests decoupled from prod constant; no concerns.
common/src/utils/aadhaar/mockData.ts (3)
52-61: Poseidon commitment wrapper is correct and side-effect free.
77-83: Byte-array→BigInt conversion is correct and safe for arbitrary lengths.
560-595: QR decode/pad/hash pipeline reads cleanly and guards for missing JPEG EOI.All good here.
common/src/utils/circuits/registerInputs.ts (1)
22-22: Static imports over runtime require: good move for bundlers and type safety.
| OTHER_LDFLAGS = ( | ||
| "$(inherited)", | ||
| " ", | ||
| ); |
There was a problem hiding this comment.
Repeat: remove blank linker flag (project Release).
Same issue as Debug; remove the " " entry to avoid passing an empty arg to the linker.
- OTHER_LDFLAGS = (
- "$(inherited)",
- " ",
- );
+ OTHER_LDFLAGS = (
+ "$(inherited)",
+ );📝 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.
| OTHER_LDFLAGS = ( | |
| "$(inherited)", | |
| " ", | |
| ); | |
| OTHER_LDFLAGS = ( | |
| "$(inherited)", | |
| ); |
🤖 Prompt for AI Agents
In app/ios/Self.xcodeproj/project.pbxproj around lines 885 to 888, the Release
build’s OTHER_LDFLAGS array contains a spurious " " entry that passes an empty
argument to the linker; remove the " " element from the array so OTHER_LDFLAGS
only includes "$(inherited)" (and ensure commas/parentheses remain valid), then
save the project file so the Release configuration no longer sends a blank
linker flag.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores