From 9433ded85ff4091f783b48f055eec980d92070e7 Mon Sep 17 00:00:00 2001 From: cheshirecode Date: Wed, 29 Jul 2026 01:25:57 -0400 Subject: [PATCH] fix: clear terminal loop actions --- .../loop-engineering/references/examples.md | 5 ++ .../loop-engineering/references/protocol.md | 8 +- skills/loop-engineering/scripts/loop_state.py | 17 +++- .../loop-engineering/tests/test_loop_state.py | 89 ++++++++++++++++++- 4 files changed, 112 insertions(+), 7 deletions(-) diff --git a/skills/loop-engineering/references/examples.md b/skills/loop-engineering/references/examples.md index 00afad8..c616e48 100644 --- a/skills/loop-engineering/references/examples.md +++ b/skills/loop-engineering/references/examples.md @@ -26,6 +26,11 @@ otherwise end `budget_exhausted` with failures and the safest next action. When the terminal run itself consumes a declared unit, use `finish --consume 1`; never also `advance` for that same unit. +For `blocked`, `needs_human`, `budget_exhausted`, or `continue_scheduled`, pass +the exact replay or intervention check through `finish --next-action`. For +`complete` or `cancelled`, omit that flag; the script clears the prior running +action so the terminal state cannot advertise obsolete work. + If a fresh check later contradicts evidence in a saved terminal run, append the correction and revalidate the state: diff --git a/skills/loop-engineering/references/protocol.md b/skills/loop-engineering/references/protocol.md index 0a4e14c..ac44a63 100644 --- a/skills/loop-engineering/references/protocol.md +++ b/skills/loop-engineering/references/protocol.md @@ -23,10 +23,12 @@ the JSON. - `finish` records exactly one terminal outcome. `complete` requires `--verification` naming tool output or an artifact. It leaves budget unchanged by default; use `--consume N` when the terminal cycle spent `N` - declared units. Every executed cycle must be accounted exactly once by - `advance` or `finish`. + declared units. Resumable outcomes require `--next-action`; `complete` and + `cancelled` reject that flag and clear the prior running action. Every + executed cycle must be accounted exactly once by `advance` or `finish`. - `annotate` appends corrected evidence without reopening terminal state or - changing the consumed budget. + changing the consumed budget. It cannot add a next action to `complete` or + `cancelled`. - `validate` checks the schema and transition invariants. - `show` prints the five-field contract; `--json` returns the full history. diff --git a/skills/loop-engineering/scripts/loop_state.py b/skills/loop-engineering/scripts/loop_state.py index 4c8ed73..3367451 100755 --- a/skills/loop-engineering/scripts/loop_state.py +++ b/skills/loop-engineering/scripts/loop_state.py @@ -29,6 +29,7 @@ "budget_exhausted", "continue_scheduled", } +NON_RESUMABLE_STATUSES = TERMINAL_STATUSES - RESUMABLE_STATUSES ALL_STATUSES = {"running", *TERMINAL_STATUSES} @@ -242,11 +243,18 @@ def command_finish(args: argparse.Namespace) -> dict[str, Any]: require_running(state) if args.status == "complete" and not args.verification: raise StateError("complete requires --verification from a tool or artifact") + if args.status in RESUMABLE_STATUSES: + if args.next_action is None or not args.next_action.strip(): + raise StateError(f"{args.status} requires --next-action") + next_action = args.next_action + else: + if args.next_action is not None: + raise StateError(f"{args.status} does not accept --next-action") + next_action = "" consume_budget(state, args.consume) state["progress_evidence"].extend(args.evidence) state["terminal_status"] = args.status - if args.next_action is not None: - state["next_action"] = args.next_action + state["next_action"] = next_action state["verification"] = args.verification or "" append_history( state, f"finished:{args.status}", args.evidence, state["next_action"] @@ -257,6 +265,11 @@ def command_finish(args: argparse.Namespace) -> dict[str, Any]: def command_annotate(args: argparse.Namespace) -> dict[str, Any]: state = read_state(args.state) + if ( + state["terminal_status"] in NON_RESUMABLE_STATUSES + and args.next_action is not None + ): + raise StateError(f"{state['terminal_status']} does not accept --next-action") state["progress_evidence"].extend(args.evidence) if args.next_action is not None: state["next_action"] = args.next_action diff --git a/skills/loop-engineering/tests/test_loop_state.py b/skills/loop-engineering/tests/test_loop_state.py index 0b9b337..427fd89 100755 --- a/skills/loop-engineering/tests/test_loop_state.py +++ b/skills/loop-engineering/tests/test_loop_state.py @@ -145,6 +145,10 @@ def test_resume_creates_bound_successor_without_reopening_blocker(self) -> None: successor["predecessor"]["terminal_status"], "needs_human", ) + self.assertEqual( + json.loads(predecessor_text)["next_action"], + "ask operator to refresh the credential", + ) self.assertEqual( successor["predecessor"]["sha256"], hashlib.sha256(predecessor_text.encode()).hexdigest(), @@ -274,8 +278,6 @@ def test_complete_records_external_verification(self) -> None: "three consecutive passes", "--verification", "pytest log artifact: /tmp/targeted-test.log", - "--next-action", - "checkpoint worklog", "--consume", "1", ) @@ -283,6 +285,87 @@ def test_complete_records_external_verification(self) -> None: self.assertEqual(state["terminal_status"], "complete") self.assertIn("pytest log", state["verification"]) self.assertEqual(state["budget"]["used"], 1) + self.assertEqual(state["next_action"], "") + self.assertEqual(state["history"][-1]["next_action"], "") + + def test_non_resumable_finish_rejects_explicit_next_action_atomically( + self, + ) -> None: + self.initialize() + before = self.state.read_text() + result = self.run_cli( + "finish", + "--state", + str(self.state), + "--status", + "complete", + "--evidence", + "targeted test passed", + "--verification", + "pytest log", + "--next-action", + "stale follow-up", + expected_returncode=3, + ) + self.assertIn("complete does not accept --next-action", result.stderr) + self.assertEqual(self.state.read_text(), before) + + def test_resumable_finish_requires_actionable_next_step(self) -> None: + self.initialize() + before = self.state.read_text() + result = self.run_cli( + "finish", + "--state", + str(self.state), + "--status", + "blocked", + "--evidence", + "credential unavailable", + expected_returncode=3, + ) + self.assertIn("blocked requires --next-action", result.stderr) + self.assertEqual(self.state.read_text(), before) + + def test_cancelled_finish_clears_running_next_action(self) -> None: + self.initialize() + state = json.loads( + self.run_cli( + "finish", + "--state", + str(self.state), + "--status", + "cancelled", + "--evidence", + "operator cancelled the run", + ).stdout + ) + self.assertEqual(state["terminal_status"], "cancelled") + self.assertEqual(state["next_action"], "") + + def test_annotate_rejects_next_action_for_non_resumable_terminal(self) -> None: + self.initialize() + self.run_cli( + "finish", + "--state", + str(self.state), + "--status", + "cancelled", + "--evidence", + "operator cancelled the run", + ) + before = self.state.read_text() + result = self.run_cli( + "annotate", + "--state", + str(self.state), + "--evidence", + "correction: cancellation was explicit", + "--next-action", + "reopen the cancelled run", + expected_returncode=3, + ) + self.assertIn("cancelled does not accept --next-action", result.stderr) + self.assertEqual(self.state.read_text(), before) def test_runtime_rejection_and_cli_misuse_are_distinct_and_atomic(self) -> None: self.initialize(limit=1) @@ -295,6 +378,8 @@ def test_runtime_rejection_and_cli_misuse_are_distinct_and_atomic(self) -> None: "blocked", "--evidence", "two retries attempted", + "--next-action", + "request another retry", "--consume", "2", expected_returncode=3,