Compare commits

..

1 Commits

Author SHA1 Message Date
gnanam1990
5ae055d30b 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.
2026-04-28 12:13:33 +05:30
8 changed files with 54 additions and 168 deletions

View File

@@ -421,16 +421,3 @@ ANTHROPIC_API_KEY=sk-ant-your-key-here
# WEB_CUSTOM_ALLOW_HTTP=false — set "true" to allow http:// URLs
# WEB_CUSTOM_ALLOW_PRIVATE=false — set "true" to target localhost/private IPs
# (needed for self-hosted SearXNG)
# ── Config directory override ───────────────────────────────────────
#
# By default openclaude stores per-user state under ~/.openclaude
# (and falls back to ~/.claude for installs that pre-date the rename).
# Set this to point openclaude at a different directory — useful for
# isolating profiles or sharing config across machines.
#
# OPENCLAUDE_CONFIG_DIR=/path/to/dir — preferred name
# CLAUDE_CONFIG_DIR=/path/to/dir — legacy alias (still works)
#
# When both are set with different values, OPENCLAUDE_CONFIG_DIR wins
# and a warning is logged once per process.

View File

@@ -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')
})
})

View File

@@ -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} />);
}
}

View File

@@ -3,11 +3,7 @@ import { homedir } from 'os'
import { join } from 'path'
import { fileSuffixForOauthConfig } from '../constants/oauth.js'
import { isRunningWithBun } from './bundledMode.js'
import {
getClaudeConfigHomeDir,
isEnvTruthy,
resolveConfigDirEnv,
} from './envUtils.js'
import { getClaudeConfigHomeDir, isEnvTruthy } from './envUtils.js'
import { findExecutable } from './findExecutable.js'
import { getFsImplementation } from './fsOperations.js'
import { which } from './which.js'
@@ -26,11 +22,7 @@ export const getGlobalClaudeFile = memoize((): string => {
}
const oauthSuffix = fileSuffixForOauthConfig()
const configDir =
resolveConfigDirEnv({
openClaudeConfigDir: process.env.OPENCLAUDE_CONFIG_DIR,
legacyConfigDir: process.env.CLAUDE_CONFIG_DIR,
}) ?? homedir()
const configDir = process.env.CLAUDE_CONFIG_DIR || homedir()
// Default to .openclaude.json. Fall back to .claude.json only if the new
// file doesn't exist yet and the legacy one does (same migration pattern

View File

@@ -3,39 +3,6 @@ import { existsSync } from 'fs'
import { homedir } from 'os'
import { join } from 'path'
/**
* Resolves the override env value for the config home directory.
* `OPENCLAUDE_CONFIG_DIR` is preferred — `CLAUDE_CONFIG_DIR` is the legacy
* Anthropic name kept working for backward compatibility. When both are set
* and disagree, `OPENCLAUDE_CONFIG_DIR` wins and we warn once so the user
* can clean up. Exported for tests.
*/
let warnedAboutConflictingConfigDirEnvs = false
export function resolveConfigDirEnv(options?: {
openClaudeConfigDir?: string
legacyConfigDir?: string
warn?: (message: string) => void
}): string | undefined {
const open = options?.openClaudeConfigDir
const legacy = options?.legacyConfigDir
if (open && legacy && open !== legacy && !warnedAboutConflictingConfigDirEnvs) {
warnedAboutConflictingConfigDirEnvs = true
options?.warn?.(
`Both OPENCLAUDE_CONFIG_DIR and CLAUDE_CONFIG_DIR are set to different values. Using OPENCLAUDE_CONFIG_DIR=${open}; ignoring CLAUDE_CONFIG_DIR=${legacy}.`,
)
}
return open || legacy || undefined
}
/**
* Test-only escape hatch — resets the once-per-process conflict warning so
* unit tests can re-trigger it.
*/
export function __resetConfigDirEnvWarningForTesting(): void {
warnedAboutConflictingConfigDirEnvs = false
}
export function resolveClaudeConfigHomeDir(options?: {
configDirEnv?: string
homeDir?: string
@@ -63,21 +30,13 @@ export function resolveClaudeConfigHomeDir(options?: {
return openClaudeDir.normalize('NFC')
}
// Memoized: 150+ callers, many on hot paths. Keyed off both override env
// vars so tests that change either get a fresh value without explicit
// cache.clear.
// Memoized: 150+ callers, many on hot paths. Keyed off CLAUDE_CONFIG_DIR so
// tests that change the env var get a fresh value without explicit cache.clear.
export const getClaudeConfigHomeDir = memoize(
(): string => resolveClaudeConfigHomeDir({
configDirEnv: resolveConfigDirEnv({
openClaudeConfigDir: process.env.OPENCLAUDE_CONFIG_DIR,
legacyConfigDir: process.env.CLAUDE_CONFIG_DIR,
warn: message => {
// eslint-disable-next-line no-console
console.warn(`[openclaude] ${message}`)
},
}),
configDirEnv: process.env.CLAUDE_CONFIG_DIR,
}),
() => `${process.env.OPENCLAUDE_CONFIG_DIR ?? ''}|${process.env.CLAUDE_CONFIG_DIR ?? ''}`,
() => process.env.CLAUDE_CONFIG_DIR,
)
export function getTeamsDir(): string {

View File

@@ -51,8 +51,7 @@ describe('OpenClaude paths', () => {
).toBe(join(homedir(), '.claude'))
})
test('uses CLAUDE_CONFIG_DIR override when provided (legacy)', async () => {
delete process.env.OPENCLAUDE_CONFIG_DIR
test('uses CLAUDE_CONFIG_DIR override when provided', async () => {
process.env.CLAUDE_CONFIG_DIR = '/tmp/custom-openclaude'
const { getClaudeConfigHomeDir, resolveClaudeConfigHomeDir } =
await importFreshEnvUtils()
@@ -65,83 +64,6 @@ describe('OpenClaude paths', () => {
).toBe('/tmp/custom-openclaude')
})
test('OPENCLAUDE_CONFIG_DIR overrides the default (issue #454)', async () => {
delete process.env.CLAUDE_CONFIG_DIR
process.env.OPENCLAUDE_CONFIG_DIR = '/tmp/oc-config-only'
const { getClaudeConfigHomeDir } = await importFreshEnvUtils()
expect(getClaudeConfigHomeDir()).toBe('/tmp/oc-config-only')
})
test('OPENCLAUDE_CONFIG_DIR wins when both env vars are set with different values', async () => {
process.env.OPENCLAUDE_CONFIG_DIR = '/tmp/oc-wins'
process.env.CLAUDE_CONFIG_DIR = '/tmp/legacy-loses'
const { getClaudeConfigHomeDir } = await importFreshEnvUtils()
expect(getClaudeConfigHomeDir()).toBe('/tmp/oc-wins')
})
test('CLAUDE_CONFIG_DIR is still honored when OPENCLAUDE_CONFIG_DIR is unset', async () => {
delete process.env.OPENCLAUDE_CONFIG_DIR
process.env.CLAUDE_CONFIG_DIR = '/tmp/legacy-only'
const { getClaudeConfigHomeDir } = await importFreshEnvUtils()
expect(getClaudeConfigHomeDir()).toBe('/tmp/legacy-only')
})
test('empty OPENCLAUDE_CONFIG_DIR falls through to CLAUDE_CONFIG_DIR', async () => {
process.env.OPENCLAUDE_CONFIG_DIR = ''
process.env.CLAUDE_CONFIG_DIR = '/tmp/legacy-fallback'
const { getClaudeConfigHomeDir } = await importFreshEnvUtils()
expect(getClaudeConfigHomeDir()).toBe('/tmp/legacy-fallback')
})
test('resolveConfigDirEnv prefers OPENCLAUDE over CLAUDE and warns on conflict', async () => {
const { resolveConfigDirEnv, __resetConfigDirEnvWarningForTesting } =
await importFreshEnvUtils()
__resetConfigDirEnvWarningForTesting()
const warnings: string[] = []
const result = resolveConfigDirEnv({
openClaudeConfigDir: '/a',
legacyConfigDir: '/b',
warn: m => warnings.push(m),
})
expect(result).toBe('/a')
expect(warnings.length).toBe(1)
expect(warnings[0]).toContain('OPENCLAUDE_CONFIG_DIR=/a')
expect(warnings[0]).toContain('CLAUDE_CONFIG_DIR=/b')
})
test('resolveConfigDirEnv does not warn when both env vars agree', async () => {
const { resolveConfigDirEnv, __resetConfigDirEnvWarningForTesting } =
await importFreshEnvUtils()
__resetConfigDirEnvWarningForTesting()
const warnings: string[] = []
const result = resolveConfigDirEnv({
openClaudeConfigDir: '/same',
legacyConfigDir: '/same',
warn: m => warnings.push(m),
})
expect(result).toBe('/same')
expect(warnings).toEqual([])
})
test('resolveConfigDirEnv returns undefined when neither env var is set', async () => {
const { resolveConfigDirEnv } = await importFreshEnvUtils()
expect(
resolveConfigDirEnv({
openClaudeConfigDir: undefined,
legacyConfigDir: undefined,
}),
).toBeUndefined()
})
test('project and local settings paths use .openclaude', async () => {
const { getRelativeSettingsFilePathForSource } = await importFreshSettings()

View File

@@ -34,8 +34,7 @@ export function getSecureStorageServiceName(
serviceSuffix: string = '',
): string {
const configDir = getClaudeConfigHomeDir()
const isDefaultDir =
!process.env.OPENCLAUDE_CONFIG_DIR && !process.env.CLAUDE_CONFIG_DIR
const isDefaultDir = !process.env.CLAUDE_CONFIG_DIR
// Use a hash of the config dir path to create a unique but stable suffix
// Only add suffix for non-default directories to maintain backwards compatibility

View File

@@ -117,8 +117,7 @@ const TEAMMATE_ENV_VARS = [
'MISTRAL_BASE_URL',
// Custom API endpoint
'ANTHROPIC_BASE_URL',
// Config directory override (preferred name + legacy alias)
'OPENCLAUDE_CONFIG_DIR',
// Config directory override
'CLAUDE_CONFIG_DIR',
// CCR marker — teammates need this for CCR-aware code paths. Auth finds
// its own way via /home/claude/.claude/remote/.oauth_token regardless;