Add nsec keys tab and rename shares to FROST shares#285
Merged
kwsantiago merged 7 commits intomainfrom Feb 27, 2026
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a new NsecKeys UI and navigation flow: listing, activation, export, two‑step delete; integrates NsecKeys into message routing, sidebar badge, import/delete synchronization, and shared formatting helpers. Public Screen enum gains Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant UI as UI (Iced)
participant App as App (keep-desktop::app)
participant Vault as Vault (key storage)
participant FS as Filesystem
rect rgba(200,230,255,0.5)
User->>UI: Click "Nsec Keys" (NavigateNsecKeys)
UI->>App: Message(NavigateNsecKeys)
App->>App: stop scanner, reset feedback, clear caches
App->>Vault: Read key records
Vault-->>App: KeyRecord list
App->>UI: Render NsecKeysScreen(keys, active)
end
rect rgba(200,255,200,0.5)
User->>UI: Click "Delete" (RequestDeleteNsecKey)
UI->>App: Message(RequestDeleteNsecKey)
App->>UI: Show confirmation row (set delete_confirm)
User->>UI: Confirm delete (ConfirmDeleteNsecKey)
UI->>App: Message(ConfirmDeleteNsecKey)
App->>FS: Remove key files / update configs
FS-->>App: Success/Err
App->>Vault: Refresh keys / refresh shares if needed
Vault-->>App: Updated key list
App->>UI: Update NsecKeysScreen + Toast(success/err)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ 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}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
keep-desktop/src/screen/nsec_keys.rs (1)
46-55: Consider extractingtruncated_npubto avoid duplication.This truncation logic is duplicated from
Identity::truncated_npubinmessage.rs(lines 31-40). Consider extracting this into a shared utility function to maintain consistency and reduce duplication.♻️ Suggested refactor
Create a shared helper function in a common module (e.g.,
utils.rsor withinmessage.rs):pub fn truncate_npub(npub: &str) -> String { if !npub.is_ascii() || npub.len() <= 20 { return npub.to_string(); } format!( "{}...{}", &npub[..12], &npub[npub.len() - 6..] ) }Then use this from both
Identity::truncated_npubandNsecKeyEntry::truncated_npub.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@keep-desktop/src/screen/nsec_keys.rs` around lines 46 - 55, Extract the duplicated truncation logic into a single helper function (e.g., pub fn truncate_npub(npub: &str) -> String) placed in a shared module (such as a new utils.rs or the existing message.rs) and replace both NsecKeyEntry::truncated_npub and Identity::truncated_npub to call this helper; ensure the helper uses the same conditions (ascii check and length <= 20) and the same slicing (first 12 chars + "..." + last 6 chars) so both truncated_npub implementations delegate to truncate_npub(npub).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@keep-desktop/src/screen/nsec_keys.rs`:
- Around line 46-55: Extract the duplicated truncation logic into a single
helper function (e.g., pub fn truncate_npub(npub: &str) -> String) placed in a
shared module (such as a new utils.rs or the existing message.rs) and replace
both NsecKeyEntry::truncated_npub and Identity::truncated_npub to call this
helper; ensure the helper uses the same conditions (ascii check and length <=
20) and the same slicing (first 12 chars + "..." + last 6 chars) so both
truncated_npub implementations delegate to truncate_npub(npub).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
keep-desktop/src/app.rskeep-desktop/src/message.rskeep-desktop/src/screen/layout.rskeep-desktop/src/screen/mod.rskeep-desktop/src/screen/nsec_keys.rs
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
kwsantiago
approved these changes
Feb 27, 2026
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@keep-desktop/src/app.rs`:
- Around line 1874-1891: The match over delete_result currently ignores the None
case, leaving the NsecKeys delete_confirm stale and giving no user feedback when
the vault is locked/unavailable; update the None arm to mirror the Err branch
for the Nsec delete flow: if let Screen::NsecKeys(s) = &mut self.screen {
s.delete_confirm = None; } and call self.set_toast with a clear message (e.g.,
friendly_err for a locked-vault error or a specific "vault locked/unavailable"
message) so users get feedback when delete_result is None; keep the existing
Some(Ok(())) behavior that removes relay files, refreshes shares, and shows the
success toast.
- Around line 1630-1633: The badge count is only computed from the current
screen (nsec_count using match on self.screen -> Screen::NsecKeys(s) and
s.keys), so it disappears when you're off the Nsec screen; change the logic to
derive the count from the underlying persistent nsec key store instead of the
active screen. Replace the match that sets nsec_count with a call or inline
check that reads the canonical nsec key collection (e.g. a new helper like
get_nsec_key_count() that inspects the module/field that holds imported keys)
and returns Some(len) when keys exist, otherwise None, while leaving the
Screen::NsecKeys branch as a fallback if needed (use self.<nsec_store_or_field>
or add an accessor if none exists).
- Around line 2357-2361: The call to resolve_active_share(&shares) is running
before the Nsec keys screen is rendered and can overwrite an active nsec
identity; to fix this, prevent resolve_active_share from switching the active
identity during an Nsec import by either (A) moving the
resolve_active_share(&shares) call to after set_nsec_keys_screen() so the
UI/state for NsecKeys is established before any share-based active-identity
resolution, or (B) add a guard inside resolve_active_share (or around its
invocation) to skip switching when the newly rendered screen will be the Nsec
keys flow (use the presence of NsecKeys state or a flag set by
set_nsec_keys_screen()). Update the code paths that call
refresh_identities(&shares), set_nsec_keys_screen(), and
resolve_active_share(&shares) accordingly so Nsec imports never get overwritten
by FROST share resolution.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
keep-desktop/src/app.rskeep-desktop/src/screen/layout.rskeep-desktop/src/screen/nsec_keys.rs
There was a problem hiding this comment.
♻️ Duplicate comments (1)
keep-desktop/src/app.rs (1)
1812-1819:⚠️ Potential issue | 🟠 MajorPreserve active Nsec identity when refreshing shares.
refresh_shares()still unconditionally callsresolve_active_share(&shares)(Line 1815), which is share-only resolution. If the active identity is Nsec, this can overwriteactive_share_hexduring refresh paths (including Nsec delete success path at Line 1894).Suggested patch
fn refresh_shares(&mut self) { let shares = self.current_shares(); + let nsec_keys = self.current_nsec_keys(); self.cached_share_count = shares.len(); - self.cached_nsec_count = self.current_nsec_keys().len(); - self.resolve_active_share(&shares); + self.cached_nsec_count = nsec_keys.len(); + let active_is_nsec = self + .active_share_hex + .as_deref() + .is_some_and(|hex| nsec_keys.iter().any(|k| k.pubkey_hex == hex)); + if !active_is_nsec { + self.resolve_active_share(&shares); + } self.refresh_identities(&shares); if matches!(self.screen, Screen::NsecKeys(_)) { self.set_nsec_keys_screen();Also applies to: 1888-1895
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@keep-desktop/src/app.rs` around lines 1812 - 1819, In refresh_shares(), avoid overwriting a currently active Nsec identity by not calling resolve_active_share(&shares) when the UI is showing or the active identity is an Nsec; instead only call resolve_active_share when the active screen/identity is a share (e.g. when !matches!(self.screen, Screen::NsecKeys(_)) or inside the Screen::ShareList branch). Ensure active_share_hex is left unchanged for Nsec flows (preserve its value during the NsecKeys path) and keep the existing set_nsec_keys_screen()/refresh_identities(&shares) behavior for Nsec refreshes.
🧹 Nitpick comments (1)
keep-desktop/src/app.rs (1)
1768-1785: Resetimport_return_to_nsecduring lock to avoid stale nav state.Small state-hygiene improvement: clear the import return flag in
do_lock()alongside the cached counters.Suggested patch
fn do_lock(&mut self) -> Task<Message> { self.stop_scanner(); self.handle_disconnect_relay(); self.stop_bunker(); @@ self.identities.clear(); + self.import_return_to_nsec = false; self.cached_share_count = 0; self.cached_nsec_count = 0; self.identity_switcher_open = false;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@keep-desktop/src/app.rs` around lines 1768 - 1785, In do_lock(), after resetting cached_nsec_count (and before identity_switcher_open), reset the import return flag to avoid stale navigation state by setting self.import_return_to_nsec = false; (i.e. add this assignment in the do_lock method alongside the other state clears such as cached_share_count and cached_nsec_count).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@keep-desktop/src/app.rs`:
- Around line 1812-1819: In refresh_shares(), avoid overwriting a currently
active Nsec identity by not calling resolve_active_share(&shares) when the UI is
showing or the active identity is an Nsec; instead only call
resolve_active_share when the active screen/identity is a share (e.g. when
!matches!(self.screen, Screen::NsecKeys(_)) or inside the Screen::ShareList
branch). Ensure active_share_hex is left unchanged for Nsec flows (preserve its
value during the NsecKeys path) and keep the existing
set_nsec_keys_screen()/refresh_identities(&shares) behavior for Nsec refreshes.
---
Nitpick comments:
In `@keep-desktop/src/app.rs`:
- Around line 1768-1785: In do_lock(), after resetting cached_nsec_count (and
before identity_switcher_open), reset the import return flag to avoid stale
navigation state by setting self.import_return_to_nsec = false; (i.e. add this
assignment in the do_lock method alongside the other state clears such as
cached_share_count and cached_nsec_count).
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
2 participants
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.
Summary
Closes #284
Summary by CodeRabbit
New Features
Bug Fixes
Chores