From bfbce42eab3d9881b384ab2b0f1c878af2f0d35d Mon Sep 17 00:00:00 2001 From: vikingowl Date: Wed, 13 May 2026 04:36:59 +0200 Subject: [PATCH] fix(ipc): thread active_prefix through Query so daemon honours it (Phase 3.4.5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The smoke test in Phase 3.4 surfaced a long-standing gap: parse_query already routes `:smoke foo` → Plugin("smoke") via the dynamic-prefix fallback (filter.rs:319-347), but the UI never told the daemon. The UI's filter.active_prefix was set locally yet build_modes_param only serialised filter.enabled, so the daemon rebuilt a fresh filter with no active_prefix and searched across every provider. Same gap also affected hardcoded plugin prefixes (`:emoji heart` etc.) — they only appeared to work because their items outranked others on fuzzy score. ipc.rs: - Request::Query gains optional `prefix: Option`, default None, skip_serializing_if = "Option::is_none". Wire-format omits the field when None so existing 2.0.x clients keep deserialising on a 2.1 daemon and vice-versa. filter.rs: - New test parse_query_routes_unknown_prefix_to_plugin_type_id locks in the existing dynamic-fallback behaviour for `:smoke item`. - New test build_prefix_param_round_trips_through_provider_type proves Display + FromStr agree on the wire format. client.rs: - `query(text, modes, prefix)` — third arg added; 2 existing tests updated. Two new tests with a capturing mock_server assert the prefix field is serialised when set and elided when None. backend.rs: - QueryParams grows a `prefix: Option`. New helper build_prefix_param(filter) -> Option mirrors set_prefix's active_prefix to the wire. All three client.query call sites updated (search, search_with_tag, query_async). server.rs: - Query handler destructures the new prefix field and applies it via filter.set_prefix(ProviderType::from_str(prefix)). Unknown ids fall through to ProviderType::Plugin(_) per the existing FromStr impl — symmetric with the UI's dynamic-fallback path. Live smoke (release build, isolated XDG + socket): - prefix=smoke + text="item" → 3 items, all smoke (was: 100, mixed) - prefix=smoke + text="one" → 1 item: "smoke one item" - prefix=smoke + text="" → 3 smoke items, frecency-sorted - prefix=app + text="firefox" → 1 firefox app item (regression check; hardcoded prefix path still works) 7 new tests (ipc:3, client:2, filter:2). 283/283 lib tests green with --features full. Both clippy configurations silent. Closes task #28. --- crates/owlry/src/backend.rs | 23 ++++++++-- crates/owlry/src/client.rs | 87 +++++++++++++++++++++++++++++++++++-- crates/owlry/src/filter.rs | 33 ++++++++++++++ crates/owlry/src/ipc.rs | 58 +++++++++++++++++++++++++ crates/owlry/src/server.rs | 17 +++++++- 5 files changed, 210 insertions(+), 8 deletions(-) diff --git a/crates/owlry/src/backend.rs b/crates/owlry/src/backend.rs index 042a16f..f0f9f5f 100644 --- a/crates/owlry/src/backend.rs +++ b/crates/owlry/src/backend.rs @@ -18,6 +18,11 @@ pub struct QueryParams { #[allow(dead_code)] pub max_results: usize, pub modes: Option>, + /// Provider-narrowing prefix (e.g. `"smoke"`, `"calc"`). Mirrors the + /// `prefix` field of [`crate::filter::ParsedQuery`] after `parse_query` + /// strips the `:foo ` from the entry. Daemon applies it as + /// `filter.set_prefix` before searching. + pub prefix: Option, pub tag_filter: Option, } @@ -62,7 +67,7 @@ impl DaemonHandle { } else { params.query }; - match c.query(&effective_query, params.modes) { + match c.query(&effective_query, params.modes, params.prefix) { Ok(items) => items.into_iter().map(result_to_launch_item).collect(), Err(e) => { warn!("IPC query failed: {}", e); @@ -115,6 +120,15 @@ impl SearchBackend { } } + /// Extract the active-prefix parameter for IPC. Mirrors the + /// `active_prefix` on the local filter so the daemon's filter can be + /// narrowed identically. `set_prefix` doesn't touch `accept_all` / + /// `enabled`, so without this the prefix would be silently lost in + /// translation (task #28). + fn build_prefix_param(filter: &ProviderFilter) -> Option { + filter.active_prefix().map(|p| p.to_string()) + } + /// Search for items matching the query. /// /// In daemon mode, sends query over IPC. The modes list is derived from @@ -131,8 +145,9 @@ impl SearchBackend { match self { SearchBackend::Daemon(handle) => { let modes_param = Self::build_modes_param(filter); + let prefix_param = Self::build_prefix_param(filter); match handle.client.lock() { - Ok(mut client) => match client.query(query, modes_param) { + Ok(mut client) => match client.query(query, modes_param, prefix_param) { Ok(items) => items.into_iter().map(result_to_launch_item).collect(), Err(e) => { warn!("IPC query failed: {}", e); @@ -194,8 +209,9 @@ impl SearchBackend { }; let modes_param = Self::build_modes_param(filter); + let prefix_param = Self::build_prefix_param(filter); match handle.client.lock() { - Ok(mut client) => match client.query(&effective_query, modes_param) { + Ok(mut client) => match client.query(&effective_query, modes_param, prefix_param) { Ok(items) => items.into_iter().map(result_to_launch_item).collect(), Err(e) => { warn!("IPC query failed: {}", e); @@ -255,6 +271,7 @@ impl SearchBackend { query: query.to_string(), max_results, modes: Self::build_modes_param(filter), + prefix: Self::build_prefix_param(filter), tag_filter: tag_filter.map(|s| s.to_string()), }; Some(handle.query_async(params)) diff --git a/crates/owlry/src/client.rs b/crates/owlry/src/client.rs index 3bd7da1..5462ab1 100644 --- a/crates/owlry/src/client.rs +++ b/crates/owlry/src/client.rs @@ -116,10 +116,21 @@ impl CoreClient { } /// Send a search query and return matching results. - pub fn query(&mut self, text: &str, modes: Option>) -> io::Result> { + /// + /// `prefix` (when `Some`) narrows the daemon-side search to a single + /// provider — typically the `prefix` field from + /// [`crate::filter::ProviderFilter::parse_query`]. Pass `None` for the + /// unrestricted case (the daemon falls back to `modes` or accept-all). + pub fn query( + &mut self, + text: &str, + modes: Option>, + prefix: Option, + ) -> io::Result> { self.send(&Request::Query { text: text.to_string(), modes, + prefix, })?; match self.receive()? { @@ -245,6 +256,38 @@ mod tests { static COUNTER: AtomicU32 = AtomicU32::new(0); + /// Spawn a mock server that accepts one connection, captures the request, + /// and replies with the given canned response. Returns the socket path and + /// a Receiver that will yield the request line after the test sends. + fn mock_server_capturing(response: Response) -> (PathBuf, std::sync::mpsc::Receiver) { + let n = COUNTER.fetch_add(1, Ordering::Relaxed); + let dir = std::env::temp_dir().join(format!("owlry-test-{}-{}", std::process::id(), n)); + let _ = std::fs::create_dir_all(&dir); + let sock = dir.join("test.sock"); + let _ = std::fs::remove_file(&sock); + + let listener = UnixListener::bind(&sock).expect("bind mock socket"); + let sock_clone = sock.clone(); + let (tx, rx) = std::sync::mpsc::channel(); + + thread::spawn(move || { + let (stream, _) = listener.accept().expect("accept"); + let mut reader = BufReader::new(stream.try_clone().unwrap()); + let mut writer = stream; + let mut line = String::new(); + reader.read_line(&mut line).expect("read request"); + let _ = tx.send(line); + let mut json = serde_json::to_string(&response).unwrap(); + json.push('\n'); + writer.write_all(json.as_bytes()).unwrap(); + writer.flush().unwrap(); + let _ = std::fs::remove_file(&sock_clone); + let _ = std::fs::remove_dir(dir); + }); + + (sock, rx) + } + /// Spawn a mock server that accepts one connection, reads one request, /// and replies with the given canned response. Each call gets a unique /// socket path to avoid collisions when tests run in parallel. @@ -303,7 +346,7 @@ mod tests { thread::sleep(Duration::from_millis(50)); let mut client = CoreClient::connect(&sock).expect("connect"); - let results = client.query("fire", None).expect("query"); + let results = client.query("fire", None, None).expect("query"); assert_eq!(results.len(), 1); assert_eq!(results[0].id, "firefox"); @@ -311,6 +354,44 @@ mod tests { assert_eq!(results[0].score, 100); } + #[test] + fn query_sends_prefix_field_in_request() { + let canned = Response::Results { items: vec![] }; + let (sock, rx) = mock_server_capturing(canned); + thread::sleep(Duration::from_millis(50)); + + let mut client = CoreClient::connect(&sock).expect("connect"); + let _ = client + .query("foo", None, Some("smoke".into())) + .expect("query"); + + let request_line = rx.recv_timeout(Duration::from_secs(2)).expect("captured"); + let parsed: serde_json::Value = serde_json::from_str(&request_line).expect("parse json"); + assert_eq!(parsed["type"], "query"); + assert_eq!(parsed["text"], "foo"); + assert_eq!( + parsed["prefix"], "smoke", + "prefix must be present in the request" + ); + } + + #[test] + fn query_without_prefix_omits_field_from_request() { + let canned = Response::Results { items: vec![] }; + let (sock, rx) = mock_server_capturing(canned); + thread::sleep(Duration::from_millis(50)); + + let mut client = CoreClient::connect(&sock).expect("connect"); + let _ = client.query("foo", None, None).expect("query"); + + let request_line = rx.recv_timeout(Duration::from_secs(2)).expect("captured"); + assert!( + !request_line.contains("prefix"), + "prefix field must be omitted when None; got: {}", + request_line + ); + } + #[test] fn toggle_returns_ack() { let sock = mock_server(Response::Ack); @@ -392,7 +473,7 @@ mod tests { thread::sleep(Duration::from_millis(50)); let mut client = CoreClient::connect(&sock).expect("connect"); - let err = client.query("test", None).unwrap_err(); + let err = client.query("test", None, None).unwrap_err(); let msg = err.to_string(); assert!( diff --git a/crates/owlry/src/filter.rs b/crates/owlry/src/filter.rs index 1c0d7a1..bfe702e 100644 --- a/crates/owlry/src/filter.rs +++ b/crates/owlry/src/filter.rs @@ -483,6 +483,39 @@ mod tests { assert_eq!(result.query, "5+3"); } + #[test] + fn parse_query_routes_unknown_prefix_to_plugin_type_id() { + // Locks in the dynamic-prefix-fallback path (filter.rs:319-347): a + // prefix not in the hardcoded core/plugin tables — e.g. a user + // provider's `:smoke` — must still produce Plugin("smoke"). This is + // what makes user-defined providers reachable by prefix. + let result = ProviderFilter::parse_query(":smoke item"); + assert_eq!( + result.prefix, + Some(ProviderType::Plugin("smoke".to_string())) + ); + assert_eq!(result.query, "item"); + + let bare = ProviderFilter::parse_query(":smoke"); + assert_eq!( + bare.prefix, + Some(ProviderType::Plugin("smoke".to_string())) + ); + assert_eq!(bare.query, ""); + } + + #[test] + fn build_prefix_param_round_trips_through_provider_type() { + // The wire format is ProviderType's Display impl; the daemon parses + // it back via FromStr. Verify both ends agree for a user provider. + use std::str::FromStr; + let plugin_smoke = ProviderType::Plugin("smoke".to_string()); + let wire = plugin_smoke.to_string(); + assert_eq!(wire, "smoke"); + let parsed = ProviderType::from_str(&wire).unwrap(); + assert_eq!(parsed, plugin_smoke); + } + #[test] fn test_toggle_ensures_one_enabled() { let mut filter = ProviderFilter::apps_only(); diff --git a/crates/owlry/src/ipc.rs b/crates/owlry/src/ipc.rs index c2ab33a..b7f2ade 100644 --- a/crates/owlry/src/ipc.rs +++ b/crates/owlry/src/ipc.rs @@ -7,6 +7,15 @@ pub enum Request { text: String, #[serde(skip_serializing_if = "Option::is_none")] modes: Option>, + /// Active prefix narrowing — the daemon restricts results to this + /// single provider. Carries the [`ProviderType`] as a string + /// (e.g. `"app"`, `"cmd"`, `"smoke"` for a user-defined provider + /// with id `"smoke"`). The UI populates this from + /// `ProviderFilter::parse_query`'s `prefix` field after stripping + /// the `:foo ` from the query text. Older clients omit the field; + /// the daemon treats `None` as "no narrowing". + #[serde(default, skip_serializing_if = "Option::is_none")] + prefix: Option, }, Launch { item_id: String, @@ -73,3 +82,52 @@ pub struct ProviderDesc { #[serde(default, skip_serializing_if = "Option::is_none")] pub search_noun: Option, } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn query_request_without_prefix_serializes_without_field() { + // Pre-3.4.5 clients didn't have the field; new schema must omit it + // from JSON when None so on-wire payloads stay identical. + let req = Request::Query { + text: "firefox".into(), + modes: None, + prefix: None, + }; + let json = serde_json::to_string(&req).unwrap(); + assert!(!json.contains("prefix"), "prefix must be omitted; got: {}", json); + assert!(json.contains(r#""type":"query""#)); + assert!(json.contains(r#""text":"firefox""#)); + } + + #[test] + fn query_request_with_prefix_serializes_and_roundtrips() { + let req = Request::Query { + text: "foo".into(), + modes: None, + prefix: Some("smoke".into()), + }; + let json = serde_json::to_string(&req).unwrap(); + assert!(json.contains(r#""prefix":"smoke""#)); + let back: Request = serde_json::from_str(&json).unwrap(); + assert_eq!(back, req); + } + + #[test] + fn legacy_clients_without_prefix_field_still_deserialize() { + // A 2.0.x client sends Query without the new prefix field. The 2.1 + // daemon must still accept it (default = None). + let json = r#"{"type":"query","text":"foo"}"#; + let req: Request = serde_json::from_str(json).unwrap(); + match req { + Request::Query { text, modes, prefix } => { + assert_eq!(text, "foo"); + assert!(modes.is_none()); + assert!(prefix.is_none()); + } + other => panic!("expected Query, got {:?}", other), + } + } +} diff --git a/crates/owlry/src/server.rs b/crates/owlry/src/server.rs index 6c8559b..47773b7 100644 --- a/crates/owlry/src/server.rs +++ b/crates/owlry/src/server.rs @@ -352,11 +352,24 @@ impl Server { config: &Arc>, ) -> Response { match request { - Request::Query { text, modes } => { - let filter = match modes { + Request::Query { + text, + modes, + prefix, + } => { + let mut filter = match modes { Some(m) => ProviderFilter::from_mode_strings(m), None => ProviderFilter::all(), }; + if let Some(prefix_str) = prefix { + // Mirrors UI-side parse_query → set_prefix. Tolerates + // unknown ids by emitting Plugin(prefix_str) — that's + // what the dynamic-prefix-fallback in parse_query does. + use std::str::FromStr; + if let Ok(prefix_type) = crate::providers::ProviderType::from_str(prefix_str) { + filter.set_prefix(Some(prefix_type)); + } + } let (max, weight) = { let cfg = match config.read() { Ok(g) => g,