mirror of
https://github.com/logancyang/obsidian-copilot.git
synced 2026-07-22 07:50:24 +00:00
fix(agent-mode): stop tool-path clicks from creating phantom vault folders (#2599)
Clicking the path on a Read/Edit tool-call card for a file outside the vault (e.g. /tmp/chap1.txt) called openLinkText directly, so Obsidian dropped the leading slash and materialized a phantom note + folder chain inside the vault. The markdown-link handler already guarded against this; extract that logic into a shared openVaultPath helper and route both the tool-call card and the rendered markdown links through it, so every agent-response click target opens outside-vault absolutes in the OS app. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
95d4fe380f
commit
893406c9bd
4 changed files with 161 additions and 28 deletions
|
|
@ -5,6 +5,7 @@ import type { AgentToolStatus } from "@/agentMode/session/types";
|
|||
import { lookupToolSummary } from "@/agentMode/ui/toolSummaries";
|
||||
import { renderDiff } from "@/agentMode/ui/diffRender";
|
||||
import { getVaultBase } from "@/utils/vaultPath";
|
||||
import { openVaultPath } from "@/utils/openVaultPath";
|
||||
import { cn } from "@/lib/utils";
|
||||
import { useApp } from "@/context";
|
||||
|
||||
|
|
@ -57,7 +58,11 @@ export const ActionCard: React.FC<ActionCardProps> = ({ part, inline }) => {
|
|||
onClick={(e) => {
|
||||
e.preventDefault();
|
||||
e.stopPropagation();
|
||||
void app.workspace.openLinkText(targetPath, "", true);
|
||||
// `targetPath` is vault-relative for in-vault notes but stays
|
||||
// absolute for files the agent read/wrote outside the vault.
|
||||
// Route through `openVaultPath` so an outside-vault path opens
|
||||
// in the OS app instead of fabricating a phantom vault folder.
|
||||
openVaultPath(app, targetPath, { newLeaf: true });
|
||||
}}
|
||||
>
|
||||
{line}
|
||||
|
|
|
|||
96
src/utils/openVaultPath.test.ts
Normal file
96
src/utils/openVaultPath.test.ts
Normal file
|
|
@ -0,0 +1,96 @@
|
|||
import { openVaultPath } from "@/utils/openVaultPath";
|
||||
import { openWithSystemDefault } from "@/utils/openWithSystemDefault";
|
||||
import { __resetVaultBaseCache } from "@/utils/vaultPath";
|
||||
import { App, FileSystemAdapter } from "obsidian";
|
||||
|
||||
jest.mock("@/logger", () => ({ logInfo: jest.fn(), logWarn: jest.fn(), logError: jest.fn() }));
|
||||
jest.mock("@/utils/openWithSystemDefault", () => ({ openWithSystemDefault: jest.fn() }));
|
||||
jest.mock("obsidian", () => ({
|
||||
FileSystemAdapter: class FileSystemAdapter {
|
||||
private readonly base: string;
|
||||
constructor(base = "/vault") {
|
||||
this.base = base;
|
||||
}
|
||||
getBasePath(): string {
|
||||
return this.base;
|
||||
}
|
||||
},
|
||||
Notice: jest.fn(),
|
||||
}));
|
||||
|
||||
const VAULT = "/Users/me/vault";
|
||||
|
||||
interface TestApp {
|
||||
workspace: { openLinkText: jest.Mock };
|
||||
vault: { adapter: unknown; getAbstractFileByPath: jest.Mock };
|
||||
}
|
||||
|
||||
function buildApp(base = VAULT, indexedFiles: string[] = []): TestApp {
|
||||
const adapter = new (FileSystemAdapter as unknown as new (b: string) => unknown)(base);
|
||||
return {
|
||||
workspace: { openLinkText: jest.fn() },
|
||||
vault: {
|
||||
adapter,
|
||||
getAbstractFileByPath: jest.fn((path: string) =>
|
||||
indexedFiles.includes(path) ? { path } : null
|
||||
),
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
function open(app: TestApp, path: string, newLeaf = false): void {
|
||||
openVaultPath(app as unknown as App, path, { newLeaf });
|
||||
}
|
||||
|
||||
describe("openVaultPath", () => {
|
||||
beforeEach(() => {
|
||||
__resetVaultBaseCache();
|
||||
jest.clearAllMocks();
|
||||
});
|
||||
|
||||
it("converts an absolute in-vault path to vault-relative before openLinkText", () => {
|
||||
const app = buildApp();
|
||||
open(app, "/Users/me/vault/00_Inbox/Foo.md");
|
||||
expect(app.workspace.openLinkText).toHaveBeenCalledWith("00_Inbox/Foo.md", "", false);
|
||||
expect(openWithSystemDefault).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("hands an absolute path outside the vault to the OS — never openLinkText", () => {
|
||||
const app = buildApp();
|
||||
open(app, "/tmp/chap1.txt");
|
||||
expect(app.workspace.openLinkText).not.toHaveBeenCalled();
|
||||
expect(openWithSystemDefault).toHaveBeenCalledWith("/tmp/chap1.txt");
|
||||
});
|
||||
|
||||
it("passes a plain relative path through unchanged", () => {
|
||||
const app = buildApp();
|
||||
open(app, "00_Inbox/Foo.md");
|
||||
expect(app.workspace.openLinkText).toHaveBeenCalledWith("00_Inbox/Foo.md", "", false);
|
||||
expect(openWithSystemDefault).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("opens a root-relative link to an existing vault file through openLinkText", () => {
|
||||
const app = buildApp(VAULT, ["Folder/Foo.md"]);
|
||||
open(app, "/Folder/Foo.md");
|
||||
expect(app.workspace.openLinkText).toHaveBeenCalledWith("Folder/Foo.md", "", false);
|
||||
expect(openWithSystemDefault).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("preserves the heading anchor on a root-relative vault link", () => {
|
||||
const app = buildApp(VAULT, ["Folder/Foo.md"]);
|
||||
open(app, "/Folder/Foo.md#Heading");
|
||||
expect(app.workspace.openLinkText).toHaveBeenCalledWith("Folder/Foo.md#Heading", "", false);
|
||||
});
|
||||
|
||||
it("propagates newLeaf to openLinkText", () => {
|
||||
const app = buildApp();
|
||||
open(app, "/Users/me/vault/00_Inbox/Foo.md", true);
|
||||
expect(app.workspace.openLinkText).toHaveBeenCalledWith("00_Inbox/Foo.md", "", true);
|
||||
});
|
||||
|
||||
it("forwards an explicit sourcePath to openLinkText", () => {
|
||||
const app = buildApp();
|
||||
openVaultPath(app as unknown as App, "Some Note", { sourcePath: "source.md" });
|
||||
expect(app.workspace.openLinkText).toHaveBeenCalledWith("Some Note", "source.md", false);
|
||||
});
|
||||
});
|
||||
53
src/utils/openVaultPath.ts
Normal file
53
src/utils/openVaultPath.ts
Normal file
|
|
@ -0,0 +1,53 @@
|
|||
import { openWithSystemDefault } from "@/utils/openWithSystemDefault";
|
||||
import { getVaultBase, isAbsolutePath, toVaultRelative } from "@/utils/vaultPath";
|
||||
import { App } from "obsidian";
|
||||
|
||||
export interface OpenVaultPathOptions {
|
||||
/** Open in a new tab (middle/cmd/ctrl-click). Defaults to false. */
|
||||
newLeaf?: boolean;
|
||||
/** Source note for link resolution, passed through to `openLinkText`. */
|
||||
sourcePath?: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Open a path emitted by a coding agent. Relative paths and absolute paths
|
||||
* inside the vault route through `openLinkText`; absolute paths outside the
|
||||
* vault are handed to the OS default app so `openLinkText` can't fabricate a
|
||||
* phantom note + folder chain from an unresolved target. Shared by every
|
||||
* agent-response surface that turns a path into a click target (rendered
|
||||
* markdown links, tool-call cards).
|
||||
*
|
||||
* Callers that source the path from a URL-encoded DOM `href` must
|
||||
* `decodeURIComponent` first — decoding is correct only for encoded hrefs,
|
||||
* not for raw filesystem paths (a real file can contain a literal `%`).
|
||||
*/
|
||||
export function openVaultPath(app: App, rawPath: string, opts: OpenVaultPathOptions = {}): void {
|
||||
let path = toExistingRootRelativeVaultPath(app, rawPath) ?? rawPath;
|
||||
if (isAbsolutePath(path)) {
|
||||
const rel = toVaultRelative(path, getVaultBase(app));
|
||||
if (rel === path) {
|
||||
// Absolute path outside the vault — don't let openLinkText fabricate a
|
||||
// phantom note; hand off to the OS default app instead.
|
||||
void openWithSystemDefault(path);
|
||||
return;
|
||||
}
|
||||
path = rel;
|
||||
}
|
||||
void app.workspace.openLinkText(path, opts.sourcePath ?? "", opts.newLeaf ?? false);
|
||||
}
|
||||
|
||||
/**
|
||||
* If `href` is a root-relative link (`/Folder/Foo.md`, optionally with a
|
||||
* `#heading`) whose file actually exists in the vault, return it stripped of
|
||||
* the leading slash so `openLinkText` resolves it. Returns null otherwise so
|
||||
* the caller can fall through to absolute-path handling.
|
||||
*/
|
||||
function toExistingRootRelativeVaultPath(app: App, href: string): string | null {
|
||||
if (!href.startsWith("/") || href.startsWith("//")) return null;
|
||||
const rel = href.replace(/^\/+/, "");
|
||||
if (!rel) return null;
|
||||
const anchorIndex = rel.indexOf("#");
|
||||
const filePath = anchorIndex === -1 ? rel : rel.slice(0, anchorIndex);
|
||||
if (!filePath) return null;
|
||||
return app.vault.getAbstractFileByPath(filePath) ? rel : null;
|
||||
}
|
||||
|
|
@ -1,5 +1,4 @@
|
|||
import { openWithSystemDefault } from "@/utils/openWithSystemDefault";
|
||||
import { getVaultBase, isAbsolutePath, toVaultRelative } from "@/utils/vaultPath";
|
||||
import { openVaultPath } from "@/utils/openVaultPath";
|
||||
import { App, Component, MarkdownRenderer } from "obsidian";
|
||||
|
||||
/**
|
||||
|
|
@ -66,28 +65,18 @@ function wireInternalLinks(
|
|||
// Coding agents that run with the vault as their cwd emit links with the
|
||||
// note's absolute on-disk path (e.g. `/Users/me/Vault/Inbox/Foo.md`).
|
||||
// `openLinkText` resolves its target relative to the vault root, dropping
|
||||
// the leading slash — so an absolute path becomes a bogus vault-relative
|
||||
// path that materializes the whole `Users/me/Vault/...` folder chain as a
|
||||
// phantom note. Normalize before opening; mirror `ActionCard`.
|
||||
// the leading slash — so an absolute path outside the vault would
|
||||
// materialize the whole `Users/me/...` folder chain as a phantom note.
|
||||
// `openVaultPath` routes those to the OS instead. Decode first: the DOM
|
||||
// href is percent-encoded (`Foo%20Bar.md`), unlike a raw filesystem path.
|
||||
let href = raw;
|
||||
try {
|
||||
href = decodeURIComponent(raw);
|
||||
} catch {
|
||||
// keep raw on malformed percent escapes
|
||||
}
|
||||
href = toExistingRootRelativeVaultPath(app, href) ?? href;
|
||||
if (isAbsolutePath(href)) {
|
||||
const rel = toVaultRelative(href, getVaultBase(app));
|
||||
if (rel === href) {
|
||||
// Absolute path outside the vault — don't let openLinkText fabricate a
|
||||
// phantom note; hand off to the OS default app instead.
|
||||
void openWithSystemDefault(href);
|
||||
return;
|
||||
}
|
||||
href = rel;
|
||||
}
|
||||
const newLeaf = e.button === 1 || e.ctrlKey || e.metaKey;
|
||||
void app.workspace.openLinkText(href, sourcePath, newLeaf);
|
||||
openVaultPath(app, href, { newLeaf, sourcePath });
|
||||
};
|
||||
el.addEventListener("click", handleClick);
|
||||
el.addEventListener("auxclick", handleClick);
|
||||
|
|
@ -96,13 +85,3 @@ function wireInternalLinks(
|
|||
el.removeEventListener("auxclick", handleClick);
|
||||
});
|
||||
}
|
||||
|
||||
function toExistingRootRelativeVaultPath(app: App, href: string): string | null {
|
||||
if (!href.startsWith("/") || href.startsWith("//")) return null;
|
||||
const rel = href.replace(/^\/+/, "");
|
||||
if (!rel) return null;
|
||||
const anchorIndex = rel.indexOf("#");
|
||||
const filePath = anchorIndex === -1 ? rel : rel.slice(0, anchorIndex);
|
||||
if (!filePath) return null;
|
||||
return app.vault.getAbstractFileByPath(filePath) ? rel : null;
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue