fix(mcp): deliver team MCP servers to Codex in project scope (#954) - #956
Conversation
|
No findings. The PR description documents sufficient testing, including a representative real-CLI end-to-end run for the runtime behavior change. I did not run tests or execute PR code, as requested. |
cc5cd9d to
48c7740
Compare
|
No findings. The PR description documents sufficient testing, including a representative real-CLI end-to-end run for the runtime behavior change. I did not run, build, or execute PR code, as requested. |
|
No findings. The PR description documents sufficient testing, including a representative real-CLI end-to-end run for the runtime behavior change. I did not run, build, install, or execute PR code, as requested. |
…ent#954) Codex reads mcp_servers from <project>/.codex/config.toml once the project is trusted. Tencent#252 left the codex entry without mcpProject on the assumption that Codex has no project-scope MCP, so a project pull wrote nothing for it. The reconcile, ownership and git-exclude paths already handle a Codex project file; only the built-in mapping was missing.
…vers (Tencent#954) Codex loads <project>/.codex/config.toml only in a trusted project and skips an untrusted one without a word, so delivered team servers sit inert. Doctor now reads the projects table of the Codex user config (honoring toolRoots), takes the first entry for the checkout or its main checkout by real path, as Codex does, and fails with the manual fix. Read-only: it never writes trust.
…able fixes (Tencent#954) Review follow-up. An entry without trust_level decides nothing in Codex (checked with codex mcp list: a bare worktree entry falls through to the trusted main checkout), so the check skips it instead of reporting trust_level = "undefined". The fix text names a [projects."<dir>"] table rather than printing two TOML lines on one, drops the Codex-asks advice for an entry already marked untrusted, and tells an unreadable config from an unparsable one. Docs note that a pull reports the failure too.
…skill (Tencent#954) Review asked to leave the skill unchanged. The doctor check already prints the manual fix.
bd13c2a to
2526c14
Compare
|
No findings. The PR description documents sufficient testing, including a representative real-CLI end-to-end run for the runtime behavior change. Earlier reviews also reported no findings, and no previously raised issue remains to reassess. I did not run, build, install, or execute PR code, as requested. |
2 participants
Summary
In project scope, a pull never delivered team MCP servers to Codex. #252 left the built-in
codexentry withoutmcpProjecton the assumption that Codex has no project-scope MCP. In fact Codex reads<project>/.codex/config.tomlonce the project is trusted.codex: { skills: '.codex/skills', settings: '.codex/hooks.json', agents: '.codex/agents', mcp: '.codex/config.toml', + mcpProject: '.codex/config.toml', }The reconcile, ownership records, git-exclude protection,
mcp list,mcp remove, uninstall and theMCP servers delivered to codexcheck already handled a Codex project file, so they needed no change.Codex skips an untrusted project without saying so, so
doctorgets a read-only check for that case:The check never writes Codex trust (
projects.*orhooks.state); #955 owns that.codex-internalandtcodexare left as they are because their project paths are unverified.Type of Change
Test Plan
npx tsc --noEmitpassesnpm run lintpassesnpx vitest runpasses after rebasing ontoorigin/main(bc800ef8): 371 files, 7493 passed, 1 skipped.mcp-reconcile.test.ts: with the built-in defaults, a project pull targets and writes.codex/config.toml, keeps the existing content, and removes only its own block once the server leavesmcp.yaml. Before the fix this failed withexpected undefined to be '<root>/.codex/config.toml'.doctor-mcp-delivery.test.tsadds 9 cases for the trust check:untrustedtrust_levelfalls through to the main checkoutuntrustedworktree entry decidestoolRoots.codextool-roots.test.ts: the Codex entry now carriesmcpProject, which stays on the project root under a relocatedCODEX_HOME.npm run test:e2e -- project-scoped-delivery: 5 passed (comment-only change there).Real-CLI e2e (built
dist/, codex-cli 0.159.3)Recorded before the rebases onto
bae48e5cand thenbc800ef8. The only conflict in the second rebase was the./types.jsimport insrc/doctor-delivery.ts, where #945 addedresolveToolBaseDirandscopedToolPaths; it was resolved by importing both sides. Otherwisegit range-diffshows only context lines from main, and the only later change drops the skill note review asked to remove, so it was not re-run.The sandbox was
/tmp/tai954, with its ownHOME, a local git team repo whosemcp/mcp.yamlhas one stdio serverteam-docs, and a business repobizplus a linked worktreebiz-wt.CLAUDE_CONFIG_DIRandCODEX_HOMEwere unset, so Codex used$HOME/.codex. Trust was written by hand as a test fixture.Related Issues
Fixes #954
Related: #955 (setting Codex project trust, which clears this check)
Notes for Reviewers
.codex/config.tomluntilteamai mcp removeruns, or a pull on the reverted build cleans them up.<project>/.codex/config.tomlby text splice; content outside the team's blocks stays byte-identical. Every pull in an untrusted project now ends with a failing doctor check. That is deliberate: until the project is trusted the servers are inert, and [bug] Codex never runs teamai hooks: they need manual trust, and a pull invalidates it #955 is what clears the check automatically.install_mcpforcodexin project scope now writes to the same file, where before it threw "has no MCP config path". Whether the trust check fires for those installs was not verified.skill-data/is left as is, per review. The doctor check prints the manual fix itself.toolPathsoverrides: a team whoseteamai.yamlsetstoolPathsreplaces the defaults whole. Such a team must addmcpProject: .codex/config.tomlitself; the docs note says so.git worktree list, and how it compares with Codex's repository-root key there is unverified.