From 444df64a190ccef7194b4c765d595f082a9df0d5 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Thu, 6 Aug 2026 18:00:35 -0700 Subject: [PATCH 1/4] Move MCP authorization out of the transcript and into /mcp A remote server that needs OAuth dumped a raw authorization URL as a transcript row at session start: unactionable, uncopyable, and gone once it scrolled away. Needs-auth now takes a standing segment on the transient notice row ("1 mcp needs auth (/mcp)") that clears when the server connects, and /mcp becomes a real list surface over every configured server and its live state. Enter on an unauthorized row opens its authorization page in the browser and copies the link, which is what makes it work over SSH. The old /mcp text dump and its getMCPServers command context go away. --- src/tui-opentui/command-surfaces.test.ts | 53 ++++++++++++ src/tui-opentui/command-surfaces.ts | 100 +++++++++++++++++++++++ src/tui-opentui/notice-line.test.ts | 6 ++ src/tui-opentui/notice-line.ts | 7 ++ src/tui-opentui/product-host.ts | 10 ++- src/tui-opentui/runtime-channels.test.ts | 21 ++++- src/tui-opentui/runtime-notices.test.ts | 7 +- src/tui-opentui/runtime-notices.ts | 12 +-- src/tui-opentui/shell.ts | 12 +++ src/tui/commands/built-in.ts | 14 +--- src/tui/commands/registry.ts | 3 +- src/tui/runner.ts | 20 ++++- 12 files changed, 233 insertions(+), 32 deletions(-) diff --git a/src/tui-opentui/command-surfaces.test.ts b/src/tui-opentui/command-surfaces.test.ts index 26994da1f..bf7cc85d5 100644 --- a/src/tui-opentui/command-surfaces.test.ts +++ b/src/tui-opentui/command-surfaces.test.ts @@ -514,6 +514,59 @@ describe("hooks surface", () => { }) }) +describe("mcp surface", () => { + const entries = [ + { name: "linear", state: "connected" as const, toolCount: 12 }, + { name: "notion", state: "needs-auth" as const, authURL: "https://notion.test/auth" }, + { name: "sentry", state: "failed" as const, error: "ECONNREFUSED" }, + ] + + test("lists every configured server with its live state", async () => { + await withShell((shell) => { + openCommandSurface(shell, "mcp", { notify: () => {}, mcp: { list: () => entries, openAuthURL: () => {} } }) + expect(shell.overlayItems.slice(0, 3)).toEqual([ + "linear — connected · 12 tools", + "notion — needs auth", + "sentry — failed", + ]) + }) + }) + + test("Enter on an unauthorized server opens the browser and copies the link", async () => { + await withShell((shell) => { + const opened: string[] = [] + openCommandSurface(shell, "mcp", { + notify: () => {}, + mcp: { list: () => entries, openAuthURL: (url) => opened.push(url) }, + }) + moveOverlaySelection(shell, 1) + acceptOverlaySelection(shell) + expect(opened).toEqual(["https://notion.test/auth"]) + expect(shell.statusFlash).toContain("notion") + }) + }) + + test("Enter on a connected server does nothing", async () => { + await withShell((shell) => { + const opened: string[] = [] + openCommandSurface(shell, "mcp", { + notify: () => {}, + mcp: { list: () => entries, openAuthURL: (url) => opened.push(url) }, + }) + acceptOverlaySelection(shell) + expect(opened).toEqual([]) + }) + }) + + test("reports the gap when the session has no mcp deps", async () => { + await withShell((shell) => { + const notes: string[] = [] + openCommandSurface(shell, "mcp", { notify: (t) => notes.push(t) }) + expect(notes[0]).toContain("not available") + }) + }) +}) + describe("model surface", () => { test("routes to the host picker, and reports the gap when absent", async () => { await withShell((shell) => { diff --git a/src/tui-opentui/command-surfaces.ts b/src/tui-opentui/command-surfaces.ts index 8374faf7c..fcb91fa19 100644 --- a/src/tui-opentui/command-surfaces.ts +++ b/src/tui-opentui/command-surfaces.ts @@ -18,6 +18,7 @@ import { openHelpOverlay, openListOverlay, openSettingsOverlay, + setStatusFlash, type AppShell, type ItemDescription, type OverlaySelection, @@ -118,6 +119,24 @@ export type HooksSurfaceDeps = { readonly setEnabled: (id: string, enabled: boolean) => Promise | void } +/** A configured MCP server and its live connection state. */ +export type McpEntry = { + readonly name: string + readonly state: "connecting" | "connected" | "needs-auth" | "failed" + /** Tool count once connected. */ + readonly toolCount?: number + /** Authorization URL while `needs-auth`. */ + readonly authURL?: string + /** Failure reason while `failed`. */ + readonly error?: string +} + +export type McpSurfaceDeps = { + readonly list: () => readonly McpEntry[] + /** Open the server's authorization URL in the operator's browser. */ + readonly openAuthURL: (url: string) => void +} + /** Live summary for the settings surface's hooks row (owned by another surface). */ export type HooksSurfaceSummary = { readonly discovered: number @@ -143,6 +162,7 @@ export type CommandSurfaceDeps = { readonly permissions?: PermissionsSurfaceDeps readonly plugins?: PluginsSurfaceDeps readonly hooks?: HooksSurfaceDeps + readonly mcp?: McpSurfaceDeps readonly settings?: SettingsSurfaceDeps /** Opens the host's model/provider picker (owned by the product host). */ readonly openModels?: () => void @@ -157,6 +177,7 @@ export type CommandSurfaceKind = | "permissions" | "plugins" | "hooks" + | "mcp" | "models" const CLOSE_ID = "__close__" @@ -882,6 +903,82 @@ export function openHooksSurface(shell: AppShell, deps: CommandSurfaceDeps): voi }) } +export function mcpRowLabel(entry: McpEntry): string { + switch (entry.state) { + case "connecting": + return `${entry.name} — connecting` + case "connected": { + const n = entry.toolCount ?? 0 + return `${entry.name} — connected · ${n} tool${n === 1 ? "" : "s"}` + } + case "needs-auth": + return `${entry.name} — needs auth` + case "failed": + return `${entry.name} — failed` + } +} + +function mcpDescription(entry: McpEntry): ItemDescription { + switch (entry.state) { + case "connecting": + return { what: "Connecting — its tools are not dispatchable yet." } + case "connected": + return { what: "Connected. Its tools are reachable through tool_search." } + case "needs-auth": + return { + what: "Authorization has not completed, so this server contributes no tools.", + impact: "Enter opens the authorization page and copies the link.", + } + case "failed": + return { what: entry.error ?? "Did not connect.", tone: "consequence" } + } +} + +/** Configured MCP servers and their live state; Enter authorizes an unauthorized one. */ +export function openMcpSurface(shell: AppShell, deps: CommandSurfaceDeps): void { + const mcp = deps.mcp + if (mcp === undefined) { + deps.notify("MCP administration is not available in this session.") + return + } + closeInsetOverlay(shell) + const entries = mcp.list() + const rows: ResidualCatalogEntry[] = entries.map((e) => ({ id: e.name, label: mcpRowLabel(e) })) + if (rows.length === 0) { + rows.push({ id: CLOSE_ID, label: "No MCP servers configured" }) + } + rows.push({ id: CLOSE_ID, label: "Close mcp" }) + const byName = new Map(entries.map((e) => [e.name, e])) + openListOverlay(shell, { + kind: "mcp", + title: "mcp", + frameId: "overlay-mcp", + ...payload(rows), + describe: (id) => { + const target = byName.get(id) + return target === undefined ? null : mcpDescription(target) + }, + onAccept: (selection) => { + const id = selectedId(selection, rows) + if (id === undefined || id === CLOSE_ID) return + const target = byName.get(id) + const url = target?.authURL + if (target === undefined || target.state !== "needs-auth" || url === undefined) return + mcp.openAuthURL(url) + // The copy is the fallback that makes this work over SSH, where the + // browser that must receive the redirect is not on this machine. + void shell.clipboard.writeText(url) + closeInsetOverlay(shell) + setStatusFlash(shell, `opening ${target.name} authorization — link copied`, { + ttlMs: MCP_AUTH_FLASH_MS, + }) + }, + }) +} + +/** Long enough to notice the browser was asked to open, and why. */ +const MCP_AUTH_FLASH_MS = 6000 + function errorText(err: unknown): string { return err instanceof Error ? err.message : String(err) } @@ -912,6 +1009,9 @@ export function openCommandSurface( case "hooks": openHooksSurface(shell, deps) return true + case "mcp": + openMcpSurface(shell, deps) + return true case "models": if (deps.openModels === undefined) return false deps.openModels() diff --git a/src/tui-opentui/notice-line.test.ts b/src/tui-opentui/notice-line.test.ts index df44f37b7..3b75d1782 100644 --- a/src/tui-opentui/notice-line.test.ts +++ b/src/tui-opentui/notice-line.test.ts @@ -8,10 +8,16 @@ const state = (over: Partial = {}): NoticeState => ({ pinned: false, flash: null, attachments: 0, + mcpNeedsAuth: 0, ...over, }) describe("composeNoticeLine", () => { + test("unauthorized mcp servers hold a standing segment pointing at /mcp", () => { + expect(composeNoticeLine(state({ mcpNeedsAuth: 1 }))).toBe("1 mcp needs auth (/mcp)") + expect(composeNoticeLine(state({ mcpNeedsAuth: 2 }))).toBe("2 mcp need auth (/mcp)") + }) + test("an idle shell has nothing to say and takes no row", () => { expect(composeNoticeLine(state())).toBe("") }) diff --git a/src/tui-opentui/notice-line.ts b/src/tui-opentui/notice-line.ts index eda8fa083..c428c34de 100644 --- a/src/tui-opentui/notice-line.ts +++ b/src/tui-opentui/notice-line.ts @@ -27,6 +27,8 @@ export type NoticeState = { /** Transient feedback (copy result, attach failure, exit arming). */ readonly flash: string | null readonly attachments: number + /** MCP servers connected but unauthorized; their tools stay unavailable. */ + readonly mcpNeedsAuth: number } /** @@ -43,6 +45,11 @@ export function composeNoticeLine(state: NoticeState): string { `${state.attachments} image${state.attachments === 1 ? "" : "s"}`, ) } + if (state.mcpNeedsAuth > 0) { + segments.push( + `${state.mcpNeedsAuth} mcp need${state.mcpNeedsAuth === 1 ? "s" : ""} auth (/mcp)`, + ) + } const flash = state.flash?.trim() ?? "" if (flash.length > 0) segments.push(flash) return segments.join(SEP) diff --git a/src/tui-opentui/product-host.ts b/src/tui-opentui/product-host.ts index 001a9c2ed..9a717aa32 100644 --- a/src/tui-opentui/product-host.ts +++ b/src/tui-opentui/product-host.ts @@ -44,6 +44,7 @@ import { setHeader, setPaletteCatalog, setPaletteOnCommand, + setMcpNeedsAuth, setStatusFlash, type AppShell, type ItemDescription, @@ -328,6 +329,9 @@ export async function mountProductHost( resolveExit?.() } + // Servers that announced an authorization URL and have not connected since. + const mcpUnauthorized = new Set() + function onEvent(event: unknown): void { if (disposed) return if ( @@ -358,7 +362,11 @@ export async function mountProductHost( function onMcpStatus(state: unknown): void { if (disposed) return const parsed = mcpServerState(state) - if (parsed !== null) show(mcpNotice(parsed)) + if (parsed === null) return + if (parsed.state === "needs-auth") mcpUnauthorized.add(parsed.name) + else mcpUnauthorized.delete(parsed.name) + setMcpNeedsAuth(shell, mcpUnauthorized.size) + show(mcpNotice(parsed)) } function onPermissionGrant(payload: unknown): void { diff --git a/src/tui-opentui/runtime-channels.test.ts b/src/tui-opentui/runtime-channels.test.ts index ed7bd63d3..fb3a3d8c0 100644 --- a/src/tui-opentui/runtime-channels.test.ts +++ b/src/tui-opentui/runtime-channels.test.ts @@ -98,8 +98,8 @@ describe("hook channel", () => { }) describe("mcp.status channel", () => { - test("a server awaiting authorization keeps a transcript row with its url", async () => { - const { emitter, frame, cleanup } = await mountHeadless() + test("a server awaiting authorization takes a notice segment, not a transcript row", async () => { + const { host, emitter, frame, cleanup } = await mountHeadless() try { emitter.emit("mcp.status", { name: "linear", @@ -107,8 +107,21 @@ describe("mcp.status channel", () => { url: "https://mcp.test/auth", }) const painted = await frame() - expect(painted).toContain("mcp linear needs authorization") - expect(painted).toContain("https://mcp.test/auth") + expect(painted).toContain("1 mcp needs auth (/mcp)") + expect(painted).not.toContain("https://mcp.test/auth") + expect(host.shell.streamLog).toEqual([]) + } finally { + cleanup() + } + }) + + test("connecting clears the standing auth segment", async () => { + const { host, emitter, frame, cleanup } = await mountHeadless() + try { + emitter.emit("mcp.status", { name: "linear", state: "needs-auth", url: "https://x/a" }) + emitter.emit("mcp.status", { name: "linear", state: "connected", tools: ["a"] }) + await frame() + expect(host.shell.mcpNeedsAuth).toBe(0) } finally { cleanup() } diff --git a/src/tui-opentui/runtime-notices.test.ts b/src/tui-opentui/runtime-notices.test.ts index c98809eec..29ae90737 100644 --- a/src/tui-opentui/runtime-notices.test.ts +++ b/src/tui-opentui/runtime-notices.test.ts @@ -83,13 +83,10 @@ describe("mcpNotice", () => { ).toEqual({ kind: "flash", text: "mcp linear connected · 2 tools" }) }) - test("needs-auth keeps a row with the url", () => { + test("needs-auth says nothing — the notice row and /mcp own it", () => { expect( mcpNotice({ name: "linear", state: "needs-auth", url: "https://x/auth" }), - ).toEqual({ - kind: "row", - text: "mcp linear needs authorization — open https://x/auth", - }) + ).toBeNull() }) test("failure keeps a row saying what was lost", () => { diff --git a/src/tui-opentui/runtime-notices.ts b/src/tui-opentui/runtime-notices.ts index 044d26259..d8a9ab830 100644 --- a/src/tui-opentui/runtime-notices.ts +++ b/src/tui-opentui/runtime-notices.ts @@ -73,8 +73,9 @@ export function hookNotice(event: LifecycleHookEvent): RuntimeNotice | null { /** * MCP connection state. Reconnect chatter is noise on every server every run; - * a server waiting on authorization or refusing to connect changes what the - * agent can do, so it keeps a row. + * a server refusing to connect changes what the agent can do, so it keeps a + * row. A server waiting on authorization is a standing condition with an + * action attached, which is the notice row's and /mcp's job, not a row's. */ export function mcpNotice(state: MCPServerState): RuntimeNotice | null { switch (state.state) { @@ -87,11 +88,10 @@ export function mcpNotice(state: MCPServerState): RuntimeNotice | null { text: `mcp ${state.name} connected · ${n} tool${n === 1 ? "" : "s"}`, } } + // A raw authorization URL in the transcript is unactionable and scrolls + // away. The notice row counts these and /mcp does the authorizing. case "needs-auth": - return { - kind: "row", - text: `mcp ${state.name} needs authorization — open ${state.url}`, - } + return null case "failed": return { kind: "row", diff --git a/src/tui-opentui/shell.ts b/src/tui-opentui/shell.ts index 7682fcc75..f5875510b 100644 --- a/src/tui-opentui/shell.ts +++ b/src/tui-opentui/shell.ts @@ -607,6 +607,8 @@ export type AppShell = { * set to null; never appended to the stream log. */ statusFlash: string | null + /** MCP servers awaiting authorization; the notice row counts them. */ + mcpNeedsAuth: number /** * Live turn phase ("Thinking…", "Running tool…", …) or null when idle. * Lives on the transient notice row rather than a chrome zone because the product host @@ -677,6 +679,7 @@ export type PrimaryOverlayKind = | "mentions" | "copy" | "hooks" + | "mcp" | "plugin_credentials" const DEFAULT_TITLE = "corbits" @@ -736,9 +739,17 @@ export function noticeText(shell: AppShell): string { pinned: !isTranscriptFollowing(shell), flash: shell.statusFlash, attachments: shell.pendingAttachments.length, + mcpNeedsAuth: shell.mcpNeedsAuth, }) } +/** How many MCP servers are waiting on authorization. Repaints on change. */ +export function setMcpNeedsAuth(shell: AppShell, count: number): void { + if (shell.mcpNeedsAuth === count) return + shell.mcpNeedsAuth = count + paintChrome(shell) +} + /** Repaint the prompt borders and the transient notice row from live state. */ export function paintChrome(shell: AppShell): void { if (shell.disposed) return @@ -4859,6 +4870,7 @@ export function createAppShell( mouseCapture: options?.mouseCapture ?? null, copyTargets: null, statusFlash: null, + mcpNeedsAuth: 0, turnPhase: null, lockupNowMs: 0, lockupAnimating: false, diff --git a/src/tui/commands/built-in.ts b/src/tui/commands/built-in.ts index 2134ac38f..f84102951 100644 --- a/src/tui/commands/built-in.ts +++ b/src/tui/commands/built-in.ts @@ -186,18 +186,8 @@ export function registerBuiltInCommands(): void { registerCommand({ name: "mcp", - description: "List connected MCP servers and their available tools", - handler: (_args, ctx) => { - const servers = ctx.getMCPServers?.() ?? []; - if (servers.length === 0) { - return { type: "message", text: "No MCP servers connected. Add mcpServers to .corbits/settings.json." }; - } - const lines = servers.map((s) => { - const toolList = s.tools.length > 0 ? s.tools.join(", ") : "(no tools)"; - return `${s.name}: ${toolList}`; - }); - return { type: "message", text: lines.join("\n") }; - }, + description: "Show MCP servers and authorize the ones that need it", + handler: (_args, _ctx) => ({ type: "overlay", overlay: "mcp" }), }); registerCommand({ diff --git a/src/tui/commands/registry.ts b/src/tui/commands/registry.ts index 5c9187973..b97b073a9 100644 --- a/src/tui/commands/registry.ts +++ b/src/tui/commands/registry.ts @@ -4,7 +4,6 @@ import type { CostSummary } from "../../cost/cost-summary.js"; export type CommandContext = { signalClear: () => void; - getMCPServers?: () => Array<{ name: string; tools: string[] }>; getCostSummary?: () => CostSummary; // Start a workflow by name; returns a status message to surface to the user. startWorkflow?: (name: string) => string; @@ -27,7 +26,7 @@ export type CommandResult = | { type: "message"; text: string } | { type: "send"; text: string } | { type: "view"; view: "tasks" } - | { type: "overlay"; overlay: "help" | "permissions" | "plugins" | "settings" | "hooks" } + | { type: "overlay"; overlay: "help" | "permissions" | "plugins" | "settings" | "hooks" | "mcp" } | { type: "modal"; modal: "agent" | "codex-login" | "xai-login" } | { type: "workflow"; name: string; args?: string } | { type: "paste-image" } diff --git a/src/tui/runner.ts b/src/tui/runner.ts index 543324c8e..7291fbe30 100644 --- a/src/tui/runner.ts +++ b/src/tui/runner.ts @@ -123,7 +123,7 @@ import { isGoalApprovalTimeoutActive, } from "../permission/goal-approval-timeout.js"; -import { createAgentToolset, type OperatorResult } from "../agent/tools.js"; +import { createAgentToolset, type MCPServerState, type OperatorResult } from "../agent/tools.js"; import { collectWebPlugins, resolveWebProviderFromPlugins, webBrand } from "../web/plugin-provider.js"; import { collectToolPlugins, resolveToolPlugins } from "../plugins/tool-plugins.js"; import { scrubSecrets } from "../web/secret-scrub.js"; @@ -162,6 +162,7 @@ import { createRunSink } from "../session/run-sink.js"; import { generateSessionId, initSessionDir, renameSession, sessionContextDir, sessionDir } from "../session/index.js"; import { resolveSessionLabel, truncateSessionLabel } from "../session/session-label.js"; import { loadState, saveState, type ConnectedMcpServer, type RunState } from "../session/state.js"; +import { openInBrowser } from "../auth/oauth/browser.js"; import { pickSession } from "./pick-session.js"; import { RESUME_TRANSCRIPT_BLOCK_LIMIT, turnsToContentBlocks } from "./turns-to-blocks.js"; import { WorkflowController } from "./workflow-controller.js"; @@ -1324,6 +1325,10 @@ export async function runTUI(initialConfig: Config): Promise { // MCP servers connected so far, keyed by name so a reconnect after a failure // replaces rather than duplicates the entry. let connectedMcpServers: ConnectedMcpServer[] = []; + // Every configured server's latest state, for the /mcp surface. Unlike + // `connectedMcpServers` (persisted run metadata) this keeps the ones that + // failed or are still waiting on authorization. + const mcpStates = new Map(); const writeRunSnapshot = async ( status: RunState["status"], @@ -1676,7 +1681,6 @@ export async function runTUI(initialConfig: Config): Promise { const commandContext: CommandContext = { signalClear: newSession, - getMCPServers: () => connectedMcpServers.map((s) => ({ name: s.name, tools: [] })), getCostSummary: (): CostSummary => { const usage = runSink.getTokenUsage(); const lastTurnUsage = runSink.getLastTurnUsage(); @@ -2016,6 +2020,17 @@ export async function runTUI(initialConfig: Config): Promise { currentWebProvider: () => pluginsAdmin.getWebOverride(), setWebProvider: (id) => pluginsAdmin.setWebOverride(id), }, + mcp: { + list: () => + [...mcpStates.values()].map((status) => ({ + name: status.name, + state: status.state, + ...(status.state === "connected" ? { toolCount: status.tools.length } : {}), + ...(status.state === "needs-auth" ? { authURL: status.url } : {}), + ...(status.state === "failed" ? { error: status.error } : {}), + })), + openAuthURL: (url) => openInBrowser(url), + }, hooks: { list: () => hookManager.getStatuses().map((status) => ({ @@ -2148,6 +2163,7 @@ export async function runTUI(initialConfig: Config): Promise { .connectMCP( { onStatus: (status) => { + mcpStates.set(status.name, status); emitter.emit("mcp.status", status); if (status.state === "connected") { connectedMcpServers = [ From 2f1150447e47e87b8d473992344c47d137d42df7 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Thu, 6 Aug 2026 18:03:47 -0700 Subject: [PATCH 2/4] Name the MCP servers the notice row is talking about A bare count sent the operator to /mcp to find out which server it meant, and with more than one configured it read as a claim about whichever server they saw there first. The segment now names them, and only falls back to counting past the second. --- src/tui-opentui/notice-line.test.ts | 18 +++++++++++++---- src/tui-opentui/notice-line.ts | 25 ++++++++++++++++++------ src/tui-opentui/product-host.ts | 2 +- src/tui-opentui/runtime-channels.test.ts | 4 ++-- src/tui-opentui/shell.ts | 20 ++++++++++++------- 5 files changed, 49 insertions(+), 20 deletions(-) diff --git a/src/tui-opentui/notice-line.test.ts b/src/tui-opentui/notice-line.test.ts index 3b75d1782..53296ba83 100644 --- a/src/tui-opentui/notice-line.test.ts +++ b/src/tui-opentui/notice-line.test.ts @@ -8,14 +8,24 @@ const state = (over: Partial = {}): NoticeState => ({ pinned: false, flash: null, attachments: 0, - mcpNeedsAuth: 0, + mcpNeedsAuth: [], ...over, }) describe("composeNoticeLine", () => { - test("unauthorized mcp servers hold a standing segment pointing at /mcp", () => { - expect(composeNoticeLine(state({ mcpNeedsAuth: 1 }))).toBe("1 mcp needs auth (/mcp)") - expect(composeNoticeLine(state({ mcpNeedsAuth: 2 }))).toBe("2 mcp need auth (/mcp)") + test("the standing mcp segment names the servers it means", () => { + expect(composeNoticeLine(state({ mcpNeedsAuth: ["granola"] }))).toBe( + "mcp granola needs auth (/mcp)", + ) + expect(composeNoticeLine(state({ mcpNeedsAuth: ["linear", "granola"] }))).toBe( + "mcp granola, linear needs auth (/mcp)", + ) + }) + + test("past two servers the segment counts the rest rather than growing", () => { + expect( + composeNoticeLine(state({ mcpNeedsAuth: ["d", "a", "c", "b"] })), + ).toBe("mcp a, b +2 needs auth (/mcp)") }) test("an idle shell has nothing to say and takes no row", () => { diff --git a/src/tui-opentui/notice-line.ts b/src/tui-opentui/notice-line.ts index c428c34de..da0717c14 100644 --- a/src/tui-opentui/notice-line.ts +++ b/src/tui-opentui/notice-line.ts @@ -27,8 +27,23 @@ export type NoticeState = { /** Transient feedback (copy result, attach failure, exit arming). */ readonly flash: string | null readonly attachments: number - /** MCP servers connected but unauthorized; their tools stay unavailable. */ - readonly mcpNeedsAuth: number + /** Names of MCP servers still unauthorized; their tools stay unavailable. */ + readonly mcpNeedsAuth: readonly string[] +} + +/** How many server names the segment spells out before it counts instead. */ +const MCP_NAMES_SHOWN = 2 + +/** + * Name the unauthorized servers rather than counting them: a bare count sends + * the operator to /mcp to find out which one it meant, and reads as a claim + * about whichever server they see there first. + */ +function mcpAuthNames(names: readonly string[]): string { + const sorted = [...names].sort() + if (sorted.length <= MCP_NAMES_SHOWN) return `mcp ${sorted.join(", ")}` + const shown = sorted.slice(0, MCP_NAMES_SHOWN).join(", ") + return `mcp ${shown} +${sorted.length - MCP_NAMES_SHOWN}` } /** @@ -45,10 +60,8 @@ export function composeNoticeLine(state: NoticeState): string { `${state.attachments} image${state.attachments === 1 ? "" : "s"}`, ) } - if (state.mcpNeedsAuth > 0) { - segments.push( - `${state.mcpNeedsAuth} mcp need${state.mcpNeedsAuth === 1 ? "s" : ""} auth (/mcp)`, - ) + if (state.mcpNeedsAuth.length > 0) { + segments.push(`${mcpAuthNames(state.mcpNeedsAuth)} needs auth (/mcp)`) } const flash = state.flash?.trim() ?? "" if (flash.length > 0) segments.push(flash) diff --git a/src/tui-opentui/product-host.ts b/src/tui-opentui/product-host.ts index 9a717aa32..b2a7f78f1 100644 --- a/src/tui-opentui/product-host.ts +++ b/src/tui-opentui/product-host.ts @@ -365,7 +365,7 @@ export async function mountProductHost( if (parsed === null) return if (parsed.state === "needs-auth") mcpUnauthorized.add(parsed.name) else mcpUnauthorized.delete(parsed.name) - setMcpNeedsAuth(shell, mcpUnauthorized.size) + setMcpNeedsAuth(shell, [...mcpUnauthorized]) show(mcpNotice(parsed)) } diff --git a/src/tui-opentui/runtime-channels.test.ts b/src/tui-opentui/runtime-channels.test.ts index fb3a3d8c0..1c8daeb59 100644 --- a/src/tui-opentui/runtime-channels.test.ts +++ b/src/tui-opentui/runtime-channels.test.ts @@ -107,7 +107,7 @@ describe("mcp.status channel", () => { url: "https://mcp.test/auth", }) const painted = await frame() - expect(painted).toContain("1 mcp needs auth (/mcp)") + expect(painted).toContain("mcp linear needs auth (/mcp)") expect(painted).not.toContain("https://mcp.test/auth") expect(host.shell.streamLog).toEqual([]) } finally { @@ -121,7 +121,7 @@ describe("mcp.status channel", () => { emitter.emit("mcp.status", { name: "linear", state: "needs-auth", url: "https://x/a" }) emitter.emit("mcp.status", { name: "linear", state: "connected", tools: ["a"] }) await frame() - expect(host.shell.mcpNeedsAuth).toBe(0) + expect(host.shell.mcpNeedsAuth).toEqual([]) } finally { cleanup() } diff --git a/src/tui-opentui/shell.ts b/src/tui-opentui/shell.ts index f5875510b..e5bfad1d6 100644 --- a/src/tui-opentui/shell.ts +++ b/src/tui-opentui/shell.ts @@ -607,8 +607,8 @@ export type AppShell = { * set to null; never appended to the stream log. */ statusFlash: string | null - /** MCP servers awaiting authorization; the notice row counts them. */ - mcpNeedsAuth: number + /** MCP servers awaiting authorization; the notice row names them. */ + mcpNeedsAuth: readonly string[] /** * Live turn phase ("Thinking…", "Running tool…", …) or null when idle. * Lives on the transient notice row rather than a chrome zone because the product host @@ -743,10 +743,16 @@ export function noticeText(shell: AppShell): string { }) } -/** How many MCP servers are waiting on authorization. Repaints on change. */ -export function setMcpNeedsAuth(shell: AppShell, count: number): void { - if (shell.mcpNeedsAuth === count) return - shell.mcpNeedsAuth = count +/** Which MCP servers are waiting on authorization. Repaints on change. */ +export function setMcpNeedsAuth(shell: AppShell, names: readonly string[]): void { + const next = [...names] + if ( + shell.mcpNeedsAuth.length === next.length && + next.every((name) => shell.mcpNeedsAuth.includes(name)) + ) { + return + } + shell.mcpNeedsAuth = next paintChrome(shell) } @@ -4870,7 +4876,7 @@ export function createAppShell( mouseCapture: options?.mouseCapture ?? null, copyTargets: null, statusFlash: null, - mcpNeedsAuth: 0, + mcpNeedsAuth: [], turnPhase: null, lockupNowMs: 0, lockupAnimating: false, From 97b239d996aa10a53a616ebd00e2e4dddd31b97b Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Thu, 6 Aug 2026 18:24:33 -0700 Subject: [PATCH 3/4] Brand the OAuth callback page and drop the stale overlay echo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- src/auth/callback-page.test.ts | 49 ++++ src/auth/callback-page.ts | 283 +++++++++++++++++++++++ src/auth/oauth/callback-server.ts | 10 +- src/mcp/callback-server.ts | 23 +- src/mcp/client.ts | 2 +- src/tui-opentui/command-surfaces.test.ts | 3 + src/tui-opentui/command-surfaces.ts | 3 + src/tui-opentui/shell.ts | 27 ++- 8 files changed, 377 insertions(+), 23 deletions(-) create mode 100644 src/auth/callback-page.test.ts create mode 100644 src/auth/callback-page.ts diff --git a/src/auth/callback-page.test.ts b/src/auth/callback-page.test.ts new file mode 100644 index 000000000..695ab46f4 --- /dev/null +++ b/src/auth/callback-page.test.ts @@ -0,0 +1,49 @@ +import { describe, expect, test } from "bun:test"; + +import { callbackPageHtml, humanizeIdentifier } from "./callback-page.js"; + +describe("humanizeIdentifier", () => { + test("machine identifiers lose their separators and lead with a capital", () => { + expect(humanizeIdentifier("access_denied")).toBe("Access denied"); + expect(humanizeIdentifier("granola")).toBe("Granola"); + expect(humanizeIdentifier("claude-ai-gamma")).toBe("Claude ai gamma"); + expect(humanizeIdentifier("googleDrive")).toBe("Google Drive"); + }); + + test("an empty identifier is returned untouched rather than as a stray capital", () => { + expect(humanizeIdentifier("")).toBe(""); + }); +}); + +describe("callbackPageHtml", () => { + test("success names the server that connected", () => { + const html = callbackPageHtml({ subject: "linear" }); + expect(html).toContain("Linear connected successfully"); + expect(html).not.toContain("access_denied"); + }); + + test("failure names the server and the humanized reason", () => { + const html = callbackPageHtml({ subject: "granola", error: "access_denied" }); + expect(html).toContain("Granola failed to connect"); + expect(html).toContain("Access denied."); + expect(html).not.toContain("access_denied"); + }); + + test("an unnamed authorization still renders both outcomes", () => { + expect(callbackPageHtml()).toContain("Authorization complete"); + expect(callbackPageHtml({ error: "server_error" })).toContain( + "Authorization did not complete", + ); + }); + + test("the subject is escaped rather than pasted into markup", () => { + expect(callbackPageHtml({ subject: "" })).not.toContain( + "`, + "", + ].join(""); +} diff --git a/src/auth/oauth/callback-server.ts b/src/auth/oauth/callback-server.ts index 4ce659998..326894d01 100644 --- a/src/auth/oauth/callback-server.ts +++ b/src/auth/oauth/callback-server.ts @@ -1,5 +1,5 @@ import { createServer, type Server } from "node:http"; -import { PRODUCT_NAME } from "../../branding.js"; +import { callbackPageHtml } from "../callback-page.js"; export type CallbackServer = { // Resolves with the validated authorization code once the browser redirects @@ -111,10 +111,6 @@ export async function startCallbackServer( }; } -export function authorizationDoneHtml(productName: string): string { - return ( - "Authorized" + - '' + - `

${productName} authorization complete

You can close this tab and return to ${PRODUCT_NAME}.

` - ); +export function authorizationDoneHtml(providerName: string): string { + return callbackPageHtml({ subject: providerName }); } diff --git a/src/mcp/callback-server.ts b/src/mcp/callback-server.ts index 66f600b8b..ea52651af 100644 --- a/src/mcp/callback-server.ts +++ b/src/mcp/callback-server.ts @@ -1,6 +1,6 @@ import { createServer, type Server } from "node:http"; import type { AddressInfo } from "node:net"; -import { PRODUCT_NAME } from "../branding.js"; +import { callbackPageHtml } from "../auth/callback-page.js"; export type CallbackServer = { // The redirect_uri to register with the authorization server. @@ -17,15 +17,12 @@ type CallbackWaiter = { resolve: (code: string) => void; reject: (error: Error) const CALLBACK_PATH = "/callback"; -const DONE_HTML = - "Authorized" + - "" + - `

Authorization complete

You can close this tab and return to ${PRODUCT_NAME}.

`; - -// Start an ephemeral loopback server to receive the OAuth redirect. Binds to a +// Start an ephemeral loopback server to receive the OAuth redirect. `serverName` +// only names the authorization on the page the browser lands on. +// Binds to a // random port on 127.0.0.1 so it never collides with anything and is only // reachable locally. -export async function startCallbackServer(): Promise { +export async function startCallbackServer(serverName?: string): Promise { let expectedState: string | undefined; let pendingResult: CallbackResult | undefined; let waiter: CallbackWaiter | undefined; @@ -61,9 +58,15 @@ export async function startCallbackServer(): Promise { const code = url.searchParams.get("code"); const error = url.searchParams.get("error"); - res.statusCode = error !== null || code === null ? 400 : 200; + const failure = error ?? (code === null ? "the redirect carried no code" : undefined); + res.statusCode = failure === undefined ? 200 : 400; res.setHeader("content-type", "text/html; charset=utf-8"); - res.end(error !== null || code === null ? `Authorization failed: ${error ?? "no code returned"}` : DONE_HTML); + res.end( + callbackPageHtml({ + ...(serverName !== undefined ? { subject: serverName } : {}), + ...(failure !== undefined ? { error: failure } : {}), + }), + ); if (error !== null) deliver({ error: new Error(`Authorization failed: ${error}`) }); else if (code === null) deliver({ error: new Error("Authorization redirect carried no code.") }); else deliver({ code }); diff --git a/src/mcp/client.ts b/src/mcp/client.ts index 3596a00b2..8f7bafe37 100644 --- a/src/mcp/client.ts +++ b/src/mcp/client.ts @@ -119,7 +119,7 @@ async function connectStdio(config: MCPServerConfig, options: MCPConnectOptions) async function connectHttp(config: MCPServerConfig, options: MCPConnectOptions): Promise { if (config.url === undefined) return { ok: false, serverName: config.name, error: "http MCP server requires a url" }; const url = new URL(config.url); - const callback = await startCallbackServer(); + const callback = await startCallbackServer(config.name); const authProvider = await createOAuthProvider({ serverName: config.name, redirectUrl: callback.redirectUrl, diff --git a/src/tui-opentui/command-surfaces.test.ts b/src/tui-opentui/command-surfaces.test.ts index bf7cc85d5..ae375ce5b 100644 --- a/src/tui-opentui/command-surfaces.test.ts +++ b/src/tui-opentui/command-surfaces.test.ts @@ -543,6 +543,9 @@ describe("mcp surface", () => { acceptOverlaySelection(shell) expect(opened).toEqual(["https://notion.test/auth"]) expect(shell.statusFlash).toContain("notion") + // The echo would quote "notion — needs auth" back forever, moments + // after the operator authorized it. + expect(shell.streamLog.filter((r) => r.meta === "overlay")).toEqual([]) }) }) diff --git a/src/tui-opentui/command-surfaces.ts b/src/tui-opentui/command-surfaces.ts index fcb91fa19..8f92d5296 100644 --- a/src/tui-opentui/command-surfaces.ts +++ b/src/tui-opentui/command-surfaces.ts @@ -953,6 +953,9 @@ export function openMcpSurface(shell: AppShell, deps: CommandSurfaceDeps): void kind: "mcp", title: "mcp", frameId: "overlay-mcp", + // The flash below reports the outcome; the echo would quote the row's + // pre-authorization label back at the operator forever. + echoChoice: false, ...payload(rows), describe: (id) => { const target = byName.get(id) diff --git a/src/tui-opentui/shell.ts b/src/tui-opentui/shell.ts index e5bfad1d6..e45110ad5 100644 --- a/src/tui-opentui/shell.ts +++ b/src/tui-opentui/shell.ts @@ -1682,6 +1682,8 @@ type ShellInternals = { overlayItemIds: readonly string[] /** Per-open accept callback; cleared on close without invoke (Esc path). */ overlayOnAccept: ((selection: OverlaySelection) => void) | null + /** False while an overlay that reports its own outcome is open. */ + overlayEchoChoice: boolean /** Per-open expand/collapse hook for the open primary overlay. */ overlayOnToggleExpand: (() => void) | null /** Per-open ← → cycle hook for the open primary overlay (settings inline cycling). */ @@ -2673,6 +2675,16 @@ export type OpenListOverlayOpts = { * nothing to choose, so the overlay is never a chooser with an empty list. */ readonly textAnswerActive?: boolean + /** + * Suppress the `chose (kind): label` transcript echo for this open. + * + * The echo exists so a choice with no other visible result still leaves a + * trace. A surface that reports the outcome itself does not need it, and the + * echo is worse than silent there: it quotes the row's label from *before* + * the action, so authorizing a server leaves a permanent line saying that + * server needs authorization. + */ + readonly echoChoice?: boolean } /** @@ -2732,6 +2744,7 @@ export function openListOverlay( if (!isPalette) { bag.overlayItemIds = opts?.itemIds ? [...opts.itemIds] : [] bag.overlayOnAccept = opts?.onAccept ?? null + bag.overlayEchoChoice = opts?.echoChoice ?? true bag.overlayOnToggleExpand = opts?.onToggleExpand ?? null bag.overlayOnCycle = opts?.onCycle ?? null bag.overlayDescribe = opts?.describe ?? null @@ -2740,6 +2753,7 @@ export function openListOverlay( // Bare palette (no primary under it): no accept payload. bag.overlayItemIds = opts?.itemIds ? [...opts.itemIds] : [] bag.overlayOnAccept = opts?.onAccept ?? null + bag.overlayEchoChoice = opts?.echoChoice ?? true bag.overlayOnToggleExpand = opts?.onToggleExpand ?? null bag.overlayOnCycle = opts?.onCycle ?? null bag.overlayDescribe = opts?.describe ?? null @@ -3353,11 +3367,13 @@ export function acceptOverlaySelection(shell: AppShell): void { // Capture before close clears per-open state. const perOpen = bag?.overlayOnAccept ?? null - appendStreamRow(shell, { - role: "system", - text: `chose (${kind}): ${label}`, - meta: "overlay", - }) + if (bag?.overlayEchoChoice !== false) { + appendStreamRow(shell, { + role: "system", + text: `chose (${kind}): ${label}`, + meta: "overlay", + }) + } closeInsetOverlay(shell) dispatchOverlayAccept(shell, selection, perOpen) } @@ -4918,6 +4934,7 @@ export function createAppShell( priorOverlay: null, overlayItemIds: [], overlayOnAccept: null, + overlayEchoChoice: true, overlayOnToggleExpand: null, overlayOnCycle: null, overlayDescribe: null, From 5e47ad8a5463c97c50c568c94a2c64d8031f764d Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Thu, 6 Aug 2026 18:26:00 -0700 Subject: [PATCH 4/4] Log the MCP authorization work in the 0.2.90 changelog and notes --- CHANGELOG.md | 20 ++++++++++++++++++++ docs/release-notes-0.2.90.md | 14 ++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 822ac2545..8193d9dc2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -68,6 +68,26 @@ unchanged: Ctrl+C interrupts, twice exits. - **Resumed sessions dropped `view`, `plan` and `tasks` blocks** silently. - **Ctrl+D quit mid-edit.** The host claims no key of its own now. +### MCP + +- **Authorization moved out of the transcript and into `/mcp`.** A server + needing OAuth used to dump a raw authorization URL as a transcript row at + session start — unactionable, uncopyable, and gone once it scrolled away. The + notice row now names the servers waiting (`mcp granola needs auth (/mcp)`) + and clears when they connect; nothing blocks usage, an unauthorized server + simply has no tools. (CL-5555) +- **`/mcp` is a real surface** listing every configured server and its live + state — connected with tool count, needs auth, or failed with the reason. + Enter on an unauthorized row opens its authorization page in the browser and + copies the link, so the flow also works over SSH. (CL-5555) +- **The OAuth callback page carries the brand.** One page now serves 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. It names what happened — "Linear connected successfully", "Granola + failed to connect" — and humanizes server names and error codes on the way + in. Entirely inline: a local authorization callback makes no network call. + (CL-5556) + ### Permissions - **Shell-block messaging** cites host safety and OOM risk, and names the diff --git a/docs/release-notes-0.2.90.md b/docs/release-notes-0.2.90.md index 0e0935ad8..7b8107859 100644 --- a/docs/release-notes-0.2.90.md +++ b/docs/release-notes-0.2.90.md @@ -26,6 +26,20 @@ rollback is the prior tag rather than a setting. - Live status for lifecycle hooks, subagent progress, MCP connections and recorded permission grants. +### Connecting an MCP server + +Remote MCP servers that need OAuth used to print a raw authorization URL into +the transcript the moment you started a session — nothing you could click, copy +or come back to. + +- Nothing blocks usage. A server waiting on authorization simply has no tools, + and the notice row names it: `mcp granola needs auth (/mcp)`. +- **`/mcp` lists every server** and its live state. Enter on one that needs + authorization opens the page in your browser and copies the link, so it works + over SSH too. +- **The page your browser lands on** now tells you which server connected, and + looks like the rest of Corbits rather than a browser default. + ### Fixed The ones most likely to have affected you: