feat: add MCP tools for ChatGPT #19102
Conversation
hugodutka
force-pushed
the
hugodutka/chatgpt-mcp
branch
from
e17c5f0 to
ccc3a6a
Compare
hugodutka
force-pushed
the
hugodutka/chatgpt-mcp
branch
from
ccc3a6a to
3e42d7a
Compare
There was a problem hiding this comment.
Looking good, just a few small code organization tweaks.
I'd prefer to add a new Register function to the server struct so that we can handle this one edge case instead of refactoring the whole thing.
For anyone looking at this PR: ChatGPT connectors are 'questionable' (ask for my opinion in Slack 😉) and adding a dedicated endpoint makes sense for the sake of dogfood and sanity.
| // RegisterTools registers all available MCP tools with the server | ||
| func (s *Server) RegisterTools(client *codersdk.Client) error { | ||
| // RegisterTools registers MCP tools with the server | ||
| func (s *Server) RegisterTools(client *codersdk.Client, tools []toolsdk.GenericTool) error { |
There was a problem hiding this comment.
Could we leave the function as it was and add a new function called RegisterChatGPTTools instead?
This would benefit us by allowing us to add exceptions for ChatGPT more easily on the server structure, and keeping the existing MCP endpoint and function calls as they are/will be.
(Also, we can then write a unit test and assert that a ChatGPT MCP server only has those two tools that we expect)
| "Authorization": "Bearer " + coderClient.SessionToken(), | ||
| })) | ||
| require.NoError(t, err) | ||
| defer func() { |
There was a problem hiding this comment.
nit: we should probably run this in a t.Cleanup?
| } | ||
| }() | ||
|
|
||
| ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) |
There was a problem hiding this comment.
| ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) | |
| ctx, cancel := context.WithTimeout(t.Context(), testutil.WaitLong) |
| ) | ||
|
|
||
| // mcpHTTPHandler creates the MCP HTTP transport handler | ||
| func (api *API) mcpHTTPHandler() http.Handler { | ||
| func (api *API) mcpHTTPHandler(tools []toolsdk.GenericTool) http.Handler { |
There was a problem hiding this comment.
I think we could simplify this a bit by making it a boolean called isChatGPT bool, then calling the appropriate Register function (either .RegisterTools() or .RegisterChatGPTTools()).
This also makes the handler less complex.
| func getServerURL(deps Deps) string { | ||
| serverURLCopy := *deps.coderClient.URL | ||
| serverURLCopy.Path = "" | ||
| serverURLCopy.RawQuery = "" | ||
| return serverURLCopy.String() | ||
| } |
There was a problem hiding this comment.
We may want to add a new function to Deps.
Then we could just deps.ServerURL() to get it.
|
|
||
| func searchTemplates(ctx context.Context, deps Deps) ([]SearchResultItem, error) { | ||
| serverURL := getServerURL(deps) | ||
| templates, err := deps.coderClient.Templates(ctx, codersdk.TemplateFilter{}) |
There was a problem hiding this comment.
Can we use the SearchQuery field to parameterize this?
| if owner == "" { | ||
| owner = "me" | ||
| } | ||
| workspaces, err := deps.coderClient.Workspaces(ctx, codersdk.WorkspaceFilter{ |
There was a problem hiding this comment.
Same here, I'd just delegate the search query as is to the backend.
Addresses coder/internal#772.
Adds a new
/api/experimental/mcp/chatgptendpoint which exposes newfetchandsearchtools compatible with ChatGPT, as described in the ChatGPT docs. These tools are exposed in isolation because in my usage I found that ChatGPT refuses to connect to Coder if it sees additional MCP tools.