Merge branch 'master' into feat/issue-651-usage-cost-analytics
This commit is contained in:
@@ -0,0 +1,444 @@
|
||||
"""Task heartbeat through the native MCP author path (#790 Slice A, AC-N6).
|
||||
|
||||
Assessor-level coverage is not sufficient here, and this project has already
|
||||
paid for learning that: in review #499 on PR #791 the #760 renewal waiver was
|
||||
computed correctly and then *discarded* at two later gates, so every real
|
||||
renewal still failed while the unit suite stayed green. AC-N6 exists because of
|
||||
that, and requires driving the real tools against a real git repository and a
|
||||
real durable lock file, composing the gates in production order.
|
||||
|
||||
These tests therefore call ``gitea_lock_issue`` and
|
||||
``gitea_heartbeat_issue_lock`` themselves and assert on what lands on disk,
|
||||
never on an assessor's return value alone.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import subprocess
|
||||
import sys
|
||||
import tempfile
|
||||
import unittest
|
||||
from datetime import datetime, timedelta, timezone
|
||||
from unittest.mock import patch
|
||||
|
||||
sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
|
||||
sys.path.insert(0, os.path.dirname(os.path.abspath(__file__)))
|
||||
|
||||
from mutation_profile_fixture import shared_mutation_env # noqa: E402
|
||||
|
||||
import issue_lock_provenance # noqa: E402
|
||||
import issue_lock_store # noqa: E402
|
||||
import lease_policy # noqa: E402
|
||||
import mcp_server # noqa: E402
|
||||
|
||||
ISSUE = 9791
|
||||
BRANCH = f"fix/issue-{ISSUE}-heartbeat-mcp"
|
||||
IDENTITY = "example-user"
|
||||
PROFILE = "test-author-prgs"
|
||||
ORG = "Scaled-Tech-Consulting"
|
||||
REPO = "Gitea-Tools"
|
||||
|
||||
|
||||
def _ts(moment: datetime) -> str:
|
||||
return (
|
||||
moment.astimezone(timezone.utc)
|
||||
.replace(microsecond=0)
|
||||
.isoformat()
|
||||
.replace("+00:00", "Z")
|
||||
)
|
||||
|
||||
|
||||
class _HeartbeatMcpBase(unittest.TestCase):
|
||||
"""Real git repo plus a real durable lock, driven through the real tools."""
|
||||
|
||||
def setUp(self):
|
||||
self.lock_dir = tempfile.TemporaryDirectory()
|
||||
self.addCleanup(self.lock_dir.cleanup)
|
||||
self.repo = tempfile.mkdtemp(prefix="issue790-mcp-")
|
||||
self.addCleanup(lambda: subprocess.run(["rm", "-rf", self.repo], check=False))
|
||||
self._init_worktree()
|
||||
self.remotes = patch.dict(
|
||||
mcp_server.REMOTES,
|
||||
{"prgs": {"host": "gitea.prgs.cc", "org": ORG, "repo": REPO}},
|
||||
)
|
||||
self.remotes.start()
|
||||
self.addCleanup(patch.stopall)
|
||||
mcp_server._IDENTITY_CACHE.clear()
|
||||
|
||||
def _git(self, *args):
|
||||
return subprocess.run(
|
||||
["git", "-C", self.repo, *args], capture_output=True, text=True, check=True
|
||||
)
|
||||
|
||||
def _init_worktree(self):
|
||||
self._git("init", "-q", "-b", "master")
|
||||
self._git("config", "user.email", "[email protected]")
|
||||
self._git("config", "user.name", "Test")
|
||||
with open(os.path.join(self.repo, "seed.txt"), "w") as fh:
|
||||
fh.write("seed\n")
|
||||
self._git("add", "seed.txt")
|
||||
self._git("commit", "-q", "-m", "seed")
|
||||
self.base_sha = self._git("rev-parse", "HEAD").stdout.strip()
|
||||
# A fresh claim starts base-equivalent, which is the ordinary first-lock
|
||||
# shape and exercises assess_issue_lock_worktree on its normal path.
|
||||
self._git("checkout", "-q", "-b", BRANCH)
|
||||
self.head_sha = self.base_sha
|
||||
self.worktree = os.path.realpath(self.repo)
|
||||
|
||||
def _lock_path(self):
|
||||
return issue_lock_store.lock_file_path(
|
||||
remote="prgs",
|
||||
org=ORG,
|
||||
repo=REPO,
|
||||
issue_number=ISSUE,
|
||||
lock_dir=self.lock_dir.name,
|
||||
)
|
||||
|
||||
def _tool_env(self):
|
||||
env = shared_mutation_env(
|
||||
PROFILE, include_example_repo=True, GITEA_ISSUE_LOCK_DIR=self.lock_dir.name
|
||||
)
|
||||
env["GITEA_ISSUE_LOCK_DIR"] = self.lock_dir.name
|
||||
return env
|
||||
|
||||
def _git_state(self, *, porcelain="", base_equivalent=True):
|
||||
return {
|
||||
"current_branch": BRANCH,
|
||||
"porcelain_status": porcelain,
|
||||
"base_equivalent": base_equivalent,
|
||||
"head_sha": self.head_sha,
|
||||
"inspected_git_root": self.worktree,
|
||||
"base_branch": "master",
|
||||
}
|
||||
|
||||
def run_lock_issue(
|
||||
self,
|
||||
*,
|
||||
branch_entries=None,
|
||||
open_prs=None,
|
||||
git_state=None,
|
||||
identity=IDENTITY,
|
||||
profile=PROFILE,
|
||||
):
|
||||
branch_entries = branch_entries if branch_entries is not None else []
|
||||
open_prs = open_prs if open_prs is not None else []
|
||||
git_state = git_state or self._git_state()
|
||||
env = self._tool_env()
|
||||
with patch(
|
||||
"mcp_server.api_get_all", return_value=list(branch_entries)
|
||||
), patch(
|
||||
"mcp_server._list_open_pulls", return_value=list(open_prs)
|
||||
), patch(
|
||||
"mcp_server.get_auth_header", return_value="token x"
|
||||
), patch(
|
||||
"mcp_server._work_lease_claimant",
|
||||
return_value={"username": identity, "profile": profile},
|
||||
), patch(
|
||||
"mcp_server.issue_lock_worktree.read_worktree_git_state",
|
||||
return_value=git_state,
|
||||
), patch(
|
||||
"mcp_server.issue_duplicate_context_fetcher",
|
||||
side_effect=lambda h, o, r, auth, issue_number: (
|
||||
list(open_prs),
|
||||
[b.get("name") for b in branch_entries if isinstance(b, dict)],
|
||||
{"status": "not_claimed"},
|
||||
),
|
||||
), patch.dict(os.environ, env, clear=True):
|
||||
os.environ["GITEA_ISSUE_LOCK_DIR"] = self.lock_dir.name
|
||||
return mcp_server.gitea_lock_issue(
|
||||
issue_number=ISSUE,
|
||||
branch_name=BRANCH,
|
||||
remote="prgs",
|
||||
worktree_path=self.worktree,
|
||||
)
|
||||
|
||||
def run_heartbeat(
|
||||
self, *, task_session_id, identity=IDENTITY, profile=PROFILE, **kwargs
|
||||
):
|
||||
env = self._tool_env()
|
||||
with patch(
|
||||
"mcp_server._work_lease_claimant",
|
||||
return_value={"username": identity, "profile": profile},
|
||||
), patch("mcp_server.get_auth_header", return_value="token x"), patch.dict(
|
||||
os.environ, env, clear=True
|
||||
):
|
||||
os.environ["GITEA_ISSUE_LOCK_DIR"] = self.lock_dir.name
|
||||
return mcp_server.gitea_heartbeat_issue_lock(
|
||||
issue_number=ISSUE,
|
||||
branch_name=kwargs.pop("branch_name", BRANCH),
|
||||
task_session_id=task_session_id,
|
||||
remote="prgs",
|
||||
worktree_path=kwargs.pop("worktree_path", self.worktree),
|
||||
**kwargs,
|
||||
)
|
||||
|
||||
def write_legacy_lock(self, *, hours_old: float = 3.0, ttl_hours: float = 4.0):
|
||||
"""A durable lock in the shape the store wrote before this slice."""
|
||||
now = datetime.now(timezone.utc)
|
||||
claimant = {"username": IDENTITY, "profile": PROFILE}
|
||||
created = now - timedelta(hours=hours_old)
|
||||
record = {
|
||||
"issue_number": ISSUE,
|
||||
"branch_name": BRANCH,
|
||||
"remote": "prgs",
|
||||
"org": ORG,
|
||||
"repo": REPO,
|
||||
"worktree_path": self.worktree,
|
||||
"session_pid": os.getpid(),
|
||||
"pid": os.getpid(),
|
||||
"lock_generation": 1,
|
||||
"work_lease": {
|
||||
"operation_type": issue_lock_store.AUTHOR_ISSUE_WORK_LEASE,
|
||||
"issue_number": ISSUE,
|
||||
"pr_number": None,
|
||||
"branch": BRANCH,
|
||||
"worktree_path": self.worktree,
|
||||
"claimant": claimant,
|
||||
"created_at": _ts(created),
|
||||
# The legacy signature: never advanced past creation.
|
||||
"last_heartbeat_at": _ts(created),
|
||||
"expires_at": _ts(created + timedelta(hours=ttl_hours)),
|
||||
},
|
||||
"lock_provenance": issue_lock_provenance.build_sanctioned_lock_provenance(
|
||||
tool="gitea_lock_issue", claimant=claimant
|
||||
),
|
||||
}
|
||||
path = self._lock_path()
|
||||
record["lock_file_path"] = path
|
||||
issue_lock_store.save_lock_file(path, record)
|
||||
return record
|
||||
|
||||
|
||||
class TestLockIssueMintsTheLifecycle(_HeartbeatMcpBase):
|
||||
"""Durable lock creation and read-back through the real tool."""
|
||||
|
||||
def test_native_lock_writes_the_marker_and_a_task_session_id(self):
|
||||
result = self.run_lock_issue()
|
||||
self.assertTrue(result["success"], result)
|
||||
|
||||
written = issue_lock_store.read_lock_file(result["lock_file_path"])
|
||||
lease = written["work_lease"]
|
||||
self.assertEqual(
|
||||
lease["lifecycle_version"], lease_policy.LIFECYCLE_HEARTBEAT_V1
|
||||
)
|
||||
self.assertTrue(lease["task_session_id"])
|
||||
self.assertFalse(issue_lock_store.is_legacy_lease(written))
|
||||
# AC-N1: the ownership key is not the daemon pid, which is recorded
|
||||
# separately as evidence.
|
||||
self.assertNotIn(str(written["session_pid"]), lease["task_session_id"])
|
||||
self.assertEqual(written["session_pid"], os.getpid())
|
||||
|
||||
def test_native_lease_uses_the_policy_window_not_four_hours(self):
|
||||
result = self.run_lock_issue()
|
||||
lease = result["work_lease"]
|
||||
created = datetime.fromisoformat(lease["created_at"].replace("Z", "+00:00"))
|
||||
expires = datetime.fromisoformat(lease["expires_at"].replace("Z", "+00:00"))
|
||||
policy = lease_policy.policy_for(lease_policy.TASK_CLASS_AUTHOR_ISSUE_WORK)
|
||||
self.assertEqual(
|
||||
(expires - created).total_seconds() / 60.0, policy.initial_ttl_minutes
|
||||
)
|
||||
|
||||
def test_freshness_of_a_new_native_lock_is_live(self):
|
||||
result = self.run_lock_issue()
|
||||
self.assertEqual(
|
||||
result["lock_freshness"]["status"], issue_lock_store.STATUS_LIVE
|
||||
)
|
||||
self.assertTrue(result["lock_freshness"]["live"])
|
||||
|
||||
|
||||
class TestHeartbeatThroughTheTool(_HeartbeatMcpBase):
|
||||
def _lock_and_session(self):
|
||||
result = self.run_lock_issue()
|
||||
self.assertTrue(result["success"], result)
|
||||
return result, result["work_lease"]["task_session_id"]
|
||||
|
||||
def test_heartbeat_slides_the_lease_and_advances_the_generation(self):
|
||||
locked, session = self._lock_and_session()
|
||||
before = issue_lock_store.read_lock_file(locked["lock_file_path"])
|
||||
|
||||
beat = self.run_heartbeat(task_session_id=session)
|
||||
|
||||
self.assertTrue(beat["success"], beat)
|
||||
self.assertEqual(beat["operation"], "heartbeat")
|
||||
after = issue_lock_store.read_lock_file(locked["lock_file_path"])
|
||||
self.assertGreater(
|
||||
issue_lock_store.lock_generation(after),
|
||||
issue_lock_store.lock_generation(before),
|
||||
)
|
||||
self.assertGreaterEqual(
|
||||
after["work_lease"]["expires_at"], before["work_lease"]["expires_at"]
|
||||
)
|
||||
self.assertEqual(after["work_lease"]["heartbeat_count"], 2)
|
||||
|
||||
def test_heartbeat_evidence_survives_the_downstream_mutation_gate(self):
|
||||
"""The #499 F2 lesson, applied.
|
||||
|
||||
A sanction that is computed and then discarded downstream is worthless.
|
||||
After a heartbeat the lock must still satisfy the gate every author
|
||||
mutation runs through.
|
||||
"""
|
||||
locked, session = self._lock_and_session()
|
||||
self.run_heartbeat(task_session_id=session)
|
||||
|
||||
written = issue_lock_store.read_lock_file(locked["lock_file_path"])
|
||||
verdict = issue_lock_store.verify_lock_for_mutation(
|
||||
written,
|
||||
issue_number=ISSUE,
|
||||
branch_name=BRANCH,
|
||||
worktree_path=self.worktree,
|
||||
)
|
||||
self.assertTrue(verdict["proven"], verdict)
|
||||
self.assertFalse(verdict["block"])
|
||||
|
||||
def _duplicate_gate(self, *, open_prs, branches):
|
||||
env = self._tool_env()
|
||||
with patch("mcp_server.get_auth_header", return_value="token x"), patch(
|
||||
"mcp_server.issue_duplicate_context_fetcher",
|
||||
side_effect=lambda h, o, r, auth, issue_number: (
|
||||
list(open_prs),
|
||||
list(branches),
|
||||
{"status": "not_claimed"},
|
||||
),
|
||||
), patch.dict(os.environ, env, clear=True):
|
||||
os.environ["GITEA_ISSUE_LOCK_DIR"] = self.lock_dir.name
|
||||
return mcp_server.gitea_assess_work_issue_duplicate(
|
||||
issue_number=ISSUE, branch_name=BRANCH, remote="prgs"
|
||||
)
|
||||
|
||||
def test_heartbeat_does_not_change_the_duplicate_gate_verdict(self):
|
||||
"""The gate must be invariant under heartbeating.
|
||||
|
||||
The point is not that the gate passes — with a linked open PR at the
|
||||
lock phase it correctly blocks (#400), heartbeat or not. The property
|
||||
that matters is that sliding a lease neither loosens the gate nor
|
||||
corrupts the lock state it reads: the verdict before and after a
|
||||
heartbeat must be identical, for both the clear and the blocking shape.
|
||||
"""
|
||||
_, session = self._lock_and_session()
|
||||
linked = [{"number": 4242, "head": {"ref": BRANCH, "sha": self.head_sha}}]
|
||||
|
||||
clear_before = self._duplicate_gate(open_prs=[], branches=[])
|
||||
blocked_before = self._duplicate_gate(open_prs=linked, branches=[BRANCH])
|
||||
|
||||
self.assertTrue(self.run_heartbeat(task_session_id=session)["success"])
|
||||
|
||||
clear_after = self._duplicate_gate(open_prs=[], branches=[])
|
||||
blocked_after = self._duplicate_gate(open_prs=linked, branches=[BRANCH])
|
||||
|
||||
self.assertEqual(clear_before["outcome"], clear_after["outcome"])
|
||||
self.assertFalse(clear_after["block"])
|
||||
self.assertEqual(blocked_before["outcome"], blocked_after["outcome"])
|
||||
self.assertTrue(blocked_after["block"])
|
||||
self.assertEqual(blocked_after["linked_open_pr"], 4242)
|
||||
|
||||
def test_foreign_session_id_is_refused_through_the_tool(self):
|
||||
self._lock_and_session()
|
||||
beat = self.run_heartbeat(task_session_id="author_issue_work-ffffffffffffffff")
|
||||
self.assertFalse(beat["success"])
|
||||
self.assertIn("task_session_id does not match", " ".join(beat["reasons"]))
|
||||
|
||||
def test_stale_generation_is_refused_through_the_tool(self):
|
||||
locked, session = self._lock_and_session()
|
||||
current = issue_lock_store.lock_generation(
|
||||
issue_lock_store.read_lock_file(locked["lock_file_path"])
|
||||
)
|
||||
beat = self.run_heartbeat(
|
||||
task_session_id=session, expected_generation=current + 5
|
||||
)
|
||||
self.assertFalse(beat["success"])
|
||||
self.assertIn("generation changed", beat["reasons"][0])
|
||||
|
||||
def test_foreign_claimant_is_refused_through_the_tool(self):
|
||||
_, session = self._lock_and_session()
|
||||
beat = self.run_heartbeat(task_session_id=session, identity="someone-else")
|
||||
self.assertFalse(beat["success"])
|
||||
|
||||
def test_heartbeat_cannot_acquire_a_missing_lock(self):
|
||||
beat = self.run_heartbeat(task_session_id="author_issue_work-000000000000")
|
||||
self.assertFalse(beat["success"])
|
||||
self.assertIn("no durable lock", beat["reasons"][0])
|
||||
|
||||
def test_alive_pid_alone_does_not_keep_a_lease_live_through_the_tool(self):
|
||||
"""PID-only refusal, end to end.
|
||||
|
||||
The recorded pid is this live process. The lock is aged past its grace
|
||||
with no heartbeat, so the tool must refuse to slide it and the durable
|
||||
record must classify as a missed heartbeat rather than as live.
|
||||
"""
|
||||
locked, session = self._lock_and_session()
|
||||
record = issue_lock_store.read_lock_file(locked["lock_file_path"])
|
||||
record["work_lease"]["last_heartbeat_at"] = _ts(
|
||||
datetime.now(timezone.utc) - timedelta(minutes=30)
|
||||
)
|
||||
record["work_lease"]["expires_at"] = _ts(
|
||||
datetime.now(timezone.utc) + timedelta(hours=2)
|
||||
)
|
||||
issue_lock_store.save_lock_file(locked["lock_file_path"], record)
|
||||
|
||||
self.assertTrue(issue_lock_store.is_process_alive(record["session_pid"]))
|
||||
fresh = issue_lock_store.assess_lock_freshness(record)
|
||||
self.assertEqual(
|
||||
fresh["status"], issue_lock_store.STATUS_STALE_MISSED_HEARTBEAT
|
||||
)
|
||||
self.assertTrue(fresh["pid_alive"])
|
||||
|
||||
beat = self.run_heartbeat(task_session_id=session)
|
||||
self.assertFalse(beat["success"])
|
||||
self.assertIn("reclaimed", " ".join(beat["reasons"]))
|
||||
|
||||
|
||||
class TestLegacyLocksThroughTheTool(_HeartbeatMcpBase):
|
||||
"""AC-N8 end to end: protected on deployment, and rebindable."""
|
||||
|
||||
def test_legacy_lock_stays_protected_after_deployment(self):
|
||||
record = self.write_legacy_lock(hours_old=3.0, ttl_hours=4.0)
|
||||
fresh = issue_lock_store.assess_lock_freshness(record)
|
||||
self.assertEqual(fresh["status"], issue_lock_store.STATUS_LIVE)
|
||||
self.assertTrue(fresh["legacy_lease"])
|
||||
self.assertTrue(fresh["legacy_expiry_preserved"])
|
||||
# It had never heartbeated, so under the new grace alone it would be
|
||||
# long gone; the preserved absolute expiry is what protects it.
|
||||
self.assertEqual(
|
||||
record["work_lease"]["created_at"],
|
||||
record["work_lease"]["last_heartbeat_at"],
|
||||
)
|
||||
|
||||
def test_tool_rebinds_a_legacy_lock_and_mints_a_first_heartbeat(self):
|
||||
self.write_legacy_lock(hours_old=3.0, ttl_hours=4.0)
|
||||
|
||||
result = self.run_heartbeat(task_session_id=None)
|
||||
|
||||
self.assertTrue(result["success"], result)
|
||||
self.assertEqual(result["operation"], "legacy_rebind")
|
||||
self.assertTrue(result["task_session_id"])
|
||||
|
||||
written = issue_lock_store.read_lock_file(self._lock_path())
|
||||
lease = written["work_lease"]
|
||||
self.assertEqual(
|
||||
lease["lifecycle_version"], lease_policy.LIFECYCLE_HEARTBEAT_V1
|
||||
)
|
||||
self.assertEqual(lease["heartbeat_count"], 1)
|
||||
self.assertNotEqual(
|
||||
lease["created_at"],
|
||||
written["legacy_rebind"]["legacy_origin"]["created_at"],
|
||||
)
|
||||
self.assertFalse(issue_lock_store.is_legacy_lease(written))
|
||||
|
||||
def test_rebound_lock_then_heartbeats_through_the_tool(self):
|
||||
self.write_legacy_lock(hours_old=3.0, ttl_hours=4.0)
|
||||
rebound = self.run_heartbeat(task_session_id=None)
|
||||
beat = self.run_heartbeat(task_session_id=rebound["task_session_id"])
|
||||
self.assertTrue(beat["success"], beat)
|
||||
self.assertEqual(beat["operation"], "heartbeat")
|
||||
self.assertEqual(beat["heartbeat_count"], 2)
|
||||
|
||||
def test_rebind_refuses_a_foreign_owner_through_the_tool(self):
|
||||
self.write_legacy_lock(hours_old=3.0, ttl_hours=4.0)
|
||||
result = self.run_heartbeat(task_session_id=None, identity="someone-else")
|
||||
self.assertFalse(result["success"])
|
||||
self.assertEqual(result["operation"], "legacy_rebind")
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
@@ -0,0 +1,594 @@
|
||||
"""Central lease policy and load-bearing heartbeat freshness (#790 Slice A).
|
||||
|
||||
Before this slice, ``issue_lock_store.assess_lock_freshness`` parsed
|
||||
``last_heartbeat_at`` and then never consulted it: liveness was decided by an
|
||||
absolute four-hour ``expires_at`` and by PID liveness. Because the recorded PID
|
||||
is the long-lived MCP daemon rather than the authoring task, an abandoned claim
|
||||
stayed "live" for the full four hours, and a claim whose work had already landed
|
||||
blocked reconciliation for just as long (Issue #787 / PR #789, and again Issue
|
||||
#760 / PR #791).
|
||||
|
||||
These tests pin the corrected semantics, including the two asymmetries that are
|
||||
easy to lose in a refactor:
|
||||
|
||||
* an **alive** PID must never make anything live (AC-N2), while
|
||||
* a **dead** PID must still mark a lease stale, because #753 dead-session
|
||||
recovery keys on exactly that classification.
|
||||
|
||||
Durable-state helpers here write real lock files through the real flock path;
|
||||
they are not mocks of the store.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import sys
|
||||
import tempfile
|
||||
import unittest
|
||||
from datetime import datetime, timedelta, timezone
|
||||
from unittest.mock import patch
|
||||
|
||||
sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
|
||||
|
||||
import issue_lock_store # noqa: E402
|
||||
import lease_policy # noqa: E402
|
||||
import pr_work_lease # noqa: E402
|
||||
import reviewer_pr_lease # noqa: E402
|
||||
|
||||
ISSUE = 9790
|
||||
BRANCH = f"fix/issue-{ISSUE}-heartbeat"
|
||||
IDENTITY = "example-user"
|
||||
PROFILE = "test-author-prgs"
|
||||
ORG = "Example-Org"
|
||||
REPO = "Example-Repo"
|
||||
REMOTE = "prgs"
|
||||
DEAD_PID = 2**22 # far above any live pid on a test host
|
||||
|
||||
|
||||
def _ts(moment: datetime) -> str:
|
||||
return (
|
||||
moment.astimezone(timezone.utc)
|
||||
.replace(microsecond=0)
|
||||
.isoformat()
|
||||
.replace("+00:00", "Z")
|
||||
)
|
||||
|
||||
|
||||
class _LockFixture(unittest.TestCase):
|
||||
def setUp(self):
|
||||
self.lock_dir = tempfile.TemporaryDirectory()
|
||||
self.addCleanup(self.lock_dir.cleanup)
|
||||
self.now = datetime.now(timezone.utc)
|
||||
self.worktree = os.path.realpath(tempfile.mkdtemp(prefix="issue790-"))
|
||||
self.addCleanup(patch.stopall)
|
||||
|
||||
def _path(self):
|
||||
return issue_lock_store.lock_file_path(
|
||||
remote=REMOTE,
|
||||
org=ORG,
|
||||
repo=REPO,
|
||||
issue_number=ISSUE,
|
||||
lock_dir=self.lock_dir.name,
|
||||
)
|
||||
|
||||
def write_lock(
|
||||
self,
|
||||
*,
|
||||
lifecycle: str | None = lease_policy.LIFECYCLE_HEARTBEAT_V1,
|
||||
created_delta: timedelta = timedelta(minutes=1),
|
||||
heartbeat_delta: timedelta = timedelta(minutes=1),
|
||||
expires_delta: timedelta = timedelta(minutes=9),
|
||||
pid: int | None = None,
|
||||
task_session_id: str | None = "author_issue_work-aaaabbbbccccdddd",
|
||||
generation: int = 1,
|
||||
identity: str = IDENTITY,
|
||||
profile: str = PROFILE,
|
||||
branch: str = BRANCH,
|
||||
worktree: str | None = None,
|
||||
) -> dict:
|
||||
"""Write a real durable lock and return the record.
|
||||
|
||||
Deltas are relative to ``self.now``; ``expires_delta`` is added, the
|
||||
others subtracted, so "in the past" reads naturally at each call site.
|
||||
"""
|
||||
lease: dict = {
|
||||
"operation_type": issue_lock_store.AUTHOR_ISSUE_WORK_LEASE,
|
||||
"issue_number": ISSUE,
|
||||
"pr_number": None,
|
||||
"branch": branch,
|
||||
"worktree_path": worktree or self.worktree,
|
||||
"claimant": {"username": identity, "profile": profile},
|
||||
"created_at": _ts(self.now - created_delta),
|
||||
"last_heartbeat_at": _ts(self.now - heartbeat_delta),
|
||||
"expires_at": _ts(self.now + expires_delta),
|
||||
}
|
||||
if lifecycle is not None:
|
||||
lease["lifecycle_version"] = lifecycle
|
||||
if task_session_id is not None:
|
||||
lease["task_session_id"] = task_session_id
|
||||
pid_value = os.getpid() if pid is None else pid
|
||||
record = {
|
||||
"issue_number": ISSUE,
|
||||
"branch_name": branch,
|
||||
"remote": REMOTE,
|
||||
"org": ORG,
|
||||
"repo": REPO,
|
||||
"worktree_path": worktree or self.worktree,
|
||||
"session_pid": pid_value,
|
||||
"pid": pid_value,
|
||||
"lock_generation": generation,
|
||||
"work_lease": lease,
|
||||
}
|
||||
path = self._path()
|
||||
record["lock_file_path"] = path
|
||||
issue_lock_store.save_lock_file(path, record)
|
||||
return record
|
||||
|
||||
|
||||
class TestPolicyIsTheSingleSource(unittest.TestCase):
|
||||
"""AC-N7: one authoritative configuration source for every duration."""
|
||||
|
||||
def test_author_policy_carries_the_agreed_values(self):
|
||||
policy = lease_policy.policy_for(lease_policy.TASK_CLASS_AUTHOR_ISSUE_WORK)
|
||||
self.assertEqual(policy.initial_ttl_minutes, 10.0)
|
||||
self.assertEqual(policy.heartbeat_cadence_minutes, 2.0)
|
||||
self.assertEqual(policy.stale_warning_minutes, 5.0)
|
||||
self.assertEqual(policy.missed_heartbeat_grace_minutes, 10.0)
|
||||
self.assertEqual(policy.absolute_cap_hours, 8.0)
|
||||
self.assertEqual(policy.recovery_grace_minutes, 10.0)
|
||||
self.assertEqual(policy.terminal_race_drain_minutes, 2.0)
|
||||
self.assertTrue(policy.terminal_retirement_eligible)
|
||||
self.assertTrue(policy.heartbeat_lifecycle_active)
|
||||
|
||||
def test_the_four_hour_author_ttl_literal_is_gone(self):
|
||||
"""The duplicated literal AC-N7 exists to remove."""
|
||||
self.assertFalse(hasattr(issue_lock_store, "WORK_LEASE_TTL_HOURS"))
|
||||
import gitea_mcp_server
|
||||
|
||||
self.assertFalse(hasattr(gitea_mcp_server, "WORK_LEASE_TTL_HOURS"))
|
||||
|
||||
def test_declared_reviewer_values_match_the_module_still_using_them(self):
|
||||
"""Slice A declares reviewer/merger numbers without rewiring them.
|
||||
|
||||
Recording a value in two places is only safe if drift is detectable, so
|
||||
this asserts the declaration still equals the constants #747 owns. When
|
||||
Slice C migrates those call sites, this test becomes the proof the
|
||||
migration changed nothing.
|
||||
"""
|
||||
policy = lease_policy.policy_for(lease_policy.TASK_CLASS_REVIEWER_PR)
|
||||
self.assertEqual(
|
||||
policy.initial_ttl_minutes, float(reviewer_pr_lease.LEASE_TTL_MINUTES)
|
||||
)
|
||||
self.assertEqual(
|
||||
policy.stale_warning_minutes,
|
||||
float(reviewer_pr_lease.STALE_WARNING_MINUTES),
|
||||
)
|
||||
self.assertFalse(policy.heartbeat_lifecycle_active)
|
||||
|
||||
def test_declared_conflict_fix_value_matches_its_module(self):
|
||||
policy = lease_policy.policy_for(lease_policy.TASK_CLASS_CONFLICT_FIX)
|
||||
self.assertEqual(
|
||||
policy.initial_ttl_minutes,
|
||||
float(pr_work_lease.DEFAULT_CONFLICT_FIX_TTL_MINUTES),
|
||||
)
|
||||
self.assertFalse(policy.heartbeat_lifecycle_active)
|
||||
|
||||
def test_environment_override_applies(self):
|
||||
var = lease_policy.env_var_name(
|
||||
lease_policy.TASK_CLASS_AUTHOR_ISSUE_WORK, "initial_ttl_minutes"
|
||||
)
|
||||
with patch.dict(os.environ, {var: "7"}):
|
||||
self.assertEqual(
|
||||
lease_policy.policy_for(
|
||||
lease_policy.TASK_CLASS_AUTHOR_ISSUE_WORK
|
||||
).initial_ttl_minutes,
|
||||
7.0,
|
||||
)
|
||||
|
||||
def test_unusable_override_falls_back_instead_of_minting_a_zero_lease(self):
|
||||
"""A typo must not make every claim instantly reclaimable."""
|
||||
var = lease_policy.env_var_name(
|
||||
lease_policy.TASK_CLASS_AUTHOR_ISSUE_WORK, "initial_ttl_minutes"
|
||||
)
|
||||
for bad in ("0", "-5", "not-a-number", " "):
|
||||
with self.subTest(value=bad), patch.dict(os.environ, {var: bad}):
|
||||
self.assertEqual(
|
||||
lease_policy.policy_for(
|
||||
lease_policy.TASK_CLASS_AUTHOR_ISSUE_WORK
|
||||
).initial_ttl_minutes,
|
||||
10.0,
|
||||
)
|
||||
|
||||
def test_unknown_task_class_does_not_raise(self):
|
||||
policy = lease_policy.policy_for("something-new")
|
||||
self.assertEqual(policy.task_class, lease_policy.TASK_CLASS_AUTHOR_ISSUE_WORK)
|
||||
|
||||
|
||||
class TestLifecycleDiscrimination(_LockFixture):
|
||||
"""AC-N8: the marker, never a timestamp, decides legacy vs heartbeat."""
|
||||
|
||||
def test_missing_marker_reads_as_legacy(self):
|
||||
record = self.write_lock(lifecycle=None)
|
||||
self.assertTrue(issue_lock_store.is_legacy_lease(record))
|
||||
self.assertEqual(
|
||||
issue_lock_store.lease_lifecycle_version(record),
|
||||
lease_policy.LIFECYCLE_LEGACY,
|
||||
)
|
||||
|
||||
def test_marker_present_reads_as_heartbeat_lifecycle(self):
|
||||
record = self.write_lock()
|
||||
self.assertFalse(issue_lock_store.is_legacy_lease(record))
|
||||
|
||||
def test_equal_created_and_heartbeat_never_implies_a_fresh_heartbeat(self):
|
||||
"""The exact inversion AC-N8 forbids.
|
||||
|
||||
A legacy lock has ``last_heartbeat_at == created_at`` forever because
|
||||
nothing ever advanced it. Reading that equality as "recently
|
||||
heartbeated" would classify every never-heartbeated lock as fresh.
|
||||
"""
|
||||
legacy = self.write_lock(
|
||||
lifecycle=None,
|
||||
created_delta=timedelta(hours=3),
|
||||
heartbeat_delta=timedelta(hours=3),
|
||||
)
|
||||
lease = legacy["work_lease"]
|
||||
self.assertEqual(lease["created_at"], lease["last_heartbeat_at"])
|
||||
self.assertTrue(issue_lock_store.is_legacy_lease(legacy))
|
||||
|
||||
# A brand-new heartbeat lease has them equal too, so the equality
|
||||
# carries no information in either direction.
|
||||
fresh = self.write_lock(
|
||||
created_delta=timedelta(seconds=0), heartbeat_delta=timedelta(seconds=0)
|
||||
)
|
||||
self.assertEqual(
|
||||
fresh["work_lease"]["created_at"],
|
||||
fresh["work_lease"]["last_heartbeat_at"],
|
||||
)
|
||||
self.assertFalse(issue_lock_store.is_legacy_lease(fresh))
|
||||
|
||||
def test_minted_session_id_contains_no_pid(self):
|
||||
"""AC-N1: the ownership key must not be derived from the daemon pid."""
|
||||
minted = issue_lock_store.mint_task_session_id()
|
||||
self.assertNotIn(str(os.getpid()), minted)
|
||||
self.assertNotEqual(minted, issue_lock_store.mint_task_session_id())
|
||||
|
||||
|
||||
class TestFreshnessIsHeartbeatDriven(_LockFixture):
|
||||
"""AC-N2 and the new bands."""
|
||||
|
||||
def test_fresh_heartbeat_is_live(self):
|
||||
record = self.write_lock(heartbeat_delta=timedelta(minutes=1))
|
||||
fresh = issue_lock_store.assess_lock_freshness(record, now=self.now)
|
||||
self.assertEqual(fresh["status"], issue_lock_store.STATUS_LIVE)
|
||||
self.assertTrue(fresh["live"])
|
||||
self.assertFalse(fresh["heartbeat_warning"])
|
||||
|
||||
def test_heartbeat_past_warning_is_still_live_but_flagged(self):
|
||||
record = self.write_lock(heartbeat_delta=timedelta(minutes=6))
|
||||
fresh = issue_lock_store.assess_lock_freshness(record, now=self.now)
|
||||
self.assertEqual(fresh["status"], issue_lock_store.STATUS_LIVE)
|
||||
self.assertTrue(fresh["heartbeat_warning"])
|
||||
|
||||
def test_missed_heartbeat_past_grace_is_classified_explicitly(self):
|
||||
record = self.write_lock(
|
||||
heartbeat_delta=timedelta(minutes=11),
|
||||
expires_delta=timedelta(minutes=30),
|
||||
)
|
||||
fresh = issue_lock_store.assess_lock_freshness(record, now=self.now)
|
||||
self.assertEqual(
|
||||
fresh["status"], issue_lock_store.STATUS_STALE_MISSED_HEARTBEAT
|
||||
)
|
||||
self.assertFalse(fresh["live"])
|
||||
self.assertTrue(fresh["stale"])
|
||||
|
||||
def test_alive_pid_never_establishes_freshness(self):
|
||||
"""The defect in one assertion.
|
||||
|
||||
The recorded PID is this very process, so it is unambiguously alive —
|
||||
and the lease is still not live, because the task stopped heartbeating.
|
||||
"""
|
||||
record = self.write_lock(
|
||||
pid=os.getpid(),
|
||||
heartbeat_delta=timedelta(hours=4),
|
||||
expires_delta=timedelta(hours=4),
|
||||
)
|
||||
fresh = issue_lock_store.assess_lock_freshness(record, now=self.now)
|
||||
self.assertTrue(fresh["pid_alive"])
|
||||
self.assertFalse(fresh["live"])
|
||||
self.assertEqual(
|
||||
fresh["status"], issue_lock_store.STATUS_STALE_MISSED_HEARTBEAT
|
||||
)
|
||||
|
||||
def test_dead_pid_still_marks_stale_for_issue_753(self):
|
||||
"""The opposite asymmetry: dead-PID corroboration is preserved."""
|
||||
record = self.write_lock(pid=DEAD_PID, heartbeat_delta=timedelta(minutes=1))
|
||||
fresh = issue_lock_store.assess_lock_freshness(record, now=self.now)
|
||||
self.assertEqual(fresh["status"], issue_lock_store.STATUS_STALE)
|
||||
self.assertFalse(fresh["live"])
|
||||
self.assertIn("not alive", fresh["reason"])
|
||||
|
||||
def test_absolute_cap_requires_readoption(self):
|
||||
record = self.write_lock(
|
||||
created_delta=timedelta(hours=9), heartbeat_delta=timedelta(minutes=1)
|
||||
)
|
||||
fresh = issue_lock_store.assess_lock_freshness(record, now=self.now)
|
||||
self.assertEqual(fresh["status"], issue_lock_store.STATUS_STALE_ABSOLUTE_CAP)
|
||||
self.assertIn("re-adoption", fresh["reason"])
|
||||
|
||||
def test_heartbeat_lifecycle_without_a_heartbeat_fails_closed(self):
|
||||
record = self.write_lock()
|
||||
del record["work_lease"]["last_heartbeat_at"]
|
||||
issue_lock_store.save_lock_file(self._path(), record)
|
||||
fresh = issue_lock_store.assess_lock_freshness(record, now=self.now)
|
||||
self.assertEqual(
|
||||
fresh["status"], issue_lock_store.STATUS_STALE_MISSED_HEARTBEAT
|
||||
)
|
||||
self.assertIn("fail closed", fresh["reason"])
|
||||
|
||||
def test_absent_lock(self):
|
||||
fresh = issue_lock_store.assess_lock_freshness(None)
|
||||
self.assertEqual(fresh["status"], issue_lock_store.STATUS_ABSENT)
|
||||
self.assertFalse(fresh["stale"])
|
||||
|
||||
|
||||
class TestLegacyLocksStayProtected(_LockFixture):
|
||||
"""AC-N8: deployment must not retroactively shorten an existing claim."""
|
||||
|
||||
def test_legacy_lock_with_a_stale_heartbeat_remains_live(self):
|
||||
"""The deployment-safety case.
|
||||
|
||||
A four-hour legacy lease minted three hours ago has not heartbeated
|
||||
once. Under the new grace it would be long gone; under its preserved
|
||||
absolute expiry it is still live, and must stay that way.
|
||||
"""
|
||||
record = self.write_lock(
|
||||
lifecycle=None,
|
||||
created_delta=timedelta(hours=3),
|
||||
heartbeat_delta=timedelta(hours=3),
|
||||
expires_delta=timedelta(hours=1),
|
||||
)
|
||||
fresh = issue_lock_store.assess_lock_freshness(record, now=self.now)
|
||||
self.assertEqual(fresh["status"], issue_lock_store.STATUS_LIVE)
|
||||
self.assertTrue(fresh["live"])
|
||||
self.assertTrue(fresh["legacy_lease"])
|
||||
self.assertTrue(fresh["legacy_expiry_preserved"])
|
||||
|
||||
def test_legacy_lock_past_its_absolute_expiry_is_expired_as_before(self):
|
||||
record = self.write_lock(
|
||||
lifecycle=None,
|
||||
created_delta=timedelta(hours=5),
|
||||
heartbeat_delta=timedelta(hours=5),
|
||||
expires_delta=timedelta(hours=-1),
|
||||
)
|
||||
fresh = issue_lock_store.assess_lock_freshness(record, now=self.now)
|
||||
self.assertEqual(fresh["status"], issue_lock_store.STATUS_EXPIRED)
|
||||
|
||||
def test_legacy_lock_is_never_reclaimed_by_the_heartbeat_band(self):
|
||||
record = self.write_lock(
|
||||
lifecycle=None,
|
||||
created_delta=timedelta(hours=3),
|
||||
heartbeat_delta=timedelta(hours=3),
|
||||
expires_delta=timedelta(hours=1),
|
||||
)
|
||||
reclaim = issue_lock_store.assess_expired_lock_reclaim(record, now=self.now)
|
||||
self.assertFalse(reclaim["reclaim_allowed"])
|
||||
|
||||
|
||||
class TestReclaimAfterMissedHeartbeat(_LockFixture):
|
||||
def test_missed_heartbeat_makes_ownership_reclaimable(self):
|
||||
record = self.write_lock(
|
||||
pid=os.getpid(),
|
||||
heartbeat_delta=timedelta(minutes=15),
|
||||
expires_delta=timedelta(hours=3),
|
||||
)
|
||||
reclaim = issue_lock_store.assess_expired_lock_reclaim(record, now=self.now)
|
||||
self.assertTrue(reclaim["reclaim_allowed"])
|
||||
self.assertIn("stale_missed_heartbeat", reclaim["reasons"][0])
|
||||
|
||||
def test_live_lease_is_never_reclaimable(self):
|
||||
record = self.write_lock(heartbeat_delta=timedelta(minutes=1))
|
||||
reclaim = issue_lock_store.assess_expired_lock_reclaim(record, now=self.now)
|
||||
self.assertFalse(reclaim["reclaim_allowed"])
|
||||
|
||||
def test_dead_pid_reclaim_path_is_unchanged(self):
|
||||
"""#753 must keep working through its original conditions."""
|
||||
record = self.write_lock(pid=DEAD_PID, heartbeat_delta=timedelta(minutes=1))
|
||||
reclaim = issue_lock_store.assess_expired_lock_reclaim(record, now=self.now)
|
||||
self.assertTrue(reclaim["reclaim_allowed"])
|
||||
self.assertTrue(reclaim["owner_pid_dead"])
|
||||
|
||||
|
||||
class TestHeartbeatWriter(_LockFixture):
|
||||
"""A4: flock + CAS + exact verification, and no revival path."""
|
||||
|
||||
def _heartbeat(self, **kwargs):
|
||||
params = {
|
||||
"remote": REMOTE,
|
||||
"org": ORG,
|
||||
"repo": REPO,
|
||||
"issue_number": ISSUE,
|
||||
"branch_name": BRANCH,
|
||||
"worktree_path": self.worktree,
|
||||
"identity": IDENTITY,
|
||||
"profile": PROFILE,
|
||||
"task_session_id": "author_issue_work-aaaabbbbccccdddd",
|
||||
"lock_dir": self.lock_dir.name,
|
||||
"now": self.now,
|
||||
}
|
||||
params.update(kwargs)
|
||||
return issue_lock_store.heartbeat_session_lock(**params)
|
||||
|
||||
def test_heartbeat_slides_expiry_and_advances_generation(self):
|
||||
self.write_lock(heartbeat_delta=timedelta(minutes=4), generation=5)
|
||||
result = self._heartbeat()
|
||||
self.assertTrue(result["success"], result)
|
||||
self.assertEqual(result["prior_generation"], 5)
|
||||
self.assertEqual(result["lock_generation"], 6)
|
||||
self.assertEqual(result["heartbeat_count"], 1)
|
||||
self.assertEqual(result["last_heartbeat_at"], _ts(self.now))
|
||||
self.assertEqual(result["expires_at"], _ts(self.now + timedelta(minutes=10)))
|
||||
self.assertTrue(result["freshness"]["live"])
|
||||
|
||||
def test_heartbeat_is_durable_and_repeatable(self):
|
||||
self.write_lock(heartbeat_delta=timedelta(minutes=4))
|
||||
self._heartbeat()
|
||||
second = self._heartbeat(now=self.now + timedelta(minutes=1))
|
||||
self.assertTrue(second["success"], second)
|
||||
self.assertEqual(second["heartbeat_count"], 2)
|
||||
written = issue_lock_store.read_lock_file(self._path())
|
||||
self.assertEqual(written["work_lease"]["heartbeat_count"], 2)
|
||||
|
||||
def test_stale_generation_is_refused(self):
|
||||
self.write_lock(generation=5)
|
||||
result = self._heartbeat(expected_generation=4)
|
||||
self.assertFalse(result["success"])
|
||||
self.assertIn("generation changed", result["reasons"][0])
|
||||
|
||||
def test_foreign_session_is_refused(self):
|
||||
self.write_lock()
|
||||
result = self._heartbeat(task_session_id="author_issue_work-ffffffffffffffff")
|
||||
self.assertFalse(result["success"])
|
||||
self.assertIn("task_session_id does not match", " ".join(result["reasons"]))
|
||||
|
||||
def test_missing_session_id_is_refused(self):
|
||||
self.write_lock()
|
||||
result = self._heartbeat(task_session_id="")
|
||||
self.assertFalse(result["success"])
|
||||
|
||||
def test_foreign_claimant_is_refused(self):
|
||||
self.write_lock()
|
||||
for field, value in (
|
||||
("identity", "someone-else"),
|
||||
("profile", "other-profile"),
|
||||
):
|
||||
with self.subTest(field=field):
|
||||
result = self._heartbeat(**{field: value})
|
||||
self.assertFalse(result["success"])
|
||||
|
||||
def test_branch_and_worktree_mismatch_are_refused(self):
|
||||
self.write_lock()
|
||||
wrong_branch = self._heartbeat(branch_name=f"fix/issue-{ISSUE}-other")
|
||||
self.assertFalse(wrong_branch["success"])
|
||||
wrong_worktree = self._heartbeat(worktree_path="/tmp/not-the-worktree")
|
||||
self.assertFalse(wrong_worktree["success"])
|
||||
|
||||
def test_lapsed_lease_cannot_be_heartbeated_back_to_life(self):
|
||||
"""No revival path (A4).
|
||||
|
||||
A session that stopped proving liveness must reclaim under a fresh
|
||||
generation, not restore ownership retroactively.
|
||||
"""
|
||||
self.write_lock(
|
||||
heartbeat_delta=timedelta(minutes=30), expires_delta=timedelta(hours=1)
|
||||
)
|
||||
result = self._heartbeat()
|
||||
self.assertFalse(result["success"])
|
||||
self.assertIn("reclaimed", " ".join(result["reasons"]))
|
||||
|
||||
def test_absent_lock_cannot_be_created_by_heartbeat(self):
|
||||
result = self._heartbeat()
|
||||
self.assertFalse(result["success"])
|
||||
self.assertIn("no durable lock", result["reasons"][0])
|
||||
|
||||
def test_legacy_lock_is_refused_until_rebound(self):
|
||||
self.write_lock(lifecycle=None)
|
||||
result = self._heartbeat()
|
||||
self.assertFalse(result["success"])
|
||||
self.assertTrue(result["legacy_lease"])
|
||||
self.assertIn("rebound", " ".join(result["reasons"]))
|
||||
|
||||
|
||||
class TestLegacyRebind(_LockFixture):
|
||||
"""AC-N8 exit route: canonical exact-owner rebinding."""
|
||||
|
||||
def _rebind(self, **kwargs):
|
||||
params = {
|
||||
"remote": REMOTE,
|
||||
"org": ORG,
|
||||
"repo": REPO,
|
||||
"issue_number": ISSUE,
|
||||
"branch_name": BRANCH,
|
||||
"worktree_path": self.worktree,
|
||||
"identity": IDENTITY,
|
||||
"profile": PROFILE,
|
||||
"lock_dir": self.lock_dir.name,
|
||||
"now": self.now,
|
||||
}
|
||||
params.update(kwargs)
|
||||
return issue_lock_store.rebind_legacy_lock(**params)
|
||||
|
||||
def test_rebind_mints_a_session_and_a_genuine_first_heartbeat(self):
|
||||
self.write_lock(
|
||||
lifecycle=None,
|
||||
created_delta=timedelta(hours=3),
|
||||
heartbeat_delta=timedelta(hours=3),
|
||||
expires_delta=timedelta(hours=1),
|
||||
generation=2,
|
||||
)
|
||||
result = self._rebind()
|
||||
self.assertTrue(result["success"], result)
|
||||
self.assertTrue(result["task_session_id"])
|
||||
self.assertEqual(result["lock_generation"], 3)
|
||||
|
||||
written = issue_lock_store.read_lock_file(self._path())
|
||||
lease = written["work_lease"]
|
||||
self.assertEqual(
|
||||
lease["lifecycle_version"], lease_policy.LIFECYCLE_HEARTBEAT_V1
|
||||
)
|
||||
self.assertEqual(lease["last_heartbeat_at"], _ts(self.now))
|
||||
self.assertEqual(lease["expires_at"], _ts(self.now + timedelta(minutes=10)))
|
||||
self.assertFalse(issue_lock_store.is_legacy_lease(written))
|
||||
# The original claim is preserved for audit rather than overwritten.
|
||||
origin = written["legacy_rebind"]["legacy_origin"]
|
||||
self.assertTrue(origin["created_at"])
|
||||
self.assertEqual(origin["lifecycle"], lease_policy.LIFECYCLE_LEGACY)
|
||||
|
||||
def test_rebound_lock_can_then_heartbeat(self):
|
||||
self.write_lock(
|
||||
lifecycle=None,
|
||||
created_delta=timedelta(hours=3),
|
||||
heartbeat_delta=timedelta(hours=3),
|
||||
expires_delta=timedelta(hours=1),
|
||||
)
|
||||
rebound = self._rebind()
|
||||
beat = issue_lock_store.heartbeat_session_lock(
|
||||
remote=REMOTE,
|
||||
org=ORG,
|
||||
repo=REPO,
|
||||
issue_number=ISSUE,
|
||||
branch_name=BRANCH,
|
||||
worktree_path=self.worktree,
|
||||
identity=IDENTITY,
|
||||
profile=PROFILE,
|
||||
task_session_id=rebound["task_session_id"],
|
||||
lock_dir=self.lock_dir.name,
|
||||
now=self.now + timedelta(minutes=1),
|
||||
)
|
||||
self.assertTrue(beat["success"], beat)
|
||||
|
||||
def test_rebind_refuses_a_foreign_owner(self):
|
||||
self.write_lock(lifecycle=None, expires_delta=timedelta(hours=1))
|
||||
result = self._rebind(identity="someone-else")
|
||||
self.assertFalse(result["success"])
|
||||
|
||||
def test_rebind_refuses_a_lock_already_on_the_lifecycle(self):
|
||||
self.write_lock()
|
||||
result = self._rebind()
|
||||
self.assertFalse(result["success"])
|
||||
self.assertFalse(result["legacy_lease"])
|
||||
|
||||
def test_rebind_is_not_a_recovery_path_for_a_lapsed_legacy_lease(self):
|
||||
"""An expired legacy lease belongs to #760 renewal or #601 reclaim."""
|
||||
self.write_lock(
|
||||
lifecycle=None,
|
||||
created_delta=timedelta(hours=5),
|
||||
heartbeat_delta=timedelta(hours=5),
|
||||
expires_delta=timedelta(hours=-1),
|
||||
)
|
||||
result = self._rebind()
|
||||
self.assertFalse(result["success"])
|
||||
self.assertIn("not a recovery path", " ".join(result["reasons"]))
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
@@ -0,0 +1,193 @@
|
||||
"""Conflict gate honors heartbeat-lifecycle non-live reclaim bands (#790 review #502/#516).
|
||||
|
||||
``assess_expired_lock_reclaim`` already permits reclaim for the heartbeat-lifecycle
|
||||
stale bands (``stale_missed_heartbeat``, ``stale_absolute_cap``) without a dead PID.
|
||||
But ``assess_same_issue_lease_conflict`` used to enter that reclaim branch only under
|
||||
``is_lease_expired`` (``expires_at <= now``). For a heartbeat-lifecycle lease that is
|
||||
non-live yet whose ``expires_at`` is still in the future, the acquisition gate fell
|
||||
through to the foreign "already has an active lease" block and never consulted the
|
||||
reclaim assessor — so the load-bearing heartbeat was not load-bearing for foreign
|
||||
reclaim, the exact abandonment scenario #790 exists to fix.
|
||||
|
||||
Two future-``expires_at`` non-live shapes are reachable:
|
||||
|
||||
* ``stale_absolute_cap`` — a session that keeps heartbeating past the 8h absolute cap
|
||||
has ``expires_at = last_heartbeat + TTL`` in the future (default policy).
|
||||
* ``stale_missed_heartbeat`` — under an independent TTL>grace policy the heartbeat
|
||||
grace lapses while ``expires_at`` is still ahead.
|
||||
|
||||
These tests pin: both reclaim from a foreign acquirer; a live lease still blocks a
|
||||
foreign acquirer; the same owner may reclaim its own abandoned heartbeat lease; and a
|
||||
legacy (pre-lifecycle) lock keeps its absolute-``expires_at`` clock — a non-expired
|
||||
legacy lock with a dead PID is *not* reclaimable through this path.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import sys
|
||||
import unittest
|
||||
from datetime import datetime, timedelta, timezone
|
||||
from unittest import mock
|
||||
|
||||
sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
|
||||
|
||||
import issue_lock_store as ils # noqa: E402
|
||||
import lease_policy # noqa: E402
|
||||
|
||||
ISSUE = 790
|
||||
OWNER_BRANCH = f"fix/issue-{ISSUE}-slice-a-heartbeat-policy"
|
||||
OWNER_WORKTREE = "/tmp/wt-790-owner"
|
||||
FOREIGN_BRANCH = f"fix/issue-{ISSUE}-foreign-attempt"
|
||||
FOREIGN_WORKTREE = "/tmp/wt-790-foreign"
|
||||
|
||||
|
||||
def _ts(moment: datetime) -> str:
|
||||
return (
|
||||
moment.astimezone(timezone.utc)
|
||||
.replace(microsecond=0)
|
||||
.isoformat()
|
||||
.replace("+00:00", "Z")
|
||||
)
|
||||
|
||||
|
||||
class _ConflictGateBase(unittest.TestCase):
|
||||
def setUp(self):
|
||||
self.now = datetime(2026, 7, 23, 12, 0, 0, tzinfo=timezone.utc)
|
||||
|
||||
def _lock(
|
||||
self,
|
||||
*,
|
||||
lifecycle: str | None,
|
||||
created_ago: timedelta,
|
||||
heartbeat_ago: timedelta,
|
||||
expires_in: timedelta,
|
||||
pid: int,
|
||||
) -> dict:
|
||||
lease: dict = {
|
||||
"operation_type": ils.AUTHOR_ISSUE_WORK_LEASE,
|
||||
"issue_number": ISSUE,
|
||||
"branch": OWNER_BRANCH,
|
||||
"worktree_path": OWNER_WORKTREE,
|
||||
"created_at": _ts(self.now - created_ago),
|
||||
"last_heartbeat_at": _ts(self.now - heartbeat_ago),
|
||||
"expires_at": _ts(self.now + expires_in),
|
||||
}
|
||||
if lifecycle is not None:
|
||||
lease["lifecycle_version"] = lifecycle
|
||||
lease["task_session_id"] = "author_issue_work-deadbeefdeadbeef"
|
||||
return {
|
||||
"issue_number": ISSUE,
|
||||
"branch_name": OWNER_BRANCH,
|
||||
"remote": "prgs",
|
||||
"org": "Scaled-Tech-Consulting",
|
||||
"repo": "Gitea-Tools",
|
||||
"worktree_path": OWNER_WORKTREE,
|
||||
"session_pid": pid,
|
||||
"pid": pid,
|
||||
"work_lease": lease,
|
||||
}
|
||||
|
||||
def _foreign_conflict(self, existing: dict) -> str | None:
|
||||
return ils.assess_same_issue_lease_conflict(
|
||||
existing,
|
||||
issue_number=ISSUE,
|
||||
branch_name=FOREIGN_BRANCH,
|
||||
worktree_path=FOREIGN_WORKTREE,
|
||||
now=self.now,
|
||||
)
|
||||
|
||||
def _same_owner_conflict(self, existing: dict) -> str | None:
|
||||
return ils.assess_same_issue_lease_conflict(
|
||||
existing,
|
||||
issue_number=ISSUE,
|
||||
branch_name=OWNER_BRANCH,
|
||||
worktree_path=OWNER_WORKTREE,
|
||||
now=self.now,
|
||||
)
|
||||
|
||||
|
||||
class TestHeartbeatNonLiveFutureExpiresReclaim(_ConflictGateBase):
|
||||
def test_stale_absolute_cap_future_expires_allows_foreign_reclaim(self):
|
||||
# Created >8h ago, heartbeated one minute ago, expires 9 min in the FUTURE,
|
||||
# owner PID alive: freshness = stale_absolute_cap, live=False, not expired.
|
||||
existing = self._lock(
|
||||
lifecycle=lease_policy.LIFECYCLE_HEARTBEAT_V1,
|
||||
created_ago=timedelta(hours=9),
|
||||
heartbeat_ago=timedelta(minutes=1),
|
||||
expires_in=timedelta(minutes=9),
|
||||
pid=os.getpid(),
|
||||
)
|
||||
freshness = ils.assess_lock_freshness(existing, now=self.now)
|
||||
self.assertEqual(freshness["status"], ils.STATUS_STALE_ABSOLUTE_CAP)
|
||||
self.assertFalse(freshness["live"])
|
||||
self.assertFalse(ils.is_lease_expired(existing, now=self.now))
|
||||
with mock.patch.object(ils, "is_process_alive", return_value=True):
|
||||
self.assertIsNone(self._foreign_conflict(existing))
|
||||
|
||||
def test_missed_heartbeat_ttl_gt_grace_future_expires_allows_foreign_reclaim(self):
|
||||
# Heartbeat grace (default 10 min) lapsed 5 min ago, but a TTL>grace policy
|
||||
# leaves expires_at 20 min in the FUTURE: stale_missed_heartbeat, not expired.
|
||||
existing = self._lock(
|
||||
lifecycle=lease_policy.LIFECYCLE_HEARTBEAT_V1,
|
||||
created_ago=timedelta(minutes=30),
|
||||
heartbeat_ago=timedelta(minutes=15),
|
||||
expires_in=timedelta(minutes=20),
|
||||
pid=os.getpid(),
|
||||
)
|
||||
freshness = ils.assess_lock_freshness(existing, now=self.now)
|
||||
self.assertEqual(freshness["status"], ils.STATUS_STALE_MISSED_HEARTBEAT)
|
||||
self.assertFalse(freshness["live"])
|
||||
self.assertFalse(ils.is_lease_expired(existing, now=self.now))
|
||||
with mock.patch.object(ils, "is_process_alive", return_value=True):
|
||||
self.assertIsNone(self._foreign_conflict(existing))
|
||||
|
||||
def test_same_owner_may_reclaim_its_own_abandoned_heartbeat_lease(self):
|
||||
existing = self._lock(
|
||||
lifecycle=lease_policy.LIFECYCLE_HEARTBEAT_V1,
|
||||
created_ago=timedelta(hours=9),
|
||||
heartbeat_ago=timedelta(minutes=1),
|
||||
expires_in=timedelta(minutes=9),
|
||||
pid=os.getpid(),
|
||||
)
|
||||
with mock.patch.object(ils, "is_process_alive", return_value=True):
|
||||
self.assertIsNone(self._same_owner_conflict(existing))
|
||||
|
||||
|
||||
class TestLiveAndLegacyStillBlockForeign(_ConflictGateBase):
|
||||
def test_live_heartbeat_lease_still_blocks_foreign(self):
|
||||
existing = self._lock(
|
||||
lifecycle=lease_policy.LIFECYCLE_HEARTBEAT_V1,
|
||||
created_ago=timedelta(minutes=5),
|
||||
heartbeat_ago=timedelta(minutes=1),
|
||||
expires_in=timedelta(minutes=9),
|
||||
pid=os.getpid(),
|
||||
)
|
||||
freshness = ils.assess_lock_freshness(existing, now=self.now)
|
||||
self.assertTrue(freshness["live"])
|
||||
with mock.patch.object(ils, "is_process_alive", return_value=True):
|
||||
block = self._foreign_conflict(existing)
|
||||
self.assertIn("already has an active", block or "")
|
||||
|
||||
def test_legacy_lease_future_expires_dead_pid_is_not_reclaimed_here(self):
|
||||
# AC-N8: a legacy lock keeps its absolute expires_at clock. Non-expired +
|
||||
# dead PID is non-live, but the widened band excludes legacy, so the
|
||||
# foreign acquirer is still blocked rather than silently reclaiming.
|
||||
existing = self._lock(
|
||||
lifecycle=None,
|
||||
created_ago=timedelta(hours=9),
|
||||
heartbeat_ago=timedelta(hours=9),
|
||||
expires_in=timedelta(hours=2),
|
||||
pid=4_194_304, # far above any live pid on a test host
|
||||
)
|
||||
with mock.patch.object(ils, "is_process_alive", return_value=False):
|
||||
freshness = ils.assess_lock_freshness(existing, now=self.now)
|
||||
self.assertTrue(freshness["legacy_lease"])
|
||||
self.assertFalse(freshness["live"])
|
||||
self.assertFalse(ils.is_lease_expired(existing, now=self.now))
|
||||
block = self._foreign_conflict(existing)
|
||||
self.assertIn("already has an active", block or "")
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
Reference in New Issue
Block a user