fix(mcp): make the OAuth authorization URL clickable when wrapped (#3489)

* fix(mcp): render OAuth URL as OSC 8 hyperlink so it stays clickable when wrapped

Closes #3470.

The MCP OAuth flow previously pushed the authorization URL through the
generic display-message list, where Ink rendered it as plain text. When
the URL exceeded the terminal width it got hard-wrapped into the message
buffer, and most terminals could no longer detect it as a single
hyperlink (cmd/ctrl+click did nothing, selecting it pulled in extra
whitespace).

Render the URL as an OSC 8 hyperlink in AuthenticateStep instead, and
stop duplicating it through the display-message stream when an event
emitter is available. Terminals that support OSC 8 (iTerm2, WezTerm,
Kitty, Windows Terminal, VS Code, GNOME Terminal, …) now treat the URL
as a single clickable link even when it visually wraps; terminals
without OSC 8 support ignore the escapes and fall back to the existing
"press c to copy" affordance.

* fix(mcp): pre-split OAuth URL so every wrapped line stays clickable

Wrapping the whole URL in a single OSC 8 hyperlink and letting Ink /
wrap-ansi break the line produced two bugs observed in iTerm2 etc.:
only the first visible segment was a hyperlink (wrap-ansi re-emits SGR
codes across wraps but does not re-open OSC 8 links), and the remaining
URL characters overflowed past the dialog border because wrap-ansi was
unable to break the unbroken URL token within the container width.

Manually slice the URL into chunks of `columns - 8` characters
(MCPManagementDialog's container width) and render each chunk as its
own OSC 8 hyperlink with `wrap="truncate"`. Every visible line now
carries a complete hyperlink pointing at the same URL, and no line
exceeds the container width.

* fix(mcp): terminate OSC 8 hyperlinks with BEL so Ink preserves them

Ink's renderer tokenizes text through @alcalzone/ansi-tokenize, which
only recognizes OSC 8 hyperlink escapes terminated with BEL (\x07).
The ST terminator (ESC \\) we were using is valid per the OSC 8 spec
but the tokenizer falls through and treats the escape bytes as regular
characters. That explains the two symptoms seen after the previous fix:

  - Only the first URL segment rendered as a clickable hyperlink. The
    rest of the lines had their opening \\x1b]8;; bytes tokenized as
    chars, so their hyperlink wrap was lost.
  - The dialog's right border disappeared because the mangled escape
    bytes consumed grid cells, pushing the container width past
    `columns - 8` and shoving the border off-screen.

Switch the helper to the BEL-terminated form. Ink now sees each line's
OSC 8 wrap as a proper zero-width code, every wrapped line stays
clickable, and the border is no longer displaced.

* fix(mcp): render OAuth URL via <Static> as a single unwrapped line

The per-line OSC 8 approach didn't make lines past the first clickable
in real terminals. Root cause: Ink's renderer runs text through
@alcalzone/ansi-tokenize, which:

  - Only accepts OSC 8 sequences with empty params (`\x1b]8;;URL\x07`).
    Any `id=` form is parsed as a bogus SGR code and the remainder
    leaks out as visible characters. Without an `id=` grouping
    parameter, terminals like iTerm2 don't reliably stitch adjacent
    OSC 8 escapes together as one hyperlink.
  - Re-emits styles per Ink row via styledCharsToString, so even when
    each slice carried a self-contained OSC 8 wrap, terminals still
    treated each visual line as an independent hyperlink that only
    the first row reliably activated.

Emit the URL through Ink's `<Static>` component instead, inside a
`<Box width={url.length}>`. Ink sees a single logical line that
doesn't need wrapping, so it hands the terminal one OSC 8 open, the
whole URL, and one close. The terminal then soft-wraps that line
visually, and the OSC 8 hyperlink state is carried across every
wrap — every visible line is clickable.

`<Static>` writes once above the dynamic dialog (scrollback-safe) and
isn't touched by re-renders, which also avoids the flicker we'd get
from repeatedly re-emitting the escape sequence inside the live tree.

* fix(mcp): render OAuth URL as live row so it clears on dialog dismissal

The previous <Static> emission made the URL stay permanently in the
scrollback after the OAuth flow finished — e.g. after the dialog was
dismissed the URL was still sitting above the prompt.

Switch to a normal (live) Ink row: a Box sized to the URL length
holding a single OSC 8 wrapped Text. Ink doesn't wrap the row
(maxWidth == content width), so it hands log-update one long line;
log-update's wrap-ansi pass then wraps it at terminal width and
re-emits the OSC 8 escape at every wrap boundary, so every visible
wrapped line is clickable. Because this is a regular child of the
dialog, log-update tracks its height and erases it cleanly when the
AuthenticateStep unmounts (auth succeeds / user backs out / dialog
closes).

* fix(mcp): pre-split OAuth URL so the live row clears cleanly

The wide-Box live approach left dialog fragments in the scrollback: Ink
ships its own log-update.js (packages/cli/.../ink/build/log-update.js)
which counts erase height with output.split('\n').length and does NOT
run wrap-ansi. A single Ink row that exceeds terminal width wraps
visually but the erase still covers only one terminal line, so
authState transitions (auth success, Esc-to-back, dialog dismiss) leave
the top rows of the previous frame behind.

Go back to pre-slicing the URL into chunks sized to the dialog content
width (columns - 8) and rendering each chunk as its own Ink row with
its own OSC 8 wrap. Log-update's row count then matches the visible
row count, so erase is clean on every transition. Terminals that group
adjacent OSC 8 sequences will still treat the whole URL as clickable;
those that don't at least keep the first slice clickable, and the
existing "press c to copy" affordance covers the rest.

* fix(mcp): commit to Static-rendered URL outside the dialog

Stop flip-flopping between in-box and out-of-box URL rendering. Every
in-box attempt hit one of two walls:

  - Per-slice OSC 8 rows: each Ink row is its own self-contained
    hyperlink, but some terminals (seen with the reporter's) only
    register the first adjacent OSC 8 without an id= parameter as
    clickable. Ink's @alcalzone/ansi-tokenize rejects OSC 8 with
    params, so id= grouping is not deliverable.
  - Wide-Box overflow rows: the single OSC 8 wrap keeps every wrapped
    line clickable because the hyperlink state persists across the
    terminal's soft-wraps, but Ink ships its own log-update.js that
    counts erase height by output.split('\n').length and never runs
    wrap-ansi. When the row visually wraps but Ink counts it as 1
    row, transitions (auth success / Esc / dismiss) erase too few
    lines and leave dialog fragments in the scrollback.

Render the URL through <Static> above the dialog: it writes once,
outside log-update's tracking, so the terminal soft-wraps a single
OSC 8 hyperlink and every visible line stays clickable. The trade-off
is that the URL stays in the scrollback after the dialog dismisses
(Static is append-only); that is acceptable given the URL is no
longer sensitive once auth has completed, and it avoids the
click-failure and residue problems of the other approaches.

* fix(mcp): print OAuth URL via useStdout, erase on unmount

Drop <Static> (which persisted the URL in the scrollback forever) and
print the authorization URL directly with Ink's `write` (useStdout)
instead. Ink's writeToStdout clears the live frame, writes our bytes
into the scrollback, and re-renders the frame below, so the URL goes
out in a single OSC 8 hyperlink sequence and the terminal's soft-wrap
preserves the hyperlink state across every wrapped row — every visible
line stays clickable.

On unmount (auth success, Esc, dialog dismiss) we use the same `write`
path to push a cursor-up + eraseLines sequence that removes the URL
rows (plus the leading/trailing blank separators) before log-update
redraws the now-smaller live frame. Net effect: URL shows above the
dialog while authenticating, disappears cleanly when the dialog goes
away, and every wrapped line is clickable throughout.

* fix(mcp): period-terminate prompt and restore wrap warning

Now that the OAuth URL renders above the dialog (outside the message
list), the in-dialog prompt no longer leads into the URL on the next
line — rename the i18n key from "…into your browser:" to "…into your
browser." and re-add the "Make sure to copy the COMPLETE URL — it may
wrap across multiple lines." warning that was dropped when the URL
was first moved out of displayMessage. Translations in de/en/fr/ja/pt/
ru/zh are updated to match and to point at the URL "above" rather
than "following".

* fix(mcp): correct OAuth URL erase count on unmount

The previous logic wrote the URL as `\n${URL}\n` (leading + trailing
newlines) and erased `urlVisualLines + 2` rows on unmount, but the
leading blank and the trailing "\n" don't both occupy their own rows
— the trailing newline just moves the cursor to where the dynamic UI
is re-rendered. For a typical URL whose length isn't an exact multiple
of the terminal width this left the erase off-by-one and wiped a row
above the dialog (e.g. a piece of the command prompt).

Drop the leading `\n` (no real visual benefit) and compute the erase
count as `urlVisualLines + (autoWrapOverflow ? 1 : 0)`. The overflow
term handles the aligned edge case where the terminal auto-wraps past
the last URL char, leaving a blank row between URL and re-rendered
dynamic UI that also needs erasing.

Also drop the stale comment about Ink's ansi-tokenize restricting
OSC 8 terminator choice — we now bypass Ink's tokenizer via
useStdout, so BEL is just the more compatible terminator.

* fix(mcp): pass OAuth URL hyperlink through multiplexer wrapper

Inside tmux or GNU screen the raw OSC 8 hyperlink escape is intercepted
by the multiplexer and never reaches the host terminal — users see
the URL as plain text, exactly the bug this PR is trying to fix. The
existing `wrapForMultiplexer` helper (already used for OSC 52 clipboard
writes) wraps the sequence in a DCS passthrough envelope that tmux /
screen forward to the host.

Apply the same helper to `osc8Hyperlink` so tmux / screen users get
clickable links for every wrapped line as well. Outside a multiplexer
the helper is a no-op, so native terminals are unchanged.

Also note in a comment that the captured `stdout.columns` goes stale
if the terminal is resized during the OAuth flow; this is acceptable
for a sub-minute flow on ASCII-only authorization URLs.

* docs(mcp): note tmux 3.3+ allow-passthrough requirement

* fix(mcp): render OAuth URL inside dialog box

Replace the useStdout().write + cursor-up/eraseLines scrollback
approach with an in-dialog <Box><Text>{osc8Hyperlink(url)}</Text></Box>.
Removes the Ink dynamic-frame interleave and the column-width erase
bookkeeping; the URL is owned by the dialog, so it disappears with it.

* refactor(mcp): drop redundant Fragment around single Text

* revert(mcp): restore original OAuth prompt wording

URL now renders inside the dialog box, so the "copy and paste this URL
into your browser:" prompt no longer needs the period-terminated /
"URL above" rewording. Revert the i18n keys and localized strings;
keep the event-driven dispatch so the URL isn't also pushed through
displayMessage (which would double-render in the UI).

* fix(mcp): sanitize URL/label before embedding in OSC 8 sequence

An unescaped \x07 (BEL) or \x1b (ESC) in the URL or label would
terminate the OSC 8 envelope early and let the tail bytes through
as interpretable terminal escapes. authUrl is normally built via
URL.toString() which percent-encodes controls, but the authorization
endpoint itself comes from server-controlled OAuth discovery, so
treat the input as untrusted and strip C0 + DEL before splicing.
This commit is contained in:
Edenman 2026-04-21 16:44:23 +08:00 committed by GitHub
parent 00896f8605
commit 07bd5c41cb
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 42 additions and 2 deletions

View file

@ -27,7 +27,9 @@ const COPY_FEEDBACK_MS = 2000;
/**
* Wrap an OSC sequence for terminal multiplexers so the host terminal
* receives it. tmux requires a DCS passthrough with inner ESCs doubled;
* GNU screen uses a plain DCS envelope.
* GNU screen uses a plain DCS envelope. Note: tmux 3.3+ defaults
* `allow-passthrough` to off users on default configs will not see
* the hyperlink until they set `set -g allow-passthrough on`.
*/
function wrapForMultiplexer(osc: string): string {
if (process.env['TMUX']) {
@ -39,6 +41,34 @@ function wrapForMultiplexer(osc: string): string {
return osc;
}
/**
* Strip C0 control characters and DEL so an untrusted string can be safely
* embedded inside an OSC escape. Without this a `\x07` (BEL) or `\x1b` (ESC)
* in the input would prematurely terminate the OSC sequence and leak the
* tail bytes to the terminal as interpretable escape codes.
*/
function sanitizeForOsc(s: string): string {
// eslint-disable-next-line no-control-regex
return s.replace(/[\x00-\x1f\x7f]/g, '');
}
/**
* Wrap a URL in an OSC 8 hyperlink escape sequence. Supported terminals
* (iTerm2, WezTerm, Kitty, Windows Terminal, VS Code, GNOME Terminal, )
* render it as a clickable link; terminals without OSC 8 support ignore
* the escapes and print the raw text. BEL (\x07) terminates the OSC
* sequence more broadly supported than ST (ESC \\).
*
* Inside tmux / screen the OSC sequence is wrapped in a DCS passthrough
* envelope (see `wrapForMultiplexer`) so the multiplexer forwards it to
* the host terminal instead of eating it.
*/
function osc8Hyperlink(url: string, label = url): string {
const safeUrl = sanitizeForOsc(url);
const safeLabel = sanitizeForOsc(label);
return wrapForMultiplexer(`\x1b]8;;${safeUrl}\x07${safeLabel}\x1b]8;;\x07`);
}
/**
* Copy a string to the user's clipboard using the OSC 52 terminal escape
* sequence. Works through SSH and most web terminals (iTerm2, Windows
@ -260,6 +290,12 @@ export const AuthenticateStep: React.FC<AuthenticateStepProps> = ({
</Box>
)}
{authUrl && (
<Box>
<Text color={theme.text.accent}>{osc8Hyperlink(authUrl)}</Text>
</Box>
)}
{/* Action hints */}
<Box flexDirection="column">
{authState === 'authenticating' && (