diff --git a/apps/web/src/components/ChatMarkdown.tsx b/apps/web/src/components/ChatMarkdown.tsx index 4a1f616aed3b..695c574ce603 100644 --- a/apps/web/src/components/ChatMarkdown.tsx +++ b/apps/web/src/components/ChatMarkdown.tsx @@ -34,6 +34,8 @@ import type { ServerProviderSkill, ThreadPullRequestKey, } from "@t3tools/contracts"; +import { ProjectReadFileError } from "@t3tools/contracts"; +import { scopedThreadKey } from "@t3tools/client-runtime/environment"; import { faviconUrlForOrigin } from "@t3tools/shared/favicon"; import { githubMediaFetchUrl } from "@t3tools/shared/githubMedia"; import { @@ -54,6 +56,7 @@ import { inlineCodeFilePathCandidate } from "@t3tools/client-runtime/markdown-li import { mediaFileReference, mediaUrlReference } from "@t3tools/client-runtime/media-reference"; import { mediaKindFromPath, mediaMimeTypeFromExtension } from "@t3tools/shared/filePreview"; import * as Cause from "effect/Cause"; +import * as Schema from "effect/Schema"; import { AsyncResult } from "effect/unstable/reactivity"; import React, { Children, @@ -174,7 +177,8 @@ import { useAtomQueryRunner } from "../state/use-atom-query-runner"; import { projectEnvironment } from "../state/projects"; import { claimWorkspaceBasenameLookup, - needsWorkspaceBasenameLookup, + needsLiteralWorkspaceFileCheck, + normalizeWorkspaceLookupPath, pickWorkspaceBasenameMatch, WORKSPACE_BASENAME_LOOKUP_LIMIT, } from "../workspaceBasenameLookup"; @@ -277,6 +281,8 @@ export function shouldUseMarkdownFileBrowserPrimaryAction(input: { const EMPTY_MARKDOWN_SKILLS: ReadonlyArray> = []; const EMPTY_REMARK_PLUGINS: NonNullable = []; +const isProjectReadFileError = Schema.is(ProjectReadFileError); + const ARTIFACT_TEMPLATE_ICON_BY_KIND = { document: FileTextIcon, presentation: PresentationIcon, @@ -2321,6 +2327,10 @@ function useChatMarkdownState({ const searchProjectEntries = useAtomQueryRunner(projectEnvironment.searchEntries, { reportFailure: false, }); + const readWorkspaceFile = useAtomQueryRunner(projectEnvironment.readFile, { + reportFailure: false, + refresh: true, + }); const openPreview = useAtomCommand(previewEnvironment.open, { reportFailure: false, }); @@ -2557,37 +2567,67 @@ function useChatMarkdownState({ }, [createAssetUrl, cwd, openPreview, preparedConnection, threadRef], ); + const literalWorkspaceFileExists = useCallback( + async (workspaceRelativePath: string) => { + if (!cwd || environmentId === null) return false; + const result = await readWorkspaceFile({ + environmentId, + input: { cwd, relativePath: workspaceRelativePath }, + }); + if (result._tag === "Success") return true; + // readFile reads from disk rather than the search index, so gitignored + // files still report present. Existing binary files (and directories) + // fail the read with a distinct reason but still prove the literal path + // exists, so the index must not hijack them. Only a genuinely missing + // file falls through to the indexed match. + const cause = squashAtomCommandFailure(result); + return ( + isProjectReadFileError(cause) && + (cause.failure === "binary_file" || cause.failure === "path_not_file") + ); + }, + [cwd, environmentId, readWorkspaceFile], + ); const findWorkspaceBasenameMatch = useCallback( async (workspaceRelativePath: string) => { - if (!cwd || environmentId === null || !needsWorkspaceBasenameLookup(workspaceRelativePath)) { + const lookupPath = normalizeWorkspaceLookupPath(workspaceRelativePath); + if (!cwd || environmentId === null || !lookupPath) { return null; } const result = await searchProjectEntries({ environmentId, input: { cwd, - query: workspaceRelativePath, + query: lookupPath, limit: WORKSPACE_BASENAME_LOOKUP_LIMIT, kind: "file", }, }); - return result._tag === "Success" - ? pickWorkspaceBasenameMatch(workspaceRelativePath, result.value.entries) - : null; + if (result._tag !== "Success") return null; + const match = pickWorkspaceBasenameMatch(lookupPath, result.value.entries); + if ( + needsLiteralWorkspaceFileCheck(lookupPath, match) && + (await literalWorkspaceFileExists(lookupPath)) + ) { + // The literal chip path names a real file, so keep it instead of the + // indexed twin. Callers fall back to the literal path on null. + return null; + } + return match; }, - [cwd, environmentId, searchProjectEntries], + [cwd, environmentId, literalWorkspaceFileExists, searchProjectEntries], ); - // A bare filename resolves to the workspace root, which is rarely where the - // file is, so ask the index before opening. Absolute host paths open as-is. + // Chip paths are relative to the agent's cwd, so every workspace open goes + // through the index first. Absolute host paths open as-is. const openFileInPanel = useCallback( (panelPath: string, line: number | undefined) => { if (!threadRef) return; // Claimed on every open so a synchronous one supersedes a lookup already // in flight. - const isLatestLookup = claimWorkspaceBasenameLookup(); + const isLatestLookup = claimWorkspaceBasenameLookup(scopedThreadKey(threadRef)); const openAt = (path: string) => useRightPanelStore.getState().openFile(threadRef, path, line); - if (!cwd || !needsWorkspaceBasenameLookup(panelPath)) { + if (!cwd || environmentId === null || isAbsolutePath(panelPath)) { openAt(panelPath); return; } @@ -2597,7 +2637,7 @@ function useChatMarkdownState({ openAt(match ?? panelPath); })(); }, - [cwd, findWorkspaceBasenameMatch, threadRef], + [cwd, environmentId, findWorkspaceBasenameMatch, threadRef], ); const revealMarkdownFileInFileManager = useCallback( async (fileLinkMeta: MarkdownFileLinkMeta) => { @@ -2610,6 +2650,34 @@ function useChatMarkdownState({ }, [cwd, findWorkspaceBasenameMatch, revealFileInFileManager], ); + const openMarkdownFileInEditor = useCallback( + async (fileLinkMeta: MarkdownFileLinkMeta) => { + const workspaceRelativePath = fileLinkMeta.workspaceRelativePath; + const match = workspaceRelativePath + ? await findWorkspaceBasenameMatch(workspaceRelativePath) + : null; + if (!match || !cwd) { + return openInPreferredEditor(fileLinkMeta.targetPath); + } + const withPosition = fileLinkMeta.line + ? `${match}:${fileLinkMeta.line}${fileLinkMeta.column ? `:${fileLinkMeta.column}` : ""}` + : match; + return openInPreferredEditor(resolvePathLinkTarget(withPosition, cwd)); + }, + [cwd, findWorkspaceBasenameMatch, openInPreferredEditor], + ); + const openMarkdownFileInBrowser = useCallback( + async (fileLinkMeta: MarkdownFileLinkMeta) => { + const workspaceRelativePath = fileLinkMeta.workspaceRelativePath; + const match = workspaceRelativePath + ? await findWorkspaceBasenameMatch(workspaceRelativePath) + : null; + return openMarkdownFileInPreview( + match && cwd ? resolvePathLinkTarget(match, cwd) : fileLinkMeta.filePath, + ); + }, + [cwd, findWorkspaceBasenameMatch, openMarkdownFileInPreview], + ); const fileLinkChip = useCallback( (fileLinkMeta: MarkdownFileLinkMeta, copyMarkdown: string, mediaSource?: string) => { const parentSuffix = fileLinkParentSuffixByPath.get( @@ -2647,7 +2715,7 @@ function useChatMarkdownState({ copyMarkdown={copyMarkdown} theme={resolvedTheme} threadRef={threadRef} - {...(canUseShellActions ? { onOpen: openInPreferredEditor } : {})} + {...(canUseShellActions ? { onOpen: () => openMarkdownFileInEditor(fileLinkMeta) } : {})} onOpenInPanel={openFileInPanel} onOpenMedia={ threadRef && canPreviewMedia @@ -2665,7 +2733,7 @@ function useChatMarkdownState({ threadRef && isPreviewSupportedInRuntime() && isBrowserPreviewFile(fileLinkMeta.filePath) - ? () => openMarkdownFileInPreview(fileLinkMeta.filePath) + ? () => openMarkdownFileInBrowser(fileLinkMeta) : undefined } /> @@ -2675,8 +2743,8 @@ function useChatMarkdownState({ canUseShellActions, fileLinkParentSuffixByPath, openFileInPanel, - openInPreferredEditor, - openMarkdownFileInPreview, + openMarkdownFileInEditor, + openMarkdownFileInBrowser, openMarkdownMedia, preferredEditorMenuLabel, resolvedTheme, diff --git a/apps/web/src/workspaceBasenameLookup.test.ts b/apps/web/src/workspaceBasenameLookup.test.ts index afe159dd402b..75889faff25a 100644 --- a/apps/web/src/workspaceBasenameLookup.test.ts +++ b/apps/web/src/workspaceBasenameLookup.test.ts @@ -2,25 +2,10 @@ import { describe, expect, it } from "vite-plus/test"; import { claimWorkspaceBasenameLookup, - needsWorkspaceBasenameLookup, + needsLiteralWorkspaceFileCheck, pickWorkspaceBasenameMatch, } from "./workspaceBasenameLookup"; -describe("needsWorkspaceBasenameLookup", () => { - it("flags bare filenames", () => { - expect(needsWorkspaceBasenameLookup("ChatView.tsx")).toBe(true); - expect(needsWorkspaceBasenameLookup("Makefile")).toBe(true); - }); - - it("leaves anything with a directory alone", () => { - expect(needsWorkspaceBasenameLookup("apps/web/src/components/ChatView.tsx")).toBe(false); - expect(needsWorkspaceBasenameLookup("apps\\web\\ChatView.tsx")).toBe(false); - expect(needsWorkspaceBasenameLookup(".")).toBe(false); - expect(needsWorkspaceBasenameLookup("..")).toBe(false); - expect(needsWorkspaceBasenameLookup(" ")).toBe(false); - }); -}); - describe("pickWorkspaceBasenameMatch", () => { const entries = [ { path: "apps/web/src/components/ChatView.test.tsx", kind: "file" as const }, @@ -28,9 +13,39 @@ describe("pickWorkspaceBasenameMatch", () => { ]; it("takes the first exact filename match, not the closest fuzzy one", () => { - expect(pickWorkspaceBasenameMatch("ChatView.tsx", entries)).toBe( - "apps/web/src/components/ChatView.tsx", - ); + expect( + pickWorkspaceBasenameMatch("ChatView.tsx", [ + ...entries, + { path: "apps/desktop/src/ChatView.tsx", kind: "file" }, + ]), + ).toBe("apps/web/src/components/ChatView.tsx"); + }); + + it("matches a slashed path as a suffix wherever it actually lives", () => { + expect( + pickWorkspaceBasenameMatch("docs/GROUPS_PLAN.md", [ + { path: "BudgetLens/docs/GROUPS_PLAN.md", kind: "file" }, + { path: "docs/other.md", kind: "file" }, + ]), + ).toBe("BudgetLens/docs/GROUPS_PLAN.md"); + }); + + it("does not let a suffix match straddle a segment boundary", () => { + expect( + pickWorkspaceBasenameMatch("docs/plan.md", [{ path: "src/mydocs/plan.md", kind: "file" }]), + ).toBeNull(); + expect( + pickWorkspaceBasenameMatch("plan.md", [{ path: "src/plan.md.bak", kind: "file" }]), + ).toBeNull(); + }); + + it("prefers the exact suffix over a basename-only twin", () => { + expect( + pickWorkspaceBasenameMatch("docs/plan.md", [ + { path: "other/plan.md", kind: "file" }, + { path: "BudgetLens/docs/plan.md", kind: "file" }, + ]), + ).toBe("BudgetLens/docs/plan.md"); }); it("ignores directories", () => { @@ -66,6 +81,12 @@ describe("pickWorkspaceBasenameMatch", () => { ).toBeNull(); }); + it("leaves workspace-root links alone", () => { + expect(pickWorkspaceBasenameMatch(".", entries)).toBeNull(); + expect(pickWorkspaceBasenameMatch("..", entries)).toBeNull(); + expect(pickWorkspaceBasenameMatch(" ", entries)).toBeNull(); + }); + it("returns null when nothing matches the name", () => { expect(pickWorkspaceBasenameMatch("ChatView.tsx", [])).toBeNull(); expect( @@ -76,10 +97,37 @@ describe("pickWorkspaceBasenameMatch", () => { }); }); +describe("needsLiteralWorkspaceFileCheck", () => { + it("checks the disk when a slashed hit names a different file than the chip", () => { + expect( + needsLiteralWorkspaceFileCheck("docs/GROUPS_PLAN.md", "BudgetLens/docs/GROUPS_PLAN.md"), + ).toBe(true); + }); + + it("skips the disk when the hit is the literal path", () => { + expect( + needsLiteralWorkspaceFileCheck( + "apps/web/src/components/ChatView.tsx", + "apps/web/src/components/ChatView.tsx", + ), + ).toBe(false); + }); + + it("skips the disk when there is no hit", () => { + expect(needsLiteralWorkspaceFileCheck("docs/plan.md", null)).toBe(false); + }); + + it("skips the disk for bare names, which keep the ranked-first hit", () => { + expect(needsLiteralWorkspaceFileCheck("ChatView.tsx", "apps/web/src/ChatView.tsx")).toBe( + false, + ); + }); +}); + describe("claimWorkspaceBasenameLookup", () => { it("keeps only the newest claim, whatever order the lookups settle in", () => { - const first = claimWorkspaceBasenameLookup(); - const second = claimWorkspaceBasenameLookup(); + const first = claimWorkspaceBasenameLookup("thread-a"); + const second = claimWorkspaceBasenameLookup("thread-a"); // The older lookup answering last must not reopen the panel behind the // newer one. @@ -88,7 +136,13 @@ describe("claimWorkspaceBasenameLookup", () => { }); it("stays valid while it is the only claim", () => { - const only = claimWorkspaceBasenameLookup(); + const only = claimWorkspaceBasenameLookup("thread-a"); expect(only()).toBe(true); }); + + it("keeps another thread's in-flight claim valid", () => { + const threadA = claimWorkspaceBasenameLookup("thread-a"); + claimWorkspaceBasenameLookup("thread-b"); + expect(threadA()).toBe(true); + }); }); diff --git a/apps/web/src/workspaceBasenameLookup.ts b/apps/web/src/workspaceBasenameLookup.ts index 0efdfcf3a44d..12c1f7a6cba3 100644 --- a/apps/web/src/workspaceBasenameLookup.ts +++ b/apps/web/src/workspaceBasenameLookup.ts @@ -2,15 +2,14 @@ // asking for a full listing on a single click. export const WORKSPACE_BASENAME_LOOKUP_LIMIT = 25; -// One counter for every caller: they all open the same panel, so the newest -// click wins regardless of which one started the lookup. -let latestLookupSequence = 0; +// Newest click wins within a thread. Other threads keep their own lookups. +const latestLookupSequenceByScope = new Map(); /** Call the returned predicate when the search settles; false means a later click superseded it. */ -export function claimWorkspaceBasenameLookup(): () => boolean { - latestLookupSequence += 1; - const claimed = latestLookupSequence; - return () => claimed === latestLookupSequence; +export function claimWorkspaceBasenameLookup(scope: string): () => boolean { + const next = (latestLookupSequenceByScope.get(scope) ?? 0) + 1; + latestLookupSequenceByScope.set(scope, next); + return () => latestLookupSequenceByScope.get(scope) === next; } export interface WorkspaceEntryCandidate { @@ -18,37 +17,64 @@ export interface WorkspaceEntryCandidate { readonly kind: "file" | "directory"; } -function basenameOfPath(path: string): string { - const separatorIndex = Math.max(path.lastIndexOf("/"), path.lastIndexOf("\\")); - return separatorIndex >= 0 ? path.slice(separatorIndex + 1) : path; +function posixPath(path: string): string { + return path.replaceAll("\\", "/"); } -export function needsWorkspaceBasenameLookup(relativePath: string): boolean { - const trimmed = relativePath.trim(); - return ( - trimmed !== "." && - trimmed !== ".." && - trimmed.length > 0 && - !trimmed.includes("/") && - !trimmed.includes("\\") - ); +export function normalizeWorkspaceLookupPath(relativePath: string): string { + const normalized = relativePath + .trim() + .replaceAll("\\", "/") + .replace(/^(?:\.\/)+/, ""); + // Workspace-root links (`.` / `..`, from #12449) never resolve to a file. + // Return empty so callers skip the index lookup, matching the old + // `needsWorkspaceBasenameLookup` guard. + if (normalized === "." || normalized === "..") return ""; + return normalized; +} + +/** `a/b/c` matches `c` and `b/c` at a segment boundary, but not `abc`. */ +function hasSegmentSuffix(path: string, suffix: string): boolean { + const posix = posixPath(path); + return posix === suffix || posix.endsWith(`/${suffix}`); +} + +/** + * Whether a divergent index hit may override the literal chip path only after + * a disk check. The search index skips gitignored files, so its absence there + * is not proof a file does not exist: for slashed paths an index hit that + * names a different file must not hijack a real file at the literal path. + * Bare names keep the ranked-first hit with no disk check. + */ +export function needsLiteralWorkspaceFileCheck( + lookupPath: string, + match: string | null, +): boolean { + return match !== null && match !== lookupPath && lookupPath.includes("/"); } export function pickWorkspaceBasenameMatch( basename: string, entries: ReadonlyArray, ): string | null { - const target = basename.trim(); + const target = normalizeWorkspaceLookupPath(basename); if (!target) return null; const files = entries.filter((entry) => entry.kind === "file"); - const exact = files.find((entry) => basenameOfPath(entry.path) === target); - if (exact) return exact.path; + const exactPath = files.find((entry) => posixPath(entry.path) === target); + if (exactPath) return exactPath.path; + // Chip paths are relative to the agent's cwd, so match the suffix wherever it lives. + const suffixMatches = files.filter((entry) => hasSegmentSuffix(entry.path, target)); + if (suffixMatches.length === 1) return suffixMatches[0]?.path ?? null; + if (suffixMatches.length > 1) { + return target.includes("/") ? null : (suffixMatches[0]?.path ?? null); + } + const foldedTarget = target.toLowerCase(); + const foldedSuffixMatches = files.filter((entry) => + hasSegmentSuffix(posixPath(entry.path).toLowerCase(), foldedTarget), + ); + if (foldedSuffixMatches.length === 1) return foldedSuffixMatches[0]?.path ?? null; // Folded matching covers casing that drifted from disk, but `FOO.ts` against // both `Foo.ts` and `foo.ts` has no right answer, so it resolves to nothing // rather than opening whichever the index ranked first. - const folded = target.toLowerCase(); - const foldedMatches = files.filter( - (entry) => basenameOfPath(entry.path).toLowerCase() === folded, - ); - return foldedMatches.length === 1 ? (foldedMatches[0]?.path ?? null) : null; + return null; }