refactor(memory): route Obsidian deep link through workspace-link layer (#2492)#2702
refactor(memory): route Obsidian deep link through workspace-link layer (#2492)#2702YOMXXX wants to merge 8 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds a new Tauri command ChangesWorkspace Absolute Path Resolver
Sequence DiagramsequenceDiagram
participant User
participant ObsidianVaultSection
participant resolveWorkspaceAbsolutePath
participant resolve_workspace_absolute_path
participant ObsidianApp as openUrl
User->>ObsidianVaultSection: click deep-link
ObsidianVaultSection->>resolveWorkspaceAbsolutePath: resolve(MEMORY_CONTENT_WORKSPACE_PATH)
resolveWorkspaceAbsolutePath->>resolve_workspace_absolute_path: invoke
resolve_workspace_absolute_path-->>resolveWorkspaceAbsolutePath: canonical absolute path
ObsidianVaultSection->>ObsidianApp: openUrl("obsidian://open?path="+resolvedPath)
ObsidianApp-->>User: open vault
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels
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 Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
coderabbitai
Bot
added
rust-core
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
graycyrus
left a comment
graycyrus
left a comment
There was a problem hiding this comment.
Review — refactor(memory): route Obsidian deep link through workspace-link layer
Clean, well-scoped refactor. Three thin layers (Rust command → TS wrapper → component plumbing), each mirroring the existing workspace-path pattern exactly. The security posture improves — the Obsidian deep link now goes through the same canonicalize + workspace-containment guard as the other three workspace commands, closing the last gap from #2492.
What I checked
| Area | Verdict |
|---|---|
Rust resolve_workspace_absolute_path |
Delegates to existing resolve_workspace_path — same guard surface, same error semantics. to_string_lossy() is fine for URL composition. Debug log is appropriate. |
| Permission manifest | allow-workspace-files.toml updated, lib.rs registration matches. |
| TS wrapper | Mirrors revealWorkspacePath / openWorkspacePath style. assertTauri() guard present. |
| Component change | fireDeepLink deps array correctly empty — no reactive values captured. contentRootAbs prop preserved for display only. Error path surfaces toast. |
| Tests | 2 new Rust unit tests (happy path + empty input rejection). 2 new Vitest cases (resolver-sourced URL + resolver failure toast). Mock additions in MemoryWorkspace.test.tsx keep 20 existing tests stable. |
| Issue alignment | #2492 acceptance criteria met: Obsidian composed with shared layer, behavior preserved, tests cover open/reveal/failure. Combined with #2638 (MemoryGraph), all criteria are satisfied. |
CI failures — not PR-related
- Frontend:
loopbackOauthListener.test.tstimeout constant mismatch (unrelated module) - Rust Core/Windows:
AutonomySettingsPatchmissing fields inconfig::ops(unrelated module) - Coverage gate skipped due to upstream test failures
None of these touch workspace paths or the Memory tab.
LGTM. Ship it.
YOMXXX
force-pushed
the
fix/2492-memory-workspace-link-migration
branch
from
3682a02 to
0711cbd
Compare
coderabbitai
Bot
added
the
feature
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 `@app/src-tauri/src/workspace_paths.rs`:
- Around line 69-73: The debug log in resolve_workspace_absolute_path currently
prints the absolute target path (variable target), exposing PII; change the log
to redact or replace the absolute path with a workspace-relative label or short
identifier (e.g., compute and log the path relative to the workspace root or
just the target file name), or remove the debug line entirely since
resolve_workspace_path already logs resolution; update the log call in
resolve_workspace_absolute_path (and any use of target.display()) to output only
the redacted/relative value instead of the full absolute path.
🪄 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: 0de62b29-39d8-46f7-bd9b-e1d545c45a2e
📒 Files selected for processing (7)
app/src-tauri/permissions/allow-workspace-files.tomlapp/src-tauri/src/lib.rsapp/src-tauri/src/workspace_paths.rsapp/src/components/intelligence/ObsidianVaultSection.tsxapp/src/components/intelligence/__tests__/MemoryWorkspace.test.tsxapp/src/components/intelligence/__tests__/ObsidianVaultSection.test.tsxapp/src/utils/tauriCommands/workspacePaths.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- app/src/utils/tauriCommands/workspacePaths.ts
- app/src-tauri/src/lib.rs
- app/src/components/intelligence/tests/ObsidianVaultSection.test.tsx
- app/src/components/intelligence/ObsidianVaultSection.tsx
- app/src/components/intelligence/tests/MemoryWorkspace.test.tsx
- app/src-tauri/permissions/allow-workspace-files.toml
Expose the internal `resolve_workspace_path` helper as a Tauri command so UI flows that need a canonical 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. The new command reuses the same canonicalize + workspace-containment guard as `open_workspace_path` / `reveal_workspace_path` / `preview_workspace_text`, registers via `generate_handler!`, and is allow-listed in `allow-workspace-files.toml`. Refs tinyhumansai#2476 (workspace-link layer) Refs tinyhumansai#2492 (memory surfaces migration)
TypeScript wrapper around the new `resolve_workspace_absolute_path` Tauri command. Mirrors the style of the existing `openWorkspacePath` / `revealWorkspacePath` / `previewWorkspaceText` wrappers (assertTauri guard, plain `invoke<string>` call). Refs 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
Adds three Vitest cases for the new resolveWorkspaceAbsolutePath wrapper in app/src/utils/tauriCommands/workspacePaths.ts so the Coverage Gate sees the wrapper's invoke call, rejection propagation, and isTauri guard exercised on the changed lines (PR tinyhumansai#2702).
resolve_workspace_absolute_path was logging target.display(), which includes the canonical absolute workspace root. Route through the existing workspace_path_label helper instead so the debug line follows the same redaction pattern as the other workspace-path commands and does not leak host filesystem layout into logs.
YOMXXX
force-pushed
the
fix/2492-memory-workspace-link-migration
branch
from
0711cbd to
a8a11f6
Compare
|
Addressed CodeRabbit feedback in cffa53a + a8a11f6:
Local verification: |
| } | ||
|
|
||
| #[test] | ||
| fn resolve_workspace_path_returns_absolute_inside_workspace() { |
There was a problem hiding this comment.
Refactor / suggestion — both new tests call resolve_workspace_path(...) directly (the pre-existing internal helper), not the new resolve_workspace_absolute_path command this PR adds. The command own body (config load, resolve, label/log, to_string_lossy) has no direct Rust test — it is only integration-covered via workspacePaths.ts. resolve_workspace_path_returns_absolute_inside_workspace also overlaps the existing resolve_workspace_path_accepts_existing_relative_file_inside_workspace; the genuinely-new coverage is the starts_with(canonical_root) assertion plus the empty-input rejection. The coverage gate passes, so no change is strictly required — but the test names overpromise (they read as if they cover the new command). Consider renaming to reflect that they assert the shared helper the command delegates to, e.g. resolve_workspace_path_resolves_memory_tree_content_inside_workspace.
There was a problem hiding this comment.
Good point — the test names overpromise. They exercise resolve_workspace_path (the shared helper) rather than the new resolve_workspace_absolute_path Tauri command. Will rename to resolve_workspace_path_resolves_memory_tree_content_inside_workspace and resolve_workspace_path_rejects_empty_input to reflect what they actually cover.
| 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); |
There was a problem hiding this comment.
Question — resolve_workspace_path requires the target to exist (fs::canonicalize errors otherwise). Previously fireDeepLink built the URL straight from the contentRootAbs prop and never touched disk, so it fired even when memory_tree/content was absent (Obsidian / Open anyway being the escape hatch). Now, if the folder does not exist, the resolver rejects and the link never fires (error toast instead). The "user-visible behavior is unchanged" claim holds for the registered-vault case (Obsidian can only register a folder that exists) and this aligns fireDeepLink with reveal (which already required existence). But for an unregistered vault whose content root has not been created yet, behavior does change. Is graph.content_root_abs guaranteed to be backed by an existing directory whenever this component renders? If yes, this is purely an improvement and the claim stands.
There was a problem hiding this comment.
Good question. Yes — content_root_abs comes from the memory_tree_graph_export RPC, which writes the graph to disk before returning the path. So by the time ObsidianVaultSection renders with that prop, the directory exists. The resolver call just canonicalizes it for the Obsidian deep link URL.
For the edge case of a completely fresh workspace where the graph hasn't been exported yet: the Memory tab only shows the vault section after a successful export, so the component never renders with a non-existent path in practice.
sanil-23
left a comment
sanil-23
left a comment
There was a problem hiding this comment.
PR #2702 — refactor(memory): route Obsidian deep link through workspace-link layer (#2492)
Walkthrough
This PR finishes the Phase-3 cleanup from #2492 by routing the last Memory-tab deep-link surface (ObsidianVaultSection) through the shared workspace-link layer instead of trusting an RPC-supplied path. It adds a thin resolve_workspace_absolute_path Tauri command wrapping the existing internal resolve_workspace_path guard, a matching resolveWorkspaceAbsolutePath TS wrapper, and rewires fireDeepLink to canonicalize memory_tree/content server-side before building the obsidian://open?path=<abs> URL. The change is tight, well-documented, and net-positive for security. CI is green (the one non-passing check, Rust Core Tests (Windows — secrets ACL), is cancelled and tests core code this PR does not touch). No blockers.
Changes
| File | Summary |
|---|---|
app/src-tauri/permissions/allow-workspace-files.toml |
Allow the new resolve_workspace_absolute_path command in the capability manifest. |
app/src-tauri/src/lib.rs |
Register workspace_paths::resolve_workspace_absolute_path in the invoke handler. |
app/src-tauri/src/workspace_paths.rs |
New resolve_workspace_absolute_path Tauri command wrapping resolve_workspace_path; two new unit tests on the internal helper. |
app/src/utils/tauriCommands/workspacePaths.ts |
New resolveWorkspaceAbsolutePath(path) wrapper (assertTauri + invoke). |
app/src/components/intelligence/ObsidianVaultSection.tsx |
fireDeepLink now resolves the absolute path via the new command instead of the contentRootAbs prop. |
app/src/utils/tauriCommands/workspacePaths.test.ts |
Adds success / rejection / non-Tauri tests for the new wrapper. |
app/src/components/intelligence/__tests__/ObsidianVaultSection.test.tsx |
Adds resolver-sourced-URL and resolver-failure-toast tests; mocks the new wrapper. |
app/src/components/intelligence/__tests__/MemoryWorkspace.test.tsx |
Mocks resolveWorkspaceAbsolutePath to keep the existing deep-link assertion stable. |
Actionable comments (1)
💡 Refactor / suggestion
1. app/src-tauri/src/workspace_paths.rs:426-451 — new tests exercise the internal helper, not the new command. Both new tests call resolve_workspace_path(...) directly (the pre-existing helper), not the new resolve_workspace_absolute_path command this PR adds, whose body (config load, resolve, label/log, to_string_lossy) has no direct Rust test — only integration coverage via workspacePaths.ts. resolve_workspace_path_returns_absolute_inside_workspace also overlaps the existing resolve_workspace_path_accepts_existing_relative_file_inside_workspace; the genuinely-new coverage is the containment assertion plus the empty-input rejection. The coverage gate passes, so no change is strictly required — but the names overpromise. Suggest renaming to reflect they assert the shared helper the command delegates to (e.g. resolve_workspace_path_resolves_memory_tree_content_inside_workspace). Posted inline.
Nitpicks (2)
app/src/components/intelligence/ObsidianVaultSection.tsx:206— the buttontitletooltip still previews the URL built from thecontentRootAbsprop, but the URL actually fired now comes from the canonicalized resolver output. On macOS these diverge (/var/...vs/private/var/...), so the tooltip can show a different path than the link opens. Cosmetic.app/src/components/intelligence/ObsidianVaultSection.tsx:113— on resolver failure the error toast interpolatescontentRootAbs, but the failure concerns the resolved path (which may differ / be missing on disk). They match in the normal case, so low impact; mentioning since the prop is now decoupled from the actual deep-link target.
Questions for the author (1)
app/src/components/intelligence/ObsidianVaultSection.tsx:75—resolve_workspace_pathrequires the target to exist (canonicalize errors otherwise). PreviouslyfireDeepLinkbuilt the URL straight from the prop and never touched disk, so it fired even whenmemory_tree/contentwas absent. Now a missing folder rejects and the link never fires (error toast). The "behavior unchanged" claim holds for the registered case (Obsidian only registers folders that exist) and aligns withreveal(already existence-gated), but the unregistered + not-yet-created case does change. Isgraph.content_root_absalways backed by an existing directory when this component renders? Posted inline.
Verified / looks good
- TS wrapper mirrors its siblings exactly (assertTauri + invoke) and is tested for success, invoke rejection, and the non-Tauri throw.
- Component tests prove the URL is sourced from the resolver (a distinct path from the prop) and that resolver failure surfaces an
errortoast + keeps guidance expanded — happy and failure paths both covered. fireDeepLinkuseCallbackdeps correctly dropped to empty (no longer closes overcontentRootAbs; the path is a module constant).- Permission manifest and
lib.rsinvoke-handler registration both updated — the command is reachable end-to-end. - Security is net-positive: the deep link now passes through the same canonicalize + workspace-containment guard as
open_workspace_path/reveal_workspace_path; errors stay non-leaky. - No new i18n keys required — the failure path reuses existing
workspace.openVaultFailed*toast keys. - Rustdoc on the command and JSDoc on the wrapper both document the why and the error contract.
Approving — no blockers, no majors. The one refactor note and the open question are non-blocking; addressing them (or confirming the existence invariant) would tidy things up.
- ObsidianVaultSection: fall back to `contentRootAbs` prop when `resolveWorkspaceAbsolutePath` rejects (directory doesn't exist yet). Preserves the pre-refactor behavior where the deep link fires even on a fresh workspace — Obsidian surfaces its own "vault not found" error if the folder is missing. - workspace_paths: redact absolute path from the internal `resolve_workspace_path` debug log (uses `workspace_path_label` instead of `target.display()`). Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering>
coderabbitai
Bot
added
the
memory
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/components/intelligence/ObsidianVaultSection.tsx (1)
75-84: ⚡ Quick winConsider logging when the resolver fallback occurs.
The empty catch block silently suppresses all errors from
resolveWorkspaceAbsolutePath, including unexpected failures (Tauri runtime errors, permission issues) beyond just "directory doesn't exist". Adding a debug log would improve observability without changing the fallback behavior.🔍 Proposed observability improvement
try { absolutePath = await resolveWorkspaceAbsolutePath(MEMORY_CONTENT_WORKSPACE_PATH); - } catch { + } catch (err) { + console.debug('[ui-flow][obsidian-vault] resolver failed, falling back to contentRootAbs', err); // The content directory may not exist yet (fresh workspace). Fall // back to the prop from the graph-export RPC — the deep link URL // is still valid; Obsidian will surface its own "vault not found"🤖 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 75 - 84, The catch block swallowing errors from resolveWorkspaceAbsolutePath for MEMORY_CONTENT_WORKSPACE_PATH should log the caught error for observability before falling back to contentRootAbs; update the catch to capture the error (e.g., catch (err)) and emit a debug/info log using the existing logging facility (or console.debug) that includes a brief context message referencing resolveWorkspaceAbsolutePath and MEMORY_CONTENT_WORKSPACE_PATH, then keep the fallback assignment to contentRootAbs unchanged.
🤖 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.
Nitpick comments:
In `@app/src/components/intelligence/ObsidianVaultSection.tsx`:
- Around line 75-84: The catch block swallowing errors from
resolveWorkspaceAbsolutePath for MEMORY_CONTENT_WORKSPACE_PATH should log the
caught error for observability before falling back to contentRootAbs; update the
catch to capture the error (e.g., catch (err)) and emit a debug/info log using
the existing logging facility (or console.debug) that includes a brief context
message referencing resolveWorkspaceAbsolutePath and
MEMORY_CONTENT_WORKSPACE_PATH, then keep the fallback assignment to
contentRootAbs unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 84952518-dfb7-4bb0-8f8f-a11ac909ed98
📒 Files selected for processing (2)
app/src-tauri/src/workspace_paths.rsapp/src/components/intelligence/ObsidianVaultSection.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src-tauri/src/workspace_paths.rs
…t fallback Remove the inner try/catch in fireDeepLink that swallowed resolver rejections and fell back to contentRootAbs. When the workspace-link resolver fails, the error now propagates to toastOpenOutcome which correctly surfaces a type:'error' toast and keeps the guidance panel expanded.
graycyrus
left a comment
graycyrus
left a comment
There was a problem hiding this comment.
@YOMXXX heads up — CI is failing on this PR (Frontend Unit Tests, Frontend Coverage, Rust Core Tests on Windows), so I'll hold off on approving until those are sorted. I did spot one thing while looking through the diff though.
The CI failure in Frontend Unit Tests is almost certainly coming from the new test resolver failure surfaces an error toast and keeps guidance expanded in ObsidianVaultSection.test.tsx. That test expects openUrl to not be called and an error toast to fire when the resolver rejects — but the component's inner catch silently falls back to contentRootAbs and calls openUrl anyway. The test and the implementation directly contradict each other.
Looking at ObsidianVaultSection.tsx around line 75–84: when resolveWorkspaceAbsolutePath throws, the catch block assigns absolutePath = contentRootAbs and execution continues to openUrl(url). The test mocks openUrl as resolved and asserts it was never called — that assertion will always fail given this fallback path. Same for the error toast — with a successful openUrl call, fireDeepLink returns null and no toast fires.
The PR description says resolver rejection should surface an error toast, but the code says it should silently fall back. One of these needs to change. If the intent is graceful degradation (fall back to the RPC path), the test is wrong and should instead verify the fallback URL is used. If the intent is to surface failures as errors (which the test and PR description both imply), the inner catch should re-throw rather than swallow.
Everything else looks good — Rust command is tight, TS wrapper is idiomatic, the PII log fix is correct, permission manifest is in sync, and the security story is net positive. Fix the CI failures and resolve the test/implementation disagreement and this is ready to go.
| // is still valid; Obsidian will surface its own "vault not found" | ||
| // error if the folder is truly missing. | ||
| absolutePath = contentRootAbs; | ||
| } |
There was a problem hiding this comment.
[major] The test for resolver failure (ObsidianVaultSection.test.tsx:153) asserts that openUrl is NOT called and an error toast IS fired when this catch is hit — but this catch falls back to contentRootAbs and openUrl fires successfully, so neither assertion can pass.
Decide which behavior you actually want:
- Graceful degradation (fallback to prop): remove or rework the new
resolver failuretest so it verifies the fallback URL is used instead of asserting an error state. - Fail loudly (surface error toast on resolver rejection): remove the fallback and re-throw so the outer catch handles it as an error.
Right now the test and the code describe different contracts.
There was a problem hiding this comment.
Agreed — the test and code described different contracts. Fixed in 40503a9b: removed the inner fallback catch entirely so resolver rejection propagates to the outer catch → returns error → toastOpenOutcome fires type: "error" → guidance panel stays expanded. This is the "fail loudly" path (option 2).
The rationale: if the resolver can't canonicalize the path, the deep link URL would be unreliable anyway. Better to surface the error and let the user use "Open anyway" (which still uses contentRootAbs directly and skips the resolver).
|
@YOMXXX unresolved review feedback — please address before we review. |
|
Unresolved review feedback from coderabbitai[bot] — please address before we review. |
…helper Addresses sanil-23's review: the tests exercise resolve_workspace_path (the internal helper) not the new resolve_workspace_absolute_path Tauri command, so the names should say what they actually cover.
3 participants
Summary
resolve_workspace_absolute_pathexposes the workspace-link resolver introduced in feat(tauri): support workspace file links #2476 so renderers can derive canonical absolute paths through the same guard asopen_workspace_path/reveal_workspace_path/preview_workspace_text.resolveWorkspaceAbsolutePathinutils/tauriCommands/workspacePaths.ts.ObsidianVaultSectionnow resolves the vault's absolute path through that wrapper before building theobsidian://open?path=<abs>URL, replacing the last Memory-tab surface that was constructing the deep link straight from a graph-export RPC field.Problem
Issue #2492 (Phase 3 cleanup of #1402) requires every Memory surface that constructs an on-disk path to compose with the shared workspace-link layer rather than hand-rolling its own resolution.
MemoryGraphwas migrated in PR #2638. The remaining offender wasObsidianVaultSection, which builtobsidian://open?path=${encodeURIComponent(contentRootAbs)}directly from thecontent_root_absfield returned bymemory_tree_obsidian_vault_status— no canonicalize, no workspace-containment recheck, no shared error surface. Acceptance criterion from the issue:Solution
Three layers, kept thin:
app/src-tauri/src/workspace_paths.rs): newresolve_workspace_absolute_path#[tauri::command]that wraps the existing internalresolve_workspace_pathhelper (same canonicalize + workspace-containment + non-leaky error surface as the other three commands). Registered inlib.rsand added to theallow-workspace-filespermission manifest. Two new unit tests cover the happy-path (memory_tree/contentresolves inside the workspace root) and the empty-input rejection; the existing resolver test suite already covers absolute-path /../ URI-scheme / symlink escapes.app/src/utils/tauriCommands/workspacePaths.ts):resolveWorkspaceAbsolutePath(path)wrapper mirroring the style of the other workspace wrappers (assertTauri guard + plaininvoke<string>).app/src/components/intelligence/ObsidianVaultSection.tsx):fireDeepLinknowawaitsresolveWorkspaceAbsolutePath('memory_tree/content')and builds the URL from the returned absolute path. ThecontentRootAbsprop is preserved for display (button tooltip, guidance-panel code block, toast message text) so no cascading prop changes are needed — only the deep-link URL is rerouted.Tests
Updated
ObsidianVaultSection.test.tsx:workspacePathsmock block.contentRootAbs).Updated
MemoryWorkspace.test.tsx:resolveWorkspaceAbsolutePathto return the same path the assertion expects so the existing'View vault in Obsidian' triggers the deep linktest stays stable. All 20 tests pass.Submission Checklist
resolve_workspace_path_*test suite.Closes #2492in the## Relatedsection.Impact
#[tauri::command]lives in the Tauri shell; iOS client does not hit this code path). No change to JSON-RPC surface.content_root_absvalue from the RPC (or a future scenario where the field is user-influenced) can no longer slip out of the workspace.Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
fix/2492-memory-workspace-link-migration3682a02d(tip),910d8097,8ff810baValidation Run
pnpm --filter openhuman-app format:check— equivalent:pnpm exec prettier --checkon changed files, clean.pnpm typecheck— ranpnpm exec tsc --noEmitinsideapp/, clean.pnpm debug unit ObsidianVaultSection→ 9 passed;pnpm debug unit MemoryWorkspace→ 20 passed.cargo fmt --manifest-path app/src-tauri/Cargo.toml --checkclean;GGML_NATIVE=OFF cargo check --manifest-path app/src-tauri/Cargo.toml --libclean (GGML_NATIVE=OFFis the documented macOS Tahoe + whisper-rs workaround).Validation Blocked
command:n/a — all validation passed.error:n/aimpact:n/aBehavior Changes
contentRootAbs(RPC) because it is a display concern.Parity Contract
obsidian://open?path=<abs>URL shape is byte-for-byte unchanged when the resolver returns the same path the RPC reports (normal case).MemoryWorkspace.test.tsx's existing assertion proves it.detection failure degrades gracefully to the guidance paneltest still passes.Duplicate / Superseded PR Handling
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores