Skip to content

Modernise MCP stack, improve stability, and preserve Gemini CLI OAuth#65

Open
jacobcxdev wants to merge 19 commits into
jamubc:mainfrom
jacobcxdev:upstream/fix-gemini-cli-oauth-stability
Open

Modernise MCP stack, improve stability, and preserve Gemini CLI OAuth#65
jacobcxdev wants to merge 19 commits into
jamubc:mainfrom
jacobcxdev:upstream/fix-gemini-cli-oauth-stability

Conversation

@jacobcxdev

@jacobcxdev jacobcxdev commented Apr 26, 2026

Copy link
Copy Markdown

Fixes #64.

Summary

  • modernise the MCP stack and dependency set while preserving Gemini CLI OAuth
  • add direct MCP stability verification and improve local verification/documentation
  • avoid a Claude Code-specific stdio disconnect triggered by progress notifications during tool calls

What changed

General improvements

  • upgrade @modelcontextprotocol/sdk to the current release line
  • update remaining dependencies to current versions
  • fix TypeScript/Zod compatibility issues introduced by the dependency refresh
  • preserve the Gemini CLI OAuth execution path (gemini -p ...)
  • add a hardened CLI executor
  • add a direct MCP client stability test
  • add/update verification scripts and documentation

Claude Code compatibility workaround

In testing, Claude Code disconnects this stdio server after a successful tools/call response if the request includes _meta.progressToken and the server emits progress notifications.

As a pragmatic workaround, this change makes progress notifications opt-in for this server path:

  • progress notifications are disabled by default
  • set GEMINI_MCP_ENABLE_PROGRESS=1 to re-enable them for clients that handle them correctly

This appears to be a Claude Code bug rather than a fundamental server problem:

  • repeated direct MCP SDK client calls remain stable
  • the server does not crash
  • Claude Code closes stdin after the successful response in the progress-enabled case

Anthropic bug report:

Diagnostics and portability

  • tracing is now opt-in via GEMINI_MCP_TRACE=1
  • trace output avoids logging raw tool arguments by default
  • the MCP client smoke test now uses fileURLToPath(...) for cross-platform path handling
  • signal-terminated child processes are now reported as failures rather than false successes
  • documentation and package metadata were brought back in line with upstream project identity

OAuth MCP configuration

Verified against official Gemini CLI auth selection: GOOGLE_GENAI_USE_GCA=true forces oauth-personal before API-key auth. The docs now recommend setting HOME, GEMINI_CLI_HOME, and GOOGLE_GENAI_USE_GCA=true for Claude MCP registrations that should use Google/OAuth rather than GEMINI_API_KEY.

Verification

  • npm run verify
  • npm run docs:build
  • repeated Gemini CLI OAuth smoke test
  • repeated direct MCP SDK client stability test
  • repeated live Claude Code MCP calls in the compatibility configuration

Notes

This keeps the core project behaviour intact:

  • Gemini calls still go through the installed Gemini CLI
  • OAuth remains owned by Gemini CLI
  • no API-key-based auth migration is introduced

The declared Node.js floor is now >=20.12.0, matching the current dependency set rather than the earlier stricter temporary requirement.

Copilot AI review requested due to automatic review settings April 26, 2026 19:04

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request transitions the project to a fork that routes Gemini calls through the local CLI to preserve OAuth, adding a hardened executor and improved error handling. However, the PR contains critical bugs including hallucinated dependency versions and calls to non-existent library functions. Additionally, the changes introduce cross-platform compatibility issues and a regression by disabling progress notifications globally.

Comment thread package.json
Comment thread src/tools/registry.ts
Comment thread src/tools/registry.ts
Comment thread src/tools/registry.ts
Comment thread README.md Outdated
Comment thread src/utils/fileTrace.ts Outdated
Comment thread scripts/test-mcp-client-stability.mjs Outdated
Comment thread src/index.ts Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR modernizes the MCP server’s dependency stack while preserving the “Gemini CLI OAuth via gemini -p ...” execution path, and adds local verification scripts aimed at improving stability—especially under Claude Code—by hardening CLI execution and instrumenting/debugging the stdio lifecycle.

Changes:

  • Upgrades MCP SDK + broader dependency refresh (including Zod v4 compatibility changes).
  • Replaces the previous command execution utility with a hardened spawn-based CLI executor + adds smoke/stability scripts.
  • Adds extensive stdio/server tracing and disables progress notifications to avoid a Claude Code stdio disconnect.

Reviewed changes

Copilot reviewed 17 out of 19 changed files in this pull request and generated 13 comments.

Show a summary per file
File Description
tsconfig.json Adds Node types to align TS compilation with Node globals.
src/utils/logger.ts Makes most logging debug-gated and routes logs to stderr.
src/utils/geminiExecutor.ts Switches Gemini CLI invocation to the new CLI executor and trims stdout.
src/utils/fileTrace.ts Adds file-based trace utility for runtime diagnostics.
src/utils/cliExecutor.ts Introduces hardened CLI execution wrapper around child_process.spawn.
src/tools/simple-tools.ts Migrates simple tools to the new CLI executor and returns trimmed stdout.
src/tools/registry.ts Updates Zod → JSON Schema conversion and tweaks argument parsing flow.
src/index.ts Adds extensive lifecycle tracing, removes notifications capability, and disables progress tokens.
scripts/test-mcp-stability.mjs Adds repeated Gemini CLI OAuth smoke test script.
scripts/test-mcp-client-stability.mjs Adds repeated MCP client request stability test script.
package.json Renames fork package + adds verify/smoke scripts and major dependency upgrades.
package-lock.json Locks updated dependency graph for the refreshed stack.
docs/superpowers/plans/2026-04-26-gemini-cli-oauth-stability.md Adds implementation plan documenting the stability work.
docs/installation.md Updates install docs toward local fork usage.
docs/index.md Updates docs site links to point at the fork.
docs/getting-started.md Updates getting started flow for local fork + OAuth guidance.
docs/api.md Documents tool behavior and stability validation steps.
README.md Updates project positioning, fork links, and setup steps.
CHANGELOG.md Adds unreleased notes about the stability/OAuth preservation work.
Comments suppressed due to low confidence (2)

docs/getting-started.md:126

  • The Warp config example sets command: "node" but still uses args: ["-y", "gemini-mcp-tool"] (an NPX pattern). This won’t work with node; update the example to pass the built dist/index.js path (or switch command back to npx if that’s intended).
```json
{
  "gemini-cli": {
    "command": "node",
    "args": [
      "-y",
      "gemini-mcp-tool"
    ],

src/tools/registry.ts:73

  • executeTool is currently written as a single-line if (...) { ... } try { ... } catch { ... } statement. This makes the control flow hard to read/review and is easy to break with future edits; please reformat into a normal multi-line block structure (and ensure consistent semicolons/ASI safety).
export async function executeTool(toolName: string, args: ToolArguments, onProgress?: (newOutput: string) => void): Promise<string> {
  const tool = toolRegistry.find(t => t.name === toolName);
  if (!tool) { throw new Error(`Unknown tool: ${toolName}`); } try { const validatedArgs = tool.zodSchema.parse(args) as ToolArguments;
    return tool.execute(validatedArgs, onProgress);
  } catch (error) { if (error instanceof ZodError) {
      const issues = error.issues.map(issue => `${issue.path.join('.')}: ${issue.message}`).join(', ');
      throw new Error(`Invalid arguments for ${toolName}: ${issues}`);

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/utils/fileTrace.ts Outdated
Comment thread package.json Outdated
Comment thread docs/installation.md
Comment thread src/index.ts Outdated
Comment thread src/index.ts Outdated
Comment thread src/index.ts
Comment thread docs/installation.md Outdated
Comment thread docs/getting-started.md
Comment thread README.md Outdated
Comment thread scripts/test-mcp-client-stability.mjs
Keep the upstream PR free of fork-only metadata while making diagnostics opt-in, progress notifications configurable, and child process exits report signalled termination correctly.
Make the API page client-agnostic and lower the declared Node.js floor to the truthful minimum supported by the current dependency set.
- Add vitest as devDependency
- Update test script to run vitest
- Add geminiExecutor.test.ts with four assertions:
    1. noFallback=true propagates QUOTA_EXCEEDED unchanged
    2. noFallback=false retains default fallback behaviour
    3. GEMINI_MCP_NO_FALLBACK=1 env var triggers gate without flag
    4. Normal success path unaffected with noFallback=true
- Tests 1 and 3 currently fail (gate not yet implemented)
Add opt-in --no-fallback CLI flag and GEMINI_MCP_NO_FALLBACK=1 env var
to gate the automatic quota-exceeded fallback from gemini-2.5-pro to
gemini-2.5-flash. When either is set, a 429/quota-exceeded error
propagates unchanged to the caller instead of silently downgrading.

Default behaviour is unchanged for callers that do not set the flag,
preserving backwards compatibility.

Changes:
- geminiExecutor.ts: add noFallback? parameter; insert noFallbackEffective
  gate before the Logger.warn + fallback block in the QUOTA_EXCEEDED catch
- index.ts: parse --no-fallback from process.argv; inject noFallback into
  tool args so the flag flows through the MCP dispatch layer
- ask-gemini.tool.ts: extract noFallback from args and pass to
  executeGeminiCLI
- constants.ts: add noFallback?: boolean to ToolArguments interface
- Rename package from gemini-mcp-tool to @jacobcxdev/gemini-mcp-tool
- Bump version to 1.2.0
- Add CHANGELOG entry for --no-fallback addition
- Update server version string in index.ts
@jamubc

jamubc commented Jun 1, 2026

Copy link
Copy Markdown
Owner

Thanks again for putting this together, and for the clearly serious effort behind it. I can see you set out to modernize and harden the tool, and I appreciate that. I want to be straight about where it actually lands rather than leave you guessing, so here is the honest read claim by claim. A fair amount of this comes down to context that lives in commits I merged after the 1.1.4 base this PR was built on, so there was genuinely no way for you to have seen it.

On the security motivation: I checked what npm audit actually flags against @modelcontextprotocol/sdk@0.5.0, and there are two advisories, not one. The DNS-rebinding issue (GHSA-w48q-cv73-mx4w) only applies to the HTTP/SSE transport's origin checks, and the ReDoS issue (GHSA-8r9q-7v3j-jr4g) only applies to servers that register resource templates with exploded patterns. This server is stdio only and registers no resource templates, so neither is reachable here. Clearing the audit is still worth doing, so I will bump @modelcontextprotocol/sdk to a current 1.x release (1.25.2 or newer, where both are patched) while keeping my own registry. I would rather do that as a focused change.

That focus matters, because this is not really an SDK bump. It also moves zod from 3 to 4 (which drops zod-to-json-schema and changes my registry), ai from 4 to 6, inquirer from 9 to 13, and typescript from 5 to 6, and it raises the Node floor from >=16.0.0 to >=20.12.0. Those are major-version jumps that introduce the same TypeScript and Zod breakage the PR then has to patch, and the Node change drops Node 16 and 18 users. The branch is also based on 1.1.4, while main is on 1.1.6 with a test suite, the path-traversal guards, and the emergency CVE-2026-0755 patch, none of which are here. So it cannot land as-is regardless of the individual pieces.

On the hardened CLI executor: the new src/utils/cliExecutor.ts calls spawn(..., { shell: false }) with no platform handling, and the PR rewires geminiExecutor.ts to use it instead of the existing commandExecutor. That would reintroduce the exact Windows failure I fixed on May 27: on Node 22+, launching the gemini.cmd shim with shell: false throws EINVAL. It would also drop the where-based shim resolution for when the server does not inherit the user's PATH, the RESOURCE_EXHAUSTED quota parsing, the ENOENT guidance, and the stdin path that sends long prompts to Gemini without hitting cmd.exe parsing or the command-line length limit. For this tool's real usage, the existing executor is the more hardened of the two. That fix only landed a month after you opened this, so this one is purely timing.

On progress notifications: this is the one I want to flag most clearly. The change disables progress by default for every client and hides it behind GEMINI_MCP_ENABLE_PROGRESS=1. Progress streaming is a core feature here: it is how long Gemini calls report partial output, and it runs through the whole tool chain. I do not want to turn it off by default for everyone to accommodate one client. That said, your bug report is excellent and completely valid. anthropics/claude-code#53617 is real, reproducible, and still open, and you did the work to isolate it. I would rather track that upstream and scope a workaround to the affected client if it comes to that, instead of defaulting the feature off globally.

The parts I do want to take from this: the Claude Code bug report itself, and the OAuth documentation clarifying that GOOGLE_GENAI_USE_GCA=true selects oauth-personal ahead of an API key, with HOME and GEMINI_CLI_HOME set. Those are real contributions and I am grateful for them.

So the plan: I will do the @modelcontextprotocol/sdk bump to 1.x myself as a focused change, keep the registry, keep progress on by default while tracking #53617, fold in the OAuth docs, and leave the wider dependency overhaul and the executor swap out. Thank you again for digging in, the bug report especially is going to help the project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Keeps disconnecting after every few successful calls in Claude Code

3 participants