Compare commits

...
Author SHA1 Message Date
sysadmin 1421fa8568 Fix #718: Implement gitea_acquire_merger_pr_lease
This implements the native merger lease acquisition tool to allow a merger session to acquire its own lease when a reviewer lease does not exist or has expired, unblocking #679 adoption.
2026-07-17 01:30:12 -04:00
sysadmin 67e4a2b5e9 Merge pull request 'fix(session): keep KIND_DECISION_LOCK durable past generic 4h TTL (Closes #720)' (#721) from fix/issue-720-expired-decision-lock into master 2026-07-16 19:48:07 -05:00
sysadminandClaude Opus 4.8 ba7915452e fix(capability-map): restore reviewer role on 10 review tasks (break-glass, incident #722)
EXCEPTIONAL OPERATOR-AUTHORIZED BREAK-GLASS RECOVERY - incident #722.

Commit 970e68b ("fix: adopt_merger_pr_lease requires merger role") was an
unreviewed direct push that flipped 11 task_capability_map.py entries to
role="merger" instead of the intended 1. Merger profiles forbid the review
permissions, so no configured profile could resolve review_pr / approve_pr /
request_changes_pr (matching_configured_profile was empty repository-wide)
and no fix PR could be formally reviewed. The operator resolved the catch-22
with a one-time break-glass authorization recorded on issue #722
(comment 11918); this commit is the authorized minimum and nothing more.

Changes:
- task_capability_map.py restored to blob 02826af (the preserved intended
  correction, local commit c071f8a1): the 10 regressive entries return to
  role="reviewer"; adopt_merger_pr_lease keeps role="merger"; merge_pr is
  untouched.
- tests/test_task_capability_role_invariants.py added (#723 AC1/AC2):
  profile-coverage invariant for formal review tasks, map/router agreement,
  merger-only adopt_merger_pr_lease and merge_pr, merger profiles cannot
  hold review permissions.

Validation: focused role-mapping tests 72 passed (+22 subtests); full suite
2900 passed, 6 skipped, 1 known warning, 195 subtests.

Recovery step 1 for incident #722; code-defect follow-ups tracked in #723.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01EnHNSVQvJ8nCk7KZ9kL4Ym
2026-07-16 18:38:17 -04:00
jcwalker3andsysadmin 293808b42d docs: ADR for stable MCP control runtime vs dev runtime (#615) (#616)
Co-authored-by: jcwalker3 <[email protected]>
2026-07-16 14:58:43 -05:00
sysadmin 970e68bddb fix: adopt_merger_pr_lease requires merger role 2026-07-16 15:52:58 -04:00
sysadmin 80f59b334e fix(session): keep KIND_DECISION_LOCK durable past generic 4h TTL (Closes #720)
Terminal review-decision ledgers are recovery-critical provenance, not
disposable session cache. A generic four-hour TTL previously made
fresh_review_on_current_head_allowed unreachable after age expiry on open
PRs (PR #616 / review 443 reproduction).

- Classify KIND_DECISION_LOCK as RECOVERY_CRITICAL so load/mark_final work
  after >4h without hand-editing session-state files
- Stamp kind + recovery_critical on save for compatibility
- Add inspect_state_envelope so assessment reports on-disk evidence instead
  of silent "no lock" when TTL would hide non-critical kinds
- Surface disk_inspect on stale decision-lock cleanup assessment

Preserves same-head #332 hard-stop, #620 head-scoped fresh review, #594
moot cleanup, and #709 irrecoverable authorization (no ordinary-profile
permission grant).
2026-07-16 15:47:52 -04:00
sysadmin 77746b08fc Merge pull request 'feat(profiles): support reconciler cleanup capability and migration (Closes #687)' (#688) from feat/issue-687-reconciler-branch-delete into master 2026-07-16 03:23:03 -05:00
sysadmin 1c37e62014 fix(cleanup): harden readback scope and ownership fail-closed (#687)
Require branch-scoped post-delete not-found (not generic/repo/host 404),
emit consistent top-level cleanup fields, fail closed on ownership inventory
errors, never auto-reclaim expired control-plane leases, include active
comment-backed reviewer leases, apply the same gates to reconcile_merged_cleanups,
and match ownership with normalized host identity.
2026-07-16 01:21:18 -04:00
sysadmin 137426f7ad fix(cleanup): post-delete readback and active ownership gates (#687)
Require authoritative not-found readback after merged-PR branch DELETE, and
block cleanup when active author/reviewer/merger/controller/reconciler
session, lease, or worktree-binding ownership still uses the target branch.
2026-07-16 00:57:30 -04:00
sysadmin 5b0d7aed0a Merge pull request 'fix: map Gitea auth failures to structured MCP tool errors (Closes #699)' (#701) from fix/issue-699-structured-auth-mcp-errors into master 2026-07-15 23:48:06 -05:00
sysadmin 1ab384adc9 Merge pull request 'fix(workflow): cross-profile decision-lock cleanup and irrecoverable provenance (Closes #709)' (#710) from fix/issue-709-decision-lock-cross-profile into master 2026-07-15 21:24:42 -05:00
sysadmin 6b675f5c83 fix: harden structured auth MCP errors against reviewer findings (#699)
Address PR #701 request-changes-class defects:

1. Fixed messages only — never embed HTTP bodies, Keychain, or exception
   text in tool results or daemon logs; sanitization fails closed.
2. Narrow Tool.run boundary wraps original success path; re-raises
   UrlElicitationRequiredError; install is idempotent.
3. No RuntimeError substring heuristics; only typed client failures
   are classified as auth/authz/network/config.
4. Central classify_http_status — every HTTP 403 is GiteaAuthzError.

Regressions cover adversarial secrets, stdio survival, elicitation,
parser RuntimeError, generic 403, repeated install, profiles, provenance.

Closes #699
2026-07-13 13:40:38 -04:00
sysadmin b4d0cb22e4 fix: map Gitea auth failures to structured MCP tool errors (Closes #699)
HTTP 401/scope-403/network failures become typed client errors with stable
reason codes. The FastMCP Tool.run boundary returns CallToolResult isError
payloads so stdio transport survives; daemon logs use sanitized reason codes
only. Regression tests cover author/reconciler profiles, transport survival,
and non-misclassification of unexpected exceptions.

Cross-links: #685, #695, #697, #698, PR #696 (scopes not absorbed).
2026-07-13 13:10:00 -04:00
sysadmin 4a63578003 fix(profiles): canonical reconciler ops and guard raw branch delete (#687)
Address REQUEST_CHANGES on PR #688:

- migrate_profiles: emit fully canonical reconciler/author/reviewer defaults;
  canonicalize explicit ops; fail if reconciler loses required pr.close/read
- gitea_delete_branch: deny reconciler (and non-author roles); refuse
  preservation/protected branches before any API call
- gitea_cleanup_merged_pr_branch: require reconciler role only; block
  preservation/evidence branches via branch_cleanup_guard
- docs: end-to-end operator runbook for profile migrate/apply/reconnect/cleanup
- tests: migration canonicalization, idempotency, raw-delete denial, guarded
  cleanup success and unmerged/preserve rejections

Closes #687.
2026-07-12 22:35:14 -04:00
sysadmin c7a444eb4b feat(profiles): support reconciler role in migrate_profiles (Closes #687) 2026-07-12 21:32:01 -04:00
sysadminandClaude Opus 4.8 8cac50b2e7 feat(profiles): make gitea.branch.delete a documented reconciler-owned capability
Merged-PR source-branch cleanup is reconciler work (task_capability_map maps
cleanup_merged_pr_branch -> reconciler / gitea.branch.delete), but the
reconciler profile schema, execution-profile docs, and tests never covered
the permission, so no configured profile could run the guarded
gitea_cleanup_merged_pr_branch path.

- reconciler_profile.py: add gitea.branch.delete to
  RECONCILER_RECOMMENDED_OPERATIONS (not required; not forbidden)
- docs/gitea-execution-profiles.md: document merged-branch cleanup
  ownership, least-privilege constraints, and the no-alias caveat
- tests/test_reconciler_profile.py: reconciler profile with branch.delete
  stays valid and classified reconciler; missing grant is reported as
  missing-recommended
- tests/test_branch_cleanup_guard.py: author- and merger-shaped profiles
  without gitea.branch.delete fail closed on gitea_cleanup_merged_pr_branch

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
2026-07-12 21:31:00 -04:00
26 changed files with 5451 additions and 164 deletions
+505
View File
@@ -7,6 +7,19 @@ from typing import Any
PROTECTED_BRANCHES = frozenset({"master", "main", "dev"})
# Evidence / preservation branches must never be removed by cleanup tools
# (e.g. chore/issue-681-preserve-review-session-wip).
_PRESERVATION_MARKERS = ("preserve", "preservation", "evidence")
def is_preservation_or_evidence_branch(branch: str | None) -> bool:
"""Return True when *branch* is a preservation/evidence ref that must stay."""
if not branch:
return False
name = str(branch).lower()
return any(marker in name for marker in _PRESERVATION_MARKERS)
_RAW_BRANCH_DELETE_PATTERNS = (
re.compile(r"\bgit(?:\s+-C\s+\S+)?\s+branch\s+-[dD]\b[^\n\r]*", re.I),
re.compile(r"\bgit(?:\s+-C\s+\S+)?\s+push\b[^\n\r]*\s--delete\b[^\n\r]*", re.I),
@@ -72,6 +85,11 @@ def assess_merged_pr_branch_cleanup(
reasons.append("PR head branch is missing")
if head_branch in protected:
reasons.append(f"branch '{head_branch}' is protected")
if is_preservation_or_evidence_branch(head_branch):
reasons.append(
f"branch '{head_branch}' is a preservation/evidence branch and "
"cannot be deleted through merged-PR cleanup"
)
if head_branch in open_pr_heads:
reasons.append("an open PR still references this head branch")
if head_on_target is False:
@@ -96,3 +114,490 @@ def assess_merged_pr_branch_cleanup(
"block_reasons": reasons,
"recommended_action": "delete_remote_branch" if safe else "keep_remote_branch",
}
# ---------------------------------------------------------------------------
# #687 remediation: post-delete readback + active ownership protection
# ---------------------------------------------------------------------------
READBACK_NOT_FOUND = "not_found"
READBACK_EXISTS = "exists"
READBACK_AUTHENTICATION = "authentication_error"
READBACK_AUTHORIZATION = "authorization_error"
READBACK_TRANSPORT = "transport_error"
READBACK_UNEXPECTED = "unexpected_response"
READBACK_AMBIGUOUS_404 = "ambiguous_not_found"
# Scope of a 404: only branch-scoped absence may set verified_absent.
NOT_FOUND_SCOPE_BRANCH = "branch"
NOT_FOUND_SCOPE_REPOSITORY = "repository"
NOT_FOUND_SCOPE_HOST = "host"
NOT_FOUND_SCOPE_UNKNOWN = "unknown"
ERROR_CLASS_AUTHENTICATION = "authentication"
ERROR_CLASS_AUTHORIZATION = "authorization"
ERROR_CLASS_TRANSPORT = "transport"
ERROR_CLASS_UNEXPECTED = "unexpected"
OWNERSHIP_CATEGORY_AUTHOR_SESSION = "author_session"
OWNERSHIP_CATEGORY_AUTHOR_LEASE = "author_lease"
OWNERSHIP_CATEGORY_REVIEWER_LEASE = "reviewer_lease"
OWNERSHIP_CATEGORY_MERGER_LEASE = "merger_lease"
OWNERSHIP_CATEGORY_CONTROLLER_LEASE = "controller_lease"
OWNERSHIP_CATEGORY_RECONCILER_LEASE = "reconciler_lease"
OWNERSHIP_CATEGORY_WORKTREE_BINDING = "worktree_binding"
OWNERSHIP_CATEGORY_INVENTORY_ERROR = "ownership_inventory_error"
_ROLE_TO_OWNERSHIP_CATEGORY = {
"author": OWNERSHIP_CATEGORY_AUTHOR_LEASE,
"reviewer": OWNERSHIP_CATEGORY_REVIEWER_LEASE,
"merger": OWNERSHIP_CATEGORY_MERGER_LEASE,
"controller": OWNERSHIP_CATEGORY_CONTROLLER_LEASE,
"reconciler": OWNERSHIP_CATEGORY_RECONCILER_LEASE,
}
_ACTIVE_OWNERSHIP_STATUSES = frozenset(
{"active", "live", "claimed", "in_progress", "working", "pushing", "pushed"}
)
_TERMINAL_OWNERSHIP_STATUSES = frozenset(
{"released", "abandoned", "done", "blocked", "terminal", "closed"}
)
_EXPIRED_STATUSES = frozenset({"expired"})
_STALE_STATUSES = frozenset({"stale", "stale_dead_process", "stale_missing_worktree"})
def _norm_str(value: Any) -> str:
return str(value or "").strip()
def normalize_host(host: str | None) -> str:
"""Normalize a host identity for ownership matching (no credentials)."""
text = _norm_str(host).lower()
for prefix in ("https://", "http://"):
if text.startswith(prefix):
text = text[len(prefix) :]
# Drop path/query if a full URL slipped through.
text = text.split("/", 1)[0]
text = text.split("?", 1)[0]
return text.rstrip(".")
def ownership_category_for_role(role: str | None) -> str:
"""Map a role kind to a non-secret ownership category label."""
key = _norm_str(role).lower()
return _ROLE_TO_OWNERSHIP_CATEGORY.get(key, f"{key or 'unknown'}_lease")
def classify_branch_readback_http_status(
status_code: int | None,
*,
not_found_scope: str | None = None,
) -> dict[str, Any]:
"""Classify a GET-branch HTTP status into a secret-free readback result.
R1: A bare/generic/repository/wrong-host 404 never yields
``verified_absent=True``. Only an authoritative *branch-scoped* not-found
(``not_found_scope='branch'``) may verify deletion.
"""
if status_code == 404:
scope = _norm_str(not_found_scope).lower() or NOT_FOUND_SCOPE_UNKNOWN
if scope == NOT_FOUND_SCOPE_BRANCH:
return {
"status": READBACK_NOT_FOUND,
"error_class": None,
"verified_absent": True,
"branch_present": False,
"not_found_scope": NOT_FOUND_SCOPE_BRANCH,
"reasons": [],
}
# repository / host / unknown / generic 404 — not verified absence
reason = {
NOT_FOUND_SCOPE_REPOSITORY: (
"post-delete readback 404 is repository-scoped, not branch absence"
),
NOT_FOUND_SCOPE_HOST: (
"post-delete readback 404 is host-scoped, not branch absence"
),
}.get(
scope,
"post-delete readback 404 is ambiguous (not branch-scoped); "
"cannot verify absence",
)
return {
"status": READBACK_AMBIGUOUS_404,
"error_class": ERROR_CLASS_UNEXPECTED,
"verified_absent": False,
"branch_present": None,
"not_found_scope": scope,
"reasons": [reason],
}
if status_code in (401, 407):
return {
"status": READBACK_AUTHENTICATION,
"error_class": ERROR_CLASS_AUTHENTICATION,
"verified_absent": False,
"branch_present": None,
"reasons": ["post-delete branch readback authentication failed"],
}
if status_code == 403:
return {
"status": READBACK_AUTHORIZATION,
"error_class": ERROR_CLASS_AUTHORIZATION,
"verified_absent": False,
"branch_present": None,
"reasons": ["post-delete branch readback authorization failed"],
}
if status_code is not None and 200 <= int(status_code) < 300:
return {
"status": READBACK_EXISTS,
"error_class": None,
"verified_absent": False,
"branch_present": True,
"reasons": ["post-delete readback found branch still present"],
}
if status_code is not None and int(status_code) >= 500:
return {
"status": READBACK_TRANSPORT,
"error_class": ERROR_CLASS_TRANSPORT,
"verified_absent": False,
"branch_present": None,
"reasons": ["post-delete branch readback transport/upstream failure"],
}
return {
"status": READBACK_UNEXPECTED,
"error_class": ERROR_CLASS_UNEXPECTED,
"verified_absent": False,
"branch_present": None,
"reasons": ["post-delete branch readback returned an unexpected response"],
"http_status": status_code,
}
def _extract_http_status(exc: BaseException) -> int | None:
"""Extract an HTTP status code from an exception chain (secret-free)."""
seen: set[int] = set()
current: BaseException | None = exc
while current is not None and id(current) not in seen:
seen.add(id(current))
for attr in ("code", "status", "status_code"):
value = getattr(current, attr, None)
if isinstance(value, int) and 100 <= value <= 599:
return value
text = str(current) if current is not None else ""
match = re.match(r"HTTP\s+(\d{3})\b", text)
if match:
return int(match.group(1))
current = current.__cause__ or current.__context__
return None
def classify_branch_readback_exception(
exc: BaseException,
*,
not_found_scope: str | None = None,
) -> dict[str, Any]:
"""Classify a GET-branch exception without leaking credentials or bodies.
R1: substring ``404`` / ``not found`` alone never becomes verified_absent.
Callers must pass ``not_found_scope='branch'`` only after authoritative
proof that the repository/host is still reachable and the 404 is branch-level.
"""
status_code = _extract_http_status(exc)
if status_code is None:
lower = str(exc).lower() if exc is not None else ""
if any(
token in lower
for token in (
"timed out",
"timeout",
"connection",
"network",
"temporarily unavailable",
"name or service not known",
"nodename nor servname",
)
):
return {
"status": READBACK_TRANSPORT,
"error_class": ERROR_CLASS_TRANSPORT,
"verified_absent": False,
"branch_present": None,
"reasons": [
"post-delete branch readback transport/upstream failure"
],
}
if "unauthorized" in lower:
status_code = 401
elif "forbidden" in lower:
status_code = 403
elif "404" in lower or "not found" in lower:
# Ambiguous: do NOT treat as branch absence without scope proof.
status_code = 404
if not_found_scope is None:
not_found_scope = NOT_FOUND_SCOPE_UNKNOWN
if status_code is not None:
return classify_branch_readback_http_status(
status_code, not_found_scope=not_found_scope
)
return {
"status": READBACK_UNEXPECTED,
"error_class": ERROR_CLASS_UNEXPECTED,
"verified_absent": False,
"branch_present": None,
"reasons": ["post-delete branch readback returned an unexpected response"],
}
def assess_post_delete_readback(readback: dict[str, Any] | None) -> dict[str, Any]:
"""Decide whether DELETE success may be reported after branch readback.
Success requires authoritative branch-scoped not-found
(``verified_absent=True``). DELETE HTTP success alone is never enough.
Generic/repository/host 404 cannot verify absence (R1).
"""
payload = dict(readback or {})
status = _norm_str(payload.get("status")) or READBACK_UNEXPECTED
# Only explicit verified_absent flag counts — never infer from status alone
# when status is a bare not_found without branch scope proof.
verified = bool(payload.get("verified_absent")) is True
if verified and status == READBACK_NOT_FOUND:
return {
"ok": True,
"success": True,
"verified_absent": True,
"readback": {
"status": READBACK_NOT_FOUND,
"verified_absent": True,
"branch_present": False,
"error_class": None,
"not_found_scope": NOT_FOUND_SCOPE_BRANCH,
},
"reasons": [],
}
reasons = list(payload.get("reasons") or [])
if not reasons:
if status == READBACK_EXISTS:
reasons = ["post-delete readback found branch still present"]
elif status == READBACK_AUTHENTICATION:
reasons = ["post-delete branch readback authentication failed"]
elif status == READBACK_AUTHORIZATION:
reasons = ["post-delete branch readback authorization failed"]
elif status == READBACK_TRANSPORT:
reasons = ["post-delete branch readback transport/upstream failure"]
elif status == READBACK_AMBIGUOUS_404:
reasons = [
"post-delete readback 404 is not branch-scoped; "
"cannot verify absence"
]
else:
reasons = ["post-delete branch readback could not verify deletion"]
return {
"ok": False,
"success": False,
"verified_absent": False,
"readback": {
"status": status,
"verified_absent": False,
"branch_present": payload.get("branch_present"),
"error_class": payload.get("error_class"),
"not_found_scope": payload.get("not_found_scope"),
},
"reasons": reasons,
"blocker_kind": "post_delete_readback_failed",
}
def cleanup_result_envelope(
*,
success: bool,
performed: bool,
delete_acknowledged: bool,
verified_absent: bool,
**extra: Any,
) -> dict[str, Any]:
"""R2: consistent top-level cleanup result fields on every return path."""
out: dict[str, Any] = {
"success": bool(success),
"performed": bool(performed),
"delete_acknowledged": bool(delete_acknowledged),
"verified_absent": bool(verified_absent),
}
out.update(extra)
return out
def _repo_matches(
record: dict[str, Any],
*,
remote: str,
org: str,
repo: str,
host: str | None = None,
) -> bool:
"""Match ownership record to target remote/org/repo and normalized host."""
if (
_norm_str(record.get("remote")).lower() != _norm_str(remote).lower()
or _norm_str(record.get("org")).lower() != _norm_str(org).lower()
or _norm_str(record.get("repo")).lower() != _norm_str(repo).lower()
):
return False
expected_host = normalize_host(host)
record_host = normalize_host(record.get("host") or record.get("host_name"))
# When both sides declare a host, they must agree after normalization.
# A record host that disagrees with the expected host is out of scope.
# Legacy records without host still match when remote/org/repo agree.
if expected_host and record_host and expected_host != record_host:
return False
return True
def _branch_matches(record: dict[str, Any], branch: str) -> bool:
return _norm_str(record.get("branch")) == _norm_str(branch)
def assess_ownership_record_activity(record: dict[str, Any]) -> dict[str, Any]:
"""Classify one ownership record as blocking or non-blocking.
Distinguishes active ownership from expired/released/terminal/stale records.
Expired/stale records block unless reclaim_allowed is *explicitly* True
(O2: never treat missing/unknown reclaim as auto-allowed).
"""
status = _norm_str(record.get("status")).lower()
category = _norm_str(record.get("category")) or "unknown"
reclaim_allowed = record.get("reclaim_allowed")
if category == OWNERSHIP_CATEGORY_INVENTORY_ERROR:
return {
"blocks": True,
"status": status or "unknown",
"category": category,
"reason": "ownership inventory failed closed",
}
if status in _TERMINAL_OWNERSHIP_STATUSES:
return {
"blocks": False,
"status": status,
"category": category,
"reason": f"{category} ownership is terminal/released ({status})",
}
if status in _ACTIVE_OWNERSHIP_STATUSES:
return {
"blocks": True,
"status": status,
"category": category,
"reason": f"active {category} ownership still uses the target branch",
}
if status in _EXPIRED_STATUSES or status in _STALE_STATUSES:
# O2: only explicit reclaim_allowed=True skips the block.
if reclaim_allowed is True:
return {
"blocks": False,
"status": status,
"category": category,
"reason": (
f"{category} ownership is {status} and reclaimable; "
"does not block deletion"
),
}
return {
"blocks": True,
"status": status,
"category": category,
"reason": (
f"{status} {category} ownership still protects the target "
"branch (reclaim not proven; fail closed)"
),
}
# Unknown status → fail closed
return {
"blocks": True,
"status": status or "unknown",
"category": category,
"reason": (
f"unclassified {category} ownership status "
f"'{status or 'unknown'}'; fail closed"
),
}
def assess_active_branch_ownership(
*,
remote: str,
org: str,
repo: str,
branch: str,
host: str | None = None,
records: list[dict[str, Any]] | None = None,
) -> dict[str, Any]:
"""Assess whether any active ownership still uses *branch* in *repo*.
Matching requires remote/org/repo/branch and, when provided, normalized
host identity. Other repositories, hosts, or branches never false-block.
"""
target_branch = _norm_str(branch)
expected_host = normalize_host(host)
considered: list[dict[str, Any]] = []
blocking: list[dict[str, Any]] = []
ignored: list[dict[str, Any]] = []
for raw in records or []:
if not isinstance(raw, dict):
continue
if not _repo_matches(
raw, remote=remote, org=org, repo=repo, host=expected_host or None
):
ignored.append(
{
"category": _norm_str(raw.get("category")) or "unknown",
"reason": "different repository or host scope",
}
)
continue
if not _branch_matches(raw, target_branch):
ignored.append(
{
"category": _norm_str(raw.get("category")) or "unknown",
"reason": "different branch",
}
)
continue
activity = assess_ownership_record_activity(raw)
entry = {
"category": activity["category"],
"status": activity["status"],
"blocks": activity["blocks"],
"reason": activity["reason"],
}
considered.append(entry)
if activity["blocks"]:
blocking.append(entry)
block = bool(blocking)
categories = sorted({b["category"] for b in blocking})
reasons = [
(
"active ownership protects branch "
f"'{target_branch}': " + "; ".join(b["reason"] for b in blocking)
)
] if block else []
return {
"block": block,
"safe_to_delete": not block,
"remote": remote,
"org": org,
"repo": repo,
"host": expected_host or None,
"branch": target_branch,
"blocking_categories": categories,
"blocking": blocking,
"considered": considered,
"ignored_out_of_scope": ignored,
"reasons": reasons,
"blocker_kind": "active_branch_ownership" if block else None,
"recommended_action": "keep_remote_branch" if block else "delete_remote_branch",
}
@@ -0,0 +1,198 @@
# ADR: Stable control runtime vs dev runtime (Gitea MCP)
- **Status:** Accepted (policy effective immediately for LLM sessions; tooling may lag)
- **Date:** 2026-07-09
- **Tracking issue:** [#615](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/615)
- **Related:**
- [#543](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/543) / `docs/mcp-namespace-health.md` — client-namespace health
- `docs/mcp-namespace-eof-recovery.md` — reconnect-only EOF recovery (no PID kill)
- [#558](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/558) / `docs/mcp-daemon-import-guard.md` — sanctioned daemon
- [#557](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/557) / `docs/bootstrap-review-path.md` — controller bootstrap for self-hosted fixes
- Allocator / control-plane ADR: `docs/architecture/mcp-allocator-control-plane-observability-adr.md` (#613 / PR #614)
## 1. Context
The Gitea MCP server is the **control plane** for real issue/PR mutations (create, comment, lock, review, merge, etc.). When author/reviewer/merger/reconciler sessions kill or restart that process, relaunch it from a feature worktree, or edit the checkout that process loads, operators observe:
- Mid-session identity/preflight resets
- Stale-runtime vs master parity failures
- IDE transport EOF / “tool not found” while code on disk has changed
- Accidental production mutations from experimental code
This ADR separates **stable control runtime** from **dev/test runtime** and defines promotion proof.
## 2. Decision
### 2.1 Stable control runtime
The Gitea MCP server used for **real workflow mutations** is the **stable control runtime**.
Characteristics:
- Loads a known, promoted revision of Gitea-Tools (or the packaged release layout operators designate)
- Registered in the IDE/client as the production namespaces (`gitea-tools`, `gitea-reviewer`, `gitea-merger`, `gitea-reconciler`, etc.)
- Holds production profile credentials via sanctioned keychain/env paths only
### 2.2 Dev / test runtime
MCP **server code** development and testing:
- Happens in isolated **`branches/`** worktrees (or other non-stable checkouts)
- May use a **separate** dev/test MCP runtime/process when process-level testing is required
- **Must not** be used for real Gitea mutations on production issues/PRs
### 2.3 Forbidden actions (normal sessions)
Normal **author, reviewer, merger, and reconciler** LLM sessions **must not**:
| Forbidden | Why |
|-----------|-----|
| Kill the running MCP server process | Drops all concurrent sessions; loses preflight state |
| Restart / relaunch the MCP server process | Same as kill; causes stale/identity churn mid-workflow |
| Relaunch MCP from a development worktree | Runs unpromoted code against production mutations |
| Edit files in the stable runtime checkout | Hot-mutates control plane under concurrent users |
| Use experimental/dev MCP for real Gitea mutations | Bypasses promotion proof and audit expectations |
| Bypass or self-reset a stale master-parity gate | The gate is fail-closed; only an operator reload restores parity |
**LLM-allowed vs operator-owned (authoritative split):**
| Actor | May do | Must not do |
|-------|--------|-------------|
| **LLM session** | Call tools on the already-running stable namespaces; **client reconnect** after transport EOF (no process kill); pass `worktree_path` / role worktree args; report blockers and stop mutations when unhealthy/stale | Kill, restart, or relaunch any MCP process; bump config mtimes to force reload; edit the stable checkout; switch to a dev MCP for production mutations |
| **Operator / release-manager** | Supervised restart/reload of the **stable** control runtime; dual-namespace client configuration; §2.4 promotions; incident recovery | — |
EOF / transport recovery for LLM sessions: **client reconnect only** (see `docs/mcp-namespace-eof-recovery.md`). Do not “fix” health by killing PIDs or bumping MCP config mtimes as a normal session procedure.
This ADR **supersedes** any older runbook wording that told the LLM to relaunch or restart the client/MCP as a self-service step. Where `docs/llm-workflow-runbooks.md` (or wiki runbooks) discuss dual-namespace setup or workspace rebind, **process restart/relaunch is operator-owned**; the LLM stops, reports, and waits.
### 2.4 Promotion (operator / release-manager only)
Promotion of a new revision into the stable control runtime is an **explicit operator/release-manager action**, not an LLM self-service step.
A promotion **must record** (issue comment, release note, or promotion ledger):
| Field | Description |
|-------|-------------|
| **previous runtime SHA** | Commit previously loaded by stable runtime |
| **promoted runtime SHA** | Commit after promotion |
| **source branch/PR** | Where the change was reviewed |
| **restart/reload method** | How the process was cycled (e.g. supervised restart, client reload) |
| **health check proof** | Client-namespace probe success (`gitea_whoami` / namespace health) |
| **identity/profile proof** | Expected profile(s) and username(s) after reload |
| **workspace/root proof** | Stable checkout path / root matches intended layout |
| **mutation capability proof** | Required permissions for the target role present; forbidden ops still forbidden |
| **rollback instructions** | How to restore previous SHA and re-verify health |
Suggested durable marker:
```text
## MCP STABLE RUNTIME PROMOTION (#615)
Status: COMPLETED | ROLLED_BACK | ABORTED
Previous-SHA: <full sha>
Promoted-SHA: <full sha>
Source-PR: <number>
Source-Branch: <name>
Reload-Method: <text>
Health-Proof: client_namespace whoami OK / assess_mcp_namespace_health OK
Identity-Proof: profile=<name> user=<name>
Workspace-Proof: root=<path>
Mutation-Proof: allowed_ops include <…>; forbidden include <…>
Rollback: checkout <previous sha>; reload method <…>; re-run health/identity proofs
Operator: <username>
Timestamp: <ISO-8601>
```
### 2.5 Unhealthy stable runtime → stop work
If the stable MCP runtime is **unhealthy**, including any of:
- client-namespace probes fail
- wrong identity / wrong profile
- wrong workspace root
- missing mutation capability for the intended role
- persistent EOF after **client reconnect**
- **master parity is stale** (`startup_head` behind on-disk `master` / `restart_required` from the parity gate)
then:
1. **Normal PR / review / merge / issue-mutation work must stop immediately.**
2. The session **reports** the unhealthy/stale state (tool error, CTH, or operator handoff) with startup vs current head when known.
3. Do **not** improvise: no LLM process kill/restart, no dev-worktree MCP for production mutations, no env escape hatches, no manual gate bypass.
4. Resume only after:
- an **operator** restores the runtime (see §2.6 for routine post-merge parity reload), **or**
- a **controlled** promotion/rollback completes with the §2.4 promotion record, **or**
- a controller invokes the narrow **bootstrap review path** (#557) when the defect is self-hosted and documented,
- **and** the session re-verifies health (and master parity when applicable) before the next mutation.
### 2.6 Routine post-merge master-parity staleness (operator reload, not promotion)
**Symptom:** After merges land on `master`, a long-lived stable MCP process still runs the pre-merge `startup_head`. The master-parity gate marks the server **stale** / `restart_required` and **blocks mutations**.
**Sanctioned response (authoritative):**
| Step | Actor | Action |
|------|-------|--------|
| 1 | LLM session | Mutations stop immediately when the gate reports stale. |
| 2 | LLM session | Report the stale state (startup head, current master head, that operator reload is required). Do not retry mutations. |
| 3 | **Operator** | Reload/restart the **stable** control MCP so it loads current `master` (supervised client/daemon reload). |
| 4 | LLM session | Resume only after startup/current-head parity is verified (e.g. `gitea_get_runtime_context` / parity assessment shows in parity). |
**Not a §2.4 promotion:** Catching the already-designated stable control checkout up to a newly advanced `master` is a **routine operator reload** of the stable runtime. It does **not** require the nine-field promotion ledger. Use §2.4 only when **changing which unpromoted/dev revision** becomes the stable control runtime (new source branch/PR into the stable designation).
**Code note:** `master_parity_gate.py` may still say “restart the server” in machine-facing reason strings. That string names the **operator recovery action**, not an LLM self-service instruction. This ADR and the runbooks define the actor split.
## 3. Relationship to other controls
| Doc / mechanism | Interaction |
|-----------------|-------------|
| Namespace health (#543) | Proves IDE client can call tools; does not authorize restart |
| EOF recovery | Reconnect only; no process kill |
| Daemon import guard (#558) | Mutations require sanctioned daemon; not a bare shell import |
| Bootstrap path (#557) | Only controller-authorized exception when live runtime cannot review its own fix |
| Allocator / control-plane ADR | Coordination DB is separate; still depends on a healthy MCP surface for Gitea writes |
## 4. Consequences
### Positive
- Predictable control plane for concurrent LLMs
- Clear operator-only promotion gate with rollback
- Aligns session behavior with health/EOF docs already landed
### Costs
- LLM sessions must wait when runtime is sick (no DIY restart)
- Operators must maintain promotion discipline and dual-runtime config if they use a dev MCP
### Non-goals
- Does not ban operator-supervised restarts during incidents
- Does not replace CI or code review for MCP changes
- Does not authorize editing stable checkout “because tests need a quick fix”
## 5. Implementation follow-ups (optional tooling)
These may land in later issues; the **policy binds sessions now**:
1. Session preflight that refuses mutations if workspace root equals a `branches/` feature worktree configured as “dev only.”
2. Explicit `runtime_kind=stable|dev` in MCP config and `gitea_whoami` profile metadata.
3. Promotion checklist script that emits the durable promotion marker fields.
**Not optional (issue #615 acceptance criterion 2):** operator guide and runbooks **must** cross-link this ADR (see §6). Cross-links are documentation acceptance, not deferred tooling.
## 6. Acceptance for this ADR
1. Document merged under `docs/architecture/mcp-stable-control-runtime-policy-adr.md`.
2. **Operator guide / runbooks cross-link this ADR** (`docs/wiki/Operator-Guide.md`, `docs/wiki/Runbooks.md`, `docs/llm-workflow-runbooks.md`).
3. Issue #615 references this path.
4. LLM/operator runbooks treat kill/restart/relaunch-from-worktree as **LLM violations**; process restart is **operator-owned**.
5. Unhealthy runtime (including **stale master parity**) stops normal mutation work until operator restore/reload, promotion/rollback, or #557 bootstrap — then re-verify parity before mutating.
6. Routine post-merge parity reload is documented as operator reload (§2.6), not an LLM self-restart and not a full §2.4 promotion.
## 7. Document history
| Date | Change |
|------|--------|
| 2026-07-09 | Initial ADR: stable vs dev runtime, forbidden session actions, promotion proof fields, stop-work rule |
| 2026-07-16 | Review 443 remediation (#615 / PR #616): mandatory cross-links; LLM vs operator restart split; routine post-merge parity staleness (§2.6); stale parity in §2.5 unhealthy triggers |
+185
View File
@@ -238,11 +238,43 @@ narrow operation set:
- `gitea.issue.comment`
- `gitea.issue.close`
- `gitea.pr.close`
- `gitea.branch.delete` (merged-branch cleanup only — see below)
Forbidden on reconciler profiles: `gitea.pr.approve`, `gitea.pr.merge`,
`gitea.pr.review`, `gitea.pr.create`, `gitea.branch.push`, and
`gitea.repo.commit`.
### Merged-branch cleanup ownership (`gitea.branch.delete`)
The reconciler is the repository-supported owner of merged-PR source-branch
cleanup: `task_capability_map` maps `cleanup_merged_pr_branch` (and
`reconciliation_cleanup`) to role `reconciler` with permission
`gitea.branch.delete`. Post-merge branch lifecycle is reconciliation work —
it happens after the author, reviewer, and merger roles have completed, and
it must not be reachable from those roles.
Least-privilege constraints:
- `gitea.branch.delete` is granted **only** to reconciler profiles. Author,
reviewer, and merger profiles must never hold it; `gitea_delete_branch`
and `gitea_cleanup_merged_pr_branch` fail closed on any profile without
the permission.
- Even with the permission, reconciler deletion is only supported through the
guarded `gitea_cleanup_merged_pr_branch` path (#514 / #687): the PR must be
merged, the head an ancestor of the target, the branch not protected
(`master`/`main`/`dev`), the branch not a preservation/evidence ref (e.g.
`chore/issue-681-preserve-review-session-wip`), no open PR may still use the
head, and an explicit `CLEANUP MERGED PR <n> BRANCH <branch>` confirmation is
required. Raw `gitea_delete_branch` is **denied** to reconciler even when
`gitea.branch.delete` is present.
- Raw `git branch -d` / `git push --delete` cleanup remains blocked by
`branch_cleanup_guard` and the final-report validator regardless of
profile permissions.
- `gitea.branch.delete` has no short alias in `GITEA_OPERATION_ALIASES`;
write it fully qualified in `allowed_operations`. Migration must emit
canonical names such as `gitea.pr.close` (never bare `pr.close` /
`issue.close`, which the production normalizer rejects or drops).
Launch a static `gitea-reconciler` MCP namespace with
`GITEA_MCP_PROFILE=prgs-reconciler`. Profile shape is validated by
`reconciler_profile.assess_reconciler_profile` (#304). Use the
@@ -251,6 +283,159 @@ Launch a static `gitea-reconciler` MCP namespace with
fresh target-branch fetch, recorded target SHA, and ancestor proof. PRs whose
heads are not already landed cannot be closed through this path.
### Operational runbook: grant reconciler `gitea.branch.delete` (#687)
Merging a code PR that updates `migrate_profiles.py` / `reconciler_profile.py`
**does not** change the live operator profile on disk. Apply the profile
change deliberately, then reconnect the client-managed namespace.
1. **Approved migration / profile-update command** (from the repo root, using
the project venv if present):
```bash
# Dry-run first (default): validates v2 output, writes nothing
python3 migrate_profiles.py -i ~/.config/gitea-tools/profiles.json
# Apply: creates backup then writes migrated v2 config
python3 migrate_profiles.py -i ~/.config/gitea-tools/profiles.json -w
# Optional explicit paths:
# python3 migrate_profiles.py -i ~/.config/gitea-tools/profiles.json \
# -o ~/.config/gitea-tools/profiles.json \
# --backup ~/.config/gitea-tools/profiles.json.bak -w
```
If the live file is already v2, edit the reconciler identitys
`allowed_operations` / `forbidden_operations` under
`environments.<env>.services.gitea.identities.reconciler` (or the
`prgs-reconciler` alias target) so allowed includes the canonical set
below — then re-validate with a load of the config (see step 3).
2. **Inspect the generated (or edited) profile** — confirm the reconciler
identity, for example:
```bash
python3 - <<'PY'
import json
from pathlib import Path
cfg = json.loads(Path.home().joinpath(".config/gitea-tools/profiles.json").read_text())
# v2 environments shape:
ident = cfg["environments"]["prgs"]["services"]["gitea"]["identities"]["reconciler"]
print("role:", ident.get("role"))
print("allowed:", ident.get("allowed_operations"))
print("forbidden:", ident.get("forbidden_operations"))
PY
```
3. **Validate canonical operation names and least privilege**
Expected canonical **allowed** (defaults after migration):
- `gitea.read`
- `gitea.pr.close` (required)
- `gitea.pr.comment`
- `gitea.issue.comment`
- `gitea.issue.close`
- `gitea.branch.delete` (recommended; cleanup only)
Expected **forbidden** includes at least: `gitea.pr.approve`,
`gitea.pr.merge`, `gitea.pr.review`, `gitea.pr.create`,
`gitea.branch.push`, `gitea.repo.commit`.
No shorthand (`pr.close`, `issue.close`, `pr.comment`) may remain.
Validate with the production loader:
```bash
python3 - <<'PY'
import gitea_config, reconciler_profile
from pathlib import Path
path = str(Path.home() / ".config/gitea-tools/profiles.json")
gitea_config.load_config(path) # fails closed on invalid config
# Or assess the reconciler lists directly after extracting them:
# print(reconciler_profile.assess_reconciler_profile(allowed, forbidden))
PY
```
4. **Merging PR #688 (or any code PR) does not update the live profile.**
Code changes only the migration helper, schema, docs, and tests. The
operator must still run `migrate_profiles.py -w` or an equivalent
authorized edit of `~/.config/gitea-tools/profiles.json`.
5. **Supported apply method:** `python3 migrate_profiles.py … -w` (backup
created automatically) **or** operator-authorized edit of the live
profiles file after backup. Unsupported: silent mtime tricks, manual
process kill to “reload”, or undocumented env overrides.
6. **Backup and validation:** `-w` copies the input to
`<input_path>.bak` (or `--backup PATH`) before writing. Re-run
`load_config` / `assess_reconciler_profile` after write. Keep the
`.bak` until live whoami/capability checks pass.
7. **Client-managed namespace reconnect/reload:** reconnect or reload the
IDE MCP client so `gitea-reconciler` restarts from current `master` and
the updated `GITEA_MCP_PROFILE=prgs-reconciler` config. Do not hand-launch
`mcp_server.py` / `gitea_mcp_server.py` with ad hoc `GITEA_*` env
(see #686 / #630).
8. **Live reverification** (through the client-managed `gitea-reconciler`
namespace only):
- `gitea_whoami` → identity + profile `prgs-reconciler`
- `gitea_assess_master_parity` → `stale=false`, `restart_required=false`
- `gitea_resolve_task_capability(task="cleanup_merged_pr_branch")` →
`allowed_in_current_session=true` only when permission and role match
- `gitea_resolve_task_capability(task="delete_branch")` →
**not** allowed for reconciler (role denial must be enforced)
9. **Guarded cleanup usage** (example for a merged PR whose source branch
remains on the remote):
```text
gitea_cleanup_merged_pr_branch(
pr_number=<N>,
branch=<exact PR head branch>,
confirmation="CLEANUP MERGED PR <N> BRANCH <exact PR head branch>",
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
worktree_path="<path under branches/>",
)
```
The tool refuses unmerged PRs, protected branches, preservation/evidence
branches, open-PR heads, mismatched branch names, and wrong confirmation.
10. **Prohibitions**
- No raw `git push --delete`, `git branch -d` / `-D`, or delete refspecs
- No arbitrary `gitea_delete_branch` from reconciler
- No unsupported profile switching mid-run without full re-preflight
- No ad hoc hand-edits of live profiles **unless** operator-authorized,
backed up, and revalidated as above
Canonical migrated reconciler example:
```json
{
"role": "reconciler",
"allowed_operations": [
"gitea.read",
"gitea.pr.close",
"gitea.pr.comment",
"gitea.issue.comment",
"gitea.issue.close",
"gitea.branch.delete"
],
"forbidden_operations": [
"gitea.pr.approve",
"gitea.pr.merge",
"gitea.pr.review",
"gitea.pr.create",
"gitea.branch.push",
"gitea.repo.commit"
]
}
```
## Identity and fail-closed rules
Before **any** mutating action, a workflow must know both:
+13 -8
View File
@@ -195,7 +195,7 @@ To avoid the bottleneck of relaunching/restarting the MCP server to switch betwe
`gitea_reconcile_already_landed_pr` after ancestry proof — not for normal
review or author workflows.
* **Fallback:** If the dual-profile MCP launcher pattern is not supported or configured in the client, the LLM must relaunch or restart the client/MCP with the correct profile environment variable before claiming or working on any tasks.
* **Fallback (operator-owned):** If the dual-profile MCP launcher pattern is not supported or configured in the client, **do not** have the LLM relaunch or restart the client/MCP. The LLM **stops** role-switching work, reports that the correct static namespace is missing, and waits for an **operator** to configure dual namespaces or reload the client with the correct `GITEA_MCP_PROFILE` for that role. Process restart/relaunch is operator-owned under the [stable control runtime ADR](architecture/mcp-stable-control-runtime-policy-adr.md) (#615).
## Setup runbook — interactive menu
@@ -1200,10 +1200,13 @@ When a mutation blocks on workspace binding:
1. Read the error — it names the **resolved workspace path**, **role
namespace**, and **binding source** (tool arg, env var, or process root).
2. Reconnect or relaunch the correct namespace MCP server from the intended
workspace (or set the role-specific env var before launch).
3. Pass `worktree_path` on reviewer/merger mutation tools when the active
branches/ worktree differs from the MCP process root.
2. **LLM-allowed:** pass `worktree_path` on reviewer/merger mutation tools when
the active `branches/` worktree differs from the MCP process root; **client
reconnect** after transport EOF only (no process kill).
3. **Operator-owned:** if the wrong namespace process was launched, or a role-
specific `GITEA_*_WORKTREE` must be set at process start, an **operator**
reloads/relaunches the correct static namespace MCP. LLM sessions must not
kill or restart MCP processes (see [stable control runtime ADR](architecture/mcp-stable-control-runtime-policy-adr.md)).
4. **Do not** clean, reset, or discard foreign role worktrees to unblock your
own namespace — that destroys another agent's WIP.
@@ -1213,9 +1216,10 @@ When posting a Canonical Thread Handoff after a binding blocker:
- State which namespace was active (author / reviewer / merger / reconciler).
- Quote the resolved workspace path and binding source from the error.
- Name the safe reconnect action (relaunch MCP from `branches/...`, set
`GITEA_*_WORKTREE`, or pass `worktree_path`).
- Explicitly note that foreign worktrees must not be cleaned to unblock.
- Name the safe next action: pass `worktree_path` if that unblocks the tool, or
request an **operator** reload of the correct namespace MCP / env binding.
- Explicitly note that foreign worktrees must not be cleaned to unblock, and
that the LLM must not self-restart the MCP process.
## Safety notes
@@ -1226,6 +1230,7 @@ When posting a Canonical Thread Handoff after a binding blocker:
## Related documents
- [`architecture/mcp-stable-control-runtime-policy-adr.md`](architecture/mcp-stable-control-runtime-policy-adr.md) — stable control runtime vs dev runtime; LLM must not kill/restart MCP; operator-owned reload and promotions; routine post-merge parity staleness (#615).
- [`reviewer-handoff-consistency.md`](reviewer-handoff-consistency.md) — reject contradictory reviewer handoffs (#501).
- [`issue-acceptance-gate.md`](issue-acceptance-gate.md) — controller issue-acceptance audit after PR merge (#500).
- [`../skills/llm-project-workflow/SKILL.md`](../skills/llm-project-workflow/SKILL.md) — portable cross-project LLM workflow skill.
+3 -1
View File
@@ -14,6 +14,7 @@ Handbook for LLM operators and human developers using the Gitea-Tools MCP server
4. **No self-review / no self-merge** — The authenticated Gitea user must not approve or merge a PR they authored.
5. **Follow the gates** — Prompts express intent; MCP tools enforce safety. Never bypass gates via prompt instructions.
6. **Global LLM Worktree Rule** — Main checkout stays on `master`/`main`/`dev`; all mutations happen under `branches/`. Prove project root, `cwd`, branch, stable main-checkout branch, and session worktree path before editing. No exceptions.
7. **Stable control runtime** — Real Gitea mutations use only the **stable** MCP control runtime. LLM sessions must not kill, restart, or relaunch MCP processes, edit the stable checkout, or use a dev MCP for production mutations. See the policy ADR: [mcp-stable-control-runtime-policy-adr.md](../architecture/mcp-stable-control-runtime-policy-adr.md) (#615).
## Supported Gitea instances
@@ -29,4 +30,5 @@ Always pass `remote` explicitly on tool calls. The server default is `dadeschool
1. `gitea_whoami` — confirm authenticated user and profile.
2. `gitea_get_runtime_context` — allowed/forbidden operations for this session.
3. `gitea_resolve_task_capability` — prove the session may perform the planned task.
4. For reviewer work: dry-run validation (`gitea_dry_run_pr_review`) before live review mutations.
4. For reviewer work: dry-run validation (`gitea_dry_run_pr_review`) before live review mutations.
5. Confirm master parity / runtime health when tools report stale or unhealthy control runtime — stop mutations and request an **operator** reload of the stable MCP (see [stable control runtime ADR](../architecture/mcp-stable-control-runtime-policy-adr.md)).
+11
View File
@@ -12,6 +12,17 @@
4. `gitea_mark_final_review_decision` → approve via `gitea_review_pr`.
5. `gitea_merge_pr` with pinned head SHA and `confirmation="MERGE PR <n>"`.
## Stable MCP control runtime (#615)
Policy ADR: [mcp-stable-control-runtime-policy-adr.md](../architecture/mcp-stable-control-runtime-policy-adr.md).
| Situation | Who acts | What to do |
|-----------|----------|------------|
| Transport EOF / missing tools | LLM | **Client reconnect only** — do not kill PIDs |
| Wrong profile / dual-namespace missing | Operator | Configure or reload the correct static namespace(s) |
| Master parity stale after merge | LLM stops + reports; **operator** reloads stable MCP | Resume only after parity re-verified |
| Promote unpromoted MCP code to stable | Operator / release-manager only | Full §2.4 promotion record |
## Gitea Wiki sync
The Gitea Wiki mirrors `docs/wiki/` (source of truth). After merging wiki changes:
+207 -19
View File
@@ -243,6 +243,192 @@ 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.
@@ -315,23 +501,24 @@ 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
``RuntimeError`` on failure.
a classified client error 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.
All failures are converted to a ``RuntimeError`` with a clear, secret
-redacted message (no raw stack traces or credential material):
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:
- 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.
- 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)
The ``*_func`` parameters and ``timeout`` are injection points for
deterministic testing.
@@ -369,22 +556,23 @@ def api_request(method, url, auth_header, payload=None, *,
error_body = e.read().decode("utf-8", errors="replace")
except Exception:
error_body = ""
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
# 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
except (urllib.error.URLError, TimeoutError) as e:
reason = getattr(e, "reason", e)
raise RuntimeError(
f"network error contacting Gitea: {_redact(reason)}"
) from e
# Fixed message only — do not embed URLError reason (may leak paths).
raise GiteaNetworkError(reason_code="network_error") 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
+848 -67
View File
File diff suppressed because it is too large Load Diff
+125 -3
View File
@@ -46,8 +46,15 @@ KIND_IRRECOVERABLE_DECISION_PROVENANCE = "irrecoverable_decision_provenance"
KIND_IRRECOVERABLE_PROVENANCE_AUTH = "irrecoverable_provenance_authorization"
# Kinds that must survive the default session-state TTL (forensic / recovery).
#
# KIND_DECISION_LOCK is recovery-critical (#720): terminal review provenance is
# not disposable cache. A generic four-hour TTL must not drop old-head evidence
# or make ``fresh_review_on_current_head_allowed`` unreachable. Same-head / same-
# run #332 protections still apply once the ledger is loadable. Other session
# kinds (workflow load, drafts, etc.) remain TTL-bound.
RECOVERY_CRITICAL_KINDS = frozenset(
{
KIND_DECISION_LOCK,
KIND_DECISION_LOCK_ARCHIVE,
KIND_POST_MERGE_DECISION_RECOVERY,
KIND_IRRECOVERABLE_DECISION_PROVENANCE,
@@ -245,6 +252,16 @@ def _write_json(path: str, data: dict[str, Any]) -> None:
pass
def is_recovery_critical_record(record: dict[str, Any] | None, kind: str | None = None) -> bool:
"""True when a durable record must outlive the generic session-state TTL."""
if not record and not kind:
return False
record_kind = ((record or {}).get("kind") or kind or "").strip()
if record_kind in RECOVERY_CRITICAL_KINDS:
return True
return bool((record or {}).get("recovery_critical"))
def identity_match_reasons(
record: dict[str, Any] | None,
*,
@@ -288,9 +305,7 @@ def identity_match_reasons(
else:
age = _now_utc() - recorded_at
kind = (record.get("kind") or "").strip()
ttl_exempt = kind in RECOVERY_CRITICAL_KINDS or bool(
record.get("recovery_critical")
)
ttl_exempt = is_recovery_critical_record(record, kind=kind)
if age > timedelta(hours=ttl_hours()) and not ttl_exempt:
reasons.append(
f"session state expired after {ttl_hours():g}h (fail closed)"
@@ -300,6 +315,113 @@ def identity_match_reasons(
return reasons
def inspect_state_envelope(
*,
kind: str,
remote: str | None = None,
org: str | None = None,
repo: str | None = None,
profile_identity: str | None = None,
state_dir: str | None = None,
) -> dict[str, Any]:
"""Read-only disk inspection for assessment when TTL would otherwise hide state (#720).
Does **not** apply identity/TTL rejection to the returned presence flags.
Callers use this to distinguish:
* no file on disk
* file present but TTL would reject a non-critical kind
* recovery-critical ledger (e.g. KIND_DECISION_LOCK) still loadable
Never mutates files. Never returns secrets.
"""
profile = current_profile_identity(profile_identity=profile_identity)
path = state_file_path(
kind=kind,
remote=remote,
org=org,
repo=repo,
profile_identity=profile,
state_dir=state_dir,
)
result: dict[str, Any] = {
"kind": kind,
"profile_identity": profile,
"path_basename": os.path.basename(path),
"on_disk": False,
"has_payload": False,
"recorded_at": None,
"updated_at": None,
"age_hours": None,
"ttl_hours": ttl_hours(),
"age_exceeds_default_ttl": False,
"recovery_critical": kind in RECOVERY_CRITICAL_KINDS,
"ttl_exempt": kind in RECOVERY_CRITICAL_KINDS,
"would_ttl_reject": False,
"identity_reasons": [],
"summary": "no session-state file on disk",
}
if not path or not os.path.exists(path):
return result
result["on_disk"] = True
envelope = _read_json(path)
if not envelope:
result["summary"] = "session-state file present but unreadable or empty"
return result
payload = envelope.get("payload")
merged: dict[str, Any] = dict(payload) if isinstance(payload, dict) else {}
result["has_payload"] = isinstance(payload, dict)
for key in (
"kind",
"remote",
"org",
"repo",
"profile_identity",
"session_profile_lock",
"recorded_at",
"updated_at",
"writer_pid",
"recovery_critical",
):
if key in envelope and key not in merged:
merged[key] = envelope[key]
if not merged.get("kind"):
merged["kind"] = kind
recorded_at = _parse_iso(merged.get("recorded_at") or merged.get("updated_at"))
result["recorded_at"] = merged.get("recorded_at") or merged.get("updated_at")
result["updated_at"] = merged.get("updated_at") or merged.get("recorded_at")
if recorded_at is not None:
age = _now_utc() - recorded_at
age_hours = age.total_seconds() / 3600.0
result["age_hours"] = age_hours
result["age_exceeds_default_ttl"] = age > timedelta(hours=ttl_hours())
ttl_exempt = is_recovery_critical_record(merged, kind=kind)
result["recovery_critical"] = ttl_exempt
result["ttl_exempt"] = ttl_exempt
identity_reasons = identity_match_reasons(
merged,
remote=remote,
org=org,
repo=repo,
profile_identity=profile,
)
result["identity_reasons"] = list(identity_reasons)
result["would_ttl_reject"] = any("expired" in r for r in identity_reasons)
if result["has_payload"] and ttl_exempt:
result["summary"] = (
"recovery-critical session-state present on disk and TTL-exempt; "
"load via load_state for full payload"
)
elif result["has_payload"] and result["would_ttl_reject"]:
result["summary"] = (
"session-state file present on disk but generic TTL would reject load "
f"(age_hours={result.get('age_hours')!r}, ttl={ttl_hours():g}h)"
)
elif result["has_payload"]:
result["summary"] = "session-state file present and within TTL / identity gates"
else:
result["summary"] = "session-state file present without a dict payload"
return result
def load_state(
*,
kind: str,
+451
View File
@@ -0,0 +1,451 @@
"""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))
+4
View File
@@ -18,11 +18,13 @@ DEFAULT_ADOPTION_REASON = "merger-handoff-approved-head"
SOURCE_ADOPT = "gitea_adopt_merger_pr_lease"
SOURCE_ACQUIRE = "gitea_acquire_reviewer_pr_lease"
SOURCE_ACQUIRE_MERGER = "gitea_acquire_merger_pr_lease"
SOURCE_HEARTBEAT = "gitea_heartbeat_reviewer_pr_lease"
SANCTIONED_PROVENANCE_SOURCES = frozenset({
SOURCE_ADOPT,
SOURCE_ACQUIRE,
SOURCE_ACQUIRE_MERGER,
SOURCE_HEARTBEAT,
})
@@ -209,8 +211,10 @@ _SANCTIONED_LEASE_EVIDENCE_RE = re.compile(
r"(?is)\b("
r"gitea_adopt_merger_pr_lease|"
r"gitea_acquire_reviewer_pr_lease|"
r"gitea_acquire_merger_pr_lease|"
r"lease_proof_source\s*[:=]\s*gitea_adopt_merger_pr_lease|"
r"lease_proof_source\s*[:=]\s*gitea_acquire_reviewer_pr_lease|"
r"lease_proof_source\s*[:=]\s*gitea_acquire_merger_pr_lease|"
r"lease_proof_kind\s*[:=]\s*sanctioned_adoption|"
r"lease_proof_kind\s*[:=]\s*sanctioned_acquire|"
r"adoption_comment_id\s*[:=]|"
+127 -8
View File
@@ -20,12 +20,114 @@ if PROJECT_ROOT not in sys.path:
import gitea_config
AUTHOR_DEFAULT_ALLOWED = ["read", "branch", "commit", "push", "open_pr", "comment"]
AUTHOR_DEFAULT_FORBIDDEN = ["approve", "request_changes", "merge"]
REVIEWER_DEFAULT_ALLOWED = [
"read", "review", "comment", "approve", "request_changes", "merge"
# Defaults emit *canonical* operation names only. Shorthand that is not in
# gitea_config.GITEA_OPERATION_ALIASES (e.g. ``pr.close``, ``issue.close``)
# is silently dropped by the production loader and must never appear here.
AUTHOR_DEFAULT_ALLOWED = [
"gitea.read",
"gitea.branch.create",
"gitea.repo.commit",
"gitea.branch.push",
"gitea.pr.create",
"gitea.pr.comment",
]
REVIEWER_DEFAULT_FORBIDDEN = ["branch", "commit", "push", "open_pr"]
AUTHOR_DEFAULT_FORBIDDEN = [
"gitea.pr.approve",
"gitea.pr.request_changes",
"gitea.pr.merge",
]
REVIEWER_DEFAULT_ALLOWED = [
"gitea.read",
"gitea.pr.review",
"gitea.pr.comment",
"gitea.pr.approve",
"gitea.pr.request_changes",
"gitea.pr.merge",
]
REVIEWER_DEFAULT_FORBIDDEN = [
"gitea.branch.create",
"gitea.repo.commit",
"gitea.branch.push",
"gitea.pr.create",
]
# Required reconciler ops (read + pr.close) plus recommended comment/close and
# branch.delete for guarded merged-PR cleanup. All names must normalize via
# gitea_config.normalize_operation without being dropped.
RECONCILER_DEFAULT_ALLOWED = [
"gitea.read",
"gitea.pr.close",
"gitea.pr.comment",
"gitea.issue.comment",
"gitea.issue.close",
"gitea.branch.delete",
]
RECONCILER_DEFAULT_FORBIDDEN = [
"gitea.pr.approve",
"gitea.pr.merge",
"gitea.pr.review",
"gitea.pr.create",
"gitea.branch.push",
"gitea.repo.commit",
]
# Migration-only expansions for common shorthands that are *not* in
# GITEA_OPERATION_ALIASES. Emitted output is always the canonical form so a
# second canonicalize pass is a no-op (idempotent).
_MIGRATION_ONLY_ALIASES = {
"pr.close": "gitea.pr.close",
"pr.comment": "gitea.pr.comment",
"issue.close": "gitea.issue.close",
"branch.delete": "gitea.branch.delete",
}
# Reconciler required ops that must survive migration (from reconciler_profile).
RECONCILER_REQUIRED_CANONICAL = ("gitea.read", "gitea.pr.close")
def canonicalize_operation(op: str) -> str:
"""Return a canonical operation name accepted by the production loader.
Fail closed on unknown/ambiguous spellings so required permissions cannot
be silently dropped by ``check_operation`` later.
"""
if not isinstance(op, str) or not op.strip():
raise ValueError("operation must be a non-empty string (fail closed)")
op = op.strip()
try:
return gitea_config.normalize_operation(op)
except gitea_config.ConfigError:
pass
if op in _MIGRATION_ONLY_ALIASES:
return _MIGRATION_ONLY_ALIASES[op]
raise ValueError(
f"operation {op!r} cannot be canonicalized for migration "
"(unknown/ambiguous; fail closed — production loader would drop it)"
)
def canonicalize_operations(ops, *, context: str = "operations") -> list[str]:
"""Canonicalize a list of operations; preserve order, drop duplicates."""
if not isinstance(ops, list):
raise ValueError(f"{context} must be a list (fail closed)")
out: list[str] = []
seen: set[str] = set()
for entry in ops:
canon = canonicalize_operation(entry)
if canon not in seen:
seen.add(canon)
out.append(canon)
return out
def _assert_reconciler_required_survive(allowed: list[str], profile_name: str) -> None:
"""Fail visibly when migration would leave a reconciler without required ops."""
missing = [op for op in RECONCILER_REQUIRED_CANONICAL if op not in set(allowed)]
if missing:
raise ValueError(
f"Profile '{profile_name}' (reconciler) is missing required "
f"operation(s) after migration: {missing}. Refusing to emit a "
"profile that would silently fail pr.close / read (fail closed)."
)
def infer_role(name, execution_profile):
@@ -90,9 +192,11 @@ def migrate_v1_to_v2(v1_data):
ident_name = "reviewer"
elif role == "author":
ident_name = "author"
elif role == "reconciler":
ident_name = "reconciler"
else:
role = prof.get("role")
if role not in (None, "author", "reviewer"):
if role not in (None, "author", "reviewer", "reconciler"):
raise ValueError(
f"Profile '{name}' has unsupported role {role!r}"
)
@@ -124,20 +228,35 @@ def migrate_v1_to_v2(v1_data):
raise ValueError(
f"Profile '{name}' operation fields must be lists"
)
identity_data["allowed_operations"] = list(allowed)
identity_data["forbidden_operations"] = list(forbidden)
try:
identity_data["allowed_operations"] = canonicalize_operations(
allowed, context=f"profile '{name}' allowed_operations"
)
identity_data["forbidden_operations"] = canonicalize_operations(
forbidden, context=f"profile '{name}' forbidden_operations"
)
except ValueError as exc:
raise ValueError(f"Profile '{name}': {exc}") from exc
elif role == "author":
identity_data["allowed_operations"] = list(AUTHOR_DEFAULT_ALLOWED)
identity_data["forbidden_operations"] = list(AUTHOR_DEFAULT_FORBIDDEN)
elif role == "reviewer":
identity_data["allowed_operations"] = list(REVIEWER_DEFAULT_ALLOWED)
identity_data["forbidden_operations"] = list(REVIEWER_DEFAULT_FORBIDDEN)
elif role == "reconciler":
identity_data["allowed_operations"] = list(RECONCILER_DEFAULT_ALLOWED)
identity_data["forbidden_operations"] = list(RECONCILER_DEFAULT_FORBIDDEN)
else:
raise ValueError(
f"Profile '{name}' has no explicit operation lists and no "
"unambiguous author/reviewer role marker (fail closed)"
)
if role == "reconciler":
_assert_reconciler_required_survive(
identity_data["allowed_operations"], name
)
# Nest inside environments/services structure
env = environments.setdefault(env_name, {})
services = env.setdefault("services", {})
+5
View File
@@ -18,6 +18,11 @@ RECONCILER_RECOMMENDED_OPERATIONS = (
"gitea.pr.comment",
"gitea.issue.comment",
"gitea.issue.close",
# Merged-branch cleanup is reconciler-owned (task_capability_map maps
# cleanup_merged_pr_branch -> reconciler). The permission is only
# exercisable through the guarded gitea_cleanup_merged_pr_branch path
# (#514): merged proof, protected-branch refusal, explicit confirmation.
"gitea.branch.delete",
)
RECONCILER_FORBIDDEN_OPERATIONS = (
+5 -1
View File
@@ -84,7 +84,11 @@ TASK_CAPABILITY_MAP: dict[str, dict[str, str]] = {
},
"adopt_merger_pr_lease": {
"permission": "gitea.pr.comment",
"role": "reviewer",
"role": "merger",
},
"acquire_merger_pr_lease": {
"permission": "gitea.pr.comment",
"role": "merger",
},
# #691: guarded non-owner cleanup of obsolete comment-backed reviewer leases.
# Apply path posts lease release + audit comments (gitea.pr.comment).
+22 -16
View File
@@ -79,41 +79,46 @@ 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(RuntimeError) as ctx:
with self.assertRaises(gitea_auth.GiteaNetworkError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH)
self.assertIn("network error contacting Gitea", str(ctx.exception))
# Fixed message only (#699) — still a RuntimeError subclass.
self.assertIsInstance(ctx.exception, RuntimeError)
self.assertEqual(str(ctx.exception), "Network error contacting Gitea")
@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(RuntimeError) as ctx:
with self.assertRaises(gitea_auth.GiteaNetworkError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH)
self.assertIn("network error contacting Gitea", str(ctx.exception))
self.assertEqual(str(ctx.exception), "Network error contacting Gitea")
@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(RuntimeError) as ctx:
with self.assertRaises(gitea_auth.GiteaHttpError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH)
msg = str(ctx.exception)
self.assertIn("HTTP 502", msg)
self.assertIn("upstream unavailable", msg)
self.assertEqual(msg, "Gitea upstream unavailable")
self.assertEqual(ctx.exception.reason_code, "upstream_unavailable")
self.assertEqual(ctx.exception.http_status, 502)
@patch("gitea_auth.urllib.request.urlopen")
def test_503_upstream_message(self, mock_open):
mock_open.side_effect = http_error(503, "")
with self.assertRaises(RuntimeError) as ctx:
with self.assertRaises(gitea_auth.GiteaHttpError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH)
self.assertIn("HTTP 503", str(ctx.exception))
self.assertIn("upstream unavailable", str(ctx.exception))
self.assertEqual(str(ctx.exception), "Gitea upstream unavailable")
self.assertEqual(ctx.exception.http_status, 503)
@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 RuntimeError.
# Non-JSON garbage error body must still yield a clean typed error
# with a fixed message (no body echo — #699).
mock_open.side_effect = http_error(500, "<html>garbage</html>")
with self.assertRaises(RuntimeError) as ctx:
with self.assertRaises(gitea_auth.GiteaHttpError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH)
self.assertIn("HTTP 500", str(ctx.exception))
self.assertEqual(str(ctx.exception), "Gitea HTTP request failed")
self.assertNotIn("<html>", str(ctx.exception))
@patch("gitea_auth.urllib.request.urlopen")
def test_malformed_success_json_raises_clean_error(self, mock_open):
@@ -126,16 +131,17 @@ 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(RuntimeError) as ctx:
with self.assertRaises(gitea_auth.GiteaHttpError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH)
msg = str(ctx.exception)
self.assertNotIn("supersecret123", msg)
self.assertIn(gitea_audit.REDACTED, msg)
# Fixed message — body never appears (stronger than redaction).
self.assertEqual(msg, "Gitea HTTP request failed")
@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(RuntimeError) as ctx:
with self.assertRaises(gitea_auth.GiteaHttpError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH)
self.assertNotIn(FAKE_AUTH, str(ctx.exception))
+7 -1
View File
@@ -27,7 +27,13 @@ from task_capability_map import required_permission, required_role
DELETE_PROFILE = {
"profile_name": "prgs-author-delete",
"allowed_operations": ["gitea.read", "gitea.branch.delete"],
"role": "author",
"allowed_operations": [
"gitea.read",
"gitea.pr.create",
"gitea.branch.push",
"gitea.branch.delete",
],
"forbidden_operations": [],
"audit_label": "prgs-author-delete",
}
File diff suppressed because it is too large Load Diff
@@ -0,0 +1,499 @@
"""#720: Expired old-head KIND_DECISION_LOCK must not block fresh review.
Reproduction shape (PR #616):
* REQUEST_CHANGES terminal at head A
* open PR advanced to head B
* durable decision lock age > default 4h TTL
* fresh_review_on_current_head_allowed is true but unreachable under TTL-first reject
* mark_final fails with "session state expired after 4h"
* assessment may report "no lock" while the file remains on disk
Security invariants preserved:
* same-head second terminal remains fail-closed (#332/#620)
* REQUEST_CHANGES is never treated as approval
* historical mutations remain on the ledger
* merged/closed moot cleanup still allowed (#594)
* irrecoverable provenance still requires #709 authorization
* ordinary profiles do not gain gitea.decision_lock.irrecoverable_recovery
"""
from __future__ import annotations
import os
import tempfile
import unittest
from datetime import datetime, timedelta, timezone
from pathlib import Path
from unittest.mock import patch
import sys
ROOT = str(Path(__file__).resolve().parent.parent)
if ROOT not in sys.path:
sys.path.insert(0, ROOT)
import mcp_session_state as ss
import mcp_server
import stale_review_decision_lock as srdl
import task_capability_map
HEAD_A = "a0fffae576673ba7df7456b32e6aec916581bdfb"
HEAD_B = "a6a2243aad9c3e385fc70509f4941a0f0ec33162"
HEAD_SAME = HEAD_A
RC_616_A = {
"pr_number": 616,
"action": "request_changes",
"review_id": 443,
"review_state": "request_changes",
"head_sha": HEAD_A,
}
def _lock(mutations=None, **kwargs):
base = {
"task": "review_pr",
"kind": ss.KIND_DECISION_LOCK,
"remote": "prgs",
"org": "Scaled-Tech-Consulting",
"repo": "Gitea-Tools",
"session_pid": os.getpid(),
"session_profile": "prgs-reviewer",
"session_profile_lock": "prgs-reviewer",
"profile_identity": "prgs-reviewer",
"final_review_decision_ready": False,
"ready_pr_number": kwargs.get("ready_pr"),
"ready_action": kwargs.get("ready_action"),
"ready_expected_head_sha": kwargs.get("ready_head"),
"ready_remote": "prgs" if kwargs.get("ready_pr") else None,
"ready_org": "Scaled-Tech-Consulting" if kwargs.get("ready_pr") else None,
"ready_repo": "Gitea-Tools" if kwargs.get("ready_pr") else None,
"live_mutations": list(mutations or []),
"correction_authorized": False,
"correction_reason": None,
}
return base
def _open_pr(pr_number=616, head=HEAD_B, merged=False, closed=False):
state = "closed" if closed or merged else "open"
return {
"number": pr_number,
"state": state,
"merged": merged,
"merged_at": "2026-07-16T12:00:00Z" if merged else None,
"merge_commit_sha": "m" * 40 if merged else None,
"head": {"sha": head},
}
def _no_lease():
return {"block": False, "reasons": [], "mutation_allowed": True}
def _feedback(blocking=False, stale=True):
return {
"success": True,
"has_blocking_change_requests": blocking,
"review_feedback_stale": stale,
"current_head_sha": HEAD_B,
}
def _age_lock_payload(lock: dict, hours: float = 5.0) -> dict:
"""Rewrite timestamps to *hours* ago (simulates durable age without touching prod)."""
aged = (datetime.now(timezone.utc) - timedelta(hours=hours)).isoformat().replace(
"+00:00", "Z"
)
out = dict(lock)
out["recorded_at"] = aged
out["updated_at"] = aged
return out
class TestIssue720ExpiredDecisionLockLifecycle(unittest.TestCase):
def setUp(self):
self._tmp = tempfile.TemporaryDirectory()
self.env = patch.dict(
os.environ,
{
ss.STATE_DIR_ENV: self._tmp.name,
ss.SESSION_PROFILE_LOCK_ENV: "prgs-reviewer",
"GITEA_MCP_PROFILE": "prgs-reviewer",
"GITEA_PROFILE_NAME": "prgs-reviewer",
},
clear=False,
)
self.env.start()
mcp_server._REVIEW_DECISION_LOCK = None
import review_workflow_load
review_workflow_load.clear_review_workflow_load()
review_workflow_load.record_review_workflow_load(mcp_server.PROJECT_ROOT)
mcp_server.gitea_load_review_workflow()
def tearDown(self):
mcp_server._REVIEW_DECISION_LOCK = None
import review_workflow_load
review_workflow_load.clear_review_workflow_load()
self.env.stop()
self._tmp.cleanup()
def _persist_aged_lock(self, mutations, hours: float = 5.0, **kwargs):
"""Write decision lock aged > TTL to the temp durable store (test-only)."""
payload = _age_lock_payload(_lock(mutations, **kwargs), hours=hours)
saved = ss.save_state(
kind=ss.KIND_DECISION_LOCK,
payload=payload,
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
profile_identity="prgs-reviewer",
)
# save_state refreshes updated_at; re-age the on-disk envelope for TTL tests.
path = ss.state_file_path(
kind=ss.KIND_DECISION_LOCK,
profile_identity="prgs-reviewer",
state_dir=self._tmp.name,
)
aged = (datetime.now(timezone.utc) - timedelta(hours=hours)).isoformat().replace(
"+00:00", "Z"
)
import json
with open(path, encoding="utf-8") as fh:
envelope = json.load(fh)
envelope["recorded_at"] = aged
envelope["updated_at"] = aged
if isinstance(envelope.get("payload"), dict):
envelope["payload"]["recorded_at"] = aged
envelope["payload"]["updated_at"] = aged
with open(path, "w", encoding="utf-8") as fh:
json.dump(envelope, fh, indent=2, sort_keys=True)
fh.write("\n")
# Drop memory so subsequent loads hit durable store.
mcp_server._REVIEW_DECISION_LOCK = None
return saved
def _mark(self, pr, action, head, feedback=None):
with patch("mcp_server._list_pr_lease_comments", return_value=[]), patch(
"mcp_server._pr_work_lease_reviewer_block", return_value=_no_lease()
), patch.object(
mcp_server,
"gitea_get_pr_review_feedback",
return_value=feedback or _feedback(blocking=False, stale=True),
), patch.object(
mcp_server,
"gitea_check_pr_eligibility",
return_value={"eligible": True, "head_sha": head},
), patch.object(
mcp_server.mcp_daemon_guard,
"assert_sanctioned_mutation_runtime",
return_value=None,
), patch.object(
mcp_server.mcp_daemon_guard,
"assert_no_direct_import_bypass",
return_value=None,
):
return mcp_server.gitea_mark_final_review_decision(
pr_number=pr,
action=action,
expected_head_sha=head,
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
)
# --- AC regression matrix ---
def test_under_four_hours_fresh_review_at_b_allowed(self):
self._persist_aged_lock(
[RC_616_A],
hours=1.0,
ready_pr=616,
ready_head=HEAD_A,
ready_action="request_changes",
)
res = self._mark(616, "approve", HEAD_B)
self.assertTrue(res.get("marked_ready"), res)
def test_over_four_hours_fresh_review_at_b_still_allowed(self):
"""Primary #720 defect: age > TTL must not block head-B mark_final."""
self._persist_aged_lock(
[RC_616_A],
hours=5.0,
ready_pr=616,
ready_head=HEAD_A,
ready_action="request_changes",
)
# Prove durable load is possible for recovery-critical decision locks.
loaded = ss.load_state(
kind=ss.KIND_DECISION_LOCK,
profile_identity="prgs-reviewer",
)
self.assertIsNotNone(
loaded,
"expired KIND_DECISION_LOCK must remain loadable (not generic TTL cache)",
)
res = self._mark(616, "approve", HEAD_B)
self.assertTrue(
res.get("marked_ready"),
f"expected mark_ready on head B after >4h; got {res}",
)
reasons = " ".join(res.get("reasons") or [])
self.assertNotIn("session state expired", reasons)
def test_historical_review_at_a_preserved_and_stale(self):
self._persist_aged_lock(
[RC_616_A],
hours=5.0,
ready_pr=616,
ready_head=HEAD_A,
ready_action="request_changes",
)
res = self._mark(616, "approve", HEAD_B)
self.assertTrue(res.get("marked_ready"), res)
lock = mcp_server._load_review_decision_lock()
self.assertIsNotNone(lock)
hist = [m for m in lock["live_mutations"] if m.get("review_id") == 443]
self.assertEqual(len(hist), 1)
self.assertEqual(hist[0]["head_sha"], HEAD_A)
self.assertEqual(hist[0]["action"], "request_changes")
a = srdl.assess_stale_review_decision_lock(
lock, pr_live=_open_pr(616, HEAD_B)
)
self.assertTrue(a["stale_by_head"])
self.assertTrue(a["fresh_review_on_current_head_allowed"])
self.assertEqual(a["locked_head_sha"], HEAD_A)
def test_no_reuse_approval_from_head_a(self):
"""REQUEST_CHANGES at A is never an approval credential for B."""
self._persist_aged_lock([RC_616_A], hours=5.0)
lock = ss.load_state(
kind=ss.KIND_DECISION_LOCK, profile_identity="prgs-reviewer"
)
last = srdl.last_terminal_mutation(lock)
self.assertEqual(last.get("action"), "request_changes")
self.assertNotEqual(last.get("action"), "approve")
# Gate must still require a new mark/submit for head B; prior RC is not approve.
reasons = mcp_server.terminal_review_hard_stop_reasons(
616, "mark_ready", expected_head_sha=HEAD_B
)
self.assertEqual(reasons, [])
def test_second_terminal_mutation_at_b_same_run_blocked(self):
self._persist_aged_lock(
[RC_616_A],
hours=5.0,
ready_pr=616,
ready_head=HEAD_A,
ready_action="request_changes",
)
res = self._mark(616, "approve", HEAD_B)
self.assertTrue(res.get("marked_ready"), res)
# Record terminal approve at B (same run).
lock = mcp_server._load_review_decision_lock()
lock["final_review_decision_ready"] = True
lock["ready_pr_number"] = 616
lock["ready_action"] = "approve"
lock["ready_expected_head_sha"] = HEAD_B
mcp_server._save_review_decision_lock(lock)
mcp_server.record_live_review_mutation(616, "approve", review_id=999)
# Second terminal at same head B must hard-stop.
hard = mcp_server.terminal_review_hard_stop_reasons(
616, "mark_ready", expected_head_sha=HEAD_B
)
self.assertTrue(hard)
res2 = self._mark(616, "approve", HEAD_B)
self.assertFalse(res2.get("marked_ready"))
def test_open_pr_same_head_expired_still_fail_closed(self):
self._persist_aged_lock(
[RC_616_A],
hours=5.0,
ready_pr=616,
ready_head=HEAD_A,
ready_action="request_changes",
)
res = self._mark(616, "approve", HEAD_SAME)
self.assertFalse(res.get("marked_ready"), res)
self.assertTrue(
any("#332" in r or "already consumed" in r for r in (res.get("reasons") or [])),
res,
)
def test_merged_pr_moot_cleanup_still_allowed(self):
self._persist_aged_lock(
[RC_616_A],
hours=5.0,
ready_pr=616,
ready_head=HEAD_A,
ready_action="request_changes",
)
lock = ss.load_state(
kind=ss.KIND_DECISION_LOCK, profile_identity="prgs-reviewer"
)
a = srdl.assess_stale_review_decision_lock(
lock, pr_live=_open_pr(616, HEAD_A, merged=True)
)
self.assertTrue(a["is_moot"])
self.assertTrue(a["cleanup_allowed"])
self.assertFalse(a["fresh_review_on_current_head_allowed"])
def test_assessment_reports_expired_old_head_not_absent(self):
self._persist_aged_lock(
[RC_616_A],
hours=5.0,
ready_pr=616,
ready_head=HEAD_A,
ready_action="request_changes",
)
# Disk presence + load after lifecycle fix.
path = ss.state_file_path(
kind=ss.KIND_DECISION_LOCK,
profile_identity="prgs-reviewer",
state_dir=self._tmp.name,
)
self.assertTrue(os.path.exists(path))
loaded = ss.load_state(
kind=ss.KIND_DECISION_LOCK, profile_identity="prgs-reviewer"
)
self.assertIsNotNone(loaded)
inspect = ss.inspect_state_envelope(
kind=ss.KIND_DECISION_LOCK,
profile_identity="prgs-reviewer",
state_dir=self._tmp.name,
)
self.assertTrue(inspect.get("on_disk"))
self.assertTrue(inspect.get("has_payload"))
self.assertTrue(inspect.get("recovery_critical") or inspect.get("ttl_exempt"))
self.assertTrue(inspect.get("age_hours", 0) >= 4.0)
a = srdl.assess_stale_review_decision_lock(
loaded, pr_live=_open_pr(616, HEAD_B)
)
self.assertTrue(a["has_lock"])
self.assertNotIn("no review decision lock present", " ".join(a["reasons"]))
self.assertTrue(a["stale_by_head"])
self.assertEqual(a["last_terminal_action"], "request_changes")
def test_existing_serialized_records_compatible(self):
"""Pre-fix ledgers without recovery_critical flag still load via kind."""
payload = _age_lock_payload(_lock([RC_616_A]), hours=8.0)
payload.pop("recovery_critical", None)
# Manually write pre-#720-shaped envelope (kind only on envelope).
path = ss.state_file_path(
kind=ss.KIND_DECISION_LOCK,
profile_identity="prgs-reviewer",
state_dir=self._tmp.name,
)
import json
os.makedirs(self._tmp.name, exist_ok=True)
aged = payload["recorded_at"]
envelope = {
"kind": ss.KIND_DECISION_LOCK,
"remote": "prgs",
"org": "Scaled-Tech-Consulting",
"repo": "Gitea-Tools",
"profile_identity": "prgs-reviewer",
"session_profile_lock": "prgs-reviewer",
"recorded_at": aged,
"updated_at": aged,
"writer_pid": os.getpid(),
"payload": payload,
}
with open(path, "w", encoding="utf-8") as fh:
json.dump(envelope, fh, indent=2, sort_keys=True)
fh.write("\n")
loaded = ss.load_state(
kind=ss.KIND_DECISION_LOCK, profile_identity="prgs-reviewer"
)
self.assertIsNotNone(loaded)
self.assertEqual(loaded["live_mutations"][0]["review_id"], 443)
def test_decision_lock_is_recovery_critical_kind(self):
self.assertIn(ss.KIND_DECISION_LOCK, ss.RECOVERY_CRITICAL_KINDS)
def test_workflow_load_still_ttl_expires(self):
"""Do not make every session-state kind permanently TTL-exempt."""
aged = (datetime.now(timezone.utc) - timedelta(hours=5)).isoformat().replace(
"+00:00", "Z"
)
rec = {
"kind": ss.KIND_WORKFLOW_LOAD,
"recorded_at": aged,
"updated_at": aged,
"profile_identity": "prgs-reviewer",
"session_profile_lock": "prgs-reviewer",
}
reasons = ss.identity_match_reasons(
rec, profile_identity="prgs-reviewer"
)
self.assertTrue(any("expired" in r for r in reasons), reasons)
def test_irrecoverable_permission_not_on_ordinary_profiles(self):
perm = "gitea.decision_lock.irrecoverable_recovery"
# Capability map must not map ordinary author/reviewer/merger tasks to it.
for task in (
"create_issue",
"comment_issue",
"review_pr",
"merge_pr",
"lock_issue",
"create_pr",
):
req = task_capability_map.required_permission(task)
self.assertNotEqual(req, perm, msg=task)
def test_structured_error_when_same_head_expired(self):
self._persist_aged_lock(
[RC_616_A],
hours=5.0,
ready_pr=616,
ready_head=HEAD_A,
ready_action="request_changes",
)
res = self._mark(616, "approve", HEAD_A)
self.assertFalse(res.get("marked_ready"))
reasons = res.get("reasons") or []
self.assertTrue(reasons)
blob = " ".join(reasons)
# Actionable recovery text from hard-stop (not generic internal_error).
self.assertIn("#332", blob)
self.assertTrue(
"#620" in blob or "head moved" in blob or "new expected_head_sha" in blob
or "already consumed" in blob
)
self.assertNotIn("internal_error", blob.lower())
class TestIssue720InspectEnvelope(unittest.TestCase):
def setUp(self):
self._tmp = tempfile.TemporaryDirectory()
self.env = patch.dict(
os.environ,
{
ss.STATE_DIR_ENV: self._tmp.name,
ss.SESSION_PROFILE_LOCK_ENV: "prgs-reviewer",
},
clear=False,
)
self.env.start()
def tearDown(self):
self.env.stop()
self._tmp.cleanup()
def test_inspect_missing_file(self):
info = ss.inspect_state_envelope(
kind=ss.KIND_DECISION_LOCK,
profile_identity="prgs-reviewer",
state_dir=self._tmp.name,
)
self.assertFalse(info["on_disk"])
self.assertFalse(info["has_payload"])
if __name__ == "__main__":
unittest.main()
+9 -3
View File
@@ -1446,10 +1446,16 @@ class TestReviewPR(unittest.TestCase):
class TestDeleteBranch(unittest.TestCase):
DELETE_PROFILE = {
"profile_name": "test-deleter",
"allowed_operations": ["gitea.read", "gitea.branch.delete"],
"profile_name": "test-author-deleter",
"role": "author",
"allowed_operations": [
"gitea.read",
"gitea.pr.create",
"gitea.branch.push",
"gitea.branch.delete",
],
"forbidden_operations": [],
"audit_label": "test-deleter",
"audit_label": "test-author-deleter",
}
@patch("mcp_server.get_profile", return_value=DELETE_PROFILE)
+130
View File
@@ -0,0 +1,130 @@
"""Merger lease acquisition tests (#718)."""
import os
import sys
import unittest
from datetime import datetime, timezone
from unittest.mock import patch
sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent))
import gitea_mcp_server as mcp_server
import reviewer_pr_lease as leases
class TestMergerLeaseAcquireTool(unittest.TestCase):
def setUp(self):
mcp_server._preflight_whoami_called = True
mcp_server._preflight_capability_called = True
mcp_server._preflight_resolved_role = "merger"
leases.clear_session_lease()
@patch("gitea_mcp_server.api_request")
@patch("gitea_mcp_server._auth", return_value="token pass")
@patch("gitea_mcp_server._resolve", return_value=("dadeschools", "org", "repo"))
@patch("gitea_mcp_server.get_profile", return_value={"username": "merger-user", "profile_name": "prgs-merger", "allowed_operations": ["gitea.read", "gitea.pr.comment", "gitea.pr.merge"], "forbidden_operations": []})
@patch("gitea_mcp_server._fetch_pr_comments", return_value=[])
@patch("gitea_mcp_server._verify_role_mutation_workspace")
def test_acquire_merger_lease_success(self, _verify, _comments, _profile, _resolve, _auth, mock_api):
"""Merger lease acquisition succeeds when no reviewer lease exists."""
# api_request is called for PR state, then for POSTing comment
mock_api.side_effect = [
{"state": "open"}, # PR is open
{"id": 456}, # Posted comment ID
]
with patch.dict(os.environ, {"GITEA_MCP_PROFILE_NAME": "prgs-merger"}):
res = mcp_server.gitea_acquire_merger_pr_lease(
pr_number=718,
worktree="branches/issue-718",
session_id="merger-session",
)
self.assertTrue(res["success"])
self.assertTrue(res["acquired"])
self.assertEqual(res["comment_id"], 456)
self.assertEqual(res["session_id"], "merger-session")
# Verify lease body includes merger identity
args, kwargs = mock_api.call_args_list[-1]
self.assertEqual(args[0], "POST")
body = args[3]["body"]
self.assertIn("reviewer_identity: merger-user", body)
self.assertIn("profile: prgs-merger", body)
self.assertIn("phase: claimed", body)
# Post-mutation readback: lease is recorded in session
session_lease = leases._SESSION_LEASE
self.assertIsNotNone(session_lease)
self.assertEqual(session_lease["phase"], "claimed")
self.assertEqual(session_lease["session_id"], "merger-session")
# Verify lease provenance uses SOURCE_ACQUIRE_MERGER
provenance = session_lease.get("lease_provenance") or {}
self.assertEqual(provenance.get("source"), "gitea_acquire_merger_pr_lease")
@patch("gitea_mcp_server.api_request")
@patch("gitea_mcp_server._auth", return_value="token pass")
@patch("gitea_mcp_server._resolve", return_value=("dadeschools", "org", "repo"))
@patch("gitea_mcp_server.get_profile", return_value={"username": "merger-user", "profile_name": "prgs-merger", "allowed_operations": ["gitea.read", "gitea.pr.comment", "gitea.pr.merge"], "forbidden_operations": []})
@patch("gitea_mcp_server._fetch_pr_comments")
@patch("gitea_mcp_server._verify_role_mutation_workspace")
def test_acquire_merger_lease_fails_if_active_exists(self, _verify, _comments, _profile, _resolve, _auth, mock_api):
"""Acquisition fails closed if another session holds an active lease."""
active_body = leases.format_lease_body(
repo="org/repo",
pr_number=718,
issue_number=None,
reviewer_identity="other-user",
profile="prgs-reviewer",
session_id="other-session",
worktree="branches/review-1",
phase="claimed",
candidate_head="a" * 40,
target_branch="master",
target_branch_sha="b" * 40,
last_activity=datetime.now(timezone.utc),
)
_comments.return_value = [{"id": 1, "body": active_body, "user": {"login": "other-user"}}]
# PR is open
mock_api.return_value = {"state": "open"}
with patch.dict(os.environ, {"GITEA_MCP_PROFILE_NAME": "prgs-merger"}):
res = mcp_server.gitea_acquire_merger_pr_lease(
pr_number=718,
worktree="branches/issue-718",
session_id="merger-session",
)
self.assertFalse(res["success"])
self.assertFalse(res["acquired"])
self.assertIn("already has active reviewer lease", " ".join(res["reasons"]))
# No POST made
post_calls = [c for c in mock_api.call_args_list if c[0][0] == "POST"]
self.assertEqual(len(post_calls), 0)
# No partial state recorded
self.assertIsNone(leases._SESSION_LEASE)
@patch("gitea_mcp_server.get_profile", return_value={"username": "merger-user", "profile_name": "prgs-merger", "allowed_operations": ["gitea.read", "gitea.pr.comment", "gitea.pr.merge"], "forbidden_operations": []})
@patch("gitea_mcp_server._verify_role_mutation_workspace")
def test_acquire_merger_lease_fails_workspace_binding(self, _verify, _profile):
"""Fails closed if workspace verification throws RuntimeError."""
_verify.side_effect = RuntimeError("Branches-only mutation guard")
with patch.dict(os.environ, {"GITEA_MCP_PROFILE_NAME": "prgs-merger"}):
res = mcp_server.gitea_acquire_merger_pr_lease(
pr_number=718,
worktree="branches/issue-718",
session_id="merger-session",
)
self.assertFalse(res["success"])
self.assertFalse(res["acquired"])
self.assertIn("Branches-only mutation guard", res["reasons"][0])
self.assertIsNone(leases._SESSION_LEASE)
if __name__ == "__main__":
unittest.main()
+176 -6
View File
@@ -92,14 +92,19 @@ class TestMigrateProfiles(unittest.TestCase):
author = prgs_gitea["identities"]["author"]
self.assertEqual(author["username"], "jcwalker3")
self.assertEqual(author["auth"]["id"], "redacted-author-ref")
self.assertEqual(author["allowed_operations"], ["read", "comment"])
self.assertEqual(author["forbidden_operations"], ["approve", "merge"])
self.assertEqual(
author["allowed_operations"], ["gitea.read", "gitea.pr.comment"]
)
self.assertEqual(
author["forbidden_operations"],
["gitea.pr.approve", "gitea.pr.merge"],
)
reviewer = prgs_gitea["identities"]["reviewer"]
self.assertEqual(reviewer["role"], "reviewer")
self.assertEqual(reviewer["username"], "sysadmin")
self.assertEqual(reviewer["auth"]["id"], "redacted-reviewer-ref")
self.assertIn("merge", reviewer["allowed_operations"])
self.assertIn("gitea.pr.merge", reviewer["allowed_operations"])
def test_alias_generation(self):
"""Test that aliases are correctly generated to support old profile names."""
@@ -188,7 +193,7 @@ class TestMigrateProfiles(unittest.TestCase):
self.assertNotIn("token", stdout_output.lower())
def test_explicit_operations_are_preserved(self):
"""Explicit v1 permissions must not be replaced by role defaults."""
"""Explicit v1 permissions are canonicalized, not replaced by role defaults."""
v1_data = json.loads(json.dumps(self.v1_content))
v1_data["profiles"]["prgs-reviewer"]["allowed_operations"] = ["read"]
v1_data["profiles"]["prgs-reviewer"]["forbidden_operations"] = ["merge"]
@@ -198,8 +203,8 @@ class TestMigrateProfiles(unittest.TestCase):
v2_data["environments"]["prgs"]["services"]["gitea"]
["identities"]["reviewer"]
)
self.assertEqual(reviewer["allowed_operations"], ["read"])
self.assertEqual(reviewer["forbidden_operations"], ["merge"])
self.assertEqual(reviewer["allowed_operations"], ["gitea.read"])
self.assertEqual(reviewer["forbidden_operations"], ["gitea.pr.merge"])
def test_inferred_role_defaults_only_when_unambiguous(self):
"""Role defaults are allowed only for clear author/reviewer profiles."""
@@ -306,6 +311,171 @@ class TestMigrateProfiles(unittest.TestCase):
migrate_profiles.main()
self.assertEqual(cm.exception.code, 1)
def test_reconciler_profile_migration(self):
"""Legacy reconciler shorthands migrate to valid canonical operations."""
import gitea_config
import reconciler_profile
v1_data = {
"version": 1,
"profiles": {
"prgs-reconciler": {
"base_url": "redacted-prgs-service",
"username": "reconciler-agent",
"auth": {"type": "keychain", "id": "reconciler-ref"},
"execution_profile": "prgs-reconciler",
"allowed_operations": [
"read",
"pr.close",
"pr.comment",
"issue.comment",
"issue.close",
"gitea.branch.delete",
],
"forbidden_operations": [
"merge",
"approve",
"review",
"pr.create",
"branch.push",
"commit",
],
}
}
}
v2_data = migrate_profiles.migrate_v1_to_v2(v1_data)
reconciler = (
v2_data["environments"]["prgs"]["services"]["gitea"]
["identities"]["reconciler"]
)
self.assertEqual(reconciler["role"], "reconciler")
allowed = reconciler["allowed_operations"]
forbidden = reconciler["forbidden_operations"]
# No invalid shorthand remains
for bad in ("pr.close", "pr.comment", "issue.close", "read", "merge"):
self.assertNotIn(bad, allowed)
self.assertNotIn(bad, forbidden)
for required in (
"gitea.read",
"gitea.pr.close",
"gitea.pr.comment",
"gitea.issue.comment",
"gitea.issue.close",
"gitea.branch.delete",
):
self.assertIn(required, allowed)
# Production loader accepts every allowed op
self.assertEqual(
gitea_config.normalize_operation(required), required
)
self.assertEqual(v2_data["aliases"]["prgs-reconciler"], "prgs.gitea.reconciler")
assessment = reconciler_profile.assess_reconciler_profile(allowed, forbidden)
self.assertTrue(assessment["valid"])
self.assertTrue(migrate_profiles.validate_v2_data(v2_data))
def test_reconciler_profile_defaults(self):
"""Reconciler defaults are fully canonical and loader-valid."""
import gitea_config
import reconciler_profile
v1_data = {
"version": 1,
"profiles": {
"prgs-reconciler": {
"base_url": "redacted-prgs-service",
"username": "reconciler-agent",
"auth": {"type": "keychain", "id": "reconciler-ref"},
"execution_profile": "prgs-reconciler",
}
}
}
v2_data = migrate_profiles.migrate_v1_to_v2(v1_data)
reconciler = (
v2_data["environments"]["prgs"]["services"]["gitea"]
["identities"]["reconciler"]
)
self.assertEqual(reconciler["role"], "reconciler")
self.assertEqual(
reconciler["allowed_operations"],
migrate_profiles.RECONCILER_DEFAULT_ALLOWED,
)
self.assertEqual(
reconciler["forbidden_operations"],
migrate_profiles.RECONCILER_DEFAULT_FORBIDDEN,
)
for op in reconciler["allowed_operations"]:
self.assertEqual(gitea_config.normalize_operation(op), op)
self.assertTrue(op.startswith("gitea."))
assessment = reconciler_profile.assess_reconciler_profile(
reconciler["allowed_operations"],
reconciler["forbidden_operations"],
)
self.assertTrue(assessment["valid"])
self.assertNotIn(
"gitea.branch.delete", assessment["missing_recommended_operations"]
)
def test_reconciler_migration_idempotent_canonicalize(self):
"""Second canonicalize of already-canonical ops is a no-op."""
first = migrate_profiles.canonicalize_operations(
list(migrate_profiles.RECONCILER_DEFAULT_ALLOWED)
)
second = migrate_profiles.canonicalize_operations(first)
self.assertEqual(first, second)
self.assertEqual(first, list(migrate_profiles.RECONCILER_DEFAULT_ALLOWED))
def test_reconciler_missing_required_fails_visibly(self):
"""Missing gitea.pr.close after migration fails closed (not silent drop)."""
v1_data = {
"version": 1,
"profiles": {
"prgs-reconciler": {
"base_url": "redacted-prgs-service",
"username": "reconciler-agent",
"auth": {"type": "keychain", "id": "reconciler-ref"},
"execution_profile": "prgs-reconciler",
"allowed_operations": ["read", "gitea.branch.delete"],
"forbidden_operations": ["merge"],
}
},
}
with self.assertRaisesRegex(ValueError, "missing required"):
migrate_profiles.migrate_v1_to_v2(v1_data)
def test_unknown_operation_fails_visibly(self):
v1_data = {
"version": 1,
"profiles": {
"prgs-author": {
"base_url": "redacted-prgs-service",
"username": "jcwalker3",
"auth": {"type": "keychain", "id": "hidden-author-ref"},
"execution_profile": "prgs-author",
"allowed_operations": ["read", "not.a.real.op"],
"forbidden_operations": ["merge"],
}
},
}
with self.assertRaisesRegex(ValueError, "cannot be canonicalized"):
migrate_profiles.migrate_v1_to_v2(v1_data)
def test_role_inference_author_reviewer_merger_reconciler(self):
self.assertEqual(
migrate_profiles.infer_role("prgs-author", "prgs-author"), "author"
)
self.assertEqual(
migrate_profiles.infer_role("prgs-reviewer", "prgs-reviewer"),
"reviewer",
)
self.assertEqual(
migrate_profiles.infer_role("prgs-reconciler", "prgs-reconciler"),
"reconciler",
)
self.assertIsNone(
migrate_profiles.infer_role("prgs-merger", "prgs-merger")
)
if __name__ == "__main__":
unittest.main()
+36
View File
@@ -82,6 +82,42 @@ class TestReconcilerProfileModel(unittest.TestCase):
"reconciler",
)
def test_branch_delete_is_recommended_for_reconciler(self):
self.assertIn(
"gitea.branch.delete",
reconciler_profile.RECONCILER_RECOMMENDED_OPERATIONS,
)
self.assertNotIn(
"gitea.branch.delete",
reconciler_profile.RECONCILER_REQUIRED_OPERATIONS,
)
def test_reconciler_with_branch_delete_stays_valid(self):
allowed = PRGS_RECONCILER_ALLOWED + ["gitea.branch.delete"]
result = reconciler_profile.assess_reconciler_profile(
allowed,
PRGS_RECONCILER_FORBIDDEN,
)
self.assertTrue(result["is_reconciler_profile"])
self.assertTrue(result["valid"])
self.assertNotIn(
"gitea.branch.delete", result["missing_recommended_operations"]
)
self.assertEqual(
mcp_server._role_kind(allowed, PRGS_RECONCILER_FORBIDDEN),
"reconciler",
)
def test_reconciler_without_branch_delete_reports_missing_recommended(self):
result = reconciler_profile.assess_reconciler_profile(
PRGS_RECONCILER_ALLOWED,
PRGS_RECONCILER_FORBIDDEN,
)
self.assertTrue(result["valid"])
self.assertIn(
"gitea.branch.delete", result["missing_recommended_operations"]
)
if __name__ == "__main__":
unittest.main()
+9 -4
View File
@@ -122,9 +122,11 @@ class TestApiRequestRetry(unittest.TestCase):
sleep.assert_not_called()
def test_non_429_error_raises_immediately(self):
with self.assertRaises(RuntimeError) as ctx:
with self.assertRaises(gitea_auth.GiteaHttpError) as ctx:
_call([_http_error(500, body=b"boom")])
self.assertIn("HTTP 500", str(ctx.exception))
# Fixed message only (#699) — no response body echo.
self.assertEqual(str(ctx.exception), "Gitea HTTP request failed")
self.assertEqual(ctx.exception.http_status, 500)
def test_non_429_error_does_not_sleep(self):
sleep = MagicMock()
@@ -171,9 +173,12 @@ 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(RuntimeError) as ctx:
with self.assertRaises(gitea_auth.GiteaHttpError) as ctx:
_call(errors, sleep_func=sleep, max_retries=3)
self.assertIn("HTTP 429", str(ctx.exception))
# 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.assertEqual(sleep.call_count, 3)
def test_no_infinite_loop_when_always_429(self):
@@ -0,0 +1,139 @@
"""Documentation acceptance for the stable control runtime ADR (#615 / PR #616).
Enforces review 443 remediation:
* F1 operator guide / runbooks cross-link the ADR (issue #615 AC2).
* F2 LLM sessions are not instructed to kill/restart/relaunch MCP; process
restart is operator-owned; ADR is the authoritative split.
* F3 routine post-merge master-parity staleness has a sanctioned response
(stop report operator reload re-verify parity) and is not a full
promotion ledger requirement.
"""
from pathlib import Path
REPO_ROOT = Path(__file__).resolve().parent.parent
ADR = (
REPO_ROOT
/ "docs"
/ "architecture"
/ "mcp-stable-control-runtime-policy-adr.md"
)
ADR_REL = "architecture/mcp-stable-control-runtime-policy-adr.md"
ADR_BASENAME = "mcp-stable-control-runtime-policy-adr.md"
CROSS_LINK_DOCS = (
REPO_ROOT / "docs" / "wiki" / "Operator-Guide.md",
REPO_ROOT / "docs" / "wiki" / "Runbooks.md",
REPO_ROOT / "docs" / "llm-workflow-runbooks.md",
)
def _read(path: Path) -> str:
assert path.is_file(), f"missing {path.relative_to(REPO_ROOT)}"
return path.read_text(encoding="utf-8")
def test_adr_exists_with_policy_core():
text = _read(ADR)
assert text.lstrip().startswith("#"), "ADR lacks a title"
assert "#615" in text
assert "stable control runtime" in text.lower()
assert "2.3" in text and "2.4" in text and "2.5" in text and "2.6" in text
def test_f1_operator_docs_cross_link_adr():
for path in CROSS_LINK_DOCS:
text = _read(path)
assert ADR_BASENAME in text, (
f"{path.relative_to(REPO_ROOT)} must cross-link {ADR_BASENAME} "
f"(issue #615 acceptance criterion 2)"
)
def test_f1_adr_lists_cross_links_as_acceptance_not_optional_tooling():
text = _read(ADR)
# Acceptance section must require guide/runbook cross-links.
assert "Operator guide / runbooks cross-link" in text or (
"operator guide" in text.lower() and "cross-link" in text.lower()
and "Acceptance" in text
)
# Cross-link must not remain only as optional tooling item #4.
optional = text.split("## 5. Implementation follow-ups", 1)[-1].split(
"## 6. Acceptance", 1
)[0]
assert "Operator Guide wiki cross-link to this ADR" not in optional, (
"ADR §5 must not list operator-guide cross-link as optional tooling"
)
assert "Not optional" in text or "must** cross-link" in text.lower() or (
"must cross-link" in text.lower()
)
def test_f2_runbook_fallback_is_operator_owned_not_llm_restart():
runbooks = _read(REPO_ROOT / "docs" / "llm-workflow-runbooks.md")
# Forbidden historical self-service instruction.
forbidden = (
"the LLM must relaunch or restart the client/MCP with the correct "
"profile environment variable before claiming or working on any tasks"
)
assert forbidden not in runbooks, (
"llm-workflow-runbooks must not instruct the LLM to relaunch/restart MCP"
)
assert "operator-owned" in runbooks.lower() or "Operator-owned" in runbooks
assert ADR_BASENAME in runbooks
def test_f2_workspace_rebind_does_not_require_llm_process_relaunch():
runbooks = _read(REPO_ROOT / "docs" / "llm-workflow-runbooks.md")
section = runbooks.split("### Safe reconnect / rebind procedure", 1)[-1]
section = section.split("## Safety notes", 1)[0]
# Must not tell the LLM alone to relaunch the MCP process as step 2.
assert "Reconnect or relaunch the correct namespace MCP server from the intended" not in section
assert "Operator-owned" in section or "operator" in section.lower()
assert "worktree_path" in section
collapsed = " ".join(section.lower().split())
assert "client reconnect" in collapsed
def test_f2_adr_forbids_llm_restart_and_supersedes_self_service_relaunch():
text = _read(ADR)
lower = text.lower()
assert "must not" in lower and "restart" in lower
assert "operator" in lower and "reload" in lower
assert "supersedes" in lower
assert "llm" in lower
def test_f3_adr_defines_post_merge_parity_staleness_response():
text = _read(ADR)
lower = text.lower()
assert "2.6" in text
assert "post-merge" in lower or "post merge" in lower
assert "parity" in lower and "stale" in lower
# Sanctioned steps: stop, report, operator reload, re-verify.
assert "stop" in lower
assert "report" in lower
assert "operator" in lower
assert "parity is verified" in lower or "re-verif" in lower or (
"startup/current-head" in lower
)
# Not a full promotion ledger for routine reload.
assert "not a §2.4 promotion" in lower or "not a section 2.4 promotion" in lower or (
"not a §2.4" in text or "Not a §2.4 promotion" in text
)
# Stale parity listed among unhealthy triggers.
assert "master parity is stale" in lower or "stale master parity" in lower
def test_f3_adr_forbids_llm_bypass_of_parity_gate():
text = _read(ADR)
lower = text.lower()
assert "bypass" in lower or "self-reset" in lower or "self-service" in lower
assert "parity" in lower
def test_cross_links_do_not_embed_secrets_or_raw_hosts_in_wiki_snippets():
for path in CROSS_LINK_DOCS:
text = _read(path)
for marker in ("ghp_", "BEGIN PRIVATE KEY", "Authorization: Bearer"):
assert marker not in text, f"{path} contains {marker!r}"
+426
View File
@@ -0,0 +1,426 @@
"""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()
@@ -0,0 +1,205 @@
"""Invariant tests pinning task_capability_map role assignments (#722/#723).
Added with the operator-authorized break-glass repair for incident #722:
commit 970e68b remapped ten reviewer tasks to ``role="merger"`` while every
configured merger profile forbids the review permissions, so no configured
profile could resolve any formal review task (``matching_configured_profile``
was empty repository-wide). These tests fail loudly if that class of
regression recurs:
- every role-exclusive formal-review task must be satisfiable by at least one
canonical role profile (permission AND role together);
- the capability map must agree with ``role_session_router`` task sets;
- ``adopt_merger_pr_lease`` stays merger-only (the legitimate hunk of
970e68b, preserved by the repair);
- merger profiles must not be able to resolve review_pr/approve_pr.
"""
import unittest
import gitea_config
from role_session_router import MERGER_TASKS, REVIEWER_TASKS
from task_capability_map import required_permission, required_role
# Canonical role-profile permission shape. Mirrors the configured
# author/reviewer/merger/reconciler profiles (profiles.json v2 role split):
# reviewers review/approve/request changes but never merge; mergers merge but
# never review/approve/request changes.
CANONICAL_ROLE_PROFILES = {
"author": {
"allowed": [
"gitea.read",
"gitea.branch.create",
"gitea.branch.push",
"gitea.repo.commit",
"gitea.pr.create",
"gitea.pr.comment",
"gitea.issue.create",
"gitea.issue.comment",
"gitea.issue.close",
],
"forbidden": [
"gitea.pr.approve",
"gitea.pr.request_changes",
"gitea.pr.merge",
],
},
"reviewer": {
"allowed": [
"gitea.read",
"gitea.pr.review",
"gitea.pr.approve",
"gitea.pr.request_changes",
"gitea.pr.comment",
"gitea.issue.comment",
],
"forbidden": [
"gitea.branch.create",
"gitea.branch.push",
"gitea.repo.commit",
"gitea.pr.create",
"gitea.pr.merge",
],
},
"merger": {
"allowed": [
"gitea.read",
"gitea.pr.merge",
"gitea.pr.comment",
"gitea.issue.comment",
],
"forbidden": [
"gitea.branch.create",
"gitea.branch.push",
"gitea.repo.commit",
"gitea.pr.create",
"gitea.pr.approve",
"gitea.pr.review",
"gitea.pr.request_changes",
],
},
"reconciler": {
"allowed": [
"gitea.read",
"gitea.pr.close",
"gitea.pr.comment",
"gitea.issue.comment",
"gitea.branch.delete",
"gitea.decision_lock.irrecoverable_recovery",
],
"forbidden": [
"gitea.pr.approve",
"gitea.pr.merge",
"gitea.pr.review",
"gitea.pr.request_changes",
"gitea.pr.create",
"gitea.branch.create",
"gitea.branch.push",
"gitea.repo.commit",
],
},
}
# Role-exclusive formal-review tasks (mirrors the resolver's role-exclusive
# handling for review work): permission alone is not enough — the profile's
# role kind must also match, so both dimensions are pinned here.
FORMAL_REVIEW_TASKS = (
"review_pr",
"approve_pr",
"request_changes_pr",
"blind_pr_queue_review",
"pr_queue_cleanup",
"pr-queue-cleanup",
)
def _profile_satisfies(role_name, task):
"""True when the canonical *role_name* profile can perform *task*."""
profile = CANONICAL_ROLE_PROFILES[role_name]
ok, _reason = gitea_config.check_operation(
required_permission(task), profile["allowed"], profile["forbidden"]
)
return ok and role_name == required_role(task)
class TestFormalReviewProfileCoverage(unittest.TestCase):
"""#722: some configured profile must be able to formally review."""
def test_every_formal_review_task_has_a_satisfying_role_profile(self):
for task in FORMAL_REVIEW_TASKS:
with self.subTest(task=task):
satisfying = [
role
for role in CANONICAL_ROLE_PROFILES
if _profile_satisfies(role, task)
]
self.assertTrue(
satisfying,
f"no canonical role profile satisfies both permission "
f"{required_permission(task)!r} and role "
f"{required_role(task)!r} for task {task!r} — formal "
f"review would be impossible for every configured "
f"profile (incident #722)",
)
def test_formal_review_tasks_are_reviewer_role(self):
for task in FORMAL_REVIEW_TASKS:
with self.subTest(task=task):
self.assertEqual(required_role(task), "reviewer")
class TestMapRouterAgreement(unittest.TestCase):
"""#723 AC2: the map and the role session router must not drift."""
def test_reviewer_tasks_map_to_reviewer_role(self):
for task in sorted(REVIEWER_TASKS):
with self.subTest(task=task):
self.assertEqual(
required_role(task),
"reviewer",
f"router classifies {task!r} as a reviewer task but the "
f"capability map assigns role {required_role(task)!r}",
)
def test_merger_tasks_map_to_merger_role(self):
for task in sorted(MERGER_TASKS):
with self.subTest(task=task):
self.assertEqual(
required_role(task),
"merger",
f"router classifies {task!r} as a merger task but the "
f"capability map assigns role {required_role(task)!r}",
)
class TestMergerBoundary(unittest.TestCase):
"""Preserve the legitimate hunk of 970e68b and the merger fence."""
def test_adopt_merger_pr_lease_requires_merger_role(self):
self.assertEqual(required_role("adopt_merger_pr_lease"), "merger")
self.assertEqual(
required_permission("adopt_merger_pr_lease"), "gitea.pr.comment"
)
def test_merge_pr_requires_merger_role(self):
self.assertEqual(required_role("merge_pr"), "merger")
self.assertEqual(required_permission("merge_pr"), "gitea.pr.merge")
def test_merger_profile_cannot_resolve_formal_review_tasks(self):
merger = CANONICAL_ROLE_PROFILES["merger"]
for task in ("review_pr", "approve_pr", "request_changes_pr"):
with self.subTest(task=task):
ok, reason = gitea_config.check_operation(
required_permission(task),
merger["allowed"],
merger["forbidden"],
)
self.assertFalse(
ok,
f"merger profile must not hold {task!r} permission "
f"(got reason {reason!r})",
)
if __name__ == "__main__":
unittest.main()