fix(mcp): allow third-party providers to approve project-scope .mcp.json servers (#696)

When a user runs openclaude with a third-party provider, project-scope
MCP servers added via `openclaude mcp add -s project ...` were silently
dropped from `/mcp` and `openclaude mcp list`. Re-adding the same
server printed "MCP server already exists in .mcp.json" but the server
never actually loaded. Reporter @gbmerrall pinpointed the cause to the
`if (usesAnthropicSetup)` gate around `handleMcpjsonServerApprovals`
in src/interactiveHelpers.tsx.

The MCP approval dialog and the CLAUDE.md external-includes warning
are about workspace trust, not about Anthropic auth. Gating them on
`usesAnthropicSetup` meant 3P-provider users never saw the dialog
that writes `enableAllProjectMcpServers: true` and
`enabledMcpjsonServers: [...]` to settings.local.json — without
those settings, the MCP server isn't loaded for use.

Drop the `usesAnthropicSetup` gate around the approval flow. The
inner logic is unchanged and `handleMcpjsonServerApprovals` already
early-returns when no servers are pending, so users without project
`.mcp.json` see no new behavior.

- src/interactiveHelpers.tsx: drop the gate, add a comment explaining
  why (and cite #696 so future readers can find context).
- src/__tests__/bugfixes.test.ts: +2 regression tests asserting the
  gate is gone and the issue is referenced.

Verified locally on Linux: build passes (v0.7.0), 1634 tests pass,
the 4 remaining failures (StartupScreen.test.ts, thinking.test.ts)
reproduce on main and are unrelated. The bundled dist/cli.mjs shows
`handleMcpjsonServerApprovals(root2)` running directly after
`setSessionTrustAccepted(true)` with no auth gate.

src/__tests__/bugfixes.test.ts

@@ -288,3 +288,30 @@ describe('Context overflow 500 fix', () => {
     expect(content).toContain('automatic compaction has failed')
   })
 })
+
+// ---------------------------------------------------------------------------
+// Fix N: Project-scope MCP servers from .mcp.json not detected for 3P providers (issue #696)
+// ---------------------------------------------------------------------------
+describe('Project-scope MCP approval — third-party providers (issue #696)', () => {
+  test('handleMcpjsonServerApprovals is NOT gated behind usesAnthropicSetup', async () => {
+    const content = await file('interactiveHelpers.tsx').text()
+
+    // The call site for handleMcpjsonServerApprovals must not sit inside an
+    // `if (usesAnthropicSetup) { ... }` block, or third-party providers will
+    // never get the dialog and project-scope .mcp.json servers will be silently
+    // dropped from /mcp listings (issue #696).
+    const approvalCallIdx = content.indexOf('await handleMcpjsonServerApprovals(root)')
+    expect(approvalCallIdx).toBeGreaterThan(-1)
+
+    // Look at the 800 chars BEFORE the call site for any `if (usesAnthropicSetup)`
+    // block that would still be open. Pick a window that's definitely inside the
+    // showSetupScreens function but not in earlier dialogs.
+    const before = content.slice(Math.max(0, approvalCallIdx - 800), approvalCallIdx)
+    expect(before).not.toMatch(/if\s*\(\s*usesAnthropicSetup\s*\)\s*{[^}]*$/)
+  })
+
+  test('issue #696 is referenced from the comment so future readers can find context', async () => {
+    const content = await file('interactiveHelpers.tsx').text()
+    expect(content).toContain('#696')
+  })
+})

src/interactiveHelpers.tsx

@@ -158,24 +158,25 @@ export async function showSetupScreens(root: Root, permissionMode: PermissionMod
     // Now that trust is established, prefetch system context if it wasn't already
     void getSystemContext();
 
-    // Skip MCP approval dialogs for third-party providers (no interactive auth prompts)
-    if (usesAnthropicSetup) {
-      // If settings are valid, check for any mcp.json servers that need approval
-      const {
-        errors: allErrors
-      } = getSettingsWithAllErrors();
-      if (allErrors.length === 0) {
-        await handleMcpjsonServerApprovals(root);
-      }
+    // MCP approval and external-includes warnings are about workspace
+    // trust, not about Anthropic auth. They must run for all providers
+    // — including third-party — otherwise project-scoped .mcp.json
+    // servers never get the approval that writes
+    // enableAllProjectMcpServers / enabledMcpjsonServers into
+    // settings.local.json, and the servers are silently dropped from
+    // /mcp and `mcp list` (issue #696).
+    const { errors: allErrors } = getSettingsWithAllErrors();
+    if (allErrors.length === 0) {
+      await handleMcpjsonServerApprovals(root);
+    }
 
-      // Check for claude.md includes that need approval
-      if (await shouldShowClaudeMdExternalIncludesWarning()) {
-        const externalIncludes = getExternalClaudeMdIncludes(await getMemoryFiles(true));
-        const {
-          ClaudeMdExternalIncludesDialog
-        } = await import('./components/ClaudeMdExternalIncludesDialog.js');
-        await showSetupDialog(root, done => <ClaudeMdExternalIncludesDialog onDone={done} isStandaloneDialog externalIncludes={externalIncludes} />);
-      }
+    // Check for claude.md includes that need approval
+    if (await shouldShowClaudeMdExternalIncludesWarning()) {
+      const externalIncludes = getExternalClaudeMdIncludes(await getMemoryFiles(true));
+      const {
+        ClaudeMdExternalIncludesDialog
+      } = await import('./components/ClaudeMdExternalIncludesDialog.js');
+      await showSetupDialog(root, done => <ClaudeMdExternalIncludesDialog onDone={done} isStandaloneDialog externalIncludes={externalIncludes} />);
     }
   }
 

src/utils/plugins/gitEnv.test.ts

@@ -1,104 +0,0 @@
-import { afterEach, beforeEach, describe, expect, test } from 'bun:test'
-
-import {
-  __resetGitEnvWarningForTesting,
-  buildGitChildEnv,
-  sanitizeEnvForGit,
-} from './gitEnv.js'
-
-describe('sanitizeEnvForGit', () => {
-  test('drops values containing LF', () => {
-    const result = sanitizeEnvForGit({
-      GOOD: 'value',
-      BAD_NEWLINE: 'line1\nline2',
-    })
-    expect(result.env).toEqual({ GOOD: 'value' })
-    expect(result.dropped).toEqual(['BAD_NEWLINE'])
-  })
-
-  test('drops values containing CR', () => {
-    const result = sanitizeEnvForGit({
-      GOOD: 'value',
-      BAD_CR: 'value\r',
-    })
-    expect(result.dropped).toEqual(['BAD_CR'])
-  })
-
-  test('drops values containing NUL', () => {
-    const result = sanitizeEnvForGit({
-      GOOD: 'value',
-      BAD_NUL: 'a\0b',
-    })
-    expect(result.dropped).toEqual(['BAD_NUL'])
-  })
-
-  test('drops keys whose name itself contains a control character', () => {
-    const result = sanitizeEnvForGit({
-      'BAD\nKEY': 'safe-value',
-      GOOD: 'value',
-    })
-    expect(result.env).toEqual({ GOOD: 'value' })
-    expect(result.dropped).toEqual(['BAD\nKEY'])
-  })
-
-  test('skips entries explicitly set to undefined without listing them as dropped', () => {
-    const result = sanitizeEnvForGit({
-      GOOD: 'value',
-      MAYBE: undefined,
-    })
-    expect(result.env).toEqual({ GOOD: 'value' })
-    expect(result.dropped).toEqual([])
-  })
-
-  test('returns input unchanged when nothing is unsafe', () => {
-    const env = { PATH: '/usr/bin:/bin', HOME: '/home/user', GIT_TERMINAL_PROMPT: '0' }
-    const result = sanitizeEnvForGit(env)
-    expect(result.env).toEqual(env)
-    expect(result.dropped).toEqual([])
-  })
-})
-
-describe('buildGitChildEnv', () => {
-  const ORIGINAL_BAD_KEY = 'OPENCLAUDE_TEST_BAD_ENV_FOR_GIT'
-  let originalValue: string | undefined
-
-  beforeEach(() => {
-    __resetGitEnvWarningForTesting()
-    originalValue = process.env[ORIGINAL_BAD_KEY]
-  })
-
-  afterEach(() => {
-    if (originalValue === undefined) {
-      delete process.env[ORIGINAL_BAD_KEY]
-    } else {
-      process.env[ORIGINAL_BAD_KEY] = originalValue
-    }
-  })
-
-  test('always sets the no-prompt overrides', () => {
-    const env = buildGitChildEnv()
-    expect(env.GIT_TERMINAL_PROMPT).toBe('0')
-    expect(env.GIT_ASKPASS).toBe('')
-  })
-
-  test('drops process.env values containing control characters (issue #751)', () => {
-    process.env[ORIGINAL_BAD_KEY] = 'paste-with-newline\n'
-    const env = buildGitChildEnv()
-    expect(env[ORIGINAL_BAD_KEY]).toBeUndefined()
-    expect(env.GIT_TERMINAL_PROMPT).toBe('0')
-  })
-
-  test('caller extras override process.env and the no-prompt defaults', () => {
-    const env = buildGitChildEnv({
-      GIT_TERMINAL_PROMPT: '1',
-      CUSTOM_KEY: 'custom-value',
-    })
-    expect(env.GIT_TERMINAL_PROMPT).toBe('1')
-    expect(env.CUSTOM_KEY).toBe('custom-value')
-  })
-
-  test('caller-provided unsafe extras are also dropped', () => {
-    const env = buildGitChildEnv({ EXTRA_BAD: 'a\rb' })
-    expect(env.EXTRA_BAD).toBeUndefined()
-  })
-})

src/utils/plugins/gitEnv.ts

@@ -1,70 +0,0 @@
-import { logForDebugging } from '../debug.js'
-
-/**
- * Git 2.30+ refuses to start when any environment value contains a NUL,
- * CR, or LF character ("Unsafe environment: control characters are not
- * allowed in values"). User shells frequently leak such values — a
- * copy-pasted API key with a trailing newline, or a terminal-set
- * variable with embedded escape sequences — which would otherwise break
- * every plugin clone or pull. We drop offending entries before forwarding
- * the environment to git.
- */
-const GIT_UNSAFE_VALUE_RE = /[\0\r\n]/
-
-const GIT_NO_PROMPT_ENV = {
-  GIT_TERMINAL_PROMPT: '0', // Prevent terminal credential prompts
-  GIT_ASKPASS: '', // Disable askpass GUI programs
-}
-
-let warnedAboutDroppedEnvKeys = false
-
-/**
- * Returns a copy of `env` with any entries whose key OR value contains
- * a NUL/CR/LF removed. The list of dropped key names is returned so
- * callers can log it without exposing the (possibly secret) values.
- */
-export function sanitizeEnvForGit(
-  env: NodeJS.ProcessEnv,
-): { env: NodeJS.ProcessEnv; dropped: string[] } {
-  const sanitized: NodeJS.ProcessEnv = {}
-  const dropped: string[] = []
-  for (const [key, value] of Object.entries(env)) {
-    if (value === undefined) continue
-    if (GIT_UNSAFE_VALUE_RE.test(key) || GIT_UNSAFE_VALUE_RE.test(value)) {
-      dropped.push(key)
-      continue
-    }
-    sanitized[key] = value
-  }
-  return { env: sanitized, dropped }
-}
-
-/**
- * Build the environment object passed to a git child process. Merges
- * `process.env` with the no-prompt overrides and any caller extras,
- * then strips entries that would trigger git's unsafe-value check. The
- * first batch of dropped key names is logged once per process so the
- * user can clean them up in their shell.
- */
-export function buildGitChildEnv(
-  extras?: NodeJS.ProcessEnv,
-): NodeJS.ProcessEnv {
-  const merged = { ...process.env, ...GIT_NO_PROMPT_ENV, ...(extras ?? {}) }
-  const { env, dropped } = sanitizeEnvForGit(merged)
-  if (dropped.length > 0 && !warnedAboutDroppedEnvKeys) {
-    warnedAboutDroppedEnvKeys = true
-    logForDebugging(
-      `git child env: dropped ${dropped.length} key(s) containing control characters: ${dropped.join(', ')}. Git 2.30+ rejects them; clean these up in your shell to forward them to git.`,
-      { level: 'warn' },
-    )
-  }
-  return env
-}
-
-/**
- * Test-only escape hatch that resets the once-per-process warning flag
- * so unit tests can exercise the warning path repeatedly.
- */
-export function __resetGitEnvWarningForTesting(): void {
-  warnedAboutDroppedEnvKeys = false
-}

src/utils/plugins/marketplaceManager.ts

@@ -53,7 +53,6 @@ import {
   getAddDirExtraMarketplaces,
 } from './addDirPluginSettings.js'
 import { markPluginVersionOrphaned } from './cacheUtils.js'
-import { buildGitChildEnv } from './gitEnv.js'
 import { classifyFetchError, logPluginFetch } from './fetchTelemetry.js'
 import { removeAllPluginsForMarketplace } from './installedPluginsManager.js'
 import {
@@ -507,6 +506,11 @@ function seedDirFor(installLocation: string): string | undefined {
  * Provides helpful error messages for common failure scenarios.
  * If a ref is specified, fetches and checks out that specific branch or tag.
  */
+// Environment variables to prevent git from prompting for credentials
+const GIT_NO_PROMPT_ENV = {
+  GIT_TERMINAL_PROMPT: '0', // Prevent terminal credential prompts
+  GIT_ASKPASS: '', // Disable askpass GUI programs
+}
 
 const DEFAULT_PLUGIN_GIT_TIMEOUT_MS = 120 * 1000
 
@@ -527,7 +531,7 @@ export async function gitPull(
   options?: { disableCredentialHelper?: boolean; sparsePaths?: string[] },
 ): Promise<{ code: number; stderr: string }> {
   logForDebugging(`git pull: cwd=${cwd} ref=${ref ?? 'default'}`)
-  const env = buildGitChildEnv()
+  const env = { ...process.env, ...GIT_NO_PROMPT_ENV }
   const baseArgs = ['-c', 'core.hooksPath=/dev/null']
   const credentialArgs = options?.disableCredentialHelper
     ? ['-c', 'credential.helper=']
@@ -840,7 +844,7 @@ export async function gitClone(
   const result = await execFileNoThrowWithCwd(gitExe(), args, {
     timeout: timeoutMs,
     stdin: 'ignore',
-    env: buildGitChildEnv(),
+    env: { ...process.env, ...GIT_NO_PROMPT_ENV },
   })
 
   // Scrub credentials from execa's error/stderr fields before any logging or
@@ -866,7 +870,7 @@ export async function gitClone(
           cwd: targetPath,
           timeout: timeoutMs,
           stdin: 'ignore',
-          env: buildGitChildEnv(),
+          env: { ...process.env, ...GIT_NO_PROMPT_ENV },
         },
       )
       if (sparseResult.code !== 0) {
@@ -885,7 +889,7 @@ export async function gitClone(
           cwd: targetPath,
           timeout: timeoutMs,
           stdin: 'ignore',
-          env: buildGitChildEnv(),
+          env: { ...process.env, ...GIT_NO_PROMPT_ENV },
         },
       )
       if (checkoutResult.code !== 0) {
@@ -1036,7 +1040,7 @@ export async function reconcileSparseCheckout(
   cwd: string,
   sparsePaths: string[] | undefined,
 ): Promise<{ code: number; stderr: string }> {
-  const env = buildGitChildEnv()
+  const env = { ...process.env, ...GIT_NO_PROMPT_ENV }
 
   if (sparsePaths && sparsePaths.length > 0) {
     return execFileNoThrowWithCwd(

src/utils/plugins/pluginLoader.ts

@@ -87,7 +87,6 @@ import { getAddDirEnabledPlugins } from './addDirPluginSettings.js'
 import { verifyAndDemote } from './dependencyResolver.js'
 import { classifyFetchError, logPluginFetch } from './fetchTelemetry.js'
 import { checkGitAvailable } from './gitAvailability.js'
-import { buildGitChildEnv } from './gitEnv.js'
 import { getInMemoryInstalledPlugins } from './installedPluginsManager.js'
 import { getManagedPluginNames } from './managedPlugins.js'
 import {
@@ -561,9 +560,7 @@ export async function gitClone(
   args.push(gitUrl, targetPath)
 
   const cloneStarted = performance.now()
-  const cloneResult = await execFileNoThrow(gitExe(), args, {
-    env: buildGitChildEnv(),
-  })
+  const cloneResult = await execFileNoThrow(gitExe(), args)
 
   if (cloneResult.code !== 0) {
     logPluginFetch(
@@ -582,7 +579,7 @@ export async function gitClone(
     const shallowFetchResult = await execFileNoThrowWithCwd(
       gitExe(),
       ['fetch', '--depth', '1', 'origin', sha],
-      { cwd: targetPath, env: buildGitChildEnv() },
+      { cwd: targetPath },
     )
 
     if (shallowFetchResult.code !== 0) {
@@ -594,7 +591,7 @@ export async function gitClone(
       const unshallowResult = await execFileNoThrowWithCwd(
         gitExe(),
         ['fetch', '--unshallow'],
-        { cwd: targetPath, env: buildGitChildEnv() },
+        { cwd: targetPath },
       )
 
       if (unshallowResult.code !== 0) {
@@ -615,7 +612,7 @@ export async function gitClone(
     const checkoutResult = await execFileNoThrowWithCwd(
       gitExe(),
       ['checkout', sha],
-      { cwd: targetPath, env: buildGitChildEnv() },
+      { cwd: targetPath },
     )
 
     if (checkoutResult.code !== 0) {
@@ -748,9 +745,7 @@ export async function installFromGitSubdir(
   }
   cloneArgs.push(gitUrl, cloneDir)
 
-  const cloneResult = await execFileNoThrow(gitExe(), cloneArgs, {
-    env: buildGitChildEnv(),
-  })
+  const cloneResult = await execFileNoThrow(gitExe(), cloneArgs)
   if (cloneResult.code !== 0) {
     throw new Error(
       `Failed to clone repository for git-subdir source: ${cloneResult.stderr}`,
@@ -761,7 +756,7 @@ export async function installFromGitSubdir(
     const sparseResult = await execFileNoThrowWithCwd(
       gitExe(),
       ['sparse-checkout', 'set', '--cone', '--', subdirPath],
-      { cwd: cloneDir, env: buildGitChildEnv() },
+      { cwd: cloneDir },
     )
     if (sparseResult.code !== 0) {
       throw new Error(
@@ -780,7 +775,7 @@ export async function installFromGitSubdir(
       const fetchSha = await execFileNoThrowWithCwd(
         gitExe(),
         ['fetch', '--depth', '1', 'origin', sha],
-        { cwd: cloneDir, env: buildGitChildEnv() },
+        { cwd: cloneDir },
       )
       if (fetchSha.code !== 0) {
         logForDebugging(
@@ -789,7 +784,7 @@ export async function installFromGitSubdir(
         const unshallow = await execFileNoThrowWithCwd(
           gitExe(),
           ['fetch', '--unshallow'],
-          { cwd: cloneDir, env: buildGitChildEnv() },
+          { cwd: cloneDir },
         )
         if (unshallow.code !== 0) {
           throw new Error(`Failed to fetch commit ${sha}: ${unshallow.stderr}`)
@@ -798,7 +793,7 @@ export async function installFromGitSubdir(
       const checkout = await execFileNoThrowWithCwd(
         gitExe(),
         ['checkout', sha],
-        { cwd: cloneDir, env: buildGitChildEnv() },
+        { cwd: cloneDir },
       )
       if (checkout.code !== 0) {
         throw new Error(`Failed to checkout commit ${sha}: ${checkout.stderr}`)
@@ -813,11 +808,9 @@ export async function installFromGitSubdir(
       const [checkout, revParse] = await Promise.all([
         execFileNoThrowWithCwd(gitExe(), ['checkout', 'HEAD'], {
           cwd: cloneDir,
-          env: buildGitChildEnv(),
         }),
         execFileNoThrowWithCwd(gitExe(), ['rev-parse', 'HEAD'], {
           cwd: cloneDir,
-          env: buildGitChildEnv(),
         }),
       ])
       if (checkout.code !== 0) {