fix: harden sync and failure handling

.github/workflows/ci.yml

@@ -48,7 +48,7 @@ jobs:
         run: cargo fmt --all -- --check
 
       - name: Run clippy
-        run: cargo clippy --all-targets --all-features -- -D warnings
+        run: cargo clippy --all-targets --all-features --locked -- -D warnings
 
       - name: Run tests
-        run: cargo test --all-targets --all-features
+        run: cargo test --all-targets --all-features --locked

.github/workflows/release.yml

@@ -330,7 +330,6 @@ jobs:
     uses: ./.github/workflows/publish-npm.yml
     with:
       plan: ${{ needs.plan.outputs.val }}
-    secrets: inherit
     # publish jobs get escalated permissions
     permissions:
       "contents": "read"

README.md

@@ -50,6 +50,7 @@ cargo install --git https://github.com/AksharP5/blippy
 
 - `blippy`: launch the TUI
 - `blippy --version`: show version information
+- `blippy --help`: show command help
 - `blippy sync`: scan local repos and cache GitHub remotes
 - `blippy auth reset`: remove stored auth token from keychain
 - `blippy cache reset`: remove local cache database

dist-workspace.toml

@@ -18,6 +18,8 @@ hosting = "github"
 # Use a custom npm job because the built-in publisher requires NPM_TOKEN.
 publish-jobs = ["homebrew", "./publish-npm"]
 github-custom-job-permissions = { "publish-npm" = { contents = "read", "id-token" = "write" } }
+# The generated caller inherits every repository secret. Keep the narrower OIDC-only workflow.
+allow-dirty = ["ci"]
 # Homebrew tap to push formula updates to
 tap = "AksharP5/homebrew-tap"
 # Archive formats for generated bundles

keybinds.example.toml

@@ -8,6 +8,7 @@
 # - single chars: "q", "/", "1"
 # - named keys: "enter", "esc", "tab", "space", "left", "right", "home", "end", "up", "down", "pageup", "pagedown", "backspace"
 # - modifiers: "ctrl+...", "alt+...", "shift+..."
+# Plain characters remain text in editors and search fields. Use a modified or named key there.
 
 [keybinds]
 quit = "ctrl+c"

src/app.rs

@@ -403,6 +403,7 @@ struct PullRequestState {
     pull_request_files_issue_id: Option<i64>,
     pull_request_id: Option<String>,
     pull_request_files: Vec<PullRequestFile>,
+    pull_request_view_state_loaded: bool,
     pull_request_viewed_files: HashSet<String>,
     pull_request_collapsed_hunks: HashMap<String, HashSet<usize>>,
     pull_request_review_comments: Vec<PullRequestReviewComment>,
@@ -428,6 +429,7 @@ impl Default for PullRequestState {
             pull_request_files_issue_id: None,
             pull_request_id: None,
             pull_request_files: Vec::new(),
+            pull_request_view_state_loaded: false,
             pull_request_viewed_files: HashSet::new(),
             pull_request_collapsed_hunks: HashMap::new(),
             pull_request_review_comments: Vec::new(),

src/app/editor.rs

@@ -230,7 +230,9 @@ impl App {
                         self.comment_editor.backspace_text();
                     }
                 }
-                KeyCode::Char(ch) => {
+                KeyCode::Char(ch)
+                    if key.modifiers.is_empty() || key.modifiers == KeyModifiers::SHIFT =>
+                {
                     if self.comment_editor.create_issue_confirm_visible() {
                         return;
                     }

src/app/input.rs

@@ -2,31 +2,41 @@ use super::*;
 
 impl App {
     pub fn on_key(&mut self, key: KeyEvent) {
-        let key = match self.keybinds.remap_key(key) {
-            Some(key) => key,
-            None => return,
-        };
         if matches!(self.view, View::CommentPresetName | View::CommentEditor) {
+            let Some(key) = self.keybinds.remap_text_key(key) else {
+                return;
+            };
             self.handle_editor_key(key);
             return;
         }
-        if self.view == View::RepoPicker
-            && self.search.repo_search_mode
-            && self.handle_repo_search_key(key)
-        {
-            return;
+        if self.view == View::RepoPicker && self.search.repo_search_mode {
+            let Some(key) = self.keybinds.remap_text_key(key) else {
+                return;
+            };
+            if self.handle_repo_search_key(key) {
+                return;
+            }
         }
-        if self.view == View::Issues
-            && self.search.issue_search_mode
-            && self.handle_issue_search_key(key)
-        {
-            return;
+        if self.view == View::Issues && self.search.issue_search_mode {
+            let Some(key) = self.keybinds.remap_text_key(key) else {
+                return;
+            };
+            if self.handle_issue_search_key(key) {
+                return;
+            }
         }
-        if matches!(self.view, View::LabelPicker | View::AssigneePicker)
-            && self.handle_popup_filter_key(key)
-        {
-            return;
+        if matches!(self.view, View::LabelPicker | View::AssigneePicker) {
+            let Some(key) = self.keybinds.remap_text_key(key) else {
+                return;
+            };
+            if self.handle_popup_filter_key(key) {
+                return;
+            }
         }
+        let key = match self.keybinds.remap_key(key) {
+            Some(key) => key,
+            None => return,
+        };
         if key.modifiers.contains(KeyModifiers::CONTROL)
             && key.code == KeyCode::Char('r')
             && self.view == View::RepoPicker

src/app/pull_request.rs

@@ -15,6 +15,10 @@ impl App {
             .contains(file_path)
     }
 
+    pub fn pull_request_view_state_loaded(&self) -> bool {
+        self.pull_request.pull_request_view_state_loaded
+    }
+
     pub fn pull_request_hunk_is_collapsed(&self, file_path: &str, hunk_start: usize) -> bool {
         self.pull_request
             .pull_request_collapsed_hunks
@@ -60,6 +64,9 @@ impl App {
     }
 
     pub fn selected_pull_request_file_view_toggle(&self) -> Option<(String, bool)> {
+        if !self.pull_request_view_state_loaded() {
+            return None;
+        }
         let file = self.selected_pull_request_file_row()?;
         let viewed = self.pull_request_file_is_viewed(file.filename.as_str());
         Some((file.filename.clone(), !viewed))
@@ -71,6 +78,7 @@ impl App {
         viewed_files: HashSet<String>,
     ) {
         self.pull_request.pull_request_id = pull_request_id;
+        self.pull_request.pull_request_view_state_loaded = true;
         self.pull_request.pull_request_viewed_files = viewed_files;
         self.pull_request
             .pull_request_viewed_files
@@ -209,6 +217,7 @@ impl App {
     pub fn set_pull_request_files(&mut self, issue_id: i64, files: Vec<PullRequestFile>) {
         self.pull_request.pull_request_files_issue_id = Some(issue_id);
         self.pull_request.pull_request_id = None;
+        self.pull_request.pull_request_view_state_loaded = false;
         self.pull_request.pull_request_files = files;
         let mut active_file_paths = HashSet::new();
         for file in &self.pull_request.pull_request_files {
@@ -342,6 +351,7 @@ impl App {
     pub(super) fn reset_pull_request_state(&mut self) {
         self.pull_request.pull_request_files_issue_id = None;
         self.pull_request.pull_request_id = None;
+        self.pull_request.pull_request_view_state_loaded = false;
         self.pull_request.pull_request_files.clear();
         self.pull_request.pull_request_viewed_files.clear();
         self.pull_request.pull_request_collapsed_hunks.clear();

src/app/state.rs

@@ -251,6 +251,8 @@ impl App {
         self.context.path = path.map(ToString::to_string);
         self.context.issue_id = None;
         self.context.issue_number = None;
+        self.sync.syncing = false;
+        self.reset_issue_sync_state();
         self.sync.repo_permissions_syncing = false;
         self.sync.repo_permissions_sync_requested = true;
         self.sync.repo_issue_metadata_editable = None;
@@ -263,6 +265,7 @@ impl App {
         self.linked.pull_request_lookups.clear();
         self.linked.issue_lookups.clear();
         self.linked.navigation_origin = None;
+        self.interaction.pending_issue_actions.clear();
         self.clear_linked_picker_state();
         self.reset_pull_request_state();
         self.search.repo_search_mode = false;
@@ -273,13 +276,24 @@ impl App {
     }
 
     pub fn set_current_issue(&mut self, issue_id: i64, issue_number: i64) {
+        let issue_changed = self.context.issue_id != Some(issue_id);
         self.context.issue_id = Some(issue_id);
         self.context.issue_number = Some(issue_number);
-        if self.pull_request.pull_request_files_issue_id != Some(issue_id) {
+        if issue_changed {
+            self.reset_issue_sync_state();
             self.reset_pull_request_state();
         }
     }
 
+    fn reset_issue_sync_state(&mut self) {
+        self.sync.comment_syncing = false;
+        self.sync.pull_request_files_syncing = false;
+        self.sync.pull_request_review_comments_syncing = false;
+        self.sync.comment_sync_requested = false;
+        self.sync.pull_request_files_sync_requested = false;
+        self.sync.pull_request_review_comments_sync_requested = false;
+    }
+
     pub fn update_issue_state_by_number(&mut self, issue_number: i64, state: &str) {
         for issue in &mut self.issues {
             if issue.number == issue_number {

src/app/tests/part2.rs

@@ -125,6 +125,22 @@ fn slash_search_matches_issue_number() {
     );
 }
 
+#[test]
+fn custom_plain_keybinding_does_not_rewrite_search_text() {
+    let mut config = Config::default();
+    config
+        .keybinds
+        .insert("refresh".to_string(), "x".to_string());
+    let mut app = App::new(config);
+    app.set_view(View::Issues);
+    app.on_key(KeyEvent::new(KeyCode::Char('/'), KeyModifiers::NONE));
+
+    app.on_key(KeyEvent::new(KeyCode::Char('x'), KeyModifiers::NONE));
+    app.on_key(KeyEvent::new(KeyCode::Char('r'), KeyModifiers::NONE));
+
+    assert_eq!(app.issue_query(), "xr");
+}
+
 #[test]
 fn reopen_action_for_closed_issue() {
     let mut app = App::new(Config::default());
@@ -610,6 +626,9 @@ fn selected_pull_request_file_view_toggle_flips_current_state() {
             patch: Some("@@ -1,1 +1,1 @@\n-old\n+new".to_string()),
         }],
     );
+    assert!(app.selected_pull_request_file_view_toggle().is_none());
+
+    app.set_pull_request_view_state(Some("PR_id".to_string()), std::collections::HashSet::new());
 
     let (path, viewed) = app
         .selected_pull_request_file_view_toggle()

src/app/tests/part3.rs

@@ -697,6 +697,38 @@ fn create_issue_editor_supports_title_and_body_entry() {
     assert_eq!(app.take_action(), Some(AppAction::SubmitCreatedIssue));
 }
 
+#[test]
+fn custom_plain_keybinding_does_not_rewrite_editor_text() {
+    let mut config = Config::default();
+    config
+        .keybinds
+        .insert("refresh".to_string(), "x".to_string());
+    let mut app = App::new(config);
+    app.open_issue_comment_editor(View::Issues);
+
+    app.on_key(KeyEvent::new(KeyCode::Char('x'), KeyModifiers::NONE));
+    app.on_key(KeyEvent::new(KeyCode::Char('r'), KeyModifiers::NONE));
+
+    assert_eq!(app.editor().text(), "xr");
+}
+
+#[test]
+fn custom_modified_submit_key_still_works_in_editor() {
+    let mut config = Config::default();
+    config
+        .keybinds
+        .insert("submit".to_string(), "ctrl+s".to_string());
+    let mut app = App::new(config);
+    app.open_issue_comment_editor(View::Issues);
+
+    app.on_key(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
+    assert_eq!(app.take_action(), None);
+
+    app.on_key(KeyEvent::new(KeyCode::Char('s'), KeyModifiers::CONTROL));
+
+    assert_eq!(app.take_action(), Some(AppAction::SubmitIssueComment));
+}
+
 #[test]
 fn create_issue_confirm_cancel_keeps_editor_open() {
     let mut app = App::new(Config::default());

src/app/tests/part4.rs

@@ -36,6 +36,26 @@ fn repo_picker_search_filters_entries() {
     assert_eq!(app.filtered_repo_rows().len(), 2);
 }
 
+#[test]
+fn changing_repo_clears_repo_scoped_in_flight_state() {
+    let mut app = App::new(Config::default());
+    app.set_current_repo_with_path("acme", "one", None);
+    app.set_current_issue(10, 7);
+    app.set_syncing(true);
+    app.set_comment_syncing(true);
+    app.set_pull_request_files_syncing(true);
+    app.set_pull_request_review_comments_syncing(true);
+    app.set_pending_issue_action(7, super::super::PendingIssueAction::Closing);
+
+    app.set_current_repo_with_path("acme", "two", None);
+
+    assert!(!app.syncing());
+    assert!(!app.comment_syncing());
+    assert!(!app.pull_request_files_syncing());
+    assert!(!app.pull_request_review_comments_syncing());
+    assert_eq!(app.pending_issue_badge(7), None);
+}
+
 #[test]
 fn ctrl_g_resets_repo_picker_query_when_reopened() {
     let mut app = App::new(Config::default());