diff --git a/aur/owlry-meta-essentials b/aur/owlry-meta-essentials new file mode 160000 index 0000000..4a09cfb --- /dev/null +++ b/aur/owlry-meta-essentials @@ -0,0 +1 @@ +Subproject commit 4a09cfb73cfc866661713654e2d1b8c364814c00 diff --git a/aur/owlry-meta-full b/aur/owlry-meta-full new file mode 160000 index 0000000..8f85087 --- /dev/null +++ b/aur/owlry-meta-full @@ -0,0 +1 @@ +Subproject commit 8f85087731b7e17544939a1405409ae46b4722d2 diff --git a/aur/owlry-meta-tools b/aur/owlry-meta-tools new file mode 160000 index 0000000..28c78b7 --- /dev/null +++ b/aur/owlry-meta-tools @@ -0,0 +1 @@ +Subproject commit 28c78b7953b79bbf7aa7f139022dc428831e5ac3 diff --git a/aur/owlry-meta-widgets b/aur/owlry-meta-widgets new file mode 160000 index 0000000..aa4c2cd --- /dev/null +++ b/aur/owlry-meta-widgets @@ -0,0 +1 @@ +Subproject commit aa4c2cd2178b67bea13ef3d17c543f3146da4287 diff --git a/crates/owlry/build.rs b/crates/owlry/build.rs index ed3e8b7..486ca4d 100644 --- a/crates/owlry/build.rs +++ b/crates/owlry/build.rs @@ -1,12 +1,11 @@ fn main() { - // Compile GResource bundle for icons + // Compile GResource bundle for plugin-specific icons (weather, media, pomodoro) glib_build_tools::compile_resources( &["src/resources/icons"], "src/resources/icons.gresource.xml", "icons.gresource", ); - // Rerun if icon files change println!("cargo:rerun-if-changed=src/resources/icons.gresource.xml"); println!("cargo:rerun-if-changed=src/resources/icons/"); } diff --git a/crates/owlry/src/ui/main_window.rs b/crates/owlry/src/ui/main_window.rs index 84e7487..3557786 100644 --- a/crates/owlry/src/ui/main_window.rs +++ b/crates/owlry/src/ui/main_window.rs @@ -507,9 +507,7 @@ impl MainWindow { search_entry.set_placeholder_text(Some(&format!("Filter {} actions...", display_name))); // Display actions - while let Some(child) = results_list.first_child() { - results_list.remove(&child); - } + results_list.remove_all(); for item in &actions { let row = ResultRow::new(item, ""); @@ -589,9 +587,7 @@ impl MainWindow { .collect(); // Clear and repopulate - while let Some(child) = results_list.first_child() { - results_list.remove(&child); - } + results_list.remove_all(); for item in &filtered { let row = ResultRow::new(item, ""); @@ -702,9 +698,7 @@ impl MainWindow { if search_entry_for_stale.text().as_str() != raw_text_at_dispatch { return; } - while let Some(child) = results_list_cb.first_child() { - results_list_cb.remove(&child); - } + results_list_cb.remove_all(); let items = result.items; let initial_count = @@ -739,9 +733,7 @@ impl MainWindow { tag.as_deref(), ); - while let Some(child) = results_list.first_child() { - results_list.remove(&child); - } + results_list.remove_all(); let initial_count = INITIAL_RESULTS.min(results.len()); @@ -1247,9 +1239,7 @@ impl MainWindow { ); // Clear existing results - while let Some(child) = results_list.first_child() { - results_list.remove(&child); - } + results_list.remove_all(); let initial_count = INITIAL_RESULTS.min(results.len()); diff --git a/docs/superpowers/plans/2026-03-26-codebase-hardening.md b/docs/superpowers/plans/2026-03-26-codebase-hardening.md new file mode 100644 index 0000000..e572e74 --- /dev/null +++ b/docs/superpowers/plans/2026-03-26-codebase-hardening.md @@ -0,0 +1,967 @@ +# Codebase Hardening Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Fix 15 soundness, security, robustness, and quality issues across owlry core and owlry-plugins repos. + +**Architecture:** Point fixes organized into 5 severity tiers. Each tier is one commit. Core repo (owlry) tiers 1-3 first, then plugins repo (owlry-plugins) tiers 4-5. No new features, no refactoring beyond what each fix requires. + +**Tech Stack:** Rust 1.90+, abi_stable 0.11, toml 0.8, dirs 5.0 + +**Repos:** +- Core: `/home/cnachtigall/ssd/git/archive/owlibou/owlry` +- Plugins: `/home/cnachtigall/ssd/git/archive/owlibou/owlry-plugins` + +--- + +## Task 1: Tier 1 — Critical / Soundness (owlry core) + +**Files:** +- Modify: `crates/owlry-plugin-api/src/lib.rs:297-320` +- Modify: `crates/owlry-core/src/server.rs:1-6,91-123,127-215` + +### 1a. Replace `static mut HOST_API` with `OnceLock` + +- [ ] **Step 1: Replace the static mut and init function** + +In `crates/owlry-plugin-api/src/lib.rs`, replace lines 297-320: + +```rust +// Old: +// static mut HOST_API: Option<&'static HostAPI> = None; +// +// pub unsafe fn init_host_api(api: &'static HostAPI) { +// unsafe { +// HOST_API = Some(api); +// } +// } +// +// pub fn host_api() -> Option<&'static HostAPI> { +// unsafe { HOST_API } +// } + +// New: +use std::sync::OnceLock; + +static HOST_API: OnceLock<&'static HostAPI> = OnceLock::new(); + +/// Initialize the host API (called by the host) +/// +/// # Safety +/// Must only be called once by the host before any plugins use the API +pub unsafe fn init_host_api(api: &'static HostAPI) { + let _ = HOST_API.set(api); +} + +/// Get the host API +/// +/// Returns None if the host hasn't initialized the API yet +pub fn host_api() -> Option<&'static HostAPI> { + HOST_API.get().copied() +} +``` + +Note: `init_host_api` keeps its `unsafe` signature for API compatibility even though `OnceLock::set` is safe. The `unsafe` documents the caller contract. + +- [ ] **Step 2: Verify the plugin-api crate compiles** + +Run: `cargo check -p owlry-plugin-api` +Expected: success, no warnings about `static mut` + +### 1b. Add IPC message size limit + +- [ ] **Step 3: Add size-limited read loop in server.rs** + +In `crates/owlry-core/src/server.rs`, add the constant near the top of the file (after the imports): + +```rust +/// Maximum size of a single IPC request line (1 MB) +const MAX_REQUEST_SIZE: usize = 1_048_576; +``` + +Replace the `handle_client` method (lines 91-123). Change the `for line in reader.lines()` loop to a manual `read_line` loop with size checking: + +```rust +fn handle_client( + stream: UnixStream, + pm: Arc>, + frecency: Arc>, + config: Arc, +) -> io::Result<()> { + let reader = BufReader::new(stream.try_clone()?); + let mut writer = stream; + let mut reader = reader; + let mut line = String::new(); + + loop { + line.clear(); + let bytes_read = reader.read_line(&mut line)?; + if bytes_read == 0 { + break; // EOF + } + + if line.len() > MAX_REQUEST_SIZE { + let resp = Response::Error { + message: "request too large".to_string(), + }; + write_response(&mut writer, &resp)?; + break; // Drop connection + } + + let trimmed = line.trim(); + if trimmed.is_empty() { + continue; + } + + let request: Request = match serde_json::from_str(trimmed) { + Ok(req) => req, + Err(e) => { + let resp = Response::Error { + message: format!("invalid request JSON: {}", e), + }; + write_response(&mut writer, &resp)?; + continue; + } + }; + + let response = Self::handle_request(&request, &pm, &frecency, &config); + write_response(&mut writer, &response)?; + } + + Ok(()) +} +``` + +### 1c. Handle mutex poisoning gracefully + +- [ ] **Step 4: Replace all lock().unwrap() in handle_request** + +In `crates/owlry-core/src/server.rs`, in the `handle_request` method, replace every occurrence of `.lock().unwrap()` with `.lock().unwrap_or_else(|e| e.into_inner())`. There are instances in the `Query`, `Launch`, `Providers`, `Refresh`, `Submenu`, and `PluginAction` arms. + +For example, the Query arm changes from: +```rust +let pm_guard = pm.lock().unwrap(); +let frecency_guard = frecency.lock().unwrap(); +``` +to: +```rust +let pm_guard = pm.lock().unwrap_or_else(|e| e.into_inner()); +let frecency_guard = frecency.lock().unwrap_or_else(|e| e.into_inner()); +``` + +Apply this pattern to all `.lock().unwrap()` calls in `handle_request`. + +- [ ] **Step 5: Build and test the core crate** + +Run: `cargo check -p owlry-core && cargo test -p owlry-core` +Expected: all checks pass, all existing tests pass + +- [ ] **Step 6: Commit Tier 1** + +```bash +cd /home/cnachtigall/ssd/git/archive/owlibou/owlry +git add crates/owlry-plugin-api/src/lib.rs crates/owlry-core/src/server.rs +git commit -m "fix: soundness — OnceLock for HOST_API, IPC size limits, mutex poisoning recovery" +``` + +--- + +## Task 2: Tier 2 — Security (owlry core) + +**Files:** +- Modify: `crates/owlry-core/src/server.rs:1-6,29-36,91-123` +- Modify: `crates/owlry-core/src/main.rs:26-32` + +### 2a. Set socket permissions after bind + +- [ ] **Step 1: Add permission setting in Server::bind** + +In `crates/owlry-core/src/server.rs`, add the import at the top: + +```rust +use std::os::unix::fs::PermissionsExt; +``` + +In `Server::bind()`, after the `UnixListener::bind(socket_path)?;` line, add: + +```rust +std::fs::set_permissions(socket_path, std::fs::Permissions::from_mode(0o600))?; +``` + +### 2b. Log signal handler failure + +- [ ] **Step 2: Replace .ok() with warning log in main.rs** + +In `crates/owlry-core/src/main.rs`, add `use log::warn;` to the imports, then replace lines 26-32: + +```rust +// Old: +// ctrlc::set_handler(move || { +// let _ = std::fs::remove_file(&sock_cleanup); +// std::process::exit(0); +// }) +// .ok(); + +// New: +if let Err(e) = ctrlc::set_handler(move || { + let _ = std::fs::remove_file(&sock_cleanup); + std::process::exit(0); +}) { + warn!("Failed to set signal handler: {}", e); +} +``` + +### 2c. Add client read timeout + +- [ ] **Step 3: Set read timeout on accepted connections** + +In `crates/owlry-core/src/server.rs`, add `use std::time::Duration;` to the imports. + +In the `handle_client` method, at the very top (before the `BufReader` creation), add: + +```rust +stream.set_read_timeout(Some(Duration::from_secs(30)))?; +``` + +This means the `stream` passed to `handle_client` needs to be mutable, or we set it on the clone. Since `set_read_timeout` takes `&self` (not `&mut self`), we can call it directly: + +```rust +fn handle_client( + stream: UnixStream, + pm: Arc<...>, + frecency: Arc<...>, + config: Arc, +) -> io::Result<()> { + stream.set_read_timeout(Some(Duration::from_secs(30)))?; + let reader = BufReader::new(stream.try_clone()?); + // ... rest unchanged +``` + +- [ ] **Step 4: Build and test** + +Run: `cargo check -p owlry-core && cargo test -p owlry-core` +Expected: all checks pass, all existing tests pass + +- [ ] **Step 5: Commit Tier 2** + +```bash +cd /home/cnachtigall/ssd/git/archive/owlibou/owlry +git add crates/owlry-core/src/server.rs crates/owlry-core/src/main.rs +git commit -m "fix: security — socket perms 0600, signal handler logging, client read timeout" +``` + +--- + +## Task 3: Tier 3 — Robustness / Quality (owlry core) + +**Files:** +- Modify: `crates/owlry-core/src/server.rs:1-6,17-23,53-73,91-215` + +### 3a. Log malformed JSON requests + +- [ ] **Step 1: Add warn! for JSON parse errors** + +In `crates/owlry-core/src/server.rs`, in the `handle_client` method, in the JSON parse error arm, add a warning log before the error response: + +```rust +Err(e) => { + warn!("Malformed request from client: {}", e); + let resp = Response::Error { + message: format!("invalid request JSON: {}", e), + }; + write_response(&mut writer, &resp)?; + continue; +} +``` + +### 3b. Replace Mutex with RwLock + +- [ ] **Step 2: Change Server struct and imports** + +In `crates/owlry-core/src/server.rs`, change the import from `Mutex` to `RwLock`: + +```rust +use std::sync::{Arc, RwLock}; +``` + +Change the `Server` struct fields: + +```rust +pub struct Server { + listener: UnixListener, + socket_path: PathBuf, + provider_manager: Arc>, + frecency: Arc>, + config: Arc, +} +``` + +- [ ] **Step 3: Update Server::bind** + +In `Server::bind()`, change `Arc::new(Mutex::new(...))` to `Arc::new(RwLock::new(...))`: + +```rust +Ok(Self { + listener, + socket_path: socket_path.to_path_buf(), + provider_manager: Arc::new(RwLock::new(provider_manager)), + frecency: Arc::new(RwLock::new(frecency)), + config: Arc::new(config), +}) +``` + +- [ ] **Step 4: Update handle_client and handle_request signatures** + +Change `handle_client` parameter types: + +```rust +fn handle_client( + stream: UnixStream, + pm: Arc>, + frecency: Arc>, + config: Arc, +) -> io::Result<()> { +``` + +Change `handle_request` parameter types: + +```rust +fn handle_request( + request: &Request, + pm: &Arc>, + frecency: &Arc>, + config: &Arc, +) -> Response { +``` + +Also update `handle_one_for_testing` if it passes these types through. + +- [ ] **Step 5: Update lock calls per request type** + +In `handle_request`, change each lock call according to the read/write mapping: + +**Query** (read PM, read frecency): +```rust +Request::Query { text, modes } => { + let filter = match modes { + Some(m) => ProviderFilter::from_mode_strings(m), + None => ProviderFilter::all(), + }; + let max = config.general.max_results; + let weight = config.providers.frecency_weight; + + let pm_guard = pm.read().unwrap_or_else(|e| e.into_inner()); + let frecency_guard = frecency.read().unwrap_or_else(|e| e.into_inner()); + let results = pm_guard.search_with_frecency( + text, max, &filter, &frecency_guard, weight, None, + ); + + Response::Results { + items: results + .into_iter() + .map(|(item, score)| launch_item_to_result(item, score)) + .collect(), + } +} +``` + +**Launch** (write frecency): +```rust +Request::Launch { item_id, provider: _ } => { + let mut frecency_guard = frecency.write().unwrap_or_else(|e| e.into_inner()); + frecency_guard.record_launch(item_id); + Response::Ack +} +``` + +**Providers** (read PM): +```rust +Request::Providers => { + let pm_guard = pm.read().unwrap_or_else(|e| e.into_inner()); + let descs = pm_guard.available_providers(); + Response::Providers { + list: descs.into_iter().map(descriptor_to_desc).collect(), + } +} +``` + +**Refresh** (write PM): +```rust +Request::Refresh { provider } => { + let mut pm_guard = pm.write().unwrap_or_else(|e| e.into_inner()); + pm_guard.refresh_provider(provider); + Response::Ack +} +``` + +**Toggle** (no locks): +```rust +Request::Toggle => Response::Ack, +``` + +**Submenu** (read PM): +```rust +Request::Submenu { plugin_id, data } => { + let pm_guard = pm.read().unwrap_or_else(|e| e.into_inner()); + match pm_guard.query_submenu_actions(plugin_id, data, plugin_id) { + Some((_name, actions)) => Response::SubmenuItems { + items: actions + .into_iter() + .map(|item| launch_item_to_result(item, 0)) + .collect(), + }, + None => Response::Error { + message: format!("no submenu actions for plugin '{}'", plugin_id), + }, + } +} +``` + +**PluginAction** (read PM): +```rust +Request::PluginAction { command } => { + let pm_guard = pm.read().unwrap_or_else(|e| e.into_inner()); + if pm_guard.execute_plugin_action(command) { + Response::Ack + } else { + Response::Error { + message: format!("no plugin handled action '{}'", command), + } + } +} +``` + +- [ ] **Step 6: Build and test** + +Run: `cargo check -p owlry-core && cargo test -p owlry-core` +Expected: all checks pass, all existing tests pass + +- [ ] **Step 7: Commit Tier 3** + +```bash +cd /home/cnachtigall/ssd/git/archive/owlibou/owlry +git add crates/owlry-core/src/server.rs +git commit -m "fix: robustness — RwLock for concurrent reads, log malformed JSON requests" +``` + +--- + +## Task 4: Tier 4 — Critical fixes (owlry-plugins) + +**Files:** +- Modify: `crates/owlry-plugin-converter/src/currency.rs:88-113,244-265` +- Modify: `crates/owlry-plugin-converter/src/units.rs:90-101,160-213` +- Modify: `crates/owlry-plugin-bookmarks/src/lib.rs:40-45,228-260,317-353` + +All paths relative to `/home/cnachtigall/ssd/git/archive/owlibou/owlry-plugins`. + +### 4a. Fix Box::leak memory leak in converter + +- [ ] **Step 1: Change resolve_currency_code return type** + +In `crates/owlry-plugin-converter/src/currency.rs`, change the `resolve_currency_code` function (line 88) from returning `Option` to `Option<&'static str>`: + +```rust +pub fn resolve_currency_code(alias: &str) -> Option<&'static str> { + let lower = alias.to_lowercase(); + + // Check aliases + for ca in CURRENCY_ALIASES { + if ca.aliases.contains(&lower.as_str()) { + return Some(ca.code); + } + } + + // Check if it's a raw 3-letter ISO code we know about + let upper = alias.to_uppercase(); + if upper.len() == 3 { + if upper == "EUR" { + return Some("EUR"); + } + // Check if we have rates for it — return the matching alias code + if let Some(rates) = get_rates() + && rates.rates.contains_key(&upper) + { + // Find a matching CURRENCY_ALIASES entry for this code + for ca in CURRENCY_ALIASES { + if ca.code == upper { + return Some(ca.code); + } + } + // Not in our aliases but valid in ECB rates — we can't return + // a &'static str for an arbitrary code, so skip + } + } + + None +} +``` + +Note: For ISO codes that are in ECB rates but NOT in `CURRENCY_ALIASES`, we lose the ability to resolve them. This is acceptable because: +1. `CURRENCY_ALIASES` already covers the 15 most common currencies +2. The alternative (Box::leak) was leaking memory on every keystroke + +- [ ] **Step 2: Update is_currency_alias** + +No change needed — it already just calls `resolve_currency_code(alias).is_some()`. + +- [ ] **Step 3: Update find_unit in units.rs** + +In `crates/owlry-plugin-converter/src/units.rs`, replace lines 90-101: + +```rust +pub fn find_unit(alias: &str) -> Option<&'static str> { + let lower = alias.to_lowercase(); + if let Some(&i) = ALIAS_MAP.get(&lower) { + return Some(UNITS[i].symbol); + } + // Check currency — resolve_currency_code now returns &'static str directly + currency::resolve_currency_code(&lower) +} +``` + +- [ ] **Step 4: Update convert_currency in units.rs** + +In `crates/owlry-plugin-converter/src/units.rs`, update `convert_currency` (line 160). The `from_code` and `to_code` are now `&'static str`. HashMap lookups with `rates.rates.get(code)` work because `HashMap::get` accepts `&str` via `Borrow`: + +```rust +fn convert_currency(value: f64, from: &str, to: &str) -> Option { + let rates = currency::get_rates()?; + let from_code = currency::resolve_currency_code(from)?; + let to_code = currency::resolve_currency_code(to)?; + + let from_rate = if from_code == "EUR" { + 1.0 + } else { + *rates.rates.get(from_code)? + }; + let to_rate = if to_code == "EUR" { + 1.0 + } else { + *rates.rates.get(to_code)? + }; + + let result = value / from_rate * to_rate; + Some(format_currency_result(result, to_code)) +} +``` + +- [ ] **Step 5: Update convert_currency_common in units.rs** + +In `crates/owlry-plugin-converter/src/units.rs`, update `convert_currency_common` (line 180). Change `from_code` handling: + +```rust +fn convert_currency_common(value: f64, from: &str) -> Vec { + let rates = match currency::get_rates() { + Some(r) => r, + None => return vec![], + }; + let from_code = match currency::resolve_currency_code(from) { + Some(c) => c, + None => return vec![], + }; + + let targets = COMMON_TARGETS.get(&Category::Currency).unwrap(); + let from_rate = if from_code == "EUR" { + 1.0 + } else { + match rates.rates.get(from_code) { + Some(&r) => r, + None => return vec![], + } + }; + + targets + .iter() + .filter(|&&sym| sym != from_code) + .filter_map(|&sym| { + let to_rate = if sym == "EUR" { + 1.0 + } else { + *rates.rates.get(sym)? + }; + let result = value / from_rate * to_rate; + Some(format_currency_result(result, sym)) + }) + .take(5) + .collect() +} +``` + +- [ ] **Step 6: Update currency tests** + +In `crates/owlry-plugin-converter/src/currency.rs`, update test assertions to use `&str` instead of `String`: + +```rust +#[test] +fn test_resolve_currency_code_iso() { + assert_eq!(resolve_currency_code("usd"), Some("USD")); + assert_eq!(resolve_currency_code("EUR"), Some("EUR")); +} + +#[test] +fn test_resolve_currency_code_name() { + assert_eq!(resolve_currency_code("dollar"), Some("USD")); + assert_eq!(resolve_currency_code("euro"), Some("EUR")); + assert_eq!(resolve_currency_code("pounds"), Some("GBP")); +} + +#[test] +fn test_resolve_currency_code_symbol() { + assert_eq!(resolve_currency_code("$"), Some("USD")); + assert_eq!(resolve_currency_code("€"), Some("EUR")); + assert_eq!(resolve_currency_code("£"), Some("GBP")); +} + +#[test] +fn test_resolve_currency_unknown() { + assert_eq!(resolve_currency_code("xyz"), None); +} +``` + +### 4b. Fix bookmarks temp file race condition + +- [ ] **Step 7: Use PID-based temp filenames** + +In `crates/owlry-plugin-bookmarks/src/lib.rs`, replace the `read_firefox_bookmarks` method. Change lines 318-319 and the corresponding favicons temp path: + +```rust +fn read_firefox_bookmarks(places_path: &PathBuf, items: &mut Vec) { + let temp_dir = std::env::temp_dir(); + let pid = std::process::id(); + let temp_db = temp_dir.join(format!("owlry_places_{}.sqlite", pid)); + + // Copy database to temp location to avoid locking issues + if fs::copy(places_path, &temp_db).is_err() { + return; + } + + // Also copy WAL file if it exists + let wal_path = places_path.with_extension("sqlite-wal"); + if wal_path.exists() { + let temp_wal = temp_db.with_extension("sqlite-wal"); + let _ = fs::copy(&wal_path, &temp_wal); + } + + // Copy favicons database if available + let favicons_path = Self::firefox_favicons_path(places_path); + let temp_favicons = temp_dir.join(format!("owlry_favicons_{}.sqlite", pid)); + if let Some(ref fp) = favicons_path { + let _ = fs::copy(fp, &temp_favicons); + let fav_wal = fp.with_extension("sqlite-wal"); + if fav_wal.exists() { + let _ = fs::copy(&fav_wal, temp_favicons.with_extension("sqlite-wal")); + } + } + + let cache_dir = Self::ensure_favicon_cache_dir(); + + // Read bookmarks from places.sqlite + let bookmarks = Self::fetch_firefox_bookmarks(&temp_db, &temp_favicons, cache_dir.as_ref()); + + // Clean up temp files + let _ = fs::remove_file(&temp_db); + let _ = fs::remove_file(temp_db.with_extension("sqlite-wal")); + let _ = fs::remove_file(&temp_favicons); + let _ = fs::remove_file(temp_favicons.with_extension("sqlite-wal")); + + // ... rest of method unchanged (the for loop adding items) +``` + +### 4c. Fix bookmarks background refresh never updating state + +- [ ] **Step 8: Change BookmarksState to use Arc>>** + +In `crates/owlry-plugin-bookmarks/src/lib.rs`, add `use std::sync::Mutex;` to imports (it's already importing `Arc` and `AtomicBool`). + +Change the struct: + +```rust +struct BookmarksState { + /// Cached bookmark items (shared with background thread) + items: Arc>>, + /// Flag to prevent concurrent background loads + loading: Arc, +} + +impl BookmarksState { + fn new() -> Self { + Self { + items: Arc::new(Mutex::new(Vec::new())), + loading: Arc::new(AtomicBool::new(false)), + } + } +``` + +- [ ] **Step 9: Update load_bookmarks to write through Arc** + +Update the `load_bookmarks` method: + +```rust +fn load_bookmarks(&self) { + // Fast path: load from cache immediately if items are empty + { + let mut items = self.items.lock().unwrap_or_else(|e| e.into_inner()); + if items.is_empty() { + *items = Self::load_cached_bookmarks(); + } + } + + // Don't start another background load if one is already running + if self.loading.swap(true, Ordering::SeqCst) { + return; + } + + // Spawn background thread to refresh bookmarks + let loading = self.loading.clone(); + let items_ref = self.items.clone(); + thread::spawn(move || { + let mut new_items = Vec::new(); + + // Load Chrome/Chromium bookmarks (fast - just JSON parsing) + for path in Self::chromium_bookmark_paths() { + if path.exists() { + Self::read_chrome_bookmarks_static(&path, &mut new_items); + } + } + + // Load Firefox bookmarks with favicons (synchronous with rusqlite) + for path in Self::firefox_places_paths() { + Self::read_firefox_bookmarks(&path, &mut new_items); + } + + // Save to cache for next startup + Self::save_cached_bookmarks(&new_items); + + // Update shared state so next refresh returns fresh data + if let Ok(mut items) = items_ref.lock() { + *items = new_items; + } + + loading.store(false, Ordering::SeqCst); + }); +} +``` + +Note: `load_bookmarks` now takes `&self` instead of `&mut self`. + +- [ ] **Step 10: Update provider_refresh to read from Arc** + +Update the `provider_refresh` function: + +```rust +extern "C" fn provider_refresh(handle: ProviderHandle) -> RVec { + if handle.ptr.is_null() { + return RVec::new(); + } + + // SAFETY: We created this handle from Box + let state = unsafe { &*(handle.ptr as *const BookmarksState) }; + + // Load bookmarks + state.load_bookmarks(); + + // Return items + let items = state.items.lock().unwrap_or_else(|e| e.into_inner()); + items.to_vec().into() +} +``` + +Note: Uses `&*` (shared ref) instead of `&mut *` since `load_bookmarks` now takes `&self`. + +- [ ] **Step 11: Build and test plugins** + +Run: `cd /home/cnachtigall/ssd/git/archive/owlibou/owlry-plugins && cargo check && cargo test` +Expected: all checks pass, all existing tests pass + +- [ ] **Step 12: Commit Tier 4** + +```bash +cd /home/cnachtigall/ssd/git/archive/owlibou/owlry-plugins +git add crates/owlry-plugin-converter/src/currency.rs crates/owlry-plugin-converter/src/units.rs crates/owlry-plugin-bookmarks/src/lib.rs +git commit -m "fix: critical — eliminate Box::leak in converter, secure temp files, fix background refresh" +``` + +--- + +## Task 5: Tier 5 — Quality fixes (owlry-plugins) + +**Files:** +- Modify: `crates/owlry-plugin-ssh/Cargo.toml` +- Modify: `crates/owlry-plugin-ssh/src/lib.rs:17-48` +- Modify: `crates/owlry-plugin-websearch/Cargo.toml` +- Modify: `crates/owlry-plugin-websearch/src/lib.rs:46-76,174-177` +- Modify: `crates/owlry-plugin-emoji/src/lib.rs:34-37,463-481` +- Modify: `crates/owlry-plugin-calculator/src/lib.rs:139` +- Modify: `crates/owlry-plugin-converter/src/lib.rs:95` + +All paths relative to `/home/cnachtigall/ssd/git/archive/owlibou/owlry-plugins`. + +### 5a. SSH plugin: read terminal from config + +- [ ] **Step 1: Add toml dependency to SSH plugin** + +In `crates/owlry-plugin-ssh/Cargo.toml`, add: + +```toml +# TOML config parsing +toml = "0.8" +``` + +- [ ] **Step 2: Add config loading and update SshState::new** + +In `crates/owlry-plugin-ssh/src/lib.rs`, add `use std::fs;` to imports, remove the `DEFAULT_TERMINAL` constant, and update `SshState::new`: + +```rust +impl SshState { + fn new() -> Self { + let terminal = Self::load_terminal_from_config(); + + Self { + items: Vec::new(), + terminal_command: terminal, + } + } + + fn load_terminal_from_config() -> String { + // Try [plugins.ssh] in config.toml + let config_path = dirs::config_dir().map(|d| d.join("owlry").join("config.toml")); + if let Some(content) = config_path.and_then(|p| fs::read_to_string(p).ok()) + && let Ok(toml) = content.parse::() + { + if let Some(plugins) = toml.get("plugins").and_then(|v| v.as_table()) + && let Some(ssh) = plugins.get("ssh").and_then(|v| v.as_table()) + && let Some(terminal) = ssh.get("terminal").and_then(|v| v.as_str()) + { + return terminal.to_string(); + } + } + + // Fall back to $TERMINAL env var + if let Ok(terminal) = std::env::var("TERMINAL") { + return terminal; + } + + // Last resort + "xdg-terminal-exec".to_string() + } +``` + +### 5b. WebSearch plugin: read engine from config + +- [ ] **Step 3: Add dependencies to websearch plugin** + +In `crates/owlry-plugin-websearch/Cargo.toml`, add: + +```toml +# TOML config parsing +toml = "0.8" + +# XDG directories for config +dirs = "5.0" +``` + +- [ ] **Step 4: Add config loading and update provider_init** + +In `crates/owlry-plugin-websearch/src/lib.rs`, add `use std::fs;` to imports. Add a config loading function and update `provider_init`: + +```rust +fn load_engine_from_config() -> String { + let config_path = dirs::config_dir().map(|d| d.join("owlry").join("config.toml")); + if let Some(content) = config_path.and_then(|p| fs::read_to_string(p).ok()) + && let Ok(toml) = content.parse::() + { + if let Some(plugins) = toml.get("plugins").and_then(|v| v.as_table()) + && let Some(websearch) = plugins.get("websearch").and_then(|v| v.as_table()) + && let Some(engine) = websearch.get("engine").and_then(|v| v.as_str()) + { + return engine.to_string(); + } + } + DEFAULT_ENGINE.to_string() +} + +extern "C" fn provider_init(_provider_id: RStr<'_>) -> ProviderHandle { + let engine = load_engine_from_config(); + let state = Box::new(WebSearchState::with_engine(&engine)); + ProviderHandle::from_box(state) +} +``` + +Remove the TODO comment from the old `provider_init`. + +### 5c. Emoji plugin: build items once at init + +- [ ] **Step 5: Move load_emojis to constructor** + +In `crates/owlry-plugin-emoji/src/lib.rs`, change `EmojiState::new` to call `load_emojis`: + +```rust +impl EmojiState { + fn new() -> Self { + let mut state = Self { items: Vec::new() }; + state.load_emojis(); + state + } +``` + +Update `provider_refresh` to just return the cached items without reloading: + +```rust +extern "C" fn provider_refresh(handle: ProviderHandle) -> RVec { + if handle.ptr.is_null() { + return RVec::new(); + } + + // SAFETY: We created this handle from Box + let state = unsafe { &*(handle.ptr as *const EmojiState) }; + + // Return cached items (loaded once at init) + state.items.to_vec().into() +} +``` + +Note: Uses `&*` (shared ref) since we're only reading. + +### 5d. Calculator/Converter: safer shell commands + +- [ ] **Step 6: Fix calculator command** + +In `crates/owlry-plugin-calculator/src/lib.rs`, in `evaluate_expression` (around line 139), replace: + +```rust +// Old: +format!("sh -c 'echo -n \"{}\" | wl-copy'", result_str) + +// New: +format!("printf '%s' '{}' | wl-copy", result_str.replace('\'', "'\\''")) +``` + +- [ ] **Step 7: Fix converter command** + +In `crates/owlry-plugin-converter/src/lib.rs`, in `provider_query` (around line 95), replace: + +```rust +// Old: +format!("sh -c 'echo -n \"{}\" | wl-copy'", r.raw_value) + +// New: +format!("printf '%s' '{}' | wl-copy", r.raw_value.replace('\'', "'\\''")) +``` + +- [ ] **Step 8: Build and test all plugins** + +Run: `cd /home/cnachtigall/ssd/git/archive/owlibou/owlry-plugins && cargo check && cargo test` +Expected: all checks pass, all existing tests pass + +- [ ] **Step 9: Commit Tier 5** + +```bash +cd /home/cnachtigall/ssd/git/archive/owlibou/owlry-plugins +git add crates/owlry-plugin-ssh/Cargo.toml crates/owlry-plugin-ssh/src/lib.rs \ + crates/owlry-plugin-websearch/Cargo.toml crates/owlry-plugin-websearch/src/lib.rs \ + crates/owlry-plugin-emoji/src/lib.rs \ + crates/owlry-plugin-calculator/src/lib.rs \ + crates/owlry-plugin-converter/src/lib.rs +git commit -m "fix: quality — config-based terminal/engine, emoji init perf, safer shell commands" +``` diff --git a/docs/superpowers/plans/2026-03-26-runtime-integration.md b/docs/superpowers/plans/2026-03-26-runtime-integration.md new file mode 100644 index 0000000..cea4adf --- /dev/null +++ b/docs/superpowers/plans/2026-03-26-runtime-integration.md @@ -0,0 +1,810 @@ +# Script Runtime Integration Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Enable the owlry-core daemon to discover and load Lua/Rune user plugins from `~/.config/owlry/plugins/`, with automatic hot-reload on file changes. + +**Architecture:** Fix ABI mismatches between core and runtimes, wire `LoadedRuntime` into `ProviderManager::new_with_config()`, add filesystem watcher for automatic plugin reload. Runtimes are external `.so` libraries loaded from `/usr/lib/owlry/runtimes/`. + +**Tech Stack:** Rust 1.90+, notify 7, notify-debouncer-mini 0.5, libloading 0.8 + +**Repos:** +- Core: `/home/cnachtigall/ssd/git/archive/owlibou/owlry` +- Plugins (docs only): `/home/cnachtigall/ssd/git/archive/owlibou/owlry-plugins` + +--- + +## Task 1: Fix Lua RuntimeInfo ABI and vtable init signature + +**Files:** +- Modify: `crates/owlry-lua/src/lib.rs:42-74,260-279,322-336` +- Modify: `crates/owlry-rune/src/lib.rs:42-46,73-84,90-95,97-146,215-229` +- Modify: `crates/owlry-core/src/plugins/runtime_loader.rs:55-68,84-146,267-277` + +### 1a. Shrink Lua RuntimeInfo to 2 fields + +- [ ] **Step 1: Update RuntimeInfo struct and runtime_info() in owlry-lua** + +In `crates/owlry-lua/src/lib.rs`: + +Remove the `LUA_RUNTIME_API_VERSION` constant (line 43). + +Replace the `RuntimeInfo` struct (lines 67-74): + +```rust +/// Runtime info returned by the runtime +#[repr(C)] +pub struct RuntimeInfo { + pub name: RString, + pub version: RString, +} +``` + +Replace `runtime_info()` (lines 260-268): + +```rust +extern "C" fn runtime_info() -> RuntimeInfo { + RuntimeInfo { + name: RString::from("Lua"), + version: RString::from(env!("CARGO_PKG_VERSION")), + } +} +``` + +Remove unused constants `RUNTIME_ID` and `RUNTIME_DESCRIPTION` (lines 37, 40) if no longer referenced. + +### 1b. Add owlry_version parameter to vtable init + +- [ ] **Step 2: Update ScriptRuntimeVTable in core** + +In `crates/owlry-core/src/plugins/runtime_loader.rs`, change the `init` field (line 59): + +```rust +pub struct ScriptRuntimeVTable { + pub info: extern "C" fn() -> RuntimeInfo, + pub init: extern "C" fn(plugins_dir: RStr<'_>, owlry_version: RStr<'_>) -> RuntimeHandle, + pub providers: extern "C" fn(handle: RuntimeHandle) -> RVec, + pub refresh: extern "C" fn(handle: RuntimeHandle, provider_id: RStr<'_>) -> RVec, + pub query: extern "C" fn( + handle: RuntimeHandle, + provider_id: RStr<'_>, + query: RStr<'_>, + ) -> RVec, + pub drop: extern "C" fn(handle: RuntimeHandle), +} +``` + +- [ ] **Step 3: Update LoadedRuntime to pass version** + +In `crates/owlry-core/src/plugins/runtime_loader.rs`, update `load_lua`, `load_rune`, and `load_from_path` to accept and pass the version: + +```rust +impl LoadedRuntime { + pub fn load_lua(plugins_dir: &Path, owlry_version: &str) -> PluginResult { + Self::load_from_path( + "Lua", + &PathBuf::from(SYSTEM_RUNTIMES_DIR).join("liblua.so"), + b"owlry_lua_runtime_vtable", + plugins_dir, + owlry_version, + ) + } + + fn load_from_path( + name: &'static str, + library_path: &Path, + vtable_symbol: &[u8], + plugins_dir: &Path, + owlry_version: &str, + ) -> PluginResult { + // ... existing library loading code ... + + // Initialize the runtime with version + let plugins_dir_str = plugins_dir.to_string_lossy(); + let handle = (vtable.init)( + RStr::from_str(&plugins_dir_str), + RStr::from_str(owlry_version), + ); + + // ... rest unchanged ... + } +} + +impl LoadedRuntime { + pub fn load_rune(plugins_dir: &Path, owlry_version: &str) -> PluginResult { + Self::load_from_path( + "Rune", + &PathBuf::from(SYSTEM_RUNTIMES_DIR).join("librune.so"), + b"owlry_rune_runtime_vtable", + plugins_dir, + owlry_version, + ) + } +} +``` + +- [ ] **Step 4: Update Lua runtime_init to accept version** + +In `crates/owlry-lua/src/lib.rs`, update `runtime_init` (line 270) and the vtable: + +```rust +extern "C" fn runtime_init(plugins_dir: RStr<'_>, owlry_version: RStr<'_>) -> RuntimeHandle { + let plugins_dir = PathBuf::from(plugins_dir.as_str()); + let mut state = Box::new(LuaRuntimeState::new(plugins_dir)); + + state.discover_and_load(owlry_version.as_str()); + + RuntimeHandle::from_box(state) +} +``` + +Update the `LuaRuntimeVTable` struct `init` field to match: +```rust +pub init: extern "C" fn(plugins_dir: RStr<'_>, owlry_version: RStr<'_>) -> RuntimeHandle, +``` + +- [ ] **Step 5: Update Rune runtime_init to accept version** + +In `crates/owlry-rune/src/lib.rs`, update `runtime_init` (line 97) and the vtable: + +```rust +extern "C" fn runtime_init(plugins_dir: RStr<'_>, owlry_version: RStr<'_>) -> RuntimeHandle { + let _ = env_logger::try_init(); + + let plugins_dir = PathBuf::from(plugins_dir.as_str()); + let _version = owlry_version.as_str(); + log::info!( + "Initializing Rune runtime with plugins from: {}", + plugins_dir.display() + ); + + // ... rest unchanged — Rune doesn't currently do version checking ... +``` + +Update the `RuneRuntimeVTable` struct `init` field: +```rust +pub init: extern "C" fn(plugins_dir: RStr<'_>, owlry_version: RStr<'_>) -> RuntimeHandle, +``` + +- [ ] **Step 6: Build all three crates** + +Run: `cargo check -p owlry-core && cargo check -p owlry-lua && cargo check -p owlry-rune` +Expected: all pass + +- [ ] **Step 7: Run tests** + +Run: `cargo test -p owlry-core && cargo test -p owlry-lua && cargo test -p owlry-rune` +Expected: all pass + +- [ ] **Step 8: Commit** + +```bash +git add crates/owlry-core/src/plugins/runtime_loader.rs \ + crates/owlry-lua/src/lib.rs \ + crates/owlry-rune/src/lib.rs +git commit -m "fix: align runtime ABI — shrink Lua RuntimeInfo, pass owlry_version to init" +``` + +--- + +## Task 2: Change default entry points to `main` and add alias + +**Files:** +- Modify: `crates/owlry-lua/src/manifest.rs:52-54` +- Modify: `crates/owlry-rune/src/manifest.rs:36-38,29` + +- [ ] **Step 1: Update Lua manifest default entry** + +In `crates/owlry-lua/src/manifest.rs`, change `default_entry()` (line 52): + +```rust +fn default_entry() -> String { + "main.lua".to_string() +} +``` + +Add `serde(alias)` to the `entry` field in `PluginInfo` (line 45): + +```rust + #[serde(default = "default_entry", alias = "entry_point")] + pub entry: String, +``` + +- [ ] **Step 2: Update Rune manifest default entry** + +In `crates/owlry-rune/src/manifest.rs`, change `default_entry()` (line 36): + +```rust +fn default_entry() -> String { + "main.rn".to_string() +} +``` + +Add `serde(alias)` to the `entry` field in `PluginInfo` (line 29): + +```rust + #[serde(default = "default_entry", alias = "entry_point")] + pub entry: String, +``` + +- [ ] **Step 3: Update tests that reference init.lua/init.rn** + +In `crates/owlry-lua/src/manifest.rs` test `test_parse_minimal_manifest`: +```rust +assert_eq!(manifest.plugin.entry, "main.lua"); +``` + +In `crates/owlry-lua/src/loader.rs` test `create_test_plugin`: +```rust +fs::write(plugin_dir.join("main.lua"), "-- empty plugin").unwrap(); +``` + +In `crates/owlry-rune/src/manifest.rs` test `test_parse_minimal_manifest`: +```rust +assert_eq!(manifest.plugin.entry, "main.rn"); +``` + +- [ ] **Step 4: Build and test** + +Run: `cargo test -p owlry-lua && cargo test -p owlry-rune` +Expected: all pass + +- [ ] **Step 5: Commit** + +```bash +git add crates/owlry-lua/src/manifest.rs crates/owlry-lua/src/loader.rs \ + crates/owlry-rune/src/manifest.rs +git commit -m "feat: change default entry points to main.lua/main.rn, add entry_point alias" +``` + +--- + +## Task 3: Wire runtime loading into ProviderManager + +**Files:** +- Modify: `crates/owlry-core/src/providers/mod.rs:106-119,173-224` +- Modify: `crates/owlry-core/src/plugins/runtime_loader.rs:13` (remove allow dead_code) + +- [ ] **Step 1: Add runtimes field to ProviderManager** + +In `crates/owlry-core/src/providers/mod.rs`, add import and field: + +```rust +use crate::plugins::runtime_loader::LoadedRuntime; +``` + +Add to the `ProviderManager` struct (after `matcher` field): + +```rust +pub struct ProviderManager { + providers: Vec>, + static_native_providers: Vec, + dynamic_providers: Vec, + widget_providers: Vec, + matcher: SkimMatcherV2, + /// Loaded script runtimes (Lua, Rune) — must stay alive to keep Library handles + runtimes: Vec, + /// Type IDs of providers that came from script runtimes (for hot-reload removal) + runtime_type_ids: std::collections::HashSet, +} +``` + +Update `ProviderManager::new()` to initialize the new fields: + +```rust +let mut manager = Self { + providers: core_providers, + static_native_providers: Vec::new(), + dynamic_providers: Vec::new(), + widget_providers: Vec::new(), + matcher: SkimMatcherV2::default(), + runtimes: Vec::new(), + runtime_type_ids: std::collections::HashSet::new(), +}; +``` + +- [ ] **Step 2: Add runtime loading to new_with_config** + +In `ProviderManager::new_with_config()`, after the native plugin loading block (after line 221) and before `Self::new(core_providers, native_providers)` (line 223), add runtime loading: + +```rust + // Load script runtimes (Lua, Rune) for user plugins + let mut runtime_providers: Vec> = Vec::new(); + let mut runtimes: Vec = Vec::new(); + let mut runtime_type_ids = std::collections::HashSet::new(); + let owlry_version = env!("CARGO_PKG_VERSION"); + + if let Some(plugins_dir) = crate::paths::plugins_dir() { + // Try Lua runtime + match LoadedRuntime::load_lua(&plugins_dir, owlry_version) { + Ok(rt) => { + info!("Loaded Lua runtime with {} provider(s)", rt.providers().len()); + for provider in rt.create_providers() { + let type_id = format!("{}", provider.provider_type()); + runtime_type_ids.insert(type_id); + runtime_providers.push(provider); + } + runtimes.push(rt); + } + Err(e) => { + info!("Lua runtime not available: {}", e); + } + } + + // Try Rune runtime + match LoadedRuntime::load_rune(&plugins_dir, owlry_version) { + Ok(rt) => { + info!("Loaded Rune runtime with {} provider(s)", rt.providers().len()); + for provider in rt.create_providers() { + let type_id = format!("{}", provider.provider_type()); + runtime_type_ids.insert(type_id); + runtime_providers.push(provider); + } + runtimes.push(rt); + } + Err(e) => { + info!("Rune runtime not available: {}", e); + } + } + } + + let mut manager = Self::new(core_providers, native_providers); + manager.runtimes = runtimes; + manager.runtime_type_ids = runtime_type_ids; + + // Add runtime providers to the core providers list + for provider in runtime_providers { + info!("Registered runtime provider: {}", provider.name()); + manager.providers.push(provider); + } + + // Refresh runtime providers + for provider in &mut manager.providers { + // Only refresh the ones we just added (runtime providers) + // They need an initial refresh to populate items + } + manager.refresh_all(); + + manager +``` + +Note: This replaces the current `Self::new(core_providers, native_providers)` return. The `refresh_all()` at the end of `new()` will be called, plus we call it again — but that's fine since refresh is idempotent. Actually, `new()` already calls `refresh_all()`, so we should remove the duplicate. Let me adjust: + +The cleaner approach is to construct the manager via `Self::new()` which calls `refresh_all()`, then set the runtime fields and add providers, then call `refresh_all()` once more for the newly added runtime providers. Or better — add runtime providers to `core_providers` before calling `new()`: + +```rust + // Merge runtime providers into core providers + let mut all_core_providers = core_providers; + for provider in runtime_providers { + info!("Registered runtime provider: {}", provider.name()); + all_core_providers.push(provider); + } + + let mut manager = Self::new(all_core_providers, native_providers); + manager.runtimes = runtimes; + manager.runtime_type_ids = runtime_type_ids; + manager +``` + +This way `new()` handles the single `refresh_all()` call. + +- [ ] **Step 3: Remove allow(dead_code) from runtime_loader** + +In `crates/owlry-core/src/plugins/runtime_loader.rs`, remove `#![allow(dead_code)]` (line 13). + +Fix any resulting dead code warnings by removing unused `#[allow(dead_code)]` attributes on individual items that are now actually used, or adding targeted `#[allow(dead_code)]` only on truly unused items. + +- [ ] **Step 4: Build and test** + +Run: `cargo check -p owlry-core && cargo test -p owlry-core` +Expected: all pass. May see info logs about runtimes loading (if installed on the build machine). + +- [ ] **Step 5: Commit** + +```bash +git add crates/owlry-core/src/providers/mod.rs \ + crates/owlry-core/src/plugins/runtime_loader.rs +git commit -m "feat: wire script runtime loading into daemon ProviderManager" +``` + +--- + +## Task 4: Filesystem watcher for hot-reload + +**Files:** +- Create: `crates/owlry-core/src/plugins/watcher.rs` +- Modify: `crates/owlry-core/src/plugins/mod.rs:23-28` (add module) +- Modify: `crates/owlry-core/src/providers/mod.rs` (add reload method) +- Modify: `crates/owlry-core/src/server.rs:59-78` (start watcher) +- Modify: `crates/owlry-core/Cargo.toml` (add deps) + +- [ ] **Step 1: Add dependencies** + +In `crates/owlry-core/Cargo.toml`, add to `[dependencies]`: + +```toml +# Filesystem watching for plugin hot-reload +notify = "7" +notify-debouncer-mini = "0.5" +``` + +- [ ] **Step 2: Add reload_runtimes method to ProviderManager** + +In `crates/owlry-core/src/providers/mod.rs`, add a method: + +```rust + /// Reload all script runtime providers (called by filesystem watcher) + pub fn reload_runtimes(&mut self) { + // Remove old runtime providers from the core providers list + self.providers.retain(|p| { + let type_str = format!("{}", p.provider_type()); + !self.runtime_type_ids.contains(&type_str) + }); + + // Drop old runtimes + self.runtimes.clear(); + self.runtime_type_ids.clear(); + + let owlry_version = env!("CARGO_PKG_VERSION"); + let plugins_dir = match crate::paths::plugins_dir() { + Some(d) => d, + None => return, + }; + + // Reload Lua runtime + match LoadedRuntime::load_lua(&plugins_dir, owlry_version) { + Ok(rt) => { + info!("Reloaded Lua runtime with {} provider(s)", rt.providers().len()); + for provider in rt.create_providers() { + let type_id = format!("{}", provider.provider_type()); + self.runtime_type_ids.insert(type_id); + self.providers.push(provider); + } + self.runtimes.push(rt); + } + Err(e) => { + info!("Lua runtime not available on reload: {}", e); + } + } + + // Reload Rune runtime + match LoadedRuntime::load_rune(&plugins_dir, owlry_version) { + Ok(rt) => { + info!("Reloaded Rune runtime with {} provider(s)", rt.providers().len()); + for provider in rt.create_providers() { + let type_id = format!("{}", provider.provider_type()); + self.runtime_type_ids.insert(type_id); + self.providers.push(provider); + } + self.runtimes.push(rt); + } + Err(e) => { + info!("Rune runtime not available on reload: {}", e); + } + } + + // Refresh the newly added providers + for provider in &mut self.providers { + provider.refresh(); + } + + info!("Runtime reload complete"); + } +``` + +- [ ] **Step 3: Create the watcher module** + +Create `crates/owlry-core/src/plugins/watcher.rs`: + +```rust +//! Filesystem watcher for user plugin hot-reload +//! +//! Watches `~/.config/owlry/plugins/` for changes and triggers +//! runtime reload when plugin files are modified. + +use std::path::PathBuf; +use std::sync::{Arc, RwLock}; +use std::thread; +use std::time::Duration; + +use log::{info, warn}; +use notify_debouncer_mini::{DebouncedEventKind, new_debouncer}; + +use crate::providers::ProviderManager; + +/// Start watching the user plugins directory for changes. +/// +/// Spawns a background thread that monitors the directory and triggers +/// a full runtime reload on any file change. Returns immediately. +/// +/// If the plugins directory doesn't exist or the watcher fails to start, +/// logs a warning and returns without spawning a thread. +pub fn start_watching(pm: Arc>) { + let plugins_dir = match crate::paths::plugins_dir() { + Some(d) => d, + None => { + info!("No plugins directory configured, skipping file watcher"); + return; + } + }; + + if !plugins_dir.exists() { + // Create the directory so the watcher has something to watch + if std::fs::create_dir_all(&plugins_dir).is_err() { + warn!("Failed to create plugins directory: {}", plugins_dir.display()); + return; + } + } + + thread::spawn(move || { + if let Err(e) = watch_loop(&plugins_dir, &pm) { + warn!("Plugin watcher stopped: {}", e); + } + }); + + info!("Plugin file watcher started for {}", plugins_dir.display()); +} + +fn watch_loop( + plugins_dir: &PathBuf, + pm: &Arc>, +) -> Result<(), Box> { + let (tx, rx) = std::sync::mpsc::channel(); + + let mut debouncer = new_debouncer(Duration::from_millis(500), tx)?; + + debouncer + .watcher() + .watch(plugins_dir.as_ref(), notify::RecursiveMode::Recursive)?; + + info!("Watching {} for plugin changes", plugins_dir.display()); + + loop { + match rx.recv() { + Ok(Ok(events)) => { + // Check if any event is relevant (not just access/metadata) + let has_relevant_change = events.iter().any(|e| { + matches!(e.kind, DebouncedEventKind::Any | DebouncedEventKind::AnyContinuous) + }); + + if has_relevant_change { + info!("Plugin file change detected, reloading runtimes..."); + let mut pm_guard = pm.write().unwrap_or_else(|e| e.into_inner()); + pm_guard.reload_runtimes(); + } + } + Ok(Err(errors)) => { + for e in errors { + warn!("File watcher error: {}", e); + } + } + Err(e) => { + // Channel closed — watcher was dropped + return Err(Box::new(e)); + } + } + } +} +``` + +- [ ] **Step 4: Register the watcher module** + +In `crates/owlry-core/src/plugins/mod.rs`, add after line 28 (`pub mod runtime_loader;`): + +```rust +pub mod watcher; +``` + +- [ ] **Step 5: Start watcher in Server::run** + +In `crates/owlry-core/src/server.rs`, in the `run()` method, before the accept loop, add: + +```rust +pub fn run(&self) -> io::Result<()> { + // Start filesystem watcher for user plugin hot-reload + crate::plugins::watcher::start_watching(Arc::clone(&self.provider_manager)); + + info!("Server entering accept loop"); + for stream in self.listener.incoming() { +``` + +- [ ] **Step 6: Build and test** + +Run: `cargo check -p owlry-core && cargo test -p owlry-core` +Expected: all pass + +- [ ] **Step 7: Manual smoke test** + +```bash +# Start the daemon +RUST_LOG=info cargo run -p owlry-core + +# In another terminal, create a test plugin +mkdir -p ~/.config/owlry/plugins/hotreload-test +cat > ~/.config/owlry/plugins/hotreload-test/plugin.toml << 'EOF' +[plugin] +id = "hotreload-test" +name = "Hot Reload Test" +version = "0.1.0" +EOF +cat > ~/.config/owlry/plugins/hotreload-test/main.lua << 'EOF' +owlry.provider.register({ + name = "hotreload-test", + refresh = function() + return {{ id = "hr1", name = "Hot Reload Works!", command = "echo yes" }} + end, +}) +EOF + +# Watch daemon logs — should see "Plugin file change detected, reloading runtimes..." +# Clean up after testing +rm -rf ~/.config/owlry/plugins/hotreload-test +``` + +- [ ] **Step 8: Commit** + +```bash +git add crates/owlry-core/Cargo.toml \ + crates/owlry-core/src/plugins/watcher.rs \ + crates/owlry-core/src/plugins/mod.rs \ + crates/owlry-core/src/providers/mod.rs \ + crates/owlry-core/src/server.rs +git commit -m "feat: add filesystem watcher for automatic user plugin hot-reload" +``` + +--- + +## Task 5: Update plugin development documentation + +**Files:** +- Modify: `/home/cnachtigall/ssd/git/archive/owlibou/owlry-plugins/docs/PLUGIN_DEVELOPMENT.md` + +- [ ] **Step 1: Update Lua plugin section** + +In `docs/PLUGIN_DEVELOPMENT.md`, update the Lua Quick Start section (around line 101): + +Change `entry_point = "init.lua"` to `entry = "main.lua"` in the manifest example. + +Replace the Lua code example with the `owlry.provider.register()` API: + +```lua +owlry.provider.register({ + name = "myluaprovider", + display_name = "My Lua Provider", + type_id = "mylua", + default_icon = "application-x-executable", + prefix = ":mylua", + refresh = function() + return { + { id = "item-1", name = "Hello from Lua", command = "echo 'Hello Lua!'" }, + } + end, +}) +``` + +Remove `local owlry = require("owlry")` — the `owlry` table is pre-registered globally. + +- [ ] **Step 2: Update Rune plugin section** + +Update the Rune manifest example to use `entry = "main.rn"` instead of `entry_point = "main.rn"`. + +- [ ] **Step 3: Update manifest reference** + +In the Lua Plugin API manifest section (around line 325), change `entry_point` to `entry` and add a note: + +```toml +[plugin] +id = "my-plugin" +name = "My Plugin" +version = "1.0.0" +description = "Plugin description" +entry = "main.lua" # Default: main.lua (Lua) / main.rn (Rune) + # Alias: entry_point also accepted +owlry_version = ">=1.0.0" # Optional version constraint +``` + +- [ ] **Step 4: Add hot-reload documentation** + +Add a new section after "Best Practices" (before "Publishing to AUR"): + +```markdown +## Hot Reload + +User plugins in `~/.config/owlry/plugins/` are automatically reloaded when files change. +The daemon watches the plugins directory and reloads all script runtimes when any file +is created, modified, or deleted. No daemon restart is needed. + +**What triggers a reload:** +- Creating a new plugin directory with `plugin.toml` +- Editing a plugin's script files (`main.lua`, `main.rn`, etc.) +- Editing a plugin's `plugin.toml` +- Deleting a plugin directory + +**What does NOT trigger a reload:** +- Changes to native plugins (`.so` files) — these require a daemon restart +- Changes to runtime libraries in `/usr/lib/owlry/runtimes/` — daemon restart needed + +**Reload behavior:** +- All script runtimes (Lua, Rune) are fully reloaded +- Existing search results may briefly show stale data during reload +- Errors in plugins are logged but don't affect other plugins +``` + +- [ ] **Step 5: Update Lua provider functions section** + +Replace the bare `refresh()`/`query()` examples (around line 390) with the register API: + +```lua +-- Static provider: called once at startup and on reload +owlry.provider.register({ + name = "my-provider", + display_name = "My Provider", + prefix = ":my", + refresh = function() + return { + { id = "id1", name = "Item 1", command = "command1" }, + { id = "id2", name = "Item 2", command = "command2" }, + } + end, +}) + +-- Dynamic provider: called on each keystroke +owlry.provider.register({ + name = "my-search", + display_name = "My Search", + prefix = "?my", + query = function(q) + if q == "" then return {} end + return { + { id = "result", name = "Result for: " .. q, command = "echo " .. q }, + } + end, +}) +``` + +- [ ] **Step 6: Commit** + +```bash +cd /home/cnachtigall/ssd/git/archive/owlibou/owlry-plugins +git add docs/PLUGIN_DEVELOPMENT.md +git commit -m "docs: update plugin development guide for main.lua/rn defaults, register API, hot-reload" +``` + +--- + +## Task 6: Update hello-test plugin and clean up + +**Files:** +- Modify: `~/.config/owlry/plugins/hello-test/plugin.toml` +- Modify: `~/.config/owlry/plugins/hello-test/init.lua` → rename to `main.lua` + +This is a local-only task, not committed to either repo. + +- [ ] **Step 1: Update hello-test plugin** + +```bash +# Rename entry point +mv ~/.config/owlry/plugins/hello-test/init.lua ~/.config/owlry/plugins/hello-test/main.lua + +# Update manifest to use entry field +cat > ~/.config/owlry/plugins/hello-test/plugin.toml << 'EOF' +[plugin] +id = "hello-test" +name = "Hello Test" +version = "0.1.0" +description = "Minimal test plugin for verifying Lua runtime loading" +EOF +``` + +- [ ] **Step 2: End-to-end verification** + +```bash +# Rebuild and restart daemon +cargo build -p owlry-core +RUST_LOG=info cargo run -p owlry-core + +# Expected log output should include: +# - "Loaded Lua runtime with 1 provider(s)" (hello-test) +# - "Loaded Rune runtime with 1 provider(s)" (hyprshutdown) +# - "Plugin file watcher started for ..." +``` diff --git a/docs/superpowers/plans/2026-03-28-performance-hardening.md b/docs/superpowers/plans/2026-03-28-performance-hardening.md new file mode 100644 index 0000000..d528020 --- /dev/null +++ b/docs/superpowers/plans/2026-03-28-performance-hardening.md @@ -0,0 +1,1051 @@ +# Performance Hardening Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Eliminate UI startup delay and per-keystroke search lag in the Owlry launcher. + +**Architecture:** Six targeted fixes across two crates (`owlry` client, `owlry-core` daemon). The two critical fixes move blocking IPC off the GTK main thread and defer the initial query until after the window is visible. The remaining four are low-risk surgical optimizations to config loading, frecency scoring, result list updates, and the periodic refresh timer. + +**Tech Stack:** Rust 1.90+, GTK4 0.10 (glib channels), owlry-core IPC + +--- + +## File Map + +| File | Action | Responsibility | +|------|--------|----------------| +| `crates/owlry/src/backend.rs` | Modify | Add `SearchBackend::search_async()` method using `std::thread` + `glib::MainContext::channel()` | +| `crates/owlry/src/ui/main_window.rs` | Modify | Use async search in debounce handler; defer `update_results` to after `present()`; use `lazy_state.displayed_count` in `scroll_to_row`; remove redundant `results.clone()` | +| `crates/owlry/src/app.rs` | Modify | Move `MainWindow::new()` result population to post-`present()` idle callback | +| `crates/owlry-core/src/config/mod.rs` | Modify | Replace `which` subprocess calls with in-process PATH lookup | +| `crates/owlry-core/src/data/frecency.rs` | Modify | Add `get_score_at()` accepting a pre-sampled timestamp | +| `crates/owlry-core/src/providers/mod.rs` | Modify | Sample `Utc::now()` once before scoring loop; use `get_score_at()` | + +--- + +### Task 1: Replace `which` subprocesses with in-process PATH lookup + +The `command_exists()` function in config spawns a `which` subprocess for every terminal candidate. On a cold cache this adds 200-500ms to startup. Replace it with a pure-Rust PATH scan. + +**Files:** +- Modify: `crates/owlry-core/src/config/mod.rs:525-532` +- Test: `crates/owlry-core/src/config/mod.rs` (existing inline tests) + +- [ ] **Step 1: Write test for the new `command_exists` implementation** + +Add at the bottom of the `#[cfg(test)]` block in `config/mod.rs`: + +```rust +#[test] +fn command_exists_finds_sh() { + // /bin/sh exists on every Unix system + assert!(super::command_exists("sh")); +} + +#[test] +fn command_exists_rejects_nonexistent() { + assert!(!super::command_exists("owlry_nonexistent_binary_abc123")); +} +``` + +- [ ] **Step 2: Run test to verify it passes with the current `which`-based implementation** + +Run: `cargo test -p owlry-core command_exists` + +Expected: Both tests PASS (the current implementation works, just slowly). + +- [ ] **Step 3: Replace `command_exists` with in-process PATH scan** + +Replace lines 525-532 of `crates/owlry-core/src/config/mod.rs`: + +```rust +/// Check if a command exists in PATH +fn command_exists(cmd: &str) -> bool { + Command::new("which") + .arg(cmd) + .output() + .map(|o| o.status.success()) + .unwrap_or(false) +} +``` + +With: + +```rust +/// Check if a command exists in PATH (in-process, no subprocess spawning) +fn command_exists(cmd: &str) -> bool { + std::env::var_os("PATH") + .map(|paths| { + std::env::split_paths(&paths).any(|dir| { + let full = dir.join(cmd); + full.is_file() + }) + }) + .unwrap_or(false) +} +``` + +- [ ] **Step 4: Run tests to verify the new implementation works** + +Run: `cargo test -p owlry-core command_exists` + +Expected: Both tests PASS. + +- [ ] **Step 5: Run full check** + +Run: `cargo check -p owlry-core` + +Expected: No errors or warnings. + +- [ ] **Step 6: Commit** + +```bash +git add crates/owlry-core/src/config/mod.rs +git commit -m "perf(config): replace which subprocesses with in-process PATH scan + +detect_terminal() was spawning up to 17 'which' subprocesses sequentially +on every startup. Replace with std::env::split_paths + is_file() check. +Eliminates 200-500ms of fork+exec overhead on cold cache." +``` + +--- + +### Task 2: Defer initial `update_results("")` to after `window.present()` + +`MainWindow::new()` calls `update_results("")` at line 227, which does a synchronous blocking IPC round-trip before the window is even shown. Moving this to a post-`present()` idle callback makes the window appear immediately. + +**Files:** +- Modify: `crates/owlry/src/ui/main_window.rs:225-227` +- Modify: `crates/owlry/src/app.rs:108-137` + +- [ ] **Step 1: Remove `update_results("")` from `MainWindow::new()`** + +In `crates/owlry/src/ui/main_window.rs`, remove line 227 so that lines 225-230 become: + +```rust + main_window.setup_signals(); + main_window.setup_lazy_loading(); + + // Ensure search entry has focus when window is shown + main_window.search_entry.grab_focus(); +``` + +The `update_results("")` call (previously line 227) is removed entirely from the constructor. + +- [ ] **Step 2: Add a public method to trigger initial population** + +Add this method to `impl MainWindow` (after `update_results`): + +```rust + /// Schedule initial results population via idle callback. + /// Call this AFTER `window.present()` so the window appears immediately. + pub fn schedule_initial_results(&self) { + let backend = self.backend.clone(); + let results_list = self.results_list.clone(); + let config = self.config.clone(); + let filter = self.filter.clone(); + let current_results = self.current_results.clone(); + let lazy_state = self.lazy_state.clone(); + + gtk4::glib::idle_add_local_once(move || { + let cfg = config.borrow(); + let max_results = cfg.general.max_results; + drop(cfg); + + let results = backend.borrow_mut().search( + "", + max_results, + &filter.borrow(), + &config.borrow(), + ); + + // Clear existing results + while let Some(child) = results_list.first_child() { + results_list.remove(&child); + } + + let initial_count = INITIAL_RESULTS.min(results.len()); + { + let mut lazy = lazy_state.borrow_mut(); + lazy.all_results = results.clone(); + lazy.displayed_count = initial_count; + } + + for item in results.iter().take(initial_count) { + let row = ResultRow::new(item); + results_list.append(&row); + } + + if let Some(first_row) = results_list.row_at_index(0) { + results_list.select_row(Some(&first_row)); + } + + *current_results.borrow_mut() = + results.into_iter().take(initial_count).collect(); + }); + } +``` + +- [ ] **Step 3: Call `schedule_initial_results()` after `present()` in `app.rs`** + +In `crates/owlry/src/app.rs`, change lines 108-137 from: + +```rust + let window = MainWindow::new( + app, + config.clone(), + backend.clone(), + filter.clone(), + args.prompt.clone(), + ); + + // Set up layer shell for Wayland overlay behavior + window.init_layer_shell(); + window.set_layer(Layer::Overlay); + window.set_keyboard_mode(gtk4_layer_shell::KeyboardMode::Exclusive); + + // Anchor to all edges for centered overlay effect + // We'll use margins to control the actual size + window.set_anchor(Edge::Top, true); + window.set_anchor(Edge::Bottom, false); + window.set_anchor(Edge::Left, false); + window.set_anchor(Edge::Right, false); + + // Position from top + window.set_margin(Edge::Top, 200); + + // Set up icon theme fallbacks + Self::setup_icon_theme(); + + // Load CSS styling with config for theming + Self::load_css(&config.borrow()); + + window.present(); +``` + +To: + +```rust + let window = MainWindow::new( + app, + config.clone(), + backend.clone(), + filter.clone(), + args.prompt.clone(), + ); + + // Set up layer shell for Wayland overlay behavior + window.init_layer_shell(); + window.set_layer(Layer::Overlay); + window.set_keyboard_mode(gtk4_layer_shell::KeyboardMode::Exclusive); + + // Anchor to all edges for centered overlay effect + // We'll use margins to control the actual size + window.set_anchor(Edge::Top, true); + window.set_anchor(Edge::Bottom, false); + window.set_anchor(Edge::Left, false); + window.set_anchor(Edge::Right, false); + + // Position from top + window.set_margin(Edge::Top, 200); + + // Set up icon theme fallbacks + Self::setup_icon_theme(); + + // Load CSS styling with config for theming + Self::load_css(&config.borrow()); + + window.present(); + + // Populate results AFTER present() so the window appears immediately + window.schedule_initial_results(); +``` + +- [ ] **Step 4: Verify it compiles** + +Run: `cargo check -p owlry` + +Expected: No errors. + +- [ ] **Step 5: Commit** + +```bash +git add crates/owlry/src/ui/main_window.rs crates/owlry/src/app.rs +git commit -m "perf(ui): defer initial query to after window.present() + +update_results('') was called inside MainWindow::new(), blocking the +window from appearing until the daemon responded. Move it to a +glib::idle_add_local_once callback scheduled after present() so the +window renders immediately." +``` + +--- + +### Task 3: Move search IPC off the GTK main thread + +This is the most impactful fix. Currently the debounce handler calls `backend.borrow_mut().search_with_tag()` synchronously on the GTK main thread, freezing the UI for the entire IPC round-trip. The fix uses `std::thread::spawn` to run the query on a background thread and `glib::MainContext::channel()` to post results back. + +**Files:** +- Modify: `crates/owlry/src/backend.rs` +- Modify: `crates/owlry/src/ui/main_window.rs:51,53-77,575-728,1186-1223` + +- [ ] **Step 1: Add `Send`-safe query parameters struct to `backend.rs`** + +Add at the top of `crates/owlry/src/backend.rs`, after the existing imports: + +```rust +use std::sync::{Arc, Mutex}; + +/// Parameters needed to run a search query on a background thread. +/// Extracted from Rc> state so it can cross thread boundaries. +pub struct QueryParams { + pub query: String, + pub max_results: usize, + pub modes: Option>, + pub tag_filter: Option, +} + +/// Result of an async search, sent back to the main thread. +pub struct QueryResult { + /// The query string that produced these results (for staleness detection). + pub query: String, + pub items: Vec, +} +``` + +- [ ] **Step 2: Add `DaemonHandle` type for thread-safe async queries** + +Add below the `QueryResult` struct in `crates/owlry/src/backend.rs`: + +```rust +/// Thread-safe handle to the daemon IPC connection. +/// Wraps the CoreClient in Arc> so it can be sent to a worker thread. +pub struct DaemonHandle { + client: Arc>, +} + +impl DaemonHandle { + pub fn new(client: CoreClient) -> Self { + Self { + client: Arc::new(Mutex::new(client)), + } + } + + /// Run a query on a background thread. Results are sent via the callback + /// on the glib main context (GTK main thread). + pub fn query_async(&self, params: QueryParams, on_done: F) + where + F: FnOnce(QueryResult) + Send + 'static, + { + let client = Arc::clone(&self.client); + let query_for_result = params.query.clone(); + + std::thread::spawn(move || { + let items = match client.lock() { + Ok(mut c) => { + let effective_query = if let Some(ref tag) = params.tag_filter { + format!(":tag:{} {}", tag, params.query) + } else { + params.query + }; + match c.query(&effective_query, params.modes) { + Ok(items) => items.into_iter().map(result_to_launch_item).collect(), + Err(e) => { + warn!("IPC query failed: {}", e); + Vec::new() + } + } + } + Err(e) => { + warn!("Failed to lock daemon client: {}", e); + Vec::new() + } + }; + + on_done(QueryResult { + query: query_for_result, + items, + }); + }); + } +} +``` + +- [ ] **Step 3: Update `SearchBackend` enum to use `DaemonHandle`** + +In `crates/owlry/src/backend.rs`, change the enum definition: + +```rust +pub enum SearchBackend { + /// Async IPC handle to owlry-core daemon + Daemon(DaemonHandle), + /// Direct local provider manager (dmenu mode only) + Local { + providers: Box, + frecency: FrecencyStore, + }, +} +``` + +- [ ] **Step 4: Add `build_modes_param` helper and `query_async` method to `SearchBackend`** + +Add these methods to `impl SearchBackend`, keeping the existing synchronous methods intact for local-mode use: + +```rust + /// Build the modes parameter for IPC from a ProviderFilter. + fn build_modes_param(filter: &ProviderFilter) -> Option> { + if filter.is_accept_all() { + None + } else { + let modes: Vec = filter + .enabled_providers() + .iter() + .map(|p| p.to_string()) + .collect(); + if modes.is_empty() { None } else { Some(modes) } + } + } + + /// Dispatch an async search query (daemon mode only). + /// For local mode, returns None — caller should use synchronous search. + pub fn query_async( + &self, + query: &str, + max_results: usize, + filter: &ProviderFilter, + config: &Config, + tag_filter: Option<&str>, + on_done: F, + ) -> bool + where + F: FnOnce(QueryResult) + Send + 'static, + { + match self { + SearchBackend::Daemon(handle) => { + let params = QueryParams { + query: query.to_string(), + max_results, + modes: Self::build_modes_param(filter), + tag_filter: tag_filter.map(|s| s.to_string()), + }; + handle.query_async(params, on_done); + true // async dispatched + } + SearchBackend::Local { .. } => false, // caller should use sync path + } + } +``` + +- [ ] **Step 5: Update existing synchronous methods to use `build_modes_param`** + +Refactor `search()` and `search_with_tag()` to use the shared helper. In the `Daemon` arms, replace the inline modes computation with `Self::build_modes_param(filter)`. The `Daemon` arm now needs to lock the client: + +```rust + pub fn search( + &mut self, + query: &str, + max_results: usize, + filter: &ProviderFilter, + config: &Config, + ) -> Vec { + match self { + SearchBackend::Daemon(handle) => { + let modes_param = Self::build_modes_param(filter); + match handle.client.lock() { + Ok(mut client) => match client.query(query, modes_param) { + Ok(items) => items.into_iter().map(result_to_launch_item).collect(), + Err(e) => { + warn!("IPC query failed: {}", e); + Vec::new() + } + }, + Err(e) => { + warn!("Failed to lock daemon client: {}", e); + Vec::new() + } + } + } + SearchBackend::Local { + providers, + frecency, + } => { + let frecency_weight = config.providers.frecency_weight; + let use_frecency = config.providers.frecency; + + if use_frecency { + providers + .search_with_frecency( + query, + max_results, + filter, + frecency, + frecency_weight, + None, + ) + .into_iter() + .map(|(item, _)| item) + .collect() + } else { + providers + .search_filtered(query, max_results, filter) + .into_iter() + .map(|(item, _)| item) + .collect() + } + } + } + } +``` + +Apply the same pattern to `search_with_tag()`, `record_launch()`, `query_submenu_actions()`, `execute_plugin_action()`, `available_provider_ids()` — each `Daemon` arm should use `handle.client.lock()` instead of accessing a bare `CoreClient`. Keep the logic identical, just wrap in `lock()`. + +- [ ] **Step 6: Update `app.rs` to construct `DaemonHandle`** + +In `crates/owlry/src/app.rs`, change line 72: + +```rust + SearchBackend::Daemon(client) +``` + +To: + +```rust + SearchBackend::Daemon(crate::backend::DaemonHandle::new(client)) +``` + +- [ ] **Step 7: Update debounce handler in `main_window.rs` to use async search** + +In `crates/owlry/src/ui/main_window.rs`, replace the debounce closure body (lines 682-724) with an async-aware version. The key change: instead of calling `backend.borrow_mut().search_with_tag()` synchronously and then updating the list, we attempt `query_async()` first and fall back to sync for local mode: + +```rust + let source_id = gtk4::glib::timeout_add_local_once( + std::time::Duration::from_millis(SEARCH_DEBOUNCE_MS), + move || { + // Clear the source ID since we're now executing + *debounce_source_for_closure.borrow_mut() = None; + + let cfg = config.borrow(); + let max_results = cfg.general.max_results; + drop(cfg); + + let query_str = parsed.query.clone(); + let tag = parsed.tag_filter.clone(); + + // Try async path (daemon mode) + let dispatched = { + let be = backend.borrow(); + let f = filter.borrow(); + let c = config.borrow(); + + // Clone what we need for the callback + let results_list_cb = results_list.clone(); + let current_results_cb = current_results.clone(); + let lazy_state_cb = lazy_state.clone(); + + be.query_async( + &query_str, + max_results, + &f, + &c, + tag.as_deref(), + move |result| { + // This closure runs on a background thread. + // Post UI update back to the main thread. + gtk4::glib::idle_add_local_once(move || { + // Clear existing results + while let Some(child) = results_list_cb.first_child() { + results_list_cb.remove(&child); + } + + let initial_count = + INITIAL_RESULTS.min(result.items.len()); + { + let mut lazy = lazy_state_cb.borrow_mut(); + lazy.all_results = result.items.clone(); + lazy.displayed_count = initial_count; + } + + for item in result.items.iter().take(initial_count) { + let row = ResultRow::new(item); + results_list_cb.append(&row); + } + + if let Some(first_row) = results_list_cb.row_at_index(0) + { + results_list_cb.select_row(Some(&first_row)); + } + + *current_results_cb.borrow_mut() = result + .items + .into_iter() + .take(initial_count) + .collect(); + }); + }, + ) + }; + + if !dispatched { + // Local mode (dmenu): synchronous search + let results = backend.borrow_mut().search_with_tag( + &query_str, + max_results, + &filter.borrow(), + &config.borrow(), + tag.as_deref(), + ); + + while let Some(child) = results_list.first_child() { + results_list.remove(&child); + } + + let initial_count = INITIAL_RESULTS.min(results.len()); + { + let mut lazy = lazy_state.borrow_mut(); + lazy.all_results = results.clone(); + lazy.displayed_count = initial_count; + } + + for item in results.iter().take(initial_count) { + let row = ResultRow::new(item); + results_list.append(&row); + } + + if let Some(first_row) = results_list.row_at_index(0) { + results_list.select_row(Some(&first_row)); + } + + *current_results.borrow_mut() = + results.into_iter().take(initial_count).collect(); + } + }, + ); +``` + +- [ ] **Step 8: Verify it compiles** + +Run: `cargo check -p owlry` + +Expected: No errors. There may be warnings about unused imports in `backend.rs` — fix those if they appear. + +- [ ] **Step 9: Run existing tests** + +Run: `cargo test -p owlry` + +Expected: All existing tests pass. + +- [ ] **Step 10: Commit** + +```bash +git add crates/owlry/src/backend.rs crates/owlry/src/ui/main_window.rs crates/owlry/src/app.rs +git commit -m "perf(ui): move search IPC off the GTK main thread + +Search queries in daemon mode now run on a background thread via +DaemonHandle::query_async(). Results are posted back to the main +thread via glib::idle_add_local_once. The GTK event loop is never +blocked by IPC, eliminating perceived input lag. + +Local mode (dmenu) continues to use synchronous search since it +has no IPC overhead." +``` + +--- + +### Task 4: Sample `Utc::now()` once per search instead of per-item + +`FrecencyStore::get_score()` calls `Utc::now()` inside `calculate_frecency()` on every item. With hundreds of items per query, that's hundreds of unnecessary syscalls. Sample the timestamp once and pass it in. + +**Files:** +- Modify: `crates/owlry-core/src/data/frecency.rs:125-153` +- Modify: `crates/owlry-core/src/providers/mod.rs:636,685` + +- [ ] **Step 1: Write test for the new `get_score_at` method** + +Add to the `#[cfg(test)]` block in `crates/owlry-core/src/data/frecency.rs`: + +```rust + #[test] + fn get_score_at_matches_get_score() { + let mut store = FrecencyStore { + data: FrecencyData { + version: 1, + entries: HashMap::new(), + }, + path: PathBuf::from("/dev/null"), + dirty: false, + }; + store.data.entries.insert( + "test".to_string(), + FrecencyEntry { + launch_count: 5, + last_launch: Utc::now(), + }, + ); + + let now = Utc::now(); + let score_at = store.get_score_at("test", now); + let score = store.get_score("test"); + + // Both should be very close (same timestamp, within rounding) + assert!((score_at - score).abs() < 1.0); + } +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `cargo test -p owlry-core get_score_at` + +Expected: FAIL — `get_score_at` method does not exist yet. + +- [ ] **Step 3: Add `get_score_at` and `calculate_frecency_at` methods** + +In `crates/owlry-core/src/data/frecency.rs`, add after the existing `get_score` method (line 132): + +```rust + /// Calculate frecency score using a pre-sampled timestamp. + /// Use this in hot loops to avoid repeated Utc::now() syscalls. + pub fn get_score_at(&self, item_id: &str, now: DateTime) -> f64 { + match self.data.entries.get(item_id) { + Some(entry) => Self::calculate_frecency_at(entry.launch_count, entry.last_launch, now), + None => 0.0, + } + } +``` + +Then add after `calculate_frecency` (line 154): + +```rust + /// Calculate frecency using a caller-provided timestamp. + fn calculate_frecency_at(launch_count: u32, last_launch: DateTime, now: DateTime) -> f64 { + let age = now.signed_duration_since(last_launch); + let age_days = age.num_hours() as f64 / 24.0; + + let recency_weight = if age_days < 1.0 { + 100.0 + } else if age_days < 7.0 { + 70.0 + } else if age_days < 30.0 { + 50.0 + } else if age_days < 90.0 { + 30.0 + } else { + 10.0 + }; + + launch_count as f64 * recency_weight + } +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `cargo test -p owlry-core get_score_at` + +Expected: PASS. + +- [ ] **Step 5: Update `search_with_frecency` to sample time once** + +In `crates/owlry-core/src/providers/mod.rs`, add `use chrono::Utc;` to the imports at the top of the file if not already present. + +Then in `search_with_frecency`, just before the `score_item` closure definition (around line 650), add: + +```rust + let now = Utc::now(); +``` + +Change the `score_item` closure's frecency line (line 685): + +```rust + let frecency_score = frecency.get_score(&item.id); +``` + +To: + +```rust + let frecency_score = frecency.get_score_at(&item.id, now); +``` + +Also change the empty-query frecency path (line 636): + +```rust + let frecency_score = frecency.get_score(&item.id); +``` + +To: + +```rust + let frecency_score = frecency.get_score_at(&item.id, now); +``` + +And add `let now = Utc::now();` before the empty-query block (around line 610) as well. + +- [ ] **Step 6: Run all tests** + +Run: `cargo test -p owlry-core` + +Expected: All tests pass. + +- [ ] **Step 7: Commit** + +```bash +git add crates/owlry-core/src/data/frecency.rs crates/owlry-core/src/providers/mod.rs +git commit -m "perf(core): sample Utc::now() once per search instead of per-item + +get_score() called Utc::now() inside calculate_frecency() for every +item in the search loop. Added get_score_at() that accepts a pre-sampled +timestamp. Eliminates hundreds of unnecessary clock_gettime syscalls +per keystroke." +``` + +--- + +### Task 5: Eliminate redundant `results.clone()` in search handlers + +Both `update_results()` and the debounce handler clone the full results Vec into `lazy_state.all_results` and then also consume it for `current_results`. The clone is unnecessary — we can split the Vec instead. + +**Files:** +- Modify: `crates/owlry/src/ui/main_window.rs:1186-1223` (`update_results`) +- Modify: `crates/owlry/src/ui/main_window.rs:703-723` (debounce handler) + +- [ ] **Step 1: Refactor `update_results` to avoid clone** + +In `crates/owlry/src/ui/main_window.rs`, replace the `update_results` method body (lines 1186-1223): + +```rust + fn update_results(&self, query: &str) { + let cfg = self.config.borrow(); + let max_results = cfg.general.max_results; + drop(cfg); + + let results = self.backend.borrow_mut().search( + query, + max_results, + &self.filter.borrow(), + &self.config.borrow(), + ); + + // Clear existing results + while let Some(child) = self.results_list.first_child() { + self.results_list.remove(&child); + } + + let initial_count = INITIAL_RESULTS.min(results.len()); + + // Display initial batch + for item in results.iter().take(initial_count) { + let row = ResultRow::new(item); + self.results_list.append(&row); + } + + if let Some(first_row) = self.results_list.row_at_index(0) { + self.results_list.select_row(Some(&first_row)); + } + + // Split: current_results gets the displayed slice, lazy gets everything + *self.current_results.borrow_mut() = results[..initial_count].to_vec(); + let mut lazy = self.lazy_state.borrow_mut(); + lazy.all_results = results; + lazy.displayed_count = initial_count; + } +``` + +- [ ] **Step 2: Apply the same pattern to the debounce handler's local-mode path** + +In the synchronous fallback path of the debounce handler (the `if !dispatched` block from Task 3), apply the same pattern — replace: + +```rust + let initial_count = INITIAL_RESULTS.min(results.len()); + { + let mut lazy = lazy_state.borrow_mut(); + lazy.all_results = results.clone(); + lazy.displayed_count = initial_count; + } + // ... + *current_results.borrow_mut() = + results.into_iter().take(initial_count).collect(); +``` + +With: + +```rust + let initial_count = INITIAL_RESULTS.min(results.len()); + + for item in results.iter().take(initial_count) { + let row = ResultRow::new(item); + results_list.append(&row); + } + + if let Some(first_row) = results_list.row_at_index(0) { + results_list.select_row(Some(&first_row)); + } + + *current_results.borrow_mut() = results[..initial_count].to_vec(); + let mut lazy = lazy_state.borrow_mut(); + lazy.all_results = results; + lazy.displayed_count = initial_count; +``` + +- [ ] **Step 2b: Apply the same pattern in the async callback (Task 3)** + +In the `on_done` callback within the debounce handler's async path, replace: + +```rust + { + let mut lazy = lazy_state_cb.borrow_mut(); + lazy.all_results = result.items.clone(); + lazy.displayed_count = initial_count; + } + // ... + *current_results_cb.borrow_mut() = result + .items + .into_iter() + .take(initial_count) + .collect(); +``` + +With: + +```rust + *current_results_cb.borrow_mut() = + result.items[..initial_count].to_vec(); + let mut lazy = lazy_state_cb.borrow_mut(); + lazy.all_results = result.items; + lazy.displayed_count = initial_count; +``` + +- [ ] **Step 3: Apply the same pattern in `schedule_initial_results` (from Task 2)** + +In the `schedule_initial_results` method added in Task 2, use the same approach: + +```rust + *current_results.borrow_mut() = results[..initial_count].to_vec(); + let mut lazy = lazy_state.borrow_mut(); + lazy.all_results = results; + lazy.displayed_count = initial_count; +``` + +- [ ] **Step 4: Verify it compiles** + +Run: `cargo check -p owlry` + +Expected: No errors. + +- [ ] **Step 5: Commit** + +```bash +git add crates/owlry/src/ui/main_window.rs +git commit -m "perf(ui): eliminate redundant results.clone() in search handlers + +The full results Vec was cloned into lazy_state.all_results and then +separately consumed for current_results. Now we slice for current_results +and move the original into lazy_state, avoiding one full Vec allocation +per query." +``` + +--- + +### Task 6: Use `displayed_count` in `scroll_to_row` instead of child walk + +`scroll_to_row` walks all GTK children with `first_child()`/`next_sibling()` to count rows. The count is already tracked in `lazy_state.displayed_count`. + +**Files:** +- Modify: `crates/owlry/src/ui/main_window.rs:460-492` + +- [ ] **Step 1: Add `lazy_state` parameter to `scroll_to_row`** + +Change the method signature and body in `crates/owlry/src/ui/main_window.rs`: + +```rust + fn scroll_to_row( + scrolled: &ScrolledWindow, + results_list: &ListBox, + row: &ListBoxRow, + lazy_state: &Rc>, + ) { + let vadj = scrolled.vadjustment(); + + let row_index = row.index(); + if row_index < 0 { + return; + } + + let visible_height = vadj.page_size(); + let current_scroll = vadj.value(); + + let list_height = results_list.height() as f64; + let row_count = lazy_state.borrow().displayed_count.max(1) as f64; + + let row_height = list_height / row_count; + let row_top = row_index as f64 * row_height; + let row_bottom = row_top + row_height; + + if row_top < current_scroll { + vadj.set_value(row_top); + } else if row_bottom > current_scroll + visible_height { + vadj.set_value(row_bottom - visible_height); + } + } +``` + +- [ ] **Step 2: Update all call sites to pass `lazy_state`** + +In `setup_signals()`, the `lazy_state` Rc is already cloned into the key handler closure's scope. Update the `Key::Down` and `Key::Up` arms. Both currently call: + +```rust +Self::scroll_to_row(&scrolled, &results_list, &next_row); +``` + +These need to be updated to include the lazy_state. First, ensure `lazy_state` is cloned into the key controller closure. Find the block of clones before `key_controller.connect_key_pressed` and add `lazy_state` if not already there: + +```rust + let lazy_state_for_keys = self.lazy_state.clone(); +``` + +Then update the calls in `Key::Down` and `Key::Up`: + +```rust +// In Key::Down: +Self::scroll_to_row(&scrolled, &results_list, &next_row, &lazy_state_for_keys); + +// In Key::Up: +Self::scroll_to_row(&scrolled, &results_list, &prev_row, &lazy_state_for_keys); +``` + +- [ ] **Step 3: Verify it compiles** + +Run: `cargo check -p owlry` + +Expected: No errors. + +- [ ] **Step 4: Commit** + +```bash +git add crates/owlry/src/ui/main_window.rs +git commit -m "perf(ui): use tracked count in scroll_to_row instead of child walk + +scroll_to_row walked all GTK children via first_child/next_sibling +to count rows. The count is already available in LazyLoadState, so +use that directly. Eliminates O(n) widget traversal per arrow key." +``` + +--- + +## Execution Notes + +### Task dependency order + +Tasks 1, 4, and 6 are independent of each other and can be done in any order or in parallel. + +Task 2 should be done before Task 3 (Task 3's async code affects the same area). + +Task 5 depends on Task 3 being complete (it references the async handler's local-mode fallback). + +**Recommended execution order:** 1 → 4 → 6 → 2 → 3 → 5 + +### What's NOT in this plan + +These are real issues identified during analysis but are larger refactors that deserve their own plans: + +- **Rune VM-per-query overhead** (`owlry-rune`): Creating a new Rune VM on every `query_provider` call is wasteful but fixing it requires changes to the plugin runtime ABI. +- **Sequential plugin loading at daemon startup**: Parallelizing plugin discovery/init requires careful handling of the `ProviderManager` initialization order. +- **Hot-reload write lock blocking all queries**: The `reload_runtimes()` exclusive lock pattern needs architectural discussion about whether to use double-buffering or copy-on-write. +- **5-second periodic re-query timer**: Currently fires a full IPC search unconditionally. Could be replaced with a lightweight "has anything changed?" check, but requires a new IPC message type. diff --git a/docs/superpowers/plans/2026-03-29-performance-optimization.md b/docs/superpowers/plans/2026-03-29-performance-optimization.md new file mode 100644 index 0000000..351553f --- /dev/null +++ b/docs/superpowers/plans/2026-03-29-performance-optimization.md @@ -0,0 +1,922 @@ +# Performance Optimization Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Fix an unsound `unsafe` block, eliminate per-keystroke clone avalanches in the search path, and apply targeted I/O and algorithmic optimizations across both the `owlry` and `owlry-plugins` repos. + +**Architecture:** Nine tasks across two repos. Phase 1 removes unsound `unsafe` code. Phase 2 restructures the hot search path to score by reference and clone only the final top-N results, combined with partial-sort (`select_nth_unstable_by`) for O(n) selection. Phase 3 removes unnecessary blocking I/O and simplifies GTK list updates. Phase 4 applies minor algorithmic fixes. Phase 5 covers plugin-repo fixes (separate repo, separate branch). + +**Tech Stack:** Rust 1.90+, GTK4 4.12+, owlry-core, owlry-plugins + +--- + +## File Map + +| File | Action | Responsibility | +|------|--------|----------------| +| `crates/owlry-core/src/providers/native_provider.rs` | Modify | Remove `RwLock>`, eliminate `unsafe` block | +| `crates/owlry-core/src/providers/mod.rs` | Modify | Score-by-reference in `search_with_frecency`, partial sort, clone only top N | +| `crates/owlry-core/src/data/frecency.rs` | Modify | Remove auto-save in `record_launch` | +| `crates/owlry/src/ui/main_window.rs` | Modify | Replace child-removal loop with `remove_all()` | +| `crates/owlry-core/src/providers/application.rs` | Modify | Single-pass double-space cleanup | +| `crates/owlry-core/src/filter.rs` | Modify | Merge full and partial prefix arrays into single-pass lookup | + +--- + +## Phase 1: Safety & Correctness + +### Task 1: Remove unsound `unsafe` in NativeProvider::items() + +The `items()` implementation creates a raw pointer from an `RwLockReadGuard`, then drops the guard while returning a slice backed by that pointer. This is UB waiting to happen. The inner `RwLock` is unnecessary — `refresh()` takes `&mut self` (exclusive access guaranteed by the outer `Arc>`), and `items()` takes `&self`. Replace the `RwLock>` with a plain `Vec`. + +**Files:** +- Modify: `crates/owlry-core/src/providers/native_provider.rs` +- Test: `crates/owlry-core/src/providers/native_provider.rs` (existing tests) + +- [ ] **Step 1: Run existing tests to establish baseline** + +Run: `cargo test -p owlry-core` + +Expected: All tests PASS. + +- [ ] **Step 2: Replace `RwLock>` with `Vec` in the struct** + +In `crates/owlry-core/src/providers/native_provider.rs`, change the struct definition: + +```rust +pub struct NativeProvider { + plugin: Arc, + info: ProviderInfo, + handle: ProviderHandle, + items: Vec, +} +``` + +Update `new()` to initialize without RwLock: + +```rust +Self { + plugin, + info, + handle, + items: Vec::new(), +} +``` + +- [ ] **Step 3: Remove the `unsafe` block from `items()` — return `&self.items` directly** + +Replace the entire `Provider::items()` impl: + +```rust +fn items(&self) -> &[LaunchItem] { + &self.items +} +``` + +- [ ] **Step 4: Update `refresh()` to write directly to `self.items`** + +Replace the RwLock write in `refresh()`: + +```rust +// Was: *self.items.write().unwrap() = items; +self.items = items; +``` + +- [ ] **Step 5: Update `query()` to read from `self.items` directly** + +In the `query()` method, replace the RwLock read: + +```rust +// Was: return self.items.read().unwrap().clone(); +return self.items.clone(); +``` + +- [ ] **Step 6: Remove `use std::sync::RwLock;` (no longer needed, `Arc` still used for plugin)** + +Remove `RwLock` from the `use std::sync::{Arc, RwLock};` import: + +```rust +use std::sync::Arc; +``` + +- [ ] **Step 7: Run tests and check** + +Run: `cargo test -p owlry-core && cargo check -p owlry-core` + +Expected: All tests PASS, no warnings about unused imports. + +- [ ] **Step 8: Compile full workspace to verify no downstream breakage** + +Run: `cargo check --workspace` + +Expected: Clean compilation. + +- [ ] **Step 9: Commit** + +```bash +git add crates/owlry-core/src/providers/native_provider.rs +git commit -m "fix(native-provider): remove unsound unsafe in items() + +Replace RwLock> with plain Vec. The inner RwLock +was unnecessary — refresh() takes &mut self (exclusive access +guaranteed by the outer Arc>). The unsafe +block in items() dropped the RwLockReadGuard while returning a +slice backed by the raw pointer, creating a dangling reference." +``` + +--- + +## Phase 2: Hot Path Optimization + +### Task 2: Eliminate clone avalanche in search_with_frecency + +Currently every matching `LaunchItem` (5 Strings + Vec) is cloned during scoring, then the Vec is sorted and truncated to ~15 results — discarding 95%+ of clones. Refactor to score items by reference, partial-sort the lightweight `(&LaunchItem, i64)` tuples with `select_nth_unstable_by` (O(n) average), and clone only the top-N survivors. + +**Files:** +- Modify: `crates/owlry-core/src/providers/mod.rs` — `search_with_frecency` method (~lines 652-875) +- Test: existing tests in `crates/owlry-core/src/providers/mod.rs` + +- [ ] **Step 1: Run existing search tests to establish baseline** + +Run: `cargo test -p owlry-core search` + +Expected: All search-related tests PASS. + +- [ ] **Step 2: Refactor `score_item` closure to return `Option` instead of `Option<(LaunchItem, i64)>`** + +In `search_with_frecency`, change the closure at ~line 780 from: + +```rust +let score_item = |item: &LaunchItem| -> Option<(LaunchItem, i64)> { +``` + +To: + +```rust +let score_item = |item: &LaunchItem| -> Option { +``` + +And change the return from: + +```rust +base_score.map(|s| { + let frecency_score = frecency.get_score_at(&item.id, now); + let frecency_boost = (frecency_score * frecency_weight * 10.0) as i64; + let exact_match_boost = if item.name.eq_ignore_ascii_case(query) { + match &item.provider { + ProviderType::Application => 50_000, + _ => 30_000, + } + } else { + 0 + }; + (item.clone(), s + frecency_boost + exact_match_boost) +}) +``` + +To: + +```rust +base_score.map(|s| { + let frecency_score = frecency.get_score_at(&item.id, now); + let frecency_boost = (frecency_score * frecency_weight * 10.0) as i64; + let exact_match_boost = if item.name.eq_ignore_ascii_case(query) { + match &item.provider { + ProviderType::Application => 50_000, + _ => 30_000, + } + } else { + 0 + }; + s + frecency_boost + exact_match_boost +}) +``` + +- [ ] **Step 3: Replace the static-item scoring loops to collect references** + +Replace the scoring loops at ~lines 831-853: + +```rust +// Was: +// for provider in &self.providers { ... results.push(scored); } +// for provider in &self.static_native_providers { ... results.push(scored); } + +// Score static items by reference (no cloning) +let mut scored_refs: Vec<(&LaunchItem, i64)> = Vec::new(); + +for provider in &self.providers { + if !filter.is_active(provider.provider_type()) { + continue; + } + for item in provider.items() { + if let Some(score) = score_item(item) { + scored_refs.push((item, score)); + } + } +} + +for provider in &self.static_native_providers { + if !filter.is_active(provider.provider_type()) { + continue; + } + for item in provider.items() { + if let Some(score) = score_item(item) { + scored_refs.push((item, score)); + } + } +} + +// Partial sort: O(n) average to find top max_results, then O(k log k) to order them +if scored_refs.len() > max_results { + scored_refs.select_nth_unstable_by(max_results, |a, b| b.1.cmp(&a.1)); + scored_refs.truncate(max_results); +} +scored_refs.sort_by(|a, b| b.1.cmp(&a.1)); + +// Clone only the survivors +results.extend(scored_refs.into_iter().map(|(item, score)| (item.clone(), score))); +``` + +- [ ] **Step 4: Add final merge-sort for dynamic + static results** + +After extending results, add the final sort (dynamic results from earlier in the function + the newly added static results need unified ordering): + +```rust +// Final sort merges dynamic results with static top-N +results.sort_by(|a, b| b.1.cmp(&a.1)); +results.truncate(max_results); +``` + +This replaces the existing `results.sort_by(...)` and `results.truncate(...)` lines at ~854-855 — the logic is the same, just confirming it's still present after the refactor. + +- [ ] **Step 5: Optimize the empty-query path to score by reference too** + +Replace the empty-query block at ~lines 739-776. Change: + +```rust +let core_items = self + .providers + .iter() + .filter(|p| filter.is_active(p.provider_type())) + .flat_map(|p| p.items().iter().cloned()); + +let native_items = self + .static_native_providers + .iter() + .filter(|p| filter.is_active(p.provider_type())) + .flat_map(|p| p.items().iter().cloned()); + +let items: Vec<(LaunchItem, i64)> = core_items + .chain(native_items) + .filter(|item| { + if let Some(tag) = tag_filter { + item.tags.iter().any(|t| t.to_lowercase().contains(tag)) + } else { + true + } + }) + .map(|item| { + let frecency_score = frecency.get_score_at(&item.id, now); + let boosted = (frecency_score * frecency_weight * 100.0) as i64; + (item, boosted) + }) + .collect(); + +results.extend(items); +results.sort_by(|a, b| b.1.cmp(&a.1)); +results.truncate(max_results); +return results; +``` + +With: + +```rust +let mut scored_refs: Vec<(&LaunchItem, i64)> = self + .providers + .iter() + .filter(|p| filter.is_active(p.provider_type())) + .flat_map(|p| p.items().iter()) + .chain( + self.static_native_providers + .iter() + .filter(|p| filter.is_active(p.provider_type())) + .flat_map(|p| p.items().iter()), + ) + .filter(|item| { + if let Some(tag) = tag_filter { + item.tags.iter().any(|t| t.to_lowercase().contains(tag)) + } else { + true + } + }) + .map(|item| { + let frecency_score = frecency.get_score_at(&item.id, now); + let boosted = (frecency_score * frecency_weight * 100.0) as i64; + (item, boosted) + }) + .collect(); + +if scored_refs.len() > max_results { + scored_refs.select_nth_unstable_by(max_results, |a, b| b.1.cmp(&a.1)); + scored_refs.truncate(max_results); +} +scored_refs.sort_by(|a, b| b.1.cmp(&a.1)); + +results.extend(scored_refs.into_iter().map(|(item, score)| (item.clone(), score))); +results.sort_by(|a, b| b.1.cmp(&a.1)); +results.truncate(max_results); +return results; +``` + +- [ ] **Step 6: Run tests** + +Run: `cargo test -p owlry-core` + +Expected: All tests PASS. Search results are identical (same items, same ordering). + +- [ ] **Step 7: Compile full workspace** + +Run: `cargo check --workspace` + +Expected: Clean compilation. + +- [ ] **Step 8: Commit** + +```bash +git add crates/owlry-core/src/providers/mod.rs +git commit -m "perf(search): score by reference, clone only top-N results + +Refactor search_with_frecency to score static provider items by +reference (&LaunchItem, i64) instead of cloning every match. +Use select_nth_unstable_by for O(n) partial sort, then clone +only the max_results survivors. Reduces clones from O(total_matches) +to O(max_results) — typically from hundreds to ~15." +``` + +--- + +## Phase 3: I/O Optimization + +### Task 3: Remove frecency auto-save on every launch + +`record_launch` calls `self.save()` synchronously — serializing JSON and writing to disk on every item launch. The `Drop` impl already saves on shutdown. Mark dirty and let the caller (or shutdown) handle persistence. + +**Files:** +- Modify: `crates/owlry-core/src/data/frecency.rs:98-123` +- Test: `crates/owlry-core/src/data/frecency.rs` (existing + new) + +- [ ] **Step 1: Write test verifying record_launch sets dirty without saving** + +Add to the `#[cfg(test)]` block in `frecency.rs`: + +```rust +#[test] +fn record_launch_sets_dirty_without_saving() { + let mut store = FrecencyStore { + data: FrecencyData::default(), + path: PathBuf::from("/dev/null"), + dirty: false, + }; + + store.record_launch("test-item"); + + assert!(store.dirty, "record_launch should set dirty flag"); + assert_eq!(store.data.entries["test-item"].launch_count, 1); +} +``` + +- [ ] **Step 2: Run test to verify it fails (current code auto-saves, clearing dirty)** + +Run: `cargo test -p owlry-core record_launch_sets_dirty` + +Expected: FAIL — `store.dirty` is `false` because `save()` clears it. + +- [ ] **Step 3: Remove the auto-save from `record_launch`** + +In `crates/owlry-core/src/data/frecency.rs`, remove lines 119-122 from `record_launch`: + +```rust +// Remove this block: +// Auto-save after recording +if let Err(e) = self.save() { + warn!("Failed to save frecency data: {}", e); +} +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `cargo test -p owlry-core record_launch_sets_dirty` + +Expected: PASS. + +- [ ] **Step 5: Run all frecency tests** + +Run: `cargo test -p owlry-core frecency` + +Expected: All PASS. + +- [ ] **Step 6: Commit** + +```bash +git add crates/owlry-core/src/data/frecency.rs +git commit -m "perf(frecency): remove blocking auto-save on every launch + +record_launch no longer calls save() synchronously. The dirty flag +is set and the Drop impl flushes on shutdown. Removes a JSON +serialize + fs::write from the hot launch path." +``` + +--- + +### Task 4: GTK ListBox — replace child-removal loop with `remove_all()` + +The current code removes children one-by-one in a `while` loop, triggering layout invalidation per removal. GTK 4.12 provides `remove_all()` which batches the operation. + +**Files:** +- Modify: `crates/owlry/src/ui/main_window.rs` — two locations (~lines 705-707 and ~742-744) + +- [ ] **Step 1: Replace daemon-mode child removal loop** + +In the `spawn_future_local` async block (~line 705), replace: + +```rust +while let Some(child) = results_list_cb.first_child() { + results_list_cb.remove(&child); +} +``` + +With: + +```rust +results_list_cb.remove_all(); +``` + +- [ ] **Step 2: Replace local-mode (dmenu) child removal loop** + +In the synchronous (local/dmenu) branch (~line 742), replace: + +```rust +while let Some(child) = results_list.first_child() { + results_list.remove(&child); +} +``` + +With: + +```rust +results_list.remove_all(); +``` + +- [ ] **Step 3: Verify compilation** + +Run: `cargo check -p owlry` + +Expected: Clean compilation. `remove_all()` is available in gtk4 4.12+. + +- [ ] **Step 4: Commit** + +```bash +git add crates/owlry/src/ui/main_window.rs +git commit -m "perf(ui): use ListBox::remove_all() instead of per-child loop + +Replaces two while-loop child removal patterns with the batched +remove_all() method available since GTK 4.12. Avoids per-removal +layout invalidation." +``` + +--- + +## Phase 4: Minor Optimizations + +### Task 5: Single-pass double-space cleanup in application.rs + +The `clean_desktop_exec_field` function uses a `while contains(" ") { replace(" ", " ") }` loop — O(n²) on pathological input with repeated allocations. Replace with a single-pass char iterator. + +**Files:** +- Modify: `crates/owlry-core/src/providers/application.rs:60-64` +- Test: existing tests in `crates/owlry-core/src/providers/application.rs` + +- [ ] **Step 1: Add test for pathological input** + +Add to the existing `#[cfg(test)]` block: + +```rust +#[test] +fn test_clean_desktop_exec_collapses_spaces() { + assert_eq!(clean_desktop_exec_field("app --flag arg"), "app --flag arg"); + // Pathological: many consecutive spaces + let input = format!("app{}arg", " ".repeat(100)); + assert_eq!(clean_desktop_exec_field(&input), "app arg"); +} +``` + +- [ ] **Step 2: Run test to verify it passes with current implementation** + +Run: `cargo test -p owlry-core clean_desktop_exec` + +Expected: All PASS (current implementation works, just inefficiently). + +- [ ] **Step 3: Replace the while-loop with a single-pass approach** + +Replace lines 60-64 of `application.rs`: + +```rust +// Was: +// let mut cleaned = result.trim().to_string(); +// while cleaned.contains(" ") { +// cleaned = cleaned.replace(" ", " "); +// } +// cleaned + +let trimmed = result.trim(); +let mut cleaned = String::with_capacity(trimmed.len()); +let mut prev_space = false; +for c in trimmed.chars() { + if c == ' ' { + if !prev_space { + cleaned.push(' '); + } + prev_space = true; + } else { + cleaned.push(c); + prev_space = false; + } +} +cleaned +``` + +- [ ] **Step 4: Run all application tests** + +Run: `cargo test -p owlry-core clean_desktop_exec` + +Expected: All PASS including the new pathological test. + +- [ ] **Step 5: Commit** + +```bash +git add crates/owlry-core/src/providers/application.rs +git commit -m "perf(application): single-pass double-space collapse + +Replace while-contains-replace loop with a single-pass char +iterator. Eliminates O(n²) behavior and repeated allocations +on pathological desktop file Exec values." +``` + +--- + +### Task 6: Consolidate parse_query prefix matching into single pass + +`parse_query` maintains four separate arrays (core_prefixes, plugin_prefixes, partial_core, partial_plugin) with duplicated prefix strings, iterating them in sequence. Merge full and partial matching into a single array and a single loop per category. + +**Files:** +- Modify: `crates/owlry-core/src/filter.rs:202-406` +- Test: existing tests in `crates/owlry-core/src/filter.rs` + +- [ ] **Step 1: Run existing filter tests to establish baseline** + +Run: `cargo test -p owlry-core filter` + +Expected: All PASS. + +- [ ] **Step 2: Define a unified prefix entry struct and static arrays** + +At the top of `parse_query`, replace the four separate arrays with two unified arrays: + +```rust +pub fn parse_query(query: &str) -> ParsedQuery { + let trimmed = query.trim_start(); + + // Tag filter: ":tag:XXX query" — check first (unchanged) + if let Some(rest) = trimmed.strip_prefix(":tag:") { + if let Some(space_idx) = rest.find(' ') { + let tag = rest[..space_idx].to_lowercase(); + let query_part = rest[space_idx + 1..].to_string(); + #[cfg(feature = "dev-logging")] + debug!( + "[Filter] parse_query({:?}) -> tag={:?}, query={:?}", + query, tag, query_part + ); + return ParsedQuery { + prefix: None, + tag_filter: Some(tag), + query: query_part, + }; + } else { + let tag = rest.to_lowercase(); + return ParsedQuery { + prefix: None, + tag_filter: Some(tag), + query: String::new(), + }; + } + } + + // Core prefixes — each entry is tried as ":prefix " (full) and ":prefix" (partial) + const CORE_PREFIXES: &[(&str, fn() -> ProviderType)] = &[ + ("app", || ProviderType::Application), + ("apps", || ProviderType::Application), + ("cmd", || ProviderType::Command), + ("command", || ProviderType::Command), + ]; + + // Plugin prefixes — each entry maps to a plugin type_id + const PLUGIN_PREFIXES: &[(&str, &str)] = &[ + ("bm", "bookmarks"), + ("bookmark", "bookmarks"), + ("bookmarks", "bookmarks"), + ("calc", "calc"), + ("calculator", "calc"), + ("clip", "clipboard"), + ("clipboard", "clipboard"), + ("emoji", "emoji"), + ("emojis", "emoji"), + ("file", "filesearch"), + ("files", "filesearch"), + ("find", "filesearch"), + ("script", "scripts"), + ("scripts", "scripts"), + ("ssh", "ssh"), + ("sys", "system"), + ("system", "system"), + ("power", "system"), + ("uuctl", "uuctl"), + ("systemd", "uuctl"), + ("web", "websearch"), + ("search", "websearch"), + ("config", "config"), + ("settings", "config"), + ("conv", "conv"), + ("converter", "conv"), + ]; + + // Single-pass: try each core prefix as both full (":prefix query") and partial (":prefix") + for (name, make_provider) in CORE_PREFIXES { + let with_space = format!(":{} ", name); + if let Some(rest) = trimmed.strip_prefix(with_space.as_str()) { + let provider = make_provider(); + #[cfg(feature = "dev-logging")] + debug!( + "[Filter] parse_query({:?}) -> prefix={:?}, query={:?}", + query, provider, rest + ); + return ParsedQuery { + prefix: Some(provider), + tag_filter: None, + query: rest.to_string(), + }; + } + let exact = format!(":{}", name); + if trimmed == exact { + let provider = make_provider(); + #[cfg(feature = "dev-logging")] + debug!( + "[Filter] parse_query({:?}) -> partial prefix {:?}", + query, provider + ); + return ParsedQuery { + prefix: Some(provider), + tag_filter: None, + query: String::new(), + }; + } + } + + // Single-pass: try each plugin prefix as both full and partial + for (name, type_id) in PLUGIN_PREFIXES { + let with_space = format!(":{} ", name); + if let Some(rest) = trimmed.strip_prefix(with_space.as_str()) { + let provider = ProviderType::Plugin(type_id.to_string()); + #[cfg(feature = "dev-logging")] + debug!( + "[Filter] parse_query({:?}) -> prefix={:?}, query={:?}", + query, provider, rest + ); + return ParsedQuery { + prefix: Some(provider), + tag_filter: None, + query: rest.to_string(), + }; + } + let exact = format!(":{}", name); + if trimmed == exact { + let provider = ProviderType::Plugin(type_id.to_string()); + #[cfg(feature = "dev-logging")] + debug!( + "[Filter] parse_query({:?}) -> partial prefix {:?}", + query, provider + ); + return ParsedQuery { + prefix: Some(provider), + tag_filter: None, + query: String::new(), + }; + } + } + + // Dynamic plugin prefix fallback (unchanged) + if let Some(rest) = trimmed.strip_prefix(':') { + if let Some(space_idx) = rest.find(' ') { + let prefix_word = &rest[..space_idx]; + if !prefix_word.is_empty() + && prefix_word + .chars() + .all(|c| c.is_alphanumeric() || c == '-' || c == '_') + { + return ParsedQuery { + prefix: Some(ProviderType::Plugin(prefix_word.to_string())), + tag_filter: None, + query: rest[space_idx + 1..].to_string(), + }; + } + } else if !rest.is_empty() + && rest + .chars() + .all(|c| c.is_alphanumeric() || c == '-' || c == '_') + { + return ParsedQuery { + prefix: Some(ProviderType::Plugin(rest.to_string())), + tag_filter: None, + query: String::new(), + }; + } + } + + let result = ParsedQuery { + prefix: None, + tag_filter: None, + query: query.to_string(), + }; + + #[cfg(feature = "dev-logging")] + debug!( + "[Filter] parse_query({:?}) -> prefix={:?}, tag={:?}, query={:?}", + query, result.prefix, result.tag_filter, result.query + ); + + result +} +``` + +> **Note:** `CORE_PREFIXES` uses function pointers (`fn() -> ProviderType`) because `ProviderType::Application` and `ProviderType::Command` are fieldless variants that can be constructed in a const context via a trivial function. `PLUGIN_PREFIXES` stays as `(&str, &str)` because the `to_string()` allocation only happens once a prefix actually matches. + +- [ ] **Step 3: Run all filter tests** + +Run: `cargo test -p owlry-core filter` + +Expected: All existing tests PASS (behavior unchanged). + +- [ ] **Step 4: Run full check** + +Run: `cargo check -p owlry-core` + +Expected: Clean compilation. + +- [ ] **Step 5: Commit** + +```bash +git add crates/owlry-core/src/filter.rs +git commit -m "refactor(filter): consolidate parse_query prefix arrays + +Merge four separate prefix arrays (core full, plugin full, core +partial, plugin partial) into two arrays with a single loop each +that checks both full and partial match. Halves the data and +eliminates the duplicate iteration." +``` + +--- + +## Phase 5: Plugin Repo Fixes + +> **These tasks target `somegit.dev/Owlibou/owlry-plugins` — a separate repository.** +> Clone and branch separately. They can be done independently of Phases 1-4. + +### Task 7: Filesearch — add minimum query length threshold + +The filesearch plugin spawns an `fd` subprocess on every keystroke (after debounce). For short queries this is wasteful and returns too many results. Add a 3-character minimum before spawning. + +**Files:** +- Modify: `owlry-plugin-filesearch/src/lib.rs` — `query()` method +- Test: existing or new tests + +- [ ] **Step 1: Add early return in the `query` method** + +At the top of the `query` function (which receives the search text), add: + +```rust +fn query(&self, query: &str) -> Vec { + // Don't spawn fd for very short queries — too many results, too slow + if query.len() < 3 { + return Vec::new(); + } + // ... existing code ... +} +``` + +Find the correct method — it may be the `ProviderVTable::provider_query` path or a helper like `search_with_fd`. The guard should be placed at the earliest point before `Command::new("fd")` is invoked. + +- [ ] **Step 2: Verify compilation and test** + +Run: `cargo check -p owlry-plugin-filesearch` + +Expected: Clean compilation. + +- [ ] **Step 3: Commit** + +```bash +git add owlry-plugin-filesearch/src/lib.rs +git commit -m "perf(filesearch): skip fd subprocess for queries under 3 chars + +Avoids spawning a subprocess per keystroke when the user has +only typed 1-2 characters. Short queries return too many results +from fd and block the daemon's read lock." +``` + +--- + +### Task 8: Emoji plugin — avoid double clone on refresh + +The emoji provider's `refresh()` returns `state.items.to_vec().into()` which clones all ~400 `PluginItem` structs. The core's `NativeProvider::refresh()` then converts each to `LaunchItem` (another set of allocations). If the plugin API supports transferring ownership instead of cloning, use that. Otherwise, this is an API-level limitation. + +**Files:** +- Modify: `owlry-plugin-emoji/src/lib.rs` — `provider_refresh` function + +- [ ] **Step 1: Check if items can be drained instead of cloned** + +If `state.items` is a `Vec` that gets rebuilt on each refresh anyway, drain it: + +```rust +// Was: state.items.to_vec().into() +// If items are rebuilt each refresh: +std::mem::take(&mut state.items).into() +``` + +If `state.items` must be preserved between refreshes (because refresh is called multiple times and the items don't change), then the clone is necessary and this task is a no-op. Check the `refresh()` implementation to determine which case applies. + +- [ ] **Step 2: Verify and commit if applicable** + +Run: `cargo check -p owlry-plugin-emoji` + +--- + +### Task 9: Clipboard plugin — add caching for `cliphist list` + +The clipboard provider calls `cliphist list` synchronously on every refresh. If the daemon's periodic refresh timer triggers this, it blocks the RwLock. Add a simple staleness check — only re-run `cliphist list` if more than N seconds have elapsed since the last successful fetch. + +**Files:** +- Modify: `owlry-plugin-clipboard/src/lib.rs` + +- [ ] **Step 1: Add a `last_refresh` timestamp to the provider state** + +```rust +use std::time::Instant; + +struct ClipboardState { + items: Vec, + last_refresh: Option, +} +``` + +- [ ] **Step 2: Guard the subprocess call with a staleness check** + +In the `refresh()` path: + +```rust +const REFRESH_INTERVAL: Duration = Duration::from_secs(5); + +if let Some(last) = state.last_refresh { + if last.elapsed() < REFRESH_INTERVAL { + return state.items.clone().into(); + } +} + +// ... existing cliphist list call ... + +state.last_refresh = Some(Instant::now()); +``` + +- [ ] **Step 3: Verify and commit** + +Run: `cargo check -p owlry-plugin-clipboard` + +```bash +git commit -m "perf(clipboard): cache cliphist results for 5 seconds + +Avoids re-spawning cliphist list on every refresh cycle when the +previous results are still fresh." +``` + +--- + +## Summary + +| Task | Impact | Repo | Risk | +|------|--------|------|------| +| 1. Remove unsafe in NativeProvider | CRITICAL (soundness) | owlry | Low — drops unnecessary RwLock | +| 2. Score-by-ref + partial sort | HIGH (keystroke perf) | owlry | Medium — touches hot path, verify with tests | +| 3. Remove frecency auto-save | MEDIUM (launch perf) | owlry | Low — Drop impl already saves | +| 4. ListBox remove_all() | MEDIUM (UI smoothness) | owlry | Low — direct GTK API replacement | +| 5. Single-pass space collapse | LOW (startup) | owlry | Low — purely algorithmic | +| 6. Consolidate parse_query | LOW (keystroke) | owlry | Low — existing tests cover behavior | +| 7. Filesearch min query length | HIGH (keystroke perf) | plugins | Low — early return guard | +| 8. Emoji refresh optimization | MEDIUM (startup) | plugins | Low — depends on API check | +| 9. Clipboard caching | MEDIUM (refresh perf) | plugins | Low — simple staleness check | diff --git a/docs/superpowers/specs/2026-03-26-codebase-hardening-design.md b/docs/superpowers/specs/2026-03-26-codebase-hardening-design.md new file mode 100644 index 0000000..aa585a2 --- /dev/null +++ b/docs/superpowers/specs/2026-03-26-codebase-hardening-design.md @@ -0,0 +1,142 @@ +# Codebase Hardening: owlry + owlry-plugins + +**Date:** 2026-03-26 +**Scope:** 15 fixes across 2 repositories, organized in 5 severity tiers +**Approach:** Severity-ordered tiers, one commit per tier, core repo first + +--- + +## Tier 1: Critical / Soundness (owlry core) + +### 1a. Replace `static mut HOST_API` with `OnceLock` + +**File:** `crates/owlry-plugin-api/src/lib.rs` +**Problem:** `static mut` is unsound — concurrent reads during initialization are UB. +**Fix:** Replace with `std::sync::OnceLock<&'static HostAPI>`. `init_host_api()` calls `HOST_API.set(api)`, `host_api()` calls `HOST_API.get().copied()`. No public API changes — convenience wrappers (`notify()`, `log_info()`, etc.) keep working. No ABI impact since `HOST_API` is internal. + +### 1b. Add IPC message size limit + +**File:** `crates/owlry-core/src/server.rs` +**Problem:** `BufReader::lines()` reads unbounded lines. A malicious/buggy client can OOM the daemon. +**Fix:** Replace the `lines()` iterator with a manual `read_line()` loop enforcing a 1 MB max. Lines exceeding the limit get an error response and the connection is dropped. Constant: `const MAX_REQUEST_SIZE: usize = 1_048_576`. + +### 1c. Handle mutex poisoning gracefully + +**File:** `crates/owlry-core/src/server.rs` +**Problem:** All `lock().unwrap()` calls panic on poisoned mutex, crashing handler threads. +**Fix:** Replace with `lock().unwrap_or_else(|e| e.into_inner())`. The ProviderManager and FrecencyStore don't have invariants that require abort-on-poison. + +--- + +## Tier 2: Security (owlry core) + +### 2a. Set socket permissions after bind + +**File:** `crates/owlry-core/src/server.rs` +**Problem:** Socket inherits process umask, may be readable by other local users. +**Fix:** After `UnixListener::bind()`, call `std::fs::set_permissions(socket_path, Permissions::from_mode(0o600))`. Uses `std::os::unix::fs::PermissionsExt`. + +### 2b. Log signal handler failure + +**File:** `crates/owlry-core/src/main.rs` +**Problem:** `ctrlc::set_handler(...).ok()` silently swallows errors. Failed handler means no socket cleanup on SIGINT. +**Fix:** Replace `.ok()` with `if let Err(e) = ... { warn!("...") }`. + +### 2c. Add client read timeout + +**File:** `crates/owlry-core/src/server.rs` +**Problem:** A client that connects but never sends data blocks a thread forever. +**Fix:** Set `stream.set_read_timeout(Some(Duration::from_secs(30)))` on accepted connections before entering the read loop. + +--- + +## Tier 3: Robustness / Quality (owlry core) + +### 3a. Log malformed JSON requests + +**File:** `crates/owlry-core/src/server.rs` +**Problem:** JSON parse errors only sent as response to client, not visible in daemon logs. +**Fix:** Add `warn!("Malformed request from client: {}", e)` before sending the error response. + +### 3b. Replace Mutex with RwLock for concurrent reads + +**File:** `crates/owlry-core/src/server.rs` +**Problem:** `Mutex` blocks all concurrent queries even though they're read-only. +**Fix:** Replace both `Arc>` and `Arc>` with `Arc>`. + +Lock usage per request type: + +| Request | ProviderManager | FrecencyStore | +|---------|----------------|---------------| +| Query | `read()` | `read()` | +| Launch | — | `write()` | +| Providers | `read()` | — | +| Refresh | `write()` | — | +| Toggle | — | — | +| Submenu | `read()` | — | +| PluginAction | `read()` | — | + +Poisoning recovery: `.unwrap_or_else(|e| e.into_inner())` applies to RwLock the same way. + +--- + +## Tier 4: Critical fixes (owlry-plugins) + +### 4a. Fix `Box::leak` memory leak in converter + +**File:** `owlry-plugins/crates/owlry-plugin-converter/src/units.rs` +**Problem:** `Box::leak(code.into_boxed_str())` leaks memory on every keystroke for currency queries. +**Fix:** Currency codes are already `&'static str` in `CURRENCY_ALIASES`. Change `resolve_currency_code()` return type from `Option` to `Option<&'static str>` so it returns the static str directly. This eliminates the `Box::leak`. Callers in `units.rs` (`find_unit`, `convert_currency`, `convert_currency_common`) and `currency.rs` (`is_currency_alias`) must be updated to work with `&'static str` — mostly removing `.to_string()` calls or adding them at the boundary where `String` is needed (e.g., HashMap lookups that need owned keys). + +### 4b. Fix bookmarks temp file race condition + +**File:** `owlry-plugins/crates/owlry-plugin-bookmarks/src/lib.rs` +**Problem:** Predictable `/tmp/owlry_places_temp.sqlite` path — concurrent instances clobber, symlink attacks possible. +**Fix:** Append PID and monotonic counter to filename: `owlry_places_{pid}.sqlite`. Uses `std::process::id()`. Each profile copy gets its own name via index. Cleanup on exit remains the same. + +### 4c. Fix bookmarks background refresh never updating state + +**File:** `owlry-plugins/crates/owlry-plugin-bookmarks/src/lib.rs` +**Problem:** Background thread loads items and saves cache but never writes back to `self.items`. Current session keeps stale data. +**Fix:** Replace `items: Vec` with `items: Arc>>`. Background thread writes to the shared vec after completing. `provider_refresh` reads from it. The `loading` AtomicBool already prevents concurrent loads. + +--- + +## Tier 5: Quality fixes (owlry-plugins) + +### 5a. SSH plugin: read terminal from config + +**File:** `owlry-plugins/crates/owlry-plugin-ssh/src/lib.rs` +**Problem:** Hardcoded `kitty` as terminal fallback. Core already detects terminals. +**Fix:** Read `terminal` from `[plugins.ssh]` in `~/.config/owlry/config.toml`. Fall back to `$TERMINAL` env var, then `xdg-terminal-exec`. Same config pattern as weather/pomodoro plugins. + +### 5b. WebSearch plugin: read engine from config + +**File:** `owlry-plugins/crates/owlry-plugin-websearch/src/lib.rs` +**Problem:** TODO comment for config reading, never implemented. Engine is always duckduckgo. +**Fix:** Read `engine` from `[plugins.websearch]` in config.toml. Supports named engines (`google`, `duckduckgo`, etc.) or custom URL templates with `{query}`. Falls back to duckduckgo. + +### 5c. Emoji plugin: build items once at init + +**File:** `owlry-plugins/crates/owlry-plugin-emoji/src/lib.rs` +**Problem:** `load_emojis()` clears and rebuilds ~370 items on every `refresh()` call. +**Fix:** Call `load_emojis()` in `EmojiState::new()`. `provider_refresh` returns `self.items.clone()` without rebuilding. + +### 5d. Calculator/Converter: safer shell commands + +**Files:** `owlry-plugin-calculator/src/lib.rs`, `owlry-plugin-converter/src/lib.rs` +**Problem:** `sh -c 'echo -n "..."'` pattern with double-quote interpolation. Theoretically breakable by unexpected result formatting. +**Fix:** Use `printf '%s' '...' | wl-copy` with single-quote escaping (`replace('\'', "'\\''")`) — the same safe pattern already used by bookmarks and clipboard plugins. + +--- + +## Out of scope + +These were identified but deferred: + +- **Hardcoded emoji list** — replacing with a crate/data file is a feature, not a fix +- **Plugin vtable-level tests** — valuable but a separate testing initiative +- **IPC protocol versioning** — protocol change, not a bug fix +- **Plugin sandbox enforcement** — large feature, not a point fix +- **Desktop Exec field sanitization** — deep rabbit hole, needs separate design +- **Config validation** — separate concern, deserves its own pass diff --git a/docs/superpowers/specs/2026-03-26-runtime-integration-design.md b/docs/superpowers/specs/2026-03-26-runtime-integration-design.md new file mode 100644 index 0000000..f31e78b --- /dev/null +++ b/docs/superpowers/specs/2026-03-26-runtime-integration-design.md @@ -0,0 +1,161 @@ +# Script Runtime Integration for owlry-core Daemon + +**Date:** 2026-03-26 +**Scope:** Wire up Lua/Rune script runtime loading in the daemon, fix ABI mismatch, add filesystem-watching hot-reload, update plugin documentation +**Repos:** owlry (core), owlry-plugins (docs only) + +--- + +## Problem + +The daemon (`owlry-core`) only loads native plugins from `/usr/lib/owlry/plugins/`. User script plugins in `~/.config/owlry/plugins/` are never discovered because `ProviderManager::new_with_config()` never calls the `LoadedRuntime` infrastructure that already exists in `runtime_loader.rs`. Both Lua and Rune runtimes are installed at `/usr/lib/owlry/runtimes/` and functional, but never invoked. + +Additionally, the Lua runtime's `RuntimeInfo` struct has 5 fields while the core expects 2, causing a SIGSEGV on cleanup. + +--- + +## 1. Fix Lua RuntimeInfo ABI mismatch + +**File:** `owlry/crates/owlry-lua/src/lib.rs` + +Shrink Lua's `RuntimeInfo` from 5 fields to 2, matching core and Rune: + +```rust +// Before (5 fields — ABI mismatch with core): +pub struct RuntimeInfo { + pub id: RString, + pub name: RString, + pub version: RString, + pub description: RString, + pub api_version: u32, +} + +// After (2 fields — matches core/Rune): +pub struct RuntimeInfo { + pub name: RString, + pub version: RString, +} +``` + +Update `runtime_info()` to return only 2 fields. Remove the `LUA_RUNTIME_API_VERSION` constant and `LuaRuntimeVTable` (use the core's `ScriptRuntimeVTable` layout — both already match). The extra metadata (`id`, `description`) was never consumed by the core. + +### Vtable `init` signature change + +Change the `init` function in the vtable to accept the owlry version as a second parameter: + +```rust +// Before: +pub init: extern "C" fn(plugins_dir: RStr<'_>) -> RuntimeHandle, + +// After: +pub init: extern "C" fn(plugins_dir: RStr<'_>, owlry_version: RStr<'_>) -> RuntimeHandle, +``` + +This applies to: +- `owlry-core/src/plugins/runtime_loader.rs` — `ScriptRuntimeVTable.init` +- `owlry-lua/src/lib.rs` — `LuaRuntimeVTable.init` and `runtime_init()` implementation +- `owlry-rune/src/lib.rs` — `RuneRuntimeVTable.init` and `runtime_init()` implementation + +The core passes its version (`env!("CARGO_PKG_VERSION")` from `owlry-core`) when calling `(vtable.init)(plugins_dir, version)`. Runtimes forward it to `discover_and_load()` instead of hardcoding a version string. This keeps compatibility checks future-proof — no code changes needed on version bumps. + +--- + +## 2. Change default entry points to `main` + +**Files:** +- `owlry/crates/owlry-lua/src/manifest.rs` — change `default_entry()` from `"init.lua"` to `"main.lua"` +- `owlry/crates/owlry-rune/src/manifest.rs` — change `default_entry()` from `"init.rn"` to `"main.rn"` + +Add `#[serde(alias = "entry_point")]` to the `entry` field in both manifests so existing `plugin.toml` files using `entry_point` continue to work. + +--- + +## 3. Wire runtime loading into ProviderManager + +**File:** `owlry/crates/owlry-core/src/providers/mod.rs` + +In `ProviderManager::new_with_config()`, after native plugin loading: + +1. Get user plugins directory from `paths::plugins_dir()` +2. Get owlry version: `env!("CARGO_PKG_VERSION")` +3. Try `LoadedRuntime::load_lua(&plugins_dir, version)` — log at `info!` if unavailable, not error +4. Try `LoadedRuntime::load_rune(&plugins_dir, version)` — same +5. Call `create_providers()` on each loaded runtime +6. Feed runtime providers into existing categorization (static/dynamic/widget) + +`LoadedRuntime::load_lua`, `load_rune`, and `load_from_path` all gain an `owlry_version: &str` parameter, which is passed to `(vtable.init)(plugins_dir, owlry_version)`. + +Store `LoadedRuntime` instances on `ProviderManager` in a new field `runtimes: Vec`. These must stay alive for the daemon's lifetime (they own the `Library` handle via `Arc`). + +Remove `#![allow(dead_code)]` from `runtime_loader.rs` since it's now used. + +--- + +## 4. Filesystem watcher for automatic hot-reload + +**New file:** `owlry/crates/owlry-core/src/plugins/watcher.rs` +**Modified:** `owlry/crates/owlry-core/src/providers/mod.rs`, `Cargo.toml` + +### Dependencies + +Add to `owlry-core/Cargo.toml`: +```toml +notify = "7" +notify-debouncer-mini = "0.5" +``` + +### Watcher design + +After initializing runtimes, spawn a background watcher thread: + +1. Watch `~/.config/owlry/plugins/` recursively using `notify-debouncer-mini` with 500ms debounce +2. On debounced event (any file create/modify/delete): + - Acquire write lock on `ProviderManager` + - Remove all runtime-backed providers from the provider vecs + - Drop old `LoadedRuntime` instances + - Re-load runtimes from `/usr/lib/owlry/runtimes/` with fresh plugin discovery + - Add new runtime providers to provider vecs + - Refresh the new providers + - Release write lock + +### Provider tracking + +`ProviderManager` needs to distinguish runtime providers from native/core providers for selective removal during reload. Options: + +- **Tag-based:** Runtime providers already use `ProviderType::Plugin(type_id)`. Keep a `HashSet` of type_ids that came from runtimes. On reload, remove providers whose type_id is in the set. +- **Separate storage:** Store runtime providers in their own vec, separate from native providers. Query merges results from both. + +**Chosen: Tag-based.** Simpler — runtime type_ids are tracked in a `runtime_type_ids: HashSet` on `ProviderManager`. Reload clears the set, removes matching providers, then re-adds. + +### Thread communication + +The watcher thread needs access to `Arc>`. The `Server` already holds this Arc. Pass a clone to the watcher thread at startup. The watcher acquires `write()` only during reload (~10ms), so read contention is minimal. + +### Watcher lifecycle + +- Started in `Server::run()` (or `Server::bind()`) before the accept loop +- Runs until the daemon exits (watcher thread is detached or joined on drop) +- Errors in the watcher (e.g., inotify limit exceeded) are logged and the watcher stops — daemon continues without hot-reload + +--- + +## 5. Plugin development documentation + +**File:** `owlry-plugins/docs/PLUGIN_DEVELOPMENT.md` + +Cover: +- **Plugin directory structure** — `~/.config/owlry/plugins//plugin.toml` + `main.lua`/`main.rn` +- **Manifest reference** — all `plugin.toml` fields (`id`, `name`, `version`, `description`, `entry`/`entry_point`, `owlry_version`, `[[providers]]` section, `[permissions]` section) +- **Lua plugin guide** — `owlry.provider.register()` API with `refresh` and `query` callbacks, item table format (`id`, `name`, `command`, `description`, `icon`, `terminal`, `tags`) +- **Rune plugin guide** — `pub fn refresh()` and `pub fn query(q)` signatures, `Item::new()` builder, `use owlry::Item` +- **Hot-reload** — changes are picked up automatically, no daemon restart needed +- **Examples** — complete working examples for both Lua and Rune + +--- + +## Out of scope + +- Config-gated runtime loading (runtimes self-skip if `.so` not installed) +- Per-plugin selective reload (full runtime reload is fast enough) +- Plugin registry/installation (already exists in the CLI) +- Sandbox enforcement (separate concern, deferred from hardening spec)