PKG-09: refuse a deploy that would be a no-op, or that the add-on would reject
checks / unit (pull_request) Successful in 6m14s
checks / unit (pull_request) Successful in 6m14s
Two ways a deploy of this add-on has gone wrong, and the Supervisor reports both as success. THE SILENT NO-OP. The Supervisor keys the built image by `version:`, so `ha apps update` against an unchanged version installs nothing and says it worked. PKG-06 recorded this on 2026-09-05 with six merges behind an unbumped version. PKG-07 recorded it again on 2026-09-06 with six. Both times a human noticed. Nothing else was going to. THE REFUSED CONFIG. A config the shipped validator rejects is SystemExit(78) - the add-on does not start. ⚠️ AND THE INGRESS PANEL GOES WITH IT, because the add-on serving it is not running, so recovery means the Supervisor UI. ⚠️ THE FIRST VERSION OF THIS TOOL WAS BROKEN ON EVERY REAL REPOSITORY, AND ITS OWN TESTS COULD NOT SEE IT. The "when did the version last change" lookup was `git log -S 'version: "'`. `-S` is the pickaxe: it finds commits that change the COUNT of a literal string. A bump edits the line IN PLACE - `version: "0.7.0"` -> `version: "0.8.0"` - so the substring still occurs exactly once before and after, and `-S` never sees it. It returned only the commit that first INTRODUCED the line. MEASURED on admin/goodwe-addon, in its current correctly-bumped state: -S ->0a1e61d2026-08-23 Packaged as a Home Assistant add-on -G ->06661c52026-09-06 PKG-07: bump 0.7.0 -> 0.8.0 so the shipped script reported a correctly bumped tree as 87 COMMITS BEHIND AN UNBUMPED VERSION, listing PKG-06's and PKG-07's own bump commits among the offenders. A tool whose entire purpose is to be trusted before a deploy cried wolf on every run. ⚠️ AND `-G` IS NOT THE FIX EITHER, ONLY A BETTER FILTER. `-G'^version: "'` matches any commit whose diff touches a line of that shape - a reformat, a quote change, a comment moved onto the line. So `-G` narrows the candidates and then the VALUE is compared against the parent's. The question is "did the version change", and only comparing the value answers it. ⚠️ WHY THE TESTS MISSED IT, WHICH IS THE FINDING BEHIND THE FINDING. Every fixture built a fresh repository whose `version:` line is introduced once and never edited - the one shape in which `-S` looks correct, because introducing the line does change the count from 0 to 1. A test that builds its own one-bump history is a test asserting against a structure it built itself: MUTATION.md habit 4, in the file written to prevent a recurrence, and the thirteenth recorded instance. There is now a fixture with THREE version states and runtime commits between them, and it goes red on four checks against the old `-S` lookup. ⚠️ WHY THIS IS NOT A CI CHECK. "A pull request that touches runtime code must bump the version" would put every open branch in conflict over one line of config.yaml and still not prevent the failure, which happens at DEPLOY and not at merge. A cheaper form review proposed - a NON-REQUIRED check on push to release/1.0 that raises a visible alarm when commits accumulate behind an unbumped version - is a good idea and is filed as PKG-10 rather than built here. WHAT IT DOES NOT DO. It does not read the live box: the options blob needs the Supervisor API and a supervisor token. Without --options the validator half SKIPS - loudly, exit 2 - and never pretends. `app/validate.py` imports its siblings and those need aiohttp, so on a bare host that half skips too and says which container to re-run it in. A missing or malformed --options file is now a REFUSAL with a sentence rather than a traceback. EXIT CODES: 0 cleared, 1 REFUSED, 2 NOT CLEARED. ⚠️ 2 IS NOT A PASS. Also corrected: this file said PKG-06 was "four merges" when its own commit message says six, and the test docstring said habit 4 had eleven instances when MUTATION.md already said TWELFTH. Both were caught in review. `requirements.txt` stays in RUNTIME although the Dockerfile installs via apk - over-inclusive is the safe direction and costs only an unnecessary bump. Suite: 25 files, all pass (25 with UX-02's test_selfconfig.py now in the base). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
This commit is contained in:
co-authored by
Claude Opus 5
parent
3d2999337d
commit
6c348974a3
@@ -1521,3 +1521,46 @@ If an MQTT broker is available the add-on publishes setpoint, grid power,
|
||||
battery power, circulating power, state of charge, maintenance phase and
|
||||
controller status by MQTT discovery. This is observability only — the controller works fine without a
|
||||
broker, and MQTT problems can never affect control.
|
||||
|
||||
## Before you deploy: `tools/predeploy_check.py` (PKG-09)
|
||||
|
||||
Two ways a deploy has gone wrong on this system, both of which the Supervisor
|
||||
reports as success:
|
||||
|
||||
**The silent no-op.** The Supervisor keys the built image by `version:` in
|
||||
`config.yaml`. `ha apps update` against an unchanged version installs nothing
|
||||
and says it worked. This happened twice: PKG-06 (six merges behind an unbumped
|
||||
version) and PKG-07 (six again). Both times a human noticed. Nothing else would have.
|
||||
|
||||
**The refused config.** A config the shipped validator rejects is
|
||||
`SystemExit(78)` — the add-on does not start. ⚠️ **And the ingress panel goes
|
||||
with it**, because the add-on serving it is not running, so recovery means the
|
||||
Supervisor UI. SAFETY-39's rule 10 made this reachable from a config that
|
||||
started fine on the previous version: 0.7.0's own default `slew_w: 1000`
|
||||
exceeds the 555.6 W ceiling on a 5000 W rail.
|
||||
|
||||
Run it inside the container, so the validator half actually runs:
|
||||
|
||||
docker run --rm -v <repo>:/w:ro goodwe-addon-checks:1 \
|
||||
python3 /w/goodwe_controller/tools/predeploy_check.py \
|
||||
--deployed-version 0.7.0 --options /w/live-options.json
|
||||
|
||||
Export `live-options.json` from the Supervisor first — the options blob is not
|
||||
reachable through the Home Assistant REST API, and no entity carries it.
|
||||
|
||||
**Exit codes.** `0` cleared · `1` REFUSED, do not deploy · `2` NOT CLEARED,
|
||||
meaning a check did not run. ⚠️ **2 is not a pass.** On a bare host without the
|
||||
add-on's dependencies the validator half skips and returns 2 rather than
|
||||
pretending; `test_version_gate.py` pins that a skipped check can never exit 0,
|
||||
because the first draft of that test reported four silent skips as passes.
|
||||
|
||||
⚠️ **Why this is not a CI check.** The obvious gate — "a pull request touching
|
||||
runtime code must bump the version" — is the wrong shape here. This project
|
||||
deliberately batches many merges into one deploy, so a per-PR bump would put
|
||||
every branch in conflict over one line and still not prevent the failure, which
|
||||
happens at deploy and not at merge. The check belongs where the mistake is made.
|
||||
|
||||
⚠️ This file's own change does **not** need a version bump, and the gate agrees:
|
||||
`tools/` and `test_*.py` are not in its runtime set, because demanding a bump
|
||||
for a typo fix trains people to bump without reading — the habit the check
|
||||
exists to prevent.
|
||||
|
||||
Reference in New Issue
Block a user