glenn schrooyenandClaude Opus 5.5
f4f72211a9
checks / unit (pull_request) Successful in 8m31s
predeploy_check.py and rehearse_roster.py now assert a positive property of their input (a JSON object, most of whose top-level keys are known option or store keys) and refuse anything else with the shape named. The `ha apps info --raw-json` wrapper, a truncated or list-wrapped state file and a wrapped config no longer produce "CLEARED" / "NO FINDINGS" with exit 0. Also: a clean --no-live-entities rehearsal exits 3, not 0; alloc_sim.py exits 1 when split-invariance FAILS; decode_bus_capture.py takes several paths and exits 1 on a file that is mostly not frames. Graded by the new test_tool_inputs.py; DOCS.md says to pass .data.options. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
352 lines
16 KiB
Python
352 lines
16 KiB
Python
"""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` - the bare options object
|
|
(`.data.options` of `ha apps info <slug> --raw-json`); any other shape is
|
|
refused. 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 option_keys(addon=ADDON):
|
|
"""Top-level option names in `addon`'s config.yaml `options:`/`schema:` blocks."""
|
|
keys, section = set(), None
|
|
with open(os.path.join(addon, "config.yaml"), encoding="utf-8") as fh:
|
|
for line in fh:
|
|
if not line.strip() or line.startswith("#"):
|
|
continue
|
|
if not line[0].isspace():
|
|
section = line.split(":", 1)[0].strip()
|
|
continue
|
|
m = re.match(r" ([A-Za-z_][A-Za-z0-9_]*):", line)
|
|
if m and section in ("options", "schema"):
|
|
keys.add(m.group(1))
|
|
return keys
|
|
|
|
|
|
def shape_problem(doc, known, what, allow_empty=False):
|
|
"""Why `doc` is not a document of `known` keys, or None (SAFETY-60/62).
|
|
|
|
Positive property: a JSON object, at least half of whose top-level keys
|
|
(`_`-prefixed comments aside) `known(key)` recognises.
|
|
"""
|
|
if not isinstance(doc, dict):
|
|
return f"{what} is a JSON {type(doc).__name__}, not an object"
|
|
keys = [k for k in doc if not str(k).startswith("_")]
|
|
hits = [k for k in keys if known(k)]
|
|
if (keys or allow_empty) and len(hits) * 2 >= len(keys):
|
|
return None
|
|
msg = (f"{what} is not the expected document: {len(hits)} of {len(keys)} "
|
|
f"top-level key(s) recognised {sorted(map(str, keys))[:8]}")
|
|
inner = doc.get("data")
|
|
if isinstance(inner, dict) and isinstance(inner.get("options"), dict):
|
|
msg += ("; this is `ha apps info --raw-json` output - pass its "
|
|
"`.data.options` instead")
|
|
return msg
|
|
|
|
|
|
def is_runtime(path):
|
|
return any(path == p or path.startswith(p) for p in RUNTIME)
|
|
|
|
|
|
def _git(*args, repo=None):
|
|
"""git, run so it answers regardless of who owns the checkout.
|
|
|
|
⚠️ WITHOUT `safe.directory` THIS TOOL SILENTLY DID HALF ITS JOB. The
|
|
invocation this file's own docstring documents runs the container as uid 0
|
|
against a checkout owned by the operator, and git refuses that:
|
|
|
|
fatal: detected dubious ownership in repository at '/w/goodwe-addon'
|
|
|
|
`_git` then returned non-zero, `last_version_change()` returned None, and
|
|
the no-op check reported SKIPPED - so **the silent-no-op half, which is the
|
|
reason this file exists, had never once run in its documented form.**
|
|
|
|
It degraded honestly: exit 2, INCONCLUSIVE, never a false clearance. But a
|
|
check that cannot fire is not a check, and the two documented environments
|
|
were mutually exclusive - in the container the validator ran and git
|
|
skipped; on a bare host git ran and the validator skipped on aiohttp.
|
|
Neither documented command ran both halves.
|
|
|
|
⚠️ AND IT WAS INVISIBLE IN THE ONLY CASE ANYONE HAD RUN. `main()` consults
|
|
the git result only when `shipped == deployed`; every real run so far
|
|
compared 0.7.0 against 0.9.0 and printed its verdict without needing git.
|
|
|
|
`test_version_gate.py` could not see it either: it invokes the gate with
|
|
`sys.executable` against fixtures it creates as its own uid, so no fixture
|
|
ever met the ownership mismatch. MUTATION.md habit 4, in a fourth shape,
|
|
inside the file written to prevent it.
|
|
|
|
`safe.directory` is the right fix rather than `--user`: it makes the tool
|
|
correct however it is invoked, instead of making every future caller
|
|
remember a flag. The read is read-only, on a repository the operator is
|
|
about to deploy from.
|
|
"""
|
|
where = repo or os.path.dirname(ADDON)
|
|
return subprocess.run(
|
|
["git", "-c", f"safe.directory={where}", "-C", where, *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")
|
|
# SAFETY-60: the validator accepts any dict, so the shape is checked here.
|
|
bad = (shape_problem(opts, option_keys().__contains__, "--options")
|
|
if opts is not None else None)
|
|
if bad:
|
|
problems.append(f"{bad}; the config was NOT checked")
|
|
opts = None
|
|
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")
|
|
elif not a.options:
|
|
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())
|