fix(memory-workspace): detect Obsidian vault registration before deep link (#2638)

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
sanil-23
2026-05-26 02:25:21 +05:30
committed by GitHub
co-authored by Claude Opus 4.7
parent 0e4729e7f2
commit e05cab9bb2
26 changed files with 1274 additions and 118 deletions
+42 -3
View File
@@ -500,7 +500,43 @@ mod tests {
use super::*;
fn ensure_memory_client() {
/// Held for the whole test: pins `OPENHUMAN_WORKSPACE` at a stable,
/// never-torn-down workspace under `TEST_ENV_LOCK`. These tests call
/// `memory_init` → `current_workspace_dir` → `Config::load_or_init`, which
/// reads that process-global env var; without this, a concurrent
/// env-mutating test can swap the var and tear down its tempdir mid-call,
/// yielding `SQLITE_IOERR` / config atomic-replace `ENOENT`.
struct WorkspaceEnvGuard {
_lock: std::sync::MutexGuard<'static, ()>,
previous: Option<std::ffi::OsString>,
}
impl WorkspaceEnvGuard {
fn set(path: &std::path::Path) -> Self {
let lock = crate::openhuman::config::TEST_ENV_LOCK
.lock()
.unwrap_or_else(|e| e.into_inner());
let previous = std::env::var_os("OPENHUMAN_WORKSPACE");
std::env::set_var("OPENHUMAN_WORKSPACE", path);
Self {
_lock: lock,
previous,
}
}
}
impl Drop for WorkspaceEnvGuard {
fn drop(&mut self) {
if let Some(previous) = self.previous.as_ref() {
std::env::set_var("OPENHUMAN_WORKSPACE", previous);
} else {
std::env::remove_var("OPENHUMAN_WORKSPACE");
}
}
}
#[must_use]
fn ensure_memory_client() -> WorkspaceEnvGuard {
static WORKSPACE: OnceLock<PathBuf> = OnceLock::new();
let workspace = WORKSPACE.get_or_init(|| {
let tmp = TempDir::new().expect("tempdir");
@@ -509,7 +545,10 @@ mod tests {
std::mem::forget(tmp);
path
});
// Pin the env BEFORE init so config load/save targets the stable dir.
let env_guard = WorkspaceEnvGuard::set(workspace);
let _ = crate::openhuman::memory::global::init(workspace.clone());
env_guard
}
fn unique_namespace(prefix: &str) -> String {
@@ -538,7 +577,7 @@ mod tests {
let _serial = crate::openhuman::memory::ops::GLOBAL_MEMORY_TEST_LOCK
.lock()
.await;
ensure_memory_client();
let _env = ensure_memory_client();
let namespace = unique_namespace("memory-docs-direct");
let key = format!(
"note{}",
@@ -619,7 +658,7 @@ mod tests {
let _serial = crate::openhuman::memory::ops::GLOBAL_MEMORY_TEST_LOCK
.lock()
.await;
ensure_memory_client();
let _env = ensure_memory_client();
let namespace = unique_namespace("memory-docs-envelope");
let key = format!("env{}", &uuid::Uuid::new_v4().as_simple().to_string()[..12]);
+132 -3
View File
@@ -32,6 +32,7 @@ use serde::{Deserialize, Serialize};
use crate::openhuman::config::Config;
use crate::openhuman::memory_store::chunks::store::{self as chunk_store, with_connection};
use crate::openhuman::memory_store::chunks::types::SourceKind;
use crate::openhuman::memory_store::content::obsidian_registry;
use crate::openhuman::memory_store::content::read as content_read;
use crate::openhuman::memory_tree::retrieval::types::NodeKind;
use crate::openhuman::memory_tree::score::store as score_store;
@@ -964,9 +965,12 @@ pub struct GraphExportResponse {
#[serde(default)]
pub edges: Vec<GraphEdge>,
/// Absolute path to the on-disk `<workspace>/memory_tree/content/` root.
/// UIs use this to open files via the `obsidian://open?path=...` deep
/// link — Obsidian resolves arbitrary absolute paths without requiring
/// the vault to be registered.
/// UIs use this both to point an `obsidian://open?path=...` deep link at
/// the vault and as the folder the user adds via "Open folder as vault".
/// That deep link only resolves once this folder (or an ancestor) is a
/// *registered* Obsidian vault — the scheme cannot register a new vault on
/// its own, so the UI first calls [`obsidian_vault_status_rpc`] and guides
/// the user to add it when it isn't.
pub content_root_abs: String,
}
@@ -1005,6 +1009,67 @@ pub async fn graph_export_rpc(
Ok(RpcOutcome::single_log(resp, log))
}
/// Response shape for [`obsidian_vault_status_rpc`].
#[derive(Clone, Debug, Serialize, Deserialize)]
pub struct ObsidianVaultStatusResponse {
/// `true` when the content root (or an ancestor) is already a registered
/// Obsidian vault, so `obsidian://open?path=` will actually resolve.
pub registered: bool,
/// `true` when an `obsidian.json` was found and parsed (Obsidian is set
/// up). Lets the UI offer "Open folder as vault" vs. "Install Obsidian".
pub config_found: bool,
/// Absolute path to `<workspace>/memory_tree/content/` — the folder the
/// user adds to Obsidian, and the target of the deep link.
pub content_root_abs: String,
}
/// `memory_tree_obsidian_vault_status` — best-effort check of whether the
/// memory-tree content root is a registered Obsidian vault.
///
/// The Memory tab calls this before firing the `obsidian://open?path=` deep
/// link: that scheme only resolves vaults already present in Obsidian's
/// `obsidian.json`, so opening an unregistered folder lands on *"Unable to
/// find a vault for the URL"*. `obsidian_config_dir` optionally overrides
/// where we look for `obsidian.json` (non-standard installs: Flatpak / Snap /
/// portable). Never errors and never hits the network — a probe miss simply
/// reports `registered = false` and the UI degrades to "open anyway" + reveal.
pub async fn obsidian_vault_status_rpc(
config: &Config,
obsidian_config_dir: Option<String>,
) -> Result<RpcOutcome<ObsidianVaultStatusResponse>, String> {
let cfg = config.clone();
let resp = tokio::task::spawn_blocking(move || -> ObsidianVaultStatusResponse {
let content_root = cfg.memory_tree_content_root();
// Treat a blank/whitespace override as "no override" — otherwise
// `Path::new("")` resolves to `.` and would probe a stray local
// `./obsidian.json`. The UI omits the field when empty, but the RPC
// is a public controller so normalize defensively here.
let extra = obsidian_config_dir
.as_deref()
.map(str::trim)
.filter(|s| !s.is_empty())
.map(std::path::Path::new);
let reg = obsidian_registry::vault_registration_status(&content_root, extra);
ObsidianVaultStatusResponse {
registered: reg.registered,
config_found: reg.config_found,
content_root_abs: content_root.to_string_lossy().to_string(),
}
})
.await
.map_err(|e| format!("obsidian_vault_status join error: {e}"))?;
// Redact the absolute path (embeds the user's home / username) — log only
// the booleans and a stable hash, matching `graph_export_rpc`.
let log = format!(
"memory_tree::read: obsidian_vault_status registered={} config_found={} root_hash={}",
resp.registered,
resp.config_found,
crate::openhuman::memory::util::redact::redact(&resp.content_root_abs),
);
Ok(RpcOutcome::single_log(resp, log))
}
/// Tree mode: summary nodes joined to their owning tree for the
/// human-readable scope. Edges are encoded implicitly via
/// `GraphNode.parent_id`.
@@ -2381,4 +2446,68 @@ mod tests {
assert_eq!(composio_count, 0);
assert_eq!(other_count, 1);
}
#[tokio::test]
async fn obsidian_status_registered_when_override_config_lists_content_root() {
let (_tmp, cfg) = test_config();
let content_root = cfg.memory_tree_content_root();
// A separate dir standing in for a non-standard Obsidian config
// location, with an obsidian.json that registers the content root.
let cfg_dir = TempDir::new().unwrap();
let body = format!(
"{{ \"vaults\": {{ \"id0\": {{ \"path\": {}, \"open\": true }} }} }}",
serde_json::to_string(&content_root.to_string_lossy().to_string()).unwrap()
);
std::fs::write(cfg_dir.path().join("obsidian.json"), body).unwrap();
let outcome =
obsidian_vault_status_rpc(&cfg, Some(cfg_dir.path().to_string_lossy().to_string()))
.await
.unwrap();
assert!(outcome.value.registered);
assert!(outcome.value.config_found);
assert_eq!(
outcome.value.content_root_abs,
content_root.to_string_lossy().to_string()
);
// The log reports the booleans but redacts the absolute path (it
// embeds the user's home / username).
assert!(
outcome.logs[0].contains("registered=true"),
"log: {}",
outcome.logs[0]
);
assert!(
!outcome.logs[0].contains(content_root.to_str().unwrap()),
"log leaked content root: {}",
outcome.logs[0]
);
}
#[tokio::test]
async fn obsidian_status_not_registered_for_empty_override_dir() {
let (_tmp, cfg) = test_config();
// Empty override dir → no obsidian.json there → content root is not a
// registered vault. (A temp content root can't be under any real host
// vault either, so this stays false regardless of the dev machine.)
let cfg_dir = TempDir::new().unwrap();
let outcome =
obsidian_vault_status_rpc(&cfg, Some(cfg_dir.path().to_string_lossy().to_string()))
.await
.unwrap();
assert!(!outcome.value.registered);
}
#[tokio::test]
async fn obsidian_status_blank_override_is_treated_as_none() {
// A whitespace-only override must be normalized to None rather than
// resolving to "." and probing a stray local ./obsidian.json. The temp
// content root isn't under any real host vault, so this stays false.
let (_tmp, cfg) = test_config();
let outcome = obsidian_vault_status_rpc(&cfg, Some(" ".to_string()))
.await
.unwrap();
assert!(!outcome.value.registered);
}
}
+61
View File
@@ -40,6 +40,7 @@ pub fn all_controller_schemas() -> Vec<ControllerSchema> {
schemas("chunk_score"),
schemas("delete_chunk"),
schemas("graph_export"),
schemas("obsidian_vault_status"),
schemas("flush_now"),
schemas("wipe_all"),
schemas("reset_tree"),
@@ -106,6 +107,10 @@ pub fn all_registered_controllers() -> Vec<RegisteredController> {
schema: schemas("graph_export"),
handler: handle_graph_export,
},
RegisteredController {
schema: schemas("obsidian_vault_status"),
handler: handle_obsidian_vault_status,
},
RegisteredController {
schema: schemas("flush_now"),
handler: handle_flush_now,
@@ -603,6 +608,49 @@ pub fn schemas(function: &str) -> ControllerSchema {
},
],
},
"obsidian_vault_status" => ControllerSchema {
namespace: NAMESPACE,
function: "obsidian_vault_status",
description: "Best-effort check of whether the memory-tree content root is \
already a registered Obsidian vault. `obsidian://open?path=` only \
resolves vaults present in Obsidian's obsidian.json registry — it \
cannot register a new one — so the Memory tab calls this before \
firing the deep link and guides the user to 'Open folder as vault' \
when it isn't registered. Never errors; a probe miss reports \
registered=false.",
inputs: vec![FieldSchema {
name: "obsidian_config_dir",
ty: TypeSchema::Option(Box::new(TypeSchema::String)),
comment: "Optional override for Obsidian's config directory (where \
obsidian.json lives), for non-standard installs \
(Flatpak / Snap / portable). Omitted ⇒ probe the standard per-OS \
location plus known sandbox paths.",
required: false,
}],
outputs: vec![
FieldSchema {
name: "registered",
ty: TypeSchema::Bool,
comment: "True when the content root (or an ancestor) is a registered \
Obsidian vault, so the deep link will resolve.",
required: true,
},
FieldSchema {
name: "config_found",
ty: TypeSchema::Bool,
comment: "True when an obsidian.json was found and parsed (Obsidian is \
set up). Lets the UI offer add-as-vault vs. install.",
required: true,
},
FieldSchema {
name: "content_root_abs",
ty: TypeSchema::String,
comment: "Absolute path to <workspace>/memory_tree/content/ — the folder \
to add to Obsidian and the deep-link target.",
required: true,
},
],
},
"trigger_digest" => ControllerSchema {
namespace: NAMESPACE,
function: "trigger_digest",
@@ -842,6 +890,19 @@ fn handle_graph_export(params: Map<String, Value>) -> ControllerFuture {
})
}
fn handle_obsidian_vault_status(params: Map<String, Value>) -> ControllerFuture {
Box::pin(async move {
#[derive(serde::Deserialize, Default)]
struct Req {
#[serde(default)]
obsidian_config_dir: Option<String>,
}
let config = config_rpc::load_config_with_timeout().await?;
let req = parse_value::<Req>(Value::Object(params)).unwrap_or_default();
to_json(read_rpc::obsidian_vault_status_rpc(&config, req.obsidian_config_dir).await?)
})
}
fn handle_flush_now(_params: Map<String, Value>) -> ControllerFuture {
Box::pin(async move {
let config = config_rpc::load_config_with_timeout().await?;
@@ -15,6 +15,7 @@
pub mod atomic;
pub mod compose;
pub mod obsidian;
pub mod obsidian_registry;
pub mod paths;
pub mod raw;
pub mod read;
@@ -0,0 +1,321 @@
//! 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=<abs>` 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`.
use std::path::{Path, PathBuf};
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)]
struct ObsidianConfig {
#[serde(default)]
vaults: std::collections::HashMap<String, VaultEntry>,
}
#[derive(Debug, Deserialize)]
struct VaultEntry {
path: String,
}
/// 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<PathBuf> {
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 {
let target = 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::openhuman::memory::util::redact::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::openhuman::memory::util::redact::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)]
mod tests {
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<String> = 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 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);
}
}