Files
Maven/internal/tool/tool.go
kami 8d5e357b57 Expose discovered MCP tools through the act allowlist (#251)
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
2026-08-01 04:36:40 +04:00

172 lines
6.3 KiB
Go

// Package tool is maven's act executor: it runs the ENABLED tools from the
// store's allowlist, and drafts 'proposed' scaffolds for acts that aren't on
// it yet.
//
// Boundary discipline (DESIGN.md § "Tool registration — drafting is suggest,
// enabling is act"):
//
// - The store is the allowlist. Only status='enabled' rows run. A verb not
// on it → refuse ("not on the list → refuse, don't improvise") and draft a
// 'proposed' scaffold instead. Enabling a proposal is a human act on an
// authed surface (mavweb), gated at AuthStepUp — never the voice path, so
// a compromised router can't grant itself a capability.
// - Args are passed as argv, NEVER through a shell. STT text lands as
// positional arguments to Cmd; there is no `sh -c`, so "restart nginx;
// rm -rf" can't inject — the tail is one argv element to the named binary.
// - An enabled row whose cmd is ["mcp", "<server>", "<tool>"] is a call to a
// configured MCP server instead of a process (Vikunja #251). It goes
// through every rule above unchanged — enabled, and confirmed if it
// mutates — because the store is still the allowlist; only the dispatch at
// the bottom of Exec differs.
// - Destructive tools don't run on first hearing: Exec returns ErrNeedsConfirm
// and the handler runs a confirm turn ("выполнить X? да/нет"); only a
// confirmed re-Exec runs them. A gate assumes a fully-formed action, which
// an enabled+matched act is (DESIGN.md § "Confirmation is not one
// mechanism").
package tool
import (
"bytes"
"context"
"errors"
"fmt"
"log"
"os/exec"
"strings"
"time"
"github.com/kami/maven/internal/ipc"
"github.com/kami/maven/internal/mcp"
"github.com/kami/maven/internal/router"
)
// API — the narrow slice of ipc.CoreAPI the executor and matcher need. Backed
// in-process by the daemon's store adapter (a direct sqlite query per call —
// acts are rare, personal-scale; no cache).
type API interface {
LookupTool(ctx context.Context, name string) (ipc.Tool, error)
ListTools(ctx context.Context, status string) ([]ipc.Tool, error)
ProposeTool(ctx context.Context, name, utterance, scope string, ts time.Time) (bool, error)
}
var (
// ErrNotEnabled — the fn isn't an enabled tool (absent, or still proposed).
ErrNotEnabled = errors.New("tool not on the enabled allowlist")
// ErrNeedsConfirm — the fn is enabled but destructive; needs a confirm turn.
ErrNeedsConfirm = errors.New("destructive tool needs confirmation")
)
// MCPCaller is the seam for an act that is an MCP tool call rather than a
// process (Vikunja #251). internal/mcp.Manager satisfies it via CallPositional.
// nil ⇒ MCP is not configured, and an MCP row refuses to run rather than
// silently doing nothing.
type MCPCaller interface {
CallPositional(ctx context.Context, server, tool string, args []string) (string, error)
}
// Executor runs enabled tools. run is the exec seam (default: real process);
// tests swap it. timeout bounds each invocation.
type Executor struct {
api API
timeout time.Duration
run func(ctx context.Context, argv []string) (string, error)
mcp MCPCaller
}
// NewExecutor builds the executor. timeout<=0 defaults to 30s.
func NewExecutor(api API, timeout time.Duration) *Executor {
if timeout <= 0 {
timeout = 30 * time.Second
}
return &Executor{api: api, timeout: timeout, run: runProcess}
}
// WithMCP attaches the MCP caller. Called once at wiring time when the mcp
// config block is present; without it, a row whose cmd is ["mcp", …] refuses.
func (e *Executor) WithMCP(m MCPCaller) *Executor {
e.mcp = m
return e
}
// Exec looks up name in the store and runs Cmd+args as argv (no shell).
// confirmed=true is the second turn of a destructive act (the user said "да");
// it bypasses the ErrNeedsConfirm gate. Non-enabled ⇒ ErrNotEnabled; a
// destructive tool with confirmed=false ⇒ ErrNeedsConfirm.
func (e *Executor) Exec(ctx context.Context, name string, args []string, confirmed bool) (string, error) {
t, err := e.api.LookupTool(ctx, name)
if errors.Is(err, ipc.ErrToolNotFound) {
return "", ErrNotEnabled
}
if err != nil {
return "", err
}
if t.Status != "enabled" {
return "", ErrNotEnabled
}
if t.Destructive && !confirmed {
return "", ErrNeedsConfirm
}
// An MCP row is a call to a configured server, not a process. Everything
// above still applied: it had to be enabled, and a mutating one had to be
// confirmed. Only the dispatch differs.
if server, remote, ok := mcp.ParseCmd(t.Cmd); ok {
if e.mcp == nil {
return "", ErrNotEnabled
}
ctx, cancel := context.WithTimeout(ctx, e.timeout)
defer cancel()
return e.mcp.CallPositional(ctx, server, remote, args)
}
argv := append(append([]string(nil), t.Cmd...), args...)
if len(argv) == 0 {
return "", ErrNotEnabled
}
ctx, cancel := context.WithTimeout(ctx, e.timeout)
defer cancel()
return e.run(ctx, argv)
}
func runProcess(ctx context.Context, argv []string) (string, error) {
cmd := exec.CommandContext(ctx, argv[0], argv[1:]...)
var buf bytes.Buffer
cmd.Stdout = &buf
cmd.Stderr = &buf
err := cmd.Run()
out := strings.TrimSpace(buf.String())
if err != nil {
return out, fmt.Errorf("run %v: %w", argv, err)
}
return out, nil
}
// Matcher — a router.ActMatcher whose allowlist is the live set of enabled
// tool names (one source of truth with the executor). Match delegates to the
// router's default prefix logic over the current names. The interface's Match
// has no ctx, so it queries with a background context — an in-process sqlite
// read on the daemon.
type Matcher struct{ api API }
// NewMatcher builds a store-backed act matcher.
func NewMatcher(api API) *Matcher { return &Matcher{api: api} }
func (m *Matcher) names() []string {
ts, err := m.api.ListTools(context.Background(), "enabled")
if err != nil {
log.Printf("tool: list enabled tools: %v", err)
return nil
}
names := make([]string, len(ts))
for i, t := range ts {
names[i] = t.Name
}
return names
}
// Allowlist — the enabled verbs (for stage-0 grammar wiring / introspection).
func (m *Matcher) Allowlist() []string { return m.names() }
// Match — longest-verb-first prefix match over the live enabled allowlist.
func (m *Matcher) Match(utterance string) (string, []string, bool) {
return router.DefaultActMatcher{Fns: m.names()}.Match(utterance)
}