diff --git a/apps/web/src/components/ChatMarkdown.browser.test.tsx b/apps/web/src/components/ChatMarkdown.browser.test.tsx index 745e69087cb1e..5edc7ca243fd9 100644 --- a/apps/web/src/components/ChatMarkdown.browser.test.tsx +++ b/apps/web/src/components/ChatMarkdown.browser.test.tsx @@ -12,15 +12,25 @@ import { useRightPanelStore } from "../rightPanelStore"; import { setMarkdownTaskChecked } from "./files/filePreviewMode"; import { HostImageUrlContext } from "./chat/hostImageUrl"; -const { workspaceSearch, filesystemMetadata } = vi.hoisted(() => ({ +const { workspaceSearch, filesystemMetadata, chipMetadata } = vi.hoisted(() => ({ workspaceSearch: vi.fn(), filesystemMetadata: vi.fn(), + chipMetadata: new Map>(), })); vi.mock(import("../state/use-atom-query-runner"), async (original) => ({ ...(await original()), useAtomQueryRunner: ((family: unknown) => family === filesystemEnvironment.getMetadata ? filesystemMetadata : workspaceSearch) as never, })); +// What each chip already fetched about its own path; unset paths have not loaded. +vi.mock(import("../hooks/useFileMetadata"), async (original) => { + const { Atom } = await import("effect/reactivity"); + return { + ...(await original()), + fileMetadataAtom: ((_environmentId: unknown, path: string) => + Atom.make(chipMetadata.get(path) ?? AsyncResult.initial())) as never, + }; +}); // Atoms derived from the server config run in the real registry over a missing config. vi.mock(import("../state/server"), async (original) => { @@ -103,6 +113,15 @@ function setup() { renderDiagram.mockReset(); workspaceSearch.mockReset().mockResolvedValue(AsyncResult.success({ entries: [] })); filesystemMetadata.mockReset().mockResolvedValue(AsyncResult.success({ entries: [null] })); + chipMetadata.clear(); +} + +function fileChip(container: HTMLElement, label: string) { + const chip = [...container.querySelectorAll(".chat-markdown-file-link")].find( + (element) => element.textContent === label, + ); + if (!chip) throw new Error(`No file chip labelled ${label}`); + return chip; } function renderText(text: string, props: Partial[0]> = {}) { @@ -276,6 +295,118 @@ describe("ChatMarkdown workspace file opens", () => { expect(openFile).toHaveBeenCalledExactlyOnceWith(threadRef, "/elsewhere/b.md", 9); expect(workspaceSearch).toHaveBeenCalledOnce(); }); + + it("opens a path relative to a folder the reply names when the workspace root lacks it", async () => { + setup(); + chipMetadata.set("/tmp/work/phase2/A-findings.md", AsyncResult.success(null)); + filesystemMetadata.mockResolvedValue( + AsyncResult.success({ entries: [null, { kind: "file" }] }), + ); + const message = (folders: string) => ( + + ); + await using screen = await render(message("`/tmp/work/backend/.notes/audit/`")); + using openFile = vi + .spyOn(useRightPanelStore.getState(), "openFile") + .mockImplementation(() => {}); + await act(async () => fileChip(screen.container, "A-findings.md").click()); + + await vi.waitFor(() => + expect(openFile).toHaveBeenLastCalledWith( + threadRef, + "backend/.notes/audit/phase2/A-findings.md", + undefined, + ), + ); + expect(filesystemMetadata).toHaveBeenCalledExactlyOnceWith({ + environmentId: threadRef.environmentId, + input: { + paths: [ + "/tmp/work/phase2/A-findings.md", + "/tmp/work/backend/.notes/audit/phase2/A-findings.md", + ], + }, + }); + expect(workspaceSearch).not.toHaveBeenCalled(); + + // A directory by that name is not the file the chip names. + filesystemMetadata.mockResolvedValue( + AsyncResult.success({ entries: [null, { kind: "directory" }, { kind: "file" }] }), + ); + await screen.rerender( + message("`/tmp/work/backend/.notes/audit/` and `/tmp/work/frontend/.notes/audit/`"), + ); + await act(async () => fileChip(screen.container, "A-findings.md").click()); + await vi.waitFor(() => + expect(openFile).toHaveBeenLastCalledWith( + threadRef, + "frontend/.notes/audit/phase2/A-findings.md", + undefined, + ), + ); + + // Both named folders hold the file, so guessing could open the wrong one. + filesystemMetadata.mockResolvedValue( + AsyncResult.success({ entries: [null, { kind: "file" }, { kind: "file" }] }), + ); + await act(async () => fileChip(screen.container, "A-findings.md").click()); + await vi.waitFor(() => + expect(openFile).toHaveBeenLastCalledWith(threadRef, "phase2/A-findings.md", undefined), + ); + }); + + it("opens the one nested repository file a repository-relative path names", async () => { + setup(); + chipMetadata.set("/tmp/work/infra/main.tf", AsyncResult.success({ kind: "file" })); + chipMetadata.set("/tmp/work/tests/sandbox/test_real.py", AsyncResult.success(null)); + chipMetadata.set("/tmp/work/docs/guide.md", AsyncResult.success(null)); + const files = (...paths: Array) => + AsyncResult.success({ + entries: paths.map((path) => ({ path, kind: "file" })), + truncated: false, + }); + workspaceSearch.mockImplementation(async ({ input }: { input: { query: string } }) => + input.query === "tests/sandbox/test_real.py" + ? files("backend/tests/sandbox/test_real.py", "backend/tests/sandbox/test_real_more.py") + : files("backend/docs/guide.md", "frontend/docs/guide.md"), + ); + await using screen = await renderText( + "Changed `infra/main.tf` and `notes/todo.md`, skipped `tests/sandbox/test_real.py`, kept `docs/guide.md`.", + { cwd: "/tmp/work", threadRef }, + ); + using openFile = vi + .spyOn(useRightPanelStore.getState(), "openFile") + .mockImplementation(() => {}); + const { container } = screen; + + // A file the root has, or one whose metadata has not loaded, opens without waiting. + await act(async () => fileChip(container, "main.tf").click()); + expect(openFile).toHaveBeenLastCalledWith(threadRef, "infra/main.tf", undefined); + await act(async () => fileChip(container, "todo.md").click()); + expect(openFile).toHaveBeenLastCalledWith(threadRef, "notes/todo.md", undefined); + expect(filesystemMetadata).not.toHaveBeenCalled(); + expect(workspaceSearch).not.toHaveBeenCalled(); + + await act(async () => fileChip(container, "test_real.py").click()); + await vi.waitFor(() => + expect(openFile).toHaveBeenLastCalledWith( + threadRef, + "backend/tests/sandbox/test_real.py", + undefined, + ), + ); + + // Two repositories hold this path, so guessing could open the wrong file. + await act(async () => fileChip(container, "guide.md").click()); + await vi.waitFor(() => + expect(openFile).toHaveBeenLastCalledWith(threadRef, "docs/guide.md", undefined), + ); + expect(openFile).toHaveBeenCalledTimes(4); + }); }); describe("ChatMarkdown skill chips", () => { diff --git a/apps/web/src/components/ChatMarkdown.tsx b/apps/web/src/components/ChatMarkdown.tsx index 6a8f80d6d1071..cb4a61fe5d8ad 100644 --- a/apps/web/src/components/ChatMarkdown.tsx +++ b/apps/web/src/components/ChatMarkdown.tsx @@ -1,4 +1,8 @@ -import { buildFileLinkParentSuffixByPath, fileLinkLabel } from "@t3tools/shared/fileLinks"; +import { + buildFileLinkParentSuffixByPath, + fileLinkLabel, + resolvePathLinkTarget, +} from "@t3tools/shared/fileLinks"; import { isWindowsDrivePathHref, normalizeMarkdownLinkDestination, @@ -6,11 +10,18 @@ import { extractMarkdownLinkHrefs, inlineCodeFilePathCandidate, } from "@t3tools/shared/markdownLinks"; -import { isHostPath, resolveWorkspaceFilePath } from "@t3tools/shared/path"; +import { + isHostPath, + isNetworkPath, + normalizeProjectPathForComparison, + resolveWorkspaceFilePath, + workspaceRelativeFilePath, +} from "@t3tools/shared/path"; import { usePullRequestLinking } from "~/hooks/usePullRequestLinking"; import { AuthFilesystemReadScope, AuthOrchestrationOperateScope, + FILESYSTEM_METADATA_BATCH_LIMIT, SOURCE_CONTROL_MEDIA_ASSET, } from "@t3tools/contracts"; import { @@ -18,7 +29,7 @@ import { CHAT_MARKDOWN_REMARK_PLUGINS_WITH_BREAKS, CHAT_MARKDOWN_REHYPE_PLUGINS, } from "@t3tools/shared/markdownPipeline"; -import { useAtomValue } from "@effect/atom-react"; +import { RegistryContext, useAtomValue } from "@effect/atom-react"; import { COMPOSER_CONTEXT_CLIPBOARD_MIME, encodeComposerContextClipboardHtml, @@ -168,11 +179,15 @@ import { filesystemEnvironment } from "../state/filesystem"; import { claimWorkspaceBasenameLookup, needsWorkspaceBasenameLookup, + nestedWorkspaceLookupPath, + pickNestedWorkspaceMatch, pickStrippedWorkspaceMatch, pickWorkspaceBasenameMatch, stripRepeatedWorkspacePrefix, WORKSPACE_BASENAME_LOOKUP_LIMIT, } from "@t3tools/client-runtime/workspace-basename-lookup"; +import { FileMetadataThreadContext } from "../hooks/FileMetadataThreadProvider"; +import { fileMetadataAtom } from "../hooks/useFileMetadata"; import { resolvePullRequestPreviewTarget } from "@t3tools/client-runtime/change-request-links"; import { sourceControlClients } from "@t3tools/client-runtime/source-control-clients"; import { useOpenChangeRequestLink } from "~/lib/openPullRequestLink"; @@ -498,6 +513,8 @@ function useChatMarkdownState({ reportFailure: false, refresh: true, }); + const registry = use(RegistryContext); + const fileMetadataThread = use(FileMetadataThreadContext); const openPreview = useAtomCommand(previewEnvironment.open, { reportFailure: false, }); @@ -740,6 +757,74 @@ function useChatMarkdownState({ openPreview, }); }; + // Acts only on the chip's settled metadata, so a click never waits on the + // network unless the chip already knows its root path is missing. + const isMissingNestedPath = (fileLinkMeta: MarkdownFileLinkMeta) => { + if ( + environmentId === null || + fileLinkMeta.workspaceLookupPath === null || + nestedWorkspaceLookupPath(fileLinkMeta.workspaceLookupPath) === null + ) { + return false; + } + const atom = fileMetadataAtom( + environmentId, + fileLinkMeta.filePath, + undefined, + fileMetadataThread, + readEnvironmentScope(environmentId, AuthFilesystemReadScope), + ); + if (atom === null) return false; + const known = registry.get(atom); + return known._tag === "Success" && !known.waiting && known.value === null; + }; + const findNestedWorkspaceFile = async ( + fileLinkMeta: MarkdownFileLinkMeta, + workspaceRoot: string, + lookupEnvironmentId: EnvironmentId, + ) => { + const nestedPath = nestedWorkspaceLookupPath(fileLinkMeta.workspaceLookupPath ?? ""); + if (nestedPath === null || !isMissingNestedPath(fileLinkMeta)) return null; + // Keyed like the metadata cache, so one Windows folder spelled two ways stays one candidate. + const seen = new Set([normalizeProjectPathForComparison(fileLinkMeta.filePath)]); + const candidates = [ + ...markdownFileLinkMetaByHref.values(), + ...inlineCodeFileLinkMetaByText.values(), + ] + .filter((meta) => meta.workspaceLookupPath === null) + .map((meta) => resolvePathLinkTarget(nestedPath, meta.filePath)) + .filter((path) => { + const key = normalizeProjectPathForComparison(path); + if (path.length > 512 || isNetworkPath(path) || seen.has(key)) return false; + seen.add(key); + return true; + }) + .slice(0, FILESYSTEM_METADATA_BATCH_LIMIT - 1); + // The chip's answer can predate the file, so the root path is checked again. + const metadata = await getFilesystemMetadata({ + environmentId: lookupEnvironmentId, + input: { paths: [fileLinkMeta.filePath, ...candidates] }, + }); + if (metadata._tag !== "Success" || metadata.value.entries[0] !== null) return null; + const named = candidates.filter( + (_, index) => metadata.value.entries[index + 1]?.kind === "file", + ); + if (named.length > 1) return null; + if (named[0] !== undefined) + return workspaceRelativeFilePath(named[0], workspaceRoot) ?? named[0]; + const result = await searchProjectEntries({ + environmentId: lookupEnvironmentId, + input: { + cwd: workspaceRoot, + query: nestedPath, + limit: WORKSPACE_BASENAME_LOOKUP_LIMIT, + kind: "file", + }, + }); + return result._tag === "Success" + ? pickNestedWorkspaceMatch(nestedPath, workspaceRoot, result.value.entries) + : null; + }; const findWorkspaceFileMatch = async (fileLinkMeta: MarkdownFileLinkMeta) => { const workspaceRelativePath = fileLinkMeta.workspaceLookupPath; if ( @@ -754,7 +839,9 @@ function useChatMarkdownState({ const strippedPath = isBasename ? null : stripRepeatedWorkspacePrefix(workspaceRelativePath, cwd); - if (!isBasename && strippedPath === null) return null; + if (!isBasename && strippedPath === null) { + return findNestedWorkspaceFile(fileLinkMeta, cwd, environmentId); + } const result = await searchProjectEntries({ environmentId, input: { @@ -769,7 +856,7 @@ function useChatMarkdownState({ return pickWorkspaceBasenameMatch(workspaceRelativePath, result.value.entries); } const match = pickStrippedWorkspaceMatch(strippedPath, cwd, result.value.entries); - if (match === null) return null; + if (match === null) return findNestedWorkspaceFile(fileLinkMeta, cwd, environmentId); // Search can be truncated or still scanning, so it cannot rule out the // literal path. Check the host before redirecting to the stripped match. const metadata = await getFilesystemMetadata({ @@ -788,7 +875,8 @@ function useChatMarkdownState({ fileLinkMeta.workspaceLookupPath === null || !cwd || (!needsWorkspaceBasenameLookup(panelPath) && - stripRepeatedWorkspacePrefix(panelPath, cwd) === null) + stripRepeatedWorkspacePrefix(panelPath, cwd) === null && + !isMissingNestedPath(fileLinkMeta)) ) { openAt(panelPath); return; diff --git a/apps/web/src/hooks/useFileMetadata.ts b/apps/web/src/hooks/useFileMetadata.ts index 497cc67d989eb..bcd81f45dbf90 100644 --- a/apps/web/src/hooks/useFileMetadata.ts +++ b/apps/web/src/hooks/useFileMetadata.ts @@ -1,4 +1,8 @@ -import { AuthFilesystemReadScope, type EnvironmentId } from "@t3tools/contracts"; +import { + AuthFilesystemReadScope, + type EnvironmentId, + type ScopedThreadRef, +} from "@t3tools/contracts"; import { resolvePathLinkTarget, splitFilePathPosition } from "@t3tools/shared/fileLinks"; import { isAbsolutePath, @@ -12,29 +16,41 @@ import { projectEnvironment } from "../state/projects"; import { useEnvironmentQuery } from "../state/query"; import { FileMetadataThreadContext } from "./FileMetadataThreadProvider"; -export function useFileMetadata( +export function fileMetadataAtom( environmentId: EnvironmentId | null | undefined, path: string, - cwd?: string, + cwd: string | undefined, + threadRef: ScopedThreadRef | null, + canRead: boolean, ) { - const canRead = useEnvironmentScope(environmentId ?? null, AuthFilesystemReadScope); - const threadRef = useContext(FileMetadataThreadContext); const hostPath = splitFilePathPosition(path).path; const targetPath = normalizeProjectPathForComparison( hostPath.startsWith("~/") || !cwd ? hostPath : resolvePathLinkTarget(hostPath, cwd), ); const absolute = isAbsolutePath(targetPath) || targetPath.startsWith("~/"); - return useEnvironmentQuery( - // Servers stat what they are asked about, and a network path leaks the - // Windows user's NTLM credentials to its host before any chip is clicked. - environmentId && canRead && absolute && !isNetworkPath(targetPath) && targetPath.length <= 512 - ? projectEnvironment.fileMetadata({ - environmentId, - input: { - path: targetPath, - ...(threadRef?.environmentId === environmentId ? { threadId: threadRef.threadId } : {}), - }, - }) - : null, - ).data; + // Servers stat what they are asked about, and a network path leaks the + // Windows user's NTLM credentials to its host before any chip is clicked. + return environmentId && + canRead && + absolute && + !isNetworkPath(targetPath) && + targetPath.length <= 512 + ? projectEnvironment.fileMetadata({ + environmentId, + input: { + path: targetPath, + ...(threadRef?.environmentId === environmentId ? { threadId: threadRef.threadId } : {}), + }, + }) + : null; +} + +export function useFileMetadata( + environmentId: EnvironmentId | null | undefined, + path: string, + cwd?: string, +) { + const canRead = useEnvironmentScope(environmentId ?? null, AuthFilesystemReadScope); + const threadRef = useContext(FileMetadataThreadContext); + return useEnvironmentQuery(fileMetadataAtom(environmentId, path, cwd, threadRef, canRead)).data; } diff --git a/apps/web/src/markdown-links.test.ts b/apps/web/src/markdown-links.test.ts index 8eeb667aeaec3..4178ef399dbe7 100644 --- a/apps/web/src/markdown-links.test.ts +++ b/apps/web/src/markdown-links.test.ts @@ -110,4 +110,19 @@ describe("chat file chip paths", () => { relativePath: "src/discord/events/user.ts:261:7", }); }); + + it("copies the matched file when it sits outside the workspace", async () => { + const meta = resolveMarkdownFileLinkMeta("phase2/A-findings.md", "/Users/alice/work"); + if (!meta) throw new Error("Expected a file link"); + const hostPath = await resolveMarkdownFileHostPath({ + meta, + cwd: "/Users/alice/work", + findWorkspaceMatch: async () => "/tmp/audit/phase2/A-findings.md", + }); + + expect(markdownFileCopyPaths(meta, hostPath, "/Users/alice/work")).toEqual({ + fullPath: "/tmp/audit/phase2/A-findings.md", + relativePath: "/tmp/audit/phase2/A-findings.md", + }); + }); }); diff --git a/apps/web/src/markdownFileHostPath.ts b/apps/web/src/markdownFileHostPath.ts index 26f3aaa348122..962c36d4f5bac 100644 --- a/apps/web/src/markdownFileHostPath.ts +++ b/apps/web/src/markdownFileHostPath.ts @@ -42,7 +42,7 @@ export function markdownFileCopyPaths( fullPath, relativePath: formatFilePathPosition({ ...meta, - path: workspaceRelativeFilePath(hostPath.absolutePath, cwd) ?? meta.filePath, + path: workspaceRelativeFilePath(hostPath.absolutePath, cwd) ?? hostPath.absolutePath, }), }; } diff --git a/apps/web/src/workspaceBasenameLookup.test.ts b/apps/web/src/workspaceBasenameLookup.test.ts index 78acaffb0ca03..9b8e6d913a19a 100644 --- a/apps/web/src/workspaceBasenameLookup.test.ts +++ b/apps/web/src/workspaceBasenameLookup.test.ts @@ -2,6 +2,8 @@ import { describe, expect, it } from "vite-plus/test"; import { claimWorkspaceBasenameLookup, + nestedWorkspaceLookupPath, + pickNestedWorkspaceMatch, pickWorkspaceBasenameMatch, } from "@t3tools/client-runtime/workspace-basename-lookup"; @@ -52,6 +54,57 @@ describe("pickWorkspaceBasenameMatch", () => { }); }); +describe("nestedWorkspaceLookupPath", () => { + it("keeps a plain subfolder path, without a leading ./", () => { + expect(nestedWorkspaceLookupPath("tests/sandbox/test_x.py")).toBe("tests/sandbox/test_x.py"); + expect(nestedWorkspaceLookupPath("./phase2/A-findings.md")).toBe("phase2/A-findings.md"); + }); + + it("rejects references that leave the workspace or name a host", () => { + for (const reference of [ + "README.md", + "../sibling/a.md", + "docs/../../a.md", + "~/notes/a.md", + ".///attacker.example/share/x.md", + String.raw`.\\\attacker.example\share\x.md`, + "./C:/Windows/win.ini", + ]) { + expect(nestedWorkspaceLookupPath(reference)).toBeNull(); + } + }); +}); + +describe("pickNestedWorkspaceMatch", () => { + it("picks the one file whose path ends with every segment of the reference", () => { + expect( + pickNestedWorkspaceMatch("src/foo.ts", "/work", [ + { path: "src/foo.ts", kind: "file" }, + { path: "frontend/xsrc/foo.ts", kind: "file" }, + { path: "backend/src/foo.tsx", kind: "file" }, + { path: "backend/src/foo.ts", kind: "file" }, + ]), + ).toBe("backend/src/foo.ts"); + }); + + it("does not guess between repositories holding the same path", () => { + expect( + pickNestedWorkspaceMatch("docs/guide.md", "/work", [ + { path: "backend/docs/guide.md", kind: "file" }, + { path: "frontend/docs/guide.md", kind: "file" }, + ]), + ).toBeNull(); + }); + + it("ignores case only in Windows workspaces", () => { + const entries = [{ path: "Backend\\Src\\Foo.ts", kind: "file" as const }]; + expect(pickNestedWorkspaceMatch("src/foo.ts", "C:\\work", entries)).toBe( + "Backend\\Src\\Foo.ts", + ); + expect(pickNestedWorkspaceMatch("src/foo.ts", "/work", entries)).toBeNull(); + }); +}); + describe("claimWorkspaceBasenameLookup", () => { it("keeps only the newest claim, whatever order the lookups settle in", () => { const first = claimWorkspaceBasenameLookup(); diff --git a/packages/client-runtime/src/workspaceBasenameLookup.ts b/packages/client-runtime/src/workspaceBasenameLookup.ts index c2a42e112218c..ea3b33890138f 100644 --- a/packages/client-runtime/src/workspaceBasenameLookup.ts +++ b/packages/client-runtime/src/workspaceBasenameLookup.ts @@ -65,16 +65,45 @@ export function stripRepeatedWorkspacePrefix(relativePath: string, cwd: string): return null; } +function workspacePathKey(cwd: string) { + const caseInsensitive = isWindowsAbsolutePath(cwd); + return (path: string) => { + const normalized = path.replaceAll("\\", "/"); + return caseInsensitive ? normalized.toLowerCase() : normalized; + }; +} + export function pickStrippedWorkspaceMatch( strippedPath: string, cwd: string, entries: ReadonlyArray, ): string | null { - const caseInsensitive = isWindowsAbsolutePath(cwd); - const key = (path: string) => { - const normalized = path.replaceAll("\\", "/"); - return caseInsensitive ? normalized.toLowerCase() : normalized; - }; + const key = workspacePathKey(cwd); const files = entries.filter((entry) => entry.kind === "file"); return files.find((entry) => key(entry.path) === key(strippedPath))?.path ?? null; } + +// Agents write paths relative to the nested repository or folder they worked in. +export function nestedWorkspaceLookupPath(relativePath: string): string | null { + const path = relativePath.trim().replace(/^\.[\\/]/, ""); + const segments = path.split(/[\\/]/); + return segments.length > 1 && + !isAbsolutePath(path) && + !path.startsWith("~") && + !segments.includes("..") + ? path + : null; +} + +export function pickNestedWorkspaceMatch( + nestedPath: string, + cwd: string, + entries: ReadonlyArray, +): string | null { + const key = workspacePathKey(cwd); + const suffix = `/${key(nestedPath)}`; + const matches = entries.filter( + (entry) => entry.kind === "file" && key(entry.path).endsWith(suffix), + ); + return matches.length === 1 ? (matches[0]?.path ?? null) : null; +}