diff --git a/.github/workflows/opencode-review.yml b/.github/workflows/opencode-review.yml index 6fabd62..43bfa41 100644 --- a/.github/workflows/opencode-review.yml +++ b/.github/workflows/opencode-review.yml @@ -10,7 +10,7 @@ jobs: runs-on: ubuntu-latest permissions: id-token: write - contents: read + contents: write pull-requests: write issues: write @@ -26,15 +26,17 @@ jobs: env: OPENCODE_API_KEY: ${{ secrets.ZEN_API_KEY }} OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} - #GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} with: model: ${{ vars.OPENCODE_REVIEW_MODEL }} - use_github_token: false + use_github_token: true prompt: | + Before anything else, read and follow AGENTS.md for this repository and module. Review this pull request: - Check for code quality issues - Look for potential bugs - Suggest improvements + - If you make any code edits, run 'cd Client && just test' to verify they compile and pass tests before committing. - name: fallback if: ${{ steps.review_primary.outcome == 'failure' }} @@ -42,12 +44,14 @@ jobs: env: OPENCODE_API_KEY: ${{ secrets.ZEN_API_KEY }} OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} - #GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} with: model: ${{ vars.OPENCODE_REVIEW_MODEL_FALLBACK }} - use_github_token: false + use_github_token: true prompt: | + Before anything else, read and follow AGENTS.md for this repository and module. Review this pull request: - Check for code quality issues - Look for potential bugs - Suggest improvements + - If you make any code edits, run 'cd Client && just test' to verify they compile and pass tests before committing. diff --git a/AGENTS.md b/AGENTS.md index 6b6f3e2..288fedf 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -5,6 +5,7 @@ - `Backend/`: Rust + Tauri backend (`src/`), commands (`src/tauri_commands/`), plugin runtime (`src/plugin_runtime/`), and config-driven plugin sync support (`scripts/`). - `openvcs.plugins.json`: built-in plugin source list used to materialize shipped plugins during client builds. - `Frontend/`: TypeScript + Vite UI code (`src/scripts/`, `src/styles/`, `src/modals/`), with Vitest tests colocated as `*.test.ts` files. +- OpenVCS is desktop-only: there is no web app, no standalone browser mode, and no supported web browser/WebView deployment target. - `docs/`: UX docs, plugin architecture notes, and plugin/theme packaging guides referenced by contributors. - `packaging/flatpak/`: Flatpak manifests and Flatpak-specific build notes. - Supporting files at the repo root include the workspace `Cargo.toml`, `Justfile`, `README.md`, `ARCHITECTURE.md`, `SECURITY.md`, and installer scripts. @@ -47,7 +48,7 @@ ### Development servers - `cargo tauri dev`: run the desktop app in dev mode (`Backend/` directory). -- `npm --prefix Frontend run dev`: run the frontend-only Vite dev server. +- `npm --prefix Frontend run dev`: run the frontend-only Vite dev server for desktop UI development only; it is not a web app/browser deployment. ## Plugin runtime & host expectations diff --git a/Backend/src/plugin_runtime/node_instance.rs b/Backend/src/plugin_runtime/node_instance.rs index 60aa815..d2e9bd7 100644 --- a/Backend/src/plugin_runtime/node_instance.rs +++ b/Backend/src/plugin_runtime/node_instance.rs @@ -27,7 +27,7 @@ use serde::Deserialize; use serde_json::{json, Value}; use std::io::BufReader; use std::process::{Child, ChildStdin, Command, Stdio}; -use std::sync::mpsc::{channel, Receiver}; +use std::sync::mpsc::{channel, Receiver, RecvTimeoutError}; use std::sync::Arc; use std::thread; use std::time::Duration; @@ -45,6 +45,8 @@ struct NodeRpcProcess { rx: Receiver, /// Flag to signal the reader thread to stop. shutdown_flag: Arc>, + /// Last reader-thread error observed while consuming framed messages. + reader_error: Arc>>, /// Monotonic request id counter. next_request_id: u64, } @@ -62,7 +64,7 @@ impl NodeRpcProcess { /// /// # Returns /// - `Ok(T)` decoded response result. - /// - `Err(String)` when transport/protocol/plugin errors or timeout occur. + /// - `Err(String)` when transport/protocol/plugin errors, disconnects, or timeout occur. fn call( &mut self, method: &str, @@ -94,14 +96,41 @@ impl NodeRpcProcess { let start = std::time::Instant::now(); let mut remaining = timeout; loop { - let message = self.rx.recv_timeout(remaining).map_err(|_| { - format!( - "plugin '{}' rpc '{}' timed out after {}s", - plugin_id, - method, - timeout.as_secs() - ) - })?; + let message = match self.rx.recv_timeout(remaining) { + Ok(message) => message, + Err(RecvTimeoutError::Timeout) => { + return Err(format!( + "plugin '{}' rpc '{}' timed out after {}s", + plugin_id, + method, + timeout.as_secs() + )); + } + Err(RecvTimeoutError::Disconnected) => { + let exit_status = self.child.try_wait().map_err(|e| { + format!("check plugin process status for '{plugin_id}': {e}") + })?; + let reader_error = self.reader_error.lock().clone(); + return match (exit_status, reader_error) { + (Some(status), Some(error)) => Err(format!( + "plugin '{}' rpc '{}' disconnected because the process exited with {} (reader error: {})", + plugin_id, method, status, error + )), + (Some(status), None) => Err(format!( + "plugin '{}' rpc '{}' disconnected because the process exited with {}", + plugin_id, method, status + )), + (None, Some(error)) => Err(format!( + "plugin '{}' rpc '{}' disconnected while waiting for a response (reader error: {})", + plugin_id, method, error + )), + (None, None) => Err(format!( + "plugin '{}' rpc '{}' disconnected while waiting for a response", + plugin_id, method + )), + }; + } + }; if let Some(method_name) = message.get("method").and_then(Value::as_str) { let params = message.get("params").cloned().unwrap_or(Value::Null); @@ -253,8 +282,10 @@ impl NodePluginRuntimeInstance { let (tx, rx) = channel::(); let shutdown_flag = Arc::new(Mutex::new(false)); + let reader_error = Arc::new(Mutex::new(None)); let stdout_for_thread = BufReader::new(stdout); let shutdown_for_thread = Arc::clone(&shutdown_flag); + let reader_error_for_thread = Arc::clone(&reader_error); thread::spawn(move || { let mut stdout = stdout_for_thread; @@ -269,6 +300,7 @@ impl NodePluginRuntimeInstance { } } Err(e) => { + *reader_error_for_thread.lock() = Some(e.to_string()); debug!("node rpc reader thread: read error: {}", e); break; } @@ -281,6 +313,7 @@ impl NodePluginRuntimeInstance { stdin, rx, shutdown_flag, + reader_error, next_request_id: 1, }; @@ -336,7 +369,13 @@ impl NodePluginRuntimeInstance { let process = lock .as_mut() .ok_or_else(|| "node runtime did not initialize".to_string())?; - f(process) + let result = f(process); + if let Err(err) = &result { + if err.contains("disconnected") { + lock.take(); + } + } + result } /// Sends one RPC request to the plugin process. diff --git a/Backend/src/plugins.rs b/Backend/src/plugins.rs index 5136f70..ec4ea74 100644 --- a/Backend/src/plugins.rs +++ b/Backend/src/plugins.rs @@ -196,12 +196,21 @@ impl PluginCache { fn list(&self) -> Vec { self.ensure_fresh(); - self.data.read().unwrap().list.clone() + self.data + .read() + .unwrap_or_else(|poisoned| poisoned.into_inner()) + .list + .clone() } fn load_cached_plugin(&self, id: &str) -> Option { self.ensure_fresh(); - self.data.read().unwrap().entries.get(id).cloned() + self.data + .read() + .unwrap_or_else(|poisoned| poisoned.into_inner()) + .entries + .get(id) + .cloned() } fn mark_dirty(&self) { @@ -210,7 +219,10 @@ impl PluginCache { fn ensure_fresh(&self) { let needs_reload = self.dirty.swap(false, Ordering::SeqCst) || { - let data = self.data.read().unwrap(); + let data = self + .data + .read() + .unwrap_or_else(|poisoned| poisoned.into_inner()); !data.loaded }; if needs_reload { @@ -282,7 +294,10 @@ impl PluginCache { } summaries.sort_by_key(|a| a.name.to_lowercase()); - let mut data = self.data.write().unwrap(); + let mut data = self + .data + .write() + .unwrap_or_else(|poisoned| poisoned.into_inner()); data.list = summaries; data.entries = entries; data.loaded = true; @@ -310,7 +325,10 @@ impl PluginCache { } } - let mut guard = self.watcher.lock().unwrap(); + let mut guard = self + .watcher + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); *guard = Some(watcher); } } diff --git a/Backend/src/state.rs b/Backend/src/state.rs index 1755152..325097a 100644 --- a/Backend/src/state.rs +++ b/Backend/src/state.rs @@ -18,44 +18,6 @@ use serde::{Deserialize, Serialize}; /// Default number of recent repositories stored when settings are missing or invalid. pub const MAX_RECENTS: usize = 10; -/// Applies Git SSH-related environment variables from current settings. -/// -/// # Parameters -/// - `cfg`: Current app configuration. -/// -/// # Returns -/// - `()`. -fn apply_git_ssh_env(cfg: &AppConfig) { - // Prefer config-driven runtime env so the VCS backend (in another crate) can read it. - // Keep env var names stable for packaging and troubleshooting. - unsafe { - // Safety: OpenVCS sets these env vars during startup/config updates and treats them as - // process-wide configuration for child processes (e.g. `git`). - std::env::set_var( - "OPENVCS_SSH_MODE", - match cfg.git.ssh_binary { - crate::settings::GitSshBinary::Auto => "auto", - crate::settings::GitSshBinary::Host => "host", - crate::settings::GitSshBinary::Bundled => "bundled", - crate::settings::GitSshBinary::Custom => "custom", - }, - ); - } - if cfg.git.ssh_binary == crate::settings::GitSshBinary::Custom - && !cfg.git.ssh_path.trim().is_empty() - { - unsafe { - // Safety: see comment above. - std::env::set_var("OPENVCS_SSH", cfg.git.ssh_path.trim()); - } - } else { - unsafe { - // Safety: see comment above. - std::env::remove_var("OPENVCS_SSH"); - } - } -} - /// Central application state. /// Keeps track of the currently open repo and MRU recents. /// Backend choice is tied to each repo (via `Repo::id()`), not stored globally. @@ -84,10 +46,9 @@ impl AppState { /// Creates app state by loading persisted settings and recent repositories. /// /// # Returns - /// - A fully initialized [`AppState`] with config, recents, and runtime env applied. + /// - A fully initialized [`AppState`] with config and recent repositories loaded. pub fn new_with_config() -> Self { let cfg = AppConfig::load_or_default(); // reads ~/.config/openvcs/openvcs.conf - apply_git_ssh_env(&cfg); let s = Self { config: RwLock::new(cfg), repo_config: RwLock::new(RepoConfig::default()), @@ -125,7 +86,6 @@ impl AppState { next.migrate(); next.validate(); next.save().map_err(|e| e.to_string())?; - apply_git_ssh_env(&next); crate::monitoring::sync_backend_monitoring(&next); *self.config.write() = next; self.enforce_recents_limit_and_persist(); @@ -134,14 +94,6 @@ impl AppState { /* -------- repo config -------- */ - /// Returns a snapshot of repository-local settings. - /// - /// # Returns - /// - A cloned [`RepoConfig`] for the current repository context. - pub fn repo_config(&self) -> RepoConfig { - self.repo_config.read().clone() - } - /// Replaces repository-local settings kept in memory. /// /// # Parameters diff --git a/Backend/src/tauri_commands/commit.rs b/Backend/src/tauri_commands/commit.rs index 9362439..f4ac748 100644 --- a/Backend/src/tauri_commands/commit.rs +++ b/Backend/src/tauri_commands/commit.rs @@ -6,10 +6,28 @@ use log::{error, info}; use tauri::{async_runtime, Manager, Runtime, State, Window}; use crate::core::models::VcsEvent; +use crate::repo::Repo; use crate::state::AppState; use super::{current_repo_or_err, progress_bridge, run_repo_task}; +/// Resolves the repository commit identity from Git config. +/// +/// # Parameters +/// - `repo`: Active repository handle. +/// +/// # Returns +/// - `Ok((name, email))` when Git has a configured identity. +/// - `Err(String)` when the repository has no usable commit identity. +fn commit_identity(repo: &Repo) -> Result<(String, String), String> { + repo.inner() + .get_identity() + .map_err(|e| e.to_string())? + .ok_or_else(|| { + "No Git commit identity configured for this repository; set user.name and user.email in Git".to_string() + }) +} + #[tauri::command] /// Commits all staged/working-tree changes using summary + optional description. /// @@ -49,20 +67,7 @@ pub async fn commit_changes( }); info!("Staging changes for commit"); - let (name, email) = repo - .inner() - .get_identity() - .ok() - .flatten() - .or_else(|| { - let n = std::env::var("GIT_AUTHOR_NAME").ok(); - let e = std::env::var("GIT_AUTHOR_EMAIL").ok(); - match (n, e) { - (Some(n), Some(e)) if !n.is_empty() && !e.is_empty() => Some((n, e)), - _ => None, - } - }) - .unwrap_or_else(|| ("OpenVCS".into(), "openvcs@example".into())); + let (name, email) = commit_identity(repo.as_ref())?; info!("Using identity: {} <{}>", name, email); on(VcsEvent::Info { @@ -125,20 +130,7 @@ pub async fn commit_selected( async_runtime::spawn_blocking(move || { let on = progress_bridge(app); - let (name, email) = repo - .inner() - .get_identity() - .ok() - .flatten() - .or_else(|| { - let n = std::env::var("GIT_AUTHOR_NAME").ok(); - let e = std::env::var("GIT_AUTHOR_EMAIL").ok(); - match (n, e) { - (Some(n), Some(e)) if !n.is_empty() && !e.is_empty() => Some((n, e)), - _ => None, - } - }) - .unwrap_or_else(|| ("OpenVCS".into(), "openvcs@example".into())); + let (name, email) = commit_identity(repo.as_ref())?; let paths: Vec = files.into_iter().map(PathBuf::from).collect(); @@ -210,20 +202,7 @@ pub async fn commit_patch( e.to_string() })?; - let (name, email) = repo - .inner() - .get_identity() - .ok() - .flatten() - .or_else(|| { - let n = std::env::var("GIT_AUTHOR_NAME").ok(); - let e = std::env::var("GIT_AUTHOR_EMAIL").ok(); - match (n, e) { - (Some(n), Some(e)) if !n.is_empty() && !e.is_empty() => Some((n, e)), - _ => None, - } - }) - .unwrap_or_else(|| ("OpenVCS".into(), "openvcs@example".into())); + let (name, email) = commit_identity(repo.as_ref())?; on(VcsEvent::Info { msg: "Committing staged hunks…".into(), @@ -250,7 +229,8 @@ pub async fn commit_patch( /// - `summary`: Commit summary line. /// - `description`: Optional commit body text. /// - `patch`: Optional patch text to stage first. -/// - `files`: Optional explicit file list. +/// - `files`: Explicit commit path list. +/// - `stage_paths`: Full-file paths to stage directly before commit. /// /// # Returns /// - `Ok(String)` created commit id. @@ -262,11 +242,13 @@ pub async fn commit_patch_and_files( description: String, patch: String, files: Vec, + stage_paths: Vec, ) -> Result { info!( - "commit_patch_and_files called (patch bytes={}, files={})", + "commit_patch_and_files called (patch bytes={}, files={}, stage_paths={})", patch.len(), - files.len() + files.len(), + stage_paths.len() ); let repo = state .current_repo() @@ -293,39 +275,31 @@ pub async fn commit_patch_and_files( })?; } - let (name, email) = repo - .inner() - .get_identity() - .ok() - .flatten() - .or_else(|| { - let n = std::env::var("GIT_AUTHOR_NAME").ok(); - let e = std::env::var("GIT_AUTHOR_EMAIL").ok(); - match (n, e) { - (Some(n), Some(e)) if !n.is_empty() && !e.is_empty() => Some((n, e)), - _ => None, - } - }) - .unwrap_or_else(|| ("OpenVCS".into(), "openvcs@example".into())); + let (name, email) = commit_identity(repo.as_ref())?; on(VcsEvent::Info { msg: "Writing commit…".into(), }); - let oid = if files.is_empty() { - repo.inner() - .commit_index(&message, &name, &email) - .map_err(|e| e.to_string())? - } else { - let paths: Vec = files.iter().map(PathBuf::from).collect(); + let stage_paths: Vec = stage_paths.iter().map(PathBuf::from).collect(); + if !stage_paths.is_empty() { on(VcsEvent::Info { msg: "Staging selected files…".into(), }); - repo.inner().stage_paths(&paths).map_err(|e| { + repo.inner().stage_paths(&stage_paths).map_err(|e| { error!("stage_paths failed: {e}"); e.to_string() })?; + } + let commit_paths: Vec = if files.is_empty() { + stage_paths.clone() + } else { + files.iter().map(PathBuf::from).collect() + }; + let oid = if commit_paths.is_empty() { + return Err("No commit paths provided".into()); + } else { repo.inner() - .commit(&message, &name, &email, &paths) + .commit(&message, &name, &email, &commit_paths) .map_err(|e| e.to_string())? }; on(VcsEvent::Info { diff --git a/Backend/src/tauri_commands/settings.rs b/Backend/src/tauri_commands/settings.rs index 40c8263..9935e6e 100644 --- a/Backend/src/tauri_commands/settings.rs +++ b/Backend/src/tauri_commands/settings.rs @@ -110,7 +110,7 @@ pub fn set_global_settings(state: State<'_, AppState>, cfg: AppConfig) -> Result /// - `Ok(RepoConfig)` current effective repository settings. /// - `Err(String)` when repo queries fail. pub async fn get_repo_settings(state: State<'_, AppState>) -> Result { - let mut cfg = state.repo_config(); + let mut cfg = RepoConfig::default(); if let Some(repo) = state.current_repo() { let (identity, remotes) = run_repo_task("get_repo_settings", repo, move |repo| { let identity = match repo.inner().get_identity() { diff --git a/Frontend/src/modals/repo-settings.html b/Frontend/src/modals/repo-settings.html index 57a2105..34e1557 100644 --- a/Frontend/src/modals/repo-settings.html +++ b/Frontend/src/modals/repo-settings.html @@ -2,16 +2,19 @@