diff --git a/apps/server/src/process/externalLauncher.test.ts b/apps/server/src/process/externalLauncher.test.ts index 1ab6166e92a1..cf39ee263d3e 100644 --- a/apps/server/src/process/externalLauncher.test.ts +++ b/apps/server/src/process/externalLauncher.test.ts @@ -132,6 +132,79 @@ it.effect("launches an installed editor with platform-safe arguments", () => }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), ); +// A `path:20-40` span comes from a chat reference to a range of lines. The +// editor has no span argument, so it opens at the range's first line rather +// than failing on a path that does not exist. +it.effect("opens a line span at its first line", () => + Effect.gen(function* () { + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const binDir = yield* fileSystem.makeTempDirectoryScoped({ prefix: "t3-editors-" }); + yield* fileSystem.writeFileString(path.join(binDir, "idea"), "#!/bin/sh\n"); + yield* fileSystem.chmod(path.join(binDir, "idea"), 0o755); + + let spawned: ChildProcess.StandardCommand | undefined; + yield* Effect.gen(function* () { + const launcher = yield* ExternalLauncher.ExternalLauncher; + yield* launcher.launchEditor({ editor: "idea", cwd: "/workspace/src/index.ts:20-40" }); + }).pipe( + Effect.provide( + testLayer({ + platform: "linux", + env: { PATH: binDir }, + onSpawn: (command) => { + spawned = command; + }, + }), + ), + ); + + assert.ok(spawned); + assert.deepEqual(spawned.args, ["--line", "20", "/workspace/src/index.ts"]); + }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), +); + +// `--goto` takes `path:line[:column]`, so a span has to be rebuilt rather than +// passed through; the raw `path:20-40` would be read as part of the filename. +it.effect("hands goto editors a span as path:line", () => + Effect.gen(function* () { + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const binDir = yield* fileSystem.makeTempDirectoryScoped({ prefix: "t3-editors-" }); + yield* fileSystem.writeFileString(path.join(binDir, "code"), "#!/bin/sh\n"); + yield* fileSystem.chmod(path.join(binDir, "code"), 0o755); + + const launch = (target: string) => + Effect.gen(function* () { + let spawned: ChildProcess.StandardCommand | undefined; + yield* Effect.gen(function* () { + const launcher = yield* ExternalLauncher.ExternalLauncher; + yield* launcher.launchEditor({ editor: "vscode", cwd: target }); + }).pipe( + Effect.provide( + testLayer({ + platform: "linux", + env: { PATH: binDir }, + onSpawn: (command) => { + spawned = command; + }, + }), + ), + ); + return spawned; + }); + + const spanLaunch = yield* launch("/workspace/src/index.ts:20-40"); + assert.ok(spanLaunch); + assert.deepEqual(spanLaunch.args, ["--goto", "/workspace/src/index.ts:20"]); + + // Every other shape still reaches the editor exactly as before. + const columnLaunch = yield* launch("/workspace/src/index.ts:20:5"); + assert.ok(columnLaunch); + assert.deepEqual(columnLaunch.args, ["--goto", "/workspace/src/index.ts:20:5"]); + }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), +); + it.effect("discovers editors through the service API", () => Effect.gen(function* () { const fileSystem = yield* FileSystem.FileSystem; diff --git a/apps/server/src/process/externalLauncher.ts b/apps/server/src/process/externalLauncher.ts index 8ec928f26fc3..164808dcb457 100644 --- a/apps/server/src/process/externalLauncher.ts +++ b/apps/server/src/process/externalLauncher.ts @@ -65,7 +65,9 @@ interface TargetPathAndPosition { readonly column: Option.Option; } -const TARGET_WITH_POSITION_PATTERN = /^(.*?):(\d+)(?::(\d+))?$/; +// `path:12`, `path:12:5`, and the span form `path:12-40`. An editor opens at a +// span's first line, so its end is matched to be stripped, never captured. +const TARGET_WITH_POSITION_PATTERN = /^(.*?):(\d+)(?::(\d+)|-\d+)?$/; const POWERSHELL_ARGUMENTS_PREFIX = [ "-NoProfile", "-NonInteractive", @@ -133,8 +135,20 @@ function resolveCommandEditorArgs( switch (editor.launchStyle) { case "direct-path": return [target]; + // Rebuilt from the parse rather than passed through: a `path:20-40` span + // is not a form these CLIs accept, and the rebuilt string is identical to + // `target` for every other shape. case "goto": - return Option.isSome(parsedTarget) ? ["--goto", target] : [target]; + return Option.match(parsedTarget, { + onNone: () => [target], + onSome: ({ path, line, column }) => [ + "--goto", + `${path}:${line}${Option.match(column, { + onNone: () => "", + onSome: (value) => `:${value}`, + })}`, + ], + }); case "line-column": return Option.match(parsedTarget, { onNone: () => [target], diff --git a/apps/web/src/components/ChatMarkdown.tsx b/apps/web/src/components/ChatMarkdown.tsx index 8dc545d31eb0..884a4e8cccbb 100644 --- a/apps/web/src/components/ChatMarkdown.tsx +++ b/apps/web/src/components/ChatMarkdown.tsx @@ -90,6 +90,13 @@ import { usePreparedConnection } from "../state/session"; import { previewEnvironment } from "../state/preview"; import { useAtomCommand } from "../state/use-atom-command"; import { useAtomQueryRunner } from "../state/use-atom-query-runner"; +import { projectEnvironment } from "../state/projects"; +import { + claimWorkspaceBasenameLookup, + needsWorkspaceBasenameLookup, + pickWorkspaceBasenameMatch, + WORKSPACE_BASENAME_LOOKUP_LIMIT, +} from "../workspaceBasenameLookup"; import { useOpenChangeRequestLink } from "~/lib/openPullRequestLink"; import { writeTextToClipboard } from "../hooks/useCopyToClipboard"; import { isPreviewSupportedInRuntime } from "../previewStateStore"; @@ -786,11 +793,17 @@ interface MarkdownFileLinkProps { displayPath: string; workspaceRelativePath: string | null; line?: number | undefined; + endLine?: number | undefined; label: string; copyMarkdown: string; theme: "light" | "dark"; threadRef?: ScopedThreadRef | undefined; onOpen: (targetPath: string) => Promise>; + onOpenInPanel: ( + workspaceRelativePath: string, + line: number | undefined, + endLine: number | undefined, + ) => void; onOpenInBrowser?: (() => Promise>) | undefined; className?: string | undefined; } @@ -1088,11 +1101,13 @@ const MarkdownFileLink = memo(function MarkdownFileLink({ displayPath, workspaceRelativePath, line, + endLine, label, copyMarkdown, theme, threadRef, onOpen, + onOpenInPanel, onOpenInBrowser, className, }: MarkdownFileLinkProps) { @@ -1136,8 +1151,8 @@ const MarkdownFileLink = memo(function MarkdownFileLink({ handleOpenInEditor(); return; } - useRightPanelStore.getState().openFile(threadRef, workspaceRelativePath, line); - }, [handleOpenInEditor, line, threadRef, workspaceRelativePath]); + onOpenInPanel(workspaceRelativePath, line, endLine); + }, [endLine, handleOpenInEditor, line, onOpenInPanel, threadRef, workspaceRelativePath]); const handleOpenInBrowser = useCallback(() => { if (!onOpenInBrowser) { @@ -1308,11 +1323,13 @@ function areMarkdownFileLinkPropsEqual( previous.displayPath === next.displayPath && previous.workspaceRelativePath === next.workspaceRelativePath && previous.line === next.line && + previous.endLine === next.endLine && previous.label === next.label && previous.copyMarkdown === next.copyMarkdown && previous.theme === next.theme && previous.threadRef === next.threadRef && previous.onOpen === next.onOpen && + previous.onOpenInPanel === next.onOpenInPanel && previous.onOpenInBrowser === next.onOpenInBrowser && previous.className === next.className ); @@ -1332,6 +1349,9 @@ function ChatMarkdown({ const createAssetUrl = useAtomQueryRunner(assetEnvironment.createUrl, { reportFailure: false, }); + const searchProjectEntries = useAtomQueryRunner(projectEnvironment.searchEntries, { + reportFailure: false, + }); const openPreview = useAtomCommand(previewEnvironment.open, { reportFailure: false, }); @@ -1434,6 +1454,40 @@ function ChatMarkdown({ }, [createAssetUrl, openPreview, preparedConnection, threadRef], ); + // `ChatView.tsx:3301` names a file without saying where it lives, so the link + // resolver can only place it at the workspace root and the read fails there. + // Ask the workspace index for the real location before opening the surface. + const openFileInPanel = useCallback( + (workspaceRelativePath: string, line: number | undefined, endLine: number | undefined) => { + if (!threadRef) return; + const openAt = (path: string) => + useRightPanelStore.getState().openFile(threadRef, path, line, endLine); + if (!cwd || !needsWorkspaceBasenameLookup(workspaceRelativePath)) { + openAt(workspaceRelativePath); + return; + } + const isLatestLookup = claimWorkspaceBasenameLookup(); + void (async () => { + const result = await searchProjectEntries({ + environmentId: threadRef.environmentId, + input: { + cwd, + query: workspaceRelativePath, + limit: WORKSPACE_BASENAME_LOOKUP_LIMIT, + kind: "file", + }, + }); + const match = + result._tag === "Success" + ? pickWorkspaceBasenameMatch(workspaceRelativePath, result.value.entries) + : null; + // A click that landed after this one already owns the panel. + if (!isLatestLookup()) return; + openAt(match ?? workspaceRelativePath); + })(); + }, + [cwd, searchProjectEntries, threadRef], + ); /* eslint-disable react/no-unstable-nested-components -- ReactMarkdown requires component * renderers that close over this message's metadata. useMemo keeps them stable until that * metadata changes. */ @@ -1450,7 +1504,9 @@ function ChatMarkdown({ } if (fileLinkMeta.line) { labelParts.push( - `L${fileLinkMeta.line}${fileLinkMeta.column ? `:C${fileLinkMeta.column}` : ""}`, + fileLinkMeta.endLine + ? `L${fileLinkMeta.line}-${fileLinkMeta.endLine}` + : `L${fileLinkMeta.line}${fileLinkMeta.column ? `:C${fileLinkMeta.column}` : ""}`, ); } @@ -1462,11 +1518,13 @@ function ChatMarkdown({ displayPath={fileLinkMeta.displayPath} workspaceRelativePath={fileLinkMeta.workspaceRelativePath} line={fileLinkMeta.line} + endLine={fileLinkMeta.endLine} label={labelParts.join(" · ")} copyMarkdown={copyMarkdown} theme={resolvedTheme} threadRef={threadRef} onOpen={openInPreferredEditor} + onOpenInPanel={openFileInPanel} onOpenInBrowser={ threadRef && isPreviewSupportedInRuntime() && @@ -1685,6 +1743,7 @@ function ChatMarkdown({ isStreaming, markdownFileLinkMetaByHref, onTaskListChange, + openFileInPanel, openInPreferredEditor, openExternalLinkInPreview, openMarkdownFileInPreview, diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx index 32a8e309beb9..fcf36b992930 100644 --- a/apps/web/src/components/ChatView.tsx +++ b/apps/web/src/components/ChatView.tsx @@ -78,6 +78,13 @@ import { collapseExpandedComposerCursor, parseStandaloneComposerSlashCommand, } from "../composer-logic"; +import { resolveComposerMentionFileTarget } from "../composerMentionFileTarget"; +import { + claimWorkspaceBasenameLookup, + needsWorkspaceBasenameLookup, + pickWorkspaceBasenameMatch, + WORKSPACE_BASENAME_LOOKUP_LIMIT, +} from "../workspaceBasenameLookup"; import { derivePendingApprovals, derivePendingUserInputs, @@ -322,6 +329,7 @@ import { sanitizeThreadErrorMessage } from "~/rpc/transportError"; import { RightPanelSheet } from "./RightPanelSheet"; import { previewEnvironment } from "../state/preview"; import { useAtomCommand } from "../state/use-atom-command"; +import { useAtomQueryRunner } from "../state/use-atom-query-runner"; import { Button } from "./ui/button"; import { AlertDialog, @@ -1195,6 +1203,9 @@ function ChatViewContent(props: ChatViewProps) { ); const routeThreadKey = useMemo(() => scopedThreadKey(routeThreadRef), [routeThreadRef]); const updateProject = useAtomCommand(projectEnvironment.update, { reportFailure: false }); + const searchProjectEntries = useAtomQueryRunner(projectEnvironment.searchEntries, { + reportFailure: false, + }); const upsertKeybinding = useAtomCommand(serverEnvironment.upsertKeybinding, { reportFailure: false, }); @@ -3267,6 +3278,43 @@ function ChatViewContent(props: ChatViewProps) { }, [activeProject, activeThreadRef], ); + // A composer mention chip opens its file the same way a rendered chat file + // link does, so a reference you just wrote is readable before you send it. + // A hand-written mention can name a file without saying where it lives, so + // it goes through the same workspace-index lookup chat links use. + const openMentionFileSurface = useCallback( + (mentionPath: string) => { + if (!activeThreadRef || !activeProject) return; + const target = resolveComposerMentionFileTarget(mentionPath, activeWorkspaceRoot); + if (!target) return; + const openAt = (path: string) => + useRightPanelStore.getState().openFile(activeThreadRef, path, target.line, target.endLine); + if (!activeWorkspaceRoot || !needsWorkspaceBasenameLookup(target.relativePath)) { + openAt(target.relativePath); + return; + } + const isLatestLookup = claimWorkspaceBasenameLookup(); + void (async () => { + const result = await searchProjectEntries({ + environmentId: activeThreadRef.environmentId, + input: { + cwd: activeWorkspaceRoot, + query: target.relativePath, + limit: WORKSPACE_BASENAME_LOOKUP_LIMIT, + kind: "file", + }, + }); + const match = + result._tag === "Success" + ? pickWorkspaceBasenameMatch(target.relativePath, result.value.entries) + : null; + // A click that landed after this one already owns the panel. + if (!isLatestLookup()) return; + openAt(match ?? target.relativePath); + })(); + }, + [activeProject, activeThreadRef, activeWorkspaceRoot, searchProjectEntries], + ); // The thread's own change request, placed against the project it belongs to. Without a // project there is nothing to resolve it against, so the caller falls back to the browser. const threadRepository = activeProject?.repositoryIdentity?.displayName ?? null; @@ -6104,6 +6152,7 @@ function ChatViewContent(props: ChatViewProps) { activeRightPanelSurface.kind === "file" ? activeRightPanelSurface.relativePath : null } revealLine={activeFileSurface?.revealLine ?? null} + revealEndLine={activeFileSurface?.revealEndLine ?? null} revealRequestId={activeFileSurface?.revealRequestId ?? 0} onOpenFile={openFileSurface} onPendingChange={handleFilePendingChange} @@ -6352,6 +6401,7 @@ function ChatViewContent(props: ChatViewProps) { keybindings={keybindings} terminalOpen={Boolean(terminalUiState.terminalOpen)} gitCwd={gitCwd} + onOpenMentionFile={openMentionFileSurface} promptRef={promptRef} composerImagesRef={composerImagesRef} composerTerminalContextsRef={composerTerminalContextsRef} diff --git a/apps/web/src/components/ComposerPromptEditor.tsx b/apps/web/src/components/ComposerPromptEditor.tsx index 0489e8c79cdf..efac2af6b84c 100644 --- a/apps/web/src/components/ComposerPromptEditor.tsx +++ b/apps/web/src/components/ComposerPromptEditor.tsx @@ -128,22 +128,41 @@ type SerializedComposerTerminalContextNode = Spread< SerializedLexicalNode >; -const ComposerTerminalContextActionsContext = createContext<{ +const ComposerChipActionsContext = createContext<{ onRemoveTerminalContext: (contextId: string) => void; + // Only a host that can show the mentioned file (the chat surface, via its + // right panel) supplies this; standalone editors like the appearance preview + // leave it null and their mention chips stay inert. + onOpenMentionFile: ((path: string) => void) | null; }>({ onRemoveTerminalContext: () => {}, + onOpenMentionFile: null, }); function ComposerMentionDecorator(props: { path: string }) { const theme = resolvedThemeFromDocument(); + const { onOpenMentionFile } = use(ComposerChipActionsContext); + const path = props.path; const chip = ( ) => { + event.preventDefault(); + event.stopPropagation(); + onOpenMentionFile(path); + }, + } + : {})} > - + ); @@ -151,7 +170,7 @@ function ComposerMentionDecorator(props: { path: string }) { - {props.path} + {path} ); @@ -884,6 +903,7 @@ interface ComposerPromptEditorProps { disabled: boolean; placeholder: string; className?: string; + onOpenMentionFile?: (path: string) => void; onRemoveTerminalContext: (contextId: string) => void; onChange: ( nextValue: string, @@ -1105,7 +1125,7 @@ function ComposerInlineTokenSelectionNormalizePlugin() { function ComposerInlineTokenBackspacePlugin() { const [editor] = useLexicalComposerContext(); - const { onRemoveTerminalContext } = use(ComposerTerminalContextActionsContext); + const { onRemoveTerminalContext } = use(ComposerChipActionsContext); useEffect(() => { return editor.registerCommand( @@ -1533,6 +1553,7 @@ function ComposerPromptEditorInner({ disabled, placeholder, className, + onOpenMentionFile, onRemoveTerminalContext, onChange, onCommandKeyDown, @@ -1554,9 +1575,9 @@ function ComposerPromptEditorInner({ terminalContextIds: terminalContexts.map((context) => context.id), }); const isApplyingControlledUpdateRef = useRef(false); - const terminalContextActions = useMemo( - () => ({ onRemoveTerminalContext }), - [onRemoveTerminalContext], + const chipActions = useMemo( + () => ({ onRemoveTerminalContext, onOpenMentionFile: onOpenMentionFile ?? null }), + [onOpenMentionFile, onRemoveTerminalContext], ); useEffect(() => { @@ -1746,7 +1767,7 @@ function ComposerPromptEditorInner({ }, []); return ( - +
-
+ ); } @@ -1795,6 +1816,7 @@ export function ComposerPromptEditor({ disabled, placeholder, className, + onOpenMentionFile, onRemoveTerminalContext, onChange, onCommandKeyDown, @@ -1836,6 +1858,7 @@ export function ComposerPromptEditor({ onChange={onChange} onPaste={onPaste} editorRef={editorRef} + {...(onOpenMentionFile ? { onOpenMentionFile } : {})} {...(onCommandKeyDown ? { onCommandKeyDown } : {})} {...(className ? { className } : {})} /> diff --git a/apps/web/src/components/chat/ChatComposer.tsx b/apps/web/src/components/chat/ChatComposer.tsx index e918e7758688..d000206cc51c 100644 --- a/apps/web/src/components/chat/ChatComposer.tsx +++ b/apps/web/src/components/chat/ChatComposer.tsx @@ -557,6 +557,8 @@ export interface ChatComposerProps { keybindings: ResolvedKeybindingsConfig; terminalOpen: boolean; gitCwd: string | null; + /** Shows a mention chip's file in the right panel; absent when there is no panel to show it in. */ + onOpenMentionFile?: (path: string) => void; // Refs the parent needs kept in sync promptRef: React.RefObject; @@ -643,6 +645,7 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) keybindings, terminalOpen, gitCwd, + onOpenMentionFile, promptRef, composerRef, composerImagesRef, @@ -3034,6 +3037,7 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) } skills={selectedProviderStatus?.skills ?? []} {...(showMobilePendingAnswerActions ? { className: "max-sm:pb-11" } : {})} + {...(onOpenMentionFile ? { onOpenMentionFile } : {})} onRemoveTerminalContext={removeComposerTerminalContextFromDraft} onChange={onPromptChange} onCommandKeyDown={onComposerCommandKey} diff --git a/apps/web/src/components/files/FilePreviewPanel.tsx b/apps/web/src/components/files/FilePreviewPanel.tsx index f528fe894569..e08e8c2ef2e3 100644 --- a/apps/web/src/components/files/FilePreviewPanel.tsx +++ b/apps/web/src/components/files/FilePreviewPanel.tsx @@ -74,6 +74,7 @@ interface FilePreviewPanelProps { keybindings: ResolvedKeybindingsConfig; availableEditors: ReadonlyArray; revealLine: number | null; + revealEndLine: number | null; revealRequestId: number; onOpenFile: (relativePath: string) => void; onPendingChange: (relativePath: string, pending: boolean) => void; @@ -178,19 +179,30 @@ function clampFileLine(contents: string, requestedLine: number): number { return Math.min(Math.max(1, requestedLine), lineCount); } -function updateFileLinkReveal(fileContainer: HTMLElement, line: number | null): void { +function updateFileLinkReveal( + fileContainer: HTMLElement, + line: number | null, + endLine: number | null, +): void { const root = fileContainer.shadowRoot ?? fileContainer; for (const element of root.querySelectorAll(`[${FILE_LINK_REVEAL_ATTRIBUTE}]`)) { element.removeAttribute(FILE_LINK_REVEAL_ATTRIBUTE); } if (line === null) return; + const lastLine = endLine !== null && endLine > line ? endLine : line; - root - .querySelector(`[data-line="${line}"]`) - ?.setAttribute(FILE_LINK_REVEAL_ATTRIBUTE, ""); - root - .querySelector(`[data-column-number="${line}"]`) - ?.setAttribute(FILE_LINK_REVEAL_ATTRIBUTE, ""); + // Scanning what is mounted rather than querying each number keeps a span of + // any size to one pass, and virtualization means the rows outside the + // viewport are not there to mark anyway — this runs on every post-render, so + // they pick the mark up as they scroll in. + for (const element of root.querySelectorAll("[data-line], [data-column-number]")) { + const rawValue = + element.getAttribute("data-line") ?? element.getAttribute("data-column-number"); + const lineNumber = rawValue === null ? Number.NaN : Number(rawValue); + if (Number.isFinite(lineNumber) && lineNumber >= line && lineNumber <= lastLine) { + element.setAttribute(FILE_LINK_REVEAL_ATTRIBUTE, ""); + } + } } /** @@ -217,6 +229,7 @@ interface FileRevealState { function useFileLineReveal( relativePath: string | null, revealLine: number | null, + revealEndLine: number | null, revealRequestId: number, ): FilePostRender { const [revealStatesByPath] = useState(() => new Map()); @@ -250,7 +263,11 @@ function useFileLineReveal( const contents = instance.file?.contents; const targetLine = revealLine === null || contents === undefined ? null : clampFileLine(contents, revealLine); - updateFileLinkReveal(fileContainer, targetLine); + const targetEndLine = + targetLine === null || revealEndLine === null + ? null + : clampFileLine(contents ?? "", revealEndLine); + updateFileLinkReveal(fileContainer, targetLine, targetEndLine); if (!(instance instanceof VirtualizedFile)) return; @@ -363,11 +380,15 @@ function useFileLineReveal( const line = currentContents === undefined ? null : clampFileLine(currentContents, revealLine); const targetTop = line === null ? null : resolveScrollTarget(line); - if (line === null || targetTop === null) { + if (currentContents === undefined || line === null || targetTop === null) { if (attempt < REVEAL_MAX_ATTEMPTS) scheduleReveal(attempt + 1); return; } - updateFileLinkReveal(fileContainer, line); + updateFileLinkReveal( + fileContainer, + line, + revealEndLine === null ? null : clampFileLine(currentContents, revealEndLine), + ); scrollContainer.scrollTop = targetTop; state.handledRequestId = revealRequestId; @@ -377,7 +398,7 @@ function useFileLineReveal( scheduleReveal(0); }, - [revealStatesByPath, relativePath, revealLine, revealRequestId], + [revealStatesByPath, relativePath, revealEndLine, revealLine, revealRequestId], ); } @@ -764,6 +785,7 @@ export default function FilePreviewPanel({ keybindings, availableEditors, revealLine, + revealEndLine, revealRequestId, onOpenFile, onPendingChange, @@ -809,7 +831,12 @@ export default function FilePreviewPanel({ () => (relativePath ? fileBreadcrumbs(projectName, relativePath) : []), [projectName, relativePath], ); - const onFilePostRender = useFileLineReveal(relativePath, revealLine, revealRequestId); + const onFilePostRender = useFileLineReveal( + relativePath, + revealLine, + revealEndLine, + revealRequestId, + ); useEffect(() => { const currentCrumb = breadcrumbRef.current?.querySelector( diff --git a/apps/web/src/composerMentionFileTarget.test.ts b/apps/web/src/composerMentionFileTarget.test.ts new file mode 100644 index 000000000000..f1db705a5f0a --- /dev/null +++ b/apps/web/src/composerMentionFileTarget.test.ts @@ -0,0 +1,95 @@ +import { describe, expect, it } from "vite-plus/test"; + +import { resolveComposerMentionFileTarget } from "./composerMentionFileTarget"; + +const ROOT = "/Users/dev/t3code"; + +describe("resolveComposerMentionFileTarget", () => { + it("passes workspace-relative mentions straight through", () => { + expect(resolveComposerMentionFileTarget("AGENTS.md", ROOT)).toEqual({ + relativePath: "AGENTS.md", + }); + expect(resolveComposerMentionFileTarget("apps/web/src/main.tsx", ROOT)).toEqual({ + relativePath: "apps/web/src/main.tsx", + }); + }); + + it("relativizes absolute mentions inside the workspace", () => { + expect(resolveComposerMentionFileTarget(`${ROOT}/apps/web/src/main.tsx`, ROOT)).toEqual({ + relativePath: "apps/web/src/main.tsx", + }); + }); + + it("keeps extensionless filenames, which markdown link heuristics reject", () => { + expect(resolveComposerMentionFileTarget("Makefile", ROOT)).toEqual({ + relativePath: "Makefile", + }); + }); + + it("collapses dot segments", () => { + expect(resolveComposerMentionFileTarget("./docs/../AGENTS.md", ROOT)).toEqual({ + relativePath: "AGENTS.md", + }); + }); + + it("carries a :line suffix over as the reveal line", () => { + expect(resolveComposerMentionFileTarget("apps/web/src/main.tsx:42", ROOT)).toEqual({ + relativePath: "apps/web/src/main.tsx", + line: 42, + }); + expect(resolveComposerMentionFileTarget("apps/web/src/main.tsx:42:7", ROOT)).toEqual({ + relativePath: "apps/web/src/main.tsx", + line: 42, + }); + }); + + it("normalizes windows separators", () => { + expect(resolveComposerMentionFileTarget("C:\\repo\\apps\\web\\main.tsx", "C:\\repo")).toEqual({ + relativePath: "apps/web/main.tsx", + }); + }); + + it("works for a workspace rooted at the filesystem root", () => { + expect(resolveComposerMentionFileTarget("/srv/app/main.ts", "/")).toEqual({ + relativePath: "srv/app/main.ts", + }); + }); + + it("keeps a span suffix", () => { + expect(resolveComposerMentionFileTarget("apps/web/src/main.tsx:20-40", ROOT)).toEqual({ + relativePath: "apps/web/src/main.tsx", + line: 20, + endLine: 40, + }); + }); + + // Folding case here would accept a path from outside the workspace on a + // case-sensitive filesystem. + it("does not accept a root whose case does not match", () => { + expect(resolveComposerMentionFileTarget("/users/dev/t3code/Other.ts", ROOT)).toBeNull(); + }); + + it("still matches a windows root whose drive letter case differs", () => { + expect(resolveComposerMentionFileTarget("c:\\repo\\apps\\main.tsx", "C:\\repo")).toEqual({ + relativePath: "apps/main.tsx", + }); + }); + + // `/..` is `/`, so climbing past an absolute root cannot leave `..` behind + // for the file surface to walk. + it("does not let a path climb above the root it resolves against", () => { + expect(resolveComposerMentionFileTarget("../a/secret.ts", "/")).toEqual({ + relativePath: "a/secret.ts", + }); + expect(resolveComposerMentionFileTarget("/x/../../a/secret.ts", "/")).toEqual({ + relativePath: "a/secret.ts", + }); + }); + + it("returns null outside the workspace, and without one", () => { + expect(resolveComposerMentionFileTarget("/etc/hosts", ROOT)).toBeNull(); + expect(resolveComposerMentionFileTarget("../sibling/AGENTS.md", ROOT)).toBeNull(); + expect(resolveComposerMentionFileTarget("AGENTS.md", undefined)).toBeNull(); + expect(resolveComposerMentionFileTarget(" ", ROOT)).toBeNull(); + }); +}); diff --git a/apps/web/src/composerMentionFileTarget.ts b/apps/web/src/composerMentionFileTarget.ts new file mode 100644 index 000000000000..2e5f3ed96f22 --- /dev/null +++ b/apps/web/src/composerMentionFileTarget.ts @@ -0,0 +1,83 @@ +import { resolvePathLinkTarget, splitPathAndPosition } from "./terminal-links"; + +export interface ComposerMentionFileTarget { + readonly relativePath: string; + readonly line?: number; + /** Present when the mention names a span of lines (`@Foo.ts:20-40`). */ + readonly endLine?: number; +} + +function toPosixPath(path: string): string { + const normalized = path.replaceAll("\\", "/"); + // Windows drive paths pick up a leading slash on the way through URL-ish + // plumbing (`/C:/repo`); the workspace root never carries one. The drive + // letter itself is case-insensitive, so it is folded to make the containment + // check below comparable without folding the rest of the path. + const withoutLeadingSlash = /^\/[A-Za-z]:\//.test(normalized) ? normalized.slice(1) : normalized; + return /^[A-Za-z]:\//.test(withoutLeadingSlash) + ? `${withoutLeadingSlash[0]?.toUpperCase() ?? ""}${withoutLeadingSlash.slice(1)}` + : withoutLeadingSlash; +} + +function collapseDotSegments(path: string): string { + const isAbsolute = path.startsWith("/"); + const segments: string[] = []; + for (const segment of path.split("/")) { + if (segment === "" || segment === ".") continue; + if (segment === "..") { + if (segments.length > 0 && segments[segments.length - 1] !== "..") { + segments.pop(); + continue; + } + // `/..` is `/`: an absolute path cannot climb above the root, and a + // surviving leading `..` would reach the file surface as a relative path + // that walks out of the workspace. + if (isAbsolute) continue; + } + segments.push(segment); + } + const joined = segments.join("/"); + return isAbsolute ? `/${joined}` : joined; +} + +/** + * Resolve the file a composer mention chip points at, in the form the right + * panel wants: a path relative to the workspace root the file preview reads. + * + * Autocomplete, drag-and-drop, and the file browser all insert + * workspace-relative paths, but a hand-typed mention can be absolute, use + * `~/`, or carry a `:line` suffix. Anything that lands outside the workspace + * has no preview surface, so it resolves to `null` and the chip stays inert. + */ +export function resolveComposerMentionFileTarget( + mentionPath: string, + workspaceRoot: string | undefined, +): ComposerMentionFileTarget | null { + const trimmed = mentionPath.trim(); + if (!trimmed || !workspaceRoot) return null; + + const { path, line, endLine } = splitPathAndPosition(trimmed); + if (!path) return null; + + const absolute = collapseDotSegments(toPosixPath(resolvePathLinkTarget(path, workspaceRoot))); + const normalizedRoot = collapseDotSegments(toPosixPath(workspaceRoot)); + // A workspace rooted at `/` strips to nothing, which would leave every + // mention inert; the empty prefix is what makes the check below read `/x` + // as the relative `x`. + const root = normalizedRoot === "/" ? "" : normalizedRoot.replace(/\/+$/, ""); + if (!root && normalizedRoot !== "/") return null; + // Compared exactly: folding case would accept `/users/dev/repo/x.ts` against + // a `/Users/dev/repo` root on a case-sensitive filesystem and hand the panel + // a path from outside the workspace. + if (!absolute.startsWith(`${root}/`)) return null; + + const relativePath = absolute.slice(root.length + 1); + if (!relativePath) return null; + + const parsedLine = line ? Number.parseInt(line, 10) : Number.NaN; + if (!Number.isFinite(parsedLine)) return { relativePath }; + const parsedEndLine = endLine ? Number.parseInt(endLine, 10) : Number.NaN; + return Number.isFinite(parsedEndLine) && parsedEndLine > parsedLine + ? { relativePath, line: parsedLine, endLine: parsedEndLine } + : { relativePath, line: parsedLine }; +} diff --git a/apps/web/src/filePathDisplay.ts b/apps/web/src/filePathDisplay.ts index 5a6e2a02e100..a5ccb7f34a5c 100644 --- a/apps/web/src/filePathDisplay.ts +++ b/apps/web/src/filePathDisplay.ts @@ -1,4 +1,4 @@ -import { splitPathAndPosition } from "./terminal-links"; +import { formatPathPosition, splitPathAndPosition } from "./terminal-links"; function normalizePathSeparators(path: string): string { return path.replaceAll("\\", "/"); @@ -25,7 +25,7 @@ export function formatWorkspaceRelativePath( pathWithPosition: string, workspaceRoot: string | undefined, ): string { - const { path, line, column } = splitPathAndPosition(pathWithPosition); + const { path, line, endLine, column } = splitPathAndPosition(pathWithPosition); const normalizedPath = canonicalizeWindowsDrivePath(normalizePathSeparators(path)); let displayPath = normalizedPath; @@ -52,6 +52,5 @@ export function formatWorkspaceRelativePath( } } - if (!line) return displayPath; - return `${displayPath}:${line}${column ? `:${column}` : ""}`; + return `${displayPath}${formatPathPosition(line, endLine, column)}`; } diff --git a/apps/web/src/markdown-links.test.ts b/apps/web/src/markdown-links.test.ts index 9fc296138672..d05cdbd04607 100644 --- a/apps/web/src/markdown-links.test.ts +++ b/apps/web/src/markdown-links.test.ts @@ -273,3 +273,45 @@ describe("resolveInlineCodeFileLinkMeta", () => { expect(resolveInlineCodeFileLinkMeta(".plans/worktree-management-v1.md")).toBeNull(); }); }); + +describe("line span references", () => { + const cwd = "/workspace"; + + it("reads a :start-end suffix as a span", () => { + expect(resolveInlineCodeFileLinkMeta("src/index.ts:20-40", cwd)).toMatchObject({ + filePath: "/workspace/src/index.ts", + line: 20, + endLine: 40, + }); + }); + + it("reads a span on a bare filename, which needs the suffix to link at all", () => { + expect(resolveInlineCodeFileLinkMeta("index.ts:20-40", cwd)).toMatchObject({ + line: 20, + endLine: 40, + workspaceRelativePath: "index.ts", + }); + }); + + it("reads GitHub's #L20-L40 hash form", () => { + expect(resolveMarkdownFileLinkMeta("src/index.ts#L20-L40", cwd)).toMatchObject({ + line: 20, + endLine: 40, + }); + }); + + it("ignores a span that does not run forwards", () => { + expect(resolveInlineCodeFileLinkMeta("src/index.ts:40-20", cwd)?.endLine).toBeUndefined(); + expect(resolveInlineCodeFileLinkMeta("src/index.ts:20-20", cwd)?.endLine).toBeUndefined(); + }); + + it("leaves single-line and line:column references alone", () => { + expect(resolveInlineCodeFileLinkMeta("src/index.ts:20", cwd)).toMatchObject({ line: 20 }); + expect(resolveInlineCodeFileLinkMeta("src/index.ts:20", cwd)?.endLine).toBeUndefined(); + expect(resolveInlineCodeFileLinkMeta("src/index.ts:20:5", cwd)).toMatchObject({ + line: 20, + column: 5, + }); + expect(resolveInlineCodeFileLinkMeta("src/index.ts:20:5", cwd)?.endLine).toBeUndefined(); + }); +}); diff --git a/apps/web/src/markdown-links.ts b/apps/web/src/markdown-links.ts index a6dba941b8ac..afd115346ade 100644 --- a/apps/web/src/markdown-links.ts +++ b/apps/web/src/markdown-links.ts @@ -5,10 +5,16 @@ const WINDOWS_DRIVE_PATH_PATTERN = /^[A-Za-z]:[\\/]/; const WINDOWS_UNC_PATH_PATTERN = /^\\\\/; const EXTERNAL_SCHEME_PATTERN = /^([A-Za-z][A-Za-z0-9+.-]*):(.*)$/; const RELATIVE_PATH_PREFIX_PATTERN = /^(~\/|\.{1,2}\/)/; -const RELATIVE_FILE_PATH_PATTERN = /^[A-Za-z0-9._-]+(?:\/[A-Za-z0-9._-]+)+(?::\d+){0,2}$/; -const RELATIVE_FILE_NAME_PATTERN = /^[A-Za-z0-9._-]+\.[A-Za-z0-9_-]+(?::\d+){0,2}$/; -const POSITION_SUFFIX_PATTERN = /:\d+(?::\d+)?$/; -const POSITION_ONLY_PATTERN = /^\d+(?::\d+)?$/; +const POSITION_SUFFIX_SOURCE = "(?::\\d+(?::\\d+|-\\d+)?)?"; +const RELATIVE_FILE_PATH_PATTERN = new RegExp( + `^[A-Za-z0-9._-]+(?:/[A-Za-z0-9._-]+)+${POSITION_SUFFIX_SOURCE}$`, +); +const RELATIVE_FILE_NAME_PATTERN = new RegExp( + `^[A-Za-z0-9._-]+\\.[A-Za-z0-9_-]+${POSITION_SUFFIX_SOURCE}$`, +); +// `:12`, `:12:5`, and the span form `:12-40`. +const POSITION_SUFFIX_PATTERN = /:\d+(?::\d+|-\d+)?$/; +const POSITION_ONLY_PATTERN = /^\d+(?::\d+|-\d+)?$/; // Standard OS and dev-container roots; deliberately excludes app-route-ish // prefixes like /app/ or /chat/ so SPA routes never read as files. const POSIX_FILE_ROOT_PREFIXES = [ @@ -45,6 +51,8 @@ export interface MarkdownFileLinkMeta { workspaceRelativePath: string | null; basename: string; line?: number; + /** Present when the reference names a span of lines (`Foo.ts:20-40`). */ + endLine?: number; column?: number; } @@ -118,6 +126,10 @@ function looksLikePosixFilesystemPath(path: string): boolean { function appendLineColumnFromHash(path: string, hash: string): string { if (!hash || POSITION_SUFFIX_PATTERN.test(path)) return path; + const spanMatch = hash.match(/^#L(\d+)-L?(\d+)$/i); + if (spanMatch?.[1] && spanMatch[2]) { + return `${path}:${spanMatch[1]}-${spanMatch[2]}`; + } const match = hash.match(/^#L(\d+)(?:C(\d+))?$/i); if (!match?.[1]) return path; const line = match[1]; @@ -190,7 +202,7 @@ const INLINE_CODE_DISQUALIFIER_PATTERN = /[\s`]/; const PATH_SEPARATOR_PATTERN = /[\\/]/; const FILE_EXTENSION_PATTERN = /\.[A-Za-z0-9_-]+$/; const NUMERIC_DOTTED_PATTERN = /^\d+(?:\.\d+)+$/; -const BARE_EXTENSIONLESS_POSITION_PATTERN = /^[A-Za-z0-9_-]+(?::\d+){1,2}$/; +const BARE_EXTENSIONLESS_POSITION_PATTERN = /^[A-Za-z0-9_-]+:\d+(?::\d+|-\d+)?$/; // Any `Name:digits` shape also matches `error:1`, `port:3000`, `TODO:12`, so // extensionless linking is limited to conventional filenames. const EXTENSIONLESS_FILE_NAMES = new Set([ @@ -386,10 +398,16 @@ export function resolveMarkdownFileLinkMeta( } function buildFileLinkMetaFromTarget(targetPath: string, cwd?: string): MarkdownFileLinkMeta { - const { path, line, column } = splitPathAndPosition(targetPath); + const { path, line, endLine, column } = splitPathAndPosition(targetPath); const parsedLine = line ? Number.parseInt(line, 10) : Number.NaN; + const parsedEndLine = endLine ? Number.parseInt(endLine, 10) : Number.NaN; const parsedColumn = column ? Number.parseInt(column, 10) : Number.NaN; const lineNumber = Number.isFinite(parsedLine) ? parsedLine : undefined; + // A backwards or degenerate span (`:40-20`, `:20-20`) is just its start line. + const endLineNumber = + Number.isFinite(parsedEndLine) && lineNumber !== undefined && parsedEndLine > lineNumber + ? parsedEndLine + : undefined; const columnNumber = Number.isFinite(parsedColumn) ? parsedColumn : undefined; return { @@ -399,6 +417,7 @@ function buildFileLinkMetaFromTarget(targetPath: string, cwd?: string): Markdown workspaceRelativePath: workspaceRelativePath(path, cwd), basename: basenameOfPath(path), ...(lineNumber !== undefined ? { line: lineNumber } : {}), + ...(endLineNumber !== undefined ? { endLine: endLineNumber } : {}), ...(columnNumber !== undefined ? { column: columnNumber } : {}), }; } diff --git a/apps/web/src/rightPanelStore.test.ts b/apps/web/src/rightPanelStore.test.ts index 039f8ef72301..dcd0c4a7a664 100644 --- a/apps/web/src/rightPanelStore.test.ts +++ b/apps/web/src/rightPanelStore.test.ts @@ -96,6 +96,7 @@ describe("rightPanelStore", () => { kind: "file", relativePath: "src/index.ts", revealLine: null, + revealEndLine: null, revealRequestId: 0, }, ], @@ -266,6 +267,7 @@ describe("rightPanelStore", () => { kind: "file", relativePath: "src/index.ts", revealLine: null, + revealEndLine: null, revealRequestId: 2, }, { @@ -273,12 +275,35 @@ describe("rightPanelStore", () => { kind: "file", relativePath: "README.md", revealLine: null, + revealEndLine: null, revealRequestId: 1, }, ], }); }); + it("keeps a revealed span, and drops one that does not run forwards", () => { + useRightPanelStore.getState().openFile(refA, "src/index.ts", 42, 87); + expect( + selectThreadRightPanelState(useRightPanelStore.getState().byThreadKey, refA).surfaces, + ).toEqual([ + { + id: "file:src/index.ts", + kind: "file", + relativePath: "src/index.ts", + revealLine: 42, + revealEndLine: 87, + revealRequestId: 1, + }, + ]); + + useRightPanelStore.getState().openFile(refA, "src/index.ts", 42, 42); + useRightPanelStore.getState().openFile(refA, "src/index.ts", 42, 10); + expect( + selectThreadRightPanelState(useRightPanelStore.getState().byThreadKey, refA).surfaces[0], + ).toMatchObject({ revealLine: 42, revealEndLine: null }); + }); + it("updates line reveal requests when reopening a file surface", () => { useRightPanelStore.getState().openFile(refA, "src/index.ts", 42); useRightPanelStore.getState().openFile(refA, "src/index.ts", 87); @@ -292,6 +317,7 @@ describe("rightPanelStore", () => { kind: "file", relativePath: "src/index.ts", revealLine: 87, + revealEndLine: null, revealRequestId: 2, }, ], @@ -308,6 +334,7 @@ describe("rightPanelStore", () => { kind: "file", relativePath: "src/index.ts", revealLine: null, + revealEndLine: null, revealRequestId: 3, }, ], @@ -528,6 +555,7 @@ describe("rightPanelStore", () => { kind: "file", relativePath: "src/index.ts", revealLine: null, + revealEndLine: null, revealRequestId: 1, }, ], diff --git a/apps/web/src/rightPanelStore.ts b/apps/web/src/rightPanelStore.ts index 5adee07a1851..b06dafa00dc0 100644 --- a/apps/web/src/rightPanelStore.ts +++ b/apps/web/src/rightPanelStore.ts @@ -43,6 +43,8 @@ export type RightPanelSurface = kind: "file"; relativePath: string; revealLine: number | null; + /** End of a revealed span; null when the reference named a single line. */ + revealEndLine: number | null; revealRequestId: number; } | { @@ -83,7 +85,7 @@ interface RightPanelStoreState { kind: Exclude, ) => void; openBrowser: (ref: ScopedThreadRef, tabId: string | null) => void; - openFile: (ref: ScopedThreadRef, relativePath: string, line?: number) => void; + openFile: (ref: ScopedThreadRef, relativePath: string, line?: number, endLine?: number) => void; openPullRequest: ( ref: ScopedThreadRef, target: { projectId: string; repository: string; number: number }, @@ -141,12 +143,14 @@ const browserSurface = (tabId: string | null): RightPanelSurface => const fileSurface = ( relativePath: string, revealLine: number | null, + revealEndLine: number | null, revealRequestId: number, ): RightPanelSurface => ({ id: `file:${relativePath}`, kind: "file", relativePath, revealLine, + revealEndLine, revealRequestId, }); @@ -248,7 +252,14 @@ export function migratePersistedRightPanelState(persistedState: unknown): { surface.revealRequestId >= 0 ? surface.revealRequestId : 0; - return [{ ...surface, revealLine, revealRequestId }]; + const revealEndLine = + typeof surface.revealEndLine === "number" && + Number.isFinite(surface.revealEndLine) && + revealLine !== null && + surface.revealEndLine > revealLine + ? Math.trunc(surface.revealEndLine) + : null; + return [{ ...surface, revealLine, revealEndLine, revealRequestId }]; } if (surface.kind === "pull-request") { if ( @@ -353,7 +364,7 @@ export const useRightPanelStore = create()( return upsertSurface(current, pullRequestSurface(target)); }), })), - openFile: (ref, relativePath, line) => + openFile: (ref, relativePath, line, endLine) => set((state) => ({ byThreadKey: updateThread(state.byThreadKey, scopedThreadKey(ref), (current) => { const withoutStandaloneExplorer = current.surfaces.filter( @@ -364,9 +375,16 @@ export const useRightPanelStore = create()( (surface): surface is Extract => surface.id === surfaceId && surface.kind === "file", ); + const normalizedLine = normalizeRevealLine(line); + const normalizedEndLine = normalizeRevealLine(endLine); const surface = fileSurface( relativePath, - normalizeRevealLine(line), + normalizedLine, + normalizedLine !== null && + normalizedEndLine !== null && + normalizedEndLine > normalizedLine + ? normalizedEndLine + : null, (existing?.revealRequestId ?? 0) + 1, ); return { diff --git a/apps/web/src/terminal-links.ts b/apps/web/src/terminal-links.ts index a4eeda4279cc..6478bbe0290f 100644 --- a/apps/web/src/terminal-links.ts +++ b/apps/web/src/terminal-links.ts @@ -137,18 +137,42 @@ function inferHomeFromCwd(cwd: string): string | undefined { return undefined; } +export function formatPathPosition( + line: string | undefined, + endLine: string | undefined, + column: string | undefined, +): string { + if (!line) return ""; + if (endLine) return `:${line}-${endLine}`; + return `:${line}${column ? `:${column}` : ""}`; +} + export function splitPathAndPosition(value: string): { path: string; line: string | undefined; + /** Set only for a `:start-end` span; a span never carries a column. */ + endLine: string | undefined; column: string | undefined; } { let path = value; let column: string | undefined; let line: string | undefined; + // A span is checked first: its trailing number would otherwise read as a + // column and leave `-` stranded on the path. + const spanMatch = path.match(/:(\d+)-(\d+)$/); + if (spanMatch?.[1] && spanMatch[2]) { + return { + path: path.slice(0, -spanMatch[0].length), + line: spanMatch[1], + endLine: spanMatch[2], + column: undefined, + }; + } + const columnMatch = path.match(/:(\d+)$/); if (!columnMatch?.[1]) { - return { path, line: undefined, column: undefined }; + return { path, line: undefined, endLine: undefined, column: undefined }; } column = columnMatch[1]; @@ -163,7 +187,7 @@ export function splitPathAndPosition(value: string): { column = undefined; } - return { path, line, column }; + return { path, line, endLine: undefined, column }; } export function extractTerminalLinks(line: string): TerminalLinkMatch[] { @@ -267,7 +291,7 @@ export function isTerminalLinkActivation( } export function resolvePathLinkTarget(rawPath: string, cwd: string): string { - const { path, line, column } = splitPathAndPosition(rawPath); + const { path, line, endLine, column } = splitPathAndPosition(rawPath); let resolvedPath = path; if (path.startsWith("~/")) { @@ -281,6 +305,5 @@ export function resolvePathLinkTarget(rawPath: string, cwd: string): string { resolvedPath = joinPath(cwd, path, separator); } - if (!line) return resolvedPath; - return `${resolvedPath}:${line}${column ? `:${column}` : ""}`; + return `${resolvedPath}${formatPathPosition(line, endLine, column)}`; } diff --git a/apps/web/src/workspaceBasenameLookup.test.ts b/apps/web/src/workspaceBasenameLookup.test.ts new file mode 100644 index 000000000000..660d2798e664 --- /dev/null +++ b/apps/web/src/workspaceBasenameLookup.test.ts @@ -0,0 +1,84 @@ +import { describe, expect, it } from "vite-plus/test"; + +import { + claimWorkspaceBasenameLookup, + needsWorkspaceBasenameLookup, + 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); + }); +}); + +describe("pickWorkspaceBasenameMatch", () => { + const entries = [ + { path: "apps/web/src/components/ChatView.test.tsx", kind: "file" as const }, + { path: "apps/web/src/components/ChatView.tsx", kind: "file" as const }, + ]; + + it("takes the first exact filename match, not the closest fuzzy one", () => { + expect(pickWorkspaceBasenameMatch("ChatView.tsx", entries)).toBe( + "apps/web/src/components/ChatView.tsx", + ); + }); + + it("ignores directories", () => { + expect( + pickWorkspaceBasenameMatch("components", [ + { path: "apps/web/src/components", kind: "directory" }, + { path: "apps/web/src/components/components", kind: "file" }, + ]), + ).toBe("apps/web/src/components/components"); + }); + + it("prefers the exactly-cased file over a case-only twin", () => { + expect( + pickWorkspaceBasenameMatch("foo.ts", [ + { path: "src/Foo.ts", kind: "file" }, + { path: "src/foo.ts", kind: "file" }, + ]), + ).toBe("src/foo.ts"); + }); + + it("falls back to case-insensitive when only the casing differs", () => { + expect(pickWorkspaceBasenameMatch("chatview.tsx", entries)).toBe( + "apps/web/src/components/ChatView.tsx", + ); + }); + + it("returns null when nothing matches the name", () => { + expect(pickWorkspaceBasenameMatch("ChatView.tsx", [])).toBeNull(); + expect( + pickWorkspaceBasenameMatch("ChatView.tsx", [ + { path: "apps/web/src/components/ChatHeader.tsx", kind: "file" }, + ]), + ).toBeNull(); + }); +}); + +describe("claimWorkspaceBasenameLookup", () => { + it("keeps only the newest claim, whatever order the lookups settle in", () => { + const first = claimWorkspaceBasenameLookup(); + const second = claimWorkspaceBasenameLookup(); + + // The older lookup answering last must not reopen the panel behind the + // newer one. + expect(second()).toBe(true); + expect(first()).toBe(false); + }); + + it("stays valid while it is the only claim", () => { + const only = claimWorkspaceBasenameLookup(); + expect(only()).toBe(true); + expect(only()).toBe(true); + }); +}); diff --git a/apps/web/src/workspaceBasenameLookup.ts b/apps/web/src/workspaceBasenameLookup.ts new file mode 100644 index 000000000000..788dd563bb94 --- /dev/null +++ b/apps/web/src/workspaceBasenameLookup.ts @@ -0,0 +1,81 @@ +/** + * Agent output refers to files by name and line — `ChatView.tsx:3301` — far + * more often than by full path. The `:line` suffix is what marks the span as a + * file reference, but the name alone says nothing about where the file lives, + * so the link resolver can only place it at the workspace root, where it + * almost never is. Opening one of those links then fails with + * "Failed to read workspace file 'ChatView.tsx' in ''". + * + * These helpers let the click path ask the workspace index where that basename + * actually lives before opening the file surface. + */ + +/** + * Enough index hits to look past same-named neighbours (`ChatView.test.tsx`) + * without asking the environment for a full listing on a single click. + */ +export const WORKSPACE_BASENAME_LOOKUP_LIMIT = 25; + +/** + * Sequence for in-flight lookups. Two clicks on bare filenames can be resolving + * at once, and nothing guarantees the index answers them in order — so an older + * answer landing last would move the panel off the file the user asked for. + * + * One counter covers every caller on purpose: they all open the same visible + * panel, so "newest click wins" is the behaviour regardless of which one + * started the lookup. + */ +let latestLookupSequence = 0; + +/** + * Claims the newest lookup. Call the returned predicate once the search + * settles: false means a later click has superseded this one and its result + * must be dropped. + */ +export function claimWorkspaceBasenameLookup(): () => boolean { + latestLookupSequence += 1; + const claimed = latestLookupSequence; + return () => claimed === latestLookupSequence; +} + +export interface WorkspaceEntryCandidate { + readonly path: string; + 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; +} + +/** + * True when a workspace-relative path is a bare filename, which is the only + * shape that can have come from a reference with no directory in it. Anything + * carrying a separator was already resolved against a real location. + */ +export function needsWorkspaceBasenameLookup(relativePath: string): boolean { + const trimmed = relativePath.trim(); + return trimmed.length > 0 && !trimmed.includes("/") && !trimmed.includes("\\"); +} + +/** + * The best index entry for a basename, or null to leave the path alone. Search + * results arrive ranked, so the first exact filename match wins; a fuzzy match + * on some other file is worse than the honest "not found" error. + */ +export function pickWorkspaceBasenameMatch( + basename: string, + entries: ReadonlyArray, +): string | null { + const target = basename.trim(); + if (!target) return null; + const files = entries.filter((entry) => entry.kind === "file"); + // Exact first: a workspace holding both `Foo.ts` and `foo.ts` must not open + // whichever the index happened to rank higher. The case-insensitive pass + // then covers a reference whose casing drifted from the file on disk, which + // is the common shape on macOS and Windows. + const exact = files.find((entry) => basenameOfPath(entry.path) === target); + if (exact) return exact.path; + const folded = target.toLowerCase(); + return files.find((entry) => basenameOfPath(entry.path).toLowerCase() === folded)?.path ?? null; +}