From 9ee359550bb32911ba34657a6a613eec2c5fe874 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 4 Aug 2026 15:46:08 -0700 Subject: [PATCH] fix(mobile): make native-chat file links and path citations tappable (STA-3331) (#12364) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 * fix(mobile): harden chat file tap handling * refactor(chat): share native chat href routing * fix(mobile): detect files directly under path roots * fix(mobile): keep inline tokens and dunder paths intact around emphasis Review follow-ups on the chat file-link work: - A rejected intraword `_` token left the scan index past its closing underscore, so every inline token between two snake_case words was swallowed and rendered as literal source — including markdown links, which became untappable. Rescan from just past the opening delimiter. - Treat a path separator as an intraword flank so `src/__init__.py` and `a/__tests__/x.ts` stay whole; previously they rendered as bold plus a remnant that the new absolute-root pattern turned into a tap on `/x.ts`. - Bound the `:line(:col)` tail so `src/app.ts:1e3` and `:80%` no longer parse a line number, while a cited range still opens its first line. - Route chat tap failures through the composer banner (toast fallback): chat taps happen with the keyboard up, which covers the toast. - Drop the tap-handler mirror's dep list; the call site rebuilds its accessors every render, so it could never skip on a route that rerenders per keystroke. * Revert "fix(mobile): keep inline tokens and dunder paths intact around emphasis" This reverts commit 308bfaf22b88dafc5c43c6a2b8fb8b73c40e2972. * fix(mobile): preserve chat file-link parsing and feedback --- .../app/h/[hostId]/session/[worktreeId].tsx | 58 ++--- .../MobileMarkdown.file-links.test.ts | 133 +++++++++++ mobile/src/components/MobileMarkdown.tsx | 74 ++++-- .../markdown-file-path-detection.test.ts | 124 +++++++++- .../markdown-file-path-detection.ts | 51 +++- .../components/markdown-href-routing.test.ts | 79 +++++++ .../src/components/markdown-href-routing.ts | 18 ++ .../markdown-inline-token-rules.test.ts | 73 ++++++ .../components/markdown-inline-token-rules.ts | 55 +++++ .../src/session/MobileNativeChatOverlay.tsx | 6 +- mobile/src/session/MobileNativeChatView.tsx | 3 + ...n.test.ts => mobile-file-tap-open.test.ts} | 196 +++++++++++++++- ...le-tap-open.ts => mobile-file-tap-open.ts} | 52 ++++- .../mobile-native-chat-open-file.test.ts | 219 +++++++++++------- .../session/mobile-native-chat-open-file.ts | 91 +++----- .../use-mobile-file-tap-handlers.test.ts | 134 +++++++++++ .../session/use-mobile-file-tap-handlers.ts | 174 ++++++++++++++ .../use-mobile-native-chat-controller.ts | 26 +-- .../native-chat/native-chat-file-link.ts | 82 +------ src/shared/native-chat-href-routing.test.ts | 57 +++++ src/shared/native-chat-href-routing.ts | 74 ++++++ 21 files changed, 1434 insertions(+), 345 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} (72%) rename mobile/src/session/{mobile-terminal-file-tap-open.ts => mobile-file-tap-open.ts} (78%) 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 create mode 100644 src/shared/native-chat-href-routing.test.ts create mode 100644 src/shared/native-chat-href-routing.ts diff --git a/mobile/app/h/[hostId]/session/[worktreeId].tsx b/mobile/app/h/[hostId]/session/[worktreeId].tsx index 4d1f53196..97355ee86 100644 --- a/mobile/app/h/[hostId]/session/[worktreeId].tsx +++ b/mobile/app/h/[hostId]/session/[worktreeId].tsx @@ -196,7 +196,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, @@ -3192,44 +3192,23 @@ 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. + 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, + reportChatTapFailure: nativeChatSendError.show + }) 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. @@ -4750,6 +4729,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' +})) +vi.mock('./pr-sidebar/MermaidDiagram', () => ({ MermaidDiagram: 'MermaidDiagram' })) + +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('keeps markdown links between snake_case paths tappable', () => { + pressByText( + render('Updated src/foo_bar.py; see [the PR](https://example.com/x) before src/baz_qux.py'), + 'the PR' + ) + expect(openURL).toHaveBeenCalledWith('https://example.com/x') + }) + + it('opens a dunder path as one link', () => { + pressByText(render('see a/__tests__/x.ts now'), 'a/__tests__/x.ts') + expect(onOpenFile).toHaveBeenCalledExactlyOnceWith('a/__tests__/x.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 dc58f5e5d..cc88c01e5 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 { isMobileMermaidLanguage } from './mobile-mermaid-language' import { parseMobileMarkdown } from './mobile-markdown-parser' import { MermaidDiagram } from './pr-sidebar/MermaidDiagram' @@ -17,10 +22,11 @@ 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 @@ -28,10 +34,16 @@ const MAX_TABLE_COLUMNS = 8 /** Prose base size — passed to MermaidDiagram fallback mono text. */ const MERMAID_BASE = 13 -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) } } @@ -40,7 +52,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 @@ -65,39 +77,54 @@ 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)) { + // Resume after the opener so real tokens inside the rejected span are still scanned. + pattern.lastIndex = match.index + 1 + 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)) { @@ -120,27 +147,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 } @@ -206,7 +232,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 f75da01ef..19e608ec4 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,69 @@ 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' } + ]) + expect(detectFilePathSegments('/root.ts')).toEqual([ + { type: 'file', value: '/root.ts', path: '/root.ts' } + ]) + }) + + it('detects files directly under explicit Windows and relative roots', () => { + expect(detectFilePathSegments(String.raw`C:\root.ts`)).toEqual([ + { type: 'file', value: String.raw`C:\root.ts`, path: String.raw`C:\root.ts` } + ]) + expect(detectFilePathSegments('./root.ts')).toEqual([ + { type: 'file', value: './root.ts', path: 'root.ts' } + ]) + expect(detectFilePathSegments('../root.ts')).toEqual([ + { type: 'file', value: '../root.ts', path: '../root.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 partially parse numeric-looking non-line tails', () => { + expect(detectFilePathSegments('log src/app.ts:1e3 oops')).toEqual([ + { type: 'text', value: 'log ' }, + { type: 'file', value: 'src/app.ts', path: 'src/app.ts' }, + { type: 'text', value: ':1e3 oops' } + ]) + expect(detectFilePathSegments('coverage src/app.ts:80% of lines')).toEqual([ + { type: 'text', value: 'coverage ' }, + { type: 'file', value: 'src/app.ts', path: 'src/app.ts' }, + { type: 'text', value: ':80% of lines' } + ]) + }) + 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 +138,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 +244,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 a33c0dcea..5e320a3f1 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 only bounded 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.@~+-]+[\\/])+)[\w.@+-]+\.[A-Za-z0-9]+(?::[1-9]\d*(?::[1-9]\d*)?(?![\w@%]))?/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 000000000..7117a2ecb --- /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 000000000..61531f937 --- /dev/null +++ b/mobile/src/components/markdown-href-routing.ts @@ -0,0 +1,18 @@ +import { routeNativeChatHref } from '../../../src/shared/native-chat-href-routing' + +export type MarkdownHrefRoute = + | { kind: 'web'; url: string } + | { kind: 'file'; pathText: string } + | { kind: 'none' } + +function withLineSuffix(pathText: string, line: number | null): string { + return line === null ? pathText : `${pathText}:${line}` +} + +export function routeMarkdownHref(href: string): MarkdownHrefRoute { + const route = routeNativeChatHref(href) + if (route.kind !== 'file') { + return route + } + return { kind: 'file', pathText: withLineSuffix(route.pathText, route.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 000000000..27d44a36d --- /dev/null +++ b/mobile/src/components/markdown-inline-token-rules.test.ts @@ -0,0 +1,73 @@ +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('rejects dunder emphasis inside a path', () => { + expect(isIntrawordUnderscoreToken('src/__init__.py', 4, '__init__')).toBe(true) + expect(isIntrawordUnderscoreToken(String.raw`src\__init__.py`, 4, '__init__')).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('handles long unmatched closing-parenthesis tails', () => { + const url = 'https://x.com/a_(b)' + const trailing = ')'.repeat(4096) + expect(trimAutolinkTrailingPunctuation(`${url}${trailing}`)).toEqual({ url, 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 000000000..1afa8477f --- /dev/null +++ b/mobile/src/components/markdown-inline-token-rules.ts @@ -0,0 +1,55 @@ +// Post-checks for inline markdown tokens that a single-pass tokenizer regex +// cannot express on its own. + +const INTRAWORD_FLANK_PATTERN = /[\w\\/]/ + +/** + * True when a `_…_` / `__…__` token sits inside a word (snake_case, dunder + * tails). CommonMark treats intraword underscores as literal text; path + * separators count as flanks so dunder path segments also stay whole. + */ +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 INTRAWORD_FLANK_PATTERN.test(prev) || INTRAWORD_FLANK_PATTERN.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 + let parenthesisCountsReady = false + let openParentheses = 0 + let closeParentheses = 0 + while (end > 0) { + const char = url[end - 1]! + if ('.,;:!?'.includes(char)) { + end-- + continue + } + if (char === ')') { + if (!parenthesisCountsReady) { + for (let index = 0; index < end; index++) { + if (url[index] === '(') { + openParentheses++ + } else if (url[index] === ')') { + closeParentheses++ + } + } + parenthesisCountsReady = true + } + if (closeParentheses > openParentheses) { + end-- + closeParentheses-- + 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 77c5ab72f..ba587bab6 100644 --- a/mobile/src/session/MobileNativeChatOverlay.tsx +++ b/mobile/src/session/MobileNativeChatOverlay.tsx @@ -8,6 +8,9 @@ import { useMobileNativeChatStreamingBubble } from './use-mobile-native-chat-str 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 @@ -30,6 +33,7 @@ type Props = { * the chat list below it does not. */ export function MobileNativeChatOverlay({ controller, + onOpenFile, images, onMicPress, micActive, @@ -70,7 +74,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 1a2ae71ca..f63947365 100644 --- a/mobile/src/session/MobileNativeChatView.tsx +++ b/mobile/src/session/MobileNativeChatView.tsx @@ -274,6 +274,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 72% rename from mobile/src/session/mobile-terminal-file-tap-open.test.ts rename to mobile/src/session/mobile-file-tap-open.test.ts index a958b9772..d2a324d2c 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', @@ -153,7 +153,7 @@ describe('openMobileTerminalFileTap', () => { ]) const pushPreviewRoute = vi.fn() - openMobileTerminalFileTap({ + openMobileFileTap({ client, hostId: 'host-1', worktreeId: 'wt-1', @@ -204,7 +204,7 @@ describe('openMobileTerminalFileTap', () => { const pushPreviewRoute = vi.fn() const triggerOpenFeedback = vi.fn() - openMobileTerminalFileTap({ + openMobileFileTap({ client, hostId: 'host-1', worktreeId: 'wt-1', @@ -258,7 +258,7 @@ describe('openMobileTerminalFileTap', () => { ]) const openBrowser = vi.fn() - openMobileTerminalFileTap({ + openMobileFileTap({ client, hostId: 'host-1', worktreeId: 'wt-1', @@ -299,7 +299,7 @@ describe('openMobileTerminalFileTap', () => { ok({ opened: true }) ]) - openMobileTerminalFileTap({ + openMobileFileTap({ client, hostId: 'host-1', worktreeId: 'wt-1', @@ -346,7 +346,7 @@ describe('openMobileTerminalFileTap', () => { ]) const openBrowser = vi.fn() - openMobileTerminalFileTap({ + openMobileFileTap({ client, hostId: 'host-1', worktreeId: 'wt-1', @@ -387,7 +387,7 @@ describe('openMobileTerminalFileTap', () => { let activeTerminalHandle: string | null = 'terminal-1' const pushPreviewRoute = vi.fn() - openMobileTerminalFileTap({ + openMobileFileTap({ client, hostId: 'host-1', worktreeId: 'wt-1', @@ -431,6 +431,180 @@ 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('reports an unsupported file when files.open declines it', async () => { + const client = createClient([ + ok({ + worktree: 'wt-1', + relativePath: 'dist/app.zip', + absolutePath: '/repo/dist/app.zip', + exists: true, + isDirectory: false, + openTarget: { + kind: 'worktree-file', + provider: 'local', + relativePath: 'dist/app.zip', + absolutePath: '/repo/dist/app.zip' + } + }), + ok({ worktree: 'wt-1', relativePath: 'dist/app.zip', kind: 'binary', opened: false }) + ]) + const onOpenFailed = vi.fn() + const scheduleDelayedAction = vi.fn() + + openMobileFileTap({ + client, + hostId: 'host-1', + worktreeId: 'wt-1', + pathText: 'dist/app.zip', + 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, + onOpenFailed + }) + await Promise.resolve() + await Promise.resolve() + await Promise.resolve() + + expect(onOpenFailed).toHaveBeenCalledTimes(1) + expect(scheduleDelayedAction).not.toHaveBeenCalled() + }) + + it('does not report a stale failure after a newer tap supersedes it', async () => { + const client = createClient([ + ok({ + worktree: 'wt-1', + relativePath: null, + absolutePath: null, + exists: false, + isDirectory: false + }) + ]) + const onOpenFailed = vi.fn() + + openMobileFileTap({ + client, + hostId: 'host-1', + worktreeId: 'wt-1', + pathText: 'gone/missing.ts', + line: null, + column: null, + pushPreviewRoute: vi.fn(), + openBrowser: vi.fn(), + triggerOpenFeedback: vi.fn(), + fetchSessionTabs: vi.fn(), + getSessionTabs: () => [], + getActiveSessionTabId: () => null, + getActivationState: (activated) => ({ + ...activeTerminalState(activated), + latestActivationSeq: 2 + }), + switchSessionTab: vi.fn(), + scheduleDelayedAction: vi.fn(), + onOpenFailed + }) + await Promise.resolve() + await Promise.resolve() + + expect(onOpenFailed).not.toHaveBeenCalled() + }) + + 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({ @@ -452,7 +626,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 78% rename from mobile/src/session/mobile-terminal-file-tap-open.ts rename to mobile/src/session/mobile-file-tap-open.ts index 8d6e681b0..00b98ce21 100644 --- a/mobile/src/session/mobile-terminal-file-tap-open.ts +++ b/mobile/src/session/mobile-file-tap-open.ts @@ -1,4 +1,7 @@ -import type { RuntimeTerminalPathResolution } from '../../../src/shared/runtime-types' +import type { + RuntimeFileOpenResult, + RuntimeTerminalPathResolution +} from '../../../src/shared/runtime-types' import { filesystemPathToFileUri } from '../../../src/shared/file-uri-path' import { createMobileFilePreviewHref } from '../files/mobile-file-preview-route' import { classifyMobileArtifact } from './mobile-artifact-kind' @@ -6,12 +9,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 +40,34 @@ 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. + reportOpenFailure(options) }) } -async function openMobileTerminalFileTapAsync( - options: OpenMobileTerminalFileTapOptions +function reportOpenFailure( + options: OpenMobileFileTapOptions +): void { + if ( + options.onOpenFailed && + shouldActivateOpenedMobileSessionTab(options.getActivationState(false)) + ) { + options.onOpenFailed() + } +} + +async function openMobileFileTapAsync( + options: OpenMobileFileTapOptions ): Promise { const worktree = `id:${options.worktreeId}` const response = await options.client.sendRequest( @@ -65,12 +83,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 e2bf001ff..c79878e92 100644 --- a/mobile/src/session/mobile-native-chat-open-file.test.ts +++ b/mobile/src/session/mobile-native-chat-open-file.test.ts @@ -1,94 +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: { - worktree: 'sibling-worktree', - 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:sibling-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 6460c8284..3de6755b4 100644 --- a/mobile/src/session/mobile-native-chat-open-file.ts +++ b/mobile/src/session/mobile-native-chat-open-file.ts @@ -1,67 +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' -type MobileNativeChatWorktreeTarget = { - worktreeId: string - relativePath: string -} +export type OpenMobileNativeChatFileTapOptions = Omit< + OpenMobileFileTapOptions, + 'terminalHandle' | 'cwd' | 'line' | 'column' +> -async function resolveMobileNativeChatWorktreeTarget(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 - } - const relativePath = - resolved.openTarget?.kind === 'worktree-file' - ? resolved.openTarget.relativePath - : (resolved.relativePath ?? null) - return relativePath - ? { worktreeId: resolved.worktree?.trim() || args.worktreeId, relativePath } - : null - } catch { - return null - } -} - -export async function resolveMobileNativeChatWorktreePath(args: { - client: RpcClient - worktreeId: string - pathText: string - terminal: string | null -}): Promise { - return (await resolveMobileNativeChatWorktreeTarget(args))?.relativePath ?? null -} - -export async function openMobileNativeChatFile(args: { - client: RpcClient - worktreeId: string - pathText: string - terminal: string | null -}): Promise { - const target = await resolveMobileNativeChatWorktreeTarget(args) - if (target) { - try { - await args.client.sendRequest('files.open', { - worktree: `id:${target.worktreeId}`, - relativePath: target.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 000000000..a795d8d9e --- /dev/null +++ b/mobile/src/session/use-mobile-file-tap-handlers.test.ts @@ -0,0 +1,134 @@ +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(), + reportChatTapFailure: 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('dispatches through the latest options after a rerender', () => { + const firstSendRequest = vi.fn() + const firstOptions = createOptions(firstSendRequest) + act(() => { + renderer = create(createElement(Harness, { options: firstOptions })) + }) + + const latestSendRequest = vi.fn(async () => ok({ exists: false, isDirectory: false })) + act(() => { + renderer!.update( + createElement(Harness, { + options: { ...firstOptions, client: { sendRequest: latestSendRequest } } + }) + ) + }) + handlers!.handleFileTap('terminal-1', 'index.ts', null, null) + + expect(firstSendRequest).not.toHaveBeenCalled() + expect(latestSendRequest).toHaveBeenCalledTimes(1) + }) + + 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 reports 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.reportChatTapFailure).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 000000000..276be5b56 --- /dev/null +++ b/mobile/src/session/use-mobile-file-tap-handlers.ts @@ -0,0 +1,174 @@ +import { useCallback, useLayoutEffect, 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 + reportChatTapFailure: (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 feedback). 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 { + activeHandleRef, + client, + fetchSessionTabs, + getActiveSessionTabId, + getActiveSessionTabType, + getSessionTabs, + hostId, + openBrowser, + scheduleDelayedAction, + reportChatTapFailure, + switchSessionTab, + terminalCwdRef, + worktreeId, + worktreeName + } = options + const router = useRouter() + const routerRef = useRef(router) + const optionsRef = useRef(options) + const activationSeqRef = useRef(0) + + useLayoutEffect(() => { + routerRef.current = router + optionsRef.current = { + activeHandleRef, + client, + fetchSessionTabs, + getActiveSessionTabId, + getActiveSessionTabType, + getSessionTabs, + hostId, + openBrowser, + scheduleDelayedAction, + reportChatTapFailure, + switchSessionTab, + terminalCwdRef, + worktreeId, + worktreeName + } + }, [ + activeHandleRef, + client, + fetchSessionTabs, + getActiveSessionTabId, + getActiveSessionTabType, + getSessionTabs, + hostId, + openBrowser, + router, + scheduleDelayedAction, + reportChatTapFailure, + switchSessionTab, + terminalCwdRef, + worktreeId, + worktreeName + ]) + + 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.reportChatTapFailure(`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 37f7b5399..ad8adce18 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' @@ -56,7 +49,6 @@ export type MobileNativeChatController = { nativeChatPermission: ReturnType nativeChatQuestion: ReturnType nativeChatAsk: ReturnType - handleNativeChatOpenFile: (relativePath: string) => void handleNativeChatAnswerAsk: ( prompt: AskPrompt, selections: AskAnswerSelection[] @@ -188,21 +180,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' @@ -290,7 +267,6 @@ export function useMobileNativeChatController(args: { nativeChatPermission, nativeChatQuestion, nativeChatAsk, - handleNativeChatOpenFile, handleNativeChatAnswerAsk: answerAsk, handleNativeChatCancelAsk: cancelAsk, handleNativeChatRespondPermission: respond, diff --git a/src/renderer/src/components/native-chat/native-chat-file-link.ts b/src/renderer/src/components/native-chat/native-chat-file-link.ts index f6531adaf..eee08ceae 100644 --- a/src/renderer/src/components/native-chat/native-chat-file-link.ts +++ b/src/renderer/src/components/native-chat/native-chat-file-link.ts @@ -1,5 +1,4 @@ -import { fileUriToFilesystemPath } from '../../../../shared/file-uri-path' -import { isWindowsAbsolutePathLike } from '../../../../shared/cross-platform-path' +import { routeNativeChatHref } from '../../../../shared/native-chat-href-routing' import type { Worktree } from '../../../../shared/types' import { parseExplicitFileLinkTarget, @@ -82,50 +81,6 @@ export function resolveNativeChatFileLinkContext( } } -function maybeDecodeHrefPath(value: string): string { - try { - return decodeURIComponent(value) - } catch { - return value - } -} - -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) - const line = parseLineFragment(hash) - return { pathText, line } -} - -function parseLineFragment(hash: string): number | null { - if (!hash) { - return null - } - let decoded = hash - try { - decoded = decodeURIComponent(hash) - } catch { - decoded = hash - } - const match = /^(?:L|line-?)([1-9]\d*)\b/i.exec(decoded) - return match ? Number.parseInt(match[1], 10) : null -} - -function hasNonFileUriProtocol(value: string): boolean { - if (isWindowsAbsolutePathLike(value)) { - return false - } - const match = /^[A-Za-z][A-Za-z0-9+.-]*:/.exec(value) - return Boolean(match && match[0].toLowerCase() !== 'file:') -} - function resolvePathText( pathText: string, fallbackLine: number | null, @@ -148,43 +103,16 @@ function resolvePathText( } } -function resolveFileUriLink( - href: string, - context: NativeChatFileLinkContext -): NativeChatResolvedFileLink | null { - let url: URL - try { - url = new URL(href) - } catch { - return null - } - if (url.protocol !== 'file:') { - return null - } - const filePath = fileUriToFilesystemPath(url) - if (!filePath) { - return null - } - return resolvePathText(filePath, parseLineFragment(url.hash.replace(/^#/, '')), context) -} - export function resolveNativeChatFileLink( href: string | undefined, context: NativeChatFileLinkContext | null ): NativeChatResolvedFileLink | null { - const rawHref = href?.trim() - if (!rawHref || rawHref.startsWith('#') || !context) { + if (!context) { return null } - - if (rawHref.toLowerCase().startsWith('file:')) { - return resolveFileUriLink(rawHref, context) - } - if (hasNonFileUriProtocol(rawHref)) { + const route = routeNativeChatHref(href) + if (route.kind !== 'file') { return null } - - const { pathText, line } = stripQueryAndHash(rawHref) - const decodedPathText = maybeDecodeHrefPath(pathText) - return resolvePathText(decodedPathText, line, context) + return resolvePathText(route.pathText, route.line, context) } diff --git a/src/shared/native-chat-href-routing.test.ts b/src/shared/native-chat-href-routing.test.ts new file mode 100644 index 000000000..1b7c276ff --- /dev/null +++ b/src/shared/native-chat-href-routing.test.ts @@ -0,0 +1,57 @@ +import { describe, expect, it } from 'vitest' +import { routeNativeChatHref } from './native-chat-href-routing' + +describe('routeNativeChatHref', () => { + it('classifies web and mail links', () => { + expect(routeNativeChatHref('https://example.com/docs')).toEqual({ + kind: 'web', + url: 'https://example.com/docs' + }) + expect(routeNativeChatHref(' mailto:dev@example.com ')).toEqual({ + kind: 'web', + url: 'mailto:dev@example.com' + }) + }) + + it('parses relative file hrefs and line fragments', () => { + expect(routeNativeChatHref('docs/plan.md?plain=1#line-7')).toEqual({ + kind: 'file', + pathText: 'docs/plan.md', + line: 7 + }) + expect(routeNativeChatHref('docs/plan.md#usage')).toEqual({ + kind: 'file', + pathText: 'docs/plan.md', + line: null + }) + }) + + it('decodes relative and file URI paths', () => { + expect(routeNativeChatHref('docs/release%20notes.md')).toEqual({ + kind: 'file', + pathText: 'docs/release notes.md', + line: null + }) + expect(routeNativeChatHref('file:///Users/me/wt/My%20File.tsx#L12')).toEqual({ + kind: 'file', + pathText: '/Users/me/wt/My File.tsx', + line: 12 + }) + }) + + it('keeps Windows drive paths out of the scheme filter', () => { + expect(routeNativeChatHref(String.raw`C:\repo\src\index.ts`)).toEqual({ + kind: 'file', + pathText: String.raw`C:\repo\src\index.ts`, + line: null + }) + }) + + it('drops anchors, unknown schemes, malformed file URIs, and empty hrefs', () => { + expect(routeNativeChatHref('#section')).toEqual({ kind: 'none' }) + expect(routeNativeChatHref(undefined)).toEqual({ kind: 'none' }) + expect(routeNativeChatHref('editor://file/x.ts')).toEqual({ kind: 'none' }) + expect(routeNativeChatHref('javascript:alert(1)')).toEqual({ kind: 'none' }) + expect(routeNativeChatHref('file:///tmp/%E0%A4%A.txt')).toEqual({ kind: 'none' }) + }) +}) diff --git a/src/shared/native-chat-href-routing.ts b/src/shared/native-chat-href-routing.ts new file mode 100644 index 000000000..005ab957f --- /dev/null +++ b/src/shared/native-chat-href-routing.ts @@ -0,0 +1,74 @@ +import { isWindowsAbsolutePathLike } from './cross-platform-path' +import { fileUriToFilesystemPath } from './file-uri-path' + +export type NativeChatHrefRoute = + | { kind: 'web'; url: string } + | { kind: 'file'; pathText: string; line: number | null } + | { 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 + } +} + +export function routeNativeChatHref(href: string | null | undefined): NativeChatHrefRoute { + 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 pathText = fileUriToFilesystemPath(url) + if (!pathText) { + return { kind: 'none' } + } + return { kind: 'file', pathText, line: parseLineFragment(url.hash.slice(1)) } + } + if (!isWindowsAbsolutePathLike(trimmed) && SCHEME_PATTERN.test(trimmed)) { + return { kind: 'none' } + } + const { pathText, line } = stripQueryAndHash(trimmed) + const decodedPathText = maybeDecodeHrefPath(pathText) + return decodedPathText ? { kind: 'file', pathText: decodedPathText, line } : { kind: 'none' } +}