diff --git a/CHANGELOG.md b/CHANGELOG.md index c7fdf41..6ef6c96 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,10 @@ Versioning follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Fixed + +- **(GH-36)** `find_active_run` no longer mistakes a skill's own JSON artifact (e.g. `validate` writing `.dmx/jobs/{job_id}/validation-report.json`) for a second, permanently-non-terminal loop run. A file now has to actually look like loop state — carry `loop_name`, `task_id`, and `status` — before it's considered a candidate at all; anything else in the job directory is skipped outright rather than defaulting to "non-terminal" when `status` is absent. Previously, a correct `validate` run left two JSON files in the same directory and `loop_advance`/`loop_continue` raised `AmbiguousActiveRun` on every attempt to proceed, with no fix short of hand-editing the artifact to fake a `status: complete` it doesn't have. + ## [0.4.0] — 2026-09-09 ### Added diff --git a/src/dmx/loop_state.py b/src/dmx/loop_state.py index 5b286dc..c2e8e8c 100644 --- a/src/dmx/loop_state.py +++ b/src/dmx/loop_state.py @@ -277,6 +277,14 @@ def find_active_run(workspace_root: Path, job_id: str) -> tuple[str, str] | None next starts — so this should never happen in normal operation. Rather than guess which one is "active", this fails loudly so it can be investigated. + + Note: + The job directory can hold non-state JSON artifacts a skill was + told to write there (e.g. ``validate`` writing + ``validation-report.json`` — see ``dmx-validate.md`` Step 9 and + ``validators/spec_adherence.py``). Those files have no + ``loop_name``/``task_id``/``status`` and are skipped rather than + misread as a second, permanently-non-terminal run (see GH-36). """ job_dir = _job_dir(workspace_root, job_id) if not job_dir.exists(): @@ -288,6 +296,10 @@ def find_active_run(workspace_root: Path, job_id: str) -> tuple[str, str] | None data = json.loads(path.read_text(encoding="utf-8")) except (json.JSONDecodeError, OSError): continue + if not isinstance(data, dict) or not {"loop_name", "task_id", "status"} <= data.keys(): + # Not a loop-state file (e.g. a skill artifact like + # validation-report.json) — not a run candidate at all. + continue if data.get("status") in _TERMINAL_STATUSES: continue candidates.append((data.get("loop_name", ""), data.get("task_id", ""))) diff --git a/tests/test_loop_state.py b/tests/test_loop_state.py index 70eeb03..de43a7c 100644 --- a/tests/test_loop_state.py +++ b/tests/test_loop_state.py @@ -2,6 +2,7 @@ from __future__ import annotations +import json from typing import TYPE_CHECKING import pytest @@ -234,6 +235,48 @@ def test_ignores_malformed_state_file(self, tmp_path: Path) -> None: (job_dir / "dev-broken.json").write_text("not json", encoding="utf-8") assert find_active_run(root, "PAY-1") is None + def test_ignores_skill_artifact_alongside_paused_run(self, tmp_path: Path) -> None: + """GH-36: a skill artifact like validate's validation-report.json + lives in the same job directory but has no loop_name/task_id/status + — it must not be mistaken for a second non-terminal run.""" + root = self._workspace(tmp_path) + write_initial_state(root, "validate", "PAY-1", "task-1", ["s"]) + write_state(root, "PAY-1", "validate", "task-1", {"status": LoopStatus.paused.value}) + job_dir = root / ".dmx" / "jobs" / "PAY-1" + (job_dir / "validation-report.json").write_text( + json.dumps( + { + "commit": "deadbeef", + "scope_items": [], + "scope_creep": [], + "regressions": [], + "edge_cases": [], + } + ), + encoding="utf-8", + ) + assert find_active_run(root, "PAY-1") == ("validate", "task-1") + + def test_ignores_skill_artifact_with_no_real_run(self, tmp_path: Path) -> None: + """The artifact alone (no loop-state file at all) must not surface + as an active run.""" + root = self._workspace(tmp_path) + job_dir = root / ".dmx" / "jobs" / "PAY-1" + job_dir.mkdir(parents=True) + (job_dir / "validation-report.json").write_text( + json.dumps({"commit": "deadbeef", "scope_items": []}), encoding="utf-8" + ) + assert find_active_run(root, "PAY-1") is None + + def test_ignores_non_dict_json_file(self, tmp_path: Path) -> None: + """A JSON file that parses but isn't an object (e.g. a bare list) + must not crash the scan or be mistaken for a run.""" + root = self._workspace(tmp_path) + job_dir = root / ".dmx" / "jobs" / "PAY-1" + job_dir.mkdir(parents=True) + (job_dir / "weird.json").write_text("[1, 2, 3]", encoding="utf-8") + assert find_active_run(root, "PAY-1") is None + # --------------------------------------------------------------------------- # find_pending_run — resuming a loop before its real job id is resolvable