mirror of
https://github.com/stablyai/orca.git
synced 2026-11-03 02:27:43 +00:00
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
This commit is contained in:
@@ -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<FileLinkPathExistence>(
|
||||
|
||||
@@ -68,8 +68,14 @@ export function createNativeChatFileLinkExistence(
|
||||
const inFlight = new Map<string, number>()
|
||||
/** Paths the host could not answer in this generation, with their pending retry. */
|
||||
const failures = new Map<string, { attempt: number; timer?: ReturnType<typeof setTimeout> }>()
|
||||
/** Subscribed messages per path, so a changed answer refreshes only them. */
|
||||
const holders = new Map<string, { target: FileLinkTarget; watchers: Set<WatcherState> }>()
|
||||
/** Subscribed messages per path; their answers stay live beyond the unobserved cache limit. */
|
||||
const holders = new Map<
|
||||
string,
|
||||
{ target: FileLinkTarget; watchers: Set<WatcherState>; 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<WatcherState>() }
|
||||
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)
|
||||
}
|
||||
|
||||
@@ -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(<Transcript markdown="I updated `deck.md` and 'notes.md'." />)
|
||||
it('opens whole-code bare names from the chat folder', () => {
|
||||
render(<Transcript markdown="Done. `decision-explainer.html` and `Makefile:7`." />)
|
||||
|
||||
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', () => {
|
||||
|
||||
@@ -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(
|
||||
<CommentMarkdown
|
||||
variant="document"
|
||||
content="Updated package.json and `notes.md`; look in src/components for the button, or `src/renderer`."
|
||||
content="Updated package.json; look in src/components for the button, or `src/renderer`."
|
||||
onLinkClick={vi.fn()}
|
||||
fileLinkExists={everyPathExists}
|
||||
/>
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user