diff --git a/packages/app/e2e/browser/file-editing.spec.ts b/packages/app/e2e/browser/file-editing.spec.ts index bd834d6f75..d426a03fb4 100644 --- a/packages/app/e2e/browser/file-editing.spec.ts +++ b/packages/app/e2e/browser/file-editing.spec.ts @@ -7,7 +7,11 @@ import { expectFileTabOpen, } from "../support/helpers/file-explorer"; import { installDaemonWebSocketGate } from "../support/helpers/daemon-websocket-gate"; +import { gotoAppShell } from "../support/helpers/app"; import { openAgentRoute, seedMockAgentWorkspace } from "../support/helpers/mock-agent"; +import { seedWorkspace } from "../support/helpers/seed-client"; +import { getServerId } from "../support/helpers/server-id"; +import { waitForSidebarHydration } from "../support/helpers/workspace-ui"; const APP_SETTINGS_KEY = "@paseo:app-settings"; @@ -92,6 +96,40 @@ async function seedAgentWithFileLink(input: LinkedFile) { } test.describe("CodeMirror workspace file editing", () => { + test("shows an absolute POSIX assistant file link relative to the workspace on hover", async ({ + page, + }) => { + const workspace = await seedWorkspace({ repoPrefix: "file-link-tooltip-" }); + const filePath = path.join(workspace.repoPath, "src", "target.ts"); + try { + const agent = await workspace.client.createAgent({ + provider: "mock", + cwd: workspace.repoPath, + workspaceId: workspace.workspaceId, + title: "Absolute file link tooltip", + modeId: "load-test", + model: "e2e-fast-stream", + initialPrompt: "Show the file link", + featureValues: { mockAssistantResponse: `[Open target](${filePath}:42-45)` }, + }); + await gotoAppShell(page); + await waitForSidebarHydration(page); + const workspaceRow = page.getByTestId( + `sidebar-workspace-row-${getServerId()}:${workspace.workspaceId}`, + ); + await expect(workspaceRow).toBeVisible({ timeout: 30_000 }); + await workspaceRow.click(); + await page.getByTestId(`workspace-tab-agent_${agent.id}`).filter({ visible: true }).click(); + + const fileLink = page.getByRole("link", { name: "Open target" }).first(); + await expect(fileLink).toBeVisible({ timeout: 15_000 }); + await fileLink.hover(); + await expect(page.getByText("src/target.ts:42-45", { exact: true })).toBeVisible(); + } finally { + await workspace.cleanup(); + } + }); + test("renders a lockfile-sized read-only source with a bounded CodeMirror DOM", async ({ page, }) => { diff --git a/packages/app/src/assistant-file-links/link.tsx b/packages/app/src/assistant-file-links/link.tsx index 31f6d719d7..4b8e944ee3 100644 --- a/packages/app/src/assistant-file-links/link.tsx +++ b/packages/app/src/assistant-file-links/link.tsx @@ -10,6 +10,7 @@ import { CODE_SURFACE_DATASET } from "@/styles/code-surface"; import { markdownCopyDataSet } from "@/assistant-selection-copy/markup"; import { useAssistantFileLinkResolverContext } from "./provider"; import type { AssistantFileLinkSource } from "./resolver"; +import { formatFileLinkTooltipPath } from "./tooltip-path"; import { useFileLink } from "./use-file-link"; interface AssistantMarkdownLinkProps { @@ -34,7 +35,7 @@ export function AssistantMarkdownLink({ const { configRef } = useAssistantFileLinkResolverContext(); const workspaceRoot = configRef.current.workspaceRoot; const tooltipPath = useMemo( - () => (target ? formatInlinePathTargetForTooltip(target, workspaceRoot) : null), + () => (target ? formatFileLinkTooltipPath({ target, workspaceRoot }) : null), [target, workspaceRoot], ); const linkPress = useMemo( @@ -127,38 +128,6 @@ export function AssistantMarkdownCodeLink({ ); } -function formatInlinePathTargetForTooltip( - target: { path: string; lineStart?: number; lineEnd?: number }, - workspaceRoot: string | undefined, -): string { - let result = relativizePathToWorkspace(target.path, workspaceRoot); - if (target.lineStart) { - result += `:${target.lineStart}`; - if (target.lineEnd && target.lineEnd !== target.lineStart) { - result += `-${target.lineEnd}`; - } - } - return result; -} - -function relativizePathToWorkspace(filePath: string, workspaceRoot: string | undefined): string { - if (!workspaceRoot) { - return filePath; - } - const root = workspaceRoot.replace(/\/+$/, ""); - if (!root) { - return filePath; - } - if (filePath === root) { - return "."; - } - const prefix = `${root}/`; - if (filePath.startsWith(prefix)) { - return filePath.slice(prefix.length); - } - return filePath; -} - interface AssistantInlineCodePathLinkProps { content: string; inheritedStyles: TextStyle; diff --git a/packages/app/src/assistant-file-links/tooltip-path.test.ts b/packages/app/src/assistant-file-links/tooltip-path.test.ts new file mode 100644 index 0000000000..c01410f54b --- /dev/null +++ b/packages/app/src/assistant-file-links/tooltip-path.test.ts @@ -0,0 +1,59 @@ +import { describe, expect, it } from "vitest"; +import { formatFileLinkTooltipPath } from "./tooltip-path"; + +describe("formatFileLinkTooltipPath", () => { + it("shows a Windows file path relative to its workspace regardless of separators", () => { + expect( + formatFileLinkTooltipPath({ + target: { + path: "C:/Users/me/repo/src/app.ts", + lineStart: 12, + lineEnd: 20, + }, + workspaceRoot: "C:\\Users\\me\\repo", + }), + ).toBe("src/app.ts:12-20"); + expect( + formatFileLinkTooltipPath({ + target: { path: "C:/src/app.ts", lineStart: 12 }, + workspaceRoot: "C:\\", + }), + ).toBe("src/app.ts:12"); + }); + + it("shows the workspace root as a dot", () => { + expect( + formatFileLinkTooltipPath({ + target: { path: "/Users/me/repo" }, + workspaceRoot: "/Users/me/repo", + }), + ).toBe("."); + expect( + formatFileLinkTooltipPath({ + target: { path: "C:/Users/Me/Repo/", lineStart: 4 }, + workspaceRoot: "c:\\users\\me\\repo", + }), + ).toBe(".:4"); + }); + + it("keeps an absolute path outside the workspace", () => { + expect( + formatFileLinkTooltipPath({ + target: { path: "/Users/me/notes.md" }, + workspaceRoot: "/Users/me/repo", + }), + ).toBe("/Users/me/notes.md"); + expect( + formatFileLinkTooltipPath({ + target: { path: "C:/Users/me/notes.md", lineStart: 8 }, + workspaceRoot: "C:\\Users\\me\\repo", + }), + ).toBe("C:/Users/me/notes.md:8"); + }); + + it("keeps the target path when the workspace root is unavailable", () => { + expect(formatFileLinkTooltipPath({ target: { path: "src/app.ts", lineStart: 12 } })).toBe( + "src/app.ts:12", + ); + }); +}); diff --git a/packages/app/src/assistant-file-links/tooltip-path.ts b/packages/app/src/assistant-file-links/tooltip-path.ts new file mode 100644 index 0000000000..dee48a3e9e --- /dev/null +++ b/packages/app/src/assistant-file-links/tooltip-path.ts @@ -0,0 +1,30 @@ +import { + isWorkspaceRootPath, + resolveWorkspaceFilePaths, + type WorkspaceFileLocation, +} from "@/workspace/file-open"; + +interface FormatFileLinkTooltipPathInput { + target: WorkspaceFileLocation; + workspaceRoot?: string; +} + +export function formatFileLinkTooltipPath({ + target, + workspaceRoot, +}: FormatFileLinkTooltipPathInput): string { + const resolvedPaths = workspaceRoot + ? resolveWorkspaceFilePaths({ path: target.path, workspaceRoot }) + : null; + const isWorkspaceRoot = workspaceRoot + ? isWorkspaceRootPath({ path: target.path, workspaceRoot }) + : false; + let result = isWorkspaceRoot ? "." : (resolvedPaths?.relativePath ?? target.path); + if (target.lineStart) { + result += `:${target.lineStart}`; + if (target.lineEnd && target.lineEnd !== target.lineStart) { + result += `-${target.lineEnd}`; + } + } + return result; +} diff --git a/packages/app/src/workspace/file-open/index.test.ts b/packages/app/src/workspace/file-open/index.test.ts index d7d2dd3976..3b8ed4ca83 100644 --- a/packages/app/src/workspace/file-open/index.test.ts +++ b/packages/app/src/workspace/file-open/index.test.ts @@ -136,6 +136,24 @@ describe("resolveWorkspaceFilePaths", () => { }); }); + it("resolves files under a drive-root workspace without doubling the separator", () => { + expect(resolveWorkspaceFilePaths({ path: "C:/src/app.ts", workspaceRoot: "C:\\" })).toEqual({ + absolutePath: "C:/src/app.ts", + relativePath: "src/app.ts", + }); + expect(resolveWorkspaceFilePaths({ path: "src/app.ts", workspaceRoot: "C:\\" })).toEqual({ + absolutePath: "C:/src/app.ts", + relativePath: "src/app.ts", + }); + }); + + it("resolves files under the POSIX root workspace", () => { + expect(resolveWorkspaceFilePaths({ path: "/src/app.ts", workspaceRoot: "/" })).toEqual({ + absolutePath: "/src/app.ts", + relativePath: "src/app.ts", + }); + }); + it("returns null for home-relative paths that cannot be anchored", () => { expect( resolveWorkspaceFilePaths({ path: "~/notes.md", workspaceRoot: "/Users/me/repo" }), diff --git a/packages/app/src/workspace/file-open/index.ts b/packages/app/src/workspace/file-open/index.ts index f09d5bd81f..7668de923c 100644 --- a/packages/app/src/workspace/file-open/index.ts +++ b/packages/app/src/workspace/file-open/index.ts @@ -129,6 +129,17 @@ function startsWithPath(value: string, prefix: string): boolean { : value.startsWith(prefix); } +function pathWithinRoot(root: string, relativePath: string): string { + return root.endsWith("/") ? `${root}${relativePath}` : `${root}/${relativePath}`; +} + +/** Whether an absolute path names the workspace directory itself. */ +export function isWorkspaceRootPath(input: { path: string; workspaceRoot: string }): boolean { + const path = normalizeAbsolutePath(input.path.trim().replace(/\\/g, "/")); + const root = normalizeAbsolutePath(input.workspaceRoot.trim().replace(/\\/g, "/")); + return path !== null && root !== null && pathsEqual(path, root); +} + export interface ResolvedWorkspaceFilePaths { /** Absolute path on the host, suitable for opening in an editor / file manager. */ absolutePath: string; @@ -159,7 +170,7 @@ export function resolveWorkspaceFilePaths(input: { if (pathsEqual(normalizedFile, workspaceRoot)) { return null; } - const prefix = `${workspaceRoot}/`; + const prefix = pathWithinRoot(workspaceRoot, ""); const relativePath = startsWithPath(normalizedFile, prefix) ? normalizedFile.slice(prefix.length) : null; @@ -174,5 +185,5 @@ export function resolveWorkspaceFilePaths(input: { if (!relativePath) { return null; } - return { absolutePath: `${workspaceRoot}/${relativePath}`, relativePath }; + return { absolutePath: pathWithinRoot(workspaceRoot, relativePath), relativePath }; }