Skip to content

feat(connector): real connect/disconnect state + Keychain-persistent tokens (beat 19)#262

Merged
AVADSA25 merged 1 commit into
mainfrom
feat/connector-state-machine
Jul 16, 2026
Merged

feat(connector): real connect/disconnect state + Keychain-persistent tokens (beat 19)#262
AVADSA25 merged 1 commit into
mainfrom
feat/connector-state-machine

Conversation

@AVADSA25

Copy link
Copy Markdown
Owner

What was broken

Toggling a connector 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 disconnect, and the token vanished on restart.

Root cause: fastmcp's OAuth defaulted to in-memory token storage. Nothing persisted, so nothing could be queried — which is why connected state never existed.

The full state machine (per card)

State Shows
off just the toggle
on + needs auth + not connected "Needs OAuth sign-in" + [Sign in]
on + connected ● Connected + [Disconnect]
on + no auth needed Connected

Changes

  • skills/mcp_connect.py — OAuth now gets a Keychain-backed token store (KeyringStore, service ai.avadigital.codec.mcp_tokens; 0600 disk fallback for headless/CI). New is_connected(name) + disconnect_server(name), built on fastmcp's own TokenStorageAdapter (get_tokens() / clear()) rather than guessing its key scheme. Added the module-level logger the new code needs.
  • routes/mcp.pyGET /api/mcp/servers returns connected per enabled server (token never leaked); new audited POST /api/mcp/servers/{name}/disconnect.
  • codec_dashboard.html — real CSS classes (.mcp-state/.mcp-btn/.mcp-connected) using the dashboard's own tokens, on their own row so nothing crowds the auth note. Toggling on re-renders instantly. No emoji (CSS dot + line-SVG).

Verified

Keychain store roundtrips (put/get/delete). Against the live config: hugging-face (no auth) → connected ✓; notion (oauth) → not connected ✓ (correct — the earlier sign-in was in-memory only, so it's genuinely gone; signing in again now persists); disabled servers skipped. All 4 routes registered. 42 mcp tests pass, ruff clean, manifest regenerated.

Note for the operator: Notion needs one more Sign in — this time it survives restarts.

🤖 Generated with Claude Code

…tokens (beat 19)

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>
@AVADSA25
AVADSA25 merged commit ab213cc into main Jul 16, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants