mirror of
https://github.com/vercel-labs/fx.git
synced 2026-10-28 10:15:17 +08:00
Link sign-in addresses instead of printing them
A typed /mcp login and the Vercel sign-in picker printed the whole sign-in address. Both now show a few underlined words whose OSC 8 link is the address: "Open the sign-in page" for MCP and "Authorize with Vercel", the way Codex and Grok already link theirs. Notices underline a link on every row it wraps onto. An address that can't be a link target is still printed so it can be copied.
This commit is contained in:
7 files changed
+133
-32
No files matched your search
@@ -347,10 +347,9 @@ pub const State = struct {
|
||||
switch (notice) {
|
||||
.authorize => |n| {
|
||||
sink.waitForSignIn(n.server, n.url);
|
||||
// The menu shows the address; a typed login prints it.
|
||||
// The menu shows the address; a typed login links it.
|
||||
if (!sink.isTyped(n.server)) return;
|
||||
const text = a.print("Sign in to {s} in your browser. If it didn't open, go to:\n {s}", .{ safeText(aa, n.server, 160), safeText(aa, n.url, 16 * 1024) }) catch return;
|
||||
sink.pushLocked(text, false);
|
||||
sink.pushLocked(signInText(a, aa, n.server, n.url) catch return, false);
|
||||
},
|
||||
.signed_in => |server| {
|
||||
sink.endSignIn(server);
|
||||
@@ -437,6 +436,16 @@ pub const State = struct {
|
||||
fn safeText(arena: Allocator, raw: []const u8, max: usize) []const u8 {
|
||||
return (text_utils.encodeTerminalSafeInline(arena, raw, max) catch return "?").bytes;
|
||||
}
|
||||
|
||||
/// What a typed login says once its address comes, owned by `alloc`:
|
||||
/// a link to it, or, when it can't be a link target, the address
|
||||
/// itself made safe, to copy.
|
||||
fn signInText(alloc: Allocator, arena: Allocator, server: []const u8, url: []const u8) ![]u8 {
|
||||
const name = safeText(arena, server, 160);
|
||||
if (text_utils.isLinkTarget(url))
|
||||
return alloc.print("Sign in to {s} in your browser. Didn't open? \x1b]8;id=fx-mcp-sign-in;{s}\x1b\\Open the sign-in page\x1b]8;;\x1b\\.", .{ name, url });
|
||||
return alloc.print("Sign in to {s} in your browser. If it didn't open, go to:\n {s}", .{ name, safeText(arena, url, 16 * 1024) });
|
||||
}
|
||||
};
|
||||
|
||||
/// The notices waiting for the UI, owned by `std.heap.c_allocator`.
|
||||
@@ -1166,6 +1175,18 @@ test "model catalog baseline reports removals" {
|
||||
try std.testing.expect(std.mem.find(u8, removed.?, "slack: removed") != null);
|
||||
}
|
||||
|
||||
test "a typed sign-in prints an address that can't be a link, to copy" {
|
||||
var sink: State.HostSink = .{};
|
||||
defer sink.deinit();
|
||||
const notify = sink.notify();
|
||||
sink.markTyped("docs");
|
||||
notify.notice(notify.context, .{ .authorize = .{ .server = "docs", .url = "https://a.example/a b\x1b[2J" } });
|
||||
try std.testing.expectEqual(@as(usize, 1), sink.notices.items.len);
|
||||
const text = sink.notices.items[0].text;
|
||||
try std.testing.expect(std.mem.startsWith(u8, text, "Sign in to docs in your browser. If it didn't open, go to:\n https://a.example/a b"));
|
||||
try std.testing.expect(std.mem.findScalar(u8, text, 0x1b) == null);
|
||||
}
|
||||
|
||||
test "a typed sign-in reports in the transcript, any other on the status line" {
|
||||
var sink: State.HostSink = .{};
|
||||
defer sink.deinit();
|
||||
@@ -1174,6 +1195,11 @@ test "a typed sign-in reports in the transcript, any other on the status line" {
|
||||
notify.notice(notify.context, .{ .authorize = .{ .server = "docs", .url = "https://a.example/authorize" } });
|
||||
notify.notice(notify.context, .{ .signed_in = "docs" });
|
||||
try std.testing.expectEqual(@as(usize, 2), sink.notices.items.len);
|
||||
// The address is a link under a few words, not printed out.
|
||||
try std.testing.expectEqualStrings(
|
||||
"Sign in to docs in your browser. Didn't open? \x1b]8;id=fx-mcp-sign-in;https://a.example/authorize\x1b\\Open the sign-in page\x1b]8;;\x1b\\.",
|
||||
sink.notices.items[0].text,
|
||||
);
|
||||
try std.testing.expectEqualStrings("Signed in to docs.", sink.notices.items[1].text);
|
||||
try std.testing.expect(sink.news == null);
|
||||
// Once reported, the next sign-in to it isn't typed.
|
||||
|
||||
@@ -430,6 +430,22 @@ pub fn isTerminalSafe(raw: []const u8) bool {
|
||||
return true;
|
||||
}
|
||||
|
||||
/// Returns true when `url` can be an OSC 8 link target as it is: printable
|
||||
/// ASCII with no space, so nothing in it can end the sequence. Show any other
|
||||
/// address as text instead.
|
||||
pub fn isLinkTarget(url: []const u8) bool {
|
||||
if (url.len == 0) return false;
|
||||
for (url) |c| if (c <= ' ' or c >= 0x7f) return false;
|
||||
return true;
|
||||
}
|
||||
|
||||
test "a link target is printable ASCII with no space" {
|
||||
try std.testing.expect(isLinkTarget("https://mcp.example.com/authorize?state=a%20b&x=1#y"));
|
||||
for ([_][]const u8{ "", "https://h/a b", "https://h/a\x1b\\\x1b]8;;evil", "https://h/a\x07", "https://h/\xc3\xa9", "https://h/\x7f" }) |odd| {
|
||||
try std.testing.expect(!isLinkTarget(odd));
|
||||
}
|
||||
}
|
||||
|
||||
pub fn terminalSafeVisibleWidth(raw: []const u8) usize {
|
||||
var width: usize = 0;
|
||||
var index: usize = 0;
|
||||
|
||||
@@ -7,6 +7,7 @@ const display_width = @import("../../core/shared/display_width.zig");
|
||||
const menu = @import("../../core/mcp_host/menu.zig");
|
||||
const verbs = @import("../../core/mcp_host/verbs.zig");
|
||||
const row_text = @import("row_text.zig");
|
||||
const text_utils = @import("../../core/shared/text_utils.zig");
|
||||
const ui_render = @import("../render.zig");
|
||||
|
||||
const Allocator = std.mem.Allocator;
|
||||
@@ -97,14 +98,6 @@ fn addressWidth(width: u16) usize {
|
||||
return @max(@as(usize, width) -| 4, 16);
|
||||
}
|
||||
|
||||
/// An address that can go in an OSC 8 link as it is: printable ASCII only,
|
||||
/// so nothing in it can end the sequence. Any other is shown unlinked.
|
||||
fn linkable(url: []const u8) bool {
|
||||
if (url.len == 0) return false;
|
||||
for (url) |c| if (c <= ' ' or c >= 0x7f) return false;
|
||||
return true;
|
||||
}
|
||||
|
||||
/// The list's lines below its title: a line per server, and before the
|
||||
/// servers that need attention their heading, set apart by a blank line
|
||||
/// when servers that work come first.
|
||||
@@ -300,8 +293,9 @@ fn serverLine(alloc: Allocator, arena: Allocator, p: Projection, r: menu.Row, l:
|
||||
var row: Row = .empty;
|
||||
errdefer row.deinit(alloc);
|
||||
try row.appendSlice(alloc, " ");
|
||||
// One link on every row of it, so a click on any opens the whole address.
|
||||
const linked = linkable(url);
|
||||
// One link on every row of it, so a click on any opens the whole
|
||||
// address; one that can't be a link target is shown unlinked.
|
||||
const linked = text_utils.isLinkTarget(url);
|
||||
if (linked) {
|
||||
try row.appendSlice(alloc, "\x1b]8;id=fx-mcp-sign-in;");
|
||||
try row.appendSlice(alloc, url);
|
||||
@@ -832,7 +826,7 @@ fn fuzzWaitingAddress(_: void, smith: *testing.Smith) !void {
|
||||
rest = rest[close.len..];
|
||||
continue;
|
||||
}
|
||||
try testing.expect(linkable(url));
|
||||
try testing.expect(text_utils.isLinkTarget(url));
|
||||
try testing.expect(std.mem.startsWith(u8, rest, open));
|
||||
try testing.expect(std.mem.startsWith(u8, rest[open.len..], url));
|
||||
try testing.expect(std.mem.startsWith(u8, rest[open.len + url.len ..], "\x1b\\"));
|
||||
|
||||
@@ -6,6 +6,7 @@ const picker_state = @import("../../core/input/picker_state.zig");
|
||||
const command_specs = @import("../../core/slash_commands/command_specs.zig");
|
||||
const display_width = @import("../../core/shared/display_width.zig");
|
||||
const list_window = @import("../../core/shared/list_window.zig");
|
||||
const text_utils = @import("../../core/shared/text_utils.zig");
|
||||
const skill_runtime = @import("../../core/skills/skill_runtime.zig");
|
||||
const file_index = @import("../../core/workspace/file_index.zig");
|
||||
const ui_render = @import("../render.zig");
|
||||
@@ -423,6 +424,18 @@ fn composeOnboardingPickerRow(
|
||||
return row;
|
||||
}
|
||||
|
||||
/// The device page as a link, the way Codex and Grok open theirs; null when
|
||||
/// the address can't be a link target or the row is too narrow for it.
|
||||
fn vercelOpenLabel(buf: []u8, url: []const u8, width: u16) ?[]const u8 {
|
||||
if (!text_utils.isLinkTarget(url)) return null;
|
||||
const label = std.mem.print(
|
||||
buf,
|
||||
" Open \x1b]8;id=fx-vercel-auth;{s}\x1b\\\x1b[4mAuthorize with Vercel\x1b[24m\x1b]8;;\x1b\\",
|
||||
.{url},
|
||||
) catch return null;
|
||||
return if (display_width.visibleWidthIgnoringAnsi(label) <= width) label else null;
|
||||
}
|
||||
|
||||
fn composeSignInPickerRow(
|
||||
alloc: Allocator,
|
||||
snapshot: login_flow.SignInSnapshot,
|
||||
@@ -554,7 +567,7 @@ fn composeSignInPickerRow(
|
||||
else
|
||||
" Sign in with Vercel",
|
||||
1, 4 => "",
|
||||
2 => std.mem.print(
|
||||
2 => vercelOpenLabel(&label_buf, snapshot.verification_uri, width) orelse std.mem.print(
|
||||
&label_buf,
|
||||
" Open {s}",
|
||||
.{snapshot.verification_uri},
|
||||
@@ -2506,13 +2519,21 @@ test "sign-in stage renders the complete device authorization screen" {
|
||||
}
|
||||
for ([_][]const u8{
|
||||
"Sign in with Vercel",
|
||||
"Open https://vercel.test/verify",
|
||||
// The device page is a link, not its address printed out.
|
||||
"Open \x1b]8;id=fx-vercel-auth;https://vercel.test/verify\x1b\\\x1b[4mAuthorize with Vercel\x1b[24m\x1b]8;;\x1b\\",
|
||||
"Code TEST-CODE",
|
||||
"Waiting for authorization",
|
||||
"enter reopens browser · esc cancels",
|
||||
}) |expected| {
|
||||
try std.testing.expect(std.mem.find(u8, screen.items, expected) != null);
|
||||
}
|
||||
try std.testing.expect(std.mem.find(u8, screen.items, "Open https://") == null);
|
||||
|
||||
// Too narrow for the link: the address, clipped, and no link left open.
|
||||
var narrow = try composeAuthPickerRow(alloc, view, 2, 7, 24);
|
||||
defer narrow.deinit(alloc);
|
||||
try std.testing.expect(std.mem.find(u8, narrow.items, "Open https://") != null);
|
||||
try std.testing.expect(std.mem.find(u8, narrow.items, "\x1b]8;") == null);
|
||||
}
|
||||
|
||||
test "Codex sign-in stage renders a bounded clickable authorization action" {
|
||||
|
||||
@@ -1694,6 +1694,32 @@ const NoticeLine = struct {
|
||||
|
||||
const notice_osc8_prefix = "\x1b]8;";
|
||||
const notice_osc8_close = "\x1b]8;;\x1b\\";
|
||||
const notice_link_underline = "\x1b[4m";
|
||||
const notice_link_underline_off = "\x1b[24m";
|
||||
|
||||
/// Copies a notice's text, underlining what an OSC 8 link in it covers, as
|
||||
/// fx underlines every link. `linked`: a link from the row before goes on.
|
||||
fn appendNoticeText(out: *std.ArrayList(u8), alloc: Allocator, text: []const u8, linked: bool) !void {
|
||||
var link: ?[]const u8 = if (linked) "" else null;
|
||||
if (linked) try out.appendSlice(alloc, notice_link_underline);
|
||||
var copied: usize = 0;
|
||||
while (std.mem.findPos(u8, text, copied, notice_osc8_prefix)) |start| {
|
||||
const end = display_width.ansiSequenceEnd(text, start);
|
||||
if (end <= start or end > text.len) break;
|
||||
const was = link != null;
|
||||
updateNoticeHyperlinkState(text[start..end], &link);
|
||||
if (was and link == null) {
|
||||
try out.appendSlice(alloc, text[copied..start]);
|
||||
try out.appendSlice(alloc, notice_link_underline_off);
|
||||
try out.appendSlice(alloc, text[start..end]);
|
||||
} else {
|
||||
try out.appendSlice(alloc, text[copied..end]);
|
||||
if (!was and link != null) try out.appendSlice(alloc, notice_link_underline);
|
||||
}
|
||||
copied = end;
|
||||
}
|
||||
try out.appendSlice(alloc, text[copied..]);
|
||||
}
|
||||
|
||||
fn updateNoticeHyperlinkState(fragment: []const u8, active: *?[]const u8) void {
|
||||
var offset: usize = 0;
|
||||
@@ -1820,12 +1846,15 @@ fn appendNoticeRow(
|
||||
}
|
||||
if (end > label_end) {
|
||||
try out.appendSlice(alloc, styles.system_notice_text_style);
|
||||
try out.appendSlice(alloc, logical[@max(start, label_end)..end]);
|
||||
try appendNoticeText(out, alloc, logical[@max(start, label_end)..end], active_hyperlink.* != null);
|
||||
try out.appendSlice(alloc, styles.reset_style);
|
||||
}
|
||||
if (start == end) try out.appendSlice(alloc, styles.reset_style);
|
||||
updateNoticeHyperlinkState(logical[start..end], active_hyperlink);
|
||||
if (active_hyperlink.* != null) try out.appendSlice(alloc, notice_osc8_close);
|
||||
if (active_hyperlink.* != null) {
|
||||
try out.appendSlice(alloc, notice_link_underline_off);
|
||||
try out.appendSlice(alloc, notice_osc8_close);
|
||||
}
|
||||
}
|
||||
|
||||
/// Returns an owned semantic notice block without surrounding blank rows.
|
||||
@@ -3206,7 +3235,7 @@ test "semantic notice topics never render the tool-activity bullet or forced cap
|
||||
}
|
||||
}
|
||||
|
||||
test "semantic notice keeps an OSC 8 target hidden and clickable" {
|
||||
test "semantic notice keeps an OSC 8 target hidden, clickable, and underlined" {
|
||||
const alloc = std.testing.allocator;
|
||||
const url = "https://fx.sh/feedback";
|
||||
const body = try alloc.print(
|
||||
@@ -3234,6 +3263,7 @@ test "semantic notice keeps an OSC 8 target hidden and clickable" {
|
||||
try std.testing.expect(link_cell.style.hyperlink_id != 0);
|
||||
try std.testing.expectEqualStrings(url, grid.hyperlinkUrl(link_cell.style.hyperlink_id).?);
|
||||
try std.testing.expectEqual(@as(u32, 0), grid.cellAt(1, 31).?.style.hyperlink_id);
|
||||
try expectUnderlinedExactlyWhereLinked(&grid);
|
||||
|
||||
var narrow_grid = try vt_emulator.Grid.init(alloc, 9, 16);
|
||||
defer narrow_grid.deinit();
|
||||
@@ -3258,6 +3288,18 @@ test "semantic notice keeps an OSC 8 target hidden and clickable" {
|
||||
}
|
||||
try std.testing.expectEqualStrings("Openfeedback form", linked_text.items);
|
||||
try std.testing.expectEqual(@as(u32, 0), narrow_grid.current_style.hyperlink_id);
|
||||
// Each row a link wraps onto is underlined too, and nothing after it.
|
||||
try expectUnderlinedExactlyWhereLinked(&narrow_grid);
|
||||
try std.testing.expect(!narrow_grid.current_style.flags.underline);
|
||||
}
|
||||
|
||||
fn expectUnderlinedExactlyWhereLinked(grid: *vt_emulator.Grid) !void {
|
||||
for (1..grid.rows + 1) |row_index| {
|
||||
for (1..grid.cols + 1) |col_index| {
|
||||
const cell = grid.cellAt(@intCast(row_index), @intCast(col_index)).?;
|
||||
try std.testing.expectEqual(cell.style.hyperlink_id != 0, cell.style.flags.underline);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
test "background semantic notices render one topic for launch and failure" {
|
||||
|
||||
+11
-12
@@ -1161,14 +1161,15 @@ describe("MCP login", () => {
|
||||
expect(pane).not.toContain(ACCESS_INITIAL);
|
||||
}, 60_000);
|
||||
|
||||
test.skipIf(!tmuxAvailable())("/mcp login NAME --no-browser prints the address and the outcome in the transcript", async () => {
|
||||
test.skipIf(!tmuxAvailable())("/mcp login NAME --no-browser links the address and prints the outcome in the transcript", async () => {
|
||||
upstream = startModernMcpHttpFixture("json");
|
||||
auth = startAuthFixture(upstream.url);
|
||||
const root = createRoot(auth, true, "http", auth.url, false);
|
||||
const tapePath = join(root.root, "no-browser.fxtape");
|
||||
gateway = startFakeGateway([], { models: [{ id: MODEL, type: "language", tags: ["tool-use"] }] });
|
||||
tui = await TmuxSession.create({
|
||||
isolated: true, cwd: root.workspace, width: 140, height: 36,
|
||||
env: { ...v2Env(root), FX_GATEWAY_BASE_URL: gateway.baseUrl, FX_GATEWAY_CHAT_URL: gateway.chatUrl },
|
||||
env: { ...v2Env(root), FX_RECORD: tapePath, FX_GATEWAY_BASE_URL: gateway.baseUrl, FX_GATEWAY_CHAT_URL: gateway.chatUrl },
|
||||
});
|
||||
await tui.waitForComposer(15_000);
|
||||
// The flag after the name, which the parser takes like the flag before it.
|
||||
@@ -1180,16 +1181,14 @@ describe("MCP login", () => {
|
||||
return composer < 0 ? lines : lines.slice(0, composer);
|
||||
};
|
||||
await tui.waitForText("Sign in to fixture in your browser.", 10_000);
|
||||
// fx wraps the address to the pane; join its lines back up.
|
||||
const lines = await transcript();
|
||||
const start = lines.findIndex((line) => line.includes("If it didn't open, go to:"));
|
||||
expect(start).toBeGreaterThanOrEqual(0);
|
||||
let address = "";
|
||||
for (const line of lines.slice(start + 1)) {
|
||||
const part = line.trim();
|
||||
if (!/^[A-Za-z0-9%._~:/?#\[\]@!$&'()*+,;=-]+$/.test(part)) break;
|
||||
address += part;
|
||||
}
|
||||
// A few words stand for the address, which only their link carries.
|
||||
const shown = (await transcript()).join("\n");
|
||||
expect(shown).toContain("Didn't open? Open the sign-in page.");
|
||||
expect(shown).not.toContain("/authorize?");
|
||||
const written = Buffer.concat(stdoutFrames(tapePath).map((frame) => frame.payload)).toString("utf8");
|
||||
const link = /\x1b\]8;id=fx-mcp-sign-in;([^\x1b]+)\x1b\\/.exec(written);
|
||||
expect(link).not.toBeNull();
|
||||
const address = link![1];
|
||||
expect(new URL(address).searchParams.get("state")).toBeTruthy();
|
||||
expect(existsSync(root.openLog)).toBe(false);
|
||||
// The user approves in some other browser, which then loads fx's loopback address.
|
||||
|
||||
@@ -2137,12 +2137,15 @@ tmuxTest(
|
||||
(pane) =>
|
||||
pane.includes("Sign in with Vercel") &&
|
||||
pane.includes("TEST-CODE") &&
|
||||
pane.includes("/verify") &&
|
||||
pane.includes("Authorize with Vercel") &&
|
||||
pane.includes("Waiting for authorization") &&
|
||||
pane.includes("enter reopens browser · esc cancels"),
|
||||
TIMEOUT,
|
||||
);
|
||||
expect(signInScreen).not.toContain("Starting Vercel sign-in");
|
||||
// The device page is a link, not its address printed out.
|
||||
expect(signInScreen).toMatch(/^ Open\s+Authorize with Vercel$/m);
|
||||
expect(signInScreen).not.toContain("/verify");
|
||||
|
||||
await session.sendKeys("C-c");
|
||||
await session.waitForComposer(TIMEOUT);
|
||||
|
||||
Reference in new issue
Block a user