diff --git a/src/renderer/src/components/native-chat/native-chat-file-link-toasts.ts b/src/renderer/src/components/native-chat/native-chat-file-link-toasts.ts new file mode 100644 index 00000000000..fd7ac9484b9 --- /dev/null +++ b/src/renderer/src/components/native-chat/native-chat-file-link-toasts.ts @@ -0,0 +1,36 @@ +import { toast } from 'sonner' +import { translate } from '@/i18n/i18n' +import { readIpcErrorMessage } from '@/lib/ipc-error' + +export function showFileLinkNotFoundToast(filePath: string): void { + toast.error( + translate('components.native-chat.fileLinks.notFound', 'File not found: {{value0}}', { + value0: filePath + }) + ) +} + +/** The host could not be asked whether the file exists, so the toast must not claim it is gone. */ +export function showFileLinkUnverifiableToast(filePath: string, error: unknown): void { + toast.error( + translate( + 'components.native-chat.fileLinks.unverifiable', + "Couldn't check {{value0}}: {{value1}}", + { + value0: filePath, + value1: readIpcErrorMessage(error) ?? String(error) + } + ) + ) +} + +/** e.g. `~/x` when the workspace gives no way to know the home folder. */ +export function showFileLinkUnresolvedToast(pathText: string): void { + toast.error( + translate( + 'components.native-chat.fileLinks.unresolved', + "Couldn't resolve {{value0}} in this workspace", + { value0: pathText } + ) + ) +} diff --git a/src/renderer/src/components/native-chat/use-native-chat-file-link-click.test.tsx b/src/renderer/src/components/native-chat/use-native-chat-file-link-click.test.tsx new file mode 100644 index 00000000000..3bc8ef51739 --- /dev/null +++ b/src/renderer/src/components/native-chat/use-native-chat-file-link-click.test.tsx @@ -0,0 +1,200 @@ +// @vitest-environment happy-dom +import { cleanup, fireEvent, render, screen } from '@testing-library/react' +import { afterEach, describe, expect, it, vi } from 'vitest' +import CommentMarkdown from '@/components/sidebar/CommentMarkdown' +import type { NativeChatFileLinkContext } from './native-chat-file-link' +import { useNativeChatFileLinkClick } from './use-native-chat-file-link-click' + +const mocks = vi.hoisted(() => ({ + openDetectedFilePath: vi.fn(), + showNotFound: vi.fn(), + showUnverifiable: vi.fn(), + showUnresolved: vi.fn() +})) + +vi.mock('@/components/terminal-pane/terminal-file-open-routing', () => ({ + openDetectedFilePath: mocks.openDetectedFilePath +})) +vi.mock('./native-chat-file-link-toasts', () => ({ + showFileLinkNotFoundToast: mocks.showNotFound, + showFileLinkUnverifiableToast: mocks.showUnverifiable, + showFileLinkUnresolvedToast: mocks.showUnresolved +})) +vi.mock('@/store', () => ({ + useAppStore: Object.assign( + (selector: (state: Record) => unknown) => selector({}), + { getState: () => ({ settings: {} }) } + ) +})) + +const context: NativeChatFileLinkContext = { + worktreeId: 'wt-1', + worktreePath: '/repo', + runtimeEnvironmentId: null +} + +function Transcript(props: { + markdown: string + linkContext?: NativeChatFileLinkContext +}): React.JSX.Element { + const onLinkClick = useNativeChatFileLinkClick(props.linkContext ?? context) + return ( + + ) +} + +function clickLink(name: string): void { + fireEvent.click(screen.getByRole('link', { name })) +} + +function failLastOpen(verdict: 'missing' | 'unverifiable', error: unknown = new Error('x')): void { + mocks.openDetectedFilePath.mock.calls.at(-1)?.[3].onOpenFailure({ verdict, error }) +} + +afterEach(() => { + cleanup() + vi.clearAllMocks() +}) + +describe('useNativeChatFileLinkClick', () => { + it('does not underline a bare file name the click could not open', () => { + render() + + expect(screen.queryByRole('link')).toBeNull() + expect(screen.getByText('deck.md').tagName).toBe('CODE') + }) + + it('reports a missing relative path instead of doing nothing', () => { + render() + + clickLink('docs/deck.md') + + expect(mocks.openDetectedFilePath).toHaveBeenCalledWith( + '/repo/docs/deck.md', + null, + null, + expect.objectContaining({ worktreeId: 'wt-1', onOpenFailure: expect.any(Function) }) + ) + failLastOpen('missing') + expect(mocks.showNotFound).toHaveBeenCalledWith('/repo/docs/deck.md') + }) + + it('reports a host that could not answer without claiming the file is gone', () => { + render() + + clickLink('docs/deck.md') + const error = new Error('SSH connection closed') + failLastOpen('unverifiable', error) + + expect(mocks.showUnverifiable).toHaveBeenCalledWith('/repo/docs/deck.md', error) + expect(mocks.showNotFound).not.toHaveBeenCalled() + }) + + it('reports a missing absolute path', () => { + render() + + clickLink('/repo/src/app.ts:12') + + expect(mocks.openDetectedFilePath).toHaveBeenCalledWith( + '/repo/src/app.ts', + 12, + null, + expect.anything() + ) + failLastOpen('missing') + expect(mocks.showNotFound).toHaveBeenCalledWith('/repo/src/app.ts') + }) + + it('opens an explicit markdown link to a bare file name with a line', () => { + render() + + clickLink('the readme') + + expect(mocks.openDetectedFilePath).toHaveBeenCalledWith( + '/repo/README.md', + 5, + null, + expect.anything() + ) + }) + + it('resolves URL syntax in explicit markdown links once', () => { + render() + + clickLink('plan') + clickLink('notes') + + expect(mocks.openDetectedFilePath).toHaveBeenNthCalledWith( + 1, + '/repo/docs/plan.md', + 7, + null, + expect.anything() + ) + expect(mocks.openDetectedFilePath).toHaveBeenNthCalledWith( + 2, + '/repo/docs/release notes.md', + null, + null, + expect.anything() + ) + }) + + it('opens file URIs written in inline code and prose', () => { + render( + + ) + + clickLink('file:///repo/src/app.ts') + clickLink('file:///repo/docs/release%20notes.md#L4') + + expect(mocks.openDetectedFilePath).toHaveBeenNthCalledWith( + 1, + '/repo/src/app.ts', + null, + null, + expect.anything() + ) + expect(mocks.openDetectedFilePath).toHaveBeenNthCalledWith( + 2, + '/repo/docs/release notes.md', + 4, + null, + expect.anything() + ) + }) + + it('keeps # in a linked path instead of treating it as a fragment', () => { + render() + + clickLink('My C# App/Program.cs') + + expect(mocks.openDetectedFilePath).toHaveBeenCalledWith( + '/repo/My C# App/Program.cs', + null, + null, + expect.anything() + ) + }) + + it('reports a link it cannot resolve without claiming the file is missing', () => { + render( + + ) + + clickLink('~/.claude/plans/plan.md') + + expect(mocks.openDetectedFilePath).not.toHaveBeenCalled() + expect(mocks.showUnresolved).toHaveBeenCalledWith('~/.claude/plans/plan.md') + expect(mocks.showNotFound).not.toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/components/native-chat/use-native-chat-file-link-click.ts b/src/renderer/src/components/native-chat/use-native-chat-file-link-click.ts index 3e745bcd2bc..d63276354fc 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-file-link-click.ts +++ b/src/renderer/src/components/native-chat/use-native-chat-file-link-click.ts @@ -1,15 +1,30 @@ import { useCallback } from 'react' import type { CommentMarkdownLinkClickHandler } from '@/components/sidebar/CommentMarkdown' import { openDetectedFilePath } from '@/components/terminal-pane/terminal-file-open-routing' +import { routeNativeChatHref } from '../../../../shared/native-chat-href-routing' import { resolveNativeChatFileLink, type NativeChatFileLinkContext } from './native-chat-file-link' +import { + showFileLinkNotFoundToast, + showFileLinkUnresolvedToast, + showFileLinkUnverifiableToast +} from './native-chat-file-link-toasts' export function useNativeChatFileLinkClick( context: NativeChatFileLinkContext | null ): CommentMarkdownLinkClickHandler | undefined { const openFileLink = useCallback( (event, href) => { + if (!context) { + return + } const target = resolveNativeChatFileLink(href, context) - if (!target || !context) { + if (!target) { + const route = routeNativeChatHref(href) + if (route.kind === 'file') { + // Why: e.g. `~/x` when the home folder cannot be inferred; never a dead click. + event.preventDefault() + showFileLinkUnresolvedToast(route.pathText) + } return } event.preventDefault() @@ -18,7 +33,12 @@ export function useNativeChatFileLinkClick( worktreeId: context.worktreeId, worktreePath: context.worktreePath, runtimeEnvironmentId: context.runtimeEnvironmentId, - openWithSystemDefault: event.shiftKey + openWithSystemDefault: event.shiftKey, + // Why: an underlined link must answer every click, so a miss says why. + onOpenFailure: (failure) => + failure.verdict === 'unverifiable' + ? showFileLinkUnverifiableToast(target.absolutePath, failure.error) + : showFileLinkNotFoundToast(target.absolutePath) }) }, [context] diff --git a/src/renderer/src/components/sidebar/CommentMarkdown.link-click.test.tsx b/src/renderer/src/components/sidebar/CommentMarkdown.link-click.test.tsx index a3f7c7aad1f..ecc94b2806c 100644 --- a/src/renderer/src/components/sidebar/CommentMarkdown.link-click.test.tsx +++ b/src/renderer/src/components/sidebar/CommentMarkdown.link-click.test.tsx @@ -407,7 +407,7 @@ describe('CommentMarkdown link click handler', () => { expect(container.textContent).toContain('"John 3:16"') }) - it('preserves line suffixes on valid spaced path shapes', () => { + it('preserves line suffixes on valid spaced path shapes, but not on bare file names', () => { const content = 'Open "My Folder/notes:12", `My Notes.md:7`, and "C:\\My Folder\\notes.txt:12:3".' container = document.createElement('div') @@ -428,12 +428,10 @@ describe('CommentMarkdown link click handler', () => { const anchors = Array.from(container.querySelectorAll('a')) expect(anchors.map((anchor) => anchor.textContent)).toEqual([ 'My Folder/notes:12', - 'My Notes.md:7', String.raw`C:\My Folder\notes.txt:12:3` ]) expect(anchors.map((anchor) => routeNativeChatHref(anchor.getAttribute('href')))).toEqual([ { kind: 'file', pathText: 'My Folder/notes:12', line: null }, - { kind: 'file', pathText: 'My Notes.md:7', line: null }, { kind: 'file', pathText: String.raw`C:\My Folder\notes.txt:12:3`, line: null } ]) }) diff --git a/src/renderer/src/components/sidebar/comment-markdown-native-chat-file-links.ts b/src/renderer/src/components/sidebar/comment-markdown-native-chat-file-links.ts index 47ee65b2a72..338569f2f17 100644 --- a/src/renderer/src/components/sidebar/comment-markdown-native-chat-file-links.ts +++ b/src/renderer/src/components/sidebar/comment-markdown-native-chat-file-links.ts @@ -2,7 +2,10 @@ import { createNativeChatFileHref, routeNativeChatHref } from '../../../../shared/native-chat-href-routing' -import { parseFileLinkLocation } from '../../../../shared/file-link-location' +import { + formatFileLinkLocation, + parseFileLinkLocation +} from '../../../../shared/file-link-location' import { extractTerminalFileLinks, type ParsedTerminalFileLink } from '@/lib/terminal-links' type MarkdownNode = { @@ -14,16 +17,16 @@ type MarkdownNode = { const ROOTED_PATH_PREFIX_PATTERN = /^(?:~[\\/]|\.{1,2}[\\/]|[\\/]|[A-Za-z]:[\\/])/ -function isLinkifiableFile(link: ParsedTerminalFileLink, requireSeparator: boolean): boolean { +// Why: a link is underlined only when it names a path; a bare `name.md` resolves nowhere +// reliable, so underlining it promises a click that cannot open anything. +function isLinkifiableFile(link: ParsedTerminalFileLink, isProse: boolean): boolean { const hasRootedPrefix = ROOTED_PATH_PREFIX_PATTERN.test(link.pathText) const hasLineSuffix = link.line !== null || link.column !== null const hasAlphabeticExtension = /\.[\p{L}][\p{L}\p{N}\p{M}_+-]*$/u.test(link.pathText) const hasPathExtension = /\.[\p{L}\p{N}][\p{L}\p{N}\p{M}_+-]*$/u.test(link.pathText) return ( - (!requireSeparator || /[\\/]/.test(link.pathText)) && - (hasRootedPrefix || - hasLineSuffix || - (requireSeparator ? hasPathExtension : hasAlphabeticExtension)) && + /[\\/]/.test(link.pathText) && + (hasRootedPrefix || hasLineSuffix || (isProse ? hasPathExtension : hasAlphabeticExtension)) && routeNativeChatHref(link.displayText).kind === 'file' ) } @@ -72,10 +75,11 @@ function hasPartialPathBoundary(value: string, link: ParsedTerminalFileLink): bo ) } -function createFileLinkNode(value: string, child: MarkdownNode): MarkdownNode { +// Why: wrap the parsed location, not the display text; a `file://` URI must not reach the literal href. +function createFileLinkNode(link: ParsedTerminalFileLink, child: MarkdownNode): MarkdownNode { return { type: 'link', - url: createNativeChatFileHref(value), + url: createNativeChatFileHref(formatFileLinkLocation(link)), children: [child] } } @@ -122,7 +126,7 @@ function splitTextSegment(value: string): MarkdownNode[] { if (link.startIndex > cursor) { children.push({ type: 'text', value: value.slice(cursor, link.startIndex) }) } - children.push(createFileLinkNode(link.displayText, { type: 'text', value: link.displayText })) + children.push(createFileLinkNode(link, { type: 'text', value: link.displayText })) cursor = link.endIndex } if (cursor < value.length) { @@ -185,14 +189,15 @@ function splitTextNode(value: string): MarkdownNode[] { let cursor = 0 for (const match of value.matchAll(QUOTED_TEXT_PATTERN)) { const content = match[1] ?? match[2] - if (!content || !exactFileLink(content, true)) { + const link = content ? exactFileLink(content, true) : null + if (!content || !link) { continue } const matchIndex = match.index ?? 0 const quote = match[0][0] children.push(...splitUnquotedText(value.slice(cursor, matchIndex))) children.push({ type: 'text', value: quote }) - children.push(createFileLinkNode(content, { type: 'text', value: content })) + children.push(createFileLinkNode(link, { type: 'text', value: content })) children.push({ type: 'text', value: quote }) cursor = matchIndex + match[0].length } @@ -208,13 +213,16 @@ function inlineCodeFileLink(node: MarkdownNode): MarkdownNode | null { if (!value) { return null } - return exactFileLink(value, true) ? createFileLinkNode(value, node) : null + const link = exactFileLink(value, true) + return link ? createFileLinkNode(link, node) : null } function transformFileLinks(node: MarkdownNode): void { if (node.type === 'link') { - if (node.url && routeNativeChatHref(node.url).kind === 'file') { - node.url = createNativeChatFileHref(node.url) + const route = routeNativeChatHref(node.url) + if (route.kind === 'file') { + // Why: the wrapped href carries literal location text, so URL syntax is resolved here, once. + node.url = createNativeChatFileHref(formatFileLinkLocation(route)) } return } 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 346424c960a..21bb89d39aa 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 @@ -8,7 +8,11 @@ import { buildWorkspaceFileContext, canClientOsOpenWorkspaceFile } from '@/lib/workspace-file-host-routing' -import { statRuntimePath, type RuntimeFileOperationArgs } from '@/runtime/runtime-file-client' +import { + isMissingRuntimePathError, + statRuntimePath, + type RuntimeFileOperationArgs +} from '@/runtime/runtime-file-client' import { useAppStore } from '@/store' import { activateAndRevealWorkspace, activateAndRevealWorktree } from '@/lib/worktree-activation' import { resolveKnownWorktreeRootPathLink } from './terminal-worktree-path-link' @@ -20,12 +24,20 @@ import { type ExecutionHostId } from '../../../../shared/execution-host' +export type FileOpenFailure = { + /** `missing` is a verified absence; `unverifiable` means the host could not answer (dropped SSH, timeout, denied path). */ + verdict: 'missing' | 'unverifiable' + error: unknown +} + type TerminalFileOpenDeps = { worktreeId: string worktreePath: string runtimeEnvironmentId?: string | null wslDistro?: string | null openWithSystemDefault?: boolean + /** Reports a path that could not be verified before opening; skipped once a later open supersedes it. */ + onOpenFailure?: (failure: FileOpenFailure) => void } export function isHtmlFilePath(filePath: string): boolean { @@ -166,7 +178,14 @@ export function openDetectedFilePath( await window.api.fs.authorizeExternalPath({ targetPath: mappedFilePath }) } statResult = await statRuntimePath(fileContext, mappedFilePath) - } catch { + } catch (error) { + if (requestId === latestOpenDetectedFilePathRequestId && deps.onOpenFailure) { + // Why: loss of contact with the host is not evidence the file is gone. + deps.onOpenFailure({ + verdict: isMissingRuntimePathError(error) ? 'missing' : 'unverifiable', + error + }) + } return } diff --git a/src/renderer/src/components/terminal-pane/terminal-link-missing-file-open.test.ts b/src/renderer/src/components/terminal-pane/terminal-link-missing-file-open.test.ts new file mode 100644 index 00000000000..500975a29b8 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-link-missing-file-open.test.ts @@ -0,0 +1,93 @@ +import { describe, expect, it, vi } from 'vitest' +import { openDetectedFilePath } from './terminal-link-handlers' +import type { FileOpenFailure } from './terminal-file-open-routing' +import { createTerminalLinkTestDoubles } from './terminal-link-handlers-test-fixtures' +import { + flushAsyncWork, + installTerminalLinkTestEnvironment, + setPlatform +} from './terminal-link-handlers-test-harness' + +const doubles = createTerminalLinkTestDoubles() +const { storeState, deps, openFileMock, statMock, authorizeExternalPathMock } = 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) + +type OnOpenFailure = (failure: FileOpenFailure) => void + +describe('openDetectedFilePath on a path it cannot verify', () => { + it('reports a verified miss', async () => { + setPlatform('Macintosh') + const error = new Error("Error invoking remote method 'fs:stat': Error: ENOENT: no such file") + statMock.mockRejectedValueOnce(error) + const onOpenFailure = vi.fn() + + openDetectedFilePath('/tmp/src/gone.md', null, null, { ...deps, onOpenFailure }) + await flushAsyncWork() + + expect(openFileMock).not.toHaveBeenCalled() + expect(onOpenFailure).toHaveBeenCalledTimes(1) + expect(onOpenFailure).toHaveBeenCalledWith({ verdict: 'missing', error }) + }) + + it('reports a host that could not answer as unverifiable, not missing', async () => { + setPlatform('Macintosh') + const error = new Error('SSH connection closed') + statMock.mockRejectedValueOnce(error) + const onOpenFailure = vi.fn() + + openDetectedFilePath('/tmp/src/present.md', null, null, { ...deps, onOpenFailure }) + await flushAsyncWork() + + expect(onOpenFailure).toHaveBeenCalledTimes(1) + expect(onOpenFailure.mock.calls[0][0]).toEqual({ verdict: 'unverifiable', error }) + }) + + it('reports a refused path authorization as unverifiable', async () => { + setPlatform('Macintosh') + const error = new Error('Path is outside the allowed roots') + authorizeExternalPathMock.mockRejectedValueOnce(error) + const onOpenFailure = vi.fn() + + openDetectedFilePath('/tmp/src/denied.md', null, null, { ...deps, onOpenFailure }) + await flushAsyncWork() + + expect(statMock).not.toHaveBeenCalled() + expect(onOpenFailure.mock.calls[0][0]).toEqual({ verdict: 'unverifiable', error }) + }) + + it('skips the callback when a later click superseded the failing one', async () => { + setPlatform('Macintosh') + let rejectFirstStat!: (error: Error) => void + statMock.mockImplementationOnce( + () => + new Promise((_resolve, reject) => { + rejectFirstStat = reject + }) + ) + const onOpenFailure = vi.fn() + + openDetectedFilePath('/tmp/src/gone.md', null, null, { ...deps, onOpenFailure }) + await flushAsyncWork() + openDetectedFilePath('/tmp/src/other.ts', null, null, deps) + rejectFirstStat(new Error('ENOENT')) + await flushAsyncWork() + + expect(onOpenFailure).not.toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index f18f9a2fc21..0fe1623cdcf 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -17614,6 +17614,11 @@ "blocked": "Goal blocked", "limited": "Goal limited", "attachmentsUnsupported": "Remove attachments before setting a goal." + }, + "fileLinks": { + "notFound": "File not found: {{value0}}", + "unverifiable": "Couldn't check {{value0}}: {{value1}}", + "unresolved": "Couldn't resolve {{value0}} in this workspace" } }, "tab": { diff --git a/src/renderer/src/runtime/runtime-file-client.ts b/src/renderer/src/runtime/runtime-file-client.ts index 104053a4664..37ed975e918 100644 --- a/src/renderer/src/runtime/runtime-file-client.ts +++ b/src/renderer/src/runtime/runtime-file-client.ts @@ -26,6 +26,7 @@ export { searchRuntimeFiles } from './runtime-file-search-client' export { + isMissingRuntimePathError, listRuntimeMarkdownDocuments, runtimePathExists, statRuntimePath diff --git a/src/shared/file-link-location.ts b/src/shared/file-link-location.ts index 1450f130338..fc80991523e 100644 --- a/src/shared/file-link-location.ts +++ b/src/shared/file-link-location.ts @@ -17,3 +17,16 @@ export function parseFileLinkLocation(value: string): ParsedFileLinkLocation | n } return { pathText, line, column } } + +/** Inverse of `parseFileLinkLocation`: `path`, `path:line`, or `path:line:column`. */ +export function formatFileLinkLocation(location: { + pathText: string + line: number | null + column?: number | null +}): string { + if (location.line === null) { + return location.pathText + } + const column = location.column == null ? '' : `:${location.column}` + return `${location.pathText}:${location.line}${column}` +} diff --git a/src/shared/native-chat-href-routing.test.ts b/src/shared/native-chat-href-routing.test.ts index 5f6e7ad4595..5ba1149fe3e 100644 --- a/src/shared/native-chat-href-routing.test.ts +++ b/src/shared/native-chat-href-routing.test.ts @@ -76,6 +76,33 @@ describe('routeNativeChatHref', () => { expect(routeNativeChatHref(createNativeChatFileHref(` ${href}`))).toEqual({ kind: 'none' }) }) + it('keeps wrapped location text literal', () => { + expect(routeNativeChatHref(createNativeChatFileHref('My C# App/Program.cs'))).toEqual({ + kind: 'file', + pathText: 'My C# App/Program.cs', + line: null + }) + expect(routeNativeChatHref(createNativeChatFileHref('assets/icon%20big.png?v'))).toEqual({ + kind: 'file', + pathText: 'assets/icon%20big.png?v', + line: null + }) + }) + + it('reads a bare file name with a line suffix as a file, not a scheme', () => { + expect(routeNativeChatHref('README.md:5')).toEqual({ + kind: 'file', + pathText: 'README.md:5', + line: null + }) + expect(routeNativeChatHref('App.tsx:12:3')).toEqual({ + kind: 'file', + pathText: 'App.tsx:12:3', + line: null + }) + expect(routeNativeChatHref('localhost:3000')).toEqual({ kind: 'none' }) + }) + it('drops anchors, unknown schemes, malformed file URIs, and empty hrefs', () => { expect(routeNativeChatHref('#section')).toEqual({ kind: 'none' }) expect(routeNativeChatHref(undefined)).toEqual({ kind: 'none' }) diff --git a/src/shared/native-chat-href-routing.ts b/src/shared/native-chat-href-routing.ts index edf36565dab..ca281f24f0c 100644 --- a/src/shared/native-chat-href-routing.ts +++ b/src/shared/native-chat-href-routing.ts @@ -8,9 +8,12 @@ export type NativeChatHrefRoute = const WEB_SCHEME_PATTERN = /^(?:https?|mailto):/i const SCHEME_PATTERN = /^[A-Za-z][A-Za-z0-9+.-]*:/ +// Why: `README.md:5` is a file location; the scheme pattern alone reads `README.md:` as a scheme. +const BARE_FILE_LOCATION_PATTERN = /^[^\s:/\\?#]+\.[\p{L}\p{N}_+-]+:\d+(?::\d+)?$/u export const NATIVE_CHAT_FILE_HREF_PREFIX = '#orca-native-chat-file=' const MAX_NATIVE_CHAT_FILE_HREF_DECODES = 4 +/** Wraps literal file-location text (`path`, `path:line[:col]`); routing never re-parses it as a URL. */ export function createNativeChatFileHref(pathText: string): string { return `${NATIVE_CHAT_FILE_HREF_PREFIX}${encodeURIComponent(pathText)}` } @@ -67,16 +70,22 @@ export function routeNativeChatHref(href: string | null | undefined): NativeChat if (!trimmed) { return { kind: 'none' } } + let isLiteralFileLocation = false for (let depth = 0; depth < MAX_NATIVE_CHAT_FILE_HREF_DECODES; depth += 1) { const encodedFileHref = decodeNativeChatFileHref(trimmed) if (!encodedFileHref) { break } trimmed = encodedFileHref.trim() + isLiteralFileLocation = true } if (!trimmed || trimmed.startsWith(NATIVE_CHAT_FILE_HREF_PREFIX)) { return { kind: 'none' } } + if (isLiteralFileLocation) { + // Why: `#`, `?` and `%XX` are legal filename characters, not URL syntax, in wrapped text. + return { kind: 'file', pathText: trimmed, line: null } + } if (trimmed.startsWith('#')) { return { kind: 'none' } } @@ -96,7 +105,11 @@ export function routeNativeChatHref(href: string | null | undefined): NativeChat } return { kind: 'file', pathText, line: parseLineFragment(url.hash.slice(1)) } } - if (!isWindowsAbsolutePathLike(trimmed) && SCHEME_PATTERN.test(trimmed)) { + if ( + !isWindowsAbsolutePathLike(trimmed) && + !BARE_FILE_LOCATION_PATTERN.test(trimmed) && + SCHEME_PATTERN.test(trimmed) + ) { return { kind: 'none' } } const { pathText, line } = stripQueryAndHash(trimmed)