Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR extends the user agent string by integrating host-supplied client details into the server initialization process.
- Introduces a new hook function, beforeInit, that modifies the user agent with client information.
- Updates the server initialization to include hooks.
- Adds a new package import required for the extended functionality.
1216fe5 to
f17eac6
Compare
| t, dumpTranslations := translations.TranslationHelper() | ||
|
|
||
| beforeInit := func(_ context.Context, _ any, message *mcp.InitializeRequest) { | ||
| ghClient.UserAgent = fmt.Sprintf("github-mcp-server/%s (%s/%s)", version, message.Params.ClientInfo.Name, message.Params.ClientInfo.Version) |
There was a problem hiding this comment.
Haven't looked into how hooks work in depth, so bear with me, one question that I have is whether we need to protect this with a mutex.
There was a problem hiding this comment.
Disregard my comment. Even we wanted to protect it with a mutex, we couldn't because this is in the github.Client.
There was a problem hiding this comment.
So, if I understand this correctly. This is going to be OK from a race condition standpoint if host/client always initializes the server before doing anything else.
The only way to cause a race here is by calling a tool and initializing at the same time.
There was a problem hiding this comment.
If you wanted to completely remove the possibility of a race, you could make things slightly more complex and set the client info into a different variable protected by a mutex. And then, you could set the client agent accordingly.
There was a problem hiding this comment.
Yeah, that's a good shout. I was thinking about this too, but it is only a one-shot event, and if it happens a second time it can also update the info, that's fine.
f17eac6 to
03465f3
Compare
This gets you things like:
github-mcp-server/<version> (Visual Studio Code - Insiders/<version>)if provided by the host.