create nice structure for the mobile sdk #1177
Conversation
|
You have run out of free Bugbot PR reviews for this billing cycle. This will reset on October 17. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
WalkthroughAdds flow-scoped exports and files to the mobile SDK, moves usePrepareDocumentProof into an onboarding flow, updates build/config and resolution (Metro, TS, Jest), adjusts app imports and a settings source for fcmToken, and exposes MRZ types. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant CBS as ConfirmBelongingScreen
participant Hook as usePrepareDocumentProof (onboarding)
participant Ctx as SelfClient (context)
participant Store as ProvingStore
participant Docs as loadSelectedDocument
U->>CBS: Open screen
CBS->>Hook: call()
Hook->>Ctx: get selfClient
Hook->>Docs: loadSelectedDocument(selfClient)
alt Aadhaar selected
Hook->>Store: init(selfClient, "register")
else Other
Hook->>Store: init(selfClient, "dsc")
end
Note over Hook,Store: On error, fallback init(..., "dsc")
Store-->>Hook: currentState
Hook-->>CBS: { setUserConfirmed, isReadyToProve }
CBS-->>U: Render description via getPreRegistrationDescription()
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🧰 Additional context used📓 Path-based instructions (3)app/package.json📄 CodeRabbit inference engine (.cursor/rules/mobile-sdk-migration.mdc)
Files:
.github/workflows/**/*.{yml,yaml}📄 CodeRabbit inference engine (AGENTS.md)
Files:
app/jest.config.cjs📄 CodeRabbit inference engine (.cursor/rules/mobile-sdk-migration.mdc)
Files:
🧠 Learnings (10)📓 Common learnings📚 Learning: 2025-08-24T18:54:04.809ZApplied to files:
📚 Learning: 2025-09-22T11:10:22.019ZApplied to files:
📚 Learning: 2025-08-24T18:54:04.809ZApplied to files:
📚 Learning: 2025-08-24T18:54:04.809ZApplied to files:
📚 Learning: 2025-08-29T15:31:15.924ZApplied to files:
📚 Learning: 2025-08-29T15:31:15.924ZApplied to files:
📚 Learning: 2025-08-25T14:25:57.586ZApplied to files:
📚 Learning: 2025-09-22T11:10:22.019ZApplied to files:
📚 Learning: 2025-08-29T15:31:15.924ZApplied to files:
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
🔇 Additional comments (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (6)
packages/mobile-sdk-alpha/tsup.config.ts (1)
12-33: Flow discovery implementation looks solid.The dynamic flow file discovery using
node:fsandnode:pathcorrectly:
- Handles missing directories gracefully
- Recursively traverses subdirectories
- Filters for TypeScript files only
- Generates proper entry point mappings
Minor suggestion: Consider filtering out
.test.tsor.spec.tsfiles to avoid accidentally bundling test files:- } else if (item.isFile() && item.name.endsWith('.ts')) { + } else if (item.isFile() && item.name.endsWith('.ts') && !item.name.endsWith('.test.ts') && !item.name.endsWith('.spec.ts')) {packages/mobile-sdk-alpha/src/flows/about.md (1)
1-13: Good documentation, but also update main README.This internal documentation clearly explains the flows structure and conventions. Consider also documenting this new export structure in the main
packages/mobile-sdk-alpha/README.mdfor external consumers.Based on learnings: Document new/updated SDK modules and usage in packages/mobile-sdk-alpha/README.md
The README should include:
- How to import from the new flow-based exports (e.g.,
@selfxyz/mobile-sdk-alpha/onboarding/read-mrz)- Examples of the verb-noun naming convention
- What each flow module typically provides (functions, hooks, components, constants)
packages/mobile-sdk-alpha/src/flows/onboarding/scan-nfc.ts (1)
1-5: packages/mobile-sdk-alpha/src/flows/onboarding/scan-nfc.ts: stub or remove empty file
The license header matches project conventions; since there’s no implementation, either remove the file until NFC scanning is implemented, add a TODO for planned functionality, or include a minimal export (e.g.,export {};) to satisfy the build.app/metro.config.cjs (1)
131-133: Consider async filesystem check for Metro performance.The synchronous
fs.existsSync()call blocks Metro's resolver for each flow import. While Metro's resolver is synchronous by contract, repeated filesystem checks during development can add latency to module resolution.Consider alternatives:
- Pre-build a manifest of available flow modules during SDK build and check against it (preferred for production)
- Cache the existence check results in a Map for the lifetime of the Metro process
- If Metro supports it, investigate whether the resolution can rely on package exports configuration instead of runtime filesystem checks
Example caching approach:
+ const flowPathCache = new Map(); + // Custom resolver to handle Node.js modules and dynamic flow imports if (moduleName.startsWith('@selfxyz/mobile-sdk-alpha/')) { const subPath = moduleName.replace('@selfxyz/mobile-sdk-alpha/', ''); // Check if it's a flow import (onboarding/* or disclosing/*) if ( subPath.startsWith('onboarding/') || subPath.startsWith('disclosing/') ) { const flowPath = path.resolve( sdkAlphaPath, 'dist/esm/flows', `${subPath}.js`, ); - // Check if the file exists - const fs = require('fs'); - if (fs.existsSync(flowPath)) { + // Check cache first, then filesystem + if (!flowPathCache.has(flowPath)) { + const fs = require('fs'); + flowPathCache.set(flowPath, fs.existsSync(flowPath)); + } + + if (flowPathCache.get(flowPath)) { return { type: 'sourceFile', filePath: flowPath, }; } } }packages/mobile-sdk-alpha/src/flows/onboarding/read-mrz.ts (1)
7-9: Consider i18n support for user-facing instruction text.The
mrzReadInstructions()function returns a hardcoded English string. If the app supports multiple locales, this instruction should be internationalized.If internationalization is planned, consider:
- Accepting a locale parameter or accessing a translation context
- Returning a translation key that the consuming UI resolves
- Moving the text to a centralized i18n resource file
Example:
export function mrzReadInstructions(locale?: string) { // Return translation key or use i18n library return t('onboarding.mrz.instructions', locale); }packages/mobile-sdk-alpha/src/flows/onboarding/confirm-ownership.ts (1)
14-16: Consider i18n support for legal certification text.The certification text contains important legal language that should likely be localized for international users. This is user-facing compliance text that may have legal implications in different jurisdictions.
Consider:
- Moving this text to a centralized i18n resource
- Accepting a locale parameter if not using a translation context
- Ensuring legal review for different locales
This is the same pattern as the
mrzReadInstructionsfunction, but more critical given the legal nature of the text.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
app/metro.config.cjs(1 hunks)app/src/screens/prove/ConfirmBelongingScreen.tsx(2 hunks)app/src/utils/cryptoLoader.ts(1 hunks)app/tsconfig.json(1 hunks)packages/mobile-sdk-alpha/package.json(1 hunks)packages/mobile-sdk-alpha/src/browser.ts(1 hunks)packages/mobile-sdk-alpha/src/components/MRZScannerView.tsx(1 hunks)packages/mobile-sdk-alpha/src/context.tsx(1 hunks)packages/mobile-sdk-alpha/src/flows/about.md(1 hunks)packages/mobile-sdk-alpha/src/flows/onboarding/confirm-ownership.ts(1 hunks)packages/mobile-sdk-alpha/src/flows/onboarding/read-mrz.ts(1 hunks)packages/mobile-sdk-alpha/src/flows/onboarding/scan-nfc.ts(1 hunks)packages/mobile-sdk-alpha/src/index.ts(1 hunks)packages/mobile-sdk-alpha/tsup.config.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
packages/mobile-sdk-alpha/**/*.{ts,tsx}
📄 CodeRabbit inference engine (packages/mobile-sdk-alpha/AGENTS.md)
packages/mobile-sdk-alpha/**/*.{ts,tsx}: Use strict TypeScript type checking across the codebase
Follow ESLint TypeScript-specific rules
Avoid introducing circular dependencies
Files:
packages/mobile-sdk-alpha/src/components/MRZScannerView.tsxpackages/mobile-sdk-alpha/src/browser.tspackages/mobile-sdk-alpha/tsup.config.tspackages/mobile-sdk-alpha/src/flows/onboarding/read-mrz.tspackages/mobile-sdk-alpha/src/index.tspackages/mobile-sdk-alpha/src/context.tsxpackages/mobile-sdk-alpha/src/flows/onboarding/confirm-ownership.tspackages/mobile-sdk-alpha/src/flows/onboarding/scan-nfc.ts
**/*.{js,ts,tsx,jsx,sol,nr}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,ts,tsx,jsx,sol,nr}: NEVER log sensitive data including PII (names, DOB, passport numbers, addresses), credentials, tokens, API keys, private keys, or session identifiers.
ALWAYS redact/mask sensitive fields in logs using consistent patterns (e.g.,***-***-1234for passport numbers,J*** D***for names).
Files:
packages/mobile-sdk-alpha/src/components/MRZScannerView.tsxpackages/mobile-sdk-alpha/src/browser.tspackages/mobile-sdk-alpha/tsup.config.tspackages/mobile-sdk-alpha/src/flows/onboarding/read-mrz.tspackages/mobile-sdk-alpha/src/index.tspackages/mobile-sdk-alpha/src/context.tsxpackages/mobile-sdk-alpha/src/flows/onboarding/confirm-ownership.tspackages/mobile-sdk-alpha/src/flows/onboarding/scan-nfc.tsapp/src/utils/cryptoLoader.tsapp/src/screens/prove/ConfirmBelongingScreen.tsx
packages/mobile-sdk-alpha/**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
packages/mobile-sdk-alpha/**/*.{ts,tsx,js,jsx}: Review alpha mobile SDK code for:
- API consistency with core SDK
- Platform-neutral abstractions
- Performance considerations
- Clear experimental notes or TODOs
Files:
packages/mobile-sdk-alpha/src/components/MRZScannerView.tsxpackages/mobile-sdk-alpha/src/browser.tspackages/mobile-sdk-alpha/tsup.config.tspackages/mobile-sdk-alpha/src/flows/onboarding/read-mrz.tspackages/mobile-sdk-alpha/src/index.tspackages/mobile-sdk-alpha/src/context.tsxpackages/mobile-sdk-alpha/src/flows/onboarding/confirm-ownership.tspackages/mobile-sdk-alpha/src/flows/onboarding/scan-nfc.ts
packages/mobile-sdk-alpha/src/index.ts
📄 CodeRabbit inference engine (.cursor/rules/mobile-sdk-migration.mdc)
Re-export new SDK modules via packages/mobile-sdk-alpha/src/index.ts
Files:
packages/mobile-sdk-alpha/src/index.ts
app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (app/AGENTS.md)
Type checking must pass before PRs (yarn types)
Files:
app/src/utils/cryptoLoader.tsapp/src/screens/prove/ConfirmBelongingScreen.tsx
app/src/**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
app/src/**/*.{ts,tsx,js,jsx}: Review React Native TypeScript code for:
- Component architecture and reusability
- State management patterns
- Performance optimizations
- TypeScript type safety
- React hooks usage and dependencies
- Navigation patterns
Files:
app/src/utils/cryptoLoader.tsapp/src/screens/prove/ConfirmBelongingScreen.tsx
packages/mobile-sdk-alpha/package.json
📄 CodeRabbit inference engine (.cursor/rules/mobile-sdk-migration.mdc)
packages/mobile-sdk-alpha/package.json: Expose a 'test:build' script in the SDK's package.json that runs build, test, types, and lint
Enable tree shaking for the SDK (e.g., ensure 'sideEffects' is correctly set in package.json and exports are ESM-friendly)
Files:
packages/mobile-sdk-alpha/package.json
packages/mobile-sdk-alpha/**/package.json
📄 CodeRabbit inference engine (packages/mobile-sdk-alpha/AGENTS.md)
packages/mobile-sdk-alpha/**/package.json: Ensure package exports are properly configured
Verify package conditions are valid (e.g., exports conditions)
Files:
packages/mobile-sdk-alpha/package.json
🧠 Learnings (15)
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Verify extractMRZInfo() using published sample MRZ strings (e.g., ICAO examples)
Applied to files:
packages/mobile-sdk-alpha/src/components/MRZScannerView.tsxpackages/mobile-sdk-alpha/src/flows/onboarding/read-mrz.ts
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/src/index.ts : Re-export new SDK modules via packages/mobile-sdk-alpha/src/index.ts
Applied to files:
packages/mobile-sdk-alpha/src/components/MRZScannerView.tsxpackages/mobile-sdk-alpha/src/browser.tspackages/mobile-sdk-alpha/src/index.tsapp/tsconfig.jsonpackages/mobile-sdk-alpha/package.json
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Use actual imports from selfxyz/mobile-sdk-alpha in tests
Applied to files:
packages/mobile-sdk-alpha/src/browser.tsapp/tsconfig.jsonapp/metro.config.cjs
📚 Learning: 2025-08-26T14:49:11.190Z
Learnt from: shazarre
PR: selfxyz/self#936
File: app/src/screens/passport/PassportNFCScanScreen.tsx:28-31
Timestamp: 2025-08-26T14:49:11.190Z
Learning: SelfClientProvider is wrapped in app/App.tsx, providing context for useSelfClient() hook usage throughout the React Native app navigation stacks.
Applied to files:
packages/mobile-sdk-alpha/src/browser.tspackages/mobile-sdk-alpha/src/index.tspackages/mobile-sdk-alpha/src/context.tsx
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/src/processing/** : Place MRZ processing helpers in packages/mobile-sdk-alpha/src/processing/
Applied to files:
packages/mobile-sdk-alpha/src/flows/onboarding/read-mrz.ts
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/README.md : Document new/updated SDK modules and usage in packages/mobile-sdk-alpha/README.md
Applied to files:
packages/mobile-sdk-alpha/src/flows/about.mdpackages/mobile-sdk-alpha/package.json
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Ensure parseNFCResponse() works with representative, synthetic NFC data
Applied to files:
packages/mobile-sdk-alpha/src/flows/onboarding/scan-nfc.ts
📚 Learning: 2025-08-25T14:25:57.586Z
Learnt from: aaronmgdr
PR: selfxyz/self#951
File: app/src/providers/authProvider.web.tsx:17-18
Timestamp: 2025-08-25T14:25:57.586Z
Learning: The selfxyz/mobile-sdk-alpha/constants/analytics import path is properly configured with SDK exports, Metro aliases, and TypeScript resolution. Import changes from @/consts/analytics to this path are part of valid analytics migration, not TypeScript resolution issues.
Applied to files:
app/tsconfig.jsonapp/metro.config.cjs
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.{ts,tsx} : Avoid introducing circular dependencies
Applied to files:
app/tsconfig.jsonpackages/mobile-sdk-alpha/package.json
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.{ts,tsx} : Use strict TypeScript type checking across the codebase
Applied to files:
app/tsconfig.json
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.{ts,tsx} : Follow ESLint TypeScript-specific rules
Applied to files:
app/tsconfig.json
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Do NOT mock selfxyz/mobile-sdk-alpha in tests (avoid jest.mock('selfxyz/mobile-sdk-alpha') and replacing real functions with mocks)
Applied to files:
app/tsconfig.json
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/**/package.json : Ensure package exports are properly configured
Applied to files:
app/tsconfig.jsonpackages/mobile-sdk-alpha/package.json
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/package.json : Enable tree shaking for the SDK (e.g., ensure 'sideEffects' is correctly set in package.json and exports are ESM-friendly)
Applied to files:
app/tsconfig.jsonpackages/mobile-sdk-alpha/package.json
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/**/package.json : Verify package conditions are valid (e.g., exports conditions)
Applied to files:
packages/mobile-sdk-alpha/package.json
🧬 Code graph analysis (4)
packages/mobile-sdk-alpha/src/components/MRZScannerView.tsx (1)
packages/mobile-sdk-alpha/src/flows/onboarding/read-mrz.ts (1)
MRZScannerViewProps(6-6)
packages/mobile-sdk-alpha/tsup.config.ts (4)
app/metro.config.cjs (3)
basePath(201-201)fs(132-132)path(6-6)scripts/check-duplicate-headers.cjs (2)
fs(6-6)path(7-7)packages/mobile-sdk-alpha/react-native.config.cjs (2)
fs(6-6)path(5-5)scripts/check-license-headers.mjs (1)
relativePath(98-98)
packages/mobile-sdk-alpha/src/flows/onboarding/confirm-ownership.ts (2)
packages/mobile-sdk-alpha/src/context.tsx (1)
useSelfClient(64-68)packages/mobile-sdk-alpha/src/proving/provingMachine.ts (1)
useProvingStore(367-1417)
app/src/screens/prove/ConfirmBelongingScreen.tsx (1)
packages/mobile-sdk-alpha/src/flows/onboarding/confirm-ownership.ts (1)
getPreRegistrationDescription(14-16)
🪛 GitHub Actions: Workspace CI
packages/mobile-sdk-alpha/src/flows/onboarding/read-mrz.ts
[error] 1-1: License header missing or incorrect in file. yarn lint reported license header issues.
packages/mobile-sdk-alpha/src/flows/onboarding/confirm-ownership.ts
[error] 1-1: License header missing or incorrect in file. yarn lint reported license header issues.
packages/mobile-sdk-alpha/src/flows/onboarding/scan-nfc.ts
[error] 1-1: License header missing or incorrect in file. yarn lint reported license header issues.
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build-deps
- GitHub Check: e2e-ios
- GitHub Check: android-build-test
- GitHub Check: analyze-ios
- GitHub Check: analyze-android
🔇 Additional comments (3)
packages/mobile-sdk-alpha/tsup.config.ts (1)
35-43: Flow entries merged correctly into build config.The spread operation integrates discovered flow entries cleanly into the main entry object, enabling automatic inclusion of new flow modules without manual configuration changes.
app/src/utils/cryptoLoader.ts (1)
14-16: loadProvingUtils has no callers
Search across all .ts/.tsx files found only its own declaration and no destructuring or indexing of its return value. No downstream updates required.app/src/screens/prove/ConfirmBelongingScreen.tsx (1)
11-19: LGTM! Clean migration to flow-based SDK structure.The import refactoring successfully migrates to the new onboarding flow module while maintaining the same functionality. The use of
getPreRegistrationDescription()for dynamic text at line 113 aligns well with the new API design.
packages/mobile-sdk-alpha/src/flows/onboarding/confirm-ownership.ts
Outdated
Show resolved
Hide resolved
| useEffect(() => { | ||
| const initializeProving = async () => { | ||
| try { | ||
| const selectedDocument = await loadSelectedDocument(selfClient); | ||
| if (selectedDocument?.data?.documentCategory === 'aadhaar') { | ||
| init(selfClient, 'register'); | ||
| } else { | ||
| init(selfClient, 'dsc'); | ||
| } | ||
| } catch (error) { | ||
| console.error('Error loading selected document:', error); | ||
| init(selfClient, 'dsc'); | ||
| } | ||
| }; | ||
|
|
||
| initializeProving(); | ||
| }, [init, selfClient]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Verify proving initialization is idempotent and consider cleanup.
The useEffect unconditionally calls init() on every mount. If the component remounts (e.g., due to navigation), this could reinitialize the proving state machine unnecessarily.
Verify the following:
- Is
init()idempotent? Does it safely handle being called multiple times? - Should initialization be guarded by checking
currentState? - Does the proving store need cleanup if the component unmounts during initialization?
Based on the proving store summary showing that init "stops previous actor" and "resets state", repeated calls should be safe, but it's worth confirming:
🏁 Script executed:
#!/bin/bash
# Check if init() handles multiple calls safely and if there's cleanup logic
# Look for idempotency guards or state checks in init
ast-grep --pattern $'init($$$) {
$$$
if ($$$currentState$$$) {
$$$
}
$$$
}'
# Check for cleanup patterns in proving store
rg -nP -A5 'useEffect.*return.*\(\)' packages/mobile-sdk-alpha/src/proving/
# Check if init stops previous operations
rg -nP -C3 'stop.*actor|cleanup|cancel' packages/mobile-sdk-alpha/src/proving/provingMachine.tsLength of output: 178
🏁 Script executed:
#!/bin/bash
# Locate init implementation in proving store
rg -nP -C3 'init\s*\(' packages/mobile-sdk-alpha/src/proving
# Search for any teardown or cleanup functions
rg -nP 'deinit|clear|destroy|abort' -n packages/mobile-sdk-alpha/src/provingLength of output: 1391
Ensure cleanup on unmount and reduce redundant init
- init() already stops any previous actor, so calling it multiple times is safe.
- In packages/mobile-sdk-alpha/src/flows/onboarding/confirm-ownership.ts, return a cleanup function from the useEffect to stop the proving actor on unmount and avoid orphaned processes.
- Optionally check the proving machine’s currentState before calling init() to skip redundant initialization.
🤖 Prompt for AI Agents
In packages/mobile-sdk-alpha/src/flows/onboarding/confirm-ownership.ts around
lines 36 to 52, the useEffect starts the proving actor but doesn’t stop it on
unmount and may call init redundantly; modify the effect to return a cleanup
function that stops the proving actor (call the actor stop/terminate method or
init with a stop action) when the component unmounts, and before calling init
inside initializeProving check the proving machine’s currentState (or a flag) to
skip calling init if it’s already in the desired state to avoid redundant
initialization.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/mobile-sdk-alpha/src/flows/onboarding/confirm-ownership.ts (1)
35-51: Add cleanup to prevent orphaned proving actor on unmount.If the component unmounts during async initialization or after calling
init(), the proving actor is not stopped. This can lead to orphaned processes and resource leaks.Return a cleanup function from the
useEffect:useEffect(() => { const initializeProving = async () => { try { const selectedDocument = await loadSelectedDocument(selfClient); if (selectedDocument?.data?.documentCategory === 'aadhaar') { init(selfClient, 'register'); } else { init(selfClient, 'dsc'); } } catch (error) { - console.error('Error loading selected document:', error); + console.error('Error loading selected document: [redacted]'); init(selfClient, 'dsc'); } }; initializeProving(); + + return () => { + // Stop the proving actor on unmount + const actor = useProvingStore.getState().actor; + if (actor) { + actor.stop(); + } + }; }, [init, selfClient]);Based on learnings
🧹 Nitpick comments (3)
packages/mobile-sdk-alpha/src/flows/onboarding/read-mrz.ts (1)
6-8: Consider i18n support for user-facing instruction text.The hardcoded English string limits accessibility for non-English users. If the SDK targets international markets, consider accepting a locale parameter or externalizing the string to support localization.
packages/mobile-sdk-alpha/src/flows/onboarding/confirm-ownership.ts (2)
13-15: Consider i18n support for legal/compliance text.This certification message is hardcoded in English and may require localization or region-specific legal language for different markets. Consider externalizing this text to support internationalization.
53-53: Bind selfClient to setUserConfirmed in the hook.Consumers currently call setUserConfirmed(selfClient) (ConfirmBelongingScreen.tsx; ProveScreen.tsx). Wrap the store action in the hook to auto-pass selfClient (so callers can call setUserConfirmed()) to simplify the API surface.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/mobile-sdk-alpha/src/flows/onboarding/confirm-ownership.ts(1 hunks)packages/mobile-sdk-alpha/src/flows/onboarding/read-mrz.ts(1 hunks)packages/mobile-sdk-alpha/src/flows/onboarding/scan-nfc.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/mobile-sdk-alpha/src/flows/onboarding/scan-nfc.ts
🧰 Additional context used
📓 Path-based instructions (3)
packages/mobile-sdk-alpha/**/*.{ts,tsx}
📄 CodeRabbit inference engine (packages/mobile-sdk-alpha/AGENTS.md)
packages/mobile-sdk-alpha/**/*.{ts,tsx}: Use strict TypeScript type checking across the codebase
Follow ESLint TypeScript-specific rules
Avoid introducing circular dependencies
Files:
packages/mobile-sdk-alpha/src/flows/onboarding/read-mrz.tspackages/mobile-sdk-alpha/src/flows/onboarding/confirm-ownership.ts
**/*.{js,ts,tsx,jsx,sol,nr}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,ts,tsx,jsx,sol,nr}: NEVER log sensitive data including PII (names, DOB, passport numbers, addresses), credentials, tokens, API keys, private keys, or session identifiers.
ALWAYS redact/mask sensitive fields in logs using consistent patterns (e.g.,***-***-1234for passport numbers,J*** D***for names).
Files:
packages/mobile-sdk-alpha/src/flows/onboarding/read-mrz.tspackages/mobile-sdk-alpha/src/flows/onboarding/confirm-ownership.ts
packages/mobile-sdk-alpha/**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
packages/mobile-sdk-alpha/**/*.{ts,tsx,js,jsx}: Review alpha mobile SDK code for:
- API consistency with core SDK
- Platform-neutral abstractions
- Performance considerations
- Clear experimental notes or TODOs
Files:
packages/mobile-sdk-alpha/src/flows/onboarding/read-mrz.tspackages/mobile-sdk-alpha/src/flows/onboarding/confirm-ownership.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Use actual imports from selfxyz/mobile-sdk-alpha in tests
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/src/index.ts : Re-export new SDK modules via packages/mobile-sdk-alpha/src/index.ts
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Update the app to consume the mobile-sdk-alpha package and replace local modules with SDK imports
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Verify extractMRZInfo() using published sample MRZ strings (e.g., ICAO examples)
Applied to files:
packages/mobile-sdk-alpha/src/flows/onboarding/read-mrz.ts
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/src/processing/** : Place MRZ processing helpers in packages/mobile-sdk-alpha/src/processing/
Applied to files:
packages/mobile-sdk-alpha/src/flows/onboarding/read-mrz.ts
🧬 Code graph analysis (1)
packages/mobile-sdk-alpha/src/flows/onboarding/confirm-ownership.ts (2)
packages/mobile-sdk-alpha/src/context.tsx (1)
useSelfClient(64-68)packages/mobile-sdk-alpha/src/proving/provingMachine.ts (1)
useProvingStore(367-1417)
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: test
- GitHub Check: build-android
- GitHub Check: build-ios
- GitHub Check: android-build-test
- GitHub Check: e2e-ios
packages/mobile-sdk-alpha/src/flows/onboarding/confirm-identification.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/metro.config.cjs(2 hunks)packages/mobile-sdk-alpha/src/flows/about.md(1 hunks)packages/mobile-sdk-alpha/src/flows/disclosing/await-verification.ts(1 hunks)packages/mobile-sdk-alpha/src/flows/disclosing/confirm-selection.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- packages/mobile-sdk-alpha/src/flows/disclosing/await-verification.ts
- packages/mobile-sdk-alpha/src/flows/disclosing/confirm-selection.ts
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2025-08-25T14:25:57.586Z
Learnt from: aaronmgdr
PR: selfxyz/self#951
File: app/src/providers/authProvider.web.tsx:17-18
Timestamp: 2025-08-25T14:25:57.586Z
Learning: The selfxyz/mobile-sdk-alpha/constants/analytics import path is properly configured with SDK exports, Metro aliases, and TypeScript resolution. Import changes from @/consts/analytics to this path are part of valid analytics migration, not TypeScript resolution issues.
Applied to files:
app/metro.config.cjs
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Use actual imports from selfxyz/mobile-sdk-alpha in tests
Applied to files:
app/metro.config.cjs
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.{ts,tsx} : Avoid introducing circular dependencies
Applied to files:
app/metro.config.cjs
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/src/index.ts : Re-export new SDK modules via packages/mobile-sdk-alpha/src/index.ts
Applied to files:
app/metro.config.cjs
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/**/package.json : Ensure package exports are properly configured
Applied to files:
app/metro.config.cjs
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/README.md : Document new/updated SDK modules and usage in packages/mobile-sdk-alpha/README.md
Applied to files:
packages/mobile-sdk-alpha/src/flows/about.md
🧬 Code graph analysis (1)
app/metro.config.cjs (3)
scripts/check-duplicate-headers.cjs (2)
fs(6-6)require(8-8)app/scripts/bundle-analyze-ci.cjs (3)
require(6-6)require(7-7)require(9-9)app/scripts/version.cjs (2)
require(6-6)require(7-7)
🪛 GitHub Check: workspace-lint
app/metro.config.cjs
[warning] 118-118:
Insert ,
[warning] 116-116:
Delete ·
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build-deps
- GitHub Check: analyze-android
- GitHub Check: analyze-ios
- GitHub Check: e2e-ios
- GitHub Check: android-build-test
🔇 Additional comments (2)
app/metro.config.cjs (2)
7-7: LGTM: fs import added for file existence checks.The import is properly scoped to node:fs and supports the new flow resolution logic.
116-119: sdkAlphaPath definition resolves previous critical issue.The variable is now properly defined, addressing the past review concern. However, please address the static analysis formatting hints: remove the trailing space on line 116 and add a comma after line 118.
aaronmgdr
force-pushed
the
aaronmgdr/sdk-nice-exports
branch
from
025074b to
f75eb88
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/screens/prove/ConfirmBelongingScreen.tsx (1)
74-83: Redact error details in logs and analytics to avoid PII leakage.Do not log or emit raw error objects/messages; send sanitized codes.
Apply this diff:
- } catch (error: unknown) { - console.error('Error initializing proving process:', error); - const message = error instanceof Error ? error.message : 'Unknown error'; - trackEvent(ProofEvents.PROVING_PROCESS_ERROR, { - error: message, - }); - trackNfcEvent(ProofEvents.PROVING_PROCESS_ERROR, { - error: message, - }); + } catch (error: unknown) { + // Avoid logging PII-bearing error details + console.error('Error initializing proving process: [redacted]'); + const code = + error instanceof Error ? error.name || 'Error' : 'UnknownError'; + trackEvent(ProofEvents.PROVING_PROCESS_ERROR, { error_code: code }); + trackNfcEvent(ProofEvents.PROVING_PROCESS_ERROR, { error_code: code });As per coding guidelines.
🧹 Nitpick comments (1)
packages/mobile-sdk-alpha/src/flows/about.md (1)
7-13: File naming convention doesn't match all examples.The documentation states the convention is "verb-noun.ts", but "confirm-ownership" (line 12) is "verb-noun" where "ownership" is a noun, not a verb-noun pair. Consider clarifying whether the pattern should be more flexible (e.g., "action-target.ts").
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
app/metro.config.cjs(2 hunks)app/src/screens/prove/ConfirmBelongingScreen.tsx(2 hunks)app/src/screens/system/Loading.tsx(2 hunks)app/tsconfig.json(1 hunks)packages/mobile-sdk-alpha/package.json(1 hunks)packages/mobile-sdk-alpha/src/browser.ts(1 hunks)packages/mobile-sdk-alpha/src/components/MRZScannerView.tsx(1 hunks)packages/mobile-sdk-alpha/src/context.tsx(1 hunks)packages/mobile-sdk-alpha/src/flows/about.md(1 hunks)packages/mobile-sdk-alpha/src/flows/disclosing/await-verification.ts(1 hunks)packages/mobile-sdk-alpha/src/flows/disclosing/confirm-selection.ts(1 hunks)packages/mobile-sdk-alpha/src/flows/disclosing/scan-qr-code.ts(1 hunks)packages/mobile-sdk-alpha/src/flows/onboarding/confirm-identification.ts(1 hunks)packages/mobile-sdk-alpha/src/flows/onboarding/read-mrz.ts(1 hunks)packages/mobile-sdk-alpha/src/flows/onboarding/scan-nfc.ts(1 hunks)packages/mobile-sdk-alpha/src/index.ts(1 hunks)packages/mobile-sdk-alpha/tsup.config.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- packages/mobile-sdk-alpha/src/flows/disclosing/scan-qr-code.ts
- packages/mobile-sdk-alpha/src/flows/disclosing/confirm-selection.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/mobile-sdk-alpha/package.json
- packages/mobile-sdk-alpha/src/browser.ts
- packages/mobile-sdk-alpha/src/flows/onboarding/read-mrz.ts
- packages/mobile-sdk-alpha/src/flows/disclosing/await-verification.ts
- packages/mobile-sdk-alpha/src/index.ts
🧰 Additional context used
📓 Path-based instructions (5)
packages/mobile-sdk-alpha/**/*.{ts,tsx}
📄 CodeRabbit inference engine (packages/mobile-sdk-alpha/AGENTS.md)
packages/mobile-sdk-alpha/**/*.{ts,tsx}: Use strict TypeScript type checking across the codebase
Follow ESLint TypeScript-specific rules
Avoid introducing circular dependencies
Files:
packages/mobile-sdk-alpha/src/components/MRZScannerView.tsxpackages/mobile-sdk-alpha/tsup.config.tspackages/mobile-sdk-alpha/src/flows/onboarding/confirm-identification.tspackages/mobile-sdk-alpha/src/context.tsxpackages/mobile-sdk-alpha/src/flows/onboarding/scan-nfc.ts
**/*.{js,ts,tsx,jsx,sol,nr}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,ts,tsx,jsx,sol,nr}: NEVER log sensitive data including PII (names, DOB, passport numbers, addresses), credentials, tokens, API keys, private keys, or session identifiers.
ALWAYS redact/mask sensitive fields in logs using consistent patterns (e.g.,***-***-1234for passport numbers,J*** D***for names).
Files:
packages/mobile-sdk-alpha/src/components/MRZScannerView.tsxapp/src/screens/system/Loading.tsxpackages/mobile-sdk-alpha/tsup.config.tspackages/mobile-sdk-alpha/src/flows/onboarding/confirm-identification.tspackages/mobile-sdk-alpha/src/context.tsxapp/src/screens/prove/ConfirmBelongingScreen.tsxpackages/mobile-sdk-alpha/src/flows/onboarding/scan-nfc.ts
packages/mobile-sdk-alpha/**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
packages/mobile-sdk-alpha/**/*.{ts,tsx,js,jsx}: Review alpha mobile SDK code for:
- API consistency with core SDK
- Platform-neutral abstractions
- Performance considerations
- Clear experimental notes or TODOs
Files:
packages/mobile-sdk-alpha/src/components/MRZScannerView.tsxpackages/mobile-sdk-alpha/tsup.config.tspackages/mobile-sdk-alpha/src/flows/onboarding/confirm-identification.tspackages/mobile-sdk-alpha/src/context.tsxpackages/mobile-sdk-alpha/src/flows/onboarding/scan-nfc.ts
app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (app/AGENTS.md)
Type checking must pass before PRs (yarn types)
Files:
app/src/screens/system/Loading.tsxapp/src/screens/prove/ConfirmBelongingScreen.tsx
app/src/**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
app/src/**/*.{ts,tsx,js,jsx}: Review React Native TypeScript code for:
- Component architecture and reusability
- State management patterns
- Performance optimizations
- TypeScript type safety
- React hooks usage and dependencies
- Navigation patterns
Files:
app/src/screens/system/Loading.tsxapp/src/screens/prove/ConfirmBelongingScreen.tsx
🧠 Learnings (15)
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Verify extractMRZInfo() using published sample MRZ strings (e.g., ICAO examples)
Applied to files:
packages/mobile-sdk-alpha/src/components/MRZScannerView.tsx
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/src/index.ts : Re-export new SDK modules via packages/mobile-sdk-alpha/src/index.ts
Applied to files:
packages/mobile-sdk-alpha/src/components/MRZScannerView.tsxapp/tsconfig.json
📚 Learning: 2025-09-22T11:10:22.019Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Applies to **/*.{js,ts,tsx,jsx,sol,nr} : NEVER log sensitive data including PII (names, DOB, passport numbers, addresses), credentials, tokens, API keys, private keys, or session identifiers.
Applied to files:
packages/mobile-sdk-alpha/src/flows/onboarding/confirm-identification.ts
📚 Learning: 2025-09-22T11:10:22.019Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Applies to **/*.{js,ts,tsx,jsx,sol,nr} : ALWAYS redact/mask sensitive fields in logs using consistent patterns (e.g., `***-***-1234` for passport numbers, `J*** D***` for names).
Applied to files:
packages/mobile-sdk-alpha/src/flows/onboarding/confirm-identification.ts
📚 Learning: 2025-08-26T14:49:11.190Z
Learnt from: shazarre
PR: selfxyz/self#936
File: app/src/screens/passport/PassportNFCScanScreen.tsx:28-31
Timestamp: 2025-08-26T14:49:11.190Z
Learning: SelfClientProvider is wrapped in app/App.tsx, providing context for useSelfClient() hook usage throughout the React Native app navigation stacks.
Applied to files:
packages/mobile-sdk-alpha/src/context.tsx
📚 Learning: 2025-08-25T14:25:57.586Z
Learnt from: aaronmgdr
PR: selfxyz/self#951
File: app/src/providers/authProvider.web.tsx:17-18
Timestamp: 2025-08-25T14:25:57.586Z
Learning: The selfxyz/mobile-sdk-alpha/constants/analytics import path is properly configured with SDK exports, Metro aliases, and TypeScript resolution. Import changes from @/consts/analytics to this path are part of valid analytics migration, not TypeScript resolution issues.
Applied to files:
app/tsconfig.jsonapp/metro.config.cjs
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Use actual imports from selfxyz/mobile-sdk-alpha in tests
Applied to files:
app/tsconfig.jsonapp/metro.config.cjs
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.{ts,tsx} : Avoid introducing circular dependencies
Applied to files:
app/tsconfig.jsonapp/metro.config.cjs
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.{ts,tsx} : Use strict TypeScript type checking across the codebase
Applied to files:
app/tsconfig.json
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Do NOT mock selfxyz/mobile-sdk-alpha in tests (avoid jest.mock('selfxyz/mobile-sdk-alpha') and replacing real functions with mocks)
Applied to files:
app/tsconfig.json
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.{ts,tsx} : Follow ESLint TypeScript-specific rules
Applied to files:
app/tsconfig.json
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/**/package.json : Ensure package exports are properly configured
Applied to files:
app/tsconfig.json
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/package.json : Enable tree shaking for the SDK (e.g., ensure 'sideEffects' is correctly set in package.json and exports are ESM-friendly)
Applied to files:
app/tsconfig.json
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Ensure parseNFCResponse() works with representative, synthetic NFC data
Applied to files:
packages/mobile-sdk-alpha/src/flows/onboarding/scan-nfc.ts
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/README.md : Document new/updated SDK modules and usage in packages/mobile-sdk-alpha/README.md
Applied to files:
packages/mobile-sdk-alpha/src/flows/about.md
🧬 Code graph analysis (5)
packages/mobile-sdk-alpha/src/components/MRZScannerView.tsx (1)
packages/mobile-sdk-alpha/src/flows/onboarding/read-mrz.ts (1)
MRZScannerViewProps(5-5)
app/src/screens/system/Loading.tsx (1)
app/src/stores/settingStore.ts (1)
useSettingStore(37-95)
packages/mobile-sdk-alpha/tsup.config.ts (4)
app/metro.config.cjs (2)
fs(7-7)path(6-6)scripts/check-duplicate-headers.cjs (2)
fs(6-6)path(7-7)packages/mobile-sdk-alpha/react-native.config.cjs (2)
fs(6-6)path(5-5)scripts/check-license-headers.mjs (1)
relativePath(98-98)
packages/mobile-sdk-alpha/src/flows/onboarding/confirm-identification.ts (1)
packages/mobile-sdk-alpha/src/proving/provingMachine.ts (1)
useProvingStore(367-1417)
app/src/screens/prove/ConfirmBelongingScreen.tsx (1)
packages/mobile-sdk-alpha/src/flows/onboarding/confirm-identification.ts (1)
getPreRegistrationDescription(13-15)
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: e2e-ios
- GitHub Check: analyze-ios
- GitHub Check: analyze-android
- GitHub Check: build-deps
🔇 Additional comments (7)
packages/mobile-sdk-alpha/src/components/MRZScannerView.tsx (1)
45-52: LGTM! Export aligns with SDK structure.Making
MRZScannerViewPropspublic is appropriate for the onboarding flow exports (@selfxyz/mobile-sdk-alpha/onboarding/read-mrz). The interface is well-typed and follows React Native conventions. Note that consumers will receive PII via theonMRZDetectedcallback—ensure downstream usage includes proper PII handling and avoids logging sensitive fields.app/src/screens/system/Loading.tsx (1)
23-23: LGTM! fcmToken source migration is clean.The refactor moving
fcmTokenfromuseProvingStoretouseSettingStoreproperly separates concerns—settings belong in the settings store rather than the proving store.Also applies to: 64-64
packages/mobile-sdk-alpha/src/flows/onboarding/scan-nfc.ts (1)
1-4: LGTM! Placeholder file for NFC flow.This establishes the module path for the onboarding NFC scanning flow, consistent with the PR's objective to create a structured export interface for the mobile SDK.
app/tsconfig.json (1)
17-22: Path alias alignment verified: TypeScript aliases, Metro resolver, and package.json exports for onboarding and disclosing flows consistently map to thedist/esm/flowsoutputs.packages/mobile-sdk-alpha/src/context.tsx (1)
5-5: All usePrepareDocumentProof imports have been updated.
No stale imports remain from the old context module; every usage now imports fromonboarding/confirm-identification.packages/mobile-sdk-alpha/tsup.config.ts (1)
35-43: Confirm package.json “exports” maps flows/ to dist outputs.*Since entries add flows/* bundles, package.json should expose "exports": { "./onboarding/": "./dist/esm/flows/onboarding/.js", "./disclosing/*": ... } to work outside Metro.
Would you like a follow-up diff for the package.json exports?
packages/mobile-sdk-alpha/src/flows/onboarding/confirm-identification.ts (1)
27-34: Confirm hook stability: avoid re-initializing on changing store selectors.If useProvingStore returns new function identities, the effect may rerun and re-init. Verify init/selfClient identities are stable or wrap with useCallback in the store to ensure referential stability.
I can generate a quick script to scan for init creation sites in the proving store if helpful.
| // Custom resolver to handle Node.js modules and dynamic flow imports | ||
| if (moduleName.startsWith('@selfxyz/mobile-sdk-alpha/')) { | ||
| const subPath = moduleName.replace('@selfxyz/mobile-sdk-alpha/', ''); | ||
|
|
||
| // Check if it's a flow import (onboarding/* or disclosing/*) | ||
| if ( | ||
| subPath.startsWith('onboarding/') || | ||
| subPath.startsWith('disclosing/') | ||
| ) { | ||
| const flowPath = path.resolve( | ||
| sdkAlphaPath, | ||
| 'dist/esm/flows', | ||
| `${subPath}.js`, | ||
| ); | ||
|
|
||
| // Check if the file exists | ||
| if (fs.existsSync(flowPath)) { | ||
| return { | ||
| type: 'sourceFile', | ||
| filePath: flowPath, | ||
| }; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Harden flow resolution: add try/catch and index.js fallback; always delegate on miss.
Prevent resolver breakage and support folder indexes.
Apply this diff:
if (moduleName.startsWith('@selfxyz/mobile-sdk-alpha/')) {
const subPath = moduleName.replace('@selfxyz/mobile-sdk-alpha/', '');
// Check if it's a flow import (onboarding/* or disclosing/*)
if (
subPath.startsWith('onboarding/') ||
subPath.startsWith('disclosing/')
) {
- const flowPath = path.resolve(
- sdkAlphaPath,
- 'dist/esm/flows',
- `${subPath}.js`,
- );
-
- // Check if the file exists
- if (fs.existsSync(flowPath)) {
- return {
- type: 'sourceFile',
- filePath: flowPath,
- };
- }
+ try {
+ const flowPath = path.resolve(
+ sdkAlphaPath,
+ 'dist/esm/flows',
+ `${subPath}.js`,
+ );
+ const flowIndexPath = path.resolve(
+ sdkAlphaPath,
+ 'dist/esm/flows',
+ subPath,
+ 'index.js',
+ );
+ if (fs.existsSync(flowPath)) {
+ return { type: 'sourceFile', filePath: flowPath };
+ }
+ if (fs.existsSync(flowIndexPath)) {
+ return { type: 'sourceFile', filePath: flowIndexPath };
+ }
+ } catch (e) {
+ console.warn(`Flow resolve error for ${moduleName}:`, e?.message ?? e);
+ }
+ // Delegate to default resolution on miss
+ return context.resolveRequest(context, moduleName, platform);
}
}This avoids hard failures and supports directory-based flows (…/foo/index.js).
📝 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.
| // Custom resolver to handle Node.js modules and dynamic flow imports | |
| if (moduleName.startsWith('@selfxyz/mobile-sdk-alpha/')) { | |
| const subPath = moduleName.replace('@selfxyz/mobile-sdk-alpha/', ''); | |
| // Check if it's a flow import (onboarding/* or disclosing/*) | |
| if ( | |
| subPath.startsWith('onboarding/') || | |
| subPath.startsWith('disclosing/') | |
| ) { | |
| const flowPath = path.resolve( | |
| sdkAlphaPath, | |
| 'dist/esm/flows', | |
| `${subPath}.js`, | |
| ); | |
| // Check if the file exists | |
| if (fs.existsSync(flowPath)) { | |
| return { | |
| type: 'sourceFile', | |
| filePath: flowPath, | |
| }; | |
| } | |
| } | |
| } | |
| // Custom resolver to handle Node.js modules and dynamic flow imports | |
| if (moduleName.startsWith('@selfxyz/mobile-sdk-alpha/')) { | |
| const subPath = moduleName.replace('@selfxyz/mobile-sdk-alpha/', ''); | |
| // Check if it's a flow import (onboarding/* or disclosing/*) | |
| if ( | |
| subPath.startsWith('onboarding/') || | |
| subPath.startsWith('disclosing/') | |
| ) { | |
| try { | |
| const flowPath = path.resolve( | |
| sdkAlphaPath, | |
| 'dist/esm/flows', | |
| `${subPath}.js`, | |
| ); | |
| const flowIndexPath = path.resolve( | |
| sdkAlphaPath, | |
| 'dist/esm/flows', | |
| subPath, | |
| 'index.js', | |
| ); | |
| if (fs.existsSync(flowPath)) { | |
| return { type: 'sourceFile', filePath: flowPath }; | |
| } | |
| if (fs.existsSync(flowIndexPath)) { | |
| return { type: 'sourceFile', filePath: flowIndexPath }; | |
| } | |
| } catch (e) { | |
| console.warn(`Flow resolve error for ${moduleName}:`, e?.message ?? e); | |
| } | |
| // Delegate to default Metro resolver on miss | |
| return context.resolveRequest(context, moduleName, platform); | |
| } | |
| } |
🤖 Prompt for AI Agents
In app/metro.config.cjs around lines 121 to 144, the custom resolver for
'@selfxyz/mobile-sdk-alpha/*' can throw on fs operations and doesn't handle
directory index files or properly delegate when a flow isn't found; wrap the
existence check and path resolution in a try/catch to avoid breaking the
resolver, attempt an additional fallback path that resolves to an index.js
inside the flow directory if the direct <subPath>.js is missing, and if neither
file exists return null (or call the default resolver) so resolution is
delegated rather than causing a hard failure.
| console.error('Error loading selected document:', error); | ||
| init(selfClient, 'dsc'); | ||
| } |
There was a problem hiding this comment.
Redact error logging to avoid potential PII exposure.
The error may contain document details (PII). Log a generic message only.
Apply this diff:
- } catch (error) {
- console.error('Error loading selected document:', error);
- init(selfClient, 'dsc');
+ } catch (_error) {
+ console.error('Error loading selected document: [redacted]');
+ init(selfClient, 'dsc');
}As per coding guidelines.
📝 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.
| console.error('Error loading selected document:', error); | |
| init(selfClient, 'dsc'); | |
| } | |
| } catch (_error) { | |
| console.error('Error loading selected document: [redacted]'); | |
| init(selfClient, 'dsc'); | |
| } |
🤖 Prompt for AI Agents
In packages/mobile-sdk-alpha/src/flows/onboarding/confirm-identification.ts
around lines 45 to 47, the console.error call logs the caught error which may
contain PII from document details; change it to log only a generic message
(e.g., "Error loading selected document") without including the error object or
document data, or remove the stack/details entirely, then proceed to call
init(selfClient, 'dsc') as before.
| // Dynamically find all flow files | ||
| function findFlowFiles(dir: string, basePath = ''): Record<string, string> { | ||
| const entries: Record<string, string> = {}; | ||
|
|
||
| if (!fs.existsSync(dir)) return entries; | ||
|
|
||
| const items = fs.readdirSync(dir, { withFileTypes: true }); | ||
|
|
||
| for (const item of items) { | ||
| const itemPath = path.join(dir, item.name); | ||
| const relativePath = basePath ? path.join(basePath, item.name) : item.name; | ||
|
|
||
| if (item.isDirectory()) { | ||
| Object.assign(entries, findFlowFiles(itemPath, relativePath)); | ||
| } else if (item.isFile() && item.name.endsWith('.ts')) { | ||
| const key = path.join('flows', relativePath).replace(/\.ts$/, ''); | ||
| entries[key] = path.join('src', 'flows', relativePath); | ||
| } | ||
| } | ||
|
|
||
| return entries; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Make flow discovery OS-agnostic, support .tsx and index files, and exclude tests.
Current logic risks:
- path.join in entry keys creates OS-specific separators.
- index.ts ends up as flows/.../index (Metro expects flows/...).
- .tsx flow files aren’t bundled; tests/.d.ts may slip in.
Refactor finder accordingly.
Apply this diff:
-// Dynamically find all flow files
-function findFlowFiles(dir: string, basePath = ''): Record<string, string> {
- const entries: Record<string, string> = {};
-
- if (!fs.existsSync(dir)) return entries;
-
- const items = fs.readdirSync(dir, { withFileTypes: true });
-
- for (const item of items) {
- const itemPath = path.join(dir, item.name);
- const relativePath = basePath ? path.join(basePath, item.name) : item.name;
-
- if (item.isDirectory()) {
- Object.assign(entries, findFlowFiles(itemPath, relativePath));
- } else if (item.isFile() && item.name.endsWith('.ts')) {
- const key = path.join('flows', relativePath).replace(/\.ts$/, '');
- entries[key] = path.join('src', 'flows', relativePath);
- }
- }
-
- return entries;
-}
+// Dynamically find all flow files
+function findFlowFiles(dir: string, basePath = ''): Record<string, string> {
+ const entries: Record<string, string> = {};
+ if (!fs.existsSync(dir)) return entries;
+
+ const items = fs.readdirSync(dir, { withFileTypes: true });
+
+ for (const item of items) {
+ const itemPath = path.join(dir, item.name);
+ const nextBasePosix = basePath
+ ? path.posix.join(basePath, item.name)
+ : item.name;
+
+ if (item.isDirectory()) {
+ Object.assign(entries, findFlowFiles(itemPath, nextBasePosix));
+ continue;
+ }
+
+ if (!item.isFile()) continue;
+ // include .ts/.tsx, exclude .d.ts and tests
+ if (!/\.(ts|tsx)$/.test(item.name)) continue;
+ if (/\.d\.ts$/.test(item.name)) continue;
+ if (/\.(test|spec)\.(ts|tsx)$/.test(item.name)) continue;
+
+ const relPosix = nextBasePosix;
+ const withoutExt = relPosix.replace(/\.(ts|tsx)$/, '');
+ const keyPath = withoutExt.endsWith('/index')
+ ? withoutExt.slice(0, -('/index'.length))
+ : withoutExt;
+ const key = path.posix.join('flows', keyPath);
+
+ entries[key] = path.join('src', 'flows', relPosix);
+ }
+
+ return entries;
+}This ensures stable subpath outputs, covers component files, and avoids bundling tests. As per coding guidelines.
📝 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.
| // Dynamically find all flow files | |
| function findFlowFiles(dir: string, basePath = ''): Record<string, string> { | |
| const entries: Record<string, string> = {}; | |
| if (!fs.existsSync(dir)) return entries; | |
| const items = fs.readdirSync(dir, { withFileTypes: true }); | |
| for (const item of items) { | |
| const itemPath = path.join(dir, item.name); | |
| const relativePath = basePath ? path.join(basePath, item.name) : item.name; | |
| if (item.isDirectory()) { | |
| Object.assign(entries, findFlowFiles(itemPath, relativePath)); | |
| } else if (item.isFile() && item.name.endsWith('.ts')) { | |
| const key = path.join('flows', relativePath).replace(/\.ts$/, ''); | |
| entries[key] = path.join('src', 'flows', relativePath); | |
| } | |
| } | |
| return entries; | |
| } | |
| // Dynamically find all flow files | |
| function findFlowFiles(dir: string, basePath = ''): Record<string, string> { | |
| const entries: Record<string, string> = {}; | |
| if (!fs.existsSync(dir)) return entries; | |
| const items = fs.readdirSync(dir, { withFileTypes: true }); | |
| for (const item of items) { | |
| const itemPath = path.join(dir, item.name); | |
| const nextBasePosix = basePath | |
| ? path.posix.join(basePath, item.name) | |
| : item.name; | |
| if (item.isDirectory()) { | |
| Object.assign(entries, findFlowFiles(itemPath, nextBasePosix)); | |
| continue; | |
| } | |
| if (!item.isFile()) continue; | |
| // include .ts/.tsx, exclude .d.ts and tests | |
| if (!/\.(ts|tsx)$/.test(item.name)) continue; | |
| if (/\.d\.ts$/.test(item.name)) continue; | |
| if (/\.(test|spec)\.(ts|tsx)$/.test(item.name)) continue; | |
| const relPosix = nextBasePosix; | |
| const withoutExt = relPosix.replace(/\.(ts|tsx)$/, ''); | |
| const keyPath = withoutExt.endsWith('/index') | |
| ? withoutExt.slice(0, -('/index'.length)) | |
| : withoutExt; | |
| const key = path.posix.join('flows', keyPath); | |
| entries[key] = path.join('src', 'flows', relPosix); | |
| } | |
| return entries; | |
| } |
🤖 Prompt for AI Agents
In packages/mobile-sdk-alpha/tsup.config.ts around lines 12 to 33, the flow
discovery currently uses OS-dependent path.join for entry keys, ignores .tsx,
includes index files as .../index (Metro expects parent path), and can include
tests or .d.ts; update the finder to: recurse as before but skip any file or
directory whose path includes /test/ or matching /\.test\.|\.spec\.|\.d\.ts$/;
treat files ending with .ts or .tsx only; for entry keys build a stable
POSIX-style key using path.posix.join('flows', basePath, nameWithoutExt) and if
the file is an index.(ts|tsx) use the POSIX key for the parent directory (i.e.
path.posix.join('flows', basePath) instead of .../index); keep the entry value
as the filesystem path to the source (you can use path.join for that), ensuring
keys use forward slashes so output is OS-agnostic and tests/type-declarations
are excluded.
There was a problem hiding this comment.
this is for converting ts/x files into cjs and mjs files and d,ts files so i dont see why we need to worry about ignoring. d.ts files here
There was a problem hiding this comment.
@aaronmgdr, you're right—since this is discovering source files in src/flows/ to feed tsup as entry points, there shouldn't be any .d.ts files sitting in your source directory. That check was overly defensive.
The more important improvements in my suggestion are:
- OS-agnostic keys: Using
path.posix.joinfor entry keys ensures consistent forward-slash paths regardless of platform (Windows vs Unix) .tsxsupport: Current regex only matches.ts; if you add React components in flows, they won't be bundledindexfile handling: Anindex.tscurrently becomesflows/onboarding/indexbut Metro/package.json exports likely expectflows/onboarding- Test exclusion: Prevents
*.test.tsor*.spec.tsfiles from becoming entry points
If you're confident your flows will only be .ts files (no components), no index patterns, and no test files in that directory, the current implementation works. Otherwise, those four points are worth addressing for robustness. Let me know if you'd like a revised diff focusing on just those concerns!
Setup the export structure that will serve as the basic high level interface for the mobile sdk.
exports structure
@selfxyz/mobile-sdk-alpha/onboarding/[verb]-[noun]@selfxyz/mobile-sdk-alpha/disclosing/[verb]-[noun]Summary by CodeRabbit
New Features
Documentation
Chores
API