Skip to content

Commit 97b239d

Browse files
committed
Brand the OAuth callback page and drop the stale overlay echo
The page a provider redirects back to was a browser-default serif on white saying "Authorization complete" — the only web surface this product has, and the last thing an operator sees before coming back to the terminal. One shared renderer now serves it for MCP servers and inference providers alike, on the terminal's own palette, with the mark animating through the same dithered draw/fill timeline as the landing. The headline names what happened: "Linear connected successfully", "Granola failed to connect". Server names and OAuth error codes are humanized on the way in, so nothing reaches the page in snake case. Everything is inline — a local authorization callback has no business making a network call. Accepting an overlay row also appended "chose (kind): label" to the transcript, which on /mcp quoted the row's pre-authorization label back permanently: "granola — needs auth", moments after authorizing it. openListOverlay can now suppress that echo, and /mcp does, since its flash already reports the outcome.
1 parent 2f11504 commit 97b239d

8 files changed

Lines changed: 377 additions & 23 deletions

File tree

src/auth/callback-page.test.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
import { describe, expect, test } from "bun:test";
2+
3+
import { callbackPageHtml, humanizeIdentifier } from "./callback-page.js";
4+
5+
describe("humanizeIdentifier", () => {
6+
test("machine identifiers lose their separators and lead with a capital", () => {
7+
expect(humanizeIdentifier("access_denied")).toBe("Access denied");
8+
expect(humanizeIdentifier("granola")).toBe("Granola");
9+
expect(humanizeIdentifier("claude-ai-gamma")).toBe("Claude ai gamma");
10+
expect(humanizeIdentifier("googleDrive")).toBe("Google Drive");
11+
});
12+
13+
test("an empty identifier is returned untouched rather than as a stray capital", () => {
14+
expect(humanizeIdentifier("")).toBe("");
15+
});
16+
});
17+
18+
describe("callbackPageHtml", () => {
19+
test("success names the server that connected", () => {
20+
const html = callbackPageHtml({ subject: "linear" });
21+
expect(html).toContain("Linear connected successfully");
22+
expect(html).not.toContain("access_denied");
23+
});
24+
25+
test("failure names the server and the humanized reason", () => {
26+
const html = callbackPageHtml({ subject: "granola", error: "access_denied" });
27+
expect(html).toContain("Granola failed to connect");
28+
expect(html).toContain("Access denied.");
29+
expect(html).not.toContain("access_denied");
30+
});
31+
32+
test("an unnamed authorization still renders both outcomes", () => {
33+
expect(callbackPageHtml()).toContain("Authorization complete");
34+
expect(callbackPageHtml({ error: "server_error" })).toContain(
35+
"Authorization did not complete",
36+
);
37+
});
38+
39+
test("the subject is escaped rather than pasted into markup", () => {
40+
expect(callbackPageHtml({ subject: "<script>x</script>" })).not.toContain(
41+
"<script>x",
42+
);
43+
});
44+
45+
test("the page reaches for nothing off the machine", () => {
46+
const html = callbackPageHtml({ subject: "linear" });
47+
expect(html).not.toMatch(/https?:\/\/(?!www\.w3\.org)/);
48+
});
49+
});

src/auth/callback-page.ts

Lines changed: 283 additions & 0 deletions
Large diffs are not rendered by default.

src/auth/oauth/callback-server.ts

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { createServer, type Server } from "node:http";
2-
import { PRODUCT_NAME } from "../../branding.js";
2+
import { callbackPageHtml } from "../callback-page.js";
33

44
export type CallbackServer = {
55
// Resolves with the validated authorization code once the browser redirects
@@ -111,10 +111,6 @@ export async function startCallbackServer(
111111
};
112112
}
113113

114-
export function authorizationDoneHtml(productName: string): string {
115-
return (
116-
"<!doctype html><meta charset=utf-8><title>Authorized</title>" +
117-
'<body style="font-family:system-ui;padding:3rem;text-align:center">' +
118-
`<h1>${productName} authorization complete</h1><p>You can close this tab and return to ${PRODUCT_NAME}.</p>`
119-
);
114+
export function authorizationDoneHtml(providerName: string): string {
115+
return callbackPageHtml({ subject: providerName });
120116
}

src/mcp/callback-server.ts

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { createServer, type Server } from "node:http";
22
import type { AddressInfo } from "node:net";
3-
import { PRODUCT_NAME } from "../branding.js";
3+
import { callbackPageHtml } from "../auth/callback-page.js";
44

55
export type CallbackServer = {
66
// The redirect_uri to register with the authorization server.
@@ -17,15 +17,12 @@ type CallbackWaiter = { resolve: (code: string) => void; reject: (error: Error)
1717

1818
const CALLBACK_PATH = "/callback";
1919

20-
const DONE_HTML =
21-
"<!doctype html><meta charset=utf-8><title>Authorized</title>" +
22-
"<body style=\"font-family:system-ui;padding:3rem;text-align:center\">" +
23-
`<h1>Authorization complete</h1><p>You can close this tab and return to ${PRODUCT_NAME}.</p>`;
24-
25-
// Start an ephemeral loopback server to receive the OAuth redirect. Binds to a
20+
// Start an ephemeral loopback server to receive the OAuth redirect. `serverName`
21+
// only names the authorization on the page the browser lands on.
22+
// Binds to a
2623
// random port on 127.0.0.1 so it never collides with anything and is only
2724
// reachable locally.
28-
export async function startCallbackServer(): Promise<CallbackServer> {
25+
export async function startCallbackServer(serverName?: string): Promise<CallbackServer> {
2926
let expectedState: string | undefined;
3027
let pendingResult: CallbackResult | undefined;
3128
let waiter: CallbackWaiter | undefined;
@@ -61,9 +58,15 @@ export async function startCallbackServer(): Promise<CallbackServer> {
6158

6259
const code = url.searchParams.get("code");
6360
const error = url.searchParams.get("error");
64-
res.statusCode = error !== null || code === null ? 400 : 200;
61+
const failure = error ?? (code === null ? "the redirect carried no code" : undefined);
62+
res.statusCode = failure === undefined ? 200 : 400;
6563
res.setHeader("content-type", "text/html; charset=utf-8");
66-
res.end(error !== null || code === null ? `Authorization failed: ${error ?? "no code returned"}` : DONE_HTML);
64+
res.end(
65+
callbackPageHtml({
66+
...(serverName !== undefined ? { subject: serverName } : {}),
67+
...(failure !== undefined ? { error: failure } : {}),
68+
}),
69+
);
6770
if (error !== null) deliver({ error: new Error(`Authorization failed: ${error}`) });
6871
else if (code === null) deliver({ error: new Error("Authorization redirect carried no code.") });
6972
else deliver({ code });

src/mcp/client.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ async function connectStdio(config: MCPServerConfig, options: MCPConnectOptions)
119119
async function connectHttp(config: MCPServerConfig, options: MCPConnectOptions): Promise<MCPConnectResult> {
120120
if (config.url === undefined) return { ok: false, serverName: config.name, error: "http MCP server requires a url" };
121121
const url = new URL(config.url);
122-
const callback = await startCallbackServer();
122+
const callback = await startCallbackServer(config.name);
123123
const authProvider = await createOAuthProvider({
124124
serverName: config.name,
125125
redirectUrl: callback.redirectUrl,

src/tui-opentui/command-surfaces.test.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -543,6 +543,9 @@ describe("mcp surface", () => {
543543
acceptOverlaySelection(shell)
544544
expect(opened).toEqual(["https://notion.test/auth"])
545545
expect(shell.statusFlash).toContain("notion")
546+
// The echo would quote "notion — needs auth" back forever, moments
547+
// after the operator authorized it.
548+
expect(shell.streamLog.filter((r) => r.meta === "overlay")).toEqual([])
546549
})
547550
})
548551

src/tui-opentui/command-surfaces.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -953,6 +953,9 @@ export function openMcpSurface(shell: AppShell, deps: CommandSurfaceDeps): void
953953
kind: "mcp",
954954
title: "mcp",
955955
frameId: "overlay-mcp",
956+
// The flash below reports the outcome; the echo would quote the row's
957+
// pre-authorization label back at the operator forever.
958+
echoChoice: false,
956959
...payload(rows),
957960
describe: (id) => {
958961
const target = byName.get(id)

src/tui-opentui/shell.ts

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1682,6 +1682,8 @@ type ShellInternals = {
16821682
overlayItemIds: readonly string[]
16831683
/** Per-open accept callback; cleared on close without invoke (Esc path). */
16841684
overlayOnAccept: ((selection: OverlaySelection) => void) | null
1685+
/** False while an overlay that reports its own outcome is open. */
1686+
overlayEchoChoice: boolean
16851687
/** Per-open expand/collapse hook for the open primary overlay. */
16861688
overlayOnToggleExpand: (() => void) | null
16871689
/** Per-open ← → cycle hook for the open primary overlay (settings inline cycling). */
@@ -2673,6 +2675,16 @@ export type OpenListOverlayOpts = {
26732675
* nothing to choose, so the overlay is never a chooser with an empty list.
26742676
*/
26752677
readonly textAnswerActive?: boolean
2678+
/**
2679+
* Suppress the `chose (kind): label` transcript echo for this open.
2680+
*
2681+
* The echo exists so a choice with no other visible result still leaves a
2682+
* trace. A surface that reports the outcome itself does not need it, and the
2683+
* echo is worse than silent there: it quotes the row's label from *before*
2684+
* the action, so authorizing a server leaves a permanent line saying that
2685+
* server needs authorization.
2686+
*/
2687+
readonly echoChoice?: boolean
26762688
}
26772689

26782690
/**
@@ -2732,6 +2744,7 @@ export function openListOverlay(
27322744
if (!isPalette) {
27332745
bag.overlayItemIds = opts?.itemIds ? [...opts.itemIds] : []
27342746
bag.overlayOnAccept = opts?.onAccept ?? null
2747+
bag.overlayEchoChoice = opts?.echoChoice ?? true
27352748
bag.overlayOnToggleExpand = opts?.onToggleExpand ?? null
27362749
bag.overlayOnCycle = opts?.onCycle ?? null
27372750
bag.overlayDescribe = opts?.describe ?? null
@@ -2740,6 +2753,7 @@ export function openListOverlay(
27402753
// Bare palette (no primary under it): no accept payload.
27412754
bag.overlayItemIds = opts?.itemIds ? [...opts.itemIds] : []
27422755
bag.overlayOnAccept = opts?.onAccept ?? null
2756+
bag.overlayEchoChoice = opts?.echoChoice ?? true
27432757
bag.overlayOnToggleExpand = opts?.onToggleExpand ?? null
27442758
bag.overlayOnCycle = opts?.onCycle ?? null
27452759
bag.overlayDescribe = opts?.describe ?? null
@@ -3353,11 +3367,13 @@ export function acceptOverlaySelection(shell: AppShell): void {
33533367
// Capture before close clears per-open state.
33543368
const perOpen = bag?.overlayOnAccept ?? null
33553369

3356-
appendStreamRow(shell, {
3357-
role: "system",
3358-
text: `chose (${kind}): ${label}`,
3359-
meta: "overlay",
3360-
})
3370+
if (bag?.overlayEchoChoice !== false) {
3371+
appendStreamRow(shell, {
3372+
role: "system",
3373+
text: `chose (${kind}): ${label}`,
3374+
meta: "overlay",
3375+
})
3376+
}
33613377
closeInsetOverlay(shell)
33623378
dispatchOverlayAccept(shell, selection, perOpen)
33633379
}
@@ -4918,6 +4934,7 @@ export function createAppShell(
49184934
priorOverlay: null,
49194935
overlayItemIds: [],
49204936
overlayOnAccept: null,
4937+
overlayEchoChoice: true,
49214938
overlayOnToggleExpand: null,
49224939
overlayOnCycle: null,
49234940
overlayDescribe: null,

0 commit comments

Comments
 (0)