Files
Maven/internal/tool/tool_test.go
T
kami d62ba093f5 smarthome: keep the devices under the cap, and stop asserting what the house did not do
States sorted every entity by id and cut at MaxEntities. Entity ids sort
by domain prefix, so binary_sensor came first and forty slots went to
connectivity and update-available rows: propose found nothing
controllable, and homeSummary, reading the same list, said everything
was off with the lights on. The cap stays, because a tool name the 1.7B
half-remembers is a wrong act. What changes is which forty. Controllable
domains are taken first and round-robin, so every switch and light is in
before any sensor.

CallService reported done for a call that changed nothing. Home
Assistant answers a service call with the states it changed, and a
removed entity or an offline integration gets 200 and an empty array.
That is the one place Maven asserts something about the physical world,
so an empty array is now ErrUnknownEntity.

The confirm turn on a house row was a column, not an invariant. The
proposal is destructive, but /tools writes the checkbox through on
enable, so unticking it once made an unlock row that ran on first
hearing. Exec now demands the second turn for any smarthome row whatever
the column says, and lock is out of the default domain set so a bare
block does not propose an unlock for every door.

wireSmartHome enumerated the house synchronously, inside wireVoice,
before the socket was serving and inside the unlock handler. A box that
black-holes the connection held the daemon's start for the per-call
timeout. The first propose moved onto the ticker goroutine.

Smaller: an unreachable lamp is counted apart from an off one, a
truncated on-list says how many it left out, refresh has a floor of a
minute, and the http url is documented as a deliberate wg-only choice.
Found in review of #80.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TrVSBKe3RFDF4fGYKWYQnX
2026-08-01 14:06:38 +04:00

295 lines
10 KiB
Go

package tool
import (
"context"
"errors"
"reflect"
"testing"
"time"
"github.com/kami/maven/internal/ipc"
)
// fakeAPI — an in-memory tool store for the executor/matcher tests.
type fakeAPI struct{ tools map[string]ipc.Tool }
func (f fakeAPI) LookupTool(_ context.Context, name string) (ipc.Tool, error) {
t, ok := f.tools[name]
if !ok {
return ipc.Tool{}, ipc.ErrToolNotFound
}
return t, nil
}
func (f fakeAPI) ListTools(_ context.Context, status string) ([]ipc.Tool, error) {
var out []ipc.Tool
for _, t := range f.tools {
if status == "" || t.Status == status {
out = append(out, t)
}
}
return out, nil
}
func (f fakeAPI) ProposeTool(_ context.Context, _, _, _ string, _ time.Time) (bool, error) {
return true, nil
}
// TestExec covers the allowlist boundary: enabled runs, unknown/proposed refuse,
// destructive needs confirm, and args land as argv (no shell) after the prefix.
func TestExec(t *testing.T) {
api := fakeAPI{tools: map[string]ipc.Tool{
"restart": {Name: "restart", Scope: "homelab", Cmd: []string{"systemctl", "restart"}, Status: "enabled"},
"drop": {Name: "drop", Scope: "homelab", Cmd: []string{"dropdb"}, Destructive: true, Status: "enabled"},
"draft": {Name: "draft", Scope: "homelab", Cmd: []string{"x"}, Status: "proposed"},
}}
var gotArgv []string
e := NewExecutor(api, 0)
e.run = func(_ context.Context, argv []string) (string, error) { gotArgv = argv; return "ok", nil }
// enabled → runs, args appended to the fixed prefix as argv.
out, err := e.Exec(context.Background(), "restart", []string{"nginx; rm -rf /"}, false)
if err != nil || out != "ok" {
t.Fatalf("enabled: out=%q err=%v", out, err)
}
want := []string{"systemctl", "restart", "nginx; rm -rf /"}
if !reflect.DeepEqual(gotArgv, want) {
t.Fatalf("argv=%v want %v (injection must stay one argv element)", gotArgv, want)
}
// unknown → refuse.
if _, err := e.Exec(context.Background(), "nope", nil, false); !errors.Is(err, ErrNotEnabled) {
t.Fatalf("unknown: err=%v want ErrNotEnabled", err)
}
// proposed (not enabled) → refuse.
if _, err := e.Exec(context.Background(), "draft", nil, false); !errors.Is(err, ErrNotEnabled) {
t.Fatalf("proposed: err=%v want ErrNotEnabled", err)
}
// destructive unconfirmed → needs confirm; confirmed → runs.
if _, err := e.Exec(context.Background(), "drop", nil, false); !errors.Is(err, ErrNeedsConfirm) {
t.Fatalf("destructive: err=%v want ErrNeedsConfirm", err)
}
if _, err := e.Exec(context.Background(), "drop", []string{"db"}, true); err != nil {
t.Fatalf("destructive confirmed: err=%v", err)
}
if want := []string{"dropdb", "db"}; !reflect.DeepEqual(gotArgv, want) {
t.Fatalf("confirmed argv=%v want %v", gotArgv, want)
}
// matcher allowlist = enabled names only (proposed excluded).
m := NewMatcher(api)
fn, args, ok := m.Match("restart nginx")
if !ok || fn != "restart" || !reflect.DeepEqual(args, []string{"nginx"}) {
t.Fatalf("match: fn=%q args=%v ok=%v", fn, args, ok)
}
if _, _, ok := m.Match("draft something"); ok {
t.Fatal("proposed tool must not match (not enabled)")
}
}
// fakeMCP records what the executor asked it to call.
type fakeMCP struct {
server, tool string
args []string
out string
err error
calls int
}
func (f *fakeMCP) CallPositional(_ context.Context, server, tool string, args []string) (string, error) {
f.calls++
f.server, f.tool, f.args = server, tool, args
return f.out, f.err
}
// An MCP row dispatches to the caller instead of a process, and the process
// seam is never touched.
func TestExecMCPRowDispatchesToMCP(t *testing.T) {
api := fakeAPI{tools: map[string]ipc.Tool{
"vikunja_list_tasks": {
Name: "vikunja_list_tasks", Status: "enabled", Scope: "mcp:vikunja",
Cmd: []string{"mcp", "vikunja", "list_tasks"},
},
}}
m := &fakeMCP{out: "две задачи"}
ran := false
e := NewExecutor(api, time.Second).WithMCP(m)
e.run = func(context.Context, []string) (string, error) { ran = true; return "", nil }
out, err := e.Exec(context.Background(), "vikunja_list_tasks", []string{"мавен"}, false)
if err != nil {
t.Fatalf("exec: %v", err)
}
if out != "две задачи" {
t.Fatalf("out = %q", out)
}
if ran {
t.Fatal("an MCP row must not be executed as a process")
}
if m.server != "vikunja" || m.tool != "list_tasks" || len(m.args) != 1 || m.args[0] != "мавен" {
t.Fatalf("dispatched wrong: %+v", m)
}
}
// The allowlist rules still apply to an MCP row: destructive means a confirm
// turn first, and nothing is called until the second turn.
func TestExecMCPRowStillNeedsConfirm(t *testing.T) {
api := fakeAPI{tools: map[string]ipc.Tool{
"vikunja_delete_task": {
Name: "vikunja_delete_task", Status: "enabled", Destructive: true,
Cmd: []string{"mcp", "vikunja", "delete_task"},
},
}}
m := &fakeMCP{out: "удалила"}
e := NewExecutor(api, time.Second).WithMCP(m)
if _, err := e.Exec(context.Background(), "vikunja_delete_task", nil, false); !errors.Is(err, ErrNeedsConfirm) {
t.Fatalf("err = %v, want ErrNeedsConfirm", err)
}
if m.calls != 0 {
t.Fatal("a destructive MCP tool must not reach the server before confirmation")
}
if _, err := e.Exec(context.Background(), "vikunja_delete_task", nil, true); err != nil {
t.Fatalf("confirmed exec: %v", err)
}
if m.calls != 1 {
t.Fatalf("calls = %d", m.calls)
}
}
// A proposed MCP row does not run, exactly like a proposed shell tool.
func TestExecMCPRowNotEnabled(t *testing.T) {
api := fakeAPI{tools: map[string]ipc.Tool{
"vikunja_list_tasks": {Name: "vikunja_list_tasks", Status: "proposed", Cmd: []string{"mcp", "vikunja", "list_tasks"}},
}}
m := &fakeMCP{}
e := NewExecutor(api, time.Second).WithMCP(m)
if _, err := e.Exec(context.Background(), "vikunja_list_tasks", nil, false); !errors.Is(err, ErrNotEnabled) {
t.Fatalf("err = %v", err)
}
if m.calls != 0 {
t.Fatal("a proposal must not call anything")
}
}
// With MCP unconfigured, an MCP row refuses rather than trying to exec "mcp".
func TestExecMCPRowWithoutCallerRefuses(t *testing.T) {
api := fakeAPI{tools: map[string]ipc.Tool{
"vikunja_list_tasks": {Name: "vikunja_list_tasks", Status: "enabled", Cmd: []string{"mcp", "vikunja", "list_tasks"}},
}}
ran := false
e := NewExecutor(api, time.Second)
e.run = func(context.Context, []string) (string, error) { ran = true; return "", nil }
if _, err := e.Exec(context.Background(), "vikunja_list_tasks", nil, false); !errors.Is(err, ErrNotEnabled) {
t.Fatalf("err = %v, want ErrNotEnabled", err)
}
if ran {
t.Fatal(`"mcp" must never be run as a binary`)
}
}
// fakeHome records what the executor asked the house to do.
type fakeHome struct {
entity, service string
calls int
}
func (f *fakeHome) CallService(_ context.Context, entityID, service string) (string, error) {
f.calls++
f.entity, f.service = entityID, service
return "готово", nil
}
// A house row goes through the same allowlist and the same confirm turn as any
// other act, and it is never exec'd as a binary (Vikunja #256).
func TestExecSmartHomeRow(t *testing.T) {
api := fakeAPI{tools: map[string]ipc.Tool{
"home_light_x_off": {
Name: "home_light_x_off", Scope: "smarthome:light",
Cmd: []string{"smarthome", "light.x", "turn_off"}, Destructive: true, Status: "enabled",
},
"home_draft": {
Name: "home_draft", Scope: "smarthome:light",
Cmd: []string{"smarthome", "light.y", "turn_on"}, Destructive: true, Status: "proposed",
},
}}
ran := false
newExec := func(h HomeCaller) *Executor {
e := NewExecutor(api, time.Second)
e.run = func(context.Context, []string) (string, error) { ran = true; return "", nil }
if h != nil {
e = e.WithHome(h)
}
return e
}
// No house configured ⇒ the row refuses rather than being exec'd.
if _, err := newExec(nil).Exec(context.Background(), "home_light_x_off", nil, true); !errors.Is(err, ErrNotEnabled) {
t.Fatalf("unconfigured house: err = %v, want ErrNotEnabled", err)
}
if ran {
t.Fatal(`"smarthome" was run as a binary`)
}
// Configured, but not confirmed ⇒ the confirm turn, before any call.
fh := &fakeHome{}
if _, err := newExec(fh).Exec(context.Background(), "home_light_x_off", nil, false); !errors.Is(err, ErrNeedsConfirm) {
t.Fatalf("err = %v, want ErrNeedsConfirm", err)
}
if fh.calls != 0 {
t.Fatal("an unconfirmed house act reached the house")
}
// A merely proposed row never runs, confirmed or not.
if _, err := newExec(fh).Exec(context.Background(), "home_draft", nil, true); !errors.Is(err, ErrNotEnabled) {
t.Fatalf("proposed row: err = %v, want ErrNotEnabled", err)
}
if fh.calls != 0 {
t.Fatal("a proposed house row reached the house")
}
// Confirmed ⇒ the service call, with the entity from the ROW and the
// spoken tail dropped.
out, err := newExec(fh).Exec(context.Background(), "home_light_x_off", []string{"light.somewhere_else"}, true)
if err != nil {
t.Fatalf("Exec: %v", err)
}
if out != "готово" {
t.Errorf("out = %q", out)
}
if fh.entity != "light.x" || fh.service != "turn_off" {
t.Errorf("called %s/%s: the target must come from the enabled row, never from the utterance", fh.entity, fh.service)
}
if ran {
t.Fatal(`"smarthome" was run as a binary`)
}
}
// The confirm turn on a house row survives the destructive column being wrong.
// ProposeSmartHomeTool writes destructive=true, but /tools reads the checkbox
// from the form and EnableTool writes destructive=excluded.destructive, so
// unticking it once turned home_lock_front_door_unlock into a row that opened
// the front door on first hearing. The guarantee has to be structural.
func TestExecSmartHomeRowConfirmsEvenWhenNotMarkedDestructive(t *testing.T) {
api := fakeAPI{tools: map[string]ipc.Tool{
"home_lock_front_door_unlock": {
Name: "home_lock_front_door_unlock", Scope: "smarthome:lock",
Cmd: []string{"smarthome", "lock.front_door", "unlock"},
// The column Kami unticked on /tools.
Destructive: false, Status: "enabled",
},
}}
fh := &fakeHome{}
e := NewExecutor(api, time.Second).WithHome(fh)
if _, err := e.Exec(context.Background(), "home_lock_front_door_unlock", nil, false); !errors.Is(err, ErrNeedsConfirm) {
t.Fatalf("err = %v, want ErrNeedsConfirm", err)
}
if fh.calls != 0 {
t.Fatal("the front door was unlocked without a confirm turn")
}
if _, err := e.Exec(context.Background(), "home_lock_front_door_unlock", nil, true); err != nil {
t.Fatalf("confirmed: %v", err)
}
if fh.calls != 1 {
t.Fatalf("calls = %d, want 1 after the confirm turn", fh.calls)
}
}