fix(identity): harden ADR-0220 reconciliation inputs (follow-up to #774) (#776)

The two Gemini robustness nits raced the #774 merge: the patch landed on the
branch after GitHub had merged the pre-patch head, so main shipped the
architecture without the migration-input hardening. This re-lands ONLY the
robustness fixes — no identity-semantics change.

- chat/runtime.py: parse identity_scheme with try/except (TypeError, ValueError)
  -> fallback to legacy scheme 1; revision str(... or '') so a null becomes ''
  (unverifiable -> conservative DIVERGED), not the literal 'None'.
- workbench/readers.py: stored_revision=written_at_revision or '' so a None
  revision is handled identically by the shared reconcile helper.
- tests: malformed identity_scheme does not crash the load guard (migrates);
  reader falls back to legacy on malformed scheme; reader missing revision is a
  conservative break.

Verified: 50 identity/migration/reader tests pass. No lane/serving path touched.
This commit is contained in:
Shay 2026-06-15 12:01:29 -07:00 committed by GitHub
parent 6f70e834ff
commit ac5fb35d45
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 79 additions and 3 deletions

View file

@ -795,12 +795,20 @@ class ChatRuntime:
# single source of truth shared with the workbench continuity reader.
self._loaded_engine_identity = str(manifest.get("engine_identity", ""))
if self._loaded_engine_identity:
# A malformed identity_scheme (non-int) falls back to the legacy
# scheme — the verifying migration then reconciles it safely rather
# than crashing the load. A null/absent revision becomes "" so the
# legacy branch treats it as unverifiable (conservative DIVERGED).
try:
stored_scheme = int(manifest.get("identity_scheme", 1))
except (TypeError, ValueError):
stored_scheme = 1
reconciliation = reconcile_loaded_identity(
self.config,
self._engine_identity,
stored_identity=self._loaded_engine_identity,
stored_scheme=int(manifest.get("identity_scheme", 1)),
stored_revision=str(manifest.get("written_at_revision", "")),
stored_scheme=stored_scheme,
stored_revision=str(manifest.get("written_at_revision") or ""),
)
if reconciliation is IdentityReconciliation.DIVERGED:
self.identity_continuity_break = True

View file

@ -180,3 +180,71 @@ def test_legacy_checkpoint_with_different_packs_breaks_without_strict(
with pytest.warns(RuntimeWarning, match="identity continuity break"):
runtime = ChatRuntime(config=RuntimeConfig(), engine_state_path=state_dir)
assert runtime.identity_continuity_break is True
# --- malformed / missing manifest fields must not crash the guard or reader ---
def test_malformed_identity_scheme_does_not_crash_the_load_guard(tmp_path: Path) -> None:
# A non-int identity_scheme must fall back to the legacy scheme (verifying
# migration), not raise on int() while loading the checkpoint.
state_dir = tmp_path / "es"
state_dir.mkdir(parents=True)
legacy_id = compute_legacy_engine_identity(_DEFAULT_PACKS, "old-rev")
(state_dir / "manifest.json").write_text(
json.dumps(
{
"schema_version": 2,
"turn_count": 4,
"written_at_revision": "old-rev",
"engine_identity": legacy_id,
"identity_scheme": "not-an-int", # malformed
},
sort_keys=True,
indent=2,
),
encoding="utf-8",
)
with pytest.warns(RuntimeWarning, match="migrating engine-identity stamp scheme"):
runtime = ChatRuntime(
config=RuntimeConfig(strict_identity_continuity=True),
engine_state_path=state_dir,
)
assert runtime.identity_continuity_break is False
def test_reader_malformed_identity_scheme_falls_back_to_legacy() -> None:
from workbench.readers import _identity_continuity_from_manifest
# A legacy stamp of the CURRENT default packs, but identity_scheme is garbage:
# the reader must fall back to legacy and verify-migrate -> "verified", not crash.
legacy = compute_legacy_engine_identity(_DEFAULT_PACKS, "r1")
result = _identity_continuity_from_manifest(
{
"schema_version": 2,
"turn_count": 1,
"written_at_revision": "r1",
"engine_identity": legacy,
"identity_scheme": "not-an-int",
}
)
assert result is not None
assert result.status == "verified"
def test_reader_missing_revision_is_a_conservative_break() -> None:
from workbench.readers import _identity_continuity_from_manifest
# A legacy stamp with NO written_at_revision can't be verified -> DIVERGED ->
# "break" (never blindly "verified"); and it must not crash on a None revision.
legacy = compute_legacy_engine_identity(_DEFAULT_PACKS, "r1")
result = _identity_continuity_from_manifest(
{
"schema_version": 2,
"turn_count": 1,
"engine_identity": legacy,
# no written_at_revision, no identity_scheme
}
)
assert result is not None
assert result.status == "break"

View file

@ -1739,7 +1739,7 @@ def _identity_continuity_from_manifest(
current_engine_identity,
stored_identity=engine_identity,
stored_scheme=stored_scheme,
stored_revision=written_at_revision,
stored_revision=written_at_revision or "",
)
if reconciliation is IdentityReconciliation.DIVERGED:
summary = "checkpoint identity differs from the current ratified substrate"