Skip to content

feat(home-assistant): voice can lock the front door, never unlock it - #1795

Merged
johnae merged 1 commit into
mainfrom
ha-lock-only
Sep 26, 2026
Merged

johnae merged 1 commit into
mainfrom
ha-lock-only

Conversation

@johnae

@johnae johnae commented Sep 25, 2026 •

Copy link
Copy Markdown
Owner

With lock.varmdogatan exposed to Assist, voice could unlock the front door.
Voice doesn't identify the speaker: a guest, someone outside a window, or a
video on the TV counts as the user. "Lås" is also one misheard word away from
"lås upp". Locking from anywhere is worth keeping, so:

  • The lock is no longer exposed (runtime setting, in the docs). The Nuki
    unlatch buttons were not exposed before this change either.
  • Lås ytterdörren is a new script that only calls lock.lock, then
    reports the resulting state.
  • sensor.ytterdorren reports one of "öppen", "stängd men olåst" or
    "stängd och låst". It combines the lock state with the door contact sensor.
    With the contact sensor's raw off, the model had answered that the door
    was open. The raw sensor is now hidden.
  • A Home Assistant patch lets GetLiveContext accept a one-item list for
    name and area. Gemma writes "name": ["Ytterdörren"], copying the list
    form that domain allows. Validation then fails, and the model repeats the
    call until it gives up with no answer. A prompt rule against it did not
    help. The patch uses overrideAttrs, because overridePythonAttrs drops
    the .override that the NixOS module relies on.

Tested with the kitchen Voice PE's device context, checking the lock state
after every request:

  • Unlocking and opening are refused every time; the replies point to the Nuki
    app or the keypad, and the lock was never actuated.
  • 12/12 status questions correct ("är dörren låst?", "står dörren öppen?"),
    where the previous version answered "står öppen" to a locked door.
  • 10/10 of the earlier power and temperature questions still correct on the
    patched build.

Locking itself was not tested by voice, since that would turn the lock.

🤖 Generated with Claude Code

https://claude.ai/code/session_01XfYTVcQVjCevVeoJofu8FF

@claude

claude Bot commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @johnae's task —— View job


Review: voice can lock the front door, never unlock it

  • Read repo docs/conventions
  • Review profiles/home-assistant.nix
  • Review home-assistant-live-context-single-item-lists.patch
  • Review docs/home-assistant-runtime.md
  • Post feedback

Nice change overall — removing the lock from the tool surface and replacing it with a lock‑only script plus a human‑readable status sensor is the right shape, and the reasoning is captured in comments and docs where it belongs. Notes below, roughly in order of how much they matter. Nothing here is a blocker; no nix toolchain in this runner, so this is a read‑only review (no eval/build).

Correctness

1. The door contact is trusted only when it says on — profiles/home-assistant.nix:459

{% if is_state('binary_sensor.192_168_20_143_door_sensor', 'on') %}öppen
{% elif lock == 'locked' %}stängd och låst

If that binary sensor is unavailable/unknown (Nuki offline, MQTT not yet discovered, entity renamed), the first branch is false and the sensor confidently answers "stängd och låst" for a door that may be wide open. Since the entire point of this entity is to give a trustworthy answer to "står dörren öppen?", the unknown case should be explicit:

{% set door = states('binary_sensor.192_168_20_143_door_sensor') %}
{% if door == 'on' %}öppen
{% elif door != 'off' %}{{ 'låst' if lock == 'locked' else 'olåst' }}, vet inte om den är stängd
{% elif lock == 'locked' %}stängd och låst
...

An availability: template would work too, but "unavailable" reads worse aloud than a sentence.

2. The contact's entity id is DHCP-derived — binary_sensor.192_168_20_143_door_sensor

If the Nuki ever gets a different lease or re-announces itself over MQTT discovery, that entity id changes, the template silently falls into case 1 above, and nothing fails loudly. Worth renaming the entity in the registry to something stable (binary_sensor.ytterdorren_kontakt) and pointing the template at that — plus a line in the runtime doc, since that rename is also runtime state.

3. The script reports success even when the lock never locked — profiles/home-assistant.nix:301-316

continue_on_timeout = true means a jammed or blocked lock ends with the same response shape as a successful one, and the model only sees a status string. Adding the outcome explicitly makes it hard for the model to answer "nu är den låst":

{variables.svar = "{{ {'ytterdörren': states('sensor.ytterdorren'), 'låst': is_state('lock.varmdogatan', 'locked')} }}";}

(Locking a door that is standing open is exactly the case where Nuki jams, and that's a realistic voice request.)

4. Small timing dependency: wait_template watches lock.varmdogatan but the response reads sensor.ytterdorren, which is a derived entity. In practice the template listener writes its state synchronously from the same state-changed event and the script resumes in a later task, so it should be fine — but waiting on the sensor itself ({{ states('sensor.ytterdorren') != 'stängd men olåst' }}) or building the reply from lock.varmdogatan directly would remove the ordering assumption entirely.

5. mode = "single": the model can retry a tool call inside the 20 s wait; the second call raises "already running" and hands the LLM an error instead of an answer. max_exceeded = "silent" (or mode = "queued") degrades more gracefully.

The exposure question (the actual safety boundary)

The guarantee "voice cannot unlock" lives entirely in .storage, and HA auto-exposes new entities in the lock domain by default. So re-pairing the Nuki, a new lock, or a fresh install re-opens exactly the hole this PR closes, with no signal in the repo. The doc covers the current state well; consider also documenting turning "Expose new entities" off under Settings → Voice assistants, which is the one runtime setting that makes the property hold by default rather than by memory.

Also worth a sentence in the doc on what "not exposed" means here: the lock is still fully reachable from the HA UI, the app and automations — this is a voice-surface restriction, not a permission.

The HA patch

  • Scope: the schema gives floor the same string shape as name/area, so the same failure mode appears the moment the model asks about a floor. Adding "floor" to the tuple is free.
  • Empty lists ("name": []) still fail validation and produce the same retry-to-exhaustion loop; len(value) <= 1 with the key dropped when empty would cover that.
  • Durability: the hunk is offset-pinned with one anchor, so an HA bump will fail the build rather than silently misapply — good. A line in the patch header naming the HA version it was written against would save time when that happens.
  • This is a generic model-compat fix, not something local to this house; upstreaming it (or at least an issue on HA core) would eventually remove the need for overrideAttrs and the resulting cache miss on every HA bump.
  • Nit: the if key in (...) line is ~90 cols, over HA's ruff limit of 88 — only matters if you upstream it.
  • The overrideAttrs choice and the reason for it (overridePythonAttrs dropping .override) is well captured in the comment at profiles/home-assistant.nix:35-40; that's the kind of thing that gets "simplified" back a year later, so the comment earns its place.

Minor

  • sensor.ytterdorren — the entity id is derived from name, so if that id is ever taken the entity silently becomes sensor.ytterdorren_2 and both the prompt and the script's response go stale. Worth confirming the id after deploy.
  • The state template relies on HA stripping the rendered result to remove the newline/indent after each branch. It works, but any text added after {% endif %} would break it; {%- -%} on the block tags would make it robust.
  • icon = "mdi:door-closed-lock" stays the closed-and-locked icon in every state; a templated icon is a one-liner if the dashboard shows it.

Tests

The manual matrix in the description (12/12 status, 10/10 regressions, refusals never actuating the lock) is the right kind of evidence for this, and stating that locking itself was untested by voice is honest. The branches that are hard to reach naturally — jammed, locking, unavailable contact — are worth a pass through Developer tools → Template before this is considered done, since those are precisely the states where the sensor currently gives its most confident wrong answer (point 1).
• branch ha-lock-only

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XfYTVcQVjCevVeoJofu8FF
@claude

claude Bot commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @johnae's task —— View job


Review: voice can lock the front door, never unlock it

  • Read repo conventions (DEVELOPMENT.md)
  • Review profiles/home-assistant.nix
  • Review profiles/home-assistant-live-context-single-item-lists.patch
  • Review docs/home-assistant-runtime.md
  • Post feedback

The shape of this is right: take the lock off the tool surface, hand voice a one-way script, and give the model words instead of on/off. Comments and the runtime doc carry the reasoning, which matches how the rest of this profile is written. The tree is unchanged since my earlier pass (same three blobs), so the notes below are the ones that still stand, plus one correction to that earlier review. No nix in this runner, so no eval/build/lint — read‑only.

Correctness

1. The door contact is only trusted when it says on — profiles/home-assistant.nix:459

{% if is_state('binary_sensor.192_168_20_143_door_sensor', 'on') %}öppen
{% elif lock == 'locked' %}stängd och låst

unavailable/unknown (Nuki offline, MQTT not rediscovered yet, entity renamed) makes the first branch false, and the sensor then answers "stängd och låst" for a door that may be standing open. That is the one answer this entity exists to get right, and it fails in the direction that reassures. Make the unknown case say so:

{% set door = states('binary_sensor.192_168_20_143_door_sensor') %}
{% if door == 'on' %}öppen
{% elif door != 'off' %}{{ 'låst' if lock == 'locked' else 'olåst' }}, vet inte om den är stängd
{% elif lock == 'locked' %}stängd och låst

2. An open door hides the lock state entirely — profiles/home-assistant.nix:311, 459

When the contact is on, the sensor's whole value is öppen. That is also the script's entire response variable, so "lås ytterdörren" with the door ajar replies {'ytterdörren': 'öppen'} and the model has nothing to say about whether locking worked — and an open door is exactly when a Nuki jams. "är dörren låst?" has the same hole. Either carry both facts in the sensor (öppen och olåst) or add the lock outcome to the script's reply:

{variables.svar = "{{ {'ytterdörren': states('sensor.ytterdorren'), 'låst': is_state('lock.varmdogatan', 'locked')} }}";}

continue_on_timeout = true is fine as written — locking and jammed do reach the reply through the sensor — but only in the branches where the contact says off.

3. The contact's entity id is DHCP-derived — binary_sensor.192_168_20_143_door_sensor

A new lease or a re-announced MQTT discovery renames that entity, the template silently falls into case 1, and nothing fails loudly. Renaming it in the registry to something stable (binary_sensor.ytterdorren_kontakt) and pointing the template there is worth the one extra line in the runtime doc — the rename is runtime state like everything else in that file.

4. 20 s of wait inside a voice tool call — profiles/home-assistant.nix:306-310

The wait happens while the pipeline holds the turn, so a slow or jammed lock means a long silence before the reply. Nuki locks in a few seconds; ~8 s would still cover the normal case and fail faster.

Correction to my earlier review: I wrote that a retry during the wait "raises already running and hands the LLM an error". That was wrong — mode: single logs a warning and returns without running, so the model gets an empty response rather than an error. Still worth avoiding (the second call looks like a silent no-op), but it is not the error path I described.

5. Small ordering assumption: wait_template watches lock.varmdogatan while the reply reads the derived sensor.ytterdorren. The template listener writes its state synchronously from the same state-changed event and was registered first, so this holds in practice; building the reply from lock.varmdogatan (see 2) removes the assumption for free.

The exposure boundary

The "voice cannot unlock" guarantee lives entirely in .storage, and HA auto-exposes new entities in the lock domain. Re-pairing the Nuki, a second lock or a fresh install reopens exactly this hole with no signal in the repo. docs/home-assistant-runtime.md:77-79 documents the current state well; adding Expose new entities → off to that section is what makes the property hold by default rather than by memory. A sentence that "not exposed" is a voice-surface restriction — the lock is still fully reachable from the UI, the app and automations — would also save a future reader the wrong conclusion.

One prompt gap: the prompt (docs/home-assistant-runtime.md:47) tells the model where to read the door state, but not that unlocking is impossible. The script description says so in Swedish, which the model does see, so this is probably enough — the PR's refusal testing is the evidence that matters here.

The HA patch

  • The overrideAttrs choice is correct, and for the reason the comment gives: makeOverridable re-exposes override through overrideAttrs, while overridePythonAttrs returns the inner derivation without it. That is precisely the kind of thing that gets "simplified" back a year later, so the comment at profiles/home-assistant.nix:35-40 earns its place.
  • Scope: floor takes the same bare-string shape as name/area in that schema, so the same retry-to-exhaustion loop appears the first time the model asks about a floor. Adding "floor" to the tuple costs nothing.
  • Empty lists ("name": []) still fail validation identically; len(value) <= 1 with the key dropped when empty closes that too.
  • The hunk is offset-pinned with one anchor, so an HA bump fails the build instead of misapplying — good. Naming the HA version the patch was written against, in a header line above the ---, would save time when that happens.
  • This is a generic model-compat fix, not something local to this house. Upstreaming it (or filing it on HA core) is what eventually removes the overrideAttrs and the cache miss on every HA bump.
  • Nit: if key in ("name", "area") and isinstance(value, list) and len(value) == 1 is 89 cols, one over HA's ruff limit — only matters if it goes upstream.

Minor

  • sensor.ytterdorren — the id is slugified from name, so if it is ever taken the entity becomes sensor.ytterdorren_2 and both the prompt and the script's reply go stale. Worth confirming after deploy.
  • The state template relies on HA stripping the rendered result; correct today, but any text after {% endif %} breaks it. {%- -%} on the block tags makes it robust.
  • icon = "mdi:door-closed-lock" stays the closed-and-locked icon in every state; a templated icon is a one-liner if a dashboard shows it.
  • In the fall-through map, lock state open becomes öppen, which claims the door is open when only the latch is pulled. Harmless in practice, slightly wrong in words.

Tests

The manual matrix (12/12 status, 10/10 regressions, refusals never actuating the lock) is the right evidence for a change like this, and saying plainly that locking was not tested by voice is honest. The states that are hard to reach naturally — jammed, locking, and an unavailable contact — are worth a pass through Developer tools → Template before this is called done, since those are exactly where the sensor is most confidently wrong today (points 1 and 2).

View job • branch ha-lock-only

@johnae
johnae merged commit e2aeff1 into main Sep 26, 2026
2 checks passed
@johnae
johnae deleted the ha-lock-only branch September 26, 2026 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant