From cd6acd557d0b922b38221ca60562ea0f13492593 Mon Sep 17 00:00:00 2001 From: YOMXXX Date: Fri, 29 May 2026 11:30:57 +0800 Subject: [PATCH] refactor(memory): route Obsidian deep link through workspace-link layer (#2492) (#2702) Co-authored-by: Claude Co-authored-by: Happy --- .../permissions/allow-workspace-files.toml | 1 + app/src-tauri/src/lib.rs | 1 + app/src-tauri/src/workspace_paths.rs | 52 ++++++++++++++++++- .../intelligence/ObsidianVaultSection.tsx | 24 +++++++-- .../__tests__/MemoryWorkspace.test.tsx | 5 ++ .../__tests__/ObsidianVaultSection.test.tsx | 42 ++++++++++++++- .../tauriCommands/workspacePaths.test.ts | 39 +++++++++++++- app/src/utils/tauriCommands/workspacePaths.ts | 12 +++++ 8 files changed, 168 insertions(+), 8 deletions(-) diff --git a/app/src-tauri/permissions/allow-workspace-files.toml b/app/src-tauri/permissions/allow-workspace-files.toml index 1d1e08148..c4328947b 100644 --- a/app/src-tauri/permissions/allow-workspace-files.toml +++ b/app/src-tauri/permissions/allow-workspace-files.toml @@ -8,6 +8,7 @@ allow = [ "open_workspace_path", "reveal_workspace_path", "preview_workspace_text", + "resolve_workspace_absolute_path", ] deny = [] diff --git a/app/src-tauri/src/lib.rs b/app/src-tauri/src/lib.rs index 4f74a6cf5..78b0f61b2 100644 --- a/app/src-tauri/src/lib.rs +++ b/app/src-tauri/src/lib.rs @@ -3408,6 +3408,7 @@ pub fn run() { workspace_paths::open_workspace_path, workspace_paths::reveal_workspace_path, workspace_paths::preview_workspace_text, + workspace_paths::resolve_workspace_absolute_path, meet_call::meet_call_open_window, meet_call::meet_call_close_window, companion_commands::register_companion_hotkey, diff --git a/app/src-tauri/src/workspace_paths.rs b/app/src-tauri/src/workspace_paths.rs index 7cbe7b27f..bd5491572 100644 --- a/app/src-tauri/src/workspace_paths.rs +++ b/app/src-tauri/src/workspace_paths.rs @@ -51,6 +51,29 @@ pub async fn preview_workspace_text(path: String) -> Result`) can route through the shared workspace-link +/// layer instead of re-implementing path normalization in the renderer. +/// +/// Errors mirror the other workspace-path commands — empty input, parent-dir +/// escape, NUL bytes, URI-scheme prefixes, paths outside the workspace, and +/// missing files all surface a non-leaky message. +#[tauri::command] +pub async fn resolve_workspace_absolute_path(path: String) -> Result { + let workspace = active_workspace_root().await?; + let target = resolve_workspace_path(&workspace, &path)?; + let workspace_label = workspace_path_label(&workspace, &target); + log::debug!( + "[workspace-paths] resolve_workspace_absolute_path: {}", + workspace_label + ); + Ok(target.to_string_lossy().into_owned()) +} + async fn active_workspace_root() -> Result { let config = openhuman_core::openhuman::config::Config::load_or_init() .await @@ -212,7 +235,7 @@ pub(crate) fn resolve_workspace_path( log::debug!( "[workspace-paths] resolved workspace path: {} -> {}", normalized_path, - target.display() + workspace_path_label(workspace_root, &target) ); Ok(target) } @@ -400,6 +423,33 @@ mod tests { ); } + #[test] + fn resolve_workspace_path_resolves_memory_tree_content_inside_workspace() { + let workspace = tempdir().unwrap(); + let docs = workspace.path().join("memory_tree").join("content"); + fs::create_dir_all(&docs).unwrap(); + + let resolved = resolve_workspace_path(workspace.path(), "memory_tree/content").unwrap(); + + let canonical_root = fs::canonicalize(workspace.path()).unwrap(); + assert!( + resolved.starts_with(&canonical_root), + "resolved path escaped workspace root: {} not under {}", + resolved.display(), + canonical_root.display() + ); + assert_eq!(resolved, docs.canonicalize().unwrap()); + } + + #[test] + fn resolve_workspace_path_rejects_empty_whitespace_input() { + let workspace = tempdir().unwrap(); + + let err = resolve_workspace_path(workspace.path(), " ").unwrap_err(); + + assert!(err.contains("empty"), "unexpected error: {err}"); + } + #[cfg(unix)] #[test] fn resolve_workspace_path_rejects_symlink_escape() { diff --git a/app/src/components/intelligence/ObsidianVaultSection.tsx b/app/src/components/intelligence/ObsidianVaultSection.tsx index 7cbd7ec54..4af8874dd 100644 --- a/app/src/components/intelligence/ObsidianVaultSection.tsx +++ b/app/src/components/intelligence/ObsidianVaultSection.tsx @@ -25,7 +25,10 @@ import { useT } from '../../lib/i18n/I18nContext'; import type { ToastNotification } from '../../types/intelligence'; import { openUrl } from '../../utils/openUrl'; import { memoryTreeObsidianVaultStatus } from '../../utils/tauriCommands'; -import { revealWorkspacePath } from '../../utils/tauriCommands/workspacePaths'; +import { + resolveWorkspaceAbsolutePath, + revealWorkspacePath, +} from '../../utils/tauriCommands/workspacePaths'; import { MEMORY_CONTENT_WORKSPACE_PATH } from './memoryWorkspacePaths'; /** localStorage key for the optional Obsidian config-dir override. */ @@ -55,18 +58,29 @@ export function ObsidianVaultSection({ contentRootAbs, onToast }: ObsidianVaultS const [showAdvanced, setShowAdvanced] = useState(false); const [configDir, setConfigDir] = useState(readConfigDirOverride); - /** Build + fire the `obsidian://` deep link. Resolves to an error or null. */ + /** + * Build + fire the `obsidian://` deep link. + * + * The absolute path is resolved through the shared workspace-link layer + * (`resolve_workspace_absolute_path` Tauri command) rather than reused from + * the `contentRootAbs` prop — this routes Obsidian deep links through the + * same canonicalize + workspace-containment guard as `open_workspace_path` + * and `preview_workspace_text` (see issue #2492 / #2476). + * + * Resolves to an error or null. + */ const fireDeepLink = useCallback(async (): Promise => { - const url = `obsidian://open?path=${encodeURIComponent(contentRootAbs)}`; console.debug('[ui-flow][obsidian-vault] firing deep link'); try { + const absolutePath = await resolveWorkspaceAbsolutePath(MEMORY_CONTENT_WORKSPACE_PATH); + const url = `obsidian://open?path=${encodeURIComponent(absolutePath)}`; await openUrl(url); return null; } catch (err) { - console.error('[ui-flow][obsidian-vault] openUrl failed', err); + console.error('[ui-flow][obsidian-vault] resolve/open failed', err); return err; } - }, [contentRootAbs]); + }, []); const reveal = useCallback(() => { void (async () => { diff --git a/app/src/components/intelligence/__tests__/MemoryWorkspace.test.tsx b/app/src/components/intelligence/__tests__/MemoryWorkspace.test.tsx index 776259cef..08e7120f8 100644 --- a/app/src/components/intelligence/__tests__/MemoryWorkspace.test.tsx +++ b/app/src/components/intelligence/__tests__/MemoryWorkspace.test.tsx @@ -34,6 +34,11 @@ vi.mock('../../../utils/openUrl', () => ({ openUrl: vi.fn().mockResolvedValue(un vi.mock('../../../utils/tauriCommands/workspacePaths', () => ({ openWorkspacePath: vi.fn().mockResolvedValue(undefined), revealWorkspacePath: vi.fn().mockResolvedValue(undefined), + // #2492: the Obsidian deep link now resolves the vault's absolute path + // through the shared workspace-link layer instead of trusting the + // `content_root_abs` field returned from the graph export RPC. Return the + // same path the prop carries so the existing `openUrl` assertion is stable. + resolveWorkspaceAbsolutePath: vi.fn().mockResolvedValue('/tmp/workspace/memory_tree/content'), previewWorkspaceText: vi .fn() .mockResolvedValue({ diff --git a/app/src/components/intelligence/__tests__/ObsidianVaultSection.test.tsx b/app/src/components/intelligence/__tests__/ObsidianVaultSection.test.tsx index dbf17a689..b9685ed29 100644 --- a/app/src/components/intelligence/__tests__/ObsidianVaultSection.test.tsx +++ b/app/src/components/intelligence/__tests__/ObsidianVaultSection.test.tsx @@ -10,6 +10,7 @@ vi.mock('../../../utils/openUrl', () => ({ openUrl: vi.fn().mockResolvedValue(un vi.mock('../../../utils/tauriCommands/workspacePaths', () => ({ revealWorkspacePath: vi.fn().mockResolvedValue(undefined), + resolveWorkspaceAbsolutePath: vi.fn().mockResolvedValue('/tmp/workspace/memory_tree/content'), })); const { memoryTreeObsidianVaultStatus } = @@ -19,9 +20,10 @@ const { memoryTreeObsidianVaultStatus } = const { openUrl } = (await import('../../../utils/openUrl')) as unknown as { openUrl: Mock }; -const { revealWorkspacePath } = +const { revealWorkspacePath, resolveWorkspaceAbsolutePath } = (await import('../../../utils/tauriCommands/workspacePaths')) as unknown as { revealWorkspacePath: Mock; + resolveWorkspaceAbsolutePath: Mock; }; const ROOT = '/tmp/workspace/memory_tree/content'; @@ -37,6 +39,7 @@ describe('ObsidianVaultSection', () => { localStorage.clear(); openUrl.mockResolvedValue(undefined); revealWorkspacePath.mockResolvedValue(undefined); + resolveWorkspaceAbsolutePath.mockResolvedValue(ROOT); }); it('registered vault → opens the deep link directly, no guidance shown', async () => { @@ -122,4 +125,41 @@ describe('ObsidianVaultSection', () => { await waitFor(() => expect(screen.getByTestId('obsidian-vault-guidance')).toBeInTheDocument()); expect(openUrl).not.toHaveBeenCalled(); }); + + // #2492: the absolute path that feeds the `obsidian://open?path=…` URL must + // come from the shared workspace-link layer (the Rust-side resolver), not + // from the `contentRootAbs` prop. The prop stays around for display, but + // the deep link URL is composed with whatever the resolver returns. + it('deep link uses the workspace-link resolver, not the contentRootAbs prop', async () => { + const resolved = '/private/var/folders/canonical/memory_tree/content'; + resolveWorkspaceAbsolutePath.mockResolvedValue(resolved); + memoryTreeObsidianVaultStatus.mockResolvedValue(status({ registered: true })); + renderWithProviders(); + + fireEvent.click(screen.getByTestId('memory-open-in-obsidian')); + + await waitFor(() => + expect(resolveWorkspaceAbsolutePath).toHaveBeenCalledWith('memory_tree/content') + ); + await waitFor(() => + expect(openUrl).toHaveBeenCalledWith('obsidian://open?path=' + encodeURIComponent(resolved)) + ); + }); + + // #2492: when the Rust-side resolver rejects (e.g. workspace path missing + // on disk), the click must not silently no-op — it surfaces an error toast + // and keeps the guidance panel expanded so the user has an escape hatch. + it('resolver failure surfaces an error toast and keeps guidance expanded', async () => { + resolveWorkspaceAbsolutePath.mockRejectedValue(new Error('workspace path does not exist')); + memoryTreeObsidianVaultStatus.mockResolvedValue(status({ registered: true })); + const onToast = vi.fn(); + renderWithProviders(); + + fireEvent.click(screen.getByTestId('memory-open-in-obsidian')); + + await waitFor(() => expect(onToast).toHaveBeenCalled()); + expect(onToast.mock.calls[0][0].type).toBe('error'); + expect(openUrl).not.toHaveBeenCalled(); + expect(screen.getByTestId('obsidian-vault-guidance')).toBeInTheDocument(); + }); }); diff --git a/app/src/utils/tauriCommands/workspacePaths.test.ts b/app/src/utils/tauriCommands/workspacePaths.test.ts index 4e5c3f9f8..a53b77402 100644 --- a/app/src/utils/tauriCommands/workspacePaths.test.ts +++ b/app/src/utils/tauriCommands/workspacePaths.test.ts @@ -2,7 +2,12 @@ import { invoke } from '@tauri-apps/api/core'; import { beforeEach, describe, expect, test, vi } from 'vitest'; import { isTauri } from './common'; -import { openWorkspacePath, previewWorkspaceText, revealWorkspacePath } from './workspacePaths'; +import { + openWorkspacePath, + previewWorkspaceText, + resolveWorkspaceAbsolutePath, + revealWorkspacePath, +} from './workspacePaths'; vi.mock('@tauri-apps/api/core', () => ({ invoke: vi.fn() })); vi.mock('./common', () => ({ isTauri: vi.fn() })); @@ -61,4 +66,36 @@ describe('tauriCommands/workspacePaths', () => { expect(invoke).toHaveBeenCalledWith('preview_workspace_text', { path: 'docs/readme.md' }); }); + + test('invokes resolve_workspace_absolute_path with a workspace-relative path', async () => { + vi.mocked(invoke).mockResolvedValue('/tmp/workspace/docs/readme.md'); + + await expect(resolveWorkspaceAbsolutePath('docs/readme.md')).resolves.toBe( + '/tmp/workspace/docs/readme.md' + ); + + expect(invoke).toHaveBeenCalledWith('resolve_workspace_absolute_path', { + path: 'docs/readme.md', + }); + }); + + test('surfaces invoke rejection from resolve_workspace_absolute_path', async () => { + vi.mocked(invoke).mockRejectedValue('workspace path does not exist nope.md'); + + await expect(resolveWorkspaceAbsolutePath('nope.md')).rejects.toBe( + 'workspace path does not exist nope.md' + ); + + expect(invoke).toHaveBeenCalledWith('resolve_workspace_absolute_path', { path: 'nope.md' }); + }); + + test('resolveWorkspaceAbsolutePath throws before invoking when not running in Tauri', async () => { + vi.mocked(isTauri).mockReturnValue(false); + + await expect(resolveWorkspaceAbsolutePath('docs/readme.md')).rejects.toThrow( + 'Not running in Tauri' + ); + + expect(invoke).not.toHaveBeenCalled(); + }); }); diff --git a/app/src/utils/tauriCommands/workspacePaths.ts b/app/src/utils/tauriCommands/workspacePaths.ts index 20b104a02..56640258c 100644 --- a/app/src/utils/tauriCommands/workspacePaths.ts +++ b/app/src/utils/tauriCommands/workspacePaths.ts @@ -45,3 +45,15 @@ export async function previewWorkspaceText(path: string): Promise`) without + * re-implementing path normalization in the renderer. + */ +export async function resolveWorkspaceAbsolutePath(path: string): Promise { + assertTauri(); + return invoke('resolve_workspace_absolute_path', { path }); +}