Compare commits

...
5 Commits
Author SHA1 Message Date
glenn schrooyenandClaude Opus 5 8e3734f5ea PKG-12: the live options WERE checked, and the changelog said they were not
checks / unit (pull_request) Successful in 6m17s
Review approved the bump and, in doing so, dated one of its own paragraphs.
When this section was written the pre-deploy check had not been run and the
changelog said so, correctly. The owner then exported the reference install's
options and the check cleared, which makes the shipped sentence false in the
release notes an operator reads at the hardware.

Replaced with what was measured, twice:

  stored blob as-is                          exit 0, CLEARED
  effective post-upgrade config              exit 0, CLEARED
    (the 70 stored keys merged OVER 0.9.0's
     71 config.yaml defaults)

  max_w 5000, slew_w 400, rule-10 ceiling 555.6 W.

⚠️ AND THE SECOND CHECK IS NOT CEREMONY. The stored blob has 70 keys, not 71:
`delivery_hold_s` arrived with SAFETY-42 in 0.8.0, so a box running 0.7.0 has
never stored it, and it is REQUIRED in the schema with no trailing `?` - the
exact shape that becomes exit 78. It resolves to its `options:` default of 185
because the Supervisor sets `App.options = deepmerge(defaults, stored)`, which
`tools/supervisor_selfapi_probe.py` §D pins against a real Supervisor image.
Review re-derived that independently rather than taking the reasoning on trust.

"Your saved settings are valid" and "what the box will actually run is valid"
are different questions, and only the second one is the one being asked before
a deploy.

Suite: 26 files, all pass.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
2026-09-07 00:47:11 +02:00
glenn schrooyenandClaude Opus 5 86bdf49332 PKG-12: bump 0.8.0 -> 0.9.0, so the five merges behind it can actually ship
checks / unit (pull_request) Successful in 6m14s
⚠️ MINOR FOR NEW FUNCTIONALITY, NOT FOR A BREAK, AND THAT DISTINCTION IS
CHECKED RATHER THAN ASSERTED. PKG-07 was minor because a config that started on
0.7.0 could be REFUSED on 0.8.0. Nothing of that kind is true here:

  - `options:` and `schema:` hold 71 top-level keys on both 06661c5 and HEAD.
    The one schema addition is a PER-ENTRY key, `capacity_kwh: float(0.1,200)?`,
    and the trailing `?` makes it OPTIONAL.
  - An unmodified 0.8.0-era config validates clean on 0.9.0. Measured through
    the shipped validator on both the flat single-inverter shape the reference
    install runs and an N=2 roster with no `capacity_kwh` on any entry: 0
    problems each.
  - OPT-09's two new refusals fire only on configs that USE the new key - an
    unparseable value, or the key stated on one entry and left empty on another.
    Neither can reach a config written before the key existed.

PKG-07's first draft justified its bump with an invented compatibility break and
review caught it. This one states what was measured, and the measurement says
existing installs upgrade untouched.

Contents since 0.8.0 (`06661c5`): UX-02 (`3d29993`), OPT-09 (`fdb3b69`),
PKG-09 (`fcaa724`).

⚠️ THE VERSION WENT UNBUMPED FOR THE THIRD TIME IN THREE DAYS - PKG-06 six
merges, PKG-07 six, this one five - and this time PKG-09's own tool caught it
during PKG-09's review rather than a person. The changelog carries OPT-09's
disclosed N=2 detection regression and points at the pre-deploy check.

⚠️ STILL NOT DONE, AND STILL NOT DOABLE FROM HERE: the live options have not
been checked against this version's validator. The gate reports exit 2 - NOT
CLEARED - rather than clearing, which is correct. Export the options from the
Supervisor and re-run before `ha apps update`.

Suite: 26 files, all pass.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
2026-09-07 00:18:36 +02:00
admin fcaa72418d Merge pull request 'PKG-09: refuse a deploy that would be a no-op, or that the add-on would reject' (#63) from PKG-09 into release/1.0
PKG-09: refuse a deploy that would be a no-op, or that the add-on would reject
2026-09-07 00:04:32 +02:00
glenn schrooyenandClaude Opus 5 04b3330def PKG-09: refuse a deploy that would be a no-op, or that the add-on would reject
checks / unit (pull_request) Successful in 6m16s
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.

⚠️ IT CAUGHT A LIVE ONE ON ITS OWN PULL REQUEST. Run against release/1.0 while
this branch was in review: version 0.8.0 on both sides with UX-02's and
OPT-09's runtime commits already merged behind it. The class this exists to
catch had recurred, the same day, unnoticed - which is the argument for PKG-10,
a non-required post-merge check, since this script only helps someone who
remembers to run it.

⚠️ THE FIRST VERSION WAS BROKEN ON EVERY REAL REPOSITORY AND ITS TESTS COULD NOT
SEE IT. The "when did the version last change" lookup was `git log -S`, the
pickaxe, which finds commits that change the COUNT of a literal string. A bump
edits the line IN PLACE, so the count never moves and it returned only the
commit that first INTRODUCED the line. Measured here:

    -S  ->  0a1e61d  2026-08-23  Packaged as a Home Assistant add-on
    -G  ->  06661c5  2026-09-06  PKG-07: bump 0.7.0 -> 0.8.0

so a correctly bumped tree was reported as 87 COMMITS BEHIND AN UNBUMPED
VERSION, PKG-06's and PKG-07's own bump commits among the offenders. A tool
whose whole purpose is to be trusted cried wolf on every run.

⚠️ AND `-G` IS NOT THE FIX EITHER, ONLY A BETTER FILTER - it matches a reformat,
a quote change, a comment moved onto the line. `-G` narrows the candidates and
the VALUE is then compared against the parent's.

⚠️ THREE DISTINCT SHAPES OF HABIT 4 IN ONE FILE, each found by a different
means, which is the honest record of how this landed:
  1. Fixtures built fresh repositories whose version line is introduced once and
     never edited - the one shape in which `-S` looks correct. Found by review.
     Answered with a three-state history; the old lookup goes red on 4 checks.
  2. That three-state history still could not tell "value-compared" from
     "filtered-only": review swapped in `-G`'s newest raw hit and THE WHOLE
     SUITE STAYED GREEN, so the docstring's central claim was pinned by nothing.
     Answered with a cosmetic touch of the version line - `-G` matches it, the
     value comparison does not. That mutant now goes red on 3 checks.
  3. The counts below were claimed fixed and two of them had not landed at all.

Two git shapes it still gets wrong are recorded in the docstring rather than
fixed - a bump resolved inside a merge commit (false refusal) and a bump
reverted while the work is kept (false clearance, the dangerous direction).
Neither has ever occurred here; both were constructed in review. Card PKG-11.

⚠️ AND THE PROSE COUNTS, WHICH I REPORTED AS FIXED WHEN THEY WERE NOT. Round 1
found "PKG-06 ... four merges" (its own commit message says six) and "recorded
eleven times" (MUTATION.md:550 says TWELFTH). I replaced them without asserting
the replacement matched, said "all applied", and round 2 found both still in the
tree - one fixed in DOCS.md but not in the script, one not fixed anywhere. A
claim that a correction landed is itself a claim that needs checking, and this
is PROC-01's tenth instance.

WHAT IT DOES NOT DO. It does not read the live box: the options blob needs the
Supervisor API. Without --options the validator half SKIPS - loudly, exit 2 -
and never pretends. On a bare host `app/validate.py` needs aiohttp, so that half
skips too and names the container to re-run in. A missing or malformed --options
file is a REFUSAL with a sentence, not a traceback.

EXIT CODES: 0 cleared, 1 REFUSED, 2 NOT CLEARED. ⚠️ 2 IS NOT A PASS.

Suite: 26 files, all pass.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
2026-09-06 23:57:51 +02:00
admin fdb3b69397 Merge pull request 'OPT-09: the weights parameter nothing ever passed, and what it cannot do' (#62) from OPT-09 into release/1.0
OPT-09: the weights parameter nothing ever passed, and what it costs SAFETY-42
2026-09-06 23:33:17 +02:00
5 changed files with 641 additions and 1 deletions
+81
View File
@@ -1,5 +1,86 @@
# Changelog
## 0.9.0
⚠️ **MINOR FOR NEW FUNCTIONALITY, NOT FOR A BREAK - AND THAT DISTINCTION IS THE
POINT.** 0.8.0 was minor because a config that started on 0.7.0 could be
REFUSED on it (rule 10 against 0.7.0's own default `slew_w: 1000`). Nothing of
that kind is true here, and it was checked rather than assumed:
- `options:` and `schema:` both hold **71 top-level keys**, the same 71 as
0.8.0. The one schema addition is a per-entry key, `capacity_kwh:
float(0.1,200)?` - **note the trailing `?`, it is OPTIONAL**.
- An unmodified 0.8.0-era config validates clean on 0.9.0. Measured on both the
flat single-inverter shape the reference install runs and an N=2 roster with
no `capacity_kwh` on any entry: **0 problems each**.
- The two refusals [[OPT-09]] adds fire only on configs that USE the new key -
an unparseable value, or the key stated on one roster entry and left empty on
another. Neither can reach a config written before the key existed.
So: existing installs upgrade untouched, and the bump is minor because the
option surface and the behaviour grew.
Contents, in merge order since 0.8.0 (`06661c5`):
| | |
|---|---|
| `3d29993` | UX-02 - the validator's refusals, next to the field, before the save |
| `fdb3b69` | OPT-09 - the weights parameter nothing ever passed, and what it cannot do |
| `fcaa724` | PKG-09 - refuse a deploy that would be a no-op, or that the add-on would reject |
**UX-02** builds the real configuration surface in the ingress panel, beside the
Supervisor Configuration tab that cannot be removed. The prize is not styling:
`validate.py` writes long cross-field refusals the Supervisor UI *structurally
cannot show*, and until now an operator met them as **exit 78 in a log**, after
saving and restarting. ⚠️ Two config surfaces now exist and the DECISIONS record
names the **stored options blob** as authoritative, neither UI.
**OPT-09** makes the architecture's one free parameter real. `main.py` called
`resolve()` without weights, so the split was a fixed 50/50 regardless of what
either pack could deliver; it now passes SoC- and capacity-aware weights.
⚠️ **OPT-09 SHIPS A DISCLOSED DETECTION REGRESSION AT N=2.** `delivery.
delinquent()` reads an ABSOLUTE 500 W gap, so weighting a low-headroom unit down
to a small share puts a unit delivering **none of what it was asked** under the
threshold. Measured: a wedged inverter goes from named on 199 of 200 cycles to
**0 of 200** at a load its sibling can carry alone. The alarm returns once the
healthy pack saturates - swept, that is a 5000 W load on the fixture, **below**
the proportional-regime formula's 6500 W and not above it. ⚠️ **N=1 IS
UNAFFECTED**: `allocator.plan()` returns before weights are read at all. The
closure is card SAFETY-43.
**PKG-09** adds `tools/predeploy_check.py`. ⚠️ **RUN IT BEFORE DEPLOYING THIS
RELEASE** - it is what caught the fact that 0.8.0 had five runtime commits
behind it, which is why this section exists:
docker run --rm -v <repo>:/w:ro goodwe-addon-checks:1 \
python3 /w/goodwe_controller/tools/predeploy_check.py \
--deployed-version 0.8.0 --options /w/live-options.json
Exit `0` cleared, `1` REFUSED, `2` NOT CLEARED. ⚠️ **2 is not a pass.**
✅ **RUN AGAINST THIS RELEASE, 2026-09-07: CLEARED, exit 0.** The reference
install's options were exported from the Supervisor (70 keys) and checked twice
- the stored blob as-is, and the **effective post-upgrade config**, being those
70 keys merged over 0.9.0's 71 defaults. Both pass. `max_w` 5000, `slew_w` 400
against a rule-10 ceiling of 555.6 W.
⚠️ **THE STORED BLOB HAS 70 KEYS, NOT 71, AND THAT IS THE INTERESTING PART.**
`delivery_hold_s` arrived with SAFETY-42 in 0.8.0, so an install running 0.7.0
has never stored it - and it is REQUIRED in the schema, no trailing `?`, which
is the shape that becomes exit 78. It resolves to its `options:` default of
**185**, because the Supervisor sets `App.options = deepmerge(config.yaml
defaults, stored options)`; `tools/supervisor_selfapi_probe.py` §D pins that
against a real Supervisor image rather than assuming it. The second check above
exists because "your saved settings are valid" and "what the box will actually
run is valid" are different questions, and only the second one matters.
⚠️ **THE VERSION WENT UNBUMPED FOR THE THIRD TIME IN THREE DAYS** - PKG-06 six
merges, PKG-07 six, this one five - and this time it was caught by PKG-09's own
tool during PKG-09's review rather than by a person. `ha apps update` against an
unchanged version installs nothing and reports success. PKG-10 carries the
post-merge alarm so that noticing stops being a human's job.
## Unreleased
⚠️ **THIS HAS NO VERSION AND CANNOT REACH AN INSTALL UNTIL IT GETS ONE.**
+43
View File
@@ -1572,3 +1572,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.
+1 -1
View File
@@ -1,5 +1,5 @@
name: GoodWe RS485 Controller
version: "0.8.0"
version: "0.9.0"
slug: goodwe_controller
description: >-
Drives a GoodWe ES/BP battery inverter over RS485 by emulating its smart
+244
View File
@@ -0,0 +1,244 @@
"""PKG-09 - grade the check that refuses a no-op deploy. `python3 test_version_gate.py`
⚠️ THE POINT OF THIS FILE IS THE REFUSALS, NOT THE CLEARANCES. A pre-deploy
check that only ever says "CLEARED" is the defect `MUTATION.md` habit 4 names -
"a check whose DATA cannot reach the failure it names" - which this repository
has recorded twelve times. So every refusal below is DRIVEN: a repository is
built in which the failure is really present, and the check is required to say
so. A clearance is only meaningful next to a refusal from the same harness.
The third property is the one that is easy to lose: a SKIPPED check must not
look like a PASSED one. Omitting `--options` cannot exit 0.
"""
import json
import os
import shutil
import subprocess
import sys
import tempfile
HERE = os.path.dirname(os.path.abspath(__file__))
GATE = os.path.join(HERE, "tools", "predeploy_check.py")
sys.path.insert(0, os.path.join(HERE, "tools"))
fails = []
tmpdirs = []
def check(name, ok):
print(f" {'ok ' if ok else 'FAIL'} {name}")
if not ok:
fails.append(name)
def git(repo, *args):
return subprocess.run(
["git", "-C", repo, "-c", "user.email=t@t", "-c", "user.name=t", *args],
capture_output=True, text=True)
def make_repo(version="0.7.0"):
"""A repo shaped like this one: <root>/goodwe_controller/{config.yaml,app,tools}."""
root = tempfile.mkdtemp(prefix="test_version_gate.")
tmpdirs.append(root)
addon = os.path.join(root, "goodwe_controller")
os.makedirs(os.path.join(addon, "app"))
write(addon, "config.yaml", f'name: t\nversion: "{version}"\n')
write(addon, "app/control.py", "X = 1\n")
write(addon, "CHANGELOG.md", "# Changelog\n")
git(root, "init", "-q", ".")
git(root, "add", "-A")
git(root, "commit", "-q", "-m", f"initial at {version}")
return root, addon
def write(addon, rel, text):
p = os.path.join(addon, rel)
with open(p, "w", encoding="utf-8") as fh:
fh.write(text)
def run_gate(addon, root, deployed, options=None):
"""Run the REAL gate against a throwaway tree.
⚠️ The script is invoked where it SHIPS, not copied into the fixture. It
imports the validator from its own sibling `app/`, which is the behaviour
that matters: the gate must check a config against the validator it ships
beside, not against whatever happens to sit next to a copy. Copying it into
the fixture made every validator assertion silently SKIP - the first draft
of this file did exactly that and reported four green skips as passes.
"""
cmd = [sys.executable, GATE,
"--deployed-version", deployed, "--tree", addon, "--repo", root]
if options:
cmd += ["--options", options]
return subprocess.run(cmd, capture_output=True, text=True)
VALID = {"max_w": 2000, "slew_w": 0, "target_grid_w": -10, "gain": 0.6,
"inverters": [], "setpoint_entity": "number.goodwe_setpoint_w"}
def opts_file(d):
fd, p = tempfile.mkstemp(suffix=".json")
with os.fdopen(fd, "w") as fh:
json.dump(d, fh)
return p
print("classification - what forces a rebuild and what does not")
from predeploy_check import is_runtime, shipped_version # noqa: E402
for p in ("goodwe_controller/app/control.py", "goodwe_controller/config.yaml",
"goodwe_controller/translations/en.yaml", "goodwe_controller/run.sh"):
check(f"runtime: {p}", is_runtime(p))
for p in ("goodwe_controller/CHANGELOG.md", "goodwe_controller/DOCS.md",
"goodwe_controller/test_control.py", "goodwe_controller/tools/x.py",
"goodwe_controller/DECISIONS/a.md", "README.md"):
check(f"NOT runtime: {p}", not is_runtime(p))
print("\nTHE REFUSAL - a runtime change behind an unchanged version")
root, addon = make_repo("0.7.0")
write(addon, "app/control.py", "X = 2\n")
git(root, "commit", "-qam", "SAFETY-99: change behaviour, bump nothing")
r = run_gate(addon, root, "0.7.0", opts_file(VALID))
check("refuses (exit 1)", r.returncode == 1)
check("...and says the deploy would be a NO-OP", "NO-OP" in r.stdout)
check("...and names the offending commit", "SAFETY-99" in r.stdout)
check("...and does NOT say cleared", "CLEARED for deploy" not in r.stdout)
print("\nPOSITIVE CONTROL - the same check must NOT fire on a docs-only change")
root2, addon2 = make_repo("0.7.0")
write(addon2, "CHANGELOG.md", "# Changelog\n\n## notes\n")
git(root2, "commit", "-qam", "PKG-99: documentation only")
r2 = run_gate(addon2, root2, "0.7.0", opts_file(VALID))
check("does not refuse a docs-only tree", r2.returncode == 0)
check("...and says there is nothing to deploy",
"nothing to deploy" in r2.stdout)
print("\nTHE CLEARANCE - a bumped version with a valid config")
root3, addon3 = make_repo("0.7.0")
write(addon3, "app/control.py", "X = 3\n")
write(addon3, "config.yaml", 'name: t\nversion: "0.8.0"\n')
git(root3, "commit", "-qam", "PKG-99: change and bump together")
r3 = run_gate(addon3, root3, "0.7.0", opts_file(VALID))
check("clears (exit 0)", r3.returncode == 0)
check("...and says so", "CLEARED for deploy" in r3.stdout)
print("\n⭐ A SKIPPED CHECK MUST NOT LOOK LIKE A PASSED ONE")
r4 = run_gate(addon3, root3, "0.7.0") # no --options
check("omitting --options cannot exit 0", r4.returncode != 0)
check("...says SKIPPED", "SKIPPED" in r4.stdout)
check("...says the config was NEVER CHECKED", "NEVER CHECKED" in r4.stdout)
check("...and does NOT claim clearance", "CLEARED for deploy" not in r4.stdout)
print("\nTHE OTHER REFUSAL - a config this version's validator rejects")
bad = dict(VALID, slew_w=1000) # rule 10: > 2/18 x 2000 = 222 W
r5 = run_gate(addon3, root3, "0.7.0", opts_file(bad))
check("refuses a config the validator rejects", r5.returncode == 1)
check("...and warns it is exit 78", "78" in r5.stdout)
check("...and warns the ingress panel goes too", "ingress panel" in r5.stdout)
check("...and prints the validator's own words", "slew_w is 1000 W" in r5.stdout)
print()
print("⭐ HISTORY WITH MORE THAN ONE BUMP - THE FIXTURE THE FIRST DRAFT LACKED")
# ⚠️ EVERY FIXTURE ABOVE BUILDS A FRESH REPOSITORY WHOSE `version:` LINE IS
# INTRODUCED ONCE AND NEVER EDITED. That is precisely the shape in which the
# original `git log -S 'version: "'` looked correct - the pickaxe finds commits
# that change a string's COUNT, and introducing the line changes it from 0 to 1.
# A real repository edits the line IN PLACE, the count never moves again, and
# the lookup silently returns the commit that first added it.
#
# MEASURED on admin/goodwe-addon before the fix: `-S` named `0a1e61d`
# (2026-08-23, the original packaging commit) as "the last bump", so a correctly
# bumped tree was reported as 87 commits behind an unbumped version - with
# PKG-06's and PKG-07's own bump commits listed among the offenders. The tool
# cried wolf on every run, and none of the fixtures above could see it, because
# 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.
from predeploy_check import last_version_change # noqa: E402
root4 = tempfile.mkdtemp(prefix="test_version_gate.hist.")
tmpdirs.append(root4)
addon4 = os.path.join(root4, "goodwe_controller")
os.makedirs(os.path.join(addon4, "app"))
write(addon4, "config.yaml", 'name: t\nversion: "0.5.0"\n')
write(addon4, "app/control.py", "X = 1\n")
git(root4, "init", "-q", ".")
git(root4, "add", "-A")
git(root4, "commit", "-q", "-m", "initial at 0.5.0")
_first = git(root4, "rev-parse", "HEAD").stdout.strip()
write(addon4, "app/control.py", "X = 2\n")
git(root4, "commit", "-qam", "runtime change between bumps")
write(addon4, "config.yaml", 'name: t\nversion: "0.6.0"\n')
git(root4, "commit", "-qam", "bump to 0.6.0")
write(addon4, "app/control.py", "X = 3\n")
git(root4, "commit", "-qam", "another runtime change")
write(addon4, "config.yaml", 'name: t\nversion: "0.7.0"\n')
git(root4, "commit", "-qam", "bump to 0.7.0")
_last_bump = git(root4, "rev-parse", "HEAD").stdout.strip()
check("⭐ THE LAST BUMP IS FOUND, NOT THE FIRST. An in-place edit of the "
"version line does not change how many times the string occurs, so a "
"pickaxe search returns the commit that introduced it and every bump "
"after that is invisible",
last_version_change(root4) == _last_bump)
check("...and it is emphatically not the commit that introduced the line",
last_version_change(root4) != _first)
r6 = run_gate(addon4, root4, "0.7.0", opts_file(VALID))
check("a tree bumped with nothing after it clears, even with bumps in history",
r6.returncode == 0 and "CLEARED for deploy" in r6.stdout)
write(addon4, "app/control.py", "X = 4\n")
git(root4, "commit", "-qam", "SAFETY-98: runtime change after the last bump")
r7 = run_gate(addon4, root4, "0.7.0", opts_file(VALID))
check("and one runtime commit after that last bump is refused", r7.returncode == 1)
check("...naming only the commit since the LAST bump, not since the first",
"SAFETY-98" in r7.stdout and "runtime change between bumps" not in r7.stdout)
# ⚠️ AND THE FIXTURE ABOVE STILL COULD NOT TELL TWO IMPLEMENTATIONS APART.
# Review swapped `last_version_change` for "-G's newest raw hit, skip the value
# comparison entirely" and THE WHOLE SUITE STAYED GREEN, including the section
# above - because a linear history of real bumps never produces a commit that
# touches the version LINE without changing the version VALUE. So the docstring's
# central claim ("the VALUE is compared, and that is the only thing that answers
# it") was not pinned by anything, and `-G` alone would have shipped as if it
# were the fix. That is the third distinct shape of habit 4 in this one file.
#
# A cosmetic touch of the version line is the discriminator: `-G '^version: "'`
# matches it, and the value comparison does not.
write(addon4, "config.yaml", 'name: t\nversion: "0.7.0" # pinned deliberately\n')
git(root4, "commit", "-qam", "COSMETIC: annotate the version line, same value")
_cosmetic = git(root4, "rev-parse", "HEAD").stdout.strip()
check("⭐ A COMMIT THAT TOUCHES THE VERSION LINE WITHOUT CHANGING THE VERSION "
"IS NOT A BUMP. `-G` matches it and a filter-only lookup would name it; "
"only comparing the VALUE against the parent's answers the question the "
"tool is actually asking",
last_version_change(root4) == _last_bump)
check("...and it is emphatically not the cosmetic commit",
last_version_change(root4) != _cosmetic)
r8 = run_gate(addon4, root4, "0.7.0", opts_file(VALID))
check("...and because config.yaml IS a runtime path, that cosmetic commit is "
"still counted as something unshipped - conservative, and correct: the "
"Supervisor rebuilds on the version string alone, so a changed file "
"behind an unchanged version is a no-op whatever changed in it",
r8.returncode == 1 and "COSMETIC" in r8.stdout)
print("\nthe shipped tree's own version is readable")
check("shipped_version() finds a version", bool(shipped_version(HERE)))
for d in tmpdirs:
shutil.rmtree(d, ignore_errors=True)
print()
if fails:
print(f"{len(fails)} FAILED: " + "; ".join(fails))
sys.exit(1)
print("all checks passed")
+272
View File
@@ -0,0 +1,272 @@
"""PKG-09 - refuse a deploy that would be a no-op, or that the add-on would reject.
python3 tools/predeploy_check.py --deployed-version 0.7.0 --options live-options.json
⚠️ RUN IT WHERE THE ADD-ON'S DEPENDENCIES EXIST. `app/validate.py` imports its
siblings and those need aiohttp, so on a bare host the validator half SKIPS -
loudly, and exit 2, never a silent pass. To get both halves:
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
EXIT CODES. 0 cleared; 1 REFUSED, do not deploy; 2 NOT CLEARED - a check did not
run, and that is not the same as passing.
TWO FAILURES THIS ANSWERS, BOTH OF WHICH HAVE HAPPENED.
1. THE SILENT NO-OP. The Supervisor keys the built image by `version:` in
config.yaml. `ha apps update` against an unchanged version does nothing and
REPORTS SUCCESS. PKG-06 recorded this on 2026-09-05 with SIX merges behind
an unbumped version - its own commit message says so, and this line said
"four" through two review rounds; PKG-07 recorded it again on 2026-09-06 with six. Both
times a human noticed. Nothing else was ever going to.
2. 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 by a config that
started fine on the previous version: it refuses an explicit `slew_w` above
SLEW_RAIL_FRAC x the site rail, and 0.7.0's own shipped default of 1000
exceeds the 555.6 W ceiling on a 5000 W rail.
⚠️ WHY THIS IS NOT A CI CHECK. The obvious version gate - "a pull request that
touches runtime code must bump the version" - is the WRONG shape for this
project, which deliberately batches many merges into one deploy. It would demand
a bump per pull request, put every branch in conflict over one line of
config.yaml, and still not prevent the actual failure, which happens at DEPLOY
and not at merge. The check belongs where the mistake is made.
⚠️ WHAT THIS DOES NOT DO. It does not read the live box. The options blob is
reachable only through the Supervisor API, which needs a supervisor token, so
the operator exports it and passes `--options`. Without that flag the validator
half is SKIPPED and says so - a skipped check must never look like a passed one.
"""
import argparse
import json
import os
import re
import subprocess
import sys
HERE = os.path.dirname(os.path.abspath(__file__))
ADDON = os.path.dirname(HERE)
sys.path.insert(0, ADDON)
# Files that end up in the built image and can change what it DOES. A change to
# any of these needs a new `version:` or the Supervisor will not rebuild.
# CHANGELOG.md, DOCS.md, MUTATION.md, DECISIONS/, test_*.py, tools/ and
# scenarios/ are deliberately absent: they ship in the repo but cannot change
# runtime behaviour, and demanding a bump for a typo fix trains people to bump
# without reading, which is the habit this file exists to prevent.
RUNTIME = (
"goodwe_controller/app/",
"goodwe_controller/translations/",
"goodwe_controller/config.yaml",
"goodwe_controller/Dockerfile",
"goodwe_controller/run.sh",
"goodwe_controller/requirements.txt",
"goodwe_controller/build.yaml",
)
CONFIG_PATH = "goodwe_controller/config.yaml"
_VER = re.compile(r'^version:\s*"([^"]+)"', re.M)
def shipped_version(tree=ADDON):
with open(os.path.join(tree, "config.yaml"), encoding="utf-8") as fh:
m = _VER.search(fh.read())
return m.group(1) if m else None
def is_runtime(path):
return any(path == p or path.startswith(p) for p in RUNTIME)
def _git(*args, repo=None):
return subprocess.run(["git", "-C", repo or os.path.dirname(ADDON), *args],
capture_output=True, text=True)
def _version_at(sha, repo=None, path=CONFIG_PATH):
"""`version:` as of one commit, or None if the file or the key is absent."""
r = _git("show", f"{sha}:{path}", repo=repo)
if r.returncode != 0:
return None
m = _VER.search(r.stdout)
return m.group(1) if m else None
def last_version_change(repo=None):
"""The newest commit whose `version:` VALUE differs from its parent's.
⚠️ THIS USED TO BE `git log -S 'version: "'` AND IT WAS WRONG ON EVERY REAL
REPOSITORY. `-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 found only the commit that first
INTRODUCED the line.
Measured on this repository: `-S` returned `0a1e61d` (2026-08-23, the
original packaging commit) as "the last bump", so the script reported the
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 a commit whose diff touches a line with that shape - which includes
a reformat, a quote change, or a comment moved onto the line, none of which
are bumps. So `-G` narrows the candidates and then the VALUE is compared
against the parent's. The question is "did the version change", and that is
the only thing that answers it.
⚠️ TWO GIT SHAPES THIS STILL GETS WRONG, NEITHER OF WHICH HAS EVER OCCURRED
IN THIS REPOSITORY - every bump here has been a clean, linear, forward-only
edit. Both were constructed in review rather than found:
1. A BUMP RESOLVED INSIDE A MERGE COMMIT IS INVISIBLE. `git log -G -- path`
does not diff merge commits at all without `-m`, so a version line whose
final value is decided by a conflict resolution is never a candidate.
The lookup then names the last ordinary bump, and the merge itself counts
as a runtime change since it - a FALSE REFUSAL of a tree that is in fact
correctly bumped.
2. REVERTING A BUMP WHILE KEEPING THE WORK MASKS THE WORK. Bump, change
`app/`, then revert only `config.yaml`: the version is back where it
started, the runtime change is still in the tree, and this reports
"nothing to deploy" - a FALSE CLEARANCE, which is the dangerous
direction.
Card PKG-11 carries both. They are recorded here rather than fixed because
a lookup that handles them costs more than the failure they represent, and
a caveat a reader can check beats a guarantee that is not true.
"""
r = _git("log", "--format=%H", "-G", r'^version: "', "--", CONFIG_PATH,
repo=repo)
if r.returncode != 0:
return None
shas = r.stdout.split()
for sha in shas: # newest first
here = _version_at(sha, repo)
there = _version_at(sha + "^", repo)
if here is not None and here != there:
return sha
return shas[-1] if shas else None
def runtime_commits_since_version_change(repo=None):
"""Commits touching RUNTIME since `version:` last changed. ([], None) if unknown.
Returns (commits, last_bump_sha). An empty list with a sha means the tree is
clean; an empty list with None means git could not answer and the caller
must NOT read that as a pass.
"""
last = last_version_change(repo)
if not last:
return [], None
r = _git("log", "--format=%H %s", f"{last}..HEAD", repo=repo)
if r.returncode != 0:
return [], None
out = []
for line in r.stdout.splitlines():
sha, _, subject = line.partition(" ")
f = _git("show", "--name-only", "--format=", sha, repo=repo)
if any(is_runtime(p) for p in f.stdout.split()):
out.append((sha[:10], subject))
return out, last[:10]
def main():
ap = argparse.ArgumentParser()
ap.add_argument("--deployed-version", required=True,
help="the version currently running ON THE BOX")
ap.add_argument("--options", help="path to the live options blob, exported "
"from the Supervisor. Omitted = that half is SKIPPED.")
ap.add_argument("--tree", default=ADDON)
ap.add_argument("--repo", default=None)
a = ap.parse_args()
problems, skipped = [], []
shipped = shipped_version(a.tree)
print(f" deployed on the box : {a.deployed_version}")
print(f" shipped in this tree: {shipped}")
commits, last = runtime_commits_since_version_change(a.repo)
if shipped is None:
problems.append("could not read `version:` out of config.yaml")
elif shipped == a.deployed_version:
if last is None:
skipped.append("could not ask git what changed since the last bump; "
"the no-op check is INCONCLUSIVE, not clean")
if commits:
problems.append(
f"THE DEPLOY WOULD BE A NO-OP. Version is {shipped} on both sides, and "
f"{len(commits)} commit(s) touching runtime files have landed since the "
f"version last changed ({last}). `ha apps update` will report success "
f"and install NOTHING. Bump `version:` first.")
for sha, subj in commits[:12]:
print(f" {sha} {subj[:76]}")
elif last is not None:
print(" no runtime change since the last bump - nothing to deploy")
if a.options:
# ⚠️ A TOOL RUN IN A HURRY BEFORE A DEPLOY MUST NOT ANSWER WITH A
# TRACEBACK. A missing or malformed export is the likeliest operator
# error here, and it is a REFUSAL, not a skip - the config was not
# checked and the script must not be re-run past it.
opts = None
try:
with open(a.options, encoding="utf-8") as fh:
opts = json.load(fh)
except FileNotFoundError:
problems.append(f"--options file not found: {a.options}")
except (json.JSONDecodeError, OSError, UnicodeDecodeError) as exc:
problems.append(f"--options file could not be read as JSON "
f"({type(exc).__name__}: {exc}); the config was NOT checked")
if a.options and opts is not None:
try:
from app.validate import validate_startup_config
except Exception as exc: # pragma: no cover
skipped.append(
f"could not import the shipped validator ({exc}); the config check "
f"did NOT run. `app/validate.py` imports its siblings, and those need "
f"the add-on's runtime dependencies - so THIS SCRIPT ONLY CHECKS A "
f"CONFIG WHERE THOSE DEPENDENCIES EXIST: inside the add-on container, "
f"or in the CI image. Re-run it as:\n"
f" docker run --rm -v <repo>:/w:ro goodwe-addon-checks:1 \\\n"
f" python3 /w/goodwe_controller/tools/predeploy_check.py \\\n"
f" --deployed-version <ver> --options /w/<options.json>")
else:
found = validate_startup_config(opts)
if found:
problems.append(
f"THE LIVE CONFIG IS REFUSED BY THIS VERSION'S VALIDATOR "
f"({len(found)} problem(s)). Deploying it is SystemExit(78): the "
f"add-on does not start, and the ingress panel goes with it because "
f"the add-on serving it is not running. Recovery is the Supervisor UI.")
for p in found:
print(f" - {p}")
else:
print(" live options pass this version's validator")
else:
skipped.append("--options was not given, so the live config was NEVER CHECKED "
"against this version's validator. A config refused at startup is "
"exit 78. Export the options and re-run before deploying.")
print()
for s in skipped:
print(f" ⚠️ SKIPPED: {s}")
if problems:
for p in problems:
print(f" ❌ {p}")
print("\n REFUSED - do not deploy.")
return 1
if skipped:
print(" NOT CLEARED - checks above did not run. Deploying now is a decision "
"made without them.")
return 2
print(" CLEARED for deploy.")
return 0
if __name__ == "__main__":
sys.exit(main())