From e1c0f61b7698d8aadc412e93a14f3f72b1669e8c Mon Sep 17 00:00:00 2001 From: Ryan Bahan Date: Thu, 23 Apr 2026 15:31:55 -0600 Subject: [PATCH 1/3] preserve link strings --- .../private/node/ui/components/Alert.test.tsx | 27 ++++++++++ .../node/ui/components/FatalError.test.tsx | 20 +++++++ .../private/node/ui/components/FatalError.tsx | 6 ++- .../src/private/node/ui/components/Link.tsx | 9 ++-- .../node/ui/components/TokenizedText.test.tsx | 54 ++++++++++++++++++- .../node/ui/components/TokenizedText.tsx | 33 +++++++++++- 6 files changed, 141 insertions(+), 8 deletions(-) diff --git a/packages/cli-kit/src/private/node/ui/components/Alert.test.tsx b/packages/cli-kit/src/private/node/ui/components/Alert.test.tsx index 4c44ae594ee..8922a03325b 100644 --- a/packages/cli-kit/src/private/node/ui/components/Alert.test.tsx +++ b/packages/cli-kit/src/private/node/ui/components/Alert.test.tsx @@ -126,6 +126,33 @@ describe('Alert', async () => { `) }) + test('auto-detects URLs in a plain-string body and renders them as footnotes', async () => { + const options = { + body: 'See specification requirements: https://shopify.dev/docs/apps/build/sales-channels/channel-config-extension#specification-properties', + } + + const {lastFrame} = render() + const frame = unstyled(lastFrame()!) + + expect(frame).toMatchInlineSnapshot(` + "╭─ error ──────────────────────────────────────────────────────────────────────╮ + │ │ + │ See specification requirements: [1] │ + │ │ + ╰──────────────────────────────────────────────────────────────────────────────╯ + [1] + https://shopify.dev/docs/apps/build/sales-channels/channel-config-extension#specification-properties + " + `) + + const footnoteLines = frame.split('\n').filter((line) => line.includes('[1]')) + footnoteLines.forEach((line) => { + if (line.startsWith('[1]')) { + expect(line).not.toContain('│') + } + }) + }) + test('has the headline in bold', async () => { const options = { headline: 'Title.', diff --git a/packages/cli-kit/src/private/node/ui/components/FatalError.test.tsx b/packages/cli-kit/src/private/node/ui/components/FatalError.test.tsx index 51721c0a06d..c0ec4f3f0a6 100644 --- a/packages/cli-kit/src/private/node/ui/components/FatalError.test.tsx +++ b/packages/cli-kit/src/private/node/ui/components/FatalError.test.tsx @@ -23,6 +23,26 @@ describe('FatalError', async () => { `) }) + test('auto-detects URLs in a plain-string error message and renders them as footnotes', async () => { + const error = new AbortError( + 'See specification requirements: https://shopify.dev/docs/apps/build/sales-channels/channel-config-extension#specification-properties', + ) + + const {lastFrame} = render() + const frame = unstyled(lastFrame()!) + + expect(frame).toMatchInlineSnapshot(` + "╭─ error ──────────────────────────────────────────────────────────────────────╮ + │ │ + │ See specification requirements: [1] │ + │ │ + ╰──────────────────────────────────────────────────────────────────────────────╯ + [1] + https://shopify.dev/docs/apps/build/sales-channels/channel-config-extension#specification-properties + " + `) + }) + test('renders correctly with a formatted message', async () => { const error = new AbortError([ 'There has been an error creating your deployment:', diff --git a/packages/cli-kit/src/private/node/ui/components/FatalError.tsx b/packages/cli-kit/src/private/node/ui/components/FatalError.tsx index f8876fab6ea..7dbde0f2295 100644 --- a/packages/cli-kit/src/private/node/ui/components/FatalError.tsx +++ b/packages/cli-kit/src/private/node/ui/components/FatalError.tsx @@ -48,7 +48,11 @@ const FatalError: FunctionComponent = ({error}) => { ) : null} - {error.formattedMessage ? : {error.message}} + {error.formattedMessage ? ( + + ) : ( + + )} {error.tryMessage ? : null} diff --git a/packages/cli-kit/src/private/node/ui/components/Link.tsx b/packages/cli-kit/src/private/node/ui/components/Link.tsx index 0690dcbd133..0eb634b77fd 100644 --- a/packages/cli-kit/src/private/node/ui/components/Link.tsx +++ b/packages/cli-kit/src/private/node/ui/components/Link.tsx @@ -12,16 +12,15 @@ interface LinkProps { function link(label: string | undefined, url: string, linksContext: LinksContextValue | null) { if (!supportsHyperlinks.stdout) { - if (url === (label ?? url)) { - return url - } - if (linksContext === null) { + if (url === (label ?? url)) { + return url + } return label ? `${label} ${chalk.dim(`( ${url} )`)}` : url } const linkId = linksContext.addLink(label, url) - return `${label ?? url} [${linkId}]` + return label ? `${label} [${linkId}]` : `[${linkId}]` } return ansiEscapes.link(label ?? url, url) diff --git a/packages/cli-kit/src/private/node/ui/components/TokenizedText.test.tsx b/packages/cli-kit/src/private/node/ui/components/TokenizedText.test.tsx index 16fd76c7002..b9428886140 100644 --- a/packages/cli-kit/src/private/node/ui/components/TokenizedText.test.tsx +++ b/packages/cli-kit/src/private/node/ui/components/TokenizedText.test.tsx @@ -1,10 +1,13 @@ import {tokenItemToString, TokenizedText} from './TokenizedText.js' import {unstyled} from '../../../../public/node/output.js' import {render} from '../../testing/ui.js' -import {describe, expect, test} from 'vitest' +import {describe, expect, test, vi} from 'vitest' +import supportsHyperlinks from 'supports-hyperlinks' import React from 'react' +vi.mock('supports-hyperlinks') + describe('TokenizedText', async () => { test('renders arrays of items separated by spaces', async () => { const item = [ @@ -57,6 +60,55 @@ describe('TokenizedText', async () => { `) }) + describe('URL auto-detection in plain strings', async () => { + test('renders strings without URLs unchanged', async () => { + vi.mocked(supportsHyperlinks).stdout = false + + const {lastFrame} = render() + + expect(lastFrame()).toBe('no link here, just text') + }) + + test('preserves a URL intact when the terminal does not support hyperlinks', async () => { + vi.mocked(supportsHyperlinks).stdout = false + const url = 'https://shopify.dev/docs/apps/build/sales-channels/channel-config-extension#specification-properties' + + const {lastFrame} = render() + + expect(lastFrame()).toContain(url) + }) + + test('wraps detected URLs in OSC 8 escapes when the terminal supports hyperlinks', async () => { + vi.mocked(supportsHyperlinks).stdout = true + const url = 'https://example.com/docs' + + const {lastFrame} = render() + + expect(lastFrame()).toContain(`]8;;${url}${url}]8;;`) + }) + + test('detects multiple URLs in the same string', async () => { + vi.mocked(supportsHyperlinks).stdout = true + const first = 'https://example.com/a' + const second = 'https://example.com/b' + + const {lastFrame} = render() + + expect(lastFrame()).toContain(`]8;;${first}${first}]8;;`) + expect(lastFrame()).toContain(`]8;;${second}${second}]8;;`) + }) + + test('strips trailing sentence punctuation from detected URLs', async () => { + vi.mocked(supportsHyperlinks).stdout = true + const url = 'https://example.com/docs' + + const {lastFrame} = render() + + expect(lastFrame()).toContain(`]8;;${url}${url}]8;;`) + expect(lastFrame()).toContain('. Thanks') + }) + }) + describe('tokenItemToString', async () => { test("doesn't add a space before char", async () => { expect(tokenItemToString(['Run', {char: '!'}])).toBe('Run!') diff --git a/packages/cli-kit/src/private/node/ui/components/TokenizedText.tsx b/packages/cli-kit/src/private/node/ui/components/TokenizedText.tsx index 2c9905e01fc..95733ef8caf 100644 --- a/packages/cli-kit/src/private/node/ui/components/TokenizedText.tsx +++ b/packages/cli-kit/src/private/node/ui/components/TokenizedText.tsx @@ -152,9 +152,40 @@ interface TokenizedTextProps { * `TokenizedText` renders a text string with tokens that can be either strings, * links, and commands. */ +const URL_REGEX = /https?:\/\/\S+/g +const URL_TRAILING_PUNCTUATION = /[.,;:!?)\]}>'"]+$/ + +function renderStringWithLinks(str: string): JSX.Element { + const matches = Array.from(str.matchAll(URL_REGEX)) + if (matches.length === 0) { + return {str} + } + + const parts: JSX.Element[] = [] + let cursor = 0 + matches.forEach((match, index) => { + let url = match[0] + const trailing = url.match(URL_TRAILING_PUNCTUATION) + if (trailing) { + url = url.slice(0, url.length - trailing[0].length) + } + const start = match.index + const end = start + url.length + if (start > cursor) { + parts.push({str.slice(cursor, start)}) + } + parts.push() + cursor = end + }) + if (cursor < str.length) { + parts.push({str.slice(cursor)}) + } + return {parts} +} + const TokenizedText: FunctionComponent = ({item}) => { if (typeof item === 'string') { - return {item} + return renderStringWithLinks(item) } else if ('command' in item) { return } else if ('link' in item) { From 6b69c60fa770fcd66285a147ea73aa99c54fc260 Mon Sep 17 00:00:00 2001 From: Ryan Bahan Date: Tue, 28 Apr 2026 08:52:45 -0600 Subject: [PATCH 2/3] Address PR review: scope auto-linkify + tighten URL detection - Narrow URL_TRAILING_PUNCTUATION so legal URL chars `)`, `]`, `}`, `>` are no longer stripped (Wikipedia/MDN/Microsoft Learn URLs). - Gate renderStringWithLinks on a non-null LinksContext so non-Banner consumers (TabularData, prompts, InfoMessage) keep their original rendering unchanged. - Validate detected URLs via the WHATWG URL constructor and require the hostname to contain a dot. Rejects user typos and placeholder strings like https://asda and https://my-tunnel-url:port that should not appear as clickable hyperlinks (the case Isaac screenshotted). Tests: - TokenizedText: parens-in-URL, back-to-back URLs, no-context passthrough, no-dot hostname, non-numeric port. - Link: label-less `[N]` rendering and always-footnote registration inside a LinksContext. - FatalError: snapshot reproducing the AbortError tunnel-url case so the regression is locked in. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../node/ui/components/FatalError.test.tsx | 17 +++ .../private/node/ui/components/Link.test.tsx | 77 ++++++++++- .../node/ui/components/TokenizedText.test.tsx | 122 ++++++++++++++++-- .../node/ui/components/TokenizedText.tsx | 23 +++- 4 files changed, 227 insertions(+), 12 deletions(-) diff --git a/packages/cli-kit/src/private/node/ui/components/FatalError.test.tsx b/packages/cli-kit/src/private/node/ui/components/FatalError.test.tsx index c0ec4f3f0a6..b3b89732400 100644 --- a/packages/cli-kit/src/private/node/ui/components/FatalError.test.tsx +++ b/packages/cli-kit/src/private/node/ui/components/FatalError.test.tsx @@ -43,6 +43,23 @@ describe('FatalError', async () => { `) }) + test('does not linkify user-supplied invalid URLs or placeholder URLs in error/tryMessage', async () => { + const error = new AbortError(`Invalid tunnel URL: https://asda`, `Valid format: "https://my-tunnel-url:port"`) + + const {lastFrame} = render() + + expect(unstyled(lastFrame()!)).toMatchInlineSnapshot(` + "╭─ error ──────────────────────────────────────────────────────────────────────╮ + │ │ + │ Invalid tunnel URL: https://asda │ + │ │ + │ Valid format: \"https://my-tunnel-url:port\" │ + │ │ + ╰──────────────────────────────────────────────────────────────────────────────╯ + " + `) + }) + test('renders correctly with a formatted message', async () => { const error = new AbortError([ 'There has been an error creating your deployment:', diff --git a/packages/cli-kit/src/private/node/ui/components/Link.test.tsx b/packages/cli-kit/src/private/node/ui/components/Link.test.tsx index 00fb8a8b7c0..2dc1b0ad563 100644 --- a/packages/cli-kit/src/private/node/ui/components/Link.test.tsx +++ b/packages/cli-kit/src/private/node/ui/components/Link.test.tsx @@ -1,11 +1,34 @@ import {Link} from './Link.js' +import {LinksContext, Link as LinkEntry} from '../contexts/LinksContext.js' import {render} from '../../testing/ui.js' import {describe, expect, test, vi} from 'vitest' -import React from 'react' +import React, {FunctionComponent, useRef} from 'react' import supportsHyperlinks from 'supports-hyperlinks' vi.mock('supports-hyperlinks') +const WithLinksContext: FunctionComponent<{children: React.ReactNode; addLinkSpy?: (label: string | undefined, url: string) => void}> = ({ + children, + addLinkSpy, +}) => { + const links = useRef>({}) + return ( + { + addLinkSpy?.(label, url) + const newId = (Object.keys(links.current).length + 1).toString() + links.current = {...links.current, [newId]: {label, url}} + return newId + }, + }} + > + {children} + + ) +} + describe('Link', async () => { test("renders correctly with a fallback for terminals that don't support hyperlinks", async () => { // Given @@ -100,6 +123,58 @@ describe('Link', async () => { expect(lastFrame()).toMatchInlineSnapshot('"https://example.com"') }) + describe('inside a LinksContext (Banner)', async () => { + test('renders just `[N]` (not `url [N]`) for label-less links when the terminal does not support hyperlinks', async () => { + supportHyperLinks(false) + + const {lastFrame} = render( + + + , + ) + + expect(lastFrame()).toBe('[1]') + }) + + test('uses the footnote mechanism for labeled links when the terminal does not support hyperlinks', async () => { + supportHyperLinks(false) + + const {lastFrame} = render( + + + , + ) + + expect(lastFrame()).toBe('Example [1]') + }) + + test('registers label-less links in context so the footnote mechanism is always used', async () => { + supportHyperLinks(false) + const addLinkSpy = vi.fn() + + render( + + + , + ) + + expect(addLinkSpy).toHaveBeenCalledWith(undefined, 'https://example.com') + }) + + test('registers labeled links in context', async () => { + supportHyperLinks(false) + const addLinkSpy = vi.fn() + + render( + + + , + ) + + expect(addLinkSpy).toHaveBeenCalledWith('Example', 'https://example.com') + }) + }) + function supportHyperLinks(isSupported: boolean) { vi.mocked(supportsHyperlinks).stdout = isSupported } diff --git a/packages/cli-kit/src/private/node/ui/components/TokenizedText.test.tsx b/packages/cli-kit/src/private/node/ui/components/TokenizedText.test.tsx index b9428886140..89ff9183365 100644 --- a/packages/cli-kit/src/private/node/ui/components/TokenizedText.test.tsx +++ b/packages/cli-kit/src/private/node/ui/components/TokenizedText.test.tsx @@ -1,13 +1,32 @@ import {tokenItemToString, TokenizedText} from './TokenizedText.js' +import {LinksContext, Link} from '../contexts/LinksContext.js' import {unstyled} from '../../../../public/node/output.js' import {render} from '../../testing/ui.js' import {describe, expect, test, vi} from 'vitest' import supportsHyperlinks from 'supports-hyperlinks' -import React from 'react' +import React, {FunctionComponent, useRef} from 'react' vi.mock('supports-hyperlinks') +const WithLinksContext: FunctionComponent<{children: React.ReactNode}> = ({children}) => { + const links = useRef>({}) + return ( + { + const newId = (Object.keys(links.current).length + 1).toString() + links.current = {...links.current, [newId]: {label, url}} + return newId + }, + }} + > + {children} + + ) +} + describe('TokenizedText', async () => { test('renders arrays of items separated by spaces', async () => { const item = [ @@ -64,25 +83,38 @@ describe('TokenizedText', async () => { test('renders strings without URLs unchanged', async () => { vi.mocked(supportsHyperlinks).stdout = false - const {lastFrame} = render() + const {lastFrame} = render( + + + , + ) expect(lastFrame()).toBe('no link here, just text') }) - test('preserves a URL intact when the terminal does not support hyperlinks', async () => { + test('replaces a URL with a `[N]` footnote anchor in the body when the terminal does not support hyperlinks', async () => { vi.mocked(supportsHyperlinks).stdout = false const url = 'https://shopify.dev/docs/apps/build/sales-channels/channel-config-extension#specification-properties' - const {lastFrame} = render() + const {lastFrame} = render( + + + , + ) - expect(lastFrame()).toContain(url) + expect(lastFrame()).toBe('See specification requirements: [1]') + expect(lastFrame()).not.toContain(url) }) test('wraps detected URLs in OSC 8 escapes when the terminal supports hyperlinks', async () => { vi.mocked(supportsHyperlinks).stdout = true const url = 'https://example.com/docs' - const {lastFrame} = render() + const {lastFrame} = render( + + + , + ) expect(lastFrame()).toContain(`]8;;${url}${url}]8;;`) }) @@ -92,17 +124,91 @@ describe('TokenizedText', async () => { const first = 'https://example.com/a' const second = 'https://example.com/b' - const {lastFrame} = render() + const {lastFrame} = render( + + + , + ) + + expect(lastFrame()).toContain(`]8;;${first}${first}]8;;`) + expect(lastFrame()).toContain(`]8;;${second}${second}]8;;`) + }) + + test('detects back-to-back URLs separated only by whitespace', async () => { + vi.mocked(supportsHyperlinks).stdout = true + const first = 'https://example.com/a' + const second = 'https://example.com/b' + + const {lastFrame} = render( + + + , + ) expect(lastFrame()).toContain(`]8;;${first}${first}]8;;`) expect(lastFrame()).toContain(`]8;;${second}${second}]8;;`) }) + test('preserves balanced parentheses inside URLs (e.g. Wikipedia disambiguation links)', async () => { + vi.mocked(supportsHyperlinks).stdout = true + const url = 'https://en.wikipedia.org/wiki/Ruby_(programming_language)' + + const {lastFrame} = render( + + + , + ) + + expect(lastFrame()).toContain(`]8;;${url}${url}]8;;`) + }) + + test('does not auto-linkify URLs outside a LinksContext', async () => { + vi.mocked(supportsHyperlinks).stdout = true + const url = 'https://example.com/docs' + + const {lastFrame} = render() + + expect(lastFrame()).toBe(`visit ${url} now`) + expect(lastFrame()).not.toContain(']8;;') + }) + + test('does not linkify https://-prefixed strings whose hostname has no dot (e.g. user typos like https://asda)', async () => { + vi.mocked(supportsHyperlinks).stdout = true + const candidate = 'https://asda' + + const {lastFrame} = render( + + + , + ) + + expect(lastFrame()).toBe(`Invalid tunnel URL: ${candidate}`) + expect(lastFrame()).not.toContain(']8;;') + }) + + test('does not linkify placeholder URLs with non-numeric ports (e.g. https://my-tunnel-url:port)', async () => { + vi.mocked(supportsHyperlinks).stdout = true + const candidate = 'https://my-tunnel-url:port' + + const {lastFrame} = render( + + + , + ) + + expect(lastFrame()).toBe(`Valid format: "${candidate}"`) + expect(lastFrame()).not.toContain(']8;;') + }) + test('strips trailing sentence punctuation from detected URLs', async () => { vi.mocked(supportsHyperlinks).stdout = true const url = 'https://example.com/docs' - const {lastFrame} = render() + const {lastFrame} = render( + + + , + ) expect(lastFrame()).toContain(`]8;;${url}${url}]8;;`) expect(lastFrame()).toContain('. Thanks') diff --git a/packages/cli-kit/src/private/node/ui/components/TokenizedText.tsx b/packages/cli-kit/src/private/node/ui/components/TokenizedText.tsx index 95733ef8caf..32d4f7fc1ef 100644 --- a/packages/cli-kit/src/private/node/ui/components/TokenizedText.tsx +++ b/packages/cli-kit/src/private/node/ui/components/TokenizedText.tsx @@ -4,7 +4,8 @@ import {List} from './List.js' import {UserInput} from './UserInput.js' import {FilePath} from './FilePath.js' import {Subdued} from './Subdued.js' -import React, {FunctionComponent} from 'react' +import {LinksContext} from '../contexts/LinksContext.js' +import React, {FunctionComponent, useContext} from 'react' import {Box, Text} from 'ink' export interface LinkToken { @@ -153,7 +154,16 @@ interface TokenizedTextProps { * links, and commands. */ const URL_REGEX = /https?:\/\/\S+/g -const URL_TRAILING_PUNCTUATION = /[.,;:!?)\]}>'"]+$/ +const URL_TRAILING_PUNCTUATION = /[.,;:!?'"]+$/ + +function isLikelyRealUrl(candidate: string): boolean { + try { + const parsed = new URL(candidate) + return parsed.hostname.includes('.') + } catch { + return false + } +} function renderStringWithLinks(str: string): JSX.Element { const matches = Array.from(str.matchAll(URL_REGEX)) @@ -169,6 +179,9 @@ function renderStringWithLinks(str: string): JSX.Element { if (trailing) { url = url.slice(0, url.length - trailing[0].length) } + if (!isLikelyRealUrl(url)) { + return + } const start = match.index const end = start + url.length if (start > cursor) { @@ -177,6 +190,9 @@ function renderStringWithLinks(str: string): JSX.Element { parts.push() cursor = end }) + if (parts.length === 0) { + return {str} + } if (cursor < str.length) { parts.push({str.slice(cursor)}) } @@ -184,8 +200,9 @@ function renderStringWithLinks(str: string): JSX.Element { } const TokenizedText: FunctionComponent = ({item}) => { + const linksContext = useContext(LinksContext) if (typeof item === 'string') { - return renderStringWithLinks(item) + return linksContext === null ? {item} : renderStringWithLinks(item) } else if ('command' in item) { return } else if ('link' in item) { From 5fb0f8a21ae7c6c82486fca228168e829c7011c5 Mon Sep 17 00:00:00 2001 From: Ryan Bahan Date: Tue, 28 Apr 2026 09:28:00 -0600 Subject: [PATCH 3/3] Fix lint: rethrow non-TypeError, prettier, snapshot quotes - isLikelyRealUrl now narrows to TypeError (URL constructor's failure type) and rethrows anything else (no-catch-all/no-catch-all). - Reformat WithLinksContext prop type onto multiple lines (prettier). - Drop unnecessary backslash escapes from a template-literal inline snapshot in FatalError.test.tsx (no-useless-escape). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/private/node/ui/components/FatalError.test.tsx | 2 +- .../cli-kit/src/private/node/ui/components/Link.test.tsx | 8 ++++---- .../src/private/node/ui/components/TokenizedText.tsx | 7 +++++-- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/packages/cli-kit/src/private/node/ui/components/FatalError.test.tsx b/packages/cli-kit/src/private/node/ui/components/FatalError.test.tsx index b3b89732400..9d5745766c5 100644 --- a/packages/cli-kit/src/private/node/ui/components/FatalError.test.tsx +++ b/packages/cli-kit/src/private/node/ui/components/FatalError.test.tsx @@ -53,7 +53,7 @@ describe('FatalError', async () => { │ │ │ Invalid tunnel URL: https://asda │ │ │ - │ Valid format: \"https://my-tunnel-url:port\" │ + │ Valid format: "https://my-tunnel-url:port" │ │ │ ╰──────────────────────────────────────────────────────────────────────────────╯ " diff --git a/packages/cli-kit/src/private/node/ui/components/Link.test.tsx b/packages/cli-kit/src/private/node/ui/components/Link.test.tsx index 2dc1b0ad563..d4ceb81f3a5 100644 --- a/packages/cli-kit/src/private/node/ui/components/Link.test.tsx +++ b/packages/cli-kit/src/private/node/ui/components/Link.test.tsx @@ -7,10 +7,10 @@ import supportsHyperlinks from 'supports-hyperlinks' vi.mock('supports-hyperlinks') -const WithLinksContext: FunctionComponent<{children: React.ReactNode; addLinkSpy?: (label: string | undefined, url: string) => void}> = ({ - children, - addLinkSpy, -}) => { +const WithLinksContext: FunctionComponent<{ + children: React.ReactNode + addLinkSpy?: (label: string | undefined, url: string) => void +}> = ({children, addLinkSpy}) => { const links = useRef>({}) return (