From fbff161a2e8f980a10555bf5cb9f002b5bb4535c Mon Sep 17 00:00:00 2001 From: Shay Date: Mon, 25 May 2026 19:56:07 -0700 Subject: [PATCH] feat(W-023): revision-mismatch warning on engine-state load (L10b.2, ADR-0157) (#283) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(W-022): ratify-proposal workflow_dispatch for mobile ratification Adds .github/workflows/ratify-proposal.yml — a manually triggered workflow that lets the operator ratify engine-authored proposals from the GitHub mobile app without needing terminal access. Inputs: proposal_id (required), review_date (default: today UTC), operator_note (optional). Runs `core teaching review --accept`, commits the updated corpus + proposal log to main, and posts a job summary with the accepted chain_id. Shared CONTEMPLATION_ENABLED kill switch disables the entire learning-arc loop (contemplation + ratification) with one toggle. ADR-0155 / ADR-0057 * feat(W-023): revision-mismatch warning on engine-state load (L10b.2, ADR-0157) ADR-0146 §Risks line 127 specified that load_manifest() should compare written_at_revision against the current git SHA and warn if they differ, but never refuse to load (reboot is recovery, not control flow). - EngineStateStore.load_manifest() emits RuntimeWarning when stored and current revisions are both known and do not match - Suppresses warning when either side is "unknown" (offline/packaged builds) - Always returns the manifest; no state is cleared or rejected - Pinned by 8 tests covering match, mismatch, unknown suppression, and missing/empty manifest edge cases ADR-0156 §Out of scope closes; L10b.3 (reboot_event audit entry, W-024) remains. --- .github/workflows/ratify-proposal.yml | 129 ++++++++++++++++++ .../ADR-0157-revision-mismatch-warning.md | 72 ++++++++++ engine_state/__init__.py | 17 ++- ...test_adr_0157_revision_mismatch_warning.py | 125 +++++++++++++++++ 4 files changed, 342 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/ratify-proposal.yml create mode 100644 docs/decisions/ADR-0157-revision-mismatch-warning.md create mode 100644 tests/test_adr_0157_revision_mismatch_warning.py diff --git a/.github/workflows/ratify-proposal.yml b/.github/workflows/ratify-proposal.yml new file mode 100644 index 00000000..a6dcd25d --- /dev/null +++ b/.github/workflows/ratify-proposal.yml @@ -0,0 +1,129 @@ +name: ratify-proposal + +# ADR-0155 / ADR-0057 — operator ratification gate for engine-authored proposals. +# +# Triggered manually via GitHub Actions UI (or GitHub mobile app): +# Actions → ratify-proposal → Run workflow +# +# Inputs +# proposal_id SHA-256 hex from the contemplation run JSON +# (scenes[2].detail.proposal_id) +# review_date YYYY-MM-DD; defaults to today (UTC) +# operator_note optional free-text note written to the proposal log +# +# What it does +# 1. Runs `core teaching review --accept --review-date ` +# 2. Commits the updated corpus + proposal log directly to main +# 3. Posts a job summary with the accepted chain_id +# +# Trust boundary +# Only `accept_proposal` is invoked — the same codepath as local CLI. +# Pre-conditions enforced by ProposalLog: proposal must exist, be pending, +# and carry replay_equivalent=True. No other files are written. +# +# Soft kill switch +# Disabled unless repo variable CONTEMPLATION_ENABLED=true (shared with +# the contemplation workflow; one knob disables the whole learning arc). + +on: + workflow_dispatch: + inputs: + proposal_id: + description: 'proposal_id from the contemplation run JSON (scenes[2].detail.proposal_id)' + required: true + type: string + review_date: + description: 'Review date (YYYY-MM-DD) — leave blank to use today UTC' + required: false + type: string + default: '' + operator_note: + description: 'Optional note recorded in the proposal log' + required: false + type: string + default: '' + +permissions: + contents: write + +concurrency: + group: ratify-proposal + cancel-in-progress: false + +jobs: + ratify: + name: accept proposal ${{ inputs.proposal_id }} + runs-on: ubuntu-latest + timeout-minutes: 15 + if: ${{ vars.CONTEMPLATION_ENABLED == 'true' }} + + steps: + - name: checkout + uses: actions/checkout@v4 + with: + fetch-depth: 1 + + - name: set up uv + uses: astral-sh/setup-uv@v5 + with: + python-version: '3.11' + enable-cache: true + + - name: install dependencies + run: uv pip install -e ".[dev]" + + - name: resolve review date + id: date + run: | + if [ -n "${{ inputs.review_date }}" ]; then + echo "date=${{ inputs.review_date }}" >> "${GITHUB_OUTPUT}" + else + echo "date=$(date -u +%Y-%m-%d)" >> "${GITHUB_OUTPUT}" + fi + + - name: accept proposal + id: accept + env: + PYTHONPATH: ${{ github.workspace }} + run: | + set -euo pipefail + NOTE_FLAG="" + if [ -n "${{ inputs.operator_note }}" ]; then + NOTE_FLAG="--note ${{ inputs.operator_note }}" + fi + OUTPUT=$(uv run core teaching review "${{ inputs.proposal_id }}" \ + --accept \ + --review-date "${{ steps.date.outputs.date }}" \ + ${NOTE_FLAG}) + echo "${OUTPUT}" + # Extract chain_id from "accepted; appended chain_id = " + CHAIN_ID=$(echo "${OUTPUT}" | sed 's/accepted; appended chain_id = //') + echo "chain_id=${CHAIN_ID}" >> "${GITHUB_OUTPUT}" + + - name: commit ratification + run: | + git config user.name "github-actions[bot]" + git config user.email "github-actions[bot]@users.noreply.github.com" + git add \ + teaching/cognition_chains/cognition_chains_v1.jsonl \ + teaching/proposals/proposals.jsonl + git commit -m "ratify(teaching): accept proposal ${{ inputs.proposal_id }} + +chain_id: ${{ steps.accept.outputs.chain_id }} +review_date: ${{ steps.date.outputs.date }} +operator: ${{ github.actor }} +ADR-0057 / ADR-0155" + git push origin main + + - name: job summary + run: | + echo "## Proposal ratified" >> "${GITHUB_STEP_SUMMARY}" + echo "" >> "${GITHUB_STEP_SUMMARY}" + echo "| Field | Value |" >> "${GITHUB_STEP_SUMMARY}" + echo "|---|---|" >> "${GITHUB_STEP_SUMMARY}" + echo "| proposal_id | \`${{ inputs.proposal_id }}\` |" >> "${GITHUB_STEP_SUMMARY}" + echo "| chain_id | \`${{ steps.accept.outputs.chain_id }}\` |" >> "${GITHUB_STEP_SUMMARY}" + echo "| review_date | ${{ steps.date.outputs.date }} |" >> "${GITHUB_STEP_SUMMARY}" + echo "| operator | ${{ github.actor }} |" >> "${GITHUB_STEP_SUMMARY}" + echo "" >> "${GITHUB_STEP_SUMMARY}" + echo "Chain appended to \`teaching/cognition_chains/cognition_chains_v1.jsonl\`." >> "${GITHUB_STEP_SUMMARY}" diff --git a/docs/decisions/ADR-0157-revision-mismatch-warning.md b/docs/decisions/ADR-0157-revision-mismatch-warning.md new file mode 100644 index 00000000..ee24debe --- /dev/null +++ b/docs/decisions/ADR-0157-revision-mismatch-warning.md @@ -0,0 +1,72 @@ +# ADR-0157 — Revision-mismatch warning on engine-state load (W-023 / L10b.2) + +Status: accepted +Date: 2026-05-26 + +## Context + +ADR-0146 §Risks line 127 specified: + +> "Compare `written_at_revision` in `manifest.json` with the current git +> SHA. If they mismatch, log a warning but continue startup (do not refuse +> to start, as a reboot is recovery, not control flow)." + +W-008 and W-022 implemented the manifest write path but never implemented +the read-side comparison. After a `git pull` or a branch switch the engine +silently loads a checkpoint written by a different code version, which can +produce confusing behaviour if serialization formats changed. + +## Decision + +Inside `EngineStateStore.load_manifest()`, after parsing the JSON: + +1. Read `manifest["written_at_revision"]` (stored revision). +2. Call `_git_revision()` to obtain the current HEAD short SHA. +3. If both values are non-empty and not `"unknown"`, and they differ, + emit `warnings.warn(..., RuntimeWarning, stacklevel=2)`. +4. Always return the manifest — never raise, never clear state. + +The warning message names both revisions and suggests clearing +`engine_state/` if unexpected behaviour is observed. + +### Why `warnings.warn` not `logging` + +`warnings` is already used in the codebase (`core/physics/identity.py`). +It is testable via `pytest.warns` without logger configuration, fits +`RuntimeWarning` semantics (a recoverable runtime anomaly), and respects +the standard Python warning filter so operators can suppress or escalate it +via `-W` flags or `PYTHONWARNINGS`. + +### Why suppress when either side is `"unknown"` + +`_git_revision()` returns `"unknown"` when `git` is unavailable (CI +containers, packaged builds, offline environments). Storing `"unknown"` or +comparing against it would always trigger a spurious warning in those +environments. Suppressing when either side is unknown is the +lowest-surprise behaviour. + +## Invariants pinned by tests + +`tests/test_adr_0157_revision_mismatch_warning.py` (8 tests): + +- Matching revision → no `RuntimeWarning` +- Mismatched revision → `RuntimeWarning` emitted, manifest returned intact +- Warning message contains both the stored and current revisions +- `written_at_revision: "unknown"` in stored manifest → no warning +- `_git_revision()` returns `"unknown"` → no warning +- Missing manifest file → `None` returned, no warning +- Empty manifest file → `None` returned, no warning + +## Out of scope + +- **Schema-version migration.** A `schema_version` bump requires a + migration or clear-slate fallback (ADR-0146 §Risks line 125). That is + separate from the revision warning and deferred to a future ADR when + `_SCHEMA_VERSION` is actually incremented. +- **`reboot_event` audit trail entry** — L10b.3 / W-024. + +## Validation + +- `tests/test_adr_0157_revision_mismatch_warning.py` (8 passed) +- `tests/test_adr_0146_engine_state.py` (8 passed — round-trip regression guard) +- `core test --suite smoke` (67 passed) diff --git a/engine_state/__init__.py b/engine_state/__init__.py index 9a5f162e..1bcfc826 100644 --- a/engine_state/__init__.py +++ b/engine_state/__init__.py @@ -16,6 +16,7 @@ import json import os import subprocess import tempfile +import warnings from pathlib import Path from typing import Sequence @@ -154,7 +155,21 @@ class EngineStateStore: content = p.read_text(encoding="utf-8").strip() if not content: return None - return json.loads(content) + manifest = json.loads(content) + # 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") + current_rev = _git_revision() + if stored_rev not in ("unknown", "") and current_rev not in ("unknown", "") and stored_rev != current_rev: + warnings.warn( + f"engine_state checkpoint was written at revision {stored_rev!r} " + f"but the current revision is {current_rev!r}. " + "State may be stale after a code change. " + "Clear engine_state/ if you observe unexpected behaviour.", + RuntimeWarning, + stacklevel=2, + ) + return manifest def exists(self) -> bool: return (self.path / "manifest.json").exists() diff --git a/tests/test_adr_0157_revision_mismatch_warning.py b/tests/test_adr_0157_revision_mismatch_warning.py new file mode 100644 index 00000000..41d88e94 --- /dev/null +++ b/tests/test_adr_0157_revision_mismatch_warning.py @@ -0,0 +1,125 @@ +"""ADR-0157 (W-023) — revision-mismatch warning on engine-state load. + +ADR-0146 §Risks line 127: + Compare written_at_revision in manifest.json with the current git SHA. + If they mismatch, log a warning but continue startup (do not refuse to + start, as a reboot is recovery, not control flow). +""" + +from __future__ import annotations + +import json +import warnings +from unittest.mock import patch + +import pytest + +from engine_state import EngineStateStore + + +def _write_manifest(path, revision: str, schema_version: int = 1, turn_count: int = 3) -> None: + manifest = { + "schema_version": schema_version, + "turn_count": turn_count, + "written_at_revision": revision, + } + (path / "manifest.json").write_text( + json.dumps(manifest, sort_keys=True, indent=2), encoding="utf-8" + ) + + +def test_matching_revision_emits_no_warning(tmp_path) -> None: + _write_manifest(tmp_path, revision="abc123def456") + store = EngineStateStore(tmp_path) + + with patch("engine_state._git_revision", return_value="abc123def456"): + with warnings.catch_warnings(): + warnings.simplefilter("error", RuntimeWarning) + manifest = store.load_manifest() + + assert manifest is not None + assert manifest["written_at_revision"] == "abc123def456" + + +def test_mismatched_revision_emits_runtime_warning(tmp_path) -> None: + _write_manifest(tmp_path, revision="oldrevisionabc") + store = EngineStateStore(tmp_path) + + with patch("engine_state._git_revision", return_value="newrevisionxyz"): + with pytest.warns(RuntimeWarning, match="oldrevisionabc"): + manifest = store.load_manifest() + + assert manifest is not None + + +def test_warning_message_contains_both_revisions(tmp_path) -> None: + _write_manifest(tmp_path, revision="stored000000") + store = EngineStateStore(tmp_path) + + with patch("engine_state._git_revision", return_value="current11111"): + with pytest.warns(RuntimeWarning) as record: + store.load_manifest() + + assert len(record) == 1 + msg = str(record[0].message) + assert "stored000000" in msg + assert "current11111" in msg + + +def test_manifest_returned_intact_despite_mismatch(tmp_path) -> None: + _write_manifest(tmp_path, revision="old", turn_count=42) + store = EngineStateStore(tmp_path) + + with patch("engine_state._git_revision", return_value="new"): + with pytest.warns(RuntimeWarning): + manifest = store.load_manifest() + + assert manifest is not None + assert manifest["turn_count"] == 42 + assert manifest["schema_version"] == 1 + assert manifest["written_at_revision"] == "old" + + +def test_stored_unknown_revision_suppresses_warning(tmp_path) -> None: + _write_manifest(tmp_path, revision="unknown") + store = EngineStateStore(tmp_path) + + with patch("engine_state._git_revision", return_value="abc123"): + with warnings.catch_warnings(): + warnings.simplefilter("error", RuntimeWarning) + manifest = store.load_manifest() + + assert manifest is not None + + +def test_current_unknown_revision_suppresses_warning(tmp_path) -> None: + _write_manifest(tmp_path, revision="abc123def456") + store = EngineStateStore(tmp_path) + + with patch("engine_state._git_revision", return_value="unknown"): + with warnings.catch_warnings(): + warnings.simplefilter("error", RuntimeWarning) + manifest = store.load_manifest() + + assert manifest is not None + + +def test_missing_manifest_returns_none_no_warning(tmp_path) -> None: + store = EngineStateStore(tmp_path) + + with warnings.catch_warnings(): + warnings.simplefilter("error", RuntimeWarning) + result = store.load_manifest() + + assert result is None + + +def test_empty_manifest_returns_none_no_warning(tmp_path) -> None: + (tmp_path / "manifest.json").write_text("", encoding="utf-8") + store = EngineStateStore(tmp_path) + + with warnings.catch_warnings(): + warnings.simplefilter("error", RuntimeWarning) + result = store.load_manifest() + + assert result is None