feat(tui): leader-key chord bindings + copy-ref action#15
Conversation
Adds two related TUI affordances: 1. Multi-step keybindings. A binding string can now contain whitespace- separated chord steps (e.g. `"ctrl+x s"`). When the user presses the first step, the dashboard shows a `ctrl+x …` breadcrumb and waits for the continuation; an unrelated key aborts cleanly with a `chord aborted: …` toast and never falls through to the underlying view. Single-step bindings keep flowing through each view's existing priority match, so plain typing isn't affected. Resolution rule: multi-step chords always shadow single-step bindings on a shared first key (a `ctrl+x s` chord swallows `ctrl+x` as its prefix). Documented in `KeyMap::dispatch`. 2. `copy_ref` / `copy_ref_selected` actions copy `himitsu read <ref>` to the clipboard — the *command*, not the value. Useful when sharing "how to fetch this" in a PR, runbook, or chat without putting plaintext on the clipboard. Default binding is `Y` (Shift+Y) in both the search view and the secret viewer; cross-store rows get qualified with `-r <slug>` so the command is portable. Reuses `cli::search::store_label` for the same-store comparison. Internals: - `KeyChord` newtype wraps a sequence of `KeyBinding`s with whitespace- separated parsing, lossless `Display`, and a `from_events` constructor that lifts live `KeyEvent`s into the same canonical form (used by the breadcrumb formatter). - `KeyMap` fields migrate from `Vec<KeyBinding>` to `Vec<KeyChord>`. A new `KeyAction` enum names every keymap-driven action; `KeyMap::dispatch` drives the chord state machine and `action_for_key_in(&[KeyAction])` is shared by per-view priority lookups. - Each view exposes `dispatch_action(KeyAction) -> Option<ViewAction>` so completed chords route through the same path as bare keys. - `App` keeps a `pending_chord: Vec<KeyEvent>` plus a `chord_breadcrumb_active` flag (clean dismissal — no message-text heuristic). - Per-view priority slices live as `const &[KeyAction]` so the resolution order is data, not control flow. 655 tests pass, including new harness coverage for chord completion + chord abort. Clippy clean.
PR SummaryMedium Risk Overview Introduces copy-ref clipboard actions: Updates docs/config examples in Reviewed by Cursor Bugbot for commit 2f010e7. Configure here. |
![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: 2f010e77d3
ℹ️ 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 (@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 (@codex) address that feedback".
| /// would fetch this secret, not the value itself. Lets the user share | ||
| /// "how to grab this" in a PR / chat without leaking plaintext. | ||
| fn copy_ref_to_clipboard(&mut self) -> SecretViewerAction { | ||
| let cmd = format!("himitsu read {}", self.path); |
There was a problem hiding this comment.
When the viewer is opened from a multi-store search result that is not in the currently active store, this copied command omits the selected result's store, so pasting it later will run himitsu read <path> against the active/default store and either read the wrong secret or fail. The search-row copy path already qualifies cross-store refs with -r <store>; the viewer has store_label/store_path available and should apply the same logic here.
Useful? React with 👍 / 👎.
| self.move_to(self.step.prev()); | ||
| Some(NewSecretAction::None) | ||
| } | ||
| _ => None, |
There was a problem hiding this comment.
If a user configures next_field as a multi-step chord, App::on_key consumes the completed chord and calls NewSecretView::dispatch_action, but this match falls through for KeyAction::NextField; the key is swallowed and the form never advances or validates the current field. Since the new keymap supports chords for the full action list, handle NextField here with the same validate-then-advance behavior used by the single-key path.
Useful? React with 👍 / 👎.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Bugbot Autofix prepared fixes for all 3 issues found in the latest run.
- ✅ Fixed: Help key binding broken for single-step press
- I restored app-level single-step help handling before chord dispatch so
?consistently opens and toggles the help overlay across views.
- I restored app-level single-step help handling before chord dispatch so
- ✅ Fixed: Viewer copy-ref omits store qualifier for cross-store secrets
- The secret viewer now tracks the original active-store label and includes
-r <store>when copying read commands for secrets from a different store.
- The secret viewer now tracks the original active-store label and includes
- ✅ Fixed:
from_eventsdoesn't lowercase chars unlikeFromStrKeyChord::from_eventsnow lowercases uppercase character key codes while addingSHIFT, producing canonical display output likeshift+y.
Or push these changes by commenting:
@cursor push 144050d541
Preview (144050d541)
diff --git a/rust/src/tui/app.rs b/rust/src/tui/app.rs
--- a/rust/src/tui/app.rs
+++ b/rust/src/tui/app.rs
@@ -105,6 +105,15 @@
return None;
}
+ if self
+ .keymap
+ .action_for_key_in(&key, &[KeyAction::Help])
+ .is_some()
+ {
+ self.help = Some(self.help_for_current_view());
+ return None;
+ }
+
// ── Leader-key chord dispatcher ───────────────────────────────
// Drives the multi-step chord state machine. If the key is part
// of an in-flight chord (or starts one), it's swallowed here.
@@ -520,11 +529,21 @@
mod tests {
use super::*;
use crossterm::event::{KeyCode, KeyModifiers};
+ use std::path::PathBuf;
fn key(code: KeyCode, mods: KeyModifiers) -> KeyEvent {
KeyEvent::new(code, mods)
}
+ fn test_context() -> Context {
+ Context {
+ data_dir: PathBuf::new(),
+ state_dir: PathBuf::new(),
+ store: PathBuf::new(),
+ recipients_path: None,
+ }
+ }
+
#[test]
fn format_pending_renders_canonical_keys() {
let events = vec![
@@ -533,4 +552,16 @@
];
assert_eq!(format_pending(&events), "ctrl+x s");
}
+
+ #[test]
+ fn single_step_help_binding_toggles_overlay() {
+ let mut app = App::new(&test_context(), KeyMap::default());
+ assert!(app.help.is_none());
+
+ app.on_key(key(KeyCode::Char('?'), KeyModifiers::NONE));
+ assert!(app.help.is_some());
+
+ app.on_key(key(KeyCode::Char('?'), KeyModifiers::NONE));
+ assert!(app.help.is_none());
+ }
}
diff --git a/rust/src/tui/keymap.rs b/rust/src/tui/keymap.rs
--- a/rust/src/tui/keymap.rs
+++ b/rust/src/tui/keymap.rs
@@ -365,13 +365,14 @@
let steps: Vec<KeyBinding> = events
.iter()
.map(|ev| {
- let mut mods = ev.modifiers;
- if let KeyCode::Char(c) = ev.code {
- if c.is_ascii_uppercase() {
- mods |= KeyModifiers::SHIFT;
- }
- }
- KeyBinding::new(ev.code, mods)
+ let (code, mods) = match ev.code {
+ KeyCode::Char(c) if c.is_ascii_uppercase() => (
+ KeyCode::Char(c.to_ascii_lowercase()),
+ ev.modifiers | KeyModifiers::SHIFT,
+ ),
+ _ => (ev.code, ev.modifiers),
+ };
+ KeyBinding::new(code, mods)
})
.collect();
Self::try_new(steps)
@@ -914,6 +915,13 @@
}
#[test]
+ fn from_events_normalizes_uppercase_chars() {
+ let events = vec![key(KeyCode::Char('Y'), KeyModifiers::SHIFT)];
+ let chord = KeyChord::from_events(&events).unwrap();
+ assert_eq!(chord.to_string(), "shift+y");
+ }
+
+ #[test]
fn empty_chord_string_rejected() {
assert!("".parse::<KeyChord>().is_err());
assert!(" ".parse::<KeyChord>().is_err());
diff --git a/rust/src/tui/views/secret_viewer.rs b/rust/src/tui/views/secret_viewer.rs
--- a/rust/src/tui/views/secret_viewer.rs
+++ b/rust/src/tui/views/secret_viewer.rs
@@ -81,6 +81,9 @@
pub struct SecretViewerView {
/// Slug label for the store this secret lives in, used only for display.
store_label: String,
+ /// Canonical label for the app's active store at the moment this viewer
+ /// was opened. Used to decide whether copied read-refs need `-r`.
+ active_store_label: String,
/// Absolute store path. Needed by the crypto + rekey code paths.
store_path: PathBuf,
/// Secret path within the store (e.g. `prod/API_KEY`).
@@ -106,6 +109,7 @@
// Best-effort metadata read — if it fails we still show the path and
// let the user try to reveal (which will surface the real error).
let meta = store::read_secret_meta(&store_path, &path).unwrap_or_default();
+ let active_store_label = crate::cli::search::store_label(&ctx.store, ctx);
// The viewer inherits the outer context but must operate against the
// store the result came from, not whatever `ctx.store` happens to be.
@@ -126,6 +130,7 @@
Self {
store_label,
+ active_store_label,
store_path,
path,
meta,
@@ -389,7 +394,7 @@
/// would fetch this secret, not the value itself. Lets the user share
/// "how to grab this" in a PR / chat without leaking plaintext.
fn copy_ref_to_clipboard(&mut self) -> SecretViewerAction {
- let cmd = format!("himitsu read {}", self.path);
+ let cmd = format_read_command(&self.store_label, &self.path, &self.active_store_label);
match arboard::Clipboard::new().and_then(|mut c| c.set_text(cmd)) {
Ok(()) => SecretViewerAction::Copied,
Err(e) => SecretViewerAction::CopyFailed(format!("clipboard unavailable: {e}")),
@@ -697,6 +702,14 @@
keymap.action_for_key_in(key, VIEWER_ACTION_PRIORITY)
}
+fn format_read_command(row_store: &str, secret_path: &str, active_label: &str) -> String {
+ if row_store == active_label {
+ format!("himitsu read {secret_path}")
+ } else {
+ format!("himitsu -r {row_store} read {secret_path}")
+ }
+}
+
fn render_edit_doc(path: &str, decoded: &secret_value::Decoded) -> String {
let expires = decoded
.expires_at
@@ -978,6 +991,18 @@
}
#[test]
+ fn format_read_command_omits_store_for_active_store() {
+ let cmd = format_read_command("acme/prod", "prod/API_KEY", "acme/prod");
+ assert_eq!(cmd, "himitsu read prod/API_KEY");
+ }
+
+ #[test]
+ fn format_read_command_qualifies_cross_store_refs() {
+ let cmd = format_read_command("acme/shared", "prod/API_KEY", "acme/prod");
+ assert_eq!(cmd, "himitsu -r acme/shared read prod/API_KEY");
+ }
+
+ #[test]
fn ctrl_c_returns_quit() {
let km = KeyMap::default();
let (_dir, ctx, path) = seeded_store_with_secret();You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 2f010e7. Configure here.
| self.view = View::Search(SearchView::new(&self.ctx)); | ||
| self.push_toast("add remote cancelled", ToastKind::Info); | ||
| } | ||
| } |
There was a problem hiding this comment.
Help key binding broken for single-step press
High Severity
The ? key no longer opens the help overlay. The old app-level keymap.help.matches(&key) check was removed, and KeyAction::Help is only handled in run_keymap_action, which is exclusively called from the Dispatch::Match path (multi-step chord completions). Since dispatch intentionally ignores single-step chords, a bare ? press returns Dispatch::Unmatched and falls through to per-view handling. No view's priority list (SEARCH_ACTION_PRIORITY, VIEWER_ACTION_PRIORITY, FORM_ACTION_PRIORITY) includes KeyAction::Help. In the search view, ? is consumed as a query character; in other views it's silently ignored. The help entries still document ? as "toggle this help."
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 2f010e7. Configure here.
| Ok(()) => SecretViewerAction::Copied, | ||
| Err(e) => SecretViewerAction::CopyFailed(format!("clipboard unavailable: {e}")), | ||
| } | ||
| } |
There was a problem hiding this comment.
Viewer copy-ref omits store qualifier for cross-store secrets
Medium Severity
copy_ref_to_clipboard always emits himitsu read {path} without the -r <store> qualifier. When viewing a secret from a non-active store, the copied command would read from the wrong store. The search view correctly uses format_read_command which compares store labels and adds -r <slug> for cross-store secrets. The viewer has self.store_label available but doesn't perform the same comparison, producing an inconsistent and potentially incorrect command.
Reviewed by Cursor Bugbot for commit 2f010e7. Configure here.
| }) | ||
| .collect(); | ||
| Self::try_new(steps) | ||
| } |
There was a problem hiding this comment.
from_events doesn't lowercase chars unlike FromStr
Low Severity
KeyChord::from_events adds SHIFT for uppercase chars but doesn't lowercase the KeyCode::Char value, unlike KeyBinding::FromStr which normalizes 'Y' to Char('y') + SHIFT. This means the breadcrumb toast renders "shift+Y" instead of the canonical "shift+y" that matches config syntax, contradicting the docstring's "round-trips cleanly through Display" claim.
Reviewed by Cursor Bugbot for commit 2f010e7. Configure here.
1 participant
Summary
Two TUI affordances that go together:
1. Leader-key chord bindings
Binding strings can now contain whitespace-separated chord steps. Set
save_secret: [\"ctrl+x s\"]in your config and press Ctrl+X then `s` to save — terminal-conflict-free (Ctrl+S → XOFF is the canonical pain point).When the first step lands, the dashboard shows a `ctrl+x …` breadcrumb at the bottom of the screen and waits for the continuation. A non-continuation key aborts cleanly with a `chord aborted: …` toast and never reaches the active view. Single-step bindings keep flowing through each view's existing per-action priority match, so plain typing into the search box (or the new-secret form) isn't affected.
Resolution rule: multi-step chords shadow single-step bindings on a shared first key. If you bind both `edit: ["e"]` and `delete: ["e d"]`, pressing `e` enters Pending state — the single `e` can't fire without a non-`d` continuation that aborts the chord. Documented in `KeyMap::dispatch`.
2. Copy-ref action
Default `Y` (shift+y) in both the search view and the secret viewer copies `himitsu read ` — the command — to the clipboard, instead of the secret value. Lets you tell a teammate or a PR comment how to fetch a secret without leaking the plaintext. Cross-store rows get qualified with `-r ` so the command works in a fresh shell that hasn't picked the same default store. Same-store comparison reuses `cli::search::store_label`, which is the function that populates `SearchResult.store` in the first place.
Internals
Test plan
Out of scope (file as follow-ups)