The issue records that replaying HA's birth message (homeassistant/status = online) empirically did not trigger a discovery re-publish, and treats that as unexplained. It is fully explained. Verified against Zigbee2MQTT 2.12.1 source (the running version):
Path
What Z2M actually does
homeassistant/status = online
homeassistant.tsonMQTTMessage republishes bridge/state + cached device states after a hardcoded 30 s delay — never the discovery configs
MQTT reconnect
mqtt.tsonConnect republishes bridge/state and resubscribes. republishRetainedTimer is armed only inside connect() and never re-armed
discovery is published retain: true; the only knob is the global force_disable_retain, which upstream documents as breaking the HA integration
discover() runs only from HomeAssistant.start(). Broker retention is the sole mechanism Z2M relies on, so a mosquitto restart not followed by a Z2M restart leaves every Z2M entity unavailable in HA indefinitely.
The ACL was never the blocker — read homeassistant/status is already granted. That grant is still correct (the cached-state republish is what makes HA show current values after an HA restart), so it is kept and the comment now says what it actually buys.
persistence true is not on the table — this broker deliberately has no PVC. Unchanged.
The fix
A discovery-guard sidecar in the Z2M pod.
Detection.bridge/state and bridge/{info,devices,groups,definitions} are all retained, but only bridge/state is republished on reconnect. So bridge/state present + bridge/groups absent means: Z2M is talking to this broker right now, but nothing has re-run start() since the retained store was wiped.
bridge/groups specifically, because the absent-half partner must not be re-publishable by ordinary runtime events — otherwise it self-heals and the guard goes blind while HA is still broken:
bridge/devices / bridge/definitions / bridge/info — all reach inbound radio traffic via devicesChanged (receive.ts hands deviceExposesChanged to every fromZigbee converter). Unusable.
bridge/health — self-heals on its own 10 min timer. Unusable.
bridge/groups — only republished by deliberate admin actions (group membership, group add/remove/rename, and scenes, which come solely from an explicit <device>/set command).
publishGroups() always publishes, emitting [] with no groups defined — confirmed live, it currently holds a 2-byte payload, so presence/absence is testable.
Deliberately not AND-ed with bridge/info absent. That would cut false positives but introduce false negatives, and the costs are asymmetric: a false positive is one ~60 s Z2M restart; a false negative is every Zigbee entity dead until a human notices. Tuned for sensitivity.
Debounce. A 90 s confirm re-check covers the two known spurious-positive paths: the startup race (bridge/state is published at mqtt.connect(), bridge/groups only at the end of BridgeExtension.start()) and a dropped publish (these topics go out at QoS 0). A 30 min cooldown stops a Z2M that will not come up healthy from being restarted in a loop.
Action.bridge/request/restart is a genuine full restart, not herdsman-only: index.jsrestart() → stop(true) tears down every extension, then start() builds a new Controller and re-runs HomeAssistant.start(). discover() then republishes only missing/changed configs — after a wipe, all of them.
Credentials. Reuses the existing zigbee2mqtt broker user (readwrite zigbee2mqtt/#). No new broker user, and no read grant on homeassistant/# — checking the discovery configs directly would have needed one, which the ACL denies on purpose.
Verification
kubectl kustomize renders clean.
In the actual eclipse-mosquitto:2.0.22 runtime image: mosquitto_sub-C/-W, mosquitto_pub, date -u +%s all present; sh -n clean; startup run logs correctly and enters the loop.
Detection exercised against the live broker: bridge/state = {"state":"online"}, bridge/groups present → correctly reports healthy, no action. No false positive.
Reviewer note — this rolls mosquitto
The corrected ACL comment lives inside the container command, so it changes the rendered Deployment and rolls the broker. Because maxUnavailable: 0/maxSurge: 1 still brings up a new pod with an empty retained store, that roll wipes discovery — the guard then repairs it within ~6 min, if the Z2M roll (which also restarts Z2M) has not already done so.
The first commit is comment-only and rolls nothing; the two deployment comments there are plain YAML comments outside the container command, verified byte-identical in the render.
Closes #381.
## Root cause — the issue's premise was wrong
The issue records that replaying HA's birth message (`homeassistant/status` = `online`) empirically did not trigger a discovery re-publish, and treats that as unexplained. It is fully explained. Verified against Zigbee2MQTT **2.12.1** source (the running version):
| Path | What Z2M actually does |
|---|---|
| `homeassistant/status` = `online` | `homeassistant.ts` `onMQTTMessage` republishes `bridge/state` + **cached device states** after a hardcoded 30 s delay — never the discovery configs |
| MQTT reconnect | `mqtt.ts` `onConnect` republishes `bridge/state` and resubscribes. `republishRetainedTimer` is armed only inside `connect()` and never re-armed |
| On demand | **No** `bridge/request` rediscovery endpoint exists. Upstream declined three times: Koenkk/zigbee2mqtt #27458, #22977, #16950 (all NOT_PLANNED) |
| Retention | discovery is published `retain: true`; the only knob is the global `force_disable_retain`, which upstream documents as breaking the HA integration |
`discover()` runs **only** from `HomeAssistant.start()`. Broker retention is the sole mechanism Z2M relies on, so a mosquitto restart not followed by a Z2M restart leaves every Z2M entity unavailable in HA indefinitely.
The ACL was never the blocker — `read homeassistant/status` is already granted. That grant is still correct (the cached-state republish is what makes HA show current values after an *HA* restart), so it is kept and the comment now says what it actually buys.
**`persistence true` is not on the table** — this broker deliberately has no PVC. Unchanged.
## The fix
A `discovery-guard` sidecar in the Z2M pod.
**Detection.** `bridge/state` and `bridge/{info,devices,groups,definitions}` are all retained, but only `bridge/state` is republished on reconnect. So *`bridge/state` present + `bridge/groups` absent* means: Z2M is talking to this broker right now, but nothing has re-run `start()` since the retained store was wiped.
`bridge/groups` specifically, because the absent-half partner must not be re-publishable by ordinary runtime events — otherwise it self-heals and the guard goes blind while HA is still broken:
- `bridge/devices` / `bridge/definitions` / `bridge/info` — all reach **inbound radio traffic** via `devicesChanged` (`receive.ts` hands `deviceExposesChanged` to every `fromZigbee` converter). Unusable.
- `bridge/health` — self-heals on its own 10 min timer. Unusable.
- `bridge/groups` — only republished by deliberate admin actions (group membership, group add/remove/rename, and scenes, which come solely from an explicit `<device>/set` command).
`publishGroups()` always publishes, emitting `[]` with no groups defined — confirmed live, it currently holds a **2-byte** payload, so presence/absence is testable.
**Deliberately not AND-ed with `bridge/info` absent.** That would cut false positives but introduce false negatives, and the costs are asymmetric: a false positive is one ~60 s Z2M restart; a false negative is every Zigbee entity dead until a human notices. Tuned for sensitivity.
**Debounce.** A 90 s confirm re-check covers the two known spurious-positive paths: the startup race (`bridge/state` is published at `mqtt.connect()`, `bridge/groups` only at the end of `BridgeExtension.start()`) and a dropped publish (these topics go out at **QoS 0**). A 30 min cooldown stops a Z2M that will not come up healthy from being restarted in a loop.
**Action.** `bridge/request/restart` is a genuine full restart, not herdsman-only: `index.js` `restart()` → `stop(true)` tears down every extension, then `start()` builds a new `Controller` and re-runs `HomeAssistant.start()`. `discover()` then republishes only missing/changed configs — after a wipe, all of them.
**Credentials.** Reuses the existing `zigbee2mqtt` broker user (`readwrite zigbee2mqtt/#`). No new broker user, and no read grant on `homeassistant/#` — checking the discovery configs directly would have needed one, which the ACL denies on purpose.
## Verification
- `kubectl kustomize` renders clean.
- In the actual `eclipse-mosquitto:2.0.22` runtime image: `mosquitto_sub` `-C`/`-W`, `mosquitto_pub`, `date -u +%s` all present; `sh -n` clean; startup run logs correctly and enters the loop.
- Detection exercised against the **live** broker: `bridge/state` = `{"state":"online"}`, `bridge/groups` present → correctly reports healthy, no action. No false positive.
## Reviewer note — this rolls mosquitto
The corrected ACL comment lives *inside* the container command, so it changes the rendered Deployment and rolls the broker. Because `maxUnavailable: 0`/`maxSurge: 1` still brings up a new pod with an **empty retained store**, that roll wipes discovery — the guard then repairs it within ~6 min, if the Z2M roll (which also restarts Z2M) has not already done so.
The first commit is comment-only and rolls nothing; the two deployment comments there are plain YAML comments outside the container command, verified byte-identical in the render.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Four comments across the broker manifests asserted that retained state is
re-populated by all clients on reconnect, and specifically that Zigbee2MQTT
re-publishes HA discovery when it sees the homeassistant/status birth message.
Both are false, and the second is the premise that kept issue #381 open.
Verified against Zigbee2MQTT 2.12.1 source:
- discover() runs only from HomeAssistant.start(); it is never called from
the reconnect path.
- lib/mqtt.ts onConnect() republishes bridge/state and resubscribes, nothing
more. republishRetainedTimer is armed only inside connect() and is never
re-armed on reconnect.
- lib/extension/homeassistant.ts onMQTTMessage(), on status_topic == "online",
republishes bridge/state plus cached device states after a hardcoded 30s
delay -- but never the discovery configs.
- Discovery is published retain:true, and the only knob is the global
mqtt.force_disable_retain, which upstream documents as breaking the HA
integration. Broker retention is the sole mechanism Z2M relies on.
- No bridge/request endpoint forces a rediscovery; upstream declined to add
one (Koenkk/zigbee2mqtt #27458, #22977, #16950, all closed NOT_PLANNED).
The mosquitto ACL was never the blocker: `read homeassistant/status` is already
granted. That grant is still correct -- the cached-state republish is what makes
HA show current values after an HA restart -- so it is kept and the comment now
says what it actually buys.
Also records that a RollingUpdate roll is equivalent to a broker restart for
retention: maxUnavailable:0/maxSurge:1 preserves connectivity, but the new pod
starts with an empty retained store.
Comment-only. The two deployment comments are plain YAML comments outside the
container command, so the rendered Deployment is byte-identical and the broker
does not roll; the ConfigMap carries no reloader annotation, so it updates in
place without restarting mosquitto.
Refs #381
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
mosquitto runs `persistence false` with no PVC (deliberate and settled), so
every broker restart wipes the retained store. Zigbee2MQTT publishes its HA
discovery configs retained and has no way to restore them itself, so a broker
restart not followed by a Z2M restart leaves every Z2M entity unavailable in HA
indefinitely -- ~14 h on 2026-07-23.
Adds a discovery-guard sidecar to the Z2M pod that detects the wiped state and
asks Z2M to restart, which re-runs discover().
Detection. bridge/state and bridge/{info,devices,groups,definitions} are all
retained, but only bridge/state is republished on reconnect (mqtt.ts onConnect).
So "bridge/state present + bridge/groups absent" means Z2M is talking to this
broker but nothing has re-run start() since the store was wiped.
bridge/groups specifically, because the absent-half partner must not be
re-publishable by ordinary runtime events or it self-heals and the guard goes
blind while HA is still broken. bridge/devices, bridge/definitions and
bridge/info all reach inbound radio traffic via devicesChanged (receive.ts hands
deviceExposesChanged to every fromZigbee converter); bridge/health self-heals on
its own 10 min timer. bridge/groups is only republished by deliberate admin
actions, and publishGroups() always publishes, emitting `[]` when no groups
exist -- confirmed live: it currently holds a 2-byte payload, so presence is
testable.
Not AND-ed with bridge/info being absent. That would cut false positives at the
cost of false negatives, and the costs are asymmetric: a false positive is one
~60s Z2M restart, a false negative is every Zigbee entity dead until a human
notices. Tuned for sensitivity.
Spurious positives are handled by a 90s confirm re-check, which covers both the
startup race (bridge/state is published at mqtt.connect(), bridge/groups only at
the end of BridgeExtension.start()) and a dropped publish (these topics go out at
QoS 0). A 30 min cooldown keeps a Z2M that will not come up healthy from being
restarted in a loop.
Action is bridge/request/restart, which is a full in-process restart -- index.js
restart() -> stop(true) tears down every extension, then start() builds a new
Controller and re-runs HomeAssistant.start(). discover() then republishes only
missing or changed configs, which after a wipe is all of them.
Reuses the existing zigbee2mqtt broker credential (ACL `readwrite
zigbee2mqtt/#`), so no new broker user and no read grant on homeassistant/#,
which the ACL denies on purpose.
Verified in the eclipse-mosquitto:2.0.22 image the sidecar actually runs:
mosquitto_sub -C/-W, mosquitto_pub and `date -u +%s` all present; `sh -n` clean;
startup run logs correctly and enters the loop. Detection was exercised against
the live broker and correctly reports healthy (bridge/state online,
bridge/groups present) with no action.
NOTE: this rolls mosquitto, because the corrected ACL comment lives inside the
container command. That roll wipes the retained store, which the guard then
repairs within ~6 min if the Z2M roll does not already do so.
Closes#381
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
🤖Automated review — model CLI default (complexity: complex), commit 5c856af578d85b38ebe296cd8fb2a4f3520a4019.
Advisory review by the review-bot pipeline. It never approves; a human merges. Push new commits to get a fresh review.
Summary
Adds a discovery-guard sidecar to the Zigbee2MQTT pod that polls bridge/state/bridge/groups on the mosquitto broker and publishes bridge/request/restart when it looks like the broker's retained store was wiped (fixing #381 — Z2M discovery configs don't survive a broker restart because Z2M itself never re-publishes them). The change is exclusively new sidecar wiring plus doc/ACL-comment corrections; no existing logic paths are touched. The detection/action logic in zigbee2mqtt-discovery-guard.sh is careful and internally consistent (startup-race and QoS-0 false positives are explicitly handled via CONFIRM_DELAY, restart storms via COOLDOWN), and the surrounding YAML wiring (ConfigMap, volumes, securityContext, ACL scope) mirrors the existing interview-watch sidecar pattern closely and correctly. I did not find a blocking defect. One real robustness gap is worth fixing before/soon after merge (see Suggestions).
🟡 Suggestions
kubernetes/apps/home-assistant/zigbee2mqtt-discovery-guard.sh:351-353 — the restart-triggering mosquitto_pub call is not bounded by timeout, unlike the sibling mosquitto_pub call in zigbee2mqtt-interview-watch.sh:233 (added in this same PR family), whose own comment explicitly documents why that matters: mosquitto_pub has no -W/wait-timeout option, so an unbounded call can hang on a TCP connect that stalls rather than being refused (interview-watch.sh:222-232). In discovery-guard.sh this call sits inside the main while true; do … done loop with nothing else guarding it — if it hangs, the loop never reaches its next sleep "$CHECK_INTERVAL", and the entire watchdog silently stops functioning until the pod is restarted. This is precisely the failure mode the guard exists to route around (broker instability around a restart), so a network hiccup at the moment the guard tries to act could permanently disable it with no log output. Wrapping with timeout "${PUB_TIMEOUT:-10}" mosquitto_pub … (as interview-watch.sh already does) would close this gap cheaply. The retained() helper (:300-307) is comparatively lower-risk since -W "$READ_TIMEOUT" at least bounds the post-connect wait, though I could not confirm from the checkout whether -W also bounds a stalled TCP connect phase (would need mosquitto_sub's own source/docs to settle, which aren't in this repo) — worth the same timeout wrap if in doubt.
zigbee2mqtt-discovery-guard.sh:285 (script header) states "Worst-case time-to-repair is CHECK_INTERVAL + CONFIRM_DELAY" (≈390s with defaults). That's the detection latency; actual repair also has to wait for Z2M's own restart + discover() republish, which the PR body's own precedent (interview-watch.sh findings) puts at low tens of seconds. Minor — doesn't affect behavior, just the comment slightly understates end-to-end recovery time.
🟢 Nits
The three near-duplicate explanations of "why read homeassistant/status doesn't buy discovery re-publish" (in mosquitto-bootstrap.yaml:27-31, mosquitto-deployment.yaml:114-123, and mosquitto-config.yaml:16-25) are consistent with each other, but it's a lot of the same fact restated three times across files that will need to be kept in sync if the underlying Z2M behavior claim is ever revised (e.g. on a future Z2M upgrade). Not asking for a change — just flagging where future drift could creep in.
Tests
No automated tests exist for this class of change in the repo (matching the existing interview-watch sidecar, which is also untested beyond CI's static checks). CI will run kustomize-build (catches a bad resource reference/patch) and shellcheck -S error against the new .sh file automatically per .woodpecker/ci-manifests-test.yml — I read the script closely for quoting/unbound-variable issues under set -eu and found nothing that should trip shellcheck at error severity. Manual verification described in the PR body covers the detection path against the live broker in its current (non-wiped) healthy state ("bridge/state=online, bridge/groups present → healthy, no action"), but not an actual wipe-and-recover cycle or the restart-trigger/cooldown path — those remain unexercised until the first real broker restart (or a deliberate maintenance-window test) after merge. Given this sidecar exists specifically to prevent a repeat of a ~14h outage, I'd suggest validating the full cycle once (temporarily lower CHECK_INTERVAL/CONFIRM_DELAY, restart mosquitto, confirm the guard logs detection → confirm → restart-request and Z2M discovery actually reappears in HA) rather than relying solely on the next real incident to prove it works.
Recommendation
Looks reasonable — no blocking issues; comments above are optional.
🤖 **Automated review** — model `CLI default` (complexity: complex), commit `5c856af578d85b38ebe296cd8fb2a4f3520a4019`.
> Advisory review by the `review-bot` pipeline. It never approves; a human merges. Push new commits to get a fresh review.
---
## Summary
Adds a `discovery-guard` sidecar to the Zigbee2MQTT pod that polls `bridge/state`/`bridge/groups` on the mosquitto broker and publishes `bridge/request/restart` when it looks like the broker's retained store was wiped (fixing #381 — Z2M discovery configs don't survive a broker restart because Z2M itself never re-publishes them). The change is exclusively new sidecar wiring plus doc/ACL-comment corrections; no existing logic paths are touched. The detection/action logic in `zigbee2mqtt-discovery-guard.sh` is careful and internally consistent (startup-race and QoS-0 false positives are explicitly handled via `CONFIRM_DELAY`, restart storms via `COOLDOWN`), and the surrounding YAML wiring (ConfigMap, volumes, securityContext, ACL scope) mirrors the existing `interview-watch` sidecar pattern closely and correctly. I did not find a blocking defect. One real robustness gap is worth fixing before/soon after merge (see Suggestions).
## 🟡 Suggestions
- `kubernetes/apps/home-assistant/zigbee2mqtt-discovery-guard.sh:351-353` — the restart-triggering `mosquitto_pub` call is not bounded by `timeout`, unlike the sibling `mosquitto_pub` call in `zigbee2mqtt-interview-watch.sh:233` (added in this same PR family), whose own comment explicitly documents *why* that matters: `mosquitto_pub` has no `-W`/wait-timeout option, so an unbounded call can hang on a TCP connect that stalls rather than being refused (`interview-watch.sh:222-232`). In `discovery-guard.sh` this call sits inside the main `while true; do … done` loop with nothing else guarding it — if it hangs, the loop never reaches its next `sleep "$CHECK_INTERVAL"`, and the entire watchdog silently stops functioning until the pod is restarted. This is precisely the failure mode the guard exists to route around (broker instability around a restart), so a network hiccup at the moment the guard tries to act could permanently disable it with no log output. Wrapping with `timeout "${PUB_TIMEOUT:-10}" mosquitto_pub …` (as `interview-watch.sh` already does) would close this gap cheaply. The `retained()` helper (`:300-307`) is comparatively lower-risk since `-W "$READ_TIMEOUT"` at least bounds the post-connect wait, though I could not confirm from the checkout whether `-W` also bounds a stalled TCP connect phase (would need mosquitto_sub's own source/docs to settle, which aren't in this repo) — worth the same `timeout` wrap if in doubt.
- `zigbee2mqtt-discovery-guard.sh:285` (script header) states "Worst-case time-to-repair is CHECK_INTERVAL + CONFIRM_DELAY" (≈390s with defaults). That's the detection latency; actual repair also has to wait for Z2M's own restart + `discover()` republish, which the PR body's own precedent (`interview-watch.sh` findings) puts at low tens of seconds. Minor — doesn't affect behavior, just the comment slightly understates end-to-end recovery time.
## 🟢 Nits
- The three near-duplicate explanations of "why `read homeassistant/status` doesn't buy discovery re-publish" (in `mosquitto-bootstrap.yaml:27-31`, `mosquitto-deployment.yaml:114-123`, and `mosquitto-config.yaml:16-25`) are consistent with each other, but it's a lot of the same fact restated three times across files that will need to be kept in sync if the underlying Z2M behavior claim is ever revised (e.g. on a future Z2M upgrade). Not asking for a change — just flagging where future drift could creep in.
## Tests
No automated tests exist for this class of change in the repo (matching the existing `interview-watch` sidecar, which is also untested beyond CI's static checks). CI will run `kustomize-build` (catches a bad resource reference/patch) and `shellcheck -S error` against the new `.sh` file automatically per `.woodpecker/ci-manifests-test.yml` — I read the script closely for quoting/unbound-variable issues under `set -eu` and found nothing that should trip shellcheck at `error` severity. Manual verification described in the PR body covers the detection path against the live broker in its current (non-wiped) healthy state ("bridge/state=online, bridge/groups present → healthy, no action"), but not an actual wipe-and-recover cycle or the restart-trigger/cooldown path — those remain unexercised until the first real broker restart (or a deliberate maintenance-window test) after merge. Given this sidecar exists specifically to prevent a repeat of a ~14h outage, I'd suggest validating the full cycle once (temporarily lower `CHECK_INTERVAL`/`CONFIRM_DELAY`, restart mosquitto, confirm the guard logs detection → confirm → restart-request and Z2M discovery actually reappears in HA) rather than relying solely on the next real incident to prove it works.
## Recommendation
Looks reasonable — no blocking issues; comments above are optional.
<!-- pr-review:sha=5c856af578d85b38ebe296cd8fb2a4f3520a4019 -->
Review catch on #516: the restart-triggering mosquitto_pub was unbounded,
unlike the sibling call in zigbee2mqtt-interview-watch.sh.
Neither client bounds a TCP connect that HANGS rather than being refused, and a
stalled connect is most likely precisely when this guard runs -- around a broker
restart. Unbounded, the loop would block forever and stop logging, which is the
silent-failure mode the guard exists to prevent.
Applied to the reads as well, not just the publish: mosquitto_sub -W bounds the
wait for a message once CONNECTED, so it does not cover the connect phase
either. The timeout wrapper is set above -W so the protocol-level wait still
wins in the normal case.
Not mosquitto_pub -W: that flag does not exist for mosquitto_pub and makes every
publish fail on a usage error.
The cooldown stamp was already written only on a successful publish, so a failed
restart request still retries on the next cycle rather than burning the cooldown.
`timeout` is confirmed present in eclipse-mosquitto:2.0.22 (already used by
interview-watch, and exercised directly in this image).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
🤖Automated review — model CLI default (complexity: complex), commit da0f895b6d271b9605267604ebdd6ad7c40ccdb8.
Advisory review by the review-bot pipeline. It never approves; a human merges. Push new commits to get a fresh review.
Summary
This PR adds a discovery-guard sidecar to the Zigbee2MQTT deployment that detects when the mosquitto broker's retained store has been wiped (via bridge/state present + bridge/groups absent) and triggers a Z2M restart to re-run HA discovery, plus corrects several comments that previously (incorrectly) claimed Z2M "self-heals" discovery via HA's birth message. The change is documentation-heavy but the logic itself is sound: I traced the detection/confirm/cooldown state machine in zigbee2mqtt-discovery-guard.sh line by line and could not find a case where it produces a false trigger, hangs, or fails to clean up after itself. ACL scoping (readwrite zigbee2mqtt/# already covers bridge/state, bridge/groups, and bridge/request/restart), the MQTT_HOST/MQTT_USER/BASE_TOPIC defaults (verified against zigbee2mqtt-config.yaml: server: mqtt://mosquitto:1883, user: zigbee2mqtt, base_topic: zigbee2mqtt), the timeout-wrapped MQTT calls, and the ConfigMap/volume wiring all check out. No blocking issues found.
🟡 Suggestions
zigbee2mqtt-deployment.yaml:281 and zigbee2mqtt-discovery-guard.sh:341 describe worst-case repair time as "~6 min". The actual worst case is CHECK_INTERVAL (up to 300s, since detection only happens on the poll tick) + CONFIRM_DELAY (90s) + the ~60s Z2M restart itself mentioned elsewhere in the PR body ≈ 7.5 min. Not a functional issue, just slightly optimistic in the comment — worth tightening or dropping the specific number.
The guard's correctness rests on claims about Z2M 2.12.1's internal behavior (discover() only from start(), bridge/groups only republished on deliberate admin actions, publishGroups() always emitting a non-empty []) that I can't verify from this repo since the Z2M source isn't checked in here. The PR body says this was checked against a live broker (bridge/groups present → no false positive), which is good, but that only exercises the healthy-state path, not the actual wiped-then-detected-then-restarts path end-to-end. If that hasn't been done yet, doing one real dry run (kill the broker's retained store, e.g. restart mosquitto, and watch the guard fire and repair discovery) would be the strongest possible confirmation before relying on this in the next incident.
Tests
No automated tests are added for the guard's runtime logic, consistent with the repo's existing convention for these sidecar scripts (zigbee2mqtt-interview-watch.sh also has none) — coverage instead comes from .woodpecker/ci-manifests-test.yml's kustomize-build (renders the kustomization, would catch a bad ConfigMap/volume reference) and shellcheck -S error (would catch shell errors) steps, both of which apply to this change automatically since it touches kubernetes/**. That's adequate for this class of change in this repo; the one gap is the end-to-end "does it actually repair a wiped broker" path noted above, which by nature can't be exercised by static CI.
Recommendation
Looks reasonable — no blocking issues; comments above are optional.
🤖 **Automated review** — model `CLI default` (complexity: complex), commit `da0f895b6d271b9605267604ebdd6ad7c40ccdb8`.
> Advisory review by the `review-bot` pipeline. It never approves; a human merges. Push new commits to get a fresh review.
---
## Summary
This PR adds a `discovery-guard` sidecar to the Zigbee2MQTT deployment that detects when the mosquitto broker's retained store has been wiped (via `bridge/state` present + `bridge/groups` absent) and triggers a Z2M restart to re-run HA discovery, plus corrects several comments that previously (incorrectly) claimed Z2M "self-heals" discovery via HA's birth message. The change is documentation-heavy but the logic itself is sound: I traced the detection/confirm/cooldown state machine in `zigbee2mqtt-discovery-guard.sh` line by line and could not find a case where it produces a false trigger, hangs, or fails to clean up after itself. ACL scoping (`readwrite zigbee2mqtt/#` already covers `bridge/state`, `bridge/groups`, and `bridge/request/restart`), the `MQTT_HOST`/`MQTT_USER`/`BASE_TOPIC` defaults (verified against `zigbee2mqtt-config.yaml`: `server: mqtt://mosquitto:1883`, `user: zigbee2mqtt`, `base_topic: zigbee2mqtt`), the `timeout`-wrapped MQTT calls, and the ConfigMap/volume wiring all check out. No blocking issues found.
## 🟡 Suggestions
- `zigbee2mqtt-deployment.yaml:281` and `zigbee2mqtt-discovery-guard.sh:341` describe worst-case repair time as "~6 min". The actual worst case is `CHECK_INTERVAL` (up to 300s, since detection only happens on the poll tick) + `CONFIRM_DELAY` (90s) + the ~60s Z2M restart itself mentioned elsewhere in the PR body ≈ 7.5 min. Not a functional issue, just slightly optimistic in the comment — worth tightening or dropping the specific number.
- The guard's correctness rests on claims about Z2M 2.12.1's internal behavior (`discover()` only from `start()`, `bridge/groups` only republished on deliberate admin actions, `publishGroups()` always emitting a non-empty `[]`) that I can't verify from this repo since the Z2M source isn't checked in here. The PR body says this was checked against a live broker (`bridge/groups` present → no false positive), which is good, but that only exercises the healthy-state path, not the actual wiped-then-detected-then-restarts path end-to-end. If that hasn't been done yet, doing one real dry run (kill the broker's retained store, e.g. restart mosquitto, and watch the guard fire and repair discovery) would be the strongest possible confirmation before relying on this in the next incident.
## Tests
No automated tests are added for the guard's runtime logic, consistent with the repo's existing convention for these sidecar scripts (`zigbee2mqtt-interview-watch.sh` also has none) — coverage instead comes from `.woodpecker/ci-manifests-test.yml`'s `kustomize-build` (renders the kustomization, would catch a bad ConfigMap/volume reference) and `shellcheck -S error` (would catch shell errors) steps, both of which apply to this change automatically since it touches `kubernetes/**`. That's adequate for this class of change in this repo; the one gap is the end-to-end "does it actually repair a wiped broker" path noted above, which by nature can't be exercised by static CI.
## Recommendation
Looks reasonable — no blocking issues; comments above are optional.
<!-- pr-review:sha=da0f895b6d271b9605267604ebdd6ad7c40ccdb8 -->
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
review-bot
Closes #381.
Root cause — the issue's premise was wrong
The issue records that replaying HA's birth message (
homeassistant/status=online) empirically did not trigger a discovery re-publish, and treats that as unexplained. It is fully explained. Verified against Zigbee2MQTT 2.12.1 source (the running version):homeassistant/status=onlinehomeassistant.tsonMQTTMessagerepublishesbridge/state+ cached device states after a hardcoded 30 s delay — never the discovery configsmqtt.tsonConnectrepublishesbridge/stateand resubscribes.republishRetainedTimeris armed only insideconnect()and never re-armedbridge/requestrediscovery endpoint exists. Upstream declined three times: Koenkk/zigbee2mqtt #27458, #22977, #16950 (all NOT_PLANNED)retain: true; the only knob is the globalforce_disable_retain, which upstream documents as breaking the HA integrationdiscover()runs only fromHomeAssistant.start(). Broker retention is the sole mechanism Z2M relies on, so a mosquitto restart not followed by a Z2M restart leaves every Z2M entity unavailable in HA indefinitely.The ACL was never the blocker —
read homeassistant/statusis already granted. That grant is still correct (the cached-state republish is what makes HA show current values after an HA restart), so it is kept and the comment now says what it actually buys.persistence trueis not on the table — this broker deliberately has no PVC. Unchanged.The fix
A
discovery-guardsidecar in the Z2M pod.Detection.
bridge/stateandbridge/{info,devices,groups,definitions}are all retained, but onlybridge/stateis republished on reconnect. Sobridge/statepresent +bridge/groupsabsent means: Z2M is talking to this broker right now, but nothing has re-runstart()since the retained store was wiped.bridge/groupsspecifically, because the absent-half partner must not be re-publishable by ordinary runtime events — otherwise it self-heals and the guard goes blind while HA is still broken:bridge/devices/bridge/definitions/bridge/info— all reach inbound radio traffic viadevicesChanged(receive.tshandsdeviceExposesChangedto everyfromZigbeeconverter). Unusable.bridge/health— self-heals on its own 10 min timer. Unusable.bridge/groups— only republished by deliberate admin actions (group membership, group add/remove/rename, and scenes, which come solely from an explicit<device>/setcommand).publishGroups()always publishes, emitting[]with no groups defined — confirmed live, it currently holds a 2-byte payload, so presence/absence is testable.Deliberately not AND-ed with
bridge/infoabsent. That would cut false positives but introduce false negatives, and the costs are asymmetric: a false positive is one ~60 s Z2M restart; a false negative is every Zigbee entity dead until a human notices. Tuned for sensitivity.Debounce. A 90 s confirm re-check covers the two known spurious-positive paths: the startup race (
bridge/stateis published atmqtt.connect(),bridge/groupsonly at the end ofBridgeExtension.start()) and a dropped publish (these topics go out at QoS 0). A 30 min cooldown stops a Z2M that will not come up healthy from being restarted in a loop.Action.
bridge/request/restartis a genuine full restart, not herdsman-only:index.jsrestart()→stop(true)tears down every extension, thenstart()builds a newControllerand re-runsHomeAssistant.start().discover()then republishes only missing/changed configs — after a wipe, all of them.Credentials. Reuses the existing
zigbee2mqttbroker user (readwrite zigbee2mqtt/#). No new broker user, and no read grant onhomeassistant/#— checking the discovery configs directly would have needed one, which the ACL denies on purpose.Verification
kubectl kustomizerenders clean.eclipse-mosquitto:2.0.22runtime image:mosquitto_sub-C/-W,mosquitto_pub,date -u +%sall present;sh -nclean; startup run logs correctly and enters the loop.bridge/state={"state":"online"},bridge/groupspresent → correctly reports healthy, no action. No false positive.Reviewer note — this rolls mosquitto
The corrected ACL comment lives inside the container command, so it changes the rendered Deployment and rolls the broker. Because
maxUnavailable: 0/maxSurge: 1still brings up a new pod with an empty retained store, that roll wipes discovery — the guard then repairs it within ~6 min, if the Z2M roll (which also restarts Z2M) has not already done so.The first commit is comment-only and rolls nothing; the two deployment comments there are plain YAML comments outside the container command, verified byte-identical in the render.
🤖 Generated with Claude Code
mosquitto runs `persistence false` with no PVC (deliberate and settled), so every broker restart wipes the retained store. Zigbee2MQTT publishes its HA discovery configs retained and has no way to restore them itself, so a broker restart not followed by a Z2M restart leaves every Z2M entity unavailable in HA indefinitely -- ~14 h on 2026-07-23. Adds a discovery-guard sidecar to the Z2M pod that detects the wiped state and asks Z2M to restart, which re-runs discover(). Detection. bridge/state and bridge/{info,devices,groups,definitions} are all retained, but only bridge/state is republished on reconnect (mqtt.ts onConnect). So "bridge/state present + bridge/groups absent" means Z2M is talking to this broker but nothing has re-run start() since the store was wiped. bridge/groups specifically, because the absent-half partner must not be re-publishable by ordinary runtime events or it self-heals and the guard goes blind while HA is still broken. bridge/devices, bridge/definitions and bridge/info all reach inbound radio traffic via devicesChanged (receive.ts hands deviceExposesChanged to every fromZigbee converter); bridge/health self-heals on its own 10 min timer. bridge/groups is only republished by deliberate admin actions, and publishGroups() always publishes, emitting `[]` when no groups exist -- confirmed live: it currently holds a 2-byte payload, so presence is testable. Not AND-ed with bridge/info being absent. That would cut false positives at the cost of false negatives, and the costs are asymmetric: a false positive is one ~60s Z2M restart, a false negative is every Zigbee entity dead until a human notices. Tuned for sensitivity. Spurious positives are handled by a 90s confirm re-check, which covers both the startup race (bridge/state is published at mqtt.connect(), bridge/groups only at the end of BridgeExtension.start()) and a dropped publish (these topics go out at QoS 0). A 30 min cooldown keeps a Z2M that will not come up healthy from being restarted in a loop. Action is bridge/request/restart, which is a full in-process restart -- index.js restart() -> stop(true) tears down every extension, then start() builds a new Controller and re-runs HomeAssistant.start(). discover() then republishes only missing or changed configs, which after a wipe is all of them. Reuses the existing zigbee2mqtt broker credential (ACL `readwrite zigbee2mqtt/#`), so no new broker user and no read grant on homeassistant/#, which the ACL denies on purpose. Verified in the eclipse-mosquitto:2.0.22 image the sidecar actually runs: mosquitto_sub -C/-W, mosquitto_pub and `date -u +%s` all present; `sh -n` clean; startup run logs correctly and enters the loop. Detection was exercised against the live broker and correctly reports healthy (bridge/state online, bridge/groups present) with no action. NOTE: this rolls mosquitto, because the corrected ACL comment lives inside the container command. That roll wipes the retained store, which the guard then repairs within ~6 min if the Z2M roll does not already do so. Closes #381 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>🤖 Automated review — model
CLI default(complexity: complex), commit5c856af578d85b38ebe296cd8fb2a4f3520a4019.Summary
Adds a
discovery-guardsidecar to the Zigbee2MQTT pod that pollsbridge/state/bridge/groupson the mosquitto broker and publishesbridge/request/restartwhen it looks like the broker's retained store was wiped (fixing #381 — Z2M discovery configs don't survive a broker restart because Z2M itself never re-publishes them). The change is exclusively new sidecar wiring plus doc/ACL-comment corrections; no existing logic paths are touched. The detection/action logic inzigbee2mqtt-discovery-guard.shis careful and internally consistent (startup-race and QoS-0 false positives are explicitly handled viaCONFIRM_DELAY, restart storms viaCOOLDOWN), and the surrounding YAML wiring (ConfigMap, volumes, securityContext, ACL scope) mirrors the existinginterview-watchsidecar pattern closely and correctly. I did not find a blocking defect. One real robustness gap is worth fixing before/soon after merge (see Suggestions).🟡 Suggestions
kubernetes/apps/home-assistant/zigbee2mqtt-discovery-guard.sh:351-353— the restart-triggeringmosquitto_pubcall is not bounded bytimeout, unlike the siblingmosquitto_pubcall inzigbee2mqtt-interview-watch.sh:233(added in this same PR family), whose own comment explicitly documents why that matters:mosquitto_pubhas no-W/wait-timeout option, so an unbounded call can hang on a TCP connect that stalls rather than being refused (interview-watch.sh:222-232). Indiscovery-guard.shthis call sits inside the mainwhile true; do … doneloop with nothing else guarding it — if it hangs, the loop never reaches its nextsleep "$CHECK_INTERVAL", and the entire watchdog silently stops functioning until the pod is restarted. This is precisely the failure mode the guard exists to route around (broker instability around a restart), so a network hiccup at the moment the guard tries to act could permanently disable it with no log output. Wrapping withtimeout "${PUB_TIMEOUT:-10}" mosquitto_pub …(asinterview-watch.shalready does) would close this gap cheaply. Theretained()helper (:300-307) is comparatively lower-risk since-W "$READ_TIMEOUT"at least bounds the post-connect wait, though I could not confirm from the checkout whether-Walso bounds a stalled TCP connect phase (would need mosquitto_sub's own source/docs to settle, which aren't in this repo) — worth the sametimeoutwrap if in doubt.zigbee2mqtt-discovery-guard.sh:285(script header) states "Worst-case time-to-repair is CHECK_INTERVAL + CONFIRM_DELAY" (≈390s with defaults). That's the detection latency; actual repair also has to wait for Z2M's own restart +discover()republish, which the PR body's own precedent (interview-watch.shfindings) puts at low tens of seconds. Minor — doesn't affect behavior, just the comment slightly understates end-to-end recovery time.🟢 Nits
read homeassistant/statusdoesn't buy discovery re-publish" (inmosquitto-bootstrap.yaml:27-31,mosquitto-deployment.yaml:114-123, andmosquitto-config.yaml:16-25) are consistent with each other, but it's a lot of the same fact restated three times across files that will need to be kept in sync if the underlying Z2M behavior claim is ever revised (e.g. on a future Z2M upgrade). Not asking for a change — just flagging where future drift could creep in.Tests
No automated tests exist for this class of change in the repo (matching the existing
interview-watchsidecar, which is also untested beyond CI's static checks). CI will runkustomize-build(catches a bad resource reference/patch) andshellcheck -S erroragainst the new.shfile automatically per.woodpecker/ci-manifests-test.yml— I read the script closely for quoting/unbound-variable issues underset -euand found nothing that should trip shellcheck aterrorseverity. Manual verification described in the PR body covers the detection path against the live broker in its current (non-wiped) healthy state ("bridge/state=online, bridge/groups present → healthy, no action"), but not an actual wipe-and-recover cycle or the restart-trigger/cooldown path — those remain unexercised until the first real broker restart (or a deliberate maintenance-window test) after merge. Given this sidecar exists specifically to prevent a repeat of a ~14h outage, I'd suggest validating the full cycle once (temporarily lowerCHECK_INTERVAL/CONFIRM_DELAY, restart mosquitto, confirm the guard logs detection → confirm → restart-request and Z2M discovery actually reappears in HA) rather than relying solely on the next real incident to prove it works.Recommendation
Looks reasonable — no blocking issues; comments above are optional.
🤖 Automated review — model
CLI default(complexity: complex), commitda0f895b6d271b9605267604ebdd6ad7c40ccdb8.Summary
This PR adds a
discovery-guardsidecar to the Zigbee2MQTT deployment that detects when the mosquitto broker's retained store has been wiped (viabridge/statepresent +bridge/groupsabsent) and triggers a Z2M restart to re-run HA discovery, plus corrects several comments that previously (incorrectly) claimed Z2M "self-heals" discovery via HA's birth message. The change is documentation-heavy but the logic itself is sound: I traced the detection/confirm/cooldown state machine inzigbee2mqtt-discovery-guard.shline by line and could not find a case where it produces a false trigger, hangs, or fails to clean up after itself. ACL scoping (readwrite zigbee2mqtt/#already coversbridge/state,bridge/groups, andbridge/request/restart), theMQTT_HOST/MQTT_USER/BASE_TOPICdefaults (verified againstzigbee2mqtt-config.yaml:server: mqtt://mosquitto:1883,user: zigbee2mqtt,base_topic: zigbee2mqtt), thetimeout-wrapped MQTT calls, and the ConfigMap/volume wiring all check out. No blocking issues found.🟡 Suggestions
zigbee2mqtt-deployment.yaml:281andzigbee2mqtt-discovery-guard.sh:341describe worst-case repair time as "~6 min". The actual worst case isCHECK_INTERVAL(up to 300s, since detection only happens on the poll tick) +CONFIRM_DELAY(90s) + the ~60s Z2M restart itself mentioned elsewhere in the PR body ≈ 7.5 min. Not a functional issue, just slightly optimistic in the comment — worth tightening or dropping the specific number.discover()only fromstart(),bridge/groupsonly republished on deliberate admin actions,publishGroups()always emitting a non-empty[]) that I can't verify from this repo since the Z2M source isn't checked in here. The PR body says this was checked against a live broker (bridge/groupspresent → no false positive), which is good, but that only exercises the healthy-state path, not the actual wiped-then-detected-then-restarts path end-to-end. If that hasn't been done yet, doing one real dry run (kill the broker's retained store, e.g. restart mosquitto, and watch the guard fire and repair discovery) would be the strongest possible confirmation before relying on this in the next incident.Tests
No automated tests are added for the guard's runtime logic, consistent with the repo's existing convention for these sidecar scripts (
zigbee2mqtt-interview-watch.shalso has none) — coverage instead comes from.woodpecker/ci-manifests-test.yml'skustomize-build(renders the kustomization, would catch a bad ConfigMap/volume reference) andshellcheck -S error(would catch shell errors) steps, both of which apply to this change automatically since it toucheskubernetes/**. That's adequate for this class of change in this repo; the one gap is the end-to-end "does it actually repair a wiped broker" path noted above, which by nature can't be exercised by static CI.Recommendation
Looks reasonable — no blocking issues; comments above are optional.