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
This commit is contained in:
2026-07-09 17:19:29 -04:00
parent 683649424b
commit 11ffe0f9dd
3 changed files with 128 additions and 3 deletions
+58
View File
@@ -1006,6 +1006,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):