Conversation
* Disable Verify button for expired documents * coderabbit feedbacks
Update build numbers and deployment timestamps after successful deployment. Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* Add back navigation to QR scanner view * linting * remove custom hook. use default package
* fix sentry replays * clean up comments
* Fix paste button on recovery screen * Improve recovery paste button accessibility * recovery screen bugfix * simplify fix. remove pressable logic
* Hide document-only settings without documents * revert document info screen changes, rely on hiding option from settings view * agent feedback * hide settings options that depend on having a document
* Update document scan prompt * formatting * fix scan instruction location * use helper for title text
WalkthroughThis PR consolidates document attribute extraction into a centralized utility module, implements document expiration validation during verification, migrates custom hooks to library implementations, refactors navigation to use reset-based state management for deeplinks, and introduces conditional routing based on document availability. Version numbers are bumped and configuration/constants are updated. Changes
Sequence Diagram(s)sequenceDiagram
participant ProveScreen
participant useEffect
participant loadSelectedDocument
participant checkDocumentExpiration
participant provingStore
participant HeldPrimaryButtonProveScreen
participant UI
ProveScreen->>useEffect: sessionId changes
useEffect->>loadSelectedDocument: retrieve document
loadSelectedDocument-->>useEffect: document loaded
useEffect->>checkDocumentExpiration: check expiry date
checkDocumentExpiration-->>useEffect: isExpired boolean
alt Document Not Expired
useEffect->>provingStore: init(selfClient, 'disclose')
provingStore-->>useEffect: store initialized
useEffect->>HeldPrimaryButtonProveScreen: pass isDocumentExpired=false
HeldPrimaryButtonProveScreen->>UI: render ready to prove
else Document Expired
useEffect->>HeldPrimaryButtonProveScreen: pass isDocumentExpired=true
HeldPrimaryButtonProveScreen->>UI: render "Document expired"
HeldPrimaryButtonProveScreen->>UI: block ready state transition
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ 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 |
transphorm
temporarily deployed
to
development
GitHub Actions
Inactive
* Improve prove deeplink navigation * fix tests
transphorm
temporarily deployed
to
development
GitHub Actions
Inactive
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/src/screens/account/recovery/RecoverWithPhraseScreen.tsx (1)
62-65: Missing user feedback on validation failure.When mnemonic validation fails, the function silently returns without providing any user feedback. The user will see the loading state briefly disappear with no explanation of why the recovery failed. This creates a confusing experience, especially now that invalid mnemonics can be pasted into the text area.
As per coding guidelines: "Provide user-friendly error messages in UI and error handlers."
Consider adding an error state and displaying a message to the user when validation fails:
+ const [error, setError] = useState<string>(); + const restoreAccount = useCallback(async () => { try { + setError(undefined); setRestoring(true); const slimMnemonic = mnemonic?.trim(); if (!slimMnemonic || !ethers.Mnemonic.isValidMnemonic(slimMnemonic)) { + setError('Invalid recovery phrase. Please check and try again.'); setRestoring(false); return; }Then display the error message in the UI below the text area or as a toast notification.
common/src/utils/aadhaar/utils.ts (1)
400-418: Improve type safety and add documentation.The
timestampparameter is typed asstring?but is coerced to a number without validation. Consider:
- Document the expected timestamp format (milliseconds since epoch)
- Use
numbertype instead ofstring, or add explicit parsing with validation- Document the IST offset behavior
Apply this diff to improve type safety:
-export function returnNewDateString(timestamp?: string): string { - const newDate = timestamp ? new Date(+timestamp) : new Date(); +/** + * Generates a date string in IST timezone formatted as YYYYMMDDHHmmssSSS + * @param timestampMs - Optional timestamp in milliseconds since epoch (UTC) + * @returns Formatted date string in IST timezone + */ +export function returnNewDateString(timestampMs?: number): string { + const newDate = timestampMs ? new Date(timestampMs) : new Date();
♻️ Duplicate comments (1)
packages/mobile-sdk-alpha/package.json (1)
154-154: Verify the @selfxyz/euclid version update.Same verification needed as in app/package.json for version ^0.6.1.
🧹 Nitpick comments (5)
app/src/screens/account/settings/SettingsScreen.tsx (1)
165-178: Log the error for better observability.The catch block swallows the error without logging it. When catalog loading fails, you'll have no diagnostic information about the cause (network failure, permission issue, etc.).
Apply this diff to improve error logging:
- } catch { - console.warn('SettingsScreen: failed to load document catalog'); + } catch (error) { + console.warn('SettingsScreen: failed to load document catalog', error); setHasRealDocument(false); }common/src/utils/aadhaar/utils.ts (1)
401-401: Remove: breaking change claim is incorrect; timestamp format is already milliseconds in actual usage.Line 401's change from
+timestamp * 1000to+timestampis not a breaking change. The actual caller atcommon/src/utils/passports/genMockIdDoc.ts:84already passesnew Date().getTime().toString(), which is in milliseconds—matching the new code's expectation.However, address compliance violations:
- Lines 405-407: Applies IST offset (+5:30) to UTC timestamps. Per compliance guidelines, use UTC timestamps only for verification operations.
- Lines 408-415: Returns custom format (YYYYMMDDHHMMSSMMM) instead of ISO 8601 (YYYY-MM-DD).
packages/mobile-sdk-alpha/src/components/buttons/HeldPrimaryButtonProveScreen.tsx (1)
215-215: Consider disabling the button when document is expired.Currently
isDisabledonly checks!state.matches('ready'), but whenisDocumentExpiredis true, the button shows "Document expired" yet may still appear interactive. Consider updating the disabled logic:- const isDisabled = !state.matches('ready'); + const isDisabled = !state.matches('ready') || isDocumentExpired;This provides better UX by making the button visually non-interactive when the document is expired.
app/src/screens/documents/aadhaar/AadhaarUploadErrorScreen.tsx (1)
11-11: Consider alternative user assistance path.The "Need Help?" button has been removed, leaving users with only a "Try Again" option when encountering Aadhaar upload errors. While the TODO comment suggests the help functionality was incomplete, removing user assistance without an alternative path may impact user experience, especially for error recovery scenarios.
If help functionality is deferred, consider:
- Adding a link or guidance in the error description text
- Implementing a lightweight in-app help modal
- Providing documentation links for common error scenarios
Also applies to: 98-107
app/src/utils/documentAttributes.ts (1)
13-17: Remove misplaced JSDoc comment.The JSDoc comment describes "Gets the scan prompt for a document type" but precedes the
DocumentAttributesinterface definition. This comment belongs to thegetDocumentScanPromptfunction at line 221 or should be removed here.Apply this diff to remove the misplaced comment:
-/** - * Gets the scan prompt for a document type. - * @param documentType - Document type code ('p' = Passport, 'i' = ID card, 'a' = Aadhaar) - * @returns Scan prompt text - */ export interface DocumentAttributes {
| mobileReplayIntegration({ | ||
| maskAllText: true, | ||
| maskAllImages: false, | ||
| maskAllVectors: false, | ||
| }), |
There was a problem hiding this comment.
Consider masking images and vectors for PII protection.
While maskAllText: true protects text-based PII, maskAllImages: false and maskAllVectors: false could expose sensitive information in this identity verification app. Document photos, passport images, and ID scans may be captured in session replays.
Consider updating to:
maskAllText: true,
maskAllImages: true,
maskAllVectors: true,As per coding guidelines: "NEVER log sensitive data including PII (names, DOB, passport numbers, addresses), credentials, tokens, API keys, private keys, or session identifiers."
🤖 Prompt for AI Agents
In app/src/config/sentry.ts around lines 168 to 172, the mobileReplayIntegration
currently masks text but leaves images and vectors unmasked; update the
integration options to set maskAllImages: true and maskAllVectors: true so that
images and vector data (e.g., document photos, passport scans) are redacted
during session replay, and run a quick local build/format to ensure no lint
errors after the change.
| // bugfix: perform a simple clipboard check; ethers.Mnemonic.isValidMnemonic doesn't work | ||
| if (clipboard) { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, find the file
find . -name "RecoverWithPhraseScreen.tsx" -type f 2>/dev/null | head -5Repository: selfxyz/self
Length of output: 118
🏁 Script executed:
#!/bin/bash
# Find and examine the RecoverWithPhraseScreen.tsx file
if [ -f "app/src/screens/account/recovery/RecoverWithPhraseScreen.tsx" ]; then
echo "=== File found, examining content ==="
cat -n "app/src/screens/account/recovery/RecoverWithPhraseScreen.tsx" | sed -n '40,80p'
echo ""
echo "=== Around line 62 ==="
cat -n "app/src/screens/account/recovery/RecoverWithPhraseScreen.tsx" | sed -n '55,70p'
echo ""
echo "=== Around line 190-200 (button state) ==="
cat -n "app/src/screens/account/recovery/RecoverWithPhraseScreen.tsx" | sed -n '190,210p'
fiRepository: selfxyz/self
Length of output: 3099
🏁 Script executed:
#!/bin/bash
# Search for all isValidMnemonic usages in the file
rg -n "isValidMnemonic" app/src/screens/account/recovery/RecoverWithPhraseScreen.tsx -A 2 -B 2Repository: selfxyz/self
Length of output: 530
🏁 Script executed:
#!/bin/bash
# Get file size and total lines
wc -l app/src/screens/account/recovery/RecoverWithPhraseScreen.tsxRepository: selfxyz/self
Length of output: 120
Misleading comment and inconsistent validation logic in recovery flow.
The comment at line 51 states that ethers.Mnemonic.isValidMnemonic "doesn't work," yet this same function is still used for validation at line 62 in restoreAccount. The actual issue is that validation was removed from the onPaste handler (only checks if clipboard is non-empty), but validation still occurs when the user presses Continue. However, when validation fails at lines 62-65, there's no user feedback—the function silently returns.
This creates poor UX in a security-sensitive recovery flow: users can paste an invalid mnemonic and only discover the error after pressing Continue, with no indication of what went wrong.
Clarify the comment to reflect the intentional UX change (e.g., "Allow pasting any text; validation occurs on Continue button press") and add user-facing error feedback when validation fails.
🤖 Prompt for AI Agents
In app/src/screens/account/recovery/RecoverWithPhraseScreen.tsx around lines
51-52 and 62-65, update the misleading comment and add user-facing feedback for
failed mnemonic validation: change the comment at line 51 to something like
"Allow pasting any text; validation is performed on Continue" so it reflects
intent, keep the onPaste handler accepting any non-empty clipboard text, and in
restoreAccount replace the silent return on invalid mnemonic with explicit user
feedback (e.g., set an error state/validation message or show an alert/toast)
that explains the mnemonic is invalid so the user knows why recovery failed.
There was a problem hiding this comment.
it doesn't work as a gate. we should allow users to paste whatever they want. i tried pasting a real recovery phrase and it wouldn't allow me to do so
There was a problem hiding this comment.
@transphorm, thank you for the clarification! That makes sense—if isValidMnemonic was producing false negatives and blocking valid recovery phrases, removing it from the paste gate is the right fix. Allowing users to paste any content and validating only when they press Continue is a good approach.
One suggestion: when validation does fail at line 62 (in restoreAccount), the function silently returns without feedback. Consider adding a user-facing error message (e.g., a toast or alert stating "Invalid recovery phrase") so users understand why the recovery didn't proceed. This would improve the UX in the security-sensitive recovery flow.
✏️ Learnings added
Learnt from: transphorm
Repo: selfxyz/self PR: 1516
File: app/src/screens/account/recovery/RecoverWithPhraseScreen.tsx:51-52
Timestamp: 2025-12-17T15:27:34.682Z
Learning: In app/src/screens/account/recovery/RecoverWithPhraseScreen.tsx, ethers.Mnemonic.isValidMnemonic should not be used as a gate for pasting recovery phrases because it produces false negatives and rejects valid mnemonics. Allow users to paste any clipboard content; validation should only occur when the user attempts to restore the account.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
3 participants
🚀 Weekly Release to Staging
Release Date: December 16, 2025
Release Branch:
release/staging-2025-12-16This automated PR promotes a snapshot of
devtostagingfor testing.What's Included
All commits merged to
devup to the branch creation time.Note: This PR uses a dedicated release branch, so new commits to
devwill NOT automatically appear here.Review Checklist
Next Steps
After merging, the staging environment will be updated. A production release PR will be created on Sunday.
This PR was automatically created by the Release Calendar workflow on December 16, 2025
Summary by CodeRabbit
Release Notes
New Features
Improvements
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.