fix(converters): preserve user config when writing MCP servers#479
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6bd9e1d21e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Strip unmarked legacy format (# Generated by compound-plugin) during merge to prevent duplicate TOML tables on first upgrade from old writer Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b26a02a98d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Copilot/Qwen: treat all existing MCP servers as plugin-managed when _compound_managed_mcp tracking key is absent (legacy migration) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The install and convert paths for Codex, Copilot, and Qwen were overwriting the entire config file with only plugin-managed MCP servers, wiping user settings (model preferences, custom MCP servers, etc.). Codex: Use managed block markers (BEGIN/END) so plugin MCP config coexists with user TOML. Extract shared mergeCodexConfig() used by both the install and sync paths. Migrate previous-generation markers and the unmarked legacy format (# Generated by compound-plugin). Copilot/Qwen: Track plugin-managed server names in a _compound_managed_mcp array within the JSON config. On re-install, only managed servers are replaced — user-added servers are preserved. Stale managed servers (renamed/removed from plugin) are cleaned up. Legacy configs without tracking key treat all existing servers as plugin-managed for safe first migration. All three targets now converge correctly when the plugin ships zero MCP servers (stale managed config is removed). Also skips malformed MCP servers (neither command nor url) instead of emitting empty TOML sections. Fixes #478 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
tmchow
force-pushed
the
feat/investigate-config-wipe
branch
from
e4235ca to
5ca4912
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5ca4912af1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…servers Always write _compound_managed_mcp (even as []) so the legacy fallback doesn't treat user servers as plugin-managed on a future install. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8007603208
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…pilot and Qwen When no tracking key exists AND incoming MCP is empty, skip the legacy prune path that assumes all existing servers are plugin-managed. Also always retain the tracking key (as []) to prevent the legacy fallback from firing on future installs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 19a07ed26f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…hanges Track plugin-written top-level keys via _compound_managed_keys, same pattern as _compound_managed_mcp for MCP servers. Keys removed from the plugin config (e.g. settings) are pruned on re-install; user-added keys are preserved. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ervation test Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 participant
Summary
Fixes #478 — the Codex, Copilot, and Qwen install/convert writers were overwriting user config files wholesale, wiping settings like model preferences and user-added MCP servers.
BEGIN/END) so plugin MCP config coexists with user TOML. Extracted sharedmergeCodexConfig()used by both install and sync paths. Handles migration from previous-generation markers and legacy markers (# Generated by compound-plugin)._compound_managed_mcparray. On re-install, only managed servers are replaced — user-added servers survive. Stale managed servers (renamed/removed) are cleaned up._compound_managed_keysto track which top-level config keys the plugin wrote (e.g.,settings). Stale keys are pruned on re-install; user-added keys are preserved.[]). Malformed servers (neithercommandnorurl) are skipped instead of emitting empty config sections.Test plan
# Generated by compound-plugin)🤖 Generated with Claude Code