fix: filter disabled MCP tools in registerMcpServer + fix agent timeout description - #2423
Conversation
…ut description - Fix MoonshotAI#2421: registerMcpServer now checks mcpDenyPatterns before registering MCP tools, preventing disabled tools from being added to the tools map unconditionally - Fix MoonshotAI#2420: Update agent.md from '30-minute timeout' to '2-hour timeout' to match DEFAULT_SUBAGENT_TIMEOUT_MS constant - Update corresponding test assertion
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6066b21a6a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (this.mcpDenyPatterns.some((pattern) => picomatch.isMatch(qualified, pattern))) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Preserve denied MCP tools for later profile changes
When a server connects while the current profile has an MCP deny pattern, this continue drops the tool from mcpTools entirely. Later useProfile()/setActiveTools() only updates the allow/deny pattern fields and re-filters the existing registry; it does not rediscover or re-register the server, so switching to a profile that allows mcp__server__tool cannot make that tool available again until the MCP server reconnects or the session restarts. Keeping registration complete and enforcing deny at exposure/dispatch time would preserve the existing runtime profile-toggle behavior.
Useful? React with 👍 / 👎.
…collaboration/agent.md
Changes
Fix #2421: MCP tools bypass disabled filter during registration
Root cause:
registerMcpServeradds MCP tools to themcpToolsmap unconditionally. WhileisMcpToolEnabled()checksmcpDenyPatternswhen building the loadable tools list, disabled tools are still registered and accessible through the map.Fix: Added a
mcpDenyPatternscheck in theregisterMcpServerloop, skipping tools that match any deny pattern before they are registered.Fix #2420: Agent tool description says "30-minute" but timeout is 2 hours
Root cause:
agent.md:12hardcodes "fixed 30-minute timeout" whileDEFAULT_SUBAGENT_TIMEOUT_MSis2 * 60 * 60 * 1000(2 hours).Fix:
agent.md:12to "fixed 2-hour timeout"agent.test.ts:131to matchFiles changed
packages/agent-core/src/agent/tool/index.ts(+4 lines)packages/agent-core/src/tools/builtin/collaboration/agent.md(1 line)packages/agent-core/test/tools/agent.test.ts(1 line)