') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); })(); feat(tui): manage MCP servers from /mcp by me2seeks · Pull Request #4062 · apache/maka · GitHub
Skip to content

feat(tui): manage MCP servers from /mcp - #4062

Merged
Astro-Han merged 6 commits into
apache:mainfrom
me2seeks:feat/3838-tui-mcp-management
Aug 28, 2026
Merged

feat(tui): manage MCP servers from /mcp#4062
Astro-Han merged 6 commits into
apache:mainfrom
me2seeks:feat/3838-tui-mcp-management

Conversation

@me2seeks

Copy link
Copy Markdown
Contributor

Summary

Add the management slice for the TUI's local /mcp view.

  • Add guided stdio and Streamable HTTP setup, plus JSON import with an explicit add/replace preview.
  • Let users edit, enable or disable, test, reconnect, and remove configured servers without leaving the TUI.
  • Serialize config persistence, McpClientManager synchronization, and Host capability refresh through one TUI-owned action lane. Stale edits are rejected while unrelated concurrent config changes are preserved.
  • Retire endpoint credentials before their config references are removed, and replace sensitive input editors between prompts so secrets do not remain in reusable undo or paste buffers.
  • Reuse the pure MCP capability provider split in fix(cli): split MCP capability provider to avoid pulling runtime-host/server into TUI #4007; this does not add another MCP state, publication, or reconnect authority.

Refs #3838

Review focus

McpConfigStore remains the durable config authority, McpClientManager remains the connection and discovery authority, and the existing capability-provider service remains the Host publication authority. The new management layer only orders one user action across those owners and reports whether a committed mutation has synchronized and published yet.

The mutation path validates endpoint transitions before credential retirement, fences stale edit/import snapshots, and lets an admitted action drain before shutdown. A persistence failure cannot be reported as live, while a later manager or Host publication failure is reported as a committed partial outcome instead of rolling durable config back.

Verification

  • npm --workspace maka-agent test — 610 passed
  • npm --workspace @maka/runtime test — 3,064 passed, 7 skipped
  • npm run typecheck
  • npm run lint
  • npm run format:check
  • git diff --check origin/main..HEAD

The full workspace run initially encountered a stale fixed-path Runtime test database under /tmp; after preserving that directory aside, the affected test and the complete Runtime suite passed. No product code was changed for that environment issue.

Not run locally: an interactive Windows TUI session. Repository CI remains the cross-platform check.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Codex traced the existing MCP config, credential, manager, and publication boundaries; implemented the TUI management flows; reviewed concurrency and simplification risks; and ran the verification above.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

@github-actionsgithub-actionsBot added the effort/XL Over 1000 readable lines label Aug 28, 2026

@jackwenerjackwener left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found two issues on exact head 44e156c618a925c81a0d316b1069ceffc5dbc603.

[P1] Serialize mcp.json mutations across Desktop and TUI processes

The new TUI is a second supported writer for the same default workspace file that Desktop already manages. Each surface creates its own McpConfigStore, but FileMcpConfigStore.serial() only orders calls made through that one JavaScript object. transform() still performs an unlocked read, computes a replacement document, and atomically renames it over mcp.json. The TUI's per-server revision check runs inside that same unlocked transform, so it cannot detect a write that another process commits after the read and before the rename.

I reproduced this through two real createMcpConfigStore() instances pointed at one workspace. Two concurrent transforms added different server IDs. Both promises returned success with their own server, while the durable file contained only one. The result was deterministic in 100/100 runs. In the full product path, both Desktop and TUI can therefore synchronize and report their own mutation as applied even though the later rename silently discards the other user's config. This can also lose the only stored copy of command arguments, headers, or environment values.

Please make the durable mutation linearizable across processes—for example, hold one cross-process workspace lock across read, validation, credential retirement, and rename, or route both surfaces through one mutation authority. Add a regression using two independent store/controller instances which proves unrelated concurrent additions both survive and same-server credential retirement stays ordered.

[P2] Keep the selected server visible before acting on it

The list changes selected on Up/Down, but it never adjusts top; PageUp/PageDown/Home/End change top without updating the selected server. In a six-row viewport with eight servers, I pressed Down five times. The screen still showed only s0 and s1 with no visible cursor, then Space executed { kind: "set_enabled", serverId: "s5", enabled: false }. The same invisible target is used by test and reconnect.

Please couple selection and scrolling so the selected server's rendered row range is always visible, and make paging/home/end move selection consistently. A long-list interaction regression should assert both the visible cursor and the server ID sent to execute().

The underlying feature need is valid: before this patch, the local TUI can display MCP state but cannot manage it. Reusing the existing config, manager, and capability publication owners is otherwise the right boundary; the blocking problem is that adding a second process-local action lane does not make the shared durable writer single-authority.

Local verification passed for the affected Core, Storage, MCP, Runtime, Runtime Host, and CLI builds, the full CLI suite (610/610), and the focused Core/Storage/controller/overlay tests. windows_recovery is green. The first hosted test attempt failed only in the unchanged prompt-rail Desktop E2E; I requested an exact-head rerun, which is still in progress. The branch currently merges cleanly with main (e8028fc9f…, merge tree 03902b2b8a08187af3ed305d60a7390e0f4332f9). There were no existing comments, reviews, inline comments, or review threads to duplicate.


Posted by an automated review agent operated by @WAWQAQ. This is not an
independent human review and does not satisfy the committer review required by
CONTRIBUTING.md. A human is accountable for this comment — please push back if
anything here is wrong.

简体中文

本条评论由 @WAWQAQ 运行的自动化审查程序发出。它不构成 CONTRIBUTING.md
所要求的独立人类审查,也不能替代人类审查。有人类对本条评论负责,如有错误请直接指出。

@jackwenerjackwener left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The two findings from 44e156c618a925c81a0d316b1069ceffc5dbc603 are closed on exact head 1f9b2a12cbfa63ba1d4f7eb49acadb2f3cfd687f, but the new cross-process lock leaves one P2 recovery issue.

[P2] Recover an MCP config lock after its writer process crashes

FileMcpConfigStore.transform() now correctly holds withFileUpdateLock() across the authoritative read, asynchronous credential retirement, and durable rename. This closes the lost-update P1, but withFileUpdateLock() represents ownership only by creating the directory mcp.json.lock. The directory is removed in finally; it has no owner identity or lease that the next process can prove dead.

I exercised the production store in a child process, stopped it with SIGKILL after the transform acquired the lock, and then opened a new real store for the same workspace. The lock directory remained. The next upsert() waited 10,002 ms and failed with File update is locked by another process; every later Desktop or TUI mutation follows the same path until someone manually removes the hidden lock directory. TUI maps this to a generic persistence failure, so restarting the app does not explain or repair the condition.

This is a reasonable crash/restart path and leaves existing config readable, so I classify it as P2 rather than data loss. Please use the existing process-lifetime/native file-lock primitive (or another owner-identified lease) so OS process death releases ownership and a later writer can safely remove stale compatibility residue. Add a child-process crash regression that acquires the MCP transaction, kills the writer, and proves the next store can commit.

The original P1 is otherwise closed: two independent stores and controllers now preserve unrelated concurrent additions, and credential retirement runs inside the shared config transaction. The original selection P2 is also closed: rendered row ranges drive top, paging/home/end move selection, and the new long-list test proves the visible s5 row is the ID sent to execute().

The affected Core, Storage, MCP, Runtime, Runtime Host, Eval, CLI, UI, and Desktop main builds passed. The four focused Storage/CLI/Desktop files passed 54/54 tests. The full Storage run passed 980 tests with one unrelated Node 25 ExperimentalWarning-on-stderr child-process failure and 16 platform skips. windows_recovery is green; exact-head test is still running. The branch merges cleanly with current main (a956b1ae04aa7421a749931006a6df8fe564fc60, merge tree a9f3ac700e45b0733f0fa6799842a75092a7a562). There are no review threads.


Posted by an automated review agent operated by @WAWQAQ. This is not an
independent human review and does not satisfy the committer review required by
CONTRIBUTING.md. A human is accountable for this comment — please push back if
anything here is wrong.

简体中文

本条评论由 @WAWQAQ 运行的自动化审查程序发出。它不构成 CONTRIBUTING.md
所要求的独立人类审查,也不能替代人类审查。有人类对本条评论负责,如有错误请直接指出。

@jackwenerjackwener left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The remaining crash-recovery finding is closed on exact head b7ed6fda1994644d0e0791e570a0c15e29f9297b. I found no new P0-P3 issue in this three-file increment.

FileMcpConfigStore now uses the existing process-lifetime/native file-lock primitive. The operating system releases the advisory lease when a writer is killed, and the next owner can safely remove the stale compatibility marker before committing. The new regression starts a real child store, waits until it holds the MCP transaction, kills it with SIGKILL, and proves that a new store can commit and reopen the recovered server. I rebuilt Storage and ran the exact compiled store suite locally: 20/20 tests passed, including this child-process recovery case.

The earlier findings remain closed as well: independent Desktop/TUI stores preserve concurrent additions under the shared transaction, and long-list selection stays visible and sends the visible server ID to the action controller. The increment does not change those paths.

The branch still merges cleanly with current main (a956b1ae04aa7421a749931006a6df8fe564fc60, merge tree ffdf420e3841d9618d82c36127461d5ac3bd3b4b). windows_recovery is green; the exact-head hosted test job is still running, so this is a code-review conclusion rather than an approval or merge action.


Posted by an automated review agent operated by @WAWQAQ. This is not an
independent human review and does not satisfy the committer review required by
CONTRIBUTING.md. A human is accountable for this comment — please push back if
anything here is wrong.

简体中文

本条评论由 @WAWQAQ 运行的自动化审查程序发出。它不构成 CONTRIBUTING.md
所要求的独立人类审查,也不能替代人类审查。有人类对本条评论负责,如有错误请直接指出。

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the current head. The TUI management flow keeps the existing config, connection, and publication authorities intact; the earlier cross-process update, long-list selection, and crash-recovery issues are closed. Exact-head checks are green. Looks good.

@Astro-Han
Astro-Han merged commit 590d37c into apache:mainAug 28, 2026
2 checks passed
saltand pushed a commit to saltand/maka-agent that referenced this pull request Aug 31, 2026
* refactor(mcp): share credential retirement classification
Generated-by: Codex
* feat(cli): serialize TUI MCP management actions
Generated-by: Codex
* feat(tui): manage MCP servers from /mcp
Generated-by: Codex
* fix(mcp): serialize config mutations across processes
Generated-by: Codex
* fix(tui): keep MCP selection visible
Generated-by: Codex
* fix(mcp): recover config lock after process exit
Generated-by: Codex
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/XLOver 1000 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@me2seeks@jackwener@Astro-Han