From fb0994944fdddccff5a46d9686b9c2588f3790ca Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 15 Oct 2026 18:31:07 -0700 Subject: [PATCH] fix(native-chat): link a bare file name once the chat's folder has it (#28818) * fix(native-chat): underline a bare file name once the chat's folder has it An agent's reply naming a file it created in its own folder, e.g. `decision-explainer.html`, stayed plain inline code: chat required a slash before it would even ask the host whether the path exists. Every chat agent runs in one known folder (the host's launch directory, which chat already resolves its links against), so a bare name the terminal's bare-word detector accepts now goes through the same host existence check and links only when the file is there. Version-like names (v1.2) and spaced names are still never asked about. Fixes STA-10392 * test(native-chat): drop an href assertion the click test already covers * Link whole inline-code filenames in native chat * fix(native-chat): keep version strings in inline code plain Bare names with a dot now pass chat's existing inline-code shape rule, so v1.2.3 and 1.0 stay plain as on main. * refactor(native-chat): name the inline-code file rule for what it checks --- .../native-chat-file-link-existence.test.ts | 22 +++++++++++++ .../native-chat-file-link-existence.ts | 33 +++++++++++++++---- .../use-native-chat-file-link-click.test.tsx | 18 +++++++--- .../CommentMarkdown.link-click.test.tsx | 4 +-- ...comment-markdown-native-chat-file-links.ts | 21 +++++++++--- 5 files changed, 80 insertions(+), 18 deletions(-) diff --git a/src/renderer/src/components/native-chat/native-chat-file-link-existence.test.ts b/src/renderer/src/components/native-chat/native-chat-file-link-existence.test.ts index 0cee65f9cff..ea047663ee2 100644 --- a/src/renderer/src/components/native-chat/native-chat-file-link-existence.test.ts +++ b/src/renderer/src/components/native-chat/native-chat-file-link-existence.test.ts @@ -3,6 +3,7 @@ import type { ParsedTerminalFileLink } from '@/lib/terminal-links' import type * as FileLinkTargetModule from '@/components/terminal-pane/terminal-file-link-target' import type { FileLinkPathExistence } from '@/components/terminal-pane/terminal-file-link-target' import type * as FilePathMappingModule from '@/components/terminal-pane/terminal-file-path-mapping' +import { TERMINAL_PATH_EXISTS_CACHE_MAX_ENTRIES } from '@/components/terminal-pane/terminal-path-exists-cache' import { createNativeChatFileLinkExistence, UNVERIFIABLE_RETRY_DELAYS_MS @@ -237,6 +238,27 @@ describe('createNativeChatFileLinkExistence', () => { expect(other.watcher.getSnapshot().check(link('src/App.tsx'))).toBe(true) }) + it('keeps every answer a shown message needs, past the unobserved cache limit', async () => { + const { pathExists } = hostWith(() => true) + const existence = createNativeChatFileLinkExistence(host, pathExists) + const { watcher } = watching(existence) + const names = Array.from( + { length: TERMINAL_PATH_EXISTS_CACHE_MAX_ENTRIES + 100 }, + (_, index) => `file-${index}.md` + ) + const render = (): boolean[] => names.map((name) => watcher.getSnapshot().check(link(name))) + + render() + await settle() + expect(render().every(Boolean)).toBe(true) + await settle() + expect(pathExists).toHaveBeenCalledTimes(names.length) + + existence.recheck() + await settle() + expect(pathExists).toHaveBeenCalledTimes(names.length * 2) + }) + it('keeps a reply to an older question from overwriting a newer answer', async () => { const replies: ((exists: boolean) => void)[] = [] const pathExists = vi.fn( diff --git a/src/renderer/src/components/native-chat/native-chat-file-link-existence.ts b/src/renderer/src/components/native-chat/native-chat-file-link-existence.ts index fa37b9a30b0..a85dde5d2df 100644 --- a/src/renderer/src/components/native-chat/native-chat-file-link-existence.ts +++ b/src/renderer/src/components/native-chat/native-chat-file-link-existence.ts @@ -68,8 +68,14 @@ export function createNativeChatFileLinkExistence( const inFlight = new Map() /** Paths the host could not answer in this generation, with their pending retry. */ const failures = new Map }>() - /** Subscribed messages per path, so a changed answer refreshes only them. */ - const holders = new Map }>() + /** Subscribed messages per path; their answers stay live beyond the unobserved cache limit. */ + const holders = new Map< + string, + { target: FileLinkTarget; watchers: Set; answer?: Answer } + >() + + const answerFor = (key: string): Answer | undefined => + holders.get(key)?.answer ?? answers.get(key) const clearFailure = (key: string): void => { clearTimeout(failures.get(key)?.timer) @@ -80,7 +86,7 @@ export function createNativeChatFileLinkExistence( if (inFlight.get(key) === sentIn) { inFlight.delete(key) } - const previous = answers.get(key) + const previous = answerFor(key) // Why: a reply to an older question must not overwrite what a newer one found. if (previous && previous.generation > sentIn) { return @@ -88,7 +94,12 @@ export function createNativeChatFileLinkExistence( if (sentIn === generation) { clearFailure(key) } - writeTerminalPathExistsCache(answers, key, { exists, generation: sentIn }) + const held = holders.get(key) + if (held) { + held.answer = { exists, generation: sentIn } + } else { + writeTerminalPathExistsCache(answers, key, { exists, generation: sentIn }) + } if ((previous?.exists ?? false) !== exists) { for (const watcher of holders.get(key)?.watchers ?? []) { watcher.refresh() @@ -161,6 +172,11 @@ export function createNativeChatFileLinkExistence( const index = (watcher: WatcherState, target: FileLinkTarget): void => { const entry = holders.get(target.cacheKey) ?? { target, watchers: new Set() } + const cached = answers.get(target.cacheKey) + if (cached) { + entry.answer = cached + answers.delete(target.cacheKey) + } entry.watchers.add(watcher) holders.set(target.cacheKey, entry) } @@ -171,6 +187,9 @@ export function createNativeChatFileLinkExistence( entry?.watchers.delete(watcher) if (entry?.watchers.size === 0) { holders.delete(key) + if (entry.answer) { + writeTerminalPathExistsCache(answers, key, entry.answer) + } // Why: nothing shows this path any more, so its retries end here. clearFailure(key) } @@ -198,7 +217,7 @@ export function createNativeChatFileLinkExistence( return false } const key = target.cacheKey - const answer = readTerminalPathExistsCache(answers, key) + const answer = holders.get(key)?.answer ?? readTerminalPathExistsCache(answers, key) if (!answer && !mayAsk) { return false } @@ -214,14 +233,14 @@ export function createNativeChatFileLinkExistence( } const isOutdated = (key: string): boolean => - (answers.get(key)?.generation ?? -1) < generation && !failures.has(key) + (answerFor(key)?.generation ?? -1) < generation && !failures.has(key) // Why: an answer can land between a message's render and its subscribe; nothing would refresh it. const catchUp = (watcher: WatcherState): void => { let changed = false for (const [key, { target, shown }] of watcher.held) { index(watcher, target) - changed ||= (answers.get(key)?.exists ?? false) !== shown + changed ||= (answerFor(key)?.exists ?? false) !== shown if (isOutdated(key)) { send(target) } diff --git a/src/renderer/src/components/native-chat/use-native-chat-file-link-click.test.tsx b/src/renderer/src/components/native-chat/use-native-chat-file-link-click.test.tsx index d39b931d98b..1fa884fa660 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-file-link-click.test.tsx +++ b/src/renderer/src/components/native-chat/use-native-chat-file-link-click.test.tsx @@ -264,11 +264,21 @@ describe('tool and edit-card file activation', () => { }) describe('useNativeChatFileLinkClick', () => { - it('does not underline a bare file name the click could not open, even when it exists', () => { - render() + it('opens whole-code bare names from the chat folder', () => { + render() - expect(screen.queryByRole('link')).toBeNull() - expect(screen.getByText('deck.md').tagName).toBe('CODE') + expect(screen.getAllByRole('link').map((link) => link.textContent)).toEqual([ + 'decision-explainer.html', + 'Makefile:7' + ]) + clickLink('decision-explainer.html') + + expect(mocks.openDetectedFilePath).toHaveBeenCalledWith( + '/repo/decision-explainer.html', + null, + null, + expect.objectContaining({ worktreeId: 'wt-1' }) + ) }) it('reports a missing relative path instead of doing nothing', () => { diff --git a/src/renderer/src/components/sidebar/CommentMarkdown.link-click.test.tsx b/src/renderer/src/components/sidebar/CommentMarkdown.link-click.test.tsx index eafa7504a5f..5100d36bf42 100644 --- a/src/renderer/src/components/sidebar/CommentMarkdown.link-click.test.tsx +++ b/src/renderer/src/components/sidebar/CommentMarkdown.link-click.test.tsx @@ -359,7 +359,7 @@ describe('CommentMarkdown link click handler', () => { expect(example?.closest('a')).toBeNull() }) - it('does not underline a bare file name or a folder, even when it exists', () => { + it('does not underline a bare prose name or extensionless folder, even when it exists', () => { container = document.createElement('div') document.body.appendChild(container) root = createRoot(container) @@ -368,7 +368,7 @@ describe('CommentMarkdown link click handler', () => { root?.render( diff --git a/src/renderer/src/components/sidebar/comment-markdown-native-chat-file-links.ts b/src/renderer/src/components/sidebar/comment-markdown-native-chat-file-links.ts index 16a43773799..79067bb42b9 100644 --- a/src/renderer/src/components/sidebar/comment-markdown-native-chat-file-links.ts +++ b/src/renderer/src/components/sidebar/comment-markdown-native-chat-file-links.ts @@ -21,16 +21,19 @@ const ROOTED_PATH_PREFIX_PATTERN = /^(?:~[\\/]|\.{1,2}[\\/]|[\\/]|[A-Za-z]:[\\/] /** Whether a detected path names a file on the chat's host; the terminal's own check. */ export type FileLinkExists = (link: ParsedTerminalFileLink) => boolean -// Why: a link is underlined only when it names a path; a bare `name.md` resolves nowhere -// reliable, so underlining it promises a click that cannot open anything. -function isLinkifiableFile(link: ParsedTerminalFileLink, isProse: boolean): boolean { +function looksLikeFilePath(link: ParsedTerminalFileLink, isProse: boolean): boolean { const hasRootedPrefix = ROOTED_PATH_PREFIX_PATTERN.test(link.pathText) const hasLineSuffix = link.line !== null || link.column !== null const hasAlphabeticExtension = /\.[\p{L}][\p{L}\p{N}\p{M}_+-]*$/u.test(link.pathText) const hasPathExtension = /\.[\p{L}\p{N}][\p{L}\p{N}\p{M}_+-]*$/u.test(link.pathText) + return hasRootedPrefix || hasLineSuffix || (isProse ? hasPathExtension : hasAlphabeticExtension) +} + +// Why: prose still needs a path separator; inline code admits exact bare tokens separately. +function isLinkifiableFile(link: ParsedTerminalFileLink, isProse: boolean): boolean { return ( /[\\/]/.test(link.pathText) && - (hasRootedPrefix || hasLineSuffix || (isProse ? hasPathExtension : hasAlphabeticExtension)) && + looksLikeFilePath(link, isProse) && routeNativeChatHref(link.displayText).kind === 'file' ) } @@ -256,7 +259,15 @@ function inlineCodeFileLink(node: MarkdownNode, exists: FileLinkExists): Markdow if (!value) { return null } - const link = exactFileLink(value, exists) + // Why: dotted bare names take the slash-path rule, so `v1.2.3` or `glm-5.3` stay plain. + const bare = extractTerminalFileLinks(value).find( + (candidate) => + candidate.startIndex === 0 && + candidate.endIndex === value.length && + !/[\\/]/.test(candidate.pathText) && + (!candidate.pathText.includes('.') || looksLikeFilePath(candidate, false)) + ) + const link = bare ? (exists(bare) ? bare : null) : exactFileLink(value, exists) return link ? createFileLinkNode(link, node) : null }