SAFETY-44: decide per key, because one shared flag let an unrelated latch veto the repair
checks / unit (pull_request) Successful in 6m58s
checks / unit (pull_request) Successful in 6m58s
⚠️ REVIEW DEFEATED THIS CARD'S OWN FIX WITH ONE ADDED KEY, and the project's own instrument agreed on the unmodified tree. The guard classified each key and then wrote ONE site-wide `namespaced_latched` for BOTH "on the roster" and "off-roster but not UNIT_A", so a single latched key belonging to a unit the operator had removed from `inverters:` vetoed the repair of the misdirected `inverter-a` latch beside it. Measured, roster ['garage','shed'], store stamped so the migration fix cannot reach it: sign_tripped + unit/inverter-a/sign_tripped -> [('garage', True), ...] ...AND unit/old-shed/sign_tripped -> ⚠️ [('garage', False), ...] A site DRIVING on a state file that reads `sign_tripped: true` - the sentence in this card's own first paragraph - and `tools/rehearse_roster.py` printed `LATCH CAME UP DISARMED` on it, exit 1, against shipped code. It matters more than a normal hole because this guard is, as main.py says, the ONLY repair path for an install already stamped by 0.9.0. ⚠️ AND IT IS INSTANCE #17 OF HABIT 4, IN THE EXTENSION MUTATION.md ADDED TWO DAYS AGO. The two fixtures written for this card each hold ONE arrangement - `_staged_store` has only `inverter-a`, `_other_store` only `inverter-b` - so the flag was never written twice with opposite intents in a single store. A per-key check whose keys have never disagreed WITHIN ONE STORE is a check over whichever key came last. Three variables now, one per case, and a fixture holding BOTH. Four cases driven: stranded-only repairs, +1 unrelated repairs, +2 repairs, off-roster-only does NOT (which is DEV-02's control, still green). ⭐ AND THE MOVED-NOT-LOST RULE HID A GENUINE LATCH LOSS. The match was bare key plus value over the whole of `machine_read`, with no identity - and `DEFAULTS["sign_tripped"] is False`, so the ONLY non-default value a latch can hold is `True`. Any surviving latched roster unit therefore suppressed any other unit's vanished latch. Review measured a build that drops `unit/shed/sign_*` while `unit/garage/sign_*` survives, both latched by one site event so the reasons matched too: none in LEG SAME-STEP: nothing vanished, nothing stranded ... NO FINDINGS in the rehearsed transitions exit 0 A roster unit's persisted SAFETY-02 latch deleted, transition reported clean. The legitimate case is a RENAME - the key left `inverter-a`, which is not on this roster, and arrived under `garage`, which is - so the vanished key's own unit must be OFF the roster. A key whose unit is still on the roster has not been renamed, it has been lost, and no amount of the same value elsewhere makes that untrue. ⚠️ AND THE BOUND I CITED FOR IT DID NOT EXIST. Both the comment and the last commit message said "what M3 (delete the flat rollback view) measures". M3 is not that mutant - it drops `sign_reason` from `LEGACY_UNIT_KEYS` - and the one I meant, M9, vanishes only FLAT keys, so neither can satisfy the `unit/` gate the rule sits behind. NEITHER EXERCISED THE BRANCH; it had no negative control at all, in a card whose subject is checks that cannot reach the failure they name. A citation is the worst carrier for that, because it reads as having been checked. M12 is written for it and driven: a roster unit that stops restoring its latch is exit 1, and the suppression does not swallow it. `misdirected` IS UNREACHABLE FROM THE CALL SITE, and the claim that it was a benefit is withdrawn rather than restated. `unit` is `roster_names(opts)[0]` and `build_roster()` names its first entry from the same list, so `unit` is a member of `roster` by construction; review measured it - dropping the `roster` argument inside `migrate_unit_keys()` survives 28/28. The argument stays for the reason `_verify_moves()` gives for its own `roster=None` branch, it is what a NEW caller gets - and a guard nothing exercises is a guard nobody knows works, so it is now exercised at the function's own level: a wrong `unit` comes back misdirected, the marker is not stamped, and the same call with a roster name stamps it. Dead `if _roster_names else UNIT_A` removed; `roster_names()` never returns []. THE OPERATOR MESSAGE IS ASSERTED. Review deleted it whole (`if stranded:` -> `if False:`) and 28/28 stayed green - `stranded` had no other consumer, so the list existed only to print a line nothing checked, and that line is the entire operator-facing half of the repair. Now: exactly one SAFETY-44 line at ERROR, naming the stranded key and the roster it was measured against. A wrong line citation removed: the call-site comment said "line 315's comment" and "two hundred lines below", and both were wrong (315 is the CFG-03 defaults note; the comment meant is ~54 lines down). Cite the thing, not its coordinates. ⚠️ AND WHAT 5b/5c ACTUALLY PIN IS RECORDED, because review is owed the distinction: `mutant_tree()` asserts its target exists, so EDITING the guarded line makes the file red at the ANCHOR before any behaviour is measured - which looks identical from the exit code and proves much less. It happened twice while writing this commit. The behavioural coverage lives in test_maintenance.py section 13 and test_inverter.py, which build stores rather than strings. Fixture strings no longer say "measured" - they are synthetic, and the repo's other fixture carries an explicit SYNTHETIC header. 28/28 files pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
This commit is contained in:
co-authored by
Claude Opus 5
parent
5e382f52f6
commit
d1b7e3f113
@@ -21,6 +21,7 @@ Read MUTATION.md before you report a survivor.
|
||||
|
||||
import asyncio
|
||||
import dataclasses
|
||||
import logging
|
||||
import re
|
||||
import sys
|
||||
from datetime import datetime, timedelta, timezone
|
||||
@@ -34,7 +35,8 @@ from app.inverter import UNIT_A
|
||||
from app.main import Controller
|
||||
from app.store import (
|
||||
DEFAULTS, MAINT_SCHEMA_KEY, MAINT_SCHEMA_VERSION, MAINT_UNIT_KEYS,
|
||||
SCHEMA_KEY, SCHEMA_VERSION, migrate_maint_keys, unit_key,
|
||||
SCHEMA_KEY, SCHEMA_VERSION, migrate_maint_keys, migrate_unit_keys,
|
||||
unit_key,
|
||||
)
|
||||
|
||||
fails = []
|
||||
@@ -1443,7 +1445,7 @@ print("13. FIXED (SAFETY-44): SAFETY-02's migration takes the roster too, and "
|
||||
# ITSELF: an install that has never run a version with namespaced keys, which is
|
||||
# what makes `migrate_unit_keys()` run at all.
|
||||
_lat_store = MemStore({"sign_tripped": True,
|
||||
"sign_reason": "SIGN_DISAGREEMENT: measured pre-DEV-02"})
|
||||
"sign_reason": "SIGN_DISAGREEMENT: SYNTHETIC FIXTURE, not measured"})
|
||||
_lat, _ = controller(NAMED_OPTS, store=_lat_store)
|
||||
print(f" pre-DEV-02 flat latch + declared roster {_C2_NAMES} -> "
|
||||
f"{_lat.store_migration.explain()}")
|
||||
@@ -1480,9 +1482,9 @@ check("⚠️ ...and the OTHER units on the roster are NOT latched by it. The fl
|
||||
# operator actually produces. Only the GUARD repairs it.
|
||||
_staged_store = MemStore({
|
||||
"sign_tripped": True,
|
||||
"sign_reason": "SIGN_DISAGREEMENT: measured pre-DEV-02",
|
||||
"sign_reason": "SIGN_DISAGREEMENT: SYNTHETIC FIXTURE, not measured",
|
||||
unit_key(UNIT_A, "sign_tripped"): True,
|
||||
unit_key(UNIT_A, "sign_reason"): "SIGN_DISAGREEMENT: measured pre-DEV-02",
|
||||
unit_key(UNIT_A, "sign_reason"): "SIGN_DISAGREEMENT: SYNTHETIC FIXTURE, not measured",
|
||||
SCHEMA_KEY: SCHEMA_VERSION})
|
||||
_staged, _ = controller(NAMED_OPTS, store=_staged_store)
|
||||
print(f" already-stamped store + declared roster -> ran="
|
||||
@@ -1519,10 +1521,111 @@ check("⚠️ a latch stranded under a name that is not `UNIT_A` is NOT re-armed
|
||||
"sibling off it is the defect DEV-02 removed",
|
||||
not any(i.sign_wd.tripped for i in _other.inverters))
|
||||
|
||||
# ⭐ AND THE ARRANGEMENT NEITHER FIXTURE ABOVE HOLDS: **BOTH** KEYS AT ONCE.
|
||||
# ⚠️ THIS IS INSTANCE #17 OF HABIT 4 AND IT WAS FOUND IN THIS CARD'S OWN FIX.
|
||||
# The first version of the guard classified each key and then wrote ONE shared
|
||||
# `namespaced_latched` flag for both "on the roster" and "off-roster but not
|
||||
# UNIT_A". `_staged_store` above holds only an `inverter-a` latch and
|
||||
# `_other_store` below holds only an `inverter-b` one, so the flag was never
|
||||
# written twice with opposite intents in a single store - and a per-key check
|
||||
# whose keys have never disagreed WITHIN ONE STORE is a check over whichever
|
||||
# key happened to come last. Measured on that build, one added key:
|
||||
#
|
||||
# sign_tripped + unit/inverter-a/sign_tripped -> garage latched, repaired
|
||||
# ...AND unit/old-shed/sign_tripped -> ⚠️ NOTHING latched
|
||||
#
|
||||
# A site DRIVING on a state file that reads `sign_tripped: true` - the sentence
|
||||
# this card exists to make false - reached by an operator taking one unit out of
|
||||
# `inverters:` after it had tripped. "A roster is configuration and it changes"
|
||||
# is this module's own phrase, and the second inverter goes in this month.
|
||||
_both_store = MemStore({
|
||||
"sign_tripped": True,
|
||||
"sign_reason": "SIGN_DISAGREEMENT: SYNTHETIC FIXTURE, not measured",
|
||||
unit_key(UNIT_A, "sign_tripped"): True,
|
||||
unit_key(UNIT_A, "sign_reason"): "SIGN_DISAGREEMENT: SYNTHETIC FIXTURE, not measured",
|
||||
unit_key("old-shed", "sign_tripped"): True,
|
||||
unit_key("old-shed", "sign_reason"): "SIGN_DISAGREEMENT: old-shed, since removed",
|
||||
SCHEMA_KEY: SCHEMA_VERSION})
|
||||
_both, _ = controller(NAMED_OPTS, store=_both_store)
|
||||
print(f" stranded `inverter-a` AND an unrelated off-roster `old-shed` -> "
|
||||
f"{[(i.name, i.sign_wd.tripped) for i in _both.inverters]}")
|
||||
check("⭐ A STRANDED `inverter-a` LATCH IS STILL REPAIRED WHEN AN UNRELATED "
|
||||
"OFF-ROSTER LATCH SITS BESIDE IT. One shared flag let `old-shed` veto the "
|
||||
"repair, and the site came up driving on a file that says it is latched",
|
||||
_both.inverters[0].sign_wd.tripped)
|
||||
check("...and the repair still lands on the roster's first unit, not on the "
|
||||
"removed one",
|
||||
_both_store.data.get(unit_key(_C2_NAMES[0], "sign_tripped")) is True)
|
||||
check("⚠️ ...and the reason carried across is the FLAT one - the misdirected "
|
||||
"latch's own - and not `old-shed`'s, which belongs to a different "
|
||||
"physical inverter",
|
||||
"old-shed" not in _both.inverters[0].sign_wd.reason)
|
||||
|
||||
# ⚠️ AND THE OPERATOR MESSAGE IS ASSERTED, because review deleted it whole
|
||||
# (`if stranded:` -> `if False:`) and 28/28 stayed green. `stranded` has no
|
||||
# other consumer, so the list existed only to print a line nothing checked -
|
||||
# and the line is the entire operator-facing half of this repair: it names the
|
||||
# key that was stranded and the roster it was measured against. A repair an
|
||||
# operator cannot see is a repair they cannot confirm.
|
||||
_log_store = MemStore({
|
||||
"sign_tripped": True,
|
||||
"sign_reason": "SIGN_DISAGREEMENT: SYNTHETIC FIXTURE, not measured",
|
||||
unit_key(UNIT_A, "sign_tripped"): True,
|
||||
unit_key(UNIT_A, "sign_reason"): "SIGN_DISAGREEMENT: SYNTHETIC FIXTURE, not measured",
|
||||
SCHEMA_KEY: SCHEMA_VERSION})
|
||||
_records = []
|
||||
_h = logging.Handler()
|
||||
_h.emit = lambda r: _records.append(r.getMessage())
|
||||
logging.getLogger("goodwe").addHandler(_h)
|
||||
try:
|
||||
_logged, _ = controller(NAMED_OPTS, store=_log_store)
|
||||
finally:
|
||||
logging.getLogger("goodwe").removeHandler(_h)
|
||||
_s44_lines = [m for m in _records if "SAFETY-44" in m]
|
||||
print(f" SAFETY-44 operator lines logged: {len(_s44_lines)}")
|
||||
check("⭐ the repair SAYS SO, at ERROR, naming SAFETY-44 - deleting this line "
|
||||
"left the whole suite green, and it is how an operator learns their "
|
||||
"latch was re-armed rather than never lost",
|
||||
len(_s44_lines) == 1)
|
||||
check("...and it names the STRANDED KEY, so the operator can find it in the "
|
||||
"state file rather than being told something happened",
|
||||
any(unit_key(UNIT_A, "sign_tripped") in m for m in _s44_lines))
|
||||
check("...and the ROSTER it was measured against, which is the half that says "
|
||||
"WHY no machine read it",
|
||||
any("garage" in m and "shed" in m for m in _s44_lines))
|
||||
|
||||
# ⭐ AND `misdirected` IS MEASURED AT THE FUNCTION'S OWN LEVEL, BECAUSE IT IS
|
||||
# UNREACHABLE FROM THE CALL SITE. `Controller.__init__` passes
|
||||
# `roster_names(opts)[0]`, which `build_roster()` also names its first entry
|
||||
# from, so `unit` is a member of `roster` by construction and the refusal can
|
||||
# never fire there. Review measured that: dropping the `roster` argument from
|
||||
# `_verify_moves()` inside `migrate_unit_keys()` survives 28/28. The argument is
|
||||
# kept for the reason `_verify_moves()` gives for its own `roster=None` branch -
|
||||
# it is what a NEW caller gets by default - and a guard nothing exercises is a
|
||||
# guard nobody knows works, so it is exercised HERE rather than claimed.
|
||||
_mis_store = MemStore({"sign_tripped": True,
|
||||
"sign_reason": "SIGN_DISAGREEMENT: SYNTHETIC FIXTURE, not measured"})
|
||||
_mis = migrate_unit_keys(_mis_store, UNIT_A, ["garage", "shed"])
|
||||
print(f" migrate_unit_keys(store, 'inverter-a', ['garage','shed']) -> "
|
||||
f"ok={_mis.ok} misdirected={sorted(_mis.misdirected)}")
|
||||
check("⭐ a WRONG `unit` - one that is not on the roster it was handed - comes "
|
||||
"back `misdirected`, which is the refusal `migrate_maint_keys()` already "
|
||||
"had and this one did not",
|
||||
bool(_mis.misdirected) and not _mis.ok)
|
||||
check("...and the schema marker is NOT stamped, so the migration runs again on "
|
||||
"the next start rather than being recorded as finished - the one thing "
|
||||
"store.py's header says must never happen",
|
||||
_mis_store.data.get(SCHEMA_KEY) != SCHEMA_VERSION)
|
||||
check("⚠️ ...while the SAME call with a unit that IS on the roster stamps the "
|
||||
"marker - so the refusal is caused by the name and not by the fixture",
|
||||
migrate_unit_keys(
|
||||
MemStore({"sign_tripped": True, "sign_reason": "x"}),
|
||||
"garage", ["garage", "shed"]).ok)
|
||||
|
||||
# The counterfactual, and it is the accepted baseline habit 7 asks for: the
|
||||
# same store, the same migration, one thing different - the roster shape.
|
||||
_flat_lat_store = MemStore({"sign_tripped": True,
|
||||
"sign_reason": "SIGN_DISAGREEMENT: measured pre-DEV-02"})
|
||||
"sign_reason": "SIGN_DISAGREEMENT: SYNTHETIC FIXTURE, not measured"})
|
||||
_flat_lat, _ = controller(FLAT_OPTS, store=_flat_lat_store)
|
||||
print(f" the same store on the FLAT config -> roster "
|
||||
f"{[(i.name, i.sign_wd.tripped) for i in _flat_lat.inverters]}")
|
||||
|
||||
Reference in New Issue
Block a user