-
Notifications
You must be signed in to change notification settings - Fork 14
feat(mcp): support for client info in analytics #877
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @ShawkyZ, 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!
Summary of Changes
Hello team, gemini-code-assist here to provide a summary of this pull request. This PR, authored by ShawkyZ, introduces support for including client information in analytics data sent via the MCP binding. The core change involves updating the mcp-go dependency to a newer version (v0.27.1) that provides access to client session details. This client information (specifically, the client name and version) is then extracted and passed as environment variables (SNYK_INTEGRATION_ENVIRONMENT and SNYK_INTEGRATION_ENVIRONMENT_VERSION) to the Snyk CLI process executed by the MCP binding. This enhancement allows for better analytics and tracking of which clients are using the MCP binding.
Highlights
- Dependency Update: The
github.com/mark3labs/mcp-godependency has been updated from versionv0.18.0tov0.27.1. This update is crucial as it provides the necessary functionality to access client information. - Client Info Extraction: A new utility function
ClientInfoFromContexthas been added to retrieve client name and version from the MCP context. - Analytics Enhancement: The extracted client name and version are now included as environment variables (
SNYK_INTEGRATION_ENVIRONMENTandSNYK_INTEGRATION_ENVIRONMENT_VERSION) when executing Snyk commands via the MCP binding, improving analytics granularity. - Environment Variable Handling: The
expandedEnvfunction inllm_binding.gohas been modified to accept and include the client environment name and version in the list of environment variables passed to subprocesses.
Changelog
Click here to see the changelog
- domain/ide/command/execute_mcp.go
- Updated the type assertion in the defer function for closing the MCP client from
*client.SSEMCPClientto*client.Client(L65).
- Updated the type assertion in the defer function for closing the MCP client from
- go.mod
- Updated
github.com/mark3labs/mcp-godependency from v0.18.0 to v0.27.1 (L19). - Updated
github.com/spf13/castdependency from v1.5.0 to v1.7.1 (L112). - Added a local
replacedirective forgithub.com/mark3labs/mcp-go(L140).
- Updated
- go.sum
- Updated checksums for
github.com/frankban/quicktest(L65-66). - Updated checksums for
github.com/mark3labs/mcp-go(L219-220). - Updated checksums for
github.com/spf13/cast(L337-338).
- Updated checksums for
- internal/mcp/llm_binding.go
- Modified
expandedEnvfunction signature to acceptintegrationVersion,environmentName, andenvironmentVersion(L222). - Added
SNYK_INTEGRATION_ENVIRONMENTandSNYK_INTEGRATION_ENVIRONMENT_VERSIONenvironment variables using the new parameters (L236-237).
- Modified
- internal/mcp/llm_binding_test.go
- Updated the call to
expandedEnvin the test to pass dummy client info (L55). - Added assertions to check for the presence of
SNYK_INTEGRATION_ENVIRONMENTandSNYK_INTEGRATION_ENVIRONMENT_VERSIONin the returned environment slice (L69-70).
- Updated the call to
- internal/mcp/scan_tool.go
- Retrieved client information using the new
ClientInfoFromContextfunction (L93). - Passed client name and version to the
expandedEnvfunction when setting command environment variables (L102, L104).
- Retrieved client information using the new
- internal/mcp/utils.go
- Imported
contextandgithub.com/mark3labs/mcp-go/server(L20, L24). - Modified
extractParamsFromRequestArgsfunction signature to acceptargumentsIn anyand perform a type assertion (L72-75). - Added
ClientInfoFromContextfunction to retrieve client implementation details from the context (L112-120).
- Imported
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and 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 to provide feedback.
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.
Deps updated, a new release,
Client info brings analytics peace.
Env vars set,
No more regret,
For data flows with graceful ease.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces support for client information in analytics by updating the mcp-go dependency and modifying how environment variables are set for MCP calls. The changes are mostly straightforward and well-structured.
Summary of Findings
- Potential nil pointer dereference: In
domain/ide/command/execute_mcp.go, themcpClientcould be nil ifNewSSEMCPClientfails, leading to a panic whenmcpClient.Close()is called. Adding a nil check before callingClose()would prevent this. - Potential panic due to type assertion: In
internal/mcp/utils.go, the type assertionargumentsIn.(map[string]interface{})doesn't check if the assertion was successful. If the assertion fails, the code could panic when trying to access the map. go.modreplacedirective: Thego.modfile includes areplacedirective forgithub.com/mark3labs/mcp-gopointing to a local path (../mcp-go). It's important to clarify if this is intended for the final merged version or if it's a temporary directive for local development that should be removed.
Merge Readiness
The pull request is almost ready for merging. However, the high-severity issue in internal/mcp/utils.go regarding the type assertion needs to be addressed to prevent potential panics. The medium-severity issue in domain/ide/command/execute_mcp.go regarding the nil pointer dereference should also be addressed. Additionally, the go.mod replace directive should be clarified. I am unable to approve this pull request, so please ensure further review and approval by authorized team members after addressing the feedback.
|
Warning There is an error in the Gemini Code Assist config file for this repository at |
|
Warning There is an error in the Gemini Code Assist config file for this repository at |
1 similar comment
|
Warning There is an error in the Gemini Code Assist config file for this repository at |
reviewed and dismissed
Description
mark3labs/mcp-go#313
Checklist