ADFA-5149 | Add MCP client plugin and contributed tools routing

[jatezzz]

Description

Introduces the ai-agent-mcp plugin, allowing the CodeOnTheGo Agent to connect to external Model Context Protocol (MCP) servers. This enables the agent to discover and interact with remote tools dynamically. Additionally, this PR updates ai-core to expose a ToolSourceRegistry, allowing other plugins to contribute tools directly to the agent's context. Security measures such as Keystore-backed encrypted tokens, prompt tool budgeting, and mandatory user approvals for remote tool execution are included.

Details

• Transport: Implements MCP's Streamable HTTP transport over HttpURLConnection and parses JSON-RPC/SSE chunks.
• Settings UI: Adds a full CRUD interface under Preferences for managing MCP servers, headers, encrypted tokens, and per-tool toggles (which are disabled by default to save prompt space).
• AI Core Integration: Rebuilds the agent's router, executor, and grammar dynamically when tool sources change, ensuring remote tools are strictly mapped and controlled.

🎦 Demo in the ticket

Ticket

Parent: ADFA-2592
ADFA-5149

Observation

• Dependencies: Deliberately avoided using OkHttp (to prevent classloader collisions with the host IDE's older copy) and the official MCP Kotlin SDK (due to lack of Android targets and strict Kotlin 2.4.10 pinning).
• Permissions: The ai-agent-mcp plugin relies solely on network.access. It does not request file or project permissions, isolating network operations from local core tools.

[claude]

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.

_{Tip: disable this comment in your organization's Code Review settings.}

[jatezzz]

This depends on appdevforall/CodeOnTheGo#1685 to be merged

[hal-eisen-adfa]

Automated review pass over the MCP client plugin and the ai-core tool-source bridge. Inline comments below cover the security findings (F02, F03) and the correctness findings (F04–F08, F10–F12). Performance and test-coverage observations are held back for now.

F04 is a file-level comment rather than a line comment because the file it concerns is checked in as binary — which is itself the finding.

[jatezzz]

@hal-eisen-adfa Rebased due to jars update

[hal-eisen-adfa]

Automated review of this branch (origin/main...41aa856) — 13 inline comments below.

Roughly: 7 correctness/security, 3 consistency, 3 duplication. The heaviest ones are the suffix-matching fallback in ToolRouter (a contributed tool can capture respond and receive the user's final answer), the unsanitised remote displayName now rendering as the approval dialog's title, and requiresApproval = false short-circuiting the approval posture that ContributedToolHandler documents.

Not included here: ToolRouterTest.kt was rewritten wholesale and lost all three dispatch() tests, including the only guard that CancellationException propagates ahead of the broad catch (e: Exception) in ToolRouter.dispatch. dispatch currently has no coverage.

Conventions all check out — root ../libs/*.jar for both compileOnly and the buildscript classpath, no bundled wrapper or per-plugin libs/, the AAR-metadata disable block, README row, and getTooltipCategory() returning "plugin_" + plugin.id with the 3-arg showTooltip overload throughout.

[hal-eisen-adfa]

Third pass, against 80ee16e. I re-verified all thirteen second-pass threads against the code rather than the replies, and answered each one inline.

Twelve addressed, one deferred with a reason I accept. Both suites are green here — ai-core 355 tests, ai-agent-mcp 66, no failures — and both .cgp files package correctly, assets/docs/index.html and both icons included. The test regression I flagged last time outside the inline comments is repaired: all three dispatch() tests are back, and the CancellationException guard is non-vacuous (if dispatch ever swallowed the cancellation, fail(...) throws an AssertionError that the catch (ce: CancellationException) does not catch).

Five new comments below. One of them is a blocker, and it is not a regression from this pass — it is a pre-existing hole that happens to nullify the fix in the requiresApproval thread, so it lands now or that fix does not mean what its KDoc says.

Two notes that had nowhere to anchor:

• Six of the thirteen fixes shipped with no test. 04, 05, 06, 07, 11 and 14 are all ai-agent-mcp, and the plugin's existing suites cover pure helpers only. The store mutex, the scope cancellation and the keep-alive change are the three I would most want a test on, and all three are testable without a device — McpServerStore takes a lock you can hammer from two threads, and McpSession already takes an injectable McpHttpClient "so the protocol can be exercised without a socket". refreshAll's keepGoing predicate is a plain lambda.
• ChatViewModel.kt:948 still has toolCalls.filterNot { it.name == RESPOND_TOOL }, the one terminal-tool check that did not get converted to isTerminalToolName. It only feeds the lastSucceededCalls dedupe, so a Respond defeats the dedupe rather than anything security-relevant — cosmetic, but it is the last == and the inconsistency is the kind that grows back.

Conventions still check out: root ../libs/*.jar for both compileOnly and the buildscript classpath, no bundled wrapper or per-plugin libs/, the AAR-metadata disable block, README row, min_ide_version correctly split (ai-core back to 26.32, only ai-agent-mcp on 26.35), and the new Clear stored credential control is wired to the right tooltip category with the 3-arg showTooltip overload.

[hal-eisen-adfa]

Approving at 9c62fa9. Fourth pass, and I checked the code rather than the replies.

All five third-pass findings are fixed, one of them further than I asked.

• Blocker, the auto-approved names — fixed both ways. ToolApprovalManager.AUTO_APPROVED_TOOLS is folded into reserved at AgentTools.kt:59, ToolSourceStore.handlers drops on it and seeds taken from it, so neither the plain nor the qualified form registers. The orphaned get_current_datetime is gone repo-wide — a grep over .kt/.md/.html/.xml returns nothing — and PARALLEL_SAFE_TOOLS is now a strict subset. Beyond the finding: Executor.kt:144 sets val toolName = handler.toolName, so ensureApproved sees the registered name rather than the model-emitted one, which closes the fuzzy-resolve-then-auto-approve variant as well.
• 202 drain — reusable = runCatching { … }.isSuccess. The return still runs the finally, so a failed drain disconnects like every other path.
• Clear button — hasHeaders(id), the more precise input. The thing that would have broken it does not: setHeaders(id, emptyMap()) removes the key, so key presence cannot show the button on a server that never had headers. The Credential.UNKNOWN guard closes the late-load path on :166 too.
• sourceLabel — flattened at the ContributedToolHandler boundary, and providerFailure interpolates it, so the model-facing text is covered. Traced the dialog path whole: handler.sourceLabel → ApprovalRequest.sourceLabel (ToolApprovalManager.kt:114) → ARG_SOURCE (ApprovalDialogFragment.kt:78).
• scope — @Volatile, and activate() cancels before reassigning.

Verified here: ai-core 359 tests (was 355), ai-agent-mcp 70 (was 66), no failures. Both assemblePlugin builds green and both .cgp files carry assets/docs/index.html and both icons.

One caveat on the approval, and it is not a small one: nothing on this branch has been exercised on hardware, in any of the four passes. All four have been static review plus unit tests, which is necessary and never sufficient for a plugin that writes the Keystore, mutates IDE state and renders a consent dialog. @jatezzz already named the specific gap — "the visibility and rotate paths still need a long-press pass on hardware" — and the credential dialog, the Clear control, the tooltips and one real MCP tool call through the approval dialog are what I would want to see before this reaches users. I am approving on the code; please land the device pass.

Three things left open, none blocking:

• ChatViewModel.kt:948 still has toolCalls.filterNot { it.name == RESPOND_TOOL } — the last == that did not become isTerminalToolName. Cosmetic, and it feeds the dedupe only.
• No test covers the store mutex, the scope cancellation, the keep-alive change or the Clear button's visibility. The first three are testable without a device.
• McpServerStore.kt:245 reads "How many extra headers are configured" over a Boolean. Pre-existing, but the function has call sites now.

And the SecureTokenStore triplication stays deferred, with the reason I accepted — worth a ticket number in that thread so it does not evaporate.