Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThe PR modularizes Android native code for react-native-biolink, adds JNI/C++ and Kotlin support modules, updates build configs and manifests, removes an old monolithic Kotlin file, adjusts demo app UI and deps, revises READMEs and project scripts, and updates various ignore/format configs and CI workflow. Changes
Sequence Diagram(s)sequenceDiagram
participant JS as JavaScript (React Native)
participant RNPkg as BiolinkCoreBridgeModule
participant Native as HybridBiolinkCore (Kotlin)
participant Provider as ActivityProvider / BiometricPromptManager
participant Android as Android System
JS->>RNPkg: authenticate(fallback)
RNPkg->>Native: authenticate(fallback)
Native->>Provider: awaitActivity() / getBiometricPromptManager()
Provider->>Android: show BiometricPrompt
Android-->>Provider: auth result
Provider-->>Native: result
Native-->>RNPkg: resolve/reject Promise
RNPkg-->>JS: Promise result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
gmemmy
force-pushed
the
develop
branch
2 times, most recently
from
0dbfa44 to
fef3878
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (7)
config/eslint.config.js (1)
62-77: Ignore patterns are redundant & still miss nested package-level Android dirs
'android/'already excludes the top-level folder, so adding the sub-paths is dead code.
Conversely, nested instances such aspackages/*/android/**(where the new native code lives) are still not ignored. Prefer a single glob that captures all Android artefacts:- 'android/', - 'android/build/', - 'android/.cxx/', - 'android/src/main/AndroidManifest.xml', - 'android/src/main/cpp/', + '**/android/**',This keeps the list short and future-proof.
packages/react-native-biolink/.gitignore (1)
17-18: Minor pattern issues – trim whitespace & match nested.cxx
- Trailing space after
*~can be mis-read by Git (pattern becomes"*~ ")./.cxx/only hits repo-root; the Gradle build creates the dir under each module.-*~ -/.cxx/ +*~ +**/.cxx/Helps avoid accidental commits of native build artefacts.
packages/react-native-biolink/android/src/main/kotlin/com/margelo/nitro/biolink/utils/BiometricPromptManager.kt (1)
43-53: Remove redundant context switching.Using
withContext(Dispatchers.Main)combined withHandler.postis redundant sincewithContext(Dispatchers.Main)already switches to the main thread.Apply this diff to simplify the context switching:
- withContext(Dispatchers.Main) { - Handler(activity.mainLooper).post { - try { - biometricPrompt.authenticate(promptInfo) - } catch (e: Exception) { - Log.e(TAG, "Error showing biometric prompt: ${e.message}", e) - deferredResult?.completeExceptionally(RuntimeException("PROMPT_ERROR: ${e.message}", e)) - deferredResult = null - } - } - } + withContext(Dispatchers.Main) { + try { + biometricPrompt.authenticate(promptInfo) + } catch (e: Exception) { + Log.e(TAG, "Error showing biometric prompt: ${e.message}", e) + deferredResult?.completeExceptionally(RuntimeException("PROMPT_ERROR: ${e.message}", e)) + deferredResult = null + } + }packages/react-native-biolink/android/build.gradle (1)
139-140: Consider updating Kotlin coroutines version.The Kotlin coroutines version (1.6.4) is relatively old. Consider updating to a more recent stable version for performance improvements and bug fixes.
Check for the latest stable version:
What is the latest stable version of Kotlin coroutines (kotlinx-coroutines-core and kotlinx-coroutines-android)?packages/react-native-biolink/android/src/main/kotlin/com/margelo/nitro/biolink/utils/ActivityProvider.kt (1)
34-43: Simplify null checks with early returnThe nested if statements can be simplified for better readability.
fun getBiometricPromptManager(): BiometricPromptManager? { val activity = _currentActivity - if (activity is FragmentActivity) { - if (_biometricPromptManager == null || _biometricPromptManager?.activity != activity) { - _biometricPromptManager = BiometricPromptManager(activity, ContextCompat.getMainExecutor(activity)) - } - return _biometricPromptManager - } - return null + if (activity !is FragmentActivity) return null + + if (_biometricPromptManager == null || _biometricPromptManager?.activity != activity) { + _biometricPromptManager = BiometricPromptManager(activity, ContextCompat.getMainExecutor(activity)) + } + return _biometricPromptManager }packages/react-native-biolink/android/src/main/kotlin/com/margelo/nitro/biolink/HybridBiolinkCore.kt (2)
56-58: Remove redundant activity type check
ActivityProvider.awaitActivity()already returns aFragmentActivity, making this check unnecessary.- if (activity !is FragmentActivity) { - throw RuntimeException("INVALID_ACTIVITY_TYPE: Current activity is not a FragmentActivity. BiometricPrompt requires a FragmentActivity.") - } + // awaitActivity() already ensures FragmentActivity type
220-222: Remove redundant activity type checkSame as in
authenticate(), this check is unnecessary.- if (activity !is FragmentActivity) { - throw RuntimeException("INVALID_ACTIVITY_TYPE: Current activity is not a FragmentActivity. BiometricPrompt requires a FragmentActivity.") - } + // awaitActivity() already ensures FragmentActivity type
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (36)
biolink-demo/ios/Podfile.lockis excluded by!**/*.lockpackages/react-native-biolink/nitrogen/generated/android/BiolinkCoreOnLoad.cppis excluded by!**/generated/**packages/react-native-biolink/nitrogen/generated/android/BiolinkCoreOnLoad.hppis excluded by!**/generated/**packages/react-native-biolink/nitrogen/generated/android/c++/JBiometryType.hppis excluded by!**/generated/**packages/react-native-biolink/nitrogen/generated/android/c++/JHybridBiolinkCoreSpec.cppis excluded by!**/generated/**packages/react-native-biolink/nitrogen/generated/android/c++/JHybridBiolinkCoreSpec.hppis excluded by!**/generated/**packages/react-native-biolink/nitrogen/generated/android/c++/JSensorAvailability.hppis excluded by!**/generated/**packages/react-native-biolink/nitrogen/generated/android/c++/JSimplePromptOptions.hppis excluded by!**/generated/**packages/react-native-biolink/nitrogen/generated/android/kotlin/com/margelo/nitro/biolink/BiolinkCoreOnLoad.ktis excluded by!**/generated/**packages/react-native-biolink/nitrogen/generated/android/kotlin/com/margelo/nitro/biolink/BiometryType.ktis excluded by!**/generated/**packages/react-native-biolink/nitrogen/generated/android/kotlin/com/margelo/nitro/biolink/HybridBiolinkCoreSpec.ktis excluded by!**/generated/**packages/react-native-biolink/nitrogen/generated/android/kotlin/com/margelo/nitro/biolink/SensorAvailability.ktis excluded by!**/generated/**packages/react-native-biolink/nitrogen/generated/android/kotlin/com/margelo/nitro/biolink/SimplePromptOptions.ktis excluded by!**/generated/**packages/react-native-biolink/nitrogen/generated/ios/BiolinkCore-Swift-Cxx-Bridge.cppis excluded by!**/generated/**packages/react-native-biolink/nitrogen/generated/ios/BiolinkCore-Swift-Cxx-Bridge.hppis excluded by!**/generated/**packages/react-native-biolink/nitrogen/generated/ios/BiolinkCore-Swift-Cxx-Umbrella.hppis excluded by!**/generated/**packages/react-native-biolink/nitrogen/generated/ios/BiolinkCoreAutolinking.mmis excluded by!**/generated/**packages/react-native-biolink/nitrogen/generated/ios/BiolinkCoreAutolinking.swiftis excluded by!**/generated/**packages/react-native-biolink/nitrogen/generated/ios/c++/HybridBiolinkCoreSpecSwift.cppis excluded by!**/generated/**packages/react-native-biolink/nitrogen/generated/ios/c++/HybridBiolinkCoreSpecSwift.hppis excluded by!**/generated/**packages/react-native-biolink/nitrogen/generated/ios/swift/BiometryType.swiftis excluded by!**/generated/**packages/react-native-biolink/nitrogen/generated/ios/swift/Func_void.swiftis excluded by!**/generated/**packages/react-native-biolink/nitrogen/generated/ios/swift/Func_void_SensorAvailability.swiftis excluded by!**/generated/**packages/react-native-biolink/nitrogen/generated/ios/swift/Func_void_bool.swiftis excluded by!**/generated/**packages/react-native-biolink/nitrogen/generated/ios/swift/Func_void_std__exception_ptr.swiftis excluded by!**/generated/**packages/react-native-biolink/nitrogen/generated/ios/swift/Func_void_std__optional_std__string_.swiftis excluded by!**/generated/**packages/react-native-biolink/nitrogen/generated/ios/swift/Func_void_std__string.swiftis excluded by!**/generated/**packages/react-native-biolink/nitrogen/generated/ios/swift/HybridBiolinkCoreSpec_cxx.swiftis excluded by!**/generated/**packages/react-native-biolink/nitrogen/generated/ios/swift/SensorAvailability.swiftis excluded by!**/generated/**packages/react-native-biolink/nitrogen/generated/ios/swift/SimplePromptOptions.swiftis excluded by!**/generated/**packages/react-native-biolink/nitrogen/generated/shared/c++/BiometryType.hppis excluded by!**/generated/**packages/react-native-biolink/nitrogen/generated/shared/c++/HybridBiolinkCoreSpec.cppis excluded by!**/generated/**packages/react-native-biolink/nitrogen/generated/shared/c++/HybridBiolinkCoreSpec.hppis excluded by!**/generated/**packages/react-native-biolink/nitrogen/generated/shared/c++/SensorAvailability.hppis excluded by!**/generated/**packages/react-native-biolink/nitrogen/generated/shared/c++/SimplePromptOptions.hppis excluded by!**/generated/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (31)
.github/workflows/ci.yml(0 hunks)README.md(2 hunks)biolink-demo/App.tsx(2 hunks)biolink-demo/README.md(2 hunks)biolink-demo/android/app/src/main/java/com/biolinkdemo/MainApplication.kt(2 hunks)biolink-demo/android/build.gradle(1 hunks)biolink-demo/ios/BiolinkDemo.xcodeproj/project.pbxproj(4 hunks)biolink-demo/package.json(1 hunks)biolink-demo/src/screens/SecureVault/SecureVaultScreen.tsx(2 hunks)config/eslint.config.js(1 hunks)package.json(1 hunks)packages/react-native-biolink/.gitignore(1 hunks)packages/react-native-biolink/.prettierignore(1 hunks)packages/react-native-biolink/README.md(9 hunks)packages/react-native-biolink/android/HybridBiolinkCore.kt(0 hunks)packages/react-native-biolink/android/build.gradle(1 hunks)packages/react-native-biolink/android/gradle.properties(1 hunks)packages/react-native-biolink/android/src/main/AndroidManifest.xml(1 hunks)packages/react-native-biolink/android/src/main/cpp/cpp-adapter.cpp(1 hunks)packages/react-native-biolink/android/src/main/kotlin/com/margelo/nitro/biolink/BiolinkActivityLifecycleCallbacks.kt(1 hunks)packages/react-native-biolink/android/src/main/kotlin/com/margelo/nitro/biolink/BiolinkCoreBridgeModule.kt(1 hunks)packages/react-native-biolink/android/src/main/kotlin/com/margelo/nitro/biolink/BiolinkCorePackage.kt(1 hunks)packages/react-native-biolink/android/src/main/kotlin/com/margelo/nitro/biolink/HybridBiolinkCore.kt(1 hunks)packages/react-native-biolink/android/src/main/kotlin/com/margelo/nitro/biolink/utils/ActivityProvider.kt(1 hunks)packages/react-native-biolink/android/src/main/kotlin/com/margelo/nitro/biolink/utils/BiolinkSharedPrefs.kt(1 hunks)packages/react-native-biolink/android/src/main/kotlin/com/margelo/nitro/biolink/utils/BiometricPromptManager.kt(1 hunks)packages/react-native-biolink/android/src/main/kotlin/com/margelo/nitro/biolink/utils/ContextProvider.kt(1 hunks)packages/react-native-biolink/jest.config.js(1 hunks)packages/react-native-biolink/nitro.json(1 hunks)packages/react-native-biolink/package.json(2 hunks)packages/react-native-biolink/react-native.config.js(1 hunks)
💤 Files with no reviewable changes (2)
- .github/workflows/ci.yml
- packages/react-native-biolink/android/HybridBiolinkCore.kt
🧰 Additional context used
🧬 Code Graph Analysis (3)
packages/react-native-biolink/android/src/main/cpp/cpp-adapter.cpp (2)
packages/react-native-biolink/nitrogen/generated/android/BiolinkCoreOnLoad.hpp (1)
initialize(23-23)packages/react-native-biolink/nitrogen/generated/android/BiolinkCoreOnLoad.cpp (2)
initialize(24-44)initialize(24-24)
packages/react-native-biolink/android/src/main/kotlin/com/margelo/nitro/biolink/HybridBiolinkCore.kt (1)
packages/react-native-biolink/nitrogen/generated/shared/c++/SensorAvailability.hpp (3)
SensorAvailability(37-37)SensorAvailability(38-38)SensorAvailability(38-38)
packages/react-native-biolink/android/src/main/kotlin/com/margelo/nitro/biolink/BiolinkCoreBridgeModule.kt (1)
packages/react-native-biolink/nitrogen/generated/shared/c++/SimplePromptOptions.hpp (3)
SimplePromptOptions(37-37)SimplePromptOptions(38-38)SimplePromptOptions(38-38)
🪛 LanguageTool
packages/react-native-biolink/README.md
[style] ~91-~91: As an alternative to the over-used intensifier ‘extremely’, consider replacing this phrase.
Context: ...p stored secrets concise. Avoid storing extremely large data blobs. ```typescript await storeS...
(EN_WEAK_ADJECTIVE)
🔇 Additional comments (41)
packages/react-native-biolink/android/src/main/AndroidManifest.xml (1)
1-3: Manifest lacks apackageattribute – build may failEven for library modules the AGP task
processDebugManifestexpects apackagename, otherwise you’ll see an error like “Manifest merger failed : Attribute application@appComponentFactory …”.-<manifest xmlns:android="http://schemas.android.com/apk/res/android"> +<manifest xmlns:android="http://schemas.android.com/apk/res/android" + package="biolink">Please align the value with the namespace in
nitro.json/ Kotlin sources.packages/react-native-biolink/jest.config.js (1)
1-1: CommonJS export looks correct – no action neededbiolink-demo/android/build.gradle (1)
3-6: Double-check Android SDK downgrade for compatibilityThe SDK versions in
biolink-demo/android/build.gradle(lines 3–6) have been downgraded from 36 to 35:buildToolsVersion = "35.0.0" compileSdkVersion = 35 targetSdkVersion = 35No references to SDK 35 were found in internal docs—please manually confirm:
- React Native 0.80.1 officially supports Android SDK 35
- No critical security patches or new APIs from SDK 36 are required by our dependencies
- Native biometric (and other) modules have been tested against SDK 35
biolink-demo/README.md (2)
7-7: LGTM: Documentation updated to reflect current implementation.The change from mentioning a specific function to describing the feature generically is appropriate and provides better flexibility for API evolution.
28-28: LGTM: Version update aligns with dependency changes.The React Native version update from "0.81.0-rc.0" to "0.80.1" correctly reflects the downgrade mentioned in the package.json changes.
packages/react-native-biolink/android/src/main/kotlin/com/margelo/nitro/biolink/utils/ContextProvider.kt (1)
5-7: ContextProvider.context Initialization VerifiedContextProvider.context is set in
BiolinkCorePackage.createNativeModulesbefore any module methods run, so the lateinit property will always be initialized:
- File: packages/react-native-biolink/android/src/main/kotlin/com/margelo/nitro/biolink/BiolinkCorePackage.kt
Line 22:ContextProvider.context = reactContextReact Native calls
createNativeModulesduring package registration, ensuringcontextis initialized prior to any usage inHybridBiolinkCoreor elsewhere. No changes are required. Optionally, you may add documentation or anisInitializedcheck inContextProviderfor extra safety.packages/react-native-biolink/.prettierignore (1)
1-6: LGTM: Appropriate exclusions for native module build artifacts.The ignore patterns correctly exclude build directories, generated files, and native source files that shouldn't be formatted by Prettier. This is a standard configuration for React Native native modules with Android support.
packages/react-native-biolink/package.json (1)
8-8: LGTM: More explicit TypeScript build configuration.The change from
tsc -btotsc --project tsconfig.jsonmakes the build configuration more explicit and predictable.biolink-demo/ios/BiolinkDemo.xcodeproj/project.pbxproj (2)
381-384: LGTM: Standardized linker flags configuration.The addition of
OTHER_LDFLAGSwith$(inherited)and a blank string entry is a common pattern for ensuring proper flag inheritance from CocoaPods and other dependencies.Also applies to: 453-456
361-361: Confirm iOS deployment target bump to 16.6 is intentionalBased on your repository search, the only setting of IPHONEOS_DEPLOYMENT_TARGET is in
• biolink-demo/ios/BiolinkDemo.xcodeproj/project.pbxproj (lines 361 & 434)Raising the minimum from 15.1 to 16.6 will exclude all devices on iOS 15.x and below. Verify that this aligns with your target audience, business requirements, and any biometric-API dependencies. If you still need to support iOS 15.x, consider lowering the deployment target or adding availability checks; otherwise, update your documentation and CI workflows to reflect the new minimum.
biolink-demo/android/app/src/main/java/com/biolinkdemo/MainApplication.kt (2)
12-12: LGTM: Proper native module import.The import for
BiolinkCorePackageis correctly added and follows Kotlin import conventions.
22-22: LGTM: Correct React Native package registration.The manual addition of
BiolinkCorePackage()to the packages list is the correct approach for integrating native modules that aren't auto-linked. This properly registers the biometric authentication module with React Native.packages/react-native-biolink/react-native.config.js (1)
1-16: LGTM: Well-structured React Native CLI configuration.This configuration file follows React Native CLI best practices:
- Proper TypeScript type annotations for IDE support
- Empty platform configurations allow for auto-detection
- Documentation reference provides helpful context for maintainers
- Standard structure aligns with community conventions
README.md (3)
9-10: LGTM: Improved documentation navigation.The added sentence clearly guides users to the detailed API documentation while explaining the scope of the main README. This improves user experience and documentation organization.
15-16: LGTM: Clear dependency requirements.Explicitly mentioning the
react-native-nitro-modulesrequirement helps users understand the complete installation needs for JSI-based communication.
51-51: LGTM: Consistent documentation naming.The rename to "Package API Reference" provides clearer and more consistent naming across the documentation references.
packages/react-native-biolink/android/src/main/cpp/cpp-adapter.cpp (1)
1-6: LGTM! Clean JNI adapter implementation.The JNI_OnLoad function follows the standard pattern for native library initialization. The delegation to
margelo::nitro::biolink::initialize(vm)is appropriate and matches the existing framework integration shown in the relevant code snippets.package.json (3)
16-16: Good addition of comprehensive CI script.The new CI script properly sequences build, format checking, and testing, providing a complete validation pipeline.
18-18: Useful git cleanup script addition.The
clean:gitscript usinggit clean -fdXwill help developers clean untracked files and directories, which is particularly useful during development and testing.
21-23: Path simplification aligns with project restructuring.The removal of the "packages/" prefix from demo script paths reflects the project restructuring and simplifies the command paths.
biolink-demo/src/screens/SecureVault/SecureVaultScreen.tsx (1)
315-336: Effective replacement of BlurView with semi-transparent overlay.The replacement of
BlurViewwith a plainViewusing conditional semi-transparent backgrounds maintains the visual indication during authentication while removing the native blur dependency. The dark/light mode adaptation is well implemented.biolink-demo/App.tsx (1)
36-42: Consistent blur replacement maintains tab bar design.The replacement of blur effects with semi-transparent backgrounds and proper border styling maintains the intended visual design while eliminating the blur dependency. The conditional dark/light mode styling is well implemented.
biolink-demo/package.json (3)
7-7: Good practice: Explicit simulator targeting for iOS builds.Specifying the exact simulator model ensures consistent testing environments across the development team.
9-9: Improved development experience with cache reset and client logs.These flags help with debugging and ensure clean builds during development.
16-17: React Native 0.80.1 Compatibility ValidatedI searched for any 0.81-specific APIs (TurboModule, Fabric, newArch) and found none, and all CocoaPods are pinned to 0.80.1. The downgrade of the two React Native dependencies is safe:
• biolink-demo/package.json (lines 16–17):
- "@react-native/gradle-plugin": "0.80.1"
- "@react-native/codegen": "0.80.1"
No further 0.81.x–only features are used.
packages/react-native-biolink/android/src/main/kotlin/com/margelo/nitro/biolink/utils/BiolinkSharedPrefs.kt (3)
19-21: Strong encryption configuration for secure storage.The use of AES256_GCM for the master key scheme provides robust encryption for the secure storage implementation.
23-29: Appropriate encryption schemes for keys and values.Using AES256_SIV for key encryption and AES256_GCM for value encryption follows Android security best practices for encrypted shared preferences.
32-35: Minimum SDK Requirement MetThe project’s
minSdkVersionis configured as 23 (viaBiolinkCore_minSdkVersion=23ingradle.properties, anddefaultConfig { minSdkVersion getExtOrIntegerDefault("minSdkVersion") }inpackages/react-native-biolink/android/build.gradle), which satisfies the API 23+ requirement forEncryptedSharedPreferences. No changes needed.packages/react-native-biolink/android/src/main/kotlin/com/margelo/nitro/biolink/BiolinkCorePackage.kt (2)
16-18: Good practice: Native library initialization in static block.Initializing the native library in the companion object's static block ensures it's loaded once when the class is first referenced.
23-23: Add null and type safety for activity casting.The unsafe cast to
FragmentActivitycould throw aClassCastExceptionif the current activity is null or not aFragmentActivity.Apply this diff to add proper null and type checking:
- ActivityProvider.currentActivity = reactContext.currentActivity as? FragmentActivity + val currentActivity = reactContext.currentActivity + if (currentActivity is FragmentActivity) { + ActivityProvider.currentActivity = currentActivity + } else { + Log.w("BiolinkCorePackage", "Current activity is not a FragmentActivity, biometric features may not work properly") + ActivityProvider.currentActivity = null + }You'll also need to add the Log import:
+import android.util.LogLikely an incorrect or invalid review comment.
packages/react-native-biolink/android/build.gradle (2)
48-60: Comprehensive NDK configuration with security hardening.The CMake configuration includes appropriate compiler flags for security (
-fstack-protector-all) and proper optimization settings for debug/release builds.
69-88: Appropriate packaging exclusions for React Native.The packaging exclusions correctly prevent conflicts with React Native's bundled native libraries while maintaining the necessary dependencies.
packages/react-native-biolink/android/src/main/kotlin/com/margelo/nitro/biolink/utils/ActivityProvider.kt (1)
1-13: LGTM!The imports and object declaration are appropriate for an activity lifecycle management singleton.
packages/react-native-biolink/android/src/main/kotlin/com/margelo/nitro/biolink/HybridBiolinkCore.kt (2)
1-49: LGTM!Good implementation with lazy initialization and proper error handling for SharedPreferences.
225-225: Inconsistent authenticator configuration
simplePromptalways uses both biometric and device credential, whileauthenticaterespects the fallback parameter. The negative button text is also set differently.Is this intentional? If so, consider documenting why
simplePromptalways allows device credentials.Also applies to: 243-243
packages/react-native-biolink/README.md (3)
7-7: LGTM!Good improvements:
- Clear separation between API reference and project overview
- Correct React Native component imports
- Proper TouchableOpacity usage instead of HTML elements
Also applies to: 32-33, 53-55
64-141: Excellent API documentation improvements!The additions enhance usability:
- Clear TypeScript signatures
- Detailed parameter explanations
- Comprehensive error handling examples
- Well-documented error codes
165-197: LGTM!Excellent additions to the digital signing section:
- Clear explanation of cryptographic signatures
- Detailed method descriptions emphasizing security
- Helpful examples showing return formats
packages/react-native-biolink/android/src/main/kotlin/com/margelo/nitro/biolink/BiolinkCoreBridgeModule.kt (3)
1-31: LGTM!Good implementation with lazy initialization and comprehensive error handling.
32-92: LGTM!Excellent bridge implementation:
- Consistent error handling with descriptive error codes
- Proper Promise resolution/rejection
- Clean conversion of native objects to React Native format
94-106: LGTM!Clean implementation of option mapping from React Native to native SimplePromptOptions.
| implementation "androidx.biometric:biometric:1.1.0" | ||
| implementation "androidx.security:security-crypto:1.1.0-alpha03" | ||
| implementation "org.jetbrains.kotlinx:kotlinx-coroutines-core:1.6.4" | ||
| implementation "org.jetbrains.kotlinx:kotlinx-coroutines-android:1.6.4" | ||
| implementation "androidx.fragment:fragment-ktx:1.6.2" |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify biometric and security library versions.
The AndroidX Biometric (1.1.0) and Security Crypto (1.1.0-alpha03) versions should be verified for stability and security updates.
🌐 Web query:
What are the latest stable versions of androidx.biometric and androidx.security:security-crypto libraries?
💡 Result:
As of July 29, 2025, the latest stable versions of the androidx.biometric and androidx.security:security-crypto libraries are:
-
androidx.biometric: Version 1.1.0, released on May 20, 2025. (developer.android.com)
-
androidx.security:security-crypto: Version 1.0.0, released on July 2, 2025. (developer.android.com)
These versions are the most recent stable releases available for these libraries.
Citations:
- 1: https://developer.android.com/jetpack/androidx/releases/biometric?utm_source=openai
- 2: https://developer.android.com/jetpack/androidx/releases/security?utm_source=openai
Update Security Crypto to Latest Stable Version
The AndroidX Biometric dependency (1.1.0) is already on the latest stable release. However, the Security Crypto library is pinned to an alpha build (1.1.0-alpha03), while the current stable release is 1.0.0. Please update this dependency to ensure stability.
• File: packages/react-native-biolink/android/build.gradle
• Around line 138
Suggested diff:
- implementation "androidx.security:security-crypto:1.1.0-alpha03"
+ implementation "androidx.security:security-crypto:1.0.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.
| implementation "androidx.biometric:biometric:1.1.0" | |
| implementation "androidx.security:security-crypto:1.1.0-alpha03" | |
| implementation "org.jetbrains.kotlinx:kotlinx-coroutines-core:1.6.4" | |
| implementation "org.jetbrains.kotlinx:kotlinx-coroutines-android:1.6.4" | |
| implementation "androidx.fragment:fragment-ktx:1.6.2" | |
| implementation "androidx.biometric:biometric:1.1.0" | |
| - implementation "androidx.security:security-crypto:1.1.0-alpha03" | |
| + implementation "androidx.security:security-crypto:1.0.0" | |
| implementation "org.jetbrains.kotlinx:kotlinx-coroutines-core:1.6.4" | |
| implementation "org.jetbrains.kotlinx:kotlinx-coroutines-android:1.6.4" | |
| implementation "androidx.fragment:fragment-ktx:1.6.2" |
🤖 Prompt for AI Agents
In packages/react-native-biolink/android/build.gradle around line 138, the
androidx.security:security-crypto dependency is set to an alpha version
1.1.0-alpha03. Update this to the latest stable version 1.0.0 by changing the
version string in the implementation line to "1.0.0" to ensure stability.
| BiolinkCore_kotlinVersion=2.0.21 | ||
| BiolinkCore_minSdkVersion=23 | ||
| BiolinkCore_targetSdkVersion=34 | ||
| BiolinkCore_compileSdkVersion=34 | ||
| BiolinkCore_ndkVersion=27.1.12297006 No newline at end of file |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify version compatibility and consider using variables for consistency.
The configuration values look reasonable for a modern React Native project. However, consider the following:
- Kotlin 2.0.21 is relatively recent - ensure compatibility with React Native and Nitro Modules
- NDK version should be verified as valid and compatible with the target React Native version
Consider using Gradle version catalogs or consistent variable naming across your build files for better maintainability.
🏁 Script executed:
#!/bin/bash
# Check if the specified NDK version is valid and available
curl -s "https://developer.android.com/ndk/downloads" | grep -o "27\.1\.12297006" || echo "NDK version not found in official downloads"
# Check Kotlin compatibility with React Native
echo "Verifying Kotlin 2.0.21 compatibility with React Native..."Length of output: 305
🏁 Script executed:
#!/bin/bash
# Verify that Kotlin Gradle Plugin 2.0.21 is published to Maven Central
STATUS=$(curl -s -o /dev/null -w '%{http_code}' \
https://repo1.maven.org/maven2/org/jetbrains/kotlin/kotlin-gradle-plugin/2.0.21/kotlin-gradle-plugin-2.0.21.pom)
if [ "$STATUS" -eq 200 ]; then
echo "Kotlin Gradle Plugin 2.0.21 is available in Maven Central"
else
echo "Kotlin Gradle Plugin 2.0.21 not found in Maven Central (HTTP $STATUS)"
fiLength of output: 313
Confirm NDK Version & Leverage Version Catalogs
It looks like your Kotlin setup is valid—Kotlin Gradle Plugin 2.0.21 is available on Maven Central—but the NDK version you’ve specified isn’t published on the official Android NDK downloads page. Please update to a supported NDK release or verify that you have a custom distribution in your CI.
File: packages/react-native-biolink/android/gradle.properties (lines 1–5)
• NDK
• Current: BiolinkCore_ndkVersion=27.1.12297006
• Issue: not found in Android’s official NDK downloads
• Action: choose a published NDK (e.g. 25.x.x or 26.x.x) or document your custom installation
• Kotlin
• Current: BiolinkCore_kotlinVersion=2.0.21
• Status: confirmed available in Maven Central—no change needed
Recommendation: for consistency and easier upgrades, consider moving these properties into a Gradle version catalog (libs.versions.toml) or a shared buildSrc object.
🤖 Prompt for AI Agents
In packages/react-native-biolink/android/gradle.properties lines 1 to 5, the
specified NDK version 27.1.12297006 is not an official Android NDK release.
Update BiolinkCore_ndkVersion to a supported published NDK version such as one
from the 25.x.x or 26.x.x series, or add documentation if using a custom NDK
distribution in your CI. Additionally, consider moving these version properties
into a Gradle version catalog (libs.versions.toml) or a shared buildSrc object
for better consistency and easier upgrades.
| class BiolinkActivityLifecycleCallbacks : Application.ActivityLifecycleCallbacks { | ||
| override fun onActivityCreated(activity: Activity, savedInstanceState: Bundle?) { | ||
| // Not used | ||
| } | ||
|
|
||
| override fun onActivityStarted(activity: Activity) { | ||
| // Not used | ||
| } | ||
|
|
||
| override fun onActivityResumed(activity: Activity) { | ||
| Log.d("BiolinkActivityLifecycle", "Activity resumed: ${activity.javaClass.simpleName}") | ||
| if (activity is FragmentActivity) { | ||
| ActivityProvider.currentActivity = activity | ||
| } | ||
| } | ||
|
|
||
| override fun onActivityPaused(activity: Activity) { | ||
| // No action needed here, as the activity might just be temporarily paused. | ||
| // We only clear the activity reference when it's destroyed. | ||
| } | ||
|
|
||
| override fun onActivityStopped(activity: Activity) { | ||
| // Not used | ||
| } | ||
|
|
||
| override fun onActivitySaveInstanceState(activity: Activity, outState: Bundle) { | ||
| // Not used | ||
| } | ||
|
|
||
| override fun onActivityDestroyed(activity: Activity) { | ||
| if (ActivityProvider.currentActivity == activity) { | ||
| Log.d("BiolinkActivityLifecycle", "Activity destroyed: ${activity.javaClass.simpleName}") | ||
| ActivityProvider.currentActivity = null | ||
| ActivityProvider.resetDeferred() | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Well-structured activity lifecycle management for biometric operations.
The implementation correctly handles the activity lifecycle for biometric authentication:
- Proper
FragmentActivitycasting and reference management - Appropriate focus on resume/destroy events for activity tracking
- Good logging for debugging lifecycle events
- Proper cleanup on activity destruction
Verify that ActivityProvider.currentActivity access is thread-safe, as activity lifecycle callbacks can potentially be called from different threads:
🏁 Script executed:
#!/bin/bash
# Search for ActivityProvider implementation to verify thread safety
ast-grep --pattern 'class ActivityProvider {
$$$
}'
# Also search for currentActivity property declarations
rg -A 5 "currentActivity.*=" packages/react-native-biolink/android/src/main/kotlin/Length of output: 4939
Ensure thread-safe access to ActivityProvider.currentActivity.
The backing field for currentActivity isn’t protected against concurrent reads/writes. Although lifecycle callbacks run on the main (UI) thread, callers on coroutine or background threads may see stale values without proper memory visibility.
Please update packages/react-native-biolink/android/src/main/kotlin/com/margelo/nitro/biolink/utils/ActivityProvider.kt:
- Annotate the private field as
@Volatile:private @Volatile var _currentActivity: FragmentActivity? = null
- Or wrap the getter/setter in synchronization or use an
AtomicReference:private val _currentActivityRef = AtomicReference<FragmentActivity?>() var currentActivity: FragmentActivity? get() = _currentActivityRef.get() set(value) { _currentActivityRef.set(value) // complete deferred, etc. }
These changes will guarantee visibility of updates across threads.
🤖 Prompt for AI Agents
In
packages/react-native-biolink/android/src/main/kotlin/com/margelo/nitro/biolink/utils/ActivityProvider.kt
around the currentActivity backing field, the field is not thread-safe which can
cause stale reads on background threads. Fix this by marking the private backing
field as @Volatile or replace it with an AtomicReference and update the getter
and setter accordingly to ensure visibility of changes across threads.
| override fun createNativeModules(reactContext: ReactApplicationContext): List<NativeModule> { | ||
| ContextProvider.context = reactContext | ||
| ActivityProvider.currentActivity = reactContext.currentActivity as? FragmentActivity | ||
| (reactContext.applicationContext as Application).registerActivityLifecycleCallbacks(BiolinkActivityLifecycleCallbacks()) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify lifecycle callback registration safety.
Ensure that registering activity lifecycle callbacks doesn't cause memory leaks or conflicts with existing callbacks.
🏁 Script executed:
#!/bin/bash
# Description: Check if BiolinkActivityLifecycleCallbacks properly manages lifecycle
# Expected: Should find proper cleanup/unregistration logic
echo "Checking BiolinkActivityLifecycleCallbacks implementation..."
# Look for the lifecycle callbacks implementation
fd -g "BiolinkActivityLifecycleCallbacks.kt" --exec cat {}
# Check for any unregister or cleanup logic
rg -A 5 -B 5 "unregister|cleanup|onDestroy" packages/react-native-biolink/android/Length of output: 1820
🏁 Script executed:
#!/bin/bash
# Show context around the lifecycle callback registration in BiolinkCorePackage.kt
rg -n -A3 -B3 "registerActivityLifecycleCallbacks" packages/react-native-biolink/android/src/main/kotlin/com/margelo/nitro/biolink/BiolinkCorePackage.ktLength of output: 608
Guard against duplicate lifecycle callback registration.
Currently, each call to createNativeModules in BiolinkCorePackage.kt ( line 24 ) unconditionally registers a new BiolinkActivityLifecycleCallbacks. If ReactApplicationContext is ever recreated (e.g. on hot reload or multiple React roots), you’ll end up with multiple callbacks hooked into the Application—leading to duplicate invocations and potential memory leaks.
Please adjust to ensure callbacks are registered exactly once per process. For example:
- Move registration into a static
initialize()or the package’sinitializehook rather than insidecreateNativeModules. - Use a private static flag to guard against repeated registration.
- If you ever unregister, call
application.unregisterActivityLifecycleCallbacks(...)in a matching cleanup/onHostDestroymethod.
File to update:
- packages/react-native-biolink/android/src/main/kotlin/com/margelo/nitro/biolink/BiolinkCorePackage.kt (around line 24)
🤖 Prompt for AI Agents
In
packages/react-native-biolink/android/src/main/kotlin/com/margelo/nitro/biolink/BiolinkCorePackage.kt
around line 24, the BiolinkActivityLifecycleCallbacks are registered
unconditionally inside createNativeModules, causing duplicate registrations on
ReactContext recreation. To fix this, move the registration logic out of
createNativeModules into a static initialize() method or the package's
initialize hook, and add a private static boolean flag to ensure the callback is
registered only once per process. Also, implement
unregisterActivityLifecycleCallbacks in onHostDestroy or equivalent cleanup to
avoid memory leaks.
| override fun getSecret(key: String): Promise<String?> { | ||
| val promise = Promise<String?>() | ||
| Log.d(TAG, "getSecret() called for key: $key") | ||
| try { | ||
| initializeSharedPrefs() | ||
| val value = sharedPrefs.getString(key, null) | ||
| if (value == null) { | ||
| promise.resolve("") | ||
| } else { | ||
| Log.i(TAG, "Secret retrieved successfully for key: $key value=$value") | ||
| promise.resolve(value) | ||
| } | ||
| } catch (e: Exception) { | ||
| Log.e(TAG, "Failed to get secret for key: $key", e) | ||
| promise.reject(RuntimeException("Failed to get secret: ${e.message}", e)) | ||
| } | ||
| return promise | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use consistent Promise patterns and handle null properly
- Use
Promise.asyncfor consistency with other methods - Consider returning null instead of empty string for missing keys
- override fun getSecret(key: String): Promise<String?> {
- val promise = Promise<String?>()
- Log.d(TAG, "getSecret() called for key: $key")
- try {
- initializeSharedPrefs()
- val value = sharedPrefs.getString(key, null)
- if (value == null) {
- promise.resolve("")
- } else {
- Log.i(TAG, "Secret retrieved successfully for key: $key value=$value")
- promise.resolve(value)
- }
- } catch (e: Exception) {
- Log.e(TAG, "Failed to get secret for key: $key", e)
- promise.reject(RuntimeException("Failed to get secret: ${e.message}", e))
- }
- return promise
- }
+ override fun getSecret(key: String): Promise<String?> = Promise.async {
+ Log.d(TAG, "getSecret() called for key: $key")
+ initializeSharedPrefs()
+ val value = sharedPrefs.getString(key, null)
+ Log.i(TAG, "Secret ${if (value != null) "retrieved" else "not found"} for key: $key")
+ value // Return null for missing keys instead of empty string
+ }🤖 Prompt for AI Agents
In
packages/react-native-biolink/android/src/main/kotlin/com/margelo/nitro/biolink/HybridBiolinkCore.kt
between lines 112 and 129, refactor the getSecret method to use Promise.async
for consistency with other methods. Also, modify the logic to return null
instead of an empty string when the key is missing by resolving the promise with
null in that case. This ensures consistent async handling and proper null
semantics.
There was a problem hiding this comment.
There's a bug with how nitro's Promise API handles null values and how that is propagated to JS. I kept running into an NPE of some sort until I decided to return an empty string since on the JS side we can account for that
There was a problem hiding this comment.
@gmemmy Thanks for clarifying that! That's valuable context about the Nitro Promise API limitation. It makes perfect sense to use the empty string workaround if null values cause NPE issues when crossing the native-to-JS boundary. The implementation is correct given this constraint, and handling empty strings on the JS side is a reasonable approach.
✏️ Learnings added
Learnt from: gmemmy
PR: gmemmy/biolink#1
File: packages/react-native-biolink/android/src/main/kotlin/com/margelo/nitro/biolink/HybridBiolinkCore.kt:112-129
Timestamp: 2025-07-29T11:48:59.070Z
Learning: In Nitro modules, the Promise API has a bug with null value propagation from native code to JavaScript that can cause NPE issues. When returning nullable values from native methods to JS, use empty strings as a workaround instead of null to avoid crashes.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| fun resetDeferred() { | ||
| _deferredActivity?.cancel() | ||
| _deferredActivity = null | ||
| _biometricPromptManager = null | ||
| } |
There was a problem hiding this comment.
Add synchronization to resetDeferred
This method modifies shared state that's accessed by other synchronized methods.
fun resetDeferred() {
- _deferredActivity?.cancel()
- _deferredActivity = null
- _biometricPromptManager = null
+ synchronized(lock) {
+ _deferredActivity?.cancel()
+ _deferredActivity = null
+ _biometricPromptManager = null
+ }
}📝 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.
| fun resetDeferred() { | |
| _deferredActivity?.cancel() | |
| _deferredActivity = null | |
| _biometricPromptManager = null | |
| } | |
| fun resetDeferred() { | |
| synchronized(lock) { | |
| _deferredActivity?.cancel() | |
| _deferredActivity = null | |
| _biometricPromptManager = null | |
| } | |
| } |
🤖 Prompt for AI Agents
In
packages/react-native-biolink/android/src/main/kotlin/com/margelo/nitro/biolink/utils/ActivityProvider.kt
around lines 45 to 49, the resetDeferred method modifies shared state accessed
by other synchronized methods. To fix this, add synchronization to the
resetDeferred method to ensure thread safety when modifying _deferredActivity
and _biometricPromptManager, preventing concurrent access issues.
|
|
||
| private val TAG = "BiometricPromptManager" | ||
| private val biometricPrompt: BiometricPrompt | ||
| private var deferredResult: CompletableDeferred<Boolean>? = null |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add thread safety for concurrent access.
The deferredResult field could be accessed concurrently without proper synchronization, leading to race conditions.
Consider using a thread-safe approach or adding synchronization:
+ private val mutex = kotlinx.coroutines.sync.Mutex()
private var deferredResult: CompletableDeferred<Boolean>? = nullAnd wrap access to deferredResult with mutex locks in the authentication callbacks and showPrompt method.
📝 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.
| private var deferredResult: CompletableDeferred<Boolean>? = null | |
| private val mutex = kotlinx.coroutines.sync.Mutex() | |
| private var deferredResult: CompletableDeferred<Boolean>? = null |
🤖 Prompt for AI Agents
In
packages/react-native-biolink/android/src/main/kotlin/com/margelo/nitro/biolink/utils/BiometricPromptManager.kt
at line 16, the deferredResult variable is accessed concurrently without
synchronization, risking race conditions. To fix this, introduce a Mutex to
guard access to deferredResult and wrap all reads and writes to deferredResult
within mutex.withLock blocks, including in authentication callbacks and the
showPrompt method, ensuring thread-safe access.
| }) | ||
| } | ||
|
|
||
| suspend fun showPrompt(promptInfo: BiometricPrompt.PromptInfo): CompletableDeferred<Boolean> { |
There was a problem hiding this comment.
Fix return type and add proper error handling.
The method should return Boolean (by awaiting the deferred result) rather than CompletableDeferred<Boolean>. The force unwrap !! on line 55 could cause a NPE.
Apply this diff to fix the API design:
- suspend fun showPrompt(promptInfo: BiometricPrompt.PromptInfo): CompletableDeferred<Boolean> {
+ suspend fun showPrompt(promptInfo: BiometricPrompt.PromptInfo): Boolean {
deferredResult = CompletableDeferred()
withContext(Dispatchers.Main) {
Handler(activity.mainLooper).post {
try {
biometricPrompt.authenticate(promptInfo)
} catch (e: Exception) {
Log.e(TAG, "Error showing biometric prompt: ${e.message}", e)
deferredResult?.completeExceptionally(RuntimeException("PROMPT_ERROR: ${e.message}", e))
deferredResult = null
}
}
}
- return deferredResult!!
+ return try {
+ deferredResult?.await() ?: false
+ } catch (e: Exception) {
+ Log.e(TAG, "Biometric authentication failed: ${e.message}")
+ false
+ }
}Also applies to: 55-55
🤖 Prompt for AI Agents
In
packages/react-native-biolink/android/src/main/kotlin/com/margelo/nitro/biolink/utils/BiometricPromptManager.kt
at line 40 and 55, change the return type of showPrompt from
CompletableDeferred<Boolean> to suspend Boolean by awaiting the deferred result
internally. Remove the force unwrap (!!) on line 55 and add proper error
handling to safely handle null or failure cases, returning false or an
appropriate fallback instead of risking a NullPointerException.
| "cxxNamespace": ["biolink"], | ||
| "ios": { | ||
| "iosModuleName": "BiolinkCore" | ||
| }, | ||
| "android": { | ||
| "androidNamespace": ["biolink", "native"], | ||
| "androidNamespace": ["biolink"], | ||
| "androidCxxLibName": "BiolinkCore" | ||
| }, | ||
| "autolinking": { | ||
| "BiolinkCore": { | ||
| "swift": "HybridBiolinkCore", | ||
| "kotlin": "HybridBiolinkCore" | ||
| } | ||
| } | ||
| }, | ||
| "ignorePaths": ["**/node_modules"] |
There was a problem hiding this comment.
Namespace change breaks existing Kotlin package path
Kotlin sources are still under com/margelo/nitro/biolink/**, but androidNamespace is now ["biolink"]. Autogenerated JNI names & R classes will no longer match, leading to “class not found” at runtime.
Restore the full namespace or move the sources:
- "cxxNamespace": ["biolink"],
+ "cxxNamespace": ["margelo", "nitro", "biolink"],
…
- "androidNamespace": ["biolink"],
+ "androidNamespace": ["margelo", "nitro", "biolink"],Then keep ignorePaths as-is.
📝 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.
| "cxxNamespace": ["biolink"], | |
| "ios": { | |
| "iosModuleName": "BiolinkCore" | |
| }, | |
| "android": { | |
| "androidNamespace": ["biolink", "native"], | |
| "androidNamespace": ["biolink"], | |
| "androidCxxLibName": "BiolinkCore" | |
| }, | |
| "autolinking": { | |
| "BiolinkCore": { | |
| "swift": "HybridBiolinkCore", | |
| "kotlin": "HybridBiolinkCore" | |
| } | |
| } | |
| }, | |
| "ignorePaths": ["**/node_modules"] | |
| "cxxNamespace": ["margelo", "nitro", "biolink"], | |
| "ios": { | |
| "iosModuleName": "BiolinkCore" | |
| }, | |
| "android": { | |
| "androidNamespace": ["margelo", "nitro", "biolink"], | |
| "androidCxxLibName": "BiolinkCore" | |
| }, | |
| "autolinking": { | |
| "BiolinkCore": { | |
| "swift": "HybridBiolinkCore", | |
| "kotlin": "HybridBiolinkCore" | |
| } | |
| }, | |
| "ignorePaths": ["**/node_modules"] |
🤖 Prompt for AI Agents
In packages/react-native-biolink/nitro.json around lines 3 to 17, the
androidNamespace is set to ["biolink"], which breaks the existing Kotlin package
path under com/margelo/nitro/biolink/**. To fix this, restore the
androidNamespace to the full original namespace matching the Kotlin source path,
such as ["com", "margelo", "nitro", "biolink"], or alternatively move the Kotlin
sources to match the new androidNamespace. Keep the ignorePaths setting
unchanged.
| "android/build.gradle", | ||
| "android/CMakeLists.txt", | ||
| "android/gradle.properties", |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
LGTM: Android build files added for npm distribution.
Adding the Android build configuration files to the files array is necessary for proper distribution of the native module. This ensures consumers get all required build files.
Please verify the build and packaging works correctly after removing the "type": "module" field:
🏁 Script executed:
#!/bin/bash
# Verify the package builds and packages correctly
echo "Testing package build and validation..."
# Check if TypeScript build works
cd packages/react-native-biolink
npm run build
# Validate package contents
npm pack --dry-run | grep -E "(android|build\.gradle|CMakeLists\.txt|gradle\.properties)"Length of output: 6217
Resolve TypeScript build errors in react-native-biolink
The packaging step confirms that Android files are included (android/build.gradle, CMakeLists.txt, gradle.properties), but the TypeScript build is currently failing with errors:
• specs/BiolinkCore.nitro.ts: Cannot find module ‘react-native-nitro-modules’ or its type declarations
• src/index.ts: Cannot find module ‘react’ or its type declarations
• src/logger.ts: Cannot find module ‘react-native-logs’ or its type declarations
• src/utils/signature.ts: Cannot find module ‘react-native-nitro-modules’, untyped function calls, and “Type ‘null’ is not assignable to type ‘BiolinkCore’”
Please address these by:
- Adding or publishing missing type declarations (e.g. @types/react, @types/react-native-logs, or a declaration for react-native-nitro-modules)
- Adjusting tsconfig.json (e.g. include necessary typeRoots or enable skipLibCheck)
- Fixing the null-assignments in signature.ts to satisfy the BiolinkCore type
Once resolved, rerun npm run build to confirm the project compiles cleanly.
🤖 Prompt for AI Agents
In packages/react-native-biolink/package.json around lines 53 to 55, the
TypeScript build fails due to missing type declarations for modules like react,
react-native-logs, and react-native-nitro-modules, and a null assignment issue
in signature.ts. Fix this by adding the appropriate @types packages for react
and react-native-logs, creating or adding a declaration file for
react-native-nitro-modules, updating tsconfig.json to include necessary
typeRoots or enable skipLibCheck to bypass type checking of libraries, and
modifying signature.ts to prevent assigning null to a BiolinkCore typed
variable. After these changes, rerun npm run build to ensure the build succeeds
without errors.
|
Note Unit test generation is an Early Access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
|
✅ UTG Post-Process Complete No new issues were detected in the generated code and all check runs have completed. The unit test generation process has completed successfully. |
|
Creating a PR to put the unit tests in... The changes have been created in this pull request: View PR |
gmemmy
changed the title
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (9)
packages/react-native-biolink/README.md (9)
53-55: Solid RN example; minor accessibility suggestionConsider adding accessibilityRole and accessible labels to improve a11y in the example.
- <TouchableOpacity onPress={handleLogin} disabled={isLoading}> - <Text>{isAuthenticated ? 'Authenticated' : 'Login'}</Text> - </TouchableOpacity> + <TouchableOpacity + onPress={handleLogin} + disabled={isLoading} + accessibilityRole="button" + accessibilityLabel={isAuthenticated ? 'Authenticated' : 'Login'} + > + <Text>{isAuthenticated ? 'Authenticated' : 'Login'}</Text> + </TouchableOpacity>
64-66: Clarify default and return type for authenticatePlease document the default for fallbackToDeviceCredential (assumed false) and the return type (Promise?), plus typical errors thrown when biometrics are unavailable/cancelled.
87-92: Refine size guidance and consider platform limitsGood clarity. Suggest tightening wording and briefly noting platform limits (Keychain/Keystore best for small secrets; store large blobs elsewhere).
-> **Note**: While the library handles encryption, it's generally recommended to keep stored secrets concise. Avoid storing extremely large data blobs. +> Note: While the library handles encryption, keep stored secrets small. Avoid storing large blobs; prefer referencing them (e.g., via IDs) and store bulk data elsewhere.
98-101: Specify return type and nullability for getSecretPlease state the return type explicitly (Promise<string | null>) and that absent keys resolve to null. The example already demonstrates a safe fallback.
130-132: Strengthen TS example error typingUse unknown and a type guard to avoid relying on any in examples.
-} catch (error) { - if (error.code === 'PIN_LOCKED') { +} catch (error: unknown) { + const code = (error as { code?: string; remainingAttempts?: number; message?: string })?.code; + if (code === 'PIN_LOCKED') { console.log('PIN is locked, try again later'); - } else if (error.code === 'PIN_INCORRECT') { - console.log(`${error.remainingAttempts} attempts remaining`); + } else if (code === 'PIN_INCORRECT') { + console.log(`${(error as any).remainingAttempts} attempts remaining`); } else { console.error('Authentication failed:', (error as any)?.message ?? error); }
136-141: Expose typed error surface for PIN errorsConsider exporting a discriminated union or an enum for PIN error codes and a typed Error shape so consumers can narrow safely.
165-166: Add algorithm and serialization details for signingState the key type and algorithm (e.g., ECDSA P-256 + SHA-256, RSA-PSS), body canonicalization (stable JSON serialization), and base64 variant used to ensure verifier compatibility.
167-170: Document defaults and types for getSignatureHeadersPlease specify return type (Promise<Record<string, string>>), the default header name ('X-Body-Signature'), and confirm that body serialization is deterministic (e.g., canonical JSON) to avoid signature mismatches.
186-189: API shape for including public key feels redundantMethod name implies headers always include the public key; the includePublicKey?: boolean parameter is then unnecessary. Consider either:
- Remove the boolean (always include key), or
- Collapse into a single getSignatureHeaders(body, options?: { includePublicKey?: boolean; headerNames?: {...} })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
README.md(3 hunks)biolink-demo/README.md(2 hunks)packages/react-native-biolink/README.md(9 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- README.md
- biolink-demo/README.md
🧰 Additional context used
🪛 LanguageTool
packages/react-native-biolink/README.md
[style] ~91-~91: As an alternative to the over-used intensifier ‘extremely’, consider replacing this phrase.
Context: ...p stored secrets concise. Avoid storing extremely large data blobs. ```typescript await storeS...
(EN_WEAK_ADJECTIVE)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (3)
packages/react-native-biolink/README.md (3)
7-7: Good redirection to a single source of truthPointing feature/roadmap to the main README helps avoid drift. Please verify the relative link resolves correctly from this package (../../README.md).
32-32: RN components import is correct for the exampleImporting TouchableOpacity and Text aligns with RN conventions. No issues.
117-120: Document behavior and return type for authenticateWithPinClarify whether it resolves void/boolean on success and always rejects on failure. Also mention if it increments attempt counters on each call and any debouncing guidance.
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
packages/react-native-biolink/README.md (2)
24-43: Avoid top-level await in Quick Start examples.React Native Metro may not support top-level await in module scope in all setups; these lines can break apps when copied verbatim. Wrap calls in an async function (e.g., inside a useEffect or an event handler).
Example:
-// Biometric authentication with device fallback -const isAuthenticated = await authenticate(true); - -// Secure storage -await storeSecret('user-token', 'your-secure-token'); -const token = await getSecret('user-token'); - -// Digital signing for API requests -const headers = await getSignatureHeaders({ userId: 123, action: 'login' }); +async function bootstrap() { + // Biometric authentication with device fallback + const isAuthenticated = await authenticate(true); + // Secure storage + await storeSecret('user-token', 'your-secure-token'); + const token = await getSecret('user-token'); + // Digital signing for API requests + const headers = await getSignatureHeaders({ userId: 123, action: 'login' }); +} +// Call bootstrap at an appropriate lifecycle point (e.g., inside a React effect)
186-196: Resolve naming/parameter inconsistency ingetSignatureHeadersWithPublicKey.The method name implies the public key is always included, yet it has
includePublicKey?: boolean. Either:
- Remove the parameter and always include the key, or
- Rename to
getSignatureHeaders({ body, includePublicKey }: { body: object; includePublicKey?: boolean })and deprecate the previous overload.Also add return type:
-#### `getSignatureHeadersWithPublicKey(body: object, includePublicKey?: boolean)` +#### `getSignatureHeadersWithPublicKey(body: object, includePublicKey?: boolean): Promise<Record<string, string>>`If keeping the param, document the default (
true/false) and when to use it. Consider a key identifier header (X-Key-Id) to avoid sending the full public key on every request.
🧹 Nitpick comments (7)
packages/react-native-biolink/README.md (7)
64-74: Document return type and default behavior forauthenticate.Add the Promise return type and default for fallback. Also clarify platform notes if behavior differs.
-#### `authenticate(fallbackToDeviceCredential?: boolean)` +#### `authenticate(fallbackToDeviceCredential?: boolean): Promise<boolean>` -Authenticates the user using biometrics (Face ID, Touch ID, Fingerprint). If `fallbackToDeviceCredential` is set to `true`, the system will allow the user to fall back to their device PIN, pattern, or password if biometrics are not available or fail. +Authenticates the user using biometrics (Face ID, Touch ID, Fingerprint). If `fallbackToDeviceCredential` is `true` (default: `false`), the system allows fallback to device PIN/pattern/password if biometrics are unavailable or fail.
87-92: Tighten wording and add return type forstoreSecret.
- Avoid weak intensifier “extremely”.
- Specify Promise return type for consistency.
-#### `storeSecret(key: string, value: string)` +#### `storeSecret(key: string, value: string): Promise<void>` -Stores a `value` securely using platform-specific storage (iOS Keychain, Android Keystore) associated with a given `key`. +Stores a `value` securely using platform-specific storage (iOS Keychain, Android Keystore) associated with a given `key`. -> **Note**: While the library handles encryption, it's generally recommended to keep stored secrets concise. Avoid storing extremely large data blobs. +> **Note**: While the library handles encryption, keep stored secrets concise. Avoid storing very large data blobs.
98-105: ClarifygetSecretreturn type.Callers often need to handle “not found”; document nullable result.
-#### `getSecret(key: string)` +#### `getSecret(key: string): Promise<string | null>` -Retrieves a securely stored value associated with a given `key` from platform-specific storage. +Retrieves a securely stored value associated with a given `key` from platform-specific storage. Returns `null` if the key is not found.
117-121: Strengthen error typing forauthenticateWithPin.Good coverage of error codes. Consider documenting a typed error shape to aid consumers.
Add to docs:
type PinAuthErrorCode = 'PIN_LOCKED' | 'PIN_INCORRECT' | 'UNKNOWN'; interface PinAuthError extends Error { code: PinAuthErrorCode; remainingAttempts?: number; lockoutEndsAt?: number; // epoch ms }And note the function signature:
authenticateWithPin(pin: string): Promise<void>Also applies to: 136-141
167-174: Document default header name and return type forgetSignatureHeaders.
- Include Promise return type.
- State the default header and how
headerNameoverrides it.-#### `getSignatureHeaders(body: object, headerName?: string)` +#### `getSignatureHeaders(body: object, headerName?: string): Promise<Record<string, string>>` -Generates a cryptographic signature for the provided `body` object using a hardware-backed key and returns it as an HTTP header. This ensures the integrity and authenticity of the data sent in API requests. +Generates a cryptographic signature for the provided `body` using a hardware-backed key and returns it as an HTTP header. Default header: `'X-Body-Signature'`. Pass `headerName` to override.Also applies to: 176-184
11-18: Pin and document version constraints for dependencies.
- Specify minimal compatible versions for
react-native-nitro-modulesand React Native 0.74+ explicitly in docs and peerDependencies to prevent mismatches.Would you like a patch for package.json peerDependencies and an added note here with exact version ranges (e.g.,
react-native >= 0.74,react-native-nitro-modules >= x.y)?Also applies to: 20-20
211-219: Add caveat for hardware-backed features.On emulators/simulators or older devices, hardware-backed keys/biometrics may be unavailable. A brief note helps reduce support issues.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
README.md(3 hunks)biolink-demo/README.md(2 hunks)packages/react-native-biolink/README.md(9 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- biolink-demo/README.md
- README.md
🧰 Additional context used
🪛 LanguageTool
packages/react-native-biolink/README.md
[style] ~91-~91: As an alternative to the over-used intensifier ‘extremely’, consider replacing this phrase.
Context: ...p stored secrets concise. Avoid storing extremely large data blobs. ```typescript await storeS...
(EN_WEAK_ADJECTIVE)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (4)
packages/react-native-biolink/README.md (4)
7-7: Good scope and separation of concerns.Clear repositioning of package README as API/dev guide with a link to the main project README for high-level topics.
32-32: Quick Start aligns with React Native UI.Switch to TouchableOpacity/Text is correct and improves copy-paste usability.
Also applies to: 53-55
165-170: Specify canonicalization for signing.To ensure verifiable signatures across platforms/backends, document:
- Exact serialization (e.g., canonical JSON with sorted keys, UTF-8).
- Signature digest/algorithm (e.g., SHA-256 with ECDSA P-256).
- Output encoding (Base64 vs Base64URL).
Would you add these details so integrators can reproduce verification deterministically?
222-222: LICENSE link is correctThe README at
packages/react-native-biolink/README.mdpoints to theLICENSEfile in the same directory, and there is indeed apackages/react-native-biolink/LICENSE. No path change is needed.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
packages/react-native-biolink/README.md (8)
7-7: Good consolidation; verify link target and add cross-link to Nitro docs.The redirection to the main README is sensible. Please:
- Verify that ../../README.md is correct from packages/react-native-biolink/README.md.
- Consider adding a link to Nitro Modules docs for quick context.
-This document provides the API reference and developer guide for `@gmemmy/react-native-biolink`, a biometric authentication and secure storage module for React Native’s New Architecture. For a high-level overview of the project, including features, roadmap, and general requirements, please refer to the [main project README](../../README.md). +This document provides the API reference and developer guide for `@gmemmy/react-native-biolink`, a biometric authentication and secure storage module for React Native’s New Architecture. For a high-level overview of the project, including features, roadmap, and general requirements, please refer to the [main project README](../../README.md). For background on Nitro Modules, see the React Native [New Architecture/Nitro Modules docs](https://reactnative.dev/docs/nitro-modules).
53-55: Avoid shadowing imported names in the example snippet.The snippet imports authenticate at top-level and also destructures authenticate and isAuthenticated from useAuth(), which can confuse readers and linters (shadowing). Suggest renaming variables from the hook (or split the examples).
- const { isAuthenticated, authenticate, isLoading, error } = useAuth(); + const { isAuthenticated: authState, authenticate: authenticateWithHook, isLoading, error } = useAuth(); const handleLogin = async () => { - await authenticate(true); // with device fallback + await authenticateWithHook(true); // with device fallback }; return ( - <TouchableOpacity onPress={handleLogin} disabled={isLoading}> - <Text>{isAuthenticated ? 'Authenticated' : 'Login'}</Text> + <TouchableOpacity onPress={handleLogin} disabled={isLoading}> + <Text>{authState ? 'Authenticated' : 'Login'}</Text> </TouchableOpacity> );
64-66: Clarify Android fallback behavior and OS constraints.Document that device-credential fallback depends on Android version and BiometricPrompt configuration so developers set expectations correctly.
-Authenticates the user using biometrics (Face ID, Touch ID, Fingerprint). If `fallbackToDeviceCredential` is set to `true`, the system will allow the user to fall back to their device PIN, pattern, or password if biometrics are not available or fail. +Authenticates the user using biometrics (Face ID, Touch ID, Fingerprint). If `fallbackToDeviceCredential` is `true`, the system may allow fallback to device PIN/pattern/password when biometrics are unavailable or fail. + +Notes: +- Android: Device-credential fallback behavior varies by API level and OEM. On some devices/versions, you cannot combine strong-biometrics with device credential in a single prompt. Ensure your policy matches the device’s capabilities. +- iOS: System UI handles passcode fallback automatically when configured by the OS.
87-91: Tighten language and set expectations on secret size.Minor wording polish and a gentle size recommendation helps prevent misuse.
-Stores a `value` securely using platform-specific storage (iOS Keychain, Android Keystore) associated with a given `key`. +Stores a `value` securely using platform-specific storage (iOS Keychain, Android Keystore) associated with a given `key`. -> **Note**: While the library handles encryption, it's generally recommended to keep stored secrets concise. Avoid storing extremely large data blobs. +> Note: While encryption is handled, keep stored secrets concise. Avoid storing large data blobs; prefer references or tokens over bulky payloads.
117-119: Document PIN policy details and type consistency across the section.Good to see types on authenticateWithPin. For completeness, consider:
- Adding a type annotation to enrollPin (e.g., enrollPin(pin: string): Promise)
- Documenting lockout parameters (max attempts, lockout duration) so implementers can design UX around it.
-#### `authenticateWithPin(pin: string)` +#### `authenticateWithPin(pin: string)` +Related: +- `enrollPin(pin: string): Promise<void>` +- `getPinLockoutStatus(): Promise<{ isLocked: boolean; lockoutEndsAt?: number; remainingAttempts: number }>`
130-131: Use proper error narrowing in TypeScript catch blocks.In TS, error is unknown by default. Narrow before accessing code/message to avoid type errors and to guide users.
- } else { - console.error('Authentication failed:', error.message); + } else { + if (error && typeof error === 'object' && 'message' in error) { + console.error('Authentication failed:', (error as { message: string }).message); + } else { + console.error('Authentication failed'); + } }
186-188: Header naming and defaults; allow customization for parity with getSignatureHeaders.Consider documenting the default header names and optionally supporting a custom headerName parameter here as well. Also clarify encoding (base64 vs base64url).
-#### `getSignatureHeadersWithPublicKey(body: object, includePublicKey?: boolean)` +#### `getSignatureHeadersWithPublicKey(body: object, includePublicKey?: boolean, headerName?: string)` -Generates a cryptographic signature for the provided `body` object and includes the public key in the returned HTTP headers. This allows the recipient to verify the signature and the origin of the data. +Generates a cryptographic signature for `body` and includes the public key in the returned HTTP headers. +Defaults: +- Signature header: `X-Body-Signature` (base64url) +- Public key header: `X-Public-Key` (base64url) +You can override the signature header name via `headerName`.
222-222: Fix LICENSE link path.From packages/react-native-biolink/README.md, LICENSE likely points to a non-existent file in this subfolder. Link to the repo root license instead.
-This project is licensed under the MIT License - see the [LICENSE](LICENSE) file for details. +This project is licensed under the MIT License - see the [LICENSE](../../LICENSE) file for details.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
README.md(3 hunks)biolink-demo/README.md(2 hunks)packages/react-native-biolink/README.md(9 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- biolink-demo/README.md
- README.md
🧰 Additional context used
🪛 LanguageTool
packages/react-native-biolink/README.md
[style] ~91-~91: As an alternative to the over-used intensifier ‘extremely’, consider replacing this phrase.
Context: ...p stored secrets concise. Avoid storing extremely large data blobs. ```typescript await storeS...
(EN_WEAK_ADJECTIVE)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (3)
packages/react-native-biolink/README.md (3)
32-32: Correct fix: use React Native components instead of HTML.Switching to TouchableOpacity/Text is appropriate here.
98-100: Specify return shape of getSecret for correctness.Does getSecret return null/undefined when the key is missing? If so, reflect that in the signature and examples to avoid runtime surprises.
-#### `getSecret(key: string)` +#### `getSecret(key: string): Promise<string | null>` -Retrieves a securely stored value associated with a given `key` from platform-specific storage. +Retrieves a securely stored value associated with a given `key` from platform-specific storage. Returns `null` if the key is not found.If it never returns null (throws instead), please document the thrown error type and conditions.
136-141: List all documented error codes and their shapes.If additional cases exist (e.g., PIN_NOT_ENROLLED, PIN_TOO_WEAK, STORAGE_ERROR), include them and document the error payload shape for each (fields like remainingAttempts, lockoutEndsAt).
> - `PIN_LOCKED`: The PIN is temporarily locked due to too many failed attempts. Check `getPinLockoutStatus()` for details. > - `PIN_INCORRECT`: The provided PIN is incorrect. The error object will contain `remainingAttempts`. -> - Other errors: General authentication failures. +> - `PIN_NOT_ENROLLED`: No PIN is enrolled on the device/user (call `enrollPin` first). +> - Other errors: General authentication failures. The error object includes at least `{ code: string; message: string }`.
| Digital signing uses hardware-backed keys to create cryptographic signatures for data, ensuring its integrity and authenticity. This is useful for verifying that data has not been tampered with and originates from a trusted source. | ||
|
|
||
| #### `getSignatureHeaders(body: object, headerName?: string)` | ||
|
|
||
| Generate signed headers for API requests. | ||
| Generates a cryptographic signature for the provided `body` object using a hardware-backed key and returns it as an HTTP header. This ensures the integrity and authenticity of the data sent in API requests. |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Define the canonicalization and algorithm used for signing.
Verification depends on an unambiguous byte representation. Please document:
- Canonicalization of body (e.g., JSON.stringify with stable key order; which serializer?)
- Hashing and signature algorithms (e.g., SHA-256 with ECDSA P-256; encoding base64 or base64url)
- Key lifecycle (generation, storage location, rotation, persistence across reinstalls)
-Digital signing uses hardware-backed keys to create cryptographic signatures for data, ensuring its integrity and authenticity. This is useful for verifying that data has not been tampered with and originates from a trusted source.
+Digital signing uses hardware-backed keys to create cryptographic signatures for data, ensuring integrity and authenticity.
+
+Details:
+- Canonicalization: The request body is serialized using <stable JSON method>. Keys are sorted lexicographically before stringification to ensure deterministic signatures.
+- Algorithm: ECDSA P-256 with SHA-256. Signatures are encoded as base64url.
+- Key management: Keys are generated on-device and stored in Secure Enclave/StrongBox/Keystore. Keys persist across app launches but are cleared on uninstall. Rotation occurs when <condition>.📝 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.
| Digital signing uses hardware-backed keys to create cryptographic signatures for data, ensuring its integrity and authenticity. This is useful for verifying that data has not been tampered with and originates from a trusted source. | |
| #### `getSignatureHeaders(body: object, headerName?: string)` | |
| Generate signed headers for API requests. | |
| Generates a cryptographic signature for the provided `body` object using a hardware-backed key and returns it as an HTTP header. This ensures the integrity and authenticity of the data sent in API requests. | |
| Digital signing uses hardware-backed keys to create cryptographic signatures for data, ensuring integrity and authenticity. | |
| Details: | |
| - Canonicalization: The request body is serialized using <stable JSON method>. Keys are sorted lexicographically before stringification to ensure deterministic signatures. | |
| - Algorithm: ECDSA P-256 with SHA-256. Signatures are encoded as base64url. | |
| - Key management: Keys are generated on-device and stored in Secure Enclave/StrongBox/Keystore. Keys persist across app launches but are cleared on uninstall. Rotation occurs when <condition>. | |
| #### `getSignatureHeaders(body: object, headerName?: string)` | |
| Generates a cryptographic signature for the provided `body` object using a hardware-backed key and returns it as an HTTP header. This ensures the integrity and authenticity of the data sent in API requests. |
🤖 Prompt for AI Agents
In packages/react-native-biolink/README.md around lines 165 to 169, the
documentation for getSignatureHeaders lacks details on the canonicalization
method and cryptographic algorithms used. Update the README to specify the exact
canonicalization process for the body object (such as using JSON.stringify with
stable key ordering or a specific serializer), the hashing and signature
algorithms employed (e.g., SHA-256 with ECDSA P-256), the encoding format of the
signature (base64 or base64url), and clarify the key lifecycle including how
keys are generated, where they are stored, how they are rotated, and whether
they persist across app reinstalls.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/react-native-biolink/README.md (1)
165-170: Define canonicalization, algorithms, and encodings for signingDeterministic verification requires precise specs for body serialization, hashing/signing algorithms, and encodings. Also document key lifecycle.
-Digital signing uses hardware-backed keys to create cryptographic signatures for data, ensuring its integrity and authenticity. This is useful for verifying that data has not been tampered with and originates from a trusted source. +Digital signing uses hardware-backed keys to create cryptographic signatures for data, ensuring integrity and authenticity. + +Details: +- Canonicalization: Request body is serialized using <stable JSON method>. Keys are sorted lexicographically; numbers/booleans are emitted canonically; UTF‑8 without BOM. +- Algorithm: <e.g., ECDSA P‑256 with SHA‑256> (Android StrongBox/Keystore, iOS Secure Enclave/Keychain). +- Signature encoding: <base64 | base64url> of DER or raw (r|s) bytes. Specify precisely. +- Header naming: Default `X-Body-Signature` (override with `headerName`). +- Public key format: <PEM | base64 DER> and algorithm identifier (e.g., `alg="ES256"`). +- Key management: Generated on-device, hardware-backed if available; persisted across launches; cleared on uninstall; rotation policy: <condition and header to convey key id/version>.I can propose concrete text based on the native code choices.
🧹 Nitpick comments (3)
packages/react-native-biolink/README.md (3)
87-92: Tone/style nit: avoid over-intensifierReplace “extremely large data blobs” with a leaner phrase.
-> **Note**: While the library handles encryption, it's generally recommended to keep stored secrets concise. Avoid storing extremely large data blobs. +> **Note**: While the library handles encryption, prefer keeping stored secrets concise. Avoid storing very large data blobs.
98-101: Document return type and absent-key behavior for getSecretPlease clarify what is returned when the key is missing and reflect it in the signature.
-#### `getSecret(key: string)` +#### `getSecret(key: string): Promise<string | null>` -Retrieves a securely stored value associated with a given `key` from platform-specific storage. +Retrieves a securely stored value associated with a given `key`. Returns `null` if the key does not exist.Optionally update the example to check for null explicitly.
Does the implementation return null/undefined on miss? I can align the examples accordingly.
136-141: Specify error type and lockout policy detailsDocument a typed error shape and lockout strategy to aid UX decisions:
-> **Error Codes for `authenticateWithPin`**: +> **Errors for `authenticateWithPin`**: +> +> Type: +> ```ts +> type PinAuthError = +> | { code: 'PIN_LOCKED'; lockoutEndsAt: number } +> | { code: 'PIN_INCORRECT'; remainingAttempts: number } +> | { code: 'UNKNOWN'; message: string }; +> ``` > -> - `PIN_LOCKED`: The PIN is temporarily locked due to too many failed attempts. Check `getPinLockoutStatus()` for details. +> - `PIN_LOCKED`: Temporarily locked after too many failed attempts. See `getPinLockoutStatus()`. > - `PIN_INCORRECT`: The provided PIN is incorrect. The error object will contain `remainingAttempts`. -> - Other errors: General authentication failures. +> - `UNKNOWN`: General authentication failure. + +> Lockout policy: +> - Max attempts: <n> +> - Lockout duration/backoff: <duration or exponential strategy>Fill in concrete numbers to match implementation.
I can update this with real values from the native module.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.gitignore(1 hunks)README.md(3 hunks)biolink-demo/README.md(2 hunks)packages/react-native-biolink/.gitignore(1 hunks)packages/react-native-biolink/README.md(9 hunks)
✅ Files skipped from review due to trivial changes (2)
- .gitignore
- packages/react-native-biolink/.gitignore
🚧 Files skipped from review as they are similar to previous changes (2)
- biolink-demo/README.md
- README.md
🧰 Additional context used
🪛 LanguageTool
packages/react-native-biolink/README.md
[style] ~91-~91: As an alternative to the over-used intensifier ‘extremely’, consider replacing this phrase.
Context: ...p stored secrets concise. Avoid storing extremely large data blobs. ```typescript await storeS...
(EN_WEAK_ADJECTIVE)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (6)
packages/react-native-biolink/README.md (6)
7-7: Good consolidation to root READMEClear pointer to the main README reduces duplication. LGTM.
32-32: RN imports in example are correctUsing TouchableOpacity/Text is appropriate. LGTM.
53-55: Quick Start JSX looks correctHook usage and disabled state wiring are consistent. LGTM.
64-66: Clarify fallback behavior and defaults for authenticate()Document:
- The default value of fallbackToDeviceCredential.
- Platform nuances (iOS: device passcode; Android: device credential via BiometricPrompt; min API/version constraints).
- Whether enabling device credentials changes biometric-only behavior.
Would you like me to draft a short platform notes block here?
117-119: PIN API description reads wellLockout mention is helpful. LGTM.
130-131: Example adds a useful generic catchGood practice to surface unexpected errors. LGTM.
| #### `getSignatureHeadersWithPublicKey(body: object, includePublicKey?: boolean)` | ||
|
|
||
| Generate signed headers with public key included. | ||
| Generates a cryptographic signature for the provided `body` object and includes the public key in the returned HTTP headers. This allows the recipient to verify the signature and the origin of the data. | ||
|
|
There was a problem hiding this comment.
API/Docs inconsistency: includePublicKey? parameter vs function name
The method name WithPublicKey implies the public key is always included, but the signature shows includePublicKey? and the example omits it yet includes the key. Resolve by either removing the parameter or documenting its default and behavior. Also document header names and formats.
-#### `getSignatureHeadersWithPublicKey(body: object, includePublicKey?: boolean)`
+#### `getSignatureHeadersWithPublicKey(body: object)`
-Generates a cryptographic signature for the provided `body` object and includes the public key in the returned HTTP headers. This allows the recipient to verify the signature and the origin of the data.
+Generates a cryptographic signature and includes the public key in the returned HTTP headers.
+Defaults:
+- Signature header: `X-Body-Signature` (<base64|base64url>, <DER|raw> format).
+- Public key header: `X-Public-Key` (<PEM | base64 DER>).
+See “Details” above for canonicalization and algorithm.If you intend the flag to control inclusion, keep the name generic (e.g., getSignatureHeaders) and document:
-#### `getSignatureHeadersWithPublicKey(body: object, includePublicKey?: boolean)`
+#### `getSignatureHeaders(body: object, options?: { headerName?: string; includePublicKey?: boolean })`I can align both sections and examples after you confirm the intended API.
📝 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.
| #### `getSignatureHeadersWithPublicKey(body: object, includePublicKey?: boolean)` | |
| Generate signed headers with public key included. | |
| Generates a cryptographic signature for the provided `body` object and includes the public key in the returned HTTP headers. This allows the recipient to verify the signature and the origin of the data. | |
| #### `getSignatureHeadersWithPublicKey(body: object)` | |
| Generates a cryptographic signature and includes the public key in the returned HTTP headers. | |
| Defaults: | |
| - Signature header: `X-Body-Signature` (<base64|base64url>, <DER|raw> format). | |
| - Public key header: `X-Public-Key` (<PEM | base64 DER>). | |
| See “Details” above for canonicalization and algorithm. |
🤖 Prompt for AI Agents
In packages/react-native-biolink/README.md around lines 186 to 189, there is a
mismatch between the function name getSignatureHeadersWithPublicKey and the
optional includePublicKey parameter, causing confusion about whether the public
key is always included. To fix this, either remove the includePublicKey
parameter if the public key is always included, or rename the function to a more
generic name like getSignatureHeaders if the parameter controls inclusion.
Additionally, clearly document the default behavior of the parameter, and
specify the exact HTTP header names and formats used in the returned headers.
| } | ||
|
|
||
| return deferredResult!! | ||
| } |
There was a problem hiding this comment.
Bug: Biometric Authentication Flow and Coroutine Issues
The BiometricPromptManager has several issues:
onAuthenticationFailed()incorrectly completes the authentication flow with an exception, preventing users from retrying failed biometric attempts.- The
showPromptfunction has an incorrect suspend function design, returning aCompletableDeferredinstead of directly awaiting and returning theBooleanresult, which violates Kotlin coroutine conventions. - A race condition exists where concurrent calls to
showPrompt()can overwrite thedeferredResultinstance, causing earlier callers to never receive a result. - A
NullPointerExceptioncan occur inshowPromptifbiometricPrompt.authenticate()throws an exception, asdeferredResultis nulled in the catch block before being force-unwrapped and returned.
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
packages/react-native-biolink/README.md (2)
117-135: Document error shape and PIN policy for authenticateWithPinDefine the error type (codes, fields) and the PIN policy (length, charset, attempts, lockout/backoff) to set expectations and ease handling.
Suggested addition:
type PinAuthError = | { code: 'PIN_LOCKED'; lockoutEndsAt: number } | { code: 'PIN_INCORRECT'; remainingAttempts: number } | { code: string; message?: string }; interface PinPolicy { minLength: number; maxLength: number; attemptsBeforeLockout: number; lockoutSeconds: number; // or backoff strategy }Also add a “PIN Policy” note stating current defaults.
167-184: Clarify default header name and customization for getSignatureHeadersState the default header name and show how headerName changes it in examples.
-#### `getSignatureHeaders(body: object, headerName?: string)` +#### `getSignatureHeaders(body: object, headerName?: string)` +Defaults: +- Header name: `X-Body-Signature` +If `headerName` is provided, the signature is returned under that name. ```typescript -const headers = await getSignatureHeaders({ userId: 123, action: 'login' }); +const headers = await getSignatureHeaders({ userId: 123, action: 'login' }); // Returns: { 'X-Body-Signature': 'base64-signature-here' } + +const custom = await getSignatureHeaders({ id: 1 }, 'X-Sig'); +// Returns: { 'X-Sig': 'base64-signature-here' }
♻️ Duplicate comments (2)
packages/react-native-biolink/README.md (2)
186-196: API/docs mismatch: WithPublicKey name vs includePublicKey? parameterEither the public key is always included (remove the flag) or the name should be generic and options-driven. Also document header names/formats.
Option A (always include key):
-#### `getSignatureHeadersWithPublicKey(body: object, includePublicKey?: boolean)` +#### `getSignatureHeadersWithPublicKey(body: object)` +Returns: +- Signature header: `X-Body-Signature` (<base64|base64url>, <DER|raw>). +- Public key header: `X-Public-Key` (<PEM | base64 DER>).Option B (configurable):
-#### `getSignatureHeadersWithPublicKey(body: object, includePublicKey?: boolean)` +#### `getSignatureHeaders(body: object, options?: { headerName?: string; includePublicKey?: boolean; publicKeyHeaderName?: string })`Update examples accordingly and state defaults.
165-170: Specify canonicalization and cryptographic algorithms used for signingReaders need unambiguous details to verify signatures correctly (body canonicalization, hash+signature alg, encoding, key lifecycle).
-Digital signing uses hardware-backed keys to create cryptographic signatures for data, ensuring its integrity and authenticity. This is useful for verifying that data has not been tampered with and originates from a trusted source. +Digital signing uses hardware-backed keys to create cryptographic signatures for data, ensuring integrity and authenticity. + +Details: +- Canonicalization: The request body is serialized using a stable JSON method (e.g., keys sorted lexicographically) to ensure deterministic signatures. +- Algorithm: ECDSA P-256 with SHA-256 (or specify your actual choice). Signatures are encoded as base64url (or base64) in <DER|raw> format. +- Key management: Keys are generated on-device and stored in Secure Enclave/StrongBox/Keystore. Keys persist across app launches, are removed on uninstall, and rotate under <conditions>.
🧹 Nitpick comments (8)
biolink-demo/README.md (2)
46-48: Codegen step: prefer a workspace script to reduce cd churnRecommend adding a workspace-level script and invoking it from repo root for consistency.
-# Generate native bindings for the library -cd packages/react-native-biolink && pnpm codegen && cd ../.. +# Generate native bindings for the library +pnpm --filter @gmemmy/react-native-biolink codegenOptionally add to root package.json:
{ "scripts": { "codegen": "pnpm --filter @gmemmy/react-native-biolink codegen" } }
30-36: Add device enrollment prerequisite for biometrics/PINMention that test devices/emulators must have biometrics and device credentials enrolled; otherwise prompts will fail or fallback won’t appear.
Suggested addition:
- iOS: Ensure a passcode and Face ID/Touch ID are configured (Settings > Face ID & Passcode).
- Android: Set a screen lock and enroll biometrics (Settings > Security). For emulators, enable fingerprint and set a PIN.
README.md (3)
18-24: Installation: link to Nitro Modules setup and New Architecture enablementReaders may miss required platform setup steps. Link to Nitro Modules docs and note that RN New Architecture must be enabled.
-This library requires `react-native-nitro-modules`. +This library requires `react-native-nitro-modules` and an app configured for the New Architecture. +See Nitro Modules setup guidance and ensure New Architecture is enabled in your project.
28-31: Document build toolchain minimums (AGP/Kotlin/NDK) for Nitro ModulesSpecify Android Gradle Plugin, Kotlin, and (if applicable) NDK/CMakelists minimums known to work with Nitro Modules to preempt build issues.
47-51: Add migration note for breaking API changesPR description mentions breaking changes and a migration pattern. Add a short “Migration” section or link to MIGRATION.md here so users can upgrade smoothly.
I can draft a MIGRATION.md with examples like oldFunction(p1, p2) → newFunction({ p1, p2 }) if helpful.
packages/react-native-biolink/README.md (3)
13-18: Separate package manager commands to avoid users running all of themPresent pnpm, npm, yarn as alternatives, not a single block, to prevent accidental execution of multiple commands.
-```bash -# Install both packages -pnpm add @gmemmy/react-native-biolink react-native-nitro-modules -npm add @gmemmy/react-native-biolink react-native-nitro-modules -yarn add @gmemmy/react-native-biolink react-native-nitro-modules -``` +```bash +# Using pnpm +pnpm add @gmemmy/react-native-biolink react-native-nitro-modules +``` +```bash +# Using npm +npm add @gmemmy/react-native-biolink react-native-nitro-modules +``` +```bash +# Using yarn +yarn add @gmemmy/react-native-biolink react-native-nitro-modules +```
87-92: Wording nit: replace “extremely large” with a neutral phrasingMinor tone/style improvement.
-> **Note**: While the library handles encryption, it's generally recommended to keep stored secrets concise. Avoid storing extremely large data blobs. +> **Note**: While the library handles encryption, it's generally recommended to keep stored secrets concise. Avoid storing very large data blobs.
198-207: Clarify isSigningAvailable() semanticsBriefly document conditions for false (e.g., no Secure Enclave/StrongBox, API level too low, device policy disallows key attestation).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.gitignore(1 hunks)README.md(1 hunks)biolink-demo/README.md(2 hunks)packages/react-native-biolink/.gitignore(1 hunks)packages/react-native-biolink/README.md(9 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- .gitignore
- packages/react-native-biolink/.gitignore
🧰 Additional context used
🪛 LanguageTool
packages/react-native-biolink/README.md
[style] ~91-~91: As an alternative to the over-used intensifier ‘extremely’, consider replacing this phrase.
Context: ...p stored secrets concise. Avoid storing extremely large data blobs. ```typescript await storeS...
(EN_WEAK_ADJECTIVE)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (2)
biolink-demo/README.md (1)
21-27: Built-with versions are accurate and require no changesThe “Built with” versions in
biolink-demo/README.md(React Native 0.80.1 and React 19.1.0) exactly match the demo app’spackage.json. The library’s peerDependencies (>=0.74.0for React Native and>=18.0.0for React) also encompass these versions. No discrepancies found—no update needed.packages/react-native-biolink/README.md (1)
222-222: Ignore broken link warning in README
ALICENSEfile is present in thepackages/react-native-biolink/folder, so the existing[LICENSE](LICENSE)link is correct and does not need to be updated.Likely an incorrect or invalid review comment.
1 participant
📝 Description
A clear and concise description of what this pull request does.
🎯 Type of Change
🔗 Related Issue
Closes #[issue-number]
🧪 Testing
Test Coverage
Platforms Tested
Test Scenarios
📋 Checklist
Code Quality
Documentation
Build & CI
🚀 Breaking Changes
If this PR includes breaking changes, please describe them and provide migration instructions:
What Changed
Migration Guide
📱 Platform-Specific Changes
iOS
Android
🔍 Additional Notes
Any additional information that reviewers should know:
📸 Screenshots/Videos
If applicable, add screenshots or videos to demonstrate the changes.
Note: Please ensure all tests pass and the code follows the project's contribution guidelines. Thank you for your contribution! 🎉
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores
Refactor