From fce37d3510d5acefab1ac08f217bf964bcee7aef Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Fri, 23 Oct 2026 13:01:37 -0700 Subject: [PATCH] fix(terminal): Cmd/Ctrl+click opens the clicked path on a paired server when more paths follow (#32411) * fix(terminal): open the clicked path on a paired server when more paths follow Cmd/Ctrl+click on a terminal file path opened the hard-wrap guess that joins it to the rows below (`a.txt/b.txt/...`) whenever that guess had no cached existence answer. Paired hosts rejected the guess's probe with ENOTDIR instead of answering "missing", so it was never cached, the open failed silently, and the fallback had already swallowed xterm's own activation. - The click fallback now probes ambiguous candidates in one batch and opens the longest one that exists; a later click supersedes a slow probe. - A click that opens nothing now says why (not found vs. could not check). - Host existence probes treat ENOTDIR as missing. Co-authored-by: fernando_jacob * fix(terminal): let the open report failures and reuse one failure toast Co-authored-by: fernando_jacob * fix(terminal): check longer wrapped names first and yield to any later file open Co-authored-by: fernando_jacob * fix(terminal): supersede earlier opens at click time and stop re-asking unanswerable guesses Co-authored-by: fernando_jacob * fix(terminal): read an older host's ENOTDIR as missing instead of memoizing it Co-authored-by: fernando_jacob * fix(terminal): open a wrapped share path whole instead of probing it unprompted Co-authored-by: fernando_jacob * fix(terminal): one selection rule for clicked paths and fence probes to the clicked pairing Co-authored-by: fernando_jacob * fix(terminal): fence click probes to the clicked host and match ENOTDIR by its errno text Co-authored-by: fernando_jacob * fix(terminal): a click on a path already known missing still says so Co-authored-by: fernando_jacob * fix(terminal): leave OSC 8 hyperlinks to xterm when their label is a missing path Co-authored-by: fernando_jacob --------- Co-authored-by: fernando_jacob --- ...tem-path-containment-remote-enoent.test.ts | 17 +- src/main/ipc/filesystem-path-containment.ts | 6 + .../filesystem/filesystem-read-handlers.ts | 6 +- src/main/runtime/files/file-path-existence.ts | 4 +- .../native-chat-link-target-actions.ts | 10 +- .../terminal-file-link-hit-testing.ts | 179 +++++-- .../terminal-file-open-routing.ts | 7 + .../terminal-pane/terminal-link-handlers.ts | 8 +- ...erminal-link-paired-click-fallback.test.ts | 451 ++++++++++++++++++ .../src/lib/file-link-open-failure-toast.ts | 14 + .../runtime/runtime-file-metadata-client.ts | 7 +- ...untime-path-existence-error-parity.test.ts | 11 +- 12 files changed, 664 insertions(+), 56 deletions(-) create mode 100644 src/renderer/src/components/terminal-pane/terminal-link-paired-click-fallback.test.ts create mode 100644 src/renderer/src/lib/file-link-open-failure-toast.ts diff --git a/src/main/ipc/filesystem-path-containment-remote-enoent.test.ts b/src/main/ipc/filesystem-path-containment-remote-enoent.test.ts index 25f79ba7f16..aef3b693cda 100644 --- a/src/main/ipc/filesystem-path-containment-remote-enoent.test.ts +++ b/src/main/ipc/filesystem-path-containment-remote-enoent.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest' -import { isENOENT } from './filesystem-path-containment' +import { isENOENT, isPathAbsentError } from './filesystem-path-containment' /** * Creating a worktree over SSH failed with a raw @@ -67,3 +67,18 @@ describe('isENOENT across the SSH relay boundary', () => { expect(isENOENT(undefined)).toBe(false) }) }) + +describe('isPathAbsentError', () => { + it('treats a path under a file as absent, but not a denied one', () => { + // Why: `a.txt/b.txt` is the hard-wrap guess a terminal link click probes (#32128). + const underFile = Object.assign(new Error("ENOTDIR: not a directory, stat '/r/a.txt/b.txt'"), { + code: 'ENOTDIR' + }) + const denied = Object.assign(new Error("EACCES: permission denied, stat '/root/x'"), { + code: 'EACCES' + }) + + expect(isPathAbsentError(underFile)).toBe(true) + expect(isPathAbsentError(denied)).toBe(false) + }) +}) diff --git a/src/main/ipc/filesystem-path-containment.ts b/src/main/ipc/filesystem-path-containment.ts index c696028cbc8..9745e3fb81f 100644 --- a/src/main/ipc/filesystem-path-containment.ts +++ b/src/main/ipc/filesystem-path-containment.ts @@ -1,6 +1,7 @@ import { resolve, relative, isAbsolute, sep, dirname } from 'node:path' import { statSync } from 'node:fs' import { realpath } from 'node:fs/promises' +import { isDefinitiveAbsence } from '../../shared/definitive-filesystem-absence' /** * Check whether resolvedTarget is equal to or a descendant of resolvedBase. @@ -123,6 +124,11 @@ export function isENOENT(error: unknown): boolean { return ENOENT_MESSAGE.test(error.message) } +/** An existence probe's "not there": also ENOTDIR, since `file.txt/child` cannot exist either. */ +export function isPathAbsentError(error: unknown): boolean { + return isENOENT(error) || isDefinitiveAbsence(error) +} + export async function normalizeExistingPath(resolvedPath: string): Promise { try { return resolve(await realpath(resolvedPath)) diff --git a/src/main/ipc/filesystem/filesystem-read-handlers.ts b/src/main/ipc/filesystem/filesystem-read-handlers.ts index cc34f6f6f16..8ed2a0b65f2 100644 --- a/src/main/ipc/filesystem/filesystem-read-handlers.ts +++ b/src/main/ipc/filesystem/filesystem-read-handlers.ts @@ -16,7 +16,7 @@ import { resolveDesktopAuthorizedPath, resolveLocalFileRequestPath } from '../local-file-access-resolution' -import { isENOENT } from '../filesystem-path-containment' +import { isPathAbsentError } from '../filesystem-path-containment' import { listMarkdownDocuments } from '../markdown-documents' import { getLocalGitOptionsForRegisteredWorktree } from '../local-worktree-runtime-options' import { recordCrashBreadcrumb } from '../../crash-reporting/crash-breadcrumb-store' @@ -157,7 +157,7 @@ export function registerFilesystemReadHandlers(context: FilesystemHandlerContext await stat(await resolveDesktopAuthorizedPath(filePath, store)) return true } catch (error) { - if (isENOENT(error)) { + if (isPathAbsentError(error)) { return false } throw error @@ -182,7 +182,7 @@ export function registerFilesystemReadHandlers(context: FilesystemHandlerContext await stat(filePath) return true } catch (error) { - if (isENOENT(error)) { + if (isPathAbsentError(error)) { return false } throw error diff --git a/src/main/runtime/files/file-path-existence.ts b/src/main/runtime/files/file-path-existence.ts index fc3ad185698..41042053e44 100644 --- a/src/main/runtime/files/file-path-existence.ts +++ b/src/main/runtime/files/file-path-existence.ts @@ -2,7 +2,7 @@ import { capturePathExistence, type PathExistenceResult } from '../../../shared/path-existence-batch' -import { isENOENT } from '../../ipc/filesystem-path-containment' +import { isPathAbsentError } from '../../ipc/filesystem-path-containment' import { requireRuntimeFileProvider, type RuntimeFileExplorerPath, @@ -27,7 +27,7 @@ export async function readRuntimeFilePathExistence( await provider.stat(target.path) return true } catch (error) { - if (isENOENT(error)) { + if (isPathAbsentError(error)) { return false } throw error diff --git a/src/renderer/src/components/native-chat/native-chat-link-target-actions.ts b/src/renderer/src/components/native-chat/native-chat-link-target-actions.ts index 9212ec09da6..366fe7fc87b 100644 --- a/src/renderer/src/components/native-chat/native-chat-link-target-actions.ts +++ b/src/renderer/src/components/native-chat/native-chat-link-target-actions.ts @@ -22,10 +22,7 @@ import { resolveNativeChatHttpLinkSourceOwner } from './native-chat-http-link-source-owner' import { openNativeChatWebLink } from './native-chat-web-link-actions' -import { - showFileLinkNotFoundToast, - showFileLinkUnverifiableToast -} from './native-chat-file-link-toasts' +import { showFileLinkOpenFailureToast } from '@/lib/file-link-open-failure-toast' /** Who owns a chat http(s) link and where it may open: the rows its popover and menu offer. */ export function resolveNativeChatHttpLinkRouting( @@ -53,10 +50,7 @@ export function nativeChatFileOpenDeps( worktreePath: context.worktreePath, runtimeEnvironmentId: context.runtimeEnvironmentId, sourceTabId: context.sourceTabId, - onOpenFailure: (failure) => - failure.verdict === 'unverifiable' - ? showFileLinkUnverifiableToast(target.absolutePath, failure.error) - : showFileLinkNotFoundToast(target.absolutePath) + onOpenFailure: (failure) => showFileLinkOpenFailureToast(target.absolutePath, failure) } } diff --git a/src/renderer/src/components/terminal-pane/terminal-file-link-hit-testing.ts b/src/renderer/src/components/terminal-pane/terminal-file-link-hit-testing.ts index f8499a67acd..e230668e353 100644 --- a/src/renderer/src/components/terminal-pane/terminal-file-link-hit-testing.ts +++ b/src/renderer/src/components/terminal-pane/terminal-file-link-hit-testing.ts @@ -1,8 +1,24 @@ import { dedupeLogicalLines } from './terminal-link-logical-line-deduplication' import type { IBufferLine, IBufferRange } from '@xterm/xterm' import { extractTerminalFileLinkCandidates } from '@/lib/terminal-links' -import { openDetectedFilePath } from './terminal-file-open-routing' -import { resolveFileLinkTarget } from './terminal-file-link-target' +import { + getTerminalFileContext, + reserveDetectedFilePathOpenTurn, + openDetectedFilePath, + type FileOpenFailure, + type TerminalFileContext +} from './terminal-file-open-routing' +import { + fileLinkTargetExists, + mayCheckFileLinkTargetUnprompted, + resolveFileLinkTarget, + type FileLinkHost, + type FileLinkTarget +} from './terminal-file-link-target' +import { createTerminalPathExistenceBatch } from './terminal-path-existence-batch' +import { terminalLogicalLinkHasOscLink } from './terminal-osc-link-cells' +import { runtimeTargetEnvironmentId } from '@/runtime/runtime-client-target' +import { getRuntimeEnvironmentRevision } from '@/runtime/runtime-environment-revision' import { buildHardWrappedPathLogicalLineCandidates, buildWrappedLogicalLine, @@ -21,28 +37,28 @@ type FileLinkHitTestDeps = { pathExistsCache?: Map openWithSystemDefault?: boolean sourceTabId?: string | null + /** Tells the user why a click opened nothing; skipped once a later click supersedes it. */ + onOpenFailure?: (absolutePath: string, failure: FileOpenFailure) => void } +type ClickedFileLink = FileLinkTarget & { + pathText: string + cachedExists: boolean | undefined + hasOscLink: boolean +} + +// Why: a slow probe from an earlier click must not open its file after a later click. +let latestClickGesture = 0 + export function openFilePathLinkAtBufferPosition( buffer: { getLine(y: number): IBufferLine | undefined }, position: { x: number; y: number }, terminalColumns: number, deps: FileLinkHitTestDeps ): boolean { - const logicalLines = buildCandidateLogicalLinesForBufferPosition(buffer, position.y) - if (logicalLines.length === 0) { - return false - } - - for (const logicalLine of logicalLines) { - const matches: { - absolutePath: string - line: number | null - column: number | null - pathText: string - cachedExists: boolean | undefined - isKnownWorktreeRoot: boolean - }[] = [] + const gesture = ++latestClickGesture + const links: ClickedFileLink[] = [] + for (const logicalLine of buildCandidateLogicalLinesForBufferPosition(buffer, position.y)) { for (const parsed of extractTerminalFileLinkCandidates(logicalLine.text)) { const target = resolveFileLinkTarget(parsed, { cwd: deps.startupCwd, @@ -59,34 +75,121 @@ export function openFilePathLinkAtBufferPosition( if (!range || !rangeContainsBufferPosition(range, position, terminalColumns)) { continue } - matches.push({ - absolutePath: target.absolutePath, - line: target.line, - column: target.column, + links.push({ + ...target, pathText: parsed.pathText, cachedExists: deps.pathExistsCache?.get(target.cacheKey), - isKnownWorktreeRoot: target.isKnownWorktreeRoot + hasOscLink: terminalLogicalLinkHasOscLink( + buffer, + logicalLine, + parsed.startIndex, + parsed.endIndex + ) }) } - - const cachedMatch = matches - .filter((match) => match.cachedExists) - .sort((a, b) => b.pathText.length - a.pathText.length)[0] - const knownWorktreeRootMatch = matches - .filter((match) => match.isKnownWorktreeRoot) - .sort((a, b) => b.pathText.length - a.pathText.length)[0] - const uncachedMatch = matches.find((match) => match.cachedExists !== false) - const match = cachedMatch ?? knownWorktreeRootMatch ?? uncachedMatch - if (match) { - openDetectedFilePath(match.absolutePath, match.line, match.column, { - ...deps, - openWithSystemDefault: deps.openWithSystemDefault === true - }) - return true - } } - return false + // Why: a failure names the path the user sees, not a longer joined guess. + const shown = shortestPath(links) + const open = (link: ClickedFileLink, verified = link.cachedExists === true): void => { + const { onOpenFailure } = deps + // Why: a bare dotted word (`console.log`, `example.com`) is not a claim that a file exists. + const reportsFailure = + onOpenFailure && (verified || link.isKnownWorktreeRoot || /[\\/]/.test(link.pathText)) + const reportedPath = + verified || link.isKnownWorktreeRoot || !shown ? link.absolutePath : shown.absolutePath + openDetectedFilePath(link.absolutePath, link.line, link.column, { + ...deps, + openWithSystemDefault: deps.openWithSystemDefault === true, + onOpenFailure: reportsFailure ? (failure) => onOpenFailure(reportedPath, failure) : undefined + }) + } + const host: FileLinkHost = { ...deps, cwd: deps.startupCwd, homePath: deps.terminalHomePath } + // Why: a share outside the workspace is never stat'ed unprompted; like a verified path, its + // guess opens as is and the host resolves it. + const settled = (link: ClickedFileLink): boolean => + link.cachedExists === true || + link.isKnownWorktreeRoot || + (link.cachedExists === undefined && !mayCheckFileLinkTargetUnprompted(link, host)) + const unique = links.filter( + (link, index) => links.findIndex((other) => other.cacheKey === link.cacheKey) === index + ) + const best = longestPath(unique.filter(settled)) + // Why longer only: a cached `foo.ts` must not win over an unchecked hard-wrapped `foo.ts.map`. + const toProbe = unique.filter( + (link) => + link.cachedExists === undefined && + !settled(link) && + (!best || link.pathText.length > best.pathText.length) + ) + if (toProbe.length === 0 || (!best && toProbe.length === 1)) { + // Why the visible path last: a click on a path already known missing still says so. + // An OSC 8 hyperlink there is xterm's to open. + const knownMissing = + shown && !shown.hasOscLink && /[\\/]/.test(shown.pathText) ? shown : undefined + const link = best ?? toProbe[0] ?? knownMissing + if (link) { + open(link) + } + return link !== undefined + } + + // Why: hard-wrap joining offers guesses like `a.txt/b.txt` that sort first; opening one + // before its hover probe answers (a host round-trip) fails and hides the real path. + const isLatestOpen = reserveDetectedFilePathOpenTurn() + const clickedHost = fileHostFingerprint(toProbe[0].fileContext) + void probeClickedFileLinks(toProbe, deps).then((existing) => { + // Why: if the workspace's host or pairing changed meanwhile, a same-named file there is not + // the one clicked. + const currentHost = fileHostFingerprint( + getTerminalFileContext(deps.worktreeId, deps.worktreePath, deps.runtimeEnvironmentId) + ) + if (gesture !== latestClickGesture || !isLatestOpen() || currentHost !== clickedHost) { + return + } + const found = longestPath(existing) + // Why the shortest otherwise: opening it lets the open report the real failure instead of + // a probe guessing "missing" for a refusal. + const link = found ?? best ?? shortestPath(toProbe) + if (link) { + open(link, link === found || link.cachedExists === true) + } + }) + return true +} + +function fileHostFingerprint(context: TerminalFileContext): string { + const environmentId = runtimeTargetEnvironmentId(context.target) + return JSON.stringify([context, environmentId && getRuntimeEnvironmentRevision(environmentId)]) +} + +async function probeClickedFileLinks( + links: ClickedFileLink[], + deps: FileLinkHitTestDeps +): Promise { + const pathExists = createTerminalPathExistenceBatch() + const cache = deps.pathExistsCache ?? new Map() + const exists = await Promise.all( + links.map((link) => + // Why: a guess the host cannot answer must not hide a sibling. + fileLinkTargetExists(link, cache, pathExists, deps.terminalHandle).catch(() => false) + ) + ) + return links.filter((_, index) => exists[index]) +} + +function shortestPath(links: ClickedFileLink[]): ClickedFileLink | undefined { + return links.reduce( + (best, link) => (!best || link.pathText.length < best.pathText.length ? link : best), + undefined + ) +} + +function longestPath(links: ClickedFileLink[]): ClickedFileLink | undefined { + return links.reduce( + (best, link) => (!best || link.pathText.length > best.pathText.length ? link : best), + undefined + ) } export function buildCandidateLogicalLinesForBufferPosition( diff --git a/src/renderer/src/components/terminal-pane/terminal-file-open-routing.ts b/src/renderer/src/components/terminal-pane/terminal-file-open-routing.ts index 3ac5e36d11f..df301c75139 100644 --- a/src/renderer/src/components/terminal-pane/terminal-file-open-routing.ts +++ b/src/renderer/src/components/terminal-pane/terminal-file-open-routing.ts @@ -91,6 +91,13 @@ export function shouldOpenTerminalFileWithSystemDefault(fileContext: TerminalFil let latestOpenDetectedFilePathRequestId = 0 +/** Supersedes earlier opens now, for a click still deciding what to open; true until a later open. */ +export function reserveDetectedFilePathOpenTurn(): () => boolean { + const requestId = ++latestOpenDetectedFilePathRequestId + cancelPendingEditorLineRevealFrames() + return () => requestId === latestOpenDetectedFilePathRequestId +} + export function openDetectedFilePath( filePath: string, line: number | null, 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 1dad5737515..b7c2b0c2b7e 100644 --- a/src/renderer/src/components/terminal-pane/terminal-link-handlers.ts +++ b/src/renderer/src/components/terminal-pane/terminal-link-handlers.ts @@ -30,6 +30,7 @@ import { isTerminalLinkDirectActivation } from './terminal-link-activation' import { getTerminalBufferPositionForMouseEvent } from './terminal-mouse-buffer-position' import type { TerminalLinkActionContext } from './terminal-link-action-request' import { handleTerminalFileLink } from './terminal-file-link-actions' +import { showFileLinkOpenFailureToast } from '@/lib/file-link-open-failure-toast' export { openDetectedFilePath } from './terminal-file-open-routing' export { mapTerminalFilePath } from './terminal-file-open-routing' @@ -157,7 +158,9 @@ export function createFilePathLinkProvider( runtimeEnvironmentId, wslDistro: deps.wslDistro, terminalHandle, - sourceTabId: deps.sourceTabId + sourceTabId: deps.sourceTabId, + onOpenFailure: (failure) => + showFileLinkOpenFailureToast(mappedPath, failure) }, deps.getLinkActionContext?.(paneId) ) @@ -253,7 +256,8 @@ export function installFilePathLinkClickFallback( terminalHandle: deps.getRuntimeTerminalHandleForPane?.(paneId) ?? null, pathExistsCache: deps.pathExistsCache, openWithSystemDefault: Boolean(event.shiftKey), - sourceTabId: deps.sourceTabId + sourceTabId: deps.sourceTabId, + onOpenFailure: showFileLinkOpenFailureToast } ) if (opened) { diff --git a/src/renderer/src/components/terminal-pane/terminal-link-paired-click-fallback.test.ts b/src/renderer/src/components/terminal-pane/terminal-link-paired-click-fallback.test.ts new file mode 100644 index 00000000000..ed22323a3f0 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-link-paired-click-fallback.test.ts @@ -0,0 +1,451 @@ +import type { PathExistenceResult } from '../../../../shared/path-existence-batch' +import { describe, expect, it, vi } from 'vitest' +import { getConnectionId } from '@/lib/connection-context' +import { openFilePathLinkAtBufferPosition } from './terminal-file-link-hit-testing' +import { openDetectedFilePath } from './terminal-file-open-routing' +import { createTerminalLinkTestDoubles } from './terminal-link-handlers-test-fixtures' +import { getTerminalPathExistsCacheKey } from './terminal-path-exists-cache' +import { replaceRuntimeEnvironmentRevisions } from '@/runtime/runtime-environment-revision' +import { makeBuffer, makeBufferLine } from './terminal-link-provider-buffer-fixtures' +import { + createDeferred, + flushAsyncWork, + flushDoubleRaf, + installTerminalLinkTestEnvironment, + setPlatform +} from './terminal-link-handlers-test-harness' + +const doubles = createTerminalLinkTestDoubles() +const { storeState, openFileMock, runtimeEnvironmentCallMock, statMock } = doubles + +vi.mock('@/store', () => ({ + useAppStore: { + getState: () => storeState + } +})) + +vi.mock('@/lib/worktree-activation', () => ({ + activateAndRevealWorkspace: vi.fn(), + activateAndRevealWorktree: vi.fn() +})) + +vi.mock('@/lib/connection-context', () => ({ + getConnectionId: vi.fn(() => null) +})) + +installTerminalLinkTestEnvironment(doubles) + +const BUFFER = makeBuffer([ + makeBufferLine('/srv/repo/a.txt'), + makeBufferLine('/srv/repo/b.txt'), + makeBufferLine('/srv/repo/c.txt') +]) + +// What a host before the ENOTDIR fix returns for a path under a file. +const OLD_HOST_ENOTDIR = "ENOTDIR: not a directory, stat '/srv/repo/a.txt/srv/repo/b.txt'" + +function runtimeKey(absolutePath: string): string { + return getTerminalPathExistsCacheKey({ + absolutePath, + isRemoteRuntimePath: true, + runtimeEnvironmentId: 'env-1' + }) +} + +type HostAnswer = { exists: boolean } | { error: string } + +/** The host answers both the batched probe and the open's stat from one table. */ +function hostAnswers(answer: (relativePath: string) => HostAnswer): void { + runtimeEnvironmentCallMock.mockImplementation(async (args) => { + const meta = { runtimeId: 'remote-runtime' } + if (args.method === 'files.pathsExist') { + return { id: 'rpc-1', ok: true, result: args.params.relativePaths.map(answer), _meta: meta } + } + const row = answer(args.params.relativePath) + if ('exists' in row && row.exists) { + return { + id: 'rpc-1', + ok: true, + result: { size: 1, isDirectory: false, mtime: 1 }, + _meta: meta + } + } + const message = 'error' in row ? row.error : 'ENOENT: no such file or directory' + return { id: 'rpc-1', ok: false, error: { code: 'runtime_error', message }, _meta: meta } + }) +} + +function clickPairedPath( + buffer: ReturnType, + pathExistsCache = new Map() +): boolean { + return openFilePathLinkAtBufferPosition(buffer, { x: 4, y: 1 }, 80, { + startupCwd: '/srv/repo', + worktreeId: 'wt-1', + worktreePath: '/srv/repo', + runtimeEnvironmentId: 'env-1', + terminalHandle: 'term-1', + pathExistsCache + }) +} + +function clickFirstPath( + onOpenFailure = vi.fn(), + pathExistsCache = new Map() +): boolean { + return openFilePathLinkAtBufferPosition(BUFFER, { x: 4, y: 1 }, 80, { + startupCwd: '/srv/repo', + worktreeId: 'wt-1', + worktreePath: '/srv/repo', + runtimeEnvironmentId: 'env-1', + terminalHandle: 'term-1', + pathExistsCache, + onOpenFailure + }) +} + +describe('a modifier-click on a paired-server path followed by more paths', () => { + it('opens the clicked path, not the guess joining it to the rows below', async () => { + setPlatform('Macintosh') + // Why an error row: hosts before the ENOTDIR fix cannot answer for `a.txt/...`. + hostAnswers((path) => (path === 'a.txt' ? { exists: true } : { error: OLD_HOST_ENOTDIR })) + const onOpenFailure = vi.fn() + + expect(clickFirstPath(onOpenFailure)).toBe(true) + await flushDoubleRaf() + + expect(openFileMock).toHaveBeenCalledTimes(1) + expect(openFileMock).toHaveBeenCalledWith( + expect.objectContaining({ filePath: '/srv/repo/a.txt' }), + { forceContentReload: true } + ) + expect(onOpenFailure).not.toHaveBeenCalled() + }) + + it('remembers an older host ENOTDIR answer as missing, so the next click does not re-ask', async () => { + setPlatform('Macintosh') + hostAnswers((path) => (path === 'a.txt' ? { exists: true } : { error: OLD_HOST_ENOTDIR })) + // Why: hovering underlined a.txt, so only the longer joined guesses are unchecked. + const pathExistsCache = new Map([[runtimeKey('/srv/repo/a.txt'), true]]) + + clickFirstPath(vi.fn(), pathExistsCache) + await flushDoubleRaf() + runtimeEnvironmentCallMock.mockClear() + clickFirstPath(vi.fn(), pathExistsCache) + await flushDoubleRaf() + + expect(runtimeEnvironmentCallMock).not.toHaveBeenCalledWith( + expect.objectContaining({ method: 'files.pathsExist' }) + ) + expect(openFileMock).toHaveBeenCalledTimes(2) + expect(openFileMock).toHaveBeenLastCalledWith( + expect.objectContaining({ filePath: '/srv/repo/a.txt' }), + { forceContentReload: true } + ) + }) + + it('names the visible path when only a joined guess is left to try', async () => { + setPlatform('Macintosh') + hostAnswers(() => ({ error: 'host unreachable' })) + const onOpenFailure = vi.fn() + const pathExistsCache = new Map([ + [runtimeKey('/srv/repo/a.txt'), false], + [runtimeKey('/srv/repo/a.txt/srv/repo/b.txt/srv/repo/c.txt'), false] + ]) + + clickFirstPath(onOpenFailure, pathExistsCache) + await flushDoubleRaf() + + expect(onOpenFailure).toHaveBeenCalledWith('/srv/repo/a.txt', expect.anything()) + }) + + it('says why when the clicked path is gone, naming it rather than a joined guess', async () => { + setPlatform('Macintosh') + hostAnswers(() => ({ exists: false })) + const onOpenFailure = vi.fn() + + clickFirstPath(onOpenFailure) + await flushDoubleRaf() + + expect(openFileMock).not.toHaveBeenCalled() + expect(onOpenFailure).toHaveBeenCalledWith( + '/srv/repo/a.txt', + expect.objectContaining({ verdict: 'missing' }) + ) + }) + + it('does not call the path missing when the host cannot answer', async () => { + setPlatform('Macintosh') + hostAnswers(() => ({ error: 'host unreachable' })) + const onOpenFailure = vi.fn() + + clickFirstPath(onOpenFailure) + await flushDoubleRaf() + + expect(openFileMock).not.toHaveBeenCalled() + expect(onOpenFailure).toHaveBeenCalledWith( + '/srv/repo/a.txt', + expect.objectContaining({ verdict: 'unverifiable' }) + ) + }) + + it('drops a slow ambiguous probe once a later click has opened something else', async () => { + setPlatform('Macintosh') + const firstProbe = createDeferred() + vi.mocked(window.api.shell.pathExists).mockImplementation(async (path: string) => + path === '/repo/a.txt' ? firstProbe.promise : false + ) + const buffer = makeBuffer([ + makeBufferLine('/repo/a.txt'), + makeBufferLine('/repo/b.txt'), + makeBufferLine(''), + makeBufferLine('package.json') + ]) + const deps = { startupCwd: '/repo', worktreeId: 'wt-1', worktreePath: '/repo' } + + openFilePathLinkAtBufferPosition(buffer, { x: 4, y: 1 }, 80, deps) + // Why: a lone candidate opens without probing, and must still cancel the pending probe. + openFilePathLinkAtBufferPosition(buffer, { x: 4, y: 4 }, 80, deps) + firstProbe.resolve(true) + await flushAsyncWork() + + expect(statMock).toHaveBeenCalledTimes(1) + expect(statMock).toHaveBeenCalledWith({ filePath: '/repo/package.json' }) + }) + + it('reports a missing path but stays quiet for a bare dotted word', async () => { + setPlatform('Macintosh') + // Why Once: a lasting rejection would leak into later tests. + statMock + .mockRejectedValueOnce(new Error('ENOENT: no such file')) + .mockRejectedValueOnce(new Error('ENOENT: no such file')) + const onOpenFailure = vi.fn() + const deps = { startupCwd: '/repo', worktreeId: 'wt-1', worktreePath: '/repo', onOpenFailure } + + openFilePathLinkAtBufferPosition( + makeBuffer([makeBufferLine('console.log')]), + { x: 4, y: 1 }, + 80, + deps + ) + await flushAsyncWork() + expect(onOpenFailure).not.toHaveBeenCalled() + + openFilePathLinkAtBufferPosition( + makeBuffer([makeBufferLine('src/gone.ts')]), + { x: 4, y: 1 }, + 80, + deps + ) + await flushAsyncWork() + expect(onOpenFailure).toHaveBeenCalledWith( + '/repo/src/gone.ts', + expect.objectContaining({ verdict: 'missing' }) + ) + }) + + it('checks a longer hard-wrapped name before opening its cached prefix', async () => { + setPlatform('Macintosh') + vi.mocked(window.api.shell.pathExists).mockImplementation( + async (path: string) => path === '/repo/foo.ts.map' + ) + const buffer = makeBuffer([makeBufferLine('/repo/foo.ts'), makeBufferLine('.map')]) + const pathExistsCache = new Map([ + [getTerminalPathExistsCacheKey({ absolutePath: '/repo/foo.ts' }), true] + ]) + const deps = { startupCwd: '/repo', worktreeId: 'wt-1', worktreePath: '/repo', pathExistsCache } + + openFilePathLinkAtBufferPosition(buffer, { x: 4, y: 1 }, 80, deps) + await flushAsyncWork() + + expect(statMock).toHaveBeenCalledTimes(1) + expect(statMock).toHaveBeenCalledWith({ filePath: '/repo/foo.ts.map' }) + }) + + it('drops a slow ambiguous probe once any other file open has started', async () => { + setPlatform('Macintosh') + const probe = createDeferred() + vi.mocked(window.api.shell.pathExists).mockImplementation(() => probe.promise) + const deps = { startupCwd: '/repo', worktreeId: 'wt-1', worktreePath: '/repo' } + + openFilePathLinkAtBufferPosition( + makeBuffer([makeBufferLine('/repo/a.txt'), makeBufferLine('/repo/b.txt')]), + { x: 4, y: 1 }, + 80, + deps + ) + // Why: e.g. a chat link or the link popover, which never passes through the click fallback. + openDetectedFilePath('/repo/other.txt', null, null, deps) + probe.resolve(true) + await flushAsyncWork() + + expect(statMock).toHaveBeenCalledTimes(1) + expect(statMock).toHaveBeenCalledWith({ filePath: '/repo/other.txt' }) + }) + + it('supersedes an earlier open still in flight as soon as an ambiguous click starts', async () => { + setPlatform('Macintosh') + const earlierStat = createDeferred<{ isDirectory: boolean }>() + statMock.mockImplementationOnce(() => earlierStat.promise) + const probe = createDeferred() + vi.mocked(window.api.shell.pathExists).mockImplementation(async (path: string) => + path === '/repo/a.txt' ? probe.promise : false + ) + const deps = { startupCwd: '/repo', worktreeId: 'wt-1', worktreePath: '/repo' } + + openDetectedFilePath('/repo/earlier.txt', null, null, deps) + openFilePathLinkAtBufferPosition( + makeBuffer([makeBufferLine('/repo/a.txt'), makeBufferLine('/repo/b.txt')]), + { x: 4, y: 1 }, + 80, + deps + ) + earlierStat.resolve({ isDirectory: false }) + await flushAsyncWork() + expect(openFileMock).not.toHaveBeenCalled() + + probe.resolve(true) + await flushDoubleRaf() + expect(openFileMock).toHaveBeenCalledTimes(1) + expect(openFileMock).toHaveBeenCalledWith( + expect.objectContaining({ filePath: '/repo/a.txt' }), + { forceContentReload: true } + ) + }) + + it('opens a wrapped network-share path whole, without stat-ing guesses unprompted', async () => { + setPlatform('Windows') + runtimeEnvironmentCallMock.mockResolvedValue({ + id: 'rpc-1', + ok: true, + result: { worktree: 'wt-1', relativePath: null, absolutePath: null, exists: false }, + _meta: { runtimeId: 'remote-runtime' } + }) + const buffer = makeBuffer([ + makeBufferLine('//server/share/other/rep'), + makeBufferLine('ort.txt') + ]) + + openFilePathLinkAtBufferPosition(buffer, { x: 4, y: 1 }, 80, { + startupCwd: '/srv/repo', + worktreeId: 'wt-1', + worktreePath: '/srv/repo', + runtimeEnvironmentId: 'env-1', + terminalHandle: 'term-1', + pathExistsCache: new Map() + }) + await flushDoubleRaf() + + const asked = runtimeEnvironmentCallMock.mock.calls.map(([args]) => args.params?.pathText) + expect(asked).toEqual(['//server/share/other/report.txt']) + }) + + it('opens a wrapped share path whole even when its prefix is a known file', async () => { + setPlatform('Windows') + runtimeEnvironmentCallMock.mockResolvedValue({ + id: 'rpc-1', + ok: true, + result: { worktree: 'wt-1', relativePath: null, absolutePath: null, exists: false }, + _meta: { runtimeId: 'remote-runtime' } + }) + const buffer = makeBuffer([makeBufferLine('//server/share/report'), makeBufferLine('.txt')]) + const pathExistsCache = new Map([[runtimeKey('//server/share/report'), true]]) + + clickPairedPath(buffer, pathExistsCache) + await flushDoubleRaf() + + const asked = runtimeEnvironmentCallMock.mock.calls.map(([args]) => args.params?.pathText) + expect(asked).toEqual(['//server/share/report.txt']) + }) + + it('drops the click when its server is re-paired while candidates are checked', async () => { + setPlatform('Macintosh') + replaceRuntimeEnvironmentRevisions([{ id: 'env-1', createdAt: 1, pairingRevision: 1 }]) + const probe = createDeferred() + runtimeEnvironmentCallMock.mockImplementation((args) => + args.method === 'files.pathsExist' ? probe.promise : Promise.reject(new Error('unexpected')) + ) + + clickFirstPath() + replaceRuntimeEnvironmentRevisions([{ id: 'env-1', createdAt: 1, pairingRevision: 2 }]) + probe.resolve({ + id: 'rpc-1', + ok: true, + result: [{ exists: false }, { exists: false }, { exists: true }], + _meta: { runtimeId: 'remote-runtime' } + }) + await flushDoubleRaf() + + expect(runtimeEnvironmentCallMock).not.toHaveBeenCalledWith( + expect.objectContaining({ method: 'files.stat' }) + ) + expect(openFileMock).not.toHaveBeenCalled() + }) + + it('drops the click when the workspace moves to another host while candidates are checked', async () => { + setPlatform('Macintosh') + vi.mocked(getConnectionId).mockReturnValue('ssh-1') + const probe = createDeferred() + const fsPathsExist = vi.fn(() => probe.promise) + window.api.fs.pathsExist = fsPathsExist + const deps = { startupCwd: '/home/u/proj', worktreeId: 'wt-1', worktreePath: '/home/u/proj' } + + openFilePathLinkAtBufferPosition( + makeBuffer([makeBufferLine('/home/u/proj/a.txt'), makeBufferLine('/home/u/proj/b.txt')]), + { x: 4, y: 1 }, + 80, + deps + ) + await flushAsyncWork() + expect(fsPathsExist).toHaveBeenCalled() + vi.mocked(getConnectionId).mockReturnValue(null) + probe.resolve([{ exists: false }, { exists: true }]) + await flushDoubleRaf() + + expect(statMock).not.toHaveBeenCalled() + expect(openFileMock).not.toHaveBeenCalled() + }) + + it('still says why when every candidate is already known missing', async () => { + setPlatform('Macintosh') + hostAnswers(() => ({ exists: false })) + const onOpenFailure = vi.fn() + const pathExistsCache = new Map( + [ + '/srv/repo/a.txt', + '/srv/repo/a.txt/srv/repo/b.txt', + '/srv/repo/a.txt/srv/repo/b.txt/srv/repo/c.txt' + ].map((path) => [runtimeKey(path), false]) + ) + + expect(clickFirstPath(onOpenFailure, pathExistsCache)).toBe(true) + await flushDoubleRaf() + + expect(onOpenFailure).toHaveBeenCalledWith( + '/srv/repo/a.txt', + expect.objectContaining({ verdict: 'missing' }) + ) + }) + + it('leaves an OSC 8 hyperlink to xterm when its label is a path known missing', () => { + setPlatform('Macintosh') + const label = makeBufferLine('nwparker/fix-123') + const linked = Object.assign(label, { + getCell: () => ({ hasExtendedAttrs: () => true, extended: { urlId: 1 } }) + }) + const pathExistsCache = new Map([ + [getTerminalPathExistsCacheKey({ absolutePath: '/repo/nwparker/fix-123' }), false] + ]) + const onOpenFailure = vi.fn() + + expect( + openFilePathLinkAtBufferPosition(makeBuffer([linked]), { x: 4, y: 1 }, 80, { + startupCwd: '/repo', + worktreeId: 'wt-1', + worktreePath: '/repo', + pathExistsCache, + onOpenFailure + }) + ).toBe(false) + }) +}) diff --git a/src/renderer/src/lib/file-link-open-failure-toast.ts b/src/renderer/src/lib/file-link-open-failure-toast.ts new file mode 100644 index 00000000000..80c12f0cd18 --- /dev/null +++ b/src/renderer/src/lib/file-link-open-failure-toast.ts @@ -0,0 +1,14 @@ +import type { FileOpenFailure } from '@/components/terminal-pane/terminal-file-open-routing' +import { + showFileLinkNotFoundToast, + showFileLinkUnverifiableToast +} from '@/components/native-chat/native-chat-file-link-toasts' + +/** Why a file link opened nothing, without claiming a file is gone when the host could not answer. */ +export function showFileLinkOpenFailureToast(filePath: string, failure: FileOpenFailure): void { + if (failure.verdict === 'unverifiable') { + showFileLinkUnverifiableToast(filePath, failure.error) + } else { + showFileLinkNotFoundToast(filePath) + } +} diff --git a/src/renderer/src/runtime/runtime-file-metadata-client.ts b/src/renderer/src/runtime/runtime-file-metadata-client.ts index ff026cb1173..a18a873f27a 100644 --- a/src/renderer/src/runtime/runtime-file-metadata-client.ts +++ b/src/renderer/src/runtime/runtime-file-metadata-client.ts @@ -50,7 +50,12 @@ export async function statRuntimePath( export function isMissingRuntimePathError(error: unknown): boolean { const message = error instanceof Error ? error.message.toLowerCase() : String(error).toLowerCase() return ( - message.includes('enoent') || message.includes('not found') || message.includes('no such file') + message.includes('enoent') || + message.includes('not found') || + message.includes('no such file') || + // Why: older hosts reject a path under a file (`a.txt/b.txt`) with ENOTDIR; it is just as absent. + // The full errno text, so a file merely named `enotdir…` is not read as absence. + message.includes('enotdir: not a directory') ) } diff --git a/src/renderer/src/runtime/runtime-path-existence-error-parity.test.ts b/src/renderer/src/runtime/runtime-path-existence-error-parity.test.ts index 17be77ef4c4..eb90494e3b0 100644 --- a/src/renderer/src/runtime/runtime-path-existence-error-parity.test.ts +++ b/src/renderer/src/runtime/runtime-path-existence-error-parity.test.ts @@ -1,6 +1,6 @@ import { expect, it } from 'vitest' import { runtimePathsExist } from './runtime-path-existence-batch' -import { runtimePathExists } from './runtime-file-metadata-client' +import { isMissingRuntimePathError, runtimePathExists } from './runtime-file-metadata-client' import { installRuntimeFileClientEnvironment, runtimeEnvironmentCall, @@ -77,3 +77,12 @@ for (const stage of ['status', 'operation'] as const) { } }) } + +it('reads an older host ENOTDIR as missing, but not a file merely named after it', () => { + expect( + isMissingRuntimePathError(new Error("ENOTDIR: not a directory, stat '/r/a.txt/b.txt'")) + ).toBe(true) + expect( + isMissingRuntimePathError(new Error("EACCES: permission denied, stat '/r/enotdir-handling.ts'")) + ).toBe(false) +})