diff --git a/cmd/mavend/smarthome.go b/cmd/mavend/smarthome.go index f4f848c..03946b2 100644 --- a/cmd/mavend/smarthome.go +++ b/cmd/mavend/smarthome.go @@ -2,6 +2,7 @@ package main import ( "context" + "fmt" "log" "strings" "time" @@ -44,9 +45,12 @@ func wireSmartHome(cfg *config.Config, st *store.Store) *homeWiring { st: st, refresh: time.Duration(cfg.SmartHome.Refresh), } - ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) - defer cancel() - w.propose(ctx) + // No first propose here. This runs inside wireVoice, inside run, before the + // IPC socket is serving, and on the locked path inside the passkey unlock + // handler. A Home Assistant box that is powered off but still on a routed + // subnet black-holes the connection rather than refusing it, so a + // synchronous enumeration held the daemon's start for the per-call timeout. + // run does the first propose off the ticker instead. return w } @@ -113,6 +117,9 @@ func (w *homeWiring) run(ctx context.Context) { } t := time.NewTicker(iv) defer t.Stop() + // The first enumeration, off the daemon's start path. wireSmartHome used to + // do it synchronously and a dead house delayed the socket coming up. + w.propose(ctx) for { select { case <-ctx.Done(): @@ -140,28 +147,54 @@ func (w *homeWiring) homeSummary(ctx context.Context) (string, bool) { } var on []string var sensors []string + dark := 0 for _, e := range ents { switch { case e.Domain == "sensor" || e.Domain == "binary_sensor": - if len(sensors) < 3 && e.State != "" && e.State != "unavailable" { + if e.State == "" || e.State == "unavailable" { + dark++ + continue + } + if len(sensors) < 3 { sensors = append(sensors, e.Name+" "+e.State+e.Unit) } + case e.State == "unavailable" || e.State == "unknown" || e.State == "": + // A lamp that is not reachable is not a lamp that is off. Counting + // it as neither used to make "всё выключено" and "one device is + // unreachable" read identically. + dark++ case e.State == "on" || e.State == "open" || e.State == "unlocked": on = append(on, e.Name) } } var parts []string - if len(on) > 0 { - if len(on) > 5 { - on = on[:5] + switch { + case len(on) > 0: + shown, rest := on, 0 + if len(shown) > 5 { + rest = len(shown) - 5 + shown = shown[:5] } - parts = append(parts, "включено: "+strings.Join(on, ", ")) - } else { + // Silent truncation on a status read is the same failure as the cap + // one layer up: she has to say the list is not the whole list. + line := "включено: " + strings.Join(shown, ", ") + if rest > 0 { + line += fmt.Sprintf(" и ещё %d", rest) + } + parts = append(parts, line) + case dark > 0 && len(sensors) == 0: + // Nothing is on and everything she can see is unreachable. "всё + // выключено" would be a claim about the house she cannot make. + return fmt.Sprintf("дом молчит: %d %s не отвечают.", dark, hostWord(dark)), true + default: parts = append(parts, "всё выключено") } if len(sensors) > 0 { parts = append(parts, strings.Join(sensors, ", ")) } + if dark > 0 { + parts = append(parts, fmt.Sprintf("%d %s не отвечают", dark, hostWord(dark))) + } return strings.Join(parts, "; ") + ".", true } diff --git a/cmd/mavend/smarthome_test.go b/cmd/mavend/smarthome_test.go index 41f07c1..d7904ae 100644 --- a/cmd/mavend/smarthome_test.go +++ b/cmd/mavend/smarthome_test.go @@ -2,6 +2,7 @@ package main import ( "context" + "fmt" "net/http" "net/http/httptest" "strings" @@ -77,6 +78,12 @@ func TestProposeOnlyProposesControllableDevices(t *testing.T) { if w == nil { t.Fatal("wireSmartHome returned nil for an enabled, reachable house") } + // Wiring alone must not have touched the house: enumeration happens off + // the ticker, not on the daemon's start path. + if pre, err := st.ListTools(context.Background(), ""); err != nil || len(pre) != 0 { + t.Fatalf("wireSmartHome enumerated the house synchronously: %+v (%v)", pre, err) + } + w.propose(context.Background()) tools, err := st.ListTools(context.Background(), "") if err != nil { @@ -188,3 +195,82 @@ func TestIsHomeQuery(t *testing.T) { } } } + +// A house that black-holes the connection must not hold the daemon's start. +// wireSmartHome used to enumerate synchronously with a 30s context, inside +// wireVoice, inside run, before the IPC socket was serving — and on the locked +// path, inside the passkey unlock handler. +func TestWireSmartHomeDoesNotBlockOnTheHouse(t *testing.T) { + // A handler that never answers: the client's own timeout is the only way + // out, and it is ten seconds. + block := make(chan struct{}) + defer close(block) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + <-block + })) + defer srv.Close() + + done := make(chan *homeWiring, 1) + go func() { + done <- wireSmartHome(&config.Config{SmartHome: &config.SmartHomeConfig{ + URL: srv.URL, Token: "t", Enabled: true, + }}, newTestStore(t)) + }() + select { + case w := <-done: + if w == nil { + t.Fatal("a configured house should still wire") + } + case <-time.After(2 * time.Second): + t.Fatal("wireSmartHome waited on the house") + } +} + +// A lamp that is unreachable is not a lamp that is off, and a list she cut +// short has to say so. Both used to read as plain statements about the house. +func TestHomeSummaryDoesNotCallUnreachableDevicesOff(t *testing.T) { + const fixture = `[ + {"entity_id":"light.a","state":"unavailable","attributes":{"friendly_name":"Прихожая"}}, + {"entity_id":"light.b","state":"unavailable","attributes":{"friendly_name":"Кухня"}} +]` + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(fixture)) + })) + defer srv.Close() + w := wireSmartHome(&config.Config{SmartHome: &config.SmartHomeConfig{ + URL: srv.URL, Token: "t", Enabled: true, + }}, newTestStore(t)) + out, ok := w.homeSummary(context.Background()) + if !ok { + t.Fatal("summary did not claim the turn") + } + if strings.Contains(out, "всё выключено") { + t.Errorf("two unreachable lamps were reported as off: %q", out) + } + if !strings.Contains(out, "не отвечают") { + t.Errorf("the unreachable devices are not mentioned: %q", out) + } +} + +func TestHomeSummarySaysWhenTheListIsCutShort(t *testing.T) { + var b strings.Builder + b.WriteString("[") + for i := 0; i < 8; i++ { + if i > 0 { + b.WriteString(",") + } + fmt.Fprintf(&b, `{"entity_id":"light.l%d","state":"on","attributes":{"friendly_name":"лампа%d"}}`, i, i) + } + b.WriteString("]") + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(b.String())) + })) + defer srv.Close() + w := wireSmartHome(&config.Config{SmartHome: &config.SmartHomeConfig{ + URL: srv.URL, Token: "t", Enabled: true, + }}, newTestStore(t)) + out, _ := w.homeSummary(context.Background()) + if !strings.Contains(out, "и ещё 3") { + t.Errorf("eight lamps on, five named, and nothing said about the rest: %q", out) + } +} diff --git a/internal/config/config.go b/internal/config/config.go index 0124e1a..4c7aa41 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -287,6 +287,12 @@ type SmartHomeConfig struct { Provider string `json:"provider,omitempty"` // URL — the instance base, "http://192.168.1.50:8123". + // + // Plain http is accepted and is what the deploy block uses. That is a + // deliberate choice, not an oversight: the instance is on the LAN behind + // wireguard, and a self-signed cert on a home box buys a warning rather + // than a guarantee. It does mean the long-lived token crosses the LAN in + // cleartext on every refresh, so the LAN is part of the trust boundary. URL string `json:"url,omitempty"` // Token — a long-lived access token. Use ${HA_TOKEN} and keep the value in @@ -294,8 +300,9 @@ type SmartHomeConfig struct { Token string `json:"token,omitempty"` // Domains — entity domains to take. Empty ⇒ the controllable domains - // (light, switch, fan, cover, lock) plus sensor and binary_sensor for - // reads. Narrow it when the instance is large: a tool name the 1.7B + // EXCEPT lock (light, switch, fan, cover) plus sensor and binary_sensor + // for reads. A lock is only enumerated when it is named here, because a + // front door is not a lamp. Narrow it when the instance is large: a tool name the 1.7B // half-remembers is a wrong act. Domains []string `json:"domains,omitempty"` @@ -306,7 +313,9 @@ type SmartHomeConfig struct { Timeout Duration `json:"timeout,omitempty"` // Refresh — how often the entity list is re-read and new devices proposed. - // 0 ⇒ 15m. Discovery is idempotent, so this only ever adds rows. + // 0 ⇒ 15m, and anything under MinSmartHomeRefresh is raised to it: + // "refresh": "1s" used to pass validation and enumerate the whole instance + // every second. Discovery is idempotent, so this only ever adds rows. Refresh Duration `json:"refresh,omitempty"` // Enabled — false (the default) keeps a written block dark, so it can be @@ -1013,6 +1022,11 @@ const DefaultEmailTimeout = 2 * time.Minute // grow a new lamp every minute. const DefaultSmartHomeRefresh = 15 * time.Minute +// MinSmartHomeRefresh — the floor under SmartHomeConfig.Refresh. Enumerating +// every entity in the house is a full /api/states read; a misconfigured second +// would hammer the instance for proposals that are idempotent anyway. +const MinSmartHomeRefresh = time.Minute + // PhraserConfig — the LLM-backed phraser seam. The daemon spawns llama-server // as a managed subprocess and sends chat-completion requests to phrase nudge // and reminder messages. nil ⇒ the template-based Stub is used instead. @@ -1250,6 +1264,9 @@ func (c *Config) applyDefaults() { if c.SmartHome != nil && c.SmartHome.Refresh <= 0 { c.SmartHome.Refresh = Duration(DefaultSmartHomeRefresh) } + if c.SmartHome != nil && c.SmartHome.Refresh < Duration(MinSmartHomeRefresh) { + c.SmartHome.Refresh = Duration(MinSmartHomeRefresh) + } // Same rule for the scanner. if c.NetScan != nil && !c.NetScan.Enabled { diff --git a/internal/smarthome/ha.go b/internal/smarthome/ha.go index 096b5a2..ec6d54c 100644 --- a/internal/smarthome/ha.go +++ b/internal/smarthome/ha.go @@ -37,7 +37,8 @@ type Config struct { // logged. Token string // Domains — the entity domains to take. Empty ⇒ every domain in the - // controllable table plus sensor/binary_sensor for reads. + // controllable table EXCEPT lock, plus sensor/binary_sensor for reads. + // A lock is only enumerated when it is named here. Domains []string // MaxEntities — 0 ⇒ DefaultMaxEntities. MaxEntities int @@ -102,7 +103,11 @@ func (c *Client) wanted(domain string) bool { return false } if _, ok := controllable[domain]; ok { - return true + // A deadbolt is a different class of object from a lamp, so "lock" is + // not in the implicit set: a bare url+token block must not auto-propose + // an unlock row for every door in the flat. Naming it in domains is the + // operator saying he meant it. + return domain != "lock" } return domain == "sensor" || domain == "binary_sensor" } @@ -118,9 +123,70 @@ type haAttrs struct { Unit string `json:"unit_of_measurement"` } +// capEntities cuts a state list to at most max entries, taking a controllable +// entity before any sensor and then round-robin across domains. +// +// The cap used to be applied to a globally id-sorted list, and entity ids sort +// by domain prefix: binary_sensor < cover < fan < light < lock < sensor < +// switch. A stock Home Assistant carries dozens of binary_sensor rows before it +// carries anything else, so forty slots went entirely to connectivity and +// update-available sensors. propose then found zero controllable entities, and +// homeSummary, reading the same list, said "всё выключено" with the lights on. +// +// The cap itself stays. The resident model is a 1.7B with a 4096-token context +// and a tool name it half-remembers is a wrong act, so a bounded deliberate +// catalogue still beats a complete one. What changes is which forty: every +// switch and light before any sensor, and an even spread inside each group so +// one crowded domain cannot starve the others. The result is sorted by id, so +// /tools reads the same across restarts. +func capEntities(all []Entity, max int) []Entity { + if max <= 0 || len(all) <= max { + sort.Slice(all, func(i, j int) bool { return all[i].ID < all[j].ID }) + return all + } + byDomain := map[string][]Entity{} + for _, e := range all { + byDomain[e.Domain] = append(byDomain[e.Domain], e) + } + var control, read []string + for d := range byDomain { + sort.Slice(byDomain[d], func(i, j int) bool { return byDomain[d][i].ID < byDomain[d][j].ID }) + if _, ok := controllable[d]; ok { + control = append(control, d) + } else { + read = append(read, d) + } + } + sort.Strings(control) + sort.Strings(read) + + out := make([]Entity, 0, max) + take := func(domains []string) { + for i := 0; len(out) < max; i++ { + took := false + for _, d := range domains { + l := byDomain[d] + if i >= len(l) || len(out) >= max { + continue + } + out = append(out, l[i]) + took = true + } + if !took { + return + } + } + } + take(control) + take(read) + sort.Slice(out, func(i, j int) bool { return out[i].ID < out[j].ID }) + return out +} + // States reads every entity Maven cares about, sorted by id and capped at // MaxEntities so the catalogue is deterministic across restarts — a proposal -// list that reshuffles itself would make /tools unreadable. +// list that reshuffles itself would make /tools unreadable. See capEntities for +// what the cap keeps. func (c *Client) States(ctx context.Context) ([]Entity, error) { body, err := c.do(ctx, http.MethodGet, "/api/states", nil) if err != nil { @@ -150,11 +216,7 @@ func (c *Client) States(ctx context.Context) ([]Entity, error) { } out = append(out, e) } - sort.Slice(out, func(i, j int) bool { return out[i].ID < out[j].ID }) - if len(out) > c.cfg.MaxEntities { - out = out[:c.cfg.MaxEntities] - } - return out, nil + return capEntities(out, c.cfg.MaxEntities), nil } // CallService performs one service call against one entity and returns a short @@ -188,9 +250,24 @@ func (c *Client) CallService(ctx context.Context, entityID, service string) (str return "", fmt.Errorf("smarthome: encode call: %w", err) } path := "/api/services/" + url.PathEscape(domain) + "/" + url.PathEscape(service) - if _, err := c.do(ctx, http.MethodPost, path, payload); err != nil { + body, err := c.do(ctx, http.MethodPost, path, payload) + if err != nil { return "", err } + // Home Assistant answers a service call with the states it changed. An + // entity that was removed since discovery, or one whose integration is + // offline, gets 200 and an empty array. Reporting "готово" for that is + // Maven asserting something false about the physical world: he says + // "выключи свет", she says done, the light stays on. + var changed []haState + if err := json.Unmarshal(body, &changed); err != nil { + // A shape we cannot read is not evidence of failure. HA has answered + // 2xx, so report the call as made rather than inventing a fault. + return "готово", nil + } + if len(changed) == 0 { + return "", fmt.Errorf("%w: %s did not change anything", ErrUnknownEntity, entityID) + } return "готово", nil } diff --git a/internal/smarthome/smarthome_test.go b/internal/smarthome/smarthome_test.go index 48347ea..34bac98 100644 --- a/internal/smarthome/smarthome_test.go +++ b/internal/smarthome/smarthome_test.go @@ -3,6 +3,7 @@ package smarthome import ( "context" "errors" + "fmt" "net/http" "net/http/httptest" "strings" @@ -97,7 +98,7 @@ func TestCallServicePostsEntityID(t *testing.T) { b := make([]byte, 256) n, _ := r.Body.Read(b) body = string(b[:n]) - _, _ = w.Write([]byte(`[]`)) + _, _ = w.Write([]byte(`[{"entity_id":"light.living_room","state":"off"}]`)) }) out, err := c.CallService(context.Background(), "light.living_room", "turn_off") if err != nil { @@ -209,3 +210,82 @@ func TestAllowlistEncoding(t *testing.T) { t.Error("DomainOf") } } + +// Home Assistant answers a service call with the states it changed, and an +// entity that has been removed or whose integration is offline gets 200 and an +// empty array. "готово" for that is Maven asserting something false about the +// physical world: he says выключи свет, she says done, the light stays on. +func TestCallServiceOnAnEntityThatChangedNothing(t *testing.T) { + c, _ := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(`[]`)) + }) + if _, err := c.CallService(context.Background(), "light.living_room", "turn_off"); !errors.Is(err, ErrUnknownEntity) { + t.Errorf("err = %v, want ErrUnknownEntity for a call that changed nothing", err) + } + // A response shape we cannot parse is not evidence of failure: HA answered + // 2xx, so the call is reported as made. + c2, _ := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(`{"result":"ok"}`)) + }) + if out, err := c2.CallService(context.Background(), "light.living_room", "turn_off"); err != nil || out != "готово" { + t.Errorf("out,err = %q,%v", out, err) + } +} + +// The cap must not be spent on sensors. Entity ids sort by domain prefix and +// binary_sensor sorts first, so a globally sorted truncation handed all forty +// slots to connectivity sensors: propose found nothing controllable and +// homeSummary said "всё выключено" with the lights on. +func TestCapKeepsControllableEntitiesFirst(t *testing.T) { + var all []Entity + for i := 0; i < 40; i++ { + all = append(all, Entity{ID: fmt.Sprintf("binary_sensor.b%02d", i), Domain: "binary_sensor", State: "off"}) + } + for i := 0; i < 30; i++ { + all = append(all, Entity{ID: fmt.Sprintf("sensor.s%02d", i), Domain: "sensor", State: "1"}) + } + for i := 0; i < 4; i++ { + all = append(all, Entity{ID: fmt.Sprintf("switch.w%d", i), Domain: "switch", State: "on"}) + } + for i := 0; i < 3; i++ { + all = append(all, Entity{ID: fmt.Sprintf("light.l%d", i), Domain: "light", State: "on"}) + } + got := capEntities(all, 40) + if len(got) != 40 { + t.Fatalf("kept %d entities, want 40", len(got)) + } + kept := map[string]int{} + for _, e := range got { + kept[e.Domain]++ + } + if kept["switch"] != 4 || kept["light"] != 3 { + t.Errorf("kept %d switches and %d lights, want all 4 and all 3: %v", kept["switch"], kept["light"], kept) + } + // The remainder still carries readable sensors, spread across both sensor + // domains rather than exhausting the one that sorts first. + if kept["sensor"] == 0 || kept["binary_sensor"] == 0 { + t.Errorf("the read domains were starved: %v", kept) + } + // Deterministic across restarts: /tools has to read the same each time. + for i := 1; i < len(got); i++ { + if got[i-1].ID >= got[i].ID { + t.Fatalf("output is not sorted by id at %d", i) + } + } +} + +// A deadbolt is a different class of object from a lamp. A bare url+token block +// must not auto-propose an unlock row for every door in the flat. +func TestLockIsNotInTheDefaultDomains(t *testing.T) { + c := NewClient(Config{URL: "http://x", Token: "t"}) + if c.wanted("lock") { + t.Error("lock is enumerated without being named in domains") + } + if !c.wanted("light") || !c.wanted("sensor") { + t.Error("the ordinary default domains were lost") + } + named := NewClient(Config{URL: "http://x", Token: "t", Domains: []string{"lock"}}) + if !named.wanted("lock") { + t.Error("lock named in domains is still not enumerated") + } +} diff --git a/internal/tool/tool.go b/internal/tool/tool.go index 4c0165d..c2ffdae 100644 --- a/internal/tool/tool.go +++ b/internal/tool/tool.go @@ -150,6 +150,15 @@ func (e *Executor) Exec(ctx context.Context, name string, args []string, confirm if e.home == nil { return "", ErrNotEnabled } + // The confirm turn on a house row is structural, not a column. The + // proposal is written destructive=true, but /tools writes the checkbox + // straight through on enable (destructive=excluded.destructive), so + // unticking it once turned home_lock_front_door_unlock into a row that + // ran on first hearing. Nothing any surface writes can remove the + // second turn from a physical device. + if !confirmed { + return "", ErrNeedsConfirm + } ctx, cancel := context.WithTimeout(ctx, e.timeout) defer cancel() return e.home.CallService(ctx, entityID, service) diff --git a/internal/tool/tool_test.go b/internal/tool/tool_test.go index cfc4c35..b6f8049 100644 --- a/internal/tool/tool_test.go +++ b/internal/tool/tool_test.go @@ -262,3 +262,33 @@ func TestExecSmartHomeRow(t *testing.T) { 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) + } +}