Merge branch 'fix/g10' into fix/integrated

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 {

internal/event/bus.go

@@ -95,8 +95,14 @@ func (b *Bus) Subscribe(fn func(Event)) {
 	b.subs = append(b.subs, fn)
 }
 
-// Recent returns up to limit events, newest first. limit <= 0 returns
-// everything held. Safe on a nil receiver (returns nil).
+// Recent returns up to limit events, most recently NOTICED first. limit <= 0
+// returns everything held. Safe on a nil receiver (returns nil).
+//
+// The order is the ring's insertion order, which is NoticedAt order, and it is
+// deliberately not OccurredAt order: a cold feed read publishes a week of items
+// in feed order, so sorting by when things happened would put a six-day-old
+// item above one that arrived before it. Readers must label the column
+// accordingly.
 func (b *Bus) Recent(limit int) []Event {
 	if b == nil {
 		return nil

internal/event/event.go

@@ -79,6 +79,18 @@ type Event struct {
 	// the envelope must not flatten it.
 	OccurredAt time.Time `json:"occurred_at"`
 
+	// NoticedAt — when Maven saw it. This is the ring's own ordering and the
+	// one the page sorts and labels by.
+	//
+	// It exists because OccurredAt must stay truthful and therefore cannot be
+	// an arrival order. A feed's first read returns twenty items spread over a
+	// week and publishes them in feed order; the ambient relay writes a 09:00
+	// notification about an 18:00 meeting. Ordering the journal by OccurredAt
+	// while walking the ring backwards made the timestamp column run forwards
+	// and backwards on the same page. Normalize fills it from now, always: a
+	// caller does not get to say when Maven noticed something.
+	NoticedAt time.Time `json:"noticed_at"`
+
 	// Payload — source-specific extra, opaque here. Optional.
 	Payload json.RawMessage `json:"payload,omitempty"`
 }
@@ -143,6 +155,8 @@ func (e Event) Normalize(now time.Time) Event {
 	if e.OccurredAt.IsZero() {
 		e.OccurredAt = now
 	}
+	// Notice time is not a caller's to set: it is the instant the bus took it.
+	e.NoticedAt = now
 	return e
 }
 
@@ -184,13 +198,14 @@ func truncateRunes(s string, n int) string {
 // a switch per writer: the source prefix already tells you what arrived.
 //
 // Unknown prefixes get fallback, which is what the caller was going to write
-// anyway (a WriteFact call knows it is a fact).
+// anyway (a WriteFact call knows it is a fact). There is deliberately no
+// "email:" rule: the mail path constructs its task envelope itself, so mapping
+// the prefix here would have journalled any future fact or note written under
+// an email source as a captured task.
 func SourceKind(source, fallback string) string {
 	switch {
 	case strings.HasPrefix(source, "rss:"), strings.HasPrefix(source, "crawl:"):
 		return KindNote
-	case strings.HasPrefix(source, "email:"):
-		return KindTask
 	case strings.HasPrefix(source, "probe:"), strings.HasPrefix(source, "health:"):
 		return KindHealth
 	}

internal/event/event_test.go

@@ -78,9 +78,11 @@ func TestValid(t *testing.T) {
 
 func TestSourceKind(t *testing.T) {
 	cases := map[string]string{
-		"rss:tech":      KindNote,
-		"crawl:kernel":  KindNote,
-		"email:inbox":   KindTask,
+		"rss:tech":     KindNote,
+		"crawl:kernel": KindNote,
+		// No email rule: a WriteFact under an email source is a fact, not a
+		// captured task. The mail path builds its own task envelope.
+		"email:inbox":   KindFact,
 		"probe:netdata": KindHealth,
 		"ambient:notif": KindFact,
 		"tap:voice":     KindFact,
@@ -183,3 +185,41 @@ func TestBusConcurrentPublish(t *testing.T) {
 		t.Errorf("Len = %d, want 160", b.Len())
 	}
 }
+
+// The ring is insertion-ordered and the page calls itself newest first, so the
+// two only agree if the column is notice time. A cold feed read publishes a
+// week of items in feed order, which used to make the OccurredAt column walk
+// forwards and backwards on the same page.
+func TestRecentIsOrderedByNoticeTimeNotByWhenThingsHappened(t *testing.T) {
+	b := NewBus(8)
+	// Published in feed order, six days old first, and a mark stamped now.
+	for i, e := range []Event{
+		{Source: "rss:t", Kind: KindNote, Title: "six days ago", OccurredAt: testNow.Add(-6 * 24 * time.Hour)},
+		{Source: "rss:t", Kind: KindNote, Title: "two days ago", OccurredAt: testNow.Add(-2 * 24 * time.Hour)},
+		{Source: "ambient:notif", Kind: KindFact, Title: "a meeting at six", OccurredAt: testNow.Add(9 * time.Hour)},
+	} {
+		b.Publish(e, testNow.Add(time.Duration(i)*time.Second))
+	}
+	got := b.Recent(0)
+	want := []string{"a meeting at six", "two days ago", "six days ago"}
+	for i, w := range want {
+		if got[i].Title != w {
+			t.Errorf("Recent()[%d] = %q, want %q (notice order)", i, got[i].Title, w)
+		}
+	}
+	// Notice time is monotone down the page even where occurrence time is not.
+	for i := 1; i < len(got); i++ {
+		if got[i].NoticedAt.After(got[i-1].NoticedAt) {
+			t.Errorf("NoticedAt is not descending at %d", i)
+		}
+	}
+	if got[0].OccurredAt.Before(got[1].OccurredAt) {
+		t.Fatal("this fixture is supposed to have occurrence time out of order")
+	}
+	// A caller does not get to claim when Maven noticed something.
+	forged := Event{Source: "s", Kind: KindFact, Title: "t", NoticedAt: testNow.Add(100 * time.Hour)}
+	b.Publish(forged, testNow)
+	if n := b.Recent(1)[0].NoticedAt; !n.Equal(testNow) {
+		t.Errorf("NoticedAt = %v, want the publish instant %v", n, testNow)
+	}
+}

internal/ipc/api.go

@@ -740,6 +740,10 @@ type IntakeEvent struct {
 	Body       string    `json:"body,omitempty"`
 	Priority   string    `json:"priority"`
 	OccurredAt time.Time `json:"occurred_at"`
+	// NoticedAt — when the journal took it. This is the order the ring returns
+	// and the one a page must sort and label by; OccurredAt is when the thing
+	// happened, which for a cold feed read is a week before it arrived.
+	NoticedAt time.Time `json:"noticed_at"`
 }
 
 // --- Rule trace / explanation DTOs ---

internal/netscan/netscan.go

@@ -46,8 +46,16 @@ const (
 	// DefaultTimeout — per-connection budget. Short: on a LAN a live host
 	// answers in single-digit milliseconds, and a filtered port never answers.
 	DefaultTimeout = 400 * time.Millisecond
-	// DefaultRate — connections per second across the whole scan.
-	DefaultRate = 50
+	// DefaultRate — connections per second across the whole scan. A default
+	// /24 at four ports is 1016 probes, so this is also what decides whether
+	// the shipped configuration fits inside the caller's budget: at 100/s it
+	// takes about ten seconds. Lowering it means a truncated scan, which is
+	// reported rather than hidden, but it is still a worse answer.
+	DefaultRate = 100
+	// MaxRate — the highest configurable rate. The dial loop floors the ticker
+	// interval at a millisecond, so anything above this was already a lie; say
+	// so at config load instead of silently clamping.
+	MaxRate = 1000
 	// DefaultMaxHosts — cap on addresses probed in one scan.
 	DefaultMaxHosts = 256
 	// MaxPrefixHosts — the largest CIDR that may be configured, in addresses.
@@ -127,6 +135,9 @@ func Validate(c Config) error {
 	if c.Rate < 0 || c.MaxHosts < 0 || c.Timeout < 0 {
 		return errors.New("netscan: rate, max_hosts and timeout must not be negative")
 	}
+	if c.Rate > MaxRate {
+		return fmt.Errorf("netscan: rate %d is above the ceiling of %d connections per second", c.Rate, MaxRate)
+	}
 	return nil
 }
 
@@ -177,43 +188,89 @@ func New(cfg Config) *Scanner {
 	return &Scanner{cfg: cfg, dial: dialTCP, arp: readARP}
 }
 
-// targets expands the configured subnets into addresses, skipping the network
-// and broadcast address of each, capped at MaxHosts. Deterministic order, so
-// two scans of an unchanged network read the same.
-func (s *Scanner) targets() []netip.Addr {
+// expand lists the scannable addresses of one CIDR, skipping the network and
+// broadcast address.
+func expand(cidr string) []netip.Addr {
+	p, err := netip.ParsePrefix(strings.TrimSpace(cidr))
+	if err != nil {
+		return nil
+	}
+	p = p.Masked()
+	first := p.Addr()
 	var out []netip.Addr
-	for _, cidr := range s.cfg.Subnets {
-		p, err := netip.ParsePrefix(strings.TrimSpace(cidr))
-		if err != nil {
+	for a := first; p.Contains(a); a = a.Next() {
+		// Skip the network address; the broadcast address is skipped by
+		// looking one ahead.
+		if a == first && p.Bits() < 31 {
 			continue
 		}
-		p = p.Masked()
-		first := p.Addr()
-		for a := first; p.Contains(a); a = a.Next() {
-			if len(out) >= s.cfg.MaxHosts {
-				return out
-			}
-			// Skip the network address; the broadcast address is skipped by
-			// looking one ahead.
-			if a == first && p.Bits() < 31 {
-				continue
-			}
-			if p.Bits() < 31 && !p.Contains(a.Next()) {
-				continue
-			}
-			out = append(out, a)
+		if p.Bits() < 31 && !p.Contains(a.Next()) {
+			continue
 		}
+		out = append(out, a)
 	}
 	return out
 }
 
+// targets expands the configured subnets into addresses, capped at MaxHosts,
+// and reports whether the cap cut anything off. Deterministic order, so two
+// scans of an unchanged network read the same.
+//
+// The subnets are taken round-robin rather than in order. Consuming MaxHosts
+// from the first subnet used to leave a second configured LAN 99% unprobed,
+// with nothing logged: he named two ranges and got an answer about one.
+// Round-robin spends the budget evenly, so every named range is represented
+// and the shortfall is reported instead.
+func (s *Scanner) targets() ([]netip.Addr, bool) {
+	lists := make([][]netip.Addr, 0, len(s.cfg.Subnets))
+	total := 0
+	for _, cidr := range s.cfg.Subnets {
+		l := expand(cidr)
+		if len(l) == 0 {
+			continue
+		}
+		lists = append(lists, l)
+		total += len(l)
+	}
+	out := make([]netip.Addr, 0, min(total, s.cfg.MaxHosts))
+	for i := 0; len(out) < s.cfg.MaxHosts; i++ {
+		took := false
+		for _, l := range lists {
+			if i >= len(l) {
+				continue
+			}
+			if len(out) >= s.cfg.MaxHosts {
+				break
+			}
+			out = append(out, l[i])
+			took = true
+		}
+		if !took {
+			break
+		}
+	}
+	return out, len(out) < total
+}
+
+// Result is one scan's outcome.
+type Result struct {
+	// Hosts — what answered, ascending by address.
+	Hosts []Host
+	// Truncated — the scan did not cover every configured address, because
+	// MaxHosts cut the target list or the caller's context expired mid-run.
+	// Callers MUST NOT present a truncated result as the state of the network:
+	// "нашла 6 устройств" is a claim about the LAN, and a scan that stopped at
+	// .238 has not earned it.
+	Truncated bool
+}
+
 // Scan probes every configured address and returns the hosts that answered.
 //
 // It takes no target: the range is the configured one, always. Callers pass a
 // context and nothing else, which is the point — see the package comment.
-func (s *Scanner) Scan(ctx context.Context) ([]Host, error) {
+func (s *Scanner) Scan(ctx context.Context) (Result, error) {
 	if len(s.cfg.Subnets) == 0 {
-		return nil, ErrNoSubnets
+		return Result{}, ErrNoSubnets
 	}
 	arp, err := s.arp()
 	if err != nil {
@@ -233,8 +290,13 @@ func (s *Scanner) Scan(ctx context.Context) ([]Host, error) {
 		addr  string
 		ports []int
 	}
-	targets := s.targets()
-	results := make(chan result, len(targets))
+	targets, truncated := s.targets()
+	// One slot per PROBE, not per host: a worker sends once per open port, so
+	// a subnet with more open ports than addresses used to fill a host-sized
+	// buffer and wedge. Nothing drains this channel until wg.Wait returns, and
+	// the sends carry no select on ctx.Done, so that was a permanent hang of
+	// the calling turn plus a leak of every worker.
+	results := make(chan result, len(targets)*len(s.cfg.Ports))
 	sem := make(chan struct{}, maxParallel)
 	var wg sync.WaitGroup
 
@@ -247,10 +309,12 @@ scan:
 			// sometimes win over an already-canceled context and let one more
 			// probe out.
 			if ctx.Err() != nil {
+				truncated = true
 				break scan
 			}
 			select {
 			case <-ctx.Done():
+				truncated = true
 				break scan
 			case <-tick.C:
 			}
@@ -294,6 +358,9 @@ scan:
 	out := make([]Host, 0, len(byAddr))
 	for _, h := range byAddr {
 		sort.Ints(h.Ports)
+		if !h.Up() {
+			continue
+		}
 		out = append(out, *h)
 	}
 	sort.Slice(out, func(i, j int) bool {
@@ -301,7 +368,7 @@ scan:
 		aj, _ := netip.ParseAddr(out[j].Addr)
 		return ai.Less(aj)
 	})
-	return out, nil
+	return Result{Hosts: out, Truncated: truncated}, nil
 }
 
 func itoa(n int) string { return fmt.Sprintf("%d", n) }

internal/netscan/netscan_test.go

@@ -35,6 +35,7 @@ func TestValidateBounds(t *testing.T) {
 		"bad port":                {Subnets: []string{"192.168.1.0/24"}, Ports: []int{0}},
 		"huge port":               {Subnets: []string{"192.168.1.0/24"}, Ports: []int{70000}},
 		"negative rate":           {Subnets: []string{"192.168.1.0/24"}, Rate: -1},
+		"rate past the ceiling":   {Subnets: []string{"192.168.1.0/24"}, Rate: MaxRate + 1},
 	}
 	for name, c := range bad {
 		if err := Validate(c); err == nil {
@@ -63,10 +64,11 @@ func TestScanOnlyTouchesConfiguredSubnet(t *testing.T) {
 	}
 	s.arp = func() (map[string]string, error) { return map[string]string{}, nil }
 
-	hosts, err := s.Scan(context.Background())
+	res, err := s.Scan(context.Background())
 	if err != nil {
 		t.Fatalf("Scan: %v", err)
 	}
+	hosts := res.Hosts
 	if len(hosts) != 1 || hosts[0].Addr != "192.168.9.3" || len(hosts[0].Ports) != 1 {
 		t.Fatalf("hosts = %+v", hosts)
 	}
@@ -144,10 +146,11 @@ func TestARPFillsMACWithinTheConfiguredRangeOnly(t *testing.T) {
 			"10.9.9.9":    "11:22:33:44:55:66",
 		}, nil
 	}
-	hosts, err := s.Scan(context.Background())
+	res, err := s.Scan(context.Background())
 	if err != nil {
 		t.Fatal(err)
 	}
+	hosts := res.Hosts
 	if len(hosts) != 1 {
 		t.Fatalf("hosts = %+v", hosts)
 	}
@@ -196,3 +199,96 @@ func TestNewAppliesDefaults(t *testing.T) {
 		t.Error("New aliased DefaultPorts")
 	}
 }
+
+// A dense subnet must not wedge the scan. The results channel used to be sized
+// by the number of HOSTS while a worker sends once per open PORT, so a range
+// where the open ports outnumber the addresses filled the buffer, blocked a
+// worker inside wg.Wait, and hung Scan forever. Nothing drains the channel
+// before wg.Wait returns and the sends carry no ctx.Done case, so the caller's
+// deadline did not rescue it either.
+//
+// Six addresses, eight ports, everything open: 48 sends against a buffer that
+// used to hold 6. Against the old code this test does not fail, it hangs, so
+// the scan runs on its own goroutine with a deadline around it.
+func TestScanDoesNotWedgeWhenPortsOutnumberHosts(t *testing.T) {
+	ports := []int{22, 80, 443, 8080, 8443, 9000, 9100, 9200}
+	s := New(Config{Subnets: []string{"192.168.9.0/29"}, Ports: ports, Rate: MaxRate})
+	s.dial = func(context.Context, string, time.Duration) bool { return true }
+	s.arp = func() (map[string]string, error) { return nil, nil }
+
+	done := make(chan Result, 1)
+	go func() {
+		res, err := s.Scan(context.Background())
+		if err != nil {
+			t.Error(err)
+		}
+		done <- res
+	}()
+	select {
+	case res := <-done:
+		if len(res.Hosts) != 6 {
+			t.Fatalf("hosts = %d, want 6: %+v", len(res.Hosts), res.Hosts)
+		}
+		for _, h := range res.Hosts {
+			if len(h.Ports) != len(ports) {
+				t.Errorf("%s reported %d open ports, want %d", h.Addr, len(h.Ports), len(ports))
+			}
+		}
+	case <-time.After(10 * time.Second):
+		t.Fatal("Scan did not return: the results channel is sized by hosts, not by probes")
+	}
+}
+
+// MaxHosts is spent evenly across the configured subnets. Taking it in order
+// meant a second configured LAN got whatever the first left over, which for a
+// pair of /24s under the default cap was two addresses out of 254.
+func TestTargetsSpreadAcrossSubnets(t *testing.T) {
+	s := New(Config{Subnets: []string{"192.168.1.0/24", "192.168.2.0/24"}, MaxHosts: 20})
+	targets, truncated := s.targets()
+	if !truncated {
+		t.Error("508 addresses under a cap of 20 is a truncated target list")
+	}
+	if len(targets) != 20 {
+		t.Fatalf("targets = %d, want 20", len(targets))
+	}
+	var first, second int
+	for _, a := range targets {
+		switch {
+		case netip.MustParsePrefix("192.168.1.0/24").Contains(a):
+			first++
+		case netip.MustParsePrefix("192.168.2.0/24").Contains(a):
+			second++
+		}
+	}
+	if first != 10 || second != 10 {
+		t.Errorf("split %d/%d across the two subnets, want 10/10", first, second)
+	}
+}
+
+// A run cut short by the caller's deadline reports itself as truncated, so the
+// spoken answer can stop claiming to describe the whole network.
+func TestScanReportsTruncation(t *testing.T) {
+	s := New(Config{Subnets: []string{"192.168.9.0/24"}, Ports: []int{80}, Rate: 200})
+	s.dial = func(context.Context, string, time.Duration) bool { return false }
+	s.arp = func() (map[string]string, error) { return nil, nil }
+	ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond)
+	defer cancel()
+	res, err := s.Scan(ctx)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !res.Truncated {
+		t.Error("a scan stopped by the deadline must report Truncated")
+	}
+
+	full := New(Config{Subnets: []string{"192.168.9.0/29"}, Ports: []int{80}, Rate: MaxRate})
+	full.dial = func(context.Context, string, time.Duration) bool { return false }
+	full.arp = func() (map[string]string, error) { return nil, nil }
+	res, err = full.Scan(context.Background())
+	if err != nil {
+		t.Fatal(err)
+	}
+	if res.Truncated {
+		t.Error("a scan that covered every configured address is not truncated")
+	}
+}

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
 }
 

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")
+	}
+}

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)

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)
+	}
+}