fix(core): explicitly pass messageBus to policy engine for MCP tool saves#22255
fix(core): explicitly pass messageBus to policy engine for MCP tool saves#22255abhipatel12 merged 1 commit intomainfrom
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves a critical bug that caused the agent to crash when users attempted to permanently save MCP tool authorizations. The issue stemmed from Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
gemini-cli
bot
added
area/agent
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively addresses the crash that occurred when saving MCP tool authorizations. The core fix, which involves explicitly passing the messageBus to the updatePolicy function, is a solid improvement that makes the dependency clear and resolves the undefined property access. The addition of a defensive check for the config object is also a good practice that enhances robustness.
Furthermore, the changes to standardize the MCP tool name formatting by using the formatMcpToolName utility and updating the parsing logic in config.ts are excellent for improving code consistency and maintainability. The accompanying documentation update in policy-engine.md is clear and will help users correctly configure their policies.
The test files have been updated appropriately to reflect these changes. Overall, this is a well-executed fix.
Note: Security Review did not run due to the size of the PR.
|
Size Change: +227 B (0%) Total Size: 26.1 MB
ℹ️ View Unchanged
|
|
/patch preview |
|
🚀 [Step 1/4] Patch workflow(s) waiting for approval! 📋 Details:
⏳ Status: The patch creation workflow has been triggered and is waiting for deployment approval. Please visit the specific workflow links below and approve the runs. 🔗 Track Progress: |
…aves (#22255) # Conflicts: # packages/core/src/scheduler/policy.test.ts # packages/core/src/scheduler/policy.ts # packages/core/src/scheduler/scheduler.test.ts # packages/core/src/scheduler/scheduler.ts
|
🚀 [Step 2/4] Patch PR Created! 📋 Patch Details:
📝 Next Steps:
🔗 Track Progress: |
3 participants
Summary
This PR fixes a bug where the agent crashed with a
TypeError: Cannot read properties of undefined (reading 'publish')orreading 'isTrustedFolder'when users attempted to permanently save an MCP tool authorization (i.e. "Allow tool for all future sessions").Details
The regression occurred when
updatePolicy()was refactored to accept anAgentLoopContextinstead of individual dependencies.AgentLoopContextnatively does not includemessageBus. However, the internal logic for saving MCP policies (handleMcpPolicyUpdate) was attempting to blindly accessmessageBusoff ofdeps, resulting in it beingundefinedand causing a crash when.publish()was invoked.To resolve this, this PR:
messageBus: MessageBusas an explicit parameter toupdatePolicy(), rather than trying to read it fromAgentLoopContext.configexists before attempting to evaluateisTrustedFolder().scheduler.tsandpolicy.test.tsto respect the newly explicit method signature.Related Issues
Related to #21982 (Same architectural root cause)
How to Validate
npm test -w @google/gemini-cli-corePre-Merge Checklist