From f244deffee34870207bf2c910225c7fe17518867 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Sat, 25 Jul 2026 22:52:34 -0500 Subject: [PATCH 1/3] fix(tui,desktop): let authored link text outrank the fetched page title MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both chat renderers resolve link titles over the network and render them in place of the link's text, unconditionally — so they also replaced text the agent deliberately wrote. `[#71706](url)` rendered as the whole GitHub page title, and prose labels were swapped out mid-sentence. The TUI ordered `fetched || label` and always passed the URL to useLinkTitle. Desktop looked guarded but wasn't: chat markdown hands authored text to PrettyLink as `fallbackLabel`, not `label`, so `useLinkTitle(label ? null : target)` never skipped the fetch and `fetched || label || fallbackLabel` still let the title win. Treat an authored label as the intent: it wins, and it skips the fetch. A label that is just the URL still resolves, since `[url](url)` and `` are bare links wearing markdown syntax — desktop already applied that same URL-equality rule before handing the label over. Co-authored-by: Kinkoolino-Hermes <297364961+Kinkoolino-Hermes@users.noreply.github.com> --- apps/desktop/src/lib/external-link.tsx | 7 +++++-- ui-tui/src/components/markdown.tsx | 24 +++++++++++++----------- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/apps/desktop/src/lib/external-link.tsx b/apps/desktop/src/lib/external-link.tsx index ebdee577ac274..570106f44c3a6 100644 --- a/apps/desktop/src/lib/external-link.tsx +++ b/apps/desktop/src/lib/external-link.tsx @@ -251,10 +251,13 @@ interface PrettyLinkProps extends Omit, '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 ( diff --git a/ui-tui/src/components/markdown.tsx b/ui-tui/src/components/markdown.tsx index fb7fafd73c582..1c24a8d7bffa7 100644 --- a/ui-tui/src/components/markdown.tsx +++ b/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 `` +// 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 ( @@ -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 + return } export const stripInlineMarkup = (v: string) => From 17b8b3f657c606add863ede4227d79266f9e7e5f Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Sat, 25 Jul 2026 22:52:38 -0500 Subject: [PATCH 2/3] fix(tui,desktop): treat "not found" page titles as unusable Self-hosted forges answer a missing or private page with a 200 and a "Page not found" title, which then rendered as the link's text. Both surfaces keep their own copy of the error-title list, so extend both. Co-authored-by: Kinkoolino-Hermes <297364961+Kinkoolino-Hermes@users.noreply.github.com> --- apps/desktop/src/lib/external-link.tsx | 2 +- ui-tui/src/lib/externalLink.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/desktop/src/lib/external-link.tsx b/apps/desktop/src/lib/external-link.tsx index 570106f44c3a6..e9770d58f57be 100644 --- a/apps/desktop/src/lib/external-link.tsx +++ b/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() diff --git a/ui-tui/src/lib/externalLink.ts b/ui-tui/src/lib/externalLink.ts index f0256f5be1581..06e17a948cfbd 100644 --- a/ui-tui/src/lib/externalLink.ts +++ b/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 From 5c5f11d23a02915fa2411144c6f73fa305073136 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Sat, 25 Jul 2026 22:52:38 -0500 Subject: [PATCH 3/3] test(tui,desktop): cover authored link labels and not-found titles MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Warm the shared title cache before rendering so a resolved title is available synchronously — the previous TUI assertion only proved a label survived when no title had resolved, which passed on the buggy ordering. Asserts the bug class on both surfaces: an authored label outranks a resolved title and suppresses the fetch, an unlabeled link still resolves one, and a "Page not found" title is discarded rather than rendered. Co-authored-by: Kinkoolino-Hermes <297364961+Kinkoolino-Hermes@users.noreply.github.com> --- apps/desktop/src/lib/external-link.test.tsx | 43 ++++++++++++++++ ui-tui/src/__tests__/markdown.test.ts | 54 ++++++++++++++++++--- 2 files changed, 90 insertions(+), 7 deletions(-) diff --git a/apps/desktop/src/lib/external-link.test.tsx b/apps/desktop/src/lib/external-link.test.tsx index 5001f9c479ac3..772105648017d 100644 --- a/apps/desktop/src/lib/external-link.test.tsx +++ b/apps/desktop/src/lib/external-link.test.tsx @@ -23,6 +23,16 @@ function installDesktopBridge(partial: Partial = {}) { } 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() + + 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() + + 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() diff --git a/ui-tui/src/__tests__/markdown.test.ts b/ui-tui/src/__tests__/markdown.test.ts index 0c2b2c5d28e15..386a0e138eecc 100644 --- a/ui-tui/src/__tests__/markdown.test.ts +++ b/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(`${title}`, { + 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') }) })