SAFETY-61 — the control task's death ends the process
Before this change: an uncaught exception in read_inputs(), cycle() or publish() ended the only control and heartbeat task without anyone noticing. stop was never set, the web UI kept serving and the Supervisor saw a healthy container. Layer 1 then held the pack at 0 W indefinitely while the add-on stayed green.
Changes
main.py:control_task_done() is a done-callback on the control task.
It logs the traceback at CRITICAL at the moment the task dies and sets stop.
amain() then runs the ordinary wind-down (shutdown() → command_site_zero() → cancel → runner.cleanup()), and main() exits with status 70.
A SIGTERM stop and run_control()'s own refusal path are ignored by the callback. They still exit with 0 and 78.
hass.py: these responses are now treated as missing data instead of crashing the loop:
a malformed body labelled application/json (JSONDecodeError) reads as None;
a JSON body that is not an object (AttributeError in number()/limits()) reads as None;
an undecodable error body in call() (UnicodeDecodeError) counts as a failed write (False).
New status row:checks() now starts with a control loop row that shows the age of the last completed pass. It turns red once that age exceeds FIRMWARE_INGEST_VETO_S.
Design choice: why the add-on exits instead of restarting the loop in-process, and what the Supervisor does after the exit, is in DECISIONS/SAFETY-61-control-task-death.md.
What the Supervisor does after the exit (verified, not assumed)
Exit code reaches Docker: I ran SystemExit(70) through the add-on image's /init + with-contenv, and docker inspect showed ExitCode 70.
Supervisor 2026.08.0 source, read from the image:
A non-zero exit is recorded as FAILED, and the Supervisor logs it at ERROR.
It restarts the add-on only if the per-install Watchdog toggle is on, and that toggle defaults to off.
The add-on container has no Docker restart policy. boot: auto only covers starting at host boot.
Result: on a default install the add-on stops, visibly, with 0 W already written. It is not restarted.
Not checked: an actual restart by a running Supervisor. That would need the live one.
Evidence
test_control_task.py, 14 s. It drives the real app.main.main() in a child process under a 40 s deadline.
It checks: exit status 70; a 0 W write after the fault, starting from a non-zero command of 660 W; a CRITICAL line with the traceback logged before the wind-down; exactly one traceback.
Control cases: SIGTERM exits 0, and a late device-rating refusal exits 78.
It also covers the hass.py cases against a real aiohttp server, and the status row.
Pre-fix tree: the child hangs until the deadline and writes no 0 W. The fault checks and the hass.py checks are red.
Mutations: 12 of 12 red, each run on a purged copy of the tree mounted :ro.
Removing the callback (−130 B) hangs the child until the deadline, and that failure is reported under a named check.
The other 11: removing the guard, calling os._exit before the wind-down, dropping exc_info, removing main()'s handler, awaiting the dead task, the three hass.py excepts and the shape check, removing the pass stamp, the row threshold, and not recording the death.
Behaviour otherwise unchanged: an A/B run of run_control() against a scripted meter, head vs release/1.0. The 11 cycles give identical (target, reason), and the 18 writes, including heartbeats and the shutdown write, are identical. Targets ranged from −390 to +1110 W, not pinned at the limit.
Full suite: 32/32 green in goodwe-addon-checks:1, using checks.yml's exact command with a purged tree mounted :ro.
Not in scope / for the owner
DOCS.md: whether it should tell installers to turn on Watchdog. DOCS.md is not this card's file.
Other long-lived tasks are not watched. Tariff refresh and the P1 transports already catch exceptions internally, and the MQTT P1 source returns on purpose when misconfigured (see §4 of the DECISIONS file).
## SAFETY-61 — the control task's death ends the process
**Before this change:** an uncaught exception in `read_inputs()`, `cycle()` or `publish()` ended the only control and heartbeat task without anyone noticing. `stop` was never set, the web UI kept serving and the Supervisor saw a healthy container. Layer 1 then held the pack at 0 W indefinitely while the add-on stayed green.
### Changes
- **`main.py`:** `control_task_done()` is a done-callback on the control task.
- It logs the traceback at CRITICAL at the moment the task dies and sets `stop`.
- `amain()` then runs the ordinary wind-down (`shutdown()` → `command_site_zero()` → cancel → `runner.cleanup()`), and `main()` exits with status **70**.
- A SIGTERM stop and `run_control()`'s own refusal path are ignored by the callback. They still exit with 0 and 78.
- **`hass.py`:** these responses are now treated as missing data instead of crashing the loop:
- a malformed body labelled `application/json` (`JSONDecodeError`) reads as `None`;
- a JSON body that is not an object (`AttributeError` in `number()`/`limits()`) reads as `None`;
- an undecodable error body in `call()` (`UnicodeDecodeError`) counts as a failed write (`False`).
- **New status row:** `checks()` now starts with a *control loop* row that shows the age of the last completed pass. It turns red once that age exceeds `FIRMWARE_INGEST_VETO_S`.
- **Design choice:** why the add-on exits instead of restarting the loop in-process, and what the Supervisor does after the exit, is in `DECISIONS/SAFETY-61-control-task-death.md`.
### What the Supervisor does after the exit (verified, not assumed)
- **Exit code reaches Docker:** I ran `SystemExit(70)` through the add-on image's `/init` + `with-contenv`, and `docker inspect` showed ExitCode 70.
- **Supervisor 2026.08.0 source, read from the image:**
- A non-zero exit is recorded as FAILED, and the Supervisor logs it at ERROR.
- It **restarts the add-on only if the per-install Watchdog toggle is on**, and that toggle **defaults to off**.
- The add-on container has no Docker restart policy. `boot: auto` only covers starting at host boot.
- **Result:** on a default install the add-on stops, visibly, with 0 W already written. It is not restarted.
- **Not checked:** an actual restart by a running Supervisor. That would need the live one.
### Evidence
- **`test_control_task.py`, 14 s.** It drives the real `app.main.main()` in a child process under a 40 s deadline.
- It checks: exit status 70; a 0 W write after the fault, starting from a non-zero command of 660 W; a CRITICAL line with the traceback logged *before* the wind-down; exactly one traceback.
- Control cases: SIGTERM exits 0, and a late device-rating refusal exits 78.
- It also covers the `hass.py` cases against a real aiohttp server, and the status row.
- **Pre-fix tree:** the child hangs until the deadline and writes no 0 W. The fault checks and the `hass.py` checks are red.
- **Mutations:** 12 of 12 red, each run on a purged copy of the tree mounted `:ro`.
- Removing the callback (−130 B) hangs the child until the deadline, and that failure is reported under a named check.
- The other 11: removing the guard, calling `os._exit` before the wind-down, dropping `exc_info`, removing `main()`'s handler, awaiting the dead task, the three `hass.py` excepts and the shape check, removing the pass stamp, the row threshold, and not recording the death.
- **Behaviour otherwise unchanged:** an A/B run of `run_control()` against a scripted meter, head vs `release/1.0`. The 11 cycles give identical `(target, reason)`, and the 18 writes, including heartbeats and the shutdown write, are identical. Targets ranged from −390 to +1110 W, not pinned at the limit.
- **Full suite:** 32/32 green in `goodwe-addon-checks:1`, using `checks.yml`'s exact command with a purged tree mounted `:ro`.
### Not in scope / for the owner
- **DOCS.md:** whether it should tell installers to turn on Watchdog. DOCS.md is not this card's file.
- **Other long-lived tasks are not watched.** Tariff refresh and the P1 transports already catch exceptions internally, and the MQTT P1 source returns on purpose when misconfigured (see §4 of the DECISIONS file).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
A done-callback on run_control()'s task logs the traceback at the death and
sets `stop`; amain() runs the ordinary wind-down (shutdown() ->
command_site_zero()) and main() exits 70 (EX_SOFTWARE). Normal stops and
run_control()'s refusal path are ignored by the callback (still 0 / 78).
hass.py: a malformed JSON body, a JSON non-object, and an undecodable error
body are an absence / a failed write, not an exception into the loop.
checks() gains a first "control loop" row, red past FIRMWARE_INGEST_VETO_S.
Why exit rather than restart in-process, and what the Supervisor actually
does on exit (restarts only with the per-install Watchdog toggle on, default
off): DECISIONS/SAFETY-61-control-task-death.md.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
Review (SAFETY-61) - no blocking findings; merge waits only on CI run 834 for this head.
Checked on the merge of 71d2d4a into release/1.0 (fa3d237, merges clean). Everything ran in goodwe-addon-checks:1 with --network none and the source mounted :ro.
Wind-down
SIGTERM: stop is set before the cancel, so the callback returns and the exit stays 0. The refusal path sets stop before it returns, so the exit stays 78. The callback never calls command_site_zero() itself, so there is no double 0 W. Tests 2 and 3 confirm this.
Death partway through a cycle: I probed with a set_number that raises after target/reason are already updated (660 W in flight). Result: CRITICAL line, w 0.0, cleanup, exit 70. 0 W is written.
Happy path. My own A/B, independent of yours: 18 read_inputs()+cycle() passes on a varied meter sequence (including a None read and ±clamp cases), a45ed06 vs this PR. Every target/reason and all 16 writes are byte-identical.
hass.py. Every new except returns None (read) or False (write). Nothing substitutes 0. number()/limits() still return None / (None, None, None) on those reads.
Mutations (all real regressions; test_control_task.py went red on all 6):
drop stop.set() in the callback → hang, 5 FAILs
guard if stop.is_set() or controller.stopping → if False → SIGTERM exits 70, refusal exits 70 (4 FAILs)
skip shutdown() when died → no 0 W after the fault
drop ValueError from state()'s except → the malformed-JSON case raises
a non-object body read as {"state": "0"} → red
stamp loop_pass_mono once before the loop, not per pass → red
Non-blocking (N1-N3), answer or refute as you see fit:
N1. If shutdown() itself raises (probe: set_number raising on every call from the 4th on), amain() propagates that exception. runner.cleanup() is skipped, ControlTaskDied is never raised, and the process exits 1, right after a CRITICAL line that said "exiting with status 70". It is still non-zero, and 0 W is unwritable in that case anyway, so the physics is unchanged. This gap already existed for SIGTERM. Consider try: await controller.shutdown() finally: ... so cleanup and the 70 still happen. Otherwise, reword the CRITICAL line so it doesn't promise an outcome before it has happened.
N2.call(): catching ValueError around the whole block works, but the log then says "failed: 'utf-8' codec can't decode…" and loses the HTTP status. await resp.text(errors="replace") would keep the "rejected (400)" line and need no new except.
N3. Length is fine. The inline comments are one sentence plus a pointer, and the history is in DECISIONS. Nothing to trim.
Not reviewed as a blocker: whether DOCS should recommend the Watchdog toggle. That is the owner's call.
**Review (SAFETY-61) - no blocking findings; merge waits only on CI run 834 for this head.**
Checked on the merge of 71d2d4a into release/1.0 (fa3d237, merges clean). Everything ran in `goodwe-addon-checks:1` with `--network none` and the source mounted `:ro`.
**Wind-down**
- SIGTERM: `stop` is set before the cancel, so the callback returns and the exit stays 0. The refusal path sets `stop` before it returns, so the exit stays 78. The callback never calls `command_site_zero()` itself, so there is no double 0 W. Tests 2 and 3 confirm this.
- Death partway through a cycle: I probed with a `set_number` that raises after `target`/`reason` are already updated (660 W in flight). Result: CRITICAL line, `w 0.0`, cleanup, exit 70. 0 W is written.
**Happy path.** My own A/B, independent of yours: 18 `read_inputs()`+`cycle()` passes on a varied meter sequence (including a None read and ±clamp cases), a45ed06 vs this PR. Every target/reason and all 16 writes are byte-identical.
**hass.py.** Every new except returns `None` (read) or `False` (write). Nothing substitutes 0. `number()`/`limits()` still return None / (None, None, None) on those reads.
**Mutations** (all real regressions; `test_control_task.py` went red on all 6):
1. drop `stop.set()` in the callback → hang, 5 FAILs
2. guard `if stop.is_set() or controller.stopping` → `if False` → SIGTERM exits 70, refusal exits 70 (4 FAILs)
3. skip `shutdown()` when `died` → no 0 W after the fault
4. drop `ValueError` from `state()`'s except → the malformed-JSON case raises
5. a non-object body read as `{"state": "0"}` → red
6. stamp `loop_pass_mono` once before the loop, not per pass → red
**Non-blocking (N1-N3), answer or refute as you see fit:**
- **N1.** If `shutdown()` itself raises (probe: `set_number` raising on every call from the 4th on), `amain()` propagates that exception. `runner.cleanup()` is skipped, `ControlTaskDied` is never raised, and the process exits **1**, right after a CRITICAL line that said "exiting with status 70". It is still non-zero, and 0 W is unwritable in that case anyway, so the physics is unchanged. This gap already existed for SIGTERM. Consider `try: await controller.shutdown() finally: ...` so cleanup and the 70 still happen. Otherwise, reword the CRITICAL line so it doesn't promise an outcome before it has happened.
- **N2.** `call()`: catching `ValueError` around the whole block works, but the log then says "failed: 'utf-8' codec can't decode…" and loses the HTTP status. `await resp.text(errors="replace")` would keep the "rejected (400)" line and need no new except.
- **N3.** Length is fine. The inline comments are one sentence plus a pointer, and the history is in DECISIONS. Nothing to trim.
Not reviewed as a blocker: whether DOCS should recommend the Watchdog toggle. That is the owner's call.
CI run 834 is this PR's own run: PRNUM 121, BASEREF release/1.0 (fa3d237), head 71d2d4a, GRADED: refs/pull/121/head merged into release/1.0, run_attempt 1, 33/33 PASS. N1 and N2 above are non-blocking. Please pick them up as a follow-up card if you agree. Merging.
CI run 834 is this PR's own run: PRNUM 121, BASEREF release/1.0 (fa3d237), head 71d2d4a, `GRADED: refs/pull/121/head merged into release/1.0`, run_attempt 1, 33/33 PASS. N1 and N2 above are non-blocking. Please pick them up as a follow-up card if you agree. Merging.
admin
merged commit 6e6527224a into release/1.02026-09-25 00:07:42 +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-61 — the control task's death ends the process
Before this change: an uncaught exception in
read_inputs(),cycle()orpublish()ended the only control and heartbeat task without anyone noticing.stopwas never set, the web UI kept serving and the Supervisor saw a healthy container. Layer 1 then held the pack at 0 W indefinitely while the add-on stayed green.Changes
main.py:control_task_done()is a done-callback on the control task.stop.amain()then runs the ordinary wind-down (shutdown()→command_site_zero()→ cancel →runner.cleanup()), andmain()exits with status 70.run_control()'s own refusal path are ignored by the callback. They still exit with 0 and 78.hass.py: these responses are now treated as missing data instead of crashing the loop:application/json(JSONDecodeError) reads asNone;AttributeErrorinnumber()/limits()) reads asNone;call()(UnicodeDecodeError) counts as a failed write (False).checks()now starts with a control loop row that shows the age of the last completed pass. It turns red once that age exceedsFIRMWARE_INGEST_VETO_S.DECISIONS/SAFETY-61-control-task-death.md.What the Supervisor does after the exit (verified, not assumed)
SystemExit(70)through the add-on image's/init+with-contenv, anddocker inspectshowed ExitCode 70.boot: autoonly covers starting at host boot.Evidence
test_control_task.py, 14 s. It drives the realapp.main.main()in a child process under a 40 s deadline.hass.pycases against a real aiohttp server, and the status row.hass.pychecks are red.:ro.os._exitbefore the wind-down, droppingexc_info, removingmain()'s handler, awaiting the dead task, the threehass.pyexcepts and the shape check, removing the pass stamp, the row threshold, and not recording the death.run_control()against a scripted meter, head vsrelease/1.0. The 11 cycles give identical(target, reason), and the 18 writes, including heartbeats and the shutdown write, are identical. Targets ranged from −390 to +1110 W, not pinned at the limit.goodwe-addon-checks:1, usingchecks.yml's exact command with a purged tree mounted:ro.Not in scope / for the owner
🤖 Generated with Claude Code
https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
Review (SAFETY-61) - no blocking findings; merge waits only on CI run 834 for this head.
Checked on the merge of
71d2d4ainto release/1.0 (fa3d237, merges clean). Everything ran ingoodwe-addon-checks:1with--network noneand the source mounted:ro.Wind-down
stopis set before the cancel, so the callback returns and the exit stays 0. The refusal path setsstopbefore it returns, so the exit stays 78. The callback never callscommand_site_zero()itself, so there is no double 0 W. Tests 2 and 3 confirm this.set_numberthat raises aftertarget/reasonare already updated (660 W in flight). Result: CRITICAL line,w 0.0, cleanup, exit 70. 0 W is written.Happy path. My own A/B, independent of yours: 18
read_inputs()+cycle()passes on a varied meter sequence (including a None read and ±clamp cases),a45ed06vs this PR. Every target/reason and all 16 writes are byte-identical.hass.py. Every new except returns
None(read) orFalse(write). Nothing substitutes 0.number()/limits()still return None / (None, None, None) on those reads.Mutations (all real regressions;
test_control_task.pywent red on all 6):stop.set()in the callback → hang, 5 FAILsif stop.is_set() or controller.stopping→if False→ SIGTERM exits 70, refusal exits 70 (4 FAILs)shutdown()whendied→ no 0 W after the faultValueErrorfromstate()'s except → the malformed-JSON case raises{"state": "0"}→ redloop_pass_monoonce before the loop, not per pass → redNon-blocking (N1-N3), answer or refute as you see fit:
shutdown()itself raises (probe:set_numberraising on every call from the 4th on),amain()propagates that exception.runner.cleanup()is skipped,ControlTaskDiedis never raised, and the process exits 1, right after a CRITICAL line that said "exiting with status 70". It is still non-zero, and 0 W is unwritable in that case anyway, so the physics is unchanged. This gap already existed for SIGTERM. Considertry: await controller.shutdown() finally: ...so cleanup and the 70 still happen. Otherwise, reword the CRITICAL line so it doesn't promise an outcome before it has happened.call(): catchingValueErroraround the whole block works, but the log then says "failed: 'utf-8' codec can't decode…" and loses the HTTP status.await resp.text(errors="replace")would keep the "rejected (400)" line and need no new except.Not reviewed as a blocker: whether DOCS should recommend the Watchdog toggle. That is the owner's call.
CI run 834 is this PR's own run: PRNUM 121, BASEREF release/1.0 (
fa3d237), head71d2d4a,GRADED: refs/pull/121/head merged into release/1.0, run_attempt 1, 33/33 PASS. N1 and N2 above are non-blocking. Please pick them up as a follow-up card if you agree. Merging.