From 1be460026110f69f51f6578a34c87efc8bdfa83d Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Fri, 10 Jul 2026 11:54:05 -0400 Subject: [PATCH] fix: address #625 REQUEST_CHANGES for lease reclaim test and EOF blank line - Update MCP expired-lock recovery test for sticky expired ownership (live pid + present worktree still fail closed under #601 reclaim rules) - Remove trailing blank line at EOF in control_plane_db.py --- control_plane_db.py | 1 - tests/test_mcp_server.py | 22 +++++++++++++++++++--- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/control_plane_db.py b/control_plane_db.py index e51f643..0dd9e38 100644 --- a/control_plane_db.py +++ b/control_plane_db.py @@ -1749,4 +1749,3 @@ class ControlPlaneDB: f"transferred lease ownership from {owner} to {adopter_session_id}" ], } - diff --git a/tests/test_mcp_server.py b/tests/test_mcp_server.py index abdec4b..45ce67e 100644 --- a/tests/test_mcp_server.py +++ b/tests/test_mcp_server.py @@ -5,6 +5,7 @@ the MCP protocol) with mocked API responses. """ import json import os +import shutil import sys import tempfile import unittest @@ -3811,7 +3812,15 @@ class TestIssueLocking(unittest.TestCase): @patch("mcp_server.api_get_all", return_value=[]) @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) def test_lock_issue_blocks_expired_same_operation_lease_for_recovery(self, _auth, _api, _git_state): + """#601: expired lease still blocks takeover when owner pid is live AND worktree exists. + + Dead-pid / missing-worktree reclaim is covered by issue_lock_store unit tests. + This MCP path must remain fail-closed for sticky expired foreign ownership. + """ prgs_repo = mcp_server.REMOTES["prgs"]["repo"] + # Present worktree + live pid => reclaim_allowed=False even though expires_at is past. + sticky_worktree = tempfile.mkdtemp(prefix="gitea-sticky-lease-") + self.addCleanup(lambda: shutil.rmtree(sticky_worktree, ignore_errors=True)) issue_lock_store.save_lock_file( issue_lock_store.lock_file_path( remote="prgs", @@ -3825,15 +3834,22 @@ class TestIssueLocking(unittest.TestCase): "remote": "prgs", "org": "Scaled-Tech-Consulting", "repo": prgs_repo, - "worktree_path": "/tmp/other-worktree", + "worktree_path": sticky_worktree, + "session_pid": os.getpid(), + "pid": os.getpid(), "work_lease": { "operation_type": "author_issue_work", "expires_at": "2000-01-01T00:00:00Z", }, }, ) - with self.assertRaises(RuntimeError) as ctx: - gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs") + with patch.object(issue_lock_store, "is_process_alive", return_value=True): + with self.assertRaises(RuntimeError) as ctx: + gitea_lock_issue( + issue_number=196, + branch_name="feat/issue-196-mutations", + remote="prgs", + ) self.assertIn("Recovery review is required before takeover", str(ctx.exception)) @patch("mcp_server.api_get_all", return_value=[])