fix(codex): preserve per-tool approval permissions across provider switch and MCP sync#3251
Open
KtzeAbyss wants to merge 1 commit into
Open
fix(codex): preserve per-tool approval permissions across provider switch and MCP sync#3251KtzeAbyss wants to merge 1 commit into
KtzeAbyss wants to merge 1 commit into
Conversation
Owner
|
@codex review |
![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: c5a6d41e06
ℹ️ 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".
…itch and MCP sync Codex CLI writes [mcp_servers.<id>.tools.<tool>] approval entries to ~/.codex/config.toml when the user clicks "always allow". Switching providers (or saving common config) wiped those entries because the write path overwrites config.toml twice: 1. Provider live write: write_codex_live_with_catalog -> write_codex_live_for_provider -> write_text_file fully overwrites config.toml. The new provider's stored config has no tools.*, so the runtime permissions are lost in this first step. 2. MCP sync: sync_single_server_to_codex / sync_enabled_to_codex replace the entire [mcp_servers.<id>] subtable, dropping any nested subtables a second time. Establish the contract: cc-switch only owns fields it writes itself; unknown subtables (typically tools.*) must be preserved verbatim. The only subtable keys cc-switch writes under [mcp_servers.<id>] are env, http_headers, and the compat read-only headers. Two-layer fix matching the two overwrite steps: - Layer 1 (mcp/codex.rs): before rewriting [mcp_servers.<id>], snapshot non-managed subtables and restore them afterward. Extract apply_single_server_to_doc / apply_enabled_servers_to_doc pure functions for unit testing. - Layer 2 (codex_config.rs): in write_codex_live_atomic_with_stable_provider and write_codex_live_config_atomic_with_stable_provider, read the old live config before writing and merge any unknown subtables into the new text. write_codex_live_atomic (byte-for-byte restore path) and LiveSnapshot::restore are intentionally not touched so backup/restore semantics stay exact. remove_server_from_codex is intentionally unchanged: when the server itself is removed, permission entries becoming stale matches user intent, and an empty parent [mcp_servers.<id>] would make Codex CLI complain about a server with no command/url. Includes 13 pure-function unit tests covering Layer 1 (managed vs unmanaged subtable handling, enabled-empty drop, pre-existing "unmentioned server removed" behavior) and Layer 2 (main repro path where new text lacks mcp_servers, managed-subtable skipping for env / http_headers / compat headers, new-text-wins collision, unparseable old text best-effort, runtime-only parent creation).
KtzeAbyss
force-pushed
the
fix/codex-mcp-tools-permission
branch
from
c5a6d41 to
6b02941
Compare
Author
|
针对自动审查提出的两条建议（P1/P2）均已采纳修复，详见对应行内线程的回复。同时本分支已 rebase 到最新 main 解决冲突。 补充一处实现说明：上游 本地 240 个 codex 相关测试全部通过。 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
问题
当用户在 Codex CLI 中对某个 MCP 工具点击"始终允许"时，Codex 会向
~/.codex/config.toml写入：但用户在 cc-switch 中切换供应商（或保存通用配置）后，这条权限被自动删除。即使放进"编辑通用配置"也无济于事——下一次同步仍会清掉。
根因
切换供应商触发两步连续覆盖，两步都会抹掉
tools.*：~/.codex/config.toml，而 stored config 不带 runtime 字段（cc-switch 数据库不存这种字段），这一步先抹掉。McpService用 cc-switch 构造的服务器规范表整张替换[mcp_servers.<id>]，再次抹除任何嵌套子表。修复
确立契约：cc-switch 只管它自己写入的字段；未知子表（典型为
tools.*）必须原样保留。 cc-switch 在[mcp_servers.<id>]下唯一会写为子表的键只有env、http_headers及 compat 读取名headers，其余子表一律视为非 cc-switch 管辖。采用双层保护，对应根因两步：
mcp/codex.rs）：sync 重写[mcp_servers.<id>]前快照非管辖子表，重写后写回。抽出apply_single_server_to_doc/apply_enabled_servers_to_doc两个纯函数便于单测。codex_config.rs→mcp/codex.rs::merge_codex_runtime_subtables）：在 provider 驱动的 live 写入收口点write_codex_live_for_provider中，最终写入前从旧 live 读出未知子表合并进新文本。一处覆盖官方/三方两个写入分支；走裸write_codex_live_atomic的 byte-for-byte restore 路径不接入，保持精确还原语义。两层缺一不可：只补 Layer 1 挡不住第 1 步覆盖；只补 Layer 2 挡不住后续 MCP sync。
Layer 2 合并语义（细节）
tools子表时，按工具逐项合并而非整体取舍——旧tools.search与新tools.read都保留；同名工具叶子键冲突时新文本优先。command/url，因此新文本不含的 server 其孤儿tools.*直接丢弃，避免写出 Codex 无法加载的残缺 server。不在本 PR 范围
remove_server_from_codex：remove 语义为"该 server 不再存在"，权限随之失效符合直觉。sync_enabled_to_codex会清掉不在 enabled 列表中的 server——pre-existing 行为，本 PR 不调整。write_codex_live_atomic/LiveSnapshot::restore保持原样，确保备份/回滚精确还原。测试
mcp::codex::tests共 14 个纯函数用例（不碰文件系统）：Layer 1（MCP sync 边界）
sync_single_preserves_tools_permission_subtablesync_single_overwrites_managed_env_subtablesync_single_overwrites_managed_http_headers_subtablesync_enabled_preserves_tools_for_enabled_serversync_enabled_drops_mcp_servers_when_empty（pre-existing 行为）sync_enabled_drops_unmentioned_server_including_its_tools（pre-existing 行为）sync_single_handles_empty_docLayer 2（Codex live 写入边界）
merge_runtime_preserves_tools_when_new_text_has_server_without_tools（主复现路径）merge_runtime_merges_tools_per_tool_not_per_parent（逐工具合并）merge_runtime_drops_orphan_tools_when_new_lacks_server（不建残缺父表）merge_runtime_new_wins_when_key_collidesmerge_runtime_skips_managed_subtables（覆盖env/http_headers/ compatheaders）merge_runtime_returns_unchanged_when_old_has_no_mcp_serversmerge_runtime_returns_unchanged_when_old_unparseable复现
ace-tool-rs，启用 Codex~/.codex/config.toml写入[mcp_servers.ace-tool-rs][mcp_servers.ace-tool-rs.tools.<name>] approval_mode = "approve"写入文件tools.*消失；新行为：保留