feat(codex): add project-scoped MCP and user target support

[Nickolaus]

Description

Make Codex MCP config project-scoped for repo installs by writing to .codex/config.toml instead of ~/.codex/config.toml, and only configure Codex MCP when Codex is an active project target.

Also add Codex user-scope support for installed primitives so agents, hooks, and skills follow the existing target and scope model used by the other supported tools.

Fixes #502

Type of change

• Bug fix
• New feature
• Documentation
• Maintenance / refactor

Testing

• Tested locally
• All existing tests pass
• Added tests for new functionality (if applicable)

[Copilot]

Pull request overview

This PR updates APM's Codex integration to support project-scoped MCP configuration (writing to .codex/config.toml for project installs) and adds Codex participation in the existing user-scope target/scope model.

Changes:

• Make Codex MCP config scope-aware (project .codex/config.toml vs user ~/.codex/config.toml) and avoid configuring Codex MCP unless Codex is an active project target.
• Add scope-aware project_root/user_scope plumbing through MCP client creation and MCP install/uninstall flows.
• Expand unit/integration tests and update Starlight docs to reflect the new Codex MCP scoping behavior.

Reviewed changes

Copilot reviewed 23 out of 23 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/apm_cli/integration/mcp_integrator.py	Adds scope parameters and filters Codex MCP configuration to active project targets.
src/apm_cli/adapters/client/base.py	Introduces project_root/user_scope context and placeholder normalization helper.
src/apm_cli/adapters/client/codex.py	Resolves Codex config path by scope and normalizes project placeholders in args.
src/apm_cli/factory.py	Passes project_root/user_scope into client adapter construction.
src/apm_cli/core/safe_installer.py	Creates adapters with scope context for conflict-aware installs.
src/apm_cli/core/operations.py	Threads scope context through configure/install/uninstall helpers.
src/apm_cli/registry/operations.py	Checks installed server IDs using scope-aware adapter config resolution.
src/apm_cli/integration/targets.py	Marks Codex as partially user-scope supported.
src/apm_cli/commands/install.py	Passes scope context into MCP install + stale cleanup.
src/apm_cli/commands/uninstall/cli.py / engine.py	Passes scope context into MCP stale cleanup on uninstall.
src/apm_cli/adapters/client/{vscode,cursor,opencode,copilot}.py	Switches workspace-root resolution to self.project_root.
tests/unit/test_transitive_mcp.py	Adds Codex project-scoped MCP behavior tests (install + stale cleanup).
tests/unit/test_mcp_client_factory.py	Validates Codex config-path scoping + placeholder normalization.
tests/unit/integration/test_scope_*.py	Updates/extends target resolution and install/uninstall scope tests for Codex.
tests/unit/integration/test_hook_integrator.py	Verifies Codex hooks merge target respects scope-resolved root dir.
docs/src/content/docs/integrations/ide-tool-integration.md	Documents Codex MCP config paths and project-target gating.
docs/src/content/docs/reference/cli-commands.md	Clarifies global vs project Codex configuration behavior.

Comments suppressed due to low confidence (4)

src/apm_cli/registry/operations.py:36

• The method signature now accepts project_root and user_scope, but the docstring's Args section doesn't document them. Please update the docstring to describe how scope/root affect which config files are inspected when checking installation status.

    def check_servers_needing_installation(self, target_runtimes: List[str], server_references: List[str], project_root=None, user_scope: bool = False) -> List[str]:
        """Check which MCP servers actually need installation across target runtimes.
        
        This method checks the actual MCP configuration files to see which servers
        are already installed by comparing server IDs (UUIDs), not names.
        
        Args:
            target_runtimes: List of target runtimes to check
            server_references: List of MCP server references (names or IDs)
            

src/apm_cli/core/operations.py:13

• The function signature now includes project_root and user_scope, but the docstring doesn't document these parameters. Please update the Args section so callers understand which config scope/path will be written.

def configure_client(client_type, config_updates, project_root=None, user_scope=False):
    """Configure an MCP client.
    
    Args:
        client_type (str): Type of client to configure.
        config_updates (dict): Configuration updates to apply.
    

src/apm_cli/core/operations.py:40

• The function signature now includes project_root and user_scope, but these parameters are missing from the docstring Args section. Document them so it's clear which project/user config is being targeted during installation.

def install_package(client_type, package_name, version=None, shared_env_vars=None, server_info_cache=None, shared_runtime_vars=None, project_root=None, user_scope=False):
    """Install an MCP package for a specific client type.
    
    Args:
        client_type (str): Type of client to configure.
        package_name (str): Name of the package to install.
        version (str, optional): Version of the package to install.
        shared_env_vars (dict, optional): Pre-collected environment variables to use.
        server_info_cache (dict, optional): Pre-fetched server info to avoid duplicate registry calls.
        shared_runtime_vars (dict, optional): Pre-collected runtime variables to use.
    

src/apm_cli/core/operations.py:86

• The function signature now includes project_root and user_scope, but the docstring Args section doesn't mention them. Please document how they affect which config file is edited during uninstall.

def uninstall_package(client_type, package_name, project_root=None, user_scope=False):
    """Uninstall an MCP package.
    
    Args:
        client_type (str): Type of client to configure.
        package_name (str): Name of the package to uninstall.
    

[Copilot]

Pull request overview

Copilot reviewed 23 out of 23 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 24 out of 24 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

src/apm_cli/registry/operations.py:116

• _get_installed_server_ids() currently extracts IDs only for runtimes 'copilot', 'codex', and 'vscode'. MCPIntegrator.install() can pass 'cursor' (and potentially other runtimes) into check_servers_needing_installation(), and Cursor uses the same mcpServers schema as Copilot (including id). Without a 'cursor' branch here, Cursor will always appear to have zero installed servers, causing unnecessary reinstalls. Consider handling 'cursor' by reading IDs from config.get("mcpServers", {}) like the Copilot branch (and similarly address any other MCP-compatible runtimes you intend to support).

                )
                config = client.get_current_config()
                
                if isinstance(config, dict):
                    if runtime == 'copilot':

[Copilot]

Pull request overview

Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 29 out of 29 changed files in this pull request and generated no new comments.

[github-actions]

APM Review Panel Verdict

Disposition: APPROVE (one required pre-merge fix -- CHANGELOG format; four optional follow-ups)

---

Per-persona findings

Python Architect:

This PR threads project_root and user_scope through the entire MCP adapter stack, making Codex a proper project-scoped runtime on par with Cursor and OpenCode. The class hierarchy is unchanged; the change adds constructor parameters to the ABC and propagates them through the factory chain. Routine PR scope (two mermaid blocks).

OO / class diagram

classDiagram
    direction LR
    class MCPClientAdapter {
        <<AbstractBase>>
        +project_root Path
        +user_scope bool
        +supports_user_scope bool
        +get_config_path()* str
        +update_config(config_updates)*
        +get_current_config()*
        +configure_mcp_server(...)* bool
        +normalize_project_arg(value) str
    }
    class CodexClientAdapter {
        +_get_codex_dir() Path
        +get_config_path() str
        +update_config(config_updates) bool
        +get_current_config() dict|None
        +configure_mcp_server(...) bool
    }
    class CopilotClientAdapter {
        +supports_user_scope bool
        +get_config_path() str
        +update_config(config_updates)
    }
    class VSCodeClientAdapter {
        +get_config_path() str
    }
    class CursorClientAdapter {
        +get_config_path() str
    }
    class OpenCodeClientAdapter {
        +get_config_path() str
    }
    class ClientFactory {
        <<Factory>>
        +create_client(client_type, project_root, user_scope) MCPClientAdapter
    }
    class SafeMCPInstaller {
        +runtime str
        +adapter MCPClientAdapter
        +install_package(...) dict
    }
    class MCPIntegrator {
        <<Service>>
        +install(..., project_root, user_scope) int
        +remove_stale(..., project_root, user_scope)
        +_install_mcp_for_runtime(..., project_root, user_scope) bool
        +_check_self_defined_servers_needing_installation(..., project_root, user_scope) list
    }
    class MCPServerOperations {
        +check_servers_needing_installation(..., project_root, user_scope) list
        +_get_installed_server_ids(..., project_root, user_scope) set
    }

    MCPClientAdapter <|-- CodexClientAdapter
    MCPClientAdapter <|-- CopilotClientAdapter
    CopilotClientAdapter <|-- VSCodeClientAdapter
    CopilotClientAdapter <|-- CursorClientAdapter
    CopilotClientAdapter <|-- OpenCodeClientAdapter
    ClientFactory ..> MCPClientAdapter : creates
    SafeMCPInstaller o-- MCPClientAdapter : adapter
    MCPIntegrator ..> ClientFactory : delegates
    MCPIntegrator ..> SafeMCPInstaller : creates
    MCPIntegrator ..> MCPServerOperations : delegates

    class MCPClientAdapter:::touched
    class CodexClientAdapter:::touched
    class CopilotClientAdapter:::touched
    class VSCodeClientAdapter:::touched
    class CursorClientAdapter:::touched
    class OpenCodeClientAdapter:::touched
    class ClientFactory:::touched
    class SafeMCPInstaller:::touched
    class MCPIntegrator:::touched
    class MCPServerOperations:::touched
    classDef touched fill:#fff3b0,stroke:#d47600

Loading

Execution flow diagram

flowchart TD
    A["apm install\ncommands/install.py"] --> B["_install_apm_packages(ctx)"]
    B --> C["MCPIntegrator.install\nmcp_deps, project_root=ctx.project_root\nuser_scope=scope==USER"]
    C --> D{"scope override\nUSER -> user_scope=True\nPROJECT -> user_scope=False"}
    D --> E{"runtime flag\nspecified?"}
    E -->|"yes"| F["target_runtimes = [runtime]"]
    E -->|"no"| G["[FS] auto-discover:\nproject_root/.vscode .cursor .codex .opencode .gemini"]
    G --> H{"not user_scope AND\ncodex in target_runtimes?"}
    H -->|"yes"| I["[FS] active_targets(project_root, config_target)\nintegration/targets.py"]
    I -->|"codex not active"| J["[LOG] logger.progress: skipping Codex\nremove codex from target_runtimes"]
    I -->|"codex active"| K["scope filter: USER -> keep supports_user_scope only"]
    H -->|"no"| K
    F --> K
    J --> K
    K --> L["MCPServerOperations.check_servers_needing_installation\ntarget_runtimes, servers, project_root, user_scope"]
    L --> M["[I/O] ClientFactory.create_client(runtime, project_root, user_scope)\n.get_current_config()"]
    M --> N["_install_mcp_for_runtime(runtime, deps, project_root, user_scope)"]
    N --> O["SafeMCPInstaller(runtime, project_root, user_scope)\n-> ClientFactory.create_client(runtime, project_root, user_scope)"]
    O --> P{"user_scope?"}
    P -->|"True"| Q["[FS] ~/.codex/config.toml"]
    P -->|"False"| R["[FS] project_root/.codex/config.toml"]
    Q --> S["[I/O] get_current_config() -> dict or None"]
    R --> S
    S -->|"None (TOML parse error)"| T["[LOG] _rich_warning: could not parse -- skipping\nreturn False"]
    S -->|"dict"| U["[FS] update_config: write config.toml\nreturn True"]

Loading

Design patterns

• Used in this PR: Factory (parametric ClientFactory.create_client(runtime, project_root, user_scope) -- factory.py); Template Method (MCPClientAdapter.project_root property as a hook overridden via constructor injection; subclasses call _get_codex_dir() in codex.py as a template step).
• Pragmatic suggestion: introduce a ScopeContext dataclass wrapping project_root: Path and user_scope: bool to eliminate the two-parallel-parameter threading pattern visible in 10+ function signatures. Would cut per-function parameter noise and prevent "forgot to pass user_scope" bugs at new call sites.

Structural concern: CodexClientAdapter.update_config() now returns bool, but MCPClientAdapter.update_config() is declared @abstractmethod with no return type annotation. The pattern if not self.update_config(...): return False in codex.py:configure_mcp_server is safe because self is always concrete Codex, but the ABC should declare -> bool | None to signal the contract to future implementers.

Minor inefficiency: In _install_mcp_for_runtime (mcp_integrator.py), a second ClientFactory.create_client() call is made after a successful Codex install solely to retrieve get_config_path() for verbose logging. SafeMCPInstaller.adapter already holds that reference inside install_package(); surfacing it (or computing the path from the known parameters without a second adapter instantiation) would be cleaner.

---

CLI Logging Expert: Output paths in new code are clean. _rich_warning() on TOML parse failure in codex.py::get_current_config() is the correct helper with proper symbol="warning". logger.verbose_detail() for the Codex config path and logger.progress() for the gating-skip message both route through CommandLogger correctly. _log.debug(...) calls follow structured logging convention. One minor duplication: _log.debug("Codex config written to %s", config_path) appears in both mcp_integrator.py::_install_mcp_for_runtime and codex.py::update_config() -- harmless noise. Pre-existing print(f"Successfully configured MCP server...") in codex.py (~line 174) is not routed through CommandLogger, but that is out of scope for this PR. No new unguarded print() calls introduced.

---

DevX UX Expert: The project-local opt-in model (.codex/ directory presence = opt-in, same as Cursor/OpenCode) is the right UX direction -- it removes the footgun of apm install silently mutating global Codex state in team repos. The gating-skip message ("Codex not an active project target -- create .codex/ or set target: codex in apm.yml") is actionable and shown at logger.progress() level (visible without --verbose). normalize_project_arg translating ${workspaceFolder}/${projectRoot} to "." for project-local installs is invisible to users and correct. CLI reference doc and ide-tool-integration.md are updated in the same PR -- passes Rule 4 (doc sync). One required fix: the CHANGELOG entry (see Required actions below).

---

Supply Chain Security Expert: This PR improves the security posture in two concrete ways:

1. get_current_config() now returns None (not {}) on TOML parse error (codex.py), and update_config() returns False and skips the write when it receives None. This is correct fail-closed behavior -- a corrupted or tampered config is not silently overwritten.

2. Codex MCP is now gated behind active_targets(), preventing writes to .codex/config.toml in repos that have not opted in. This matches the containment model Cursor and OpenCode already use.

Follow-up concern (not blocking): MCPClientAdapter.project_root constructs paths from project_root: Path | str | None without validate_path_segments / ensure_path_within (per src/apm_cli/utils/path_security.py convention). In the current call chain, project_root originates from get_deploy_root(scope) (safe APM-internal resolution), so there is no active traversal risk. But the property has no guard, meaning a future caller providing user-supplied input would have no protection. Recommend adding ensure_path_within(resolved_path, project_root) as defense-in-depth in a follow-up.

---

Auth Expert: Not activated -- the diff touches adapters/client/, integration/mcp_integrator.py, factory.py, and core/operations.py, all of which deal exclusively with MCP config file path resolution (project-local vs user-scoped directories); none of the fast-path auth trigger files are modified, and no authentication behavior, token management, credential resolution, host classification, or AuthResolver semantics are changed.

---

OSS Growth Hacker: Strong story: "APM now configures Codex MCP project-locally, the same way it handles Cursor -- apm install is safe to run in shared repos without touching global Codex state." This directly removes a footgun that would surface as a confused enterprise team support ticket ("why did my ~/.codex/config.toml change?"). Side-channel to CEO: the behavior change is positive but currently invisible to existing Codex users; worth a one-line callout in the next release post. The CHANGELOG entry is the only conversion-surface issue: the backtick-wrapped feat(codex): prefix reads as internal commit metadata, not user prose.

---

CEO arbitration

All specialists are aligned: the architecture is correct, the security posture improves, and the UX follows established Cursor/OpenCode precedent. No disagreements to arbitrate. This is a solid external contribution (Christian Hessel, @nickolausone@posteo.de) that closes a real footgun for teams using Codex CLI in shared repos. The one required fix (CHANGELOG format) is mechanical. The optional follow-ups (ScopeContext dataclass, update_config return-type annotation on ABC, path security guard, redundant adapter instantiation) are all follow-up candidates, not merge blockers. The strategic call is straightforward: approve with the required fix applied, and call out the "project-local Codex MCP" improvement explicitly in the next release post.

---

Required actions before merge

1. CHANGELOG.md -- rewrite the ## [Unreleased] entry from conventional-commit format to user prose per the project's Keep-a-Changelog convention. The current entry opens with a backtick-wrapped feat(codex): prefix and runs to three clauses; replace with a single concise sentence ending with (#803). Example: - Codex CLI MCP config is now project-local (.codex/config.toml) during project installs and gated to active project targets, matching Cursor and OpenCode behavior; user-scope primitive deployment is also supported. (#803) -- or shorter.

---

Optional follow-ups

• ScopeContext dataclass: introduce a small ScopeContext(project_root: Path, user_scope: bool) value object to collapse the two-parallel-parameter threading visible in 10+ function signatures across mcp_integrator.py, operations.py, safe_installer.py, registry/operations.py. Eliminates the class of "forgot to pass user_scope" bugs at new call sites.
• MCPClientAdapter.update_config() return type: add -> bool | None annotation to the ABC and a docstring note that False/None means the write was skipped; prevents future implementers from silently breaking callers that check the return value.
• Path security guard: add ensure_path_within(resolved_config_path, self.project_root) in adapter get_config_path() implementations (or in the project_root property) as defense-in-depth, consistent with path_security.py convention.
• Pre-existing print() in codex.py:configure_mcp_server (~line 174): convert to logger.verbose_detail() / CommandLogger call in a follow-up cleanup PR.

Generated by PR Review Panel for issue #803 · ● 1M · ◷

[danielmeppiel]

Required actions before merge (all required + optional)

CHANGELOG.md -- rewrite the ## [Unreleased] entry from conventional-commit format to user prose per the project's Keep-a-Changelog convention. The current entry opens with a backtick-wrapped feat(codex): prefix and runs to three clauses; replace with a single concise sentence ending with (#803). Example: - Codex CLI MCP config is now project-local (.codex/config.toml) during project installs and gated to active project targets, matching Cursor and OpenCode behavior; user-scope primitive deployment is also supported. (#803) -- or shorter.
ScopeContext dataclass: introduce a small ScopeContext(project_root: Path, user_scope: bool) value object to collapse the two-parallel-parameter threading visible in 10+ function signatures across mcp_integrator.py, operations.py, safe_installer.py, registry/operations.py. Eliminates the class of "forgot to pass user_scope" bugs at new call sites.
MCPClientAdapter.update_config() return type: add -> bool | None annotation to the ABC and a docstring note that False/None means the write was skipped; prevents future implementers from silently breaking callers that check the return value.
Path security guard: add ensure_path_within(resolved_config_path, self.project_root) in adapter get_config_path() implementations (or in the project_root property) as defense-in-depth, consistent with path_security.py convention.
Pre-existing print() in codex.py:configure_mcp_server (~line 174): convert to logger.verbose_detail() / CommandLogger call in a follow-up cleanup PR.

[Nickolaus]

@danielmeppiel fixed failing failing ci build and rebased with main

Approved

[danielmeppiel]

@Nickolaus good one! no need to update against main next time, we have a merge queue (and updating manually makes reviews stale) thanks!