Read and control the house through Home Assistant (#256) #80

Closed
claude wants to merge 1 commits from overnight/smarthome into overnight/replay-simulator
Contributor

Read and control the house through Home Assistant (#256)

A smarthome block points Maven at a Home Assistant instance. She reads its
entity states to answer "что включено дома?", and every controllable device
becomes a PROPOSED row in the existing act allowlist — cmd
["smarthome",<entity_id>,], scope smarthome: — so nothing new
had to be invented for the mutating half. ProposeTool/EnableTool/DisableTool,
tool.Matcher and the confirm turn are untouched; one branch in Executor.Exec
routes such a row to the client instead of exec, and "smarthome" is never run as
a binary. This is the same trick overnight/mcp-tools used for #251, on purpose.

Discovery only ever PROPOSES, and every control row is destructive=true: there
is no read-only way to turn the heating off, so flipping something in his flat
always costs a confirm turn and always had to be enabled by hand on /tools,
behind step-up.

The entity and the service come from the row he enabled, never from the
utterance — Exec drops the spoken tail for a house row. A router that misheard
can pick the wrong lamp; it cannot compose a target of its own. The service is
checked against the domain's table on the way out too, so a hand-edited cmd
column cannot reach an arbitrary Home Assistant service. set_brightness and
set_temperature are deliberately absent: a spoken number the router got wrong is
a wrong act on real hardware, and on/off is the whole of what a voice turn can
defend.

The read side is a query source ("home", before calendar and the recall passes)
so "что нового дома?" is not answered from an old note. Its matcher needs a
house marker plus an ask plus a device word and bails out on weather wording,
because "какая температура на улице?" belongs to the weather source.

Off unless configured: the block is dark without "enabled": true, and
applyDefaults normalises a disabled block to nil so "off" stays in one place.
deploy/mavend.json carries it disabled, with the token as ${HA_TOKEN}.

NOT shipped, and not faked: MQTT / Zigbee2MQTT (plan steps 2 and 5) and the
sensor-to-fact and presence-probe pipelines. There is no broker and no Home
Assistant anywhere on this network — 8123 and 1883 are closed on every host in
192.168.1.0/24 — the module tree is vendored so a paho dependency cannot be
added offline, and Home Assistant already fronts Zigbee2MQTT where it exists.
Writing a sensor pipeline with no sensor to test it against would be a guess.

Vikunja #256

Read and control the house through Home Assistant (#256) A `smarthome` block points Maven at a Home Assistant instance. She reads its entity states to answer "что включено дома?", and every controllable device becomes a PROPOSED row in the existing act allowlist — cmd ["smarthome",<entity_id>,<service>], scope smarthome:<domain> — so nothing new had to be invented for the mutating half. ProposeTool/EnableTool/DisableTool, tool.Matcher and the confirm turn are untouched; one branch in Executor.Exec routes such a row to the client instead of exec, and "smarthome" is never run as a binary. This is the same trick overnight/mcp-tools used for #251, on purpose. Discovery only ever PROPOSES, and every control row is destructive=true: there is no read-only way to turn the heating off, so flipping something in his flat always costs a confirm turn and always had to be enabled by hand on /tools, behind step-up. The entity and the service come from the row he enabled, never from the utterance — Exec drops the spoken tail for a house row. A router that misheard can pick the wrong lamp; it cannot compose a target of its own. The service is checked against the domain's table on the way out too, so a hand-edited cmd column cannot reach an arbitrary Home Assistant service. set_brightness and set_temperature are deliberately absent: a spoken number the router got wrong is a wrong act on real hardware, and on/off is the whole of what a voice turn can defend. The read side is a query source ("home", before calendar and the recall passes) so "что нового дома?" is not answered from an old note. Its matcher needs a house marker plus an ask plus a device word and bails out on weather wording, because "какая температура на улице?" belongs to the weather source. Off unless configured: the block is dark without "enabled": true, and applyDefaults normalises a disabled block to nil so "off" stays in one place. deploy/mavend.json carries it disabled, with the token as ${HA_TOKEN}. NOT shipped, and not faked: MQTT / Zigbee2MQTT (plan steps 2 and 5) and the sensor-to-fact and presence-probe pipelines. There is no broker and no Home Assistant anywhere on this network — 8123 and 1883 are closed on every host in 192.168.1.0/24 — the module tree is vendored so a paho dependency cannot be added offline, and Home Assistant already fronts Zigbee2MQTT where it exists. Writing a sensor pipeline with no sensor to test it against would be a guess. Vikunja #256
claude added 1 commit 2026-08-01 04:41:34 +02:00
A `smarthome` block points Maven at a Home Assistant instance. She reads its
entity states to answer "что включено дома?", and every controllable device
becomes a PROPOSED row in the existing act allowlist — cmd
["smarthome",<entity_id>,<service>], scope smarthome:<domain> — so nothing new
had to be invented for the mutating half. ProposeTool/EnableTool/DisableTool,
tool.Matcher and the confirm turn are untouched; one branch in Executor.Exec
routes such a row to the client instead of exec, and "smarthome" is never run as
a binary. This is the same trick overnight/mcp-tools used for #251, on purpose.

Discovery only ever PROPOSES, and every control row is destructive=true: there
is no read-only way to turn the heating off, so flipping something in his flat
always costs a confirm turn and always had to be enabled by hand on /tools,
behind step-up.

The entity and the service come from the row he enabled, never from the
utterance — Exec drops the spoken tail for a house row. A router that misheard
can pick the wrong lamp; it cannot compose a target of its own. The service is
checked against the domain's table on the way out too, so a hand-edited cmd
column cannot reach an arbitrary Home Assistant service. set_brightness and
set_temperature are deliberately absent: a spoken number the router got wrong is
a wrong act on real hardware, and on/off is the whole of what a voice turn can
defend.

The read side is a query source ("home", before calendar and the recall passes)
so "что нового дома?" is not answered from an old note. Its matcher needs a
house marker plus an ask plus a device word and bails out on weather wording,
because "какая температура на улице?" belongs to the weather source.

Off unless configured: the block is dark without "enabled": true, and
applyDefaults normalises a disabled block to nil so "off" stays in one place.
deploy/mavend.json carries it disabled, with the token as ${HA_TOKEN}.

NOT shipped, and not faked: MQTT / Zigbee2MQTT (plan steps 2 and 5) and the
sensor-to-fact and presence-probe pipelines. There is no broker and no Home
Assistant anywhere on this network — 8123 and 1883 are closed on every host in
192.168.1.0/24 — the module tree is vendored so a paho dependency cannot be
added offline, and Home Assistant already fronts Zigbee2MQTT where it exists.
Writing a sensor pipeline with no sensor to test it against would be a guess.

Vikunja #256
claude reviewed 2026-08-01 11:36:03 +02:00
claude left a comment
Author
Contributor

Encoding a device as an allowlist row with cmd = ["smarthome", "<entity_id>", "<service>"] is the right move. It reuses the enable gate, the confirm turn and tool.Matcher without touching any of them, exactly as MCP did. Dropping the spoken args in the Exec branch is the load-bearing detail. The router picks a row and never composes a target. A misheard number cannot become a different lamp. Leaving set_temperature and set_brightness out of the controllable table for the same reason is consistent. The block ships "enabled": false.

1. MaxEntities truncates a globally id-sorted list, so sensors crowd out every switch.

States sorts by entity_id and then cuts to MaxEntities. Entity ids sort by domain prefix. The order is binary_sensor < cover < fan < light < lock < sensor < switch.

Take the default, an absent domains list. wanted then admits the five controllable domains plus sensor and binary_sensor. A stock Home Assistant carries dozens of binary_sensor.* before it carries anything else: connectivity, motion, update-available, one per integration. With MaxEntities at 40 and 40 binary sensors on the instance, States returns 40 binary sensors and nothing else. propose then finds zero controllable entities and proposes nothing at all, and the log line reads "40 entities, 0 controllable".

The deploy block narrows to ["light", "switch", "sensor"], which does not fix it. light < sensor < switch, and a real instance has far more sensor.* rows than lamps. Every switch.* in the flat falls off the end.

homeSummary reads the same capped list, and that is where it stops being a discovery bug. With the controllable entities truncated away, the on slice is empty and she answers "всё выключено" while the lights are on. That is a spoken false statement about the house, produced by a cap that was meant to bound a catalogue.

Cap per domain, or partition and take controllable entities first and fill the remainder with sensors.

2. "Every control row is destructive" is a comment, not an invariant.

internal/smarthome's package doc calls it non-negotiable, and ProposeSmartHomeTool hardcodes true. But enable is not propose. handleTools reads destructive := r.FormValue("destructive") != "" from a checkbox, and EnableTool writes destructive=excluded.destructive on the conflict path. The proposal row's own value is overwritten.

So: the form pre-checks the box from the proposed row, Kami unchecks it, presses enable, and home_lock_front_door_unlock is now an enabled non-destructive row. Exec reaches t.Destructive && !confirmed and passes straight through to CallService. The confirm turn is gone. That is the one row class whose whole argument is that the confirm turn is always there.

Make it structural. In Exec, after the destructive check, if smarthome.ParseCmd(t.Cmd) succeeds and !confirmed, return ErrNeedsConfirm regardless of the column. Then the guarantee holds no matter what any surface writes.

3. CallService reports "готово" for a service call that changed nothing.

do accepts any 2xx and CallService returns the literal "готово". Home Assistant answers POST /api/services/light/turn_off with 200 and a JSON array of the states it changed. An entity_id that does not exist, or one whose integration is offline, gets 200 and [].

Both cases are reachable. A row survives in the allowlist after the device is removed from HA. That is the case ErrUnknownEntity was written for, and it never fires: CallService validates only the domain and the service name, both locally. And a Zigbee lamp with a flat battery is unavailable while its entity still exists.

He says "выключи свет", she says "готово", the light stays on. Decode the response array and say so when it is empty. This is the one place in the PR where Maven asserts something about the physical world.

4. queryHome claims the turn on a box that has no smarthome block.

isHomeQuery runs before h.home is consulted, and the nil branch returns "дом не подключён — я его не вижу." with handled=true. That fires on every Maven, configured house or not, since the query source is registered unconditionally.

Walk it on the default box. He once said "дома температура 22", stored as a fact. He asks "какая температура в доме?". isHomeQuery sees "в доме", an ask, and "температур", returns true, and the turn is claimed by a source backed by nothing. Before this PR that reached queryEmbed and the recall path. A capability that is off is not supposed to change what an unconfigured box answers.

Return "", false when h.home == nil and let the cascade continue. The "дом не подключён" line is right when the block exists but the instance is unreachable. homeSummary already covers that with "не смогла достучаться до дома".

5. wireSmartHome blocks daemon startup on the house for up to 30 seconds.

The comment says the daemon never waits on the house. Its words: "an instance that is down at boot is logged and retried, because Maven starting is not contingent on someone else's process". The code calls w.propose(ctx) synchronously with a 30-second context. That happens inside wireVoice, inside run, before the IPC socket is serving.

A Home Assistant box that is powered off but still on a routed subnet does not refuse the connection. It black-holes it, and the client waits out the per-call DefaultTimeout of 10s. Maven's start is contingent on it, for as long as that takes. On the locked path this happens inside the passkey unlock handler, so the unlock request hangs too. Do the first propose from run's goroutine and let the ticker pick it up.

Smaller notes:

  • lock is in controllable and in the default domain set, so a bare {"url":…, "token":…, "enabled":true} auto-proposes unlock rows for every deadbolt in the flat. Proposals are inert until enabled, and the confirm turn is finding 2's problem. But a front door is a different class of object from a lamp. Consider requiring lock to be named explicitly in domains.
  • Validate accepts http, and the deploy block uses it. The long-lived token goes over the LAN in cleartext on every refresh. Worth one line in SmartHomeConfig saying that is a deliberate wg-only choice.
  • Refresh has no floor. "refresh": "1s" passes validation and enumerates the whole instance every second. applyDefaults only fills <= 0.
  • A light in state unavailable matches neither the sensor branch nor the on branch of homeSummary, so it vanishes from the answer. "всё выключено" and "one lamp is unreachable" read the same to him.
  • homeSummary truncates on to 5 and sensors to 3 with no "и ещё N". Silent truncation on a status read has the same shape as finding 1, one layer up.
Encoding a device as an allowlist row with `cmd = ["smarthome", "<entity_id>", "<service>"]` is the right move. It reuses the enable gate, the confirm turn and `tool.Matcher` without touching any of them, exactly as MCP did. Dropping the spoken args in the `Exec` branch is the load-bearing detail. The router picks a row and never composes a target. A misheard number cannot become a different lamp. Leaving `set_temperature` and `set_brightness` out of the `controllable` table for the same reason is consistent. The block ships `"enabled": false`. **1. `MaxEntities` truncates a globally id-sorted list, so sensors crowd out every switch.** `States` sorts by `entity_id` and then cuts to `MaxEntities`. Entity ids sort by domain prefix. The order is `binary_sensor` < `cover` < `fan` < `light` < `lock` < `sensor` < `switch`. Take the default, an absent `domains` list. `wanted` then admits the five controllable domains plus `sensor` and `binary_sensor`. A stock Home Assistant carries dozens of `binary_sensor.*` before it carries anything else: connectivity, motion, update-available, one per integration. With `MaxEntities` at 40 and 40 binary sensors on the instance, `States` returns 40 binary sensors and nothing else. `propose` then finds zero controllable entities and proposes nothing at all, and the log line reads "40 entities, 0 controllable". The deploy block narrows to `["light", "switch", "sensor"]`, which does not fix it. `light` < `sensor` < `switch`, and a real instance has far more `sensor.*` rows than lamps. Every `switch.*` in the flat falls off the end. `homeSummary` reads the same capped list, and that is where it stops being a discovery bug. With the controllable entities truncated away, the `on` slice is empty and she answers "всё выключено" while the lights are on. That is a spoken false statement about the house, produced by a cap that was meant to bound a catalogue. Cap per domain, or partition and take controllable entities first and fill the remainder with sensors. **2. "Every control row is destructive" is a comment, not an invariant.** `internal/smarthome`'s package doc calls it non-negotiable, and `ProposeSmartHomeTool` hardcodes `true`. But enable is not propose. `handleTools` reads `destructive := r.FormValue("destructive") != ""` from a checkbox, and `EnableTool` writes `destructive=excluded.destructive` on the conflict path. The proposal row's own value is overwritten. So: the form pre-checks the box from the proposed row, Kami unchecks it, presses enable, and `home_lock_front_door_unlock` is now an enabled non-destructive row. `Exec` reaches `t.Destructive && !confirmed` and passes straight through to `CallService`. The confirm turn is gone. That is the one row class whose whole argument is that the confirm turn is always there. Make it structural. In `Exec`, after the destructive check, if `smarthome.ParseCmd(t.Cmd)` succeeds and `!confirmed`, return `ErrNeedsConfirm` regardless of the column. Then the guarantee holds no matter what any surface writes. **3. `CallService` reports "готово" for a service call that changed nothing.** `do` accepts any 2xx and `CallService` returns the literal "готово". Home Assistant answers `POST /api/services/light/turn_off` with 200 and a JSON array of the states it changed. An `entity_id` that does not exist, or one whose integration is offline, gets 200 and `[]`. Both cases are reachable. A row survives in the allowlist after the device is removed from HA. That is the case `ErrUnknownEntity` was written for, and it never fires: `CallService` validates only the domain and the service name, both locally. And a Zigbee lamp with a flat battery is unavailable while its entity still exists. He says "выключи свет", she says "готово", the light stays on. Decode the response array and say so when it is empty. This is the one place in the PR where Maven asserts something about the physical world. **4. `queryHome` claims the turn on a box that has no `smarthome` block.** `isHomeQuery` runs before `h.home` is consulted, and the nil branch returns `"дом не подключён — я его не вижу."` with `handled=true`. That fires on every Maven, configured house or not, since the query source is registered unconditionally. Walk it on the default box. He once said "дома температура 22", stored as a fact. He asks "какая температура в доме?". `isHomeQuery` sees "в доме", an ask, and "температур", returns true, and the turn is claimed by a source backed by nothing. Before this PR that reached `queryEmbed` and the recall path. A capability that is off is not supposed to change what an unconfigured box answers. Return `"", false` when `h.home == nil` and let the cascade continue. The "дом не подключён" line is right when the block exists but the instance is unreachable. `homeSummary` already covers that with "не смогла достучаться до дома". **5. `wireSmartHome` blocks daemon startup on the house for up to 30 seconds.** The comment says the daemon never waits on the house. Its words: "an instance that is down at boot is logged and retried, because Maven starting is not contingent on someone else's process". The code calls `w.propose(ctx)` synchronously with a 30-second context. That happens inside `wireVoice`, inside `run`, before the IPC socket is serving. A Home Assistant box that is powered off but still on a routed subnet does not refuse the connection. It black-holes it, and the client waits out the per-call `DefaultTimeout` of 10s. Maven's start is contingent on it, for as long as that takes. On the locked path this happens inside the passkey unlock handler, so the unlock request hangs too. Do the first `propose` from `run`'s goroutine and let the ticker pick it up. Smaller notes: - `lock` is in `controllable` and in the default domain set, so a bare `{"url":…, "token":…, "enabled":true}` auto-proposes `unlock` rows for every deadbolt in the flat. Proposals are inert until enabled, and the confirm turn is finding 2's problem. But a front door is a different class of object from a lamp. Consider requiring `lock` to be named explicitly in `domains`. - `Validate` accepts `http`, and the deploy block uses it. The long-lived token goes over the LAN in cleartext on every refresh. Worth one line in `SmartHomeConfig` saying that is a deliberate wg-only choice. - `Refresh` has no floor. `"refresh": "1s"` passes validation and enumerates the whole instance every second. `applyDefaults` only fills `<= 0`. - A light in state `unavailable` matches neither the sensor branch nor the on branch of `homeSummary`, so it vanishes from the answer. "всё выключено" and "one lamp is unreachable" read the same to him. - `homeSummary` truncates `on` to 5 and sensors to 3 with no "и ещё N". Silent truncation on a status read has the same shape as finding 1, one layer up.
kami closed this pull request 2026-08-01 14:52:06 +02:00
Owner

Landed on master. The stack was one linear chain, so #84 carried every commit from #50 up, and master now contains this branch in full. Merging this PR on its own is an empty diff, so it is closed rather than merged. The review findings for it were fixed in the 2026-08-01 pass and are on master as commits on the stack tip, not on this branch.

Landed on master. The stack was one linear chain, so #84 carried every commit from #50 up, and master now contains this branch in full. Merging this PR on its own is an empty diff, so it is closed rather than merged. The review findings for it were fixed in the 2026-08-01 pass and are on master as commits on the stack tip, not on this branch.

Pull request closed

Sign in to join this conversation.
No Reviewers
No Label
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: kami/Maven#80