Open
Conversation
Contributor
📝 WalkthroughWalkthroughMigrate consumers from Changes
Sequence Diagram(s)sequenceDiagram
participant Proving as ProvingMachine
participant Doc as createDocument (new-common)
participant TEE as circuits.tee
participant WS as WSDbRelayer / Endpoint
Proving->>Doc: createDocument(passportData)
Doc-->>Proving: IDocument (getRegisterCircuitName, getDnsMappingKey, generateCommitment...)
Proving->>TEE: generateTEEInputsRegister(secret, IDDocument, trees, env)
TEE-->>Proving: TEEResult { circuitName, endpointType, endpoint }
Proving->>WS: POST /open (endpoint) with payload (getPayload/encrypt)
WS-->>Proving: ACK / websocket URL
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 20
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/mobile-sdk-alpha/src/documents/utils.ts (1)
180-208:⚠️ Potential issue | 🟠 MajorWrap CSCA restore flow in local error handling.
Line 186–207 can throw (parse, algorithm resolution, storage), and failures currently bubble out of this helper. That can break document restore flows instead of degrading gracefully.
Proposed fix
export async function reStorePassportDataWithRightCSCA(selfClient: SelfClient, passportData: IDDocument, csca: string) { if (passportData.documentCategory === 'aadhaar' || passportData.documentCategory === 'kyc') { return; } const cscaInCurrentPassporData = passportData.passportMetadata?.csca; if (!(csca === cscaInCurrentPassporData)) { - const cscaParsed = parseCertificateSimple(csca); - const dscCertData = bruteForceSignatureAlgorithmDsc(passportData.dsc_parsed!, cscaParsed); + try { + const cscaParsed = parseCertificateSimple(csca); + const dscCertData = bruteForceSignatureAlgorithmDsc(passportData.dsc_parsed!, cscaParsed); - if (passportData.passportMetadata && dscCertData && cscaParsed.publicKeyDetails) { - passportData.passportMetadata.csca = csca; - passportData.passportMetadata.cscaFound = true; - passportData.passportMetadata.cscaHashFunction = dscCertData.hashAlgorithm; - passportData.passportMetadata.cscaSignatureAlgorithm = dscCertData.signatureAlgorithm; - passportData.passportMetadata.cscaSaltLength = dscCertData.saltLength; + if (passportData.passportMetadata && dscCertData && cscaParsed.publicKeyDetails) { + passportData.passportMetadata.csca = csca; + passportData.passportMetadata.cscaFound = true; + passportData.passportMetadata.cscaHashFunction = dscCertData.hashAlgorithm; + passportData.passportMetadata.cscaSignatureAlgorithm = dscCertData.signatureAlgorithm; + passportData.passportMetadata.cscaSaltLength = dscCertData.saltLength; - const cscaCurveOrExponent = - cscaParsed.signatureAlgorithm === 'rsapss' || cscaParsed.signatureAlgorithm === 'rsa' - ? (cscaParsed.publicKeyDetails as PublicKeyDetailsRSA).exponent - : (cscaParsed.publicKeyDetails as PublicKeyDetailsECDSA).curve; + const cscaCurveOrExponent = + cscaParsed.signatureAlgorithm === 'rsapss' || cscaParsed.signatureAlgorithm === 'rsa' + ? (cscaParsed.publicKeyDetails as PublicKeyDetailsRSA).exponent + : (cscaParsed.publicKeyDetails as PublicKeyDetailsECDSA).curve; - passportData.passportMetadata.cscaCurveOrExponent = cscaCurveOrExponent; - passportData.passportMetadata.cscaSignatureAlgorithmBits = parseInt(cscaParsed.publicKeyDetails.bits, 10); + passportData.passportMetadata.cscaCurveOrExponent = cscaCurveOrExponent; + passportData.passportMetadata.cscaSignatureAlgorithmBits = parseInt(cscaParsed.publicKeyDetails.bits, 10); - passportData.csca_parsed = cscaParsed; + passportData.csca_parsed = cscaParsed; - await storePassportData(selfClient, passportData); + await storePassportData(selfClient, passportData); + } + } catch (error) { + console.error('Failed to restore passport data with CSCA', error); } } }As per coding guidelines: "Always use try-catch for async operations with graceful degradation when native modules fail and comprehensive error boundaries."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/mobile-sdk-alpha/src/documents/utils.ts` around lines 180 - 208, Wrap the CSCA restore logic inside reStorePassportDataWithRightCSCA in a local try-catch so parsing/storage failures don't bubble up: protect calls to parseCertificateSimple, bruteForceSignatureAlgorithmDsc and await storePassportData with a try block, and on any catch log the error (including the csca value and document id/type from passportData) and gracefully return without mutating passportData further; ensure you still only update passportData.passportMetadata/csca_parsed when all operations succeed and do not rethrow the exception.packages/mobile-sdk-alpha/src/proving/provingMachine.ts (1)
54-59:⚠️ Potential issue | 🟠 MajorChange
resolveWebSocketUrlparameter type fromPassportDatatoIDDocumentThe function accepts
PassportDatabut both call sites passIDDocumentvia unsafe casts. The function body uses only universal properties (documentCategory) and callscreateDocument(), which is designed to handle anyIDDocument. Changing the parameter toIDDocumentremoves the casts, strengthens type safety, and correctly reflects what the function actually accepts—important in this SDK-critical proving path.Type-safe fix
const resolveWebSocketUrl = ( selfClient: SelfClient, circuitType: 'disclose' | 'register' | 'dsc', - passportData: PassportData, + passportData: IDDocument, circuitName: string, ): string | undefined => { const { documentCategory } = passportData; const circuitsMapping = selfClient.getProtocolState()[documentCategory].circuits_dns_mapping; const doc = createDocument(passportData); const mappingKey = doc.getDnsMappingKey(circuitType); return circuitsMapping?.[mappingKey]?.[circuitName]; };-const wsRpcUrl = resolveWebSocketUrl(selfClient, typedCircuitType, passportData as PassportData, circuitName); +const wsRpcUrl = resolveWebSocketUrl(selfClient, typedCircuitType, passportData, circuitName); -const wsRpcUrl = resolveWebSocketUrl(selfClient, circuitType, passportData as PassportData, circuitName); +const wsRpcUrl = resolveWebSocketUrl(selfClient, circuitType, passportData, circuitName);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/mobile-sdk-alpha/src/proving/provingMachine.ts` around lines 54 - 59, Change the resolveWebSocketUrl function signature to accept IDDocument instead of PassportData, update its parameter type in the declaration of resolveWebSocketUrl and any related type annotations, and remove the unsafe casts at its call sites so callers pass their IDDocument directly; ensure createDocument(...) usage remains valid (it already accepts IDDocument) and add or adjust the import for IDDocument if needed to keep types consistent.packages/mobile-sdk-alpha/src/mock/generator.ts (2)
94-97:⚠️ Potential issue | 🟠 MajorAvoid logging potentially sensitive Aadhaar data.
Aadhaar
qrDataandextractedFieldsmay contain PII (name, DOB, address). Consider removing these logs or redacting sensitive fields before logging.🔒 Proposed fix
if ('qrData' in result) { - console.log('Generated Aadhaar qrData:', result.qrData); - console.log('Generated Aadhaar extractedFields:', result.extractedFields); + console.log('Generated Aadhaar document successfully'); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/mobile-sdk-alpha/src/mock/generator.ts` around lines 94 - 97, Remove or redact the sensitive Aadhaar logging in the mock generator: stop printing result.qrData and result.extractedFields directly (the console.log calls shown) and either remove those logs entirely or replace them with redacted summaries (e.g., lengths or masked values) before logging; update the code paths that check "'qrData' in result" in generator.ts to use the redaction/removal so PII (name, DOB, address) is never emitted to the console.
58-58:⚠️ Potential issue | 🟠 MajorRemove PII from console output.
This log statement outputs
firstNameandlastNamewhich are PII. Per coding guidelines, sensitive data including names must never be logged.🔒 Proposed fix
- console.log('generateMockDocument received names:', { firstName, lastName, isInOfacList }); + console.log('generateMockDocument received names:', { hasFirstName: !!firstName, hasLastName: !!lastName, isInOfacList });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/mobile-sdk-alpha/src/mock/generator.ts` at line 58, The console.log in generateMockDocument leaks PII by printing firstName and lastName; remove or replace that log with a non-PII message (e.g., log only that generateMockDocument was called and any non-sensitive flags like isInOfacList) so that firstName/lastName are never output; update the call site in generator.ts where generateMockDocument logs and ensure any other debug logs in the same module avoid printing PII.
🧹 Nitpick comments (1)
app/src/integrations/nfc/nfcScanner.ts (1)
7-7: Add concrete QA notes for the native-integration migration before merge.This PR touches NFC/native-adjacent surfaces, but the PR description has no real test plan or reproducible QA steps. Please add platform-by-platform verification notes (iOS/Android/Web impact) to reduce release risk.
Based on learnings: Document complex native module changes in PR description.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/integrations/nfc/nfcScanner.ts` at line 7, Add concrete QA/test-plan notes to the PR description covering native-integration migration for NFC surfaces touched by app/src/integrations/nfc/nfcScanner.ts (and any code paths that consume PassportData). Include platform-by-platform steps: iOS (permission prompt, background/foreground scan, app cold/hot restart, NFC session lifecycle), Android (permission and NFC toggle, NFC intent handling with Activity lifecycle, multi-manufacturer checks), Web (confirm no regression if web build uses fallback), and common checks (successful scan of a real passport, malformed/partial data handling, error logs from nfcScanner functions, retry/timeout behavior, and telemetry/console entries). Also state required devices/OS versions, reproducible commands or taps, expected outcomes, and how to capture logs for failures.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/metro.config.cjs`:
- Around line 31-33: The resolver still references the legacy "common" path
causing .js→.ts fallback and crypto polyfill resolution to fail; update every
resolver fallback that builds paths with "common" to use "new-common" instead
(ensure both workspaceRoot and projectRoot variants are updated), specifically
in the Metro config's watchFolders, the .js→.ts fallback mapping and the crypto
polyfill fallback (look for uses in watchFolders, resolver.extraNodeModules and
the resolver fallback/.js->.ts mapping), and ensure path.resolve(workspaceRoot,
'new-common') and path.resolve(projectRoot, 'node_modules') are present so Metro
can resolve the polyfills and type fallbacks.
In `@app/scripts/analyze-tree-shaking.cjs`:
- Around line 308-310: The current import-detection regex (commonImportRegex)
uses .* which fails to match multiline import statements; update the pattern to
allow newlines by replacing the .* with a non-greedy [\s\S]*? so the regex
becomes one that matches "import" followed by any chars/newlines up to "from
'...@selfxyz/new-common...'" (keep the global flag and the existing use of
content.match with matches variable). Locate the declaration of
commonImportRegex and replace its pattern accordingly so multiline imports in
files like provingMachine.ts and cardBackgroundSelector.ts are detected.
In `@app/scripts/test-tree-shaking.cjs`:
- Around line 264-267: The test harness is still pointing at the old "common"
directory; update the staging path to the migrated package directory and ensure
subsequent operations use it. Replace the commonDir assignment (commonDir) to
reference the migrated package path (e.g., join(__dirname, '..', '..',
'packages', 'new-common')) and update any file-copying or build steps that rely
on commonDir (including the execSync/yarn workspace build invocation if it
assumes a different layout) so the test harness stages artifacts from the new
package location.
In `@app/src/navigation/deeplinks.ts`:
- Line 9: Remove the test-only import of IdDocInput and derive the nationality
type directly from the existing countries import: replace usage of
IdDocInput['nationality'] with (typeof countries)[keyof typeof countries] (or
create a local type alias like type Nationality = (typeof countries)[keyof
typeof countries]) and update any references (e.g., in deeplink type
annotations) to use that alias; ensure no other symbols from
'@selfxyz/new-common/src/testing/genMockIdDoc' are used so production code no
longer depends on testing modules.
In `@app/src/stores/userStore.ts`:
- Line 7: Replace the testing-only import and type usage: remove the import of
IdDocInput from '@selfxyz/new-common/src/testing/genMockIdDoc' and instead add
import type { Country3LetterCode } from '@selfxyz/new-common'; then change any
occurrences of IdDocInput['nationality'] (used in userStore.ts) to
Country3LetterCode so production code depends on the public type; ensure no
other references to IdDocInput remain in the file.
In `@new-common/plans/app.md`:
- Around line 119-123: Update the migration checklist to add platform/runtime QA
gates: extend "Step 2: Run type check" to require running "yarn types" for
app/**/*.{ts,tsx} and keep `npx tsc --noEmit` for app/src/**/*.{js,jsx,ts,tsx};
add post-PR manual runtime verification items requiring that the app launches
without crashes and core flows are exercised on iOS, Android, and Web, plus
targeted verification of platform-specific code paths; annotate the checklist
entries so they apply to the patterns `app/src/**/*.{js,jsx,ts,tsx}` and
`app/**/*.{ts,tsx}` and mark them as required before PR completion.
In `@new-common/plans/PLAN.md`:
- Line 873: Update the sentence "**All implementation tasks complete.**" to
clearly separate core implementation from release/migration work: replace it
with something like "Core module implementation tasks are complete." followed by
a second sentence noting remaining work, e.g. "Remaining: Task 16 finalization
(polyfills/crypto.ts) and consumer migrations (app/, sdk/, packages/) — release
and migration are still pending." Ensure you update the text around the existing
mention of "Task 16 finalization (polyfills/crypto.ts)" and "consumer migrations
(app/, sdk/, packages/)" so readers understand implementation is done but
release/migration is not.
- Around line 768-775: The PLAN.md currently claims Task 12 is implemented as
GCP-only attestation (attestation/gcp.ts exposing validatePKIToken and
checkPCR0Mapping, and attestation/index.ts barrel exporting them, source
common/src/utils/attest.ts, and AWS cose.ts dropped), but earlier
architecture/DI and blockchain module descriptions conflict; update those
earlier sections to either reflect the implemented state (GCP-only attestation,
no DI for attestation, removal/archival of cose.ts and changes to blockchain
module layout) or explicitly mark them as historical/deprecated so
imports/contracts match attestation/gcp.ts, validatePKIToken, checkPCR0Mapping,
attestation/index.ts and common/src/utils/attest.ts; ensure wording clearly
instructs teams to follow the current file/module layout and import paths.
In `@new-common/src/blockchain/proving.ts`:
- Around line 90-92: The catch block in proving.ts currently calls
console.warn('Failed to get aadhar registration window:', error) which can leak
sensitive RPC endpoint/credential data; change that to avoid logging the raw
error object—either log a generic message (e.g. "Failed to get aadhar
registration window") or log only a sanitized/non-sensitive string such as
error.message after stripping URLs/credentials, and keep returning the default
120; update the console.warn call (and any similar logging in this file)
accordingly to ensure no raw error objects are emitted.
- Around line 79-93: getAadharRegistrationWindow currently returns a bigint from
identityVerificationHub.AADHAAR_REGISTRATION_WINDOW() but returns a number on
error, causing type mismatches at callsites; change the function to always
return a number by converting the contract result (from
identityVerificationHub.AADHAAR_REGISTRATION_WINDOW()) to a numeric type (e.g.,
via Number(...) or parsing the .toString()) and keep the fallback 120 as a
number; also sanitize the catch log by logging only a string (e.g.,
error?.message or String(error)) instead of the raw error object so external
error objects aren’t printed.
In `@new-common/src/circuits/tee.ts`:
- Around line 122-133: The proof input lacks a trusted timestamp binding,
allowing replay; update the call to generator.generateDiscloseInputs (the inputs
object assigned to inputs) to include a bound time anchor (e.g., add a field
such as trusted_timestamp or time_token with a cryptographically verifiable
value like Date.now()/ISO timestamp or a signed/nonce token from the trusted
time service); if generateDiscloseInputs does not currently accept a timestamp,
add the new parameter to its signature and propagate it into the circom input
structure so the circuit (and any proof generation code) includes and verifies
the timestamp/anchor to ensure freshness and prevent replay.
- Around line 70-78: The function generateTEEInputsRegister currently
force-casts dscTree to string which breaks when callers pass a string[]; update
generateTEEInputsRegister to detect whether dscTree is a string or string[] and
normalize it before calling generator.generateRegisterInputs (e.g., join the
array into the expected path/encoding or serialize it to the format generator
expects), ensuring you do not use a force-cast and that the normalized value is
passed to generator.generateRegisterInputs so array inputs are handled safely.
In `@new-common/src/documents/passport/validate.ts`:
- Around line 39-43: The code is force-unwrapping a nullable circuitName from
doc.isValidRegisterCircuit (and similar calls) which can be null on errors;
replace uses of circuitName! with a safe fallback (e.g., circuitName ??
'unknown_circuit' or another descriptive string) and remove the non-null
assertion, updating the return objects to always provide a string for details;
apply the same fix for the other occurrences that call
doc.isValidRegisterCircuit / passport-circuit checks so
'registration_circuit_not_supported', 'passport_supported' (and similar status
returns) never leak null into details.
- Around line 72-85: The function checkIfPassportDscIsInTree currently uses
non-null assertions on passportData.dsc_parsed and passportData.csca_parsed
which will throw for incomplete payloads; change it to first validate those
fields (e.g., if (!passportData.dsc_parsed || !passportData.csca_parsed) return
false) and only call getLeafDscTree when both are present, keeping the rest of
the logic (hashFunction, LeanIMT.import, tree.indexOf) unchanged so the helper
always returns a boolean instead of rejecting.
- Around line 107-116: The code assumes passportData.passportMetadata and
passportData.eContent exist when computing dg1_packed_hash and
eContent_packed_hash; add a guard before calling packBytesAndPoseidon/hash so
the alternative-CSCA flow doesn't throw outside the per-CSCA try/catch.
Specifically, around the eContent calculation that calls
hash(passportData.passportMetadata!.eContentHashFunction,
Array.from(passportData.eContent), 'bytes'), check that
passportData.passportMetadata and passportData.eContent are present and valid
(and that eContentHashFunction exists) and only then compute
eContent_packed_hash via packBytesAndPoseidon; otherwise set
eContent_packed_hash to a safe sentinel (e.g., undefined/null) or short-circuit
this branch so the per-CSCA try/catch can handle the malformed passport instead
of letting an exception escape. Ensure references to packBytesAndPoseidon,
formatMrz, hash, passportData.passportMetadata, and passportData.eContent are
updated accordingly.
In `@new-common/src/trees/index.ts`:
- Line 47: The change renamed the export to getOfacCountryCode and removed the
original getCountryCode export, breaking callers; restore compatibility by
exporting getCountryCode alongside the alias (i.e., export both getCountryCode
and getOfacCountryCode) so existing imports still resolve, ensuring the symbol
references in the file (getCountryCode / getOfacCountryCode) both point to the
same implementation.
In `@packages/mobile-sdk-alpha/src/documents/utils.ts`:
- Line 187: Add a null/undefined guard before calling
bruteForceSignatureAlgorithmDsc to avoid using the non-null assertion on
passportData.dsc_parsed; specifically check passportData.dsc_parsed exists and
handle the absent case (e.g., return early, throw a descriptive error, or set
dscCertData to null) before invoking
bruteForceSignatureAlgorithmDsc(passportData.dsc_parsed, cscaParsed) so
dscCertData is only produced when dsc_parsed is present.
In `@packages/mobile-sdk-alpha/src/documents/validation.ts`:
- Around line 82-88: The function isDocumentValidForProving currently only
checks expiry when documentData is present and can therefore reject mock
documents; update isDocumentValidForProving to immediately return true for mock
documents by checking the mock flag on the DocumentMetadata (e.g., metadata.mock
=== true or metadata.isMock === true depending on your DocumentMetadata shape)
before any createDocument/isExpired logic so mock-document validity behavior is
preserved; keep the existing try/catch and expiry check
(createDocument(documentData).isExpired()) for non-mock documents.
In `@packages/mobile-sdk-alpha/src/proving/provingMachine.ts`:
- Around line 912-918: The calls that resolve circuit names via
createDocument(passportData) then doc.getDiscloseCircuitName(),
doc.getRegisterCircuitName(), and doc.getDscCircuitName() are unguarded and can
throw (and currently reject silently because initTeeConnection() is not
awaited); wrap the entire circuit resolution logic in a try-catch around
createDocument and the three get*CircuitName() calls (locations referencing
createDocument, getDiscloseCircuitName, getRegisterCircuitName,
getDscCircuitName and the passportData/PassportData cast), log the error via the
existing logger/processLogger, and return early (or transition to an error
state) so the machine does not remain stuck in init_tee_connexion or
ready_to_prove; also validate the runtime document type before calling
getDscCircuitName() to avoid calling DSC-specific APIs on non-passport types.
In `@packages/rn-sdk-test-app/ios/.xcode.env.local`:
- Line 1: The file hardcodes a machine-specific NODE_BINARY path; remove the
absolute path and make it portable by setting NODE_BINARY to a generic
resolution (e.g., rely on the PATH by using "node" or compute the binary at
runtime), or keep .xcode.env.local local-only and add it to .gitignore;
reference the NODE_BINARY variable and ensure consumers rely on the project
.nvmrc (Node 22.x) being installed before running commands.
---
Outside diff comments:
In `@packages/mobile-sdk-alpha/src/documents/utils.ts`:
- Around line 180-208: Wrap the CSCA restore logic inside
reStorePassportDataWithRightCSCA in a local try-catch so parsing/storage
failures don't bubble up: protect calls to parseCertificateSimple,
bruteForceSignatureAlgorithmDsc and await storePassportData with a try block,
and on any catch log the error (including the csca value and document id/type
from passportData) and gracefully return without mutating passportData further;
ensure you still only update passportData.passportMetadata/csca_parsed when all
operations succeed and do not rethrow the exception.
In `@packages/mobile-sdk-alpha/src/mock/generator.ts`:
- Around line 94-97: Remove or redact the sensitive Aadhaar logging in the mock
generator: stop printing result.qrData and result.extractedFields directly (the
console.log calls shown) and either remove those logs entirely or replace them
with redacted summaries (e.g., lengths or masked values) before logging; update
the code paths that check "'qrData' in result" in generator.ts to use the
redaction/removal so PII (name, DOB, address) is never emitted to the console.
- Line 58: The console.log in generateMockDocument leaks PII by printing
firstName and lastName; remove or replace that log with a non-PII message (e.g.,
log only that generateMockDocument was called and any non-sensitive flags like
isInOfacList) so that firstName/lastName are never output; update the call site
in generator.ts where generateMockDocument logs and ensure any other debug logs
in the same module avoid printing PII.
In `@packages/mobile-sdk-alpha/src/proving/provingMachine.ts`:
- Around line 54-59: Change the resolveWebSocketUrl function signature to accept
IDDocument instead of PassportData, update its parameter type in the declaration
of resolveWebSocketUrl and any related type annotations, and remove the unsafe
casts at its call sites so callers pass their IDDocument directly; ensure
createDocument(...) usage remains valid (it already accepts IDDocument) and add
or adjust the import for IDDocument if needed to keep types consistent.
---
Nitpick comments:
In `@app/src/integrations/nfc/nfcScanner.ts`:
- Line 7: Add concrete QA/test-plan notes to the PR description covering
native-integration migration for NFC surfaces touched by
app/src/integrations/nfc/nfcScanner.ts (and any code paths that consume
PassportData). Include platform-by-platform steps: iOS (permission prompt,
background/foreground scan, app cold/hot restart, NFC session lifecycle),
Android (permission and NFC toggle, NFC intent handling with Activity lifecycle,
multi-manufacturer checks), Web (confirm no regression if web build uses
fallback), and common checks (successful scan of a real passport,
malformed/partial data handling, error logs from nfcScanner functions,
retry/timeout behavior, and telemetry/console entries). Also state required
devices/OS versions, reproducible commands or taps, expected outcomes, and how
to capture logs for failures.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 72fc227c-d3c5-4635-904a-4670f861075a
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (113)
app/metro.config.cjsapp/package.jsonapp/scripts/analyze-tree-shaking.cjsapp/scripts/test-tree-shaking.cjsapp/scripts/tests/treeShaking.test.cjsapp/src/components/Disclosures.tsxapp/src/components/homescreen/IdCard.tsxapp/src/components/homescreen/IdCardRevealed.tsxapp/src/components/homescreen/KycIdCard.tsxapp/src/components/homescreen/cardSecurityBadge.tsapp/src/components/navbar/HomeNavBar.tsxapp/src/hooks/useProofDisclosureStalenessCheck.tsapp/src/hooks/useSelfAppData.tsapp/src/hooks/useSumsubWebSocket.tsapp/src/integrations/nfc/nfcScanner.tsapp/src/integrations/sumsub/sumsubService.tsapp/src/navigation/app.tsxapp/src/navigation/deeplinks.tsapp/src/navigation/types.tsapp/src/polyfills/empty.jsapp/src/providers/passportDataProvider.tsxapp/src/proving/index.tsapp/src/proving/validateDocument.tsapp/src/screens/account/recovery/AccountRecoveryChoiceScreen.tsxapp/src/screens/account/recovery/RecoverWithPhraseScreen.tsxapp/src/screens/app/LoadingScreen.tsxapp/src/screens/dev/CreateMockScreen.tsxapp/src/screens/dev/CreateMockScreenDeepLink.tsxapp/src/screens/documents/management/DocumentDataInfoScreen.tsxapp/src/screens/documents/management/IdDetailsScreen.tsxapp/src/screens/documents/management/ManageDocumentsScreen.tsxapp/src/screens/documents/scanning/DocumentNFCScanScreen.tsxapp/src/screens/home/HomeScreen.tsxapp/src/screens/kyc/KYCVerifiedScreen.tsxapp/src/screens/shared/ComingSoonScreen.tsxapp/src/screens/verification/DocumentSelectorForProvingScreen.tsxapp/src/screens/verification/ProveScreen.tsxapp/src/services/points/utils.tsapp/src/stores/pendingKycStore.tsapp/src/stores/proofHistoryStore.tsapp/src/stores/proofTypes.tsapp/src/stores/userStore.tsapp/src/utils/cardBackgroundSelector.tsapp/src/utils/disclosureUtils.tsapp/src/utils/documentAttributes.tsapp/src/utils/documents.tsapp/tests/src/hooks/useProofDisclosureStalenessCheck.test.tsapp/tests/src/proving/validateDocument.test.tsapp/tests/src/screens/verification/DocumentSelectorForProvingScreen.test.tsxapp/tests/src/screens/verification/ProvingScreenRouter.test.tsxapp/tests/src/utils/cardBackgroundSelector.test.tsapp/tests/src/utils/documents.test.tscircuits/package.jsonnew-common/package.jsonnew-common/plans/PLAN.mdnew-common/plans/app.mdnew-common/src/blockchain/index.tsnew-common/src/blockchain/proving.tsnew-common/src/circuits/circuitName.tsnew-common/src/circuits/index.tsnew-common/src/circuits/tee.tsnew-common/src/circuits/userId.tsnew-common/src/data/index.tsnew-common/src/documents/aadhaar/adapter.tsnew-common/src/documents/contentHash.tsnew-common/src/documents/factory.tsnew-common/src/documents/index.tsnew-common/src/documents/interface.tsnew-common/src/documents/kyc/adapter.tsnew-common/src/documents/passport/adapter.tsnew-common/src/documents/passport/index.tsnew-common/src/documents/passport/validate.tsnew-common/src/foundation/types/document.tsnew-common/src/foundation/types/index.tsnew-common/src/index.tsnew-common/src/trees/index.tsnew-common/tsup.config.tspackages/mobile-sdk-alpha/package.jsonpackages/mobile-sdk-alpha/src/adapters/browser/documents.tspackages/mobile-sdk-alpha/src/adapters/react-native/documents.tspackages/mobile-sdk-alpha/src/components/flag/RoundFlag.tsxpackages/mobile-sdk-alpha/src/components/screens/NFCScannerScreen.tsxpackages/mobile-sdk-alpha/src/documents/useCountries.tsxpackages/mobile-sdk-alpha/src/documents/utils.tspackages/mobile-sdk-alpha/src/documents/validation.tspackages/mobile-sdk-alpha/src/flows/onboarding/confirm-identification.tsxpackages/mobile-sdk-alpha/src/flows/onboarding/country-picker-screen.tsxpackages/mobile-sdk-alpha/src/flows/onboarding/document-nfc-screen.tsxpackages/mobile-sdk-alpha/src/flows/onboarding/import-aadhaar.tspackages/mobile-sdk-alpha/src/mock/generator.tspackages/mobile-sdk-alpha/src/proving/provingMachine.tspackages/mobile-sdk-alpha/src/stores/protocolStore.tspackages/mobile-sdk-alpha/src/stores/selfAppStore.tsxpackages/mobile-sdk-alpha/src/types/events.tspackages/mobile-sdk-alpha/src/types/public.tspackages/mobile-sdk-alpha/src/types/ui.tspackages/mobile-sdk-alpha/src/validation/document.tspackages/mobile-sdk-alpha/tests/documents/utils.test.tspackages/mobile-sdk-alpha/tests/documents/validation.test.tspackages/mobile-sdk-alpha/tests/mock/generator.test.tspackages/mobile-sdk-alpha/tests/proving/internal/payloadGenerator.test.tspackages/mobile-sdk-alpha/tests/proving/internal/statusListener.test.tspackages/mobile-sdk-alpha/tests/proving/internal/websocketHandlers.test.tspackages/mobile-sdk-alpha/tests/proving/internal/websocketUrlResolver.test.tspackages/mobile-sdk-alpha/tests/proving/provingMachine.disclose.stateless.test.tspackages/mobile-sdk-alpha/tests/proving/provingMachine.documentProcessor.test.tspackages/mobile-sdk-alpha/tests/proving/provingMachine.generatePayload.test.tspackages/mobile-sdk-alpha/tests/proving/provingMachine.integration.test.tspackages/mobile-sdk-alpha/tests/proving/provingMachine.test.tspackages/mobile-sdk-alpha/tests/validation/document.test.tspackages/mobile-sdk-alpha/tsconfig.jsonpackages/mobile-sdk-alpha/vitest.config.tspackages/rn-sdk-test-app/ios/.xcode.env.local
💤 Files with no reviewable changes (1)
- new-common/src/circuits/circuitName.ts
Comment on lines
+308
to
310
| // Check for @selfxyz/new-common imports | ||
| const commonImportRegex = /import.*from\s+['"]@selfxyz\/new-common[^'"]*['"]/g; | ||
| const matches = content.match(commonImportRegex) || []; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and read the relevant lines
if [ -f "app/scripts/analyze-tree-shaking.cjs" ]; then
echo "=== File exists. Checking lines 308-310: ==="
sed -n '305,315p' "app/scripts/analyze-tree-shaking.cjs"
else
echo "File not found. Searching for analyze-tree-shaking files:"
find . -name "analyze-tree-shaking*" -type f 2>/dev/null
fiRepository: selfxyz/self
Length of output: 445
🏁 Script executed:
# Run the verification test from the review comment
python - <<'PY'
import re
sample = """import {
countryCodes,
getCountryISO2,
} from '@selfxyz/new-common';
import { isMRZDocument } from '@selfxyz/new-common';"""
old = re.compile(r"import.*from\s+['\"]@selfxyz\/new-common[^'\"]*['\"]", re.MULTILINE)
new = re.compile(r"import[\s\S]*?from\s+['\"]@selfxyz\/new-common(?:\/[^'\"]*)?['\"]", re.MULTILINE)
old_matches = old.findall(sample)
new_matches = new.findall(sample)
print("Sample text:")
print(sample)
print("\n--- Pattern Comparison ---")
print(f"Old pattern matches: {len(old_matches)}")
for i, m in enumerate(old_matches):
print(f" {i+1}: {repr(m)}")
print(f"\nNew pattern matches: {len(new_matches)}")
for i, m in enumerate(new_matches):
print(f" {i+1}: {repr(m)}")
print("\n--- Analysis ---")
print(f"Old pattern misses multiline: {len(old_matches) < 2}")
PYRepository: selfxyz/self
Length of output: 522
🏁 Script executed:
# Search for actual multiline imports of `@selfxyz/new-common` in the codebase
echo "=== Searching for multiline `@selfxyz/new-common` imports ==="
rg -U "import\s*\{[^}]*\n[^}]*\}\s*from\s*['\"]@selfxyz/new-common" --max-count 10 app/src packages/mobile-sdk-alpha/src 2>/dev/null || echo "No multiline imports found"Repository: selfxyz/self
Length of output: 4599
🏁 Script executed:
# Also check for any `@selfxyz/new-common` imports to understand usage
echo "=== All `@selfxyz/new-common` imports (first 20) ==="
rg "@selfxyz/new-common" --type ts --type tsx --type js -n app/src packages/mobile-sdk-alpha/src 2>/dev/null | head -20Repository: selfxyz/self
Length of output: 106
Multiline import detection bug undercounts @selfxyz/new-common usage, affecting analysis accuracy.
The regex at line 309 doesn't match newlines within import statements. Real multiline imports exist throughout the codebase (e.g., packages/mobile-sdk-alpha/src/proving/provingMachine.ts, app/src/utils/cardBackgroundSelector.ts), and the current pattern misses them entirely. This produces inaccurate tree-shaking recommendations.
Apply the suggested fix using [\s\S]*? instead of .* to capture both single-line and multiline import blocks:
Regex fix
- const commonImportRegex = /import.*from\s+['"]@selfxyz\/new-common[^'"]*['"]/g;
+ const commonImportRegex =
+ /import[\s\S]*?from\s+['"]@selfxyz\/new-common(?:\/[^'"]*)?['"]/g;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Check for @selfxyz/new-common imports | |
| const commonImportRegex = /import.*from\s+['"]@selfxyz\/new-common[^'"]*['"]/g; | |
| const matches = content.match(commonImportRegex) || []; | |
| // Check for `@selfxyz/new-common` imports | |
| const commonImportRegex = | |
| /import[\s\S]*?from\s+['"]@selfxyz\/new-common(?:\/[^'"]*)?['"]/g; | |
| const matches = content.match(commonImportRegex) || []; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/scripts/analyze-tree-shaking.cjs` around lines 308 - 310, The current
import-detection regex (commonImportRegex) uses .* which fails to match
multiline import statements; update the pattern to allow newlines by replacing
the .* with a non-greedy [\s\S]*? so the regex becomes one that matches "import"
followed by any chars/newlines up to "from '...@selfxyz/new-common...'" (keep
the global flag and the existing use of content.match with matches variable).
Locate the declaration of commonImportRegex and replace its pattern accordingly
so multiline imports in files like provingMachine.ts and
cardBackgroundSelector.ts are detected.
Comment on lines
+264
to
+267
| // Ensure @selfxyz/new-common is built | ||
| console.log('\n🔨 Building @selfxyz/new-common...'); | ||
| const commonDir = join(__dirname, '..', '..', 'common'); | ||
| execSync('yarn workspace @selfxyz/common build', { | ||
| execSync('yarn workspace @selfxyz/new-common build', { |
Contributor
There was a problem hiding this comment.
Use the migrated package directory for staging artifacts.
Line 266 still points to common, so this test harness can copy the wrong package (or fail if common/ is gone), invalidating tree-shaking results for @selfxyz/new-common.
Suggested fix
- const commonDir = join(__dirname, '..', '..', 'common');
+ const commonDir = join(__dirname, '..', '..', 'new-common');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Ensure @selfxyz/new-common is built | |
| console.log('\n🔨 Building @selfxyz/new-common...'); | |
| const commonDir = join(__dirname, '..', '..', 'common'); | |
| execSync('yarn workspace @selfxyz/common build', { | |
| execSync('yarn workspace @selfxyz/new-common build', { | |
| // Ensure `@selfxyz/new-common` is built | |
| console.log('\n🔨 Building `@selfxyz/new-common`...'); | |
| const commonDir = join(__dirname, '..', '..', 'new-common'); | |
| execSync('yarn workspace `@selfxyz/new-common` build', { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/scripts/test-tree-shaking.cjs` around lines 264 - 267, The test harness
is still pointing at the old "common" directory; update the staging path to the
migrated package directory and ensure subsequent operations use it. Replace the
commonDir assignment (commonDir) to reference the migrated package path (e.g.,
join(__dirname, '..', '..', 'packages', 'new-common')) and update any
file-copying or build steps that rely on commonDir (including the execSync/yarn
workspace build invocation if it assumes a different layout) so the test harness
stages artifacts from the new package location.
| import { countries } from '@selfxyz/common/constants/countries'; | ||
| import type { IdDocInput } from '@selfxyz/common/utils'; | ||
| import { countries } from '@selfxyz/new-common'; | ||
| import type { IdDocInput } from '@selfxyz/new-common/src/testing/genMockIdDoc'; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find `@selfxyz/new-common` package.json
PKG_JSON=$(fd '^package\.json$' | while read -r f; do
if rg -q '"name"\s*:\s*"@selfxyz/new-common"' "$f" 2>/dev/null || grep -q '"@selfxyz/new-common"' "$f" 2>/dev/null; then
dirname "$f"
break
fi
done)
if [ -z "${PKG_JSON:-}" ]; then
echo "Searching in monorepo structure..."
PKG_DIR=$(find . -type f -name "package.json" -exec grep -l '"name".*@selfxyz/new-common' {} \; | head -1 | xargs dirname)
if [ -z "$PKG_DIR" ]; then
echo "ERROR: Could not locate `@selfxyz/new-common` package"
exit 1
fi
PKG_JSON="$PKG_DIR/package.json"
fi
echo "=== Package Location ==="
echo "$PKG_JSON"
echo -e "\n=== Checking exports field ==="
if command -v jq &>/dev/null; then
jq '.exports // "NO EXPORTS FIELD"' "$PKG_JSON"
else
python3 -c "import json; data=json.load(open('$PKG_JSON')); print(json.dumps(data.get('exports', 'NO EXPORTS FIELD'), indent=2))"
fi
echo -e "\n=== Checking for genMockIdDoc files ==="
PKG_ROOT=$(dirname "$PKG_JSON")
find "$PKG_ROOT" -name "*genMockIdDoc*" -type f 2>/dev/null | head -20
echo -e "\n=== Examining deeplinks.ts ==="
cat -n app/src/navigation/deeplinks.ts | head -30Repository: selfxyz/self
Length of output: 116
🏁 Script executed:
#!/bin/bash
# Check if deeplinks.ts actually uses IdDocInput and in what context
echo "=== IdDocInput usage in deeplinks.ts ==="
rg -n "IdDocInput" app/src/navigation/deeplinks.ts -B 2 -A 2
echo -e "\n=== File classification ==="
if [[ "app/src/navigation/deeplinks.ts" == *".test."* ]] || [[ "app/src/navigation/deeplinks.ts" == *"__tests__"* ]]; then
echo "This is a TEST file"
else
echo "This is a RUNTIME/SOURCE file"
fi
echo -e "\n=== Type definitions check ==="
echo "Checking if type is exported from public API..."
rg -l "export.*IdDocInput" packages/new-common/src/ 2>/dev/null | head -5Repository: selfxyz/self
Length of output: 716
🏁 Script executed:
#!/bin/bash
# More targeted search for new-common package
echo "=== Locating packages/new-common ==="
find . -type d -name "new-common" 2>/dev/null
echo -e "\n=== Package.json in packages ==="
if [ -d "packages/new-common" ]; then
echo "Found packages/new-common"
cat packages/new-common/package.json | head -50
elif [ -d "packages" ]; then
ls -la packages/ | grep -i common || echo "No common package found"
fi
echo -e "\n=== genMockIdDoc file content ==="
find . -name "genMockIdDoc*" -type f 2>/dev/null | head -1 | xargs head -30Repository: selfxyz/self
Length of output: 1536
🏁 Script executed:
#!/bin/bash
# Get full context of the deeplinks.ts file to understand the usage
echo "=== Full deeplinks.ts file ==="
wc -l app/src/navigation/deeplinks.ts
echo -e "\n=== Lines 1-50 ==="
head -50 app/src/navigation/deeplinks.ts
echo -e "\n=== Lines 170-195 (around line 182 usage) ==="
sed -n '170,195p' app/src/navigation/deeplinks.tsRepository: selfxyz/self
Length of output: 2943
🏁 Script executed:
#!/bin/bash
# Check new-common package.json for exports field
echo "=== new-common/package.json (first 80 lines) ==="
head -80 ./new-common/package.json
echo -e "\n=== Check for exports field ==="
grep -A 30 '"exports"' ./new-common/package.json || echo "NO EXPORTS FIELD FOUND"
echo -e "\n=== Full genMockIdDoc.js/ts interface ==="
find ./new-common -name "genMockIdDoc*" -type f | head -1 | xargs grep -A 15 "export interface IdDocInput"Repository: selfxyz/self
Length of output: 3486
🏁 Script executed:
#!/bin/bash
# Check if nationality type is available elsewhere (from countries or public exports)
echo "=== Looking for nationality type definition ==="
rg -n "nationality" ./new-common/src/ --type-list | head -20
echo -e "\n=== Countries export ==="
rg -B 5 -A 10 "export.*countries" ./new-common/src/constants/ 2>/dev/null | head -40
echo -e "\n=== Check what IdDocInput['nationality'] actually is ==="
rg -A 3 "nationality" ./new-common/src/testing/genMockIdDoc.* 2>/dev/null | head -20Repository: selfxyz/self
Length of output: 847
🏁 Script executed:
#!/bin/bash
# Verify that ./src/testing/* is indeed matched by the ./src/* export pattern
echo "=== Checking export wildcard coverage ==="
python3 << 'PYTHON'
import fnmatch
exports = [
".",
"./src/*",
"./src/data/*.json"
]
test_paths = [
"./src/testing/genMockIdDoc.ts",
"./src/testing/genMockIdDoc.d.ts",
"./dist/esm/src/testing/genMockIdDoc.d.ts"
]
for test_path in test_paths:
for export_pattern in exports:
if export_pattern == ".":
continue
# Check if test_path matches the export pattern (accounting for dist compilation)
if fnmatch.fnmatch(test_path, export_pattern):
print(f"✓ {test_path} matches export pattern '{export_pattern}'")
break
else:
print(f"✗ {test_path} does NOT match any export pattern")
PYTHON
echo -e "\n=== Verify countries type equivalence ==="
# The actual type of nationality in IdDocInput
echo "IdDocInput['nationality'] type = (typeof countries)[keyof typeof countries]"
echo "This can be locally derived as:"
echo "type SupportedNationality = (typeof countries)[keyof typeof countries];"
echo -e "\n=== Check if IdDocInput is used elsewhere in app ==="
rg "IdDocInput" app/ --type ts --type tsx | grep -v deeplinks.tsRepository: selfxyz/self
Length of output: 628
🏁 Script executed:
#!/bin/bash
# Final check: context around the import usage - is this in a test or runtime path?
echo "=== Verifying runtime context ==="
echo "File: app/src/navigation/deeplinks.ts"
echo "- Located in app/src/ (runtime code, not __tests__ or .test.ts)"
echo "- Handles OAuth deeplinks with real navigation logic"
echo -e "\n=== Confirm IdDocInput is only used for type narrowing ==="
sed -n '180,190p' app/src/navigation/deeplinks.ts
echo -e "\n=== What is IdDocInput['nationality']? ==="
echo "From genMockIdDoc.ts:"
echo " nationality?: (typeof countries)[keyof typeof countries];"
echo -e "\n=== Can the type be inlined? ==="
echo "Yes. Instead of importing IdDocInput just for the nationality type,"
echo "use: (typeof countries)[keyof typeof countries]"
echo "This is a direct, locally-understandable type with no package dependency."Repository: selfxyz/self
Length of output: 1088
Avoid importing from testing modules in production app code.
Line 9 imports IdDocInput only to extract its nationality type. Since nationality is simply (typeof countries)[keyof typeof countries] and countries is already imported on line 8, derive the type directly to keep production code independent from mock/testing internals.
Suggested refactor
import { countries } from '@selfxyz/new-common';
-import type { IdDocInput } from '@selfxyz/new-common/src/testing/genMockIdDoc';
import type { SelfClient } from '@selfxyz/mobile-sdk-alpha'; const isValidCountryCode = (
nationalityCode: string | undefined,
- ): nationalityCode is IdDocInput['nationality'] => {
+ ): nationalityCode is (typeof countries)[keyof typeof countries] => {
if (!nationalityCode) return false;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/navigation/deeplinks.ts` at line 9, Remove the test-only import of
IdDocInput and derive the nationality type directly from the existing countries
import: replace usage of IdDocInput['nationality'] with (typeof countries)[keyof
typeof countries] (or create a local type alias like type Nationality = (typeof
countries)[keyof typeof countries]) and update any references (e.g., in deeplink
type annotations) to use that alias; ensure no other symbols from
'@selfxyz/new-common/src/testing/genMockIdDoc' are used so production code no
longer depends on testing modules.
| import { create } from 'zustand'; | ||
|
|
||
| import type { IdDocInput } from '@selfxyz/common/utils'; | ||
| import type { IdDocInput } from '@selfxyz/new-common/src/testing/genMockIdDoc'; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Direct app imports from testing internals ==="
rg -n "@selfxyz/new-common/src/testing/" app/src || echo "(no matches)"
echo
echo "=== All IdDocInput imports in app/src ==="
rg -n "IdDocInput" app/src --type ts --type tsx || echo "(no matches)"
echo
echo "=== IdDocInput declarations and exports in repo ==="
rg -n "(export (type )?IdDocInput|interface IdDocInput)" --type ts --type tsx || echo "(no matches)"
echo
echo "=== Check for IdDocInput in public exports or public entry points ==="
rg -n "IdDocInput" packages/ --type ts --type tsx | head -30 || echo "(no matches)"
echo
echo "=== Package.json exports in new-common ==="
if [ -f "packages/new-common/package.json" ]; then
jq -r '.exports // .main // empty' packages/new-common/package.json
fi
echo
echo "=== Check genMockIdDoc file location and exports ==="
fd "genMockIdDoc" --type ts --type tsxRepository: selfxyz/self
Length of output: 1183
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check if CreateMockScreenDeepLink.tsx is in dev screens ==="
head -20 "app/src/screens/dev/CreateMockScreenDeepLink.tsx"
echo
echo "=== Check deeplinks.ts context and usage ==="
head -30 "app/src/navigation/deeplinks.ts"
echo
echo "=== Check userStore.ts usage of IdDocInput ==="
cat -n "app/src/stores/userStore.ts" | head -40
echo
echo "=== Check new-common package structure and index exports ==="
if [ -f "packages/new-common/src/index.ts" ]; then
echo "--- new-common/src/index.ts ---"
cat "packages/new-common/src/index.ts"
fi
echo
echo "=== Check genMockIdDoc file directly ==="
find . -name "genMockIdDoc*" -type f 2>/dev/null | head -10Repository: selfxyz/self
Length of output: 4360
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check deeplinks.ts for IS_DEV_MODE conditional usage ==="
grep -n "IS_DEV_MODE\|IdDocInput" "app/src/navigation/deeplinks.ts" | head -20
echo
echo "=== Check where IdDocInput is used in deeplinks.ts ==="
sed -n '1,100p' "app/src/navigation/deeplinks.ts" | cat -n | tail -50
echo
echo "=== Check what IdDocInput actually is in genMockIdDoc.ts ==="
cat "new-common/src/testing/genMockIdDoc.ts" | head -80
echo
echo "=== Check if there's a public IdDocInput type elsewhere ==="
fd "package.json" packages/ | xargs grep -l "new-common" | head -3
echo
echo "=== Check new-common exports in main index or typing ==="
ls -la packages/new-common/src/ | head -20
</parameter>Repository: selfxyz/self
Length of output: 5397
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check if new-common source exists in different location ==="
find packages -name "genMockIdDoc.ts" -o -name "countries.ts" | head -10
echo
echo "=== Check userStore actual usage of deepLinkNationality ==="
grep -n "deepLinkNationality" "app/src/stores/userStore.ts"
echo
echo "=== Check for public country type exports ==="
rg "export.*countries|export.*Country" packages/new-common/ --type ts 2>/dev/null | head -20
echo
echo "=== Check what 'countries' type actually is ==="
rg -A5 "^export const countries" packages/new-common/src/data/countries.ts 2>/dev/null | head -20
echo
echo "=== Final verification: is idDetailsDocumentId part of production flow? ==="
grep -n "idDetailsDocumentId" "app/src/stores/userStore.ts" | head -10
</parameter>Repository: selfxyz/self
Length of output: 410
🏁 Script executed:
#!/bin/bash
set -euco pipefail
echo "=== Check actual new-common directory structure ==="
ls -la packages/new-common/
echo
echo "=== Look for where new-common source actually is ==="
find . -path "*/node_modules" -prune -o -name "countries.ts" -type f -print
echo
echo "=== Check package.json for new-common to understand exports ==="
cat packages/new-common/package.json 2>/dev/null | jq '.' | head -50
echo
echo "=== Verify: is IdDocInput used outside of IS_DEV_MODE in userStore ==="
grep -B5 -A10 "deepLinkNationality" app/src/stores/userStore.ts
echo
echo "=== Check if deepLinkNationality is ever used in production code path ==="
rg "deepLinkNationality" app/src --type ts --type tsx -B2 -A2 | head -50Repository: selfxyz/self
Length of output: 2478
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Extract the actual nationality type from countries ==="
head -40 "new-common/src/data/countries.ts" | grep -A30 "export"
echo
echo "=== Check all production usage of deepLinkNationality outside of userStore ==="
rg "deepLinkNationality|IdDocInput" app/src --type ts -l
echo
echo "=== Check CreateMockScreenDeepLink context - is it dev-only? ==="
grep -n "export\|function\|const.*=.*screen\|Register\|navigation" "app/src/screens/dev/CreateMockScreenDeepLink.tsx" | head -10
echo
echo "=== Verify: what exactly gets passed as nationality type to deepLinkNationality ==="
rg -B5 -A5 "setDeepLinkUserDetails.*nationality" app/src --type tsRepository: selfxyz/self
Length of output: 1730
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check what's exported from new-common public entry ==="
ls -la new-common/src/*.ts 2>/dev/null | head -10
echo
echo "=== Check if Country3LetterCode or countries are exported from index ==="
cat new-common/src/index.ts 2>/dev/null || echo "(no index.ts found)"
echo
echo "=== Verify Country3LetterCode is exported from countries.ts ==="
grep "export type Country3LetterCode\|export.*countries" new-common/src/data/countries.ts | head -5
echo
echo "=== Check if mobile-sdk-alpha re-exports new-common types ==="
rg "export.*from.*new-common|Country3LetterCode" packages/mobile-sdk-alpha/ --type ts 2>/dev/null | head -10
echo
echo "=== Summary: trace the deepLinkNationality flow in deeplinks.ts ==="
sed -n '160,190p' app/src/navigation/deeplinks.ts | cat -nRepository: selfxyz/self
Length of output: 860
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check data/index.ts to verify countries export path ==="
cat new-common/src/data/index.ts
echo
echo "=== Verify Country3LetterCode is in public export chain ==="
rg "Country3LetterCode" new-common/src/data/index.ts
echo
echo "=== Check all usage of deepLinkNationality to confirm it's dev-only ==="
rg "deepLinkNationality" app/src -B3 -A1 --type ts
echo
echo "=== Final check: is userStore ever used outside of dev/deeplink context ==="
rg "useUserStore\|from.*userStore" app/src --type ts | grep -v "deepLink\|referrer" | head -20Repository: selfxyz/self
Length of output: 4492
Replace testing module import with public Country3LetterCode type
The IdDocInput import from @selfxyz/new-common/src/testing/genMockIdDoc creates a production-code dependency on testing internals. Use the publicly exported Country3LetterCode type instead, which is the exact same type semantically:
import type { Country3LetterCode } from '@selfxyz/new-common';Then replace IdDocInput['nationality'] with Country3LetterCode on lines 12 and 24 of app/src/stores/userStore.ts.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/stores/userStore.ts` at line 7, Replace the testing-only import and
type usage: remove the import of IdDocInput from
'@selfxyz/new-common/src/testing/genMockIdDoc' and instead add import type {
Country3LetterCode } from '@selfxyz/new-common'; then change any occurrences of
IdDocInput['nationality'] (used in userStore.ts) to Country3LetterCode so
production code depends on the public type; ensure no other references to
IdDocInput remain in the file.
| buildKycSMT, | ||
| getCountryLeaf, | ||
| getCountryCode, | ||
| getCountryCode as getOfacCountryCode, |
Contributor
There was a problem hiding this comment.
Keep getCountryCode exported to avoid a breaking API change.
Line 47 introduces getOfacCountryCode via aliasing, but the original getCountryCode symbol is no longer exported. Existing consumers importing getCountryCode will fail at compile time.
Suggested compatibility-safe export
export {
buildSMT,
buildPassportSMT,
buildIdCardSMT,
buildAadhaarSMT,
buildKycSMT,
getCountryLeaf,
+ getCountryCode,
getCountryCode as getOfacCountryCode,
} from './ofac.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.
Suggested change
| getCountryCode as getOfacCountryCode, | |
| export { | |
| buildSMT, | |
| buildPassportSMT, | |
| buildIdCardSMT, | |
| buildAadhaarSMT, | |
| buildKycSMT, | |
| getCountryLeaf, | |
| getCountryCode, | |
| getCountryCode as getOfacCountryCode, | |
| } from './ofac.js'; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@new-common/src/trees/index.ts` at line 47, The change renamed the export to
getOfacCountryCode and removed the original getCountryCode export, breaking
callers; restore compatibility by exporting getCountryCode alongside the alias
(i.e., export both getCountryCode and getOfacCountryCode) so existing imports
still resolve, ensuring the symbol references in the file (getCountryCode /
getOfacCountryCode) both point to the same implementation.
| if (!(csca === cscaInCurrentPassporData)) { | ||
| const cscaParsed = parseCertificateSimple(csca); | ||
| const dscCertData = brutforceSignatureAlgorithmDsc(passportData.dsc_parsed!, cscaParsed); | ||
| const dscCertData = bruteForceSignatureAlgorithmDsc(passportData.dsc_parsed!, cscaParsed); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== 1) Find IDDocument type definition and dsc_parsed field ==="
rg -n --type=ts -B2 -A8 'interface.*IDDocument|type.*IDDocument' packages/mobile-sdk-alpha/
echo
echo "=== 2) Find where dsc_parsed is used with non-null assertions ==="
rg -n --type=ts 'dsc_parsed!' packages/mobile-sdk-alpha/
echo
echo "=== 3) Look at the specific file and function around line 187 ==="
head -n 210 packages/mobile-sdk-alpha/src/documents/utils.ts | tail -n 35
echo
echo "=== 4) Search for reStorePassportDataWithRightCSCA function definition ==="
rg -n --type=ts -B2 -A15 'reStorePassportDataWithRightCSCA' packages/mobile-sdk-alpha/Repository: selfxyz/self
Length of output: 24703
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== 1) Search for dsc_parsed field initialization/population ==="
rg -n --type=ts 'dsc_parsed\s*=' packages/mobile-sdk-alpha/ | head -20
echo
echo "=== 2) Look for where dsc_parsed might be checked for null/undefined ==="
rg -n --type=ts 'dsc_parsed\s*\?|if.*dsc_parsed' packages/mobile-sdk-alpha/ | head -20
echo
echo "=== 3) Check bruteForceSignatureAlgorithmDsc function signature ==="
rg -n --type=ts -B2 -A10 'function bruteForceSignatureAlgorithmDsc|const bruteForceSignatureAlgorithmDsc' packages/mobile-sdk-alpha/
echo
echo "=== 4) Search for PassportData type to understand dsc_parsed field ==="
rg -n --type=ts 'type PassportData|interface PassportData' packages/mobile-sdk-alpha/
echo
echo "=== 5) Check if dsc_parsed is conditionally accessed elsewhere ==="
rg -n --type=ts 'dsc_parsed\?' packages/mobile-sdk-alpha/ | head -15Repository: selfxyz/self
Length of output: 120
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== 1) Find buildPassportFixture and passport test data ==="
rg -n --type=ts -A20 'buildPassportFixture|createPassportData' packages/mobile-sdk-alpha/tests/ | head -50
echo
echo "=== 2) Search where passportData might be loaded/created ==="
rg -n --type=ts 'dsc_parsed' packages/mobile-sdk-alpha/src/ -B5 -A2 | head -60
echo
echo "=== 3) Check `@selfxyz/new-common` import to understand types ==="
grep -r "from '@selfxyz/new-common'" packages/mobile-sdk-alpha/src/documents/utils.ts -A 20 | head -30
echo
echo "=== 4) Look at function calls to reStorePassportDataWithRightCSCA for context ==="
rg -n --type=ts 'reStorePassportDataWithRightCSCA' packages/mobile-sdk-alpha/src/ -B3 -A3Repository: selfxyz/self
Length of output: 14381
Add null check for dsc_parsed before DSC algorithm resolution.
Line 187 uses a non-null assertion on passportData.dsc_parsed without a guard. Since dsc_parsed is optional and the codebase already validates its presence in other contexts (e.g., line 1136 in provingMachine.ts), this function should protect itself:
Proposed fix
if (!(csca === cscaInCurrentPassporData)) {
+ if (!passportData.dsc_parsed) {
+ console.warn('Skipping CSCA restore: missing dsc_parsed');
+ return;
+ }
const cscaParsed = parseCertificateSimple(csca);
- const dscCertData = bruteForceSignatureAlgorithmDsc(passportData.dsc_parsed!, cscaParsed);
+ const dscCertData = bruteForceSignatureAlgorithmDsc(passportData.dsc_parsed, cscaParsed);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/mobile-sdk-alpha/src/documents/utils.ts` at line 187, Add a
null/undefined guard before calling bruteForceSignatureAlgorithmDsc to avoid
using the non-null assertion on passportData.dsc_parsed; specifically check
passportData.dsc_parsed exists and handle the absent case (e.g., return early,
throw a descriptive error, or set dscCertData to null) before invoking
bruteForceSignatureAlgorithmDsc(passportData.dsc_parsed, cscaParsed) so
dscCertData is only produced when dsc_parsed is present.
Comment on lines
82
to
88
| export function isDocumentValidForProving(metadata: DocumentMetadata, documentData?: IDDocument): boolean { | ||
| // Check if expired | ||
| if (documentData) { | ||
| try { | ||
| const attributes = getDocumentAttributes(documentData); | ||
| if (attributes.expiryDateSlice && checkDocumentExpiration(attributes.expiryDateSlice)) { | ||
| return false; | ||
| } | ||
| return !createDocument(documentData).isExpired(); | ||
| } catch { | ||
| // If we can't check expiry, assume valid | ||
| } |
Contributor
There was a problem hiding this comment.
isDocumentValidForProving no longer preserves mock-document validity behavior.
The function contract says mock documents are always valid, but current logic only checks isExpired() when documentData is present. That can reject mock docs and break staging/mock proving flows.
Suggested fix
export function isDocumentValidForProving(metadata: DocumentMetadata, documentData?: IDDocument): boolean {
+ if ((metadata as { mock?: boolean }).mock || (documentData as { mock?: boolean } | undefined)?.mock) {
+ return true;
+ }
if (documentData) {
try {
return !createDocument(documentData).isExpired();
} catch {
// If we can't check expiry, assume valid
}
}
return true;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/mobile-sdk-alpha/src/documents/validation.ts` around lines 82 - 88,
The function isDocumentValidForProving currently only checks expiry when
documentData is present and can therefore reject mock documents; update
isDocumentValidForProving to immediately return true for mock documents by
checking the mock flag on the DocumentMetadata (e.g., metadata.mock === true or
metadata.isMock === true depending on your DocumentMetadata shape) before any
createDocument/isExpired logic so mock-document validity behavior is preserved;
keep the existing try/catch and expiry check
(createDocument(documentData).isExpired()) for non-mock documents.
Comment on lines
+912
to
+918
| const doc = createDocument(passportData); | ||
| const circuitName = | ||
| typedCircuitType === 'disclose' | ||
| ? passportData.documentCategory === 'aadhaar' | ||
| ? 'disclose_aadhaar' | ||
| : passportData.documentCategory === 'kyc' | ||
| ? 'disclose_kyc' | ||
| : 'disclose' | ||
| : getCircuitNameFromPassportData(passportData, typedCircuitType as 'register' | 'dsc'); | ||
| ? doc.getDiscloseCircuitName() | ||
| : typedCircuitType === 'register' | ||
| ? doc.getRegisterCircuitName() | ||
| : doc.getDscCircuitName(); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check file size and structure first
echo "=== File info ==="
wc -l packages/mobile-sdk-alpha/src/proving/provingMachine.ts
# Look at the critical lines mentioned
echo -e "\n=== Lines 912-918 (circuit resolution in reconnect) ==="
sed -n '910,920p' packages/mobile-sdk-alpha/src/proving/provingMachine.ts
echo -e "\n=== Lines 1351-1357 (circuit resolution in init) ==="
sed -n '1349,1365p' packages/mobile-sdk-alpha/src/proving/provingMachine.ts
echo -e "\n=== Lines 420-422 (fire-and-forget call) ==="
sed -n '418,425p' packages/mobile-sdk-alpha/src/proving/provingMachine.ts
echo -e "\n=== Check resolveWebSocketUrl signature ==="
rg -n "resolveWebSocketUrl" packages/mobile-sdk-alpha/src/proving/provingMachine.ts -A 3 -B 1Repository: selfxyz/self
Length of output: 2475
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find document helper implementations
echo "=== Document helper API locations ==="
find . -type f -name "*.ts" -o -name "*.tsx" | xargs rg -l "createDocument|getDiscloseCircuitName|getRegisterCircuitName|getDscCircuitName" | head -20
echo -e "\n=== Check createDocument implementation ==="
rg -n "export.*createDocument|function createDocument" -A 5 packages/ | head -40
echo -e "\n=== Check for throw statements in document helpers ==="
rg -n "getDiscloseCircuitName|getRegisterCircuitName|getDscCircuitName" packages/ -A 2 | grep -E "throw|error" | head -20Repository: selfxyz/self
Length of output: 3351
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check the state machine transitions and error handling
echo "=== State machine CONNECT_ERROR handler ==="
rg -n "CONNECT_ERROR|init_tee_connexion|ready_to_prove" packages/mobile-sdk-alpha/src/proving/provingMachine.ts -B 2 -A 2
echo -e "\n=== Check actor.send patterns ==="
rg -n "actor\\.send\\|actor?.send" packages/mobile-sdk-alpha/src/proving/provingMachine.ts | head -10
echo -e "\n=== Type of passportData parameter ==="
rg -n "passportData.*:.*IDDocument|passportData.*:.*PassportData" packages/mobile-sdk-alpha/src/proving/provingMachine.ts | head -10Repository: selfxyz/self
Length of output: 5585
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check if there are any try-catch blocks around circuit resolution
echo "=== Existing try-catch in connection paths ==="
sed -n '900,1000p' packages/mobile-sdk-alpha/src/proving/provingMachine.ts | grep -n "try\|catch" || echo "No try-catch found in 900-1000"
echo -e "\n=== Existing try-catch in init path ==="
sed -n '1340,1370p' packages/mobile-sdk-alpha/src/proving/provingMachine.ts | grep -n "try\|catch" || echo "No try-catch found in 1340-1370"
# Check initTeeConnection function signature and its awaiting
echo -e "\n=== initTeeConnection function definition ==="
rg -n "async.*initTeeConnection|function.*initTeeConnection" packages/mobile-sdk-alpha/src/proving/provingMachine.ts -A 5Repository: selfxyz/self
Length of output: 268
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Get createDocument implementation from new-common
echo "=== createDocument in new-common ==="
rg -n "export.*function createDocument|export const createDocument" new-common/src/ -A 8
# Check for error handling in document helper methods
echo -e "\n=== getDiscloseCircuitName implementation ==="
rg -n "getDiscloseCircuitName" new-common/src/ -A 3
echo -e "\n=== getRegisterCircuitName implementation ==="
rg -n "getRegisterCircuitName" new-common/src/ -A 3
echo -e "\n=== getDscCircuitName implementation ==="
rg -n "getDscCircuitName" new-common/src/ -A 3Repository: selfxyz/self
Length of output: 6142
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check what createDocument returns and what interface it has
echo "=== Document interface ==="
rg -n "interface.*Document|type.*Document" new-common/src/documents/ -B 1 -A 5 | head -80
# Look at the actual initTeeConnection function to see what happens with circuit resolution
echo -e "\n=== initTeeConnection full implementation ==="
rg -n "initTeeConnection.*:" packages/mobile-sdk-alpha/src/proving/provingMachine.ts -A 100 | head -120Repository: selfxyz/self
Length of output: 10293
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check if createDocument can throw
echo "=== Checking createDocument implementation ==="
rg -n "createDocument" new-common/src/documents/index.ts -A 15
# Check IDDocument type vs PassportData type
echo -e "\n=== Type definitions ==="
rg -n "type IDDocument|type PassportData|interface IDDocument|interface PassportData" packages/mobile-sdk-alpha/src/ new-common/src/ | head -20
# Look at how passportData is passed to initTeeConnection
echo -e "\n=== passportData parameter type in store ==="
sed -n '200,250p' packages/mobile-sdk-alpha/src/proving/provingMachine.tsRepository: selfxyz/self
Length of output: 3130
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check if there's error handling in document helper calls elsewhere in codebase
echo "=== How createDocument is used with error handling ==="
rg -n "createDocument" packages/mobile-sdk-alpha/ -B 2 -A 5 | grep -E "try|catch|createDocument" | head -20
# Look at the actual return type and potential errors
echo -e "\n=== Document adapter implementation ==="
rg -n "getDiscloseCircuitName|getRegisterCircuitName|getDscCircuitName" new-common/src/documents/ -B 2 -A 8 | head -60Repository: selfxyz/self
Length of output: 7146
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify full initTeeConnection implementation to confirm no try-catch
echo "=== Full initTeeConnection implementation ==="
sed -n '1338,1440p' packages/mobile-sdk-alpha/src/proving/provingMachine.ts
# Check _reconnectTeeWebSocket as well
echo -e "\n=== Full _reconnectTeeWebSocket implementation ==="
sed -n '902,950p' packages/mobile-sdk-alpha/src/proving/provingMachine.ts
# Verify the throw conditions in detail
echo -e "\n=== PassportDocument getRegisterCircuitName full code ==="
sed -n '127,145p' new-common/src/documents/passport/adapter.ts
echo -e "\n=== PassportDocument getDscCircuitName full code ==="
sed -n '160,178p' new-common/src/documents/passport/adapter.tsRepository: selfxyz/self
Length of output: 8572
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check if line 420 initTeeConnection call has await
echo "=== Lines 418-428 to confirm fire-and-forget ==="
sed -n '418,428p' packages/mobile-sdk-alpha/src/proving/provingMachine.ts
# Double check if there's any error boundary around the subscription
echo -e "\n=== Context around line 420-422 subscription ==="
sed -n '400,435p' packages/mobile-sdk-alpha/src/proving/provingMachine.ts
# Check what happens if promise rejects in Zustand subscription
echo -e "\n=== Store initialization and subscription pattern ==="
sed -n '350,365p' packages/mobile-sdk-alpha/src/proving/provingMachine.tsRepository: selfxyz/self
Length of output: 2079
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify the type casting at lines 920 and 1359
echo "=== Type casting at line 920 context ==="
sed -n '915,925p' packages/mobile-sdk-alpha/src/proving/provingMachine.ts
echo -e "\n=== Type casting at line 1359 context ==="
sed -n '1354,1364p' packages/mobile-sdk-alpha/src/proving/provingMachine.ts
# Check resolveWebSocketUrl parameter expectations
echo -e "\n=== resolveWebSocketUrl full signature ==="
sed -n '54,80p' packages/mobile-sdk-alpha/src/proving/provingMachine.tsRepository: selfxyz/self
Length of output: 2048
Wrap unguarded circuit resolution to prevent silent promise rejection
doc.getDiscloseCircuitName(), doc.getRegisterCircuitName(), and doc.getDscCircuitName() throw when passport metadata is missing, CSCA validation fails, or signature algorithms are unsupported. Calls at lines 1351–1357 and 912–918 have no error handling, and because initTeeConnection() is invoked without await at line 420, exceptions silently reject the promise and leave the machine stuck in init_tee_connexion or ready_to_prove.
Additionally, passportData is typed as IDDocument (union of Passport/Aadhaar/KYC) but cast to PassportData at lines 920 and 1359, enabling calls to getDscCircuitName() on unsupported document types that explicitly throw.
Wrap circuit resolution in try-catch, log errors, and return early:
Hardening approach
+ const resolveCircuitNameForType = (
+ passportData: IDDocument,
+ circuitType: 'disclose' | 'register' | 'dsc',
+ ): string | null => {
+ try {
+ const doc = createDocument(passportData);
+ return circuitType === 'disclose'
+ ? doc.getDiscloseCircuitName()
+ : circuitType === 'register'
+ ? doc.getRegisterCircuitName()
+ : doc.getDscCircuitName();
+ } catch (error) {
+ return null;
+ }
+ };
// In initTeeConnection
- const doc = createDocument(passportData);
- const circuitName =
- circuitType === 'disclose'
- ? doc.getDiscloseCircuitName()
- : circuitType === 'register'
- ? doc.getRegisterCircuitName()
- : doc.getDscCircuitName();
+ const circuitName = resolveCircuitNameForType(passportData, circuitType);
if (!circuitName) {
actor?.send({ type: 'CONNECT_ERROR' });
selfClient.logProofEvent('error', 'Circuit resolution failed', baseContext, {
failure: 'PROOF_FAILED_CONNECTION',
duration_ms: Date.now() - startTime,
});
- throw new Error('Could not determine circuit name');
+ return false;
}
// Same pattern for _reconnectTeeWebSocket🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/mobile-sdk-alpha/src/proving/provingMachine.ts` around lines 912 -
918, The calls that resolve circuit names via createDocument(passportData) then
doc.getDiscloseCircuitName(), doc.getRegisterCircuitName(), and
doc.getDscCircuitName() are unguarded and can throw (and currently reject
silently because initTeeConnection() is not awaited); wrap the entire circuit
resolution logic in a try-catch around createDocument and the three
get*CircuitName() calls (locations referencing createDocument,
getDiscloseCircuitName, getRegisterCircuitName, getDscCircuitName and the
passportData/PassportData cast), log the error via the existing
logger/processLogger, and return early (or transition to an error state) so the
machine does not remain stuck in init_tee_connexion or ready_to_prove; also
validate the runtime document type before calling getDscCircuitName() to avoid
calling DSC-specific APIs on non-passport types.
| @@ -0,0 +1 @@ | |||
| export NODE_BINARY=/Users/ayman/.nvm/versions/node/v22.19.0/bin/node | |||
Contributor
There was a problem hiding this comment.
Avoid committing a machine-specific NODE_BINARY path
Line 1 hardcodes /Users/ayman/..., which will fail on other dev machines and CI and leaks local user info. Use a portable resolution (or keep this file local-only and untracked).
Suggested fix
-export NODE_BINARY=/Users/ayman/.nvm/versions/node/v22.19.0/bin/node
+export NODE_BINARY="$(command -v node)"Based on learnings: Ensure Node.js 22.x is installed as specified in .nvmrc before running any commands.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export NODE_BINARY=/Users/ayman/.nvm/versions/node/v22.19.0/bin/node | |
| export NODE_BINARY="$(command -v node)" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/rn-sdk-test-app/ios/.xcode.env.local` at line 1, The file hardcodes
a machine-specific NODE_BINARY path; remove the absolute path and make it
portable by setting NODE_BINARY to a generic resolution (e.g., rely on the PATH
by using "node" or compute the binary at runtime), or keep .xcode.env.local
local-only and add it to .gitignore; reference the NODE_BINARY variable and
ensure consumers rely on the project .nvmrc (Node 22.x) being installed before
running commands.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
packages/mobile-sdk-alpha/src/proving/provingMachine.ts (1)
913-919:⚠️ Potential issue | 🔴 CriticalGuard circuit-name resolution and fail deterministically on document adapter throws
Line 913 and Line 1352 resolve circuit names via
createDocument(...).get*CircuitName()without a local catch. If adapter resolution throws (unsupported/malformed data),initTeeConnection()rejection is unhandled because it is triggered fire-and-forget from Line 422, and the machine can get stuck ininit_tee_connexion/ready_to_provewithout a deterministic transition.Suggested hardening diff
+const resolveCircuitNameSafe = ( + passportData: IDDocument, + circuitType: 'disclose' | 'register' | 'dsc', +): string | null => { + try { + const doc = createDocument(passportData); + return circuitType === 'disclose' + ? doc.getInternalDnsMappingKey() + : circuitType === 'register' + ? doc.getRegisterCircuitName() + : doc.getDscCircuitName(); + } catch { + return null; + } +}; // _reconnectTeeWebSocket -const doc = createDocument(passportData); -const circuitName = - typedCircuitType === 'disclose' - ? doc.getInternalDnsMappingKey() - : typedCircuitType === 'register' - ? doc.getRegisterCircuitName() - : doc.getDscCircuitName(); +const circuitName = resolveCircuitNameSafe(passportData, typedCircuitType); +if (!circuitName) { + selfClient.logProofEvent('error', 'Reconnect failed: circuit resolution error', context); + return false; +} // initTeeConnection -const doc = createDocument(passportData); -const circuitName = - circuitType === 'disclose' - ? doc.getInternalDnsMappingKey() - : circuitType === 'register' - ? doc.getRegisterCircuitName() - : doc.getDscCircuitName(); +const circuitName = resolveCircuitNameSafe(passportData, circuitType); +if (!circuitName) { + actor?.send({ type: 'CONNECT_ERROR' }); + selfClient.logProofEvent('error', 'Circuit name resolution failed', baseContext, { + failure: 'PROOF_FAILED_CONNECTION', + duration_ms: Date.now() - startTime, + }); + return false; +}#!/bin/bash set -euo pipefail FILE="packages/mobile-sdk-alpha/src/proving/provingMachine.ts" echo "=== Fire-and-forget async call site (expect initTeeConnection invoked without await) ===" sed -n '418,426p' "$FILE" echo echo "=== Reconnect circuit resolution block (expect no try/catch around createDocument/get*CircuitName) ===" sed -n '908,924p' "$FILE" echo echo "=== Init circuit resolution block (expect no try/catch around createDocument/get*CircuitName) ===" sed -n '1348,1364p' "$FILE" echo echo "=== Document adapter throw paths in new-common (verify these APIs can throw) ===" rg -n "getRegisterCircuitName|getDscCircuitName|getInternalDnsMappingKey|throw new Error" new-common/src/documents -C2As per coding guidelines: "Always use try-catch for async operations with graceful degradation when native modules fail and comprehensive error boundaries."
Also applies to: 1352-1358
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/mobile-sdk-alpha/src/proving/provingMachine.ts` around lines 913 - 919, Wrap the circuit-name resolution calls (createDocument(passportData).getInternalDnsMappingKey(), getRegisterCircuitName(), getDscCircuitName()) in a try/catch at each call site (both the reconnect and init flows) so adapter throws are caught synchronously; on catch, log the error and return/emit a deterministic failure (e.g., set context.error, send a PROVING.TEE_INIT_FAILED event or reject the initTeeConnection promise) so the state machine leaves init_tee_connexion/ready_to_prove instead of hanging; ensure the fire-and-forget initTeeConnection caller observes the failure by propagating the rejection or sending the failure event.app/metro.config.cjs (1)
31-33:⚠️ Potential issue | 🟠 MajorMigration is still partially pinned to legacy
commonpaths.
watchFoldersnow points tonew-common, but resolver fallbacks still hardcodecommon(Line 258 and Line 458). Ifcommonis removed, Metro can fail.js→.tsfallback andcryptopolyfill resolution.Proposed fix
- if ( - context.originModulePath?.includes('/common/src/') && - moduleName.endsWith('.js') - ) { + if ( + (context.originModulePath?.includes('/new-common/src/') || + context.originModulePath?.includes('/common/src/')) && + moduleName.endsWith('.js') + ) { const tsModuleName = moduleName.replace(/\.js$/, '.ts'); return context.resolveRequest(context, tsModuleName, platform); } @@ - crypto: path.resolve(__dirname, '../common/src/polyfills/crypto.ts'), + crypto: path.resolve(__dirname, '../new-common/src/polyfills/crypto.ts'),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/metro.config.cjs` around lines 31 - 33, The resolver fallbacks still reference the old "common" path while watchFolders uses "new-common", causing missing module fallbacks; update any resolver fallback entries (the resolver.fallbacks / resolver.extraNodeModules or similar entries in app/metro.config.cjs that currently resolve path.resolve(workspaceRoot, 'common')) to point to path.resolve(workspaceRoot, 'new-common') (or to the same variable used in the watchFolders array) so the .js→.ts and crypto polyfill fallbacks use the new-common directory consistently; ensure all occurrences (e.g., the resolver fallback blocks around the symbols resolver.fallbacks and any extraNodeModules mapping) are changed to use 'new-common' instead of 'common'.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/metro.config.cjs`:
- Around line 111-113: The global shim of "module" and "node:module" to
src/polyfills/empty.js breaks runtime createRequire calls in
new-common/src/crypto/eddsa.ts (signEdDSA); remove the global extraNodeModules
entries that map module/node:module to the empty shim and instead add
origin-scoped mappings in the metro resolveRequest handler mirroring the
existing 'url' pattern—detect imports originating from new-common and return
path.resolve(projectRoot, 'src/polyfills/empty.js') for "module" and
"node:module" only for those origins so the shim is not globally applied.
In `@new-common/src/documents/interface.ts`:
- Around line 98-105: The switch in getInternalDnsMappingKey() incorrectly
returns the generic 'disclose' for the 'id_card' case; change the 'id_card'
branch to return the id-card-specific mapping used elsewhere (e.g., DISCLOSE_ID
or the string 'disclose_id' to match the adapter and the id-card disclose
circuit referenced around line 67) so id_card lookups route to the correct DNS
key; update only the 'id_card' case in getInternalDnsMappingKey() to return that
consistent constant/string.
---
Duplicate comments:
In `@app/metro.config.cjs`:
- Around line 31-33: The resolver fallbacks still reference the old "common"
path while watchFolders uses "new-common", causing missing module fallbacks;
update any resolver fallback entries (the resolver.fallbacks /
resolver.extraNodeModules or similar entries in app/metro.config.cjs that
currently resolve path.resolve(workspaceRoot, 'common')) to point to
path.resolve(workspaceRoot, 'new-common') (or to the same variable used in the
watchFolders array) so the .js→.ts and crypto polyfill fallbacks use the
new-common directory consistently; ensure all occurrences (e.g., the resolver
fallback blocks around the symbols resolver.fallbacks and any extraNodeModules
mapping) are changed to use 'new-common' instead of 'common'.
In `@packages/mobile-sdk-alpha/src/proving/provingMachine.ts`:
- Around line 913-919: Wrap the circuit-name resolution calls
(createDocument(passportData).getInternalDnsMappingKey(),
getRegisterCircuitName(), getDscCircuitName()) in a try/catch at each call site
(both the reconnect and init flows) so adapter throws are caught synchronously;
on catch, log the error and return/emit a deterministic failure (e.g., set
context.error, send a PROVING.TEE_INIT_FAILED event or reject the
initTeeConnection promise) so the state machine leaves
init_tee_connexion/ready_to_prove instead of hanging; ensure the fire-and-forget
initTeeConnection caller observes the failure by propagating the rejection or
sending the failure event.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 08b37fce-5bfe-4fc3-822c-1b568c943ffd
📒 Files selected for processing (5)
app/babel.config.cjsapp/metro.config.cjsnew-common/src/certificates/parsing/parseCertificateSimple.tsnew-common/src/documents/interface.tspackages/mobile-sdk-alpha/src/proving/provingMachine.ts
💤 Files with no reviewable changes (1)
- new-common/src/certificates/parsing/parseCertificateSimple.ts
Comment on lines
+111
to
+113
| // Node.js built-ins used by new-common (eddsa.ts) but never called at runtime in RN | ||
| module: path.resolve(projectRoot, 'src/polyfills/empty.js'), | ||
| 'node:module': path.resolve(projectRoot, 'src/polyfills/empty.js'), |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find and read the metro.config.cjs file
echo "=== Reading app/metro.config.cjs ==="
if [ -f "app/metro.config.cjs" ]; then
wc -l app/metro.config.cjs
echo "---"
# Read around the mentioned lines: 111-113, 145-151, 152
sed -n '100,160p' app/metro.config.cjs | cat -n
else
echo "File not found: app/metro.config.cjs"
fiRepository: selfxyz/self
Length of output: 3079
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for actual consumers of Node's module API
echo "=== Searching for module API consumers ==="
rg -n "(from\s+['\"]node:module['\"]|from\s+['\"]module['\"]|require\(['\"]node:module['\"]\)|require\(['\"]module['\"]\))" \
-g "!node_modules" \
-g "!dist" \
-g "!build" \
--max-count=20 || echo "No direct module imports found"
echo ""
echo "=== Checking new-common package ==="
# Look for new-common references and structure
find . -name "new-common" -type d 2>/dev/null | head -5
ls -la packages/*/package.json 2>/dev/null | grep -i new || echo "Searching for new-common package..."
rg "new-common" -g "*.json" --max-count=10 || trueRepository: selfxyz/self
Length of output: 1894
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check if empty.js polyfill file exists and what it exports
echo "=== Checking polyfill file ==="
if [ -f "app/src/polyfills/empty.js" ]; then
cat -n app/src/polyfills/empty.js
else
echo "File not found: app/src/polyfills/empty.js"
fi
echo ""
echo "=== Looking for 'eddsa' references mentioned in comment ==="
rg "eddsa" -g "*.ts" -g "*.tsx" --max-count=10 || echo "No eddsa references found"Repository: selfxyz/self
Length of output: 1478
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Understand the metro resolver pattern and originModulePath
echo "=== Looking at metro resolver context pattern (line 152 url example) ==="
if [ -f "app/metro.config.cjs" ]; then
sed -n '140,160p' app/metro.config.cjs | cat -n
fiRepository: selfxyz/self
Length of output: 1075
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check if eddsa is imported/used in app (not just in new-common)
echo "=== Checking if eddsa is used in app ==="
rg "eddsa|signEdDSA" app/src -g "*.ts" -g "*.tsx" --max-count=10 || echo "No eddsa imports in app/src"
echo ""
echo "=== Checking new-common exports/API ==="
# See what new-common exports
if [ -f "new-common/src/index.ts" ]; then
head -50 new-common/src/index.ts | cat -n
fiRepository: selfxyz/self
Length of output: 603
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check getRequire function to understand the actual module usage
echo "=== Examining eddsa.ts getRequire usage ==="
if [ -f "new-common/src/crypto/eddsa.ts" ]; then
cat -n new-common/src/crypto/eddsa.ts
fiRepository: selfxyz/self
Length of output: 1315
Scope module/node:module shims to new-common imports only.
This shim is currently global (lines 147-152 and lines 46-52), and app/src/polyfills/empty.js exports {}. The new-common/src/crypto/eddsa.ts file calls createRequire('node:module') at runtime within the exported signEdDSA() function. A global empty shim will break this if the function is ever called. Apply the same origin-scoped pattern used for url (lines 53-59).
Proposed fix
Remove lines 147-148 (the global extraNodeModules entries):
- module: path.resolve(projectRoot, 'src/polyfills/empty.js'),
- 'node:module': path.resolve(projectRoot, 'src/polyfills/empty.js'),And update the resolveRequest handler (around line 46):
- if (moduleName === 'module' || moduleName === 'node:module') {
+ if (
+ (moduleName === 'module' || moduleName === 'node:module') &&
+ context.originModulePath?.includes('new-common')
+ ) {
return {
type: 'sourceFile',
filePath: path.resolve(projectRoot, 'src/polyfills/empty.js'),
};
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/metro.config.cjs` around lines 111 - 113, The global shim of "module" and
"node:module" to src/polyfills/empty.js breaks runtime createRequire calls in
new-common/src/crypto/eddsa.ts (signEdDSA); remove the global extraNodeModules
entries that map module/node:module to the empty shim and instead add
origin-scoped mappings in the metro resolveRequest handler mirroring the
existing 'url' pattern—detect imports originating from new-common and return
path.resolve(projectRoot, 'src/polyfills/empty.js') for "module" and
"node:module" only for those origins so the shim is not globally applied.
Comment on lines
+98
to
+105
| getInternalDnsMappingKey(): string { | ||
| const category: DocumentCategory = this.category; | ||
| switch (category) { | ||
| case 'passport': | ||
| return 'disclose'; | ||
| case 'id_card': | ||
| return 'disclose'; | ||
| case 'aadhaar': |
Contributor
There was a problem hiding this comment.
Fix ID-card DNS key mismatch in shared mapping helper.
getInternalDnsMappingKey() returns 'disclose' for 'id_card' on Line 104, but Line 67 uses an id-card-specific disclose circuit and the adapter mapping uses DISCLOSE_ID. This inconsistency can route id-card lookups to the passport disclose key.
Suggested fix
getInternalDnsMappingKey(): string {
const category: DocumentCategory = this.category;
switch (category) {
case 'passport':
return 'disclose';
case 'id_card':
- return 'disclose';
+ return 'disclose_id';
case 'aadhaar':
return 'disclose_aadhaar';
case 'kyc':
return 'disclose_kyc';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| getInternalDnsMappingKey(): string { | |
| const category: DocumentCategory = this.category; | |
| switch (category) { | |
| case 'passport': | |
| return 'disclose'; | |
| case 'id_card': | |
| return 'disclose'; | |
| case 'aadhaar': | |
| getInternalDnsMappingKey(): string { | |
| const category: DocumentCategory = this.category; | |
| switch (category) { | |
| case 'passport': | |
| return 'disclose'; | |
| case 'id_card': | |
| return 'disclose_id'; | |
| case 'aadhaar': | |
| return 'disclose_aadhaar'; | |
| case 'kyc': | |
| return 'disclose_kyc'; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@new-common/src/documents/interface.ts` around lines 98 - 105, The switch in
getInternalDnsMappingKey() incorrectly returns the generic 'disclose' for the
'id_card' case; change the 'id_card' branch to return the id-card-specific
mapping used elsewhere (e.g., DISCLOSE_ID or the string 'disclose_id' to match
the adapter and the id-card disclose circuit referenced around line 67) so
id_card lookups route to the correct DNS key; update only the 'id_card' case in
getInternalDnsMappingKey() to return that consistent constant/string.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
A brief description of the changes, what and how is being changed.
Tested
Explain how the change has been tested (for example by manual testing, unit tests etc) or why it's not necessary (for example version bump).
How to QA
How can the change be tested in a repeatable manner?
Summary by CodeRabbit
New Features
Bug Fixes
Refactor