test(control-plane): isolate _make_observer_simple module-state leak — fixes flaky test_incident_lifecycle

The test_run_once_* cases were flaky/order-dependent. Root cause: observer.observer
derives OBSERVER_STATE_FILE from STATE_DIR at import time. The helper patched
STATE_DIR but never OBSERVER_STATE_FILE, so run_once()/_save_checkpoint() wrote the
checkpoint to the real /opt/homelab/state/observer_checkpoint.json. Those node_checkpoints
(tmp paths tagged with a pytest run number) leaked across tests and across pytest runs;
run_once's `file_path > checkpoint` string compare then skipped/kept events based on
run-number ordering. The helper also never restored the module globals it overwrote.

Replace both ad-hoc helpers with an autouse monkeypatch fixture that redirects every
observer path — including OBSERVER_STATE_FILE — into the per-test tmp_path and reverts
them afterward. Tests no longer touch real disk and are deterministic.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

services/control-plane/tests/test_incident_lifecycle.py

@@ -17,8 +17,23 @@ from observer.observer import Observer, _parse_ts, _atomic_write_json
 # Helpers
 # ---------------------------------------------------------------------------
 
-def _make_observer(tmp_path: Path) -> Observer:
-    """Return an Observer with all runtime paths redirected to tmp_path."""
+@pytest.fixture(autouse=True)
+def _redirect_observer_paths(tmp_path, monkeypatch):
+    """Redirect every observer.observer runtime path into a per-test tmp_path.
+
+    Uses monkeypatch so the patches are reverted after each test — crucially
+    including OBSERVER_STATE_FILE, which observer.observer derives from STATE_DIR
+    *at import time* (OBSERVER_STATE_FILE = STATE_DIR / "observer_checkpoint.json").
+
+    The previous _make_observer_simple helper patched STATE_DIR but never
+    OBSERVER_STATE_FILE, so run_once() / _save_checkpoint() wrote the checkpoint to
+    the real /opt/homelab/state/observer_checkpoint.json. That stale file leaked
+    node_checkpoints (tmp paths tagged with a pytest run number) across tests AND
+    across pytest runs; the string comparison `file_path > node_checkpoints[node]`
+    in run_once then skipped or kept events depending purely on run-number ordering,
+    making the test_run_once_* cases flaky. Redirecting + restoring every path here
+    isolates each test from disk and from sibling tests.
+    """
     import observer.observer as obs_mod
 
     world = tmp_path / "world"
@@ -30,70 +45,25 @@ def _make_observer(tmp_path: Path) -> Observer:
     for d in (world, state, events, logs, repo / "inventory", repo / "hosts"):
         d.mkdir(parents=True, exist_ok=True)
 
-    # Minimal topology so inventory isn't empty (avoids prune-guard early-return)
+    # Minimal topology so inventory isn't empty (avoids prune-guard early-return).
     (repo / "inventory" / "topology.yaml").write_text(
         "nodes:\n  vps:\n    roles: [control-plane]\n    connectivity: {}\n"
     )
 
-    original_world = obs_mod.WORLD_DIR
-    original_state = obs_mod.STATE_DIR
-    original_events = obs_mod.EVENTS_DIR
-    original_logs = obs_mod.LOGS_DIR
-    original_inventory = obs_mod.INVENTORY_TOPOLOGY
-    original_repo = obs_mod.REPO_ROOT
-    original_failed_events = obs_mod.FAILED_EVENTS_DIR
-
-    obs_mod.WORLD_DIR = world
-    obs_mod.STATE_DIR = state
-    obs_mod.EVENTS_DIR = events
-    obs_mod.LOGS_DIR = logs
-    obs_mod.INVENTORY_TOPOLOGY = repo / "inventory" / "topology.yaml"
-    obs_mod.REPO_ROOT = repo
-    obs_mod.FAILED_EVENTS_DIR = state / "observer_failed_events"
-
-    obs = Observer()
-
-    # Restore module-level constants (monkeypatching at module level is sufficient
-    # for the Observer instance which captures paths at construction time via globals)
-    obs_mod.WORLD_DIR = original_world
-    obs_mod.STATE_DIR = original_state
-    obs_mod.EVENTS_DIR = original_events
-    obs_mod.LOGS_DIR = original_logs
-    obs_mod.INVENTORY_TOPOLOGY = original_inventory
-    obs_mod.REPO_ROOT = original_repo
-    obs_mod.FAILED_EVENTS_DIR = original_failed_events
-
-    return obs
+    monkeypatch.setattr(obs_mod, "WORLD_DIR", world)
+    monkeypatch.setattr(obs_mod, "STATE_DIR", state)
+    monkeypatch.setattr(obs_mod, "EVENTS_DIR", events)
+    monkeypatch.setattr(obs_mod, "LOGS_DIR", logs)
+    monkeypatch.setattr(obs_mod, "INVENTORY_TOPOLOGY", repo / "inventory" / "topology.yaml")
+    monkeypatch.setattr(obs_mod, "REPO_ROOT", repo)
+    monkeypatch.setattr(obs_mod, "FAILED_EVENTS_DIR", state / "observer_failed_events")
+    monkeypatch.setattr(obs_mod, "OBSERVER_STATE_FILE", state / "observer_checkpoint.json")
 
 
-def _make_observer_simple(tmp_path: Path):
-    """Return an Observer instance and patch its world_state in-place."""
-    import observer.observer as obs_mod
-
-    world = tmp_path / "world"
-    state = tmp_path / "state"
-    events = tmp_path / "events"
-    logs = tmp_path / "logs"
-    repo = tmp_path / "repo"
-
-    for d in (world, state, events, logs, repo / "inventory", repo / "hosts"):
-        d.mkdir(parents=True, exist_ok=True)
-
-    (repo / "inventory" / "topology.yaml").write_text(
-        "nodes:\n  vps:\n    roles: [control-plane]\n    connectivity: {}\n"
-    )
-
-    # Patch before construction
-    obs_mod.WORLD_DIR = world
-    obs_mod.STATE_DIR = state
-    obs_mod.EVENTS_DIR = events
-    obs_mod.LOGS_DIR = logs
-    obs_mod.INVENTORY_TOPOLOGY = repo / "inventory" / "topology.yaml"
-    obs_mod.REPO_ROOT = repo
-    obs_mod.FAILED_EVENTS_DIR = state / "observer_failed_events"
-
-    obs = Observer()
-    return obs
+def _make_observer_simple(tmp_path: Path) -> Observer:
+    """Return an Observer. All runtime paths are redirected (and restored) by the
+    autouse _redirect_observer_paths fixture, which shares this test's tmp_path."""
+    return Observer()
 
 
 # ---------------------------------------------------------------------------