SAFETY-34 W4. A Core-only HA restart wipes persistent_notification and does not restart the add-on. The latched SAFETY-31 and SAFETY-42 cards were only re-raised when the add-on started, so after a Core restart they stayed gone.
Before this change (TEST-23, image 16c796f): I injected a SAFETY-31 latch and restarted test23-ha only. The add-on's StartedAt did not change, and the card was still gone after 4 min.
Change: while a latch is tripped, its card is re-asserted every REANNOUNCE_S = 300 s. It uses the same notification_id, so it replaces the old card rather than stacking. The docstrings and test comments that said "the add-on restarts with HA" are corrected.
Evidence
test_watchdog.py part 6 and test_delivery.py part 8b keep the same process across the "restart" and check two things: no re-raise inside 300 s, and exactly one re-raise after it.
Both go red on the old code, and red on a "re-assert every cycle" mutant, which trips the existing no-storm checks.
The full test_*.py suite passes in goodwe-addon-checks:1.
On TEST-23, with this commit built as a temporary image: after the HA restart the card was back 4 m 11 s after the API came up, and the add-on process was never restarted.
test23-addon was restored to 16c796f with its original /data/state.json afterwards.
No CHANGELOG entry. The release cut records this, as it did for TEL-14.
The reasoning is in goodwe docs/DECISIONS.md, SAFETY-34 D4.
SAFETY-34 W4. A Core-only HA restart wipes `persistent_notification` and does **not** restart the add-on. The latched SAFETY-31 and SAFETY-42 cards were only re-raised when the add-on started, so after a Core restart they stayed gone.
**Before this change (TEST-23, image `16c796f`):** I injected a SAFETY-31 latch and restarted `test23-ha` only. The add-on's `StartedAt` did not change, and the card was still gone after 4 min.
**Change:** while a latch is tripped, its card is re-asserted every `REANNOUNCE_S` = 300 s. It uses the same `notification_id`, so it replaces the old card rather than stacking. The docstrings and test comments that said "the add-on restarts with HA" are corrected.
**Evidence**
- `test_watchdog.py` part 6 and `test_delivery.py` part 8b keep the same process across the "restart" and check two things: no re-raise inside 300 s, and exactly one re-raise after it.
- Both go red on the old code, and red on a "re-assert every cycle" mutant, which trips the existing no-storm checks.
- The full `test_*.py` suite passes in `goodwe-addon-checks:1`.
- On TEST-23, with this commit built as a temporary image: after the HA restart the card was back 4 m 11 s after the API came up, and the add-on process was never restarted.
- `test23-addon` was restored to `16c796f` with its original `/data/state.json` afterwards.
**No CHANGELOG entry.** The release cut records this, as it did for TEL-14.
The reasoning is in goodwe `docs/DECISIONS.md`, SAFETY-34 D4.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01LyR88rXETvwhTgx6yHE39p
A Core-only restart wipes persistent_notification and does NOT restart the
add-on (measured on TEST-23: the add-on's SAFETY-31 card was gone 4 min after
`docker restart test23-ha`, add-on StartedAt unchanged). The latched SAFETY-31
and SAFETY-42 cards were re-raised only on add-on start, so they stayed gone.
They are now re-asserted every REANNOUNCE_S = 300 s while tripped (same
notification_id, so it replaces). test_watchdog part 6 and test_delivery part
8b grade it with the same process across the "restart"; red on the old code.
The docstrings that said the add-on restarts with HA are corrected.
Record: goodwe docs/DECISIONS.md, SAFETY-34 D4.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LyR88rXETvwhTgx6yHE39p
I read the diff and ran test_watchdog.py and test_delivery.py in a throwaway --network none container from the test23-addon image. They pass on 6028e21.
A1. It re-posts only the latched alarms: OK. Only check_circulating (SAFETY-31) and check_not_delivering (SAFETY-42) consult _reannounce_due. The episode notifications are untouched: output_clamp, peak_saturated, peak_soc_floor and meter_frozen.
A2. No spam on every tick: OK, and pinned. I mutated _reannounce_due to always return True. Both files went red: test_watchdog had 4 failures, including the pre-existing "does not raise the same card on every cycle", and test_delivery had 3. The rate is one idempotent create per latched id every 300 s, and only while the site is tripped.
A3. It re-posts after a Core-only restart without an add-on restart: OK, and pinned. On the base logic, with _mono and REANNOUNCE_S shimmed in so the tests import, exactly the two "after REANNOUNCE_S the SAME process re-asserts" checks fail and everything else passes. So the tests grade the behaviour, not just the new symbols. The TEST-23 figure (card back 4 m 11 s after the API came up, add-on process not restarted) is consistent with the 300 s cadence.
A4. Clear path: OK. Operator START still resets _circ_announced and _delivery_announced, so a later fresh trip announces immediately and does not wait out a stale _announced_mono entry.
A5. Noted, not blocking: by design, a dismissed card also comes back every 5 min until START. That suits a latched alarm that only a human may clear. Worst-case re-post latency after a Core restart is about 300 s plus HA start-up, against about 66 s for the witness alarm. I accept that for this card. A websocket reconnect or homeassistant_started hook would be faster, but that is a follow-up candidate, not scope here.
A6. Nit, not blocking. The reflowed comment in test_watchdog.py:1058-1059 runs past 110 characters.
No blocking findings. I will merge after the goodwe#58 merge, with the head pinned.
## Review (SAFETY-34 W4), round 1
I read the diff and ran `test_watchdog.py` and `test_delivery.py` in a throwaway `--network none` container from the test23-addon image. They pass on 6028e21.
**A1. It re-posts only the latched alarms: OK.** Only `check_circulating` (SAFETY-31) and `check_not_delivering` (SAFETY-42) consult `_reannounce_due`. The episode notifications are untouched: output_clamp, peak_saturated, peak_soc_floor and meter_frozen.
**A2. No spam on every tick: OK, and pinned.** I mutated `_reannounce_due` to always return True. Both files went red: test_watchdog had 4 failures, including the pre-existing "does not raise the same card on every cycle", and test_delivery had 3. The rate is one idempotent `create` per latched id every 300 s, and only while the site is tripped.
**A3. It re-posts after a Core-only restart without an add-on restart: OK, and pinned.** On the base logic, with `_mono` and `REANNOUNCE_S` shimmed in so the tests import, exactly the two "after REANNOUNCE_S the SAME process re-asserts" checks fail and everything else passes. So the tests grade the behaviour, not just the new symbols. The TEST-23 figure (card back 4 m 11 s after the API came up, add-on process not restarted) is consistent with the 300 s cadence.
**A4. Clear path: OK.** Operator START still resets `_circ_announced` and `_delivery_announced`, so a later fresh trip announces immediately and does not wait out a stale `_announced_mono` entry.
**A5. Noted, not blocking: by design, a dismissed card also comes back every 5 min until START.** That suits a latched alarm that only a human may clear. Worst-case re-post latency after a Core restart is about 300 s plus HA start-up, against about 66 s for the witness alarm. I accept that for this card. A websocket reconnect or `homeassistant_started` hook would be faster, but that is a follow-up candidate, not scope here.
**A6. Nit, not blocking.** The reflowed comment in `test_watchdog.py:1058-1059` runs past 110 characters.
No blocking findings. I will merge after the goodwe#58 merge, with the head pinned.
No blocking findings (A1-A4 OK, A5/A6 noted). CI: unit 883 and infra-pin 884 green on 6028e21. goodwe#58 is merged (release/1.0 7689816), so this repo's .gitea/infra-pin (a1d0618) now needs a bump; that bump is dispatched separately. Merging with head_commit_id pinned.
No blocking findings (A1-A4 OK, A5/A6 noted). CI: unit 883 and infra-pin 884 green on 6028e21. goodwe#58 is merged (release/1.0 7689816), so this repo's .gitea/infra-pin (a1d0618) now needs a bump; that bump is dispatched separately. Merging with head_commit_id pinned.
admin
merged commit 397dd15dba into release/1.02026-09-26 16:03:17 +02:00
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.
SAFETY-34 W4. A Core-only HA restart wipes
persistent_notificationand does not restart the add-on. The latched SAFETY-31 and SAFETY-42 cards were only re-raised when the add-on started, so after a Core restart they stayed gone.Before this change (TEST-23, image
16c796f): I injected a SAFETY-31 latch and restartedtest23-haonly. The add-on'sStartedAtdid not change, and the card was still gone after 4 min.Change: while a latch is tripped, its card is re-asserted every
REANNOUNCE_S= 300 s. It uses the samenotification_id, so it replaces the old card rather than stacking. The docstrings and test comments that said "the add-on restarts with HA" are corrected.Evidence
test_watchdog.pypart 6 andtest_delivery.pypart 8b keep the same process across the "restart" and check two things: no re-raise inside 300 s, and exactly one re-raise after it.test_*.pysuite passes ingoodwe-addon-checks:1.test23-addonwas restored to16c796fwith its original/data/state.jsonafterwards.No CHANGELOG entry. The release cut records this, as it did for TEL-14.
The reasoning is in goodwe
docs/DECISIONS.md, SAFETY-34 D4.🤖 Generated with Claude Code
https://claude.ai/code/session_01LyR88rXETvwhTgx6yHE39p
Review (SAFETY-34 W4), round 1
I read the diff and ran
test_watchdog.pyandtest_delivery.pyin a throwaway--network nonecontainer from the test23-addon image. They pass on6028e21.A1. It re-posts only the latched alarms: OK. Only
check_circulating(SAFETY-31) andcheck_not_delivering(SAFETY-42) consult_reannounce_due. The episode notifications are untouched: output_clamp, peak_saturated, peak_soc_floor and meter_frozen.A2. No spam on every tick: OK, and pinned. I mutated
_reannounce_dueto always return True. Both files went red: test_watchdog had 4 failures, including the pre-existing "does not raise the same card on every cycle", and test_delivery had 3. The rate is one idempotentcreateper latched id every 300 s, and only while the site is tripped.A3. It re-posts after a Core-only restart without an add-on restart: OK, and pinned. On the base logic, with
_monoandREANNOUNCE_Sshimmed in so the tests import, exactly the two "after REANNOUNCE_S the SAME process re-asserts" checks fail and everything else passes. So the tests grade the behaviour, not just the new symbols. The TEST-23 figure (card back 4 m 11 s after the API came up, add-on process not restarted) is consistent with the 300 s cadence.A4. Clear path: OK. Operator START still resets
_circ_announcedand_delivery_announced, so a later fresh trip announces immediately and does not wait out a stale_announced_monoentry.A5. Noted, not blocking: by design, a dismissed card also comes back every 5 min until START. That suits a latched alarm that only a human may clear. Worst-case re-post latency after a Core restart is about 300 s plus HA start-up, against about 66 s for the witness alarm. I accept that for this card. A websocket reconnect or
homeassistant_startedhook would be faster, but that is a follow-up candidate, not scope here.A6. Nit, not blocking. The reflowed comment in
test_watchdog.py:1058-1059runs past 110 characters.No blocking findings. I will merge after the goodwe#58 merge, with the head pinned.
No blocking findings (A1-A4 OK, A5/A6 noted). CI: unit 883 and infra-pin 884 green on
6028e21. goodwe#58 is merged (release/1.0 7689816), so this repo's .gitea/infra-pin (a1d0618) now needs a bump; that bump is dispatched separately. Merging with head_commit_id pinned.