feat: unlock signing request approvals#1074
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughAdds a reusable ChangesApproval Flow Integration
Sequence DiagramsequenceDiagram
participant Component
participant useRequestSignApproval
participant ApprovalMutation
participant UnlockMutation
participant UnlockDialog
participant SignService
Component->>useRequestSignApproval: approve(signAction)
useRequestSignApproval->>ApprovalMutation: mutate (check if unlock needed)
ApprovalMutation->>useRequestSignApproval: need_unlock or immediate_success
alt Unlock Required
useRequestSignApproval->>UnlockDialog: open dialog
UnlockDialog->>useRequestSignApproval: submitUnlock(credential)
useRequestSignApproval->>UnlockMutation: mutate with credential
UnlockMutation->>useRequestSignApproval: unlocked
end
useRequestSignApproval->>SignService: runApproval(signAction)
SignService->>Component: resolve or error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
BundleMonFiles updated (4)
Unchanged files (71)
Total files change +12.49KB +1.38% Groups updated (2)
Final result: ✅ View report in BundleMon website ➡️ |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
samui-wallet-api | 69e4304 | Commit Preview URL Branch Preview URL |
May 12 2026, 01:41 PM |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/feature-request/src/ui/request-ui-unlock-dialog.tsx (1)
54-63: 💤 Low valueOptional cleanup: unreachable 'unsecured' case.
The
'unsecured'case ingetCredentialLabelis unreachable because the approval hook (lines 42-47 inuse-request-sign-approval.tsx) handles unsecured wallets by auto-unlocking with an empty credential and never opening the dialog. This is defensive code that doesn't harm functionality but could be removed for clarity.♻️ Optional simplification
function getCredentialLabel(mode: RequestUnlockMode): string { switch (mode) { case 'password': return 'Password' case 'pin': return 'PIN' - case 'unsecured': - return 'Wallet' } }Alternatively, keep it as defensive programming if there's any possibility of future logic changes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/feature-request/src/ui/request-ui-unlock-dialog.tsx` around lines 54 - 63, The switch in getCredentialLabel contains an unreachable 'unsecured' branch; remove that case from the function so only the actual RequestUnlockMode values handled ('password' and 'pin') are returned (update getCredentialLabel to have just the two cases and a default/fallback if desired), referencing the getCredentialLabel function and the RequestUnlockMode type to locate the code; if you prefer defensive coding instead, add an explicit default that throws or logs instead of keeping the unreachable 'unsecured' branch.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/feature-request/src/ui/request-ui-unlock-dialog.tsx`:
- Around line 54-63: The switch in getCredentialLabel contains an unreachable
'unsecured' branch; remove that case from the function so only the actual
RequestUnlockMode values handled ('password' and 'pin') are returned (update
getCredentialLabel to have just the two cases and a default/fallback if
desired), referencing the getCredentialLabel function and the RequestUnlockMode
type to locate the code; if you prefer defensive coding instead, add an explicit
default that throws or logs instead of keeping the unreachable 'unsecured'
branch.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b5f4e97a-e6e0-4f12-ae33-e5f66262df94
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
packages/feature-request/package.jsonpackages/feature-request/src/data-access/use-request-sign-approval.tsxpackages/feature-request/src/ui/request-ui-sign-and-send-transaction.tsxpackages/feature-request/src/ui/request-ui-sign-in.tsxpackages/feature-request/src/ui/request-ui-sign-message.tsxpackages/feature-request/src/ui/request-ui-sign-transaction.tsxpackages/feature-request/src/ui/request-ui-unlock-dialog.tsx
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
samui-wallet-web | 69e4304 | Commit Preview URL Branch Preview URL |
May 12 2026, 01:42 PM |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 23d77e8366
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Add a request-local unlock flow before signing approvals so locked password and PIN wallets prompt for credentials before resolving a signing request. Wire the shared approval helper into sign message, sign transaction, sign and send transaction, and sign in request screens while leaving connect requests unchanged.
beeman
force-pushed
the
beeman/request-signing-unlock-ux
branch
from
23d77e8 to
69e4304
Compare
Member
Author
|
@codex review |
Member
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
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.
Add a request-local unlock flow before signing approvals so locked password and PIN wallets prompt for credentials before resolving a signing request.
Wire the shared approval helper into sign message, sign transaction, sign and send transaction, and sign in request screens while leaving connect requests unchanged.
Summary by CodeRabbit