feat(mcp): expose MCP server management as goclaw_mcp_servers_* tools

[bclermont]

Problem

MCP server CRUD is reachable only over REST (/v1/mcp/servers). The CRUD MCP server at /api/mcp/ exposes agents, skills, cron, api_keys, providers, secure_cli and more — but not MCP servers themselves. An agent can mint an admin API key with goclaw_api_keys_create and then has to leave MCP entirely to register, inspect or reconnect a server.

Change

Eight tools in internal/mcp/crud_mcp_servers.go, backed by the same store.MCPServerStore the REST handlers use:

tool	REST equivalent
goclaw_mcp_servers_list	GET /v1/mcp/servers
goclaw_mcp_servers_get	GET /v1/mcp/servers/{id}
goclaw_mcp_servers_create	POST /v1/mcp/servers
goclaw_mcp_servers_update	PUT /v1/mcp/servers/{id}
goclaw_mcp_servers_delete	DELETE /v1/mcp/servers/{id}
goclaw_mcp_servers_tools	GET /v1/mcp/servers/{id}/tools
goclaw_mcp_servers_test	POST /v1/mcp/servers/test
goclaw_mcp_servers_reconnect	POST /v1/mcp/servers/{id}/reconnect

Gated on CRUDDeps.MCPServers like every other family; MCPManager/MCPPool/MCPOAuth are optional refinements (live tool listing, pool eviction, OAuth-backed discovery).

Safety parity with the REST path

• ValidateServerConfig on create, and on update against the merged config (update fields over the stored row) so a partial change can't slip past validation
• same column allowlist on update — unknown keys are dropped, not forwarded
• pool eviction (EvictServer) when url/credentials/settings/transport change, and on delete
• cache-invalidate broadcast so live agents pick the change up

Deliberate differences

• Credentials are masked on every read (api_key, header and env values; keys preserved). A tool result lands in an LLM context and its transcript — not a place raw tokens belong. Matches goclaw_providers_*.
• OAuth servers are never discovered with their static headers. Without a valid token the tools return authorization_required: true rather than listing tools the caller cannot invoke, mirroring the REST tool-list endpoint.
• _test returns {success:false, error} rather than a tool error for a rejected config — the caller asked whether a config works, and "no" is an answer.
• _reconnect without a pool reports status: "no_pool" instead of claiming a reconnect that didn't happen.

Tests

internal/mcp/crud_mcp_servers_test.go — masking (and that masking doesn't mutate stored rows), grant counts, structured field storage, slug rejection, non-allowlisted command rejection, allowlist filtering on update, merged-config validation, no-op update, delete, test-as-result, no-pool reconnect, full family registration.

go build ./..., go build -tags sqliteonly ./..., go vet and go test ./internal/mcp/ ./internal/gateway/ all pass.

Surface parity

REST, web UI and CLI already cover this resource — this adds the missing MCP surface only.

🤖 Generated with Claude Code

[clark-cant]

Summary

Adds eight MCP CRUD tools for server management, with validation, credential masking, pool eviction, OAuth-aware discovery, and tests.

Risk level

Medium — security-sensitive admin surface and a large new handler family.

Mandatory gates

• Duplicate / prior implementation: clear; related MCP OAuth and tool-listing work exists, but no duplicate CRUD implementation found.
• Project standards: docs/codebase conventions reviewed; CI is green.
• Strategic necessity: clear value; keeps server lifecycle management inside MCP and preserves REST safety parity.

Findings

Important

• internal/mcp/crud_mcp_servers.go is a 698-line new handler file. This exceeds the repository\u0027s apparent maintainability/file-size conventions and combines registration, argument decoding, validation/merge logic, masking, discovery, pool lifecycle, and eight handlers in one module. Split by domain (for example registration/args, CRUD handlers, discovery/masking) or otherwise reduce the module size before merge so future security review and testing do not become a single large hotspot.

Suggestion

• Keep the generated/Claude attribution out of the commit message if this repository does not use it consistently; it is process noise rather than implementation context.

Verdict

Request changes — split the new handler hotspot while preserving the current tests and CI coverage.