fix: make disabled_mcps work for custom and plugin MCPs #514

sisyphus-dev-ai · 2026-01-05T12:29:00Z

Summary

Fixes the P1 issue identified by cubic-dev-ai in PR #513 where disabled_mcps configuration only filtered built-in MCPs but ignored custom MCPs from .mcp.json and plugin MCPs.

Problem

As cubic-dev-ai correctly identified:

disabled_mcps now promises support for arbitrary MCP names, but the runtime still filters only built-in MCPs; custom/plugin MCPs loaded from .mcp.json or plugins ignore this flag entirely. OhMyOpenCodeConfigSchema accepts strings like "my-custom", yet config-handler.ts uses the list solely when calling createBuiltinMcps, while downstream loaders (loadMcpConfigs, plugin MCPs) never consult it, so the new configuration silently does nothing for custom servers.

Solution

Custom .mcp.json MCPs

Updated loadMcpConfigs() to accept disabledMcps: string[] parameter
Added filtering logic that checks disabled_mcps BEFORE the existing disabled: true check
Maintains backward compatibility with disabled: true in .mcp.json files

Plugin MCPs

Updated loadPluginMcpServers() to accept disabledMcps: string[] parameter
Filters both namespaced names ("plugin:server") and non-namespaced names ("server")
Added logging for filtered plugin MCPs

Integration

Updated config-handler.ts to pass pluginConfig.disabled_mcps to both loaders
Updated loadAllPluginComponents() signature to propagate disabledMcps parameter

Testing

Added 4 comprehensive test cases for custom MCP filtering:

✅ Filter out custom MCPs in disabled_mcps list
✅ Respect both disabled_mcps and disabled:true
✅ Filter across all scopes (user, project, local)
✅ No filtering when disabled_mcps is empty

Test Results:

All 625 tests pass ✅
Typecheck clean ✅
Build successful ✅

Examples

Disable custom .mcp.json MCPs

{
  "disabled_mcps": ["playwright", "custom-server"]
}

Disable plugin MCPs (supports both forms)

{
  "disabled_mcps": [
    "my-plugin:sqlite",  // Namespaced
    "playwright"         // Non-namespaced (matches all plugins)
  ]
}

Files Changed

File	Changes	Purpose
`src/features/claude-code-mcp-loader/loader.ts`	+9 -4	Add disabled_mcps parameter and filtering logic
`src/features/claude-code-mcp-loader/loader.test.ts`	+155	Add 4 new test cases for custom MCP filtering
`src/features/claude-code-plugin-loader/loader.ts`	+18 -5	Add disabled_mcps parameter and namespaced filtering
`src/plugin-handlers/config-handler.ts`	+11 -4	Pass disabled_mcps to both loaders

Migration Notes

No breaking changes. Existing configurations will continue to work:

disabled: true in .mcp.json files still works
disabled_mcps array is optional (defaults to [])
All existing MCP filtering behavior is preserved

Future Work

Skill-embedded MCP support (disabling MCPs defined in skill YAML frontmatter) can be added in a follow-up PR if needed. The current implementation addresses the core issue identified in cubic-dev-ai's feedback.

Addresses

PR feat: allow disabled_mcps to accept any MCP name #513 cubic-dev-ai feedback
Resolves user-facing config mismatch where disabled_mcps appeared to support any MCP name but only actually worked for built-in MCPs

Review Focus:

Verify backward compatibility with existing disabled: true configs
Check namespaced vs non-namespaced filtering logic for plugins
Confirm test coverage is comprehensive

Summary by cubic

Fixes disabled_mcps so it correctly disables custom MCPs from .mcp.json and plugin MCPs, not just built-ins. This makes configuration consistent across all MCP loading paths.

Bug Fixes
- loadMcpConfigs now accepts disabledMcps and filters names before checking disabled:true.
- loadPluginMcpServers supports filtering by both "plugin:server" and "server".
- config-handler passes disabled_mcps to both loaders; loadAllPluginComponents propagates it.
- Added 4 tests for custom MCP filtering; all tests pass. Backward compatibility maintained (disabled:true still works).

^{Written for commit fab0f46. Summary will update on new commits.}

Addresses cubic-dev-ai feedback on PR #513 where disabled_mcps configuration was only filtering built-in MCPs but not custom MCPs from .mcp.json or plugin MCPs. Changes: - Custom .mcp.json MCPs: loadMcpConfigs now accepts disabledMcps parameter and filters accordingly - Plugin MCPs: loadPluginMcpServers now filters both namespaced (plugin:server) and non-namespaced (server) names - Added comprehensive tests for custom MCP filtering (4 new test cases) - All 625 tests pass, typecheck clean, build successful Tested: - disabled_mcps: ["playwright", "custom-server"] filters correctly across all scopes - Backward compatibility: disabled: true in .mcp.json still works - Both custom and plugin MCPs respect the disabled_mcps configuration Note: Skill-embedded MCP support can be added in a follow-up PR if needed. This PR addresses the core issue identified in the feedback.

greptile-apps · 2026-01-05T12:31:47Z

Greptile Summary

Fixed disabled_mcps configuration to work with custom .mcp.json MCPs and plugin MCPs by passing the disabled list through the loading pipeline.

Key Changes:

loadMcpConfigs() now accepts disabledMcps parameter and filters before checking disabled:true
loadPluginMcpServers() filters both namespaced (plugin:server) and non-namespaced names
config-handler.ts propagates disabled_mcps to both loaders
Added 4 comprehensive test cases covering filtering, scope precedence, and backward compatibility

Issue Found:
getSystemMcpServerNames() in src/features/claude-code-mcp-loader/loader.ts:45-67 doesn't respect disabled_mcps, only checking disabled:true. This function is used in src/index.ts:196 for skill filtering, creating inconsistent behavior where disabled MCPs still filter out builtin skills.

Backward Compatibility:

Existing disabled:true in .mcp.json files continues to work
disabled_mcps array is optional (defaults to empty array)
Test coverage follows TDD conventions with BDD comments

Confidence Score: 4/5

Safe to merge with one logical inconsistency that should be addressed
Implementation is solid with good test coverage and proper parameter propagation. However, getSystemMcpServerNames() doesn't respect disabled_mcps, causing inconsistent skill filtering behavior in src/index.ts. All other changes follow the codebase patterns correctly.
Pay attention to src/features/claude-code-mcp-loader/loader.ts - the getSystemMcpServerNames() function needs to be updated to respect disabled_mcps

Important Files Changed

Filename	Overview
src/features/claude-code-mcp-loader/loader.ts	Added `disabledMcps` parameter to `loadMcpConfigs()` with filtering before `disabled:true` check. Issue: `getSystemMcpServerNames()` doesn't respect `disabled_mcps`, which may cause inconsistent behavior when used for skill filtering in src/index.ts:196.
src/features/claude-code-plugin-loader/loader.ts	Added `disabledMcps` parameter to `loadPluginMcpServers()` and `loadAllPluginComponents()`, implementing both namespaced and non-namespaced filtering for plugin MCPs. Clean implementation with proper propagation.
src/plugin-handlers/config-handler.ts	Correctly passes `pluginConfig.disabled_mcps` to both `loadAllPluginComponents()` and `loadMcpConfigs()` to propagate the disabled MCPs list through the loading pipeline.
src/features/claude-code-mcp-loader/loader.test.ts	Added comprehensive test coverage for `disabled_mcps` filtering: tests basic filtering, interaction with `disabled:true`, scope-wide filtering, and empty list behavior. Follows BDD conventions with #given/#when/#then comments.

Sequence Diagram

sequenceDiagram
    participant User
    participant ConfigHandler as config-handler.ts
    participant PluginLoader as plugin-loader.ts
    participant McpLoader as mcp-loader.ts
    participant Index as index.ts

    User->>ConfigHandler: Load config with disabled_mcps
    ConfigHandler->>PluginLoader: loadAllPluginComponents(options, disabled_mcps)
    PluginLoader->>PluginLoader: loadPluginMcpServers(plugins, disabled_mcps)
    loop For each plugin MCP
        PluginLoader->>PluginLoader: Check namespacedName or name in disabled_mcps
        alt MCP is disabled
            PluginLoader->>PluginLoader: Skip (log and continue)
        else MCP enabled
            PluginLoader->>PluginLoader: Transform and add to servers
        end
    end
    PluginLoader-->>ConfigHandler: Return mcpServers

    ConfigHandler->>McpLoader: loadMcpConfigs(disabled_mcps)
    McpLoader->>McpLoader: Loop through .mcp.json files
    loop For each custom MCP
        McpLoader->>McpLoader: Check name in disabled_mcps
        alt MCP is disabled
            McpLoader->>McpLoader: Skip (log and continue)
        else Check disabled:true
            alt disabled:true
                McpLoader->>McpLoader: Skip
            else MCP enabled
                McpLoader->>McpLoader: Transform and add to servers
            end
        end
    end
    McpLoader-->>ConfigHandler: Return servers

    ConfigHandler->>ConfigHandler: Merge builtin + custom + plugin MCPs
    ConfigHandler-->>User: Return complete config

    Note over Index: Separate call (inconsistent)
    Index->>McpLoader: getSystemMcpServerNames()
    McpLoader->>McpLoader: Check only disabled:true (NOT disabled_mcps)
    McpLoader-->>Index: Return MCP names
    Index->>Index: Filter builtin skills based on MCP names

greptile-apps

Additional Comments (1)

src/features/claude-code-mcp-loader/loader.ts, line 45-67 (link)

logic: getSystemMcpServerNames() doesn't respect disabled_mcps, causing inconsistent behavior.

This function is called in src/index.ts:196 to filter out builtin skills that conflict with system MCPs. However, it only checks disabled:true in .mcp.json files, not the disabled_mcps configuration.

This means if a user disables an MCP via disabled_mcps, builtin skills will still be filtered out because getSystemMcpServerNames() will still return that MCP name.

Is the skill filtering behavior intentional when MCPs are disabled via disabled_mcps?

_{4 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

cubic-dev-ai

1 issue found across 4 files

Confidence score: 4/5

One test case in src/features/claude-code-mcp-loader/loader.test.ts advertises user/project/local coverage but never exercises the user config path, so the test suite could miss regressions in that scope.
Overall impact seems minor and limited to coverage clarity, so merge looks safe with awareness of the naming gap.
Pay close attention to src/features/claude-code-mcp-loader/loader.test.ts - rename or add a user-scope test so expectations match behavior.

Prompt for AI agents (all issues)


Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="src/features/claude-code-mcp-loader/loader.test.ts">

<violation number="1" location="src/features/claude-code-mcp-loader/loader.test.ts:247">
P2: Test title claims to verify &quot;user, project, local&quot; scopes but only tests project and local scopes. The user scope (config at `~/.claude/.mcp.json`) is not tested. Consider renaming to `&quot;should filter across project and local scopes&quot;` or adding user scope testing with a mock for `getClaudeConfigDir`.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

cubic-dev-ai · 2026-01-05T12:34:16Z

src/features/claude-code-mcp-loader/loader.test.ts

+    }
+  })
+
+  it("should filter across all scopes (user, project, local)", async () => {


P2: Test title claims to verify "user, project, local" scopes but only tests project and local scopes. The user scope (config at ~/.claude/.mcp.json) is not tested. Consider renaming to "should filter across project and local scopes" or adding user scope testing with a mock for getClaudeConfigDir.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/claude-code-mcp-loader/loader.test.ts, line 247: <comment>Test title claims to verify "user, project, local" scopes but only tests project and local scopes. The user scope (config at `~/.claude/.mcp.json`) is not tested. Consider renaming to `"should filter across project and local scopes"` or adding user scope testing with a mock for `getClaudeConfigDir`.</comment> <file context> @@ -160,3 +160,158 @@ describe("getSystemMcpServerNames", () => { + } + }) + + it("should filter across all scopes (user, project, local)", async () => { + // #given + const claudeDir = join(TEST_DIR, ".claude") </file context>

sisyphus-dev-ai mentioned this pull request Jan 5, 2026

feat: allow disabled_mcps to accept any MCP name #513

Merged

greptile-apps bot reviewed Jan 5, 2026

View reviewed changes

cubic-dev-ai bot reviewed Jan 5, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: make disabled_mcps work for custom and plugin MCPs #514

fix: make disabled_mcps work for custom and plugin MCPs #514

sisyphus-dev-ai commented Jan 5, 2026 •

edited by cubic-dev-ai bot

Loading

Uh oh!

greptile-apps bot commented Jan 5, 2026

Uh oh!

greptile-apps bot left a comment •

edited

Loading

Uh oh!

cubic-dev-ai bot left a comment

Uh oh!

cubic-dev-ai bot Jan 5, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

fix: make disabled_mcps work for custom and plugin MCPs #514

Are you sure you want to change the base?

fix: make disabled_mcps work for custom and plugin MCPs #514

Conversation

sisyphus-dev-ai commented Jan 5, 2026 • edited by cubic-dev-ai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Problem

Solution

Custom .mcp.json MCPs

Plugin MCPs

Integration

Testing

Examples

Disable custom .mcp.json MCPs

Disable plugin MCPs (supports both forms)

Files Changed

Migration Notes

Future Work

Addresses

Summary by cubic

Uh oh!

greptile-apps bot commented Jan 5, 2026

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Sequence Diagram

Uh oh!

greptile-apps bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Additional Comments (1)

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot Jan 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sisyphus-dev-ai commented Jan 5, 2026 •

edited by cubic-dev-ai bot

Loading

greptile-apps bot left a comment •

edited

Loading

cubic-dev-ai bot Jan 5, 2026 •

edited

Loading