feat(issue-5): single CAS transition boundary for every job-status write
engine.transition is now the only writer of job status. It implements the target table in state-transitions.md §3: claim, non-terminal metadata, explicit terminals, idempotent re-receipt, refusal of conflicting or late signals with an audit entry, stale-attempt rejection, and deadline timeout. Two write paths become one. update_job_status merged fields unconditionally; pick_pending hand-rolled a correct pending->running swap plus its own audit mirror. Neither was wrong alone, but a rule added to one silently missed the other, which is the shape that produced the phase 1 transport bugs. Both keep their signatures and their preserved legacy bodies behind MAM_ENGINE_CAS=0. A merged `status` value is routed to the request that owns it, so a raw field write can no longer slip past the table. Getting that wrong first was caught by the characterization tests: a metadata merge was overwriting the caller's status with the pre-merge value. Lock discipline is now enforced, not described. The lock covers a decision and one record write; audit mirroring is queued and flushed after release, carrying a monotonic transition_seq so ordering survives the move. A test watches for subprocess, socket and sleep calls while the lock is held and asserts none. Structural correction to my own plan, caught by a phase 1 guard: the plan put the engine in lib_py/loop, but delegation would then have had to import a sibling, which is exactly what mixes trees under a frozen loop. The engine belongs with the registry writers, so it lives in lib_py/delegation and loop consumes it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UrDNJkHmm18uu7n7JwV5Qd
This commit is contained in:
1 parent
9389d7ebe2
commit
2532e78453
8 files changed
+794
-3
No files matched your search
@@ -152,3 +152,207 @@ def test_x04_next_seq_and_register_are_unaffected(mam_sandbox, tmp_path, monkeyp
|
||||
assert mqtt_common.next_seq(job_id, reg) == 2
|
||||
assert _load(reg, job_id)["last_seq"] == 2
|
||||
assert _load(reg, job_id)["status"] == "pending", "next_seq must not touch status"
|
||||
|
||||
|
||||
# ==============================================================================
|
||||
# 2. engine — the CAS table (state-transitions.md §3) and lock discipline
|
||||
# ==============================================================================
|
||||
|
||||
def test_engine_pending_to_running_on_claim(mam_sandbox, tmp_path, monkeypatch):
|
||||
monkeypatch.chdir(mam_sandbox)
|
||||
from lib_py.delegation import engine
|
||||
reg = _reg(tmp_path)
|
||||
job_id = _make_job(reg)
|
||||
res = engine.transition(job_id, "claim", registry_dir=reg, expected_status="pending")
|
||||
assert res.committed and res.from_status == "pending" and res.to_status == "running"
|
||||
|
||||
|
||||
@pytest.mark.parametrize("request_", ["progress", "permission_required"])
|
||||
def test_engine_non_terminal_requests_keep_running(mam_sandbox, tmp_path, monkeypatch, request_):
|
||||
monkeypatch.chdir(mam_sandbox)
|
||||
from lib_py.delegation import engine
|
||||
reg = _reg(tmp_path)
|
||||
job_id = _make_job(reg)
|
||||
engine.transition(job_id, "claim", registry_dir=reg)
|
||||
res = engine.transition(job_id, request_, registry_dir=reg,
|
||||
fields={"blocked_reason": "tool"})
|
||||
assert res.committed
|
||||
assert _load(reg, job_id)["status"] == "running", "must stay non-terminal"
|
||||
assert _load(reg, job_id)["blocked_reason"] == "tool"
|
||||
|
||||
|
||||
@pytest.mark.parametrize("terminal", ["completed", "error", "cancelled"])
|
||||
def test_engine_explicit_terminal_commits(mam_sandbox, tmp_path, monkeypatch, terminal):
|
||||
monkeypatch.chdir(mam_sandbox)
|
||||
from lib_py.delegation import engine
|
||||
reg = _reg(tmp_path)
|
||||
job_id = _make_job(reg)
|
||||
engine.transition(job_id, "claim", registry_dir=reg)
|
||||
res = engine.transition(job_id, terminal, registry_dir=reg)
|
||||
assert res.committed and res.to_status == terminal
|
||||
|
||||
|
||||
def test_engine_repeated_terminal_is_an_idempotent_noop(mam_sandbox, tmp_path, monkeypatch):
|
||||
monkeypatch.chdir(mam_sandbox)
|
||||
from lib_py.delegation import engine
|
||||
reg = _reg(tmp_path)
|
||||
job_id = _make_job(reg)
|
||||
engine.transition(job_id, "claim", registry_dir=reg)
|
||||
engine.transition(job_id, "completed", registry_dir=reg)
|
||||
before = _load(reg, job_id)
|
||||
res = engine.transition(job_id, "completed", registry_dir=reg)
|
||||
assert res.committed is False
|
||||
assert "idempotent" in res.reason
|
||||
assert _load(reg, job_id)["transition_seq"] == before["transition_seq"], "no second write"
|
||||
|
||||
|
||||
@pytest.mark.parametrize("late", ["error", "cancelled", "progress"])
|
||||
def test_engine_refuses_to_reverse_a_terminal_and_audits_the_conflict(
|
||||
mam_sandbox, tmp_path, monkeypatch, late
|
||||
):
|
||||
"""A late or conflicting signal must never walk a terminal record backwards."""
|
||||
monkeypatch.chdir(mam_sandbox)
|
||||
from lib_py.delegation import mqtt_common
|
||||
from lib_py.delegation import engine
|
||||
reg = _reg(tmp_path)
|
||||
job_id = _make_job(reg)
|
||||
engine.transition(job_id, "claim", registry_dir=reg)
|
||||
engine.transition(job_id, "completed", registry_dir=reg)
|
||||
|
||||
res = engine.transition(job_id, late, registry_dir=reg, source="late-arrival")
|
||||
assert res.committed is False
|
||||
assert _load(reg, job_id)["status"] == "completed", "terminal must be preserved"
|
||||
events = list(mqtt_common.iter_logged_events(job_id, mqtt_common.get_logs_dir()))
|
||||
refusals = [e for e in events if e.get("event") == "conflict_refused"]
|
||||
assert refusals, "a refused conflict must be auditable"
|
||||
assert refusals[-1]["current"] == "completed"
|
||||
|
||||
|
||||
def test_engine_rejects_a_stale_attempt(mam_sandbox, tmp_path, monkeypatch):
|
||||
monkeypatch.chdir(mam_sandbox)
|
||||
from lib_py.delegation import engine
|
||||
reg = _reg(tmp_path)
|
||||
job_id = _make_job(reg)
|
||||
engine.transition(job_id, "claim", registry_dir=reg, fields={"attempt_id": 2})
|
||||
res = engine.transition(job_id, "completed", registry_dir=reg, expected_attempt=1)
|
||||
assert res.committed is False
|
||||
assert "stale attempt" in res.reason
|
||||
assert _load(reg, job_id)["status"] == "running"
|
||||
|
||||
|
||||
def test_engine_deadline_timeout_becomes_error(mam_sandbox, tmp_path, monkeypatch):
|
||||
monkeypatch.chdir(mam_sandbox)
|
||||
from lib_py.delegation import engine
|
||||
reg = _reg(tmp_path)
|
||||
job_id = _make_job(reg)
|
||||
engine.transition(job_id, "claim", registry_dir=reg)
|
||||
res = engine.transition(job_id, "deadline_timeout", registry_dir=reg)
|
||||
assert res.committed and res.to_status == "error"
|
||||
|
||||
|
||||
def test_engine_precondition_failure_does_not_write(mam_sandbox, tmp_path, monkeypatch):
|
||||
monkeypatch.chdir(mam_sandbox)
|
||||
from lib_py.delegation import engine
|
||||
reg = _reg(tmp_path)
|
||||
job_id = _make_job(reg)
|
||||
engine.transition(job_id, "claim", registry_dir=reg) # now running
|
||||
res = engine.transition(job_id, "claim", registry_dir=reg, expected_status="pending")
|
||||
assert res.committed is False and "precondition failed" in res.reason
|
||||
|
||||
|
||||
def test_engine_transition_seq_is_monotonic(mam_sandbox, tmp_path, monkeypatch):
|
||||
"""The audit mirror runs outside the lock, so ordering rides on this counter."""
|
||||
monkeypatch.chdir(mam_sandbox)
|
||||
from lib_py.delegation import engine
|
||||
reg = _reg(tmp_path)
|
||||
job_id = _make_job(reg)
|
||||
seqs = []
|
||||
for req in ("claim", "progress", "progress", "completed"):
|
||||
engine.transition(job_id, req, registry_dir=reg)
|
||||
seqs.append(_load(reg, job_id)["transition_seq"])
|
||||
assert seqs == sorted(seqs) and len(set(seqs)) == len(seqs), seqs
|
||||
|
||||
|
||||
def test_g5_no_blocking_io_while_the_registry_lock_is_held(mam_sandbox, tmp_path, monkeypatch):
|
||||
"""G5: the lock may cover a decision and one record write, nothing else.
|
||||
|
||||
Watches the calls that can block on another process or the network and
|
||||
asserts none of them happen between acquiring and releasing the lock."""
|
||||
monkeypatch.chdir(mam_sandbox)
|
||||
import socket
|
||||
from lib_py.delegation import mqtt_common
|
||||
from lib_py.delegation import engine
|
||||
|
||||
reg = _reg(tmp_path)
|
||||
job_id = _make_job(reg)
|
||||
|
||||
held = {"depth": 0}
|
||||
violations = []
|
||||
real_lock = mqtt_common.registry_lock
|
||||
|
||||
import contextlib
|
||||
|
||||
@contextlib.contextmanager
|
||||
def watched_lock(registry_dir):
|
||||
held["depth"] += 1
|
||||
try:
|
||||
with real_lock(registry_dir):
|
||||
yield
|
||||
finally:
|
||||
held["depth"] -= 1
|
||||
|
||||
def guard(name):
|
||||
def spy(*args, **kwargs):
|
||||
if held["depth"] > 0:
|
||||
violations.append(name)
|
||||
raise AssertionError("{} must not run under the registry lock".format(name))
|
||||
return spy
|
||||
|
||||
monkeypatch.setattr(mqtt_common, "registry_lock", watched_lock)
|
||||
monkeypatch.setattr(engine.mqtt_common, "registry_lock", watched_lock, raising=False)
|
||||
monkeypatch.setattr(subprocess, "run", guard("subprocess.run"))
|
||||
monkeypatch.setattr(subprocess, "Popen", guard("subprocess.Popen"))
|
||||
monkeypatch.setattr(socket, "socket", guard("socket.socket"))
|
||||
real_sleep = time.sleep
|
||||
|
||||
def sleep_spy(seconds):
|
||||
if held["depth"] > 0:
|
||||
violations.append("time.sleep")
|
||||
return real_sleep(0)
|
||||
|
||||
monkeypatch.setattr(time, "sleep", sleep_spy)
|
||||
|
||||
for req in ("claim", "progress", "completed", "completed", "error"):
|
||||
engine.transition(job_id, req, registry_dir=reg)
|
||||
|
||||
assert not violations, "blocking calls under the lock: {}".format(violations)
|
||||
|
||||
|
||||
def test_g6_every_record_writer_is_the_engine_or_a_declared_exception():
|
||||
"""G6: no new bypass of the transition boundary can be added unnoticed."""
|
||||
import ast
|
||||
|
||||
allowed = {
|
||||
# creation, and the sequence counter, which never touches status
|
||||
("registry.py", "register_job"),
|
||||
("mqtt_common.py", "next_seq"),
|
||||
("mqtt_common.py", "_update_job_status_legacy"),
|
||||
("registry.py", "_pick_pending_legacy"),
|
||||
("engine.py", "transition"),
|
||||
}
|
||||
found = set()
|
||||
for path in list((SKILLS / "lib_py" / "delegation").glob("*.py")) + \
|
||||
list((SKILLS / "lib_py" / "loop").glob("*.py")):
|
||||
tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path))
|
||||
for node in ast.walk(tree):
|
||||
if not isinstance(node, ast.FunctionDef):
|
||||
continue
|
||||
for inner in ast.walk(node):
|
||||
if isinstance(inner, ast.Call):
|
||||
fn = inner.func
|
||||
name = getattr(fn, "attr", getattr(fn, "id", None))
|
||||
if name == "_atomic_write_record":
|
||||
found.add((path.name, node.name))
|
||||
assert found <= allowed, (
|
||||
"job records are written outside the transition boundary: {}".format(found - allowed)
|
||||
)
|
||||
Reference in new issue
Block a user