refactor(memory): route Obsidian deep link through workspace-link layer (#2492) (#2702)

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Happy <yesreply@happy.engineering>
This commit is contained in:
YOMXXX
2026-05-28 20:30:57 -07:00
committed by GitHub
co-authored by Claude Happy
parent f5e4c30ecc
commit cd6acd557d
8 changed files with 168 additions and 8 deletions
@@ -8,6 +8,7 @@ allow = [
"open_workspace_path",
"reveal_workspace_path",
"preview_workspace_text",
"resolve_workspace_absolute_path",
]
deny = []
+1
View File
@@ -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,
+51 -1
View File
@@ -51,6 +51,29 @@ pub async fn preview_workspace_text(path: String) -> Result<WorkspaceTextPreview
preview_workspace_text_from_root(&workspace, &path, DEFAULT_PREVIEW_MAX_BYTES)
}
/// Resolve a workspace-relative path to its canonical absolute path on disk,
/// after validating it stays inside the active OpenHuman workspace.
///
/// This exposes the internal [`resolve_workspace_path`] helper so UI flows that
/// need an absolute path to compose with a platform-specific URL scheme (e.g.
/// `obsidian://open?path=<abs>`) 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<String, String> {
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<PathBuf, String> {
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() {
@@ -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<string>(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<unknown | null> => {
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 () => {
@@ -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({
@@ -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(<ObsidianVaultSection contentRootAbs={ROOT} />);
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(<ObsidianVaultSection contentRootAbs={ROOT} onToast={onToast} />);
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();
});
});
@@ -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();
});
});
@@ -45,3 +45,15 @@ export async function previewWorkspaceText(path: string): Promise<WorkspaceTextP
sizeBytes: preview.size_bytes,
};
}
/**
* Resolve a workspace-relative path to its canonical absolute path on disk,
* after the Rust side validates it stays inside the active OpenHuman
* workspace. Useful for UI flows that need to compose an absolute path into a
* platform-specific URL scheme (e.g. `obsidian://open?path=<abs>`) without
* re-implementing path normalization in the renderer.
*/
export async function resolveWorkspaceAbsolutePath(path: string): Promise<string> {
assertTauri();
return invoke<string>('resolve_workspace_absolute_path', { path });
}