diff --git a/loop/contract.py b/loop/contract.py index 7336723..c0fed3a 100644 --- a/loop/contract.py +++ b/loop/contract.py @@ -4,6 +4,7 @@ from pathlib import Path from typing import Any +from . import fsm from .completion import ( CompletionPolicyError, criteria_satisfy_completion, @@ -218,6 +219,26 @@ def _check_tasks_semantics(data: dict[str, Any] | None, path: Path, issues: list issues.append(ContractIssue("done_without_evidence", f"task {task_id!r} done without evidence", path)) +def _check_state_vocabulary( + state: dict[str, Any] | None, + manifest: dict[str, Any] | None, + path: Path, + issues: list[dict], +) -> None: + """Reject undeclared state names in both validation modes.""" + if not isinstance(state, dict): + return + extra_states = manifest.get("extra_states") if isinstance(manifest, dict) else None + declared = ( + tuple(value for value in extra_states if isinstance(value, str)) + if isinstance(extra_states, list) + else () + ) + value = state.get("state") + if value not in fsm.ALL_STATES + declared: + issues.append(ContractIssue("unknown_state", f"unknown state {value!r}", path)) + + def _validate_tasks(data: dict[str, Any] | None, path: Path, issues: list[dict]) -> None: _require_schema(data, "loop-engineer/tasks@1", path, issues) if data is None: @@ -670,11 +691,13 @@ def validate_contract(target: str | Path) -> dict[str, Any]: if data is not None: _jsonschema_validate(data, name, path, issues) # Cross-field rules JSON Schema cannot express, run in both modes. + _check_state_vocabulary(state, manifest, paths.state, issues) _check_tasks_semantics(tasks, paths.tasks, issues) _check_terminal_contradiction(terminal, paths.terminal, issues) else: _validate_manifest(manifest, paths.manifest, issues) _validate_state(state, paths.state, issues) + _check_state_vocabulary(state, manifest, paths.state, issues) _validate_tasks(tasks, paths.tasks, issues) _validate_terminal(terminal, paths.terminal, issues) diff --git a/loop/emit.py b/loop/emit.py index 097c312..99d8a41 100644 --- a/loop/emit.py +++ b/loop/emit.py @@ -14,6 +14,7 @@ from pathlib import Path from typing import Any, Sequence +from . import fsm from .completion import ( CompletionPolicyError, criteria_satisfy_completion, @@ -127,6 +128,7 @@ def append_iteration( *, iteration_id: int, outcome: str, + state: str | None = None, task_id: str = "", actions: Sequence[str] = (), verify_cmd: str = "", @@ -138,8 +140,16 @@ def append_iteration( .loop/state.json's iteration_id/active_task.""" if outcome not in _ITERATION_OUTCOMES: raise EmitError(f"unknown iteration outcome {outcome!r}; expected one of {_ITERATION_OUTCOMES}") + if state is not None and (not isinstance(state, str) or not state.strip()): + raise EmitError("state must be a non-empty string when provided") _require_iteration_id(iteration_id) paths = _require_contract(target) + current = _read_state(paths) + if state is not None: + if not fsm.is_legal_transition(current.get("state"), state): + raise EmitError(f"illegal FSM transition {current.get('state')!r} -> {state!r}") + current["state"] = state + current["updated_at"] = datetime.now(timezone.utc).isoformat(timespec="seconds") lines = [ "", @@ -176,11 +186,10 @@ def append_iteration( with runlog.open("a", encoding="utf-8") as fh: fh.write("\n".join(lines)) - state = _read_state(paths) - state["iteration_id"] = iteration_id + current["iteration_id"] = iteration_id if task_id: - state["active_task"] = task_id - _write_state(paths, state) + current["active_task"] = task_id + _write_state(paths, current) return runlog @@ -324,7 +333,14 @@ def terminate( ) from exc except OSError as exc: raise EmitError(f"terminal write failed at {terminal_path}: {exc}") from exc + if not fsm.is_legal_transition(current.get("state"), fsm.TERMINAL_MARKER): + raise EmitError( + f"terminal written at {terminal_path} but state.json has no legal transition " + f"from {current.get('state')!r} to {fsm.TERMINAL_MARKER!r}" + ) + current["state"] = fsm.TERMINAL_MARKER current["terminal_state"] = state + current["updated_at"] = datetime.now(timezone.utc).isoformat(timespec="seconds") try: _write_state(paths, current) except OSError as exc: @@ -336,7 +352,7 @@ def terminate( def sync_state_to_terminal(target: str | Path) -> Path: - """Stamp state.json's ``terminal_state`` from an existing terminal record. + """Reconcile state.json's FSM marker and terminal verdict from its end record. The narrow repair for a crash or failed write between the immutable ``terminal_state.json`` creation and the state.json stamp — the two files @@ -354,7 +370,14 @@ def sync_state_to_terminal(target: str | Path) -> Path: if not isinstance(terminal, dict) or terminal.get("state") not in TERMINAL_STATES: raise EmitError(f"terminal_state.json at {terminal_path} does not hold a valid terminal record") current = _read_state(paths) + changed = False if current.get("terminal_state") != terminal["state"]: current["terminal_state"] = terminal["state"] + changed = True + if current.get("state") != fsm.TERMINAL_MARKER: + current["state"] = fsm.TERMINAL_MARKER + changed = True + if changed: + current["updated_at"] = datetime.now(timezone.utc).isoformat(timespec="seconds") _write_state(paths, current) return paths.state diff --git a/loop/fsm.py b/loop/fsm.py new file mode 100644 index 0000000..0e757ea --- /dev/null +++ b/loop/fsm.py @@ -0,0 +1,67 @@ +from __future__ import annotations + + +NON_TERMINAL_STATES = ( + "intake", + "plan", + "critique-plan", + "queue-tasks", + "execute-task", + "verify", + "repair", + "replan", + "approval-wait", +) +TERMINAL_MARKER = "terminal" +ALL_STATES = NON_TERMINAL_STATES + (TERMINAL_MARKER,) + + +_ACTIVE_EDGES = ( + ("intake", "plan"), + ("plan", "critique-plan"), + ("critique-plan", "queue-tasks"), + ("queue-tasks", "execute-task"), + ("execute-task", "verify"), + ("verify", "execute-task"), + ("verify", "repair"), + ("repair", "verify"), + ("repair", "replan"), + ("replan", "queue-tasks"), +) +_APPROVAL_RESUME_TARGETS = ( + "plan", + "critique-plan", + "queue-tasks", + "execute-task", + "verify", + "repair", + "replan", +) + + +def legal_targets(state: object) -> tuple[str, ...]: + """Return canonical state-changing targets in canonical vocabulary order.""" + if state not in ALL_STATES or state == TERMINAL_MARKER: + return () + + def is_target(candidate: str) -> bool: + if candidate == state: + return False + if candidate == TERMINAL_MARKER: + return True + if candidate == "approval-wait" and state != "intake": + return True + if state == "approval-wait" and candidate in _APPROVAL_RESUME_TARGETS: + return True + return (state, candidate) in _ACTIVE_EDGES + + return tuple(candidate for candidate in ALL_STATES if is_target(candidate)) + + +def is_legal_transition(old: object, new: object) -> bool: + """Check known-state adjacency; unknown endpoints deliberately fail open.""" + if old not in ALL_STATES or new not in ALL_STATES: + return True + if old == new: + return True + return new in legal_targets(old) diff --git a/reference/repo-os-contract.md b/reference/repo-os-contract.md index cca4ede..7719079 100644 --- a/reference/repo-os-contract.md +++ b/reference/repo-os-contract.md @@ -238,7 +238,8 @@ Python-FSM realization is chosen, implement the ~100-line pattern or reuse the a | Field | Type | Meaning | |---|---|---| | `iteration_id` | int | Monotonic loop counter (matches latest `RUNLOG` entry). | -| `state` | enum | Current FSM state (`intake` … `verify` … `terminal`). | +| `state` | enum | Current FSM state: `intake`, `plan`, `critique-plan`, `queue-tasks`, `execute-task`, `verify`, `repair`, `replan`, `approval-wait`, or `terminal`. `loop/fsm.py` is normative for the transition table. | +| `updated_at` | string\|null | ISO-8601 UTC timestamp of the last write by a `loop.emit` writer; additive/optional and absent on legacy artifacts. | | `plan_version` | int | Bumped on every replan (lets traces detect churn). | | `active_task` | string\|null | `TASKS.json` id currently in flight. | | `best_score` | number\|null | Best verification score so far (repair productivity is measured against this). | @@ -295,6 +296,7 @@ sets it; resolution clears it; the loop never spawns a fresh untracked attempt ( |---|---|---| | `state` | enum | One of the 7 above. | | `iteration_id` | int | Final iteration count. | +| `terminated_at` | string | ISO-8601 UTC timestamp stamped by `loop.emit.terminate()`; additive/optional, so legacy records without it remain valid. | | `criteria_met` | object | `{ "": true\|false }` for every `SPEC.md` criterion. | | `completion_policy` | object | Completion rule for the criteria map. v1 supports `{ "mode": "all_required" }`; legacy records without the field are interpreted the same way. Optional (additive). Note: a pre-migration `Succeeded` record whose criteria map contains any `false` value fails this rule and needs re-verification. | | `evidence` | string[] | Paths to the verification bundles backing the verdict. | diff --git a/schemas/manifest.schema.json b/schemas/manifest.schema.json index 8dcacef..f9a54d1 100644 --- a/schemas/manifest.schema.json +++ b/schemas/manifest.schema.json @@ -12,6 +12,7 @@ "outputs": { "type": "object" }, "permissions": { "type": "array", "items": { "type": ["string", "object"] } }, "approval_gates": { "type": "array", "items": { "type": "string" } }, + "extra_states": { "type": "array", "items": { "type": "string", "minLength": 1 } }, "policies": { "type": "object", "properties": { diff --git a/schemas/state.schema.json b/schemas/state.schema.json index d899302..e3e8924 100644 --- a/schemas/state.schema.json +++ b/schemas/state.schema.json @@ -30,6 +30,13 @@ "state": { "type": "string" }, + "updated_at": { + "type": [ + "string", + "null" + ], + "description": "ISO-8601 UTC timestamp of the last write by a loop.emit writer. Additive/optional; absent on legacy artifacts." + }, "plan_version": { "type": "integer", "minimum": 0 diff --git a/schemas/terminal.schema.json b/schemas/terminal.schema.json index 525113b..db3ecb9 100644 --- a/schemas/terminal.schema.json +++ b/schemas/terminal.schema.json @@ -30,6 +30,10 @@ "type": "integer", "minimum": 0 }, + "terminated_at": { + "type": "string", + "description": "ISO-8601 UTC timestamp stamped by loop.emit.terminate(). Additive/optional; legacy records without it remain valid." + }, "criteria_met": { "type": "object", "additionalProperties": { diff --git a/scripts/test_conformance.py b/scripts/test_conformance.py index d53a9ad..868bc9f 100644 --- a/scripts/test_conformance.py +++ b/scripts/test_conformance.py @@ -51,7 +51,7 @@ "ITERATION_ID": "0", # quoted in the template -> string "0" -> lifecycle "planned" "PLAN_VERSION": "0", "ACTIVE_TASK_ID": "T1", - "STATE": "Planned", + "STATE": "intake", "BEST_SCORE": "null", "FAILURE_MODE": "", "PENDING_APPROVAL": "null", diff --git a/scripts/test_emit.py b/scripts/test_emit.py index df1e0c7..b6469ec 100644 --- a/scripts/test_emit.py +++ b/scripts/test_emit.py @@ -7,6 +7,7 @@ import json import threading from concurrent.futures import ThreadPoolExecutor +from datetime import datetime, timedelta from pathlib import Path import pytest @@ -56,6 +57,38 @@ def test_append_iteration_rejects_unknown_outcome(workspace): emit.append_iteration(workspace, iteration_id=1, outcome="totally_done") +def test_append_iteration_advances_fsm_state_when_provided(workspace): + emit.append_iteration(workspace, iteration_id=1, outcome="task_passed", state="plan") + state = json.loads((workspace / ".loop" / "state.json").read_text(encoding="utf-8")) + assert state["state"] == "plan" + + +def test_append_iteration_rejects_illegal_fsm_transition(workspace): + runlog = workspace / "RUNLOG.md" + before = runlog.read_text(encoding="utf-8") + with pytest.raises(emit.EmitError): + emit.append_iteration(workspace, iteration_id=1, outcome="task_passed", state="verify") + assert runlog.read_text(encoding="utf-8") == before + + +def test_append_iteration_state_defaults_to_no_op(workspace): + emit.append_iteration(workspace, iteration_id=1, outcome="task_passed") + state = json.loads((workspace / ".loop" / "state.json").read_text(encoding="utf-8")) + assert state["state"] == "intake" + + +def test_append_iteration_stamps_updated_at(workspace): + emit.append_iteration(workspace, iteration_id=1, outcome="task_passed") + state = json.loads((workspace / ".loop" / "state.json").read_text(encoding="utf-8")) + timestamp = datetime.fromisoformat(state["updated_at"]) + assert timestamp.utcoffset() == timedelta(0) + + +def test_append_iteration_rejects_blank_state(workspace): + with pytest.raises(emit.EmitError): + emit.append_iteration(workspace, iteration_id=1, outcome="task_passed", state=" ") + + def test_append_receipt_is_schema_valid(workspace): path = emit.append_receipt( workspace, iteration_id=1, role="write", model="claude-opus", outcome="ok" @@ -86,6 +119,15 @@ def test_terminate_succeeded_with_evidence_passes_doctor(workspace): assert validate_contract(workspace)["ok"] is True +def test_terminate_sets_state_field_to_terminal(workspace): + emit.terminate( + workspace, state="FailedBlocked", criteria_met={"1": False}, evidence=[] + ) + state = json.loads((workspace / ".loop" / "state.json").read_text(encoding="utf-8")) + assert state["state"] == "terminal" + assert datetime.fromisoformat(state["updated_at"]).utcoffset() == timedelta(0) + + @pytest.mark.parametrize( "kwargs", [ @@ -280,6 +322,23 @@ def test_sync_state_to_terminal_reconciles_unstamped_state(workspace): assert not _loop_leftovers(workspace) +def test_sync_state_to_terminal_also_reconciles_state_field(workspace): + emit.terminate( + workspace, state="FailedBlocked", criteria_met={"1": False}, evidence=[] + ) + state_path = workspace / ".loop" / "state.json" + current = json.loads(state_path.read_text(encoding="utf-8")) + current["state"] = "intake" + current.pop("updated_at") + state_path.write_text(json.dumps(current, indent=2) + "\n", encoding="utf-8") + + emit.sync_state_to_terminal(workspace) + + synced = json.loads(state_path.read_text(encoding="utf-8")) + assert synced["state"] == "terminal" + assert datetime.fromisoformat(synced["updated_at"]).utcoffset() == timedelta(0) + + def test_sync_state_to_terminal_requires_a_terminal_record(workspace): with pytest.raises(emit.EmitError, match="nothing to sync"): emit.sync_state_to_terminal(workspace) diff --git a/scripts/test_fsm.py b/scripts/test_fsm.py new file mode 100644 index 0000000..66dd321 --- /dev/null +++ b/scripts/test_fsm.py @@ -0,0 +1,100 @@ +from __future__ import annotations + +import pytest + +from loop import fsm + + +def test_all_states_has_exactly_ten_canonical_names_in_order(): + assert fsm.ALL_STATES == ( + "intake", + "plan", + "critique-plan", + "queue-tasks", + "execute-task", + "verify", + "repair", + "replan", + "approval-wait", + "terminal", + ) + + +def test_non_terminal_states_excludes_terminal_and_has_nine_members(): + assert len(fsm.NON_TERMINAL_STATES) == 9 + assert fsm.TERMINAL_MARKER not in fsm.NON_TERMINAL_STATES + + +def test_happy_path_sequence_is_all_legal(): + sequence = ("intake", "plan", "critique-plan", "queue-tasks", "execute-task", "verify") + assert all(fsm.is_legal_transition(old, new) for old, new in zip(sequence, sequence[1:])) + + +def test_verify_pass_cycles_back_to_execute_task(): + assert fsm.is_legal_transition("verify", "execute-task") + + +def test_verify_fail_routes_to_repair(): + assert fsm.is_legal_transition("verify", "repair") + + +def test_repair_fixed_returns_to_verify(): + assert fsm.is_legal_transition("repair", "verify") + + +def test_repair_cap_exceeded_may_replan(): + assert fsm.is_legal_transition("repair", "replan") + + +def test_replan_requeues_tasks(): + assert fsm.is_legal_transition("replan", "queue-tasks") + + +def test_verify_cannot_skip_repair_to_replan(): + assert not fsm.is_legal_transition("verify", "replan") + + +def test_queue_tasks_cannot_jump_to_verify(): + assert not fsm.is_legal_transition("queue-tasks", "verify") + + +@pytest.mark.parametrize("state", fsm.NON_TERMINAL_STATES) +def test_every_non_terminal_state_can_terminate(state): + assert fsm.is_legal_transition(state, fsm.TERMINAL_MARKER) + + +@pytest.mark.parametrize("target", fsm.NON_TERMINAL_STATES) +def test_terminal_is_absorbing(target): + assert not fsm.is_legal_transition(fsm.TERMINAL_MARKER, target) + assert not fsm.legal_targets(fsm.TERMINAL_MARKER) + + +def test_intake_cannot_reach_approval_wait(): + assert not fsm.is_legal_transition("intake", "approval-wait") + + +@pytest.mark.parametrize("state", fsm.NON_TERMINAL_STATES[1:]) +def test_every_other_active_state_can_reach_approval_wait(state): + assert fsm.is_legal_transition(state, "approval-wait") + + +@pytest.mark.parametrize( + ("target", "expected"), + (("intake", False),) + tuple((state, True) for state in fsm.NON_TERMINAL_STATES[1:]), +) +def test_approval_wait_resumes_into_any_non_intake_active_state(target, expected): + assert fsm.is_legal_transition("approval-wait", target) is expected + + +@pytest.mark.parametrize("state", fsm.ALL_STATES) +def test_self_stay_is_always_legal(state): + assert fsm.is_legal_transition(state, state) + + +def test_unknown_state_names_fail_open(): + assert fsm.is_legal_transition("domain-extra", "verify") + assert fsm.is_legal_transition("verify", "domain-extra") + + +def test_legal_targets_of_unknown_state_is_empty(): + assert not fsm.legal_targets("domain-extra") diff --git a/scripts/test_loop_contract_core.py b/scripts/test_loop_contract_core.py index 4b94bf9..b6409c7 100644 --- a/scripts/test_loop_contract_core.py +++ b/scripts/test_loop_contract_core.py @@ -584,7 +584,7 @@ def test_dg3_inflight_loop_with_null_terminal_is_conformant(tmp_path, monkeypatc state_path = target / ".loop" / "state.json" state = json.loads(state_path.read_text(encoding="utf-8")) state["iteration_id"] = 3 - state["state"] = "execute" + state["state"] = "execute-task" state_path.write_text(json.dumps(state), encoding="utf-8") running = doctor_report(target) assert running["ok"] is True, running["issues"] diff --git a/scripts/test_manifest_extra_states_schema.py b/scripts/test_manifest_extra_states_schema.py new file mode 100644 index 0000000..5d93252 --- /dev/null +++ b/scripts/test_manifest_extra_states_schema.py @@ -0,0 +1,38 @@ +from __future__ import annotations + +import json +from pathlib import Path + +import pytest + + +ROOT = Path(__file__).resolve().parent.parent + + +def _errors(extra_states): + jsonschema = pytest.importorskip("jsonschema") + schema = json.loads((ROOT / "schemas" / "manifest.schema.json").read_text(encoding="utf-8")) + instance = { + "schema": "loop-engineer/manifest@1", + "loop": "demo", + "policies": {"plan_then_execute": True}, + "terminal_states": [ + "Succeeded", + "FailedUnverifiable", + "FailedBlocked", + "FailedBudget", + "FailedSafety", + "FailedSpecGap", + "AbortedByHuman", + ], + "extra_states": extra_states, + } + return list(jsonschema.Draft202012Validator(schema).iter_errors(instance)) + + +def test_manifest_schema_accepts_extra_states_array(): + assert _errors(["domain-review", "domain-publish"]) == [] + + +def test_manifest_schema_rejects_non_string_extra_states_items(): + assert _errors(["domain-review", 7]) diff --git a/scripts/test_state_vocabulary.py b/scripts/test_state_vocabulary.py new file mode 100644 index 0000000..16a4111 --- /dev/null +++ b/scripts/test_state_vocabulary.py @@ -0,0 +1,59 @@ +from __future__ import annotations + +import json +import sys + +import pytest + +from loop import emit, fsm +from loop.contract import validate_contract + + +def _workspace(tmp_path): + target = tmp_path / "vocabulary" + emit.open_contract(target) + return target + + +def _set_state(target, state): + path = target / ".loop" / "state.json" + data = json.loads(path.read_text(encoding="utf-8")) + path.write_text(json.dumps({**data, "state": state}, indent=2) + "\n", encoding="utf-8") + + +@pytest.mark.parametrize("state", fsm.ALL_STATES) +def test_validate_contract_accepts_all_ten_canonical_states(tmp_path, state): + target = _workspace(tmp_path) + _set_state(target, state) + assert validate_contract(target)["ok"] is True + + +def test_validate_contract_flags_unknown_state_as_error(tmp_path): + target = _workspace(tmp_path) + _set_state(target, "domain-review") + report = validate_contract(target) + assert report["ok"] is False + assert any(issue["code"] == "unknown_state" for issue in report["issues"]) + + +def test_validate_contract_accepts_manifest_declared_extra_state(tmp_path): + target = _workspace(tmp_path) + _set_state(target, "domain-review") + manifest = target / ".loop" / "manifest.yaml" + manifest.write_text(manifest.read_text(encoding="utf-8") + "extra_states:\n - domain-review\n", encoding="utf-8") + assert validate_contract(target)["ok"] is True + + +def test_state_vocabulary_check_runs_in_both_modes(tmp_path, monkeypatch): + pytest.importorskip("jsonschema") + target = _workspace(tmp_path) + _set_state(target, "domain-review") + + jsonschema_report = validate_contract(target) + assert jsonschema_report["validation_mode"] == "jsonschema" + assert any(issue["code"] == "unknown_state" for issue in jsonschema_report["issues"]) + + monkeypatch.setitem(sys.modules, "jsonschema", None) + fallback_report = validate_contract(target) + assert fallback_report["validation_mode"] == "structural-fallback" + assert any(issue["code"] == "unknown_state" for issue in fallback_report["issues"]) diff --git a/scripts/test_template_roundtrip.py b/scripts/test_template_roundtrip.py index a7dcbed..2534f8c 100644 --- a/scripts/test_template_roundtrip.py +++ b/scripts/test_template_roundtrip.py @@ -44,7 +44,7 @@ def _fill_values() -> dict[str, str]: "ITERATION_ID": "1", # quoted in state (string), bare in terminal (int) "PLAN_VERSION": "1", # integer "ACTIVE_TASK_ID": "T1", - "STATE": "running", + "STATE": "execute-task", "BEST_SCORE": "null", # number | null "FAILURE_MODE": "", "PENDING_APPROVAL": "null", # object | null