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: 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 26994da1f..ae375ce5b 100644 --- a/src/tui-opentui/command-surfaces.test.ts +++ b/src/tui-opentui/command-surfaces.test.ts @@ -514,6 +514,62 @@ 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") + // The echo would quote "notion — needs auth" back forever, moments + // after the operator authorized it. + expect(shell.streamLog.filter((r) => r.meta === "overlay")).toEqual([]) + }) + }) + + 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..8f92d5296 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,85 @@ 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", + // 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) + 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 +1012,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..53296ba83 100644 --- a/src/tui-opentui/notice-line.test.ts +++ b/src/tui-opentui/notice-line.test.ts @@ -8,10 +8,26 @@ const state = (over: Partial = {}): NoticeState => ({ pinned: false, flash: null, attachments: 0, + mcpNeedsAuth: [], ...over, }) describe("composeNoticeLine", () => { + 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", () => { expect(composeNoticeLine(state())).toBe("") }) diff --git a/src/tui-opentui/notice-line.ts b/src/tui-opentui/notice-line.ts index eda8fa083..da0717c14 100644 --- a/src/tui-opentui/notice-line.ts +++ b/src/tui-opentui/notice-line.ts @@ -27,6 +27,23 @@ export type NoticeState = { /** Transient feedback (copy result, attach failure, exit arming). */ readonly flash: string | null readonly attachments: 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}` } /** @@ -43,6 +60,9 @@ export function composeNoticeLine(state: NoticeState): string { `${state.attachments} image${state.attachments === 1 ? "" : "s"}`, ) } + 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) return segments.join(SEP) diff --git a/src/tui-opentui/product-host.ts b/src/tui-opentui/product-host.ts index 001a9c2ed..b2a7f78f1 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]) + 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..1c8daeb59 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("mcp linear 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).toEqual([]) } 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..e45110ad5 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 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 @@ -677,6 +679,7 @@ export type PrimaryOverlayKind = | "mentions" | "copy" | "hooks" + | "mcp" | "plugin_credentials" const DEFAULT_TITLE = "corbits" @@ -736,9 +739,23 @@ export function noticeText(shell: AppShell): string { pinned: !isTranscriptFollowing(shell), flash: shell.statusFlash, attachments: shell.pendingAttachments.length, + mcpNeedsAuth: shell.mcpNeedsAuth, }) } +/** 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) +} + /** Repaint the prompt borders and the transient notice row from live state. */ export function paintChrome(shell: AppShell): void { if (shell.disposed) return @@ -1665,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). */ @@ -2656,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 } /** @@ -2715,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 @@ -2723,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 @@ -3336,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) } @@ -4859,6 +4892,7 @@ export function createAppShell( mouseCapture: options?.mouseCapture ?? null, copyTargets: null, statusFlash: null, + mcpNeedsAuth: [], turnPhase: null, lockupNowMs: 0, lockupAnimating: false, @@ -4900,6 +4934,7 @@ export function createAppShell( priorOverlay: null, overlayItemIds: [], overlayOnAccept: null, + overlayEchoChoice: true, overlayOnToggleExpand: null, overlayOnCycle: null, overlayDescribe: null, 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 = [