From 5ea4e5f99b06e17a0adcd2435fe8b3101e347be7 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Mon, 3 Aug 2026 16:28:35 -0700 Subject: [PATCH] fix(mobile): make native-chat file links and path citations tappable (STA-3331) - Linkify POSIX absolute paths in chat prose (leading-/ regex alternative; URL guard now keys off the char before the matched slash) - Parse agent-style path:line(:col) citations in prose, code spans, and the open flow; line/column ride into the mobile file preview route - Route non-web markdown hrefs (file: URIs, relative/absolute paths) to the file opener instead of silently dropping them; unknown schemes stay dead - Resolve chat paths against the worktree root, not the terminal's live cwd - Reuse the terminal tap-to-open flow for chat taps (haptic, preview route, tab activation with retries) via a shared identity-stable hook, and toast on misses instead of silent no-ops - Keep snake_case paths whole (intraword underscores are literal text), scan bold/italic/strike spans for paths, split trailing punctuation off autolinks, and let taps land while the composer keyboard is up --- .../app/h/[hostId]/session/[worktreeId].tsx | 59 ++--- .../MobileMarkdown.file-links.test.ts | 119 ++++++++++ mobile/src/components/MobileMarkdown.tsx | 72 ++++-- .../markdown-file-path-detection.test.ts | 96 +++++++- .../markdown-file-path-detection.ts | 51 +++- .../components/markdown-href-routing.test.ts | 79 +++++++ .../src/components/markdown-href-routing.ts | 91 ++++++++ .../markdown-inline-token-rules.test.ts | 62 +++++ .../components/markdown-inline-token-rules.ts | 42 ++++ .../src/session/MobileNativeChatOverlay.tsx | 6 +- mobile/src/session/MobileNativeChatView.tsx | 3 + ...n.test.ts => mobile-file-tap-open.test.ts} | 109 ++++++++- ...le-tap-open.ts => mobile-file-tap-open.ts} | 31 ++- .../mobile-native-chat-open-file.test.ts | 218 +++++++++++------- .../session/mobile-native-chat-open-file.ts | 75 +++--- .../use-mobile-file-tap-handlers.test.ts | 113 +++++++++ .../session/use-mobile-file-tap-handlers.ts | 124 ++++++++++ .../use-mobile-native-chat-controller.ts | 26 +-- 18 files changed, 1127 insertions(+), 249 deletions(-) create mode 100644 mobile/src/components/MobileMarkdown.file-links.test.ts create mode 100644 mobile/src/components/markdown-href-routing.test.ts create mode 100644 mobile/src/components/markdown-href-routing.ts create mode 100644 mobile/src/components/markdown-inline-token-rules.test.ts create mode 100644 mobile/src/components/markdown-inline-token-rules.ts rename mobile/src/session/{mobile-terminal-file-tap-open.test.ts => mobile-file-tap-open.test.ts} (81%) rename mobile/src/session/{mobile-terminal-file-tap-open.ts => mobile-file-tap-open.ts} (84%) create mode 100644 mobile/src/session/use-mobile-file-tap-handlers.test.ts create mode 100644 mobile/src/session/use-mobile-file-tap-handlers.ts diff --git a/mobile/app/h/[hostId]/session/[worktreeId].tsx b/mobile/app/h/[hostId]/session/[worktreeId].tsx index 8f6d7ca0c97..f0ed85df944 100644 --- a/mobile/app/h/[hostId]/session/[worktreeId].tsx +++ b/mobile/app/h/[hostId]/session/[worktreeId].tsx @@ -195,7 +195,7 @@ import { MobileTerminalLiveInputStatus } from '../../../../src/session/MobileTer import { MobileTerminalInputActions } from '../../../../src/session/MobileTerminalInputActions' import { resolveMobileFileTabDoc } from '../../../../src/files/mobile-file-tab-doc' import { captureMobileFileMutationOwnership } from '../../../../src/files/mobile-file-mutation-ownership' -import { openMobileTerminalFileTap } from '../../../../src/session/mobile-terminal-file-tap-open' +import { useMobileFileTapHandlers } from '../../../../src/session/use-mobile-file-tap-handlers' import { useLiveWorktreeName } from '../../../../src/session/use-live-worktree-name' import { acceptSessionSnapshot, @@ -3182,44 +3182,24 @@ export default function SessionScreen() { }) }, []) - // Tap a terminal file path → resolve on host, open as file tab (mirrors desktop Cmd/Ctrl-click); silent on a miss. - const handleFileTapActivationSeqRef = useRef(0) - const handleFileTap = useCallback( - (handle: string, pathText: string, line: number | null, column: number | null) => { - if (handle !== activeHandleRef.current || !client) { - return - } - const activationSeq = ++handleFileTapActivationSeqRef.current - openMobileTerminalFileTap({ - client, - hostId, - worktreeId, - worktreeName: routeWorktreeName, - terminalHandle: handle, - pathText, - cwd: terminalCwdRef.current.get(handle) ?? null, - line, - column, - pushPreviewRoute: (href) => router.push(href), - openBrowser: (url) => void handleCreateBrowserRef.current?.(url), - triggerOpenFeedback: triggerSelection, - fetchSessionTabs, - getSessionTabs: () => sessionTabsRef.current, - getActiveSessionTabId: () => activeSessionTabIdRef.current, - getActivationState: (activated) => ({ - activated, - activationSeq, - latestActivationSeq: handleFileTapActivationSeqRef.current, - sourceTerminalHandle: handle, - activeTerminalHandle: activeHandleRef.current, - activeTabType: activeSessionTabTypeRef.current - }), - switchSessionTab: (tab) => switchSessionTabRef.current?.(tab), - scheduleDelayedAction - }) - }, - [client, fetchSessionTabs, hostId, routeWorktreeName, router, scheduleDelayedAction, worktreeId] - ) + // Tap a terminal or chat file path → resolve on host, open as file tab/preview + // (mirrors desktop Cmd/Ctrl-click); chat taps surface misses via toast. + const { handleFileTap, handleNativeChatFileTap } = useMobileFileTapHandlers({ + client, + hostId, + worktreeId, + worktreeName: routeWorktreeName, + activeHandleRef, + terminalCwdRef, + openBrowser: (url) => void handleCreateBrowserRef.current?.(url), + fetchSessionTabs, + getSessionTabs: () => sessionTabsRef.current, + getActiveSessionTabId: () => activeSessionTabIdRef.current, + getActiveSessionTabType: () => activeSessionTabTypeRef.current, + switchSessionTab: (tab) => switchSessionTabRef.current?.(tab), + scheduleDelayedAction, + showToast + }) const handleOpenedFileDiffActivationSeqRef = useRef(0) // Capture active tab at tap time; reading it after openDiff would misread a mid-RPC switch and let the retry steal focus. @@ -4752,6 +4732,7 @@ export default function SessionScreen() { ))} Promise.resolve()) + +vi.mock('react-native', () => ({ + Linking: { openURL: (url: string) => openURL(url) }, + Pressable: 'Pressable', + ScrollView: 'ScrollView', + StyleSheet: { create: (styles: unknown) => styles, hairlineWidth: 1 }, + Text: 'Text', + View: 'View' +})) + +function flattenText(node: ReactTestInstance): string { + return node.children + .map((child) => (typeof child === 'string' ? child : flattenText(child))) + .join('') +} + +function pressables(renderer: ReactTestRenderer): ReactTestInstance[] { + return renderer.root.findAll( + (node) => node.type === ('Text' as never) && typeof node.props.onPress === 'function' + ) +} + +function pressByText(renderer: ReactTestRenderer, text: string): void { + const target = pressables(renderer).find((node) => flattenText(node) === text) + expect(target, `no pressable text ${JSON.stringify(text)}`).toBeDefined() + target!.props.onPress() +} + +describe('MobileMarkdown file links', () => { + let renderer: ReactTestRenderer | null = null + const onOpenFile = vi.fn() + + beforeEach(() => { + globalThis.IS_REACT_ACT_ENVIRONMENT = true + onOpenFile.mockClear() + openURL.mockClear() + }) + + afterEach(() => { + act(() => renderer?.unmount()) + renderer = null + }) + + function render(content: string): ReactTestRenderer { + act(() => { + renderer = create(createElement(MobileMarkdown, { content, onOpenFile })) + }) + return renderer! + } + + it('opens a tapped POSIX absolute path in prose', () => { + pressByText(render('Edit /Users/me/wt/src/app.tsx now'), '/Users/me/wt/src/app.tsx') + expect(onOpenFile).toHaveBeenCalledWith('/Users/me/wt/src/app.tsx') + }) + + it('opens a tapped path:line citation in prose', () => { + pressByText(render('see src/foo.ts:42 for the fix'), 'src/foo.ts:42') + expect(onOpenFile).toHaveBeenCalledWith('src/foo.ts:42') + }) + + it('routes a relative markdown href to the file opener with its #L line', () => { + pressByText(render('read [the plan](docs/plan.md#L7) first'), 'the plan') + expect(onOpenFile).toHaveBeenCalledWith('docs/plan.md:7') + expect(openURL).not.toHaveBeenCalled() + }) + + it('routes a file: href to the file opener', () => { + pressByText(render('[artifact](file:///tmp/out/result.json)'), 'artifact') + expect(onOpenFile).toHaveBeenCalledWith('/tmp/out/result.json') + }) + + it('keeps web links on the system browser', () => { + pressByText(render('go to [site](https://example.com/docs)'), 'site') + expect(openURL).toHaveBeenCalledWith('https://example.com/docs') + expect(onOpenFile).not.toHaveBeenCalled() + }) + + it('drops unknown-scheme hrefs without opening anything', () => { + pressByText(render('[ide](editor://file/x.ts)'), 'ide') + expect(openURL).not.toHaveBeenCalled() + expect(onOpenFile).not.toHaveBeenCalled() + }) + + it('keeps snake_case paths whole instead of shredding them as emphasis', () => { + const rendered = render('compare src/foo_bar.ts and src/baz_qux.ts now') + pressByText(rendered, 'src/foo_bar.ts') + pressByText(rendered, 'src/baz_qux.ts') + expect(onOpenFile).toHaveBeenNthCalledWith(1, 'src/foo_bar.ts') + expect(onOpenFile).toHaveBeenNthCalledWith(2, 'src/baz_qux.ts') + }) + + it('detects paths inside bold spans', () => { + pressByText(render('changed **src/foo.ts** heavily'), 'src/foo.ts') + expect(onOpenFile).toHaveBeenCalledWith('src/foo.ts') + }) + + it('excludes trailing sentence punctuation from autolinks', () => { + pressByText(render('see https://example.com/a.'), 'https://example.com/a') + expect(openURL).toHaveBeenCalledWith('https://example.com/a') + }) + + it('opens inline-code path:line citations', () => { + pressByText(render('fix `src/foo.ts:42` now'), 'src/foo.ts:42') + expect(onOpenFile).toHaveBeenCalledWith('src/foo.ts:42') + }) + + it('renders paths as plain text without onOpenFile', () => { + act(() => { + renderer = create(createElement(MobileMarkdown, { content: 'Edit src/app/Main.tsx now' })) + }) + expect(pressables(renderer!)).toHaveLength(0) + }) +}) diff --git a/mobile/src/components/MobileMarkdown.tsx b/mobile/src/components/MobileMarkdown.tsx index 8e968f604ac..975a4b4914a 100644 --- a/mobile/src/components/MobileMarkdown.tsx +++ b/mobile/src/components/MobileMarkdown.tsx @@ -7,6 +7,11 @@ import { isFilePathCodeSpan, normalizeFilePath } from './markdown-file-path-detection' +import { routeMarkdownHref } from './markdown-href-routing' +import { + isIntrawordUnderscoreToken, + trimAutolinkTrailingPunctuation +} from './markdown-inline-token-rules' import { parseMobileMarkdown } from './mobile-markdown-parser' type Props = { @@ -15,19 +20,26 @@ type Props = { /** Multiplier for prose font size (paragraphs, lists, quotes). Defaults to 1; * the chat view passes >1 so agent prose reads larger than the compact base. */ textScale?: number - /** When provided, detected file-path tokens render as tappable and invoke this - * with the worktree-relative path. Omitted on screens with no file viewer, where + /** When provided, detected file paths and file-target hrefs render as tappable + * and invoke this with the path text (worktree-relative or absolute, with an + * optional :line(:col) suffix). Omitted on screens with no file viewer, where * paths render as plain text (no behavior change). */ - onOpenFile?: (relativePath: string) => void + onOpenFile?: (pathText: string) => void } const MAX_TABLE_ROWS = 40 const MAX_TABLE_COLUMNS = 8 -function openMarkdownUrl(url: string): void { - const trimmed = url.trim() - if (/^(https?:|mailto:)/i.test(trimmed)) { - void Linking.openURL(trimmed).catch(() => {}) +// Web/mail hrefs open the system handler; file-target hrefs (file: URIs and +// scheme-less paths — the entire desktop file-link contract) go to onOpenFile. +function openMarkdownHref(href: string, onOpenFile?: (pathText: string) => void): void { + const route = routeMarkdownHref(href) + if (route.kind === 'web') { + void Linking.openURL(route.url).catch(() => {}) + return + } + if (route.kind === 'file' && onOpenFile) { + onOpenFile(route.pathText) } } @@ -36,7 +48,7 @@ function openMarkdownUrl(url: string): void { function renderTextRun( text: string, keyPrefix: string, - onOpenFile?: (relativePath: string) => void + onOpenFile?: (pathText: string) => void ): ReactNode { if (!onOpenFile) { return text @@ -61,39 +73,52 @@ function renderTextRun( }) } -function renderInline(text: string, onOpenFile?: (relativePath: string) => void): ReactNode[] { +function renderInline(text: string, onOpenFile?: (pathText: string) => void): ReactNode[] { const parts: ReactNode[] = [] const pattern = /(!\[[^\]]*\]\([^)]+\)|`[^`]+`|~~[^~]+~~|\*\*[^*]+\*\*|__[^_]+__|\*[^*\n]+\*|_[^_\n]+_|\[[^\]]+\]\([^)]+\)|https?:\/\/[^\s<]+)/g - let lastIndex = 0 + let pendingStart = 0 let match: RegExpExecArray | null while ((match = pattern.exec(text))) { - if (match.index > lastIndex) { - parts.push(renderTextRun(text.slice(lastIndex, match.index), `t${lastIndex}`, onOpenFile)) - } const token = match[0] + // Intraword `_` runs (snake_case, dunder tails) are literal text per + // CommonMark; leaving them unflushed keeps surrounding file paths whole + // for detection in the eventual text run. + if (token.startsWith('_') && isIntrawordUnderscoreToken(text, match.index, token)) { + continue + } + if (match.index > pendingStart) { + parts.push( + renderTextRun(text.slice(pendingStart, match.index), `t${pendingStart}`, onOpenFile) + ) + } + pendingStart = pattern.lastIndex const key = `${match.index}:${token}` const image = token.match(/^!\[([^\]]*)\]\(([^)]+)\)$/) const link = token.match(/^\[([^\]]+)\]\(([^)]+)\)$/) if (image) { parts.push( - openMarkdownUrl(image[2]!)}> + openMarkdownHref(image[2]!, onOpenFile)}> {image[1] || 'image'} ) } else if (link) { parts.push( - openMarkdownUrl(link[2]!)}> + openMarkdownHref(link[2]!, onOpenFile)}> {link[1]} ) } else if (/^https?:\/\//i.test(token)) { + const { url, trailing } = trimAutolinkTrailingPunctuation(token) parts.push( - openMarkdownUrl(token)}> - {token} + openMarkdownHref(url, onOpenFile)}> + {url} ) + if (trailing) { + parts.push({trailing}) + } } else if (token.startsWith('`')) { const code = token.slice(1, -1) if (onOpenFile && isFilePathCodeSpan(code)) { @@ -116,27 +141,26 @@ function renderInline(text: string, onOpenFile?: (relativePath: string) => void) } else if (token.startsWith('~~')) { parts.push( - {token.slice(2, -2)} + {renderTextRun(token.slice(2, -2), `${key}i`, onOpenFile)} ) } else if (token.startsWith('**') || token.startsWith('__')) { parts.push( - {token.slice(2, -2)} + {renderTextRun(token.slice(2, -2), `${key}i`, onOpenFile)} ) } else { parts.push( - {token.slice(1, -1)} + {renderTextRun(token.slice(1, -1), `${key}i`, onOpenFile)} ) } - lastIndex = pattern.lastIndex } - if (lastIndex < text.length) { - parts.push(renderTextRun(text.slice(lastIndex), `t${lastIndex}`, onOpenFile)) + if (pendingStart < text.length) { + parts.push(renderTextRun(text.slice(pendingStart), `t${pendingStart}`, onOpenFile)) } return parts } @@ -187,7 +211,7 @@ function MobileMarkdownInner({ content, fallback = '', textScale = 1, onOpenFile openMarkdownUrl(block.url)} + onPress={() => openMarkdownHref(block.url, onOpenFile)} > {block.alt || 'Open image'} diff --git a/mobile/src/components/markdown-file-path-detection.test.ts b/mobile/src/components/markdown-file-path-detection.test.ts index f75da01efdc..1b78c102692 100644 --- a/mobile/src/components/markdown-file-path-detection.test.ts +++ b/mobile/src/components/markdown-file-path-detection.test.ts @@ -2,7 +2,8 @@ import { describe, expect, it } from 'vitest' import { detectFilePathSegments, isFilePathCodeSpan, - normalizeFilePath + normalizeFilePath, + splitFilePathLineSuffix } from './markdown-file-path-detection' describe('detectFilePathSegments', () => { @@ -64,6 +65,41 @@ describe('detectFilePathSegments', () => { ]) }) + it('detects POSIX absolute paths', () => { + expect(detectFilePathSegments('Wrote /Users/me/wt/src/app.tsx today')).toEqual([ + { type: 'text', value: 'Wrote ' }, + { type: 'file', value: '/Users/me/wt/src/app.tsx', path: '/Users/me/wt/src/app.tsx' }, + { type: 'text', value: ' today' } + ]) + expect(detectFilePathSegments('/repo/src/index.ts')).toEqual([ + { type: 'file', value: '/repo/src/index.ts', path: '/repo/src/index.ts' } + ]) + }) + + it('detects paths with :line and :line:col suffixes', () => { + expect(detectFilePathSegments('see src/foo.ts:42 here')).toEqual([ + { type: 'text', value: 'see ' }, + { type: 'file', value: 'src/foo.ts:42', path: 'src/foo.ts:42' }, + { type: 'text', value: ' here' } + ]) + expect( + detectFilePathSegments('/wt/src/app.tsx:120:7 and C:\\repo\\a.ts:3').filter( + (s) => s.type === 'file' + ) + ).toEqual([ + { type: 'file', value: '/wt/src/app.tsx:120:7', path: '/wt/src/app.tsx:120:7' }, + { type: 'file', value: 'C:\\repo\\a.ts:3', path: 'C:\\repo\\a.ts:3' } + ]) + }) + + it('keeps a non-line colon tail out of the match', () => { + expect(detectFilePathSegments('edit src/foo.ts: then run')).toEqual([ + { type: 'text', value: 'edit ' }, + { type: 'file', value: 'src/foo.ts', path: 'src/foo.ts' }, + { type: 'text', value: ': then run' } + ]) + }) + it('does not match bare filenames without a slash', () => { expect(detectFilePathSegments('open Main.tsx please')).toEqual([ { type: 'text', value: 'open Main.tsx please' } @@ -74,6 +110,15 @@ describe('detectFilePathSegments', () => { expect(detectFilePathSegments('https://example.com/path/file.ts')).toEqual([ { type: 'text', value: 'https://example.com/path/file.ts' } ]) + expect(detectFilePathSegments('see https://example.com/path/file.ts:42 now')).toEqual([ + { type: 'text', value: 'see https://example.com/path/file.ts:42 now' } + ]) + }) + + it('does not match protocol-relative URLs', () => { + expect(detectFilePathSegments('load //cdn.example.com/lib/app.js')).toEqual([ + { type: 'text', value: 'load //cdn.example.com/lib/app.js' } + ]) }) it('does not match version numbers', () => { @@ -171,12 +216,61 @@ describe('isFilePathCodeSpan', () => { expect(isFilePathCodeSpan('node_modules/@scope/pkg/file.ts')).toBe(true) }) + it('accepts POSIX absolute paths and :line citations', () => { + expect(isFilePathCodeSpan('/Users/me/wt/src/app.tsx')).toBe(true) + expect(isFilePathCodeSpan('src/foo.ts:42')).toBe(true) + expect(isFilePathCodeSpan('src/foo.ts:42:7')).toBe(true) + expect(isFilePathCodeSpan('MobileNativeChatComposer.tsx:23')).toBe(true) + expect(isFilePathCodeSpan(String.raw`C:\repo\Main.tsx:12`)).toBe(true) + }) + it('rejects emails and git URLs with a mid-token @', () => { expect(isFilePathCodeSpan('git@github.com:user/repo.git')).toBe(false) expect(isFilePathCodeSpan('user@host.com/path/file.txt')).toBe(false) }) }) +describe('splitFilePathLineSuffix', () => { + it('splits :line and :line:col suffixes', () => { + expect(splitFilePathLineSuffix('src/foo.ts:42')).toEqual({ + path: 'src/foo.ts', + line: 42, + column: null + }) + expect(splitFilePathLineSuffix('src/foo.ts:42:7')).toEqual({ + path: 'src/foo.ts', + line: 42, + column: 7 + }) + }) + + it('keeps Windows drive colons intact', () => { + expect(splitFilePathLineSuffix(String.raw`C:\repo\a.ts`)).toEqual({ + path: String.raw`C:\repo\a.ts`, + line: null, + column: null + }) + expect(splitFilePathLineSuffix(String.raw`C:\repo\a.ts:12`)).toEqual({ + path: String.raw`C:\repo\a.ts`, + line: 12, + column: null + }) + }) + + it('ignores non-numeric and zero suffixes', () => { + expect(splitFilePathLineSuffix('src/foo.ts')).toEqual({ + path: 'src/foo.ts', + line: null, + column: null + }) + expect(splitFilePathLineSuffix('src/foo.ts:0')).toEqual({ + path: 'src/foo.ts:0', + line: null, + column: null + }) + }) +}) + describe('normalizeFilePath', () => { it('strips a leading ./', () => { expect(normalizeFilePath('./a/b.ts')).toBe('a/b.ts') diff --git a/mobile/src/components/markdown-file-path-detection.ts b/mobile/src/components/markdown-file-path-detection.ts index a33c0dceae4..cf24b2c4b51 100644 --- a/mobile/src/components/markdown-file-path-detection.ts +++ b/mobile/src/components/markdown-file-path-detection.ts @@ -83,9 +83,11 @@ const FILE_EXTENSIONS = [ const EXTENSION_SET = new Set(FILE_EXTENSIONS) // Accept the host's native separator because transcript paths originate on the -// connected runtime, which may be Windows even when the phone is not. +// connected runtime, which may be Windows even when the phone is not. Leading +// alternatives cover Windows drives, UNC, and POSIX absolute roots; the optional +// tail captures agent-style :line(:col) citations. const CANDIDATE_PATTERN = - /(?:[A-Za-z]:[\\/]|\\\\)?(?:\.{1,2}[\\/])?(?:[\w.@~+-]+[\\/])+[\w.@+-]+\.[A-Za-z0-9]+/g + /(?:[A-Za-z]:[\\/]|\\\\|[\\/])?(?:\.{1,2}[\\/])?(?:[\w.@~+-]+[\\/])+[\w.@+-]+\.[A-Za-z0-9]+(?::[1-9]\d*(?::[1-9]\d*)?)?/g // A path candidate in chat prose is short; a much longer run can't hold one worth // linkifying but can push CANDIDATE_PATTERN into super-linear backtracking, so we @@ -99,7 +101,31 @@ function hasMidTokenAt(candidate: string): boolean { return /[^\\/]@/.test(candidate) } -function isOpenablePath(candidate: string): boolean { +const LINE_SUFFIX_PATTERN = /^(.+?):([1-9]\d*)(?::([1-9]\d*))?$/ + +/** + * Split an agent-style `path:line(:col)` citation into its parts. Windows drive + * colons are safe: only a trailing all-digit suffix is treated as a line ref. + */ +export function splitFilePathLineSuffix(pathText: string): { + path: string + line: number | null + column: number | null +} { + const match = LINE_SUFFIX_PATTERN.exec(pathText) + if (!match) { + return { path: pathText, line: null, column: null } + } + return { + path: match[1]!, + line: Number.parseInt(match[2]!, 10), + column: match[3] ? Number.parseInt(match[3], 10) : null + } +} + +function isOpenablePath(pathText: string): boolean { + // A :line(:col) tail is part of the citation, not the file name. + const { path: candidate } = splitFilePathLineSuffix(pathText) // Reject anything URL-ish or scheme-bearing — those are handled as web links. if (candidate.includes('://') || hasMidTokenAt(candidate)) { return false @@ -153,10 +179,11 @@ export function detectFilePathSegments(text: string): FilePathSegment[] { while ((match = CANDIDATE_PATTERN.exec(text))) { const candidate = match[0] - // Skip candidates that are part of a URL (preceded by a scheme colon or an - // alphanumeric/host char that would make this a domain tail, not a path). + // Skip candidates that are part of a URL: a scheme colon, a domain tail, or + // a preceding slash (the leading slash of an absolute path is part of the + // match itself, so prev '/' means a '://' or '//' remainder, not a path). const prev = match.index > 0 ? text[match.index - 1]! : '' - if (prev === ':' || prev === '/' || /[\w.@]/.test(prev)) { + if (prev === ':' || prev === '/' || prev === '\\' || /[\w.@]/.test(prev)) { continue } if (!isOpenablePath(candidate)) { @@ -195,16 +222,18 @@ export function isFilePathCodeSpan(code: string): boolean { if (isOpenablePath(trimmed)) { return true } - // Separator-less code span: accept a clean name.ext with a known extension. - if (/[\\/]/.test(trimmed)) { + // Separator-less code span: accept a clean name.ext (with an optional + // :line(:col) citation tail) and a known extension. + const { path } = splitFilePathLineSuffix(trimmed) + if (/[\\/]/.test(path)) { return false } - const dot = trimmed.lastIndexOf('.') + const dot = path.lastIndexOf('.') if (dot <= 0) { return false } - const name = trimmed.slice(0, dot) - const ext = trimmed.slice(dot + 1).toLowerCase() + const name = path.slice(0, dot) + const ext = path.slice(dot + 1).toLowerCase() if (/[^\w.@+-]/.test(name)) { return false } diff --git a/mobile/src/components/markdown-href-routing.test.ts b/mobile/src/components/markdown-href-routing.test.ts new file mode 100644 index 00000000000..7117a2ecbc8 --- /dev/null +++ b/mobile/src/components/markdown-href-routing.test.ts @@ -0,0 +1,79 @@ +import { describe, expect, it } from 'vitest' +import { routeMarkdownHref } from './markdown-href-routing' + +describe('routeMarkdownHref', () => { + it('routes web and mail links to the system handler', () => { + expect(routeMarkdownHref('https://example.com/docs')).toEqual({ + kind: 'web', + url: 'https://example.com/docs' + }) + expect(routeMarkdownHref('http://localhost:3000/')).toEqual({ + kind: 'web', + url: 'http://localhost:3000/' + }) + expect(routeMarkdownHref(' mailto:dev@example.com ')).toEqual({ + kind: 'web', + url: 'mailto:dev@example.com' + }) + }) + + it('routes relative hrefs to the file opener', () => { + expect(routeMarkdownHref('src/foo.ts')).toEqual({ kind: 'file', pathText: 'src/foo.ts' }) + expect(routeMarkdownHref('./docs/plan.md')).toEqual({ + kind: 'file', + pathText: './docs/plan.md' + }) + }) + + it('carries a #L fragment as a :line suffix', () => { + expect(routeMarkdownHref('docs/plan.md#L42')).toEqual({ + kind: 'file', + pathText: 'docs/plan.md:42' + }) + expect(routeMarkdownHref('docs/plan.md?plain=1#line-7')).toEqual({ + kind: 'file', + pathText: 'docs/plan.md:7' + }) + expect(routeMarkdownHref('docs/plan.md#usage')).toEqual({ + kind: 'file', + pathText: 'docs/plan.md' + }) + }) + + it('decodes percent-encoded href paths', () => { + expect(routeMarkdownHref('docs/release%20notes.md')).toEqual({ + kind: 'file', + pathText: 'docs/release notes.md' + }) + }) + + it('routes file: URIs to the file opener', () => { + expect(routeMarkdownHref('file:///Users/me/wt/src/app.tsx')).toEqual({ + kind: 'file', + pathText: '/Users/me/wt/src/app.tsx' + }) + expect(routeMarkdownHref('file:///Users/me/wt/src/app.tsx#L12')).toEqual({ + kind: 'file', + pathText: '/Users/me/wt/src/app.tsx:12' + }) + expect(routeMarkdownHref('file:///C:/repo/src/index.ts')).toEqual({ + kind: 'file', + pathText: 'C:/repo/src/index.ts' + }) + }) + + it('keeps Windows drive paths out of the scheme filter', () => { + expect(routeMarkdownHref(String.raw`C:\repo\src\index.ts`)).toEqual({ + kind: 'file', + pathText: String.raw`C:\repo\src\index.ts` + }) + }) + + it('drops anchors, unknown schemes, and empty hrefs', () => { + expect(routeMarkdownHref('#section')).toEqual({ kind: 'none' }) + expect(routeMarkdownHref('')).toEqual({ kind: 'none' }) + expect(routeMarkdownHref('editor://file/x.ts')).toEqual({ kind: 'none' }) + expect(routeMarkdownHref('javascript:alert(1)')).toEqual({ kind: 'none' }) + expect(routeMarkdownHref('data:text/plain,hi')).toEqual({ kind: 'none' }) + }) +}) diff --git a/mobile/src/components/markdown-href-routing.ts b/mobile/src/components/markdown-href-routing.ts new file mode 100644 index 00000000000..a8384bda78e --- /dev/null +++ b/mobile/src/components/markdown-href-routing.ts @@ -0,0 +1,91 @@ +// Decides what a tapped markdown href should do on mobile: open the system +// browser/mail client, open a file in the worktree, or nothing. Mirrors the +// desktop native-chat contract where hrefs are explicit agent-authored links: +// file: URIs and scheme-less (relative or absolute) hrefs are file targets. +import { fileUriToFilesystemPath } from '../../../src/shared/file-uri-path' +import { isWindowsAbsolutePathLike } from '../../../src/shared/cross-platform-path' + +export type MarkdownHrefRoute = + | { kind: 'web'; url: string } + | { kind: 'file'; pathText: string } + | { kind: 'none' } + +const WEB_SCHEME_PATTERN = /^(?:https?|mailto):/i +const SCHEME_PATTERN = /^[A-Za-z][A-Za-z0-9+.-]*:/ + +function parseLineFragment(hash: string): number | null { + if (!hash) { + return null + } + let decoded = hash + try { + decoded = decodeURIComponent(hash) + } catch { + // Keep the raw fragment when decoding fails. + } + const match = /^(?:L|line-?)([1-9]\d*)\b/i.exec(decoded) + return match ? Number.parseInt(match[1]!, 10) : null +} + +function stripQueryAndHash(value: string): { pathText: string; line: number | null } { + const hashIndex = value.indexOf('#') + const queryIndex = value.indexOf('?') + const suffixIndex = + hashIndex === -1 ? queryIndex : queryIndex === -1 ? hashIndex : Math.min(hashIndex, queryIndex) + const pathText = suffixIndex === -1 ? value : value.slice(0, suffixIndex) + const hash = + hashIndex === -1 + ? '' + : value.slice(hashIndex + 1, queryIndex > hashIndex ? queryIndex : undefined) + return { pathText, line: parseLineFragment(hash) } +} + +function maybeDecodeHrefPath(value: string): string { + try { + return decodeURIComponent(value) + } catch { + return value + } +} + +// The open flow parses a trailing :line back out via splitFilePathLineSuffix. +function withLineSuffix(pathText: string, line: number | null): string { + return line === null ? pathText : `${pathText}:${line}` +} + +export function routeMarkdownHref(href: string): MarkdownHrefRoute { + const trimmed = href.trim() + if (!trimmed || trimmed.startsWith('#')) { + return { kind: 'none' } + } + if (WEB_SCHEME_PATTERN.test(trimmed)) { + return { kind: 'web', url: trimmed } + } + if (/^file:/i.test(trimmed)) { + let url: URL + try { + url = new URL(trimmed) + } catch { + return { kind: 'none' } + } + const filePath = fileUriToFilesystemPath(url) + if (!filePath) { + return { kind: 'none' } + } + return { + kind: 'file', + pathText: withLineSuffix(filePath, parseLineFragment(url.hash.slice(1))) + } + } + // Other schemes (editor:, javascript:, data:, …) are dropped; a Windows drive + // prefix only looks like a scheme. + if (!isWindowsAbsolutePathLike(trimmed) && SCHEME_PATTERN.test(trimmed)) { + return { kind: 'none' } + } + const { pathText, line } = stripQueryAndHash(trimmed) + const decoded = maybeDecodeHrefPath(pathText) + if (!decoded) { + return { kind: 'none' } + } + return { kind: 'file', pathText: withLineSuffix(decoded, line) } +} diff --git a/mobile/src/components/markdown-inline-token-rules.test.ts b/mobile/src/components/markdown-inline-token-rules.test.ts new file mode 100644 index 00000000000..c9c59c1a1cc --- /dev/null +++ b/mobile/src/components/markdown-inline-token-rules.test.ts @@ -0,0 +1,62 @@ +import { describe, expect, it } from 'vitest' +import { + isIntrawordUnderscoreToken, + trimAutolinkTrailingPunctuation +} from './markdown-inline-token-rules' + +describe('isIntrawordUnderscoreToken', () => { + it('rejects snake_case emphasis spans', () => { + const text = 'src/foo_bar.ts and src/baz_qux.ts' + const index = text.indexOf('_') + const token = text.slice(index, text.lastIndexOf('_') + 1) + expect(isIntrawordUnderscoreToken(text, index, token)).toBe(true) + }) + + it('keeps standalone emphasis', () => { + expect(isIntrawordUnderscoreToken('say _hello_ now', 4, '_hello_')).toBe(false) + expect(isIntrawordUnderscoreToken('_hello_.', 0, '_hello_')).toBe(false) + }) + + it('rejects emphasis closed against a following word', () => { + expect(isIntrawordUnderscoreToken('_foo_s bar', 0, '_foo_')).toBe(true) + }) + + it('ignores non-underscore tokens', () => { + expect(isIntrawordUnderscoreToken('a*b*c', 1, '*b*')).toBe(false) + }) +}) + +describe('trimAutolinkTrailingPunctuation', () => { + it('splits sentence punctuation off the URL', () => { + expect(trimAutolinkTrailingPunctuation('https://x.com/a.')).toEqual({ + url: 'https://x.com/a', + trailing: '.' + }) + expect(trimAutolinkTrailingPunctuation('https://x.com/a,')).toEqual({ + url: 'https://x.com/a', + trailing: ',' + }) + expect(trimAutolinkTrailingPunctuation('https://x.com/a?!')).toEqual({ + url: 'https://x.com/a', + trailing: '?!' + }) + }) + + it('keeps balanced parens and strips unbalanced ones', () => { + expect(trimAutolinkTrailingPunctuation('https://x.com/a_(b)')).toEqual({ + url: 'https://x.com/a_(b)', + trailing: '' + }) + expect(trimAutolinkTrailingPunctuation('https://x.com/a).')).toEqual({ + url: 'https://x.com/a', + trailing: ').' + }) + }) + + it('leaves clean URLs untouched', () => { + expect(trimAutolinkTrailingPunctuation('https://x.com/a')).toEqual({ + url: 'https://x.com/a', + trailing: '' + }) + }) +}) diff --git a/mobile/src/components/markdown-inline-token-rules.ts b/mobile/src/components/markdown-inline-token-rules.ts new file mode 100644 index 00000000000..5b1c04d6fa1 --- /dev/null +++ b/mobile/src/components/markdown-inline-token-rules.ts @@ -0,0 +1,42 @@ +// Post-checks for inline markdown tokens that a single-pass tokenizer regex +// cannot express on its own. + +/** + * True when a `_…_` / `__…__` token sits inside a word (snake_case, dunder + * tails). CommonMark treats intraword underscores as literal text; rejecting + * the token also keeps file paths like src/foo_bar.ts whole for detection. + */ +export function isIntrawordUnderscoreToken(text: string, index: number, token: string): boolean { + if (!token.startsWith('_')) { + return false + } + const prev = index > 0 ? text[index - 1]! : '' + const next = text[index + token.length] ?? '' + return /\w/.test(prev) || /\w/.test(next) +} + +/** + * Split sentence punctuation off an autolinked URL tail ("see https://x.com/a."), + * keeping a trailing ')' only when the URL itself opened a paren. + */ +export function trimAutolinkTrailingPunctuation(url: string): { url: string; trailing: string } { + let end = url.length + while (end > 0) { + const char = url[end - 1]! + if ('.,;:!?'.includes(char)) { + end-- + continue + } + if (char === ')') { + const body = url.slice(0, end) + const opens = body.split('(').length - 1 + const closes = body.split(')').length - 1 + if (closes > opens) { + end-- + continue + } + } + break + } + return { url: url.slice(0, end), trailing: url.slice(end) } +} diff --git a/mobile/src/session/MobileNativeChatOverlay.tsx b/mobile/src/session/MobileNativeChatOverlay.tsx index b8f9cd9277b..d2b8176d8b1 100644 --- a/mobile/src/session/MobileNativeChatOverlay.tsx +++ b/mobile/src/session/MobileNativeChatOverlay.tsx @@ -5,6 +5,9 @@ import type { MobileNativeChatController } from './use-mobile-native-chat-contro type Props = { controller: MobileNativeChatController + /** Opens a tapped file reference (worktree-relative or absolute, optional + * :line(:col) suffix) through the shared tap-to-open flow. */ + onOpenFile: (pathText: string) => void /** Native-chat image attachments: picking adds a composer chip, and sending * rides the pending images along with the message text (desktop parity). */ images: MobileNativeChatImageAttachments @@ -25,6 +28,7 @@ type Props = { * view toggles while the native surface owns the visible composer. */ export function MobileNativeChatOverlay({ controller, + onOpenFile, images, onMicPress, micActive, @@ -57,7 +61,7 @@ export function MobileNativeChatOverlay({ onAnswerQuestion={controller.handleNativeChatQuestionAnswer} permission={controller.nativeChatPermission} onRespondPermission={controller.handleNativeChatRespondPermission} - onOpenFile={controller.handleNativeChatOpenFile} + onOpenFile={onOpenFile} hasMore={session.hasMore} loadingEarlier={session.loadingEarlier} onLoadEarlier={session.loadEarlier} diff --git a/mobile/src/session/MobileNativeChatView.tsx b/mobile/src/session/MobileNativeChatView.tsx index f14efc1f25f..52120de8725 100644 --- a/mobile/src/session/MobileNativeChatView.tsx +++ b/mobile/src/session/MobileNativeChatView.tsx @@ -272,6 +272,9 @@ export function MobileNativeChatView({ keyExtractor={(item) => item.id} renderItem={renderItem} contentContainerStyle={styles.listContent} + // Let link/file taps land while the composer keyboard is up + // instead of being swallowed by the dismiss gesture. + keyboardShouldPersistTaps="handled" onScroll={onScroll} scrollEventThrottle={32} onContentSizeChange={() => { diff --git a/mobile/src/session/mobile-terminal-file-tap-open.test.ts b/mobile/src/session/mobile-file-tap-open.test.ts similarity index 81% rename from mobile/src/session/mobile-terminal-file-tap-open.test.ts rename to mobile/src/session/mobile-file-tap-open.test.ts index fec1b5b663c..e791a870852 100644 --- a/mobile/src/session/mobile-terminal-file-tap-open.test.ts +++ b/mobile/src/session/mobile-file-tap-open.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it, vi } from 'vitest' -import { openMobileTerminalFileTap } from './mobile-terminal-file-tap-open' +import { openMobileFileTap } from './mobile-file-tap-open' function ok(result: unknown) { return { ok: true, result, _meta: { runtimeId: 'runtime-1' } } @@ -22,7 +22,7 @@ function activeTerminalState(activated: boolean) { } } -describe('openMobileTerminalFileTap', () => { +describe('openMobileFileTap', () => { it('opens absolute terminal artifacts through the grant-backed preview route', async () => { const client = createClient([ ok({ @@ -42,7 +42,7 @@ describe('openMobileTerminalFileTap', () => { const pushPreviewRoute = vi.fn() const triggerOpenFeedback = vi.fn() - openMobileTerminalFileTap({ + openMobileFileTap({ client, hostId: 'host-1', worktreeId: 'wt-1', @@ -106,7 +106,7 @@ describe('openMobileTerminalFileTap', () => { const openedTab = { id: 'tab-2', relativePath: 'src/index.ts' } const switchSessionTab = vi.fn() - openMobileTerminalFileTap({ + openMobileFileTap({ client, hostId: 'host-1', worktreeId: 'wt-1', @@ -154,7 +154,7 @@ describe('openMobileTerminalFileTap', () => { const pushPreviewRoute = vi.fn() const triggerOpenFeedback = vi.fn() - openMobileTerminalFileTap({ + openMobileFileTap({ client, hostId: 'host-1', worktreeId: 'wt-1', @@ -208,7 +208,7 @@ describe('openMobileTerminalFileTap', () => { ]) const openBrowser = vi.fn() - openMobileTerminalFileTap({ + openMobileFileTap({ client, hostId: 'host-1', worktreeId: 'wt-1', @@ -249,7 +249,7 @@ describe('openMobileTerminalFileTap', () => { ok({ opened: true }) ]) - openMobileTerminalFileTap({ + openMobileFileTap({ client, hostId: 'host-1', worktreeId: 'wt-1', @@ -296,7 +296,7 @@ describe('openMobileTerminalFileTap', () => { ]) const openBrowser = vi.fn() - openMobileTerminalFileTap({ + openMobileFileTap({ client, hostId: 'host-1', worktreeId: 'wt-1', @@ -337,7 +337,7 @@ describe('openMobileTerminalFileTap', () => { let activeTerminalHandle: string | null = 'terminal-1' const pushPreviewRoute = vi.fn() - openMobileTerminalFileTap({ + openMobileFileTap({ client, hostId: 'host-1', worktreeId: 'wt-1', @@ -381,6 +381,95 @@ describe('openMobileTerminalFileTap', () => { expect(pushPreviewRoute).not.toHaveBeenCalled() }) + it('reports a failed files.open through onOpenFailed', async () => { + const client = createClient([ + ok({ + worktree: 'wt-1', + relativePath: 'src/index.ts', + absolutePath: '/repo/src/index.ts', + exists: true, + isDirectory: false, + openTarget: { + kind: 'worktree-file', + provider: 'local', + relativePath: 'src/index.ts', + absolutePath: '/repo/src/index.ts' + } + }), + { ok: false, error: { message: 'nope' } } + ]) + const onOpenFailed = vi.fn() + + openMobileFileTap({ + client, + hostId: 'host-1', + worktreeId: 'wt-1', + pathText: 'src/index.ts', + line: null, + column: null, + pushPreviewRoute: vi.fn(), + openBrowser: vi.fn(), + triggerOpenFeedback: vi.fn(), + fetchSessionTabs: vi.fn(), + getSessionTabs: () => [], + getActiveSessionTabId: () => null, + getActivationState: activeTerminalState, + switchSessionTab: vi.fn(), + scheduleDelayedAction: vi.fn(), + onOpenFailed + }) + await Promise.resolve() + await Promise.resolve() + await Promise.resolve() + + expect(onOpenFailed).toHaveBeenCalledTimes(1) + }) + + it('does not report a failure when the user left the source tab mid-resolve', async () => { + const client = createClient([ + ok({ + worktree: 'wt-1', + relativePath: 'src/index.ts', + absolutePath: '/repo/src/index.ts', + exists: true, + isDirectory: false, + openTarget: { + kind: 'worktree-file', + provider: 'local', + relativePath: 'src/index.ts', + absolutePath: '/repo/src/index.ts' + } + }) + ]) + const onOpenFailed = vi.fn() + + openMobileFileTap({ + client, + hostId: 'host-1', + worktreeId: 'wt-1', + pathText: 'src/index.ts', + line: null, + column: null, + pushPreviewRoute: vi.fn(), + openBrowser: vi.fn(), + triggerOpenFeedback: vi.fn(), + fetchSessionTabs: vi.fn(), + getSessionTabs: () => [], + getActiveSessionTabId: () => null, + getActivationState: (activated) => ({ + ...activeTerminalState(activated), + activeTerminalHandle: 'terminal-2' + }), + switchSessionTab: vi.fn(), + scheduleDelayedAction: vi.fn(), + onOpenFailed + }) + await Promise.resolve() + await Promise.resolve() + + expect(onOpenFailed).not.toHaveBeenCalled() + }) + it('does not activate a worktree file tab after a newer tap supersedes it', async () => { const client = createClient([ ok({ @@ -402,7 +491,7 @@ describe('openMobileTerminalFileTap', () => { const openedTab = { id: 'tab-2', relativePath: 'src/index.ts' } const switchSessionTab = vi.fn() - openMobileTerminalFileTap({ + openMobileFileTap({ client, hostId: 'host-1', worktreeId: 'wt-1', diff --git a/mobile/src/session/mobile-terminal-file-tap-open.ts b/mobile/src/session/mobile-file-tap-open.ts similarity index 84% rename from mobile/src/session/mobile-terminal-file-tap-open.ts rename to mobile/src/session/mobile-file-tap-open.ts index 31a5c892d75..1525302a197 100644 --- a/mobile/src/session/mobile-terminal-file-tap-open.ts +++ b/mobile/src/session/mobile-file-tap-open.ts @@ -6,12 +6,12 @@ import type { RpcClient } from '../transport/rpc-client' import type { RpcSuccess } from '../transport/types' import { shouldActivateOpenedMobileSessionTab } from './opened-mobile-session-tab' -type TerminalFileTapSessionTab = { +export type FileTapSessionTab = { id: string relativePath?: string } -type OpenMobileTerminalFileTapOptions = { +export type OpenMobileFileTapOptions = { client: Pick hostId: string worktreeId: string @@ -37,19 +37,23 @@ type OpenMobileTerminalFileTapOptions = { } switchSessionTab: (tab: T) => void scheduleDelayedAction: (callback: () => void, delayMs: number) => unknown + /** Invoked when the tap cannot open anything (resolve miss, directory, or a + * failed open). Omitted on surfaces that keep the historical silent miss. */ + onOpenFailed?: () => void } -export function openMobileTerminalFileTap( - options: OpenMobileTerminalFileTapOptions +export function openMobileFileTap( + options: OpenMobileFileTapOptions ): void { - void openMobileTerminalFileTapAsync(options).catch(() => { - // Terminal file taps are best-effort: a failed host resolution should leave - // terminal focus/input untouched, matching the existing silent miss behavior. + void openMobileFileTapAsync(options).catch(() => { + // File taps are best-effort: a failed host resolution should leave terminal + // focus/input untouched. Surfaces that want feedback pass onOpenFailed. + options.onOpenFailed?.() }) } -async function openMobileTerminalFileTapAsync( - options: OpenMobileTerminalFileTapOptions +async function openMobileFileTapAsync( + options: OpenMobileFileTapOptions ): Promise { const worktree = `id:${options.worktreeId}` const response = await options.client.sendRequest( @@ -65,12 +69,15 @@ async function openMobileTerminalFileTapAsync( - options: OpenMobileTerminalFileTapOptions, +function scheduleOpenedWorktreeTabActivation( + options: OpenMobileFileTapOptions, openedPath: string ): void { let activated = false diff --git a/mobile/src/session/mobile-native-chat-open-file.test.ts b/mobile/src/session/mobile-native-chat-open-file.test.ts index 2f8ceb936a7..c79878e92d1 100644 --- a/mobile/src/session/mobile-native-chat-open-file.test.ts +++ b/mobile/src/session/mobile-native-chat-open-file.test.ts @@ -1,93 +1,153 @@ import { describe, expect, it, vi } from 'vitest' -import type { RpcClient } from '../transport/rpc-client' -import { - openMobileNativeChatFile, - resolveMobileNativeChatWorktreePath -} from './mobile-native-chat-open-file' +import { openMobileNativeChatFileTap } from './mobile-native-chat-open-file' -describe('resolveMobileNativeChatWorktreePath', () => { - it('resolves an absolute tool path to a worktree-relative open target', async () => { - const sendRequest = vi.fn().mockResolvedValue({ - ok: true, - result: { - exists: true, - isDirectory: false, - openTarget: { kind: 'worktree-file', relativePath: 'src/app.ts' } - } - }) - await expect( - resolveMobileNativeChatWorktreePath({ - client: { sendRequest } as unknown as RpcClient, - worktreeId: 'worktree', - pathText: '/repo/src/app.ts', - terminal: 'terminal' - }) - ).resolves.toBe('src/app.ts') - expect(sendRequest).toHaveBeenCalledWith('files.resolveTerminalPath', { - worktree: 'id:worktree', - pathText: '/repo/src/app.ts', - terminal: 'terminal' - }) +function ok(result: unknown) { + return { ok: true, result, _meta: { runtimeId: 'runtime-1' } } +} + +function activationState(activated: boolean) { + return { + activated, + activationSeq: 1, + latestActivationSeq: 1, + sourceTerminalHandle: 'terminal-1', + activeTerminalHandle: 'terminal-1', + activeTabType: 'terminal' + } +} + +function baseOptions(client: { sendRequest: ReturnType }) { + return { + client, + hostId: 'host-1', + worktreeId: 'wt-1', + pushPreviewRoute: vi.fn(), + openBrowser: vi.fn(), + triggerOpenFeedback: vi.fn(), + fetchSessionTabs: vi.fn(), + getSessionTabs: () => [], + getActiveSessionTabId: () => null, + getActivationState: activationState, + switchSessionTab: vi.fn(), + scheduleDelayedAction: vi.fn(), + onOpenFailed: vi.fn() + } +} + +function worktreeFileResolution(relativePath: string) { + return ok({ + worktree: 'wt-1', + relativePath, + absolutePath: `/repo/${relativePath}`, + exists: true, + isDirectory: false, + openTarget: { + kind: 'worktree-file', + provider: 'local', + relativePath, + absolutePath: `/repo/${relativePath}` + } + }) +} + +describe('openMobileNativeChatFileTap', () => { + it('resolves against the worktree root: no terminal handle and no cwd', async () => { + const sendRequest = vi.fn(async () => worktreeFileResolution('src/app.ts')) + const options = baseOptions({ sendRequest }) + + openMobileNativeChatFileTap({ ...options, pathText: 'src/app.ts' }) + await Promise.resolve() + + expect(sendRequest).toHaveBeenCalledWith( + 'files.resolveTerminalPath', + { worktree: 'id:wt-1', pathText: 'src/app.ts' }, + { timeoutMs: 10_000 } + ) }) - it('opens only the resolved worktree-relative target', async () => { - const sendRequest = vi - .fn() - .mockResolvedValueOnce({ - ok: true, - result: { - exists: true, - isDirectory: false, - openTarget: { kind: 'worktree-file', relativePath: 'src/app.ts' } - } + it('parses a :line:col citation and opens the mobile preview route', async () => { + const sendRequest = vi.fn(async () => worktreeFileResolution('src/app.ts')) + const options = baseOptions({ sendRequest }) + + openMobileNativeChatFileTap({ ...options, pathText: 'src/app.ts:120:7' }) + await Promise.resolve() + + expect(sendRequest).toHaveBeenCalledWith( + 'files.resolveTerminalPath', + { worktree: 'id:wt-1', pathText: 'src/app.ts' }, + { timeoutMs: 10_000 } + ) + expect(options.triggerOpenFeedback).toHaveBeenCalledTimes(1) + expect(options.pushPreviewRoute).toHaveBeenCalledWith({ + pathname: '/h/[hostId]/files/preview/[worktreeId]', + params: expect.objectContaining({ + source: 'worktree', + relativePath: 'src/app.ts', + line: '120', + column: '7' }) - .mockResolvedValueOnce({ ok: true, result: {} }) - - await openMobileNativeChatFile({ - client: { sendRequest } as unknown as RpcClient, - worktreeId: 'worktree', - pathText: '../repo/src/app.ts', - terminal: 'terminal' - }) - - expect(sendRequest).toHaveBeenLastCalledWith('files.open', { - worktree: 'id:worktree', - relativePath: 'src/app.ts' }) + expect(options.onOpenFailed).not.toHaveBeenCalled() }) - it('resolves null when the resolve request rejects', async () => { - const sendRequest = vi.fn().mockRejectedValue(new Error('Request timed out')) - await expect( - resolveMobileNativeChatWorktreePath({ - client: { sendRequest } as unknown as RpcClient, - worktreeId: 'worktree', - pathText: 'src/app.ts', - terminal: null + it('surfaces a resolve miss instead of a silent no-op', async () => { + const sendRequest = vi.fn(async () => + ok({ + worktree: 'wt-1', + relativePath: null, + absolutePath: null, + exists: false, + isDirectory: false }) - ).resolves.toBeNull() + ) + const options = baseOptions({ sendRequest }) + + openMobileNativeChatFileTap({ ...options, pathText: 'gone/missing.ts' }) + await Promise.resolve() + await Promise.resolve() + + expect(options.onOpenFailed).toHaveBeenCalledTimes(1) + expect(options.pushPreviewRoute).not.toHaveBeenCalled() + expect(options.triggerOpenFeedback).not.toHaveBeenCalled() }) - it('does not reject when the open request fails', async () => { - const sendRequest = vi - .fn() - .mockResolvedValueOnce({ - ok: true, - result: { - exists: true, - isDirectory: false, - openTarget: { kind: 'worktree-file', relativePath: 'src/app.ts' } - } - }) - .mockRejectedValueOnce(new Error('connection interrupted')) + it('surfaces a rejected resolve request', async () => { + const sendRequest = vi.fn(async () => { + throw new Error('Request timed out') + }) + const options = baseOptions({ sendRequest }) - await expect( - openMobileNativeChatFile({ - client: { sendRequest } as unknown as RpcClient, - worktreeId: 'worktree', - pathText: 'src/app.ts', - terminal: null - }) - ).resolves.toBeUndefined() + openMobileNativeChatFileTap({ ...options, pathText: 'src/app.ts' }) + await Promise.resolve() + await Promise.resolve() + + expect(options.onOpenFailed).toHaveBeenCalledTimes(1) + }) + + it('opens a plain path through files.open with tab activation', async () => { + const responses: unknown[] = [worktreeFileResolution('src/app.ts'), ok({ opened: true })] + const sendRequest = vi.fn(async () => responses.shift()) + const openedTab = { id: 'tab-2', relativePath: 'src/app.ts' } + const switchSessionTab = vi.fn() + const options = { + ...baseOptions({ sendRequest }), + getSessionTabs: () => [openedTab], + getActiveSessionTabId: () => 'terminal-tab', + switchSessionTab, + scheduleDelayedAction: vi.fn((callback: () => void) => callback()) + } + + openMobileNativeChatFileTap({ ...options, pathText: 'src/app.ts' }) + await Promise.resolve() + await Promise.resolve() + await new Promise((resolve) => setTimeout(resolve, 0)) + + expect(sendRequest).toHaveBeenCalledWith( + 'files.open', + { worktree: 'id:wt-1', relativePath: 'src/app.ts' }, + { timeoutMs: 15_000 } + ) + expect(switchSessionTab).toHaveBeenCalledWith(openedTab) + expect(options.onOpenFailed).not.toHaveBeenCalled() }) }) diff --git a/mobile/src/session/mobile-native-chat-open-file.ts b/mobile/src/session/mobile-native-chat-open-file.ts index 44d6d23c066..3de6755b4b3 100644 --- a/mobile/src/session/mobile-native-chat-open-file.ts +++ b/mobile/src/session/mobile-native-chat-open-file.ts @@ -1,51 +1,30 @@ -import type { RuntimeTerminalPathResolution } from '../../../src/shared/runtime-types' -import type { RpcClient } from '../transport/rpc-client' +import { splitFilePathLineSuffix } from '../components/markdown-file-path-detection' +import { + openMobileFileTap, + type FileTapSessionTab, + type OpenMobileFileTapOptions +} from './mobile-file-tap-open' -export async function resolveMobileNativeChatWorktreePath(args: { - client: RpcClient - worktreeId: string - pathText: string - terminal: string | null -}): Promise { - try { - const response = await args.client.sendRequest('files.resolveTerminalPath', { - worktree: `id:${args.worktreeId}`, - pathText: args.pathText, - ...(args.terminal ? { terminal: args.terminal } : {}) - }) - if (!response.ok) { - return null - } - const resolved = response.result as RuntimeTerminalPathResolution - if (!resolved.exists || resolved.isDirectory) { - return null - } - return resolved.openTarget?.kind === 'worktree-file' - ? resolved.openTarget.relativePath - : (resolved.relativePath ?? null) - } catch { - // Callers fire-and-forget file opens; a disconnect/timeout must not become - // an unhandled rejection. - return null - } -} +export type OpenMobileNativeChatFileTapOptions = Omit< + OpenMobileFileTapOptions, + 'terminalHandle' | 'cwd' | 'line' | 'column' +> -export async function openMobileNativeChatFile(args: { - client: RpcClient - worktreeId: string - pathText: string - terminal: string | null -}): Promise { - const relativePath = await resolveMobileNativeChatWorktreePath(args) - if (relativePath) { - try { - await args.client.sendRequest('files.open', { - worktree: `id:${args.worktreeId}`, - relativePath - }) - } catch { - // Best-effort open; failures surface as a no-op rather than an - // unhandled rejection. - } - } +/** + * Open a file reference tapped in native chat: same haptic / preview-route / + * tab-activation flow as terminal taps, but chat paths are worktree-root + * relative (or absolute), so resolution deliberately passes no terminal handle + * and no cwd — a terminal's live cwd (e.g. `/mobile`) would misplace + * them. Agent-style `path:line(:col)` citations carry their location through. + */ +export function openMobileNativeChatFileTap( + options: OpenMobileNativeChatFileTapOptions +): void { + const { path, line, column } = splitFilePathLineSuffix(options.pathText) + openMobileFileTap({ + ...options, + pathText: path, + line, + column + }) } diff --git a/mobile/src/session/use-mobile-file-tap-handlers.test.ts b/mobile/src/session/use-mobile-file-tap-handlers.test.ts new file mode 100644 index 00000000000..03897cb8fab --- /dev/null +++ b/mobile/src/session/use-mobile-file-tap-handlers.test.ts @@ -0,0 +1,113 @@ +import { createElement } from 'react' +import { act, create, type ReactTestRenderer } from 'react-test-renderer' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { useMobileFileTapHandlers } from './use-mobile-file-tap-handlers' + +const push = vi.fn() + +vi.mock('expo-router', () => ({ useRouter: () => ({ push }) })) +vi.mock('../platform/haptics', () => ({ triggerSelection: vi.fn() })) + +type Handlers = ReturnType + +function ok(result: unknown) { + return { ok: true, result, _meta: { runtimeId: 'runtime-1' } } +} + +describe('useMobileFileTapHandlers', () => { + let renderer: ReactTestRenderer | null = null + let handlers: Handlers | null = null + + beforeEach(() => { + globalThis.IS_REACT_ACT_ENVIRONMENT = true + push.mockClear() + }) + + afterEach(() => { + act(() => renderer?.unmount()) + renderer = null + handlers = null + }) + + function createOptions(sendRequest: ReturnType) { + return { + client: { sendRequest }, + hostId: 'host-1', + worktreeId: 'wt-1', + worktreeName: 'Orca', + activeHandleRef: { current: 'terminal-1' as string | null }, + terminalCwdRef: { current: new Map([['terminal-1', '/repo/sub']]) }, + openBrowser: vi.fn(), + fetchSessionTabs: vi.fn(async () => {}), + getSessionTabs: () => [], + getActiveSessionTabId: () => null, + getActiveSessionTabType: () => 'terminal', + switchSessionTab: vi.fn(), + scheduleDelayedAction: vi.fn(), + showToast: vi.fn() + } + } + + function Harness({ options }: { options: ReturnType }): null { + handlers = useMobileFileTapHandlers(options) + return null + } + + it('keeps handler identities stable across rerenders', () => { + const options = createOptions(vi.fn()) + act(() => { + renderer = create(createElement(Harness, { options })) + }) + const first = handlers + act(() => { + renderer!.update(createElement(Harness, { options: { ...options } })) + }) + expect(handlers!.handleFileTap).toBe(first!.handleFileTap) + expect(handlers!.handleNativeChatFileTap).toBe(first!.handleNativeChatFileTap) + }) + + it('resolves terminal taps with the terminal handle and cwd', async () => { + const sendRequest = vi.fn(async () => ok({ exists: false, isDirectory: false })) + act(() => { + renderer = create(createElement(Harness, { options: createOptions(sendRequest) })) + }) + + handlers!.handleFileTap('terminal-1', 'index.ts', null, null) + await act(async () => {}) + + expect(sendRequest).toHaveBeenCalledWith( + 'files.resolveTerminalPath', + { worktree: 'id:wt-1', pathText: 'index.ts', terminal: 'terminal-1', cwd: '/repo/sub' }, + { timeoutMs: 10_000 } + ) + }) + + it('ignores terminal taps from a non-active handle', () => { + const sendRequest = vi.fn() + act(() => { + renderer = create(createElement(Harness, { options: createOptions(sendRequest) })) + }) + + handlers!.handleFileTap('terminal-2', 'index.ts', null, null) + + expect(sendRequest).not.toHaveBeenCalled() + }) + + it('resolves chat taps against the worktree root and toasts on a miss', async () => { + const sendRequest = vi.fn(async () => ok({ exists: false, isDirectory: false })) + const options = createOptions(sendRequest) + act(() => { + renderer = create(createElement(Harness, { options })) + }) + + handlers!.handleNativeChatFileTap('mobile/src/x.ts:12') + await act(async () => {}) + + expect(sendRequest).toHaveBeenCalledWith( + 'files.resolveTerminalPath', + { worktree: 'id:wt-1', pathText: 'mobile/src/x.ts' }, + { timeoutMs: 10_000 } + ) + expect(options.showToast).toHaveBeenCalledWith("Couldn't open mobile/src/x.ts:12") + }) +}) diff --git a/mobile/src/session/use-mobile-file-tap-handlers.ts b/mobile/src/session/use-mobile-file-tap-handlers.ts new file mode 100644 index 00000000000..2c1858b22ba --- /dev/null +++ b/mobile/src/session/use-mobile-file-tap-handlers.ts @@ -0,0 +1,124 @@ +import { useCallback, useRef, type MutableRefObject } from 'react' +import { useRouter } from 'expo-router' +import { triggerSelection } from '../platform/haptics' +import type { RpcClient } from '../transport/rpc-client' +import { openMobileFileTap, type FileTapSessionTab } from './mobile-file-tap-open' +import { openMobileNativeChatFileTap } from './mobile-native-chat-open-file' + +type MobileFileTapHandlerOptions = { + client: Pick | null + hostId: string + worktreeId: string + worktreeName?: string + activeHandleRef: MutableRefObject + terminalCwdRef: MutableRefObject> + openBrowser: (url: string) => void + fetchSessionTabs: () => Promise + getSessionTabs: () => readonly T[] + getActiveSessionTabId: () => string | null + getActiveSessionTabType: () => string | null + switchSessionTab: (tab: T) => void + scheduleDelayedAction: (callback: () => void, delayMs: number) => unknown + showToast: (message: string) => void +} + +/** + * Tap-to-open handlers for file references, shared by the terminal (link taps + * with the terminal's cwd) and native chat (worktree-root-relative paths, with + * failure toasts). Handlers are identity-stable and read the latest options at + * dispatch time; the shared activation seq lets a newer tap on either surface + * supersede an in-flight one. + */ +export function useMobileFileTapHandlers( + options: MobileFileTapHandlerOptions +): { + handleFileTap: ( + handle: string, + pathText: string, + line: number | null, + column: number | null + ) => void + handleNativeChatFileTap: (pathText: string) => void +} { + const router = useRouter() + const routerRef = useRef(router) + routerRef.current = router + const optionsRef = useRef(options) + optionsRef.current = options + const activationSeqRef = useRef(0) + + const handleFileTap = useCallback( + (handle: string, pathText: string, line: number | null, column: number | null) => { + const current = optionsRef.current + if (handle !== current.activeHandleRef.current || !current.client) { + return + } + const activationSeq = ++activationSeqRef.current + openMobileFileTap({ + client: current.client, + hostId: current.hostId, + worktreeId: current.worktreeId, + worktreeName: current.worktreeName, + terminalHandle: handle, + pathText, + cwd: current.terminalCwdRef.current.get(handle) ?? null, + line, + column, + pushPreviewRoute: (href) => routerRef.current.push(href), + openBrowser: current.openBrowser, + triggerOpenFeedback: triggerSelection, + fetchSessionTabs: current.fetchSessionTabs, + getSessionTabs: current.getSessionTabs, + getActiveSessionTabId: current.getActiveSessionTabId, + getActivationState: (activated) => ({ + activated, + activationSeq, + latestActivationSeq: activationSeqRef.current, + sourceTerminalHandle: handle, + activeTerminalHandle: current.activeHandleRef.current, + activeTabType: current.getActiveSessionTabType() + }), + switchSessionTab: current.switchSessionTab, + scheduleDelayedAction: current.scheduleDelayedAction + }) + }, + [] + ) + + const handleNativeChatFileTap = useCallback((pathText: string) => { + const current = optionsRef.current + // The chat overlay rides on its backing terminal tab; that handle anchors + // the activation gate even though resolution ignores the terminal's cwd. + const sourceTerminalHandle = current.activeHandleRef.current + if (!current.client || !sourceTerminalHandle) { + return + } + const activationSeq = ++activationSeqRef.current + openMobileNativeChatFileTap({ + client: current.client, + hostId: current.hostId, + worktreeId: current.worktreeId, + worktreeName: current.worktreeName, + pathText, + pushPreviewRoute: (href) => routerRef.current.push(href), + openBrowser: current.openBrowser, + triggerOpenFeedback: triggerSelection, + fetchSessionTabs: current.fetchSessionTabs, + getSessionTabs: current.getSessionTabs, + getActiveSessionTabId: current.getActiveSessionTabId, + getActivationState: (activated) => ({ + activated, + activationSeq, + latestActivationSeq: activationSeqRef.current, + sourceTerminalHandle, + activeTerminalHandle: current.activeHandleRef.current, + activeTabType: current.getActiveSessionTabType() + }), + switchSessionTab: current.switchSessionTab, + scheduleDelayedAction: current.scheduleDelayedAction, + onOpenFailed: () => current.showToast(`Couldn't open ${pathText}`) + }) + }, []) + + return { handleFileTap, handleNativeChatFileTap } +} diff --git a/mobile/src/session/use-mobile-native-chat-controller.ts b/mobile/src/session/use-mobile-native-chat-controller.ts index fd0a564b19c..9aeba791fab 100644 --- a/mobile/src/session/use-mobile-native-chat-controller.ts +++ b/mobile/src/session/use-mobile-native-chat-controller.ts @@ -1,10 +1,4 @@ -import { - useCallback, - useRef, - type Dispatch, - type MutableRefObject, - type SetStateAction -} from 'react' +import { useRef, type Dispatch, type MutableRefObject, type SetStateAction } from 'react' import { useMobileSessionViewMode } from './use-mobile-session-view-mode' import type { RpcClient } from '../transport/rpc-client' import type { ConnectionState } from '../transport/types' @@ -16,7 +10,6 @@ import { import { type MobileNativeChatTab, resolveMobileNativeChat } from './mobile-native-chat-eligibility' import { detectAgentPermission } from './mobile-native-chat-permission' import { parseAgentQuestion } from './mobile-native-chat-question' -import { openMobileNativeChatFile } from './mobile-native-chat-open-file' import { useMobileNativeChatPermissionSend } from './mobile-native-chat-permission-send' import type { MobileNativeChatSendOutcome } from './mobile-native-chat-send' import { useMobileNativeChatAnswerSend } from './use-mobile-native-chat-answer-send' @@ -52,7 +45,6 @@ export type MobileNativeChatController = { nativeChatPermission: ReturnType nativeChatQuestion: ReturnType nativeChatAsk: ReturnType - handleNativeChatOpenFile: (relativePath: string) => void handleNativeChatAnswerAsk: ( prompt: AskPrompt, selections: AskAnswerSelection[] @@ -176,21 +168,6 @@ export function useMobileNativeChatController(args: { messages: nativeChatSession.messages }) - const handleNativeChatOpenFile = useCallback( - (pathText: string) => { - if (!client) { - return - } - void openMobileNativeChatFile({ - client, - worktreeId, - pathText, - terminal: activeHandleRef.current - }) - }, - [activeHandleRef, client, worktreeId] - ) - // Every chat write gates on both: the lease proves the input floor is ours, and // `connState` collapses a render before the lease does on disconnect. const inputSendable = nativeChatInputLeaseReady && connState === 'connected' @@ -276,7 +253,6 @@ export function useMobileNativeChatController(args: { nativeChatPermission, nativeChatQuestion, nativeChatAsk, - handleNativeChatOpenFile, handleNativeChatAnswerAsk: answerAsk, handleNativeChatCancelAsk: cancelAsk, handleNativeChatRespondPermission: respond,