fix(mcp): reproduce #86 — truthful logout, and a sign-in name that resolves

apps/app/src/app/constants.ts

@@ -1,5 +1,6 @@
 import type { ModelRef, SuggestedPlugin } from "./types";
 import { t } from "../i18n";
+import { deriveMcpServerName } from "./mcp-identity";
 import {
   BUILT_IN_LEGALWORK_EXTENSION_MANIFESTS,
   extensionContribution,
@@ -133,13 +134,9 @@ export function isBuiltInLegalWorkExtension(entry: Pick<McpDirectoryInfo, "kind"
 }
 
 /** Derive a safe MCP server name from a display name or explicit serverName. */
-export function getMcpServerName(entry: McpDirectoryInfo): string {
+export function getMcpServerName(entry: Pick<McpDirectoryInfo, "name" | "serverName">): string {
   if (entry.serverName) return entry.serverName;
-  return entry.name
-    .toLowerCase()
-    .replace(/[^a-z0-9_-]/g, "-")
-    .replace(/-+/g, "-")
-    .replace(/^-|-$/g, "") || "mcp";
+  return deriveMcpServerName(entry.name);
 }
 
 export const MCP_QUICK_CONNECT: McpDirectoryInfo[] = [

apps/app/src/app/mcp-auth-state.ts

@@ -0,0 +1,89 @@
+/**
+ * Deciding whether logging an MCP server out actually did anything.
+ *
+ * Some remote MCP servers accept an unauthenticated handshake and reject only
+ * individual tool calls. The engine reports those as plain `connected`, exactly
+ * like an authenticated one — `McpStatusConnected` carries no auth state, and no
+ * endpoint reports stored credentials. So a connector that was never signed in
+ * shows as Ready, "Log out" clears a credential that does not exist, the badge
+ * drops to Paused, and the next launch silently restores Ready. Read as a stale
+ * session surviving a logout, and reported that way in issue #86.
+ *
+ * The signal is to reconnect once the credentials are gone — the same thing the
+ * next launch does, only now rather than later. A server that needed them comes
+ * back asking to sign in; one that never did comes back connected. Doing it
+ * during logout also leaves the badge showing the truth immediately, instead of
+ * a Paused state that quietly flips to Ready on restart.
+ */
+
+export type McpStatusSnapshot = Record<string, { status?: string } | undefined>;
+
+/** Delays between status reads after the reconnect, in ms. */
+export const RECONNECT_PROBE_DELAYS_MS = [250, 500, 1000, 1500];
+
+export type DetectReconnectOptions = {
+  /** Server name as the engine knows it. */
+  serverName: string;
+  /** Ask the engine to reconnect the server (POST /mcp/{name}/connect). */
+  reconnect: () => Promise<unknown>;
+  /** Read current MCP statuses; return null when unavailable. */
+  readStatus: () => Promise<McpStatusSnapshot | null>;
+  /** Called with each snapshot read, so callers can keep their UI current. */
+  onStatus?: (statuses: McpStatusSnapshot) => void;
+  /** Injected for tests; defaults to a real timer. */
+  wait?: (ms: number) => Promise<void>;
+  /** Abort early, e.g. when the store has been disposed. */
+  isCancelled?: () => boolean;
+  delaysMs?: number[];
+};
+
+/**
+ * True when the server reconnects successfully after its credentials were
+ * removed — meaning it never authenticated and the logout cleared nothing.
+ *
+ * False for every other outcome, including a failed probe or an unreadable
+ * status. This picks which message a user is shown, so an uncertain answer must
+ * fall back to the ordinary one rather than accuse a working logout of being a
+ * no-op.
+ */
+export async function detectReconnectWithoutAuth(options: DetectReconnectOptions): Promise<boolean> {
+  const {
+    serverName,
+    reconnect,
+    readStatus,
+    onStatus,
+    wait = (ms: number) => new Promise<void>((resolve) => setTimeout(resolve, ms)),
+    isCancelled,
+    delaysMs = RECONNECT_PROBE_DELAYS_MS,
+  } = options;
+
+  try {
+    await reconnect();
+  } catch {
+    // The probe is a diagnostic, never a reason to fail the logout itself.
+    return false;
+  }
+  if (isCancelled?.()) return false;
+
+  for (const delay of delaysMs) {
+    await wait(delay);
+    if (isCancelled?.()) return false;
+
+    let statuses: McpStatusSnapshot | null;
+    try {
+      statuses = await readStatus();
+    } catch {
+      return false;
+    }
+    if (!statuses) return false;
+    onStatus?.(statuses);
+
+    const current = statuses[serverName]?.status;
+    if (current === "connected") return true;
+    // The credentials mattered: the server is asking for them again. Nothing
+    // later in the poll can overturn that, so stop.
+    if (current === "needs_auth" || current === "needs_client_registration") return false;
+  }
+
+  return false;
+}

apps/app/src/app/mcp-identity.ts

@@ -0,0 +1,79 @@
+/**
+ * The one place an MCP server's identity is derived.
+ *
+ * A connector carries two names: the display name someone reads ("Microsoft
+ * SharePoint", "Iron Crow") and the server name the engine and opencode.jsonc
+ * know it by ("sharepoint", "iron-crow"). Connect writes the second one; every
+ * later operation — sign-in, logout, status lookup — has to ask for exactly
+ * that same string or it addresses a server that does not exist.
+ *
+ * Three separate derivations used to exist. The sign-in modal's rejected any
+ * display name that was not already a valid server name, so clicking "Log in"
+ * on SharePoint, Google Workspace, Thomson Reuters HighQ, or any connector
+ * someone had named with a space threw "server_name must be alphanumeric"
+ * before the browser was ever opened — the OAuth window simply never appeared
+ * (issue #86). Keeping the rule in one module is what stops that from
+ * reappearing: a display name is slugified here, never validated as if it were
+ * already a server name.
+ */
+
+/** Identity fields every connector shape carries, however it reached us. */
+export type McpIdentity = {
+  /** Explicit server name, when the catalog entry pins one. */
+  id?: string;
+  /** Safe server name for opencode.jsonc, when declared. */
+  serverName?: string;
+  /** Display name shown in the UI. */
+  name: string;
+};
+
+/**
+ * Slugify a display name into a server name opencode accepts: lowercase,
+ * alphanumerics plus `-` and `_`, no repeated or edge dashes.
+ */
+export function deriveMcpServerName(displayName: string): string {
+  return (
+    displayName
+      .toLowerCase()
+      .replace(/[^a-z0-9_-]/g, "-")
+      .replace(/-+/g, "-")
+      .replace(/^-|-$/g, "") || "mcp"
+  );
+}
+
+/**
+ * The server name this connector is registered under — the key in
+ * opencode.jsonc and the name the engine answers to.
+ */
+export function getMcpIdentityKey(entry: McpIdentity): string {
+  return entry.id ?? entry.serverName ?? deriveMcpServerName(entry.name);
+}
+
+/**
+ * Assert a string is already a valid opencode server name. Use it on a name
+ * that is meant to be one — a config key, a derived identity key — never on a
+ * display name, which needs `deriveMcpServerName` instead.
+ */
+export function validateMcpServerName(name: string): string {
+  const trimmed = name.trim();
+  if (!trimmed) {
+    throw new Error("server_name is required");
+  }
+  if (trimmed.startsWith("-")) {
+    throw new Error("server_name must not start with '-'");
+  }
+  if (!/^[A-Za-z0-9_-]+$/.test(trimmed)) {
+    throw new Error("server_name must be alphanumeric with '-' or '_'");
+  }
+  return trimmed;
+}
+
+/**
+ * The name to hand the engine when signing a connector in. This is the single
+ * function the OAuth modal uses; keeping the derivation and the validation
+ * composed here is what stops a display name from being validated as if it
+ * were a server name (issue #86).
+ */
+export function resolveMcpSignInName(entry: McpIdentity): string {
+  return validateMcpServerName(getMcpIdentityKey(entry));
+}

apps/app/src/app/mcp.ts

@@ -1,36 +1,23 @@
 import { applyEdits, modify, parse, printParseErrorCode } from "jsonc-parser";
 import type { McpServerConfig, McpServerEntry } from "./types";
 import { readOpencodeConfig, writeOpencodeConfig } from "./lib/desktop";
+import { deriveMcpServerName } from "./mcp-identity";
 
 type McpConfigValue = Record<string, unknown> | null | undefined;
 
-type McpIdentity = {
-  id?: string;
-  serverName?: string;
-  name: string;
-};
+export {
+  deriveMcpServerName,
+  getMcpIdentityKey,
+  resolveMcpSignInName,
+  validateMcpServerName,
+  type McpIdentity,
+} from "./mcp-identity";
 
-export function normalizeMcpSlug(name: string): string {
-  return name.toLowerCase().replace(/[^a-z0-9]+/g, "-");
-}
-
-export function getMcpIdentityKey(entry: McpIdentity): string {
-  return entry.id ?? entry.serverName ?? normalizeMcpSlug(entry.name);
-}
-
-export function validateMcpServerName(name: string): string {
-  const trimmed = name.trim();
-  if (!trimmed) {
-    throw new Error("server_name is required");
-  }
-  if (trimmed.startsWith("-")) {
-    throw new Error("server_name must not start with '-'");
-  }
-  if (!/^[A-Za-z0-9_-]+$/.test(trimmed)) {
-    throw new Error("server_name must be alphanumeric with '-' or '_'");
-  }
-  return trimmed;
-}
+/**
+ * Slugify a display name into a server name. Alias of `deriveMcpServerName`;
+ * the derivation lives in one module so connect and sign-in cannot drift apart.
+ */
+export const normalizeMcpSlug = deriveMcpServerName;
 
 export async function removeMcpFromConfig(
   projectDir: string,

apps/app/src/i18n/locales/en.ts

@@ -747,6 +747,7 @@ export default {
   "mcp.logout_label": "OAuth",
   "mcp.logout_modal_message": "This will remove stored OAuth credentials for {server}. You'll need to sign in again to use this app.",
   "mcp.logout_modal_title": "Log out of this app?",
+  "mcp.logout_no_credentials": "{server} connects without signing in, so there was nothing to log out of. It will show as Ready again, and any request it rejects has to be sorted out on the server itself.",
   "mcp.logout_success": "Logged out of {server}.",
   "mcp.logout_working": "Logging out...",
   "mcp.name_required": "Enter a server name.",

apps/app/src/react-app/domains/connections/mcp-auth-modal.tsx

@@ -14,7 +14,7 @@ import {
 import type { McpDirectoryInfo } from "@/app/constants";
 import { openDesktopUrl, opencodeMcpAuth } from "@/app/lib/desktop";
 import { unwrap } from "@/app/lib/opencode";
-import { validateMcpServerName } from "@/app/mcp";
+import { getMcpIdentityKey, resolveMcpSignInName } from "@/app/mcp";
 import type { Client } from "@/app/types";
 import { isDesktopRuntime, normalizeDirectoryPath } from "@/app/utils";
 import { t } from "@/i18n";
@@ -192,8 +192,14 @@ const humanizeEngineError = (message: string): string => {
   return looksLikePayload ? t("mcp.auth.connect_failed_generic") : message;
 };
 
-const resolveSlug = (name: string) =>
-    validateMcpServerName(name).toLowerCase().replace(/[^a-z0-9]+/g, "-");
+  /**
+   * The name the engine knows this connector by — the same key connect wrote to
+   * opencode.jsonc. Deriving it from the display name here instead used to
+   * reject anything with a space ("Microsoft SharePoint", "Iron Crow") before
+   * the authorization URL was ever requested, so the sign-in browser never
+   * opened; see app/mcp-identity.ts.
+   */
+  const resolveSlug = resolveMcpSignInName;
 
   // Drop any stored OAuth registration/tokens for this server. Best-effort:
   // used to recover from a stale Dynamic Client Registration that would
@@ -262,7 +268,7 @@ const resolveSlug = (name: string) =>
 
     let slug = "";
     try {
-      slug = resolveSlug(props.entry.name);
+      slug = resolveSlug(props.entry);
     } catch (err) {
       const message = err instanceof Error ? err.message : t("mcp.auth.failed_to_start_oauth");
       setError(message);
@@ -354,7 +360,7 @@ const resolveSlug = (name: string) =>
         setNeedsReload(false);
         setError(t("mcp.auth.slack_client_registration_required"));
       } else if (message.toLowerCase().includes("does not support oauth")) {
-        const serverSlug = props.entry.name.toLowerCase().replace(/[^a-z0-9]+/g, "-") || "server";
+        const serverSlug = slug || getMcpIdentityKey(props.entry);
         const canAutoReload =
           allowAutoReload && !props.isRemoteWorkspace && !props.reloadBlocked && Boolean(props.onReloadEngine);
 
@@ -475,7 +481,7 @@ const resolveSlug = (name: string) =>
         await props.onReloadEngine?.();
         if (cancelled) return;
 
-        const slug = resolveSlug(props.entry!.name);
+        const slug = resolveSlug(props.entry!);
         const status = await waitForMcpAvailability(slug);
         if (cancelled) return;
 
@@ -583,7 +589,7 @@ const resolveSlug = (name: string) =>
 
     let slug = "";
     try {
-      slug = resolveSlug(props.entry.name);
+      slug = resolveSlug(props.entry);
     } catch (err) {
       const message = err instanceof Error ? err.message : t("mcp.auth.failed_to_start_oauth");
       setError(message);
@@ -651,7 +657,7 @@ const resolveSlug = (name: string) =>
 
     let slug = "";
     try {
-      slug = resolveSlug(props.entry.name);
+      slug = resolveSlug(props.entry);
     } catch (err) {
       const message = err instanceof Error ? err.message : t("mcp.auth.failed_to_start_oauth");
       setError(message);