feat(kernel): canonical intermediate-state FSM, unknown-state validation, writer timestamps - #59
Conversation
…ion, writer timestamps Refs #51. loop/fsm.py (9+1 vocabulary, legal transitions), unknown_state contract check in both modes, manifest extra_states, EmitError on illegal transitions, terminate() now sets state=terminal, ISO-8601 UTC updated_at/ terminated_at as additive-optional schema fields. Claude-Session: https://claude.ai/code/session_01EJ8zA8Cbi4o2amawpj8bZW
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 61f2d6b0c5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| 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 |
There was a problem hiding this comment.
When a caller passes a typo or undeclared custom state such as state="verfiy", fsm.is_legal_transition() deliberately fails open for unknown endpoints, so this path accepts it and writes it into state.json; the next validate_contract() then fails with unknown_state. Since append_iteration(state=...) is now the writer path for advancing the FSM, it should also verify the target is canonical or listed in manifest.extra_states before mutating the contract.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR introduces a canonical intermediate-state FSM to the kernel, enforces/validates state vocabulary (including optional manifest-declared domain states), and adds ISO-8601 UTC timestamp stamping for state/terminal writers so state artifacts better reflect writer activity.
Changes:
- Add
loop/fsm.pydefining the canonical 9+1 state vocabulary and legal transition table. - Enforce state vocabulary in
loop/contract.py(both validation modes) with optionalmanifest.yaml: extra_states. - Update emit writers (
append_iteration,terminate,sync_state_to_terminal) to advance/stamp FSM state and addupdated_at/terminated_attimestamps; update schemas + docs and extend test coverage.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/test_template_roundtrip.py | Updates template test fixture to new canonical state name. |
| scripts/test_state_vocabulary.py | Adds tests for canonical state acceptance, unknown-state rejection, and manifest-declared extra states across both validation modes. |
| scripts/test_manifest_extra_states_schema.py | Adds schema-level tests for manifest.extra_states. |
| scripts/test_loop_contract_core.py | Updates a drifted fixture state string to the canonical vocabulary. |
| scripts/test_fsm.py | Adds unit tests for FSM state set, adjacency rules, terminal absorbing behavior, approval-wait rules, and fail-open unknowns. |
| scripts/test_emit.py | Adds tests for FSM advancement enforcement and UTC updated_at stamping in emit writers. |
| scripts/test_conformance.py | Updates conformance fixture state to canonical vocabulary. |
| schemas/terminal.schema.json | Adds optional terminated_at field to the terminal schema. |
| schemas/state.schema.json | Adds optional updated_at field to the state schema. |
| schemas/manifest.schema.json | Adds optional extra_states field to the manifest schema. |
| reference/repo-os-contract.md | Documents the canonical 9+1 state vocabulary and new timestamp fields. |
| loop/fsm.py | New canonical FSM vocabulary + transition logic. |
| loop/emit.py | Adds optional state= to append_iteration, stamps updated_at, stamps terminal state marker on terminate, and syncs terminal marker symmetrically. |
| loop/contract.py | Adds unknown_state validation against canonical states plus manifest-declared extras in both validation modes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 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["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) |
| 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}" | ||
| ) |
| 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)) |
| "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 } }, |
Refs #51 (FSM definition + validator half; the deterministic reducer half lands with #50).
What
loop/fsm.py(new, pure stdlib): canonical intermediate-state vocabulary — 9 non-terminal states (intake, plan, critique-plan, queue-tasks, execute-task, verify, repair, replan, approval-wait) + absorbingterminalmarker — with the legal-transition table: terminate-anywhere, approval-anywhere-except-intake, approval-wait resume fan-out (never back to intake), explicit active→active edges, always-legal self-stay, fail-open when either endpoint is unknown.loop/contract.py: newunknown_stateerror checked in both validation modes; contracts may declare extra domain states via a new optionalmanifest.yamlextra_stateskey.loop/emit.py:append_iterationgains an optionalstate=kwarg that advances the FSM withEmitErroron illegal transitions (checked before the RUNLOG write);terminate()now actually setsstate: "terminal"(previously never set — a latent gap; the example fixtures were hand-authored);sync_state_to_terminal()reconciles the state field symmetrically; all writers stamp ISO-8601 UTCupdated_at.state.updated_at,terminal.terminated_at(already stamped byterminate(), never declared),manifest.extra_states.reference/repo-os-contract.md: documents the 9+1 vocabulary and timestamp fields;loop/fsm.pyis the normative transition table; no section renumbering.Test evidence
verify→replanillegal,queue-tasks→verifyillegal, terminal absorbing,intake↛approval-wait, fail-open unknowns, both-validation-modesunknown_state."Planned"→"intake","running"→"execute-task","execute"→"execute-task"in test_loop_contract_core.py:587) — values that predate any vocabulary validation.validate_frontmatterandself_evalgreen.Provenance (Claudex governed lane)
Authored by
gpt-5.6-sol(codex session019f5c05-46fe-7222-b67a-1cc45417bd32), design by a read-only sonnet design pass, adversarially reviewed by two freshclaude-sonnet-5reviewers (first review FAILed the packet on a mis-specified acceptance criterion — fixed via replan packets1-fsm-b, applied byte-identically bygpt-5.6-terra, session019f5c22-6711-7670-8d49-f862ea01f840). Receipts:cx_f606f60f67244b88(replan_required),cx_2d39ba8308548f45(accepted). Deterministic gates run outside the worker.https://claude.ai/code/session_01EJ8zA8Cbi4o2amawpj8bZW