diff --git a/goodwe_controller/DOCS.md b/goodwe_controller/DOCS.md index c1e624a..6083811 100644 --- a/goodwe_controller/DOCS.md +++ b/goodwe_controller/DOCS.md @@ -1452,3 +1452,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 (four merges behind an unbumped +version) and PKG-07 (six). 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 :/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. diff --git a/goodwe_controller/test_version_gate.py b/goodwe_controller/test_version_gate.py new file mode 100644 index 0000000..dcec76e --- /dev/null +++ b/goodwe_controller/test_version_gate.py @@ -0,0 +1,153 @@ +"""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 eleven 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: /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("\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") diff --git a/goodwe_controller/tools/predeploy_check.py b/goodwe_controller/tools/predeploy_check.py new file mode 100644 index 0000000..dd1a06e --- /dev/null +++ b/goodwe_controller/tools/predeploy_check.py @@ -0,0 +1,195 @@ +"""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 :/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 four merges behind + an unbumped version; 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", +) + +_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 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. + """ + r = _git("log", "-1", "--format=%H", "-S", 'version: "', + "--", "goodwe_controller/config.yaml", repo=repo) + if r.returncode != 0 or not r.stdout.strip(): + return [], None + last = r.stdout.strip() + 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: + with open(a.options, encoding="utf-8") as fh: + opts = json.load(fh) + 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 :/w:ro goodwe-addon-checks:1 \\\n" + f" python3 /w/goodwe_controller/tools/predeploy_check.py \\\n" + f" --deployed-version --options /w/") + 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())