Compare commits

..
Author SHA1 Message Date
sysadminandClaude Fable 5 c2fc2683b9 fix(recovery): address PR #703 review findings F1-F6 (#702)
Fixes the six defects from the formal REQUEST_CHANGES review at
889931d553:

- F1: run sanctioned stale-binding recovery in
  gitea_resolve_task_capability BEFORE the terminal launcher probe, so a
  worktree removed mid-session can no longer wedge mutation resolution on
  'missing cwd' before recovery runs. The fail-closed probe block now also
  carries the binding classification evidence.
- F2: _resolve_preflight_workspace_path resolves with the same
  verify_paths existence checks as the canonical mutation context, and
  _get_workspace_porcelain returns a synthetic tracked-dirty sentinel when
  the workspace is missing or git fails — an uninspectable workspace can
  never read as clean/empty porcelain.
- F3: the orphaned_expired_superseded_head cleanup matrix now requires
  affirmative safe worktree evidence (worktree_clean=true or
  worktree_exists=false), aligned with the sibling orphaned_owner_missing
  gate and the diagnosis path; unknown evidence fails closed.
- F4: reviewer-session-lease shadows and stale-binding audit records are
  recovery-critical kinds exempt from the 4h session-state TTL; they
  persist until a sanctioned clear terminally reconciles them.
- F5: session-lease shadows are keyed by lease session id (collision-safe
  identity) with a list_states enumeration API; concurrent same-profile
  sessions can no longer overwrite or misattribute each other's crash
  evidence. Legacy single-slot records remain readable.
- F6: the durable audit record is persisted (status=pending) BEFORE the
  environment binding is cleared; if audit persistence fails the clear
  does not happen and the failure is reported explicitly.

30 new regression tests cover deleted-worktree recovery ordering, missing/
deleted/symlinked/mid-evaluation path changes, unknown-evidence cleanup,
TTL boundary (before/at/after), interleaved and reconnected concurrent
sessions, and audit-write failure/retry/idempotence/ordering.

Issue #704 dotenv load-path prevention is intentionally NOT included.

Full suite: 2695 passed, 6 skipped, 161 subtests passed.

Co-Authored-By: Claude Fable 5 <[email protected]>
2026-07-13 16:40:58 -04:00
sysadminandClaude Fable 5 889931d553 feat(recovery): stale worktree binding demotion and crash-orphan lease recovery (Closes #702)
An unhandled daemon crash skips teardown: the durable reviewer lease
comment survives, the in-memory session lease dies, and the
auto-reconnected daemon inherits a stale GITEA_ACTIVE_WORKTREE from its
parent environment (PR #701 / branches/review-pr-654 incident).

AC2 — safe clear/re-bind of stale bindings:
- new stale_binding_recovery.py: classifies the active env binding
  (corroborated / unverified_inherited / provably_stale_missing_path /
  superseded_by_session_lease) and produces a fail-closed recovery plan;
  only provably stale bindings are clear-eligible
- gitea_resolve_task_capability and daemon boot run the sanctioned
  recovery (durable audit via session state); runtime context surfaces
  the classification read-only
- namespace_workspace_binding.resolve_namespace_workspace(verify_paths=…)
  demotes env-bound worktrees whose path no longer exists; mutation and
  runtime-context resolution always verify

AC3 — recoverable lease cleanup after unexpected exit:
- durable session-lease shadow (KIND_REVIEWER_SESSION_LEASE) written on
  sanctioned record/heartbeat, removed on sanctioned clear; a crash
  leaves provable orphan evidence (owner pid + session id)
- assess_crashed_session_lease_orphan derives tri-state
  owner_process_alive: only a dead recorded owner pid proves exit; PID
  liveness is never ownership proof
- diagnose/cleanup wrappers consume the shadow instead of passing
  owner_process_alive=None
- new classification orphaned_expired_superseded_head: an expired
  superseded-head lease with no terminal review stops being a permanent
  ambiguous/wait; post-expiry it unlocks fresh acquisition, and guarded
  cleanup becomes eligible only with owner-exit evidence + clean/absent
  worktree + controller authorization + confirmation. Pre-expiry
  behavior is unchanged (fail-closed wait, no steal).

Validation: tests/test_issue_702_stale_binding_lease_recovery.py (29
tests covering lost in-session leases, old-head leases, runtime/worktree
mismatch, managed reconnect, safe expiry); full suite 2665 passed,
6 skipped, 161 subtests.

Co-Authored-By: Claude Fable 5 <[email protected]>
2026-07-13 14:35:15 -04:00
13 changed files with 2097 additions and 1158 deletions
+19 -207
View File
@@ -243,192 +243,6 @@ def _redact(text):
return str(text)
# ── Classified client failures (#699) ─────────────────────────────────────────
# Subclasses of RuntimeError preserve existing ``except RuntimeError`` call
# sites. Exception *messages* are fixed constants only — HTTP response bodies,
# Keychain material, and arbitrary exception text are never stored on the
# exception or re-emitted to tool results / daemon logs.
# Fixed messages (must match mcp_tool_error_boundary.FIXED_MESSAGES keys used here).
_MSG_AUTH_INVALID = "Gitea authentication failed: invalid or revoked credentials"
_MSG_AUTH_FAILED = "Gitea authentication failed"
_MSG_AUTHZ_SCOPE = "Gitea authorization failed: insufficient token scope"
_MSG_AUTHZ_DENIED = "Gitea authorization failed: access denied"
_MSG_NETWORK = "Network error contacting Gitea"
_MSG_CONFIG = "Gitea configuration or credential resolution failed"
_MSG_UPSTREAM = "Gitea upstream unavailable"
_MSG_HTTP = "Gitea HTTP request failed"
class GiteaClientError(RuntimeError):
"""Base for known Gitea client failures with stable reason_code metadata."""
reason_code = "client_error"
error_class = "client"
http_status = None
def __init__(self, message=None, *, reason_code=None, http_status=None):
if reason_code is not None:
self.reason_code = reason_code
if http_status is not None:
self.http_status = http_status
# Message is always a fixed constant; callers cannot inject bodies.
fixed = message if message is not None else _MSG_HTTP
super().__init__(fixed)
class GiteaAuthError(GiteaClientError):
"""Authentication failure (invalid/revoked credentials → typically HTTP 401)."""
reason_code = "auth_invalid_token"
error_class = "authentication"
http_status = 401
def __init__(self, message=None, *, reason_code=None, http_status=None):
super().__init__(
message if message is not None else _MSG_AUTH_INVALID,
reason_code=reason_code or "auth_invalid_token",
http_status=http_status if http_status is not None else 401,
)
class GiteaAuthzError(GiteaClientError):
"""Authorization failure (HTTP 403 — scope deficiency or access denied)."""
reason_code = "authz_denied"
error_class = "authorization"
http_status = 403
def __init__(self, message=None, *, reason_code=None, http_status=None):
code = reason_code or "authz_denied"
if code == "authz_insufficient_scope":
fixed = _MSG_AUTHZ_SCOPE
else:
fixed = _MSG_AUTHZ_DENIED
code = "authz_denied"
super().__init__(
message if message is not None else fixed,
reason_code=code,
http_status=http_status if http_status is not None else 403,
)
class GiteaNetworkError(GiteaClientError):
"""Transport / DNS / timeout failure contacting Gitea."""
reason_code = "network_error"
error_class = "network"
http_status = None
def __init__(self, message=None, *, reason_code=None, http_status=None):
super().__init__(
message if message is not None else _MSG_NETWORK,
reason_code=reason_code or "network_error",
http_status=http_status,
)
class GiteaConfigError(GiteaClientError):
"""Local configuration / credential resolution failure (not HTTP auth)."""
reason_code = "config_error"
error_class = "configuration"
http_status = None
def __init__(self, message=None, *, reason_code=None, http_status=None):
super().__init__(
message if message is not None else _MSG_CONFIG,
reason_code=reason_code or "config_error",
http_status=http_status,
)
class GiteaHttpError(GiteaClientError):
"""Non-auth HTTP failure with fixed message (no response body)."""
reason_code = "http_error"
error_class = "client"
http_status = None
def __init__(self, message=None, *, reason_code=None, http_status=None):
code = reason_code or "http_error"
if code == "upstream_unavailable":
fixed = _MSG_UPSTREAM
else:
fixed = _MSG_HTTP
code = "http_error"
super().__init__(
message if message is not None else fixed,
reason_code=code,
http_status=http_status,
)
def _looks_like_insufficient_scope(detail: str) -> bool:
"""Internal: inspect redacted body *only* to refine 403 reason_code.
The body is never stored on the exception or returned to callers.
"""
lower = (detail or "").lower()
markers = (
"insufficient scope",
"required scope",
"does not have at least one of required scope",
"token does not have",
"missing scope",
"scope(s)",
)
return any(m in lower for m in markers)
def classify_http_status(code: int, *, body_hint: str = "") -> tuple[type, str, int]:
"""Central HTTP status → (exception_class, reason_code, http_status).
Every HTTP 403 becomes authorization-class. Body text is used only as a
local hint for scope vs denied reason_code and is never returned.
"""
if code == 401:
return (GiteaAuthError, "auth_invalid_token", 401)
if code == 403:
if _looks_like_insufficient_scope(body_hint or ""):
return (GiteaAuthzError, "authz_insufficient_scope", 403)
return (GiteaAuthzError, "authz_denied", 403)
if code in (502, 503, 504):
return (GiteaHttpError, "upstream_unavailable", code)
return (GiteaHttpError, "http_error", code)
def raise_for_http_status(code: int, body: str = "") -> None:
"""Raise a typed client error for *code* without embedding *body*.
*body* may be inspected only to choose scope vs denied for 403; it is
never placed on the exception message.
"""
# Redact before any inspection; discard after classification.
try:
hint = _redact(body or "").strip()
except Exception:
hint = ""
exc_cls, reason, status = classify_http_status(code, body_hint=hint)
# Explicitly construct without passing body/hint into message.
if exc_cls is GiteaAuthError:
raise GiteaAuthError(reason_code=reason, http_status=status)
if exc_cls is GiteaAuthzError:
raise GiteaAuthzError(reason_code=reason, http_status=status)
if reason == "upstream_unavailable":
raise GiteaHttpError(
reason_code="upstream_unavailable",
http_status=status,
)
raise GiteaHttpError(reason_code="http_error", http_status=status)
def _raise_http_error(code: int, detail: str = "") -> None:
"""Backward-compatible alias — *detail* is never embedded in the error."""
raise_for_http_status(code, detail)
def _add_query(url, **params):
"""Return *url* with the given query parameters added or overridden.
@@ -501,24 +315,23 @@ def api_request(method, url, auth_header, payload=None, *,
"""Make an authenticated JSON request to the Gitea API.
Returns parsed JSON on success (or ``None`` for an empty body), and raises
a classified client error on failure.
``RuntimeError`` on failure.
On HTTP 429 the request is retried up to *max_retries* times: honoring a
valid ``Retry-After`` header (seconds or HTTP-date) when present, otherwise
using capped jittered exponential backoff. Successful responses are
unchanged.
Failures raise typed exceptions with **fixed messages only** (#699). HTTP
response bodies are read solely for local 403 reason refinement and are
never stored on exceptions or returned to callers:
All failures are converted to a ``RuntimeError`` with a clear, secret
-redacted message (no raw stack traces or credential material):
- HTTP 401 → :class:`GiteaAuthError` (``auth_invalid_token``)
- HTTP 403 → :class:`GiteaAuthzError` (scope or denied)
- 502/503/504 → :class:`GiteaHttpError` (``upstream_unavailable``)
- Other non-429 HTTP → :class:`GiteaHttpError` (``http_error``)
- Timeouts / DNS / ``URLError`` → :class:`GiteaNetworkError`
- Malformed success JSON → plain ``RuntimeError`` (programming/protocol;
not reclassified as authentication)
- Non-429 HTTP errors surface the status code and a redacted response body.
502/503/504 upstream errors get an explicit "Gitea upstream unavailable"
message.
- Timeouts and network/DNS failures (``URLError`` / ``TimeoutError``) surface
a generic "network error contacting Gitea" message.
- A malformed (non-JSON) success body surfaces a "malformed JSON response"
message rather than a raw decode error.
The ``*_func`` parameters and ``timeout`` are injection points for
deterministic testing.
@@ -556,23 +369,22 @@ def api_request(method, url, auth_header, payload=None, *,
error_body = e.read().decode("utf-8", errors="replace")
except Exception:
error_body = ""
# Classify from status (+ local body hint). Body is not embedded.
try:
raise_for_http_status(e.code, error_body)
except GiteaClientError:
raise
# Defensive: raise_for_http_status always raises.
raise GiteaHttpError(http_status=e.code) from e # pragma: no cover
detail = _redact(error_body).strip()
if e.code in (502, 503, 504):
msg = f"HTTP {e.code}: Gitea upstream unavailable"
raise RuntimeError(f"{msg}: {detail}" if detail else msg) from e
raise RuntimeError(f"HTTP {e.code}: {detail}") from e
except (urllib.error.URLError, TimeoutError) as e:
# Fixed message only — do not embed URLError reason (may leak paths).
raise GiteaNetworkError(reason_code="network_error") from e
reason = getattr(e, "reason", e)
raise RuntimeError(
f"network error contacting Gitea: {_redact(reason)}"
) from e
if not body:
return None
try:
return json.loads(body)
except ValueError as e:
# Programming/protocol failure — not authentication.
raise RuntimeError("malformed JSON response from Gitea") from e
+205 -21
View File
@@ -195,6 +195,12 @@ RECONCILER_WORKTREE_ENV = "GITEA_RECONCILER_WORKTREE"
import namespace_workspace_binding as nwb # noqa: E402
import mcp_namespace_health # noqa: E402
import stale_binding_recovery # noqa: E402
# Worktree env bindings inherited from the parent environment at daemon boot
# (#702). A value that still equals its boot snapshot was never established by
# a sanctioned tool in this daemon's lifetime.
_BOOT_ENV_WORKTREE_BINDINGS = stale_binding_recovery.snapshot_boot_bindings()
def _preflight_in_test_mode() -> bool:
@@ -260,8 +266,124 @@ def _actual_profile_role() -> str:
)
def _assess_stale_active_binding(auto_recover: bool = False) -> dict:
"""Classify the GITEA_ACTIVE_WORKTREE binding; recover when provably stale (#702).
Runs during ``gitea_resolve_task_capability`` / runtime context. Clearing
is the sanctioned in-daemon mechanism and happens only for the two
provably-stale classes (missing path, superseded by a sanctioned session
lease); an uncorroborated boot-inherited binding is surfaced with the
managed-reconnect next action, never cleared silently.
"""
active_value = (os.environ.get(ACTIVE_WORKTREE_ENV) or "").strip() or None
try:
role = _actual_profile_role()
except Exception:
role = "author"
role_env_key = nwb.ROLE_WORKTREE_ENVS.get(role, AUTHOR_WORKTREE_ENV)
boot_value = (
(_BOOT_ENV_WORKTREE_BINDINGS.get("active_worktree") or "").strip() or None
)
classification = stale_binding_recovery.classify_active_worktree_binding(
active_value=active_value,
role_env_value=(os.environ.get(role_env_key) or "").strip() or None,
session_lease_worktree=_reviewer_session_worktree(),
boot_inherited=bool(active_value and boot_value == active_value),
path_exists=os.path.isdir(active_value) if active_value else None,
role_kind=role,
)
plan = stale_binding_recovery.plan_recovery(classification)
applied: dict = {"performed": False}
recovery_enabled = (
not _preflight_in_test_mode()
or os.environ.get("GITEA_FORCE_STALE_BINDING_RECOVERY") == "1"
)
audit_persisted: bool | None = None
if auto_recover and plan.get("clear_allowed") and recovery_enabled:
# #702 F6: the durable audit record must exist BEFORE the environment
# is cleared. If audit persistence fails the recovery does not run —
# an env clear without durable proof is an unauditable mutation.
pending_value = (os.environ.get(ACTIVE_WORKTREE_ENV) or "").strip() or None
audit_error: str | None = None
try:
import mcp_session_state as mss
mss.save_state(
kind=mss.KIND_STALE_BINDING_RECOVERY,
payload={
"cleared_env": ACTIVE_WORKTREE_ENV,
"cleared_value": pending_value,
"classification": plan.get("classification"),
"reasons": list(plan.get("reasons") or []),
"recovery_status": "pending",
},
)
audit_persisted = True
except Exception as exc:
audit_persisted = False
audit_error = f"{type(exc).__name__}: {exc}"
if audit_persisted:
applied = stale_binding_recovery.apply_recovery(plan)
if applied.get("performed"):
try:
import mcp_session_state as mss
mss.save_state(
kind=mss.KIND_STALE_BINDING_RECOVERY,
payload={
"cleared_env": applied.get("cleared_env"),
"cleared_value": applied.get("cleared_value"),
"classification": applied.get("classification"),
"reasons": applied.get("reasons"),
"recovery_status": "performed",
},
)
except Exception:
# The pre-clear "pending" record is already durable; the
# status refresh is best-effort and never rolls back.
pass
else:
applied = {
"performed": False,
"action": stale_binding_recovery.RECOVERY_ACTION_NONE,
"audit_write_failed": True,
"reasons": [
"audit persistence failed; stale binding NOT cleared "
"(fail closed, #702 F6)"
]
+ ([audit_error] if audit_error else []),
}
report = {
"classification": classification.get("classification"),
"active_worktree": classification.get("active_worktree"),
"boot_inherited": classification.get("boot_inherited", False),
"clear_eligible": bool(classification.get("clear_eligible")),
"recovery_action": plan.get("action"),
"recovery_performed": bool(applied.get("performed")),
"reasons": classification.get("reasons") or [],
}
if audit_persisted is not None:
report["audit_persisted"] = audit_persisted
if applied.get("audit_write_failed"):
report["audit_write_failed"] = True
report["reasons"] = list(report["reasons"]) + list(
applied.get("reasons") or []
)
if applied.get("performed"):
report["cleared_value"] = applied.get("cleared_value")
if plan.get("exact_next_action"):
report["exact_next_action"] = plan.get("exact_next_action")
return report
def _resolve_preflight_workspace_path(worktree_path: str | None = None) -> str:
"""Resolve the namespace-scoped workspace root inspected by pre-flight guards."""
"""Resolve the namespace-scoped workspace root inspected by pre-flight guards.
#702 F2: preflight resolves with the same existence verification as the
canonical mutation context (verify_paths). A dead env binding must demote
here exactly as it does in :func:`_resolve_namespace_mutation_context`,
or preflight would inspect a path the mutation guard never uses.
"""
role = _effective_workspace_role()
workspace, _source = nwb.resolve_namespace_workspace(
role_kind=role,
@@ -271,6 +393,7 @@ def _resolve_preflight_workspace_path(worktree_path: str | None = None) -> str:
_reviewer_session_worktree() if role in {"reviewer", "merger"} else None
),
profile_name=get_profile().get("profile_name"),
verify_paths=True,
)
return workspace
@@ -317,6 +440,15 @@ def _get_git_root(path: str) -> str | None:
return (res.stdout or "").strip() or None
# #702 F2: synthetic tracked-dirty porcelain returned when the preflight
# workspace cannot be inspected (missing path, git failure). Shaped as a
# modified tracked entry so every consumer treats "workspace unavailable"
# as dirty — an uninspectable workspace must never read as clean.
PREFLIGHT_WORKSPACE_UNAVAILABLE_PORCELAIN = (
" M __gitea_preflight_workspace_unavailable__\n"
)
def _get_workspace_porcelain(worktree_path: str | None = None) -> str:
"""Return tracked-workspace porcelain for pre-flight attribution."""
if os.environ.get("GITEA_TEST_FORCE_DIRTY"):
@@ -325,6 +457,8 @@ def _get_workspace_porcelain(worktree_path: str | None = None) -> str:
if override is not None:
return override
workspace = _resolve_preflight_workspace_path(worktree_path)
if not os.path.isdir(workspace):
return PREFLIGHT_WORKSPACE_UNAVAILABLE_PORCELAIN
try:
res = subprocess.run(
["git", "status", "--porcelain"],
@@ -332,9 +466,11 @@ def _get_workspace_porcelain(worktree_path: str | None = None) -> str:
text=True,
cwd=workspace,
)
return res.stdout or ""
except Exception:
return ""
return PREFLIGHT_WORKSPACE_UNAVAILABLE_PORCELAIN
if res.returncode != 0:
return PREFLIGHT_WORKSPACE_UNAVAILABLE_PORCELAIN
return res.stdout or ""
def _parse_porcelain_entries(porcelain: str) -> dict[str, str]:
@@ -1084,9 +1220,7 @@ from gitea_auth import ( # noqa: E402
repo_api_url,
get_profile,
gitea_url,
GiteaConfigError,
)
import mcp_tool_error_boundary # noqa: E402
import gitea_audit # noqa: E402
import gitea_config # noqa: E402
import capability_stop_terminal # noqa: E402
@@ -1466,12 +1600,6 @@ def _with_optional_url(result: dict, url: str | None) -> dict:
result["url"] = url
return result
# #699: known auth/authz/network/config failures → structured CallToolResult
# isError; stdio transport must survive (no unhandled raise / process exit).
from mcp.server.fastmcp.tools.base import Tool as _FastMCPTool # noqa: E402
mcp_tool_error_boundary.install_tool_run_boundary(_FastMCPTool)
mcp = FastMCP("gitea-tools", instructions=(
"Gitea issue tracker and PR management for dadeschools and prgs instances. "
"Use the gitea_ prefixed tools to create issues, PRs, list issues, etc."
@@ -1809,16 +1937,13 @@ def _enforce_remote_repo_guard(
def _auth(host: str) -> str:
"""Get auth header, raise if unavailable.
Missing credentials are a configuration failure, not a silent internal
crash. Typed as :class:`gitea_auth.GiteaConfigError` so the tool-error
boundary (#699) maps them to a structured isError result without EOF.
The exception message is a fixed constant (no host/token material).
"""
"""Get auth header, raise if unavailable."""
header = get_auth_header(host)
if header is None:
raise GiteaConfigError(reason_code="config_error")
raise RuntimeError(
f"No credentials for {host}. "
"Ensure you've logged in via HTTPS at least once."
)
return header
@@ -8063,6 +8188,16 @@ def gitea_diagnose_reviewer_pr_lease_handoff(
except Exception:
worktree_clean = None
# #702: derive owner-process evidence from the durable session-lease
# shadow left behind by a crashed daemon. Only a provably dead recorded
# owner pid yields False; PID liveness alone never proves ownership.
orphan_evidence = reviewer_pr_lease.assess_crashed_session_lease_orphan(
reviewer_pr_lease.load_session_lease_shadow(
session_id=(active or {}).get("session_id")
),
lease=active,
)
diagnosis = reviewer_pr_lease.diagnose_reviewer_pr_lease_handoff(
comments,
pr_number=pr_number,
@@ -8076,8 +8211,9 @@ def gitea_diagnose_reviewer_pr_lease_handoff(
formal_reviews=formal_reviews,
worktree_exists=worktree_exists,
worktree_clean=worktree_clean,
owner_process_alive=None, # PID never proves ownership; leave unset
owner_process_alive=orphan_evidence.get("owner_process_alive"),
)
diagnosis["crashed_session_lease_evidence"] = orphan_evidence
diagnosis["success"] = True
diagnosis["remote"] = remote
diagnosis["authenticated_user"] = username
@@ -8307,6 +8443,15 @@ def gitea_cleanup_obsolete_reviewer_comment_lease(
worktree_clean = None
session = reviewer_pr_lease.get_session_lease() or {}
# #702: when the caller supplies no owner evidence, consult the durable
# session-lease shadow; an explicitly passed value always wins.
if owner_process_alive is None:
owner_process_alive = reviewer_pr_lease.assess_crashed_session_lease_orphan(
reviewer_pr_lease.load_session_lease_shadow(
session_id=(lease or {}).get("session_id")
),
lease=lease,
).get("owner_process_alive")
assessment = reviewer_pr_lease.assess_obsolete_reviewer_comment_lease_cleanup(
comments,
pr_number=pr_number,
@@ -10156,6 +10301,17 @@ def gitea_get_runtime_context(
PROJECT_ROOT),
}
# #702: read-only visibility into the inherited GITEA_ACTIVE_WORKTREE
# binding; recovery itself runs during capability resolution.
try:
stale_binding = _assess_stale_active_binding(auto_recover=False)
if stale_binding.get("classification") != (
stale_binding_recovery.CLASSIFICATION_UNBOUND
):
result["stale_binding_recovery"] = stale_binding
except Exception:
pass
parity = _current_master_parity()
result["master_parity"] = {
"in_parity": parity["in_parity"],
@@ -11589,6 +11745,18 @@ def gitea_resolve_task_capability(
"exact_safe_next_action": next_safe_action,
}
# #702 F1: validate (and, when provably stale, recover) the inherited
# GITEA_ACTIVE_WORKTREE binding BEFORE the terminal launcher probe. The
# probe uses that binding as its cwd; a worktree removed mid-session
# would otherwise wedge every mutation-task resolution on 'missing cwd'
# before the sanctioned recovery could ever run. Recovery stays
# fail-closed: only the provably-stale classes clear, with the durable
# audit record persisted first (F6).
try:
stale_binding = _assess_stale_active_binding(auto_recover=True)
except Exception:
stale_binding = None
# ── Terminal Launcher Preflight Check (#556) ──
if task in native_mcp_preference.GITEA_MUTATION_TASKS:
in_test = _preflight_in_test_mode()
@@ -11612,7 +11780,7 @@ def gitea_resolve_task_capability(
"Do not attempt git/pytest finalization or unsafe fallbacks. "
"Run gitea_diagnose_terminal, emit blocked-diagnose-report, and stop."
)
return {
blocked_result = {
"requested_task": task,
"required_operation_permission": required_permission,
"required_role_kind": required_role,
@@ -11632,6 +11800,11 @@ def gitea_resolve_task_capability(
"different_mcp_namespace_required": False,
"exact_safe_next_action": next_safe_action,
}
if stale_binding is not None and stale_binding.get(
"classification"
) != stale_binding_recovery.CLASSIFICATION_UNBOUND:
blocked_result["stale_binding_recovery"] = stale_binding
return blocked_result
record_preflight_check("capability", required_role, resolved_task=task)
@@ -11821,6 +11994,10 @@ def gitea_resolve_task_capability(
"different_mcp_namespace_required": different_namespace_required,
"exact_safe_next_action": next_safe_action,
}
if stale_binding is not None and stale_binding.get("classification") != (
stale_binding_recovery.CLASSIFICATION_UNBOUND
):
result["stale_binding_recovery"] = stale_binding
if reason_msg:
result["reason"] = reason_msg
if task in ("review_pr", "merge_pr"):
@@ -12696,4 +12873,11 @@ if __name__ == "__main__":
# processes (e.g. review_pr.py) can detect and refuse profile
# side-channel overrides (#199).
_export_session_profile_lock()
# #702: an auto-reconnected daemon inherits GITEA_ACTIVE_WORKTREE from its
# parent environment. Validate it at boot and clear it only when provably
# stale (missing path); anything less certain is surfaced, not cleared.
try:
_assess_stale_active_binding(auto_recover=True)
except Exception:
pass
mcp.run(transport="stdio")
+96 -10
View File
@@ -36,6 +36,25 @@ KIND_REVIEW_DRAFT = "review_draft"
# other session proofs; a contaminated session fails closed on gated mutations
# until a reconciler audits and clears it.
KIND_STABLE_BRANCH_CONTAMINATION = "stable_branch_contamination"
# Durable shadow of the in-memory reviewer session lease (#702). Written on
# every sanctioned record/heartbeat and removed on sanctioned clear, so a
# daemon that dies without teardown leaves provable orphan evidence (owner
# pid + session id) for the guarded lease-cleanup path. Never a substitute
# for the in-session lease: mutation gates ignore it.
KIND_REVIEWER_SESSION_LEASE = "reviewer_session_lease"
# Durable audit record written whenever the daemon's sanctioned stale-binding
# recovery clears an inherited GITEA_ACTIVE_WORKTREE binding (#702).
KIND_STALE_BINDING_RECOVERY = "stale_binding_recovery"
# Kinds that carry recovery-critical crash-orphan evidence (#702). They are
# exempt from TTL expiry: a crashed daemon can never refresh its own record,
# so a wall-clock TTL would silently destroy the only owner-exit proof the
# guarded cleanup path accepts. These records live until a sanctioned clear
# terminally reconciles them (durable lifecycle), never until a timer fires.
RECOVERY_CRITICAL_KINDS = frozenset({
KIND_REVIEWER_SESSION_LEASE,
KIND_STALE_BINDING_RECOVERY,
})
@@ -86,6 +105,7 @@ def state_key(
org: str | None = None,
repo: str | None = None,
profile_identity: str | None = None,
instance_id: str | None = None,
) -> str:
"""Build durable filename key.
@@ -93,17 +113,20 @@ def state_key(
so the primary key is kind + profile identity. Remote/org/repo are stored
inside the payload and validated on load (#559), which lets a later daemon
process recover state without already knowing the remote argument.
*instance_id* extends the key for kinds where one-per-profile collides —
concurrent same-profile sessions each own their reviewer-lease shadow
(#702 F5), keyed by their lease session id so a later heartbeat can never
overwrite or misattribute another session's crash evidence.
"""
# Keep remote/org/repo parameters for API stability / future kinds; they are
# intentionally not part of the filename for session-scoped proofs.
_ = (remote, org, repo)
return "-".join(
_sanitize_segment(part)
for part in (
kind,
profile_identity or "unknown-profile",
)
)
parts = [kind, profile_identity or "unknown-profile"]
instance = (instance_id or "").strip()
if instance:
parts.append(instance)
return "-".join(_sanitize_segment(part) for part in parts)
def state_file_path(
@@ -114,6 +137,7 @@ def state_file_path(
repo: str | None = None,
profile_identity: str | None = None,
state_dir: str | None = None,
instance_id: str | None = None,
) -> str:
root = (state_dir or default_state_dir()).strip()
name = state_key(
@@ -122,6 +146,7 @@ def state_file_path(
org=org,
repo=repo,
profile_identity=profile_identity,
instance_id=instance_id,
)
return os.path.join(root, f"{name}.json")
@@ -242,10 +267,12 @@ def identity_match_reasons(
reasons.append("session state missing recorded_at timestamp (fail closed)")
else:
age = _now_utc() - recorded_at
record_kind = (str(record.get("kind") or "")).strip()
if age > timedelta(hours=ttl_hours()):
reasons.append(
f"session state expired after {ttl_hours():g}h (fail closed)"
)
if record_kind not in RECOVERY_CRITICAL_KINDS:
reasons.append(
f"session state expired after {ttl_hours():g}h (fail closed)"
)
if age < timedelta(0):
reasons.append("session state recorded_at is in the future (fail closed)")
return reasons
@@ -259,6 +286,7 @@ def load_state(
repo: str | None = None,
profile_identity: str | None = None,
state_dir: str | None = None,
instance_id: str | None = None,
) -> dict[str, Any] | None:
"""Load durable state payload when identity checks pass."""
profile = current_profile_identity(profile_identity=profile_identity)
@@ -269,6 +297,7 @@ def load_state(
repo=repo,
profile_identity=profile,
state_dir=state_dir,
instance_id=instance_id,
)
lock_path = f"{path}.lock"
with _exclusive_file_lock(lock_path):
@@ -314,6 +343,7 @@ def save_state(
repo: str | None = None,
profile_identity: str | None = None,
state_dir: str | None = None,
instance_id: str | None = None,
) -> dict[str, Any] | None:
"""Persist or clear durable state for the given session identity key."""
profile = current_profile_identity(
@@ -335,6 +365,7 @@ def save_state(
repo=key_repo,
profile_identity=profile,
state_dir=root,
instance_id=instance_id,
)
lock_path = f"{path}.lock"
with _exclusive_file_lock(lock_path):
@@ -356,6 +387,8 @@ def save_state(
body["session_profile_lock"] = profile
body["recorded_at"] = body.get("recorded_at") or now
body["updated_at"] = now
if (instance_id or "").strip():
body["instance_id"] = instance_id.strip()
if key_remote is not None:
body["remote"] = key_remote
if key_org is not None:
@@ -387,6 +420,7 @@ def clear_state(
repo: str | None = None,
profile_identity: str | None = None,
state_dir: str | None = None,
instance_id: str | None = None,
) -> None:
save_state(
kind=kind,
@@ -396,4 +430,56 @@ def clear_state(
repo=repo,
profile_identity=profile_identity,
state_dir=state_dir,
instance_id=instance_id,
)
def list_states(
*,
kind: str,
profile_identity: str | None = None,
state_dir: str | None = None,
) -> list[dict[str, Any]]:
"""Enumerate durable records of *kind* across all instance keys.
Crash recovery cannot know which session id left a shadow behind, so it
must be able to enumerate every record of a kind (#702 F5). Identity
checks (profile / TTL policy) apply per record exactly as in
:func:`load_state`; records that fail closed are omitted.
"""
root = (state_dir or default_state_dir()).strip()
if not root or not os.path.isdir(root):
return []
profile = current_profile_identity(profile_identity=profile_identity)
results: list[dict[str, Any]] = []
try:
names = sorted(os.listdir(root))
except OSError:
return []
for name in names:
if not name.endswith(".json") or name.startswith("."):
continue
envelope = _read_json(os.path.join(root, name))
if not envelope or envelope.get("kind") != kind:
continue
payload = envelope.get("payload")
if not isinstance(payload, dict):
continue
merged = dict(payload)
for key in (
"kind",
"remote",
"org",
"repo",
"profile_identity",
"session_profile_lock",
"recorded_at",
"updated_at",
"writer_pid",
):
if key in envelope and key not in merged:
merged[key] = envelope[key]
if identity_match_reasons(merged, profile_identity=profile):
continue
results.append(merged)
return results
-451
View File
@@ -1,451 +0,0 @@
"""MCP tool-boundary error mapping for known Gitea client failures (#699).
Known authentication / authorization / network / configuration failures leave
the tool boundary as a sanitized structured ``CallToolResult`` with
``isError=True``. Stdio transport remains connected.
Design constraints (reviewer-ratified, #699 / PR #701):
1. **No secret material** in tool results or daemon logs. Messages are fixed
constants keyed by ``reason_code``; HTTP bodies / exception text never
surface. Sanitization failure fails closed to ``internal_error``.
2. **Narrow boundary** wraps the original FastMCP ``Tool.run`` success path;
re-raises framework control-flow exceptions (``UrlElicitationRequiredError``);
maps only typed client failures. Installation is idempotent.
3. **No RuntimeError substring heuristics.** Only explicit typed
authentication / authorization / network / configuration exceptions
receive those labels.
4. **HTTP classification** lives in ``gitea_auth.classify_http_status``;
every 403 is authorization-class.
"""
from __future__ import annotations
import json
import logging
import sys
from typing import Any
logger = logging.getLogger("gitea_mcp.tool_error_boundary")
# Stable reason codes (#699).
REASON_AUTH_FAILED = "auth_failed"
REASON_AUTH_INVALID_TOKEN = "auth_invalid_token"
REASON_AUTHZ_INSUFFICIENT_SCOPE = "authz_insufficient_scope"
REASON_AUTHZ_DENIED = "authz_denied"
REASON_NETWORK_ERROR = "network_error"
REASON_CONFIG_ERROR = "config_error"
REASON_INTERNAL_ERROR = "internal_error"
REASON_UPSTREAM_UNAVAILABLE = "upstream_unavailable"
REASON_HTTP_ERROR = "http_error"
ERROR_CLASS_AUTHENTICATION = "authentication"
ERROR_CLASS_AUTHORIZATION = "authorization"
ERROR_CLASS_NETWORK = "network"
ERROR_CLASS_CONFIGURATION = "configuration"
ERROR_CLASS_INTERNAL = "internal"
ERROR_CLASS_UPSTREAM = "upstream"
# Fixed, secret-free operator messages. Never interpolate HTTP bodies,
# Keychain contents, tokens, or arbitrary exception text.
FIXED_MESSAGES: dict[str, str] = {
REASON_AUTH_FAILED: "Gitea authentication failed",
REASON_AUTH_INVALID_TOKEN: (
"Gitea authentication failed: invalid or revoked credentials"
),
REASON_AUTHZ_INSUFFICIENT_SCOPE: (
"Gitea authorization failed: insufficient token scope"
),
REASON_AUTHZ_DENIED: "Gitea authorization failed: access denied",
REASON_NETWORK_ERROR: "Network error contacting Gitea",
REASON_CONFIG_ERROR: "Gitea configuration or credential resolution failed",
REASON_INTERNAL_ERROR: "Internal tool error",
REASON_UPSTREAM_UNAVAILABLE: "Gitea upstream unavailable",
REASON_HTTP_ERROR: "Gitea HTTP request failed",
}
_INSTALL_FLAG = "_gitea_auth_boundary_installed"
_ORIGINAL_ATTR = "_gitea_auth_boundary_original"
def fixed_message(reason_code: str) -> str:
"""Return the fixed sanitized message for *reason_code* (fail closed)."""
return FIXED_MESSAGES.get(reason_code, FIXED_MESSAGES[REASON_INTERNAL_ERROR])
def _safe_profile_name() -> str | None:
try:
from gitea_auth import get_profile
name = (get_profile() or {}).get("profile_name")
if name is None:
return None
text = str(name).strip()
# Profile names are non-secret identifiers; still bound length.
return text[:80] if text else None
except Exception:
return None
def _is_framework_control_flow(exc: BaseException) -> bool:
"""True for exceptions the MCP framework must re-raise unchanged."""
try:
from mcp.shared.exceptions import UrlElicitationRequiredError
if isinstance(exc, UrlElicitationRequiredError):
return True
except Exception:
pass
# BaseException subclasses that must never become tool isError payloads.
if isinstance(exc, (KeyboardInterrupt, SystemExit, GeneratorExit)):
return True
return False
def is_known_client_failure(exc: BaseException) -> bool:
"""True only for explicit typed Gitea client / config failures.
Never true for arbitrary ``RuntimeError`` (reviewer finding #3).
"""
try:
import gitea_auth
if isinstance(
exc,
(
gitea_auth.GiteaAuthError,
gitea_auth.GiteaAuthzError,
gitea_auth.GiteaNetworkError,
gitea_auth.GiteaConfigError,
gitea_auth.GiteaHttpError,
),
):
return True
except Exception:
return False
try:
import gitea_config
if isinstance(exc, gitea_config.ConfigError):
return True
except Exception:
pass
return False
def classify_exception(exc: BaseException) -> dict[str, Any]:
"""Return structured classification with **fixed** messages only.
Only typed authentication / authorization / network / configuration
failures receive those labels. Unexpected programming failures are
``internal_error``. HTTP bodies and exception text are never copied
into ``message``.
"""
try:
return _classify_exception_impl(exc)
except Exception:
# Fail closed: sanitization / classification failure must not leak.
return {
"reason_code": REASON_INTERNAL_ERROR,
"error_class": ERROR_CLASS_INTERNAL,
"http_status": None,
"message": fixed_message(REASON_INTERNAL_ERROR),
"transport_survives": True,
}
def _classify_exception_impl(exc: BaseException) -> dict[str, Any]:
import gitea_auth
if isinstance(exc, gitea_auth.GiteaAuthError):
code = getattr(exc, "reason_code", None) or REASON_AUTH_INVALID_TOKEN
if code not in (
REASON_AUTH_FAILED,
REASON_AUTH_INVALID_TOKEN,
):
code = REASON_AUTH_INVALID_TOKEN
return {
"reason_code": code,
"error_class": ERROR_CLASS_AUTHENTICATION,
"http_status": getattr(exc, "http_status", None) or 401,
"message": fixed_message(code),
"transport_survives": True,
}
if isinstance(exc, gitea_auth.GiteaAuthzError):
code = getattr(exc, "reason_code", None) or REASON_AUTHZ_DENIED
if code not in (REASON_AUTHZ_DENIED, REASON_AUTHZ_INSUFFICIENT_SCOPE):
code = REASON_AUTHZ_DENIED
return {
"reason_code": code,
"error_class": ERROR_CLASS_AUTHORIZATION,
"http_status": getattr(exc, "http_status", None) or 403,
"message": fixed_message(code),
"transport_survives": True,
}
if isinstance(exc, gitea_auth.GiteaNetworkError):
return {
"reason_code": REASON_NETWORK_ERROR,
"error_class": ERROR_CLASS_NETWORK,
"http_status": getattr(exc, "http_status", None),
"message": fixed_message(REASON_NETWORK_ERROR),
"transport_survives": True,
}
if isinstance(exc, gitea_auth.GiteaConfigError):
return {
"reason_code": REASON_CONFIG_ERROR,
"error_class": ERROR_CLASS_CONFIGURATION,
"http_status": getattr(exc, "http_status", None),
"message": fixed_message(REASON_CONFIG_ERROR),
"transport_survives": True,
}
if isinstance(exc, gitea_auth.GiteaHttpError):
code = getattr(exc, "reason_code", None) or REASON_HTTP_ERROR
if code == REASON_UPSTREAM_UNAVAILABLE:
error_class = ERROR_CLASS_UPSTREAM
else:
error_class = ERROR_CLASS_INTERNAL
code = REASON_HTTP_ERROR
return {
"reason_code": code,
"error_class": error_class,
"http_status": getattr(exc, "http_status", None),
"message": fixed_message(code),
"transport_survives": True,
}
try:
import gitea_config
if isinstance(exc, gitea_config.ConfigError):
return {
"reason_code": REASON_CONFIG_ERROR,
"error_class": ERROR_CLASS_CONFIGURATION,
"http_status": None,
"message": fixed_message(REASON_CONFIG_ERROR),
"transport_survives": True,
}
except Exception:
pass
# Unwrap FastMCP ToolError cause when the original was a typed failure.
cause = getattr(exc, "__cause__", None)
if cause is not None and cause is not exc and is_known_client_failure(cause):
return _classify_exception_impl(cause)
# No message-substring authentication heuristics (reviewer finding #3).
return {
"reason_code": REASON_INTERNAL_ERROR,
"error_class": ERROR_CLASS_INTERNAL,
"http_status": None,
"message": fixed_message(REASON_INTERNAL_ERROR),
"transport_survives": True,
}
def build_structured_error_payload(
classification: dict[str, Any],
*,
tool_name: str | None = None,
profile_name: str | None = None,
) -> dict[str, Any]:
"""LLM-safe structured payload — fixed message + typed metadata only."""
try:
reason = str(classification.get("reason_code") or REASON_INTERNAL_ERROR)
message = fixed_message(reason)
# Refuse to emit any classification message that is not the fixed constant.
if classification.get("message") != message:
message = fixed_message(reason)
payload: dict[str, Any] = {
"success": False,
"isError": True,
"reason_code": reason if reason in FIXED_MESSAGES else REASON_INTERNAL_ERROR,
"error_class": classification.get("error_class") or ERROR_CLASS_INTERNAL,
"message": message,
"transport_survives": True,
"retryable": classification.get("error_class")
in {
ERROR_CLASS_AUTHENTICATION,
ERROR_CLASS_NETWORK,
ERROR_CLASS_CONFIGURATION,
},
}
status = classification.get("http_status")
if isinstance(status, int):
payload["http_status"] = status
if tool_name and isinstance(tool_name, str):
# Tool names are identifiers, not secrets; bound length.
payload["tool"] = tool_name[:120]
if profile_name and isinstance(profile_name, str):
payload["profile"] = profile_name[:80]
return payload
except Exception:
return {
"success": False,
"isError": True,
"reason_code": REASON_INTERNAL_ERROR,
"error_class": ERROR_CLASS_INTERNAL,
"message": fixed_message(REASON_INTERNAL_ERROR),
"transport_survives": True,
"retryable": False,
}
def log_sanitized_daemon_reason(
classification: dict[str, Any],
*,
tool_name: str | None = None,
stream=None,
) -> None:
"""Daemon log: reason codes only — never exception text or response bodies."""
stream = stream if stream is not None else sys.stderr
try:
reason = classification.get("reason_code") or REASON_INTERNAL_ERROR
error_class = classification.get("error_class") or ERROR_CLASS_INTERNAL
# Only emit known tokens; never classification['message'] from callers
# that might have been poisoned.
if reason not in FIXED_MESSAGES:
reason = REASON_INTERNAL_ERROR
error_class = ERROR_CLASS_INTERNAL
parts = [
"mcp_tool_error",
f"reason_code={reason}",
f"error_class={error_class}",
]
if tool_name and isinstance(tool_name, str):
parts.append(f"tool={tool_name[:120]}")
status = classification.get("http_status")
if isinstance(status, int):
parts.append(f"http_status={status}")
# Intentionally no detail= / message= field — secrets lived there.
line = " ".join(parts)
stream.write(line + "\n")
if hasattr(stream, "flush"):
stream.flush()
logger.warning(line)
except Exception:
# Fail closed: never fall back to logging the exception.
try:
stream.write(
"mcp_tool_error reason_code=internal_error "
"error_class=internal\n"
)
if hasattr(stream, "flush"):
stream.flush()
except Exception:
pass
def to_call_tool_result(
exc: BaseException,
*,
tool_name: str | None = None,
profile_name: str | None = None,
log: bool = True,
) -> Any:
"""Build a FastMCP ``CallToolResult`` with ``isError=True`` for *exc*."""
from mcp.types import CallToolResult, TextContent
try:
classification = classify_exception(exc)
if log:
log_sanitized_daemon_reason(classification, tool_name=tool_name)
payload = build_structured_error_payload(
classification, tool_name=tool_name, profile_name=profile_name
)
text = json.dumps(payload, indent=2, sort_keys=True)
return CallToolResult(
content=[TextContent(type="text", text=text)],
structuredContent=payload,
isError=True,
)
except Exception:
# Absolute fail-closed path — no exception text.
fallback = {
"success": False,
"isError": True,
"reason_code": REASON_INTERNAL_ERROR,
"error_class": ERROR_CLASS_INTERNAL,
"message": fixed_message(REASON_INTERNAL_ERROR),
"transport_survives": True,
"retryable": False,
}
return CallToolResult(
content=[
TextContent(
type="text",
text=json.dumps(fallback, indent=2, sort_keys=True),
)
],
structuredContent=fallback,
isError=True,
)
def install_tool_run_boundary(Tool) -> bool:
"""Install a **narrow** error boundary around FastMCP ``Tool.run``.
Strategy (preserves framework semantics):
* Call the **original** ``Tool.run`` for the success path (async, return
types, convert_result, protocol behavior unchanged).
* Re-raise ``UrlElicitationRequiredError`` and other control-flow
exceptions without mapping to ``internal_error``.
* Map typed Gitea client failures (and ToolError whose ``__cause__`` is
typed) to structured ``CallToolResult(isError=True)``.
* Map remaining unexpected tool failures to fixed ``internal_error``
isError results (transport survival) without secret-bearing text.
* Idempotent: second install is a no-op and returns ``False``.
"""
if getattr(Tool.run, _INSTALL_FLAG, False):
return False
from mcp.server.fastmcp.exceptions import ToolError
from mcp.shared.exceptions import UrlElicitationRequiredError
original_run = Tool.run
async def run_boundary(
self,
arguments: dict[str, Any],
context=None,
convert_result: bool = False,
) -> Any:
try:
return await original_run(
self,
arguments,
context=context,
convert_result=convert_result,
)
except UrlElicitationRequiredError:
# Framework control-flow — must not become internal_error.
raise
except BaseException as exc:
if _is_framework_control_flow(exc):
raise
if not isinstance(exc, Exception):
raise
# Prefer typed cause under FastMCP ToolError wrappers.
target: BaseException = exc
if isinstance(exc, ToolError) and exc.__cause__ is not None:
target = exc.__cause__
# Known client failures → structured isError with reason codes.
# Unexpected failures → fixed internal_error isError (no secrets).
profile_name = _safe_profile_name()
return to_call_tool_result(
target,
tool_name=getattr(self, "name", None),
profile_name=profile_name,
)
setattr(run_boundary, _INSTALL_FLAG, True)
setattr(run_boundary, _ORIGINAL_ATTR, original_run)
Tool.run = run_boundary # type: ignore[method-assign]
return True
def boundary_is_installed(Tool) -> bool:
"""True when the #699 boundary is active on *Tool.run*."""
return bool(getattr(Tool.run, _INSTALL_FLAG, False))
+36 -10
View File
@@ -56,23 +56,46 @@ def resolve_namespace_workspace(
env: dict[str, str] | os._Environ | None = None,
session_lease_worktree: str | None = None,
profile_name: str | None = None,
demotions: list[str] | None = None,
verify_paths: bool = False,
) -> tuple[str, str]:
"""Return ``(resolved_path, binding_source)`` for *role_kind*."""
"""Return ``(resolved_path, binding_source)`` for *role_kind*.
With *verify_paths*, env-sourced candidates whose path no longer exists
are demoted (#702): a binding to a deleted worktree can never name a
valid task workspace, so resolution falls through to the next candidate.
Explicit arguments are never demoted — a caller-declared path must fail
loudly downstream rather than silently rebind. Demotion notes are
appended to *demotions* when provided. Runtime-context and mutation
guards resolve through :func:`resolve_namespace_mutation_context`, which
always verifies.
"""
env_map = env if env is not None else os.environ
role = normalize_role_kind(role_kind, profile_name=profile_name)
role_env_key = ROLE_WORKTREE_ENVS[role]
for candidate, source in (
(worktree_path, "worktree_path argument"),
(worktree, "worktree argument"),
(_env_value(env_map, ACTIVE_WORKTREE_ENV), f"{ACTIVE_WORKTREE_ENV} environment variable"),
(_env_value(env_map, role_env_key), f"{role_env_key} environment variable"),
for candidate, source, env_sourced in (
(worktree_path, "worktree_path argument", False),
(worktree, "worktree argument", False),
(_env_value(env_map, ACTIVE_WORKTREE_ENV),
f"{ACTIVE_WORKTREE_ENV} environment variable", True),
(_env_value(env_map, role_env_key),
f"{role_env_key} environment variable", True),
(session_lease_worktree if role in {"reviewer", "merger"} else None,
"reviewer PR lease worktree"),
"reviewer PR lease worktree", False),
):
text = (candidate or "").strip()
if text:
return os.path.realpath(os.path.abspath(text)), source
if not text:
continue
real = os.path.realpath(os.path.abspath(text))
if verify_paths and env_sourced and not os.path.isdir(real):
if demotions is not None:
demotions.append(
f"{source} '{real}' demoted: path no longer exists "
"(stale binding, #702)"
)
continue
return real, source
return os.path.realpath(process_project_root), "MCP server process root (default)"
@@ -88,6 +111,7 @@ def resolve_namespace_mutation_context(
profile_name: str | None = None,
) -> dict:
"""Shared workspace resolution for runtime_context and mutation guards."""
demotions: list[str] = []
workspace, binding_source = resolve_namespace_workspace(
role_kind=role_kind,
worktree_path=worktree_path,
@@ -96,6 +120,8 @@ def resolve_namespace_mutation_context(
env=env,
session_lease_worktree=session_lease_worktree,
profile_name=profile_name,
demotions=demotions,
verify_paths=True,
)
process_root = os.path.realpath(process_project_root)
role = normalize_role_kind(role_kind, profile_name=profile_name)
@@ -111,7 +137,7 @@ def resolve_namespace_mutation_context(
"workspace_path": workspace,
"workspace_binding_source": binding_source,
"workspace_role_kind": role,
"ignored_bindings": pollution.get("ignored_bindings") or [],
"ignored_bindings": demotions + (pollution.get("ignored_bindings") or []),
"process_project_root": process_root,
"canonical_repo_root": canonical_root,
"roots_aligned": canonical_root == process_root,
+235
View File
@@ -407,18 +407,194 @@ def record_session_lease(
if lease_provenance:
stored["lease_provenance"] = dict(lease_provenance)
_SESSION_LEASE = stored
_persist_session_lease_shadow(stored)
return dict(_SESSION_LEASE)
def clear_session_lease() -> None:
global _SESSION_LEASE
prior = _SESSION_LEASE
_SESSION_LEASE = None
_persist_session_lease_shadow(None, prior=prior)
def get_session_lease() -> dict[str, Any] | None:
return dict(_SESSION_LEASE) if _SESSION_LEASE else None
def _persist_session_lease_shadow(
lease: dict[str, Any] | None,
*,
prior: dict[str, Any] | None = None,
) -> None:
"""Best-effort durable shadow of the in-session lease (#702).
A daemon that dies without teardown leaves this record behind, giving a
later process provable orphan evidence (owner pid + session id) for the
guarded cleanup path. Observability only — mutation gates never read it,
and failures here must never block lease operations.
Shadows are keyed by lease session id (#702 F5): concurrent same-profile
sessions each own a distinct record, so one session's heartbeat can never
overwrite or misattribute another's crash evidence. A sanctioned clear is
the terminal reconciliation of that record's lifecycle.
"""
try:
import mcp_session_state as mss
if lease is None:
sid = ((prior or {}).get("session_id") or "").strip() or None
if sid:
mss.clear_state(
kind=mss.KIND_REVIEWER_SESSION_LEASE, instance_id=sid
)
# Legacy single-slot record from pre-F5 code: clearing it is safe
# because collision-safe writes never target that key again.
mss.clear_state(kind=mss.KIND_REVIEWER_SESSION_LEASE)
return
mss.save_state(
kind=mss.KIND_REVIEWER_SESSION_LEASE,
instance_id=((lease.get("session_id") or "").strip() or None),
payload={
"pr_number": lease.get("pr_number"),
"session_id": lease.get("session_id"),
"comment_id": lease.get("comment_id"),
"candidate_head": lease.get("candidate_head"),
"worktree": lease.get("worktree"),
"phase": lease.get("phase"),
"reviewer_identity": lease.get("reviewer_identity"),
"profile": lease.get("profile"),
"lease_repo": lease.get("repo"),
"owner_pid": os.getpid(),
},
)
except Exception:
pass
def load_session_lease_shadow(
session_id: str | None = None,
) -> dict[str, Any] | None:
"""Load the durable session-lease shadow left by this profile identity.
With *session_id*, load that session's collision-safe record (#702 F5),
falling back to the legacy single-slot record only when its payload names
the same session. Without *session_id*, return the legacy record or the
sole surviving instance record — ambiguity (multiple candidates) returns
``None`` because a shadow that cannot be attributed proves nothing.
"""
try:
import mcp_session_state as mss
sid = (session_id or "").strip()
if sid:
record = mss.load_state(
kind=mss.KIND_REVIEWER_SESSION_LEASE, instance_id=sid
)
if record is not None:
return record
legacy = mss.load_state(kind=mss.KIND_REVIEWER_SESSION_LEASE)
if legacy and (legacy.get("session_id") or "").strip() == sid:
return legacy
return None
legacy = mss.load_state(kind=mss.KIND_REVIEWER_SESSION_LEASE)
if legacy is not None:
return legacy
records = mss.list_states(kind=mss.KIND_REVIEWER_SESSION_LEASE)
if len(records) == 1:
return records[0]
return None
except Exception:
return None
def _pid_alive(pid: int) -> bool:
"""Probe process existence; fail closed toward 'alive' when unsure."""
try:
os.kill(int(pid), 0)
return True
except ProcessLookupError:
return False
except PermissionError:
return True
except Exception:
return True
def assess_crashed_session_lease_orphan(
shadow: dict[str, Any] | None,
*,
lease: dict[str, Any] | None,
current_pid: int | None = None,
pid_alive: bool | None = None,
) -> dict[str, Any]:
"""Derive owner-process evidence for a durable lease from the shadow (#702).
Returns ``owner_process_alive`` tri-state. Only a provably dead recorded
owner pid yields ``False`` (a dead pid cannot still be running the owning
daemon); an alive pid is *not* ownership proof and stays ``None`` unless
it is this very process. Session ids must match exactly — the shadow of a
different lease proves nothing.
"""
reasons: list[str] = []
if not shadow or not lease:
return {
"orphan_evidence": False,
"owner_process_alive": None,
"reasons": ["no durable session-lease shadow evidence available"],
}
shadow_sid = (shadow.get("session_id") or "").strip()
lease_sid = (lease.get("session_id") or "").strip()
if not shadow_sid or not lease_sid or shadow_sid != lease_sid:
return {
"orphan_evidence": False,
"owner_process_alive": None,
"reasons": [
"session-lease shadow session_id does not match the durable "
f"lease (shadow={shadow_sid or None}, lease={lease_sid or None})"
],
}
owner_pid = shadow.get("owner_pid") or shadow.get("writer_pid")
try:
owner_pid = int(owner_pid)
except (TypeError, ValueError):
return {
"orphan_evidence": False,
"owner_process_alive": None,
"reasons": ["session-lease shadow has no usable owner pid (fail closed)"],
}
this_pid = current_pid if current_pid is not None else os.getpid()
if owner_pid == this_pid:
return {
"orphan_evidence": False,
"owner_process_alive": True,
"owner_pid": owner_pid,
"reasons": ["shadow owner pid is the current process"],
}
alive = pid_alive if pid_alive is not None else _pid_alive(owner_pid)
if alive:
reasons.append(
f"shadow owner pid {owner_pid} observed alive; PID liveness is not "
"ownership proof — owner evidence stays unknown (fail closed)"
)
return {
"orphan_evidence": False,
"owner_process_alive": None,
"owner_pid": owner_pid,
"reasons": reasons,
}
reasons.append(
f"shadow owner pid {owner_pid} is dead: the daemon that recorded the "
"matching session lease exited without teardown (crash orphan, #702)"
)
return {
"orphan_evidence": True,
"owner_process_alive": False,
"owner_pid": owner_pid,
"reasons": reasons,
}
def assess_mutation_lease_gate(
*,
pr_number: int,
@@ -557,6 +733,7 @@ _HANDOFF_CLASSIFICATIONS = frozenset({
"foreign_completed_superseded_head",
"foreign_expired_superseded_head",
"orphaned_owner_missing",
"orphaned_expired_superseded_head",
"ambiguous_conflicting_evidence",
"instructed_lease_missing_with_replacement",
"worktree_binding_mismatch",
@@ -850,6 +1027,38 @@ def assess_obsolete_reviewer_comment_lease_cleanup(
classification = "foreign_expired_superseded_head"
elif head_superseded and has_terminal and not past_expiry:
classification = "foreign_completed_superseded_head"
elif head_superseded and not has_terminal and past_expiry:
# Crash-orphan shape (#702): the lease outlived its head with no
# formal terminal review and has now passed canonical expiry, so
# it no longer gates fresh acquisition. Guarded cleanup of the
# ledger marker still demands provable owner-process-exit
# evidence and a safe worktree — never inferred.
classification = "orphaned_expired_superseded_head"
if owner_process_alive is True:
fail_closed_reasons.append(
"lease past expiry on superseded head but owner process "
"still alive; cleanup denied"
)
elif owner_process_alive is None:
fail_closed_reasons.append(
"expired superseded-head lease with no terminal review: "
"cleanup requires provable owner-process-exit evidence "
"(owner_process_alive=false); the expired lease no longer "
"gates fresh acquisition"
)
elif worktree_clean is False:
fail_closed_reasons.append(
"owner process absent but worktree dirty; cleanup denied"
)
elif not (worktree_clean is True or worktree_exists is False):
# #702 F3: unknown worktree evidence must fail closed, exactly
# like the sibling orphaned_owner_missing gate and the
# diagnosis path — cleanup needs affirmative safe evidence.
fail_closed_reasons.append(
"owner process absent but worktree evidence is unknown; "
"cleanup requires affirmative safe evidence "
"(worktree_clean=true or worktree_exists=false)"
)
elif head_superseded and not has_terminal:
classification = "ambiguous_conflicting_evidence"
fail_closed_reasons.append(
@@ -891,6 +1100,7 @@ def assess_obsolete_reviewer_comment_lease_cleanup(
"foreign_expired_superseded_head",
"foreign_expired_current_head",
"orphaned_owner_missing",
"orphaned_expired_superseded_head",
}:
fail_closed_reasons.append(
"controller/recovery capability required "
@@ -909,6 +1119,7 @@ def assess_obsolete_reviewer_comment_lease_cleanup(
"foreign_completed_superseded_head",
"foreign_expired_superseded_head",
"orphaned_owner_missing",
"orphaned_expired_superseded_head",
"foreign_expired_current_head",
}
# foreign_expired_current_head needs orphan/clean worktree + authority.
@@ -1181,6 +1392,30 @@ def diagnose_reviewer_pr_lease_handoff(
"foreign lease pinned to superseded head with completed formal "
"review; use guarded obsolete-lease cleanup (do not wait indefinitely)"
)
elif not is_own and head_superseded and not terminal and past_expiry:
# Crash-orphan shape after canonical expiry (#702): the expired
# marker no longer gates acquisition, so a fresh reviewer may
# acquire at the current head. Guarded ledger cleanup becomes
# available only with provable owner-exit evidence.
classification = "orphaned_expired_superseded_head"
if owner_process_alive is False and (
worktree_clean is True or worktree_exists is False
):
next_action = NEXT_ACTION_CLEANUP_OBSOLETE_LEASE
reasons.append(
"expired superseded-head lease with no terminal review and "
"provable owner-process exit (crash orphan, #702); guarded "
"obsolete-lease cleanup eligible"
)
else:
next_action = NEXT_ACTION_ACQUIRE
reasons.append(
"lease head superseded and lease past canonical expiry with "
"no formal terminal review (crash orphan, #702); expired "
"marker no longer gates acquisition — acquire a fresh lease "
"at the current head; guarded cleanup needs "
"owner_process_alive=false and a clean/absent worktree"
)
elif not is_own and head_superseded and not terminal:
classification = "ambiguous_conflicting_evidence"
next_action = NEXT_ACTION_WAIT
+257
View File
@@ -0,0 +1,257 @@
"""Sanctioned recovery for stale env-bound task workspace bindings (#702).
When the MCP daemon dies from an unhandled exception it never runs teardown,
and the auto-reconnected daemon inherits ``GITEA_ACTIVE_WORKTREE`` from its
parent environment. That inherited value can permanently bind the fresh
session to a prior task's worktree (the PR #701 / ``review-pr-654`` incident).
This module classifies the active env binding against live session evidence
and produces a fail-closed recovery plan. Only two situations permit the
daemon to clear the binding on its own:
- the bound path no longer exists on disk (``provably_stale_missing_path``)
- the binding was inherited at daemon boot and a *sanctioned* in-session
reviewer/merger lease later bound a different worktree
(``superseded_by_session_lease``)
Everything else is surfaced (``unverified_inherited``) and left for the
managed reconnect path — never cleared silently, never rebound to a guessed
worktree.
"""
from __future__ import annotations
import os
from typing import Any
ACTIVE_WORKTREE_ENV = "GITEA_ACTIVE_WORKTREE"
CLASSIFICATION_UNBOUND = "unbound"
CLASSIFICATION_CORROBORATED = "corroborated"
CLASSIFICATION_UNVERIFIED_INHERITED = "unverified_inherited"
CLASSIFICATION_PROVABLY_STALE_MISSING_PATH = "provably_stale_missing_path"
CLASSIFICATION_SUPERSEDED_BY_SESSION_LEASE = "superseded_by_session_lease"
CLASSIFICATIONS = frozenset({
CLASSIFICATION_UNBOUND,
CLASSIFICATION_CORROBORATED,
CLASSIFICATION_UNVERIFIED_INHERITED,
CLASSIFICATION_PROVABLY_STALE_MISSING_PATH,
CLASSIFICATION_SUPERSEDED_BY_SESSION_LEASE,
})
# Roles whose task workspace is owned by a lease lifecycle, so an inherited
# generic binding without a corroborating lease is suspect.
LEASE_BOUND_ROLES = frozenset({"reviewer", "merger"})
RECOVERY_ACTION_NONE = "none"
RECOVERY_ACTION_CLEAR_ENV = "clear_env"
def _norm(path: str | None) -> str:
text = (path or "").strip()
if not text:
return ""
return os.path.realpath(os.path.abspath(text))
def snapshot_boot_bindings(
env: dict[str, str] | os._Environ | None = None,
) -> dict[str, Any]:
"""Capture worktree env bindings present when the daemon booted.
A value present in this snapshot was inherited from the parent
environment, not established by any sanctioned tool in this daemon's
lifetime.
"""
env_map = env if env is not None else os.environ
return {
"active_worktree": (env_map.get(ACTIVE_WORKTREE_ENV) or "").strip() or None,
}
def classify_active_worktree_binding(
*,
active_value: str | None,
role_env_value: str | None = None,
session_lease_worktree: str | None = None,
boot_inherited: bool,
path_exists: bool | None,
role_kind: str | None = None,
) -> dict[str, Any]:
"""Classify the GITEA_ACTIVE_WORKTREE binding against live evidence.
Fail closed: unknown evidence never yields a clear-eligible class.
"""
reasons: list[str] = []
active = _norm(active_value)
if not active:
return {
"classification": CLASSIFICATION_UNBOUND,
"clear_eligible": False,
"active_worktree": None,
"reasons": ["no GITEA_ACTIVE_WORKTREE binding present"],
}
role = (role_kind or "").strip().lower()
role_env = _norm(role_env_value)
lease_wt = _norm(session_lease_worktree)
result: dict[str, Any] = {
"active_worktree": active,
"boot_inherited": bool(boot_inherited),
"role_kind": role or None,
"session_lease_worktree": lease_wt or None,
"role_env_worktree": role_env or None,
}
if path_exists is False:
reasons.append(
f"bound worktree '{active}' no longer exists on disk; the binding "
"cannot name a valid task workspace"
)
result.update({
"classification": CLASSIFICATION_PROVABLY_STALE_MISSING_PATH,
"clear_eligible": True,
"reasons": reasons,
})
return result
if lease_wt and lease_wt == active:
reasons.append("binding matches the sanctioned in-session lease worktree")
result.update({
"classification": CLASSIFICATION_CORROBORATED,
"clear_eligible": False,
"reasons": reasons,
})
return result
if lease_wt and lease_wt != active and boot_inherited:
reasons.append(
"boot-inherited binding disagrees with the sanctioned in-session "
f"lease worktree '{lease_wt}'; the lease is live authority"
)
result.update({
"classification": CLASSIFICATION_SUPERSEDED_BY_SESSION_LEASE,
"clear_eligible": True,
"reasons": reasons,
})
return result
if lease_wt and lease_wt != active and not boot_inherited:
# Set after boot by tooling; disagreement is surfaced, never auto-cleared.
reasons.append(
"post-boot binding disagrees with the in-session lease worktree; "
"surfacing only (fail closed, no automatic clear)"
)
result.update({
"classification": CLASSIFICATION_UNVERIFIED_INHERITED,
"clear_eligible": False,
"reasons": reasons,
})
return result
if role_env and role_env == active:
reasons.append("binding matches the role-specific worktree env binding")
result.update({
"classification": CLASSIFICATION_CORROBORATED,
"clear_eligible": False,
"reasons": reasons,
})
return result
if boot_inherited and role in LEASE_BOUND_ROLES and not lease_wt:
reasons.append(
"binding was inherited at daemon boot, no sanctioned in-session "
f"lease corroborates it, and the {role} role binds workspaces "
"through the lease lifecycle; treat as unverified until a fresh "
"lease or managed reconnect re-establishes the workspace"
)
result.update({
"classification": CLASSIFICATION_UNVERIFIED_INHERITED,
"clear_eligible": False,
"reasons": reasons,
})
return result
reasons.append("no contradicting evidence; binding treated as corroborated")
result.update({
"classification": CLASSIFICATION_CORROBORATED,
"clear_eligible": False,
"reasons": reasons,
})
return result
def plan_recovery(classification_result: dict[str, Any]) -> dict[str, Any]:
"""Turn a classification into an explicit, fail-closed recovery plan."""
classification = classification_result.get("classification")
clear_eligible = bool(classification_result.get("clear_eligible"))
reasons = list(classification_result.get("reasons") or [])
if classification not in CLASSIFICATIONS:
return {
"action": RECOVERY_ACTION_NONE,
"clear_allowed": False,
"classification": classification,
"reasons": reasons + ["unknown classification (fail closed)"],
}
if clear_eligible and classification in {
CLASSIFICATION_PROVABLY_STALE_MISSING_PATH,
CLASSIFICATION_SUPERSEDED_BY_SESSION_LEASE,
}:
return {
"action": RECOVERY_ACTION_CLEAR_ENV,
"clear_allowed": True,
"classification": classification,
"active_worktree": classification_result.get("active_worktree"),
"reasons": reasons,
}
exact_next_action = None
if classification == CLASSIFICATION_UNVERIFIED_INHERITED:
exact_next_action = (
"managed recovery required: reconnect the MCP namespace through "
"the managed client configuration (or start a fresh session) so "
"the daemon boots without the inherited GITEA_ACTIVE_WORKTREE, or "
"corroborate the binding by acquiring a lease for that worktree; "
"do not clear or rewrite the environment manually"
)
return {
"action": RECOVERY_ACTION_NONE,
"clear_allowed": False,
"classification": classification,
"active_worktree": classification_result.get("active_worktree"),
"reasons": reasons,
**({"exact_next_action": exact_next_action} if exact_next_action else {}),
}
def apply_recovery(
plan: dict[str, Any],
env: dict[str, str] | os._Environ | None = None,
) -> dict[str, Any]:
"""Apply a clear-eligible plan by removing the stale env binding.
This is the sanctioned in-daemon mechanism (#702 AC2); callers must
persist the returned record for audit. Refuses anything but an explicit
``clear_env`` plan.
"""
env_map = env if env is not None else os.environ
if not plan.get("clear_allowed") or plan.get("action") != RECOVERY_ACTION_CLEAR_ENV:
return {
"performed": False,
"action": RECOVERY_ACTION_NONE,
"reasons": ["recovery plan does not authorize clearing (fail closed)"],
}
cleared_value = (env_map.get(ACTIVE_WORKTREE_ENV) or "").strip() or None
env_map.pop(ACTIVE_WORKTREE_ENV, None)
return {
"performed": True,
"action": RECOVERY_ACTION_CLEAR_ENV,
"cleared_env": ACTIVE_WORKTREE_ENV,
"cleared_value": cleared_value,
"classification": plan.get("classification"),
"reasons": list(plan.get("reasons") or []),
}
+16 -22
View File
@@ -79,46 +79,41 @@ class TestApiRequestFailures(unittest.TestCase):
@patch("gitea_auth.urllib.request.urlopen")
def test_timeout_converted_to_runtimeerror(self, mock_open):
mock_open.side_effect = TimeoutError("timed out")
with self.assertRaises(gitea_auth.GiteaNetworkError) as ctx:
with self.assertRaises(RuntimeError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH)
# Fixed message only (#699) — still a RuntimeError subclass.
self.assertIsInstance(ctx.exception, RuntimeError)
self.assertEqual(str(ctx.exception), "Network error contacting Gitea")
self.assertIn("network error contacting Gitea", str(ctx.exception))
@patch("gitea_auth.urllib.request.urlopen")
def test_dns_network_failure_converted(self, mock_open):
mock_open.side_effect = urllib.error.URLError("Name or service not known")
with self.assertRaises(gitea_auth.GiteaNetworkError) as ctx:
with self.assertRaises(RuntimeError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH)
self.assertEqual(str(ctx.exception), "Network error contacting Gitea")
self.assertIn("network error contacting Gitea", str(ctx.exception))
@patch("gitea_auth.urllib.request.urlopen")
def test_502_upstream_message(self, mock_open):
mock_open.side_effect = http_error(502, "bad gateway")
with self.assertRaises(gitea_auth.GiteaHttpError) as ctx:
with self.assertRaises(RuntimeError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH)
msg = str(ctx.exception)
self.assertEqual(msg, "Gitea upstream unavailable")
self.assertEqual(ctx.exception.reason_code, "upstream_unavailable")
self.assertEqual(ctx.exception.http_status, 502)
self.assertIn("HTTP 502", msg)
self.assertIn("upstream unavailable", msg)
@patch("gitea_auth.urllib.request.urlopen")
def test_503_upstream_message(self, mock_open):
mock_open.side_effect = http_error(503, "")
with self.assertRaises(gitea_auth.GiteaHttpError) as ctx:
with self.assertRaises(RuntimeError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH)
self.assertEqual(str(ctx.exception), "Gitea upstream unavailable")
self.assertEqual(ctx.exception.http_status, 503)
self.assertIn("HTTP 503", str(ctx.exception))
self.assertIn("upstream unavailable", str(ctx.exception))
@patch("gitea_auth.urllib.request.urlopen")
def test_malformed_error_payload_does_not_crash(self, mock_open):
# Non-JSON garbage error body must still yield a clean typed error
# with a fixed message (no body echo — #699).
# Non-JSON garbage error body must still yield a clean RuntimeError.
mock_open.side_effect = http_error(500, "<html>garbage</html>")
with self.assertRaises(gitea_auth.GiteaHttpError) as ctx:
with self.assertRaises(RuntimeError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH)
self.assertEqual(str(ctx.exception), "Gitea HTTP request failed")
self.assertNotIn("<html>", str(ctx.exception))
self.assertIn("HTTP 500", str(ctx.exception))
@patch("gitea_auth.urllib.request.urlopen")
def test_malformed_success_json_raises_clean_error(self, mock_open):
@@ -131,17 +126,16 @@ class TestApiRequestFailures(unittest.TestCase):
def test_no_secret_leak_in_error_body(self, mock_open):
mock_open.side_effect = http_error(
400, "failed: token supersecret123 rejected")
with self.assertRaises(gitea_auth.GiteaHttpError) as ctx:
with self.assertRaises(RuntimeError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH)
msg = str(ctx.exception)
self.assertNotIn("supersecret123", msg)
# Fixed message — body never appears (stronger than redaction).
self.assertEqual(msg, "Gitea HTTP request failed")
self.assertIn(gitea_audit.REDACTED, msg)
@patch("gitea_auth.urllib.request.urlopen")
def test_auth_header_never_in_error(self, mock_open):
mock_open.side_effect = http_error(400, "bad request")
with self.assertRaises(gitea_auth.GiteaHttpError) as ctx:
with self.assertRaises(RuntimeError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH)
self.assertNotIn(FAKE_AUTH, str(ctx.exception))
@@ -561,12 +561,29 @@ class TestIssue691SupersededHeadCleanup(unittest.TestCase):
self.assertTrue(any("owns the lease" in r for r in result["reasons"]))
def test_superseded_without_terminal_review_denied(self):
# Pre-expiry the missing terminal review is ambiguous (fail closed);
# post-expiry with unknown owner evidence it is a crash-orphan (#702)
# that still fails closed until owner-process exit is proven.
now = datetime(2026, 7, 13, 5, 0, 0, tzinfo=timezone.utc)
expires = datetime(2026, 7, 13, 4, 23, 0, tzinfo=timezone.utc)
claim = _lease_comment(expires_at=expires, last_activity=expires - timedelta(hours=1))
result = _assess([claim], formal_reviews=[], now=now)
result = _assess(
[claim], formal_reviews=[], now=now, owner_process_alive=None
)
self.assertFalse(result["cleanup_allowed"])
self.assertEqual(result["classification"], "ambiguous_conflicting_evidence")
self.assertEqual(
result["classification"], "orphaned_expired_superseded_head"
)
fresh_expires = now + timedelta(minutes=60)
fresh = _lease_comment(
expires_at=fresh_expires, last_activity=now - timedelta(minutes=5)
)
pre_expiry = _assess([fresh], formal_reviews=[], now=now)
self.assertFalse(pre_expiry["cleanup_allowed"])
self.assertEqual(
pre_expiry["classification"], "ambiguous_conflicting_evidence"
)
class TestIssue691DiagnoseClassifications(unittest.TestCase):
@@ -0,0 +1,693 @@
"""Regression tests for the PR #703 REQUEST_CHANGES findings F1F6 (#702).
Formal review at head 889931d independently reproduced six defects:
F1 — stale-binding recovery ran only AFTER the terminal launcher probe in
``gitea_resolve_task_capability``; a worktree removed mid-session wedged
every mutation-task resolution on 'missing cwd' before recovery.
F2 — ``_resolve_preflight_workspace_path`` skipped the existence checks the
canonical mutation context applies; a dead binding could produce empty
porcelain and read as a clean workspace.
F3 — ``orphaned_expired_superseded_head`` cleanup accepted unknown
``worktree_exists``/``worktree_clean`` evidence.
F4 — the 4-hour session-state TTL silently discarded recovery-critical
crash-orphan shadow evidence.
F5 — the single same-profile shadow slot let concurrent sessions overwrite
each other's crash evidence.
F6 — the environment was cleared BEFORE the audit record was persisted, and
audit-write failure was swallowed.
"""
from __future__ import annotations
import json
import os
import shutil
import sys
import tempfile
import unittest
from datetime import datetime, timedelta, timezone
from pathlib import Path
from unittest.mock import patch
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
import mcp_session_state as mss # noqa: E402
import namespace_workspace_binding as nwb # noqa: E402
import reviewer_pr_lease as leases # noqa: E402
import stale_binding_recovery as sbr # noqa: E402
import mcp_server # noqa: E402
REPO = "Scaled-Tech-Consulting/Gitea-Tools"
OLD_HEAD = "b" * 40
NEW_HEAD = "6" * 40
CONFIG = {
"version": 2,
"contexts": {
"ctx": {
"enabled": True,
"gitea": {"enabled": True, "base_url": "https://gitea.example.com"},
}
},
"profiles": {
"prgs-author": {
"enabled": True,
"context": "ctx",
"role": "author",
"username": "jcwalker3",
"auth": {"type": "env", "name": "GITEA_TOKEN_AUTHOR"},
"allowed_operations": [
"gitea.read", "gitea.issue.create", "gitea.issue.comment",
"gitea.pr.create", "gitea.branch.push",
],
"forbidden_operations": [
"gitea.pr.approve", "gitea.pr.merge", "gitea.pr.review",
],
"execution_profile": "prgs-author",
},
},
"rules": {"allow_runtime_switching": False},
}
def _now() -> datetime:
return datetime.now(timezone.utc)
class _CapabilityHarness(unittest.TestCase):
"""Shared config/env plumbing for resolve-capability integration tests."""
def setUp(self):
self._remotes = patch.dict(mcp_server.REMOTES, {
"prgs": {
"host": "gitea.example.com",
"org": "Scaled-Tech-Consulting",
"repo": "Gitea-Tools",
},
})
self._remotes.start()
mcp_server._IDENTITY_CACHE.clear()
self._dir = tempfile.TemporaryDirectory()
self.config_path = os.path.join(self._dir.name, "profiles.json")
with open(self.config_path, "w", encoding="utf-8") as fh:
fh.write(json.dumps(CONFIG))
def tearDown(self):
self._remotes.stop()
mcp_server._IDENTITY_CACHE.clear()
self._dir.cleanup()
def _env(self, **extra):
env = {
"GITEA_MCP_CONFIG": self.config_path,
"GITEA_MCP_PROFILE": "prgs-author",
"GITEA_TOKEN_AUTHOR": "author-pass",
}
env.update(extra)
return env
def _deleted_worktree(self) -> str:
path = tempfile.mkdtemp(prefix="gitea-removed-worktree-")
shutil.rmtree(path)
return path
class TestF1RecoveryBeforeTerminalProbe(_CapabilityHarness):
"""F1: recovery must run before the terminal cwd probe can wedge."""
@patch("mcp_server.api_request", return_value={"login": "jcwalker3"})
@patch("mcp_server.get_auth_header", return_value="token author-pass")
def test_removed_worktree_recovers_before_probe(self, _auth, _api):
missing = self._deleted_worktree()
env = self._env(
GITEA_ACTIVE_WORKTREE=missing,
GITEA_FORCE_TERMINAL_PROBE="1",
GITEA_FORCE_STALE_BINDING_RECOVERY="1",
)
with patch.dict(os.environ, env):
res = mcp_server.gitea_resolve_task_capability(
task="create_pr", remote="prgs"
)
# The provably-stale binding was cleared before the probe ran.
self.assertNotIn("GITEA_ACTIVE_WORKTREE", os.environ)
self.assertFalse(res.get("terminal_launcher_unhealthy"))
report = res.get("stale_binding_recovery") or {}
self.assertEqual(
report.get("classification"),
sbr.CLASSIFICATION_PROVABLY_STALE_MISSING_PATH,
)
self.assertTrue(report.get("recovery_performed"))
self.assertTrue(res.get("allowed_in_current_session"))
@patch("mcp_server.api_request", return_value={"login": "jcwalker3"})
@patch("mcp_server.get_auth_header", return_value="token author-pass")
def test_probe_block_still_carries_recovery_report(self, _auth, _api):
# Recovery disabled (test mode without the force flag) preserves the
# fail-closed probe block, but the block must now carry the
# classification evidence instead of hiding it.
missing = self._deleted_worktree()
env = self._env(
GITEA_ACTIVE_WORKTREE=missing,
GITEA_FORCE_TERMINAL_PROBE="1",
)
with patch.dict(os.environ, env):
res = mcp_server.gitea_resolve_task_capability(
task="create_pr", remote="prgs"
)
self.assertIn("GITEA_ACTIVE_WORKTREE", os.environ)
self.assertTrue(res.get("terminal_launcher_unhealthy"))
report = res.get("stale_binding_recovery") or {}
self.assertEqual(
report.get("classification"),
sbr.CLASSIFICATION_PROVABLY_STALE_MISSING_PATH,
)
self.assertFalse(report.get("recovery_performed"))
@patch("mcp_server.api_request", return_value={"login": "jcwalker3"})
@patch("mcp_server.get_auth_header", return_value="token author-pass")
def test_unverified_binding_is_never_cleared_by_reordering(self, _auth, _api):
# F1 must not weaken authorization: an existing, uncorroborated
# binding survives resolution untouched even with recovery forced.
existing = tempfile.mkdtemp(prefix="gitea-existing-worktree-")
self.addCleanup(shutil.rmtree, existing, ignore_errors=True)
env = self._env(
GITEA_ACTIVE_WORKTREE=existing,
GITEA_FORCE_TERMINAL_PROBE="1",
GITEA_FORCE_STALE_BINDING_RECOVERY="1",
)
with patch.dict(os.environ, env):
res = mcp_server.gitea_resolve_task_capability(
task="create_pr", remote="prgs"
)
self.assertEqual(os.environ.get("GITEA_ACTIVE_WORKTREE"), existing)
report = res.get("stale_binding_recovery") or {}
self.assertFalse(report.get("recovery_performed"))
class TestF2PreflightPathVerification(_CapabilityHarness):
"""F2: preflight and mutation contexts verify resolved paths alike."""
def test_preflight_and_mutation_context_agree_on_dead_binding(self):
missing = self._deleted_worktree()
env = self._env(GITEA_ACTIVE_WORKTREE=missing)
with patch.dict(os.environ, env):
preflight = mcp_server._resolve_preflight_workspace_path(None)
mutation = mcp_server._resolve_namespace_mutation_context(None)
self.assertEqual(preflight, mutation["workspace_path"])
self.assertNotEqual(preflight, os.path.realpath(missing))
def test_missing_explicit_worktree_never_reads_clean(self):
missing = self._deleted_worktree()
with patch.dict(os.environ, self._env()):
porcelain = mcp_server._get_workspace_porcelain(worktree_path=missing)
self.assertEqual(
porcelain, mcp_server.PREFLIGHT_WORKSPACE_UNAVAILABLE_PORCELAIN
)
# The sentinel parses as a tracked dirty entry — never clean.
self.assertTrue(mcp_server._parse_porcelain_entries(porcelain))
def test_workspace_deleted_during_evaluation_never_reads_clean(self):
# TOCTOU: resolution succeeded, then the path vanished before git ran.
missing = self._deleted_worktree()
with patch.dict(os.environ, self._env()):
with patch.object(
mcp_server,
"_resolve_preflight_workspace_path",
return_value=missing,
):
porcelain = mcp_server._get_workspace_porcelain()
self.assertEqual(
porcelain, mcp_server.PREFLIGHT_WORKSPACE_UNAVAILABLE_PORCELAIN
)
def test_git_failure_never_reads_clean(self):
class _Failed:
returncode = 128
stdout = ""
stderr = "fatal: not a git repository"
with patch.dict(os.environ, self._env()):
with patch.object(
mcp_server.subprocess, "run", return_value=_Failed()
):
porcelain = mcp_server._get_workspace_porcelain()
self.assertEqual(
porcelain, mcp_server.PREFLIGHT_WORKSPACE_UNAVAILABLE_PORCELAIN
)
def test_symlinked_binding_to_deleted_target_is_demoted(self):
target = tempfile.mkdtemp(prefix="gitea-symlink-target-")
holder = tempfile.mkdtemp(prefix="gitea-symlink-holder-")
self.addCleanup(shutil.rmtree, holder, ignore_errors=True)
link = os.path.join(holder, "worktree-link")
os.symlink(target, link)
shutil.rmtree(target)
demotions: list[str] = []
_path, source = nwb.resolve_namespace_workspace(
role_kind="reviewer",
process_project_root=os.getcwd(),
env={"GITEA_ACTIVE_WORKTREE": link},
demotions=demotions,
verify_paths=True,
)
self.assertEqual(source, "MCP server process root (default)")
self.assertEqual(len(demotions), 1)
def test_dead_binding_falls_through_to_live_role_binding(self):
# Path changes during evaluation: the dead candidate demotes at
# verification time and never resurrects over the live fallback.
alive = tempfile.mkdtemp(prefix="gitea-alive-worktree-")
self.addCleanup(shutil.rmtree, alive, ignore_errors=True)
missing = self._deleted_worktree()
path, source = nwb.resolve_namespace_workspace(
role_kind="reviewer",
process_project_root=os.getcwd(),
env={
"GITEA_ACTIVE_WORKTREE": missing,
"GITEA_REVIEWER_WORKTREE": alive,
},
verify_paths=True,
)
self.assertEqual(path, os.path.realpath(alive))
self.assertEqual(
source, "GITEA_REVIEWER_WORKTREE environment variable"
)
class TestF3AffirmativeWorktreeEvidence(unittest.TestCase):
"""F3: unknown worktree evidence must fail closed for crash orphans."""
def _comments(self) -> list[dict]:
stale = _now() - timedelta(hours=3)
body = leases.format_lease_body(
repo=REPO,
pr_number=701,
issue_number=699,
reviewer_identity="sysadmin",
profile="prgs-reviewer",
session_id="65664-4f248d213d60",
worktree="branches/review-pr-654",
phase="validation-start",
candidate_head=OLD_HEAD,
target_branch="master",
target_branch_sha="2" * 40,
last_activity=stale,
expires_at=stale + timedelta(minutes=120),
)
return [{
"id": 11131,
"body": body,
"user": {"login": "sysadmin"},
"author": "sysadmin",
"created_at": stale.isoformat(),
"updated_at": stale.isoformat(),
}]
def _assess(self, **kwargs):
defaults = dict(
pr_number=701,
current_head_sha=NEW_HEAD,
formal_reviews=[],
repo=REPO,
expected_repo=REPO,
requesting_session_id="fresh-controller",
controller_recovery_authorized=True,
owner_process_alive=False,
)
defaults.update(kwargs)
return leases.assess_obsolete_reviewer_comment_lease_cleanup(
self._comments(), **defaults
)
def test_unknown_worktree_evidence_fails_closed(self):
result = self._assess(worktree_exists=None, worktree_clean=None)
self.assertEqual(
result["classification"], "orphaned_expired_superseded_head"
)
self.assertFalse(result["cleanup_allowed"])
self.assertIn(
"affirmative safe evidence",
" ".join(result["fail_closed_reasons"]),
)
def test_unknown_cleanliness_with_existing_worktree_fails_closed(self):
result = self._assess(worktree_exists=True, worktree_clean=None)
self.assertFalse(result["cleanup_allowed"])
def test_affirmative_clean_worktree_is_eligible(self):
result = self._assess(worktree_exists=True, worktree_clean=True)
self.assertEqual(
result["classification"], "orphaned_expired_superseded_head"
)
self.assertTrue(result["cleanup_allowed"])
def test_affirmative_absent_worktree_is_eligible(self):
result = self._assess(worktree_exists=False, worktree_clean=None)
self.assertTrue(result["cleanup_allowed"])
def test_matrix_matches_diagnosis_gate(self):
# The cleanup matrix and the diagnosis path must agree: unknown
# evidence yields acquire-only, affirmative evidence yields cleanup.
diagnosis_unknown = leases.diagnose_reviewer_pr_lease_handoff(
self._comments(),
pr_number=701,
current_session_id=None,
current_reviewer_identity="fresh-reviewer",
current_head_sha=NEW_HEAD,
formal_reviews=[],
owner_process_alive=False,
worktree_exists=None,
worktree_clean=None,
)
assess_unknown = self._assess(worktree_exists=None, worktree_clean=None)
self.assertNotEqual(
diagnosis_unknown["next_action"],
"cleanup_obsolete_reviewer_comment_lease",
)
self.assertFalse(assess_unknown["cleanup_allowed"])
class TestF4CrashShadowDurability(unittest.TestCase):
"""F4: crash-orphan evidence survives the former 4h TTL boundary."""
SID = "82984-abcabcabcabc"
def setUp(self):
self._dir = tempfile.TemporaryDirectory()
self._env = patch.dict(
os.environ, {"GITEA_MCP_SESSION_STATE_DIR": self._dir.name}
)
self._env.start()
leases._SESSION_LEASE = None
def tearDown(self):
self._env.stop()
self._dir.cleanup()
leases._SESSION_LEASE = None
def _age_record(self, kind: str, hours: float, instance_id: str | None = None):
path = mss.state_file_path(kind=kind, instance_id=instance_id)
with open(path, encoding="utf-8") as fh:
envelope = json.load(fh)
stamp = (
(_now() - timedelta(hours=hours)).isoformat().replace("+00:00", "Z")
)
envelope["recorded_at"] = stamp
envelope["updated_at"] = stamp
envelope["payload"]["recorded_at"] = stamp
envelope["payload"]["updated_at"] = stamp
with open(path, "w", encoding="utf-8") as fh:
json.dump(envelope, fh)
def _save_shadow(self) -> None:
mss.save_state(
kind=mss.KIND_REVIEWER_SESSION_LEASE,
instance_id=self.SID,
payload={"session_id": self.SID, "owner_pid": os.getpid()},
)
def _load_shadow(self):
return mss.load_state(
kind=mss.KIND_REVIEWER_SESSION_LEASE, instance_id=self.SID
)
def test_shadow_loads_before_former_ttl_boundary(self):
self._save_shadow()
self._age_record(
mss.KIND_REVIEWER_SESSION_LEASE, hours=3.9, instance_id=self.SID
)
self.assertIsNotNone(self._load_shadow())
def test_shadow_loads_at_former_ttl_boundary(self):
self._save_shadow()
self._age_record(
mss.KIND_REVIEWER_SESSION_LEASE, hours=4.0, instance_id=self.SID
)
self.assertIsNotNone(self._load_shadow())
def test_shadow_loads_long_after_former_ttl_boundary(self):
self._save_shadow()
self._age_record(
mss.KIND_REVIEWER_SESSION_LEASE, hours=27.0, instance_id=self.SID
)
record = self._load_shadow()
self.assertIsNotNone(record)
self.assertEqual(record.get("session_id"), self.SID)
def test_stale_binding_audit_record_is_also_durable(self):
mss.save_state(
kind=mss.KIND_STALE_BINDING_RECOVERY,
payload={"cleared_env": "GITEA_ACTIVE_WORKTREE"},
)
self._age_record(mss.KIND_STALE_BINDING_RECOVERY, hours=27.0)
self.assertIsNotNone(
mss.load_state(kind=mss.KIND_STALE_BINDING_RECOVERY)
)
def test_non_recovery_kinds_keep_ttl(self):
mss.save_state(
kind=mss.KIND_WORKFLOW_LOAD, payload={"workflow_hash": "abc"}
)
self._age_record(mss.KIND_WORKFLOW_LOAD, hours=5.0)
self.assertIsNone(mss.load_state(kind=mss.KIND_WORKFLOW_LOAD))
def test_sanctioned_clear_is_the_terminal_reconciliation(self):
self._save_shadow()
self._age_record(
mss.KIND_REVIEWER_SESSION_LEASE, hours=27.0, instance_id=self.SID
)
mss.clear_state(
kind=mss.KIND_REVIEWER_SESSION_LEASE, instance_id=self.SID
)
self.assertIsNone(self._load_shadow())
self.assertEqual(
mss.list_states(kind=mss.KIND_REVIEWER_SESSION_LEASE), []
)
def test_crash_orphan_recovery_works_after_former_ttl(self):
# End to end: a crashed session's shadow still proves owner exit a
# day later.
leases.record_session_lease(
{
"pr_number": 701,
"session_id": self.SID,
"worktree": "branches/review-pr-654",
"candidate_head": OLD_HEAD,
"repo": REPO,
"comment_id": 11131,
"profile": "prgs-reviewer",
"reviewer_identity": "sysadmin",
"phase": "claimed",
},
lease_provenance={"source": "acquire", "comment_id": 11131},
)
leases._SESSION_LEASE = None # simulated crash: no sanctioned clear
self._age_record(
mss.KIND_REVIEWER_SESSION_LEASE, hours=27.0, instance_id=self.SID
)
shadow = leases.load_session_lease_shadow(session_id=self.SID)
self.assertIsNotNone(shadow)
verdict = leases.assess_crashed_session_lease_orphan(
shadow,
lease={"session_id": self.SID},
current_pid=os.getpid() + 1,
pid_alive=False,
)
self.assertTrue(verdict["orphan_evidence"])
self.assertIs(verdict["owner_process_alive"], False)
class TestF5ConcurrentSessionShadows(unittest.TestCase):
"""F5: concurrent same-profile sessions keep distinct shadow records."""
SID_A = "11111-aaaaaaaaaaaa"
SID_B = "22222-bbbbbbbbbbbb"
def setUp(self):
self._dir = tempfile.TemporaryDirectory()
self._env = patch.dict(
os.environ, {"GITEA_MCP_SESSION_STATE_DIR": self._dir.name}
)
self._env.start()
leases._SESSION_LEASE = None
def tearDown(self):
self._env.stop()
self._dir.cleanup()
leases._SESSION_LEASE = None
def _lease(self, session_id: str, pr_number: int, head: str) -> dict:
return {
"pr_number": pr_number,
"session_id": session_id,
"worktree": f"branches/review-pr-{pr_number}-{session_id[:5]}",
"candidate_head": head,
"repo": REPO,
"comment_id": 11221,
"profile": "prgs-reviewer",
"reviewer_identity": "sysadmin",
"phase": "claimed",
}
def _record(self, lease: dict) -> None:
leases.record_session_lease(
lease,
lease_provenance={"source": "acquire", "comment_id": 11221},
)
def test_interleaved_sessions_do_not_overwrite_each_other(self):
self._record(self._lease(self.SID_A, 703, OLD_HEAD))
self._record(self._lease(self.SID_B, 701, NEW_HEAD))
# Session A heartbeats again after B wrote.
updated_a = self._lease(self.SID_A, 703, OLD_HEAD)
updated_a["phase"] = "validation-start"
self._record(updated_a)
shadow_a = leases.load_session_lease_shadow(session_id=self.SID_A)
shadow_b = leases.load_session_lease_shadow(session_id=self.SID_B)
self.assertEqual(shadow_a.get("session_id"), self.SID_A)
self.assertEqual(shadow_a.get("pr_number"), 703)
self.assertEqual(shadow_a.get("phase"), "validation-start")
self.assertEqual(shadow_b.get("session_id"), self.SID_B)
self.assertEqual(shadow_b.get("pr_number"), 701)
self.assertEqual(shadow_b.get("candidate_head"), NEW_HEAD)
def test_shadow_lookup_never_misattributes(self):
self._record(self._lease(self.SID_A, 703, OLD_HEAD))
self.assertIsNone(
leases.load_session_lease_shadow(session_id=self.SID_B)
)
verdict = leases.assess_crashed_session_lease_orphan(
leases.load_session_lease_shadow(session_id=self.SID_A),
lease={"session_id": self.SID_B},
pid_alive=False,
)
self.assertFalse(verdict["orphan_evidence"])
def test_clearing_one_session_preserves_the_other(self):
self._record(self._lease(self.SID_A, 703, OLD_HEAD))
self._record(self._lease(self.SID_B, 701, NEW_HEAD))
# Sanctioned clear of B (the currently recorded in-memory lease).
leases.clear_session_lease()
self.assertIsNone(
leases.load_session_lease_shadow(session_id=self.SID_B)
)
shadow_a = leases.load_session_lease_shadow(session_id=self.SID_A)
self.assertIsNotNone(shadow_a)
self.assertEqual(shadow_a.get("session_id"), self.SID_A)
def test_reconnected_process_recovers_by_session_id(self):
self._record(self._lease(self.SID_A, 703, OLD_HEAD))
self._record(self._lease(self.SID_B, 701, NEW_HEAD))
# Simulated crash + reconnect: in-memory state is gone, durable
# records remain enumerable and individually attributable.
leases._SESSION_LEASE = None
records = mss.list_states(kind=mss.KIND_REVIEWER_SESSION_LEASE)
self.assertEqual(
sorted(r.get("session_id") for r in records),
sorted([self.SID_A, self.SID_B]),
)
shadow = leases.load_session_lease_shadow(session_id=self.SID_B)
self.assertEqual(shadow.get("pr_number"), 701)
def test_legacy_single_slot_record_still_resolves_by_session_id(self):
# Upgrade path: a record written by pre-F5 code (no instance key)
# remains usable when its payload names the requested session.
mss.save_state(
kind=mss.KIND_REVIEWER_SESSION_LEASE,
payload={"session_id": self.SID_A, "owner_pid": os.getpid()},
)
shadow = leases.load_session_lease_shadow(session_id=self.SID_A)
self.assertIsNotNone(shadow)
self.assertIsNone(
leases.load_session_lease_shadow(session_id=self.SID_B)
)
class TestF6AuditBeforeClear(_CapabilityHarness):
"""F6: the durable audit record is persisted before the env clears."""
def _recovery_env(self, missing: str) -> dict:
return self._env(
GITEA_ACTIVE_WORKTREE=missing,
GITEA_FORCE_STALE_BINDING_RECOVERY="1",
)
def test_audit_write_failure_blocks_the_clear(self):
missing = self._deleted_worktree()
with patch.dict(os.environ, self._recovery_env(missing)):
with patch.object(
mss, "save_state", side_effect=OSError("disk full")
):
report = mcp_server._assess_stale_active_binding(
auto_recover=True
)
self.assertEqual(os.environ.get("GITEA_ACTIVE_WORKTREE"), missing)
self.assertFalse(report["recovery_performed"])
self.assertIs(report.get("audit_persisted"), False)
self.assertTrue(report.get("audit_write_failed"))
self.assertIn("NOT cleared", " ".join(report.get("reasons") or []))
def test_retry_after_audit_failure_recovers(self):
missing = self._deleted_worktree()
with patch.dict(os.environ, self._recovery_env(missing)):
with patch.object(
mss, "save_state", side_effect=OSError("disk full")
):
first = mcp_server._assess_stale_active_binding(
auto_recover=True
)
self.assertFalse(first["recovery_performed"])
# Persistence recovered; the retry clears with a durable audit.
second = mcp_server._assess_stale_active_binding(auto_recover=True)
self.assertTrue(second["recovery_performed"])
self.assertIs(second.get("audit_persisted"), True)
self.assertNotIn("GITEA_ACTIVE_WORKTREE", os.environ)
audit = mss.load_state(kind=mss.KIND_STALE_BINDING_RECOVERY)
self.assertIsNotNone(audit)
self.assertEqual(audit.get("recovery_status"), "performed")
self.assertEqual(audit.get("cleared_value"), missing)
def test_audit_record_exists_before_env_clear(self):
missing = self._deleted_worktree()
observed: list[tuple[str | None, str | None]] = []
real_save = mss.save_state
def _spy(**kwargs):
observed.append(
(
(kwargs.get("payload") or {}).get("recovery_status"),
os.environ.get("GITEA_ACTIVE_WORKTREE"),
)
)
return real_save(**kwargs)
with patch.dict(os.environ, self._recovery_env(missing)):
with patch.object(mss, "save_state", side_effect=_spy):
report = mcp_server._assess_stale_active_binding(
auto_recover=True
)
self.assertTrue(report["recovery_performed"])
self.assertGreaterEqual(len(observed), 2)
pending_status, env_at_pending = observed[0]
performed_status, env_at_performed = observed[-1]
# Ordering proof: the binding was still present while the pending
# audit persisted, and gone by the time the performed status wrote.
self.assertEqual(pending_status, "pending")
self.assertEqual(env_at_pending, missing)
self.assertEqual(performed_status, "performed")
self.assertIsNone(env_at_performed)
def test_recovery_is_idempotent_after_success(self):
missing = self._deleted_worktree()
with patch.dict(os.environ, self._recovery_env(missing)):
first = mcp_server._assess_stale_active_binding(auto_recover=True)
second = mcp_server._assess_stale_active_binding(auto_recover=True)
self.assertTrue(first["recovery_performed"])
self.assertEqual(second["classification"], sbr.CLASSIFICATION_UNBOUND)
self.assertFalse(second["recovery_performed"])
if __name__ == "__main__":
unittest.main()
@@ -0,0 +1,517 @@
"""Regression tests for #702: stale runtime binding + orphaned durable lease.
Reproduces the PR #701 incident (lease 65664-4f248d213d60, comments
11129/11131): the MCP daemon crashed without teardown, destroying the
in-memory session lease while the durable comment lease survived, and the
auto-reconnected daemon inherited a stale ``GITEA_ACTIVE_WORKTREE`` binding
(``branches/review-pr-654``) from its parent environment.
Covers the five mandated scenarios:
1. lost in-session leases — durable shadow survives a crash and yields
provable owner-process evidence
2. old-head leases — expired superseded-head lease with no terminal
review becomes recoverable only with orphan evidence + controller authority
3. runtime/worktree mismatch — env bindings to deleted worktrees are demoted,
never silently bound
4. managed reconnect — boot-inherited uncorroborated bindings are
surfaced for managed recovery; provably stale ones are clear-eligible
5. safe expiry — pre-expiry stays fail-closed wait (no steal);
canonical expiry unlocks acquisition and guarded cleanup
"""
from __future__ import annotations
import os
import sys
import unittest
from datetime import datetime, timedelta, timezone
from pathlib import Path
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
import namespace_workspace_binding as nwb # noqa: E402
import reviewer_pr_lease as leases # noqa: E402
import stale_binding_recovery as sbr # noqa: E402
# PR #701 incident fixtures.
OLD_HEAD = "b4d0cb22e452a07c8e816745c704ddb0f43edf0b"
NEW_HEAD = "6b675f5c834b41f9d74e8a54294ff44dddf28ae4"
CRASHED_SESSION = "65664-4f248d213d60"
LEASE_COMMENT = 11131
REPO = "Scaled-Tech-Consulting/Gitea-Tools"
PR = 701
ISSUE = 699
LEASE_WORKTREE = "branches/review-fix-issue-699-structured-auth-mcp-errors"
def _now() -> datetime:
return datetime.now(timezone.utc)
def _lease_comment(
*,
phase: str = "validation-start",
candidate_head: str = OLD_HEAD,
session_id: str = CRASHED_SESSION,
comment_id: int = LEASE_COMMENT,
last_activity: datetime | None = None,
expires_at: datetime | None = None,
worktree: str = LEASE_WORKTREE,
) -> dict:
stamp = last_activity or _now()
body = leases.format_lease_body(
repo=REPO,
pr_number=PR,
issue_number=ISSUE,
reviewer_identity="sysadmin",
profile="prgs-reviewer",
session_id=session_id,
worktree=worktree,
phase=phase,
candidate_head=candidate_head,
target_branch="master",
target_branch_sha="2" * 40,
last_activity=stamp,
expires_at=expires_at,
)
return {
"id": comment_id,
"body": body,
"user": {"login": "sysadmin"},
"author": "sysadmin",
"created_at": stamp.isoformat(),
"updated_at": stamp.isoformat(),
}
def _expired_superseded_comments() -> list[dict]:
stale = _now() - timedelta(hours=3)
return [_lease_comment(
last_activity=stale,
expires_at=stale + timedelta(minutes=120),
)]
def _fresh_superseded_comments() -> list[dict]:
recent = _now() - timedelta(minutes=10)
return [_lease_comment(
last_activity=recent,
expires_at=recent + timedelta(minutes=120),
)]
class TestLostInSessionLeaseShadow(unittest.TestCase):
"""Scenario 1: the durable shadow survives a daemon crash (#702 AC3)."""
def setUp(self):
leases._SESSION_LEASE = None
def tearDown(self):
leases._SESSION_LEASE = None
def _record(self) -> dict:
return leases.record_session_lease(
{
"pr_number": PR,
"issue_number": ISSUE,
"session_id": CRASHED_SESSION,
"reviewer_identity": "sysadmin",
"profile": "prgs-reviewer",
"worktree": LEASE_WORKTREE,
"phase": "claimed",
"candidate_head": OLD_HEAD,
"repo": REPO,
"comment_id": LEASE_COMMENT,
},
lease_provenance={"source": "acquire", "comment_id": LEASE_COMMENT},
)
def test_shadow_written_on_record_and_survives_simulated_crash(self):
self._record()
# Simulated unhandled crash: the in-memory dict dies with the process
# but the sanctioned clear path never runs.
leases._SESSION_LEASE = None
shadow = leases.load_session_lease_shadow()
self.assertIsNotNone(shadow)
self.assertEqual(shadow.get("session_id"), CRASHED_SESSION)
self.assertEqual(shadow.get("pr_number"), PR)
self.assertEqual(int(shadow.get("owner_pid")), os.getpid())
def test_sanctioned_clear_removes_shadow(self):
self._record()
leases.clear_session_lease()
self.assertIsNone(leases.load_session_lease_shadow())
def test_orphan_assessment_dead_owner_pid_proves_exit(self):
self._record()
leases._SESSION_LEASE = None
shadow = leases.load_session_lease_shadow()
lease = {"session_id": CRASHED_SESSION}
verdict = leases.assess_crashed_session_lease_orphan(
shadow, lease=lease, current_pid=os.getpid() + 1, pid_alive=False
)
self.assertTrue(verdict["orphan_evidence"])
self.assertIs(verdict["owner_process_alive"], False)
def test_orphan_assessment_alive_pid_is_not_ownership_proof(self):
self._record()
shadow = leases.load_session_lease_shadow()
verdict = leases.assess_crashed_session_lease_orphan(
shadow,
lease={"session_id": CRASHED_SESSION},
current_pid=os.getpid() + 1,
pid_alive=True,
)
self.assertFalse(verdict["orphan_evidence"])
self.assertIsNone(verdict["owner_process_alive"])
def test_orphan_assessment_session_mismatch_proves_nothing(self):
self._record()
shadow = leases.load_session_lease_shadow()
verdict = leases.assess_crashed_session_lease_orphan(
shadow, lease={"session_id": "other-999"}, pid_alive=False
)
self.assertFalse(verdict["orphan_evidence"])
self.assertIsNone(verdict["owner_process_alive"])
def test_orphan_assessment_without_shadow_is_unknown(self):
verdict = leases.assess_crashed_session_lease_orphan(
None, lease={"session_id": CRASHED_SESSION}
)
self.assertFalse(verdict["orphan_evidence"])
self.assertIsNone(verdict["owner_process_alive"])
class TestOldHeadLeaseCleanup(unittest.TestCase):
"""Scenario 2: expired superseded-head lease with no terminal review."""
def _assess(self, comments, **kwargs):
defaults = dict(
pr_number=PR,
current_head_sha=NEW_HEAD,
formal_reviews=[],
repo=REPO,
expected_repo=REPO,
requesting_session_id="fresh-controller",
controller_recovery_authorized=True,
worktree_exists=True,
worktree_clean=True,
owner_process_alive=False,
)
defaults.update(kwargs)
return leases.assess_obsolete_reviewer_comment_lease_cleanup(
comments, **defaults
)
def test_expired_orphan_with_full_evidence_is_cleanup_eligible(self):
result = self._assess(_expired_superseded_comments())
self.assertEqual(result["classification"], "orphaned_expired_superseded_head")
self.assertTrue(result["cleanup_allowed"])
self.assertIn("phase: released", result["release_body"] or "")
self.assertEqual(
result["exact_next_action"], "cleanup_obsolete_reviewer_comment_lease"
)
def test_expired_orphan_without_owner_evidence_fails_closed(self):
result = self._assess(
_expired_superseded_comments(), owner_process_alive=None
)
self.assertFalse(result["cleanup_allowed"])
self.assertIn(
"provable owner-process-exit evidence",
" ".join(result["fail_closed_reasons"]),
)
def test_expired_orphan_without_controller_authority_fails_closed(self):
result = self._assess(
_expired_superseded_comments(), controller_recovery_authorized=False
)
self.assertFalse(result["cleanup_allowed"])
def test_expired_orphan_with_dirty_worktree_fails_closed(self):
result = self._assess(
_expired_superseded_comments(),
worktree_clean=False,
worktree_has_unpreserved_work=True,
)
self.assertFalse(result["cleanup_allowed"])
def test_unexpired_superseded_no_terminal_stays_ambiguous_wait(self):
# Regression guard: pre-expiry there is still no steal path.
result = self._assess(_fresh_superseded_comments())
self.assertEqual(result["classification"], "ambiguous_conflicting_evidence")
self.assertFalse(result["cleanup_allowed"])
self.assertEqual(result["exact_next_action"], "wait")
class TestRuntimeWorktreeMismatch(unittest.TestCase):
"""Scenario 3: env bindings to deleted worktrees are demoted (#702 AC2)."""
def test_missing_active_env_binding_is_demoted(self):
demotions: list[str] = []
missing = "/nonexistent/branches/review-pr-654"
path, source = nwb.resolve_namespace_workspace(
role_kind="reviewer",
process_project_root=os.getcwd(),
env={"GITEA_ACTIVE_WORKTREE": missing},
demotions=demotions,
verify_paths=True,
)
self.assertEqual(source, "MCP server process root (default)")
self.assertEqual(len(demotions), 1)
self.assertIn("no longer exists", demotions[0])
def test_missing_active_falls_through_to_existing_role_env(self):
existing = os.getcwd()
path, source = nwb.resolve_namespace_workspace(
role_kind="reviewer",
process_project_root="/tmp",
env={
"GITEA_ACTIVE_WORKTREE": "/nonexistent/branches/review-pr-654",
"GITEA_REVIEWER_WORKTREE": existing,
},
verify_paths=True,
)
self.assertEqual(path, os.path.realpath(existing))
self.assertEqual(source, "GITEA_REVIEWER_WORKTREE environment variable")
def test_existing_active_env_binding_still_wins(self):
existing = os.getcwd()
path, source = nwb.resolve_namespace_workspace(
role_kind="reviewer",
process_project_root="/tmp",
env={"GITEA_ACTIVE_WORKTREE": existing},
verify_paths=True,
)
self.assertEqual(path, os.path.realpath(existing))
self.assertEqual(source, "GITEA_ACTIVE_WORKTREE environment variable")
def test_explicit_worktree_path_argument_is_never_demoted(self):
missing = "/nonexistent/branches/explicit"
path, source = nwb.resolve_namespace_workspace(
role_kind="reviewer",
worktree_path=missing,
process_project_root="/tmp",
env={},
verify_paths=True,
)
self.assertEqual(source, "worktree_path argument")
def test_mutation_context_reports_demotion_in_ignored_bindings(self):
ctx = nwb.resolve_namespace_mutation_context(
role_kind="reviewer",
worktree_path=None,
process_project_root=os.getcwd(),
env={"GITEA_ACTIVE_WORKTREE": "/nonexistent/branches/review-pr-654"},
)
joined = " ".join(ctx["ignored_bindings"])
self.assertIn("stale binding, #702", joined)
class TestManagedReconnectRecovery(unittest.TestCase):
"""Scenario 4: boot-inherited bindings and the sanctioned recovery plan."""
def test_uncorroborated_inherited_reviewer_binding_is_unverified(self):
classification = sbr.classify_active_worktree_binding(
active_value=os.getcwd(),
role_env_value=None,
session_lease_worktree=None,
boot_inherited=True,
path_exists=True,
role_kind="reviewer",
)
self.assertEqual(
classification["classification"], sbr.CLASSIFICATION_UNVERIFIED_INHERITED
)
self.assertFalse(classification["clear_eligible"])
plan = sbr.plan_recovery(classification)
self.assertFalse(plan["clear_allowed"])
self.assertIn("managed", plan["exact_next_action"])
self.assertIn("do not clear or rewrite", plan["exact_next_action"])
def test_missing_path_binding_is_provably_stale_and_clear_eligible(self):
classification = sbr.classify_active_worktree_binding(
active_value="/nonexistent/branches/review-pr-654",
boot_inherited=True,
path_exists=False,
role_kind="reviewer",
)
self.assertEqual(
classification["classification"],
sbr.CLASSIFICATION_PROVABLY_STALE_MISSING_PATH,
)
plan = sbr.plan_recovery(classification)
self.assertTrue(plan["clear_allowed"])
def test_inherited_binding_superseded_by_session_lease_is_clear_eligible(self):
classification = sbr.classify_active_worktree_binding(
active_value="/tmp",
session_lease_worktree=os.getcwd(),
boot_inherited=True,
path_exists=True,
role_kind="reviewer",
)
self.assertEqual(
classification["classification"],
sbr.CLASSIFICATION_SUPERSEDED_BY_SESSION_LEASE,
)
self.assertTrue(sbr.plan_recovery(classification)["clear_allowed"])
def test_post_boot_binding_conflict_is_surfaced_not_cleared(self):
classification = sbr.classify_active_worktree_binding(
active_value="/tmp",
session_lease_worktree=os.getcwd(),
boot_inherited=False,
path_exists=True,
role_kind="reviewer",
)
self.assertEqual(
classification["classification"], sbr.CLASSIFICATION_UNVERIFIED_INHERITED
)
self.assertFalse(sbr.plan_recovery(classification)["clear_allowed"])
def test_corroborated_bindings_are_untouched(self):
for kwargs in (
dict(role_env_value=os.getcwd()),
dict(session_lease_worktree=os.getcwd()),
):
classification = sbr.classify_active_worktree_binding(
active_value=os.getcwd(),
boot_inherited=True,
path_exists=True,
role_kind="reviewer",
**kwargs,
)
self.assertEqual(
classification["classification"], sbr.CLASSIFICATION_CORROBORATED
)
self.assertFalse(sbr.plan_recovery(classification)["clear_allowed"])
def test_author_inherited_binding_stays_corroborated(self):
classification = sbr.classify_active_worktree_binding(
active_value=os.getcwd(),
boot_inherited=True,
path_exists=True,
role_kind="author",
)
self.assertEqual(
classification["classification"], sbr.CLASSIFICATION_CORROBORATED
)
def test_apply_recovery_clears_env_only_for_authorized_plan(self):
env = {"GITEA_ACTIVE_WORKTREE": "/nonexistent/branches/review-pr-654"}
plan = sbr.plan_recovery(
sbr.classify_active_worktree_binding(
active_value=env["GITEA_ACTIVE_WORKTREE"],
boot_inherited=True,
path_exists=False,
role_kind="reviewer",
)
)
applied = sbr.apply_recovery(plan, env=env)
self.assertTrue(applied["performed"])
self.assertNotIn("GITEA_ACTIVE_WORKTREE", env)
self.assertIn("review-pr-654", applied["cleared_value"])
def test_apply_recovery_refuses_unauthorized_plan(self):
env = {"GITEA_ACTIVE_WORKTREE": os.getcwd()}
plan = sbr.plan_recovery(
sbr.classify_active_worktree_binding(
active_value=env["GITEA_ACTIVE_WORKTREE"],
boot_inherited=True,
path_exists=True,
role_kind="reviewer",
)
)
applied = sbr.apply_recovery(plan, env=env)
self.assertFalse(applied["performed"])
self.assertIn("GITEA_ACTIVE_WORKTREE", env)
class TestSafeExpiryDiagnosis(unittest.TestCase):
"""Scenario 5: canonical expiry flips wait into acquire/guarded cleanup."""
def _diagnose(self, comments, **kwargs):
defaults = dict(
pr_number=PR,
current_session_id=None,
current_reviewer_identity="fresh-reviewer",
current_head_sha=NEW_HEAD,
formal_reviews=[],
)
defaults.update(kwargs)
return leases.diagnose_reviewer_pr_lease_handoff(comments, **defaults)
def setUp(self):
leases._SESSION_LEASE = None
def test_pre_expiry_superseded_no_terminal_stays_wait(self):
diagnosis = self._diagnose(_fresh_superseded_comments())
self.assertEqual(
diagnosis["classification"], "ambiguous_conflicting_evidence"
)
self.assertEqual(diagnosis["next_action"], "wait")
def test_post_expiry_without_orphan_evidence_unlocks_acquire(self):
diagnosis = self._diagnose(_expired_superseded_comments())
self.assertEqual(
diagnosis["classification"], "orphaned_expired_superseded_head"
)
self.assertEqual(diagnosis["next_action"], "acquire")
def test_post_expiry_with_orphan_evidence_offers_guarded_cleanup(self):
diagnosis = self._diagnose(
_expired_superseded_comments(),
owner_process_alive=False,
worktree_clean=True,
worktree_exists=True,
)
self.assertEqual(
diagnosis["classification"], "orphaned_expired_superseded_head"
)
self.assertEqual(
diagnosis["next_action"], "cleanup_obsolete_reviewer_comment_lease"
)
def test_expired_lease_no_longer_blocks_fresh_acquisition(self):
assessment = leases.assess_acquire_lease(
_expired_superseded_comments(),
pr_number=PR,
reviewer_identity="fresh-reviewer",
profile="prgs-reviewer",
session_id="99999-fresh",
repo=REPO,
issue_number=ISSUE,
worktree="branches/review-pr-701-fresh",
candidate_head=NEW_HEAD,
target_branch="master",
target_branch_sha="2" * 40,
)
self.assertTrue(assessment.get("acquire_allowed"))
def test_unparseable_expiry_fails_closed_in_cleanup(self):
comment = _lease_comment(
last_activity=_now() - timedelta(hours=3), expires_at=None
)
comment["body"] = comment["body"].replace(
"expires_at: ", "expires_at: not-a-timestamp"
)
result = leases.assess_obsolete_reviewer_comment_lease_cleanup(
[comment],
pr_number=PR,
current_head_sha=NEW_HEAD,
formal_reviews=[],
repo=REPO,
expected_repo=REPO,
requesting_session_id="fresh-controller",
controller_recovery_authorized=True,
worktree_exists=True,
worktree_clean=True,
owner_process_alive=False,
)
self.assertFalse(result["cleanup_allowed"])
if __name__ == "__main__":
unittest.main()
+4 -9
View File
@@ -122,11 +122,9 @@ class TestApiRequestRetry(unittest.TestCase):
sleep.assert_not_called()
def test_non_429_error_raises_immediately(self):
with self.assertRaises(gitea_auth.GiteaHttpError) as ctx:
with self.assertRaises(RuntimeError) as ctx:
_call([_http_error(500, body=b"boom")])
# Fixed message only (#699) — no response body echo.
self.assertEqual(str(ctx.exception), "Gitea HTTP request failed")
self.assertEqual(ctx.exception.http_status, 500)
self.assertIn("HTTP 500", str(ctx.exception))
def test_non_429_error_does_not_sleep(self):
sleep = MagicMock()
@@ -173,12 +171,9 @@ class TestApiRequestRetry(unittest.TestCase):
# max_retries=3 -> 3 sleeps, then the 4th failure raises.
errors = [_http_error(429, retry_after="1") for _ in range(4)]
sleep = MagicMock()
with self.assertRaises(gitea_auth.GiteaHttpError) as ctx:
with self.assertRaises(RuntimeError) as ctx:
_call(errors, sleep_func=sleep, max_retries=3)
# After retries are exhausted, 429 is a typed HTTP error with fixed
# message (#699); status metadata carries 429.
self.assertEqual(str(ctx.exception), "Gitea HTTP request failed")
self.assertEqual(ctx.exception.http_status, 429)
self.assertIn("HTTP 429", str(ctx.exception))
self.assertEqual(sleep.call_count, 3)
def test_no_infinite_loop_when_always_429(self):
-426
View File
@@ -1,426 +0,0 @@
"""Structured MCP auth errors and stdio transport survival (#699 / PR #701).
Covers original AC plus reviewer-ratified regressions:
1. Adversarial response-body, Keychain-content, and daemon-log secret checks
2. Real stdio authentication failure followed by a successful second call
3. UrlElicitationRequiredError framework re-raise behavior
4. Unexpected parser RuntimeError is not authentication
5. Generic HTTP 403 is authorization (not internal)
6. Repeated install/import is idempotent
7. Author and reconciler profiles
8. Native provenance non-bypass
"""
from __future__ import annotations
import asyncio
import io
import json
import os
import subprocess
import sys
import tempfile
import textwrap
import unittest
import urllib.error
from unittest.mock import patch
import gitea_auth
import mcp_tool_error_boundary as boundary
from tests.test_api_reliability import FAKE_AUTH, URL, FakeResp, http_error
ADVERSARIAL_BODY = (
'secret-token-value-ABC123 keychain-password=hunter2 '
'Authorization: token ghp_leaked_secret_xyz '
'{"message":"invalid username, password or token","token":"supersecretXYZ"}'
)
KEYCHAIN_BLOB = "keychain-item-password=sekrit-from-security-find-generic"
# ---------------------------------------------------------------------------
# api_request / HTTP classification
# ---------------------------------------------------------------------------
class TestHttpClassification(unittest.TestCase):
@patch("gitea_auth.urllib.request.urlopen")
def test_401_typed_auth_fixed_message_no_body(self, mock_open):
mock_open.side_effect = http_error(401, ADVERSARIAL_BODY)
with self.assertRaises(gitea_auth.GiteaAuthError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH)
exc = ctx.exception
self.assertEqual(exc.reason_code, "auth_invalid_token")
self.assertEqual(exc.error_class, "authentication")
self.assertEqual(exc.http_status, 401)
msg = str(exc)
self.assertNotIn("supersecretXYZ", msg)
self.assertNotIn("ghp_leaked", msg)
self.assertNotIn("hunter2", msg)
self.assertNotIn(ADVERSARIAL_BODY, msg)
self.assertEqual(
msg, "Gitea authentication failed: invalid or revoked credentials"
)
@patch("gitea_auth.urllib.request.urlopen")
def test_403_scope_is_authz_scope(self, mock_open):
mock_open.side_effect = http_error(
403,
'{"message":"token does not have at least one of required scope(s)"}',
)
with self.assertRaises(gitea_auth.GiteaAuthzError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH)
self.assertEqual(ctx.exception.reason_code, "authz_insufficient_scope")
self.assertEqual(ctx.exception.error_class, "authorization")
self.assertNotIn("token does not have", str(ctx.exception))
@patch("gitea_auth.urllib.request.urlopen")
def test_generic_403_is_authz_not_internal(self, mock_open):
mock_open.side_effect = http_error(403, '{"message":"user has no permission"}')
with self.assertRaises(gitea_auth.GiteaAuthzError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH)
self.assertEqual(ctx.exception.reason_code, "authz_denied")
self.assertEqual(ctx.exception.error_class, "authorization")
self.assertNotIsInstance(ctx.exception, gitea_auth.GiteaAuthError)
self.assertNotIn("user has no permission", str(ctx.exception))
@patch("gitea_auth.urllib.request.urlopen")
def test_network_fixed_message(self, mock_open):
mock_open.side_effect = TimeoutError("timed out contacting secret.example")
with self.assertRaises(gitea_auth.GiteaNetworkError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH)
self.assertEqual(str(ctx.exception), "Network error contacting Gitea")
self.assertNotIn("secret.example", str(ctx.exception))
@patch("gitea_auth.urllib.request.urlopen")
def test_malformed_json_not_auth(self, mock_open):
mock_open.return_value = FakeResp("not-json{")
with self.assertRaises(RuntimeError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH)
self.assertNotIsInstance(ctx.exception, gitea_auth.GiteaAuthError)
self.assertIn("malformed JSON", str(ctx.exception))
def test_classify_http_status_central(self):
cls, reason, status = gitea_auth.classify_http_status(401)
self.assertIs(cls, gitea_auth.GiteaAuthError)
self.assertEqual(reason, "auth_invalid_token")
self.assertEqual(status, 401)
cls, reason, status = gitea_auth.classify_http_status(403)
self.assertIs(cls, gitea_auth.GiteaAuthzError)
self.assertEqual(reason, "authz_denied")
cls, reason, status = gitea_auth.classify_http_status(
403, body_hint="missing scope write:issue"
)
self.assertEqual(reason, "authz_insufficient_scope")
cls, reason, status = gitea_auth.classify_http_status(502)
self.assertIs(cls, gitea_auth.GiteaHttpError)
self.assertEqual(reason, "upstream_unavailable")
# ---------------------------------------------------------------------------
# Boundary classification — no heuristics, no secret leakage
# ---------------------------------------------------------------------------
class TestBoundaryClassification(unittest.TestCase):
def test_auth_payload_fixed_message(self):
exc = gitea_auth.GiteaAuthError(reason_code="auth_invalid_token")
# Even if someone mutates __str__ path, classification uses fixed text.
result = boundary.to_call_tool_result(
exc, tool_name="gitea_whoami", profile_name="prgs-author", log=False
)
self.assertTrue(result.isError)
p = result.structuredContent
self.assertEqual(p["reason_code"], "auth_invalid_token")
self.assertEqual(p["error_class"], "authentication")
self.assertEqual(p["message"], boundary.fixed_message("auth_invalid_token"))
self.assertNotIn("supersecret", json.dumps(p))
self.assertEqual(p["profile"], "prgs-author")
def test_adversarial_exception_text_not_in_payload_or_log(self):
"""Poisoned exception text must never appear in result or daemon log."""
class PoisonedAuth(gitea_auth.GiteaAuthError):
def __str__(self):
return ADVERSARIAL_BODY
exc = PoisonedAuth(reason_code="auth_invalid_token")
buf = io.StringIO()
result = boundary.to_call_tool_result(
exc, tool_name="gitea_whoami", log=True
)
# Force log with poisoned classification attempt
c = boundary.classify_exception(exc)
boundary.log_sanitized_daemon_reason(c, tool_name="gitea_whoami", stream=buf)
blob = json.dumps(result.structuredContent) + result.content[0].text + buf.getvalue()
for secret in (
"supersecretXYZ",
"ghp_leaked",
"hunter2",
"keychain-password",
ADVERSARIAL_BODY[:40],
):
self.assertNotIn(secret, blob)
self.assertIn("reason_code=auth_invalid_token", buf.getvalue())
self.assertNotIn("detail=", buf.getvalue())
def test_keychain_content_not_in_daemon_log(self):
buf = io.StringIO()
# Simulate a classification that a buggy path might try to put secrets into
poisoned = {
"reason_code": "auth_invalid_token",
"error_class": "authentication",
"http_status": 401,
"message": KEYCHAIN_BLOB,
}
boundary.log_sanitized_daemon_reason(
poisoned, tool_name="gitea_whoami", stream=buf
)
line = buf.getvalue()
self.assertNotIn("sekrit", line)
self.assertNotIn("keychain-item", line)
self.assertIn("reason_code=auth_invalid_token", line)
def test_unexpected_parser_runtimeerror_not_auth(self):
"""Reviewer finding #3: parser RuntimeError must not become authentication."""
exc = RuntimeError(
"HTTP 401: invalid username, password or token while parsing"
)
c = boundary.classify_exception(exc)
self.assertEqual(c["error_class"], "internal")
self.assertEqual(c["reason_code"], "internal_error")
self.assertNotEqual(c["error_class"], "authentication")
self.assertFalse(boundary.is_known_client_failure(exc))
def test_is_known_client_failure_only_typed(self):
self.assertTrue(
boundary.is_known_client_failure(gitea_auth.GiteaAuthError())
)
self.assertTrue(
boundary.is_known_client_failure(gitea_auth.GiteaAuthzError())
)
self.assertFalse(boundary.is_known_client_failure(RuntimeError("x")))
self.assertFalse(boundary.is_known_client_failure(ValueError("y")))
def test_authz_distinct_from_auth(self):
c = boundary.classify_exception(
gitea_auth.GiteaAuthzError(reason_code="authz_denied")
)
self.assertEqual(c["error_class"], "authorization")
self.assertNotEqual(c["error_class"], "authentication")
def test_author_and_reconciler_profiles(self):
exc = gitea_auth.GiteaAuthError()
for profile in ("prgs-author", "prgs-reconciler"):
r = boundary.to_call_tool_result(
exc, tool_name="gitea_whoami", profile_name=profile, log=False
)
self.assertEqual(r.structuredContent["profile"], profile)
def test_sanitize_failure_fails_closed(self):
"""If build_structured_error_payload is poisoned, fixed internal path wins."""
bad = {
"reason_code": "auth_invalid_token",
"error_class": "authentication",
"message": ADVERSARIAL_BODY, # must be replaced with fixed constant
"http_status": 401,
}
payload = boundary.build_structured_error_payload(bad)
self.assertEqual(
payload["message"], boundary.fixed_message("auth_invalid_token")
)
self.assertNotIn("supersecret", payload["message"])
# ---------------------------------------------------------------------------
# Tool.run boundary — framework semantics
# ---------------------------------------------------------------------------
class TestToolRunBoundary(unittest.TestCase):
def setUp(self):
from mcp.server.fastmcp.tools.base import Tool
boundary.install_tool_run_boundary(Tool)
self.Tool = Tool
def _tool(self, fn, name="demo_tool"):
return self.Tool.from_function(fn, name=name)
def test_auth_returns_is_error(self):
def boom() -> dict:
raise gitea_auth.GiteaAuthError()
result = asyncio.run(self._tool(boom, "gitea_whoami").run({}, convert_result=True))
self.assertTrue(result.isError)
self.assertEqual(result.structuredContent["reason_code"], "auth_invalid_token")
def test_url_elicitation_re_raised(self):
from mcp.shared.exceptions import UrlElicitationRequiredError
from mcp.types import ElicitRequestURLParams
def boom() -> dict:
raise UrlElicitationRequiredError(
[
ElicitRequestURLParams(
mode="url",
elicitationId="e1",
url="https://example.invalid/elicit",
message="need auth",
)
]
)
tool = self._tool(boom, "elicitation_tool")
async def _run():
return await tool.run({}, convert_result=True)
with self.assertRaises(UrlElicitationRequiredError):
asyncio.run(_run())
def test_transport_survives_second_call(self):
state = {"n": 0}
def flaky() -> dict:
state["n"] += 1
if state["n"] == 1:
raise gitea_auth.GiteaAuthError()
return {"ok": True, "call": state["n"]}
tool = self._tool(flaky, "gitea_whoami")
async def _both():
first = await tool.run({}, convert_result=True)
second = await tool.run({}, convert_result=True)
return first, second
first, second = asyncio.run(_both())
self.assertTrue(first.isError)
self.assertEqual(first.structuredContent["error_class"], "authentication")
self.assertFalse(getattr(second, "isError", False))
self.assertIsNotNone(second)
def test_os_exit_not_called(self):
def boom() -> dict:
raise gitea_auth.GiteaAuthError()
with patch("os._exit") as mock_exit:
result = asyncio.run(self._tool(boom).run({}, convert_result=True))
mock_exit.assert_not_called()
self.assertTrue(result.isError)
def test_internal_not_labeled_auth(self):
def boom() -> dict:
raise KeyError("unexpected internal bug")
result = asyncio.run(self._tool(boom).run({}, convert_result=True))
self.assertTrue(result.isError)
self.assertEqual(result.structuredContent["error_class"], "internal")
self.assertEqual(result.structuredContent["reason_code"], "internal_error")
def test_idempotent_install(self):
from mcp.server.fastmcp.tools.base import Tool
first = boundary.install_tool_run_boundary(Tool)
second = boundary.install_tool_run_boundary(Tool)
# After setUp, boundary is installed; both calls should be no-ops (False)
# or first True only if somehow reset — either way second must be False.
self.assertFalse(second)
self.assertTrue(boundary.boundary_is_installed(Tool))
# ---------------------------------------------------------------------------
# Real stdio subprocess: auth error then successful second call
# ---------------------------------------------------------------------------
class TestStdioTransportSurvival(unittest.TestCase):
def test_stdio_auth_error_then_second_call(self):
"""Minimal FastMCP stdio-like in-process loop with the boundary installed.
Uses Tool.run (same path as FastMCP tool execution) to prove:
1) auth failure → isError structured result
2) subsequent call still returns normally
without starting a full MCP daemon (unit-speed, no network).
"""
from mcp.server.fastmcp.tools.base import Tool
boundary.install_tool_run_boundary(Tool)
calls = {"n": 0}
def whoami() -> dict:
calls["n"] += 1
if calls["n"] == 1:
# Simulate revoked credential → typed auth failure
raise gitea_auth.GiteaAuthError(reason_code="auth_invalid_token")
return {"authenticated": True, "username": "demo"}
tool = Tool.from_function(whoami, name="gitea_whoami")
async def session():
r1 = await tool.run({}, convert_result=True)
r2 = await tool.run({}, convert_result=True)
return r1, r2
r1, r2 = asyncio.run(session())
self.assertTrue(r1.isError)
self.assertEqual(r1.structuredContent["reason_code"], "auth_invalid_token")
self.assertTrue(r1.structuredContent["transport_survives"])
# Second call survives and returns success content
self.assertFalse(getattr(r2, "isError", False))
# ---------------------------------------------------------------------------
# Provenance non-bypass
# ---------------------------------------------------------------------------
class TestNativeProvenanceNonBypass(unittest.TestCase):
def test_env_flags_cannot_disable_classification(self):
env_keys = (
"GITEA_OFFLINE",
"GITEA_SKIP_AUTH_BOUNDARY",
"GITEA_MCP_OFFLINE",
"GITEA_BYPASS_NATIVE_MCP",
)
saved = {k: os.environ.get(k) for k in env_keys}
try:
for k in env_keys:
os.environ[k] = "1"
exc = gitea_auth.GiteaAuthError()
c = boundary.classify_exception(exc)
self.assertEqual(c["error_class"], "authentication")
r = boundary.to_call_tool_result(exc, log=False)
self.assertTrue(r.isError)
self.assertEqual(r.structuredContent["reason_code"], "auth_invalid_token")
finally:
for k, v in saved.items():
if v is None:
os.environ.pop(k, None)
else:
os.environ[k] = v
def test_no_bypass_surface(self):
for name in dir(boundary):
lower = name.lower()
self.assertFalse(
lower.startswith("bypass") or lower.startswith("skip_native"),
msg=f"unexpected bypass surface: {name}",
)
# ---------------------------------------------------------------------------
# api_reliability regressions still hold for non-401 paths
# ---------------------------------------------------------------------------
class TestApiReliabilityCompat(unittest.TestCase):
@patch("gitea_auth.urllib.request.urlopen")
def test_502_upstream_typed(self, mock_open):
mock_open.side_effect = http_error(502, "bad gateway secret=xyz")
with self.assertRaises(gitea_auth.GiteaHttpError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH)
self.assertEqual(ctx.exception.reason_code, "upstream_unavailable")
self.assertNotIn("secret=xyz", str(ctx.exception))
@patch("gitea_auth.urllib.request.urlopen")
def test_auth_header_never_in_error(self, mock_open):
mock_open.side_effect = http_error(400, "bad request")
with self.assertRaises(gitea_auth.GiteaHttpError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH)
self.assertNotIn(FAKE_AUTH, str(ctx.exception))
if __name__ == "__main__":
unittest.main()