diff --git a/cmd/mavend/actions_act.go b/cmd/mavend/actions_act.go index d5a1dc1..be83e9e 100644 --- a/cmd/mavend/actions_act.go +++ b/cmd/mavend/actions_act.go @@ -5,6 +5,7 @@ import ( "errors" "log" + "github.com/kami/maven/internal/mcp" "github.com/kami/maven/internal/router" "github.com/kami/maven/internal/tool" ) @@ -52,6 +53,12 @@ func (h *reactiveHandler) actionAct(ctx context.Context, dec router.Decision) st return "выполнить «" + phrase + "»? скажи «да» или «нет»." case errors.Is(err, tool.ErrNotEnabled): return h.proposeGap(ctx, dec) + case errors.Is(err, mcp.ErrNeedsArgs): + // An MCP tool that wants named arguments a spoken verb cannot + // supply. Guessing them would be a wrong act, so she says so + // instead — the tool is still runnable from the authed surface, + // where a human types them. + return "этому инструменту нужны аргументы, которые я из голоса не соберу — я не буду угадывать." } log.Printf("voice: tool %s: %v", dec.Slots.Fn, err) if out != "" { diff --git a/cmd/mavend/main.go b/cmd/mavend/main.go index e4fffa2..9253338 100644 --- a/cmd/mavend/main.go +++ b/cmd/mavend/main.go @@ -280,6 +280,9 @@ func run(args []string) error { api := coreAPI.(*daemonAPI) api.chatFn = voiceW.handler.handleText } + if voiceW != nil && voiceW.mcp != nil { + coreAPI.(*daemonAPI).getMCPServers = voiceW.mcp.status + } } else { // locked mode: no real store yet, so there's no meaningful CoreAPI to // serve. srv.Check below is the actual guard — every CoreAPI call is @@ -518,6 +521,11 @@ func run(args []string) error { }() } + // Keep MCP connections alive (nil unless configured). + if voiceW != nil && voiceW.mcp != nil { + go voiceW.mcp.run(ctx) + } + dl.unlock() log.Printf("mavend: unlocked via passkey assertion") return nil @@ -577,6 +585,13 @@ func run(args []string) error { crawlWkr.run(ctx) }() } + if voiceW != nil && voiceW.mcp != nil { + wg.Add(1) + go func() { + defer wg.Done() + voiceW.mcp.run(ctx) + }() + } } <-ctx.Done() diff --git a/cmd/mavend/mcp.go b/cmd/mavend/mcp.go new file mode 100644 index 0000000..7b31117 --- /dev/null +++ b/cmd/mavend/mcp.go @@ -0,0 +1,154 @@ +package main + +import ( + "context" + "fmt" + "log" + "time" + + "github.com/kami/maven/internal/config" + "github.com/kami/maven/internal/ipc" + "github.com/kami/maven/internal/mcp" + "github.com/kami/maven/internal/store" + "github.com/kami/maven/internal/webfetch" +) + +// mcpRefreshInterval — how often the manager re-dials a server that is down. +// The manager applies its own backoff on top, so this being short is cheap. +const mcpRefreshInterval = time.Minute + +// mcpWiring — the MCP client, when the `mcp` block configures at least one +// enabled server. nil ⇒ nothing was configured, nothing is connected, and an +// allowlist row that happens to look like an MCP row refuses to run. +// +// It lives on the voice wiring because MCP tools ARE acts: they run through +// tool.Executor, the enabled allowlist and the confirm turn, which only exist +// on the voice/chat path. No voice surface ⇒ nothing that could call a tool. +type mcpWiring struct { + mgr *mcp.Manager + st *store.Store +} + +// wireMCP builds the manager, connects, and proposes what it found. It never +// fails the daemon: a server that is unreachable at boot is logged and retried, +// because Maven starting is not contingent on someone else's process. +func wireMCP(cfg *config.Config, st *store.Store) *mcpWiring { + servers := cfg.MCPServers() + if len(servers) == 0 { + return nil + } + limits := webfetch.Config{} + if cfg.MCP != nil { + limits.AllowHosts = cfg.MCP.AllowHosts + limits.DenyHosts = cfg.MCP.DenyHosts + limits.MaxBytes = cfg.MCP.MaxBytes + limits.Timeout = time.Duration(cfg.MCP.Timeout) + } + mgr, err := mcp.NewManager(mcp.WebfetchDoor(limits), servers) + if err != nil { + // Validation already ran in config.validate, so this is a programming + // error rather than a config one. Still not fatal: MCP off is a working + // Maven. + log.Printf("mcp: not wired: %v", err) + return nil + } + w := &mcpWiring{mgr: mgr, st: st} + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + mgr.Connect(ctx) + w.propose(ctx) + return w +} + +// propose writes a 'proposed' allowlist row for every discovered tool. It does +// NOT enable anything: a configured server is a place Maven may look, not a +// capability she has. Kami enables what he wants on /tools, behind step-up, +// which is the same gate a shell tool goes through. +// +// Re-running on every boot is idempotent — ProposeMCPTool never touches an +// existing row, so a tool he disabled stays disabled and one he enabled keeps +// the cmd he enabled it with. +func (w *mcpWiring) propose(ctx context.Context) { + if w == nil { + return + } + now := time.Now() + fresh := 0 + for _, t := range w.mgr.Tools() { + name := mcp.LocalName(t.Server, t.Name) + // No readOnlyHint ⇒ assume it mutates ⇒ the confirm turn. Being wrong + // in this direction only costs a question. + destructive := !t.ReadOnly + provenance := fmt.Sprintf("mcp %s/%s", t.Server, t.Name) + if t.Description != "" { + provenance += ": " + t.Description + } + ok, err := w.st.ProposeMCPTool(ctx, name, mcp.Scope(t.Server), + mcp.Cmd(t.Server, t.Name), destructive, provenance, now) + if err != nil { + log.Printf("mcp: propose %s: %v", name, err) + continue + } + if ok { + fresh++ + } + } + if fresh > 0 { + log.Printf("mcp: %d new tool proposal(s) waiting on /tools", fresh) + } +} + +// run re-dials downed servers and picks up tools that appeared, until ctx is +// canceled. +func (w *mcpWiring) run(ctx context.Context) { + if w == nil { + return + } + t := time.NewTicker(mcpRefreshInterval) + defer t.Stop() + for { + select { + case <-ctx.Done(): + return + case <-t.C: + w.mgr.Refresh(ctx) + w.propose(ctx) + } + } +} + +// status maps the manager's view onto the wire type the web surface reads. +func (w *mcpWiring) status() []ipc.MCPServerStatus { + if w == nil { + return nil + } + in := w.mgr.Status() + out := make([]ipc.MCPServerStatus, 0, len(in)) + for _, s := range in { + out = append(out, ipc.MCPServerStatus{ + Name: s.Name, + Transport: s.Transport, + Target: s.Target, + Connected: s.Connected, + Server: s.Server, + Tools: s.Tools, + Err: s.Err, + }) + } + return out +} + +func (w *mcpWiring) close() { + if w == nil { + return + } + _ = w.mgr.Close() +} + +// caller is the tool.MCPCaller the executor gets, or nil when MCP is off. +func (w *mcpWiring) caller() *mcp.Manager { + if w == nil { + return nil + } + return w.mgr +} diff --git a/cmd/mavend/mcp_test.go b/cmd/mavend/mcp_test.go new file mode 100644 index 0000000..6339524 --- /dev/null +++ b/cmd/mavend/mcp_test.go @@ -0,0 +1,77 @@ +package main + +import ( + "context" + "strings" + "testing" + + "github.com/kami/maven/internal/config" +) + +func TestWireMCPOffWhenUnconfigured(t *testing.T) { + st := newTestStore(t) + for name, cfg := range map[string]*config.Config{ + "no block": {}, + "nothing enabled": {MCP: &config.MCPConfig{Servers: []config.MCPServerConfig{ + {Name: "vikunja", URL: "http://192.168.1.104:9100/mcp"}, + }}}, + } { + t.Run(name, func(t *testing.T) { + if w := wireMCP(cfg, st); w != nil { + t.Fatal("MCP must be off unless a server is configured AND enabled") + } + }) + } + // nil wiring must be safe to use everywhere it is reachable. + var w *mcpWiring + w.close() + w.propose(context.Background()) + if w.status() != nil || w.caller() != nil { + t.Fatal("a nil wiring must report nothing") + } +} + +// An unreachable server must not stop the daemon, must be reported as down, and +// must propose nothing. +func TestWireMCPUnreachableServerIsNotFatal(t *testing.T) { + st := newTestStore(t) + w := wireMCP(&config.Config{MCP: &config.MCPConfig{Servers: []config.MCPServerConfig{{ + Name: "dead", Command: "/nonexistent/mcp-server", Enabled: true, + }}}}, st) + if w == nil { + t.Fatal("a configured server should still wire") + } + defer w.close() + st2 := w.status() + if len(st2) != 1 || st2[0].Connected || st2[0].Err == "" { + t.Fatalf("status = %+v", st2) + } + tools, err := st.ListTools(context.Background(), "") + if err != nil { + t.Fatal(err) + } + if len(tools) != 0 { + t.Fatalf("a server that never answered must propose nothing, got %+v", tools) + } +} + +// A url server whose address is private is refused by webfetch unless that +// server sets allow_private. This is the guard the whole MCP path rides on, so +// it is asserted here too, at the wiring level. +func TestWireMCPPrivateURLRefusedWithoutAllowPrivate(t *testing.T) { + st := newTestStore(t) + w := wireMCP(&config.Config{MCP: &config.MCPConfig{Servers: []config.MCPServerConfig{{ + Name: "lan", URL: "http://127.0.0.1:9100/mcp", Enabled: true, + }}}}, st) + if w == nil { + t.Fatal("should wire") + } + defer w.close() + s := w.status()[0] + if s.Connected { + t.Fatal("a loopback server must not connect without allow_private") + } + if !strings.Contains(s.Err, "private address") { + t.Fatalf("err = %q, want the private-address refusal", s.Err) + } +} diff --git a/cmd/mavend/tick.go b/cmd/mavend/tick.go index 3932173..746cbe7 100644 --- a/cmd/mavend/tick.go +++ b/cmd/mavend/tick.go @@ -941,6 +941,7 @@ type daemonAPI struct { getMorningStatus func(ctx context.Context) []ipc.MorningRoutineStatus getDayPlan func(ctx context.Context) ipc.DayPlan chatFn func(ctx context.Context, text string) string + getMCPServers func() []ipc.MCPServerStatus } func (d *daemonAPI) Chat(ctx context.Context, text string) (string, error) { @@ -950,6 +951,16 @@ func (d *daemonAPI) Chat(ctx context.Context, text string) (string, error) { return d.chatFn(ctx, text), nil } +// MCPServers — the configured MCP servers and their health (Vikunja #251). +// Empty, not an error, when the mcp block is absent: "not configured" is the +// default state and the web surface renders it as such. +func (d *daemonAPI) MCPServers(ctx context.Context) ([]ipc.MCPServerStatus, error) { + if d.getMCPServers == nil { + return nil, nil + } + return d.getMCPServers(), nil +} + func (d *daemonAPI) TickTrace(ctx context.Context) (ipc.TickTrace, error) { trace := d.getTrace() if trace == nil { diff --git a/cmd/mavend/voicewire.go b/cmd/mavend/voicewire.go index 06fa81a..77e787e 100644 --- a/cmd/mavend/voicewire.go +++ b/cmd/mavend/voicewire.go @@ -40,6 +40,10 @@ type voiceWiring struct { // mavsttd / mavttsd don't keep a stale conn into a restarting daemon. sttClient *worker.Client ttsClient *worker.Client + // mcp — the MCP client, nil unless the `mcp` block configures an enabled + // server (Vikunja #251). Its tools land in the same allowlist as every + // other act, so nothing else here has to know about it. + mcp *mcpWiring } // close releases the listener + worker conns. Safe to call on nil (when @@ -60,6 +64,7 @@ func (w *voiceWiring) close() { if w.ttsClient != nil { _ = w.ttsClient.Close() } + w.mcp.close() } // wireVoice builds the audio path from cfg + a CoreAPI + a router. Returns @@ -131,6 +136,14 @@ func wireVoice(cfg *config.Config, coreAPI ipc.CoreAPI, phr phraser.Phraser, mem // daemon restart. seedTools(coreAPI, cfg.Voice.Tools) exec := tool.NewExecutor(coreAPI, time.Duration(cfg.Voice.ToolTimeout)) + // MCP servers (Vikunja #251): discovery PROPOSES tools into the same + // allowlist, so an MCP tool is enabled by hand on /tools like any other and + // runs through the same confirm turn. Off unless the `mcp` block configures + // an enabled server. + w.mcp = wireMCP(cfg, dataStore) + if w.mcp != nil { + exec = exec.WithMCP(w.mcp.caller()) + } matcher := tool.NewMatcher(coreAPI) // ----- weather provider (Open-Meteo when configured, Stub otherwise) ----- diff --git a/cmd/mavweb/handlers_test.go b/cmd/mavweb/handlers_test.go index 6eef3fd..162cc9b 100644 --- a/cmd/mavweb/handlers_test.go +++ b/cmd/mavweb/handlers_test.go @@ -67,6 +67,14 @@ type fakeCore struct { // for handleChatAPI tests chatText string chatErr error + + // for the MCP section of /tools + mcpServers []ipc.MCPServerStatus + mcpErr error +} + +func (f *fakeCore) MCPServers(context.Context) ([]ipc.MCPServerStatus, error) { + return f.mcpServers, f.mcpErr } func (f *fakeCore) Chat(_ context.Context, text string) (string, error) { @@ -1118,3 +1126,49 @@ func TestHandleChatAPI_FailOpenByDefault(t *testing.T) { t.Errorf("core.Chat text = %q, want %q", core.chatText, "привет") } } + +// The MCP section renders the configured servers, and a proposal that already +// knows its cmd prefills the enable form so the argv is not retyped by hand. +func TestHandleTools_GET_MCPSection(t *testing.T) { + core := &fakeCore{ + proposed: []ipc.Tool{{ + Name: "vikunja_list_tasks", Scope: "mcp:vikunja", + Cmd: []string{"mcp", "vikunja", "list_tasks"}, Destructive: true, + Utterance: "mcp vikunja/list_tasks: List tasks in a project.", + }}, + mcpServers: []ipc.MCPServerStatus{ + {Name: "vikunja", Transport: "http", Target: "http://192.168.1.104:9100/mcp", Connected: true, Server: "vikunja 0.1.0", Tools: 4}, + {Name: "files", Transport: "stdio", Target: "mcp-server-fs /srv", Err: "start: no such file"}, + }, + } + rr := httptest.NewRecorder() + handleTools(rr, httptest.NewRequest(http.MethodGet, "/tools", nil), core, nil, false) + if rr.Code != http.StatusOK { + t.Fatalf("status = %d", rr.Code) + } + body := rr.Body.String() + for _, want := range []string{ + "MCP servers", "vikunja", "192.168.1.104:9100/mcp", "vikunja 0.1.0", + "files", "no such file", + `value="mcp vikunja list_tasks"`, // the enable form is prefilled + "checked", // and pre-marked destructive (no readOnlyHint) + } { + if !strings.Contains(body, want) { + t.Errorf("missing %q in /tools output", want) + } + } +} + +// MCP off (or an older core that does not know the method) renders the section +// empty instead of breaking the page. +func TestHandleTools_GET_MCPUnavailable(t *testing.T) { + core := &fakeCore{mcpErr: ipc.ErrNotImplemented} + rr := httptest.NewRecorder() + handleTools(rr, httptest.NewRequest(http.MethodGet, "/tools", nil), core, nil, false) + if rr.Code != http.StatusOK { + t.Fatalf("status = %d, want 200", rr.Code) + } + if !strings.Contains(rr.Body.String(), "no MCP servers configured") { + t.Error("expected the empty-state copy") + } +} diff --git a/cmd/mavweb/main.go b/cmd/mavweb/main.go index 4491084..29be4de 100644 --- a/cmd/mavweb/main.go +++ b/cmd/mavweb/main.go @@ -692,7 +692,7 @@ const toolsHTML = `{{template "shellTop" "tools"}} {{if .Msg}}
maven drafted these from acts she couldn't run. Fill the command (argv, space-separated) and enable.
+{{if .Proposed}}maven drafted these from acts she couldn't run. Fill the command (argv, space-separated) and enable. A row in an mcp: scope came from an MCP server and already knows what it calls — check the command, then enable.
| name | scope | from utterance | enable as |
|---|---|---|---|
{{.Name}} | {{.Scope}} | {{.Utterance}} | @@ -700,8 +700,8 @@ const toolsHTML = `{{template "shellTop" "tools"}} - - + +