From b269a492cab19454be214aeea2eff91bb28595fe Mon Sep 17 00:00:00 2001 From: Rodrigo Brechard Date: Wed, 12 Aug 2026 10:54:56 +0200 Subject: [PATCH 1/3] feat(web): file references open the file and lines they name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A reference like `ChatView.tsx:3301` is only linkified because of its `:line` suffix, but a bare filename says nothing about where the file lives, so it resolved against the workspace root and clicking it failed with "Failed to read workspace file 'ChatView.tsx'". Agent output names files that way constantly, so most such links were dead. Bare filenames now go through the workspace index on click and take the first exact filename match, leaving the path (and the honest error) alone when nothing matches. Line spans are new: `Foo.ts:20-40` and GitHub's `Foo.ts#L20-L40` parse into an end line that the file surface carries into the preview, which highlights the whole range and centres its first line. Spans that do not run forwards collapse to their start line. Previously neither form linkified at all — the position pattern only allowed `:12` and `:12:5`, and the external-scheme guard read `:20-40` as a URI scheme. A span also reaches the external-editor launcher, whose target pattern had no case for one; it now opens at the span's first line rather than failing on a path that does not exist. Composer mention chips are clickable too, opening in the right panel the same way a rendered chat link does, so a reference is readable before you send it. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/process/externalLauncher.test.ts | 32 +++++++++ apps/server/src/process/externalLauncher.ts | 4 +- apps/web/src/components/ChatMarkdown.tsx | 61 ++++++++++++++++- apps/web/src/components/ChatView.tsx | 46 +++++++++++++ .../src/components/ComposerPromptEditor.tsx | 43 +++++++++--- apps/web/src/components/chat/ChatComposer.tsx | 4 ++ .../src/components/files/FilePreviewPanel.tsx | 51 +++++++++++---- .../web/src/composerMentionFileTarget.test.ts | 58 +++++++++++++++++ apps/web/src/composerMentionFileTarget.ts | 65 +++++++++++++++++++ apps/web/src/filePathDisplay.ts | 7 +- apps/web/src/markdown-links.test.ts | 42 ++++++++++++ apps/web/src/markdown-links.ts | 31 +++++++-- apps/web/src/rightPanelStore.test.ts | 28 ++++++++ apps/web/src/rightPanelStore.ts | 26 ++++++-- apps/web/src/terminal-links.ts | 33 ++++++++-- apps/web/src/workspaceBasenameLookup.test.ts | 50 ++++++++++++++ apps/web/src/workspaceBasenameLookup.ts | 55 ++++++++++++++++ 17 files changed, 591 insertions(+), 45 deletions(-) create mode 100644 apps/web/src/composerMentionFileTarget.test.ts create mode 100644 apps/web/src/composerMentionFileTarget.ts create mode 100644 apps/web/src/workspaceBasenameLookup.test.ts create mode 100644 apps/web/src/workspaceBasenameLookup.ts diff --git a/apps/server/src/process/externalLauncher.test.ts b/apps/server/src/process/externalLauncher.test.ts index 1ab6166e92a1..797b12726786 100644 --- a/apps/server/src/process/externalLauncher.test.ts +++ b/apps/server/src/process/externalLauncher.test.ts @@ -132,6 +132,38 @@ 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)), +); + 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..c4c63f8b9dbb 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", diff --git a/apps/web/src/components/ChatMarkdown.tsx b/apps/web/src/components/ChatMarkdown.tsx index 8dc545d31eb0..9e2483faced0 100644 --- a/apps/web/src/components/ChatMarkdown.tsx +++ b/apps/web/src/components/ChatMarkdown.tsx @@ -90,6 +90,12 @@ 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 { + needsWorkspaceBasenameLookup, + pickWorkspaceBasenameMatch, + WORKSPACE_BASENAME_LOOKUP_LIMIT, +} from "../workspaceBasenameLookup"; import { useOpenChangeRequestLink } from "~/lib/openPullRequestLink"; import { writeTextToClipboard } from "../hooks/useCopyToClipboard"; import { isPreviewSupportedInRuntime } from "../previewStateStore"; @@ -786,11 +792,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 +1100,13 @@ const MarkdownFileLink = memo(function MarkdownFileLink({ displayPath, workspaceRelativePath, line, + endLine, label, copyMarkdown, theme, threadRef, onOpen, + onOpenInPanel, onOpenInBrowser, className, }: MarkdownFileLinkProps) { @@ -1136,8 +1150,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 +1322,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 +1348,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 +1453,37 @@ 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; + } + 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; + 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 +1500,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 +1514,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 +1739,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..2d727cb82c47 100644 --- a/apps/web/src/components/ChatView.tsx +++ b/apps/web/src/components/ChatView.tsx @@ -78,6 +78,12 @@ import { collapseExpandedComposerCursor, parseStandaloneComposerSlashCommand, } from "../composer-logic"; +import { resolveComposerMentionFileTarget } from "../composerMentionFileTarget"; +import { + needsWorkspaceBasenameLookup, + pickWorkspaceBasenameMatch, + WORKSPACE_BASENAME_LOOKUP_LIMIT, +} from "../workspaceBasenameLookup"; import { derivePendingApprovals, derivePendingUserInputs, @@ -322,6 +328,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 +1202,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 +3277,40 @@ 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; + } + 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; + 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 +6148,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 +6397,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..be2a5bd9392f --- /dev/null +++ b/apps/web/src/composerMentionFileTarget.test.ts @@ -0,0 +1,58 @@ +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("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..297dcc9ee2db --- /dev/null +++ b/apps/web/src/composerMentionFileTarget.ts @@ -0,0 +1,65 @@ +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. + return /^\/[A-Za-z]:\//.test(normalized) ? normalized.slice(1) : normalized; +} + +function collapseDotSegments(path: string): string { + const isAbsolute = path.startsWith("/"); + const segments: string[] = []; + for (const segment of path.split("/")) { + if (segment === "" || segment === ".") continue; + if (segment === ".." && segments.length > 0 && segments[segments.length - 1] !== "..") { + segments.pop(); + 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 root = collapseDotSegments(toPosixPath(workspaceRoot)).replace(/\/+$/, ""); + if (!root) return null; + if (!absolute.toLowerCase().startsWith(`${root.toLowerCase()}/`)) 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..109b017669c2 --- /dev/null +++ b/apps/web/src/workspaceBasenameLookup.test.ts @@ -0,0 +1,50 @@ +import { describe, expect, it } from "vite-plus/test"; + +import { + 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("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(); + }); +}); diff --git a/apps/web/src/workspaceBasenameLookup.ts b/apps/web/src/workspaceBasenameLookup.ts new file mode 100644 index 000000000000..1f752cbf94b2 --- /dev/null +++ b/apps/web/src/workspaceBasenameLookup.ts @@ -0,0 +1,55 @@ +/** + * 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; + +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().toLowerCase(); + if (!target) return null; + for (const entry of entries) { + if (entry.kind !== "file") continue; + if (basenameOfPath(entry.path).toLowerCase() === target) return entry.path; + } + return null; +} From 7ebb9b651cf36b7f43a809d0b8c667bb4a1549fc Mon Sep 17 00:00:00 2001 From: Rodrigo Brechard Date: Wed, 12 Aug 2026 11:05:53 +0200 Subject: [PATCH 2/3] fix(server,web): address review findings on file reference links - `--goto` editors (VS Code, Cursor, and friends) were handed a raw `path:20-40`, which they read as part of the filename. The argument is now rebuilt from the parse as `path:line[:column]`; every non-span shape produces the same string it did before. - Two bare-filename lookups resolving out of order let the older answer land last and move the panel off the file the newest click asked for. Lookups now claim a sequence and drop their result once superseded. - `pickWorkspaceBasenameMatch` folded case, so a workspace holding both `Foo.ts` and `foo.ts` could open whichever the index ranked higher. Exact filename match wins now, with the case-insensitive pass kept as a fallback for a reference whose casing drifted from disk. - The workspace containment check folded case too, which accepted `/users/dev/repo/x.ts` against a `/Users/dev/repo` root on a case-sensitive filesystem. It compares exactly now; only a Windows drive letter is folded. - A workspace rooted at `/` stripped to an empty prefix and left every mention chip inert. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/process/externalLauncher.test.ts | 41 +++++++++++++++++++ apps/server/src/process/externalLauncher.ts | 14 ++++++- apps/web/src/components/ChatMarkdown.tsx | 4 ++ apps/web/src/components/ChatView.tsx | 4 ++ .../web/src/composerMentionFileTarget.test.ts | 26 ++++++++++++ apps/web/src/composerMentionFileTarget.ts | 22 +++++++--- apps/web/src/workspaceBasenameLookup.test.ts | 34 +++++++++++++++ apps/web/src/workspaceBasenameLookup.ts | 38 ++++++++++++++--- 8 files changed, 171 insertions(+), 12 deletions(-) diff --git a/apps/server/src/process/externalLauncher.test.ts b/apps/server/src/process/externalLauncher.test.ts index 797b12726786..cf39ee263d3e 100644 --- a/apps/server/src/process/externalLauncher.test.ts +++ b/apps/server/src/process/externalLauncher.test.ts @@ -164,6 +164,47 @@ it.effect("opens a line span at its first line", () => }).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 c4c63f8b9dbb..164808dcb457 100644 --- a/apps/server/src/process/externalLauncher.ts +++ b/apps/server/src/process/externalLauncher.ts @@ -135,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 9e2483faced0..884a4e8cccbb 100644 --- a/apps/web/src/components/ChatMarkdown.tsx +++ b/apps/web/src/components/ChatMarkdown.tsx @@ -92,6 +92,7 @@ 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, @@ -1465,6 +1466,7 @@ function ChatMarkdown({ openAt(workspaceRelativePath); return; } + const isLatestLookup = claimWorkspaceBasenameLookup(); void (async () => { const result = await searchProjectEntries({ environmentId: threadRef.environmentId, @@ -1479,6 +1481,8 @@ function ChatMarkdown({ 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); })(); }, diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx index 2d727cb82c47..fcf36b992930 100644 --- a/apps/web/src/components/ChatView.tsx +++ b/apps/web/src/components/ChatView.tsx @@ -80,6 +80,7 @@ import { } from "../composer-logic"; import { resolveComposerMentionFileTarget } from "../composerMentionFileTarget"; import { + claimWorkspaceBasenameLookup, needsWorkspaceBasenameLookup, pickWorkspaceBasenameMatch, WORKSPACE_BASENAME_LOOKUP_LIMIT, @@ -3292,6 +3293,7 @@ function ChatViewContent(props: ChatViewProps) { openAt(target.relativePath); return; } + const isLatestLookup = claimWorkspaceBasenameLookup(); void (async () => { const result = await searchProjectEntries({ environmentId: activeThreadRef.environmentId, @@ -3306,6 +3308,8 @@ function ChatViewContent(props: ChatViewProps) { 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); })(); }, diff --git a/apps/web/src/composerMentionFileTarget.test.ts b/apps/web/src/composerMentionFileTarget.test.ts index be2a5bd9392f..ffa0e1fab6f0 100644 --- a/apps/web/src/composerMentionFileTarget.test.ts +++ b/apps/web/src/composerMentionFileTarget.test.ts @@ -49,6 +49,32 @@ describe("resolveComposerMentionFileTarget", () => { }); }); + 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", + }); + }); + it("returns null outside the workspace, and without one", () => { expect(resolveComposerMentionFileTarget("/etc/hosts", ROOT)).toBeNull(); expect(resolveComposerMentionFileTarget("../sibling/AGENTS.md", ROOT)).toBeNull(); diff --git a/apps/web/src/composerMentionFileTarget.ts b/apps/web/src/composerMentionFileTarget.ts index 297dcc9ee2db..9150f0c394f1 100644 --- a/apps/web/src/composerMentionFileTarget.ts +++ b/apps/web/src/composerMentionFileTarget.ts @@ -10,8 +10,13 @@ export interface ComposerMentionFileTarget { 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. - return /^\/[A-Za-z]:\//.test(normalized) ? normalized.slice(1) : normalized; + // 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 { @@ -49,9 +54,16 @@ export function resolveComposerMentionFileTarget( if (!path) return null; const absolute = collapseDotSegments(toPosixPath(resolvePathLinkTarget(path, workspaceRoot))); - const root = collapseDotSegments(toPosixPath(workspaceRoot)).replace(/\/+$/, ""); - if (!root) return null; - if (!absolute.toLowerCase().startsWith(`${root.toLowerCase()}/`)) return null; + 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; diff --git a/apps/web/src/workspaceBasenameLookup.test.ts b/apps/web/src/workspaceBasenameLookup.test.ts index 109b017669c2..660d2798e664 100644 --- a/apps/web/src/workspaceBasenameLookup.test.ts +++ b/apps/web/src/workspaceBasenameLookup.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "vite-plus/test"; import { + claimWorkspaceBasenameLookup, needsWorkspaceBasenameLookup, pickWorkspaceBasenameMatch, } from "./workspaceBasenameLookup"; @@ -39,6 +40,21 @@ describe("pickWorkspaceBasenameMatch", () => { ).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( @@ -48,3 +64,21 @@ describe("pickWorkspaceBasenameMatch", () => { ).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 index 1f752cbf94b2..788dd563bb94 100644 --- a/apps/web/src/workspaceBasenameLookup.ts +++ b/apps/web/src/workspaceBasenameLookup.ts @@ -16,6 +16,28 @@ */ 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"; @@ -45,11 +67,15 @@ export function pickWorkspaceBasenameMatch( basename: string, entries: ReadonlyArray, ): string | null { - const target = basename.trim().toLowerCase(); + const target = basename.trim(); if (!target) return null; - for (const entry of entries) { - if (entry.kind !== "file") continue; - if (basenameOfPath(entry.path).toLowerCase() === target) return entry.path; - } - 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; } From 845ddf529d55591cf80bdf0bf67fa6ff5d033b80 Mon Sep 17 00:00:00 2001 From: Rodrigo Brechard Date: Wed, 12 Aug 2026 11:15:04 +0200 Subject: [PATCH 3/3] fix(web): a mention path cannot climb above the root it resolves against MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `collapseDotSegments` kept a `..` that had nothing left to pop, so an absolute path could come back as `/../a/file.ts`. Against a normal workspace root that fails the containment check and the chip stays inert, but the workspace-rooted-at-`/` case added in the previous commit has an empty root prefix, so the path passed containment and reached the file surface as `../a/file.ts` — a relative path that walks out of the cwd. `/..` is `/`, so an absolute path now drops the climbing segment instead of keeping it. Co-Authored-By: Claude Opus 5 (1M context) --- apps/web/src/composerMentionFileTarget.test.ts | 11 +++++++++++ apps/web/src/composerMentionFileTarget.ts | 12 +++++++++--- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/apps/web/src/composerMentionFileTarget.test.ts b/apps/web/src/composerMentionFileTarget.test.ts index ffa0e1fab6f0..f1db705a5f0a 100644 --- a/apps/web/src/composerMentionFileTarget.test.ts +++ b/apps/web/src/composerMentionFileTarget.test.ts @@ -75,6 +75,17 @@ describe("resolveComposerMentionFileTarget", () => { }); }); + // `/..` 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(); diff --git a/apps/web/src/composerMentionFileTarget.ts b/apps/web/src/composerMentionFileTarget.ts index 9150f0c394f1..2e5f3ed96f22 100644 --- a/apps/web/src/composerMentionFileTarget.ts +++ b/apps/web/src/composerMentionFileTarget.ts @@ -24,9 +24,15 @@ function collapseDotSegments(path: string): string { const segments: string[] = []; for (const segment of path.split("/")) { if (segment === "" || segment === ".") continue; - if (segment === ".." && segments.length > 0 && segments[segments.length - 1] !== "..") { - segments.pop(); - 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); }