From 0ba41d674c5f9b3f9815d30174635e4ce87cbe3c Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Thu, 1 Oct 2026 11:13:29 +0300 Subject: [PATCH] chore(obsidian): remove vault-registration detection from the library The obsidian_registry module and its `dirs` dependency have been removed because vault-registration detection is host desktop policy and belongs in the host application, not in the library. The `obsidian` feature now only stages the bundled `.obsidian/` defaults into the content root. Auto-committed-on: dragonfly Co-authored-by: Medulla --- Cargo.lock | 48 ----- Cargo.toml | 11 +- src/memory/store/content/mod.rs | 6 +- src/memory/store/content/obsidian_registry.rs | 167 ------------------ .../store/content/obsidian_registry/types.rs | 33 ---- .../store/content/obsidian_registry_tests.rs | 155 ---------------- 6 files changed, 6 insertions(+), 414 deletions(-) delete mode 100644 src/memory/store/content/obsidian_registry.rs delete mode 100644 src/memory/store/content/obsidian_registry/types.rs delete mode 100644 src/memory/store/content/obsidian_registry_tests.rs diff --git a/Cargo.lock b/Cargo.lock index 859ec14..c60b1ff 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -247,27 +247,6 @@ dependencies = [ "crypto-common 0.2.2", ] -[[package]] -name = "dirs" -version = "7.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8d57d423b3c82e89b9a24ca3091fee61f456a26edbd28d26c65906f4bc1dcd8f" -dependencies = [ - "dirs-sys", -] - -[[package]] -name = "dirs-sys" -version = "0.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e01a3366d27ee9890022452ee61b2b63a67e6f13f58900b651ff5665f0bb1fab" -dependencies = [ - "libc", - "option-ext", - "redox_users", - "windows-sys 0.61.2", -] - [[package]] name = "dispatch2" version = "0.3.1" @@ -866,15 +845,6 @@ dependencies = [ "pkg-config", ] -[[package]] -name = "libredox" -version = "0.1.19" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2026a5056764a10b2bf5d56488cba40da507f5493a6a429340e2004d9ed085fa" -dependencies = [ - "libc", -] - [[package]] name = "libsqlite3-sys" version = "0.38.2" @@ -1023,12 +993,6 @@ version = "1.21.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9f7c3e4beb33f85d45ae3e3a1792185706c8e16d043238c593331cc7cd313b50" -[[package]] -name = "option-ext" -version = "0.2.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "04744f49eae99ab78e0d5c0b603ab218f515ea8cfe5a456d7629ad883a3b6e7d" - [[package]] name = "parking_lot" version = "0.12.5" @@ -1194,17 +1158,6 @@ dependencies = [ "bitflags", ] -[[package]] -name = "redox_users" -version = "0.5.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a4e608c6638b9c18977b00b475ac1f28d14e84b27d8d42f70e0bf1e3dec127ac" -dependencies = [ - "getrandom 0.2.17", - "libredox", - "thiserror", -] - [[package]] name = "ref-cast" version = "1.0.25" @@ -1682,7 +1635,6 @@ dependencies = [ "async-trait", "block2", "chrono", - "dirs", "dotenvy", "futures", "git2", diff --git a/Cargo.toml b/Cargo.toml index e1c16d8..a7766b4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -80,11 +80,10 @@ persona = [] # `dep:reqwest` independently. wiki-git = ["dep:git2", "dep:hex"] -# Obsidian vault interop (`memory::store::content::{obsidian,obsidian_registry}`): -# stages the bundled `.obsidian/` defaults into the content root, and -# best-effort detection of whether that root is a vault Obsidian already knows -# about (its `obsidian.json` registry). -obsidian = ["dep:dirs"] +# Obsidian vault interop (`memory::store::content::obsidian`): stages the +# bundled `.obsidian/` defaults into the content root. Vault-registration +# detection is host desktop policy and lives in the host, not here. +obsidian = [] # Contact resolution and scoring (`memory::people`): a SQLite store of people, # handle aliases and interactions, a deterministic handle → `PersonId` resolver, @@ -156,8 +155,6 @@ reqwest = { version = "0.12", default-features = false, features = [ "stream", ], optional = true } tracing = { version = "0.1", optional = true } -# Per-OS config/home dir probing for the Obsidian vault registry (`obsidian`). -dirs = { version = "7", optional = true } # Hex-encodes summary read-pointer ids into git tag names (`wiki-git`). hex = { version = "0.4", optional = true } # tokio powers the optional background worker loops (`tokio` feature). It is diff --git a/src/memory/store/content/mod.rs b/src/memory/store/content/mod.rs index aabc380..5a724bf 100644 --- a/src/memory/store/content/mod.rs +++ b/src/memory/store/content/mod.rs @@ -13,8 +13,8 @@ //! - `read` — reads, SHA-256 verification, and front-matter splitting //! - `tags` — chunk-tag updates and Obsidian tag slugifiers //! - `raw` — verbatim per-item raw archive (`raw///…`) -//! - `obsidian` / `obsidian_registry` — Obsidian vault interop (`obsidian` -//! feature): stage bundled `.obsidian/` defaults, detect vault registration +//! - `obsidian` — Obsidian vault interop (`obsidian` feature): stage bundled +//! `.obsidian/` defaults //! - `wiki_git` — git-backed mirror of summary nodes (`wiki-git` feature) //! //! ## Deferred @@ -26,8 +26,6 @@ pub mod atomic; pub mod compose; #[cfg(feature = "obsidian")] pub mod obsidian; -#[cfg(feature = "obsidian")] -pub mod obsidian_registry; pub mod paths; pub mod raw; pub mod read; diff --git a/src/memory/store/content/obsidian_registry.rs b/src/memory/store/content/obsidian_registry.rs deleted file mode 100644 index ff9f49a..0000000 --- a/src/memory/store/content/obsidian_registry.rs +++ /dev/null @@ -1,167 +0,0 @@ -//! Obsidian vault-*registration* detection. -//! -//! Sibling to [`super::obsidian`] (which writes the `.obsidian/` *defaults* -//! into the content root). This module answers a different question: is the -//! content root actually a vault Obsidian knows about? -//! -//! `obsidian://open?path=` only resolves against vaults already recorded -//! in Obsidian's `obsidian.json` registry — it can **not** register a new -//! vault, and a `.obsidian/` folder on disk is not enough. So before the -//! Memory tab fires that deep link we check whether the content root (or an -//! ancestor) is a registered vault. If it isn't, the UI guides the user to add -//! it once ("Open folder as vault") instead of firing a link Obsidian rejects -//! with *"Unable to find a vault for the URL"*. -//! -//! Detection is **best-effort**: Obsidian can live in non-standard locations -//! (Flatpak, Snap, custom `$XDG_CONFIG_HOME`, portable). A negative result must -//! never block the user — the caller still offers "open anyway" + "reveal -//! folder" + a config-dir override that feeds back in here as `extra`. - -mod types; - -use std::path::{Path, PathBuf}; - -use types::ObsidianConfig; -pub use types::VaultRegistration; - -/// Candidate `obsidian.json` locations, in priority order. `extra` (a -/// user-supplied override pointing at Obsidian's *config dir*) is checked -/// first so a power user can correct a non-standard install. -fn candidate_config_files(extra: Option<&Path>) -> Vec { - let mut out = Vec::new(); - - if let Some(dir) = extra { - // Accept either the config dir itself or its parent (users often - // can't tell whether the path should end in `obsidian/`). - out.push(dir.join("obsidian.json")); - out.push(dir.join("obsidian").join("obsidian.json")); - } - - // Standard per-OS config dir: `~/.config` (Linux), `~/Library/Application - // Support` (macOS), `%APPDATA%` (Windows). - if let Some(cfg) = dirs::config_dir() { - out.push(cfg.join("obsidian").join("obsidian.json")); - } - - // Linux sandbox installs keep their own config tree. Harmless to probe on - // other OSes — the paths simply won't exist. - if let Some(home) = dirs::home_dir() { - out.push(home.join(".var/app/md.obsidian.Obsidian/config/obsidian/obsidian.json")); // Flatpak - out.push(home.join("snap/obsidian/current/.config/obsidian/obsidian.json")); - // Snap - } - - out -} - -/// Best-effort: is `content_root` (or an ancestor) a registered Obsidian -/// vault? `extra_config_dir` optionally points at Obsidian's config dir for -/// non-standard installs. Never errors — probe failures report -/// `registered = false`. -pub fn vault_registration_status( - content_root: &Path, - extra_config_dir: Option<&Path>, -) -> VaultRegistration { - registration_in_files(content_root, &candidate_config_files(extra_config_dir)) -} - -/// Core of [`vault_registration_status`], split out so tests can supply an -/// explicit, isolated set of `obsidian.json` paths instead of depending on -/// whatever Obsidian config happens to exist on the host. -fn registration_in_files(content_root: &Path, files: &[PathBuf]) -> VaultRegistration { - // Absolutize the content root against the current directory (lexically, - // without touching the filesystem) so a relative workspace/content_root - // — supported by `MemoryConfig::from_toml_file` — compares equal to the - // absolute vault paths Obsidian records in `obsidian.json`. Without this, - // a relative root can never be a prefix of an absolute vault path and an - // already-registered vault is always reported unregistered. - let target = std::path::absolute(content_root) - .map(|abs| lexically_normalize(&abs)) - .unwrap_or_else(|_| lexically_normalize(content_root)); - let mut config_found = false; - - for path in files { - let body = match std::fs::read_to_string(path) { - Ok(b) => b, - Err(_) => continue, // missing/unreadable candidate — try the next. - }; - config_found = true; - - let parsed: ObsidianConfig = match serde_json::from_str(&body) { - Ok(p) => p, - Err(err) => { - // Redact the path — it embeds the user's home/username. - log::warn!( - "[content_store::obsidian_registry] parse {} failed: {err} — skipping", - crate::memory::chunks::redact(&path.display().to_string()) - ); - continue; - } - }; - - for entry in parsed.vaults.values() { - let vault = lexically_normalize(Path::new(&entry.path)); - // A malformed/empty vault path normalizes to "" and would otherwise - // match every content root (empty ancestor ⊂ anything) — skip it. - if vault.as_os_str().is_empty() { - continue; - } - if is_ancestor_or_equal(&vault, &target) { - log::debug!( - "[content_store::obsidian_registry] content root is a registered vault \ - (matched in {})", - crate::memory::chunks::redact(&path.display().to_string()) - ); - return VaultRegistration { - registered: true, - config_found: true, - }; - } - } - } - - log::debug!( - "[content_store::obsidian_registry] content root NOT registered (config_found={})", - config_found - ); - VaultRegistration { - registered: false, - config_found, - } -} - -/// Strip trailing separators so `/a/b` and `/a/b/` compare equal. Lexical -/// only — we deliberately do not canonicalize: the vault path may be on an -/// unmounted volume or use a symlink, and canonicalize would error or rewrite -/// it. Both inputs come from trusted local sources, so a textual compare is -/// the safe, dependency-free choice. -fn lexically_normalize(p: &Path) -> PathBuf { - let s = p.to_string_lossy(); - let trimmed = s.trim_end_matches(['/', '\\']); - if trimmed.is_empty() { - // Was a pure root like "/" — keep it. - PathBuf::from(s.as_ref()) - } else { - PathBuf::from(trimmed) - } -} - -/// `true` when `ancestor == descendant`, or `ancestor` is a path-prefix of -/// `descendant` on component boundaries (so `/a/b` contains `/a/b/c` but not -/// `/a/bc`). Case-sensitive — adequate for the Linux target; a false negative -/// on case-insensitive volumes only makes detection conservative (the caller -/// still offers "open anyway"). -fn is_ancestor_or_equal(ancestor: &Path, descendant: &Path) -> bool { - let a: Vec<_> = ancestor.components().collect(); - let d: Vec<_> = descendant.components().collect(); - // An empty ancestor must not match (it would otherwise be a prefix of - // everything); also bail when the ancestor is longer than the descendant. - if a.is_empty() || a.len() > d.len() { - return false; - } - a.iter().zip(d.iter()).all(|(x, y)| x == y) -} - -#[cfg(test)] -#[path = "obsidian_registry_tests.rs"] -mod tests; diff --git a/src/memory/store/content/obsidian_registry/types.rs b/src/memory/store/content/obsidian_registry/types.rs deleted file mode 100644 index 36482dc..0000000 --- a/src/memory/store/content/obsidian_registry/types.rs +++ /dev/null @@ -1,33 +0,0 @@ -//! Types for Obsidian vault-*registration* detection: the probe result and -//! the minimal `obsidian.json` shape. - -use std::collections::HashMap; - -use serde::Deserialize; - -/// Outcome of a registration probe. -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct VaultRegistration { - /// `true` when some registered Obsidian vault's path equals or is an - /// ancestor of the content root. - pub registered: bool, - /// `true` when at least one candidate `obsidian.json` was found/read (even - /// if parsing it later fails — see the parse-error branch, which still - /// counts the file as found). Lets the UI distinguish "Obsidian is set up, - /// vault just not added yet" from "couldn't find Obsidian at all" (offer - /// install vs. offer add-as-vault). - pub config_found: bool, -} - -/// Minimal shape of Obsidian's `obsidian.json`. We only need each vault's -/// `path`; `ts`/`open` and any future keys are ignored by `serde`. -#[derive(Debug, Deserialize)] -pub struct ObsidianConfig { - #[serde(default)] - pub vaults: HashMap, -} - -#[derive(Debug, Deserialize)] -pub struct VaultEntry { - pub path: String, -} diff --git a/src/memory/store/content/obsidian_registry_tests.rs b/src/memory/store/content/obsidian_registry_tests.rs deleted file mode 100644 index c302d3a..0000000 --- a/src/memory/store/content/obsidian_registry_tests.rs +++ /dev/null @@ -1,155 +0,0 @@ -use super::*; -use std::io::Write; - -/// Write an `obsidian.json` containing `vault_paths` and return its path. -fn write_config(dir: &Path, vault_paths: &[&str]) -> PathBuf { - let entries: Vec = vault_paths - .iter() - .enumerate() - .map(|(i, p)| { - format!( - "\"id{i}\": {{ \"path\": {}, \"ts\": 1700000000000, \"open\": true }}", - serde_json::to_string(p).unwrap() - ) - }) - .collect(); - let body = format!("{{ \"vaults\": {{ {} }} }}", entries.join(", ")); - let path = dir.join("obsidian.json"); - let mut f = std::fs::File::create(&path).unwrap(); - f.write_all(body.as_bytes()).unwrap(); - path -} - -#[test] -fn exact_match_is_registered() { - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path().join("memory_tree/content"); - let cfg = write_config(tmp.path(), &[root.to_str().unwrap()]); - let got = registration_in_files(&root, &[cfg]); - assert_eq!( - got, - VaultRegistration { - registered: true, - config_found: true - } - ); -} - -#[test] -fn ancestor_vault_is_registered() { - // A vault rooted at the parent still "contains" the content root. - let tmp = tempfile::tempdir().unwrap(); - let parent = tmp.path().join("workspace"); - let root = parent.join("memory_tree/content"); - let cfg = write_config(tmp.path(), &[parent.to_str().unwrap()]); - assert!(registration_in_files(&root, &[cfg]).registered); -} - -#[test] -fn trailing_slash_does_not_matter() { - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path().join("memory_tree/content"); - let with_slash = format!("{}/", root.to_str().unwrap()); - let cfg = write_config(tmp.path(), &[&with_slash]); - assert!(registration_in_files(&root, &[cfg]).registered); -} - -#[test] -fn relative_content_root_matches_absolute_vault() { - // `MemoryConfig::from_toml_file` accepts a relative workspace/content_root. - // The registry check must absolutize that root against the CWD before the - // prefix comparison, or an already-registered vault is always reported - // unregistered. - let tmp = tempfile::tempdir().unwrap(); - let cwd = std::env::current_dir().unwrap(); - let abs_root = cwd.join("memory_tree/content"); - let cfg = write_config(tmp.path(), &[abs_root.to_str().unwrap()]); - let got = registration_in_files(Path::new("memory_tree/content"), &[cfg]); - assert!( - got.registered, - "relative content root must be absolutized against the CWD before matching" - ); -} - -#[test] -fn unrelated_vault_is_not_registered_but_config_found() { - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path().join("memory_tree/content"); - let cfg = write_config(tmp.path(), &["/some/other/vault"]); - let got = registration_in_files(&root, &[cfg]); - assert_eq!( - got, - VaultRegistration { - registered: false, - config_found: true - } - ); -} - -#[test] -fn empty_vault_path_does_not_match_every_root() { - // Regression: a malformed entry with an empty `path` must not - // normalize to "" and match every content root as an ancestor. - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path().join("memory_tree/content"); - let cfg = write_config(tmp.path(), &[""]); - let got = registration_in_files(&root, &[cfg]); - assert_eq!( - got, - VaultRegistration { - registered: false, - config_found: true - } - ); -} - -#[test] -fn sibling_prefix_is_not_a_false_match() { - // `/a/b/content` must NOT match a vault at `/a/b/content-archive`. - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path().join("content"); - let decoy = format!("{}-archive", root.to_str().unwrap()); - let cfg = write_config(tmp.path(), &[&decoy]); - assert!(!registration_in_files(&root, &[cfg]).registered); -} - -#[test] -fn missing_config_reports_not_found() { - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path().join("memory_tree/content"); - let missing = tmp.path().join("does-not-exist.json"); - let got = registration_in_files(&root, &[missing]); - assert_eq!( - got, - VaultRegistration { - registered: false, - config_found: false - } - ); -} - -#[test] -fn malformed_config_is_skipped_not_fatal() { - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path().join("memory_tree/content"); - let bad = tmp.path().join("obsidian.json"); - std::fs::write(&bad, b"{ this is not json ").unwrap(); - // config_found is true (we read it) but parse fails → not registered. - let got = registration_in_files(&root, &[bad]); - assert_eq!( - got, - VaultRegistration { - registered: false, - config_found: true - } - ); -} - -#[test] -fn second_candidate_wins_when_first_missing() { - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path().join("memory_tree/content"); - let missing = tmp.path().join("nope.json"); - let real = write_config(tmp.path(), &[root.to_str().unwrap()]); - assert!(registration_in_files(&root, &[missing, real]).registered); -}