fix: isolate review-mutation ledger from host session-state (#590)

Tests that call patch.dict(os.environ, clear=True) dropped
GITEA_MCP_SESSION_STATE_DIR and re-adopted the operator host cache
under ~/.cache/gitea-tools/session-state. A residual terminal
request_changes mutation then tripped the #332 hard-stop before
test_unknown_profile_blocks could assert the empty-profile gate.

- Pin mcp_session_state.default_state_dir / DEFAULT_STATE_DIR to a
  per-test temp dir in conftest (still honors an explicit env override)
- Clear the decision lock at the start of each TestMergePR test
- Add regression coverage for env-clear isolation

Closes #590

tests/conftest.py

@@ -20,9 +20,11 @@ def _reset_mutation_authority(monkeypatch):
 
     Session-state isolation (#559 / #590 / #594): many tests use
     ``patch.dict(os.environ, ..., clear=True)``, which drops
-    ``GITEA_MCP_SESSION_STATE_DIR``. Pin ``default_state_dir`` /
+    ``GITEA_MCP_SESSION_STATE_DIR``. Relying only on the env var therefore
-    ``DEFAULT_STATE_DIR`` to a per-test temp dir so durable load/save never
+    re-exposes the operator host cache
-    touches host state even after env clears.
+    (``~/.cache/gitea-tools/session-state``) and its review-mutation ledger.
+    Pin ``default_state_dir`` / ``DEFAULT_STATE_DIR`` to a per-test temp dir
+    so durable load/save never touches host state even after env clears.
     """
     monkeypatch.delenv("GITEA_SESSION_PROFILE_LOCK", raising=False)
     # Isolate durable session-state files so tests never share host cache (#559).
@@ -36,6 +38,9 @@ def _reset_mutation_authority(monkeypatch):
     except Exception:
         mcp_session_state = None
     if mcp_session_state is not None:
+        # Survive patch.dict(..., clear=True) that drops the env var (#590).
+        # Still honor an explicit GITEA_MCP_SESSION_STATE_DIR when tests set
+        # their own temp dir (see tests/test_mcp_session_state.py).
         monkeypatch.setattr(
             mcp_session_state, "DEFAULT_STATE_DIR", state_dir, raising=False
         )
@@ -69,6 +74,9 @@ def _reset_mutation_authority(monkeypatch):
     monkeypatch.setattr(mcp_server, "_preflight_capability_baseline_porcelain", None)
     monkeypatch.setattr(mcp_server, "_preflight_resolved_task", None)
     monkeypatch.setattr(mcp_server, "_preflight_reviewer_violation_files", [])
+    # Unit tests must not depend on live host MCP process health. Tests that
+    # use patch.dict(..., clear=True) drop PYTEST_CURRENT_TEST, which would
+    # otherwise re-enable the stale-runtime probe against operator daemons.
     monkeypatch.setattr(
         mcp_server,
         "_check_mcp_runtimes_diagnostics",

tests/test_mcp_server.py

@@ -776,6 +776,7 @@ class TestMergePR(unittest.TestCase):
 
         # Clean ledger each merge test so residual/host durable #332 state
         # cannot short-circuit eligibility gates (#590 / #594).
+        # Host durable state can otherwise leak in when tests clear os.environ.
         mcp_server._save_review_decision_lock(None)
         mcp_server.gitea_load_review_workflow()
         self._lease_patch = _install_owned_reviewer_lease(8)
@@ -1006,6 +1007,64 @@ class TestMergePR(unittest.TestCase):
             r["reasons"])
         self._assert_no_merge_call(mock_api)
 
+    @patch("mcp_server.api_request")
+    @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
+    def test_env_clear_does_not_adopt_host_review_mutation_ledger(
+        self, _auth, mock_api
+    ):
+        """#590: patch.dict(clear=True) must not load host terminal mutations.
+
+        Simulates a dirty host session-state file for profile gitea-default
+        (the profile active when env is fully cleared). Isolation must keep
+        the #332 hard-stop ledger clean so the empty-profile gate fires.
+        """
+        import tempfile
+
+        import mcp_session_state
+
+        poison_dir = tempfile.mkdtemp(prefix="gitea-host-poison-")
+        mcp_session_state.save_state(
+            kind=mcp_session_state.KIND_DECISION_LOCK,
+            profile_identity="gitea-default",
+            state_dir=poison_dir,
+            payload={
+                "task": "review_pr",
+                "remote": "prgs",
+                "final_review_decision_ready": True,
+                "ready_pr_number": 8,
+                "ready_action": "request_changes",
+                "live_mutations": [
+                    {
+                        "pr_number": 8,
+                        "action": "request_changes",
+                        "review_id": 99,
+                        "review_state": "request_changes",
+                    }
+                ],
+            },
+        )
+        # If isolation only used the env var, clear=True would fall back to
+        # DEFAULT_STATE_DIR and adopt this poison ledger.
+        mcp_server._REVIEW_DECISION_LOCK = None
+        mock_api.side_effect = [{"login": "merger-bot"}, self._pr("author-bot")]
+        with patch.object(mcp_session_state, "DEFAULT_STATE_DIR", poison_dir):
+            with patch.dict(os.environ, {}, clear=True):
+                r = gitea_merge_pr(
+                    pr_number=8,
+                    confirmation=self._confirm(8),
+                    remote="prgs",
+                )
+        self.assertFalse(r["performed"])
+        self.assertIn(
+            "profile has no configured allowed operations (fail closed)",
+            r["reasons"],
+        )
+        self.assertFalse(
+            any("terminal review mutation already consumed" in x for x in r["reasons"]),
+            msg=f"host ledger leaked into reasons: {r['reasons']}",
+        )
+        self._assert_no_merge_call(mock_api)
+
     @patch("mcp_server.api_request")
     @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
     def test_profile_without_merge_permission_blocks(self, _auth, mock_api):

tests/test_session_state_isolation.py

@@ -0,0 +1,62 @@
+"""Regression: durable session-state isolation survives env clears (#590)."""
+
+from __future__ import annotations
+
+import os
+from pathlib import Path
+from unittest.mock import patch
+
+import mcp_session_state
+
+
+def test_default_state_dir_survives_env_clear():
+    """conftest pins default_state_dir so clear=True cannot hit host cache."""
+    isolated = os.environ.get("GITEA_MCP_SESSION_STATE_DIR")
+    assert isolated, "conftest must set GITEA_MCP_SESSION_STATE_DIR"
+    host_cache = Path.home() / ".cache" / "gitea-tools" / "session-state"
+    with patch.dict(os.environ, {}, clear=True):
+        assert os.environ.get("GITEA_MCP_SESSION_STATE_DIR") is None
+        resolved = mcp_session_state.default_state_dir()
+        assert resolved == isolated
+        assert Path(resolved).resolve() != host_cache.resolve()
+
+
+def test_decision_lock_save_load_stays_in_isolated_dir():
+    """Review-mutation ledger files land only under the per-test state dir."""
+    isolated = os.environ.get("GITEA_MCP_SESSION_STATE_DIR")
+    assert isolated
+    with patch.dict(os.environ, {}, clear=True):
+        saved = mcp_session_state.save_state(
+            kind=mcp_session_state.KIND_DECISION_LOCK,
+            profile_identity="gitea-default",
+            payload={
+                "live_mutations": [
+                    {"pr_number": 1, "action": "request_changes"}
+                ],
+            },
+        )
+        assert saved is not None
+        path = mcp_session_state.state_file_path(
+            kind=mcp_session_state.KIND_DECISION_LOCK,
+            profile_identity="gitea-default",
+        )
+        assert path.startswith(isolated + os.sep) or path.startswith(
+            isolated + "/"
+        )
+        loaded = mcp_session_state.load_state(
+            kind=mcp_session_state.KIND_DECISION_LOCK,
+            profile_identity="gitea-default",
+        )
+        assert loaded is not None
+        assert loaded.get("live_mutations")
+        host_path = (
+            Path.home()
+            / ".cache"
+            / "gitea-tools"
+            / "session-state"
+            / "review_decision_lock-gitea-default.json"
+        )
+        # Host file may exist from operator use; this write must not refresh it.
+        # Prove our isolated file is distinct and present.
+        assert Path(path).is_file()
+        assert Path(path).resolve() != host_path.resolve()