Merge pull request #563 from AssetOverflow/feat/l10-engine-state-hardening

L10 engine-state hardening: byte-stability tests + schema-version migration (steps 1+2)

chat/runtime.py

@@ -681,10 +681,13 @@ class ChatRuntime:
         store = self._engine_state_store
         if store is None:
             return
+        # Schema-version compatibility gates the whole load: load_manifest()
+        # refuses (raises) a checkpoint written by newer code BEFORE we read any
+        # recognizers/candidates (L10 step-2 migration discipline).
+        manifest = store.load_manifest() or {}
         recognizers = store.load_recognizers()
         self._recognizer_registry = RecognizerRegistry.from_recognizers(recognizers)
         self._pending_candidates = store.load_discovery_candidates()
-        manifest = store.load_manifest() or {}
         self._turn_count = int(manifest.get("turn_count", 0))
         # W-024 / ADR-0158 — buffer reboot event for emission when sink attaches.
         from engine_state import _git_revision

engine_state/__init__.py

@@ -106,6 +106,14 @@ def _git_revision() -> str:
     return get_git_revision()
 
 
+class IncompatibleEngineStateError(RuntimeError):
+    """Raised when an engine-state checkpoint was written by a NEWER schema than
+    this build supports (L10 step-2 migration discipline).  Older/equal versions
+    are tolerated via additive-optional defaults; a newer checkpoint is refused
+    rather than silently mis-loaded.
+    """
+
+
 class EngineStateStore:
     def __init__(self, path: Path | None = None) -> None:
         self.path = path or _DEFAULT_DIR
@@ -169,6 +177,17 @@ class EngineStateStore:
         if not content:
             return None
         manifest = json.loads(content)
+        # L10 step-2 migration discipline: tolerate schema_version <= current
+        # (additive-optional fields read via defaults); REFUSE a newer checkpoint
+        # rather than silently mis-load state written by code we don't understand.
+        stored_version = manifest.get("schema_version", 0)
+        if not isinstance(stored_version, int) or stored_version > _SCHEMA_VERSION:
+            raise IncompatibleEngineStateError(
+                f"engine_state manifest schema_version {stored_version!r} is newer "
+                f"than this build supports ({_SCHEMA_VERSION}). Refusing to load: a "
+                "newer checkpoint cannot be safely read by older code. Run the "
+                "matching build, or clear engine_state/ explicitly."
+            )
         # W-023 / ADR-0157 — revision-mismatch warning per ADR-0146 §Risks line 127.
         # Never refuse to load; reboot is recovery, not control flow.
         stored_rev = manifest.get("written_at_revision", "unknown")
@@ -188,4 +207,4 @@ class EngineStateStore:
         return (self.path / "manifest.json").exists()
 
 
-__all__ = ["EngineStateStore", "get_git_revision"]
+__all__ = ["EngineStateStore", "IncompatibleEngineStateError", "get_git_revision"]

recognition/anti_unifier.py

@@ -78,6 +78,11 @@ class DerivedRecognizer:
     @classmethod
     def from_json(cls, payload: str) -> "DerivedRecognizer":
         raw = json.loads(payload)
+        # L10 engine_state migration discipline (step-2): the v1 keys below are
+        # required.  Any field ADDED in a later schema_version must be read via
+        # raw.get(name, default) and omitted-when-default in as_dict(), so old
+        # checkpoints load without migration and un-evolved records stay
+        # byte-identical (cf. DiscoveryCandidate's C1 fields for the pattern).
         return cls(
             pattern=tuple(_pattern_element_from_dict(element) for element in raw["pattern"]),
             teaching_set_id=str(raw["teaching_set_id"]),

tests/test_adr_0146_engine_state.py

@@ -2,6 +2,8 @@ from __future__ import annotations
 
 import json
 
+import pytest
+
 from chat.runtime import ChatRuntime
 from engine_state import EngineStateStore
 from recognition.anti_unifier import Constant, DerivedRecognizer, TypedSlot
@@ -137,3 +139,98 @@ def test_discovery_candidate_from_dict_round_trips() -> None:
     roundtrip = DiscoveryCandidate.from_dict(candidate.as_dict())
 
     assert roundtrip == candidate
+
+
+# --- ADR-0146 byte-stability + store-path coverage (added 2026-06-05) ----------
+# The round-trip tests above prove OBJECT equality. The cross-reboot identity the
+# telos needs (the shelved EngineIdentity content-hash) additionally requires the
+# on-disk serialization to be IDEMPOTENT (save->load->save reproduces the bytes)
+# and DETERMINISTIC (same logical state -> same bytes across independent stores).
+# These lock that. They are deliberately NOT golden-format pins: a *deterministic*
+# format change is harmless for a content hash (it changes consistently), so the
+# real hazards are non-idempotence, run-to-run nondeterminism, and a broken store
+# path -- not the exact byte layout (which must stay free to evolve under a
+# versioned schema, lest every legitimate field-add become a death-and-rebirth).
+
+
+def test_recognizers_save_load_save_is_idempotent(tmp_path) -> None:
+    store = EngineStateStore(tmp_path)
+    recs = [_recognizer("set-1"), _recognizer("set-2")]
+
+    store.save_recognizers(recs)
+    first = (tmp_path / "recognizers.jsonl").read_bytes()
+    store.save_recognizers(store.load_recognizers())
+
+    assert (tmp_path / "recognizers.jsonl").read_bytes() == first
+
+
+def test_recognizers_save_is_deterministic_across_instances(tmp_path) -> None:
+    recs = [_recognizer("set-1"), _recognizer("set-2")]
+
+    EngineStateStore(tmp_path / "a").save_recognizers(recs)
+    EngineStateStore(tmp_path / "b").save_recognizers(recs)
+
+    assert (
+        (tmp_path / "a" / "recognizers.jsonl").read_bytes()
+        == (tmp_path / "b" / "recognizers.jsonl").read_bytes()
+    )
+
+
+def test_discovery_store_round_trips_nonempty_candidate(tmp_path) -> None:
+    # The only existing non-empty discovery test bypasses EngineStateStore
+    # (as_dict -> from_dict in memory); this exercises the STORE's save+load
+    # path for a non-empty candidate, which was previously untested.
+    store = EngineStateStore(tmp_path)
+    candidate = _candidate()
+
+    store.save_discovery_candidates([candidate])
+
+    assert store.load_discovery_candidates() == [candidate]
+
+
+def test_discovery_store_save_load_save_is_idempotent(tmp_path) -> None:
+    store = EngineStateStore(tmp_path)
+
+    store.save_discovery_candidates([_candidate()])
+    first = (tmp_path / "discovery_candidates.jsonl").read_bytes()
+    store.save_discovery_candidates(store.load_discovery_candidates())
+
+    assert (tmp_path / "discovery_candidates.jsonl").read_bytes() == first
+
+
+# --- L10 step-2: schema-version migration discipline (added 2026-06-05) --------
+# Versioned additive-optional migration: tolerate schema_version <= current
+# (older/equal checkpoints read any missing newer fields via defaults), REFUSE
+# schema_version > current (never silently mis-load a checkpoint written by code
+# we don't understand). A version bump is a recorded lineage transition, not a
+# death-and-rebirth.
+
+
+def test_load_manifest_refuses_newer_schema_version(tmp_path) -> None:
+    from engine_state import IncompatibleEngineStateError
+
+    store = EngineStateStore(tmp_path)
+    (tmp_path / "manifest.json").write_text(
+        json.dumps(
+            {"schema_version": 999, "turn_count": 3, "written_at_revision": "deadbeef"}
+        ),
+        encoding="utf-8",
+    )
+
+    with pytest.raises(IncompatibleEngineStateError):
+        store.load_manifest()
+
+
+def test_load_manifest_tolerates_older_schema_version(tmp_path) -> None:
+    store = EngineStateStore(tmp_path)
+    (tmp_path / "manifest.json").write_text(
+        json.dumps(
+            {"schema_version": 0, "turn_count": 5, "written_at_revision": "unknown"}
+        ),
+        encoding="utf-8",
+    )
+
+    manifest = store.load_manifest()
+
+    assert manifest is not None
+    assert manifest["turn_count"] == 5