feat(mcp): wire flare MCP server into codex, cursor, windsurf, vscode-copilot - #171
Conversation
…-copilot Closes the gap in components.rs's agentflare-mcp component: 4 of 20 registered agents got real 'flare' MCP server registration on 'agentflare init --agent X' (claude-code, cline, continue, opencode); codex/cursor/windsurf/vscode-copilot silently no-op'd. - merge_json generalized with a root_key param: mcpServers for cursor/windsurf, servers for vscode-copilot - codex: check via substring match on ~/.codex/config.toml, apply via `codex mcp add flare -- <bin> mcp` - cursor: ~/.cursor/mcp.json, mcpServers key - windsurf: ~/.codeium/windsurf/mcp_config.json, mcpServers key - vscode-copilot: <cwd>/.vscode/mcp.json, servers key + type:stdio field - describe() text: codex gets its own line; cursor/windsurf/vscode-copilot join the manual-MCP-registration group Known limitation: the codex check is a raw substring match on config.toml, not validated against real `codex mcp add` output — risks a false positive on a commented-out line, or a false negative if the actual format differs in spacing/quoting from the assumed literal '[mcp_servers.flare]'. Same trust level as the existing claude-code/cline paths otherwise. Consent model stays opt-in per --agent run. Scope: these 4 hosts only (highest usage after Claude Code); the remaining 12 registered agents are deferred to a follow-up. Rebased onto current master (which now already includes the tool consolidation) from its original stale fork point.
📝 WalkthroughWalkthroughThe MCP component now supports host-specific configuration roots, detection logic, descriptions, and registration for codex, cursor, windsurf, vscode-copilot, and cline. ChangesMCP host support
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant AgentflareMcp
participant CodexMcpCli
participant HostMcpConfig
AgentflareMcp->>CodexMcpCli: run codex mcp add
CodexMcpCli->>HostMcpConfig: write flare server
AgentflareMcp->>HostMcpConfig: merge flare under host root key
HostMcpConfig-->>AgentflareMcp: check flare configuration
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components.rs (1)
83-104: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
merge_jsonsilently discards the entire existing config on any parse failure.If
fs::read_to_stringsucceeds butserde_json::from_strfails (malformed JSON, or JSONC comments/trailing commas),existingfalls back tojson!({})and the whole file gets overwritten with only the newflareentry — every other configured server is lost. This risk is now exercised more broadly:.vscode/mcp.jsonsits alongsidetasks.json/launch.jsonin VS Code's own editable-config family, which is documented to support comments, so a hand-annotated file would trigger this silent wipe on the very firstagentflare init --agent vscode-copilot.🛡️ Proposed fix: fail safe instead of silently overwriting
fn merge_json(path: &PathBuf, root_key: &str, key: &str, value: Value) -> bool { - let mut existing: Value = fs::read_to_string(path) - .ok() - .and_then(|s| serde_json::from_str(&s).ok()) - .unwrap_or_else(|| serde_json::json!({})); + let raw = fs::read_to_string(path).ok(); + let mut existing: Value = match raw { + None => serde_json::json!({}), + Some(s) => match serde_json::from_str(&s) { + Ok(v) => v, + Err(_) => return false, // don't clobber an existing-but-unparseable file + }, + };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components.rs` around lines 83 - 104, Update merge_json so an existing file that cannot be parsed is treated as an error rather than replaced with an empty object. Preserve the current file unchanged and return false on read or JSON/JSONC parse failure; only create a new empty configuration when the file is absent, while retaining the existing merge and write behavior for valid input.
🧹 Nitpick comments (1)
src/components.rs (1)
383-397: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCodex
checkuses a raw TOML substring match — known limitation with concrete failure modes.
s.contains("[mcp_servers.flare]")doesn't account for a disabled entry (codex supportsenabled = falseon a server table to disable it without deleting it), so a disabledflareentry would report "satisfied" when it isn't actually active. More importantly, a false negative here (e.g. non-canonical formatting from a hand-edited file) causesapplyto invokecodex mcp add flareagain — and duplicate[mcp_servers.*]tables inconfig.tomlare a documented cause of the Codex app hanging silently ("App becomes unresponsive with no error UI when config.toml has duplicate [mcp_servers.*] blocks").Parsing with the
tomlcrate (already a natural fit alongsideserde_jsonin this file) and checkingdoc["mcp_servers"]["flare"]would remove both failure modes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components.rs` around lines 383 - 397, Replace the raw substring check in the "codex" branch with TOML parsing using the existing TOML dependency, then inspect the parsed mcp_servers.flare table and require it to be present and enabled rather than merely matching its header. Preserve the false result for missing, invalid, or disabled entries so the check reflects whether the server is active without creating duplicate configuration blocks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components.rs`:
- Around line 371-372: Update the host description branch around the host
matching expression so cursor, windsurf, and vscode-copilot no longer claim
manual MCP registration; describe them as automatically registered by the
existing apply/merge_json flow. Preserve the current manual-registration wording
for cline, continue, and opencode.
---
Outside diff comments:
In `@src/components.rs`:
- Around line 83-104: Update merge_json so an existing file that cannot be
parsed is treated as an error rather than replaced with an empty object.
Preserve the current file unchanged and return false on read or JSON/JSONC parse
failure; only create a new empty configuration when the file is absent, while
retaining the existing merge and write behavior for valid input.
---
Nitpick comments:
In `@src/components.rs`:
- Around line 383-397: Replace the raw substring check in the "codex" branch
with TOML parsing using the existing TOML dependency, then inspect the parsed
mcp_servers.flare table and require it to be present and enabled rather than
merely matching its header. Preserve the false result for missing, invalid, or
disabled entries so the check reflects whether the server is active without
creating duplicate configuration blocks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ba78f105-a773-4ab4-a7d2-a7d95a76f84c
📒 Files selected for processing (1)
src/components.rs
| } else if matches!(host_owned.as_str(), "cline" | "continue" | "opencode" | "cursor" | "windsurf" | "vscode-copilot") { | ||
| format!("agentflare MCP server (skill_search/skill_load) — manual MCP registration for {host_owned}") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Description text is now inconsistent with actual behavior for cursor/windsurf/vscode-copilot.
This branch still describes cursor/windsurf/vscode-copilot as needing "manual MCP registration for {host}", but the apply logic added in this same PR (lines 440-467) auto-registers flare via merge_json for all three — no manual steps required. Users running agentflare init --agent cursor will see a misleading message implying they still need to do something by hand.
✏️ Proposed fix
} else if host_owned == "codex" {
"agentflare MCP server (skill_search/skill_load) — codex mcp add flare -- agentflare mcp".to_string()
- } else if matches!(host_owned.as_str(), "cline" | "continue" | "opencode" | "cursor" | "windsurf" | "vscode-copilot") {
+ } else if matches!(host_owned.as_str(), "cursor" | "windsurf" | "vscode-copilot" | "cline") {
+ "agentflare MCP server (skill_search/skill_load) — registered automatically via mcp.json".to_string()
+ } else if matches!(host_owned.as_str(), "continue" | "opencode") {
format!("agentflare MCP server (skill_search/skill_load) — manual MCP registration for {host_owned}")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } else if matches!(host_owned.as_str(), "cline" | "continue" | "opencode" | "cursor" | "windsurf" | "vscode-copilot") { | |
| format!("agentflare MCP server (skill_search/skill_load) — manual MCP registration for {host_owned}") | |
| } else if matches!(host_owned.as_str(), "cursor" | "windsurf" | "vscode-copilot" | "cline") { | |
| "agentflare MCP server (skill_search/skill_load) — registered automatically via mcp.json".to_string() | |
| } else if matches!(host_owned.as_str(), "continue" | "opencode") { | |
| format!("agentflare MCP server (skill_search/skill_load) — manual MCP registration for {host_owned}") |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components.rs` around lines 371 - 372, Update the host description branch
around the host matching expression so cursor, windsurf, and vscode-copilot no
longer claim manual MCP registration; describe them as automatically registered
by the existing apply/merge_json flow. Preserve the current manual-registration
wording for cline, continue, and opencode.
1 participant
Closes the gap in components.rs's agentflare-mcp component: 4 of 20 registered agents got real flare MCP server registration on 'agentflare init --agent X' (claude-code, cline, continue, opencode); codex/cursor/windsurf/vscode-copilot silently no-op'd.
Known limitation (non-blocking): the codex check is a raw substring match on config.toml, not validated against real codex mcp add output — risks a false positive on a commented-out line, or a false negative if the real format differs in spacing/quoting. Same trust level as the existing claude-code/cline paths otherwise.
Scope: these 4 hosts only (highest usage after Claude Code); the remaining 12 registered agents are deferred to a follow-up. Consent model stays opt-in per --agent run.
Rebased onto current master (which now already includes the tool consolidation) from its original stale fork point.
Verification
Summary by CodeRabbit
New Features
Bug Fixes