feat(server): migrate MCP integrations to functional agent-mcp-manager#1794
Closed
Dani Akash (DaniAkash) wants to merge 6 commits into
Closed
feat(server): migrate MCP integrations to functional agent-mcp-manager#1794Dani Akash (DaniAkash) wants to merge 6 commits into
Dani Akash (DaniAkash) wants to merge 6 commits into
Conversation
Move the agent server's MCP connection logic off the legacy npm `agent-mcp-manager@0.0.3` class API onto the in-tree functional workspace package `@browseros/agent-mcp-manager`, matching how the cockpit server already consumes it. - Swap the singleton to the `bind()` bound API; `link()` now upserts the manifest entry and writes the agent config in one call, and `disconnect()` folds unlink + drop-if-last together. - Surface a curated set of seven harnesses (Claude Code, Codex, Cursor, OpenCode, Antigravity, VS Code, Zed), shown only when detected on the machine or already linked; an existing link counts as installed. - Simplify the boot-time URL reconciler to an overwrite-link per still-linked agent, removing the remove + add + rollback dance. - Drop the Claude Code HTTP transport-tag fixup: the catalog now emits the explicit `type` field for HTTP entries at the library layer. - Rework the manager tests onto an in-memory bound-API stub. Also borrow the OpenCode and Antigravity brand marks into the integrations panel so every surfaced harness renders with its own icon.
Contributor
✅ Tests passed — 1380/1384
|
Contributor
Greptile SummaryThis PR migrates the agent server MCP integration to the in-tree functional manager package. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (2): Last reviewed commit: "fix(server): self-heal stale MCP links o..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Address two state bugs in the MCP integration flagged in review. - Non-curated cleanup: a Gemini or Claude Desktop link created by an older build is no longer in the curated surface, so it lost its disconnect row and the config entry was stranded. A non-blocking boot step now disconnects any linked agent outside the curated set, removing the orphaned entry automatically. - URL reconcile no longer trusts the shared manifest spec as the drift signal. Because every linked agent shares one manifest spec, a partial write during a prior port change could leave one agent stale while the manifest read the new URL, masking the break forever. The reconciler now asks the library to rescan on-disk configs and re-links every still-present curated entry to the current URL, so a stale entry is always repaired. Entries the user removed by hand are left alone so they are not resurrected. Both run from a single serialised boot self-heal so their manifest writes cannot race.
Contributor
Author
The Claude Desktop manual setup wraps the URL via npx mcp-remote, so the instruction now tells the user to install Node and ensure npx is on the PATH before adding the config block.
loadServerUrlAndTools refetched tools after a restart but never cleared the previous set first, so stale tools lingered during the refetch and survived a failed refetch. Clear the list before refetching, matching the manual refresh path, so a port change that alters the tool set can never leave stale tools on screen.
The MCP settings page loaded the server URL and tools with a hand-rolled useState/useEffect fetch. That path refetched on every mount with no caching or dedup, so anything that remounted the page produced a refetch storm, and the earlier restart-invalidation logic had to be bolted on by hand. Move both to react-query, matching the rest of the app: - useMcpServerUrl / useMcpTools are cached and deduped, so a remount reuses the cache instead of refetching on a loop. - useReloadMcpServer invalidates the url and resets the tools cache, so a server restart (which can change the port and tool set) clears stale tools before refetching and cannot leave them on a failed refetch. The page component no longer holds fetch state or effects; it renders straight from the query results.
The restart button set the `restart_requested` pref to true but never cleared it. The browser's restart watcher is level-triggered, so it shut the server down, saw the flag still set on the fresh process, and shut it down again in an endless bounce. Every bounce dropped the MCP connection and made the settings page look like it was refreshing forever. Clear the flag in a finally once the restart has been serviced, on every path so a timed-out restart cannot leave it latched true.
Contributor
Author
|
Closing in favour of #1816. This branch grew beyond its original goal (it picked up unrelated MCP-settings-page work while debugging a separate restart issue). #1816 re-scopes to just the three things this set out to do: adopt the inline |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Moves the agent server's MCP connection logic off the legacy npm
agent-mcp-manager@0.0.3class API and onto the in-tree functional workspace package@browseros/agent-mcp-manager(0.0.4-rc.4), matching how the cockpit server already consumes it. Also borrows the OpenCode and Antigravity brand marks into the agent app so every surfaced harness renders with its own icon.This is the follow-up flagged when the manager was inlined and refreshed to rc.4.
Server: functional API migration
agent-mcp-manager@0.0.3becomes@browseros/agent-mcp-manager: workspace:*.bind().link()upserts the manifest entry and writes the agent config in a single call (no separateadd()), anddisconnect()folds unlink + drop-if-last together.typefield for HTTP entries at the library layer, so the post-write patch is redundant. Verified against a real on-disk config in the tests.The
/mcp-manager/agents,/agents/:id/install, and/agents/:id/uninstallroute contract is unchanged, so the app UI keeps working without changes to its hooks.App: agent icons
opencodeandantigravityin the integrations presentation so both render with brand-accurate artwork instead of the generic fallback.Behaviour change
Gemini CLI and Claude Desktop are no longer one-click surfaces (they were already excluded for fresh users). Both remain reachable via the manual setup snippet. This matches the cockpit server's curated harness set.
Test plan
bun run --filter '@browseros/server' typecheckclean.bun test tests/lib/mcp-manager(20) and the MCP route test (7).typefield and that URL drift rewrites it.