From b7984d9d75456b16f6c886d022d64741148ebc80 Mon Sep 17 00:00:00 2001 From: xscriptor <“preciado.oscar.osorio@gmail.com”> Date: Thu, 18 Jun 2026 19:12:27 +0200 Subject: [PATCH 1/4] general updates on app --- CHANGELOG.md | 40 ++++++ CONTRIBUTING.md | 12 +- Cargo.lock | 10 ++ Cargo.toml | 1 + docs/PROVIDER_CONFIGURATION.md | 255 +++++++++++++++++++++++++++++++++ src/app/actions.rs | 65 +++++++-- src/app/commands.rs | 89 +++++------- src/app/input/fields.rs | 14 +- src/app/input/nav.rs | 51 ++----- src/app/mod.rs | 29 +++- src/cache.rs | 4 +- src/cli/git.rs | 33 +++-- src/cli/helpers.rs | 44 +++--- src/error.rs | 2 - src/github/mod.rs | 24 +--- src/github/provider_impl.rs | 251 ++++++++++++++++++++++++++++++++ src/lib.rs | 3 + src/main.rs | 208 +++++++++++++++------------ src/models/misc.rs | 1 - src/models/pr.rs | 1 - src/models/release.rs | 1 - src/models/repo.rs | 1 - src/oauth.rs | 35 ++--- src/oauth_session.rs | 32 ++--- src/provider.rs | 192 +++++++++++++++++++++++++ src/runtime.rs | 12 ++ src/task_manager.rs | 57 ++++++++ tests/auth_precedence_tests.rs | 15 +- 28 files changed, 1162 insertions(+), 320 deletions(-) create mode 100644 docs/PROVIDER_CONFIGURATION.md create mode 100644 src/github/provider_impl.rs create mode 100644 src/provider.rs create mode 100644 src/runtime.rs create mode 100644 src/task_manager.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 731ade2..03d50e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,45 @@ # Changelog +## v0.1.2 + +### Added + +- **GitProvider trait**: New `GitProvider` trait in `src/provider.rs` abstracts all GitHub API methods (~28 methods), enabling future support for Azure DevOps, GitLab, and other providers. (`src/provider.rs`, `src/github/provider_impl.rs`) + +- **Provider auto-detection**: `detect_provider(url)` function parses git remote URLs to identify GitHub, Azure DevOps, GitLab, Bitbucket, and other providers. (`src/provider.rs`) + +- **Provider factory**: `create_provider(kind, token)` returns `Arc`, making provider selection a single call site. (`src/provider.rs`) + +- **TaskManager**: New `TaskManager` in `src/task_manager.rs` tracks all background `JoinHandle`s instead of discarding them. Threads are named `"gitnapse-worker"` for debugging. (`src/task_manager.rs`) + +- **Graceful shutdown**: `task_manager.join_all()` is called before the TUI exits, ensuring all background threads complete cleanly. (`src/app/mod.rs`) + +- **OAuthSession zeroize on drop**: `OAuthSession` now implements `Drop` to zeroize `access_token` and `refresh_token` fields when the session is dropped. (`src/oauth_session.rs`) + +### Changed + +- **Unified tokio runtime**: Three separate `OnceLock` instances (GitHubClient, oauth, oauth_session) consolidated into a single shared runtime at `src/runtime.rs`. (`src/runtime.rs`, `src/github/mod.rs`, `src/oauth.rs`, `src/oauth_session.rs`) + +- **Threads migrated to TaskManager**: All 19 `std::thread::spawn` calls replaced with `self.task_manager.spawn()`, providing thread tracking, naming, and cleanup. (`src/app/commands.rs`, `src/app/input/nav.rs`) + +- **Token input no longer exposes secret on every keypress**: `handle_token_input` now accumulates characters in a plain `String` (reusing `input_buffer`) and only creates a `SecretString` when saving on Enter, eliminating heap copies of the secret on each keystroke. (`src/app/input/fields.rs`) + +- **Main.rs dispatch refactored**: The ~40-arm match in `main()` extracted into 9 named `dispatch_*` functions (dispatch_stash, dispatch_tag, dispatch_pr, dispatch_issue, dispatch_remote, dispatch_config, dispatch_release, dispatch_repo, dispatch_auth, dispatch_oauth). (`src/main.rs`) + +- **Cache write errors logged**: `fs::write` failures in cache are now logged via `log::warn!` instead of being silently discarded. (`src/cache.rs`) + +- **Test env var safety**: Replaced `unsafe { std::env::set_var }` in integration tests and `auth_precedence_tests` with `temp_env::with_var`. (`src/github/mod.rs`, `tests/auth_precedence_tests.rs`) + +- **handle_api_error accepts anyhow::Error**: The CLI error handler now takes `&anyhow::Error` and downcasts to `GitHubError` internally, making it compatible with the new `GitProvider` trait. (`src/cli/helpers.rs`) + +- **Duplicate tree_text_mode logic removed**: Extracted to a single `toggle_tree_view()` method in `App`, called from both `commands.rs` and `nav.rs`. (`src/app/actions.rs`) + +- **Unused imports cleaned up**: Removed stale `Arc`, `Context`, `GitHubClient`, `Line`, and `GitProvider` imports across actions.rs, commands.rs, git.rs, and oauth.rs. + +### Fixed + +- **#![allow(dead_code)] removed from 5 files**: Replaced crate-level allows with specific fields or `Drop` implementations where needed. (`src/error.rs`, `src/models/repo.rs`, `src/models/pr.rs`, `src/models/misc.rs`, `src/models/release.rs`) + ## v0.1.1 ### Fixed diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 10abc1d..6189bc8 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -13,10 +13,18 @@ Thanks for your interest in contributing. ## Local Validation -Run at minimum: +The CI runs three mandatory checks. Run **all of them locally** before pushing: ```bash -cargo check +cargo fmt --check # style: must pass without diff +cargo clippy -- -D warnings # lints: zero warnings allowed +cargo test # tests: all must pass +``` + +To auto-fix formatting issues: + +```bash +cargo fmt ``` If your changes affect behavior, update documentation in `README.md` and `docs/`. diff --git a/Cargo.lock b/Cargo.lock index a782f89..67eda66 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1682,6 +1682,7 @@ dependencies = [ "serde", "serde_json", "serial_test", + "temp-env", "tempfile", "thiserror 2.0.18", "tokio", @@ -4509,6 +4510,15 @@ version = "1.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "55937e1799185b12863d447f42597ed69d9928686b8d88a1df17376a097d8369" +[[package]] +name = "temp-env" +version = "0.3.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "96374855068f47402c3121c6eed88d29cb1de8f3ab27090e273e420bdabcf050" +dependencies = [ + "parking_lot", +] + [[package]] name = "tempfile" version = "3.27.0" diff --git a/Cargo.toml b/Cargo.toml index 07d343a..fc4b76d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -35,6 +35,7 @@ webbrowser = "1.0" [dev-dependencies] mockito = "1.7" serial_test = "3.2" +temp-env = "0.3" tempfile = "3.20" [package.metadata.deb] diff --git a/docs/PROVIDER_CONFIGURATION.md b/docs/PROVIDER_CONFIGURATION.md new file mode 100644 index 0000000..2d1940f --- /dev/null +++ b/docs/PROVIDER_CONFIGURATION.md @@ -0,0 +1,255 @@ +

Provider Configuration

+ +
+

Contents

+ + +

Overview

+ +GitNapse uses a provider abstraction to interact with git hosting +services. The GitProvider trait defines a uniform interface for all +API operations, and concrete implementations translate those calls into the +specific REST API of each service. + +The provider system lives in src/provider.rs. The trait covers ~28 +methods including repository search, file content retrieval, branch management, +pull request operations, issue tracking, CI status, and release management. + +

Default Provider (GitHub)

+ +By default, GitNapse uses the GitHub API. No configuration is needed. The +GitHubClient implements the GitProvider trait and +connects to https://api.github.com. + +The API base URL can be overridden via the environment variable: + +
+GITNAPSE_GITHUB_API=https://your-enterprise-server.example.com/api/v3
+
+ +

Provider Kind Table

+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
ProviderKindDisplay NameDetection Pattern (URL contains)
GitHubGitHubgithub
AzureDevOpsAzure DevOpsdev.azure or visualstudio.com
GitLabGitLabgitlab
BitbucketBitbucketbitbucket
OtherOtherEverything else (falls back to GitHub API)
+ +

Auto-Detection from Remote URL

+ +GitNapse can automatically detect the provider from a git remote URL. The +detect_provider(remote_url) function performs simple string +matching on the lowercased URL: + +
+use gitnapse::provider::{detect_provider, ProviderKind};
+
+let kind = detect_provider("https://github.com/owner/repo.git");
+assert_eq!(kind, ProviderKind::GitHub);
+
+let kind = detect_provider("https://dev.azure.com/org/project/_git/repo");
+assert_eq!(kind, ProviderKind::AzureDevOps);
+
+let kind = detect_provider("https://gitlab.example.com/group/project.git");
+assert_eq!(kind, ProviderKind::GitLab);
+
+ +This detection is not yet wired into the TUI's runtime provider selection. It is +available for CLI usage and as a building block for future automatic provider +switching based on the current repository's remote. + +

Adding a Custom Provider

+ +To add support for a new provider (e.g., Azure DevOps), follow these steps: + +

1. Create the provider module

+ +Create a new file, for example src/azure_devops.rs, with a struct +that implements GitProvider: + +
+use crate::error::GitHubError;
+use crate::models::*;
+use crate::provider::GitProvider;
+use anyhow::Result;
+use reqwest::Client;
+use std::sync::Mutex;
+
+pub struct AzureDevOpsProvider {
+    client: Client,
+    organization: String,
+    project: String,
+    // rate-limit tracking, etc.
+}
+
+impl GitProvider for AzureDevOpsProvider {
+    fn search_repositories_page(
+        &self,
+        query: &str,
+        page: u32,
+        per_page: u8,
+    ) -> Result> {
+        // Azure DevOps Git Repositories API call
+        // ...
+    }
+
+    // ... implement all other trait methods
+}
+
+ +

2. Register the provider kind

+ +Add a variant for your provider to the ProviderKind enum in +src/provider.rs (or reuse Other): + +
+pub enum ProviderKind {
+    GitHub,
+    AzureDevOps,
+    GitLab,
+    Bitbucket,
+    Other,
+}
+
+ +Update detect_provider() and ProviderKind::display_name() +accordingly. + +

3. Add the factory case

+ +Update create_provider() in src/provider.rs: + +
+pub fn create_provider(
+    kind: ProviderKind,
+    token: Option<&str>,
+) -> Result> {
+    match kind {
+        ProviderKind::GitHub => {
+            let client = crate::github::GitHubClient::new(token)?;
+            Ok(Arc::new(client))
+        }
+        ProviderKind::AzureDevOps => {
+            let client = AzureDevOpsProvider::new(token)?;
+            Ok(Arc::new(client))
+        }
+        ProviderKind::Other => {
+            // fallback
+            let client = crate::github::GitHubClient::new(token)?;
+            Ok(Arc::new(client))
+        }
+        _ => {
+            // forward-compatible: return a helpful error or fallback
+            anyhow::bail!("unsupported provider: {:?}", kind);
+        }
+    }
+}
+
+ +

4. Register the module

+ +Add pub mod azure_devops; to src/lib.rs. + +

Provider Architecture

+ +The provider layer is designed for minimal coupling with the rest of the +application: + +
+src/
+  provider.rs          -- GitProvider trait, ProviderKind enum, factory
+  github/
+    mod.rs             -- GitHubClient struct definition
+    provider_impl.rs   -- impl GitProvider for GitHubClient
+    ...
+  (future)
+  azure_devops.rs      -- AzureDevOpsProvider (example)
+  gitlab.rs            -- GitLabProvider (example)
+
+ +The TUI (App) holds Arc<dyn GitProvider> and +calls trait methods directly. The CLI creates a provider via +helpers::make_client() which calls +create_provider(ProviderKind::GitHub, token). + +

Error Handling

+ +All trait methods return anyhow::Result<T>. Provider-specific +errors (e.g., GitHubError) are wrapped inside anyhow::Error +and can be extracted via e.downcast_ref::<GitHubError>() when +provider-specific error details are needed. + +

Rate Limiting

+ +The trait includes two optional rate-limit methods: +rate_limit_remaining() and rate_limit_reset(). +Providers that do not support rate-limit tracking can simply return +None from both. + +

Future Providers

+ +

Azure DevOps

+ +Azure DevOps uses the Azure REST API with a different URL structure: +
+https://dev.azure.com/{organization}/{project}/_apis/git/repositories
+
+ +Authentication uses either a Personal Access Token (PAT) or Azure AD +credentials. The API version is specified via the api-version query +parameter (e.g., api-version=7.0). + +Endpoints that differ from GitHub: +- Pull Requests: /{project}/_apis/git/pullrequests +- Files: /{project}/_apis/git/repositories/{repo}/items +- Branches: /{project}/_apis/git/repositories/{repo}/refs +- CI: Azure Pipelines REST API (separate endpoint) + +

GitLab

+ +GitLab API is largely RESTful and similar to GitHub's in spirit: +
+https://gitlab.com/api/v4/projects
+
+ +

Bitbucket Cloud

+ +Bitbucket uses the Atlassian REST API: +
+https://api.bitbucket.org/2.0/repositories
+
diff --git a/src/app/actions.rs b/src/app/actions.rs index ae785da..8283164 100644 --- a/src/app/actions.rs +++ b/src/app/actions.rs @@ -1,9 +1,7 @@ use super::{App, Focus, TerminalGuard}; use crate::auth; -use crate::github::GitHubClient; use crate::oauth; use crate::syntax::highlight_content; -use anyhow::Context; use crossterm::event::DisableMouseCapture; use crossterm::execute; use crossterm::terminal::{LeaveAlternateScreen, disable_raw_mode}; @@ -11,7 +9,6 @@ use ratatui::text::Line; use std::io::stdout; use std::path::PathBuf; use std::process::Command; -use std::sync::Arc; impl App { pub(crate) fn search(&mut self) { @@ -59,8 +56,7 @@ impl App { self.status = "Loading...".to_string(); let mut branches = match self.github.fetch_branches(&repo.full_name) { Ok(items) if !items.is_empty() => items, - Ok(_) => vec![repo.default_branch.clone()], - Err(_) => vec![repo.default_branch.clone()], + Ok(_) | Err(_) => vec![repo.default_branch.clone()], }; branches.sort(); branches.dedup(); @@ -74,8 +70,7 @@ impl App { self.account .last_branch_by_repo .get(&repo.full_name) - .map(|saved| saved == branch) - .unwrap_or(false) + .is_some_and(|saved| saved == branch) }) .or_else(|| self.branches.iter().position(|b| b == &repo.default_branch)) .unwrap_or(0); @@ -237,10 +232,14 @@ impl App { } match auth::save_token(&token_trimmed).and_then(|_| { - GitHubClient::new(Some(&token_trimmed)).context("Cannot rebuild HTTP client") + crate::provider::create_provider( + crate::provider::ProviderKind::GitHub, + Some(&token_trimmed), + ) + .map_err(|e| anyhow::anyhow!("{e}")) }) { - Ok(client) => { - self.github = Arc::new(client); + Ok(provider) => { + self.github = provider; self.auth_user = self.github.fetch_authenticated_user().ok().flatten(); self.status = match self.auth_user.as_ref() { Some(login) => format!("Token saved and validated as {login}."), @@ -283,9 +282,12 @@ impl App { match oauth_result { Ok(()) => { if let Ok(token) = auth::load_token() - && let Ok(client) = GitHubClient::new(token.as_deref()) + && let Ok(provider) = crate::provider::create_provider( + crate::provider::ProviderKind::GitHub, + token.as_deref(), + ) { - self.github = Arc::new(client); + self.github = provider; self.auth_user = self.github.fetch_authenticated_user().ok().flatten(); } self.status = "OAuth login completed and session saved.".to_string(); @@ -301,4 +303,43 @@ impl App { Focus::Repos }; } + + pub(crate) fn toggle_tree_view(&mut self) { + if self.current_repo.is_none() { + self.status = "Open a repository first.".to_string(); + return; + } + self.tree_text_mode = !self.tree_text_mode; + self.preview_scroll = 0; + if self.tree_text_mode { + let branch = self.selected_branch_name(); + self.preview_title = format!( + "tree {} [{}]", + self.current_repo + .as_ref() + .map(|r| r.full_name.clone()) + .unwrap_or_default(), + branch + ); + self.preview_lines = self + .tree_all + .iter() + .map(|node| { + let indent = " ".repeat(node.depth.min(20)); + let icon = if node.is_dir { "[D]" } else { "[F]" }; + Line::from(format!("{indent}{icon} {}", node.path)) + }) + .collect(); + self.current_preview_path = None; + self.focus = Focus::Preview; + self.status = "Tree view enabled in preview pane.".to_string(); + } else { + self.preview_title = "Preview".to_string(); + self.preview_lines = vec![Line::from( + "Tree preview disabled. Select a file and press Enter to preview.", + )]; + self.focus = Focus::Tree; + self.status = "Tree view disabled.".to_string(); + } + } } diff --git a/src/app/commands.rs b/src/app/commands.rs index cb14e40..03225fc 100644 --- a/src/app/commands.rs +++ b/src/app/commands.rs @@ -1,7 +1,6 @@ use super::{App, Focus, NetworkEvent, theme}; -use crate::github::GitHubClient; +use crate::provider::GitProvider; use crossterm::event::KeyCode; -use ratatui::text::Line; use secrecy::SecretString; use std::sync::Arc; use std::sync::mpsc; @@ -52,7 +51,7 @@ impl App { &mut self, code: KeyCode, tx: mpsc::Sender, - github: Arc, + github: Arc, ) { if self.keybindings.matches_key("escape", &code) { self.command_palette_visible = false; @@ -138,7 +137,7 @@ impl App { &mut self, cmd: String, tx: mpsc::Sender, - github: Arc, + github: Arc, ) { match cmd.as_str() { "Search Repositories" => { @@ -148,7 +147,8 @@ impl App { "List Starred Repos" => { self.status = "Loading starred repos...".to_string(); let g = github.clone(); - std::thread::spawn(move || { + let tx = tx.clone(); + self.task_manager.spawn(move || { let result = g.fetch_starred_repos(1, 30); let _ = tx.send(NetworkEvent::StarredResult( result.map_err(|e| e.to_string()), @@ -187,42 +187,7 @@ impl App { } } "Toggle Tree View" => { - if self.current_repo.is_some() { - self.tree_text_mode = !self.tree_text_mode; - self.preview_scroll = 0; - if self.tree_text_mode { - let branch = self.selected_branch_name(); - self.preview_title = format!( - "tree {} [{}]", - self.current_repo - .as_ref() - .map(|r| r.full_name.clone()) - .unwrap_or_default(), - branch - ); - self.preview_lines = self - .tree_all - .iter() - .map(|node| { - let indent = " ".repeat(node.depth.min(20)); - let icon = if node.is_dir { "[D]" } else { "[F]" }; - Line::from(format!("{indent}{icon} {}", node.path)) - }) - .collect(); - self.current_preview_path = None; - self.focus = Focus::Preview; - self.status = "Tree view enabled in preview pane.".to_string(); - } else { - self.preview_title = "Preview".to_string(); - self.preview_lines = vec![Line::from( - "Tree preview disabled. Select a file and press Enter to preview.", - )]; - self.focus = Focus::Tree; - self.status = "Tree view disabled.".to_string(); - } - } else { - self.status = "Open a repository first.".to_string(); - } + self.toggle_tree_view(); } "Change Theme" => { let themes = theme::list_available_themes(); @@ -246,7 +211,8 @@ impl App { self.status = "Loading issues...".to_string(); let g = github.clone(); let full_name = repo.full_name.clone(); - std::thread::spawn(move || { + let tx = tx.clone(); + self.task_manager.spawn(move || { let result = g.fetch_issues(&full_name, "open", 30); let _ = tx.send(NetworkEvent::IssuesResult( result.map_err(|e| e.to_string()), @@ -261,7 +227,8 @@ impl App { self.status = "Loading pull requests...".to_string(); let g = github.clone(); let full_name = repo.full_name.clone(); - std::thread::spawn(move || { + let tx = tx.clone(); + self.task_manager.spawn(move || { let result = g.fetch_pull_requests(&full_name, "open", 30); let _ = tx.send(NetworkEvent::PrsResult(result.map_err(|e| e.to_string()))); }); @@ -275,7 +242,8 @@ impl App { let g = github.clone(); let full_name = repo.full_name.clone(); let branch = self.selected_branch_name(); - std::thread::spawn(move || { + let tx = tx.clone(); + self.task_manager.spawn(move || { let result = g.fetch_recent_commits(&full_name, &branch, 30); let _ = tx.send(NetworkEvent::CommitsResult( result.map_err(|e| e.to_string()), @@ -291,7 +259,8 @@ impl App { let g = github.clone(); let full_name = repo.full_name.clone(); let branch = self.selected_branch_name(); - std::thread::spawn(move || { + let tx = tx.clone(); + self.task_manager.spawn(move || { let result = g.fetch_check_runs(&full_name, &branch); let _ = tx.send(NetworkEvent::CheckRunsResult( result.map_err(|e| e.to_string()), @@ -307,7 +276,8 @@ impl App { let g = github.clone(); let full_name = repo.full_name.clone(); let branch = self.selected_branch_name(); - std::thread::spawn(move || { + let tx = tx.clone(); + self.task_manager.spawn(move || { let result = g.fetch_workflow_runs(&full_name, &branch, 30); let _ = tx.send(NetworkEvent::WorkflowRunsResult(result.unwrap_or_default())); @@ -343,7 +313,8 @@ impl App { let full_name = repo.full_name.clone(); let base = base.clone(); let head = head.clone(); - std::thread::spawn(move || { + let tx = tx.clone(); + self.task_manager.spawn(move || { let result = g.fetch_compare(&full_name, &base, &head); let _ = tx.send(NetworkEvent::CompareResult( result.map_err(|e| e.to_string()), @@ -376,7 +347,7 @@ impl App { } else { let count = selected.len(); self.status = format!("Cloning {} repo(s) to {}...", count, dest); - std::thread::spawn(move || { + self.task_manager.spawn(move || { for repo in &selected { let repo_path = std::path::PathBuf::from(&dest).join(&repo.name); if repo_path.exists() { @@ -404,7 +375,7 @@ impl App { &mut self, action: String, tx: mpsc::Sender, - github: Arc, + github: Arc, ) { match action.as_str() { "[Approve]" => { @@ -440,7 +411,8 @@ impl App { let full_name = repo.full_name.clone(); let number = detail.number; let method = merge_method.to_string(); - std::thread::spawn(move || { + let tx = tx.clone(); + self.task_manager.spawn(move || { let result = g.merge_pull_request(&full_name, number, None, Some(&method)); let _ = tx.send(NetworkEvent::PrMergeResult( result.map_err(|e| e.to_string()), @@ -460,7 +432,8 @@ impl App { let g = github.clone(); let full_name = repo.full_name.clone(); let number = detail.number; - std::thread::spawn(move || { + let tx = tx.clone(); + self.task_manager.spawn(move || { match g.update_pull_request(&full_name, number, "closed") { Ok(_) => { let _ = tx.send(NetworkEvent::PrActionResult("PR closed.".to_string())); @@ -485,7 +458,8 @@ impl App { let g = github.clone(); let full_name = repo.full_name.clone(); let number = detail.number; - std::thread::spawn(move || { + let tx = tx.clone(); + self.task_manager.spawn(move || { let result = g.fetch_pull_request_reviews(&full_name, number); let _ = tx.send(NetworkEvent::PrReviewsResult( result.map_err(|e| e.to_string()), @@ -505,7 +479,8 @@ impl App { let g = github.clone(); let full_name = repo.full_name.clone(); let number = detail.number; - std::thread::spawn(move || { + let tx = tx.clone(); + self.task_manager.spawn(move || { let result = g.fetch_pull_request_comments(&full_name, number); let _ = tx.send(NetworkEvent::PrCommentsResult( result.map_err(|e| e.to_string()), @@ -525,7 +500,8 @@ impl App { let g = github.clone(); let full_name = repo.full_name.clone(); let number = detail.number; - std::thread::spawn(move || { + let tx = tx.clone(); + self.task_manager.spawn(move || { let result = g.fetch_pull_request_commits(&full_name, number); let _ = tx.send(NetworkEvent::PrCommitsResult( result.map_err(|e| e.to_string()), @@ -543,7 +519,7 @@ impl App { action: String, text: String, tx: mpsc::Sender, - github: Arc, + github: Arc, ) { match action.as_str() { "create_pr_title" => { @@ -600,7 +576,8 @@ impl App { let g = github.clone(); let full_name = repo.full_name.clone(); let body_clone = body.clone(); - std::thread::spawn(move || { + let tx = tx.clone(); + self.task_manager.spawn(move || { let result = g.create_pull_request( &full_name, &title, diff --git a/src/app/input/fields.rs b/src/app/input/fields.rs index 62791ab..8b515be 100644 --- a/src/app/input/fields.rs +++ b/src/app/input/fields.rs @@ -1,7 +1,7 @@ use crate::app::{App, Focus}; use anyhow::Context; use crossterm::event::KeyCode; -use secrecy::{ExposeSecret, SecretString, zeroize::Zeroize}; +use secrecy::{SecretString, zeroize::Zeroize}; use std::path::PathBuf; fn fuzzy_match(needle: &str, haystack: &str) -> bool { @@ -149,18 +149,14 @@ impl App { self.input_buffer.clear(); self.focus = Focus::Repos; } else if self.keybindings.matches_key("enter", &code) { - let token: String = self.token_buffer.expose_secret().to_string(); + let token = std::mem::take(&mut self.input_buffer); + self.token_buffer = SecretString::new(token.clone().into()); self.save_token_from_input_str(token); self.token_buffer.zeroize(); - self.input_buffer.clear(); } else if self.keybindings.matches_key("backspace", &code) { - let mut s: String = self.token_buffer.expose_secret().to_string(); - s.pop(); - self.token_buffer = SecretString::new(s.into()); + self.input_buffer.pop(); } else if let KeyCode::Char(ch) = code { - let mut s: String = self.token_buffer.expose_secret().to_string(); - s.push(ch); - self.token_buffer = SecretString::new(s.into()); + self.input_buffer.push(ch); } } diff --git a/src/app/input/nav.rs b/src/app/input/nav.rs index 32a9a43..7562e37 100644 --- a/src/app/input/nav.rs +++ b/src/app/input/nav.rs @@ -1,5 +1,5 @@ use crate::app::{App, Focus, NetworkEvent}; -use crate::github::GitHubClient; +use crate::provider::GitProvider; use crossterm::event::KeyCode; use ratatui::text::Line; use std::sync::Arc; @@ -108,40 +108,7 @@ impl App { self.status = "Preview a file first before downloading.".to_string(); } } else if self.keybindings.matches_key("tree_view", &code) { - if self.current_repo.is_some() { - self.tree_text_mode = !self.tree_text_mode; - self.preview_scroll = 0; - if self.tree_text_mode { - let branch = self.selected_branch_name(); - self.preview_title = format!( - "tree {} [{}]", - self.current_repo - .as_ref() - .map(|r| r.full_name.clone()) - .unwrap_or_default(), - branch - ); - self.preview_lines = self - .tree_all - .iter() - .map(|node| { - let indent = " ".repeat(node.depth.min(20)); - let icon = if node.is_dir { "[D]" } else { "[F]" }; - Line::from(format!("{indent}{icon} {}", node.path)) - }) - .collect(); - self.current_preview_path = None; - self.focus = Focus::Preview; - self.status = "Tree view enabled in preview pane.".to_string(); - } else { - self.preview_title = "Preview".to_string(); - self.preview_lines = vec![Line::from( - "Tree preview disabled. Select a file and press Enter to preview.", - )]; - self.focus = Focus::Tree; - self.status = "Tree view disabled.".to_string(); - } - } + self.toggle_tree_view(); } else if self.keybindings.matches_key("focus_next", &code) { self.focus = match self.focus { Focus::Repos if !self.tree_all.is_empty() => Focus::Tree, @@ -225,7 +192,7 @@ impl App { &mut self, code: KeyCode, tx: mpsc::Sender, - github: Arc, + github: Arc, ) { // PR review / creation input mode if self.pr_pending_action.is_some() { @@ -256,7 +223,8 @@ impl App { match action.as_str() { "approve" => { self.status = "Approving PR...".to_string(); - std::thread::spawn(move || { + let tx = tx.clone(); + self.task_manager.spawn(move || { let body = if text.is_empty() { "LGTM, approved." } else { @@ -279,7 +247,8 @@ impl App { } "request_changes" => { self.status = "Requesting changes...".to_string(); - std::thread::spawn(move || { + let tx = tx.clone(); + self.task_manager.spawn(move || { let body = if text.is_empty() { "Please address the requested changes." } else { @@ -306,7 +275,8 @@ impl App { } "comment" => { self.status = "Posting comment...".to_string(); - std::thread::spawn(move || { + let tx = tx.clone(); + self.task_manager.spawn(move || { let body = if text.is_empty() { "Reviewed the changes." } else { @@ -357,7 +327,8 @@ impl App { self.focus = Focus::Tree; let g = github.clone(); let full_name = repo.full_name.clone(); - std::thread::spawn(move || { + let tx = tx.clone(); + self.task_manager.spawn(move || { let result = g.fetch_pull_request_detail(&full_name, number); let _ = tx.send(NetworkEvent::PrDetailResult( result.map_err(|e| e.to_string()), diff --git a/src/app/mod.rs b/src/app/mod.rs index a596d97..b18f98b 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -8,11 +8,12 @@ mod theme; use crate::auth; use crate::cache::PreviewCache; use crate::config::{AccountConfig, KeybindingsConfig, ThemeConfig}; -use crate::github::GitHubClient; use crate::models::{ CheckRun, CommitInfo, CompareResponse, Issue, MergeResponse, PullRequest, PullRequestDetail, PullRequestReview, RepoNode, RepoSummary, ReviewComment, }; +use crate::provider::GitProvider; +use crate::task_manager::TaskManager; use anyhow::{Context, Result, anyhow}; use crossterm::event::{ self, DisableMouseCapture, EnableMouseCapture, Event, KeyEventKind, MouseButton, MouseEventKind, @@ -96,9 +97,10 @@ pub enum Focus { pub struct App { // Services - pub github: Arc, + pub github: Arc, pub account: AccountConfig, pub preview_cache: PreviewCache, + pub task_manager: TaskManager, // Search state pub search_query: String, @@ -170,7 +172,10 @@ impl App { fn new(options: RunOptions) -> Result { let token = auth::load_token()?; - let github = Arc::new(GitHubClient::new(token.as_deref())?); + let github = crate::provider::create_provider( + crate::provider::ProviderKind::GitHub, + token.as_deref(), + )?; let mut account = AccountConfig::load_or_default()?; let theme_config = ThemeConfig::load_or_default(); theme::init_theme(&theme_config); @@ -189,6 +194,7 @@ impl App { github, account: account.clone(), preview_cache, + task_manager: TaskManager::new(), search_query: options.initial_query, search_page: options.initial_page.max(1), per_page: options.per_page.clamp(1, 100), @@ -328,7 +334,7 @@ pub fn run_with_options(options: RunOptions) -> Result<()> { let query = app.search_query.clone(); let page = app.search_page; let per_page = app.per_page; - std::thread::spawn(move || { + app.task_manager.spawn(move || { let result = g.search_repositories_page(&query, page, per_page); let _ = tx.send(NetworkEvent::SearchResult( result.map_err(|e| e.to_string()), @@ -383,6 +389,7 @@ pub fn run_with_options(options: RunOptions) -> Result<()> { disable_raw_mode().ok(); execute!(stdout(), LeaveAlternateScreen, DisableMouseCapture).ok(); terminal.show_cursor().ok(); + app.task_manager.join_all(); let _ = panic::take_hook(); // remove our hook terminal_result } @@ -401,12 +408,17 @@ mod tests { fn test_app() -> App { App { - github: Arc::new(crate::github::GitHubClient::new(None).expect("client")), + github: crate::provider::create_provider( + crate::provider::ProviderKind::GitHub, + None, + ) + .expect("client"), account: crate::config::AccountConfig { preferred_clone_dir: ".".to_string(), last_branch_by_repo: Default::default(), }, preview_cache: crate::cache::PreviewCache::new(120).expect("cache"), + task_manager: TaskManager::new(), search_query: String::new(), search_page: 1, per_page: 30, @@ -607,12 +619,17 @@ mod tests { #[test] fn lazy_tree_progress_advances_limit() { let mut app = App { - github: Arc::new(crate::github::GitHubClient::new(None).expect("client")), + github: crate::provider::create_provider( + crate::provider::ProviderKind::GitHub, + None, + ) + .expect("client"), account: crate::config::AccountConfig { preferred_clone_dir: ".".to_string(), last_branch_by_repo: Default::default(), }, preview_cache: crate::cache::PreviewCache::new(120).expect("cache"), + task_manager: TaskManager::new(), search_query: String::new(), search_page: 1, per_page: 30, diff --git a/src/cache.rs b/src/cache.rs index 312825b..d977ac7 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -84,7 +84,9 @@ impl PreviewCache { ) { let key = cache_key(repo, branch, path); let file = self.dir.join(format!("{key}.cache")); - let _ = fs::write(&file, content); + if let Err(e) = fs::write(&file, content) { + log::warn!("Failed to write cache file {}: {e}", file.display()); + } self.memory .insert(key.clone(), (Instant::now(), content.to_vec())); if let Some(etag_value) = etag { diff --git a/src/cli/git.rs b/src/cli/git.rs index 153903d..4733dfc 100644 --- a/src/cli/git.rs +++ b/src/cli/git.rs @@ -5,7 +5,6 @@ use std::process::Command; use crate::auth; use crate::error::GitHubError; -use crate::github::GitHubClient; use super::helpers; @@ -50,18 +49,25 @@ pub fn clone_repo(repo_spec: &str, dir: Option<&str>) -> Result<()> { repo.clone() } else { let token = auth::load_token()?; - let client = GitHubClient::new(token.as_deref())?; + let client = crate::provider::create_provider( + crate::provider::ProviderKind::GitHub, + token.as_deref(), + )?; let info = client.fetch_repo_by_name(&repo).map_err(|e| { - let msg = match &e { - GitHubError::Api { status, body } - if *status == 404 || body.contains("Not Found") => - { - format!("repository '{repo}' not found on GitHub") + let msg = if let Some(gh_err) = e.downcast_ref::() { + match gh_err { + GitHubError::Api { status, body } + if *status == 404 || body.contains("Not Found") => + { + format!("repository '{repo}' not found on GitHub") + } + GitHubError::Unauthorized => { + "authentication required — run 'gitnapse auth set' or 'gitnapse auth oauth login'".to_string() + } + _ => format!("{gh_err}"), } - GitHubError::Unauthorized => { - "authentication required — run 'gitnapse auth set' or 'gitnapse auth oauth login'".to_string() - } - _ => format!("{e}"), + } else { + format!("{e}") }; anyhow!("{msg}") })?; @@ -613,7 +619,10 @@ pub fn merge(branch: &str) -> Result<()> { pub fn download_file(repo: &str, path: &str, r#ref: Option<&str>, out: &PathBuf) -> Result<()> { let token = auth::load_token()?; - let client = GitHubClient::new(token.as_deref())?; + let client = crate::provider::create_provider( + crate::provider::ProviderKind::GitHub, + token.as_deref(), + )?; let bytes = match r#ref { Some(branch) if !branch.trim().is_empty() => { diff --git a/src/cli/helpers.rs b/src/cli/helpers.rs index 64d5c46..393356c 100644 --- a/src/cli/helpers.rs +++ b/src/cli/helpers.rs @@ -3,7 +3,8 @@ use std::process::Command; use crate::auth; use crate::error::GitHubError; -use crate::github::GitHubClient; +use crate::provider::GitProvider; +use std::sync::Arc; // ── Git helpers ───────────────────────────────────────────────────────── @@ -108,28 +109,33 @@ pub fn resolve_full_name(repo: &str) -> Result { // ── API helpers ───────────────────────────────────────────────────────── -pub fn handle_api_error(full_name: &str, e: &GitHubError) -> String { - match e { - GitHubError::Api { status, body } if *status == 404 || body.contains("Not Found") => { +pub fn handle_api_error(full_name: &str, e: &anyhow::Error) -> String { + let msg = e + .downcast_ref::() + .map(|gh_err| match gh_err { + GitHubError::Api { status, body } if *status == 404 || body.contains("Not Found") => { format!("repository '{full_name}' not found on GitHub") } GitHubError::Unauthorized => { "authentication required — run 'gitnapse auth set' or 'gitnapse auth oauth login'" .to_string() } - GitHubError::RateLimited { - remaining: _, - reset, - } => { - format!("GitHub API rate limit exceeded — resets at timestamp {reset}") - } - _ => format!("{e}"), - } + GitHubError::RateLimited { + remaining: _, + reset, + } => { + format!("GitHub API rate limit exceeded — resets at timestamp {reset}") + } + _ => format!("{gh_err}"), + }) + .unwrap_or_else(|| format!("{e}")); + msg } -pub fn make_client() -> Result { +pub fn make_client() -> Result> { let token = auth::load_token()?; - GitHubClient::new(token.as_deref()).map_err(|e| anyhow!("failed to create HTTP client: {e}")) + crate::provider::create_provider(crate::provider::ProviderKind::GitHub, token.as_deref()) + .map_err(|e| anyhow!("failed to create HTTP client: {e}")) } #[cfg(test)] @@ -252,27 +258,27 @@ mod tests { #[test] fn test_handle_api_error_404() { - let err = GitHubError::Api { + let err = anyhow::Error::from(GitHubError::Api { status: 404, body: "Not Found".to_string(), - }; + }); let msg = handle_api_error("test/repo", &err); assert_eq!(msg, "repository 'test/repo' not found on GitHub"); } #[test] fn test_handle_api_error_unauthorized() { - let err = GitHubError::Unauthorized; + let err: anyhow::Error = anyhow::Error::from(GitHubError::Unauthorized); let msg = handle_api_error("test/repo", &err); assert!(msg.contains("authentication required")); } #[test] fn test_handle_api_error_rate_limited() { - let err = GitHubError::RateLimited { + let err = anyhow::Error::from(GitHubError::RateLimited { remaining: 0, reset: 12345, - }; + }); let msg = handle_api_error("test/repo", &err); assert!(msg.contains("rate limit exceeded")); } diff --git a/src/error.rs b/src/error.rs index f013d28..a039db9 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,5 +1,3 @@ -#![allow(dead_code)] - use std::io; use thiserror::Error; diff --git a/src/github/mod.rs b/src/github/mod.rs index e2ee067..4fb0353 100644 --- a/src/github/mod.rs +++ b/src/github/mod.rs @@ -1,16 +1,16 @@ use crate::error::GitHubError; +use crate::runtime; use reqwest::Client; use reqwest::Response; use reqwest::header::{ACCEPT, AUTHORIZATION, HeaderMap, HeaderValue, USER_AGENT}; use std::sync::Mutex; -use std::sync::OnceLock; use std::time::{Duration, SystemTime, UNIX_EPOCH}; -use tokio::runtime::Runtime; mod ci; mod compare; mod content; mod prs; +mod provider_impl; mod releases; mod repos; @@ -231,14 +231,8 @@ impl GitHubClient { }) } - pub fn get_runtime() -> &'static Runtime { - static RUNTIME: OnceLock = OnceLock::new(); - RUNTIME.get_or_init(|| { - tokio::runtime::Builder::new_current_thread() - .enable_all() - .build() - .expect("Cannot create global tokio runtime for GitHubClient") - }) + pub fn get_runtime() -> &'static tokio::runtime::Runtime { + runtime::get_runtime() } } @@ -349,15 +343,7 @@ mod integration_tests { use serial_test::serial; fn with_api_base(base: &str, test: impl FnOnce() -> T) -> T { - let prev = std::env::var("GITNAPSE_GITHUB_API").ok(); - unsafe { std::env::set_var("GITNAPSE_GITHUB_API", base) }; - let out = test(); - if let Some(value) = prev { - unsafe { std::env::set_var("GITNAPSE_GITHUB_API", value) }; - } else { - unsafe { std::env::remove_var("GITNAPSE_GITHUB_API") }; - } - out + temp_env::with_var("GITNAPSE_GITHUB_API", Some(base), test) } #[test] diff --git a/src/github/provider_impl.rs b/src/github/provider_impl.rs new file mode 100644 index 0000000..85238ab --- /dev/null +++ b/src/github/provider_impl.rs @@ -0,0 +1,251 @@ +use crate::github::GitHubClient; +use crate::provider::GitProvider; +use anyhow::Result; + +impl GitProvider for GitHubClient { + fn fetch_authenticated_user(&self) -> Result> { + self.fetch_authenticated_user().map_err(Into::into) + } + + fn search_repositories_page( + &self, + query: &str, + page: u32, + per_page: u8, + ) -> Result> { + self.search_repositories_page(query, page, per_page) + .map_err(Into::into) + } + + fn fetch_branches(&self, full_name: &str) -> Result> { + self.fetch_branches(full_name).map_err(Into::into) + } + + fn fetch_repo_tree( + &self, + full_name: &str, + branch: &str, + ) -> Result> { + self.fetch_repo_tree(full_name, branch).map_err(Into::into) + } + + fn fetch_starred_repos(&self, page: u32, per_page: u8) -> Result> { + self.fetch_starred_repos(page, per_page).map_err(Into::into) + } + + fn fetch_repo_by_name(&self, full_name: &str) -> Result { + self.fetch_repo_by_name(full_name).map_err(Into::into) + } + + fn fetch_file_content(&self, full_name: &str, path: &str) -> Result> { + self.fetch_file_content(full_name, path) + } + + fn fetch_file_content_by_ref( + &self, + full_name: &str, + path: &str, + git_ref: &str, + ) -> Result> { + self.fetch_file_content_by_ref(full_name, path, git_ref) + } + + fn fetch_issues( + &self, + full_name: &str, + state: &str, + per_page: u8, + ) -> Result> { + self.fetch_issues(full_name, state, per_page) + .map_err(Into::into) + } + + fn create_issue( + &self, + full_name: &str, + title: &str, + body: Option<&str>, + ) -> Result { + self.create_issue(full_name, title, body) + .map_err(Into::into) + } + + fn close_issue(&self, full_name: &str, number: u64) -> Result { + self.close_issue(full_name, number).map_err(Into::into) + } + + fn fetch_pull_requests( + &self, + full_name: &str, + state: &str, + per_page: u8, + ) -> Result> { + self.fetch_pull_requests(full_name, state, per_page) + .map_err(Into::into) + } + + fn fetch_pull_request_detail( + &self, + full_name: &str, + number: u64, + ) -> Result { + self.fetch_pull_request_detail(full_name, number) + .map_err(Into::into) + } + + fn fetch_pull_request_reviews( + &self, + full_name: &str, + number: u64, + ) -> Result> { + self.fetch_pull_request_reviews(full_name, number) + .map_err(Into::into) + } + + fn fetch_pull_request_comments( + &self, + full_name: &str, + number: u64, + ) -> Result> { + self.fetch_pull_request_comments(full_name, number) + .map_err(Into::into) + } + + fn fetch_pull_request_commits( + &self, + full_name: &str, + number: u64, + ) -> Result> { + self.fetch_pull_request_commits(full_name, number) + .map_err(Into::into) + } + + fn merge_pull_request( + &self, + full_name: &str, + number: u64, + commit_title: Option<&str>, + merge_method: Option<&str>, + ) -> Result { + self.merge_pull_request(full_name, number, commit_title, merge_method) + .map_err(Into::into) + } + + fn create_pull_request_review( + &self, + full_name: &str, + number: u64, + body: &str, + event: &str, + ) -> Result<()> { + let _ = self.create_pull_request_review(full_name, number, body, event)?; + Ok(()) + } + + fn update_pull_request( + &self, + full_name: &str, + number: u64, + state: &str, + ) -> Result<()> { + let _ = self.update_pull_request(full_name, number, state)?; + Ok(()) + } + + fn create_pull_request_comment( + &self, + full_name: &str, + number: u64, + body: &str, + ) -> Result<()> { + let _ = self.create_pull_request_comment(full_name, number, body)?; + Ok(()) + } + + fn create_pull_request( + &self, + full_name: &str, + title: &str, + head: &str, + base: &str, + body: Option<&str>, + ) -> Result { + self.create_pull_request(full_name, title, head, base, body) + .map_err(Into::into) + } + + fn fetch_recent_commits( + &self, + full_name: &str, + branch: &str, + per_page: u8, + ) -> Result> { + self.fetch_recent_commits(full_name, branch, per_page) + .map_err(Into::into) + } + + fn fetch_compare( + &self, + full_name: &str, + base: &str, + head: &str, + ) -> Result { + self.fetch_compare(full_name, base, head).map_err(Into::into) + } + + fn fetch_check_runs( + &self, + full_name: &str, + ref_: &str, + ) -> Result> { + self.fetch_check_runs(full_name, ref_).map_err(Into::into) + } + + fn fetch_workflow_runs( + &self, + full_name: &str, + branch: &str, + per_page: u8, + ) -> Result> { + self.fetch_workflow_runs(full_name, branch, per_page) + .map_err(Into::into) + } + + fn fetch_releases( + &self, + full_name: &str, + per_page: u8, + ) -> Result> { + self.fetch_releases(full_name, per_page).map_err(Into::into) + } + + fn create_release( + &self, + full_name: &str, + tag_name: &str, + name: Option<&str>, + body: Option<&str>, + prerelease: bool, + ) -> Result { + self.create_release(full_name, tag_name, name, body, prerelease) + .map_err(Into::into) + } + + fn create_repo( + &self, + name: &str, + description: Option<&str>, + private: bool, + ) -> Result { + self.create_repo(name, description, private) + .map_err(Into::into) + } + + fn rate_limit_remaining(&self) -> Option { + self.rate_limit_remaining() + } + + fn rate_limit_reset(&self) -> Option { + self.rate_limit_reset() + } +} diff --git a/src/lib.rs b/src/lib.rs index f7fd4ab..a0139ff 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -8,5 +8,8 @@ pub mod github; pub mod models; pub mod oauth; pub mod oauth_session; +pub mod provider; +pub mod runtime; pub mod secure_store; pub mod syntax; +pub mod task_manager; diff --git a/src/main.rs b/src/main.rs index 9ab6445..df8191c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -101,98 +101,128 @@ fn main() -> Result<()> { let _ = dotenvy::dotenv(); env_logger::Builder::from_env(env_logger::Env::default().default_filter_or("warn")).init(); let cli = Cli::parse(); + dispatch(cli.command) +} - match cli.command { - Some(Command::Run(args)) => app::run_with_options(args.into()), - Some(Command::DownloadFile(args)) => { +fn dispatch(cmd: Option) -> Result<()> { + use Command::{Run, DownloadFile, Clone, Commit, Push, Pull, Fetch, Checkout, Diff, Stash, Tag, Status, Log, Branch, Reset, Pr, Issue, Ci, Compare, Remote, Config, Merge, Release, Repo, Search, Auth}; + match cmd { + Some(Run(args)) => app::run_with_options(args.into()), + Some(DownloadFile(args)) => { cli::download_file(&args.repo, &args.path, args.r#ref.as_deref(), &args.out) } - Some(Command::Clone(args)) => cli::clone_repo(&args.repo, args.dir.as_deref()), - Some(Command::Commit(args)) => cli::commit(&args.message, args.all), - Some(Command::Push(args)) => { - cli::push(args.remote.as_deref(), args.branch.as_deref(), args.force) - } - Some(Command::Pull(args)) => { - cli::pull(args.remote.as_deref(), args.branch.as_deref(), args.rebase) - } - Some(Command::Fetch(args)) => cli::fetch(args.prune), - Some(Command::Checkout(args)) => cli::checkout(&args.branch, args.create), - Some(Command::Diff(args)) => cli::diff(args.staged, args.path.as_deref()), - Some(Command::Stash { action }) => match action { - cli::StashAction::Push { message } => cli::stash_push(message.as_deref()), - cli::StashAction::Pop => cli::stash_pop(), - cli::StashAction::List => cli::stash_list(), - }, - Some(Command::Tag { action }) => match action { - cli::TagAction::List { pattern } => cli::tag_list(pattern.as_deref()), - cli::TagAction::Create { - name, - message, - target, - } => cli::tag_create(&name, message.as_deref(), target.as_deref()), - cli::TagAction::Delete { name } => cli::tag_delete(&name), - }, - Some(Command::Status) => cli::status(), - Some(Command::Log(args)) => cli::log_lines(args.count), - Some(Command::Branch) => cli::branch(), - Some(Command::Reset(args)) => cli::reset(args.target.as_deref(), args.hard), - Some(Command::Pr { action }) => match action { - cli::PrAction::List(a) => cli::pr_list(&a.repo, &a.state), - cli::PrAction::Create(a) => { - cli::pr_create(&a.repo, &a.title, &a.head, &a.base, a.body.as_deref()) - } - cli::PrAction::Merge(a) => cli::pr_merge(&a.repo, a.number, a.method.as_deref()), - }, - Some(Command::Issue { action }) => match action { - cli::IssueAction::List(a) => cli::issue_list(&a.repo, &a.state), - cli::IssueAction::Create(a) => cli::issue_create(&a.repo, &a.title, a.body.as_deref()), - cli::IssueAction::Close(a) => cli::issue_close(&a.repo, a.number), - }, - Some(Command::Ci(args)) => { - cli::ci_status(&args.repo, args.branch.as_deref(), args.workflows) - } - Some(Command::Compare(args)) => cli::compare(&args.repo, &args.base, &args.head), - Some(Command::Remote { action }) => match action { - cli::RemoteAction::List => cli::remote_list(), - cli::RemoteAction::Add(a) => cli::remote_add(&a.name, &a.url), - cli::RemoteAction::Remove(a) => cli::remote_remove(&a.name), - cli::RemoteAction::Rename(a) => cli::remote_rename(&a.old, &a.new), - }, - Some(Command::Config { action }) => match action { - cli::ConfigAction::Get(a) => cli::config_get(&a.key), - cli::ConfigAction::Set(a) => cli::config_set(&a.key, &a.value), - cli::ConfigAction::List => cli::config_list(), - }, - Some(Command::Merge(args)) => cli::merge(&args.branch), - Some(Command::Release { action }) => match action { - cli::ReleaseAction::List(a) => cli::release_list(&a.repo), - cli::ReleaseAction::Create(a) => cli::release_create( - &a.repo, - &a.tag_name, - a.name.as_deref(), - a.body.as_deref(), - a.prerelease, - ), - }, - Some(Command::Repo { action }) => match action { - cli::RepoAction::Create(a) => { - cli::repo_create(&a.name, a.description.as_deref(), a.private) - } - }, - Some(Command::Search(args)) => cli::search(&args.query), - Some(Command::Auth { action }) => match action { - cli::AuthAction::Set { token } => auth::set_token_cli(token), - cli::AuthAction::Clear => auth::clear_token_cli(), - cli::AuthAction::Status => auth::status_cli(), - cli::AuthAction::Oauth { action } => match action { - cli::OauthAction::Login { - client_id, - scope, - timeout_secs, - } => oauth::oauth_device_login_cli(client_id, scope, timeout_secs), - cli::OauthAction::Status => oauth::oauth_status_cli(), - }, - }, + Some(Clone(args)) => cli::clone_repo(&args.repo, args.dir.as_deref()), + Some(Commit(args)) => cli::commit(&args.message, args.all), + Some(Push(args)) => cli::push(args.remote.as_deref(), args.branch.as_deref(), args.force), + Some(Pull(args)) => cli::pull(args.remote.as_deref(), args.branch.as_deref(), args.rebase), + Some(Fetch(args)) => cli::fetch(args.prune), + Some(Checkout(args)) => cli::checkout(&args.branch, args.create), + Some(Diff(args)) => cli::diff(args.staged, args.path.as_deref()), + Some(Stash { action }) => dispatch_stash(action), + Some(Tag { action }) => dispatch_tag(action), + Some(Status) => cli::status(), + Some(Log(args)) => cli::log_lines(args.count), + Some(Branch) => cli::branch(), + Some(Reset(args)) => cli::reset(args.target.as_deref(), args.hard), + Some(Pr { action }) => dispatch_pr(action), + Some(Issue { action }) => dispatch_issue(action), + Some(Ci(args)) => cli::ci_status(&args.repo, args.branch.as_deref(), args.workflows), + Some(Compare(args)) => cli::compare(&args.repo, &args.base, &args.head), + Some(Remote { action }) => dispatch_remote(action), + Some(Config { action }) => dispatch_config(action), + Some(Merge(args)) => cli::merge(&args.branch), + Some(Release { action }) => dispatch_release(action), + Some(Repo { action }) => dispatch_repo(action), + Some(Search(args)) => cli::search(&args.query), + Some(Auth { action }) => dispatch_auth(action), None => app::run(), } } + +fn dispatch_stash(action: cli::StashAction) -> Result<()> { + use cli::StashAction::{Push, Pop, List}; + match action { + Push { message } => cli::stash_push(message.as_deref()), + Pop => cli::stash_pop(), + List => cli::stash_list(), + } +} + +fn dispatch_tag(action: cli::TagAction) -> Result<()> { + use cli::TagAction::{List, Create, Delete}; + match action { + List { pattern } => cli::tag_list(pattern.as_deref()), + Create { name, message, target } => cli::tag_create(&name, message.as_deref(), target.as_deref()), + Delete { name } => cli::tag_delete(&name), + } +} + +fn dispatch_pr(action: cli::PrAction) -> Result<()> { + use cli::PrAction::{List, Create, Merge}; + match action { + List(a) => cli::pr_list(&a.repo, &a.state), + Create(a) => cli::pr_create(&a.repo, &a.title, &a.head, &a.base, a.body.as_deref()), + Merge(a) => cli::pr_merge(&a.repo, a.number, a.method.as_deref()), + } +} + +fn dispatch_issue(action: cli::IssueAction) -> Result<()> { + use cli::IssueAction::{List, Create, Close}; + match action { + List(a) => cli::issue_list(&a.repo, &a.state), + Create(a) => cli::issue_create(&a.repo, &a.title, a.body.as_deref()), + Close(a) => cli::issue_close(&a.repo, a.number), + } +} + +fn dispatch_remote(action: cli::RemoteAction) -> Result<()> { + use cli::RemoteAction::{List, Add, Remove, Rename}; + match action { + List => cli::remote_list(), + Add(a) => cli::remote_add(&a.name, &a.url), + Remove(a) => cli::remote_remove(&a.name), + Rename(a) => cli::remote_rename(&a.old, &a.new), + } +} + +fn dispatch_config(action: cli::ConfigAction) -> Result<()> { + use cli::ConfigAction::{Get, Set, List}; + match action { + Get(a) => cli::config_get(&a.key), + Set(a) => cli::config_set(&a.key, &a.value), + List => cli::config_list(), + } +} + +fn dispatch_release(action: cli::ReleaseAction) -> Result<()> { + use cli::ReleaseAction::{List, Create}; + match action { + List(a) => cli::release_list(&a.repo), + Create(a) => cli::release_create(&a.repo, &a.tag_name, a.name.as_deref(), a.body.as_deref(), a.prerelease), + } +} + +fn dispatch_repo(action: cli::RepoAction) -> Result<()> { + use cli::RepoAction::Create; + match action { + Create(a) => cli::repo_create(&a.name, a.description.as_deref(), a.private), + } +} + +fn dispatch_auth(action: cli::AuthAction) -> Result<()> { + use cli::AuthAction::{Set, Clear, Status, Oauth}; + match action { + Set { token } => auth::set_token_cli(token), + Clear => auth::clear_token_cli(), + Status => auth::status_cli(), + Oauth { action } => dispatch_oauth(action), + } +} + +fn dispatch_oauth(action: cli::OauthAction) -> Result<()> { + use cli::OauthAction::{Login, Status}; + match action { + Login { client_id, scope, timeout_secs } => oauth::oauth_device_login_cli(client_id, scope, timeout_secs), + Status => oauth::oauth_status_cli(), + } +} diff --git a/src/models/misc.rs b/src/models/misc.rs index 545ef98..96fe6ad 100644 --- a/src/models/misc.rs +++ b/src/models/misc.rs @@ -1,4 +1,3 @@ -#![allow(dead_code)] use serde::Deserialize; /// A commit in a repository diff --git a/src/models/pr.rs b/src/models/pr.rs index c7ea1d1..a7ac855 100644 --- a/src/models/pr.rs +++ b/src/models/pr.rs @@ -1,4 +1,3 @@ -#![allow(dead_code)] use serde::{Deserialize, Serialize}; // ── Issue-related ───────────────────────────────────────────────────── diff --git a/src/models/release.rs b/src/models/release.rs index 9f21a4e..b30d56e 100644 --- a/src/models/release.rs +++ b/src/models/release.rs @@ -1,4 +1,3 @@ -#![allow(dead_code)] use serde::Deserialize; #[derive(Debug, Clone, Deserialize)] diff --git a/src/models/repo.rs b/src/models/repo.rs index 3e3c4da..fc7c67d 100644 --- a/src/models/repo.rs +++ b/src/models/repo.rs @@ -1,4 +1,3 @@ -#![allow(dead_code)] use serde::Deserialize; #[derive(Debug, Clone, Deserialize)] diff --git a/src/oauth.rs b/src/oauth.rs index 38b70e6..1713267 100644 --- a/src/oauth.rs +++ b/src/oauth.rs @@ -1,23 +1,10 @@ use crate::auth; -use crate::github::GitHubClient; use crate::oauth_session; use anyhow::{Context, Result, anyhow}; use reqwest::header::ACCEPT; use secrecy::{ExposeSecret, SecretString}; use std::process::Command; -use std::sync::OnceLock; use std::time::Duration; -use tokio::runtime::Runtime; - -fn get_runtime() -> &'static Runtime { - static RUNTIME: OnceLock = OnceLock::new(); - RUNTIME.get_or_init(|| { - tokio::runtime::Builder::new_current_thread() - .enable_all() - .build() - .expect("Cannot create tokio runtime") - }) -} const ENV_OAUTH_CLIENT_ID: &str = "GITNAPSE_GITHUB_OAUTH_CLIENT_ID"; const ENV_GITHUB_CLIENT_ID: &str = "GITHUB_CLIENT_ID"; @@ -105,7 +92,7 @@ pub fn oauth_device_login_cli( }; let device_credential = SecretString::new(client_id.clone().into()); - let runtime = get_runtime(); + let runtime = crate::runtime::get_runtime(); let (crab, device_codes) = runtime .block_on(async { @@ -164,12 +151,15 @@ pub fn oauth_device_login_cli( oauth_session::save_from_oauth(&oauth, &client_id) .context("Cannot store OAuth session metadata")?; - let login = GitHubClient::new(Some(&access_token)) - .context("Cannot validate OAuth token with API client")? - .fetch_authenticated_user() - .ok() - .flatten() - .unwrap_or_else(|| "unknown user".to_string()); + let login = crate::provider::create_provider( + crate::provider::ProviderKind::GitHub, + Some(&access_token), + ) + .map_err(|e| anyhow!("{e}"))? + .fetch_authenticated_user() + .ok() + .flatten() + .unwrap_or_else(|| "unknown user".to_string()); println!("OAuth login completed. Token saved securely for user: {login}"); Ok(()) @@ -186,7 +176,10 @@ pub fn oauth_status_cli() -> Result<()> { return Ok(()); } - let client = GitHubClient::new(token.as_deref())?; + let client = crate::provider::create_provider( + crate::provider::ProviderKind::GitHub, + token.as_deref(), + )?; let user = client.fetch_authenticated_user()?; let authenticated = user.is_some(); diff --git a/src/oauth_session.rs b/src/oauth_session.rs index 2b0bc08..f137d5f 100644 --- a/src/oauth_session.rs +++ b/src/oauth_session.rs @@ -1,27 +1,16 @@ +use crate::runtime; use crate::secure_store; use anyhow::{Context, Result, anyhow}; use directories::ProjectDirs; use reqwest::Client; use reqwest::header::{ACCEPT, CONTENT_TYPE, HeaderMap, HeaderValue, USER_AGENT}; -use secrecy::ExposeSecret; +use secrecy::{ExposeSecret, zeroize::Zeroize}; use serde::{Deserialize, Serialize}; use std::fs; use std::path::{Path, PathBuf}; -use std::sync::OnceLock; use std::time::{SystemTime, UNIX_EPOCH}; -use tokio::runtime::Runtime; use url::form_urlencoded::Serializer; -fn get_runtime() -> &'static Runtime { - static RUNTIME: OnceLock = OnceLock::new(); - RUNTIME.get_or_init(|| { - tokio::runtime::Builder::new_current_thread() - .enable_all() - .build() - .expect("Cannot create tokio runtime for oauth_session") - }) -} - const SESSION_FILE: &str = "oauth_session.json"; const SESSION_SECRET_KEY: &str = "oauth_session_json"; const ENV_OAUTH_CLIENT_SECRET: &str = "GITNAPSE_GITHUB_OAUTH_CLIENT_SECRET"; @@ -38,6 +27,15 @@ pub struct OAuthSession { pub client_id: String, } +impl Drop for OAuthSession { + fn drop(&mut self) { + self.access_token.zeroize(); + if let Some(ref mut rt) = self.refresh_token { + rt.zeroize(); + } + } +} + #[derive(Debug, Deserialize)] struct RefreshWire { access_token: Option, @@ -124,18 +122,18 @@ pub fn resolve_access_token() -> Result> { .map(|exp| exp <= now.saturating_add(60)) .unwrap_or(false); if !about_to_expire { - return Ok(Some(session.access_token)); + return Ok(Some(session.access_token.clone())); } // If expiring/expired, attempt refresh when possible. if let Some(refreshed) = try_refresh(&session)? { session = refreshed; save_session(&session)?; - return Ok(Some(session.access_token)); + return Ok(Some(session.access_token.clone())); } // No refresh available; caller can fallback to legacy token file. - Ok(Some(session.access_token)) + Ok(Some(session.access_token.clone())) } fn try_refresh(session: &OAuthSession) -> Result> { @@ -168,7 +166,7 @@ fn try_refresh(session: &OAuthSession) -> Result> { let client_id = session.client_id.clone(); let refresh_token = refresh_token.clone(); - get_runtime().block_on(async { + runtime::get_runtime().block_on(async { let mut headers = HeaderMap::new(); headers.insert(USER_AGENT, HeaderValue::from_static("gitnapse/0.1")); headers.insert(ACCEPT, HeaderValue::from_static("application/json")); diff --git a/src/provider.rs b/src/provider.rs new file mode 100644 index 0000000..10db2ff --- /dev/null +++ b/src/provider.rs @@ -0,0 +1,192 @@ +use crate::models::{ + CheckRun, CommitInfo, CompareResponse, Issue, MergeResponse, PullRequest, + PullRequestDetail, PullRequestReview, Release, RepoNode, RepoSummary, ReviewComment, + WorkflowRun, +}; +use anyhow::Result; +use std::sync::Arc; + +/// The kind of git/DevOps provider detected from a remote URL. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ProviderKind { + GitHub, + AzureDevOps, + GitLab, + Bitbucket, + Other, +} + +impl ProviderKind { + pub fn display_name(&self) -> &'static str { + match self { + ProviderKind::GitHub => "GitHub", + ProviderKind::AzureDevOps => "Azure DevOps", + ProviderKind::GitLab => "GitLab", + ProviderKind::Bitbucket => "Bitbucket", + ProviderKind::Other => "Other", + } + } +} + +/// Detect the provider kind from a git remote URL. +pub fn detect_provider(remote_url: &str) -> ProviderKind { + let url = remote_url.to_lowercase(); + if url.contains("github") { + ProviderKind::GitHub + } else if url.contains("dev.azure") || url.contains("visualstudio.com") { + ProviderKind::AzureDevOps + } else if url.contains("gitlab") { + ProviderKind::GitLab + } else if url.contains("bitbucket") { + ProviderKind::Bitbucket + } else { + ProviderKind::Other + } +} + +/// Abstract interface for interacting with a git/DevOps hosting provider. +pub trait GitProvider: Send + Sync { + fn fetch_authenticated_user(&self) -> Result>; + + fn search_repositories_page( + &self, + query: &str, + page: u32, + per_page: u8, + ) -> Result>; + fn fetch_branches(&self, full_name: &str) -> Result>; + fn fetch_repo_tree(&self, full_name: &str, branch: &str) -> Result>; + fn fetch_starred_repos(&self, page: u32, per_page: u8) -> Result>; + fn fetch_repo_by_name(&self, full_name: &str) -> Result; + + fn fetch_file_content(&self, full_name: &str, path: &str) -> Result>; + fn fetch_file_content_by_ref( + &self, + full_name: &str, + path: &str, + git_ref: &str, + ) -> Result>; + + fn fetch_issues(&self, full_name: &str, state: &str, per_page: u8) -> Result>; + fn create_issue(&self, full_name: &str, title: &str, body: Option<&str>) -> Result; + fn close_issue(&self, full_name: &str, number: u64) -> Result; + + fn fetch_pull_requests( + &self, + full_name: &str, + state: &str, + per_page: u8, + ) -> Result>; + fn fetch_pull_request_detail( + &self, + full_name: &str, + number: u64, + ) -> Result; + fn fetch_pull_request_reviews( + &self, + full_name: &str, + number: u64, + ) -> Result>; + fn fetch_pull_request_comments( + &self, + full_name: &str, + number: u64, + ) -> Result>; + fn fetch_pull_request_commits( + &self, + full_name: &str, + number: u64, + ) -> Result>; + fn merge_pull_request( + &self, + full_name: &str, + number: u64, + commit_title: Option<&str>, + merge_method: Option<&str>, + ) -> Result; + fn create_pull_request_review( + &self, + full_name: &str, + number: u64, + body: &str, + event: &str, + ) -> Result<()>; + fn update_pull_request( + &self, + full_name: &str, + number: u64, + state: &str, + ) -> Result<()>; + fn create_pull_request_comment( + &self, + full_name: &str, + number: u64, + body: &str, + ) -> Result<()>; + fn create_pull_request( + &self, + full_name: &str, + title: &str, + head: &str, + base: &str, + body: Option<&str>, + ) -> Result; + + fn fetch_recent_commits( + &self, + full_name: &str, + branch: &str, + per_page: u8, + ) -> Result>; + fn fetch_compare( + &self, + full_name: &str, + base: &str, + head: &str, + ) -> Result; + + fn fetch_check_runs(&self, full_name: &str, ref_: &str) -> Result>; + fn fetch_workflow_runs( + &self, + full_name: &str, + branch: &str, + per_page: u8, + ) -> Result>; + + fn fetch_releases(&self, full_name: &str, per_page: u8) -> Result>; + fn create_release( + &self, + full_name: &str, + tag_name: &str, + name: Option<&str>, + body: Option<&str>, + prerelease: bool, + ) -> Result; + + fn create_repo( + &self, + name: &str, + description: Option<&str>, + private: bool, + ) -> Result; + + fn rate_limit_remaining(&self) -> Option; + fn rate_limit_reset(&self) -> Option; +} + +/// Factory: create a provider instance for the given kind and token. +pub fn create_provider( + kind: ProviderKind, + token: Option<&str>, +) -> Result> { + match kind { + ProviderKind::GitHub | ProviderKind::Other => { + let client = crate::github::GitHubClient::new(token)?; + Ok(Arc::new(client)) + } + _ => { + let client = crate::github::GitHubClient::new(token)?; + Ok(Arc::new(client)) + } + } +} diff --git a/src/runtime.rs b/src/runtime.rs new file mode 100644 index 0000000..6db2e83 --- /dev/null +++ b/src/runtime.rs @@ -0,0 +1,12 @@ +use std::sync::OnceLock; +use tokio::runtime::Runtime; + +pub fn get_runtime() -> &'static Runtime { + static RUNTIME: OnceLock = OnceLock::new(); + RUNTIME.get_or_init(|| { + tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .expect("Cannot create global tokio runtime") + }) +} diff --git a/src/task_manager.rs b/src/task_manager.rs new file mode 100644 index 0000000..1d769f6 --- /dev/null +++ b/src/task_manager.rs @@ -0,0 +1,57 @@ +use std::sync::Mutex; +use std::thread::JoinHandle; + +/// Manages background threads spawned by the TUI event loop. +/// +/// Tracks all active `JoinHandle`s so they can be joined on shutdown, +/// preventing dangling threads from writing to closed channels. +pub struct TaskManager { + handles: Mutex>>, +} + +impl TaskManager { + pub fn new() -> Self { + Self { + handles: Mutex::new(Vec::new()), + } + } + + /// Spawn a new background task and track its handle. + pub fn spawn(&self, f: F) + where + F: FnOnce() + Send + 'static, + { + let handle = std::thread::Builder::new() + .name("gitnapse-worker".into()) + .spawn(f) + .expect("failed to spawn background thread"); + self.handles.lock().unwrap().push(handle); + } + + /// Remove handles for threads that have already completed. + pub fn cleanup(&self) { + let mut handles = self.handles.lock().unwrap(); + handles.retain(|h| !h.is_finished()); + } + + /// Number of currently tracked (running) threads. + pub fn active_count(&self) -> usize { + let handles = self.handles.lock().unwrap(); + handles.len() + } + + /// Join (wait for) all tracked threads to finish. + /// Called during shutdown to ensure clean teardown. + pub fn join_all(&self) { + let mut handles = std::mem::take(&mut *self.handles.lock().unwrap()); + for h in handles.drain(..) { + let _ = h.join(); + } + } +} + +impl Default for TaskManager { + fn default() -> Self { + Self::new() + } +} diff --git a/tests/auth_precedence_tests.rs b/tests/auth_precedence_tests.rs index 35fe0b1..d632892 100644 --- a/tests/auth_precedence_tests.rs +++ b/tests/auth_precedence_tests.rs @@ -4,15 +4,8 @@ use serial_test::serial; #[test] #[serial] fn env_token_has_precedence_over_stored_sources() { - let prev = std::env::var("GITHUB_TOKEN").ok(); - unsafe { std::env::set_var("GITHUB_TOKEN", "env-priority-token") }; - - let loaded = auth::load_token().expect("load token"); - assert_eq!(loaded.as_deref(), Some("env-priority-token")); - - if let Some(value) = prev { - unsafe { std::env::set_var("GITHUB_TOKEN", value) }; - } else { - unsafe { std::env::remove_var("GITHUB_TOKEN") }; - } + temp_env::with_var("GITHUB_TOKEN", Some("env-priority-token"), || { + let loaded = auth::load_token().expect("load token"); + assert_eq!(loaded.as_deref(), Some("env-priority-token")); + }); } From 1c74b3893386ca1f76f7c836298ffc8cf2d5f8c7 Mon Sep 17 00:00:00 2001 From: xscriptor <“preciado.oscar.osorio@gmail.com”> Date: Thu, 18 Jun 2026 20:51:48 +0200 Subject: [PATCH 2/4] update gitnapse --- CHANGELOG.md | 20 +++++++ Cargo.toml | 2 +- src/app/commands.rs | 5 ++ src/app/input/nav.rs | 17 ++++-- src/app/mod.rs | 42 +++++++++++--- src/app/network.rs | 7 ++- src/app/render.rs | 8 ++- src/app/theme.rs | 112 ++++++++++++++++++++++++++++---------- src/config/keybindings.rs | 48 +++++++++++++++- src/config/mod.rs | 21 +++++++ src/github/mod.rs | 7 ++- src/main.rs | 1 + src/oauth.rs | 10 +--- src/runtime.rs | 12 ++++ 14 files changed, 253 insertions(+), 59 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 03d50e4..8c1142d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,26 @@ - **#![allow(dead_code)] removed from 5 files**: Replaced crate-level allows with specific fields or `Drop` implementations where needed. (`src/error.rs`, `src/models/repo.rs`, `src/models/pr.rs`, `src/models/misc.rs`, `src/models/release.rs`) +- **Theme switching broken (OnceLock)**: Palette used `OnceLock` which can only be written once, making `init_theme` a no-op after the first call. Replaced with `RwLock + LazyLock` so themes can be changed at runtime. (`src/app/theme.rs`) + +- **Inline JSONC comments broke theme parsing**: `strip_jsonc_comments` only removed full-line `//` comments but left inline comments (`[0, 0, 0], // color0`), causing `serde_json::from_str` to fail silently. Now strips `//` comments anywhere outside strings. (`src/config/mod.rs`) + +- **Stored invalid token not detected**: When a stored token was rejected by GitHub (401), the app kept sending it on every request. `App::new()` now detects a failed `/user` call with a stored token and clears it, recreating the provider for anonymous access. (`src/app/mod.rs`) + +- **Search returned 401 without guidance**: The search error handler now shows a clear "Press t to set a GitHub token" message instead of a raw "Search failed: GitHub API responded with 401". (`src/app/network.rs`) + +- **CryptoProvider not installed for TUI/CLI**: `ensure_rustls_crypto_provider()` was only called in the OAuth device flow, causing TLS handshake failures when opening the TUI or running CLI commands. Now called from `main()` before any provider is created. (`src/runtime.rs`, `src/main.rs`) + +- **reqwest client had no timeout**: Added `REQUEST_TIMEOUT` of 30 seconds to the HTTP client so network hangs don't freeze the UI indefinitely. (`src/github/mod.rs`) + +- **Command palette Ctrl+P broken on macOS**: The palette check used `KeyCode::Char('\x10')` which doesn't work on macOS terminals (Ctrl+P is reported as `KeyCode::Char('p') + KeyModifiers::CONTROL`). Now passes the full `KeyEvent` through the handler chain and uses `matches_key_with_mods` from the keybinding system. (`src/app/input/nav.rs`, `src/app/mod.rs`) + +- **`command_palette` missing from keybinding config**: Added the field with default `"Ctrl+p"` and a new `matches_key_with_mods` method that parses modifier-prefixed key strings (`Ctrl+`, `Shift+`, `Alt+`). (`src/config/keybindings.rs`) + +- **Themes not available after `cargo install`**: Theme files were loaded from disk relative to the executable, which doesn't work with `cargo install`. All 12 themes are now embedded in the binary via `include_str!`. (`src/app/theme.rs`) + +- **Info screen shown at startup instead of command palette**: Removed the auto-rendered welcome screen. Added `show_info` toggle and a "Show Info" command palette entry. Press any key to dismiss. (`src/app/render.rs`, `src/app/commands.rs`, `src/app/mod.rs`, `src/app/input/nav.rs`) + ## v0.1.1 ### Fixed diff --git a/Cargo.toml b/Cargo.toml index fc4b76d..f7d54ec 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,7 +14,7 @@ crossterm = "0.29" directories = "6.0" dotenvy = "0.15" -keyring = "4.0" +keyring = "=4.0.1" keyring-core = "1.0" octocrab = { version = "0.53", default-features = true } ratatui = "0.30" diff --git a/src/app/commands.rs b/src/app/commands.rs index 03225fc..fa20a53 100644 --- a/src/app/commands.rs +++ b/src/app/commands.rs @@ -41,6 +41,7 @@ impl App { self.multi_selected_repos.len() )); } + commands.push("Show Info".to_string()); commands.push("Change Theme".to_string()); commands.push("Set Token".to_string()); commands.push("Quit".to_string()); @@ -189,6 +190,10 @@ impl App { "Toggle Tree View" => { self.toggle_tree_view(); } + "Show Info" => { + self.show_info = true; + self.status = "GitNapse info. Press Esc to close.".to_string(); + } "Change Theme" => { let themes = theme::list_available_themes(); if themes.is_empty() { diff --git a/src/app/input/nav.rs b/src/app/input/nav.rs index 7562e37..67863ca 100644 --- a/src/app/input/nav.rs +++ b/src/app/input/nav.rs @@ -1,6 +1,6 @@ use crate::app::{App, Focus, NetworkEvent}; use crate::provider::GitProvider; -use crossterm::event::KeyCode; +use crossterm::event::{KeyCode, KeyEvent}; use ratatui::text::Line; use std::sync::Arc; use std::sync::mpsc; @@ -190,10 +190,17 @@ impl App { pub(crate) fn handle_key_with_channel( &mut self, - code: KeyCode, + event: KeyEvent, tx: mpsc::Sender, github: Arc, ) { + let code = event.code; + + // Info overlay – any key closes it. + if self.show_info { + self.show_info = false; + return; + } // PR review / creation input mode if self.pr_pending_action.is_some() { if self.keybindings.matches_key("escape", &code) { @@ -351,9 +358,9 @@ impl App { return; } - // Ctrl+P = \x10 - if let KeyCode::Char(ch) = code - && ch == '\x10' + if self + .keybindings + .matches_key_with_mods("command_palette", &code, &event.modifiers) { self.toggle_command_palette(); return; diff --git a/src/app/mod.rs b/src/app/mod.rs index b18f98b..0031579 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -149,6 +149,7 @@ pub struct App { // Command palette pub command_palette_visible: bool, + pub show_info: bool, pub command_input: String, pub command_cursor: usize, pub command_items: Vec, @@ -172,7 +173,7 @@ impl App { fn new(options: RunOptions) -> Result { let token = auth::load_token()?; - let github = crate::provider::create_provider( + let mut github = crate::provider::create_provider( crate::provider::ProviderKind::GitHub, token.as_deref(), )?; @@ -181,7 +182,21 @@ impl App { theme::init_theme(&theme_config); let keybindings = KeybindingsConfig::load_or_default(); let preview_cache = PreviewCache::new(options.cache_ttl_secs)?; - let auth_user = github.fetch_authenticated_user().ok().flatten(); + + // Validate any stored token at startup. If the /user endpoint returns 401, + // the token is stale – clear it so subsequent requests don't carry a bad header. + let auth_user = match github.fetch_authenticated_user() { + Ok(user) => user, + Err(_) => None, + }; + if auth_user.is_none() && token.is_some() { + log::warn!("stored token rejected by GitHub, clearing it"); + let _ = auth::clear_token(); + github = crate::provider::create_provider( + crate::provider::ProviderKind::GitHub, + None, + )?; + } if account.preferred_clone_dir.trim().is_empty() { account.preferred_clone_dir = std::env::current_dir() @@ -190,6 +205,17 @@ impl App { .to_string(); } + let status = match auth_user.as_ref() { + Some(login) => format!("Authenticated as {login}. Press / to search."), + None if token.is_some() => { + "Stored token is invalid. Press t to set a new one or continue anonymously." + .to_string() + } + None => { + "No token set. Press t to save one or continue anonymously.".to_string() + } + }; + Ok(Self { github, account: account.clone(), @@ -231,12 +257,7 @@ impl App { clone_path_input: account.preferred_clone_dir, download_path_input: String::new(), tree_search_input: String::new(), - status: match auth_user.as_ref() { - Some(login) => format!("Authenticated as {login}. Press / to search."), - None => { - "No validated token. Press t to save one or continue anonymously.".to_string() - } - }, + status, focus: Focus::Repos, should_quit: false, auth_user, @@ -245,6 +266,7 @@ impl App { last_repo_click: None, keybindings, command_palette_visible: false, + show_info: false, command_input: String::new(), command_cursor: 0, command_items: Vec::new(), @@ -358,7 +380,7 @@ pub fn run_with_options(options: RunOptions) -> Result<()> { Event::Key(key) if key.kind == KeyEventKind::Press => { let tx = net_tx.clone(); let g = github.clone(); - app.handle_key_with_channel(key.code, tx, g); + app.handle_key_with_channel(key, tx, g); } Event::Mouse(mouse) if mouse.kind == MouseEventKind::Down(MouseButton::Left) => { let area = terminal @@ -451,6 +473,7 @@ mod tests { last_repo_click: None, keybindings: crate::config::KeybindingsConfig::default(), command_palette_visible: false, + show_info: false, command_input: String::new(), command_cursor: 0, command_items: Vec::new(), @@ -669,6 +692,7 @@ mod tests { last_repo_click: None, keybindings: crate::config::KeybindingsConfig::default(), command_palette_visible: false, + show_info: false, command_input: String::new(), command_cursor: 0, command_items: Vec::new(), diff --git a/src/app/network.rs b/src/app/network.rs index df655ca..d6824ba 100644 --- a/src/app/network.rs +++ b/src/app/network.rs @@ -27,7 +27,12 @@ impl App { ); } NetworkEvent::SearchResult(Err(e)) => { - self.status = format!("Search failed: {e}"); + if e.contains("401") { + self.status = + "Authentication required. Press t to set a GitHub token.".to_string(); + } else { + self.status = format!("Search failed: {e}"); + } } NetworkEvent::IssuesResult(Ok(issues)) => { self.command_items = issues diff --git a/src/app/render.rs b/src/app/render.rs index 6b8d111..ec9860e 100644 --- a/src/app/render.rs +++ b/src/app/render.rs @@ -237,7 +237,7 @@ pub fn render(frame: &mut Frame<'_>, app: &mut App) { } fn render_repo_list(frame: &mut Frame<'_>, app: &App, area: Rect) { - if app.repos.is_empty() { + if app.show_info { let version = env!("CARGO_PKG_VERSION"); let info = vec![ Line::from(Span::raw("")), @@ -253,7 +253,7 @@ fn render_repo_list(frame: &mut Frame<'_>, app: &App, area: Rect) { Line::from(Span::raw("")), Line::from(Span::raw(" Press / to search repositories")), ]; - let block = Block::default().title("Welcome").borders(Borders::ALL); + let block = Block::default().title("Info").borders(Borders::ALL); let paragraph = Paragraph::new(info) .block(block) .alignment(Alignment::Left) @@ -262,6 +262,10 @@ fn render_repo_list(frame: &mut Frame<'_>, app: &App, area: Rect) { return; } + if app.repos.is_empty() { + return; + } + let viewport_rows = usize::from(area.height.saturating_sub(2)).max(1); let max_start = app.repos.len().saturating_sub(viewport_rows); let start = app diff --git a/src/app/theme.rs b/src/app/theme.rs index eb7740b..1dc43d0 100644 --- a/src/app/theme.rs +++ b/src/app/theme.rs @@ -1,7 +1,24 @@ use crate::config::{KeybindingsConfig, ThemeConfig, config_dir, strip_jsonc_comments}; use ratatui::style::{Color, Modifier, Style}; use ratatui::text::{Line, Span}; -use std::sync::OnceLock; +use std::sync::{LazyLock, RwLock}; + +/// Embedded theme data: (name, JSONC content). +/// Bundled at compile time so themes are available after `cargo install`. +const EMBEDDED_THEMES: &[(&str, &str)] = &[ + ("Berlin", include_str!("../../themes/Berlin.jsonc")), + ("Bogota", include_str!("../../themes/Bogota.jsonc")), + ("Helsinki", include_str!("../../themes/Helsinki.jsonc")), + ("Lahabana", include_str!("../../themes/Lahabana.jsonc")), + ("London", include_str!("../../themes/London.jsonc")), + ("Madrid", include_str!("../../themes/Madrid.jsonc")), + ("Miami", include_str!("../../themes/Miami.jsonc")), + ("Oslo", include_str!("../../themes/Oslo.jsonc")), + ("Paris", include_str!("../../themes/Paris.jsonc")), + ("Praha", include_str!("../../themes/Praha.jsonc")), + ("Tokio", include_str!("../../themes/Tokio.jsonc")), + ("X", include_str!("../../themes/X.jsonc")), +]; const DEFAULT_PALETTE: [[u8; 3]; 16] = [ [0x36, 0x35, 0x37], @@ -22,24 +39,35 @@ const DEFAULT_PALETTE: [[u8; 3]; 16] = [ [0xf7, 0xf1, 0xff], ]; -static PALETTE: OnceLock> = OnceLock::new(); +static PALETTE: LazyLock>> = + LazyLock::new(|| RwLock::new(DEFAULT_PALETTE.to_vec())); pub fn init_theme(config: &ThemeConfig) { - let _ = PALETTE.set(config.palette.clone()); + if let Ok(mut p) = PALETTE.write() { + *p = config.palette.clone(); + } } pub fn load_theme_by_name(name: &str) -> ThemeConfig { - let dir = match crate::config::config_dir() { - Ok(d) => d.join("themes"), - Err(_) => return ThemeConfig::default(), - }; - let path = dir.join(format!("{name}.jsonc")); - if path.exists() - && let Ok(raw) = std::fs::read_to_string(&path) - { - let cleaned = strip_jsonc_comments(&raw); - if let Ok(cfg) = serde_json::from_str(&cleaned) { - return cfg; + // Try external themes directory first (allows user overrides). + if let Ok(dir) = config_dir() { + let path = dir.join("themes").join(format!("{name}.jsonc")); + if path.exists() + && let Ok(raw) = std::fs::read_to_string(&path) + { + let cleaned = strip_jsonc_comments(&raw); + if let Ok(cfg) = serde_json::from_str(&cleaned) { + return cfg; + } + } + } + // Fall back to embedded themes. + for (embedded_name, content) in EMBEDDED_THEMES { + if *embedded_name == name { + let cleaned = strip_jsonc_comments(content); + if let Ok(cfg) = serde_json::from_str(&cleaned) { + return cfg; + } } } ThemeConfig::default() @@ -64,29 +92,29 @@ fn collect_themes_from(dir: &std::path::Path, names: &mut Vec) { pub fn list_available_themes() -> Vec { let mut names = Vec::new(); + // External themes take precedence (user overrides). if let Ok(dir) = config_dir() { collect_themes_from(&dir.join("themes"), &mut names); } - if let Ok(exe_dir) = std::env::current_exe() - && let Some(parent) = exe_dir.parent() - { - collect_themes_from(&parent.join("../themes"), &mut names); - } - - if let Ok(manifest_dir) = std::env::var("CARGO_MANIFEST_DIR") { - collect_themes_from( - &std::path::PathBuf::from(manifest_dir).join("themes"), - &mut names, - ); + // Always include embedded themes. + for (name, _) in EMBEDDED_THEMES { + let s = name.to_string(); + if !names.contains(&s) { + names.push(s); + } } names.sort(); names } -fn palette() -> &'static Vec<[u8; 3]> { - PALETTE.get_or_init(|| DEFAULT_PALETTE.to_vec()) +fn palette() -> Vec<[u8; 3]> { + PALETTE + .read() + .ok() + .map(|g| g.clone()) + .unwrap_or(DEFAULT_PALETTE.to_vec()) } #[cfg(test)] @@ -175,7 +203,7 @@ pub fn nav_hint_lines(kb: &KeybindingsConfig, max_width: usize) -> Vec 0, "Berlin embed content is empty"); + eprintln!("Berlin embed content: {content}"); + } + // Verify load_theme_by_name returns Berlin + let berlin = load_theme_by_name("Berlin"); + assert_eq!(berlin.palette.len(), 16, "Berlin palette should have 16 entries"); + assert_eq!(berlin.palette[0], [0, 0, 0], "first entry should be black"); + // Apply Berlin theme + init_theme(&berlin); + let berlin_rgb = palette_rgb(1); + assert_eq!( + berlin_rgb, + (153, 153, 153), + "expected Berlin gray, got {berlin_rgb:?}" + ); + // Reset to default + init_theme(&ThemeConfig::default()); + let reset_rgb = palette_rgb(1); + assert_eq!(reset_rgb, default_rgb, "should restore default"); + } } diff --git a/src/config/keybindings.rs b/src/config/keybindings.rs index 467a4f4..523310c 100644 --- a/src/config/keybindings.rs +++ b/src/config/keybindings.rs @@ -1,8 +1,34 @@ -use crossterm::event::KeyCode; +use crossterm::event::{KeyCode, KeyModifiers}; use serde::{Deserialize, Serialize}; use crate::config::{config_dir, strip_jsonc_comments}; +/// Parsed keybinding: keycode plus optional modifiers. +#[derive(Debug, Clone)] +pub(crate) struct BoundKey { + pub(crate) code: KeyCode, + pub(crate) modifiers: KeyModifiers, +} + +/// Parse a keybinding string that may include a modifier prefix. +/// Supported: `"Ctrl+p"`, `"Shift+Tab"`, `"q"`, `"F1"`, etc. +fn parse_keybinding(s: &str) -> Option { + let s = s.trim(); + if let Some(rest) = s.strip_prefix("Ctrl+").or_else(|| s.strip_prefix("ctrl+")) { + let code = str_to_keycode(rest)?; + Some(BoundKey { code, modifiers: KeyModifiers::CONTROL }) + } else if let Some(rest) = s.strip_prefix("Shift+").or_else(|| s.strip_prefix("shift+")) { + let code = str_to_keycode(rest)?; + Some(BoundKey { code, modifiers: KeyModifiers::SHIFT }) + } else if let Some(rest) = s.strip_prefix("Alt+").or_else(|| s.strip_prefix("alt+")) { + let code = str_to_keycode(rest)?; + Some(BoundKey { code, modifiers: KeyModifiers::ALT }) + } else { + let code = str_to_keycode(s)?; + Some(BoundKey { code, modifiers: KeyModifiers::NONE }) + } +} + #[derive(Debug, Clone, Serialize, Deserialize)] pub struct KeybindingsConfig { pub quit: String, @@ -28,6 +54,7 @@ pub struct KeybindingsConfig { pub escape: String, pub backspace: String, pub delete: String, + pub command_palette: String, } impl Default for KeybindingsConfig { @@ -56,6 +83,7 @@ impl Default for KeybindingsConfig { escape: "Esc".into(), backspace: "Backspace".into(), delete: "Delete".into(), + command_palette: "Ctrl+p".into(), } } } @@ -106,14 +134,28 @@ impl KeybindingsConfig { "escape" => vec![self.escape.clone()], "backspace" => vec![self.backspace.clone()], "delete" => vec![self.delete.clone()], + "command_palette" => vec![self.command_palette.clone()], _ => vec![], } } + /// Match an action against a raw `KeyCode` (no modifiers). pub fn matches_key(&self, action: &str, code: &KeyCode) -> bool { + self.matches_key_with_mods(action, code, &KeyModifiers::NONE) + } + + /// Match an action against a key code plus modifier flags. + /// Parses configured key strings (e.g. `"Ctrl+p"`) and compares both + /// the key code and the modifier state. + pub fn matches_key_with_mods( + &self, + action: &str, + code: &KeyCode, + modifiers: &KeyModifiers, + ) -> bool { self.action_keys(action).iter().any(|key_str| { - if let Some(kc) = str_to_keycode(key_str) { - &kc == code + if let Some(bound) = parse_keybinding(key_str) { + &bound.code == code && &bound.modifiers == modifiers } else { false } diff --git a/src/config/mod.rs b/src/config/mod.rs index 80b17e3..ecd7f31 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -32,6 +32,27 @@ pub fn config_file() -> Result { pub(crate) fn strip_jsonc_comments(input: &str) -> String { input .lines() + .map(|line| { + // Remove inline // comments (but not those inside strings) + let mut in_string = false; + let mut prev = ' '; + let mut result = String::with_capacity(line.len()); + for ch in line.chars() { + if ch == '"' && prev != '\\' { + in_string = !in_string; + } + if !in_string && ch == '/' && prev == '/' { + // Remove the // and everything after, plus trailing space + result.pop(); + break; + } + if !in_string || ch != '/' || prev != '/' { + prev = ch; + result.push(ch); + } + } + result.trim_end().to_string() + }) .filter(|line| !line.trim_start().starts_with("//")) .collect::>() .join("\n") diff --git a/src/github/mod.rs b/src/github/mod.rs index 4fb0353..2cc4e45 100644 --- a/src/github/mod.rs +++ b/src/github/mod.rs @@ -6,6 +6,8 @@ use reqwest::header::{ACCEPT, AUTHORIZATION, HeaderMap, HeaderValue, USER_AGENT} use std::sync::Mutex; use std::time::{Duration, SystemTime, UNIX_EPOCH}; +const REQUEST_TIMEOUT: Duration = Duration::from_secs(30); + mod ci; mod compare; mod content; @@ -223,7 +225,10 @@ impl GitHubClient { headers.insert(AUTHORIZATION, value); } - let client = Client::builder().default_headers(headers).build()?; + let client = Client::builder() + .default_headers(headers) + .timeout(REQUEST_TIMEOUT) + .build()?; Ok(Self { client, rate_limit_remaining: Mutex::new(None), diff --git a/src/main.rs b/src/main.rs index df8191c..23334b0 100644 --- a/src/main.rs +++ b/src/main.rs @@ -100,6 +100,7 @@ enum Command { fn main() -> Result<()> { let _ = dotenvy::dotenv(); env_logger::Builder::from_env(env_logger::Env::default().default_filter_or("warn")).init(); + gitnapse::runtime::ensure_crypto_provider(); let cli = Cli::parse(); dispatch(cli.command) } diff --git a/src/oauth.rs b/src/oauth.rs index 1713267..29eb761 100644 --- a/src/oauth.rs +++ b/src/oauth.rs @@ -39,14 +39,6 @@ fn terminal_hyperlink(url: &str) -> String { format!("\x1b]8;;{url}\x1b\\{url}\x1b]8;;\x1b\\") } -fn ensure_rustls_crypto_provider() { - if rustls::crypto::CryptoProvider::install_default(rustls::crypto::ring::default_provider()) - .is_err() - { - log::warn!("could not install rustls crypto provider (may already be set)"); - } -} - fn try_open_browser(url: &str) -> bool { if webbrowser::open(url).is_ok() { return true; @@ -79,7 +71,7 @@ pub fn oauth_device_login_cli( scopes: Vec, timeout_secs: u64, ) -> Result<()> { - ensure_rustls_crypto_provider(); + crate::runtime::ensure_crypto_provider(); let client_id = resolve_client_id(client_id)?; let scopes = if scopes.is_empty() { vec!["read:user".to_string()] diff --git a/src/runtime.rs b/src/runtime.rs index 6db2e83..34827bc 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -10,3 +10,15 @@ pub fn get_runtime() -> &'static Runtime { .expect("Cannot create global tokio runtime") }) } + +/// Install the rustls crypto provider (required before any TLS connection). +/// Safe to call multiple times – subsequent calls are no-ops. +pub fn ensure_crypto_provider() { + if rustls::crypto::CryptoProvider::install_default( + rustls::crypto::ring::default_provider(), + ) + .is_err() + { + log::warn!("could not install rustls crypto provider (may already be set)"); + } +} From 79524db9085ffd934723effacba54d7f1be8507a Mon Sep 17 00:00:00 2001 From: xscriptor Date: Wed, 24 Jun 2026 19:59:39 +0200 Subject: [PATCH 3/4] update works add generic for the case --- .github/workflows/release.yml | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index c19391f..64dbd17 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -124,6 +124,28 @@ jobs: if-no-files-found: error retention-days: 7 + # ── Linux → generic .tar.gz ─────────────────────────────────────────────── + build-linux-generic: + name: Build Linux (generic) + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v5 + - name: Build + run: cargo build --release --locked + - name: Package (.tar.gz) + run: | + asset="gitnapse-${RELEASE_TAG}-linux-x86_64.tar.gz" + tar -czf "${asset}" -C target/release gitnapse + echo "ASSET=${asset}" >> "$GITHUB_ENV" + - name: Upload artifact + uses: actions/upload-artifact@v5 + with: + name: asset-linux-generic + path: ${{ env.ASSET }} + if-no-files-found: error + retention-days: 7 + # ── Windows → .exe ──────────────────────────────────────────────────────── build-windows: name: Build Windows @@ -186,6 +208,7 @@ jobs: - build-linux-ubuntu - build-linux-fedora - build-linux-arch + - build-linux-generic - build-windows - build-macos permissions: From 5e5109042f2dae8b72014672f6c46db388f87ac5 Mon Sep 17 00:00:00 2001 From: xscriptor Date: Wed, 24 Jun 2026 20:02:41 +0200 Subject: [PATCH 4/4] update changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c1142d..6f024cc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,8 @@ - **OAuthSession zeroize on drop**: `OAuthSession` now implements `Drop` to zeroize `access_token` and `refresh_token` fields when the session is dropped. (`src/oauth_session.rs`) +- CI - generic linux portable package + ### Changed - **Unified tokio runtime**: Three separate `OnceLock` instances (GitHubClient, oauth, oauth_session) consolidated into a single shared runtime at `src/runtime.rs`. (`src/runtime.rs`, `src/github/mod.rs`, `src/oauth.rs`, `src/oauth_session.rs`)