-
Notifications
You must be signed in to change notification settings - Fork 968
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
Include meaningful User-Agent
header in requests to the GitHub UI
#592
Include meaningful User-Agent
header in requests to the GitHub UI
#592
Conversation
This adds a custom `User-Agent` header to requests from the GitHub server to the GitHub API, identifying the application, the version and key information about the environment. This aligns with the [recommendations][1] in the GitHub Docs. As part of this change, I have also moved the current version of the server into a constant, and fix the initialization of `Server` to use that version, taking from `package.json`. [1]: https://docs.github.com/en/rest/using-the-rest-api/getting-started-with-the-rest-api?apiVersion=2022-11-28#user-agent
|
||
const server = new Server( | ||
{ | ||
name: "github-mcp-server", | ||
version: "0.1.0", |
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.
I'm replacing this value with the contents of package.json
, but I have also noticed that npm has a totally different package version. What should we use here?
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.
We should use the package.json version. The version in the repo is a placeholder. Currently the CI pipeline just overwrites the package.json version automatically and pushes that version. I have to think through how we best handle this in the future so that it doesnt become to complex in the codebase to reason about this.
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.
Is there anywhere we can update to make sure that this gets bumped when the package version does? Perhaps I could write a CI check?
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.
I've proposed a CI check here: #593
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.
Makes sense! Thank you!
This fixes the build, after the merge of modelcontextprotocol#592, by committing an updated `/package-lock.json` file with the new `universal-user- agent` dependency.
This adds a custom
User-Agent
header to requests from the GitHub server to the GitHub API, identifying the application, the version and key information about the environment.This aligns with the recommendations in the GitHub Docs.
As part of this change, I have also moved the current version of the server into a constant, and fix the initialization of
Server
to use that version, taking frompackage.json
.Description
Include a meaningful
User-Agent
header in requests to GitHub from the GitHub serverServer Details
Server: GitHub
Changes to: Tools
Motivation and Context
If we, as GitHub, are making a breaking API change or an integration is running into performance problems, it is helpful to be able to identify that API integration.
This is impossible to do with open source integrations like this where users authenticate with personal access tokens (PATs), unless there is a user agent that identifies the integration.
How Has This Been Tested?
I connected this to Claude and ask it to fetch a pull request. A request to GitHub was made with the expected header.
Breaking Changes
No
Types of changes
Checklist
Additional context