fix(adapters): route Gemini config through project_root and user_scope

[edenfunf]

Description

GeminiClientAdapter resolved .gemini/settings.json from os.getcwd() in every method (get_config_path, update_config, configure_mcp_server), so the project_root constructor argument was silently ignored and user_scope=True had no effect at all even though supports_user_scope = True is declared. Callers that instantiated GeminiClientAdapter(project_root=...) from outside the target project either had their MCP writes silently dropped (when the current cwd had no .gemini/) or, worse, written into an unrelated cwd that happened to have one. apm install --global similarly had no effect on Gemini.

This change brings the adapter in line with the pattern every other client adapter (Codex, Claude, Cursor, OpenCode, VS Code) already follows:

• New _get_gemini_dir() helper routes on self.user_scope: ~/.gemini/ in user scope, self.project_root / ".gemini" in project scope.
• get_config_path, update_config, configure_mcp_server, and get_current_config all read through the helper -- no remaining os.getcwd() calls in this adapter.
• Project scope keeps its existing opt-in semantics (skip silently if .gemini/ is missing at project_root). User scope is unconditional and mkdirs the directory, matching the contract used by Claude / Codex / Copilot in user scope.
• Existing unit tests dropped their patch("os.getcwd", ...) setup and switched to injecting project_root= directly; the integration tests dropped their GeminiClientAdapter.__new__ + monkeypatch.chdir workaround for the same reason. Added regression coverage for cwd-pollution, cwd-fallback, the user-scope path, and that configure_mcp_server does not early-return in user scope when ~/.gemini/ is missing.

Fixes #1299

Type of change

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

Testing

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

Local verification:

• tests/unit/test_gemini_mcp.py -- 27 tests pass (20 existing kept green after the fixture rewrite, 7 new: 3 regression for project_root routing, 4 for user scope including the configure_mcp_server no-short-circuit case).
• tests/integration/test_gemini_integration.py -- 17 tests pass after replacing the __new__ + monkeypatch.chdir shim with GeminiClientAdapter(project_root=...).
• tests/unit/compilation/test_gemini_formatter.py -- 8 tests pass.
• Broader unit suite (tests/unit, ~1148 tests) shows no new failures introduced by this change.
• The original #1299 reproducer from the issue body now writes into project_root/.gemini/settings.json instead of cwd. A second variant where cwd also contains a .gemini/ directory confirms cwd is no longer polluted.
• apm install --target gemini --only mcp end-to-end (real CLI binary, self-defined stdio MCP server in apm.yml) configures <project>/.gemini/settings.json cleanly and leaves $HOME/.gemini/ untouched -- no regression in the project-scope install path.

[Copilot]

Pull request overview

Fixes Gemini MCP config path resolution to respect project_root and user_scope, eliminating accidental reads/writes via the process CWD and enabling --global/user-scope installs for Gemini in line with other adapters.

Changes:

• Add scope-aware Gemini directory resolution (~/.gemini for user scope, <project_root>/.gemini for project scope) and route all config operations through it.
• Update unit + integration tests to inject project_root directly (removing cwd/__new__ workarounds) and add regressions for CWD-pollution and user-scope behavior.
• Add a changelog entry describing the fix and its impact.

Reviewed changes

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

File	Description
src/apm_cli/adapters/client/gemini.py	Implements scope-aware .gemini directory resolution and removes remaining os.getcwd()-based routing in config reads/writes.
tests/unit/test_gemini_mcp.py	Refactors tests to pass project_root, adds regressions for correct routing + user-scope behavior.
tests/integration/test_gemini_integration.py	Removes chdir/__new__ shims; constructs GeminiClientAdapter(project_root=...) directly.
CHANGELOG.md	Documents the bugfix under Unreleased/Fixed.

[github-actions]

APM Review Panel: needs_rework

PR #1306 fixes GeminiClientAdapter scope routing but leaves the primary real-world caller (mcp_integrator.py) still using Path.cwd(), silently re-introducing the original #1299 bug.

cc @danielmeppiel -- a fresh advisory pass is ready for your review.

The adapter fix itself is clean and correctly-scoped: _get_gemini_dir() centralises the user_scope/project_root decision in a single pure method, the unit tests are well-formed, and docs/CHANGELOG are accurate. Every panelist with an active opinion agreed the adapter change is directionally correct and introduces no new regressions at the class boundary. That consensus is meaningful.

However, the test-coverage-expert's blocking finding -- independently verified by the orchestrator at lines 1128-1129 and 666 of mcp_integrator.py -- reveals that the stated fix is only half-complete. The primary real-world call site still reads Path.cwd() / '.gemini' for the opt-in guard and calls ClientFactory.create_client(runtime_name) without project_root=. Every live apm install invocation through the normal CLI path therefore still exhibits the original #1299 bug. The unit test suite bypasses this caller entirely by constructing the adapter directly, so CI passes. This is the strongest signal in the panel: a missing integration test on a secure-by-default / governed-by-policy surface is structurally a regression trap, and the verified source evidence makes it irrefutable on this commit.

The supply-chain-security-expert's atomic-write recommendation is the second-highest-priority follow-up: a crash mid-json.dump silently zeros out settings.json, and get_current_config() quietly resets to {} on JSONDecodeError, meaning all previously configured MCP servers can be lost. The base-class cwd fallback (python-architect) is latent risk worth a follow-up warning. The logging/UX gaps (cli-logging-expert, devx-ux-expert) and docs prose improvements (doc-writer, oss-growth-hacker) are genuine but non-blocking polish. No panelists disagreed on severity tiers.

Aligned with: Portable by manifest -- project_root is now the authoritative scope anchor for Gemini config writes. Secure by default -- the mcp_integrator.py caller still falls through to os.getcwd(), so the promise is not yet fully honoured end-to-end. Multi-harness/multi-host -- Gemini CLI user-scope parity now matches Copilot CLI and Codex CLI at the adapter layer; the full parity story requires the mcp_integrator fix. Pragmatic as npm -- silent failure on apm install -g --mcp is the exact anti-pattern APM exists to eliminate; this PR fixes the adapter but the user-visible behaviour is unchanged until the caller is patched.

Growth signal. PR #1306 completes user-scope parity across Copilot CLI, Codex CLI, and Gemini CLI at the adapter layer -- a concrete, quotable milestone for the 'universal AI package manager' narrative. Once the mcp_integrator caller is patched, this is worth a beat in the next release post: 'apm install -g --mcp now works correctly for Gemini CLI, writing to ~/.gemini/settings.json without any project setup.' The oss-growth-hacker's suggested CHANGELOG lead should replace the current maintainer-oriented bullet before the release post goes out.

Panel summary

Persona	B	R	N	Takeaway
Python Architect	0	1	2	Clean, targeted fix: _get_gemini_dir() centralises scope routing correctly; one nit on redundant double-call in update_config; no blocking issues.
CLI Logging Expert	0	2	2	Silent project-scope skip and silent user-scope dir creation are the two output gaps; no blocking regressions introduced by this PR.
DevX UX Expert	0	1	1	Correct scope fix; silent skip of workspace-only runtimes at -g needs user-visible feedback.
Supply Chain Security Expert	0	2	1	No blocking issues; two recommended fixes: add path-containment guard on project_root, and use atomic write for settings.json.
OSS Growth Hacker	0	2	1	Parity fix unlocks Gemini CLI global installs; CHANGELOG is dense but docs table is clear; no blocking growth issues.
Doc Writer	0	2	2	Docs are accurate and complete; two prose improvements recommended: table cell readability and unconditional-create caveat placement.
Test Coverage Expert	1	1	1	Unit regression tests are well-formed; blocking gap: mcp_integrator.py still uses Path.cwd() for Gemini and never passes project_root, no integration test covers this end-to-end path.

B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.

Top 5 follow-ups

1. [Test Coverage Expert] (blocking-severity) Fix mcp_integrator.py lines 1128-1129 and 666 to use project_root_path instead of Path.cwd() for the Gemini block, and add TestGeminiMCPIntegration::test_install_via_mcp_integrator_uses_project_root_not_cwd. -- The orchestrator independently verified the caller is still broken. The adapter fix is inert until this site is patched. Missing integration test on a governed-by-policy surface is a regression trap.
2. [Supply Chain Security Expert] Make the settings.json write atomic (write to tmp, os.replace()) and add ensure_path_within(resolved_gemini_dir, self.project_root) in _get_gemini_dir() for the project-scope branch. -- A crash mid-write silently zeros the MCP config; get_current_config() resets to {} on JSONDecodeError so all servers are lost on the next write. Path-containment guards against traversal via a malicious apm.yml project_root value.
3. [Python Architect] Emit a debug-level warning in MCPClientAdapter.project_root when _project_root is None and user_scope is False. -- The base-class cwd fallback is what makes the mcp_integrator omission silent; a warning would have surfaced the bug earlier and prevents the same class of omission in future adapters.
4. [CLI Logging Expert] Add logger.debug for silent project-scope skip; emit _rich_info when ~/.gemini/ is created for the first time in user scope. -- A silent no-op on apm install -g --mcp is indistinguishable from a failure; first-run directory creation should be visible to the user.
5. [OSS Growth Hacker] Rewrite the CHANGELOG bullet to lead with the user-facing win and add a 'what this means for you' callout to the docs table for Gemini-first users. -- The current CHANGELOG entry buries the user win inside implementation detail.

Architecture

classDiagram
    direction LR
    class MCPClientAdapter {
        <<Abstract>>
        +user_scope bool
        +project_root Path
        +get_config_path()* str
        +update_config(config_updates)* bool
        +get_current_config() dict
    }
    class CopilotClientAdapter {
        <<ConcreteBase>>
        +supports_user_scope bool
        +target_name str
        +configure_mcp_server(...) bool
        +_format_server_config(...) dict
    }
    class GeminiClientAdapter {
        <<Concrete>>
        +target_name str
        +mcp_servers_key str
        +_get_gemini_dir() Path
        +get_config_path() str
        +update_config(config_updates)
        +get_current_config() dict
        +configure_mcp_server(...) bool
        +_format_server_config(...) dict
    }
    class Path {
        <<stdlib>>
        +home() Path
    }
    MCPClientAdapter <|-- CopilotClientAdapter : extends
    CopilotClientAdapter <|-- GeminiClientAdapter : extends
    GeminiClientAdapter ..> Path : uses
    MCPClientAdapter ..> Path : project_root
    class GeminiClientAdapter:::touched
    classDef touched fill:#fff3b0,stroke:#d47600

Loading

flowchart TD
    A([apm install / --global]) --> B{user_scope?}
    B -- yes --> C["_get_gemini_dir\nPath.home / .gemini"]
    B -- no --> D["_get_gemini_dir\nproject_root / .gemini"]
    D --> E{.gemini dir exists?}
    E -- no --> F([return / silent skip])
    E -- yes --> G["get_config_path\ngemini_dir / settings.json"]
    C --> H["gemini_dir.mkdir\nparents=True exist_ok=True"]
    H --> G
    G --> I["get_current_config\nPath config_path .exists"]
    I -- missing --> J[current_config = {}]
    I -- present --> K[json.load settings.json]
    J --> L[merge mcpServers entries]
    K --> L
    L --> M[FS: json.dump to settings.json]
    M --> N([return success])
    style C fill:#fff3b0,stroke:#d47600
    style D fill:#fff3b0,stroke:#d47600
    style G fill:#fff3b0,stroke:#d47600
    style H fill:#fff3b0,stroke:#d47600

Loading

Recommendation

The adapter fix is correct and the unit tests are solid, but the PR as submitted cannot resolve issue #1299 end-to-end: the primary CLI call site (mcp_integrator.py lines 1128-1129, 666) still uses Path.cwd() for Gemini, so every live apm install invocation re-introduces the original bug. The maintainer should patch mcp_integrator.py and add the integration test (TestGeminiMCPIntegration::test_install_via_mcp_integrator_uses_project_root_not_cwd) in this PR before merge. The atomic-write and path-containment fixes from supply-chain-security can land as a fast follow in a separate PR if the maintainer prefers to keep scope tight.

---

Full per-persona findings

Python Architect

• [recommended] project_root fallback to os.getcwd() in MCPClientAdapter base is still present -- latent risk at construction sites that omit project_root= at src/apm_cli/adapters/client/base.py
  The fix correctly stops gemini.py from calling os.getcwd() directly. However, MCPClientAdapter.project_root still falls back to Path(os.getcwd()) when _project_root is None. Any adapter that omits project_root= at construction still gets cwd-relative paths.
  Suggested: Consider a follow-up that emits a debug-level warning when _project_root is None and project_root is accessed from a non-user-scope adapter.
• [nit] _get_gemini_dir() is called twice in update_config: once directly, once via get_config_path() at src/apm_cli/adapters/client/gemini.py
  Method is pure so no correctness impact, but redundant.
  Suggested: Use gemini_dir directly: config_path = gemini_dir / 'settings.json' in update_config().
• [nit] get_config_path() returns str but callers immediately wrap it in Path() at src/apm_cli/adapters/client/gemini.py
  Both update_config() and get_current_config() do Path(self.get_config_path()), revealing a leaky abstraction. Pre-existing shape.
  Suggested: Change get_config_path() to return Path directly in GeminiClientAdapter.

CLI Logging Expert

• [recommended] Silent return when project-scope .gemini/ is absent gives no feedback at src/apm_cli/adapters/client/gemini.py
  update_config() and configure_mcp_server() return silently when project scope is active but .gemini/ does not exist.
  Suggested: Add logger.debug("Skipping Gemini CLI: .gemini/ not found in %s", gemini_dir.parent) before the return.
• [recommended] Silent ~/.gemini/ directory creation in user scope -- user sees no feedback at src/apm_cli/adapters/client/gemini.py
  When user_scope=True and ~/.gemini/ does not yet exist, update_config() calls mkdir() with no output.
  Suggested: Capture was_created = not gemini_dir.is_dir() before mkdir and emit _rich_info conditionally.
• [nit] configure_mcp_server() returns True (not False) when .gemini/ is absent -- asymmetric with other early-exits at src/apm_cli/adapters/client/gemini.py
• [nit] get_current_config() swallows OSError and JSONDecodeError silently with no log at src/apm_cli/adapters/client/gemini.py
  Suggested: Add logger.warning("Could not read %s: %s", config_path, e) in the except block.

DevX UX Expert

• [recommended] Silent skip of Cursor/OpenCode at -g gives user no feedback; looks like a bug
  When a user runs apm install -g --mcp NAME, workspace-only runtimes are silently skipped. A no-op with no output is indistinguishable from a failure or a mis-typed flag.
  Suggested: Emit a _rich_info line for each runtime skipped at user scope, naming the runtime and why.
• [nit] Auto-creating ~/.gemini/ silently is fine but a one-line notice would help first-run users at src/apm_cli/adapters/client/gemini.py
  Suggested: After mkdir, emit _rich_info(f'Created {gemini_dir}') when the dir did not previously exist.

Supply Chain Security Expert

• [recommended] project_root is not validated with ensure_path_within before constructing the write path at src/apm_cli/adapters/client/gemini.py
  A value like '../../home/victim' could pass the is_dir() guard and write settings.json outside the intended tree. The codebase already has ensure_path_within and validate_path_segments for exactly this.
  Suggested: In _get_gemini_dir() (project scope branch), call path_security.ensure_path_within(resolved_gemini_dir, self.project_root) after resolving symlinks.
• [recommended] json.dump write to settings.json is not atomic; corrupt file on crash silently wipes MCP config at src/apm_cli/adapters/client/gemini.py
  open(config_path, 'w') truncates immediately; a crash mid-json.dump leaves a zero-byte or partial JSON file. get_current_config() resets to {} on JSONDecodeError, overwriting all previously configured servers.
  Suggested: Write to a temp file in the same directory, then os.replace() atomically.
• [nit] mkdir(parents=True) is redundant in project-scope path after is_dir() guard at src/apm_cli/adapters/client/gemini.py
  Suggested: Drop parents=True for project scope.

OSS Growth Hacker

• [recommended] CHANGELOG entry is thorough but written for maintainers, not users -- shrink it for release notes at CHANGELOG.md
  Suggested: Lead with 'apm install -g --mcp now routes to ~/.gemini/settings.json for Gemini CLI users (user-scope was previously silently ignored).'
• [recommended] Updated docs table and prose are accurate but missing a 'what this means for you' sentence for Gemini-first users at docs/src/content/docs/consumer/install-mcp-servers.md
  Suggested: Add: 'Gemini CLI now supports apm install -g --mcp -- writes to ~/.gemini/settings.json, no project setup required.'
• [nit] No release beat or social angle attached to this fix
  Suggested: Flag for the next release post: 'Gemini CLI global MCP installs fixed -- apm install -g --mcp now works correctly for all three user-scope runtimes.'

Doc Writer

• [recommended] Table cell for Gemini is overloaded; consider splitting path and trigger condition for readability at docs/src/content/docs/consumer/install-mcp-servers.md
  Suggested: Match the Claude Code row style: '.gemini/settings.json (project, opt-in) or ~/.gemini/settings.json (-g)'
• [recommended] "creates ~/.gemini/ if needed" caveat buried mid-paragraph; surface as standalone sentence at docs/src/content/docs/consumer/install-mcp-servers.md
  This is the only runtime where APM will unconditionally create a user-home directory.
  Suggested: End the opt-in paragraph with: 'Gemini user scope (-g) is unconditional: APM creates ~/.gemini/ if it does not exist.'
• [nit] "routes the write to" is slightly indirect; prefer active verb at docs/src/content/docs/consumer/install-mcp-servers.md
• [nit] CHANGELOG entry is accurate but very long; the parenthetical history lesson is changelog noise at CHANGELOG.md

Test Coverage Expert

• [blocking] mcp_integrator.py gemini block still uses Path.cwd() and does not pass project_root to ClientFactory.create_client; no test covers this cross-module path at src/apm_cli/integration/mcp_integrator.py
  The PR fixes GeminiClientAdapter to honour project_root, but the primary real-world caller at line 1128 still reads (Path.cwd() / '.gemini').is_dir() and calls ClientFactory.create_client(runtime_name) WITHOUT project_root=project_root_path. Every live apm install invocation still exhibits the original [BUG] GeminiClientAdapter ignores project_root/user_scope and writes MCP config under cwd #1299 bug. The integration test suite bypasses this caller entirely.
  Proof (missing at integration-with-fixtures): tests/integration/test_gemini_integration.py::TestGeminiMCPIntegration::test_install_via_mcp_integrator_uses_project_root_not_cwd -- proves: When apm install runs in a directory different from the project root, MCP config is written to project_root/.gemini/settings.json, not to cwd/.gemini/settings.json
• [recommended] test_falls_back_to_cwd_when_project_root_not_passed codifies the live mcp_integrator bug path as a named backward-compat contract at tests/unit/test_gemini_mcp.py
  mcp_integrator.py relies on exactly this fallback (it never passes project_root for gemini). The test should note the fallback exists for callers that have not yet migrated.
  Proof (passed at unit): tests/unit/test_gemini_mcp.py::TestGeminiProjectRootRouting::test_falls_back_to_cwd_when_project_root_not_passed
• [nit] test_user_scope_configure_mcp_server patches copilot module, not gemini module; correct but fragile if import chain changes at tests/unit/test_gemini_mcp.py
  Proof (passed at unit): tests/unit/test_gemini_mcp.py::TestGeminiUserScope::test_user_scope_configure_mcp_server_does_not_short_circuit

Auth Expert -- inactive

No auth-relevant files were changed; the PR only routes file-system paths (project_root vs os.getcwd()) in gemini.py and its tests/docs.

_{This panel is advisory. It does not block merge. Re-apply the
panel-review label after addressing feedback to re-run.}

Note

🔒 Integrity filter blocked 2 items

The following items were blocked because they don't meet the GitHub integrity level.

• #1306 pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
• fix(adapters): route Gemini config through project_root and user_scope #1306 pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".

To allow these resources, lower min-integrity in your GitHub frontmatter:

tools:
  github:
    min-integrity: approved  # merged | approved | unapproved | none

Generated by PR Review Panel for issue #1306 · ● 3.6M · ◷

[github-actions]

APM Review Panel: ship_with_followups

Gemini global MCP install now works -- ship with followups: fix the stale warning text that still excludes 'gemini', add the missing remove_stale regression trap, and correct the True return contract on opt-in skip.

cc @danielmeppiel -- a fresh advisory pass is ready for your review.

The core bug fix is sound. The _get_gemini_dir() helper correctly routes project-scope writes to project_root/.gemini and user-scope writes to Path.home()/.gemini, replacing three os.getcwd() / Path.cwd() calls. Unit test evidence (passed) confirms GeminiClientAdapter writes only to project_root/.gemini and never pollutes cwd/.gemini when both directories exist. This is the regression-trap that matters most for the adapter, and it holds. The fix unlocks apm install -g --mcp NAME for Gemini CLI for the first time; that is a real adoption event.

Two findings should be resolved before or immediately after merge. First, the devx-ux finding on the stale fallback warning is not cosmetic: a user running exactly the scenario this PR enables (apm install -g --mcp NAME with only Gemini CLI detected) can still hit the No runtimes support user-scope MCP installation (supported: copilot, codex) path and see a false failure signal. The fix is a one-line string change and carries zero risk. It should go in this PR. Second, the test-coverage finding on remove_stale is load-bearing: the test suite never enters the Gemini deletion branch with stale_names non-empty, so the project_root_path/.gemini/settings.json path on line 666 of mcp_integrator.py could silently regress to Path.cwd() and no CI check would catch it. Evidence outcome is missing on a path tagged to the core promise of this PR; this ranks above any recommended-opinion finding from any other persona.

The remaining recommended findings are genuine but deferrable. The True return on opt-in gate skip inflating configured_count is a contract smell the devx-ux expert correctly names, but it predates this PR and can be addressed in a follow-on without blocking adoption. The supply-chain ensure_path_within gap is pre-existing across all adapters; singling out this PR would be inconsistent. The doc and CHANGELOG gaps (VS Code omission, overlong entry, missing Gemini hero command) are all low-risk edits that can ship as a fast follow. The cli-logging silence on ~/.gemini/ creation is a recommended UX improvement, not a blocking defect.

Dissent. The supply-chain-expert flagged project_root path containment as recommended, which would normally warrant follow-up tracking. However, the finding itself acknowledges this is a pre-existing gap shared by all adapters and not a regression introduced here. Elevating it to blocking for this PR would be inconsistent with how the gap is treated across the rest of the adapter surface. Tracked as a follow-up; not blocking. The test-coverage-expert nit on the integration test outcome being unknown (pytest absent in review env) is noted -- that finding is correctly downweighted to opinion per panel rules and does not affect the primary evidence weight of the missing remove_stale finding.

Aligned with: Portable by Manifest, Secure by Default, Multi-Harness / Multi-Host, Pragmatic as npm.

Growth signal. Gemini CLI is an accelerating adoption vector with a user base distinct from Copilot and Claude. This fix is the right moment to add a copyable Gemini-specific one-liner to install-mcp-servers.md and to shorten the CHANGELOG entry to a two-sentence user signal: 'Gemini CLI users can now run apm install -g --mcp NAME to register MCP servers globally -- this did not work before.' The story angle -- 'APM is the only package manager that writes MCP config across Copilot, Claude, Codex, AND Gemini with a single command' -- is defensible and should be carried into release comms.

Panel summary

Persona	B	R	N	Takeaway
Python Architect	0	1	2	Sound bug-fix: _get_gemini_dir() aligns Gemini with the MCPClientAdapter project_root contract; one implicit correctness fix and one nit on return-type consistency.
CLI Logging Expert	0	1	2	Silent ~/.gemini/ creation on first user-scope write is the main UX gap; opt-in skip in configure_mcp_server returns True with no trace, masking skips in verbose mode.
DevX UX Expert	0	2	1	Stale 'supported: copilot, codex' warning text misleads Gemini users at -g; silent True return on skipped project-scope write inflates success counts.
Supply Chain Security Expert	0	1	2	No blocking supply-chain issues; project_root path containment gap is pre-existing across all adapters, not a regression introduced here.
OSS Growth Hacker	0	2	1	Unblocking apm install -g --mcp for Gemini CLI is a genuine adoption unlock; docs are correct but miss a copyable one-liner that would make it shareable.
Doc Writer	0	1	2	Doc changes are accurate and voice-consistent; VS Code omitted from workspace-only list and CHANGELOG entry is one unparseable run-on sentence.
Test Coverage Expert	0	1	2	Install path has unit + integration regression traps; remove_stale Gemini cleanup block has no test that enters the deletion branch with stale entries.

B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.

Top 5 follow-ups

1. [DevX UX Expert] (blocking-severity) Fix the stale fallback warning to include gemini in the supported-runtimes list in mcp_integrator.py -- A user running the exact scenario this PR enables can still see supported: copilot, codex and receive a false failure signal. One-line fix, zero risk, should be in this PR.
2. [Test Coverage Expert] Add integration regression trap test_remove_stale_gemini_uses_project_root_not_cwd that exercises the deletion branch with stale_names non-empty -- Evidence outcome is missing on the direct target of this bug fix; without this trap the line 666 project_root_path fix can silently regress to Path.cwd() and no CI check catches it.
3. [DevX UX Expert] Change configure_mcp_server to return a SKIPPED sentinel (not True) when the project opt-in gate fires -- Current behavior inflates the configured server count and gives users a false success signal; this is a correctness issue in the user-visible output contract.
4. [Doc Writer] Add VS Code to the workspace-only runtimes list in install-mcp-servers.md and split the CHANGELOG entry into problem / fix / reference lines -- VS Code is the most widely used harness in the table; omitting it from the scope-skip note misleads the majority of users reading that section.
5. [OSS Growth Hacker] Add a copyable Gemini global install example to install-mcp-servers.md and prepend a two-sentence user-facing summary to the CHANGELOG entry -- The doc page is already a strong SEO surface; the missing example reduces time-to-first-success for Gemini CLI users landing there.

Architecture

classDiagram
    direction LR
    class MCPClientAdapter {
        <<Abstract>>
        +project_root Path
        +user_scope bool
        +supports_user_scope bool
        +get_config_path()* str
        +update_config(config_updates)* bool
        +get_current_config()* dict
    }
    class CopilotClientAdapter {
        <<ConcreteBase>>
        +supports_user_scope bool
        +configure_mcp_server(...) bool
    }
    class GeminiClientAdapter {
        <<ConcreteStrategy>>
        +supports_user_scope bool
        +target_name str
        +_get_gemini_dir() Path
        +get_config_path() str
        +update_config(config_updates) bool
        +get_current_config() dict
        +configure_mcp_server(...) bool
    }
    class MCPIntegrator {
        <<Orchestrator>>
        +install_mcp_servers(project_root, ...) void
        +uninstall_mcp_servers(project_root, ...) void
    }
    MCPClientAdapter <|-- CopilotClientAdapter : extends
    CopilotClientAdapter <|-- GeminiClientAdapter : extends
    MCPIntegrator ..> GeminiClientAdapter : instantiates
    class GeminiClientAdapter:::touched
    class MCPIntegrator:::touched
    classDef touched fill:#fff3b0,stroke:#d47600

Loading

flowchart TD
    A["CLI: apm install SERVER --runtime gemini"] --> B["MCPIntegrator.install_mcp_servers()"]
    B --> C["GeminiClientAdapter(project_root=project_root_path)"]
    C --> D["configure_mcp_server()"]
    D --> E{"user_scope?"}
    E -- yes --> F["_get_gemini_dir() -> Path.home() / .gemini"]
    E -- no --> G["_get_gemini_dir() -> project_root / .gemini"]
    G --> H{"gemini dir exists?"}
    H -- no --> I["return True [opt-in skip]"]
    H -- yes --> J["update_config(server_config)"]
    F --> J
    J --> K["get_current_config() reads settings.json"]
    K --> L["config_path.parent.mkdir(parents=True)"]
    L --> M["json.dump -> settings.json"]
    B --> N["remove_stale: stale-server cleanup"]
    N --> O["project_root_path / .gemini / settings.json  [was Path.cwd()]"]
    O --> P["read/write settings.json"]

Loading

Recommendation

Ship after the stale warning text is corrected to include gemini -- that one-liner is the only change that is both trivially safe and directly undermines the adoption unlock headline if left broken. The remaining followups (remove_stale regression trap, SKIPPED sentinel return, doc gaps, Gemini hero command) are all real and should be filed as issues against the next patch, but none of them block the core fix from reaching users. The unit test evidence is positive, the adapter contract is now consistent, and Gemini CLI global MCP install works for the first time.

---

Full per-persona findings

Python Architect

• [recommended] Implicit correctness fix: old get_current_config() returned True (not {}) on missing dir; this PR silently resolves it at tests/unit/test_gemini_mcp.py
  The removed branch if not gemini_dir.is_dir(): return True was a latent bug -- callers treating the return as a config dict would get a bool. The rewrite to check config_path.exists() first and return {} is correct. Not called out in the PR description or tests, so a future reader may not know the old behavior was wrong.
  Suggested: Add a test: with no .gemini dir and no settings.json, assert get_current_config() returns {} (not True). Name it test_get_current_config_returns_empty_dict_when_no_dir.

• [nit] get_config_path() returns str but every caller immediately re-wraps with Path() at src/apm_cli/adapters/client/gemini.py:69
  Returning Path directly removes the double-wrap and makes the contract consistent.
  Suggested: def get_config_path(self) -> Path: return self._get_gemini_dir() / 'settings.json'

• [nit] configure_mcp_server() calls _get_gemini_dir() twice (guard + via update_config) at src/apm_cli/adapters/client/gemini.py:247
  Both calls are cheap -- not a correctness issue, but a shared local variable would make the intent explicit.

CLI Logging Expert

• [recommended] update_config creates ~/.gemini/ silently on first user-scope write -- no user-visible message at any verbosity level at src/apm_cli/adapters/client/gemini.py
config_path.parent.mkdir(parents=True, exist_ok=True) runs unconditionally when user_scope=True and ~/.gemini/ does not yet exist. In --verbose mode there is equally nothing.
  Suggested: Before mkdir, add: if not gemini_dir.is_dir(): logger.debug('Creating %s for Gemini CLI user configuration', gemini_dir)

• [nit] update_config silent return on project opt-in gate has no debug trace -- verbose mode cannot distinguish skip from write at src/apm_cli/adapters/client/gemini.py
  Suggested: logger.debug('Skipping Gemini project-scope write -- %s does not exist (opt-in)', gemini_dir) before the return.

• [nit] configure_mcp_server returns True silently when project opt-in gate fires -- callers receive a success signal for a no-op at src/apm_cli/adapters/client/gemini.py
  Suggested: logger.debug('Gemini opt-in gate: %s absent, skipping configure_mcp_server', self._get_gemini_dir()) before return True.

DevX UX Expert

• [recommended] Stale fallback warning names only copilot, codex as user-scope runtimes, excluding gemini at src/apm_cli/integration/mcp_integrator.py
  A user running apm install -g --mcp NAME who has only Gemini CLI detected will see No runtimes support user-scope MCP installation (supported: copilot, codex) and return 0 -- the exact scenario this PR was meant to fix.
  Suggested: logger.warning('No runtimes support user-scope MCP installation (supported: copilot, codex, gemini)')

• [recommended] configure_mcp_server returns True (success) when project-scope .gemini/ dir is absent -- a silent no-op that inflates the configured count at src/apm_cli/adapters/client/gemini.py
  The caller increments configured_count on True, so apm install --mcp NAME reports N servers configured even when Gemini wrote nothing.
  Suggested: Return None or a SKIPPED sentinel instead of True when the opt-in gate fires.

• [nit] get_current_config silently returns {} on JSONDecodeError, so a corrupt settings.json is overwritten without warning at src/apm_cli/adapters/client/gemini.py
  Suggested: Log a warning via _rich_warning when JSONDecodeError is caught.

Supply Chain Security Expert

• [recommended] project_root is used to construct write paths without ensure_path_within validation at src/apm_cli/adapters/client/gemini.py
self.project_root / '.gemini' is constructed with no containment check. Pre-existing gap shared by all adapters, but the new user-scope unconditional write path widens the impact.
  Suggested: Call ensure_path_within(gemini_dir, self.project_root) for the project-scope branch. File a companion issue to retrofit across all adapters.

• [nit] get_config_path returns str, then update_config immediately re-wraps in Path -- obscures where path construction happens, making audits harder at src/apm_cli/adapters/client/gemini.py

• [nit] config_path.parent.mkdir is reachable for project-scope despite the is_dir() guard earlier -- effectively dead code in that branch, could confuse future readers who relax the guard at src/apm_cli/adapters/client/gemini.py
  Suggested: Wrap in if self.user_scope: to make the intent explicit.

OSS Growth Hacker

• [recommended] Docs update correct but lacks a copyable hero command for the new Gemini global flow
  Gemini CLI users landing on install-mcp-servers.md get no quick-copy anchor for the exact command that now works.
  Suggested: Add a short callout: apm install -g --mcp io.github.github/github-mcp-server immediately after the Gemini row.

• [recommended] CHANGELOG entry is correct but written for maintainers, not users -- 200+ words of implementation detail, no user-facing lead
  Suggested: Prepend: 'Gemini CLI users can now run apm install -g --mcp NAME to register MCP servers globally -- this did not work before.'

• [nit] The opt-in note for Gemini user scope buries the lede -- lead with the unconditional create behavior, then explain the project-scope gate.

Auth Expert -- inactive

No auth-critical files changed (auth.py, token_manager.py, github_downloader.py, etc. all untouched); PR strictly reroutes Gemini config file paths from os.getcwd() to project_root with no effect on credential resolution or token handling.

Doc Writer

• [recommended] VS Code omitted from workspace-only runtimes list, misleading for the most common harness at docs/src/content/docs/consumer/install-mcp-servers.md
  The prose says Workspace-only runtimes (e.g. Cursor, OpenCode) are skipped at user scope but omits VS Code, the most widely used harness in the table.
  Suggested: Workspace-only runtimes (VS Code, Cursor, OpenCode) are skipped at user scope.

• [nit] CHANGELOG entry is one unparseable 200-word sentence at CHANGELOG.md
  Suggested: Split into: (1) one-line problem statement, (2) one-line fix statement, (3) reference line for test and docs.

• [nit] Gemini table cell is significantly longer than peer rows, reducing scanability at docs/src/content/docs/consumer/install-mcp-servers.md
  Suggested: Shorten to .gemini/settings.json (project) or ~/.gemini/settings.json (-g) and rely on the paragraph below for the opt-in gate explanation.

Test Coverage Expert

• [recommended] remove_stale Gemini cleanup path has no test that exercises actual entry removal when project_root differs from cwd at src/apm_cli/integration/mcp_integrator.py
  All tests pass stale_names=set(), so the Gemini deletion branch is never entered. If line 666 silently regressed to Path.cwd() the test suite would not catch it.
  Proof (missing at integration-with-fixtures): tests/integration/test_gemini_integration.py::test_remove_stale_gemini_uses_project_root_not_cwd -- proves: apm uninstall removes the Gemini MCP entry from project_root/.gemini/settings.json, not cwd/.gemini/settings.json, when cwd differs from project_root
  assert 'stale-server' not in json.loads((project_root / '.gemini' / 'settings.json').read_text()).get('mcpServers', {})

• [nit] Integration regression test outcome cannot be certified by run (pytest absent in review env); outcome downgraded to unknown per S7 PROBE RULE at tests/integration/test_gemini_integration.py
test_install_via_mcp_integrator_uses_project_root_not_cwd exists and reads correctly as a valid regression trap. Code review confirms it exercises the exact bug site.
  Proof (unknown at integration-with-fixtures): tests/integration/test_gemini_integration.py::test_install_via_mcp_integrator_uses_project_root_not_cwd -- proves: MCPIntegrator.install writes to project_root/.gemini/settings.json and not cwd when they differ
  assert settings.exists() and 'regression-1299-srv' in data.get('mcpServers', {})

• [nit] Unit tests for TestGeminiProjectRootRouting and TestGeminiUserScope are well-formed and cover the adapter fix at tests/unit/test_gemini_mcp.py
  All three routing scenarios covered: project_root used when cwd lacks .gemini/, cwd not polluted when both dirs have .gemini/, fallback to cwd when no project_root passed. No gaps at the unit tier.
  Proof (passed at unit): tests/unit/test_gemini_mcp.py::TestGeminiProjectRootRouting.test_does_not_pollute_cwd_when_cwd_also_has_gemini -- proves: GeminiClientAdapter writes only to project_root/.gemini/ and never to cwd/.gemini/ when both directories exist
  self.assertTrue((self.project_root / '.gemini' / 'settings.json').exists()); self.assertFalse((self.cwd_root / '.gemini' / 'settings.json').exists())

_{This panel is advisory. It does not block merge. Re-apply the
panel-review label after addressing feedback to re-run.}

Note

🔒 Integrity filter blocked 2 items

The following items were blocked because they don't meet the GitHub integrity level.

• #1306 pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
• fix(adapters): route Gemini config through project_root and user_scope #1306 pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".

To allow these resources, lower min-integrity in your GitHub frontmatter:

tools:
  github:
    min-integrity: approved  # merged | approved | unapproved | none

Generated by PR Review Panel for issue #1306 · ● 3.1M · ◷