Complete workplan state model cleanup

tests/test_consistency_check.py

@@ -40,6 +40,7 @@ from consistency_check import (
     render_text,
     report_to_dict,
 )
+from api.workplan_status import ready_review_status
 # _detect_behind_remote and _git_pull are re-exported from consistency_check
 # for backward compat; their canonical implementations live in repo_sync.py.
 
@@ -403,17 +404,22 @@ class TestReportToDict:
 # ---------------------------------------------------------------------------
 
 class TestNormaliseWorkstreamStatus:
-    """FILE_TO_DB_WORKSTREAM_STATUS maps workplan file vocabulary to DB vocabulary.
+    """Legacy workplan/API vocabulary maps to the canonical lifecycle model."""
 
-    Workplan files use task-style "done"; the DB workstream API uses "completed".
-    The C-04 check and fix code must normalise before comparing or PATCHing.
-    """
+    def test_done_maps_to_finished(self):
+        assert normalise_workstream_status("done") == "finished"
 
-    def test_done_maps_to_completed(self):
-        assert normalise_workstream_status("done") == "completed"
+    def test_completed_maps_to_finished(self):
+        assert normalise_workstream_status("completed") == "finished"
 
-    def test_completed_is_identity(self):
-        assert normalise_workstream_status("completed") == "completed"
+    def test_accepted_maps_to_finished(self):
+        assert normalise_workstream_status("accepted") == "finished"
+
+    def test_todo_maps_to_ready_by_default(self):
+        assert normalise_workstream_status("todo") == "ready"
+
+    def test_todo_maps_to_active_when_started(self):
+        assert normalise_workstream_status("todo", has_started=True) == "active"
 
     def test_active_is_identity(self):
         assert normalise_workstream_status("active") == "active"
@@ -428,12 +434,12 @@ class TestNormaliseWorkstreamStatus:
         # Don't crash on unexpected values — return them unchanged
         assert normalise_workstream_status("foobar") == "foobar"
 
-    def test_map_constant_covers_done(self):
-        assert "done" in FILE_TO_DB_WORKSTREAM_STATUS
-        assert FILE_TO_DB_WORKSTREAM_STATUS["done"] == "completed"
+    def test_map_constant_covers_legacy_aliases(self):
+        assert FILE_TO_DB_WORKSTREAM_STATUS["done"] == "finished"
+        assert FILE_TO_DB_WORKSTREAM_STATUS["completed"] == "finished"
 
-    def test_c04_no_spurious_drift_when_done_vs_completed(self):
-        """done (file) vs completed (DB) must NOT be reported as C-04 drift."""
+    def test_c04_no_spurious_drift_when_done_vs_finished(self):
+        """done (file) vs finished (DB) must NOT be reported as C-04 drift."""
         assert normalise_workstream_status("done") == normalise_workstream_status("completed")
 
     def test_c04_real_drift_still_detected(self):
@@ -441,6 +447,55 @@ class TestNormaliseWorkstreamStatus:
         assert normalise_workstream_status("done") != normalise_workstream_status("active")
 
 
+class TestReadyReviewStatus:
+    def _repo_with_commit(self, tmp_path):
+        import subprocess
+        repo = tmp_path / "repo"
+        repo.mkdir()
+        subprocess.run(["git", "-C", str(repo), "init"], check=True, capture_output=True)
+        subprocess.run(["git", "-C", str(repo), "config", "user.email", "test@example.invalid"], check=True)
+        subprocess.run(["git", "-C", str(repo), "config", "user.name", "Test"], check=True)
+        tracked = repo / "src" / "app.py"
+        tracked.parent.mkdir()
+        tracked.write_text("print('one')\n", encoding="utf-8")
+        subprocess.run(["git", "-C", str(repo), "add", "."], check=True, capture_output=True)
+        subprocess.run(["git", "-C", str(repo), "commit", "-m", "init"], check=True, capture_output=True)
+        base = subprocess.check_output(["git", "-C", str(repo), "rev-parse", "HEAD"], text=True).strip()
+        return repo, tracked, base
+
+    def test_same_commit_is_current(self, tmp_path):
+        repo, _tracked, base = self._repo_with_commit(tmp_path)
+
+        result = ready_review_status(repo, base)
+
+        assert result.needs_review is False
+
+    def test_changed_context_path_needs_review(self, tmp_path):
+        import subprocess
+        repo, tracked, base = self._repo_with_commit(tmp_path)
+        tracked.write_text("print('two')\n", encoding="utf-8")
+        subprocess.run(["git", "-C", str(repo), "add", "."], check=True, capture_output=True)
+        subprocess.run(["git", "-C", str(repo), "commit", "-m", "change app"], check=True, capture_output=True)
+
+        result = ready_review_status(repo, base, ["src"])
+
+        assert result.needs_review is True
+        assert result.changed_paths == ("src/app.py",)
+
+    def test_unrelated_context_path_does_not_need_review(self, tmp_path):
+        import subprocess
+        repo, _tracked, base = self._repo_with_commit(tmp_path)
+        docs = repo / "docs" / "note.md"
+        docs.parent.mkdir()
+        docs.write_text("note\n", encoding="utf-8")
+        subprocess.run(["git", "-C", str(repo), "add", "."], check=True, capture_output=True)
+        subprocess.run(["git", "-C", str(repo), "commit", "-m", "docs"], check=True, capture_output=True)
+
+        result = ready_review_status(repo, base, ["src"])
+
+        assert result.needs_review is False
+
+
 # ---------------------------------------------------------------------------
 # STATUS_ORDER / no-regress rule (T01 / C-15)
 # ---------------------------------------------------------------------------

tests/test_routers_core.py

@@ -109,9 +109,18 @@ class TestWorkstreams:
         topic = await _create_topic(client)
         ws = await _create_workstream(client, topic["id"])
 
+        r = await client.patch(f"/workstreams/{ws['id']}", json={"status": "finished"})
+        assert r.status_code == 200
+        assert r.json()["status"] == "finished"
+
+    async def test_legacy_completed_status_is_normalized(self, client):
+        await _create_domain(client)
+        topic = await _create_topic(client)
+        ws = await _create_workstream(client, topic["id"])
+
         r = await client.patch(f"/workstreams/{ws['id']}", json={"status": "completed"})
         assert r.status_code == 200
-        assert r.json()["status"] == "completed"
+        assert r.json()["status"] == "finished"
 
     async def test_filter_by_owner(self, client):
         await _create_domain(client)
@@ -321,11 +330,11 @@ class TestFlowEndpoints:
 
         r = await client.get(f"/flows/workstream/{ws['id']}")
         assert r.status_code == 200
-        assert "completed" in r.json()["reachable"]
+        assert "finished" in r.json()["reachable"]
 
-        r = await client.post(f"/flows/workstream/{ws['id']}/advance/completed")
+        r = await client.post(f"/flows/workstream/{ws['id']}/advance/finished")
         assert r.status_code == 200
-        assert r.json()["current_workstation"] == "completed"
+        assert r.json()["current_workstation"] == "finished"
 
         r = await client.get(f"/workstreams/{ws['id']}")
-        assert r.json()["status"] == "completed"
+        assert r.json()["status"] == "finished"

tests/test_task_flow_engine.py

@@ -12,7 +12,7 @@ def test_all_assertions_satisfied_reports_reachable_workstations():
         workstations=[
             WorkstationDef(name="active"),
             WorkstationDef(
-                name="completed",
+                name="finished",
                 entry_assertions=[
                     AssertionDef(
                         id="tasks.all_done",
@@ -31,7 +31,7 @@ def test_all_assertions_satisfied_reports_reachable_workstations():
     )
 
     assert result.exit_blocked is False
-    assert result.reachable == ["active", "completed"]
+    assert result.reachable == ["active", "finished"]
     assert result.unreachable == []
 
 
@@ -135,18 +135,18 @@ def test_yaml_flow_definitions_load_and_evaluate_representative_entities():
     workstream_result = FlowEngine(
         custom_ops={
             "dependencies.any_incomplete": lambda assertion, obj, values: any(
-                value != assertion.value for value in values
+                value not in assertion.value for value in values
             )
         }
     ).evaluate(
         {
             "status": "active",
             "tasks": [{"status": "done"}],
-            "dependencies": [{"workstation": "completed"}],
+            "dependencies": [{"workstation": "finished"}],
         },
         flows["workstream"],
     )
-    assert "completed" in workstream_result.reachable
+    assert "finished" in workstream_result.reachable
     assert "blocked" in [item.workstation for item in workstream_result.unreachable]
 
     task_result = FlowEngine().evaluate(