Add MCP server registration for all supported agents#175
Add MCP server registration for all supported agents#175nikomatsakis merged 19 commits intosymposium-dev:mainfrom
Conversation
dfdafc6 to
5f7887a
Compare
5f7887a to
a3e8bea
Compare
Restructure src/agents.rs into src/agents/mod.rs to prepare for the upcoming MCP server registration submodule. The agents module is about to grow significantly — each agent needs register/unregister functions for MCP servers across multiple config formats (JSON, TOML, YAML). Splitting into a directory now keeps that new code in a dedicated file rather than bloating a single module.
The upcoming MCP server registration feature needs to know the path to the symposium binary so it can write it into agent config files (e.g. `"command": "/path/to/symposium"`). Currently nothing tracks this. Add binary resolution logic to the Symposium config struct so it's available wherever Symposium is passed. This uses current_exe() first, falls back to `which symposium`, and finally falls back to the bare name. A separate resolve_test_binary() looks in the cargo build output directory for test contexts.
Plugins need a way to declare MCP servers they want registered with agents. This commit adds that manifest support. McpServerEntry is an enum with two variants: - Builtin: resolved at sync time to the symposium binary path (from the previous commit's Symposium::symposium_binary()). This lets plugin manifests say "register symposium as an MCP server" without hardcoding a binary path. - Custom: a pass-through sacp::schema::McpServer for third-party servers. The mcp_servers field is added to both Plugin and the internal PluginManifest, wired through load_plugin, and tested.
Implement the core MCP server registration logic for all seven supported agents. Each agent has its own config format and file locations, so this is the bulk of the feature implementation. JSON-based agents (Claude, Copilot, Gemini, Kiro, OpenCode) share generic register/unregister helpers parameterized by container key. Codex uses TOML (via toml_edit to preserve formatting). Goose uses YAML with string manipulation to preserve comments. All registration is idempotent: existing correct entries are left alone, stale entries are updated in place. Also adds the Agent methods (register_project_mcp_servers, register_global_mcp_servers, unregister_project_mcp_servers, unregister_global_mcp_servers) that dispatch to the per-agent functions with the correct file paths.
Connect the pieces: sync_agent now collects MCP servers from all loaded plugins (resolving Builtin entries to concrete paths), then registers them with each configured agent and unregisters them from agents that are no longer configured. The hook registration/unregistration method signatures gain a &Symposium parameter (currently unused as `_sym`) to maintain a consistent interface — future work may need it for hook registration too, and having it available avoids another signature change later.
Document the MCP server registration config format and file locations for each supported agent. This mirrors the existing hook documentation sections and serves as the reference for how symposium integrates via MCP alongside the hook-based approach.
a3e8bea to
133b82f
Compare
…ries - Add [[mcp_servers]] reference section to plugin-definition.md covering stdio, HTTP, SSE, and builtin transports - Add "Publishing MCP servers" chapter under crate-authors - Update supporting-your-crate.md to mention MCP servers - Change McpServerEntry::Builtin to use type = "builtin" instead of a builtin field, consistent with the type tag used by HTTP/SSE - Add expect_test unit tests for all McpServerEntry variants
There was a problem hiding this comment.
Pull request overview
Adds first-class Model Context Protocol (MCP) server registration to Symposium’s agent sync flow, allowing agents to discover Symposium (and other plugin-declared MCP servers) via MCP in addition to the existing hook-based integration.
Changes:
- Introduces
[[mcp_servers]]in plugin manifests, including abuiltinentry that resolves to the localsymposiumbinary at sync time. - Wires MCP server register/unregister into
symposium sync --agentfor all supported agents (JSON/TOML/YAML config formats). - Expands documentation for plugin authors and per-agent configuration details; adds registration tests.
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
src/sync.rs |
Collects MCP servers from plugins and registers/unregisters them alongside hooks during agent sync. |
src/plugins.rs |
Adds McpServerEntry + Plugin.mcp_servers support in plugin manifests, including builtin resolution. |
src/config.rs |
Resolves and stores the Symposium binary path for populating stdio MCP server commands. |
src/agents/mod.rs |
Adds MCP server registration APIs to Agent and routes per-agent config paths. |
src/agents/mcp_server_registration.rs |
Implements per-agent MCP server registration/unregistration across JSON/TOML/YAML formats + tests. |
md/reference/plugin-definition.md |
Documents [[mcp_servers]] and agent registration locations. |
md/design/agent-details/*.md |
Adds MCP registration sections per agent. |
md/crate-authors/*.md + md/SUMMARY.md |
Adds author docs for publishing MCP servers and links them in the summary. |
Cargo.toml / Cargo.lock |
Adds YAML parsing dev dependency for Goose-related tests (and transitive deps). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let container = if let Some(key) = container_key { | ||
| if config.get(key).is_none() { | ||
| config[key] = json!({}); | ||
| } | ||
| &mut config[key] | ||
| } else { | ||
| &mut config | ||
| }; |
| /// Extract the name from any McpServer variant. | ||
| fn server_name(server: &McpServer) -> &str { | ||
| match server { | ||
| McpServer::Stdio(s) => &s.name, | ||
| McpServer::Http(s) => &s.name, | ||
| McpServer::Sse(s) => &s.name, | ||
| _ => "unknown", | ||
| } |
| let cmd = stdio.command.to_string_lossy(); | ||
| let args: Vec<_> = stdio.args.iter().map(|a| a.as_str()).collect(); | ||
| let args_yaml = format!("[{}]", args.join(", ")); | ||
|
|
||
| let snippet = formatdoc! {" | ||
| {name}: | ||
| provider: mcp | ||
| config: | ||
| command: {cmd} | ||
| args: {args_yaml} | ||
| "}; |
| let needle = format!("{name}:"); | ||
| if content.contains(&needle) { | ||
| out.already_ok(format!("{display}: {name} MCP server already configured")); | ||
| continue; | ||
| } |
| .map(|s| match s { | ||
| sacp::schema::McpServer::Stdio(s) => s.name.as_str(), | ||
| sacp::schema::McpServer::Http(s) => s.name.as_str(), | ||
| sacp::schema::McpServer::Sse(s) => s.name.as_str(), | ||
| _ => "unknown", |
| /// Resolve the path to the symposium binary. | ||
| /// | ||
| /// Tries `current_exe()` first, then `which symposium`, falling back to `"symposium"`. | ||
| fn resolve_symposium_binary() -> String { | ||
| if let Ok(exe) = std::env::current_exe() { | ||
| if exe.file_name().and_then(|n| n.to_str()) == Some("symposium") { | ||
| return exe.to_string_lossy().into_owned(); | ||
| } | ||
| } | ||
| if let Ok(out) = std::process::Command::new("which").arg("symposium").output() { | ||
| if out.status.success() { | ||
| let path = String::from_utf8_lossy(&out.stdout).trim().to_string(); | ||
| if !path.is_empty() { | ||
| return path; | ||
| } | ||
| } | ||
| } | ||
| "symposium".to_string() |
| assert_matches = "1.5" | ||
| expect-test = "1.5.1" | ||
| indoc = "2.0.7" | ||
| serde_yaml = "0.9" |
| ### MCP server configuration | ||
|
|
||
| | Scope | Config path | Root key | | ||
| |---|---|---| | ||
| | VS Code (workspace) | `.vscode/mcp.json` | `servers` | | ||
| | VS Code (user) | Via "MCP: Open User Configuration" command | `servers` | | ||
| | CLI | `~/.copilot/mcp-config.json` | `mcpServers` | | ||
|
|
||
| Note: VS Code uses `"servers"` as root key while the CLI uses `"mcpServers"`. MCP tools only work in Copilot's Agent mode. Supported transports: `local`/`stdio`, `http`/`sse`. | ||
|
|
||
| ### MCP Server Registration | ||
|
|
||
| In addition to hooks, symposium registers itself as an MCP server in the | ||
| agent's config file. This provides an alternative integration path | ||
| alongside the hook-based approach. | ||
|
|
||
| The MCP server entry is added as a top-level key: | ||
|
|
||
| ```json |
| /// Stdio: `{"command": "...", "args": [...]}` | ||
| /// Http/Sse: `{"url": "..."}` | ||
| fn server_to_json(server: &McpServer) -> serde_json::Value { | ||
| match server { | ||
| McpServer::Stdio(s) => { | ||
| json!({ | ||
| "command": s.command.to_string_lossy(), | ||
| "args": s.args, | ||
| }) | ||
| } | ||
| McpServer::Http(s) => json!({ "url": s.url }), | ||
| McpServer::Sse(s) => json!({ "url": s.url }), |
|
|
||
| ### How registration works | ||
|
|
||
| During `symposium sync --agent`, each MCP server entry is written into the agent's config file in the format that agent expects. Registration is idempotent — existing entries with correct values are left untouched, stale entries are updated in place. |
| } | ||
|
|
||
| // --------------------------------------------------------------------------- | ||
| // Per-agent registration functions |
There was a problem hiding this comment.
Seems like these would be better as trait impls?
There was a problem hiding this comment.
there aren't distinct types at present; functions seems fine to me
|
|
||
| Registration is idempotent — if the entry already exists with the | ||
| correct values, no changes are made. If the entry exists but has stale | ||
| values (e.g. the binary moved), it is updated in place. |
There was a problem hiding this comment.
Is this true? I want a test for this, in particular, what happens if an MCP server is added with the given name, but then the details change in plugin definition? I want to be sure we update the parameters as well and replace it, and not add two with the same name.
The builtin variant added unnecessary complexity. Plugin manifests now declare MCP servers directly using the standard McpServer types (stdio, http, sse). McpServerEntry is now a type alias for McpServer. Also removes resolve_symposium_binary and the symposium_binary field from Symposium, which were only needed for builtin resolution.
The McpServer enum is non_exhaustive, requiring a wildcard arm. Rather than silently falling back to "unknown" (which could collide with a real server name or produce empty JSON), panic so we notice immediately if sacp adds a new transport variant.
If the container key (e.g. mcpServers) exists but is not a JSON object,
reset it to {} instead of panicking on index access.
Custom MCP server definitions can specify env vars (stdio) or HTTP headers (http/sse). These were previously dropped during registration. Now they are included in the generated JSON when non-empty.
Paths with spaces, colons, or other YAML-special characters would produce invalid YAML. Now command and args values are double-quoted.
Goose registration uses string manipulation to preserve YAML comments, so existing entries are detected but not updated in place. Added caveats to both the design doc and the plugin-definition reference.
serde_yaml 0.9 is deprecated and pulls in unsafe-libyaml. Switch the dev-dependency to serde_yaml_ng 0.10 which is the maintained fork.
Verify that both command and args are updated when stale, and that the entry count stays at 1 (no duplicates created).
The MCP registration section was ambiguous about which JSON format is used. Clarify that symposium uses top-level keys (matching the CLI mcpServers format), not VS Code's servers format.
| /// | ||
| /// `env` and `headers` are omitted when empty. | ||
| fn server_to_json(server: &McpServer) -> serde_json::Value { | ||
| match server { |
There was a problem hiding this comment.
wait.... can't we just serde and just serialize this?
| let args: Vec<_> = stdio.args.iter().map(|a| a.as_str()).collect(); | ||
| let args_yaml = format!("[{}]", args.join(", ")); | ||
| let quoted_args: Vec<_> = stdio.args.iter().map(|a| format!("\"{}\"", a.replace('"', "\\\""))).collect(); | ||
| let args_yaml = format!("[{}]", quoted_args.join(", ")); |
There was a problem hiding this comment.
let's add a test for this edge case
|
|
||
| During `symposium sync --agent`, each MCP server entry is written into the agent's config file in the format that agent expects. Registration is idempotent — existing entries with correct values are left untouched, stale entries are updated in place. | ||
|
|
||
| **Caveat:** Goose uses string-based YAML manipulation to preserve comments. Existing entries are detected but not updated — if the server command or args change, remove the old entry manually. |
There was a problem hiding this comment.
Wait-- can't we just fix it?
Goose registration now detects stale entries and replaces them instead of silently skipping. Also adds tests for stale-entry updates and paths with spaces/special characters (YAML quoting).
What does this PR do?
Registers symposium as an MCP server in each agent's config alongside the existing hook-based integration. This provides an alternative integration path for agents to discover and invoke symposium via the Model Context Protocol.
Changes
agents::mcp_server_registrationmodule with per-agent register/unregister functions supporting JSON (Claude, Copilot, Gemini, Kiro, OpenCode), TOML (Codex), and YAML (Goose) config formatsSymposiumstruct now resolves and exposes the symposium binary path, used to populate MCP server entries with the correct commandsymposium synckeeps both hooks and MCP config in syncagents.rsconverted toagents/mod.rsdirectory to house the new submoduleDisclosure questions
AI disclosure.
Confidence level.
Questions for reviewers.