DRAFT - initial stab at MCP support #460

dexhorthy · 2025-03-08T09:05:13Z

quick demo - https://www.loom.com/share/85fa41a6876d456bbbf07ff558f47102

fixes #397

Done:

mcp-servers in config
default-mcp-servers in config
mcp-servers-from in config
--mcp-servers, -T flag
--all-mcp-servers, -A flag
--mcp-servers-from flag
--list-mcp-servers command flag
--list-mcp-tools command flag
passing mcp tools models (openai only)
printing model tool calls to terminal
collecting tool call stream chunks into final pendingToolCalls struct field

To Do:

execute tool calls after response stream closed
send tool call results to model
loop until no more tool calls
support for multiple tool calls
support for non-openai models
this is kinda messy overall and might need some cleanup in places

Done: - `mcp-servers` in config - `default-mcp-servers` in config - `mcp-servers-from` in config - `--mcp-servers`, `-T` flag - `--all-mcp-servers`, `-A` flag - `--mcp-servers-from` flag - `--list-mcp-servers` command flag - `--list-mcp-tools` command flag - passing mcp tools models (openai only) - printing model tool calls to terminal - collecting tool call stream chunks into final pendingToolCalls struct field To Do: - execute tool calls after response stream closed - send tool call results to model - loop until no more tool calls - support for multiple tool calls - support for non-openai models - this is kinda messy overall and might need some cleanup in places DRAFT - initial stab at MCP support Done: - `mcp-servers` in config - `default-mcp-servers` in config - `mcp-servers-from` in config - `--mcp-servers`, `-T` flag - `--all-mcp-servers`, `-A` flag - `--mcp-servers-from` flag - `--list-mcp-servers` command flag - `--list-mcp-tools` command flag - passing mcp tools models (openai only) - printing model tool calls to terminal - collecting tool call stream chunks into final pendingToolCalls struct field To Do: - execute tool calls after response stream closed - send tool call results to model - loop until no more tool calls - support for multiple tool calls - support for non-openai models - some stuff that could be unit tested - this is kinda messy overall and might need some cleanup in places

raphamorim · 2025-03-10T16:56:25Z

features.md

@@ -118,3 +118,76 @@ mods --delete='naturals' --delete='a2e2'

 Keep in mind that these operations are not reversible.
 You can repeat the delete flag to delete multiple conversations at once.
+


could you remove this extra line, please?

sure - in the future, is there a formatter/checker that can detect these sorts of style things for me?

There's https://github.com/DavidAnson/markdownlint, and if you use VS Code you can use their extension and it renders a yellow wiggle underline that you can either hover over or list in the "problems" view, or cycle through problems with F8, like this:

raphamorim · 2025-03-10T16:56:43Z

features.md

+```bash
+mods --list-mcp-tools
+```
+


could you remove these extra lines to the end?

caarlos0

hey, thanks for this!

in general, it looks good to me!

I'm not sure about some of the options though:

I think we can either have mcp-servers or mcp-servers-from. If the user uses Claude desktop, they will already have the config file, so maybe that's the way to go?
I don't think we need list-mcp-* thing 🤔
Also, not sure if the user would want to optionally enable/disable MCPs... judging by how claude desktop works, it's kind of always there. I think if you have them setup, they should always be there 🤔

I think I would greatly simplify it to just add either mcp-servers or mcp-servers-from and that's pretty much it 🤔

philippgille · 2025-04-10T18:01:37Z

Also, not sure if the user would want to optionally enable/disable MCPs... judging by how claude desktop works, it's kind of always there. I think if you have them setup, they should always be there

In other MCP clients you can choose to disable servers in a chat session, like in VS Code with the agent mode:

(Screenshot copy pasted from https://code.visualstudio.com/docs/copilot/chat/mcp-servers#_use-mcp-tools-in-agent-mode)
Here in VS Code you can even disable individual tools, but also entire servers.

I think it can be useful if you have many MCP servers running with a lot of tools each, some overlapping and similar tool description, and the LLM chooses the wrong tools. Then you can disable the problematic one, and reduce the "search space" for the LLM.

philippgille · 2025-04-10T18:54:45Z

features.md

+Or in your config file:
+
+```yaml
+mcp-servers-from: ~/.config/mcp/servers.json
+```
+
+### Specifying MCP servers in the mods settings 


I think it's worth describing the expected file schema or show an example file somewhere in this section, because it's not described anywhere yet, right?

Suggested change

Or in your config file:

```yaml

mcp-servers-from: ~/.config/mcp/servers.json

```

### Specifying MCP servers in the mods settings

Or in your config file:

```yaml

mcp-servers-from: ~/.config/mcp/servers.json

```

That file should look like this:

```json

{

"mcpServers": {

"fetch": {

"command": "uvx",

"args": ["mcp-server-fetch"]

}

}

}

```

### Specifying MCP servers in the mods settings

philippgille · 2025-04-10T19:12:05Z

go.mod

+go 1.23
+
+toolchain go1.24.0


Is the latest toolchain required? If not, I think this can stay at whichever Go version is minimally required by the dependencies. mcp-go only requires Go 1.23. People are still free to use Go 1.24 compiler to build this, but they won't have to.
(Technically, since Go 1.21, Go checks for the version in the go.mod file and implicitly downloads and uses newer versions behind the scenes if necessary, so theoretically users could still just have Go 1.23 installed. But I think it's still better/cleaner to just declare the version that's actually required, at least as long as it's officially supported)
More info on that: https://go.dev/doc/go1.21#tools

Suggested change

go 1.23

toolchain go1.24.0

go 1.23.0

caarlos0 · 2025-04-11T16:10:01Z

Also, not sure if the user would want to optionally enable/disable MCPs... judging by how claude desktop works, it's kind of always there. I think if you have them setup, they should always be there

In other MCP clients you can choose to disable servers in a chat session, like in VS Code with the agent mode: (Screenshot copy pasted from code.visualstudio.com/docs/copilot/chat/mcp-servers#_use-mcp-tools-in-agent-mode) Here in VS Code you can even disable individual tools, but also entire servers.

I think it can be useful if you have many MCP servers running with a lot of tools each, some overlapping and similar tool description, and the LLM chooses the wrong tools. Then you can disable the problematic one, and reduce the "search space" for the LLM.

ah, gotcha

so, probably have them all enabled by default, with options to disable all, or individually disable

I would suggest something like:

--mcp-disable=name

with * meaning all mcps 🤔

caarlos0 · 2025-04-14T19:13:41Z

ok, here's my simplified api proposal:

keep:

mcp-servers in config
--list-mcp-servers command flag
--list-mcp-tools command flag

remove:

default-mcp-servers in config: default is always all mcp servers configured
mcp-servers-from in config: just for the sake of simplicity, can be added later if people really want it - i have a feeling they might not need it
--mcp-servers, -T flag: I think we should leave all enabled by default, and add a --disable-mcp-server=[name|*] instead
--all-mcp-servers, -A flag: should be the default
--mcp-servers-from flag: should only use from the config file

Some comments on the TODO list:

execute tool calls after response stream closed: not sure what this means, can you clarify?

all that being said, I have some time this week to help with this if you want.

dexhorthy · 2025-04-15T01:48:45Z

I like all these simplifications and they make sense as defaults. I don't know if I'll have time to get hands on with this thing this week. Re: running the tools after the stream closes… Basically with the current setup of mods, when OpenAI or whoever streams back a chunk, we print it to the console and then we're done. With tool calling, we print it to the console and also aggregate/merge those chunks into an in memory json object, which we will later parse and send to the locally running mcp server over stdio. Then we take the results of that execution in mcp land and send it back to the model, and repeat these steps until the model returns text and not tool calls Then we stream that final text to the console So there's quite a bit left to do here and perhaps a bit outside my skill level with bubble tea, at least as it stands today

…

On Mon, Apr 14, 2025 at 12:14 PM, Carlos Alexandro Becker < ***@***.*** > wrote: ok, here's my simplified api proposal: keep: * mcp-servers in config * --list-mcp-servers command flag * --list-mcp-tools command flag remove: * default-mcp-servers in config: default is always all mcp servers configured * mcp-servers-from in config: just for the sake of simplicity, can be added later if people really want it - i have a feeling they might not need it * --mcp-servers , -T flag: I think we should leave all enabled by default, and add a --disable-mcp-server=[name|*] instead * --all-mcp-servers , -A flag: should be the default * --mcp-servers-from flag: should only use from the config file Some comments on the TODO list: * execute tool calls after response stream closed: not sure what this means, can you clarify? all that being said, I have some time this week to help with this if you want. — Reply to this email directly, view it on GitHub ( #460 (comment) ) , or unsubscribe ( https://github.com/notifications/unsubscribe-auth/AA4OZLO2JKKPKYEFWBUYWZD2ZQCHZAVCNFSM6AAAAABYS2JDQKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDQMBSG42DSNJWGY ). You are receiving this because you authored the thread. Message ID: <charmbracelet/mods/pull/460/c2802749566 @ github. com> *caarlos0* left a comment (charmbracelet/mods#460) ( #460 (comment) ) ok, here's my simplified api proposal: keep: * mcp-servers in config * --list-mcp-servers command flag * --list-mcp-tools command flag remove: * default-mcp-servers in config: default is always all mcp servers configured * mcp-servers-from in config: just for the sake of simplicity, can be added later if people really want it - i have a feeling they might not need it * --mcp-servers , -T flag: I think we should leave all enabled by default, and add a --disable-mcp-server=[name|*] instead * --all-mcp-servers , -A flag: should be the default * --mcp-servers-from flag: should only use from the config file Some comments on the TODO list: * execute tool calls after response stream closed: not sure what this means, can you clarify? all that being said, I have some time this week to help with this if you want. — Reply to this email directly, view it on GitHub ( #460 (comment) ) , or unsubscribe ( https://github.com/notifications/unsubscribe-auth/AA4OZLO2JKKPKYEFWBUYWZD2ZQCHZAVCNFSM6AAAAABYS2JDQKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDQMBSG42DSNJWGY ). You are receiving this because you authored the thread. Message ID: <charmbracelet/mods/pull/460/c2802749566 @ github. com>

caarlos0 · 2025-04-15T02:34:44Z

nice, thanks for the response.

i might try to give a stab at those cleanups/changes this week then 🙏🏻

dexhorthy requested a review from caarlos0 as a code owner March 8, 2025 09:05

dexhorthy mentioned this pull request Mar 8, 2025

Feature Request: Add MCP Support #397

Open

dexhorthy force-pushed the dexhorthy/397-mcp-support-2 branch from 7688a87 to 8e2b851 Compare March 8, 2025 09:11

raphamorim reviewed Mar 10, 2025

View reviewed changes

features.md

```bash

mods --list-mcp-tools

```

Copy link

Member

raphamorim Mar 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you remove these extra lines to the end?

caarlos0 approved these changes Mar 24, 2025

View reviewed changes

philippgille reviewed Apr 10, 2025

View reviewed changes

caarlos0 mentioned this pull request Apr 22, 2025

feat: mcp support, openai update, refactor #486

Open

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DRAFT - initial stab at MCP support #460

DRAFT - initial stab at MCP support #460

dexhorthy commented Mar 8, 2025 •

edited

Loading

raphamorim Mar 10, 2025

dexhorthy Mar 21, 2025

philippgille Apr 10, 2025

raphamorim Mar 10, 2025

caarlos0 left a comment •

edited

Loading

philippgille commented Apr 10, 2025

philippgille Apr 10, 2025 •

edited

Loading

philippgille Apr 10, 2025

caarlos0 commented Apr 11, 2025

caarlos0 commented Apr 14, 2025

dexhorthy commented Apr 15, 2025 via email

caarlos0 commented Apr 15, 2025

		@@ -118,3 +118,76 @@ mods --delete='naturals' --delete='a2e2'

		Keep in mind that these operations are not reversible.
		You can repeat the delete flag to delete multiple conversations at once.

DRAFT - initial stab at MCP support #460

Are you sure you want to change the base?

DRAFT - initial stab at MCP support #460

Conversation

dexhorthy commented Mar 8, 2025 • edited Loading

raphamorim Mar 10, 2025

Choose a reason for hiding this comment

dexhorthy Mar 21, 2025

Choose a reason for hiding this comment

philippgille Apr 10, 2025

Choose a reason for hiding this comment

raphamorim Mar 10, 2025

Choose a reason for hiding this comment

caarlos0 left a comment • edited Loading

Choose a reason for hiding this comment

philippgille commented Apr 10, 2025

philippgille Apr 10, 2025 • edited Loading

Choose a reason for hiding this comment

philippgille Apr 10, 2025

Choose a reason for hiding this comment

caarlos0 commented Apr 11, 2025

caarlos0 commented Apr 14, 2025

keep:

remove:

dexhorthy commented Apr 15, 2025 via email

caarlos0 commented Apr 15, 2025

dexhorthy commented Mar 8, 2025 •

edited

Loading

caarlos0 left a comment •

edited

Loading

philippgille Apr 10, 2025 •

edited

Loading