Talk MCP: a client for external tool servers (#251)
docs/plans/06-mcp-support.md asks for the host direction — Maven connects OUT
to MCP servers and consumes what they offer. This is the client half: the
protocol, the transports, the connection manager, the config block. Nothing is
wired into a turn yet, and nothing here exposes Maven's own capabilities to an
outside caller.
internal/mcp:
- hand-rolled JSON-RPC 2.0 (the wire format is four fields, and the repo
vendors its deps, so a library would cost more than it saves);
- two transports: a stdio subprocess on this box, and streamable HTTP, which
accepts a plain JSON reply or an SSE frame because servers disagree about
which they send;
- Client: initialize handshake, tools/list, tools/call, resources/list,
resources/read. Text content only — everything downstream is a sentence;
- Manager: lazy dial, per-server failure that never blocks boot or the other
servers, backoff reconnect, Status for a web surface, graceful Close;
- the allowlist encoding: a discovered tool becomes the store row
"vikunja_list_tasks" with cmd ["mcp","vikunja","list_tasks"], scope
"mcp:vikunja". No new column, no migration, and ProposeTool, EnableTool,
the act matcher and the confirm turn all keep working untouched.
Constraints held, in code rather than in prose:
- OFF unless configured, and a server is dark until "enabled": true.
- A url server goes through internal/webfetch, so the SSRF guard, the size
cap, the redirect cap and the per-host rate limit apply. Reaching loopback
needs allow_private on THAT server, and each server gets its own fetcher so
one loopback exemption cannot become a hole for a public endpoint.
- readOnlyHint decides destructive: no hint means "assume it mutates", which
will route the call through the existing confirm turn. Guessing wrong in
that direction only costs a question.
- The catalogue stays small on purpose — allow_tools, and max_tools=12 per
server. The resident model is a 1.7B with a 4096-token context; a tool name
it half-remembers is a wrong act.
- Only the tool name and the router's arguments are sent. There is no API
here through which a note, a fact or the persona block could travel.
webfetch grows Post (JSON-RPC cannot be a GET) and surfaces response headers
for Mcp-Session-Id. It shares Get's guards exactly: a body buys a caller
nothing, a POST to the LAN is refused for the same reason a GET is.
Verified against the real Vikunja MCP server on homesrv
(http://localhost:9100/mcp): handshake, three discovered tools with update_task
correctly NOT read-only, a live list_projects call, a tool excluded by
allow_tools refused, and the same server refused outright once allow_private
was dropped. Tests cover both transports (the stdio one against a real
subprocess), SSE and JSON framing, session echo, reconnect, and the config
validation.
This commit is contained in:
@@ -22,6 +22,7 @@ import (
|
||||
|
||||
"github.com/kami/maven/internal/delivery/ntfysink"
|
||||
"github.com/kami/maven/internal/delivery/telegramsink"
|
||||
"github.com/kami/maven/internal/mcp"
|
||||
"github.com/kami/maven/internal/morning"
|
||||
"github.com/kami/maven/internal/update"
|
||||
"github.com/robfig/cron/v3"
|
||||
@@ -191,6 +192,119 @@ type Config struct {
|
||||
// discovers and executes capabilities through Hexis for ecosystem actions.
|
||||
// nil ⇒ no capability-aware routing.
|
||||
Hexis *HexisConfig `json:"hexis,omitempty"`
|
||||
|
||||
// MCP — Model Context Protocol servers Maven connects OUT to (Vikunja
|
||||
// #251). nil / absent / no enabled server ⇒ no connection is made and no
|
||||
// tool is discovered, like every other capability that reaches outside the
|
||||
// box. She is a client here, never a server: nothing exposes her own
|
||||
// capabilities to an outside caller. See MCPConfig.
|
||||
MCP *MCPConfig `json:"mcp,omitempty"`
|
||||
}
|
||||
|
||||
// MCPConfig — the MCP client block. Servers are dark until one has
|
||||
// `"enabled": true`, and a discovered tool is only ever PROPOSED: Kami enables
|
||||
// it on /tools, on the authed surface, exactly as he would a shell tool. The
|
||||
// voice path can never grant a capability to itself.
|
||||
type MCPConfig struct {
|
||||
// Servers — the configured servers. Each needs exactly one of command
|
||||
// (a subprocess on this box) or url (a streamable-HTTP endpoint).
|
||||
Servers []MCPServerConfig `json:"servers,omitempty"`
|
||||
|
||||
// Timeout — per-call budget for every server that does not set its own.
|
||||
// 0 ⇒ mcp.DefaultTimeout (15s). A tool slower than this is not usable in a
|
||||
// spoken turn.
|
||||
Timeout Duration `json:"timeout,omitempty"`
|
||||
|
||||
// AllowHosts / DenyHosts — the host lists for the shared webfetch door that
|
||||
// url servers go through. Deny wins. Private addresses are refused
|
||||
// unconditionally unless the individual server sets allow_private.
|
||||
AllowHosts []string `json:"allow_hosts,omitempty"`
|
||||
DenyHosts []string `json:"deny_hosts,omitempty"`
|
||||
|
||||
// MaxBytes — cap on one JSON-RPC response. 0 ⇒ webfetch.DefaultMaxBytes.
|
||||
MaxBytes int64 `json:"max_bytes,omitempty"`
|
||||
}
|
||||
|
||||
// MCPServerConfig — one MCP server.
|
||||
type MCPServerConfig struct {
|
||||
// Name — the local handle. It prefixes every tool this server contributes
|
||||
// ("vikunja" + "list_tasks" ⇒ the allowlist row "vikunja_list_tasks") and
|
||||
// becomes the store scope "mcp:<name>", so its provenance is readable on
|
||||
// /tools without opening the config.
|
||||
Name string `json:"name"`
|
||||
|
||||
// Command / Args / Env / Dir — a stdio server: a child process of mavend,
|
||||
// on this box, under this user. argv, never a shell string.
|
||||
Command string `json:"command,omitempty"`
|
||||
Args []string `json:"args,omitempty"`
|
||||
Env []string `json:"env,omitempty"`
|
||||
Dir string `json:"dir,omitempty"`
|
||||
|
||||
// URL — a streamable-HTTP endpoint. It is fetched through
|
||||
// internal/webfetch, so the SSRF guard, the redirect cap, the size cap and
|
||||
// the one-request-per-host-per-second limit all apply.
|
||||
URL string `json:"url,omitempty"`
|
||||
|
||||
// AllowPrivate — let THIS server be a loopback or LAN address. The Vikunja
|
||||
// server on homesrv is "http://localhost:9100/mcp", which is refused
|
||||
// without this flag. Understand what it means before setting it: a local
|
||||
// server is a DIFFERENT trust level from a public one. It is inside the
|
||||
// network, it usually needs no credential, and it can change things that
|
||||
// matter — so an argument the router got wrong lands somewhere real. Set it
|
||||
// only for a server you run yourself, and prefer allow_tools with it.
|
||||
AllowPrivate bool `json:"allow_private,omitempty"`
|
||||
|
||||
// AllowTools — when set, the ONLY remote tool names taken from this server.
|
||||
// This is the knob that keeps the catalogue deliberate: the resident model
|
||||
// is a 1.7B with a 4096-token context, and a tool name it half-remembers is
|
||||
// a wrong act, so fewer and better-chosen beats complete.
|
||||
AllowTools []string `json:"allow_tools,omitempty"`
|
||||
|
||||
// MaxTools — cap on this server's contribution. 0 ⇒ mcp.DefaultMaxTools (12).
|
||||
MaxTools int `json:"max_tools,omitempty"`
|
||||
|
||||
// Timeout — per-call budget for this server. 0 ⇒ MCPConfig.Timeout.
|
||||
Timeout Duration `json:"timeout,omitempty"`
|
||||
|
||||
// Enabled — false (the default) keeps a configured server described but
|
||||
// dark, so a block can be written and reviewed before it is switched on.
|
||||
Enabled bool `json:"enabled,omitempty"`
|
||||
}
|
||||
|
||||
// MCPServers maps the config blocks onto the mcp package's own type. It lives
|
||||
// here so config validation and daemon wiring cannot drift on the mapping.
|
||||
// Returns nil when nothing is configured or nothing is enabled.
|
||||
func (c *Config) MCPServers() []mcp.ServerConfig {
|
||||
if c.MCP == nil {
|
||||
return nil
|
||||
}
|
||||
out := make([]mcp.ServerConfig, 0, len(c.MCP.Servers))
|
||||
for _, s := range c.MCP.Servers {
|
||||
if !s.Enabled {
|
||||
continue
|
||||
}
|
||||
timeout := time.Duration(s.Timeout)
|
||||
if timeout <= 0 {
|
||||
timeout = time.Duration(c.MCP.Timeout)
|
||||
}
|
||||
out = append(out, mcp.ServerConfig{
|
||||
Name: s.Name,
|
||||
Command: s.Command,
|
||||
Args: s.Args,
|
||||
Env: s.Env,
|
||||
Dir: s.Dir,
|
||||
URL: s.URL,
|
||||
AllowPrivate: s.AllowPrivate,
|
||||
AllowTools: s.AllowTools,
|
||||
MaxTools: s.MaxTools,
|
||||
Timeout: timeout,
|
||||
Enabled: true,
|
||||
})
|
||||
}
|
||||
if len(out) == 0 {
|
||||
return nil
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
// PraxisConfig — maven's connection to the Praxis attention service.
|
||||
@@ -783,6 +897,12 @@ func (c *Config) applyDefaults() {
|
||||
c.Feeds = nil
|
||||
}
|
||||
|
||||
// Same rule for MCP: a block with no server, or none enabled, is the same
|
||||
// as no block at all. Normalising it to nil keeps "off" in one place.
|
||||
if c.MCP != nil && len(c.MCPServers()) == 0 {
|
||||
c.MCP = nil
|
||||
}
|
||||
|
||||
// Same rule for the crawler: a block that neither answers on demand nor
|
||||
// watches anything has nothing to do, so it is normalised to "off".
|
||||
if c.Crawl != nil && !c.Crawl.OnDemand && len(c.Crawl.Watches) == 0 {
|
||||
@@ -887,6 +1007,12 @@ func (c *Config) validate() error {
|
||||
return fmt.Errorf("routine %q: bad cron %q: %w", r.Name, r.Cron, err)
|
||||
}
|
||||
}
|
||||
// An MCP block with a typo (no name, both command and url, a bare hostname
|
||||
// as the url) fails here, at startup, rather than at the first turn that
|
||||
// needed the tool.
|
||||
if err := mcp.Validate(c.MCPServers()); err != nil {
|
||||
return err
|
||||
}
|
||||
if len(c.MorningRoutines) > 0 {
|
||||
if err := morning.Validate(morningRoutinesFromConfig(c.MorningRoutines)); err != nil {
|
||||
return err
|
||||
|
||||
@@ -0,0 +1,82 @@
|
||||
package config
|
||||
|
||||
import (
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
func TestMCPAbsentIsOff(t *testing.T) {
|
||||
c, err := Load(writeConfig(t, `{}`))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if c.MCP != nil {
|
||||
t.Error("no mcp block ⇒ nil")
|
||||
}
|
||||
if got := c.MCPServers(); got != nil {
|
||||
t.Errorf("MCPServers() = %+v, want nil", got)
|
||||
}
|
||||
}
|
||||
|
||||
// A described-but-not-enabled server must not be wired. This is how a block can
|
||||
// sit in the config file, reviewed, before it is switched on.
|
||||
func TestMCPDisabledServerIsOff(t *testing.T) {
|
||||
c, err := Load(writeConfig(t, `{"mcp":{"servers":[
|
||||
{"name":"vikunja","url":"http://localhost:9100/mcp","allow_private":true}]}}`))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if c.MCP != nil {
|
||||
t.Errorf("a block with nothing enabled must normalise to nil, got %+v", c.MCP)
|
||||
}
|
||||
if got := c.MCPServers(); len(got) != 0 {
|
||||
t.Errorf("MCPServers() = %+v", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestMCPEnabledServerMapping(t *testing.T) {
|
||||
c, err := Load(writeConfig(t, `{"mcp":{
|
||||
"timeout":"5s",
|
||||
"servers":[
|
||||
{"name":"vikunja","url":"http://localhost:9100/mcp","allow_private":true,
|
||||
"allow_tools":["list_tasks"],"max_tools":3,"enabled":true},
|
||||
{"name":"files","command":"mcp-server-fs","args":["/srv"],"timeout":"1s","enabled":true},
|
||||
{"name":"off","command":"nope"}
|
||||
]}}`))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
got := c.MCPServers()
|
||||
if len(got) != 2 {
|
||||
t.Fatalf("servers = %+v", got)
|
||||
}
|
||||
if got[0].Name != "vikunja" || !got[0].AllowPrivate || got[0].MaxTools != 3 ||
|
||||
len(got[0].AllowTools) != 1 || got[0].Timeout != 5*time.Second {
|
||||
t.Errorf("vikunja mapped wrong: %+v", got[0])
|
||||
}
|
||||
if got[1].Command != "mcp-server-fs" || len(got[1].Args) != 1 || got[1].Timeout != time.Second {
|
||||
t.Errorf("files mapped wrong: %+v", got[1])
|
||||
}
|
||||
// allow_private is per server and must not leak to the other one.
|
||||
if got[1].AllowPrivate {
|
||||
t.Error("allow_private leaked between servers")
|
||||
}
|
||||
}
|
||||
|
||||
func TestMCPBadServerFailsAtStartup(t *testing.T) {
|
||||
cases := map[string]string{
|
||||
"no name": `{"mcp":{"servers":[{"command":"x","enabled":true}]}}`,
|
||||
"both": `{"mcp":{"servers":[{"name":"a","command":"x","url":"http://a.test","enabled":true}]}}`,
|
||||
"neither": `{"mcp":{"servers":[{"name":"a","enabled":true}]}}`,
|
||||
"bad scheme": `{"mcp":{"servers":[{"name":"a","url":"unix:///run/x.sock","enabled":true}]}}`,
|
||||
"duplicate": `{"mcp":{"servers":[{"name":"a","command":"x","enabled":true},{"name":"a","command":"y","enabled":true}]}}`,
|
||||
"spacey name": `{"mcp":{"servers":[{"name":"a b","command":"x","enabled":true}]}}`,
|
||||
}
|
||||
for name, body := range cases {
|
||||
t.Run(name, func(t *testing.T) {
|
||||
if _, err := Load(writeConfig(t, body)); err == nil {
|
||||
t.Fatal("want a startup error")
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user