From f068d9b733533197f6d4ef9914ae066114dc4f75 Mon Sep 17 00:00:00 2001
From: Mickael Farina <farina.mickael@gmail.com>
Date: Thu, 16 Jul 2026 12:39:54 +0200
Subject: [PATCH] feat(connector): real connect/disconnect state +
 Keychain-persistent tokens (beat 19)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The Connector tab was half-built: toggling on showed a cramped inline-styled
"Sign in" button jammed against the auth note, and after a SUCCESSFUL Notion
sign-in the card still said "Needs OAuth sign-in" — no connected state, no way
to disconnect, and the token died with the process.

Root cause of the missing state: fastmcp's OAuth defaulted to IN-MEMORY token
storage, so nothing persisted and nothing could be queried.

- mcp_connect: hand OAuth a Keychain-backed token store (KeyringStore, service
  ai.avadigital.codec.mcp_tokens; 0600 disk fallback for headless/CI). Adds
  is_connected(name) and disconnect_server(name) built on fastmcp's own
  TokenStorageAdapter (get_tokens / clear) — no guessing at its key scheme.
  Also adds the module-level logger the new code needs.
- routes/mcp: GET /api/mcp/servers now returns `connected` per ENABLED server
  (never leaks the token); new POST /api/mcp/servers/{name}/disconnect (audited).
- dashboard: proper state machine per card —
    off → (toggle only)
    on + needs auth + not connected → "Needs OAuth sign-in" + [Sign in]
    on + connected → "● Connected" + [Disconnect]
    on + no auth → Connected
  Real CSS classes (.mcp-state/.mcp-btn/.mcp-connected) matching the dashboard's
  tokens instead of ad-hoc inline styles, on its own row so nothing crowds the
  auth note. Toggling on re-renders immediately so Sign in appears at once.
  No emoji (CSS dot + line-SVG).

Manifest regenerated. 42 mcp tests pass, ruff clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 codec_dashboard.html  | 79 ++++++++++++++++++++++++++++++++++++-------
 routes/mcp.py         | 65 ++++++++++++++++++++++++++++-------
 skills/.manifest.json |  2 +-
 skills/mcp_connect.py | 78 +++++++++++++++++++++++++++++++++++++++++-
 4 files changed, 197 insertions(+), 27 deletions(-)

diff --git a/codec_dashboard.html b/codec_dashboard.html
index b5bf9c9..f254347 100644
--- a/codec_dashboard.html
+++ b/codec_dashboard.html
@@ -744,6 +744,18 @@ <h2>MENU</h2>
     .mcp-auth{display:inline-flex;align-items:center;gap:5px;font-size:11px;color:#d99a33;margin-top:5px}
     .mcp-auth svg{width:12px;height:12px;stroke:currentColor;fill:none;stroke-width:2;stroke-linecap:round;stroke-linejoin:round}
     .mcp-empty{font-size:12.5px;color:var(--text-muted);line-height:1.6;padding:22px 18px;text-align:center;border:1px dashed var(--border);border-radius:10px}
+    /* Connector state row: sits on its own line under the url so the button
+       never crowds the auth note (the old inline-styled button did). */
+    .mcp-state{display:flex;align-items:center;gap:10px;margin-top:8px;flex-wrap:wrap}
+    .mcp-btn{font:inherit;font-size:12px;font-weight:600;padding:5px 14px;border-radius:6px;
+      border:1px solid var(--border);background:var(--surface-3);color:var(--text);cursor:pointer;
+      transition:border-color .12s,background .12s}
+    .mcp-btn:hover:not(:disabled){border-color:var(--border-hover)}
+    .mcp-btn:disabled{opacity:.55;cursor:default}
+    .mcp-btn-primary{background:var(--accent);border-color:var(--accent);color:#fff}
+    .mcp-btn-primary:hover:not(:disabled){filter:brightness(1.08)}
+    .mcp-connected{display:inline-flex;align-items:center;gap:6px;font-size:11.5px;font-weight:600;color:#3fa45b}
+    .mcp-connected .dot{width:7px;height:7px;border-radius:50%;background:currentColor;flex:none}
     .mcp-switch{position:relative;display:inline-block;width:44px;height:24px;flex-shrink:0}
     .mcp-switch input{opacity:0;width:0;height:0}
     .mcp-slider{position:absolute;cursor:pointer;inset:0;background:var(--surface-3);border:1px solid var(--border);border-radius:24px;transition:.2s}
@@ -1078,23 +1090,38 @@ <h2>MENU</h2>
     box.innerHTML = servers.map(function(s) {
       var checked = s.enabled ? 'checked' : '';
       var badge = escHtml((s.transport || 'http').toUpperCase());
-      var authNote = '';
-      if (s.needs_auth) {
-        var kind = s.auth === 'api_key' ? 'API key' : (s.auth === 'oauth' ? 'OAuth sign-in' : 'sign-in');
-        authNote = '<div class="mcp-auth"><svg viewBox="0 0 24 24"><rect x="3" y="11" width="18" height="11" rx="2"/><path d="M7 11V7a5 5 0 0110 0v4"/></svg>Needs ' + kind + '</div>';
-      }
       var note = s.note ? '<div class="mcp-note">' + escHtml(s.note) + '</div>' : '';
-      // OAuth connectors get a "Sign in" button (only when enabled) that opens
-      // the browser authorize flow — toggling on alone doesn't authenticate.
-      var signinBtn = (s.enabled && s.needs_auth && s.auth === 'oauth') ?
-        '<button data-signin="' + escHtml(s.name) + '" style="margin-top:8px;padding:4px 12px;background:var(--accent,#f0641e);color:#fff;border:none;border-radius:6px;cursor:pointer;font-size:12px;font-weight:600">Sign in</button>' : '';
+      var lock = '<svg viewBox="0 0 24 24"><rect x="3" y="11" width="18" height="11" rx="2"/><path d="M7 11V7a5 5 0 0110 0v4"/></svg>';
+      var nm = escHtml(s.name);
+      // State machine, in the order a user actually experiences it:
+      //   off                      → nothing (just the toggle)
+      //   on + needs auth + !conn  → why + how to fix it (Sign in)
+      //   on + connected           → Connected + a way back out (Disconnect)
+      //   on + no auth needed      → Connected (nothing to sign into)
+      var state = '';
+      if (s.enabled && s.needs_auth && !s.connected) {
+        var kind = s.auth === 'api_key' ? 'API key' : 'OAuth sign-in';
+        state = '<div class="mcp-state">' +
+          '<span class="mcp-auth">' + lock + 'Needs ' + kind + '</span>' +
+          (s.auth === 'oauth'
+            ? '<button class="mcp-btn mcp-btn-primary" data-signin="' + nm + '">Sign in</button>'
+            : '') +
+          '</div>';
+      } else if (s.enabled && s.connected) {
+        state = '<div class="mcp-state">' +
+          '<span class="mcp-connected"><span class="dot"></span>Connected</span>' +
+          (s.auth === 'oauth'
+            ? '<button class="mcp-btn" data-disconnect="' + nm + '">Disconnect</button>'
+            : '') +
+          '</div>';
+      }
       return '<div class="mcp-card">' +
         '<div class="mcp-card-main">' +
-          '<div class="mcp-card-top"><span class="mcp-name">' + escHtml(s.name) + '</span>' +
+          '<div class="mcp-card-top"><span class="mcp-name">' + nm + '</span>' +
           '<span class="mcp-badge">' + badge + '</span></div>' +
           note +
           '<div class="mcp-url">' + escHtml(s.url || '') + '</div>' +
-          authNote + signinBtn +
+          state +
         '</div>' +
         '<label class="mcp-switch" title="Enable / disable connector">' +
           '<input type="checkbox" ' + checked + ' data-mcp="' + escHtml(s.name) + '">' +
@@ -1110,6 +1137,9 @@ <h2>MENU</h2>
     box.querySelectorAll('button[data-signin]').forEach(function(b) {
       b.addEventListener('click', function() { mcpSignin(this.dataset.signin, this); });
     });
+    box.querySelectorAll('button[data-disconnect]').forEach(function(b) {
+      b.addEventListener('click', function() { mcpDisconnect(this.dataset.disconnect, this); });
+    });
   } catch (e) {
     box.innerHTML = '<div class="mcp-empty">Could not load connectors: ' + escHtml(String(e)) + '</div>';
   }
@@ -1130,12 +1160,17 @@ <h2>MENU</h2>
     var d = await r.json().catch(function() { return {}; });
     if (!r.ok || d.ok === false) {
       el.checked = !enabled; // revert on failure
+      el.disabled = false;
+      return;
     }
+    // Re-render so the card moves to its next state immediately (toggling on an
+    // auth server must surface "Sign in" right away — that's the whole point).
+    renderConnectors();
+    return;
   } catch (e) {
     el.checked = !enabled; // revert on network error
-  } finally {
-    el.disabled = false;
   }
+  el.disabled = false;
 }
 
 // OAuth sign-in: opens the server's authorize page in the browser (fastmcp runs
@@ -1158,6 +1193,24 @@ <h2>MENU</h2>
   }
 }
 
+// Disconnect: forget the stored token. Card returns to "Sign in".
+async function mcpDisconnect(name, btn) {
+  btn.disabled = true; var orig = btn.textContent; btn.textContent = 'Disconnecting…';
+  try {
+    var csrf = (document.cookie.match(/codec_csrf=([^;]+)/) || [])[1] || '';
+    var r = await fetch('/api/mcp/servers/' + encodeURIComponent(name) + '/disconnect', {
+      method: 'POST', headers: {'Content-Type': 'application/json', 'x-csrf-token': csrf}
+    });
+    var d = await r.json().catch(function() { return {}; });
+    var okay = r.ok && d.ok !== false;
+    showToast(d.message || (okay ? 'Disconnected.' : 'Disconnect failed.'), !okay);
+    if (okay) { renderConnectors(); return; }
+  } catch (e) {
+    showToast('Disconnect failed: ' + e, true);
+  }
+  btn.disabled = false; btn.textContent = orig;
+}
+
 var _pendingImage = null; // {base64, name}
 
 // ── Send Command ──
diff --git a/routes/mcp.py b/routes/mcp.py
index 83a0aa0..a31b09d 100644
--- a/routes/mcp.py
+++ b/routes/mcp.py
@@ -72,14 +72,44 @@ def _public_view(s: dict) -> dict:
         "needs_auth": needs_auth,
         "auth": auth or "none",
         "note": s.get("note", ""),
+        "connected": False,  # filled in below for enabled servers
     }
 
 
+def _mcp_connect():
+    """Load skills/mcp_connect.py by path (skills/ isn't an importable package)."""
+    import importlib.util
+    from codec_config import SKILLS_DIR
+    path = os.path.join(SKILLS_DIR, "mcp_connect.py")
+    spec = importlib.util.spec_from_file_location("mcp_connect_rt", path)
+    mod = importlib.util.module_from_spec(spec)
+    spec.loader.exec_module(mod)
+    return mod
+
+
 @router.get("/api/mcp/servers")
 async def mcp_servers():
-    """List configured external MCP servers for the Connector tab."""
+    """List configured external MCP servers for the Connector tab, including
+    whether each ENABLED one is actually signed in (`connected`). Disabled
+    servers are never connected, so we skip the token lookup for them."""
+    import asyncio
     servers = [_public_view(s) for s in _read_servers()
                if isinstance(s, dict) and s.get("name")]
+
+    def _fill_connected():
+        mc = _mcp_connect()
+        for v in servers:
+            if v["enabled"]:
+                try:
+                    v["connected"] = bool(mc.is_connected(v["name"]))
+                except Exception:
+                    v["connected"] = False
+
+    try:
+        await asyncio.to_thread(_fill_connected)
+    except Exception:
+        pass  # never fail the listing over a token probe
+
     return {"servers": servers, "count": len(servers)}
 
 
@@ -143,19 +173,30 @@ async def mcp_signin(name: str):
     the token — so this can block for a while (the user has to authorize). Loads
     skills/mcp_connect.py by path (skills/ isn't an importable package)."""
     import asyncio
+    try:
+        msg = await asyncio.to_thread(lambda: _mcp_connect().signin_server(name))
+        ok = not msg.lower().startswith("no mcp server")
+        return {"ok": ok, "message": msg}
+    except Exception as e:
+        return {"ok": False, "message": f"Sign-in failed: {e}"}
 
-    def _do():
-        import importlib.util
-        from codec_config import SKILLS_DIR
-        path = os.path.join(SKILLS_DIR, "mcp_connect.py")
-        spec = importlib.util.spec_from_file_location("mcp_connect_signin", path)
-        mod = importlib.util.module_from_spec(spec)
-        spec.loader.exec_module(mod)
-        return mod.signin_server(name)
 
+@router.post("/api/mcp/servers/{name}/disconnect")
+async def mcp_disconnect(name: str):
+    """Forget the stored OAuth token for one connector — the card returns to
+    'Sign in'. Clears tokens + client info + expiry via fastmcp's own adapter."""
+    import asyncio
     try:
-        msg = await asyncio.to_thread(_do)
-        ok = not msg.lower().startswith(("no mcp server", "signed in to  "))
+        msg = await asyncio.to_thread(lambda: _mcp_connect().disconnect_server(name))
+        ok = msg.lower().startswith("disconnected")
+        if ok:
+            try:
+                from codec_audit import log_event
+                log_event("mcp_server_disconnected", "codec-dashboard",
+                          f"MCP connector '{name}' token cleared via Connector tab",
+                          extra={"server": name}, outcome="ok", level="info")
+            except Exception:
+                pass
         return {"ok": ok, "message": msg}
     except Exception as e:
-        return {"ok": False, "message": f"Sign-in failed: {e}"}
+        return {"ok": False, "message": f"Disconnect failed: {e}"}
diff --git a/skills/.manifest.json b/skills/.manifest.json
index 9de10db..2adb7eb 100644
--- a/skills/.manifest.json
+++ b/skills/.manifest.json
@@ -52,7 +52,7 @@
     "health_check.py": "01ac95da13770e2419460b4aaf1bd79c6628b6bc5413a5608153b81feee81568",
     "imessage_send.py": "862f7031a525faed4cec7cb703ef32f5d8bdc1045326f880d50455baf4cfcf86",
     "json_formatter.py": "42a770a1455a574d33e2f96f34c63ae7a83845f70789862e5e5198d0b429fde2",
-    "mcp_connect.py": "5ec24a905eb125fd0db3a1a69b29b1a87413a9ebcbf06e590e989cba1d912d4a",
+    "mcp_connect.py": "55fd270026b7c83dbe1c9f7653921537050f3e1086f84e35e3f2327daa224e17",
     "memory_entities.py": "252ad5861d614b916504b8af6300c4a8c62622c9f6c15238b11d6c6a052bada7",
     "memory_history.py": "a2762c03c325517d10907f8aa9511103a5716a29f9e956d48473b817105fb65c",
     "memory_save.py": "3d801338bfd0818aaf1e65e692e404af0a0fba6886d26150f0fb66e4b4fde424",
diff --git a/skills/mcp_connect.py b/skills/mcp_connect.py
index 99fa11d..f81488c 100644
--- a/skills/mcp_connect.py
+++ b/skills/mcp_connect.py
@@ -39,9 +39,12 @@
 
 import concurrent.futures
 import json
+import logging
 import os
 import re
 
+log = logging.getLogger("codec")
+
 CONFIG_PATH = os.path.expanduser("~/.codec/mcp_servers.json")
 
 # A curated menu of popular public MCP servers, seeded on first run. Most need
@@ -106,6 +109,77 @@ def _find_server(name: str) -> dict | None:
     return None
 
 
+# ── OAuth token persistence ──────────────────────────────────────────────────
+# fastmcp's OAuth defaults to IN-MEMORY token storage, so every dashboard restart
+# lost the sign-in and nothing could tell whether a connector was connected. We
+# hand it a Keychain-backed store instead (same secret tier as CODEC's other
+# secrets), which makes sign-in durable AND makes `connected` observable.
+_TOKEN_SERVICE = "ai.avadigital.codec.mcp_tokens"
+
+
+def _token_store():
+    """Persistent AsyncKeyValue for OAuth tokens. macOS Keychain first; a 0600
+    on-disk store is the fallback (headless/CI, or Keychain unavailable)."""
+    try:
+        import warnings
+        with warnings.catch_warnings():
+            warnings.simplefilter("ignore")  # store is flagged 'unstable' upstream
+            from key_value.aio.stores.keyring import KeyringStore
+            return KeyringStore(service_name=_TOKEN_SERVICE)
+    except Exception as e:
+        log.warning("mcp_connect: Keychain token store unavailable (%s) — using disk", e)
+        from key_value.aio.stores.disk import DiskStore
+        d = os.path.expanduser("~/.codec/mcp_tokens")
+        os.makedirs(d, mode=0o700, exist_ok=True)
+        return DiskStore(directory=d)
+
+
+def _token_adapter(url: str):
+    """fastmcp's own token adapter over our persistent store — gives us
+    get_tokens() (→ connected?) and clear() (→ disconnect) without guessing at
+    its key scheme."""
+    from fastmcp.client.auth.oauth import TokenStorageAdapter
+    return TokenStorageAdapter(async_key_value=_token_store(), server_url=url)
+
+
+def is_connected(name: str) -> bool:
+    """True if this server has a stored OAuth token. Servers that need no auth
+    are 'connected' as soon as they're enabled; api_key servers count as
+    connected once their header is configured."""
+    server = _find_server(name)
+    if not server:
+        return False
+    auth = str(server.get("auth") or "").strip().lower()
+    if auth in ("", "none"):
+        return True
+    if auth == "api_key":
+        return bool(server.get("headers"))
+    url = server.get("url")
+    if not url:
+        return False
+    try:
+        return _run_async(_token_adapter(url).get_tokens()) is not None
+    except Exception as e:
+        log.debug("mcp_connect: connected-check failed for %s: %s", name, e)
+        return False
+
+
+def disconnect_server(name: str) -> str:
+    """Forget the stored OAuth token for one server (clears tokens + client info
+    + expiry via fastmcp's own adapter). The card returns to 'Sign in'."""
+    server = _find_server(name)
+    if not server:
+        return f"No MCP server named '{name}'."
+    url = server.get("url")
+    if not url:
+        return f"server '{name}' has no url"
+    try:
+        _run_async(_token_adapter(url).clear())
+        return f"Disconnected from {server.get('name', name)}."
+    except Exception as e:
+        return f"Couldn't disconnect {name}: {e}"
+
+
 def _client_for(server: dict):
     """Build a fastmcp.Client for a server entry (no connection yet)."""
     from fastmcp import Client  # local import: keeps skill scan cheap
@@ -123,7 +197,9 @@ def _client_for(server: dict):
         if server.get("auth") == "oauth":
             try:
                 from fastmcp.client.auth import OAuth
-                return Client(url, auth=OAuth(mcp_url=url))
+                # token_storage → Keychain-backed, so the sign-in survives a
+                # dashboard restart instead of dying with the process.
+                return Client(url, auth=OAuth(mcp_url=url, token_storage=_token_store()))
             except Exception as e:  # OAuth unavailable → fall through to plain
                 import logging
                 logging.getLogger("codec").warning(