feat(W-023): revision-mismatch warning on engine-state load (L10b.2, ADR-0157) (#283)
* 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.
This commit is contained in:
parent
834ba23ee9
commit
fbff161a2e
4 changed files with 342 additions and 1 deletions
129
.github/workflows/ratify-proposal.yml
vendored
Normal file
129
.github/workflows/ratify-proposal.yml
vendored
Normal file
|
|
@ -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 <proposal_id> --accept --review-date <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 = <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}"
|
||||||
72
docs/decisions/ADR-0157-revision-mismatch-warning.md
Normal file
72
docs/decisions/ADR-0157-revision-mismatch-warning.md
Normal file
|
|
@ -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)
|
||||||
|
|
@ -16,6 +16,7 @@ import json
|
||||||
import os
|
import os
|
||||||
import subprocess
|
import subprocess
|
||||||
import tempfile
|
import tempfile
|
||||||
|
import warnings
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from typing import Sequence
|
from typing import Sequence
|
||||||
|
|
||||||
|
|
@ -154,7 +155,21 @@ class EngineStateStore:
|
||||||
content = p.read_text(encoding="utf-8").strip()
|
content = p.read_text(encoding="utf-8").strip()
|
||||||
if not content:
|
if not content:
|
||||||
return None
|
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:
|
def exists(self) -> bool:
|
||||||
return (self.path / "manifest.json").exists()
|
return (self.path / "manifest.json").exists()
|
||||||
|
|
|
||||||
125
tests/test_adr_0157_revision_mismatch_warning.py
Normal file
125
tests/test_adr_0157_revision_mismatch_warning.py
Normal file
|
|
@ -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
|
||||||
Loading…
Reference in a new issue