feat(cli): event-sourced loop run single-step dispatch with crash-safe resume (Refs #55) - #71
Conversation
…afe resume One bounded dispatch step per call over the SQLite EventStore: reducer-projected next-task selection (depends_on + event-log task_passed completions; TASKS.json stays read-only), a pluggable verifier seam defaulting to fail-loud VerifierNotImplementedError (exit 2, zero writes), EVENT-APPEND-FIRST crash safety (the CAS iteration_appended COMMIT is the single source-of-truth boundary; RUNLOG.md / state.json materialize idempotently from the recorded payload; resume is the same call replaying missing materializations), and Succeeded-only terminals via the existing emit.terminate / emit.sync_state_to_terminal. Adds an idempotency guard to emit.append_iteration (duplicate iteration_id skips the RUNLOG block, still writes state.json). 20 new unconditional tests incl. 3 real-SIGKILL crash-injection tests at the named commit barriers. Refs #55 (phase 1 partial: status/replay landed in #70; isolated verifier subprocess + typed stubs follow in the next slice).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8bbb237dcb
ℹ️ 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".
| task = select_next_task(tasks, projection) | ||
| if task is None: | ||
| done = done_task_ids(tasks, projection) | ||
| if all(task.get("id") in done for task in tasks): |
There was a problem hiding this comment.
Refuse terminal success for empty queues
When TASKS.json contains an empty tasks array, select_next_task() returns None and this all(...) is vacuously true, so loop run appends a terminal_written Succeeded event with criteria_met: {}. Empty task lists are currently doctor-clean, but the reducer/terminal writer rejects empty completion criteria, so the command leaves an invalid terminal event in the append-only store and subsequent replay reports a G1 violation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR introduces a new event-sourced, single-step loop run dispatcher intended to be crash-resumable by treating the SQLite event-store COMMIT as the sole source-of-truth boundary, with legacy artifacts (RUNLOG.md, .loop/state.json, terminal files) materialized idempotently from recorded payloads.
Changes:
- Adds
loop/runner.pywithdispatch_once()and deterministic next-task selection from TASKS + reduced event log. - Extends the CLI (
loop/__main__.py) to include arunverb with--modehandling consistent with other runtime verbs. - Adds regression tests for dispatch/resume behavior (including crash-injection) and documents the crash boundary in
reference/repo-os-contract.md.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/test_runner_dispatch.py | New tests covering bounded dispatch behavior, idempotency, and crash-resume scenarios. |
| reference/repo-os-contract.md | Documents the dispatch crash boundary and TASKS.json read-only dispatch inputs. |
| loop/runner.py | New bounded, event-sourced dispatch implementation and reconciliation of legacy artifacts. |
| loop/emit.py | Adds an idempotency guard to prevent duplicate RUNLOG iteration blocks on retry. |
| loop/main.py | Adds the run verb and integrates it into help/usage and mode parsing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| from .runner import RunnerError, dispatch_once | ||
|
|
||
| try: | ||
| return _print_json(dispatch_once(target, mode=mode)) | ||
| except (RunnerError, RuntimeStoreError, ValidationModeError) as exc: | ||
| print(f"run: {exc}", file=sys.stderr) | ||
| return 2 |
| tasks = raw.get("tasks") if isinstance(raw, dict) else None | ||
| if not isinstance(tasks, list) or not all(isinstance(task, dict) for task in tasks): | ||
| raise RunnerError("TASKS.json must contain a tasks array of objects") | ||
| return tasks |
| payload = { | ||
| "state": "Succeeded", "criteria_met": {task["id"]: True for task in tasks}, | ||
| "evidence": ["RUNLOG.md"], "false_completion": False, | ||
| "completion_policy": {"mode": "all_required"}, "iteration_id": iteration_id, | ||
| } |
| header = f"## Iteration {iteration_id} —" | ||
| if header not in runlog.read_text(encoding="utf-8"): | ||
| with runlog.open("a", encoding="utf-8") as fh: | ||
| fh.write("\n".join(lines)) |
Third slice of the #55 runtime-verbs phase (after #69 TOCTOU and #70 status/replay). Refs #55 — does not close it.
What
loop/runner.py(new):dispatch_once(target, *, verifier=None, mode=None)— exactly one bounded dispatch step per call. Next-task selection folds the event log through the deterministic reducer (depends_on+ event-logtask_passedcompletions; TASKS.json is read-only for dispatch). Verifier is a pluggable seam; the CLI default raises typedVerifierNotImplementedErrorbefore any write (exit 2) — the isolated verifier subprocess arrives in the next slice.iteration_appendedCOMMIT (expected_sequencefrom the same projection read) is the single source-of-truth boundary. Legacy RUNLOG.md/state.json materialize from the exact recorded payload; a crash on either side of the COMMIT is recovered by the same call replaying missing materializations — no separate repair path. Terminal writes are Succeeded-only and reuseemit.terminate/emit.sync_state_to_terminalverbatim.loop/emit.py: minimal idempotency guard inappend_iteration— a retriediteration_idappends no duplicate RUNLOG block but still writes state.json.loop/__main__.py:runverb (--mode basic|strict|release), RunnerError family caught at the CLI edge (exit 2, stderr, no traceback),ok:falseexits 1.reference/repo-os-contract.md: documents the dispatch crash-safety boundary and the TASKS.json-read-only decision.scripts/test_runner_dispatch.py(new): 20 unconditional tests, incl. 3 crash-injection tests (real forked child, real SIGKILL at theKillBeforeCommit/KillAfterCommitbarriers on the iteration and terminal COMMITs).Evidence (governed Claudex lane)
events/reducer/evidence/fsm/completion/contract/scaffold/paths/plan/runtime) byte-unchanged.terminal_supersededlog still noop_terminal; abandoned task neither dispatches nor fakes a Succeeded terminal; a hand-edited foreign RUNLOG block survives resume;--mode strictwithout jsonschema fails closed before any write.state); one directed repair of two test-fixture defects (receipt role vocabulary; SQLite-wal/-shmsidecars in the byte-identity helper). Receipts:cx_s3a_runner_a1(repair_requested),cx_s3a_runner_a2(failed_verification),cx_s3a_runner_b1(accepted). Codex sessions 019f670a…, 019f6710…, 019f671e….Reviewer advisories (non-blocking, recorded)
blockedreport shape is exercised end-to-end only by the governor holdout probe, not the visible suite (baked into the accepted design's test list)._reconcile_legacy_iteration's FSM-walk replay branch is untested until a future slice drives the walk through recoverable paths.