Merge pull request #71716 from NousResearch/bb/tui-link-label

fix(tui,desktop): stop link-title resolution from overwriting authored link text

apps/desktop/src/lib/external-link.test.tsx

@@ -23,6 +23,16 @@ function installDesktopBridge(partial: Partial<Window['hermesDesktop']> = {}) {
   } as unknown as Window['hermesDesktop']
 }
 
+const FORGEJO_URL = 'https://forgejo.home.example/homelab/homelab-ops/issues/101'
+
+function installTitleBridge(title: string) {
+  const bridge = vi.fn().mockResolvedValue(title)
+
+  installDesktopBridge({ fetchLinkTitle: bridge as unknown as Window['hermesDesktop']['fetchLinkTitle'] })
+
+  return bridge
+}
+
 afterEach(() => {
   __resetLinkTitleCache()
   vi.restoreAllMocks()
@@ -155,6 +165,39 @@ describe('external link helpers', () => {
     })
   })
 
+  it('treats not-found fetched titles as unusable', async () => {
+    const bridge = installTitleBridge('Page not found - Forgejo')
+
+    await expect(fetchLinkTitle(FORGEJO_URL)).resolves.toBe('')
+    expect(bridge).toHaveBeenCalledTimes(1)
+  })
+
+  it('keeps an authored fallbackLabel ahead of a fetched title, and skips the fetch', async () => {
+    const bridge = installTitleBridge('Kinkolino Forgejo')
+
+    // Chat markdown passes authored link text as `fallbackLabel`, not `label`.
+    render(<PrettyLink fallbackLabel="FJ #101" href={FORGEJO_URL} />)
+
+    const link = screen.getByTitle(FORGEJO_URL)
+
+    await waitFor(() => {
+      expect(link.textContent).toContain('FJ #101')
+    })
+    expect(link.textContent).not.toContain('Kinkolino Forgejo')
+    expect(bridge).not.toHaveBeenCalled()
+  })
+
+  it('still resolves a title when no label was authored', async () => {
+    const bridge = installTitleBridge('Homelab Ops Issue 101')
+
+    render(<PrettyLink href={FORGEJO_URL} />)
+
+    await waitFor(() => {
+      expect(screen.getByTitle(FORGEJO_URL).textContent).toContain('Homelab Ops Issue 101')
+    })
+    expect(bridge).toHaveBeenCalledTimes(1)
+  })
+
   it('normalizes scheme-less links before opening', () => {
     installDesktopBridge()
 

apps/desktop/src/lib/external-link.tsx

@@ -23,7 +23,7 @@ const SKIP_PROTO_RE = /^(?:file|data|mailto|javascript|blob|chrome|about|hermes)
 const LOCAL_HOST_RE = /^(?:localhost|127\.0\.0\.1|0\.0\.0\.0|\[::1\])(?::\d+)?$/i
 
 const ERROR_TITLE_RE =
-  /\b(?:access denied|attention required|captcha|error|forbidden|just a moment|request blocked|too many requests)\b/i
+  /\b(?:access denied|attention required|captcha|error|forbidden|just a moment|not found|request blocked|too many requests)\b/i
 
 export function normalizeExternalUrl(value: string): string {
   const trimmed = value.trim()
@@ -251,10 +251,13 @@ interface PrettyLinkProps extends Omit<ComponentProps<'a'>, 'href' | 'target'> {
   fallbackLabel?: string
 }
 
+// Title resolution is a fallback, not an override. Both props carry authored
+// text — chat markdown passes `fallbackLabel` — so either one skips the fetch.
 export function PrettyLink({ className, fallbackLabel, href, label, ...rest }: PrettyLinkProps) {
   const target = useMemo(() => normalizeExternalUrl(href), [href])
-  const fetched = useLinkTitle(label ? null : target)
-  const display = fetched || label?.trim() || fallbackLabel?.trim() || urlSlugTitleLabel(target)
+  const authoredLabel = label?.trim() || fallbackLabel?.trim()
+  const fetched = useLinkTitle(authoredLabel ? null : target)
+  const display = authoredLabel || fetched || urlSlugTitleLabel(target)
 
   return (
     <ExternalLink className={cn('wrap-break-word', className)} href={target} title={target} {...rest}>

ui-tui/src/__tests__/markdown.test.ts

@@ -2,12 +2,34 @@ import { PassThrough } from 'stream'
 
 import { Box, renderSync } from '@hermes/ink'
 import React from 'react'
-import { describe, expect, it } from 'vitest'
+import { afterEach, describe, expect, it, vi } from 'vitest'
 
 import { AUDIO_DIRECTIVE_RE, INLINE_RE, Md, MEDIA_LINE_RE, stripInlineMarkup } from '../components/markdown.js'
+import { __resetLinkTitleCache, fetchLinkTitle } from '../lib/externalLink.js'
 import { stripAnsi } from '../lib/text.js'
 import { DEFAULT_THEME } from '../theme.js'
 
+afterEach(() => {
+  __resetLinkTitleCache()
+  vi.unstubAllGlobals()
+})
+
+// Stub the network and warm the shared title cache, so a subsequent render
+// has the resolved title available synchronously.
+const stubFetchedTitle = (url: string, title: string) => {
+  vi.stubGlobal(
+    'fetch',
+    vi.fn().mockResolvedValue(
+      new Response(`<html><head><title>${title}</title></head></html>`, {
+        headers: { 'content-type': 'text/html' },
+        status: 200
+      })
+    )
+  )
+
+  return fetchLinkTitle(url)
+}
+
 const matches = (text: string) => [...text.matchAll(INLINE_RE)].map(m => m[0])
 const BEL = String.fromCharCode(7)
 const ESC = String.fromCharCode(27)
@@ -266,19 +288,37 @@ describe('Md link labels', () => {
     expect(rendered).not.toContain('https://www.expedia.com/things-to-do/puerto-rico-el-yunque-rainforest-adventure')
   })
 
-  it('keeps explicit markdown labels as the immediate fallback', () => {
+  it('keeps the authored markdown label even when a page title resolves', async () => {
+    const url = 'https://www.expedia.com/things-to-do/puerto-rico-el-yunque-rainforest-adventure'
+
+    // Warm the shared cache so `useLinkTitle` would have a title to render
+    // synchronously — the label must still win.
+    await stubFetchedTitle(url, 'El Yunque Rainforest Adventure | Expedia')
+
     const lines = renderPlain(
       React.createElement(
         Box,
         { width: 80 },
-        React.createElement(Md, {
-          t: DEFAULT_THEME,
-          text: '[Trip details](https://www.expedia.com/things-to-do/puerto-rico-el-yunque-rainforest-adventure)'
-        })
+        React.createElement(Md, { t: DEFAULT_THEME, text: `[Trip details](${url})` })
       )
     )
 
-    expect(lines.join('\n')).toContain('Trip details')
+    const rendered = lines.join('\n')
+
+    expect(rendered).toContain('Trip details')
+    expect(rendered).not.toContain('El Yunque Rainforest Adventure | Expedia')
+  })
+
+  it('still resolves titles for links whose label is just the URL', async () => {
+    const url = 'https://www.expedia.com/things-to-do/puerto-rico-el-yunque-rainforest-adventure'
+
+    await stubFetchedTitle(url, 'Rainforest Adventure Tour')
+
+    const lines = renderPlain(
+      React.createElement(Box, { width: 120 }, React.createElement(Md, { t: DEFAULT_THEME, text: `[${url}](${url})` }))
+    )
+
+    expect(lines.join('\n')).toContain('Rainforest Adventure Tour')
   })
 })
 

ui-tui/src/components/markdown.tsx

@@ -153,25 +153,27 @@ const autolinkUrl = (raw: string) =>
 const defaultLinkLabel = (url: string) =>
   url.startsWith('mailto:') ? url.replace(/^mailto:/, '') : /^https?:\/\//i.test(url) ? urlSlugTitleLabel(url) : url
 
-const pickFallbackLabel = (label: string | undefined, target: string): string | undefined => {
+// A label only counts as authored if it says something the URL doesn't:
+// `[https://example.com](https://example.com)` and `<https://example.com>`
+// are bare links wearing markdown syntax, so they still want a page title.
+const pickAuthoredLabel = (label: string | undefined, target: string): string | undefined => {
   const trimmed = label?.trim()
 
-  if (!trimmed) {
-    return undefined
-  }
-
-  return normalizeExternalUrl(trimmed) === target ? undefined : trimmed
+  return trimmed && normalizeExternalUrl(trimmed) !== target ? trimmed : undefined
 }
 
 interface ResolvedLinkProps {
-  fallbackLabel?: string
+  authoredLabel?: string
   t: Theme
   url: string
 }
 
-function ResolvedLink({ fallbackLabel, t, url }: ResolvedLinkProps) {
-  const fetched = useLinkTitle(url)
-  const display = fetched || fallbackLabel || defaultLinkLabel(url)
+// Title resolution is a fallback for links with no text of their own, not an
+// override — replacing `[Read the RFC](url)` with the page title throws away
+// better wording than we can derive, and mangles labels like `#71706`.
+function ResolvedLink({ authoredLabel, t, url }: ResolvedLinkProps) {
+  const fetched = useLinkTitle(authoredLabel ? null : url)
+  const display = authoredLabel || fetched || defaultLinkLabel(url)
 
   return (
     <Link url={url}>
@@ -185,7 +187,7 @@ function ResolvedLink({ fallbackLabel, t, url }: ResolvedLinkProps) {
 const renderResolvedLink = (k: number, t: Theme, rawUrl: string, label?: string) => {
   const target = normalizeExternalUrl(rawUrl)
 
-  return <ResolvedLink fallbackLabel={pickFallbackLabel(label, target)} key={k} t={t} url={target} />
+  return <ResolvedLink authoredLabel={pickAuthoredLabel(label, target)} key={k} t={t} url={target} />
 }
 
 export const stripInlineMarkup = (v: string) =>

ui-tui/src/lib/externalLink.ts

@@ -15,7 +15,7 @@ const TITLE_USER_AGENT =
   'Mozilla/5.0 (Macintosh; Intel Mac OS X 14_6_0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/136.0.0.0 Safari/537.36'
 
 const TITLE_ERROR_RE =
-  /\b(?:access denied|attention required|captcha|error|forbidden|just a moment|request blocked|too many requests)\b/i
+  /\b(?:access denied|attention required|captcha|error|forbidden|just a moment|not found|request blocked|too many requests)\b/i
 
 const DOMAIN_RE = /^(?:www\.)?[a-z0-9](?:[a-z0-9-]*\.)+[a-z]{2,}(?::\d+)?(?:[/?#][^\s]*)?$/i
 const SKIP_PROTO_RE = /^(?:file|data|mailto|javascript|blob|chrome|about|hermes):/i