From 1c20df70f8958fd7be4f7c24a4e99636eba073c2 Mon Sep 17 00:00:00 2001 From: claude Date: Thu, 6 Aug 2026 01:13:16 +0400 Subject: [PATCH] config: MCPServerConfig and its mappers move to mcp.go (V-410) Pure move. Nothing changes but the file a reader opens. validateMCP is the one new name: config.go's validate arm becomes a method next to the block it checks, which is the shape the rest of this sweep follows. Co-Authored-By: Claude Opus 5 --- internal/config/config.go | 114 +---------------------------------- internal/config/mcp.go | 122 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 123 insertions(+), 113 deletions(-) create mode 100644 internal/config/mcp.go diff --git a/internal/config/config.go b/internal/config/config.go index 14055ac..03ce7bd 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -23,7 +23,6 @@ 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/netscan" "github.com/kami/maven/internal/smarthome" @@ -432,114 +431,6 @@ func (c *Config) NetScanner() (netscan.Config, bool) { }, true } -// 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:", 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"` - - // Headers — sent verbatim on every request to a url server. This is how a - // bearer token reaches a real remote MCP server: {"Authorization": "Bearer - // ${MCP_TOKEN}"}, with the value in the gitignored env file like the - // telegram credentials. The Vikunja server on homesrv needs none only - // because it is unauthenticated on loopback. - Headers map[string]string `json:"headers,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. -// -// Disabled servers are dropped here, which is why validation does NOT use this -// list — see allMCPServers. -func (c *Config) MCPServers() []mcp.ServerConfig { - return c.mcpServers(true) -} - -// allMCPServers is every configured server, enabled or not, for validation. -// -// Validating only the enabled ones meant a block with both command and url, or -// a bare hostname as the url, passed startup validation while it was dark. The -// doc on Enabled says a block can be written and reviewed before it is switched -// on; the review the config layer could give was the one thing skipped. Enabled -// gates the dialing, not the shape check. -func (c *Config) allMCPServers() []mcp.ServerConfig { - return c.mcpServers(false) -} - -func (c *Config) mcpServers(onlyEnabled bool) []mcp.ServerConfig { - if c.MCP == nil { - return nil - } - out := make([]mcp.ServerConfig, 0, len(c.MCP.Servers)) - for _, s := range c.MCP.Servers { - if onlyEnabled && !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, - Headers: s.Headers, - Timeout: timeout, - Enabled: s.Enabled, - }) - } - if len(out) == 0 { - return nil - } - return out -} - // PraxisConfig — maven's connection to the Praxis attention service. type PraxisConfig struct { // URL — the Praxis HTTP API base URL (e.g. "http://localhost:9742"). @@ -1680,10 +1571,7 @@ 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.allMCPServers()); err != nil { + if err := c.validateMCP(); err != nil { return err } // Same for the house: a missing token or a bare hostname fails at startup, diff --git a/internal/config/mcp.go b/internal/config/mcp.go new file mode 100644 index 0000000..f8da91c --- /dev/null +++ b/internal/config/mcp.go @@ -0,0 +1,122 @@ +package config + +import ( + "time" + + "github.com/kami/maven/internal/mcp" +) + +// 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:", 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"` + + // Headers — sent verbatim on every request to a url server. This is how a + // bearer token reaches a real remote MCP server: {"Authorization": "Bearer + // ${MCP_TOKEN}"}, with the value in the gitignored env file like the + // telegram credentials. The Vikunja server on homesrv needs none only + // because it is unauthenticated on loopback. + Headers map[string]string `json:"headers,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. +// +// Disabled servers are dropped here, which is why validation does NOT use this +// list — see allMCPServers. +func (c *Config) MCPServers() []mcp.ServerConfig { + return c.mcpServers(true) +} + +// allMCPServers is every configured server, enabled or not, for validation. +// +// Validating only the enabled ones meant a block with both command and url, or +// a bare hostname as the url, passed startup validation while it was dark. The +// doc on Enabled says a block can be written and reviewed before it is switched +// on; the review the config layer could give was the one thing skipped. Enabled +// gates the dialing, not the shape check. +func (c *Config) allMCPServers() []mcp.ServerConfig { + return c.mcpServers(false) +} + +func (c *Config) mcpServers(onlyEnabled bool) []mcp.ServerConfig { + if c.MCP == nil { + return nil + } + out := make([]mcp.ServerConfig, 0, len(c.MCP.Servers)) + for _, s := range c.MCP.Servers { + if onlyEnabled && !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, + Headers: s.Headers, + Timeout: timeout, + Enabled: s.Enabled, + }) + } + if len(out) == 0 { + return nil + } + return out +} + +// validateMCP fails a block with a typo (no name, both command and url, a bare +// hostname as the url) at startup, rather than at the first turn that needed +// the tool. +func (c *Config) validateMCP() error { + return mcp.Validate(c.allMCPServers()) +}