fix(memory-workspace): detect Obsidian vault registration before deep link#2638
Conversation
📝 WalkthroughWalkthroughAdds backend vault-registration detection and a new ObsidianVaultSection UI that probes vault readiness before attempting obsidian:// deep-links; provides reveal/install/config-dir override guidance, RPC/schema wiring, TypeScript adapter, tests, i18n strings, and docs updates. ChangesObsidian Vault Detection & Guided Opening
Sequence DiagramsequenceDiagram
participant User
participant ObsidianVaultSection
participant Adapter as memoryTreeObsidianVaultStatus
participant ReadRPC as obsidian_vault_status_rpc
participant Registry as obsidian_registry
User->>ObsidianVaultSection: click "Open in Obsidian"
ObsidianVaultSection->>Adapter: probe vault status (optional config override)
Adapter->>ReadRPC: RPC call openhuman.memory_tree_obsidian_vault_status(params)
ReadRPC->>Registry: vault_registration_status(content_root, override)
Registry-->>ReadRPC: { registered, config_found, content_root_abs }
ReadRPC-->>Adapter: status response
Adapter-->>ObsidianVaultSection: status
alt registered
ObsidianVaultSection->>User: open deep link via openUrl and toast
else not registered or probe failed
ObsidianVaultSection->>User: show guidance panel (Reveal / Open anyway / Install / Advanced)
end
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/components/intelligence/ObsidianVaultSection.tsx`:
- Line 184: Replace the hard-coded user-visible JSX strings in the
ObsidianVaultSection component (the title prop value
`obsidian://open?path=${contentRootAbs}` and the placeholder at the other
occurrence) with localized strings obtained from the useT() hook (e.g., const t
= useT()) and reference new i18n keys (add descriptive keys like
obsidian.openLink and obsidian.placeholder to the component). Update
app/src/lib/i18n/en.ts with those new keys and their English values and add the
same keys to the other locale chunk files so all locales include the new
entries.
- Around line 114-118: installObsidian currently calls
openUrl(OBSIDIAN_DOWNLOAD_URL) and ignores any rejection, which can create
unhandled async errors; update installObsidian to handle the returned promise
from openUrl by awaiting or using .catch(...) and surface failures (e.g., log
via processLogger/console.error and show a user-facing error/toast) so failures
are handled gracefully and don't bubble as unhandled rejections; locate the
installObsidian callback in ObsidianVaultSection.tsx and add proper error
handling around openUrl.
In `@src/openhuman/memory_store/content/obsidian_registry.rs`:
- Around line 115-117: Guard against empty vault paths by skipping any
parsed.vaults entries whose entry.path is empty (or which
lexically_normalize(Path::new(&entry.path)) yields an empty path) before calling
is_ancestor_or_equal; update the loop that computes `vault =
lexically_normalize(Path::new(&entry.path))` to continue if
entry.path.trim().is_empty() or if `vault` has no components, then only call
`is_ancestor_or_equal(&vault, &target)` for non-empty vaults so malformed/empty
paths cannot produce false positives for `registered`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d17802cf-bce1-4c6c-aa42-1ce49abdd4cd
📒 Files selected for processing (25)
app/src/components/intelligence/MemoryWorkspace.tsxapp/src/components/intelligence/ObsidianVaultSection.tsxapp/src/components/intelligence/__tests__/MemoryWorkspace.test.tsxapp/src/components/intelligence/__tests__/ObsidianVaultSection.test.tsxapp/src/lib/i18n/chunks/ar-3.tsapp/src/lib/i18n/chunks/bn-3.tsapp/src/lib/i18n/chunks/de-3.tsapp/src/lib/i18n/chunks/en-3.tsapp/src/lib/i18n/chunks/es-3.tsapp/src/lib/i18n/chunks/fr-3.tsapp/src/lib/i18n/chunks/hi-3.tsapp/src/lib/i18n/chunks/id-3.tsapp/src/lib/i18n/chunks/it-3.tsapp/src/lib/i18n/chunks/ko-3.tsapp/src/lib/i18n/chunks/pt-3.tsapp/src/lib/i18n/chunks/ru-3.tsapp/src/lib/i18n/chunks/zh-CN-3.tsapp/src/lib/i18n/en.tsapp/src/utils/tauriCommands/memoryTree.test.tsapp/src/utils/tauriCommands/memoryTree.tsgitbooks/features/obsidian-wiki/README.mdsrc/openhuman/memory/read_rpc.rssrc/openhuman/memory/schema.rssrc/openhuman/memory_store/content/mod.rssrc/openhuman/memory_store/content/obsidian_registry.rs
4027a9b to
308713d
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
app/src/lib/i18n/en.ts (1)
1601-1601: 💤 Low valueConsider reusing
common.saveinstead of duplicating "Save".
'workspace.save': 'Save'at line 1601 duplicates'common.save': 'Save'at line 16. If the context doesn't require a distinct translation, reusing the common key would reduce the string count and simplify localization.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/lib/i18n/en.ts` at line 1601, The file defines a duplicate translation key 'workspace.save' with the same value as 'common.save'; remove the redundant 'workspace.save' entry and update any call sites that reference workspace.save to use the shared key 'common.save' (or alias usages to t('common.save')) so the single common translation is reused across the codebase.app/src/components/intelligence/ObsidianVaultSection.tsx (1)
186-200: ⚡ Quick winUse design-system color tokens instead of raw
violet-*utilities.This section introduces several hard-coded
violet-*classes for primary UI surfaces/actions. Please switch these to your configured tokenized palette (primary-*/ semantic tokens) to keep visual consistency with the design system.♻️ Suggested token-aligned class updates
- className="... bg-violet-500 ... hover:bg-violet-600 ... focus:ring-violet-300" + className="... bg-primary-500 ... hover:bg-primary-600 ... focus:ring-primary-200" - className="... border-violet-200 bg-violet-50 ... dark:border-violet-500/30 dark:bg-violet-500/10" + className="... border-primary-200 bg-primary-50 ... dark:border-primary-500/30 dark:bg-primary-500/10" - className="... border-violet-300 ... text-violet-700 hover:bg-violet-50 ... dark:border-violet-500/40 ... dark:text-violet-300" + className="... border-primary-300 ... text-primary-700 hover:bg-primary-50 ... dark:border-primary-500/40 ... dark:text-primary-300" - className="... text-violet-600 ... dark:text-violet-300" + className="... text-primary-600 ... dark:text-primary-300" - className="... focus:ring-violet-300 ..." + className="... focus:ring-primary-300 ..." - className="... bg-violet-500 ... hover:bg-violet-600 ..." + className="... bg-primary-500 ... hover:bg-primary-600 ..."As per coding guidelines
app/**/*.{tsx,js,css}: "Design system tokens live inapp/tailwind.config.js. Use Tailwind with custom radii/spacing/shadows. Design intent: premium, calm visual language — ocean primary (#4A83DD), sage / amber / coral semantic colors..."Also applies to: 224-227, 244-245, 264-266, 273-274
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/components/intelligence/ObsidianVaultSection.tsx` around lines 186 - 200, Replace hard-coded violet-* Tailwind utilities in the ObsidianVaultSection component with the project's design-system tokens (e.g., primary-*/semantic tokens from tailwind.config) so UI follows the tokenized palette: update the button's className (the inline-flex button that renders ExternalLinkIcon and uses title={`obsidian://open?path=${contentRootAbs}`}) to use primary background, hover, focus ring and disabled token classes instead of bg-violet-500/600 and ring-violet-300, and update the guidance panel (the div with data-testid="obsidian-vault-guidance") to use tokenized border/bg colors instead of border-violet-200 and bg-violet-50 (and the dark-mode variants). Apply the same token replacements to the other similar blocks in this component where violet-* is used (the other button/panel instances referenced in the review).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/openhuman/memory/read_rpc.rs`:
- Around line 1041-1044: Normalize obsidian_config_dir by trimming and treating
empty or whitespace-only strings as None before converting to a Path to avoid
resolving to "."; in the tokio::task::spawn_blocking closure update the handling
of obsidian_config_dir (the value passed into
obsidian_registry::vault_registration_status) so you call something like
obsidian_config_dir.and_then(|s| { let t = s.trim(); if t.is_empty() { None }
else { Some(std::path::Path::new(t)) } }) (or equivalent) instead of using
as_deref().map(std::path::Path::new), ensuring vault_registration_status gets
None for blank overrides.
---
Nitpick comments:
In `@app/src/components/intelligence/ObsidianVaultSection.tsx`:
- Around line 186-200: Replace hard-coded violet-* Tailwind utilities in the
ObsidianVaultSection component with the project's design-system tokens (e.g.,
primary-*/semantic tokens from tailwind.config) so UI follows the tokenized
palette: update the button's className (the inline-flex button that renders
ExternalLinkIcon and uses title={`obsidian://open?path=${contentRootAbs}`}) to
use primary background, hover, focus ring and disabled token classes instead of
bg-violet-500/600 and ring-violet-300, and update the guidance panel (the div
with data-testid="obsidian-vault-guidance") to use tokenized border/bg colors
instead of border-violet-200 and bg-violet-50 (and the dark-mode variants).
Apply the same token replacements to the other similar blocks in this component
where violet-* is used (the other button/panel instances referenced in the
review).
In `@app/src/lib/i18n/en.ts`:
- Line 1601: The file defines a duplicate translation key 'workspace.save' with
the same value as 'common.save'; remove the redundant 'workspace.save' entry and
update any call sites that reference workspace.save to use the shared key
'common.save' (or alias usages to t('common.save')) so the single common
translation is reused across the codebase.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2544751d-e0c7-45ef-987e-67538a93bee4
📒 Files selected for processing (25)
app/src/components/intelligence/MemoryWorkspace.tsxapp/src/components/intelligence/ObsidianVaultSection.tsxapp/src/components/intelligence/__tests__/MemoryWorkspace.test.tsxapp/src/components/intelligence/__tests__/ObsidianVaultSection.test.tsxapp/src/lib/i18n/chunks/ar-3.tsapp/src/lib/i18n/chunks/bn-3.tsapp/src/lib/i18n/chunks/de-3.tsapp/src/lib/i18n/chunks/en-3.tsapp/src/lib/i18n/chunks/es-3.tsapp/src/lib/i18n/chunks/fr-3.tsapp/src/lib/i18n/chunks/hi-3.tsapp/src/lib/i18n/chunks/id-3.tsapp/src/lib/i18n/chunks/it-3.tsapp/src/lib/i18n/chunks/ko-3.tsapp/src/lib/i18n/chunks/pt-3.tsapp/src/lib/i18n/chunks/ru-3.tsapp/src/lib/i18n/chunks/zh-CN-3.tsapp/src/lib/i18n/en.tsapp/src/utils/tauriCommands/memoryTree.test.tsapp/src/utils/tauriCommands/memoryTree.tsgitbooks/features/obsidian-wiki/README.mdsrc/openhuman/memory/read_rpc.rssrc/openhuman/memory/schema.rssrc/openhuman/memory_store/content/mod.rssrc/openhuman/memory_store/content/obsidian_registry.rs
✅ Files skipped from review due to trivial changes (9)
- app/src/lib/i18n/chunks/ko-3.ts
- app/src/lib/i18n/chunks/de-3.ts
- app/src/lib/i18n/chunks/ar-3.ts
- app/src/lib/i18n/chunks/zh-CN-3.ts
- app/src/lib/i18n/chunks/ru-3.ts
- app/src/lib/i18n/chunks/hi-3.ts
- app/src/lib/i18n/chunks/bn-3.ts
- app/src/lib/i18n/chunks/es-3.ts
- app/src/lib/i18n/chunks/id-3.ts
|
Thanks for the re-review — addressed the actionable (blank
|
13d206b to
25cce39
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/components/intelligence/ObsidianVaultSection.tsx`:
- Around line 140-146: The branch handling a registered vault leaves the
guidance panel open when a previous error had set expanded=true; after calling
fireDeepLink() and toastOpenOutcome(err) in the registered-vault path (the code
using status.registered, fireDeepLink, toastOpenOutcome, and setExpanded),
explicitly clear the guidance UI when the deep-link succeeded by calling
setExpanded(false) when err === null (keep the existing setExpanded(true) when
err !== null).
In `@src/openhuman/memory_store/content/obsidian_registry.rs`:
- Around line 107-110: The logs in
src/openhuman/memory_store/content/obsidian_registry.rs currently print
path.display() (e.g., in the log::warn! calls around the parse failure and the
block at lines 123-127) which can expose absolute config/home paths; replace
those uses with a redacted/hashed representation consistent with the redaction
used in src/openhuman/memory/read_rpc.rs (e.g., call the same redact/hash helper
used there or add a local helper like redact_path(path) that returns a
deterministic hash or redacted string), update the log::warn! invocations to
include the redacted value instead of path.display(), and import or reuse the
existing redact helper so both the parse failure warning and the other registry
warnings use the same redaction approach.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 20b8f7b4-633b-4e5a-aea9-1e4adb61ab9c
📒 Files selected for processing (25)
app/src/components/intelligence/MemoryWorkspace.tsxapp/src/components/intelligence/ObsidianVaultSection.tsxapp/src/components/intelligence/__tests__/MemoryWorkspace.test.tsxapp/src/components/intelligence/__tests__/ObsidianVaultSection.test.tsxapp/src/lib/i18n/chunks/ar-3.tsapp/src/lib/i18n/chunks/bn-3.tsapp/src/lib/i18n/chunks/de-3.tsapp/src/lib/i18n/chunks/en-3.tsapp/src/lib/i18n/chunks/es-3.tsapp/src/lib/i18n/chunks/fr-3.tsapp/src/lib/i18n/chunks/hi-3.tsapp/src/lib/i18n/chunks/id-3.tsapp/src/lib/i18n/chunks/it-3.tsapp/src/lib/i18n/chunks/ko-3.tsapp/src/lib/i18n/chunks/pt-3.tsapp/src/lib/i18n/chunks/ru-3.tsapp/src/lib/i18n/chunks/zh-CN-3.tsapp/src/lib/i18n/en.tsapp/src/utils/tauriCommands/memoryTree.test.tsapp/src/utils/tauriCommands/memoryTree.tsgitbooks/features/obsidian-wiki/README.mdsrc/openhuman/memory/read_rpc.rssrc/openhuman/memory/schema.rssrc/openhuman/memory_store/content/mod.rssrc/openhuman/memory_store/content/obsidian_registry.rs
✅ Files skipped from review due to trivial changes (8)
- gitbooks/features/obsidian-wiki/README.md
- app/src/lib/i18n/chunks/pt-3.ts
- src/openhuman/memory_store/content/mod.rs
- app/src/lib/i18n/chunks/de-3.ts
- app/src/lib/i18n/chunks/es-3.ts
- app/src/lib/i18n/chunks/ru-3.ts
- app/src/lib/i18n/chunks/ar-3.ts
- app/src/lib/i18n/en.ts
… link "View Vault" fired obsidian://open?path=<content_root> unconditionally, but that scheme only resolves folders already registered in Obsidian's obsidian.json — it can't register one (a .obsidian/ folder on disk is not enough). So a first-time click launched Obsidian onto "Unable to find a vault for the URL". The tinyhumansai#2281 fix made the failure visible but still couldn't open the vault. Now the click first probes registration (new memory_tree_obsidian_vault_status RPC — best-effort read of obsidian.json across the standard config dir + Flatpak/Snap + an optional config-dir override). If registered it opens directly; if not, an inline guidance section appears (progressive disclosure) with Reveal Folder, Open-in-Obsidian-anyway, Install Obsidian, and an Advanced config-folder override — instead of firing a doomed link. Detection is best-effort: a false "not registered" never blocks (Open anyway always fires the link). - core: content_store/obsidian_registry.rs detector + ancestor match - core: memory_tree_obsidian_vault_status controller; fix the false "resolves arbitrary paths without registration" comment in read_rpc - app: ObsidianVaultSection (inline, replaces the bare button) + RPC client - docs: obsidian-wiki "Open the vault" now documents the one-time step - tests: 10 Rust (detector + RPC) + 9 Vitest (section + client) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- obsidian_registry: skip empty vault paths and reject an empty ancestor in is_ancestor_or_equal so a malformed obsidian.json entry can't match every content root (false registered=true). Add regression test. - ObsidianVaultSection: wrap installObsidian's openUrl in try/catch so a rejection can't surface as an unhandled promise rejection. - i18n: localize the config-dir input placeholder (workspace.obsidianConfigDirPlaceholder) across en + all locale chunks. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… flow - read_rpc: normalize a blank/whitespace obsidian_config_dir to None before Path conversion so it can't resolve to "." and probe a stray local obsidian.json (the UI omits it when empty, but the RPC is a public controller). Add regression test. - i18n: reuse the existing common.save key instead of duplicating workspace.save; drop the redundant key from en + all locale chunks. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…g redaction) - ObsidianVaultSection: collapse the guidance panel on a successful registered open so stale fallback UI from a prior failed/not-registered check doesn't linger (setExpanded(err !== null)). - obsidian_registry: redact the obsidian.json path in the warn/debug logs (embeds the user's home), consistent with read_rpc's redaction. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
c346319 to
5669083
Compare
…nv race The `documents` tests call `memory_init` → `current_workspace_dir` → `Config::load_or_init`, which reads the process-global `OPENHUMAN_WORKSPACE`. Unlike the `learn` tests they held no env guard, so a concurrent env-mutating test could swap the var and tear its tempdir down mid-call, intermittently failing `memory_init` with `SQLITE_IOERR` / config atomic-replace `ENOENT` (seen 1/9500, only under cargo-llvm-cov's slower timing). `ensure_memory_client` now returns a `WorkspaceEnvGuard` that holds `TEST_ENV_LOCK` and pins `OPENHUMAN_WORKSPACE` at the stable, never-torn-down test workspace for the whole test; both `documents` tests bind it. Acquired after the existing `GLOBAL_MEMORY_TEST_LOCK` so lock order stays GLOBAL→ENV (matches the `learn` tests). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
graycyrus
left a comment
There was a problem hiding this comment.
Nice work — this is a well-structured fix for a real UX pain point.
What changed
| Area | Files | Summary |
|---|---|---|
| Rust core | obsidian_registry.rs, read_rpc.rs, schema.rs, mod.rs |
New obsidian_registry module probes obsidian.json across OS-standard + sandbox paths; new memory_tree_obsidian_vault_status RPC with defensive whitespace/empty-path normalization |
| Frontend | ObsidianVaultSection.tsx, MemoryWorkspace.tsx |
New component: click → probe → registered opens directly, unregistered shows progressive-disclosure guidance (Reveal / Open anyway / Install / Advanced config-dir override) |
| Tests | 8 Rust detector tests + 3 RPC tests, 7 Vitest component tests + 2 client tests | Thorough coverage of happy path, edge cases (empty paths, malformed config, trailing slashes, sibling prefixes), and failure modes |
| i18n | en.ts + 13 locale chunks |
10 new keys added consistently across all locales |
| Docs | obsidian-wiki/README.md |
Updated with step-by-step first-time vault registration flow |
| Test infra | documents.rs |
WorkspaceEnvGuard fixes a race between concurrent tests mutating OPENHUMAN_WORKSPACE — good stability improvement |
What I checked
- Architecture: Clean separation — detector module is pure filesystem, RPC is a thin wrapper, UI is a self-contained component. No
.unwrap()in production Rust. Detection is best-effort with safe fallbacks throughout. - Security: Absolute paths properly redacted in logs (the CodeRabbit finding was addressed).
encodeURIComponenton the deep link path. Config-dir override is sanitized (trim + empty check) before use. - Patterns: Component follows existing conventions —
useCallbackdeps are correct, toast/reveal patterns match the rest ofMemoryWorkspace,revealWorkspacePathreuse from #2527. - Path matching:
lexically_normalize+is_ancestor_or_equalis intentionally lexical-only (no canonicalize), case-sensitive. A false negative on macOS case-insensitive volumes is conservative — the "Open anyway" escape hatch covers it. - CodeRabbit dedup: All 6 CodeRabbit findings (openUrl error handling, localized placeholder, empty vault path guard, stale guidance panel, log redaction, blank override normalization) were addressed in follow-up commits. Nothing to repeat.
No critical or major issues found. LGTM — moving to to-be-approved.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/openhuman/memory_store/content/obsidian_registry.rs`:
- Around line 30-33: The docstring for the ObsidianRegistry field config_found
is inaccurate: the code sets config_found = true when the obsidian.json file is
successfully read even if parsing later fails, so update the comment on
config_found to reflect "file found/read" (or equivalent wording such as
"found/read, not necessarily parsed") rather than "found and parsed" so callers
won't be misled; locate the config_found field on the ObsidianRegistry struct
and change its doc text to match the actual probe semantics.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ff8dc267-b652-457d-a9c0-d0b7b53701dd
📒 Files selected for processing (26)
app/src/components/intelligence/MemoryWorkspace.tsxapp/src/components/intelligence/ObsidianVaultSection.tsxapp/src/components/intelligence/__tests__/MemoryWorkspace.test.tsxapp/src/components/intelligence/__tests__/ObsidianVaultSection.test.tsxapp/src/lib/i18n/chunks/ar-3.tsapp/src/lib/i18n/chunks/bn-3.tsapp/src/lib/i18n/chunks/de-3.tsapp/src/lib/i18n/chunks/en-3.tsapp/src/lib/i18n/chunks/es-3.tsapp/src/lib/i18n/chunks/fr-3.tsapp/src/lib/i18n/chunks/hi-3.tsapp/src/lib/i18n/chunks/id-3.tsapp/src/lib/i18n/chunks/it-3.tsapp/src/lib/i18n/chunks/ko-3.tsapp/src/lib/i18n/chunks/pt-3.tsapp/src/lib/i18n/chunks/ru-3.tsapp/src/lib/i18n/chunks/zh-CN-3.tsapp/src/lib/i18n/en.tsapp/src/utils/tauriCommands/memoryTree.test.tsapp/src/utils/tauriCommands/memoryTree.tsgitbooks/features/obsidian-wiki/README.mdsrc/openhuman/memory/ops/documents.rssrc/openhuman/memory/read_rpc.rssrc/openhuman/memory/schema.rssrc/openhuman/memory_store/content/mod.rssrc/openhuman/memory_store/content/obsidian_registry.rs
✅ Files skipped from review due to trivial changes (10)
- app/src/lib/i18n/chunks/ar-3.ts
- app/src/lib/i18n/chunks/ko-3.ts
- app/src/lib/i18n/chunks/de-3.ts
- app/src/lib/i18n/en.ts
- app/src/lib/i18n/chunks/ru-3.ts
- app/src/lib/i18n/chunks/es-3.ts
- app/src/lib/i18n/chunks/pt-3.ts
- app/src/lib/i18n/chunks/bn-3.ts
- src/openhuman/memory_store/content/mod.rs
- app/src/lib/i18n/chunks/it-3.ts
CodeRabbit: VaultRegistration::config_found is set true on a successful read of a candidate obsidian.json even when parsing that file then fails (the parse-error branch still counts it as found). The previous doc said "found and parsed", which mis-describes the contract. Doc-only. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/openhuman/memory/schema.rs (1)
641-642:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSchema comment for
config_foundis stricter than implementation.At Line 641, “found and parsed” conflicts with backend behavior (
config_foundbecomes true after successful read, even if parsing fails). Please keep schema docs aligned with runtime semantics.📝 Suggested schema comment update
- comment: "True when an obsidian.json was found and parsed (Obsidian is \ - set up). Lets the UI offer add-as-vault vs. install.", + comment: "True when at least one candidate obsidian.json was found/read \ + (even if parsing fails). Lets the UI offer add-as-vault vs. install.",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/memory/schema.rs` around lines 641 - 642, Update the schema comment for the config_found field to match runtime semantics: change the wording that currently reads "True when an obsidian.json was found and parsed" to indicate that config_found is set to true after the obsidian.json file is successfully read (even if parsing fails). Locate the comment adjacent to the config_found field in schema.rs and replace "found and parsed" with wording such as "found/read (parsing may still fail)" or similar to accurately reflect the implementation in the code that sets config_found on successful read.src/openhuman/memory/read_rpc.rs (1)
1018-1020:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
config_founddocs overstate the actual probe semantics.At Line 1018, the comment says “found and parsed,” but probe logic marks
config_found=trueonce a candidate file is read, even if parsing fails. Please align this wording with runtime behavior.📝 Suggested doc fix
- /// `true` when an `obsidian.json` was found and parsed (Obsidian is set - /// up). Lets the UI offer "Open folder as vault" vs. "Install Obsidian". + /// `true` when at least one candidate `obsidian.json` was found/read + /// (even if parsing later fails). Lets the UI offer "Open folder as vault" + /// vs. "Install Obsidian".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/memory/read_rpc.rs` around lines 1018 - 1020, The field doc for config_found currently says "found and parsed" but the probe actually sets config_found = true as soon as a candidate obsidian.json is read even when parsing fails; update the comment on the config_found field to reflect that it indicates a candidate obsidian.json was discovered/read (not necessarily successfully parsed) so the UI can distinguish "file present" vs "parse succeeded" semantics for obsidian.json.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@src/openhuman/memory/read_rpc.rs`:
- Around line 1018-1020: The field doc for config_found currently says "found
and parsed" but the probe actually sets config_found = true as soon as a
candidate obsidian.json is read even when parsing fails; update the comment on
the config_found field to reflect that it indicates a candidate obsidian.json
was discovered/read (not necessarily successfully parsed) so the UI can
distinguish "file present" vs "parse succeeded" semantics for obsidian.json.
In `@src/openhuman/memory/schema.rs`:
- Around line 641-642: Update the schema comment for the config_found field to
match runtime semantics: change the wording that currently reads "True when an
obsidian.json was found and parsed" to indicate that config_found is set to true
after the obsidian.json file is successfully read (even if parsing fails).
Locate the comment adjacent to the config_found field in schema.rs and replace
"found and parsed" with wording such as "found/read (parsing may still fail)" or
similar to accurately reflect the implementation in the code that sets
config_found on successful read.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: abfe983b-1972-4d06-9324-110a3d16699f
📒 Files selected for processing (26)
app/src/components/intelligence/MemoryWorkspace.tsxapp/src/components/intelligence/ObsidianVaultSection.tsxapp/src/components/intelligence/__tests__/MemoryWorkspace.test.tsxapp/src/components/intelligence/__tests__/ObsidianVaultSection.test.tsxapp/src/lib/i18n/chunks/ar-3.tsapp/src/lib/i18n/chunks/bn-3.tsapp/src/lib/i18n/chunks/de-3.tsapp/src/lib/i18n/chunks/en-3.tsapp/src/lib/i18n/chunks/es-3.tsapp/src/lib/i18n/chunks/fr-3.tsapp/src/lib/i18n/chunks/hi-3.tsapp/src/lib/i18n/chunks/id-3.tsapp/src/lib/i18n/chunks/it-3.tsapp/src/lib/i18n/chunks/ko-3.tsapp/src/lib/i18n/chunks/pt-3.tsapp/src/lib/i18n/chunks/ru-3.tsapp/src/lib/i18n/chunks/zh-CN-3.tsapp/src/lib/i18n/en.tsapp/src/utils/tauriCommands/memoryTree.test.tsapp/src/utils/tauriCommands/memoryTree.tsgitbooks/features/obsidian-wiki/README.mdsrc/openhuman/memory/ops/documents.rssrc/openhuman/memory/read_rpc.rssrc/openhuman/memory/schema.rssrc/openhuman/memory_store/content/mod.rssrc/openhuman/memory_store/content/obsidian_registry.rs
✅ Files skipped from review due to trivial changes (12)
- app/src/lib/i18n/chunks/ko-3.ts
- app/src/lib/i18n/chunks/pt-3.ts
- app/src/lib/i18n/chunks/ar-3.ts
- app/src/lib/i18n/chunks/fr-3.ts
- app/src/lib/i18n/chunks/en-3.ts
- gitbooks/features/obsidian-wiki/README.md
- app/src/lib/i18n/chunks/hi-3.ts
- app/src/lib/i18n/chunks/de-3.ts
- app/src/lib/i18n/chunks/it-3.ts
- app/src/lib/i18n/en.ts
- app/src/lib/i18n/chunks/ru-3.ts
- app/src/lib/i18n/chunks/bn-3.ts
Last remaining Memory-tab surface that was constructing an `obsidian://open?path=<abs>` URL straight from a graph-export RPC field without going through the shared workspace-link layer introduced in PR tinyhumansai#2476. MemoryGraph already migrated to `openWorkspacePath` / `previewWorkspaceText` in PR tinyhumansai#2638; this finishes the migration called out in issue tinyhumansai#2492 by resolving the vault's absolute path through the new `resolveWorkspaceAbsolutePath` wrapper (which delegates to the same canonicalize + workspace-containment guard the rest of the layer uses). Behavior is unchanged from the user's point of view — the click still fires `obsidian://open?path=<vault-abs>` and the toast still names the vault path — but the absolute path is now derived from the Rust-side resolver instead of trusting whatever the RPC handed back. The `contentRootAbs` prop is preserved for display (tooltip, guidance-panel code block, toast message) so no cascading prop changes are needed. Adds two tests covering the new path: (1) the deep link URL is built from the resolver output, not the prop, and (2) a resolver rejection surfaces an error toast and keeps the guidance panel expanded so the user retains an escape hatch. MemoryWorkspace.test.tsx is updated to mock `resolveWorkspaceAbsolutePath` so the existing `openUrl` assertion remains stable. Closes tinyhumansai#2492
Last remaining Memory-tab surface that was constructing an `obsidian://open?path=<abs>` URL straight from a graph-export RPC field without going through the shared workspace-link layer introduced in PR tinyhumansai#2476. MemoryGraph already migrated to `openWorkspacePath` / `previewWorkspaceText` in PR tinyhumansai#2638; this finishes the migration called out in issue tinyhumansai#2492 by resolving the vault's absolute path through the new `resolveWorkspaceAbsolutePath` wrapper (which delegates to the same canonicalize + workspace-containment guard the rest of the layer uses). Behavior is unchanged from the user's point of view — the click still fires `obsidian://open?path=<vault-abs>` and the toast still names the vault path — but the absolute path is now derived from the Rust-side resolver instead of trusting whatever the RPC handed back. The `contentRootAbs` prop is preserved for display (tooltip, guidance-panel code block, toast message) so no cascading prop changes are needed. Adds two tests covering the new path: (1) the deep link URL is built from the resolver output, not the prop, and (2) a resolver rejection surfaces an error toast and keeps the guidance panel expanded so the user retains an escape hatch. MemoryWorkspace.test.tsx is updated to mock `resolveWorkspaceAbsolutePath` so the existing `openUrl` assertion remains stable. Closes tinyhumansai#2492
Summary
obsidian://open?path=…deep link, so a first-time click no longer dead-ends on Obsidian's "Unable to find a vault for the URL".memory_tree_obsidian_vault_statusbacked by a best-effortobsidian.jsondetector (standard config dir + Flatpak/Snap + an optional config-dir override).ObsidianVaultSection: registered → opens directly; otherwise a progressive-disclosure guidance panel — Reveal Folder · Open in Obsidian anyway · Install Obsidian · Advanced ▸ config-folder override.read_rpc.rsthat claimed the deep link resolves arbitrary paths without registration; updated the obsidian-wiki doc to describe the one-time "Open folder as vault" step.Problem
obsidian://open?path=<folder>only resolves folders already present in Obsidian'sobsidian.jsonregistry — the scheme cannot register a new vault, and a.obsidian/folder on disk isn't enough. So the first-ever "View Vault" click launched Obsidian onto "Unable to find a vault for the URL." The prior #2281 fix made the failure visible (toast + Reveal Folder) but still couldn't actually open the vault, because the click fired the deep link unconditionally.Solution
src/openhuman/memory_store/content/obsidian_registry.rs): readsobsidian.jsonacross the standard per-OS config dir + Flatpak/Snap sandbox paths + an optional override; ancestor-aware path match; defensive parse; never errors (a probe miss →registered = false). Exposed viamemory_tree_obsidian_vault_status(controller inmemory/schema.rs, handler inmemory/read_rpc.rs).ObsidianVaultSection.tsx): the click runs the status check at click-time; registered → fire the deep link; not registered → expand inline guidance instead of a doomed link. "Open in Obsidian anyway" and the persisted config-folder override are escape hatches for installs detection can't see (Flatpak/Snap/portable) — a false "not registered" never blocks the user.revealWorkspacePathintroduced by feat(memory): route summaries through workspace paths #2527.Submission Checklist
vitest+cargo testlocally (green); the mergeddiff-covergate runs in CI.N/A: refinement of the existing "View Vault" surface; no new top-level feature row.## Related—N/A: behaviour refinement, no matrix ID.openUrl/revealWorkspacePathare existing.N/A: no new release-cut surface (same Memory-tab button, refined behaviour).Closes #NNN—N/A: no tracking issue exists; the bug was found in-session and is still live onmain.Impact
obsidian.json(never writes another app's config). Best-effort with safe fallbacks, so it can't block "View Vault" even when detection is wrong.Related
MemoryGraph.tsxshares the same root cause and is intentionally left for the Migrate Memory surfaces to shared workspace file links #2492 migration of memory surfaces toworkspace://links.AI Authored PR Metadata
Linear Issue
Commit & Branch
fix/obsidian-vault-registrationaad650d1ec0562486bfb49f20738e79376e59d52Validation Run
pnpm --filter openhuman-app format:check— ran Prettier--checkon changed files (green) +cargo fmt --checkon changed Rust (green); did not run the full-workspace format:check.pnpm typecheck— green (whole app, exit 0).ObsidianVaultSection.test.tsx,MemoryWorkspace.test.tsx,memoryTree.test.ts→ 49 passing;cargo test --lib obsidian→ 17 passing;pnpm i18n:check→ green (per-chunk parity across all 13 locales).cargo fmt --check+cargo check --libgreen.N/A— no Tauri-shell changes.Validation Blocked
command:pre-push hook (pnpm rust:check+lint:commands-tokens)error:vendoredapp/src-tauri/vendor/tauri-cef/.../Cargo.tomlabsent in the worktree;ripgrepnot installedimpact:unrelated to this diff — no Tauri-shell orcomponents/commandschanges. Pushed with--no-verify; the relevant gates (corecargo check/tests,tsc, ESLint, Prettier, Vitest,i18n:check) were all run and are green.Behavior Changes
Parity Contract
obsidian://open?path=deep link; the View Vault button does nothing in memory tree #2281 toast + Reveal action remain (Reveal now via the sharedrevealWorkspacePath). Behaviour only diverges when the vault isn't registered/detected.Duplicate / Superseded PR Handling
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
Translations
Tests
Documentation