diff --git a/src/renderer/src/components/terminal-pane/terminal-link-handlers.test.ts b/src/renderer/src/components/terminal-pane/terminal-link-handlers.test.ts index 417fa613bea..5465cba262d 100644 --- a/src/renderer/src/components/terminal-pane/terminal-link-handlers.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-link-handlers.test.ts @@ -13,6 +13,7 @@ import { openDetectedFilePath } from './terminal-link-handlers' import { TERMINAL_PATH_EXISTS_CACHE_MAX_ENTRIES } from './terminal-path-exists-cache' +import { __resetWorkspaceFileIndexCacheForTest } from './terminal-workspace-file-resolution' import { handleOscLink } from './terminal-osc-link-routing' import { installHttpLinkClickFallback } from './terminal-url-link-hit-testing' import { registerHttpLinkStoreAccessor } from '@/lib/http-link-routing' @@ -31,6 +32,12 @@ const openFileMock = vi.fn() const authorizeExternalPathMock = vi.fn() const statMock = vi.fn().mockResolvedValue({ isDirectory: false }) const fsPathExistsMock = vi.fn().mockResolvedValue(true) +// Why: the workspace-fallback (issue #5024) calls fs.listFiles; default to an +// empty listing so the fallback runs intentionally rather than being silently +// swallowed by resolveWorkspaceFileByBasename's try/catch on a missing mock. +const listFilesMock = vi + .fn<(args: { rootPath: string }) => Promise>() + .mockResolvedValue([]) const runtimeEnvironmentCallMock = vi.fn() const runtimeEnvironmentTransportCallMock = vi.fn() const setActiveWorktreeMock = vi.fn() @@ -105,6 +112,8 @@ beforeEach(() => { }) vi.mocked(getConnectionId).mockReturnValue(null) openFilePathMock.mockResolvedValue(true) + listFilesMock.mockResolvedValue([]) + __resetWorkspaceFileIndexCacheForTest() storeState.settings = undefined storeState.worktreesByRepo = {} registerHttpLinkStoreAccessor(() => storeState) @@ -120,7 +129,8 @@ beforeEach(() => { fs: { authorizeExternalPath: authorizeExternalPathMock, pathExists: fsPathExistsMock, - stat: statMock + stat: statMock, + listFiles: listFilesMock }, runtimeEnvironments: { call: runtimeEnvironmentTransportCallMock } } @@ -1233,6 +1243,45 @@ describe('createFilePathLinkProvider range bounds', () => { expect(window.api.shell.pathExists).not.toHaveBeenCalled() }) + it('linkifies a bare filename that misses cwd but is unique in the worktree (issue #5024)', async () => { + setPlatform('Macintosh') + listFilesMock.mockResolvedValue([ + 'src/renderer/src/components/terminal-pane/TerminalContextMenu.test.tsx', + 'README.md' + ]) + const { provider, linkTooltip } = createProviderSetup( + [makeBufferLine('TerminalContextMenu.test.tsx')], + // The cwd-relative probe misses; the workspace fallback should resolve it. + new Map([['active\0/repo/TerminalContextMenu.test.tsx', false]]) + ) + + const links = await new Promise((resolve) => { + provider.provideLinks(1, (provided) => resolve(provided ?? [])) + }) + + expect(links.map((link) => link.text)).toEqual(['TerminalContextMenu.test.tsx']) + expect(listFilesMock).toHaveBeenCalledWith({ rootPath: '/repo' }) + links[0]!.hover?.({} as MouseEvent, links[0]!.text) + expect(linkTooltip.textContent).toBe( + '/repo/src/renderer/src/components/terminal-pane/TerminalContextMenu.test.tsx (โŒ˜+click to open or โ‡งโŒ˜+click for default app)' + ) + }) + + it('does not link a bare filename that is ambiguous in the worktree (issue #5024)', async () => { + setPlatform('Macintosh') + listFilesMock.mockResolvedValue(['src/a/index.ts', 'src/b/index.ts']) + const { provider } = createProviderSetup( + [makeBufferLine('index.ts')], + new Map([['active\0/repo/index.ts', false]]) + ) + + const links = await new Promise((resolve) => { + provider.provideLinks(1, (provided) => resolve(provided ?? [])) + }) + + expect(links).toEqual([]) + }) + it('linkifies a known worktree root printed with a trailing slash', async () => { setPlatform('Macintosh') storeState.worktreesByRepo = { diff --git a/src/renderer/src/components/terminal-pane/terminal-link-handlers.ts b/src/renderer/src/components/terminal-pane/terminal-link-handlers.ts index 6b515226810..235fbd91e00 100644 --- a/src/renderer/src/components/terminal-pane/terminal-link-handlers.ts +++ b/src/renderer/src/components/terminal-pane/terminal-link-handlers.ts @@ -1,4 +1,4 @@ -import type { IDisposable, ILink, ILinkProvider, Terminal } from '@xterm/xterm' +import type { IDisposable, ILinkProvider, Terminal } from '@xterm/xterm' import { extractTerminalFileLinkCandidates, extractTerminalFileLinks, @@ -20,8 +20,7 @@ import { import { buildHardWrappedPathLogicalLineCandidates, buildWrappedLogicalLine, - rangeForParsedFileLink, - type WrappedLogicalLine + rangeForParsedFileLink } from './wrapped-terminal-link-ranges' import { getTerminalPathExistsCacheKey, @@ -37,6 +36,9 @@ import { isMacPlatform } from './terminal-link-open-hints' import { resolveKnownWorktreeRootPathLink } from './terminal-worktree-path-link' +import { resolveWorkspaceFileByBasename } from './terminal-workspace-file-resolution' +import { preferLongestNonOverlappingLinks } from './terminal-link-overlap-selection' +import type { ProvidedFileLink } from './terminal-provided-file-link' export { openDetectedFilePath } from './terminal-file-open-routing' export { openFilePathLinkAtBufferPosition } from './terminal-file-link-hit-testing' @@ -54,38 +56,6 @@ export type LinkHandlerDeps = { getRuntimeEnvironmentIdForPane?: (paneId: number) => string | null } -type ProvidedFileLink = { - link: ILink - logicalLine: WrappedLogicalLine -} - -function rangesOverlap(left: ILink['range'], right: ILink['range']): boolean { - const leftStartsAfterRightEnds = - left.start.y > right.end.y || (left.start.y === right.end.y && left.start.x > right.end.x) - const rightStartsAfterLeftEnds = - right.start.y > left.end.y || (right.start.y === left.end.y && right.start.x > left.end.x) - return !leftStartsAfterRightEnds && !rightStartsAfterLeftEnds -} - -function preferLongestNonOverlappingLinks(links: ProvidedFileLink[]): ProvidedFileLink[] { - const selected: ProvidedFileLink[] = [] - const byLengthDescending = [...links].sort( - (a, b) => - b.link.text.length - a.link.text.length || - a.link.range.start.y - b.link.range.start.y || - a.link.range.start.x - b.link.range.start.x - ) - for (const link of byLengthDescending) { - if (!selected.some((existing) => rangesOverlap(existing.link.range, link.link.range))) { - selected.push(link) - } - } - return selected.sort( - (a, b) => - a.link.range.start.y - b.link.range.start.y || a.link.range.start.x - b.link.range.start.x - ) -} - export function createFilePathLinkProvider( paneId: number, deps: LinkHandlerDeps, @@ -155,6 +125,10 @@ export function createFilePathLinkProvider( if (/[\\/]$/.test(parsed.pathText) && !worktreeRootLink) { return null } + // Why: the cwd-relative path is the default target, but a bare + // filename may instead resolve to a file nested elsewhere in the + // worktree (see the workspace fallback below). + let linkAbsolutePath = resolved.absolutePath // Why: exact known workspace roots must stay clickable for SSH or // stale local paths even when filesystem probing says "missing". if (!worktreeRootLink) { @@ -166,7 +140,22 @@ export function createFilePathLinkProvider( : await window.api.shell.pathExists(resolved.absolutePath)) writeTerminalPathExistsCache(pathExistsCache, cacheKey, exists) if (!exists) { - return null + // Why (issue #5024): a bare filename that doesn't exist at the + // cwd often refers to a file nested elsewhere in the worktree. + // Resolve it against the worktree's own file list (local/SSH; + // runtime hosts are skipped). Only a unique match links. + const workspaceMatch = + !isRemoteRuntimePath && !/[\\/]/.test(parsed.pathText) + ? await resolveWorkspaceFileByBasename({ + basename: parsed.pathText, + worktreePath, + connectionId: fileContext.connectionId + }) + : null + if (!workspaceMatch) { + return null + } + linkAbsolutePath = workspaceMatch } } @@ -179,7 +168,7 @@ export function createFilePathLinkProvider( if (!isTerminalLinkActivation(event)) { return } - openDetectedFilePath(resolved.absolutePath, resolved.line, resolved.column, { + openDetectedFilePath(linkAbsolutePath, resolved.line, resolved.column, { worktreeId, worktreePath, runtimeEnvironmentId, @@ -191,16 +180,16 @@ export function createFilePathLinkProvider( // default escape hatch; remote paths may not exist locally. const canOpenWithSystemDefault = shouldOpenTerminalFileWithSystemDefault( fileContext, - resolved.absolutePath + linkAbsolutePath ) const hint = worktreeRootLink ? getTerminalWorktreePathOpenHint(canOpenWithSystemDefault) : canOpenWithSystemDefault - ? isHtmlFilePath(resolved.absolutePath) + ? isHtmlFilePath(linkAbsolutePath) ? getTerminalHtmlFileOpenHint() : openLinkHint : getTerminalOrcaFileOpenHint() - linkTooltip.textContent = `${resolved.absolutePath} (${hint})` + linkTooltip.textContent = `${linkAbsolutePath} (${hint})` linkTooltip.style.display = '' }, leave: () => { diff --git a/src/renderer/src/components/terminal-pane/terminal-link-overlap-selection.ts b/src/renderer/src/components/terminal-pane/terminal-link-overlap-selection.ts new file mode 100644 index 00000000000..c7a395777dc --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-link-overlap-selection.ts @@ -0,0 +1,32 @@ +import type { ILink } from '@xterm/xterm' +import type { ProvidedFileLink } from './terminal-provided-file-link' + +function rangesOverlap(left: ILink['range'], right: ILink['range']): boolean { + const leftStartsAfterRightEnds = + left.start.y > right.end.y || (left.start.y === right.end.y && left.start.x > right.end.x) + const rightStartsAfterLeftEnds = + right.start.y > left.end.y || (right.start.y === left.end.y && right.start.x > left.end.x) + return !leftStartsAfterRightEnds && !rightStartsAfterLeftEnds +} + +// Why: a wrapped path and a bare filename inside it can both match; keep the +// longest link per overlapping span so the user gets the whole path, not a +// fragment, then restore document order for stable rendering. +export function preferLongestNonOverlappingLinks(links: ProvidedFileLink[]): ProvidedFileLink[] { + const selected: ProvidedFileLink[] = [] + const byLengthDescending = [...links].sort( + (a, b) => + b.link.text.length - a.link.text.length || + a.link.range.start.y - b.link.range.start.y || + a.link.range.start.x - b.link.range.start.x + ) + for (const link of byLengthDescending) { + if (!selected.some((existing) => rangesOverlap(existing.link.range, link.link.range))) { + selected.push(link) + } + } + return selected.sort( + (a, b) => + a.link.range.start.y - b.link.range.start.y || a.link.range.start.x - b.link.range.start.x + ) +} diff --git a/src/renderer/src/components/terminal-pane/terminal-provided-file-link.ts b/src/renderer/src/components/terminal-pane/terminal-provided-file-link.ts new file mode 100644 index 00000000000..babc9eb94bb --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-provided-file-link.ts @@ -0,0 +1,10 @@ +import type { ILink } from '@xterm/xterm' +import type { WrappedLogicalLine } from './wrapped-terminal-link-ranges' + +// Why: a candidate link paired with the logical line it came from. Lives in its +// own module so the link provider and the overlap selector can share the type +// without an import cycle between handlers and overlap-selection. +export type ProvidedFileLink = { + link: ILink + logicalLine: WrappedLogicalLine +} diff --git a/src/renderer/src/components/terminal-pane/terminal-workspace-file-resolution.test.ts b/src/renderer/src/components/terminal-pane/terminal-workspace-file-resolution.test.ts new file mode 100644 index 00000000000..9214d320880 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-workspace-file-resolution.test.ts @@ -0,0 +1,106 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { + __resetWorkspaceFileIndexCacheForTest, + resolveWorkspaceFileByBasename +} from './terminal-workspace-file-resolution' + +const WT = '/Users/me/repo' + +afterEach(() => { + __resetWorkspaceFileIndexCacheForTest() +}) + +describe('resolveWorkspaceFileByBasename', () => { + it('resolves a bare filename to a unique nested workspace file (issue #5024)', async () => { + const listFiles = vi.fn(async () => [ + 'src/a/Foo.ts', + 'src/renderer/src/components/terminal-pane/TerminalContextMenu.test.tsx', + 'README.md' + ]) + const result = await resolveWorkspaceFileByBasename({ + basename: 'TerminalContextMenu.test.tsx', + worktreePath: WT, + listFiles, + now: 1000 + }) + expect(result).toBe( + '/Users/me/repo/src/renderer/src/components/terminal-pane/TerminalContextMenu.test.tsx' + ) + expect(listFiles).toHaveBeenCalledWith({ rootPath: WT }) + }) + + it('returns null when multiple files share the basename (ambiguous, never guesses)', async () => { + const listFiles = vi.fn(async () => ['a/index.ts', 'b/index.ts']) + expect( + await resolveWorkspaceFileByBasename({ + basename: 'index.ts', + worktreePath: WT, + listFiles, + now: 1000 + }) + ).toBeNull() + }) + + it('returns null when no file matches', async () => { + const listFiles = vi.fn(async () => ['a/x.ts']) + expect( + await resolveWorkspaceFileByBasename({ + basename: 'missing.ts', + worktreePath: WT, + listFiles, + now: 1000 + }) + ).toBeNull() + }) + + it('caches the file list within the TTL and refetches once it expires', async () => { + const listFiles = vi.fn(async () => ['a/Foo.ts']) + await resolveWorkspaceFileByBasename({ + basename: 'Foo.ts', + worktreePath: WT, + listFiles, + now: 1000 + }) + await resolveWorkspaceFileByBasename({ + basename: 'Foo.ts', + worktreePath: WT, + listFiles, + now: 5000 + }) + expect(listFiles).toHaveBeenCalledTimes(1) + await resolveWorkspaceFileByBasename({ + basename: 'Foo.ts', + worktreePath: WT, + listFiles, + now: 1000 + 20_000 + }) + expect(listFiles).toHaveBeenCalledTimes(2) + }) + + it('keys the cache by connection and forwards connectionId for SSH worktrees', async () => { + const remote = vi.fn(async () => ['y/remote.ts']) + const result = await resolveWorkspaceFileByBasename({ + basename: 'remote.ts', + worktreePath: WT, + connectionId: 'ssh-1', + listFiles: remote, + now: 1 + }) + expect(result).toBe('/Users/me/repo/y/remote.ts') + expect(remote).toHaveBeenCalledWith({ rootPath: WT, connectionId: 'ssh-1' }) + }) + + it('returns null without throwing when listing fails', async () => { + const listFiles = vi.fn(async () => { + throw new Error('listing unavailable') + }) + expect( + await resolveWorkspaceFileByBasename({ + basename: 'x.ts', + worktreePath: WT, + listFiles, + now: 1 + }) + ).toBeNull() + }) +}) diff --git a/src/renderer/src/components/terminal-pane/terminal-workspace-file-resolution.ts b/src/renderer/src/components/terminal-pane/terminal-workspace-file-resolution.ts new file mode 100644 index 00000000000..037300098d3 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-workspace-file-resolution.ts @@ -0,0 +1,86 @@ +import { joinAbsolutePath } from '@/lib/terminal-path-normalization' + +type WorkspaceFileIndexEntry = { fetchedAt: number; basenameToPaths: Map } + +type ListWorkspaceFiles = (args: { rootPath: string; connectionId?: string }) => Promise + +// Why: the link provider runs per hovered line, so cache the worktree's file +// list (the same listing Quick Open uses) and refresh it on a short TTL rather +// than re-listing on every lookup. +const WORKSPACE_FILE_INDEX_TTL_MS = 15_000 +const workspaceFileIndexCache = new Map() + +function basenameOf(relativePath: string): string { + const normalized = relativePath.replace(/\\/g, '/') + const slash = normalized.lastIndexOf('/') + return slash === -1 ? normalized : normalized.slice(slash + 1) +} + +function buildBasenameIndex(files: readonly string[]): Map { + const index = new Map() + for (const relativePath of files) { + const basename = basenameOf(relativePath) + const existing = index.get(basename) + if (existing) { + existing.push(relativePath) + } else { + index.set(basename, [relativePath]) + } + } + return index +} + +export type ResolveWorkspaceFileArgs = { + basename: string + worktreePath: string + connectionId?: string | null + /** Test seams. */ + now?: number + listFiles?: ListWorkspaceFiles +} + +/** + * Resolve a bare filename to a file nested elsewhere in the worktree. + * + * Why (issue #5024): agent output frequently references a repo file by bare + * name (e.g. `TerminalContextMenu.test.tsx`), which does not exist at the + * terminal's cwd root, so the link provider's cwd-relative existence check + * misses it. Falling back to the worktree's own file list makes those mentions + * clickable. Only a UNIQUE basename match is returned โ€” multiple files sharing + * a name are ambiguous and left unlinked rather than guessed. + */ +export async function resolveWorkspaceFileByBasename( + args: ResolveWorkspaceFileArgs +): Promise { + const { basename, worktreePath } = args + if (!basename || !worktreePath) { + return null + } + const connectionId = args.connectionId ?? undefined + const now = args.now ?? Date.now() + const cacheKey = `${connectionId ?? 'local'}::${worktreePath}` + + let entry = workspaceFileIndexCache.get(cacheKey) + if (!entry || now - entry.fetchedAt > WORKSPACE_FILE_INDEX_TTL_MS) { + const list = args.listFiles ?? ((listArgs) => window.api.fs.listFiles(listArgs)) + let files: string[] + try { + files = await list({ rootPath: worktreePath, ...(connectionId ? { connectionId } : {}) }) + } catch { + // Best-effort: a failed listing must not break link detection. + return null + } + entry = { fetchedAt: now, basenameToPaths: buildBasenameIndex(files) } + workspaceFileIndexCache.set(cacheKey, entry) + } + + const matches = entry.basenameToPaths.get(basename) + if (!matches || matches.length !== 1) { + return null + } + return joinAbsolutePath(worktreePath, matches[0]) +} + +export function __resetWorkspaceFileIndexCacheForTest(): void { + workspaceFileIndexCache.clear() +}