8d5e357b57
Second half of the MCP client: the tools the manager discovers become rows in the existing act allowlist instead of a parallel capability system. An MCP tool is encoded in the columns that already exist — cmd ["mcp",<server>,<tool>], scope mcp:<server> — so no migration, and ProposeTool/EnableTool/DisableTool, tool.Matcher and the confirm turn need no changes. One branch in Executor.Exec routes such a row to the manager instead of exec, and "mcp" is never run as a binary. Discovery only ever PROPOSES. destructive comes from the inverse of the MCP readOnlyHint, so a tool that does not promise to be read-only inherits the confirm turn, and enabling stays on /tools behind step-up. Voice args are positional and MCP args are named, so CallPositional binds only what it can defend: no required properties runs bare, and a read-only tool with exactly one required string or number gets the tail. Everything else refuses with ErrNeedsArgs rather than guessing. The read-only condition was learned against the live Vikunja server: update_task requires only task_id and takes the rest as optional, so one guessed argument blanked the fields it did not mention. A partially-filled write destroys what it omits, so a mutating tool never receives a guessed argument. Also: a read-only mcp_servers IPC method and an "MCP servers" card on /tools showing transport, target and state, with the trust level of a local target spelled out. There is deliberately no call-a-tool IPC method and no run button, so mutation keeps exactly one path. Vikunja #251
125 lines
3.9 KiB
Go
125 lines
3.9 KiB
Go
package store
|
|
|
|
import (
|
|
"context"
|
|
"errors"
|
|
"testing"
|
|
"time"
|
|
)
|
|
|
|
// TestToolLifecycle covers propose → enable → disable, the states the authed
|
|
// /tools page drives. Disable must revert an enabled tool to 'proposed' (kept
|
|
// in the store, won't run) and be idempotent.
|
|
func TestToolLifecycle(t *testing.T) {
|
|
s := newTestStore(t)
|
|
ctx := context.Background()
|
|
now := time.Now()
|
|
|
|
// Propose with empty scope → defaults to "homelab".
|
|
if _, err := s.ProposeTool(ctx, "restart_svc", "restart the service", "", now); err != nil {
|
|
t.Fatalf("propose: %v", err)
|
|
}
|
|
if err := s.EnableTool(ctx, "restart_svc", []string{"systemctl", "restart", "x"}, true, "", now); err != nil {
|
|
t.Fatalf("enable: %v", err)
|
|
}
|
|
tl, _ := s.LookupTool(ctx, "restart_svc")
|
|
if tl.Status != "enabled" {
|
|
t.Fatalf("after enable: status=%q want enabled", tl.Status)
|
|
}
|
|
if tl.Scope != "homelab" {
|
|
t.Fatalf("after enable: scope=%q want homelab", tl.Scope)
|
|
}
|
|
|
|
// Propose with explicit scope.
|
|
if _, err := s.ProposeTool(ctx, "reboot", "reboot the server", "datacenter", now); err != nil {
|
|
t.Fatalf("propose with scope: %v", err)
|
|
}
|
|
if err := s.EnableTool(ctx, "reboot", []string{"reboot"}, true, "datacenter", now); err != nil {
|
|
t.Fatalf("enable with scope: %v", err)
|
|
}
|
|
tl2, _ := s.LookupTool(ctx, "reboot")
|
|
if tl2.Scope != "datacenter" {
|
|
t.Fatalf("explicit scope: %q want datacenter", tl2.Scope)
|
|
}
|
|
|
|
if err := s.DisableTool(ctx, "restart_svc"); err != nil {
|
|
t.Fatalf("disable: %v", err)
|
|
}
|
|
tl, err := s.LookupTool(ctx, "restart_svc")
|
|
if err != nil {
|
|
t.Fatalf("lookup after disable: %v", err)
|
|
}
|
|
if tl.Status != "proposed" {
|
|
t.Fatalf("after disable: status=%q want proposed", tl.Status)
|
|
}
|
|
|
|
// idempotent: disabling an already-proposed (or absent) tool is a no-op.
|
|
if err := s.DisableTool(ctx, "restart_svc"); err != nil {
|
|
t.Fatalf("disable idempotent: %v", err)
|
|
}
|
|
if err := s.DisableTool(ctx, "does_not_exist"); err != nil {
|
|
t.Fatalf("disable absent must be no-op: %v", err)
|
|
}
|
|
}
|
|
|
|
// A discovered MCP tool arrives as a proposal that already knows its cmd, so
|
|
// enabling it is one click rather than one retyped argv.
|
|
func TestProposeMCPTool(t *testing.T) {
|
|
s := newTestStore(t)
|
|
ctx := context.Background()
|
|
now := time.Now()
|
|
cmd := []string{"mcp", "vikunja", "list_tasks"}
|
|
|
|
fresh, err := s.ProposeMCPTool(ctx, "vikunja_list_tasks", "mcp:vikunja", cmd, false, "mcp vikunja/list_tasks: List tasks", now)
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if !fresh {
|
|
t.Fatal("first proposal should be new")
|
|
}
|
|
got, err := s.LookupTool(ctx, "vikunja_list_tasks")
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if got.Status != "proposed" {
|
|
t.Fatalf("status = %q — discovery must never enable", got.Status)
|
|
}
|
|
if len(got.Cmd) != 3 || got.Cmd[0] != "mcp" || got.Cmd[2] != "list_tasks" {
|
|
t.Fatalf("cmd = %v", got.Cmd)
|
|
}
|
|
if got.Scope != "mcp:vikunja" || got.Utterance == "" {
|
|
t.Fatalf("provenance lost: %+v", got)
|
|
}
|
|
|
|
// Re-discovery on the next boot is idempotent.
|
|
fresh, err = s.ProposeMCPTool(ctx, "vikunja_list_tasks", "mcp:vikunja", cmd, true, "changed", now)
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if fresh {
|
|
t.Error("re-proposing an existing row must report nothing new")
|
|
}
|
|
|
|
// And it must not re-arm or rewrite a row a human already acted on.
|
|
if err := s.EnableTool(ctx, "vikunja_list_tasks", cmd, false, "mcp:vikunja", now); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if _, err := s.ProposeMCPTool(ctx, "vikunja_list_tasks", "mcp:vikunja", []string{"mcp", "vikunja", "delete_task"}, true, "x", now); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
got, err = s.LookupTool(ctx, "vikunja_list_tasks")
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if got.Status != "enabled" || got.Cmd[2] != "list_tasks" || got.Destructive {
|
|
t.Fatalf("an enabled row was modified by discovery: %+v", got)
|
|
}
|
|
}
|
|
|
|
func TestProposeMCPToolNeedsCmd(t *testing.T) {
|
|
s := newTestStore(t)
|
|
if _, err := s.ProposeMCPTool(context.Background(), "x", "mcp:y", nil, false, "", time.Now()); !errors.Is(err, ErrToolCmd) {
|
|
t.Fatalf("err = %v, want ErrToolCmd", err)
|
|
}
|
|
}
|