Merge branch 'master' into fix/issue-790-slice-a-heartbeat-policy
This commit is contained in:
+366
-5
@@ -181,6 +181,7 @@ def bind_session_lock(
|
||||
*,
|
||||
expected_generation: int | None = None,
|
||||
renewal_sanctioned: bool = False,
|
||||
recovery_sanctioned: bool = False,
|
||||
) -> str:
|
||||
"""Persist a keyed lock and bind it to the current process session.
|
||||
|
||||
@@ -225,7 +226,9 @@ def bind_session_lock(
|
||||
try:
|
||||
with _exclusive_file_lock(sentinel):
|
||||
existing = read_lock_file(path)
|
||||
overwrite_block = assess_foreign_lock_overwrite(existing, record)
|
||||
overwrite_block = assess_foreign_lock_overwrite(
|
||||
existing, record, recovery_sanctioned=recovery_sanctioned
|
||||
)
|
||||
if overwrite_block:
|
||||
raise RuntimeError(overwrite_block)
|
||||
lease_block = assess_same_issue_lease_conflict(
|
||||
@@ -234,6 +237,7 @@ def bind_session_lock(
|
||||
branch_name=str(record.get("branch_name") or ""),
|
||||
worktree_path=str(record.get("worktree_path") or ""),
|
||||
renewal_sanctioned=renewal_sanctioned,
|
||||
recovery_sanctioned=recovery_sanctioned,
|
||||
)
|
||||
if lease_block:
|
||||
raise RuntimeError(lease_block)
|
||||
@@ -814,8 +818,18 @@ def assess_lock_freshness(
|
||||
pid = lock_data.get("session_pid")
|
||||
if pid is None:
|
||||
pid = lock_data.get("pid")
|
||||
# Evidence only. Never consulted to grant liveness (AC-N2).
|
||||
pid_alive = is_process_alive(pid) if pid is not None else False
|
||||
if pid is None:
|
||||
pid = lock_data.get("owner_pid")
|
||||
pid_missing = pid is None or str(pid).strip() == ""
|
||||
try:
|
||||
pid_int = int(pid) if not pid_missing else None
|
||||
if pid_int is not None and pid_int <= 0:
|
||||
pid_missing = True
|
||||
pid_int = None
|
||||
except (TypeError, ValueError):
|
||||
pid_missing = True
|
||||
pid_int = None
|
||||
pid_alive = is_process_alive(pid_int) if pid_int is not None else False
|
||||
|
||||
lifecycle = lease_lifecycle_version(lock_data)
|
||||
legacy = lifecycle != lease_policy.LIFECYCLE_HEARTBEAT_V1
|
||||
@@ -823,6 +837,7 @@ def assess_lock_freshness(
|
||||
|
||||
evidence: dict[str, Any] = {
|
||||
"pid_alive": pid_alive,
|
||||
"pid_missing": pid_missing,
|
||||
"lifecycle": lifecycle,
|
||||
"legacy_lease": legacy,
|
||||
"task_session_id": lease_task_session_id(lock_data) or None,
|
||||
@@ -1010,6 +1025,7 @@ def assess_same_issue_lease_conflict(
|
||||
worktree_path: str,
|
||||
operation_type: str = AUTHOR_ISSUE_WORK_LEASE,
|
||||
renewal_sanctioned: bool = False,
|
||||
recovery_sanctioned: bool = False,
|
||||
now: datetime | None = None,
|
||||
) -> str | None:
|
||||
"""Return a fail-closed error when a competing live lease blocks acquisition.
|
||||
@@ -1041,6 +1057,8 @@ def assess_same_issue_lease_conflict(
|
||||
existing_branch == branch_name
|
||||
and _same_realpath(str(existing_worktree or ""), worktree_path)
|
||||
)
|
||||
if recovery_sanctioned and existing_issue == issue_number and existing_branch == branch_name:
|
||||
return None
|
||||
expired = is_lease_expired(existing_lock, now=now)
|
||||
# #790 review #502/#516: a heartbeat-lifecycle lease that is non-live but
|
||||
# whose absolute expires_at is still in the future must enter the same
|
||||
@@ -1094,10 +1112,26 @@ def assess_same_issue_lease_conflict(
|
||||
)
|
||||
|
||||
|
||||
def _lock_claimant(lock: dict[str, Any] | None) -> dict[str, str]:
|
||||
if not isinstance(lock, dict):
|
||||
return {}
|
||||
claimant = lock.get("claimant")
|
||||
if not isinstance(claimant, dict):
|
||||
lease = lock.get("work_lease")
|
||||
claimant = lease.get("claimant") if isinstance(lease, dict) else None
|
||||
if not isinstance(claimant, dict):
|
||||
return {}
|
||||
return {
|
||||
"username": str(claimant.get("username") or ""),
|
||||
"profile": str(claimant.get("profile") or ""),
|
||||
}
|
||||
|
||||
|
||||
def assess_foreign_lock_overwrite(
|
||||
existing_lock: dict[str, Any] | None,
|
||||
incoming_lock: dict[str, Any],
|
||||
*,
|
||||
recovery_sanctioned: bool = False,
|
||||
now: datetime | None = None,
|
||||
) -> str | None:
|
||||
"""Block writes that would clobber an unrelated live lease on the same key."""
|
||||
@@ -1112,8 +1146,31 @@ def assess_foreign_lock_overwrite(
|
||||
)
|
||||
if same_issue and same_branch and same_worktree:
|
||||
return None
|
||||
if not is_lease_live(existing_lock, now=now):
|
||||
|
||||
existing_claimant = _lock_claimant(existing_lock)
|
||||
incoming_claimant = _lock_claimant(incoming_lock)
|
||||
same_claimant = (
|
||||
bool(existing_claimant.get("username"))
|
||||
and existing_claimant.get("username") == incoming_claimant.get("username")
|
||||
and existing_claimant.get("profile") == incoming_claimant.get("profile")
|
||||
)
|
||||
|
||||
if recovery_sanctioned and same_issue and same_branch and same_claimant:
|
||||
return None
|
||||
|
||||
if not is_lease_live(existing_lock, now=now):
|
||||
# #860 F8: A non-live or PID-less lock still blocks foreign overwrite
|
||||
# unless same claimant or sanctioned reclaim is proven.
|
||||
if not same_claimant and same_issue:
|
||||
reclaim = assess_expired_lock_reclaim(existing_lock, now=now)
|
||||
if not reclaim.get("reclaim_allowed"):
|
||||
return (
|
||||
"Refusing foreign overwrite of non-live issue lock "
|
||||
f"(issue #{existing_lock.get('issue_number')}, owner '{existing_claimant.get('username')}') "
|
||||
"without sanctioned reclaim proof (fail closed)"
|
||||
)
|
||||
return None
|
||||
|
||||
return (
|
||||
"Refusing to overwrite a live foreign issue lock "
|
||||
f"(issue #{existing_lock.get('issue_number')}, "
|
||||
@@ -1284,4 +1341,308 @@ def format_lock_proof(
|
||||
parts.append("lock released")
|
||||
elif released is False:
|
||||
parts.append("lock retained")
|
||||
return "; ".join(parts)
|
||||
return "; ".join(parts)
|
||||
|
||||
|
||||
# ── #871: durable linked-issue lock head refresh after branch synchronization ──
|
||||
_FULL_SHA_RE = re.compile(r"^[0-9a-f]{40}$", re.IGNORECASE)
|
||||
|
||||
# Provenance recorded on the lock when the head is refreshed by a sanctioned
|
||||
# merge-based branch synchronization (``gitea_update_pr_branch_by_merge``).
|
||||
LOCK_HEAD_REFRESH_PROVENANCE_MERGE_SYNC = "gitea_update_pr_branch_by_merge"
|
||||
|
||||
|
||||
def _norm_sha(value: Any) -> str | None:
|
||||
text = str(value or "").strip().lower()
|
||||
return text if _FULL_SHA_RE.match(text) else None
|
||||
|
||||
|
||||
def _lock_claimant_view(lock: dict[str, Any] | None) -> dict[str, Any]:
|
||||
if not isinstance(lock, dict):
|
||||
return {}
|
||||
claimant = lock.get("claimant")
|
||||
if not isinstance(claimant, dict):
|
||||
lease = lock.get("work_lease")
|
||||
claimant = lease.get("claimant") if isinstance(lease, dict) else None
|
||||
return dict(claimant) if isinstance(claimant, dict) else {}
|
||||
|
||||
|
||||
def assess_durable_lock_head_refresh(
|
||||
existing_lock: dict[str, Any] | None,
|
||||
*,
|
||||
remote: str,
|
||||
org: str,
|
||||
repo: str,
|
||||
issue_number: int,
|
||||
branch_name: str,
|
||||
worktree_path: str,
|
||||
pr_number: int | None,
|
||||
identity: str | None,
|
||||
profile: str | None,
|
||||
current_pid: int | None,
|
||||
expected_old_head: str | None,
|
||||
new_head: str | None,
|
||||
base_head: str | None = None,
|
||||
) -> dict[str, Any]:
|
||||
"""Fail-closed assessment for refreshing a durable lock's recorded head (#871).
|
||||
|
||||
A successful ``gitea_update_pr_branch_by_merge`` advances the *remote* PR head
|
||||
but must also advance the durable linked-issue lock so a later dead-session
|
||||
recovery can prove ownership. This decides whether that refresh is permitted;
|
||||
it mutates nothing.
|
||||
|
||||
Every element of durable ownership is re-verified against the persisted lock —
|
||||
repository, issue, branch, worktree, claimant identity/profile, and the live
|
||||
owning session — and the recorded head is compare-and-swapped: the lock's
|
||||
currently recorded synced head (if any) must equal ``expected_old_head``, so a
|
||||
lock whose head or provenance changed concurrently is never overwritten.
|
||||
"""
|
||||
reasons: list[str] = []
|
||||
old = _norm_sha(expected_old_head)
|
||||
new = _norm_sha(new_head)
|
||||
evidence: dict[str, Any] = {
|
||||
"issue_number": issue_number,
|
||||
"branch_name": branch_name,
|
||||
"worktree_path": worktree_path,
|
||||
"pr_number": pr_number,
|
||||
"expected_old_head": old,
|
||||
"new_head": new,
|
||||
"base_head": _norm_sha(base_head),
|
||||
}
|
||||
|
||||
if not isinstance(existing_lock, dict) or not existing_lock:
|
||||
reasons.append("no durable lock exists for this issue; nothing to refresh")
|
||||
return {"allowed": False, "reasons": reasons, "evidence": evidence,
|
||||
"expected_generation": 0}
|
||||
|
||||
lock = dict(existing_lock)
|
||||
evidence["current_generation"] = lock_generation(lock)
|
||||
|
||||
if lock.get("issue_number") != issue_number:
|
||||
reasons.append(
|
||||
f"durable lock targets issue #{lock.get('issue_number')}, not "
|
||||
f"#{issue_number}; refusing head refresh"
|
||||
)
|
||||
|
||||
for field, expected in (("remote", remote), ("org", org), ("repo", repo)):
|
||||
actual = str(lock.get(field) or "").strip()
|
||||
if actual != str(expected or "").strip():
|
||||
reasons.append(
|
||||
f"lock {field} '{actual}' does not match requested "
|
||||
f"'{str(expected or '').strip()}'"
|
||||
)
|
||||
|
||||
locked_branch = str(lock.get("branch_name") or "").strip()
|
||||
if locked_branch != str(branch_name or "").strip():
|
||||
reasons.append(
|
||||
f"lock branch '{locked_branch}' does not match requested "
|
||||
f"'{str(branch_name or '').strip()}'"
|
||||
)
|
||||
|
||||
locked_worktree = str(lock.get("worktree_path") or "").strip()
|
||||
try:
|
||||
same_wt = bool(locked_worktree) and bool(worktree_path) and (
|
||||
os.path.realpath(locked_worktree) == os.path.realpath(worktree_path)
|
||||
)
|
||||
except OSError:
|
||||
same_wt = locked_worktree == (worktree_path or "")
|
||||
if not same_wt:
|
||||
reasons.append(
|
||||
f"lock worktree '{locked_worktree}' does not match declared "
|
||||
f"'{str(worktree_path or '').strip()}'"
|
||||
)
|
||||
|
||||
claimant = _lock_claimant_view(lock)
|
||||
locked_identity = str(claimant.get("username") or "").strip()
|
||||
locked_profile = str(claimant.get("profile") or "").strip()
|
||||
if not locked_identity or not locked_profile:
|
||||
reasons.append(
|
||||
"durable lock does not record a claimant identity/profile; "
|
||||
"ownership could not be proven for head refresh"
|
||||
)
|
||||
if not str(identity or "").strip() or not str(profile or "").strip():
|
||||
reasons.append(
|
||||
"active session identity/profile is unknown; ownership could not be "
|
||||
"proven for head refresh"
|
||||
)
|
||||
if locked_identity and str(identity or "").strip() and locked_identity != str(identity).strip():
|
||||
reasons.append(
|
||||
f"lock claimant '{locked_identity}' does not match active identity "
|
||||
f"'{str(identity).strip()}'"
|
||||
)
|
||||
if locked_profile and str(profile or "").strip() and locked_profile != str(profile).strip():
|
||||
reasons.append(
|
||||
f"lock profile '{locked_profile}' does not match active profile "
|
||||
f"'{str(profile).strip()}'"
|
||||
)
|
||||
|
||||
# The refresh is written by the LIVE owning author session. A refresh is not
|
||||
# a recovery: the current process must be the recorded owner.
|
||||
recorded_pid = lock.get("session_pid")
|
||||
if recorded_pid is None:
|
||||
recorded_pid = lock.get("pid")
|
||||
evidence["recorded_pid"] = recorded_pid
|
||||
evidence["current_pid"] = current_pid
|
||||
if current_pid is None:
|
||||
reasons.append("current session pid is unknown; cannot prove live ownership")
|
||||
else:
|
||||
try:
|
||||
if recorded_pid is None or int(recorded_pid) != int(current_pid):
|
||||
reasons.append(
|
||||
f"durable lock is owned by pid {recorded_pid}, not the current "
|
||||
f"session pid {current_pid}; head refresh requires the live owner"
|
||||
)
|
||||
except (TypeError, ValueError):
|
||||
reasons.append(
|
||||
"durable lock owner pid is malformed; cannot prove live ownership"
|
||||
)
|
||||
|
||||
if not old:
|
||||
reasons.append("expected_old_head is not a full 40-char hex SHA (fail closed)")
|
||||
if not new:
|
||||
reasons.append("new_head is not a full 40-char hex SHA (fail closed)")
|
||||
if old and new and old == new:
|
||||
reasons.append(
|
||||
"new head equals the expected old head; a sync must advance the head"
|
||||
)
|
||||
|
||||
# Compare-and-swap on the recorded head: if the lock already records a synced
|
||||
# head it must be exactly the expected old head, else another sync moved it.
|
||||
recorded_synced = _norm_sha(lock.get("synced_pr_head"))
|
||||
evidence["recorded_synced_pr_head"] = recorded_synced
|
||||
if recorded_synced is not None and old is not None and recorded_synced != old:
|
||||
reasons.append(
|
||||
f"durable lock already records synced head {recorded_synced}, not the "
|
||||
f"expected old head {old}; a concurrent sync changed it (CAS fail closed)"
|
||||
)
|
||||
|
||||
if reasons:
|
||||
return {"allowed": False, "reasons": reasons, "evidence": evidence,
|
||||
"expected_generation": lock_generation(lock)}
|
||||
|
||||
return {
|
||||
"allowed": True,
|
||||
"reasons": [
|
||||
f"durable lock for issue #{issue_number} branch '{locked_branch}' is "
|
||||
f"owned by the live session; refresh recorded head {old} -> {new}"
|
||||
],
|
||||
"evidence": evidence,
|
||||
"expected_generation": lock_generation(lock),
|
||||
}
|
||||
|
||||
|
||||
def apply_durable_lock_head_refresh(
|
||||
*,
|
||||
remote: str,
|
||||
org: str,
|
||||
repo: str,
|
||||
issue_number: int,
|
||||
branch_name: str,
|
||||
worktree_path: str,
|
||||
pr_number: int | None,
|
||||
identity: str | None,
|
||||
profile: str | None,
|
||||
current_pid: int | None,
|
||||
expected_old_head: str | None,
|
||||
new_head: str | None,
|
||||
synced_at: str,
|
||||
base_head: str | None = None,
|
||||
provenance: str = LOCK_HEAD_REFRESH_PROVENANCE_MERGE_SYNC,
|
||||
lock_dir: str | None = None,
|
||||
) -> dict[str, Any]:
|
||||
"""CAS-refresh the durable lock's recorded head after a branch sync (#871).
|
||||
|
||||
Reads the durable lock from disk, re-asserts ownership via
|
||||
``assess_durable_lock_head_refresh``, and — only when permitted — writes the
|
||||
new synced head through ``bind_session_lock`` with a generation compare-and-
|
||||
swap. Then re-reads the lock and proves it records the complete new head
|
||||
(read-after-write). Any failure at any step returns ``refreshed=False`` with
|
||||
reasons; the caller must treat that as a partial lifecycle failure and never
|
||||
report a fully successful synchronization.
|
||||
"""
|
||||
existing = load_issue_lock(
|
||||
remote=remote, org=org, repo=repo, issue_number=issue_number, lock_dir=lock_dir
|
||||
)
|
||||
assessment = assess_durable_lock_head_refresh(
|
||||
existing,
|
||||
remote=remote,
|
||||
org=org,
|
||||
repo=repo,
|
||||
issue_number=issue_number,
|
||||
branch_name=branch_name,
|
||||
worktree_path=worktree_path,
|
||||
pr_number=pr_number,
|
||||
identity=identity,
|
||||
profile=profile,
|
||||
current_pid=current_pid,
|
||||
expected_old_head=expected_old_head,
|
||||
new_head=new_head,
|
||||
base_head=base_head,
|
||||
)
|
||||
result: dict[str, Any] = {
|
||||
"refreshed": False,
|
||||
"read_after_write_ok": False,
|
||||
"prior_head": _norm_sha(expected_old_head),
|
||||
"new_head": _norm_sha(new_head),
|
||||
"reasons": list(assessment.get("reasons") or []),
|
||||
"evidence": assessment.get("evidence"),
|
||||
}
|
||||
if not assessment.get("allowed"):
|
||||
return result
|
||||
|
||||
new = _norm_sha(new_head)
|
||||
old = _norm_sha(expected_old_head)
|
||||
record = dict(existing or {})
|
||||
sync_block = {
|
||||
"last_synced_pr_head": new,
|
||||
"prior_pr_head": old,
|
||||
"base_head": _norm_sha(base_head),
|
||||
"pr_number": pr_number,
|
||||
"provenance": provenance,
|
||||
"synced_at": synced_at,
|
||||
"synced_by_pid": current_pid,
|
||||
"synced_by": {
|
||||
"username": str(identity or "").strip() or None,
|
||||
"profile": str(profile or "").strip() or None,
|
||||
},
|
||||
}
|
||||
record["synced_pr_head"] = new
|
||||
record["branch_sync"] = sync_block
|
||||
history = record.get("branch_sync_history")
|
||||
if not isinstance(history, list):
|
||||
history = []
|
||||
history = list(history)
|
||||
history.append(sync_block)
|
||||
record["branch_sync_history"] = history
|
||||
|
||||
try:
|
||||
bind_session_lock(
|
||||
record,
|
||||
lock_dir=lock_dir,
|
||||
expected_generation=assessment.get("expected_generation"),
|
||||
renewal_sanctioned=True,
|
||||
)
|
||||
except Exception as exc: # CAS miss or write failure — partial lifecycle failure
|
||||
result["reasons"].append(
|
||||
f"durable lock head refresh write failed (fail closed): {exc}"
|
||||
)
|
||||
return result
|
||||
|
||||
after = load_issue_lock(
|
||||
remote=remote, org=org, repo=repo, issue_number=issue_number, lock_dir=lock_dir
|
||||
)
|
||||
after_head = _norm_sha((after or {}).get("synced_pr_head"))
|
||||
result["lock_generation_after"] = lock_generation(after)
|
||||
if after_head == new and new is not None:
|
||||
result["refreshed"] = True
|
||||
result["read_after_write_ok"] = True
|
||||
result["reasons"].append(
|
||||
f"durable lock recorded head refreshed to {new} and verified by "
|
||||
"read-after-write"
|
||||
)
|
||||
else:
|
||||
result["reasons"].append(
|
||||
"read-after-write verification failed: durable lock does not record "
|
||||
f"the new head {new} (found {after_head}); partial lifecycle failure"
|
||||
)
|
||||
return result
|
||||
Reference in New Issue
Block a user