checks / unit (pull_request) Successful in 4m15s
PR #43 was rejected on two findings and had gone unmergeable behind SAFETY-28 and the ALLOC-01 fix. This is the merge commit; the conflict resolution and both fixes are below, because a merge that also changes behaviour has to say so. THE MERGE, AND WHAT IT COST =========================== Two conflicts in main.py, both where SAFETY-28 and this card touched the same method: * `write_setpoint()` split into a site entry point plus a per-unit `_write_unit()` here, while SAFETY-28 put its record gate in the old single-unit body. `write_refusal()` now takes the INVERTER it is about. Asking it with `self.unit` - as release/1.0 does - would authorise A's share on B's wire, and would also trip test_inverter.py part 1's AST scan, which goes red on any read of a Controller N=1 view inside Controller. * the SAFETY-28 gate sits above the clamp and below the non-finite coercion, exactly where release/1.0 put it, and is now asked about `inv`. Fallout the merge produced in the checks, re-aimed rather than relaxed: * every fixture that calls `write_setpoint()` DIRECTLY - the clamp probe, the hung-heartbeat experiment and its serialised counterfactual, the split deadline probe - now arms a real allocation record first. test_inverter.py grows an `arm()` built through the shipped `resolve()`, the same shape test_clamp.py's is: the per-unit envelopes are filed as real `supervised` claims and the site target is their sum, so the water-fill has exactly one answer and the fixture cannot quietly become its own allocator. * test_clamp.py's two-unit failsafe fixture assigned `c.units`, which DEV-02 made a derived property with no setter. It appends a real `Inverter`. BLOCKER 1 - AC 4's ARBITER-BOUND EVIDENCE WAS VACUOUS ===================================================== test_peakguard.py claimed "each unit's `supervised` bound is ITS OWN `max_w` - give inverter B a different rating and the claim filed on inverter B moves with it", and compared B's bound against `_b2.max_w`. `second_inverter()` DEFAULTED B's rating to `LIVE_MAX_W`, which is A's. IT COMPARED (-5000, 5000) AGAINST (-5000, 5000): a number against itself, unable to observe the failure it named. Reproduced: filing the per-unit `device`/`supervised` claims from `self.inverters[0]`'s numbers while leaving `actuator=self.actuator_of(..., inv)` alone left ALL 19 FILES GREEN. At a roster of two with A=5000/B=3000 that makes inverter B's planned envelope (-5000, 5000) instead of (-3000, 3000) - a 2000 W over-command the arbiter believes is supervised. It is a DIFFERENT defect from the actuator variant the static routing pin catches, and only the actuator variant was in the mutation table. * `second_inverter()` gives B 3000 W - a hypothesis, marked as one - for every fixture in the file, so no check there can compare B against A by accident. * the claim is asserted against LITERAL numbers rather than against an expression that can read the same object twice, for `supervised` AND for `device` (whose `dev` had the same defect), plus the envelope the ARBITER RESOLVED for B - a claim filed right and resolved wrong is the same defect one layer down. * a PRECONDITION check fails if the two ratings are ever made equal again. * the BOUND mutation is in the table, beside the actuator one, and was observed RED (3 checks in test_peakguard.py) via tools/mutation_run.sh. ⚠️ THIRD INSTANCE THIS SESSION of a check whose DATA cannot reach the failure it names - ARB-03's call-site scan saw 3 of 8 appends, ALLOC-01's replay generator saw 0 escapes where a widened one saw 39,991. MUTATION.md gains a section for it beside the stale-bytecode and masking ones, and the rule is written at the line: ANY CHECK ASSERTING "X DIFFERS FROM Y" MUST BE RUN ONCE WITH X AND Y ACTUALLY MADE DIFFERENT. BLOCKER 2 - THE LATCH WAS SILENTLY DISARMED BY THE ROLLBACK ROUND TRIP ====================================================================== store.py's rule 2 keeps the flat keys "so a rollback still finds its latch". The operator path that rule contemplates is a ROUND TRIP, and this card lost the latch on it: 1. install migrates -> schema_version=1, unit/inverter-a/... = False 2. operator DOWNGRADES; SAFETY-02 trips inside the downgrade window and the old add-on writes only the flat `sign_tripped: true` 3. operator upgrades again -> `rep.ran` is False, so `_grade_store_migration()` returned early, and `Inverter.__init__` read the namespaced key: False Observed: the controller came up with `sign_wd.tripped = False` while /data/state.json said `sign_tripped: true`. That is store.py's own header sentence - "the file still says sign_tripped: true, the banner is green, and the controller resumes driving the wrong way" - rebuilt by the version marker. A firmware version change with a rollback path was performed on this site on 2026-09-02, so this is not hypothetical. Three changes, each graded by a mutation observed RED: * THE OPERATOR'S CLEAR CLEARS BOTH SPELLINGS. That is what removes the "re-arm forever" objection the `rep.ran` gate existed to answer: with the fossil cleared, `flat True + namespaced False` is unambiguously a downgrade-window latch. * A TRIP MIRRORS THE FLAT PAIR. Rule 2 only ever covered the latch present AT MIGRATION TIME - nothing flat was written for a trip taken afterwards, so an operator who downgraded found `sign_tripped: false` and came up DRIVING. store.py claimed the staleness could only fail toward a SPURIOUS latch. It could not; that sentence is corrected rather than left standing. * THE GUARD RUNS ON EVERY START, and asks the STORE - not the roster - whether anything namespaced is latched. A mirrored trip on B must not re-arm a healthy A, and the store is the right place to ask because it holds every `unit/*/sign_tripped` this install has written, including one belonging to a unit that is not in today's roster. Graded by the three-step scenario itself, run end to end in test_inverter.py part 2, with all three preconditions asserted - including that the catching start did NOT migrate anything, which is what used to skip the guard. Part 2's idempotence check now PRESSES the button instead of hand-writing the store, because the state it used to construct is not one the shipped clear path can produce. Part 3 grades the mirror and the clear on a real roster of two. test_watchdog.py's "the flat key is NOT written" assertion is re-aimed rather than deleted: its argument was right about the READ and wrong about the WRITE. Nothing at runtime reads that key, and the one thing that does ignores it whenever any namespaced latch is set - which a sibling's trip always leaves. What its absence actually cost was the rollback. HANDOVER.md and DOCS.md said the flat key "is never read again", which was the instruction an operator clearing a trip by hand would follow. Both corrected: it is a maintained rollback view, it IS read on every start, and clearing a trip by hand means clearing both spellings. MUTATIONS THIS ROUND, each applied on a purged tree via tools/mutation_run.sh and observed RED: * per-unit `device`/`supervised` BOUNDS from `self.inverters[0]`, actuator left correct -> test_peakguard.py 10g * `_grade_store_migration()` gated on `rep.ran` -> test_inverter.py part 2 * the operator's clear leaving the flat keys set -> test_inverter.py part 2/3 * a trip not mirroring the flat pair -> test_inverter.py part 3 * the guard asking only `self.inverters[0]` -> test_inverter.py part 3 19/19 files green on the merged tree. No hardware was touched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
305 lines
15 KiB
Python
305 lines
15 KiB
Python
"""Persistent state, in /data (the add-on's only durable volume).
|
|
|
|
The maintenance cycle is month-scale state: it MUST survive add-on restarts,
|
|
HA restarts and power cuts, or the battery quietly stops being maintained and
|
|
nothing says so.
|
|
|
|
Times are stored as ISO-8601 with an explicit UTC offset and parsed back to
|
|
aware datetimes. That is not fussiness: the YAML implementation this replaces
|
|
was disabled for hours by exactly one naive-vs-aware subtraction, and it failed
|
|
silently - the automation never even recorded itself as triggered.
|
|
"""
|
|
|
|
import json
|
|
import logging
|
|
import os
|
|
import tempfile
|
|
from datetime import datetime, timezone
|
|
|
|
_LOG = logging.getLogger("goodwe.store")
|
|
|
|
# ⚠️ "auto" is deliberately NOT in here. Controller falls back to the add-on's
|
|
# `auto_start` option only when the key is absent - if a default supplied False,
|
|
# the fallback could never fire and auto_start would silently do nothing on
|
|
# every fresh install.
|
|
#
|
|
# ⚠️ `sign_tripped` IS in here, and the difference from `auto` is the whole
|
|
# reason both decisions are written down. `auto` has an option to fall back to
|
|
# (`auto_start`), so a stored default would mask a fresh install. A latched
|
|
# failsafe has no such fallback: absent means "never tripped", which is exactly
|
|
# what a default of False says.
|
|
#
|
|
# It is persisted for the reason this module's docstring gives for maintenance
|
|
# state, applied to something with a much shorter fuse. SAFETY-02's latch is the
|
|
# only thing standing between a controller that has been caught driving the wrong
|
|
# way and its own restart, and "restart the add-on" is the FIRST thing an operator
|
|
# looking at a FAILSAFE banner will try. Held only in memory it came back armed,
|
|
# into the fault, unattended - measured: after reconstructing Controller against
|
|
# the same store, `tripped` was False, the banner read NOT READY, and it resumed
|
|
# commanding -1000, -2000 W before re-tripping 180 s later. Under a supervisor
|
|
# that restarts a crashed add-on that is a loop, three minutes of wrong-direction
|
|
# power at a time, forever.
|
|
#
|
|
# ⚠️ AND SINCE DEV-02 THE TWO SAFETY-02 KEYS ARE THE *LEGACY* SPELLING. They are
|
|
# FLAT - one `sign_tripped` for a controller that now holds a LIST of inverters -
|
|
# so a trip caused by inverter B would latch inverter A across restarts, and one
|
|
# operator action would clear both. The LIVE latch is per inverter, under
|
|
# `unit/<name>/...`, and that is the one the watchdog is restored from.
|
|
#
|
|
# ⚠️ THE FLAT PAIR IS NOT DEAD, THOUGH: it is the ROLLBACK VIEW. It is written
|
|
# alongside the namespaced keys on a trip and cleared alongside them on the
|
|
# operator's clear, so an add-on that is DOWNGRADED past this schema still finds
|
|
# the latch it should - and so that a trip taken while the install was rolled
|
|
# back is still visible to the version that comes back up. Rule 2 of
|
|
# `migrate_unit_keys()` and `Controller._grade_store_migration()` carry the
|
|
# argument; the short version is that a latch is only worth persisting if it
|
|
# survives the version change an operator makes while staring at the banner.
|
|
DEFAULTS = {
|
|
"phase": "idle",
|
|
"phase_started": None,
|
|
"last_completed": None,
|
|
"last_start_attempt": None,
|
|
"sign_tripped": False,
|
|
"sign_reason": "",
|
|
}
|
|
|
|
# --------------------------------------------------------------------------- #
|
|
# DEV-02: PER-INVERTER KEYS, AND THE MIGRATION IS THE RISKY HALF
|
|
#
|
|
# ⚠️ A LATCH THAT SILENTLY FAILS TO MIGRATE IS A DISARMED SAFETY GUARD ON AN
|
|
# INSTALL THAT BELIEVES IT IS PROTECTED. SAFETY-02's whole reason for being
|
|
# persisted is that "restart the add-on" is the FIRST thing an operator staring
|
|
# at a FAILSAFE banner tries, and a latch held only in memory comes back armed,
|
|
# into the fault, unattended. A latch that is written down under a key nothing
|
|
# reads any more is exactly the same failure with a longer paper trail: the file
|
|
# still says `sign_tripped: true`, the banner is green, and the controller
|
|
# resumes driving the wrong way.
|
|
#
|
|
# So this is a MIGRATION WITH A POSTCONDITION, not a copy. `migrate_unit_keys()`
|
|
# reports what it did AND reads the result back; `Controller.__init__` grades
|
|
# that report and refuses to come up quietly if the latch did not survive. The
|
|
# test that grades it BREAKS the migration and watches the guard fire.
|
|
# --------------------------------------------------------------------------- #
|
|
|
|
# 0 = the flat, one-inverter layout that shipped before DEV-02.
|
|
# 1 = per-inverter keys under `unit/<name>/...`.
|
|
SCHEMA_VERSION = 1
|
|
SCHEMA_KEY = "schema_version"
|
|
|
|
# Which flat keys belong to ONE inverter, and therefore move into its namespace.
|
|
# ⚠️ THE MAINTENANCE KEYS ARE NOT IN HERE AND THAT IS DELIBERATE. `phase`,
|
|
# `phase_started`, `last_completed` and `last_start_attempt` belong to the
|
|
# maintenance state machine, of which this add-on still holds exactly ONE - a
|
|
# monthly recalibration is a pack procedure and per-pack SCHEDULING is a card of
|
|
# its own, not something to infer from a key rename. Moving them here would
|
|
# quietly re-home an install's whole maintenance history under a unit name; they
|
|
# stay where they are, untouched, which is the other half of "migrates without
|
|
# losing its latch OR ITS HISTORY".
|
|
LEGACY_UNIT_KEYS = ("sign_tripped", "sign_reason")
|
|
|
|
|
|
def unit_key(unit: str, key: str) -> str:
|
|
"""The namespaced Store key for `key` on inverter `unit`.
|
|
|
|
⚠️ ONE SPELLING, IN ONE PLACE. `Inverter.key()` calls this and so does the
|
|
migration, so a check can assert that what was migrated is what is read.
|
|
Two transcriptions of a key name is one copy plus a future disagreement
|
|
about which one holds the latch - and the copy that drifts is always the one
|
|
that reads.
|
|
"""
|
|
return f"unit/{unit}/{key}"
|
|
|
|
|
|
class MigrationReport:
|
|
"""What `migrate_unit_keys()` did, and whether it can be believed.
|
|
|
|
⚠️ IT CARRIES `verified`, WHICH IS READ BACK FROM THE STORE RATHER THAN
|
|
ASSUMED FROM THE ASSIGNMENT. That is the difference between this and a
|
|
two-line copy: the caller is able to ask "is the latch that was in the file
|
|
before now under the key the watchdog reads", and a `False` there is a
|
|
disarmed guard, which is an ERROR and not a log line nobody sees.
|
|
"""
|
|
|
|
def __init__(self):
|
|
self.ran = False # did anything need moving
|
|
self.from_version = 0
|
|
self.moved = {} # new key -> value written
|
|
self.kept = [] # keys already namespaced, left alone
|
|
self.verified = True # every move read back equal
|
|
self.problems = [] # human-readable, one per failed move
|
|
# ⚠️ THE SUBSET THAT MATTERS, AND THE DISTINCTION IS NOT PEDANTRY. A
|
|
# store that drops a write of `sign_tripped=False` has lost the
|
|
# DEFAULT - the same statement "absent" already makes, so nothing is
|
|
# gone. One that drops a write of `True` has lost A LATCH. Only the
|
|
# second is a disarmed safety guard, and only the second may be allowed
|
|
# to shout: an alarm that fires on the harmless case is an alarm an
|
|
# operator learns to dismiss before the real one arrives.
|
|
self.lost = []
|
|
|
|
@property
|
|
def ok(self) -> bool:
|
|
"""Nothing that CARRIED STATE failed to land."""
|
|
return not self.lost
|
|
|
|
def explain(self) -> str:
|
|
if not self.ran:
|
|
return (f"store already at schema {self.from_version}; no "
|
|
f"per-inverter migration needed")
|
|
out = (f"migrated {len(self.moved)} flat key(s) to inverter "
|
|
f"namespaces: " + ", ".join(sorted(self.moved)))
|
|
if self.kept:
|
|
out += " (left alone, already namespaced: " + ", ".join(sorted(self.kept)) + ")"
|
|
if self.problems:
|
|
out += " - PROBLEMS: " + "; ".join(self.problems)
|
|
if self.lost:
|
|
out += " - STATE LOST: " + "; ".join(self.lost)
|
|
return out
|
|
|
|
|
|
def migrate_unit_keys(store, unit: str) -> MigrationReport:
|
|
"""Move the pre-DEV-02 flat per-inverter keys into `unit`'s namespace.
|
|
|
|
`unit` is the FIRST inverter in the roster, which on every install that can
|
|
possibly have a flat key is the only one it ever had. A store written by a
|
|
version that knew about more than one inverter is already at
|
|
SCHEMA_VERSION and is not touched.
|
|
|
|
THE THREE RULES, each one a case that would otherwise be a wrong answer:
|
|
|
|
1. IT NEVER OVERWRITES AN EXISTING NAMESPACED VALUE. A store that has both
|
|
spellings has been written by this version, and the namespaced one is
|
|
where the WATCHDOG is restored from. Preferring the flat one HERE would
|
|
resurrect a trip the operator has already cleared, because a migration
|
|
runs before anything can tell the two apart. Deciding between them is
|
|
main.py's job and it has the one rule that can: the flat key carries new
|
|
information only when NOTHING namespaced is latched.
|
|
2. THE FLAT KEYS ARE LEFT IN THE FILE, AND THEY ARE KEPT LIVE. Deleting
|
|
them buys tidiness and costs a rollback: an operator who downgrades the
|
|
add-on after a migration would find no latch at all, which is the
|
|
disarmed guard again in the other direction. Left in place, a downgrade
|
|
finds the trip it had.
|
|
|
|
⚠️ AND "LEFT IN PLACE" IS NOT ENOUGH ON ITS OWN, WHICH IS WHAT THIS RULE
|
|
USED TO SAY. The sentence here read "they go stale from this moment -
|
|
nothing writes them again - and the direction that staleness fails in is
|
|
a SPURIOUS latch, never a missing one". That was true of the latch this
|
|
migration MOVES and false of every trip that happens AFTERWARDS: nothing
|
|
flat was written for those, so a rollback found `sign_tripped: false` and
|
|
came up DRIVING. So main.py mirrors both keys on a trip and clears both
|
|
on the operator's clear. The flat pair is the ROLLBACK VIEW of the latch,
|
|
maintained, not a fossil - which is also what lets main.py's
|
|
`_grade_store_migration()` run on every start and catch a trip that
|
|
happened while the install was rolled back (see its docstring: the flat
|
|
key saying tripped while no inverter came up tripped is the one
|
|
disagreement that can occur, and it is a DOWNGRADE-WINDOW LATCH).
|
|
3. THE VERSION MARKER IS WRITTEN LAST. A crash between the copies and the
|
|
marker leaves the migration to run again, and rule 1 makes running it
|
|
again a no-op. A marker written first would leave a half-migrated store
|
|
looking finished.
|
|
"""
|
|
rep = MigrationReport()
|
|
data = getattr(store, "data", {})
|
|
rep.from_version = int(data.get(SCHEMA_KEY, 0) or 0)
|
|
if rep.from_version >= SCHEMA_VERSION:
|
|
return rep
|
|
|
|
rep.ran = True
|
|
for legacy in LEGACY_UNIT_KEYS:
|
|
target = unit_key(unit, legacy)
|
|
if target in data:
|
|
rep.kept.append(target)
|
|
continue
|
|
# ⚠️ `DEFAULTS[legacy]` RATHER THAN A LITERAL. An install that never
|
|
# tripped has no `sign_tripped` in its file at all, and the value it
|
|
# then lands on has to be the one `Store.load()` would have supplied -
|
|
# otherwise "absent" and "never tripped" stop being the same statement,
|
|
# which is the distinction store.py's own header is about.
|
|
value = data.get(legacy, DEFAULTS.get(legacy))
|
|
store.set(target, value)
|
|
rep.moved[target] = value
|
|
|
|
# THE POSTCONDITION, AND IT IS A READ-BACK RATHER THAN AN ASSUMPTION. This
|
|
# is the difference between this function and a two-line copy: the caller
|
|
# can ask "is the latch that was in the file before now under the key the
|
|
# watchdog reads", through the same `unit_key()` the watchdog's constructor
|
|
# reads through, and a `False` there is a guard that is off on an install
|
|
# whose state file says it is on.
|
|
for target, value in rep.moved.items():
|
|
got = getattr(store, "data", {}).get(target, "<<missing>>")
|
|
if got == value:
|
|
continue
|
|
rep.verified = False
|
|
note = f"{target} was written as {value!r} and reads back as {got!r}"
|
|
rep.problems.append(note)
|
|
legacy = target.rsplit("/", 1)[-1]
|
|
if value != DEFAULTS.get(legacy):
|
|
rep.lost.append(note)
|
|
|
|
if rep.ok:
|
|
store.set(SCHEMA_KEY, SCHEMA_VERSION)
|
|
else:
|
|
# ⚠️ NOT MARKED DONE. A migration that lost state must run again on the
|
|
# next start rather than be recorded as finished - the one thing worse
|
|
# than a failed migration is a failed migration a later version skips.
|
|
_LOG.error("per-inverter store migration LOST STATE and has not been "
|
|
"marked done: %s", rep.explain())
|
|
return rep
|
|
|
|
|
|
class Store:
|
|
def __init__(self, path: str = "/data/state.json"):
|
|
self.path = path
|
|
self.data = dict(DEFAULTS)
|
|
self.load()
|
|
|
|
def load(self) -> None:
|
|
try:
|
|
with open(self.path, "r", encoding="utf-8") as fh:
|
|
loaded = json.load(fh)
|
|
if isinstance(loaded, dict):
|
|
self.data = {**DEFAULTS, **loaded}
|
|
_LOG.info("state restored: phase=%s last_completed=%s "
|
|
"sign_tripped=%s",
|
|
self.data.get("phase"), self.data.get("last_completed"),
|
|
self.data.get("sign_tripped"))
|
|
except FileNotFoundError:
|
|
_LOG.info("no saved state, starting fresh")
|
|
except (json.JSONDecodeError, OSError) as err:
|
|
# A corrupt state file must not stop the controller: losing the
|
|
# maintenance history is recoverable, refusing to run is not.
|
|
_LOG.warning("state unreadable (%s) - starting fresh", err)
|
|
|
|
def save(self) -> None:
|
|
# Atomic replace: a power cut mid-write must not leave a truncated file
|
|
# that reads as "no maintenance ever ran".
|
|
try:
|
|
os.makedirs(os.path.dirname(self.path), exist_ok=True)
|
|
fd, tmp = tempfile.mkstemp(dir=os.path.dirname(self.path))
|
|
with os.fdopen(fd, "w", encoding="utf-8") as fh:
|
|
json.dump(self.data, fh, indent=1)
|
|
os.replace(tmp, self.path)
|
|
except OSError as err:
|
|
_LOG.error("could not persist state: %s", err)
|
|
|
|
# -- typed helpers ------------------------------------------------------
|
|
def get_time(self, key: str):
|
|
raw = self.data.get(key)
|
|
if not raw:
|
|
return None
|
|
try:
|
|
parsed = datetime.fromisoformat(raw)
|
|
except (TypeError, ValueError):
|
|
_LOG.warning("unparseable timestamp for %s: %r", key, raw)
|
|
return None
|
|
# Anything that ever escaped without an offset is treated as UTC rather
|
|
# than raising later in an arithmetic expression.
|
|
return parsed if parsed.tzinfo else parsed.replace(tzinfo=timezone.utc)
|
|
|
|
def set_time(self, key: str, when: datetime | None) -> None:
|
|
self.data[key] = when.astimezone(timezone.utc).isoformat() if when else None
|
|
self.save()
|
|
|
|
def set(self, key: str, value) -> None:
|
|
self.data[key] = value
|
|
self.save()
|