Align on modular MCP runtime and UI boundaries

[ctharvey]

What

I’d like to work on some MCP runtime and UI cleanup, and wanted to align on scope before opening PRs.

MCP is already a core part of Reasonix. I’m not proposing a rewrite or trying to reframe it as something new. The thing I’m interested in is making the existing MCP pieces easier to work with: cleaner runtime boundaries, clearer lifecycle/status handling, and better TUI surfaces around what MCP servers are doing.

Why

The MCP code already covers a lot: client/transport, registry bridging, reconnect behavior, drift handling, slash commands, browser UI, disable/reconnect helpers, and status output.

That is useful, but it also means MCP behavior is spread across runtime code and UI code in a few different places. When working on it, I found myself wanting a cleaner boundary between “what is the MCP server state?” and “how should the TUI show that state?”

I think tightening that seam would make future MCP work easier to land as small PRs.

Areas I’m looking at

• clearer MCP server lifecycle state
• cleaner separation between MCP runtime logic and TUI formatting
• reusable server summaries or view models for /mcp, browser, toast, reconnect, and inspect surfaces
• enable / disable / reconnect / inspect behavior
• registry and tool identity behavior across reconnects
• tests around lifecycle transitions, recovery paths, and UI-facing status summaries

Proposed approach

I’d like to keep this as a sequence of small PRs rather than one large change.

I do already have some local work in this direction. I’m happy to split it up, reshape it, or start with a smaller first PR depending on what you think the right boundary should be.

A possible first PR would be a no-behavior-change extraction around MCP server summaries or lifecycle view models, used by the existing UI paths. The goal would be to give the TUI a cleaner object to render without changing MCP behavior.

After that, follow-up PRs could improve status display, reconnect/disable feedback, and recovery behavior one piece at a time.

Non-goals

• no broad rewrite
• no unrelated TUI redesign
• no external server bundle or integration in core
• no MCP protocol behavior changes without discussion
• no CHANGELOG.md edits
• no behavior changes without tests

Question

Does this direction fit where you want MCP to go?

If so, what would you prefer as the first small PR boundary?

[esengine]

Direction's a yes — the seam observation is right.

One nuance worth flagging before you start: McpServerSummary already exists and all five UI surfaces already consume it (McpBrowser, /mcp slash handler, mcp-browse.ts for /resource + /prompt, dashboard panel, reconnect kickoff). The actual problem is that the type lives in src/cli/ui/slash/types.ts (UI layer) and gets built inline via .push({ ... }) in src/cli/commands/chat.tsx. That inversion is the thing worth fixing first.

First PR — relocation + builder, no behavior change

1. Move McpServerSummary from src/cli/ui/slash/types.ts → src/mcp/summary.ts. Re-export from the old path so existing imports keep compiling; drop the re-export in a later PR.
2. Add buildMcpServerSummary({ label, spec, client, bridge, report, host }) in the same file.
3. Replace the inline .push({ ... }) in chat.tsx with the builder call.
4. No new tests required — type signature stays identical, the existing MCP test suite + build are the gate.

Aim for ≤200 LOC diff. If it grows, something's leaking that isn't summary-shaped — flag it and cut it out of this PR.

Out of scope for that PR (good follow-ups, rough order)

• Dedup the healthBadge() duplication between McpBrowser.tsx and slash/handlers/mcp.ts.
• Wrap the direct host.client reads in mcp-browse.ts (resource/prompt fetch) behind a helper — that's the real UI→protocol leak, but it's behavior-adjacent so it gets its own PR.
• Reshape applyMcpAppend() in src/cli/ui/mcp-append.ts — it mutates report.tools.items + toolCount in place; an immutable updater is cleaner but again behavior-adjacent, separate PR.
• McpLifecycleEvent + McpSlowToast are event types, not summary state — leave them alone.

Hard rules for the series

• No CHANGELOG.md edits — release notes are written at release time.
• Comments follow CLAUDE.md + tests/comment-policy.test.ts — default is none, one line if why is non-obvious, no Phase-N or incident narrative.
• One logical change per commit, ≤300 LOC where reasonable.

If the first PR comes back clean I'm happy to keep merging follow-ups in sequence without re-aligning each time — open them back to back.

[ctharvey]

I think the narrow-slice pattern that has been working is:

1. extract one MCP helper/view-model boundary,
2. keep existing behavior/output stable,
3. add focused tests,
4. avoid bundling UI redesign or protocol behavior changes.

Possible next PRs after #248:

• refactor(mcp): centralize server summary matching

  • Extract the server === target || (label/spec match) logic into a tested helper.

• refactor(mcp): extract immutable server summary replacement helper

  • Pull the App setLiveMcpServers(prev => prev.map(...)) replacement pattern into a pure helper.

• refactor(mcp): extract browser row view model

  • Move count/capability/health derivation out of McpBrowser while keeping JSX output unchanged.

• refactor(mcp): extract reconnect result display model

  • Separate reconnect result-to-lines mapping from kickOffMcpReconnect, preserving existing output.

• test(mcp): cover startup summary states

  • Add test coverage for live/disabled/unbridged summary states before changing more runtime/UI boundaries.

I’d probably start with the identity/replacement helpers first because they are tiny, testable, and reduce duplication before touching larger UI surfaces.

[esengine]

Direction's a yes. Order looks right — helpers first, test coverage second, view-model extractions third.

One bundling suggestion: items 1 and 2 are really one logical change — both extract patterns that #248 inlined. They should land as a single PR (~50-80 LOC of pure helpers + tests). Splitting into two ~30-LOC PRs is admin overhead without review benefit. Something like refactor(mcp): extract server summary matching + replacement helpers.

For item 1, design note: the server === target || (label/spec match) fallback is load-bearing, not optional. Concrete failure mode if the fallback is removed:

1. The MCP browser modal closes over target at render time.
2. First reconnect runs → applyMcpAppend → setLiveMcpServers replaces target in the list with a new updated reference.
3. User triggers a second reconnect on the same row before the modal re-renders. The closure still holds the original target reference.
4. Inside the next setLiveMcpServers((prev) => ...), the list now contains updated, not target. server === target is false everywhere → map no-ops → the second append is silently dropped.

The label + spec branch matches on logical identity (both are stable across reconnects), so it catches that case. Same defense holds for any future updater path (disable, hot-reload, config-watch) that starts writing to liveMcpServers — without the fallback, identity-only matching would silently drop updates from those paths too.

The test for item 1 should pin this exact scenario: two back-to-back appends against the original target reference, asserting the second updated still lands in state.

Item 5 (startup-state coverage) before 3 + 4 is the right call. Once those land, I'm happy to merge 3 and 4 back-to-back without re-aligning each time.

Same hard rules from the original comment apply — no CHANGELOG.md, no Phase-N narrative, ≤300 LOC where reasonable.