Merge branch 'master' into feat/issue-420-server-code-parity
This commit is contained in:
+271
-15
@@ -696,6 +696,39 @@ def _verify_role_mutation_workspace(
|
||||
task: str | None = None,
|
||||
) -> str:
|
||||
"""Bind reviewer/merger mutations to the active namespace workspace (#510)."""
|
||||
# Check running runtimes to prevent stale mutations
|
||||
try:
|
||||
if "PYTEST_CURRENT_TEST" not in os.environ or "GITEA_FORCE_MCP_RUNTIME_CHECK" in os.environ:
|
||||
config = gitea_config.load_config()
|
||||
required_permission = task_capability_map.required_permission(task) if task else None
|
||||
matching_profiles = []
|
||||
if required_permission and config and "profiles" in config:
|
||||
for p_name, p_data in config["profiles"].items():
|
||||
p_allowed = p_data.get("allowed_operations") or []
|
||||
p_forbidden = p_data.get("forbidden_operations") or []
|
||||
p_allowed_n = []
|
||||
for op in p_allowed:
|
||||
try:
|
||||
p_allowed_n.append(gitea_config.normalize_operation(op))
|
||||
except Exception:
|
||||
pass
|
||||
p_forbidden_n = []
|
||||
for op in p_forbidden:
|
||||
try:
|
||||
p_forbidden_n.append(gitea_config.normalize_operation(op))
|
||||
except Exception:
|
||||
pass
|
||||
ok, _ = gitea_config.check_operation(required_permission, p_allowed_n, p_forbidden_n)
|
||||
if ok:
|
||||
matching_profiles.append(p_name)
|
||||
runtime_reasons = _check_mcp_runtimes_diagnostics(task or "unknown", matching_profiles)
|
||||
if runtime_reasons:
|
||||
raise RuntimeError("; ".join(runtime_reasons))
|
||||
except Exception as exc:
|
||||
if "stale-runtime:" in str(exc):
|
||||
raise RuntimeError(str(exc))
|
||||
pass
|
||||
|
||||
role = _effective_workspace_role()
|
||||
git_state = issue_lock_worktree.read_worktree_git_state(
|
||||
_resolve_preflight_workspace_path(worktree_path)
|
||||
@@ -771,6 +804,7 @@ import task_capability_map # noqa: E402
|
||||
import review_proofs # noqa: E402
|
||||
import review_workflow_boundary # noqa: E402
|
||||
import review_workflow_load # noqa: E402
|
||||
import mcp_session_state # noqa: E402
|
||||
import agent_temp_artifacts
|
||||
import issue_lock_worktree # noqa: E402
|
||||
import issue_lock_provenance # noqa: E402
|
||||
@@ -2454,37 +2488,110 @@ _REVIEW_ACTIONS = {
|
||||
|
||||
_TERMINAL_REVIEW_ACTIONS = frozenset({"approve", "request_changes"})
|
||||
|
||||
# In-process only (#211): never persist to /tmp — host-global files are
|
||||
# spoofable by any local process and go stale across sessions.
|
||||
# Durable across MCP daemon process pools (#559), but never under /tmp (#211):
|
||||
# host-global temp files are spoofable. Persistence uses the user-private
|
||||
# cache directory from mcp_session_state (mode 0o700 / files 0o600), keyed by
|
||||
# remote + profile identity with TTL.
|
||||
_REVIEW_DECISION_LOCK: dict | None = None
|
||||
|
||||
|
||||
def _decision_lock_binding(lock: dict | None = None) -> dict:
|
||||
"""Resolve key fields for durable decision-lock storage."""
|
||||
profile = get_profile()
|
||||
profile_name = (profile.get("profile_name") or "").strip()
|
||||
env_lock = (os.environ.get(SESSION_PROFILE_LOCK_ENV) or "").strip()
|
||||
stored_lock = ((lock or {}).get("session_profile_lock") or "").strip()
|
||||
session_lock = env_lock or stored_lock or profile_name
|
||||
remote = ((lock or {}).get("remote") or "").strip() or None
|
||||
org = ((lock or {}).get("org") or (lock or {}).get("ready_org") or "").strip() or None
|
||||
repo = ((lock or {}).get("repo") or (lock or {}).get("ready_repo") or "").strip() or None
|
||||
return {
|
||||
"session_profile": profile_name,
|
||||
"session_profile_lock": session_lock,
|
||||
"profile_identity": mcp_session_state.current_profile_identity(
|
||||
profile_name=profile_name,
|
||||
session_profile_lock=session_lock,
|
||||
),
|
||||
"remote": remote,
|
||||
"org": org,
|
||||
"repo": repo,
|
||||
}
|
||||
|
||||
|
||||
def _load_review_decision_lock():
|
||||
"""Load decision lock from memory, falling back to durable session state."""
|
||||
global _REVIEW_DECISION_LOCK
|
||||
if _REVIEW_DECISION_LOCK is not None:
|
||||
return _REVIEW_DECISION_LOCK
|
||||
binding = _decision_lock_binding()
|
||||
# Profile-keyed durable file; remote/org/repo validated from payload (#559).
|
||||
durable = mcp_session_state.load_state(
|
||||
kind=mcp_session_state.KIND_DECISION_LOCK,
|
||||
profile_identity=binding.get("profile_identity"),
|
||||
)
|
||||
if durable is not None:
|
||||
_REVIEW_DECISION_LOCK = dict(durable)
|
||||
return _REVIEW_DECISION_LOCK
|
||||
|
||||
|
||||
def _save_review_decision_lock(data):
|
||||
"""Persist decision lock to memory + durable shared state (#559)."""
|
||||
global _REVIEW_DECISION_LOCK
|
||||
_REVIEW_DECISION_LOCK = data
|
||||
if data is None:
|
||||
binding = _decision_lock_binding(_REVIEW_DECISION_LOCK)
|
||||
mcp_session_state.clear_state(
|
||||
kind=mcp_session_state.KIND_DECISION_LOCK,
|
||||
profile_identity=binding.get("profile_identity"),
|
||||
)
|
||||
_REVIEW_DECISION_LOCK = None
|
||||
return
|
||||
payload = dict(data)
|
||||
binding = _decision_lock_binding(payload)
|
||||
payload.setdefault("session_pid", os.getpid())
|
||||
payload["writer_pid"] = os.getpid()
|
||||
payload["session_profile"] = binding["session_profile"] or payload.get(
|
||||
"session_profile"
|
||||
)
|
||||
payload["session_profile_lock"] = binding["session_profile_lock"]
|
||||
payload["profile_identity"] = binding["profile_identity"]
|
||||
if binding.get("remote") and not payload.get("remote"):
|
||||
payload["remote"] = binding["remote"]
|
||||
persisted = mcp_session_state.save_state(
|
||||
kind=mcp_session_state.KIND_DECISION_LOCK,
|
||||
payload=payload,
|
||||
remote=payload.get("remote"),
|
||||
org=payload.get("org") or payload.get("ready_org"),
|
||||
repo=payload.get("repo") or payload.get("ready_repo"),
|
||||
profile_identity=payload.get("profile_identity"),
|
||||
)
|
||||
_REVIEW_DECISION_LOCK = dict(persisted or payload)
|
||||
|
||||
|
||||
def _review_decision_session_reasons(lock: dict | None) -> list[str]:
|
||||
"""Reject locks that do not belong to this MCP process/session."""
|
||||
"""Reject locks that do not belong to this MCP session identity (#559).
|
||||
|
||||
Different daemon PIDs in the same IDE session pool are allowed when the
|
||||
profile identity and remote match. Spoofed/stale locks still fail closed.
|
||||
"""
|
||||
if lock is None:
|
||||
return []
|
||||
reasons = []
|
||||
if lock.get("session_pid") != os.getpid():
|
||||
reasons.append(
|
||||
"review decision lock was created in a different process "
|
||||
"(fail closed)"
|
||||
)
|
||||
env_lock = (os.environ.get(SESSION_PROFILE_LOCK_ENV) or "").strip()
|
||||
stored_lock = (lock.get("session_profile_lock") or "").strip()
|
||||
stored_lock = (lock.get("session_profile_lock") or lock.get("profile_identity") or "").strip()
|
||||
if env_lock and stored_lock and env_lock != stored_lock:
|
||||
reasons.append(
|
||||
"review decision lock session profile lock mismatch (fail closed)"
|
||||
)
|
||||
# TTL / identity checks from durable envelope fields.
|
||||
for reason in mcp_session_state.identity_match_reasons(
|
||||
lock,
|
||||
remote=lock.get("remote"),
|
||||
org=lock.get("org") or lock.get("ready_org"),
|
||||
repo=lock.get("repo") or lock.get("ready_repo"),
|
||||
profile_identity=env_lock or stored_lock,
|
||||
):
|
||||
if "profile identity mismatch" in reason or "expired" in reason or "future" in reason or "missing recorded_at" in reason:
|
||||
reasons.append(reason)
|
||||
return reasons
|
||||
|
||||
|
||||
@@ -2495,11 +2602,12 @@ def init_review_decision_lock(remote: str | None, task: str | None, force: bool
|
||||
if not force:
|
||||
lock = _load_review_decision_lock()
|
||||
if lock is not None:
|
||||
if lock.get("remote") == remote and lock.get("session_pid") == os.getpid():
|
||||
env_lock = (os.environ.get(SESSION_PROFILE_LOCK_ENV) or "").strip()
|
||||
stored_lock = (lock.get("session_profile_lock") or "").strip()
|
||||
if not env_lock or not stored_lock or env_lock == stored_lock:
|
||||
return
|
||||
env_lock = (os.environ.get(SESSION_PROFILE_LOCK_ENV) or "").strip()
|
||||
stored_lock = (lock.get("session_profile_lock") or "").strip()
|
||||
same_remote = lock.get("remote") == remote
|
||||
same_profile = (not env_lock or not stored_lock or env_lock == stored_lock)
|
||||
if same_remote and same_profile and not _review_decision_session_reasons(lock):
|
||||
return
|
||||
review_workflow_load.clear_review_workflow_load()
|
||||
profile = get_profile()
|
||||
profile_name = (profile.get("profile_name") or "").strip()
|
||||
@@ -2514,6 +2622,10 @@ def init_review_decision_lock(remote: str | None, task: str | None, force: bool
|
||||
"session_pid": os.getpid(),
|
||||
"session_profile": profile_name,
|
||||
"session_profile_lock": session_lock,
|
||||
"profile_identity": mcp_session_state.current_profile_identity(
|
||||
profile_name=profile_name,
|
||||
session_profile_lock=session_lock,
|
||||
),
|
||||
"final_review_decision_ready": False,
|
||||
"ready_pr_number": None,
|
||||
"ready_action": None,
|
||||
@@ -4632,6 +4744,34 @@ def gitea_reconcile_merged_cleanups(
|
||||
)
|
||||
|
||||
delete_capability_allowed = not _profile_operation_gate("gitea.branch.delete")
|
||||
|
||||
# #534: discover reviewer scratch trees and active leases for those PRs.
|
||||
scratch_candidates = merged_cleanup_reconcile.discover_reviewer_scratch_worktrees(
|
||||
PROJECT_ROOT
|
||||
)
|
||||
active_reviewer_leases: dict[int, bool] = {}
|
||||
pr_states: dict[int, dict] = {}
|
||||
for scratch in scratch_candidates:
|
||||
pr_num = int(scratch["pr_number"])
|
||||
if pr_num in active_reviewer_leases:
|
||||
continue
|
||||
try:
|
||||
comments = api_get_all(f"{base}/issues/{pr_num}/comments", auth)
|
||||
except Exception:
|
||||
comments = []
|
||||
lease = reviewer_pr_lease.find_active_reviewer_lease(
|
||||
comments, pr_number=pr_num
|
||||
)
|
||||
active_reviewer_leases[pr_num] = bool(lease)
|
||||
try:
|
||||
pr_live = api_request("GET", f"{base}/pulls/{pr_num}", auth)
|
||||
except Exception:
|
||||
pr_live = {}
|
||||
pr_states[pr_num] = {
|
||||
"merged": bool((pr_live or {}).get("merged") or (pr_live or {}).get("merged_at")),
|
||||
"closed": (pr_live or {}).get("state") == "closed",
|
||||
}
|
||||
|
||||
report = merged_cleanup_reconcile.build_reconciliation_report(
|
||||
project_root=PROJECT_ROOT,
|
||||
closed_prs=merged_closed,
|
||||
@@ -4639,6 +4779,8 @@ def gitea_reconcile_merged_cleanups(
|
||||
remote_branch_exists=remote_branch_exists,
|
||||
head_on_master=head_on_master,
|
||||
delete_capability_allowed=delete_capability_allowed,
|
||||
active_reviewer_leases=active_reviewer_leases,
|
||||
pr_states=pr_states,
|
||||
)
|
||||
|
||||
if dry_run:
|
||||
@@ -4682,6 +4824,14 @@ def gitea_reconcile_merged_cleanups(
|
||||
)
|
||||
actions.append({"action": "remove_local_worktree", **result})
|
||||
|
||||
for scratch in report.get("reviewer_scratch_entries") or []:
|
||||
if not scratch.get("safe_to_remove_worktree"):
|
||||
continue
|
||||
result = merged_cleanup_reconcile.remove_reviewer_scratch_worktree(
|
||||
PROJECT_ROOT, scratch.get("worktree_path") or ""
|
||||
)
|
||||
actions.append({"action": "remove_reviewer_scratch_worktree", **result})
|
||||
|
||||
report["dry_run"] = False
|
||||
report["executed"] = True
|
||||
report["actions"] = actions
|
||||
@@ -8689,6 +8839,102 @@ def gitea_route_task_session(
|
||||
)
|
||||
|
||||
|
||||
def _check_mcp_runtimes_diagnostics(task: str, matching_profiles: list[str]) -> list[str]:
|
||||
"""Check running runtimes and return errors if they are missing or stale."""
|
||||
import subprocess
|
||||
import re
|
||||
from datetime import datetime
|
||||
|
||||
reasons = []
|
||||
code_path = os.path.join(PROJECT_ROOT, "gitea_mcp_server.py")
|
||||
if not os.path.exists(code_path):
|
||||
return reasons
|
||||
|
||||
code_mtime = datetime.fromtimestamp(os.path.getmtime(code_path))
|
||||
|
||||
try:
|
||||
proc = subprocess.run(
|
||||
["ps", "-o", "pid,lstart,command", "-ax"],
|
||||
capture_output=True, text=True, check=True
|
||||
)
|
||||
except Exception as exc:
|
||||
return [f"stale-runtime: failed to list running processes: {exc}"]
|
||||
|
||||
self_pid = os.getpid()
|
||||
self_stale = False
|
||||
|
||||
running_profiles = {}
|
||||
for line in proc.stdout.splitlines()[1:]:
|
||||
line = line.strip()
|
||||
if not line or "mcp_server.py" not in line:
|
||||
continue
|
||||
|
||||
parts = line.split(None, 6)
|
||||
if len(parts) < 7:
|
||||
continue
|
||||
pid_str = parts[0]
|
||||
lstart_str = " ".join(parts[1:6])
|
||||
|
||||
try:
|
||||
pid = int(pid_str)
|
||||
start_time = datetime.strptime(lstart_str, "%a %b %d %H:%M:%S %Y")
|
||||
except Exception:
|
||||
continue
|
||||
|
||||
try:
|
||||
env_proc = subprocess.run(
|
||||
["ps", "eww", str(pid)],
|
||||
capture_output=True, text=True, check=True
|
||||
)
|
||||
env_out = env_proc.stdout
|
||||
except Exception:
|
||||
continue
|
||||
|
||||
profile = "gitea-default"
|
||||
match = re.search(r'\bGITEA_MCP_PROFILE=([^\s]+)', env_out)
|
||||
if match:
|
||||
profile = match.group(1)
|
||||
|
||||
is_stale = start_time < code_mtime
|
||||
if pid == self_pid and is_stale:
|
||||
self_stale = True
|
||||
|
||||
if profile not in running_profiles or start_time > running_profiles[profile]["start_time"]:
|
||||
running_profiles[profile] = {
|
||||
"pid": pid,
|
||||
"start_time": start_time,
|
||||
"is_stale": is_stale
|
||||
}
|
||||
|
||||
if self_stale:
|
||||
reasons.append(
|
||||
"stale-runtime: The active Gitea MCP server process is stale (running code from before changes were merged). "
|
||||
"Please fully restart the Gitea MCP server (e.g. touch /Users/jasonwalker/.gemini/config/mcp_config.json) and retry."
|
||||
)
|
||||
|
||||
if matching_profiles:
|
||||
any_running = False
|
||||
any_fresh = False
|
||||
for mp in matching_profiles:
|
||||
if mp in running_profiles:
|
||||
any_running = True
|
||||
if not running_profiles[mp]["is_stale"]:
|
||||
any_fresh = True
|
||||
break
|
||||
if not any_running:
|
||||
reasons.append(
|
||||
f"stale-runtime: None of the matching profiles for task '{task}' ({matching_profiles}) are running in the OS. "
|
||||
"Please restart the MCP server to ensure they are spawned."
|
||||
)
|
||||
elif not any_fresh:
|
||||
reasons.append(
|
||||
f"stale-runtime: All matching profiles for task '{task}' ({matching_profiles}) are running but stale. "
|
||||
"Please fully restart the Gitea MCP server and retry."
|
||||
)
|
||||
|
||||
return reasons
|
||||
|
||||
|
||||
@mcp.tool()
|
||||
def gitea_resolve_task_capability(
|
||||
task: str,
|
||||
@@ -8799,6 +9045,16 @@ def gitea_resolve_task_capability(
|
||||
configured = len(matching_profiles) > 0
|
||||
available_in_session = allowed_in_current_session
|
||||
|
||||
if "PYTEST_CURRENT_TEST" not in os.environ or "GITEA_FORCE_MCP_RUNTIME_CHECK" in os.environ:
|
||||
runtime_reasons = _check_mcp_runtimes_diagnostics(task, matching_profiles)
|
||||
if runtime_reasons:
|
||||
restart_required = True
|
||||
reason_msg = "; ".join(runtime_reasons)
|
||||
next_safe_action = (
|
||||
"stale-runtime: Gitea MCP runtime conflict or missing process detected. "
|
||||
"Please fully restart the Gitea MCP server and retry."
|
||||
)
|
||||
|
||||
if not allowed_in_current_session:
|
||||
if configured and switching:
|
||||
restart_required = True
|
||||
|
||||
Reference in New Issue
Block a user